Skip to content

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

Merged
brennanb2025 merged 2 commits into
mainfrom
brennanb2025/pane-key-attribution-9236
Aug 17, 2026
Merged

brennanb2025 merged 2 commits into
mainfrom
brennanb2025/pane-key-attribution-9236

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 5 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​530 $\color{#cf222e}{\Huge{\mathbf{−}}}$​2 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​528
Prod 11 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​481 $\color{#cf222e}{\Huge{\mathbf{−}}}$​70 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​411

What you see, before and after

Before this fix: you open Claude in a terminal and start working. The spinner shows up on some other terminal — frequently in a different workspace — while the pane you are actually typing in sits blank. Your sidebar tells you an unrelated old terminal is busy and that your real work is idle. Nothing you do in the pane fixes it, because the status was never addressed to that pane in the first place.

After this fix: the spinner appears on the terminal that is actually running Claude.

Why it happens. Newer Claude Code stopped giving each terminal session its own process. It hands them all to one shared background helper, and that helper is started by whichever pane opens Claude first. The helper then reports every later session under that first pane's identity — so pane #1 gets the status for work happening in pane #4.

That also explains the shape of the bug: it only appears once you have used Claude in more than one pane, it survives restarts of the individual panes, and the wrongly-labelled row is often somewhere you are not even looking.

How it is fixed. At the moment Orca launches Claude it is the only moment when both facts are known together — which pane this is, and which conversation is starting. Orca now mints a session id, passes it to Claude, and records "this conversation belongs to this pane." When a status arrives later carrying the wrong pane, that note is used to redirect it to the right one.

What Changed

A session→pane binding, established at spawn (src/main/agent-hooks/agent-session-pane-bindings.ts)

A bounded (512, least-recently-bound eviction) registry keyed by (hook source, session id) holding { paneKey, ptyId, worktreeId }. Bindings are dropped when the spawning PTY dies, so a dead pane can never claim a later session.

Spawn-side pin (src/shared/claude-session-pin-launch-command.ts, src/main/ipc/pty.ts)

Claude launches get a minted --session-id <uuid> spliced in, and the resulting binding is registered against the pane's key. The splice reuses the repo's existing shell-aware machinery rather than a new regex: tokenizeStartupCommand + findClaudeExecutableIndex (lifted verbatim out of agent-resume-launch-command.ts into claude-launch-executable-token.ts) + isFullyModelableStartupCommand (the modelability guard that was inline in buildClaudeResumeLaunchCommand, now shared with it).

It fails closed — this is an attribution improvement, never a launch requirement, so any doubt leaves the command byte-for-byte alone:

Command Pinned?
claude, claude "fix it", FOO=1 claude, /usr/local/bin/claude --model opus, & claude (pwsh), claude -- --raw yes
claude mcp list, claude doctor, claude update and every other subcommand yes — pin goes before the subcommand
already carries --session-id / --resume / --continue / --fork-session / -r / -c no
claude && deploy, claude; x, claude | tee, claude > out (operators tokenize as shell-divergent) no
nvm exec claude (not in command position), ssh -i ~/.ssh/claude host (merely ends in /claude) no
PowerShell --% stop-parsing no

The pin is spliced immediately after the executable token, not appended. --session-id is a
root option, so claude mcp list --session-id <uuid> exits with error: unknown option '--session-id' — a trailing pin would break subcommand launches that work today. Root position is
also, by construction, ahead of claude's own -- terminator. Verified against the real binary
(Claude Code 2.1.233).

Ingest-side correction (src/main/agent-hooks/server.ts)

Both seams resolve the binding before pane-migration aliasing, so aliasing still applies on top:

  • HTTP path — normalizeHookBodyAgentSessionPane runs ahead of normalizeHookBodyPaneKeyAlias.
  • Relay path — ingestRemote resolves from the envelope's providerSession.

A corrected event has its tabId rewritten too (normalizeHookPayload rejects a tabId/paneKey disagreement outright) and its worktreeId, since the same inherited env named the daemon's workspace.

The session id is read by a new readHookBodyProviderSessionId in agent-hook-listener.ts rather than a second parser — it reuses parseAgentHookJson and extractAgentProviderSession, the exact path normalizeHookPayload takes, including the Codex-child agent_id carve-out. Correction must run before normalizeHookPayload, whose prompt/compaction state machine is keyed on the posted pane key.

Not touched deliberately: an unbound session (hand-typed claude, anything Orca didn't launch) passes through unchanged. This mechanism can only ever move status onto a pane Orca itself started.

A known gap, stated plainly: the "already carries a selector" row above is not only the user's own flag — Orca itself writes --resume <id> via buildClaudeResumeLaunchCommand, so every session-restore / cold-restore Claude pane is unpinned and keeps the daemon's inherited key. Fresh spawns are fixed here; restored panes are not. Binding the fork at the point Orca issues the resume is the follow-up, and is the same change the declined roster-lineage mechanism was reaching for.

Why

The pane's identity reaches an agent's hooks only through ORCA_PANE_KEY in the process environment. A worker under a prewarmed daemon inherited that env from a different pane, so the posted key is simply wrong — and normalizeHookPayload requires a valid posted paneKey and has no other pane coordinate in the payload to fall back on.

That settles the design question this cluster raises. Two shapes were on the table:

  • (a) identity that cannot be inherited — establish the binding where Orca still knows the pane.
  • (b) defensive validation — refuse hook status whose reported context disproves its pane.

(a) is the root-cause fix; (b) can only ever convert "status on the wrong pane" into "status on no pane." The pane that is actually running the agent still shows nothing under (b) alone. So (a) ships here, and (b) remains worth having as a bounded fail-safe layered on top — see the superseded-PR notes below.

Session-scoped rather than process-scoped on purpose: the daemon's worker is not the pane's PTY and outlives it, so process ancestry proves nothing. Transport-agnostic for the same reason — a remote host's daemon inherits a stale key exactly like a local one, which is why the relay seam is covered too and why the pin is not gated on !connectionId (that flag guards local Claude account switching, not attribution).

Binding a host-side pane to a process-side reporter by injecting a session id at launch, with a guard for the user's own session flags, is the conventional approach across comparable agent-workspace tools; this follows it rather than inventing a mechanism.

Linked Issue

Fixes #9236 (STA-2069)

Supersedes #9237 (@BrianDai22), which diagnosed this correctly and independently. That PR is CONFLICTING against main and layers three mechanisms; this one keeps the first and declines the other two:

  • Spawn pin — kept, but spliced through the repo's existing shell-aware tokenizer instead of a new APPENDABLE_CLAUDE_COMMAND_RE, and fail-closed instead of best-effort. That matters on Windows, where the command may be parsed by PowerShell, cmd, or Git Bash.
  • Fork lineage via ~/.claude/daemon/roster.json — declined. It reverse-engineers an undocumented internal file of a third-party CLI, at a path derived from a hook-supplied transcript_path.
  • Prompt-time rebind to "the pane with the freshest keystroke within 5s" — declined. It can mis-bind an unrelated pane; that PR documents the residual case itself.
    Both declined mechanisms address real follow-on gaps (resume forks the session id; the fleet view attaches without a spawn). They are better handled by binding the fork explicitly at the point Orca issues the resume, which is a separate change.

Related, not superseded:

Explicitly NOT claimed fixed by this PR, despite being in the same symptom cluster — each is a distinct root cause:

Visual Proof

Validated live in an isolated Orca dev instance (own ORCA_DEV_USER_DATA_PATH, own CDP port,
Playwright over CDP — never computer-use), against real Claude Code 2.1.233.

The spawn-side pin is real, not just unit-tested. Orca's own Claude launcher writes the minted
id into the pty:

spawn pin

$ claude '--dangerously-skip-permissions' --session-id 05f70dcc-…

Before / after, same app, same persisted workspace, one line of code different. In both runs an
identical UserPromptSubmit was POSTed to the local hook listener carrying a different pane's key
(what a daemon worker sends when it inherited another pane's ORCA_PANE_KEY) plus the session id
Orca pinned at spawn. Only server.ts's ingest seam differed; main was rebuilt for each run.

fix reverted fix restored
status landed on 6083dd99…:dce8f439… — Terminal 1, the wrong pane ba8c89f2…:26b518cb… — the pane that spawned the session
pane actually running Claude shows nothing shows the working row

How to read these two screenshots. Look at the sidebar agent rows, and specifically which terminal name the working spinner sits next to. Claude is running in the Claude tab in both runs; only the ingest seam differs.

before

Before — the spinner is beside Terminal 1, which is not running Claude. The Claude tab (Terminal 3) has no row at all. This is the user-visible bug: the wrong terminal looks busy and the right one looks idle.

after

After — the spinner is beside Terminal 4, the tab actually running Claude, and Terminal 1 is clean.

One thing to be clear about, because it is why these images look synthetic: the daemon-inheritance condition is impractical to provoke on demand, so the hook POST that a daemon worker would send — an identical UserPromptSubmit carrying a different pane's key alongside the real pinned session id — was injected directly into the local hook listener. Everything downstream of that POST is production code. The spawn-side pin above is genuine, unforced Orca behaviour. So: real pin, real ingest path, real rows; a hand-delivered envelope.

Split panes, and "no pane" ruled out. With a Claude pane and a plain sibling in one tab, the
corrected row resolves to the Claude pane's key — it carries data-focused-agent-pane="true" while
that pane is focused, the sibling never gets a row, and the tab-level chip is not what is being
matched. A bound session and an unbound one posting the same wrong key land on two different rows
simultaneously, which is the passthrough path (pre-fix behaviour) and the corrected path side by side:

split panes

Testing

  • I manually tested these changes locally
  • Automated tests added/updated, or explained why not below

What is and is not verified live. Verified in the running app (see Visual Proof): the pin lands on the command Orca writes, the spawn records the binding, and an event posted with another pane's key is re-filed onto the spawning pane — with a true before/after across a code revert. Not reproduced: the daemon causing the wrong key in the first place. I did not stand up a Claude daemon already holding another workspace's environment; the inherited-key payload was posted directly, which is byte-identical to what such a worker sends but does not independently confirm #9236's inheritance mechanics (ps eww, the roster's "source": "spare" dispatch, the env allowlist), which are taken from that report.

New tests (vitest --config config/vitest.config.ts):

  • src/main/agent-hooks/server-agent-session-pane-attribution.test.ts — real routing outcomes over the live HTTP listener and ingestRemote, asserting the resulting status rows, not mock calls:
    • a daemon-hosted turn posting the inherited key lands on the spawn-pinned pane;
    • a corrected event is re-filed under the spawned workspace, not the daemon's;
    • two split panes in one tab (same tabId, different leaf) posting the same inherited key keep separate rows with separate prompts — without the binding both collapse and the second prompt clobbers the first;
    • an unpinned session stays on the pane it posted (hand-typed claude is unaffected);
    • re-routing stops once the pinning PTY exits;
    • a codex binding cannot capture a claude post reusing the same id;
    • a $$<hash>:L$$ pane key fails isValidPaneKey and a binding cannot introduce one;
    • readHookBodyProviderSessionId across the object and JSON-string payload forms, the Codex-child carve-out, and malformed payloads.
  • src/main/agent-hooks/agent-session-pane-bindings.test.ts — source scoping, re-bind follows the pane, PTY-exit cleanup hits only that PTY, cap eviction is least-recently-bound (a re-bound entry survives), incomplete bindings are refused.
  • src/shared/claude-session-pin-launch-command.test.ts — 21 cases covering the pin/no-pin table above across posix, cmd and PowerShell.
  • src/main/pty/spawn-startup-shell.test.ts — dialect resolution: SSH and WSL panes are posix even on a Windows host; cmd and Git Bash overrides recognized.
  • src/main/ipc/pty.test.ts — the existing startup-command timing test now asserts the written command matches claude --session-id <uuid>, making the spawn-side pin covered end to end.

Red/green oracle. With both ingest seams reverted to their previous single line, exactly the 3 routing assertions fail and the remaining 8 passthrough/negative assertions still pass (they assert preserved behavior). Restored: 11/11.

Regression scope. src/main/agent-hooks, src/main/pty, src/main/ipc/pty, src/relay, src/shared/agent-hook-listener, src/shared/agent-resume-launch-command, src/shared/claude-session-pin-launch-command, src/main/ssh — 3055 passed, 12 failed. All 12 failures are pre-existing: I ran the same files on a clean stash of this branch's base and got the identical 12 (10 in pty.test.ts around MiMo/Pi/OMP overlay env and WSL handles, 2 in the SSH/Codex-prefill suites). Nothing this PR touches fails.

The extraction into isFullyModelableStartupCommand / claude-launch-executable-token.ts is behavior-preserving: agent-resume-launch-command.test.ts is 104/104 before and after, untouched.

typecheck passes on all three projects (node, cli, web) run individually. oxlint, both --type-aware and native code-quality configs, oxfmt, and the max-lines ratchet pass on every changed file. No max-lines suppression added. I did not run the full pnpm test / pnpm build; CI covers those.

Platforms. Written on macOS. Cross-platform handling is in the code rather than in my test matrix: the splice dialect is resolved explicitly (resolveSpawnStartupShell) instead of read off process.platform, so an SSH pane and a WSL pane are treated as POSIX even when Orca runs on Windows, and a Windows pane pointed at cmd or Git Bash is not treated as PowerShell. Folder workspaces are unaffected — nothing here reads git state. I did not exercise Windows or a live SSH host.

Review

  • Security. The only new untrusted input is the hook payload's session id, used as an exact Map key for lookup — never as a path, glob, or command fragment. No new filesystem access, no third-party internal files read. The minted id is randomUUID(), so no user input is interpolated into the command string, and it is only ever spliced into a command that fully tokenizes. Existing x-orca-agent-hook-token auth is unchanged. The registry is bounded at 512 with PTY-exit cleanup.
  • Cross-platform. See Platforms above.
  • Remote SSH. Covered on purpose — the relay ingest seam resolves the same binding, and the pin is not gated on locality.
  • Mobile. No wire change: no new RPC params, no new stream opcodes, and nothing new published to a paired client. A corrected paneKey/worktreeId is the same field shape an old client already handles.
  • Backwards compatibility. Unbound sessions take the pre-existing path byte for byte. Nothing is persisted: the registry is in-memory only, so no state-file migration and no downgrade hazard.
  • Performance. One Map lookup per hook event, on a parse the ingest path already performs.

Author

  • X / Twitter: @BrennanKB5

@coderabbitai

coderabbitai Bot commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The change adds shell-aware Claude session pinning for eligible PTY launches. It binds pinned sessions to PTYs, panes, tabs, and worktrees, and clears bindings during PTY cleanup. AgentHookServer resolves provider sessions and rewrites local and remote hook attribution before pane-key alias handling. Shared helpers support shell detection, executable lookup, command validation, and provider session extraction. Tests cover command rewriting, PTY wiring, binding behavior, and hook routing.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The changes satisfy issue #9236 by pinning eligible Claude sessions, correcting HTTP and relay attribution, preserving unbound behavior, and cleaning up bindings.
Out of Scope Changes check ✅ Passed The production and test changes support session pinning, pane attribution, shell handling, relay ingestion, cleanup, and regression coverage for issue #9236.
Title check ✅ Passed The title clearly identifies the primary change: binding agent status to the pane where its session starts.
Description check ✅ Passed The description explains the change, rationale, linked issue, visual proof, testing, scope, and risks, but omits several non-critical template sections.
✨ Finishing Touches 💡 1
📝 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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/shared/tui-agent-startup-shell.ts (1)

162-172: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Shorten the new TypeScript comment blocks.

  • src/shared/tui-agent-startup-shell.ts#L162-L172: reduce the block to the modelability precondition and remove implementation walkthrough.
  • src/shared/claude-session-pin-launch-command.ts#L28-L44: reduce the block to the attribution purpose and fail-closed behavior.

As per coding guidelines: “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: ccbd4667-f485-42ce-908f-d1032fb0133f

📥 Commits

Reviewing files that changed from the base of the PR and between 9bb8836 and 11286b5.

📒 Files selected for processing (14)
  • 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.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

Comment on lines +19 to +24
const base =
opts.shellOverride?.trim().replaceAll('\\', '/').split('/').pop()?.toLowerCase() ?? ''
if (/^(?:ba|z|k|da|)sh(?:\.exe)?$/.test(base)) {
return 'posix'
}
return base.startsWith('cmd') ? 'cmd' : 'powershell'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Classify Fish as fish.

Line 21 does not recognize fish.exe. The resolver returns powershell for Fish on Windows. pinClaudeLaunchSessionId then tokenizes Fish text as PowerShell text. This can reject eligible launches or produce an invalid rewritten command. Return fish for Fish executables. Add a fish.exe fixture in src/main/pty/spawn-startup-shell.test.ts.

Proposed fix
   const base =
     opts.shellOverride?.trim().replaceAll('\\', '/').split('/').pop()?.toLowerCase() ?? ''
+  if (/^fish(?:\.exe)?$/.test(base)) {
+    return 'fish'
+  }
   if (/^(?:ba|z|k|da|)sh(?:\.exe)?$/.test(base)) {
     return 'posix'
   }

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@brennanb2025
brennanb2025 force-pushed the brennanb2025/pane-key-attribution-9236 branch from 26a7020 to a8b7147 Compare August 17, 2026 08:12
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@brennanb2025
brennanb2025 force-pushed the brennanb2025/pane-key-attribution-9236 branch from a8b7147 to d9ac901 Compare August 17, 2026 08:13
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Validation pass — $electron live A/B + review loop

Two defects found and fixed on this branch, one of them launch-breaking. The PR was also CONFLICTING, so it had never produced a CI run; it is now rebased and checks are running for the first time.

P1 — the pin broke every claude <subcommand> launch

--session-id is a root option. Appending it, as the original patch did, produced:

$ claude mcp list --session-id aaaaaaaa-bbbb-4ccc-8ddd-eeeeeeeeeeee
error: unknown option '--session-id'

Verified against the real binary (Claude Code 2.1.233), and reproduced through the shipped code path — pinClaudeLaunchSessionId pinned every subcommand it was given: mcp list, daemon status, doctor, update, config get, setup-token, install, migrate-installer. Reachable from any spawn carrying args.command (custom startup commands, agent command overrides, orca terminal create --command). This also contradicted the function's own stated "fails closed" contract — the pin was not attribution-only, it made the launch fail.

Fixed by splicing the pin immediately after the executable token instead of appending. Confirmed against the real binary:

claude --session-id <uuid> mcp list   -> No MCP servers configured. ...
claude --session-id <uuid> doctor     -> Claude Code doctor ...
claude --session-id <uuid> --version  -> 2.1.233 (Claude Code)

Root position is also, by construction, ahead of claude's own -- terminator, so the special-case terminator branch collapses into the general rule. Five subcommand cases added to the table test.

P2 — the new file was a binary blob

agent-session-pane-bindings.ts contained a literal NUL byte (`${source}<NUL>${sessionId}`) rather than an escape, so git classified it as binary — Bin 0 -> 3288 bytes in the diffstat, no reviewable diff, no blame, no textual merge. Now written as a \u0000 escape; file(1) reports UTF-8 text.

Rebase hazard caught

The one merge conflict was with #15018 ("one portable Unix startup dialect"), which changed findClaudeExecutableIndex to use isPosixStartupShell(shell) instead of shell === 'posix'. This PR had extracted that function into a new file carrying the pre-#15018 predicate — so resolving the conflict the obvious way would have silently reverted main's fish handling inside the extracted copy. Main's version is carried into claude-launch-executable-token.ts.

Mutation testing — three hunks no test could fail on

Every production hunk was reverted individually. Most were properly gated; these three were not, and now are:

Hunk Before Now
pty.ts — both bindAgentSessionPane call sites + clearAgentSessionPaneBindingsForPty deletable with 3009/3009 src/main/ipc tests green new spawn test asserts the id on the command line equals the id recorded against the pane
ingestRemote's boundPane?.worktreeId ?? no test failed relay test binds a worktreeId and asserts it
resolveBoundPaneOverride's bound.paneKey === postedPaneKey early-return no test failed asserts an already-correct pane is not restamped from the binding

Each was re-mutated after the fix to confirm it now goes red. Also fixed a vacuous parameterized case — 'refuses to compete with an existing --fork-session selector' used claude --resume abc --fork-session, which the --resume clause already rejected, so it never exercised the flag it was named after; and added the missing =-form selectors (--session-id=, --continue=, -r=, -c=), 5 of 11 clauses previously deletable with all cases green.

Supersede claim vs #9237 (@BrianDai22) — audited

The body now also discloses a gap it had understated: Orca itself writes --resume via buildClaudeResumeLaunchCommand, so every session-restore pane is unpinned and keeps the daemon's key. Fresh spawns are fixed; restored panes are not.

Live validation

Isolated dev instance (own ORCA_DEV_USER_DATA_PATH, own CDP port, Playwright over CDP — no computer-use), real Claude Code 2.1.233. True before/after across a code revert with main rebuilt each time: identical POST, wrong pane key, fix off → lands on Terminal 1; fix on → lands on the pane that spawned the session. The pane running the agent shows the row, so this does not convert "wrong pane" into "no pane". Screenshots in the PR body.

Not reproduced: the daemon causing the stale key. The inherited-key payload was posted directly — byte-identical to what such a worker sends, but it does not independently confirm #9236's inheritance mechanics.

Gates

oxfmt clean on all 16 files; oxlint clean (gate proven live by injecting a no-debugger violation); tsc --noEmit clean on the node project; 70 test files / 805 tests green on the rebased tree. No max-lines suppression added.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/shared/claude-session-pin-launch-command.ts (1)

28-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Shorten the rationale comments.

Keep the non-obvious invariant. Remove release history and test-suite mechanics from inline comments.

  • src/shared/claude-session-pin-launch-command.ts#L28-L43: reduce the function documentation to the pinning contract and fail-closed condition.
  • src/main/ipc/pty-login-shell-startup-commands.test.ts#L247-L250: reduce the test comment to the production binding invariant.

As per coding guidelines, “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: 2ebda63a-bc08-4e2f-a9dd-c8171410c73a

📥 Commits

Reviewing files that changed from the base of the PR and between c7995a6 and d9ac901.

📒 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 skipped from review as they are similar to previous changes (12)
  • src/main/ipc/pty-ipc-mock-registry.ts
  • src/main/pty/spawn-startup-shell.test.ts
  • src/shared/agent-resume-launch-command.ts
  • src/main/ipc/pty-ipc-suite-environment.ts
  • src/main/pty/spawn-startup-shell.ts
  • src/shared/claude-launch-executable-token.ts
  • src/shared/tui-agent-startup-shell.ts
  • src/main/agent-hooks/agent-session-pane-bindings.test.ts
  • src/main/ipc/pty.ts
  • src/main/agent-hooks/server-agent-session-pane-attribution.test.ts
  • src/shared/agent-hook-listener.ts
  • src/main/agent-hooks/server.ts

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

brennanb2025 and others added 2 commits August 17, 2026 11:26
…ed into (STA-2069)

Claude Code >= 2.1.206 hosts TUI sessions as workers under a shared daemon,
and the daemon forwards only its own allowlisted env — so hook posts carry
whichever pane first started the daemon, not the pane the user is in.

Pin a minted --session-id at spawn where Orca still knows the pane, record
sessionId -> pane, and correct the posted key at both hook ingest seams.

Co-authored-by: Brian Dai <43929761+BrianDai22@users.noreply.github.com>
…ended

Appending `--session-id <uuid>` broke every `claude <subcommand>` launch:
`--session-id` is a ROOT option, so `claude mcp list --session-id <uuid>`
exits with "error: unknown option '--session-id'". Splice it immediately
after the executable token instead, which is valid for both a bare session
and a subcommand, and is already before claude's own `--` terminator.

Also write the binding-key separator as an escape rather than a raw
NUL byte, which made the file a binary blob in git.

Close three hunks that no test could fail on: the pty.ts spawn call site
that records the binding, the relay seam's worktreeId override, and the
already-correct-pane early-return that suppresses a worktree restamp.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/pane-key-attribution-9236 branch from d9ac901 to fae16a9 Compare August 17, 2026 18:27
@brennanb2025
brennanb2025 merged commit 15efc87 into main Aug 17, 2026
45 checks passed
brennanb2025 added a commit that referenced this pull request Aug 18, 2026
…as spawned into (STA-2069) (#14615)"

This reverts commit 15efc87.

The premise does not reproduce, the fix 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 as
   workers under a shared daemon. On 2.1.233 `claude daemon status` reports
   "not running" while 69 interactive sessions are live, and all 68 running
   clients are direct children of their own pane's shell. Measured across the
   fleet: 68 distinct pane keys, ZERO collisions. The 19 keys with no live pane
   also have a dead ORCA_TERMINAL_HANDLE (pane closed, process outlived it).
   Terminals live under a *different* pane key: 0. Foreground attribution was
   never broken, so the correction has nothing to correct.

2. DOES NOT FIX THE REAL BUG. The failure #9236 describes is real but scoped to
   BACKGROUNDED sessions: a `--bg` / `/background` worker inherits the
   dispatching pane's whole ORCA_* set, and the hook script self-heals its
   endpoint while taking paneKey straight from ambient env, so the POST lands
   under the wrong pane. #14615 mints a binding only for launches Orca
   constructs, so a typed `claude --bg` produces none. That needs a `kind: "bg"`
   guard at ingest instead; #9236 is reopened for it.

3. INTRODUCES A MISATTRIBUTION. Bindings are created at two spawn sites and
   removed only on PTY death. A pane whose user exits Claude keeps its PTY, so
   the binding survives; 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. That is the reported symptom, on a path
   that previously worked.

Kept #14706's observations.rebind() in the conflicting hunk — it postdates
#14615 and is not part of this revert; only #14615's comments were dropped and
the surviving rationale reworded to alias resolution, which is what actually
makes paneKey differ from envelope.paneKey now.
brennanb2025 added a commit that referenced this pull request Aug 18, 2026
…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.
@AmethystLiang

Copy link
Copy Markdown
Contributor

This change was reverted in #15295 because the foreground attribution path needed a narrower scope. Please reland the intended background-session fix with the corrected behavior; tracking: https://linear.app/stably/issue/STA-4769.

paidaxingyo666 pushed a commit to paidaxingyo666/Manta that referenced this pull request Aug 21, 2026
…ed into (STA-2069) (stablyai#14615)

* fix(agent-hooks): bind agent status to the pane the session was spawned into (STA-2069)

Claude Code >= 2.1.206 hosts TUI sessions as workers under a shared daemon,
and the daemon forwards only its own allowlisted env — so hook posts carry
whichever pane first started the daemon, not the pane the user is in.

Pin a minted --session-id at spawn where Orca still knows the pane, record
sessionId -> pane, and correct the posted key at both hook ingest seams.

Co-authored-by: Brian Dai <43929761+BrianDai22@users.noreply.github.com>

* fix(agent-hooks): pin the session id in root-option position, not appended

Appending `--session-id <uuid>` broke every `claude <subcommand>` launch:
`--session-id` is a ROOT option, so `claude mcp list --session-id <uuid>`
exits with "error: unknown option '--session-id'". Splice it immediately
after the executable token instead, which is valid for both a bare session
and a subcommand, and is already before claude's own `--` terminator.

Also write the binding-key separator as an escape rather than a raw
NUL byte, which made the file a binary blob in git.

Close three hunks that no test could fail on: the pty.ts spawn call site
that records the binding, the relay seam's worktreeId override, and the
already-correct-pane early-return that suppresses a worktree restamp.

---------

Co-authored-by: Brian Dai <43929761+BrianDai22@users.noreply.github.com>
paidaxingyo666 pushed a commit to paidaxingyo666/Manta that referenced this pull request Aug 21, 2026
…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.
dallascrilley pushed a commit to dallascrilley/orca that referenced this pull request Aug 27, 2026
…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Claude Code >=2.1.206 shared daemon breaks agent-status pane attribution — hooks inherit the daemon's stale ORCA_PANE_KEY

2 participants