Skip to content

fix(runtime): attempt and usage ledgers never block run synchronization - #1186

Merged
aviggiano merged 12 commits into
mainfrom
claude/w02-ledgers-never-block-sync
Sep 29, 2026
Merged

aviggiano merged 12 commits into
mainfrom
claude/w02-ledgers-never-block-sync

Conversation

@aviggiano

@aviggiano aviggiano commented Sep 29, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #1099
Refs #1139, #1138, #1087

Problem

attempts.jsonl and run.json#accounting could stop a campaign's run state from converging for bookkeeping reasons:

  • Terminal workflow stays running in status after unrecorded attempt supersession #1099. After a timetravel/retry-task reset, Smithers restarts attempt numbering at 1 and upserts the attempt row. If no sync had recorded the earlier occurrence, terminalWorkflowAttempts threw supersedes terminal event N before durable attempt recording in the authority pre-pass. syncRun then returned ok:false before any node or run state was reconciled, so a finished workflow stayed running/failed on every poll. This includes the product's own resume --retry-failed path after a pre-agent failure.
  • Attempt and cancellation ledgers are incomplete after recovery #1139. The ledger was rebuilt from the full event log on every pass, and every anomaly failed closed:
    • Cancelled attempts were never recorded.
    • Recorded rows were re-derived from mutable node status and finalization, and from the verifier's attempt number, then compared byte for byte. Any drift froze the task's ledger with already recorded with different immutable data.
    • A finished attempt that a host gate rejected got category unknown whenever no diagnostic code contained ARTIFACT. The in-sync join gate then rejected it, so it was never recorded.
    • smithers node ran for every terminal task on every pass, and any inspection failure failed the whole sync.
    • The fix(runtime): preserve launch policy and failed attempts during continuation #1117 pre-reset checkpoint could refuse resume --retry-failed/--reset-node itself.
  • Usage accounting can report complete while most worker sessions are absent #1138 crash window. Every pass asserted the stored run.json accounting against usage.jsonl before recomputing it. A pass stopped between the usage.jsonl append and the run.json write therefore made every later sync throw run.json#$.accounting.segments[0] does not exactly correspond to the usage ledger. That throw propagated out of syncRun, so status and the other syncing commands failed for the rest of the run.

Root cause

The ledgers were treated as independent, fail-closed authorities rather than as projections of Smithers' append-only event log, and their checks ran on the path that projects node and run status.

Change

Commits 1-4 follow the spec's order; commits 6-10 address the two adversarial reviews.

  1. Supersession is unconditional (Terminal workflow stays running in status after unrecorded attempt supersession #1099). A NodeStarted that reuses an attempt number supersedes the earlier occurrence. The fix(runtime): reconcile superseded attempt occurrences #952 authorization machinery is deleted: recordedTerminalSequences/authorizeUnrecordedSuperseded, the three trace/published-replacement authority helpers, TerminalAttemptSupersessionContext, and the authorityEvents parameter. This reverses Resume pre-sync rejects a superseded terminal occurrence after an attempt number is reused #951/fix(runtime): reconcile superseded attempt occurrences #952's rule that missing historical authority stays fail-closed.
  2. Record each attempt once; bookkeeping failures become warnings (Attempt and cancellation ledgers are incomplete after recovery #1139).
    • Pairing no longer throws on unpairable, duplicate or reused events. A superseded occurrence is marked superseded and kept, since its identity is its own terminal event sequence. A terminal with no live NodeStarted in the same activation is skipped. That covers a NodeCancelled persisted before its start, one after the attempt's terminal, and one with attempt: null; PR Reconcile invocation usage and attempt gaps #1155's version lacked this guard and bricked retry-failed.
    • Cancellations are recorded. NodeCancelled is a terminal with outcome/category canceled and the Smithers reason as its message.
    • Rows are keyed and never re-derived. Rows are keyed by (workflow_run_id, terminal sequence). appendNodeAttempts returns an existing identity unchanged. reconcileNodeAttemptLedgerEntry, the conflict throws and the in-sync semantic-gate re-check are deleted.
    • Classification fix. The current attempt is chosen by the agent task's own attempt number. A host rejection of a finished attempt is recorded as failed with invalid-output for findings validation and artifact-validation otherwise, never unknown.
    • Inspection only where needed. smithers node runs only for unrecorded, unsuperseded local occurrences. A superseded occurrence is recorded without agent, because Smithers' row now describes its replacement; a superseded finished occurrence that was not recorded before the reset is not recorded, because the output manifest now belongs to the replacement.
    • Failures are warnings. A failed inspection, an unresolvable selection or a failed append is now a warning (WORKFLOW_ATTEMPT_INSPECT_FAILED/NODE_ATTEMPT_LEDGER_WRITE_FAILED), and the node and run still reconcile.
    • Pre-reset checkpoint deleted. preserveFailedWorkflowAttemptsBeforeReset, its helpers and the beforeStoppedReset plumbing in start-run.ts/smithers.ts are gone. A superseded failure is now recorded from its own events.
  3. Accounting is a derived cache (Usage accounting can report complete while most worker sessions are absent #1138 crash window).
    • The stored run.json accounting is parsed best-effort, only for cached prices, and is always rebuilt from usage.jsonl.
    • A usage event is recorded once by identity and never re-derived. The in-sync usage-gate re-check and the post-append exactness throw are deleted.
    • A model that a successfully fetched catalog does not list is settled, instead of re-downloading the catalog on every sync.
    • The TokenUsageReported exact-key check is dropped. Per-field validation is kept, and after commit 8 it runs only where the usage ledger records a field.
    • An accounting failure is a WORKFLOW_ACCOUNTING_FAILED warning instead of an exception out of syncRun.
  4. Event-limit truncation is reported. smithers events --limit 100000 returns the oldest 100,000 events and gives no truncation marker under --json. The pinned 0.35.0 events command has no sequence cursor (--since is a time window; logs --from-seq prints human text capped at 1,000 lines). A stream that returns exactly the limit is therefore reported as a WORKFLOW_EVENTS_TRUNCATED warning.
  5. Docs (docs/reference/artifacts-reports.md) and the CHANGELOG.

Review fixes:

  1. A terminal stamped before its start is skipped. Pairing NodeCancelled could freeze a task's ledger. Smithers 0.35.0's finalizeCancelledRun takes its cancellation instant before its transaction and stamps NodeCancelled with it, so a start that commits meanwhile has the earlier sequence but the later timestamp. That row fails lifecycle.finished_at cannot precede lifecycle.started_at, and since a task's pending rows are appended together, no later attempt of the task was ever recorded. terminalWorkflowAttempts now skips such a terminal, like any other unpairable event (one condition). This inversion is argued from engine source and reproduced with the fake runner; it has not been observed in a live run.
  2. A finalized node's retry_count follows later-recorded attempts. With inspection failures now warnings, the pass that finalizes a node as succeeded can defer its occurrences and write retry_count from an incomplete ledger. The immutable-node branch then never patched the node again, so the undercount stayed for the whole run. That branch now writes retry_count alone when the ledger-derived count differs, and only once the ledger holds the current attempt as an executed occurrence. My first version lacked that guard. The existing fix(runtime): reconcile superseded attempt occurrences #952 test preserves a published replacement verified under a later activation caught it: after Smithers restarted the published attempt number with no terminal yet, the published attempt was counted as a retry. Status, provenance and failure state stay frozen. A reused node's current attempt is recorded as reused, so its count is never re-patched.
  3. Usage fields are validated inside the accounting pass. validateSmithersEventPayload re-checked every TokenUsageReported field while parsing the event streams, so a malformed field (for example a future costUsd: null) still threw out of syncRun. That copy is deleted, and the correlation-envelope check moves into normalizedUsageLedgerInput, where the same fields were already validated inside the accounting try/catch. A malformed usage event is now a WORKFLOW_ACCOUNTING_FAILED warning and is not recorded. The tautological self-check of freshly computed accounting against the ledger it came from is deleted too.
  4. matchesRedactedText and advanceCodePoints are deleted. Their only production caller was the deleted reconcileNodeAttemptLedgerEntry. PR refactor(artifacts): delete never-dispatched gates, production-dead code, and the unread event index #1181 does not remove them, and knip in CI does not check exports. w05c's hunks in sensitive-redaction.ts are elsewhere in the file.
  5. Claims corrected.
    • The docs no longer say a pre-agent failure is never recorded. Once a reset supersedes it, it is recorded without agent and counts toward retry_count, and a test now pins that.
    • The docs state the superseded-finished rule as the code applies it.
    • Two code comments are corrected.
    • The CHANGELOG names which failures no longer stop synchronization.

Deliberately not built (and why)

  • Paging smithers events past 100,000. There is no sequence cursor in the pinned CLI. Time-window paging with --since would be fragile and not seq-exact, and a Smithers source patch would add patch surface. A persisted "incomplete" marker would need a run.json schema change (a new usage_incomplete_reasons code), which rotates the validator build identity. The warning is recomputed on every pass instead.
  • PR Reconcile invocation usage and attempt gaps #1155's reconciliation layer. Not built: no execution_reconciliation document, no inspect --pool, no read-time cross-invariants.
  • Crash-abandoned attempts. Smithers cancels in-progress rows at resume without emitting an event, so these stay unrecorded. That gap needs an upstream NodeCancelled{reason:"resumed"}.
  • Agent provenance for superseded occurrences. Smithers upserts the row, and no reader consumes the ledger's agent field, so it is not reconstructed. A superseded pre-agent failure therefore cannot be told apart from an executed failure (see Risk).
  • Deleting the smithers node pre-pass (follow-up). After this PR the pre-pass only feeds the ledger's agent field and the pre-agent exclusion. Removing it would delete WORKFLOW_ATTEMPT_INSPECT_FAILED, the deferral in commit 7's scenario and the per-pass re-inspection of pre-agent failures. That changes what the ledger means, which is beyond this spec.
  • Appending each occurrence separately. A task's pending rows are still appended in one call. Commit 6 removes the one Smithers event sequence the reviews found that produced an unappendable row. A pre-existing case remains: reusedSourceAttempt throws when a reused node's source has no recorded output. That can still defer a task's other new rows, with a warning each pass.
  • Skipping only the malformed usage event. The alternative to failing the accounting pass is to skip the bad event and account the rest. run.json would then claim usage_complete while missing that event, and marking it incomplete needs the schema change above. So a malformed usage event stops further usage recording for the run, with a warning each pass (documented).
  • Contextual ledger gates (follow-up). attempt-reuse-source-link, attempt-source-event-join, usage-ledger-event-order and usage-ledger-source-event-join are still registered in artifact-schema-metadata.ts/semantic-gates.ts. Nothing in runtime or CLI supplies the context they need, and attempt-source-event-join would reject every canceled row if it were dispatched. Neither this PR nor refactor(artifacts): delete never-dispatched gates, production-dead code, and the unread event index #1181 removes them; deleting them touches contract metadata.
  • lifecycle_trace_summary patch (follow-up). The patch and its mirror entry put AgentTraceSummary into the lifecycle event list to feed the trace-authority check that commit 1 deletes, and sync no longer reads it. It belongs to w09, which owns SMITHERS_COMPATIBILITY_PATCHES.
  • Out of scope:
    • The stats reader's own exact accounting check (assertRunMetadataAccountingUsageAuthority, CLI) is unchanged. stats syncs first, and that sync now rebuilds the cache.
    • cumulativeAccountingForSourceRun still asserts a continuation's source-run accounting against that run's usage.jsonl. A source run stopped in the crash window therefore makes each continuation accounting pass a WORKFLOW_ACCOUNTING_FAILED warning until the source run is synced again. Main threw in that case.
    • A failed smithers events --type token fetch or a malformed token-stream envelope still fails the pass, as on main. That is an evidence read (w03's area), not ledger bookkeeping.
    • The stats counts operator-cancelled nodes as failed while status counts them as other #1087 node-status mapping of cancellations is unchanged.

#1139 is referenced, not closed. Two of its acceptance criteria stay open: terminal attempt counts that reconcile with scheduler and adapter records (crash-abandoned attempts are not recorded), and distinct signal-termination and OOM outcomes. Two are partly met: cancellation and timeout are now distinct outcomes, and the only missing-history report is the event-limit warning. Repeated recovery is idempotent.

Verification

All tests below use the repo's fake-Smithers harness. None were run against a live Smithers or a real campaign.

Implementer's discriminating runs (commits 1-5). Each row passes on this branch and was checked against origin/main. The two adversarial reviewers reproduced the table independently.

Test (runtime.test.ts unless noted) origin/main
reconciles a finished workflow after an unrecorded failed / succeeded occurrence is reused ok:false, supersedes terminal event 2 before durable attempt recording
resume --retry-failed after a pre-agent failure keeps synchronizing the reused attempt same error
records an unrecorded failed occurrence superseded by a reused attempt number same error
tolerates duplicate active starts for one attempt identity ok:false, has multiple active NodeStarted events
reconciles node state when Smithers attempt detail is unavailable and records it later ok:false, WORKFLOW_ATTEMPT_INSPECT_FAILED (error)
records an attempt without agent provenance when Smithers selection does not match the sealed chain ok:false
resume resets a stopped run even when its attempt ledger holds an edited row WORKFLOW_LIFECYCLE_FAILED recorded failed attempt no longer matches its immutable event authority
keeps redacted failure state ... across credential rotation; never re-derives or re-inspects a recorded attempt NODE_ATTEMPT_LEDGER_WRITE_FAILED (the second also makes 2 smithers node calls where the branch makes 1)
keeps an immutable output-validation failure when its successful occurrence is superseded NODE_ATTEMPT_LEDGER_WRITE_FAILED (category unknown rejected by the join gate)
records a cancelled attempt with its Smithers reason ledger []
keeps a cancelled occurrence that a reset superseded before any sync ledger lacks the cancel
attributes a verifier rejection to the agent's own attempt and never re-derives it ledger [[1, failed, executor-error, "verifier process crashed"]], attempt 2 missing
rebuilds run.json accounting after a pass stopped between the usage and run.json writes throws run.json#$.accounting.segments[0] does not exactly correspond to the usage ledger
keeps a recorded usage row that this build would derive differently throws ... does not exactly match usage-ledger accounting
reports a malformed usage ledger without blocking run status throws usage ledger record 1 is invalid strict JSON
settles a model the fetched pricing catalog does not list instead of refetching it 2 catalog fetches, expected 1
warns that a Smithers event stream at the CLI event limit may be truncated no warning
artifacts.test.ts: node attempt ledger records an occurrence once and never re-derives it throws ... was already recorded with different immutable data

Four tests pass on main by design; they are regression guards:

Review-fix discriminating runs (commits 6-10). I ran the new and changed tests three ways. The same test file was compiled against the fixed source, against the PR-head (2b80ca7) workflow-sync.ts, and against an origin/main (b6dd1da) worktree. On main the only type error is the known .allowed→.known rename, and tsc still emits.

Test Fixed source PR-head workflow-sync.ts origin/main
syncRun skips a cancellation stamped before its attempt started and records the task's later attempts (new) pass fail: NODE_ATTEMPT_LEDGER_WRITE_FAILED $[0].lifecycle.finished_at cannot precede lifecycle.started_at pass (main never pairs NodeCancelled; regression guard)
syncRun keeps a succeeded node's retry count in step with attempts recorded after it finalized (new) pass fail: retry_count 0, expected 1 fail: first pass ok:false, WORKFLOW_ATTEMPT_INSPECT_FAILED (error)
syncRun preserves a published replacement verified under a later activation (existing, #952) pass pass not run
syncRun accepts the runner's correlation envelope and rejects a mismatched one (changed) pass fail: throws Smithers TokenUsageReported payload at line 2 correlation attempt disagrees with the reported usage same throw
syncRun accepts the pinned 0.35.0 usage payload and still bounds its new fields (changed) pass fail: throws Smithers TokenUsageReported payload at line 2 freshInputTokens is invalid same throw
resume --retry-failed after a pre-agent failure keeps synchronizing the reused attempt (now pins the recorded row and retry_count 1) pass pass (behaviour unchanged; the new assertions pin it) fail: ok:false, supersedes terminal event 2 before durable attempt recording

The #952 test also discriminates for commit 7's guard. With the unguarded version it failed (retry_count 1, expected 0), and it passes with the guard.

The PR-head run of the pinned-usage test first failed with WORKFLOW_SUBMISSION_FAILED sealed workflow execution file changed. My own concurrent typecheck in the same worktree caused that by rebuilding config/dist. The rerun failed for the reason shown.

Regression runs at the final head. I ran seven targeted --test-name-pattern groups at f58b79f, and all pass:

  • runtime.test.js: 53 tests. These cover:
    • every runtime test this PR adds or changes;
    • the five tests from the review fixes;
    • the accounting and pricing tests;
    • the reset/retry/refresh resume guards.
  • dynamic-lifecycle.test.js: 2/2, the retry/skip/timeout ledger tests.
  • cli.test.js: 2 stats tests (4 subtests), including the fallback for malformed events/token-events output.

An earlier round with the unguarded commit 7 passed everything except the #952 test above. packages/artifacts is unchanged by the review fixes, so I did not rerun its tests.

Checks at the final head. All pass:

  • prettier --check and eslint on the changed files.
  • CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci.
  • pnpm --filter @ultrafuzz/{runtime,security,artifacts,cli} typecheck.
  • pnpm -w knip.
  • node scripts/docs-check.mjs.
  • packages/security sensitive-redaction.test.js (14/14).

Not run: the full runtime or CLI suites (only the CLI and dynamic-lifecycle tests named above), the Bun adapter contracts, or a real Smithers campaign.

Risk / compatibility

  • Retry counts can change.
    • A failed, timed-out or cancelled occurrence that a reset superseded before it was recorded is now recorded without agent.
    • A pre-agent failure is never recorded while unsuperseded. Once resume --retry-failed or --reset-node supersedes it, it is recorded as an executed failed attempt, even if an earlier sync saw it as pre-agent. retry_count and executed-attempt counts can therefore be one higher per such reset than under main's fix(runtime): preserve launch policy and failed attempts during continuation #1117 checkpoint, which excluded it.
    • After commit 7, an immutable node's retry_count can change after the node finalized. That happens when the ledger holds its current executed attempt and attempts that the finalizing pass deferred have since been recorded.
  • The first recorded classification is final. For example, an agent attempt recorded as failed/artifact-validation stays so even if a verifier-only re-run later accepts the same output. A run cancelled while the verifier runs is also recorded as artifact-validation, unchanged from main.
  • No tamper detection on ledger rows. A hand-edited attempts.jsonl or usage.jsonl row is kept as written rather than reported.
  • Severity changes. WORKFLOW_ATTEMPT_INSPECT_FAILED and NODE_ATTEMPT_LEDGER_WRITE_FAILED are now warnings, and syncRun returns ok:true. WORKFLOW_ACCOUNTING_FAILED and WORKFLOW_EVENTS_TRUNCATED are new warning codes. An invalid TokenUsageReported field or a mismatched correlation envelope is now a WORKFLOW_ACCOUNTING_FAILED warning instead of a sync failure. For stats, whose sync tolerates invalid event streams, the sync now proceeds and reports that warning instead of being skipped with an invalid-event-stream warning. The malformed event itself still mutates nothing.
  • A malformed usage event stops further usage recording. Its accounting pass fails on every later sync, so usage.jsonl and run.json#accounting stay at their last good state for the rest of the run, with a warning each pass.
  • Pricing is settled for the run. A model a fetched catalog did not list stays unpriced for the rest of the run, with no TTL. Changing ULTRAFUZZ_PRICING_CATALOG_URL mid-run does not reprice it; this matches how the disabled status already behaved.
  • Unknown usage fields are ignored. A new upstream TokenUsageReported accounting field would be left out rather than failing sync. The pinned-runner drift test (CURRENT_SMITHERS_TOKEN_EVENT_KEY_CONTRACT.known) still flags it at the pin bump.
  • Pre-agent failures are re-inspected. They stay unrecorded while unsuperseded, so each pass re-inspects them. Steady-state inspection is therefore O(new attempts + unsuperseded pre-agent failures), not strictly O(new attempts).
  • resume --reset-node without smithers/tasks.json. Main refused it (workflow task manifest does not exist) as a side effect of the deleted fix(runtime): preserve launch policy and failed attempts during continuation #1117 checkpoint's evidence read. It now resets and relaunches, consistent with start-run treating a missing manifest as legacy. A reviewer verified this with a probe.
  • Removed exports. preserveFailedWorkflowAttemptsBeforeReset (runtime), reconcileNodeAttemptLedgerEntry (artifacts) and matchesRedactedText (security) were star-exported. None is imported by the generated workflow.tsx or anything else in the repo.
  • Merge overlap. I ran git merge-tree against every current claude/w* branch. Only w03 has a code conflict: one region in workflow-sync.ts around the deleted assertStoppedResetAuthorityUnchanged, which already existed before the review fixes. Nine branches conflict only on the CHANGELOG.md "Other changes" insertion point. w01, w05c, w15a and w19 merge cleanly.

🤖 Generated with Claude Code

RetriggerConfidence Score: 4/5

The PR does not yet appear safe to merge because a superseded pre-agent failure can still inflate retry counts.

Fix All in Claude CodeFindings

  1. P1 Pre-agent failures inflate retry counts ▶
Fix with agent prompt
### Issue 1
packages/runtime/src/workflow-sync.ts:undefined-4898
If `resume --retry-failed` reuses an attempt number before synchronization records an earlier pre-agent failure, Smithers’ replacement row no longer shows whether the earlier attempt selected an agent. This branch skips that check but records the failure as executed. The node’s `retry_count` and attempt summaries then count a model attempt that never ran.

---

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

Summary

The PR makes attempt and usage ledgers recoverable projections so bookkeeping failures no longer prevent node and run synchronization.

  • Records terminal occurrences once, including cancellations, while treating unavailable attempt detail as a warning.
  • Rebuilds accounting from the usage ledger and warns when Smithers event streams reach their limit.
  • Adds regression coverage and documents the changed ledger behavior.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  E[Smithers events] --> S[Run synchronization]
  S --> N[Node and run state]
  S --> A[Attempt ledger]
  E --> U[Usage ledger]
  U --> C[Rebuilt accounting cache]
  A -. bookkeeping warning .-> S
  C -. accounting warning .-> S
Loading

Reviews (4) · Last reviewed commit: "Merge origin/main into claude/w02-ledger..."

aviggiano and others added 5 commits September 29, 2026 00:23
…ence

Smithers restarts attempt numbering after a timetravel/retry-task reset and
upserts the (run, node, iteration, attempt) row, so a later NodeStarted can
reopen an identity whose earlier terminal occurrence no sync recorded.
terminalWorkflowAttempts threw in that case unless the occurrence was
already ledgered or passed the #952 trace/published-replacement proof. The
reducer runs in the authority pre-pass, so the throw returned ok:false
before any node or run state was reconciled: a finished workflow stayed
running/failed on every status poll, including after the supported
`resume --retry-failed` of a pre-agent failure (#1099).

Both allowed branches only deleted the superseded occurrence, and the proof
guarded the optional `agent` field of a ledger row that nothing reads. The
supersession is now unconditional, and the authorization machinery
(recordedTerminalSequences/authorizeUnrecordedSuperseded, the activation
tracking, the three trace/published-replacement authority helpers,
TerminalAttemptSupersessionContext and appendTerminalTaskAttempts'
authorityEvents parameter) is deleted. This reverses #951/#952's rule that
missing historical authority stays fail-closed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ger block sync

attempts.jsonl was rebuilt from the whole event log on every pass and every
anomaly failed closed (#1139):

- terminalWorkflowAttempts threw on duplicate starts and on terminals
  without a start, and only knew NodeFinished/NodeFailed, so cancelled
  attempts were never recorded;
- recorded rows were re-derived from mutable node status, finalization and
  the verifier's attempt number, then compared byte for byte, so any drift
  froze the task's ledger with "already recorded with different immutable
  data" on every later pass;
- a finished attempt rejected host-side got category "unknown" unless a
  diagnostic code contained "ARTIFACT", which the in-sync join gate then
  rejected, so the attempt was never recorded;
- the authority pre-pass ran `smithers node` for every terminal task on
  every pass and any failure returned ok:false before node or run state
  was reconciled.

Pairing now never throws: a NodeStarted that reuses an attempt number marks
the earlier occurrence superseded, a terminal without a live start in the
same activation (including a NodeCancelled for an attempt that already
ended, never started, or has `attempt: null`) is skipped, and NodeCancelled
is a terminal with outcome/category canceled and the Smithers reason. Rows
are keyed by (workflow_run_id, terminal sequence) and appended once; an
existing identity is returned without re-deriving or comparing it, and
reconcileNodeAttemptLedgerEntry plus the in-sync semantic-gate re-check are
deleted. The current attempt is chosen by the agent task's own attempt
number, and a host rejection of a finished attempt is failed with
invalid-output or artifact-validation. `smithers node` runs only for
unrecorded, unsuperseded local occurrences; a failed inspection, an
unresolvable selection or a failed append is a warning, and a superseded
occurrence is recorded without agent provenance because Smithers' row now
describes its replacement.

With superseded failures recorded from their own events, the #1117
pre-reset checkpoint is redundant: preserveFailedWorkflowAttemptsBeforeReset,
its helpers and the beforeStoppedReset plumbing are deleted, so bookkeeping
can no longer refuse `resume --retry-failed`/`--reset-node`.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every pass asserted the stored run.json accounting against usage.jsonl
before recomputing it. usage.jsonl is appended before run.json is
rewritten, so a pass stopped between the two writes (Ctrl-C on `status`,
a timeout, an OOM) left the ledger one step ahead, and every later sync
threw "run.json#$.accounting.segments[0] does not exactly correspond to
the usage ledger". That throw was not caught, so `status`, `stats`, `why`
and the eval poller failed for the rest of the run (#1138).

run.json accounting is now a cache derived from usage.jsonl: the stored
copy is parsed best-effort only for its cached prices and is always
rebuilt from the ledger. A usage event is recorded once by its Smithers
identity and never re-derived, so a row an earlier build wrote cannot
become a conflict; the in-sync usage-gate re-check and the post-append
exactness throw are deleted. A model that a successfully fetched pricing
catalog does not list is settled instead of re-downloading the catalog on
every sync. The TokenUsageReported exact-key check is dropped (per-field
validation stays), so an extra upstream key is ignored instead of failing
every sync. An accounting failure is now a WORKFLOW_ACCOUNTING_FAILED
warning and node and run status still reconcile.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…nt limit

Synchronization reads both event streams with `smithers events --limit
100000`. The pinned 0.35.0 CLI returns the oldest 100,000 matching events
and says nothing under --json when it stops there, so on a larger run every
later lifecycle or usage event silently never reached node evidence, the
attempt ledger or accounting. The `events` command has no sequence cursor
to page past the limit (`--since` is a time window and `logs --from-seq`
prints human text), so a stream that returns exactly the limit is now
reported as a WORKFLOW_EVENTS_TRUNCATED warning naming the last sequence
read.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Documents that each attempt is recorded once by its terminal Smithers
event, that cancelled and reset-superseded attempts are recorded, how host
rejections are categorized, that bookkeeping failures are warnings, that
run.json accounting is rebuilt from usage.jsonl, the pricing negative
cache, and the WORKFLOW_EVENTS_TRUNCATED warning.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aviggiano
aviggiano requested a review from a team as a code owner September 29, 2026 01:35
continue;
}
let agent: NodeAttemptAgentProvenance | undefined;
if (input.task.execution.mode === "local" && !attempt.superseded) {

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 Pre-agent failures inflate retry counts

If resume --retry-failed reuses an attempt number before synchronization records an earlier pre-agent failure, Smithers’ replacement row no longer shows whether the earlier attempt selected an agent. This branch skips that check but records the failure as executed. The node’s retry_count and attempt summaries then count a model attempt that never ran.

Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/workflow-sync.ts
Line: 4809

Comment:
**Pre-agent failures inflate retry counts**

If `resume --retry-failed` reuses an attempt number before synchronization records an earlier pre-agent failure, Smithers’ replacement row no longer shows whether the earlier attempt selected an agent. This branch skips that check but records the failure as executed. The node’s `retry_count` and attempt summaries then count a model attempt that never ran.

---

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

Fix in Claude Code

aviggiano and others added 5 commits September 29, 2026 03:55
Pairing NodeCancelled with the live NodeStarted could freeze a task's
attempt ledger. Smithers 0.35.0's finalizeCancelledRun takes its
cancellation instant before its transaction and stamps NodeCancelled
with it, so a task start that commits meanwhile has the earlier sequence
but the later timestamp. That row fails the ledger's "finished_at cannot
precede started_at" check, and because a task's pending rows are
appended together, every later attempt of the task stayed unrecorded,
with a warning on every pass.

terminalWorkflowAttempts now treats a terminal stamped before its live
start as unpairable and skips it, the rule it already applies to other
unpairable events; that attempt stays unrecorded like any abandoned
start. The inversion is argued from the engine source and reproduced
with the fake runner, not observed in a live run.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…recorded attempts

With attempt inspection failures demoted to warnings, the pass that
finalizes a node as succeeded can defer its occurrences (no `smithers
node` detail that pass) and write retry_count from an incomplete ledger.
The node is then immutable, and the immutable branch skipped the node
patch, so later passes recorded the deferred attempts but never updated
retry_count: state, status, the terminal report and evals kept the
undercount for the rest of the run. Invalid-output nodes, which are also
immutable, had the same gap.

The immutable branch now writes retry_count alone when the ledger-derived
count differs from the stored one, but only once the ledger holds the
current attempt as an executed occurrence. Without that guard a later
restart of the published attempt number, which has no terminal yet,
counted the published attempt as a retry; the existing "preserves a
published replacement verified under a later activation" test pins that
the node state stays unchanged there. A reused-from-prior-run node's
current attempt is recorded as reused, so its count is never re-patched.
The count is computed exactly as in the finalizing pass; status,
provenance and failure state stay frozen.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
validateSmithersEventPayload re-checked every TokenUsageReported usage
field while parsing the Smithers event streams, so a malformed usage
field (for example a future `costUsd: null`) still threw out of syncRun
before any node or run reconciliation. normalizedUsageLedgerInput
already validates the same fields where the usage ledger records them,
inside the accounting try/catch.

Delete the parse-time copy and move the correlation-envelope check into
normalizedUsageLedgerInput. A malformed usage event is now a
WORKFLOW_ACCOUNTING_FAILED warning and is not recorded, while node and
run status reconcile as they would with valid usage. Also delete the
self-check that asserted freshly computed accounting against the ledger
it was just computed from; the `stats` reader keeps its own check.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…pt-ledger change

reconcileNodeAttemptLedgerEntry, deleted earlier on this branch, was the
only production caller of matchesRedactedText and its helper
advanceCodePoints, so both are now referenced only by their own tests.
Delete them and those tests. redactedTextSpanCodePointLengths, which the
attempt ledger still uses, keeps its assertions.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
…nts as a retry

The docs said a pre-agent failure is not recorded. Once a reset
supersedes it, though, Smithers' attempt row describes the replacement,
so synchronization records the failure from its events without agent
provenance and it counts toward retry_count, even when an earlier sync
saw it as pre-agent. The docs also described the superseded-finished
rule more narrowly than the code applies it: any superseded finished
attempt not recorded before the reset is skipped.

Correct both, name the skipped terminal-before-start case and the
malformed-usage accounting warning, fix the matching code comments, and
pin the pre-agent behaviour in its test. The CHANGELOG entry now names
the failures that no longer stop synchronization instead of claiming all
usage bookkeeping, and states the retry-count consequence.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
aviggiano added a commit that referenced this pull request Sep 29, 2026
…on interrupt

Review fixes for the end-to-end campaign test.

- The no-rerun checks now run straight after the events poll, before any
  command synchronizes the run. Those checks cover the stub start counts,
  the RunStarted count, the untruncated event page and no NodeStarted after
  NodeFinished. First the test checks that the engine's first run-terminal
  event is RunFinished. A resume that re-runs a finished node now fails on
  the named count. Before, it failed with an opaque
  ARTIFACT_VERIFICATION_AUTHORITY_INVALID from the status call that came
  first.
- The stub now fails, and logs why, in three cases: it finds no output
  contract, a named authority file is missing, or the report render line
  is not all `--flag 'value'` pairs. Before, a missed contract match
  exited 0 with a successful Codex turn and wrote nothing. The test's
  "workflow stopped" and RunFinished failures include the stub's call log.
- SIGINT and SIGTERM handlers, and the exit hook, now SIGKILL the
  campaign's processes and delete the fixture; the signal handlers then
  re-raise. A Ctrl-C'd run used to leave the detached engine, the
  supervisor and the ~1 GB fixture behind.
- The wait for the held agent to exit counts a zombie as exited. kill(pid,
  0) succeeds on a zombie; its /proc cmdline is empty.
- Drops the stats `attempts_complete === true` pin. That field only
  says the attempt ledger exists, and pinning it would break a fix that
  reports the undercounted attempts as partial evidence.
- The todo reason now names the cause that outlasts #1186. Smithers
  emits no terminal event for the attempt it abandons at resume. The
  reason cites #1187.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every pull request in this batch inserts its entry at the same place in
CHANGELOG.md, so each merge would conflict with the next. The entries are
collected into one changelog update instead.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@aviggiano
aviggiano merged commit 15144d9 into main Sep 29, 2026
1 of 5 checks passed
@aviggiano
aviggiano deleted the claude/w02-ledgers-never-block-sync branch September 29, 2026 06:35
aviggiano added a commit that referenced this pull request Sep 29, 2026
…on the real pinned engine (#1187)

* test(cli): run one campaign end to end with a controller kill and resume

Every runtime and CLI test drove a fake `smithers` shell script or ran the
engine on a hand-written workflow, so a break between the generated
workflow, the pinned Smithers engine, the sealed execution snapshot, and the
CLI only surfaced in a real campaign.

The new test runs `init`, `run`, `status`, `resume`, `stats`, `report`, and
`events` as separate CLI processes against the pinned engine that `run` and
`resume` install and start under Bun. A stub `codex` on PATH writes the
artifacts each prompt's output contract names, builds the final report from
the host-injected authorities, and renders it with the prompt's `ultrafuzz
report render` command. The stub holds the second node open while the test
SIGKILLs the detached engine and supervisor; once status reports the run
orphaned, the test resumes it. It asserts that the run succeeds with a
verified report, that only the interrupted node's agent ran twice, that no
engine task started again after it finished, and that status and stats
describe the same complete run.

A todo subtest records a gap it found: stats counts three agent attempts
where status and the stub count four, because attempts.jsonl is built from
NodeFinished/NodeFailed events and resume cancels the interrupted attempt
without one.

The file lives in test/e2e/ with its own `test:e2e` script, so the CLI suite
glob does not run it twice.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* ci: require the end-to-end campaign lane on pull requests

Adds a `cli-e2e` release-validation gate that runs
`pnpm --filter @ultrafuzz/cli test:e2e`, and a lane for it that is
required on pull requests. The lane has a 60-minute budget and the test
itself a 45-minute timeout. It runs beside the runtime lanes rather than
inside the push-only CLI lane.

The budget test in release-validation-lanes.test.ts keeps its 120-minute
expectation for the complete runtime and CLI suites only.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* docs: disable markdownlint line length for CHANGELOG.md

Super-linter lints every changed file in full, so any pull request that
adds a CHANGELOG entry fails MD013 on the file's existing entries, which
are single lines of up to 1,800 characters. This is the same file-level
directive #1179 adds, byte for byte, so the two merge without conflict.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test(cli): poll the event log for the end of the resumed campaign

`ultrafuzz status` synchronizes the run before it answers, unless the
control evidence has diverged. `events` streams the engine's event log
without synchronizing. The test now polls `events` until a terminal run
event, calls `status` once, and reuses those events for the no-restart
assertion. Locally, resume to run end fell from 165 s to 122 s.

The wait for the held node also fails at once if the workflow stops
before that node starts, instead of after the 15-minute bound.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* test(cli): report e2e reruns and stub failures by name, and clean up on interrupt

Review fixes for the end-to-end campaign test.

- The no-rerun checks now run straight after the events poll, before any
  command synchronizes the run. Those checks cover the stub start counts,
  the RunStarted count, the untruncated event page and no NodeStarted after
  NodeFinished. First the test checks that the engine's first run-terminal
  event is RunFinished. A resume that re-runs a finished node now fails on
  the named count. Before, it failed with an opaque
  ARTIFACT_VERIFICATION_AUTHORITY_INVALID from the status call that came
  first.
- The stub now fails, and logs why, in three cases: it finds no output
  contract, a named authority file is missing, or the report render line
  is not all `--flag 'value'` pairs. Before, a missed contract match
  exited 0 with a successful Codex turn and wrote nothing. The test's
  "workflow stopped" and RunFinished failures include the stub's call log.
- SIGINT and SIGTERM handlers, and the exit hook, now SIGKILL the
  campaign's processes and delete the fixture; the signal handlers then
  re-raise. A Ctrl-C'd run used to leave the detached engine, the
  supervisor and the ~1 GB fixture behind.
- The wait for the held agent to exit counts a zombie as exited. kill(pid,
  0) succeeds on a zombie; its /proc cmdline is empty.
- Drops the stats `attempts_complete === true` pin. That field only
  says the attempt ledger exists, and pinning it would break a fix that
  reports the undercounted attempts as partial evidence.
- The todo reason now names the cause that outlasts #1186. Smithers
  emits no terminal event for the attempt it abandons at resume. The
  reason cites #1187.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

* docs: say the e2e CLI runs under Node and the workflow under Bun

The CHANGELOG entry said the CLI commands ran on the pinned Smithers
engine under Bun. They run as Node CLI processes; only the generated
workflow runs on the engine under Bun. The development guide gets the
same precise wording, plus one sentence on what the test does when it
is interrupted.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.

Terminal workflow stays running in status after unrecorded attempt supersession

1 participant