Skip to content

Align background agent pickers and defaults - #136

Closed
hamzamerzic wants to merge 4 commits into
mobius-os:mainfrom
hamzamerzic:fix/background-agent-picker-defaults
Closed

Align background agent pickers and defaults#136
hamzamerzic wants to merge 4 commits into
mobius-os:mainfrom
hamzamerzic:fix/background-agent-picker-defaults

Conversation

@hamzamerzic

Copy link
Copy Markdown
Collaborator

Summary

  • give Settings and background-agent apps the same ordered primary/fallback model semantics
  • make the first connected provider the primary default without disrupting an existing provider order
  • update the built-in Claude and Codex catalogs and friendly labels to the current supported choices
  • preserve explicit app overrides while allowing background apps to inherit “Default from settings”

Companion app PRs

Verification

  • 41 focused backend provider/auth/settings tests
  • focused Settings appearance contract
  • production frontend build

@hamzamerzic

hamzamerzic commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Second-pass review is holding this exact head 2ca48e080b83c4289127b7efdfe829ac943980bc for one durability/UI-state blocker and one documentation correction:

  1. onProviderConnected starts persistBackgroundAgents(next) and, for the first provider, a separate api.settings.save({ provider }), then immediately closes the auth panel. Neither mutation is awaited as one outcome; the provider failure is swallowed completely. This can leave the UI looking complete while only one half of the promised first-provider default has persisted. Please coordinate/await both writes (or send one atomic settings request), retain a visible retryable error on either failure, and add a regression where one write succeeds and the other times out/fails.
  2. get_model_prefs still says absent and explicit-empty prefs are equivalent/show every entry, but this PR intentionally makes absent prefs use the curated hidden set while explicit {"hidden_ids": []} shows all. Please update the contract text and test the missing-vs-explicit-empty distinction at the route boundary.

The provider/model/background-default separation otherwise looks directionally coherent. Please do not merge this SHA until the two-write transition is truthful under partial failure.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

The first-provider write is now one backend request, but exact 9031162 still has a frontend stale-request bypass: persistBackgroundAgents returns true on reqId !== backgroundSaveReqRef.current before checking res.ok. If a later picker edit increments the request id while the companion {provider, background_agents} write is queued/in flight, a 4xx/5xx companion response is treated as success; onProviderConnected clears the auth panel even though the interactive provider/default transition never persisted. Please always validate the response for the promise's own caller; use request id only to suppress stale shared UI/cache updates. Add a deferred-response regression: first-provider companion save starts, a later background save supersedes the display generation, companion returns failure, and the connection flow must stay open/report failure.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Final review follow-up on exact head 59d18e567cd2f802a56d3d3e17572e97b3ecfc76: both previously found transition races are fixed. First-provider setup now persists provider plus background-agent defaults in one awaited request, stale request ids suppress only shared UI/cache updates (never the caller result), and failure keeps the auth panel open with a retryable error. The missing-vs-explicit-empty hidden-model contract is documented and tested. Integration review also caught and fixed the Recovery Claude snapshot after merging current main. Backend focused suite: 108 passed; frontend library/hook suite and pre-push gate pass; production build passed before the final main merge. No remaining source finding; hosted backend/e2e are the final gate.

@hamzamerzic
hamzamerzic force-pushed the fix/background-agent-picker-defaults branch from 59d18e5 to 98ff642 Compare July 22, 2026 18:36
@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Implemented the requested stale-save blocker fix on exact head 98ff6428a93a0381b5a15934068d3f41df77703f.

The response is now validated before request staleness is consulted. A failed companion provider save therefore still throws, remains visible through the existing companion-error path, returns false, and leaves the auth panel open even if a later background-agent edit has advanced the request id. Successful superseded responses still take the stale-success path.

Added runtime regressions for both cases, including an assertion that the stale callback is never consulted for a failed response. Rebased onto restored main (38f2e158) and preserved both sides of the additive Recovery registry-test overlap.

Validation on the exact head:

  • focused backend settings/auth/registry suite: 91 passed
  • full frontend unit suite: 1,772 lib + 47 hooks passed
  • production frontend build: passed
  • git diff --check origin/main..HEAD: clean

Hosted exact-head CI is in progress; the completed jobs are green so far.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

APPROVED on exact head 98ff6428a93a0381b5a15934068d3f41df77703f.

The stale-response blocker is fixed: HTTP failure is authoritative before request-id staleness, companion failure remains surfaced, and the auth panel cannot close on that failed save. The regression exercises the runtime helper and proves a failed response does not even consult the stale predicate; successful superseded responses still settle as stale success.

Exact-head hosted CI is fully green (backend, E2E, frontend unit, core-app unit/sync, packager, privacy), in addition to the local 91-test focused backend run, complete frontend unit run, and production build documented above.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Updated-head review — delta round 1/2 (98ff6428)

Reviewed the new settleBackgroundAgentSave extraction and all call-site control flow. The ordering is now correct: a failed response is interpreted before freshness, so a first-provider companion save cannot be misclassified as harmlessly superseded; persistBackgroundAgents returns false, preserves the visible error, and onProviderConnected keeps the auth panel open. A successful superseded save still settles without a stale cache invalidation.

Verification on the exact head: 7 focused frontend tests passed, including both stale/failure cases. No blocker found in this delta.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Updated-head review — delta round 2/2 (98ff6428)

Second pass traced the serialized save chain through overlapping normal edits and first-provider companion writes, including response JSON failure, later queued success/failure, panel closing, error clearing, and owner-query invalidation. The companion-write flag correctly keeps its own durable failure visible even after a later request has incremented freshness; ordinary superseded edits remain allowed to defer to the newest save.

Verification: 91 focused backend tests passed, Python compile and diff checks passed, and hosted frontend/unit checks are green (backend/e2e still running at review time). The updated head remains approved subject to CI.

@hamzamerzic

Copy link
Copy Markdown
Collaborator Author

Landed on protected main through reviewed integration #147 (merge 226106c). The integrated patch matches this PR's reviewed head; closing the superseded source PR to keep the queue truthful.

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