fix(runtime)!: keep the Forge guard under group-writable umasks, warn about old cloud runs' Modal storage in clean, and drop the smithers shim - #1235
Merged
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…ritable umask Problem: under a umask that leaves new directories group writable, such as Ubuntu's default 0002, ensureSafeDirectory created <run>/safe-bin 0775. composeSmithersCommandPath admits a target-local PATH entry only through isPreparedForgeGuardBin, which refuses a group-writable directory, so the workflow engine's PATH dropped the wrapper and agents ran the real forge without forge_vmem_limit_kb or forge_rayon_threads while run.json recorded forge_guard.active: true. A custom run.output_dir produces the same false claim under any umask, because that check admits the wrapper only from <project>/.ultrafuzz/runs/<run-id>/safe-bin. Change: prepareForgeGuardEnvironment sets safe-bin to 0700 after creating it (chmod does not apply the umask, and it also repairs the directory of a run launched earlier), then applies the engine's own admission check to it. It reports the guard active only when that check passes. Otherwise it leaves PATH and the guard variables unset and returns a FORGE_GUARD_INACTIVE warning, which launch, resume, replay and fork return with their result. The admission check itself is unchanged. resume now also records the guard of the controller it starts in run.json, best effort like its state projection, so run.json no longer keeps a launch's claim after a resume that dropped the wrapper. The OpenRouter adapter contract test failed on such hosts for a different reason: its fixture rooted the operator-owned provider homes under the target's .ultrafuzz, which `ultrafuzz init` creates with the process umask. The adapter creates every provider-home component itself with mode 0700, so the fixture now uses its own private root; the provider-home ancestor check is unchanged. The new tests pin umask 0002 through a shared helper, so they reproduce on CI runners, whose umask is 022. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…loud run Problem: before #1197 removed per-node cloud execution, `ultrafuzz clean` of a run planned with `[execution] mode = "cloud"` terminated its sandboxes (tagged purpose=ultrafuzz-node) and deleted its Modal volume before deleting the run directory. #1197 dropped that step and kept the deletion, so cleaning such a run now leaves its Modal volume and any running sandbox in place, still billed, and deletes the plan.json that records the Modal app they live in, without saying so. Change: before anything is removed, clean reads the mode and Modal app from each selected run's plan.json, which copies the execution block of the run's resolved config and is what the removed cleanup read. For a cloud run it returns a CLEAN_CLOUD_STORAGE_RETAINED warning naming the app, the volume and the sandboxes' run tag, and the `modal volume delete` command. `--dry-run` reports it without deleting, and every later result, including a failed removal, carries it. The deletion itself is unchanged. The volume is not `ultrafuzz-node-<run-id>`: the removed provider named it `ultrafuzz-node-` plus a bounded identity of the Smithers run ID `ultrafuzz-<run-id>` (at most 32 normalized characters, then 12 hex digits of its SHA-256). The same identity was the sandboxes' `run` tag. clean reproduces that function, and the test pins the name the removed code computes for a fixed run ID. An unreadable plan.json yields no warning. It also makes the source revision step fail before anything is removed, as it did before, so no run that clean deletes skips the check. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Problem: #1197 made compilation write every task's execution block as `{ mode: "local", resources, agentCredentialEnv: [] }` with no `modal` entry, but synchronization still added task.execution.agentCredentialEnv and task.execution.modal?.credentialEnv to its exact-value redaction list. For every run compiled since #1197 both are empty. The generated agent environment also still blanked ULTRAFUZZ_MODAL_MODULE, which only the removed cloud provider set. And the doctor tests still wrote a fake engine into the target's .smithers, although since #1201 doctor inspects only the runner Ultrafuzz's own install provides. Change: synchronization derives its redaction values from the environment alone, the agent environment no longer lists ULTRAFUZZ_MODAL_MODULE, and seven doctor tests lose the dead fake-engine setup. One test used it only to create the .smithers/node_modules/.bin directory it writes into, and now creates that directory itself. The doctor test that installs a 0.29.0 project-local engine keeps it: it asserts that doctor ignores that engine. The task manifest keeps agentCredentialEnv, because its sealed schema requires the field. Effect on runs compiled before #1197 in cloud mode, whose lists were not empty: sensitiveEnvironmentValues still redacts, by name, every variable that looks like a credential (*_API_KEY, *_TOKEN, *_SECRET, ...), which includes MODAL_TOKEN_ID and MODAL_TOKEN_SECRET. What these lists added beyond that were route variables such as *_BASE_URL and allowlisted variables. Those carry no secret, or have a value the secret patterns already match. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Problem: #1201 wrote <run>/trusted-bin/smithers at launch, resume, replay and fork so that the generated workflow's bare `smithers node` call (#1143) found a runner. #1183 removed that call: final-report producer retries and verifiers now read the selections each producer attempt records in the run. The current template spawns only git, bash and the run's `ultrafuzz` launcher, and every command the runtime spawns runs an executable it bound by path, never `smithers` from PATH. smithers-report-retry.integration.test.ts puts a failing `smithers` first on the engine PATH around the production report code and still passes. The shim therefore served only workflows persisted by earlier releases, and it put an engine CLI first on every task's PATH. It also made replay and fork bind the installed runner, which they otherwise never run: they run the run's sealed engine. Change: delete writeTrustedSmithersShim and its three callers. When resume cannot re-verify the trusted CLI, it again keeps an existing run launcher first on PATH itself, which the shim had done as a side effect; the existing test for that path covers it. Launch still binds the installed runner before creating a run, because resume runs it; the comments and the doctor summaries now say launch and resume. The capability and native-continuation tests drop their shim assertions, and a resume test now asserts that trusted-bin holds only the ultrafuzz launcher. The docs no longer say that the shim serves the workflow's smithers calls or that tasks can drive their run through it. The CHANGELOG entry for #1201, which has not been released, drops its shim claims, and a new entry records the removal. BREAKING CHANGE: tasks no longer find a `smithers` CLI that Ultrafuzz provides on PATH. A run launched by an earlier release and continued by plain `resume` still runs its persisted workflow, whose bare `smithers` call in a restarted controller finds a runner only on the operator's own PATH, as before #1201; continue such a run with `resume --refresh-controller`. Co-Authored-By: Claude Opus 5.5 <[email protected]>
aviggiano
force-pushed
the
claude/post-train-followups
branch
from
September 30, 2026 20:56
e4c74ea to
40c8855
Compare
aviggiano
added a commit
that referenced
this pull request
Sep 30, 2026
…rator's private group Under Ubuntu's default umask 0002, ~/.local is 0775 and belongs to the user's private group. assertSafeDirectory refused every group-writable, non-sticky ancestor, so the default provider-home root ~/.local/state/ultrafuzz/provider-homes was refused, and OpenRouterAgent, DeepSeekAgent and any agent with a config_dir could not start. A group-writable ancestor is now accepted when the operator owns it, its group is the operator's primary group, /etc/group lists no member of that group other than the operator (under any /etc/passwd name with the operator's UID), and no other /etc/passwd account has it as its primary group. The /etc/passwd half matters: primary-group members are not listed in /etc/group, so a shared primary group such as `users` has an empty member list there. Anything the files cannot show fails closed: an unreadable file, an unrecognized entry, or an operator account or group missing from them (LDAP). World-writable ancestors without the sticky bit stay refused. Both refusals now name the directory and the remedy. Tests pin umask 0002 (test/process-umask.ts, the same file as #1235) and point the adapter at test-written account files by rewriting its /etc/passwd and /etc/group literals. They cover the accepted private group, eight shared or unprovable cases, world-writable ancestors, and the OpenRouter adapter contract with the default root. Closes #1236 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…e run Problem: for a run an earlier release planned for per-node Modal execution, the CLEAN_CLOUD_STORAGE_RETAINED warning names the Modal volume, app and sandbox tag that `clean` leaves behind, and the run's plan.json, which the removal deletes, is the only record of the app. cleanRun computed the warning before removing anything but returned it only with its result, and the CLI prints a successful result's diagnostics after "Removed: ...". The operator therefore saw it only once the plan was gone, and never when clean did not return: the audit append after the removal had no try/catch, so a journal lock timeout, a full disk or a read-only .ultrafuzz rejected cleanRun after the run was deleted and dropped the warning with it. The CHANGELOG and the how-to also gave the volume as ultrafuzz-node-ultrafuzz-<run-id>-<hash>, which is wrong for every generated run ID. The removed provider kept at most 32 characters of ultrafuzz-<run-id>, and a generated ID such as run-20260930t161149123z-ab12cd34 makes that 42, so its volume is ultrafuzz-node-ultrafuzz-run-20260930t161149123-d99772718708. The runtime message was right; the test pinned only a short run ID. Change: - CleanGeneratedInput takes an optional onRetainedStorage callback. cleanRun calls it with the warnings just before the first deletion, after the source-ref step that can still stop the removal; a dry run does not call it. Every result still carries the warnings. - The CLI's text output prints them from that callback and leaves them out of its final text, so they appear once, before anything is removed. JSON output and the dashboard keep reading them from the result. - A failed audit append is now a CLEAN_AUDIT_FAILED failure that says whether anything was removed and still carries the warnings. - The docs describe the volume name as derived, say to copy it from the warning or find it with `modal volume list`, and the runtime test pins the truncated name for a generated run ID. On the branch before this commit, the new tests fail as intended: the callback is never called, the audit test rejects with EACCES on .ultrafuzz/clean-audit.jsonl.lock, and the CLI test sees the warning first written after plan.json was deleted. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Problem: four ways the Forge guard follow-up could still leave run.json or the operator with a wrong picture. - A synchronization pass (status --watch, the dashboard, the eval runner) reads run.json, then awaits usage replay and possibly a pricing fetch, and wrote back that copy plus its accounting. It holds only .workflow-sync.lock, and resume records its controller's guard holding only the lifecycle lock, so a pass that read before a resume's write and wrote after it put the launch's forge_guard back, and nothing rewrote it until the next lifecycle command. A test whose pricing fetch rewrites the guard mid-pass reproduces this on the branch: run.json ends with the launch's active: true. replay and fork, which record the guard the same way, had this on main. - The engine PATH admits safe-bin only while it holds the wrapper alone. The .forge.tmp-* file an interrupted writeFileDurable leaves, or any other entry, therefore dropped the guard for every later launch, resume, replay and fork of the run, and nothing removed it. - writeFileDurable created the wrapper 0600 and chmod made it 0700 after the rename. resume and replay rewrite the wrapper while tasks may be running, and a PATH lookup in that window skips a file without its execute bit and runs the real Forge. - A resume that only attached to a running controller still returned FORGE_GUARD_INACTIVE about the controller it did not start. The warning, config.md and configuration.md also left out the admission's no-symbolic-link condition, so a project reached through a symlinked parent directory was told the engine admits only the directory the wrapper was in. Change: - @ultrafuzz/artifacts adds updateRunMetadataDocument, which re-reads run.json and writes the update while holding run.json.lock, taken with the existing journal lock. Recording the Forge guard and synchronization's accounting write both use it. The accounting now goes into the document as it is at the write, which must still be bound to the workflow it was computed for; otherwise the pass reports WORKFLOW_ACCOUNTING_FAILED, as it already did for that mismatch at its start, and writes nothing. - prepareForgeGuardEnvironment removes every entry but forge from safe-bin before writing the wrapper, best effort: an entry it cannot remove still fails the admission, which it reports. It writes the wrapper with mode 0700, so the rename replaces an executable file with an executable file. - resume returns the Forge guard diagnostics only when it starts a controller, the same condition under which it records the guard. - The warning and the docs state the symbolic-link condition. The unit test's stray-file case, which expected the guard inactive, now expects it cleared and active; the inactive cases are a custom output_dir and a symlinked parent. The resume test makes the guard inactive with an entry resume cannot remove, and is skipped under root. On the branch before this commit, the new tests fail as intended, including the attach case, which returned the warning. Not covered: replay and fork rebind run.json's workflow under the control lock, not run.json.lock. The accounting write now refuses a document bound to another workflow instead of writing the old binding back, which narrows that race on main to the lock-held re-read and write, but does not close it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…shim Problem: a Breaking-changes entry announced that launch, resume, replay and fork no longer write <run>/trusted-bin/smithers. No release wrote it: #1201 added it after v0.1.2, and this branch already rewrote #1201's entry without it. Measured against v0.1.2, nothing the entry lists changes: replay and fork never refused an unpatched install, and a run launched by an earlier release already found its persisted workflow's bare `smithers` only on the operator's PATH. Change: remove the entry. Its one piece of advice moves to the #1183 entry, which already explains that plain resume keeps running the persisted workflow: such a run still runs `smithers node` in a restarted controller, so continue it with `resume --refresh-controller`. The #1183 entry already gives the --reset-node form for a report producer that has succeeded. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…safe-bin Launch holds the workflow control lock and resume, replay and fork the lifecycle lock, so a launch and a resume can prepare the same run's Forge guard at once. The wrapper was written through a temporary file inside safe-bin, so the stray-entry cleanup of one command could delete the other's temporary file before its rename, failing that command with ENOENT. An engine PATH composed meanwhile also saw two entries in safe-bin and dropped the wrapper, although run.json recorded the guard active. writeFileDurable now takes an optional temporaryDirectory, and the wrapper's temporary file goes in the run root, so safe-bin only ever holds the wrapper. The cleanup still removes temporary files that interrupted writes left in safe-bin before this change. Co-Authored-By: Claude Opus 5.5 <[email protected]>
aviggiano
force-pushed
the
claude/post-train-followups
branch
from
September 30, 2026 22:34
40c8855 to
66dc543
Compare
…ugh testWhen
The test "resume records the Forge guard of the controller it starts"
passed { skip: ... } to test() to stay out of root runs. Bun's node:test
shim ignores that option, so the monolithic suite bans it (#709), and "the
monolithic runtime suite registers exclusively through the shard wrapper"
failed on the stray skip:.
Register the test through testWhen(process.getuid?.() !== 0), which
selects test.skip for root in both the Node and Bun lanes, as the lock
synchronization test does. A comment keeps the reason the skip option
carried.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
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
1 participant
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.
Post-merge-train follow-ups on
main(#1197, #1201, #1183, #1198), in four commits, plus four that address the review of them (see Review follow-ups below).Problem
1. Forge guard dropped from the engine PATH, while run.json said it was active (bug on
main)Under a umask that leaves new directories group writable, such as Ubuntu's default
0002,ensureSafeDirectorycreated<run>/safe-binwith mode0775.composeSmithersCommandPathadmits a PATH entry inside the target only throughisPreparedForgeGuardBin, which refuses a directory wheremode & 0o022is set. The workflow engine's PATH therefore dropped the wrapper, and agents ran the realforgewithoutforge_vmem_limit_kborforge_rayon_threads, whilerun.jsonrecordedforge_guard.active: true.main, run it under this host's umask0002.startRun injects the configured Forge guard into the workflow environment and metadatafails because the fake engine resolves/tmp/ufz-runtime-…-fake-bin/forgeinstead of<run>/safe-bin/forge.run.output_dir,run.jsonmakes the same false claim under umask022too, becauseisPreparedForgeGuardBinonly admits the wrapper from<project>/.ultrafuzz/runs/<run-id>/safe-bin. A probe onmainprinted.ultrafuzz/runs active: true safe-bin on engine PATH: trueandaudit-runs active: true safe-bin on engine PATH: false.generated OpenRouter adapter preserves opaque model IDs…failed onmainunder0002withprovider-home ancestors cannot be group/world writable. The ancestor the check refused was<project>/.ultrafuzz, created byultrafuzz initwith the process umask (a probe showed0775). The fixture had rooted the operator-owned provider homes under the target. The adapter itself creates every provider-home component withmkdirSync(…, { mode: 0o700 }), and the umask cannot widen that mode. In production the default root is$XDG_STATE_HOMEor~/.local/state/ultrafuzz/provider-homes, which is not under the target.2.
ultrafuzz cleanof a run planned for per-node Modal execution (Greptile P1 on #1197)Before #1197,
cleanterminated such a run's sandboxes (taggedpurpose=ultrafuzz-node) and deleted its Modal volume, then deleted the run directory. Now it deletes only the directory. The volume and any running sandboxes stay in place and may still be billed, and the deletion removes theplan.jsonthat names their Modal app.3. Leftovers of #1197 and #1201
workflow-sync.tsstill addedtask.execution.agentCredentialEnvandtask.execution.modal?.credentialEnvto its redaction list. Compilation has written both empty since refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197.ULTRAFUZZ_MODAL_MODULE, which only the removed cloud provider set..smithers, although doctor has inspected only Ultrafuzz's installed runner since feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201.4. The
<run>/trusted-bin/smithersshim (#1201) had no remaining caller#1183 removed the generated workflow's bare
smithers nodecall. I checked that nothing else needs the shim:git,bashand the run'sultrafuzzlauncher.smithersfallback insmithersExecutableis unreachable, becauseprepareSmithersExecutableEnvironmentalways binds a capability or throws.smithersby name.smithers-report-retry.integration.test.tsputs a failingsmithersfirst on the engine PATH around the production report functions, which it extracts from the template. It passes on this branch.So the shim served only workflows persisted by earlier releases, and it put an engine CLI first on every task's PATH. It was also the only reason
replayandforkbound the installed runner. They otherwise run the run's sealed engine (they have no pre-inspection, and the snapshot environment carries the sealed runner's capability).Change
fix(runtime): keep the Forge guard on the engine PATH under a group-writable umaskprepareForgeGuardEnvironmentsetssafe-binto0700after creating it.chmoddoes not apply the umask, and it also repairs the directory of a run launched earlier.isPreparedForgeGuardBin) to the prepared directory. It reportsactive: trueonly when that check passes. Otherwise it leaves PATH and the guard variables unset and returns aFORGE_GUARD_INACTIVEwarning, which launch,resume,replayandforkreturn with their result. The admission check itself is unchanged.output_dirwhile Forge is installed and the guard is enabled (the default). Those runs are unguarded onmaintoday too, but at leastrun.jsonnow says so and the command warns.resumenow also records the guard of the controller it starts inrun.json. The write is best effort, like its state projection, so a legacyrun.jsoncannot fail a continuation.0002. The provider-home check is unchanged.test/process-umask.ts(underGroupWritableUmask), pins umask0002for a test body, so these tests reproduce on CI runners too (their umask is022).fix(runtime): name retained Modal storage when clean deletes an old cloud runcleanreads each selected run'splan.json. That file is the run's resolved execution config asplan-run.tscopies it, and it is what the removed cleanup read.cleanreturns aCLEAN_CLOUD_STORAGE_RETAINEDwarning naming the Modal app, the volume, the sandboxes'runtag andmodal volume delete <volume>.--dry-runshows it before anything is deleted. Every later result carries it, including a failed removal. The deletion itself is unchanged.ultrafuzz-node-<run-id>. The removed provider named itultrafuzz-node-plusboundedIdentity("ultrafuzz-<run-id>"): the run ID normalized to at most 32 characters, then 12 hex digits of its SHA-256. The same identity was the sandboxes'runtag.cleanreproduces that function, and the test pins the name that the removed function, evaluated from the pre-refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 tree, computes for a fixed run ID.plan.jsonand notsmithers/resolved-config.json?cleanalready readsplan.jsonstrictly before it deletes a run, and an unreadable plan stops it withCLEAN_SOURCE_REF_FAILED. So no run thatcleandeletes can skip the check. The strict resolved-config parser would miss older documents the current schema rejects, and a run that fails the parse would be deleted with no warning.refactor(runtime): drop reads that per-node cloud removal left emptyULTRAFUZZ_MODAL_MODULE..smithers/node_modules/.bindirectory it writes into itself. The test that installs a0.29.0project-local engine keeps it, because it asserts that doctor ignores that engine.agentCredentialEnv, because its sealed schema requires the field.mode === "local"checks inworkflow-sync.tsstill skip the tasks of persisted pre-refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 cloud runs, so they are not dead and I left them.refactor(runtime)!: remove the run's trusted-bin/smithers shimwriteTrustedSmithersShimand its three callers.resumecannot re-verify the trusted CLI, it again keeps an existing run launcher first on PATH itself. The shim had done that as a side effect. The existing test for that path covers it, and it now also asserts thattrusted-binholds onlyultrafuzz.resumeruns it. The comments, the doctor summaries and the docs now say "launch and resume".docs/security.mdanddocs/reference/cli.mdno longer claim that the shim serves the workflow'ssmitherscalls or that tasks can drive their run through it.Review follow-ups
fix(runtime): print clean's retained Modal storage before removing the runcleanRuncomputedCLEAN_CLOUD_STORAGE_RETAINEDbefore removing anything but returned it only with its result, and the CLI prints a successful result's diagnostics afterRemoved: …, so the operator saw it only after theplan.jsonthat names the Modal app was gone. Worse, the audit append after the removal had no try/catch: a journal lock timeout, a full disk or a read-only.ultrafuzzrejectedcleanRunafter the run was deleted, and the warning was lost.CleanGeneratedInputtakes an optionalonRetainedStoragecallback, called with the warnings just before the first deletion (never on--dry-run). The CLI's text output prints them from it and leaves them out of its final text, so they appear once, before anything is removed. JSON output and the dashboard read them from the result, which still carries them.CLEAN_AUDIT_FAILEDfailure that says whether anything was removed and still carries the warnings.ultrafuzz-node-ultrafuzz-<run-id>-<hash>, which is wrong for every generated run ID: the removed provider kept at most 32 characters ofultrafuzz-<run-id>. They now say to copy the name from the warning or find it withmodal volume list, and the test pinsultrafuzz-node-ultrafuzz-run-20260930t161149123-d99772718708for the generated-format IDrun-20260930t161149123z-ab12cd34.fix(runtime): keep run.json's Forge guard record true after resumestatus --watch, the dashboard, the eval runner) readsrun.json, awaits usage replay and possibly a pricing fetch, and wrote that copy back with its accounting, holding only.workflow-sync.lock. A pass that straddled a resume's write put the launch'sforge_guardback. A new test whose pricing fetch rewrites the guard mid-pass reproduces it on the branch before this commit (active: truecomes back).@ultrafuzz/artifactsaddsupdateRunMetadataDocument, which re-readsrun.jsonand writes the update holdingrun.json.lock(the existing journal lock). Recording the Forge guard and the accounting write both use it. The accounting write re-checks that the document is still bound to the workflow it computed for, and otherwise reportsWORKFLOW_ACCOUNTING_FAILED, as for the same mismatch at the pass's start, and writes nothing.prepareForgeGuardEnvironmentremoves every entry butforgefromsafe-bin, best effort, so the.forge.tmp-*file of an interrupted write no longer drops the guard for the rest of the run; an entry it cannot remove still fails the admission, which it reports. It writes the wrapper with mode0700, so a rewrite while tasks run never leaves a non-executable wrapper onPATH, which a lookup would skip for the realforge.resumereturnsFORGE_GUARD_INACTIVEonly when it starts a controller, the condition under which it records the guard.docs/config.mdanddocs/reference/configuration.mdnow state the admission's no-symbolic-link condition, which a project reached through a symlinked parent fails.docs(changelog): drop the entry for removing the unreleased smithers shimtrusted-bin/smithers: feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201 came after v0.1.2. Measured against v0.1.2, nothing the Breaking-changes entry listed changes, so it is gone. Its one piece of advice, to continue an earlier release's run withresume --refresh-controller, moved into the fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 entry.fix(runtime): stage the Forge guard wrapper's temporary file outside safe-binforge-guard.ts:60, "Concurrent wrapper writes can fail"): confirmed. Launch holds the control lock, whileresume,replayandforkhold the lifecycle lock, so a launch and aresumeof one run can prepare its guard at the same time.writeFileDurableput the wrapper's temporary file insidesafe-bin, so one command's stray-entry cleanup could delete the other's.forge.tmp-*file before it was renamed. A new test runs a second preparation at exactly that moment (inside arenameSynchook, no timing). On the branch before this commit the first preparation then throwsENOENTon the rename, and for a launch that becomes a recorded launch failure (WORKFLOW_SUBMISSION_FAILED). The same moment exposes a second hazard, already present onmain: an engine PATH composed then sees two entries insafe-binand drops the wrapper, whilerun.jsonrecords the guard as active.writeFileDurabletakes an optionaltemporaryDirectory, and the wrapper's temporary file now goes in the run root, sosafe-binonly ever holds the wrapper. No age or PID heuristic is needed, because no live writer's file is ever insafe-binfor the cleanup to find. The cleanup still removes the.forge.tmp-*files that interrupted writes left there before this change. An interrupted write now leaves its temporary file in the run root, where nothing admits it to PATH or lists it strictly.forge-guard.test.js8/8 andruntime.test.jsForge guard4/4 pass. On the branch before this commit, the new test fails with theENOENTabove, and its mid-write admission assertion fails there too. Gates:format:check,lint,knip,typecheck(@ultrafuzz/artifacts,@ultrafuzz/runtime) anddocs-checkall exit 0.lint:strict:ciagainstorigin/mainreports 21 errors, all in linesmainchanged after this branch's base (the branch is one commit behind), none in this commit's files. Against the merge base it exits 0.Verification
dist-testwas rebuilt from each commit's source. Suites marked "both" ran under ambient umask0002(this host) and022(CI).forge-guard.test.js+provider-home.test.js+clean.test.jsruntime.test.jsForge guard|trusted CLI leaves tasks(launch, customoutput_dir, resume, kept launcher)bun test … '^Bun adapter contract:')lifecycle-inspection.test.jsdiagnoseProject|installation inspectionruntime.test.jssyncRun tests (19, including redaction across credential rotation)runtime.test.jstrusted CLI|resume, replay, and fork delegatesmithers-executable-capability.test.jssmithers-preparation-race.integrationnative continuation (real engine)smithers-report-retry.integration(real engine, poisonsmitherson PATH)run, ps, status, … clean, and lifecycle commands;doctor reports install postureThe new tests fail on
main. I swappedmain'sforge-guard.tsandstart-run.tsinto the tree and ran under ambient umask022. All six new or changed Forge-guard tests failed for the intended reasons:safe-binmode509(0775) where448(0700) was expected;…-fake-bin/forgewhere<run>/safe-bin/forgewas expected;active: truewherefalsewas expected, for the customoutput_dircase, the stray-file case and the resume case.The OpenRouter contract test fails on
mainunder0002. It passes here under both umasks.Review follow-ups (
dist-testrebuilt at this HEAD; "both" as above):forge-guard+provider-home+clean+source-revision+audit-contractsruntime.test.jsForge guard|syncRun|accounting|trusted CLI leaves tasks|resume, replay, and fork delegate|replay|forkartifactsrun-documents+journal-lockclean prints the Modal storage …andrun, ps, status, … clean, and lifecycle commandssmithers-preparation-race.integrationnative continuation (real engine)The new tests fail on the branch before these commits, for the intended reasons (its sources swapped in, the tests compiled against them):
onRetainedStorageis never called; the audit test rejects withEACCESon.ultrafuzz/clean-audit.jsonl.lockafter the run is gone; the CLI test sees the warning first written afterplan.jsonwas deleted.active: false; the inactive message lacks the symbolic-link condition; the attaching resume returnsFORGE_GUARD_INACTIVE.forge_guard.active: trueafter the pass.CLI check of
cleanon a demo run whose plan hasmode = "cloud", with the CLI built at this HEAD.--yesprints the warning before it removes anything, and only once:--dry-runremoves nothing and prints the same warning after itsRemoved: runs/old-cloud-runline, as every successful result's diagnostics are printed.The removed provider's
boundedIdentity, evaluated from the pre-#1197 tree, gives the same volume name.Gates (exit codes checked directly):
pnpm -w format:check0pnpm -w lint0CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci0pnpm -w knip0, both on a fresh unbuilt worktree at this HEAD and afterpnpm -w buildthere (the build exited 0)pnpm --filter <pkg> typecheck0 for@ultrafuzz/artifacts,@ultrafuzz/runtimeand@ultrafuzz/clipnpm --workspace-concurrency=1 --filter "...^@ultrafuzz/artifacts" typecheck0 (its 11 dependents, serially)node scripts/docs-check.mjs0Risk
run.output_dirstill gets no Forge guard. That is unchanged frommain, but it is now recorded (active: false) and warned about (FORGE_GUARD_INACTIVE). Actually guarding those runs needs the engine PATH admission to learn the configured runs root without looseningisPreparedForgeGuardBin. That is a separate change.resumenow writesrun.json#forge_guard, best effort, only when it starts a controller (not on an attach), and returnsFORGE_GUARD_INACTIVEunder the same condition.safe-binis cleared of everything butforgeby every launch,resume,replayandfork. The directory is run-owned and documented to hold only the wrapper. Its temporary file is staged in the run root, so the clearing never deletes a concurrent command's write. A write interrupted between its open and its rename leaves a small.forge.tmp-*file in the run root.run.json.lockcovers only the Forge guard record and the accounting write.replayandforkrebindrun.json's workflow under the control lock, not this one. The accounting write now refuses a document bound to another workflow instead of writing the old binding back, which narrows that race, present onmain, to the lock-held re-read and write, but does not close it.cleanwarning goes to stdout in text mode, beforeRemoved: …, as a successful command's diagnostics already do.cleanwarning has no Modal access. It does not check whether the volume still exists, so a cloud run that never dispatched a node gets the warning even though it has no volume. Replays and forks of a cloud run could have used other Smithers run IDs. The old cleanup also handled onlyultrafuzz-<run-id>, and the warning names only that ID.sensitiveEnvironmentValuesstill matches every credential-looking name (*_API_KEY,*_TOKEN,*_SECRET, …, includingMODAL_TOKEN_*). The dropped names added only route variables (*_BASE_URL) and allowlisted variables, whose values either carry no secret or already match the secret patterns.resumestill runs its persisted workflow. That workflow's baresmitherscall, in a restarted controller, finds a runner only on the operator's own PATH, as before feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201.resume --refresh-controlleravoids it (see fix(runtime): record final-report producer selections in the run instead of querying smithers #1183's entry).mainbetween feat(runtime)!: run lifecycle commands from the pnpm-patched install instead of per-command npm installs (#921 step 1) #1201 and this change (no release) keep theirtrusted-bin/smithers. It stays on PATH, becausetrusted-binleads PATH for the validator launcher, and it is no longer rewritten, so it fails once apnpm installmoves the engine.replayandforkno longer refuse an unpatched install; they never run it.~/.localis group writable, which is what Ubuntu's default umask produces (it is0775on this host), the provider-home check refuses the default root~/.local/state/ultrafuzz/provider-homes. That is the check working as designed, and I did not loosen it. Operators needchmod g-w ~/.localorULTRAFUZZ_PROVIDER_HOME_ROOT.Changelog
Under
## Unreleased:safe-bin, clearing it of everything but the wrapper, the truthfulforge_guard.active,FORGE_GUARD_INACTIVE(customoutput_diror a path through a symbolic link),resumerecording the guard only when it starts a controller, synchronization no longer writing the earlier guard back, and the wrapper's temporary file staged in the run directory, so overlapping commands of one run keep it on each other's engine PATH.cleanwarning (CLEAN_CLOUD_STORAGE_RETAINED, printed before the removal,modal volume delete, and how the volume name is derived).resume --refresh-controller.resume(notreplayandfork) refuse an unpatched install, and that they needbunonPATHto run the engine.🤖 Generated with Claude Code
The PR appears safe to merge; no outstanding findings or new actionable issues were identified.
Summary
This PR repairs Forge guard admission and metadata recording, warns before cleaning runs that may retain Modal storage, removes the unused Smithers shim, and updates related tests and documentation. The change since the previous review only changes how one root-sensitive test is registered.
Reviews (5) · Last reviewed commit: "test(runtime): register the root-skipped..."