Skip to content

fix(agents): suppress raw subagent tool output#80110

Closed
steipete wants to merge 2 commits into
mainfrom
maint/subagent-no-raw-output
Closed

fix(agents): suppress raw subagent tool output#80110
steipete wants to merge 2 commits into
mainfrom
maint/subagent-no-raw-output

Conversation

@steipete

@steipete steipete commented May 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This keeps the correct core fix from #80049: selectSubagentOutputText() must not fall through to snapshot.latestRawText when a child subagent produced only tool/toolResult output. Tool output is not user-facing completion text, so the requester session should receive (no output) or a post-compaction assistant reply instead of raw command/search output.

The replacement also fixes the gaps in #80049:

  • updates both public docs pages that described sanitized tool/toolResult fallback
  • adds direct regression coverage for readSubagentOutput()
  • strengthens announce-format e2e assertions to prove raw tool text is absent
  • adds a changelog entry with contributor and reporter credit
  • includes fresh Crabbox proof from a real Linux Testbox run

Verification

Real behavior proof (required for external PRs)

  • Behavior or issue addressed: Subagent completion announce could select raw child tool/toolResult content as completion text when no assistant-produced text existed. In Telegram this can surface code/search/exec dumps from the child transcript. This PR makes tool-only histories produce no selected completion output, so the existing announce fallback reports (no output) instead of leaking raw tool text.
  • Real environment tested: Blacksmith Testbox via Crabbox, Linux runner, Node 24.13.0, repo branch maint/subagent-no-raw-output synced from this PR checkout.
  • Exact steps or command run after this patch: Ran /Users/steipete/Projects/crabbox/bin/crabbox run --provider blacksmith-testbox --blacksmith-org openclaw --blacksmith-workflow .github/workflows/ci-check-testbox.yml --blacksmith-job check --blacksmith-ref main --idle-timeout 90m --ttl 240m --timing-json --shell -- '<probe tool-only child history>; oxfmt --check; targeted pnpm test ...'. The probe imports src/agents/subagent-announce-output.ts, stubs Gateway chat.history with assistant-empty + toolResult: raw grep output, calls readSubagentOutput("agent:main:subagent:child"), and fails if the result contains raw tool output or returns any non-undefined text.
  • Evidence after fix (screenshot, recording, terminal capture, console output, redacted runtime log, linked artifact, or copied live output): Crabbox/Testbox terminal capture from tbx_01kr83we61rvr3xjxxbtdme3x2, workflow run https://github.com/openclaw/openclaw/actions/runs/25620248580:
{"scenario":"replacement-tool-only-subagent-output","result":null,"leaked":false}
Checking formatting...
All matched files use the correct format.
[test] passed 3 Vitest shards in 15.91s
blacksmith run summary sync=delegated command=1m2.483s total=1m4.837s exit=0

Pre-fix comparison from Crabbox/Testbox tbx_01kr83fbqn923dkp3zja0stp6d:

{"scenario":"prefix-tool-only-subagent-output","result":"raw grep output","leaked":true}
  • Observed result after fix: Tool-only subagent completion history no longer returns raw tool output from readSubagentOutput(). The Crabbox probe result is null/undefined with leaked:false, and the announce-format tests verify requester completion handoff contains (no output) and does not contain the raw tool text.
  • What was not tested: A live Telegram bot DM was not exercised. The tested path covers the runtime selector that fed raw child tool text into subagent completion announces; the separate duplicate parent progress symptom in [Bug] Subagent announce leaks raw tool output to Telegram and duplicates parent progress reply #79986 is not claimed fixed by this PR.
  • Before evidence (optional but encouraged): Pre-fix Crabbox/Testbox probe returned raw tool output with leaked:true, shown above.

Root Cause

  • Root cause: The completion-output selector kept a sanitized raw-text fallback for tool-only child transcripts, even though tool/toolResult content is not assistant-authored completion text.
  • Missing detection / guardrail: Existing e2e coverage checked formatted completion output but did not directly assert that readSubagentOutput() rejects tool-only histories or that post-compaction assistant text still wins over raw tool text.
  • Contributing context (if known): Public docs still documented tool/toolResult fallback behavior, so the selector and docs drifted together.

