Skip to content

Suppress internal agent failure traces before channel delivery#89125

Merged
obviyus merged 4 commits into
openclaw:mainfrom
fuller-stack-dev:codex/agent-failed-message-suppression
Jun 3, 2026
Merged

Suppress internal agent failure traces before channel delivery#89125
obviyus merged 4 commits into
openclaw:mainfrom
fuller-stack-dev:codex/agent-failed-message-suppression

Conversation

@fuller-stack-dev

@fuller-stack-dev fuller-stack-dev commented Jun 1, 2026

Copy link
Copy Markdown
Member

Summary

  • Move internal agent/tool trace-line stripping into the shared assistant-visible text sanitizer so non-Discord delivery users get the same suppression for failed-tool payloads.
  • Preserve Discord's stricter send-boundary guard for plain internal channel labels such as analysis:, commentary:, thinking:, and reasoning:.
  • Strip trace-only error-context payloads before user-facing delivery while preserving explicit tool-progress payloads and non-text Discord payloads.

Verification

  • node scripts/run-vitest.mjs src/shared/text/assistant-visible-text.test.ts src/agents/embedded-agent-helpers.sanitizeuserfacingtext.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/monitor/message-handler.process.test.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/followup-runner.test.ts -t "tool progress"
  • git diff --check

Real behavior proof

Behavior addressed: Internal agent/tool failure trace lines such as ⚠️ 🛠️ \run ... (agent)` failedare no longer delivered as user-visible Discord messages when they are the only final text. Discord delivery also continues to suppress plain internal channel-label lines such asanalysis:andcommentary:` while still sending media, channelData, and presentation payloads when their text scrubs empty.

Real environment tested: Local OpenClaw source checkout on current origin/main plus real Discord archive evidence from a redacted OpenClaw Discord channel. Discrawl status reported the archive current at 2026-06-01T15:48:21Z with 1,478,897 messages; the leaked Discord rows below are from May 20-25, 2026. Discord channel names, author identifiers, guild identifiers, channel identifiers, and message identifiers are intentionally redacted from the public PR text.

Exact steps or command run after this patch: Queried the Discord archive for real leaked messages with redacted presentation fields, then invoked the patched Discord sanitizer directly with the same visible failure shape:

DISCRAWL_NO_AUTO_UPDATE=1 discrawl --json sql "select m.created_at, '<redacted-discord-channel>' as channel, '<redacted-discord-bot>' as author, m.content from messages m where m.content like '%⚠️ 🛠️%' and m.created_at >= '2026-05-20' order by m.created_at desc limit 2;"
node_modules/.bin/tsx -e 'import { sanitizeDiscordFrontChannelReplyPayloads } from "./extensions/discord/src/monitor/reply-safety.ts"; const samples = [{ text: "⚠️ 🛠️ `run python3 inline script (heredoc) (agent)` failed" }, { text: "commentary: calling tool\nanalysis: inspect private state", mediaUrl: "https://example.com/result.png" }]; console.log(JSON.stringify(sanitizeDiscordFrontChannelReplyPayloads(samples, { kind: "final" }), null, 2));'

Before evidence: Copied live Discord archive output shows the actual leaked user-visible messages this PR targets, with channel and author identifiers redacted:

{
  "columns": ["created_at", "channel", "author", "content"],
  "rows": [
    [
      "2026-05-25T17:42:52.029Z",
      "<redacted-discord-channel>",
      "<redacted-discord-bot>",
      "⚠️ 🛠️ `run python3 inline script (heredoc) (agent)` failed"
    ],
    [
      "2026-05-25T17:01:58.578Z",
      "<redacted-discord-channel>",
      "<redacted-discord-bot>",
      "⚠️ 🛠️ `pkill -f \"gh pr view\" || true (agent)` failed"
    ]
  ]
}

Evidence after fix: The patched Discord sanitizer drops the text-only leaked failure payload and preserves only the media payload after stripping its internal commentary:/analysis: text:

[
  {
    "mediaUrl": "https://example.com/result.png"
  }
]

Observed result after fix: The text-only leaked failure message produced no deliverable Discord payload. The media sample remained deliverable, but the internal channel-label text was removed. Focused sanitizer and Discord tests also passed, including the process test that suppresses warning-only final replies instead of sending them.

What was not tested: A deployed live Discord gateway running this exact branch was not exercised. The proof combines copied output from real Discord archive rows, with public identifiers redacted, and direct execution of the patched Discord send-boundary sanitizer in a local source checkout.

