feat(ios): model picker favorites and recents with working iOS model switching#100774
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6ff06ea335
ℹ️ 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".
| sessionKey: sessionKey, | ||
| agentId: self.selectedGlobalAgentId(for: sessionKey), | ||
| model: model) | ||
| _ = try await self.gateway.request(method: "sessions.patch", paramsJSON: json, timeoutSeconds: 15) |
There was a problem hiding this comment.
Request admin scope before patching models
On the default paired iOS operator connection, this new path can list models but cannot switch them: NodeAppModel.makeOperatorConnectOptions defaults to operator.read, operator.write, and operator.talk.secrets and only appends operator.admin for explicit admin upgrades, while src/gateway/methods/core-descriptors.ts:164 declares sessions.patch as operator.admin. That means normal iOS users hit this sessions.patch call, get rejected by the gateway, and the selection rolls back with an error unless they happen to be connected with password/token/admin credentials.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 6:00 AM ET / 10:00 UTC. Summary PR surface: Other +439. Total +439 across 9 files. Reproducibility: yes. from source inspection: the PR calls Review metrics: 2 noteworthy metrics.
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: Land the UX only after maintainers choose a safe authorization path for iOS model switching, implement it with focused coverage for default setup-code paired users, and add real simulator/device proof of a successful model switch. Do we have a high-confidence way to reproduce the issue? Yes, from source inspection: the PR calls Is this the best way to solve the issue? No: reusing the macOS Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 24bca38cdaad. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Other +439. Total +439 across 9 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
|
|
Merged via squash.
|
Related: #100699
What Problem This Solves
Fixes an issue where iOS users could not see or switch models in chat at all: the iOS gateway transport never implemented
models.list/sessions.patch(model), so the shared composer's model picker stayed hidden on iOS (it only worked in the macOS webchat). On top of that, the picker treated all models equally — frequently used models required scanning the full flat catalog list on every switch.Why This Change Was Made
Implements the missing
listModels/setSessionModeloverrides on the iOS gateway transport (samesessions.patchwire format the macOS transport uses), and adds favorites + recents to the shared model picker: pin/unpin the selected model from the composer, with the picker ordered Pinned → Recent → Models. Pins and recents persist per device inUserDefaultsvia a small injectable store; recents (capped at 5) are recorded only when a selection round-trips successfully, so failed or stale patches never pollute the list. Fresh installs with no pins/recents keep today's flat list with no section chrome.User Impact
iOS users can now switch models from the chat composer, pin the models they use most, and reach recently used models first instead of scrolling the full catalog. macOS webchat gets the same pinning/recents behavior.
Evidence
swift test --package-path apps/shared/OpenClawKit— full shared-kit suite green (431 Swift Testing tests / 39 suites, 21 XCTest).ChatModelPickerStoreTests(persistence round-trip, recents dedupe/cap/sentinel-skip, pin-order sectioning) andChatViewModelTestsadditions (recents recorded only on confirmed selection success, stale-completion protection, pin toggling), plus iOS transport params/decoding tests.swiftformat --lintvia the iOS filelist — clean;node scripts/ios-write-swift-filelist.mjsregenerated with the new shared source registered.corepack pnpm native:i18n:syncrun for the new picker strings.