Sta 4062 folder note rollback (#14232)

* fix(persistence): keep folder-workspace notes across a build rollback

normalizeFolderWorkspaces rebuilds each FolderWorkspace field-by-field, so the
inline diffComments field #14112 added is dropped by any build that predates it
— and the next full-state write makes the loss durable with no user edit.

Move the on-disk home to an optional top-level PersistedState.folderWorkspaceDiffComments,
which older builds round-trip untouched through their {...defaults, ...parsed} load
spread and omit-style getDurableState(). load() hydrates it onto the records and
deletes it from Store state; buildStateToSave() is the only producer. The in-memory
FolderWorkspace shape, and therefore every IPC/RPC/renderer/mobile path, is unchanged.

Co-authored-by: Orca <help@stably.ai>

* fix(persistence): prefer inline folder notes over a stale map entry

Hydrate preferred a non-empty folderWorkspaceDiffComments entry over non-empty
inline notes. A rollback to a notes-capable #14112 build writes notes inline and
leaves the older map untouched, so re-upgrading deleted everything authored while
rolled back. Inline now wins when present; the map only fills a stripped record.

Co-authored-by: Orca <help@stably.ai>

* Extract folder workspace diff comments to dedicated module

Moves normalizeFolderWorkspaceDiffComments and
collectFolderWorkspaceDiffComments from persistence.ts to a new
folder-workspace-diff-comments.ts module for better code organization.

---------

Co-authored-by: Orca <help@stably.ai>

src/main/folder-workspace-diff-comments.ts

@@ -0,0 +1,37 @@
+import type { DiffComment, FolderWorkspace } from '../shared/types'
+
+// Why shape-only: this replaces folder-workspaces.ts's verbatim `Array.isArray(raw.diffComments)` read.
+// Filtering members would make the fix itself a new deletion path for user-authored prose.
+export function normalizeFolderWorkspaceDiffComments(
+  value: unknown
+): Record<string, DiffComment[]> | undefined {
+  if (typeof value !== 'object' || value === null || Array.isArray(value)) {
+    return undefined
+  }
+  const normalized: Record<string, DiffComment[]> = {}
+  let kept = false
+  for (const [id, comments] of Object.entries(value)) {
+    if (!Array.isArray(comments)) {
+      continue
+    }
+    normalized[id] = comments as DiffComment[]
+    kept = true
+  }
+  return kept ? normalized : undefined
+}
+
+// Why derive on every write: the map self-GCs, so delete paths need no pruning code.
+export function collectFolderWorkspaceDiffComments(
+  workspaces: readonly FolderWorkspace[] | undefined
+): Record<string, DiffComment[]> | undefined {
+  const collected: Record<string, DiffComment[]> = {}
+  let kept = false
+  for (const workspace of workspaces ?? []) {
+    const comments = workspace.diffComments
+    if (Array.isArray(comments) && comments.length > 0) {
+      collected[workspace.id] = comments
+      kept = true
+    }
+  }
+  return kept ? collected : undefined
+}

src/main/persistence.test.ts

@@ -36,6 +36,7 @@ import {
   ONBOARDING_FLOW_VERSION
 } from '../shared/constants'
 import { folderWorkspaceKey, worktreeWorkspaceKey } from '../shared/workspace-scope'
+import { folderWorkspaceToWorktree } from '../shared/folder-workspace-worktree'
 import { toRuntimeExecutionHostId, toSshExecutionHostId } from '../shared/execution-host'
 import { SshConnectionStore } from './ssh/ssh-connection-store'
 import { setSourceControlActionDefault } from '../shared/source-control-ai-actions'
@@ -5418,6 +5419,369 @@ describe('Store', () => {
     expect(session.browserPagesByWorkspace?.['browser-workspace']).toBeUndefined()
   })
 
+  // ── 8b. Folder-workspace review notes across a build rollback ──
+
+  function makeFolderNote(id: string, body: string, workspaceId = 'fw-1') {
+    return {
+      id,
+      worktreeId: folderWorkspaceKey(workspaceId),
+      filePath: 'README.md',
+      source: 'markdown' as const,
+      lineNumber: 1,
+      body,
+      createdAt: 100,
+      side: 'modified' as const
+    }
+  }
+
+  function folderWorkspaceRecord(overrides: Record<string, unknown> = {}) {
+    return {
+      id: 'fw-1',
+      projectGroupId: 'root',
+      name: 'Refund fix',
+      folderPath: '/workspace/platform',
+      linkedTask: null,
+      comment: '',
+      isArchived: false,
+      isUnread: false,
+      isPinned: false,
+      sortOrder: 10,
+      lastActivityAt: 5,
+      createdAt: 2,
+      updatedAt: 3,
+      ...overrides
+    }
+  }
+
+  function writeFolderWorkspaceProfile(options: {
+    workspaces: Record<string, unknown>[]
+    diffCommentsMap?: unknown
+    connectionId?: string | null
+    extraTopLevel?: Record<string, unknown>
+  }): void {
+    writeDataFile({
+      schemaVersion: 1,
+      repos: [],
+      worktreeMeta: {},
+      settings: {},
+      ui: {},
+      githubCache: { pr: {}, issue: {} },
+      projectGroups: [
+        {
+          id: 'root',
+          name: 'Platform',
+          parentPath: '/workspace/platform',
+          parentGroupId: null,
+          connectionId: options.connectionId ?? null,
+          createdFrom: 'folder-scan',
+          tabOrder: 0,
+          isCollapsed: false,
+          color: null,
+          createdAt: 1,
+          updatedAt: 1
+        }
+      ],
+      folderWorkspaces: options.workspaces,
+      ...('diffCommentsMap' in options
+        ? { folderWorkspaceDiffComments: options.diffCommentsMap }
+        : {}),
+      ...options.extraTopLevel
+    })
+  }
+
+  // The previous build's normalizeFolderWorkspaces has no diffComments projection (the line this
+  // checkout carries at src/shared/folder-workspaces.ts:107 does not exist on v1.4.179–v1.4.181),
+  // and its full-state write re-serializes everything else verbatim (the `{ ...defaults, ...parsed }`
+  // load spread and the omit-style getDurableState()).
+  function simulatePreviousBuildLoadAndFlush(onDisk: PersistedState): PersistedState {
+    return {
+      ...onDisk,
+      folderWorkspaces: onDisk.folderWorkspaces.map(
+        ({ diffComments: _droppedByOldNormalizer, ...rest }) => rest
+      )
+    }
+  }
+
+  it('keeps folder-workspace review notes across a previous-build rollback and re-upgrade', async () => {
+    const store = await createStore()
+    const group = store.createProjectGroup({
+      name: 'Platform',
+      parentPath: '/workspace/platform',
+      createdFrom: 'folder-scan'
+    })
+    const workspace = store.createFolderWorkspace({
+      projectGroupId: group.id,
+      name: 'Refund fix'
+    })
+    const note = makeFolderNote('note-1', 'Review this paragraph', workspace.id)
+    store.updateFolderWorkspace(workspace.id, { diffComments: [note] })
+    store.flush()
+
+    const persisted = readDataFile() as PersistedState
+    expect(persisted.folderWorkspaceDiffComments?.[workspace.id]).toEqual([note])
+    expect(persisted.folderWorkspaces[0]).not.toHaveProperty('diffComments')
+    // The strip lives at the serialization boundary only; live records keep their notes.
+    expect(store.getFolderWorkspace(workspace.id)?.diffComments).toEqual([note])
+    expect(folderWorkspaceToWorktree(store.getFolderWorkspace(workspace.id)!).diffComments).toEqual(
+      [note]
+    )
+
+    writeDataFile(simulatePreviousBuildLoadAndFlush(persisted))
+
+    const restored = await createStore()
+    expect(restored.getFolderWorkspace(workspace.id)?.diffComments).toEqual([note])
+    restored.flush()
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments?.[workspace.id]).toEqual([
+      note
+    ])
+  })
+
+  it('survives repeated rollback / re-upgrade hops', async () => {
+    const store = await createStore()
+    const group = store.createProjectGroup({
+      name: 'Platform',
+      parentPath: '/workspace/platform',
+      createdFrom: 'folder-scan'
+    })
+    const workspace = store.createFolderWorkspace({ projectGroupId: group.id, name: 'Refund fix' })
+    const note = makeFolderNote('note-1', 'Review this paragraph', workspace.id)
+    store.updateFolderWorkspace(workspace.id, { diffComments: [note] })
+    store.flush()
+
+    for (let hop = 0; hop < 2; hop++) {
+      writeDataFile(simulatePreviousBuildLoadAndFlush(readDataFile() as PersistedState))
+      const reupgraded = await createStore()
+      expect(reupgraded.getFolderWorkspace(workspace.id)?.diffComments).toEqual([note])
+      reupgraded.flush()
+    }
+  })
+
+  it('keeps notes and remote provenance for an SSH folder workspace across a rollback', async () => {
+    const store = await createStore()
+    const group = store.createProjectGroup({
+      name: 'Platform',
+      parentPath: '/workspace/platform',
+      connectionId: 'ssh-1',
+      createdFrom: 'folder-scan'
+    })
+    const workspace = store.createFolderWorkspace({ projectGroupId: group.id, name: 'Remote fix' })
+    expect(workspace.connectionId).toBe('ssh-1')
+    const note = makeFolderNote('note-remote', 'Remote review', workspace.id)
+    store.updateFolderWorkspace(workspace.id, { diffComments: [note] })
+    store.flush()
+
+    writeDataFile(simulatePreviousBuildLoadAndFlush(readDataFile() as PersistedState))
+
+    const restored = await createStore()
+    expect(restored.getFolderWorkspace(workspace.id)?.diffComments).toEqual([note])
+    expect(restored.getFolderWorkspace(workspace.id)?.connectionId).toBe('ssh-1')
+  })
+
+  it('round-trips unknown top-level state keys through load and flush', async () => {
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord({ unknownNestedField: 'from-a-newer-build' })],
+      extraTopLevel: { unknownTopLevelKey: { kept: 'verbatim' } }
+    })
+
+    const store = await createStore()
+    store.updateFolderWorkspace('fw-1', { comment: 'touch' })
+    store.flush()
+
+    const persisted = readDataFile() as PersistedState & { unknownTopLevelKey?: unknown }
+    expect(
+      persisted.unknownTopLevelKey,
+      'Unknown top-level keys must survive load + flush. Note preservation relies on this: converting the ' +
+        '`{ ...defaults, ...parsed }` load spread or the omit-style getDurableState() into an ' +
+        'allowlist re-opens folder-workspace note loss for the then-previous build.'
+    ).toEqual({ kept: 'verbatim' })
+    expect(
+      persisted.folderWorkspaces[0],
+      'Documents existing behavior, not a guarantee: normalizeFolderWorkspaces rebuilds each ' +
+        'record field-by-field, so nested unknown fields are dropped. That is why notes moved ' +
+        'to a top-level key.'
+    ).not.toHaveProperty('unknownNestedField')
+  })
+
+  it('migrates legacy inline folder-workspace notes into the top-level map', async () => {
+    const note = makeFolderNote('note-1', 'Legacy inline note')
+    writeFolderWorkspaceProfile({ workspaces: [folderWorkspaceRecord({ diffComments: [note] })] })
+
+    const store = await createStore()
+    expect(store.getFolderWorkspace('fw-1')?.diffComments).toEqual([note])
+    store.flush()
+
+    const persisted = readDataFile() as PersistedState
+    expect(persisted.folderWorkspaceDiffComments?.['fw-1']).toEqual([note])
+    expect(persisted.folderWorkspaces[0]).not.toHaveProperty('diffComments')
+  })
+
+  it('keeps notes across a rollback when the session never edits anything', async () => {
+    const note = makeFolderNote('note-1', 'Legacy inline note')
+    writeFolderWorkspaceProfile({ workspaces: [folderWorkspaceRecord({ diffComments: [note] })] })
+
+    // Launch and quit on the fixed build, with no mutation: loadNeedsSave must make the
+    // relocation durable on its own. This is the reported P0 sequence.
+    const upgraded = await createStore()
+    upgraded.flush()
+
+    writeDataFile(simulatePreviousBuildLoadAndFlush(readDataFile() as PersistedState))
+
+    const restored = await createStore()
+    expect(restored.getFolderWorkspace('fw-1')?.diffComments).toEqual([note])
+  })
+
+  it('keeps notes authored inline on a rolled-back #14112 build over a staler map entry', async () => {
+    const mapped = makeFolderNote('note-mapped', 'Note from the fixed build')
+    const authored = makeFolderNote('note-inline', 'Note authored while rolled back')
+
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord()],
+      diffCommentsMap: { 'fw-1': [mapped] }
+    })
+    // #14112 persists notes inline and never learned about the top-level map, so a note authored
+    // there lands inline while the untouched map entry goes stale. Inline is the newer write.
+    const rolledBack = readDataFile() as PersistedState
+    writeDataFile({
+      ...rolledBack,
+      folderWorkspaces: rolledBack.folderWorkspaces.map((workspace) => ({
+        ...workspace,
+        diffComments: [authored]
+      }))
+    })
+
+    const store = await createStore()
+    expect(store.getFolderWorkspace('fw-1')?.diffComments).toEqual([authored])
+    store.flush()
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments).toEqual({
+      'fw-1': [authored]
+    })
+  })
+
+  it('never lets an empty or unrelated map entry delete inline notes', async () => {
+    const inline = makeFolderNote('note-inline', 'Inline note')
+    const mapped = makeFolderNote('note-mapped', 'Mapped note')
+
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord({ diffComments: [inline] })],
+      diffCommentsMap: { 'fw-1': [] }
+    })
+    expect((await createStore()).getFolderWorkspace('fw-1')?.diffComments).toEqual([inline])
+
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord({ diffComments: [inline] })],
+      diffCommentsMap: { other: [mapped] }
+    })
+    const store = await createStore()
+    expect(store.getFolderWorkspace('fw-1')?.diffComments).toEqual([inline])
+    store.flush()
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments).toEqual({
+      'fw-1': [inline]
+    })
+  })
+
+  it('drops orphaned note entries and prunes them when workspaces are deleted', async () => {
+    const note = makeFolderNote('note-1', 'Kept note')
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord()],
+      diffCommentsMap: { 'fw-1': [note], ghost: [makeFolderNote('note-ghost', 'Orphan', 'ghost')] }
+    })
+
+    const store = await createStore()
+    store.flush()
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments).toEqual({
+      'fw-1': [note]
+    })
+
+    const reloaded = await createStore()
+    reloaded.flush()
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments).not.toHaveProperty(
+      'ghost'
+    )
+
+    // Delete paths carry no pruning code: the map is derived from live workspaces on every write.
+    expect(reloaded.removeFolderWorkspace('fw-1')).toBe(true)
+    reloaded.flush()
+    expect(readDataFile() as PersistedState).not.toHaveProperty('folderWorkspaceDiffComments')
+
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord()],
+      diffCommentsMap: { 'fw-1': [note] }
+    })
+    const groupDelete = await createStore()
+    expect(groupDelete.deleteProjectGroup('root')).toBe(true)
+    groupDelete.flush()
+    expect(readDataFile() as PersistedState).not.toHaveProperty('folderWorkspaceDiffComments')
+  })
+
+  it('writes no folderWorkspaceDiffComments key for note-free profiles', async () => {
+    const store = await createStore()
+    const group = store.createProjectGroup({
+      name: 'Platform',
+      parentPath: '/workspace/platform',
+      createdFrom: 'folder-scan'
+    })
+    const workspace = store.createFolderWorkspace({ projectGroupId: group.id, name: 'No notes' })
+    store.flush()
+    expect('folderWorkspaceDiffComments' in (readDataFile() as object)).toBe(false)
+
+    store.updateFolderWorkspace(workspace.id, { diffComments: [] })
+    store.flush()
+    expect('folderWorkspaceDiffComments' in (readDataFile() as object)).toBe(false)
+  })
+
+  it.each([
+    ['null root', null, undefined],
+    ['string root', 'oops', undefined],
+    ['array root', [], undefined],
+    ['non-array entry value', { 'fw-1': 'oops' }, undefined],
+    ['array of non-DiffComment members', { 'fw-1': [7] }, [7]]
+  ])(
+    'tolerates a corrupt folderWorkspaceDiffComments map (%s)',
+    async (_label, diffCommentsMap, expected) => {
+      writeFolderWorkspaceProfile({ workspaces: [folderWorkspaceRecord()], diffCommentsMap })
+
+      const store = await createStore()
+
+      expect(store.getFolderWorkspace('fw-1')?.diffComments).toEqual(expected)
+    }
+  )
+
+  it('passes note members through verbatim on load and re-write', async () => {
+    // Shape-only guard: the moment member filtering is added here, the fix itself becomes a new
+    // deletion path for user-authored prose.
+    const noteWithExtras = { ...makeFolderNote('note-1', 'Body'), unknownNoteField: 'kept' }
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord(), folderWorkspaceRecord({ id: 'fw-2', name: 'Second' })],
+      diffCommentsMap: { 'fw-1': [7], 'fw-2': [noteWithExtras] }
+    })
+
+    const store = await createStore()
+    store.flush()
+
+    const persisted = readDataFile() as PersistedState
+    expect(persisted.folderWorkspaceDiffComments).toEqual({
+      'fw-1': [7],
+      'fw-2': [noteWithExtras]
+    })
+  })
+
+  it('derives the written note map from live workspaces, never a stale loaded copy', async () => {
+    const loadedNote = makeFolderNote('note-loaded', 'Loaded note')
+    const editedNote = makeFolderNote('note-edited', 'Edited note')
+    writeFolderWorkspaceProfile({
+      workspaces: [folderWorkspaceRecord()],
+      diffCommentsMap: { 'fw-1': [loadedNote] }
+    })
+
+    const store = await createStore()
+    store.updateFolderWorkspace('fw-1', { diffComments: [editedNote] })
+    store.flush()
+
+    expect((readDataFile() as PersistedState).folderWorkspaceDiffComments).toEqual({
+      'fw-1': [editedNote]
+    })
+  })
+
   // ── 9. Settings: get/update ────────────────────────────────────────
 
   it('updateSettings merges partial updates', async () => {

src/main/persistence.ts

@@ -256,6 +256,10 @@ import { normalizeUiLanguage } from '../shared/ui-language'
 import { normalizeBrowserPageZoomLevel } from '../shared/browser-page-zoom'
 import { persistedUIValuesEqual } from '../shared/persisted-ui-equality'
 import { ActiveViewPreference } from './active-view-preference'
+import {
+  collectFolderWorkspaceDiffComments,
+  normalizeFolderWorkspaceDiffComments
+} from './folder-workspace-diff-comments'
 import {
   normalizeFolderWorkspaceName,
   normalizeFolderWorkspaces
@@ -2858,6 +2862,7 @@ export class Store {
     // profile avoids serializing the multi-MB recovery store on navigation.
     this.activeViewPreference = new ActiveViewPreference(this.dataFile, this.state.ui?.activeView)
     const adaptedProjectGroups = this.adaptFlatFolderScanProjectGroups()
+    this.hydrateFolderWorkspaceDiffComments()
     for (const entry of normalized.migrationUnsupportedEntries) {
       setMigrationUnsupportedPty(entry)
     }
@@ -2878,6 +2883,33 @@ export class Store {
     }
   }
 
+  // Why: notes live top-level on disk so an older build's field-by-field
+  // normalizeFolderWorkspaces can't drop them; re-attach them to the in-memory records here.
+  private hydrateFolderWorkspaceDiffComments(): void {
+    const stored = this.state.folderWorkspaceDiffComments
+    let relocatedInline = false
+    for (const workspace of this.state.folderWorkspaces ?? []) {
+      if (Array.isArray(workspace.diffComments) && workspace.diffComments.length > 0) {
+        // Inline wins: an intervening rollback to a #14112 build writes notes inline and leaves the
+        // older map untouched, so inline is the last notes-aware write. Also makes the relocation
+        // durable even if the user never edits anything this session.
+        relocatedInline = true
+        continue
+      }
+      const comments = stored?.[workspace.id]
+      // Not `??`: a degenerate `{ id: [] }` entry must not delete an intact inline value.
+      if (Array.isArray(comments) && comments.length > 0) {
+        workspace.diffComments = comments
+      }
+    }
+    if (relocatedInline) {
+      this.loadNeedsSave = true
+    }
+    // Write-only projection: buildStateToSave() is the only producer, so leaving the loaded map in
+    // state would make it a stale second source of truth that getDurableState() spreads back out.
+    delete this.state.folderWorkspaceDiffComments
+  }
+
   private adaptFlatFolderScanProjectGroups(): boolean {
     // Why: older folder imports kept a real parent path but flat repos; upgrade that shape into v1 sparse folder scopes.
     const groups = this.state.projectGroups ?? []
@@ -3427,6 +3459,9 @@ export class Store {
             parsed.folderWorkspaces,
             normalizedProjectGroups
           ),
+          folderWorkspaceDiffComments: normalizeFolderWorkspaceDiffComments(
+            parsed.folderWorkspaceDiffComments
+          ),
           worktreeLineageById: parsed.worktreeLineageById ?? {},
           mobileClientTabSelectionsByDeviceId: normalizePersistedMobileClientTabSelections(
             parsed.mobileClientTabSelectionsByDeviceId
@@ -4021,6 +4056,13 @@ export class Store {
     // Why: clone before encrypting secrets so in-memory this.state stays plaintext.
     const stateToSave = {
       ...this.getDurableState(),
+      // Why both keys unconditionally: the explicit keys always win over the spread, and
+      // JSON.stringify drops the `undefined` value so a note-free profile gains no key on disk.
+      // The strip builds a new array here only; this.state records keep their notes in memory.
+      folderWorkspaces: (this.state.folderWorkspaces ?? []).map(
+        ({ diffComments: _relocated, ...rest }) => rest
+      ),
+      folderWorkspaceDiffComments: collectFolderWorkspaceDiffComments(this.state.folderWorkspaces),
       sshPtyConsumerRecoveries: (this.state.sshPtyConsumerRecoveries ?? []).map((record) => ({
         ...record,
         ownerLease: encryptToSentinel(

src/shared/folder-workspaces.ts

@@ -104,6 +104,8 @@ export function normalizeFolderWorkspaces(
         typeof raw.createdAt === 'number' && Number.isFinite(raw.createdAt) ? raw.createdAt : now,
       updatedAt:
         typeof raw.updatedAt === 'number' && Number.isFinite(raw.updatedAt) ? raw.updatedAt : now,
+      // Legacy read: unreleased #14112 builds wrote notes inline. Canonical home is
+      // PersistedState.folderWorkspaceDiffComments.
       ...(Array.isArray(raw.diffComments) ? { diffComments: raw.diffComments } : {})
     })
   }

src/shared/types.ts

@@ -3694,6 +3694,14 @@ export type PersistedState = {
   projectHostSetups: ProjectHostSetup[]
   projectGroups: ProjectGroup[]
   folderWorkspaces: FolderWorkspace[]
+  /** Folder-workspace review notes, keyed by FolderWorkspace.id. Top-level, NOT nested in
+   *  folderWorkspaces[]: normalizeFolderWorkspaces rebuilds each record field-by-field, so an
+   *  older build drops nested fields, while unknown top-level keys round-trip untouched.
+   *
+   *  WRITE-ONLY PROJECTION. FolderWorkspace.diffComments is the single in-memory home; load()
+   *  hydrates from this key and then deletes it from Store state, and buildStateToSave() is the
+   *  only producer of it. Never read Store.state.folderWorkspaceDiffComments outside load(). */
+  folderWorkspaceDiffComments?: Record<string, DiffComment[]>
   /** Sparse-checkout presets keyed by repoId. */
   sparsePresetsByRepo: Record<string, SparsePreset[]>
   /** Per paired device last tab selection by worktree; keeps mobile navigation across host restarts. */