[aviggiano]

Root cause

A terminally failed workflow whose failing smithers task was the artifact preparation wrapper left no trace on any durable node. packages/runtime/src/workflow-sync.ts built its per-attempt evidence map from exactly two smithers node ids — node:<attemptId> and verify:<attemptId>. prepare:<attemptId> was never looked up. A failed prepare: wrapper is precisely the state in which the agent task it gates never starts, so completionEvidenceForTask saw agentEvidence === undefined, returned undefined, and the caller continued without touching the node.

The result is the R43 durable state: run failed, zero failed nodes, and a node still "status":"pending","wait_reason":"ready","next_eligible_action":"dispatch". Nothing is resettable, so every resume re-finalizes identically. prepareArtifactMirror throws from ~8 sites and is rendered with retries={0}, so this is reachable from any artifact-contract violation during preparation.

R43 primary evidence, identical across all three recovery generations:

✗ prepare:property-specification-fanin (attempt 1): artifact-contract failure:
    artifact dependency has not passed verification property-specification-crytic for property-specification-fanin
✗ Run failed: Task failed: prepare:property-specification-fanin
key: "prepare:property-specification-fanin::0"

Two corrections to the original triage, both confirmed in code.

1. The stated chain blamed parseStoredTask dropping records with no logicalNodeId. That is not the mechanism: parseStoredTask parses <runRoot>/smithers/tasks.json, the ultrafuzz compiled task manifest (one record per agentic attempt), and wrapper tasks were never rows in it. The drop happens one layer down, in synchronizeTasks' evidence lookup.
2. The "or verify" half was already handled on main. When the agent succeeds and verify: fails, completionEvidenceForTask already returns the verifier evidence and the node is finalized failed. Only the prepare half was missing. This PR preserves the verify behaviour byte-for-byte — same ARTIFACT_VERIFIER_FAILED code, same message text — and adds the prepare half.

The fix

Entirely inside packages/runtime/src/workflow-sync.ts.

synchronizeTasks now also merges evidence for prepare:<attemptId>. completionEvidenceForTask gained a leading branch: when the preparation wrapper ended terminally failed and the agent task is not itself terminal, the attempt is finalized from the wrapper's evidence with source: "preparation". finalizeTerminalTask generalizes its verifier branch to "wrapper" — category artifact-contract (an existing member of the closed PUBLIC_EVAL_FAILURE_CATEGORIES enum, so public eval diagnostics keep working with no cross-package schema change), diagnostic ARTIFACT_PREPARATION_FAILED, and causal_task_id naming the wrapper. The non-succeeded branch is pure computation with no filesystem effects: nothing is reset, deleted or re-verified.

The wrapper id is derived (prepare:${attemptId}) rather than made a required field of tasks.json, because parseStoredTask discards any record missing a required field — making it required would discard the manifest of exactly the already-stranded runs this is meant to recover. An assertion pinning the compiled "preparationId": "prepare:project-discovery" shape was added to the existing compile test so the two sides cannot drift.

As a structural backstop for the "or a typed diagnosable outcome" clause, a terminal failed/timed-out workflow that still records zero terminal-failed durable nodes emits WORKFLOW_TERMINAL_WITHOUT_FAILED_NODE naming the failing workflow tasks from failedChildKeys plus failed steps. Cancelled and paused states are excluded on purpose.

Explicitly not done, per the two banned approaches: no reset of verify:/node: tasks, no reads of the in-process invariant-suite Map, no marker deletion or re-sealing, and no rewind/timetravel/fork anywhere in the resume path. packages/runtime/src/smithers.ts is untouched.

What the review round changed

The backstop is now durable — this was the major finding. It only pushed onto the returned in-memory diagnostics array. Unlike the adjacent node-synced / workflow-synced paths it never called appendEvent and never touched state.json, and both automated consumers throw diagnostics away: the eval runner's defaultRowSync returns without recording whenever result.ok, and the Modal worker's runChecked explicitly drains the CLI child's stdout and stderr without persisting them ("Drain child output without persisting provider responses or benchmark contents"). So for the residual class the backstop exists to cover — a terminal failure charged to a task that is neither prepare: nor verify:, such as the ultrafuzz-agent-tasks parent that appears in the R43 log — a Modal generation still ended with state.json byte-identical to the pre-fix black hole. Only a human running ultrafuzz status/inspect and reading the JSON would ever have seen it.

