Skip to content

fix: chat.send no reply when thinking metadata is set#98855

Merged
jesse-merhi merged 3 commits into
mainfrom
fix/chat-send-thinking-metadata
Jul 3, 2026
Merged

fix: chat.send no reply when thinking metadata is set#98855
jesse-merhi merged 3 commits into
mainfrom
fix/chat-send-thinking-metadata

Conversation

@jesse-merhi

@jesse-merhi jesse-merhi commented Jul 2, 2026

Copy link
Copy Markdown
Member

What Problem This Solves

Fixes an issue where a normal chat.send message with per-message thinking metadata could persist the user turn but produce no assistant response. The gateway was converting structured thinking metadata 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, /think command-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.send already has a structured dispatch path for one-turn thinking metadata via replyOptions.thinkingLevelOverride. This PR removes the duplicate /think <level> <message> rewrite for normal messages, leaving CommandBody, BodyForCommands, and CommandTurn.body as 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.thinking defaults 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.send clients can set a one-shot thinking level without accidentally turning a normal user message into a text slash command. thinking remains turn-local metadata: it applies to the dispatched run, does not alter the visible/user command text, and does not persist thinkingLevel into the session store.

Evidence

Behavioral proof:

  • chat.send with message: "hello from phone" and thinking: "low" now dispatches with BodyForCommands === "hello from phone", CommandBody === "hello from phone", CommandTurn.kind === "normal", and replyOptions.thinkingLevelOverride === "low".
  • The persisted session store is checked at the canonical agent:main:main key and the legacy main view to prove one-turn thinking metadata does not become durable session thinkingLevel.
  • Auto-fallback tests cover params.thinking values false, "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.
  • A broad affected-file Vitest run hit older media/inline-reply failures in 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:

  • Native Codex review: 3 consecutive clean whole-target passes after review fixes.
  • Independent cold review: 3 consecutive clean reviewer passes on the same final tree.

AI-assisted: yes. A sanitized transcript was not included because adding transcript logs requires explicit approval.

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui gateway Gateway runtime size: XS maintainer Maintainer-authored PR labels Jul 2, 2026
@clawsweeper

clawsweeper Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed July 3, 2026, 7:00 AM ET / 11:00 UTC.

Summary
The PR removes synthetic /think command rewriting from normal chat.send messages with thinking metadata, changes thinking/reasoning default resolution, and includes adjacent Android, Control UI, plugin, docs, workflow, generated-locale, and changelog changes.

Reproducibility: yes. at source level: current main rewrites a normal non-slash chat.send message with thinking metadata into /think ... command text before dispatch. I did not run a live gateway repro in this read-only review.

Review metrics: 3 noteworthy metrics.

  • Compatibility-affecting default paths: 3 default sources changed. The PR changes explicit thinking-off, per-agent thinking defaults, and per-model params.thinking behavior, which can alter existing model/provider runs on upgrade.
  • Release-owned files changed: 1 CHANGELOG.md edit. OpenClaw release notes are generated and release-owned, so this edit needs removal or explicit release-process ownership.
  • Automation config changed: 1 workflow changed. The PR changes CI build memory settings, which is outside the central runtime bug and should be reviewed as automation behavior.

Stored data model
Persistent data-model change detected: serialized state: ui/src/ui/chat/session-controls.ts, serialized state: ui/src/ui/controllers/sessions.ts, unknown-data-model-change: ui/src/ui/chat/session-controls.ts, unknown-data-model-change: ui/src/ui/controllers/sessions.ts, unknown-truncated-pull-files. Confirm migration or upgrade compatibility proof before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: ready for maintainer review.

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

Rank-up moves:

  • [P2] Confirm or narrow the thinking-off/default reasoning semantics before merge.
  • Remove the release-owned changelog edit or route it through release generation.
  • Split or explicitly sponsor the unrelated Android, UI, plugin, docs, and workflow changes.

Risk before merge

  • [P2] The branch changes compatibility-sensitive thinking/default semantics: explicit thinking-off, per-agent thinking defaults, and per-model params.thinking defaults now suppress model reasoning defaults in more paths, including auto-fallback probes.
  • [P1] The branch is much broader than the stated chat.send bugfix, with Android voice setup, Control UI session/sidebar changes, Ollama and QQBot plugin work, workflow memory settings, docs, generated locale files, and CHANGELOG edits in the same PR.
  • [P1] The workflow memory change alters repository automation behavior and should be intentionally owned rather than landing as incidental bugfix churn.

Maintainer options:

  1. Narrow And Confirm Before Merge (recommended)
    Keep the chat.send rewrite removal and tests, but have a maintainer explicitly confirm the thinking-off/default semantics and remove unrelated surfaces before landing.
  2. Accept Broad Maintainer Batch
    A maintainer can intentionally accept the multi-surface branch as a coordinated batch after documenting that the Android, UI, plugin, docs, workflow, and changelog changes are sponsored here.
  3. Pause For Split PRs
    If the extra surfaces are accidental carry-over, pause this PR and split the chat.send fix from the UI, Android, plugin, workflow, and release-note changes.

Next step before merge

  • [P2] The protected maintainer label plus compatibility-sensitive thinking/default changes and broad branch scope need maintainer judgment rather than an automated repair job.

Security
Cleared: No concrete security or supply-chain regression was identified; workflow changes remain merge-risk/automation scope rather than a specific security finding.

Review findings

  • [P3] Remove the release-owned changelog edit — CHANGELOG.md:39
