Skip to content

fix(discord): deliver reasoning payloads when /reasoning on is active#95014

Closed
1052326311 wants to merge 2 commits into
openclaw:mainfrom
1052326311:fix/issue-new-batch
Closed

fix(discord): deliver reasoning payloads when /reasoning on is active#95014
1052326311 wants to merge 2 commits into
openclaw:mainfrom
1052326311:fix/issue-new-batch

Conversation

@1052326311

@1052326311 1052326311 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Closes #94936

Summary

Discord's message handler had two suppression points that dropped all isReasoning block and final replies. The shared agent subscriber already emits isReasoning block replies when /reasoning on is active, but Discord immediately suppressed them.

Changes

  • Removed both isReasoning suppression guards in message-handler.process.ts
  • Updated tests from "suppresses" to "delivers" assertions

Fixed: 2 files, +9/-30

  • extensions/discord/src/monitor/message-handler.process.ts
  • extensions/discord/src/monitor/message-handler.process.test.ts

Verification

  • All 107 Discord process tests pass
  • Reasoning payloads now reach deliverDiscordReply instead of being dropped

@1052326311
fix(discord): deliver reasoning payloads when /reasoning on is active
2b3a7c5 
Sweeper 🦞 openclaw#94936: Discord's message handler suppressed reasoning block
and final payloads at two points (beforeDiscordPayloadDelivery and
deliverDiscordPayload). The shared agent subscriber already emits
isReasoning block replies when reasoningMode === on, but Discord
immediately dropped them.

Remove both suppression guards. Update tests:
- "suppresses reasoning payload" → "delivers reasoning payload"
- "suppresses reasoning-tagged final" → "delivers reasoning-tagged final"

All 107 Discord process tests pass.
@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 19, 2026, 12:19 PM ET / 16:19 UTC.

Summary
The branch removes two Discord isReasoning suppression guards and updates Discord handler tests, while also adding a separate Gateway-backed openclaw skills status path.

PR surface: Source +5, Tests +3. Total +8 across 3 files.

Reproducibility: yes. Source inspection shows /reasoning on can emit isReasoning block replies, while current Discord delivery suppresses those payloads before sending.

Review metrics: 2 noteworthy metrics.

  • Issue scopes bundled: 2 unrelated fixes. The branch targets Discord reasoning delivery and remote skills CLI eligibility, so maintainers should split review and proof before merge.
  • Reasoning delivery gates: 2 removed. Removing both isReasoning guards changes Discord message delivery and makes output formatting important before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94936
Summary: This PR is a candidate fix for the open Discord /reasoning on message-loss issue, but it also carries an adjacent unrelated skills CLI fix that already has its own open PR.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

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

Rank-up moves:

  • Remove the unrelated skills CLI commit from this Discord PR.
  • Format delivered Discord reasoning with the existing Thinking formatter and assert the final delivered text.
  • [P1] Add redacted real Discord proof showing /reasoning on with a reasoning model after the patch.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR only reports Discord process tests and deliverDiscordReply mock assertions; it needs redacted real Discord after-fix proof before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Mantis proof suggestion
A visible real Discord proof would materially show whether /reasoning on renders a separate Thinking message before the final answer. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify Discord /reasoning on with a reasoning model shows a separate Thinking message before the final answer.

Risk before merge

  • [P1] Merging as-is would ship a remote skills CLI behavior change from a separate issue without the matching PR body, review focus, or real CLI/Gateway proof on this PR.
  • [P1] Removing the Discord guards alone would send raw reasoning text as an ordinary Discord reply rather than the existing Thinking-formatted reasoning message.
  • [P1] The PR only reports mocked/unit Discord process tests; there is no redacted real Discord session showing /reasoning on with a reasoning model after the patch.

Maintainer options:

  1. Split and finish the Discord fix (recommended)
    Remove the unrelated skills CLI commit, format Discord reasoning payloads before delivery, and attach redacted real Discord proof before merge.
  2. Accept plain reasoning intentionally
    Maintainers could choose raw reasoning output, but that should be explicit and backed by docs/tests because it changes the visible reasoning contract.
  3. Pause behind canonical candidates
    If another open Discord reasoning PR becomes the chosen path, pause or close this branch after the canonical path is viable and proof-positive.

Next step before merge

  • [P1] Needs contributor or maintainer handling because the branch must be split, the Discord output shape needs repair, and real Discord proof cannot be supplied by automation.

Security
Cleared: No supply-chain, workflow, dependency, credential, or permission change was found; the new CLI Gateway call is a functional compatibility concern rather than a concrete security issue.

Review findings

  • [P1] Remove the unrelated skills CLI change from this Discord PR — src/cli/skills-cli.ts:103
  • [P2] Format reasoning before delivering it — extensions/discord/src/monitor/message-handler.process.ts:609
Review details

Best possible solution:

Land one canonical Discord fix that formats isReasoning payloads as Thinking messages with real Discord proof, and keep the remote skills CLI fix in its own PR.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows /reasoning on can emit isReasoning block replies, while current Discord delivery suppresses those payloads before sending.

Is this the best way to solve the issue?

No. Removing the suppression is necessary but not the best complete fix because the PR also bundles unrelated skills work and does not format Discord reasoning as the documented Thinking message.

Full review comments:

  • [P1] Remove the unrelated skills CLI change from this Discord PR — src/cli/skills-cli.ts:103
    This branch is titled and described as the Discord reasoning fix, but it also adds a Gateway-backed loadSkillsStatusReport path for Regression: remote macOS apple-notes eligibility still misses memo/darwin in 2026.6.8 after #71877 #94956. That work already has fix(skills): use remote-aware Gateway skills.status in CLI when available #95016 and needs its own CLI/Gateway proof, so merging here would couple unrelated release decisions.
    Confidence: 0.94
  • [P2] Format reasoning before delivering it — extensions/discord/src/monitor/message-handler.process.ts:609
    After these guards are removed, deliverDiscordPayload forwards isReasoning payloads unchanged. The shared subscriber emits raw trimmed reasoning, while docs and formatReasoningMessage define visible reasoning as a separate Thinking message; deliverDiscordReply only sanitizes and sends, so users would see raw reasoning as an ordinary Discord reply.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a normal-priority Discord message-delivery bugfix PR with limited blast radius but clear merge blockers.
  • add merge-risk: 🚨 compatibility: The branch changes openclaw skills behavior for existing users with gateway.remote.url configured, while that work belongs to a separate review path.
  • add merge-risk: 🚨 message-delivery: The Discord diff changes which reasoning messages are delivered and currently risks sending them in the wrong visible form.

Label justifications:

  • P2: This is a normal-priority Discord message-delivery bugfix PR with limited blast radius but clear merge blockers.
  • merge-risk: 🚨 compatibility: The branch changes openclaw skills behavior for existing users with gateway.remote.url configured, while that work belongs to a separate review path.
  • merge-risk: 🚨 message-delivery: The Discord diff changes which reasoning messages are delivered and currently risks sending them in the wrong visible form.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR only reports Discord process tests and deliverDiscordReply mock assertions; it needs redacted real Discord after-fix proof before merge. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +5, Tests +3. Total +8 across 3 files.

View PR surface stats
Area Files Added Removed Net
Source 2 29 24 +5
Tests 1 9 6 +3
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 3 38 30 +8

What I checked:

Likely related people:

  • scoootscooob: Moved the Discord channel implementation into the current extension path, which is relevant history for this message delivery module. (role: feature-history contributor; confidence: medium; commits: 5682ec37fada; files: extensions/discord/src/monitor/message-handler.process.ts, extensions/discord/src/monitor/message-handler.process.test.ts)
  • steipete: Introduced and refactored the skills CLI and remote skills support paths touched by the unrelated second commit. (role: adjacent skills CLI and remote eligibility contributor; confidence: medium; commits: 903f5af59ca2, b2b331230b00, c21792f5a0d6; files: src/cli/skills-cli.ts, src/gateway/server-methods/skills.ts, src/skills/runtime/remote.ts)
  • joshavant: Current checkout blame points to this recent snapshot commit for the central Discord and skills lines, but the grafted history makes this a weak routing signal. (role: recent area contributor; confidence: low; commits: f13dc76ba181; files: extensions/discord/src/monitor/message-handler.process.ts, src/cli/skills-cli.ts, src/gateway/server-methods/skills.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.

@1052326311
fix(skills): use remote-aware Gateway skills.status in CLI when avail…
9c7656c 
…able

Sweeper 🦞 openclaw#94956: the CLI skills subcommands (list, check, info)
used a local-only status build that did not account for remote macOS
node capabilities. Even after the parser fix (openclaw#71877) for system.which
object-map payloads, CLI status reports still showed darwin+bins(memo)
as missing on Linux gateways with connected macOS nodes.

Now loadSkillsStatusReport tries the Gateway skills.status RPC
(which already builds with getRemoteSkillEligibility) when
config.gateway.remote.url is set, falling back to local-only buildWorkspaceSkillStatus
when the Gateway is unreachable.

Test results:
- skills-cli.commands: 52/52 pass
- skills/runtime/remote: 12/12 pass
- gateway skills.proposals: 10/10 pass
@openclaw-barnacle openclaw-barnacle Bot added cli CLI command changes size: S and removed size: XS labels Jun 19, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 19, 2026
@1052326311 1052326311 mentioned this pull request Jun 19, 2026
Closed
This was referenced Jun 19, 2026
Closed
Merged
@1052326311

1052326311 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Closing: replaced by cleaner PR with only the Discord fix (no unrelated skills commit) and proper Thinking message formatting per sweeper P1 feedback.

@1052326311 1052326311 closed this Jun 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

channel: discord Channel integration: discord cli CLI command changes merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Reasoning-model thinking (type:"thinking") never rendered on Discord — /reasoning on shows no thoughts

1 participant

@1052326311