fix: claude signal drain act-then-delete parity - multi-instance safety + signal-independent first-bind - #578
Merged
Merged
Conversation
…drain-parity plan All 23 validated assumptions confirmed (0 falsified). Precision fixes from validator evidence: correct module-header anchor (:1-14), Phase 4 wording lives in a comment not an assert message, make the npm-ci precondition for freshell-ws integration tests explicit with its failure signature (MCP inject resolves node_modules/tsx at pane create), note the deliberate new warn from splitting the retired/foreign-provider guard, and allow the harness's pid-scoped temp roots alongside tempfile::tempdir().
…n on rejects 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
…ution 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
…minals survive drains, TTL-reaped) Phase 5 appended to claude_session_rebind: a SessionStart signal naming a terminal id unknown to this instance (another freshell server sharing $HOME owns that pane) must be retained across two drain cycles, never emit a terminal.session.associated frame, and be reaped only after the staleness TTL (mtime backdated past STALE_SIGNAL_MAX_AGE = 600s). Mirrors opencode's Phase 9 plus a TTL-reap tail it lacks. Red-proofed against the pre-parity drain (1d7449a~1): Phase 5 fails at the first retention assertion because the destructive drain ate the foreign signal. Green at HEAD. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
…d (review finding) The Discard arms of apply_claude_signal had zero direct test coverage: if a Discard regressed to Retain, a foreign-provider signal would warn-log every 1s sweep for 10 minutes and no test would catch it. Port the opencode lane's foreign-provider phase as Phase 7: a claude signal naming a shell-mode pane (valid unbound UUID session id) must be CONSUMED, not retained, and must never emit a terminal.session.associated frame. Red-direction verified by temporarily inverting the file-consumed assertion and observing the failure. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root Cause
The claude signal-file drain was destructive: it deleted signal files on read without checking whether they belonged to the current instance. In multi-instance scenarios (dev servers on different ports, test/drill runs, or agents testing in worktrees), a second freshell instance sharing
$HOMEsilently destroyed claude rebind signals from a first instance.Proven in live drill: production 3002 server swept the shared signal dir every second and consumed all 7 of the drill's claude signals, leaving those panes with
sessionRef: None.The Fix
Port the proven opencode signal-drain pattern to claude:
--session-ideven if signal dir is destroyedTests
Reviews
Plan review: passed round 1, zero blockers
Delta review: passed round 1, zero blockers