mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
c525113452
The effort control this PR added rendered for Claude Code and Goose but wrote
to `BUZZ_AGENT_THINKING_EFFORT` — a variable neither harness reads, and which
nothing translates at spawn (`runtime_metadata_env_vars` handles only model and
provider). Picking "high" on a Claude agent saved successfully and reached
nothing: a silent no-op.
The bug survived review because the effort descriptor's `currentPersistence`
was pinned to that one key for every runtime while `targetApplication` carried
the real one, and the divergence was documented as deliberate legacy pending a
migration that never landed. The unit fixture built `runtime("claude")` with
`thinking_env_var: null`, so no test ever saw a real catalog entry.
`currentPersistence` and `targetApplication` are now identical by construction:
`deriveEffortEnvDescriptor` reads the catalog's declared target and returns
either `{kind:"envVar",key}` (CLAUDE_CODE_EFFORT_LEVEL, GOOSE_THINKING_EFFORT,
BUZZ_AGENT_THINKING_EFFORT) or `{kind:"structuredJsonEnv",envKey,jsonKey}`
(CODEX_CONFIG/model_reasoning_effort), with the JSON target winning when a
harness declares both. `readEffortEnvValue`/`updateEffortEnvValue` dispatch on
the shape, so no call site branches on a runtime id and JSON writes still merge
rather than clobber unrelated CODEX_CONFIG properties. There is no translation
step left that can drop an edit. The now-unreachable `acpConfigOption` /
`unavailable` persistences and the `ownedByModelId` omission reason are removed
rather than left as dead alternatives.
The option set is narrowed by the harness's implied provider instead of always
offering the buzz-agent list: `CLAUDE_CODE_EFFORT_LEVEL=none` resolves to
"unset" in the 2.1.222 binary, so offering "none" on Claude would be another
silent no-op. Provider-locked harnesses (Claude, Codex) offer only their
provider's valid levels; harnesses whose provider the user picks (Goose) keep
the full list and are narrowed on surfaces that know the provider.
`implicitEffortProvider` lives in the core module, not in a component, per
rule 1 of the feature AGENTS.md.
Claude and Goose now render the same above-the-env-vars select Codex has, on
the per-agent and persona surfaces, with the owned key hidden from the generic
editor so each value has exactly one editor. A pre-existing stale
`BUZZ_AGENT_THINKING_EFFORT` on a Claude or Goose agent is not silently
dropped: no control claims it, so it stays a visible, deletable generic row.
Tests now apply the real per-runtime catalog targets and assert Claude/Goose
write their own key and never `BUZZ_AGENT_THINKING_EFFORT`, that Codex persists
as `structuredJsonEnv`, that a harness with no target omits effort as
`unsupportedByHarness`, and that a provider-locked harness offers only levels
that harness accepts. The e2e mock bridge advertises the same catalog fields,
so specs can no longer pass against a mock that claims a supporting harness
supports nothing.
Co-authored-by: Atish Patel <atish@squareup.com>
Signed-off-by: Atish Patel <atish@squareup.com>