Skip to content

feat(ios): model picker favorites and recents with working iOS model switching#100774

Merged
steipete merged 1 commit into
mainfrom
feat/ios-model-picker-pins-nash
Jul 6, 2026
Merged

feat(ios): model picker favorites and recents with working iOS model switching#100774
steipete merged 1 commit into
mainfrom
feat/ios-model-picker-pins-nash

Conversation

@steipete

@steipete steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

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/setSessionModel overrides on the iOS gateway transport (same sessions.patch wire 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 in UserDefaults via 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).
  • New coverage: ChatModelPickerStoreTests (persistence round-trip, recents dedupe/cap/sentinel-skip, pin-order sectioning) and ChatViewModelTests additions (recents recorded only on confirmed selection success, stale-completion protection, pin toggling), plus iOS transport params/decoding tests.
  • swiftformat --lint via the iOS filelist — clean; node scripts/ios-write-swift-filelist.mjs regenerated with the new shared source registered.
  • corepack pnpm native:i18n:sync run for the new picker strings.

@openclaw-barnacle openclaw-barnacle Bot added app: ios App: ios scripts Repository scripts size: L maintainer Maintainer-authored PR labels Jul 6, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

@clawsweeper

clawsweeper Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 6:00 AM ET / 10:00 UTC.

Summary
The PR adds iOS models.list/sessions.patch(model) transport support plus shared Apple model-picker favorites and recents persisted in UserDefaults, with Swift tests and i18n/filelist updates.

PR surface: Other +439. Total +439 across 9 files.

Reproducibility: yes. from source inspection: the PR calls sessions.patch, current main requires operator.admin for that method, and the default iOS setup-code operator scopes omit admin. I did not run a live iOS app repro.

Review metrics: 2 noteworthy metrics.

  • Admin-scoped Gateway calls: 1 added. The new iOS model-selection path adds a sessions.patch call that requires operator.admin, which default setup-code paired iOS operators do not have.
  • Persistent picker preferences: 2 UserDefaults keys added. Pins and recents become persisted app state, so upgrade behavior and stale model cleanup need to remain intentional.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Resolve the Gateway authorization path for the default setup-code paired iOS operator case.
  • [P1] Add simulator or device proof showing the model picker lists models and successfully switches the session model with private details redacted.

Proof guidance:

  • [P1] Needs real behavior proof before merge: Only test/check claims are present; add a simulator/device recording, terminal transcript, or redacted logs showing successful iOS model switching, then update the PR body for a fresh review.

Risk before merge

  • [P1] Merging as-is would show iOS users a model picker that can list models but fails to switch models on the normal setup-code paired connection because sessions.patch requires operator.admin.
  • [P1] The authorization repair is security-boundary sensitive: silently adding default admin to mobile pairing would contradict the bounded handoff contract, while a narrower model-switch method or explicit admin upgrade needs maintainer approval.
  • [P1] The PR body reports Swift tests and generated-file checks, but no simulator/device proof, screenshot, recording, terminal transcript, or redacted logs show the real iOS model-switch path working.

Maintainer options:

  1. Fix the iOS authorization path before merge (recommended)
    Choose a narrow model-switch API or an explicit admin-upgrade gate, then prove the default setup-code paired iOS connection can switch models successfully.
  2. Ship only for admin-capable sessions
    Maintain the existing admin boundary by hiding or disabling model switching for non-admin iOS sessions and documenting the user-visible limitation.
  3. Pause for gateway API design
    Hold this PR until the linked mobile polish issue has a maintainer-approved authorization shape for model-selection UX.

Next step before merge

  • [P1] A maintainer needs to choose the iOS model-switch authorization boundary before a safe automated repair or merge path exists.

Maintainer decision needed

  • Question: How should iOS model switching be authorized for setup-code paired devices that currently do not receive operator.admin?
  • Rationale: The current PR uses an admin-scoped session patch method, but the mobile bootstrap contract intentionally withholds admin from default paired devices; automation should not choose between broadening that boundary and adding a narrower API.
  • Likely owner: steipete — Peter owns the linked mobile polish tracker and authored recent bounded-bootstrap scope follow-up, making him the best available owner for this security/UX tradeoff.
  • Options:
    • Add a narrow model-switch path (recommended): Introduce or reuse a model-only Gateway authorization path that works for normal write-scoped paired iOS operators without granting full admin.
    • Require explicit admin upgrade: Keep sessions.patch as the transport but gate iOS model switching behind a visible admin-capable pairing or token flow.
    • Keep iOS model switching admin-only for now: Hide or disable the picker for non-admin paired devices until the broader mobile auth policy is decided.

Security
Needs attention: The diff does not add third-party code or secret-handling changes, but it introduces an unresolved Gateway authorization-boundary issue for iOS model switching.

