Repository navigation
fix(goose): preserve default-model focus during pending saves - #1495
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source review via Wes’s account, integrating independent systems/lifecycle, correctness/security/concurrency, and product/accessibility reviews.
No actionable findings. The Goose default-model control retains its selected label and reuses the existing focusable loading state and activation guard. The no-provider disabled state and save/error/finally lifecycle remain intact. The public PR description and feature commit metadata were inspected; there are no attached images or identified disclosure issues.
Validation limits: source-only; no tests, builds, or application execution performed for this review. Hosted CI run 39663567323 is successful for this head/base; Windows native validation and browser flake checks were skipped. The added deferred-save test covers stable naming/focus, blocked Enter and successful completion. It does not itself cover compact-save rejection/retry or moving focus elsewhere during that save; existing rejection and movement cases exercise the provider dialog and discovery paths respectively. The PR’s reported browser/native checks were not independently reproduced. Sustained pending/failure focus, screen-reader behavior, visual layout and human acceptance remain unverified by this review.
Retain the selected label and use existing focusable loading behavior while saving. Hold the save in the existing focus cases to check blocked activation, stable naming and recovery. Co-authored-by: Codex <noreply@openai.com> Signed-off-by: James Kraus <jamespkraus@gmail.com>
04fcd77 to
2a67be7
Compare
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Star Lord’s automated source follow-up via Wes’s account, integrating Rocket, Mantis and Groot. No new actionable findings: the production patch is unchanged, and the revised tests exercise the real AgentDefaults host, blocked activation and focus remaining elsewhere during a pending save.
Head 2a67be7; base dc7bc1e. Source-only: no tests or app execution; native pending/error/retry behavior and the attached video’s contents remain unverified.
wesbillman
left a comment
There was a problem hiding this comment.
🤖 Carl, an automated reviewer, commenting via Wes's GitHub account. Mongo did an independent challenge pass.
Verdict: no actionable findings at 2a67be70.
- Only a Goose save sets
pending:saving, plusblocking, whichGooseSetup.saveDefaultssets. Provider and model reads keep their values while they reload, sodisabled={!id}doesn't flip during the save and the trigger stays focusable. - On the family-menu path,
pendinggoes through the shared focusable loading state inButton(aria-disabled, aria-busy, activation blocked), and the accessible name keeps the selected model. Every otherModelPickercaller defaults topending=false, so its behavior is unchanged. - The gated host test holds the save through a deferred promise, checks blocked Enter and focus staying elsewhere, and releases the save in
finally.
Remaining gaps (already disclosed): no native evidence of a sustained pending save, and no native check of save failure and retry. Human acceptance is still pending.
* origin/main: (39 commits) fix(github): keep PR action menu trigger in place (#1736) fix(goose): preserve default-model focus during pending saves (#1495) test(fixtures): reduce polling waits and repeated signing (#1493) test(inbox): wait for conversation focus before filtering (#1491) fix(desktop): keep the main webview running while it is hidden (#1713) chore(release): version desktop previews from 0.7.0 (#1643) fix(agent-harness): remove the unavailable-tools line from the base prompt (#1715) fix(agents): keep View transcript visible in narrow Pi and goose activity panes (#1717) fix(github-plugin): prefix GitHub plugin tools with github_ (#1726) perf(agents): load Claude and Codex activity without scanning every session (#1674) Improve new DM previews, picker performance, and composer focus (#1664) Polish individual message reactions and emoji settings (#1628) Improve dark-mode separators and composer contrast (#1711) Make decision providers plugins, and move TypeSafe into one (#1353) fix(avatars): use static pack portraits in chat (#1670) fix(channels): read the open thread when returning to a channel (#1639) Add app-wide GIF shortcut and composer previews (#1558) Add PR failure/conflict actions and sidebar unlinking (#1599) Polish queued follow-up previews and controls (#1554) feat(conversation): make emoji and mentions built in (#1673) ... Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com>
…-stop * origin/main: (132 commits) feat(workflows): add combined filters and recent sorting (block#1699) ci: smoke test the browser perf suite on pull requests (block#1662) feat(acp): share one ACP client and server pool across harnesses (block#1227) Preserve recipient picker rows and scroll position during paging (block#1739) Group matching emoji reactions at nine people (block#1740) Open channel Canvas from the channel menu in a side-pane tab (block#570) fix(github): keep PR action menu trigger in place (block#1736) fix(goose): preserve default-model focus during pending saves (block#1495) test(fixtures): reduce polling waits and repeated signing (block#1493) test(inbox): wait for conversation focus before filtering (block#1491) fix(desktop): keep the main webview running while it is hidden (block#1713) chore(release): version desktop previews from 0.7.0 (block#1643) fix(agent-harness): remove the unavailable-tools line from the base prompt (block#1715) fix(agents): keep View transcript visible in narrow Pi and goose activity panes (block#1717) fix(github-plugin): prefix GitHub plugin tools with github_ (block#1726) perf(agents): load Claude and Codex activity without scanning every session (block#1674) Improve new DM previews, picker performance, and composer focus (block#1664) Polish individual message reactions and emoji settings (block#1628) Improve dark-mode separators and composer contrast (block#1711) Make decision providers plugins, and move TypeSafe into one (block#1353) ... Signed-off-by: Joah Gerstenberg <joah@squareup.com> # Conflicts: # src/bundled/voice-notes/VoiceCapture.tsx
Why
Saving Goose's default model natively disables the menu trigger, which prevents reliable focus restoration. Reuse the shared button's focusable loading state during the save.
Explainer video
pr1495-explainer-pr.mp4
6:50, narrated. The before and after clips render the real Agent setup section in Chromium, with Goose's save held for about three seconds so the pending state is visible. Chapters: 00:00 Overview · 00:32 The bug · 01:25 The fix · 01:52 What it uses · 02:13 How it works · 04:29 Scope · 04:44 Tests · 05:39 Status · 06:30 Recap.
What changed
ModelPicker.AgentDefaultshost, including completion after focus moves elsewhere.Scope
Covers the Goose default-model control. Shared styles, persistence, and other picker callers keep their existing behavior. Human acceptance remains pending.
Blast radius
The shared picker gains an optional pending state, used by Goose. The shared menu and button own activation blocking and spinner geometry.
Verification
5c043b99d) passed the mandatory TypeScript, full Vitest and design-system gates with one worker.Human check: Settings → Agent setup → Harness defaults → Goose. Change the model, check focus and keyboard navigation, then restart and confirm persistence. During a slow save, check blocked repeated activation and no focus theft after moving elsewhere.
No browser journeys were added or removed. Uses the documented model-menu exception and shared loading recipe.
Stack, bottom to top: #1491 → #1493 → #1495 → #1496.