Conversation
b5843fb to
1e79fde
Compare
cba0851 to
84b7dac
Compare
…edDependencies The repository install now carries every Smithers compatibility patch: pnpm applies patches/[email protected] and patches/@smthrs__{agents,cli,db,engine,scheduler}@0.35.0.patch at install time. scripts/smithers-patches.mjs generates those files: it applies SMITHERS_COMPATIBILITY_PATCHES to pristine copies of the pinned packages with applySmithersCompatibilityPatches, the function launch uses, and diffs them the way `pnpm patch-commit` does. A new CI build-gate step runs it with --check and fails when a committed patch file or its pnpm-workspace.yaml entry no longer matches the registry. The registry stays the source of truth, and launch still installs and patches its own TMPDIR controller, so production behaviour is unchanged by this commit. Regenerate the files and the lockfile's patch hashes with: pnpm -w build && node scripts/smithers-patches.mjs && pnpm install --config.optimistic-repeat-install=false pnpm's repeat-install check ignores patch-file contents, so after an earlier install a plain `pnpm install` prints "Already up to date", leaves the hashes stale, and CI's frozen install then fails with ERR_PNPM_LOCKFILE_CONFIG_MISMATCH. Tests that read the pinned runner now see the bytes production runs: - the upstream-shape tests (patch anchors, envelope contract, delegation refusal) undo the registry's replacements, checking that each is installed exactly once and that re-applying the registry reproduces the installed file; - the usage, cost and path-acceptance tests use the installed patched modules instead of patching a copy; - the controller-engine, resume-reopen and supervisor-relaunch integration tests drop their own load-time patch plugin and patched store copies, and the shared patched-smithers-runner test helper goes (the relaunch test's leftover-process cleanup matches its run ID instead of the copy's path); - the Bun adapter contracts' OpenRouter helper checks the installed ordered-stdout patch instead of applying it, and the DeepSeek usage contract asserts what the patched agents report: inputTokens counts cache reads and writes (527 = 120 fresh + 400 read + 7 write), with each component in inputTokenDetails. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ling one Every Smithers command whose environment carries no sealed runner used to npm-install the pinned engine closure into mkdtemp(os.tmpdir()) and patch it at runtime: native resume and its pre-resume inspect, the --refresh-controller ownership inspection, and commands on runs without a sealed snapshot. Resume marked that root retained for its detached engine, so each resume left one behind in TMPDIR and depended on the registry. Those commands now bind installedWorkflowRunner(): the smthrs package @ultrafuzz/runtime resolves, which pnpm has patched at install time. It is refused, with "reinstall Ultrafuzz with pnpm install --frozen-lockfile", unless every registered compatibility patch and engine anchor classifies as applied. Like the unsealed native continuation it replaces, its command process runs with BUN_TARGET_CONFIGURATION_GUARD_ARGS (--config=/dev/null --no-env-file --no-install --no-addons), and a continued target workflow resolves its bare imports from the runner's own dependency directory (NODE_PATH). - Delete the native-continuation marker, the retention of its TMPDIR root, the per-command controller-closure seal and the TMPDIR controller's Bun interpreter assertion. - Launch, resume, replay and fork write <run>/trusted-bin/smithers, a shim that runs the installed runner with the same flags, so the generated workflow's bare `smithers` calls resolve (#1143 point 1). - smthrs becomes a runtime dependency. The production advisory gate now audits the engine tree and found GHSA-p95v-992w-h6c3 in @toon-format/toon 2.3.0, which @smthrs/cli pins exactly; a scoped override takes 2.3.1. The execution-snapshot and trusted-CLI closure walkers skip a first-party package's smthrs edge, so launch does not copy a second engine closure into every run (both walkers go away in later #921 steps). - validate:pack copies the patch files, root overrides and build allowlist into the consumer and requires doctor to report both engine checks ok. - doctor reports the installed runner's layout and patch posture as ok/error checks instead of an ignored project-local tree. - docs/reference/cli.md tells operators to run long campaigns from a dedicated checkout or worktree. The resumed engine and its supervisor run from that install, and after a pnpm install that changes a Smithers patch, pnpm deletes the package directory the supervisor relaunches the engine from, in that install or a later one (by default it clears orphaned package directories once seven days have passed since it last did). `ultrafuzz resume` continues the run from the new install. CHANGELOG.md records the change under Unreleased. Launch otherwise works as before: it still installs, patches and seals its own controller into the run's execution snapshot. pause, cancel, status, replay and fork on a sealed run still execute the sealed runner, which never installed one. Tests: the real-engine native-continuation test resumes with every proxy and npm_config_registry pointed at a listener that counts connections, and asserts zero of them, an empty TMPDIR, NODE_PATH at the installed runner's dependency directory and a working trusted-bin/smithers. The report-retry tests give the engine a PATH with no runner but the shim. Unit tests cover the guard, doctor and the closure exclusion. BREAKING CHANGE: a pnpm checkout with the Smithers patchedDependencies is the only supported install. An install without the patches (a packed or plain npm install) is refused at launch, resume, replay and fork, which also need bun on PATH to write the shim. Refs #921, #1143, #1147 Co-Authored-By: Claude Opus 5.5 <[email protected]>
84b7dac to
42a1bc8
Compare
…ort record The verifier's "never recorded" error told operators to run `resume --refresh-controller --retry-failed`, but `--retry-failed` resets every failed or stalled task, so in a best-effort campaign it would also rerun each failed strategy task. Both record errors now name `resume <run-id> --refresh-controller --reset-node <producer node>`, which resets the producer's latest attempt and its dependents, here its verifier. A scratch run against the pinned Smithers confirmed that this timetravel resets exactly node:report and verify:report and that the re-dispatched attempt rewrites the record. A malformed record blocked every re-dispatched attempt above 1 without naming a way out; its error now names the file to delete. The history test now asserts both errors' exact text and pins that a first attempt replaces an unreadable record instead of failing on it. The docs paragraph states the two cases where failed_attempts is inexact and that a Codex workspace-write agent can reach the record when the project lives under /tmp or $TMPDIR. The CHANGELOG entry drops its self-contradicting PATH clause and names the mixed plain and refreshed resume failure with its recovery. The integration test's poison-stub comment is reworded so it stays true after #1201. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Review found code and wording the earlier cleanup commit missed because
neither TypeScript nor knip flags them:
- `compileTask`'s `env` input: its only reader was the deleted
`cloudAgentCredentialEnv`. `startRun` still built
`{ ...process.env, ...input.env }` and passed it through
`compileSmithersWorkflow`, where nothing read it. The field goes from
`SmithersCompileInput` and `compileTask`, with both call-site props and
the `startRun` argument.
- `workflowModuleEntryUrls()` and the empty-string filter on its result
existed only for the conditional `modal: ""` entry. Both URLs are now
always present, so the two `import.meta.resolve` calls are inlined.
- The fake runners' `SMITHERS_FAKE_CLOUD_ENV_LOG` hooks, which printed
MODAL_TOKEN_ID and MODAL_TOKEN_SECRET, and the
`SMITHERS_FAKE_CLOUD_ENV_LOG`/`SMITHERS_FAKE_CLOUD_SELECTOR_LOG` entries
in the Smithers test environment allowlist: only the deleted cloud
credential-forwarding tests set them.
- The Effect-pinning comments in smithers-package.ts, runtime.test.ts and
the workspace override CI test justified the pin with two cloud
containers; the reason holds for a run's launch and a later resume,
which install at different times. The pnpm-workspace.yaml copy is left
to #1201, which rewrites that block.
- A stateful-profile test named "preserves cloud timeout inheritance" now
only checks that the resource timeout origin survives TOML snapshots.
Refs #134
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…usable installed runners at launch Review of the installed-runner change found that it let target code run inside read-only and controller commands. Smithers 0.35.0 imports <workspace>/.smithers/smithers.config.ts to choose its store backend unless --backend or SMITHERS_BACKEND is set, and the installed runner has no module confinement to refuse that import. A planted config therefore ran inside `ultrafuzz ps`, the dashboard listing, the refresh inspection and the run's trusted-bin/smithers shim, which the finalizer calls with the engine's provider credentials. smithersCommandEnv now gives every engine process SMITHERS_BACKEND=sqlite, the shim exports it, and the detached engine, supervisor and relaunch inherit it. For a sealed runner the pin also removes a failure: its confinement refused the import and failed the command. Launch now binds the installed runner in its preflight, so an unpatched install, a missing Bun or an engine inside the target project fails with RUN_PREFLIGHT_FAILED before the run directory, the controller install or any registry access. The installed runner is refused inside the target project, as an explicit SMITHERS_BIN already was. The refusal gives a count instead of 2.7 KB of patch ids and says to reinstall and rebuild in the repository checkout, because a stale build is refused the same way; doctor shares that hint and the unapplied-patch filter. A long-lived process whose cached runner directory a pnpm install deleted is told to restart: re-resolving cannot follow the move, because Node and Bun cache import.meta.resolve. Also deletes launch-controller code only the removed per-command path used (the TMPDIR controller's Bun startup controls and their seal entries, writeCurrentBunStartupControls, a never-passed control parameter and its signal plumbing) and a dead ULTRAFUZZ_TRUSTED_BIN store in resume. The launch test now asserts that the snapshot never follows @ultrafuzz/runtime's smthrs edge. The dedicated-checkout guidance names the real trigger: any install that changes the engine's resolved dependency tree, not only a patch change, moves the engine. Co-Authored-By: Claude Opus 5.5 <[email protected]>
| const resolved = await loadResolvedProject({ projectRoot, env }); | ||
| const references = referencesStatus({ projectRoot }); | ||
| const installation = inspectSmithersInstallation(projectRoot); | ||
| const installation = inspectSmithersInstallation(); |
There was a problem hiding this comment.
Doctor misses target-contained runner When Ultrafuzz’s checkout is inside the target project, doctor reports the installed engine as healthy, but launch and resume reject it because the runner is inside the target. This leaves operators without a diagnosis of why those commands fail. Have doctor check the runner against the project root too.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/doctor.ts
Line: 69
Comment:
**Doctor misses target-contained runner** When Ultrafuzz’s checkout is inside the target project, doctor reports the installed engine as healthy, but launch and resume reject it because the runner is inside the target. This leaves operators without a diagnosis of why those commands fail. Have doctor check the runner against the project root too.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const trustedBin = ensureSafeDirectory(runRoot, "trusted-bin"); | ||
| writeFileDurable( | ||
| path.join(trustedBin, "smithers"), | ||
| `#!/bin/sh\nexport SMITHERS_BACKEND=sqlite\nexec ${[capability.interpreter.path, ...BUN_TARGET_CONFIGURATION_GUARD_ARGS, capability.runner.path].map(quote).join(" ")} "$@"\n`, |
There was a problem hiding this comment.
Shim skips runner revalidation The shim records the installed runner’s pathname when it is written, then executes that pathname without checking it again. If the runner’s bytes change later, a workflow’s
smithers node call runs the changed bytes, unlike direct Ultrafuzz commands, which recheck the executable before each invocation. Revalidate the runner when the shim runs, or avoid describing shim calls as checked on every command.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/smithers.ts
Line: 6332
Comment:
**Shim skips runner revalidation** The shim records the installed runner’s pathname when it is written, then executes that pathname without checking it again. If the runner’s bytes change later, a workflow’s `smithers node` call runs the changed bytes, unlike direct Ultrafuzz commands, which recheck the executable before each invocation. Revalidate the runner when the shim runs, or avoid describing shim calls as checked on every command.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Step 1 of the #921 plan (the judged plan's PR-1 and PR-7). It changes which engine bytes a resumed run executes.
Owner decision: a pnpm checkout with the Smithers
patchedDependenciesis the only supported install, so packed and plain-npm installs are refused (#921 question 3), and long campaigns run from a dedicated checkout or worktree (#921 question 4), whichdocs/reference/cli.mdnow tells operators.Problem
Any Ultrafuzz process that ran a Smithers command without a sealed runner installed its own copy of the engine first. That covers
ultrafuzz resume(including an attach to a live run), its pre-resumeinspect,ultrafuzz psand the dashboard's run listing, the--refresh-controllerownership inspection, and commands on runs without a sealed snapshot. The install was a registrynpm installof the pinned closure (the vendored npm 11.19.0 with the--beforecutoff) intomkdtemp(os.tmpdir())/ultrafuzz-controller-*, followed by the 69-entry string-patch registry.On this host, measured on
maina46a496:npm_config_registrypointed at a listener that drops connections, the native-continuation integration test's resume opened 6 registry connections, and the test took 213 s instead of 9 s.ultrafuzz-controller-*roots have piled up in/tmp. A tmp cleaner that reaps a root also breaks the live engine's later supervisor relaunches (Pin required executables and runtime dependencies for detached runs #1143, point 5).smithersunresolvable (Pin required executables and runtime dependencies for detached runs #1143 point 1). The generated workflow calls a baresmithers nodeto rebuild final-report producer authority after a restart. The controller PATH carries no runner, so on a production-like PATH every restarted report attempt fails withExecutable not found in $PATH: "smithers".smthrsinstall was unpatched, so the real-Smithers tests ran different bytes from production, and three of them patched their own copy of the runner.Root cause
The Smithers compatibility patches existed only as a TypeScript registry applied at runtime to a freshly npm-installed tree. So every process that needed a runner had to create and patch one, and the repository install never had them.
Change
Commit 1:
build(runtime), ship the patches as pnpm patchedDependenciesscripts/smithers-patches.mjsgeneratespatches/[email protected]andpatches/@smthrs__{agents,cli,db,engine,scheduler}@0.35.0.patch(2,166 lines). It appliesSMITHERS_COMPATIBILITY_PATCHESto pristine copies of the pinned packages withapplySmithersCompatibilityPatches, the same function launch uses, and diffs them the waypnpm patch-commitdoes.pnpm-workspace.yamlgets thepatchedDependenciesentries, and the lockfile is refreshed.node scripts/smithers-patches.mjs --check. It fails when a committed patch file or its workspace entry no longer matches the registry. The registry stays the source of truth until a later step deletes it.patched-smithers-runner.tstest helper is deleted. Its leftover-process cleanup now matches the run ID, because the runner no longer lives below the test root.inputTokens527 = 120 fresh + 400 cache read + 7 cache write, with each component ininputTokenDetails. It used to expect upstream's 120, which production never reported.Commit 2:
feat(runtime)!, commands after launch run the installed runnerThe installed runner
installedWorkflowRunner()instead. That is thesmthrspackage@ultrafuzz/runtimeresolves, which pnpm has already patched. These commands are the ones listed under Problem.BUN_TARGET_CONFIGURATION_GUARD_ARGS(--config=/dev/null --no-env-file --no-install --no-addons), the constant fix(runtime): the Smithers supervisor can relaunch a crashed engine #1199 added for the unsealed native continuation, so it does not load the target'sbunfig.tomlor.env. The flags do not confine what the runner imports; commit 3 closes the one target import Smithers makes.the installed Bun runner ignores target startup files and still runs its attested bytescovers the flags. fix(runtime): the Smithers supervisor can relaunch a crashed engine #1199's narrower test (native operator continuations run Bun without the target repository's bunfig.toml or .env) repeated that test's setup and assertions, so it is deleted.docs/security.mdnow says so.import.meta.resolve, because once Bun has loadedsmthrs, itsrequire.resolve("smthrs")returns the bare specifier. The Bun contract suite hit this.Deleted
.ultrafuzz-native-continuationretention of the TMPDIR root, and its exit-hook exemption.<run>/trusted-bin/smithersshimtrusted-binis first on the engine's PATH, so the workflow's baresmitherscalls resolve (Pin required executables and runtime dependencies for detached runs #1143 point 1).smthrsbecomes a runtime dependency of@ultrafuzz/runtime@toon-format/toon2.3.0, which@smthrs/cli0.35.0 pins exactly."@smthrs/cli>@toon-format/toon": "2.3.1", takes the patch release. With it, the engine tree adds no High/Critical advisory: the registry reports the same advisories formain's production tree as for this branch's (see Verification).packs.js,manifest.js). Ultrafuzz runs neither.smthrs. Each walker follows every declared dependency of the@ultrafuzz/*packages: launch's execution-snapshot walker and the trusted-CLI closure walker. Followingsmthrswould copy a second engine closure into every run's snapshot and into every trusted-CLI closure, which is re-hashed on eachultrafuzzcall. So both skip a first-party package'ssmthrsedge, and the sealed engine keeps using the snapshot's own root runner. This is one line in each walker, and both walkers are deleted in later steps (plan PR-8 and PR-10).validate:packallowBuildsmap into the consumer workspace.ultrafuzz doctorthere to report both workflow-engine checks asok.doctorok/errorchecks. Before, these checks were alwaysunknownand described a project-local tree launch never used.Docs and changelog
docs/reference/cli.md(resume, doctor) anddocs/security.md(workflow engine install).CHANGELOG.mdgets the entry under## Unreleased→ Breaking changes.Commit 3:
fix(runtime), review fixesPin the SQLite store for every engine process (the major review finding)
resolveSmithersBackendPreference(smthrs/src/resolveSmithersBackendChoice.js). Unless--backendorSMITHERS_BACKENDis set, that function runsawait import("<workspace>/.smithers/smithers.config.ts"). Ultrafuzz set neither, and the store-opening commands it runs (ps,inspect,node) all go through it.<target>/.smithers/smithers.config.tswrote its marker file fromrunSmithersInspectionCommand(["ps", …]), the code behindultrafuzz psand the dashboard's listing, and from<run>/trusted-bin/smithers ps. The shim's run hadANTHROPIC_API_KEYin its environment. Until fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 lands, the finalizer callssmithers nodethrough that shim with the engine's provider credentials.smithersCommandEnvnow setsSMITHERS_BACKEND=sqlitefor every engine process, and the shim exports it. Ultrafuzz's store is always<target>/smithers.db. With the pin, Smithers'.smithers/backend.jsonandmigrated.jsonmarkers no longer choose the backend, and it skips its PGlite probe....process.env, so they inherit the pin.installed-runner commands and the run's smithers shim never import the target's Smithers config, on the real installed runner. It fails if either pin is removed.Launch refuses an unusable installed runner before creating the run
beforeMaterialize) calls the newbindInstalledWorkflowRunner. This is the same binding the shim writer and every installed-runner command use: the patch guard, the outside-target check, the runner digest and Bun resolution.RUN_PREFLIGHT_FAILED. It happens before the run directory, the controller install or any registry access exists.The installed runner is refused inside the target project, as an explicit
SMITHERS_BINalready was. The check isassertExecutableOutsideRootinbindOperatorSmithersExecutableCapability, andcapability binding rejects target-contained runner and interpreter pathscovers it.Refusal text
…lacks Ultrafuzz's compatibility patches (<2.7 KB of id: posture pairs>); reinstall Ultrafuzz with pnpm install --frozen-lockfile.pnpm install --frozen-lockfileapplies no patches.installed workflow runner <path> lacks N of 71 Ultrafuzz compatibility patches (ultrafuzz doctor lists them); reinstall and rebuild Ultrafuzz in its repository checkout: pnpm install --frozen-lockfile && pnpm -w build.workflow-engine-*checks share the hint (WORKFLOW_RUNNER_REINSTALL_HINT) and the filter (unappliedCompatibilityPatches). The patches check still lists the patch ids.A long-lived process whose runner directory is gone is told to restart
eval runstill binds the deleted path.import.meta.resolve. I checked with a throwaway package: I re-pointed itsnode_modulessymlink and removed the old directory.import.meta.resolvein the same process still returned the old path, andrealpathfailed with ENOENT.installedWorkflowRunner()now throwsinstalled workflow runner <path> no longer exists, most likely because a pnpm install in Ultrafuzz's checkout replaced it; restart this Ultrafuzz process, instead of anlstatENOENT.docs/reference/cli.mdtells operators to restart the dashboard andeval runafter such an install.Dead code the per-command path left behind
controls/{bun-module-confinement.js,bun-empty.env,bunfig.toml}into the TMPDIR controller, and no longer hashes them into its in-process seal. Sealed snapshots take theirs from<run>/smithers/bun-startup-controls.writeCurrentBunStartupControlsis deleted.operatorControllerProjectRoot'scontrolparameter, which no caller passed, and thesignalplumbing inensureSmithersDependencies.ULTRAFUZZ_TRUSTED_BINjust before the shim writer sets it to the same directory.Tests
module:@ultrafuzz/runtimeissuer has nosmthrsedge. Before, disabling that walker exclusion only slowed the test (36 s instead of 7 s), because every snapshot then copied the engine closure.BaseCliAgent.js, the one file it changes.Docs
smthrs's directory from its full dependency path, which includes about 25 peers such as[email protected]and[email protected].@ultrafuzz/runtime;trusted-bin/smitherspoints into the install.docs/security.mdsays:validate:packcomment and doctor'soksummary no longer describe the guard as resume-only.Unchanged
pause,cancel,status,replayandforkon a sealed run still execute the run's sealed runner through the snapshot. They never installed a controller.Deliberately not built
pause/cancel/status/replay/forkto the installed runner.replayandforkload the sealed workflow, whose bare imports resolve to the snapshot's own smthrs, React and Effect copies.patches/smithers-patched-files.jsonin the plan).node_moduleswalk. The documented answer is to restart after such an install.PATH. The owner kept the shim; fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 removes the finalizer's own call.docs/security.mdsays agents can reach it.writeFakeInstalledEngine(project, …)setup in seven doctor tests. Doctor now reads only Ultrafuzz's install.lifecycle-inspection.test.tsconflicts with refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197 and test(runtime): delete vacuous and source-text tests, keep behavioural coverage #1203, so this waits until after the merge train..cmdshim.Verification
Review-fix head (a811973, on
maina46a496)Load 4–7. Every result below ran on this head's code, except for three later edits. One was a test-only lint fix,
assert.ok(run.value)in place of a non-null assertion, and the launch test was re-run after it. The other two were a code comment and changelog wording. The static checks were re-run after all three.smithers-executable-capability.test.tsandtrusted-cli.test.tslifecycle-inspection.test.ts,diagnoseProject|installation inspection|diagnoseRunruntime.test.tswith^startRun|operator controller locks|usage compatibility|resume|replay|fork|continuation|refresh|compatibility patch|patch anchors|envelope contracts|manifest parsing|package-manager-owneddoctor reports install posture in human and JSON outputbun test … '^Bun adapter contract:',umask 022)startRun injects the configured Forge guard into the workflow environment and metadata. With umask 0002 the guard'ssafe-binis created group-writable, andisPreparedForgeGuardBinrefuses a group-writable directory, so the guard'sforgenever reaches PATH. Withumask 022the test passes; I re-ran it together with the launch test (2/2). A reviewer saw it fail the same way onmain.provider-home ancestors cannot be group/world writable). Underumask 022that test passes.Mutation checks, run on the compiled test tree, which was recompiled afterwards:
SMITHERS_BACKENDpin insmithersCommandEnvpsassertionexport SMITHERS_BACKEND=sqliteassertExecutableOutsideRootin the operator bindingMissing expected exception)smthrsskipLaunch against an unpatched installed runner (a manual check; a test process cannot swap Ultrafuzz's own install)
smithers.jsto upstream's delegation line, then restored it byte-identically and confirmed with sha256 andsmithers-patches.mjs --check.startRunreturnedRUN_PREFLIGHT_FAILEDin 0.9 s:installed workflow runner … lacks 1 of 71 Ultrafuzz compatibility patches (ultrafuzz doctor lists them); reinstall and rebuild Ultrafuzz in its repository checkout: …. It left no run directory, and the fresh TMPDIR held noultrafuzz-controller-*root.ultrafuzz-controller-*root appeared).Planted
.smithers/smithers.config.tsrunSmithersInspectionCommand(ps)and<run>/trusted-bin/smithers psran it.SMITHERS_BACKEND=sqliteset by hand on the old shim,inspectandnodedid not import it either.Static checks, all passing
prettier --checkon the changed files, andformat:check;lint(complexity ceiling 83), andlint:strict:ciagainstorigin/main;knipwith everydist/dist-testmoved out of the worktree, and again afterpnpm -w build;pnpm --filter @ultrafuzz/runtime --filter @ultrafuzz/cli typecheck;docs:checkandnode scripts/docs-check.mjs;node scripts/smithers-patches.mjs --check;pnpm install --frozen-lockfile --offline, after deletingnode_modules/.pnpm-workspace-state-v1.json. Commit 3 changes no manifest, lockfile or patch file.Not re-run on this head:
validate:pack, the CLI e2e lane,bun test scripts/ci/*.test.ts, the full runtime shards, and the full CLI and modal suites. Commit 3 changes no packaging, CI script or e2e code; itsvalidate-packed-install.mjsedit is a comment.Before the review fixes (42a1bc8)
All on the rebased head (42a1bc8, on
maina46a496), at load 3–17 unless noted. Every test ran on this head's code; the only edits made after the test runs are to docs, the changelog, comments and thesmithers-patches.mjsfailure hint, and the static checks and--checkwere re-run after them.Patch files and install
mainchanged no registry entry since this branch's previous base (2cacf4c), andnode scripts/smithers-patches.mjsregenerates all six patch files byte-identically.node scripts/smithers-patches.mjs --checkis clean.pnpm install --frozen-lockfile --prefer-offlineinstalls the patched packages.pnpm install --frozen-lockfile --offlinepasses, including after deletingnode_modules/.pnpm-workspace-state-v1.json.applied.When pnpm deletes the directory a resumed engine runs from (a throwaway worktree of this branch; the docs' dedicated-checkout guidance rests on this)
pnpm installafter editingpatches/[email protected]adds a new[email protected]_patch_hash=…directory and keeps the old one as an orphan. pnpm skipped that install untilnode_modules/.pnpm-workspace-state-v1.jsonwas deleted.prunedAtinnode_modules/.modules.yamlis older thanmodules-cache-max-age(7 days by default).prunedAtset 8 days back, the patch-changing install itself deleted the old directory. So the deletion happens in that install or a later one, not always immediately.Discriminating test against
mainnative continuation keeps a finished producer and runs only a newly rendered downstream taskresumes viaresumeRun. Every proxy variable andnpm_config_registrypoint at a local listener that counts and drops each connection, and TMPDIR is fresh.<run>/trusted-bin;trusted-bin/smithers inspectanswersfinished.mainfails after 213 s withresume reached the package registry(6 connections).First Smithers command of a fresh process
runSmithersInspectionCommandwith no runner override, runninginspectof a missing run with a fresh TMPDIR, at load 14–15.maina46a496ultrafuzz-controller-*root per run. Aninspectdeletes it at exit; a resume keeps it.Suites
trusted-cli.test.tsandsmithers-executable-capability.test.tsruntime.test.ts, patterns for: compatibility patch, usage and cost compatibility, patched engine/runner admission, patch anchors and envelope contracts, manifest parsing, #1199's fd-transfer and module-confinement tests, native continuation, native resume, controller refresh, trusted CLI, resume/replay/fork delegation,startRunrunner install and target-local runnerslifecycle-inspection.test.ts,diagnoseProject|installation inspection|diagnoseRundoctor reports install posture in human and JSON outputbun test … '^Bun adapter contract:')generated OpenRouter adapter preserves opaque model IDs and enables the authenticated provider catalogue, fails the same way onmaina46a496:provider-home ancestors cannot be group/world writable. That is the host's umask 0002; it passes underumask 022(see the review-fix head above).bun test scripts/ci/*.test.tsvalidate:packcampaign-resume.test.ts): SIGKILL the engine, let the supervisor relaunch it, SIGKILL the controller, thenultrafuzz resumemain's CI run of a46a496; the hosts differ, so this is indicative only..envloading (dotenv: 'hostile').format:check,prettier --checkon the changed files,lint(undermain's complexity ceiling of 83),lint:strict:ciagainstorigin/main,knipbefore and afterpnpm -w build(now also checking unused exports, types and duplicates),docs:check,pnpm --filter @ultrafuzz/runtime --filter @ultrafuzz/cli typecheck, and the runtime and CLI test compiles all pass.security:dependency-advisoriesfails, onmaina46a496 as wellapproved registry advisory brace-expansion[2] duplicates GHSA-6j4f-fj2g-mc7p. The registry now returns that advisory twice, once per vulnerable range, and the gate's strict parser rejects duplicates.main's own CI run for a46a496 fails this step too.@toon-format/toonto 2.3.1.Not run: the full runtime shards and the full CLI and modal suites.
Earlier results that still apply (measured before this rebase, on
origin/integration/wave1a48ad9c):a real Smithers {fallback,quota} report retry survives producer and verifier restartstests give the engine a PATH whose onlysmithersis the shim. Both pass on this branch (in the integration run above). On the base, both failed withExecutable not found in $PATH: "smithers"/ "Smithers report-producer authority is unavailable" (Pin required executables and runtime dependencies for detached runs #1143), because the base had no shim writer.smthrsmoved todependenciesbut the exclusion removed, the trusted-CLI closure listssmthrsand the snapshot seals@smthrs/engine.validate:packnegative control: a consumer without the patch files fails as intended, with doctor reportingworkflow-engine-patches: error … lacks compatibility patches.security:dependency-advisoriesfailed on toon 2.3.0 before the override and passed after it.Rebase notes
Rebased from 2cacf4c onto
maina46a496, which adds #1202–#1215, #1219–#1222 and #1228. Commit 1 applied cleanly. Commit 2 conflicted in two files:docs/reference/cli.md(doctor section) vs refactor(runtime): give resume, replay and fork explicit result types #1208. Kept this PR's installed-engine wording and refactor(runtime): give resume, replay and fork explicit result types #1208's sentence that controller-root sizing stops after about one second. The refusal sentence now names launch,resume,replayandfork, since all four write the shim through the guard; it used to say onlyresume.packages/runtime/test/lifecycle-inspection.test.tsvs test(runtime): delete vacuous and source-text tests, keep behavioural coverage #1203. Kept this PR'sdiagnoseProjectand installation-inspection behaviour (tests read the installed runner, andinspectSmithersInstallation/patchedWorkflowRunnerare tested directly). Kept test(runtime): delete vacuous and source-text tests, keep behavioural coverage #1203's structure:projectWithFakeRunner()replaceslaunchedProject({})in the doctor tests, andinspectedRunandlaunchedProject(fixtures, runId)from test(runtime): delete vacuous and source-text tests, keep behavioural coverage #1203/fix(cli): recovery hints name ultrafuzz commands, not raw runner invocations #1207 are unchanged. DeleteddiagnoseProject does not mutate the installed dependency layout, as this PR already did: doctor no longer reads the project-local layout.Merged without a textual conflict and checked by hand:
packages/runtime/src/smithers.ts: this PR's hunks apply identically next to fix(cli): recovery hints name ultrafuzz commands, not raw runner invocations #1207'srunnerReportedError.start-run.ts(fix(runtime): a pnpm install elsewhere on the host no longer fails a launch #1206, refactor(runtime): give resume, replay and fork explicit result types #1208 result types),doctor.ts(refactor(runtime): give resume, replay and fork explicit result types #1208's sizing budget) andtrusted-cli-closure.ts(ci: fail on unused exports, types and duplicates #1219's unexported types).Changes made while promoting:
docs/reference/cli.mdand the changelog entry. The guidance states when pnpm deletes the old engine directory as measured above; the earlier claim that any patch-changing install deletes it at once was only true when the last prune was over seven days old.validate:packgave doctor. Its only purpose was to keep doctor from sizing a host's leftover controller roots, which stalled one run for over 18 minutes. refactor(runtime): give resume, replay and fork explicit result types #1208 now stops that sizing after about one second.resumeinstalls the engine from npm.scripts/smithers-patches.mjs(its header and its--checkfailure hint). After an earlier install, pnpm 11's optimistic repeat-install check ignores patch-file contents: a plainpnpm installprintedAlready up to dateand left the lockfile's patch hash stale, and a frozen install then failed withERR_PNPM_LOCKFILE_CONFIG_MISMATCH. With--config.optimistic-repeat-install=false, the install refreshed the hash (tested in a throwaway worktree).… changed while readingwhen pnpm hardlinks store files: fix(runtime): a pnpm install elsewhere on the host no longer fails a launch #1206 fixed it.Risk / compatibility
patchedDependencies.validate:packshows the supported packed path.--project, as they already refused an explicitSMITHERS_BINthere. Before this PR that layout worked, because the engine was installed in TMPDIR.bunon PATH to write the shim, even whenSMITHERS_BINpoints elsewhere.doctorreports the patch and layout problems too.pnpm installthere that changes the engine's resolved dependency tree moves the engine: a Smithers patch, or a version change anywhere in its dependency graph. pnpm deletes the old directory in that install or a later one (see Verification).trusted-bin/smithersfails.ultrafuzz resume, which also rewrites the shim.eval runcallsstartRunin-process for every row, and the dashboard also stays up; both resolve the engine once.… restart this Ultrafuzz process. The docs say to restart them after such an install.mainand now run the installed runner without it:psand the dashboard's listing;--refresh-controllerownership inspection;BUN_TARGET_CONFIGURATION_GUARD_ARGS, so Bun loads no targetbunfig.tomlor.env, and theSMITHERS_BACKEND=sqlitepin, so Smithers imports no targetsmithers.config.ts.smithers nodecall with the engine's full provider-credential environment, until fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 removes that call.trusted-binfirst), so besides the validator launcher they can runsmithersthrough the shim against their own run (ps,cancel,signal, …). Same-UID agents could already run the runner by absolute path.docs/security.mdsays so.SMITHERS_COMPATIBILITY_PATCHESmust regenerate the patch files and the lockfile's patch hashes (pnpm -w build && node scripts/smithers-patches.mjs && pnpm install --config.optimistic-repeat-install=false), or the new CI check or CI's frozen install fails.[email protected]patchedDependenciesentry and its lockfile hash.smithers-report-retry.integration.test.ts, whose engine PATH this PR changes.smithers.tsandruntime.test.ts.git merge-treeagainst refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, fix(runtime): record final-report producer selections in the run instead of querying smithers #1183 and feat(topology)!: a failed property lens no longer skips the rest of the campaign #1198 lists the same conflicted files for a811973 as for 42a1bc8:lifecycle-inspection.test.ts(one hunk),pnpm-lock.yamlandpnpm-workspace.yamlwith refactor!: remove per-node cloud execution (execution.mode = "cloud") #1197, andsmithers-report-retry.integration.test.ts(two hunks) with fix(runtime): record final-report producer selections in the run instead of querying smithers #1183. The hunk counts in those test files are also unchanged.Changelog entry
Added to
CHANGELOG.mdunder## Unreleased→ Breaking changes:Refs #921, #1143, #1147
🤖 Generated with Claude Code
The PR appears safe to merge, with non-blocking gaps in doctor’s diagnosis and shim invocation checks.
Fix with agent prompt
Summary
This PR moves post-launch Smithers commands to Ultrafuzz’s pnpm-patched installation while launch continues to seal a separate controller. It adds an installed-runner guard, a per-run
smithersshim, SQLite backend pinning, generated pnpm patches, and install guidance. Doctor does not yet diagnose one runner location that commands refuse, and shim invocations do not repeat direct commands’ executable checks.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Launch] --> B[Sealed controller snapshot] C[Resume and inspection] --> D[Installed patched runner] B --> E[Workflow] D --> E E --> F[trusted-bin/smithers shim] F --> DReviews (1) · Last reviewed commit: "fix(runtime): pin the SQLite store for e..."