fix(runtime): attempts abandoned by a crash are recorded, so stats and status agree - #1200
Merged
Merged
Conversation
When a controller dies mid-attempt, the resumed Smithers run marks the in-progress attempt row cancelled but emits no event for it (the engine's resume-cancel-stale-attempt path). terminalWorkflowAttempts dropped any start still open at the next RunStarted, so attempts.jsonl never recorded that attempt. After a controller SIGKILL and resume, `ultrafuzz stats` counted 3 agent attempts while `status` and the stub agent counted 4, and `ultrafuzz node` listed the interrupted node's first attempt as cancelled (#1139). A start still open at a RunStarted is now kept as abandoned. It is recorded as outcome and category canceled, with the message "abandoned: the controller stopped during this attempt, and the resumed run cancelled it", when its task has its next event, normally the replacement attempt's NodeStarted. That event is the terminal because one RunStarted can abandon several attempts, and each needs its own (workflow_run_id, source_event_sequence) identity. Agent provenance comes from the `smithers node` attempt row that synchronization already reads, so a pre-agent attempt is still skipped. An abandoned attempt whose number a reset reuses is recorded without the agent block, like other superseded occurrences. Nothing new can throw: an abandoned start whose task has no later event stays unrecorded, and a terminal stamped before its start is still skipped. The campaign-resume e2e todo is now a hard assertion that stats and status count the same agent attempts. Refs #1139 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Run state has no cancelled status: synchronization maps a Smithers `cancelled` task to failed, so `stats` counted nodes interrupted by `ultrafuzz cancel` as failed, while `status` counted the same tasks as other (#1087). stats now reports a failed node as canceled when every failed task of it has Smithers state `cancelled` in its recorded workflow provenance. That is the state `status` reads. The attempt ledger is not used for this: a task cancelled before Smithers selected an agent has no ledger row, and a cancellation while the verifier runs is recorded as a failed agent attempt. This adds `canceled` to the stats node status enum and to `totals.status_counts` in the CLI result schema; the envelope stays ultrafuzz.cli.result.v2 and stats stays ultrafuzz.stats.v1. Refs #1087 Co-Authored-By: Claude Opus 5.5 <[email protected]>
aggregateAttemptOutcome took the last attempts.jsonl row of each strategy attempt in file order. Rows are appended when they are recorded, so once a version records attempts an earlier one skipped, such as the crash-abandoned attempts recorded by the previous commit, the next sync of an already synchronized run appends the earlier attempt after its replacement. `stats` would then report the node's outcome as canceled although its last attempt succeeded. Within one workflow run the row with the higher source_event_sequence now wins; rows from different workflow runs keep file order. Refs #1139 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…n upgrade - stats: the #1087 test also covers a fan-out node, whose canonical state aggregates its strategy tasks and has no Smithers state. It now fails if the rule requires the aggregate state to be cancelled, or accepts any cancelled task instead of every one. - runtime: a finished, reported run that an earlier version synchronized without its abandoned attempt gets the canceled row appended on the next sync. That raises retry_count on the immutable node, and the same pass publishes the terminal report again, so status keeps reporting a verified report. The test fails on the base sync code and when the republish is skipped. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Comment on lines
+491
to
+495
| if (nodeState.status !== "failed" || provenance === undefined || !("workflow" in provenance)) return []; | ||
| const workflow = provenance.workflow; | ||
| return workflow !== undefined && "state" in workflow ? [workflow.state] : []; | ||
| }); | ||
| return failedTaskStates.length > 0 && failedTaskStates.every((state) => state === "cancelled"); |
There was a problem hiding this comment.
Failed tasks can disappear When a fan-out node has one task marked
cancelled and another failed task without a recorded workflow state, this check drops the second task. It then reports the whole node as canceled, hiding the failure in totals.status_counts. A failed task needs cancellation evidence before it can be counted as canceled.
Knowledge Base Used: CLI workflows
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/run-statistics.ts
Line: 491-495
Comment:
**Failed tasks can disappear** When a fan-out node has one task marked `cancelled` and another failed task without a recorded workflow state, this check drops the second task. It then reports the whole node as `canceled`, hiding the failure in `totals.status_counts`. A failed task needs cancellation evidence before it can be counted as canceled.
**Knowledge Base Used:** [CLI workflows](https://app.greptile.com/monad-foudnation/-/custom-context/knowledge-base/monad-developers/ultrafuzz/-/docs/cli-workflows.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
This was referenced Sep 29, 2026
aviggiano
added a commit
that referenced
this pull request
Sep 29, 2026
…1213) `stats` reports a failed node as `canceled` when Smithers cancelled every failed task of it (#1087, #1200). `workflowCancelled` in `packages/cli/src/run-statistics.ts` used a missing Smithers `state` as the mark of a fan-out aggregate record. So it also skipped a failed *task* record without a state. For a fan-out node with one cancelled task and one failed task without a state, it saw only `cancelled` and reported the node `canceled`. The failure disappeared from `totals.status_counts`, which contradicts `docs/reference/cli.md` ("When Smithers cancelled every failed task of a node"). A triage of the finding found no writer on `main`, or in past releases, that records a failed task without a state. So this fixes the rule, not an observed run. Co-Authored-By: Claude Opus 5.5 <[email protected]>
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The campaign-resume e2e test (
packages/cli/test/e2e/campaign-resume.test.ts) SIGKILLs the detached controller whilesummarizeruns, then resumes the run. Afterwardsultrafuzz statscounted 3 agent attempts, whilestatus(model_mix) and the stub agent counted 4. The test recorded this as atodosubtest. Smithers' own attempt rows, whichultrafuzz node node:summarize --attemptslists, hold two attempts for the node, the first onecancelled(observed in a run with this change; Ultrafuzz does not write those rows).attempts.jsonlheld only the second.Separately (#1087),
statscounted a node interrupted byultrafuzz cancelasfailed, whilestatuscounted the same task as other.Root cause
terminalWorkflowAttempts(packages/runtime/src/workflow-sync.ts) pairs eachNodeStartedwith a terminalNodeFinished,NodeFailedorNodeCancelledevent. At eachRunStartedit cleared the starts that were still open, and nothing recorded them. When a run resumes, Smithers 0.35.0 marks the dead activation's in-progress attempt rowscancelledin itsresume-cancel-stale-attempttransaction but emits no event for them. So the attempt the kill interrupted never reached the ledger, and fix(runtime): attempt and usage ledgers never block run synchronization #1186'sNodeCancelledhandling had no event to handle.statusFromWorkflowStatemaps a Smitherscancelledtask tofailed, andstatscounted run state.statuscounts Smithers' own task states, wherecancelledis other.Change
Source and schema +88/−39, tests +260/−15, docs +22/−7.
fix(runtime): record attempts a stopped controller abandoned as canceled. A start still open at aRunStartedis kept as abandoned. It is recorded when its task has its next event, normally the replacement attempt'sNodeStarted, with:canceled;abandoned: the controller stopped during this attempt, and the resumed run cancelled it;It needs its own terminal event because the ledger identity is
(workflow_run_id, source_event_sequence)and oneRunStartedcan abandon several attempts. The rest follows the existing rules:smithers nodeattempt row that synchronization already fetches, so a pre-agent attempt is still skipped.Normal pairing and the abandoned path share one
end()helper. The e2etodois now a hard assertion. Docs: the attempt-ledger section ofdocs/reference/artifacts-reports.md, and the e2e description indocs/reference/development.md.fix(cli): stats counts a node whose task Smithers cancelled as canceled (stats counts operator-cancelled nodes as failed while status counts them as other #1087). A node that run state records asfailedis reported, and counted intotals.status_counts, ascanceledwhen every failed task of it has Smithers statecancelledin its recordedprovenance.workflow.state. That is the Smithers task state thatstatuscounts. The CLI result schema gainscanceledin the stats node status enum and instatsStatusCounts. The envelope staysultrafuzz.cli.result.v2and stats staysultrafuzz.stats.v1. Docs:docs/reference/cli.md.fix(cli): stats takes a node's latest attempt in event order.aggregateAttemptOutcometook the last ledger row per strategy attempt in file order. After change 1, the next sync of a run that an earlier version already synchronized appends the abandoned row after the replacement's row. Without this change, that run'soutcomecolumn would readcanceledfor a node whose last attempt succeeded. Within one workflow run the highersource_event_sequencenow wins. Rows from different workflow runs keep file order.Deliberately not built
NodeCancelled{reason: "resumed"}for stale attempts. The events Ultrafuzz already reads are enough.smithers nodeattempt rows. The spec offered it as an alternative source. Those rows carry no event sequence to key the ledger identity on, and using them would add a second pairing path.NODE_STATE_STATUSES).statsderivescanceledfrom the recorded Smithers state instead, sostate.jsonand event schemas are unchanged.statusin both cases.attempt-source-event-joinsemantic gate is unchanged. It expectsNodeFailedfor every non-success outcome, so it would reject these rows, as it already rejects fix(runtime): attempt and usage ledgers never block run synchronization #1186'sNodeCancelled-terminated rows. No production code runs it. Nothing supplies itseventLogcontext, and no production caller runs any of thenode-attempt-ledger.schema.jsongates (the ledger reader has its own checks). Onlypackages/artifacts/test/semantic-gates.test.tsruns it. Deleting this one gate would leave the other ledger-schema gates, which no production code runs either, so deleting them together is a separate cleanup.NodeStarted,NodeFinished,NodeFailedorNodeCancelled. If the task's next event is some other type (NodeSkipped,NodePending,NodeRetrying) or never comes, the attempt stays unrecorded, as before.Verification
Discriminating tests, each run against
origin/integration/wave1sources by restoring the base file, running, then re-applying the change:syncRun records each attempt that a killed controller abandoned as canceled(new,packages/runtime/test/runtime.test.ts). Two tasks are left in progress by oneRunStartedand restarted as attempt 2. Two syncs must record onecanceledrow per task, with agent provenance, no diagnostics andretry_count1. Base: the ledger is[]. Fixed: passes.syncRun records an attempt abandoned at a run activation boundary even when a reset reuses its number. This replacessyncRun abandons an unterminated occurrence at a later run activation boundary, which asserted onlyokandrunning. Base:[]. Fixed: onecanceledrow without the agent block.stats counts a failed node whose task Smithers cancelled as canceled(new,packages/cli/test/run-statistics.test.ts). It covers a single-task node and a fan-out node. The fan-out node's canonical state aggregates two strategy tasks and has no Smithers state of its own. The test also validates the envelope against the closed CLI result schema. Base:[ 'failed', 1, undefined ]. Fixed:[ 'canceled', 0, 1 ]. With the fixedrun-statistics.tsand the base schema it also fails; the schema is the only difference. Two simpler rules fail it too:cancelled: the fan-out node whose tasks were both cancelled gets[ 'failed', 1, 0 ].[ 'canceled', 0, 1 ].stats reports a node's latest attempt outcome in event order, not ledger row order(new). With the baseaggregateAttemptOutcome, the case where the abandoned row comes after its replacement gets'canceled'instead of'succeeded'. Fixed: passes.syncRun records an abandoned attempt of an already reported run and publishes the report again(new,packages/runtime/test/runtime.test.ts). The first synchronization sees the event log without the abandonedNodeStarted. That reproduces what an earlier version recorded for a finished, reported run: ledger[[2, 3, 4, 'succeeded']](attempt, start, terminal, outcome),retry_count0 and a verified report. The second synchronization sees the full log. It must append[1, 1, 3, 'canceled']after the replacement's row and raiseretry_countto 1. The report publication status thatstatusreads must stayavailableandverified, andloadCurrentFinalReportSnapshotmust still returnverified-runtime-report. Base: the ledger keeps one row andretry_countstays 0. Two mutants fail it withterminal report receipt does not match current controller evidence: one skips the terminal-report republish once a publication status exists, and one stops an immutable node'sretry_countfrom following the ledger.campaign-resume.test.ts. Base: a probe copy of this test with the same flow printed per-nodeattempt_count1/1/1 (3) against 4statusattempts,failure_categories[]forsummarize, and the todo failing with3 !== 4. Fixed: the test passes with the former todo as a hard assertion, on e9ee6dd (351 s) and on e64c6db (384 s), the last commit that changes source; the later commit adds tests only. The fixed run's ledger showssummarizeattempt 1canceledfrom sequence 39 (itsNodeStarted) to 45 (attempt 2'sNodeStarted), with agent chain index 0; attempt 2 succeeded 45 to 57.statsgavesummarizeattempt_count2,retry_count1 andfailure_categories["canceled"].ultrafuzz nodelisted attempt 1 as cancelled, finishing 0.25 s before the ledger'sfinished_at.Existing tests:
attempt|ledger|occurrence|supersed|activation|abandon|cancelran on the final HEAD: 48 tests, including the new one; 46 pass and 2 are skipped (Bun-only adapter contracts).run-statistics,stats-commandandcli-contractstest files pass (34 tests).prettier --checkon the changed files,eslint,pnpm -w lint,CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/integration/wave1 pnpm -w lint:strict:ci,pnpm --filter @ultrafuzz/runtime --filter @ultrafuzz/cli typecheck,pnpm -w knipandnode scripts/docs-check.mjspass. The complexity ceiling is unchanged. The repository maximum is still 90 (synchronizeLinkedWorkflowRun, not touched), and the changed functions measure 2 to 37.Risk / compatibility
attempts.jsonlgains acanceledrow for each crash-abandoned attempt. Pre-agent attempts are skipped as before, unless a reset supersedes them. Those rows count towardretry_count,statsattempt_count, executed-attempt counts,ultrafuzz inspectattempt summaries and the eval recovery metrics (observed_node_attempts,repeated_model_backed_node_executions). All of these undercounted before.finished_at, andstatsduration_msfor the node, include the time the controller was down. Smithers' own attempt row ends at the resume too: 0.25 s earlier in the e2e run.outcomecolumn correct. The other ledger readers I found (retry and attempt counts,inspectsummaries, eval recovery metrics) count rows and do not depend on order.status,stats,inspectandwhyeach run one) also raisesretry_countinstate.jsonfor the node that lost an attempt. This happens even when the node's successful publication is immutable, because the retry count follows the ledger. The raisedretry_count, and theworkflow-syncedevent that the same synchronization appends, both make the published terminal report stale. The report publication status is keyed to a state fingerprint that includesretry_count, so the same synchronization publishes the report again: a newreview/runtime-report/<generation>, a rewrittencurrent.json, and a rewrittenreview/report-publication.json. The report stays verified andstatuskeeps reporting it available. Theretry_countchange is what triggers that republish; the new runtime test fails without it.cancelednode status and acanceledkey intotals.status_counts. A consumer that validates stats output against the previous closed schema will reject the new key.canceledonly when every failed task of it has recorded Smithers statecancelled. A failed node whose provenance has no Smithers state (the field is optional) keeps reportingfailed.Changelog entry
attempts.jsonlascanceledonce the resumed run restarts or cancels its task. Smithers cancels such attempts on resume without emitting an event, so they were missing before, and after a crash and resumestatsnow counts the same agent attempts asstatus.statsalso reports a failed node whose task Smithers cancelled, for example byultrafuzz cancel, ascanceledinstead offailed, and takes a node's latest attempt outcome in event order (Attempt and cancellation ledgers are incomplete after recovery #1139, stats counts operator-cancelled nodes as failed while status counts them as other #1087).Refs #1139, #1087
🤖 Generated with Claude Code
The PR should not merge until
statspreserves failures when a fan-out task lacks cancellation evidence.Fix with agent prompt
Summary
The PR records controller-abandoned attempts on resume, aligns
statscancellation counts with Smithers task state, and selects latest attempt outcomes by event sequence.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[RunStarted after controller stop] --> B[Retain open attempt by task] B --> C[Next task event] C --> D[Append canceled attempt to ledger] D --> E[stats attempt counts and outcome] F[Stored task workflow states] --> G[stats node status] G --> H[status counts]Reviews (1) · Last reviewed commit: "test: cover a cancelled fan-out node and..."