fix(agents): suppress commentary-phase output leaks by mbelinky · Pull Request #61282 · openclaw/openclaw

mbelinky · 2026-04-05T10:09:30Z

Summary

Problem: assistant messages carrying phase:"commentary" were already being preserved by the OpenAI Responses conversion path, but the embedded subscribe delivery bridge still treated them like normal user-visible assistant output.
Why it matters: internal planning shards like Need send... or Need update tracker... could leak into Telegram partials and final reply paths before the real answer.
What changed: suppress commentary-phase assistant output at the shared embedded subscribe boundary, whether the phase arrives as message.phase or only inside textSignature metadata.
Hardening: commentary text_* updates now bail out before mutating visible-output buffers, partial-reply state, or block-reply flush paths.

Root Cause

Root cause: src/agents/pi-embedded-subscribe.handlers.messages.ts ignored assistant delivery phase when deciding what to stream or flush to the user.
Existing evidence in code: phase metadata is already preserved upstream, including the textSignature-only shape covered by src/agents/openai-ws-stream.test.ts.
Failure mode: commentary text could enter onPartialReply, block-reply flush, or final assistant text emission even though it was internal-only output.

Why This Layer

This is the correct production boundary for OpenClaw because it controls the actual user-visible delivery paths.
A Pi-side render/print fix is still reasonable upstream hygiene, but it does not replace fixing the OpenClaw delivery bridge that Telegram and other embedded surfaces actually use here.
Fixing only Telegram lane wiring would be too narrow; the bug is broader than one preview transport.

Red-Team / Hardening Review

Checked whether the fix only covered message.phase: it did not originally, so this PR explicitly covers the textSignature-only commentary shape too.
Checked whether commentary still polluted internal visible-output state even when suppressed: it did originally, so this PR now returns early before mutating partial/block buffers on commentary text_* updates.
Checked whether the change accidentally suppresses legitimate final answers: suppression is limited to phase:"commentary"; final_answer and normal assistant messages still flow unchanged.
Checked whether this belongs in a refactor instead: no. The minimal root-cause fix is small and local; broader dedupe with other phase helpers can wait.

Change Type

Scope

User-visible / Behavior Changes

Commentary-phase planning text no longer leaks into user-visible assistant partials or final replies.
This applies both to direct message.phase commentary and to replay/live shapes that only carry the phase in textSignature metadata.

Tests

Targeted serial verification on mb-air:

bunx vitest run \
  src/agents/pi-embedded-subscribe.handlers.messages.test.ts \
  src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.suppresses-commentary-phase-output.test.ts \
  src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts \
  --pool forks --maxWorkers 1

Result: 3 files, 38 tests passed.

AI / Testing Notes

AI-assisted: yes
Testing: targeted local serial tests passed

greptile-apps · 2026-04-05T10:13:17Z

Greptile Summary

This PR fixes commentary-phase assistant output leaking into user-visible delivery paths (onPartialReply, block-reply flush, Telegram partials) by adding shouldSuppressAssistantVisibleOutput at the shared embedded subscribe boundary and returning early before any visible-output buffer is mutated. Both the direct message.phase shape and the textSignature-only metadata shape are covered, buffer state is properly cleared via finalizeMessageEnd on the early path, and the new tests cover all affected paths end-to-end.

Confidence Score: 5/5

Safe to merge; the fix is minimal, well-targeted, and comprehensively tested.

The only remaining finding is a P2 observation: handleMessageStart fires the typing-indicator callback unconditionally for commentary messages, causing a spurious typing flash. Teardown is unconditional so there is no stuck-indicator risk. No P0 or P1 issues were found. The suppression logic is correct for both delivery shapes, buffer cleanup is correct, and the test suite covers all affected code paths.

No files require special attention.

Comments Outside Diff (1)

src/agents/pi-embedded-subscribe.handlers.messages.ts, line 214-231 (link)

handleMessageStart fires typing signal for commentary messages

onAssistantMessageStart is called unconditionally for all non-transcript-only assistant messages (line 230), including commentary-phase ones. In production this triggers typingSignals.signalMessageStart(), producing a brief typing-indicator flash before the run completes and the finally-block cleanup tears it down. No stuck-indicator risk exists, but a guard here would prevent the spurious flash:

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.messages.ts
Line: 214-231

Comment:
**`handleMessageStart` fires typing signal for commentary messages**

`onAssistantMessageStart` is called unconditionally for all non-transcript-only assistant messages (line 230), including commentary-phase ones. In production this triggers `typingSignals.signalMessageStart()`, producing a brief typing-indicator flash before the run completes and the `finally`-block cleanup tears it down. No stuck-indicator risk exists, but a guard here would prevent the spurious flash:



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

Prompt To Fix All With AI

This is a comment left during a code review.
Path: src/agents/pi-embedded-subscribe.handlers.messages.ts
Line: 214-231

Comment:
**`handleMessageStart` fires typing signal for commentary messages**

