Skip to content

feat(desktop): show which config fields need a restart - #3652

Closed
wpfleger96 wants to merge 2 commits into
mainfrom
duncan/restart-diff-rust-core
Closed

feat(desktop): show which config fields need a restart#3652
wpfleger96 wants to merge 2 commits into
mainfrom
duncan/restart-diff-rust-core

Conversation

@wpfleger96

@wpfleger96 wpfleger96 commented Jul 30, 2026

Copy link
Copy Markdown
Member

The "Restart required" badge is driven by a lossy u64 digest of the effective spawn config (spawn_config_hash()), so it can report that something drifted but never what. This replaces the digest with a typed SpawnConfigSnapshot and ships a redacted field-by-field diff on ManagedAgentSummary.restart_diff for the UI to render.

One comparison, one vector

SpawnConfigSnapshot::canonical() is the single JSON projection that both the badge and the diff read. Drift is to_value(stamped) != to_value(current); the diff is a generic leaf walk over those same two values, so badge-on and diff-non-empty hold by construction rather than by convention.

eligible_restart_diff() returns the final vector — snapshot walk entries plus a synthetic adapter_availability entry. It returns empty for an orphaned instance (spawn_agent_child refuses to spawn one, so the badge would offer an action guaranteed to fail) and for an agent with no tracked pair runtime, which was never stamped and so can never be shown to have drifted. Both ineligible states are inputs to that one helper rather than conditions at the call site. needs_restart = !restart_diff.is_empty() is derived from that vector and nothing else; restart_eligible() is gone.

Generic by default

The walk knows nothing about which fields exist: serde field names are the diff path ids, dynamic map keys append verbatim (env.OPENAI_API_KEY), and object-key unions are walked in lexicographic order for stable UI truncation. Adding a snapshot field reaches the UI with no change to the diff code. mutation_table_covers_every_serialized_field fails if a new field arrives without a mutation row.

The only per-field knowledge is policy_for(path), four arms:

Policy Paths Rendering
Text system_prompt, team_instructions character counts only
MaskedBare args, relay_url ••••, no suffix
MaskedSuffix auth_tag, env.* •••• + last 4 chars when longer than 8
Plain everything else verbatim

args and relay_url get no suffix because both can legitimately carry credentials — --token=... is a legal argument, and normalize_relay_url rejects userinfo but deliberately preserves query strings. Masking is character-based throughout, and raw values drive the comparison so two secrets with colliding suffixes still read as drift.

That same policy backs the snapshot's manual Debug via redacted_canonical(). ManagedAgentProcess derives Debug, so a derived impl on the snapshot would print secrets to logs; routing both surfaces through one policy means they cannot diverge.

Pre-spawn stamping

The stamp is now built from the already-resolved values that populated the Command, before command.spawn(). The previous post-spawn re-resolution left a window where a persona, harness, or global edit landing in between would stamp the new config onto a child running the old one, silently suppressing the badge.

Wire shape

"restart_diff": [
  { "field": "model",              "change": { "kind": "value",  "before": "gpt-5", "after": "claude-4" } },
  { "field": "system_prompt",      "change": { "kind": "text",   "before_chars": 1234, "after_chars": 1410 } },
  { "field": "env.OPENAI_API_KEY", "change": { "kind": "masked", "before": "••••bc12", "after": "••••xyz9" } },
  { "field": "env.BUZZ_LOG",       "change": { "kind": "added" } }
]

Omitted when empty. added/removed occur only for dynamic-map keys; nullable struct fields always serialize as null, and arrays are atomic leaves (args, never args.0).

The restart-required badge was driven by a lossy u64 digest of the
effective spawn config, so it could report that something drifted but
never what. Replace the digest with a typed SpawnConfigSnapshot whose
canonical JSON is the single representation both the badge and a new
redacted `restart_diff` read, making badge-on and diff-non-empty true
by construction rather than by convention.

The diff is a generic walk of that JSON, so a future snapshot field
reaches the UI with no diff-code change; the only per-field knowledge
is a path-based masking policy that also backs the snapshot's manual
Debug, keeping one redaction authority for env values, auth tags, CLI
args (`--token=` is legal), and relay URLs (the normalizer preserves
query strings).

The stamp now happens before spawn(), built from the values that
populated the Command. Re-resolving afterwards let an edit landing in
between stamp the new config onto a child running the old one.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96
wpfleger96 requested a review from a team as a code owner July 30, 2026 00:15
An agent with no tracked pair runtime — adopted via a persisted
runtime_pid, or simply stopped — was never stamped with a spawn config,
so it can never be shown to have drifted. That state lived only in the
Option chain at the call site and could not be expressed by the final
vector helper, which required a stamp. Absent state is now an explicit
input, making the sixth eligibility case structural and directly
testable.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
@wpfleger96

Copy link
Copy Markdown
Member Author

🤖 Consolidated into #3637 at Will's request. All of Duncan's Rust changes (spawn snapshot, diff engine, redaction policy, tests) are present in #3637 at tip ad4cdb1ea. Closing this PR.

@wpfleger96 wpfleger96 closed this Jul 30, 2026
wpfleger96 added a commit that referenced this pull request Aug 4, 2026
The "Restart required" badge reports that an agent's running config has
drifted from its spawn-time config, but never says what changed. This
ships the full feature: a typed Rust diff engine and a TS/UI layer that
renders it at every badge site.

## Rust core (spawn-snapshot diff engine)

Replaces the lossy `u64` `spawn_config_hash` with a typed
`SpawnConfigSnapshot`. The snapshot is stamped from the already-resolved
command/env/config values immediately before `spawn()`, closing the race
window where a mid-spawn config edit would suppress the badge.

`SpawnConfigSnapshot::canonical()` is the single JSON projection shared
by the badge and the diff. Drift is `to_value(stamped) !=
to_value(current)`; the diff is a generic leaf walk over those same two
values, so badge-on and diff-non-empty are structurally guaranteed.
Adding a snapshot field reaches the UI with no code change to the diff
engine — `mutation_table_covers_every_serialized_field` fails CI if a
new field arrives without a mutation row.

`eligible_restart_diff(persona_orphaned, Option<TrackedSpawnState>)`
returns the final vector — snapshot walk entries plus a synthetic
`adapter_availability` entry. It returns empty for an orphaned instance
(spawning one would fail) and for agents with no tracked spawn state
(never stamped, can never have drifted). `needs_restart =
!restart_diff.is_empty()` derives from that vector and nothing else.

Redaction policy (`policy_for(path)`) is shared by the wire diff and the
snapshot's manual `Debug` via `is_safe_to_reveal()` from
`managed_agents::env_vars` as the single authority for env-key masking:

| Policy | Paths | Rendering |
|---|---|---|
| `Text` | `system_prompt`, `team_instructions` | character counts only
|
| `MaskedBare` | `args`, `relay_url` | `••••`, no suffix |
| `MaskedSuffix` | non-allowlisted `env.*` | `••••` + last 4 chars when
longer than 8 |
| `Plain` | allowlisted `env.*` (`BUZZ_AGENT_THINKING_EFFORT`,
`BUZZ_AGENT_PROVIDER`, `BUZZ_AGENT_MODEL`, `DATABRICKS_HOST/MODEL`) and
everything else | verbatim |

Default-deny: every env key not in the explicit allowlist stays masked.
`is_safe_to_reveal()` is the single allowlist authority for both the
baked-env display and the diff.

`restart_diff` is omitted from the wire when empty
(`skip_serializing_if`).

## TypeScript / UI layer

New `restartDiff.ts` module defines `RestartDiffEntry`, `RestartChange`,
`JsonValue`; `tauri.ts` and `types.ts` re-export and add `restart_diff`
/ `restartDiff` fields (Rust omission → `restartDiff: []`).

**`RestartDiffBadge`** — hover tooltip capped at 6 entries + "and N
more", `asChild` span trigger (never inside a `<button>`), auto-restart
blurb below the diff list (on/off variant from `autoRestartEnabled`
prop; same `AUTO_RESTART_ON_BLURB` / `AUTO_RESTART_OFF_BLURB` constants
shared with the Runtime-tab banner). **`RestartDiffList`** renders the
full uncapped list for the Runtime-tab banner with `tooltip`/`inline`
presentation variants for correct foreground in both surfaces.

**`ManagedAgentRow` B4 fix** — badge moved to a sibling `div` of the row
expansion button; tooltip trigger has no `button` ancestor.

**`UnifiedAgentsSection`** — both badge sites render
`<RestartDiffBadge>` instead of a raw `<Badge>`, with
`autoRestartEnabled` threaded from `agent.autoRestartOnConfigChange`.

**Side-panel fix** — `RestartDiffBadge` rendered tab-independently in
the `ProfileSummaryView` hero area (was Runtime-tab only — root cause of
the ~50% inconsistency Will reported). Hero badge is `self-center` in
the flex column. `ProfileRuntimeTabContent` early-return checks
`needsRestart` so the banner is never dropped when all other content is
empty. Auto-restart blurb in the Runtime-tab banner uses the shared
constants.

## Wire shape

```jsonc
"restart_diff": [
  { "field": "model",              "change": { "kind": "value",  "before": "gpt-5", "after": "claude-4" } },
  { "field": "system_prompt",      "change": { "kind": "text",   "before_chars": 1234, "after_chars": 1410 } },
  { "field": "env.OPENAI_API_KEY", "change": { "kind": "masked", "before": "••••bc12", "after": "••••xyz9" } },
  { "field": "env.BUZZ_AGENT_THINKING_EFFORT", "change": { "kind": "value", "before": "medium", "after": "high" } }
]
```

`added`/`removed` occur only for dynamic-map keys; nullable struct
fields always serialize as `null`; arrays are atomic leaves (`args`,
never `args.0`).

## Tests

**Rust** — 1902 passing: snapshot mutation coverage, diff entry
serialization, allowlist-aware env masking
(`allowlisted_env_key_shows_plain_value`,
`allowlisted_env_key_is_case_insensitive`,
`non_allowlisted_env_key_stays_masked`),
`unstamped_agent_yields_no_badge_and_no_entries` (both orphan values),
`summary_without_drift_omits_restart_diff_from_the_wire`,
`unstamped_availability_is_not_drift`. Clippy clean, fmt clean.

**TypeScript** — `needs-restart-screenshots.spec.ts`: 11 E2E cases
registered in the smoke project — all three badge sites, tooltip +
keyboard focus, DOM no-button-ancestor assertion, 6+1 truncation,
uncapped Runtime list, unknown field humanisation, side-panel badge on
default Info tab, inactive/friendly-error Runtime opening path.

Consolidates [#3652](#3652)

Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant