Skip to content

fix(telegram): keep tool progress after non-final commentary#90997

Open
wsyjh8 wants to merge 1 commit into
openclaw:mainfrom
wsyjh8:fix/telegram-90962-commentary-tool-progress
Open

fix(telegram): keep tool progress after non-final commentary#90997
wsyjh8 wants to merge 1 commit into
openclaw:mainfrom
wsyjh8:fix/telegram-90962-commentary-tool-progress

Conversation

@wsyjh8

@wsyjh8 wsyjh8 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Summary

In non-persist progress streaming mode with streaming.progress.commentary
enabled, Telegram drops the tool-progress lines that follow inter-tool
commentary: the live progress draft shows the model's "Let me check X…"
commentary, but the subsequent 📖 Read … / tool lines never appear. Every
other streaming channel keeps tool progress and commentary coexisting.

Root cause

Non-final inter-tool commentary suppressed the progress draft via
prepareAnswerLaneForText. In progress mode commentary arrives as assistant
partial text (onPartialReply → ingestDraftLaneSegments); for an answer-lane
segment that path calls prepareAnswerLaneForText →
rotateAnswerLaneAfterToolProgress, which clear()s the draft and calls
suppressProgressDraftState(). The next tool's pushStreamToolProgress is then
dropped inside the compositor on the progressSuppressed guard, so the
tool-progress lines after commentary vanish.

(For precision: the lane is not "finalized" — rotateAnswerLaneAfterToolProgress
resets finalized to false and suppresses the compositor; the drop is the
progressSuppressed guard, not the finalized guard.)

Fix

In progress mode, when commentary is enabled, route non-final answer-lane
partial text into the shared progress draft's commentary lane
(progressDraft.pushCommentaryProgress, keyed per assistant message) so
commentary and tool progress accumulate together in one open draft instead of
tearing the lane down. This reuses the same commentary lane the onItemEvent
preamble path already uses.

Deliberately unchanged:

  • The pushStreamToolProgress finalized / final-delivery guard — it is correct
    for real final delivery and is not relaxed.
  • The final-answer cleanup path — the draft still rotates into its own final
    message on completion.
  • No new config field or accessor; the branch is gated on the existing
    progressDraft.commentaryProgressEnabled.

Trigger conditions / scope

Reproduces only when all three hold within one turn:

  1. streaming.progress.commentary enabled (default: off)
  2. non-persist progress streaming mode
  3. inter-tool commentary and tool calls interleave in the turn

Default config is unaffected. The diverting branch is gated on commentary
being enabled; with commentary off, the answer-lane path is unchanged from main
(prepareAnswerLaneForText), so users on defaults see no behavior change.

Testing

  • New dispatch regression in bot-message-dispatch.test.ts drives the
    commentary → tool → commentary → tool interleave and asserts every
    tool-progress line (and the commentary) stays in the draft. It fails on main
    (the second tool line is dropped) and passes with this change.
  • Green locally: extensions/telegram/src/bot-message-dispatch.test.ts,
    src/plugin-sdk/channel-streaming.test.ts,
    extensions/discord/src/monitor/message-handler.process.test.ts; plus
    tsgo:extensions (+ :test), lint:extensions, and oxfmt --check clean.
  • Observed a pre-existing, unrelated failure in the prompt-context-cache test
    under Windows (/tmp path resolution); it reproduces on clean main and is
    out of scope for this PR.
  • Live Telegram Desktop capture to follow once the fix(cli): bridge inter-tool commentary events to channel progress #90883 producer path lands —
    that's what surfaces real inter-tool commentary to reproduce this end to end.

Relation to #90883

This is the Telegram consumer-side fix and is independent of #90883 (the
commentary producer). It should land first: once #90883 merges, every
user with commentary + non-persist progress would immediately hit this clobber,
so shipping this guard ahead of it prevents the regression.

Fixes #90962

@wsyjh8
fix(telegram): keep tool progress after non-final commentary
4b42898 
Root cause: non-final inter-tool commentary suppressed the progress draft via
prepareAnswerLaneForText. In non-persist progress streaming mode (with
streaming.progress.commentary enabled) commentary arrives as assistant partial
text (onPartialReply -> ingestDraftLaneSegments), whose answer-lane rotation in
rotateAnswerLaneAfterToolProgress called suppressProgressDraftState. The next
tool's pushStreamToolProgress was then dropped on the compositor's
progressSuppressed guard, so the tool-progress lines after commentary vanished.

Fix: in progress mode, route non-final answer-lane partial text into the shared
progress draft's commentary lane (progressDraft.pushCommentaryProgress, keyed
per assistant message) so commentary and tool progress accumulate in one open
draft instead of tearing the lane down. This reuses the same commentary lane the
onItemEvent preamble path already uses. The pushStreamToolProgress finalized /
final-delivery guard and the final-answer cleanup path are left unchanged, and
no new config is added.

