Skip to content

fix(agents): serialize structured non-image tool-result blocks in extractToolResultText (fixes #97267)#97276

Closed
zw-xysk wants to merge 2 commits into
openclaw:mainfrom
zw-xysk:fix/issue-97267-tool-result-structured-fallback
Closed

fix(agents): serialize structured non-image tool-result blocks in extractToolResultText (fixes #97267)#97276
zw-xysk wants to merge 2 commits into
openclaw:mainfrom
zw-xysk:fix/issue-97267-tool-result-structured-fallback

Conversation

@zw-xysk

@zw-xysk zw-xysk commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Fixes #97267.

extractToolResultText only collected explicit text content blocks from tool results. When a tool result contained only structured non-image blocks (JSON, resource, etc.), it returned undefined — causing the tool result to collapse to an attachment placeholder in the visible transcript.

Changes Made

  • src/agents/embedded-agent-subscribe.tools.ts: Added a structured-block fallback inside extractToolResultText. When no text blocks are present, the fallback serializes structured non-image blocks via JSON.stringify. text and image blocks are skipped in the fallback: images remain handled by media delivery, and text blocks already returned directly.
  • src/agents/embedded-agent-subscribe.tools.test.ts: Added 5 focused tests covering structured fallback, image skip, mixed text+structured separation, and edge cases.

Evidence

  • Unit tests: node scripts/run-vitest.mjs run src/agents/embedded-agent-subscribe.tools.test.ts — 56/56 passed
  • Lint: npx oxlint --tsconfig tsconfig.json src/agents/embedded-agent-subscribe.tools.ts src/agents/embedded-agent-subscribe.tools.test.ts — clean
  • Lockfile: unchanged

Test coverage

Scenario Status
Non-object result → undefined ✅
Explicit text blocks → collected ✅
Structured non-image block → JSON-serialized ✅
Image-only block → undefined (media delivery) ✅
Mixed text + structured → text returned, fallback skipped ✅

What was not tested: Live end-to-end tool subscription path (fix is in the shared extractToolResultText utility, unit-level verification suffices).

AI Assistance 🤖

  • AI-assisted: Yes
  • Co-Authored-By: Claude [email protected]
  • Human confirmed understanding: Yes

@zw-xysk
fix(agents): serialize structured non-image tool-result blocks in ext…
073990e 
…ractToolResultText (openclaw#97267)

extractToolResultText only collected explicit text content blocks.
When a tool result contained only structured non-image blocks
(json, resource, etc.), it returned undefined — causing the
tool result to collapse to an attachment placeholder in the
visible transcript.

Add a fallback that serializes structured non-image blocks via
JSON.stringify when no text blocks are present. Text and image
blocks are skipped in the fallback; images remain handled by
media delivery, and text blocks are returned as before.
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S labels Jun 27, 2026
@clawsweeper

clawsweeper Bot commented Jun 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 27, 2026, 2:44 PM ET / 18:44 UTC.

Summary
The PR adds a fallback in extractToolResultText that serializes structured non-image content blocks when no text blocks exist, plus focused helper tests including truncation coverage.

PR surface: Source +27, Tests +64. Total +91 across 2 files.

Reproducibility: yes. at source level: current main returns undefined for structured-only non-image content blocks, and the visible-output caller emits only when the helper returns text. I did not reproduce the live UI placeholder in a rebuilt OpenClaw session.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/agents/embedded-agent-subscribe.tools.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #97267
Summary: This PR is a candidate fix for the canonical visible structured tool-result text issue; a same-root sibling PR remains open but is not merged or proof-positive.

Members:

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

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🐚 platinum hermit
Result: blocked until 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 rebuilt-session proof showing structured non-image tool output appears as visible text instead of an attachment placeholder.
  • Confirm maintainers want this smaller branch rather than the same-root sibling PR before landing.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body and comments show unit/lint/CI evidence only; add redacted terminal output, logs, screenshots, or a recording from a clean rebuilt or reinstalled session and update the PR body so ClawSweeper can re-review.

Mantis proof suggestion
A short real-session visual or terminal proof would directly show the user-visible transcript behavior this PR changes. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify structured non-image tool-result content appears as visible text instead of an attachment placeholder in an OpenClaw session.

Risk before merge

  • [P1] Real behavior proof is still missing; unit tests, lint, CI, and source inspection do not show a rebuilt OpenClaw session rendering structured-only tool output visibly.
  • [P1] fix(agents): preserve structured tool result visible text #97268 targets the same helper and issue, so maintainers should choose one landing path rather than merging duplicate fixes.

Maintainer options:

  1. Decide the mitigation before merge
    Land one narrow helper-boundary fix after redacted rebuilt-session proof, preferably the smaller branch if maintainers choose it, then let the linked issue close on merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Contributor proof and maintainer branch selection are still needed; there is no narrow automated repair task after the cap fix.

Security
Cleared: Cleared: the diff only changes an agent helper and tests, and it does not touch dependencies, workflows, permissions, credentials, lockfiles, or install paths.

Review details

Best possible solution:

Land one narrow helper-boundary fix after redacted rebuilt-session proof, preferably the smaller branch if maintainers choose it, then let the linked issue close on merge.

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

Yes at source level: current main returns undefined for structured-only non-image content blocks, and the visible-output caller emits only when the helper returns text. I did not reproduce the live UI placeholder in a rebuilt OpenClaw session.

Is this the best way to solve the issue?

Yes for the code shape after the cap follow-up: the helper boundary is narrow and preserves existing text and image behavior. It is not merge-ready until real behavior proof is added and maintainers choose it over the same-root sibling PR.

AGENTS.md: found and applied where relevant.

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

Label changes

Label justifications:

  • P2: This is a normal-priority agent transcript visibility bug fix with limited blast radius, and the remaining blocker is proof rather than an urgent runtime outage.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body and comments show unit/lint/CI evidence only; add redacted terminal output, logs, screenshots, or a recording from a clean rebuilt or reinstalled session and update the PR body so ClawSweeper can re-review.
Evidence reviewed

PR surface:

Source +27, Tests +64. Total +91 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 30 3 +27
Tests 1 64 0 +64
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 94 3 +91

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs src/agents/embedded-agent-subscribe.tools.test.ts.
  • [P1] Redacted rebuilt or reinstalled OpenClaw session proof showing structured-only non-image tool output remains visible.

What I checked:

Likely related people:

  • steipete: gh api and git show identify commit 85ebdf88b0a9 as introducing the shared text-block extraction helper that the current extractor depends on. (role: shared helper introducer; confidence: high; commits: 85ebdf88b0a9, 0d0632d88d0c; files: src/agents/content-blocks.ts)
  • vincentkoc: Recent GitHub path history shows repeated changes to the embedded-agent subscribe helper and handler surfaces around visible delivery and tool-result handling. (role: recent area contributor; confidence: high; commits: 0771ac856379, d75613e794da, fec6a0407f44; files: src/agents/embedded-agent-subscribe.tools.ts, src/agents/embedded-agent-subscribe.handlers.tools.ts)
  • brokemac79: Recent path history shows adjacent helper test coverage in embedded-agent-subscribe.tools.test.ts, the test surface this PR extends. (role: adjacent test contributor; confidence: medium; commits: d1299658ac4b; files: src/agents/embedded-agent-subscribe.tools.test.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. P2 Normal backlog priority with limited blast radius. labels Jun 27, 2026
@zw-xysk

zw-xysk commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

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

@zw-xysk
fix(agents): apply truncation cap to structured tool-result fallback (o…
3c7be77 
…penclaw#97267)

Preserve the existing TOOL_RESULT_MAX_CHARS truncation invariant
in the new structured-block JSON serialization fallback path.
ClawSweeper review noted the path was added without cap coverage.
@clawsweeper clawsweeper Bot mentioned this pull request Jun 27, 2026
Merged
@zw-xysk

zw-xysk commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

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

@snowzlmbot snowzlmbot mentioned this pull request Jun 28, 2026
Closed
@steipete

steipete commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Superseded by #97268, which landed in 8d168c836af38332a75529ca8c1af2cf28494427.

Both PRs address #97267 in extractToolResultText, but the landed implementation covers the actual top-level Claude/Gemini CLI arrays and typed Anthropic result objects in addition to {content:[...]} envelopes. It also preserves canonical error-status handling, caps text and JSON output, and suppresses/redacts structured credentials, auth headers, binary media/document payloads, data URIs, and opaque encrypted fields.

Focused proof on the prepared head passed 159 tests, a direct helper probe, clean final autoreview, and exact-head CI run 28317871184. Closing this same-root alternative so the cluster has one canonical implementation.

@steipete steipete closed this Jun 28, 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 P2 Normal backlog priority with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug(agents): visible tool-result text can collapse to attachment placeholder

2 participants

@zw-xysk @steipete