Regression Test Plan

  • Coverage level that should have caught this: Unit test and seam/e2e announce-format test.
  • Target test or file: src/agents/subagent-announce-output.test.ts and src/agents/subagent-announce.format.e2e.test.ts.
  • Scenario the test should lock in: Tool-only child histories produce no selected completion text; post-compaction assistant replies still produce selected text; requester handoff does not include raw tool output.
  • Why this is the smallest reliable guardrail: The bug is in the selector seam, with one e2e announce-format check to prove the user-visible fallback text.
  • Existing test that already covers this (if any): None covered the selector directly before this patch.
  • If no new test is added, why not: New tests are added.

User-visible / Behavior Changes

Subagent completion announces no longer surface raw tool output when the child produced no assistant text. They now use the existing (no output) fallback for that case.

Diagram

N/A

@openclaw-barnacle openclaw-barnacle Bot added docs Improvements or additions to documentation agents Agent runtime and tooling size: S maintainer Maintainer-authored PR labels May 10, 2026
@steipete steipete mentioned this pull request May 10, 2026
Closed
3 tasks
@clawsweeper

clawsweeper Bot commented May 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: found issues before merge. Reviewed July 4, 2026, 12:02 AM ET / 04:02 UTC.

Summary
The PR removes the raw tool/toolResult fallback from subagent completion output selection, updates subagent docs and regression tests, and adds a changelog entry.

PR surface: Source +5, Tests +36, Docs +1. Total +42 across 6 files.

Reproducibility: yes. The old failure is source-reproducible with an empty assistant turn followed by tool/toolResult-only history; current main now has focused tests asserting raw tool text is not selected or announced.

Review metrics: 1 noteworthy metric.

  • Release-owned changelog entry: 1 added. The PR edits CHANGELOG.md, which release generation owns, so maintainers should notice it before deciding whether to refresh or close the branch.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #79986
Summary: This PR is a candidate fix for the closed Telegram/subagent raw-output leak report; current main already ships the central behavior, while several older reports and PRs cover the same raw subagent output family.

Members:

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

Merge readiness
Overall: 🦐 gold shrimp
Proof: 🦞 diamond lobster ✨ media proof bonus
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:

  • Maintainer should close this branch as superseded or refresh it against current main.
  • [P2] If refreshed, remove the CHANGELOG.md entry before merge review.

Risk before merge

  • [P1] The branch is live-conflicting and maintainerCanModify=false, so maintainers cannot land it without author-side work, maintainer replacement, or manual conflict resolution.
  • [P1] The PR carries a normal PR-authored CHANGELOG.md entry even though release generation owns changelog entries and current release notes already cover the linked report.
  • [P1] Current main and v2026.6.11 already ship the central no-raw-tool-output behavior, so any refresh must preserve newer subagent completion semantics instead of replaying the older branch wholesale.

Maintainer options:

  1. Manually close as superseded (recommended)
    Because current main and v2026.6.11 already contain the central behavior, a maintainer can close this conflicting branch after confirming no unique credit or test content still needs preservation.
  2. Refresh before merge
    If maintainers want this PR lineage, refresh the branch against current main, resolve conflicts, remove the release-owned changelog entry, and rerun focused subagent announce validation.

Next step before merge

  • [P2] Manual maintainer disposition is needed because the protected label prevents cleanup and the live branch is conflicting with maintainerCanModify=false.

Security
Cleared: The diff changes subagent output selection, docs, tests, and changelog text without adding dependencies, workflow execution, permissions, secrets handling, or new supply-chain surface.

Review findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:20
Review details

Best possible solution:

Keep the shipped current-main implementation; maintainers should manually close this branch as superseded or refresh it only if preserving the PR lineage, resolving conflicts and dropping the changelog edit first.

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

Yes. The old failure is source-reproducible with an empty assistant turn followed by tool/toolResult-only history; current main now has focused tests asserting raw tool text is not selected or announced.

Is this the best way to solve the issue?

Yes for the core behavior: refusing to promote raw tool output is the right owner-boundary fix. The submitted branch is no longer the best landing vehicle because current main already ships the behavior and the live PR is conflicting with a release-owned changelog edit.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:20
    CHANGELOG.md is release-generated, and normal PRs should keep release-note context in the PR body or commit message. This added entry conflicts with the repository's release ownership and should be dropped before any branch refresh.
    Confidence: 0.9

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P3: The useful bug-fix behavior is already shipped, leaving low-risk maintainer cleanup of a stale conflicting branch.
  • merge-risk: 🚨 other: The remaining merge risk is stale-branch and release-owned changelog disposition that normal CI does not resolve.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (screenshot): The PR body includes before/after Crabbox terminal output for the selector path, and Mantis supplied Telegram Desktop screenshot/artifact proof at the PR head; no contributor proof action is needed.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes before/after Crabbox terminal output for the selector path, and Mantis supplied Telegram Desktop screenshot/artifact proof at the PR head; no contributor proof action is needed.
  • proof: 📸 screenshot: Contributor real behavior proof includes screenshot evidence. The PR body includes before/after Crabbox terminal output for the selector path, and Mantis supplied Telegram Desktop screenshot/artifact proof at the PR head; no contributor proof action is needed.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. The PR changes Telegram-visible subagent completion output by preventing raw child tool text from reaching chat completion messages.
