fix: chat.send no reply when thinking metadata is set#98855
Conversation
|
Codex review: found issues before merge. Reviewed July 3, 2026, 7:00 AM ET / 11:00 UTC. Summary Reproducibility: yes. at source level: current main rewrites a normal non-slash chat.send message with thinking metadata into Review metrics: 3 noteworthy metrics.
Stored data model Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land a narrow maintainer-approved patch that preserves chat.send message text while applying structured thinking metadata, and separately sponsor or split the unrelated Android, UI, plugin, docs, workflow, and release-note work. Do we have a high-confidence way to reproduce the issue? Yes at source level: current main rewrites a normal non-slash chat.send message with thinking metadata into Is this the best way to solve the issue? Partly: removing the synthetic command rewrite is the right layer for the central bug, but the branch also changes default reasoning semantics and many unrelated surfaces that need maintainer confirmation or splitting. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 3e2d373d4340. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat 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. |
|
/clawsweeper re-review |
There was a problem hiding this comment.
Pull request overview
Fixes a gateway chat.send edge case where per-message thinking metadata could cause a normal user message to be rewritten into /think … command text, leading to “no reply” behavior for clients that aren’t allowed to issue text slash-commands. The change keeps the user’s message body intact while still applying the one-turn thinking override through the structured reply options, and it hardens reasoning/thinking default interactions during auto-fallback primary probes.
Changes:
- Stop rewriting normal
chat.sendtext into/think <level> <message>; keep command/body fields as the original user message. - Refine model-selection / reply directive handling so explicit thinking-off (including config-driven) prevents model reasoning defaults from being re-enabled during directive resolution and auto-fallback primary probes.
- Add regression tests covering (a) thinking metadata not becoming command text, (b) one-turn thinking not persisting to session store, and (c) auto-fallback probe behavior with configured thinking-off aliases.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/gateway/server-methods/chat.ts | Removes the /think … body rewrite; keeps thinking override as structured replyOptions.thinkingLevelOverride. |
| src/gateway/server-methods/chat.directive-tags.test.ts | Adds coverage ensuring thinking metadata does not alter CommandBody/BodyForCommands for normal messages and is passed via reply options. |
| src/gateway/server.chat.gateway-server-chat.test.ts | Adds coverage ensuring one-turn thinking metadata does not persist thinkingLevel into the session store. |
| src/auto-reply/reply/model-selection.ts | Centralizes per-model thinking-default parsing and exposes hasConfiguredThinkingDefault for downstream reasoning/thinking decisions. |
| src/auto-reply/reply/get-reply.ts | Adjusts primary-probe thinking/reasoning resolution to respect explicit/configured thinking-off and avoid re-enabling reasoning defaults. |
| src/auto-reply/reply/get-reply-directives.ts | Tracks configured thinking defaults and model-config thinking defaults when deciding whether thinking was explicitly set (affects reasoning-default application). |
| src/auto-reply/reply/get-reply-run.ts | Treats one-turn thinking overrides as “explicit” when emitting unsupported-thinking errors for models without thinking support. |
| src/auto-reply/reply/get-reply-run.media-only.test.ts | Adds regression test for unsupported explicit one-turn thinking overrides. |
| src/auto-reply/reply/get-reply.test-fixtures.ts | Extends fixtures to carry resolved think/reasoning levels through directive results. |
| src/auto-reply/reply/get-reply.auto-fallback.test.ts | New test suite covering reasoning/thinking behavior during auto-fallback primary probes (including params.thinking disable aliases). |
| src/auto-reply/reply/get-reply-directives.target-session.test.ts | Adds cases ensuring thinking-off (override/per-agent/per-model) prevents model reasoning defaults from being re-enabled. |
|
/clawsweeper re-review |
2 similar comments
|
/clawsweeper re-review |
|
/clawsweeper re-review |
1048484 to
fd2a8d8
Compare
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
/clawsweeper re-review |
|
/clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
/clawsweeper re-review |
1 similar comment
|
/clawsweeper re-review |
|
/clawsweeper re-review |
|
Merged via squash.
|
* fix(gateway): preserve chat thinking metadata * test: harden chat thinking metadata coverage * test: harden chat thinking metadata coverage
What Problem This Solves
Fixes an issue where a normal
chat.sendmessage with per-messagethinkingmetadata could persist the user turn but produce no assistant response. The gateway was converting structuredthinkingmetadata into textual/think ...command input, so clients without text slash-command authority could have a normal message treated as command text before model dispatch.No related open GitHub issue or PR was found. I searched live GitHub and local gitcrawl for
chat.send thinking, Android/chat no-reply reports,/thinkcommand-body reports, and paired-device slash-command authorization reports. The only direct keyword hit was already-merged #79385, which is a broader thinking/fast controls PR rather than this no-reply bug report.Why This Change Was Made
chat.sendalready has a structured dispatch path for one-turn thinking metadata viareplyOptions.thinkingLevelOverride. This PR removes the duplicate/think <level> <message>rewrite for normal messages, leavingCommandBody,BodyForCommands, andCommandTurn.bodyas the user’s actual message while still passing the requested thinking override to reply resolution.The review pass also hardened the adjacent reply/model-selection behavior so explicit thinking-off inputs, per-agent thinking defaults, and per-model
params.thinkingdefaults prevent model reasoning defaults from being re-enabled during normal directive resolution and auto-fallback primary probes.User Impact
Mobile, WebChat, consult, and other
chat.sendclients can set a one-shot thinking level without accidentally turning a normal user message into a text slash command.thinkingremains turn-local metadata: it applies to the dispatched run, does not alter the visible/user command text, and does not persistthinkingLevelinto the session store.Evidence
Behavioral proof:
chat.sendwithmessage: "hello from phone"andthinking: "low"now dispatches withBodyForCommands === "hello from phone",CommandBody === "hello from phone",CommandTurn.kind === "normal", andreplyOptions.thinkingLevelOverride === "low".agent:main:mainkey and the legacymainview to prove one-turnthinkingmetadata does not become durable sessionthinkingLevel.params.thinkingvaluesfalse,"disabled", and"none"so configured thinking-off aliases keep reasoning defaults off when probing a primary model.Validation:
node scripts/run-vitest.mjs src/gateway/server-methods/chat.directive-tags.test.ts -t "chat.send keeps thinking metadata out of command text for normal messages"- passed, 2 tests run with unrelated tests skipped.node scripts/run-vitest.mjs src/gateway/server.chat.gateway-server-chat.test.ts -t "chat.send does not persist one-turn thinking metadata"- passed, 1 test run with unrelated tests skipped.node scripts/run-vitest.mjs src/auto-reply/reply/get-reply.auto-fallback.test.ts src/auto-reply/reply/get-reply-directives.target-session.test.ts src/auto-reply/reply/model-selection.test.ts src/auto-reply/reply/get-reply-run.media-only.test.ts- passed, 173 tests.node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.test.src.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/test-src.tsbuildinfo- passed.corepack pnpm exec oxfmt --check --threads=1 <10 touched TS files>- passed.git diff --check 7fa26e088d283f9dbde607d1148e62c682b7d471 && git diff --check- passed.chat.directive-tags.test.ts; the new thinking test and a representative media failure case both passed in isolation, so this is recorded as a validation caveat rather than an accepted regression from this PR.Review proof:
AI-assisted: yes. A sanitized transcript was not included because adding transcript logs requires explicit approval.