fix(server): drop mirrored typing whose sender left the pane before its turn

[devswha]

What

On a mirrored pane (Windows x64: herdr cannot terminal attach, so server/mirror.ts mirrors the screen), text typed while a message is still being sent no longer reaches the pane when the sender leaves it, or leaves and opens it again, before the text's turn comes. The sender gets input_failed, the same answer queued keys and queued pty typing give.

Why

The mirror branch of the WS input handler in server/index.ts waits its turn in serialize(...) and then calls paneSendText, checking only that the connection is still open. Unlike the pty branch and the keys handler, it never kept the sender's claim (client.data.attached) or the attachment it typed into, so a detach, or a detach followed by a reattach, did not cancel it.

How

Before queueing, keep origin (the attachment the sender is a member of, if any) and claim. Inside the queued step, after the asynchronous paste shaping, answer input_failed and send nothing when the claim changed, or when origin was replaced, closed, or no longer holds the sender. Typing from a connection that never attached keeps its existing path (both values stay undefined). No wire shape changes, so shared/protocol.ts, the demo transport and the remote bundle are untouched.

How verified

• New contract case in server/submit.contract.test.ts (typing without terminal attach, at the edges): a server with terminalAttach: false and submitDelayMs: 1000 holds the pane's turn with a submit, then the test sends input and detach (second round: detach + attach).
  • On f8f09d1 it fails: the recorder receives one\rtypedtwo\r.
  • With the fix it passes, and the sender gets input_failed.
• Mutation: with the whole new guard disabled, the case fails again with typedtwo. With only the claim half disabled it stays green, because the attachment check covers both rounds on its own.
• bun run check run bun test ./server/submit.contract.test.ts: 22 pass, 0 fail.
• bun run check fast: workflow syntax, generated types, typecheck, build, and unit tests all pass (1699 pass, 4 platform skips, 0 fail).

Fixes #546

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: devswha/herdr-web-ui/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1147391e-6934-4a2a-9e5d-fa762ffd9386

📥 Commits

Reviewing files that changed from the base of the PR and between 99a17d5 and df0bf4a.

📒 Files selected for processing (1)

• CHANGELOG.md

🚧 Files skipped from review as they are similar to previous changes (1)

• CHANGELOG.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 4 remain after this review.

---

📝 Summary

Summary by CodeRabbit

• Bug Fixes
  • Prevented text queued while a message is being sent from reaching a mirrored Windows pane after you leave it, including if you reopen the pane before the queued text is sent. The queued input is reported as failed, while the message already being sent completes and later submitted messages can still reach the pane.

Walkthrough

The mirrored-terminal fallback now checks attachment state before sending queued input. Contract tests cover detachment and detachment followed by reattachment. The changelog records the fix.

Changes

Mirrored Input

Layer / File(s)	Summary
Validate queued mirrored input server/index.ts, server/submit.contract.test.ts, CHANGELOG.md	The fallback checks the attachment claim and originating attachment after input shaping. It reports input_failed if the claim or attachment membership changed. Contract tests cover detachment and reattachment, and the changelog documents the fix.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to df0bf

Queued mirrored input is guarded against delivery after detachment, while input from connections that never attached retains its existing behavior. No merge-blocking issue was identified.

Security Architecture Review

Security architecture risk: ⚪ Minimal · up to 99a17

The change narrows queued input delivery by rejecting text after its original attachment changes. It preserves existing access checks and does not introduce a new endpoint, privilege or delivery path. No material security risk introduced or worsened by this change was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The sensitive outcome remains delivery of client-supplied text to the selected pane through the existing pane.send_text RPC. The PR restricts which queued inputs reach that sink; it does not add a caller, privilege or delivery mechanism.

Trust Boundaries and Controls

• observed — Attachment claims distinguish attachment lifetimes rather than just pane identifiers. Detach deletes the claim and membership; reattach creates a new claim. Closure removes the attachment and member claims, while disconnect removes the client and clears its claims. Input captured without an attachment or claim retains its pre-existing path; that behavior is not introduced by this PR.

Resilience and Maintainability Implications

• observed — The new stale-state rejection occurs before invoking the delivery RPC and does not retry canceled text. Separately, an RPC rejection still means delivery could not be confirmed, not that downstream partial application has been ruled out. That acknowledgement uncertainty and the error advising users to check the terminal both predate this change.

🚥 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 1 functions across 2 files. (1 skipped: 1 …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the server fix and the mirrored typing scenario it addresses.
Description check	✅ Passed	The description explains the problem, cause, implementation, behavior, and validation. It includes contract-test coverage and reported test results, although it uses equivalent headings instead of the…
Linked Issues check	✅ Passed	Issue #546 requires queued mirrored input cancellation after an attachment claim change, attachment replacement or closure, or sender membership loss. server/index.ts now captures the claim and orig…
Out of Scope Changes check	✅ Passed	The changes modify the mirrored input guard, add its contract test, and document the fix in CHANGELOG.md. These changes support issue #546. The summary reports no public entity or wire-format change…

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 1 functions across 2 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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

[devswha]

Independent review (merge-own lane).

Classification: serious bug fix (#546) on the input path. On a mirrored pane (Windows x64, no PTY), text typed while a message was being sent still reached the pane after the sender left it, or left and opened it again. That breaks the bridge rule that nothing typed outlives the context it was typed in.

What I checked against the bridge invariants:

• Before queueing, the mirror branch of the WS input handler now records the sender's claim (client.data.attached) and the attachment the sender is a member of. Inside its serialize turn, after paste shaping, it answers input_failed and sends nothing when the claim changed or that attachment was replaced or no longer holds the sender. This matches what the pty branch and the keys handler already do. Nothing is queued while offline, and nothing is resent.
• A connection that never attached keeps its old path, since both values stay undefined. A connection that attaches while its typing waits now gets input_failed, which is the conservative side. No wire shape changes, so shared/protocol.ts, the demo transport and the remote bundle need nothing.
• The test can fail: with the whole guard replaced by if (false), the new #546 case fails (21 pass, 1 fail). With only the claim half disabled it stays green, as the PR body says, because the attachment check covers both rounds.
• Codex (gpt-6.1-sol, xhigh, adversarial, read-only) found no material issue. It checked cancellation after detach, reattach, attachment replacement, disconnect, and detach during paste shaping.

What I ran (separate worktree):

• bun run check run bun test --timeout 30000 ./server/submit.contract.test.ts: 22 pass, 0 fail.
• Updated with main (df0bf4a, CHANGELOG conflict resolved after #574). bun run check fast: 1715 pass, 0 fail. Fast checks and Integration and browser are green on the merged head. There are no review threads.