diff --git a/src/studio/auto-launch.ts b/src/studio/auto-launch.ts index 1755db08..5b4f9875 100644 --- a/src/studio/auto-launch.ts +++ b/src/studio/auto-launch.ts @@ -117,11 +117,14 @@ export interface AutoLaunchDeps { /** * Start the substrate. Injectable so tests never spawn a real process. * - * Returning `false` means DECLINED — nothing was started, so there is nothing to wait for. Any - * other return (including `void`) means a start was attempted and the handle poll is worth - * running. A {@link LaunchOutcome} says both, and additionally lets the poll give up early on a - * start that has since died. See {@link defaultLaunch} for why a launcher that answered - * `launchable` can still decline. + * Returning `false` means DECLINED — nothing was started, so there is nothing to wait for. + * `true` or `void` means a start was attempted and the handle poll is worth running. A {@link + * LaunchOutcome} says both, and additionally lets the poll give up early on a start that has + * since died. See {@link defaultLaunch} for why a launcher that answered `launchable` can still + * decline. + * + * NOTHING ELSE IS AN ANSWER. `null`, `0`, `''` and the rest are rejected rather than guessed at; + * see {@link normalizeLaunch} for why a near-miss decline must not read as a start. */ launch?: () => boolean | void | LaunchOutcome; /** True when the substrate can actually be started on this machine. Injectable. */ @@ -303,7 +306,8 @@ const NEVER_FAILED = (): boolean => false; * reporting". Only a launcher that actually spawns something can say more, and only `defaultLaunch` * does. * - * ⚠ IT REJECTS ANY OTHER OBJECT, LOUDLY, and a thenable by name. Reading `.started` off whatever + * ⚠ IT REJECTS EVERY OTHER SHAPE, LOUDLY — any other object, a thenable by name, and any primitive + * that is not `false`/`true`/`void`. Reading `.started` off whatever * arrives used to make an async launcher the worst possible answer: a Promise's `.started` is * `undefined`, so the launch was reported as a DECLINE on the same tick its body started running — * the poll never ran, the handle was never read, and the caller was told nothing had been started @@ -315,6 +319,20 @@ const NEVER_FAILED = (): boolean => false; */ export function normalizeLaunch(result: boolean | void | LaunchOutcome): LaunchOutcome { if (result === false) return { started: false, failed: NEVER_FAILED }; + // THE LEGAL PRIMITIVES, ENUMERATED — this used to be a trailing "anything else means started", + // which is the same plausible lie the paragraph above refuses to tell, just told about a value + // rather than an object. `null`, `0`, `''` and `NaN` are all ONE TOKEN from a correct decline — + // `() => null` is what a launcher written against `readSubstrateRecord`'s own return shape + // produces — and each of them bought the full 30 s handle poll for a process nobody started, + // followed by a 60 s negative memo locking the next caller out. `1` and `'yes'` are the identical + // guess about a truthy value, so the guard is an allowlist rather than a falsy check. + // + // It THROWS rather than declining quietly, for the same reason every other unreadable shape here + // does: one rule at the seam, not a second dialect for primitives. It is not a behavioural gamble + // either — the throw is synchronous, so `ensureStudioRunning`'s launcher try/catch turns it into + // exactly the clean zero-tick, un-memoized decline the alternative would have produced, plus a + // logged reason. DECISIONS-AUTO 2026-08-29 carries the reversal condition. + if (result === true || result === undefined) return { started: true, failed: NEVER_FAILED }; if (result && typeof result === 'object') { if (typeof (result as { then?: unknown }).then === 'function') { throw new TypeError( @@ -342,7 +360,10 @@ export function normalizeLaunch(result: boolean | void | LaunchOutcome): LaunchO } return { started: result.started, failed: result.failed ?? NEVER_FAILED }; } - return { started: true, failed: NEVER_FAILED }; + throw new TypeError( + `studio auto-launch received a ${result === null ? 'null' : typeof result} from deps.launch, which is ` + + 'not a launcher answer — only `false` (declined), `true`/`void` (started) and a LaunchOutcome say anything' + ); } /** @@ -381,7 +402,20 @@ let noHandleUntil = 0; */ export async function ensureStudioRunning(deps: AutoLaunchDeps = {}): Promise { const read = deps.readHandleFn ?? readHandle; - const existing = read(deps.dataDir); + // GUARDED, because this is the first thing the function does and it sat outside every try in it: + // a reader that throws rejected straight out of the entry point, past the "never throws" promise + // in the docstring above and into `studioBridgeFetch`, which awaits with no catch. + // + // A read that cannot answer is NOT evidence the substrate is absent, so it falls through to the + // launch path rather than declining: the honest reading is "no handle I can see", which is what + // the null branch already means. If the substrate is in fact up, the launcher's own decline or + // the poll bounds what that costs. + let existing: SessionHandle | null = null; + try { + existing = read(deps.dataDir); + } catch (err) { + log.debug('studio auto-launch could not read the session handle', { error: err instanceof Error ? err.message : String(err) }); + } if (existing) { // AHEAD OF THE MEMO CHECK, and it clears it: a handle on disk is direct evidence that whatever // stopped the last launch has been resolved — most often a human who started the app by hand @@ -431,23 +465,41 @@ export async function ensureStudioRunning(deps: AutoLaunchDeps = {}): Promise= deadline) { - log.debug('studio auto-launch did not publish a handle within budget'); - return { handle: null, attempted: true }; + try { + for (;;) { + const h = read(deps.dataDir); + if (h) return { handle: h, attempted: true }; + // Checked AFTER the handle read, because a spawn can both publish and then error, and a + // published handle is the answer the caller wanted either way. Checked BEFORE the deadline + // because that is the whole point: a dead process is knowable now, and the alternative is + // 120 more ticks of waiting for a handle nothing will write. + if (failed()) { + log.debug('studio auto-launch spawned a process that failed to start — abandoning the handle poll'); + return { handle: null, attempted: true }; + } + if (Date.now() >= deadline) { + log.debug('studio auto-launch did not publish a handle within budget'); + return { handle: null, attempted: true }; + } + await sleep(pollMs); } - await sleep(pollMs); + } catch (err) { + log.debug('studio auto-launch could not carry out the handle poll — treating it as a launch that published nothing', { + error: err instanceof Error ? err.message : String(err), + }); + return { handle: null, attempted: true }; } })().then(({ handle, attempted }) => { // Recorded on the SHARED promise rather than in the first caller's `await` below: the other @@ -464,7 +516,20 @@ export async function ensureStudioRunning(deps: AutoLaunchDeps = {}): Promise 0) noHandleUntil = Date.now() + memoMs; + // + // THE CLEAR IS THE SAME EVENT AS THE WRITE, so it lives here rather than in the poll body. The + // memo's own docstring promises "a handle appearing invalidates it immediately", but only the + // TOP-OF-CALL read delivered that: a recovery that ran through the LAUNCH path — a + // memo-bypassing caller whose substrate came up on the second attempt, and whose handle the + // poll therefore saw — left the stale window standing, so when that session ended a default + // caller still inside the original 60 s was declined with no spawn attempt. Reached by the + // succeeding path instead of the failing one, but the same lockout. + // + // Cleared without consulting `memoMs`, exactly like the top-of-call read: `memoMs` says what + // THIS caller wants remembered, and the memo is module state. A live handle disproves it for + // everyone. + if (handle) noHandleUntil = 0; + else if (attempted && memoMs > 0) noHandleUntil = Date.now() + memoMs; return handle; }); try { diff --git a/tests/unit/studio/auto-launch.test.ts b/tests/unit/studio/auto-launch.test.ts index d0b758e1..faf3e177 100644 --- a/tests/unit/studio/auto-launch.test.ts +++ b/tests/unit/studio/auto-launch.test.ts @@ -757,6 +757,51 @@ describe('a launch that produced no handle is remembered briefly', () => { } }); + /** + * A HANDLE APPEARING INVALIDATES THE MEMO — INCLUDING ONE THE POLL ITSELF SAW. + * + * The docstring's promise is unqualified ("a handle appearing invalidates it immediately, which + * is what keeps it a memo rather than a lockout"), but only the top-of-call read cleared + * `noHandleUntil`. So the recovery that runs THROUGH the launch path — a memo-bypassing caller + * whose launch publishes, i.e. a substrate that came up on the second attempt — left the stale + * window standing: when that session ended, a default caller still inside the original 60 s was + * declined with no spawn attempt, having just been served by a live handle. That is the lockout, + * reached by the successful path rather than the failing one. + * + * The clear belongs on the shared promise beside the memo write, not in the poll body: the other + * single-flight participants return `inFlight` directly and never run the loop. + */ + it('is invalidated by a handle observed INSIDE the poll, not only by the top-of-call read', async () => { + vi.useFakeTimers(); + try { + const sleep = async (ms: number): Promise => { vi.advanceTimersByTime(ms); }; + const launch = vi.fn(); + const args = { dataDir: dir, launch, launchable: () => true, timeoutMs: 0, sleep }; + + // Arm the memo the ordinary way: a launch that produced no handle. + expect(await ensureStudioRunning(args)).toBeNull(); + expect(launch).toHaveBeenCalledTimes(1); + + // A memo-bypassing caller launches, and THIS one comes up. The handle is published by the + // launcher, so the top-of-call read — which ran before it — cannot be what sees it; only the + // poll can, which is the whole point of the arm. + const lateLaunch = vi.fn(() => { publishHandle(); }); + const served = await ensureStudioRunning({ ...args, memoMs: 0, launch: lateLaunch, timeoutMs: 30_000, pollMs: 250 }); + expect(lateLaunch).toHaveBeenCalledTimes(1); + expect(served?.endpoint).toBe(HANDLE.endpoint); + + // The session ends well inside the ORIGINAL window. The next default caller must launch: it + // was just served by a live substrate, so the remembered failure has been disproved. + rmSync(join(dir, 'studio', 'current.json'), { force: true }); + vi.advanceTimersByTime(1_000); + const third = vi.fn(); + expect(await ensureStudioRunning({ ...args, launch: third })).toBeNull(); + expect(third).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); + it('pins the shipped default window — long enough to spare one fan-out, short enough not to lock out', async () => { // The constant is load-bearing and was unpinned: every arm above passes `memoMs` explicitly, so // `DEFAULT_NO_HANDLE_MEMO_MS` could be raised to an hour with the suite still green — turning @@ -809,6 +854,33 @@ describe('normalizeLaunch rejects a shape it cannot honestly read', () => { expect(() => normalizeLaunch({ launched: true } as never)).toThrow(/LaunchOutcome/); }); + /** + * THE CHOICE, RECORDED: an illegal launcher answer THROWS rather than declining quietly. + * + * `started` was reached by a trailing "anything else = legacy true", so every falsy value that is + * not `false` — `null`, `0`, `''`, `NaN` — was read as STARTED: one token away from a correct + * decline, and the reward was the full 30 s handle poll for a process nobody launched, followed + * by a 60 s negative memo locking the next caller out. `() => null` is what a launcher written + * against `readSubstrateRecord`'s own return shape produces. + * + * Throwing was chosen over silently declining for two reasons. It is the treatment the seam + * already gives every other unreadable shape (a thenable, a non-LaunchOutcome object, a + * non-callable `failed`) — one rule, not a second dialect for primitives. And it is not a + * behavioural gamble: the throw is synchronous at the seam, so `ensureStudioRunning`'s launcher + * try/catch turns it into a clean zero-tick decline that is NOT memoized, which is the safer of + * the two candidate behaviours anyway. The difference is purely that the operator gets a logged + * reason instead of a mystery. Reversal condition in DECISIONS-AUTO (2026-08-29). + * + * The guard is an ALLOWLIST rather than a falsy check, because the same trailing branch also read + * `1` and `'yes'` as started — the identical guess, made about a value that merely happens to be + * truthy. + */ + it('throws on any answer that is not `false`, `true`, `void` or a LaunchOutcome', () => { + for (const illegal of [null, 0, '', Number.NaN, 1, 'started']) { + expect(() => normalizeLaunch(illegal as never)).toThrow(/not a launcher answer/); + } + }); + it('still reads all three legacy shapes', () => { expect(normalizeLaunch(false).started).toBe(false); expect(normalizeLaunch(true).started).toBe(true); @@ -863,6 +935,116 @@ describe('normalizeLaunch rejects a shape it cannot honestly read', () => { }); }); +/** + * NEVER-THROWS IS A CONTRACT, AND THE POLL LOOP SAT OUTSIDE EVERY GUARD THAT ENFORCED IT. + * + * `ensureStudioRunning`'s docstring ends "never throws, because every caller has a degraded path and + * a launch problem must not become the user's error". The try/catch that delivered that only covered + * the LAUNCHER CALL. Everything the poll loop invokes each tick — `failed()`, `readHandleFn()`, + * `sleep()` — ran a tick past it, the `.then` memo mapper had no rejection branch, and the body is + * `try { return await inFlight } finally` with no catch. So a probe that throws propagated all the + * way into `studioBridgeFetch`, which awaits with no catch: a launch problem became the caller's + * fetch error. + * + * The rejection also SKIPPED THE MEMO WRITE, so the fan-out re-paid the poll budget per URL — the + * second failure hiding behind the first. + * + * These resolve rather than reject, and they are memoized, because `started` was true: something was + * launched and no handle appeared, which is exactly the case the negative memo exists for. A + * LAUNCHER that throws stays un-memoized (arms above) because nothing was started at all. + */ +describe('a fault inside the handle poll is an outcome, never a rejection', () => { + const throwingProbe = (): boolean => { throw new Error('probe boom'); }; + + it('a `failed` probe that throws gives up the poll instead of rejecting, and is remembered', async () => { + vi.useFakeTimers(); + try { + let ticks = 0; + const sleep = async (ms: number): Promise => { ticks += 1; vi.advanceTimersByTime(ms); }; + const launch = vi.fn(() => ({ started: true, failed: throwingProbe })); + const args = { dataDir: dir, launchable: () => true, timeoutMs: 30_000, pollMs: 250, sleep }; + + await expect(ensureStudioRunning({ ...args, launch })).resolves.toBeNull(); + expect(launch).toHaveBeenCalledTimes(1); + // Abandoned on the tick it threw, not on tick 120: a probe that cannot answer is no more + // waitable than one that answered "dead". + expect(ticks).toBe(0); + + // …and the attempt was recorded, so the next challenged page is declined for free rather than + // re-paying the budget into the same broken seam. + const second = vi.fn(); + expect(await ensureStudioRunning({ ...args, launch: second })).toBeNull(); + expect(second).not.toHaveBeenCalled(); + } finally { + vi.useRealTimers(); + } + }); + + it('a `sleep` that rejects mid-poll resolves null rather than rejecting', async () => { + const launch = vi.fn(() => ({ started: true, failed: () => false })); + const sleep = async (): Promise => { throw new Error('sleep boom'); }; + // A real timeout, so the loop genuinely reaches the sleep rather than expiring on tick one. + const args = { dataDir: dir, launch, launchable: () => true, timeoutMs: 30_000, pollMs: 250, sleep }; + + await expect(ensureStudioRunning(args)).resolves.toBeNull(); + expect(launch).toHaveBeenCalledTimes(1); + }); + + it('a `readHandleFn` that throws resolves null — at the top-of-call read as well as in the poll', async () => { + // The top read is the FIRST thing the function does and sat outside every guard too, so the + // plainest possible fake — a reader that always throws — never even reached the poll: it + // rejected synchronously from the entry point. + const readHandleFn = vi.fn((): SessionHandle | null => { throw new Error('handle read boom'); }); + const launch = vi.fn(); + const args = { dataDir: dir, launch, launchable: () => true, timeoutMs: 0, sleep: noSleep, readHandleFn }; + + await expect(ensureStudioRunning(args)).resolves.toBeNull(); + // Reached the launch path rather than being answered by the unreadable read: a handle that + // cannot be read is not evidence the substrate is absent. + expect(launch).toHaveBeenCalledTimes(1); + expect(readHandleFn.mock.calls.length).toBeGreaterThan(1); + }); + + /** + * THE PAIRED POSITIVE ARM. Without it, a `catch { return NOT_ATTEMPTED }` wrapped around the whole + * body satisfies every arm above while quietly deleting the poll. + */ + it('still returns a handle the poll finds, and still honours a probe that reports failure', async () => { + let dead = false; + const launch = vi.fn(() => { dead = true; return { started: true, failed: () => dead }; }); + expect(await ensureStudioRunning({ dataDir: dir, launch, launchable: () => true, timeoutMs: 30_000, pollMs: 1, sleep: noSleep })).toBeNull(); + + resetAutoLaunchState(); + const good = vi.fn(() => { publishHandle(); return { started: true, failed: () => false }; }); + const h = await ensureStudioRunning({ dataDir: dir, launch: good, launchable: () => true, timeoutMs: 30_000, pollMs: 1, sleep: noSleep }); + expect(h?.endpoint).toBe(HANDLE.endpoint); + }); + + /** + * The end-to-end half of the falsy-answer arm: whichever way the seam reports it, the cost must be + * zero ticks and no memo. Mirrors the launcher-decline arm — nothing was started, so nothing is + * waited for and nothing is remembered. + */ + it('a falsy non-`false` launch answer costs zero poll ticks and is not remembered', async () => { + vi.useFakeTimers(); + try { + let ticks = 0; + const sleep = async (ms: number): Promise => { ticks += 1; vi.advanceTimersByTime(ms); }; + const launch = vi.fn(() => null); + const args = { dataDir: dir, launchable: () => true, timeoutMs: 30_000, pollMs: 250, sleep }; + + expect(await ensureStudioRunning({ ...args, launch: launch as never })).toBeNull(); + expect(ticks).toBe(0); + + const second = vi.fn(); + expect(await ensureStudioRunning({ ...args, launch: second })).toBeNull(); + expect(second).toHaveBeenCalledTimes(1); + } finally { + vi.useRealTimers(); + } + }); +}); + /** * THE HIDDEN FLAG SURVIVES A CASE-INSENSITIVE PARENT, which is not the same claim as "the flag is * set" that `defaultLaunch`'s spawn arm already makes.