Skip to content

fix(runtime): recover runs stranded by an unverified dependency - #278

Closed
aviggiano wants to merge 3 commits into
mainfrom
fix/unverified-dependency-recovery
Closed

aviggiano wants to merge 3 commits into
mainfrom
fix/unverified-dependency-recovery

Conversation

@aviggiano

@aviggiano aviggiano commented Aug 6, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

The failure this recovers from

A published artifact that diverges from its verification marker fails the dependent's prepare: wrapper on every attempt. Because the failure lands on the wrapper rather than the node body, the durable state records the dependent as pending with last_error: null and zero failed nodes — so --retry-failed has nothing to reset and the run is stranded permanently.

R43 (aave-v4-v0012-main-issue270-r43-invariant-only) lost 21 completed nodes and ~83.3 M tokens / ~$482 this way. Byte-level evidence from its volume:

artifact marker sha256 on-disk sha256 match
properties/crytic.md 97b050b0…0538 97b050b0…0538 yes
findings.json 37517e5f…b570 37517e5f…b570 yes
properties/crytic.json 8cfef182…1328 19d3976b…3e9d NO

prepare:property-specification-fanin then failed identically on every recovery generation (artifact-contract failure: artifact dependency has not passed verification property-specification-crytic … verified dependency artifact changed properties/crytic.json) until the no-progress budget was exhausted.

Why reset the dependency rather than repair the marker

The sha is the anti-tampering seal, and in this product the artifacts are authored by the model under test. Re-sealing the marker from current bytes would let a post-verification rewrite through, so this deliberately does not do that. Resetting verify:<dep> and node:<dep> makes the real verifier run again over freshly produced bytes — one node re-run (~9 min for crytic) instead of stranding 21 completed nodes.

Quarantine rather than delete, because publishFileDurableExclusive → assertPublishedBytes throws existing-file-conflict if the re-run publishes different bytes over the stale file, and because the diverged bytes are the only remaining evidence of the divergence.

Scope: this is recovery, not prevention

The writer that rewrites an already-published artifact is not yet identified. Ruled out with source evidence in #275:

  • publishFileDurableExclusive — assertPublishedBytes throws on differing bytes (packages/artifacts/src/safe-paths.ts:201-204, :440-445)
  • reconcileTargets / copyRegularFileExclusive — both skip when the destination exists (packages/runtime/src/artifact-reconciliation.ts:99-120)
  • smithers worktree sync — only fetch + rebase in a separate worktree, and R43's rebase failed non-destructively
  • workspace-handoff — .ultrafuzz and artifacts are excluded via SENSITIVE_SEGMENTS

Since publication asserts byte equality, disk necessarily matched the marker when verify:property-specification-crytic completed at 08:59:42Z, so the rewrite happened afterwards, across the sandbox exit and three restore cycles before prepare:fanin ran at 09:41:14Z.

Known limitation

Resetting a dependency drops successful_nodes from 21 to 20 and back. The Modal overseer's progress ratchet is canonical.successful_nodes > next.successful_nodes (packages/modal/src/recovery.ts:458), so that round trip does not register as progress. If the rewrite recurs on every restart rather than being a one-off race, the no-progress budget could still exhaust. This is strictly better than a permanent hard stop, but it is not a guarantee, and prevention still matters.

Validation

  • new regression proven red without the source change and green with it on this branch, so it is not vacuous
  • pnpm --filter @ultrafuzz/runtime test
  • pnpm --filter @ultrafuzz/runtime typecheck
  • npx eslint packages/runtime/src/smithers.ts packages/runtime/test/runtime.test.ts
  • npx prettier --check on both files
  • git diff --check

R36, R37 and R43 were never stopped or restarted; R43's overseer is still running and its evidence is preserved.

Greptile Summary