@openclaw-barnacle openclaw-barnacle Bot added channel: discord Channel integration: discord agents Agent runtime and tooling size: M triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 1, 2026
@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed June 2, 2026, 12:19 PM ET / 16:19 UTC.

Summary
The PR moves internal agent/tool trace stripping into shared assistant-visible text sanitization and applies it at Discord delivery boundaries while adding focused sanitizer and Discord delivery tests.

PR surface: Source +70, Tests +99. Total +169 across 8 files.

Reproducibility: yes. from source and related live reports: current main lacks shared compact failure stripping, while #89415 captured the Telegram leak; I did not run a live channel reproduction.

Review metrics: 2 noteworthy metrics.

  • Public Sanitizer Behavior: 1 profile added, 1 delivery-default strip stage added. The touched sanitizer is exported through plugin-sdk/text-chunking, so the change affects more than the Discord implementation.
  • Patch Surface: 8 files changed, +245/-76. The diff spans shared text sanitization, generic reply normalization, Discord delivery, and tests, so both channel and SDK behavior need review.

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🐚 platinum hermit
Patch quality: 🦐 gold shrimp
Result: needs maintainer review before merge.

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

Rank-up moves:

  • [P2] Preserve or genericize policy-generated mutating failure warnings before merge.
  • [P2] Add a focused regression test for a successful assistant reply plus a failed mutating native tool warning.
  • Capture Telegram proof for the related gh-search compact trace path.

Mantis proof suggestion
A Telegram Desktop run would materially prove that the related #89415 compact gh-search trace no longer posts a standalone bot message while the final answer still appears. A maintainer can ask Mantis to capture proof by posting a new PR comment that starts with the OpenClaw Mantis account mention, followed by:

telegram desktop proof for this PR using a Codex-backed Telegram session that produces a compact gh search "(agent) failed" trace; record that no standalone failure message is delivered while the final answer remains visible, with private identifiers redacted.

Risk before merge

  • [P1] The shared delivery sanitizer is a public Plugin SDK behavior path, so existing channel or plugin callers may lose lines matching the internal-trace regex unless maintainers accept that compatibility break.
  • [P1] The related Telegram regression has source and live-report evidence, but this PR still lacks after-fix Telegram Desktop proof for the exact gh-search leakage path.

Maintainer options:

  1. Preserve Safe Failure Warnings (recommended)
    Before merge, keep policy-generated mutating tool failures visible as generic safe copy while removing the raw command/path trace detail.
  2. Accept Full Suppression
    Maintainers can intentionally accept that compact native failure warnings are dropped even when they were the only indication that a mutating action failed.
  3. Pause For Narrower Patch
    If the warning-policy change is not intended, pause this branch and replace it with a narrower non-terminal/internal-only suppression patch.

Next step before merge

  • Maintainers should decide or patch how to preserve safe mutating-failure warnings while removing raw internal command traces.

Security
Cleared: No workflow, dependency, lockfile, secret-handling, or supply-chain execution surface changes were found; the security-relevant intent is reducing trace leakage.

Review findings

  • [P1] Preserve mutating failure warnings — src/agents/embedded-agent-helpers/sanitize-user-facing-text.ts:418-420
Review details

Best possible solution:

Land only after the raw command/path trace is stripped without deleting the existing mutating-tool warning contract, with focused Discord and generic-channel coverage for the safe replacement behavior.

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

Yes from source and related live reports: current main lacks shared compact failure stripping, while #89415 captured the Telegram leak; I did not run a live channel reproduction.

Is this the best way to solve the issue?

No as written: centralizing trace stripping is the right direction, but the patch should preserve a safe user warning for policy-visible mutating failures rather than dropping the payload completely.

Full review comments:

  • [P1] Preserve mutating failure warnings — src/agents/embedded-agent-helpers/sanitize-user-facing-text.ts:418-420
    This now strips every error-context payload that looks like ⚠️ 🛠️ … (agent) failed. That includes warning payloads deliberately generated from lastToolError when a mutating command or file-change fails after an otherwise successful answer, so the user can see success while the only failure signal is silently dropped. Keep the warning contract by replacing the raw trace with safe generic failure copy, or restrict full suppression to metadata-marked non-terminal/internal-only warnings.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.78

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live Discord archive rows for the leak plus local after-patch sanitizer output showing the text-only failure payload is dropped, though not a deployed gateway run.
  • add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (logs): The PR body includes redacted live Discord archive rows for the leak plus local after-patch sanitizer output showing the text-only failure payload is dropped, though not a deployed gateway run.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The shared sanitizer is intended to cover the related Telegram gh-search leak, which is visible in chat and suitable for a short Telegram Desktop proof.
  • remove rating: 🌊 off-meta tidepool: Current PR rating is rating: 🦐 gold shrimp, so this older rating label is no longer current.

