Repository navigation
test(cli): cover ACP follow's stale-attach guard - #154
Conversation
Add a regression test for the guard in turn.follow (packages/cli/src/acp/turn.ts) that stops a stale session/load from interrupting the newer attachment's follower. The test holds load A's MCP registration via the wire fixture's fetch override, lets load B attach and follow, then releases A and asserts a later permission ask is still answered by B's follower. Verified: fails with the guard removed (1 fail, the new test), passes with it; 10/10 under --rerun-each 10; full ACP suite 116/116; cli typecheck clean. Closes #125
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 26 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 72 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
The following comment was made by an LLM, it may be inaccurate: |
Problem
#125: the stale-attach guard in
follow(packages/cli/src/acp/turn.ts, added by PR #58) had no test — deleting it left all 115 ACP tests passing. Without it, an overlappingsession/loadwhose MCP registration finishes after a newer load of the same session stops the newer attachment's follower, so later asks from that session reach the client only on the next prompt/load/resume.Solution
One new test in
packages/cli/test/acp/turn.test.ts: "a stale load finishing after a newer load does not stop the newer follower".PUT /api/experimental/mcp/toolsvia the wire fixture'sfetchoverride (the fixture records the request before awaiting the override, so the test observes the held registration viaserver.requests).permissionAskedevent and asserts the client still receives the reply from B's follower.Test-only change; no production code touched.
Verification
turn.ts, the suite shows exactly 1 failure — this test (15 pass / 1 fail). With the guard, 16/16 inturn.test.ts.--rerun-each 10.packages/clitypecheck clean; prettier clean.Closes #125
Need help on this PR? Tag
@codesmith-botwith what you need.