Skip to content

feat(quit): add Quit and Stop All Sessions to the app menu and tray - #32153

Merged
nwparker merged 15 commits into
stablyai:mainfrom
seijikohara:feat/quit-and-stop-all-sessions
Oct 27, 2026
Merged

nwparker merged 15 commits into
stablyai:mainfrom
seijikohara:feat/quit-and-stop-all-sessions

Conversation

@seijikohara

@seijikohara seijikohara commented Oct 23, 2026 •

Copy link
Copy Markdown
Contributor

ELI5

Quitting Orca leaves your terminals and agents running in the background on purpose, so they are still there when you come back. Sometimes you want the opposite: quit and stop everything. This PR adds Quit and Stop All Sessions... next to Quit. It asks first, closes every terminal tab and stops its session the way Settings → Kill all sessions does, and then quits.

What Changed

Before: Quit (Cmd+Q, the menu bar icon, the tray) always left every daemon session running. Stopping them took Settings → Terminal → Manage Sessions → Kill all sessions, then Quit, or killing processes by hand.

After: Quit and Stop All Sessions... sits directly above Quit in the macOS app menu, the macOS menu bar icon menu and the Windows tray menu, and above Exit in the Windows and Linux File menus. It shows a native confirmation with Cancel as the default button. On confirm, Orca closes every terminal tab, stops every session in every local daemon generation, and quits; the empty daemons then retire through their existing idle path. Projects and worktrees stay in the sidebar, and the closed tabs do not come back. Plain Quit is unchanged.

sequenceDiagram
  participant Main
  participant Renderer
  Main->>Main: Confirm (Cancel is the default)
  Main->>Main: Show the window, record a one-shot request for its webContents (30 s)
  Main->>Renderer: ui:quitAndStopAllSessions
  Renderer->>Main: ui:consumePendingQuitAndStopAllSessions (also pulled on mount)
  Renderer->>Renderer: Wait for the restored session, then run Kill all sessions
  alt Every session stopped and no terminal tab is left
    Renderer->>Main: ui:quitAfterStoppingAllSessions
    Main->>Main: List every daemon generation again (up to 6.5 s)
    alt Every generation answered and none lists a session
      Main->>Main: Show the window, then the existing user Quit
    else A session is still listed, or a generation did not answer
      Main->>Renderer: false
      Renderer->>Renderer: Error toast, Orca stays open
    end
  else Anything failed
    Renderer->>Renderer: Error toast, Orca stays open
  end