Label justifications:

  • P1: The PR addresses live user-visible agent/tool trace leakage in chat channels and can also affect whether failure messages are delivered.
  • merge-risk: 🚨 compatibility: Changing the public shared delivery sanitizer can alter existing plugin or channel callers that already use sanitizeAssistantVisibleText.
  • merge-risk: 🚨 message-delivery: The current patch can suppress a whole error payload after sanitization, including warnings that existing payload policy intended to send.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🐚 platinum hermit and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (logs): The PR body includes redacted live Discord archive rows for the leak plus local after-patch sanitizer output showing the text-only failure payload is dropped, though not a deployed gateway run.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live Discord archive rows for the leak plus local after-patch sanitizer output showing the text-only failure payload is dropped, though not a deployed gateway run.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The shared sanitizer is intended to cover the related Telegram gh-search leak, which is visible in chat and suitable for a short Telegram Desktop proof.
Evidence reviewed

PR surface:

Source +70, Tests +99. Total +169 across 8 files.

View PR surface stats
Area Files Added Removed Net
Source 4 116 46 +70
Tests 4 129 30 +99
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 8 245 76 +169

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/shared/text/assistant-visible-text.test.ts src/agents/embedded-agent-helpers.sanitizeuserfacingtext.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/monitor/message-handler.process.test.ts.
  • [P1] node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/payloads.errors.test.ts src/auto-reply/reply/followup-runner.test.ts -t "tool progress".
  • [P1] git diff --check.
  • [P1] Telegram Desktop proof for a Codex-backed compact gh-search '(agent) failed' trace.

What I checked:

  • Root review policy read: Root AGENTS.md requires full scoped review, dependency/Codex source inspection, and real behavior proof for channel-visible changes; those rules were applied. (AGENTS.md:1, 388dc56ba572)
  • Scoped plugin policy read: extensions/AGENTS.md treats bundled plugins as the same boundary third-party plugins see and requires SDK/public-barrel imports for plugin code. (extensions/AGENTS.md:1, 388dc56ba572)
  • Scoped SDK policy read: src/plugin-sdk/AGENTS.md says Plugin SDK changes can affect third-party plugins and that public SDK entrypoints are compatibility-sensitive. (src/plugin-sdk/AGENTS.md:1, 388dc56ba572)
  • Telegram maintainer note checked: Telegram maintainer notes require real Telegram proof for Telegram behavior touching transport, streaming, topics, callbacks, authorization, or reply context; the related issue is a Telegram-visible leak. (.agents/maintainer-notes/telegram.md:1, 388dc56ba572)
  • Shared sanitizer change: The PR adds internal trace-line regexes and strips them in the default delivery profile, so the behavior moves from Discord-local filtering into a shared sanitizer exported through plugin-sdk/text-chunking. (src/shared/text/assistant-visible-text.ts:773, 84905d99da26)
  • Generic route change: The PR strips internal trace lines from every sanitizeUserFacingText call with errorContext=true, which is the generic route used before non-Discord channel delivery. (src/agents/embedded-agent-helpers/sanitize-user-facing-text.ts:418, 84905d99da26)

