Skip to content

feat(interactive-card): client fallback to finalize stuck progress cards (WS-99) - #1035

Open
pkuWMH wants to merge 2 commits into
Mininglamp-OSS:mainfrom
pkuWMH:agent/ken/dd556f64
Open

feat(interactive-card): client fallback to finalize stuck progress cards (WS-99)#1035
pkuWMH wants to merge 2 commits into
Mininglamp-OSS:mainfrom
pkuWMH:agent/ken/dd556f64

Conversation

@pkuWMH

@pkuWMH pkuWMH commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

What & why

Client-side fallback (方案 C) for stuck agent progress cards. When an assistant framework (openclaw / cc / hermes / codex) forgets to send the terminal frame for its type=17 agent_progress_v1 card, the card stays stuck on "🤖 正在处理…" even though the assistant already delivered its type=1 final text. This makes octo-web close those zombie cards on its own.

Root cause fix lives on the sender side (WS-97 Sub-1, openclaw-channel-octo); this is the defensive client backstop.

Behavior

On receiving a type=1 final text (non-stream, non-empty, not self):

  • find that assistant's most recent type=17 card, matched strictly by senderId;
  • if it's a non-terminal progress card (header starts with 🤖 / ⏳) and has been idle ≥ 3s since its last frame,
  • overlay it in the render layer as "⚠️ 已完成(未收到显式终态)", swap the running-step ⏳ for ⚠️, dim the in-progress visuals, and add a "兜底触发 / Client fallback" tag.

Design notes

  • No message-data mutation. Detection sets a local localFallbackApplied flag on MessageWrap; the card tree and server content are untouched, so the sender-side fix (Sub-1) never conflicts — a real terminal frame just renders normally.
  • Conservative triggering. Terminal / waiting headers (✅ ⚠️ ❌ ⏹ ⏱ ⏸) and unrecognized headers never trigger. The 3s idle guard avoids interrupting fast action sequences.
  • Multi-assistant safe. Only the most recent card from the same sender is considered.
  • Trigger is logged via console.debug for troubleshooting.

Tests

  • agentProgressFallback.test.ts — header extraction + terminal/non-terminal/idle logic (11 cases).
  • agentProgressFallback.dom.test.ts — jsdom DOM overlay (⏳→⚠️, marker class, no-op without timeline).
  • Full InteractiveCard suite green (25 files / 242 tests).

Scope

Only octo-web. No IM server changes, no message-type changes. iOS / Android (Sub-3 / Sub-4) will follow this behavior once stable.

Closes #1034

When an assistant sends a type=1 final text but its most recent type=17
agent-progress card is still non-terminal (header starts with 🤖/⏳) and
has been idle for >=3s since its last frame, downgrade that card in the
render layer to "completed (no terminal update received)": swap the
running-step ⏳ for ⚠️, dim the in-progress visuals, and show a
"client fallback" tag. Matches strictly by senderId so concurrent
assistants are never mis-closed, and only overlays local UI state without
mutating message content.

Co-authored-by: multica-agent <github@multica.ai>
@pkuWMH
pkuWMH requested review from a team as code owners July 23, 2026 09:45
@github-actions github-actions Bot added the size/L PR size: L label Jul 23, 2026

@Jerry-Xin Jerry-Xin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client-side fallback that finalizes stuck agent_progress_v1 (type=17) cards when the sender omits the terminal frame. Detection is conservative and display-only (no message/server mutation), but the local fallback flag is never cleared when a genuine terminal frame later arrives — leaving a "no explicit terminal" banner stuck on a legitimately-completed card.

🔴 Blocking

Sticky fallback banner on late-arriving genuine terminal frame — the reconcile the PR is meant to handle is not fully closed.

  • packages/dmworkbase/src/Components/Conversation/vm.ts:1491 sets m.localFallbackApplied = true and it is never reset (only set-once; the only other reference at vm.ts:1484 is the idempotency early-return).
  • When the real server terminal frame arrives later via updateMessageByMessageExtras (vm.ts:1504), it updates remoteExtra and re-stamps progressUpdatedAtSec but does not clear localFallbackApplied.
  • The terminal frame keeps metadata.octo_layout === "agent_progress_v1", so isAgentProgressCard stays true. InteractiveCardCell.isFallbackFinalized = localFallbackApplied && isAgentProgressCard(card) therefore stays true.
  • Result: after the server's real "✅ 已完成" frame renders, the wk-interactive-card-fallback banner ("⚠️ 已完成(未收到显式终态)" + "Client fallback" tag) remains displayed on a card that genuinely reached its real terminal state. This is exactly the client-fallback vs late-server-terminal race, and here server does NOT fully win the annotation.
  • Fix: when a frame refresh makes the card terminal (header hits a terminal marker) or on any updateMessageByMessageExtras re-stamp for that card, clear localFallbackApplied so the fallback annotation is retracted once a real terminal frame arrives.

💬 Non-blocking

  • vm.ts:1489 idle guard reads progressUpdatedAtSec, stamped only via stampProgressCardArrival on WS arrival / extras update. For a card loaded from history (first-page load, no live arrival stamp), progressUpdatedAtSec is undefined and isProgressCardIdleEnough(undefined, ...) returns false — so a card that was already stuck before the session opened will not be finalized by a subsequently-received final text. Acceptable/conservative, but worth a comment noting history-loaded stuck cards are out of scope.
  • Consider adding one test that asserts the flag is cleared (or the banner retracted) once a terminal frame arrives, to lock the reconcile behavior above.

✅ Highlights

  • Display-only by design: only a local MessageWrap flag (Model.tsx) is set; card tree and server content are untouched — no forged terminal state is sent back to server or other clients (trust/data-integrity: clean).
  • Conservative triggering: strict senderId match on the sender's most-recent type=17 card, terminal/waiting markers (✅ ⚠️ ❌ ⏹ ⏱ ⏸) never trigger, sessions_yield correctly excluded, unrecognized headers do not trigger, plus a 3s idle guard against fast action sequences.
  • No spinner/flash regression: the fallback path sets a flag + rebuilds; the render key includes card JSON and an fb: marker so it re-mounts at most once and never toggles a global loading/full-screen Spin state.
  • XSS-safe: header text is read as plain strings and the DOM overlay only rewrites text nodeValue (⏳→⚠️); no dangerouslySetInnerHTML / innerHTML sink.
  • Good idempotency (vm.ts:1484) and solid unit + jsdom DOM coverage of header extraction, terminal/non-terminal/idle logic, and the no-op-without-timeline path.

mochashanyao
mochashanyao previously approved these changes Jul 23, 2026

@mochashanyao mochashanyao left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Octo-Q · automated review]

Verdict: Approve — no blocking findings; notes below (data-flow traced).


Reviewer: Octo-Q (automated review)
Head SHA: ecdca31

Code Review — PR #1035 (octo-web)

Summary

This PR adds a client-side fallback to finalize stuck agent progress cards (type=17) when the sending framework fails to emit a terminal frame. When a type=1 (final text) message arrives from the same assistant, the client checks whether the most recent progress card is still in a non-terminal state and has been idle long enough, then downgrades its display to "Completed (no terminal update received)" with visual de-emphasis. The implementation spans pure-judgment helpers (agentProgressFallback.ts), ConversationVM integration, SDK DOM overlay, CSS, i18n, and unit tests.

The design is conservative and well-guarded: every edge (null header, unknown emoji, non-progress card, self-sent text, streaming chunk) falls through to "no fallback" rather than risk a false positive. The fallback is strictly local UI state — it never writes back to message content or server.

Verification

  • Data flow: VM→Cell agreement — both resolveProgressCard in vm.ts and isFallbackFinalized in InteractiveCardCell.tsx call resolveEffectiveCardContent() then isAgentProgressCard() on the same MessageWrap, so they always agree on which card qualifies.
  • Rendered key invalidation — the "fb:" prefix in syncSdkCard() (InteractiveCardCell.tsx:190) correctly forces a full SDK re-render when fallback state changes, preventing stale DOM.
  • No race between React render and SDK DOMcomponentDidUpdate()syncSdkCard()renderOctoCard()enhanceRenderedOctoCard() is synchronous; applyAgentProgressFallbackVisual runs on the freshly mounted SDK DOM.
  • IdempotencylocalFallbackApplied flag at vm.ts:1484 prevents repeated fallback application; the return at :1498 ensures at most one card per invocation.
  • Real terminal update compatibilityupdateMessageByMessageExtras overwrites remoteExtra independently of localFallbackApplied, so a later server-side terminal frame renders correctly through the normal path.
  • streamNo streaming guard — uses the same canonical field as the message listener at vm.ts:892; consistent streaming-chunk detection.
  • Input-shape defensivenessgetProgressCardHeaderText returns null for every structural anomaly (missing body, no ColumnSet, empty columns/items); isNonTerminalProgressCard returns false for unrecognized headers; isProgressCardIdleEnough returns false for non-numeric timestamps.
  • i18n parity — both en-US.json:982-983 and zh-CN.json:982-983 add the same two keys (fallbackFinalized, fallbackTag).

