fix(cli-updates): an update that cannot start says so and stays dismissable

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.

src/main/ipc/cliUpdates.test.ts

@@ -59,3 +59,15 @@ it('opens nothing for a path, an unknown CLI, or a CLI whose state is not failed
   expect(await openLog('codex')).toBe(false)
   expect(shell.openPath).not.toHaveBeenCalled()
 })
+
+// #1425: an update that could not start has no log; there is nothing to open.
+it('opens nothing for a could-not-start failure, which has no log', async () => {
+  shell.openPath.mockResolvedValue('')
+  snapshot.claude = { ...failed, reason: 'could-not-start', logPath: null }
+  try {
+    expect(await openLog('claude')).toBe(false)
+    expect(shell.openPath).not.toHaveBeenCalled()
+  } finally {
+    snapshot.claude = failed
+  }
+})

src/main/ipc/cliUpdates.ts

@@ -73,7 +73,9 @@ export function registerCliUpdatesIpc(orchestrator: CliUpdateOrchestrator): void
     // the one path it wrote: the current failed state's log. Anything else
     // (an unknown CLI, a state that is not `failed`) opens nothing.
     const state = cli === 'claude' || cli === 'codex' ? orchestrator.getSnapshot()[cli] : undefined
-    if (state?.kind !== 'failed') return false
+    // #1425: a `could-not-start` failure has no log (the attempt stopped
+    // before creating one); there is nothing to open.
+    if (state?.kind !== 'failed' || state.logPath === null) return false
     // shell.openPath RESOLVES with '' on success and an error string on
     // failure. WHY the answer is returned (#1250 row 10): this used to discard
     // the string on the belief that "the OS shell surfaces its own error

src/main/setup/cliUpdateOrchestrator.couldNotStart.test.ts

