fix(buzz-acp): report missing models without retrying - #7538
Conversation
Signed-off-by: Diem Nguyen <diem@squareup.com>
🔐 Codex Security Review
|
Signed-off-by: Diem Nguyen <diem@squareup.com>
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Reviewed head a37b5214f1b63592eeb72d750b039b450b2d5264 against base 82656ffea080cc28cec9163ebbf7d1c6be327e43.
One P2: the new recovery instructions omit the step that applies the selected model to the running agent. Please include saving and restarting/applying the configuration before resending, and update the notice regression expectation. No new restart machinery is needed.
The narrow error discriminator matches buzz-agent’s actual error serializer and ACP decoding. Other resource errors retain the previous retry path; the healthy process return and detailed error observer payload remain intact.
Validation: exact-range source review; an exact-head execution of the production auto-restart predicate confirmed that an immediate settings save only arms its timer, while opted-out auto-restart stays disabled. Existing GitHub Rust lint/unit/Windows checks passed at this head. No broad suites rerun locally, and no live Desktop/provider recovery E2E performed. The overall GitHub Actions run was still in progress at the status snapshot; merge/security gates are separate from this code finding.
| let content = "⚠️ I couldn't process the last request: the configured model \ | ||
| wasn't found at the provider's endpoint. Open agent settings and select a \ | ||
| different one from the dropdown, then re-send your request." | ||
| .to_string(); |
There was a problem hiding this comment.
[P2] Include the restart/apply step before asking the user to resend
After this application error, the agent process and its existing session are returned to the pool (pool.rs:3370–3395, lib.rs:5065–5075). For a running standalone agent, selecting another model in AgentInstanceEditDialog and saving only calls updateManagedAgent; the backend persists the model but restarts immediately only for access-policy changes (agent_models_update.rs:137–181,266–295,453–471). The ordinary auto-restart policy requires three continuous idle minutes and can be disabled (autoRestartPolicy.ts:49–83).
Therefore a user who follows the new instruction, saves another model, and immediately resends still runs the unavailable model and gets the same failure. Please explicitly tell them to save and restart/apply the agent configuration before resending, and adjust the regression test text. This is a correction to the newly added recovery advice, not a request to change the existing restart policy.
There was a problem hiding this comment.
🤖 Addressed in ed5f54f. The notice now tells users to select another model, save their changes, restart the agent to apply the configuration, and then re-send. Updated the existing notice regression expectation; the restart policy is unchanged. All 30 focused error-outcome tests and the full local just ci gate passed, including 932 buzz-acp library tests, 6,483 desktop tests, and 2,098 mobile tests. Refreshed the PR description and local UI captures with the exact corrected text. Please take another look.
Signed-off-by: Diem Nguyen <diem@squareup.com>
|
🤖 Updated local test captures for the model-not-found recovery notice. These use seeded conversation data and the exact text from commit ed5f54f. They verify the thread layout, not a deployed provider recovery flow. BeforeThe first failure leaves the thread without a reply while Buzz retries the unavailable model. AfterBuzz immediately directs the user to select another model, save, restart the agent to apply the configuration, and re-send the request. |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-review: prior blocker resolved; no actionable blockers remain
Reviewed head ed5f54f6cc38c5e556a95748fe2328c5a4f426c7 against base 82656ffea080cc28cec9163ebbf7d1c6be327e43, focusing on the revision since previously reviewed head a37b5214f1b63592eeb72d750b039b450b2d5264.
The prior P2 is resolved: the notice now explicitly tells the user to select another model, save, restart the agent to apply the configuration, and only then resend. The production notice and its exact regression expectation are the entire two-hunk delta. The narrow model-not-found discriminator, other-resource retries, detailed error telemetry, healthy-process reuse, and existing restart policy remain unchanged. No new runtime machinery is needed.
Princess Donut independently confirmed the original recovery-copy finding is closed; Carl verified the complete delta before integrating that result.
Validation: verified the complete delta and read the production notice/HTTP-capture regression. Existing exact-head GitHub Actions run 34400965627 completed successfully. Its Rust Unit Tests log explicitly records the missing-model notice test and non-model resource retry test passing, plus all 932 buzz-acp library tests (and the 941-test package run). No local suite rerun or live Desktop/provider recovery E2E was performed for this copy-only revision. The author's seeded UI captures are presentation evidence, not a deployed recovery workflow.
The current-range Codex security review remains stale/skipped in the inspected snapshot; that is a separate review gate, not a code finding. This is a review comment, not an approval.


Summary
When an agent reports model-not-found, Buzz retries the unavailable model and delays the failure reply until retries are exhausted. Stop retrying this error and immediately post a threaded recovery notice. The notice tells users to select a different model in agent settings, save, restart the agent to apply the configuration, and re-send their request.
This adds one error-handling branch and regression coverage in
buzz-acp. It matches-32002errors containingmodel not found. Other resource-not-found errors, such as stale sessions, retain the existing retry behavior. Detailed error events remain available for diagnosis. The existing restart policy is unchanged.Related issue
None found in existing issue/PR searches for model-not-found recovery.
Testing
Playwright captured and visually checked the thread UI with seeded conversation data and the exact recovery text. The check opens the request's thread, confirms no reply before the failure, injects the notice, and verifies the full text is visible. Before/after screenshots show the corrected save-and-restart instructions. These are local test captures, not a deployed provider recovery flow.
Generated with Codex