Revert "fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069)" (#14615)

[brennanb2025]

	Files	Added	Deleted	Net
Test	5	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​2	$\color{#cf222e}{\Huge{\mathbf{−}}}$​530	$\color{#cf222e}{\Huge{\mathbf{−}}}$​528
Prod	11	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​72	$\color{#cf222e}{\Huge{\mathbf{−}}}$​483	$\color{#cf222e}{\Huge{\mathbf{−}}}$​411

ELI5

We shipped a fix for a bug based on how we thought Claude Code worked. It doesn't work that way. The thing we "fixed" was never broken, the thing that is broken isn't covered, and the fix can send an agent's status to the wrong pane in a case that used to work. So: undo it.

What Changed

Reverts #14615 (15efc87e35) in full — the minted --session-id pin, the AgentSessionPaneBindings table, and the two ingest-side pane corrections.

One deliberate deviation from a pure revert: the conflicting hunk in src/main/agent-hooks/server.ts contains this.observations.rebind(paneKey), which comes from #14706 (26bdfc0fe4), not #14615. That call is kept. Only #14615's comments were dropped, and the surviving rationale reworded — post-revert, paneKey differs from envelope.paneKey because of legacy-alias resolution, not daemon correction.

Why

1. The premise does not reproduce. #14615 states Claude Code >= 2.1.206 hosts TUI sessions as workers under a shared daemon that forwards only its own allowlisted env. On 2.1.233, claude daemon status reports not running while 69 interactive sessions are live, and every running client is a direct child of its own pane's shell. Every daemon reference in the CLI changelog scopes to background sessions; the three acknowledged stale-env fixes all read "from the shell that started the background daemon."

2. Foreground attribution was never broken. Reading the environment of all 69 live clients against 200 Orca terminals: 68 distinct pane keys, zero collisions. The 19 keys with no live pane also have a dead ORCA_TERMINAL_HANDLE — the pane closed and the process outlived it; key correct, pane gone. Terminals still live under a different pane key: 0. No adoption drift, no re-keying. 65 of 69 clients predate the current Orca main process, so app restart is well covered by accident and shows no drift either. A correction with nothing to correct is pure risk.

3. It does not reach the failure that does reproduce. The real bug is scoped to backgrounded sessions. Reproduced in an isolated config dir — pane A starts a background session, pane B starts one on the same daemon, and pane B's worker inherits pane A's entire ORCA_* set:

pid 60709  (worker running the session dispatched from pane B)
  ORCA_PANE_KEY=PANE-A-AAAA      <- pane B's session, pane A's key

The tree is daemon run (ppid 1) -> bg-pty-host -> bg-spare, detached from every pane PTY. It fails silently and successfully because src/main/claude/hook-service.ts:100-116 re-sources $ORCA_AGENT_HOOK_ENDPOINT — port and token self-heal so the POST lands — while paneKey=${ORCA_PANE_KEY} comes straight from ambient env with no refresh path. #14615 mints a binding only for launches Orca constructs, so a typed claude --bg or an in-session /background produces none.

4. It introduces a misattribution on a path that worked. Bindings are created at two spawn sites (pty.ts:5477, :7163) and removed only on PTY death (pty.ts:2164). A user who exits Claude keeps the pane's PTY, so the binding survives. Resuming that session in another pane does not rebind — --resume is in isClaudeSessionSelector, so the pin declines — and resolveBoundPaneOverride then rewrites paneKey and tabId onto the original pane despite a correct posted key. Status lands on the wrong pane: the exact symptom #14615 set out to fix.

Linked Issue

Reopened #9236 with the reproduction and the correct mechanism. This PR removes the incorrect fix; the actual fix is a kind: "bg" guard at ingest and will land separately.

Refs #9236

Visual Proof

N/A — this restores previously shipped behavior. There is no UI change; the observable difference is that hook events keep the pane key they post, which is what the fleet measurement above shows is already correct.

Testing

macOS. Verified against the real Claude Code fleet on this machine (2.1.227 / 2.1.233, 69 live sessions), not against mocks:

• claude daemon status -> not running with 69 live interactive sessions; no daemon run or bg-pty-host process for any foreground pane.
• Environments of all 69 clients read directly and cross-checked against orca terminal list (200 terminals): 68 distinct pane keys, 0 collisions, 0 terminals live under a different key.
• Background misattribution reproduced in an isolated CLAUDE_CONFIG_DIR on its own daemon socket; probe daemon stopped and all probe state removed afterward.

Gates, run locally on the rebased branch:

• pnpm typecheck — exit 0. Proven live, not assumed: injecting const x: number = "s" made it exit 1 with TS2322, then reverted. This repo's typecheck prints nothing on success, so a silent pass is otherwise indistinguishable from a dead gate.
• vitest run --config config/vitest.config.ts over src/main/agent-hooks, src/shared/agent-hook-listener, src/main/ipc/pty, src/main/pty, src/shared/tui-agent-startup-shell, src/shared/agent-resume-launch-command — 162 files, 1772 passed, 13 skipped.
• oxfmt --check — clean, echoed "on 9 files" so the list actually arrived.

Revert-safety checks, because a revert can silently delete a later commit's work:

• Enumerated every commit touching each of the 16 reverted paths after 15efc87e35. Three files had one: server.ts and agent-hook-listener.ts (feat(agent-status): step 1 — stamp observation provenance and ordering at every status ingress (STA-4293) #14706), pty.ts (fix(runtime): stop one unreachable relay from freezing every workspace as active (STA-517) #14649).

• server.ts — conflict; kept feat(agent-status): step 1 — stamp observation provenance and ordering at every status ingress (STA-4293) #14706's observations.rebind (confirmed via git log -S that feat(agent-status): step 1 — stamp observation provenance and ordering at every status ingress (STA-4293) #14706 introduced it, and that fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) #14615's diff never mentions rebind).

• agent-hook-listener.ts — auto-merged; feat(agent-status): step 1 — stamp observation provenance and ordering at every status ingress (STA-4293) #14706's isNewTurnEvent confirmed still present, only fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) #14615's readHookBodyProviderSessionId removed.

• pty.ts — auto-merged; fix(runtime): stop one unreachable relay from freezing every workspace as active (STA-517) #14649's STA-517 markers all confirmed present (STA-517, onSshInventoryUnavailable, listProcessesWithHostScope, deadlineMs, markSshInventoryUnverifiable), and the full diff versus main contains only fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) #14615's removals.

• Zero dangling references to any removed module or symbol.

• I manually tested these changes locally

• Automated tests added/updated, or explained why not below

No tests added: this restores the prior state, and the three test files it removes are #14615's own. The behavior being restored is covered by the 1772 existing tests above.

Review

Agent skill upstream boundary

• Not applicable.

Notes

• Cross-platform: removal only; no platform-conditional code added or left behind.
• Remote SSH: fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) #14615 applied its correction at both ingest seams including the relay path. Removing it restores the posted key on both, which the fleet measurement shows is the correct one for pane-hosted sessions.
• Backwards compatibility: no wire or persisted-state change. AgentSessionPaneBindings was in-memory only.
• Performance: strictly less work per hook event.

Checklist

• This PR is small and focused
• I explained what changed and why (including ELI5)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Author

• X / Twitter: @BrennanKB5

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR removes session-to-pane binding storage, attribution, cleanup, exports, mocks, and related tests. Agent hook routing now uses pane-key aliases and envelope worktree IDs. PTY spawn paths no longer inject Claude session IDs. Claude startup tests now expect the plain command. Shared Claude executable and startup-shell helpers, session-pinning logic, and related tests are deleted. Resume-command handling now performs local Claude executable detection and shell syntax validation.

Merge Risk: 🟡 Moderate · up to d82a5

The command-resume parsing change can rewrite unrelated shell commands in some valid invocations, potentially launching or resuming the wrong command. This bounded correctness issue should be fixed and covered by a regression test before merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies this pull request as a revert of the specific agent-hooks pane-binding fix.
Description check	✅ Passed	The description follows the template, explains the revert and rationale, links the issue, documents testing, and addresses cross-platform and compatibility concerns.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[brennanb2025]

Adding the measurement behind reason 4 in the description, since a code-reading argument is not evidence.

A failing test against origin/main. Built from the existing server-agent-session-pane-attribution.test.ts scaffolding, using the real ingest seam — server.start(), a genuine POST to /hook/claude, then getStatusSnapshot(). Run in a throwaway git archive extraction of origin/main, so no tracked source was modified. The existing suite passes there first (12/12), confirming the harness is sound.

Result on main: 2 failed, 1 passed. The event lands on pane A although the post named pane B:

+ "paneKey": "tab-a:aaaaaaaa-1111-4111-8111-111111111111",
+ "tabId": "tab-a",
+ "worktreeId": "wt-a",

The third case is a control — identical payload, no binding — and it passes, so the fixture is routable and the assertions can be reached. The test observes its own failure case.

Causation isolated. Neutralizing only resolveBoundPaneOverride (return null) in the throwaway copy, changing nothing else:

• the regression test → 3/3 pass
• fix(agent-hooks): bind agent status to the pane its session was spawned into (STA-2069) #14615's own suite → exactly 4 failed / 8 passed, and the 4 are precisely its behavioural tests (daemon-hosted routing, worktree re-filing, split panes, relay path)

So the lever moves this correction and nothing else.

Every link in the chain verified, including one that could have killed the claim.

1. Does the PTY survive /exit? Yes — proven on live processes, not inferred. Every Orca-launched Claude on this machine is a child of a surviving login shell, never the PTY root: claude 16475 <- ppid 16421 : -/bin/zsh -l (and the same for 6647, 8298). clearAgentSessionPaneBindingsForPty is reachable only from clearProviderPtyState (pty.ts:2164), i.e. PTY teardown. It never fires on agent exit.
2. Does the resume path decline the pin? Yes — buildClaudeResumeLaunchCommand produces --resume X, and pinClaudeLaunchSessionId returns null for it. A control in the same test confirms a fresh claude is pinned, so that assertion can go the other way.
3. Any other rebind or invalidate path? No. Two bind sites (pty.ts:5477, :7163), one removal path, and resolve() is a bare Map lookup with no liveness check on the bound ptyId and no check that the bound pane still exists.
4. Does a resumed session keep id X? This was the link that could have refuted the whole thing — if resume minted a fresh id, the stale binding could never match. It doesn't: 19/19 resumed sessions in the live fleet have a registry sessionId byte-identical to their --resume argument, 0 mismatches.
5. Is worktreeId rewritten too? Yes — "worktreeId": "wt-a" above. The event files under pane A's worktree as well as its pane, so this is cross-workspace.

Blast radius, stated fairly. The binding is cleared by PTY teardown (so pane close clears it), by connection disconnect — note that clear sits outside the preserveAgentSessionOwners guard, so it runs even on a reconnectable disconnect — by app restart, since it is an in-memory Map, and by LRU eviction past MAX_BINDINGS = 512. It is not cleared by agent /exit, /clear, or crash. The window is therefore launch → pane close or app restart. On this machine — 69 live sessions, panes alive for days, last app restart 2026-08-17 16:06 — that window is long. This narrows the bug; it does not remove it.

And resuming into a different worktree is a first-class feature (buildAiVaultResumeStartupForWorktree takes a target worktree), so pane A ≠ pane B is the ordinary case here, not a contrived one.

Not added as a test in this PR: it calls bindAgentSessionPane, which this revert removes, so it cannot compile against the post-revert tree. It is evidence for the revert, not a guard to land.

One clarification on a separate point: /clear mints a new session id, which breaks the binding harmlessly — it makes the pin inert rather than misattributing. That is a reason #14615 under-delivers, not a second regression.

[brennanb2025]

The e2e / changed e2e specs failure here is not caused by this PR. Diagnosed and fixed separately in #15303.

The relay builds correctly in the e2e build job — all seven targets — but config/scripts/build-relay.mjs:141,169 writes each marker as a dotfile (out/relay/<platform>/.version), and actions/upload-artifact excludes hidden files by default. The build log states it directly:

Built relay for linux-x64 → out/relay/linux-x64/relay.js
...
##[group]Run actions/upload-artifact@v7
  include-hidden-files: false

So every consumer that actually starts a relay fails with missing its version marker. It reproduced identically across two independent runs here, which ruled out a flake.

It surfaces on this PR only because the diff touches SSH-adjacent code, which makes the changed e2e specs lane set ORCA_E2E_SSH_DOCKER and genuinely start a relay. The sharded e2e lane never sets that variable, so its SSH specs skip and the bug stays invisible on main.

Will rerun this lane once #15303 lands.

[brennanb2025]

Second failure (e2e / ssh docker watcher isolation) is the same cause — 10 occurrences of missing its version marker in that job's log. Both failing jobs on this PR are the stripped relay dotfile, fixed in #15303. Neither is related to this revert.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/shared/agent-resume-launch-command.ts (1)

28-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shorten the new parser comments. Keep each comment to one concise statement of the safety constraint.

• src/shared/agent-resume-launch-command.ts#L28-L31: Reduce the command-position explanation to one line.
• src/shared/agent-resume-launch-command.ts#L103-L107: Reduce the modelability explanation to one line.

As per coding guidelines, "**/*.{ts,tsx,js,jsx}: Comments must be concise, non-obvious, and brief—prefer one line; do not explain obvious behavior or walk through code."

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b7b01234-30c1-4812-8132-49a3d3de109f

📥 Commits

Reviewing files that changed from the base of the PR and between 53bd956 and d82a5cf.

📒 Files selected for processing (16)

• src/main/agent-hooks/agent-session-pane-bindings.test.ts
• src/main/agent-hooks/agent-session-pane-bindings.ts
• src/main/agent-hooks/server-agent-session-pane-attribution.test.ts
• src/main/agent-hooks/server.ts
• src/main/ipc/pty-ipc-mock-registry.ts
• src/main/ipc/pty-ipc-suite-environment.ts
• src/main/ipc/pty-login-shell-startup-commands.test.ts
• src/main/ipc/pty.ts
• src/main/pty/spawn-startup-shell.test.ts
• src/main/pty/spawn-startup-shell.ts
• src/shared/agent-hook-listener.ts
• src/shared/agent-resume-launch-command.ts
• src/shared/claude-launch-executable-token.ts
• src/shared/claude-session-pin-launch-command.test.ts
• src/shared/claude-session-pin-launch-command.ts
• src/shared/tui-agent-startup-shell.ts

💤 Files with no reviewable changes (11)

• src/main/ipc/pty-ipc-suite-environment.ts
• src/shared/claude-launch-executable-token.ts
• src/main/agent-hooks/server-agent-session-pane-attribution.test.ts
• src/main/pty/spawn-startup-shell.ts
• src/main/agent-hooks/agent-session-pane-bindings.ts
• src/shared/claude-session-pin-launch-command.ts
• src/shared/tui-agent-startup-shell.ts
• src/main/pty/spawn-startup-shell.test.ts
• src/shared/agent-hook-listener.ts
• src/main/agent-hooks/agent-session-pane-bindings.test.ts
• src/shared/claude-session-pin-launch-command.test.ts

Included review availability: Your plan includes up to 10 reviews per rolling hour; 0 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Restrict command-position resets to supported wrappers.

Line 50 treats every -- as a wrapper delimiter. For echo -- claude --resume stale, this function identifies claude as an executable and rewrites the unrelated command.

Track recognized wrapper syntax before restoring command position. Add a regression test that keeps this command append-only.