fix(runtime): the Smithers supervisor can relaunch a crashed engine - #1199
Merged
Merged
Conversation
…ntinuations `ultrafuzz resume` runs every Smithers command with the target repository as its working directory. A native continuation has no sealed Bun startup controls, so the runner was started with no Bun flags at all: Bun read the target's bunfig.toml, ran its `preload` list inside the controller process, and loaded the target's .env, whose variables the detached engine then inherited through its environment. A Bun runner without startup controls now gets `--config=/dev/null --no-env-file --no-install --no-addons`: Bun reads no bunfig.toml, loads no .env, never auto-installs and loads no native addons, which the sealed startup controls already guarantee for sealed runs. Co-Authored-By: Claude Opus 5.5 <[email protected]>
When a detached engine died (SIGKILL, OOM), the Smithers supervisor that
Ultrafuzz starts beside it could not finish the run:
- The resume-detached patch always passed the /proc/self/fd/3 Bun startup
flags, although only a process that inherited the snapshot descriptor has
that path. A supervisor holding none (every native continuation) relaunched
into "ENOENT ... /proc/self/fd/3/controls/bunfig.toml" three times, then
marked the run failed with AUTO_RESUME_GAVE_UP.
- Upstream relaunches a workflow file from dirname(workflowPath). For a
sealed launch that directory is inside the read-only execution snapshot,
so Smithers could not open the relaunch's detached log there ("detached
log is unavailable"): the supervisor retried every poll without ever
relaunching, and the run stayed orphaned. For a plain-path engine the
workflow opened its database relative to that directory, so with the
target outside $HOME (no `.smithers` anchor) the relaunch opened a new,
empty database and failed with RUN_NOT_FOUND. The generated workflow also
resolves its task paths from the working directory.
- The relaunch dropped --log-dir, so the relaunched engine wrote its events
to <root>/.smithers/executions/<id>/logs instead of the run's log
directory.
All through the compatibility patch registry:
- resume_snapshot_transfer passes the startup flags only when a descriptor
is inherited, and the Bun guard flags otherwise.
- New supervisor_resume_root: a workflow-file relaunch starts in the rootDir
the engine persisted in the run's config, read as `up --resume` reads it.
- New resume_log_dir: supervisor_descriptor hands the launch's --log-dir to
the supervisor as ULTRAFUZZ_SMITHERS_LOG_DIR, and the relaunch argv passes
it on.
- The detached engine and supervisor spawns pass the Bun guard flags when no
snapshot is transferred, so a target bunfig.toml preload or .env never
runs in them.
The new real-engine test SIGKILLs an engine mid-task in a target outside
$HOME. The first supervisor relaunch must finish the run from the launch
root, append to the configured log directory, and never load the target's
bunfig.toml or .env. On the parent commit the run is marked failed after
three relaunches, and removing any one of the four patch changes fails it.
A real sealed `ultrafuzz run` recovered from two consecutive engine SIGKILLs
the same way and finished.
Refs: #921
Co-Authored-By: Claude Opus 5.5 <[email protected]>
… target A supervisor relaunch now starts in the run's root, so Smithers' resumeRunDetached writes the relaunch's own output to <target>/.smithers/logs/<run-id>.log. The file is untracked in the target, so the governed target identity of the next campaign on that target read the worktree as dirty, which a private campaign refuses (DATA_GOVERNANCE_PRIVATE_TARGET_UNBOUND). .smithers/logs joins the controller-owned paths the governed identity excludes, beside the engine's smithers.db. Co-Authored-By: Claude Opus 5.5 <[email protected]>
The supervisor_descriptor patch reads options.logDir, which upstream's `up -d` scope binds. The two runtime.test.ts harnesses that execute that patch text did not bind it, so both failed with "ReferenceError: options is not defined". Both now declare options. The Bun (fd-3) harness also asserts that the sealed supervisor inherits ULTRAFUZZ_SMITHERS_LOG_DIR from the launch's --log-dir. Nothing else checks that line on the sealed spawn chain. The relaunch test: - runs at production's 30 s stale threshold, so asserting one relaunch keeps its margin under load; - waits long enough to see the supervisor give up, so a regression reports RunFailed rather than a timeout; - kills leftover processes by the regex-escaped fixture root. The old pkill pattern held regex `+` quantifiers and named a path the detached engine and supervisor never run. The relaunch and resume-reopen tests now share one patched-runner helper. It also copies the store's hoisted node_modules, so each Smithers module loads once, from the patched copy. Before, engine, scheduler and db also loaded an unpatched second instance from the store. The Bun guard flags now live in one exported list, which the patch texts, the native continuation and the test all use. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Comment on lines
+45
to
+59
| const runner = patchedSmithersRunner(path.join(root, "runner")); | ||
| const runId = `supervisor-relaunch-${process.pid}`; | ||
| const cli = [...BUN_TARGET_CONFIGURATION_GUARD_ARGS, path.join(runner, "src", "bin", "smithers.js")]; | ||
| const smithers = (...args: string[]): string => { | ||
| const result = spawnSync("bun", [...cli, ...args], { | ||
| cwd: target, | ||
| encoding: "utf8", | ||
| env: { | ||
| HOME: home, | ||
| PATH: process.env.PATH, | ||
| ...(process.env.TMPDIR === undefined ? {} : { TMPDIR: process.env.TMPDIR }), | ||
| NODE_PATH: path.dirname(runner), | ||
| SMITHERS_MONITOR_SUPPRESS: "1", | ||
| SMITHERS_POST_FAILURE: "0", | ||
| ULTRAFUZZ_WORKFLOW_PERSISTED_PATH: workflow |
There was a problem hiding this comment.
Sealed relaunch lacks coverage The new real-engine test starts from plain paths without a snapshot descriptor. It therefore cannot catch a regression where a sealed launch fails to resume after the engine crashes. The descriptor test checks inheritance, but not whether the run finishes. An automated sealed-launch crash-and-relaunch test would cover that gap.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/test/smithers-supervisor-relaunch.integration.test.ts
Line: 45-59
Comment:
**Sealed relaunch lacks coverage** The new real-engine test starts from plain paths without a snapshot descriptor. It therefore cannot catch a regression where a sealed launch fails to resume after the engine crashes. The descriptor test checks inheritance, but not whether the run finishes. An automated sealed-launch crash-and-relaunch test would cover that gap.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Every Smithers engine Ultrafuzz submits runs with
--supervise, so when the detached engine dies (SIGKILL, OOM kill) the Smithers supervisor should relaunch it and the campaign should continue. Onorigin/integration/wave1the relaunch never finishes the run:ultrafuzz resumeruns it: the supervisor relaunches three times, each relaunch dies before the engine activates, and the supervisor then marks the runfailed(AUTO_RESUME_GAVE_UP). An engine crash that leaves the supervisor alive, such as an OOM kill of the engine alone, therefore turns into a failed run that only an operator can recover. The new real-engine test reproduces this on the base:RunStarted, NodeStarted, RunAutoResumed ×3, RunAutoResumeSkipped, RunFailed.ultrafuzz run): the relaunch starts in<snapshot>/.smithers/workflows, and Smithers puts the relaunch's detached log below that directory, inside the read-only snapshot. Every poll fails withCannot resume run …: detached log is unavailable at <snapshot>/.smithers/workflows/.smithers/logs/<id>.log. The failed spawn releases its claim, so the supervisor never relaunches, never gives up, and never emitsRunAutoResumed. I reproduced this with a realultrafuzz runon the base (manual experiment): 70 identical failures in the 12 minutes after the SIGKILL.ultrafuzz statusthen reportedstatus: running,verdict: orphanedwith 4 of 9 tasks finished, so the campaign stays stopped until an operator runsultrafuzz resume.Separately, Bun reads
bunfig.toml(and runs itspreloadlist) and.envfrom its working directory, which for every controller process is the target repository. The native-continuation command, and the patched detached-engine and supervisor spawns when they carry no snapshot descriptor, started Bun with no flags. A target'sbunfig.tomlpreload therefore ran inside the controller processes, and its.envvariables reached the engine. A probe on the base recorded the preload running in the top-level command, the detached engine and the supervisor, and the engine's task seeing the target's.envvalue.Root cause
Three defects in the relaunch path, all in code Ultrafuzz patches or should patch:
resume_snapshot_transferpatch (@smthrs/cli/src/resume-detached.js) always passed the/proc/self/fd/3/controls/...Bun startup flags. Only a process that inherited the snapshot descriptor has that path. A supervisor holding none relaunched intoENOENT: No such file or directory (open()) while reading config "/proc/self/fd/3/controls/bunfig.toml".resolveResumeTargetrelaunches a workflow file withcwd: dirname(workflowPath)(resume-target.js:99) and passes no--root. The workflow opens its store from its working directory (createSmithers:findSmithersAnchorDir(process.cwd()),smthrs/src/create.js:442). That anchor walk stops at$HOME(findSmithersAnchorDir.js:25), so with the target outside$HOMEthe relaunch created a new, emptysmithers.dbbeside the workflow and failed withRUN_NOT_FOUND. The generated workflow also resolves its task paths fromprocess.cwd()(workflow.tsx, e.g.dynamicRunRoot), so the working directory matters inside$HOMEtoo. For a sealed launch, that directory is inside the read-only snapshot, and upstreamresumeRunDetachedderives the relaunch's detached log path from it (resolveDetachedRunLogFile(runId, { cwd })), so the relaunch can't even open its log.--log-dir.buildResumeArgs(resume-detached.js:40) rebuilds the relaunch argv without--log-dir. The relaunched engine then falls back to<root>/.smithers/executions/<id>/logs(@smthrs/engine/src/engine.js:2889), and<run>/smithers/logs/stream.ndjsonstops receiving events, including theNodeFailedpayloads (the comment inrunSmithersLifecycleCommandrecords why that matters).The Bun exposure comes from patched spawns passing
[]when no descriptor is transferred, and fromacquireSmithersExecutableAnchorpassing no interpreter arguments for an unsealed native continuation.Change
All runner changes go through the existing
SMITHERS_COMPATIBILITY_PATCHESregistry, and every new or rewritten text is covered by the existing anchor, helper-scope, patcher and doctor-posture tests.BUN_TARGET_CONFIGURATION_GUARD_ARGS(smithers-executable-capability.ts):--config=/dev/null --no-env-file --no-install --no-addons. Bun then reads nobunfig.toml, loads no.env, never auto-installs and loads no native addons, which the sealed startup controls already guarantee for sealed runs. This is the only copy of the flag list: the patch texts embed it (SMITHERS_BUN_GUARD_ARGSinsmithers.ts), and the native-continuation branch and the new test import it.detached_snapshot_transferandsupervisor_descriptor: the no-transfer branch passes the guard instead of nothing.resume_snapshot_transfer: passes the descriptor-rooted startup flags only when a descriptor is inherited, and the guard otherwise (fixes 1).supervisor_resume_root(src/supervisor.js,resolveResumeTargetEffect): a workflow-file relaunch starts in therootDirthe engine persisted in the run'sconfig_json, read the wayparsePersistedRootDirreads it forup --resume(fixes 2). Because the patch sits in the supervisor's shared target resolution, the stale-run, timer, approval and quota relaunch paths all use it. Without a persisted root it keeps upstream's target.resume_log_dir(src/resume-detached.js,buildResumeArgs): the relaunch argv appends--log-dir $ULTRAFUZZ_SMITHERS_LOG_DIR.supervisor_descriptorsets that variable in the supervisor's environment from the launch's own--log-dir, so no Ultrafuzz environment plumbing is needed (fixes 3).applySmithersCompatibilityPatchesapplies the two new patches and requiressrc/supervisor.js.acquireSmithersExecutableAnchor: a Bun runner without startup controls (only a native continuation reaches that branch) gets the same guard flags.data-governance.ts:.smithers/logsjoinscontrollerOwnedGovernancePaths. A relaunch now starts in the run's root, so upstreamresumeRunDetachedwrites the relaunch's own output to<target>/.smithers/logs/<run-id>.log. That file is untracked in the target, so the next campaign's governed target identity would read the worktree as dirty, which a private campaign refuses (DATA_GOVERNANCE_PRIVATE_TARGET_UNBOUND).Source: +114/-21 lines, most of it patch text and comments. Tests: +295/-34, including a shared
test/patched-smithers-runner.tsthat replaces the resume-reopen test's private copy of the same helper.Deliberately not built
--rooton the relaunch. With the working directory set to the persistedrootDir,up --resumealready reuses that root for the task root. The store anchor and the workflow's own paths depend on the working directory, which--rootwould not change.manifest_relaunch). That relaunchchdirs into the@smthrs/clipackage before re-executing, and the logical workspace is restored without a realchdir, so Bun never reads the target'sbunfig.tomlor.envthere. I checked this with a jj-conflictedpackage.jsonplus a hostilebunfig.toml/.envon the base patches: the relaunch ran and no preload executed. A change there would be unobservable.refreshedSmithersControllerSnapshot, which has no production caller (v03 deletes it).BUN_OPTIONSguard. Detached children would inherit it, including agent CLIs and Bun-based target test commands. Explicit flags on the patched spawns do not leak.patchedDependencies(v10).--log-dirand supervisor-descriptor fixes to Smithers would shrink this patch set; that is outside this repository.Verification
New real-engine test.
packages/runtime/test/smithers-supervisor-relaunch.integration.test.tsuses the pinned Smithers 0.35.0 with every registry patch applied. The runner comes fromtest/patched-smithers-runner.ts, whichsmithers-resume-reopen.integration.test.tsnow uses too. It copies every Smithers package and the store's hoistednode_modules, so each Smithers module loads once, from the copy. The store itself is never written. Anstraceof a toysmithers upon such a copy opened each of the 12 patched modules once, from the copy, and no Smithers module from the store. Without the hoistednode_modulescopy, patched engine, scheduler and db modules also loaded a second, unpatched instance from the store. This was seen withstraceboth in review, on the first revision's helper, and again on the reopen test's helper. The setup:$HOME;bunfig.tomlpreload and.env;up --detach --supervisewith a 1 s supervisor interval and a 30 s stale threshold, production'scontroller_lease_secondsdefault.The test SIGKILLs the engine mid-task. It asserts:
RunAutoResumedfires exactly once (resumeAttempt1);cwdequal to the launch root;<logDir>/stream.ndjsonhas twoRunStartedand ends inRunFinished, and no<target>/.smithers/executionsexists;.envvalue.Results:
origin/integration/wave1, with this test and helper copied in: fails in 125 s (RunAutoResumed ×3, RunAutoResumeSkipped, RunFailed; statusfailed). The wait for the run's end allows 180 s, so a regression reports the supervisor's give-up events instead of timing out.pkill -fon the regex-escaped root). The detached engine and supervisor run<root>/runner/@smthrs+cli@…/src/index.js, which the first revision's pattern (thesmthrsrunner path, whose+characters are regex quantifiers) never matched.Ablations. Each run removes one change from the compiled registry, and each fails the final test. The log diagnoses in parentheses were recorded against the first revision of the test.
/proc/self/fd/3RunAutoResumed ×3, RunAutoResumeSkipped, RunFailed(three relaunch logs withENOENT)supervisor_resume_rootRunAutoResumed ×3, RunAutoResumeSkipped, RunFailed(every relaunch log saysRUN_NOT_FOUND, straysmithers.dbin the workflow's directory)resume_log_dirRunStarted(the relaunched engine logs to<target>/.smithers/executions/<id>/logs/stream.ndjson).envOther new tests (each passes with the change and fails on the base):
smithers-executable-capability.test.ts, "native operator continuations run Bun without the target repository's bunfig.toml or .env". On the base,dotenv: 'loaded'.data-governance.test.ts, "a supervisor relaunch's log in the target leaves its governed identity unchanged". On the base,dirty: true.runtime.test.ts, "fixed fd transfer survives parent exit and fd reuse across Bun engine, supervisor, and resume" now also asserts that the sealed (fd-3) supervisor inheritsULTRAFUZZ_SMITHERS_LOG_DIRfrom the launch's--log-dir. This is the only automated check of that line on the sealed spawn chain. With thesupervisor_descriptorenvironment line removed, it fails withactual: undefined.Sealed launch mode (manual experiment on the first revision). The later revision changes nothing on this path; it only moves the guard flag list into one constant. I ran a real
ultrafuzz runwith the CLI e2e's three-agent topology, a stubcodexandcontroller_lease_seconds = 30, so the engine ran from a sealed execution snapshot through the held descriptor:summarizewas held. After the relaunch, the relaunched engine was SIGKILLed again whilesummarizewas still held.Resuming stale run … attempt 1twice).summarizeran three times and the run endedRunFinished.ultrafuzz statusreportedsucceededwith 9/9 tasks finished.<run>/smithers/logs/stream.ndjsonholds all threeRunStarted. No.smithers/executionsdirectory and no straysmithers.dbexist. The relaunch launcher's log is<target>/.smithers/logs/<id>.log, hence the governance change.The same experiment on the base is the sealed-launch failure described under Problem.
CLI campaign e2e.
packages/cli/test/e2e/campaign-resume.test.tslaunches a campaign, SIGKILLs engine and supervisor, then resumes it through the native-continuation path, which gets the new Bun guard. It passes on the final revision in 296 s (load average 4–10), and its existing #1187todostill reports as a todo. On the first revision it passed in 274 s at a load average of 12–16. An earlier attempt there, at a load average of about 45, failed before the resume step: anultrafuzz statuscall printed nothing 350 s after the controller kill, which fits the helper's 5-minute kill timeout.statuson a sealed run executes none of the changed code paths.Existing tests:
runtime.test.ts, the two harnesses that execute thesupervisor_descriptorpatch text: "the patched runner admits engine and supervisor process-owned execution snapshot descriptors" (Node) and "fixed fd transfer survives parent exit and fd reuse across Bun engine, supervisor, and resume" (Bun). The first revision broke both withReferenceError: options is not defined: the patch readsoptions.logDir, which upstream'sup -dscope binds and the harness scopes did not. Both harnesses now declareoptions, and both pass.runtime.test.tspatch tests, all passing:runtime.test.tsnative-continuation and native-resume tests: 9 pass. "native continuation hands generated agents the run's TOML config, so CodexAgent keeps API-key auth" fails as it does on the base (fixtureERR_MODULE_NOT_FOUND, fixed by fix: reconcile interactions between the merged stability batch and record it in the changelog #1191).lifecycle-inspection.test.ts: diagnoseProject reports a posture for every tracked compatibility patch.smithers-controller-engine.integration.test.ts(2 tests), which loads every registry patch into the engine.smithers-resume-reopen.integration.test.ts(3 tests), now on the shared runner helper.smithers-executable-capability.test.tsanddata-governance.test.ts: every test passes.Gates:
pnpm -w format:checknpx prettier --checkandnpx eslinton the changed filesCI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/integration/wave1 pnpm -w lint:strict:cipnpm -w lintpnpm --filter @ultrafuzz/runtime typecheckpnpm -w knippnpm -w docs:checkRisk / compatibility
ultrafuzz resumeinstalls a fresh controller and gets the fix.resume_snapshot_transfer,detached_snapshot_transferandsupervisor_descriptorwould classify as incompatible. Only the deadrefreshedSmithersControllerSnapshotre-classifies such sources.config_json.rootDir, which@smthrs/enginewrites. If a future Smithers stops persisting it, the patch falls back to upstream's target.upstreamAbsent: ["rootDir", "parsePersistedRootDir"]flags a Smithers release whose supervisor starts reading the root itself.controller_lease_seconds, default 30 s). If it doesn't, the supervisor claims another attempt and gives up after three. That is unchanged upstream behaviour, and it now matters because relaunches can succeed. In the sealed experiment, run at the 30 s lease on a host with load average above 30, both relaunches activated on their first attempt. The new test uses the same 30 s threshold. A sealed run whose relaunch misses activation three times now endsfailed(AUTO_RESUME_GAVE_UP) instead of staying orphaned. Both states needultrafuzz resume, and the engine accepts afailedrun for resume (RESUMABLE_RUN_STATUSES,@smthrs/engine/src/engine.js:3434).ULTRAFUZZ_SMITHERS_LOG_DIR: the run's log directory path, not a credential..smithers/logsno longer count toward its governed identity. Tracked files there still do.--no-install --no-addons, flags sealed runs already use.Changelog entry
When the workflow engine dies mid-run, the Smithers supervisor now relaunches it and the run continues. Before, every relaunch failed: a freshly launched run stayed orphaned until an operator resumed it, and a resumed run was marked failed after three attempts. Relaunched engines keep writing events to the run's log directory, and controller Bun processes no longer load a target repository's
bunfig.toml(or its preload scripts) or.env.Refs #921
🤖 Generated with Claude Code
The PR appears safe to merge, with a non-blocking gap in automated sealed-launch relaunch coverage.
Fix with agent prompt
Summary
The PR patches Smithers’ supervisor relaunch to restore the persisted working directory and run log directory, and adds Bun startup guards for controller processes without snapshot controls.
Reviews (1) · Last reviewed commit: "test(runtime): the patch harnesses and r..."