fix(runtime): give agent retries a real wait, stop retrying deterministic failures, and label timeouts by code - #1171
Conversation
Agent retries used an exponential backoff from 1s, so a three-attempt budget was spent in about 25 seconds, inside the minute a contended Claude Code OAuth refresh can need to clear (#1084). The agent Task also inherited Smithers' default identical-failure stall verdict (3), which ended a chain before later same-agent attempts (exhaustive plans five) or any [retry].agents fallback profile ran. Against real Smithers 0.35.0, a [fail, fail, fail, fallback] chain ended `stalled` after attempt 3 and never ran the fallback; with maxIdenticalFailures: 0 the fallback ran at attempt 4 and the run finished. - compileTask: initialDelayMs 60_000, so retries wait 60s, 120s, 240s, then Smithers' 300s cap. - Agent Task: maxIdenticalFailures: 0, so the planned chain is the budget. It is set in the template only; the compiled manifest, cloud handoff schema and sealed task documents keep their exact shape. - The real `smithers graph` smoke test never ran: it required a workspace-root .smithers install that no checkout has, and could not resolve the sealed module paths. It now uses the runtime package's own Smithers and asserts the retryPolicy Smithers receives. Refs #1084 Co-Authored-By: Claude Opus 5.5 <[email protected]>
| case "NodeFailed": { | ||
| const failureMessage = errorText(event.payload.error); | ||
| return errorLooksLikeTimeout(event.payload.error) | ||
| return workflowErrorIsTimeout(event.payload.error) |
There was a problem hiding this comment.
Upgraded runs block ledger updates
If an in-flight run already recorded an attempt as timed out, but its error has no typed code, this change reclassifies the same attempt as failed on the next sync. The attempt ledger treats the recorded outcome as immutable, so reconciliation fails with NODE_ATTEMPT_LEDGER_WRITE_FAILED and later entries for that task cannot be appended. Preserve the recorded outcome or provide a migration path.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/workflow-sync.ts
Line: 5686
Comment:
**Upgraded runs block ledger updates**
If an in-flight run already recorded an attempt as timed out, but its error has no typed code, this change reclassifies the same attempt as failed on the next sync. The attempt ledger treats the recorded outcome as immutable, so reconciliation fails with `NODE_ATTEMPT_LEDGER_WRITE_FAILED` and later entries for that task cannot be appended. Preserve the recorded outcome or provide a migration path.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| function assertTaskDependencyInputs(task: (typeof taskSpecs)[number]): void { | ||
| try { | ||
| admitTaskDependencyInputs(task); | ||
| } catch (error) { | ||
| throw nonRetryableFailure(error); | ||
| } |
There was a problem hiding this comment.
Transient admission errors cannot retry
This catch marks every admission error as non-retryable, including a temporary failure while checking or reading a dependency file. Preparation normally has retries, but Smithers will now fail the consumer after one attempt even if the filesystem operation would succeed on retry. Mark only deterministic admission failures as non-retryable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/runtime/src/templates/smithers/workflows/workflow.tsx
Line: 5584-5589
Comment:
**Transient admission errors cannot retry**
This catch marks every admission error as non-retryable, including a temporary failure while checking or reading a dependency file. Preparation normally has retries, but Smithers will now fail the consumer after one attempt even if the filesystem operation would succeed on retry. Mark only deterministic admission failures as non-retryable.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.…l timeouts by code Two defects from #1144. Retries. A consumer's dependency-admission failure re-reads the same producer bytes, so every retry fails the same way; each consumer still spent its whole retry budget and agent fallback chain. The admission entry points (preparation's assertTaskDependencyInputs and the agent's assertDependencyArtifactAdmissionCurrent rechecks) now throw with details.failureRetryable=false, which Smithers honors, and preparationStep keeps that flag when it rewraps the error. Everything else stays retryable, including the #672 engine-boundary TypeError. A real Smithers run of the template's preparationStep and helper pins this: the admission failure runs once and its agent is skipped, while a transient TypeError is retried and finishes. With the flag removed the same preparation runs three times and ends `stalled`. Timeout labels. errorLooksLikeTimeout ran /timeout|timed out|heartbeat/ over every string in the NodeFailed error, including the stack and causes. That is a failure taxonomy from free text, which #572 rules out, and since #1027 every JSON-validator preflight failure mentions "timeout", so a 2ms `spawnSync ultrafuzz ENOENT` was recorded as a timed-out provider interruption. A node now times out only on Smithers' typed deadline codes (TASK_TIMEOUT, TASK_HEARTBEAT_TIMEOUT, PROCESS_TIMEOUT, PROCESS_IDLE_TIMEOUT) or a TaskHeartbeatTimeout event. The agent-failure normalizer used to strip PROCESS_TIMEOUT and PROCESS_IDLE_TIMEOUT, which left the regex as the only label for real agent CLI deadlines, so it now keeps them. The #1144 analysis also proposed guarding generate()'s catch-path recheck. It is not included: resetTaskArtifactsForRetry admits dependencies before that try block on every first generation in a process, so the catch path never runs without an admission. Refs #1144 Co-Authored-By: Claude Opus 5.5 <[email protected]>
…ines are labelled failed Review of this change found the docs, CHANGELOG and one comment stronger than the code: - docs/config.md said the planned chain is the whole budget. Smithers still stops a chain at a failure it classifies as non-retryable, such as a CLI auth or configuration error, and pauses the run on a quota limit. - The admission wrappers mark every failure inside admission non-retryable, including a file-system or validator error while reading the producer files, not only missing or changed bytes. - Timeout labelling also keeps the TaskHeartbeatTimeout event rule, and a deadline reported only as text, such as the Modal provider's cloud-node deadline, is now labelled failed. docs/reference/artifacts-reports.md states the rule next to the node statuses. - The workflow-sync comment read as if it listed every Smithers deadline code. Refs #1144 Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every CHANGELOG entry is one long line, so super-linter's markdownlint fails MD013 (line length 400) on any pull request that touches the file. release-gates then fails and every full release-validation lane is skipped, so the runtime suite never runs on the pull request. This is the same directive #1179 adds, byte for byte, so whichever lands first merges cleanly with the other. Co-Authored-By: Claude Opus 5.5 <[email protected]>
25ee12c to
668ad91
Compare
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]>
Problem
spawnSync ultrafuzz ENOENTpreflight failure, recorded as a timed-out provider interruption.Root cause
compileTasksetsretryPolicy: { backoff: "exponential", initialDelayMs: 1_000 }, so retries waited 1 s, then 2 s. Claude Code's refresh lock can take about a minute to clear.Taskinherited Smithers' defaultmaxIdenticalFailuresof 3. That verdict overrides Ultrafuzz's explicit chain: identical failures ended the chain before later same-agent attempts (exhaustiveplans five) or any[retry].agentsfallback profile ran.Errors, so Smithers retried them.preparationStepalso discardeddetailswhen it rewrapped an error.errorLooksLikeTimeoutran/timeout|timed out|heartbeat/over every string in theNodeFailederror, including stack and causes. That is a failure taxonomy built from free text, which Add bounded error-agnostic agent retries with backoff and optional model fallback #572 rules out. Since fix(runtime): let the smoke lane reach a scoreable result #1027, every JSON-validator preflight failure message mentions "timeout". The agent-error normalizer also stripped Smithers'PROCESS_TIMEOUTandPROCESS_IDLE_TIMEOUTcodes, which left that regex as the only thing labelling real agent CLI deadlines.Change
smithers.ts, workflow template)TaskgetsmaxIdenticalFailures: 0, so identical failures no longer end the planned chain early.maxIdenticalFailuresis set only in the template. The compiled manifest, cloud handoff schema and sealed task documents keep their exact shape.smithers graphsmoke test never ran: it needed a workspace-root.smithersinstall that no checkout has, and it could not resolve the sealed module paths. It now uses the runtime package's own Smithers and asserts theretryPolicySmithers receives.workflow-sync.ts)nonRetryableFailurehelper setsdetails.failureRetryable = false. Smithers already honors that flag.assertTaskDependencyInputs, and the agent's pre- and post-generationassertDependencyArtifactAdmissionCurrentrechecks. Both wrap their unchanged bodies.preparationStepkeeps the flag when it rewraps. Failures outside admission stay retryable, including the Concurrent prepare:* worktree tasks fail nondeterministically with a Bun-erased TypeError: undefined is not an object (evaluating 'get') #672 engine-boundaryTypeError.errorLooksLikeTimeoutis replaced by a check of the top-level typed code (TASK_TIMEOUT,TASK_HEARTBEAT_TIMEOUT,PROCESS_TIMEOUT,PROCESS_IDLE_TIMEOUT). The existingTaskHeartbeatTimeoutevent rule stays.PROCESS_TIMEOUTandPROCESS_IDLE_TIMEOUT.docs/config.mddocuments the retry timing, what can still end a chain early (a failure Smithers classifies as non-retryable, or a quota pause), and that admission failures are not retried, including file-system and validator errors during admission.docs/reference/artifacts-reports.mdstates the timeout-labelling rule next to the node statuses.<!-- markdownlint-disable-file MD013 -->at the end ofCHANGELOG.md, byte-identical to perf(runtime): halve launch fsync work, and doctor stops failing on unused agent profiles #1179's. Without it, super-linter fails every PR that touchesCHANGELOG.md,release-gatesfails, and every full release-validation lane is skipped. That happened on this PR's first push.Deliberately not built (and why)
Reporting Claude Code's
resulttext instead ofClaude run failed. This PR carried it until review, as commit2c7fa0d12ba832ab139c362184d997cad44167fe. I removed it, so Concurrent agents race on OAuth token refresh; three immediate retries all re-race and kill the run with an opaque "Claude run failed" #1084's opaque-error half stays open. It was not a diagnostic-only change, because Smithers classifies that text before it decides whether to retry.API Error: 401 … OAuth token has expired …) matches Smithers' auth pattern and becomes a non-retryableAGENT_CONFIG_INVALID. That ends the node on its first attempt, with no retry wait and no[retry].agentsfallback. Main retries the same output asClaude run failed, and with commit 1 of this PR that chain now reaches the fallback.waiting-quota. Without a reset time, the run waits forultrafuzz resume.ultrafuzz init --forcebefore its next launch.The Concurrent agents race on OAuth token refresh; three immediate retries all re-race and kill the run with an opaque "Claude run failed" #1084 analysis already proposed shipping this separately. Whether Claude's stated failure should drive those Smithers decisions is the owner's call; the commit can be cherry-picked from this PR's history.
Error-message classification of OAuth or transient provider failures. It would break Add bounded error-agnostic agent retries with backoff and optional model fallback #572 and the documented rule that "Ultrafuzz does not inspect provider error text". A uniform one-minute base wait recovers the same class without a taxonomy.
Staggered agent starts, a pre-fan-out token refresh, or per-agent credential copies. In the default topologies only one agentic node runs at campaign start. Claude Code refresh tokens rotate, so copied credentials would turn a transient race into a permanent auth failure. That last point is an inference from the Claude Code binary (per the Concurrent agents race on OAuth token refresh; three immediate retries all re-race and kill the run with an opaque "Claude run failed" #1084 analysis) and was not tested against the OAuth server.
The attempt-ledger reconcile tolerance proposed in Classify deterministic failures correctly and stop retry cascades #1144 (step 5). fix(runtime): attempt and usage ledgers never block run synchronization #1186 removes that reconcile path; see Risk.
The
generate()catch-path guard proposed in Classify deterministic failures correctly and stop retry cascades #1144. I checked it against current code and it can never fire.resetTaskArtifactsForRetryruns before thattryon every first generation in a process, and it always callsprepareArtifactMirror→assert-task-inputs. That either publishes the dependency admission or throws outside thetry, and admissions are never cleared. The masking reported in the analysis needed a harness that stubbed the reset. Both reviews agreed, by reading the code.Retrying environmental failures inside admission. The wrappers mark everything thrown inside admission as non-retryable. That includes a validator-isolate deadline or worker error, and file-system errors other than ENOENT. The validator does return a typed
setup-errorkind, but acting on it would add branching toassertVerifiedDependency's validation path, and both reviews advised against adding machinery there. The docs and Risk section state the behavior.Process-group termination on timeout (Classify deterministic failures correctly and stop retry cascades #1144's fourth acceptance criterion). Smithers already spawns agent CLIs as process-group leaders and kills the group on timeout. The only direct-child-only timeouts left are Ultrafuzz's two
execFileSynccalls, the per-node JSON-validator preflight and the report-authority call, and their grandchild work is bounded. Memoizing the preflight (fix(runtime): workflow renders no longer crash on prompt text, and per-render cost drops #1168) is the better cut. That criterion is why this PR refers to Classify deterministic failures correctly and stop retry cascades #1144 rather than closing it.A typed code on Modal cloud-node deadlines. The host-side
cloud node execution timed outcould carryPROCESS_TIMEOUTin about five lines. An inner agent timeout, though, reaches the host through the worker's error document, which has no code field, and adding one changes a registered schema and rotates the validator build identity. Fixing only the host-side half would label cloud deadlines inconsistently, so both are left for a follow-up and disclosed under Risk.Upstream Smithers issues (not filed from here, for the owner to file):
ClaudeCodeAgentdropsresult.claude auth statusprocess on every invocation. The Concurrent agents race on OAuth token refresh; three immediate retries all re-race and kill the run with an opaque "Claude run failed" #1084 analysis identifies this as a likely contender for the refresh lock.Verification
Each test below fails with
origin/main(b6dd1da9) source and passes at this PR's head. I re-ran the node tests against main after the review changes. I copied this PR's test files into a freshorigin/mainworktree, compiled them, and ran them there. The same results were reproduced independently by the implementer and both reviews.origin/mainsourceruntime.test.ts› compiled Smithers workflow passes a real non-executing graph smoke{backoff:"exponential", initialDelayMs:1000}, expected{…, initialDelayMs:60000, maxIdenticalFailures:0}runtime.test.ts› compileSmithersWorkflow exhausts same-profile retries before ordered fallbackinitialDelayMs1000 vs 60000runtime.test.ts› syncRun maps failed workflow nodes into durable failed run state, with realistic payloads: aTASK_HEARTBEAT_TIMEOUTcode on attempt 1 and the #1144 timeout-worded failure on attempt 2timed-out, expectedfailedgenerated-workflow-verifier› optional admission rejects a present malformed marker…: real admission code, marked non-retryabledetailsundefined, expected{ failureRetryable: false }generated-workflow-verifier› dependency admission retains one exact snapshot epoch…: the agent recheck is marked non-retryabledetailsundefined, expected{ failureRetryable: false }generated-workflow-verifier› agent failure normalization preserves only validated Smithers recovery controls: CLI deadline codes survivecodeundefined, expectedPROCESS_TIMEOUTsmithers-dependency-skip.integration› a dependency admission failure fails its preparation once while a transient failure is retried. Real Smithers 0.35.0 runs the template's ownpreparationStepand helper.preparationStepdropping the flag or with the helper marking nothing,prepare:consumerendsstalledinstead offailed. Unmodified, it passes.Experiment, not committed. The implementer ran this and one review reproduced it. In real Smithers 0.35.0, a
[fail, fail, fail, fallback]agent chain withretries=3endedstalledafter attempt 3 under the default policy and never ran the fallback. WithmaxIdenticalFailures: 0, the fallback ran at attempt 4 and the run finished.Ran at this head, all passing:
generated-workflow-verifierfile (140), plussmithers-dependency-skip.integration(2) andagent-adapter-boundaries(10). The adapter pins are main's again, becauseclaude.tsxis unchanged.runtime.test.tstests: the 29 whose bodies feedNodeFailed,TaskHeartbeatTimeout,timed-out, typed deadline codes orretryPolicythrough compile, start or sync, plus the init test and resume retries stalled nodes alongside failed ones. 29 passed in one batch. The other two, syncRun keeps redacted failure state… and syncRun records failed primaries and the actual fallback producer, failed withWORKFLOW_SUBMISSION_FAILED … changed while readingon a dependency file while a concurrentpnpm installran on this host. Both passed when re-run alone.prettier --checkandeslinton the changed files,CI=1 ESLINT_PLUGIN_DIFF_COMMIT=origin/main pnpm -w lint:strict:ci,pnpm --filter @ultrafuzz/runtime typecheck,pnpm -w knipandnode scripts/docs-check.mjs.Not run locally: the full runtime and CLI suites, and the live OAuth refresh race against a real credential (a real refresh would rotate a token that other sessions on this host share). CI ran the full runtime suite instead; its lane selector did not pick the CLI lane for this diff. On this head, External static analysis passes, and so do the five full release-validation lanes (runtime shards 1–4, and runtime-supporting with the Bun adapter contracts) and
release-gates. The now-unconditional graph smoke test ran in shard 4 and passed. On the first push those lanes were skipped.Risk / compatibility
exhaustive's five. Each fallback rung adds up to 5 more. That is small next to the 1–2 h node timeouts. With the stall verdict gone, repeated identical agent failures now use the full planned chain.failed/agent-failurerather thantimed-out/provider-interruption. That changes report categories and eval dispositions.cloud node execution timed out, and an inner agent timeout reported through the worker's error document. Both are nowfailedunless a typed Smithers deadline fires first. By my reading, the provider's deadline usually wins, because its clock starts before the provider's only in-flight heartbeat (at launch). In Modal eval disposition such a deadline moves fromincompletetooperational-failure. I traced this by readingnode-provider.tsandterminal-disposition.ts, not by running a cloud node.timed-outoutcome only from the old text rule, for example an agent CLI deadline whose code the old normalizer stripped. Those attempts now re-derive asfailed, while the immutable attempt ledger holdstimed-out.reconcileNodeAttemptLedgerEntryhas no match for that difference.NODE_ATTEMPT_LEDGER_WRITE_FAILEDand appends no later ledger entries for that task. That includesultrafuzz statuson a run that completed before the upgrade.ok: true. fix(runtime): attempt and usage ledgers never block run synchronization #1186 (w02) deletes that reconcile path, so merging it first or together removes the diagnostic. I confirmed this by reading the code; I did not re-run the upgrade experiment.initialDelayMs: 1000in their sealed task manifests. The template'smaxIdenticalFailures: 0reaches them only throughresume --refresh-controller, which re-renders the current template. Ordinary resume continues the persisted workflow.ultrafuzz init --forcefor this PR.Refs #1084
Refs #1144
Refs #676
🤖 Generated with Claude Code
The PR is not ready to merge: changelog linting blocks CI, and two previously reported runtime issues remain outstanding.
Fix with agent prompt
Summary
The PR lengthens agent retry waits, preserves the planned retry chain, makes dependency-admission failures non-retryable, and classifies workflow timeouts by typed code. It also expands tests and documentation. The latest change removes a changelog lint suppression needed by the current CI gate.
Reviews (3) · Last reviewed commit: "chore: move the changelog entry to the c..."