Skip to content

fix(feishu): skip reply anchor for DM streaming cards (fixes #94922)#94969

Closed
bowenluo718 wants to merge 1 commit into
openclaw:mainfrom
bowenluo718:fix/issue-94922-feishu-dm-streaming-reply-anchor
Closed

fix(feishu): skip reply anchor for DM streaming cards (fixes #94922)#94969
bowenluo718 wants to merge 1 commit into
openclaw:mainfrom
bowenluo718:fix/issue-94922-feishu-dm-streaming-reply-anchor

Conversation

@bowenluo718

@bowenluo718 bowenluo718 commented Jun 19, 2026

Copy link
Copy Markdown

Summary

Fix Feishu streaming cards incorrectly using reply mode for ordinary direct messages, causing "Reply to [User]" labels and slower message delivery.

Fixes #94922

Real behavior proof (required for external PRs)

Behavior addressed:
Feishu DM replies were showing "Reply to [User]" labels due to streaming path not respecting the skipReplyToInMessages decision that was already correctly applied to non-streaming path.

Real setup tested:

  • Runtime: Unit tests with mocked Feishu client and streaming session

Exact steps or command run after fix:

node scripts/run-vitest.mjs extensions/feishu/src/reply-dispatcher.test.ts --reporter=verbose

After-fix evidence:

✓ |extension-feishu| extensions/feishu/src/reply-dispatcher.test.ts > createFeishuReplyDispatcher streaming behavior > skips reply anchor for ordinary DM with streaming enabled (fixes #94922) 2ms
✓ |extension-feishu| extensions/feishu/src/reply-dispatcher.test.ts > createFeishuReplyDispatcher streaming behavior > preserves reply anchor for group topic with streaming enabled 2ms
✓ |extension-feishu| extensions/feishu/src/reply-dispatcher.test.ts > createFeishuReplyDispatcher streaming behavior > preserves reply anchor for P2P thread reply with streaming enabled 2ms

Test Files  1 passed (1)
     Tests  85 passed (85)

Observed result after the fix:
All unit tests pass including 3 new tests specifically covering the reported bug scenario and regression prevention.

What was not tested:
Live Feishu tenant validation - requires maintainer access to Feishu environment to verify the visual "Reply to" label is removed in actual DM conversations.

Tests and validation

  • Added 3 new test cases in reply-dispatcher.test.ts:

    1. DM skip reply anchor (regression test for [Bug]: Feishu Plugin Incorrectly Uses Reply Mode in Direct (DM) Chats #94922)
    2. Group topic preserve reply anchor (regression prevention)
    3. P2P thread preserve reply anchor (regression prevention)

  • All existing tests pass: 85/85 in reply-dispatcher.test.ts, 87/87 in bot.test.ts

  • No breaking changes to existing functionality

Risk checklist

Did user-visible behavior change? (Yes / No)

Yes - Intentional fix: Feishu DM replies will no longer show "Reply to [User]" labels when streaming is enabled (default behavior). This restores the correct behavior that existed before the streaming feature was added.

Did config, environment, or migration behavior change? (Yes / No)

No - No configuration changes required. The fix uses existing skipReplyToInMessages logic that was already calculated but not applied to streaming path.

Did security, auth, secrets, network, or tool execution behavior change? (Yes / No)

No - Only affects message routing mode (reply vs create), no security implications.

What is the highest-risk area?

  • Streaming card delivery for Feishu channel

How is that risk mitigated?

  • Comprehensive test coverage of all three scenarios (DM, group topic, P2P thread)
  • Minimal one-line code change using existing well-tested logic
  • Preserves backward compatibility for group chats and thread replies

Current review state

What is the next action?

  • Maintainer review
  • Live Feishu validation (verify DM replies no longer show "Reply to" label)

@bowenluo718
fix(feishu): skip reply anchor for DM streaming cards (fixes openclaw…
1997cc2 
…#94922)

- Fix streaming.start() to use sendReplyToMessageId instead of raw replyToMessageId
- Add tests for DM skip, group topic preserve, and P2P thread preserve scenarios
- All existing tests pass (85/85 in reply-dispatcher.test.ts, 87/87 in bot.test.ts)

Fixes openclaw#94922
@openclaw-barnacle openclaw-barnacle Bot added channel: feishu Channel integration: feishu size: S proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 19, 2026
@clawsweeper

clawsweeper Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 20, 2026, 1:25 PM ET / 17:25 UTC.

Summary
The PR changes Feishu streaming-card startup to use the dispatcher’s filtered reply target and adds regression tests for ordinary DM, group/topic, and P2P-thread streaming behavior.

PR surface: Source 0, Tests +125. Total +125 across 2 files.

Reproducibility: yes. from source: ordinary Feishu DMs set skipReplyToInMessages, but current main still passes raw replyToMessageId into streaming-card start, which selects reply mode. I did not run a live Feishu tenant reproduction in this read-only review.

Review metrics: 1 noteworthy metric.

  • Feishu streaming send target: 1 start option changed. That option controls whether streaming cards use Feishu reply mode or direct create mode, which is the central merge-relevant behavior.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94922
Summary: This PR is one of several candidate fixes for the canonical Feishu DM streaming-card reply-anchor regression; no sibling has merged or supplied enough live proof to supersede this PR yet.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Remove unsupported isGroup and directThreadReply fields from the added dispatcher tests and model those cases with supported parameters.
  • [P1] Add redacted live Feishu DM proof, or get an explicit maintainer proof override.
  • [P1] Coordinate with maintainers so only one same-root Feishu fix PR lands.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies mocked unit-test output and explicitly says live Feishu tenant validation was not tested, so the contributor should add redacted Feishu screenshot, recording, log, terminal/live output, or get a maintainer proof override before merge. 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.

Mantis proof suggestion
The changed behavior is a visible Feishu Reply-to label and thread-routing outcome that real client proof would materially improve. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify in a real Feishu DM that a streaming card reply appears without a Reply-to label while P2P direct-thread or group/topic replies still route correctly.

Risk before merge

  • [P1] Live Feishu DM delivery has not been shown after this patch, so the visible Reply-to label and thread-preservation behavior still need redacted real-environment proof or an explicit maintainer proof override.
  • [P1] There are multiple open same-root candidate PRs for the linked Feishu issue, so maintainers should choose one canonical landing path rather than merge duplicate branches.
  • [P1] The added tests currently use unsupported dispatcher parameters, so the PR needs a test repair before the TypeScript test/type lane can be trusted.

Maintainer options:

  1. Repair tests and require Feishu proof (recommended)
    Ask for supported dispatcher test parameters plus redacted live Feishu DM evidence before merge.
  2. Choose a sibling candidate
    If another same-root PR gets clean tests and stronger live proof first, pause this branch in favor of that canonical landing path.
  3. Accept maintainer proof override
    A maintainer with Feishu credentials may explicitly accept the source/test proof after independently verifying or owning the channel-routing risk.

Next step before merge

  • [P1] Manual review is needed because the remaining blockers are contributor/live Feishu proof, repair of the added tests, and canonical selection among same-root PRs.

Security
Cleared: The diff only changes Feishu dispatcher routing and colocated tests; it does not touch secrets, dependencies, workflows, packaging, or install scripts.

Review findings

  • [P2] Remove unsupported dispatcher test fields — extensions/feishu/src/reply-dispatcher.test.ts:2173-2174
Review details

Best possible solution:

Land one canonical narrow Feishu dispatcher fix after the tests use supported dispatcher parameters and redacted live Feishu proof or a maintainer override confirms ordinary DMs lose the Reply-to label while direct-thread and group/topic replies remain routed.

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

Yes, from source: ordinary Feishu DMs set skipReplyToInMessages, but current main still passes raw replyToMessageId into streaming-card start, which selects reply mode. I did not run a live Feishu tenant reproduction in this read-only review.

Is this the best way to solve the issue?

No, not as submitted. Reusing sendReplyToMessageId is the right narrow runtime fix, but the added tests use unsupported parameters and the PR still needs real Feishu proof or maintainer override.

Full review comments:

  • [P2] Remove unsupported dispatcher test fields — extensions/feishu/src/reply-dispatcher.test.ts:2173-2174
    createFeishuReplyDispatcher takes the already-derived fields skipReplyToInMessages, replyInThread, threadReply, and rootId, not caller-only inputs like isGroup or directThreadReply. These object literals are type-checked directly, so the added tests can fail the TS test/type lane here and again in the P2P-thread case; express the scenarios with supported dispatcher params instead.
    Confidence: 0.92

Overall correctness: patch is incorrect
Overall confidence: 0.92

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ad5d2cbc1bcb.

Label changes

Label justifications:

  • P2: This is a bounded Feishu channel regression fix with visible DM delivery impact and limited blast radius.
  • merge-risk: 🚨 message-delivery: The PR intentionally changes Feishu streaming-card delivery routing for direct messages, so live routing behavior matters beyond mocked tests.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies mocked unit-test output and explicitly says live Feishu tenant validation was not tested, so the contributor should add redacted Feishu screenshot, recording, log, terminal/live output, or get a maintainer proof override before merge. 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.
Evidence reviewed

PR surface:

Source 0, Tests +125. Total +125 across 2 files.

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

What I checked:

  • Repository policy read: Root AGENTS.md and scoped extensions/AGENTS.md were read fully; their read-beyond-diff, bundled-plugin boundary, message-delivery risk, and real behavior proof guidance apply to this Feishu plugin PR. (AGENTS.md:1, ad5d2cbc1bcb)
  • Current-main source reproduction: Current main computes sendReplyToMessageId from skipReplyToInMessages, but startStreaming still passes raw replyToMessageId to streaming.start, so ordinary DM streaming cards can still select reply mode. (extensions/feishu/src/reply-dispatcher.ts:173, ad5d2cbc1bcb)
  • Caller establishes the DM/thread split: The Feishu bot caller passes skipReplyToInMessages as true for ordinary P2P DMs and false for direct-thread replies, so the dispatcher already receives the correct policy decision. (extensions/feishu/src/bot.ts:1783, ad5d2cbc1bcb)
  • Streaming send-mode contract: resolveStreamingCardSendMode selects reply when replyToMessageId is present, root_create when only rootId is present, and create when neither routing field is present. (extensions/feishu/src/streaming-card.ts:194, ad5d2cbc1bcb)
  • PR runtime diff: The PR changes the streaming.start options from raw replyToMessageId to replyToMessageId: sendReplyToMessageId, aligning streaming-card startup with sibling text, media, fallback, and structured-card send paths. (extensions/feishu/src/reply-dispatcher.ts:395, 1997cc20644a)
  • Patch defect in added tests: The added group/topic and P2P-thread tests pass isGroup and directThreadReply into createFeishuReplyDispatcher even though that function accepts already-derived dispatcher parameters, so TypeScript excess-property checking can reject the test literals. (extensions/feishu/src/reply-dispatcher.test.ts:2173, 1997cc20644a)

Likely related people:

  • Vincentwei1021: Merged history for feat(feishu): skip reply-to in DM conversations #13211 established that ordinary Feishu DMs should omit reply-to metadata while preserving typing behavior. (role: introduced prior DM reply suppression behavior; confidence: high; commits: d9230b13a4ad; files: extensions/feishu/src/bot.ts, extensions/feishu/src/reply-dispatcher.ts)
  • qiangu: Merged history for fix(feishu): stream plain replies as cards #88276 routed ordinary Feishu final replies through the streaming-card path implicated by this bug. (role: streaming-card reply-path contributor; confidence: high; commits: 3b8ab4e11232; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/reply-dispatcher.test.ts, extensions/feishu/src/streaming-card.ts)
  • xzq-xu: Feishu Card Kit streaming support history credits this handle and introduced the streaming session surface involved here. (role: original streaming-card contributor; confidence: medium; commits: 65be9ccf63f3; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/streaming-card.ts)
  • vincentkoc: Current blame and recent Feishu history show repeated maintenance on the affected dispatcher, streaming-card, and caller paths. (role: recent area contributor; confidence: medium; commits: e02e3d697146, c692fabeba07; files: extensions/feishu/src/reply-dispatcher.ts, extensions/feishu/src/streaming-card.ts, extensions/feishu/src/bot.ts)
  • LiaoyuanNing: Merged P2P direct-message thread work credits this source contributor for adjacent behavior this fix must preserve. (role: adjacent direct-thread contributor; confidence: medium; commits: c692fabeba07; files: extensions/feishu/src/bot.ts, extensions/feishu/src/bot.test.ts)
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: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. labels Jun 19, 2026
This was referenced Jun 19, 2026
Open
Closed
Closed
Closed
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: feishu Channel integration: feishu merge-risk: 🚨 message-delivery 🚨 May drop, duplicate, misroute, suppress, or wrongly target messages. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Feishu Plugin Incorrectly Uses Reply Mode in Direct (DM) Chats

1 participant

@bowenluo718