Skip to content

fix(messaging): queue Teams chat.send turns like webchat (Refs #5851) - #5862

Closed
deepujain wants to merge 1 commit into
NVIDIA:mainfrom
deepujain:fix/5851-teams-tool-error
Closed

deepujain wants to merge 1 commit into
NVIDIA:mainfrom
deepujain:fix/5851-teams-tool-error

Conversation

@deepujain

@deepujain deepujain commented Jun 26, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Extends NemoClaw's OpenClaw chat.send compatibility patch so Microsoft Teams queued turns use the same per-message followup behavior as webchat.
  • Upgrades older webchat-only shims in existing OpenClaw installs instead of treating them as already patched.
  • Adds regression coverage for fresh patching, reruns, and old-shim upgrade behavior.

Changes

  • Broadened the queue-mode guard from sessionCtx.Provider === "webchat" to webchat or msteams.
  • Added upgrade logic for previously patched dist files that still contain the old webchat-only marker.
  • Updated patch verification markers and tests to cover Teams.

Testing

  • npx vitest run test/openclaw-chat-send-patch.test.ts
  • npm run build:cli
  • npm run typecheck:cli
  • npm run source-shape:check
  • git diff --check

Evidence it works

The regression test rewrites a freshly patched OpenClaw fixture back to the old webchat-only shim, reruns the patch script, and verifies it upgrades to the webchat/Teams guard without leaving the old provider check behind.

Refs #5851

Signed-off-by: Deepak Jain deepujain@gmail.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved chat reply handling so queued replies work more reliably across both web and Microsoft Teams.
    • Updated the patching behavior to recognize and upgrade older queue logic, helping avoid inconsistent reply behavior after updates.
    • Expanded automated coverage to verify the fix applies correctly, remains idempotent, and can upgrade a web-only configuration to include Microsoft Teams support.

@copy-pr-bot

copy-pr-bot Bot commented Jun 26, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 36d53029-14bb-4449-82eb-8cc0d04d8b91

📥 Commits

Reviewing files that changed from the base of the PR and between 299b60a and ed006f4.

📒 Files selected for processing (1)
  • scripts/patch-openclaw-chat-send.js
💤 Files with no reviewable changes (1)
  • scripts/patch-openclaw-chat-send.js

📝 Walkthrough

Walkthrough

The patcher now treats webchat and msteams as valid providers when updating the queued get-reply shim, renames the marker text accordingly, and updates recognizer verification. Tests were adjusted and expanded to cover the new marker and an upgrade from the older webchat-only form.

Changes

OpenClaw chat-send queue patch

Layer / File(s) Summary
Patch marker and provider condition
scripts/patch-openclaw-chat-send.js
The get-reply patch now matches sessionCtx.Provider values of webchat or msteams, updates the marker text to webchat/msteams, and changes the recognizer verification strings.
Compatibility tests and upgrade regression
test/openclaw-chat-send-patch.test.ts
The compatibility assertions expect the new marker and provider condition, and a regression test rewrites an older webchat-only shim before repatching it to the webchat/msteams form.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

Suggested labels

  • integration: openclaw
  • chore

Suggested reviewers

  • prekshivyas
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: extending chat.send queuing for Teams to behave like webchat.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
scripts/patch-openclaw-chat-send.js (1)

134-177: 🩺 Stability & Availability | 🔵 Trivial

Run the targeted TUI correlation E2E before merge.

These unit tests cover the text rewrite, but this helper also affects live turn ordering and duplicate-send behavior in the sandbox image. Please run openclaw-tui-chat-correlation-e2e on this branch before shipping. As per path instructions, this helper can affect “TUI message ordering, duplicate sends, and user/assistant turn correlation” and the recommended selective check is openclaw-tui-chat-correlation-e2e.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@scripts/patch-openclaw-chat-send.js` around lines 134 - 177, Run the targeted
openclaw-tui-chat-correlation-e2e before merging this patch, since the change in
the chat send helper can affect live turn ordering, duplicate sends, and
user/assistant correlation beyond the text rewrite covered by unit tests. Verify
the updated logic in the patch-openclaw-chat-send.js helper, especially the
resolvedQueue rewrite and the webchat/msteams condition, with the selective TUI
correlation E2E check.

Source: Path instructions

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@scripts/patch-openclaw-chat-send.js`:
- Around line 134-177: Run the targeted openclaw-tui-chat-correlation-e2e before
merging this patch, since the change in the chat send helper can affect live
turn ordering, duplicate sends, and user/assistant correlation beyond the text
rewrite covered by unit tests. Verify the updated logic in the
patch-openclaw-chat-send.js helper, especially the resolvedQueue rewrite and the
webchat/msteams condition, with the selective TUI correlation E2E check.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 86c65a9d-139c-4b0c-b707-3d30e9259cf5

📥 Commits

Reviewing files that changed from the base of the PR and between c1c7c1d and 299b60a.

📒 Files selected for processing (2)
  • scripts/patch-openclaw-chat-send.js
  • test/openclaw-chat-send-patch.test.ts

@wscurran wscurran added area: messaging Messaging channels, bridges, manifests, or channel lifecycle bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior labels Jun 26, 2026
@wscurran

Copy link
Copy Markdown
Contributor

✨ Thanks for extending the OpenClaw chat.send patch to queue Teams turns with the same per-message followup behavior as webchat. This proposes a way to upgrade older webchat-only shims and adds regression coverage for fresh patching, reruns, and old-shim upgrade behavior.


Related open issues:

@sandl99

sandl99 commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Thanks for putting this together. I’d like to hold this PR for now rather than merge it on the current OpenClaw baseline.

After looking at #5851 again, the duplicate/stale Teams reply behavior looks likely to be coming from the OpenClaw runtime path. #5595 is already bumping NemoClaw to OpenClaw 2026.6.9 and updates the same scripts/patch-openclaw-chat-send.js compatibility surface, so I think we
should let that land first and then retest #5851 against the bumped baseline.

If the issue still reproduces after #5595 is checked in, we can rebase this PR on top of the new OpenClaw patch shape and decide whether extending the queue-mode behavior to msteams is still the right targeted fix.

Thanks again for the quick patch and regression coverage.

@sandl99 sandl99 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting QA to confirm after #5995

@deepujain
deepujain force-pushed the fix/5851-teams-tool-error branch from 299b60a to ed006f4 Compare July 3, 2026 01:56
@deepujain

Copy link
Copy Markdown
Collaborator Author

Rebased on current main, including the merged #5995 baseline. The patch test, build, typecheck, and lint/checks pass; leaving this parked for QA confirmation.

Refs NVIDIA#5851

Signed-off-by: Deepak Jain <deepujain@gmail.com>
@deepujain
deepujain force-pushed the fix/5851-teams-tool-error branch from ed006f4 to 0f912e9 Compare July 6, 2026 10:40
@deepujain

Copy link
Copy Markdown
Collaborator Author

Rebased on current main and resolved the OpenClaw 2026.6.10 test-shape conflict while keeping the Teams chat.send regression. Patch test, build, typecheck, lint/checks, and the conditional scan pass; still parked for QA confirmation.

@sandl99

sandl99 commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator

Thanks @deepujain for supporting on this duplicate changes. After #5995 is merged, QA confirmed it's fixed and no longer repro. I'm closing this PR.

@sandl99 sandl99 closed this Jul 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: messaging Messaging channels, bridges, manifests, or channel lifecycle bug-fix PR fixes a bug or regression integration: openclaw OpenClaw integration behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants