fix(ai-workspace): a write whose status save fails is reported as done, with a warning

docs/plans/2026-09-27-ai-workspace-write-status-warning.md

@@ -0,0 +1,48 @@
+# An AI Workspace write whose status save fails is reported as done (#1285)
+
+## Problem
+`AiWorkspaceRegistry.writeFile` first replaces the user's file on disk (`atomicWriteTextFile`), then refreshes the entry status. The refresh saves registry state, and a save must first make any preservation copy it owes (#1260: rows the load could not read are copied aside before a save drops them).
+- **When the copy is blocked** (for example, a directory sits on `ai-workspaces.json.invalid-<digest>.json`), the save throws. `writeFile` then returned `{ ok: false, error: "is a directory" }` although the file was written.
+- **What that caused.** An agent or the editor told "failed" retries (converging through the version conflict) or reports a failure that did not happen.
+- **Cosmetic.** Refused saves relayed the raw errno text, with no hint that clearing the copy path unblocks them.
+
+## Evidence
+- **The sequence** was reproduced and confirmed by #1260 review B (round 2), and recorded as residual #1285 (steering q24). The owed-copy state itself comes from the real registry file: `testing/fixtures/ai-workspace/real-workspaces-2026-09-25.json`, two real workspaces from the owner's `ai-workspaces.json`. The existing #1260 tests make that state owe a copy by marking one real workspace malformed, and block the copy with a directory.
+- **The only consumer** of the write result is the renderer editor over IPC (`src/main/ipc/aiWorkspace.ts` → `AiWorkspaceEditor.tsx`). There is no MCP tool on this path.
+
+## Decisions (defaults)
+1. **A failed status refresh after a successful write does not fail the write.** It returns `ok: true` with an optional `warning` on the success branch of `AiWorkspaceWriteFileResult`, documented as "the file IS written; do not retry". It logs the error, and still emits `file-written` to every workspace holding the file.
+2. **The owed-copy invariant is unchanged.** The state file is still never saved while a copy is owed.
+3. **A refused save explains itself.** `preserveOwedCopy` rewrites a copy failure: "AI Workspace storage needs attention: N unreadable row(s) must be copied aside before saving, and the copy could not be written (<code>). <advice for that code>" (the advice names the state file's folder) Only the errno code is kept, not the raw text.
+
+## Tests (fail-first, `AiWorkspaceRegistry.test.ts`)
+The input is the real recorded workspace state. One real entry is pointed at a temp file (the recorded paths are redacted), and one real workspace is made malformed so a copy is owed. The copy path is blocked with a directory.
+- `writeFile` puts the new text on disk and returns `ok: true` with a status warning. The state file is unchanged. Red without the fix: `{ ok: false, error: 'is a directory' }`.
+- A following `create` is refused with the actionable message.
+- Mutations: dropping the warning, and misreporting the write as failed, each fail the first test.
+
+## Review a (round 1)
+- **The editor showed no warning.** It now keeps a separate `storageWarning`, set from the write result on both Save and Overwrite and cleared by the next write that has no warning. It is shown through the file list's existing alert (`error ?? storageWarning`). It cannot ride `error`, because `loadWorkspace` clears that after every save.
+- **The warning is fixed text.** `AI_WORKSPACE_STATUS_NOT_SAVED` is main's own sentence; the raw cause goes only to the log.
+- **A throwing `changed` listener no longer fails a landed write.** Each emit is guarded, so every other workspace still hears about the write.
+- **Advice matches the cause.** An occupied copy path (EISDIR/EEXIST/ENOTDIR), a missing folder (ENOENT), a permission or read-only refusal, and a full disk each get their own advice.
+- **Tests added:** the file reads back after the warning; an ordinary write carries no warning; a throwing listener still gives `ok: true`, and the listener is still called; ENOENT advice. The always-warn, unguarded-emit and wrong-advice mutations each fail.
+- **Not tested:** the editor's display of the notice is not covered at the component level (there is no AiWorkspaceEditor renderer harness). The change there is three lines, and it is stated in the body.
+
+## Review b (round 1)
+- **The notice was lost on a workspace switch, and never shown if the switch happened mid-write.** The editor held it only in React state. It is now durable in main: `get` returns a runtime-only `storageWarning` (`AI_WORKSPACE_STORAGE_BLOCKED`) while a copy is owed and its last attempt failed. The flag is set when the copy fails and cleared when it succeeds, and it is never persisted. `loadWorkspace` sets the editor's notice from it on every load, so a remount shows it again. The write-result warning still sets it immediately.
+- **Wrong advice for ENOTDIR and EROFS.** ENOTDIR now gets the generic advice, since it means a component of the folder path is a file. EROFS gets its own read-only-volume advice.
+- **Surviving mutations, now killed:**
+  - appending the raw error text to the refusal (asserted absent);
+  - stopping the fan-out after the first workspace (two-workspace test);
+  - dropping `get`'s notice.
+
+## Review c (at 589d42a9; MERGE-READY)
+Test-strength notes, not defects:
+- The emit on the warning path and the fan-out are now pinned. The two-workspace listener test landed in `d8f1eae6`.
+- Four of the six advice branches (EACCES/EPERM, EROFS, ENOSPC, generic) have no test. They are not reachable portably from a unit test without mocking the filesystem module.
+- The quoted refusal sentence in this plan and the body is corrected to the code's wording, and a test title is fixed.
+
+## Verification b
+- **The owed copy succeeded, then the state write failed.** `copyBlocked` was already false, so `get` reported nothing, and the editor's next load cleared the notice although the status was unsaved. The registry now also tracks `lastSaveFailed`, set by any failed save step and cleared by a successful save. `get` reports `AI_WORKSPACE_STORAGE_BLOCKED` while the copy is blocked, else `AI_WORKSPACE_STATUS_NOT_SAVED` while the last save failed.
+- **Test:** the reviewer's exact sequence on the real recorded state (blocked copy, unblocked, the state path turned into a directory, write, `get`, then a successful save clears it). Red on `2bb9d19c`.

src/main/aiWorkspace/AiWorkspaceRegistry.test.ts

@@ -3,7 +3,7 @@ import { tmpdir } from 'os'
 import { dirname, join } from 'path'
 import { afterEach, describe, expect, it } from 'vitest'
 
-import { AiWorkspaceRegistry } from './AiWorkspaceRegistry.js'
+import { AI_WORKSPACE_STATUS_NOT_SAVED, AI_WORKSPACE_STORAGE_BLOCKED, AiWorkspaceRegistry } from './AiWorkspaceRegistry.js'
 
 const tempRoots: string[] = []
 
@@ -287,3 +287,172 @@ describe('malformed rows in a real registry (#1246)', () => {
     expect(await registry.list()).toHaveLength(2)
   })
 })
