You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Keep existing Continue actions reliable after agent exit and failed starts - #36074
Continuing a terminal conversation in Chat UI can lose the conversation after the agent exits. Codex conversations can open with missing earlier messages, and a failed terminal start can leave its process running without a cleanup owner. This fixes those lifecycle problems for the Continue buttons already in Orca.
What Changed
Retain the last conversation for an existing terminal pane, bounded and invalidated when its source changes.
Import the complete saved Codex conversation, including inherited and compressed history, from the resolved account's store.
Keep cleanup ownership of a native process if terminal creation fails after it starts, through retry and disposal, until the terminal’s existing exit signal.
Continue in Chat UI still opens a new chat tab and stops nothing. Continue in terminal agent still creates a fork. The Continue buttons become the same-tab switcher in #31595; this PR does not change what they do.
On Windows, cleanup no longer force-kills a process by its number, which could hit an unrelated program; the terminal's process group handles it.
Why
Conversation identity must outlive live status, saved history can span multiple files, and a started process needs a cleanup owner before its terminal is published.
Account-scoped ownership was considered and dropped: accounts can share one conversation log, so main's refusal is correct. Existing ownership checks and their ordering stay unchanged.
No layout or control is added. On the current head, fresh terminal conversations import their complete marked history after normal exit and retain their source.
Claude immediate (221ms):
Codex immediate (222ms):
Paused Claude and Codex imports also passed. The Codex paused run included an unrelated filesystem filename listing during its fixed 30-second pause; the listing did not read app state or gate the click. That procedural departure is retained in the evidence record.
Testing
Full review path. Fresh whole-head correctness, consumer, reference, performance and readiness source reviews agree with cd7964c. Current-head CI is green, including static checks, typechecks and all 12 unit shards. The latest focused repair batch passed 198 tests with four existing skips across 11 named suites; earlier overlapping batches cover retained conversation identity, Codex ancestry/compression/canonical history, unchanged ownership order and native creation failure/retry/disposal. Tests were pared to real bugs and production boundaries under independent counsel.
Targeted Mac validation passed with real Claude and Codex, each immediate and after a recorded 30-second pause: all four history messages rendered once and in order in a new chat, with the terminal source retained. Every actual request exercised absent source-owner rows plus captured local source provenance. Both app launches stopped gracefully and sampled real config hashes matched. Windows: Codex after-exit Continue passed 1 trial (immediate, ~2 s) with context and fork; 2 further trials not reached (driver stopped before the agent replied); Claude on Windows not exercised (QA machine's Claude launcher broken); Claude and Codex both passed on Mac. The lane owner accepted this targeted coverage and stopped further trials. Stage 2 stopped before exit, with input submission unconfirmed; stage 3 was not started. The recorded 1.969-second Windows interval ends at click invocation, rather than a captured DOM click event. Prior Windows evidence covers graceful upgrade, resumed Codex history/context/fork, native Claude context/fork, ordinary close and command-start failure, with the original input limits retained. Earlier fresh imports failed; only a later captured Claude trial directly established the missing-locator refusal. The initial Claude and Codex rejecting branches remain unproved.
post-spawn UI failure: NOT EXERCISED (not user-reachable; proved by boundary tests)
Review
The split preserves current Continue behavior. It adds no switch RPC, Stop protocol, source retirement, same-tab placement or same-ID terminal reopening. Current main's cases and newer startup/host behavior are preserved. Windows cleanup route assertions are updated under narrow owner exceptions to verify owned cleanup without unverified process-number signals.
Agent skill upstream boundary
Not applicable; no skill-installer source is copied or translated.
Notes
Cross-platform native creation, SSH/old-daemon behavior and mixed client/server versions are in scope for review. The full review, targeted QA and final raw-artifact audit are complete under the coverage and limitations described above.
Checklist
Scoped to the existing Continue actions and ordinary failed-start cleanup
Explained the user-facing change and mechanism
Targeted before/after evidence attached
Final correctness, consumer, reference and performance source review complete
Cross-platform, SSH/remote, mobile and mixed-version impact considered
CI passes on cd7964c; local checks limited to the normal commit hook and focused tests
Known adapter limits: cleanup uses the existing terminal exit signal. On Windows, a relay/socket failure can report exit before the shell completes. A native spawn that creates a child then throws before returning its handle can still escape ownership. These existing boundaries are separate follow-up work; the panel found no demonstrated worse outcome on the same input. This change adds no independent native-exit confirmation. Both limitations are tracked in STA-12894.
Intended continuation contract
After a terminal agent exits, Continue in Chat UI can import its saved conversation when its status locator has disappeared and the host captured a supported local source. Conflicting or unknown source evidence still refuses. The existing unary replay test now uses an unknown-provider source for its invalid-source case; a production import case covers the formerly rejected absent-locator input and verifies early and late history. Its ownership, account-drift, ordering and publication guarantees remain.
The source repair has passed fresh independent review and current-head CI. Targeted Mac and Windows Continue validation is complete under the accepted scope above. The independent audit verified all 47 selected Windows artifacts and supports the successful import/context/fork chain. Current Windows success is supported by recorded rendered UI text and context/fork receipts; no passing Windows screenshot was captured under the accepted no-helper scope. Earlier failures remain part of the QA record; later successes cannot reconstruct their missing captures.
The successful Windows driver initially wrote new automatic UI evidence files into the admitted QA worktree, then reattached from its evidence folder. This procedural departure is retained in the QA record. Natural compressed/history_base runtime formats and supplementary folder-workspace QA were not exercised; fixtures and consumer tests cover those contracts.
Keeps existing Continue actions available after agent exit and retains cleanup ownership after failed terminal starts.
Terminal starts keep a cleanup owner until the process exits or the session takes over.
Chat UI imports earlier turns from the saved Codex conversation.
Continue in Chat UI can still find a pane's last conversation after its agent exits.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Agent reports conversation] --> B[Retain pane identity]
B --> C[Continue in Chat UI]
C --> D[Resolve account history]
D --> E[Read ancestor prefixes and selected file]
E --> F[Import into new chat]
G[Start native process] --> H[Record cleanup owner]
H --> I{Terminal published?}
I -->|Yes| J[Session owns cleanup]
I -->|No| K[Retain cleanup through retry or disposal]
K --> L[Release after physical exit]
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
The daemon changes track PTY creation outcomes and physical ownership through spawn, session publication, and cleanup. Codex transcript import now reads paginated history across rollout files, validates segment boundaries, and carries account-store context through adoption. The renderer stores pane conversation identities separately from agent status, validates them during collection, and uses last-reported identities for resume-source resolution.
Priority: Normal
Estimated code review effort: Complex
Merge Risk: High · up to 48dae
Concurrent Continue actions can terminate an existing agent, while failed starts can leave later attempts waiting. Resolve these issues and confirm legacy history retention before merging.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 29.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Title check
The title clearly identifies the main purpose: improving existing Continue actions after agent exits and failed terminal starts.
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Description check
The description is complete and relevant. It covers the user impact, implementation scope, rationale, linked issue, visual evidence, testing, platform coverage, limitations, review notes, and checklis…
Full details: Description check
Explanation
The description is complete and relevant. It covers the user impact, implementation scope, rationale, linked issue, visual evidence, testing, platform coverage, limitations, review notes, and checklist. The AI Disclosure section is omitted, but the template allows internal contributors to omit it; the Linked Issue section references #31595.
Full details: Docstring Coverage
Explanation
Docstring coverage is 29.79% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 47 functions across 59 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Finishing Touches: 💡 1
📝 Generate docstrings: 💡
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.
85-85: Trivial · Data Integrity & Integration · Heavy lift
Preserve wrapped Codex response blocks during history import.
Wrapped response_item messages still use claudeContentBlocks, so input_text, output_text, and input_image are not decoded. The focused fixtures do not cover this path: paginated response items are filtered out, while the visible assertions use completed items or unwrapped records.
Use a decoder that supports Codex response-item blocks while preserving existing supported blocks, then deduplicate mirrored records separately. This behavior predates the PR.
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Repository UI
Review profile: CHILL
Plan: Advanced
Run ID: cbe159da-a92e-4c0c-9914-e2c671b61a72
Commits
Reviewing files that changed from the base of the PR and between 8d1a2e5 and 0a8a324.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Follow-up Comments
Strip the creator's recovery callback before joining its disposition.
32-40: Major · Stability & Availability · Quick win
Strip the creator's recovery callback before joining its disposition.
A joiner receives the creator's creation disposition. The current join(creation) also inherits its recover callback. For different requested session IDs, adoption attaches to the creator's live session, but the outer cleanup checks the joiner's original ID and finds no local session. Cleanup can then call the creator's recovery callback, and shutdownOwnedTerminalProcess can terminate the live process that the joiner did not start.
Preserve the creator's completion and exit metadata, but remove its recovery authority at this join boundary.
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.
After a terminal agent exits, Continue in Chat UI can lose its conversation. Codex imports can omit earlier messages, and a failed terminal start can lose the process cleanup owner. This PR remembers the pane's last valid conversation, imports complete saved Codex history from the selected account store, and keeps cleanup ownership across ordinary creation failures. Continue in Chat UI opens a new tab and stops nothing; Continue in terminal agent forks and retains its source. The same-tab switcher is separate, tracked in #31595.
Six semantic review rounds are complete. Fresh correctness, consumer, reference, performance and readiness source reviews agree with cd7964c. Current-head CI has 53 successful and 23 skipped checks, including static checks, typechecks and all 12 unit shards. The latest focused repair batch passed 198 tests with 4 existing skips across 11 suites; overlapping earlier batch counts are not additive. Independent test counsel pared additions to real bugs and production boundaries. Source-counted performance bounds do not establish measured runtime latency.
On the exact current head, Mac Claude and Codex each passed fresh ordinary-terminal two-turn imports immediately after normal exit and after a recorded 30-second pause. Measured shell-observation-to-click intervals: Claude 221ms/30.214s; Codex 222ms/30.223s. Each new chat rendered all four marked history messages once/in order, retained its source, and captured zero addressed ownership rows plus exact supported local route. Codex delayed included an unrelated filename listing during its fixed pause: a disclosed procedural departure, no app-state/readiness gate or altered timer. Immediate Codex independently met the no-intervening-wait condition. Build/launch/identity receipts bind the head; they are not independent cryptographic loaded-bundle attestation.
Windows: Codex after-exit Continue passed 1 trial (immediate, ~2 s) with context and fork; 2 further trials not reached (driver stopped before the agent replied); Claude on Windows not exercised (QA machine's Claude launcher broken); Claude and Codex both passed on Mac. The successful Windows import rendered all four original messages once and in order, produced a real prior-context chat reply, then a distinct-provider terminal fork with a real context reply and both sources retained. Its recorded 1.969-second interval is shell observation to click invocation, not a captured DOM event. The second trial stopped before exit with submission unconfirmed; the third was not started. The lane owner accepted the targeted evidence and stopped further trials. An independent audit of all 47 selected Windows artifacts supports this chain. Initial automatic UI evidence files were written into the admitted QA worktree before the driver reattached from the evidence folder; this procedural departure is retained in the record. Existing native-Claude and resumed-Codex context/fork evidence retains its original input limits.
Earlier Windows fresh imports failed. Only a later captured fresh Claude input directly established missing-locator admission; initial Claude/Codex rejection branches remain unproved. The Mac initial absent-menu attempt had an isolated config/hook-target mismatch; later aligned passes do not establish whether that original identity was never reported or cleared, or uniquely reconstruct historical failures. Natural compressed/history_base runtime formats and supplementary folder-workspace QA were not run; retained history fixtures and consumer tests cover those contracts.
post-spawn UI failure: NOT EXERCISED (not user-reachable; proved by boundary tests)
Prior Windows ordinary command-start failure/close evidence and exact known-handle receipts carry. Finite recorded cleanup is not universal native/descendant death proof. Existing Windows exit signalling can precede native completion; a native spawn that creates a child then throws before returning its handle can escape ownership. Both remain tracked in STA-12894; this PR adds no independent native-exit witness. A custom callback throwing without creation disposition stays conservatively unknown; no shipped failing path was demonstrated.
Post-merge Windows verification for #36074 (squash merge 28e4764): the two remaining Codex after-exit imports and all three Claude imports passed. Claude also passed the fresh chat-context and terminal-fork chain. No product failure was observed and no product code was changed.
All five runs used Windows high spec through qa-gate. Codex used main e9be993f476a3131062239b35be40e2f9e0a69e9; Claude used main 561de18ad8a7b42809f52e2f52634e348c19768d. Both contain 28e4764 and were fetched/pinned for their respective runs.
Each used ordinary New tab → agent, two real completed turns, normal /exit, then ordinary Continue in Chat UI in one sequential operation as soon as the selected pane showed its shell prompt. Each opened a distinct, visibly bound new chat with the four original messages exactly once and in order, retaining the source terminal and its history. The copied executed scripts, native transcripts, rendered bindings, bounded logs and original screenshots were independently audited. Timings include ordinary menu overhead and measure observation to locator invocation/completion; physical exit and DOM event dispatch were not measured. The runtime source-admission branch remains unknown.
In Claude A1, the imported chat answered the earlier codeword without receiving it again. Ordinary Continue with Terminal Agent then opened a distinct native conversation (20aa0e7d…, versus source/chat fd823d36…) and answered the earlier number, 86327, without receiving it again. The source terminal and imported chat were both retained. The fork's distinct native ID was proven after its first completed turn, when its transcript materialized. This chain ran once. The original Codex context/fork proof remains historical at cd7964c, including its missing passing-PNG and old helper-identity limitations; these two import repetitions do not relabel it as fresh context/fork evidence.
The original low-spec Claude launch failure was a stale npm shim pointing to a missing executable. Production PATH-merge replay reproduced why a duplicate native-directory prefix can lose precedence; the exact historical reordering stage was not captured. The revised QA scope allowed either admitted Windows machine. These Claude trials verified ordinary app-shell resolution/version and a real New tab reply with native Claude 2.1.312 on high spec. They do not claim app verification of the original low-spec forwarding remedy. No product, typed command, npm installation, real profile or real configuration was changed.
Preserved nonpasses were QA setup failures: missing E2E observer exposure, premature or unmounted-pane observer checks, preflight/capture-script mistakes, one pre-exit interruption after misclassifying permitted OS-assigned listeners, and a setup reply missing its requested period. None is counted as a trial pass or an Orca product failure; held attempts and raw receipts remain retained.
Cleanup receipts: both app copies stopped gracefully; their configured ports are empty. The Codex finite audit covered 465 identity records: 459 absent, six reused records representing five numeric PIDs, zero matching owned identities. The Claude finite audit found all 65 recorded identity records absent (record count, including timestamp-format duplicates). Nine sampled real-config states matched for Codex; all 16 selected before/after config/profile states matched for Claude (the eight overlapping supervisor-baseline records also matched). Source and index remained clean. The leads and transfer controls exited normally before their exact handles were closed, and the Electron, diagnostic and unused queued admissions were released. These finite checks do not establish universal descendant or native-process death.
Receipt limits remain explicit: the first Claude setup capture has a null exit-code receipt with later finite absence, A1's source-native birth tuple was missed, and early lead tool-shell waits/launcher internals lack birth tuples. Harness completion is not independent PID-absence proof. Deferred STA-12894 native-witness limits remain unchanged. post-spawn UI failure: NOT EXERCISED (not user-reachable; proved by boundary tests).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ELI5
Continuing a terminal conversation in Chat UI can lose the conversation after the agent exits. Codex conversations can open with missing earlier messages, and a failed terminal start can leave its process running without a cleanup owner. This fixes those lifecycle problems for the Continue buttons already in Orca.
What Changed
Continue in Chat UI still opens a new chat tab and stops nothing. Continue in terminal agent still creates a fork. The Continue buttons become the same-tab switcher in #31595; this PR does not change what they do.
On Windows, cleanup no longer force-kills a process by its number, which could hit an unrelated program; the terminal's process group handles it.
Why
Conversation identity must outlive live status, saved history can span multiple files, and a started process needs a cleanup owner before its terminal is published.
Account-scoped ownership was considered and dropped: accounts can share one conversation log, so main's refusal is correct. Existing ownership checks and their ordering stay unchanged.
Linked Issue
tracked in #31595
Visual Proof
No layout or control is added. On the current head, fresh terminal conversations import their complete marked history after normal exit and retain their source.
Claude immediate (221ms):
Codex immediate (222ms):
Paused Claude and Codex imports also passed. The Codex paused run included an unrelated filesystem filename listing during its fixed 30-second pause; the listing did not read app state or gate the click. That procedural departure is retained in the evidence record.
Testing
Full review path. Fresh whole-head correctness, consumer, reference, performance and readiness source reviews agree with cd7964c. Current-head CI is green, including static checks, typechecks and all 12 unit shards. The latest focused repair batch passed 198 tests with four existing skips across 11 named suites; earlier overlapping batches cover retained conversation identity, Codex ancestry/compression/canonical history, unchanged ownership order and native creation failure/retry/disposal. Tests were pared to real bugs and production boundaries under independent counsel.
Targeted Mac validation passed with real Claude and Codex, each immediate and after a recorded 30-second pause: all four history messages rendered once and in order in a new chat, with the terminal source retained. Every actual request exercised absent source-owner rows plus captured local source provenance. Both app launches stopped gracefully and sampled real config hashes matched. Windows: Codex after-exit Continue passed 1 trial (immediate, ~2 s) with context and fork; 2 further trials not reached (driver stopped before the agent replied); Claude on Windows not exercised (QA machine's Claude launcher broken); Claude and Codex both passed on Mac. The lane owner accepted this targeted coverage and stopped further trials. Stage 2 stopped before exit, with input submission unconfirmed; stage 3 was not started. The recorded 1.969-second Windows interval ends at click invocation, rather than a captured DOM click event. Prior Windows evidence covers graceful upgrade, resumed Codex history/context/fork, native Claude context/fork, ordinary close and command-start failure, with the original input limits retained. Earlier fresh imports failed; only a later captured Claude trial directly established the missing-locator refusal. The initial Claude and Codex rejecting branches remain unproved.
post-spawn UI failure: NOT EXERCISED (not user-reachable; proved by boundary tests)
Review
The split preserves current Continue behavior. It adds no switch RPC, Stop protocol, source retirement, same-tab placement or same-ID terminal reopening. Current main's cases and newer startup/host behavior are preserved. Windows cleanup route assertions are updated under narrow owner exceptions to verify owned cleanup without unverified process-number signals.
Agent skill upstream boundary
Notes
Cross-platform native creation, SSH/old-daemon behavior and mixed client/server versions are in scope for review. The full review, targeted QA and final raw-artifact audit are complete under the coverage and limitations described above.
Checklist
Known adapter limits: cleanup uses the existing terminal exit signal. On Windows, a relay/socket failure can report exit before the shell completes. A native spawn that creates a child then throws before returning its handle can still escape ownership. These existing boundaries are separate follow-up work; the panel found no demonstrated worse outcome on the same input. This change adds no independent native-exit confirmation. Both limitations are tracked in STA-12894.
Intended continuation contract
After a terminal agent exits, Continue in Chat UI can import its saved conversation when its status locator has disappeared and the host captured a supported local source. Conflicting or unknown source evidence still refuses. The existing unary replay test now uses an unknown-provider source for its invalid-source case; a production import case covers the formerly rejected absent-locator input and verifies early and late history. Its ownership, account-drift, ordering and publication guarantees remain.
The source repair has passed fresh independent review and current-head CI. Targeted Mac and Windows Continue validation is complete under the accepted scope above. The independent audit verified all 47 selected Windows artifacts and supports the successful import/context/fork chain. Current Windows success is supported by recorded rendered UI text and context/fork receipts; no passing Windows screenshot was captured under the accepted no-helper scope. Earlier failures remain part of the QA record; later successes cannot reconstruct their missing captures.
The successful Windows driver initially wrote new automatic UI evidence files into the admitted QA worktree, then reattached from its evidence folder. This procedural departure is retained in the QA record. Natural compressed/history_base runtime formats and supplementary folder-workspace QA were not exercised; fixtures and consumer tests cover those contracts.