Skip to content

Reply: allow authorized group command sessions to receive tool updates#64182

Closed
hongzexin wants to merge 1 commit into
openclaw:mainfrom
hongzexin:codex/feishu-authorized-group-tool-summaries
Closed

Reply: allow authorized group command sessions to receive tool updates#64182
hongzexin wants to merge 1 commit into
openclaw:mainfrom
hongzexin:codex/feishu-authorized-group-tool-summaries

Conversation

@hongzexin

@hongzexin hongzexin commented Apr 10, 2026

Copy link
Copy Markdown

Why

Authorized group command sessions can be an explicitly-invoked operator flow, but the current dispatch logic suppresses both tool summaries and tool-start statuses for all non-forum groups.

That makes command-driven group sessions less observable than direct messages, even when the current message is a real control command and the sender has already passed command authorization.

What changed

  • Allow tool summaries and tool-start statuses in group sessions only when all of the following are true:
    • the session is not a forum topic
    • CommandAuthorized === true
    • the current inbound message is a real control command
  • Keep the existing suppression behavior for normal group chatter
  • Keep forum-topic behavior unchanged
  • Keep the implementation provider-agnostic instead of hardcoding Feishu/Lark logic into core dispatch
  • Add coverage for:
    • authorized group command sessions
    • unauthorized group sessions
    • slash-prefixed path-like text such as /tmp/..., which should remain suppressed

Tests

  • node scripts/run-vitest.mjs run src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/command-control.test.ts extensions/feishu/src/bot.test.ts (3 files, 217 tests)
  • pnpm exec oxlint src/auto-reply/command-detection.ts src/auto-reply/command-control.test.ts src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/command-detection.ts src/auto-reply/command-control.test.ts src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts CHANGELOG.md
  • git diff --check origin/main..HEAD

Real behavior proof

  • Behavior or issue addressed: Authorized non-forum group control-command turns can emit verbose tool progress, while ordinary group chatter, path-like slash text, unauthorized group commands, and Slack non-direct surfaces stay quiet.
  • Real environment tested: Local OpenClaw checkout on macOS, rebased onto current origin/main at b680360fde68a129e72b5ed41803bf788f4b8c3e, running the repaired Reply: allow authorized group command sessions to receive tool updates #64182 code at 9a625fd6a70a3efe39dacdf1a16c5afdbc5c190a.
  • Exact steps or command run after this patch: Ran node --import tsx --input-type=module - <<'EOF' ... EOF from the OpenClaw repo to exercise the real shared command detector and the same verbose-gate predicate shape used by reply dispatch.
  • Evidence after fix: Console output from that command:
mention command detected: true
path-like slash detected: false
authorized group command verbose gate: true
unauthorized group command verbose gate: false
path-like group text verbose gate: false
slack channel command verbose gate: false
  • Observed result after fix: Mention-prefixed /status is recognized as a real control command, @OpenClaw /tmp/openclaw.log is not, only an authorized group control command opens the verbose progress gate, and Slack channel commands remain suppressed by the Slack non-direct guard.
  • What was not tested: No additional gaps for this PR's changed behavior; forum-topic behavior and media-only suppressed group delivery remain covered by existing focused dispatch tests.

Out of scope

  • Any change to forum-topic behavior
  • Any change to media-only tool-result delivery for suppressed group sessions
  • Any provider-specific capability flag or per-channel config surface

@greptile-apps

greptile-apps Bot commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR allows inline tool summaries to be delivered in Feishu/Lark group sessions when CommandAuthorized === true, leaving suppression in place for unauthorized groups and all non-Feishu channels.

  • The implementation embeds \"feishu\" and \"lark\" string literals directly into dispatch-from-config.ts, which violates the project's explicit architecture rule against hardcoded provider names in core dispatch logic. The Feishu plugin should express this opt-in through a manifest capability or plugin-owned channel contract so core remains provider-agnostic.
  • shouldSendToolStartStatuses on the immediately following line is not updated to match, leaving tool start statuses suppressed even for the same authorized Feishu sessions the PR intends to unlock.

Confidence Score: 4/5

Merge after resolving the architecture boundary violation — the functional intent is correct but the implementation approach is explicitly prohibited by project rules.

A P1 finding (hardcoded provider names in core) matches a rule that CLAUDE.md flags as a refactor trigger, and there is an asymmetric behavior gap in shouldSendToolStartStatuses. Both need to be addressed before landing.

src/auto-reply/reply/dispatch-from-config.ts — the shouldSendToolSummaries / shouldSendToolStartStatuses block around line 570

Prompt To Fix All With AI
This is a comment left during a code review.
Path: src/auto-reply/reply/dispatch-from-config.ts
Line: 570-574

Comment:
**Hardcoded provider names in core violate the architecture boundary**

The CLAUDE.md/AGENTS.md project rules explicitly state: *"do not add hardcoded bundled extension/provider/channel/capability id lists, maps, or named special cases in core when a manifest, capability, registry, or plugin-owned contract can express the same behavior."* Embedding `"feishu"` and `"lark"` string literals directly into `dispatch-from-config.ts` is exactly this pattern — it bakes Feishu-owned behavior into the generic dispatch layer, making it harder to add similar behavior for other channels without further core edits.

The correct approach is for the Feishu plugin to express this opt-in through a manifest capability flag or a plugin-owned channel contract hook (e.g. `allowToolSummariesWhenAuthorized: true`), letting the generic dispatch code read that flag without knowing which provider it belongs to.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/auto-reply/reply/dispatch-from-config.ts
Line: 575

Comment:
**`shouldSendToolStartStatuses` is not updated symmetrically**

`shouldSendToolSummaries` now allows authorized Feishu group sessions through, but `shouldSendToolStartStatuses` on the very next line keeps the original `ChatType !== "group" || IsForum` check, so tool *start* statuses remain suppressed even for these same sessions. If the intent is to treat authorized Feishu groups like an explicitly-invoked command session (as the PR description says), the two flags should behave consistently.

```suggestion
    const shouldSendToolStartStatuses =
      ctx.ChatType !== "group" ||
      ctx.IsForum === true ||
      ((normalizedSurface === "feishu" || normalizedSurface === "lark") && ctx.CommandAuthorized);
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/auto-reply/reply/dispatch-from-config.ts
Line: 570

Comment:
**Use the already-imported `normalizeLowercaseStringOrEmpty` helper**

The file already imports `normalizeLowercaseStringOrEmpty` from `../../shared/string-coerce.js` (line 40). Rolling a separate inline `(... ?? "").trim().toLowerCase()` chain duplicates that utility.

```suggestion
    const normalizedSurface = normalizeLowercaseStringOrEmpty(ctx.Surface ?? ctx.Provider);
