Conversation
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Model choice has multiple competing interpretations across KeyVault, the desktop picker, mobile catalog, session persistence, and the running provider. A refresh can overwrite newer account preferences; a model/account change can be acknowledged while a stale runtime remains installed; rapid selections and late save replies can mix or roll back the pair. Ordinary KeyVault saves also merge a UI snapshot instead of the latest persisted record, so concurrent edits can erase unrelated fields.
Solution
Make KeyVault the authoritative account catalog and apply discovery and ordinary account edits against its latest locked record. Keep discovered defaults separate from explicit user overrides, preserve an intentionally empty enabled inventory, and project the same selectable models to desktop and mobile. Route desktop and mobile session identity changes through one serialized persistence/runtime boundary, capture the admitted provider and lease for an in-flight turn, and queue optimistic frontend pair/default saves so only confirmed current intent updates recent/default choices. Existing selection controls and interaction steps remain intact.
Potential risks
credentials.jsongains serde-defaulted catalog generation and discovered-default fields. Old files load without migration; legacydefault_variantsare conservatively treated as explicit user choices. The new optional RPC catalog and override fields remain backward compatible, but an older app version cannot enforce these new concurrency guarantees. To roll back, revert this commit and retain a backup of the credentials file before any older build rewrites it; no automatic historical cleanup is performed. Per-session and per-account queues add bounded memory and waiting time, with no new polling. Cross-process writes to the same credentials file remain outside the in-process lock. Real provider discovery, signed mobile UI, and live CPU/RSS were not measured. Six agent-core files also overlap the open agent finalization PR #2105; GitHub may require conflict resolution when that PR advances.Verification
CI follow-up: reproduced the sole failing
ModelVariantInlineCardassertion locally, then corrected its inferredo4-minifixture and added explicit catalog-key compatibility plusmini/nanoresolver coverage. Runtime behavior is unchanged; the component edit only corrects a stale comment.pnpm test src/modules/MainApp/Integrations/KeyVault/shared/ModelTable src/util/__tests__/modelVariants.test.ts src/util/__tests__/selectableModelVariants.test.ts src/util/__tests__/defaultModelVariant.test.ts src/util/__tests__/variantEditOptions.test.ts src/hooks/keyVault/defaultVariantSaveCoordinator.test.ts src/hooks/models/accountModelCatalog.test.ts: 8 files, 58 tests passed after the CI fixture correction.pnpm exec eslint --max-warnings 0 src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.test.ts src/modules/MainApp/Integrations/KeyVault/shared/ModelTable/ModelVariantInlineCard.tsx src/util/__tests__/modelVariants.test.tsandNODE_OPTIONS=--max-old-space-size=8192 pnpm typecheck:fast: passed after the fixture correction. The complete frontend suite was not rerun locally; the updated branch will run CI again.Fetched current
origin/develop;git merge-tree --write-tree HEAD origin/developfound no conflicts. No published history was rewritten.pnpm testwith the model hooks, KeyVault, palette, session queue, variant and context suites: 34 files, 180 tests passed in the isolated branch.NODE_OPTIONS='--max-old-space-size=8192' pnpm typecheck:fast: passed after rebasing onto the latest develop.ESLint with
--max-warnings 0on every changed TS/TSX file: passed. The six files flagged by CI were also checked with the typed ESLint configuration after explicit async completion handling.cargo test -p key_vault --lib --quiet: 494 passed, 1 ignored.cargo test -p agent_core --lib -- state::session_identity --test-threads=1: 2 passed.cargo test -p agent_core --lib -- entry::runtime_tests --test-threads=1: 2 passed.cargo test -p org2 --lib -- agent_sessions::session_directory::patch::tests --test-threads=1: 9 passed.cargo test -p org2 --lib -- api::mobile_bridge::adapters::model::tests --test-threads=1: 6 passed.After the async-lint correction, 19 focused frontend files / 87 tests and
pnpm typecheck:fastpassed.NODE_OPTIONS=--max-old-space-size=6144 pnpm check:typed-lintfound 0 new or increased findings.git diff --check origin/develop...HEAD: passed. Architecture, UI and performance reviews are included underdocs/; UI audit verdict: 0 fix, 6 keep with reason, 0 abstract.Full workspace Clippy, real account/provider requests, mobile visual QA, and dual-instance runtime checks were not run. No new screenshots were captured; the existing control layout and flow were preserved, while the underlying state transitions changed.