Integration batch P (9 PRs)

docs/plans/2026-09-26-undo-close-derived-identity.md

@@ -0,0 +1,10 @@
+# Undo Close keeps a derived TLDR identity (#1347)
+
+## Problem
+Undo Close respawns a closed agent with `tldrIdentity: meta.tldrIdentity`. An agent whose identity is DERIVED has no explicit field. That happens when reporting domains are on but main created the agent, or its metadata arrived after it acquired TLDR or Goal. The rule is `tldrIdentityForSession`: the identity is the session id. Undo Close passes `undefined` for such an agent, so `spawn` mints a fresh UUID: the restored agent gets a new TLDR/Goal identity, and Agent Analytics (#1342 keys by it) splits its time. Reload Agents already passes `tldrIdentityForSession(oldId, meta)`.
+
+## Decision
+The Undo Close respawn uses `tldrIdentityForSession(closedSessionId, meta)`, the same rule as reload. `respawn` now takes the closed id; every caller has it (a single pane's entry, a project member).
+
+## Test
+`undoCloseFailure.renderer.test.tsx`: a closed agent with a reporting domain and no explicit identity is respawned with its closed session id as `tldrIdentity`; an explicit identity is still passed through unchanged. Red on main.

docs/plans/2026-09-27-cli-update-could-not-start.md

@@ -0,0 +1,81 @@
+# An update that cannot start says so and stays dismissable (#1425)
+
+Size: short plan. The root cause is known and the change is bounded
+(found by #1423's review b).
+
+## Outcome
+
+When **Update Now** or an automatic update cannot start, the CLI update
+banner stops showing an undismissable "Updating…" row forever. It shows a
+dismissable failure in fixed words, with **Update now** to retry. A click
+whose IPC call rejects is also said instead of dropped.
+
+## Evidence (verified 2026-09-27, do not re-derive)
+
+- `CliUpdateOrchestrator.runUpdate` publishes `updating`, then awaits
+  `openLog(cli)`, which `mkdir`s `CLI_UPDATE_LOG_DIR` outside any catch.
+  An EACCES or ENOSPC there rejects, and the snapshot stays `updating`.
+  `describeState` marks `updating` as `undismissable`.
+- Every later await in `runUpdate` already swallows or returns a result:
+  - `appendLog` and `appendDiagnostics` catch;
+  - `readInstalledVersion` returns `{ ok }`;
+  - the command runs inside a try.
+
+  So `openLog` is the one throw that strands the state.
+- `CliUpdateBanner` fires `cliUpdatesUpdateNow` with `void` in both the
+  `notify` and the user-`deferred` rows. The IPC handler's rejection
+  (`updateOnce` throwing) is dropped, and nothing is said.
+- #1423 already gave banner actions a result contract: `onClick` may
+  resolve `false`, and the row then shows `failureText`, keyed by
+  `resultKey`.
+- `failed.logPath` is read only by the `cli-updates:open-log` handler and
+  the banner's `resultKey`.
+
+## Change
+
+- `@shared/types/cliUpdate`:
+  - `failed.logPath: string | null` (null = there is no log, because the
+    update never started);
+  - `CliUpdateFailureReason` adds `'could-not-start'`.
+- `runUpdate`: `openLog` runs in a try. On rejection it warns the cause
+  in main's log (never the renderer; q22) and publishes
+  `failed { reason: 'could-not-start', logPath: null }`, then returns
+  without running the command.
+  - Ruling: do not run the update without a log. A state dir we cannot
+    write to (ENOSPC, EACCES) makes a package install likely to fail too,
+    and a failure with no log would leave the user nothing to look at.
+    Cost if wrong: one retry after the user fixes the disk.
+- `cli-updates:open-log`: a `failed` state with `logPath === null` opens
+  nothing and answers `false`. The banner shows no View Log for it anyway.
+- Banner:
+  - `could-not-start` reads: "Couldn't start the <label> update: Agent
+    Code couldn't create its update log." The hint names disk space or
+    permissions on the Agent Code data folder. The action is **Update
+    now**, as a retry. Fixed words only.
+  - Every Update now action awaits `cliUpdatesUpdateNow`, resolves
+    `false` on rejection, and has `failureText` "Couldn't start the
+    update. Try again."
+
+## Tests (fail-first)
+
+- Orchestrator test: `mkdir` of the log dir rejects (EACCES). The snapshot
+  goes `updating` → `failed { reason: 'could-not-start', logPath: null }`,
+  the update command is never run, and nothing stays `updating`. Before
+  the fix: `updateOnce` rejects and the state stays `updating`.
+- Banner renderer test:
+  - a rejecting `cliUpdatesUpdateNow` shows the failure alert;
+  - `describeState` for `could-not-start` is dismissable, shows the fixed
+    sentence and offers Update now.
+  - Before the fix: no alert, and there is no such state.
+- `cli-updates:open-log` with a null `logPath` answers `false` without
+  calling `shell.openPath`.
+
+## Verification
+
+`npx tsc -b` and the scoped vitest runs. Boundary: the app is never
+launched, and a read-only state dir is simulated at the `fs` boundary.
+
+## Out of scope
+
+- Pruning `cli-update-logs` (nothing prunes it; unchanged).
+- The phone has no CLI update surface.

docs/plans/2026-09-27-conversation-prompts-unreadable.md

@@ -0,0 +1,82 @@
+# View Prompts says an unreadable conversation, not "no prompts" (#1306)
+
+Short plan: a bug with a known root cause (#1306, class C3, P3). Fixes #1306.
+
+## Outcome
+View Prompts on a conversation whose transcript or store is there but unreadable (damaged, or permission-broken) says "Couldn't read this conversation's prompts." instead of "no prompts". The issue asked to check every source: all five had the pattern.
+
+## Root cause (verified in source, origin/main)
+- **Pi** (`pi.ts`): `catch { return [] }` around resolution and branch reading. The package's resolution also skips a file whose header it cannot read, so an unreadable session reads as "no such session".
+- **Grok** (`grok.ts`): `catch { return [] }` around the snapshot load.
+- **Claude and Codex** read through `extractPromptsFromFile` (`promptFolder.ts`), which turned every stat and read failure into an empty answer.
+- **OpenCode** (`opencode.ts`): a store that could not be opened answered `[]`.
+- `ViewPromptsModal` showed the raw IPC message for a rejection (q22).
+
+## Design (contract)
+- **`ConversationPromptsUnreadable`** (`sources/types.ts`): a typed error with a fixed message and the cause.
+- **The rule for every source:** no file (yet) answers `[]`, because a fresh session has no transcript and View Prompts then shows the live feed. A file or store that is there but unreadable throws.
+  - `promptFolder` returns empty only for ENOENT/ENOTDIR, and rethrows anything else.
+  - Pi checks whether a file named for the session exists but cannot be opened.
+  - OpenCode throws only when `opencode.db` exists.
+- **The service's `prompts`** logs the cause in main and rethrows. Search already catches per conversation, so an unreadable one degrades to label-only.
+- **`ViewPromptsModal`** shows the fixed sentence.
+
+## Tests (real files: the recorded conversation corpus, made unreadable with chmod)
+- **`promptsUnreadable.system.test.ts`:** Claude, Codex, OpenCode (a non-database store) and Grok each throw the typed error, and a missing file is still `[]`.
+- **`pi.system.test.ts`:** the same for Pi.
+- **`service.system.test.ts`:** View Prompts rejects, and search still finds another conversation.
+- **`ViewPromptsModal.renderer.test.tsx`:** fixed words, no IPC text.
+- All red on main.
+
+## Out of scope
+- Discovery's own handling of unreadable files (it already counts them as `unreadable`).
+
+## Steering q116: finding the file is part of the rule
+- **The discovery step still mapped every failure to "not here".** That covered Claude's direct `stat` and the projects walk, Codex's rollout walk, and Pi's session listing, and it ran before the typed read handling, so a transient EACCES/EIO on a known conversation still read as "no prompts". Each now treats only ENOENT/ENOTDIR as absence and raises `ConversationPromptsUnreadable` for anything else.
+- **The Codex and Pi packages swallow their own `readdir` errors.** They answer "none", so each source probes its root directories first: Codex's `sessions`, and Pi's sessions root and per-cwd directory. The packages are unchanged.
+- **Tests (fail then recover, through `prompts()`):** an unlistable Claude project directory, then a successful read; an unlistable Codex sessions tree, then "no such thread" (a missing tree stays `[]`); an unlistable Pi session directory, then `[]`. All three are red on the previous head.
+- **History:** the branch is rebuilt so its first commit holds only this plan (worker-common line 33).
+
+## Review round 1 (a, b, c: FIX-BEFORE-MERGE), fixed fail-first
+
+- a, b (major): unknown still read as `[]` one level down.
+  - **Cause:** `existsSync` answers false for EACCES. That hit Codex's
+    known and indexed rollouts and OpenCode's database in a locked
+    directory.
+  - **Codex:** its walk skipped a locked `sessions/YYYY/MM/DD`, and an
+    index that is there but won't open, with no rollout found, gave `[]`.
+  - **Pi:** a session named for the id with a damaged header gave `[]`.
+  - **Grok:** a transcript without its `summary.json` gave `[]`.
+  - **Claude/Codex:** a transcript whose every line is garbage gave `[]`.
+  - Fixed in 0974c69b with `isPresent` and `assertTreeListable` (absent
+    vs unknown), `TranscriptUnparseable` in the prompt folder, a Pi header
+    parse check and a Grok sibling check. Six tests, all red on the
+    previous head.
+  - Also: an OpenCode query that fails after the open is typed, and a Grok
+    read racing its writer retries twice.
+- c (major, test gap): the Pi probe's "absent → `[]`" half. Pinned on a
+  fresh home with no `.pi`.
+- c (minor): ENOTDIR (a path through a plain file) is pinned as absence,
+  and the service's cause log is asserted.
+- c residuals, not changed:
+  - the prompt folder's stat catch is unreachable through the sources;
+  - the stat/read vanish race is untested;
+  - the Codex and Pi package-walk catches are unreachable (the packages
+    swallow their own errors, and the probes cover the real path);
+  - the modal showing the error above "no prompts found" is pre-existing.
+- b (minor), declined: an unrelated inaccessible Claude project aborts the
+  fallback walk. That is the stated contract (unknown is never "no
+  prompts"), and c judged it a design stance, not a defect.
+
+## Verification a (FIX-BEFORE-MERGE), fixed fail-first
+
+- **Codex:** the conversation's own rollout with an unreadable mode, in a
+  listable tree, still read as absent. The locator skips a file it cannot
+  open, and the tree check only listed directories. `assertTreeListable`
+  now also fails on a file named for this conversation.
+- **Pi:** a named file whose first row parses but is not this session's
+  header (another type, another id) still read as absent. Only a real
+  `session` header for this id can say "another cwd".
+- **Tests:** the OpenCode typed query error and the prompt folder's
+  cache-hit check are pinned.
+- Each of the four mutations is red.

docs/plans/2026-09-27-goal-loop-shutdown-dispose.md

@@ -0,0 +1,34 @@
+# Dispose the goal loop at quit (#1372)
+
+## Evidence
+- **Mechanism, recorded:** #1341 (CI run 36156560842). `GoalLoopService` persists loop state with fire-and-forget `void this.persist()`. When its owner tore down first, the late write failed: 13 `[goal-loop] persisting loop state failed: ENOENT … rename …goal-loop.json.<uuid>.tmp` warnings, 12 `rename` and 1 `open`. There the owner was a test's `afterEach`. At quit it is the process exit, and there is no log line, because the process is gone.
+- **The drain exists:** #1371 added `GoalLoopService.dispose()`, which cancels timers, detaches the session manager, and awaits in-flight persists and continuations. It was left out of the shutdown inventory on purpose, because that inventory has its own owner and test (#1371 body).
+- **The gap, read from source:** `installApplicationShutdown` (`src/main/applicationShutdown.ts`) has no goal-loop stage, and `index.ts` keeps the service in a startup local, so shutdown cannot reach it.
+- **Not recorded:** a real quit that lost loop state. No production incident exists; this closes the window the #1341 mechanism leaves open at quit.
+
+## Constraints on placement
+- **After sessions stop:** a turn boundary from a live session starts a persist (and possibly a continuation), so disposing before `killAll()` settles could lose that last write.
+- **After the built-in MCP host stops:** `goal_loop_start` / `goal_loop_complete` reach the service through MCP tools, and a call after `dispose()` would be ignored.
+- **Before quit is allowed:** its drain must settle.
+That is the support-disposal wave beside control and caffeinate.
+
+## Change
+- `ApplicationShutdownServices.disposeGoalLoop` runs in that wave.
+- `index.ts` publishes `disposeGoalLoop` once the service exists (the `disposeControlHost` pattern).
+
+## Tests (fail-first)
+- `applicationShutdown.test.ts`, with deferred inventory services:
+  - dispose is not called while sessions are stopping or while the MCP host is stopping;
+  - quit waits for its drain.
+  - It fails on the pre-change inventory, and a mutation that disposes it in the first wave, beside sessions, also fails.
+- The existing veto test iterates every inventory entry, so an editor veto disposes nothing.
+- This models the inventory, not a real quit (the app is never launched in this loop).
+
+## Overlap
+W3's #1430 work (`.worktrees/worktree-timeout-consumers`) also edits `src/main/index.ts`, in different hunks (`resolveRepoRoot` / conversation setup). Merge order is the manager's call; the two changes do not interact.
+
+## Review a finding (fixed)
+`dispose()` waited only for work the service TRACKED. The public mutators (`goal_loop_start` / `goal_loop_complete` over MCP) awaited `persist()` without tracking it, so quit could still cut off their write, and the next launch turned a completed loop into paused(interrupted). `persist()` now tracks every write, awaited or not. Fail-first: complete and startLoop parked on a gated write keep `dispose()` pending; both are red without the tracking.
+
+## Residual
+The `index.ts` wiring (`disposeGoalLoop: () => disposeGoalLoop?.()`) is not unit-tested: `index.ts` has no composition test for any inventory entry. Review a's mutation that disconnects it survives. Adding one is out of scope under the freeze.