```

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "Feishu: allow tool summaries in authoriz..." | Re-trigger Greptile

Comment thread src/auto-reply/reply/dispatch-from-config.ts Outdated
Comment thread src/auto-reply/reply/dispatch-from-config.ts Outdated
Comment thread src/auto-reply/reply/dispatch-from-config.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f5488edd23

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/auto-reply/reply/dispatch-from-config.ts Outdated
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from f5488ed to f2109c7 Compare April 10, 2026 08:25
@hongzexin hongzexin changed the title Feishu: allow tool summaries in authorized groups Reply: allow authorized group command sessions to receive tool updates Apr 10, 2026

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f2109c7544

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread src/auto-reply/reply/dispatch-from-config.ts
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch 2 times, most recently from 6b47117 to dd895b4 Compare April 10, 2026 12:46
@clawsweeper

clawsweeper Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I did a careful shell check against current main, and the useful part of this older PR is already implemented there.

Close as mostly implemented on main: the useful group tool/progress visibility problem is now covered by the shipped shared /verbose policy from #85488, while this older branch is conflicting and its remaining command-only parser/start-status details are a separate follow-up only if they still reproduce.

So I’m closing this older PR as already covered on main rather than keeping a mostly-duplicated branch open.

Review details

Best possible solution:

Keep the shipped #85488 /verbose group/channel progress policy and open a fresh narrow follow-up only for a current mention-prefix or tool-start-status repro.

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

Not applicable as a feature/policy PR; source inspection shows the historical group suppression path, and current main plus v2026.6.6 show the superseding explicit /verbose behavior.

Is this the best way to solve the issue?

Yes for closing: #85488 is the better maintainer solution because it uses one shared verbose-state gate, preserves quiet defaults, and avoids adding a separate command-authorization bypass.

Security review:

Security review needs attention: If merged, the branch would expand group-visible progress output and needs privacy review; closing in favor of #85488 avoids that merge risk.

  • [medium] Group-visible progress can expose internal context — src/auto-reply/reply/dispatch-from-config.ts:985
    The dispatch change allows verbose progress and tool-start messages in non-forum groups for authorized command turns, which can reveal command names, arguments, file paths, URLs, or diagnostics to room participants.
    Confidence: 0.84

AGENTS.md: found and applied where relevant.

What I checked:

Likely related people:

  • kurplunkin: Authored the merged fix(channels): honor /verbose in group sessions #85488 series that now owns the shared group /verbose progress behavior covering this PR's central useful change. (role: feature owner; confidence: high; commits: 45fbf2d81a0e, 38f1af701778, 0ba4f9e2386b; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.test.ts)
  • steipete: Merged fix(channels): honor /verbose in group sessions #85488 and authored the maintainer fixup stack for quiet defaults, live verbose gating, and follow-up progress behavior on the same dispatch surface. (role: merger and recent area contributor; confidence: high; commits: 45fbf2d81a0e, 3ff396b6e279, d8e20b764911; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/followup-runner.ts)
  • Mariano Belinky: Earlier history on this dispatch path includes the reply: make progress updates respect verbose change, which is adjacent to the later shared verbose-progress policy. (role: earlier adjacent contributor; confidence: medium; commits: b66454115801; files: src/auto-reply/reply/dispatch-from-config.ts)

Codex review notes: model internal, reasoning high; reviewed against c78f9376d929; fix evidence: release v2026.6.6, commit 45fbf2d81a0e.

@clawsweeper

clawsweeper Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge.

What this changes:

The PR updates command detection and reply dispatch so authorized non-forum group command sessions can receive text tool summaries and tool-start statuses, with tests for authorized/unauthorized groups, mention-prefixed commands, and path-like slash text.

Maintainer follow-up before merge:

This is an open implementation PR with a narrow and valid goal, but it changes group-visible progress behavior and needs maintainer review/rebase against the current shared verbose-progress gate rather than an automatic replacement fix lane.

Review details

Best possible solution:

Update and review this PR against current main by adding a provider-agnostic authorized group command predicate at the shared verbose-progress gate, applying it consistently to tool summaries and tool-start statuses, preserving Slack non-direct suppression, normal group-chatter suppression, unauthorized suppression, and forum-topic behavior, and covering mention-prefixed commands plus path-like slash inputs in tests.

Acceptance criteria:

  • pnpm test src/auto-reply/reply/dispatch-from-config.test.ts src/auto-reply/command-control.test.ts extensions/feishu/src/bot.test.ts
  • pnpm exec oxfmt --check --threads=1 src/auto-reply/command-detection.ts src/auto-reply/command-detection.test.ts src/auto-reply/reply/dispatch-from-config.ts src/auto-reply/reply/dispatch-from-config.test.ts

What I checked:

  • Current main still suppresses requested group updates: shouldSendVerboseProgressMessages is false for Slack non-direct surfaces and for ChatType === "group" unless IsForum === true; both shouldSendToolSummaries and shouldSendToolStartStatuses directly inherit that value, with no CommandAuthorized or command-message exception. (src/auto-reply/reply/dispatch-from-config.ts:774, acae48b790fa)
  • Tool-result suppression path is still active: When summaries are disabled, resolveToolDeliveryPayload returns text tool payloads only for exec approval or media-bearing results; ordinary text summaries return null. (src/auto-reply/reply/dispatch-from-config.ts:1023, acae48b790fa)
  • Tool-start statuses share the same suppression: maybeSendWorkingStatus returns before sending when shouldSendToolStartStatuses is false, so authorized group command sessions do not receive tool-start status messages on current main. (src/auto-reply/reply/dispatch-from-config.ts:927, acae48b790fa)
  • Existing tests still cover the old group behavior: Current tests assert that ordinary group sessions suppress text tool summaries while preserving media-only tool results, and separately assert forum-topic groups deliver summaries. There is no current authorized non-forum group-command exception test. (src/auto-reply/reply/dispatch-from-config.test.ts:1323, acae48b790fa)
  • Feishu already forwards command authorization: The Feishu plugin normalizes group command probe text, computes command authorization from channel authorizers, and writes CommandAuthorized onto the inbound context, which supports a provider-agnostic dispatch gate rather than hardcoded provider names. (extensions/feishu/src/bot.ts:639, acae48b790fa)
  • Mention-aware control-command parsing is not on main: isControlCommandMessage trims, checks hasControlCommand, strips inbound metadata, and checks abort triggers, but it does not strip leading Feishu <at ...> tags or plain @Bot prefixes before command matching. (src/auto-reply/command-detection.ts:54, acae48b790fa)

Likely related people:

  • steipete: Recent history shows repeated maintenance of dispatch-from-config.ts, the shared verbose-progress gate, and Feishu group/auth behavior, including the Slack verbose-progress suppression commit and several Feishu group-policy/reply-target fixes. (role: recent maintainer and adjacent owner; confidence: high; commits: 76a4c167f7ef, 249cb5437364, 67b16a4a6d25; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.test.ts, extensions/feishu/src/bot.ts)
  • scoootscooob: Authored the recent group visible-replies behavior change that made group/channel replies tool-only by default and updated the same dispatch tests/docs surface this PR affects. (role: adjacent owner; confidence: medium; commits: 3c636208b0a0; files: src/auto-reply/reply/dispatch-from-config.ts, src/auto-reply/reply/dispatch-from-config.test.ts, docs/channels/groups.md)
  • vincentkoc: Recent history shows work on command-listing/detection and Feishu runtime seams, both relevant to a provider-agnostic authorized-command dispatch predicate. (role: adjacent maintainer; confidence: medium; commits: 1f1b50498049, 2b96f53f9782, d609f71c9b74; files: src/auto-reply/command-detection.ts, extensions/feishu/src/bot.ts)

Remaining risk / open question:

  • The intended behavior expands group-visible progress output, so the gate must stay strict: CommandAuthorized === true, a real standalone control command, and no path-like slash text or ordinary group chatter leakage.
  • Current main now gates summaries, start statuses, and plan updates through shouldSendVerboseProgressMessages; the PR patch was written against an older shape and needs a maintainer decision on whether plan updates should share the authorized-command exception.
  • No tests were run because this was a read-only cleanup review.

Codex review notes: model gpt-5.5, reasoning high; reviewed against acae48b790fa.

Re-review progress:

@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from dd895b4 to 14ea864 Compare May 6, 2026 17:26
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 6, 2026
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from 14ea864 to b680dfd Compare May 6, 2026 17:29
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from b680dfd to 08ee7c5 Compare May 6, 2026 18:26
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from 08ee7c5 to 4639b6d Compare May 6, 2026 18:29
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 6, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 6, 2026
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from 4639b6d to cba538f Compare May 7, 2026 03:57
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from cba538f to 0e35f8a Compare May 7, 2026 04:08
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from 0e35f8a to ac949b2 Compare May 7, 2026 04:14
@clawsweeper clawsweeper Bot added the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026
@hongzexin
hongzexin force-pushed the codex/feishu-authorized-group-tool-summaries branch from ac949b2 to 9a625fd Compare May 7, 2026 04:23
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 7, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels May 7, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal backlog priority with limited blast radius. 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. labels May 19, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🔥 Warming up: real-behavior proof passed; findings, security review, or rank-up moves are still in progress.

Hatch command

Comment @clawsweeper hatch when this PR is hatchable.

Hatchability rules:

  • Merged PRs are hatchable.
  • Open PRs are hatchable when they are status: 👀 ready for maintainer look, status: 🚀 automerge armed, or labeled clawsweeper:automerge.
  • Closed unmerged PRs are hatchable only when one of those hatchable labels is still present in the durable record.
What is this egg doing here?
  • Eggs appear after the PR passes real-behavior proof. It is here for vibes, not verdicts: it does not change labels, ratings, merge decisions, or automation.
  • The shell reacts to review momentum: open follow-up work warms it up, re-review makes it wobble, and a clean final review lets it hatch.
  • Hatchability usually comes from sufficient real-behavior proof, no blocking P0/P1/P2 findings, no security attention needed, and clean correctness. A merged PR is already final, so merge makes the egg hatchable independently.
  • The hatch is seeded from this repository and PR number, so the same PR keeps the same creature; the reviewed head SHA can only change safe visual details.
  • Rarity is just collectible sparkle: 🥚 common, 🌱 uncommon, 💎 rare, ✨ glimmer, and 🌈 legendary.

@openclaw-barnacle

Copy link
Copy Markdown

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 5, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. labels Jun 5, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 6, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

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

Labels

mantis: telegram-visible-proof Mantis should capture Telegram visible proof. 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. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant