fix(agent-hooks): stop backgrounded Claude sessions posting a stale pane key (STA-4769) - #15304
Conversation
39fbc91 to
dc68111
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 39fbc916147d68d0174236c035cdc0da20a119ec and 3e3d5a8. 📒 Files selected for processing (2)
Included review availability: Your plan includes up to 10 reviews per rolling hour; 1 remains after this review. 📝 WalkthroughWalkthroughHook and statusline scripts now exit when 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d2fe2db-0e4c-4602-9f36-22c60963166d
📥 Commits
Reviewing files that changed from the base of the PR and between 6efd406 and 39fbc916147d68d0174236c035cdc0da20a119ec.
📒 Files selected for processing (2)
src/main/claude/hook-service.test.tssrc/main/claude/hook-service.ts
Included review availability: Your plan includes up to 10 reviews per rolling hour; 0 remain after this review.
…as spawned into (STA-2069) (#14615)" (#15295) Reverts #14615. Its premise does not reproduce, it does not reach the failure that does, and the correction it installs can misattribute status on a path that worked before. 1. PREMISE FALSE. #14615 asserts Claude Code >= 2.1.206 hosts TUI sessions under a shared daemon. On 2.1.233 `claude daemon status` reports "not running" with 69 live interactive sessions, and every client is a direct child of its own pane's shell. Measured across the fleet: 68 distinct pane keys, zero collisions. Foreground attribution was never broken. 2. DOES NOT FIX THE REAL BUG. The failure in #9236 is real but scoped to BACKGROUNDED sessions, whose workers inherit the dispatching pane's whole ORCA_* set. #14615 mints a binding only for launches Orca constructs, so a typed `claude --bg` produces none. Fixed properly in #15304. 3. INTRODUCES A MISATTRIBUTION. Bindings are removed only on PTY death, and a user who exits Claude keeps the pane's PTY. Resuming that session in another pane does not rebind (`--resume` is a session selector, so the pin declines), and resolveBoundPaneOverride then rewrites paneKey and tabId onto the ORIGINAL pane despite a correct posted key. Demonstrated with a failing test against main; causation isolated to resolveBoundPaneOverride. Kept #14706's observations.rebind() in the conflicting hunk — it postdates #14615 and is not part of this revert.
…ane key (#9236) A session started with `claude --bg` or `/background` runs in a worker under the shared daemon, and that worker inherits the environment of whichever pane first started the daemon — not the pane that dispatched it. ORCA_PANE_KEY there names an unrelated pane, so the session's hooks overwrite that pane's sidebar row, from any worktree. It fails silently and successfully: the script re-sources the endpoint file, so port and token self-heal and the POST lands, while the pane key has no refresh path and stays wrong. Measured on the wire against a throwaway listener: three sessions on one daemon, and the one dispatched from pane B posted pane A's key and pane A's worktree on SessionStart, UserPromptSubmit and Stop. CLAUDE_JOB_DIR is set only in those workers — absent from all 68 live foreground sessions on this machine — so it is the signal to decline. Declining is the only option that exists: normalizeHookPayload rejects an absent paneKey outright and AgentHookEventPayload.paneKey is a required string, so there is no "session with no pane" representation to post instead. A backgrounded session genuinely has no pane; attributing it to nothing is correct. The Windows guard exits rather than jumping to the stdin drain: the drain parks in more.com and a daemon worker is outside an Orca pane, which is exactly the abandoned-stdin hang #11549 guards against.
dc68111 to
918ab2b
Compare
…pane key The statusline command IS invoked inside a backgrounded worker — measured, with no client ever attached, and its ancestry terminates at the daemon rather than at any pane: statusline pid=41746 <- claude bg-spare <- claude bg-pty-host <- claude daemon CLAUDE_JOB_DIR=/tmp/.../jobs/f1f9edd2 A second session dispatched from a different pane saw the first pane's ORCA_PANE_KEY with its own correct session id, so this script is a live second producer of the same misattribution the hook guard closes. Windows uses exit /b 0 before stdin is owned, per the #11549 contract; POSIX places the guard after capture, since exiting mid-write there surfaces as EPIPE the agent can see (#8110).
Electron CDP validation (geoduck worktree)Isolated-profile Playwright CDP attach against this branch. Identity matched What the rendered app showed
Not shown (expected gaps)
App loadedAgents session historySettings → Agents (status hooks)Stats & UsageUsage / statusline footer |
Review status: PASS — landI completed the opening and final readiness-checklist scans, a same-model senior code review, a performance audit, and the required Electron CDP visible-proof pass. No proven P0/P1/P2 findings were found, no scope-expanding fixes were warranted, and the worktree is clean. Verification:
Conclusion: land as-is. |
Behavioural validation — the guard actually suppresses, and foreground still postsThe earlier Electron CDP comment on this PR confirmed the guard text reached the installed script, and said so itself: "the stale-pane-key overwrite (or its absence) was not reproduced on a sidebar row." That is a deployment check, not a behaviour check, and the "PASS — land" verdict that followed it rested on more than the evidence showed. This closes that gap. Three arms, because two would not have been enough. Method
The pre-fix control is that same generated file with only the three guard lines removed, in the scratch copy. Results
Three things follow, and the third is why the control arm exists:
POST timestamps line up with the transcript hook events in both posting arms; the full log is in the run report. Honest limits
Cleanup verified: listener stopped, port closed, spawned daemon PIDs gone, scratch HOME and cwd removed, and the user's |
…as spawned into (STA-2069) (stablyai#14615)" (stablyai#15295) Reverts stablyai#14615. Its premise does not reproduce, it does not reach the failure that does, and the correction it installs can misattribute status on a path that worked before. 1. PREMISE FALSE. stablyai#14615 asserts Claude Code >= 2.1.206 hosts TUI sessions under a shared daemon. On 2.1.233 `claude daemon status` reports "not running" with 69 live interactive sessions, and every client is a direct child of its own pane's shell. Measured across the fleet: 68 distinct pane keys, zero collisions. Foreground attribution was never broken. 2. DOES NOT FIX THE REAL BUG. The failure in stablyai#9236 is real but scoped to BACKGROUNDED sessions, whose workers inherit the dispatching pane's whole ORCA_* set. stablyai#14615 mints a binding only for launches Orca constructs, so a typed `claude --bg` produces none. Fixed properly in stablyai#15304. 3. INTRODUCES A MISATTRIBUTION. Bindings are removed only on PTY death, and a user who exits Claude keeps the pane's PTY. Resuming that session in another pane does not rebind (`--resume` is a session selector, so the pin declines), and resolveBoundPaneOverride then rewrites paneKey and tabId onto the ORIGINAL pane despite a correct posted key. Demonstrated with a failing test against main; causation isolated to resolveBoundPaneOverride. Kept stablyai#14706's observations.rebind() in the conflicting hunk — it postdates stablyai#14615 and is not part of this revert.
…ane key (STA-4769) (stablyai#15304) * fix(agent-hooks): stop backgrounded Claude sessions posting a stale pane key (stablyai#9236) A session started with `claude --bg` or `/background` runs in a worker under the shared daemon, and that worker inherits the environment of whichever pane first started the daemon — not the pane that dispatched it. ORCA_PANE_KEY there names an unrelated pane, so the session's hooks overwrite that pane's sidebar row, from any worktree. It fails silently and successfully: the script re-sources the endpoint file, so port and token self-heal and the POST lands, while the pane key has no refresh path and stays wrong. Measured on the wire against a throwaway listener: three sessions on one daemon, and the one dispatched from pane B posted pane A's key and pane A's worktree on SessionStart, UserPromptSubmit and Stop. CLAUDE_JOB_DIR is set only in those workers — absent from all 68 live foreground sessions on this machine — so it is the signal to decline. Declining is the only option that exists: normalizeHookPayload rejects an absent paneKey outright and AgentHookEventPayload.paneKey is a required string, so there is no "session with no pane" representation to post instead. A backgrounded session genuinely has no pane; attributing it to nothing is correct. The Windows guard exits rather than jumping to the stdin drain: the drain parks in more.com and a daemon worker is outside an Orca pane, which is exactly the abandoned-stdin hang stablyai#11549 guards against. * fix(agent-hooks): guard the Claude statusline against the same stale pane key The statusline command IS invoked inside a backgrounded worker — measured, with no client ever attached, and its ancestry terminates at the daemon rather than at any pane: statusline pid=41746 <- claude bg-spare <- claude bg-pty-host <- claude daemon CLAUDE_JOB_DIR=/tmp/.../jobs/f1f9edd2 A second session dispatched from a different pane saw the first pane's ORCA_PANE_KEY with its own correct session id, so this script is a live second producer of the same misattribution the hook guard closes. Windows uses exit /b 0 before stdin is owned, per the stablyai#11549 contract; POSIX places the guard after capture, since exiting mid-write there surfaces as EPIPE the agent can see (stablyai#8110).
…as spawned into (STA-2069) (stablyai#14615)" (stablyai#15295) Reverts stablyai#14615. Its premise does not reproduce, it does not reach the failure that does, and the correction it installs can misattribute status on a path that worked before. 1. PREMISE FALSE. stablyai#14615 asserts Claude Code >= 2.1.206 hosts TUI sessions under a shared daemon. On 2.1.233 `claude daemon status` reports "not running" with 69 live interactive sessions, and every client is a direct child of its own pane's shell. Measured across the fleet: 68 distinct pane keys, zero collisions. Foreground attribution was never broken. 2. DOES NOT FIX THE REAL BUG. The failure in stablyai#9236 is real but scoped to BACKGROUNDED sessions, whose workers inherit the dispatching pane's whole ORCA_* set. stablyai#14615 mints a binding only for launches Orca constructs, so a typed `claude --bg` produces none. Fixed properly in stablyai#15304. 3. INTRODUCES A MISATTRIBUTION. Bindings are removed only on PTY death, and a user who exits Claude keeps the pane's PTY. Resuming that session in another pane does not rebind (`--resume` is a session selector, so the pin declines), and resolveBoundPaneOverride then rewrites paneKey and tabId onto the ORIGINAL pane despite a correct posted key. Demonstrated with a failing test against main; causation isolated to resolveBoundPaneOverride. Kept stablyai#14706's observations.rebind() in the conflicting hunk — it postdates stablyai#14615 and is not part of this revert.
…ane key (STA-4769) (stablyai#15304) * fix(agent-hooks): stop backgrounded Claude sessions posting a stale pane key (stablyai#9236) A session started with `claude --bg` or `/background` runs in a worker under the shared daemon, and that worker inherits the environment of whichever pane first started the daemon — not the pane that dispatched it. ORCA_PANE_KEY there names an unrelated pane, so the session's hooks overwrite that pane's sidebar row, from any worktree. It fails silently and successfully: the script re-sources the endpoint file, so port and token self-heal and the POST lands, while the pane key has no refresh path and stays wrong. Measured on the wire against a throwaway listener: three sessions on one daemon, and the one dispatched from pane B posted pane A's key and pane A's worktree on SessionStart, UserPromptSubmit and Stop. CLAUDE_JOB_DIR is set only in those workers — absent from all 68 live foreground sessions on this machine — so it is the signal to decline. Declining is the only option that exists: normalizeHookPayload rejects an absent paneKey outright and AgentHookEventPayload.paneKey is a required string, so there is no "session with no pane" representation to post instead. A backgrounded session genuinely has no pane; attributing it to nothing is correct. The Windows guard exits rather than jumping to the stdin drain: the drain parks in more.com and a daemon worker is outside an Orca pane, which is exactly the abandoned-stdin hang stablyai#11549 guards against. * fix(agent-hooks): guard the Claude statusline against the same stale pane key The statusline command IS invoked inside a backgrounded worker — measured, with no client ever attached, and its ancestry terminates at the daemon rather than at any pane: statusline pid=41746 <- claude bg-spare <- claude bg-pty-host <- claude daemon CLAUDE_JOB_DIR=/tmp/.../jobs/f1f9edd2 A second session dispatched from a different pane saw the first pane's ORCA_PANE_KEY with its own correct session id, so this script is a live second producer of the same misattribution the hook guard closes. Windows uses exit /b 0 before stdin is owned, per the stablyai#11549 contract; POSIX places the guard after capture, since exiting mid-write there surfaces as EPIPE the agent can see (stablyai#8110).





ELI5
If you background a Claude session, it runs inside a shared helper process that was started by whatever terminal happened to use it first. That helper hands the session the first terminal's identity, so the backgrounded session's status lands on a stranger's sidebar row and keeps overwriting it. This teaches the hook to notice it's in that situation and stay quiet, because a backgrounded session doesn't belong to any pane.
What Changed
One guard in the Claude managed hook script, both branches of
src/main/claude/hook-service.ts: ifCLAUDE_JOB_DIRis set, exit without posting.Why
A session started with
claude --bgor/backgroundruns in a worker under the shared daemon. That worker inherits the environment of whichever pane first started the daemon — not the pane that dispatched it. Reproduced in an isolatedCLAUDE_CONFIG_DIR:Process tree is
claude daemon run(ppid 1) →bg-pty-host→ worker, detached from every pane's PTY. It also inherits pane A'sORCA_WORKTREE_ID, so events file under the wrong workspace too.Captured on the wire, not inferred — three sessions on one daemon, posting to a throwaway listener:
5b0c7d88paneKey=BG-PANE-A wt=WT-A✓de34110cpaneKey=BG-PANE-A wt=WT-A✗9bdb77f8paneKey=FG-PANE-Z wt=WT-Z✓Arm B's
SessionStart,UserPromptSubmitandStopall posted pane A's identity. That is the reported symptom: two sessions collapsing onto one row across different worktrees.It fails silently and successfully. The script re-sources
$ORCA_AGENT_HOOK_ENDPOINT, so port and token self-heal and the POST lands — while the pane key has no refresh path and stays wrong. Nothing errors anywhere.Why
CLAUDE_JOB_DIR, and why not the alternativesFive discriminators were measured on a real rig. Four are unusable:
CLAUDE_JOB_DIRpsps??tooThe payload was diffed field by field between a backgrounded and a non-daemon session:
cwd,hook_event_name,permission_mode,promptidentical;transcript_pathdiffers only in the session-id filename component, both under the ordinaryprojects/<encoded-cwd>/directory. Fields present only in one: none. So there is nothing to key on at ingest without a script change.CLAUDE_JOB_DIRis present in the worker, absent from the foreground — and independently confirmed absent from all 68 live Claude sessions on this machine. It costs one shell variable test.Why decline rather than post without a pane key
There is no third option.
normalizeHookPayload(src/shared/agent-hook-listener.ts:4404) rejects an empty or absentpaneKeyoutright, andAgentHookEventPayload.paneKeyis a requiredstring— there is no "session exists but has no pane" representation. Posting with the key omitted is identical in effect to declining, minus a wasted round trip per event. A backgrounded session genuinely has no pane; attributing it to nothing is correct, and attributing it to a stranger is the bug.The shape already exists one branch above: the
DEVIN_PROJECT_DIRearly-exit 0.The Windows placement is deliberate
The guard sits with the environment guards and uses
exit /b 0rather than jumping to the stdin-drain label. The drain parks inmore.com, and the comment at that site records why the env guards must outrank the Devin skip: "outside an Orca pane the caller can abandon stdin, so more.com never returns." A daemon worker is outside an Orca pane by definition — precisely the #11549 hang. Backgrounded sessions on Windows use the same daemon architecture over named pipes, so this branch is load-bearing, not theoretical.Linked Issue
Fixes #9236
Closes STA-4769 — that ticket asks to "reland the intended background-session fix" from #14615. There is nothing to reland: #14615 was foreground-only, built on a daemon premise that does not reproduce, and had no background-session handling. This PR is that fix, written from the mechanism that does reproduce.
Note that #9236's stated mechanism — that Claude Code hosts foreground TUI sessions under a shared daemon — does not reproduce. Foreground panes are direct children of their own pane's shell with a correct pane key (68 distinct keys, zero collisions across the live fleet). The reporter observed a real failure and generalised the mechanism too far. #15295 reverts the fix built on that premise; this PR fixes the mechanism that actually reproduces.
Visual Proof
N/A— the visible effect is the absence of a spurious status row. A before/after would be a sidebar row that stops being overwritten by an unrelated session, which cannot be captured meaningfully in a still. The wire capture above is the evidence.Testing
Two tests added, both proven to observe their own failure — removing the guards turns both red, restoring them turns both green:
declines to post from a daemon worker, before spawning curl— asserts the guard exists and precedes thecurlit is meant to prevent, and that neutral JSON is still emitted first for permission hooks that fail closed ([Bug]: Windows shell command failures #14818). Ordering is the whole point; a guard after the post is worthless.exits rather than draining stdin on Windows, where a worker has no Orca pane— pins the exactexit /b 0form and asserts it does not route to the drain label.The Windows test stubs
process.platformso it actually executes rather than beingskipIf-gated like the file's other Windows assertions. Those skip on Linux CI and would never have run.Gates:
pnpm typecheckexit 0;vitestoversrc/main/claude+src/main/agent-hooks— 96 files, 927 passed, 7 skipped;oxfmt --checkclean on both changed files.Review
Agent skill upstream boundary
Notes
Cross-platform: both script branches covered. macOS/Linux via the POSIX script, Windows via the
.cmd. Nopsdependency, which is what ruled out the alternatives.Remote SSH: closed for free — the hook script runs on the host that owns the PTY, so the variable is set by the same process on the same host. No wire change.
Backwards compatibility: no wire, schema, or persisted-state change. Behaviour for foreground panes is byte-identical.
Performance: one shell variable test; strictly less work when it fires.
Known limitation: the guard depends on an upstream variable name. If it were renamed the guard would silently stop working — but it fails open, back to today's behaviour, never worse. An upstream request for a payload-level
jobId/kindwould retire the dependency; drafted separately.statusline-script.tsis guarded too — I had assumed a status line was a TUI concern and therefore unreachable for a backgrounded session. That was wrong, and measuring it corrected me. It is invoked inside the worker, with no client ever attached, and its ancestry terminates at the daemon rather than at any pane:A second session dispatched from a different pane saw the first pane's
ORCA_PANE_KEYalongside its own correct session id — so this file was a live second producer of the same bug. Same guard, same placement reasoning:exit /b 0before stdin is owned on Windows, after capture on POSIX (exiting mid-write there surfaces as EPIPE the agent can see, [Bug] Codex hooks failing when launched via Orca (from Grok) #8110).Pre-existing, deliberately not touched: the statusline's own
ORCA_PANE_KEYgate jumps to the stdin-drain label, which by the [Bug]: Codex managed hook leaves persistent cmd.exe windows outside Orca on Windows #11549 contract is the shape to avoid for a guard meaning "no Orca pane". That predates this PR and changing it belongs in its own change.Adjacent weakness, not fixed here:
server.ts:2170takesworktreeIdstraight from the post with no validation, whiletabIdtwo lines above is derived from the parsed pane key. The same asymmetry exists innormalizeHookBodyPaneKeyAlias, which rewritespaneKeyandtabIdto the current owner and leavesworktreeIdas posted. This guard removes the proven producer of a wrongworktreeId; I could not prove the alias path can cross worktrees, so I'd call the remainder a consistency fix on principle rather than a confirmed live bug. Worth its own small change, not urgent.Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)Author