fix(runtime): share the Claude Code stated-failure hook and label it in last_error - #1246
Merged
Merged
Conversation
…ode base class #1225 gave DeepSeekClaudeCodeAgent a copy of CompatibleClaudeCodeAgent's statedFailure field and its createOutputInterpreter() and generate() overrides, and claude.tsx exported GENERIC_CLAUDE_FAILURE, claudeResultText and attachStatedFailure only for that copy, so a fix to one copy could miss the other. StatedFailureClaudeCodeAgent in claude.tsx now holds the field, both overrides and the #1084 comment, and CompatibleClaudeCodeAgent and DeepSeekClaudeCodeAgent extend it. DeepSeek does not extend CompatibleClaudeCodeAgent, whose buildCommand forces the "user" settings source; DeepSeek loads none. The three helpers are module-private again, and deepseek.tsx imports the Smithers class as a type only. The adapter-boundary policy now says the hook overrides generate as well as createOutputInterpreter, names the upstream line it works around (@smthrs/agents src/ClaudeCodeAgent.js builds a failed result's error as `limitBannerText || resultError || "Claude run failed"` and drops `payload.result`), and records that deepseek.tsx inherits the hook, which the gate's static detector no longer sees in that file. #1084 stays the tracked reference. Behaviour is unchanged: the Claude and DeepSeek Bun adapter contracts and the boundary gate pass with their assertions unchanged. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…de states claudeResultText kept the whole trimmed `result` of a failed Claude Code result line. The workflow's failure normalizer redacts that whole string before it cuts it to the 1,000-byte failure-message cap (normalizeNodeAttemptFailureMessage in packages/artifacts/src/attempt-ledger.ts), so an oversized statement spent redaction time on text that is dropped anyway. Measured with that normalizer on this host, text made of BIP39 words in punctuated runs, each shorter than the 49 words at which redaction replaces the whole text, takes about 2.5 to 3.3 s per 256 KiB, 11 to 14 s per MiB, and 0.15 to 0.2 s at 16,384 characters; lowercase non-words of the same shape take 30 to 40 ms per 256 KiB, and prose, hex and base64 0.1 to 0.3 s per MiB. The adapter now keeps the first 16,384 characters (MAX_STATED_FAILURE_LENGTH). The Claude adapter contract pins the cap. The record differs from an uncapped one whenever text past the cut would have reached its first 1,000 bytes: when the first 16,384 characters collapse below that after redaction and whitespace collapsing, as a long whitespace run or long redacted secrets do, the text after the cut is lost. The cut can also split a secret, which may then no longer match its redaction pattern. A PEM private-key block cut in two is no longer matched whole, and its lines fall back to the per-line entropy check, which missed 66 of 2,000 random 64-character base64 lines here, so such a block can leave a key line in last_error and failure_message. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… after its error errorText() in workflow-sync.ts joined the error message and details.agentStatedFailure with ": ". The message Smithers throws for the generic Claude failure is a SmithersError, which appends its docs link (@smthrs/errors SmithersError.js), so last_error and the attempt ledger's failure_message read Claude run failed See https://smithers.sh/reference/errors: Failed to refresh OAuth token: ... which attaches the statement to the URL. The join is now `${message} (agent stated: ${stated})`: Claude run failed See https://smithers.sh/reference/errors (agent stated: Failed to refresh OAuth token: ...) No other code joins the two strings: the dashboard, `inspect --json` and the public eval diagnostics read last_error as written. The sync test now feeds that real thrown message instead of a bare "Claude run failed", and anchors the whole line. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…dapter The workflow can run several attempts of a task on one adapter: a retry that Smithers schedules before the workflow re-renders reuses the wrapper's executionAgent (workflow.tsx: `executionAgent ??= admittedAgent()`). The Bun adapter contracts built a new agent for every generation, so deleting the `this.statedFailure = undefined` reset, or widening the `event.error === GENERIC_CLAUDE_FAILURE` guard to any failed completed event, still passed them. One test, run for ClaudeAgent and DeepSeekAgent, now drives a single instance through a stated failure (the #1084 OAuth refresh race for Claude, the 401 that Claude Code reported from DeepSeek's endpoint in #1225), a crash that prints only stderr, a session-limit banner, a result with an explicit `error` field, and a result that states nothing. Only the first carries details.agentStatedFailure, and its thrown message is exactly `Claude run failed See https://smithers.sh/reference/errors`. Checked by mutation on both adapters: without the reset, the crash inherits the earlier statement; with the guard widened to `!event.ok`, the banner is attached; with a widened guard that spares banners, the explicit-error result's text is attached. The new test subsumes the DeepSeek fresh-instance test and the Claude test's OAuth race and empty-result cases, which are removed; the Claude test keeps the auth-worded statement and the length cap. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…tial option
The test passed `execution: { agentCredentialEnv: [credentialName] }` to the
wrapped task, but the workflow's failure normalizer calls
sensitiveEnvironmentValues(process.env) with no explicit names and never
reads that field. The credential was redacted only because its variable
name, ULTRAFUZZ_TEST_STATED_FAILURE_CREDENTIAL, matches the credential-name
pattern; renamed to ULTRAFUZZ_TEST_STATED_FAILURE_VALUE, the test fails.
The option is removed and a comment says what the case covers. The
redaction itself is unchanged.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
… say where it shows The two Unreleased bullets for #1084 now read as one, for ClaudeAgent and DeepSeekAgent. The ClaudeAgent bullet claimed `status` and `why` show the stated failure. They do not: `ultrafuzz status` relays Smithers' status reason and gating detail, built from the attempt error's message alone (attemptErrorSnippet in @smthrs/cli src/run-status.js), and `ultrafuzz why` relays Smithers' blockers, which parseErrorSummary builds from the error's name and message (@smthrs/cli src/why-diagnosis.js). The bullet now names where it does show: `inspect --json` (data.state.nodes.<node-id>.last_error), the dashboard's latest error, the attempt ledger's failure_message, and the failed nodes in a public Modal eval worker's public-eval-diagnostics.json. It also uses the new "(agent stated: ...)" format and mentions the 16,384-character cap. The upgrade step said existing projects "pick up the changed adapter by re-running `ultrafuzz init`", which reads as optional. `ultrafuzz run` refuses a project whose stock `.smithers/agents` files differ from this release's (CONTROLLER_SOURCE_UNTRUSTED, "rerun ultrafuzz init"); checked with this branch's base claude.ts in an initialized project, where a plain `ultrafuzz init` replaced it and kept an edited ultrafuzz.toml. So existing projects must re-run it. A plain resume keeps running the workflow persisted at launch, and an earlier release's workflow drops details.agentStatedFailure when it normalizes the error, so such a run records the statement only after `resume --refresh-controller`, which renders this release's controller and packaged adapters. No other doc claims that status or why show it. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…sed-adapter test The per-call Claude adapter test kept two cases the reused-instance test did not cover, an auth-worded statement and the 16,384-character cap, and duplicated that test's fake `claude` helper. The adapter treats both cases the same on a fresh or a reused instance, so they now run as steps of the reused-instance test: the expired-login statement beside the OAuth refresh race for ClaudeAgent, and the cap for both adapters. The per-call test and its helper are removed. The test comment said the workflow reuses one adapter for every attempt. It reuses one only until the workflow re-renders: the adapter lives in the agent wrapper that each render builds, and a retry that Smithers schedules without a re-render runs on the same wrapper. Co-Authored-By: Claude Opus 5.5 <[email protected]>
… it would split The 16,384-character cap cut Claude Code's `result` text before the controller redacts it, so a secret that crossed the cut reached redaction in part, where it may no longer match its pattern. Both detectors that match a PEM private-key block need its BEGIN and END lines (the supplemental pattern in packages/security/src/sensitive-redaction.ts and secretlint's privatekey rule), so a block cut in two falls back to the per-line entropy check, which never matches a line that starts with `+` or `/`. When the text before the cut collapses to a few bytes, as a long whitespace run or other redacted secrets do, those lines land in the 1,000-byte record: with 15,200 newlines before a 4096-bit PKCS#8 key, the plain cut left a full key line in the normalized failure message for 19 of 40 keys, and an uncapped statement for none. The cut could also end on half of a UTF-16 surrogate pair. boundedStatedFailure now drops the token the cut would split, then a PEM block left without an END line, and returns no statement if nothing is left. Text of up to 16,384 characters is kept as before. With the same keys, no key line was left. The CHANGELOG entry says how the cut falls. The cap still loses the text after the cut, so when the first 16,384 characters collapse below 1,000 bytes the record can be shorter than an uncapped one. A secret made of several words, such as a quoted password with spaces or a BIP39 mnemonic, can still cross the cut; its words before the cut then reach redaction on their own, which can miss them. The reused-adapter test pins the cap at a whitespace boundary, a token across the cut, a PEM block across the cut, and a block that ends before the cut, which is kept whole. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…rror The how-to said the public diagnostic's failed-node projection holds only the node ID, status, timeout flag and failure category or code, and never messages, paths, findings or provider output. Since #415 a failed or timed-out node's entry also carries `failure_message` when the node has a last_error: that error, redacted and cut to at most 1,000 bytes by normalizeNodeAttemptFailureMessage (packages/modal/src/public-eval-diagnostics.ts). It can name paths, such as an artifact path in an artifact-contract failure, and since #1084 it can quote what Claude Code stated about the failure, such as DeepSeek's 401 for a rejected key, which the CHANGELOG entry for #1084 now names. The how-to now lists the field and what it can hold; the other fields still carry none of that. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…re-followups Co-Authored-By: Claude Opus 5.5 <[email protected]>
…oller to redact The 16,384-character cut ran before the controller's redaction, so a secret that crossed it reached redaction only in part, where it may no longer match its pattern. Cutting only at word and PEM-block boundaries narrowed that but could not close it: a quoted password with spaces or a BIP39 mnemonic that crosses the cut still leaves its first words to be redacted on their own, and a probe kept the first 10 words of a 24-word mnemonic. The cut only bounded redaction time for a statement of several megabytes, and Claude Code's is_error results are short error messages. So the adapter now keeps the statement whole, as #1224 did, and the controller redacts all of it before its 1,000-byte cap. The reused-adapter test pins that a long statement arrives whole; with a cut restored, it fails for both adapters. Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.
Follow-up to #1224 and #1225, which made
ClaudeAgentandDeepSeekAgentrecord the failure Claude Code states in a failed result (#1084). Thanks @mrthankyou for the original work. This branch addresses the review of both.Summary
ClaudeAgentandDeepSeekAgentshare one stated-failure hook,StatedFailureClaudeCodeAgentinclaude.tsx, instead of two copies.last_errorand the ledger'sfailure_messagereadClaude run failed See https://smithers.sh/reference/errors (agent stated: …).statusorwhy) and says projects must re-runultrafuzz init.failure_message.Change
1. One base class for the hook (
packages/runtime/src/templates/smithers/agents/claude.tsx,deepseek.tsx)#1225 gave
DeepSeekClaudeCodeAgenta copy ofCompatibleClaudeCodeAgent'sstatedFailurefield and itscreateOutputInterpreter()andgenerate()overrides, andclaude.tsxexportedGENERIC_CLAUDE_FAILURE,claudeResultTextandattachStatedFailureonly for that copy.StatedFailureClaudeCodeAgent extends SmithersClaudeCodeAgentnow holds the field, both overrides and the #1084 comment, and both adapters extend it. DeepSeek does not extendCompatibleClaudeCodeAgent, whosebuildCommandforces the"user"settings source; DeepSeek's sets"". The three helpers are module-private again, anddeepseek.tsximports the Smithers class withimport type.Nothing pins these templates' bytes:
packages/runtime/src/controller-source.tscompares a project's.smithers/agents/*.tswith the packaged templates whenultrafuzz runplans a run, and the init test matches their text by pattern.2. No cap on the kept text (
claude.tsx:claudeResultText)The review proposed capping the kept
resultat 16,384 characters, because the controller's normalizer (normalizeNodeAttemptFailureMessageinpackages/artifacts/src/attempt-ledger.ts) redacts the whole string before its 1,000-byte cap. On this host redaction takes about 11 s per MiB of BIP39 words in punctuated runs, and 0.1 to 0.3 s per MiB of prose or base64.This branch tried that cap and then dropped it. A cut runs before redaction, so a secret that crosses it reaches redaction only in part:
password: "correct horse battery staple"cut afterhorsekepthorse, and the first 10 words of a 24-word mnemonic survived.Claude Code's
is_errorresults are short error messages, so the cap would only have bounded the cost of a statement of several megabytes. The adapter keeps the statement whole, as #1224 did. The reused-adapter test pins this: a 19,999-character statement arrives whole.3.
(agent stated: …)(packages/runtime/src/workflow-sync.ts,errorText)SmithersErrorappendsSee https://smithers.sh/reference/errorsto its message, so the${text}: ${stated}join read…/reference/errors: Failed to refresh OAuth token: …. The join is now${text} (agent stated: ${stated}).errorTextfeeds bothlast_errorand the ledger'sfailure_message; the dashboard,inspect --jsonand the Modal diagnostics readlast_erroras written, so no other code joins the two. The sync test feeds the real thrown message and anchors the whole line.4. One reused-instance adapter test (
packages/runtime/test/runtime.test.ts)The workflow can run several attempts of a task on one adapter: a retry that Smithers schedules before the workflow re-renders reuses the wrapper's
executionAgent(executionAgent ??= admittedAgent()inworkflow.tsx). The old tests built a new agent for every generation and had no banner or explicit-error case, so they could not see a missing reset or a wider guard.generated <adapter> adapter attaches a stated failure only to its own generationruns, on one instance per adapter:Claude run failed See https://smithers.sh/reference/errorsand the codeAGENT_CLI_ERROR;AGENT_QUOTA_EXCEEDED) and a result with an expliciterrorfield, which get no statement;The per-call Claude and DeepSeek tests are gone; every case they had is a step here.
5. Normalizer test (
packages/runtime/test/generated-workflow-verifier.test.ts)The test passed
execution: { agentCredentialEnv: [...] }, which the normalizer never reads: it callssensitiveEnvironmentValues(process.env)with no names. The option is gone, and a comment says the value is redacted because its variable name looks like a credential. Redaction is unchanged.6. CHANGELOG (
CHANGELOG.md)The two #1084 bullets are one, for
ClaudeAgentandDeepSeekAgent. It drops "(sostatusandwhyshow it)":ultrafuzz statusrelays the attempt error's message (attemptErrorSnippetin@smthrs/cli), andultrafuzz whyrelays blockers thatparseErrorSummarybuilds from the error's name and message. It names where the text shows:inspect <run-id> --json(data.state.nodes.<node-id>.last_error), the dashboard's latest error, the ledger'sfailure_message, and a public Modal eval worker'spublic-eval-diagnostics.json.ultrafuzz runrefuses stock adapters from an earlier release (CONTROLLER_SOURCE_UNTRUSTED, "rerun ultrafuzz init"), so the entry says projects must re-runultrafuzz init, which keepsultrafuzz.toml, topology and prompts. A run launched by an earlier release records the statement only afterresume --refresh-controller.7. Adapter-boundary wording (
docs/reference/agent-adapter-boundaries.md,packages/runtime/test/agent-adapter-boundaries.test.ts)The
claude.tsxrow says the hook overridesgenerateas well ascreateOutputInterpreter, and names the upstream gap:@smthrs/agents0.35.0src/ClaudeCodeAgent.jsbuilds a failed result's error aslimitBannerText || resultError || "Claude run failed"(line 476) and dropspayload.result. Thedeepseek.tsxrow says it inherits the hook, and the gate keepsoutput-interpretationdeclared for it as inherited. #1084 stays the tracked reference.Also: the Modal how-to (
docs/how-to/run-evals-on-modal.md)It said the public diagnostic's failed-node projection "never contains messages, paths, findings, or provider output". Since #415 a failed node's entry carries
failure_messagewhen the node has alast_error: that error, redacted and cut to at most 1,000 bytes (packages/modal/src/public-eval-diagnostics.ts). It can name paths and, since #1084, quote what an agent CLI stated, such as DeepSeek's 401. The paragraph now lists the field and what it can hold.Verification
The branch merges current
main(#1162 and #1245). The only conflict was inCHANGELOG.md, where #1162 had added the friction-log bullet directly above the two #1084 bullets this branch folds into one; the friction bullet stays above the folded one.On the final head, after
pnpm install --frozen-lockfileandpnpm -w build:pnpm -w typecheckpnpm -w lintCI=1 ESLINT_PLUGIN_DIFF_COMMIT=<origin/main> pnpm -w lint:strict:cipnpm -w knippnpm -w format:checkpnpm -w docs:checkbun test dist-test/test/runtime.test.js --test-name-pattern '^Bun adapter contract:'(Bun 1.3.14)node --test --test-name-pattern '#1084' dist-test/test/runtime.test.jssyncRun shows the failure an agent stated beside its generic error (#1084)passesBefore the merge and the cap removal, from a clean build of every package, the same static gates passed. So did
test:release:supporting(Node 941/941, Bun adapter contracts 53 pass, 1 skip, 0 fail) and all four runtime shards (89 + 86 + 90 + 77 = 342 pass, 0 fail). CI reruns all of them on this head.Mutation checks on
claude.tsx: each change was applied on its own, the new test run withbun test dist-test/test/runtime.test.js --test-name-pattern 'attaches a stated failure only to its own generation', and the file restored. Each one fails both adapters' tests, at the step named:this.statedFailure = undefineddeleted!event.okerrorfieldNot in this PR
ultrafuzz why.agentCredentialEnv) in the normalizer.last_errorsince Retain bounded failed-node messages in durable diagnostics #415. Since Concurrent agents race on OAuth token refresh; three immediate retries all re-race and kill the run with an opaque "Claude run failed" #1084 that can include what an agent CLI stated, such as DeepSeek's 401, with the key redacted. Leaving it out would be a change topackages/modal/src/public-eval-diagnostics.ts.🤖 Generated with Claude Code
The PR appears safe to merge; no actionable issue was identified.
Summary
The PR consolidates Claude Code stated-failure handling for Claude and DeepSeek, labels the statement in persisted error text, and updates adapter tests and documentation.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Claude Code failed result] --> B[Shared stated-failure hook] B --> C[Error details] C --> D[Workflow synchronization] D --> E[Redacted state last_error] D --> F[Redacted and capped attempt failure_message] E --> G[Redacted and capped public diagnostic]Reviews (1) · Last reviewed commit: "fix(runtime): keep the whole failure Cla..."