Skip to content

refactor(runtime): give resume, replay and fork explicit result types - #1208

Merged
aviggiano merged 4 commits into
mainfrom
claude/x05-typed-resume-result
Sep 29, 2026
Merged

aviggiano merged 4 commits into
mainfrom
claude/x05-typed-resume-result

Conversation

@aviggiano

@aviggiano aviggiano commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Lifecycle result types. resumeRun, replayRun and forkRun had inferred return types. Each was a two-member union: RuntimeResult<WorkflowLifecycleValue> from the runtimeFailure<WorkflowLifecycleValue>(…) returns, where workflow_run_id was optional, and RuntimeResult<{ run_id; workflow_run_id: string; action; submitted }> from the success return. The type a commandFromRuntime(…, (value) => …) callback saw for value.workflow_run_id depended on which member inference picked, and that changed from one program to the next:

Doctor latency. The temporary-directory check sums the size of every file under every ultrafuzz-controller-* directory, with no bound. A full engine install is about 45,000 entries (about 600 MB). On this host, walking the 100 roots in /tmp (41.4 GiB in total) took 10.8 s with a warm page cache. With 101 roots, an offline diagnoseProject took 15.1 s.

Root cause

  • The lifecycle functions had no declared contract. WorkflowLifecycleValue.workflow_run_id was declared optional, but every success path sets it: resume sets it to the validated Smithers run ID, and replay and fork set it to lifecycleResult.workflowRunId ?? evidence.smithersRunId. So the two union members disagreed about the field, and the answer came from whichever member inference happened to choose.
  • The doctor walk had no budget. Counting the roots takes one readdir of the temporary directory, but sizing them walks every entry under them.

Change

  • packages/runtime/src/types.ts: WorkflowLifecycleValue.workflow_run_id is required.
  • packages/runtime/src/start-run.ts: resumeRun, replayRun and forkRun declare Promise<RuntimeResult<WorkflowLifecycleValue>>. The emitted start-run.js and types.js are byte-identical to origin/main's build. The emitted declarations are now that single type instead of a union.
  • packages/cli/src/commands/resume.ts: removes the two ?? value.run_id fallbacks from fix: reconcile interactions between the merged stability batch and record it in the changelog #1191. Under the declared type the left side is string, and the only producer sets it to the validated Smithers run ID, so the fallback never ran.
  • packages/runtime/src/doctor.ts: sizing controller roots stops after a 1 s budget. The clock is performance.now(), checked before each directory read and each entry, so no directory is opened after the budget runs out. The root count stays exact. When the budget runs out, the summary says … directories hold at least <size>.
  • docs/reference/cli.md: one sentence about the sizing budget.

Deliberately not built

  • No return-type annotations on the private helpers submitSmithersContinuation and submitLifecycleAction. Putting their signature lines in the diff applies the strict per-function budgets to those 218- and 162-line functions, and lint:strict:ci reported 5 errors. The public annotations already check what the helpers return.
  • No JSON schema change. In cli-result.schema.json, lifecycleData still lists workflow_run_id as optional. The output is unchanged and already includes the field on every success. I also left workflow_path? in place. Nothing produces it, but removing it would break TS consumers for no behaviour gain.
  • No entry-count or root-count budget in doctor. A count budget would give the same output on every run, but it would cap scans that would have finished quickly. A time budget bounds the latency directly, and the size stays exact whenever the walk is fast.
  • No edits to replay.ts or fork.ts. Their text is already correct, and with the declared type those lines now pass strict lint when someone next edits them.
  • No cleanup of leftover roots. Doctor still never removes them, because a native resume may still be using one.
  • No truncation flag in doctor. at least follows a clock reading taken after the walk, not a flag set where the walk stops. The clock is monotonic, so every truncated walk gets the label. A walk that finishes within a clock tick of the budget gets it too, and at least is still true there. A flag would add code only to drop a label that is still true.

Verification

Discriminating tests (both in packages/runtime/test/lifecycle-inspection.test.ts). I applied only the test changes to origin/main (2cacf4c) in a separate worktree:

  • resume, replay and fork declare workflow_run_id on every lifecycle value: on origin/main, tsc -p packages/runtime/tsconfig.test.json fails with TS2322: Type 'string | undefined' is not assignable to type 'string' at all three readers. It compiles and passes on the branch. The test covers the required field only. With the three return annotations removed and the field still required, the test program still compiles, so nothing in the test enforces the annotations themselves.
  • diagnoseProject reports a lower bound once sizing many controller roots runs out of time: this test uses 100 roots with a sparse 1 MiB file each, plus a mocked performance.now that advances 100 ms per reading. It also spies on fs.readdirSync and asserts that doctor opens at most one root it does not count. On origin/main it fails with …; 100 ultrafuzz-controller-* directories hold 100 MiB. On the branch it passes (hold at least 4 MiB, 5 roots opened). Both results are the same with the default disk-backed /tmp and with TMPDIR on a tmpfs directory under /dev/shm, where the summary takes its warning form.

