From 36cd8a33472487aa267f52679e18a1974767baf3 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 14 Jul 2026 00:15:34 -0700 Subject: [PATCH] fix(terminal): retire sessions when tabs close (#8628) * fix(terminal): retire sessions when tabs close * fix(terminal): close remaining session lifecycle gaps * fix(terminal): close review-discovered lifecycle gaps * fix(terminal): revalidate bulk session retirement * fix(terminal): harden retirement review edges * fix(agent): reverify restored pane authority * test: make terminal retirement gate portable on Windows * test: use POSIX join in Linux PATH assertion * test: keep simulated Linux PATH host-consistent --- config/reliability-gates.jsonc | 127 +++- docs/reference/terminal-session-lifecycle.md | 552 ++++++++++++++++++ package.json | 2 +- pty-exhaustion-agent-session-leak.md | 178 ++++++ .../agent-hooks/server-pane-authority.test.ts | 157 +++++ src/main/agent-hooks/server.ts | 216 ++++++- src/main/ipc/agent-hooks.test.ts | 80 +++ src/main/ipc/agent-hooks.ts | 53 +- src/main/ipc/agent-pane-authority-ipc.ts | 54 ++ .../agent-pane-authority-ownership.test.ts | 31 + .../ipc/agent-pane-authority-ownership.ts | 44 ++ src/main/ipc/agent-status-ipc-boundary.ts | 35 ++ src/main/ipc/pty-management.test.ts | 23 +- src/main/ipc/pty-management.ts | 25 +- src/main/ipc/pty.test.ts | 36 +- src/main/ipc/pty.ts | 9 +- src/main/ipc/register-core-handlers.test.ts | 4 +- src/main/ipc/register-core-handlers.ts | 3 +- src/main/persistence.test.ts | 38 ++ src/main/persistence.ts | 41 +- src/preload/api-types.ts | 14 +- src/preload/index.ts | 10 + src/renderer/src/components/Terminal.tsx | 36 +- .../FloatingTerminalPanel.test.tsx | 2 +- .../FloatingTerminalPanel.tsx | 2 +- .../OnboardingInlineCommandTerminal.tsx | 2 +- .../shared/kill-all-terminal-surfaces.test.ts | 238 +++++++- .../shared/kill-all-terminal-surfaces.ts | 197 +++++-- .../useTabGroupWorkspaceModel.focus.test.ts | 69 ++- .../tab-group/useTabGroupWorkspaceModel.ts | 8 +- .../TerminalPaneOverlayLayer.tsx | 5 +- .../terminal-pane/pty-connection.test.ts | 2 + .../terminal-parked-watcher-registry.ts | 7 + .../use-terminal-pane-lifecycle.test.ts | 13 +- .../use-terminal-pane-lifecycle.ts | 24 +- .../close-local-terminal-tab-state.ts | 45 ++ .../terminal/terminal-close-target.ts | 66 +++ .../terminal-tab-actions-kill-all.test.ts | 5 +- .../terminal/terminal-tab-actions.test.ts | 32 +- .../terminal/terminal-tab-actions.ts | 145 +++-- src/renderer/src/hooks/useIpcEvents.ts | 27 +- .../lib/agent-background-draft-delivery.ts | 17 + .../launch-agent-background-session.test.ts | 89 ++- .../lib/launch-agent-background-session.ts | 57 +- ...unch-worktree-background-terminals.test.ts | 74 ++- .../launch-worktree-background-terminals.ts | 19 + .../lib/retire-unowned-background-terminal.ts | 38 ++ .../store/slices/agent-pane-authority.test.ts | 125 ++++ .../src/store/slices/agent-pane-authority.ts | 86 +++ src/renderer/src/store/slices/agent-status.ts | 219 ++++++- src/renderer/src/store/slices/tabs.ts | 11 +- ...erminal-pane-detach-agent-identity.test.ts | 33 +- ...erminal-retirement-teardown-reservation.ts | 33 ++ .../terminal-tab-retirement-store.test.ts | 259 ++++++++ .../slices/terminal-tab-retirement.test.ts | 270 +++++++++ .../store/slices/terminal-tab-retirement.ts | 221 +++++++ src/renderer/src/store/slices/terminals.ts | 181 ++++-- src/renderer/src/web/web-preload-api.ts | 6 +- src/shared/agent-hook-listener.ts | 41 ++ src/shared/types.ts | 3 + tests/e2e/helpers/orca-app.ts | 48 +- tests/e2e/helpers/orca-restart.ts | 79 ++- .../terminal-parked-close-retirement.spec.ts | 96 +++ 63 files changed, 4185 insertions(+), 477 deletions(-) create mode 100644 docs/reference/terminal-session-lifecycle.md create mode 100644 pty-exhaustion-agent-session-leak.md create mode 100644 src/main/agent-hooks/server-pane-authority.test.ts create mode 100644 src/main/ipc/agent-pane-authority-ipc.ts create mode 100644 src/main/ipc/agent-pane-authority-ownership.test.ts create mode 100644 src/main/ipc/agent-pane-authority-ownership.ts create mode 100644 src/main/ipc/agent-status-ipc-boundary.ts create mode 100644 src/renderer/src/components/terminal/close-local-terminal-tab-state.ts create mode 100644 src/renderer/src/components/terminal/terminal-close-target.ts create mode 100644 src/renderer/src/lib/agent-background-draft-delivery.ts create mode 100644 src/renderer/src/lib/retire-unowned-background-terminal.ts create mode 100644 src/renderer/src/store/slices/agent-pane-authority.test.ts create mode 100644 src/renderer/src/store/slices/agent-pane-authority.ts create mode 100644 src/renderer/src/store/slices/terminal-retirement-teardown-reservation.ts create mode 100644 src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts create mode 100644 src/renderer/src/store/slices/terminal-tab-retirement.test.ts create mode 100644 src/renderer/src/store/slices/terminal-tab-retirement.ts create mode 100644 tests/e2e/terminal-parked-close-retirement.spec.ts diff --git a/config/reliability-gates.jsonc b/config/reliability-gates.jsonc index 5307bc71292..b3a9d893dc0 100644 --- a/config/reliability-gates.jsonc +++ b/config/reliability-gates.jsonc @@ -590,7 +590,7 @@ "https://github.com/stablyai/orca/issues/8001" ], "invariant": "Every terminal surface confirmed in the invoking renderer is force-closed exactly once after daemon management settles, later-created surfaces and non-terminal tabs survive, and exact shutdown requests are limited to deduplicated current non-runtime PTY bindings of the confirmed surfaces.", - "oracle": "Snapshot terminal entity IDs before the first await; mutate ownership, active selection, bindings, and tab presence while daemon management is pending; then assert only the immutable targets disappear from both terminal stores, active targets close last with valid editor/browser/deactivated post-state, every captured exact PTY promise settles before callbacks, and no provider inventory sweep or late-tab kill occurs.", + "oracle": "Snapshot terminal entity IDs before the first await; mutate ownership, active selection, bindings, and tab presence while daemon management is pending and between bounded close batches; then assert only the immutable targets disappear from both terminal stores, active targets close last with valid editor/browser/deactivated post-state, every captured exact PTY promise settles before callbacks, and no provider inventory sweep or late-tab kill occurs.", "commands": [ "pnpm exec vitest run --config config/vitest.config.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts src/renderer/src/components/shared/useDaemonActions.test.tsx src/renderer/src/components/terminal/terminal-tab-actions-kill-all.test.ts src/main/ipc/pty-management.test.ts" ], @@ -609,7 +609,7 @@ "current exact PTY bindings are deduplicated, remote runtime IDs and stale/late bindings are excluded, and all per-PTY settlements finish before completion", "the production dependency path calls daemon management exactly once and never invokes listSessions for a post-kill sweep", "management rejection and per-close/provider failures do not stop remaining cleanup and produce bounded count/latency diagnostics", - "a real 100-tab Zustand fixture records 100 close attempts and exact kills, at least 100 writes, 49 event-loop yields, and no close batch over 50 ms" + "a real 100-tab Zustand fixture records 100 close attempts and exact kills, at least 100 writes, and 49 event-loop yields; ownership is revalidated at most once after each yield when the store changed" ] }, { @@ -658,11 +658,11 @@ }, "redGreenEvidence": { "status": "partial", - "evidence": "The initial 100-tab implementation exposed a 145.82 ms close batch and failed the 50 ms budget; two-close event-loop batching reduced the focused run below the budget. Saved CI artifacts and intentional-break evidence for the confirmation-boundary assertions are still needed." + "evidence": "The initial 100-tab implementation exposed an oversized close batch; two-close event-loop batching fixed the structural issue. The gate now uses deterministic batch instrumentation because full-suite CPU saturation made wall-clock assertions flaky; saved CI artifacts and intentional-break evidence are still needed." }, "performanceBudget": { "required": true, - "evidence": "The coordinator performs one existing management call, zero provider inventory sweeps, one close per present unique target, and at most one exact kill per unique current non-runtime PTY binding. A real 100-tab Zustand fixture asserts 100 close attempts, 100 exact kill calls, at least 100 store writes, 49 event-loop yields, and a maximum two-close batch at or below 50 ms." + "evidence": "The coordinator performs one management sweep, builds an initial live-owner index, and revalidates at most once after each two-close yield when the Zustand state changed (at most 49 replans for the 100-tab fixture). It closes each present unique target once and sends at most one exact kill per unique current non-runtime PTY not already settled by daemon management. The fixture asserts 100 close attempts, 100 local kill calls, at least 100 store writes, and 49 yields; planner-level tests prove each individual plan build scans terminal and unified ownership stores once. Production diagnostics report measured close-batch duration, while the deterministic gate makes no machine-load-sensitive latency claim." }, "promotionCriteria": [ "Run the focused gate for at least 100 consecutive passes or 14 days across required CI platforms.", @@ -676,7 +676,124 @@ "Runtime-host terminal close is best-effort because closeTerminalTab still discards the existing async host result and its close-intent lifetime is shorter than the possible RPC flow.", "Daemon adapter listing failures remain suppressed by the existing management API, so reported daemon counts are not authoritative verification of every process." ], - "demotionRule": "Keep experimental or demote to protection none if the gate flakes, permits a late-created tab or unrelated PTY to close, or exceeds the 50 ms maximum close-batch budget without a tracked product or harness bug." + "demotionRule": "Keep experimental or demote to protection none if the gate flakes, permits a late-created tab or unrelated PTY to close, duplicates provider shutdown, or performs more than one ownership replan per bounded yield." + }, + { + "id": "terminal-session.explicit-close-retirement", + "title": "Explicit terminal close retires parked PTYs and agent authority exactly once", + "maturity": "experimental", + "protection": "partial", + "owner": "terminal-runtime", + "layer": "main-preload-renderer-electron-contract", + "surfaces": [ + "terminal tab close", + "split pane close and detach", + "hidden terminal parking", + "agent resume authority" + ], + "platforms": [ + "macos", + "linux", + "windows" + ], + "providers": [ + "local", + "daemon", + "ssh", + "runtime" + ], + "coveredPlatforms": [ + "macos" + ], + "coveredProviders": [ + "local", + "daemon", + "ssh", + "runtime" + ], + "coverageNotes": "A live macOS Electron test proves exact local PTY disappearance after a parked-tab close. Deterministic unit/store tests cover daemon and SSH routing, ordinary runtime close ownership, unified-only hydration, split ownership, pane detach transfer, restart alias hydration, and late-hook suppression; live Linux, Windows, WSL, SSH, and remote-runtime process evidence remains pending.", + "motivatingLinks": [ + "https://github.com/stablyai/orca/pull/8628" + ], + "invariant": "Close permanently removes the owned provider session and resume authority even when no TerminalPane is mounted; detach and park preserve both; aliases prevent a detached agent's immutable physical pane key from being retired with its former tab.", + "oracle": "Capture the exact PTY before parking, prove it remains listed while the view is absent, close through the product state boundary, and poll the provider inventory until that exact ID disappears; unit tests assert canonical owner dedupe, one teardown owner, exact pane tombstones, chained detach transfer, and restart alias restoration.", + "commands": [ + "pnpm dlx node@24 ./node_modules/vitest/vitest.mjs run --config config/vitest.config.ts src/main/agent-hooks/server-pane-authority.test.ts src/main/ipc/agent-hooks.test.ts src/main/ipc/agent-pane-authority-ownership.test.ts src/main/ipc/pty-management.test.ts src/main/persistence.test.ts src/renderer/src/store/slices/agent-pane-authority.test.ts src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts src/renderer/src/store/slices/terminal-tab-retirement.test.ts src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts", + "pnpm run test:e2e -- tests/e2e/terminal-parked-close-retirement.spec.ts --workers=1" + ], + "testFiles": [ + "src/main/agent-hooks/server-pane-authority.test.ts", + "src/main/ipc/agent-hooks.test.ts", + "src/main/ipc/agent-pane-authority-ownership.test.ts", + "src/main/ipc/pty-management.test.ts", + "src/main/persistence.test.ts", + "src/renderer/src/store/slices/agent-pane-authority.test.ts", + "src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts", + "src/renderer/src/store/slices/terminal-tab-retirement.test.ts", + "src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts", + "src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts", + "tests/e2e/terminal-parked-close-retirement.spec.ts" + ], + "assertionRefs": [ + { + "file": "tests/e2e/terminal-parked-close-retirement.spec.ts", + "assertions": [ + "a long-lived exact PTY remains alive after its terminal view is parked", + "closing the parked tab removes the exact PTY from the provider inventory and the visible tab model" + ] + }, + { + "file": "src/main/ipc/agent-pane-authority-ownership.test.ts", + "assertions": [ + "pane authority transfer accepts only the PTY bound to the physical local pane or the canonical legacy/scoped runtime handle" + ] + }, + { + "file": "src/renderer/src/store/slices/agent-pane-authority.test.ts", + "assertions": [ + "exact pane retirement removes resume and launch authority while preserving siblings", + "chained detach keeps physical hooks and resume authority routed to the current owner until that owner closes" + ] + } + ], + "evidenceRuns": [ + { + "date": "2026-07-13", + "runner": "local", + "platform": "macos", + "command": "pnpm run test:e2e -- tests/e2e/terminal-parked-close-retirement.spec.ts --workers=1", + "result": "passed", + "durationSeconds": 40.3, + "summary": "A fresh E2E build launched an isolated Electron profile, parked a live terminal, closed it through closeTab, and observed its exact PTY disappear." + } + ], + "runtimeBudget": { + "p95Seconds": 60, + "scope": "fresh E2E build plus one isolated local Electron parked-close test" + }, + "flakeHistory": { + "status": "unknown", + "evidence": "The Electron gate passed three times locally, including the final review-fix head through the registered fresh-build command; CI and soak history are not yet available." + }, + "redGreenEvidence": { + "status": "partial", + "evidence": "The test exercises the original parked-view failure shape and passed with the retirement boundary; an archived intentional-break run is not yet attached." + }, + "performanceBudget": { + "required": false, + "evidence": "The close path is user-triggered and bounded by canonical live-owner indexing; kill-all scale remains covered by terminal-session.kill-all-surface-cleanup." + }, + "promotionCriteria": [ + "Accumulate 100 clean runs or 14 days on required CI platforms.", + "Add live Windows/ConPTY, Linux, WSL, SSH, and ordinary runtime process-absence evidence.", + "Add restart/no-resurrection and repeated park-close soak coverage." + ], + "knownGaps": [ + "The live Electron proof currently covers macOS local PTYs only.", + "Disconnected SSH relay death still requires reconnect-aware provider ownership.", + "Daemon owner leases and durable retry inventory remain follow-up hardening." + ], + "demotionRule": "Keep experimental or demote to protection none if exact PTY disappearance flakes, a sibling/detached pane is retired, or late hooks can recreate closed authority." }, { "id": "terminal-session.startup-cwd-missing-dir-recovery", diff --git a/docs/reference/terminal-session-lifecycle.md b/docs/reference/terminal-session-lifecycle.md new file mode 100644 index 00000000000..29161cd6fdf --- /dev/null +++ b/docs/reference/terminal-session-lifecycle.md @@ -0,0 +1,552 @@ +# Terminal Session Ownership and Teardown + +**Status:** Incident fix and pane-authority transfer implemented +**Date:** 2026-07-13 +**Incident:** [`../../pty-exhaustion-agent-session-leak.md`](../../pty-exhaustion-agent-session-leak.md) +**Related contracts:** [`terminal-model-view-contract.md`](./terminal-model-view-contract.md), +[`terminal-hidden-view-parking.md`](./terminal-hidden-view-parking.md) + +## Summary + +Terminal process lifetime must be owned by explicit product intent, never by +whether a React view happens to be mounted. + +Orca currently preserves a PTY when a terminal view detaches, which is correct +for view parking, tab moves, renderer reload, and warm reattach. It also relies +on `TerminalPane` unmount to destroy a PTY after a tab is closed. Hidden-view +parking invalidated that implicit invariant: a parked tab has no mounted pane, +so closing it removes renderer state and observers without terminating the PTY. +Agent resume records survive the same close and later interpret the missing pane +as failed recovery, launching the deliberately closed session again. + +This design establishes one authoritative lifecycle contract: + +- **Close tab or pane:** permanently retire the surface, terminate every PTY it + owns, and revoke every resume record for its panes. +- **Detach or park:** remove only the view connection; preserve the PTY and its + durable identity. +- **Sleep:** intentionally checkpoint resumable agents, terminate the PTYs, and + preserve only the checkpoint needed for wake. +- **Quit or reload:** follow the configured persistence policy; warm detach is + allowed, but it is not a tab close. + +The immediate implementation makes the existing terminal-tab state close the +authoritative compatibility boundary for explicit retirement while adding an +explicit natural-exit reason for callers that are only reconciling an already +dead PTY. A follow-up separates the command and reducer APIs once all direct +store callers have migrated. + +## Implementation status + +The incident patch ships the retirement planner, provider-aware close routing, +unified-only retirement, parked watcher/candidate disposal, resume-authority +revocation, explicit natural-exit handling, late-binding rejection, and direct +background-launch ownership checks. It also adds exact pane retirement and a +persisted physical-to-owner pane-key alias for detach, plus a live Electron +parked-close gate. Focused tests cover local, SSH, split, parked, shared, +paired-host, ordinary runtime, pane-transfer, and late-spawn paths. + +Origin-aware defensive expiry remains a follow-up. Immediate deletion on +explicit close fixes the incident without applying a wall-clock policy that +could invalidate intentional long-lived `worktree-sleep` checkpoints. + +## Goals + +1. Closing a terminal tab submits retirement for all exclusively owned local, + daemon, WSL, SSH, or runtime PTYs, including every split pane, whether its + views are mounted or parked. +2. Closing a terminal pane terminates only that pane's PTY; detaching a pane to + another tab preserves its PTY, hook identity, and resume authority under the + new owning pane. +3. Closing a tab permanently removes its agent resume authority. App restart, + worktree activation, mobile wake, and periodic capture must not resurrect it. +4. Parking, tab-group moves, renderer reload, and warm reattach keep their + existing process-preservation behavior. +5. Remote-runtime and web-session surfaces are terminated by their owning host, + not by sending a host-scoped ID to the local PTY provider. +6. Teardown is idempotent and safe under duplicate close requests, late PTY + exits, late agent-hook events, in-flight spawns, and provider disconnects. +7. A future bookkeeping regression cannot grow without bound: unowned defensive + resume records have an origin-aware expiry, and provider-owned detached + sessions gain a bounded orphan policy in the daemon-lease phase. + +## Non-goals + +- Changing terminal output, snapshot, replay, query-response, or hidden-delivery + behavior. +- Killing processes that deliberately daemonize away from the terminal process + group. This contract covers the PTY and its attached foreground process tree. +- Changing agent-provider resume commands or permission flags. +- Making a UI close wait for a remote process to exit before the tab disappears. +- Replacing worktree sleep with tab close. Sleep remains resumable by design. +- Claiming that a disconnected SSH relay process is already dead when main can + only tombstone its app-scoped ID. Durable relay-side kill-on-reconnect is a + follow-up. + +## Terminology + +- **Surface:** a terminal tab or a pane within a split terminal tab. +- **View:** a renderer xterm or pane-less parked watcher observing a surface. +- **Session:** the provider-owned PTY identified by a PTY ID. +- **Resume authority:** a `sleepingAgentSessionsByPaneKey` record that permits + Orca to launch an agent-provider resume command. +- **Retire:** permanently close a surface and revoke its process and resume + ownership. +- **Detach:** disconnect a view while preserving the session for reattachment. + +## Open-source prior art + +The lifecycle distinction is established in mature terminal implementations: + +- VS Code's terminal instance disposal calls process-manager disposal, and the + process manager marks the process `KilledByUser` before shutdown. Its separate + `detachFromProcess` path deliberately clears the client reference without + shutting down the process. It also shuts down a process whose asynchronous + creation completes after the manager was disposed. See + [`terminalInstance.ts`](https://github.com/microsoft/vscode/blob/3f5c62a95ddb886424da463b41ac3ac5e45aa04f/src/vs/workbench/contrib/terminal/browser/terminalInstance.ts#L1287-L1325) + and + [`terminalProcessManager.ts`](https://github.com/microsoft/vscode/blob/3f5c62a95ddb886424da463b41ac3ac5e45aa04f/src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts#L207-L239). +- tmux exposes the same contract as separate commands: `detach-client` removes + a client while leaving the session intact, whereas `kill-session` destroys + the session and its windows. See + [`tmux.1`](https://github.com/tmux/tmux/blob/42b3ea0d7411acb4cd0357a3c2829d986b455918/tmux.1#L1177-L1218). + +Orca needs the same semantic split, extended across local, WSL, SSH, persistent +daemon, and remote-runtime ownership. The important precedent is the explicit +intent boundary, not a particular framework or process API. + +## Non-negotiable invariants + +1. View lifetime and process lifetime are independent. Mount and unmount may + attach or detach observers; they do not decide whether a session should live. +2. Every live PTY has an owning surface or an explicit bounded detached state. +3. A user close is terminal: once accepted, no status event, session snapshot, + or runtime replay may recreate the closed surface or resume its agent. +4. A tab's retirement candidates are the union of its live index, tab row, + persisted split layout, last-known relay ID, deferred SSH ID, and pending + reconnect ID. A candidate is killable only when no other live surface claims + it. +5. Provider routing follows the execution owner: + - local/daemon/WSL and app-scoped SSH IDs use `pty.kill`; + - `remote:` runtime mirror IDs are never passed to the local PTY provider; + - paired web tabs are closed through the host session RPC. +6. Teardown requests are idempotent. Repeated kill, exit-after-kill, and + close-after-exit must converge on the same empty state. +7. Resume-record validity is origin-aware. Intentional `worktree-sleep` records + are not expired by agent-status freshness; unowned defensive `live` and + `quit` records use a documented decision-time horizon. +8. Natural process exit and explicit user retirement are distinct intents. + Reconciliation after an exit must not silently adopt destructive user-close + semantics. + +## Lifecycle matrix + +| Intent | View | PTY | Resume record | Persistent tab state | +| --------------------- | ------------------------------------------ | ---------------------------------- | --------------------------------------------------- | -------------------- | +| Switch tabs/worktrees | hidden or parked | keep | keep/update | keep | +| Move pane/tab group | detach, then remount | keep | keep | move | +| Renderer reload | detach | keep when warm persistence applies | keep | keep | +| App quit | detach or terminate per persistence policy | policy-dependent | capture live agents | keep | +| Sleep worktree | unmount | terminate | capture intentional sleep checkpoint | keep identifiers | +| Close split pane | destroy | terminate that pane | delete that pane record | remove pane | +| Close terminal tab | destroy or already absent | terminate all tab PTYs | delete all tab records | remove tab | +| Remove worktree | destroy | terminate all worktree PTYs | delete all worktree records | remove worktree | +| PTY exits naturally | disconnect | already dead | clear or retain completed evidence per agent policy | reconcile surface | + +## Current failure + +The current close path deletes `ptyIdsByTabId[tabId]` and the tab layout, then +relies on a mounted `TerminalPane` cleanup to call `transport.destroy()`. A +parked tab has already run the detach branch and has no mounted cleanup left. +`disposeParkedTabWatchers` only unregisters observers. The provider session is +therefore alive after Orca discards its last renderer-side owner. + +The same state close drops live agent status but not +`sleepingAgentSessionsByPaneKey`. Worktree activation intentionally fresh-resumes +records that no preserved pane can own. A deliberately closed pane therefore +looks like a failed restore. The existing age check compares `capturedAt` with +`updatedAt`; that detects a status that was stale at capture time but never +expires a record as wall time advances. + +## Proposed architecture + +### 1. Authoritative tab retirement plan + +Before deleting any tab-scoped state, build a `TerminalTabRetirementPlan` from +one store snapshot: + +```ts +type TerminalTabRetirementPlan = { + tabId: string + worktreeId: string | null + ptyIds: string[] + localOrSshPtyIds: string[] + runtimeTerminals: Array<{ environmentId: string; handle: string }> + sharedPtyIds: string[] + paneKeys: string[] +} +``` + +`ptyIds` is the deduplicated union of: + +- `ptyIdsByTabId[tabId]`; +- the tab row's legacy `ptyId`; +- every value in `terminalLayoutsByTabId[tabId].ptyIdsByLeafId`; +- `lastKnownRelayPtyIdByTabId[tabId]`; +- `deferredSshSessionIdsByTabId[tabId]`; +- `pendingReconnectPtyIdByTabId[tabId]`. + +This snapshot must be built before any of those maps are pruned. PTY IDs are +classified structurally with `parseRemoteRuntimePtyId`, not by assuming a host +or platform from the current active workspace. Before shutdown, the planner +subtracts IDs referenced by another tab row, live index, split layout, or relay +and reconnect map. A partially completed pane move must not let closing the +source tab kill the target tab's session. + +The immediate compatibility boundary is `closeTab`, because several production +paths still call the store action directly. Its options carry an explicit +`reason: 'user' | 'pty-exit' | 'cleanup'`; `user` and `cleanup` retire, while +`pty-exit` reconciles an already-dead session without killing siblings or +revoking crash-recovery policy by accident. Making only the rendered tab-bar +action destructive would leave background launch cleanup, floating terminal, +onboarding, runtime notifications, and future direct callers vulnerable. + +### 2. Teardown ordering + +A local tab retirement executes in this order: + +1. Snapshot the retirement plan and its execution-owner classifications. +2. Mark the tab recently closed so late agent-hook events cannot reintroduce + status or resume authority. +3. Dispose the whole tab's parked registry entry and captured candidates, and + silence teardown side-effect handlers while preserving deterministic exit + observation. +4. Submit `pty.kill` for exclusive local or SSH IDs and `terminal.close` for + exclusive ordinary runtime handles. A paired host-session close remains + owned by `session.tabs.close` and is not duplicated locally. +5. Atomically remove the tab, layout/binding maps, agent status, and resume + records from renderer state. +6. Let subsequent React unmount cleanup run idempotently. It is no longer the + process-lifetime authority. + +Steps 3 and 4 are issued before the state loses its IDs. The UI state mutation +remains synchronous and does not await provider exit. Completion is observed +with `Promise.allSettled` so rejections cannot become unhandled promises. The +current Phase 1 fallback for a rejected retirement is provider `listSessions` +plus Resource Manager orphan cleanup; durable retry inventory belongs to the +main-owned retirement phase. + +Remote-runtime mirrors are different: their `remote:` IDs are excluded from +local `pty.kill`. Ordinary runtime terminals retire through `terminal.close`. +Paired host-session tabs retire through `session.tabs.close`, which owns the +entire host tab graph. The local mirror may be pruned optimistically only after +the host close intent is recorded, as it is today. + +### 3. Resume-authority revocation + +Tab retirement removes every sleeping record whose: + +- map key begins with `${tabId}:`; or +- record has `tabId === tabId`. + +The explicit `record.tabId` check covers migrated or legacy keys whose key no +longer encodes the current tab identity. Sibling tabs and other worktrees retain +their records by reference when unchanged. + +The existing recently-closed tab registry remains the short-lived race guard +for hook events already in flight. It must prevent those events from creating a +new `origin: 'live'` resume record after retirement. Tests must cover both event +orders: close then late status, and status queued in the same turn as close. + +Periodic capture iterates only current live status entries. Once close removes +the live entry and resume record, the periodic pass has nothing to persist. + +### 4. Origin-aware defensive record expiry + +An unowned defensive `live` or `quit` record may expire when: + +```ts +Date.now() - record.capturedAt > DEFENSIVE_AGENT_RESUME_MAX_AGE_MS +``` + +Intentional `worktree-sleep` records are exempt from the agent-status freshness +window. Originless legacy behavior remains unchanged until its migration policy +is explicitly chosen. The same validator must run before desktop activation and +mobile/background wake so one entry point cannot bypass it. + +Expiry is defense in depth, not the primary close mechanism. A correctly closed +record is deleted immediately; it is not retained for 30 minutes. + +### 5. Pane close remains pane-scoped + +`PaneManager.closePane` already distinguishes `reason: 'close'` from +`reason: 'detach'`. Preserve that distinction with explicit pane authority: + +- `close` must clear the exact pane's resume authority, add a pane-scoped late + event tombstone, and destroy the pane transport; +- `detach` must atomically transfer or alias resume ownership because the agent + process retains its immutable source `ORCA_PANE_KEY`. + +The last-pane path routes to terminal-tab retirement so it receives the same +parked, split, SSH, and resume cleanup behavior as a tab-bar close. + +### 6. Provider-side safety net + +Client correctness is necessary but not sufficient for a daemon deliberately +designed to outlive renderer and app processes. Add a follow-up provider-owned +orphan policy: + +- track attachments/ownership leases rather than treating `detach` as a no-op; +- distinguish warm-reattach grace from ownerless retirement; +- reap sessions that have no owner after a bounded grace period; +- never idle-reap a session merely because it produces no output; +- scope leases to native, WSL, SSH provider, relay connection, or runtime host; +- expose orphan count and oldest orphan age in Resource Manager diagnostics. + +This hardening is not required to land the immediate close fix, because a lease +protocol changes daemon compatibility and needs its own migration. It is the +defense against a future client bookkeeping regression. + +## Provider behavior + +| Provider/session kind | Close operation | Notes | +| ----------------------- | ----------------------------------- | ---------------------------------------------------------------------------------------------- | +| Local native | `pty.kill(id)` | Main routes to the owning local/daemon provider. | +| Persistent daemon | `pty.kill(id)` | Explicit close overrides warm-reattach persistence. | +| Windows ConPTY | `pty.kill(id)` | Provider shutdown owns Windows process-tree semantics. | +| WSL | `pty.kill(id)` | Route by PTY ownership; do not infer from path separators. | +| SSH | `pty.kill(appScopedId)` | Main resolves the connection-specific SSH provider. | +| Disconnected SSH | `pty.kill(appScopedId)` | Tombstones locally and prevents fallback; relay-side death requires reconnect-aware follow-up. | +| Ordinary remote runtime | host `terminal.close` | Classify environment and handle from the `remote:` ID. | +| Paired web host tab | host `session.tabs.close` | Host owns the complete tab and pane graph. | +| Web client local mirror | optimistic prune after close intent | Host snapshot confirms final removal. | + +## Failure and race handling + +### Duplicate close + +The first close snapshots and retires the tab. Later closes find no target and +no-op. Provider shutdown is idempotent for an already exited session. + +### PTY exit races close + +Observers are unregistered before shutdown. If an exit event was already +queued, the recently-closed tab guard and missing tab make it a no-op. It must +not recreate a tab, completion row, resume record, or notification. + +### Spawn completes after close + +The transport already kills a PTY when its spawn resolves after `destroyed` is +set, but the two direct background launchers do not use that transport. Each +direct launcher must revalidate tab ownership immediately after `pty.spawn` or +`terminal.create` resolves and before writing any binding, layout, eager buffer, +subscription, or mount request. A retired local or SSH result is killed; a +retired runtime result is closed through `terminal.close`. + +### Provider shutdown fails + +The tab remains closed; reappearing would violate user intent. Report the +provider class through structured diagnostics and leave the session visible in +provider inventory so existing Resource Manager orphan cleanup can retry. A +durable pending-retirement inventory is a later main-process API and is not +claimed by Phase 1. Do not restore resume authority after a failed terminal +kill. + +### Remote host is unavailable + +Keep the local close intent until either a host snapshot confirms removal or the +existing intent TTL expires. If it expires, the host remains authoritative and +may republish the tab; log the failed close. Never pretend a local mirror prune +terminated the host PTY. + +### App exits during close + +Submit shutdown IPC before discarding the only local PTY identifiers. The main +process/provider owns completion after accepting the request. A future batched +shutdown IPC may return per-ID acceptance, but UI close does not wait for it. + +## Implementation plan + +### Phase 1: correctness patch (implemented) + +1. Add a small store-independent terminal-tab retirement planner that collects + all six ID sources, classifies execution owners, and excludes IDs shared by + another live surface. +2. Add an explicit close reason, update natural-exit callers, and make explicit + `closeTab` retirement dispose the whole parked-tab registry, submit local, + SSH, or runtime shutdown, atomically clear all reconnect maps, and remove + tab-owned resume records. +3. Add post-await owner checks to both direct background launchers so a late + local, SSH, or runtime spawn is terminated before any renderer binding. +4. Defer decision-time expiry until unowned defensive `live` or `quit` records + can be distinguished without changing intentional `worktree-sleep` or legacy + recovery behavior. +5. Add focused planner, store, launcher-race, and resume-policy tests. Keep one + local Electron parked-close proof as an RC release gate. + +### Phase 2: remaining API cleanup + +1. Rename the state-only reducer primitive so `closeTab` cannot ambiguously mean + either UI cleanup or lifecycle teardown. +2. Route all user and runtime close call sites through one domain command. +3. Keep explicit `detach` and `sleep` APIs; do not encode them as close options. +4. Add a development assertion when tab state is removed with provider PTY IDs + but no retirement plan. + +Exact pane retirement, persisted detach authority transfer, and unified-only +terminal routing landed with Phase 1 because exact-head review found they were +required to avoid introducing adjacent close regressions. + +### Phase 3: daemon ownership leases + +1. Define versioned attach/detach/retire lease messages. +2. Preserve warm reattach across renderer/app restarts within a bounded grace. +3. Reap ownerless sessions and expose pressure telemetry. +4. Add compatibility tests across the supported daemon protocol versions and + native/WSL/SSH host isolation. + +## Test plan + +### Unit/store tests + +- Closing an active single-pane tab submits exactly its PTY for shutdown and + removes all tab state. +- Closing a parked tab with no mounted transport still submits its PTY. +- Closing a split tab submits every unique PTY found across tab row, index, and + layout plus relay/deferred/reconnect maps, including IDs present in only one + source. +- Duplicate IDs are submitted once by the authoritative retirement plan. +- An ID referenced by another live tab is not killed when the source tab closes. +- `remote:` IDs are excluded from local PTY shutdown. +- App-scoped SSH IDs are included and are not mistaken for remote runtime IDs. +- All sleeping records for the tab are removed by key prefix or `record.tabId`; + sibling records retain identity. +- Periodic capture after close does not recreate the record. +- Late agent status after close cannot recreate live status or resume authority. +- Closing twice and close-after-exit are no-ops after the first retirement. +- A spawn resolving after close kills the newly returned PTY. +- A defensive live/quit record at the expiry boundary is retained; one + millisecond past it is cleared and never launched, while an older intentional + worktree-sleep record still resumes. + +### Renderer integration tests + +- Mounted tab close still destroys its xterm and terminates the PTY. +- Parked watcher disposal occurs before or with PTY shutdown and emits no final + completion/bell notification. +- Closing one split pane preserves the sibling; closing the last pane retires + the tab. +- Pane-to-tab detach preserves the PTY and resume record. +- Pinned-tab cancellation performs no shutdown; confirmed close does. +- Bulk close (`close others`, `close right`, kill-all) uses the same semantics. + +### Provider/main tests + +- Local, daemon, WSL, ConPTY, and SSH ownership route explicit close to the + correct provider. +- Disconnected SSH close tombstones the app-scoped ID and never falls through to + a local provider. +- Repeated `pty.kill` for the same ID is benign. +- Remote-runtime IDs cannot reach the local kill handler from renderer close. + +### Electron end-to-end regression + +1. Create a worktree terminal and start a deterministic long-lived child. +2. Switch worktrees and wait until the source worktree is cold-parked. +3. Close the parked tab. +4. Assert its exact PTY disappears from `pty:listSessions` and the child exits. +5. Restart/reload Orca and activate the worktree. +6. Assert no terminal or agent resume command is recreated for the closed pane. +7. Repeat with two split panes and with an SSH fixture where CI supports it. + +### Soak and pressure gate + +Run repeated create, park, close, reload, and reopen cycles. App-owned PTY count +must return to baseline after each cycle and remain bounded over the run. Record +the peak and final provider session count in CI artifacts. + +## Observability + +Emit a synchronous scheduled summary and an asynchronous completion summary: + +- tab/worktree identifiers in hashed or existing diagnostic form; +- number of PTY IDs discovered by each source; +- provider classification counts; +- submitted count, then fulfilled/rejected counts after `Promise.allSettled`; +- number of resume records removed; +- whether the tab was parked or mounted when retired. + +After the daemon-lease phase, Resource Manager should distinguish: + +- attached sessions; +- warm-detached sessions eligible for reattach; +- ownerless/orphan sessions; +- pending/failed explicit retirement. + +Alert locally before app-owned PTYs approach platform pressure. The warning is a +defense, not a substitute for lifecycle correctness. + +## Rollout + +1. Land the correctness patch without a feature flag. User close semantics are + restorative behavior, not an experiment. +2. Keep hidden-view parking enabled; disabling it masks the ownership bug and + forfeits its memory benefit. +3. Run focused unit/integration tests plus the live parked-close Electron gate. +4. Cut an RC and soak create/close/restart cycles while monitoring provider and + OS PTY counts. +5. Ship daemon leases separately behind protocol compatibility and telemetry. + +## Alternatives rejected + +### Disable hidden-view parking + +This restores the old accidental unmount behavior at significant memory cost +and leaves every other state-only close path fragile. It does not repair resume +records or daemon ownership. + +### Kill only from `TerminalPane` cleanup + +There is intentionally no pane during parking and some headless/runtime flows. +View cleanup cannot be process authority. + +### Clear only resume records + +This prevents resurrection but still leaks the live PTY until app/daemon exit. + +### Kill only the tab row's `ptyId` + +Split panes and partially reconciled layouts can own multiple IDs. The full +union is required. + +### Rely only on a daemon idle timeout + +Interactive agents can be legitimately idle for hours. Reaping by output +idleness loses user work; ownership, not activity, is the safe signal. + +### Await every provider exit before hiding the tab + +Remote and disconnected providers can take seconds or fail. UI retirement must +be immediate once the user confirms; provider completion is asynchronous and +observable. + +## Phase 1 acceptance criteria + +- A confirmed terminal-tab close submits retirement for every exclusively owned + session Orca can currently reach and reports rejected requests without an + unhandled promise. +- A closed agent session is never resumed by restart, activation, or mobile wake. +- Parking, move/detach, sleep/wake, and warm app reattach retain their documented + behavior. +- Split, local, SSH, and remote-runtime ownership are covered by deterministic + tests; Windows/WSL process-tree behavior remains a live-platform follow-up. +- PTY counts remain bounded across the soak scenario. +- No process-lifetime decision depends on React component mount state. + +## Full-contract follow-ups + +- Durable main-owned retirement intent and bounded retry inventory. +- Disconnected SSH relay kill-on-reconnect. +- Serializable daemon attachment leases with protocol-version fallback. +- Positive ownership validation for late hook events beyond the bounded recent- + close registry. diff --git a/package.json b/package.json index 9f161f101fb..2d0392a97d2 100644 --- a/package.json +++ b/package.json @@ -72,7 +72,7 @@ "build:mac": "pnpm run build:desktop && pnpm run build:computer-macos && pnpm run build:notification-status-macos && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --mac", "build:mac:release": "node config/scripts/verify-macos-release-env.mjs && ORCA_MAC_RELEASE=1 pnpm run build:desktop && ORCA_MAC_RELEASE=1 pnpm run build:computer-macos && ORCA_MAC_RELEASE=1 pnpm run build:notification-status-macos && pnpm run ensure:electron-runtime && ORCA_MAC_RELEASE=1 electron-builder --config config/electron-builder.config.cjs --mac", "build:linux": "pnpm run build:desktop && pnpm run ensure:electron-runtime && electron-builder --config config/electron-builder.config.cjs --linux AppImage deb", - "test:e2e": "pnpm run ensure:electron-runtime && npx playwright test --config tests/playwright.config.ts --project electron-headless", + "test:e2e": "pnpm run ensure:electron-runtime && npx playwright test --config tests/playwright.config.ts --project=electron-headless", "test:e2e:floating-mobile-emulator": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/floating-mobile-emulator-tab.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=1", "test:e2e:terminal-rendering-golden": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/terminal-raw-emoji-table-scroll-restore.spec.ts --grep @terminal-rendering-golden --config tests/playwright.config.ts --project electron-headless --workers=1", "test:e2e:terminal-rendering-release-evidence": "pnpm run ensure:electron-runtime && npx playwright test tests/e2e/terminal-opencode-emoji-table-rendering.spec.ts tests/e2e/terminal-long-table-scroll-restore.spec.ts --config tests/playwright.config.ts --project electron-headless --workers=2", diff --git a/pty-exhaustion-agent-session-leak.md b/pty-exhaustion-agent-session-leak.md new file mode 100644 index 00000000000..64cbb3d93ce --- /dev/null +++ b/pty-exhaustion-agent-session-leak.md @@ -0,0 +1,178 @@ +# Design Doc: PTY Exhaustion from Leaked Agent Sessions + +**Date:** 2026-07-13 +**Status:** Root cause identified; fixes proposed +**Severity:** P0 — machine-wide outage. Once the pty cap is hit, _nothing_ on the host can open a terminal (Orca, Ghostty, IDEs, `ssh`, agent spawns all fail with "cannot allocate pty"). +**Source refs:** file:line references below are against the main checkout at `~/orca/orca` as of 2026-07-13. + +--- + +## 1. Summary + +An Orca desktop instance exhausted the macOS pseudo-terminal limit (`kern.tty.ptmx_max = 511`). Investigation on the live machine found **526 allocated ptys**, of which **~476 belonged to three `Orca Helper` processes** — hundreds of idle `login → zsh → codex resume ` chains, some alive for almost 4 days, all at 0% CPU. Sessions the user had explicitly closed in the UI were still running, and closed sessions kept _coming back_ in bulk waves after restarts. + +The root cause is **two independent defects that compound into a self-perpetuating leak**: + +- **Bug A (teardown):** Closing a session tab never kills the underlying pty. Tab close is pure renderer-state cleanup; the actual `pty.kill` only happens as a side effect of a mounted `TerminalPane` unmounting. For "cold-parked" tabs in background worktrees — whose panes are unmounted by design — closing the tab silently orphans the live process chain. +- **Bug B (resume):** Closing a tab never removes the session's record from the persisted resume list (`sleepingAgentSessionsByPaneKey`), and the record staleness check is a no-op (it compares `capturedAt - updatedAt`, which is always ≈ 0, instead of `now - updatedAt`). Every workspace open / app restart therefore bulk-launches `codex resume` for the entire orphaned backlog — and a periodic capture timer re-persists all of them, so the backlog only grows. + +Every restart makes it worse: Bug B resurrects the dead-but-remembered sessions (each burning a fresh pty), Bug A guarantees closing them doesn't actually free anything, and the capture loop re-persists the ever-larger set. + +--- + +## 2. Observed incident (live-machine evidence) + +Measurements taken 2026-07-13 on the affected desktop: + +| Metric | Value | +| ------------------------------------------------- | ----------------------------------------------------------------------- | +| Allocated ptys (`/dev/ttys*`) | 526 (cap: `kern.tty.ptmx_max = 511`) | +| `login` + `zsh` session pairs | 506 | +| Sessions parented to installed `Orca.app` helpers | 476 (helper pids 53960: **377**, 31662: 58, 1510: 41) | +| Sessions from dev-build Orca instances | ~25 | +| `codex` agent processes | ~280 (~270 **unique** session IDs; a handful resumed 2–3× concurrently) | +| `claude` agent processes | ~38 | +| Processes using >1% CPU | 7 of 763 — everything else fully idle | + +Key observations: + +- **`Orca Helper` pid 53960 alone held 376 open ptmx master fds.** Every leaked chain was still fully parented: `Orca Helper → /usr/bin/login → zsh → node → codex`. Nothing was orphaned to launchd — the app-side master fd was simply never closed. +- **Working directories were overwhelmingly finished PR-review worktrees** (`review-p1-pr-7511-issue-7026` alone had 79 codex processes; `review-p0-pr-8370-issue-8212` had 76; `review-p0-pr-8279-issue-8260` had 67). +- **Start times cluster in same-second waves** — batches on Fri Jul 10 (~15:41–18:14) and large bursts on Sun Jul 12 (14:03, 14:27, 14:36–14:40, 15:09). These are bulk resumes on workspace open / app restart, not user-opened sessions. +- **The user had closed these tabs.** The tabs were gone from the UI; the processes survived (Bug A) and were then re-resumed in later waves (Bug B). +- Nearly all resumed with `codex --dangerously-bypass-approvals-and-sandbox resume ` — i.e., they idle at an interactive prompt and never exit on their own. + +--- + +## 3. Background: pty ownership architecture + +- Terminal sessions are spawned in an Electron utility/helper process which holds the **pty master** (hence the ptmx fds on `Orca Helper`). +- A **daemon** hosts sessions so they survive app restarts for warm reattach: it is forked `detached: true` + `unref()` and deliberately outlives the app (`daemon/daemon-init.ts:306-319`; comment at `:741` — "sessions stay alive for warm reattach"). +- Background (non-active) worktrees are **"cold-parked"**: their `TerminalPane` React components unmount, the transport calls `detach()` (`pty-transport.ts:929` — "keep the PTY exit observer alive"), and pane-less byte watchers observe the still-running pty. +- Agent sessions are additionally tracked in a persisted map, `sleepingAgentSessionsByPaneKey` (`src/renderer/src/store/slices/agent-status.ts:121`), so they can be resumed (`codex resume `) after sleep/quit. + +Keep-alive-on-detach is **intentional**. The defects are in what is supposed to end a session's life. + +--- + +## 4. Bug A: closing a tab never kills the pty + +### The close path contains no kill + +`closeTerminalTab()` (`terminal-tab-actions.ts:152`) → `closeLocalTerminalTabState` → `state.closeTab()` (`store/slices/terminals.ts:1192-1381`). `closeTab` scrubs ~20 per-tab maps — including `delete ptyIdsByTabId[tabId]` (`terminals.ts:1217-1218`) — but issues **no `pty.kill`, no shutdown, no IPC**. It just _forgets the pty id_. + +### The only kill is a React-unmount side effect + +The pty dies only when a _mounted_ `TerminalPane` unmounts with the tab gone: `shouldDetachPaneTransportOnUnmount()` (`use-terminal-pane-lifecycle.ts:456`) returns false → unmount cleanup (`use-terminal-pane-lifecycle.ts:1725-1826`, kill branch at `:1807`) → `transport.destroy()` (`pty-transport.ts:1031`) → `window.api.pty.kill(id)` → main `pty:kill` (`ipc/pty.ts:4953`) → `provider.shutdown(id, {immediate: true})`. + +This works for the **active** worktree. But for a **cold-parked** tab there is no mounted pane; closing it runs `syncParkedTerminalTabWatchers` → `disposeParkedTabWatchers` (`terminal-parked-watcher-registry.ts:53-63`), which stops the watchers and **never kills the pty**. The `login → zsh → codex` chain is orphaned with nothing pointing at it. **This is the zombie source.** + +### Nothing else ever reaps it + +Every existing reaper is scoped to an event _other than_ tab close: + +| Mechanism | Why it doesn't help | +| --------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Daemon `detach` handler | Logging no-op — "full implementation would track tokens" (`daemon/daemon-server.ts:508-512`). No idle timeout exists anywhere. | +| `reapSession` (`daemon/terminal-host.ts:215`) | Fires only on the child's natural exit or explicit kill; an idle interactive `codex` never exits. | +| App-quit `killAllPty()` (`ipc/pty.ts:5191`), reload `killOrphanedPtys` (`ipc/pty.ts:2851`) | Both guarded by `if (localProvider instanceof LocalPtyProvider)` — false for the daemon adapter, so daemon-hosted sessions are excluded _by design_ (warm reattach). | +| Worktree removal (`runtime/worktree-teardown.ts:39`, `killAllProcessesForWorktree`) | Works, but only on full worktree delete. | +| Sleep flow (`sidebar/sleep-worktree-flow.ts:148` → `shutdownWorktreeTerminals`, `terminals.ts:2331`, kill at `:2665`) | Works, but only when the user explicitly sleeps the worktree. | + +### The codebase already knows close ≠ kill + +`kill-all-terminal-surfaces.ts` performs daemon `killAll()`, then `closeTerminalTab({force: true})` per tab, **then an explicit `window.api.pty.kill(ptyId)` loop over `ptyIdsByTabId`** (`:140-147`, `:188-190`) — direct evidence that tab close alone is known not to kill. There is even a manual mop-up UI: **Manage Sessions → "Kill orphan terminals"** for sessions "that have no tab in this Orca instance" (`status-bar/ResourceUsageStatusSegment.tsx:~1114-1155`) — a hand-operated workaround for exactly this leak. + +--- + +## 5. Bug B: closed sessions are resurrected in bulk forever + +### The resume list never learns about tab close + +Agent sessions are recorded in `sleepingAgentSessionsByPaneKey` (`agent-status.ts:121`; record shape in `src/shared/agent-session-resume.ts:38-59`, with `state`, `capturedAt`, `updatedAt`, and `origin: 'worktree-sleep' | 'quit' | 'live'`). Records are written: + +- **Live:** on every non-done agent-status update, an `origin: 'live'` record is captured (`agent-status.ts:1492-1544`). +- **Quit + periodic:** `captureAllSleepingAgentSessions` writes an `origin: 'quit'` record for _every currently-live pane_ (`agent-status.ts:2312-2350`), called on quit (`App.tsx:1367`) **and on a periodic interval** (`App.tsx:1394`). + +`closeTab` (`terminals.ts:1192-1381`) calls `dropAgentStatusByTabPrefix` (`terminals.ts:1894-2010`), which removes the agent-_status_ entry but **never touches `sleepingAgentSessionsByPaneKey`**. There is no concept of "user closed this tab" in the resume store. The orphaned record persists to disk with the workspace session state (`workspace-session-sleeping-agents.ts` → `workspace-session.ts:393`). + +### The staleness gate is a no-op + +On workspace/worktree open, `resumeSleepingAgentSessionsForWorktree` (`resume-sleeping-agent-session.ts:143-218`) launches `codex resume ` (argv built in `agent-session-resume.ts:195-220`; full command in `tui-agent-startup.ts:189-238`; launched via `sleeping-agent-session-launch.ts:65-128`) for every record unless a filter rejects it: + +- `isPassiveCompletedHibernationEvidence` (`sleeping-agent-pane-ownership.ts:16-18`): only skips records that are `done` **and** not `live`/`quit` origin. Orphaned records are `live`/`quit`, so they pass. +- `isInvalidWorktreeActivationRecord` (`resume-sleeping-agent-session.ts:131-141`): rejects records where `state !== 'done' && (capturedAt - updatedAt) > 30 min` (`AGENT_STATUS_STALE_AFTER_MS`, `shared/agent-status-types.ts:256`). **But live records are captured with `capturedAt === updatedAt`** (`agent-status.ts:1501`) and quit records stamp `capturedAt = Date.now()` immediately after the last update — so the difference is always ≈ 0. The check should compare against **`now`**; as written, a record from three days ago is perpetually "fresh." +- `recordPaneIsOwnedByPreservedPane`: skips only if the pane still exists in the restored layout — but the user closed the tab, so it doesn't. + +Result: orphaned records are neither passive, nor stale, nor pane-owned → **all of them get `codex resume`'d**, dozens in the same second. Triggers: folder-workspace open (`worktree-activation.ts:229`), worktree open (`:363`), startup hydration of the active worktree (`Terminal.tsx:1062-1075`), and mobile wake (`wake-sleeping-agents-in-background.ts:165-212`, wired at `useIpcEvents.ts:836`). + +### The feedback loop + +1. Workspace opens → every orphaned record is resumed → N fresh ptys, N idle `codex` processes. +2. Resumed sessions idle at a prompt and never reach `done`, so done-cleanup (`agent-status.ts:1541-1544`) never runs. +3. The periodic/quit capture (`agent-status.ts:2312-2350`) re-persists a record for every one of them. +4. User closes the tabs → Bug A leaves the processes alive; Bug B leaves the records in place. +5. Next open/restart → goto 1, with a strictly larger backlog. + +Distinct historical session IDs produce distinct claim keys, so per-claim dedup doesn't collapse them — which is exactly why one worktree accumulated 79 live codex processes across ~270 unique session IDs machine-wide. + +--- + +## 6. Proposed fixes + +### Fix 1 (primary, Bug A): make tab close actually kill the pty + +In `closeTab` (`terminals.ts:1192`) / `closeTerminalTab` (`terminal-tab-actions.ts:152`), issue `window.api.pty.kill()` for the tab's pty ids **before** `delete ptyIdsByTabId[tabId]`, mirroring the explicit kill loop in `kill-all-terminal-surfaces.ts:140-147`. This closes the parked-pane gap directly: the kill no longer depends on a mounted `TerminalPane` unmounting. Cover the split-pane close paths (`use-terminal-pane-lifecycle.ts`, `TerminalPaneOverlayLayer.tsx`) as well. + +Design decision needed: if warm-reattach semantics should be preserved for _some_ tab closes, distinguish "close tab" (kill) from "sleep/detach" (keep alive) explicitly in the UI action — today the distinction exists in the code (Sleep uses `keepIdentifiers: true`, `sleep-worktree-flow.ts:148`) but plain tab close falls into an unintended third state: forgotten but alive. + +### Fix 2 (primary, Bug B): remove resume records on tab close + +Have `closeTab` (and the split-pane close paths) delete the closed panes' entries from `sleepingAgentSessionsByPaneKey`. A user closing a tab is expressing "I'm done with this session" — it must not remain resumable-by-default. + +### Fix 3 (secondary, Bug B): origin-aware wall-clock expiry + +Do not globally expire resume records by wall time: intentional `worktree-sleep` +checkpoints may remain valid for days. A defensive expiry can apply only to +unowned `live` or `quit` records after their origin and ownership are validated +at every wake entry point. Explicit tab close must delete its records +immediately and remains the primary fix. + +### Fix 4 (secondary, Bug A): daemon-side reaper + +Implement the daemon `detach` handler (`daemon-server.ts:508-512`) and an idle reaper for sessions that are detached with **no owning tab**. The daemon deliberately outlives the app and is excluded from every `instanceof LocalPtyProvider` sweep, so it needs its own lifecycle policy — otherwise any future client-side bookkeeping bug leaks forever again. + +### Defense in depth + +- Surface a pty-pressure warning (the data already exists for the "Kill orphan terminals" feature) when app-owned ptys exceed a threshold (e.g. 300), since exhaustion is a machine-wide outage. +- The `--dangerously-bypass-approvals-and-sandbox` resume flag amplifies blast radius: hundreds of unattended, sandbox-less agents idling for days. Consider whether bulk background resume should ever use it. + +--- + +## 7. Verification plan + +1. **Repro (pre-fix):** in a background worktree, open an agent session, switch to another worktree (parking the pane), close the tab → `ps` shows the `login/zsh/codex` chain still alive and the helper still holds its ptmx fd. Restart Orca → the session is resumed again. +2. **Post-fix:** same steps → chain dies on tab close (Fix 1); record removed so restart resumes nothing (Fix 2). Origin-aware defensive expiry is verified separately when implemented (Fix 3). +3. **Regression:** worktree Sleep → wake still resumes sessions; quit → relaunch still warm-reattaches sessions whose tabs were _not_ closed; daemon warm-reattach unaffected. +4. **Soak:** count `Orca Helper` ptmx fds (`lsof -p | grep -c ptmx`) across a day of open/close/restart cycles — should stay flat. + +## 8. Immediate mitigation (ops, no code) + +- Use **Manage Sessions → "Kill orphan terminals"** or exact-session termination + to release the leaked ptys. A normal Orca.app restart is insufficient because + the persistent terminal daemon deliberately survives for warm reattach. +- `sudo sysctl kern.tty.ptmx_max=999` raises the cap to its macOS hard max — buys headroom only, hours at current leak rate. +- Restart the terminal daemon only when exact-session cleanup is unavailable; + this terminates all daemon-backed sessions, including legitimate in-flight work. + +--- + +## Appendix: diagnostic commands used + +```sh +ls /dev/ttys* | wc -l && sysctl kern.tty.ptmx_max # allocation vs cap +ps -ax -o pid,tty,etime,comm | grep -E 'ttys[0-9]+' # who holds ptys +lsof -a -d cwd -c codex | awk '{print $NF}' | sort | uniq -c | sort -rn # per-worktree counts +lsof -p | grep -c ptmx # master fds per Orca Helper +ps -ax -o args | grep -oE '[0-9a-f-]{36}' | sort | uniq -c # duplicate session resumes +``` diff --git a/src/main/agent-hooks/server-pane-authority.test.ts b/src/main/agent-hooks/server-pane-authority.test.ts new file mode 100644 index 00000000000..7d622b5a8c9 --- /dev/null +++ b/src/main/agent-hooks/server-pane-authority.test.ts @@ -0,0 +1,157 @@ +import { describe, expect, it, vi } from 'vitest' +import { makePaneKey } from '../../shared/stable-pane-id' +import { AgentHookServer, PANE_KEY_ALIASES_MAX } from './server' + +const SOURCE = makePaneKey('tab-source', '11111111-1111-4111-8111-111111111111') +const TARGET = makePaneKey('tab-target', '22222222-2222-4222-8222-222222222222') +const FINAL = makePaneKey('tab-final', '33333333-3333-4333-8333-333333333333') +const SIBLING = makePaneKey('tab-target', '44444444-4444-4444-8444-444444444444') + +describe('AgentHookServer pane authority', () => { + it('keeps physical hooks routed after the source tab closes and suppresses them after owner retire', () => { + const server = new AgentHookServer() + server.ingestTerminalStatus({ + paneKey: SOURCE, + tabId: 'tab-source', + worktreeId: 'wt-1', + payload: { state: 'working', prompt: 'source' } + }) + + server.transferPaneAuthority(SOURCE, TARGET, 'pty-1') + server.dropStatusEntriesByTabPrefix('tab-source') + server.ingestTerminalStatus({ + paneKey: SOURCE, + tabId: 'tab-source', + worktreeId: 'wt-1', + payload: { state: 'working', prompt: 'after source close' } + }) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + paneKey: TARGET, + tabId: 'tab-target', + prompt: 'after source close' + }) + ]) + + server.ingestTerminalStatus({ + paneKey: SIBLING, + tabId: 'tab-target', + worktreeId: 'wt-1', + payload: { state: 'working', prompt: 'sibling' } + }) + server.retirePaneAuthority(TARGET) + server.ingestTerminalStatus({ + paneKey: SOURCE, + tabId: 'tab-source', + worktreeId: 'wt-1', + payload: { state: 'done', prompt: 'too late' } + }) + + expect(server.getStatusSnapshot()).toEqual([ + expect.objectContaining({ paneKey: SIBLING, prompt: 'sibling' }) + ]) + }) + + it('persists one physical alias while chained transfers advance its owner', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setPaneKeyAliasPersistenceListener(listener) + + server.transferPaneAuthority(SOURCE, TARGET, 'pty-1', 10) + server.transferPaneAuthority(TARGET, FINAL, 'pty-1', 20) + + expect(listener).toHaveBeenLastCalledWith([ + { + legacyPaneKey: SOURCE, + stablePaneKey: FINAL, + ptyId: 'pty-1', + updatedAt: 20 + } + ]) + }) + + it('requires live PTY ownership for a first transfer and trusts the chained alias afterward', () => { + const server = new AgentHookServer() + + server.ingestTerminalStatus({ + paneKey: SOURCE, + tabId: 'tab-source', + worktreeId: 'wt-1', + payload: { state: 'working', prompt: 'unverified source' } + }) + + expect(server.canTransferPaneAuthority(SOURCE, undefined, () => false)).toBe(false) + expect(server.canTransferPaneAuthority(SOURCE, 'pty-1', () => false)).toBe(false) + expect( + server.canTransferPaneAuthority(SOURCE, 'pty-1', (paneKey, ptyId) => { + return paneKey === SOURCE && ptyId === 'pty-1' + }) + ).toBe(true) + + server.transferPaneAuthority(SOURCE, TARGET, 'pty-1') + expect(server.canTransferPaneAuthority(TARGET, undefined, () => false)).toBe(true) + expect(server.canTransferPaneAuthority(TARGET, 'pty-1', () => false)).toBe(true) + expect(server.canTransferPaneAuthority(TARGET, 'other-pty', () => false)).toBe(false) + }) + + it('does not treat registered or restored aliases as verified authority', () => { + const registered = new AgentHookServer() + registered.registerPaneKeyAlias('tab-source:0', SOURCE, 'pty-1') + + expect(registered.canTransferPaneAuthority(SOURCE, undefined, () => false)).toBe(false) + expect(registered.canTransferPaneAuthority(SOURCE, 'pty-1', () => false)).toBe(false) + expect( + registered.canTransferPaneAuthority(SOURCE, 'pty-1', (paneKey, ptyId) => { + return paneKey === 'tab-source:0' && ptyId === 'pty-1' + }) + ).toBe(true) + + const restored = new AgentHookServer() + restored.transferPaneAuthority(SOURCE, TARGET, 'pty-1', 10, { authorityVerified: false }) + expect(restored.canTransferPaneAuthority(TARGET, undefined, () => false)).toBe(false) + expect(restored.canTransferPaneAuthority(TARGET, 'pty-1', () => false)).toBe(false) + expect( + restored.canTransferPaneAuthority(TARGET, 'pty-1', (paneKey, ptyId) => { + return paneKey === TARGET && ptyId === 'pty-1' + }) + ).toBe(true) + restored.transferPaneAuthority(TARGET, FINAL, 'pty-1', 20) + expect(restored.canTransferPaneAuthority(FINAL, undefined, () => false)).toBe(true) + }) + + it('prefers a spawn-verified legacy alias over an earlier migration fallback', () => { + const server = new AgentHookServer() + server.registerPaneKeyAlias('tab-source:0', SOURCE, 'pty-1') + server.registerPaneKeyAlias('tab-source:1', SOURCE, 'pty-1', 20, { + authorityVerified: true + }) + + expect(server.canTransferPaneAuthority(SOURCE, undefined, () => false)).toBe(true) + expect(server.canTransferPaneAuthority(SOURCE, 'pty-1', () => false)).toBe(true) + }) + + it('bounds persisted aliases by evicting the oldest authority', () => { + const server = new AgentHookServer() + const listener = vi.fn() + server.setPaneKeyAliasPersistenceListener(listener) + + for (let index = 0; index <= PANE_KEY_ALIASES_MAX; index += 1) { + const suffix = index.toString(16).padStart(12, '0') + server.transferPaneAuthority( + makePaneKey(`source-${index}`, `00000000-0000-4000-8000-${suffix}`), + makePaneKey(`target-${index}`, `10000000-0000-4000-8000-${suffix}`), + `pty-${index}`, + index + 1 + ) + } + + const persisted = listener.mock.calls.at(-1)?.[0] + expect(persisted).toHaveLength(PANE_KEY_ALIASES_MAX) + expect(persisted).not.toEqual( + expect.arrayContaining([ + expect.objectContaining({ legacyPaneKey: expect.stringContaining('source-0:') }) + ]) + ) + }) +}) diff --git a/src/main/agent-hooks/server.ts b/src/main/agent-hooks/server.ts index 32954344084..513735bd679 100644 --- a/src/main/agent-hooks/server.ts +++ b/src/main/agent-hooks/server.ts @@ -28,6 +28,7 @@ import { HOOK_REQUEST_SLOWLORIS_MS, markClaudeLeadTurnInterrupted, MAX_PANE_KEY_LEN, + movePaneCacheState, normalizeHookPayload, parseFormEncodedBody, readRequestBody, @@ -89,6 +90,7 @@ type PaneKeyAliasEntry = { stablePaneKey: string ptyId: string | null updatedAt: number + authorityVerified: boolean } // Why: name of the on-disk cache that survives Orca restart. Lives next to @@ -128,6 +130,8 @@ const HYDRATE_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000 // gone. Bound the set so it can't grow one entry per tab close for the whole // session (it is otherwise only cleared at app quit). export const CLOSED_AGENT_STATUS_TAB_IDS_MAX = 1024 +export const CLOSED_AGENT_STATUS_PANE_KEYS_MAX = 1024 +export const PANE_KEY_ALIASES_MAX = 1024 type LastStatusFile = { version: number @@ -165,7 +169,9 @@ function equivalentInterruptAgentType( // Why: paneKey is `${tabId}:${leafUuid}` — validate the durable leaf suffix // at write/hydrate time so legacy numeric rows fail closed. export function isValidPaneKey(value: unknown): value is string { - return typeof value === 'string' && parsePaneKey(value) !== null + return ( + typeof value === 'string' && value.length <= MAX_PANE_KEY_LEN && parsePaneKey(value) !== null + ) } function sanitizeHydratedEntry( @@ -473,6 +479,7 @@ export class AgentHookServer { private promptSentDedupeByPaneKey = new Map() private promptSentHashSalt = randomBytes(16).toString('hex') private closedAgentStatusTabIds = new Set() + private closedAgentStatusPaneKeys = new Set() // Why: identity check — skip writes when the JSON-stringified contents // exactly match the last successful disk write. Cheap protection against // re-firing trailing timers when nothing changed. @@ -641,13 +648,32 @@ export class AgentHookServer { } private shouldSuppressClosedTabStatus(paneKey: string): boolean { - const tabId = parsePaneKey(paneKey)?.tabId + const ownerPaneKey = this.resolvePaneKeyAlias(paneKey) + if ( + this.closedAgentStatusPaneKeys.has(paneKey) || + this.closedAgentStatusPaneKeys.has(ownerPaneKey) + ) { + return true + } + const tabId = parsePaneKey(ownerPaneKey)?.tabId if (!tabId) { return false } return this.closedAgentStatusTabIds.has(tabId) } + private markPaneClosedForAgentStatus(paneKey: string): void { + this.closedAgentStatusPaneKeys.delete(paneKey) + this.closedAgentStatusPaneKeys.add(paneKey) + while (this.closedAgentStatusPaneKeys.size > CLOSED_AGENT_STATUS_PANE_KEYS_MAX) { + const oldest = this.closedAgentStatusPaneKeys.keys().next().value + if (oldest === undefined) { + break + } + this.closedAgentStatusPaneKeys.delete(oldest) + } + } + private attachStatusTiming( payload: AgentHookEventPayload, now = Date.now() @@ -937,15 +963,67 @@ export class AgentHookServer { this.paneKeyAliasPersistenceListener?.(this.getPersistedPaneKeyAliases()) } + private boundPaneKeyAliases(): void { + while (this.legacyPaneKeyAliases.size > PANE_KEY_ALIASES_MAX) { + // Why: renderer-originated aliases are untrusted process-lifetime state; + // insertion-order eviction bounds both memory and per-message cleanup. + const oldestKey = this.legacyPaneKeyAliases.keys().next().value + if (!oldestKey) { + break + } + this.legacyPaneKeyAliases.delete(oldestKey) + } + } + + private getPhysicalPaneKeyForAuthority(paneKey: string, ptyId?: string): string { + const ownerPaneKey = this.resolvePaneKeyAlias(paneKey) + let fallbackPaneKey = paneKey + for (const [physicalPaneKey, entry] of this.legacyPaneKeyAliases) { + if ( + entry.stablePaneKey === ownerPaneKey && + (!ptyId || !entry.ptyId || entry.ptyId === ptyId) + ) { + if (entry.authorityVerified) { + return physicalPaneKey + } + fallbackPaneKey = physicalPaneKey + } + } + return fallbackPaneKey + } + + canTransferPaneAuthority( + fromPaneKey: string, + ptyId: string | undefined, + ownsPty: (physicalPaneKey: string, ptyId: string) => boolean + ): boolean { + if (!isValidPaneKey(fromPaneKey)) { + return false + } + const ownerPaneKey = this.resolvePaneKeyAlias(fromPaneKey) + const physicalPaneKey = this.getPhysicalPaneKeyForAuthority(fromPaneKey, ptyId) + const alias = this.legacyPaneKeyAliases.get(physicalPaneKey) + if (ptyId) { + return Boolean( + (alias?.authorityVerified && alias.ptyId === ptyId) || + ownsPty(physicalPaneKey, ptyId) || + (ownerPaneKey !== physicalPaneKey && ownsPty(ownerPaneKey, ptyId)) + ) + } + // Why: hook status is renderer-originated evidence, not PTY ownership. + // ID-less moves are safe only after a prior verified transfer minted an alias. + return alias?.authorityVerified === true + } + registerPaneKeyAlias( legacyPaneKey: string, stablePaneKey: string, ptyId?: string, updatedAt = Date.now(), - options?: { overwriteExisting?: boolean } + options?: { overwriteExisting?: boolean; authorityVerified?: boolean } ): void { const legacy = parseLegacyNumericPaneKey(legacyPaneKey) - const stable = parsePaneKey(stablePaneKey) + const stable = isValidPaneKey(stablePaneKey) ? parsePaneKey(stablePaneKey) : null if (!legacy || !stable || legacy.tabId !== stable.tabId) { return } @@ -957,24 +1035,110 @@ export class AgentHookServer { typeof ptyId === 'string' && ptyId.trim().length > 0 ? ptyId.trim() : existing?.ptyId const normalizedUpdatedAt = Number.isFinite(updatedAt) && updatedAt > 0 ? updatedAt : (existing?.updatedAt ?? Date.now()) + const authorityVerified = options?.authorityVerified ?? false if ( existing && existing.stablePaneKey === stablePaneKey && existing.ptyId === (normalizedPtyId ?? null) && - existing.updatedAt === normalizedUpdatedAt + existing.updatedAt === normalizedUpdatedAt && + existing.authorityVerified === authorityVerified ) { return } this.legacyPaneKeyAliases.set(legacy.paneKey, { stablePaneKey, ptyId: normalizedPtyId ?? null, - updatedAt: normalizedUpdatedAt + updatedAt: normalizedUpdatedAt, + authorityVerified }) + this.boundPaneKeyAliases() if (normalizedPtyId) { this.notifyPaneKeyAliasPersistenceListener() } } + transferPaneAuthority( + fromPaneKey: string, + toPaneKey: string, + ptyId?: string, + updatedAt = Date.now(), + options?: { authorityVerified?: boolean } + ): void { + if (!isValidPaneKey(fromPaneKey) || !isValidPaneKey(toPaneKey)) { + return + } + const previousOwnerPaneKey = this.resolvePaneKeyAlias(fromPaneKey) + const physicalPaneKey = this.getPhysicalPaneKeyForAuthority(fromPaneKey, ptyId) + const existing = this.legacyPaneKeyAliases.get(physicalPaneKey) + const normalizedPtyId = ptyId?.trim() || existing?.ptyId || null + const hadStatus = this.state.lastStatusByPaneKey.has(previousOwnerPaneKey) + movePaneCacheState(this.state, previousOwnerPaneKey, toPaneKey) + const movedStatus = this.state.lastStatusByPaneKey.get(toPaneKey) as + | EnrichedAgentHookEventPayload + | undefined + if (movedStatus) { + const owner = parsePaneKey(toPaneKey) + this.state.lastStatusByPaneKey.set(toPaneKey, { + ...movedStatus, + paneKey: toPaneKey, + tabId: owner?.tabId + }) + } + if (this.runtimeObservedStatusPaneKeys.delete(previousOwnerPaneKey)) { + this.runtimeObservedStatusPaneKeys.add(toPaneKey) + } + const promptDedupe = this.promptSentDedupeByPaneKey.get(previousOwnerPaneKey) + if (promptDedupe !== undefined) { + this.promptSentDedupeByPaneKey.delete(previousOwnerPaneKey) + this.promptSentDedupeByPaneKey.set(toPaneKey, promptDedupe) + } + this.clearAssistantMessageRetry(previousOwnerPaneKey) + // Why: the live process keeps posting the physical source key after detach; + // persist one chain-safe mapping to whichever surface currently owns it. + this.legacyPaneKeyAliases.set(physicalPaneKey, { + stablePaneKey: toPaneKey, + ptyId: normalizedPtyId, + updatedAt, + authorityVerified: options?.authorityVerified ?? true + }) + this.boundPaneKeyAliases() + this.closedAgentStatusPaneKeys.delete(toPaneKey) + this.notifyPaneKeyAliasPersistenceListener() + if (hadStatus) { + this.scheduleStatusPersist() + this.notifyStatusChangeListeners() + } + } + + retirePaneAuthority(paneKey: string): void { + const ownerPaneKey = this.resolvePaneKeyAlias(paneKey) + const paneKeys = new Set([paneKey, ownerPaneKey]) + let aliasChanged = false + for (const [physicalPaneKey, entry] of this.legacyPaneKeyAliases) { + if (physicalPaneKey === paneKey || entry.stablePaneKey === ownerPaneKey) { + this.legacyPaneKeyAliases.delete(physicalPaneKey) + paneKeys.add(physicalPaneKey) + paneKeys.add(entry.stablePaneKey) + aliasChanged = true + } + } + const hadStatus = [...paneKeys].some((key) => this.state.lastStatusByPaneKey.has(key)) + for (const key of paneKeys) { + this.markPaneClosedForAgentStatus(key) + this.clearAssistantMessageRetry(key) + clearPaneCacheState(this.state, key) + this.runtimeObservedStatusPaneKeys.delete(key) + this.promptSentDedupeByPaneKey.delete(key) + } + if (aliasChanged) { + this.notifyPaneKeyAliasPersistenceListener() + } + if (hadStatus) { + this.scheduleStatusPersist() + this.notifyStatusChangeListeners() + } + } + clearPaneKeyAliasesForPty( ptyId: string, options?: { shouldClearStablePaneKey?: (paneKey: string) => boolean } @@ -1030,10 +1194,9 @@ export class AgentHookServer { if (!stablePaneKey) { return body } - // Why: pre-migration live shells keep posting their immutable numeric - // ORCA_PANE_KEY. The reattach path proves the UUID leaf once, then this - // bridge lets hook caches and renderer state use only the stable key. - return { ...record, paneKey: stablePaneKey } + // Why: migrated and detached shells keep posting an immutable physical + // pane key; normalize both pane and tab identity to the current owner. + return { ...record, paneKey: stablePaneKey, tabId: parsePaneKey(stablePaneKey)?.tabId } } ingestTerminalStatus(event: { @@ -1043,7 +1206,8 @@ export class AgentHookServer { connectionId?: string | null payload: ParsedAgentStatusPayload }): void { - const paneKey = this.resolvePaneKeyAlias(event.paneKey.trim()) + const physicalPaneKey = event.paneKey.trim() + const paneKey = this.resolvePaneKeyAlias(physicalPaneKey) const parsedPaneKey = parsePaneKey(paneKey) if (paneKey.length === 0) { track('agent_hook_unattributed', { reason: 'empty_pane_key' }) @@ -1052,11 +1216,16 @@ export class AgentHookServer { if (paneKey.length > MAX_PANE_KEY_LEN || !parsedPaneKey) { return } - const tabId = + const reportedTabId = event.tabId !== undefined && event.tabId.trim().length > 0 ? event.tabId.trim() : undefined - if (tabId !== undefined && tabId !== parsedPaneKey.tabId) { + if ( + paneKey === physicalPaneKey && + reportedTabId !== undefined && + reportedTabId !== parsedPaneKey.tabId + ) { return } + const tabId = paneKey !== physicalPaneKey ? parsedPaneKey.tabId : reportedTabId if (this.shouldSuppressClosedTabStatus(paneKey)) { return } @@ -1135,7 +1304,8 @@ export class AgentHookServer { // Why: match the listener's HTTP path — `normalizeHookPayload` trims and // length-caps paneKey before caching, so the cache key here must follow // the same rule or remote-vs-local events for the same pane would diverge. - const paneKey = this.resolvePaneKeyAlias(envelope.paneKey.trim()) + const physicalPaneKey = envelope.paneKey.trim() + const paneKey = this.resolvePaneKeyAlias(physicalPaneKey) const parsedPaneKey = parsePaneKey(paneKey) if (paneKey.length === 0) { track('agent_hook_unattributed', { reason: 'empty_pane_key' }) @@ -1156,13 +1326,18 @@ export class AgentHookServer { // Why: mirror the HTTP path's `readStringField` behavior — trim and treat // empty-after-trim as undefined rather than letting a literal "" leak // into the event. - const tabId = + const reportedTabId = envelope.tabId !== undefined && envelope.tabId.trim().length > 0 ? envelope.tabId.trim() : undefined - if (tabId !== undefined && tabId !== parsedPaneKey.tabId) { + if ( + paneKey === physicalPaneKey && + reportedTabId !== undefined && + reportedTabId !== parsedPaneKey.tabId + ) { return } + const tabId = paneKey !== physicalPaneKey ? parsedPaneKey.tabId : reportedTabId if (this.shouldSuppressClosedTabStatus(paneKey)) { return } @@ -1363,6 +1538,7 @@ export class AgentHookServer { this.runtimeObservedStatusPaneKeys.clear() this.promptSentDedupeByPaneKey.clear() this.closedAgentStatusTabIds.clear() + this.closedAgentStatusPaneKeys.clear() this.legacyPaneKeyAliases.clear() clearAllListenerCaches(this.state) this.notifyStatusChangeListeners() @@ -1432,13 +1608,13 @@ export class AgentHookServer { let aliasChanged = false for (const [legacyPaneKey, entry] of this.legacyPaneKeyAliases) { - if ( - paneCacheKeyMatchesTab(legacyPaneKey, tabId) || - paneCacheKeyMatchesTab(entry.stablePaneKey, tabId) - ) { + const ownerMatches = paneCacheKeyMatchesTab(entry.stablePaneKey, tabId) + if (ownerMatches) { this.legacyPaneKeyAliases.delete(legacyPaneKey) paneKeysToClear.add(legacyPaneKey) paneKeysToClear.add(entry.stablePaneKey) + this.markPaneClosedForAgentStatus(legacyPaneKey) + this.markPaneClosedForAgentStatus(entry.stablePaneKey) aliasChanged = true } } diff --git a/src/main/ipc/agent-hooks.test.ts b/src/main/ipc/agent-hooks.test.ts index b4ff9701854..48440713cd0 100644 --- a/src/main/ipc/agent-hooks.test.ts +++ b/src/main/ipc/agent-hooks.test.ts @@ -9,6 +9,9 @@ import { makePaneKey } from '../../shared/stable-pane-id' const dropStatusEntry = vi.fn() const dropStatusEntriesByTabPrefix = vi.fn() +const retirePaneAuthority = vi.fn() +const transferPaneAuthority = vi.fn() +const canTransferPaneAuthority = vi.fn(() => true) const getStatusSnapshot = vi.fn() const inferInterrupt = vi.fn() const clearMigrationUnsupportedPtysByTabPrefix = vi.fn() @@ -42,6 +45,9 @@ vi.mock('../agent-hooks/server', async () => { agentHookServer: { dropStatusEntry, dropStatusEntriesByTabPrefix, + retirePaneAuthority, + transferPaneAuthority, + canTransferPaneAuthority, getStatusSnapshot, inferInterrupt } @@ -100,6 +106,10 @@ vi.mock('../kimi/hook-service', () => ({ beforeEach(() => { dropStatusEntry.mockReset() dropStatusEntriesByTabPrefix.mockReset() + retirePaneAuthority.mockReset() + transferPaneAuthority.mockReset() + canTransferPaneAuthority.mockReset() + canTransferPaneAuthority.mockReturnValue(true) getStatusSnapshot.mockReset() inferInterrupt.mockReset() clearMigrationUnsupportedPtysByTabPrefix.mockReset() @@ -378,3 +388,73 @@ describe('agentStatus:dropByTabPrefix IPC', () => { expect(removeAllListeners).toHaveBeenCalledWith('agentStatus:dropByTabPrefix') }) }) + +describe('agent pane authority IPC', () => { + it('retires one validated pane authority', async () => { + const { registerAgentHookHandlers } = await import('./agent-hooks') + registerAgentHookHandlers() + + onHandlers.get('agentStatus:retirePaneAuthority')!({}, PANE_KEY) + + expect(retirePaneAuthority).toHaveBeenCalledWith(PANE_KEY) + expect(clearMigrationUnsupportedPtysForPaneKey).toHaveBeenCalledWith(PANE_KEY) + }) + + it('transfers validated pane authority with its provider PTY', async () => { + const { registerAgentHookHandlers } = await import('./agent-hooks') + registerAgentHookHandlers() + + onHandlers.get('agentStatus:transferPaneAuthority')!( + {}, + { + fromPaneKey: PANE_KEY, + toPaneKey: CHILD_PANE_KEY, + ptyId: 'pty-1' + } + ) + + expect(transferPaneAuthority).toHaveBeenCalledWith(PANE_KEY, CHILD_PANE_KEY, 'pty-1') + }) + + it('rejects malformed pane authority messages and replaces prior listeners', async () => { + const { registerAgentHookHandlers } = await import('./agent-hooks') + registerAgentHookHandlers() + + onHandlers.get('agentStatus:retirePaneAuthority')!({}, 'tab-1:0') + onHandlers.get('agentStatus:transferPaneAuthority')!( + {}, + { + fromPaneKey: PANE_KEY, + toPaneKey: 'invalid', + ptyId: '' + } + ) + + expect(retirePaneAuthority).not.toHaveBeenCalled() + expect(transferPaneAuthority).not.toHaveBeenCalled() + expect(removeAllListeners).toHaveBeenCalledWith('agentStatus:retirePaneAuthority') + expect(removeAllListeners).toHaveBeenCalledWith('agentStatus:transferPaneAuthority') + }) + + it('rejects unowned and oversized pane authority transfers', async () => { + const { registerAgentHookHandlers } = await import('./agent-hooks') + registerAgentHookHandlers() + const transfer = onHandlers.get('agentStatus:transferPaneAuthority')! + + canTransferPaneAuthority.mockReturnValue(false) + transfer({}, { fromPaneKey: PANE_KEY, toPaneKey: CHILD_PANE_KEY, ptyId: 'forged-pty' }) + transfer({}, { fromPaneKey: PANE_KEY, toPaneKey: CHILD_PANE_KEY }) + canTransferPaneAuthority.mockReturnValue(true) + transfer({}, { fromPaneKey: PANE_KEY, toPaneKey: CHILD_PANE_KEY, ptyId: 'x'.repeat(513) }) + transfer( + {}, + { + fromPaneKey: `${'x'.repeat(180)}:11111111-1111-4111-8111-111111111111`, + toPaneKey: CHILD_PANE_KEY, + ptyId: 'pty-1' + } + ) + + expect(transferPaneAuthority).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/ipc/agent-hooks.ts b/src/main/ipc/agent-hooks.ts index b1e8cb4b2cb..76b98ccb821 100644 --- a/src/main/ipc/agent-hooks.ts +++ b/src/main/ipc/agent-hooks.ts @@ -5,9 +5,7 @@ import type { MigrationUnsupportedPtyEntry } from '../../shared/agent-status-types' import type { AgentInterruptInferenceRequest } from '../../shared/agent-interrupt-intent' -import type { OrcaRuntimeService } from '../runtime/orca-runtime' import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' -import { isValidTerminalTabId } from '../../shared/terminal-tab-id' import { ampHookService } from '../amp/hook-service' import { clearMigrationUnsupportedPtysByTabPrefix, @@ -27,44 +25,26 @@ import { hermesHookService } from '../hermes/hook-service' import { devinHookService } from '../devin/hook-service' import { kimiHookService } from '../kimi/hook-service' import { openClaudeHookService } from '../openclaude/hook-service' +import { registerAgentPaneAuthorityIpcHandlers } from './agent-pane-authority-ipc' +import { createAgentPaneAuthorityOwnership } from './agent-pane-authority-ownership' +import { + enrichAgentStatusIpcPayload, + isValidAgentStatusDropTabId, + type AgentStatusRuntimeEnrichment +} from './agent-status-ipc-boundary' -type AgentStatusRuntimeEnrichment = Pick< - OrcaRuntimeService, - 'getAgentStatusTerminalHandleForPaneKey' | 'getAgentStatusOrchestrationContextForPaneKey' -> - -const MAX_AGENT_STATUS_DROP_TAB_ID_LENGTH = 160 - -function enrichAgentStatusIpcPayload( - data: AgentStatusIpcPayload, - runtime: AgentStatusRuntimeEnrichment | undefined -): AgentStatusIpcPayload { - if (!runtime) { - return data - } - const terminalHandle = runtime.getAgentStatusTerminalHandleForPaneKey(data.paneKey) - const orchestration = runtime.getAgentStatusOrchestrationContextForPaneKey(data.paneKey) - return { - ...data, - ...(terminalHandle ? { terminalHandle } : {}), - ...(orchestration ? { orchestration } : {}) - } -} - -function isValidAgentStatusDropTabId(value: unknown): value is string { - return ( - typeof value === 'string' && - value.length <= MAX_AGENT_STATUS_DROP_TAB_ID_LENGTH && - value.trim() === value && - isValidTerminalTabId(value) - ) +type AgentHookHandlerDependencies = { + getPtyIdForPaneKey?: (paneKey: string) => string | undefined } // Why: install/remove are intentionally not exposed to the renderer. Orca // auto-installs managed hooks at app startup (see src/main/index.ts), so a // renderer-triggered remove would be silently reverted on the next launch // and mislead the user. -export function registerAgentHookHandlers(runtime?: AgentStatusRuntimeEnrichment): void { +export function registerAgentHookHandlers( + runtime?: AgentStatusRuntimeEnrichment, + dependencies: AgentHookHandlerDependencies = {} +): void { // Why: matches the defensive pattern in src/main/ipc/pty.ts so re-registration // never throws "Attempted to register a second handler..." if this function is // ever invoked more than once (e.g. the macOS app re-activation path that @@ -120,6 +100,13 @@ export function registerAgentHookHandlers(runtime?: AgentStatusRuntimeEnrichment console.warn('[agent-hooks] dropStatusEntriesByTabPrefix failed:', err) } }) + registerAgentPaneAuthorityIpcHandlers({ + ownsPty: createAgentPaneAuthorityOwnership({ + getPtyIdForPaneKey: dependencies.getPtyIdForPaneKey, + getRuntimeTerminalHandleForPaneKey: (paneKey) => + runtime?.getAgentStatusTerminalHandleForPaneKey(paneKey) + }) + }) ipcMain.handle('agentStatus:getSnapshot', (): AgentStatusIpcPayload[] => { // Why: the renderer pulls this after workspace hydration, so startup cannot // lose replayed statuses while its local store is still empty. Match the diff --git a/src/main/ipc/agent-pane-authority-ipc.ts b/src/main/ipc/agent-pane-authority-ipc.ts new file mode 100644 index 00000000000..a8956ecdbd3 --- /dev/null +++ b/src/main/ipc/agent-pane-authority-ipc.ts @@ -0,0 +1,54 @@ +import { ipcMain } from 'electron' +import { agentHookServer, isValidPaneKey } from '../agent-hooks/server' +import { clearMigrationUnsupportedPtysForPaneKey } from '../agent-hooks/migration-unsupported-pty-state' + +const MAX_PTY_ID_LENGTH = 512 + +export type AgentPaneAuthorityOwnership = { + ownsPty: (paneKey: string, ptyId: string) => boolean +} + +export function registerAgentPaneAuthorityIpcHandlers( + ownership: AgentPaneAuthorityOwnership +): void { + ipcMain.removeAllListeners('agentStatus:retirePaneAuthority') + ipcMain.removeAllListeners('agentStatus:transferPaneAuthority') + ipcMain.on('agentStatus:retirePaneAuthority', (_event, paneKey: unknown) => { + if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) { + return + } + try { + agentHookServer.retirePaneAuthority(paneKey) + clearMigrationUnsupportedPtysForPaneKey(paneKey) + } catch (err) { + console.warn('[agent-hooks] retirePaneAuthority failed:', err) + } + }) + ipcMain.on('agentStatus:transferPaneAuthority', (_event, value: unknown) => { + if (!value || typeof value !== 'object') { + return + } + const args = value as Record + const ptyId = typeof args.ptyId === 'string' ? args.ptyId : undefined + if ( + typeof args.fromPaneKey !== 'string' || + typeof args.toPaneKey !== 'string' || + !isValidPaneKey(args.fromPaneKey) || + !isValidPaneKey(args.toPaneKey) || + args.fromPaneKey === args.toPaneKey || + (args.ptyId !== undefined && + (typeof args.ptyId !== 'string' || + args.ptyId.length > MAX_PTY_ID_LENGTH || + args.ptyId.trim() !== args.ptyId || + args.ptyId.length === 0)) || + !agentHookServer.canTransferPaneAuthority(args.fromPaneKey, ptyId, ownership.ownsPty) + ) { + return + } + try { + agentHookServer.transferPaneAuthority(args.fromPaneKey, args.toPaneKey, ptyId) + } catch (err) { + console.warn('[agent-hooks] transferPaneAuthority failed:', err) + } + }) +} diff --git a/src/main/ipc/agent-pane-authority-ownership.test.ts b/src/main/ipc/agent-pane-authority-ownership.test.ts new file mode 100644 index 00000000000..2b5c9c016c8 --- /dev/null +++ b/src/main/ipc/agent-pane-authority-ownership.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it, vi } from 'vitest' +import { createAgentPaneAuthorityOwnership } from './agent-pane-authority-ownership' + +describe('agent pane authority ownership', () => { + it('accepts only the PTY bound to the physical local pane', () => { + const ownsPty = createAgentPaneAuthorityOwnership({ + getPtyIdForPaneKey: (paneKey) => (paneKey === 'source-pane' ? 'pty-1' : undefined) + }) + + expect(ownsPty('source-pane', 'pty-1')).toBe(true) + expect(ownsPty('source-pane', 'pty-forged')).toBe(false) + expect(ownsPty('other-pane', 'pty-1')).toBe(false) + }) + + it('matches scoped and legacy runtime IDs to the authoritative terminal handle', () => { + const getRuntimeTerminalHandleForPaneKey = vi.fn(() => 'terminal:one') + const ownsPty = createAgentPaneAuthorityOwnership({ + getRuntimeTerminalHandleForPaneKey + }) + + expect(ownsPty('source-pane', 'remote:terminal:one')).toBe(true) + expect(ownsPty('source-pane', 'remote:env-1@@terminal%3Aone')).toBe(true) + expect(ownsPty('source-pane', 'remote:env-1@@other')).toBe(false) + expect(ownsPty('source-pane', 'remote:env-1@@%E0%A4%A')).toBe(false) + expect(ownsPty('source-pane', 'remote:@@terminal%3Aone')).toBe(false) + expect(ownsPty('source-pane', 'remote:%20@@terminal%3Aone')).toBe(false) + expect(ownsPty('source-pane', 'remote:env-1@@')).toBe(false) + expect(ownsPty('source-pane', 'remote:env-1@@terminal:one')).toBe(false) + expect(ownsPty('source-pane', 'remote:terminal%3Aone')).toBe(false) + }) +}) diff --git a/src/main/ipc/agent-pane-authority-ownership.ts b/src/main/ipc/agent-pane-authority-ownership.ts new file mode 100644 index 00000000000..266bb19663a --- /dev/null +++ b/src/main/ipc/agent-pane-authority-ownership.ts @@ -0,0 +1,44 @@ +export type AgentPaneAuthorityOwnershipSources = { + getPtyIdForPaneKey?: (paneKey: string) => string | undefined + getRuntimeTerminalHandleForPaneKey?: (paneKey: string) => string | undefined +} + +function getRemoteRuntimeHandle(ptyId: string): string | null { + if (!ptyId.startsWith('remote:')) { + return null + } + const rest = ptyId.slice(7) + const separatorIndex = rest.indexOf('@@') + if (separatorIndex === -1) { + return rest.length > 0 && rest.trim() === rest ? rest : null + } + const encodedOwner = rest.slice(0, separatorIndex) + const encodedHandle = rest.slice(separatorIndex + 2) + if (!encodedOwner || !encodedHandle) { + return null + } + try { + const owner = decodeURIComponent(encodedOwner) + const handle = decodeURIComponent(encodedHandle) + if (!owner || owner.trim() !== owner || !handle || handle.trim() !== handle) { + return null + } + return `remote:${encodeURIComponent(owner)}@@${encodeURIComponent(handle)}` === ptyId + ? handle + : null + } catch { + return null + } +} + +export function createAgentPaneAuthorityOwnership( + sources: AgentPaneAuthorityOwnershipSources +): (paneKey: string, ptyId: string) => boolean { + return (paneKey, ptyId) => { + if (sources.getPtyIdForPaneKey?.(paneKey) === ptyId) { + return true + } + const runtimeHandle = sources.getRuntimeTerminalHandleForPaneKey?.(paneKey) + return Boolean(runtimeHandle && getRemoteRuntimeHandle(ptyId) === runtimeHandle) + } +} diff --git a/src/main/ipc/agent-status-ipc-boundary.ts b/src/main/ipc/agent-status-ipc-boundary.ts new file mode 100644 index 00000000000..cec23d681f6 --- /dev/null +++ b/src/main/ipc/agent-status-ipc-boundary.ts @@ -0,0 +1,35 @@ +import type { AgentStatusIpcPayload } from '../../shared/agent-status-types' +import { isValidTerminalTabId } from '../../shared/terminal-tab-id' +import type { OrcaRuntimeService } from '../runtime/orca-runtime' + +export type AgentStatusRuntimeEnrichment = Pick< + OrcaRuntimeService, + 'getAgentStatusTerminalHandleForPaneKey' | 'getAgentStatusOrchestrationContextForPaneKey' +> + +const MAX_AGENT_STATUS_DROP_TAB_ID_LENGTH = 160 + +export function enrichAgentStatusIpcPayload( + data: AgentStatusIpcPayload, + runtime: AgentStatusRuntimeEnrichment | undefined +): AgentStatusIpcPayload { + if (!runtime) { + return data + } + const terminalHandle = runtime.getAgentStatusTerminalHandleForPaneKey(data.paneKey) + const orchestration = runtime.getAgentStatusOrchestrationContextForPaneKey(data.paneKey) + return { + ...data, + ...(terminalHandle ? { terminalHandle } : {}), + ...(orchestration ? { orchestration } : {}) + } +} + +export function isValidAgentStatusDropTabId(value: unknown): value is string { + return ( + typeof value === 'string' && + value.length <= MAX_AGENT_STATUS_DROP_TAB_ID_LENGTH && + value.trim() === value && + isValidTerminalTabId(value) + ) +} diff --git a/src/main/ipc/pty-management.test.ts b/src/main/ipc/pty-management.test.ts index 443b356ba4a..4e30669fb9e 100644 --- a/src/main/ipc/pty-management.test.ts +++ b/src/main/ipc/pty-management.test.ts @@ -226,10 +226,11 @@ describe('pty:management IPC handlers', () => { async function runKillAllWithPolls( handler: (event: unknown, args?: unknown) => unknown, pollCount: number = 65 - ): Promise<{ killedCount: number; remainingCount: number }> { + ): Promise<{ killedCount: number; remainingCount: number; killedSessionIds: string[] }> { const resultPromise = handler({}) as Promise<{ killedCount: number remainingCount: number + killedSessionIds: string[] }> // Why: advance the loop's sleeps one at a time. Between each sleep the // handler awaits collectSessions (a microtask), so we need to flush @@ -275,7 +276,11 @@ describe('pty:management IPC handlers', () => { const handlers = buildHandlerMap() const result = await runKillAllWithPolls(handlers['pty:management:killAll']) - expect(result).toEqual({ killedCount: 3, remainingCount: 0 }) + expect(result).toEqual({ + killedCount: 3, + remainingCount: 0, + killedSessionIds: ['new-1', 'new-2', 'old-1'] + }) // Each initial session receives exactly one shutdown — no retries. expect(current.shutdown).toHaveBeenCalledTimes(2) expect(current.shutdown).toHaveBeenCalledWith('new-1', { immediate: true }) @@ -302,7 +307,7 @@ describe('pty:management IPC handlers', () => { const handlers = buildHandlerMap() const result = await runKillAllWithPolls(handlers['pty:management:killAll']) - expect(result).toEqual({ killedCount: 0, remainingCount: 1 }) + expect(result).toEqual({ killedCount: 0, remainingCount: 1, killedSessionIds: [] }) // One shutdown fired — no per-session retry. Initial-snapshot // accounting means the stuck session is counted once. expect(current.shutdown).toHaveBeenCalledTimes(1) @@ -336,7 +341,11 @@ describe('pty:management IPC handlers', () => { const handlers = buildHandlerMap() const result = await runKillAllWithPolls(handlers['pty:management:killAll']) - expect(result).toEqual({ killedCount: 2, remainingCount: 0 }) + expect(result).toEqual({ + killedCount: 2, + remainingCount: 0, + killedSessionIds: ['a', 'b'] + }) }) it('swallows per-session shutdown rejections without stopping the batch', async () => { @@ -370,7 +379,11 @@ describe('pty:management IPC handlers', () => { expect(current.shutdown).toHaveBeenCalledWith('a', { immediate: true }) expect(current.shutdown).toHaveBeenCalledWith('b', { immediate: true }) // 'a' rejected and is still alive → counts as remaining; 'b' reaped. - expect(result).toEqual({ killedCount: 1, remainingCount: 1 }) + expect(result).toEqual({ + killedCount: 1, + remainingCount: 1, + killedSessionIds: ['b'] + }) }) }) diff --git a/src/main/ipc/pty-management.ts b/src/main/ipc/pty-management.ts index 107ca8e150d..9459984c5e3 100644 --- a/src/main/ipc/pty-management.ts +++ b/src/main/ipc/pty-management.ts @@ -74,7 +74,11 @@ export function registerDaemonManagementHandlers(): void { // daemons aren't killed here. ipcMain.handle( 'pty:management:killAll', - async (): Promise<{ killedCount: number; remainingCount: number }> => { + async (): Promise<{ + killedCount: number + remainingCount: number + killedSessionIds: string[] + }> => { const adapters = getDaemonAdapters() // Why: snapshot the initial session set once, up front. All subsequent // accounting is relative to these IDs. If the renderer respawns panes @@ -87,7 +91,7 @@ export function registerDaemonManagementHandlers(): void { const initialCount = initial.length if (initialCount === 0) { - return { killedCount: 0, remainingCount: 0 } + return { killedCount: 0, remainingCount: 0, killedSessionIds: [] } } // Why: fire one shutdown per initial session, in parallel, once — no @@ -125,20 +129,29 @@ export function registerDaemonManagementHandlers(): void { // session count) is what keeps the math honest when the renderer // respawns panes with fresh IDs mid-kill. let remainingOriginalCount = initialCount + let remainingOriginalIds = initialIds for (let attempt = 0; attempt < MAX_POLL_ATTEMPTS; attempt += 1) { await sleep(POLL_INTERVAL_MS) const current = await collectSessions(adapters) - remainingOriginalCount = current.reduce( - (count, s) => (initialIds.has(s.sessionId) ? count + 1 : count), - 0 + remainingOriginalIds = new Set( + current + .filter((session) => initialIds.has(session.sessionId)) + .map((session) => session.sessionId) ) + remainingOriginalCount = remainingOriginalIds.size if (remainingOriginalCount === 0) { break } } const killedCount = initialCount - remainingOriginalCount - return { killedCount, remainingCount: remainingOriginalCount } + return { + killedCount, + remainingCount: remainingOriginalCount, + killedSessionIds: [...initialIds].filter( + (sessionId) => !remainingOriginalIds.has(sessionId) + ) + } } ) diff --git a/src/main/ipc/pty.test.ts b/src/main/ipc/pty.test.ts index 4838711803d..d7c2b11ca71 100644 --- a/src/main/ipc/pty.test.ts +++ b/src/main/ipc/pty.test.ts @@ -1695,7 +1695,11 @@ describe('registerPtyHandlers', () => { value: 'linux' }) try { - const env = await daemonSpawnAndGetEnv({ PATH: '/usr/local/bin:/usr/bin' }) + // Why: overriding process.platform does not change the already-loaded + // node:path dialect; keep this synthetic PATH internally consistent. + const env = await daemonSpawnAndGetEnv({ + PATH: ['/usr/local/bin', '/usr/bin'].join(delimiter) + }) const entries = env.PATH.split(delimiter) const shimDir = join('/tmp/orca-user-data', 'linux-orca-cli-shim') // Why: bare `orca` must resolve to the Orca CLI before /usr/bin/orca @@ -3326,6 +3330,17 @@ describe('registerPtyHandlers', () => { ) }) + it('rejects runtime terminal IDs before unowned local provider routing', async () => { + const shutdown = vi.spyOn(getLocalPtyProvider(), 'shutdown') + handlers.clear() + registerPtyHandlers(mainWindow as never) + + await expect( + handlers.get('pty:kill')!(null, { id: 'remote:env-1@@terminal-1' }) + ).rejects.toThrow('Invalid PTY provider id') + expect(shutdown).not.toHaveBeenCalled() + }) + it('synthesizes runtime exit after ordinary daemon-backed pty kill', async () => { const shutdown = vi.fn(async () => undefined) const runtime = { @@ -6724,6 +6739,8 @@ describe('registerPtyHandlers', () => { it('spawns a plain POSIX login shell and queues startup commands for the live session', async () => { const originalPlatform = process.platform + const originalHome = process.env.HOME + const originalOrcaOrigZdotdir = process.env.ORCA_ORIG_ZDOTDIR const originalShell = process.env.SHELL const originalZdotdir = process.env.ZDOTDIR @@ -6731,6 +6748,9 @@ describe('registerPtyHandlers', () => { configurable: true, value: 'darwin' }) + // Why: this test simulates macOS even when Vitest runs on a Windows host. + process.env.HOME = '/Users/test' + delete process.env.ORCA_ORIG_ZDOTDIR process.env.SHELL = '/bin/zsh' delete process.env.ZDOTDIR @@ -6748,6 +6768,16 @@ describe('registerPtyHandlers', () => { configurable: true, value: originalPlatform }) + if (originalHome === undefined) { + delete process.env.HOME + } else { + process.env.HOME = originalHome + } + if (originalOrcaOrigZdotdir === undefined) { + delete process.env.ORCA_ORIG_ZDOTDIR + } else { + process.env.ORCA_ORIG_ZDOTDIR = originalOrcaOrigZdotdir + } if (originalShell === undefined) { delete process.env.SHELL } else { @@ -10469,7 +10499,9 @@ describe('registerPtyHandlers', () => { expect(registerPaneKeyAliasMock).toHaveBeenCalledWith( 'tab-1:0', stablePaneKey, - expect.any(String) + expect.any(String), + expect.any(Number), + { authorityVerified: true } ) expect(clearMigrationUnsupportedPtysForPaneKeyMock).toHaveBeenCalledWith(stablePaneKey) expect(setMigrationUnsupportedPtyMock).not.toHaveBeenCalled() diff --git a/src/main/ipc/pty.ts b/src/main/ipc/pty.ts index 0dd30b24b1e..d1c501a3230 100644 --- a/src/main/ipc/pty.ts +++ b/src/main/ipc/pty.ts @@ -4322,7 +4322,9 @@ export function registerPtyHandlers( agentHookServer.registerPaneKeyAlias( legacySpawnPaneKey.paneKey, migrationUnsupportedPaneKey, - result.id + result.id, + Date.now(), + { authorityVerified: true } ) clearMigrationUnsupportedPtysForPaneKey(migrationUnsupportedPaneKey) } else if (validatedPaneKey) { @@ -4951,6 +4953,11 @@ export function registerPtyHandlers( }) ipcMain.handle('pty:kill', async (_event, args: { id: string; keepHistory?: boolean }) => { + if (typeof args?.id !== 'string' || !args.id || args.id.startsWith('remote:')) { + // Why: runtime terminal handles belong to terminal.close; allowing them + // to fall through unowned PTY routing could target the local provider. + throw new Error('Invalid PTY provider id') + } const ownedConnectionId = ptyOwnership.get(args.id) const parsedSshId = ownedConnectionId === undefined ? parseAppSshPtyId(args.id) : null const connectionId = ownedConnectionId ?? parsedSshId?.connectionId diff --git a/src/main/ipc/register-core-handlers.test.ts b/src/main/ipc/register-core-handlers.test.ts index 08af6977efa..e3456d3dad4 100644 --- a/src/main/ipc/register-core-handlers.test.ts +++ b/src/main/ipc/register-core-handlers.test.ts @@ -466,7 +466,9 @@ describe('registerCoreHandlers', () => { expect(registerOpenCodeUsageHandlersMock).toHaveBeenCalledWith(openCodeUsage) expect(registerAppHandlersMock).toHaveBeenCalledWith(store, { onBeforeRelaunch }) expect(registerCodexAccountHandlersMock).toHaveBeenCalledWith(codexAccounts) - expect(registerAgentHookHandlersMock).toHaveBeenCalledWith(runtime) + expect(registerAgentHookHandlersMock).toHaveBeenCalledWith(runtime, { + getPtyIdForPaneKey: expect.any(Function) + }) expect(registerPetHandlersMock).toHaveBeenCalled() expect(registerClaudeAccountHandlersMock).toHaveBeenCalledWith(claudeAccounts) expect(registerMiniMaxCredentialsHandlersMock).toHaveBeenCalledWith(rateLimits) diff --git a/src/main/ipc/register-core-handlers.ts b/src/main/ipc/register-core-handlers.ts index dc2c219025a..c827ba561df 100644 --- a/src/main/ipc/register-core-handlers.ts +++ b/src/main/ipc/register-core-handlers.ts @@ -53,6 +53,7 @@ import { registerSpeechHandlers } from './speech' import { registerOrcaProfileHandlers } from './orca-profiles' import { registerCodexAccountHandlers } from './codex-accounts' import { registerAgentHookHandlers } from './agent-hooks' +import { getPtyIdForPaneKey } from './pty' import { registerAgentTrustHandlers } from './agent-trust' import { registerClaudeAccountHandlers } from './claude-accounts' import { registerMiniMaxCredentialsHandlers } from './minimax-credentials' @@ -122,7 +123,7 @@ export function registerCoreHandlers( registerCodexUsageHandlers(codexUsage) registerOpenCodeUsageHandlers(openCodeUsage) registerCodexAccountHandlers(codexAccounts) - registerAgentHookHandlers(runtime) + registerAgentHookHandlers(runtime, { getPtyIdForPaneKey }) registerAgentTrustHandlers() registerClaudeAccountHandlers(claudeAccounts) registerMiniMaxCredentialsHandlers(rateLimits) diff --git a/src/main/persistence.test.ts b/src/main/persistence.test.ts index b9acb16ae66..b2919fc88e4 100644 --- a/src/main/persistence.test.ts +++ b/src/main/persistence.test.ts @@ -7355,6 +7355,44 @@ describe('Store', () => { } }) + it('restores cross-tab pane authority aliases before hook ingestion', async () => { + const physicalPaneKey = makePaneKey('tab-source', TEST_LEAF_1) + const ownerPaneKey = makePaneKey('tab-target', TEST_LEAF_2) + writeDataFile({ + schemaVersion: 1, + repos: [], + worktreeMeta: {}, + settings: {}, + ui: {}, + githubCache: { pr: {}, issue: {} }, + legacyPaneKeyAliasEntries: [ + { + ptyId: 'pty-detached', + legacyPaneKey: physicalPaneKey, + stablePaneKey: ownerPaneKey, + updatedAt: 10 + } + ] + }) + + await createStore() + const { agentHookServer } = await import('./agent-hooks/server') + agentHookServer.ingestTerminalStatus({ + paneKey: physicalPaneKey, + tabId: 'tab-source', + worktreeId: 'wt1', + payload: { state: 'working', prompt: 'detached after restart' } + }) + + expect(agentHookServer.getStatusSnapshot()).toEqual([ + expect.objectContaining({ + paneKey: ownerPaneKey, + tabId: 'tab-target', + prompt: 'detached after restart' + }) + ]) + }) + it('persists fallback aliases when a legacy split layout has no PTY leaf bindings', async () => { writeDataFile({ schemaVersion: 1, diff --git a/src/main/persistence.ts b/src/main/persistence.ts index 9d32c98d6bc..a72a9238fdd 100644 --- a/src/main/persistence.ts +++ b/src/main/persistence.ts @@ -2264,11 +2264,34 @@ function normalizeLegacyPaneKeyAliasEntries(value: unknown): LegacyPaneKeyAliasE return false } const legacy = parseLegacyNumericPaneKey(candidate.legacyPaneKey) + const relocatedSource = parsePaneKey(candidate.legacyPaneKey) const stable = parsePaneKey(candidate.stablePaneKey) - return Boolean(legacy && stable && legacy.tabId === stable.tabId) + return Boolean(stable && ((legacy && legacy.tabId === stable.tabId) || relocatedSource)) }) } +function registerPersistedPaneKeyAlias(entry: LegacyPaneKeyAliasEntry): void { + if (parseLegacyNumericPaneKey(entry.legacyPaneKey)) { + agentHookServer.registerPaneKeyAlias( + entry.legacyPaneKey, + entry.stablePaneKey, + entry.ptyId, + entry.updatedAt, + { overwriteExisting: false } + ) + return + } + // Why: detached agents keep their original UUID pane key across restarts; + // restore the physical-to-current-owner mapping before hook replay begins. + agentHookServer.transferPaneAuthority( + entry.legacyPaneKey, + entry.stablePaneKey, + entry.ptyId, + entry.updatedAt, + { authorityVerified: false } + ) +} + function mergeLegacyPaneKeyAliasEntries( entries: LegacyPaneKeyAliasEntry[] ): LegacyPaneKeyAliasEntry[] { @@ -2648,13 +2671,7 @@ export class Store { setMigrationUnsupportedPty(entry) } for (const entry of normalized.legacyPaneKeyAliasEntries) { - agentHookServer.registerPaneKeyAlias( - entry.legacyPaneKey, - entry.stablePaneKey, - entry.ptyId, - entry.updatedAt, - { overwriteExisting: false } - ) + registerPersistedPaneKeyAlias(entry) } setMigrationUnsupportedPtyPersistenceListener((entries) => { this.state.migrationUnsupportedPtyEntries = entries @@ -5689,13 +5706,7 @@ export class Store { } } for (const entry of normalized.legacyPaneKeyAliasEntries) { - agentHookServer.registerPaneKeyAlias( - entry.legacyPaneKey, - entry.stablePaneKey, - entry.ptyId, - entry.updatedAt, - { overwriteExisting: false } - ) + registerPersistedPaneKeyAlias(entry) } session = normalized.session const remappedLeases = remapSshRemotePtyLeaseLeafIds( diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 9fd010a8695..80e284bf31f 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -667,7 +667,11 @@ export type PtyManagementApi = { // `degraded` is true when the daemon is alive but cannot spawn fresh PTYs, so // new terminals run on the local provider without daemon persistence. listSessions: () => Promise<{ sessions: PtyManagementSession[]; degraded: boolean }> - killAll: () => Promise<{ killedCount: number; remainingCount: number }> + killAll: () => Promise<{ + killedCount: number + remainingCount: number + killedSessionIds?: string[] + }> killOne: (args: { sessionId: string }) => Promise<{ success: boolean }> restart: () => Promise<{ success: boolean }> } @@ -3125,6 +3129,14 @@ export type PreloadApi = { /** Drop every cached hook status under one terminal tab prefix. * Fire-and-forget. */ dropByTabPrefix: (tabId: string) => void + /** Permanently retire one pane's hook authority while siblings stay live. */ + retirePaneAuthority: (paneKey: string) => void + /** Move hook authority when a live pane is detached into another tab. */ + transferPaneAuthority: (args: { + fromPaneKey: string + toPaneKey: string + ptyId?: string + }) => void } mobile: { listNetworkInterfaces: () => Promise<{ diff --git a/src/preload/index.ts b/src/preload/index.ts index 9e5b7e419e1..bfea3e27a5d 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -4348,6 +4348,16 @@ const api = { * explicit tab close even when the renderer has no matching local row. */ dropByTabPrefix: (tabId: string): void => { ipcRenderer.send('agentStatus:dropByTabPrefix', tabId) + }, + retirePaneAuthority: (paneKey: string): void => { + ipcRenderer.send('agentStatus:retirePaneAuthority', paneKey) + }, + transferPaneAuthority: (args: { + fromPaneKey: string + toPaneKey: string + ptyId?: string + }): void => { + ipcRenderer.send('agentStatus:transferPaneAuthority', args) } }, diff --git a/src/renderer/src/components/Terminal.tsx b/src/renderer/src/components/Terminal.tsx index f8ae284a365..efee4e9ec3e 100644 --- a/src/renderer/src/components/Terminal.tsx +++ b/src/renderer/src/components/Terminal.tsx @@ -1334,9 +1334,9 @@ function Terminal(): React.JSX.Element | null { if (shouldDeferParkedPtyExitTabClose(tabId, ptyId)) { return } - handleCloseTab(tabId) + closeTerminalTab(tabId, { reason: 'pty-exit' }) }, - [consumeSuppressedPtyExit, handleCloseTab] + [consumeSuppressedPtyExit] ) const handleCloseOthers = useCallback( @@ -1364,11 +1364,17 @@ function Terminal(): React.JSX.Element | null { (unifiedTab?.contentType === 'browser' && browserWorkspaceHasRemoteOwner(state, unifiedTab.entityId, runtimeEnvironmentId))) ) { - void closeWebRuntimeSessionTab({ - worktreeId: activeWorktreeId, - tabId: unifiedTab.contentType === 'browser' ? unifiedTab.id : unifiedTab.entityId, - environmentId: runtimeEnvironmentId - }) + if (unifiedTab.contentType === 'terminal') { + // Why: paired-host bulk close must revoke renderer resume and hook + // authority as well as removing the host-owned session tab. + closeTerminalTab(unifiedTab.entityId) + } else { + void closeWebRuntimeSessionTab({ + worktreeId: activeWorktreeId, + tabId: unifiedTab.id, + environmentId: runtimeEnvironmentId + }) + } continue } if ((state.tabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id)) { @@ -1423,11 +1429,17 @@ function Terminal(): React.JSX.Element | null { (unifiedTab?.contentType === 'browser' && browserWorkspaceHasRemoteOwner(state, unifiedTab.entityId, runtimeEnvironmentId))) ) { - void closeWebRuntimeSessionTab({ - worktreeId: activeWorktreeId, - tabId: unifiedTab.contentType === 'browser' ? unifiedTab.id : unifiedTab.entityId, - environmentId: runtimeEnvironmentId - }) + if (unifiedTab.contentType === 'terminal') { + // Why: route every terminal close through the destructive local + // lifecycle boundary before the paired host RPC. + closeTerminalTab(unifiedTab.entityId) + } else { + void closeWebRuntimeSessionTab({ + worktreeId: activeWorktreeId, + tabId: unifiedTab.id, + environmentId: runtimeEnvironmentId + }) + } continue } if ((state.tabsByWorktree[activeWorktreeId] ?? []).some((tab) => tab.id === id)) { diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx index 228ddcbf53a..822c7207091 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.test.tsx @@ -2227,7 +2227,7 @@ describe('FloatingTerminalPanel close behavior', () => { const terminalPane = findByTypeName(element, 'TerminalPane') ;(terminalPane.props.onPtyExit as () => void)() - expect(mocks.closeTab).toHaveBeenCalledWith('tab-1') + expect(mocks.closeTab).toHaveBeenCalledWith('tab-1', { reason: 'pty-exit' }) expect(onOpenChange).not.toHaveBeenCalled() mocks.closeTab.mockClear() diff --git a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx index 706e3d76e77..ee85d577804 100644 --- a/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx +++ b/src/renderer/src/components/floating-terminal/FloatingTerminalPanel.tsx @@ -1519,7 +1519,7 @@ export function FloatingTerminalPanel({ // atlas to corrupt) while hidden, and the resume on // reopen rebuilds the renderer from scratch. isVisible={isActive && open} - onPtyExit={() => closeTab(tab.id)} + onPtyExit={() => closeTab(tab.id, { reason: 'pty-exit' })} onCloseTab={() => closeFloatingItem(tab.id)} /> diff --git a/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx b/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx index 9f7c8da1398..07027c00dd5 100644 --- a/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx +++ b/src/renderer/src/components/onboarding/OnboardingInlineCommandTerminal.tsx @@ -296,7 +296,7 @@ export function OnboardingInlineCommandTerminal({ isVisible onPtyExit={() => { onTerminalExit?.() - closeTab(tabId, { recordInteraction: false }) + closeTab(tabId, { recordInteraction: false, reason: 'pty-exit' }) }} onCloseTab={() => closeTab(tabId, { recordInteraction: false })} /> diff --git a/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts b/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts index 09de670c689..226d787c642 100644 --- a/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts +++ b/src/renderer/src/components/shared/kill-all-terminal-surfaces.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from 'vitest' import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' import { useAppStore, type AppState } from '@/store' +import { closeTerminalTab } from '../terminal/terminal-tab-actions' import { runKillAllTerminalSurfaces, snapshotKillAllTerminalSurfaceIds, @@ -29,6 +30,10 @@ function state(overrides: Partial = {}): KillAllTer tabsByWorktree: {}, unifiedTabsByWorktree: {}, ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + lastKnownRelayPtyIdByTabId: {}, + deferredSshSessionIdsByTabId: {}, + pendingReconnectPtyIdByTabId: {}, ...overrides } } @@ -120,7 +125,7 @@ describe('runKillAllTerminalSurfaces', () => { const targetIds = snapshotKillAllTerminalSurfaceIds(current) const calls: string[] = [] const killDaemonSessions = vi.fn(() => management.promise) - const closeSurface = vi.fn((targetId: string) => { + const closeSurface = vi.fn((targetId: string, _options: unknown) => { calls.push(`close:${targetId}`) removeSurface(current, targetId) }) @@ -166,10 +171,10 @@ describe('runKillAllTerminalSurfaces', () => { 'wt-new-active': [unified('later-visible', 'later-tab', 'wt-new-active')] }, ptyIdsByTabId: { - 'background-target': ['pty-shared', 'remote:runtime-only'], + 'background-target': ['pty-shared', 'pty-external', 'remote:runtime-only'], 'unified-only-target': ['pty-unified', 'pty-shared'], 'moved-target': ['ssh:host@@pty-last', 'pty-shared'], - 'later-tab': ['pty-later'] + 'later-tab': ['pty-later', 'pty-external'] } }) ;( @@ -177,29 +182,40 @@ describe('runKillAllTerminalSurfaces', () => { terminalLayoutsByTabId: Record }> } ).terminalLayoutsByTabId = { - 'moved-target': { ptyIdsByLeafId: { leaf: 'layout-restore-hint' } } + 'moved-target': { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { leaf: 'layout-restore-hint' } + } } management.resolve({ killedCount: 2, remainingCount: 1 }) - await vi.waitFor(() => expect(killPty).toHaveBeenCalledTimes(3)) + await vi.waitFor(() => expect(killPty).toHaveBeenCalledTimes(5)) - expect(closeSurface.mock.calls).toEqual([ - ['background-target', { force: true }], - ['unified-only-target', { force: true }], - ['moved-target', { force: true }] - ]) - expect(calls.slice(0, 3)).toEqual([ - 'close:background-target', - 'close:unified-only-target', - 'close:moved-target' + expect(closeSurface.mock.calls.map(([targetId]) => targetId)).toEqual([ + 'background-target', + 'unified-only-target', + 'moved-target' ]) + for (const [, options] of closeSurface.mock.calls) { + expect(options).toEqual( + expect.objectContaining({ + force: true, + localPtyTeardownOwnedExternally: true, + precomputedRetirementPlan: expect.any(Object) + }) + ) + } + expect(calls.indexOf('kill:pty-shared')).toBeLessThan(calls.indexOf('close:moved-target')) expect(killPty).toHaveBeenCalledWith('pty-shared') expect(killPty).toHaveBeenCalledWith('pty-unified') expect(killPty).toHaveBeenCalledWith('ssh:host@@pty-last') expect(killPty).not.toHaveBeenCalledWith('remote:runtime-only') expect(killPty).not.toHaveBeenCalledWith('pty-later') + expect(killPty).not.toHaveBeenCalledWith('pty-external') expect(killPty).not.toHaveBeenCalledWith('stale-pty') - expect(killPty).not.toHaveBeenCalledWith('tab-restore-hint') - expect(killPty).not.toHaveBeenCalledWith('layout-restore-hint') + expect(killPty).toHaveBeenCalledWith('tab-restore-hint') + expect(killPty).toHaveBeenCalledWith('layout-restore-hint') expect(snapshotKillAllTerminalSurfaceIds(current)).toEqual(['later-tab']) let settled = false @@ -215,7 +231,7 @@ describe('runKillAllTerminalSurfaces', () => { closeAttemptCount: 3, absentTargetCount: 4, failedCloseAttemptCount: 0, - exactKillAcceptedCount: 2, + exactKillAcceptedCount: 4, exactKillRejectedCount: 1, daemon: { status: 'fulfilled', killedCount: 2, remainingCount: 1 } }) @@ -240,11 +256,42 @@ describe('runKillAllTerminalSurfaces', () => { expect(summary.daemon).toEqual({ status: 'rejected' }) expect(summary.absentTargetCount).toBe(1) - expect(closeSurface).toHaveBeenCalledWith('target', { force: true }) + expect(closeSurface).toHaveBeenCalledWith( + 'target', + expect.objectContaining({ + force: true, + localPtyTeardownOwnedExternally: true, + precomputedRetirementPlan: expect.any(Object), + precomputedCloseState: expect.any(Object) + }) + ) expect(killPty).toHaveBeenCalledWith('local-pty') expect(snapshotKillAllTerminalSurfaceIds(current)).toEqual(['later']) }) + it('does not exact-kill daemon sessions already settled by management', async () => { + const current = state({ + tabsByWorktree: { wt: [terminal('target', 'wt')] }, + ptyIdsByTabId: { target: ['daemon-pty', 'ssh:host@@relay-pty'] } + }) + const killPty = vi.fn().mockResolvedValue(undefined) + + await runKillAllTerminalSurfaces(['target'], { + getState: () => current, + killDaemonSessions: vi.fn().mockResolvedValue({ + killedCount: 1, + remainingCount: 0, + killedSessionIds: ['daemon-pty'] + }), + closeSurface: (targetId) => removeSurface(current, targetId), + killPty, + reportSummary: vi.fn() + }) + + expect(killPty).toHaveBeenCalledOnce() + expect(killPty).toHaveBeenCalledWith('ssh:host@@relay-pty') + }) + it('reports the informational zero state without provider fanout', async () => { const killDaemonSessions = vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }) const closeSurface = vi.fn() @@ -326,7 +373,135 @@ describe('runKillAllTerminalSurfaces', () => { }) }) - it('records bounded count and latency evidence for 100 terminal tabs', async () => { + it('replans after a pre-mutation close failure and keeps the failed tab as a survivor', async () => { + const current = state({ + activeWorktreeId: 'wt', + tabsByWorktree: { wt: [terminal('fails', 'wt'), terminal('closes', 'wt')] }, + ptyIdsByTabId: { fails: ['pty-fails'], closes: ['pty-closes'] } + }) + const closeSurface = vi.fn( + ( + targetId: string, + _options: { precomputedCloseState: { terminalCountBeforeClose: number } } + ) => { + if (targetId === 'fails') { + throw new Error('pre-mutation failure') + } + removeSurface(current, targetId) + } + ) + const killPty = vi.fn().mockResolvedValue(undefined) + + const summary = await runKillAllTerminalSurfaces(['fails', 'closes'], { + getState: () => current, + killDaemonSessions: vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }), + closeSurface, + killPty, + yieldToRenderer: vi.fn().mockResolvedValue(undefined), + reportSummary: vi.fn() + }) + + expect(closeSurface.mock.calls[1]?.[1].precomputedCloseState).toEqual({ + owningWorktreeId: 'wt', + terminalCountBeforeClose: 2, + nextTerminalTabId: 'fails' + }) + expect(killPty).not.toHaveBeenCalledWith('pty-fails') + expect(killPty).toHaveBeenCalledWith('pty-closes') + expect(snapshotKillAllTerminalSurfaceIds(current)).toEqual(['fails']) + expect(summary).toMatchObject({ + closeAttemptCount: 2, + absentTargetCount: 1, + failedCloseAttemptCount: 1, + closeYieldCount: 1 + }) + }) + + it('revalidates remaining ownership after a yield before killing an exact PTY', async () => { + let current = state({ + activeWorktreeId: 'wt', + settings: { activeRuntimeEnvironmentId: 'env-1' }, + tabsByWorktree: { + wt: [terminal('first', 'wt'), terminal('second', 'wt'), terminal('third', 'wt')] + }, + ptyIdsByTabId: { + first: ['pty-first', 'remote:terminal-1'], + second: ['pty-second'], + third: ['pty-shared-after-yield', 'remote:env-1@@terminal-1'] + } + }) + const closeSurface = vi.fn( + ( + targetId: string, + options: { + precomputedRetirementPlan: { + runtimeTerminals: unknown[] + cleanupOnlyPtyIds: string[] + } + precomputedCloseState: { + terminalCountBeforeClose: number + nextTerminalTabId: string | null + } + } + ) => { + removeSurface(current, targetId) + if (targetId === 'second') { + current = { ...current } + } + expect(options.precomputedCloseState.terminalCountBeforeClose).toBeGreaterThan(0) + } + ) + const killPty = vi.fn().mockResolvedValue(undefined) + const yieldToRenderer = vi.fn(async () => { + current = state({ + activeWorktreeId: 'wt', + settings: { activeRuntimeEnvironmentId: 'env-1' }, + tabsByWorktree: { + wt: [terminal('third', 'wt'), terminal('survivor', 'wt')] + }, + ptyIdsByTabId: { + third: ['pty-shared-after-yield', 'remote:env-1@@terminal-1'], + survivor: ['pty-shared-after-yield'] + } + }) + }) + + const summary = await runKillAllTerminalSurfaces(['first', 'second', 'third'], { + getState: () => current, + killDaemonSessions: vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }), + closeSurface, + killPty, + yieldToRenderer, + reportSummary: vi.fn() + }) + + expect(closeSurface.mock.calls.map(([targetId]) => targetId)).toEqual([ + 'first', + 'second', + 'third' + ]) + expect(closeSurface.mock.calls[2]?.[1].precomputedCloseState).toEqual({ + owningWorktreeId: 'wt', + terminalCountBeforeClose: 2, + nextTerminalTabId: 'survivor' + }) + expect(closeSurface.mock.calls[2]?.[1].precomputedRetirementPlan).toMatchObject({ + runtimeTerminals: [], + cleanupOnlyPtyIds: ['remote:env-1@@terminal-1'] + }) + expect(killPty).toHaveBeenCalledWith('pty-first') + expect(killPty).toHaveBeenCalledWith('pty-second') + expect(killPty).not.toHaveBeenCalledWith('pty-shared-after-yield') + expect(snapshotKillAllTerminalSurfaceIds(current)).toEqual(['survivor']) + expect(summary).toMatchObject({ + closeAttemptCount: 3, + absentTargetCount: 3, + exactKillAcceptedCount: 2, + closeYieldCount: 1 + }) + }) + + it('records bounded fanout and two-close batches for 100 terminal tabs', async () => { const tabs = Array.from({ length: 100 }, (_, index) => terminal(`tab-${index}`, 'wt')) const ptyIdsByTabId = Object.fromEntries(tabs.map((tab, index) => [tab.id, [`pty-${index}`]])) const previousState = useAppStore.getState() @@ -340,11 +515,25 @@ describe('runKillAllTerminalSurfaces', () => { const unsubscribe = useAppStore.subscribe(() => { zustandWrites += 1 }) - const killPty = vi.fn().mockResolvedValue(undefined) + const providerKill = vi.fn().mockResolvedValue(undefined) + vi.stubGlobal('window', { api: { pty: { kill: providerKill } } }) + let nowMs = 0 try { const summary = await runKillAllTerminalSurfaces(snapshotKillAllTerminalSurfaceIds(), { - killDaemonSessions: vi.fn().mockResolvedValue({ killedCount: 0, remainingCount: 0 }), - killPty, + killDaemonSessions: vi.fn().mockResolvedValue({ + killedCount: 0, + remainingCount: 0, + killedSessionIds: [] + }), + closeSurface: (tabId, options) => { + closeTerminalTab(tabId, options) + nowMs += 1 + }, + killPty: providerKill, + now: () => nowMs, + yieldToRenderer: async () => { + nowMs += 0.1 + }, reportSummary: vi.fn() }) @@ -352,12 +541,13 @@ describe('runKillAllTerminalSurfaces', () => { expect(summary.absentTargetCount).toBe(100) expect(summary.exactKillAcceptedCount).toBe(100) expect(summary.closeDurationMs).toBeGreaterThanOrEqual(0) - expect(summary.maxCloseBatchDurationMs).toBeLessThanOrEqual(50) + expect(summary.maxCloseBatchDurationMs).toBeLessThanOrEqual(2.1) expect(summary.closeYieldCount).toBe(49) expect(summary.closePhaseExceededLongTaskBudget).toBe(false) expect(zustandWrites).toBeGreaterThanOrEqual(100) - expect(killPty).toHaveBeenCalledTimes(100) + expect(providerKill).toHaveBeenCalledTimes(100) } finally { + vi.unstubAllGlobals() unsubscribe() useAppStore.setState(previousState, true) } diff --git a/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts b/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts index 4509b2017a1..8d280ba3f73 100644 --- a/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts +++ b/src/renderer/src/components/shared/kill-all-terminal-surfaces.ts @@ -1,17 +1,25 @@ import { useAppStore, type AppState } from '@/store' -import { closeTerminalTab } from '../terminal/terminal-tab-actions' +import { + closeTerminalTab, + type PrecomputedTerminalCloseState +} from '../terminal/terminal-tab-actions' +import { + buildTerminalTabRetirementPlans, + type TerminalTabRetirementPlan, + type TerminalTabRetirementState +} from '@/store/slices/terminal-tab-retirement' +import { reserveTerminalRetirementTeardowns } from '@/store/slices/terminal-retirement-teardown-reservation' const CLOSE_BATCH_SIZE = 2 type DaemonKillAllResult = { killedCount: number remainingCount: number + killedSessionIds?: string[] } -export type KillAllTerminalSurfaceState = Pick< - AppState, - 'activeWorktreeId' | 'ptyIdsByTabId' | 'tabsByWorktree' | 'unifiedTabsByWorktree' -> +export type KillAllTerminalSurfaceState = TerminalTabRetirementState & + Pick export type KillAllTerminalSurfacesSummary = { targetCount: number @@ -34,7 +42,15 @@ export type KillAllTerminalSurfacesSummary = { type KillAllTerminalSurfaceDependencies = { getState: () => KillAllTerminalSurfaceState killDaemonSessions: () => Promise - closeSurface: (tabId: string, options: { force: true }) => void + closeSurface: ( + tabId: string, + options: { + force: true + localPtyTeardownOwnedExternally: true + precomputedRetirementPlan: TerminalTabRetirementPlan + precomputedCloseState: PrecomputedTerminalCloseState + } + ) => void killPty: (ptyId: string) => Promise now: () => number yieldToRenderer: () => Promise @@ -60,20 +76,31 @@ export function snapshotKillAllTerminalSurfaceIds( return [...targetIds] } -function getTargetOwners( +function getTargetIndex( state: KillAllTerminalSurfaceState, targetIds: ReadonlySet -): Map { +): { + ownerByTargetId: Map + terminalIdsByWorktree: Map> +} { const ownerByTargetId = new Map() + const terminalIdsByWorktree = new Map>() for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { + const ids = terminalIdsByWorktree.get(worktreeId) ?? new Set() for (const tab of tabs) { + ids.add(tab.id) if (targetIds.has(tab.id) && !ownerByTargetId.has(tab.id)) { ownerByTargetId.set(tab.id, worktreeId) } } + terminalIdsByWorktree.set(worktreeId, ids) } for (const [worktreeId, tabs] of Object.entries(state.unifiedTabsByWorktree)) { + const ids = terminalIdsByWorktree.get(worktreeId) ?? new Set() for (const tab of tabs) { + if (tab.contentType === 'terminal') { + ids.add(tab.entityId) + } if ( tab.contentType === 'terminal' && targetIds.has(tab.entityId) && @@ -82,12 +109,18 @@ function getTargetOwners( ownerByTargetId.set(tab.entityId, worktreeId) } } + terminalIdsByWorktree.set(worktreeId, ids) } - return ownerByTargetId + return { ownerByTargetId, terminalIdsByWorktree } } -function isTargetPresent(state: KillAllTerminalSurfaceState, targetId: string): boolean { - return snapshotKillAllTerminalSurfaceIds(state).includes(targetId) +function getNextTerminalId(ids: ReadonlySet, closingId: string): string | null { + for (const id of ids) { + if (id !== closingId) { + return id + } + } + return null } function createDefaultDependencies(): KillAllTerminalSurfaceDependencies { @@ -117,7 +150,6 @@ export async function runKillAllTerminalSurfaces( ): Promise { const deps = { ...createDefaultDependencies(), ...dependencies } const targetIds = [...new Set(snapshotTargetIds)] - const targetIdSet = new Set(targetIds) let daemon: KillAllTerminalSurfacesSummary['daemon'] try { @@ -127,53 +159,105 @@ export async function runKillAllTerminalSurfaces( } const cleanupState = deps.getState() - const ownerByTargetId = getTargetOwners(cleanupState, targetIdSet) - const presentTargetIds = targetIds.filter((targetId) => ownerByTargetId.has(targetId)) - const activeWorktreeId = cleanupState.activeWorktreeId - // Why: keeping the active worktree until its targets close lets the existing - // tab action choose editor/browser/deactivation without spawning a replacement. - const closeOrder = [ - ...presentTargetIds.filter((targetId) => ownerByTargetId.get(targetId) !== activeWorktreeId), - ...presentTargetIds.filter((targetId) => ownerByTargetId.get(targetId) === activeWorktreeId) - ] - - const exactPtyIds = new Set() - for (const targetId of presentTargetIds) { - for (const ptyId of cleanupState.ptyIdsByTabId[targetId] ?? []) { - if (ptyId.length > 0 && !ptyId.startsWith('remote:')) { - exactPtyIds.add(ptyId) + const remainingTargetIds = new Set(targetIds) + const createCloseWave = (state: KillAllTerminalSurfaceState) => { + const remainingIdSet = new Set(remainingTargetIds) + const { ownerByTargetId, terminalIdsByWorktree } = getTargetIndex(state, remainingIdSet) + const presentTargetIds = [...remainingTargetIds].filter((targetId) => + ownerByTargetId.has(targetId) + ) + for (const targetId of remainingTargetIds) { + if (!ownerByTargetId.has(targetId)) { + remainingTargetIds.delete(targetId) } } + const activeWorktreeId = state.activeWorktreeId + // Why: keeping the active worktree until its targets close lets the existing + // tab action choose editor/browser/deactivation without spawning a replacement. + const closeOrder = [ + ...presentTargetIds.filter((targetId) => ownerByTargetId.get(targetId) !== activeWorktreeId), + ...presentTargetIds.filter((targetId) => ownerByTargetId.get(targetId) === activeWorktreeId) + ] + return { + state, + ownerByTargetId, + terminalIdsByWorktree, + closeOrder, + retirementPlans: buildTerminalTabRetirementPlans(state, closeOrder) + } } - let failedCloseAttemptCount = 0 + let closeWave = createCloseWave(cleanupState) + const daemonKilledSessionIds = new Set( + daemon.status === 'fulfilled' ? (daemon.killedSessionIds ?? []) : [] + ) + const scheduledPtyOwners = new Set() + const exactKillTasks: Promise[] = [] + const attemptedTargetIds: string[] = [] + const failedCloseTargetIds = new Set() const closeStartedAt = deps.now() let closeBatchStartedAt = closeStartedAt let maxCloseBatchDurationMs = 0 let closeYieldCount = 0 - for (let index = 0; index < closeOrder.length; index += 1) { - const targetId = closeOrder[index] - let failed = false - try { - deps.closeSurface(targetId, { force: true }) - } catch { - failed = true + while (closeWave.closeOrder.length > 0) { + const batchTargetIds = closeWave.closeOrder.splice(0, CLOSE_BATCH_SIZE) + let mustReplanAfterYield = false + for (const targetId of batchTargetIds) { + remainingTargetIds.delete(targetId) + attemptedTargetIds.push(targetId) + const owningWorktreeId = closeWave.ownerByTargetId.get(targetId)! + const remainingTerminalIds = + closeWave.terminalIdsByWorktree.get(owningWorktreeId) ?? new Set() + const nextTerminalTabId = getNextTerminalId(remainingTerminalIds, targetId) + const { plan: retirementPlan, newlyScheduledPtyOwners } = reserveTerminalRetirementTeardowns( + closeWave.state, + closeWave.retirementPlans.get(targetId)!, + scheduledPtyOwners + ) + let closeFailed = false + try { + deps.closeSurface(targetId, { + force: true, + localPtyTeardownOwnedExternally: true, + precomputedRetirementPlan: retirementPlan, + precomputedCloseState: { + owningWorktreeId, + terminalCountBeforeClose: remainingTerminalIds.size, + nextTerminalTabId + } + }) + } catch { + closeFailed = true + failedCloseTargetIds.add(targetId) + } + const failedTargetSurvived = + closeFailed && snapshotKillAllTerminalSurfaceIds(deps.getState()).includes(targetId) + if (failedTargetSurvived) { + // Why: a pre-mutation failure leaves the tab as a live non-target owner. + // Replan before touching siblings so it still protects counts and PTYs. + for (const owner of newlyScheduledPtyOwners) { + scheduledPtyOwners.delete(owner) + } + mustReplanAfterYield = true + break + } + remainingTerminalIds.delete(targetId) + for (const ptyId of retirementPlan.localOrSshPtyIds) { + if (daemonKilledSessionIds.has(ptyId)) { + continue + } + try { + exactKillTasks.push(deps.killPty(ptyId)) + } catch (error) { + exactKillTasks.push(Promise.reject(error)) + } + } } - try { - failed ||= isTargetPresent(deps.getState(), targetId) - } catch { - failed = true - } - if (failed) { - failedCloseAttemptCount += 1 - } - const isBatchEnd = (index + 1) % CLOSE_BATCH_SIZE === 0 || index + 1 === closeOrder.length - if (isBatchEnd) { - maxCloseBatchDurationMs = Math.max(maxCloseBatchDurationMs, deps.now() - closeBatchStartedAt) - } - if (isBatchEnd && index + 1 < closeOrder.length) { + maxCloseBatchDurationMs = Math.max(maxCloseBatchDurationMs, deps.now() - closeBatchStartedAt) + if (remainingTargetIds.size > 0) { // Why: closeTab cascades clone several store maps, so large confirmed // snapshots yield between bounded batches instead of monopolizing a frame. + const stateAfterBatch = deps.getState() try { await deps.yieldToRenderer() } catch { @@ -181,21 +265,30 @@ export async function runKillAllTerminalSurfaces( } closeYieldCount += 1 closeBatchStartedAt = deps.now() + const stateAfterYield = deps.getState() + if (mustReplanAfterYield || stateAfterYield !== stateAfterBatch) { + // Why: a yield lets tabs move, detach, or appear. Replanning the + // remaining snapshot prevents stale ownership from killing a survivor. + closeWave = createCloseWave(stateAfterYield) + } } } const closeDurationMs = Math.max(0, deps.now() - closeStartedAt) - const exactKillResults = await Promise.allSettled( - [...exactPtyIds].map((ptyId) => Promise.resolve().then(() => deps.killPty(ptyId))) - ) + // Why: the management sweep already settled daemon-owned IDs; awaiting only + // reserved exact kills keeps each provider at one request per ownership identity. + const exactKillResults = await Promise.allSettled(exactKillTasks) const exactKillAcceptedCount = exactKillResults.filter( (result) => result.status === 'fulfilled' ).length const finalTargetIds = new Set(snapshotKillAllTerminalSurfaceIds(deps.getState())) const absentTargetCount = targetIds.filter((targetId) => !finalTargetIds.has(targetId)).length + const failedCloseAttemptCount = attemptedTargetIds.filter( + (targetId) => failedCloseTargetIds.has(targetId) || finalTargetIds.has(targetId) + ).length const summary: KillAllTerminalSurfacesSummary = { targetCount: targetIds.length, - closeAttemptCount: closeOrder.length, + closeAttemptCount: attemptedTargetIds.length, absentTargetCount, failedCloseAttemptCount, exactKillAcceptedCount, diff --git a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.focus.test.ts b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.focus.test.ts index a8d3fd0a571..da137c5d6af 100644 --- a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.focus.test.ts +++ b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.focus.test.ts @@ -72,7 +72,8 @@ vi.mock('../../runtime/web-runtime-session', () => ({ closeWebRuntimeSessionTab: mocks.closeWebRuntimeSessionTab, createWebRuntimeSessionBrowserTab: vi.fn(), createWebRuntimeSessionTerminal: vi.fn(), - isWebRuntimeSessionActive: mocks.isWebRuntimeSessionActive + isWebRuntimeSessionActive: mocks.isWebRuntimeSessionActive, + toHostSessionTabId: (tabId: string) => tabId })) vi.mock('../../store/slices/browser-webview-cleanup', () => ({ @@ -233,6 +234,72 @@ describe('useTabGroupWorkspaceModel terminal activation focus', () => { expect(event.detail).toEqual({ tabId: 'terminal-1' }) }) + it('revokes local terminal state before paired-host bulk close', async () => { + const secondTerminal = { + id: 'terminal-2', + ptyId: 'remote:env-1@@pty-2', + worktreeId: 'wt-1', + title: 'Terminal 2', + defaultTitle: 'Terminal 2', + customTitle: null, + color: null, + sortOrder: 1, + createdAt: 1 + } + const secondUnified = { + id: 'unified-terminal-2', + entityId: secondTerminal.id, + groupId: 'group-1', + worktreeId: 'wt-1', + contentType: 'terminal', + label: 'Terminal 2', + customLabel: null, + color: null, + sortOrder: 1, + createdAt: 1 + } + const currentState = storeBox.state as { + tabsByWorktree: Record + unifiedTabsByWorktree: Record + } + const firstUnified = currentState.unifiedTabsByWorktree['wt-1'][0] + storeBox.state = { + ...storeBox.state, + settings: { activeRuntimeEnvironmentId: 'env-1' }, + tabsByWorktree: { + 'wt-1': [...currentState.tabsByWorktree['wt-1'], secondTerminal] + }, + unifiedTabsByWorktree: { + 'wt-1': [firstUnified, secondUnified] + }, + groupsByWorktree: { + 'wt-1': [ + { + id: 'group-1', + worktreeId: 'wt-1', + activeTabId: firstUnified.id, + tabOrder: [firstUnified.id, secondUnified.id] + } + ] + } + } + mocks.isWebRuntimeSessionActive.mockReturnValue(true) + const { useTabGroupWorkspaceModel } = await import('./useTabGroupWorkspaceModel') + const model = useTabGroupWorkspaceModel({ groupId: 'group-1', worktreeId: 'wt-1' }) + + model.commands.closeOthers(firstUnified.id) + + expect(mocks.closeTab).toHaveBeenCalledWith( + 'terminal-2', + expect.objectContaining({ remoteCloseOwnedByHost: true }) + ) + expect(mocks.closeWebRuntimeSessionTab).toHaveBeenCalledWith({ + worktreeId: 'wt-1', + tabId: 'terminal-2', + environmentId: 'env-1' + }) + }) + it('records terminal split completion when splitting a single terminal tab group', async () => { mocks.createEmptySplitGroup.mockReturnValue('group-2') mocks.createTab.mockReturnValue({ id: 'terminal-2' }) diff --git a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts index 87cbc63b0a0..5920077ac52 100644 --- a/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts +++ b/src/renderer/src/components/tab-group/useTabGroupWorkspaceModel.ts @@ -316,11 +316,9 @@ export function useTabGroupWorkspaceModel({ worktreeId ) if (item.contentType === 'terminal' && isWebRuntimeSessionActive(runtimeEnvironmentId)) { - void closeWebRuntimeSessionTab({ - worktreeId, - tabId: item.entityId, - environmentId: runtimeEnvironmentId - }) + // Why: paired-host bulk close must revoke local resume and hook + // authority before asking the host to remove its canonical tab. + closeTerminalTab(item.entityId) continue } if (item.contentType === 'browser') { diff --git a/src/renderer/src/components/terminal-pane/TerminalPaneOverlayLayer.tsx b/src/renderer/src/components/terminal-pane/TerminalPaneOverlayLayer.tsx index c30f5754327..af3468484ca 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPaneOverlayLayer.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPaneOverlayLayer.tsx @@ -15,6 +15,7 @@ import { shouldMountBackgroundWorktreeTab } from '../terminal/background-termina import { useNativeChatToggleShortcut } from '../native-chat/use-native-chat-toggle-shortcut' import { shouldDeferParkedPtyExitTabClose } from './terminal-parked-tab-watchers' import { useTerminalTabColdParking } from './use-terminal-tab-cold-parking' +import type { TerminalTabCloseReason } from '@/store/slices/terminal-tab-retirement' type TerminalOverlayAssignment = { unifiedTabId: string @@ -61,7 +62,7 @@ type TerminalOverlaySlotProps = { activityTerminalPortal: ActivityTerminalPortalTarget | null onFocusOwningGroup: ((groupId: string) => void) | undefined consumeSuppressedPtyExit: (ptyId: string) => boolean - closeTab: (tabId: string) => void + closeTab: (tabId: string, options?: { reason?: TerminalTabCloseReason }) => void leaveWorktreeIfEmpty: () => void } @@ -246,7 +247,7 @@ const TerminalOverlaySlot = memo(function TerminalOverlaySlot({ if (shouldDeferParkedPtyExitTabClose(terminalTabId, ptyId)) { return } - closeTab(terminalTabId) + closeTab(terminalTabId, { reason: 'pty-exit' }) leaveWorktreeIfEmpty() }} onCloseTab={() => { diff --git a/src/renderer/src/components/terminal-pane/pty-connection.test.ts b/src/renderer/src/components/terminal-pane/pty-connection.test.ts index 6c72e259c72..d22e1fb30a0 100644 --- a/src/renderer/src/components/terminal-pane/pty-connection.test.ts +++ b/src/renderer/src/components/terminal-pane/pty-connection.test.ts @@ -200,6 +200,7 @@ type StoreState = { setAgentStatus: ReturnType removeAgentStatus: ReturnType dropAgentStatus: ReturnType + retireAgentPaneAuthority: ReturnType setPaneForegroundAgent: ReturnType clearPaneForegroundAgent: ReturnType markTerminalTabUnread: ReturnType @@ -828,6 +829,7 @@ describe('connectPanePty', () => { ), removeAgentStatus: vi.fn(), dropAgentStatus: vi.fn(), + retireAgentPaneAuthority: vi.fn(), setPaneForegroundAgent: vi.fn((paneKey: string, entry: PaneForegroundAgentEntry) => { mockStoreState.paneForegroundAgentByPaneKey[paneKey] = entry }), diff --git a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts index d6dd76e817a..53086e28361 100644 --- a/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts +++ b/src/renderer/src/components/terminal-pane/terminal-parked-watcher-registry.ts @@ -62,6 +62,13 @@ export function disposeParkedTabWatchers(tabId: string): void { entry.disposersByPtyId.clear() } +export function retireParkedTerminalTab(tabId: string): void { + // Why: explicit tab retirement permanently invalidates both live parked + // observers and unmounted-pane candidates; neither may reattach later. + disposeParkedTabWatchers(tabId) + capturedPanesByTabId.delete(tabId) +} + /** * Synchronously disposes any parked watcher subscribed to these PTYs. * shutdownWorktreeTerminals silences the live transports' final teardown diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts index d3f35c8f5e1..744fc1d0f60 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.test.ts @@ -151,7 +151,7 @@ describe('shouldDetachPaneTransportOnUnmount', () => { ).toBe(true) }) - it('destroys when the tab is gone and no replacement owns the PTY', () => { + it('detaches when closeTab already owns provider shutdown for the removed tab', () => { expect( shouldDetachPaneTransportOnUnmount({ tabStillExists: false, @@ -159,6 +159,17 @@ describe('shouldDetachPaneTransportOnUnmount', () => { ptyId: 'remote:env@@term-1', worktreeTabs: [] }) + ).toBe(true) + }) + + it('destroys an ID-less transport so a pending spawn cannot outlive unmount', () => { + expect( + shouldDetachPaneTransportOnUnmount({ + tabStillExists: false, + tabId: 'tab-1', + ptyId: null, + worktreeTabs: [] + }) ).toBe(false) }) }) diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts index feeedbc6cd0..3430580e68d 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-lifecycle.ts @@ -459,15 +459,9 @@ export function shouldDetachPaneTransportOnUnmount(args: { ptyId: string | null worktreeTabs: readonly TerminalTab[] | undefined }): boolean { - if (!args.ptyId) { - return false - } - if (args.tabStillExists) { - return true - } - return Boolean( - args.worktreeTabs?.some((tab) => tab.id !== args.tabId && tab.ptyId === args.ptyId) - ) + // Why: mounted transport teardown is renderer-only; closeTab or the pane-close + // action already owns provider shutdown. Destroy only pending, ID-less spawns. + return Boolean(args.ptyId) } /** @@ -1296,16 +1290,10 @@ export function useTerminalPaneLifecycle({ } const leafId = closedPane?.leafId if (leafId && !isDetachedToTab) { - // Why: closing a pane is user-initiated teardown of this row — drop - // (not remove) so any retained `done` snapshot for this pane is also - // cleared and a same-frame live→gone transition cannot re-snapshot - // it via the retention sync. This is pane-keyed state, so it must - // clear even if the PTY transport was already removed. + // Why: pane close permanently revokes only this pane's authority; + // an exact tombstone blocks queued hooks without suppressing siblings. const paneKey = makePaneKey(tabId, leafId) - useAppStore.getState().setCacheTimerStartedAt(paneKey, null) - clearTerminalPaneUnread(paneKey) - useAppStore.getState().dropAgentStatus(paneKey) - useAppStore.getState().clearPaneForegroundAgent(paneKey) + useAppStore.getState().retireAgentPaneAuthority(paneKey) } if (transport) { if (isDetachedToTab) { diff --git a/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts b/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts new file mode 100644 index 00000000000..80cb239a57b --- /dev/null +++ b/src/renderer/src/components/terminal/close-local-terminal-tab-state.ts @@ -0,0 +1,45 @@ +import { useAppStore } from '@/store' +import type { + TerminalTabCloseReason, + TerminalTabRetirementPlan +} from '@/store/slices/terminal-tab-retirement' + +export function closeLocalTerminalTabState( + terminalTabId: string, + options?: { + reason?: TerminalTabCloseReason + remoteCloseOwnedByHost?: boolean + localPtyTeardownOwnedExternally?: boolean + precomputedRetirementPlan?: TerminalTabRetirementPlan + } +): void { + const state = useAppStore.getState() + if ( + options?.precomputedRetirementPlan?.tabId === terminalTabId || + Object.values(state.tabsByWorktree).some((tabs) => tabs.some((tab) => tab.id === terminalTabId)) + ) { + if ( + options?.reason || + options?.remoteCloseOwnedByHost || + options?.localPtyTeardownOwnedExternally || + options?.precomputedRetirementPlan + ) { + state.closeTab(terminalTabId, options) + } else { + state.closeTab(terminalTabId) + } + return + } + + for (const tabs of Object.values(state.unifiedTabsByWorktree ?? {})) { + const unified = tabs.find( + (tab) => + tab.contentType === 'terminal' && + (tab.entityId === terminalTabId || tab.id === terminalTabId) + ) + if (unified) { + state.closeTab(unified.entityId, options) + return + } + } +} diff --git a/src/renderer/src/components/terminal/terminal-close-target.ts b/src/renderer/src/components/terminal/terminal-close-target.ts new file mode 100644 index 00000000000..580cd6a9ebd --- /dev/null +++ b/src/renderer/src/components/terminal/terminal-close-target.ts @@ -0,0 +1,66 @@ +import type { AppState } from '@/store' +import type { TerminalTabRetirementPlan } from '@/store/slices/terminal-tab-retirement' + +export type PrecomputedTerminalCloseState = { + owningWorktreeId: string + terminalCountBeforeClose: number + nextTerminalTabId: string | null +} + +export type TerminalCloseTarget = { + worktreeId: string + terminalTabId: string +} + +export function validatePrecomputedTerminalCloseState( + tabId: string, + retirementPlan: TerminalTabRetirementPlan | undefined, + closeState: PrecomputedTerminalCloseState | undefined +): PrecomputedTerminalCloseState | undefined { + return retirementPlan?.tabId === tabId && + retirementPlan.worktreeId === closeState?.owningWorktreeId + ? closeState + : undefined +} + +export function resolveTerminalCloseTarget( + state: Pick, + tabId: string, + precomputed: PrecomputedTerminalCloseState | undefined +): TerminalCloseTarget | null { + if (precomputed) { + return { worktreeId: precomputed.owningWorktreeId, terminalTabId: tabId } + } + for (const [worktreeId, worktreeTabs] of Object.entries(state.tabsByWorktree)) { + if (worktreeTabs.some((tab) => tab.id === tabId)) { + return { worktreeId, terminalTabId: tabId } + } + } + for (const [worktreeId, unifiedTabs] of Object.entries(state.unifiedTabsByWorktree ?? {})) { + const unified = unifiedTabs.find( + (tab) => tab.contentType === 'terminal' && (tab.entityId === tabId || tab.id === tabId) + ) + if (unified) { + return { worktreeId, terminalTabId: unified.entityId } + } + } + return null +} + +// Why: host-backed terminals may exist only in unified state, so sibling +// selection must merge both representations into one terminal entity set. +export function getWorktreeTerminalTabIds( + state: Pick, + worktreeId: string +): string[] { + const ids = new Set() + for (const tab of state.tabsByWorktree[worktreeId] ?? []) { + ids.add(tab.id) + } + for (const tab of state.unifiedTabsByWorktree?.[worktreeId] ?? []) { + if (tab.contentType === 'terminal') { + ids.add(tab.entityId) + } + } + return [...ids] +} diff --git a/src/renderer/src/components/terminal/terminal-tab-actions-kill-all.test.ts b/src/renderer/src/components/terminal/terminal-tab-actions-kill-all.test.ts index a8ec093c60d..3c9710682b0 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions-kill-all.test.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions-kill-all.test.ts @@ -59,6 +59,7 @@ describe('closeTerminalTab kill-all routing', () => { }) it('force-closes a pinned terminal without opening a second confirmation', () => { + const closeTab = vi.fn() const closeUnifiedTab = vi.fn() const requestPinnedTabCloseConfirm = vi.fn() getStateMock.mockReturnValue( @@ -74,6 +75,7 @@ describe('closeTerminalTab kill-all routing', () => { } ] }, + closeTab, closeUnifiedTab, requestPinnedTabCloseConfirm }) @@ -82,7 +84,8 @@ describe('closeTerminalTab kill-all routing', () => { closeTerminalTab('terminal-1', { force: true }) expect(requestPinnedTabCloseConfirm).not.toHaveBeenCalled() - expect(closeUnifiedTab).toHaveBeenCalledWith('visible-pinned') + expect(closeTab).toHaveBeenCalledWith('terminal-1', { reason: undefined }) + expect(closeUnifiedTab).not.toHaveBeenCalled() }) it('routes the last active terminal to an existing editor without closing it', () => { diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.test.ts b/src/renderer/src/components/terminal/terminal-tab-actions.test.ts index a070cdc9331..540635961c2 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.test.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.test.ts @@ -197,7 +197,10 @@ describe('closeTerminalTab', () => { closeTerminalTab('local-tab-1') - expect(closeTab).toHaveBeenCalledWith('local-tab-1') + expect(closeTab).toHaveBeenCalledWith('local-tab-1', { + reason: undefined, + remoteCloseOwnedByHost: true + }) expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({ worktreeId: 'wt-1', tabId: 'host-tab-1', @@ -206,6 +209,7 @@ describe('closeTerminalTab', () => { }) it('closes unified-only terminal tabs when tabsByWorktree is missing the row', () => { + const closeTab = vi.fn() const closeUnifiedTab = vi.fn() getStateMock.mockReturnValue({ settings: { activeRuntimeEnvironmentId: null }, @@ -232,7 +236,7 @@ describe('closeTerminalTab', () => { activeTabId: 'terminal-entity-1', openFiles: [], browserTabsByWorktree: {}, - closeTab: vi.fn(), + closeTab, closeUnifiedTab, setActiveTab: vi.fn(), setActiveWorktree: vi.fn() @@ -240,10 +244,12 @@ describe('closeTerminalTab', () => { closeTerminalTab('terminal-entity-1') - expect(closeUnifiedTab).toHaveBeenCalledWith('unified-tab-1') + expect(closeTab).toHaveBeenCalledWith('terminal-entity-1', { reason: undefined }) + expect(closeUnifiedTab).not.toHaveBeenCalled() }) it('activates the next unified terminal tab when closing the active unified-only tab', () => { + const closeTab = vi.fn() const closeUnifiedTab = vi.fn() const setActiveTab = vi.fn() getStateMock.mockReturnValue({ @@ -285,7 +291,7 @@ describe('closeTerminalTab', () => { activeTabId: 'terminal-entity-1', openFiles: [], browserTabsByWorktree: {}, - closeTab: vi.fn(), + closeTab, closeUnifiedTab, setActiveTab, setActiveWorktree: vi.fn() @@ -294,7 +300,8 @@ describe('closeTerminalTab', () => { closeTerminalTab('terminal-entity-1') expect(setActiveTab).toHaveBeenCalledWith('terminal-entity-2') - expect(closeUnifiedTab).toHaveBeenCalledWith('unified-tab-1') + expect(closeTab).toHaveBeenCalledWith('terminal-entity-1', { reason: undefined }) + expect(closeUnifiedTab).not.toHaveBeenCalled() }) it('routes closes on a remote worktree to the host even when the local→host map has no entry', () => { @@ -319,7 +326,10 @@ describe('closeTerminalTab', () => { closeTerminalTab('plain-uuid-tab') - expect(closeTab).toHaveBeenCalledWith('plain-uuid-tab') + expect(closeTab).toHaveBeenCalledWith('plain-uuid-tab', { + reason: undefined, + remoteCloseOwnedByHost: true + }) expect(closeWebRuntimeSessionTabMock).toHaveBeenCalledWith({ worktreeId: 'wt-1', tabId: 'plain-uuid-tab', @@ -388,11 +398,13 @@ describe('closeTerminalTab', () => { it('closes the pinned tab when the confirmation callback runs', () => { const requestPinnedTabCloseConfirm = vi.fn() + const closeTab = vi.fn() const closeUnifiedTab = vi.fn() getStateMock.mockReturnValue( makePinnedTabState({ confirmClosePinnedTab: true, requestPinnedTabCloseConfirm, + closeTab, closeUnifiedTab }) ) @@ -401,7 +413,8 @@ describe('closeTerminalTab', () => { const { onConfirm } = requestPinnedTabCloseConfirm.mock.calls[0][0] as { onConfirm: () => void } onConfirm() - expect(closeUnifiedTab).toHaveBeenCalledWith('unified-pinned-1') + expect(closeTab).toHaveBeenCalledWith('pinned-entity-1', { reason: undefined }) + expect(closeUnifiedTab).not.toHaveBeenCalled() }) it('guards a pinned tab closed by its unified id (workspace overlay path)', () => { @@ -425,11 +438,13 @@ describe('closeTerminalTab', () => { it('closes a pinned tab immediately when the confirmation setting is off', () => { const requestPinnedTabCloseConfirm = vi.fn() + const closeTab = vi.fn() const closeUnifiedTab = vi.fn() getStateMock.mockReturnValue( makePinnedTabState({ confirmClosePinnedTab: false, requestPinnedTabCloseConfirm, + closeTab, closeUnifiedTab }) ) @@ -437,7 +452,8 @@ describe('closeTerminalTab', () => { closeTerminalTab('pinned-entity-1') expect(requestPinnedTabCloseConfirm).not.toHaveBeenCalled() - expect(closeUnifiedTab).toHaveBeenCalledWith('unified-pinned-1') + expect(closeTab).toHaveBeenCalledWith('pinned-entity-1', { reason: undefined }) + expect(closeUnifiedTab).not.toHaveBeenCalled() }) }) diff --git a/src/renderer/src/components/terminal/terminal-tab-actions.ts b/src/renderer/src/components/terminal/terminal-tab-actions.ts index 9991b7648ea..288c536dcf6 100644 --- a/src/renderer/src/components/terminal/terminal-tab-actions.ts +++ b/src/renderer/src/components/terminal/terminal-tab-actions.ts @@ -12,6 +12,18 @@ import { import { resolveHostSessionTabIdForWebSessionTab } from '@/runtime/web-session-tabs-sync' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' import { guardPinnedTabClose, resolvePinnedTabLabel } from '@/store/pinned-tab-close-guard' +import type { + TerminalTabCloseReason, + TerminalTabRetirementPlan +} from '@/store/slices/terminal-tab-retirement' +import { closeLocalTerminalTabState } from './close-local-terminal-tab-state' +import { + getWorktreeTerminalTabIds, + resolveTerminalCloseTarget, + validatePrecomputedTerminalCloseState, + type PrecomputedTerminalCloseState +} from './terminal-close-target' +export type { PrecomputedTerminalCloseState } from './terminal-close-target' const EDITOR_TAB_CONTENT_TYPES = new Set([ 'editor', @@ -22,71 +34,6 @@ const EDITOR_TAB_CONTENT_TYPES = new Set([ type TerminalTabActionState = ReturnType -type CloseTerminalTabTarget = { - worktreeId: string - terminalTabId: string -} - -function resolveCloseTerminalTabTarget( - state: TerminalTabActionState, - tabId: string -): CloseTerminalTabTarget | null { - for (const [worktreeId, worktreeTabs] of Object.entries(state.tabsByWorktree)) { - if (worktreeTabs.some((tab) => tab.id === tabId)) { - return { worktreeId, terminalTabId: tabId } - } - } - - for (const [worktreeId, unifiedTabs] of Object.entries(state.unifiedTabsByWorktree ?? {})) { - const unified = unifiedTabs.find( - (tab) => tab.contentType === 'terminal' && (tab.entityId === tabId || tab.id === tabId) - ) - if (unified) { - return { worktreeId, terminalTabId: unified.entityId } - } - } - - return null -} - -// Why: host-backed terminals may only exist in unifiedTabsByWorktree as -// terminal entities, so close/sibling selection must merge tabsByWorktree and -// unified terminal entityIds into one deduped list per worktree. -function getWorktreeTerminalTabIds(state: TerminalTabActionState, worktreeId: string): string[] { - const ids = new Set() - for (const tab of state.tabsByWorktree[worktreeId] ?? []) { - ids.add(tab.id) - } - for (const tab of state.unifiedTabsByWorktree?.[worktreeId] ?? []) { - if (tab.contentType === 'terminal') { - ids.add(tab.entityId) - } - } - return [...ids] -} - -function closeLocalTerminalTabState(terminalTabId: string): void { - const state = useAppStore.getState() - if ( - Object.values(state.tabsByWorktree).some((tabs) => tabs.some((tab) => tab.id === terminalTabId)) - ) { - state.closeTab(terminalTabId) - return - } - - for (const tabs of Object.values(state.unifiedTabsByWorktree ?? {})) { - const unified = tabs.find( - (tab) => - tab.contentType === 'terminal' && - (tab.entityId === terminalTabId || tab.id === terminalTabId) - ) - if (unified) { - state.closeUnifiedTab(unified.id) - return - } - } -} - function isPinnedVisibleTab( state: TerminalTabActionState, worktreeId: string, @@ -149,9 +96,23 @@ export function createNewTerminalTab( state.setTabBarOrder(activeWorktreeId, order) } -export function closeTerminalTab(tabId: string, options?: { force?: boolean }): void { +export function closeTerminalTab( + tabId: string, + options?: { + force?: boolean + reason?: TerminalTabCloseReason + localPtyTeardownOwnedExternally?: boolean + precomputedRetirementPlan?: TerminalTabRetirementPlan + precomputedCloseState?: PrecomputedTerminalCloseState + } +): void { const state = useAppStore.getState() - const target = resolveCloseTerminalTabTarget(state, tabId) + const precomputedCloseState = validatePrecomputedTerminalCloseState( + tabId, + options?.precomputedRetirementPlan, + options?.precomputedCloseState + ) + const target = resolveTerminalCloseTarget(state, tabId, precomputedCloseState) if (!target) { return } @@ -159,7 +120,11 @@ export function closeTerminalTab(tabId: string, options?: { force?: boolean }): // Why: a pinned tab routes through the confirmation guard instead of closing // outright. `force` is the post-confirmation re-entry, which skips the guard. - if (!options?.force && isPinnedVisibleTab(state, owningWorktreeId, terminalTabId)) { + if ( + options?.reason !== 'pty-exit' && + !options?.force && + isPinnedVisibleTab(state, owningWorktreeId, terminalTabId) + ) { guardPinnedTabClose({ isPinned: true, tabLabel: resolvePinnedTabLabel(state, owningWorktreeId, terminalTabId), @@ -187,7 +152,16 @@ export function closeTerminalTab(tabId: string, options?: { force?: boolean }): }) ?? toHostSessionTabId(terminalTabId) // Why: prune local mirrors immediately so close feels responsive while the // host session snapshot catches up. - closeLocalTerminalTabState(terminalTabId) + closeLocalTerminalTabState(terminalTabId, { + reason: options?.reason, + remoteCloseOwnedByHost: true, + ...(options?.localPtyTeardownOwnedExternally + ? { localPtyTeardownOwnedExternally: true } + : {}), + ...(options?.precomputedRetirementPlan + ? { precomputedRetirementPlan: options.precomputedRetirementPlan } + : {}) + }) void closeWebRuntimeSessionTab({ worktreeId: owningWorktreeId, tabId: hostBackedTabId, @@ -196,9 +170,21 @@ export function closeTerminalTab(tabId: string, options?: { force?: boolean }): return } - const currentTerminalTabIds = getWorktreeTerminalTabIds(state, owningWorktreeId) - if (currentTerminalTabIds.length <= 1) { - closeLocalTerminalTabState(terminalTabId) + const currentTerminalTabIds = precomputedCloseState + ? null + : getWorktreeTerminalTabIds(state, owningWorktreeId) + const terminalCountBeforeClose = + precomputedCloseState?.terminalCountBeforeClose ?? currentTerminalTabIds!.length + if (terminalCountBeforeClose <= 1) { + closeLocalTerminalTabState(terminalTabId, { + reason: options?.reason, + ...(options?.localPtyTeardownOwnedExternally + ? { localPtyTeardownOwnedExternally: true } + : {}), + ...(options?.precomputedRetirementPlan + ? { precomputedRetirementPlan: options.precomputedRetirementPlan } + : {}) + }) if (state.activeWorktreeId === owningWorktreeId) { // Why: only deactivate the worktree when no tabs of any kind remain. // Editor files are a separate tab type; closing the last terminal tab @@ -221,15 +207,22 @@ export function closeTerminalTab(tabId: string, options?: { force?: boolean }): } if (state.activeWorktreeId === owningWorktreeId && terminalTabId === state.activeTabId) { - const currentIndex = currentTerminalTabIds.indexOf(terminalTabId) - const nextTabId = - currentTerminalTabIds[currentIndex + 1] ?? currentTerminalTabIds[currentIndex - 1] + const currentIndex = currentTerminalTabIds?.indexOf(terminalTabId) ?? -1 + const nextTabId = precomputedCloseState + ? precomputedCloseState.nextTerminalTabId + : (currentTerminalTabIds![currentIndex + 1] ?? currentTerminalTabIds![currentIndex - 1]) if (nextTabId) { state.setActiveTab(nextTabId) } } - closeLocalTerminalTabState(terminalTabId) + closeLocalTerminalTabState(terminalTabId, { + reason: options?.reason, + ...(options?.localPtyTeardownOwnedExternally ? { localPtyTeardownOwnedExternally: true } : {}), + ...(options?.precomputedRetirementPlan + ? { precomputedRetirementPlan: options.precomputedRetirementPlan } + : {}) + }) } export function closeOtherTerminalTabs(tabId: string, activeWorktreeId: string | null): void { diff --git a/src/renderer/src/hooks/useIpcEvents.ts b/src/renderer/src/hooks/useIpcEvents.ts index 96697628bf4..efec4bae01c 100644 --- a/src/renderer/src/hooks/useIpcEvents.ts +++ b/src/renderer/src/hooks/useIpcEvents.ts @@ -124,6 +124,7 @@ import { showTerminalShortcutCaptureNotification } from '@/lib/terminal-shortcut import { resolveAgentStatusTerminalTitle } from '@/lib/agent-status-terminal-title' import { titleHasAgentName } from '../../../shared/agent-detection' import { getRuntimeEnvironmentIdForWorktree } from '@/lib/worktree-runtime-owner' +import { resolveAgentPaneAuthorityKey } from '@/store/slices/agent-pane-authority' import { translate } from '@/i18n/i18n' import { closeTerminalTab } from '@/components/terminal/terminal-tab-actions' import { initialAgentTabViewModeProps } from '@/lib/native-chat-initial-view-mode' @@ -251,10 +252,14 @@ let remoteWorkspaceSnapshotWriteSuppressUntil = 0 const REMOTE_WORKSPACE_SNAPSHOT_WRITE_SUPPRESS_MS = 1000 function isAgentStatusForRecentlyClosedTab( - store: Pick, + store: Pick, paneKey: string ): boolean { - const tabId = parsePaneKey(paneKey)?.tabId + const ownerPaneKey = resolveAgentPaneAuthorityKey(paneKey) + if (store.recentlyRetiredAgentStatusPaneKeys?.[ownerPaneKey] === true) { + return true + } + const tabId = parsePaneKey(ownerPaneKey)?.tabId if (!tabId) { return false } @@ -2968,6 +2973,8 @@ export function useIpcEvents(): void { if (isAgentStatusForRecentlyClosedTab(store, data.paneKey)) { return 'dropped' } + const paneKey = resolveAgentPaneAuthorityKey(data.paneKey) + const ownerTabId = parsePaneKey(paneKey)?.tabId ?? data.tabId const payload = normalizeAgentStatusPayload({ state: data.state, prompt: data.prompt, @@ -2993,7 +3000,7 @@ export function useIpcEvents(): void { repoConnectionId, repoConnectionResolved, owningWorktreeId - } = resolvePaneKey(store, data.paneKey) + } = resolvePaneKey(store, paneKey) if (!exists && data.worktreeId && hasRuntimeBackedWorktreeAttribution(data)) { // Why: orchestration worker hooks can carry main-side worktree // attribution before this renderer has a terminal tab for the pane. @@ -3075,7 +3082,7 @@ export function useIpcEvents(): void { const statusPayloadWithTurnBoundary = data.promptInteractionKey ? { ...statusPayload, promptInteractionKey: data.promptInteractionKey } : statusPayload - const existingStatus = store.agentStatusByPaneKey[data.paneKey] + const existingStatus = store.agentStatusByPaneKey[paneKey] if (existingStatus && data.receivedAt < existingStatus.updatedAt) { // Why: the store rejects out-of-order status rows; keep notification and // terminal lifecycle effects on the same accepted event boundary. @@ -3105,8 +3112,8 @@ export function useIpcEvents(): void { } if ( shouldSuppressCodexAutoApprovalStatus(statusPayload, { - paneKey: data.paneKey, - tabId: data.tabId, + paneKey, + tabId: ownerTabId, terminalHandle: data.terminalHandle, launchToken: data.launchToken, providerSession: data.providerSession, @@ -3120,7 +3127,7 @@ export function useIpcEvents(): void { const terminalTitle = resolveAgentStatusTerminalTitle(statusPayload, title) const statusWorktreeId = data.worktreeId ?? owningWorktreeId store.setAgentStatus( - data.paneKey, + paneKey, statusPayloadWithTurnBoundary, terminalTitle, { @@ -3128,7 +3135,7 @@ export function useIpcEvents(): void { stateStartedAt: data.stateStartedAt }, { - tabId: data.tabId, + tabId: ownerTabId, worktreeId: statusWorktreeId, terminalHandle: data.terminalHandle }, @@ -3139,7 +3146,7 @@ export function useIpcEvents(): void { } : undefined ) - applyResolvedAgentTerminalTitleToTab(store, data.paneKey, title, terminalTitle) + applyResolvedAgentTerminalTitleToTab(store, paneKey, title, terminalTitle) if (options?.replay !== true && statusWorktreeId) { // Why: local Codex/Claude hooks arrive through this main-process IPC // path, not the PTY OSC fallback, so task-complete notifications must @@ -3149,7 +3156,7 @@ export function useIpcEvents(): void { ? { ...resolvedPayload, stateStartedAt: data.stateStartedAt } : resolvedPayload observeAgentHookCompletionForNotification({ - paneKey: data.paneKey, + paneKey, worktreeId: statusWorktreeId, payload: notificationPayload }) diff --git a/src/renderer/src/lib/agent-background-draft-delivery.ts b/src/renderer/src/lib/agent-background-draft-delivery.ts new file mode 100644 index 00000000000..745248f7239 --- /dev/null +++ b/src/renderer/src/lib/agent-background-draft-delivery.ts @@ -0,0 +1,17 @@ +import type { TuiAgent } from '../../../shared/types' +import { pasteDraftWhenAgentReady } from '@/lib/agent-paste-draft' +import { showAutomationPromptNotSentToast } from '@/lib/agent-background-session-timeout-toast' + +export function scheduleAgentBackgroundDraft( + tabId: string, + content: string, + agent: TuiAgent +): void { + void pasteDraftWhenAgentReady({ + tabId, + content, + agent, + submit: true, + onTimeout: () => showAutomationPromptNotSentToast(agent) + }) +} diff --git a/src/renderer/src/lib/launch-agent-background-session.test.ts b/src/renderer/src/lib/launch-agent-background-session.test.ts index 652fddb9bcf..1c24ae7683a 100644 --- a/src/renderer/src/lib/launch-agent-background-session.test.ts +++ b/src/renderer/src/lib/launch-agent-background-session.test.ts @@ -68,6 +68,7 @@ const state = { } ] }, + tabsByWorktree: { 'wt-1': [] as { id: string; title: string }[] }, allWorktrees: vi.fn(() => state.worktreesByRepo['repo-1']), createTab: mockCreateTab, setTabCustomTitle: mockSetTabCustomTitle, @@ -143,7 +144,15 @@ describe('launchAgentBackgroundSession', () => { } ] } - mockCreateTab.mockReturnValue({ id: 'tab-1', title: 'Terminal 1' }) + state.tabsByWorktree = { 'wt-1': [] } + mockCreateTab.mockImplementation(() => { + const tab = { id: 'tab-1', title: 'Terminal 1' } + state.tabsByWorktree['wt-1'].push(tab) + return tab + }) + mockCloseTab.mockImplementation((tabId: string) => { + state.tabsByWorktree['wt-1'] = state.tabsByWorktree['wt-1'].filter((tab) => tab.id !== tabId) + }) mockSpawn.mockResolvedValue({ id: 'pty-1' }) mockRuntimeEnvironmentCall.mockResolvedValue({ ok: true, @@ -273,6 +282,84 @@ describe('launchAgentBackgroundSession', () => { ) }) + it('kills a local PTY when its tab closes before spawn resolves', async () => { + let resolveSpawn!: (result: { id: string }) => void + mockSpawn.mockReturnValueOnce( + new Promise<{ id: string }>((resolve) => { + resolveSpawn = resolve + }) + ) + const { launchAgentBackgroundSession } = await import('./launch-agent-background-session') + + const launch = launchAgentBackgroundSession({ + agent: 'claude', + worktreeId: 'wt-1', + prompt: 'run slowly' + }) + await vi.waitFor(() => expect(mockCreateTab).toHaveBeenCalledOnce()) + state.tabsByWorktree['wt-1'] = [] + resolveSpawn({ id: 'pty-after-close' }) + + await expect(launch).resolves.toBeNull() + expect(mockKill).toHaveBeenCalledWith('pty-after-close') + expect(mockUpdateTabPtyId).not.toHaveBeenCalled() + expect(mockSubscribeToPtyData).not.toHaveBeenCalled() + expect(mockDispatchEvent).not.toHaveBeenCalled() + }) + + it('closes a runtime terminal when its tab closes before creation resolves', async () => { + state.settings = { + agentCmdOverrides: {}, + activeRuntimeEnvironmentId: 'env-1', + terminalMainSideEffectAuthority: undefined + } + let resolveCreate!: (result: { + ok: true + result: { terminal: { handle: string; worktreeId: string; title: null } } + }) => void + const createResult = new Promise<{ + ok: true + result: { terminal: { handle: string; worktreeId: string; title: null } } + }>((resolve) => { + resolveCreate = resolve + }) + mockRuntimeEnvironmentCall.mockImplementation((args: { method: string }) => { + if (args.method === 'terminal.create') { + return createResult + } + return Promise.resolve({ ok: true, result: {} }) + }) + const { launchAgentBackgroundSession } = await import('./launch-agent-background-session') + + const launch = launchAgentBackgroundSession({ + agent: 'claude', + worktreeId: 'wt-1', + prompt: 'run remotely' + }) + await vi.waitFor(() => expect(mockCreateTab).toHaveBeenCalledOnce()) + await vi.waitFor(() => + expect(mockRuntimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ method: 'terminal.create' }) + ) + ) + state.tabsByWorktree['wt-1'] = [] + resolveCreate({ + ok: true, + result: { terminal: { handle: 'terminal-after-close', worktreeId: 'wt-1', title: null } } + }) + + await expect(launch).resolves.toBeNull() + expect(mockRuntimeEnvironmentCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.close', + params: { terminal: 'terminal-after-close' } + }) + ) + expect(mockUpdateTabPtyId).not.toHaveBeenCalled() + expect(mockRuntimeEnvironmentSubscribe).not.toHaveBeenCalled() + expect(mockDispatchEvent).not.toHaveBeenCalled() + }) + it('records effective launch config returned by local PTY spawn', async () => { const effectiveLaunchConfig = { agentCommand: "claude '--dangerously-skip-permissions'", diff --git a/src/renderer/src/lib/launch-agent-background-session.ts b/src/renderer/src/lib/launch-agent-background-session.ts index edb4c6d9f68..bb2f102f681 100644 --- a/src/renderer/src/lib/launch-agent-background-session.ts +++ b/src/renderer/src/lib/launch-agent-background-session.ts @@ -7,8 +7,7 @@ import type { import { getAgentLaunchPlatformForRepo } from '@/lib/agent-launch-platform' import { CLIENT_PLATFORM } from '@/lib/new-workspace' import { tuiAgentToAgentKind } from '@/lib/telemetry' -import { pasteDraftWhenAgentReady } from '@/lib/agent-paste-draft' -import { showAutomationPromptNotSentToast } from '@/lib/agent-background-session-timeout-toast' +import { scheduleAgentBackgroundDraft } from '@/lib/agent-background-draft-delivery' import { getLocalProjectExecutionRuntimeContext } from '@/lib/local-preflight-context' import { requestBackgroundTerminalWorktreeMount } from '@/components/terminal/background-terminal-worktree-mount' import { @@ -28,6 +27,7 @@ import { callRuntimeRpc, getActiveRuntimeTarget } from '@/runtime/runtime-rpc-cl import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner' import { toRuntimeWorktreeSelector } from '@/runtime/runtime-worktree-selector' import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' +import { retireProvider, retireUnownedTerminal } from '@/lib/retire-unowned-background-terminal' import { createBrowserUuid } from '@/lib/browser-uuid' import { subscribeToRuntimeTerminalData, @@ -115,12 +115,13 @@ export async function launchAgentBackgroundSession( const leafId = createBrowserUuid() const paneKey = makePaneKey(tab.id, leafId) const launchToken = createBrowserUuid() - store.registerAgentLaunchConfig(paneKey, startupPlan.launchConfig, { + const launchRegistration = { agentType: agent, launchToken, tabId: tab.id, leafId - }) + } + store.registerAgentLaunchConfig(paneKey, startupPlan.launchConfig, launchRegistration) // Why: `title` labels the tab/worktree entry. Pane titles render as an // in-terminal title row, so background sessions must not persist it there. store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId)) @@ -148,6 +149,7 @@ export async function launchAgentBackgroundSession( ) let ptyId = '' let runtimeTerminalHandle: string | null = null + let returnedLaunchConfig: typeof startupPlan.launchConfig | undefined let exitHandled = false let eagerPtyBuffer: EagerPtyHandle | null = null let unsubscribeExit = (): void => {}, @@ -236,14 +238,25 @@ export async function launchAgentBackgroundSession( } }) ptyId = result.id - if (result.launchConfig) { - store.registerAgentLaunchConfig(paneKey, result.launchConfig, { - agentType: agent, - launchToken, - tabId: tab.id, - leafId - }) - } + returnedLaunchConfig = result.launchConfig + } + if ( + await retireUnownedTerminal({ + tabId: tab.id, + ptyId, + runtimeTarget, + runtimeTerminalHandle, + onRetire: () => { + exitHandled = true + sshStartupDelivery.clear() + store.clearAgentLaunchConfig(paneKey) + } + }) + ) { + return null + } + if (returnedLaunchConfig) { + store.registerAgentLaunchConfig(paneKey, returnedLaunchConfig, launchRegistration) } store.updateTabPtyId(tab.id, ptyId) store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId, ptyId)) @@ -297,13 +310,7 @@ export async function launchAgentBackgroundSession( requestBackgroundTerminalWorktreeMount({ worktreeId, tabIds: [tab.id] }) if (pasteDraftAfterLaunch !== null) { - void pasteDraftWhenAgentReady({ - tabId: tab.id, - content: pasteDraftAfterLaunch, - agent, - submit: true, - onTimeout: () => showAutomationPromptNotSentToast(agent) - }) + scheduleAgentBackgroundDraft(tab.id, pasteDraftAfterLaunch, agent) } return { tabId: tab.id, paneKey, ptyId, startupPlan } @@ -317,17 +324,7 @@ export async function launchAgentBackgroundSession( runBestEffortAgentBackgroundCleanups(() => store.clearTabPtyId(tab.id, ptyId)) runBestEffortAgentBackgroundCleanups(() => store.clearAgentLaunchConfig(paneKey)) if (ptyId) { - try { - if (runtimeTarget.kind === 'environment' && runtimeTerminalHandle) { - await callRuntimeRpc(runtimeTarget, 'terminal.close', { - terminal: runtimeTerminalHandle - }) - } else if (runtimeTarget.kind === 'local') { - await window.api.pty.kill(ptyId) - } - } catch { - // Best-effort close; retiring the invalid hidden tab must still proceed. - } + await retireProvider({ ptyId, runtimeTarget, runtimeTerminalHandle }) } runBestEffortAgentBackgroundCleanups(() => store.closeTab(tab.id, { recordInteraction: false })) throw error diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts index a33671bf9df..10dce9d658a 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.test.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' const mockSpawn = vi.fn() +const mockKill = vi.fn() const mockCreateTab = vi.fn() const mockSetTabCustomTitle = vi.fn() const mockSetTabColor = vi.fn() @@ -31,6 +32,7 @@ const state = { } ] }, + tabsByWorktree: { 'wt-1': [] as { id: string }[] }, allWorktrees: vi.fn(() => state.worktreesByRepo['repo-1'] ?? []), createTab: mockCreateTab, setTabCustomTitle: mockSetTabCustomTitle, @@ -78,15 +80,24 @@ describe('launchWorktreeBackgroundTerminals', () => { displayName: 'Worktree' } ] + state.tabsByWorktree = { 'wt-1': [] } let tabIndex = 0 - mockCreateTab.mockImplementation(() => ({ id: `tab-${++tabIndex}` })) + mockCreateTab.mockImplementation(() => { + const tab = { id: `tab-${++tabIndex}` } + state.tabsByWorktree['wt-1'].push(tab) + return tab + }) + mockCloseTab.mockImplementation((tabId: string) => { + state.tabsByWorktree['wt-1'] = state.tabsByWorktree['wt-1'].filter((tab) => tab.id !== tabId) + }) let ptyIndex = 0 mockSpawn.mockImplementation(async () => ({ id: `pty-${++ptyIndex}` })) mockGetActiveRuntimeTarget.mockReturnValue({ kind: 'local' }) vi.stubGlobal('window', { api: { pty: { - spawn: mockSpawn + spawn: mockSpawn, + kill: mockKill } } }) @@ -288,4 +299,63 @@ describe('launchWorktreeBackgroundTerminals', () => { expect(mockCreateTab).not.toHaveBeenCalled() expect(mockSpawn).not.toHaveBeenCalled() }) + + it('kills a PTY whose tab is closed before the spawn resolves', async () => { + let resolveSpawn!: (result: { id: string }) => void + mockSpawn.mockReturnValueOnce( + new Promise<{ id: string }>((resolve) => { + resolveSpawn = resolve + }) + ) + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + const { launchWorktreeBackgroundTerminals } = + await import('./launch-worktree-background-terminals') + + const launch = launchWorktreeBackgroundTerminals({ + worktreeId: 'wt-1', + defaultTabs: { runCommands: true, tabs: [{ command: 'pnpm dev' }] } + }) + await vi.waitFor(() => expect(mockCreateTab).toHaveBeenCalledOnce()) + state.tabsByWorktree['wt-1'] = [] + resolveSpawn({ id: 'pty-after-close' }) + await launch + + expect(mockKill).toHaveBeenCalledWith('pty-after-close') + expect(mockUpdateTabPtyId).not.toHaveBeenCalled() + expect(mockRegisterEagerPtyBuffer).not.toHaveBeenCalled() + warn.mockRestore() + }) + + it('kills a late setup split PTY when its parent tab closes', async () => { + state.settings = { activeRuntimeEnvironmentId: null, setupScriptLaunchMode: 'split-horizontal' } + let resolveSetupSpawn!: (result: { id: string }) => void + mockSpawn.mockResolvedValueOnce({ id: 'pty-primary' }).mockReturnValueOnce( + new Promise<{ id: string }>((resolve) => { + resolveSetupSpawn = resolve + }) + ) + const { launchWorktreeBackgroundTerminals } = + await import('./launch-worktree-background-terminals') + + const launch = launchWorktreeBackgroundTerminals({ + worktreeId: 'wt-1', + setup: setupLaunch + }) + await vi.waitFor(() => expect(mockSpawn).toHaveBeenCalledTimes(2)) + state.closeTab('tab-1') + resolveSetupSpawn({ id: 'pty-setup-after-close' }) + await launch + + expect(mockKill).toHaveBeenCalledWith('pty-setup-after-close') + expect(mockUpdateTabPtyId).toHaveBeenCalledTimes(1) + expect(mockUpdateTabPtyId).toHaveBeenCalledWith('tab-1', 'pty-primary') + expect(mockSetTabLayout).not.toHaveBeenCalledWith( + 'tab-1', + expect.objectContaining({ + ptyIdsByLeafId: expect.objectContaining({ + '00000000-0000-4000-8000-000000000002': 'pty-setup-after-close' + }) + }) + ) + }) }) diff --git a/src/renderer/src/lib/launch-worktree-background-terminals.ts b/src/renderer/src/lib/launch-worktree-background-terminals.ts index 896da11f8e5..568f68e6f22 100644 --- a/src/renderer/src/lib/launch-worktree-background-terminals.ts +++ b/src/renderer/src/lib/launch-worktree-background-terminals.ts @@ -6,6 +6,7 @@ import { createBrowserUuid } from '@/lib/browser-uuid' import { getSettingsForWorktreeRuntimeOwner } from '@/lib/worktree-runtime-owner' import { getActiveRuntimeTarget } from '@/runtime/runtime-rpc-client' import { singlePaneLayoutSnapshot } from '@/store/slices/terminal-helpers' +import { retireUnownedTerminal } from '@/lib/retire-unowned-background-terminal' import { useAppStore } from '@/store' import { translate } from '@/i18n/i18n' import { isWindowsAbsolutePathLike } from '../../../shared/cross-platform-path' @@ -180,6 +181,15 @@ async function createBackgroundTab(args: { store.closeTab(tab.id, { recordInteraction: false }) throw error } + if ( + await retireUnownedTerminal({ + tabId: tab.id, + ptyId, + runtimeTarget: { kind: 'local' } + }) + ) { + throw new Error('The terminal tab was closed before its session finished starting.') + } store.updateTabPtyId(tab.id, ptyId) store.setTabLayout(tab.id, singlePaneLayoutSnapshot(leafId, ptyId)) registerBackgroundPaneBuffer(tab.id, leafId, ptyId) @@ -203,6 +213,15 @@ async function addSetupSplit(args: { command: buildSetupCommand(args.setup), env: args.setup.envVars }) + if ( + await retireUnownedTerminal({ + tabId: args.tab.tabId, + ptyId: setupPtyId, + runtimeTarget: { kind: 'local' } + }) + ) { + return + } store.updateTabPtyId(args.tab.tabId, setupPtyId) store.setTabLayout( args.tab.tabId, diff --git a/src/renderer/src/lib/retire-unowned-background-terminal.ts b/src/renderer/src/lib/retire-unowned-background-terminal.ts new file mode 100644 index 00000000000..7a7f128125a --- /dev/null +++ b/src/renderer/src/lib/retire-unowned-background-terminal.ts @@ -0,0 +1,38 @@ +import { useAppStore } from '@/store' +import { callRuntimeRpc, type RuntimeClientTarget } from '@/runtime/runtime-rpc-client' +import { isTerminalTabPresent } from '@/store/slices/terminal-tab-retirement' + +export async function retireUnownedTerminal(args: { + tabId: string + ptyId: string + runtimeTarget: RuntimeClientTarget + runtimeTerminalHandle?: string | null + onRetire?: () => void +}): Promise { + if (isTerminalTabPresent(useAppStore.getState(), args.tabId)) { + return false + } + // Why: close can win while provider creation is in flight, before the + // returned handle is bindable to store state or visible to tab retirement. + args.onRetire?.() + await retireProvider(args) + return true +} + +export async function retireProvider(args: { + ptyId: string + runtimeTarget: RuntimeClientTarget + runtimeTerminalHandle?: string | null +}): Promise { + try { + if (args.runtimeTarget.kind === 'environment' && args.runtimeTerminalHandle) { + await callRuntimeRpc(args.runtimeTarget, 'terminal.close', { + terminal: args.runtimeTerminalHandle + }) + } else if (args.runtimeTarget.kind === 'local') { + await window.api.pty.kill(args.ptyId) + } + } catch { + // Best-effort provider teardown; the retired tab must not be recreated. + } +} diff --git a/src/renderer/src/store/slices/agent-pane-authority.test.ts b/src/renderer/src/store/slices/agent-pane-authority.test.ts new file mode 100644 index 00000000000..a37f3057625 --- /dev/null +++ b/src/renderer/src/store/slices/agent-pane-authority.test.ts @@ -0,0 +1,125 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { makePaneKey } from '../../../../shared/stable-pane-id' +import { + resetAgentPaneAuthorityAliasesForTests, + resolveAgentPaneAuthorityKey +} from './agent-pane-authority' +import { createTestStore } from './store-test-helpers' + +const SOURCE = makePaneKey('tab-source', '11111111-1111-4111-8111-111111111111') +const TARGET = makePaneKey('tab-target', '22222222-2222-4222-8222-222222222222') +const FINAL = makePaneKey('tab-final', '33333333-3333-4333-8333-333333333333') +const SIBLING = makePaneKey('tab-target', '44444444-4444-4444-8444-444444444444') + +const retirePaneAuthority = vi.fn() +const transferPaneAuthority = vi.fn() +const dropByTabPrefix = vi.fn() + +beforeEach(() => { + resetAgentPaneAuthorityAliasesForTests() + vi.clearAllMocks() + vi.stubGlobal('window', { + api: { + agentStatus: { + retirePaneAuthority, + transferPaneAuthority, + dropByTabPrefix, + drop: vi.fn() + } + } + }) +}) + +afterEach(() => { + resetAgentPaneAuthorityAliasesForTests() + vi.unstubAllGlobals() +}) + +describe('agent pane authority', () => { + it('retires one pane, clears resume authority, and rejects late status without harming siblings', () => { + const store = createTestStore() + store.getState().setAgentStatus(TARGET, { state: 'working', prompt: 'target' }) + store.getState().setAgentStatus(SIBLING, { state: 'working', prompt: 'sibling' }) + store.getState().registerAgentLaunchConfig(TARGET, { agentArgs: '', agentEnv: {} }) + store.setState({ + sleepingAgentSessionsByPaneKey: { + [TARGET]: { + paneKey: TARGET, + tabId: 'tab-target', + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: 'session-1' }, + prompt: 'continue', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } + } + }) + + store.getState().retireAgentPaneAuthority(TARGET) + store.getState().setAgentStatus(TARGET, { state: 'done', prompt: 'late' }) + + const state = store.getState() + expect(state.agentStatusByPaneKey[TARGET]).toBeUndefined() + expect(state.agentLaunchConfigByPaneKey[TARGET]).toBeUndefined() + expect(state.sleepingAgentSessionsByPaneKey[TARGET]).toBeUndefined() + expect(state.agentStatusByPaneKey[SIBLING]).toBeDefined() + expect(state.recentlyRetiredAgentStatusPaneKeys[TARGET]).toBe(true) + expect(retirePaneAuthority).toHaveBeenCalledWith(TARGET) + }) + + it('keeps a physical pane routed through chained detaches until its current owner closes', () => { + const store = createTestStore() + store.getState().setAgentStatus(SOURCE, { state: 'working', prompt: 'source' }) + store.setState({ + sleepingAgentSessionsByPaneKey: { + [SOURCE]: { + paneKey: SOURCE, + tabId: 'tab-source', + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: 'session-1' }, + prompt: 'continue', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } + } + }) + + store + .getState() + .transferAgentPaneAuthority({ fromPaneKey: SOURCE, toPaneKey: TARGET, ptyId: 'pty-1' }) + store + .getState() + .transferAgentPaneAuthority({ fromPaneKey: TARGET, toPaneKey: FINAL, ptyId: 'pty-1' }) + store.getState().dropAgentStatusByTabPrefix('tab-source') + + expect(resolveAgentPaneAuthorityKey(SOURCE)).toBe(FINAL) + expect(store.getState().sleepingAgentSessionsByPaneKey[FINAL]).toMatchObject({ + paneKey: FINAL, + tabId: 'tab-final', + providerSession: { key: 'session_id', id: 'session-1' } + }) + store.getState().setAgentStatus(SOURCE, { state: 'working', prompt: 'after source close' }) + expect(store.getState().agentStatusByPaneKey[FINAL]?.prompt).toBe('after source close') + expect(transferPaneAuthority).toHaveBeenNthCalledWith(1, { + fromPaneKey: SOURCE, + toPaneKey: TARGET, + ptyId: 'pty-1' + }) + expect(transferPaneAuthority).toHaveBeenNthCalledWith(2, { + fromPaneKey: TARGET, + toPaneKey: FINAL, + ptyId: 'pty-1' + }) + + store.getState().dropAgentStatusByTabPrefix('tab-final') + store.getState().setAgentStatus(SOURCE, { state: 'done', prompt: 'too late' }) + + expect(store.getState().agentStatusByPaneKey[SOURCE]).toBeUndefined() + expect(store.getState().agentStatusByPaneKey[FINAL]).toBeUndefined() + expect(store.getState().recentlyRetiredAgentStatusPaneKeys[SOURCE]).toBe(true) + }) +}) diff --git a/src/renderer/src/store/slices/agent-pane-authority.ts b/src/renderer/src/store/slices/agent-pane-authority.ts new file mode 100644 index 00000000000..c5f25d0ec36 --- /dev/null +++ b/src/renderer/src/store/slices/agent-pane-authority.ts @@ -0,0 +1,86 @@ +import { parsePaneKey } from '../../../../shared/stable-pane-id' + +type AgentPaneAuthorityAlias = { + ownerPaneKey: string + ptyId: string | null +} + +const aliasesByPhysicalPaneKey = new Map() + +export type AgentPaneAuthorityTransfer = { + physicalPaneKey: string + previousOwnerPaneKey: string + ownerPaneKey: string + ptyId: string | null +} + +export function resolveAgentPaneAuthorityKey(paneKey: string): string { + return aliasesByPhysicalPaneKey.get(paneKey)?.ownerPaneKey ?? paneKey +} + +export function transferAgentPaneAuthorityAlias(args: { + fromPaneKey: string + toPaneKey: string + ptyId?: string | null +}): AgentPaneAuthorityTransfer | null { + if (!parsePaneKey(args.fromPaneKey) || !parsePaneKey(args.toPaneKey)) { + return null + } + const previousOwnerPaneKey = resolveAgentPaneAuthorityKey(args.fromPaneKey) + let physicalPaneKey = args.fromPaneKey + for (const [candidatePhysicalPaneKey, alias] of aliasesByPhysicalPaneKey) { + if ( + alias.ownerPaneKey === previousOwnerPaneKey && + (!args.ptyId || !alias.ptyId || alias.ptyId === args.ptyId) + ) { + physicalPaneKey = candidatePhysicalPaneKey + break + } + } + const ptyId = args.ptyId?.trim() || aliasesByPhysicalPaneKey.get(physicalPaneKey)?.ptyId || null + if (physicalPaneKey !== args.toPaneKey) { + // Why: the process keeps posting its original ORCA_PANE_KEY after detach; + // one physical-to-owner alias keeps chained moves on the current surface. + aliasesByPhysicalPaneKey.set(physicalPaneKey, { + ownerPaneKey: args.toPaneKey, + ptyId + }) + } + return { + physicalPaneKey, + previousOwnerPaneKey, + ownerPaneKey: args.toPaneKey, + ptyId + } +} + +export function retireAgentPaneAuthorityAliases(paneKey: string): string[] { + const ownerPaneKey = resolveAgentPaneAuthorityKey(paneKey) + const retiredPaneKeys = new Set([paneKey, ownerPaneKey]) + for (const [physicalPaneKey, alias] of aliasesByPhysicalPaneKey) { + if (physicalPaneKey === paneKey || alias.ownerPaneKey === ownerPaneKey) { + aliasesByPhysicalPaneKey.delete(physicalPaneKey) + retiredPaneKeys.add(physicalPaneKey) + retiredPaneKeys.add(alias.ownerPaneKey) + } + } + return [...retiredPaneKeys] +} + +export function retireAgentPaneAuthorityAliasesByOwnerTab(tabId: string): string[] { + const ownerPrefix = `${tabId}:` + const retiredPaneKeys = new Set() + for (const [physicalPaneKey, alias] of aliasesByPhysicalPaneKey) { + if (!alias.ownerPaneKey.startsWith(ownerPrefix)) { + continue + } + aliasesByPhysicalPaneKey.delete(physicalPaneKey) + retiredPaneKeys.add(physicalPaneKey) + retiredPaneKeys.add(alias.ownerPaneKey) + } + return [...retiredPaneKeys] +} + +export function resetAgentPaneAuthorityAliasesForTests(): void { + aliasesByPhysicalPaneKey.clear() +} diff --git a/src/renderer/src/store/slices/agent-status.ts b/src/renderer/src/store/slices/agent-status.ts index 10ab468ab38..f6a902d2e49 100644 --- a/src/renderer/src/store/slices/agent-status.ts +++ b/src/renderer/src/store/slices/agent-status.ts @@ -32,6 +32,12 @@ import { isOrcaDispatchPrompt, orchestrationLabelsMatchLiveDispatch } from '@/lib/agent-row-primary-text' +import { + resolveAgentPaneAuthorityKey, + retireAgentPaneAuthorityAliases, + retireAgentPaneAuthorityAliasesByOwnerTab, + transferAgentPaneAuthorityAlias +} from './agent-pane-authority' import { createFreshnessScheduler } from './agent-status-freshness-scheduler' /** Snapshot of a finished (or vanished) agent status entry, kept around so @@ -133,6 +139,16 @@ export type AgentStatusSlice = { * drop late in-flight IPC statuses and stale main-cache replays. */ recentlyClosedAgentStatusTabIds: Record + /** Exact pane authorities retired while sibling panes in the tab stay live. */ + recentlyRetiredAgentStatusPaneKeys: Record + + retireAgentPaneAuthority: (paneKey: string) => void + transferAgentPaneAuthority: (args: { + fromPaneKey: string + toPaneKey: string + ptyId?: string | null + }) => void + /** Update or insert an agent status entry from a status payload. */ setAgentStatus: ( paneKey: string, @@ -838,6 +854,7 @@ function getLaunchConfigForEntry( // so it must outlive the tab briefly — but tabId is ephemeral and it was only // ever added to, growing one entry per tab-close for the renderer's whole life. export const RECENTLY_CLOSED_AGENT_STATUS_TAB_IDS_MAX = 1024 +export const RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX = 1024 // delete-then-set for LRU recency, then evict the oldest keys past the cap (Record // key order is insertion order for non-integer string keys). A status event for a @@ -862,6 +879,58 @@ function boundRecentlyClosedAgentStatusTabIds( return next } +function boundRecentlyRetiredAgentStatusPaneKeys( + existing: Record, + paneKeys: readonly string[] +): Record { + const additions = new Set(paneKeys) + const next: Record = {} + for (const key of Object.keys(existing)) { + if (!additions.has(key)) { + next[key] = true + } + } + for (const paneKey of additions) { + next[paneKey] = true + } + const keys = Object.keys(next) + for (const stale of keys.slice(0, -RECENTLY_RETIRED_AGENT_STATUS_PANE_KEYS_MAX)) { + delete next[stale] + } + return next +} + +function movePaneKeyedRecord( + record: Record, + fromPaneKey: string, + toPaneKey: string, + transform: (value: T) => T = (value) => value +): Record { + const value = record[fromPaneKey] + if (value === undefined || fromPaneKey === toPaneKey) { + return record + } + const next = { ...record } + delete next[fromPaneKey] + next[toPaneKey] = transform(value) + return next +} + +function removePaneKeys( + record: Record, + paneKeys: ReadonlySet +): Record { + const matchingKeys = Object.keys(record).filter((key) => paneKeys.has(key)) + if (matchingKeys.length === 0) { + return record + } + const next = { ...record } + for (const key of matchingKeys) { + delete next[key] + } + return next +} + function getLaunchConfigForStatusMetadata( state: AppState, metadata: AgentLaunchConfigStatusMetadata @@ -1011,8 +1080,142 @@ export const createAgentStatusSlice: StateCreator freshness.schedule(), + retireAgentPaneAuthority: (paneKey) => { + const ownerPaneKey = resolveAgentPaneAuthorityKey(paneKey) + const retiredPaneKeys = retireAgentPaneAuthorityAliases(paneKey) + const retiredPaneKeySet = new Set(retiredPaneKeys) + let hadLive = false + set((s) => { + const retiredLivePaneKeys = retiredPaneKeys.filter((key) => key in s.agentStatusByPaneKey) + hadLive = retiredLivePaneKeys.length > 0 + let nextRetentionSuppressedPaneKeys = removePaneKeys( + s.retentionSuppressedPaneKeys, + retiredPaneKeySet + ) + if ( + retiredLivePaneKeys.length > 0 && + nextRetentionSuppressedPaneKeys === s.retentionSuppressedPaneKeys + ) { + nextRetentionSuppressedPaneKeys = { ...nextRetentionSuppressedPaneKeys } + } + for (const key of retiredLivePaneKeys) { + nextRetentionSuppressedPaneKeys[key] = true + } + return { + agentStatusByPaneKey: removePaneKeys(s.agentStatusByPaneKey, retiredPaneKeySet), + runtimeAgentOrchestrationByPaneKey: removePaneKeys( + s.runtimeAgentOrchestrationByPaneKey, + retiredPaneKeySet + ), + retainedAgentsByPaneKey: removePaneKeys(s.retainedAgentsByPaneKey, retiredPaneKeySet), + sleepingAgentSessionsByPaneKey: removePaneKeys( + s.sleepingAgentSessionsByPaneKey, + retiredPaneKeySet + ), + agentLaunchConfigByPaneKey: removePaneKeys( + s.agentLaunchConfigByPaneKey, + retiredPaneKeySet + ), + acknowledgedAgentsByPaneKey: removePaneKeys( + s.acknowledgedAgentsByPaneKey, + retiredPaneKeySet + ), + paneForegroundAgentByPaneKey: removePaneKeys( + s.paneForegroundAgentByPaneKey, + retiredPaneKeySet + ), + unreadTerminalPanes: removePaneKeys(s.unreadTerminalPanes, retiredPaneKeySet), + unreadAgentCompletionPanes: removePaneKeys( + s.unreadAgentCompletionPanes, + retiredPaneKeySet + ), + lastTerminalInputAtByPaneKey: removePaneKeys( + s.lastTerminalInputAtByPaneKey, + retiredPaneKeySet + ), + cacheTimerByKey: removePaneKeys(s.cacheTimerByKey, retiredPaneKeySet), + retentionSuppressedPaneKeys: nextRetentionSuppressedPaneKeys, + recentlyRetiredAgentStatusPaneKeys: boundRecentlyRetiredAgentStatusPaneKeys( + s.recentlyRetiredAgentStatusPaneKeys, + retiredPaneKeys + ), + agentStatusEpoch: hadLive ? s.agentStatusEpoch + 1 : s.agentStatusEpoch, + sortEpoch: hadLive ? s.sortEpoch + 1 : s.sortEpoch + } + }) + if (hadLive) { + queueMicrotask(() => freshness.schedule()) + } + if (typeof window !== 'undefined') { + window.api?.agentStatus?.retirePaneAuthority?.(ownerPaneKey) + } + }, + + transferAgentPaneAuthority: ({ fromPaneKey, toPaneKey, ptyId }) => { + const transfer = transferAgentPaneAuthorityAlias({ fromPaneKey, toPaneKey, ptyId }) + if (!transfer || transfer.previousOwnerPaneKey === transfer.ownerPaneKey) { + return + } + const from = transfer.previousOwnerPaneKey + const to = transfer.ownerPaneKey + const targetTabId = getTabIdFromPaneKey(to) ?? undefined + const targetLeafId = getLeafIdFromPaneKey(to) ?? undefined + set((s) => ({ + agentStatusByPaneKey: movePaneKeyedRecord(s.agentStatusByPaneKey, from, to, (entry) => ({ + ...entry, + paneKey: to, + tabId: targetTabId + })), + runtimeAgentOrchestrationByPaneKey: movePaneKeyedRecord( + s.runtimeAgentOrchestrationByPaneKey, + from, + to + ), + retainedAgentsByPaneKey: movePaneKeyedRecord( + s.retainedAgentsByPaneKey, + from, + to, + (retained) => ({ + ...retained, + entry: { ...retained.entry, paneKey: to, tabId: targetTabId }, + tab: targetTabId ? { ...retained.tab, id: targetTabId } : retained.tab + }) + ), + sleepingAgentSessionsByPaneKey: movePaneKeyedRecord( + s.sleepingAgentSessionsByPaneKey, + from, + to, + (record) => ({ ...record, paneKey: to, tabId: targetTabId }) + ), + agentLaunchConfigByPaneKey: movePaneKeyedRecord( + s.agentLaunchConfigByPaneKey, + from, + to, + (entry) => ({ + ...entry, + identity: { ...entry.identity, tabId: targetTabId, leafId: targetLeafId } + }) + ), + acknowledgedAgentsByPaneKey: movePaneKeyedRecord(s.acknowledgedAgentsByPaneKey, from, to), + paneForegroundAgentByPaneKey: movePaneKeyedRecord(s.paneForegroundAgentByPaneKey, from, to), + unreadTerminalPanes: movePaneKeyedRecord(s.unreadTerminalPanes, from, to), + unreadAgentCompletionPanes: movePaneKeyedRecord(s.unreadAgentCompletionPanes, from, to), + lastTerminalInputAtByPaneKey: movePaneKeyedRecord(s.lastTerminalInputAtByPaneKey, from, to), + cacheTimerByKey: movePaneKeyedRecord(s.cacheTimerByKey, from, to), + retentionSuppressedPaneKeys: movePaneKeyedRecord(s.retentionSuppressedPaneKeys, from, to) + })) + if (typeof window !== 'undefined') { + window.api?.agentStatus?.transferPaneAuthority?.({ + fromPaneKey: from, + toPaneKey: to, + ...(transfer.ptyId ? { ptyId: transfer.ptyId } : {}) + }) + } + }, + setRuntimeAgentOrchestrationByPaneKey: (entries) => { const generatedTitleUpdates: AgentStatusEntry[] = [] set((s) => { @@ -1180,8 +1383,10 @@ export const createAgentStatusSlice: StateCreator { + paneKey = resolveAgentPaneAuthorityKey(paneKey) const updatedAt = timing?.updatedAt ?? Date.now() if ( + paneKey in get().recentlyRetiredAgentStatusPaneKeys || // Why: a closed terminal tab is no longer a valid destination for hook // replays or late status events, even if main still receives them. isRecentlyClosedAgentStatusTab( @@ -1893,6 +2098,7 @@ export const createAgentStatusSlice: StateCreator { const prefix = `${tabIdPrefix}:` + const retiredAliasPaneKeys = retireAgentPaneAuthorityAliasesByOwnerTab(tabIdPrefix) let hadLive = false set((s) => { const completedOrphanKeys = findCompletedOrphanPaneKeysForTabClose( @@ -1932,6 +2138,10 @@ export const createAgentStatusSlice: StateCreator 0 @@ -2004,6 +2218,7 @@ export const createAgentStatusSlice: StateCreator void closeUnifiedTab: ( tabId: string, - opts?: { recordInteraction?: boolean } + opts?: { recordInteraction?: boolean; terminalRetirementHandled?: boolean } ) => { closedTabId: string; wasLastTab: boolean; worktreeId: string } | null reorderUnifiedTabs: ( groupId: string, @@ -899,6 +899,15 @@ export const createTabsSlice: StateCreator = (set, return null } + if (tab.contentType === 'terminal' && !opts?.terminalRetirementHandled) { + const dedupedGroupOrder = dedupeTabOrder(group.tabOrder) + const wasLastTab = dedupeTabOrder(dedupedGroupOrder.filter((id) => id !== tabId)).length === 0 + // Why: unified-only hydrated tabs still own provider sessions even when + // their legacy terminal row is missing, so every terminal close retires by entity id. + get().closeTab(tab.entityId, { recordInteraction: opts?.recordInteraction }) + return { closedTabId: tabId, wasLastTab, worktreeId } + } + const dedupedGroupOrder = dedupeTabOrder(group.tabOrder) const remainingOrder = dedupeTabOrder(dedupedGroupOrder.filter((id) => id !== tabId)) const wasLastTab = remainingOrder.length === 0 diff --git a/src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts b/src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts index 5afdcb6fe43..6d12b987b30 100644 --- a/src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts +++ b/src/renderer/src/store/slices/terminal-pane-detach-agent-identity.test.ts @@ -3,7 +3,7 @@ import { resolveWindowsShiftEnterEncodingForPane } from '@/components/terminal-p import { createTestStore, makeTab, makeWorktree, seedStore } from './store-test-helpers' describe('syncPaneDetachPtyOwnership agent identity', () => { - it('moves process and launch identity to the detached leaf without forging hook identity', () => { + it('moves process, hook, launch, and resume authority to the detached leaf', () => { const store = createTestStore() const worktreeId = 'repo::/repo/worktree' const sourceTabId = 'tab-source' @@ -49,6 +49,21 @@ describe('syncPaneDetachPtyOwnership agent identity', () => { prompt: '', agentType: 'droid' }) + store.setState({ + sleepingAgentSessionsByPaneKey: { + [sourcePaneKey]: { + paneKey: sourcePaneKey, + tabId: sourceTabId, + worktreeId, + agent: 'codex', + providerSession: { key: 'session_id', id: 'session-1' }, + prompt: 'continue', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } + } + }) store.getState().syncPaneDetachPtyOwnership({ detachedLeafId, @@ -76,11 +91,19 @@ describe('syncPaneDetachPtyOwnership agent identity', () => { tabId: targetTabId, leafId: detachedLeafId }) - // Why: the PTY keeps emitting its immutable source ORCA_PANE_KEY. A copied - // target hook row would look live but could never receive later updates. expect(state.agentStatusByPaneKey[sourcePaneKey]).toBeUndefined() - expect(state.agentStatusByPaneKey[targetPaneKey]).toBeUndefined() - expect(state.retentionSuppressedPaneKeys[sourcePaneKey]).toBe(true) + expect(state.agentStatusByPaneKey[targetPaneKey]).toMatchObject({ + paneKey: targetPaneKey, + tabId: targetTabId, + state: 'working' + }) + expect(state.sleepingAgentSessionsByPaneKey[sourcePaneKey]).toBeUndefined() + expect(state.sleepingAgentSessionsByPaneKey[targetPaneKey]).toMatchObject({ + paneKey: targetPaneKey, + tabId: targetTabId, + providerSession: { key: 'session_id', id: 'session-1' } + }) + expect(state.retentionSuppressedPaneKeys[sourcePaneKey]).toBeUndefined() expect(state.paneForegroundAgentByPaneKey[siblingPaneKey]).toEqual({ agent: 'antigravity', shellForeground: false diff --git a/src/renderer/src/store/slices/terminal-retirement-teardown-reservation.ts b/src/renderer/src/store/slices/terminal-retirement-teardown-reservation.ts new file mode 100644 index 00000000000..79a9e3b3161 --- /dev/null +++ b/src/renderer/src/store/slices/terminal-retirement-teardown-reservation.ts @@ -0,0 +1,33 @@ +import { + getTerminalPtyOwnershipIdentity, + type TerminalTabRetirementPlan, + type TerminalTabRetirementState +} from './terminal-tab-retirement' + +export function reserveTerminalRetirementTeardowns( + state: TerminalTabRetirementState, + plan: TerminalTabRetirementPlan, + scheduledPtyOwners: Set +): { plan: TerminalTabRetirementPlan; newlyScheduledPtyOwners: string[] } { + const cleanupOnlyPtyIds = new Set(plan.cleanupOnlyPtyIds) + const newlyScheduledPtyOwners: string[] = [] + const reserve = (ptyId: string): boolean => { + const owner = getTerminalPtyOwnershipIdentity(state, ptyId, plan.worktreeId) + if (scheduledPtyOwners.has(owner)) { + cleanupOnlyPtyIds.add(ptyId) + return false + } + scheduledPtyOwners.add(owner) + newlyScheduledPtyOwners.push(owner) + return true + } + return { + plan: { + ...plan, + localOrSshPtyIds: plan.localOrSshPtyIds.filter(reserve), + runtimeTerminals: plan.runtimeTerminals.filter((terminal) => reserve(terminal.ptyId)), + cleanupOnlyPtyIds: [...cleanupOnlyPtyIds] + }, + newlyScheduledPtyOwners + } +} diff --git a/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts b/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts new file mode 100644 index 00000000000..13df54d1d9f --- /dev/null +++ b/src/renderer/src/store/slices/terminal-tab-retirement-store.test.ts @@ -0,0 +1,259 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' + +const mockKill = vi.fn().mockResolvedValue(undefined) +const mockRuntimeCall = vi.fn().mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: {}, + _meta: { runtimeId: 'local-runtime' } +}) + +vi.stubGlobal('window', { + api: { + pty: { kill: mockKill }, + runtime: { call: mockRuntimeCall }, + runtimeEnvironments: { call: vi.fn() } + } +}) + +import { + capturedPanesByTabId, + parkedWatchersByTabId +} from '@/components/terminal-pane/terminal-parked-watcher-registry' +import { + createTestStore, + makeTab, + makeTabGroup, + makeUnifiedTab, + seedStore +} from './store-test-helpers' + +function sleepingRecord(paneKey: string, tabId: string): SleepingAgentSessionRecord { + return { + paneKey, + tabId, + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: paneKey }, + prompt: 'continue', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } +} + +describe('terminal tab retirement store boundary', () => { + beforeEach(() => { + vi.clearAllMocks() + mockKill.mockResolvedValue(undefined) + mockRuntimeCall.mockResolvedValue({ + id: 'rpc-1', + ok: true, + result: {}, + _meta: { runtimeId: 'local-runtime' } + }) + parkedWatchersByTabId.clear() + capturedPanesByTabId.clear() + }) + + it('retires split, relay, deferred, and pending sessions for a parked tab', async () => { + const store = createTestStore() + const dispose = vi.fn() + const siblingRecord = sleepingRecord('tab-2:leaf-2', 'tab-2') + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-primary' })] + }, + ptyIdsByTabId: { 'tab-1': ['pty-primary', 'pty-split'] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { leaf1: 'pty-primary', leaf2: 'pty-split' } + } + }, + lastKnownRelayPtyIdByTabId: { 'tab-1': 'ssh:ssh-1@@relay' }, + deferredSshSessionIdsByTabId: { 'tab-1': 'pty-deferred' }, + pendingReconnectPtyIdByTabId: { 'tab-1': 'pty-pending' }, + sleepingAgentSessionsByPaneKey: { + 'tab-1:leaf-1': sleepingRecord('tab-1:leaf-1', 'tab-1'), + 'legacy-key': sleepingRecord('legacy-key', 'tab-1'), + 'tab-2:leaf-2': siblingRecord + } + }) + parkedWatchersByTabId.set('tab-1', { + worktreeId: 'wt-1', + tabPtyId: 'pty-primary', + paneIdByPtyId: new Map([['pty-primary', 1]]), + disposersByPtyId: new Map([['pty-primary', dispose]]) + }) + capturedPanesByTabId.set('tab-1', { worktreeId: 'wt-1', panes: [] }) + + store.getState().closeTab('tab-1') + await vi.waitFor(() => expect(mockKill).toHaveBeenCalledTimes(5)) + + expect(new Set(mockKill.mock.calls.map(([ptyId]) => ptyId))).toEqual( + new Set(['pty-primary', 'pty-split', 'ssh:ssh-1@@relay', 'pty-deferred', 'pty-pending']) + ) + expect(store.getState().tabsByWorktree['wt-1']).toEqual([]) + expect(store.getState().deferredSshSessionIdsByTabId['tab-1']).toBeUndefined() + expect(store.getState().pendingReconnectPtyIdByTabId['tab-1']).toBeUndefined() + expect(store.getState().sleepingAgentSessionsByPaneKey).toEqual({ + 'tab-2:leaf-2': siblingRecord + }) + expect(store.getState().sleepingAgentSessionsByPaneKey['tab-2:leaf-2']).toBe(siblingRecord) + expect(dispose).toHaveBeenCalledOnce() + expect(parkedWatchersByTabId.has('tab-1')).toBe(false) + expect(capturedPanesByTabId.has('tab-1')).toBe(false) + }) + + it('routes runtime handles to runtime close and preserves shared PTYs', async () => { + const store = createTestStore() + seedStore(store, { + tabsByWorktree: { + 'wt-1': [ + makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'remote:terminal-1' }), + makeTab({ id: 'tab-2', worktreeId: 'wt-1', ptyId: 'pty-shared' }) + ] + }, + ptyIdsByTabId: { + 'tab-1': ['remote:terminal-1', 'pty-shared'], + 'tab-2': ['pty-shared'] + } + }) + + store.getState().closeTab('tab-1') + await vi.waitFor(() => expect(mockRuntimeCall).toHaveBeenCalled()) + + expect(mockRuntimeCall).toHaveBeenCalledWith({ + method: 'terminal.close', + params: { terminal: 'terminal-1' } + }) + expect(mockKill).not.toHaveBeenCalled() + }) + + it('preserves shared-owner snapshots while closing the source tab', async () => { + const store = createTestStore() + const snapshot = { snapshot: 'shared snapshot' } + const coldRestore = { scrollback: 'shared scrollback', cwd: 'C:\\workspace' } + seedStore(store, { + tabsByWorktree: { + 'wt-1': [ + makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-shared' }), + makeTab({ id: 'tab-2', worktreeId: 'wt-1', ptyId: 'pty-shared' }) + ] + }, + ptyIdsByTabId: { 'tab-1': ['pty-shared'], 'tab-2': ['pty-shared'] }, + pendingSnapshotByPtyId: { 'pty-shared': snapshot }, + pendingColdRestoreByPtyId: { 'pty-shared': coldRestore } + }) + + store.getState().closeTab('tab-1') + await Promise.resolve() + + expect(mockKill).not.toHaveBeenCalled() + expect(store.getState().pendingSnapshotByPtyId['pty-shared']).toBe(snapshot) + expect(store.getState().pendingColdRestoreByPtyId['pty-shared']).toBe(coldRestore) + }) + + it('reconciles natural exit without issuing teardown or revoking resume authority', async () => { + const store = createTestStore() + const record = sleepingRecord('tab-1:leaf-1', 'tab-1') + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-dead' })] + }, + ptyIdsByTabId: { 'tab-1': ['pty-dead'] }, + sleepingAgentSessionsByPaneKey: { 'tab-1:leaf-1': record } + }) + + store.getState().closeTab('tab-1', { reason: 'pty-exit' }) + await Promise.resolve() + + expect(mockKill).not.toHaveBeenCalled() + expect(mockRuntimeCall).not.toHaveBeenCalled() + expect(store.getState().sleepingAgentSessionsByPaneKey['tab-1:leaf-1']).toBe(record) + }) + + it('does not recreate PTY indexes for a tab that no longer exists', () => { + const store = createTestStore() + + store.getState().updateTabPtyId('closed-tab', 'pty-after-close') + + expect(store.getState().ptyIdsByTabId['closed-tab']).toBeUndefined() + expect(store.getState().lastKnownRelayPtyIdByTabId['closed-tab']).toBeUndefined() + }) + + it('retires a unified-only terminal instead of removing only its wrapper', async () => { + const store = createTestStore() + const unified = makeUnifiedTab({ + id: 'unified-tab-1', + entityId: 'terminal-tab-1', + worktreeId: 'wt-1', + groupId: 'group-1' + }) + seedStore(store, { + tabsByWorktree: { 'wt-1': [] }, + unifiedTabsByWorktree: { 'wt-1': [unified] }, + groupsByWorktree: { + 'wt-1': [ + makeTabGroup({ + id: 'group-1', + worktreeId: 'wt-1', + activeTabId: unified.id, + tabOrder: [unified.id] + }) + ] + }, + ptyIdsByTabId: { 'terminal-tab-1': ['pty-unified-only'] } + }) + + store.getState().closeUnifiedTab(unified.id) + await vi.waitFor(() => expect(mockKill).toHaveBeenCalledWith('pty-unified-only')) + + expect(store.getState().unifiedTabsByWorktree['wt-1']).toEqual([]) + expect(store.getState().ptyIdsByTabId['terminal-tab-1']).toBeUndefined() + }) + + it('lets a paired host own runtime teardown while pruning local state', async () => { + const store = createTestStore() + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'remote:terminal-1' })] + }, + ptyIdsByTabId: { 'tab-1': ['remote:terminal-1'] } + }) + + store.getState().closeTab('tab-1', { remoteCloseOwnedByHost: true }) + await Promise.resolve() + + expect(mockRuntimeCall).not.toHaveBeenCalled() + expect(store.getState().tabsByWorktree['wt-1']).toEqual([]) + }) + + it('keeps the tab retired and reports provider rejection without an unhandled promise', async () => { + const store = createTestStore() + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}) + mockKill.mockRejectedValueOnce(new Error('provider unavailable')) + seedStore(store, { + tabsByWorktree: { + 'wt-1': [makeTab({ id: 'tab-1', worktreeId: 'wt-1', ptyId: 'pty-1' })] + }, + ptyIdsByTabId: { 'tab-1': ['pty-1'] } + }) + + store.getState().closeTab('tab-1') + await vi.waitFor(() => + expect(warn).toHaveBeenCalledWith('[terminal-retirement] provider teardown failed', { + tabId: 'tab-1', + localOrSshFailures: 1, + runtimeFailures: 0 + }) + ) + + expect(store.getState().tabsByWorktree['wt-1']).toEqual([]) + warn.mockRestore() + }) +}) diff --git a/src/renderer/src/store/slices/terminal-tab-retirement.test.ts b/src/renderer/src/store/slices/terminal-tab-retirement.test.ts new file mode 100644 index 00000000000..1203a3937a2 --- /dev/null +++ b/src/renderer/src/store/slices/terminal-tab-retirement.test.ts @@ -0,0 +1,270 @@ +import { describe, expect, it } from 'vitest' +import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' +import type { TerminalTab } from '../../../../shared/types' +import { + buildTerminalTabRetirementPlan, + buildTerminalTabRetirementPlans, + isTerminalTabPresent, + removeSleepingAgentSessionsForTab +} from './terminal-tab-retirement' + +type RetirementState = Parameters[0] + +function makeTab(id: string, worktreeId: string, ptyId: string | null): TerminalTab { + return { + id, + worktreeId, + ptyId, + title: id, + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0 + } +} + +function makeState(overrides: Partial = {}): RetirementState { + return { + tabsByWorktree: {}, + unifiedTabsByWorktree: {}, + ptyIdsByTabId: {}, + terminalLayoutsByTabId: {}, + lastKnownRelayPtyIdByTabId: {}, + deferredSshSessionIdsByTabId: {}, + pendingReconnectPtyIdByTabId: {}, + ...overrides + } +} + +function makeSleepingRecord(paneKey: string, tabId?: string): SleepingAgentSessionRecord { + return { + paneKey, + tabId, + worktreeId: 'wt-1', + agent: 'codex', + providerSession: { key: 'session_id', id: paneKey }, + prompt: 'continue', + state: 'working', + capturedAt: 1, + updatedAt: 1 + } +} + +describe('terminal tab retirement planning', () => { + it('collects and deduplicates every ownership source before routing providers', () => { + const state = makeState({ + tabsByWorktree: { + 'wt-1': [makeTab('tab-1', 'wt-1', 'pty-row')] + }, + ptyIdsByTabId: { 'tab-1': ['pty-index', 'pty-row'] }, + terminalLayoutsByTabId: { + 'tab-1': { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { + leaf1: 'pty-layout', + leaf2: 'remote:env-1@@terminal-1' + } + } + }, + lastKnownRelayPtyIdByTabId: { 'tab-1': 'ssh:ssh-1@@relay-pty' }, + deferredSshSessionIdsByTabId: { 'tab-1': 'pty-deferred' }, + pendingReconnectPtyIdByTabId: { 'tab-1': 'pty-pending' } + }) + + expect(buildTerminalTabRetirementPlan(state, 'tab-1')).toEqual({ + tabId: 'tab-1', + worktreeId: 'wt-1', + ptyIds: [ + 'pty-index', + 'pty-row', + 'pty-layout', + 'remote:env-1@@terminal-1', + 'ssh:ssh-1@@relay-pty', + 'pty-deferred', + 'pty-pending' + ], + localOrSshPtyIds: [ + 'pty-index', + 'pty-row', + 'pty-layout', + 'ssh:ssh-1@@relay-pty', + 'pty-deferred', + 'pty-pending' + ], + runtimeTerminals: [ + { + ptyId: 'remote:env-1@@terminal-1', + environmentId: 'env-1', + handle: 'terminal-1' + } + ], + cleanupOnlyPtyIds: [], + sharedPtyIds: [], + unroutablePtyIds: [] + }) + expect(isTerminalTabPresent(state, 'tab-1')).toBe(true) + }) + + it('does not retire a PTY still referenced by another live surface', () => { + const shared = 'pty-in-transfer' + const state = makeState({ + tabsByWorktree: { + 'wt-1': [makeTab('tab-1', 'wt-1', shared), makeTab('tab-2', 'wt-1', null)] + }, + ptyIdsByTabId: { 'tab-1': [shared], 'tab-2': [shared] }, + terminalLayoutsByTabId: { + 'tab-2': { + root: null, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { leaf2: shared } + } + } + }) + + const plan = buildTerminalTabRetirementPlan(state, 'tab-1') + expect(plan.sharedPtyIds).toEqual([shared]) + expect(plan.localOrSshPtyIds).toEqual([]) + expect(plan.runtimeTerminals).toEqual([]) + }) + + it('protects a scoped runtime terminal referenced through its legacy alias', () => { + const scoped = 'remote:env-1@@terminal-1' + const legacy = 'remote:terminal-1' + const state = makeState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + tabsByWorktree: { + 'wt-1': [makeTab('tab-1', 'wt-1', legacy)], + 'wt-2': [makeTab('tab-2', 'wt-2', scoped)] + }, + ptyIdsByTabId: { 'tab-1': [legacy], 'tab-2': [scoped] } + }) + + const plan = buildTerminalTabRetirementPlan(state, 'tab-1') + expect(plan.sharedPtyIds).toEqual([legacy]) + expect(plan.runtimeTerminals).toEqual([]) + }) + + it('deduplicates legacy and scoped aliases owned by the closing tab', () => { + const state = makeState({ + settings: { activeRuntimeEnvironmentId: 'env-1' }, + tabsByWorktree: { + 'wt-1': [makeTab('tab-1', 'wt-1', 'remote:terminal-1')] + }, + ptyIdsByTabId: { + 'tab-1': ['remote:terminal-1', 'remote:env-1@@terminal-1'] + } + }) + + expect(buildTerminalTabRetirementPlan(state, 'tab-1').runtimeTerminals).toEqual([ + { + ptyId: 'remote:terminal-1', + environmentId: null, + handle: 'terminal-1' + } + ]) + }) + + it('ignores stale ownership maps and never routes malformed remote ids locally', () => { + const malformedRemote = 'remote:' + const state = makeState({ + tabsByWorktree: { + 'wt-1': [makeTab('tab-1', 'wt-1', malformedRemote)] + }, + ptyIdsByTabId: { + 'tab-1': [malformedRemote, 'pty-live'], + 'stale-tab': ['pty-live'] + } + }) + + const plan = buildTerminalTabRetirementPlan(state, 'tab-1') + expect(plan.unroutablePtyIds).toEqual([malformedRemote]) + expect(plan.localOrSshPtyIds).toEqual(['pty-live']) + expect(plan.sharedPtyIds).toEqual([]) + }) + + it('deduplicates batch-owned PTYs while protecting owners outside the close set', () => { + const state = makeState({ + tabsByWorktree: { + 'wt-1': [ + makeTab('tab-1', 'wt-1', 'pty-batch'), + makeTab('tab-2', 'wt-1', 'pty-batch'), + makeTab('later-tab', 'wt-1', 'pty-external') + ] + }, + ptyIdsByTabId: { + 'tab-1': ['pty-batch', 'pty-external'], + 'tab-2': ['pty-batch'], + 'later-tab': ['pty-external'] + } + }) + + const plans = buildTerminalTabRetirementPlans(state, ['tab-1', 'tab-2']) + + expect(plans.get('tab-1')).toMatchObject({ + localOrSshPtyIds: ['pty-batch'], + sharedPtyIds: ['pty-external'] + }) + expect(plans.get('tab-2')).toMatchObject({ + localOrSshPtyIds: [], + cleanupOnlyPtyIds: ['pty-batch'], + sharedPtyIds: [] + }) + }) + + it('indexes live owners once for a 100-tab batch', () => { + const tabs = Array.from({ length: 100 }, (_, index) => + makeTab(`tab-${index}`, 'wt-1', `pty-${index}`) + ) + let terminalStoreScans = 0 + let unifiedStoreScans = 0 + const state = makeState({ + tabsByWorktree: new Proxy( + { 'wt-1': tabs }, + { + ownKeys(target) { + terminalStoreScans += 1 + return Reflect.ownKeys(target) + } + } + ), + unifiedTabsByWorktree: new Proxy( + {}, + { + ownKeys(target) { + unifiedStoreScans += 1 + return Reflect.ownKeys(target) + } + } + ), + ptyIdsByTabId: Object.fromEntries(tabs.map((tab, index) => [tab.id, [`pty-${index}`]])) + }) + + const plans = buildTerminalTabRetirementPlans( + state, + tabs.map((tab) => tab.id) + ) + + expect(plans).toHaveLength(100) + expect(terminalStoreScans).toBe(1) + expect(unifiedStoreScans).toBe(1) + }) +}) + +describe('sleeping agent retirement', () => { + it('removes key- and metadata-owned records while preserving siblings by reference', () => { + const sibling = makeSleepingRecord('tab-2:leaf-2', 'tab-2') + const records = { + 'tab-1:leaf-1': makeSleepingRecord('tab-1:leaf-1'), + 'legacy-pane-key': makeSleepingRecord('legacy-pane-key', 'tab-1'), + 'tab-2:leaf-2': sibling + } + + const next = removeSleepingAgentSessionsForTab(records, 'tab-1') + expect(next).toEqual({ 'tab-2:leaf-2': sibling }) + expect(next['tab-2:leaf-2']).toBe(sibling) + expect(removeSleepingAgentSessionsForTab(next, 'missing-tab')).toBe(next) + }) +}) diff --git a/src/renderer/src/store/slices/terminal-tab-retirement.ts b/src/renderer/src/store/slices/terminal-tab-retirement.ts new file mode 100644 index 00000000000..e7b26582e7b --- /dev/null +++ b/src/renderer/src/store/slices/terminal-tab-retirement.ts @@ -0,0 +1,221 @@ +import type { SleepingAgentSessionRecord } from '../../../../shared/agent-session-resume' +import type { AppState } from '../types' +import { + getRuntimeEnvironmentIdForWorktree, + type WorktreeRuntimeOwnerState +} from '@/lib/worktree-runtime-owner' +import { parseRemoteRuntimePtyId } from '@/runtime/runtime-terminal-stream' + +export type TerminalTabCloseReason = 'user' | 'cleanup' | 'pty-exit' + +export type TerminalTabRetirementState = WorktreeRuntimeOwnerState & + Pick< + AppState, + | 'tabsByWorktree' + | 'unifiedTabsByWorktree' + | 'ptyIdsByTabId' + | 'terminalLayoutsByTabId' + | 'lastKnownRelayPtyIdByTabId' + | 'deferredSshSessionIdsByTabId' + | 'pendingReconnectPtyIdByTabId' + > + +export type TerminalTabRetirementPlan = { + tabId: string + worktreeId: string | null + ptyIds: string[] + localOrSshPtyIds: string[] + runtimeTerminals: { + ptyId: string + environmentId: string | null + handle: string + }[] + cleanupOnlyPtyIds: string[] + sharedPtyIds: string[] + unroutablePtyIds: string[] +} + +function appendPtyId(ids: Set, ptyId: string | null | undefined): void { + if (ptyId) { + ids.add(ptyId) + } +} + +export function getTerminalPtyOwnershipIdentity( + state: TerminalTabRetirementState, + ptyId: string, + worktreeId: string | null +): string { + const remote = parseRemoteRuntimePtyId(ptyId) + if (!remote?.handle) { + return `pty:${ptyId}` + } + // Why: hydrated legacy runtime IDs omit their owner, but still refer to the + // same provider terminal as a scoped ID in the owning worktree. + const environmentId = + remote.environmentId?.trim() || getRuntimeEnvironmentIdForWorktree(state, worktreeId) || '' + return JSON.stringify(['runtime', environmentId, remote.handle]) +} + +function collectPtyIdsForTab( + state: TerminalTabRetirementState, + tabId: string, + rowPtyId: string | null | undefined +): string[] { + const ids = new Set() + for (const ptyId of state.ptyIdsByTabId[tabId] ?? []) { + appendPtyId(ids, ptyId) + } + appendPtyId(ids, rowPtyId) + for (const ptyId of Object.values(state.terminalLayoutsByTabId[tabId]?.ptyIdsByLeafId ?? {})) { + appendPtyId(ids, ptyId) + } + appendPtyId(ids, state.lastKnownRelayPtyIdByTabId[tabId]) + appendPtyId(ids, state.deferredSshSessionIdsByTabId[tabId]) + appendPtyId(ids, state.pendingReconnectPtyIdByTabId[tabId]) + return [...ids] +} + +function collectLiveTerminalTabs( + state: TerminalTabRetirementState +): Map { + const liveTabs = new Map() + for (const [worktreeId, tabs] of Object.entries(state.tabsByWorktree)) { + for (const tab of tabs) { + liveTabs.set(tab.id, { worktreeId, rowPtyId: tab.ptyId }) + } + } + for (const [worktreeId, tabs] of Object.entries(state.unifiedTabsByWorktree)) { + for (const tab of tabs) { + if (tab.contentType === 'terminal' && !liveTabs.has(tab.entityId)) { + liveTabs.set(tab.entityId, { worktreeId, rowPtyId: null }) + } + } + } + return liveTabs +} + +function hasOwnerOutsideTargets( + ownerTabIds: ReadonlySet | undefined, + targetIds: ReadonlySet +): boolean { + for (const ownerTabId of ownerTabIds ?? []) { + if (!targetIds.has(ownerTabId)) { + return true + } + } + return false +} + +export function isTerminalTabPresent( + state: Pick, + tabId: string +): boolean { + return Object.values(state.tabsByWorktree).some((tabs) => tabs.some((tab) => tab.id === tabId)) +} + +export function buildTerminalTabRetirementPlan( + state: TerminalTabRetirementState, + tabId: string +): TerminalTabRetirementPlan { + return buildTerminalTabRetirementPlans(state, [tabId]).get(tabId)! +} + +export function buildTerminalTabRetirementPlans( + state: TerminalTabRetirementState, + tabIds: readonly string[] +): Map { + const targetIds = [...new Set(tabIds)] + const targetIdSet = new Set(targetIds) + const liveTabs = collectLiveTerminalTabs(state) + const ptyIdsByLiveTab = new Map() + const ownerTabIdsByIdentity = new Map>() + + // Why: bulk close must not rebuild the live-owner index after every tab; + // one snapshot keeps planning linear while preserving shared-surface safety. + for (const [tabId, owner] of liveTabs) { + const ptyIds = collectPtyIdsForTab(state, tabId, owner.rowPtyId) + ptyIdsByLiveTab.set(tabId, ptyIds) + for (const ptyId of ptyIds) { + const identity = getTerminalPtyOwnershipIdentity(state, ptyId, owner.worktreeId) + const owners = ownerTabIdsByIdentity.get(identity) ?? new Set() + owners.add(tabId) + ownerTabIdsByIdentity.set(identity, owners) + } + } + + const plans = new Map() + const scheduledPtyOwners = new Set() + for (const tabId of targetIds) { + const owner = liveTabs.get(tabId) + const worktreeId = owner?.worktreeId ?? null + const ptyIds = + ptyIdsByLiveTab.get(tabId) ?? collectPtyIdsForTab(state, tabId, owner?.rowPtyId ?? null) + const sharedPtyIds: string[] = [] + const localOrSshPtyIds: string[] = [] + const runtimeTerminals: TerminalTabRetirementPlan['runtimeTerminals'] = [] + const cleanupOnlyPtyIds: string[] = [] + const unroutablePtyIds: string[] = [] + + for (const ptyId of ptyIds) { + const ownerIdentity = getTerminalPtyOwnershipIdentity(state, ptyId, worktreeId) + const ownerTabIds = ownerTabIdsByIdentity.get(ownerIdentity) + if (hasOwnerOutsideTargets(ownerTabIds, targetIdSet)) { + sharedPtyIds.push(ptyId) + continue + } + if (scheduledPtyOwners.has(ownerIdentity)) { + // Why: another closing tab already owns provider teardown, but this + // tab can still hold alias-keyed snapshots that must be discarded. + cleanupOnlyPtyIds.push(ptyId) + continue + } + scheduledPtyOwners.add(ownerIdentity) + const remote = parseRemoteRuntimePtyId(ptyId) + if (remote) { + if (!remote.handle) { + unroutablePtyIds.push(ptyId) + continue + } + runtimeTerminals.push({ + ptyId, + environmentId: remote.environmentId?.trim() || null, + handle: remote.handle + }) + } else if (ptyId.startsWith('remote:')) { + unroutablePtyIds.push(ptyId) + } else { + localOrSshPtyIds.push(ptyId) + } + } + + plans.set(tabId, { + tabId, + worktreeId, + ptyIds, + localOrSshPtyIds, + runtimeTerminals, + cleanupOnlyPtyIds, + sharedPtyIds, + unroutablePtyIds + }) + } + return plans +} + +export function removeSleepingAgentSessionsForTab( + records: Record, + tabId: string +): Record { + let next = records + for (const [paneKey, record] of Object.entries(records)) { + if (!paneKey.startsWith(`${tabId}:`) && record.tabId !== tabId) { + continue + } + if (next === records) { + next = { ...records } + } + delete next[paneKey] + } + return next +} diff --git a/src/renderer/src/store/slices/terminals.ts b/src/renderer/src/store/slices/terminals.ts index be20477bc78..e127c1973a8 100644 --- a/src/renderer/src/store/slices/terminals.ts +++ b/src/renderer/src/store/slices/terminals.ts @@ -67,7 +67,10 @@ import { // Why: import the store-free registry, not terminal-parked-tab-watchers — // that module imports @/store, and a slice importing it would re-enter store // creation before this slice finishes evaluating. -import { disposeParkedTerminalWatchersForPtyIds } from '@/components/terminal-pane/terminal-parked-watcher-registry' +import { + disposeParkedTerminalWatchersForPtyIds, + retireParkedTerminalTab +} from '@/components/terminal-pane/terminal-parked-watcher-registry' import { normalizeTerminalLayoutSnapshot, resolvePtyBoundActiveLeafId @@ -94,6 +97,13 @@ import { removeSleepingRecordsReplacedByManualWorktreeSleep, type AgentStatusWorktreeShutdownReason } from './agent-status' +import { + buildTerminalTabRetirementPlan, + isTerminalTabPresent, + removeSleepingAgentSessionsForTab, + type TerminalTabCloseReason, + type TerminalTabRetirementPlan +} from './terminal-tab-retirement' function getNextTerminalOrdinal(tabs: TerminalTab[]): number { const usedOrdinals = new Set() @@ -593,7 +603,16 @@ export type TerminalSlice = { } ) => TerminalTab openNewTerminalTabInActiveWorkspace: (groupId: string) => Promise - closeTab: (tabId: string, opts?: { recordInteraction?: boolean }) => void + closeTab: ( + tabId: string, + opts?: { + recordInteraction?: boolean + reason?: TerminalTabCloseReason + remoteCloseOwnedByHost?: boolean + localPtyTeardownOwnedExternally?: boolean + precomputedRetirementPlan?: TerminalTabRetirementPlan + } + ) => void reorderTabs: (worktreeId: string, tabIds: string[]) => void setTabBarOrder: (worktreeId: string, order: string[]) => void setActiveTab: (tabId: string) => void @@ -1197,18 +1216,69 @@ export const createTerminalSlice: StateCreator }, closeTab: (tabId, opts) => { + const closeReason = opts?.reason ?? 'user' + const retiresSession = closeReason === 'user' || closeReason === 'cleanup' + const retirementPlan = + opts?.precomputedRetirementPlan?.tabId === tabId + ? opts.precomputedRetirementPlan + : buildTerminalTabRetirementPlan(get(), tabId) let closingWorktreeId: string | null = null + + // Why: a parked tab has no mounted TerminalPane cleanup. Retirement must + // synchronously revoke its observer/candidate state before provider exit races. + retireParkedTerminalTab(tabId) + if (retiresSession) { + const fallbackRuntimeEnvironmentId = retirementPlan.worktreeId + ? getRuntimeEnvironmentIdForWorktree(get(), retirementPlan.worktreeId) + : null + const retirementTasks: Promise[] = opts?.localPtyTeardownOwnedExternally + ? [] + : retirementPlan.localOrSshPtyIds.map(async (ptyId) => window.api.pty.kill(ptyId)) + const localOrSshTaskCount = retirementTasks.length + if (!opts?.remoteCloseOwnedByHost) { + for (const terminal of retirementPlan.runtimeTerminals) { + const environmentId = terminal.environmentId ?? fallbackRuntimeEnvironmentId + retirementTasks.push( + callRuntimeRpc( + environmentId ? { kind: 'environment', environmentId } : { kind: 'local' }, + 'terminal.close', + { terminal: terminal.handle } + ) + ) + } + } + if (retirementPlan.unroutablePtyIds.length > 0) { + console.warn('[terminal-retirement] skipped unroutable runtime handles', { + tabId, + count: retirementPlan.unroutablePtyIds.length + }) + } + // Why: close remains synchronous and idempotent; provider failures must + // not reject into the UI or prevent renderer ownership from being revoked. + void Promise.allSettled(retirementTasks).then((results) => { + const localOrSshFailures = results + .slice(0, localOrSshTaskCount) + .filter((result) => result.status === 'rejected').length + const runtimeFailures = results + .slice(localOrSshTaskCount) + .filter((result) => result.status === 'rejected').length + if (localOrSshFailures > 0 || runtimeFailures > 0) { + console.warn('[terminal-retirement] provider teardown failed', { + tabId, + localOrSshFailures, + runtimeFailures + }) + } + }) + } + set((s) => { const next = { ...s.tabsByWorktree } - let closingPtyId: string | null = null for (const wId of Object.keys(next)) { const before = next[wId] const closingTab = before.find((t) => t.id === tabId) if (closingTab) { closingWorktreeId = wId - if (!closingPtyId) { - closingPtyId = closingTab.ptyId ?? null - } } const after = before.filter((t) => t.id !== tabId) if (after.length !== before.length) { @@ -1225,6 +1295,10 @@ export const createTerminalSlice: StateCreator delete nextPtyIdsByTabId[tabId] const nextLastKnownRelay = { ...s.lastKnownRelayPtyIdByTabId } delete nextLastKnownRelay[tabId] + const nextDeferredSshSessionIdsByTabId = { ...s.deferredSshSessionIdsByTabId } + delete nextDeferredSshSessionIdsByTabId[tabId] + const nextPendingReconnectPtyIdByTabId = { ...s.pendingReconnectPtyIdByTabId } + delete nextPendingReconnectPtyIdByTabId[tabId] const nextRuntimePaneTitlesByTabId = { ...s.runtimePaneTitlesByTabId } delete nextRuntimePaneTitlesByTabId[tabId] // Why: preserve the unreadTerminalTabs reference when the closing tab had @@ -1260,6 +1334,9 @@ export const createTerminalSlice: StateCreator delete nextLastTerminalInputAtByPaneKey[paneKey] } } + const nextSleepingAgentSessionsByPaneKey = retiresSession + ? removeSleepingAgentSessionsForTab(s.sleepingAgentSessionsByPaneKey, tabId) + : s.sleepingAgentSessionsByPaneKey const nextPendingStartupByTabId = { ...s.pendingStartupByTabId } delete nextPendingStartupByTabId[tabId] const nextAutomaticAgentResumeClaimsByTabId = { ...s.automaticAgentResumeClaimsByTabId } @@ -1307,14 +1384,20 @@ export const createTerminalSlice: StateCreator // prevent unbounded store growth across restarts. let nextSnapshots = s.pendingSnapshotByPtyId let nextColdRestores = s.pendingColdRestoreByPtyId - if (closingPtyId) { - if (closingPtyId in nextSnapshots) { + const closingPtyIds = new Set([ + ...retirementPlan.localOrSshPtyIds, + ...retirementPlan.runtimeTerminals.map((terminal) => terminal.ptyId), + ...retirementPlan.cleanupOnlyPtyIds, + ...retirementPlan.unroutablePtyIds + ]) + for (const closingId of closingPtyIds) { + if (closingId in nextSnapshots) { nextSnapshots = { ...nextSnapshots } - delete nextSnapshots[closingPtyId] + delete nextSnapshots[closingId] } - if (closingPtyId in nextColdRestores) { + if (closingId in nextColdRestores) { nextColdRestores = { ...nextColdRestores } - delete nextColdRestores[closingPtyId] + delete nextColdRestores[closingId] } } @@ -1324,7 +1407,12 @@ export const createTerminalSlice: StateCreator activeTabIdByWorktree: nextActiveTabIdByWorktree, ptyIdsByTabId: nextPtyIdsByTabId, lastKnownRelayPtyIdByTabId: nextLastKnownRelay, + deferredSshSessionIdsByTabId: nextDeferredSshSessionIdsByTabId, + pendingReconnectPtyIdByTabId: nextPendingReconnectPtyIdByTabId, runtimePaneTitlesByTabId: nextRuntimePaneTitlesByTabId, + ...(nextSleepingAgentSessionsByPaneKey !== s.sleepingAgentSessionsByPaneKey + ? { sleepingAgentSessionsByPaneKey: nextSleepingAgentSessionsByPaneKey } + : {}), // Why: skip writing unreadTerminalTabs when the reference is unchanged — // avoids a no-op top-level state allocation that would force re-evaluation // of full-state selectors. Mirrors the sibling pattern in tabs.ts. @@ -1382,7 +1470,10 @@ export const createTerminalSlice: StateCreator (entry) => entry.contentType === 'terminal' && entry.entityId === tabId ) if (workspaceItem) { - get().closeUnifiedTab(workspaceItem.id, opts) + get().closeUnifiedTab(workspaceItem.id, { + recordInteraction: opts?.recordInteraction, + terminalRetirementHandled: true + }) } } }, @@ -1872,6 +1963,11 @@ export const createTerminalSlice: StateCreator }, updateTabPtyId: (tabId, ptyId) => { + // Why: async spawn owners must perform provider teardown themselves, but + // this final guard prevents any late caller from recreating retired tab maps. + if (!isTerminalTabPresent(get(), tabId)) { + return + } let worktreeId: string | null = null let wasActivationSpawn = false const isRemoteRuntimeMirror = isRemoteRuntimePtyId(ptyId) @@ -2800,6 +2896,8 @@ export const createTerminalSlice: StateCreator sourceTabId, targetTabId }) => { + const sourcePaneKey = makePaneKey(sourceTabId, detachedLeafId) + const targetPaneKey = makePaneKey(targetTabId, detachedLeafId) set((s) => { const layoutSourcePtyIds = uniquePtyIds(Object.values(sourceLayout.ptyIdsByLeafId ?? {})) const existingSourcePtyIds = (s.ptyIdsByTabId[sourceTabId] ?? []).filter( @@ -2839,62 +2937,19 @@ export const createTerminalSlice: StateCreator ? withTerminalTabPtyId(sourceTabsByWorktree, targetTabId, detachedPtyId) : sourceTabsByWorktree - const sourcePaneKey = makePaneKey(sourceTabId, detachedLeafId) - const targetPaneKey = makePaneKey(targetTabId, detachedLeafId) - const sourceForeground = s.paneForegroundAgentByPaneKey[sourcePaneKey] - const sourceLaunchConfig = s.agentLaunchConfigByPaneKey[sourcePaneKey] - const hadSourceHookStatus = sourcePaneKey in s.agentStatusByPaneKey - let nextPaneForegroundAgentByPaneKey = s.paneForegroundAgentByPaneKey - let nextAgentLaunchConfigByPaneKey = s.agentLaunchConfigByPaneKey - let nextAgentStatusByPaneKey = s.agentStatusByPaneKey - let nextRetentionSuppressedPaneKeys = s.retentionSuppressedPaneKeys - if (sourceForeground) { - nextPaneForegroundAgentByPaneKey = { ...s.paneForegroundAgentByPaneKey } - delete nextPaneForegroundAgentByPaneKey[sourcePaneKey] - nextPaneForegroundAgentByPaneKey[targetPaneKey] = sourceForeground - } - if (sourceLaunchConfig) { - nextAgentLaunchConfigByPaneKey = { ...s.agentLaunchConfigByPaneKey } - delete nextAgentLaunchConfigByPaneKey[sourcePaneKey] - nextAgentLaunchConfigByPaneKey[targetPaneKey] = { - ...sourceLaunchConfig, - identity: { - ...sourceLaunchConfig.identity, - tabId: targetTabId, - leafId: detachedLeafId - } - } - } - if (hadSourceHookStatus) { - nextAgentStatusByPaneKey = { ...s.agentStatusByPaneKey } - delete nextAgentStatusByPaneKey[sourcePaneKey] - nextRetentionSuppressedPaneKeys = { - ...s.retentionSuppressedPaneKeys, - [sourcePaneKey]: true - } - } - return { ptyIdsByTabId: nextPtyIdsByTabId, lastKnownRelayPtyIdByTabId: nextLastKnownRelayPtyIdByTabId, - ...(nextTabsByWorktree !== s.tabsByWorktree ? { tabsByWorktree: nextTabsByWorktree } : {}), - ...(nextPaneForegroundAgentByPaneKey !== s.paneForegroundAgentByPaneKey - ? { paneForegroundAgentByPaneKey: nextPaneForegroundAgentByPaneKey } - : {}), - ...(nextAgentLaunchConfigByPaneKey !== s.agentLaunchConfigByPaneKey - ? { agentLaunchConfigByPaneKey: nextAgentLaunchConfigByPaneKey } - : {}), - ...(nextAgentStatusByPaneKey !== s.agentStatusByPaneKey - ? { - // Why: the PTY keeps its immutable source ORCA_PANE_KEY; retire - // rather than re-key a hook row that future events cannot update. - agentStatusByPaneKey: nextAgentStatusByPaneKey, - agentStatusEpoch: s.agentStatusEpoch + 1, - retentionSuppressedPaneKeys: nextRetentionSuppressedPaneKeys - } - : {}) + ...(nextTabsByWorktree !== s.tabsByWorktree ? { tabsByWorktree: nextTabsByWorktree } : {}) } }) + // Why: detach keeps the process and its immutable physical pane key alive; + // move resume/status authority to the new surface before the source can close. + get().transferAgentPaneAuthority({ + fromPaneKey: sourcePaneKey, + toPaneKey: targetPaneKey, + ptyId: detachedPtyId + }) }, queueTabStartupCommand: (tabId, startup) => { diff --git a/src/renderer/src/web/web-preload-api.ts b/src/renderer/src/web/web-preload-api.ts index 0d104751b17..1af722e3899 100644 --- a/src/renderer/src/web/web-preload-api.ts +++ b/src/renderer/src/web/web-preload-api.ts @@ -745,7 +745,9 @@ function createWebPreloadApi(): Partial { onMigrationUnsupportedClear: () => noopUnsubscribe, getMigrationUnsupportedSnapshot: () => Promise.resolve([]), drop: () => {}, - dropByTabPrefix: () => {} + dropByTabPrefix: () => {}, + retirePaneAuthority: () => {}, + transferPaneAuthority: () => {} }, mobile: { listNetworkInterfaces: () => Promise.resolve({ interfaces: [] }), @@ -2804,7 +2806,7 @@ function createPtyApi(): NonNullable['pty']> { clearPendingPaneSerializer: () => Promise.resolve(), management: { listSessions: () => Promise.resolve({ sessions: [], degraded: false }), - killAll: () => Promise.resolve({ killedCount: 0, remainingCount: 0 }), + killAll: () => Promise.resolve({ killedCount: 0, remainingCount: 0, killedSessionIds: [] }), killOne: () => Promise.resolve({ success: false }), restart: () => Promise.resolve({ success: false }) } diff --git a/src/shared/agent-hook-listener.ts b/src/shared/agent-hook-listener.ts index 1e3728eaef7..5870fd5f43b 100644 --- a/src/shared/agent-hook-listener.ts +++ b/src/shared/agent-hook-listener.ts @@ -155,6 +155,47 @@ export function clearPaneCacheState(state: HookListenerState, paneKey: string): state.claudeLeadStateByPaneKey.delete(paneKey) } +function movePaneScopedMapEntries( + map: Map, + fromPaneKey: string, + toPaneKey: string +): void { + for (const [key, value] of Array.from(map.entries())) { + if (key !== fromPaneKey && !key.startsWith(`${fromPaneKey}\0`)) { + continue + } + map.delete(key) + map.set(`${toPaneKey}${key.slice(fromPaneKey.length)}`, value) + } +} + +function movePaneScopedSetEntries(set: Set, fromPaneKey: string, toPaneKey: string): void { + for (const key of Array.from(set)) { + if (key !== fromPaneKey && !key.startsWith(`${fromPaneKey}\0`)) { + continue + } + set.delete(key) + set.add(`${toPaneKey}${key.slice(fromPaneKey.length)}`) + } +} + +export function movePaneCacheState( + state: HookListenerState, + fromPaneKey: string, + toPaneKey: string +): void { + if (fromPaneKey === toPaneKey) { + return + } + movePaneScopedMapEntries(state.lastPromptByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.lastToolByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.lastStatusByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.antigravityCompletedTranscriptByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedSetEntries(state.ampCompletedCacheKeys, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.claudeSubagentRosterByPaneKey, fromPaneKey, toPaneKey) + movePaneScopedMapEntries(state.claudeLeadStateByPaneKey, fromPaneKey, toPaneKey) +} + function clearPaneTurnCacheState(state: HookListenerState, paneKey: string): void { state.lastPromptByPaneKey.delete(paneKey) state.lastToolByPaneKey.delete(paneKey) diff --git a/src/shared/types.ts b/src/shared/types.ts index 0766ace56f4..6c1ebaf8cc8 100644 --- a/src/shared/types.ts +++ b/src/shared/types.ts @@ -3569,7 +3569,10 @@ export type PersistedTrustedOrcaHooks = Record({ // Why: calling window.api.repos.add() goes through the same code path as // the "Add Project" UI flow, ensuring worktrees are fetched and the session // initializes properly. - await page.evaluate(async (repoPath) => { - await window.api.repos.add({ path: repoPath }) + const seededRepoId = await page.evaluate(async (repoPath) => { + const result = await window.api.repos.add({ path: repoPath }) + if ('error' in result) { + throw new Error(result.error) + } + return result.repo.id }, repoPath) // Fetch repos in the renderer store so it picks up the new repo, then opt @@ -300,13 +304,13 @@ export const test = base.extend({ await playwrightExpect .poll( () => - page.evaluate(async (repoPath) => { + page.evaluate(async (repoId) => { const store = window.__store if (!store) { return false } await store.getState().fetchRepos() - const repo = store.getState().repos.find((candidate) => candidate.path === repoPath) + const repo = store.getState().repos.find((candidate) => candidate.id === repoId) if (!repo) { return false } @@ -314,7 +318,7 @@ export const test = base.extend({ // repos hide those by default after the visibility rollout. await store.getState().updateRepo(repo.id, { externalWorktreeVisibility: 'show' }) return true - }, repoPath), + }, seededRepoId), { timeout: 30_000, message: `Expected e2e repo to be loaded: ${repoPath}` @@ -322,21 +326,18 @@ export const test = base.extend({ ) .toBe(true) - // Best-effort fetch of every repo's worktrees. Why: the renderer can still + // Best-effort fetch of the seeded repo's worktrees. Why: the renderer can still // re-navigate during initial hydration and destroy the execution context // mid-evaluate; the authoritative seeded-worktree poll below is the real wait, // so swallow a hydration-reload failure here instead of failing setup. await page - .evaluate(async () => { + .evaluate(async (repoId) => { const store = window.__store if (!store) { return } - const repos = store.getState().repos - for (const repo of repos) { - await store.getState().fetchWorktrees(repo.id) - } - }) + await store.getState().fetchWorktrees(repoId) + }, seededRepoId) .catch(() => false) // Why: parallel specs mutate real git worktrees in the shared fixture repo. @@ -345,18 +346,14 @@ export const test = base.extend({ await playwrightExpect .poll( () => - page.evaluate(async (repoPath) => { + page.evaluate(async (repoId) => { const store = window.__store if (!store) { return 0 } - const repo = store.getState().repos.find((candidate) => candidate.path === repoPath) - if (!repo) { - return 0 - } - await store.getState().fetchWorktrees(repo.id) - return store.getState().worktreesByRepo[repo.id]?.length ?? 0 - }, repoPath), + await store.getState().fetchWorktrees(repoId) + return store.getState().worktreesByRepo[repoId]?.length ?? 0 + }, seededRepoId), { timeout: 30_000, message: 'seeded e2e worktrees did not load' @@ -378,21 +375,22 @@ export const test = base.extend({ // Why: workspaceSessionReady restoration can overwrite activeWorktreeId // after earlier setup calls. Selecting it here ensures every test starts on // the seeded repo instead of the "Select a worktree" empty state. - await page.evaluate((repoPath: string) => { + await page.evaluate((repoId: string) => { const store = window.__store if (!store) { return } const state = store.getState() - const allWorktrees = Object.values(state.worktreesByRepo).flat() - const testWorktree = allWorktrees.find( - (worktree) => worktree.path === repoPath || worktree.path.startsWith(repoPath) + // Why: provider-returned identity is stable across Windows path casing + // and separator normalization, unlike comparing renderer path strings. + const testWorktree = state.worktreesByRepo[repoId]?.find( + (worktree) => worktree.isMainWorktree ) if (testWorktree) { state.setActiveWorktree(testWorktree.id) } - }, repoPath) + }, seededRepoId) // Best-effort seed of a baseline terminal tab when a fresh isolated // profile has none yet. diff --git a/tests/e2e/helpers/orca-restart.ts b/tests/e2e/helpers/orca-restart.ts index 3d7af0b1d44..742d34ffdda 100644 --- a/tests/e2e/helpers/orca-restart.ts +++ b/tests/e2e/helpers/orca-restart.ts @@ -130,36 +130,40 @@ export async function attachRepoAndOpenTerminal(page: Page, repoPath: string): P throw new Error(`attachRepoAndOpenTerminal: ${repoPath} is not a git repo`) } - await page.evaluate(async (repoPath) => { - await window.api.repos.add({ path: repoPath }) - }, repoPath) - - await page.evaluate(async () => { - const store = window.__store - if (!store) { - return - } - await store.getState().fetchRepos() - }) - const repoId = await page.evaluate(async (repoPath) => { - const store = window.__store - if (!store) { - return null + const result = await window.api.repos.add({ path: repoPath }) + if ('error' in result) { + throw new Error(result.error) } - const repo = store.getState().repos.find((candidate) => candidate.path === repoPath) - if (!repo) { - return null - } - // Why: this restart fixture uses the global e2e repo, whose seeded Git - // worktree is external to Orca's workspace root after the visibility rollout. - await store.getState().updateRepo(repo.id, { externalWorktreeVisibility: 'show' }) - return repo.id + return result.repo.id }, repoPath) - if (!repoId) { - throw new Error(`attachRepoAndOpenTerminal: expected e2e repo to be loaded: ${repoPath}`) - } + await expect + .poll( + () => + page.evaluate(async (repoId) => { + const store = window.__store + if (!store) { + return false + } + // Why: repos.add emits a concurrent refresh whose generation can + // supersede this fetch; poll until either refresh publishes the repo. + await store.getState().fetchRepos() + const repo = store.getState().repos.find((candidate) => candidate.id === repoId) + if (!repo) { + return false + } + // Why: this restart fixture uses the global e2e repo, whose seeded Git + // worktree is external to Orca's workspace root after the visibility rollout. + await store.getState().updateRepo(repo.id, { externalWorktreeVisibility: 'show' }) + return true + }, repoId), + { + timeout: 30_000, + message: `attachRepoAndOpenTerminal: expected e2e repo to be loaded: ${repoPath}` + } + ) + .toBe(true) await page.waitForFunction( () => window.__store?.getState().workspaceSessionReady === true, @@ -170,10 +174,7 @@ export async function attachRepoAndOpenTerminal(page: Page, repoPath: string): P // Why: fetchWorktrees() is async. Awaiting the outer page.evaluate returns // before the Zustand worktree slice has observed the hydrated state, so a // single evaluate() that reads worktreesByRepo can see an empty map. Poll - // the store until *any* worktree shows up, then pick the one that matches - // our seeded repo. Matching by basename is sufficient because each test - // run uses a unique tmp-dir suffix, and it avoids symlink-canonicalization - // mismatches between the path we pass in and the one the store records. + // the store until the seeded repo's worktree shows up. await expect .poll( async () => @@ -192,28 +193,22 @@ export async function attachRepoAndOpenTerminal(page: Page, repoPath: string): P ) .toBe(true) - const repoBasename = path.basename(repoPath) - const worktreeId = await page.evaluate((repoBasename: string) => { + const worktreeId = await page.evaluate((repoId: string) => { const store = window.__store if (!store) { return null } const state = store.getState() - const allWorktrees = Object.values(state.worktreesByRepo).flat() - // Why: the primary worktree lives at the repo root, so its path ends with - // the repo directory name. The secondary worktree is a sibling directory - // and will not match this suffix. This gives us the primary deterministically - // without depending on boolean fields on the worktree record. - const primary = - allWorktrees.find( - (worktree) => worktree.path.split(/[\\/]+/).findLast(Boolean) === repoBasename - ) ?? allWorktrees[0] + // Why: repo identity remains stable when Windows canonicalizes path casing + // or separators between the IPC and renderer layers. + const repoWorktrees = state.worktreesByRepo[repoId] ?? [] + const primary = repoWorktrees.find((worktree) => worktree.isMainWorktree) ?? repoWorktrees[0] if (!primary) { return null } state.setActiveWorktree(primary.id) return primary.id - }, repoBasename) + }, repoId) if (!worktreeId) { throw new Error('attachRepoAndOpenTerminal: test repo did not surface in the store') diff --git a/tests/e2e/terminal-parked-close-retirement.spec.ts b/tests/e2e/terminal-parked-close-retirement.spec.ts new file mode 100644 index 00000000000..acd6a42b708 --- /dev/null +++ b/tests/e2e/terminal-parked-close-retirement.spec.ts @@ -0,0 +1,96 @@ +import type { Page } from '@stablyai/playwright-test' +import { randomUUID } from 'node:crypto' +import { test, expect } from './helpers/orca-app' +import { ensureTerminalVisible, waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { parkHiddenTabBehindDecoy } from './helpers/terminal-hidden-parking' +import { + execInTerminal, + getTerminalContent, + waitForActiveTerminalManager, + waitForPaneIdentitySnapshot +} from './helpers/terminal' +import { nodeTerminalCommand } from './terminal-node-command' + +const PARKING_DELAY_MS = Number(process.env.ORCA_E2E_TERMINAL_PARKING_DELAY_MS) || 500 + +test.use({ + orcaAppExtraEnv: { ORCA_E2E_TERMINAL_PARKING_DELAY_MS: String(PARKING_DELAY_MS) } +}) + +async function hasPtySession(page: Page, ptyId: string): Promise { + return page.evaluate(async (id) => { + const sessions = await window.api.pty.listSessions() + return sessions.some((session) => session.id === id) + }, ptyId) +} + +async function createActiveTerminalTab(page: Page, worktreeId: string): Promise { + const tabId = await page.evaluate((id) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + const state = store.getState() + const tab = state.createTab(id, undefined, undefined, { activate: true }) + state.setActiveTab(tab.id) + state.setActiveTabType('terminal') + return tab.id + }, worktreeId) + + await waitForActiveTerminalManager(page, 30_000) + expect((await waitForPaneIdentitySnapshot(page, 1)).tabId).toBe(tabId) +} + +test('closing a parked terminal tab retires its exact PTY session', async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + const worktreeId = await waitForActiveWorktree(orcaPage) + await ensureTerminalVisible(orcaPage) + await waitForActiveTerminalManager(orcaPage, 30_000) + + const snapshot = await waitForPaneIdentitySnapshot(orcaPage, 1) + const tabId = snapshot.tabId + const ptyId = snapshot.panes[0]?.ptyId + if (!ptyId) { + throw new Error('active terminal pane did not bind a PTY') + } + + const tab = orcaPage.locator(`[data-testid="sortable-tab"][data-tab-id="${tabId}"]`) + await expect(tab).toBeVisible() + + const marker = `PARKED_CLOSE_READY_${randomUUID()}` + const keepAliveScript = `process.stdout.write(${JSON.stringify(`${marker}\n`)}); setInterval(() => {}, 1000)` + await execInTerminal(orcaPage, ptyId, nodeTerminalCommand(['-e', keepAliveScript])) + await expect + .poll(() => getTerminalContent(orcaPage), { + timeout: 10_000, + message: 'long-lived terminal child did not print its ready marker' + }) + .toContain(marker) + await expect.poll(() => hasPtySession(orcaPage, ptyId), { timeout: 10_000 }).toBe(true) + + // Why: the most recently hidden tab stays warm, so tab B must take that + // exemption before the helper opens decoy tab C and makes tab A parkable. + await createActiveTerminalTab(orcaPage, worktreeId) + await parkHiddenTabBehindDecoy(orcaPage, worktreeId, tabId, { + parkDelayMs: PARKING_DELAY_MS + }) + // Why: parking must remove only the renderer view; otherwise the retirement + // assertion could pass because the PTY died before the close action ran. + await expect.poll(() => hasPtySession(orcaPage, ptyId), { timeout: 10_000 }).toBe(true) + + await orcaPage.evaluate((id) => { + const store = window.__store + if (!store) { + throw new Error('window.__store is unavailable') + } + store.getState().closeTab(id) + }, tabId) + + await expect + .poll(() => hasPtySession(orcaPage, ptyId), { + timeout: 15_000, + message: `parked tab close did not retire PTY ${ptyId}` + }) + .toBe(false) + await expect(tab).toHaveCount(0) +})