fix(msteams): reset stream state after tool calls to prevent message loss by SidU · Pull Request #56071 · openclaw/openclaw

SidU · 2026-03-27T23:04:55Z

Summary

Fixes preparePayload() in reply-stream-controller.ts to reset streamReceivedTokens and finalize the stream after suppressing a delivery, so subsequent text segments (after tool calls) use fallback delivery instead of being silently dropped
Adds stream.isFinalized guard so a finalized stream never re-suppresses fallback delivery even if onPartialReply fires again
Adds 5 unit tests covering the multi-segment delivery path (text → tool call → text)

Root Cause

When an agent uses tools mid-response, the LLM produces discontinuous text segments. preparePayload() used a one-shot streamReceivedTokens flag that never reset after the first segment was streamed — so every subsequent deliver() call was suppressed, even for text the stream never saw.

Before / After

Before fix: When an agent uses tools mid-response (text → tool calls → more text), preparePayload() suppresses fallback delivery for ALL text segments because streamReceivedTokens stays true. The second text segment after tool calls is silently lost or the response gets fragmented into duplicate messages.

After fix: preparePayload() resets streamReceivedTokens and finalizes the stream after suppressing the first segment. Subsequent text segments (after tool calls) are delivered via the fallback proactive messaging path. The stream.isFinalized guard ensures this holds even if onPartialReply fires again for post-tool tokens.

Test plan

Unit tests: 5 new tests in reply-stream-controller.test.ts — all pass
Existing tests: reply-dispatcher.test.ts (5) + streaming-message.test.ts (10) — all pass
VM regression: A1 (basic reply), A4 (streaming), A5 (streaming + tool use), E4 (endpoint) — all pass
Full test report

Fixes #56040

🤖 Generated with Claude Code

…very When an agent uses tools mid-response (text → tool calls → more text), the stream controller's preparePayload would suppress fallback delivery for ALL text segments because streamReceivedTokens stayed true. This caused the second text segment to be silently lost or duplicated. Fix: after preparePayload suppresses delivery for a streamed segment, finalize the stream and reset streamReceivedTokens so subsequent segments use fallback delivery. Fixes openclaw#56040 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

greptile-apps · 2026-03-27T23:09:14Z

Greptile Summary

This PR fixes a silent message-loss bug in the MS Teams reply stream controller where streamReceivedTokens was never reset after suppressing the first streamed segment — causing every subsequent deliver() call (post tool-call text segments) to be silently dropped. The fix resets streamReceivedTokens to false and eagerly calls stream.finalize() (which is idempotent) inside preparePayload(), so subsequent segments fall through to fallback delivery. Four unit tests are added that validate the multi-segment path.

Key changes:

preparePayload() in reply-stream-controller.ts now resets streamReceivedTokens = false and calls void stream.finalize() before suppressing a delivery, so any post-tool-call text segment defaults to fallback.
4 new unit tests cover: suppression of first streamed segment, fallback delivery after tool calls, stream finalization on suppression, and media-payload text-stripping.

Concern:

onPartialReply() does not guard against re-setting streamReceivedTokens = true when the stream is already finalized. If the LLM pipeline calls onPartialReply() for a second text segment (e.g. tokens streaming after tool results), the flag is set back to true and, because stream.hasContent stays true (the real TeamsHttpStream never clears accumulatedText), the second segment would be suppressed again — the same bug this PR fixes. The current test and VM test pass because they don't exercise this path, but a one-line guard (|| stream.isFinalized) in onPartialReply() would make the fix airtight.

Confidence Score: 4/5

Safe to merge with low risk, but the unguarded onPartialReply() path could silently re-suppress post-tool-call segments if the streaming pipeline delivers tokens for second segments.

The core fix is correct for the observed production flow (VM-tested), and the 4 new tests validate the intended path. Score reduced from 5 because onPartialReply() can silently re-set the flag on a finalized stream — a present correctness gap that becomes a real defect if the LLM pipeline calls onPartialReply() for second text segments.

extensions/msteams/src/reply-stream-controller.ts — onPartialReply() needs a stream.isFinalized guard to prevent the flag from being spuriously re-set after the stream has been finalized.

Important Files Changed

