Codex review: needs changes before merge. Reviewed August 29, 2026, 7:00 PM ET / 23:00 UTC.

ClawSweeper review

What this changes

The PR sends Feishu replies with more than five Markdown tables through post delivery instead of a static card that Feishu rejects.

Regression provenance

Possible regression — suspected (reviewed change). No predecessor PR is attributed.

Merge readiness

⚠️ Needs maintainer review before merge - 4 items remain

Keep this PR open: its static-card fallback addresses a real Feishu delivery failure, but the new card-wide counter also rejects deliverable font-wrapped context cards. The prior P2 finding remains unfixed at the exact reviewed head.

Priority: P1
Reviewed head: bef9d7f357b7432514f094ea422f36a93d5e789b

Review scores

Verification

How this fits together

The Feishu plugin turns agent replies and action presentations into either static interactive cards or post messages. This change selects post delivery when a static card would exceed Feishu’s table-component limit.

flowchart LR
A[Agent reply or action] --> B[Feishu plugin]
B --> C{Static card eligible?}
C -->|Within table limit| D[Interactive card]
C -->|Over limit| E[Post renderer]
D --> F[Feishu chat]
E --> F

Before merge

• Preserve deliverable context cards (P2) - The collector counts every markdown element, but generated context blocks are font-wrapped and the supplied Feishu evidence says six tables there are accepted without table components. This changes a deliverable context-only card into post fallback; exclude the renderer-owned context form and cover both presentation routes.
• Resolve merge risk (P2) - A generated presentation containing only a six-table context block regresses from an accepted static card to post fallback.
• Resolve merge risk (P1) - GitHub reports this branch as dirty against main and no pinned test merge is available, so the repaired head needs a rebase and exact-head review.
• Complete next step (P2) - A bounded mechanical repair can preserve the established context-card behavior and add the missing route coverage.

Findings

• [P2] Preserve deliverable context cards — extensions/feishu/src/presentation-card.ts:93-98

@clawsweeper automerge

Special instructions:
Preserve generated font-wrapped context cards in table-limit accounting, add focused coverage for both presentation routes, then rebase and request an exact-head review.

ClawSweeper PR egg: ✨ hatched 🌱 uncommon Gilded Lint Imp. Rarity: 🌱 uncommon. Trait: stacks clean commits.

🦞🧹
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 finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26492894829
• Updated: 2026-05-27T05:40:39.231Z

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

@clawsweeper review

We accept Option 2: narrow the PR to static cards only.

• Static cards (im/v1/messages, msg_type: "interactive") — the 5-table limit is confirmed by Feishu docs and production logs (ErrCode 11310). This PR fixes that.
• Streaming cards (cardkit/v1/cards) — separate API, unverified whether the same limit applies. Out of scope.

The PR already stated "What was not tested: Streaming card path". Please re-evaluate with this scope clarification.

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re-review progress:

• State: Complete
• Detail: The targeted re-review finished, the durable review comment was updated, and the synced verdict was routed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/26567367804
• Updated: 2026-05-28T09:53:15.841Z

Thanks for the detailed review! Addressing each item:

1. [P2] Closing reference narrowed — Done. The closing footer Closes #43690 has been replaced with Part of #43690. The PR is now explicitly scoped to the static-card path.

2. [P1] Dirty branch / rebase — Done. The branch has been fully rewritten on top of current main (not rebased commit-by-commit, since the Feishu delivery code was restructured in the meantime): the guard is re-applied to the current useStaticCard / sendOutboundText / sendText decision points, and the tests are rewritten against the current test helpers (this also fixes the previous CI failure, which was caused by a stale helper reference firstTypingDispatcherOptions left over from a bulk edit).

3. [P1] Release-note context for the explicit-card compatibility change — Added a dedicated "Release note" section to the PR body: messages with >5 tables now render as posts (tables converted to code blocks) instead of failing delivery, including when renderMode: "card" is explicit.

4. [P1] Streaming CardKit path — This PR intentionally guards only static cards, but we have since empirically verified that the CardKit streaming path is not subject to this limit. The full streaming-card API sequence (POST cardkit/v1/cards → message send with card reference → incremental PUT .../elements/content/content → PATCH .../settings) was exercised against the real Feishu API with 6 and 18 markdown tables: all calls succeeded, while the equivalent static-card send fails with ErrCode 11310 at 6 tables. Streaming cards carry content in a single markdown element, so Feishu does not count table components there. Details and the verification table are in the PR body ("Streaming CardKit verification"). We also re-confirmed the static 11310 failure in a production gateway log on 2026-08-06, and verified end-to-end that the same 6-table reply delivers successfully through a streaming card when streaming is enabled. This has been left for maintainer judgement on the broader report rather than asserted as a reason to close it.