Adds a dispatch regression covering the commentary -> tool -> commentary -> tool
interleave that asserts every tool-progress line stays in the draft.

This is the Telegram consumer-side fix, independent of openclaw#90883 (the commentary
producer); it should land first so enabling commentary + non-persist progress
does not immediately regress.

Fixes openclaw#90962

Signed-off-by: Jason Yao <wsyjh8@gmail.com>
@openclaw-barnacle openclaw-barnacle Bot added channel: telegram Channel integration: telegram size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 6, 2026
@clawsweeper

clawsweeper Bot commented Jun 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 6, 2026, 3:45 PM ET / 19:45 UTC.

Summary
The PR routes opted-in Telegram progress-mode answer partials into the shared commentary progress lane and adds a regression test for commentary-to-tool interleaving.

PR surface: Source +17, Tests +48. Total +65 across 2 files.

Reproducibility: yes. for source-level reproduction, but not live reproduction here. Current main suppresses the progress compositor after answer-lane partial text, and the compositor then drops subsequent tool progress on the progressSuppressed guard.

Review metrics: none identified.

Merge readiness
Overall: 🦪 silver shellfish
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 live Telegram proof showing commentary and the later tool-progress line in the same progress draft after this patch.
  • Include the exact real setup or command used after the patch; redact private details such as phone numbers, API keys, IP addresses, and non-public endpoints.

Proof guidance:

  • [P1] Needs real behavior proof before merge: No after-fix Telegram proof is present; add a redacted Telegram Desktop recording, bot-to-bot transcript, terminal/live output, or logs showing the draft behavior, then update the PR body to trigger a fresh review.

Mantis proof suggestion
A native Telegram Desktop proof would directly show the visible draft-editing behavior that the unit test cannot prove. 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: verify progress-mode commentary and the subsequent tool-progress line stay together in one live Telegram draft.

Risk before merge

  • [P1] No after-fix real Telegram proof is present yet, so unit tests do not settle whether native Telegram draft editing shows commentary and later tool progress together in a live chat.
  • [P1] The end-to-end trigger is tied to the related open producer-side PR, so current evidence is source and synthetic dispatcher proof rather than a real combined user path.

Maintainer options:

  1. Require live Telegram proof (recommended)
    Ask for a redacted Telegram Desktop recording, bot-to-bot transcript, live output, or logs showing the after-fix progress draft keeps both commentary and the later tool-progress line.
  2. Accept synthetic-only evidence
    Maintainers could intentionally merge on the source-level and unit-test evidence, accepting that the live Telegram draft behavior remains unverified.
  3. Pause until producer path is provable
    If the producer-side path is still needed for a faithful live scenario, hold this PR until a stacked proof or landed producer path is available.

Next step before merge

  • [P1] The remaining blocker is contributor or maintainer live Telegram proof, not a narrow code repair ClawSweeper can safely perform.

Security
Cleared: The diff only changes Telegram TypeScript dispatch/test code and adds no dependency, workflow, secret, package-resolution, or supply-chain surface.

Review details

Best possible solution:

Land only after a redacted Telegram Desktop or equivalent live Telegram probe confirms commentary and subsequent tool progress remain together in the progress draft, with the focused regression test kept.

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

Yes for source-level reproduction, but not live reproduction here. Current main suppresses the progress compositor after answer-lane partial text, and the compositor then drops subsequent tool progress on the progressSuppressed guard.

Is this the best way to solve the issue?

Yes, this looks like the narrow owner-boundary fix: it keeps final-delivery guards intact and reuses the existing progress commentary compositor. The missing piece is live Telegram proof, not a different code shape.

AGENTS.md: found and applied where relevant.

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

Label changes

Label changes:

  • add P2: This is a limited-blast-radius Telegram streaming bug in optional progress commentary mode, not a core outage or security issue.
  • add merge-risk: 🚨 message-delivery: The diff changes visible Telegram progress draft delivery, and live transport proof is still missing for the edited-message behavior.
  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🧂 unranked krab and patch quality is 🐚 platinum hermit.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: No after-fix Telegram proof is present; add a redacted Telegram Desktop recording, bot-to-bot transcript, terminal/live output, or logs showing the draft behavior, then update the PR body to trigger a fresh review.
  • add mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. This changes visible Telegram progress-draft behavior that a short Telegram Desktop proof can demonstrate.

Label justifications:

  • P2: This is a limited-blast-radius Telegram streaming bug in optional progress commentary mode, not a core outage or security issue.
  • merge-risk: 🚨 message-delivery: The diff changes visible Telegram progress draft delivery, and live transport proof is still missing for the edited-message behavior.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; 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: No after-fix Telegram proof is present; add a redacted Telegram Desktop recording, bot-to-bot transcript, terminal/live output, or logs showing the draft behavior, then update the PR body to trigger a fresh review.
  • mantis: telegram-visible-proof: Mantis should capture Telegram visible proof. This changes visible Telegram progress-draft behavior that a short Telegram Desktop proof can demonstrate.