Other evidence:

  • Real /tmp on this host (101 roots): an offline diagnoseProject took 15.1 s on origin/main and reported hold 41.9 GiB. On the branch it took 4.5 s and reported hold at least 3.5 GiB. The existing CLI test doctor reports install posture in human and JSON output runs doctor against that same /tmp: it took 117.6 s on origin/main and 37.2 s on the branch.
  • Strict-lint probe: I edited the replay.ts and fork.ts template lines and ran ULTRAFUZZ_STRICT_LINT=1 eslint. On origin/main both report restrict-template-expressions (string | undefined). On the branch the same edit passes.
  • Emitted JavaScript: cmp of packages/runtime/dist/start-run.js and types.js against origin/main's build shows they are identical.
  • Tests run:
    • runtime lifecycle-inspection.test.js with --test-name-pattern='resume, replay and fork declare|diagnoseProject': 18 tests pass, with the default TMPDIR and with TMPDIR on /dev/shm.
    • cli cli.test.js (--test-concurrency=1): run, ps, status, inspect, report, materialize, clean, and lifecycle commands expose product workflow evidence and resume of an already-active run says no controller was started instead of claiming a submission pass.
    • cli lifecycle-commands.test.js: doctor reports install posture in human and JSON output passes.
  • Checks: all of these pass:
    • prettier --check on the changed files
    • pnpm -w lint
    • CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci
    • pnpm --filter @ultrafuzz/runtime typecheck, and the same for @ultrafuzz/cli and @ultrafuzz/dashboard (the dashboard calls all three functions)
    • pnpm -w knip
    • pnpm -w docs:check
  • Complexity ceiling: still 83, which is still the repository maximum (verifyCoverageProductionInventory).
  • Greptile follow-up (1a547b3): after the budget ran out, each remaining root still got one top-level readdir before its first clock check. regularFileBytes now also checks the clock before reading a directory. With the previous doctor.ts, the extended lower-bound test fails with 100 roots opened; … hold at least 9 MiB. On this host's real /tmp (now 140 roots), doctor opened all 140 before the fix, and the last open came about 1 ms after the walk stopped (warm cache). After the fix it opens 9, the last at 920 ms. Doctor took 4.5 s either way. I re-ran these after the fix: the 18 runtime tests (both TMPDIR settings), the CLI doctor test, prettier --check, pnpm -w lint, lint:strict:ci (against origin/main and the merge-base), the runtime typecheck and knip. I did not re-run the cli and dashboard typechecks or docs:check, because the change is inside a private function and touches no docs. The real-/tmp and CLI-test timings above are from before this follow-up.

Risk / compatibility

  • TypeScript consumers of @ultrafuzz/runtime: reading WorkflowLifecycleValue.workflow_run_id now gives string. Code that builds a WorkflowLifecycleValue without that field no longer compiles. Nothing in this repository does: the runtime, CLI and dashboard typechecks pass, and evmbench has its own lenient parser type. The declared return type is assignable anywhere the old inferred union was used.
  • CLI output: unchanged, both JSON and text. The resume text reads the same field, and that field is always set.
  • Doctor: the summary gains at least only when the budget runs out. A capped size depends on machine speed and page-cache state. The check's status and warnings depend only on statfs (tmpfs, free bytes), never on the size. The budget is checked before each directory read and each entry, so it can be overrun only by the syscall in flight when it ends.
  • Test mocking: the new doctor test mocks the global performance.now and spies on fs.readdirSync (the spy calls through) for its duration. In runtime src, the only readers are doctor.ts and an agent template that doctor does not execute.

Changelog entry

ultrafuzz doctor stops sizing leftover ultrafuzz-controller-* directories after about one second and reports their size as at least the bytes counted. On a host with 101 leftover roots, an offline doctor check took 4.5 s instead of 15.1 s. resumeRun, replayRun and forkRun now declare Promise<RuntimeResult<WorkflowLifecycleValue>>, and WorkflowLifecycleValue.workflow_run_id is required, which matches what every successful call returns.

