Skip to content

fix(message): keep automatic replies out of implicit sends#88228

Closed
qiangu wants to merge 1 commit into
openclaw:mainfrom
qiangu:codex/feishu-automatic-message-tool-replies
Closed

fix(message): keep automatic replies out of implicit sends#88228
qiangu wants to merge 1 commit into
openclaw:mainfrom
qiangu:codex/feishu-automatic-message-tool-replies

Conversation

@qiangu

@qiangu qiangu commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This PR fixes an automatic source-delivery edge case seen with OpenClaw + Feishu/Lark streaming cards + Codex:

  • In embedded runs that have an internal source-reply subscriber, accidental text-only message(action="send") calls to the current source conversation now stay on the source-reply/card path.
  • MCP/CLI/default callers do not opt into that internal sink, so the same implicit send still uses normal outbound instead of being acknowledged and dropped.
  • Explicit routes, explicit targets, cross-channel sends, media, attachments, reply/threading, delivery modifiers, presentation/interactive payloads, and plugin-owned send options still use normal outbound.

Why this matters: previously Codex could emit the first visible answer through the shared message tool as a normal Feishu post while the final/footer came through the Feishu streaming card. The turn then looked split even though streaming cards were enabled.

Behavior Boundary

The automatic source-reply sink is intentionally narrow. It only applies when all of these are true:

  • action === "send"
  • sourceReplyDeliveryMode === "automatic"
  • the embedded runner opts in with allowInternalSourceReplySink: true
  • there is a current source channel context
  • the call has no explicit route/target to another context
  • the send payload is text-only and has no structured send semantics

Normal outbound is preserved for:

  • explicit target, to, channelId, or targets
  • explicit different channel
  • same-channel sends when no internal source-reply consumer is present
  • media hints (media, mediaUrl, base64, image, paths, attachments, mediaUrls)
  • reply/thread controls (replyTo, replyToId, threadId, threadId: null)
  • Slack-style send controls (topLevel: true, replyBroadcast: true)
  • voice/silent/document/pin modifiers when enabled
  • bestEffort when present
  • delivery, channelData, presentation, interactive
  • plugin-owned send parameters

No-op false booleans such as silent:false, asVoice:false, forceDocument:false, gifPlayback:false, topLevel:false, and replyBroadcast:false are still treated as text-only.

Real behavior proof

  • Behavior or issue addressed: Automatic visible replies in Feishu/Lark streaming-card mode could be split when Codex emitted ordinary text through the shared message(action="send") tool. The first visible answer appeared as a normal Feishu message, while the final/completion footer appeared in the streaming card.
  • Real environment tested: OpenClaw 2026.5.27 (27ae826) with Feishu/Lark p2p, Codex runtime, messages.visibleReplies=automatic, channels.feishu.streaming=true, channels.feishu.footer.elapsed=true, and channels.feishu.footer.status=true.
  • Exact steps or command run after this patch: Reproduced the production configuration, inspected the live Feishu/CardKit dispatcher logs, then validated the mitigation by removing the shared message tool from the active embedded agent so normal assistant text was forced through the automatic source-reply dispatcher. This patch implements the same routing intent narrowly in code and verifies it with source checkout tests.
  • Evidence after fix: Copied live terminal/log output from the real Feishu p2p setup showing automatic mode, streaming-card footer config, the message tool removed during live mitigation, and the reply completing through the streaming card path instead of a normal Feishu post:
$ openclaw --version
OpenClaw 2026.5.27 (27ae826)

$ openclaw config get messages.visibleReplies
automatic
$ openclaw config get channels.feishu.streaming
true
$ openclaw config get channels.feishu.footer.elapsed
true
$ openclaw config get channels.feishu.footer.status
true
$ openclaw config get agents.list
[
  {
    "id": "main",
    "tools": {
      "deny": ["...", "message"]
    }
  }
]

