Skip to content

fix(agents): keep terminal resume identity with its provider owner - #24297

Closed
brennanb2025 wants to merge 17 commits into
mainfrom
brennanb2025/codex-worker-resume-identity
Closed

brennanb2025 wants to merge 17 commits into
mainfrom
brennanb2025/codex-worker-resume-identity

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 19 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2409 $\color{#cf222e}{\Huge{\mathbf{−}}}$​12 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​2397
Prod 22 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​389 $\color{#cf222e}{\Huge{\mathbf{−}}}$​102 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​287

What this fixes

When Codex runs inside a working Claude terminal pane, Orca can save Claude as the displayed agent alongside Codex’s conversation id. Restarting Orca or waking that workspace then runs claude --resume <codex id> and fails with “No conversation found.” This affects terminal session restore on local and remote workspaces across macOS, Linux and Windows. Native structured chat and the separate Codex account-switch restart bug are not changed. Related: #24103 / STA-9012; the reported separate-tab sequence remains unproven, so this PR does not claim to close the whole ticket.

How a user runs into this

  1. You have Claude running in an Orca terminal tab.
  2. Claude runs Codex as part of its own work, inside that same tab. For example, you ask Claude to get a second opinion from Codex and it runs codex exec "..." or codex review in its shell tool, or a skill or plugin Claude uses calls Codex for it. That nested Codex inherits the tab's Orca identity, so its status updates report into Claude's tab.
  3. While that nested Codex is running, Orca's saved record for the tab says "Claude" but holds Codex's conversation id.
  4. If Orca quits, restarts (an update, a crash) or you put the workspace to sleep inside that window, the tab later comes back by typing claude --resume <codex id>. Claude answers "No conversation found", and your actual Claude conversation is not reopened.

The window usually closes by itself the next time Claude reports its own status (for example when its turn finishes), so you hit this only if the quit, restart or sleep lands while the nested Codex is still part of Claude's current turn. Live QA reproduced it this way on main: quit mid-nest, and sleep mid-nest.

Not affected: a Codex worker that runs in its own tab (for example one an orchestrator starts), and Claude or Codex running on their own. Each of those has its own tab identity and always resumes its own conversation.

After this PR: a nested Codex can no longer replace Claude's saved conversation, so the tab comes back as Claude with Claude's own conversation. A tab that was already saved in the mixed state by an older Orca reopens the Codex conversation with codex resume <id> instead of failing.

ELI5

A session saved under the wrong agent label now reopens in the agent that owns it. A Codex conversation opens with Codex, even if the pane was labelled Claude, and it opens once: if that Codex conversation is already open, already queued to reopen, or saved twice under both labels, Orca does not start a second copy of it. This includes sessions saved by earlier Orca versions before the owner label existed: Orca recognises the owner from the conversation file the agent itself reported, so a Claude pane that held a nested Codex conversation reopens it with codex resume <id> after upgrading. New nested-agent events also preserve the parent’s whole conversation record so they cannot create that mixed pair again.

What Changed

Before, the hook writer could keep the parent’s displayed agent while replacing its conversation with the child’s. Resume then trusted the display label. Now the session carries an optional owner label stamped from the authenticated hook route. Nested events preserve the existing owner’s whole record when the previous row still represents that owner; retained exit remnants do not count as an owner.

Cold restore, sleeping wake, the shared command builder, remote resume requests and runtime claim identity use one resolver. It decides the owner in this order: the session’s owner label; for an unlabelled session, the agent whose known conversation-file layout the session’s own saved transcript path matches; otherwise the displayed agent, as before. When the owner differs from the display label, captured commands, arguments and environment from the other agent are discarded. Local launches use the owner’s current command override, default arguments and environment; remote host-authority requests send the resolved agent and omit the other agent’s overrides so the execution host uses its current settings. Matching and unlabelled records keep their existing settings behavior.

The duplicate checks that stop a wake from reopening a conversation twice follow the same owner. Before this, the wake launched the owner but its checks still compared the display label, so a Claude-labelled record holding a Codex conversation could start a second codex resume <id> for a conversation already queued, already running in a Codex pane, or also saved in a Codex-labelled record; two Codex processes would then write to one transcript. Now one helper compares the owning agent on both sides for the record’s duplicate key, the live-pane check, the queued-launch check, the automatic-resume claim, and the cleanup of alias records after an in-place restore. Matching and unlabelled records compare exactly as before.

The transcript path is an intended second evidence source, for records saved before the label existed. Those records cannot gain a label: the saved hook row of a pane that quit mid-nest is removed at startup before any workspace opens, and a slept workspace keeps only its sleeping record. But they do keep the path the agent’s own hook reported, and that path names its owner. A path counts only when it is a Codex rollout (sessions/YYYY/MM/DD/rollout-<time>-<id>.jsonl) or a Claude transcript directly under Claude’s config folder (.claude/projects/<folder>/<id>.jsonl), and the file name ends in the session’s own id. A path in neither shape, in both, named for another id, or a Claude subagent transcript is ignored, and the displayed agent is used. Claude-derived CLIs that write the same projects/ shape under their own folder (Qoder, CodeBuddy) do not match, so their sessions keep their own agent. The check is string parsing only, with no file access, and reads Windows, WSL and SSH paths by shape; nothing is written back to the record. The Codex layout checks main already used for account-home provenance now live in one shared file, used by both sides.

Hook rows saved by main can recover their owner from the saved hook route during hydration. Sleeping records retain an owner label if one was captured. Remote requests still project the session to the locator fields accepted by older strict decoders; no new RPC parameter or stream opcode is required. Host scoping remains unchanged for SSH, WSL, paired runtimes and folder workspaces.

There is no ownership-mismatch refusal path, banner or toast. Saved records are consumed through the ordinary launch path. The writer fix remains intact, and no new retry flag or cleanup obligation is introduced.

Why

The authenticated route identifies which agent owns the conversation more reliably than the pane label. Refusing a known Codex session because the pane says Claude blocks a valid resume. Guessing from the id’s shape is unnecessary, and reusing Claude’s captured command or settings would recreate the failure even after selecting Codex.

The owner label contains only agent identity. Launch settings stay with their existing launch records and current settings; host identity stays with existing execution-host scoping. Relative to the common pattern, reading an unlabelled record’s owner from its transcript path is an intended difference: the path is the agent’s own report of where it writes that conversation, so it is owner evidence the record already holds, not a guess from the id. Keeping the displayed-agent fallback when the path is missing or ambiguous is an intended compatibility choice that preserves existing behavior. This does not claim to replace Orca’s pane-ownership or process-lifecycle model.

Known weaknesses

A record saved before owner labels existed falls back to its display label only when its saved transcript path gives no evidence: no path at all, a Claude config folder moved away from .claude (for example by CLAUDE_CONFIG_DIR, which Orca’s managed Claude accounts use), a Claude version that names its transcript file differently from the session id, or an agent other than Codex and Claude. This change cannot reconstruct a missing owner. A hook row with a saved route can be corrected during hydration.

The existing displayed-owner rules can still select an unintended conversation after an SSH reconnect into cleared rows or an agent crash without exit evidence. The writer preserves the owner those rules identify; it does not redesign those rules. In those cases the conversation that reopens is a real session of the labelled agent, and it now opens without an error message, where main typed a command that failed visibly. Identity-only events continue to use their own incoming agent rather than introducing a new parent-inheritance decision.

Older peers that drop the optional owner label retain legacy behavior. The client still sends only the bare locator with the resolved agent, so older hosts keep working. A newer host publishes the owner label to paired clients, and an older paired client sends it back when it wakes a sleeping session; the host's resume request now accepts that optional label and resolves the owner itself (its own default settings when the owner differs from the agent the client named), so the wake works instead of being rejected as an unknown field. The separate paired-web creation path identified in review remains outside this patch; its current AI Vault caller supplies an unlabelled locator.

A row saved by an Orca build older than mid-September 2026, before Orca began rejecting status events that Grok or OpenCode sent through another agent’s hook route, and still loaded within its 7-day retention after upgrading, can carry the label of a different agent’s process. That row now resumes the labelled agent once and fails visibly, where the old build resumed it correctly. Rows saved by current builds are not affected, and such a row cannot be told apart from a genuine mixed row, so no code guard is added for it. Similarly, while a pre-upgrade unlabelled record with no usable transcript path and an upgraded labelled row for the same conversation sit in different panes, the duplicate check treats them as different owners, so one extra tab may open with a resume command that fails visibly; no conversation is ever opened twice.

Linked Issue

Related to #24103 (STA-9012). Fixes the proven nested-pane mixing and best-effort recovery of labelled mixed rows, without claiming the unproven separate-tab/account-switch sequence is resolved.

Visual Proof

Earlier revision evidence already attached to this PR shows the nested-writer fix. These images are retained as evidence of that earlier revision, not as live validation of the new best-effort recovery behavior.

Before: origin/main restores the Claude pane using the Codex conversation id.

origin/main mixed conversation resume

After: the earlier PR revision preserves Claude’s own conversation through the nested event.

earlier PR revision preserves parent conversation

Live upgrade check of the current head (b204b58), on a remote Mac with stub agent CLIs and isolated homes. Sessions were saved by the main build f69052e, then the PR build opened the same profile. The main-saved mixed records hold a Claude label, a nested Codex conversation and its Codex rollout path, with no owner label.

Before (main build, same kind of saved session): the quit Claude pane reopens with claude --resume <codex id>.

main reopens a quit mixed pane with claude

After, quit mid-nest then relaunch: the PR reopens it with codex --qa-codex-r2-current resume f05e4e3a-… using Codex's current settings.

PR reopens the main-saved quit pane with codex resume

After, worktree slept on main then woken on the PR: codex --qa-codex-r2-current resume 06ecebc0-….

PR wakes the main-saved sleeping pane with codex resume

Unchanged: ordinary Claude and Codex sessions saved by main reopen with their own agent, id and saved arguments, after quit and after sleep.

legacy sessions after quit resume as before

legacy sessions after sleep resume as before

No refusal, error banner or toast appeared in any flow. A Codex nested in a Claude pane on the PR build still reopens with Claude's own conversation id (checked live on b44eb4c).

Testing

  • Latest revision manually tested in the app; app launches are prohibited in this lane.
  • Automated tests added/updated: 729 tests across 79 named files pass on macOS, with real-CLI, real-binary and integration suites excluded. Of the 13 cross-version wire suites, run one at a time, 12 pass; the release-checkout suite timed out unpacking an old release at a machine load average above 60, and it passed in the previous run of the same suites.
  • Records saved by main: records built by the unchanged quit and workspace-sleep capture code (a Claude pane holding a nested Codex session with its rollout path) reopen with codex … resume <id> and Codex’s current settings through cold restore and sleeping wake; the remote request sends Codex; and the host resolves Codex from the same path and gives the same session claim as a Codex request. Reverse nesting (a Codex pane holding a Claude transcript) reopens in Claude. A record with no path, a path named for another id, or an owner label keeps its existing behavior. Windows, WSL and SSH path forms and the Qoder, CodeBuddy, Muse, Pi and OMP layouts are covered.
  • Transcript-path ablation: temporarily removing the path step from the resolver makes the six path-based regressions fail (resolver, cold restore, both wakes, remote request, host); restoring it passes. The no-path, other-id and label-wins controls pass either way. Main’s Codex provenance and rollout-proof tests pass unchanged against the shared layout checks.
  • Ablation: temporarily returning the display agent from the resolver makes all three mixed-row regressions fail (cold restore, sleeping wake, remote request); restoring it passes. Each failure shows the wrong agent’s command or request replacing the expected Codex resume.
  • Older-paired-client wake: a request built the way the previous client builds it, carrying the host's owner label, was rejected by the host before this change ("Unrecognized key: resumeIdentity"); it now resumes the owner, and gives the same session claim as a current client's request. Removing the schema field or the host-side owner resolution makes it fail again.
  • Duplicate-wake ablation: temporarily comparing display labels again in the shared duplicate helper and record key makes the new regressions fail (a replayed mixed record after its resume was queued, the same Codex conversation already live in a Codex pane, a mixed record plus a Codex-labelled record for one conversation, and alias cleanup after an in-place restore), each launching or keeping a second Codex resume; restoring it passes. A control for matching and unlabelled records passes either way.
  • Matching and unlabelled sessions, captured/current launch settings, authenticated route hydration, nested-owner preservation, host scoping, WSL/paired connections and Windows shell quoting covered by named tests.
  • Node and web TypeScript projects pass through the machine-wide gate. Direct changed-file oxlint and the changed-code-quality gate pass.

No production build, native Windows/Linux run, live SSH session, mobile UI, live paired-server resume or live reverse nesting (Claude inside a Codex pane) is claimed; those are covered by unit tests only. Full scripts and broader checks remain for CI.

AI Disclosure

Review

The writer keeps the previous owner’s whole session only for a nested event with a matching, non-retained owner row. Resume resolution then prefers the session’s authenticated-route identity, then the layout of its own saved transcript path, over the display label, and the wake duplicate checks compare that same owner on both sides. Captured launch settings must not cross that agent change. The remote decoder continues to receive a bare locator paired with the resolved agent.

Agent skill upstream boundary

  • Not applicable; no upstream skill source or artifacts copied.

Notes

No new persistent launch-setting copy, ownership cache, retry latch, process probe or file read; the transcript-path owner is derived on each resume from the record as saved. The added owner label lives and dies with the existing session record. Git and folder workspaces use the same resume path. Existing host-boundary checks and platform-aware shell quoting remain in force.

Checklist

  • This PR is focused on session ownership and resume.
  • User-visible before/after, mechanism and alternatives explained.
  • Live before/after proof for the latest change (upgrade from main, quit and sleep), shown above.
  • Self-reviewed for correctness, security and performance.
  • Cross-platform, SSH/remote, mixed-version and folder workspace impact considered.
  • Full lint, typecheck, test and build scripts; named tests, direct lint and both gated project typechecks passed locally, with broader checks left to CI.

Records saved before resume identity existed carry no identity; they now
resume exactly as before instead of being refused. Only an unreadable or
mismatched identity refuses, through one shared predicate.

Resume falls back to the record's saved settings and then the current
command override, default args and env, as before. A replayed hook row
re-captures the settings the session started with, so restart no longer
drops a custom command, flags or env. Remote resume omits agentArgs when
nothing was captured so the host keeps its defaults.
The identity stored with a provider session is now just the agent that
owns it. Launch settings and the connection id are gone from it:

- Launch settings already live in the launch registry and the sleeping
  record. A third copy inside the identity was dropped on every hook
  replay. Every resume path is back to main's launch-settings code.
- The connection id was compared with a copy of itself, and it differs
  by design for WSL relays and paired-runtime mirrors, so those sessions
  were refused. Host scope stays with the existing execution-host check.
- A malformed identity reads as absent and is re-derived from the hook
  route, instead of being stored as a permanent refusal.

A nested event now borrows the previous row's session only when that
row is the displayed agent. A first subagent event, an SSH replay into
cleared rows, or a Codex child after a finished Claude owner keeps its
own session instead of losing it or pairing it with the wrong agent.

Cold restore again builds the resume for a pane with a saved session
even when the pane has an explicit startup command, as main does.
The identity type and its reader now live with AgentProviderSessionMetadata
in agent-session-resume, so agent-resume-identity depends on it one way
only. inheritAgentResumeIdentity accepts the payload's optional agent
type, and never borrows a previous row when no agent is resolved. Test
fixtures use a typed createTab mock and default settings.
Automatic worktree wake refused a sleeping record whose saved owner is a
different agent, but kept it, so every activation showed the same error
again. The refused record is now consumed the way a launched record is,
and its message says the saved resume entry was removed while the
agent's own history is untouched. A record whose pane is still preserved
is left to that pane's cold restore, which keeps it and still reports
"The saved session is preserved".
The cold-restore startup builder also runs speculatively before every
reattach, so a pane whose agent was still running and reattached live
showed "Cannot resume this session" even though nothing was resumed.
The builder is silent again, as on main. The refusal is reported once,
where a cold restore is actually applied, by re-running the same
resolution.

The renderer test now drives a real cold restore: nothing is typed, the
refusal is shown once, the saved session is kept, and the same pane can
then start a fresh agent. A live reattach of the same mismatched pane
reports nothing.
The resume owner rule borrowed a previous row's provider session too
readily:

- A Claude that exited mid-turn leaves a retained identity-only row.
  Codex started in the same pane then stored Claude's ended session, so
  wake would silently resume Claude for the whole Codex run. A retained
  row is never an owner now, the same predicate the presence rule uses.
- A terminal update naming no agent re-attached a finished session that
  terminal ingest had deliberately withheld. Only an event naming another
  agent, or a subagent event, defers to the previous row.
- A borrowed session saved before identity existed kept the child's
  route source on the row, so after a restart it was derived as the
  child's and refused. It now records its resumable owner when borrowed.
The refused-pane test called the shared fixture's untyped sendInput
mock directly, which the web typecheck rejects and which only proved the
mock returns true. The same-pane fresh agent launch that follows is the
real evidence that the pane can start fresh.
After a finished Claude turn, a terminal update starts the next turn
without the finished session. A Claude subagent event then borrowed that
empty row and stored no session, where main kept the Claude session. An
owner row with no session now keeps the event's own session when it
belongs to the displayed agent; a nested agent's session is still not
stored under it.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status (head 4f5fab9, still draft)

The problem. When Codex runs inside a working Claude pane, both agents report through the same pane. On origin/main the status writer kept Claude as the pane's agent but saved Codex's session id, so after a restart, sleep or lost terminal Orca typed claude --resume <codex session id> and Claude answered "No conversation found". Live QA reproduced exactly that on origin/main (quit while the nested Codex is still running).

What changes for users. That pane now resumes Claude's own conversation. Sessions saved by older builds resume exactly as before, with their custom command, args and env. If a saved session turns out to belong to a different agent than the one Orca would launch, Orca shows a refusal in the pane and types nothing, and a fresh agent can still be started. Automatic worktree wake shows that refusal once and then drops the entry, instead of repeating it on every activation.

Reported path. The reported separate-tab worker plus account-switch sequence was not reproduced: on main and on this PR the worker gets its own pane and resumes its own Codex session. The Codex account-switch Restart button leaves the worker pane blank (TerminalKilledError) on main and on this PR alike; that is a separate existing bug, not touched here. This PR is therefore "Related to", not "Fixes", #24103.

Fixed during review (new commits on this branch):

  • Blocking: sessions saved by origin/main (a slept workspace, or a pane restored after quit) were refused after the upgrade. Now anything saved without the new owner label resumes exactly like main.
  • Resume had dropped the user's custom agent command, default args and env. Launch settings are back to main's code (the label no longer carries a copy of them).
  • Windows WSL panes and paired-runtime clients were refused because the host and the window name connections differently. The label now holds only the agent; host scoping stays in main's existing check.
  • Four edge cases in the "whose session does this pane keep" rule: a subagent's first event or an SSH reconnect replay dropped the Claude session; an agent that had exited mid-turn was treated as the owner, so starting Codex afterwards silently resumed the old Claude session; a terminal status update naming no agent re-attached a finished session; a borrowed older session could pick up the child's agent on restart.
  • The refusal message no longer appears over a running agent during an ordinary reattach; it appears only when a cold restore actually runs.
  • A refused entry no longer re-appears on every worktree activation.
  • CI: two type errors, an import cycle, and four main tests the PR had broken without running them (expectations now include the new label). Merged current main.

Deferred (known, not blocking):

  • A mixed session saved by main before the upgrade still resumes wrongly once, exactly as main does (seen live).
  • When main shows the wrong agent for a pane (an SSH reconnect replaying a nested event, or Claude crashing without an exit signal before Codex starts), resume follows the displayed agent and resumes that agent's own session.
  • The refusal uses the generic pane error banner, which adds "If this persists, please file an issue" and version lines; it reads like a crash report.
  • Both refusal messages are untranslated English constants.
  • The paired-web resume request type allows the new label but the host's strict schema rejects it; no caller sends it today (only AI Vault, which sends {key, id}), but a future caller could break paired-web resume without a type error.
  • The host-side ownership check cannot be reached over the strict RPC, so enforcement is client-side.

Verified:

  • Named Vitest files: 1,750 files / 18,697 tests passed at 979a8f4; the PR's test files and the core resume suites passed again after the main merge. Every new regression test fails without its fix.
  • Gated typecheck (node and web) clean at 979a8f4 and at 4f5fab9. Direct oxlint, the import-cycle check and the changed-lines gate pass.
  • Live QA on a remote Mac with isolated homes and stub CLIs (screenshots in the description): upgrade from main (quit and sleep paths), nested Codex in a working Claude pane (fixed), separate-tab worker, refusal with nothing typed plus a fresh start in the same pane, and a live reattach with no error.
  • Readiness checklist: pass, no P0/P1.

Not verified: live SSH resume (no target without real keys or sudo); the one-time wake toast live (unit tests only); native Windows/WSL and Linux; the mobile test suite; the last owner-rule commit (979a8f4) live, since QA ran on a7f1525 (unit tests only).

CI on 4f5fab9: all 18 checks that ran passed (including typecheck, static analysis, the five test shards, relay integration and Windows packaging); 16 were skipped by path filters.

The PR stays a draft; nothing was merged or marked ready.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 1, 2026 09:32
@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Refactors agent session identity and resume logic across multiple modules.

The PR appears safe to merge based on the reviewed resume paths.

Findings

  1. P2 Legacy fallback skips ownership check ▶

Summary

The PR keeps terminal resume identity with the provider that owns the conversation and uses that identity for restore, wake, remote launch, and duplicate detection.

  • It stamps owner identity from hook routes and can infer an unlabelled record’s owner from a narrowly matched transcript path.
  • It discards captured settings from a different agent while preserving host-side defaults and legacy request compatibility.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Saved terminal session] --> B{Owner label?}
  B -->|Yes| C[Use labelled owner]
  B -->|No| D{Recognized transcript layout?}
  D -->|Yes| E[Use layout owner]
  D -->|No| F[Use displayed agent]
  C --> G[Build resume and duplicate identity]
  E --> G
  F --> G
Loading

Reviews (2) · Last reviewed commit: "Merge remote-tracking branch 'origin/mai..."

Comment on lines +2281 to +2284
providerSession: providerSessionForResumeRequest(
launchAgentToSend!,
resumeProviderSessionToSend
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Legacy fallback skips ownership check If a remote pane's startup options contain a session labelled for another agent and the host lacks terminal.ensureAgentSession, the transport uses terminal.create instead. That path sends the session without checking its owner, and the legacy decoder strips the label. Validate ownership before choosing either launch path so the fallback cannot bypass the refusal.

@coderabbitai

coderabbitai Bot commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
AGENTS.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 72240aa0-4b3c-4fe1-bfe7-199f27e1b5e5
📥 Commits

Reviewing files that changed from the base of the PR and between 4f5fab9 and f9062da.

📒 Files selected for processing (26)
  • src/main/agent-hooks/resume-identity-owner-rule.test.ts
  • src/main/agent-hooks/resume-identity-writer.test.ts
  • src/main/codex/codex-pinned-rollout-proof.ts
  • src/main/codex/codex-session-resume-home.test.ts
  • src/main/codex/codex-session-resume-home.ts
  • src/main/runtime/agent-session-claim-identity.test.ts
  • src/main/runtime/agent-session-claim-identity.ts
  • src/main/runtime/orca-runtime-agent-session-resume-identity.test.ts
  • src/main/runtime/orca-runtime-get-agent-session-execution-namespace.ts
  • src/renderer/src/components/terminal-pane/pty-connection-resume-identity.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection/cold-restore-resume-startup.ts
  • src/renderer/src/components/terminal-pane/pty-connection/sleeping-record-access.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-host-session-launch.test.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts
  • src/renderer/src/lib/resume-sleeping-agent-session-identity-mismatch.test.ts
  • src/renderer/src/lib/resume-sleeping-agent-session.ts
  • src/renderer/src/lib/sleeping-agent-pane-ownership.ts
  • src/renderer/src/lib/sleeping-agent-session-launch-windows-quoting.test.ts
  • src/renderer/src/lib/sleeping-agent-session-launch.ts
  • src/renderer/src/store/slices/agent-resume-launch-settings-upgrade.test.ts
  • src/shared/agent-resume-identity.test.ts
  • src/shared/agent-resume-identity.ts
  • src/shared/agent-transcript-layout.test.ts
  • src/shared/agent-transcript-layout.ts
  • src/shared/rpc-contract/agent-session-params.ts
  • src/shared/tui-agent-resume-startup.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Provider sessions now carry validated agent ownership metadata. Hook processing and persistence preserve or restore that identity across nested events and restarts. Runtime and renderer resume paths resolve ownership from identity or transcript layout, then select the corresponding agent and launch settings. Sleeping-session matching and recovery compare resolved resume targets. The changes also add tests for provider attribution, mixed-agent sessions, remote connections, legacy records, and resume behavior.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to f9062

This change makes terminal resume use the agent that owns the saved conversation rather than the displayed agent. The supplied context shows no remaining merge-blocking risk, and the added tests cover the mixed-agent, legacy, and remote resume cases.

Security Architecture Review

Security architecture risk: 🔵 Low · up to f9062

The change aligns restored sessions with their owning provider and avoids reusing another provider’s launch settings. No newly enabled attack path was established, but interrupted recovery and mixed-version behavior are not fully verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected sensitive outcome is terminal execution in the selected workspace using the execution host’s principal and provider-specific command and environment settings. An incorrect owner can therefore affect launch policy and transcript ownership, not merely the displayed label.

Security Findings and Attack Paths

  • inferred — An explicit RPC caller can supply owner metadata, which the resolver treats as routing input rather than verified provenance. However, the base already accepted a caller-selected resumable agent and launch arguments. Forging this field alone has not been shown to confer additional launch authority; finer-grained authorization and downstream ownership verification remain incompletely covered.

Trust Boundaries and Controls

  • observed — Hook normalization extracts provider-session fields using the supplied route source rather than echoing a body-supplied providerSession. The inspected regression test asserts that forged body source and owner metadata do not override the authenticated route’s identity.
  • observed — Legacy transcript inference requires a recognized layout tied to the session ID. Codex inference requires an exact thread match; ambiguous or unrecognized layouts fall back rather than establish ownership. This is compatibility evidence, not an authentication mechanism.
  • observed — The inspected runtime rejects automatic requests based on legacy renderer sleep records. SSH workspaces without execution-namespace support require the legacy path before local identity canonicalization or terminal creation; explicit claims retain the resolved workspace ID.

Resilience and Maintainability Implications

  • observed — Sleeping recovery synchronously creates a tab carrying pending startup and an ownership claim before clearing its sleeping record. That ordering predates this PR. The inspected evidence does not establish durable atomicity across a process interruption between those steps.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.15% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 61 functions across 42 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: preserving terminal resume identity with the provider session owner.
Description check ✅ Passed The description is complete and focused. It explains the user-facing issue, the change and rationale, linked issue context, visual proof, testing, review notes, and known limitations. Some checklist i…
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.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes — the provider-ownership label on saved terminal resume records, across the hook writer, hydration, resume choke points, and the sleeping/cold-restore/remote paths.

  • Ownership on the locator. AgentProviderSessionMetadata.resumeIdentity: { agent } is stamped from the authenticated hook route in decodeHookResumeSession, never from the hook body. Malformed values read as absent; pre-upgrade rows without a route stay unlabeled and resume as on main.
  • One owner rule in the writer. inheritAgentResumeIdentity keeps the live owner's whole session for nested events (subagent toolAgentId, or a different known agent), never borrows a providerSessionOnly remnant, and withholds an ownerless session unless the event's own locator names the displayed agent.
  • One resume rule. agentResumeIdentityPermits refuses only a mismatching identity (undefined permits); enforced at buildAgentResumeStartupPlan, cold restore, sleeping wake, the host claim check, and the remote projection.
  • Refusal behaviour. Cold restore types nothing, shows the error once and keeps the record; automatic wake shows one toast and consumes the record like a launch.
  • Hydration and wire. The sleeping-session schema keeps the field, recovery-equivalence compares identity, and remote resume projects a bare locator for the strict terminal.ensureAgentSession decoder.

I traced the writer rule against main across single-agent, subagent, remnant, withheld-session, legacy-restart, and relay-hydration scenarios, and ran two independent passes (writer-regression and remote-refusal lenses). Both refuted the central risk: identity is route-derived and preserved verbatim on the relay/WSL/paired paths, and no legitimate single-agent pane loses or mislabels a session relative to main. The pi↔omp mirror normalization was the only edge worth naming — it rewrites the displayed agent across the compatible group, so a pi-launched pane reporting an omp session now refuses rather than attempting a cross-harness resume; that is the intended refusal, not a regression. The source === 'omp' && connectionId !== null legacy guard is permissive (it can only withhold a label, never refuse) and has no dedicated test.

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Sleeping wake launches the agent named by the session's resume identity,
but its duplicate checks still compared the record's display agent. A
mixed record (shown as Claude, session owned by Codex) could launch a
second `codex resume` for a session already queued, claimed, or live in a
Codex pane, forking the transcript.

Claim keys, the live/queued/claimed checks, and the cold-restore alias
cleanup now compare the owning agent on both sides through one helper.
Hosts now publish the session's owner label in session.tabs, and a paired
client older than the label copies it unchanged into its sleeping-wake
request. The strict provider-session schema rejected that request with an
unrecognized key, so the wake failed for every labelled session.

The schema now accepts the optional label, and the host resolves the
owning agent from it: the claim, namespace, enable check, launch agent
and settings all follow the owner. Arguments, launch preferences and the
OMP path sent for a different display agent are dropped in favour of the
host's defaults for the owner, matching what current clients send.
Sessions saved before the owner label existed still resumed with the
pane's displayed agent: a Claude pane that held a nested Codex session
typed `claude --resume <codex id>` after quit or sleep, because the only
saved hook row is removed at startup and the sleeping record has no
label.

Those records still carry the transcript path the hook reported. The
resolver now reads the owner from the label first, then from that path
when it is in Codex's dated rollout layout or under Claude's
.claude/projects tree and the file is named by the session's own id,
and only then falls back to the displayed agent. Every resume, wake,
remote request, host claim and duplicate check shares that resolver.
The Codex layout readers move to src/shared so main and renderer use
one copy.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status (draft kept; not merged, not marked ready)

The problem: a Claude pane that had a nested Codex could save Claude as its agent but the Codex conversation id, so after a restart or wake Orca typed claude --resume <codex id>, which fails with "No conversation found" (#24103 / STA-9012). Sessions saved by the current main build in that state also still failed after upgrading to earlier revisions of this PR.

What changes for users: a session saved under the wrong agent label now reopens in the agent that owns it, including sessions saved by earlier Orca versions. A Claude pane that held a nested Codex conversation reopens with codex resume <id> and Codex's current settings, after a quit or a worktree sleep. It opens once, never as two copies. Ordinary sessions resume exactly as before, and nested agents no longer overwrite the parent's saved conversation.

Fixed during this review (all pushed; head b204b58):

  • aaad3c4: waking a mislabelled sleeping session could open a second codex resume <same id> tab, because the duplicate checks still compared the displayed agent. They now compare the owning agent on both sides.
  • b44eb4c: an older paired desktop client sent the new owner label back to a newer host, and the host rejected every sleeping-session wake ("Unrecognized key"). The host now accepts the optional label and resolves the owner itself, with the same session claim as a current client.
  • 32ca75c: sessions saved by main never got an owner label, because the startup cleanup removes the mixed status row and a slept worktree keeps only its sleeping record. For a record with no label, the owner now comes from the conversation file path the agent itself reported. A Codex rollout file named for the id means Codex; a .claude/projects/…/<id>.jsonl file means Claude. Otherwise the displayed agent is used, as before. This is string parsing only, with no file access, and covers Windows, WSL and SSH path forms.
  • Merged main twice (e632ef8, b204b58). The second merge brought in main's own fix for an unrelated lint failure.

Deferred (minor):

  • A row saved by a build older than mid-September 2026, whose label came from another agent's process, resumes that agent once and fails visibly.
  • During the upgrade window, an unlabelled record with no usable path and a labelled one for the same conversation can open one extra tab that fails visibly.
  • When the displayed agent itself is wrong (SSH replay, a crash without exit evidence), Orca now quietly reopens a real session of the labelled agent instead of failing visibly.
  • A Claude session kept under a moved CLAUDE_CONFIG_DIR, nested inside a Codex pane, still falls back to the displayed agent, as before.
  • Small cleanups: an unreachable branch in the resume command builder, a duplicate check in cold restore, a duplicated code comment, and the disabled-owner message not naming the owner.

Verified:

  • Node, web and CLI typechecks pass, as do full-repo lint and the changed-code-quality gate.
  • 735 targeted tests pass. Each fix has a recorded failing run without its change.
  • CI is green on b204b58.
  • Each fix had an independent review. A final readiness review of the full diff found no blocking issues.
  • Live on a remote Mac with stub agents and isolated homes, upgrading main f69052e to b204b58: a main-saved mixed session reopened as codex … resume <id> after both quit and sleep. Ordinary sessions resumed as before. No refusal or error appeared. Screenshots are in the PR description.
  • On b44eb4c, a Codex nested in a Claude pane reopened with Claude's own id.

Not verified live:

  • Paired-server resume, including an older client against a newer host. This is covered by tests that send the exact request shape an older client builds.
  • Claude nested inside a Codex pane. This is covered by tests.
  • Real (non-stub) agent CLIs, native Windows or Linux, SSH, and mobile.

@brennanb2025
brennanb2025 marked this pull request as ready for review October 2, 2026 04:34

@pullfrog pullfrog 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.

✅ No new issues found.

Reviewed changes

  • Owner label on saved sessions — AgentProviderSessionMetadata gains an optional resumeIdentity: { agent } stamped from the authenticated hook route, so a record remembers which CLI owns its locator instead of trusting the pane's display label.
  • Nested events inherit the owner's whole record — inheritAgentResumeIdentity makes a nested event (subagent toolAgentId or a different known agent) borrow the live owner row's full session rather than the child's locator, while providerSessionOnly remnants never own.
  • One resume resolver everywhere — resolveResumeAgent ranks owner label > transcript-path layout (Codex rollout / Claude project transcript) > display label and drives cold restore, sleeping wake, the shared command builder, host-authority resume, runtime claim identity, and remote projection.
  • Settings follow the owner — when the owner differs from the display label, the other agent's captured command/args/env and launch preferences are discarded; local launches use the owner's current settings and remote requests send the resolved agent with a bare locator.
  • Duplicate-wake checks compare the owner — sameResumeTarget and getProviderSessionClaimKey key on the resolved agent, so a mixed record and its owner's record collide instead of launching twice.
  • Cross-version wire — ProviderSession keeps .strict() but accepts the optional label, the client strips it before terminal.ensureAgentSession, and terminal.create remains a plain object so older hosts ignore the new field.

The new tests pin behavior rather than restating it: they assert exact resume commands, cover the main-era unlabelled-record recovery for both nesting directions, and include ablation controls for the transcript-path and duplicate-wake rules. Two independent passes over the transcript-path classifier and the duplicate-target comparison found no reachable false positive or order-induced fork; the only gaps are the false negatives the PR discloses.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

…s Claude's

Claude reports a managed account's transcript at the unresolved
<Orca data>/claude-profiles/<id>/home/projects/<cwd>/<session>.jsonl, so the
.claude-anchored rule read it as no evidence. Accept that config dir too,
still anchored so Claude-derived CLIs' relocated projects/ trees don't match.

@pullfrog pullfrog 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.

ℹ️ No critical issues — one scope question inline.

Reviewed changes — the incremental delta since the prior pullfrog review (b204b586d9), a single commit that widens Claude attribution for unlabelled resume records.

  • Managed-account transcript recognition. claimsClaudeProjectTranscript now also accepts <...>/claude-profiles/<accountId>/home/projects/<encoded cwd>/<session id>.jsonl, in addition to <...>/.claude/projects/....
  • Parser tests. Added macOS, Windows, and WSL managed-account path cases plus negative controls (subagent transcript, empty account id, a home under a non-claude-profiles root).

The path parsing itself is sound, cannot collide with the Qoder, CodeBuddy, OMP, Muse, or Pi shapes, and the new positive cases would fail against the pre-commit code — the sole open point is whether the newly recognized layout is the one production actually writes today.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

const fromEnd = (offset: number): string => segments[count - offset]?.toLowerCase() ?? ''
const configDir =
fromEnd(4) === '.claude' ||
(fromEnd(4) === 'home' && fromEnd(5) !== '' && fromEnd(6) === 'claude-profiles')

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.

I can't find a producer of the claude-profiles/<id>/home layout on this branch or on origin/main. Main's managed Claude config dir is <userData>/claude-accounts/<accountId>/auth (managed-auth-path.ts, runtime-auth-preparation.ts:30-34), so a managed WSL transcript at .../claude-accounts/<id>/auth/projects/<enc>/<id>.jsonl still falls through to the display agent. This recognized shape only exists on your unmerged claude-profile-* branches (claude-profile-paths.ts sets home = <dataRoot>/claude-profiles/<accountId>/home and CLAUDE_CONFIG_DIR to it). Please confirm whether the recognition is landing ahead of that work or is meant to cover today's managed records — if the latter, the current auth tree needs matching too.

Technical details
# `claude-profiles/<id>/home` layout is not reachable on `main`

## Affected sites
- `src/shared/agent-transcript-layout.ts:58-60` — new `claude-profiles/<id>/home` acceptance branch.
- `src/main/claude-accounts/managed-auth-path.ts:8-10` — managed root is `<userData>/claude-accounts`.
- `src/main/claude-accounts/runtime-auth/runtime-auth-preparation.ts:30-34` — WSL managed `configDir`/`CLAUDE_CONFIG_DIR` is `wslLinuxAuthPath` (`.../claude-accounts/<accountId>/auth`).
- `src/shared/agent-resume-identity.ts:83` — `resolveResumeAgent` only reaches this classifier for unlabelled records.

## Required outcome
- Either the current `<...>/claude-accounts/<accountId>/auth/projects/<enc>/<id>.jsonl` shape is recognized, or the new shape is confirmed to arrive with the profile-routing feature.

## Open questions for the human
- Is `claude-profiles/<id>/home` expected on `main` before the `claude-profile-*` branch merges, or should today's `auth` tree also resolve as Claude?

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Closing this older resume implementation and rebuilding conversation ownership on current main. Wrong-agent resume after nested Codex work remains tracked in #24103; this does not claim every reported sequence was reproduced.

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.

1 participant