Static analysis only at head ecdca3133e; build and tests not executed in this environment.

Findings

No P0/P1 issues. Three P2 items below.

P2 — Dead CSS class on timeline container (packages/dmworkbase/src/Messages/InteractiveCard/sdk/agentProgressFallback.ts:10)

wk-interactive-card-progress--fallback is added to #timeline_detail via classList.add() at :15 and the header comment says it enables style de-emphasis of in-progress visuals. However, no CSS rule for this class exists in index.css or anywhere else in the codebase. The outer wrapper opacity change works via the separate wk-interactive-card-sdk--fallback-finalized class applied by InteractiveCardCell.tsx:511, so the user-visible effect is partial — the mount container fades to 0.92 opacity, but the timeline interior (spinners, step glyphs) gets no additional visual treatment. Either add the intended CSS rule to suppress spinner animations or further de-emphasize the timeline, or remove the classList.add call and the AGENT_PROGRESS_FALLBACK_CLASS export if no inner styling is planned.

Diff-scope: new (introduced by this PR).

P2 — progressUpdatedAtSec lost on reconnect or history reload (packages/dmworkbase/src/Components/Conversation/vm.ts:1459)

stampProgressCardArrival is called only on real-time message arrival (:919) and extras-patch updates (:1514). History-load paths (loadMessages at :~1862, pulldownMessages at :~2163, pullupMessages at :~2206) create fresh MessageWrap instances via toMessageWraps() without stamping. If a reconnect or page reload occurs between a progress card's last frame and the arrival of a final text, progressUpdatedAtSec will be undefined on the new MessageWrap, isProgressCardIdleEnough returns false, and the fallback silently does not fire. The card remains visually stuck until the server sends an explicit terminal frame. This is likely acceptable given the server-side fix (WS-97 Sub-1) is the authoritative path, but consider either stamping from a server-provided timestamp during history load or documenting this as a known scope limitation.