Review details

Best 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 /think ... command text before dispatch. I did not run a live gateway repro in this read-only review.

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:

  • [P3] Remove the release-owned changelog edit — CHANGELOG.md:39
    CHANGELOG.md is generated/release-owned in this repo, and this normal chat.send bugfix PR adds an unrelated QQBot release note plus count change. Please remove this edit or route it through release generation so the release record does not drift from the actual merged range.
    Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.72

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 3e2d373d4340.

Label changes

Label changes:

  • add merge-risk: 🚨 automation: The diff changes a CI workflow memory setting, so merging changes repository automation behavior beyond the runtime fix.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P1: The central bug can suppress assistant replies for normal chat.send clients that set thinking metadata.
  • merge-risk: 🚨 compatibility: The diff changes thinking/reasoning default precedence and auto-fallback behavior that can alter existing provider, model, or session behavior on upgrade.
  • merge-risk: 🚨 automation: The diff changes a CI workflow memory setting, so merging changes repository automation behavior beyond the runtime fix.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The PR is maintainer-authored/labeled, so the external-contributor proof gate is not applicable; PR-body targeted command outputs and green CI are supporting evidence.
Evidence reviewed

What I checked:

  • Root policy read: Read the full root AGENTS.md; it requires full-policy review, treats provider/auth/session/default/fallback behavior as compatibility-sensitive, and says CHANGELOG.md is release-only for normal PRs. (AGENTS.md:29, 3e2d373d4340)
  • Scoped policy read: Read gateway, server-methods, UI, Android, agents, and test scoped AGENTS.md files; the UI guide treats non-English locale bundles as generated output. (ui/AGENTS.md:7, 3e2d373d4340)
  • Live PR surface: Live GitHub metadata reports 203 changed files, 12051 additions, 2568 deletions, head e629788, clean merge state, green relevant checks, and the protected maintainer label. (e62978808dee)
  • Current main source: Current main rewrites a non-slash chat.send message with p.thinking into /think <level> <message> for CommandBody and BodyForCommands while separately passing replyOptions.thinkingLevelOverride. (src/gateway/server-methods/chat.ts:3905, 3e2d373d4340)
  • PR diff central fix: The PR removes injectThinking and leaves commandBody as the original parsedMessage in the chat.send handler. (src/gateway/server-methods/chat.ts:3903, e62978808dee)
  • Release-owned changelog edit: The PR edits CHANGELOG.md with an unrelated QQBot release note and contribution-count change despite the repo policy that release generation owns CHANGELOG.md. (CHANGELOG.md:39, e62978808dee)

Likely related people:

  • VACInc: Authored merged related thinking/fast-control work that touched chat.send and resolve-reply directive semantics. (role: adjacent feature owner; confidence: medium; commits: 242a71663add; files: src/gateway/server-methods/chat.ts, src/auto-reply/reply/get-reply-directives.ts)
  • steipete: Committed/merged the related thinking-control PR merge commit according to live GitHub commit metadata. (role: merger of related feature history; confidence: medium; commits: 242a71663add; files: src/gateway/server-methods/chat.ts, src/auto-reply/reply/get-reply-directives.ts)
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: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P1 High-priority user-facing bug, regression, or broken workflow. labels Jul 2, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 3, 2026
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@jesse-merhi
jesse-merhi marked this pull request as ready for review July 3, 2026 04:54
Copilot AI review requested due to automatic review settings July 3, 2026 04:54

Copilot AI 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.

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.send text 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.

@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

2 similar comments
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@jesse-merhi jesse-merhi self-assigned this Jul 3, 2026
@clawsweeper clawsweeper Bot added status: 🔁 re-review loop A fresh ClawSweeper review was explicitly requested after the latest review. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jul 3, 2026
@jesse-merhi
jesse-merhi force-pushed the fix/chat-send-thinking-metadata branch from 1048484 to fd2a8d8 Compare July 3, 2026 06:13
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added the merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. label Jul 3, 2026
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@jesse-merhi
jesse-merhi requested a review from a team as a code owner July 3, 2026 10:23
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation app: android App: android app: ios App: ios commands Command implementations agents Agent runtime and tooling channel: feishu Channel integration: feishu channel: qqbot plugin: file-transfer extensions: ollama size: XL and removed size: L labels Jul 3, 2026
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

1 similar comment
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. labels Jul 3, 2026
@jesse-merhi

Copy link
Copy Markdown
Member Author

/clawsweeper re-review

@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 3, 2026
@jesse-merhi
jesse-merhi merged commit 776b34a into main Jul 3, 2026
193 of 207 checks passed
@jesse-merhi
jesse-merhi deleted the fix/chat-send-thinking-metadata branch July 3, 2026 11:03
@jesse-merhi

Copy link
Copy Markdown
Member Author

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 4, 2026
* fix(gateway): preserve chat thinking metadata

* test: harden chat thinking metadata coverage

* test: harden chat thinking metadata coverage
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling app: android App: android app: ios App: ios app: web-ui App: web-ui channel: feishu Channel integration: feishu channel: qqbot commands Command implementations docs Improvements or additions to documentation extensions: ollama gateway Gateway runtime maintainer Maintainer-authored PR merge-risk: 🚨 automation 🚨 May affect CI, automerge, proof capture, label sync, or maintainer automation. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P1 High-priority user-facing bug, regression, or broken workflow. plugin: file-transfer rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XL status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants