Integration batch 2026-09-26 A: #1326, #1333, #1337

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

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.

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`).

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.