feat(agents): bind reasoning effort and fast mode to each provider chain entry - #57
Merged
tatelilith merged 10 commits intoAug 17, 2026
Conversation
…ain entry
Both controls are stored per chain BINDING, beside the model, not once per
Agent. A chain mixes Providers and models for fallback and the legal effort
levels follow the model, so a single Agent-wide value is necessarily invalid
for at least one entry: codex advertises `ultra`, Claude never does, and
Haiku 4.5 accepts no effort at all.
Nothing is hard-coded. The manifest declares only WHETHER a CLI accepts each
control; the legal values are discovered per model alongside the model ids,
reusing the existing per-credential probe. An absent level list means "not
discovered" and an empty one means "discovered, none exist" — collapsing the
two would make an unprobeable credential look like a model with no levels.
Execution validates neither, deliberately: by then only the model string
survives and it can still change mid-Run through fallback. So `diagnose`
reports a control bound to a Provider whose CLI has no such setting
(provider_reasoning_effort_unsupported / provider_fast_mode_unsupported,
severity warn), and a level the MODEL rejects is reported by the CLI itself
with the accepted set named in its error.
Engine plumbing:
- claude-code passes `--effort <level>` and `--settings {"fastMode":true}`.
minVersion rises to 2.1.47, bisected against npm: 2.1.45 rejects `--effort`
and 2.1.46 was never published.
- codex passes `-c model_reasoning_effort=…` and `-c service_tier="priority"`,
re-passed on resume because `-c` is accepted there.
Fast mode is only ever REQUESTED — the plan, the model and the endpoint each
hold a veto and the CLI settles it at run time. The result log therefore
records what was SERVED, with the server's own answer beating the client's
intent: claude reports `usage.speed`, so a claimed `on` served at standard
records as `denied`. codex reports nothing about the tier at all, which is
why an absent state is never read as `off`.
The eligibility probe fails OPEN. Every failure path resolves to undefined
rather than `available: false`, because refusing a feature on the strength of
a failed probe is worse than letting the run report the outcome itself.
clearProviderBinding clears both alongside the model — without it a codex
`ultra` leaks onto a Claude fallback that has no such level.
Evaluation freezes both in configSnapshot, so two tasks differing only in
reasoning depth are no longer indistinguishable.
…val output Four CLI surfaces could not see the two new controls, and one of them could not see the execution plan at all. `agents get` printed `config: configured (keys: …)` — bare key names, because config can hold credentials. That rule also hid which models the Agent runs, in what order, at which reasoning depth; not even the model was shown. The chain is now rendered from the four schema-declared fields of providerChainItemSchema, which carry no secret; the providerApiKey / providerOauthToken / providerBaseUrl sitting beside them in the same object are never read, and a test pins that. A chain-less Agent shows its single `Model:` line, which was previously absent for the most common Agent shape. `runs get` gained `[init] model=…`, `[params] reasoningEffort=… fastMode=true` (the request) and `[done] … fastMode=<state>` (the verdict). The state prints verbatim including `off` and `denied`: unlike the web summary, which is a one-line scan, this log is what someone reads when a run was slower than the config promised, and "requested but refused" is the answer. `[params]` renders only those two named fields — every other value there is a path or a redaction marker, since both engines pass the controls through blanket-redacted flags. `eval tasks get` prints the frozen effort and fast mode beside provider and model. Both appear only when actually frozen: an absent effort means the CLI's own default was used and `fastMode=false` is a deliberate off, so `n/a` would misreport both. `agents apply --example` documents the controls. They already worked through the freeform config passthrough, but nothing in the CLI named them, so the example was the only place a user could learn they exist — or that they belong to a chain entry rather than to the Agent.
`agents apply` upgrades to `high-risk-write` when its diff REMOVES something, but the check only understood two shapes: an array that lost a member, and a non-empty scalar replaced by an empty one. `config` is neither. The API replaces `config` WHOLESALE, so a YAML naming only `providerChain` deletes every key it did not repeat. Measured against a real Agent, a one-entry YAML dropped `model`, `force`, `timeoutMinutes`, `maxRetries`, `readOnly`, `cleanResult` and a whole fallback provider — and the gate returned `[]`, so it applied with no confirmation. That shape becomes routine now that reasoning effort and fast mode live in the chain: "change one level" is exactly the edit that produces a partial config. The walk now recurses into plain objects, reporting removed keys under a dotted path, and treats an object replaced by a non-object as a full clear. Array entries are matched by IDENTITY (`id`, else `name`, else `providerId`) rather than by deep equality. Equality would call every edited entry removed — changing one binding's effort would report the binding as unmounted — which gates an ordinary edit behind `--yes` and teaches the caller that the warning means nothing. Lists of primitives keep the exact value comparison they had, and anonymous object entries fall back to a length count, since naming them printed `[object Object]`. Verified end to end: the partial YAML is now refused, names both losses, and writes nothing (exit 1, config intact); a full YAML changing only one entry's reasoning effort still applies with no prompt. The example YAML and the command reference now state the wholesale-replacement rule, which nothing said before.
`test-results/` was already excluded but `playwright-report/` was not, so running the E2E suite left a generated HTML report that biome then linted: 3574 errors from bundled report JavaScript (2557 `noAssignInExpressions`, 484 `noDoubleEquals`, 397 `noInnerDeclarations`). Both directories are gitignored, so nothing showed up in `git status` — the mandatory `pnpm lint` gate simply started failing after an E2E run, with every error pointing at a file the author never wrote. Hit while running the E2E suite for the reasoning-effort work.
The antd wrapper destructures a fixed prop list, so anything not named there is dropped in silence. `data-testid` was never forwarded: the attribute reached the DOM as nothing at all, and a `getByTestId` selector matched no element while the switch itself rendered and worked perfectly. That failure mode is worse than a type error. The component looks correct in the browser and only the test disappears, so it reads as a broken feature rather than a missing attribute — it cost two rounds of E2E debugging to find, with the DOM snapshot showing the switch present the whole time. Found via the provider-chain E2E spec, which is currently the only place passing a testid to `<Switch>`. Tests pin the forwarding, the untestid'd path that every other call site uses, and the disabled behaviour.
Three surfaces shipped with claims nothing verified. The fast-mode eligibility probe had zero tests, while the commit that added it promised it "fails OPEN". That promise is the whole design: the endpoint is an internal Anthropic route, not a published contract, so a probe that cannot answer must leave the switch usable rather than grey out a working feature on the strength of a 404. Nine failure paths now assert `undefined` and never `available: false` — non-200, missing field, non-boolean field, unparseable body, thrown request, timeout — alongside the positive cases (granted, refused with reason, reason clipped to 64, no reason when granted, bearer + beta headers, skipped entirely for a proxy). Writing them caught a wrong assumption of my own: the endpoint is on api.anthropic.com, not console.anthropic.com. The probe-models route carried the new fields with no route-level test. The one that matters is that `absent` and `empty` survive JSON: a model that answered "no levels" serialises as `[]` and a model never asked for stays missing, since the picker greys out for one and stays open for the other. Also mocks the rate limiter — the file already sat at 16 of its 20 requests per minute, so the next test added would have 429'd for a reason unrelated to what it asserts. The E2E spec was written but never run, and failed on the first execution: both controls live inside the entry's collapsed body, and a saved chain renders every entry closed. It now opens the row first. (Running it also surfaced the Switch data-testid bug fixed in the previous commit.) Also verified, not fixed: `restart-recovery.sh` passes 4/4, which matters here because this feature changes applyProviderBinding / clearProviderBinding — the provider-fallback switch point that recovery walks.
…hine These tests spawn the real binary, so anything the host leaks into the child lands in the assertions. Two things did, and both failed the mandatory `pnpm test` gate for reasons unrelated to the code under test. `npx` put npm between the assertion and the process. This repo's `.npmrc` carries pnpm-only keys (`only-built-dependencies[]`), which npm answers on stderr with `npm warn Unknown project config` — ahead of the CLI's own output. `JSON.parse(stderr)` then failed on npm's prose and reported a broken error envelope when the envelope was fine. Running the installed tsx binary directly removes npm from the path entirely, and with it npm's cold start: the file drops from 3.1s to 1.4s (it carries a 60s timeout for exactly that cost). Falls back to `npx tsx` when the binary is absent. HOME was inherited, so the child read the developer's real `~/.a2wave/config.json`. "No credential" is the precondition every case here depends on — it is what makes each data command fail fast and exercise the error path — but inheriting HOME makes that a property of the machine. On a laptop logged into an instance, `resolveCredential` correctly answers "no stored credential for <url>" rather than "not logged in", and the assertions fail on a message that is right. The child now gets an empty temp HOME. Neither failure reproduced in CI, which starts from a clean image — which is what let both reach main.
Reviewed the branch with four specialist passes plus a Codex adversarial pass. Five real defects, four of them in code this branch introduced. **fastMode survived a Provider switch** (config-tab.tsx). The credential-change reset cleared `reasoningEffort` but not `fastMode`, so switching a chain entry to a Provider whose CLI has no fast mode left the switch on in the saved config — dropped silently at spawn, and warned about by `diagnose` for a control the operator could no longer see. The API-side comment already claimed both were cleared. Found independently by two specialists. The reset lived as a closure inside a `useCallback`, which is why the missing field went unnoticed: nothing could assert on it. Extracted as `applyProviderEntryPatch` with tests for both controls, the probe-writeback exemption, and the same-Provider no-op. **The snapshot could not represent "frozen off"** (evaluation-snapshot.ts). `null` meant both "captured while unset" and "row written before these fields existed", and `??` read both as "inherit the live value". A task queued with fast mode off would therefore run WITH it if the operator toggled the Agent in between, filed as though nothing had changed — the exact drift the snapshot exists to prevent. Now decided by KEY PRESENCE: a snapshot carrying the key has an opinion (including `null` = stay unset), one without it is pre-change and still inherits. Caught by the Codex pass, independently flagged by the API contract pass. **The web could not see either field.** `evaluationConfigSnapshotSchema` — the only type the web has for a snapshot — was never extended, so the two frozen values existed in the DB, the API and the CLI but nowhere in the UI, while the manual said the snapshot records them. Extended the shared schema, rendered both in the task detail, and taught `snapshotDelta` to compare them: two tasks differing only in reasoning depth were showing as identical, which is the comparison the fields were added for. Compared through `?? null`, or every task straddling the change reads as "config changed" over a storage-format difference. **All-unreadable level lists claimed "none"** (model-capabilities.ts). `toReasoningEffortOptions` dropped tokens failing the shape check and returned `[]` — the positive claim "this model takes no reasoning level", which greys the picker out. A source whose vocabulary this code could not read has made no such claim; it now returns `undefined` (unknown), and only a genuinely empty input yields `[]`. **The stored `$type<>` lied about existing rows.** Both dialects declared the two new keys required with no backfill; every pre-change row has neither. Now optional, matching what is on disk. Plus: the run-summary comment described a denylist while the code was an allowlist (`cooldown` also drops the marker, correctly — the comment was wrong), and two weak assertions were tightened — `--effort` now asserts adjacency rather than membership (`arrayContaining` passes on `['--effort', '--model', …]`), and the codex duration gets an upper bound, which is what distinguishes an elapsed delta from a raw `Date.now()`. New tests: codex `fastModeState: 'requested'` and its absence, the engine→CLI exec-params contract for both controls, unreadable-vocabulary discovery, the 200-char description clip, the two snapshot key-presence cases, and six for the extracted patch helper.
Seven findings I had proposed deferring as debt. Checking their provenance
showed every one originates in this branch — `collectRemovals` / `isPlainObject`
do not exist in the base, `model-capabilities.ts` is a new file, the three
`fastModeState` declarations are 0-to-1 additions, and the four untested regions
are +25/+36/+6/+82 with no deletions. Filing them as issues would have shifted
this branch's own defects onto whoever picked them up.
**A removal warning could echo a secret** (agent-yaml.ts). Recursing into `config`
for the destructive-diff gate made a value-printing path reachable in data the
repo treats as secret-bearing — `agents get` prints its keys and never its values
for exactly this reason. `{config:{allowedTokens:['sk-live-…']}}` losing an entry
rendered the token verbatim into the confirmation prompt. Values are now named
only at the top level, where members are ids the user picked; below it the count
is all that may be said. Identity fields (`pc_codex`) stay named at any depth —
they are what make the warning actionable, and an id is not a credential.
**`fastModeState` was three prose lists typed as `string`.** One per app, each
naming a different incomplete subset of the five states, none of them right. Now
`fastModeStateEnum` in packages/shared, used by all three. `resolveFastModeState`
validates against it instead of forwarding the CLI's token: that string reaches
the run record, the web log and a terminal, and every other third-party value in
that file is already capped or shape-checked. An unrecognised state degrades to
absent, which the UI already renders as "the engine said nothing".
**Diagnose checked only the bound Provider** while its comment claimed to be the
only place a mismatch is ever noticed. A control belongs to its chain entry, so a
mismatch on a fallback is just as real and strictly harder to spot — it bites
once the primary has already failed and nobody is watching. Every entry is
checked now, positioned in the message, at no extra query since each carries its
own `providerKind`.
**The two truncation bounds were literals on both sides** of a contract: `.max()`
in the shared schema, `.slice()` in the engines, with nothing tying them. The
probe route returns the adapter result verbatim, so the slice IS the enforcement,
and raising a bound would have left the engines clipping at the old one. Both are
now exported constants.
Tests for the three regions that had none: the run-log fast-mode verdict and the
collapsed summary (11 cases, including the marker rule for all five states and
the unrecognised-state fallback), the dirty check for both controls (an operator
who changed only the reasoning level had Save greyed out and no unsaved-changes
guard — the edit was lost on navigation, with the page never looking dirty), and
the e2e now drives fast mode ON through the real UI instead of only OFF, where
`?? false` had been absorbing a chain that lost the field entirely.
…st-mode copy The snapshot's whole purpose is comparing one run against another, and the two web surfaces that serve that purpose shipped untested — both added during the review pass earlier today, which makes this the same gap the review existed to close. `snapshotDelta` decides the "config changed" flag. It now compares reasoning effort and fast mode, so a pair differing only in reasoning depth raises it — previously the two tasks rendered identically, which is exactly what freezing the fields was meant to prevent. The regression test that matters most is the one for the `?? null` normalisation: a task frozen before these fields existed carries `undefined` where a new one carries `null`, and comparing those directly flags every pair straddling the change over a difference that is purely storage format. `task-detail.tsx` renders both values, and had no test file at all. Six cases, including `fastMode: false` — a real frozen answer, since the snapshot exists so a task queued with fast mode off cannot inherit a later toggle. Hiding it would make "off" and "never recorded" identical in the one view built for comparison. Both suites match either locale rather than pinning one, matching the sibling tests in this directory. Also shortens the run-log fast-mode copy, which was carrying an explanation where a label belongs: requested 已请求快速模式，该引擎不回报实际执行速度 -> 快速模式已请求 denied 已请求快速模式，服务端按普通速度执行 -> 快速模式被拒 The set now reads 已生效 / 已请求 / 被拒 / 未生效 / 冷却中. Deliberately NOT worded as "the Provider does not support fast mode": that is a different condition — the switch is not even rendered when a Provider's CLI lacks the capability — and `denied` is an ACCOUNT-level refusal (extra_usage_disabled), so that wording would point the operator at swapping Provider when the fix is `/usage-credits`. The config page already states the real cause and its fix.
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
2 participants
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.
Summary
Adds reasoning effort and fast mode as per-binding execution controls for
the
claude-codeandcodexengines.Both live on a provider-chain entry, beside the model — not on the Agent. A
chain mixes Providers and models for fallback, and the legal effort levels follow
the model, so a single Agent-wide value is necessarily invalid for at least one
entry: codex advertises
ultra, Claude never does, and Haiku 4.5 accepts noeffort at all.
Nothing is hard-coded. The provider manifest declares only whether a CLI
accepts each control; the legal values are discovered per model alongside the
model ids, reusing the existing per-credential probe. An absent level list means
"not discovered" and an empty one means "discovered, none exist" — collapsing the
two would make an unprobeable credential look like a model with no levels.
Execution validates neither, deliberately: by then only the model string survives
and it can still change mid-Run through fallback. So
diagnosereports a controlbound to a Provider whose CLI has no such setting, and a level the model rejects
is reported by the CLI itself with the accepted set named in its error.
Fast mode is only ever requested — the plan, the model and the endpoint each
hold a veto — so the run log records what was served, with the server's answer
beating the client's intent.
Surfaced in the web config page, the run log, the CLI (
agents get,runs get,eval tasks get,agents apply --example), and frozen in the evaluation configsnapshot so two tasks differing only in reasoning depth are distinguishable.
Engine plumbing:
claude-code:--effort <level>and--settings {"fastMode":true}.minVersionrises to 2.1.47, bisected against npm (2.1.45 rejects
--effort; 2.1.46 wasnever published).
codex:-c model_reasoning_effort=…and-c service_tier="priority",re-passed on resume because
-cis accepted there.Type of change
Testing checklist
pnpm lintpasses — 0 errors, 752 warnings (per-rule identical tomain;verified by running biome against a clean checkout of the base commit)
pnpm typecheckpasses — fully green, test files includedpnpm testpasses — shared 246, web 986, cli 1080, api 6233 (2 skipped);every changed behaviour ships with tests
scripts/e2e/restart-recovery.sh4/4 (this change touchesapplyProviderBinding/clearProviderBinding, the provider-fallback switchpoint recovery walks), and
e2e/tests/agents/provider-chain.spec.ts3/3 with anew case covering both controls surviving a save that cannot probe levels
zh.jsonanden.json), and the in-appmanual updated
Also verified end to end against a live instance: two real evaluation tasks on the
same Agent, one with effort
low/ fast off and one withhigh/ fast on. Thestored snapshots differ, the CLI prints them differently, and the spawned argv
carried
--effortalone for the first and--effort+--settingsfor the second.Backward compatibility
No migration, and no DDL. Neither
drizzle/nordrizzle-pg/gains a file:the two controls are optional keys inside existing JSON columns. There is nothing
to run on upgrade and nothing to undo on rollback.
Verified against the real schemas and the real restore path rather than by
inspection:
providerChainat allThe one that needed proving is the snapshot restore, because zod's
.default(null)would MATERIALISE the missing keys if the stored row were parsedon read — and
applyEvaluationSnapshotdecides by key presence, so apre-change row would then be misread as "frozen as unset" and stop inheriting the
live value, silently changing the behaviour of tasks created before this change.
It is not parsed:
evaluationConfigSnapshotSchemais defined and exported for itstype but never used to parse anything, and the drizzle JSON column reaches
applyEvaluationSnapshotraw.Driven directly, with the live Agent set to
ultra+ fast on, to check an oldtask cannot be contaminated by it:
ultra/truenulllow/falselow/ unsetOther surfaces:
providerCapabilitiesgains two required booleans, butcapabilities are code-owned (
BUILTIN_PROVIDER_MANIFESTS), never persisted andnever accepted from a client. The probe-models response additions are optional, so
an older client ignores them.
configSnapshot's$type<>declares both new keysoptional, matching what is actually on disk. An older published CLI passes
configthrough untouched; a newer CLI against an older server reads
undefinedand omitsthe two columns.
Not a compatibility break but worth flagging:
claude-code'sminVersionrises to2.1.47. It does not block runs —
agent-execution-diagnose.tssays so explicitly —it only raises a warning and a "version too old" label, and
provider-cli-lock.jsonalready pins 2.1.212, so managed installs clear the floor.
Cross-cutting change matrix
entry (the controls ride existing audited routes)
section above
configSnapshotgains two$type<>fields in both dialects (type-level only, no DDL);
schema.pg.tsregeneratedAI assistance disclosure
understand the code and have tested it myself (see CONTRIBUTING.md).
Additional notes
Three fixes in here are repo hygiene found while building this, not part of the
feature — each is its own commit and can be dropped if you would rather take them
separately:
build: keep the playwright report out of the lint gate—playwright-report/was not excluded, so running the E2E suite made
pnpm lintfail with 3574 errorsfrom generated report JavaScript.
fix(web): forward data-testid through the Switch wrapper— the antd wrapperdestructures a fixed prop list and silently dropped
data-testid, so a selectormatched nothing while the switch rendered fine.
test(cli): make the dispatch suite independent of the developer's machine—the suite spawned
npx tsx(npm'sUnknown project configwarning landed in thestderr it parses as JSON) and inherited
HOME(so it read the developer's real~/.a2wave/config.json). Neither reproduces on a clean CI image.One behaviour worth a reviewer's eye:
fix(cli): catch config removals in the apply destructive-diff gate.agents applyreplacesconfigwholesale, so a YAML namingonly
providerChainsilently deleted every key it did not repeat — measured againsta real Agent, six keys and a whole fallback provider, with the existing gate
returning no findings. The gate now walks nested objects and matches array entries
by identity rather than deep equality, so editing one binding's effort stays an
edit rather than reading as an unmount.