It now appends a durable workflow-failure-unattributed run event with the workflow run id, the workflow state, the failing workflow task ids and the distinct durable node statuses. Ids only — no error text, so the no-credential-surface property holds. Because synchronization re-derives the backstop on every status/inspect, the append is deduplicated against what is already in events.jsonl on the (workflow_run_id, workflow_state, failed_workflow_tasks) triple: a repeatedly re-observed terminal failure records one event, not one per pass.

The backstop no longer reads only the in-pass status map. It was handed syncResult.nodeStatuses, which synchronizeTasks populates only for tasks that reach the bottom of its loop — it continues without recording anything when the graph node or the attempt's evidence is missing that pass. So on a resume where a previously-recorded failed node had no synchronizable evidence (inspect steps absent for it, smithers events having taken the WORKFLOW_EVENTS_FAILED path), the backstop reported "no failed durable node" as a severity-error diagnostic even though state.json attributed the failure. It now unions the in-pass map with readRunState(layout).nodes, which is the durable source of truth a crash-restart reader should trust.

A cancelled or stalled wrapper is no longer adopted as a genuine failure — all three reviewers raised this independently. PREPARATION_FAILURE_STATUSES was checked against the already-normalised NodeStatus, and statusFromWorkflowState maps cancelled/canceled/stuck onto failed. This is reachable without an operator cancel: sync's own deadline path calls requestSmithersCancel, and any later getRunStatus/inspectRun re-syncs. So every node whose preparation wrapper was in flight at cancel time — up to max-concurrency of them — was durably written as status: failed, last_error: "artifact preparation ended with status failed", provenance.failure.category: "artifact-contract", plus an ARTIFACT_PREPARATION_FAILED error diagnostic, and publicEvalFailedNodes then published that node with failure_category: artifact-contract. A cancelled or deadline-killed run was recorded, durably and publicly, as an artifact-contract violation it never committed — and because needsFinalization only re-fires on a status change, the record was never revised. The same mapping made a controller-loss stuck wrapper look like a terminal contract failure.

Attribution now additionally requires the wrapper's raw workflow state to be a genuine terminal failure (failed, error, timeout, timed-out, timedout, heartbeat-timeout). Cancellation and stalls fall through to the existing pending/cascade handling, exactly as on main.

That allow-list is exhaustive rather than approximate: statusFromWorkflowState produces NodeStatus "failed" from exactly {failed, error, cancelled, canceled, stuck} and "timed-out" from exactly {timeout, timed-out, timedout, heartbeat-timeout}, so the new gate is precisely the complement of {cancelled, canceled, stuck} within the states that can reach PREPARATION_FAILURE_STATUSES. No genuine failure shape is lost, and both event-sourced signatures the R43 recovery depends on — NodeFailed (failed) and TaskHeartbeatTimeout (timeout) — pass it.

A node that recovers no longer keeps a failure attribution it has outlived. The succeeded return of finalizeTerminalTask sets provenance without a failure key, and the caller merged it over withoutTerminalDisposition(previous?.provenance), which strips only terminal_disposition. So on the intended recovery path for this very issue — sync 1 records node A failed with failure.causal_task_id: "prepare:A", resume.ts resumes with --retry-failed, smithersSnapshotFailedTasks finds prepare:A::0 in failedChildKeys and timetravels it, prepare succeeds and A succeeds — node A ended succeeded while still carrying provenance.failure = {category: "artifact-contract", causal_task_id: "prepare:A"}. dependencyCascadeFailure walks task.dependencies and returns the first dependency that has a failure record, so any later node depending on A that is skipped for an unrelated reason was durably attributed to prepare:A / artifact-contract instead of its actual cause, and that category is what shipped in the public eval row. The mechanism pre-dates this change, but this change makes retry-then-succeed the expected recovery for prepare wrappers. The merge now drops a superseded failure when the node reaches a terminal success and the current finalization records no failure of its own.

Regression tests and red/green evidence

Six cases in packages/runtime/test/runtime.test.ts drive syncRun end to end against a fake smithers CLI.

Against pre-#272 main (77da78a): 5 of 6 fail.

not ok 73 - syncRun surfaces a terminal preparation wrapper failure as a failed durable node
not ok 75 - syncRun reports a typed diagnostic when a terminal workflow failure has no failed durable node
not ok 76 - syncRun records an unattributed terminal workflow failure durably and only once
not ok 78 - syncRun clears a preparation failure attribution once the node succeeds
not ok 79 - syncRun does not report an unattributed failure that state.json already attributes
# tests 142 / # pass 122 / # fail 5