The PR adds resume-time recovery for dependencies whose published artifacts no longer match their verification markers.

  • Parses dependency verification failures from Smithers inspection output.
  • Rechecks marker hashes against durable artifacts before acting.
  • Resets the dependency verifier and records recovery evidence.
  • Adds regression coverage for divergence, stale errors, in-flight resets, and active workflows.

Confidence Score: 3/5

This PR is not safe to merge because recovery can re-attest post-verification artifact modifications without rerunning the artifact-producing node.

Resetting only verify:<dependency> causes the verifier to delete the old marker, validate the existing changed bytes, and issue a new marker whenever those bytes remain contract-valid, defeating the anti-tampering guarantee.

Files Needing Attention: packages/runtime/src/smithers.ts

Security Review

The recovery resets only the verifier, so changed artifact bytes that remain contract-valid can be re-attested without rerunning their producing node. This defeats the verification marker’s anti-tampering boundary and must be corrected before merging.

Important Files Changed

Filename Overview
packages/runtime/src/smithers.ts Adds divergence recovery, but resetting only the verifier can re-attest post-verification artifact modifications that remain contract-valid.
packages/runtime/test/runtime.test.ts Adds broad recovery-path coverage, but the fixtures do not test a diverged artifact whose modified bytes still satisfy its contract.

Sequence Diagram

sequenceDiagram
  participant Resume
  participant Marker
  participant Verifier
  participant Artifact
  participant Dependent
  Resume->>Marker: Detect recorded SHA mismatch
  Resume->>Verifier: Reset verify:dependency only
  Verifier->>Marker: Delete stale marker
  Verifier->>Artifact: Read changed bytes
  Verifier->>Verifier: Validate artifact contract
  Verifier->>Marker: Write marker for changed bytes
  Dependent->>Marker: Check new SHA
  Dependent->>Artifact: Accept changed bytes
Loading

Fix All in Claude Code

Prompt To Fix All With AI
### Issue 1
packages/runtime/src/smithers.ts:975-976
**Verifier re-seals modified artifacts**

When a previously verified artifact is rewritten to different but contract-valid bytes, recovery resets only `verify:<dependency>`, which validates the modified bytes and replaces the old marker without rerunning the producing node. Dependent tasks then accept artifacts that were modified after their original verification, and the old marker evidence is lost.

**How this was verified:** The reset targets only the verifier, whose generated implementation deletes the old marker, validates the existing bytes, and writes a new matching marker.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (3): Last reviewed commit: "fix(runtime): repair a diverged dependen..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

A published artifact that diverges from its verification marker fails the
dependent's `prepare:` wrapper on every attempt. Because the failure lands on
the wrapper rather than the node body, the durable state records the dependent
as pending with no error and zero failed nodes, so --retry-failed has nothing to
reset and the run is stranded permanently.

R43 lost 21 completed nodes and ~83M tokens this way: `properties/crytic.json`
was rewritten after `verify:property-specification-crytic` sealed it, so
`prepare:property-specification-fanin` failed identically on every recovery
generation until the budget was exhausted.

Reset the dependency's verify and node tasks so it re-runs and is genuinely
re-verified rather than waved through, and move its marker and every artifact it
declares into `.ultrafuzz-quarantine/` so the exclusive publish has a clean path
on the re-run. Quarantine rather than delete: the diverged bytes are the only
remaining evidence of the divergence.

This is recovery, not prevention. The writer that rewrites a published artifact
is not yet identified; the publish primitive, artifact reconciliation, worktree
sync and workspace handoff are all ruled out with source evidence in #275.

Refs #275

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread packages/runtime/src/smithers.ts Outdated
@aviggiano

Copy link
Copy Markdown
Collaborator Author

Verified against R43's real bytes: the detection does reach the resume side

The main risk with this PR was that it might be dead code — that the prepare: task's failure never reaches smithers inspect --format json, so the detection would never fire in production. That is the failure mode that got #273 drafted instead of merged, so I checked it directly rather than assuming.