Happy to adjust scope or wording as maintainers prefer.

@clawsweeper re-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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: A newer review tuple already exists; this stale result was superseded.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/31078139959
• Updated: 2026-08-06T06:41:46.992Z

Addressing the presentation-card findings from the re-review:

Guard generated presentation cards (P1) — Done. buildFeishuPayloadCard now applies the same withinCardTableLimit() check to the embedded fallback text and returns undefined when the limit is exceeded, so generated presentation cards (including action-bearing ones) fall back to the existing text path instead of failing with ErrCode 11310. The guard is at the card-construction boundary as suggested; opaque native Feishu cards supplied by the caller remain untouched (that branch returns before the guard).

Real behavior proof for the action-bearing path — Added to the PR body. We reproduced the failure at the API level: sendPayload with a 6-table text plus an action-bearing presentation builds an interactive card embedding all 6 tables; sending that exact card JSON to the real Feishu API returns HTTP 400 / code 230099 / ErrCode: 11310. After the fix the same scenario delivers via the post path (tables converted to code blocks).

Regressions — Added in outbound.test.ts ("presentation card table-limit"): 6 tables + presentation with buttons refuse the generated card (sendCardFeishu not called) and deliver via post mode; 5 tables still build the card. Plus the earlier sendPayload/post-fallback boundary tests.

Explicit-card fallback coverage (P2) — Retained: the explicit renderMode: "card" boundary tests remain, and the Release note section still documents the card→post change.

All local checks pass (272 feishu extension tests + extension type check). Requesting a fresh review.

@clawsweeper re-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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/31086398495
• Updated: 2026-08-06T08:48:29.040Z

Two updates since the last review:

1. The message-action presentation path is now guarded too. In addition to buildFeishuPayloadCard, the channel.ts message-action path (send / thread-reply with a presentation param) builds presentation cards directly via buildFeishuPresentationCard. We reproduced the failure there in production: sending a presentation message with 6-table text returned HTTP 400 / code 230099 / ErrCode: 11310. The generated card is now rejected when the embedded text exceeds the table limit and delivery routes through the existing presentationFellBack → sendPayload text fallback. withinCardTableLimit() is now a single shared exported helper in presentation-card.ts (next to isFeishuCardWithinEnvelope) used by all static-card decision points. Opaque native Feishu cards remain untouched.

2. Regression coverage added for the new path. channel.test.ts: message-action send with presentation + 6-table text refuses the generated card (sendCardFeishu not called) and routes to the sendPayload fallback; the existing presentation tests (card rendering, oversized fallbacks) still pass. Full feishu extension suite: 1581 tests passed, extension type check clean.

The earlier items remain addressed: Part of #43690 reference, branch current with main, Release note section, streaming CardKit verification (6 and 18 tables, all API calls succeeded vs static 11310 at 6 tables), and the action-bearing sendPayload presentation regression.

@clawsweeper re-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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/31094465107
• Updated: 2026-08-06T10:44:57.152Z

Addressing the three P1 findings from the previous review:

1. Count tables with the canonical GFM parser (P1) — Done. Counting is now parser-backed: countMarkdownTables uses markdownToIRWithMeta (the same shared parser used for post-mode table conversion), so every parser-recognized table form is caught — including pipe-less GFM tables and alignment-colon delimiters — while fenced code, thematic breaks, and plain pipes in prose are not counted. The delimiter-row regex is gone, and the single shared implementation in presentation-card.ts is now used by every static-card decision point; the reply-dispatcher copy was removed as requested.

Why this matters in practice: we verified against the real Feishu API that pipe-less tables are rejected exactly like piped ones — 6 pipe-less GFM tables fail with HTTP 400 / code 230099 / ErrCode: 11310, the same as 6 piped tables. So the parser-backed count is required, not just preferable.

2. Route failed streaming recovery through post delivery (P1) — Done. ensureVisibleStreamingDelivery (the re-send after an invisible CardKit result) now checks the recovered content against the table limit and delivers over-limit content through the post path instead of retrying a static card the API would reject again. Covered by a regression test: streaming content with 6 tables that was never accepted recovers through sendMessageFeishu (post), and sendStructuredCardFeishu is not called. The useStreamingCard decision itself remains unchanged (streaming CardKit is not subject to this limit — see the verification table in the PR body).

