refactor(buzz-agent): remove implicit Databricks fallback from resolve_provider#1307
Draft
wpfleger96 wants to merge 24 commits into
Draft
refactor(buzz-agent): remove implicit Databricks fallback from resolve_provider#1307wpfleger96 wants to merge 24 commits into
wpfleger96 wants to merge 24 commits into
Conversation
Four-tier config bridge that reads agent configuration from config files (goose YAML, claude JSON, codex TOML), ACP session data, env vars, and Sprout-explicit overrides — surfacing a unified normalized config surface to the desktop UI regardless of runtime. Key changes: - Config bridge module with per-runtime file readers - ACP session config caching for post-spawn config visibility - AgentConfigPanel component with origin badges and tier provenance - Serde internally-tagged enums matching TypeScript discriminated unions - TOCTOU-safe write path with single lock scope - E2E mock handler for get_agent_config_surface with per-runtime fixtures and a Playwright screenshot spec covering 7 scenarios - Info icon + tooltip on read-only fields; override/strikethrough rendering for superseded config values Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…place badges with provenance sentences
Persona-linked agents had their inherited model/prompt/provider invisible
in the config panel, and the source indicators (checkmark/hourglass icons,
colored badges, footer) were undecipherable.
Phase 1: Resolve all three persona fields (prompt, model, provider) in a
single resolve_config_surface helper called by both get_agent_config_surface
(read) and write_agent_config_field (write). The helper injects resolved
values into the record where absent, calls the reader, then re-tags the
injected fields from BuzzExplicit to PersonaDefault. The re-tag is triple-
gated so a value the user set explicitly in Buzz is never re-tagged. Sharing
the helper keeps the read and write surfaces identical, so plan_config_write
never returns "field not available" for a persona-sourced field. Reader stays
untouched (pure tier-merge function).
Phase 2: Add AgentConfigPanel to ProfileSummaryView in the profile pop-out,
gated on isBot && isOwner && managedAgent defined.
Phase 3: Remove SourcesFooter and colored OriginBadge pills. Replace with
gray inline provenance sentences below each value ("Set in Buzz", "Inherited
from persona", "From environment variable", etc). No action clauses.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Picking a model on a persona-linked running agent now applies to the live instance instead of forcing a restart. The switch rides the existing `desired_model` lever plus the Interrupt requeue/invalidate machinery: a busy turn cancel-switch-requeues onto a fresh session under the new model; an idle session invalidates and re-applies on its next turn. The override is runtime-only — never persisted to `record.model`, gone on restart/respawn, so spawn resolution stays persona-wins. `ControlSignal` drops `Copy` to carry the owned model id; the post-match re-read is replaced by a `requeues()` predicate. A model absent from the agent's catalog surfaces an `unsupported_model` control_result (idle path guards pre-cancel; busy path validates on the re-created session post-cancel) so the picker rejects rather than silently no-opping. The desktop keys the override-active display off the ACP `current_model` diverging from the persona model (the harness-only `desired_model` is unreadable by the reader), shows the persona as a non-struck secondary, and folds the standalone Configuration block into the metadata card. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… flag The runtime-override display keyed off `acp_model != persona_model`, which cannot distinguish a live ModelPicker pick from a session left stale after a mid-life persona edit. Editing a running persona-linked agent's model A->B false-positived as a live override — re-introducing the display-versus-reality bug this surface exists to kill. The harness now stamps a `model_overridden` flag into session_config_captured (true only when a SwitchModel control signal set desired_model, reset on spawn); the reader gates the override on that flag. Also fix multi-channel sendLiveSwitch: it resolved on the first control_result of any status, so a fast `sent` from one channel masked a later `unsupported_model` from another. Now any single rejection fails the pick immediately (fail-fast); success requires every channel to acknowledge. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The config-bridge provenance rows and ModelPicker origin/restart labels used hardcoded text-[11px]/text-[10px] literals. The px-text guard (added to main via the rem-token migration) forbids arbitrary font-size literals so text scales with Cmd +/- zoom. Swap all four to the text-2xs meta-text token (0.6875rem), the documented sibling for these decorative sub-labels. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The any-unsupported-fails-fast counting for a live model switch was locked inside the sendLiveSwitch useCallback in ModelPicker, verified by read but not unit-pinned. Extract the counting (remaining decrement, immediate unsupported reject, resolve-once, unsubscribe-once, timeout fallback) into a pure awaitLiveSwitchOutcome helper with the relay subscription, per-channel sends, and timeout scheduler injected. The component wires the real subscribeControlResults / window timer / dispatch; behavior is unchanged. The helper is node:test-drivable with synthetic frames and a manual clock. The masking-guard test fails against a first-ack-resolves variant and passes against the shipped fail-fast logic. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The interim settled===false checks used a single await Promise.resolve() drain, which the outcome.then callback outruns by one microtask tick, so the guard passed even against a first-ack-resolves variant. Drain five ticks before each interim check so it deterministically regresses an early resolve. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
A genuine-explicit agent (own record model, no persona) that live-switched mid-session rendered nowhere: resolve_config_surface passed no override baseline for an explicit record, so apply_runtime_override early-returned and the panel showed the stale record model as primary with the live model struck through — display contradicting reality. Carry the override baseline as a typed (value, origin) pair end-to-end so the secondary is tagged by its true source (BuzzExplicit for the record case, PersonaDefault for personas) instead of a hardcoded PersonaDefault. Build the record-model baseline only when model_overridden is true, so a persona edited mid-life still does not false-positive. On a live pick equal to the baseline, yield a clean single-value field rather than passing the pre-polluted base through (build_model_field independently sets an AcpConfigOption secondary for the record-plus-live-session case). Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The live-acp-vs-record override is now exclusively apply_runtime_override's job, gated on model_overridden. build_model_field's acp-derived secondary predated that gate: with record_model=Some(X) and acp_model=Some(Y) it populated overridden_value=Some(Y) unconditionally, and that row passed straight through apply_runtime_override's !model_overridden early-return — surfacing a live override before any switch was applied. Collapse the secondary to express only the static record-vs-file precedence (a Buzz-explicit model shadowing a config-file model); drop all acp_model references from it. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The v4 screenshots captured the old origin-badge UI. The shipped surface
now renders inline provenance sentences ("Set in Buzz", "Inherited from
persona", "Live override (this session only)", etc.), a folded config panel,
and a non-struck persona baseline for runtime overrides.
Re-grounds the screenshot scenarios to the sentence-based render and adds a
multiOriginSurface fixture (one distinct origin per row) so the provenance-
sentences shot witnesses multiple distinct sentences in one frame instead of
duplicating the folded-panel capture.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…tooltips The agent side-panel Configuration section rendered with AgentConfigPanel's own dense header (small inline icon, muted heading) inside the ProfileFieldGroup card, so it read as bolted on rather than part of the card. Re-skin the section header to the panel's ProfileFieldRow convention (circular icon badge, px-4 row rhythm, foreground label) so it reads as one more section; the divider stays. Long config values truncate correctly but had no way to reveal the full text. Add a native title attribute carrying the untruncated value on the truncated NormalizedRow/AdvancedRow values, guarded so null placeholders get no tooltip — matching the existing ProfileFieldRow title convention. Covers both the side panel and the Agents nav menu since both embed the same AgentConfigPanel. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
The truncated NormalizedRow override span (config-file system prompt in the dual-prompt case) rendered inside the truncate container with no title, so its full text was unreadable on hover — the same gap the primary value tooltip already closes. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Main's d256935 added agent_command_override, persona_source_version, and provider to ManagedAgentRecord. Two test fixture initializers on this branch were missing them, causing E0063 compile errors in CI. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Opens the user-profile-panel for a managed agent by clicking its avatar in the #agents channel, waits for the Configuration section to render, and captures the full panel. Covers the isBot && isOwner path that renders the Configuration footer inside ProfileSummaryView. Co-authored-by: Will Pfleger <wpfleger@block.xyz> Signed-off-by: Will Pfleger <wpfleger@block.xyz>
… extraction codex.rs and claude.rs previously enumerated a fixed set of config keys in their extra blocks, silently dropping any field not in the list as new options were added upstream. This meant settings like service_tier, plan_mode_reasoning_effort, and alwaysThinkingEnabled were invisible in the config surface. Check in JSON Schema snapshots for both runtimes and introduce schema_walker::extract_schema_fields, which walks the schema's top-level properties and surfaces every key the user has actually set. Scalars stringify directly; arrays become '[N items]'; objects flatten one level as key.subkey = value. Normalized fields (model, provider, mode, thinking_effort, etc.) are passed via a skip list to avoid double-counting. Also adds schema_version (fetched_at from versions.json) to RuntimeFileConfig and threads it through to ConfigSourceReport as config_schema_version so the frontend can surface which schema snapshot was used. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…d, add SHA-change warnings, consolidate VERSIONS_JSON
schema_walker.rs: expand doc comment to explain that object subkeys are
iterated from the config value (not filtered against schema properties),
making arbitrary keys like env vars visible. Also move VERSIONS_JSON
embed and schema_version() lookup here so codex.rs and claude.rs share
one embed instead of two identical constants.
claude.rs: add inline comment on provider_locked insert to make clear
it is a Buzz-synthesized annotation, not a field from the user's file.
codex.rs: remove duplicate VERSIONS_JSON constant and parse_codex_schema_version();
delegate to schema_walker::schema_version("codex").
refresh-harness-schemas.sh: read old SHAs from versions.json before
overwriting, then print a warning line for any harness whose SHA changed.
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…ld extraction The schema-driven approach (extract_schema_fields + vendored JSON Schema snapshots) had a fundamental flaw: a drifted snapshot is functionally equivalent to a hardcoded field list. Any new config key added upstream would be silently invisible until someone ran refresh-harness-schemas.sh. The committed schema files also triggered Biome linting failures on CI. Replace with config-driven iteration: walk the user's own config object directly, surface every key they have set, skip only the normalized fields extracted into typed struct fields (model, provider, mode, etc.). No schema required, no vendored JSON, no drift, no Biome issue. extract_schema_fields(schema_json, config, skip) → extract_config_fields(config, skip) Remove: CODEX_SCHEMA, CLAUDE_SCHEMA, VERSIONS_JSON constants; schemas/ directory (codex.config.schema.json, claude-code-settings.schema.json, versions.json); schema_version() function; schema_version field on RuntimeFileConfig; config_schema_version field on ConfigSourceReport; refresh-harness-schemas.sh script. The skip lists in codex.rs and claude.rs are the only harness-specific coupling that remains — they map harness config keys to normalized struct fields. That coupling is intentional and load-bearing. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… config bridge Two bugs in the agent config panel: 1. agent_config.rs resolved runtime_meta via record.agent_command (the create-time snapshot, never updated). When a user changes an agent's harness in the UI the change goes to the persona's runtime field, not agent_command. Both get_agent_config_surface and write_agent_config_field now call effective_agent_command (the same resolution the spawn path uses) so the config bridge dispatches to the correct harness. 2. reader.rs built the advanced section exclusively from file_config.extra (tier 2b). Any env vars in record.env_vars beyond the four normalized fields (model, provider, thinking effort, system prompt) were invisible. read_config_surface now appends remaining record.env_vars to advanced as BuzzExplicit / RespawnWithEnvVar fields, skipping keys already covered by normalized fields and keys already present in file_config.extra. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…flattening
Four display correctness fixes in the config bridge:
1. types.rs: add HarnessConstraint variant to ConfigOrigin. The locked
provider branch in reader.rs was using EnvVar which caused the UI to
show "From environment variable" for Alia's Anthropic provider — it is
a harness constraint, not a user-set env var.
2. reader.rs: use ConfigOrigin::HarnessConstraint in the provider_locked
branch so Claude Code's locked Anthropic provider displays correctly.
3. goose.rs: infer provider="databricks" when DATABRICKS_HOST is present
but no explicit GOOSE_PROVIDER or active_provider is set. This surfaces
the effective provider for agents on the Databricks OAuth path (Paul,
Duncan) who previously showed no provider in the panel.
4. codex.rs: default provider="openai" when model_provider is absent.
Codex uses OpenAI implicitly when no provider is configured; Thufir
previously showed no provider.
5. schema_walker.rs: recurse two levels deep instead of one. Objects at
depth 2 are now flattened as "key.subkey.subsubkey" rather than "{...}".
This correctly surfaces codex [projects."<path>"] trust_level entries
and [tui.model_availability_nux] model keys. Depth 3+ still shows "{...}".
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…e types Add "harnessConstraint" to the ConfigOrigin union in types.ts to match the new Rust variant added in 786bbe5. Add a provenanceSentence case in AgentConfigPanel.tsx so the UI shows "Locked by harness" instead of falling through to an unhandled case. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
… values
Three fixes:
1. goose.rs: remove dead nested.host branch from Databricks inference arm.
The nested struct is Some only when active_provider is Some, but arm 2
already returns Some(active_provider) in that case — arm 3 never executes
when active_provider is set. The nested.host condition was always false
when reached. Only the flat DATABRICKS_HOST check is needed.
2. schema_walker.rs: emit "{...}" for empty inner objects instead of
silently dropping the key. The previous two-level walker iterated over
an empty object's entries and produced zero output, making the key
disappear entirely. Reverts the claude.rs test workaround (pre-commit: {}
now correctly asserts hooks.pre-commit = "{...}").
3. schema_walker.rs: join scalar arrays comma-separated instead of showing
"[N items]". Adds format_array() helper: when all elements are scalars
(string, bool, number, null), joins them with ", "; when any element is
a nested object or array, falls back to "[N items]". This surfaces
tui.status_line as its actual values rather than "[6 items]".
Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Mark normalized config fields as required per harness so the UI can eventually prompt users to set the minimum viable config before an agent will work. Required-ness is harness-specific: buzz-agent and goose require model + provider; claude code and codex have no user-configurable required fields. Adds required_normalized_fields to KnownAcpRuntime, threads is_required through the Rust reader/writer, and mirrors isRequired in the TypeScript NormalizedField type and e2e bridge fixtures. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…e_provider buzz-agent had three silent fallback paths in resolve_provider() that would infer or switch to the Databricks provider when credentials for the requested provider were absent. This made Databricks a first-class special case in what should be a provider-agnostic runtime. Remove all three paths: - anthropic requested but no ANTHROPIC_API_KEY → now errors (was: silently use Databricks) - openai requested but no OPENAI_COMPAT_API_KEY → now errors (was: silently use Databricks) - no BUZZ_AGENT_PROVIDER set → now errors (was: infer Databricks if DATABRICKS_HOST+MODEL present) BUZZ_AGENT_PROVIDER is now the canonical required selector. The explicit databricks provider path is unchanged — DATABRICKS_HOST and DATABRICKS_MODEL are still read from the environment (inherited from the parent shell; no env_clear() in runtime.rs). Replace build_databricks_defaults() in runtime.rs with build_buzz_agent_provider_defaults(), which bakes BUZZ_AGENT_PROVIDER and BUZZ_AGENT_MODEL at compile time via BUZZ_BUILD_BUZZ_AGENT_PROVIDER and BUZZ_BUILD_BUZZ_AGENT_MODEL. Swap the corresponding vars in build.rs. buzz-releases will be updated in tandem to set these new vars. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
…sertion Two test names in config.rs still described the old fallback behavior after the behavior was inverted. Rename to match what they now test: - resolve_provider_falls_back_to_databricks_when_requested_token_missing → resolve_provider_errors_when_requested_provider_key_missing - resolve_provider_can_auto_select_databricks_without_explicit_provider → resolve_provider_errors_when_provider_env_absent Also strengthen buzz_agent_provider_defaults_empty_in_oss_build in runtime/tests.rs: the previous version only verified the function didn't panic. Now it runs `env` as the command and asserts BUZZ_AGENT_PROVIDER and BUZZ_AGENT_MODEL are absent from the child's environment, matching the strength of the old databricks_defaults_empty_in_oss_build test. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
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
1 participant
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.
What
Remove all three implicit Databricks fallback paths from
resolve_provider()incrates/buzz-agent/src/config.rs, makingBUZZ_AGENT_PROVIDERthe canonical required selector.Previously, buzz-agent had three silent fallback paths that made Databricks a first-class special case in a runtime that should be provider-agnostic:
BUZZ_AGENT_PROVIDER=anthropicbut noANTHROPIC_API_KEY→ silently switched to DatabricksBUZZ_AGENT_PROVIDER=openaibut noOPENAI_COMPAT_API_KEY→ silently switched to DatabricksBUZZ_AGENT_PROVIDERset butDATABRICKS_HOST+DATABRICKS_MODELpresent → silently inferred DatabricksAll three paths are removed. Each now returns an error instead.
Why
The implicit fallback is an abstraction violation — buzz-agent should be provider-agnostic like goose. Silently switching providers when credentials are missing makes debugging harder and the behavior surprising. Making
BUZZ_AGENT_PROVIDERrequired is the correct model: explicit provider, no inference.DATABRICKS_HOSTandDATABRICKS_MODELare still read from the environment for the explicitdatabricksprovider path. Sinceruntime.rsdoes not callenv_clear(), these are inherited from the parent shell for all Block developers who already have them set for goose.Changes
crates/buzz-agent/src/config.rs— remove 3 fallback paths fromresolve_provider(), removedatabricks_available()helper anddatabricks_readyvariable, shrink signature from 5 params to 3, rename 2 tests whose names described the old (inverted) behavior, update all 4 tests to assert new behaviordesktop/src-tauri/src/managed_agents/runtime.rs— replacebuild_databricks_defaults()withbuild_buzz_agent_provider_defaults()that bakesBUZZ_AGENT_PROVIDERandBUZZ_AGENT_MODELat compile timedesktop/src-tauri/build.rs— swapBUZZ_BUILD_DATABRICKS_HOST/MODELforBUZZ_BUILD_BUZZ_AGENT_PROVIDER/MODELdesktop/src-tauri/src/managed_agents/runtime/tests.rs— strengthenbuzz_agent_provider_defaults_empty_in_oss_buildto assertBUZZ_AGENT_PROVIDERandBUZZ_AGENT_MODELare absent from the child's environment (not just panic-free)Companion work
buzz-releases will be updated in tandem to set
BUZZ_BUILD_BUZZ_AGENT_PROVIDER=databricksandBUZZ_BUILD_BUZZ_AGENT_MODEL=<model>, replacing the removedBUZZ_BUILD_DATABRICKS_HOST/MODELvars.