Repository navigation
test(agents): pin the fail-closed launch contract without a session manager - #2681
Conversation
…anager gentle-shell#1709 asked the launch seam to treat a missing session manager as an unbound session. The #1991 fail-closed contract instead blocks the launch with the session-profile authority message, and the authority reader already swallows an unobservable manager instead of throwing a TypeError. Pin that behavior with a direct subagent_run test so the clean failure cannot regress into a crash.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Missing session manager launch check tests/gentle-agents.test.ts |
Adds a test that configures a pinned profile, removes sessionManager, and expects subagent_run to reject with the specified authority error. |
Priority:
Estimated code review effort: 1 (Trivial) | ~4 minutes
Change: Other
Possibly related PRs
- Gentleman-Programming/gentle-shell#1991: Adds the session profile authority check that this test exercises.
Suggested labels: type:bug
Suggested reviewers: alan-thegentleman
Merge Risk: 
1fedf
-
This test-only change is mergeable; it adds coverage for managerless subagent launches.
Pre-merge checks: 
-
Check name Status Explanation Resolution Linked Issues check Issue #1709(open, direct) requires that a missing sessionManager resolves to an unbound session and that the launch seam guards the read. At head, lib/session-profile-authority.ts readSessionProfileA…Choose one. (a) Change the subagent_run and subagent_continue launch path so an absent sessionManager resolves to an unbound session, as #1709requests. Make the new test assert that outcome. (b) Have the owner of#1709record in the issue …-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the added regression test for the fail-closed launch behavior when no session manager is available. Out of Scope Changes check The PR summary describes one added regression test in tests/gentle-agents.test.ts (+20/-0). The test asserts that subagent_run rejects a launch when the context has no session manager, with the messag… Docstring Coverage No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Full details: Out of Scope Changes check
-
Explanation
The PR summary describes one added regression test in tests/gentle-agents.test.ts (+20/-0). The test asserts that subagent_run rejects a launch when the context has no session manager, with the message 'Session profile authority unavailable'. It relates directly to the launch-authority behavior named in
#1709. The PR reports no production code changes. The test diff could not be read because the changed blobs are missing from the checkout. This verdict therefore rests on the PR summary and description.
Full details: Docstring Coverage
-
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 1 files.
-
Full details: Linked Issues check
-
Explanation
Issue
#1709(open, direct) requires that a missing sessionManager resolves to an unbound session and that the launch seam guards the read. At head, lib/session-profile-authority.ts readSessionProfileAuthority wraps the manager access in try/catch. With no manager, the TypeError is caught and the function returns { available: false }. requireSessionProfileAuthority then throws 'Session profile authority unavailable; affected launches are blocked...'. The launch is blocked. It is not treated as an unbound session. The raw TypeError symptom is gone, but the expected outcome in#1709is not met. The PR description says#1991superseded the unbound-session outcome. No#1991evidence is in the supplied material.#1709remains open with its expected outcome unchanged. The added test pins the blocked behavior, so it does not implement the#1709requirement. I did not inspect the launch call sites in extensions/gentle-agents.ts beyond the first 16000 bytes. The call-order claim rests on the PR description.Resolution
Choose one. (a) Change the subagent_run and subagent_continue launch path so an absent sessionManager resolves to an unbound session, as
#1709requests. Make the new test assert that outcome. (b) Have the owner of#1709record in the issue that#1991replaced the unbound-session requirement, with a link to the#1991evidence. Then re-assess against the updated requirement. Do not close#1709from this PR until one of these is done.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
Summary
Closes #1709 by pinning the current, deliberate behavior instead of changing it.
The issue reported that a context without a
sessionManagermade the launch seam crash with an unguardedctx.sessionManager.getSessionId()read instead of treating the session as unbound. On currentmainthat defect no longer exists, and the desired outcome deliberately changed shape:subagent_runandsubagent_continue) callrequireSessionProfileAuthoritybeforebuildRequest, so the profile gate runs first.readSessionProfileAuthorityswallows an unobservable manager (inner try/catch) and returns{ available: false }, so the launch fails with the clean fail-closed message from feat(profiles): gate subagent launches on session profile authority #1991 instead of a TypeError.What was actually missing was coverage, so this PR adds one test: a
subagent_runlaunch with no session manager must reject with the session-profile authority message. Test-only change, no production code.Verification
Cannot read properties of undefined (reading 'getSessionId')), confirming the pin guards the reported defect.tests/gentle-agents.test.ts: 276 pass, 0 fail.Risk: none
Summary by CodeRabbit