fix(runtime): resume keeps agent auth config and a no-op attach no longer rewrites run state - #1177
Merged
Merged
Conversation
Native continuation set ULTRAFUZZ_CONFIG_PATH to <run>/smithers/resolved-config.json. Every stock agent adapter reads that path with its TOML reader, which finds no [agents.*] tables in JSON and returns an empty config, so each adapter fell back to its defaults on resume: auth, api_key_env and config_dir were silently dropped. With the default ultrafuzz.toml (CodexAgent auth = "api-key"), every resumed Codex task ran with subscription auth and a cleared OPENAI_API_KEY. Launch hands adapters a sealed copy of <run>/smithers/execution-config.toml, the TOML rendering of the same resolved config. Resume now points them at that file. It is written at compile time since #635, and resume only sets the variable when resolved-config.json parses as the current v4 schema (#1120), so every run that reaches this branch was compiled with it. The new regression test resumes a run, rebuilds the generated CodexAgent from the config path the runner received, and checks it still carries the API key. It fails on main (CODEX_API_KEY is empty) and passes with this change. "native resume delegates the persisted workflow after mutable project sources are replaced" asserted the JSON path, i.e. the bug; it now asserts the TOML path. Co-Authored-By: Claude Opus 5.5 <[email protected]>
`resume` treats a run whose derived Smithers state is still active as an idempotent attach and starts no controller. That includes a run still draining a pause and, for up to the 30 s heartbeat window, a run whose controller just died. submitSmithersContinuation nevertheless called recordNativeContinuationState, which rewrote state.json and moved workflow_deadline_at forward on every such no-op, and the CLI printed "Submitted resume" regardless of `submitted`. Only record continuation state when a controller was actually started, and drop the now-unused alreadyRunning parameter and branch. The CLI now prints "Run already active: <id>; no new controller was started" when nothing was submitted, with the hint to resume again once a draining pause has parked. JSON output is unchanged. #1153 asked for an Ultrafuzz-side controller-generation fence. The pinned engine already provides the guarantee, so this pins it instead of re-implementing it. A real-Smithers integration test shows that: - a graceful pause lets held in-flight tasks finish; - while the owner drains, `up --resume --force` and `timetravel --force` are refused with RUN_OWNER_ALIVE, and inspect still reports the run state as `running`, so `ultrafuzz resume` only attaches; - a resume after the park runs only the remaining task, in a new controller. Swapping --force for --steal-ownership in that test makes it fail, so it detects a takeover. The state test fails on main (workflow_deadline_at moves on a submitted:false resume) and the CLI test fails on main ("Submitted resume: ..."); both pass with this change. The pin test passes on both by design. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…r failing on worktree cleanup Two resume-time preparation steps were wrong in opposite directions. When prepareTrustedCliEnvironment threw on resume, for example because the run was launched under a different Node binary and the launcher bytes no longer match a fresh render, the error was swallowed and the trusted CLI variables were cleared. composeSmithersCommandPath then left <run>/trusted-bin off the engine PATH, so every task's validator preflight ran whatever `ultrafuzz` the operator's PATH held (or hit ENOENT). Keep ULTRAFUZZ_TRUSTED_BIN pointed at the run-owned launcher when it exists: the launcher re-verifies its closure on every dispatch. The failure is now reported as a WORKFLOW_TRUSTED_CLI_UNVERIFIED warning. repairPrunableRunWorktreeRegistrations is cleanup, but a failed `git worktree remove` (or an unreadable workspace path) threw out of resume and failed the whole continuation. It now yields a WORKFLOW_WORKTREE_REPAIR_FAILED warning and the continuation proceeds. Resume returns these warnings as diagnostics (also appended after the error on a failed resume), and the CLI prints them in text mode. Both tests fail on main (the runner PATH starts with the caller's PATH instead of trusted-bin; the resume fails with WORKFLOW_LIFECYCLE_FAILED) and pass with this change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Comment on lines
+608
to
+611
| const launcherKept = fs.existsSync( | ||
| path.join(trustedBin, process.platform === "win32" ? "ultrafuzz.cmd" : "ultrafuzz") | ||
| ); | ||
| if (launcherKept) trustedCli.env[ULTRAFUZZ_TRUSTED_BIN_ENV] = trustedBin; |
There was a problem hiding this comment.
Broken launcher shadows working CLI If a Node upgrade removes the executable embedded in the run-owned launcher, the launcher file still passes this existence check and takes first place on
PATH. Resume reports success with a warning, but schema-backed tasks cannot run their ultrafuzz validation commands, even if another working CLI is on PATH. This case needs a recovery or explicit rejection path rather than treating file existence as proof that the launcher works.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/start-run.ts
Line: 608-611
Comment:
**Broken launcher shadows working CLI** If a Node upgrade removes the executable embedded in the run-owned launcher, the launcher file still passes this existence check and takes first place on `PATH`. Resume reports success with a warning, but schema-backed tasks cannot run their `ultrafuzz` validation commands, even if another working CLI is on `PATH`. This case needs a recovery or explicit rejection path rather than treating file existence as proof that the launcher works.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…esume
When resume cannot re-verify the trusted CLI it keeps <run>/trusted-bin
first on PATH. The WORKFLOW_TRUSTED_CLI_UNVERIFIED warning described that
launcher as a working fallback ("checks itself on every call"). That is
only true when the launcher itself is intact. With damaged metadata, a
changed closure, or a removed Node binary, the launcher refuses or fails
to exec. Then every task's preflight-json-validator step fails while
resume reports ok. The warning now says that tasks still call the named
launcher and that their validator preflight fails while it cannot verify
itself. When no launcher exists, it says tasks use whatever `ultrafuzz`
is on PATH.
I checked the damaged case by running the launcher after the existing
test corrupts trusted-cli.json and resumes twice. It exits 1, both resumes
return ok, and each carries the new warning.
The existing #1143 test drove the catch branch only through that
corrupted metadata. In that state keeping the launcher cannot help, so
its PATH assertion checked the mechanism, not the outcome. It keeps only
the warning assertion. A new test covers the case the change exists for:
resume without a CLI entrypoint, so preparation throws while the launcher
is intact. It resolves `ultrafuzz` through the PATH the runner received,
and that must be the run's launcher. It then runs the task's `json
validate` call through it and parses the success envelope. On main the
lookup finds no `ultrafuzz` at all (undefined instead of the run's
trusted-bin/ultrafuzz); with this change it passes.
docs/schemas.md now states the existence condition and the failure
modes, and that --refresh-controller does not repair such a launcher.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
… exited An attach happens in two cases. One is a pause that is still draining. The other is a controller process that exited less than 30 seconds ago. In the pinned Smithers, deriveRunState keeps a run `running` until its heartbeat is 30 s stale. `ultrafuzz status` also maps the later `stale`/`orphaned` states to `running`. So "resume again once status reports paused" never fires in the second case, and an operator who follows it waits indefinitely. The message now adds: "if its controller process just exited, resume again after 30 seconds". After that window the run is no longer active and resume submits a continuation. Three statements were wider than the code: - docs/reference/cli.md said adapters read the launch config unconditionally. They do so only when the run's resolved-config.json parses as the current schema; otherwise resume sets no ULTRAFUZZ_CONFIG_PATH. - The start-run comment said the state "belongs to the live owner", but in the second case no owner is alive. - The CHANGELOG entry carried the same unconditional claims. All three now match the code. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ting their root The guard's finally block wrote the release marker and then deleted the temp root in the same tick, so the marker was deleted too. The held tasks poll every 50 ms and almost never saw it. After any assertion failure the detached owner kept running until its 120 s hold expired and then wrote its logs back under the deleted root. Passing runs were not affected, because by then every task had finished. Teardown now writes the marker and then waits, bounded at 30 s, until every process that traced a task has exited before it deletes the root. Checked with a copy of the test where the takeover uses --steal-ownership instead of --force. The test fails as intended (DETACHED_ADMISSION_FAILED / RUN_RESUME_CLAIM_FAILED instead of RUN_OWNER_ALIVE). The owner engine had exited by the time the run returned. Two and a half minutes later, past the 120 s hold, the root had not reappeared. The unmodified file still passes, all 4 tests. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every pull request in this batch inserts its entry at the same place in CHANGELOG.md, so each merge would conflict with the next. The entries are collected into one changelog update instead. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Sep 29, 2026
This was referenced Sep 29, 2026
Merged
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
ultrafuzz resume(submitSmithersContinuationinpackages/runtime/src/start-run.ts) had four problems:Resume lost the agent auth config. Native continuation set
ULTRAFUZZ_CONFIG_PATH=<run>/smithers/resolved-config.json. All seven stock adapters (codex,claude,kimi,pi,opencode,openrouter,deepseek) read that path with the TOML reader inagents/toml.tsx. On JSON that reader finds no[agents.*]table and returns{}, so every adapter fell back to its defaults and silently droppedauth,api_key_envandconfig_dir. With the defaultultrafuzz.toml([agents.CodexAgent] auth = "api-key"), resumed Codex tasks were built with subscription auth and an emptyOPENAI_API_KEY/CODEX_API_KEY.A resume that started nothing still rewrote run state (Make pause, hijack, and continuation handoff atomic #1153). When Smithers reports the run as active,
resumeonly attaches (alreadyRunning). That covers two cases:pause;deriveRunStatekeeps a runrunninguntil its heartbeat isRUN_STATE_HEARTBEAT_STALE_MS(30 000 ms) stale.recordNativeContinuationStatestill ran on those attaches. It rewrotestate.jsonand pushedworkflow_deadline_atforward, silently extending the deadline that workflow_deadline_seconds is only enforced while an operator runs a CLI command; unattended runs have no wall clock #1110/docs: state that workflow_deadline_seconds is only enforced on sync #1157 document. The CLI printedSubmitted resume: <id>whether or not anything was submitted.A failed trusted-CLI check on resume dropped the run's launcher (Pin required executables and runtime dependencies for detached runs #1143, part 2). When
prepareTrustedCliEnvironmentthrew, the error was swallowed and every trusted-CLI variable was cleared. One way it throws is a run launched under a different Node binary path, because the launcher bytes then differ from a fresh render.composeSmithersCommandPaththen left<run>/trusted-binoff the enginePATH. Every task'spreflight-json-validatorstep then ran whateverultrafuzzwas on the operator'sPATH, or hit ENOENT.Stale-worktree cleanup could fail the whole resume. A failed
git worktree removeinrepairPrunableRunWorktreeRegistrationsthrew out of resume. So did a non-ENOENTlstaterror.Root cause
<run>/smithers/execution-config.toml(the execution snapshot'scontrols/ultrafuzz.toml). That file is the TOML rendering of the same resolved config (serializeResolvedConfigToml).alreadyRunningintorecordNativeContinuationState. That flag skipped only the lease update; status and deadline were still rewritten.resume.tsignoredsubmitted.Change
Config path. Resume points
ULTRAFUZZ_CONFIG_PATHat<run>/smithers/execution-config.toml. It sets the variable only whenresolved-config.jsonparses as the currentultrafuzz.resolved-config.v4schema (feat(runtime): default to best-effort runs with agent-written PARTIAL reports #1120, 2026-09-09). The only writer of that JSON (compileSmithersWorkflow) writes the TOML next to it, and has done so since fix(security): complete remaining high-priority AppSec controls #635 (2026-08-18). So every run that reaches this branch has the TOML. Runs whose resolved config does not parse get noULTRAFUZZ_CONFIG_PATH, as before.No-op attach.
recordNativeContinuationStateruns only whenresult.alreadyRunning !== true. Its now-unusedalreadyRunningparameter and branch are deleted. When nothing was submitted,ultrafuzz resumeprints:Run already active: <id>; no new controller was started. If a pause is still draining, resume again once status reports paused; if its controller process just exited, resume again after 30 seconds.The second clause is needed because
statusmaps Smithersstale/orphanedtorunning, so it never reportspausedfor a dead controller. After the 30 s window the run is no longer active and resume submits a continuation. JSON output is unchanged; it already carriedsubmitted: false.Trusted launcher. When trusted-CLI preparation throws and
<run>/trusted-bin/ultrafuzzexists,ULTRAFUZZ_TRUSTED_BINkeeps pointing at<run>/trusted-bin. Resume never substitutes an ambientultrafuzzfor it. The failure becomes aWORKFLOW_TRUSTED_CLI_UNVERIFIEDwarning, and the warning says what follows:… (tasks still call <run>/trusted-bin/ultrafuzz, and their preflight-json-validator step fails while that launcher cannot verify itself): <error>.… (<path> does not exist, so tasks call whateverultrafuzzis on PATH): <error>.Worktree repair. The worktree-registration repair is wrapped in a try/catch. A failure becomes a
WORKFLOW_WORKTREE_REPAIR_FAILEDwarning.Warnings are returned. Resume returns its warnings as diagnostics; on a failed resume they follow the error. The CLI prints them in text mode, as
initdoes.Docs.
docs/reference/cli.md: the config sentence is scoped to runs with a current resolved config, and the attach paragraph now covers the 30 s window.docs/schemas.md: the existence condition, the failure modes of a kept launcher, and that--refresh-controllerdoes not repair one.docs/reference/configuration.md: one sentence; the deadline is re-recorded only by a resume that starts a controller.Source change:
start-run.ts+72/−31, much of it re-indentation of the unwrapped lease block, andresume.ts+9/−5. The rest is tests and docs.Deliberately not built (and why)
No controller-generation fence, owner-fenced pause, or blocking pause (Make pause, hijack, and continuation handoff atomic #1153's proposal). The pinned engine already refuses a second controller while the owner is alive:
up --resume --forceandtimetravel --forcefail withRUN_OWNER_ALIVE;--forcedoes not bypass this;--steal-ownershipdoes, and nothing in this repo passes it (git grep);This PR pins those facts with a real-Smithers integration test instead of re-implementing them.
No probe of, fallback from, or rejection over the kept launcher. Greptile P1 Improve agent tools #2 and both reviews flagged that file existence does not prove the launcher works. That is correct, and the warning and docs now say so. I kept the spec'd behaviour:
ultrafuzzis what Pin required executables and runtime dependencies for detached runs #1143 removes.No change to
trusted-cli.ts. This is the root cause of the unrecoverable cases and is a follow-up. Resume cannot re-render a launcher whose embedded interpreter path changed.prepareTrustedCliEnvironmentthrows "trusted Ultrafuzz CLI launcher changed" before it reaches rotation, even under--refresh-controller. Fixing it means either re-rendering for the current interpreter on refresh, or dropping the interpreter from the launcher identity. Both change launcher identity handling for in-flight runs, so neither belongs in this PR.No sealed-copy indirection for the agent config (Greptile P1 Cleanup whitelisted commands #1: "read
controls/ultrafuzz.tomlin the execution snapshot, or verify the live bytes"). Main already handed resumed adapters the equally mutable<run>/smithers/resolved-config.json, so this is not a regression. Resume also takes credential names, lease and deadline from that same mutable file. And by design it runs the mutable persisted workflow (native resume delegates the persisted workflow after mutable project sources are replaced). A verification layer is the kind of fail-closed machinery this effort is removing.No regeneration of a missing
execution-config.toml. By the argument above, every run that reaches this branch has it. If someone deletes it, adapters now fail loudly (ENOENT on the named path) instead of silently running on default auth.No change to the pre-resume
inspectgate. One option is to always pass--forceand mapRUN_OWNER_ALIVEtoalreadyRunning, which would close the 30 s window. That changes lifecycle behaviour, so the CLI message documents the window instead.No attempt-ledger work. The pre-reset preservation hook is untouched; w02 owns it.
Verification
Discriminating tests
Each fails on
origin/mainand passes with this change:runtime.test.ts"native continuation hands generated agents the run's TOML config, so CodexAgent keeps API-key auth" (new). Resumes, then builds the generatedCodexAgentfrom theULTRAFUZZ_CONFIG_PATHthe fake runner received.CODEX_API_KEYis''(subscription fallback)runtime.test.ts"ordinary resume checks active-run ownership before detached preflight" (extended).state.jsonmust be byte-identical after both no-op attaches.workflow_deadline_atmoved on asubmitted: falseresumecli.test.ts"resume of an already-active run says no controller was started instead of claiming a submission" (new)Submitted resume: ultrafuzz-resume-already-activeRun already active: …runtime.test.ts"a resume that cannot re-verify the trusted CLI leaves tasks on the run's own working launcher" (new, fixer round). Resumes without a CLI entrypoint, so preparation throws while the launcher is intact. Resolvesultrafuzzthrough the runner'sPATH, requires the run'strusted-bin/ultrafuzz, then runs the task'sjson validatecall through it and parses the success envelope.ultrafuzz(undefinedinstead of<run>/trusted-bin/ultrafuzz)runtime.test.ts"native continuation does not use historical trusted CLI identity as an authorization gate" (extended with a warning assertion only)WORKFLOW_TRUSTED_CLI_UNVERIFIEDdiagnosticruntime.test.ts"resume continues with a warning when stale task-worktree cleanup fails" (new). A fakegitlists a prunable registration owned by the run and failsworktree remove.WORKFLOW_LIFECYCLE_FAILED: failed to remove stale task-worktree registrationHow the main runs were done:
start-run.tstoorigin/mainand recompiled.Tests rewritten because they locked in the bug
^config=.*/smithers/resolved-config.json$. It now assertsexecution-config.toml.PATHonly in the corrupted-metadata scenario. In that scenario the kept launcher cannot help, so that assertion checked the mechanism rather than the outcome. It is replaced by the working-launcher test above.Kept launcher that cannot verify itself
I checked this by execution. In the corrupted-metadata scenario (
trusted-cli.jsonoverwritten with{}), after an ordinary and a--refresh-controllerresume:<run>/trusted-bin/ultrafuzz json validate --jsonexits 1;ok: truewith the new warning;trusted-cli.jsonis still{}.The missing-interpreter case (exit 127,
exec: … not found) was reproduced by a reviewer, not by me.Guard test (not discriminating by design)
smithers-preparation-race.integration.test.ts"graceful pause drains in-flight tasks and refuses a second controller until the run parks" runs on the real pinnedsmithers:pauseis requested.up --resume --forceandtimetravel --forceboth exit non-zero withRUN_OWNER_ALIVE.inspectstill reportsrunning.paused, and the downstream task never started.Timing:
Mutation check: replacing
--forcewith--steal-ownershipin the takeover makes the test fail. Depending on timing, it fails on theRUN_OWNER_ALIVEmatch afterDETACHED_ADMISSION_FAILED/RUN_RESUME_CLAIM_FAILED(my run and one reviewer's), or with status 0 (the implementer's run).Teardown (fixer round): the
finallynow waits, bounded at 30 s, for every process that traced a task to exit before deleting the root. Previously, on a failure the engine kept running until the 120 s hold expired and wrote logs back under the deleted root. In the--steal-ownershipmutant, the owner had exited when I checked 5 s after the test returned, and 2.5 minutes later (past the 120 s hold) the root had not reappeared.Also ran, at the final head
10ce4e5druntime.test.ts: the six tests above plus "native resume delegates the persisted workflow…". Earlier rounds also ran the 38runtime.test.tstests that callresumeRun, thedynamic-lifecycle.test.tsresume test, and thecli.test.tslifecycle test. The fixer round changed only the warning text and a comment instart-run.ts, and I did not re-run those.cli.test.ts"resume of an already-active run…".smithers-preparation-race.integration.test.tsfile.npx prettier --checkon all changed files;npx eslinton changed sources and tests;CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci;pnpm --filter @ultrafuzz/runtime typecheck;pnpm --filter @ultrafuzz/cli typecheck;pnpm -w knip;node scripts/docs-check.mjs. All pass.Not run
Risk / compatibility
A kept launcher that cannot verify itself fails every task preparation after resume. When
<run>/trusted-bin/ultrafuzzexists but its metadata or closure is damaged, or the Node binary it names is gone, each task'spreflight-json-validatorstep fails. A likely way to lose the Node binary is an upgrade that deletes the old versioned binary, because the launcher embedsprocess.execPath; this is inferred, not reproduced. Resume still returnsok, with theWORKFLOW_TRUSTED_CLI_UNVERIFIEDwarning, which now says so.ultrafuzz, which is the documented tutorial setup, main failed these tasks too, with ENOENT.ultrafuzz, main fell back to it and may have completed. There this is a regression, accepted in exchange for never running a different CLI than the one the run launched with.--refresh-controllerdoes not recover these runs (pre-existing). The follow-up is thetrusted-cli.tsfix under "Deliberately not built".Resume can now return warnings. On a successful resume,
diagnosticscan be non-empty (WORKFLOW_TRUSTED_CLI_UNVERIFIED,WORKFLOW_WORKTREE_REPAIR_FAILED), and text output gains those lines. Both the CLI and the evmbench result schemas acceptwarningseverity whenokis true. The JSONdatashape is unchanged.Resumed runs now use the configured auth. A resumed run authenticates as configured, so api-key mode reads the configured key variable. Operators who were unknowingly running resumed Codex tasks on a ChatGPT subscription will now need the key set, which is what their config asked for.
Overlap with other open work. Checked with
git merge-treeagainst the current heads:packages/cli/test/cli.test.ts, because it changestempProject()totempProject(t: TestContext). Whichever PR lands second should convert the new CLI test toasync (t) => { const project = tempProject(t); … }.CHANGELOG.md.workflow-control.ts(w03).Closes #1153
Refs #1143, #1110
🤖 Generated with Claude Code
The PR does not appear safe to merge while an existing but unusable trusted launcher can shadow a working CLI and leave resumed tasks unable to validate.
Fix with agent prompt
Summary
The PR makes resumed agents read the run’s TOML configuration, avoids rewriting state on an active-run attach, retains an existing run-owned trusted launcher when verification fails, and reports best-effort repair failures as warnings. It also updates CLI messaging, documentation, and lifecycle tests.
Reviews (3) · Last reviewed commit: "chore: move the changelog entry to the c..."