Fix Control UI agent thinking defaults#81795
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed May 31, 2026, 9:34 AM ET / 13:34 UTC. Summary PR surface: Source +52, Tests +153. Total +205 across 12 files. Reproducibility: yes. Source inspection shows current main omits per-agent thinking metadata from 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:
Proof guidance:
Mantis proof suggestion Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land a current-head-proven version that keeps gateway runtime output, shared types, gateway-protocol schema, and Control UI fallback behavior aligned for per-agent thinking metadata. Do we have a high-confidence way to reproduce the issue? Yes. Source inspection shows current main omits per-agent thinking metadata from Is this the best way to solve the issue? Yes in direction. Exposing gateway-resolved per-agent thinking metadata and guarding cross-model fallbacks is the maintainable fix, provided exact-head proof and additive protocol compatibility are accepted before merge. AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 9518d1f27ccd. Label changesLabel changes:
Label justifications:
Evidence reviewedPR surface: Source +52, Tests +153. Total +205 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
|
This comment was marked as low quality.
This comment was marked as low quality.
e480666 to
634b90d
Compare
|
ClawSweeper PR egg 🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat. Where did the egg go?
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
Proof freshness note for maintainers The live gateway + Control UI transcript in the PR body was captured on the pre-rebase head Current-head focused regression suite is green on If a fresh current-head live gateway/browser capture is required, that path is Mantis (maintainer-invoked) — happy for a maintainer to run it. The one failing check, |
634b90d to
788d2e0
Compare
788d2e0 to
9e462a0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e462a0bce
ℹ️ 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".
| const meta = configuredById.get(id); | ||
| const model = resolveGatewayAgentModel(cfg, id); | ||
| const resolvedModel = resolveDefaultModelForAgent({ cfg, agentId: id }); | ||
| const thinkingLevels = listThinkingLevelOptions(resolvedModel.provider, resolvedModel.model); |
There was a problem hiding this comment.
Use model catalog when computing agent thinking levels
For agents whose model capabilities come from models.providers or the runtime model catalog, this new agents.list metadata can report the wrong options because it calls listThinkingLevelOptions without any catalog. The existing sessions.list path loads the model catalog in src/gateway/server-methods/sessions.ts:1138 and passes it through to the same thinking resolver, which is what makes configured reasoning: false entries collapse to off-only and configured compat add xhigh. As written, a custom/non-reasoning configured model can show broad default thinking levels in agents.list even though sessions for the same model expose the narrower catalog-backed set.
Useful? React with 👍 / 👎.
|
Behavior addressed: Control UI agent summaries now expose each agent's supported thinking levels and default thinking state from the existing model and agent configuration. The gateway protocol and generated Swift model were updated so the UI can read that metadata without adding a new config surface. Real environment tested: local targeted Vitest plus protocol generation check in this checkout; GitHub Actions on PR head 84f1566. Exact steps or command run after this patch:
Evidence after fix:
Observed result after fix: agents.list includes thinking metadata based on the current model catalog, so custom reasoning-capable models are represented correctly instead of falling back to stale or empty thinking options. What was not tested: full local suite; GitHub Actions covered the broad PR gate. |
Fixes #81760
Summary
Real behavior proof
Behavior or issue addressed: Control UI should show and inherit the effective thinking default for the selected agent/session, not the global default from a different model.
Real environment tested: Local OpenClaw gateway from PR head
e4806661795f81e12849864fe227b9f4b5ef92f1on macOS, with Control UI rebuilt bypnpm ui:build, a temporaryOPENCLAW_HOME, token auth on127.0.0.1:18895, and a proof config containing:{ "agents": { "defaults": { "model": { "primary": "minimax/MiniMax-M2.7" }, "thinkingDefault": "off" }, "list": [ { "id": "main", "default": true }, { "id": "investment-master", "model": { "primary": "deepseek/deepseek-v4-flash" }, "thinkingDefault": "xhigh" } ] } }For the local headless browser run only,
gateway.controlUi.dangerouslyDisableDeviceAuth=truewas set so Chrome could connect over local HTTP without a one-time pairing ceremony. The gateway still used token auth; the behavior under test was the liveagents.listpayload and Control UI rendering.Exact steps or command run after this patch:
Evidence after fix:
Live gateway output:
Live Control UI browser output:
Observed result after fix: The running gateway returns the
investment-masterrow withthinkingDefault: "xhigh"and model-specific thinking levels instead of the globaloffdefault. The running Control UI/agentspage, connected to that gateway, renders the selectedinvestment-masteroverview withThinking Defaultshown asxhigh.What was not tested: I did not run a live LLM chat turn or
/thinkcommand against a provider because the proof config intentionally has no provider API keys. The focused tests below cover the chat selector and/thinkfallback paths.Re-verified on current head
634b90d5a2e422e778592949e07d9339fba1095b(2026-05-25): the branch was rebased onto currentorigin/main, reconcilingresolveThinkingLevelOptionswith main's added model-catalog argument while keeping thedefaultFromSessionDefaultsfix. The live gateway and Control UI transcripts above were captured on the pre-rebase heade4806661795f81e12849864fe227b9f4b5ef92f1; the rebase only reconciled the merge and did not change the user-facing behavior under test. The focused regression suite re-runs green on the current head:Verification