Test 73's assertion message is the R43 signature verbatim: terminal failed run recorded no failed node: {"actors-flows":{… "status":"pending" … "wait_reason":"ready","next_eligible_action":"dispatch" …}, "project-discovery":{… "status":"succeeded" …}}. Test 75 failed with expected a typed diagnostic, got [].

Against the first commit of this PR (79a99c5), all four remediation tests fail:

not ok - syncRun records an unattributed terminal workflow failure durably and only once
not ok - syncRun leaves a cancelled preparation wrapper unattributed
not ok - syncRun clears a preparation failure attribution once the node succeeds
not ok - syncRun does not report an unattributed failure that state.json already attributes
# fail 4

syncRun leaves a cancelled preparation wrapper unattributed is green on pre-#272 main — of course it is, main never consults the wrapper, so the node simply stays pending. It is proved by mutation instead.

Every clause was mutated in turn, and each mutation killed exactly one test:

Mutation	Test that caught it
raw-workflow-state gate removed from the preparation branch	syncRun leaves a cancelled preparation wrapper unattributed
superseded-failure clearing reverted to withoutTerminalDisposition alone	syncRun clears a preparation failure attribution once the node succeeds
durable statuses no longer unioned into the backstop's input	syncRun does not report an unattributed failure that state.json already attributes
durable workflow-failure-unattributed append removed	syncRun records an unattributed terminal workflow failure durably and only once
append dedup disabled (if (true))	same test, on its second-sync assertion

Two further cases are honest guards, not regression tests, and passed before and after: syncRun keeps a preparation failure superseded by a later successful attempt (guards this change against over-firing) and the "preparationId": "prepare:project-discovery" assertion added to the existing compile test (a contract pin against drift).

Review findings

9 actionable findings across three lenses (correctness-regression, blast-radius-compatibility, durability-crash-restart); 1 major. Several were raised by more than one reviewer.

Addressed in code (6 distinct findings, covering 7 reports):

1. major (blast-radius) — the backstop is never persisted and is discarded by every automated consumer, so the "typed diagnosable outcome" half never reaches the post-mortem surface → durable workflow-failure-unattributed event, ids only, deduplicated. A reviewer also raised the unpersisted half as a separate minor under the correctness lens; same fix.
2. minor ×3 (all three lenses, independently) — a cancelled or stalled preparation wrapper is durably and publicly recorded as an artifact-contract failure of a node that never started → attribution now requires the wrapper's raw workflow state to be a genuine terminal failure.
3. minor (durability) — provenance.failure survives a successful retry of the same node and then misdirects every later dependency-cascade attribution → cleared on terminal success.
4. nit (durability) — the backstop reads the in-pass status map instead of durable state.json → the two are unioned.

Accepted as documentation rather than a code change (2 reports of 1 finding):

• minor ×2 (correctness + durability) — "wrapper-attributed failures flip the Modal terminal disposition from incomplete to operational-failure, which a publication gate treats differently." The reviewers are right, and the original PR's claim that this is "strictly better than the previous silent incomplete" was not verified. Because provenance.workflow.task_id becomes prepare:<attemptId>, hasExactTaskBinding (packages/modal/src/terminal-disposition.ts) fails and the node lands in bindingFailures — but it is now failed rather than pending, so incomplete drops to 0. Verified on the reviewers' fixture: classifyTerminalDisposition returns {kind:"incomplete",operationalFailures:3} pre-fix and {kind:"operational-failure",operationalFailures:1} post-fix whenever no node is left non-terminal. publicEvalDiagnosticsRowIsFailedDatapoint treats operational-failure rows as failed datapoints, which drops the workflow-not-scoreable / final-status-not-scoreable / terminal-disposition-not-scoreable reason codes and puts the row into failedDatapointRows in assertSmokeFindingFloor — precisely the set exempted from "smoke benchmark row must report at least one normalized finding". Concretely: a smoke run whose final node's prepare wrapper fails, with every other node terminal, previously blew up the bundle gate with zero findings and is now accepted as a legitimate failed datapoint. This does not bite the R43 shape itself (11 nodes stay pending there, so it remains incomplete).

  Kept as-is deliberately, and one reviewer explicitly offered this resolution. The alternative both reviewers floated — keep provenance.workflow.task_id = node:<attemptId> and carry the wrapper id only in causal_task_id — is worse: it satisfies hasExactTaskBinding, so an artifact-preparation infrastructure failure would be classified as a genuine task failure and scored as a real model outcome. Mis-attributing infrastructure to model quality is the exact class of error this workstream exists to remove. Verify-wrapper failures already behave this way on main, so this is a new trigger for an existing class rather than a new class, and the correct fix is the named follow-up: teach hasExactTaskBinding/hasCompletedTaskEvidence about prepare:/verify: ids and give wrapper failures their own non-scoreable kind. That belongs in packages/modal and is deliberately out of scope for the narrow runtime change.