Likely related people:

  • Peter Steinberger: Git history attributes the shared assistant-visible sanitizer profile work and current main blame for src/shared/text/assistant-visible-text.ts to Peter, including the profile unification commit. (role: introduced and recent area contributor; confidence: high; commits: 712479eea1a1, 5d6216a7f12f; files: src/shared/text/assistant-visible-text.ts, src/agents/embedded-agent-runner/run/payloads.ts, src/auto-reply/tool-meta.ts)
  • obviyus: The latest PR commits add the gh-search trace coverage and tool-progress sanitizer profile on top of the contributor branch, and the PR timeline assigns obviyus. (role: recent PR fixup author and assignee; confidence: medium; commits: 1a40edfe29ee, 84905d99da26; files: src/shared/text/assistant-visible-text.ts, extensions/discord/src/monitor/reply-safety.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: 🧂 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. labels Jun 1, 2026
@fuller-stack-dev

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

Addressed the code finding in follow-up commit 63376624dd:

  • restored Discord send-boundary suppression for plain analysis:, commentary:, thinking:, and reasoning: lines;
  • restored Discord delivery tests that prove those labels scrub empty while media/channelData/presentation payloads still send;
  • kept the shared sanitizer narrower so ordinary user-visible Analysis: headings remain preserved outside the Discord-specific guard.

Local proof:

  • node scripts/run-vitest.mjs src/shared/text/assistant-visible-text.test.ts src/agents/embedded-agent-helpers.sanitizeuserfacingtext.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/monitor/message-handler.process.test.ts
  • node scripts/run-vitest.mjs src/auto-reply/reply/followup-runner.test.ts -t "tool progress"
  • git diff --check

The PR body now also calls out the remaining live Discord proof gap separately.

@clawsweeper

clawsweeper Bot commented Jun 1, 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 P1 High-priority user-facing bug, regression, or broken workflow. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 1, 2026
@fuller-stack-dev

fuller-stack-dev commented Jun 1, 2026

Copy link
Copy Markdown
Member Author

@clawsweeper re-review

Added redacted real behavior proof to the PR body:

  • copied live Discord archive output from a redacted channel showing the actual leaked ⚠️ 🛠️ ... failed messages;
  • direct post-patch execution of sanitizeDiscordFrontChannelReplyPayloads() with the same leaked text shape, showing the text-only failure payload is dropped and the media payload survives with internal commentary:/analysis: text stripped.

I also left the remaining gap explicit: this was not a deployed live Discord gateway run of this exact branch. Public Discord channel names, author IDs, guild IDs, channel IDs, and message IDs are redacted.

@clawsweeper

clawsweeper Bot commented Jun 1, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: mock-only-proof Candidate: PR proof only shows tests, mocks, snapshots, lint, typecheck, or CI. labels Jun 1, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. 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. and removed 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. labels Jun 1, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 1, 2026
@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 1, 2026
@brokemac79

Copy link
Copy Markdown
Contributor

Hi - I raised issue #89415 but clawsweeper closed it as duplicate. My codex felt this PR in it's current state would not resolve the gh search issue that was one of the issues i encountered. @fuller-stack-dev - could you maybe look at the ticket I mentioned and check / amend your PR to account for this case if needed? Thanks

@obviyus obviyus self-assigned this Jun 2, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 2, 2026
@clawsweeper clawsweeper Bot removed the rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. label Jun 2, 2026
@clawsweeper clawsweeper Bot added rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. and removed status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Jun 2, 2026
@clawsweeper

clawsweeper Bot commented Jun 2, 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.

Re-review progress:

@clawsweeper clawsweeper Bot added 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. and removed rating: 🌊 off-meta tidepool PR readiness rating does not apply to this item. labels Jun 2, 2026
@obviyus
obviyus force-pushed the codex/agent-failed-message-suppression branch from 84905d9 to 0ffedba Compare June 3, 2026 02:33
@openclaw-barnacle openclaw-barnacle Bot removed the proof: sufficient ClawSweeper judged the real behavior proof convincing. label Jun 3, 2026
@obviyus
obviyus merged commit 8f1ae59 into openclaw:main Jun 3, 2026
158 of 159 checks passed
@obviyus

obviyus commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Landed via rebase onto main.

  • Scoped tests: node scripts/run-vitest.mjs src/shared/text/assistant-visible-text.test.ts src/agents/embedded-agent-helpers.sanitizeuserfacingtext.test.ts extensions/discord/src/monitor/reply-delivery.test.ts extensions/discord/src/monitor/message-handler.process.test.ts; node scripts/run-vitest.mjs src/auto-reply/reply/followup-runner.test.ts -t "tool progress"; git diff --check
  • Changelog: not edited; release-note context is in the PR body and commit subjects.
  • Waived gates: earlier OpenGrep PR Diff and Mantis Telegram proof failures treated as non-blocking by maintainer before merge; post-rebase checks were pending at merge time.
  • Land commit: 0ffedbaf32e9f2e355f69d493651d7d69a8c4ac8
  • Merge commit: 8f1ae5967ef27e251e361622259db6faea6c21b0

Thanks @fuller-stack-dev!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling channel: discord Channel integration: discord 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. P1 High-priority user-facing bug, regression, or broken workflow. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: M status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants