From 738fe3aac9ee95c8df1c0fa20a65a1701b6a3302 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Thu, 13 Aug 2026 13:49:09 -0700 Subject: [PATCH] test(daemon): reproduce STA-4173 process-wide checkpoint tail hang Session A's stalled history write blocks an unrelated session B warm reattach because runExclusiveCheckpoint chains onto one adapter-wide promise with no deadline. --- STA-4173-REPRO.md | 86 +++++++ .../sta-4173-checkpoint-tail-repro.test.ts | 226 ++++++++++++++++++ 2 files changed, 312 insertions(+) create mode 100644 STA-4173-REPRO.md create mode 100644 src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts diff --git a/STA-4173-REPRO.md b/STA-4173-REPRO.md new file mode 100644 index 00000000000..0777af1c39d --- /dev/null +++ b/STA-4173-REPRO.md @@ -0,0 +1,86 @@ +# STA-4173 reproduction + +Reproduction only. No fix is in this branch. + +## 1. Test file + +`/Users/brennanbenson/orca/workspaces/orca/sta-4173-repro/src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts` + +## 2. Command + +```bash +npx vitest run --config config/vitest.config.ts sta-4173-checkpoint-tail-repro +``` + +## 3. Verbatim failure on current main (`ee8dd4796e`) + +``` + RUN v4.1.5 /Users/brennanbenson/orca/workspaces/orca/sta-4173-repro + + ❯ src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts (2 tests | 1 failed) 592ms + × lets a healthy session warm-reattach while another session history write is stalled 506ms + ✓ unblocks the healthy session when the process-wide checkpoint tail is skipped 85ms + +⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯ + + FAIL src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts > STA-4173 process-wide checkpoint tail > lets a healthy session warm-reattach while another session history write is stalled +AssertionError: expected 'timeout' to be 'completed' // Object.is equality + +Expected: "completed" +Received: "timeout" + + ❯ src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts:188:22 + 186| // Desired isolation: B's warm reattach must finish while A is sti… + 187| // Today runExclusiveCheckpoint tails onto one adapter-wide promis… + 188| expect(bOutcome).toBe('completed') + | ^ + 189| expect(bCheckpoint).toHaveBeenCalled() + 190| await expect(bReattach).resolves.toMatchObject({ + +⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯ + + Test Files 1 failed (1) + Tests 1 failed | 1 passed (2) + Duration 8.78s (transform 6.65s, setup 61ms, import 7.69s, tests 592ms, environment 0ms) +``` + +The suite does not hang. Session B's warm reattach is raced against 400ms and loses. + +## 4. Non-vacuous evidence + +Setup: real `DaemonServer` + `DaemonPtyAdapter` + temp `historyPath`. Two independent sessions. Only session A's `TerminalHistorySessionWriter.checkpoint` is stalled (never-resolving gate). Session B is untouched. + +Guards against the overlay `try/catch` fallback: + +- `stall.entered()` is true — A's history write was actually entered, not skipped. +- A's overlay is still `'timeout'` — the stall hung, it did not throw. A throw would settle A and unblock the tail via `previous.catch(() => {})`, which is a different (vacuous) path. +- The failing assertion is that B's `spawn({ sessionId: B })` completes while A is still stalled. Today it is `'timeout'`. +- A second test in the same file replaces only `runExclusiveCheckpoint` so it starts `operation()` immediately (no `checkpointInFlight` chain). B then reattaches in ~114ms, B's writer `checkpoint` is called, and the snapshot contains `SESSION_B_MARKER`. A remains stalled. That is not the live-snapshot catch fallback. + +Production mutation (reverted; not in this branch): + +In `src/main/daemon/daemon-pty-adapter.ts` `runExclusiveCheckpoint`, changed + +```ts +const previous = this.checkpointInFlight ?? Promise.resolve() +``` + +to + +```ts +const previous = Promise.resolve() +``` + +Then: + +```bash +npx vitest run --config config/vitest.config.ts sta-4173-checkpoint-tail-repro -t "lets a healthy session warm-reattach" +``` + +Result: **PASS** in 113ms (`1 passed | 1 skipped`). Reverting that one line restores the `'timeout'` failure. The blast radius is the adapter-wide `checkpointInFlight` tail, not session B's own history write. + +## 5. Branch + +`sta-4173-repro` + +No production code change is committed. diff --git a/src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts b/src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts new file mode 100644 index 00000000000..f1857c2ac64 --- /dev/null +++ b/src/main/daemon/sta-4173-checkpoint-tail-repro.test.ts @@ -0,0 +1,226 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { mkdtempSync, rmSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { DaemonPtyAdapter } from './daemon-pty-adapter' +import { DaemonServer } from './daemon-server' +import { getDaemonSocketPath } from './daemon-spawner' +import type { HistoryManager } from './history-manager' +import type { TerminalHistorySessionWriter } from './terminal-history-session-writer' +import type { DaemonFileLog } from './daemon-file-log' +import type { SubprocessHandle } from './session' +import type { PtySpawnResult } from '../providers/types' + +const SESSION_A = 'sta-4173-session-a' +const SESSION_B = 'sta-4173-session-b' +const SESSION_B_MARKER = 'SESSION_B_HEALTHY_MARKER' +// Why this short: a healthy local createOrAttach + overlay finishes well under this; a chained hang must lose the race. +const BOUNDED_WAIT_MS = 400 + +type AdapterCheckpointInternals = { + runExclusiveCheckpoint: ( + operation: () => Promise, + options?: { rescheduleDirty?: boolean } + ) => Promise + checkpointInFlight: Promise | null +} + +function createMockSubprocess(): SubprocessHandle & { + emitData: (data: string) => void +} { + let onData: ((data: string) => void) | undefined + let onExit: ((code: number) => void) | undefined + return { + pid: 4242, + getForegroundProcess: vi.fn(() => null), + write: vi.fn(), + resize: vi.fn(), + kill: vi.fn(() => setTimeout(() => onExit?.(0), 1)), + forceKill: vi.fn(() => onExit?.(137)), + signal: vi.fn(), + onData(callback) { + onData = callback + }, + onExit(callback) { + onExit = callback + }, + dispose: vi.fn(), + emitData(data) { + onData?.(data) + } + } +} + +function sessionWriter(manager: HistoryManager, sessionId: string): TerminalHistorySessionWriter { + const writer = ( + manager as unknown as { writers: Map } + ).writers.get(sessionId) + if (!writer) { + throw new Error(`missing history writer for ${sessionId}`) + } + return writer +} + +async function raceOutcome( + promise: Promise, + timeoutMs: number +): Promise<'completed' | 'rejected' | 'timeout'> { + let finished: 'completed' | 'rejected' | undefined + const tracked = promise.then( + () => { + finished = 'completed' + return 'completed' as const + }, + () => { + finished = 'rejected' + return 'rejected' as const + } + ) + return await Promise.race([ + tracked, + new Promise<'timeout'>((resolve) => { + setTimeout(() => resolve(finished ?? 'timeout'), timeoutMs) + }) + ]) +} + +describe('STA-4173 process-wide checkpoint tail', () => { + let dir: string + let historyDir: string + let server: DaemonServer + let adapter: DaemonPtyAdapter + let subprocessBySession: Map> + + beforeEach(async () => { + dir = mkdtempSync(join(tmpdir(), 'sta-4173-checkpoint-tail-')) + historyDir = join(dir, 'history') + subprocessBySession = new Map() + const log: DaemonFileLog = { log: () => {}, close: () => {} } + server = new DaemonServer({ + socketPath: getDaemonSocketPath(dir), + tokenPath: join(dir, 'test.token'), + log, + spawnSubprocess: (opts) => { + const subprocess = createMockSubprocess() + subprocessBySession.set(opts.sessionId, subprocess) + return subprocess + } + }) + await server.start() + adapter = new DaemonPtyAdapter({ + socketPath: getDaemonSocketPath(dir), + tokenPath: join(dir, 'test.token'), + historyPath: historyDir + }) + }) + + afterEach(async () => { + adapter?.dispose() + await server?.shutdown() + rmSync(dir, { recursive: true, force: true }) + }) + + async function spawnIndependentSessions(): Promise<{ + manager: HistoryManager + writerA: TerminalHistorySessionWriter + writerB: TerminalHistorySessionWriter + internals: AdapterCheckpointInternals + }> { + await adapter.spawn({ cols: 80, rows: 24, sessionId: SESSION_A, cwd: '/tmp' }) + await adapter.spawn({ cols: 80, rows: 24, sessionId: SESSION_B, cwd: '/tmp' }) + subprocessBySession.get(SESSION_B)?.emitData(`${SESSION_B_MARKER}\r\n`) + const manager = adapter.getHistoryManager() + if (!manager) { + throw new Error('expected HistoryManager from historyPath') + } + expect(manager.hasWriter(SESSION_A)).toBe(true) + expect(manager.hasWriter(SESSION_B)).toBe(true) + return { + manager, + writerA: sessionWriter(manager, SESSION_A), + writerB: sessionWriter(manager, SESSION_B), + internals: adapter as unknown as AdapterCheckpointInternals + } + } + + function stallSessionAWriter(writerA: TerminalHistorySessionWriter): { + entered: () => boolean + } { + let entered = false + // Intentionally never resolved: one session's history write must hang, not throw. + const gate = new Promise(() => {}) + vi.spyOn(writerA, 'checkpoint').mockImplementation(async () => { + entered = true + await gate + return { result: 'committed' } + }) + return { entered: () => entered } + } + + it('lets a healthy session warm-reattach while another session history write is stalled', async () => { + const { writerA, writerB } = await spawnIndependentSessions() + const stall = stallSessionAWriter(writerA) + const bCheckpoint = vi.spyOn(writerB, 'checkpoint') + + const aOverlay = adapter.getBufferSnapshot(SESSION_A) + await vi.waitFor(() => { + expect(stall.entered()).toBe(true) + }) + + const bReattach = adapter.spawn({ + cols: 80, + rows: 24, + sessionId: SESSION_B, + cwd: '/tmp' + }) + const bOutcome = await raceOutcome(bReattach, BOUNDED_WAIT_MS) + const aOutcome = await raceOutcome(aOverlay, 20) + + // Why: a throw from A's writer is swallowed by overlayDurableRestoreSnapshot and would + // unblock B via the catch path — that is not this hang and must not look like a pass. + expect(stall.entered()).toBe(true) + expect(aOutcome).toBe('timeout') + // Desired isolation: B's warm reattach must finish while A is still writing. + // Today runExclusiveCheckpoint tails onto one adapter-wide promise, so this fails. + expect(bOutcome).toBe('completed') + expect(bCheckpoint).toHaveBeenCalled() + await expect(bReattach).resolves.toMatchObject({ + id: SESSION_B, + isReattach: true + } satisfies Partial) + }) + + it('unblocks the healthy session when the process-wide checkpoint tail is skipped', async () => { + const { writerA, writerB, internals } = await spawnIndependentSessions() + const stall = stallSessionAWriter(writerA) + const bCheckpoint = vi.spyOn(writerB, 'checkpoint') + + const aOverlay = adapter.getBufferSnapshot(SESSION_A) + await vi.waitFor(() => { + expect(stall.entered()).toBe(true) + }) + + internals.runExclusiveCheckpoint = async (operation) => { + await operation() + } + + const bReattach = adapter.spawn({ + cols: 80, + rows: 24, + sessionId: SESSION_B, + cwd: '/tmp' + }) + const bOutcome = await raceOutcome(bReattach, BOUNDED_WAIT_MS) + const aOutcome = await raceOutcome(aOverlay, 20) + + expect(stall.entered()).toBe(true) + expect(aOutcome).toBe('timeout') + expect(bOutcome).toBe('completed') + expect(bCheckpoint).toHaveBeenCalled() + await expect(bReattach).resolves.toMatchObject({ + id: SESSION_B, + isReattach: true, + snapshot: expect.stringContaining(SESSION_B_MARKER) + }) + }) +})