No finding was silently dropped. Nothing was rejected as incorrect.

One reviewer finding was explicitly marked non-actionable and is recorded here as requested: the pending→failed flip counts as recovery progress and resets the Modal no-progress counter once. observedModalRecoveryProgress declares progress when the count of nodes in a terminal status increases across a generation, and recovery.ts resets no_progress_generations to 0 whenever progress is observed. So an already-stranded run resumed after this ships spends up to one extra Modal generation plus backoff before write-off. This cannot repeat unboundedly, and #122's livelock stays closed: once the node is failed it stays failed (a re-running prepare yields running/pending evidence, which returns undefined and leaves the node untouched), so each further reset of the counter requires a genuinely new node to reach a terminal status. The durable resume path is also unchanged — --retry-failed already reset failed wrapper tasks off failedChildKeys regardless of ultrafuzz node status, so this PR newly causes no timetravel reset.

Residual risk

• The named terminal-disposition follow-up is not done here (see the accepted finding above). Until it lands, a wrapper-attributed failure is classified operational-failure, which is a scoreable failed datapoint and therefore exempt from the smoke findings floor.
• repairModalEvalRunRecord still declines to finalize the eval row, because provenance.workflow.task_id is prepare:<attemptId> and hasExactTaskBinding requires node:<attemptId>. Identical to the pre-existing behaviour for verify-wrapper failures, and strictly better than the previous silent incomplete, but it is not a full end-to-end recovery.
• This surfaces and attributes the failure; it does not make the underlying prepare failure go away. If the cause recurs on every restart rather than being a one-off, resetting the now-visible failed node re-runs prepare and can fail again. fix(runtime): keep the verification marker consistent when sanitizing a property lens #279 fixed the known writer; this makes any recurrence diagnosable instead of a black hole.
• WORKFLOW_TERMINAL_WITHOUT_FAILED_NODE and workflow-failure-unattributed are a new diagnostic code and a new event type on the sync path. No existing test or consumer keys off diagnostic-array length or event-type counts, and neither changes ok or the run status — but a downstream tool that treats every error-severity sync diagnostic as fatal now sees one more code.
• timed-out is retained in PREPARATION_FAILURE_STATUSES. Because evidenceFromEvents flips status to timed-out on any error text matching /timeout|timed out|heartbeat/i, a prepare failure whose underlying fs/git error merely mentions a timeout makes finalRunStatus return timed-out rather than failed (the timed-out branch is checked first), changing the eval record's final_status. A genuinely timed-out preparation is a real failure worth surfacing, so this is kept; it mirrors the pre-existing verify-wrapper behaviour.
• Preparation wrappers exist only on the local <Worktree> execution branch. The cloud-sandbox branch calls prepareArtifactMirror inside the sandbox with no prepare: smithers task, so for cloud tasks the new lookup finds no step and no events and is a no-op. Cloud failures are unchanged by this work.
• The dedup read is replayEvents(layout), i.e. a full events-file scan — but only on a pass that actually has an unattributed terminal failure, which is rare by construction. replayEvents applies its default replay limit, so on a run with an extremely long event log the earlier record could fall outside the window and one duplicate record be appended. Bounded and cosmetic; the alternative (a new field on the run-state schema) would be a cross-package schema change for a diagnostic.

Validation

Ran from /home/ubuntu/uf-fix-272; df -h / checked before every gate (164–171G free on /, 66–67% used).