3. Inspect Markdown in presentation blocks (P1) — Done. The presentation paths now use a card-level check (feishuCardWithinTableLimit) that counts tables across every markdown element of the built card — fallback text and text/context presentation blocks alike — rather than only the fallback text. Both buildFeishuPayloadCard (outbound) and the message-action path (channel.ts) reject the generated card at the construction boundary and fall back to the text path. Covered by regression tests: a presentation whose text block embeds 6 tables refuses the card and delivers via post on both paths.

After-fix proof — Added inspectable evidence to the PR body ("Real behavior evidence"): the same 6-table content through the post fallback (convertMarkdownTables → post) was delivered successfully against the real Feishu API (HTTP 200 / code 0, message_id: om_x100b6873165d1cacb22fa74e53d3f13), plus the five pre-fix failure captures (static piped, static pipe-less, generated presentation card, message-action send, original delivery-queue failure).

Full feishu extension suite: 1593 tests passed; extension type check clean.

@clawsweeper re-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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Review in progress
• Detail: The exact-review queue leased this run; Codex is reviewing the item.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/31102726622
• Updated: 2026-08-06T12:45:42.273Z

Updates addressing the remaining blocker (inspectable real behavior proof) and two CI failures:

1. Exact-path after-fix proof added (PR body → "Evidence") — The patched code paths were executed against the real Feishu API; each delivery receipt part shows which transport was actually used:

• sendText with 6 tables → post fallback delivered, messageId: om_x100b687c343ae53cb1a02a151b50f37, parts[0].kind: "text"
• sendText with 5 tables → card preserved at the boundary, messageId: om_x100b687c35ccaca8b20a9a7be054a36, parts[0].kind: "card"
• sendPayload with action-bearing presentation + 6-table text → generated card refused, delivered via the presentationFellBack post path, messageId: om_x100b687c35dfaca4b1cb06cf3f4a73b, parts[0].kind: "text"

Pre-fix failure captures (static piped, static pipe-less, generated presentation card, message-action send, original delivery-queue failure) remain in the same section.

2. CI failure fixed: check-dependencies (knip) — countMarkdownTables was flagged as an unused export; it is now module-private (only withinCardTableLimit and feishuCardWithinTableLimit are exported) and its regression tests were rewritten against the exported boundary functions. Both knip lanes (--production and all-exports) verified clean locally.

3. CI failure fixed: PR context and evidence — The PR body now uses the required authored sections: "What Problem This Solves" and "Evidence".

@clawsweeper re-review

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

Re the remaining P1 ("Count context tables before adding Feishu markup"): this scenario was tested directly against the real Feishu API, and it does not fail delivery — so no guard is needed there, and adding one would be a regression rather than a fix.

A context presentation block renders its text wrapped in <font color='grey'>…</font> (presentation-card.ts). To test whether Feishu counts tables inside that decoration, I sent a card whose only element is a markdown element containing <font color='grey'> + 6 markdown tables + </font> — the exact bytes the context-block renderer emits — to the real im/v1/messages endpoint:

HTTP 200 / code 0 / msg success

Feishu accepts the card. The <font> wrapper keeps the table rows from being parsed as table components, so Feishu's 5-component counter never sees them and delivery succeeds — the same mechanism as the CardKit streaming path (content that isn't a table component isn't counted).

Consequence: the card-level check intentionally counts what Feishu counts — bare table components — and leaves font-decorated content unguarded. Counting semantic context text "before decoration" would block cards that Feishu delivers fine, degrading working card rendering to post fallback for no delivery benefit. This is now documented in the PR body's scope note plus evidence item 9.

The other three items in the previous round were the proof request (now supplied, per the updated Proof confidence) and maintainer-acceptance notes already covered by the Release note section.

@clawsweeper re-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: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

Re-review progress:

• State: Complete
• Detail: The durable review result and its route handoff completed.
• Run: https://github.com/openclaw/clawsweeper/actions/runs/31130391703
• Updated: 2026-08-06T23:16:49.660Z

Thanks @name5566 for the original diagnosis, implementation, and extensive real Feishu proof. The canonical repair has now landed through #136382 as 7e21eeb and closed #43690.

The landed change incorporates the parser-backed five-table ceiling across outbound text, generated presentations, message actions, ordinary replies, and failed-stream recovery. It also preserves font-wrapped context cards that Feishu accepts and active CardKit streams whose markdown is not subject to the static-card component limit, with focused coverage for both boundaries.

The squash commit preserves your incorporated source contribution with co-author credit. This older conflicting branch is now superseded, so I'm closing it without merge while leaving its discussion and proof visible as project history.