2026-05-30T11:53:28.337+08:00 info feishu/card/reply-dispatcher ... reply mode resolved (effectiveReplyMode=auto, replyMode=streaming, chatType=p2p)
2026-05-30T11:53:28.338+08:00 info feishu/card/reply-dispatcher ... footer config resolved (... useStreamingCards=true, rawFooter={"elapsed":true,"status":true}, resolvedFooter={"status":true,"elapsed":true,...})
2026-05-30T11:53:29.511+08:00 info agents/tool-policy ... tool policy removed 1 tool(s) via agents.main.tools.deny: message; matched message
2026-05-30T11:55:02.696+08:00 info feishu/card/streaming ... phase transition (from=idle, to=creating, source=ensureCardCreated)
2026-05-30T11:55:03.688+08:00 info feishu/card/streaming ... sent CardKit card (messageId=redacted)
2026-05-30T11:55:03.693+08:00 info feishu/card/streaming ... phase transition (from=streaming, to=completed, source=onIdle, reason=normal)
2026-05-30T11:55:03.696+08:00 info channels/feishu ... dispatch complete (queuedFinal=true, replies=1)
2026-05-30T11:55:04.329+08:00 info feishu/card/streaming ... reply completed, card finalized (elapsedMs=95990, isCardKit=true)
  • Observed result after fix: The live mitigation produced a single finalized streaming card in the same Feishu p2p setup. In this branch, embedded automatic text-only current-source message/send calls now take that same source-reply/card path, while explicit or structured sends keep normal outbound behavior.
  • What was not tested: A patched source checkout was not driven end-to-end against Feishu because the available Lark credentials/scope and bot membership were insufficient for a patched GUI send. The source branch behavior is covered by focused runner/tool tests and the live production mitigation above demonstrates the affected real OpenClaw + Feishu runtime path.

Validation

Commands run locally:

  • npm exec --yes [email protected] -- pnpm exec oxfmt --check ... on the 14 changed files
  • npm exec --yes [email protected] -- pnpm exec oxlint ... on the 14 changed files
  • git diff --check
  • npm exec --yes [email protected] -- pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.infra.config.ts src/infra/outbound/message-action-runner.send-validation.test.ts src/infra/outbound/message-action-params.test.ts src/infra/outbound/message-action-spec.test.ts — 105 tests
  • npm exec --yes [email protected] -- pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.agents-tools.config.ts src/agents/tools/message-tool.test.ts — 68 tests
  • npm exec --yes [email protected] -- pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.unit-fast.config.ts src/agents/embedded-agent-runner/run/payloads.test.ts src/agents/embedded-agent-runner/run/tool-media-payloads.test.ts — 41 tests
  • npm exec --yes [email protected] -- pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.agents-support.config.ts src/agents/embedded-agent-subscribe.handlers.messages.test.ts — 31 tests
  • npm exec --yes [email protected] -- pnpm exec node scripts/run-vitest.mjs run --config test/vitest/vitest.agents.config.ts src/agents/agent-tools.create-openclaw-coding-tools.test.ts — 56 tests
  • npm exec --yes [email protected] -- pnpm tsgo:test:src

A local Codex review found a thread-control gap (topLevel, threadId:null, replyBroadcast, plugin send options). This update addresses it and adds regression coverage.

Risk

Highest risk: accidentally capturing a legitimate current-channel send that had non-text semantics.

Mitigation:

  • the internal sink requires an explicit embedded-run opt-in
  • default/MCP/CLI callers stay on normal outbound
  • structured send detection now includes thread controls and plugin-owned send parameters
  • tests cover source-reply capture, no-consumer fallback, explicit target escape, channel alias handling, structured payload escape, no-op false options, and source-reply payload suppression behavior

AI-assisted

  • AI-assisted PR.
  • I reviewed the code path and understand the change.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed May 30, 2026, 3:41 AM ET / 07:41 UTC.

Summary
Adds an allowInternalSourceReplySink path so embedded/Codex/loopback text-only current-source message sends can be captured into source-reply delivery while explicit or structured sends stay on normal outbound.

PR surface: Source +230, Tests +661. Total +891 across 18 files.