• pnpm -w run ci → exit 0 (format:check, lint, build across all 13 projects, validate:release).
• pnpm -w test → exit 0, all 13 packages green.
• pnpm -w typecheck → exit 0, all 13 workspace projects.
• pnpm -w lint, prettier --check on the touched files, git diff --check → all clean.
• Full runtime node suite (node scripts/run-tests.mjs, all 14 files) → 298 tests, 0 fail, 15 skipped.
• Focused node --test dist-test/test/runtime.test.js → 142 tests, 127 pass, 0 fail, 15 skipped (was 122 pass / 5 fail against pre-Resume terminal failed workflows with pending ready nodes #272 main).
• Mutation testing: 5 mutations, each killing exactly one test (table above). workflow-sync.ts was restored byte-for-byte after each.

Merge order

Merge #287 (fix/invariant-suite-handoff-durability, Fixes #217 and #219 (both already merged)) before this PR. Both are runtime fixes and both are in packages/runtime; #287 rewrites large parts of packages/runtime/src/templates/smithers/workflows/workflow.tsx, while this PR is the narrower change and touches only packages/runtime/src/workflow-sync.ts plus its test and the changelog. Landing the broader template change first and rebasing this onto it keeps the conflict surface to the changelog.

Relates to #272 — this surfaces the strand as a diagnosable failed node, but is not a full end-to-end recovery (see Residual risk). #272 stays open until a live resume confirms the failed node dispatches under --retry-failed.

Greptile Summary

The PR reconciles failed artifact-preparation wrappers into durable node failures and adds a durable fallback event for terminal workflow failures that cannot be attributed to a node. It also avoids attributing cancellations as artifact failures and clears stale failure provenance after successful recovery.

• Adds preparation-wrapper evidence to task synchronization.
• Persists deduplicated unattributed terminal-failure events.
• Adds end-to-end regression coverage for failure, cancellation, retry, and restart behavior.

Confidence Score: 4/5

The PR is not yet safe to merge because a terminal workflow-level failure can still remain absent from durable evidence whenever an unrelated node is skipped.

The new persistence backstop returns early for any skipped durable node without establishing that the skipped node attributes the workflow task that actually failed, so the archived run can still omit the terminal failure the fix is intended to preserve.

Files Needing Attention: packages/runtime/src/workflow-sync.ts

Important Files Changed

Filename	Overview
packages/runtime/src/workflow-sync.ts	Adds preparation-wrapper attribution and durable unattributed-failure handling, but the broad skipped-node suppression can still hide an unrelated workflow-level failure.
packages/runtime/test/runtime.test.ts	Adds substantial end-to-end coverage for preparation failures, cancellation, recovery, durability, deduplication, and restart state.
CHANGELOG.md	Documents the new preparation-failure attribution and durable terminal-failure diagnostic behavior.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Inspect Smithers workflow] --> B[Merge agent, verifier, and preparation evidence]
  B --> C{Preparation genuinely failed?}
  C -->|Yes| D[Finalize durable node as failed]
  C -->|No| E[Retain existing node outcome]
  D --> F[Read durable and in-pass node statuses]
  E --> F
  F --> G{Terminal workflow failure lacks matching node attribution?}
  G -->|Yes| H[Emit diagnostic and append durable unattributed-failure event]
  G -->|No| I[Complete synchronization]

Loading

Prompt To Fix All With AI

### Issue 1
packages/runtime/src/workflow-sync.ts:3097-3098
**Skipped nodes suppress unrelated failures**

When a terminal workflow-level task such as `ultrafuzz-agent-tasks` fails while any durable node is already `skipped`, this early return treats that dependency-cascade outcome as attribution for the unrelated failure. `WORKFLOW_TERMINAL_WITHOUT_FAILED_NODE` is therefore not emitted and no `workflow-failure-unattributed` event is appended, leaving the terminal failure absent from the archived run evidence.

---

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

_{Reviews (3): Last reviewed commit: "Merge remote-tracking branch 'origin/mai..." | Re-trigger Greptile}

Greptile also left 1 inline comment on this PR.

Context used:

• Knowledge Base — Runtime: Run Orchestration Lifecycle

[greptile-apps]

Skipped nodes suppress unrelated failures

When a terminal workflow-level task such as ultrafuzz-agent-tasks fails while any durable node is already skipped, this early return treats that dependency-cascade outcome as attribution for the unrelated failure. WORKFLOW_TERMINAL_WITHOUT_FAILED_NODE is therefore not emitted and no workflow-failure-unattributed event is appended, leaving the terminal failure absent from the archived run evidence.

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/workflow-sync.ts
Line: 3097-3098

Comment:
**Skipped nodes suppress unrelated failures**

When a terminal workflow-level task such as `ultrafuzz-agent-tasks` fails while any durable node is already `skipped`, this early return treats that dependency-cascade outcome as attribution for the unrelated failure. `WORKFLOW_TERMINAL_WITHOUT_FAILED_NODE` is therefore not emitted and no `workflow-failure-unattributed` event is appended, leaving the terminal failure absent from the archived run evidence.

**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.