Diff-scope: new (this PR introduces the stamping mechanism; the gap is in the new code's coverage, not pre-existing behavior).

P2 — Only the most recent stuck card per sender receives fallback (packages/dmworkbase/src/Components/Conversation/vm.ts:1479)

The backward scan through messagesOfOrigin stops at the first type=17 card from the matching sender and returns unconditionally after processing it (:1498). If an assistant emitted multiple progress cards and more than one is stuck in a non-terminal state, only the most recent one gets the fallback visual. Earlier stuck cards remain indefinitely in their "processing" display. Each subsequent final-text message from the same sender would clear at most one card. If multi-card stuck scenarios are realistic, consider continuing the scan past already-fallback or already-terminal cards to check earlier ones. If single-card scope is intentional, a brief inline comment would prevent future confusion.

Diff-scope: new (introduced by this PR; the single-card-per-scan design is a choice in the new code).

Data Flow Backtracking

Consumed data Upstream source Verified flow
progressUpdatedAtSec stampProgressCardArrival sets Date.now()/1000 on MessageWrap ✅ Real-time path; ⚠️ History paths create fresh MessageWrap without stamping
localFallbackApplied maybeFinalizeStuckProgressCard sets true on MessageWrap ✅ Consumed by isFallbackFinalized in Cell; not cleared by terminal update (harmless — idempotency guard only)
decision.card computeState()resolveEffectiveCardContent()effective.card ✅ Both VM and Cell use identical resolution; agreement confirmed
isAgentProgressCard result cardLayout.ts:3-11 checks metadata.octo_layout === "agent_progress_v1" ✅ Correct discriminator; not structural
headerText getProgressCardHeaderText walks body→ColumnSet→columns[0]→items[0] ✅ Returns null on structural anomaly; conservative
fromUID MessageWrap.fromUID getter → message.fromUID (SDK field) ✅ Always set for server-delivered messages
streamNo message.streamNo (SDK field) ✅ Canonical streaming detection; consistent with listener gate at :892

Blindspot Checklist

  • C1 (Dual-path parity): N/A — no paired add/remove or subscribe/unsubscribe paths changed.
  • C2 (Control-flow ordering/nested reuse): Clear — maybeFinalizeStuckProgressCard is called once per new message arrival; no nested/double-invocation risk.
  • C3 (Authorization boundary): N/A — no auth/permission changes; fallback is purely client-side UI state.
  • C4 (Auth lifecycle/container-member): N/A — no auth changes.
  • C5 (Build/note ≠ runtime): Clear — CSS classes verified against actual CSS rules; one gap found (P2 above).
  • C6 (Governance/docs self-consistency): N/A — no governance/docs changes.

Cross-round Blocker Check

N/A — first review round.

Things I checked that are fine

  • isAgentProgressCard (cardLayout.ts:3-11) correctly discriminates on metadata.octo_layout === "agent_progress_v1", not on structural fields — robust against body shape variations.
  • resolveEffectiveCardContent (resolveContent.ts:28-37) is a full-frame replacement (edit frame or original), not a merge — no risk of partial metadata leakage.
  • elementText (agentProgressFallback.ts:37-51) handles both TextBlock and RichTextBlock shapes; returns empty string for unknown types.
  • The "fb:" prefix in the rendered key is correctly scoped to only affect the fallback transition, not unrelated card re-renders.
  • MessageContentType.text (1, from SDK) and MessageContentTypeConst.interactiveCard (17, app-level) occupy the same number space without collision.
  • The send getter (Model.tsx:358-360) correctly compares fromUID against WKApp.loginInfo.uid — self-sent text exclusion works.
  • Unit tests (agentProgressFallback.test.ts) cover the pure-judgment functions with proper edge cases including malformed card structures and emoji boundaries.
  • DOM test (agentProgressFallback.dom.test.ts) verifies the glyph replacement and class application on a real DOM tree.

Verdict: APPROVED

Well-structured, defensively coded client fallback that correctly scopes itself to local UI state without writing back to message data. The three P2 items are non-blocking: a dead CSS class (cosmetic gap), a known-scope limitation around reconnect, and single-card-per-sender fallback scope. None of these produce user-visible incorrect behavior beyond the stated design intent, and the server-side fix remains the authoritative terminal-state path.

[Octo-Q] verdict: APPROVE — No P0/P1 issues found. Three P2 observations are non-blocking design-scope items. Code is well-guarded, conservative in fallback triggering, and correctly isolated to local UI state.

…terminal frame

The local fallback annotation set on a stuck progress card was never cleared
when the real terminal frame later arrived, so the "no terminal update
received" banner lingered over an already-rendered ✅/⚠️ card. When
updateMessageByMessageExtras refreshes a fallback-marked card and its effective
frame is terminal (terminal header, or no longer an agent_progress card as with
the combined terminal+answer frame), clear localFallbackApplied so the real
terminal state renders. Read receipts and non-terminal re-edits keep the flag,
so read-receipt syncs never drop the banner on a still-stuck card.

Co-authored-by: multica-agent <github@multica.ai>
@pkuWMH

pkuWMH commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the fallback race (2434a3e).

updateMessageByMessageExtras now clears localFallbackApplied when the refreshed card frame is terminal — either a terminal header (✅/⚠️/…) or a card that is no longer agent_progress_v1 (the combined terminal+answer frame strips the layout metadata). So once the authoritative terminal frame lands, the "no terminal update received" banner is retracted and the real terminal state renders.

Scoped deliberately to terminal frames: read-receipt extra syncs (no contentEdit) and non-terminal re-edits keep the flag, so a read receipt on a still-stuck card never drops the banner prematurely.

Added clientFallbackFinalize.test.ts (real SDK + real MessageWrap): apply fallback → terminal frame retracts (both the agent_progress terminal header and the metadata-stripped combined frame); read receipt and non-terminal re-edit do not. Full InteractiveCard + Conversation suites green (369 tests).

@github-actions github-actions Bot added the size/XL PR size: XL label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL PR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

octo-web: 收到 final text 后主动降级同 assistant 未终态进度卡(客户端兜底)

4 participants