diff --git a/packages/modal/src/resume.ts b/packages/modal/src/resume.ts index feda72a4e..65d21d979 100644 --- a/packages/modal/src/resume.ts +++ b/packages/modal/src/resume.ts @@ -3,6 +3,8 @@ import { lstat, mkdir, open, readFile, readdir, rename, unlink } from "node:fs/p import type { FileHandle } from "node:fs/promises"; import path from "node:path"; +import { layoutForRunRoot } from "@ultrafuzz/artifacts"; + import { EVAL_WATCH_TIMEOUT_SECONDS } from "./defaults.js"; import type { TerminalDisposition } from "./terminal-disposition.js"; @@ -92,34 +94,288 @@ export function modalEvalRunCommand(input: { ]; } -export async function locateModalResumeWorkspace(workRoot: string): Promise { +/** + * What a volume holds: either a run that can genuinely be resumed, or one that never got far enough to + * have anything to resume from. + * + * `not-started` is deliberately NOT an error. A worker that finds it should build the workspace through + * the normal pre-model path instead of failing (#378). + */ +export type ModalResumeLookup = + | { readonly kind: "resumable"; readonly workspace: ModalResumeWorkspace } + | { + readonly kind: "not-started"; + readonly reason: string; + readonly staleEvalRunIds?: readonly string[]; + /** Run roots that exist but can never be resumed, so a restart must clear them (`RUN_ALREADY_EXISTS`). */ + readonly staleRunRootIds?: readonly string[]; + }; + +/** + * Decide whether a volume holds a resumable run, WITHOUT treating "nothing to resume" as a failure. + * + * The problem this solves (#378): `runs.jsonl` is the journal that links an evaluation row to the durable + * run executing it, and that link is appended only AFTER the launcher returns. `startRun` creates the run + * root, writes `state.json`, compiles the workflow and submits it before returning. So there is a wide + * window — the whole of compilation and submission — in which a real durable run exists on disk, with real + * completed nodes, and nothing in the journal points at it. Reading the journal alone, that is + * indistinguishable from "never started", and the worker used to reject it outright and wedge the run + * permanently, because the state is on a durable volume and so recurs every generation. + * + * The journal is therefore treated as a cache, not as the source of truth. When it holds no link, the + * durable run directory is consulted directly: + * + * - **a run linked to a workflow exists** -> resumable. Its journal link is missing, not the run. + * `repairModalEvalRunRecord` writes that link back when the run finalizes. Restarting here would + * abandon completed work, and would fail anyway: row run ids are deterministic, so `planRun` would + * refuse with `RUN_ALREADY_EXISTS`. + * - **no linked run exists** -> genuinely not started. Both ids involved are deterministic, so the caller + * is told what to clear before restarting: `staleEvalRunIds` for eval run directories + * (`EVAL_RUN_ALREADY_EXISTS`) and `staleRunRootIds` for unlinked run roots (`RUN_ALREADY_EXISTS`). + * + * Counts of linked runs stay asymmetric: more than one durable run linked to a single evaluation is + * corruption no correct producer can create, and silently picking a side would destroy a real run. + */ +export async function findModalResumeWorkspace(workRoot: string): Promise { const target = path.join(workRoot, "target"); const control = path.join(workRoot, "control"); - if (!(await isDirectoryNotSymlink(target)) || !(await isDirectoryNotSymlink(control))) { - throw new Error("persistent workspace is incomplete"); - } const evalRoot = path.join(control, ".ultrafuzz", "evals", "runs"); + // Only a genuinely absent directory means "nothing here". EACCES, EIO or ESTALE on a durable volume are + // faults, and reading them as "the run never started" is how a transient mount problem would become a + // restart over live work. + const names = await readdirIfMissing(evalRoot); + const directories = []; const candidates = []; - for (const name of await readdir(evalRoot)) { + const unclassifiableEvalEntries = []; + for (const name of names) { const root = path.join(evalRoot, name); - if ((await isDirectoryNotSymlink(root)) && (await isRegularFileNotSymlink(path.join(root, "eval.json")))) { - candidates.push(name); + // Same reasoning as the run-root loop below: `runEvalSuite` refuses on `fs.existsSync`, which follows + // symlinks and does not care about entry type, so an entry skipped silently here would still block a + // restart. Report it instead of dropping it into invisibility. + if (!(await isDirectoryNotSymlink(root))) { + unclassifiableEvalEntries.push(name); + continue; + } + // Tracked separately from `candidates`: `runEvalSuite` refuses to reuse an id whose DIRECTORY exists, + // and it creates that directory before writing `eval.json`. A kill in between leaves a directory that + // is not a candidate but still blocks reuse, so a restart has to be told to clear it. + directories.push(name); + if (await isRegularFileNotSymlink(path.join(root, "eval.json"))) candidates.push(name); + } + // Assert the shape of whatever IS present before deciding anything, including in the zero-candidate case. + // A path that exists but is a symlink is damage at any candidate count, and the rest of this file is + // careful to refuse symlinks everywhere; skipping that here would let a symlinked `control` through. + await assertWorkspaceShape(target); + await assertWorkspaceShape(control); + if (candidates.length === 0) { + const durable = await durableRunsOnDisk(target); + if (durable.resumable.length > 0) { + // A linked run with no evaluation to attach it to is damage, not a fresh start. Restarting would + // abandon it and deleting it is exactly what this function exists to avoid, so refuse instead. + throw new Error(`workspace has ${durable.resumable.length} linked durable run(s) but no evaluation run`); } + assertNoDamagedRunRoots(durable.damaged); + assertNoUnclassifiableEvalEntries(unclassifiableEvalEntries); + return { + kind: "not-started", + reason: "resume requires exactly one evaluation run, found 0", + ...(directories.length === 0 ? {} : { staleEvalRunIds: directories }), + ...(durable.orphaned.length === 0 ? {} : { staleRunRootIds: durable.orphaned }) + }; } - if (candidates.length !== 1) { + if (candidates.length > 1) { throw new Error(`resume requires exactly one evaluation run, found ${candidates.length}`); } + // The workspace now claims to hold an evaluation, so target and control must both actually be there. + if (!(await isDirectoryNotSymlink(target)) || !(await isDirectoryNotSymlink(control))) { + throw new Error("persistent workspace is incomplete"); + } const evalRunId = candidates[0]!; - const records = await readRecords(path.join(evalRoot, evalRunId, "runs.jsonl")); + // A missing journal is the earliest form of the same window: `eval.json` is written before the first + // append, so a kill in between leaves no file at all. That is a state to classify, not to crash on. + const records = await readRecordsIfMissing(path.join(evalRoot, evalRunId, "runs.jsonl")); const productRunIds = new Set( records .map((record) => record.ultrafuzz_run_id) .filter((value): value is string => typeof value === "string" && value.trim() !== "") ); - if (productRunIds.size !== 1) { + if (productRunIds.size > 1) { throw new Error(`resume requires exactly one linked durable run, found ${productRunIds.size}`); } - return { target, control, evalRunId, productRunId: [...productRunIds][0]! }; + if (productRunIds.size === 1) { + return { kind: "resumable", workspace: { target, control, evalRunId, productRunId: [...productRunIds][0]! } }; + } + const durable = await durableRunsOnDisk(target); + if (durable.resumable.length > 1) { + throw new Error(`resume requires exactly one durable run on disk, found ${durable.resumable.length}`); + } + if (durable.resumable.length === 1) { + return { kind: "resumable", workspace: { target, control, evalRunId, productRunId: durable.resumable[0]! } }; + } + // Damage only matters on the branch that would restart: a damaged root blocks `planRun`, but it must not + // preempt resuming a run that is perfectly good. This applies to the `damaged` LIST only — a read fault + // or malformed metadata raised while classifying some other root still propagates from the loop above and + // will preempt a resume. That is deliberate (it fails closed and never deletes), but it is not what this + // deferral promises. + assertNoDamagedRunRoots(durable.damaged); + assertNoUnclassifiableEvalEntries(unclassifiableEvalEntries); + return { + kind: "not-started", + reason: "resume requires exactly one linked durable run, found 0", + // Every eval run directory, not just the candidate: `runEvalSuite` refuses any id whose directory + // exists, so naming only the candidate would leave a non-candidate directory blocking the restart. + staleEvalRunIds: directories, + ...(durable.orphaned.length === 0 ? {} : { staleRunRootIds: durable.orphaned }) + }; +} + +/** + * Run roots on disk, split by whether `resume` could actually resume them. + * + * The line is the workflow link in the run metadata, whose path is taken from `layoutForRunRoot` and never + * restated here — see `hasLinkedWorkflow`. `persistSmithersEvidence` writes that link immediately + * after the workflow compiles and BEFORE it is submitted, and `readLinkedWorkflowEvidence` refuses any run + * without it (`WORKFLOW_RUN_ID_MISSING`). So the two halves of the failure window differ: + * + * - compiled, then failed to submit -> linked, and genuinely resumable. This is the R54 case. + * - died during compilation, or with only a partial run layout -> no link, and `resume` refuses it + * forever. Calling that resumable only relocates the wedge; it has to be cleared instead, or a restart + * trips `RUN_ALREADY_EXISTS` on its deterministic run id. + * + * `state.json` alone is not the test: `createRunLayout` writes it well before compilation, so requiring + * only that would classify a run nothing can resume as resumable. + */ +async function durableRunsOnDisk( + target: string +): Promise<{ resumable: string[]; orphaned: string[]; damaged: string[] }> { + const runsRoot = path.join(target, ".ultrafuzz", "runs"); + const resumable = []; + const orphaned = []; + const damaged = []; + for (const name of await readdirIfMissing(runsRoot)) { + const root = path.join(runsRoot, name); + // An entry this function cannot classify is reported, never skipped. `planRun` refuses on + // `existsSync`, which follows symlinks and does not care whether the name is layout-addressable, so + // anything dropped here silently would still block a restart — invisibly, which is the whole subject + // of #378. + if (!(await isDirectoryNotSymlink(root)) || !isLayoutAddressable(root)) { + damaged.push(name); + continue; + } + const linked = await hasLinkedWorkflow(root); + const hasState = await isRegularFileNotSymlink(path.join(root, "state.json")); + // `orphaned` is a POSITIVE determination — the metadata was read and carries no workflow link — and not + // merely "not resumable". The difference is destructive: the caller deletes orphaned roots, and the + // link is written before submission, so a root that HAS a link is one a workflow may have run from. + // Missing its state is damage to report, never something to delete. + if (!linked) orphaned.push(name); + else if (hasState) resumable.push(name); + else damaged.push(name); + } + return { resumable: resumable.sort(), orphaned: orphaned.sort(), damaged: damaged.sort() }; +} + +/** + * Whether the runtime's own layout helper can address this run root. + * + * Derived by asking the helper rather than restating its id rule. A name it rejects would otherwise throw + * out of the lookup on every generation, with nothing able to clear it — one stray directory would strand + * the run permanently. + */ +function isLayoutAddressable(runRoot: string): boolean { + try { + layoutForRunRoot(runRoot); + return true; + } catch { + return false; + } +} + +/** + * Whether the run metadata carries the workflow link `resume` requires. + * + * The path comes from `layoutForRunRoot`, the same helper the runtime writes and reads it through, and is + * deliberately not restated here. An earlier revision named the file itself and named it wrongly, which + * classified every real run root as unresumable — and because the caller deletes unresumable roots, that + * turned this predicate into a destructive one while leaving #378 unfixed. + * + * Errors are NOT swallowed. Absence means the link was never written; EACCES or EIO on a durable volume + * mean the answer is unknown, and answering "unlinked" to an unknown is how a read fault becomes a + * deletion. Only ENOENT/ENOTDIR count as absence, matching `readdirIfMissing`. + */ +async function hasLinkedWorkflow(runRoot: string): Promise { + const text = await readFile(layoutForRunRoot(runRoot).runMetadataPath, "utf8").catch( + (error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return undefined; + throw error; + } + ); + if (text === undefined) return false; + let metadata: { workflow?: { run_id?: unknown; workflowRunId?: unknown }; smithers?: { workflowRunId?: unknown } }; + try { + metadata = JSON.parse(text) as typeof metadata; + } catch (error) { + // Failing closed is the decision here, not an oversight: reading unparseable metadata as "unlinked" + // would route the root to deletion. Name the file so the refusal is diagnosable from the volume. + throw new Error(`run metadata at ${layoutForRunRoot(runRoot).runMetadataPath} is not valid JSON`, { + cause: error + }); + } + const runId = metadata.workflow?.run_id ?? metadata.workflow?.workflowRunId ?? metadata.smithers?.workflowRunId; + return typeof runId === "string" && runId.length > 0; +} + +/** + * Refuse to restart over an eval run entry that is not a directory. Deleting something we cannot classify + * would be worse, and ignoring it hands the restart an `EVAL_RUN_ALREADY_EXISTS` it cannot clear. + */ +function assertNoUnclassifiableEvalEntries(entries: readonly string[]): void { + if (entries.length === 0) return; + throw new Error( + `evaluation run entr(ies) ${[...entries].sort().join(", ")} are not directories and would block a restart; ` + + `resolve them on the volume` + ); +} + +/** + * Refuse to restart over a run root that cannot be classified, rather than deleting it or ignoring it. + * + * Sorted so the message is deterministic when there is more than one. + */ +function assertNoDamagedRunRoots(damaged: readonly string[]): void { + if (damaged.length === 0) return; + throw new Error( + `durable run root(s) ${damaged.join(", ")} cannot be classified as resumable or unstarted; ` + + `resolve them on the volume before this run can restart` + ); +} + +/** Refuse a path that exists but is not a real directory. An absent path is the ordinary fresh case. */ +async function assertWorkspaceShape(directoryPath: string): Promise { + const stat = await lstat(directoryPath).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return undefined; + throw error; + }); + if (stat !== undefined && !stat.isDirectory()) { + throw new Error("persistent workspace is incomplete"); + } +} + +async function readdirIfMissing(directoryPath: string): Promise { + return readdir(directoryPath).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return [] as string[]; + throw error; + }); +} + +/** + * The strict form. Retained for tests and for any caller that has already established the run must be + * resumable; `worker.ts` deliberately uses the tolerant lookup above instead. + */ +export async function locateModalResumeWorkspace(workRoot: string): Promise { + const found = await findModalResumeWorkspace(workRoot); + if (found.kind === "not-started") throw new Error(found.reason); + return found.workspace; } export async function repairModalEvalRunRecord( @@ -136,12 +392,41 @@ export async function repairModalEvalRunRecord( } const evalDir = path.join(workspace.control, ".ultrafuzz", "evals", "runs", workspace.evalRunId); const recordsPath = path.join(evalDir, "runs.jsonl"); - const records = await readRecords(recordsPath); - const linked = records.filter((record) => record.ultrafuzz_run_id === state.run_id); + const records = await readRecordsIfMissing(recordsPath); + let linked: Array> = records.filter((record) => record.ultrafuzz_run_id === state.run_id); + // #378: the journal may never have recorded the link, because it is appended only after the launcher + // returns. The run itself is the evidence it existed, so write the link the journal is missing rather + // than refuse a run that plainly ran. Only safe when NOTHING is linked: a journal that points at some + // other durable run is a genuine mismatch and must still be refused. + const linkageMissing = linked.length === 0 && !records.some((record) => isLinked(record)); + if (linkageMissing) { + linked = [{ ...(records[records.length - 1] ?? {}), row_id: await soleRowId(evalDir, records) }]; + } if (linked.length === 0) throw new Error(`evaluation run does not reference durable run ${state.run_id}`); const rowIds = new Set(linked.map((record) => record.row_id)); if (rowIds.size !== 1) throw new Error("evaluation run has ambiguous linked rows"); - const record = linked[linked.length - 1]!; + // A repaired row has to be the row a successful launch would have written, not the failed row with a link + // stamped on it. `scoreEvalRun` resolves the terminal report through `ultrafuzz_run_root`, and without it + // falls back to a path built from `row.run_id` — a different directory from the bounded run id actually on + // disk — so the run would drive to success and then fail scoring, wedging one step further downstream. + // `status` matters for the same reason: efficiency and status reporting read it, and would otherwise call + // a launch failed for a run that succeeded. + const record: Record = { + ...linked[linked.length - 1]!, + ultrafuzz_run_id: state.run_id, + ...(linkageMissing + ? { + ultrafuzz_run_root: path.join(workspace.target, ".ultrafuzz", "runs", state.run_id), + status: "launched", + launcher: { + ...(typeof (linked[linked.length - 1] as Record | undefined)?.launcher === "object" + ? ((linked[linked.length - 1] as Record).launcher as Record) + : {}), + status: "succeeded" + } + } + : {}) + }; const finalStatus = state.status; const currentWorkflow = typeof record.workflow === "object" && record.workflow !== null && !Array.isArray(record.workflow) @@ -167,7 +452,9 @@ export async function repairModalEvalRunRecord( currentWorkflow?.finished_at === workflow.finished_at ? record : { ...record, final_status: finalStatus, workflow, finished_at: finishedAt }; - if (updated !== record) await appendLineDurable(recordsPath, `${JSON.stringify(updated)}\n`); + // A repaired link must reach the journal even when nothing else about the row changed, otherwise the + // next generation would have to rediscover it from disk all over again. + if (updated !== record || linkageMissing) await appendLineDurable(recordsPath, `${JSON.stringify(updated)}\n`); await writeJsonAtomic(path.join(evalDir, "run-summary.json"), { eval_run_id: workspace.evalRunId, launched: 1, @@ -177,10 +464,54 @@ export async function repairModalEvalRunRecord( }); } +function isLinked(record: Record): boolean { + return typeof record.ultrafuzz_run_id === "string" && record.ultrafuzz_run_id.trim() !== ""; +} + +/** + * The one evaluation row a repaired link belongs to. + * + * Prefers the journal, and falls back to `matrix.json` for the case where the journal was never written at + * all. Refuses anything ambiguous: attaching a durable run to the wrong row would misreport which + * configuration produced the result, which is worse than failing to repair. + */ +async function soleRowId(evalDir: string, records: Array>): Promise { + const journalRowIds = new Set( + records.map((record) => record.row_id).filter((value): value is string => typeof value === "string") + ); + if (journalRowIds.size === 1) return [...journalRowIds][0]!; + if (journalRowIds.size > 1) throw new Error("evaluation run has ambiguous linked rows"); + const matrix = (await readFile(path.join(evalDir, "matrix.json"), "utf8").then( + (text) => JSON.parse(text) as unknown, + () => undefined + )) as Array<{ id?: unknown }> | undefined; + const matrixRowIds = Array.isArray(matrix) + ? matrix.map((row) => row?.id).filter((value): value is string => typeof value === "string") + : []; + if (matrixRowIds.length !== 1) { + throw new Error(`evaluation run has no unambiguous row to link, found ${matrixRowIds.length}`); + } + return matrixRowIds[0]!; +} + function timestamp(value: unknown): string | undefined { return typeof value === "string" && value.trim() !== "" && Number.isFinite(Date.parse(value)) ? value : undefined; } +/** + * Records, treating an absent journal as no records. + * + * A malformed line still throws, deliberately. `appendJsonLine` does not fsync, so a torn final line is + * possible on a durable volume — and a torn line may be the very line carrying the durable run link. + * Dropping it silently would read as "never linked" and could strand a live run, so this stays loud. + */ +async function readRecordsIfMissing(filePath: string): Promise>> { + return readRecords(filePath).catch((error: NodeJS.ErrnoException) => { + if (error.code === "ENOENT" || error.code === "ENOTDIR") return []; + throw error; + }); +} + async function readRecords(filePath: string): Promise>> { return (await readFile(filePath, "utf8")) .split(/\r?\n/u) diff --git a/packages/modal/src/worker.ts b/packages/modal/src/worker.ts index 9083b5acd..b87ed5cbf 100644 --- a/packages/modal/src/worker.ts +++ b/packages/modal/src/worker.ts @@ -17,7 +17,7 @@ import { resolvePersistentRemoteRoot } from "./layout.js"; import { - locateModalResumeWorkspace, + findModalResumeWorkspace, modalDurableResumeCommand, modalDurableRunAdvanced, modalDurableRunNeedsResume, @@ -141,13 +141,36 @@ async function main(): Promise { let control: string; let evalRunId: string; let terminalDisposition: TerminalDisposition | undefined; - const existing = await hasExistingEvalWorkspace(); - if (existing) { - const workspace = await locateModalResumeWorkspace(WORK_ROOT); + // Route on whether a durable run actually exists, not on whether an eval run directory exists. Those + // are not the same question, and answering the second one stranded R54: a generation-0 failure during + // workflow submission leaves the directory behind with nothing linked to it, and every later + // generation then died on the resume precondition in ~92s until the no-progress budget ran out (#378). + const found = await findModalResumeWorkspace(WORK_ROOT); + if (found.kind === "resumable") { + const workspace = found.workspace; target = workspace.target; const resumed = await resumeExistingEvaluation(workspace, writer); ({ control, evalRunId, terminalDisposition } = resumed); } else { + // Nothing resumable exists, but a previous attempt may have left state that a restart would trip + // over. Both ids involved are deterministic, so leaving either behind only moves the permanent + // failure: `runEvalSuite` refuses an existing eval run directory (`EVAL_RUN_ALREADY_EXISTS`), and + // `planRun` refuses an existing run root (`RUN_ALREADY_EXISTS`). + // + // This is the only destructive step, and it is bounded by what the lookup proved: a run root is + // named here only when its metadata is absent or carries no workflow link. That link is written + // before submission, so either way no workflow was ever submitted from that root. A root that IS + // linked but is missing its state is reported as damage rather than named here. A run that + // compiled — the R54 case — is linked, so it resumes above and never reaches this branch. + for (const staleEvalRunId of found.staleEvalRunIds ?? []) { + await rm(path.join(WORK_ROOT, "control", ".ultrafuzz", "evals", "runs", staleEvalRunId), { + recursive: true, + force: true + }); + } + for (const runRootId of found.staleRunRootIds ?? []) { + await rm(path.join(WORK_ROOT, "target", ".ultrafuzz", "runs", runRootId), { recursive: true, force: true }); + } const prepared = await prepareWorkspace(); target = prepared.target; ({ control, evalRunId } = prepared); @@ -210,11 +233,6 @@ async function main(): Promise { }); } -async function hasExistingEvalWorkspace(): Promise { - const evalRoot = path.join(WORK_ROOT, "control", ".ultrafuzz", "evals", "runs"); - return (await readdirIfExists(evalRoot)).length > 0; -} - async function resumeExistingEvaluation( workspace: ModalResumeWorkspace, writer: WorkerResultWriter @@ -229,10 +247,14 @@ async function resumeExistingEvaluation( runRoot: path.join(workspace.target, ".ultrafuzz", "runs", workspace.productRunId) }); if (repairedPrompts > 0) await flushVolume(); - modelWorkStarted = true; await writer.writePartial(await readWorkerCheckpoint(workspace.target)); let state = await durableRunState(workspace.target, workspace.productRunId); if (state === undefined) throw new CheckpointIncompatibleError("persistent workspace is missing durable run state"); + // Claim model work only once the durable run is known to be readable. Claiming it earlier costs the tight + // pre-model retry bound: an attempt reporting model work resets the streak `MODAL_PRE_MODEL_RETRY_LIMIT` + // counts, so a run that cannot even load its state would burn the whole no-progress budget instead of + // failing after three attempts with an accurate diagnosis. + modelWorkStarted = true; let disposition = await terminalDispositionForState(workspace, state); const checkpoint = await readWorkerCheckpoint(workspace.target); if (modalDurableRunNeedsResume(state, checkpoint.counts)) { diff --git a/packages/modal/test/resume.test.ts b/packages/modal/test/resume.test.ts index e378df486..55dce0c7f 100644 --- a/packages/modal/test/resume.test.ts +++ b/packages/modal/test/resume.test.ts @@ -4,7 +4,11 @@ import path from "node:path"; import { describe, expect, it } from "vitest"; +import { layoutForRunRoot } from "@ultrafuzz/artifacts"; +import { evalRunRoot } from "@ultrafuzz/evals"; + import { + findModalResumeWorkspace, locateModalResumeWorkspace, modalDurableResumeCommand, modalDurableRunAdvanced, @@ -18,6 +22,23 @@ const T0 = "2026-07-19T00:00:00.000Z"; const T1 = "2026-07-19T00:01:00.000Z"; const T2 = "2026-07-19T00:02:00.000Z"; +function writeRunRoot(target: string, runId: string, options: { linked: boolean }) { + const runRoot = path.join(target, ".ultrafuzz", "runs", runId); + fs.mkdirSync(runRoot, { recursive: true }); + fs.writeFileSync(path.join(runRoot, "state.json"), JSON.stringify({ run_id: runId, nodes: {} })); + if (options.linked) { + // The workflow link `resume` requires, at the path the RUNTIME writes it to. Derived from + // `layoutForRunRoot`, never spelled out: an earlier revision invented the filename here and in the + // source, so the suite agreed with the bug and all tests passed while every real run root was + // misclassified as unresumable -- and therefore deletable. + fs.writeFileSync( + layoutForRunRoot(runRoot).runMetadataPath, + JSON.stringify({ workflow: { run_id: "wf-1", path: "workflow.tsx" } }) + ); + } + return runRoot; +} + function fixture() { const workRoot = mkdtempSync(path.join(tmpdir(), "ultrafuzz-modal-resume-")); const target = path.join(workRoot, "target"); @@ -156,6 +177,293 @@ describe("Modal durable evaluation resume", () => { await expect(locateModalResumeWorkspace(value.workRoot)).rejects.toThrow("exactly one linked durable run"); }); + it("resumes a durable run that exists on disk but was never linked in the journal (#378)", async () => { + const value = fixture(); + // The R54 state, and the one that matters most: the launcher created the run, wrote state.json, + // compiled and submitted -- and died before the link was appended. Nine nodes had already succeeded. + // Reading the journal alone this looks identical to "never started"; restarting would abandon that + // work and would fail anyway, because row run ids are deterministic. + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + writeRunRoot(value.target, "durable-run-one", { linked: true }); + await expect(findModalResumeWorkspace(value.workRoot)).resolves.toEqual({ + kind: "resumable", + workspace: { + target: value.target, + control: value.control, + evalRunId: value.evalRunId, + productRunId: "durable-run-one" + } + }); + }); + + it("reports not started, and names the directory to clear, when no durable run exists (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + // Naming it is the point: eval run ids are deterministic and `runEvalSuite` refuses to reuse one, so a + // caller that restarts without clearing this hits EVAL_RUN_ALREADY_EXISTS forever. + expect(found.kind === "not-started" && found.staleEvalRunIds).toEqual([value.evalRunId]); + }); + + it("classifies a journal that was never written, rather than crashing on it (#378)", async () => { + const value = fixture(); + // `eval.json` is written before the first journal append, so a kill in between leaves no runs.jsonl. + fs.rmSync(path.join(value.evalDir, "runs.jsonl")); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + }); + + it("refuses to guess when several durable runs are on disk and none is linked", async () => { + const value = fixture(); + fs.writeFileSync(path.join(value.evalDir, "runs.jsonl"), `${JSON.stringify({ row_id: "row-one" })}\n`); + for (const runId of ["durable-run-one", "durable-run-two"]) writeRunRoot(value.target, runId, { linked: true }); + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("exactly one durable run on disk"); + }); + + it("reports a workspace with no eval run directory at all as not started", async () => { + const workRoot = mkdtempSync(path.join(tmpdir(), "ultrafuzz-modal-resume-fresh-")); + // A fresh sandbox must not be mistaken for a corrupt one. + const found = await findModalResumeWorkspace(workRoot); + expect(found.kind).toBe("not-started"); + expect(found.kind === "not-started" && found.staleEvalRunIds).toBeUndefined(); + }); + + it("still refuses a symlinked control even when it holds no evaluation run", async () => { + const value = fixture(); + const elsewhere = mkdtempSync(path.join(tmpdir(), "ultrafuzz-modal-resume-elsewhere-")); + fs.rmSync(value.control, { recursive: true }); + fs.symlinkSync(elsewhere, value.control); + // readdir follows symlinks, so without an explicit shape assertion this would read as "not started" + // and a swapped control directory would be accepted silently. + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("persistent workspace is incomplete"); + }); + + it("treats a non-directory eval runs path as not started, and clears nothing", async () => { + const value = fixture(); + // ENOTDIR is classified like absence, deliberately: there is no evaluation to resume. What matters is + // that it names nothing to delete, so the not-started path cannot remove anything on this route. + const evalRoot = path.join(value.control, ".ultrafuzz", "evals", "runs"); + fs.rmSync(evalRoot, { recursive: true }); + fs.writeFileSync(evalRoot, "not a directory\n"); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + expect(found.kind === "not-started" && found.staleEvalRunIds).toBeUndefined(); + expect(found.kind === "not-started" && found.staleRunRootIds).toBeUndefined(); + }); + + it("refuses to call a run root resumable when no workflow was ever linked to it (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + // A run killed while compiling has state.json but no workflow link, and `resume` refuses it forever with + // WORKFLOW_RUN_ID_MISSING. Calling it resumable would relocate the wedge rather than remove it; it has + // to be named for clearing, or the restart trips RUN_ALREADY_EXISTS on its deterministic run id. + writeRunRoot(value.target, "durable-run-one", { linked: false }); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + expect(found.kind === "not-started" && found.staleRunRootIds).toEqual(["durable-run-one"]); + }); + + it("still resumes a linked durable run, and still rejects ambiguity, through the tolerant lookup", async () => { + const value = fixture(); + await expect(findModalResumeWorkspace(value.workRoot)).resolves.toEqual({ + kind: "resumable", + workspace: { + target: value.target, + control: value.control, + evalRunId: value.evalRunId, + productRunId: "durable-run-one" + } + }); + fs.appendFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-two", ultrafuzz_run_id: "durable-run-two" })}\n` + ); + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("exactly one linked durable run"); + }); + + it("writes back the durable run link the journal never recorded (#378)", async () => { + const value = fixture(); + // Resume rediscovered the run from disk; finalizing must repair the journal rather than refuse a run + // that plainly ran, or the next generation would have to rediscover it all over again. + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + await repairModalEvalRunRecord( + { target: value.target, control: value.control, evalRunId: value.evalRunId, productRunId: "durable-run-one" }, + { run_id: "durable-run-one", status: "succeeded", started_at: T0, finished_at: T1 }, + undefined + ); + const rows = fs + .readFileSync(path.join(value.evalDir, "runs.jsonl"), "utf8") + .split("\n") + .filter((line) => line.trim() !== "") + .map((line) => JSON.parse(line) as Record); + // The link alone is not enough. `scoreEvalRun` resolves the terminal report through + // `ultrafuzz_run_root`, and without it falls back to a path built from `row.run_id`, which is a + // different directory from the bounded run id on disk -- so the run would succeed and then fail + // scoring. `status` matters too: efficiency and status reporting read it, not `final_status`. + expect(rows[rows.length - 1]).toMatchObject({ + row_id: "row-one", + ultrafuzz_run_id: "durable-run-one", + ultrafuzz_run_root: path.join(value.target, ".ultrafuzz", "runs", "durable-run-one"), + status: "launched", + final_status: "succeeded" + }); + // And the repaired journal must now be resumable on its own, without consulting the disk again. + await expect(findModalResumeWorkspace(value.workRoot)).resolves.toMatchObject({ + kind: "resumable", + workspace: { productRunId: "durable-run-one" } + }); + }); + + it("refuses to adopt a row when the journal points at a different durable run", async () => { + const value = fixture(); + // The fixture row links durable-run-one. Finalizing a DIFFERENT run must stay a hard mismatch: this is + // the guard that keeps link repair from attaching a run to somebody else's row. + await expect( + repairModalEvalRunRecord( + { target: value.target, control: value.control, evalRunId: value.evalRunId, productRunId: "durable-run-two" }, + { run_id: "durable-run-two", status: "succeeded", started_at: T0, finished_at: T1 }, + undefined + ) + ).rejects.toThrow("does not reference durable run"); + }); + + it("names a stale directory that is exactly the one runEvalSuite refuses to reuse (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + const stale = found.kind === "not-started" ? found.staleEvalRunIds?.[0] : undefined; + expect(stale).toBeDefined(); + // This is the coupling that made the first attempt at this fix a no-op. `runEvalSuite` throws + // EVAL_RUN_ALREADY_EXISTS when `evalRunRoot(control, evalRunId)` exists, and eval run ids are + // deterministic, so a caller that restarts without removing precisely THIS path fails forever. Pinning + // the identity here means a change to either side breaks a test rather than a run. + const refusedByRunner = evalRunRoot(value.control, stale!); + expect(refusedByRunner).toBe(path.join(value.control, ".ultrafuzz", "evals", "runs", stale!)); + expect(fs.existsSync(refusedByRunner)).toBe(true); + fs.rmSync(refusedByRunner, { recursive: true, force: true }); + expect(fs.existsSync(refusedByRunner)).toBe(false); + }); + + it("reads the workflow link from the path the runtime actually writes it to (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + const runRoot = writeRunRoot(value.target, "durable-run-one", { linked: true }); + // Pin the coupling itself, not just the behaviour. The metadata file is `run.json`, and the only reason + // this predicate is trustworthy is that it derives the path instead of naming it -- when it named it, + // it named it wrongly and the caller deleted every real run root. + expect(path.basename(layoutForRunRoot(runRoot).runMetadataPath)).toBe("run.json"); + expect(fs.existsSync(path.join(runRoot, "run.json"))).toBe(true); + await expect(findModalResumeWorkspace(value.workRoot)).resolves.toMatchObject({ kind: "resumable" }); + }); + + it("names an eval run directory with no eval.json for clearing (#378)", async () => { + const value = fixture(); + // `runEvalSuite` creates the directory and only then writes eval.json, and it refuses to reuse an id + // whose DIRECTORY exists. A kill in that window leaves a directory that is not a candidate but still + // blocks reuse, so failing to name it wedges on EVAL_RUN_ALREADY_EXISTS just as surely. + fs.rmSync(path.join(value.evalDir, "eval.json")); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + expect(found.kind === "not-started" && found.staleEvalRunIds).toEqual([value.evalRunId]); + }); + + it("reports a linked run with no state as damage rather than deleting it (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + const runRoot = writeRunRoot(value.target, "durable-run-one", { linked: true }); + fs.rmSync(path.join(runRoot, "state.json")); + // The link is written before submission, so a linked root is one a workflow may have run from. It must + // never be named for deletion merely because it is not resumable. + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("cannot be classified"); + }); + + it("refuses a linked durable run that has no evaluation run to attach it to", async () => { + const value = fixture(); + fs.rmSync(value.evalDir, { recursive: true }); + writeRunRoot(value.target, "durable-run-one", { linked: true }); + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("no evaluation run"); + }); + + it("does not let a damaged run root preempt resuming a perfectly good one (#378)", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + writeRunRoot(value.target, "durable-run-one", { linked: true }); + const broken = writeRunRoot(value.target, "durable-run-two", { linked: true }); + fs.rmSync(path.join(broken, "state.json")); + // Damage blocks a RESTART, because `planRun` would trip over it. It must not block a resume: refusing + // here would strand a run that is ready to continue. + await expect(findModalResumeWorkspace(value.workRoot)).resolves.toMatchObject({ + kind: "resumable", + workspace: { productRunId: "durable-run-one" } + }); + }); + + it("refuses a run root whose name the runtime layout cannot address, instead of throwing forever", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + // `layoutForRunRoot` rejects ids outside its safe-id rule. Letting that throw escape would strand the + // run permanently, since the lookup would fail before naming anything a restart could clear. + fs.mkdirSync(path.join(value.target, ".ultrafuzz", "runs", ".tmp-junk"), { recursive: true }); + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("cannot be classified"); + }); + + it("names every eval run directory that would block a restart, not just the candidate", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + // A directory with no eval.json is not a candidate, but `runEvalSuite` still refuses to reuse its id. + fs.mkdirSync(path.join(value.control, ".ultrafuzz", "evals", "runs", "leftover-run"), { recursive: true }); + const found = await findModalResumeWorkspace(value.workRoot); + expect(found.kind).toBe("not-started"); + expect(found.kind === "not-started" && [...(found.staleEvalRunIds ?? [])].sort()).toEqual( + [value.evalRunId, "leftover-run"].sort() + ); + }); + + it("refuses an eval run entry that is not a directory, instead of silently skipping it", async () => { + const value = fixture(); + fs.writeFileSync( + path.join(value.evalDir, "runs.jsonl"), + `${JSON.stringify({ row_id: "row-one", status: "failed", final_status: "failed" })}\n` + ); + // `runEvalSuite` refuses on `existsSync`, which follows symlinks and ignores entry type, so an entry + // dropped here would block the restart with nothing able to clear it -- the same asymmetry that the + // run-root loop closes. Refusing is the conservative half: it never deletes what it cannot classify. + fs.writeFileSync(path.join(value.control, ".ultrafuzz", "evals", "runs", "not-a-directory"), "x\n"); + await expect(findModalResumeWorkspace(value.workRoot)).rejects.toThrow("are not directories"); + }); + it("finalizes succeeded and genuine task outcomes without resetting completed nodes", async () => { const value = fixture(); const workspace = await locateModalResumeWorkspace(value.workRoot);