1. The error is persisted at the run level. From R43's preserved smithers.db snapshot, _smithers_runs.error_json is 5470 bytes and contains the signature:

{"name":"SmithersError","code":"SESSION_ERROR",
 "message":"Task failed: prepare:property-specification-fanin ...", ...}

with the nested cause carrying artifact-contract failure: artifact dependency has not passed verification property-specification-crytic for property-specification-fanin.

2. inspect surfaces it. buildInspectSnapshot parses r.errorJson and splices it in as run.error whenever present (@smithers-orchestrator/cli/src/index.js:1576-1595):

let error = undefined;
if (r.errorJson) { try { error = JSON.parse(r.errorJson); } catch {} }
...
run: { id, workflow, status, ..., ...(error ? { error } : {}) },

3. The parser extracts the right dependency from those exact bytes. Feeding R43's real error_json into the production payload shape {ok, data:{run:{..., error}}} and running the regex from this PR:

EXTRACTED DEPENDENCIES: ["property-specification-crytic"]

So for R43 this fix would have quarantined property-specification-crytic's marker and artifacts and reset verify: + node: for it, instead of re-failing prepare:property-specification-fanin on every generation until the budget exhausted. It is not dead code.

Note this also means steps[] carrying no per-task error field is irrelevant here — the signal arrives via run.error, which is why the detection scans the serialized snapshot rather than the step rows.

Validation status

  • full detached pnpm --filter @ultrafuzz/runtime test: 272 pass / 0 fail, exit 0
  • new regression proven red without the source change, green with it, on this branch
  • prettier, eslint, git diff --check, typecheck: clean

aviggiano and others added 2 commits August 6, 2026 11:29
Review findings on the first commit, all of which made the recovery unsafe.

The trigger was sticky error text on the run row while the action was
irreversible deletion, with nothing checking the divergence still existed. A
recovered run that later stalled for an unrelated reason would have its correct
artifacts quarantined and an expensive agent node re-run once per generation,
driving a healthy run toward no-progress budget exhaustion. Recovery now
re-derives the divergence from the marker and the actual on-disk sha256 and acts
only on a real mismatch, or on a missing or unparseable marker.

Quarantine now covers the union of the marker's artifacts and publications.
Publications is a strict superset: it also carries generated-test companions and
invariant-suite files plus the suite manifest. Leaving those behind makes the
re-run's exclusive publish throw existing-file-conflict, trading one permanent
stall for another, and the invariant-only topology contains exactly those node
classes.

Also: check reset exit codes and roll the quarantine back on failure, so a
throw can no longer leave the only copies moved away with nothing reset; skip
recovery while the workflow is active, matching the failed-task retry above;
harden the quarantine root against symlink pre-creation the way the
verification-marker root already is, because model-controlled workspaces live
inside the run root; and give each recovery its own quarantine subdirectory so a
second occurrence cannot overwrite preserved evidence.

Corrects the rationale comment: R43's `prepare:property-specification-fanin` was
`state='failed'` in the workflow DB and the retry loop did reset it on all three
recovery generations. The zero-failed-nodes observation was true of the
Ultrafuzz durable state, not the workflow backend's. Completing the resume still
relies on that retry to reset the dependent, because `--no-deps` here does not
invalidate dependents.

Regression coverage: publication-only companion paths are quarantined, a stale
error with matching shas is a no-op, a failed reset restores the quarantine, and
an active run is skipped.

Refs #275

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial review found the quarantine design looping on itself. Recovery moved
the verification marker aside, and the currency check read that same marker to
decide whether to act, so once it fired the marker was gone, every later resume
read "still diverged", and the dependency was reset again each generation. With
`row.successful_nodes` a high-water mark and `baseline_successful_nodes` taken
from it, no such generation could ever be credited with progress, so the run
still died at the no-progress budget after burning three agent executions.

