fix(#442): acknowledge-and-retry for the draft guard, and split the stale case by globalsecurepayments · Pull Request #585 · EtanHey/cmuxlayer

globalsecurepayments · 2026-09-02T08:35:44Z

Follows up my comment on #504. That issue's ask #4 already names this case — "this same styling would let the draft guard tell a human's real draft from any other dim/ghost UI text" — so this is deliberately not the styling work. It is the interim half, and it is written so #504 makes it unnecessary rather than conflicting with it.

The problem

send_to returns terminal blocked_by_foreign_draft quoting text the pane's visible composer does not hold: a cmux autocomplete suggestion drawn over an empty composer. Because composerHoldsForeignDraft receives only the flattened frame, ghost text and a real draft are byte-identical to it.

The caller has no way forward. Escape and ctrl+u dispatch to the terminal; the suggestion is drawn above it, so the next surface.read_text still shows it. The pane becomes undeliverable and the only remedy is tearing down and re-attaching the session — which loses its context. Observed ~9 times in ~18h across 4 panes on 0.4.68.

What this does

Adds override_foreign_draft — an acknowledgement, not a bypass.

The caller echoes back the exact text the refusal reported, and delivery proceeds only if the composer still holds exactly that. It is a compare-and-swap, and that is the whole design:

A human mid-draft is typing, so their text moves between the refusal and the retry and the echo stops matching. Static ghost text does not move.

A blanket allow_*: true boolean — which would have matched the existing allow_busy / allow_long_inline convention — would have traded #442 away entirely, so I deliberately did not follow that convention here. If you'd rather it were a boolean, that's your call and I'll change it, but I think the CAS is what keeps the guard meaningful while it cannot classify.

A stale acknowledgement gets its own error code, blocked_by_draft_override_stale, rather than being folded back into the original refusal. That case — the composer moved after the caller looked — is the dangerous one, and it is precisely the one a caller most needs to distinguish. The unacknowledged refusal message now also points at the new parameter.

Test plan

3 tests added to tests/delivery-truth-t2.test.ts, in the existing #442 block:

ghost text + exact echo → delivers
composer moved + stale echo → refused blocked_by_draft_override_stale, pane not mutated
no acknowledgement → still blocked_by_foreign_draft, now naming the override

Verified in both directions, because a test that has only ever seen the fixed code hasn't been tested:

src reverted, tests kept  ->  3 failed | 13 passed   (all three fail)
src restored              ->  16 passed
full suite                ->  156 files | 3762 tests | 0 failures

The 13 pre-existing tests in that block pass either way, so the new arms are the only thing these changes move.

tsc --noEmit clean.

Notes

Wired through both delivery entry points: the send_input handler (covers send_to mode=surface and send_input) and deliverAgentInput (covers mode=agent). I deliberately did not wire it into the broadcast path, which has its own args type and where a per-pane acknowledgement doesn't make sense.
Default behaviour is unchanged when the parameter is absent.
I'm a first-time contributor and I don't have a voucher. CONTRIBUTING says to name one and I'd rather say so than leave it blank — happy for this to wait on that, or to be told the vouching model applies differently to a bug I reported on Classify Codex ghost text by render styling (faint), not a literal placeholder list — terminal.replay already exposes it #504 myself.

🤖 Generated with Claude Code

Note

Add `override_foreign_draft` acknowledgement to delivery safety guard and split stale case

Introduces an optional override_foreign_draft string on send_to and send_input; when its trimmed value matches the currently observed foreign composer text, the guarded delivery proceeds instead of being blocked.
Adds the blocked_by_draft_override_stale error code to DeliverySafetyGateError for acknowledgements that no longer match the composer, while the existing blocked_by_foreign_draft error now names override_foreign_draft as the retry mechanism.
Fixes background delivery so draftOverrideText is retained on the DeliveryRecord and forwarded into executeDeliveryEngine / assertDeliveryTargetIsSafe, instead of being dropped on the async path.
Routes the acknowledgement through surface delegation, single-agent, and multi-agent send_to paths via deliverAgentInput.
Risk: the guard in server.ts compares trimmed text only; leading/trailing whitespace differences between the acknowledgement and the observed composer frame are ignored, so reviewers should confirm that is the intended compare-and-swap semantics.

^{Macroscope summarized 863f8a8.}

Summary by CodeRabbit

New Features
- Added optional draft acknowledgement support when sending text to a composer with an existing draft.
- Available through the send_to and send_input tools, including background delivery.
- Delivery proceeds only when the acknowledgement exactly matches the current draft.
- Added a distinct error when an acknowledgement is outdated.
Bug Fixes
- Prevented text from being sent when a draft changes after acknowledgement.
- Improved safeguards to ensure refused deliveries do not modify the composer.

…t the stale case The draft guard reads a FLATTENED frame, so UI ghost text drawn over an empty composer is byte-identical to a human's real draft. It cannot classify, and a caller that hits the refusal has no way forward: no key clears the suggestion (it lives above the terminal), so the pane becomes undeliverable. Adds `override_foreign_draft` as an ACKNOWLEDGEMENT, not a bypass. The caller echoes back the exact text the refusal reported; delivery proceeds only if the composer STILL holds exactly that. A human mid-draft is typing, so their text moves between the refusal and the retry and the echo stops matching -- static ghost text does not move. A blanket boolean would have traded EtanHey#442 away. A stale acknowledgement gets its own code, `blocked_by_draft_override_stale`, rather than being conflated with the original refusal: that is the dangerous case, and it is the one a caller most needs to tell apart. Tests: 3 added, verified in both directions -- with src reverted all three fail (3 failed / 13 passed); restored, 16 pass. Full suite 156 files / 3762 tests. Refs EtanHey#504, which would let the guard classify by render styling and remove the need to ask the caller at all. Co-Authored-By: Claude <noreply@anthropic.com>

deepsource-io · 2026-09-02T08:36:13Z

DeepSource Code Review

We reviewed changes in 2d1fb62...863f8a8 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade	Security Reliability Complexity Hygiene

Code Review Summary

Analyzer	Status	Updated (UTC)	Details
JavaScript		Sep 3, 2026 10:40a.m.	Review ↗
Shell		Sep 3, 2026 10:40a.m.	Review ↗

Important

AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.

coderabbitai · 2026-09-02T08:36:53Z

📝 Walkthrough

Walkthrough

The delivery draft guard now supports override_foreign_draft. Delivery proceeds only when the acknowledgement exactly matches the current composer text. Missing or stale acknowledgements return distinct safety errors. The value is forwarded through synchronous and background send_to and send_input delivery paths.

Changes

Foreign Draft Override Flow

Layer / File(s)	Summary
Draft guard contract and compare-and-swap check `src/server.ts`	Adds the optional override parameter and `blocked_by_draft_override_stale`. The guard compares the acknowledgement with the current composer text.
Delivery override propagation `src/server.ts`	Forwards the override through `send_to`, `send_input`, `deliverAgentInput`, and `executeDeliveryEngine`. The background path stores and forwards the acknowledgement through `DeliveryRecord`.
Override behavior validation `tests/delivery-truth-t2.test.ts`	Tests exact acknowledgements, stale acknowledgement rejection, unchanged refusal behavior, error codes, and pane mutation rules.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🟡 Moderate · up to 863f8

This change permits acknowledged delivery over an unchanged foreign draft while retaining stale-draft protection. Queued deliveries for paused targets can still discard that acknowledgement and fail after resume, so the queue path needs to preserve it before merge.

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant send_to
  participant DeliveryRecord
  participant executeDeliveryEngine
  participant Composer
  participant assertDeliveryTargetIsSafe
  Caller->>send_to: send_to with override_foreign_draft
  send_to->>DeliveryRecord: store draftOverrideText for background delivery
  DeliveryRecord->>executeDeliveryEngine: forward draftOverrideText
  executeDeliveryEngine->>Composer: read current composer text
  Composer-->>assertDeliveryTargetIsSafe: current text
  assertDeliveryTargetIsSafe->>assertDeliveryTargetIsSafe: compare acknowledgement
  assertDeliveryTargetIsSafe-->>executeDeliveryEngine: allow delivery or safety error

Poem

I’m a rabbit with a draft in my sight
Exact words let delivery hop right
Stale words stop at the gate
Empty bows still must wait
Tests check each pane stays bright

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: adding acknowledge-and-retry behavior to the draft guard and separating stale acknowledgements.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 too large.)

Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

deepsource-io · 2026-09-02T08:37:37Z

+    screenText = "Claude Code\n> merge #208 when green and send me the commit\n";
+    mockExec.mockClear();
+
+    const result = await (server as any)._registeredTools["send_to"].handler(