+
+// #1285 (residual of #1260 review B, round 2): the registry owes a
+// preservation copy of rows it could not read, and that copy is blocked (a
+// directory sits on its path). The user's file write succeeds; only the status
+// refresh that follows it, which saves state and so must make the owed copy
+// first, fails. Reporting the WRITE as failed told the editor (and an agent)
+// that the edit had not landed when it had.
+//
+// The state is the REAL recorded registry (two workspaces from the owner's
+// ai-workspaces.json). Its paths are redacted, so one real entry is pointed at
+// a temp file; one real workspace is made malformed, exactly as the #1260
+// owed-copy tests do, so a copy is owed.
+describe('a write whose status refresh cannot be saved (#1285)', () => {
+  it('reports the write as done, with a warning, when only the status save fails', async () => {
+    const recorded = JSON.parse(await readFile(join(import.meta.dirname,
+      '../../../testing/fixtures/ai-workspace/real-workspaces-2026-09-25.json'), 'utf8')) as {
+      state: { workspaces: Array<Record<string, any>> }
+    }
+    const state = recorded.state
+    const root = await mkdtemp(join(tmpdir(), 'agent-code-ai-workspace-owed-'))
+    tempRoots.push(root)
+    const filePath = join(root, 'attached.txt')
+    await writeFile(filePath, 'v1')
+    state.workspaces[0]!.entries[0]!.path = filePath
+    state.workspaces[0]!.entries[0]!.projectRoot = root
+    state.workspaces[1]!.updatedAt = 1789000000
+    const statePath = join(root, 'ai-workspaces.json')
+    const source = JSON.stringify(state)
+    await writeFile(statePath, source)
+    const { createHash } = await import('node:crypto')
+    const { mkdir } = await import('fs/promises')
+    await mkdir(join(root, `ai-workspaces.json.invalid-${createHash('sha256').update(source).digest('hex').slice(0, 16)}.json`))
+
+    const registry = new AiWorkspaceRegistry(statePath)
+    const target = await realpath(filePath)
+    const result = await registry.writeFile({ path: target, text: 'v2' })
+
+    // The write landed, and the result says so, with the status failure as a
+    // warning rather than as the outcome.
+    expect(await readFile(target, 'utf8')).toBe('v2')
+    expect(result).toMatchObject({ ok: true, path: target })
+    expect((result as { warning?: string }).warning).toBe(AI_WORKSPACE_STATUS_NOT_SAVED)
+    // The file stays fully usable: it reads back through the registry.
+    expect(await registry.readFile(target)).toMatchObject({ ok: true, text: 'v2' })
+    // The stored state was not rewritten: the owed copy still blocks saves.
+    expect(await readFile(statePath, 'utf8')).toBe(source)
+    // A refused save says why and how to unblock it, not the raw errno text.
+    const refusal = await registry.create({ name: 'Blocked' }).then(() => null, (err: Error) => err.message)
+    expect(refusal).toMatch(/needs attention.*\(EISDIR\).*already occupies the copy path/s)
+    // Only the code, never the raw filesystem message.
+    expect(refusal).not.toMatch(/illegal operation|is a directory/i)
+
+    // #1416 review b: every LOAD reports the blocked storage, so an editor
+    // that remounts (a workspace switch) still shows it.
+    const workspaceId = state.workspaces[0]!.workspaceId as string
+    expect((await registry.get(workspaceId))?.storageWarning).toBe(AI_WORKSPACE_STORAGE_BLOCKED)
+    // Unblocked: the next save writes the copy and the notice clears.
+    await rm(join(root, `ai-workspaces.json.invalid-${createHash('sha256').update(source).digest('hex').slice(0, 16)}.json`), { recursive: true })
+    await registry.create({ name: 'Now' })
+    expect((await registry.get(workspaceId))?.storageWarning).toBeUndefined()
+    // It was never persisted.
+    expect(await readFile(statePath, 'utf8')).not.toContain('storageWarning')
+  })
+
+  it('keeps reporting an unsaved status when the copy succeeds but the state write fails', async () => {
+    // #1416 verification b: copy blocked, then unblocked, then the state file
+    // itself cannot be written (its path became a directory). copyBlocked is
+    // false by then, so `get` must still report the unsaved status until a
+    // state save really succeeds.
+    const recorded = JSON.parse(await readFile(join(import.meta.dirname,
+      '../../../testing/fixtures/ai-workspace/real-workspaces-2026-09-25.json'), 'utf8')) as {
+      state: { workspaces: Array<Record<string, any>> }
+    }
+    const state = recorded.state
+    const root = await mkdtemp(join(tmpdir(), 'agent-code-ai-workspace-statewrite-'))
+    tempRoots.push(root)
+    const filePath = join(root, 'attached.txt')
+    await writeFile(filePath, 'v1')
+    state.workspaces[0]!.entries[0]!.path = filePath
+    state.workspaces[0]!.entries[0]!.projectRoot = root
+    state.workspaces[1]!.updatedAt = 1789000000
+    const statePath = join(root, 'ai-workspaces.json')
+    const source = JSON.stringify(state)
+    await writeFile(statePath, source)
+    const { createHash } = await import('node:crypto')
+    const { mkdir } = await import('fs/promises')
+    const copyPath = join(root, `ai-workspaces.json.invalid-${createHash('sha256').update(source).digest('hex').slice(0, 16)}.json`)
+    await mkdir(copyPath)
+    const registry = new AiWorkspaceRegistry(statePath)
+    const workspaceId = state.workspaces[0]!.workspaceId as string
+    expect((await registry.get(workspaceId))?.storageWarning).toBe(AI_WORKSPACE_STORAGE_BLOCKED)
+
+    await rm(copyPath, { recursive: true })
+    await rm(statePath)
+    await mkdir(statePath)
+    const target = await realpath(filePath)
+    const result = await registry.writeFile({ path: target, text: 'v2' })
+    expect(result).toMatchObject({ ok: true, warning: AI_WORKSPACE_STATUS_NOT_SAVED })
+    expect(await readFile(copyPath, 'utf8')).toBe(source)
+    expect((await registry.get(workspaceId))?.storageWarning).toBe(AI_WORKSPACE_STATUS_NOT_SAVED)
+
+    // Once the state file can be written again, a save clears it.
+    await rm(statePath, { recursive: true })
+    await registry.create({ name: 'Now' })
+    expect((await registry.get(workspaceId))?.storageWarning).toBeUndefined()
+  })
+
+  it('gives no warning on an ordinary write, and a throwing listener does not fail it', async () => {
+    // #1416 review a: a throwing `changed` listener (the production one
+    // broadcasts to every window) turned a landed write into `ok: false`,
+    // and stopped the second workspace hearing about it.
+    const { registry, filePath } = await registryWithAttachedFile('v1')
+    await registry.list()
+    const heard: string[] = []
+    registry.on('changed', (event: { workspaceId: string }) => {
+      heard.push(event.workspaceId)
+      throw new Error('event delivery failed')
+    })
+    const result = await registry.writeFile({ path: filePath, text: 'v2' })
+    expect(result).toMatchObject({ ok: true })
+    expect(result).not.toHaveProperty('warning')
+    expect(await readFile(filePath, 'utf8')).toBe('v2')
+    expect(heard).toEqual(['workspace-1'])
+  })
+
+  it('tells every workspace holding the file, even when the first listener call throws', async () => {
+    const root = await mkdtemp(join(tmpdir(), 'agent-code-ai-workspace-fanout-'))
+    tempRoots.push(root)
+    const filePath = join(root, 'shared.txt')
+    await writeFile(filePath, 'v1')
+    const entry = (entryId: string) => ({
+      entryId, path: filePath, projectRoot: root, title: 'shared.txt', attachedAt: '2026-01-01T00:00:00.000Z',
+      status: { exists: true, readable: true, staleReason: null, size: 2, mtimeMs: null },
+    })
+    const workspace = (workspaceId: string) => ({
+      workspaceId, name: workspaceId, createdAt: '2026-01-01T00:00:00.000Z', updatedAt: '2026-01-01T00:00:00.000Z', entries: [entry(`${workspaceId}-e`)],
+    })
+    const statePath = join(root, 'ai-workspaces.json')
+    await writeFile(statePath, JSON.stringify({ workspaces: [workspace('ws-a'), workspace('ws-b')] }))
+    const registry = new AiWorkspaceRegistry(statePath)
+    await registry.list()
+    const heard: string[] = []
+    registry.on('changed', (event: { workspaceId: string }) => {
+      heard.push(event.workspaceId)
+      throw new Error('event delivery failed')
+    })
+    const result = await registry.writeFile({ path: await realpath(filePath), text: 'v2' })
+    expect(result).toMatchObject({ ok: true })
+    expect(heard.sort()).toEqual(['ws-a', 'ws-b'])
+  })
+
+  it('gives advice that matches the cause when the owed copy cannot be written', async () => {
+    // A missing state folder is not "something occupies the path".
+    const root = await mkdtemp(join(tmpdir(), 'agent-code-ai-workspace-enoent-'))
+    tempRoots.push(root)
+    const stateDir = join(root, 'state')
+    const { mkdir } = await import('fs/promises')
+    await mkdir(stateDir)
+    const statePath = join(stateDir, 'ai-workspaces.json')
+    const source = JSON.stringify({ workspaces: [null] })
+    await writeFile(statePath, source)
+    const { createHash } = await import('node:crypto')
+    await mkdir(join(stateDir, `ai-workspaces.json.invalid-${createHash('sha256').update(source).digest('hex').slice(0, 16)}.json`))
+    const registry = new AiWorkspaceRegistry(statePath)
+    await registry.list()
+    await rm(stateDir, { recursive: true })
+    await expect(registry.create({ name: 'Blocked' })).rejects.toThrow(/\(ENOENT\).*folder holding .* is missing/s)
+  })
+})