Skip to content

Close the terminal before continuing its conversation in chat - #36732

Merged
brennanb2025 merged 21 commits into
mainfrom
brennanb2025/fix-32154-main
Nov 1, 2026
Merged

brennanb2025 merged 21 commits into
mainfrom
brennanb2025/fix-32154-main

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 13 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​690 $\color{#cf222e}{\Huge{\mathbf{−}}}$​1075 $\color{#cf222e}{\Huge{\mathbf{−}}}$​385
Prod 36 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​592 $\color{#cf222e}{\Huge{\mathbf{−}}}$​472 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​120

Problem

Clicking Continue in Chat UI on a terminal whose agent (Claude, Codex and the rest) was still running didn't open a chat. Orca refused with "...already running this conversation in another terminal... open this conversation from Session History", even though the only thing running it was the terminal the user clicked. On Windows (the report, tracked in #32154) it refused even after the agent had quit. The reason was that Orca decided whether the terminal had let go of the conversation by inspecting the terminal's processes, which is often wrong and impossible on Windows. It also never made the terminal let go.

What this changes for users

  • With the agent still running in the clicked terminal, Continue in Chat UI closes that terminal tab (the whole tab, including split panes) and then opens the same conversation in a new chat tab. There is no extra confirmation for the agent itself. If another split pane in the tab is running something else (a dev server, for example), Orca's existing "close terminal with running process" confirmation appears, as for any close. Pinned tabs still get the existing pinned-tab confirmation. Cancel leaves everything as it was.
  • The chat opens only after Orca has confirmed that every process in that tab has exited, locally and on paired hosts. If that can't be confirmed, no chat opens, and one message says so and points to Session History.
  • If the agent in the clicked terminal has already quit, the terminal stays open and the chat opens, as before.
  • If a chat already has this conversation open, Orca switches to that chat and leaves the terminal alone, as before.
  • Failure messages name the agent and say what to do. If the chat can't open after the terminal closed, the message says why and how to get back to the conversation.

How it works

  1. Before closing anything, Orca asks the host whether it can open a chat for this agent and checks chat permissions, using the existing checks.
  2. Orca asks the host for a handle to each terminal in the tab, starts an exit wait on every handle, and then closes the tab through the ordinary tab close.
  3. Each wait must report a real exit. On paired hosts the host keeps answering an exit wait for a terminal whose pane was just removed, from its own record of that terminal, until that record is pruned. A wait that arrives after the close still gets the real answer.
  4. The chat is then created by conversation id through the existing chat creation, in the same workspace, which stays selected.

Hosts that support this advertise a new capability. Older paired hosts keep today's behavior (they don't close the terminal), with clearer wording. SSH is unchanged. No saved data, wire fields or RPC methods are added.

Intended behavior differences

  • The status store alone decides whether another terminal holds the conversation. The host's hook status store blocks only when it has a live process for the conversation in another terminal, and the old process-inspection check is removed. This applies to Continue in Chat UI, Session History continuation, chat creation and every later agent turn in a chat. Only Claude on macOS and Linux records its process, so for Codex, Grok, OMP, OpenCode, OpenCode2, Pi, and Claude on Windows a chat can open while that agent is still live in another terminal. QA also saw a resumed Claude on macOS whose status row stored no process, so it didn't block either; that capture gap is tracked separately. This is intended: a wrong refusal is worse than letting the user continue. A possible later improvement is for the host to retire an agent's status row on its own when the shell reports the command finished.
  • No check before the close for another terminal running the conversation. The terminal's own status row in the app can't tell live from stale, so the host decides at chat creation. If the host then refuses (a live Claude in another terminal on macOS or Linux), the source tab is already closed, and the message names the other terminal and what to do.
  • Older paired hosts keep the terminal and refuse while the agent runs there. The message now says to quit the agent in that terminal and open the conversation from Session History.
  • The whole tab closes, background jobs in it included, through the ordinary close. Existing confirmations cover work other than the agent.
  • A reply in progress is cut off when the terminal closes. The chat shows the conversation up to the last saved turn.

Validation

  • CI and targeted unit tests cover these:
    • store-only admission;
    • the close-then-continue flow, including split tabs, Cancel, changed terminals, unproven exits, an open chat owner, an already-quit agent and older hosts, compared on old and new hosts;
    • host exit waits after pane removal, in every order (exit before or after the wait, list in between, same-id replacement, prune, unknown incarnation).
  • QA on real builds of this branch, macOS and Windows:
    • Agents and shells: Claude and Codex, idle and mid-reply, from the pane header and from the tab menu. On Windows this used PowerShell and Git Bash.
    • Upgrade from main.
    • Confirmations: a split tab with a dev server got the existing "Stop running command?" confirmation, where Cancel kept everything and confirming closed it. A pinned tab got only the pinned confirmation.
    • Agent already quit: the terminal was kept and the chat opened.
    • Paired host: a new paired host worked end to end, and losing contact during the close gave one message and no chat. An older paired host kept its terminal.
    • Every successful click: each shell and agent process in the closed tab was gone in two process samples, and the chat opened on the same conversation with two real replies.
  • Interrupted reply: after a mid-reply continuation the chat shows the interrupted prompt without the partial answer, because the agent hadn't saved one.

This supersedes #33721's approach for main. That PR remains separate.

@brennanb2025 brennanb2025 changed the title Stop the terminal agent when continuing its conversation in chat Close the terminal before continuing its conversation in chat Oct 31, 2026
…r on the new chat

Only work in other panes of the tab asks before the close; the worktree stays selected so the chat shows when it opens; an open chat for the conversation is opened instead of closing the terminal; the pre-close check skips the rows the host skips and counts restored ones; the preparation deadline no longer runs while a close dialog is open; failure messages say what happened once and point to Session History.
…ission

The hook server no longer tags OpenCode and tmux conversation bindings with a writer topology, the admission check drops its unused source pane argument, and the hook that clears a saved launch result is named for what it does.
Another open change still reads it, so its removal waits for that one.
…nt that is still running

Continue in Chat UI closes the terminal tab only while the clicked pane still shows its agent on this conversation; otherwise the chat opens and the terminal stays, as before. The host now refuses a takeover only when the agent's process is proven running: an agent with no recorded process, one that reported its own end, or one that can't be checked no longer blocks Session History or a chat.
@brennanb2025
brennanb2025 marked this pull request as ready for review October 31, 2026 19:59
@coderabbitai

coderabbitai Bot commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Note

Reviews paused

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.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • Resume reviews
  • Trigger review
Walkthrough

The change revises CLI conversation-holder checks and adds a capability-gated terminal-close flow for chat resume. The flow validates the source terminal, checks host admission and permission, and waits for exit evidence before launching. A confirmed close omits CLI-source metadata and preserves worktree selection. Resume feedback and tests cover close outcomes, holder conflicts, and terminal-specific failures. Terminal handle reconciliation also retains handles across pane removal and rejects stale handles.

Merge Risk: Moderate · up to 803cc

The PR can still permit concurrent CLI and native-chat writers, either through incomplete writer admission or by waiting on the wrong terminal during continuation. Both issues should be fixed before merge.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 23.08% 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 Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title "Close the terminal before continuing its conversation in chat" matches the main change. The diff makes Continue in Chat UI close the source terminal tab, wait for its PTYs to exit, and then…
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.
Description check Passed The description clearly explains the problem, user-facing changes, implementation, intended behavior, and validation, including macOS and Windows QA. It does not include the template’s explicit Visual…
Full details: Title check

Explanation

The title "Close the terminal before continuing its conversation in chat" matches the main change. The diff makes Continue in Chat UI close the source terminal tab, wait for its PTYs to exit, and then open the same conversation in a new native chat tab. The title names the main behavior change and does not describe unrelated work. It does not mention the broader hook status store admission rework, which is a secondary aspect.

Full details: Description check

Explanation

The description clearly explains the problem, user-facing changes, implementation, intended behavior, and validation, including macOS and Windows QA. It does not include the template’s explicit Visual Proof, Linked Issue (Fixes #...), or checklist sections, but the core description is substantially complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 23.08% 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 13 functions across 35 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches
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.

Share

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


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: ea530905-cfc7-43e3-a18b-fae6dcde8280
Commits

Reviewing files that changed from the base of the PR and between 24d1d10 and ce0b8bc.

Files selected for processing: 31
  • src/main/agent-hooks/server-agent-presence.test.ts
  • src/main/agent-hooks/server/server-agent-presence.ts
  • src/main/daemon/terminal-host-process-inspection.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-terminal-writer-acquire.test.ts
  • src/main/providers/local-pty-provider-foreground-process.test.ts
  • src/main/runtime/agent-status-observed-pane-identity.ts
  • src/main/runtime/cli-terminal-create-host-session-binding.test.ts
  • src/main/runtime/orca-runtime-structured-agent-session-create-intent.test.ts
  • src/main/runtime/rpc/methods/structured-agent-session-adoption-replay.test.ts
  • src/main/runtime/runtime-terminal-state-records.ts
  • src/main/runtime/session-tabs/structured-session-cli-writer-host.ts
  • src/main/runtime/session-tabs/structured-session-create-support.ts
  • src/main/runtime/structured-agent-cli-writers.test.ts
  • src/main/runtime/structured-agent-cli-writers.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-close-terminal.test.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-close-terminal.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-feedback.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-run.test.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-run.ts
  • src/renderer/src/components/terminal/close-local-terminal-tab-state.ts
  • src/renderer/src/components/terminal/running-terminal-close-guard.ts
  • src/renderer/src/components/terminal/terminal-tab-actions-unified-close.test.ts
  • src/renderer/src/components/terminal/terminal-tab-actions.ts
  • src/renderer/src/components/terminal/terminal-tab-close-running-confirm.test.ts
  • src/renderer/src/i18n/en-runtime-required.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/lib/cli-conversation-exit-guidance.ts
  • src/renderer/src/store/terminals/terminal-actions.ts
  • src/renderer/src/store/terminals/terminal-tab-close.ts
  • src/shared/structured-agent-cli-conversation-adoption.ts
  • src/shared/structured-agent-session-surface-capabilities.ts
Files with no reviewable changes: 2
  • src/main/runtime/runtime-terminal-state-records.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-terminal-writer-acquire.test.ts

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

…irst

A terminal handle a pane issued now keeps naming its process after the pane is removed, until that process record is pruned or replaced, so a late exit wait reads the recorded exit instead of failing. Before closing for Continue in Chat UI, only an open chat for the conversation is answered; whether another terminal still holds it is left to the host. A run that was already retired says nothing when its old confirmation is answered.
…minal holds the conversation

A terminal that no pane shows is listed, read and controlled through its own handle again, as before; the closed pane's handle still answers a late exit wait and nothing else, and only for a known process incarnation. After Continue in Chat UI closes the terminal, a conversation still running in another terminal names that terminal and the remedy. A continuation retired with no newer one for the same tab says once that the terminal changed when its confirmed close is refused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to GitHub limitations.

⚠️ Outside diff range comments (1)

Major · Bind each resolved PTY to its renderer layout binding. · cli-chat-resume-close-terminal.ts:48-70

src/renderer/src/components/tab-bar/cli-chat-resume-close-terminal.ts:48-70
Major · Functional Correctness · Quick win

Bind each resolved PTY to its renderer layout binding.

terminal.resolvePane can return a fallback handle for a disconnected leaf, a reminted PTY, or a same-leaf-ID PTY from another tab. The current checks accept that result and wait on terminal.handle. The continuation can then open native chat while the source-bound PTY remains active.

Compare the resolved PTY with the captured surface identity before accepting the result.

Suggested fix
       const original = originals.find((entry) => entry?.leafId === leaf)
       if (
         !terminal.ptyId ||
         !terminal.incarnationId ||
+        terminal.ptyId !== original?.ptyId ||
         !isCurrent() ||
         (original?.incarnationId && original.incarnationId !== terminal.incarnationId)
       ) {
Additional comments: 1
src/renderer/src/i18n/locales/es.json-19301-19301: 1

19301-19301: Minor · Functional Correctness · Quick win

Unverified finding
Verification ran but could not confirm this finding. It is shown for review, not as a verified issue.

Restore the Spanish terminal-conflict translations.

cli-conversation-exit-guidance.ts still requests exitUnconfirmedFromTerminal and exitUnconfirmed. The English locale still defines both keys, but this Spanish locale no longer does, so affected users receive the English fallback instead of Spanish recovery guidance. Restore both entries or remove the call sites in the same change. (raw.githubusercontent.com)


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: 4d51c0b8-d99b-4148-bbd7-63003dfb8d4a
Commits

Reviewing files that changed from the base of the PR and between 2c01904 and 803cc29.

Files selected for processing: 13
  • src/main/native-chat/agent-session-wire/structured-agent-session-terminal-writer-acquire.test.ts
  • src/main/runtime/orca-runtime-terminal-handle-leaf-removal.test.ts
  • src/renderer/src/components/tab-bar/cli-chat-resume-close-terminal.test.ts
  • src/renderer/src/i18n/locales/de.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ja.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/ru.json
  • src/renderer/src/i18n/locales/tr.json
  • src/renderer/src/i18n/locales/zh-Hant.json
  • src/renderer/src/i18n/locales/zh.json
Files with no reviewable changes: 5
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/de.json
  • src/renderer/src/i18n/locales/zh-Hant.json
  • src/renderer/src/i18n/locales/tr.json
  • src/renderer/src/i18n/locales/ru.json
Files skipped from review as they are similar to previous changes: 2
  • src/renderer/src/components/tab-bar/cli-chat-resume-close-terminal.test.ts
  • src/main/native-chat/agent-session-wire/structured-agent-session-terminal-writer-acquire.test.ts

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

…e a restart

Continue in Chat UI closes the terminal only while its agent is confirmed running there; a status row carried over a restart, with no live update since, now takes the keep-the-terminal path.
@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review loop status

The problem. Clicking Continue in Chat UI on a terminal whose agent was still running didn't open a chat. Orca refused with "already running this conversation in another terminal... open it from Session History", even though the clicked terminal was the only thing running it. On Windows (#32154) it refused even after the agent quit. Orca judged whether the terminal had let go by inspecting its processes, which is often wrong and impossible on Windows, and it never made the terminal let go.

What users get now.

  • With the agent running in the clicked terminal, the button closes that terminal tab, waits until every process in it has exited, and opens the same conversation in a new chat tab. The agent itself gets no confirmation. Other running work in a split gets Orca's existing "Stop running command?" confirmation, and a pinned tab gets the pinned confirmation.
  • If the agent already quit, the terminal stays and the chat opens, as before.
  • If a chat already owns the conversation, Orca switches to that chat.
  • If an exit can't be confirmed, one message appears and no chat opens.
  • Only a live process recorded in the host's status store blocks a chat. Brennan's ruling was "a lot better if we let the user do the continue in native chat than restricting them wrongly".

Fixed during review (6 rounds, full path):

  • Every click had opened "Stop this agent?". It no longer does.
  • Closing a worktree's only tab used to drop the user on the welcome screen with the chat hidden. The worktree now stays selected.
  • Time spent reading the confirmation counted against a 30 s timer, so a late confirm did nothing.
  • A chat that already owned the conversation was ignored.
  • Several failure messages were misleading or shown twice.
  • A stale status row (any agent but Claude on Mac/Linux) could block Session History until the terminal closed. Store admission now refuses only on a live process.
  • On paired hosts, the exit wait could arrive after the close and fail. The host now keeps answering exit waits for a just-closed terminal from its own record, until that record is pruned. A three-model panel ruled this design.
  • After an agent quit, the tab was closed anyway. It is now kept.
  • The old-host refusal pointed to a retry that's impossible there. It now points to Session History.

Deferred (tickets):

  • STA-13214: right after a restart, a terminal whose agent didn't come back can still be closed by the button. A decider ruled the fix that was tried worse.
  • STA-13222: a resumed Claude on macOS stored no process in its status row, so it didn't block a second writer. This is in main's capture code; the coordinator ruled it ships as is under the store-only ruling.
  • Unused writer-topology tagging stays, because it's shared with Switch chats between Chat UI and a terminal without forking #31595.

Verified (QA on head d133d27d, real builds, Claude and Codex):

  • Mac (this Mac):
    • upgrade from main;
    • idle and mid-reply clicks from the header and the tab menu;
    • a split with a dev server: Cancel kept everything, and confirming closed it and stopped the server;
    • a pinned tab;
    • agent already quit: the terminal was kept;
    • a paired new host (Codex idle, Claude mid-reply);
    • contact lost during the close: one message, "Orca closed Claude's terminal but couldn't confirm it stopped, so the chat didn't open. Check the host connection, then open the conversation from Session History.", and no chat;
    • an old paired host.
  • Windows (win-high):
    • Claude in PowerShell and Codex in Git Bash, idle and mid-reply, including after a graceful restart;
    • a second terminal on the same conversation.
  • Every success:
    • each shell and agent process in the closed tab was gone in two process samples;
    • the chat opened on the same conversation;
    • two real replies came back.
  • CI and checks: CI is green. The references comparison has no unlabelled differences. The readiness checklist found no P0/P1.

Not verified or limited:

  • An OpenCode 1.19.3 continuation closed and then reported "not saved yet". The rig ran OpenCode with a non-default data folder, which confounds that result.
  • A mid-reply continuation shows the interrupted prompt without the partial answer, because the agent hadn't saved one.

Seen outside this PR:

  • On Windows, a Git Bash tab came back as PowerShell after a graceful restart (not checked on main).
  • An info toast, "Orca now runs Codex without its shared server", appeared during a Codex run.

After QA: 1ce52ec9 changes only the old-host refusal text (exitUnconfirmedFromTerminal), a copy-only tests-row flow. It is covered by cli-conversation-exit-guidance.test.ts and was checked by the continued reviewer.

For Brennan's review

M1-chat-open.png M5-dialog.png
Mac: idle Claude continued in a new chat (left). A split with a dev server shows the existing confirmation (right).

M7-result.png P1a-chat.png
Agent already quit: the terminal is kept (left). A paired new host, Codex (right).

W1-idle-chat.png W2-mid-chat.png
Windows: Claude idle in PowerShell (left), Codex mid-reply in Git Bash (right).

W3-result.png M4-unexpected-chat-open.png
A second terminal still running the conversation doesn't block, by the store-only ruling (Windows left, Mac right).

Spots where judgment matters:

  1. structured-agent-cli-writers.ts:24: only a live recorded process refuses a chat. Every other case admits.
  2. cli-chat-resume-run.ts:152: the tab closes only while the clicked pane's agent row is live.
  3. terminal-handle-bindings.ts:57 and :182: a closed pane's handle keeps answering exit waits from the host's record.

@brennanb2025
brennanb2025 merged commit af211d6 into main Nov 1, 2026
90 of 93 checks passed
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