Filename	Overview
extensions/msteams/src/reply-stream-controller.ts	Adds stream reset logic to `preparePayload()`: resets `streamReceivedTokens` and eagerly calls `finalize()` so second text segments use fallback delivery. Correct for the observed flow but `onPartialReply()` lacks a guard against re-setting the flag on a finalized stream.
extensions/msteams/src/reply-stream-controller.test.ts	New test file with 4 unit tests covering the multi-segment delivery path. Tests validate the intended flow but do not cover the scenario where `onPartialReply()` is called before the second `preparePayload()`.

Comments Outside Diff (1)

extensions/msteams/src/reply-stream-controller.ts, line 48-54 (link)

streamReceivedTokens can be re-set to true by a finalized stream

After preparePayload() resets streamReceivedTokens = false and calls void stream.finalize(), if the LLM subsequently streams tokens for the second text segment through onPartialReply(), the flag is set back to true. Because the real TeamsHttpStream never clears accumulatedText, stream.hasContent remains true from the first segment even after finalization. This means the second preparePayload() call would see streamReceivedTokens = true + stream.hasContent = true and suppress the second segment again — recreating the exact bug this PR is fixing.

The current test (line 53-67) only validates the case where onPartialReply() is not called before the second preparePayload(), which matches the assumed flow. But if the LLM pipeline does deliver streaming tokens for the second segment via onPartialReply(), the fix is incomplete.

Adding a stream.isFinalized guard here is the minimal defensive fix — stream.update() already no-ops after finalization, so the flag should follow suit:

Prompt To Fix With AI

This is a comment left during a code review.
Path: extensions/msteams/src/reply-stream-controller.ts
Line: 48-54

Comment:
**`streamReceivedTokens` can be re-set to `true` by a finalized stream**

After `preparePayload()` resets `streamReceivedTokens = false` and calls `void stream.finalize()`, if the LLM subsequently streams tokens for the second text segment through `onPartialReply()`, the flag is set back to `true`. Because the real `TeamsHttpStream` never clears `accumulatedText`, `stream.hasContent` remains `true` from the first segment even after finalization. This means the second `preparePayload()` call would see `streamReceivedTokens = true` + `stream.hasContent = true` and suppress the second segment again — recreating the exact bug this PR is fixing.

The current test (line 53-67) only validates the case where `onPartialReply()` is **not** called before the second `preparePayload()`, which matches the assumed flow. But if the LLM pipeline does deliver streaming tokens for the second segment via `onPartialReply()`, the fix is incomplete.

Adding a `stream.isFinalized` guard here is the minimal defensive fix — `stream.update()` already no-ops after finalization, so the flag should follow suit:



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: extensions/msteams/src/reply-stream-controller.ts
Line: 48-54

Comment:
**`streamReceivedTokens` can be re-set to `true` by a finalized stream**

After `preparePayload()` resets `streamReceivedTokens = false` and calls `void stream.finalize()`, if the LLM subsequently streams tokens for the second text segment through `onPartialReply()`, the flag is set back to `true`. Because the real `TeamsHttpStream` never clears `accumulatedText`, `stream.hasContent` remains `true` from the first segment even after finalization. This means the second `preparePayload()` call would see `streamReceivedTokens = true` + `stream.hasContent = true` and suppress the second segment again — recreating the exact bug this PR is fixing.

The current test (line 53-67) only validates the case where `onPartialReply()` is **not** called before the second `preparePayload()`, which matches the assumed flow. But if the LLM pipeline does deliver streaming tokens for the second segment via `onPartialReply()`, the fix is incomplete.

Adding a `stream.isFinalized` guard here is the minimal defensive fix — `stream.update()` already no-ops after finalization, so the flag should follow suit:

```suggestion
    onPartialReply(payload: { text?: string }): void {
      if (!stream || !payload.text || stream.isFinalized) {
        return;
      }
      streamReceivedTokens = true;
      stream.update(payload.text);
    },
```

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

_{Reviews (1): Last reviewed commit: "fix(msteams): reset stream state after p..." | Re-trigger Greptile}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 5be1c99104

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

extensions/msteams/src/reply-stream-controller.ts

…ssion When onPartialReply fires after the stream is finalized (post-tool partial tokens), streamReceivedTokens gets set back to true but the stream can't deliver. Add stream.isFinalized check so a finalized stream never suppresses fallback delivery. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6828e56310

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

extensions/msteams/src/reply-stream-controller.ts

Store the fire-and-forget finalize promise from preparePayload and await it in the controller's finalize() method. This ensures markDispatchIdle waits for the in-flight stream finalization to complete before context cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

Add tests for 3+ tool call rounds (text → tool → text → tool → text) and media+text payloads after stream finalization, covering the full contract of preparePayload across all input types and cycle counts. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>

BradGroux

Detailed Review: fix(msteams): reset stream state after tool calls

Problem

When an agent uses tools mid-response, the LLM produces discontinuous text segments (text > tool call > more text). The preparePayload() method used a one-shot streamReceivedTokens flag that was set to true on the first partial reply but never reset. Every subsequent deliver() call after the first segment was silently suppressed because the flag stayed true and the stream still reported hasContent. The second (and any further) text segments were simply lost.

This is a real-world regression for any agent that uses tools during a conversation, which is basically all of them.

Fix Analysis (3 commits)

Commit 1 (5be1c99): Core fix

Resets streamReceivedTokens = false after preparePayload suppresses the first streamed segment
Calls stream.finalize() immediately so the stream is closed and the first segment is delivered as a final message
Subsequent calls to preparePayload now correctly fall through to fallback/proactive delivery because streamReceivedTokens is false

This is the right approach. The stream handled segment one, so finalize it and let the fallback path handle everything after that.

Commit 2 (6828e56): Finalized-stream guard

Adds stream.isFinalized to the guard condition in preparePayload
Prevents a subtle race: if onPartialReply fires again for post-tool tokens (which it does), streamReceivedTokens gets set back to true, but the finalized stream cannot actually deliver anything
Without this guard, the second segment would be suppressed again because streamReceivedTokens and hasContent are both truthy

This directly addresses the Codex bot feedback on commit 1. Good catch and clean fix.

Commit 3 (1531c53): Finalization ordering

Stores the fire-and-forget stream.finalize() promise as pendingFinalize
The controller finalize() method now awaits pendingFinalize before calling stream.finalize() again
This closes a race where markDispatchIdle could tear down the turn context before the stream finalization completes

TeamsHttpStream.finalize() sets this.finalized = true synchronously at the top, so the second call from finalize() is a no-op. No double-send risk.

Edge Cases Verified

Group/channel conversations: stream is undefined (only created for personal chats), so preparePayload returns the payload unmodified. No regression.
Multiple tool boundaries (text > tools > text > tools > text): After the first finalize, every subsequent preparePayload call hits the isFinalized guard and returns the payload for fallback delivery. Works correctly for any number of segments.
Media payloads: The hasMedia branch only executes when the stream is active, has content, and is not finalized (i.e., first segment only). Post-tool media+text payloads pass through fully via fallback.
Finalize idempotency: TeamsHttpStream.finalize() checks this.finalized internally. The double-call path (pendingFinalize then stream?.finalize()) is safe.
No partial tokens before tool call: If the LLM goes straight to tool use without emitting text, streamReceivedTokens stays false and preparePayload returns the payload as-is. Correct.

Test Coverage

Five new tests in reply-stream-controller.test.ts:

Test	What it covers
Suppresses fallback for first streamed segment	Basic streaming delivery path
Allows fallback for second segment after tools	The core bug fix
Finalizes stream when suppressing first segment	Ensures stream closes properly
Fallback works even when onPartialReply fires post-finalize	The commit 2 race condition
Strips text from media payloads when stream handled text	Media+text first-segment behavior

The mocks are clean. They test the controller contract without requiring the full Teams SDK or Bot Framework. The vi.hoisted pattern for tracking stream instances is a nice touch for verifying finalization calls.

VM Regression

The PR body references passing VM tests: A1 (basic reply), A4 (streaming), A5 (streaming + tool use), E4 (endpoint). Full test report linked.

Verdict

Small, surgical, well-tested fix for a real bug that affects every tool-using agent on Teams. The three-commit progression is logical and each commit addresses a distinct concern. No regressions identified. Approved.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31af10ebda

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-03-28T02:36:55Z

extensions/msteams/src/reply-stream-controller.ts

+      streamReceivedTokens = false;
+      pendingFinalize = stream.finalize();


Restrict early stream finalization to segment boundaries

When block streaming is enabled (agentCfg.blockStreamingDefault === "on" in src/auto-reply/reply/get-reply-directives.ts:397-409), onBlockReply can deliver multiple payloads during one assistant turn (src/auto-reply/reply/dispatch-from-config.ts:690-720). Finalizing the Teams stream on the first suppressed payload here marks the stream finalized immediately, so subsequent block payloads bypass suppression and are sent via fallback as separate messages; at the same time, final payloads are commonly dropped after successful block streaming (src/auto-reply/reply/agent-runner-payloads.ts:157-215). In that configuration this regresses from a single coherent streamed message to fragmented/duplicated delivery, so finalization should not happen for every suppressed payload.

Useful? React with 👍 / 👎.

SidU · 2026-03-28T04:57:12Z

Thanks Brad! Added both suggested tests in 31af10e — 3+ segment rounds and media+text after finalization. Also verified the 3+ rounds case on the VM (TCP → fetch → UDP → fetch → ICMP, all segments delivered).

…loss (openclaw#56071) * fix(msteams): reset stream state after preparePayload suppresses delivery When an agent uses tools mid-response (text → tool calls → more text), the stream controller's preparePayload would suppress fallback delivery for ALL text segments because streamReceivedTokens stayed true. This caused the second text segment to be silently lost or duplicated. Fix: after preparePayload suppresses delivery for a streamed segment, finalize the stream and reset streamReceivedTokens so subsequent segments use fallback delivery. Fixes openclaw#56040 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(msteams): guard preparePayload against finalized stream re-suppression When onPartialReply fires after the stream is finalized (post-tool partial tokens), streamReceivedTokens gets set back to true but the stream can't deliver. Add stream.isFinalized check so a finalized stream never suppresses fallback delivery. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix(msteams): await pending finalize in controller to prevent race Store the fire-and-forget finalize promise from preparePayload and await it in the controller's finalize() method. This ensures markDispatchIdle waits for the in-flight stream finalization to complete before context cleanup. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * test(msteams): add edge case tests for multi-round and media payloads Add tests for 3+ tool call rounds (text → tool → text → tool → text) and media+text payloads after stream finalization, covering the full contract of preparePayload across all input types and cycle counts. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>

openclaw-barnacle bot added channel: msteams Channel integration: msteams size: S maintainer Maintainer-authored PR labels Mar 27, 2026

chatgpt-codex-connector bot reviewed Mar 27, 2026

View reviewed changes

extensions/msteams/src/reply-stream-controller.ts Outdated Show resolved Hide resolved

SidU requested review from BradGroux and fabianwilliams March 27, 2026 23:12

SidU self-assigned this Mar 27, 2026

chatgpt-codex-connector bot reviewed Mar 27, 2026

View reviewed changes

extensions/msteams/src/reply-stream-controller.ts Outdated Show resolved Hide resolved

SidU and others added 2 commits March 27, 2026 16:50

BradGroux approved these changes Mar 28, 2026

View reviewed changes

BradGroux merged commit 295d1de into openclaw:main Mar 28, 2026
108 of 126 checks passed

chatgpt-codex-connector bot reviewed Mar 28, 2026

View reviewed changes

BradGroux mentioned this pull request Mar 28, 2026

WORKING: All Microsoft Issues and PRs #49126

Draft

jlian mentioned this pull request Mar 31, 2026

msteams: streaming + block delivery duplicate text when response exceeds 4000 chars #58601

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(msteams): reset stream state after tool calls to prevent message loss#56071

fix(msteams): reset stream state after tool calls to prevent message loss#56071
BradGroux merged 4 commits intoopenclaw:mainfrom
SidU:fix/msteams-streaming-tool-calls

SidU commented Mar 27, 2026 •

edited

Loading

Uh oh!

greptile-apps bot commented Mar 27, 2026 •

edited

Loading

Important Files Changed

Comments Outside Diff (1)

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

Uh oh!

BradGroux left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

chatgpt-codex-connector bot Mar 28, 2026

Uh oh!

SidU commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		streamReceivedTokens = false;
		pendingFinalize = stream.finalize();

Uh oh!

Conversation

SidU commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Before / After

Test plan

Uh oh!

greptile-apps bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Comments Outside Diff (1)

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

BradGroux left a comment

Choose a reason for hiding this comment

Detailed Review: fix(msteams): reset stream state after tool calls

Problem

Fix Analysis (3 commits)

Edge Cases Verified

Test Coverage

VM Regression

Verdict

Uh oh!

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector bot Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

SidU commented Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

SidU commented Mar 27, 2026 •

edited

Loading

greptile-apps bot commented Mar 27, 2026 •

edited

Loading