diff --git a/src/renderer/src/workspace/hook/ipc/useWorkspaceAdoption.ts b/src/renderer/src/workspace/hook/ipc/useWorkspaceAdoption.ts index c4de66b13..721622da6 100644 --- a/src/renderer/src/workspace/hook/ipc/useWorkspaceAdoption.ts +++ b/src/renderer/src/workspace/hook/ipc/useWorkspaceAdoption.ts @@ -121,7 +121,21 @@ export function useWorkspaceAdoption( return } - const adoption = adoptWorkspace(refs.latestStateRef.current, incoming) + // WHY the migration is inside the same guard as a JSON failure (#1048 + // Codex review): adoptWorkspace migrates the incoming document, and that + // now THROWS on a corrupt project container rather than silently + // producing an empty pool. An escaping exception was only logged by the + // callers, so main — which has already transferred session routing here + // and recorded a pending bequest — never heard a refusal, and those + // sessions stayed pinned to a window that would never display them. + let adoption: ReturnType + try { + adoption = adoptWorkspace(refs.latestStateRef.current, incoming) + } catch (err) { + console.warn('[workspace] unmigratable adoption payload:', err) + await window.api.refuseWorkspaceAdoption(windowId) + return + } if (!adoption.ok) { // Refusing tells main to leave the slice on disk AND to roll back the // session routing it moved here optimistically. Staying silent would diff --git a/src/renderer/src/workspace/workspaceShape.test.ts b/src/renderer/src/workspace/workspaceShape.test.ts index f570f0de3..d4849e486 100644 --- a/src/renderer/src/workspace/workspaceShape.test.ts +++ b/src/renderer/src/workspace/workspaceShape.test.ts @@ -5,11 +5,14 @@ import type { SessionId, TabId } from '@renderer/workspace/types' import { isStageWorkspace, liveWorkspaceFromPersisted, + MalformedWorkspaceContainerError, migrateWorkspaceToStage, } from '@renderer/workspace/workspaceShape' import { resolveTabSessions } from '@renderer/workspace/queries' import { collectOwnedSessionIds } from '@renderer/workspace/sessionOwnership' import { ownerV2Workspace } from '@renderer/workspace/workspaceShape.ownerV2Fixture' +import { readFileSync } from 'node:fs' +import { resolve } from 'node:path' // The migration contract (plan 2026-09-17-unified-stage-layout.md §6, §10). // Every fixture class here is either recorded from a real workspace.json or @@ -547,3 +550,92 @@ describe('migrateWorkspaceToStage — v3 and hybrid files', () => { expect(migrated.sessions[S('a1')]?.joinedAt).toBe(0) }) }) + +// #1030 items 2 and 3, against the RECORDED live workspace +// (testing/fixtures/workspace-v2/2026-09-19-live-workspace.sanitized.json: +// one window, 3 tabs, 27 sessions) rather than an invented shape. Each case +// below is that file with exactly one field degraded, which is how both +// states arise in the wild: a hand-edited file, or a writer that emptied the +// tabs while parked rows remained. +function recordedLiveWorkspace(): PersistedWorkspace { + const file = JSON.parse(readFileSync( + resolve(__dirname, '../../../../testing/fixtures/workspace-v2/2026-09-19-live-workspace.sanitized.json'), + 'utf8', + )) as { windows: Array<{ workspace: PersistedWorkspace }> } + return file.windows[0]!.workspace +} + +describe('migrateWorkspaceToStage — a corrupt container is never an empty workspace (#1030)', () => { + it.each(['tabs', 'projects'] as const)('refuses a %s field that is present but not a list', field => { + // Migrating this to an empty pool let rehydrate mint a fresh tab and + // report `complete`, which unlocks autosave — so the next tick wrote an + // empty workspace over the real file. Throwing is what puts bootstrap in + // its locked fallback, which is what v2 did. + const degraded = { ...recordedLiveWorkspace(), [field]: null } as unknown as PersistedWorkspace + expect(() => migrateWorkspaceToStage(degraded)).toThrow(MalformedWorkspaceContainerError) + }) + + it('still migrates the recording itself, and a file that is simply empty', () => { + expect(migrateWorkspaceToStage(recordedLiveWorkspace()).projects).toHaveLength(3) + // `tabs: []` is a writer's empty workspace, not corruption. + expect(migrateWorkspaceToStage({ tabs: [], sessions: {} }).projects).toEqual([]) + }) + + it('keeps buried rows when the file has no tabs left, instead of dropping them', () => { + // v2 kept buried panes whether or not a tab survived; here every session + // needs a project, so with none left the re-parent target was '' and the + // rows — sometimes the only copy of that session's metadata — were + // dropped on upgrade. + const recorded = recordedLiveWorkspace() + const [sessionId, meta] = Object.entries(recorded.sessions)[0]! + const buriedOnly: PersistedWorkspace = { + ...recorded, + tabs: [], + sessions: {}, + // A v2 buried record carries its own copy of the meta (`sessionMeta`), + // which is exactly why dropping it can lose the only copy. + buried: [{ + id: 'buried-1', sessionId: sessionId as SessionId, sessionMeta: meta, buriedAt: 1, + sourceTabId: recorded.tabs![0]!.id, sourceTabTitle: 'gone', sourceTabIndex: 0, + } as never], + } + const migrated = migrateWorkspaceToStage(buriedOnly, () => 'recovered-project' as TabId) + expect(migrated.projects).toEqual([expect.objectContaining({ id: 'recovered-project' })]) + expect(migrated.sessions[sessionId as SessionId]).toMatchObject({ projectId: 'recovered-project' }) + }) + + it('keeps them when the buried row is ALSO still listed in sessions (#1048 review)', () => { + // The common shape, and the one the first fix missed: v2 buries a pane + // without removing its `sessions` row, so the buried record carries no + // metadata copy — and keying the recovery on that copy dropped exactly + // these rows. + const recorded = recordedLiveWorkspace() + const [sessionId, meta] = Object.entries(recorded.sessions)[0]! + const migrated = migrateWorkspaceToStage({ + ...recorded, + tabs: [], + sessions: { [sessionId]: meta }, + buried: [{ + id: 'buried-1', sessionId: sessionId as SessionId, sessionMeta: meta, buriedAt: 1, + sourceTabId: recorded.tabs![0]!.id, sourceTabTitle: 'gone', sourceTabIndex: 0, + } as never], + }, () => 'recovered-project' as TabId) + expect(migrated.sessions[sessionId as SessionId]).toMatchObject({ projectId: 'recovered-project' }) + }) + + it('mints nothing for rows that cannot be re-homed into it (#1048 re-review)', () => { + // A hybrid file: `projects: []` (so nothing is derived from the v2 tabs) + // beside those tabs and their sessions. Those memberships still name their + // old TAB ids, which are not project ids, so every one of them is dropped + // further down whatever we mint. Minting anyway left an empty, nameless + // phantom project — and, worse, a file with one project no longer looks + // empty to bootstrap, so the user lost the first-run path too. + const recorded = recordedLiveWorkspace() + const migrated = migrateWorkspaceToStage( + { ...recorded, projects: [] } as PersistedWorkspace, + () => 'phantom-project' as TabId, + ) + expect(migrated.projects).toEqual([]) + expect(Object.keys(migrated.sessions)).toEqual([]) + }) +}) diff --git a/src/renderer/src/workspace/workspaceShape.ts b/src/renderer/src/workspace/workspaceShape.ts index 2c817f205..30e09aab5 100644 --- a/src/renderer/src/workspace/workspaceShape.ts +++ b/src/renderer/src/workspace/workspaceShape.ts @@ -18,6 +18,7 @@ import { scrubGridRowMetadata, } from '@renderer/workspace/dispatch/tiledDispatchSelectors' import { normalizeGridShape } from '@renderer/workspace/dispatch/gridShape' +import { titleFromCwd } from '@renderer/workspace/layout/helpers' // --------------------------------------------------------------------------- // Read-time normalization of workspace.json into the unified shape (#992). @@ -120,7 +121,28 @@ export function defaultSeededStage(seed: SessionId | null): TiledDispatchState { * migration can never resurrect a row into a backend process (the #258 * fork-bomb shape). */ -export function migrateWorkspaceToStage(persisted: PersistedWorkspace): StageWorkspace { +export class MalformedWorkspaceContainerError extends Error {} + +export function migrateWorkspaceToStage( + persisted: PersistedWorkspace, + // Only a file with zero projects AND parked sessions to rehome needs this; + // injected so the test does not have to match a random id. + mintProjectId: () => TabId = () => crypto.randomUUID(), +): StageWorkspace { + // Rule 8 (#1030 item 3): a container that is PRESENT but not a list is + // corruption, not an empty workspace. Migrating it to an empty pool let + // rehydrate mint a fresh tab and report `complete`, which unlocks autosave — + // so the next 400 ms tick overwrote whatever the real file held. v2 threw + // here, and the throw is what put bootstrap into its locked fallback with + // the disk file untouched. `undefined` is not corruption: a v2 file has no + // `projects`, and a v3 file has no `tabs`. + for (const [field, value] of [['projects', persisted.projects], ['tabs', persisted.tabs]] as const) { + if (value !== undefined && !Array.isArray(value)) { + throw new MalformedWorkspaceContainerError( + `workspace.json has a malformed \`${field}\`; refusing to migrate it to an empty pool`, + ) + } + } // --- Rule 1. const projects: ProjectRef[] = Array.isArray(persisted.projects) ? persisted.projects @@ -136,12 +158,51 @@ export function migrateWorkspaceToStage(persisted: PersistedWorkspace): StageWor // a hand-emptied file must still migrate to something renderable. '' is // never a project id; readers treat it as "no active project". const recordedActive = persisted.activeProjectId ?? persisted.activeTabId ?? '' - const activeProjectId = projectIds.has(recordedActive) + let activeProjectId = projectIds.has(recordedActive) ? recordedActive : (projects[0]?.id ?? '') // --- Rules 2, 3, 7. const legacy = legacyMemberships(persisted) + // Rule 9 (#1030 item 2): a v2 file with zero tabs still carried its buried + // panes — burial was independent of tabs there. Here every session needs a + // project to live in, so with none left the re-parent target was '' and + // every buried row was dropped: the one place some sessions' metadata + // existed, deleted on upgrade. Mint one project to receive them instead. + // Only for files that genuinely have parked rows; an empty file stays empty. + if (projects.length === 0) { + // WHY membership rather than `restoredMeta` (#1048 Codex review): + // `restoredMeta` is set ONLY when the metadata is missing from `sessions`, + // because it exists to carry the copy a buried record holds. The common + // case is the opposite — the row is buried AND still listed in `sessions` + // — and keying the mint on `restoredMeta` dropped exactly those rows, + // which is the bug this clause exists to fix. Any parked membership at + // all is enough; the metadata is then resolved from either source, as the + // loop below already does. + // + // WHY `projectId === null` and not "any membership" (#1048 re-review): a + // minted project only ever receives rows that RE-PARENT into it, and only + // a membership whose own project is gone does that (`null` is exactly that + // state; a membership naming a live v2 tab keeps that name and is dropped + // below when the name is not a project). A hybrid file — `projects: []` + // beside stale v2 tabs — satisfied the looser test, so migration minted a + // project, then dropped every session because their tab ids still were not + // project ids. The result was an empty phantom project, which also hid the + // file from bootstrap's empty-workspace fallback: the user got a nameless + // project instead of the first-run path. + const rehomed = [...legacy.entries()].find(([sessionId, membership]) => + (membership.projectId ?? null) === null + && (membership.restoredMeta !== undefined || hasSessionMeta(persisted.sessions ?? {}, sessionId))) + if (rehomed) { + const [sessionId, membership] = rehomed + const meta = membership.restoredMeta ?? (persisted.sessions ?? {})[sessionId] + const id = mintProjectId() + const cwd = meta?.cwd + projects.push({ id, title: typeof cwd === 'string' ? titleFromCwd(cwd) : '', ...(typeof cwd === 'string' ? { cwd } : {}) }) + projectIds.add(id) + activeProjectId = id + } + } const sessions: Record = {} const candidateIds = new Set([ ...Object.keys(persisted.sessions ?? {}),