Review findings

  • [P1] Authorize iOS model patches before calling sessions.patch — apps/ios/Sources/Chat/IOSGatewayChatTransport.swift:291
Review details

Best 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 sessions.patch, current main requires operator.admin for that method, and the default iOS setup-code operator scopes omit admin. I did not run a live iOS app repro.

Is this the best way to solve the issue?

No: reusing the macOS sessions.patch wire format is plausible for admin-capable clients, but it is not the best merge-ready solution for default paired iOS users until the authorization boundary is resolved.

Full review comments:

  • [P1] Authorize iOS model patches before calling sessions.patch — apps/ios/Sources/Chat/IOSGatewayChatTransport.swift:291
    On setup-code paired iOS connections this call can list models but cannot switch them: the default operator options request read/write/talk secrets without operator.admin, while sessions.patch is admin-scoped. The selection then rolls back with an error for normal paired users, so either request/verify an admin-capable flow before exposing this path or use a narrower model-switch authorization surface.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 24bca38cdaad.

Label changes

Label changes:

  • add P2: This is a normal-priority iOS chat feature PR with a concrete merge blocker, not an emergency or currently shipped crash/data-loss regression.
  • add merge-risk: 🚨 compatibility: Fixing the blocker by changing default mobile pairing scopes or session model APIs would affect existing paired-device scope and upgrade expectations.
  • add merge-risk: 🚨 auth-provider: The new iOS model-switch path depends on an admin-scoped Gateway method while normal paired iOS operator credentials are intentionally non-admin.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only test/check claims are present; add a simulator/device recording, terminal transcript, or redacted logs showing successful iOS model switching, then update the PR body for a fresh review.

Label justifications:

  • P2: This is a normal-priority iOS chat feature PR with a concrete merge blocker, not an emergency or currently shipped crash/data-loss regression.
  • merge-risk: 🚨 auth-provider: The new iOS model-switch path depends on an admin-scoped Gateway method while normal paired iOS operator credentials are intentionally non-admin.
  • merge-risk: 🚨 compatibility: Fixing the blocker by changing default mobile pairing scopes or session model APIs would affect existing paired-device scope and upgrade expectations.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Only test/check claims are present; add a simulator/device recording, terminal transcript, or redacted logs showing successful iOS model switching, then update the PR body for a fresh review.
Evidence reviewed

PR surface:

Other +439. Total +439 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 0 0 0 0
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 9 482 43 +439
Total 9 482 43 +439

Security concerns:

  • [medium] Admin-scoped model switch is unresolved — apps/ios/Sources/Chat/IOSGatewayChatTransport.swift:291
    The PR calls sessions.patch from the iOS paired-device path without ensuring operator.admin, and broadening that default would weaken the documented mobile bootstrap boundary.
    Confidence: 0.9

What I checked:

Likely related people:

  • steipete: Peter authored the linked mobile polish tracker and the 7d22a16a bounded-bootstrap follow-up that documented and enforced the non-admin handoff boundary. (role: recent area contributor and likely product owner; confidence: high; commits: 7d22a16adb21; files: src/shared/device-bootstrap-profile.ts, docs/gateway/protocol.md, apps/shared/OpenClawKit/Tests/OpenClawKitTests/GatewayNodeSessionTests.swift)
  • ngutman: Nimrod authored the QR bootstrap handoff change that added the iOS/operator device-token path and bounded paired-device scopes. (role: introduced behavior; confidence: medium; commits: a9140abea6d4; files: apps/ios/Sources/Model/NodeAppModel.swift, apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift, src/shared/device-bootstrap-profile.ts)
  • ImLukeF: Luke introduced the macOS/shared chat model selector and sessions.patch model-selection flow that this PR ports to iOS. (role: adjacent owner; confidence: medium; commits: 061b8258bc35; files: apps/macos/Sources/OpenClaw/WebChatSwiftUI.swift, apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatViewModel.swift, apps/shared/OpenClawKit/Sources/OpenClawChatUI/ChatComposer.swift)
  • masatohoshino: Current blame on the relevant iOS operator-scope and Gateway descriptor lines points to 9b0c3abbca90, though the broad commit title makes this a weaker ownership signal. (role: recent area contributor; confidence: low; commits: 9b0c3abbca90; files: apps/ios/Sources/Model/NodeAppModel.swift, src/gateway/methods/core-descriptors.ts, apps/shared/OpenClawKit/Sources/OpenClawKit/GatewayChannel.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. labels Jul 6, 2026
@steipete
steipete merged commit 1f06b89 into main Jul 6, 2026
153 of 167 checks passed
@steipete
steipete deleted the feat/ios-model-picker-pins-nash branch July 6, 2026 10:44
@steipete

steipete commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

Merged via squash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: ios App: ios maintainer Maintainer-authored PR merge-risk: 🚨 auth-provider 🚨 May break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. scripts Repository scripts size: L status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant