Codex review: needs real behavior proof before merge. Reviewed July 25, 2026, 8:09 PM ET / July 26, 2026, 00:09 UTC.

ClawSweeper review

What this changes

This PR adds Microsoft Teams inbound normalization for mentions, quoted replies, forwarded content, and debounced message batches before agent dispatch.

Merge readiness

⛔ Blocked until real behavior proof from a real setup is added - 6 items remain

The repository inspection sandbox could not start, so I cannot responsibly make a code verdict from the supplied PR context alone.

Priority: P2
Reviewed head: 88316f50861e747685caaa3d93e2a94012dad14d
Owner decision: Required. See Decision needed.

Review scores

Verification

How this fits together

The Microsoft Teams plugin turns Bot Framework activities into agent turns. Incoming activity text, entities, and HTML attachments are normalized and filtered before dispatch to the agent and supplemental quote context.

flowchart LR
  A[Teams activity] --> B[Teams inbound handler]
  B --> C[Text and quote normalization]
  C --> D[Debounce and thread scoping]
  D --> E[Authorization and quote visibility]
  E --> F[Agent dispatch]

Decision needed

Why: The supplied proof confirms test fixtures and aggregate path activity but explicitly lacks a current-head live Teams capture; the correct merge gate depends on maintainer acceptance of that evidence threshold.

Before merge

• Add real behavior proof - Needs real behavior proof before merge: The PR body reports fixture tests and aggregate production telemetry, but explicitly says no current-head live Microsoft Teams tenant capture or replay was run; add redacted tenant output, diagnostics, or a recording that shows the changed behavior. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
• Resolve security concern: Complete the required security diff pass - The inspection sandbox failed before the current head and changed functions could be read, so this review cannot clear the patch's input-handling and dependency-facing changes.
• Resolve merge risk (P1) - The mandatory source, caller/callee, scoped-policy, dependency-contract, and current-main comparison could not be completed because the read-only inspection sandbox failed to start.
• Resolve merge risk (P1) - The submitted evidence remains mock and aggregate-telemetry based: it does not directly demonstrate current-head mention, quote, and forward output in a real Microsoft Teams tenant.
• Complete next step (P2) - A current-head source and proof review is required before any repair or merge recommendation; the read-only inspection sandbox prevented that review.

Findings

• [low] Complete the required security diff pass

@clawsweeper recheck

🦞👀
ClawSweeper assist is taking a look at your question.

I queued a lightweight read-only assist pass. It will post a separate answer comment and will not edit the durable ClawSweeper review comment or trigger close, merge, repair, label, or branch changes.

Request: recheck

ClawSweeper assist: New exact-head behavior evidence is present, but confirming whether it resolves the prior concerns requires a full correctness review.

Evidence:

• Azure Crabbox tested SHA 154ed0faf5473be7f46f62840726c7bab8a903ea.
• Focused Teams tests passed: 2 files, 64 tests.
• Both extension TypeScript lanes passed.
• The proof uses sanitized Teams-shaped payloads, not a live Teams tenant capture.

Suggested next action: Comment @clawsweeper review on #102379.

Source: #102379 (comment)
Assist reasoning: high.

@clawsweeper review

🦞🧹
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.

Re-review progress:

• State: Complete
• Detail: The targeted re-review published an exact durable review comment and queued an executing target-wide serialized router scan.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29177484974
• Updated: 2026-07-12T03:05:55.990Z

Codex maintainer review: changes needed before merge.

The PR moves Microsoft Teams inbound mention, quoted-reply, and forward normalization into the Teams plugin, which is the right owner boundary for BUG-052 / #102376. The direct no-debounce behavior is covered by the new focused tests and Azure Crabbox proof, but the batched Teams path still drops the new quote facts.

LOC: +565/-17 (4 files)

Findings:

• [P2] Preserve quotedReply facts through multi-entry Teams debounce batches - extensions/msteams/src/monitor-handler/message-handler.ts:1076

  handleTeamsMessage now normalizes each activity before enqueue and handleTeamsMessageNow extracts quote metadata from attachments plus activity.entities, but the multi-entry debounce flush constructs a synthetic dispatch from last.context, combinedText, combinedRawText, and attachments: []. If the first buffered Teams activity is an entity-only quotedReply and the second buffered activity is plain text from the same sender/conversation, the agent receives the first normalized body text in the combined batch while extractMSTeamsQuoteInfo(attachments, activity.entities) can only see the last activity's entities. That loses supplemental quote context and the quoted-sender allowlist decision for the message text that was just delivered.

  This is not fixed by the attachment debounce guard: shouldDebounce skips entries with attachments, but the new quotedReply parser also supports activity.entities, and those entries can have no attachments. The existing tests cover direct dispatch and blocked quote filtering, but they do not cover nonzero messages.inbound.debounceMs with a quoted entry followed by a second same-key entry.

  Best fix: prepare quote facts at the same point as the normalized text, carry them on MSTeamsDebounceEntry, and define the batched merge rule explicitly. A conservative merge could keep the last entry that has quote metadata, or suppress quote metadata when multiple quoted entries would be ambiguous, but it should not silently couple combined text to only last.context.activity.entities. Add allowed and blocked quoted-sender regression tests under nonzero debounce.

Best-fix verdict: acceptable boundary, too narrow implementation. Teams-owned normalization is the right layer; the missing piece is carrying the newly parsed metadata through the existing Teams debounce lifecycle.

Alternatives considered: changing the shared debouncer is unnecessary because it already passes every buffered entry to onFlush; extracting quotes only inside handleTeamsMessageNow is the current problem because multi-entry batches no longer have one native activity; disabling debounce for any quotedReply entity would be safe but loses batching for a normal text-only Teams shape and is less complete than preserving prepared facts.

Provenance: likely introduced by this PR's new activity.entities quote support interacting with the older Teams debounce merge from #971. The older merge already retained last.context, but before this PR entity-only quoted replies were not converted into supplemental quote context.

Code read: extensions/AGENTS.md; extensions/msteams/src/inbound.ts; extensions/msteams/src/inbound.test.ts; extensions/msteams/src/monitor-handler/message-handler.ts; extensions/msteams/src/monitor-handler/message-handler.authz.test.ts; src/auto-reply/inbound-debounce.ts; sibling debounce handling in extensions/signal/src/monitor/event-handler.ts, extensions/signal/src/monitor/event-handler.inbound-context.test.ts, and extensions/mattermost/src/mattermost/monitor.ts; current upstream/main for the touched Teams path.

Remaining uncertainty: I did not run a live Teams tenant replay. Existing Azure proof covers sanitized direct dispatch fixtures and type lanes, but not this nonzero-debounce scenario.

@clawsweeper review

🦞🧹
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.

Re-review progress:

• State: Complete
• Detail: The targeted re-review published an exact durable review comment and queued an executing target-wide serialized router scan.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/29185403260
• Updated: 2026-07-12T08:19:53.523Z

Maintainer review for current head 516e68a930a122f3abf9a3b8ba38683107d990d8.

This PR fixes Teams inbound text normalization for mention tags, quotedReply entities, forwarded-message markers, and debounced Teams batches. The owner boundary is right: the repair stays in the bundled Microsoft Teams plugin and feeds quote previews through the existing supplemental-context visibility filter.

Findings:

• Blocking: extensions/msteams/src/monitor-handler/message-handler.ts:768 still lets the current channel-thread parent override a quotedReply entity sender before the allowlist decision. extractMSTeamsQuoteInfo() now returns quoteInfo.senderId from quotedReply entities, but when the same turn also has activity.replyToId and thread-parent Graph lookup succeeds, this block assigns quoteSenderId from parentMsg whenever quoteContext === context; the later quoteSenderId ??= quoteInfo.senderId never restores the entity sender. That means a single non-debounced channel reply can quote a blocked sender, fetch an allowlisted thread/root parent, and pass quoteSenderAllowed, exposing the blocked quotedReply.preview through SupplementalContext. The new debounced test covers the “earlier quote plus later allowed parent” case, but the same-turn path is still open. Please prefer the quotedReply sender facts when present and use the Graph parent sender only as a fallback for legacy HTML Reply attachment metadata that lacks a sender id, then add a focused no-debounce regression: allowlist Alice, use quotedReply.senderId = mallory-aad, replyToId = parent-msg, mocked parent sender Alice, and assert SupplementalContext is {} and the blocked preview is absent.

Best-fix verdict: acceptable layer, but too narrow until current-turn quotedReply authorization keeps the entity sender authoritative over unrelated thread-parent context.

Alternatives considered: moving this to core supplemental filtering would be the wrong layer because core only receives senderAllowed; disabling parent Graph sender fallback entirely would risk regressing existing HTML Reply attachment quotes that have sender names but no AAD sender id. The bounded fix is to distinguish entity-backed quote sender facts from parent-derived fallback facts in the Teams handler.

Code read: extensions/AGENTS.md; extensions/msteams/src/inbound.ts; extensions/msteams/src/monitor-handler/message-handler.ts; extensions/msteams/src/inbound.test.ts; extensions/msteams/src/monitor-handler/message-handler.authz.test.ts; src/channels/inbound-event/context.ts; src/auto-reply/inbound-debounce.ts; extensions/msteams/src/graph-thread.ts.

Remaining uncertainty: I did not run a live Teams tenant replay. Current PR CI is still running; checks-fast-loc-ratchet has reported failure, but the finding above is independent of CI status.