@@ -0,0 +1,109 @@
+import { describe, expect, it, vi } from 'vitest'
+
+// #1425 (found by #1423's review b): runUpdate published `updating`, then
+// awaited openLog's mkdir outside any catch. A read-only or full state dir
+// rejected there and the snapshot stayed `updating` — an undismissable
+// "Updating…" row forever, and a dropped IPC rejection for Update Now.
+//
+// The fs boundary is where the failure really happens, so that is the one
+// thing replaced: mkdir of the log folder rejects the way macOS does for a
+// folder the user cannot write (EACCES). The update command runner is a spy
+// that must never be reached — an update without a log is not attempted.
+const fsState = vi.hoisted(() => ({ mkdirError: null as NodeJS.ErrnoException | null, writeError: null as NodeJS.ErrnoException | null }))
+vi.mock('fs/promises', async importOriginal => {
+  const actual = await importOriginal<typeof import('fs/promises')>()
+  return {
+    ...actual,
+    mkdir: async (...args: Parameters<typeof actual.mkdir>) => {
+      if (fsState.mkdirError) throw fsState.mkdirError
+      return actual.mkdir(...args)
+    },
+    writeFile: async (...args: Parameters<typeof actual.writeFile>) => {
+      if (fsState.writeError) throw fsState.writeError
+      return actual.writeFile(...args)
+    },
+  }
+})
+const runShellCommand = vi.hoisted(() => vi.fn(async () => ({ stdout: '', stderr: '' })))
+vi.mock('@main/setup/shell.js', () => ({ runShellCommand }))
+vi.mock('@main/setup/toolchain.js', () => ({ getToolPath: (cli: string) => `/usr/local/bin/${cli}` }))
+vi.mock('@main/setup/cliVersion.js', async importOriginal => ({
+  ...(await importOriginal<typeof import('@main/setup/cliVersion.js')>()),
+  readInstalledVersion: async () => ({ ok: true, version: '2.1.281' }),
+}))
+vi.mock('@main/setup/setupState.js', () => ({
+  loadSetupState: async () => ({ cliUpdateCache: { claude: { latestVersion: '2.1.282' }, codex: { latestVersion: '2.1.282' } } }),
+  updateCliUpdateCache: async () => undefined,
+}))
+vi.mock('@main/setup/cliInstallMethod.js', () => ({ detectCliInstallMethod: async () => 'npm' }))
+
+import { CliUpdateOrchestrator } from './cliUpdateOrchestrator.js'
+
+describe('an update that cannot start (#1425)', () => {
+  it('fails as could-not-start with no log, instead of staying Updating', async () => {
+    fsState.mkdirError = Object.assign(new Error("EACCES: permission denied, mkdir '/state/cli-update-logs'"), { code: 'EACCES' })
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    try {
+      const orchestrator = new CliUpdateOrchestrator({ list: () => [], getSessionKind: () => 'claude' } as never, { acquireUpdateLease: () => () => undefined })
+      const kinds: string[] = []
+      orchestrator.on('state', snapshot => { kinds.push(snapshot.claude.kind) })
+
+      const snapshot = await orchestrator.updateOnce('claude')
+
+      expect(snapshot.claude).toMatchObject({
+        kind: 'failed', reason: 'could-not-start', logPath: null, from: '2.1.281', wantedLatest: '2.1.282', installMethod: 'npm',
+      })
+      // It did announce the attempt, and it did not stop there.
+      expect(kinds).toEqual(['updating', 'failed'])
+      expect(runShellCommand).not.toHaveBeenCalled()
+      // The OS text stays in main's log (q22), never in the state.
+      expect(JSON.stringify(snapshot.claude)).not.toContain('EACCES')
+      expect(warn).toHaveBeenCalled()
+    } finally {
+      fsState.mkdirError = null
+      warn.mockRestore()
+    }
+  })
+})
+
+// #1447 review a: the folder was the only thing checked. A folder that already
+// exists but will not take a new file (EACCES on the folder, ENOSPC) let the
+// update run with no log, and View Log then pointed at a file that never was.
+describe('an update whose log file cannot be created (#1447 review a)', () => {
+  it('fails as could-not-start and never runs the command', async () => {
+    fsState.writeError = Object.assign(new Error("ENOSPC: no space left on device, open '/state/cli-update-logs/claude-x.log'"), { code: 'ENOSPC' })
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    runShellCommand.mockClear()
+    try {
+      const orchestrator = new CliUpdateOrchestrator({ list: () => [], getSessionKind: () => 'claude' } as never, { acquireUpdateLease: () => () => undefined })
+      const snapshot = await orchestrator.updateOnce('claude')
+      expect(snapshot.claude).toMatchObject({ kind: 'failed', reason: 'could-not-start', logPath: null })
+      expect(runShellCommand).not.toHaveBeenCalled()
+    } finally {
+      fsState.writeError = null
+      warn.mockRestore()
+    }
+  })
+})
+
+// #1447 review a: publishing a state is synchronous (emit -> the IPC broadcast
+// -> webContents.send). A listener that throws on `updating` (a window torn
+// down mid-send) aborted runUpdate before any later state, leaving the
+// snapshot `updating` for good. Publication failures must not steer the
+// update's own state machine.
+describe('a state listener that throws (#1447 review a)', () => {
+  it('does not strand updating', async () => {
+    fsState.mkdirError = Object.assign(new Error('EACCES'), { code: 'EACCES' })
+    const warn = vi.spyOn(console, 'warn').mockImplementation(() => {})
+    try {
+      const orchestrator = new CliUpdateOrchestrator({ list: () => [], getSessionKind: () => 'claude' } as never, { acquireUpdateLease: () => () => undefined })
+      orchestrator.on('state', snapshot => { if (snapshot.claude.kind === 'updating') throw new Error('Object has been destroyed') })
+      const snapshot = await orchestrator.updateOnce('claude')
+      expect(snapshot.claude.kind).toBe('failed')
+      expect(orchestrator.getSnapshot().claude.kind).not.toBe('updating')
+    } finally {
+      fsState.mkdirError = null
+      warn.mockRestore()
+    }
+  })
+})

src/main/setup/cliUpdateOrchestrator.ts

@@ -432,7 +432,32 @@ export class CliUpdateOrchestrator extends EventEmitter {
       startedAt: Date.now(),
     })
 
-    const logPath = await this.openLog(cli)
+    // #1425: `updating` is already published, and it is undismissable in the
+    // banner — so a failure here must END in another state, never leave this
+    // one standing. openLog's mkdir is the one await in this method that can
+    // throw (every later step catches or returns a result; see appendLog,
+    // appendDiagnostics, readInstalledVersion). A folder we cannot create
+    // (EACCES on the data folder, ENOSPC) means we do not run the update at
+    // all: a package install into a full or unwritable disk is likely to
+    // fail too, and a failure with no log would leave the user nothing to
+    // look at. The OS text stays here; the state carries a fixed reason.
+    let logPath: string
+    try {
+      logPath = await this.openLog(cli)
+    } catch (err) {
+      console.warn('[cli-update] could not create the update log; the update was not started:', err)
+      this.updateSnapshot(cli, {
+        kind: 'failed',
+        cli,
+        from: ctx.from,
+        wantedLatest: ctx.to,
+        installMethod: ctx.installMethod,
+        reason: 'could-not-start',
+        logPath: null,
+        finishedAt: Date.now(),
+      })
+      return
+    }
     const command = updateCommandFor(cli, ctx.installMethod)
     let commandFailed = false
     let timedOut = false
@@ -556,7 +581,19 @@ export class CliUpdateOrchestrator extends EventEmitter {
 
   private updateSnapshot(cli: CliUpdateKind, state: CliUpdateState): void {
     this.snapshot = { ...this.snapshot, [cli]: state }
-    this.emit('state', this.snapshot)
+    // WHY publication is contained (#1447 review a): emit is synchronous and
+    // its listener broadcasts to every window through webContents.send, which
+    // throws for a window torn down mid-send. That exception used to escape
+    // into runUpdate right after `updating` was published and abort it before
+    // any later state — leaving the snapshot (the source of truth, already
+    // committed above) and the other windows on an undismissable "Updating…".
+    // The state machine decides the state; a failed publication is logged and
+    // the next publication (or the renderer's snapshot fetch) catches up.
+    try {
+      this.emit('state', this.snapshot)
+    } catch (error) {
+      console.warn('[cli-update] publishing the update state failed:', error)
+    }
   }
 
   private async openLog(cli: CliUpdateKind): Promise<string> {
@@ -565,7 +602,16 @@ export class CliUpdateOrchestrator extends EventEmitter {
     // file per attempt so a re-run doesn't overwrite the last failure's
     // log while a user is trying to inspect it.
     const stamp = new Date().toISOString().replace(/[:.]/g, '-')
-    return join(CLI_UPDATE_LOG_DIR, `${cli}-${stamp}.log`)
+    const logPath = join(CLI_UPDATE_LOG_DIR, `${cli}-${stamp}.log`)
+    // WHY the file is CREATED here, not just named (#1447 review a): a folder
+    // that already exists but will not take a new file (EACCES on it, ENOSPC)
+    // passed mkdir, the update ran, every appendLog was swallowed, and View
+    // Log pointed at a file that was never written. Creating it up front is
+    // the real "can we keep a log" check; it throws into runUpdate's
+    // could-not-start path before the command runs. `wx` never truncates an
+    // earlier attempt's log that happens to share the name.
+    await writeFile(logPath, `[Agent Code] ${cli} update log, ${new Date().toISOString()}\n`, { flag: 'wx' })
+    return logPath
   }
 }
 

src/renderer/src/features/cli-updates/CliUpdateBanner.renderer.test.tsx

@@ -104,3 +104,48 @@ describe('CliUpdateBanner View Log', () => {
     expect(screen.queryByRole('alert')).toBeNull()
   })
 })
+
+// #1425: an update that could not start used to leave an undismissable
+// "Updating…" row forever (main), and a rejected Update now request was dropped
+// by `void` (renderer). The first is now its own failed state; the second is
+// said on the row that was clicked.
+describe('CliUpdateBanner: an update that could not start', () => {
+  const couldNotStart = { kind: 'failed' as const, cli: 'claude' as const, from: '2.1.281', wantedLatest: '2.1.282', installMethod: 'npm' as const, reason: 'could-not-start' as const, logPath: null, finishedAt: 5 }
+
+  it('says so in fixed words, offers a retry instead of a log, and can be dismissed', async () => {
+    const updateNow = vi.fn(async () => DEFAULT_CLI_UPDATE_SNAPSHOT)
+    Object.defineProperty(window, 'api', { configurable: true, value: { ...(window as { api?: object }).api, cliUpdatesUpdateNow: updateNow } })
+    useCliUpdateStore.setState({ snapshot: { ...DEFAULT_CLI_UPDATE_SNAPSHOT, claude: couldNotStart }, dismissed: new Set() })
+    render(<CliUpdateBanner />)
+    expect(screen.getByText("Couldn't start the Claude Code update: Agent Code couldn't create its update log. Still at 2.1.281.")).toBeInTheDocument()
+    expect(screen.queryByRole('button', { name: 'View Log' })).toBeNull()
+    await act(async () => { fireEvent.click(screen.getByRole('button', { name: 'Update now' })) })
+    expect(updateNow).toHaveBeenCalledWith('claude')
+    // A request that resolves says nothing (#1447 review c: the success half
+    // of the action was unpinned).
+    expect(screen.queryByRole('alert')).toBeNull()
+    expect(describeState('claude', couldNotStart)?.undismissable).not.toBe(true)
+    // The hint is user-visible copy too: fixed words, no OS text (q22).
+    expect(describeState('claude', couldNotStart)?.hint).toBe('Check that the disk has free space and that the Agent Code data folder is writable, then choose Update now.')
+  })
+
+  it('keys each attempt separately, so a dismissed failure does not hide the next click\'s', () => {
+    expect(dismissKey('claude', couldNotStart)).not.toBe(dismissKey('claude', { ...couldNotStart, finishedAt: 6 }))
+  })
+
+  it.each([
+    ['the offer', { kind: 'notify' as const, cli: 'claude' as const, installed: '2.1.281', latest: '2.1.282', severity: 'patch' as const, installMethod: 'npm' as const, checkedAt: 1 }, 'Update Now'],
+    ['a deferral of the user\'s click', { ...deferred('claude'), requestedByUser: true as const }, 'Update now'],
+    // #1447 review b: the could-not-start row's own retry must say a rejection too.
+    ['the could-not-start retry', { kind: 'failed' as const, cli: 'claude' as const, from: '2.1.281', wantedLatest: '2.1.282', installMethod: 'npm' as const, reason: 'could-not-start' as const, logPath: null, finishedAt: 5 }, 'Update now'],
+  ] as const)('says a rejected Update now request on %s instead of dropping it', async (_name, state, label) => {
+    const updateNow = vi.fn(async () => { throw new Error('Error invoking remote method \'cli-updates:update-now\': EACCES /state') })
+    Object.defineProperty(window, 'api', { configurable: true, value: { ...(window as { api?: object }).api, cliUpdatesUpdateNow: updateNow } })
+    useCliUpdateStore.setState({ snapshot: { ...DEFAULT_CLI_UPDATE_SNAPSHOT, claude: state }, dismissed: new Set() })
+    render(<CliUpdateBanner />)
+    await act(async () => { fireEvent.click(screen.getByRole('button', { name: label })) })
+    // Fixed words; the IPC/OS text never reaches the screen (q22).
+    expect(screen.getByRole('alert')).toHaveTextContent("Couldn't start the update. Try again.")
+    expect(screen.getByRole('alert').textContent).not.toContain('EACCES')
+  })
+})