Reproducibility: Partly. Source inspection shows current main does not consume automatic implicit message-tool sends into the internal source-reply sink and instead can infer a normal outbound current target, but I did not establish a full current-main or patched Feishu/Lark live reproduction.

Review metrics: 1 noteworthy metric.

  • Internal source-reply sink surface: 1 option added, 3 production opt-ins. The new routing option changes message delivery semantics, so maintainers should confirm the opt-in remains limited to embedded/Codex/loopback callers before merge.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦐 gold shrimp
Patch quality: 🦞 diamond lobster
Result: blocked until stronger real behavior proof is added.

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

Rank-up moves:

  • [P1] Add redacted patched-source Feishu/Lark proof showing one streaming card and no separate normal post.
  • Redact private details such as IP addresses, API keys, phone numbers, message IDs, and non-public endpoints before posting proof.
  • After updating the PR body, ClawSweeper should re-review automatically; if it does not, ask a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The PR body includes useful live Feishu/Lark logs for an equivalent mitigation, but it explicitly says the patched source checkout was not driven end-to-end; add redacted patched-branch logs, a screenshot, recording, or linked artifact and update the PR body for re-review.

Mantis proof suggestion
A visible real-transport proof would materially reduce the remaining Feishu/Lark streaming-card delivery risk. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

visual task: verify patched branch in Feishu/Lark p2p automatic visible replies produces one streaming card and no separate normal post.

Risk before merge

  • [P1] The PR intentionally changes opted-in embedded automatic-mode, text-only implicit current-source message sends from normal outbound delivery to the internal source-reply/card path, which can affect workflows that relied on implicit same-channel posts.
  • [P1] The branch also treats an omitted source-reply mode as automatic when an internal source consumer is present, so maintainers should confirm every opt-in caller means automatic delivery when that field is absent.
  • [P1] The supplied real-environment evidence validates the Feishu/Lark configuration and an equivalent mitigation, but not this patched branch exercising the new sink in a live Feishu/Lark gateway path.

Maintainer options:

  1. Require patched-source Feishu/Lark proof (recommended)
    Ask for a redacted run from this branch that exercises the new sink in Feishu/Lark p2p automatic streaming-card mode and shows one card with no separate normal post.
  2. Accept the routing compatibility change
    Maintainers can explicitly accept that opted-in embedded automatic text-only implicit current-source sends now become source replies while explicit and structured escape paths remain covered.
  3. Pause for a narrower delivery contract
    If omitted-mode or same-channel behavior is too broad, pause this PR and define the narrower source-reply contract before changing automatic-mode message delivery.

Next step before merge

  • [P1] Remaining actions are maintainer acceptance of the compatibility-sensitive delivery semantics and patched real behavior proof, not a narrow automation repair.

Security
Cleared: The diff changes TypeScript routing/tests only and does not touch dependencies, workflows, lockfiles, secrets handling, package execution, or other concrete supply-chain surfaces.

Review details

Best possible solution:

Land only after maintainers accept the automatic-mode source-reply sink boundary and a redacted patched Feishu/Lark run shows one streaming card with no separate normal post.

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

Partly. Source inspection shows current main does not consume automatic implicit message-tool sends into the internal source-reply sink and instead can infer a normal outbound current target, but I did not establish a full current-main or patched Feishu/Lark live reproduction.

Is this the best way to solve the issue?

Unclear as a merge decision. The opt-in sink is narrow and well covered by focused tests, but maintainers still need to accept the automatic-mode delivery boundary and require patched-source real proof.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority message-delivery bug fix with focused blast radius but proof and compatibility questions before merge.
  • merge-risk: 🚨 compatibility: The patch changes how existing opted-in embedded automatic-mode implicit current-source text sends are routed.
  • merge-risk: 🚨 message-delivery: A wrong boundary here could split, suppress, or misroute visible replies between normal outbound posts and source-reply streaming cards.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🦞 diamond lobster.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes useful live Feishu/Lark logs for an equivalent mitigation, but it explicitly says the patched source checkout was not driven end-to-end; add redacted patched-branch logs, a screenshot, recording, or linked artifact and update the PR body for re-review.
Evidence reviewed

PR surface:

Source +230, Tests +661. Total +891 across 18 files.

View PR surface stats
Area Files Added Removed Net
Source 11 245 15 +230
Tests 7 715 54 +661
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 18 960 69 +891

What I checked:

Likely related people:

  • steipete: Current-main blame and GitHub commit search connect this handle to the message-action runner sink region and earlier message-tool/sourceReplyDeliveryMode wiring. (role: recent area contributor; confidence: high; commits: 56f46a258166, b62166301efd; files: src/infra/outbound/message-action-runner.ts, src/agents/tools/message-tool.ts, extensions/codex/src/app-server/dynamic-tool-build.ts)
  • loongfay: GitHub commit search found prior merged work on sourceReplyDeliveryMode automatic handling for group chat, adjacent to this delivery-mode boundary. (role: adjacent source-reply contributor; confidence: medium; commits: 128b20431922; files: src/auto-reply/reply/source-reply-delivery-mode.ts)
  • nyanjou: GitHub commit search shows prior message-tool send option work in the outbound runner path, relevant to the structured send options this PR must not capture. (role: adjacent message-tool contributor; confidence: medium; commits: 77df8b11049b; files: src/infra/outbound/message-action-runner.ts, src/agents/tools/message-tool.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.

@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 30, 2026
@qiangu
qiangu marked this pull request as ready for review May 30, 2026 04:25
@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. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. 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. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 30, 2026
@qiangu
qiangu force-pushed the codex/feishu-automatic-message-tool-replies branch from 7bfe23b to fd519c9 Compare May 30, 2026 06:13
@qiangu

qiangu commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Updated after review and force-pushed fd519c9a94.

The review-found thread-control gap is fixed:

  • topLevel:true, threadId:null, and replyBroadcast:true now stay on normal outbound.
  • plugin-owned send parameters stay on normal outbound.
  • no-op false booleans (silent:false, asVoice:false, forceDocument:false, gifPlayback:false, topLevel:false, replyBroadcast:false) still count as text-only.
  • generated idempotencyKey no longer prevents the embedded automatic source-reply sink from doing its job.

I also tightened the PR body so it describes the current behavior accurately: embedded runs opt into the internal source-reply sink; MCP/CLI/default callers do not; explicit/structured sends are preserved.

Latest local validation is listed in the PR body: format, lint, whitespace, focused infra/tool/runner tests, and tsgo:test:src all pass.

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented May 30, 2026 •
edited
Loading

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.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot added triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. proof: supplied External PR includes structured after-fix real behavior proof. and removed proof: supplied External PR includes structured after-fix real behavior proof. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels May 30, 2026
@qiangu
qiangu force-pushed the codex/feishu-automatic-message-tool-replies branch from fd519c9 to 5b89ecf Compare May 30, 2026 07:08
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels May 30, 2026
@qiangu
qiangu force-pushed the codex/feishu-automatic-message-tool-replies branch from 5b89ecf to 922ca5f Compare May 30, 2026 07:15
@qiangu
qiangu force-pushed the codex/feishu-automatic-message-tool-replies branch 2 times, most recently from 43af312 to 07f0618 Compare May 30, 2026 07:28
@openclaw-barnacle openclaw-barnacle Bot added the gateway Gateway runtime label May 30, 2026
@qiangu
qiangu force-pushed the codex/feishu-automatic-message-tool-replies branch from 07f0618 to 555b261 Compare May 30, 2026 07:34
@qiangu qiangu mentioned this pull request May 30, 2026
Merged
@qiangu

qiangu commented May 30, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #88276. This earlier PR focused on message-tool/source-reply routing, but the original Feishu issue is better fixed in the normal Feishu streaming-card reply dispatcher path. Closing this one to keep maintainer review on the smaller, target-aligned patch.

@qiangu qiangu closed this May 30, 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

agents Agent runtime and tooling extensions: codex gateway Gateway runtime 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. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: XL status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@qiangu