Repository navigation
fix(agent-status): status follows the terminal, not the pane key it was spawned with - #24493
brennanb2025 wants to merge 6 commits into
Conversation
4adee50 to
3165f83
Compare
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review. 📝 WalkthroughWalkthroughThe change carries the pane key exported to a PTY process through daemon and relay listings into runtime records. The runtime resolves eligible keys to current terminal panes and reconciles agent-hook rows when pane bindings change. Hook ingestion uses terminal-aware resolution, while spool replay retains alias-based normalization. The renderer can also adopt a PTY under its recorded tab when that tab is absent and has no closure tombstone. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The change routes agent status to the pane where a surviving terminal is currently mounted. No concrete merge-blocking risk remains from the supplied evidence. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The inspected routing preserves authentication and execution-host checks and rejects ambiguous terminal ownership. No security bypass was established. Remaining uncertainty concerns recovery after interrupted state migration and ownership when an old pane identity is reused. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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: Advanced
Run ID: 62999f1b-29bb-4b02-a363-bfaaaacd9b98
📒 Files selected for processing (40)
docs/reference/agent-status-store.mdsrc/main/agent-hooks/server-terminal-pane-routing.test.tssrc/main/agent-hooks/server/server-authority-aliases.tssrc/main/agent-hooks/server/server-authority-fences.tssrc/main/agent-hooks/server/server-cleanup.tssrc/main/agent-hooks/server/server-ingest-normalization.tssrc/main/agent-hooks/server/server-ingest-remote.tssrc/main/agent-hooks/server/server-state.tssrc/main/agent-hooks/server/server-terminal-pane-routing.tssrc/main/daemon/daemon-pty-session-inventory.tssrc/main/daemon/session-options.tssrc/main/daemon/session.tssrc/main/daemon/terminal-host-env-pane-key.test.tssrc/main/daemon/terminal-host-session-create.tssrc/main/daemon/terminal-host-session-listing.tssrc/main/daemon/types.tssrc/main/ipc/pty/ipc/spawn-commit.tssrc/main/orcad/orcad-entry.tssrc/main/providers/pty-process-info.tssrc/main/providers/pty-process-list-admission.test.tssrc/main/providers/pty-process-list-admission.tssrc/main/runtime/agent-status-store-wiring.test-fixture.tssrc/main/runtime/orca-runtime-get-orchestration-dispatch-authority.tssrc/main/runtime/orca-runtime-on-pty-exit.tssrc/main/runtime/orca-runtime-preserved-branch-cleanup.tssrc/main/runtime/orca-runtime-record-pty-worktree.tssrc/main/runtime/orca-runtime-refresh-pty-worktree-records-with-controller-inventory.tssrc/main/runtime/orca-runtime-register-pty.tssrc/main/runtime/orca-runtime-state-fields.tssrc/main/runtime/orca-runtime-sync-window-graph.tssrc/main/runtime/orca-runtime-terminal-env-pane-key.test.tssrc/main/runtime/runtime-terminal-state-records.tssrc/main/runtime/terminal-env-pane-key-routing.test.tssrc/main/runtime/terminal-env-pane-key-routing.tssrc/main/startup/main-process-runtime-service.tssrc/relay/pty-handler-ownership-attestation.test.tssrc/relay/pty-handler.tssrc/renderer/src/lib/worktree-agent-activation-gate.test.tssrc/renderer/src/lib/worktree-agent-live-surface-adoption.tstests/e2e/cross-version-wire/cross-version-pty-listing-env-pane-key.unit.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.
There was a problem hiding this comment.
Important
The routing never engages for WSL-relayed hooks, so the fix is inert on Windows/WSL folder workspaces, and a stale spool replay can overwrite a live row during reconciliation. Details inline.
Reviewed changes
- Execution host records the key it exported — the daemon (
Session.envPaneKey) and SSH relay publish an optionalenvPaneKeyon PTY listings; the runtime stores it on its PTY record (spawn, inventory, or registration) and clears it on certified exit. - Hook ingress resolves the exported key to the terminal's current pane —
resolveHookPaneKey(HTTP and relay) routes to the live terminal's current pane viaresolveTerminalPaneForEnvPaneKey; spool replay deliberately stays on the posted key. - Store relocates filed rows — new
reconcileMovedTerminalPaneKeysmoves rows, authority, and aliases to the current pane via the extractedmovePaneAuthorityState;transferPaneAuthoritynow reuses that extraction without minting an alias. - Renderer adoption — a recorded tab the renderer no longer has is recreated under its recorded id unless a closed-tab tombstone exists.
- Wire compatibility —
envPaneKeyis optional and bounds-checked; a cross-version suite pins old/new reader skew. - Tests — new runtime, daemon, relay, ingress-routing, admission, and cross-version suites.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
…as spawned with (STA-8862) A terminal that survives a restart keeps exporting the ORCA_PANE_KEY it was spawned with, so hook status landed on a tab that no longer exists. The daemon and SSH relay now record that exported key with the process and publish it as an optional listing field; the hook server resolves a posted key to the pane the live terminal shows now, moves rows already filed under it, and clears both keys on exit. A recorded tab the renderer lost comes back under its own id.
Keep the newer row when the moved key and the live pane both report, drop the loser by its own key, scope ingress routing to the posting host, stop routing for a local record whose exit was observed, and honor the tombstone TTL.
The runtime hands the store resolved moves from one indexed pass, scoped to the terminal's host. A newer moved row fully replaces the pane's older state. Aliases that targeted the moved key are repointed. Exit cleanup skips a key another live terminal shows or exported. Spool replay stays on the posted key. Inventory tolerates a non-string exported key. The closed-tab check uses the tab id alone.
Treat a WSL relay connection as the local host it relays for. Judge which row wins by the age of its evidence, not its delivery, so a stale spool replay cannot replace a live row. Keep the winning row's own launch authority. Follow the alias this terminal's own pane move minted, so its state is moved too.
3165f83 to
6b296cf
Compare
…liases or fences A move rejected because the destination pane holds another host's row had already repointed and persisted the aliases, so later posts falling back to an alias could land on the other host's pane.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Destination host is checked before any mutation —
reconcileMovedTerminalPaneKeysnow rejects a move whose destination pane already carries another host's row before consuming retired restart ids or repointing aliases, so a rejected move leaves fences and aliases exactly as they were (server-terminal-pane-routing.ts:48-55). - Regression coverage — a new test proves a rejected cross-host move does not repoint an alias that falls back onto the exported key (
server-terminal-pane-routing.test.ts:181). - Prior review feedback confirmed addressed — the three threads from the last pullfrog review are fixed in
6b296cfand retired: WSL relay ids normalize to the local host, the reconcile winner is judged by evidence age (evidenceObservedAt ?? receivedAt), and an alias minted for the sameptyIdis followed.
Local verification: server-terminal-pane-routing.test.ts passes 12/12; the PR's other new suites pass (90 passed / 5 skipped). The cross-version suite fails only because the checkout is shallow with no release tags — an environment limitation, not a code issue.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
|
Closing this older implementation and rebuilding status routing at the current terminal owner. Status reports reaching a dead tab after restart remain tracked in #23753. |

ELI5
When Orca restarts, your terminals keep running in the background. Sometimes Orca puts a surviving terminal back into a tab with a brand-new name. The agent inside that terminal (Claude, Codex, …) keeps reporting its status under the old tab's name, because that name was written into the terminal's environment when it started and a running program's environment can never change. So Orca filed the status under a tab that no longer exists, and the real tab showed nothing.
Now the background terminal host remembers which tab name it gave each terminal. When an agent reports under that old name, Orca looks up which terminal it belongs to and files the status on the tab that terminal is showing in now.
What Changed
The problem (GitHub #23753, Linear STA-8862). A terminal survived an Orca quit and relaunch, and the relaunch showed it in a tab with a new id. After that:
orca worktree pslisted the agent under a pane key no tab has;claude --resumein the same tab did not help, because it inherits the same old environment.Why it happened. Every agent hook reports
ORCA_PANE_KEY, the tab-and-pane position the terminal had when it was spawned. That value is frozen in the process environment. When the terminal later shows in a different tab, nothing connected the old key to the terminal, so status was filed by layout position instead of by terminal. After a restart the main process had also lost its in-memory "which terminal carried which key" map, so it could not even verify a move.What you see now.
orca worktree psrow for that agentlast-status.jsonfor up to 7 daysMechanism.
Session.envPaneKey, fromORCA_PANE_KEYat spawn) and the SSH relay (its existing PTY record's pane key) now publish it as an optionalenvPaneKeyfield on their PTY listings, next to the terminal handle they already publish. A reattach never re-exports environment, so the daemon keeps the spawn-time value. The main runtime stores it on its PTY record from that listing, and from the spawn itself for a fresh terminal. It lives exactly as long as the process: no TTL, nothing persisted beside the status store.resolveHookPaneKey: the runtime finds the one live terminal that exported that key and returns the pane it is mounted in now (the same leaf lookup the in-app terminal status path already uses). Only a terminal on the host the post came from qualifies, and a local terminal whose exit was observed does not. Spool replay stays on the posted key, and the move below carries it over. It refuses to guess when the key still names a live pane, two live terminals exported it, or the terminal shows in more than one pane. Those posts land exactly where they do today.transferPaneAuthorityalready does, extracted so it no longer has to mint an alias. If the current pane already has its own row, the newer of the two wins. The loser is dropped by its own key, never through the alias table.orca terminal listids stay stable. A tab the user closed (a closed-tab record within its expiry) still gets a fresh tab. This is not the fix; it just keeps ids stable.Hook scripts do not change.
docs/reference/agent-status-store.mddocuments the new keying contract.Why
The common pattern is that an agent event carries a host-owned terminal or session identity that does not depend on layout, and the layout maps that identity to a pane. Orca's event identity was the layout position itself. This change moves attribution to the execution host, which owns the process and already records its terminal handle, instead of asking the renderer to remember old-to-new mappings.
Alternatives considered:
Differences from the common pattern:
ORCA_PANE_KEYinstead of a new terminal-id field. Safe because the host maps that key to a live terminal through its own record, so the key works as a process identity, and no hook script or agent integration has to change.Mixed versions (
docs/reference/remote-wire-compatibility.md, Rule 1):envPaneKeyand keeps today's behaviour (the saved binding when incarnation ids match, otherwise the row stays where it lands).tests/e2e/cross-version-wire/cross-version-pty-listing-env-pane-key.unit.test.tsruns the newest release's listing readers against rows carrying the field and pins that they read the rest of the row unchanged.SSH, WSL and folder workspaces: the relay records the key it exported, and loss of contact never stops routing (only a certified exit does). A WSL hook relay is transport only, so its posts route to the local terminal. Identity is per terminal, so folder workspaces and git worktrees behave the same.
How this interacts with open work:
Linked Issue
Fixes #23753
Linear: STA-8862
Visual Proof
Real restart in an isolated rig (own HOME and profile, stub agent CLIs). A terminal survives quit and relaunch and is adopted into a tab with a new id. Its shell still exports the old pane key, and the agent hook is posted from that shell.
Testing
src/main/runtime/orca-runtime-terminal-env-pane-key.test.ts: the restart scenario through the real runtime and status store — inventory with the field, reattach into a new tab, hook post under the old key lands on the new pane; rows filed before the host knew are moved (both orders); exit clears both keys; an old daemon/relay without the field keeps today's behaviour.src/main/agent-hooks/server-terminal-pane-routing.test.ts: HTTP hook ingress routing, row move, stale-row drop, and that cleanup of the dead key never reaches the live pane.src/main/runtime/terminal-env-pane-key-routing.test.ts: every refuse-to-guess case.terminal-host-env-pane-key.test.ts), relay (pty-handler-ownership-attestation.test.ts), listing admission and the cross-version suite.worktree-agent-activation-gate.test.ts: recorded tab recreated under its id; a closed tab still mints.tscnode and web projects clean; changed-lines quality gate and React Doctor clean.Real-restart QA (macOS, local isolated rig, QA driver Codex
gpt-6.1-solhigh for the BEFORE arm and the repeat runs; Grok was out of balance (HTTP 402). Two AFTER runs used Codexgpt-5.6-terramedium, because the driver order in the global instructions read that way during the session):last-status.jsonlast-status.jsonhas neitherNot covered by the rig:
orca worktree psand mobile were not exercised directly. They read the same store snapshot the rig checked, and the runtime tests cover the routing. SSH relay posts are covered by unit tests, not a live remote. Two early AFTER runs dropped the very first hook post in a brand-new profile. A targeted repeat (3 fresh profiles, posting after the terminal settled with no dialog open) produced the row every time, so I attribute it to posting during onboarding and terminal creation, not to this change. Real~/.claudeand~/.codexconfig hashes were identical before and after every run. The live AFTER runs used commit 4adee50 (first review round). The second round changed only the store-side move and exit-cleanup rules, and those are covered by the unit and runtime tests above, not by a new live run.Review
/code-review(high) found 10 issues. Fixed:hasClosedTerminalTabRecord);transferPaneAuthoritydoes (it now does);A second
/code-review(high) pass on the fixes found 10 more issues. Fixed:Kept from pass 2:
PR review bots (Greptile, CodeRabbit, pullfrog) raised 6 more issues. All are fixed, with tests:
wsl:<distro>) are treated as the local host they relay for, so routing works for WSL terminals;Kept from pass 1, with reasons:
Agent skill upstream boundary
docs/reference/agent-skill-sharing-upstream-boundary.mdand copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.Notes
Security: the exported key comes from the execution host's own spawn record, not from a renderer claim; listing admission bounds it like the terminal handle. Cross-platform: no path or shortcut changes. SSH: covered above. Mobile: no client change; the row lands on the live pane. Performance: resolution scans the runtime's PTY records only for posts and on layout changes, and is a no-op when no terminal has moved.
Author
Checklist
N/Awith reasonpnpm lint,pnpm typecheck,pnpm test, andpnpm buildpass (or CI will cover; local preferred)