`verifyArtifacts` clears its own stale marker before validating, so resetting
`verify:<dependency>` alone is sufficient and correct: it re-reads the artifacts,
re-checks every contract, republishes identical bytes as a no-op, and writes a
marker matching what is on disk. Verification is genuinely re-performed rather
than re-sealed, and no agent re-runs - R43's verify tasks took 1-2s each against
~9 minutes for the lens node.

That removes the quarantine machinery entirely: no destructive renames, no
rollback, no directory sweep, no quarantine root to harden, and nothing
accumulating in the run root on the durable volume.

Two guards keep it from repeating: the divergence is re-derived from the marker
and the on-disk sha256 rather than trusted from error text that outlives the
failure it describes, and a dependency whose verifier is no longer finished is
skipped because recovery is already in flight.

Regression coverage: a real divergence resets only the verifier and never the
node, a stale error with matching digests is declined and recorded, an
already-reset verifier does not re-fire, and an active run is skipped.

Refs #275

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment on lines +975 to +976
"--no-deps",
"--force",

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Verifier re-seals modified artifacts

When a previously verified artifact is rewritten to different but contract-valid bytes, recovery resets only verify:<dependency>, which validates the modified bytes and replaces the old marker without rerunning the producing node. Dependent tasks then accept artifacts that were modified after their original verification, and the old marker evidence is lost.

How this was verified: The reset targets only the verifier, whose generated implementation deletes the old marker, validates the existing bytes, and writes a new matching marker.

Knowledge Base Used: Runtime: Run Orchestration Lifecycle

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/smithers.ts
Line: 975-976

Comment:
**Verifier re-seals modified artifacts**

When a previously verified artifact is rewritten to different but contract-valid bytes, recovery resets only `verify:<dependency>`, which validates the modified bytes and replaces the old marker without rerunning the producing node. Dependent tasks then accept artifacts that were modified after their original verification, and the old marker evidence is lost.

**How this was verified:** The reset targets only the verifier, whose generated implementation deletes the old marker, validates the existing bytes, and writes a new matching marker.

**Knowledge Base Used:** [Runtime: Run Orchestration Lifecycle](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/runtime.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code

@aviggiano

Copy link
Copy Markdown
Collaborator Author

Closing unmerged. It targets #275, which was closed on 2026-08-06 by PR #279 (77da78a). The Greptile review scored it 3/5 with "not safe to merge… defeating the anti-tampering guarantee", and the body advertises a node: reset and artifact quarantine that the diff does not contain — the only quarantin string in it is a comment.

smithersSnapshotUnverifiedDependencies is genuinely useful and was proven against R43's real error_json; it is tracked for extraction in #324. The recovery path itself is #288's job.

Tracked in #327.

@aviggiano aviggiano closed this Aug 7, 2026
aviggiano added a commit that referenced this pull request Aug 7, 2026
…and #278 (#338)

Recovers the four pieces of #273/#278 that survived review, onto current
main, without their refuted premises.

- Adopt the deterministic replacement lineage when a fresh submission
  reports RUN_EXISTS. An earlier recovery generation can create the
  replacement run and die before the new lineage is persisted; every
  later generation then recomputes the same id and wedges on exit 4.
- Drop the rewind to the latest timeline frame. jumpToFrame returns
  early with zero mutation when the target equals the latest frame
  (time-travel/src/jumpToFrame.js:812-824) and up --resume already
  un-terminalizes a failed run, so the timeline read and the rewind
  were two subprocesses per generation that changed nothing.
- Fall back to a top-level data.state / data.status in
  smithersSnapshotRunState, for snapshot shape variants that carry no
  runState or run wrapper.
- Add smithersSnapshotUnverifiedDependencies, which reads a dependent
  prepare: failure off the run row. Proven against R43's real
  error_json; the recovery built on it is #288's job.

The refuted premises are deliberately left out: no rewind to an earlier
frame, no verifier reset, no artifact quarantine.

Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant