feat(android): hide thinking control and gate sends for models without reasoning support#101002
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: dbc0f96e72
ℹ️ 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 thinking = normalizeThinking(thinkingLevel) | ||
| // agent-command.ts throws for explicit unsupported levels, so hidden controls must send off. | ||
| val thinking = | ||
| if (thinkingSupportedForSelection(_selectedModelRef.value, modelCatalog())) { |
There was a problem hiding this comment.
When Android opens or switches to an already-patched session, beginHistoryLoad resets _selectedModelRef to null and it is never repopulated from chat.history/sessions.list: the gateway emits modelProvider and model on session rows (src/gateway/session-event-payload.ts:66-67), but Android parseSessionEntry ignores those fields (apps/android/app/src/main/java/ai/openclaw/app/chat/ChatController.kt:1914-1949). In that scenario this check fail-opens, so a session whose persisted model is non-reasoning still sends the stored high/medium thinking level and can hit the same gateway rejection until the user re-selects the model in this process.
Useful? React with 👍 / 👎.
|
Codex review: needs real behavior proof before merge. Reviewed July 6, 2026, 10:05 PM ET / 02:05 UTC. Summary PR surface: Source +3, Tests +36, Other +198. Total +237 across 12 files. Reproducibility: yes. source-reproducible. Current main sends stored thinking blindly, and the PR still has source-visible unknown-metadata collapse plus a health/reconnect metadata race; I did not run a live Android device reproduction. Review metrics: 1 noteworthy metric.
Stored data model 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
Security Review findings
Review detailsBest possible solution: Mirror the merged iOS contract on Android: preserve true/false/unknown reasoning metadata, gate only explicit unsupported models, make live/outbox sends metadata-ready, then add redacted Android runtime proof. Do we have a high-confidence way to reproduce the issue? Yes, source-reproducible. Current main sends stored thinking blindly, and the PR still has source-visible unknown-metadata collapse plus a health/reconnect metadata race; I did not run a live Android device reproduction. Is this the best way to solve the issue? No. The direction matches the merged iOS sibling, but this implementation is not the best fix until Android preserves unknown reasoning metadata and waits for current model metadata before applying live/outbox send gates. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against f3106057e62b. Label changesLabel justifications:
Evidence reviewedPR surface: Source +3, Tests +36, Other +198. Total +237 across 12 files. View PR surface stats
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 (5 earlier review cycles)
|
clawsweeper
Bot
added
rating: 🧂 unranked krab
dbc0f96 to
d02e5a4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d02e5a4a05
ℹ️ 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".
| catalog: List<GatewayModelSummary>, | ||
| ): Boolean { | ||
| val selected = selectedModelRef ?: return true | ||
| return catalog.firstOrNull { it.providerQualifiedRef() == selected }?.supportsReasoning != false |
There was a problem hiding this comment.
When the selected model's catalog row omits the optional reasoning field, parseGatewayModels currently materializes it as supportsReasoning=false, so this new helper treats unknown metadata as an explicit no-reasoning signal. That makes Android hide the thinking control and send off for any older Gateway or catalog/plugin row that does not yet populate reasoning, even when the model supports reasoning; the gate should distinguish absent/unknown metadata from an explicit reasoning: false.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
d02e5a4 to
0033948
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0033948a4b
ℹ️ 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 thinking = | ||
| if ( | ||
| queuedSessionKey == _sessionKey.value && | ||
| !thinkingSupportedForSelection(_selectedModelRef.value, _modelCatalog.value) | ||
| ) { |
There was a problem hiding this comment.
On reconnect after onDisconnected() clears _modelCatalog, markHealthOk() starts flushOutbox() before fetchChatMetadata() repopulates the catalog. For an active session already set to a non-reasoning model, this new gate sees the selected ref but an empty catalog, so thinkingSupportedForSelection fails open and the else path sends the queued high/medium value; the server's explicit thinking override path then produces the unsupported-thinking response instead of the intended turn. The active-session outbox path needs to hydrate/retain the catalog before flushing, or otherwise treat a known selected ref with missing metadata as not ready.
Useful? React with 👍 / 👎.
This comment has been minimized.
This comment has been minimized.
clawsweeper
Bot
added
the
merge-risk: 🚨 message-delivery
0033948 to
9b07da0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b07da06b5
ℹ️ 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".
| // Applied at enqueue time too so durable rows never persist a level the selected model | ||
| // rejects; reconnect flushes with a cleared catalog fail open, matching pre-gating behavior. | ||
| val thinking = | ||
| if (thinkingSupportedForSelection(_selectedModelRef.value, _modelCatalog.value)) { |
There was a problem hiding this comment.
When a health event or reconnect marks the gateway healthy, _healthOk becomes true before fetchChatMetadata() finishes, and the health-event path launches that metadata fetch asynchronously. If chat.history has already populated a selected non-reasoning model but _modelCatalog is still empty/cleared, this check treats the missing catalog as supported and sends the stored high/medium override, so the first send in that window can still hit the gateway's unsupported-thinking rejection instead of being downgraded to off; the known-selected/metadata-missing state needs to block or wait rather than fail open.
Useful? React with 👍 / 👎.
9b07da0 to
41a03be
Compare
…t reasoning support
41a03be to
b59f82e
Compare
…t reasoning support (openclaw#101002)
…plicit intents (#101799) * fix(android): keep cold-start gateway auto-connect from overriding explicit intents NodeRuntime's init-time auto-connect now atomically claims the first gateway lifecycle intent (CAS 0->1) and stands down permanently when any explicit connect/disconnect/switch intent already exists, so a late discovery emission can no longer override a user decision. This was also the root cause of the CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at :616/:627 and switchToUndiscoveredGateway* on PRs #101002/#101387/#101396). GatewaySession.Connection.sendRequestFrame now starts its response watcher with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right after failPending(), and a DEFAULT-start watcher still queued for dispatch was cancelled without running, silently dropping the terminal UNAVAILABLE onError owed to fire-and-forget callers (root cause of the flaky GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc). Tests neutralize runtime background work (cancel+join the runtime scope) before arming the registry and scripting lifecycle steps, and add regression coverage for the auto-connect intent claim. Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures) and :app:ktlintCheck green on the same box. * chore(i18n): resync native inventory line positions after NodeRuntime edits
…plicit intents (openclaw#101799) * fix(android): keep cold-start gateway auto-connect from overriding explicit intents NodeRuntime's init-time auto-connect now atomically claims the first gateway lifecycle intent (CAS 0->1) and stands down permanently when any explicit connect/disconnect/switch intent already exists, so a late discovery emission can no longer override a user decision. This was also the root cause of the CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at :616/:627 and switchToUndiscoveredGateway* on PRs openclaw#101002/openclaw#101387/openclaw#101396). GatewaySession.Connection.sendRequestFrame now starts its response watcher with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right after failPending(), and a DEFAULT-start watcher still queued for dispatch was cancelled without running, silently dropping the terminal UNAVAILABLE onError owed to fire-and-forget callers (root cause of the flaky GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc). Tests neutralize runtime background work (cancel+join the runtime scope) before arming the registry and scripting lifecycle steps, and add regression coverage for the auto-connect intent claim. Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures) and :app:ktlintCheck green on the same box. * chore(i18n): resync native inventory line positions after NodeRuntime edits
…t reasoning support (openclaw#101002)
…plicit intents (openclaw#101799) * fix(android): keep cold-start gateway auto-connect from overriding explicit intents NodeRuntime's init-time auto-connect now atomically claims the first gateway lifecycle intent (CAS 0->1) and stands down permanently when any explicit connect/disconnect/switch intent already exists, so a late discovery emission can no longer override a user decision. This was also the root cause of the CI-only flakes in GatewayBootstrapAuthTest (refreshGatewayConnection_* at :616/:627 and switchToUndiscoveredGateway* on PRs openclaw#101002/openclaw#101387/openclaw#101396). GatewaySession.Connection.sendRequestFrame now starts its response watcher with CoroutineStart.ATOMIC: disconnect teardown cancels connectionJob right after failPending(), and a DEFAULT-start watcher still queued for dispatch was cancelled without running, silently dropping the terminal UNAVAILABLE onError owed to fire-and-forget callers (root cause of the flaky GatewaySessionInvokeTest.disconnectReportsUnknownOutcomeForFireAndForgetRpc). Tests neutralize runtime background work (cancel+join the runtime scope) before arming the registry and scripting lifecycle steps, and add regression coverage for the auto-connect intent claim. Proof: refresh flake reproduced at iteration 1 under taskset -c 0,1 on a Blacksmith Linux box; post-fix 3 pinned stress rounds (120/150 looped iterations) plus full :app:testPlayDebugUnitTest (109 classes, 0 failures) and :app:ktlintCheck green on the same box. * chore(i18n): resync native inventory line positions after NodeRuntime edits
1 participant
Related: #100699
What Problem This Solves
Fixes an issue where Android chat users could set a thinking level for models that don't support configurable reasoning — the control silently did nothing, and worse, an explicit non-off level sent to such a model is rejected by the gateway (
agent-command.tsthrows for explicit unsupported levels), failing the send.Why This Change Was Made
Completes the reasoning-gating pack item on Android, mirroring the iOS behavior landed in #100875. A pure fail-open helper matches the selected model ref (from the in-chat picker landed in #100798) against the gateway model catalog's
supportsReasoningflag: unknown/default selections keep the control (fail open); a matched non-reasoning model hides the thinking control (context meter shows a context-only label, composer dropdown hides the entries). The send boundary applies the same rule: gated sends carrythinking: "off"without mutating the stored level, so switching back to a reasoning-capable model restores the user's preference. Queued outbox sends re-evaluate the gate at flush for the active session only; unknown queued sessions deliberately fail open and keep their enqueue-time level (commented at the site) — identical to the iOS contract.User Impact
The thinking control on Android now appears only when it does something for the selected model, and sends can no longer fail because a stale thinking level hit a non-reasoning model.
Evidence
offwhile the stored level is preserved and restored on selection flip, outbox active-session gating and unknown-session fail-open.corepack pnpm native:i18n:syncrun; snapshot consistent at head.