Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/renderer/src/workspace/hook/ipc/useWorkspaceAdoption.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<typeof adoptWorkspace>
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
Expand Down
92 changes: 92 additions & 0 deletions src/renderer/src/workspace/workspaceShape.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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([])
})
})
65 changes: 63 additions & 2 deletions src/renderer/src/workspace/workspaceShape.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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
Expand All @@ -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<SessionId, PoolSession> = {}
const candidateIds = new Set<SessionId>([
...Object.keys(persisted.sessions ?? {}),
Expand Down
Loading