🤖 Generated with Claude Code

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no new actionable issue or outstanding previous finding was identified.

Summary

The PR makes lifecycle result types explicit and limits how long doctor spends sizing leftover controller directories.

  • Resume, replay, and fork now expose a required workflow run ID in their success value.
  • Doctor reports a lower-bound size when its sizing budget expires, with documentation and test coverage.

Reviews (2) · Last reviewed commit: "fix(runtime): doctor opens no controller..."

aviggiano and others added 3 commits September 29, 2026 10:37
resumeRun, replayRun and forkRun had inferred return types: a union of
RuntimeResult<WorkflowLifecycleValue>, whose workflow_run_id was
optional, and RuntimeResult of the success literal, whose
workflow_run_id is a string. Which member a caller's inference picked
varied between programs. Before #1185, strict ESLint typed the resume
CLI callback's workflow_run_id as `string` while tsc typed it
`string | undefined`, and adding one file that mentions
ReturnType<typeof resumeRun> to the CLI program made the unchanged
resume.ts fail to compile (TS2345). #1185 reversed the member order in
the emitted declaration, strict lint then rejected resume.ts, and #1191
added a `?? value.run_id` fallback. replay.ts and fork.ts interpolate an
ID typed `string | undefined` and pass lint only because their lines
are outside recent diffs.

Every success value sets workflow_run_id (resume to the validated
Smithers run ID; replay and fork to the lifecycle result's ID or the
linked run ID), so WorkflowLifecycleValue now declares it required and
the three public functions declare
Promise<RuntimeResult<WorkflowLifecycleValue>>. The emitted JavaScript
for start-run.ts and types.ts is unchanged. The resume fallback cannot
be reached under that type and is removed.

The private helpers keep inferred types: annotating their signatures
puts those long functions under the strict per-function size budgets,
and the public annotations already check what they return.

A lifecycle-inspection test assigns each function's workflow_run_id to
a string; on origin/main it fails to compile with TS2322 for all three.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…econd

The temporary-directory check summed the size of every file under every
ultrafuzz-controller-* directory. A full engine install is about 45,000
entries (about 600 MB). On this host, walking the 100 roots in /tmp
(41.4 GiB) took 10.8 s with a warm page cache, and with 101 roots an
offline diagnoseProject took 15.1 s.

Sizing now stops after a one-second budget (monotonic clock, checked
before each entry). The root count stays exact; when the budget runs
out, the size is reported as "at least" the bytes counted so far. The
same diagnoseProject call took 4.5 s and reported "at least 3.5 GiB".

The new test creates 100 roots and a clock that advances 100 ms per
reading. origin/main reports the exact 100 MiB; with this change the
summary is a lower bound below 100 MiB.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
The new doctor test anchored its summary regex at the end of the line.
When os.tmpdir() is a tmpfs or has less than 2 GiB free, the
temporary-directory summary is the warning text, which continues after
the size, so the test failed on those hosts although doctor behaved
correctly. The regex is now unanchored, like the exact-size sibling
test. Against origin/main's doctor it still fails ("hold 100 MiB") on
both a disk-backed /tmp and a tmpfs TMPDIR.

The compile-time lifecycle test's comment now describes only what it
checks: that the returned value types workflow_run_id as a string.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aviggiano
aviggiano requested a review from a team as a code owner September 29, 2026 13:45
Comment thread packages/runtime/src/doctor.ts
After the one-second sizing budget ran out, doctor still called
regularFileBytes for every remaining ultrafuzz-controller-* root, and each
call read the root's top-level directory before its first clock check. The
budget bounded the recursive walk but not that tail. With 140 leftover roots
in this host's /tmp, doctor opened all 140, 131 of them after the budget.
That cost about 1 ms with a warm cache, but it grows with the number of roots,
which doctor never removes, and with directory cache misses.

regularFileBytes now also checks the clock before reading a directory, so no
directory is opened after the deadline. Remaining roots cost one clock
reading each. The "at least" label is unchanged: it still comes from a clock
reading taken after the walk, and performance.now() is monotonic, so every
truncated walk is labelled.

The lower-bound test now spies on fs.readdirSync and asserts that doctor
opens at most one root that it does not count. Without the fix, doctor opens
all 100 roots and counts 9 of them.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aviggiano
aviggiano merged commit 73e70f3 into main Sep 29, 2026
17 checks passed
@aviggiano
aviggiano deleted the claude/x05-typed-resume-result branch September 29, 2026 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant