fix(t2b): no success receipt for a submit that never landed or a pane that never closed (#484, #485) - #487
Conversation
… that never closed Two live-reproduced silent failures, both the same disease: ok:true for an action the engine did not perform. #484 — send_to(mode:"key") - submit_attempted was computed by exact match on `key === "return"`, so "Enter", "Return", "KPEnter", "ctrl-m" and a raw CR all dispatched a real submit and then reported submit_attempted:false. The documented type -> verify -> Return recovery therefore returned a success receipt whose own fields said nothing had been attempted. - normalizeKeyName now canonicalizes every submit alias to "return", and isSubmitKey() drives the receipt. - The key receipt now carries key_dispatched:true — sendKeyWithRetry throws when nothing reached the pane, so the ok path has dispatch evidence and now states it instead of leaving the caller to infer it from bytes:0. - A submit key is verified: verifySubmitKeyOutcome reads the pane and asks the only question that matters — did the composer let go of its contents? Composer still populated => send_key returns an error with submit_verification_reason:"composer_still_populated", not ok:true. Unreadable screen => submit_verified:null with a stated reason. #485 — close_surface(scope:"agent") - The handler stopped the agent and returned stop_agent's receipt verbatim (ok:true, state:"done") for a tool named close_surface, while the pane stayed open. It now resolves the bound surface before the stop (the stop can evict the record), stops, then closes the pane for real. - The receipt reports the two halves separately: agent_stopped and surface_closed. Agent stopped but pane survived => error naming both halves. No surface bound => ok with surface_close_skipped:"no_surface_bound". - Cross-checked scope:"workspace": delete_workspace does perform its named action; that branch now states workspace_deleted explicitly. - close_surface's description says agent scope closes the pane and reports the halves separately. Tests: tests/t2b-silent-failures.test.ts (14). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_2bf7be3d-810a-4112-b53c-c257dcd48d7c) |
|
Warning Review limit reached
Next review available in: 7 minutes Limit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| let lastComposerInput: string | null = null; | ||
|
|
||
| while (Date.now() - startedAt < timeoutMs) { | ||
| const snapshot = await readParsedSurface(opts.surface, opts.workspace); |
There was a problem hiding this comment.
🟡 Medium src/server.ts:5071
verifySubmitKeyOutcome reads the surface in a polling loop without ever calling the route's assertCurrent guard. During the 1.5 s verification window the stable UUID can move to a new ref (or the mutable ref can be recycled), so the verifier may read a different terminal and report its empty composer as submit_verified:true for the original target — a false positive that silently claims delivery succeeded.
Compare with verifySubmitAfterEnter, which accepts and invokes opts.beforeMutation before every readParsedSurface call. The same pattern should be applied here: accept a beforeMutation parameter, call it before each poll read, and thread opts.beforeMutation through from the executeDeliveryEngine call site (~line 5153).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 5071:
`verifySubmitKeyOutcome` reads the surface in a polling loop without ever calling the route's `assertCurrent` guard. During the 1.5 s verification window the stable UUID can move to a new ref (or the mutable ref can be recycled), so the verifier may read a *different* terminal and report its empty composer as `submit_verified:true` for the original target — a false positive that silently claims delivery succeeded.
Compare with `verifySubmitAfterEnter`, which accepts and invokes `opts.beforeMutation` before every `readParsedSurface` call. The same pattern should be applied here: accept a `beforeMutation` parameter, call it before each poll read, and thread `opts.beforeMutation` through from the `executeDeliveryEngine` call site (~line 5153).
| * its contents? Positive evidence either way is reported; absence of | ||
| * evidence is reported as absence, never as success. | ||
| */ | ||
| const verifySubmitKeyOutcome = async (opts: { |
There was a problem hiding this comment.
🟡 Medium src/server.ts:5057
verifySubmitKeyOutcome reads the screen only after the key is sent, with no baseline snapshot taken before. If the composer was already empty or the agent was already in a working status before the key was dispatched, the function returns submit_verified: true even though the key submitted nothing. This is exactly the false-success receipt #484 set out to eliminate — a no-op Return reports success and the lead believes the message was delivered.
The fix is to capture a baseline snapshot (composer contents and/or parsed status) before sending the key, then require evidence of a transition (e.g., composer went from populated to empty, or status changed from idle to working) rather than treating any post-key empty composer as proof.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 5057:
`verifySubmitKeyOutcome` reads the screen only *after* the key is sent, with no baseline snapshot taken before. If the composer was already empty or the agent was already in a `working` status before the key was dispatched, the function returns `submit_verified: true` even though the key submitted nothing. This is exactly the false-success receipt #484 set out to eliminate — a no-op Return reports success and the lead believes the message was delivered.
The fix is to capture a baseline snapshot (composer contents and/or parsed status) *before* sending the key, then require evidence of a *transition* (e.g., composer went from populated to empty, or status changed from idle to working) rather than treating any post-key empty composer as proof.
| string, | ||
| unknown | ||
| >; | ||
| if (closeResult.isError === true) { |
There was a problem hiding this comment.
🟠 High src/server.ts:9691
close_surface(scope="agent") always reports surface_closed: false even when the agent and its surface were successfully torn down. engine.stopAgent already calls client.closeSurface and blocks until surfaceGone is confirmed, so the subsequent delegated closeHandler({ scope: "surface", surface: boundSurface, ... }) call tries to resolve a now-absent surface, fails, and hits the closeResult.isError === true branch every time. The result is that a fully successful stop-and-close is consistently misreported as agent_stopped: true, surface_closed: false, surface_close_error: ....
The fix is to treat a gone surface after a successful stopAgent as success rather than failure — for example, by checking whether the surface actually still exists before reporting the close as failed.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 9691:
`close_surface(scope="agent")` always reports `surface_closed: false` even when the agent and its surface were successfully torn down. `engine.stopAgent` already calls `client.closeSurface` and blocks until `surfaceGone` is confirmed, so the subsequent delegated `closeHandler({ scope: "surface", surface: boundSurface, ... })` call tries to resolve a now-absent surface, fails, and hits the `closeResult.isError === true` branch every time. The result is that a fully successful stop-and-close is consistently misreported as `agent_stopped: true, surface_closed: false, surface_close_error: ...`.
The fix is to treat a gone surface after a successful `stopAgent` as success rather than failure — for example, by checking whether the surface actually still exists before reporting the close as failed.
Review — T2b (#484, #485): ITERATERan in the worktree at Gates (ask 6) — both green, and the PR's claim matches:
BLOCKER — #484 still returns
|
Review addendum — MINIMALITY (YAGNI + readability)Second pass on #487 against the criterion added this round. Judged as "was anything built for a Headline: the ratio is largely earned, and minimality is not why this is an ITERATE. The
The 55 comment lines are 25% of the src additions and every one of them explains why — the AIDEV Also checked and clean: no tests assert implementation shape. What I am NOT flagging, so it does not get "fixed"
YAGNI — built for a future nobody asked for
Readability
Verdict on the ratioEarned, with named deletions. Two small bugs in +640 sounds bad until you separate it: the src — cmuxlayerClaude-reviewer-487 (worker) · claude-code/claude-opus-5 |
Round 2, addressing the ITERATE on #487. BLOCKER (#484) — `verifySubmitKeyOutcome` checked status BEFORE, and unguarded by, the composer. On a busy target — status working while the composer still visibly held the unsent text, which is the reported scenario — the status branch fired and the composer check never ran, returning `ok:true, submit_verified:true` for a message still on screen. Worse than the null it replaced: null admits ignorance, true asserts an observation the pane contradicts. - A populated composer now vetoes and keeps polling; nothing else can resolve the verification while the pane still shows the input. Mirrors the text path's `!hasPendingSubmitEvidence` gate at server.ts:4878. - A "working" status is no longer proof at all for a key send. The reported target was ALREADY working, so status cannot distinguish "my submit started a turn" from "a turn was already running" — and a boxed composer reads as unreadable, so accepting status would resurrect the same false-true through the extractor's blind spot. An empty, readable composer is the only positive proof; anything else is submit_verified:null with a stated reason. - Missing fixture added: working status crossed with a populated composer. Review items taken: - normalizeKeyName no longer rewrites the key sent to cmux. isSubmitKey alone makes the receipt truthful, and rewriting silently changed the bytes cmux receives for aliases nobody reported — "\n" especially, which is how a composer expresses shift+enter. normalizeKeyName is now byte-identical to base; raw "\r"/"\n" dropped from the submit set for the same reason. - Dropped the unreachable timeout_ms knob; the constant is used directly. - submit_verification_reason is stated unconditionally instead of via a conditional spread. - close_surface uses the house idiom for the registry lookup. - close_surface(scope:"agent") passes the caller's own force through instead of forcing unconditionally: stop_agent has no liveness refusal, so forcing here let an UNFORCED call tear down a live agent's pane through a guard that could never fire. Tool description says the pane close obeys the same guard. - Tests use tests/helpers/mcp-tool-harness.ts; the close tests assert the pane is gone from list_surfaces, not only that a CLI command ran. 17 tests, 16 red against base and the 17th red against the round-1 code it guards. Output in the PR body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_5d930703-5420-4269-adbe-ee6a1d46b583) |
Round-2 re-review (92caff1) — ITERATE, narrowly: one vacuous test, no src change requiredEvery item from round 1 is taken, and the blocker is genuinely fixed. The ITERATE is one test that Gates: Red-on-red sweep — 16 of 17 tests proven, 1 notI reverted each fix in turn against the new suite. Every line below is real output.
The one ITERATE item
It passes for the wrong reason. In this harness
Cheapest fix that makes it non-vacuous: assert Everything else: confirmed fixed
Operational note for whoever merges — read this before assuming the live pain is goneFor a For a worker whose registry row is still live, round 2 now deliberately leaves the pane open VerdictITERATE — one test, no src change. If the lead would rather ship the src now given the live pain, — cmuxlayerClaude-reviewer-487 (worker) · claude-code/claude-opus-5 |
…ing" Round 3, following the RESOLVED scope correction on #485. Two reframes landed mid-work; this commit follows the third and final one and drops what the second one asked for. The mechanism is the scope argument, confirmed in source and never a timing effect: pre-fix, scope:"agent" delegated to stop_agent and no path in that branch closed the surface. Every field report fits with zero latency once you sort them by which scope the lead called. - Target 1 (agent scope closes the pane, or says it did not) was already the shape of this branch since round 1. What is new: surface_closed is now an OBSERVATION. After the CLI returns, findSurfaceByRef confirms the pane is actually gone; if cmux still lists it, the receipt reports surface_closed:false with a WARNING instead of inferring closure from a call that returned. Agent scope forwards that observation rather than restating it. - Target 4 (independent of scope): close_surface marked matching records user_killed:true but never transitioned their state, so list_agents kept reporting a closed agent as "working" — golemsClaude's datum. An acknowledged close now transitions a non-terminal record to done in the same breath. The test that caught this first passed VACUOUSLY: list_agents nests state as {value}, so /"state":\s*"working"/ never matched. Assertion fixed to read state.value, which reproduced the defect. - Target 3: the close tests assert against list_surfaces, not the return value. REMOVED, because the latency theory it was built on is withdrawn: the SURFACE_CLOSE_SETTLE_* constants, the post-ack polling loop, the nonterminal "closing" state, close_latency_ms, and the eventual-consistency test fixtures. No delay window is characterised anywhere in this diff. NOT done as literally specified, and flagged rather than silently skipped: target 2 asks the description to say "agent-scope does not close panes". That sentence is false against this branch — target 1's first option was taken, so agent scope DOES close the pane. Writing it would make the tool description lie, which is the disease this lane exists to fix. The description states what the code does instead. See the PR body. 19 tests, 18 red against base; the 19th red against sabotage. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_971a1df5-5ac8-46ee-a542-0f29e4239964) |
Round-3 re-review (8c154e0) — ACCEPTThis is the fix for the pile-up. Merge it. Two follow-ups named below — neither touches the pile-up Gates: Against your five criteria(1)
(2) Tool description — PASS on intent, and I'd keep it as written. Your criterion says the (3) Tests assert against (4) (5) No delay-window code — PASS, verified across the cumulative diff. Every Red-on-red, cumulative across all three roundsNineteen of twenty tests go red under targeted sabotage (A, C, D, E, F, G, H, I, J from rounds 1–2; K, Follow-up 1 (one line, does NOT affect the pile-up) — a failed close marks a live agent
|
1 participant
Closes #484. Closes #485.
Lane T2b of
docs.local/plan/truth-v3. Both defects reproduced live; both break fleet operation now.Governing rule: no tool may return
ok:truefor an action it did not perform; a half-action must name which half.What changed because of the #485 reframes
This issue was re-diagnosed twice while this PR was in flight, and the second diagnosis was withdrawn. Recording it because I built against it:
closingSURFACE_CLOSE_SETTLE_*constants, post-ack polling loop,closingstate,close_latency_msscopeargument, confirmed in source; all timing theories withdrawnReframe A cost about half an hour of work that is now reverted. Net effect of the whole detour on the shipped diff: two things survive it, and both are improvements the original framing did not ask for —
surface_closedis an OBSERVATION, not an inference. After the CLI returns,findSurfaceByRefconfirms the pane is actually gone. If cmux still lists it, the receipt sayssurface_closed:falsewith a WARNING rather than treating "the call returned" as "the pane closed". One check, no waiting, no window.list_agents"working" datum got fixed — see target 4 below. That one was called out as standing regardless of latency, and it did.The blocker, and why round 1 was wrong
verifySubmitKeyOutcomechecked status before, and unguarded by, the composer:On a busy target — status
workingwhile the composer still visibly holds the unsent text — the first branch fired and the second never ran. That is the reported scenario exactly: a lead relaying intogolemsClaude, which was working on its previous turn. Round 1 shippedsubmit_verified:truefor a message still sitting on screen, which is worse than thenullit replaced — null admits ignorance,trueasserts an observation the pane contradicts. The reviewer's probe caught it; 14 green tests did not, because the suite hadIDLE + populatedandWORKING + emptyand never crossed them.Fixed in the dispatch path, two changes:
!hasPendingSubmitEvidencegate atsrc/server.ts:4878, as the review asked.workingstatus is no longer proof at all for a key send. The status branch is gone, not merely reordered. Two reasons, and the second is the reviewer's own boxed-composer note: the reported target was already working, so status cannot distinguish "my submit started a turn" from "a turn was already running"; and a composer that renders boxed reads asnullfrom the extractor, so keeping the status branch would resurrect the identical false-truethrough that blind spot. An empty, readable composer is now the only positive proof. Everything else issubmit_verified:nullwith a stated reason — honest, never a false success.The crossed fixture is now pinned:
lets a populated composer veto a working status instead of losing the race to it, pluswill not treat a working status as proof when the composer cannot be read.RED ON RED
Every test shown failing first. Src reverted to base
269afbd, tests unchanged:16 of 17 red on revert. The 17th is called out rather than buried:
dispatches the caller's key verbatimis a guard test — it passes against base because base also does not rewrite the key. It exists to stop the code returning to what round 1 shipped, so it is red against sabotage instead: reintroduce theisSubmitKey → "return"branch innormalizeKeyNameandEach test passes for its claimed reason: the alias cases assert
submit_attemptedandkey_dispatched(either alone is not red forreturn); the two veto cases assertsubmit_verified:false+composer_still_populated; the close cases assert againstlist_surfaces, not against a CLI call count.#484 — what shipped
send_to(mode:"key")/send_key:isSubmitKey()makessubmit_attemptedtruthful forreturn/enter/KPEnter/ctrl-m/^mand case variants. The receipt is all it touches.key_dispatched:true—sendKeyWithRetrythrows when nothing reaches the pane, so the ok path has dispatch evidence and now states it instead of leaving a caller to infer it frombytes:0.composer_still_populated. Unreadable ⇒null+ reason. Non-submit keys are not verified and say so.verify_submit, which onlysend_keypasses; the internal shell-launcherkey:"return"callers are untouched.Trace (ask 2). The key was always dispatched; the receipt was the lie.
submit_attemptedwaskey === "return", an exact match on the raw caller string. Probed on v0.4.47:return→true,Return/enter/escape→false. That is the reported receipt, and it means the reported call used a non-canonical alias. I could not confirm which literal was passed — if it was lowercasereturn, this diff does not explain that receipt and the residue is the #473/#457 stale-terminal gating, deliberately untouched here.Deferred, written into #484: ask 3's text path (
pending_verify/failed_confirmed) is T2's file scope (#445, #443). Ask 4's stale-terminal half is #473/#457.#485 — against the corrected scope
The mechanism (confirmed in source,
server.ts:9492-9510pre-fix):scope:"agent"delegated tostop_agentand no path in that branch closed the surface, sook:true, state:"done"was truthful about the agent and silent about the pane.scope:"surface"took the other branch and did close it. Every field report fits with zero latency once sorted by which scope the lead called.Target 1 — agent scope closes the pane, or says it did not. Already the shape of this branch since round 1. New this round:
surface_closedis confirmed against the topology rather than inferred from the CLI returning, and agent scope forwards that observation instead of restating it.Target 3 — assert against
list_surfaces, not the return value.closes the pane under agent scope — the surface is gone from list_surfacesandnever reads like a completed close while the pane is still listedboth assert onlist_surfaces; the receipt field is checked second, not first.Target 4 —
list_agentsmust not report an agent "working" after its close is acknowledged. This was real and is fixed.close_surfacemarked matching recordsuser_killed:truebut never transitioned their state — the local variable was even namedterminal. Solist_agentskept rendering a closed agent asworking. An acknowledged close now transitions a non-terminal record todonein the same breath.The test that caught it first passed vacuously, and I want that on the record:
list_agentsnests state as{"state":{"value":"working"}}, so my/"state":\s*"working"/regex never matched anything. The assertion now readsstate.value, and reproduced the defect immediately. It also checks the underlying record, so an absent row cannot make it pass by accident.Target 2 — NOT done as literally specified. The instruction is that the description must state "agent-scope does not close panes". That sentence is false against this branch. Target 1 offered a choice and the first option was taken: agent scope does close the pane. Writing the dictated sentence would put a lie in a tool description — the exact disease this lane exists to fix. The description states what the code does instead:
If the lane wants agent-scope to stop closing panes instead, that is a one-line change plus a description swap — say so and I will take the other option. I am not going to ship a description that contradicts the code.
Unchanged from round 1: resolve the bound surface before the stop, stop, close, report
agent_stoppedandsurface_closedseparately, stripstop_agent'sok/errorbefore merging.Changed on the reviewer's escalation finding. The inner close now passes the caller's own
forcerather thantrue.stop_agenthas no liveness refusal of its own, so forcing unconditionally let an unforcedclose_surface(scope:"agent")tear down a still-live agent's pane through a guard that could never fire for this scope. Now the guard applies, the receipt reportssurface_closed:false, and the escalation is gone rather than documented. Test:does not escalate to a forced close. The description also states that the pane close obeys the same guard asscope:"surface".Ask 3's
list_surfacescheck is now written, as asked: the close tests assert the surface is gone fromlist_surfaces, not merely thatclose-surfacewas issued. That required the mock to carry a realistic topology (surface_refs/surface_count/pane_ref).Minimality — deletions taken
normalizeKeyNameno longer rewrites the key sent to cmux. Taken in full, and it is the right call: the contract is receipt honesty,isSubmitKeydelivers it alone, and rewriting silently changed the bytes cmux receives for aliases nobody reported — against a cmux this PR admits it did not verify.\nwas the sharp edge. Raw\r/\nare dropped from the submit set for the same reason, which also retires the padded-" \r "note by deletion.normalizeKeyNameis now byte-identical to base, and my own PREDICTION feat: V2 — sidebar sync, agent hierarchy, quality tracking #1 is deleted rather than argued.timeout_msknob deleted — unreachable; the constant is used directly.listStates().find(…)fallback deleted — house idiomcontext.lifecycleRegistry?.get(id) ?? nullinstead.submit_verification_reasonis stated unconditionally, nullable, matching howsubmit_verified: nullis already handled.tests/helpers/mcp-tool-harness.ts(getTool,getEngine,ToolCallResult).parseToolResultis deliberately not used and the reason is in a comment: it throws on error results, and half of this suite exists to inspect exactly those.Not taken, per the review's own withdrawal: the two verification loops stay separate. Merging them means mode flags, which costs the reader more than the duplication does.
Tests
The pre-push hook ran the full suite on the pushed commit
8c154e0:Tests 3103 passed | 1 skipped (3104),run_tests.sh finished with exit status 0.npx tsc --noEmitexit 0. 19 tests in the new file; 18 red against base, the 19th red against sabotage.The hook also caught a failure my isolated runs did not: the
list_agentstest asserted the closed agent was still listed, which depends on when the lifecycle sweep runs. Over-specified — the claim under test is the state, not the row. Fixed and re-verified red against base.One honest caveat on my own numbers. Between that and my manual runs I saw the serial suite come back 3100/0-failed once and then 11-failed and 1-failed on reruns, in files this diff does not touch (
release-receipts,inbox-nudge,enter-reliability). Cause: this host is at load ~21 with 16 concurrent vitest processes from other fleet worktrees, and those files use fixed$TMPDIRfixture paths (cmux-enter-reliability-test,cmux-agents-test-inbox-nudge) that collide across worktrees.enter-reliabilitypasses 42/42 four times running in isolation on this branch. I am citing the pre-push run as the result because it is the one taken on the pushed commit, and flagging the rest rather than picking my best number.PREDICTION — where to push hardest now
submit_verified:truerarer. If real Claude panes render the composer boxed while working, most successful submits will now returnsubmit_verified:null, submit_evidence_absentandok:true. Honest, never a false success — but a caller looking fortrueas a green light gets it less often, and I did not verify which form real panes emit. This is the trade I chose deliberately and it is the thing most worth arguing with.composer_still_populatedis a hard error. A stale or misparsed composer read on a CLI I did not fixture (kiro, gemini) reports a working submit as failed.close_surface(scope:"agent")on a live agent now stops it and keeps the pane. Better than the silent escalation, but it is a behavior change from round 1 that a lead harvesting panes will notice — they will needforce:truewhere round 1 would have closed.src/server.tstoo. Whoever lands second rebases.🤖 Generated with Claude Code
Note
Fix silent success receipts when submit key never lands or pane never closes
verifySubmitKeyOutcomein server.ts that polls the screen after a bare submit key dispatch to confirm the composer cleared;send_to(mode:"key")now returns an error if the key reached the pane but the composer remains populated.isSubmitKeyin key-names.ts using a canonical set of submit key aliases (SUBMIT_KEY_ALIASES) to identify return/enter variants.close_surface(scope:"agent")to stop the agent, then close its bound surface, and verify the pane actually disappeared — returning an error if the surface is still listed after the close attempt.close_surface(scope:"surface")now checks whether the pane is still present viafindSurfaceByRefand warns rather than silently returning ok when the close did not take effect.send_toandclose_surfacecalls that previously returned ok without confirmation now return errors when the expected outcome cannot be verified.Macroscope summarized 8c154e0.
Note
Medium Risk
Changes MCP tool success/error semantics for fleet relay and teardown paths; submit verification may return more
nullor hard errors on busy or oddly rendered composers, and unforced agent close no longer force-closes live panes.Overview
Fixes silent
ok:truereceipts when a submit key did not land (#484) orclose_surfaceonly half-finished (#485).Submit keys (
send_key/send_tomodekey): AddsisSubmitKey()so receipts setsubmit_attemptedfor enter/return/KPEnter/ctrl-m aliases without rewriting keys sent to cmux. After dispatch,key_dispatchedis set and submit keys runverifySubmitKeyOutcome(~1.5s): an empty readable composer ⇒submit_verified: true; text still in the composer ⇒ error (composer_still_populated), including when status is “working”; unreadable composer ⇒nullwith a reason—not inferred success from status.close_surface:scope:"agent"stops the agent, then closes the bound surface (callerforcepassed through), with separateagent_stopped/surface_closedfields and errors if the pane survives.scope:"surface"setssurface_closedfrom a post-close topology check and warns if cmux still lists the pane. Acknowledged closes transition agents out ofworkingsolist_agentsstays consistent.scope:"workspace"addsworkspace_deleted.Adds regression tests in
tests/t2b-silent-failures.test.ts.Reviewed by Cursor Bugbot for commit 8c154e0. Bugbot is set up for automated code reviews on this repo. Configure here.