From 6eb7f58752b6b7c05d1dcdc0b34f5dbfe660694d Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 3 May 2026 16:45:03 -0400 Subject: [PATCH] test(terminal-history): regression coverage for deterministic ids + reaper Cross-restart collision prevention: - mintPtySessionId: same (worktreeId, tabId, leafId) produces same suffix; different tabIds or leafIds never collide; missing either field falls back to random (no silent "empty-input" collision). - mintPtySessionIdWithOrigin: reports deterministic / explicit / random-fallback so the ipc/pty log-level split is testable. Pane-counter advance (replayTerminalLayout): - Bumps the counter past max(persisted N). - Skips non-matching leafIds (legacy/corrupt snapshots) without throwing. - No-ops when the snapshot has no root. Orphan reaper: - Keep-set coverage: live daemon ids, verbatim ptyIds, ptyIdsByLeafId, deterministic derivation from (worktreeId, tabId, leafId), and arbitrary SSH remote ids. - mtime gate: dirs newer than REAP_MTIME_GRACE_MS never reaped. - Two-launch quarantine: reap only after >= 2 elapsed launches after first orphan sighting; keep-set re-hit resets the clock; corrupt sidecar => extra quarantine, never premature reap. - No-ops on fresh install (no history dir); ignores malformed directory names. Co-authored-by: Orca --- src/main/daemon/orphan-history-reaper.test.ts | 412 ++++++++++++++++++ src/main/daemon/pty-session-id.test.ts | 79 +++- .../layout-serialization.test.ts | 135 +++++- 3 files changed, 624 insertions(+), 2 deletions(-) create mode 100644 src/main/daemon/orphan-history-reaper.test.ts diff --git a/src/main/daemon/orphan-history-reaper.test.ts b/src/main/daemon/orphan-history-reaper.test.ts new file mode 100644 index 00000000000..c81b5000ceb --- /dev/null +++ b/src/main/daemon/orphan-history-reaper.test.ts @@ -0,0 +1,412 @@ +/* oxlint-disable max-lines -- Why: these tests cover the full reaper +decision matrix (keep set, mtime gate, two-launch quarantine, edge +cases) in one place so a regression anywhere trips a single suite. */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { + existsSync, + mkdirSync, + mkdtempSync, + readFileSync, + rmSync, + utimesSync, + writeFileSync +} from 'fs' +import { join } from 'path' +import { tmpdir } from 'os' +import { getHistorySessionDirName } from './history-paths' +import { mintPtySessionId } from './pty-session-id' +import { + REAP_MTIME_GRACE_MS, + REAP_QUARANTINE_LAUNCHES, + runOrphanHistoryReaper +} from './orphan-history-reaper' +import type { WorkspaceSessionState } from '../../shared/types' + +function freshTmp(): { sidecarDir: string; historyBasePath: string; cleanup: () => void } { + const sidecarDir = mkdtempSync(join(tmpdir(), 'orca-reaper-')) + const historyBasePath = join(sidecarDir, 'terminal-history') + mkdirSync(historyBasePath, { recursive: true }) + return { + sidecarDir, + historyBasePath, + cleanup: () => rmSync(sidecarDir, { recursive: true, force: true }) + } +} + +// Helper: create a session dir with a synthetic mtime by writing a file and +// then reaching back in time. Returns the directory path. +function makeSessionDir( + historyBasePath: string, + sessionId: string, + opts?: { ageMs?: number } +): string { + const dir = join(historyBasePath, getHistorySessionDirName(sessionId)) + mkdirSync(dir, { recursive: true }) + // Write something so the dir has distinct mtime semantics on all FSes. + writeFileSync(join(dir, 'checkpoint.json'), '{}') + if (opts?.ageMs !== undefined) { + const ts = Date.now() - opts.ageMs + utimesSync(dir, ts / 1000, ts / 1000) + utimesSync(join(dir, 'checkpoint.json'), ts / 1000, ts / 1000) + } + return dir +} + +function emptySession(overrides: Partial = {}): WorkspaceSessionState { + return { + activeRepoId: null, + activeWorktreeId: null, + activeTabId: null, + tabsByWorktree: {}, + terminalLayoutsByTabId: {}, + ...overrides + } +} + +describe('runOrphanHistoryReaper — keep set', () => { + let tmp: ReturnType + + beforeEach(() => { + tmp = freshTmp() + }) + afterEach(() => { + tmp.cleanup() + }) + + it('never reaps a dir whose sessionId appears in the live daemon set, even if old', async () => { + // Why: the daemon is the ground truth for live sessions. A running PTY + // whose workspace-session slot failed to persist must still be kept. + const sessionId = 'wt-a@@deadbeef' + makeSessionDir(tmp.historyBasePath, sessionId, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [sessionId] + }) + + expect(stats.reaped).toBe(0) + expect(stats.keptByKeepSet).toBe(1) + expect(existsSync(join(tmp.historyBasePath, getHistorySessionDirName(sessionId)))).toBe(true) + }) + + it('keeps a dir whose sessionId is only reachable via deterministic derivation', async () => { + // Why: this is the core correctness case — the ptyId field wasn't + // persisted (SIGKILL-before-persist), but the layout snapshot has a + // pane entry whose (worktreeId, tabId, leafId) still derives to the + // right id. Without deterministic keep-set derivation, this dir would + // be treated as orphan and eventually reaped. + const wt = 'wt-alpha' + const tabId = 'tab-xyz' + const leafId = 'pane:2' + const derivedId = mintPtySessionId(wt, tabId, leafId) + makeSessionDir(tmp.historyBasePath, derivedId, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const session = emptySession({ + tabsByWorktree: { + [wt]: [ + { + id: tabId, + ptyId: null, + worktreeId: wt, + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + ] + }, + terminalLayoutsByTabId: { + [tabId]: { + root: { type: 'leaf', leafId }, + activeLeafId: null, + expandedLeafId: null + } + } + }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: session, + liveDaemonSessionIds: [] + }) + + expect(stats.reaped).toBe(0) + expect(stats.keptByKeepSet).toBe(1) + }) + + it('keeps verbatim ptyIds stored in the workspace session', async () => { + // Why: legacy panes whose pre-restart ptyId persisted must still + // round-trip through the reaper's keep set even after the + // deterministic path lands — failure to keep them would surface as + // one-time scrollback loss on the upgrade boot. + const id = 'wt-legacy@@cafebabe' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const session = emptySession({ + tabsByWorktree: { + 'wt-legacy': [ + { + id: 'tab-1', + ptyId: id, + worktreeId: 'wt-legacy', + title: 'Terminal', + customTitle: null, + color: null, + sortOrder: 0, + createdAt: 0 + } + ] + } + }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: session, + liveDaemonSessionIds: [] + }) + + expect(stats.reaped).toBe(0) + }) + + it('keeps ptyIdsByLeafId entries verbatim', async () => { + const id = 'wt-alpha@@abcdef12' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const session = emptySession({ + terminalLayoutsByTabId: { + 'tab-1': { + root: { type: 'leaf', leafId: 'pane:1' }, + activeLeafId: null, + expandedLeafId: null, + ptyIdsByLeafId: { 'pane:1': id } + } + } + }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: session, + liveDaemonSessionIds: [] + }) + + expect(stats.reaped).toBe(0) + }) + + it('keeps remote SSH session ids (arbitrary format)', async () => { + // Why: remote session ids do not follow the `worktreeId@@suffix` + // pattern. The reaper must not reap them just because they miss + // every other keep-set source. + const id = 'ssh::user@host::session-xyz' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const session = emptySession({ + remoteSessionIdsByTabId: { 'tab-ssh': id } + }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: session, + liveDaemonSessionIds: [] + }) + + expect(stats.reaped).toBe(0) + }) +}) + +describe('runOrphanHistoryReaper — mtime gate', () => { + let tmp: ReturnType + beforeEach(() => { + tmp = freshTmp() + }) + afterEach(() => { + tmp.cleanup() + }) + + it('never reaps a dir whose mtime is within the grace window, even if orphan', async () => { + const id = 'wt-a@@aaaaaaaa' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS - 60_000 }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + + expect(stats.reaped).toBe(0) + expect(stats.keptByMtimeGrace).toBe(1) + }) +}) + +describe('runOrphanHistoryReaper — two-launch quarantine', () => { + let tmp: ReturnType + beforeEach(() => { + tmp = freshTmp() + }) + afterEach(() => { + tmp.cleanup() + }) + + it('requires at least REAP_QUARANTINE_LAUNCHES elapsed launches before reaping', async () => { + // Why: quarantine is measured in *elapsed* launches after the first + // sighting, not sightings. So with REAP_QUARANTINE_LAUNCHES = 2, an + // orphan first seen on launch N is reaped no earlier than launch N+2. + // That asymmetry is deliberate: a single "dropped" launch (crash after + // workspace-session write but before reaper pass) cannot eat a + // surviving dir — the user has an extra boot to recover. + expect(REAP_QUARANTINE_LAUNCHES).toBe(2) + const id = 'wt-a@@bbbbbbbb' + const dir = makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + // Launch 1: firstSeenLaunchN = 1. Quarantined. + const s1 = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(s1.reaped).toBe(0) + expect(s1.keptByQuarantine).toBe(1) + expect(existsSync(dir)).toBe(true) + + // Launch 2: launchesSinceSeen = 1. Still quarantined — the whole + // point of the two-launch bound is "one in-flight bad run cannot + // reap data the user still wants". + const s2 = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(s2.reaped).toBe(0) + expect(s2.keptByQuarantine).toBe(1) + expect(existsSync(dir)).toBe(true) + + // Launch 3: launchesSinceSeen = 2. Eligible for reap. + const s3 = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(s3.reaped).toBe(1) + expect(existsSync(dir)).toBe(false) + }) + + it('resets the quarantine clock when a previously-orphaned id reappears in the keep set', async () => { + const id = 'wt-a@@cccccccc' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + // Launch 1: orphan — records. + await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + + // Launch 2: id reappears in keep set (e.g. the workspace session + // persistence that was failing is now healthy). Must NOT reap. + const s2 = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [id] + }) + expect(s2.reaped).toBe(0) + expect(s2.keptByKeepSet).toBe(1) + + // Launch 3: id is orphaned again — the counter must have been reset, + // so this launch re-records rather than immediately reaping. + const s3 = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(s3.reaped).toBe(0) + expect(s3.keptByQuarantine).toBe(1) + }) + + it('treats a corrupt sidecar as "nothing seen yet" (extra quarantine, never premature reap)', async () => { + const id = 'wt-a@@dddddddd' + const dir = makeSessionDir(tmp.historyBasePath, id, { + ageMs: REAP_MTIME_GRACE_MS + 10_000 + }) + + // Seed a corrupt sidecar that would otherwise appear to have seen + // the id on launch 1 with counter at 10 — if we trusted it, the reaper + // would fire immediately. We must NOT. + writeFileSync(join(tmp.sidecarDir, 'reaper-seen.json'), '{not: "json') + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(stats.reaped).toBe(0) + expect(stats.keptByQuarantine).toBe(1) + expect(existsSync(dir)).toBe(true) + + // Sidecar should have been rewritten with a valid JSON shape. + const written = JSON.parse(readFileSync(join(tmp.sidecarDir, 'reaper-seen.json'), 'utf-8')) + expect(written.version).toBe(1) + expect(written.launchCounter).toBe(1) + }) +}) + +describe('runOrphanHistoryReaper — edge cases', () => { + let tmp: ReturnType + beforeEach(() => { + tmp = freshTmp() + }) + afterEach(() => { + tmp.cleanup() + }) + + it('no-ops when the history directory does not exist (fresh install)', async () => { + rmSync(tmp.historyBasePath, { recursive: true, force: true }) + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + expect(stats).toEqual({ + scanned: 0, + keptByKeepSet: 0, + keptByMtimeGrace: 0, + keptByQuarantine: 0, + reaped: 0, + errors: 0 + }) + }) + + it('ignores directory entries whose names do not round-trip through encodeURIComponent', async () => { + // A directory named literally `%` cannot be a valid minted session id + // because decodeURIComponent('%') throws. The reaper must skip it. + mkdirSync(join(tmp.historyBasePath, '%'), { recursive: true }) + // And a legitimate orphan alongside. + const id = 'wt-a@@eeeeeeee' + makeSessionDir(tmp.historyBasePath, id, { ageMs: REAP_MTIME_GRACE_MS + 10_000 }) + + const stats = await runOrphanHistoryReaper({ + historyBasePath: tmp.historyBasePath, + sidecarDir: tmp.sidecarDir, + workspaceSession: emptySession(), + liveDaemonSessionIds: [] + }) + // Malformed entry: not counted in keep/mtime/quarantine/reaped. Scanned + // increments for every readdir entry. + expect(stats.scanned).toBe(2) + expect(existsSync(join(tmp.historyBasePath, '%'))).toBe(true) + }) +}) diff --git a/src/main/daemon/pty-session-id.test.ts b/src/main/daemon/pty-session-id.test.ts index 93f7a4a84f3..9b03ea2ef73 100644 --- a/src/main/daemon/pty-session-id.test.ts +++ b/src/main/daemon/pty-session-id.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { isSafePtySessionId, mintPtySessionId } from './pty-session-id' +import { isSafePtySessionId, mintPtySessionId, mintPtySessionIdWithOrigin } from './pty-session-id' const USER_DATA = '/tmp/orca-userdata' @@ -21,6 +21,83 @@ describe('mintPtySessionId', () => { const id = mintPtySessionId('repo-123::/Users/me/work/wt-1') expect(id).toMatch(/^repo-123::\/Users\/me\/work\/wt-1@@[0-9a-f]{8}$/) }) + + it('derives a deterministic suffix from (worktreeId, tabId, leafId)', () => { + // Why: cold-restore after a SIGKILL-between-spawn-and-persist only + // works when re-minting the id for the same pane triple produces the + // same 8-char suffix — otherwise the post-restart PTY lands on a + // fresh history directory and scrollback is lost. + const a = mintPtySessionId('wt-alpha', 'tab-xyz', 'pane:3') + const b = mintPtySessionId('wt-alpha', 'tab-xyz', 'pane:3') + expect(a).toBe(b) + expect(a).toMatch(/^wt-alpha@@[0-9a-f]{8}$/) + }) + + it('does not collide across sibling panes in the same tab', () => { + // Why: two live panes in the same tab must map to distinct history + // dirs so their scrollback files do not interleave. Collision is a + // correctness bug, not a performance concern. + const p1 = mintPtySessionId('wt-alpha', 'tab-xyz', 'pane:1') + const p2 = mintPtySessionId('wt-alpha', 'tab-xyz', 'pane:2') + expect(p1).not.toBe(p2) + }) + + it('does not collide across sibling tabs in the same worktree', () => { + const a = mintPtySessionId('wt-alpha', 'tab-a', 'pane:1') + const b = mintPtySessionId('wt-alpha', 'tab-b', 'pane:1') + expect(a).not.toBe(b) + }) + + it('falls back to a random suffix when tabId is absent', () => { + // Why: the deterministic path requires BOTH tabId and leafId. A + // missing piece must degrade to the legacy random-suffix behavior — + // never silently produce a "deterministic-looking" but empty-input + // digest that would collide across every tabId-less caller. + const a = mintPtySessionId('wt-alpha', undefined, 'pane:1') + const b = mintPtySessionId('wt-alpha', undefined, 'pane:1') + expect(a).not.toBe(b) + expect(a).toMatch(/^wt-alpha@@[0-9a-f]{8}$/) + }) + + it('falls back to a random suffix when leafId is absent', () => { + const a = mintPtySessionId('wt-alpha', 'tab-xyz', undefined) + const b = mintPtySessionId('wt-alpha', 'tab-xyz', undefined) + expect(a).not.toBe(b) + }) +}) + +describe('mintPtySessionIdWithOrigin', () => { + it('reports `explicit` when the caller supplies a session id verbatim', () => { + const r = mintPtySessionIdWithOrigin({ explicitSessionId: 'sess-preexisting' }) + expect(r.sessionId).toBe('sess-preexisting') + expect(r.derivedFrom).toBe('explicit') + }) + + it('reports `deterministic` when (worktreeId, tabId, leafId) are all present', () => { + const r = mintPtySessionIdWithOrigin({ + worktreeId: 'wt-alpha', + tabId: 'tab-xyz', + leafId: 'pane:3' + }) + expect(r.derivedFrom).toBe('deterministic') + expect(r.sessionId).toMatch(/^wt-alpha@@[0-9a-f]{8}$/) + }) + + it('reports `random-fallback` when worktreeId is present but tabId or leafId is missing', () => { + // Why: this is the diagnostic signal the renderer-threading work is + // supposed to eliminate for daemon-host spawns. The caller (ipc/pty.ts) + // escalates this to a warn log so a plumbing regression surfaces + // without silently reintroducing the race. + const r = mintPtySessionIdWithOrigin({ worktreeId: 'wt-alpha', leafId: 'pane:1' }) + expect(r.derivedFrom).toBe('random-fallback') + expect(r.sessionId).toMatch(/^wt-alpha@@[0-9a-f]{8}$/) + }) + + it('reports `random-fallback` when worktreeId is absent', () => { + const r = mintPtySessionIdWithOrigin({}) + expect(r.derivedFrom).toBe('random-fallback') + expect(r.sessionId).toMatch(/^[0-9a-f-]{36}$/) + }) }) describe('isSafePtySessionId', () => { diff --git a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts index f4fe088ad73..3ace8df909e 100644 --- a/src/renderer/src/components/terminal-pane/layout-serialization.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-serialization.test.ts @@ -1,3 +1,6 @@ +/* oxlint-disable max-lines -- Why: layout-serialization concentrates the +replay/snapshot/fallback-font logic and its regressions are easiest to +catch together. */ import { describe, expect, it, beforeAll } from 'vitest' import type { TerminalPaneLayoutNode } from '../../../../shared/types' @@ -39,8 +42,10 @@ import { serializeTerminalLayout, EMPTY_LAYOUT, collectLeafIdsInOrder, - collectLeafIdsInReplayCreationOrder + collectLeafIdsInReplayCreationOrder, + replayTerminalLayout } from './layout-serialization' +import type { PaneManager } from '@/lib/pane-manager/pane-manager' // --------------------------------------------------------------------------- // Helper to create mock elements @@ -300,3 +305,131 @@ describe('collectLeafIdsInReplayCreationOrder', () => { expect(collectLeafIdsInReplayCreationOrder(layout)).toEqual(['A', 'B', 'C']) }) }) + +// --------------------------------------------------------------------------- +// replayTerminalLayout — pane-counter advance +// --------------------------------------------------------------------------- +/** + * Why: a persisted layout stores leafIds as `pane:` verbatim. After replay + * the PaneManager's monotonic counter (a per-instance `nextPaneId` that + * resets to 1 on each app start) must be advanced past max(N) so the next + * user-triggered split does not mint a pane whose `paneLeafId` collides with + * an existing persisted leaf. Collision would collapse the determinism of + * session ids derived from `(worktreeId, tabId, leafId)` — two distinct panes + * sharing one history directory. Covered by tests below. + */ + +function createMockPaneManager(): { + manager: PaneManager + createdIds: number[] + advanceCalls: number[] + // How the mock simulates the real counter. On createInitialPane / splitPane + // we hand out `nextId++`; advanceNextPaneIdTo bumps the counter forward. + nextId: () => number +} { + let counter = 1 + const created: number[] = [] + const advances: number[] = [] + + const mintPane = (): { id: number } => { + const id = counter++ + created.push(id) + return { id } + } + + const manager = { + createInitialPane: ({ focus }: { focus?: boolean } = {}) => { + void focus + return mintPane() + }, + splitPane: () => mintPane(), + advanceNextPaneIdTo: (minNextId: number) => { + advances.push(minNextId) + if (minNextId > counter) { + counter = minNextId + } + } + // Why: we cast through unknown because the mock only implements the + // narrow surface replayTerminalLayout actually calls. Keeps the test + // from having to stand up WebGL/DOM dependencies. + } as unknown as PaneManager + + return { + manager, + createdIds: created, + advanceCalls: advances, + nextId: () => counter + } +} + +describe('replayTerminalLayout — pane counter advance', () => { + it('bumps the counter past the highest persisted pane:N', () => { + // Why: persisted leafIds can encode panes with N higher than what the + // replay itself mints (e.g. the user closed pane 2 and then split + // pane 5 before shutdown — the snapshot still contains pane:5 but + // only 3 panes end up replayed). The counter must reach max(N)+1. + const { manager, advanceCalls, nextId } = createMockPaneManager() + const snapshot = { + root: { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: 'pane:7' }, + second: { type: 'leaf' as const, leafId: 'pane:3' } + }, + activeLeafId: null, + expandedLeafId: null + } + + replayTerminalLayout(manager, snapshot, false) + + // highest persisted N is 7; counter must be advanced to >= 8. + expect(advanceCalls).toEqual([8]) + expect(nextId()).toBeGreaterThanOrEqual(8) + }) + + it('does not regress the counter when the highest persisted N is small', () => { + const { manager, advanceCalls } = createMockPaneManager() + const snapshot = { + root: { type: 'leaf' as const, leafId: 'pane:1' }, + activeLeafId: null, + expandedLeafId: null + } + + replayTerminalLayout(manager, snapshot, false) + + // advanceNextPaneIdTo(2) is called but has no effect if the counter + // is already >= 2 — in the mock `createInitialPane` minted id 1 and + // left the counter at 2 so this is a no-op move, which the mock + // ignores (the real implementation's guard matches). + expect(advanceCalls).toEqual([2]) + }) + + it('skips non-matching leaf ids without throwing', () => { + // Why: a corrupted or legacy snapshot might contain leafIds that + // don't match `pane:`. These must be skipped rather than crash + // the restore — the counter fix is a best-effort safety valve. + const { manager, advanceCalls } = createMockPaneManager() + const snapshot = { + root: { + type: 'split' as const, + direction: 'vertical' as const, + first: { type: 'leaf' as const, leafId: 'legacy-shape-123' }, + second: { type: 'leaf' as const, leafId: 'pane:4' } + }, + activeLeafId: null, + expandedLeafId: null + } + + replayTerminalLayout(manager, snapshot, false) + + expect(advanceCalls).toEqual([5]) + }) + + it('advances to 1 (a no-op) when the snapshot has no persisted panes', () => { + const { manager, advanceCalls } = createMockPaneManager() + replayTerminalLayout(manager, { root: null, activeLeafId: null, expandedLeafId: null }, false) + // Snapshot has no root → replayTerminalLayout returns early, before + // the advance call path. Assert no advance was issued. + expect(advanceCalls).toEqual([]) + }) +})