Loading
  • The request is one-shot and scoped to the window main sent it to, and main accepts the follow-up quit only from the renderer that consumed it. A cancelled dialog or a cancelled quit leaves nothing behind.
  • The renderer waits for waitForWorkspaceSessionReady() and requires hydrationSucceeded, because closed tabs are persisted only after a successful restore. It then reuses runKillAllTerminalSurfaces, the Settings Kill all path, which also sweeps legacy daemon generations.
  • It quits only on a clean result: the daemon sweep fulfilled with every daemon generation listed and nothing remaining, no failed tab close, no rejected PTY shutdown, and no terminal tab left over (for example, one opened during the sweep).
  • Main then lists every daemon generation again before it quits. Kill all counts only the sessions of its first listing, so a session started since then would otherwise outlive the quit, for example one started from the CLI or an automation before its tab reaches the renderer. Main polls for up to the same 6.5 s, so the PTY shutdowns of the tab sweep can finish. It quits only after a listing in which every generation answers and none lists a session; otherwise Orca stays open with the same error.
  • pty:management:killAll no longer reads a daemon generation that failed to list as "no sessions":
    • A poll updates only the generations that answered, so a failed listing keeps that generation's sessions remaining.
    • A failed initial listing, no daemon provider at all (failed startup), or a held daemon from a newer Orca version sets listingIncomplete.
    • The poll runs in a 6.5 s window instead of 65 attempts, because a wedged generation fails each listing only at the 30 s request timeout.
    • Before, a generation that stopped answering after the shutdown requests had its sessions counted as killed. Settings' Kill all now shows them as remaining. Only this quit action reads listingIncomplete.
  • When Kill all could not list every session, because the daemon provider is not installed (the daemon is still starting or failed to start) or a daemon from a newer Orca version is held, the item explains that instead of asking and stops nothing. listingIncomplete stays as the backstop.
  • orca serve hosts do not get the item, because their user Quit only closes the windows ([Bug]: macOS — ⌘Q on a serve-hosted desktop window kills the headless runtime #15537).
  • The tray and the app menu share one label key. The app-menu item has a stable id because its label is localized. The web client stubs the request listener and pull.
  • New strings are translated for es, fr, ja, ko, pt and zh. docs/site/content/docs/model/session-restore.mdx documents the action.

Why

#3191 was closed as intended behavior, with a request for a feature request proposing either a stop-on-close preference or an explicit "Quit and stop all sessions" action. An explicit per-quit action keeps warm reattach as the default and lets users stop everything when they want to, without a persistent setting. #14652 proposes the setting; the two complement each other.

I first tried stopping the sessions inside will-quit so the tabs would survive. That needs the renderer to report every cancelled close (running-work, unsaved-file, close-guard and checkpoint prompts), or a later plain Quit would still stop everything, and the restored tabs would resume their agents on the next launch. Reusing Kill all sessions before quitting has neither problem: nothing is left pending if the quit is cancelled, and nothing is left to resume.

Linked Issue

Fixes #31448

Visual Proof

macOS, in English. Before: a local build of the base commit d667e54. After: a local build of this branch. Both run as dev instances, so macOS shows the unpackaged app as "Electron" in the menu bar, and the tray menu's first line names the dev worktree; release builds show neither.

Before After
App menu before-app-menu after-app-menu
Menu bar icon menu before-tray-menu after-tray-menu

The confirmation, and the sidebar after confirming and relaunching: the project and its worktree stay, and no terminal tab comes back.

Confirmation After relaunch
after-confirm-dialog after-relaunch-sidebar

Testing

Automated and manual tests ran on macOS (Apple silicon). Windows is covered by the per-platform menu unit tests only; CI also runs the E2E spec on Linux once a maintainer approves the workflow runs. SSH: not tested; a connected SSH tab is closed through the same Kill all path as Settings, and the dialog says sessions on remote hosts may keep running.

  • Manual, with a local build of this branch (its own dev profile and daemon) and a terminal running sleep 86401:

    • The item sits directly above Quit in the app menu and in the menu bar icon menu.
    • Cancel stops nothing: the app, the daemon, the shell and sleep keep running, and the tab stays open.
    • Confirming from the menu bar icon menu quits in about a second, and the shell, sleep and the daemon exit. A release Orca (1.4.232) running alongside with its own profile kept its daemon running.
    • After relaunch, the project and its worktree are still in the sidebar, the closed tab does not come back, and no terminal is running.
    • Plain Quit still leaves sleep and the daemon running, and Orca reattaches to them on relaunch.
    • Not checked by hand: that Return picks Cancel (the prompt unit test covers the default button), Windows, and Linux.
  • E2E tests/e2e/quit-and-stop-all-sessions.spec.ts (electron-headless): opens a terminal in the seeded repo, records the shell and daemon PIDs, triggers the app-menu item with the native dialog stubbed to confirm, and checks that the app exits, the shell and the daemon are gone, and after relaunch the worktree is still listed while the closed tab is not restored. It finds the item by id, so it also passes with a non-English UI language.

  • Unit:

    • main-window-actions-quit-and-stop-all-sessions.test.ts: confirmation on the revealed window, the windowless path that opens a window, request expiry, cancel, one dialog at a time, a failed confirmation that can be retried, the explanation shown instead while the daemon is not ready or a newer version's daemon is held, serve mode, the tray and app-menu quit handlers for desktop and serve hosts, and a final quit that reveals the window and is accepted once, only from the consuming renderer, even after the request has expired, and that stays open, without using up a later request, when the final listing still shows a session.
    • quit-and-stop-all-sessions-ipc-bridge.test.ts: push and pull consume once, the wait for the restored session, failed hydration, one Kill all at a time, staying open on every failure, including a tab opened during the sweep and a thrown error (logging why for the not-ready, leftover-tab and thrown cases; Kill all logs its own summary), a failed request pull, main refusing the quit after its final listing, and a retry after a failure.
    • pty-management.test.ts: the final listing before the quit waits for a session that is still exiting, and refuses when a session is still listed or a generation cannot list at the end of the window. For Kill all, a generation that stops answering keeps its sessions remaining, and the poll stops after the first listing that times out past the 6.5 s window; an answering generation still ends the sweep early; a failed listing, a missing provider or a held daemon from a newer version sets listingIncomplete. A generation that has confirmed its stop and then fails a later poll keeps that stop and does not set listingIncomplete.
    • Prompt, tray, app-menu and useIpcEvents-lifecycle tests: Cancel as the default, placement above Quit per platform, the label refresh on a language switch, and subscribing before the pull.
  • pnpm lint and pnpm typecheck pass locally, as do the changed-code quality gate and the localization verifiers.

  • pnpm test fails locally in 385 of 13,460 test files. None of the failures comes from this change, and no failing file covers a module this PR changes:

    • 369 of those files also fail on unmodified main @ d667e54, this branch's base, in the same environment. The causes are local: macOS bash 3.2 (mapfile: command not found, MOBILE_CI_ARGS[@]: unbound variable), /var versus /private/var temp paths, zsh prompt and PTY timeouts, and an unresolved @electron-toolkit/tsconfig.
    • 15 of the other 16 pass when run alone. Under full-suite load they hit timeouts, a timing threshold, or assertion failures; the two PTY tests, the native-chat test and the macOS update test also passed three runs in a row alone.
    • The last one, the issuedForPaneKey census in replaced-pane-handle-recovery.test.ts, times out when run alone on both this branch and d667e54.
  • pnpm build fails locally while bundling the mobile web app: react-native-web and expo-router cannot be resolved. Unmodified d667e54 fails the same way here.

  • After merging main @ a02a635 to resolve conflicts with the comment audit ([comment audit] Clean up IPC, host lifecycle and native-chat comments #32966) and the locale updates, pnpm tc, the changed modules' tests, the localization checks, the changed-code quality gate and the E2E spec pass again.

  • After merging main @ 3aec926 to resolve conflicts with refactor: group PTY bindings and lifecycle suites #33566 (which moved pty-management.ts and its test into src/main/ipc/pty/), fix(terminal): show the stalled-daemon banner when the daemon is actually stalled #33000, feat(browser): dock DevTools in the browser pane, with dock side, resizing and pop-out #33003 and the locale updates, these pass again: the changed modules' tests and the menu, tray and PTY suites, the localization checks, the lint-staged commands on the changed files, the changed-code quality gate, the other pnpm lint steps and the E2E spec. The E2E spec's test.setTimeout now has the reason comment that the new test-timeout-needs-reason rule (perf(lint): keep JS plugins out of default oxlint and adopt ast-grep #33430) requires. Two failures come from main @ 3aec926 itself and involve no file in this PR; main fixed both in fix(ci): repair main's line limits and a stale test import #33675:

  • After merging main @ d5ac6eb to resolve a conflict with fix(native-chat): render math and center standalone equations #31546 in en.json (both added a top-level block at the end; both kept), these pass again: pnpm typecheck, the localization checks, the pre-commit lint-staged commands on the merge commit and the E2E spec.

    • Of the changed modules' tests and the menu, tray and PTY suites, 135 of 139 files pass at a load average of about 30. The other four:
      • pty-spawn-scaling.performance.test.ts passes alone.
      • pty-claude-agent-view-readiness.test.ts and useIpcEvents-lifecycle.test.ts (its 30 s listener test) fail the same way on a checkout of main @ d5ac6eb that carries only fix(i18n): localize hosted review creation errors #33368's hosted-review and locale changes.
      • pty-hidden-at-spawn-mark.test.ts waits 1 s for the daemon spawn. Run alone, it passes in 2 of 4 runs on this branch, and it also fails in the suite run and in a four-file rerun. On that checkout it passes in 2 of 2 runs alone and in a three-file rerun. None of this PR's changed TypeScript modules is among the 7,468 files it imports statically.
    • pnpm build fails at the same mobile web app bundle step as above.
  • e03805a adds the final listing before the quit (CodeRabbit's review of 52f0775). With it, these pass: pnpm typecheck, the pre-commit lint-staged commands, the changed-code quality gate, the max-lines ratchet, and the E2E spec. Of the changed modules' tests and the menu, tray, startup and PTY suites, 211 of 220 files pass and 2 are skipped, at a load average of about 20 to 70. The other seven are PTY start timing tests and the 30 s listener test in useIpcEvents-lifecycle.test.ts. In two more runs on this branch and two on the main @ d5ac6eb checkout above, tests in the same seven files failed on both trees, 4 to 6 a run, in a different combination each time.

  • check:flaky-test-pattern-ratchet could not run locally: it diffs against this fork's outdated origin/main, and the diff output overflows. Its test-* rules report no finding in this PR's test files.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Implemented and reviewed with Claude Code (Claude Opus 5.5); see Review below. I chose the design (reuse Kill all sessions before quitting) and the scope.

Review

AI review with Claude Code, six rounds, each finding checked by an adversarial verifier:

  • Rounds 1-3 reviewed the whole change in six dimensions: project conventions, idiom and redundancy, correctness, tests, text and localization, and cross-platform/security/performance.
  • Rounds 4-6 reviewed each previous round's fixes (six, four, then two dimensions).
  • The main fixes it led to: Kill all no longer reads a daemon generation that failed to list as "no sessions"; a tab opened during the sweep keeps Orca open; the daemon poll is bounded in time; and the item explains instead of running Kill all when not every session can be listed.
  • CodeRabbit's review of 52f0775 found that a session started after Kill all's first listing could outlive the quit; main now lists every generation again before quitting.
  • Round 6 confirmed no finding of severity minor or above; its three nits are fixed.

Compatibility and risk:

  • Cross-platform: the item is in the macOS app menu and menu bar icon menu, the Windows File menu and tray, and the Linux File menu (Linux has no tray). Per-platform menu unit tests cover each placement; manual testing was on macOS only.
  • SSH/remote/local: Kill all closes a connected SSH tab and stops its PTY, as Settings does. A tab on a disconnected SSH host can only be retired locally, so the dialog says sessions on remote hosts may keep running. orca serve hosts do not get the item.
  • Agents and integrations: no agent- or integration-specific code. Agents stop because their terminal sessions stop, through the same Kill all path as Settings.
  • Performance: plain Quit is unchanged. Kill all closes tabs in bounded batches, and its daemon poll and the final listing before the quit are each bounded to 6.5 s.
  • UI quality: native menu items and a native message box, with labels in all seven locales and Cancel as the default button; see the screenshots above.
  • Security: the two renderer-to-main channels act only for the webContents that main sent the request to; no new privilege.

Known limits:

  • If the renderer is frozen, the request expires after 30 s without stopping anything and without a message.
  • Clicking the item again while Kill all runs can show a second confirmation, which is then ignored.
  • While Orca is still starting, after its terminal daemon failed to start, or while it holds a newer Orca version's daemon, the item explains that and stops nothing. Orca reads the newer-version hold once when it starts (Restart daemon does not re-read it), so the explanation can repeat after that daemon exits on its own, until Reopen on a held terminal or a relaunch. If that daemon still runs a session that no terminal here shows, for example because a held tab was closed instead of reopened, the hold returns on every launch until that session is stopped from the newer version.
  • A session started after main's final listing and before Orca exits can still survive. Closing that gap would need a spawn fence across the CLI, automations and paired clients, which this PR does not add.
  • Kill all does not await a paired Orca server's tab close, so that tab can come back if the server never received the close. The docs say so; awaiting those closes in Kill all is a separate change.

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Security: the two renderer-to-main channels act only for the webContents that main sent the request to; no new privilege, since the renderer can already call app:relaunch.
  • Cross-platform: the tray item exists on macOS and Windows; Linux has no tray, so it gets the File menu item only. No accelerator, because macOS uses ⌥⌘Q for "Quit and Keep Windows". One label is used everywhere, including next to "Exit" on Windows and Linux.
  • SSH/remote: as with Settings → Kill all sessions, closing a connected SSH tab stops its PTY; a tab on a disconnected SSH host can only be retired locally, so the dialog says remote sessions may keep running.
  • Mobile: N/A; the mobile app has no Kill all or quit action, and this quits the desktop app only.
  • Backwards compatibility: no setting or persisted-state change; killAll gains an optional field.
  • Folder workspaces: Kill all closes terminal tabs in every workspace kind, and folder workspaces stay in the sidebar like worktrees.
  • Performance: plain Quit is unchanged; Kill all closes tabs in bounded batches, and its daemon poll is now bounded in time.

Author

X handle: none.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

Quit keeps daemon sessions alive for warm reattach. The new item, placed
directly above Quit in the macOS app menu and menu bar icon, the Windows
File menu and tray, and the Linux File menu, asks for confirmation, runs
Settings' Kill all sessions in the renderer, and quits only when every
session stopped. Closing the tabs first means nothing reopens or resumes
on the next launch. Serve hosts do not offer the item.
…opped

- Kill all no longer reads a daemon generation that failed to list as
  "no sessions": a failed poll keeps the last confirmed remaining set,
  and a failed initial listing sets listingIncomplete.
- Quit and Stop All Sessions stays open on an incomplete listing or a
  terminal tab opened during the sweep, logs why it stayed open, and
  reveals the window before the final quit.
- Share the main-window message box helper, stub the request on the web
  client, and add Portuguese strings.
- Cover the windowless path, request expiry, the in-flight guard, and
  failed listings in tests; bound the E2E exit wait.
- A poll updates only the daemon generations that answered, so one
  failing generation no longer hides the kills of the others.
- Poll for 6.5 s of wall-clock time instead of 65 attempts, because a
  wedged generation fails each listing only at the request timeout.
- Flag a sweep with no daemon provider as listingIncomplete.
- Log the leftover-tab exit, share the one-shot window intent with
  Open Settings, and cover the sender check, the reveal before the final
  quit, and retries in tests.
- Set listingIncomplete while this build holds a newer protocol's
  daemon, whose sessions it can neither list nor stop.
- Reuse getDaemonAdapters in Kill all, type the shared one-shot helper
  by its flag factory, and move the app-menu quit handlers next to the
  tray options so both serve/desktop splits are tested.
- Cover a quit that outlasts the request expiry and a failed request
  pull, and fix the docs lead-in for the new quit-time case.
…runs terminals

- Check the newer-version daemon hold before the confirmation and show
  an explanation instead, so Kill all no longer closes every tab and
  then refuses. listingIncomplete stays as the backstop.
- Translate the explanation, describe listingIncomplete generically,
  and assert the single timed-out poll in the Kill all test.
…session

- Show the explanation instead of the confirmation while the daemon
  provider is missing (still starting or failed) as well as while a
  newer version's daemon is held.
- Name only the steps that work (wait for terminals to open, reopen a
  terminal opened in a newer version), avoid stating a hold as fact, and
  label the dialog's button through the catalog.
@seijikohara
seijikohara marked this pull request as ready for review October 23, 2026 04:59
Copilot AI balanced review requested due to automatic review settings October 23, 2026 04:59

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@coderabbitai

coderabbitai Bot commented Oct 23, 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

Adds a localized “Quit and Stop All Sessions” action to app menus and supported system trays. After confirmation, the renderer stops terminal sessions and permits quitting only when the stop completes and no tabs remain. Session-listing failures are reported as incomplete. The change also updates restoration documentation and adds unit and end-to-end tests.

Priority: Normal

Merge Risk: Moderate · previous review e0380

Quit and Stop All Sessions can leave local sessions running after a concurrent daemon restart. Recheck the provider during final confirmation before merging.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title "feat(quit): add Quit and Stop All Sessions to the app menu and tray" names the main change: a new Quit and Stop All Sessions action in the app menu and tray. The changeset matches it. The t…
Description check Passed The description follows the repository template closely. It includes ELI5, What Changed (before and after, with a mechanism diagram), Why (with alternatives considered), a Linked Issue (Fixes #31448),…
Linked Issues check Passed The active directly linked issue is [#31448]. The PR adds the action in the required macOS, Windows, and Linux menus and in the macOS and Windows tray menus. It uses a native confirmation with Cancel …
Out of Scope Changes check Passed The changes remain within [#31448]. Menu, tray, prompt, IPC, renderer bridge, PTY listing, localization, documentation, and E2E changes implement or verify the requested action. The `listingIncomplete…
Docstring Coverage Passed Docstring coverage is inconclusive: files from earlier reviews still need to be rechecked within the check's analysis limits. Not blocking.
Full details: Title check

Explanation

The title "feat(quit): add Quit and Stop All Sessions to the app menu and tray" names the main change: a new Quit and Stop All Sessions action in the app menu and tray. The changeset matches it. The title does not mention the confirmation, the File menus on Windows and Linux, or the daemon-listing safeguards, but it does not need to cover every detail. It is 67 characters, which is within a normal title length and a little over the preferred concise form.

Full details: Description check

Explanation

The description follows the repository template closely. It includes ELI5, What Changed (before and after, with a mechanism diagram), Why (with alternatives considered), a Linked Issue (Fixes #31448), Visual Proof with before and after screenshots for macOS, a detailed Testing section listing manual, E2E, and unit coverage, AI Disclosure, Review, the Agent skill upstream boundary checkbox, Notes covering security, cross-platform, SSH, mobile, and backwards compatibility, and a completed Checklist. Gaps: the Testing section does not cover Windows or Linux beyond unit tests, and the SSH path was not tested by hand. The description is also very long, which goes against the template's 'keep scope tight' guidance, but it is complete in structure and substance.

Full details: Linked Issues check

Explanation

The active directly linked issue is [#31448]. The PR adds the action in the required macOS, Windows, and Linux menus and in the macOS and Windows tray menus. It uses a native confirmation with Cancel as the default. The confirmed flow closes terminal tabs, stops sessions across local daemon generations, verifies complete shutdown, and quits only on success. Failures keep Orca open and show an error. Projects and worktrees remain, while closed tabs do not restore. Plain Quit and orca serve behavior remain unchanged. The dialog documents remote-session limits. Tests cover menu placement, confirmation, IPC flow, incomplete listings, failure handling, and end-to-end shutdown.

Full details: Out of Scope Changes check

Explanation

The changes remain within [#31448]. Menu, tray, prompt, IPC, renderer bridge, PTY listing, localization, documentation, and E2E changes implement or verify the requested action. The listingIncomplete and bounded polling changes support the required safe behavior when daemon listings fail or sessions remain. No demonstrated unrelated feature or change is present.

  • 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

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

Resolve the conflicts with the comment audit (stablyai#32966) by keeping this
branch's code and upstream's shorter comment style, and the locale
conflicts by taking upstream's catalogs and re-adding the quit keys.
Copilot AI balanced review requested due to automatic review settings October 25, 2026 00:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@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: 385f3a12-14a5-4f78-8fde-7d045c7ff194
Commits

Reviewing files that changed from the base of the PR and between 40845a0 and c4e92f7.

Files selected for processing: 14
  • src/main/ipc/pty-management.test.ts
  • src/main/ipc/pty-management.ts
  • src/main/menu/app-menu-quit-item.ts
  • src/main/menu/register-app-menu.test.ts
  • src/main/menu/register-app-menu.ts
  • src/preload/api/pty-management-api.ts
  • src/preload/api/ui-command-event-api.ts
  • src/renderer/src/i18n/locales/en.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/zh.json
Files skipped from review as they are similar to previous changes: 7
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/ja.json

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

Comment thread src/main/ipc/pty/pty-management.ts Resolved
# Conflicts:
#	src/main/ipc/pty/pty-management.test.ts
#	src/main/ipc/pty/pty-management.ts
#	src/main/menu/register-app-menu.ts
#	src/renderer/src/i18n/locales/en.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/zh.json
Copilot AI balanced review requested due to automatic review settings October 26, 2026 01:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

# Conflicts:
#	src/renderer/src/i18n/locales/en.json
Copilot AI balanced review requested due to automatic review settings October 27, 2026 02:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@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 · Fence post-listing session creation before reporting success. · pty-management.ts:211-237

src/main/ipc/pty/pty-management.ts:211-237
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Fence post-listing session creation before reporting success.

killAll filters the polling result back to initial, so a session created or respawned after the first listing is omitted from both remainingCount and killedSessionIds. If its PTY tab has not been published, the renderer can also pass its empty-tab check. Main then calls quitFromSystemTray() without another daemon check. Propagating listingIncomplete from later polls does not fix this omission. Fence session creation until the final snapshot, or perform a final daemon re-list and reconcile newly created sessions before returning success.


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: 4137c776-d640-4f7d-9a2c-d9319e17e4a8
Commits

Reviewing files that changed from the base of the PR and between 766133d and 52f0775.

Files selected for processing: 8
  • src/preload/api/ui-command-event-api.ts
  • src/renderer/src/i18n/locales/en.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/zh.json
Files skipped from review as they are similar to previous changes: 7
  • src/renderer/src/i18n/locales/zh.json
  • src/renderer/src/i18n/locales/es.json
  • src/renderer/src/i18n/locales/en.json
  • src/renderer/src/i18n/locales/fr.json
  • src/renderer/src/i18n/locales/ko.json
  • src/renderer/src/i18n/locales/pt.json
  • src/renderer/src/i18n/locales/ja.json

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

… all

Kill all counts only the sessions of its first listing, so a session
started since then, for example from the CLI or an automation before its
tab reaches the renderer, would outlive the quit. Main now lists every
daemon generation again before quitting, and quits only after a listing
in which every generation answers and none lists a session. It polls
within Kill all's 6.5 s window so the tab sweep's PTY shutdowns can
finish; otherwise the renderer shows the existing error and Orca stays
open.

ui:quitAfterStoppingAllSessions becomes an invoke that answers whether
main quits.
Copilot AI balanced review requested due to automatic review settings October 27, 2026 07:41

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@seijikohara

Copy link
Copy Markdown
Contributor Author

Fence post-listing session creation before reporting success. (pty-management.ts:211-237, outside-diff finding in the review of 52f0775)

Fixed in e03805a with the final re-list, not a fence:

  • Before quitting, main lists every daemon generation again (confirmNoDaemonSessionsRemain in pty-management.ts, called from quitAfterRendererStoppedAllSessions).
  • It quits only after a listing in which every generation answers and none lists a session. It polls within the same 6.5 s window as Kill all, so the tab sweep's PTY shutdowns can finish.
  • Otherwise main answers false, and the renderer shows the existing "Couldn't stop every terminal session, so Orca stayed open." error.
  • killAll itself is unchanged. Counting only its first listing is how Settings' Kill all already works on main, and changing it there is out of scope.

A session started between that final listing and process exit can still survive. Closing that gap needs a spawn fence across the CLI, automations and paired clients, which this PR does not add; the PR description lists it under Known limits.

@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: 5e6efe8c-1604-41ff-bb58-256856e1cf7b
Commits

Reviewing files that changed from the base of the PR and between 52f0775 and e03805a.

Files selected for processing: 9
  • src/main/ipc/pty/pty-management.test.ts
  • src/main/ipc/pty/pty-management.ts
  • src/main/startup/main-process-ipc-bootstrap.ts
  • src/main/startup/main-window-actions-quit-and-stop-all-sessions.test.ts
  • src/main/startup/main-window-actions.ts
  • src/preload/api/ui-bridge-state-and-menu-commands.ts
  • src/preload/api/ui-command-event-api.ts
  • src/renderer/src/hooks/ipc-events/quit-and-stop-all-sessions-ipc-bridge.test.ts
  • src/renderer/src/hooks/ipc-events/quit-and-stop-all-sessions-ipc-bridge.ts

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

Comment on lines +99 to +103
const adapters = getDaemonAdapters()
const pollDeadline = Date.now() + POLL_WINDOW_MS
do {
const { sessions, unlistedVersions } = await listSessionsOfEveryAdapter(adapters)
if (sessions.length === 0 && listedEveryGeneration(adapters, unlistedVersions)) {

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.

Major · Stability & Availability · Quick win

Supported by static analysis

Script executed:

sed -n '55,115p' src/main/ipc/pty/pty-management.ts
rg -n 'replaceDaemonProvider|getDaemonProvider|listSessions|confirmNoDaemonSessionsRemain' src/main/daemon src/main/ipc/pty/pty-management.ts src/main/startup/main-window-actions.ts

Repository: stablyai/orca

Length of output: 25706


Script executed:

set -o pipefail
printf '%s\n' '--- pty management imports and handlers ---'
sed -n '1,70p' src/main/ipc/pty/pty-management.ts
sed -n '115,255p' src/main/ipc/pty/pty-management.ts
printf '%s\n' '--- provider state ---'
sed -n '1,180p' src/main/daemon/daemon-provider-state.ts
printf '%s\n' '--- provider/router adapter definitions ---'
rg -n -F --glob '*.ts' -e 'class DaemonPtyRouter' -e 'class DaemonPtyAdapter' -e 'getDaemonAdapters' -e 'listSessions(): Promise' src/main/daemon
printf '%s\n' '--- replacement implementation ---'
sed -n '1,235p' src/main/daemon/daemon-provider-restart.ts
printf '%s\n' '--- stop-and-quit caller ---'
sed -n '130,215p' src/main/startup/main-window-actions.ts

Repository: stablyai/orca

Length of output: 27937


Script executed:

set -o pipefail
sed -n '1,70p' src/main/ipc/pty/pty-management.ts
sed -n '115,255p' src/main/ipc/pty/pty-management.ts
sed -n '1,180p' src/main/daemon/daemon-provider-state.ts
rg -n -F --glob '*.ts' -e 'class DaemonPtyRouter' -e 'class DaemonPtyAdapter' -e 'getDaemonAdapters' -e 'listSessions(): Promise' src/main/daemon
sed -n '1,235p' src/main/daemon/daemon-provider-restart.ts
sed -n '130,215p' src/main/startup/main-window-actions.ts

Repository: stablyai/orca

Length of output: 27762


Script executed:

set -o pipefail
printf '%s\n' '--- provider routing ---'
sed -n '1,260p' src/main/daemon/daemon-provider-routing.ts
printf '%s\n' '--- adapter and inventory ---'
sed -n '1,180p' src/main/daemon/daemon-pty-adapter.ts
sed -n '90,135p' src/main/daemon/daemon-pty-session-inventory.ts
printf '%s\n' '--- client request settlement ---'
rg -n -F --glob '*.ts' -e 'async request' -e 'request<T' -e 'pending' -e 'disconnect()' src/main/daemon/client.ts
sed -n '1,280p' src/main/daemon/client.ts
printf '%s\n' '--- server list and shutdown behavior ---'
sed -n '130,175p' src/main/daemon/daemon-request-router.ts
sed -n '1,110p' src/main/daemon/daemon-server.ts
printf '%s\n' '--- restart callers ---'
rg -n -F --glob '*.ts' -e 'restartDaemon(' -e 'replaceDaemonProvider(' src/main | head -120

Repository: stablyai/orca

Length of output: 33190


Script executed:

set -o pipefail
printf '%s\n' '--- protocol cleanup ---'
sed -n '1,260p' src/main/daemon/daemon-protocol-cleanup.ts
printf '%s\n' '--- client disconnect settlement ---'
sed -n '260,350p' src/main/daemon/client.ts
sed -n '1,240p' src/main/daemon/daemon-client-pending-requests.ts
printf '%s\n' '--- server request dispatch and connection close ---'
rg -n -F --glob '*.ts' -e 'requestRouter' -e 'route(' -e 'destroy()' -e 'close()' -e 'shutdown' src/main/daemon/daemon-server.ts src/main/daemon/daemon-client-connections.ts src/main/daemon/daemon-server-lifecycle.ts
printf '%s\n' '--- cleanup callers/tests ---'
rg -n -F --glob '*.ts' -e 'cleanupDaemonForProtocol' -e 'listSessions failed' -e 'listSessions' src/main/daemon/daemon-protocol-cleanup.test.ts src/main/daemon/daemon-init-restart-sequence.test.ts

Repository: stablyai/orca

Length of output: 12652


Script executed:

set -o pipefail
sed -n '1,260p' src/main/daemon/daemon-protocol-cleanup.ts
sed -n '260,350p' src/main/daemon/client.ts
sed -n '1,240p' src/main/daemon/daemon-client-pending-requests.ts
rg -n -F --glob '*.ts' -e 'requestRouter' -e 'route(' -e 'destroy()' -e 'close()' -e 'shutdown' src/main/daemon/daemon-server.ts src/main/daemon/daemon-client-connections.ts src/main/daemon/daemon-server-lifecycle.ts
rg -n -F --glob '*.ts' -e 'cleanupDaemonForProtocol' -e 'listSessions failed' src/main/daemon/daemon-protocol-cleanup.test.ts src/main/daemon/daemon-init-restart-sequence.test.ts

Repository: stablyai/orca

Length of output: 12411


Script executed:

set -o pipefail
printf '%s\n' '--- confirmation and replacement line numbers ---'
nl -ba src/main/ipc/pty/pty-management.ts | sed -n '94,108p;280,300p'
nl -ba src/main/daemon/daemon-provider-restart.ts | sed -n '165,195p'
nl -ba src/main/daemon/daemon-provider-state.ts | sed -n '94,104p'
printf '%s\n' '--- request dispatch and list response ---'
nl -ba src/main/daemon/daemon-server.ts | sed -n '286,305p'
nl -ba src/main/daemon/daemon-request-router.ts | sed -n '145,160p'
printf '%s\n' '--- shutdown ordering ---'
nl -ba src/main/daemon/daemon-server-lifecycle.ts | sed -n '150,190p;245,275p'

Repository: stablyai/orca

Length of output: 7680


Script executed:

set -o pipefail
nl -ba src/main/ipc/pty/pty-management.ts | sed -n '94,108p;280,300p'
nl -ba src/main/daemon/daemon-provider-restart.ts | sed -n '165,195p'
nl -ba src/main/daemon/daemon-provider-state.ts | sed -n '94,104p'
nl -ba src/main/daemon/daemon-server.ts | sed -n '286,305p'
nl -ba src/main/daemon/daemon-request-router.ts | sed -n '145,160p'
nl -ba src/main/daemon/daemon-server-lifecycle.ts | sed -n '150,190p;245,275p'

Repository: stablyai/orca

Length of output: 7561


Re-list when the daemon provider changes during confirmation.

confirmNoDaemonSessionsRemain waits on the adapter array captured before the listing. A concurrent restart can replace the current adapter while a legacy adapter listing is still pending. The captured listings can then all return empty, while the replacement provider is never listed. The stop-and-quit path can therefore quit with sessions on the replacement daemon.

Suggested fix
--- "a/src/main/ipc/pty/pty-management.ts"
+++ "b/src/main/ipc/pty/pty-management.ts"
@@ -95,14 +95,19 @@
 
 // Kill all counts only its first listing; a session started since (CLI, automation) would outlive the quit.
 // Polls so the tab sweep's exact kills can settle.
 export async function confirmNoDaemonSessionsRemain(): Promise<boolean> {
-  const adapters = getDaemonAdapters()
   const pollDeadline = Date.now() + POLL_WINDOW_MS
   do {
+    const provider = getDaemonProvider()
+    const adapters = provider ? [...getAllDaemonAdapters(provider)] : []
     const { sessions, unlistedVersions } = await listSessionsOfEveryAdapter(adapters)
-    if (sessions.length === 0 && listedEveryGeneration(adapters, unlistedVersions)) {
+    if (
+      getDaemonProvider() === provider &&
+      sessions.length === 0 &&
+      listedEveryGeneration(adapters, unlistedVersions)
+    ) {
       return true
     }
     await sleep(POLL_INTERVAL_MS)
   } while (Date.now() < pollDeadline)
   return false
Committable suggestion

IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const adapters = getDaemonAdapters()
const pollDeadline = Date.now() + POLL_WINDOW_MS
do {
const { sessions, unlistedVersions } = await listSessionsOfEveryAdapter(adapters)
if (sessions.length === 0 && listedEveryGeneration(adapters, unlistedVersions)) {
const pollDeadline = Date.now() + POLL_WINDOW_MS
do {
const provider = getDaemonProvider()
const adapters = provider ? [...getAllDaemonAdapters(provider)] : []
const { sessions, unlistedVersions } = await listSessionsOfEveryAdapter(adapters)
if (
getDaemonProvider() === provider &&
sessions.length === 0 &&
listedEveryGeneration(adapters, unlistedVersions)
) {

Copilot AI balanced review requested due to automatic review settings October 27, 2026 10:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@nwparker
nwparker merged commit 2327180 into stablyai:main Oct 27, 2026
49 checks passed
@seijikohara
seijikohara deleted the feat/quit-and-stop-all-sessions branch October 27, 2026 13:23
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.

[Feature]: Quit and Stop All Sessions action in the app menu and menu bar/tray icon

3 participants