Skip to content

feat(messages): add toolMessageLogging override#82691

Closed
laorient wants to merge 1 commit into
openclaw:mainfrom
laorient:feat/tool-message-logging-override
Closed

feat(messages): add toolMessageLogging override#82691
laorient wants to merge 1 commit into
openclaw:mainfrom
laorient:feat/tool-message-logging-override

Conversation

@laorient

@laorient laorient commented May 16, 2026

Copy link
Copy Markdown

Summary

  • add a messages.toolMessageLogging configuration override to control tool-message logging behavior
  • wire the setting through schema labels/help/types and session zod schema
  • cover config dispatch behavior with auto-reply tests

Test

  • pnpm exec vitest run src/auto-reply/reply/dispatch-from-config.test.ts --config vitest.config.ts

Copilot AI review requested due to automatic review settings May 16, 2026 18:07
@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation gateway Gateway runtime size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 16, 2026
Copilot AI reviewed May 16, 2026
View reviewed changes

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

Adds a messages.toolMessageLogging config override that explicitly controls whether intermediate tool result/status messages are sent, replacing the previously hardcoded DM/forum-vs-group default (which is preserved as the fallback).

Changes:

  • New optional messages.toolMessageLogging field on the messages config (schema, type, label, help text).
  • dispatchReplyFromConfig now uses cfg.messages?.toolMessageLogging to drive both shouldSendToolSummaries and shouldSendToolStartStatuses, falling back to the prior chat-type heuristic.
  • Documentation and unit tests covering both the suppress-in-DM and enable-in-group cases.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/config/zod-schema.session.ts Adds optional boolean field to MessagesSchema.
src/config/types.messages.ts Adds typed toolMessageLogging field with doc comment.
src/config/schema.labels.ts UI label for the new field.
src/config/schema.help.ts Help text describing default auto behavior.
src/auto-reply/reply/dispatch-from-config.ts Honors override for tool summaries/start statuses.
src/auto-reply/reply/dispatch-from-config.test.ts Tests for false in DM and true in group.
docs/gateway/configuration-reference.md Documents the new option.

@clawsweeper

clawsweeper Bot commented May 16, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the useful tool/progress visibility behavior is now owned by the merged verbose-state work on current main and in the latest release, while this stale branch adds a second config knob that conflicts with the shipped direction and lacks real behavior proof.

Canonical path: Keep the shipped /verbose and verbose-default controls as the canonical tool/progress visibility path; any new global delivery knob should start as a fresh product/API proposal against current main.

So I’m closing this here because the remaining work is already tracked in the canonical issue.

Review details

Best possible solution:

Keep the shipped /verbose and verbose-default controls as the canonical tool/progress visibility path; any new global delivery knob should start as a fresh product/API proposal against current main.

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

Not applicable as a feature/config PR. The related bug path is already handled by merged verbose-state work, and this branch provides no after-fix real chat proof for the proposed new config key.

Is this the best way to solve the issue?

No. The best current solution is the shipped /verbose and verbose-default path; adding messages.toolMessageLogging would duplicate privacy-sensitive delivery policy and needs a fresh product/API decision if still desired.

Security review:

Security review needs attention: The diff changes privacy-sensitive tool/status visibility and can expose intermediate progress text in shared rooms if the new override is enabled.

  • [medium] Duplicate privacy-sensitive progress control — src/auto-reply/reply/dispatch-from-config.ts:560
    The new toolMessageLogging override bypasses the shipped verbose-state policy and can make intermediate tool/status summaries visible in shared chat surfaces without a fresh product/security decision.
    Confidence: 0.88

AGENTS.md: found and applied where relevant.

What I checked:

  • PR diff adds a new messages config surface: The branch adds messages.toolMessageLogging and uses it to override tool-summary and tool-start-status delivery in dispatchReplyFromConfig. (src/auto-reply/reply/dispatch-from-config.ts:558, 06d32dbc41fd)
  • Current main uses verbose state as the delivery gate: Current main derives tool-summary delivery from shouldEmitVerboseProgress() through shouldSuppressDefaultToolProgressMessages, with no toolMessageLogging key in the current config type. (src/auto-reply/reply/dispatch-from-config.ts:2182, cae64ee360a8)
  • Current docs document the shipped control: The groups docs say sessions suppress verbose tool/progress summaries by default and use /verbose on or /verbose off to control summaries for the current session. Public docs: docs/channels/groups.md. (docs/channels/groups.md:65, cae64ee360a8)
  • Latest release contains the verbose-gated implementation: Release tag v2026.6.6 contains the shouldSuppressDefaultToolProgressMessages / shouldSendToolSummaries implementation and no toolMessageLogging matches in the changed config/runtime paths. (src/auto-reply/reply/dispatch-from-config.ts:2091, 8c802aa68351)
  • Superseding PR merged: The merged verbose-state PR explicitly kept normal groups quiet by default, used /verbose as the opt-in, stated no new config flag, and merged as 45fbf2d81a0e6955d977bc4b1c7d29458c2894d0 on 2026-05-23. (45fbf2d81a0e)
  • Related bug cluster resolved by the superseding work: The related Matrix issue was closed with a comment saying the merged verbose-state work fixed the leak, and the narrower alternative PR was closed as superseded by the same merged PR. (45fbf2d81a0e)

Likely related people:

  • kurplunkin: Authored the merged verbose-state PR that made /verbose the canonical group/channel tool-progress control without adding a new config flag. (role: introduced superseding behavior; confidence: high; commits: 45fbf2d81a0e; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.test.ts, docs/channels/groups.md)
  • steipete: Authored earlier group/channel visible-reply and Slack room-suppression behavior that the superseding path builds on, and posted verification on the merged verbose-state work. (role: recent adjacent contributor and verifier; confidence: high; commits: 76a4c167f7ef, 1e5450f23e1c; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/source-reply-delivery-mode.ts, src/auto-reply/reply/dispatch-from-config.test.ts)
  • anyech: Authored the merged Discord verbose tool-progress delivery work whose explicit progress-delivery contract constrained the later shared fix. (role: adjacent feature contributor; confidence: medium; commits: 57da466ecbb5; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.test.ts)
  • jalehman: Current blame on the live verbose-progress dispatch gate points to the session accessor refactor that carried the current implementation forward. (role: recent current-main area contributor; confidence: medium; commits: ef47dd610c87; files: src/auto-reply/reply/dispatch-from-config.ts)

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

@clawsweeper clawsweeper Bot added mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. labels May 16, 2026
@lukeboyett lukeboyett mentioned this pull request May 21, 2026
Closed
@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. 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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. and removed merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

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

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

docs Improvements or additions to documentation gateway Gateway runtime mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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. merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. 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.

2 participants

@laorient