fix(desktop): auto-restart setup-mode agents when an edit satisfies readiness#1619
fix(desktop): auto-restart setup-mode agents when an edit satisfies readiness#1619baxen wants to merge 1 commit into
Conversation
…eadiness A harness spawned with BUZZ_ACP_SETUP_PAYLOAD runs as a nudge-only setup listener and never re-evaluates its config — readiness is only computed at spawn. Fixing the missing model/provider in Edit Agent therefore changed nothing: the stale setup-mode process stayed connected to the relay and kept replying "needs configuration" to every mention, with no hint that a restart was required. Stamp was-spawned-in-setup-mode on the in-memory ManagedAgentProcess at spawn time (runtime-only, never persisted), and in update_managed_agent stop + respawn the process when the edit makes the record pass readiness — inside the same store-lock section the save already holds, reusing stop_managed_agent_process / start_managed_agent_process. The respawn is best-effort: a failure is surfaced via record.last_error and never fails the save itself. Healthy (non-setup-mode) processes keep the existing "takes effect on next spawn" contract and the needs_restart badge. The readiness gate lives in setup_mode_now_ready, which mirrors the spawn-time computation (effective harness resolution + effective env), with unit tests covering fixed config, still-missing model, still-missing credential, and the non-setup-mode no-op. Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b76292d457
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let restart = | ||
| stop_managed_agent_process(&app, record, &mut runtimes).and_then(|()| { | ||
| start_managed_agent_process(&app, record, &mut runtimes, Some(&owner_hex)) |
There was a problem hiding this comment.
Run the normal pre-spawn snapshot before auto-restarting
For persona-backed agents this new auto-restart path calls start_managed_agent_process directly, bypassing the persona re-snapshot that the normal start path performs in start_local_agent_with_preflight before every spawn. If the linked persona was edited while the setup listener was running, the restarted child is launched from the stale record snapshot, while the stored spawn hash is computed against the current persona, so the UI can show no restart-needed badge even though the process is not actually running the persona config that a normal Stop/Start would use.
Useful? React with 👍 / 👎.
Problem
A harness spawned with
BUZZ_ACP_SETUP_PAYLOADruns as a nudge-only setup listener and never re-evaluates its config — readiness is only computed at spawn (spawn_agent_child). So when a user fixes the missing model/provider in Edit Agent, nothing changes: the stale setup-mode process stays connected to the relay and replies "needs configuration" to every mention, forever, with no hint that a restart is required.update_managed_agentwas explicitly documented "Does NOT auto-restart the agent."Field report: baxen's Fizz (record wiped by the pre-#1583 persona-snapshot bug) kept nudging after the config was fixed; a manual Stop/Start recovered it. Aaron hit the same loop.
Fix
in_setup_modeon the in-memoryManagedAgentProcessat spawn time (runtime-only, never persisted) — set exactly when the spawn emittedBUZZ_ACP_SETUP_PAYLOAD, so the check is cheap and instance-safe (no probing the child's env).update_managed_agent, when the edited record now passes readiness and the live process was spawned in setup mode, stop + respawn it inside the same store-lock section the save already holds, reusingstop_managed_agent_process/start_managed_agent_process.record.last_errorand never fails the save itself.needs_restartbadge from feat(desktop): restart-required badge from spawn-time config hash #1602 — no behavior change for them.The readiness gate is
setup_mode_now_ready, mirroring the spawn-time computation (effective harness resolution → effective env →agent_readiness).Tests
4 new unit tests on the pure seam (
setup_mode_now_ready): fixed config → restart; still-missing model → no restart; still-missing provider credential → no restart; non-setup-mode process → never restarted by an edit.just desktop-tauri-clippyand fulljust desktop-tauri-test(1033 passed) green. Also verified the end-to-end shape manually against the live repro from the bug report (stale setup-mode pid + fixed record → readiness passes → respawn path).Also ratchets the
agent_models.rsfile-size exception (1079 → 1133) per the existing convention; the file is already queued for a discovery-module split.Fixes the forever-"needs configuration" loop reported in buzz-bugs.