diff --git a/docs/plans/2026-09-25-pty-attach-survives-wake.md b/docs/plans/2026-09-25-pty-attach-survives-wake.md index 453041b7b..ee8ef725c 100644 --- a/docs/plans/2026-09-25-pty-attach-survives-wake.md +++ b/docs/plans/2026-09-25-pty-attach-survives-wake.md @@ -10,7 +10,7 @@ The attach count and restore size describe the renderer's VIEW, not the process. Process cleanup leaves them alone, and the view's own `detachAgentPty` (on unmount, including pane close) remains the only release. ## Residuals (not in this PR) -- Plain terminals: `terminalAttached` has no detach call at all and is only cleared by cleanup. Keeping it would leak an id per closed terminal, so the terminal half needs a renderer detach first. Recorded on #1281. +- Plain terminals: `terminalAttached` has no detach call at all and is only cleared by cleanup. Keeping it would leak an id per closed terminal, so the terminal half needs a renderer detach first. Recorded on #1281. Done in `2026-09-26-terminal-attach-survives-respawn.md`. - A renderer reload or crash never runs the leaf cleanup (#1283 item 3). That predates this change, but it now also survives a process exit. ## Test diff --git a/docs/plans/2026-09-26-goal-loop-grace-test.md b/docs/plans/2026-09-26-goal-loop-grace-test.md new file mode 100644 index 000000000..a2a1c560d --- /dev/null +++ b/docs/plans/2026-09-26-goal-loop-grace-test.md @@ -0,0 +1,53 @@ +# GoalLoop "unrelated traffic cannot renew the screen grace" failed on main CI (#1314) + +## Evidence + +- Main CI run 36156560842 (merge `54287d90`) failed + `src/main/goalLoop/GoalLoopService.test.ts:397` with 2 deliveries instead of 1. The same tree passed + on the PR run, and locally the test passes when run alone. +- Instrumented locally (fake timers, real file store): + - the first delivery happens at fake t = 45 s; + - the traffic ticks at 60 s and 90 s open no hold; + - the delivery run is still `continuing` at every later tick, with the 60 s turn boundary parked in + `pendingContinue`. +- Why it stays `continuing`: after delivering, `maybeContinue` awaits `persist()`, which is **real disk + I/O**. `vi.advanceTimersByTimeAsync` moves fake time forward without waiting for it, so the fake + instant at which the parked trigger resumes depends on disk latency. That latency is the uncontrolled + variable. +- **Deterministic reproduction:** the same test body with an instant in-memory store fails + **3/3** with 2 deliveries. When persistence completes promptly, the 60 s boundary opens a fresh hold + at 60 s and delivers at about 105 s, inside the test's 120 s window. + +## Root cause + +The test asserts "exactly one delivery in 120 s", but its own traffic contains a **legitimate second +turn boundary**. The `turn_started`/`turn_completed` pair at 60 s comes after our 45 s delivery, so it is +the delivered continuation's turn ending. A second continuation after another bounded 45 s grace is +correct behaviour. Whether that second continuation falls inside the window depends only on how long +the first delivery's real `persist()` takes relative to fake time: slow disk means 1 delivery (green), +prompt disk means 2 (red). That is why it failed on a loaded CI runner and passes locally. + +This is a test bug, not a product bug: +- the product holds the screen grace for 45 s from the first hold; +- traffic during the hold (the 30 s tick) does not renew it; +- the first delivery lands at exactly 45 s. + +That is the #1033 round-4 claim the test exists for. + +## Decision (default) + +Assert the claim, and only the claim: +- drive traffic **during** the hold, at 0 s and 30 s; +- check that nothing is delivered at 44.999 s; +- check that exactly one delivery happens at 45 s. + +The test stops there, before any later turn boundary it does not mean to test. It runs against **both** +the real file store and an instant in-memory store, so persistence latency is pinned in both +directions. No timeout is widened, and no production code changes. + +## Tests + +- The rewritten case, parametrized over both stores. +- **Fail-first:** the old assertion window with the instant store is red 3/3 (reproduced above). +- **Mutation:** if held-progress traffic renewed `heldSince` (the bug #1033 round 4 fixed), the new test + is red for both stores. diff --git a/docs/plans/2026-09-26-reload-all-ownership.md b/docs/plans/2026-09-26-reload-all-ownership.md new file mode 100644 index 000000000..c9927d9f8 --- /dev/null +++ b/docs/plans/2026-09-26-reload-all-ownership.md @@ -0,0 +1,50 @@ +# Reload Agents re-checks ownership around every await (#1282) + +## Evidence +`reloadAgentSessions` (session.ts) walks a snapshot of the agents. For each one it kills the old backend, awaits `spawnSession`, and commits the whole id map at the end. Nothing re-checks the workspace between those awaits: +- **A close mid-reload** (an agent closed after the snapshot) is still respawned. It comes back under a new id, or as an invisible orphan if its project closed. +- **A replace of the same agent mid-reload** (for example a provider switch) yields two successors on one provider transcript. `replaceSession` guards this with `canCommit` (#815); reload does not. +- **The draft** comes from a runtime snapshot taken before the slow sequence, so text typed meanwhile is lost. `draftImages` and `unreadSince`/`unreadKind` are never copied, so toggling dangerous mode clears every "finished, not seen" marker. + +## Change +- **Before each kill,** the agent must still be the same owned agent (`canCommit`'s rule: same cwd, kind and runtime, and still owned). Otherwise it is skipped, with no kill and no spawn. +- **After its kill and after its spawn,** the same check runs again. If the agent went away, nothing is spawned, or the new process is killed and nothing is filed. (Superseded in detail by the round sections below: each agent now commits on its own.) +- **The draft, draft images and the unread marker** are read from the live runtime at commit time. + +## Tests +On the recorded workspace fixture: a close during the reload, a replace during it, a close before the loop reaches an agent, and a draft typed and an unread marker set during it. Red on main. + +## Review round 1 (#1326) +- **Overlapping reloads (A1).** On -> off before the first reload settled ran two loops over one snapshot and could leave the dangerous successors running. Reloads now run one at a time; the later click restarts the earlier one's successors with the final setting. +- **Project merge mid-reload (A2).** The successor was filed under its captured, now deleted, project. The commit now files it from the LIVE row; only the spawn's own fields (identity, MCP domains, resumed provider session) come from the loop. +- **Close during an orphan kill, and a rejected orphan kill (A3/A4, B1).** The commit-time checks and the commit are now one synchronous run. Orphans are killed after it, each in its own try/catch. +- **Failed respawn of a closed agent (A5).** It no longer recreates a `failed` runtime under the closed id. +- **Mutation survivors (cwd check, project-ownership clause).** Pinned by a cwd change and a removed project mid-reload. + +Tests: 7 new cases on the same fixture, 5 red against the round-1 head (the two mutation pins pass there, as they cover existing behavior). + +## Review round 2 (#1326): each agent commits on its own +Round 1's synchronous batch commit left two intervals open, and round 2 (reviewers A and C, steering q47) found them: +- **C1: after the pre-spawn kill.** A close that finished while the old backend's kill was awaited was followed by a spawn anyway. The loop now re-checks ownership after that await. +- **C2: an early successor unfiled while a later spawn stalls.** Closing the pane killed only the old backend, and the successor kept running, possibly in dangerous mode. **Each agent is now committed as soon as its own spawn returns** (the same one-pair remap `replaceSession` uses), so the successor IS the pane, and a close from then on is an ordinary close. The only remaining interval is the spawn itself; a close there orphans the successor, which is killed at once, not after the whole loop. +- **C3: a swallowed orphan-kill rejection.** The contract is now: retry once at once; if that also fails, keep the successor in `unstoppedReloadSuccessorsRef`, which the next reload retries first; log a curated line (no IPC text, q22). Application quit (`killAll`) stops every backend regardless, so none is ever ownerless. It still does not throw, so the other agents are restarted and filed (round 1, A4). +- **A1: dropped durable provider id.** A fresh start (the old id was provisional, so nothing is resumed) now keeps the `providerSessionId` spawn reports, as `runtime-start`, the same rule as `spawn()`. The live row's provider fields are never copied. +- **The `providerRuntime` clause** of the ownership rule is now pinned too. + +Not adopted: routing reload through `replaceSession`. That function spawns successor-first with the ambient dangerous-mode setting and a Codex same-rollout handoff; reload kills first and passes an explicit mode. Merging them changes behavior beyond this fix. + +Tests (on the same fixture): C2 is asserted with the later spawn still held, closing through the real `killSession` action. C1 closes while the reload's kill is held. C3 counts the retry and the next reload's retry. A1 covers both directions: a reported durable id is kept, and a provisional id is not carried. 4 of these are red on the round-2 head; the `providerRuntime` and provisional-id cases pin existing guards, and a mutation removing either one fails them. + +## Verification pass (#1326, reviewers A and C) and steering q56 +- **A remembered orphan was never retried once no live agents remained (A, C; Major).** The retry loop ran after the no-live-agents return. It now runs first. +- **A refused kill cleared the retry record (C; Major).** `session:kill-owned` answers `false` for both "gone" and "not yours, still running". Only `true` now clears the record. A backend that was already gone costs one IPC call per later reload. +- **An all-failed reload no longer pruned unowned rows (A; Minor).** They are pruned once, up front. The per-commit prune is removed: with the up-front prune it was a second mechanism nothing pinned (A's surviving mutation). +- **Relationship and pin remap survivors (C).** The early-successor test now asserts, while the later spawn is still held, that a child's `linkedParentId`/`orchestrationParentId` and the pin already follow the successor. Removing either remap fails it. + +Tests: 3 new cases (red on `f897700f`) plus the interval assertions above. Workspace suites: 1354/1354. + +## Second verification (A, C) +- **Rows unowned mid-reload (A, Minor).** A project removed while an earlier spawn was in flight left its rows after the up-front prune. The per-successor commit prunes again. Both prunes are pinned: the all-failed test and the new mid-reload test. +- **An orphan's early runtime entry (A, C; Minor).** A confirmed orphan kill now also drops the renderer's bookkeeping for that id. `forgetSessionLocally`, extracted from `killSession`, is shared by both. + +Tests: 2 new cases (red on `b6440143`). diff --git a/docs/plans/2026-09-26-terminal-attach-survives-respawn.md b/docs/plans/2026-09-26-terminal-attach-survives-respawn.md new file mode 100644 index 000000000..134577173 --- /dev/null +++ b/docs/plans/2026-09-26-terminal-attach-survives-respawn.md @@ -0,0 +1,25 @@ +# A plain terminal view survives a same-id respawn (#1281 residual, #1283 item 3 for terminals) + +## Problem +#1281 is two halves. The agent raw-PTY half shipped in `2ab60d45`, and #1311's review (`6cf6f054`) made each renderer page own its agent-PTY attaches, which covers #1283 item 3 for agents. The plain-terminal half is still open, and its plan (`2026-09-25-pty-attach-survives-wake.md`) recorded it as a residual. +- `SessionManager.terminalAttached` is a Set that `cleanupSessionState` deletes when the shell exits. +- `TerminalLeaf` attaches once per sessionId (effect deps `[sessionId]`), after `ensureSessionLive`. +- A same-id respawn under a still-mounted leaf buffers the new shell's bytes but never forwards them: the xterm stays on the dead shell until a remount. The respawn happens through `recover` with `recoverTmuxName`, from a delivery, a control call, or a wake. +- There is no terminal detach at all, so the flag could not be kept past exit without leaking one id per closed terminal. +- `session:terminal-attach` ignores the sender, so a reloaded or crashed renderer could never release a reference either (#1283 item 3 for terminals). + +## Evidence +- Source: `sessionManager.ts` (`terminalAttached` add in `attachTerminal`; delete in `cleanupSessionState`; data forwarding gated on `has`), `ipc/session.ts` (`session:terminal-attach` handler), `TerminalLeaf.tsx` (attach in the `[sessionId]` effect, no detach in its cleanup). +- The agent-half fix and its tests (`sessionManager.ptyAttachWake.test.ts`, `ipc/session.test.ts` "raw PTY attach ownership (#1311)") are the recorded model this copies. + +## Decisions (defaults) +- **The attach describes the VIEW, not the process.** `terminalAttached` becomes a count, which process cleanup no longer touches. A new `detachTerminal` releases one reference. +- **A missing session still takes a reference.** A leaf can mount while its shell is down, and the respawn then forwards. A kind mismatch (an agent id) takes none, and the IPC layer records only references main took. +- **Renderer ownership:** the IPC ownership table for agent-PTY attaches becomes one generic per-page table, keyed by view type and session. Terminal attach and detach use it with the page document, like agent PTYs. A reload or destroy releases a page's terminal references too; a stale page's late detach is ignored. +- **TerminalLeaf detaches in its effect cleanup** when its attach resolved; a late attach after unmount detaches itself, as `AgentInlineTerminal` does. +- **No terminal resize restore:** plain terminals have none today, so none is added. + +## Tests +- **Manager:** a still-attached terminal view keeps forwarding across exit and a same-id respawn, and its detach ends forwarding. Red on main. +- **IPC:** terminal attaches are released on reload and destroy; a stale page's detach is ignored; a kind-mismatch attach takes no reference. +- **Renderer:** TerminalLeaf detaches on unmount, and a late attach after unmount releases itself. diff --git a/src/main/goalLoop/GoalLoopService.test.ts b/src/main/goalLoop/GoalLoopService.test.ts index 5d4fd0d73..d172e38f5 100644 --- a/src/main/goalLoop/GoalLoopService.test.ts +++ b/src/main/goalLoop/GoalLoopService.test.ts @@ -7,6 +7,7 @@ import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import type { SessionManager } from '@main/sessionManager.js' import type { PromptDeliveryResult } from '@shared/types/providerConfig.js' +import type { GoalLoopState } from '@shared/types/goalLoop.js' import { buildGoalLoopContinuationPrompt } from '@mcp/shared/goalLoopPrompt.js' import { GOAL_LOOP_ACTIVITY_GRACE_MS, GOAL_LOOP_BACKGROUND_WORK_LIMIT_MS, GOAL_LOOP_HOLD_LIMIT_MS, GOAL_LOOP_HOLD_STALL_MS, GOAL_LOOP_QUIET_TURN_MS, GoalLoopService, @@ -18,7 +19,17 @@ afterEach(async () => { await Promise.all(directories.splice(0).map(d => rm(d, { type Deliver = SessionManager['deliverPromptToAgent'] type FakeManager = EventEmitter & { deliverPromptToAgent: Deliver; getProcessStateSnapshot: () => { active: boolean } } -async function service(deliver: Deliver = vi.fn(async () => ({ ok: true } as PromptDeliveryResult))) { +/** Persistence that completes at once, so real disk latency cannot hide an + * ordering a slow disk happens to mask (#1314). */ +const instantStore = () => { + let saved: Record = {} + return { + read: async () => saved, + write: async (states: Record) => { saved = states }, + } as unknown as GoalLoopStore +} + +async function service(deliver: Deliver = vi.fn(async () => ({ ok: true } as PromptDeliveryResult)), store?: GoalLoopStore) { const directory = await mkdtemp(join(tmpdir(), 'agent-code-goal-loop-')) directories.push(directory) // The provider's activity level, mutable so a test can put the agent back @@ -26,7 +37,7 @@ async function service(deliver: Deliver = vi.fn(async () => ({ ok: true } as Pro // boundary, not about #1033's delivery hold. const processState = { active: false } const manager = Object.assign(new EventEmitter(), { deliverPromptToAgent: deliver, getProcessStateSnapshot: () => processState }) as FakeManager - const svc = new GoalLoopService({ manager, store: new GoalLoopStore(join(directory, 'goal-loop.json')), now: () => new Date('2026-09-18T00:00:00.000Z') }) + const svc = new GoalLoopService({ manager, store: store ?? new GoalLoopStore(join(directory, 'goal-loop.json')), now: () => new Date('2026-09-18T00:00:00.000Z') }) await svc.start() return { svc, manager, deliver, processState, storePath: join(directory, 'goal-loop.json') } } @@ -378,26 +389,67 @@ describe('GoalLoopService', () => { } finally { vi.useRealTimers() } }) - it('unrelated traffic cannot renew the screen grace (#1033 round 4)', async () => { + // #1314: this case once asserted "exactly one delivery in 120 s" while its own + // traffic carried a second, legitimate turn boundary (the pair at 60 s ends + // the turn our 45 s delivery started). Whether the resulting second + // continuation landed inside the window depended only on how long the first + // delivery's REAL `persist()` took relative to fake time — slow disk: 1, + // prompt disk: 2 — so it failed on a loaded CI runner. An instant store + // reproduced it 3/3. The case now asserts only its claim — traffic during + // the hold does not renew the grace, so the first delivery lands at exactly + // 45 s — and runs against both stores so persistence latency cannot decide it. + it.each([ + ['the file store', undefined], + ['an instant store', instantStore], + ] as const)('unrelated traffic cannot renew the screen grace (#1033 round 4), with %s', async (_label, makeStore) => { // The screen is the signal that latches on a stale row, so the time it is // allowed to hold a delivery must be a real bound — not one any passing // event can renew. With a latched active screen and an idle phase, turn // metadata arriving every 30 s held a continuation forever. - const { svc, manager, deliver, processState } = await service() + const { svc, manager, deliver, processState } = await service(undefined, makeStore?.()) vi.useFakeTimers() try { await svc.startLoop('s1', { goal: 'G.', loopPrompt: 'P.' }) processState.active = true idleTurn(manager) - for (let tick = 0; tick < 4; tick += 1) { + for (let tick = 0; tick < 2; tick += 1) { manager.emit('semantic-event', { sessionId: 's1', event: { type: 'turn_started', turnId: `t${tick}` } }) manager.emit('semantic-event', { sessionId: 's1', event: { type: 'turn_completed', turnId: `t${tick}` } }) - await vi.advanceTimersByTimeAsync(30_000) + await vi.advanceTimersByTimeAsync(tick === 0 ? 30_000 : 14_999) } + // 44.999 s: the 30 s traffic did not restart the 45 s grace, and it has + // not expired yet either. + expect(deliver).not.toHaveBeenCalled() + // 45.000 s exactly (review of #1337: asserting at 46 s let a 46 s grace pass). + await vi.advanceTimersByTimeAsync(1) expect(deliver).toHaveBeenCalledTimes(1) } finally { vi.useRealTimers() } }) + // The other half of #1314: the turn our 45 s delivery started ends at 60 s, and + // with the screen still latched that boundary gets its OWN bounded grace — a + // second continuation at 105 s, not at once. Instant store only: with the file + // store, whether the 60 s trigger is processed inside this window depends on + // real disk latency, which is exactly what made the old case unstable. + it('a turn boundary after our delivery gets a fresh bounded grace, not an immediate prompt (#1314)', async () => { + const { svc, manager, deliver, processState } = await service(undefined, instantStore()) + vi.useFakeTimers() + try { + await svc.startLoop('s1', { goal: 'G.', loopPrompt: 'P.' }) + processState.active = true + idleTurn(manager) + await vi.advanceTimersByTimeAsync(45_000) + expect(deliver).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(15_000) + manager.emit('semantic-event', { sessionId: 's1', event: { type: 'turn_started', turnId: 't-ours' } }) + manager.emit('semantic-event', { sessionId: 's1', event: { type: 'turn_completed', turnId: 't-ours' } }) + await vi.advanceTimersByTimeAsync(44_999) + expect(deliver).toHaveBeenCalledTimes(1) + await vi.advanceTimersByTimeAsync(1) + expect(deliver).toHaveBeenCalledTimes(2) + } finally { vi.useRealTimers() } + }) + it('flow bookkeeping is not progress, so it cannot postpone a stall forever (#1033 round 5)', async () => { // `flow_selected` / `flow_ignored` report which upstream call the proxy // renders from — title generation, a retry, a subagent's stream. They say @@ -799,8 +851,8 @@ describe('GoalLoopService hook turns that end without a Stop (#1028 review)', () // (Codex reasoning with no summary deltas, a long tool, Resume during a // running tool) are the busy cases Resume must NOT mistake for it. afterEach(() => { vi.useRealTimers() }) - const hookTurn = async () => { - const ctx = await service() + const hookTurn = async (store?: GoalLoopStore) => { + const ctx = await service(undefined, store) vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'Date'] }) await ctx.svc.startLoop('s1', { goal: 'G.', loopPrompt: 'P.' }) ctx.svc.observeProviderHook('s1', 'post-tool-use') @@ -934,7 +986,10 @@ describe('GoalLoopService hook turns that end without a Stop (#1028 review)', () }) it('Resume right after our own delivery waits for that turn', async () => { - const { svc, manager, deliver } = await hookTurn() + // Instant store (review of #1337): with the file store the delivery's real + // persist() is still in flight when Resume fires, which parks the trigger + // and hid a missing markOwedATurn from this case. + const { svc, manager, deliver } = await hookTurn(instantStore()) idleTurn(manager) svc.observeProviderHook('s1', 'stop', { blocked: false }) await vi.advanceTimersByTimeAsync(0) diff --git a/src/main/ipc/session.test.ts b/src/main/ipc/session.test.ts index 4781e2744..79d8cc4a4 100644 --- a/src/main/ipc/session.test.ts +++ b/src/main/ipc/session.test.ts @@ -332,6 +332,58 @@ describe('raw PTY attach ownership (#1311)', () => { }) }) +// #1281 / #1283 item 3 (terminal half): a plain terminal's view reference now +// outlives the shell too, so it gets the same page ownership as the agent PTY +// above. Before, the manager flag had no release at all, and a reloaded or +// crashed renderer could never give one back. +describe('terminal attach ownership (#1281)', () => { + function register(id: number) { + const detached: string[] = [] + const manager = Object.assign(new EventEmitter(), { + attachTerminal: (sessionId: string) => (sessionId === 'agent-pane' ? null : 'replay'), + detachTerminal: (sessionId: string) => { detached.push(sessionId) }, + detachAgentPty: vi.fn(), + }) + registerSessionIpc(manager as never, {} as never, new SessionFeedTap(manager as never)) + const sender = Object.assign(new EventEmitter(), { id }) + return { + detached, + sender, + attach: (sessionId: string, document: string) => harness.handlers.get('session:terminal-attach')!({ sender }, sessionId, document), + detach: (sessionId: string, document: string) => harness.handlers.get('session:terminal-detach')!({ sender }, sessionId, document), + announce: (document: string) => harness.handlers.get('session:screen-document')!({ sender }, document), + } + } + + it('releases a page\'s terminal views when it reloads or dies, and ignores its late detach', () => { + const page = register(6161) + page.announce('doc-1') + expect(page.attach('shell', 'doc-1')).toBe('replay') + // Not a terminal: main took no reference, so the reload releases none. + expect(page.attach('agent-pane', 'doc-1')).toBe('') + page.announce('doc-2') + expect(page.detached).toEqual(['shell']) + // The dead page's queued detach must not take the new page's reference. + page.attach('shell', 'doc-2') + page.detach('shell', 'doc-1') + expect(page.detached).toEqual(['shell']) + page.sender.emit('destroyed') + expect(page.detached).toEqual(['shell', 'shell']) + }) + + it('passes a leaf\'s detach through once per attach, and a stale page gets the replay without a reference', () => { + const page = register(6262) + page.announce('doc') + page.attach('shell', 'doc') + page.detach('shell', 'doc') + page.detach('shell', 'doc') + expect(page.detached).toEqual(['shell']) + // A page that already reloaded away takes nothing that could leak. + expect(page.attach('shell', 'old-doc')).toBe('') + page.sender.emit('destroyed') + expect(page.detached).toEqual(['shell']) + }) +}) // #1267 (steering q22 at the source): session:spawn relayed the raw provider // exception over IPC, where every renderer surface had to remember not to diff --git a/src/main/ipc/session.ts b/src/main/ipc/session.ts index 767b303f3..94c137474 100644 --- a/src/main/ipc/session.ts +++ b/src/main/ipc/session.ts @@ -221,15 +221,6 @@ export function registerSessionIpc( return manager.getSessionKind(sessionId) }) - // Terminal attach/replay. Called once by TerminalLeaf on mount. - // Returns the full buffered output of the session so far AND flips - // the manager's "attached" flag so subsequent PTY data events - // broadcast live. See SessionManager.terminalBuffers for the race - // being fixed. - ipcMain.handle('session:terminal-attach', (_evt, sessionId: string) => { - return manager.attachTerminal(sessionId) - }) - // Snapshot for a renderer that restored after the last change event. The // monitor emits on change only, so without this a reload would show every // busy shell idle until its foreground moved again. Filtered to the caller's @@ -243,30 +234,57 @@ export function registerSessionIpc( ) }) - // Agent PTY attach/replay. DebugPanel uses this for Claude - // and Codex panes when the user asks to see the raw underlying TUI - // as an xterm terminal. Kept separate from terminal-attach because - // plain terminal panes and agent panes have different primary - // renderers and different live IPC channels. + // Raw views (agent PTY xterms and plain-terminal leaves) are OWNED by the + // calling renderer page (#1311 review for agents, #1283 item 3 for + // terminals), the same lifecycle as the screen leases below. Main's attach + // counts outlive the process (a same-id wake keeps a mounted view live, + // #1281), so a renderer that reloads or crashes without running its leaf + // cleanup would otherwise pin a count forever: bytes forwarded to nobody, + // and for agents a restore size applied to some later process. Each + // renderer's outstanding attaches are released when it is destroyed or + // loads a new document. // - // Attaches are OWNED by the calling renderer page (#1311 review), the same - // lifecycle as the screen leases below. The attach count now outlives the - // provider process (a same-id wake keeps a mounted view live), so a - // renderer that reloads or crashes without running its leaf cleanup would - // otherwise pin a count forever: raw PTY bytes forwarded to nobody, and a - // restore size applied to some later process. Each renderer's outstanding - // attaches are released when it is destroyed or loads a new document. - const agentPtyAttaches = new Map>() - // Each renderer's current page document, so only a real reload releases. - const agentPtyDocuments = new Map() - const releaseAgentPtyAttaches = (owner: number): void => { - const owned = agentPtyAttaches.get(owner) - if (!owned) return - agentPtyAttaches.delete(owner) - for (const [sessionId, count] of owned) { - for (let i = 0; i < count; i += 1) manager.detachAgentPty(sessionId) + // WHY one table per view type from one factory, rather than the agent-only + // table this used to be plus a copy for terminals: the terminal half of + // #1281 needs exactly the same take / give-back-only-what-you-hold / + // release-on-reload rules, and two hand-written copies are how one of them + // drifts. `detach` is the manager call one reference is worth. + const rawViewOwnership = (detach: (sessionId: string) => void) => { + const byOwner = new Map>() + return { + take(owner: number, sessionId: string): void { + const owned = byOwner.get(owner) ?? new Map() + owned.set(sessionId, (owned.get(sessionId) ?? 0) + 1) + byOwner.set(owner, owned) + }, + /** Give back one reference this owner holds; one it does not hold is ignored. */ + give(owner: number, sessionId: string): void { + const owned = byOwner.get(owner) + const count = owned?.get(sessionId) ?? 0 + if (count === 0) return + if (count === 1) owned!.delete(sessionId) + else owned!.set(sessionId, count - 1) + if (owned!.size === 0) byOwner.delete(owner) + detach(sessionId) + }, + releaseOwner(owner: number): void { + const owned = byOwner.get(owner) + if (!owned) return + byOwner.delete(owner) + for (const [sessionId, count] of owned) { + for (let i = 0; i < count; i += 1) detach(sessionId) + } + }, } } + const agentPtyViews = rawViewOwnership(sessionId => manager.detachAgentPty(sessionId)) + const terminalViews = rawViewOwnership(sessionId => manager.detachTerminal(sessionId)) + // Each renderer's current page document, so only a real reload releases. + const rawViewDocuments = new Map() + const releaseRawViews = (owner: number): void => { + agentPtyViews.releaseOwner(owner) + terminalViews.releaseOwner(owner) + } // Is this call from the renderer's CURRENT page? #1311 round 2 (review A): // a reload can keep the webContents id, so the sender alone cannot tell // the dead page's queued detach from the live page's; without the page @@ -275,39 +293,61 @@ export function registerSessionIpc( // once per load, the same token the screen leases carry). A renderer that // has not announced yet adopts the caller's document, which its own // announcement then confirms. - const isCurrentAgentPtyDocument = (owner: number, document: string): boolean => { - const current = agentPtyDocuments.get(owner) + const isCurrentRawViewDocument = (owner: number, document: string): boolean => { + const current = rawViewDocuments.get(owner) if (current === undefined) { - agentPtyDocuments.set(owner, document) + rawViewDocuments.set(owner, document) return true } return current === document } + + // Agent PTY attach/replay. DebugPanel uses this for Claude + // and Codex panes when the user asks to see the raw underlying TUI + // as an xterm terminal. Kept separate from terminal-attach because + // plain terminal panes and agent panes have different primary + // renderers and different live IPC channels. ipcMain.handle('session:agent-pty-attach', (evt, sessionId: string, document: string) => { const owner = watchLeaseOwner(evt.sender) // A dead page's attach must take no reference: nothing would release it, // because that page's release already ran. - if (!isCurrentAgentPtyDocument(owner, document)) return null + if (!isCurrentRawViewDocument(owner, document)) return null const buffer = manager.attachAgentPty(sessionId) // null = no backend, and main took no reference; nothing to own. if (buffer === null) return buffer - const owned = agentPtyAttaches.get(owner) ?? new Map() - owned.set(sessionId, (owned.get(sessionId) ?? 0) + 1) - agentPtyAttaches.set(owner, owned) + agentPtyViews.take(owner, sessionId) return buffer }) ipcMain.handle('session:agent-pty-detach', (evt, sessionId: string, document: string) => { // Only a reference the CURRENT page holds. After a reload released the // old page's references, its late detach must not take the new page's. - if (agentPtyDocuments.get(evt.sender.id) !== document) return - const owned = agentPtyAttaches.get(evt.sender.id) - const count = owned?.get(sessionId) ?? 0 - if (count === 0) return - if (count === 1) owned!.delete(sessionId) - else owned!.set(sessionId, count - 1) - if (owned!.size === 0) agentPtyAttaches.delete(evt.sender.id) - manager.detachAgentPty(sessionId) + if (rawViewDocuments.get(evt.sender.id) !== document) return + agentPtyViews.give(evt.sender.id, sessionId) + }) + + // Terminal attach/replay. Called once by TerminalLeaf per session id. + // Returns the full buffered output of the session so far AND takes a view + // reference so subsequent PTY data events broadcast live. See + // SessionManager.terminalBuffers for the race being fixed. Same page + // ownership as the agent PTY above (#1283 item 3): the reference used to be + // a manager flag no renderer could release, cleared only by the shell's + // exit, which is what froze a mounted leaf across a same-id respawn (#1281). + ipcMain.handle('session:terminal-attach', (evt, sessionId: string, document: string) => { + const owner = watchLeaseOwner(evt.sender) + // A dead page gets the replay for nothing: no reference, since nothing + // would release it. '' keeps the renderer contract a plain string. + if (!isCurrentRawViewDocument(owner, document)) return '' + const buffer = manager.attachTerminal(sessionId) + // null = not a terminal: main took no reference, so there is none to own. + if (buffer === null) return '' + terminalViews.take(owner, sessionId) + return buffer + }) + + ipcMain.handle('session:terminal-detach', (evt, sessionId: string, document: string) => { + if (rawViewDocuments.get(evt.sender.id) !== document) return + terminalViews.give(evt.sender.id, sessionId) }) // #762. Live `session:screen` frames are forwarded only while a renderer @@ -327,8 +367,8 @@ export function registerSessionIpc( leaseOwnersWatched.add(owner) sender.once('destroyed', () => { screenInterest.dropOwner(owner) - releaseAgentPtyAttaches(owner) - agentPtyDocuments.delete(owner) + releaseRawViews(owner) + rawViewDocuments.delete(owner) leaseOwnersWatched.delete(owner) }) } @@ -339,11 +379,12 @@ export function registerSessionIpc( ipcMain.handle('session:screen-document', (evt, document: string): void => { const owner = watchLeaseOwner(evt.sender) screenInterest.enterDocument(owner, document) - // A NEW document is a page load: the previous page's raw PTY attaches - // died with it. A re-announce of the live document changes nothing. - const previous = agentPtyDocuments.get(owner) - agentPtyDocuments.set(owner, document) - if (previous !== undefined && previous !== document) releaseAgentPtyAttaches(owner) + // A NEW document is a page load: the previous page's raw views (agent + // PTY and terminal attaches) died with it. A re-announce of the live + // document changes nothing. + const previous = rawViewDocuments.get(owner) + rawViewDocuments.set(owner, document) + if (previous !== undefined && previous !== document) releaseRawViews(owner) }) ipcMain.handle('session:screen-lease', (evt, sessionId: string, document: string): void => { screenInterest.acquire(watchLeaseOwner(evt.sender), sessionId, document) diff --git a/src/main/sessionManager.terminalAttachRespawn.test.ts b/src/main/sessionManager.terminalAttachRespawn.test.ts new file mode 100644 index 000000000..ad560d9c7 --- /dev/null +++ b/src/main/sessionManager.terminalAttachRespawn.test.ts @@ -0,0 +1,102 @@ +import { EventEmitter } from 'node:events' +import { expect, it, vi } from 'vitest' + +// #1281 (terminal half): a mounted TerminalLeaf attaches once per session id, +// but process cleanup cleared the attach when the shell exited. A same-id +// respawn under the still-mounted leaf (recover from a delivery, a control +// call, a wake) then buffered the new shell's bytes and never forwarded them: +// the xterm stayed on the dead shell until a remount. The agent half of the +// same bug is sessionManager.ptyAttachWake.test.ts. +const terminals = vi.hoisted(() => ({ created: [] as EventEmitter[] })) +vi.mock('@shared/runtime/terminalSession.js', () => ({ + TerminalSession: class FakeTerminalSession extends EventEmitter { + constructor() { super(); terminals.created.push(this) } + async start(): Promise { this.emit('started') } + async stop(): Promise {} + write(): void {} + resize(): void {} + }, +})) +vi.mock('@main/workspaceDirectory.js', () => ({ + MissingWorkspaceDirectoryError: class extends Error {}, + assertWorkspaceDirectoryExists: vi.fn(async () => {}), +})) +vi.mock('@main/setup/toolchain.js', () => ({ getToolPath: () => '/usr/bin/true' })) +vi.mock('@main/performance/PerformanceService.js', () => ({ + performanceService: { mark: vi.fn(), record: vi.fn(), error: vi.fn(), metric: vi.fn(), span: () => ({ end: vi.fn(), fail: vi.fn() }) }, +})) +vi.mock('@main/storage/feedDebugLog.js', () => ({ forgetFeedDebugSession: vi.fn() })) + +// Imported once at module scope (#1333 review a): a cold import of the +// manager's graph took ~4 s inside the first test and hit the default 5 s +// timeout on a contended machine. Collection time is not test time. +const { SessionManager } = await import('./sessionManager') + +async function managerWithShell() { + // No tmux: a direct PTY terminal, the case every machine without tmux runs. + const manager = new SessionManager({ isAvailable: () => false, getBinary: () => null } as never) + const forwarded: string[] = [] + manager.on('terminal-data', event => forwarded.push(event.data)) + const { sessionId } = await manager.spawn({ kind: 'terminal', cwd: '/tmp/project' }) + return { manager, forwarded, sessionId } +} + +it('keeps forwarding to a still-attached terminal view after its shell exits and respawns under the same id', async () => { + const { manager, forwarded, sessionId } = await managerWithShell() + manager.attachTerminal(sessionId) + const first = terminals.created.at(-1)! + first.emit('data', 'old shell') + first.emit('exit', { exitCode: 0 }) + + await manager.recover({ sessionId, kind: 'terminal', cwd: '/tmp/project' }) + const second = terminals.created.at(-1)! + expect(second).not.toBe(first) + second.emit('data', 'fresh shell') + expect(forwarded).toEqual(['old shell', 'fresh shell']) + + // The view's own detach still ends forwarding. + manager.detachTerminal(sessionId) + second.emit('data', 'after detach') + expect(forwarded).not.toContain('after detach') + await manager.kill(sessionId) +}) + +it('forgets the view when the pane detaches while its shell is down', async () => { + const { manager, forwarded, sessionId } = await managerWithShell() + manager.attachTerminal(sessionId) + terminals.created.at(-1)!.emit('exit', { exitCode: 0 }) + manager.detachTerminal(sessionId) + await manager.recover({ sessionId, kind: 'terminal', cwd: '/tmp/project' }) + terminals.created.at(-1)!.emit('data', 'nobody is watching') + expect(forwarded).toEqual([]) + await manager.kill(sessionId) +}) + +// Two views of one shell in the same renderer (a lane and a retained +// Spotlight copy): one closing must not cut the other off. Two WINDOWS cannot +// both receive a shell's bytes: the window router gives a session one owner +// (#1333 review c), so this is about views within the receiving renderer. +it('keeps forwarding while any view of the shell is still attached', async () => { + const { manager, forwarded, sessionId } = await managerWithShell() + manager.attachTerminal(sessionId) + manager.attachTerminal(sessionId) + manager.detachTerminal(sessionId) + terminals.created.at(-1)!.emit('data', 'still watched') + expect(forwarded).toEqual(['still watched']) + await manager.kill(sessionId) +}) + +// #1333 review (a, b, c; surviving mutant): a leaf can attach while its shell +// is down (it exited between the leaf's wake and its attach reaching main). +// That attach must still take a reference, or the same-id respawn buffers its +// bytes and the leaf stays frozen: the bug this file is about, one step +// earlier. +it('forwards a respawned shell to a view that attached while the shell was down', async () => { + const { manager, forwarded, sessionId } = await managerWithShell() + terminals.created.at(-1)!.emit('exit', { exitCode: 0 }) + expect(manager.attachTerminal(sessionId)).toBe('') + await manager.recover({ sessionId, kind: 'terminal', cwd: '/tmp/project' }) + terminals.created.at(-1)!.emit('data', 'after absent attach') + expect(forwarded).toEqual(['after absent attach']) + await manager.kill(sessionId) +}) diff --git a/src/main/sessionManager.terminalReplay.test.ts b/src/main/sessionManager.terminalReplay.test.ts index b37cd9972..943e8da31 100644 --- a/src/main/sessionManager.terminalReplay.test.ts +++ b/src/main/sessionManager.terminalReplay.test.ts @@ -40,8 +40,9 @@ it('a remounted shell terminal gets the alternate screen and mouse mode its evic for (let written = 0; written < 300 * 1024; written += frame.length) shell.emit('data', frame) const replay = manager.attachTerminal(sessionId) + expect(replay).not.toBeNull() const terminal = new Terminal({ cols: 120, rows: 36, allowProposedApi: true }) - await new Promise(done => terminal.write(replay, done)) + await new Promise(done => terminal.write(replay!, done)) expect(terminal.buffer.active.type).toBe('alternate') expect(terminal.modes.mouseTrackingMode).toBe('vt200') await manager.kill(sessionId) diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index 96dec9f75..3d463350c 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -808,7 +808,14 @@ export class SessionManager extends EventEmitter { // cappedTextBuffer.ts — the string version copied the whole cap on every // chunk and retained sliced-string parents (#726). private readonly terminalBuffers = new Map() - private readonly terminalAttached = new Set() + // How many renderer views of each shell are attached. A count, not a flag, + // because two views in the receiving renderer (a lane and a retained + // Spotlight copy) can show one shell and one closing must + // not cut the other off; and it describes the VIEWS, not the process, so + // process cleanup leaves it alone (#1281, see cleanupSessionState). + // detachTerminal is the only release, and the IPC layer releases a + // renderer page's references when it reloads or is destroyed (#1283). + private readonly terminalAttachCounts = new Map() // Shell activity producer (#865). Emits on a channel of its own, NOT // 'process-state': the remote tap and the recorder subscribe to @@ -1093,7 +1100,14 @@ export class SessionManager extends EventEmitter { this.sessionSizes.delete(sessionId) if (kind === 'terminal') { this.terminalBuffers.delete(sessionId) - this.terminalAttached.delete(sessionId) + // NOT terminalAttachCounts (#1281, terminal half): a TerminalLeaf stays + // mounted while its shell exits and is respawned under the SAME id (a + // delivery, a control call or a wake goes through recover without + // remounting it), and it attaches only once per id. Deleting the count + // here stopped forwarding to it: the xterm stayed on the dead shell + // while keystrokes reached the new one unseen. The agent PTY count + // below is kept for the same reason. The leaf's own detachTerminal (on + // unmount, pane close included) is the release. this.terminalForeground.untrack(sessionId) } else { this.agentPtyBuffers.delete(sessionId) @@ -3585,7 +3599,7 @@ export class SessionManager extends EventEmitter { // replayed when the renderer calls attachTerminal. See the // block comment on terminalBuffers for why this is // race-free. - if (this.terminalAttached.has(sessionId)) { + if ((this.terminalAttachCounts.get(sessionId) ?? 0) > 0) { this.emit('terminal-data', { sessionId, data }) } }) @@ -3731,16 +3745,17 @@ export class SessionManager extends EventEmitter { * see the queue logic in TerminalLeaf. * 3. Subsequent live events write directly to xterm. * - * Returns '' if the session doesn't exist or isn't a terminal — - * silently safe so a stale attach call on a dead session doesn't - * error. + * Returns the replay buffer and takes one view reference, or NULL when no + * reference was taken (the id is not a terminal). A session that does not + * exist right now DOES take a reference and returns '': a TerminalLeaf can + * mount while its shell is down, and a later respawn under the same id must + * forward to it (#1281). The IPC layer records only references main took, + * so a renderer's detach can never release one it does not hold. */ - attachTerminal(sessionId: string): string { + attachTerminal(sessionId: string): string | null { const entry = this.sessions.get(sessionId) if (!entry) { - // Caller is asking to attach to a session that's already gone. - // Silent empty-string is fine here; any TerminalLeaf that mounts - // for a dead session will simply see an empty xterm. + this.terminalAttachCounts.set(sessionId, (this.terminalAttachCounts.get(sessionId) ?? 0) + 1) return '' } if (entry.kind !== 'terminal') { @@ -3755,17 +3770,30 @@ export class SessionManager extends EventEmitter { `[SessionManager] attachTerminal called on non-terminal session`, { sessionId, kind: entry.kind }, ) - return '' + return null } // replay(), not read(): the modes the evicted bytes set come first (#843). const buffer = this.terminalBuffers.get(sessionId)?.replay() ?? '' - // Flip the attach flag in the SAME synchronous block as reading + // Take the reference in the SAME synchronous block as reading // the buffer. JavaScript is single-threaded and event emission // can only happen on a later tick, so nothing can sneak in. - this.terminalAttached.add(sessionId) + this.terminalAttachCounts.set(sessionId, (this.terminalAttachCounts.get(sessionId) ?? 0) + 1) return buffer } + /** + * Release one terminal view reference (a TerminalLeaf unmounting, or the + * IPC layer releasing a reloaded or destroyed renderer's references). The + * last one stops live forwarding; the buffer keeps accumulating for the + * next attach's replay. Works whether or not a shell exists right now, + * because the reference outlives the process (#1281). + */ + detachTerminal(sessionId: string): void { + const attachCount = this.terminalAttachCounts.get(sessionId) ?? 0 + if (attachCount > 1) this.terminalAttachCounts.set(sessionId, attachCount - 1) + else this.terminalAttachCounts.delete(sessionId) + } + /** * Agent PTY attach/replay entry point. * diff --git a/src/preload/api/session.ts b/src/preload/api/session.ts index 6477a2786..659e94f5e 100644 --- a/src/preload/api/session.ts +++ b/src/preload/api/session.ts @@ -120,8 +120,15 @@ export const sessionApi = { * and attach response are silently missed. See TerminalLeaf.tsx * for the queue pattern. */ + // Both carry this page's document, like the agent PTY calls below: main + // owns each terminal view reference per page, releases a reloaded page's, + // and ignores a dead page's late detach (#1281 / #1283). attachTerminal: (sessionId: string): Promise => - ipcRenderer.invoke('session:terminal-attach', sessionId), + ipcRenderer.invoke('session:terminal-attach', sessionId, screenLeaseDocument), + + /** Release this leaf's view of a shell (TerminalLeaf unmount). */ + detachTerminal: (sessionId: string): Promise => + ipcRenderer.invoke('session:terminal-detach', sessionId, screenLeaseDocument), /** * Attach this renderer to an agent runtime's raw PTY terminal. diff --git a/src/renderer/src/workspace/hook/actions/reloadAgentsOwnership.renderer.test.tsx b/src/renderer/src/workspace/hook/actions/reloadAgentsOwnership.renderer.test.tsx new file mode 100644 index 000000000..dc7547c8f --- /dev/null +++ b/src/renderer/src/workspace/hook/actions/reloadAgentsOwnership.renderer.test.tsx @@ -0,0 +1,535 @@ +import { readFileSync } from 'node:fs' +import { join } from 'node:path' + +import { act, cleanup, renderHook } from '@testing-library/react' +import { afterEach, expect, it, vi } from 'vitest' +import type { SessionSpawnOptions } from '@preload/api/types' +import { emptyRuntime, type SessionRuntime } from '@renderer/session-runtime/state' +import { mergeProjectTabs } from '@renderer/workspace/mergeProjectTabs' +import { useSessionActions } from './session' +import { makeRefs, stateWriter } from './testing/paneActionsHarness' +import type { SessionMeta, WorkspaceState } from '@renderer/workspace/types' + +// #1282: Reload Agents (Dangerous Agents toggle) walked a snapshot and +// re-checked nothing across its awaits. The workspace is the owner's +// sanitized persisted v3 workspace (a Claude and a Codex lane agent). The +// mocked edges are the main-process IPC calls (spawnSession, killOwnedSession, +// and the goal-loop control/carry calls) plus the history loader; spawn and +// kill can be held open so a test changes the workspace in the exact interval +// a user can, and the assertions are made INSIDE that interval where it is +// the dangerous one (#1326 round 2), not only on the final state. + +vi.mock('./initialHistory', () => ({ loadInitialHistoryForSession: vi.fn(async () => undefined) })) +const originalApi = window.api +afterEach(() => { cleanup(); window.api = originalApi }) + +const persisted = JSON.parse(readFileSync( + join(import.meta.dirname, '../../../../../../testing/fixtures/workspace-v3/2026-09-20-live-workspace.sanitized.json'), 'utf8', +)) as { windows: Array<{ workspace: { + projects: Array<{ id: string; title: string }> + activeProjectId: string + stage: WorkspaceState['stage'] + sessions: Record +} }> } + +function harness(heldKind: 'claude' | 'codex' = 'claude', opts: { heldSpawnRejects?: boolean } = {}) { + const recorded = persisted.windows[0]!.workspace + const [claudeLane, codexLane] = recorded.stage.lanes.map(lane => lane.selectedSessionId!) as [string, string] + const state = { + tabs: recorded.projects, + activeTabId: recorded.activeProjectId, + sessions: recorded.sessions, + pinnedSessionIds: [], + stage: recorded.stage, + } as unknown as WorkspaceState + const refs = makeRefs(state), writer = stateWriter(state, refs) + refs.latestRuntimesRef.current = { + [claudeLane]: { ...emptyRuntime(), processStatus: 'started', draftInput: 'before' }, + [codexLane]: { ...emptyRuntime(), processStatus: 'started' }, + } + const setRuntimes = (update: Record | ((prev: Record) => Record)) => { + refs.latestRuntimesRef.current = typeof update === 'function' ? update(refs.latestRuntimesRef.current) : update + } + // The Claude respawn is held until the test releases it. + let release!: () => void + const held = new Promise(resolve => { release = resolve }) + const spawnSession = vi.fn(async (options: SessionSpawnOptions) => { + if (options.kind === heldKind) { + await held + if (opts.heldSpawnRejects) throw new Error('spawn failed') + } + // A safe (dangerous-off) respawn gets its own id so two overlapping + // reloads' successors can be told apart. + const suffix = options.dangerousMode === false ? 'safe' : 'restarted' + return { sessionId: `${options.kind}-${suffix}`, providerSessionId: options.resumeSessionId } + }) + // Kills resolve immediately unless the test installs a hold for one id. + const killHolds = new Map>() + const killOwnedSession = vi.fn(async (req: { sessionId: string }) => killHolds.get(req.sessionId) ?? true) + window.api = { ...originalApi, spawnSession, killOwnedSession, controlGoalLoop: vi.fn(async () => null), carryGoalLoop: vi.fn(async () => null) } + const hook = renderHook(() => useSessionActions(state, writer.setState, setRuntimes, refs)) + // The Claude agent is first in the snapshot, so it is the one in flight. + const order = Object.keys(recorded.sessions).filter(id => id === claudeLane || id === codexLane) + return { hook, writer, refs, spawnSession, killOwnedSession, killHolds, release, claudeLane, codexLane, order } +} + +it('does not bring back an agent closed while its respawn was in flight', async () => { + const h = harness() + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + // The user closes the Claude agent while it is respawning. + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + const after = h.writer.getState() + expect(after.sessions['claude-restarted']).toBeUndefined() + expect(after.sessions[h.claudeLane]).toBeUndefined() + // Its new process is not left running unowned. + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) +}) + +it('does not double an agent replaced while its respawn was in flight', async () => { + const h = harness() + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + // A provider switch replaced the same agent meanwhile. + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + const meta = sessions[h.claudeLane]! + delete sessions[h.claudeLane] + sessions['switched-successor'] = meta + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + const after = h.writer.getState() + expect(after.sessions['switched-successor']).toBeDefined() + expect(after.sessions['claude-restarted']).toBeUndefined() + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) +}) + +it('skips an agent closed before the reload reached it', async () => { + const h = harness() + const second = h.order[1]! + const secondKind = h.writer.getState().sessions[second]!.kind + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledTimes(1)) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[second] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + expect(h.spawnSession).not.toHaveBeenCalledWith(expect.objectContaining({ kind: secondKind })) + expect(h.spawnSession).toHaveBeenCalledTimes(1) +}) + +it('keeps a draft typed, and an unread marker set, while the reload ran', async () => { + const h = harness() + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.refs.latestRuntimesRef.current = { + ...h.refs.latestRuntimesRef.current, + [h.claudeLane]: { ...h.refs.latestRuntimesRef.current[h.claudeLane]!, draftInput: 'typed during the reload', draftImages: [{ id: 'img-1' } as never], unreadSince: 1234, unreadKind: 'output' as never }, + } + await act(async () => { h.release(); await reload }) + expect(h.refs.latestRuntimesRef.current['claude-restarted']).toMatchObject({ + draftInput: 'typed during the reload', + draftImages: [{ id: 'img-1' }], + unreadSince: 1234, + unreadKind: 'output', + }) +}) + +// #1326 round-2 review C2: an agent respawned EARLY in the loop used to run +// unfiled until every later spawn returned, so closing its pane while a later +// spawn stalled killed only the old backend and left the successor running. +// Pinned on that interval, with the later spawn still held. +it('files an early successor before a later spawn returns, so closing it stops it', async () => { + const probe = harness() + const [first, second] = probe.order as [string, string] + cleanup() + const secondKind = probe.writer.getState().sessions[second]!.kind as 'claude' | 'codex' + const firstKind = probe.writer.getState().sessions[first]!.kind as string + const h = harness(secondKind) + // A child row that names the first agent as its parent, and a pin on it. + h.writer.setState(prev => ({ + ...prev, + pinnedSessionIds: [first], + sessions: { ...prev.sessions, 'child-of-first': { ...prev.sessions[second]!, kind: 'terminal', linkedParentId: first, orchestrationParentId: first } }, + })) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: secondKind }))) + // The later spawn is still pending; the early successor is already the pane, + // and everything that pointed at the old id points at it (#1326 + // verification C survivors: the relationship and pin remaps). + expect(h.writer.getState().sessions[`${firstKind}-restarted`]).toBeDefined() + expect(h.writer.getState().sessions['child-of-first']).toMatchObject({ linkedParentId: `${firstKind}-restarted`, orchestrationParentId: `${firstKind}-restarted` }) + expect(h.writer.getState().pinnedSessionIds).toEqual([`${firstKind}-restarted`]) + expect(h.writer.getState().sessions[first]).toBeUndefined() + expect(h.writer.getState().stage.lanes.map(lane => lane.selectedSessionId)).toContain(`${firstKind}-restarted`) + // The user closes it through the real close action: its backend is killed. + await act(async () => { await h.hook.result.current.killSession(`${firstKind}-restarted`, 'close.focused') }) + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: `${firstKind}-restarted`, caller: 'close.focused' })) + await act(async () => { h.release(); await reload }) + expect(h.writer.getState().sessions[`${firstKind}-restarted`]).toBeUndefined() +}) + +// #1326 round-2 review C1: a close that completes while reload's pre-spawn +// kill is in flight must not be followed by a spawn for the closed agent. +it('does not spawn for an agent closed while its old backend was being killed', async () => { + const h = harness() + let releaseKill!: (value: boolean) => void + h.killHolds.set(h.claudeLane, new Promise(resolve => { releaseKill = resolve })) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: h.claudeLane, caller: 'reload.agent-sessions' }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { releaseKill(true); h.release(); await reload }) + expect(h.spawnSession).not.toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' })) +}) + +// #1326 round-2 review A1: a fresh start (the old row's id was a provisional +// proxy-header one, which is never resumed) reports the new process's durable +// provider id; the successor must keep it, and its history load must run. +it('keeps the durable provider id a fresh respawn reports', async () => { + const h = harness() + h.writer.setState(prev => ({ + ...prev, + sessions: { ...prev.sessions, [h.claudeLane]: { ...prev.sessions[h.claudeLane]!, providerSessionId: 'provisional-id', providerSessionIdSource: 'proxy-header' } }, + })) + h.spawnSession.mockImplementation(async (options: SessionSpawnOptions) => ({ + sessionId: `${options.kind}-restarted`, + providerSessionId: options.resumeSessionId ?? `${options.kind}-native-new`, + })) + await act(async () => { h.release(); await h.hook.result.current.reloadAgentSessions(true) }) + expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude', resumeSessionId: undefined })) + expect(h.writer.getState().sessions['claude-restarted']).toMatchObject({ providerSessionId: 'claude-native-new', providerSessionIdSource: 'runtime-start' }) +}) + +// #1326 round-2 review C (survivor): a provider-runtime change on the same +// row (ensureSessionLive can update it after recovery) means the spawned +// successor runs the wrong runtime; it must be orphaned, not filed. +it('does not file a successor when the agent’s provider runtime changed mid-reload', async () => { + const h = harness() + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => ({ + ...prev, + sessions: { ...prev.sessions, [h.claudeLane]: { ...prev.sessions[h.claudeLane]!, providerRuntime: prev.sessions[h.claudeLane]!.providerRuntime === 'terminal' ? undefined : 'terminal' } }, + })) + await act(async () => { h.release(); await reload }) + expect(h.writer.getState().sessions['claude-restarted']).toBeUndefined() + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) +}) + +// #1326 review A1: the Dangerous Agents switch does not serialize clicks, so +// on -> off can start a second reload before the first settles. The last +// click must win: the safe successors are filed, the dangerous ones are not. +it('lets the later of two overlapping reloads decide the final mode', async () => { + const h = harness() + let on!: Promise, off!: Promise + await act(async () => { + on = h.hook.result.current.reloadAgentSessions(true) + off = h.hook.result.current.reloadAgentSessions(false) + }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + await act(async () => { h.release(); await on; await off }) + const sessions = h.writer.getState().sessions + expect(sessions['claude-safe']).toBeDefined() + expect(sessions['codex-safe']).toBeDefined() + expect(sessions['claude-restarted']).toBeUndefined() + expect(sessions['codex-restarted']).toBeUndefined() + // The dangerous successors were reloaded away, not left running. + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted' })) + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'codex-restarted' })) +}) + +// #1326 review A2: a project merge keeps the session id but re-files it under +// the target project and deletes the source. The successor must follow it. +it('files the successor under the project an agent was merged into mid-reload', async () => { + const h = harness() + const source = h.writer.getState().sessions[h.claudeLane]!.projectId! + const target = h.writer.getState().tabs.find(tab => tab.id !== source)!.id + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => { + const merged = mergeProjectTabs(prev, { sourceTabIds: [source], targetTabId: target, now: Date.now() }) + if (!merged.ok) throw new Error(merged.reason) + return merged.state + }) + await act(async () => { h.release(); await reload }) + expect(h.writer.getState().sessions['claude-restarted']?.projectId).toBe(target) +}) + +// #1326 review A3/B1: orphan kills used to be awaited INSIDE the commit-time +// check loop, so a close during one was never re-checked. The commit must be +// done, with no await, before any orphan kill starts. +it('commits before it awaits any orphan kill', async () => { + const probe = harness() + const [first, second] = probe.order as [string, string] + cleanup() + const secondKind = probe.writer.getState().sessions[second]!.kind as 'claude' | 'codex' + const firstKind = probe.writer.getState().sessions[first]!.kind as string + const h = harness(secondKind) + let releaseKill!: (value: boolean) => void + h.killHolds.set(`${secondKind}-restarted`, new Promise(resolve => { releaseKill = resolve })) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: secondKind }))) + // The second agent is closed while its own respawn is in flight. + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[second] + return { ...prev, sessions } + }) + await act(async () => { h.release() }) + await vi.waitFor(() => expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: `${secondKind}-restarted` }))) + // While that orphan's kill is still pending, the first agent is already + // committed under its successor id; a close now targets that live row. + expect(h.writer.getState().sessions[`${firstKind}-restarted`]).toBeDefined() + expect(h.writer.getState().sessions[first]).toBeUndefined() + await act(async () => { releaseKill(true); await reload }) +}) + +// #1326 review A4: a rejected orphan kill must neither abort the reload nor +// strand the other successor unfiled. +it('files the other successors when an orphan kill rejects', async () => { + const probe = harness() + const [first, second] = probe.order as [string, string] + cleanup() + const secondKind = probe.writer.getState().sessions[second]!.kind as 'claude' | 'codex' + const firstKind = probe.writer.getState().sessions[first]!.kind as string + const h = harness(secondKind) + h.killHolds.set(`${secondKind}-restarted`, Promise.reject(new Error('kill IPC failed'))) + h.killHolds.get(`${secondKind}-restarted`)!.catch(() => undefined) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: secondKind }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[second] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + expect(h.writer.getState().sessions[`${firstKind}-restarted`]).toBeDefined() + expect(h.writer.getState().sessions[`${secondKind}-restarted`]).toBeUndefined() + // #1326 round-2 review C3: not swallowed. Retried once at once, then kept + // and retried by the next reload, which stops it once the IPC recovers. + const orphanKills = () => h.killOwnedSession.mock.calls.filter(([req]) => (req as { sessionId: string }).sessionId === `${secondKind}-restarted`).length + expect(orphanKills()).toBe(2) + h.killHolds.delete(`${secondKind}-restarted`) + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(3) + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(3) +}) + +// #1326 review A5: a failed respawn of an agent closed meanwhile has no pane +// to show `failed` in; it must not recreate a runtime under the closed id. +it('does not recreate a runtime for a closed agent whose respawn failed', async () => { + const h = harness('claude', { heldSpawnRejects: true }) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + const runtimes = { ...h.refs.latestRuntimesRef.current } + delete runtimes[h.claudeLane] + h.refs.latestRuntimesRef.current = runtimes + await act(async () => { h.release(); await reload }) + expect(h.refs.latestRuntimesRef.current[h.claudeLane]).toBeUndefined() +}) + +// #1326 review mutation survivors: the same id can stop being "the agent we +// set out to reload" without its row disappearing, by a cwd change or by its +// project going away. Each must orphan the successor. +it.each([ + ['its cwd changed', (prev: WorkspaceState, id: string) => ({ + ...prev, sessions: { ...prev.sessions, [id]: { ...prev.sessions[id]!, cwd: `${prev.sessions[id]!.cwd}-moved` } }, + })], + ['its project was removed', (prev: WorkspaceState, id: string) => ({ + ...prev, tabs: prev.tabs.filter(tab => tab.id !== prev.sessions[id]!.projectId), + })], +])('does not file a successor when %s mid-reload', async (_label, change) => { + const h = harness() + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => change(prev, h.claudeLane)) + await act(async () => { h.release(); await reload }) + expect(h.writer.getState().sessions['claude-restarted']).toBeUndefined() + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) +}) + +// #1326 round-2 review A (survivor): the old row's provisional proxy-header id +// is never resumed, so when the fresh process reports none, the successor must +// carry none; copying it would point history and resume at the old process. +it('does not carry a provisional provider id onto a fresh successor', async () => { + const h = harness() + h.writer.setState(prev => ({ + ...prev, + sessions: { ...prev.sessions, [h.claudeLane]: { ...prev.sessions[h.claudeLane]!, providerSessionId: 'provisional-id', providerSessionIdSource: 'proxy-header' } }, + })) + h.spawnSession.mockImplementation(async (options: SessionSpawnOptions) => ({ sessionId: `${options.kind}-restarted`, providerSessionId: undefined })) + await act(async () => { h.release(); await h.hook.result.current.reloadAgentSessions(true) }) + const successor = h.writer.getState().sessions['claude-restarted']! + expect(successor.providerSessionId).toBeUndefined() + expect(successor.providerSessionIdSource).toBeUndefined() +}) + +// Integration of #1324 (curated failure sentence on a failed respawn) with +// #1326 (ownership across awaits): one agent's respawn fails while the other +// is closed mid-reload. The failed pane shows main's curated sentence (never +// raw IPC text), and the closed one is neither filed nor left running. +it('shows the curated sentence on a failed respawn while a closed agent stays closed', async () => { + const h = harness('claude') + const cliMissing = 'codex CLI not found. Open Setup (File › Setup…) to install it or enter its path.' + let releaseClaude!: () => void + const claudeHeld = new Promise(resolve => { releaseClaude = resolve }) + h.spawnSession.mockImplementation(async (options: SessionSpawnOptions) => { + if (options.kind === 'codex') throw new Error(`Error invoking remote method 'session:spawn': Error: ${cliMissing}`) + await claudeHeld + return { sessionId: `${options.kind}-restarted`, providerSessionId: options.resumeSessionId } + }) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { releaseClaude(); await reload }) + const after = h.writer.getState() + // The failed Codex agent keeps its pane, id and a curated message. + expect(after.sessions[h.codexLane]).toBeDefined() + expect(h.refs.latestRuntimesRef.current[h.codexLane]).toMatchObject({ processStatus: 'failed', processError: cliMissing }) + // The closed Claude agent is not resurrected, and its successor is stopped. + expect(after.sessions['claude-restarted']).toBeUndefined() + expect(after.sessions[h.claudeLane]).toBeUndefined() + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) +}) + +// #1326 verification A/C: a successor whose kill failed is remembered, and +// the next reload must retry it EVEN WHEN no live agent is left to reload +// (the user closed them all, which is exactly when it would run unseen). +it('retries a remembered orphan when no live agent is left', async () => { + const h = harness('claude') + h.killHolds.set('claude-restarted', Promise.reject(new Error('kill IPC failed'))) + h.killHolds.get('claude-restarted')!.catch(() => undefined) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + // The Codex successor is the last live agent; the user closes it too. + await act(async () => { await h.hook.result.current.killSession('codex-restarted', 'close.focused') }) + const orphanKills = () => h.killOwnedSession.mock.calls.filter(([req]) => (req as { sessionId: string }).sessionId === 'claude-restarted').length + expect(orphanKills()).toBe(2) + h.killHolds.delete('claude-restarted') + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(3) +}) + +// #1326 verification C: `session:kill-owned` answers false both for "gone" +// and for "not yours" (the backend still runs). Only `true` may clear the +// retry record. +it('keeps retrying an orphan whose kill main refused', async () => { + const h = harness('claude') + h.killHolds.set('claude-restarted', Promise.resolve(false)) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + const orphanKills = () => h.killOwnedSession.mock.calls.filter(([req]) => (req as { sessionId: string }).sessionId === 'claude-restarted').length + expect(orphanKills()).toBe(1) + // Next reload: still refused, still remembered. + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(2) + // Main now stops it: the record clears and later reloads leave it alone. + h.killHolds.delete('claude-restarted') + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(3) + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(orphanKills()).toBe(3) +}) + +// #1326 verification A: an unowned row (its project gone) is announced as +// dropped by every reload, including one where every respawn fails and so +// nothing is committed. +it('drops unowned rows even when every respawn fails', async () => { + const h = harness('claude', { heldSpawnRejects: true }) + h.spawnSession.mockImplementation(async () => { throw new Error('spawn failed') }) + h.writer.setState(prev => ({ + ...prev, + sessions: { ...prev.sessions, ghost: { ...prev.sessions[h.claudeLane]!, projectId: 'deleted-project' } }, + })) + await act(async () => { await h.hook.result.current.reloadAgentSessions(true) }) + expect(h.writer.getState().sessions.ghost).toBeUndefined() + expect(h.refs.latestRuntimesRef.current[h.claudeLane]).toMatchObject({ processStatus: 'failed' }) +}) + +// #1326 second verification A: a project removed while an earlier spawn is in +// flight leaves rows that become unowned AFTER the up-front prune; the +// successor's commit must drop them too. +it('drops rows that became unowned while the reload ran', async () => { + const h = harness('claude') + const lanes = new Set([h.claudeLane, h.codexLane]) + const state = h.writer.getState() + const laneProjects = new Set([...lanes].map(id => state.sessions[id]!.projectId)) + const doomed = state.tabs.find(tab => !laneProjects.has(tab.id))!.id + const doomedRows = Object.entries(state.sessions).filter(([, meta]) => meta.projectId === doomed).map(([id]) => id) + expect(doomedRows.length).toBeGreaterThan(0) + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + // The project closes (its tab goes) while Claude's spawn is held. + h.writer.setState(prev => ({ ...prev, tabs: prev.tabs.filter(tab => tab.id !== doomed) })) + await act(async () => { h.release(); await reload }) + for (const id of doomedRows) expect(h.writer.getState().sessions[id]).toBeUndefined() +}) + +// #1326 second verification A/C: an orphan's startup feed can create its +// runtime entry before spawn resolves; a confirmed kill must drop it, or it +// stays behind as unreachable `started` state. +it('drops an orphan successor’s early runtime entry once its kill is confirmed', async () => { + const h = harness('claude') + let reload!: Promise + await act(async () => { reload = h.hook.result.current.reloadAgentSessions(true) }) + await vi.waitFor(() => expect(h.spawnSession).toHaveBeenCalledWith(expect.objectContaining({ kind: 'claude' }))) + h.refs.latestRuntimesRef.current = { ...h.refs.latestRuntimesRef.current, 'claude-restarted': { ...emptyRuntime(), processStatus: 'started' } } + h.writer.setState(prev => { + const sessions = { ...prev.sessions } + delete sessions[h.claudeLane] + return { ...prev, sessions } + }) + await act(async () => { h.release(); await reload }) + expect(h.killOwnedSession).toHaveBeenCalledWith(expect.objectContaining({ sessionId: 'claude-restarted', caller: 'reload.orphaned-successor' })) + expect(h.refs.latestRuntimesRef.current['claude-restarted']).toBeUndefined() +}) diff --git a/src/renderer/src/workspace/hook/actions/session.ts b/src/renderer/src/workspace/hook/actions/session.ts index fe47689ec..17509c627 100644 --- a/src/renderer/src/workspace/hook/actions/session.ts +++ b/src/renderer/src/workspace/hook/actions/session.ts @@ -360,6 +360,9 @@ function waitForSessionInputReady( }) } +/** A reload successor whose agent went away while it spawned. */ +type ReloadOrphan = { newId: SessionId; owner: Pick } + export function useSessionActions( state: { activeTabId: string; sessions: Record; tabs: Tab[] }, setState: WorkspaceSetState, @@ -1111,13 +1114,13 @@ export function useSessionActions( ], ) - const killSession = useCallback( - async ( - sessionId: SessionId, - caller: KillCaller, - capturedOwner?: Pick, - ) => { - await killSessionBackendIfOwned(refs, sessionId, caller, capturedOwner) + // Everything the renderer holds for a session, dropped once its backend is + // gone. Shared by killSession and reload's orphan kill (#1326 second + // verification A/C): an orphan's startup feed can create a runtime entry + // before its spawn resolves, and a kill that skipped this left that entry + // behind as unreachable `started` state. + const forgetSessionLocally = useCallback( + (sessionId: SessionId) => { setRuntimes(prev => { const next = { ...prev } delete next[sessionId] @@ -1148,6 +1151,18 @@ export function useSessionActions( [refs.bootstrapTimersRef, refs.seenUuidsRef, setRuntimes, setState], ) + const killSession = useCallback( + async ( + sessionId: SessionId, + caller: KillCaller, + capturedOwner?: Pick, + ) => { + await killSessionBackendIfOwned(refs, sessionId, caller, capturedOwner) + forgetSessionLocally(sessionId) + }, + [forgetSessionLocally, refs], + ) + // Kills the current session in the focused leaf and spawns a new // one in the same position. Used by the resume flow to swap a // session without changing the tile tree structure — the pane @@ -1482,8 +1497,50 @@ export function useSessionActions( // dangerous mode, then remaps visible panes and buried records // onto the fresh session ids. Plain terminal sessions are left // untouched. - const reloadAgentSessions = useCallback( - async (dangerousMode = refs.dangerousAgentsRef.current) => { + // Successors of a reload whose agent was closed while they spawned, and + // whose kill has failed twice; keyed by successor id. + const unstoppedReloadSuccessorsRef = useRef(new Map()) + // WHY a failed orphan kill is retried and remembered instead of swallowed + // (#1326 round-2 review C3): the successor is filed nowhere, so no pane + // exists from which the user could close it, and a swallowed rejection + // meant a hidden backend with no owner at all. A rejection here is an IPC + // or main-side failure, not "already gone" (that resolves false), so one + // immediate retry covers a transient one. After that the successor stays in + // `unstoppedReloadSuccessorsRef`, which the next reload retries first, and + // application quit kills every backend regardless (applicationShutdown's + // killAll), so it is never ownerless. It does not throw: the other agents + // of this reload must still be restarted and filed (#1326 review A4). + // The log line is curated: IPC error text can carry env values (q22). + // + // WHY a `false` answer is NOT treated as stopped (#1326 verification C): + // `session:kill-owned` answers false both for "nothing left to stop" and + // for "you do not own that backend" (a refused ownership proof or window + // lease), and in the second case the process is still running. The renderer + // cannot tell them apart, so only `true` clears the record. A backend that + // was already gone then costs one IPC call per later reload, which is the + // price of never forgetting one that is still alive. + const killOrphan = useCallback(async (orphan: ReloadOrphan): Promise => { + for (let attempt = 0; attempt < 2; attempt++) { + try { + if (await killSessionBackendIfOwned(refs, orphan.newId, 'reload.orphaned-successor', orphan.owner)) { + unstoppedReloadSuccessorsRef.current.delete(orphan.newId) + forgetSessionLocally(orphan.newId) + return + } + // Refused (or already gone): not retried at once, a refusal will not + // change within this reload. + break + } catch { + // Retried once below, then remembered. + } + } + unstoppedReloadSuccessorsRef.current.set(orphan.newId, orphan) + // eslint-disable-next-line no-console + console.warn('[workspace] could not confirm a reload successor of a closed agent stopped; retrying on the next reload:', orphan.newId) + }, [forgetSessionLocally, refs]) + + const reloadAgentSessionsNow = useCallback( + async (dangerousMode: boolean) => { const current = refs.stateRef.current const ownedIds = collectOwnedSessionIds(current) const staleIds = collectUnownedSessionIds(current) @@ -1523,42 +1580,84 @@ export function useSessionActions( const kind = meta.kind ?? DEFAULT_PROVIDER return isAgentProviderKind(kind) }) + // Successors an earlier reload spawned for an agent closed meanwhile + // and then could not confirm stopped (see killOrphan). Retried FIRST, + // before the no-live-agents return below (#1326 verification A/C): the + // user who closed every agent is exactly the one whose hidden + // successor would otherwise run until quit. + for (const orphan of [...unstoppedReloadSuccessorsRef.current.values()]) await killOrphan(orphan) + + // Pruned up front, not only inside each successor's commit + // (#1326 verification A): a reload whose every respawn failed commits + // nothing, and it still announced above that it drops these rows. + if (staleIds.length > 0) setState(prev => ({ ...prev, sessions: pickOwnedSessions(prev.sessions, collectOwnedSessionIds(prev)) })) + if (agentEntries.length === 0) return - const oldRuntimes = refs.latestRuntimesRef.current - const idMap = new Map() - // Old ids whose respawn failed. See the setState below for why these - // stay in the workspace instead of being removed (#1239). - const failedIds = new Map() - const freshSessions: Record = {} - // Successors that can still complete a goal loop; see carryGoalLoops. - const goalLoopCapable = new Set() + // Is `oldId` still the agent this loop set out to reload? (#1282) The + // loop awaits a kill and a spawn per agent while the user keeps + // working, and it used to re-check nothing: an agent closed meanwhile + // was respawned under a new id (an invisible orphan if its project had + // closed too), and one replaced meanwhile (a provider switch) got two + // successors on one provider transcript. Same rule as replaceSession's + // canCommit (#815). Read through the setState updater, the one place + // that sees the state as it is now, not as this closure captured it. + // + // Returns the LIVE row (not the captured one) so the commit files the + // successor with whatever changed meanwhile that does not affect the + // spawn: a project merge re-files the same id under another project + // (#1326 review A2), a rename changes its title. + const liveReloadable = (oldId: SessionId, meta: SessionMeta): SessionMeta | null => { + let live: SessionMeta | null = null + setState(prev => { + const current = prev.sessions[oldId] + if (current && current.cwd === meta.cwd && current.kind === meta.kind + && current.providerRuntime === meta.providerRuntime + && collectOwnedSessionIds(prev).has(oldId)) live = current + return prev + }) + return live + } + // WHY each agent is committed as soon as ITS spawn returns, and not in + // one batch at the end (#1326 round-2 review C): while an early + // successor waited for a later agent's slow spawn it was running but + // filed nowhere, so closing its pane killed only the OLD backend and the + // hidden successor kept working (with Dangerous Agents on, after Close). + // Round 1 had made the batch commit synchronous, which fixed the commit + // itself but not that interval. Filed immediately, the successor is the + // pane: a close from then on is an ordinary close of the new id. for (const [oldId, meta] of agentEntries) { + // Closed or replaced since the snapshot: nothing of ours to reload. + if (!liveReloadable(oldId, meta)) continue try { await killSessionBackendIfOwned(refs, oldId, 'reload.agent-sessions') } catch { // Kill failures still fall through to respawn — the old // process may already be gone. } + // Again after the kill's await (#1326 round-2 review C1): a close that + // finished meanwhile must not be followed by a spawn for it. + if (!liveReloadable(oldId, meta)) continue delete refs.seenUuidsRef.current[oldId] clearLiveEntryWindowSession(oldId) + const kind: SessionKind = meta.kind ?? DEFAULT_PROVIDER + const builtInMcpOverrides = sessionMcpOverrides(meta) + const builtInMcpDomains = + isAgentProviderKind(kind) + ? resolveSessionBuiltInMcpDomains({ + provider: kind, + sessionOverrides: builtInMcpOverrides, + defaultDomains: refs.defaultBuiltInMcpDomainsRef.current, + }) + : undefined + const resumeSessionId = resumableProviderSessionId(meta) + const owner = { cwd: meta.cwd, kind, providerRuntime: meta.providerRuntime } + let spawned: Awaited> try { - const kind: SessionKind = meta.kind ?? DEFAULT_PROVIDER - const builtInMcpOverrides = sessionMcpOverrides(meta) - const builtInMcpDomains = - isAgentProviderKind(kind) - ? resolveSessionBuiltInMcpDomains({ - provider: kind, - sessionOverrides: builtInMcpOverrides, - defaultDomains: refs.defaultBuiltInMcpDomainsRef.current, - }) - : undefined - const resumeSessionId = resumableProviderSessionId(meta) - const restoredMeta = withoutProvisionalProviderSession(meta) - const { sessionId: newId, userMcpServerIds } = await window.api.spawnSession({ + spawned = await window.api.spawnSession({ tldrIdentity: tldrIdentityForSession(oldId, meta), kind, providerRuntime: meta.providerRuntime, @@ -1569,53 +1668,97 @@ export function useSessionActions( builtInMcpDomains, ...(isAgentProviderKind(kind) ? { userMcpOverrides: userMcpOverridesFrom(builtInMcpOverrides) } : {}), }) - idMap.set(oldId, newId) - if (builtInMcpDomains?.includes('goal_loop')) goalLoopCapable.add(newId) - freshSessions[newId] = { - // `agentNameId` needs no line here: withoutProvisionalProviderSession - // is field-preserving, so `...restoredMeta` carries the identity from - // the pre-reload session onto its new local id. Do not add a - // `?? oldId` fallback — a workspace with no identity has no - // allocated name to lose, and minting here would put a second - // author on the one decision the reconciler owns. The unit case in - // this task's test pins the helper's field-preservation, which is - // the only thing this spread relies on. - ...restoredMeta, - tldrIdentity: tldrIdentityForSession(oldId, meta), - ...(builtInMcpDomains !== undefined ? { builtInMcpDomains, builtInMcpOverrides } : {}), - ...(userMcpServerIds !== undefined ? { userMcpServerIds } : {}), - } } catch (err) { // WHY only a curated sentence survives (#1252 review, #1324 review // C): the pane renders `processError` verbatim. Main now launders // the rejection, but the pane keeps the same rule as every create: // a missing CLI or folder names its fix, anything else is the fixed // sentence main's recovery path uses. - failedIds.set(oldId, (err instanceof Error ? curatedSpawnMessage(err.message, meta.cwd) : null) ?? SESSION_START_FAILED_MESSAGE) + // + // A failed respawn whose agent was closed meanwhile has nothing + // left to show a `failed` pane in (#1326 review A5); recreating its + // runtime would leave an invisible stale entry under a closed id. + if (liveReloadable(oldId, meta)) { + markRespawnFailed(oldId, (err instanceof Error ? curatedSpawnMessage(err.message, meta.cwd) : null) ?? SESSION_START_FAILED_MESSAGE) + } + continue + } + const newId = spawned.sessionId + // No await between this check and the commit below: nothing the user + // does can land in between. + const live = liveReloadable(oldId, meta) + if (!live) { + await killOrphan({ newId, owner }) + continue } - } - if (idMap.size === 0 && failedIds.size === 0) return + // The provider session the successor actually runs. A resumed one is + // the id it was asked to resume. A fresh start (the old row had only + // a provisional proxy-header id, which is never resumed) may report + // the new process's durable id at startup (#1326 round-2 review A1): + // dropping it skipped the history load and let an autosave persist a + // row with no resume identity. Same rule as spawn()'s own commit. + // The live row's provider fields are never used: the successor did + // not resume whatever the old process reported meanwhile. + const restoredMeta = withoutProvisionalProviderSession(meta) + const { providerSessionId: _liveProviderSessionId, providerSessionIdSource: _liveSource, ...liveRest } = live + const providerIdentity: Partial = resumeSessionId + ? { providerSessionId: restoredMeta.providerSessionId, providerSessionIdSource: restoredMeta.providerSessionIdSource } + : spawned.providerSessionId + ? { providerSessionId: spawned.providerSessionId, providerSessionIdSource: 'runtime-start' } + : {} + const fresh: SessionMeta = { + // `agentNameId` needs no line here: the live row still carries the + // identity, and `...liveRest` moves it onto the new local id. Do not + // add a `?? oldId` fallback — a workspace with no identity has no + // allocated name to lose, and minting here would put a second + // author on the one decision the reconciler owns. + ...liveRest, + ...Object.fromEntries(Object.entries(providerIdentity).filter(([, value]) => value !== undefined)), + tldrIdentity: tldrIdentityForSession(oldId, meta), + ...(builtInMcpDomains !== undefined ? { builtInMcpDomains, builtInMcpOverrides } : {}), + ...(spawned.userMcpServerIds !== undefined ? { userMcpServerIds: spawned.userMcpServerIds } : {}), + } + commitSuccessor(oldId, newId, fresh) + // Reload Agents resolves each successor's domains from CURRENT + // defaults; one that lost goal_loop keeps no loop it cannot complete + // (#1287 review A). + if (builtInMcpDomains?.includes('goal_loop')) carryGoalLoops(new Map([[oldId, newId]])) + else stopGoalLoops([oldId]) + if (hasDurableProviderSession(fresh)) { + void loadInitialHistoryForSession({ sessionId: newId, meta: fresh, refs, setRuntimes }) + } + } - setRuntimes(prev => { - const next: Record = { ...prev } - for (const oldId of idMap.keys()) delete next[oldId] + function markRespawnFailed(oldId: SessionId, processError: string): void { // A failed respawn keeps its pane, in the same `failed` state a // failed wake uses: its backend was killed above, so the pane must // say so and offer Retry (which wakes it under the same id). - for (const [oldId, processError] of failedIds) { - const current = next[oldId] ?? emptyRuntime() - next[oldId] = { - ...current, + // + // WHY it stays exactly where it was (#1239): this used to delete it, + // and the project it left empty, with no message. Toggling Dangerous + // Agents By Default therefore made any agent with a broken CLI, proxy + // or MCP start vanish from the workspace, its process already killed, + // and autosave made that permanent. + setRuntimes(prev => ({ + ...prev, + [oldId]: { + ...(prev[oldId] ?? emptyRuntime()), processStatus: 'failed', processError, recoveryFailureCode: 'start-failed', inputReady: false, inputReadinessReason: null, inputReadinessChangedAt: Date.now(), - } - } - for (const [oldId, newId] of idMap.entries()) { + }, + })) + } + + function commitSuccessor(oldId: SessionId, newId: SessionId, fresh: SessionMeta): void { + const idMap = new Map([[oldId, newId]]) + setRuntimes(prev => { + const next: Record = { ...prev } + delete next[oldId] // WHY merge instead of replacing with a fresh runtime: // // Codex resume can synchronously replay JSONL and even emit @@ -1628,61 +1771,43 @@ export function useSessionActions( // "Agent has exited". const existing = prev[newId] const restored: SessionRuntime = { ...(existing ?? emptyRuntime()) } - restored.draftInput = oldRuntimes[oldId]?.draftInput ?? existing?.draftInput ?? '' + // From the LIVE predecessor runtime (#1282), not a snapshot taken + // before the slow kill/spawn sequence: text typed, images pasted + // and a "finished, not seen" marker set meanwhile all carry over. + const predecessor = prev[oldId] + restored.draftInput = predecessor?.draftInput ?? existing?.draftInput ?? '' + if (predecessor?.draftImages?.length) restored.draftImages = predecessor.draftImages + if (predecessor?.unreadSince != null) { + restored.unreadSince = predecessor.unreadSince + restored.unreadKind = predecessor.unreadKind + } // Terminal-runtime agents included: their history reloads into - // `entries` with everyone else's below, and the pane stays on the - // raw TUI regardless (see loadInitialHistoryForSession). - Object.assign(restored, seedResumedRuntimeFields(existing, freshSessions[newId])) + // `entries` with everyone else's, and the pane stays on the raw TUI + // regardless (see loadInitialHistoryForSession). + Object.assign(restored, seedResumedRuntimeFields(existing, fresh)) next[newId] = restored - } - return next - }) - - setState(prev => { - const nextSessions = pickOwnedSessions( - prev.sessions, - collectOwnedSessionIds(prev), - ) - for (const oldId of idMap.keys()) delete nextSessions[oldId] - for (const [newId, meta] of Object.entries(freshSessions)) { - nextSessions[newId] = meta - } - - // A successor carries its predecessor's pool membership through - // `...restoredMeta` above, so it keeps its project and its place. - // - // WHY an agent whose respawn FAILED stays exactly where it was - // (#1239): this used to delete it, and the project it left empty, - // with no message. Toggling Dangerous Agents By Default therefore - // made any agent with a broken CLI, proxy or MCP start vanish from - // the workspace, its process already killed, and autosave made that - // permanent. It now keeps its id, lane and metadata, and its runtime - // (set above) shows `failed` with the spawn error and Retry. - // - // Remap lanes through the old->new idMap; failed ids have no entry, - // so their lanes keep pointing at the pane that is still there. - return { - ...prev, - // Reload-all gives every restarted agent a fresh id; remap - // relationship pointers across all sessions (children keep pointing - // at the right parent) and remap the pinned list (pins follow). - sessions: remapSessionsRelationships(nextSessions, idMap), - pinnedSessionIds: remapPinnedSessionIds(prev.pinnedSessionIds, idMap), - stage: remapTiledLanes(prev.stage, idMap), - } - }) - // Reload Agents resolves each successor's domains from CURRENT - // defaults; one that lost goal_loop keeps no loop it cannot complete - // (#1287 review A). - carryGoalLoops(new Map([...idMap].filter(([, newId]) => goalLoopCapable.has(newId)))) - stopGoalLoops([...idMap].filter(([, newId]) => !goalLoopCapable.has(newId)).map(([oldId]) => oldId)) - for (const [newId, meta] of Object.entries(freshSessions)) { - if (!hasDurableProviderSession(meta)) continue - void loadInitialHistoryForSession({ - sessionId: newId, - meta, - refs, - setRuntimes, + return next + }) + setState(prev => { + // Unowned rows are dropped up front (so an all-failed reload drops + // them too) AND here: a project removed while an earlier spawn was + // in flight leaves rows that became unowned after that first prune + // (#1326 second verification A). Both are pinned by tests. + const sessions = pickOwnedSessions(prev.sessions, collectOwnedSessionIds(prev)) + delete sessions[oldId] + // The successor carries its predecessor's LIVE pool membership (the + // `...liveRest` above), so it keeps its project and its place, even + // when a project merge moved it while the reload ran. + sessions[newId] = fresh + return { + ...prev, + // A fresh id: remap relationship pointers across all sessions + // (children keep pointing at the right parent), the pinned list + // (pins follow) and the lanes. + sessions: remapSessionsRelationships(sessions, idMap), + pinnedSessionIds: remapPinnedSessionIds(prev.pinnedSessionIds, idMap), + stage: remapTiledLanes(prev.stage, idMap), + } }) } }, @@ -1694,9 +1819,33 @@ export function useSessionActions( refs.useProxyStreamingRef, setRuntimes, setState, + killOrphan, ], ) + // WHY reloads run one at a time (#1326 review A1): the Dangerous Agents + // switch does not serialize clicks, and its handler flips the setting and + // then awaits a reload. On -> off before the first reload settles used to + // start two loops over the SAME snapshot; the on-reload committed first, + // the off-reload then found its agents gone, killed its safe successors as + // orphans, and left the dangerous ones running while Settings said off. + // Chaining makes the last click the last reload: it starts from the + // workspace the earlier one committed (refs are store-subscribed, so they + // are current the moment that commit returns) and restarts those + // successors with the final setting. A per-call default of the mode is + // read when the call is queued, which is the setting that click chose. + const reloadChainRef = useRef>(Promise.resolve()) + const reloadAgentSessions = useCallback( + (dangerousMode = refs.dangerousAgentsRef.current): Promise => { + const run = reloadChainRef.current.then(() => reloadAgentSessionsNow(dangerousMode)) + // A failed reload must not wedge every later one behind a rejection; + // the caller of THAT reload still sees its own rejection via `run`. + reloadChainRef.current = run.catch(() => undefined) + return run + }, + [refs.dangerousAgentsRef, reloadAgentSessionsNow], + ) + const softReloadAgentView = useCallback( async (sessionIdOverride?: SessionId): Promise => { const snapshot = refs.stateRef.current diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx index ecf879624..eb11badc9 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.retention.renderer.test.tsx @@ -176,4 +176,36 @@ describe('TerminalLeaf retention', () => { expect(resize).toHaveBeenCalledTimes(2) expect(resize).toHaveBeenLastCalledWith('shell-1', 120, 40) }) + + // #1281 / #1283 (terminal half): main keeps a shell's view reference across + // the shell's exit and a same-id respawn, so the leaf's unmount is what + // releases it. Without the release main forwards the shell's bytes to + // nobody until the page reloads. + it('releases its view of the shell when it unmounts', async () => { + const view = render( + {}} workspace={workspace} showStatusMode={false} />, + ) + await act(async () => { + attach.resolve('') + await attach.promise + }) + expect(window.api.detachTerminal).not.toHaveBeenCalled() + view.unmount() + expect(window.api.detachTerminal).toHaveBeenCalledExactlyOnceWith('shell-1') + }) + + it('releases an attach that lands after the leaf is gone', async () => { + const view = render( + {}} workspace={workspace} showStatusMode={false} />, + ) + // Let the mount's wake settle so the attach is in flight. + await act(async () => { await Promise.resolve() }) + view.unmount() + expect(window.api.detachTerminal).not.toHaveBeenCalled() + await act(async () => { + attach.resolve('late replay') + await attach.promise + }) + expect(window.api.detachTerminal).toHaveBeenCalledExactlyOnceWith('shell-1') + }) }) diff --git a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx index 59ed8ed13..76a11ab81 100644 --- a/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx +++ b/src/renderer/src/workspace/tile-tree/TerminalLeaf.tsx @@ -223,6 +223,11 @@ export function TerminalLeaf({ let resizeObserver: ResizeObserver | null = null let resizeFrame: number | null = null let disposed = false + // Whether main holds a view reference for this leaf (#1281 / #1283). + // Released in the cleanup below; an attach that resolves after the leaf + // is gone releases itself instead, or main would forward this shell's + // bytes to nobody until the page reloads. + let attached = false let attachedBackfillDone = false let lastCols = 0 let lastRows = 0 @@ -470,7 +475,11 @@ export function TerminalLeaf({ // The cleanup below may have disposed the term already // (transient remount, rapid tab switch). Check before // touching it. - if (disposed || termRef.current !== term) return + if (disposed || termRef.current !== term) { + void window.api.detachTerminal?.(sessionId).catch(() => undefined) + return + } + attached = true const liveTerm = term if (!liveTerm) return // Replay the buffer, then drain any live events that arrived @@ -560,6 +569,10 @@ export function TerminalLeaf({ return () => { disposed = true + // The view is gone, so main may stop forwarding to it. Main keeps the + // reference across a shell exit and a same-id respawn (#1281), so this + // is the release; a pane close, a remount or a tab switch all land here. + if (attached) void window.api.detachTerminal?.(sessionId).catch(() => undefined) offUserInput?.() if (resizeFrame !== null) cancelAnimationFrame(resizeFrame) resizeObserver?.disconnect() diff --git a/src/shared/lifecycle/events.ts b/src/shared/lifecycle/events.ts index 3cb100100..3c6cc9137 100644 --- a/src/shared/lifecycle/events.ts +++ b/src/shared/lifecycle/events.ts @@ -637,6 +637,9 @@ export const KILL_CALLERS = [ // the spawn — or a successor whose source pane disappeared mid-swap. 'replace.predecessor', 'replace.orphaned-successor', + // Reload Agents: a successor whose agent was closed or replaced while it + // was respawning (#1282). + 'reload.orphaned-successor', // Dangerous-mode toggle reloading every running agent. 'reload.agent-sessions', ] as const