fix(thinking): keep explicit session thinkingLevel when runtime downgrades (#87740)#87923
fix(thinking): keep explicit session thinkingLevel when runtime downgrades (#87740)#87923hoobnn wants to merge 1 commit into
Conversation
…rades (openclaw#87740) When a session's stored thinkingLevel is unsupported by the active model, the runtime fell back to a supported level for the turn AND wrote that fallback back onto the persisted session override. Because the persistence condition fired exactly when the stored value was the explicit override, the user's explicit choice (e.g. "high") was permanently reset to the supported level (e.g. "off") after every turn — re-setting it just got clobbered again next turn. Downgrade only the level used for the current turn; never persist the support fallback onto the stored override. The explicit override is the user's intent and must survive turns (so it re-applies if a supporting model is used later). Both the reply path (get-reply-run) and the agent-command path carried the same duplicated write-back; both are fixed. Reported by @TitanBob2026.
|
Codex review: needs maintainer review before merge. Reviewed May 29, 2026, 12:56 PM ET / 16:56 UTC. Summary PR surface: Source -40, Tests +36. Total -4 across 3 files. Reproducibility: yes. Current main has a clear source-level reproduction path in both per-turn fallback blocks, and the PR body includes A/B live agent output showing the stored level is clobbered before the fix and preserved after it. Review metrics: 1 noteworthy metric.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused per-turn persistence fix after required checks, while keeping model-switch remap policy isolated in #87925. Do we have a high-confidence way to reproduce the issue? Yes. Current main has a clear source-level reproduction path in both per-turn fallback blocks, and the PR body includes A/B live agent output showing the stored level is clobbered before the fix and preserved after it. Is this the best way to solve the issue? Yes for the per-turn reset. The PR removes only the unintended persistence write-back and keeps the turn-local fallback, while the broader model-switch contract remains a separate product decision in #87925. AGENTS.md: found and applied where relevant. Codex review notes: reasoning high; reviewed against 67faef01828d. Label changesLabel justifications:
Evidence reviewedPR surface: Source -40, Tests +36. Total -4 across 3 files. View PR surface stats
Acceptance criteria:
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
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Fixes #87740. An explicit session
thinkingLeveloverride (e.g.high) was permanently reset to a supported level (e.g.off) after every agent turn whenever the active model didn't support the stored level.Root cause: when the stored level was unsupported, the runtime correctly fell back to a supported level for the turn, but also wrote that fallback back onto the persisted session override. The write-back condition fired precisely when the stored value was the user's explicit override, so the explicit choice was clobbered every turn — re-setting it just got overwritten again next turn. Leaving the level
inherited/null was unaffected, which matches the report.Fix: downgrade only the level used for the current turn (
resolvedThinkLevel); never persist the support fallback onto the stored override. The same duplicated write-back lived in both the reply path (src/auto-reply/reply/get-reply-run.ts) and the agent-command path (src/agents/agent-command.ts); both are fixed. Removed the now-deadloadSessionStoreRuntimeloader inget-reply-run.ts. Production code shrinks (-17 / -23 lines). An explicitoffset via directive/sessions.patchstill persists through the normal path; only the involuntary support downgrade no longer persists.Out of scope (sibling sites)
Two sites also persist a support-driven downgrade onto the stored level, but only on a user-initiated model switch (not the per-turn reset this issue reports), and one already notifies the client:
src/gateway/sessions-patch.ts:604— a PATCH that changes model (without an explicitthinkingLevel) downgrades an inherited unsupported level and persists it (no notification).src/auto-reply/reply/directive-handling.persist.ts:326— a/modelswitch remaps the stored level and persists it, returning athinkingRemapnotice.These are deliberately coded (explicit-vs-inherited branching); whether a model switch should preserve the original preference is a product/compat decision, so they're intentionally left out. Follow-up: #87925.
Verification
pnpm test src/agents/agent-command.live-model-switch.test.ts— 27 passed, incl. new regression testpreserves an explicit session thinkingLevel override when the level is unsupported(red before fix, green after).thinking/directive-handling.levels/commands-status.thinking-default/get-reply.config-override— 58 passed.directive-handling.model(50) +gateway/sessions-patch(72) green — confirms an explicitoffstill persists; only the involuntary downgrade stops.pnpm check:changed— clean (oxlint, import cycles, typecheck, guards), exit 0.Real behavior proof
Behavior addressed: An explicit session
thinkingLevel: "high"override is no longer reset to the support fallback after a turn when the active model rejects that level at runtime; the stored override survives the turn.Real environment tested: A live agent turn against a real OpenAI-compatible Xiaomi MiMo model (
xiaomi/mimo-v2.5-pro, real/v1/chat/completionsAPI, genuine replies) using a source build of this branch (pnpm build) in an isolated profile. The model is configuredreasoning: false, sohighis unsupported at runtime and the per-turn support downgrade fires — reproducing the #87740 condition with a real model. This exercises the embedded agent path (openclaw agent --local→src/agents/agent-command.ts). Theget-reply-run.ts(WebChat) site was not driven through a live inbound channel; it is the byte-identical removal and is covered by the A/B below plus the existing get-reply suites.A/B method: identical isolated profile, identical session pre-seeded with
thinkingLevel: "high", identical model. Only the two fixed source files (src/agents/agent-command.ts,src/auto-reply/reply/get-reply-run.ts) were reverted to the pre-fix parent (2e042fbca8) and rebuilt for the "before" run, then restored and rebuilt for the "after" run — so this PR's change is the only variable.Exact steps or command run after this patch:
xiaomi/mimo-v2.5-prowithreasoning: false(makeshighunsupported at runtime) in an isolated profile; pre-seed sessionagent:main:mainwiththinkingLevel: "high"inagents/main/sessions/sessions.json.XIAOMI_API_KEY=… pnpm openclaw --profile <p> agent --local --to +15555550123 -m "Reply with a single word: …" --jsonagents/main/sessions/sessions.json→["agent:main:main"].thinkingLevelbefore and after the turn.Evidence after fix (live turns, real model replies, same model
xiaomi/mimo-v2.5-pro):thinkingLevelafter turn2e042fbca8)pongoff(downgraded)off← bug reproducedokoff(downgraded)high← override preservedObserved result after fix: with stored
thinkingLevel: "high"on a model that rejects it at runtime, the turn runs normally (real reply,meta.requestShaping.thinking: "off") and the stored override stayshigh. Reverting only the two fixed files flips the stored value back tooff, confirming this change is what preserves the override.What was not tested: a live WebChat/Control UI or Voice UI round-trip against a running gateway — the
get-reply-run.tspath was not executed via an inbound channel. It carries the identical removed block and is covered by the A/B above and the existing get-reply suites.