Skip to content

Commit 5c1251a

Browse files
wangyuyan-agent超渡法師chaodu-agent
authored
fix(streaming): recover from Feishu 20-edit cap (errcode 230072) (#1122)
* fix(streaming): recover from Feishu 20-edit cap (errcode 230072) Feishu's PATCH /im/v1/messages/{id} caps each message at 20 edits (errcode 230072). OAB streaming flushed cosmetic edits at ~1.5s intervals, exhausting the quota on long replies and silently failing with set_done (success reaction) over half-truncated content. Recovery now: track edit count per message, detect cap (pre-check + on-wire 230072), abort cosmetic loop after 3 consecutive cap failures, and finalize by deleting the half-edited placeholder (via native Feishu DELETE — not subject to the 20-edit cap) and posting full content as fresh messages. Cross-layer changes: - src/adapter.rs: streaming finalize sites now feed a delivery_failed flag; closure returns Err when delivery incomplete so dispatch surfaces error reaction instead of false success. - src/gateway.rs: edit_message carries request_id + 800ms timeout so gateway failures (cap_reached) reach core; delete_message override added. - gateway/src/adapters/feishu.rs: edit_counts tracker, EditOutcome enum, handle_reply edit_message reports cap as success=false (no per-flush spam), new delete_feishu_message via native DELETE API. * fix(streaming): address chaodu-agent review for Feishu edit cap PR Addresses all 10 findings from #1122 (comment) . src/adapter.rs (F1, F2, F6) - F1: draft-placeholder fallback chunks now propagate `delivery_failed` on `send_message` failure; previously a `let _ =` swallowed the error and dispatch reported 🆗 over an undelivered turn. - F2: Discord bot-mention path's first chunk now matches Ok/Err and feeds `delivery_failed` on Err, mirroring every other delivery path. - F6: cosmetic edit loop's `tracing::debug!` / `tracing::warn!` now include `message_id` and `platform` fields so concurrent streams can be correlated in logs. src/gateway.rs (F5) - F5: `edit_message` request/response cycle (800ms timeout) is now gated on `platform == "feishu"`. LINE / Teams / other adapters that don't emit `GatewayResponse` for edits go back to the original fire-and-forget path with zero added latency. The Feishu adapter is the only one with a known per-message edit cap and the only one that emits a response for edits today; other platforms can opt in by extending the allowlist when they wire request/response feedback. gateway/src/adapters/feishu.rs (F3, F4, F7, F8, F10) - F3: 230072 detection (`is_feishu_cap_reached_body`) trusts the JSON `code` field when the body parses as JSON; substring matching of "230072" / "number of times it can be edited" is only used as a fallback for non-JSON bodies (proxy HTML, truncated responses). A JSON body whose unrelated `msg` happens to contain "230072" no longer false-positively flags the message as cap-reached. - F4: introduce `EditCountsCache` (HashMap<String, u32> + VecDeque<String>) for FIFO insertion-order eviction. The previous strategy sorted by count ascending and evicted lowest-count entries first, which targeted active streams (low count = just started). FIFO never evicts active streams from under themselves. - F7: `EditOutcome::CapReached` doc and the on-cap `tracing::warn!` no longer claim an "append-new fallback" — the gateway intentionally does not append-new (would spam 20+ duplicates per long reply); recovery is owned by core's finalize path. - F8: `is_valid_feishu_message_id` shape guard checks `^om_[A-Za-z0-9_]+$` with length ≤128; `delete_feishu_message` rejects non-conforming ids before URL interpolation. Defence in depth — trust boundary remains the core↔gateway WebSocket. - F10: 9 new unit tests covering 230072 JSON/substring detection, cap pre-check thresholds, sentinel-no-double-increment, FIFO eviction (active-stream survival regression guard), and message_id shape validation. Verification - cargo build --release: core + gateway clean - cargo test --bins: 504 passed core / 195 passed gateway (+9 new) - cargo clippy --release --bins -- -D warnings: clean PR body has been updated separately to reflect the new edit-counts type signature, the JSON-first 230072 detection, the platform-gated edit response, and the updated `let _ =` accounting (delivery paths all covered; three remaining sites are non-delivery cosmetic ones). * fix(streaming): harden Feishu edit-cap path from internal review Second-round hardening after an internal architecture/bug review of the chaodu-agent fixup. No behavioural regression in the previous round; these close latent issues surfaced by the fixes themselves. gateway/src/adapters/feishu.rs - edit_feishu_message now decides success/failure on the response *body* `code` (Feishu's HTTP-200 + business-code convention), consistent with token refresh and the WS endpoint in this file. Previously it gated success on HTTP status alone, so a `230072` (or any error) returned with HTTP 200 would have been miscounted as a successful edit and never reached cap detection. `is_feishu_cap_reached_body` is now the sole cap authority, checked before success/failure classification. - Lift message_id shape validation to the handle_reply dispatch seam so it covers every command that interpolates `reply.reply_to` into a REST URL path (edit_message / delete_message / add_reaction / remove_reaction), not just delete. Removed the now-redundant inner check in delete_feishu_message. - Fix a stale header comment on the edit_message branch that claimed an append-new fallback (the opposite of what the code does). - 3 new integration tests (wiremock) proving cap detection is reached through the status gate: HTTP 200 + {code:230072} → CapReached + sentinel; HTTP 200 + {code:0} → Edited + count incremented; HTTP 200 + {code:99991} → Failed, no increment. (The earlier unit tests only exercised the detector in isolation, never through edit_feishu_message's status handling.) - nits: as_i64 (was as_u64) in the cap detector to match the file convention; soften the over-absolute "active streams are never evicted" doc wording. src/adapter.rs - Capture the cosmetic edit loop's JoinHandle and abort+await it at finalize, right after dropping the watch sender. Without this join, a cosmetic edit issued just before channel close could land *after* the authoritative finalize edit and overwrite it with stale mid-stream content — a window the per-edit response wait (feishu) widened. src/gateway.rs - Extract EDIT_RESPONSE_PLATFORMS + platform_acks_writes() to replace the hardcoded `platform == "feishu"` gate, with a tech-debt note that this is platform-identity standing in for a capability (proper fix is a connect-time capability handshake). Note: an internal review also raised wiring delete_message request/response so core could observe delete outcome, but that was deliberately not adopted — the recovery path sends fresh content regardless of whether the delete landed, so it would only buy a log line at the cost of a per-finalize wait. delete_message stays fire-and-forget. Verification - cargo build --release: core + gateway clean - cargo test --bins: core 504 / gateway 200 (+3 integration, +2 seam tests) - cargo clippy --release --bins --tests -- -D warnings: clean - live E2E (Feishu): both cap paths confirmed — preemptive local count (18) and on-wire 230072 — each driving the full recovery chain (cosmetic 3-strike abort -> finalize delete placeholder -> send fresh, no overlap). Confirmed Feishu returns 230072 as HTTP 400, so R1 is consistency/robustness hardening rather than an active-bug fix. * fix(streaming): add platform/message_id context to finalization warn logs Addresses review nit F1: all tracing::warn! sites in the streaming finalization path now include platform and message_id fields for cross-platform ops traceability. * fix(streaming): address review nits — doc comment, draft log level, trace context - Add doc comment to edit_feishu_message explaining EditOutcome, preemptive cap check, and body-code-first detection (F1) - Downgrade 'draft' message_id shape-validation log from warn to debug; unknown invalid IDs still warn (security-relevant) (F2) - Add message_id to 'native overflow chunk send failed' warn for consistency with all other delivery-failure traces in scope (F7) --------- Co-authored-by: wangyuyan-agent <wangyuyan-agent@users.noreply.github.com> Co-authored-by: 超渡法師 <chaodu-agent@openab.dev> Co-authored-by: chaodu-agent <chaodu-agent@users.noreply.github.com>
1 parent 4195802 commit 5c1251a

4 files changed

Lines changed: 1054 additions & 50 deletions

File tree

gateway/Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)