Evidence reviewed

PR surface:

Source +5, Tests +36, Docs +1. Total +42 across 6 files.

View PR surface stats
Area Files Added Removed Net
Source 1 6 1 +5
Tests 2 40 4 +36
Docs 3 4 3 +1
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 6 50 8 +42

What I checked:

Likely related people:

  • steipete: Recent history shows Peter Steinberger touched the affected subagent announce helper/docs path, and earlier history added assistant/tool output handling for subagent completion. (role: recent area contributor and PR author with current-main history; confidence: high; commits: 589b1f6aec69, 05e31bbeddd0, 81db05962752; files: src/agents/subagent-announce-output.ts, src/agents/subagent-announce.ts, docs/tools/subagents.md)
  • vincentkoc: Recent current-main history changed subagent completion handoff/delivery paths, and the v2026.6.11 release commit carries the shipped selector/docs/test state reviewed here. (role: recent adjacent owner and release committer; confidence: high; commits: 68a1e00b73bd, 7fc4bbc0bcba, e085fa1a3ffd; files: src/agents/subagent-announce.ts, src/agents/subagent-announce-delivery.ts, src/agents/subagent-announce-output.ts)
  • tyler6204: CONTRIBUTING.md lists Tyler Yust for Agents/subagents, and older history shows subagent announce and read-tool overflow work around this behavior family. (role: listed agents/subagents area owner and earlier announce contributor; confidence: medium; commits: 087dca8fa9f5, f918b336d11a; files: src/agents/subagent-announce-output.ts, src/agents/subagent-announce.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 the proof: sufficient ClawSweeper judged the real behavior proof convincing. label May 10, 2026
This was referenced May 10, 2026
Closed
Closed
Closed
Closed
Closed
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request May 19, 2026
Open
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. labels Jun 14, 2026
@clawsweeper
clawsweeper Bot temporarily deployed to qa-live-shared June 14, 2026 21:22 Inactive
@openclaw-mantis

openclaw-mantis Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Mantis Telegram Desktop Proof

Summary: Mantis captured native Telegram Desktop before/after GIFs showing tool-only subagent completion no longer surfaces raw tool output.

Main screenshot This PR screenshot
Baseline native Telegram Desktop screenshot Candidate native Telegram Desktop screenshot
Main This PR
Baseline native Telegram Desktop proof GIF Candidate native Telegram Desktop proof GIF

Motion-trimmed clips:

Raw QA files: https://artifacts.openclaw.ai/mantis/telegram-desktop/pr-80110/run-27512405187-1/index.json

@clawsweeper clawsweeper Bot added proof: 🎥 video Contributor real behavior proof includes video or recording evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jun 15, 2026
@clawsweeper clawsweeper Bot mentioned this pull request Jun 20, 2026
Closed
@clawsweeper clawsweeper Bot added merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. proof: 🎥 video Contributor real behavior proof includes video or recording evidence. and removed proof: 🎥 video Contributor real behavior proof includes video or recording evidence. labels Jun 20, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. and removed proof: 🎥 video Contributor real behavior proof includes video or recording evidence. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jun 24, 2026
@steipete

steipete commented Jul 5, 2026

Copy link
Copy Markdown
Contributor Author

Closing as superseded by current main. Commit f4b92f5e6c7 removed the raw completion fallback, and current regression tests cover trailing tool output plus tool-only histories. This conflicted branch has no remaining unique fix.

@steipete steipete closed this Jul 5, 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 docs Improvements or additions to documentation maintainer Maintainer-authored PR mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P3 Low-priority cleanup, docs, polish, ergonomics, or speculative work. proof: 📸 screenshot Contributor real behavior proof includes screenshot evidence. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. size: S status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug] Subagent announce leaks raw tool output to Telegram and duplicates parent progress reply

1 participant

@steipete