feat(android): in-chat model picker with favorites and recents#100798
Conversation
openclaw-barnacle
Bot
added
app: android
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a33875a5a
ℹ️ 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".
| if (trimmed.isEmpty() && attachments.isEmpty()) return false | ||
| // Model patches and sends share one ordering boundary; the first post-selection turn | ||
| // must not leave on the previous model while sessions.patch is still in flight. | ||
| modelSelectionMutex.withLock {} |
There was a problem hiding this comment.
When the pending sessions.patch fails (for example on previously paired Android tokens that lack operator.admin), this empty lock only waits for it; sendMessageAwaitAcceptance then continues to chat.send and clears the patch error at updateErrorText(null), so an immediate post-selection message is sent on the previous session model while the UI just accepted a new choice. The queued selection should surface failure back to the send path or block/retry the send instead of silently falling through.
Useful? React with 👍 / 👎.
| ): Long { | ||
| val generation = historyLoadGeneration.incrementAndGet() | ||
| _sessionKey.value = key | ||
| _selectedModelRef.value = null |
There was a problem hiding this comment.
When opening or switching to a session that already has a persisted model override, this clears the chip state and the later chat.history/sessions.list parsing never restores it from the gateway's sessionInfo.modelProvider/model fields. The composer can therefore show the generic Model/default state even though the gateway will keep sending on the stored override, causing users to make choices from an incorrect current model.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 10:33 AM ET / 14:33 UTC. Summary PR surface: Other +629. Total +629 across 13 files. Reproducibility: yes. for PR-introduced issues from source: failed model patches are not propagated to the immediate send path, loaded sessions cannot hydrate selected model state, and the picker uses a default-agent catalog for active-agent chats. I did not run a real Android device or emulator reproduction. Review metrics: 2 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Fix the remaining source blockers, use an active-session/agent-aware model catalog path, add real Android proof, and merge only after maintainers explicitly accept Android admin-scope parity or choose a narrower gateway permission. Do we have a high-confidence way to reproduce the issue? Yes for PR-introduced issues from source: failed model patches are not propagated to the immediate send path, loaded sessions cannot hydrate selected model state, and the picker uses a default-agent catalog for active-agent chats. I did not run a real Android device or emulator reproduction. Is this the best way to solve the issue? No: the Android UI direction is plausible and matches the parent issue, but the patch is not the best mergeable solution until it fixes failed-selection send gating, model hydration, active-agent catalog scoping, proof, and the admin-scope decision. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 94a0d2ec3b79. Label changesLabel justifications:
Evidence reviewedPR surface: Other +629. Total +629 across 13 files. View PR surface stats
Security concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (3 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
0a33875 to
47040a7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47040a7a61
ℹ️ 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".
| val modelSections = | ||
| remember(modelCatalog, modelFavorites, modelRecents) { | ||
| chatModelPickerSections( | ||
| catalog = modelCatalog, |
There was a problem hiding this comment.
When the current chat session belongs to a non-default agent, this sheet is still built from the single modelCatalog that NodeRuntime.refreshModelCatalogFromGateway() loads with models.list and {} (apps/android/app/src/main/java/ai/openclaw/app/NodeRuntime.kt:2837), while the gateway's default list view scopes visibility to resolveDefaultAgentId when no agent is supplied (src/gateway/server-methods/models-list-result.ts:249). sessions.patch later validates against the agent resolved from the session key, so agent-specific models can be absent from the picker, and default-agent entries can be offered only to fail after selection. The picker needs an active-agent/all-model catalog source instead of the default-agent catalog.
Useful? React with 👍 / 👎.
47040a7 to
8cf70f3
Compare
8cf70f3 to
5a9e540
Compare
|
Merged via squash.
|
|
Post-merge review found three synchronization defects in the landed picker: failed model patches could be followed by a send on the prior model, session history did not hydrate/precedence-protect selected model state, and the picker used the default agent's catalog for non-default sessions. Follow-up #100985 fixes those paths, adds bounded empty-catalog retry and regressions, and landed as 255b763. Exact-head CI proof: https://github.com/openclaw/openclaw/actions/runs/28804597098. |
1 participant
Related: #100699
What Problem This Solves
Fixes an issue where Android users could not switch models in chat at all: the app had no model picker anywhere — the model catalog was only visible as a read-only provider readiness overview in Settings. iOS/macOS users can switch the session model from the composer; Android had no equivalent.
Why This Change Was Made
Adds a model chip to the Android chat composer that opens a bottom-sheet picker fed by the existing gateway model catalog, with Pinned / Recent / Models sections: star to pin/unpin without closing the sheet, tap to select. Selection patches the session via
sessions.patch { key, model }(the same wire contract iOS/macOS use); pins and recents (capped at 5) persist in the app's plain preferences, and a recent is recorded only after gateway acceptance. A selection mutex orders model patches ahead of sends so the first post-selection message can never leave on the previous model. Becausesessions.patchrequiresoperator.admin, fresh Android connects now request that scope — exact parity with iOS, which already requests admin for token/password connects; previously paired device tokens keep their granted scopes (existing stored-scope narrowing) and simply can't patch models until re-paired, mirroring howoperator.talk.secretsrolled out.Security note (deliberate decision, please weigh in):
sessions.patchis classifiedoperator.adminin the gateway method descriptors, so this PR addsoperator.adminto Android's fresh-connect scope request. This is exact parity with iOS, which already requests admin for every token/password connect (shouldRequestOperatorAdminScopeinapps/ios/Sources/Model/NodeAppModel.swift), and follows the same rollout pattern asoperator.talk.secrets: previously paired device tokens keep their narrower granted scopes. The alternative — a narrower gateway permission for model-only session patches — is a protocol/security contract change beyond this client PR's boundary; if maintainers prefer that, it should be its own gateway-side follow-up, and this scope request can be revisited then.User Impact
Android users can now switch the session model from the chat composer, pin favorites, and reach recently used models first. On gateways where the operator connection lacks admin scope, selection fails with a visible chat error and no state is polluted.
Evidence
./gradlew :app:testPlayDebugUnitTestfocused suites green:ChatModelPickerTest(section ordering, provider-qualified ref derivation),SecurePrefsTest(favorites/recents persistence round-trip, dedupe/cap),ChatControllerModelSelectionTest(recent recorded only on RPC success, rejection surfaces error and records nothing, send-after-select ordering),ConnectionManagerTest/GatewayBootstrapAuthTest(fresh-connect scope set incl.operator.admin, stored-token narrowing unchanged).:app:ktlintFormatclean on changed files;:app:ktlintCheckfailures are pre-existing in five untouched test files (called out for transparency).corepack pnpm native:i18n:syncrun for the new picker strings.