`onAssistantMessageStart` is called unconditionally for all non-transcript-only assistant messages (line 230), including commentary-phase ones. In production this triggers `typingSignals.signalMessageStart()`, producing a brief typing-indicator flash before the run completes and the `finally`-block cleanup tears it down. No stuck-indicator risk exists, but a guard here would prevent the spurious flash:

```suggestion
  if (msg?.role !== "assistant" || isTranscriptOnlyOpenClawAssistantMessage(msg)) {
    return;
  }
  if (shouldSuppressAssistantVisibleOutput(msg)) {
    // Reset state for accurate baseline tracking, but skip the typing signal —
    // commentary-phase messages produce no visible output.
    ctx.resetAssistantMessageState(ctx.state.assistantTexts.length);
    return;
  }

  // KNOWN: Resetting at `text_end` is unsafe (late/duplicate end events).
  // ASSUME: `message_start` is the only reliable boundary for "new assistant message begins".
  // Start-of-message is a safer reset point than message_end: some providers
  // may deliver late text_end updates after message_end, which would otherwise
  // re-trigger block replies.
  ctx.resetAssistantMessageState(ctx.state.assistantTexts.length);
  // Use assistant message_start as the earliest "writing" signal for typing.
  void ctx.params.onAssistantMessageStart?.();
```

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

_{Reviews (1): Last reviewed commit: "fix(agents): suppress commentary-phase o..." | Re-trigger Greptile}

mbelinky · 2026-04-05T11:04:19Z

Merged via squash.

Prepared head SHA: e392904f73ebe0ff03bab728754c53e7ff680367
Merge commit: 4175caee6e955aac7cfe82adb4963212ab284dc9

Thanks @mbelinky!

@mbelinky

Merged via squash. Prepared head SHA: e392904 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky

@mbelinky

Merged via squash. Prepared head SHA: e392904 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky

@mbelinky

Merged via squash. Prepared head SHA: e392904 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky

@mbelinky

Merged via squash. Prepared head SHA: e392904 Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Co-authored-by: mbelinky <132747814+mbelinky@users.noreply.github.com> Reviewed-by: @mbelinky

openclaw-barnacle Bot added agents Agent runtime and tooling size: M maintainer Maintainer-authored PR labels Apr 5, 2026

mbelinky force-pushed the fix/commentary-phase-output-leak branch from 5f72beb to b3f1093 Compare April 5, 2026 11:00

mbelinky added 4 commits April 5, 2026 13:02

fix(agents): suppress commentary-phase output leaks

eb35c1c

test(agents): fix commentary phase test typing

9647b27

docs(changelog): move commentary entry to end of fixes

cfe2e4b

docs(changelog): add attribution for commentary fix

e392904

mbelinky force-pushed the fix/commentary-phase-output-leak branch from b3f1093 to e392904 Compare April 5, 2026 11:03

mbelinky merged commit 4175cae into main Apr 5, 2026
9 checks passed

mbelinky deleted the fix/commentary-phase-output-leak branch April 5, 2026 11:04

github-actions Bot mentioned this pull request Apr 5, 2026

📡 Upstream Digest — 2026-04-05 12:39 UTC curtismercier/openclaw-mods#475

Open

100yenadmin mentioned this pull request Apr 5, 2026

fix(agents): suppress commentary partial visibility and buffer unphased early deltas #61457

Closed

3 tasks

afurm mentioned this pull request Apr 6, 2026

fix(agents): suppress commentary leaks in history-based replies #61747

Closed

19 tasks

tlnini-afk mentioned this pull request Apr 6, 2026

DeepSeek preamble text leaks to Telegram after 3.13 → 4.5 upgrade (untagged assistant text bypasses commentary filter) #62121

Closed

wangwllu mentioned this pull request Apr 12, 2026

Telegram direct main session leaks commentary-phase assistant text to user-visible chat #65276

Closed

clawsweeper Bot mentioned this pull request Apr 29, 2026

[Bug]: Discord early commentary is not committed immediately and can be lost on gateway restart #55036

Closed

clawsweeper Bot mentioned this pull request May 4, 2026

[Bug]: Exposes the format of tool calls, for example:exec{"command": "openclaw status", "description": "Check OpenClaw version and status"} #39626

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(agents): suppress commentary-phase output leaks#61282

fix(agents): suppress commentary-phase output leaks#61282
mbelinky merged 4 commits into
mainfrom
fix/commentary-phase-output-leak

mbelinky commented Apr 5, 2026

Uh oh!

greptile-apps Bot commented Apr 5, 2026 •

edited

Loading

Comments Outside Diff (1)

Uh oh!

Uh oh!

mbelinky commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

mbelinky commented Apr 5, 2026

Summary

Root Cause

Why This Layer

Red-Team / Hardening Review

Change Type

Scope

User-visible / Behavior Changes

Tests

AI / Testing Notes

Uh oh!

greptile-apps Bot commented Apr 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Comments Outside Diff (1)

Uh oh!

Uh oh!

mbelinky commented Apr 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Apr 5, 2026 •

edited

Loading