fix(gateway/feishu): enable CardKit typewriter streaming in card mode

[SunnyYYLin]

What problem does this solve?

When FEISHU_CARD_STREAMING_MODE=card, messages are delivered as CardKit cards but with no visible typewriter effect 鈥?the full reply appears all at once after the agent finishes. Root cause: the unified adapter's show_streaming_placeholder() returns false, so the core creates a draft placeholder (message_id="draft"), and all incremental edit_message commands are skipped by the is_valid_feishu_message_id seam.

This bug first appeared in v0.9.0, caused by the unified adapter replacing the legacy Feishu adapter's streaming path.

Closes #1367

Discord Discussion URL: https://discordapp.com/channels/1491295327620169908/1530153804333449226

Review Contract

Goal

Restore CardKit typewriter streaming for Feishu in unified mode by giving the streaming placeholder a real om_ message ID, so incremental edits flow through handle_card_edit 鈫?update_card_stream instead of being dropped at the draft seam.

Non-goals

• No changes to Telegram streaming (retains Draft strategy).
• No changes to the default Feishu post-edit path (mode unset or post).
• No multi-card splitting for very long responses (follow-up).
• No changes to the ACP platform streaming path (append-only deltas, unrelated).

Accepted Residual Risks

• Feishu CardKit streaming mode has a server-side idle timeout. If the agent is idle longer than FEISHU_CARD_IDLE_FINALIZE_MS, the idle reaper finalizes the card. Late edits after finalize fall back to delete-and-resend (existing fix(streaming): recover from Feishu 20-edit cap (errcode 230072) #1122 recovery path).
• The unified adapter's send_message remains fire-and-forget for non-streaming paths. Only send_streaming_placeholder waits for the platform response (via a dedicated response channel). This keeps the change isolated to the streaming path.

Acceptance Criteria

• Without FEISHU_CARD_STREAMING_MODE=card, behavior is identical to before (post-edit path).
• With FEISHU_CARD_STREAMING_MODE=card, real CardKit updates stream during generation.
• 33 sequential CardKit updates (seq 1鈫?3, ~1.7s cadence) captured in production logs.
• cargo test --features unified passes.
• cargo clippy --workspace --features unified -- -D warnings clean.
• No trait-breaking changes for external/custom adapters.

Follow-ups

• Multi-card splitting for responses exceeding Feishu's per-card content limit.
• Configurable streaming flush interval (currently tied to dispatch batch cadence).

At a Glance

Before (broken since v0.9.0):
  core 鈫?draft placeholder ("draft") 鈫?edit_message("draft", text)
       鈫?is_valid_feishu_message_id("draft") == false 鈫?SKIPPED
       鈫?try_send_initial_card at turn end 鈫?full text, no streaming

After (restores pre-v0.9.0 behavior):
  core 鈫?EditablePlaceholder 鈫?send_streaming_placeholder()
       鈫?create Feishu post 鈫?dispatch_with_response() 鈫?real om_ ID
       鈫?edit_message(om_xxx, text) 鈫?handle_card_edit
       鈫?update_card_stream(card_id, seq++) 鈫?typewriter effect
       鈫?idle > FEISHU_CARD_IDLE_FINALIZE_MS 鈫?finish_card_stream

Prior Art & Industry Research

OpenClaw:
Uses draft-message editing for Telegram streaming (edit_message every N tokens). Feishu path uses post-edit with a 20-edit cap before promoting to card.

Hermes Agent:
N/A 鈥?Hermes does not support Feishu.

Other references:

• Feishu CardKit streaming API: PUT /cardkit/v1/cards/:id/elements/:eid/content with strictly-increasing sequence numbers.
• Feishu errcode 300317: sequence must be strictly increasing.
• Feishu errcode 300309: streaming mode closed after server-side idle timeout.

Proposed Solution

The fix restores the pre-v0.9.0 streaming path through the unified adapter architecture:

1. adapter.rs: Additive StreamingStrategy enum (Disabled / Draft / EditablePlaceholder) + streaming_strategy() method with default impl. Legacy use_streaming() / show_streaming_placeholder() retained 鈥?no trait break.

2. unified_adapter.rs: Selects EditablePlaceholder for Feishu when FEISHU_CARD_STREAMING_MODE=card. Adds dispatch_with_response() 鈥?a request-response channel that waits for the Feishu adapter to return the real om_ message ID. Only the streaming placeholder path uses this; normal send_message remains fire-and-forget.

3. main.rs: Passes streaming_strategy into the ACP turn loop .

4. feishu.rs: +1 line observability log on CardOutcome::Updated (card_id, msg_id, seq).

Why a response channel instead of modifying send_message?

The unified adapter's send_message is fire-and-forget by design. Changing it to always wait for a platform response would:

• Add ~500ms latency to every message send (not just streaming)
• Require all platform adapters (Telegram, Discord, Line) to return message IDs
• Break the fire-and-forget contract that non-streaming paths rely on

The response channel isolates the wait to send_streaming_placeholder only.

Alternatives Considered

1. Modify send_message to return real IDs: Fewer lines (~50 vs ~140) but changes behavior for all platforms and all message paths. Rejected 鈥?too broad for an opt-in feature.
2. One-line fix (show_streaming_placeholder() 鈫?true): Works but leaks Feishu-specific semantics into the generic adapter trait. No mechanism for adapters to customize placeholder behavior.
3. Delta-based updates: Rejected 鈥?Feishu CardKit requires full content on each PUT, not deltas.

Validation

• cargo test --features unified 鈥?all pass
• cargo clippy --workspace --features unified -- -D warnings 鈥?clean
• cargo build --release --features unified 鈥?success
• Manual testing: deployed to local service, sent webhook smoke prompt, observed 33 sequential CardKit updates (seq 1鈫?3, ~1.7s cadence) on card_id=7666195248321989574. stopReason=end_turn, 5888 output tokens. Full typewriter effect visible in Feishu client.

[chaodu-obk]

Note

LGTM ✅ - The opt-in CardKit path preserves native Feishu message IDs and isolates gateway responses from unified user-event dispatch; no blocking findings.

What This PR Does

This PR restores visible CardKit typewriter streaming for unified Feishu deployments when FEISHU_CARD_STREAMING_MODE=card. It keeps the existing send-once behavior for unset, auto, and post modes while adding a response path that returns the real Feishu om_ message ID.

How It Works

• Adds an additive StreamingStrategy API and an editable-placeholder hook to the core adapter trait, preserving existing implementations.
• Makes the unified adapter select the editable-placeholder strategy only for Feishu card mode, then correlates gateway responses by request ID.
• Uses the native Feishu message ID for subsequent full-content CardKit updates, while preserving fire-and-forget behavior for ordinary sends.
• Filters GatewayResponse envelopes out of the unified inbound event bridge and adds update observability.

Findings

#	Severity	Finding	Location
1	🟢	The strategy selection is explicitly platform-aware and preserves legacy behavior for existing adapter implementations.	crates/openab-core/src/adapter.rs:476
2	🟢	Request correlation uses a per-request ID and the native Feishu message ID, preventing synthetic IDs from reaching CardKit edit APIs.	src/unified_adapter.rs:143
3	🟢	The bridge now prevents response envelopes from being reprocessed as user events.	src/main.rs:1249
4	🟢	The exact-head CI checks reported success for validation, checks, build, and unified smoke-test jobs.	55df8b6d95fdc8961c470ebe9a1ec7196e5ab903

Finding Details

🟢 F1: Backward-compatible strategy API

The new trait methods have defaults, and the default strategy maps the pre-existing use_streaming and show_streaming_placeholder contract to the corresponding lifecycle. The unified override narrows the new CardKit behavior to the explicit Feishu card mode.

🟢 F2: Correct native-ID response path

The unified adapter subscribes before dispatch, filters by the exact request ID, waits only for Feishu responses, and returns the native om_ message ID to the core streaming loop. Ordinary send_message remains fire-and-forget.

🟢 F3: Response/event separation

The embedded unified bridge recognizes openab.gateway.response.v1 envelopes and skips user-event processing, while the request waiter consumes the matching response directly.

🟢 F4: Validation evidence

The reviewed head has successful repository check runs, including validate, check, build-builder, unified smoke tests, and packaged-pin validation. Local Rust compilation was not available because cargo, rustc, and rustup are absent in the review environment; exact-source diagnostics and git diff --check were clean.

Addressing External Reviewer Feedback

No external GitHub review comments or threads were present at review time, so there are no unresolved external concerns.

Baseline Check

• PR opened: 2026-07-25T20:26:19Z
• Declared base: main at 53061d696148106b2b7529f9d6c5dd802dff4545
• Reviewed head: 55df8b6d95fdc8961c470ebe9a1ec7196e5ab903
• Merge-base: 967270087ab74b32bcba9f6bc89a402e7abc3aca
• Diff stat: 5 files, 412 insertions, 22 deletions
• Main already has: the existing adapter, Feishu CardKit, and unified event-bridge foundations.
• Net-new value: the additive streaming strategy, native-ID response correlation, unified bridge response filtering, and update observability needed for opt-in CardKit streaming.

5. Three Reasons We Might Not Need This PR

1. The feature is opt-in - deployments that do not select Feishu card mode receive no new streaming behavior.
2. The response channel adds complexity - a simpler send-once path would avoid request correlation, but it would not provide the requested typewriter effect.
3. CardKit has operational limits - idle finalization and platform API behavior still require monitoring and the documented long-response follow-up.

These are tradeoffs and follow-up considerations, not blocking findings for this change.

What's Good (🟢)

• The implementation keeps the ordinary send path fire-and-forget and scopes waiting to the streaming placeholder and card edits.
• Existing trait implementations remain source-compatible through default methods.
• The core and gateway tests added by the change cover strategy selection and request correlation.
• The change includes a useful CardOutcome::Updated log with card ID, message ID, and sequence.

[chaodu-obk]

Note

LGTM ✅ - No blocking findings; the opt-in Feishu CardKit streaming path is backward-compatible and correctly correlates native message IDs.

Consolidated review: #1458 (comment)

[wangyuyan-agent]

Reviewed at head 55df8b6d. Verdict: request changes — not because the approach is wrong, but because three statements in the PR description do not match the code, and the recovery path this change depends on is not wired in unified mode.

Direction first, because it is sound. Issue #1367 laid out three options and recommended A (add platform: &str to the streaming trait methods, accepting a breaking change). What ships here is more accurate to describe as A's contract shell plus B's message-ID passthrough: streaming_strategy() gives per-channel strategy selection without the breaking parameter, while send_streaming_placeholder() + dispatch_with_response() — core waiting for a GatewayResponse before entering the edit loop — is precisely what the issue defined as Option B's mechanism, minus B's per-platform inner adapter. That combination is a good trade, and the default impls mean no existing ChatAdapter implementation changes behavior. dispatch_with_response subscribes before dispatching, so there is no lost-response race. Every CardOutcome arm in handle_card_edit emits a response, so the 5s wait cannot stall the loop on a rate-limited frame. try_send_initial_card falling back to the post path still yields a real om_ id.

Two things need to change before merge, plus two findings that need an answer.

Blocking 1 — three claims in the PR description are not true of the code

Each is independently checkable without a Feishu tenant:

1. "Late edits after finalize fall back to delete-and-resend (existing #1122 recovery path)." handle_card_edit's Existing::Finalized arm calls emit_response(..., true, Some(om_post), None) — success: true. Core's edit_message therefore returns Ok, and the finalization branch in AdapterRouter only deletes-and-resends inside if let Err(e) = adapter.edit_message(...). For the reaper's own finalize, that recovery never runs. (It does run for CardOutcome::Failed, so the sentence is right for hard API failures and wrong for idle finalize — which is the case the paragraph is actually about.)

2. "main.rs: Passes streaming_strategy into the ACP turn loop." streaming_strategy has zero occurrences in src/main.rs at this head. The file's only changes are a comment relocation and the GatewayResponse filter in the event bridge. The threading happens entirely inside AdapterRouter; no main.rs change was needed. Please drop or correct the claim.

3. "Only send_streaming_placeholder waits for the platform response." edit_message also waits, on every frame, whenever uses_feishu_card_streaming(&msg.channel) is true — that is the point of the change and it is correct behavior, but it makes the "Accepted Residual Risks" statement about send_message remaining fire-and-forget read as broader than it is. Worth restating as "ordinary send_message remains fire-and-forget; the streaming placeholder and card-mode edits both wait."

Blocking 2 — unified never overrides delete_message, so the recovery this PR depends on cannot complete

UnifiedGatewayAdapter has no delete_message implementation, so it inherits the trait default at adapter.rs:396: self.edit_message(msg, "\u{200b}"). In card mode that edit is routed straight back into handle_card_edit — it does not delete anything. Depending on session state it either overwrites the card with a zero-width space or reports success and does nothing.

The standalone adapter already solved exactly this, and its own comment states why:

Override default delete_message (which falls back to edit-to-zero-width) so platforms with native delete APIs (e.g. Feishu DELETE /im/v1/messages/{id}) can perform real deletions. Critical for the streaming-edit-cap recovery path … The default zero-width-edit fallback would itself fail on a cap-reached message, leaving the placeholder visible.

Since this PR is what makes core's edit errors reachable in unified mode at all, it is also what makes the missing override matter. Without it, every Err path that ends in delete-then-resend leaves the original card in place and appends a second copy of the reply. Mirroring the standalone override on UnifiedGatewayAdapter closes it.

This also constrains the fix for F1 below: reporting failure from a finalized session without a working delete would produce a stale card sitting above a fresh full reply. The two have to land together.

F1 — the idle reaper finalizes mid-turn and the reply is silently dropped

Mechanism, all three links checkable in-tree:

• Core's cosmetic edit loop issues an edit only when the rendered text actually changes (buf_rx.has_changed() then content != last). Nothing is sent while a tool runs and produces no display change.
• FeishuStreamRegistry::idle_keys selects sessions with sequence > 0 && is_idle(idle_ms), and card_idle_finalize_ms defaults to 3000. run_idle_reaper then rebuilds the card as static and calls mark_finalized.
• Every later edit lands on Existing::Finalized, which reports success (Blocking 1, item 1). Core sees Ok, so the end-of-turn delivery does not fall back either.

So one tool call longer than 3s closes the stream mid-turn, and the answer that follows it is discarded with no signal in the client or the logs.

Reproduced on PR head 55df8b6d merged with main at 53061d69 — the four files carrying this change (adapter.rs, unified_adapter.rs, feishu.rs, feishu_card.rs) are byte-identical to the PR head in that tree — with FEISHU_CARD_STREAMING_MODE=card on a real Feishu tenant over WebSocket. Prompt: run sleep 15 via shell, then write ~400 words plus a markdown table.

01:57:30  feishu first reply sent directly as card   card_msg_id=om_x100…c0a
01:57:33  feishu card stream updated                 card_id=7668505399948233666 seq=1
01:57:37  feishu card stream finalized (idle)        <- reaper; sleep 15 still running
01:57:37–01:58:10  agent emits 315 agent_message_chunk updates
01:58:10  turn ends

No further card stream updated, no warning, no error, no delete, no fresh send. The agent produced 892 characters including the complete table; the card, read back from the API, contains exactly 🔧 Running: sleep 15.... Precisely: what is lost is the first delivery chunk, i.e. the whole reply whenever it fits in one message. Content beyond message_limit still reaches the user, because the finalization path sends chunks.iter().skip(1) via send_message.

Scope. The Existing::Finalized arm and the reaper are pre-existing #1159 design, not introduced by this PR. Standalone gateway card mode can reach the same state, though only when [gateway] streaming = true — which is not the default (GatewayConfig.streaming defaults to false). What belongs to this PR is turning the path on for unified mode, which is the point of the change, and describing the risk backwards.

On the fix, and the order matters. Reporting success: false from Existing::Finalized is a safety net, not the fix — and on its own it converts a truncated card into a stale card plus a duplicate plain-text reply (see Blocking 2). The root cause is that the reaper infers turn boundaries from idleness, while the fact of a turn ending is known only to core, and the core→gateway command vocabulary has no terminal signal for the card lifecycle. This PR extends that lifecycle to start and update; the missing third state is finish. Note the trait already has a stream_finish seam (adapter.rs:429, currently used only on the native-streaming path) — an explicit finish command, or simply not finalizing while a turn is still open, addresses the cause. Then success: false plus the delete_message override from Blocking 2 becomes a correct backstop rather than a new failure mode.

F2 — post / auto defaults do change, contrary to the description

The body states default behavior is unchanged and that card mode is the only opt-in. For post and auto in unified mode that does not hold:

• unified = ["telegram", "line", "feishu", …], so a unified binary always carries the telegram feature, and TELEGRAM_RICH_MESSAGES resolves via unwrap_or(true). With neither that variable nor TELEGRAM_STREAMING set, UnifiedGatewayAdapter::use_streaming returns true.
• Before this PR that made Feishu take the streaming path with show_streaming_placeholder() == false, i.e. StreamingStrategy::Draft. After it, streaming_strategy returns Disabled for any mode other than card.
• streaming does not only gate the placeholder: keep_full_text = streaming || narration_display. So the delivered message changes from the full turn buffer to the final answer block only — inter-tool narration disappears for post and auto users who never opted into anything.

I think the new behavior is the better one — send-once should not replay narration the user never saw stream. The issue is that it is a user-visible change presented as a no-op, and the new tests cannot catch it because AppState::test_default sets telegram_rich_messages: false, which is the opposite of the from_env default. Suggest saying so in the body and adding a case with telegram_rich_messages: true asserting the intended post/auto strategy.

What's good

• Additive trait extension with defaults; streaming_strategy_preserves_legacy_placeholder_contract pins the legacy mapping down.
• Subscribe-before-dispatch, correlation by exact request_id, ordinary send_message untouched.
• Filtering openab.gateway.response.v1 out of the unified inbound bridge is the right seam, and it is a necessary companion to introducing request_id on this path — before this change unified produced no responses at all. GatewayResponse cannot swallow inbound events either, since request_id and success are required fields absent from the event schema.
• Deriving streaming from a single strategy value instead of two independent predicates removes a real footgun for the next multiplexed adapter.
• The CardOutcome::Updated log is what made the timeline above legible.

[wangyuyan-agent]

This path inverts three deliberate choices in the standalone equivalent. GatewayAdapter::edit_message uses EDIT_RESPONSE_TIMEOUT_MS = 800, returns Ok(()) on timeout — with the comment "Treat as success to avoid false-positive ❌; the cap-reached path already short-circuits much faster" — and returns Ok(()) when the response channel closes. dispatch_with_response uses 5000ms and turns both timeout and closed-channel into Err.

Consequences, in order of likelihood:

• A merely slow tenant (not a failing one) now produces an error at end-of-turn, which drives the delete-and-resend path: the card is dropped and the reply is re-sent as plain text. That is the false positive the standalone comment was written to avoid, and it is worse here because it discards a rendered card.
• On the placeholder specifically, ? lets a cosmetic element abort the whole turn: the caller surfaces an error to the user instead of degrading. send_streaming_placeholder failing is a good reason to fall back to StreamingStrategy::Disabled (send-once), not to fail the turn.
• A timeout on the placeholder also leaves an orphan: the card exists with … and its session sits at sequence == 0, which idle_keys deliberately skips, so the reaper will never finalize it. It stays until FIFO eviction.
• RecvError::Lagged(_) => continue drops frames silently; if the matching response is in a dropped frame the wait necessarily runs the full 5s and then reports failure. Worth aligning the timeout with the standalone value, or at least documenting why card mode needs 5s where post mode needs 800ms.

Measured for margin: creating and sending the initial card took ~1.7s of the 5s budget in a healthy local run (two REST round-trips).

[wangyuyan-agent]

Two small things.

The name covers only one of the two uses — next_request_id() also produces the synthetic message_id returned by send_message and send_message_with_reply.

And saturating_mul is unreachable defense here: nanos-since-epoch is ~1.8e18, so the product is ~1.8e24 against a u128 ceiling of ~3.4e38. If it ever did saturate, every id would collapse to one value — worse than overflowing. Uniqueness, incidentally, does not come from the 1e6 slot: REQUEST_SEQUENCE never resets, so it will eventually exceed 1e6 and spill into the next timestamp's band. It holds because the timestamp is non-decreasing while the counter strictly increases, so t*1e6 + s is strictly increasing overall. A comment recording that reasoning, or checked_mul with an explicit branch, would be clearer than a saturating op that cannot fire.