Evidence reviewed

PR surface:

Source +17, Tests +48. Total +65 across 2 files.

View PR surface stats
Area Files Added Removed Net
Source 1 17 0 +17
Tests 1 48 0 +48
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 2 65 0 +65

What I checked:

Likely related people:

  • Vincent Koc: Current shallow blame points the Telegram answer-lane progress code and shared compositor to ec4c79c, and git history shows b5c3379 changed the clear-progress-before-answer behavior that this PR works around. (role: recent area contributor; confidence: medium; commits: ec4c79cb388e, b5c337909713; files: extensions/telegram/src/bot-message-dispatch.ts, src/channels/progress-draft-compositor.ts)
  • Ayaan Zaidi: Commit 41ee6b1 added Telegram progress commentary support across dispatch, tests, docs, config schema, and UI hints, which is the feature path affected here. (role: feature introducer; confidence: high; commits: 41ee6b1dd68c; files: extensions/telegram/src/bot-message-dispatch.ts, extensions/telegram/src/bot-message-dispatch.test.ts, docs/channels/telegram.md)
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: 🦪 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. mantis: telegram-visible-proof Mantis should capture Telegram visible proof. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 6, 2026
@849261680 849261680 mentioned this pull request Jun 6, 2026
Open
3 tasks
@wsyjh8

wsyjh8 commented Jun 6, 2026

Copy link
Copy Markdown
Contributor Author

Correction after a deeper producer-side trace

A closer look at where this fix's inputs actually come from shows the PR's stated
trigger — and its "default config is unaffected" claim — are partly wrong. Details
and a scope question below.

1. Corrected trigger

  • Phase-tagged commentary routes via onItemEvent/preamble → pushCommentaryProgress,
    which already coexists with tool progress — so it is not what this PR fixes.
    (Verified for CLI/fix(cli): bridge inter-tool commentary events to channel progress #90883: read its diff and cherry-picked it onto the fix branch;
    codex extensions/codex/src/app-server/event-projector.ts:482-490: read.)
  • The path this PR actually touches is untagged inter-tool assistant text reaching
    onPartialReply     (non-OpenAI-Responses providers, e.g. Anthropic). This part is
    inferred from code, not yet confirmed with a live run: resolveAssistantMessagePhase
    (src/shared/chat-message-content.ts:70-96) returns undefined without a
    phase/textSignature, so the unconditional if (deliveryPhase === "commentary") return;
    at src/agents/embedded-agent-subscribe.handlers.messages.ts:678 doesn't fire; and
    src/agents/embedded-agent-subscribe.handlers.lifecycle.test.ts:469 shows pre-tool text
    captured as streamed (assistantTexts: ["Initial analysis…"]) for a stopReason:"toolUse"
    message — though that test asserts the turn is marked abandoned.

2. Gating gap (retraction) — confirmed by a dispatcher run

The fix is gated on progressDraft.commentaryProgressEnabled, so it only covers
streaming.progress.commentary: on. The trigger above is commentary-agnostic. Driving the
dispatcher with commentary off and the untagged sequence (onPartialReply text →
onToolStart → onPartialReply text → onItemEvent tool), the draft renders only the
first tool line, then clear()s, and the second tool line is dropped — the branch
falls through to prepareAnswerLaneForText(). So I'm retracting the PR body's
"default config is unaffected / this is the fix": the default (and highest-frequency)
case is not fixed here.

3. Scope question

Before I finish this, could you confirm #90962's intended scope:

  • (a) commentary-on subset only (keep the gate), or
  • (b) all inter-tool partial answer text (drop the gate so the default config is covered)?

Note I haven't confirmed live that a shipped provider actually emits untagged inter-tool text
through onPartialReply in a real turn — it's inferred from the phase resolver + line-678 +
lifecycle.test.ts:469. If in practice nothing reaches this path, that reframes #90962
entirely, so I'd value your read on whether this is a real user-facing path or a latent one.

Going by its title, #71589 ("suppress tool-use assistant text", open) may point the
opposite way — suppressing the inter-tool text this PR surfaces. I haven't read its diff;
flagging in case surface-vs-suppress is unreconciled.

4. Next step

Once you confirm direction I'll adjust accordingly (drop/keep the gate per (a)/(b)) and add
an extracted-unit before/after proof driven by the real Anthropic onPartialReply event
shape, rather than re-skinning the existing unit test.

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

channel: telegram Channel integration: telegram mantis: telegram-visible-proof Mantis should capture Telegram visible proof. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Telegram: inter-tool commentary clobbers tool progress in non-persist progress mode (diverges from other streaming channels)

1 participant

@wsyjh8