Skip to content

Route active Feishu card text sends#500

Closed
zhulijin1991 wants to merge 1 commit into
larksuite:mainfrom
zhulijin1991:codex/lark-active-streaming-card
Closed

Route active Feishu card text sends#500
zhulijin1991 wants to merge 1 commit into
larksuite:mainfrom
zhulijin1991:codex/lark-active-streaming-card

Conversation

@zhulijin1991
Copy link
Copy Markdown
Contributor

Summary

  • Track active streaming cards by session and target while a controller is live.
  • Route same-session text-only sends into the active card instead of creating a separate message.
  • Add focused store coverage for active-card routing and cleanup behavior.

Validation

  • pnpm exec vitest run tests/active-streaming-card-store.test.ts
  • pnpm exec tsc --noEmit

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 11, 2026

CLA assistant check
All committers have signed the CLA.

Copy link
Copy Markdown
Collaborator

@evandance evandance left a comment

Choose a reason for hiding this comment

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

Thanks for picking this up — the bug is real (agent finishes with both a "running" streaming card and a separate final-text message). Worth fixing.

But I think this is the wrong layer. The agent calls feishu.send at turn-end because as far as it knows, that's how you send text. This PR addresses that by silently rerouting send at runtime — hiding the agent's intent rather than fixing it. Three cleaner places: tell the agent in the tool description not to double-send while a card is active; finalize the card in the dispatcher before letting send go through; or add an explicit feishu.finalizeStreamingCard() action. Runtime routing is the only option where callers have to look at a global lookup to predict what send will do — that's the smell.

Two concrete issues even if we accepted runtime routing as the design.

isCurrentTurnTarget returns true when to is empty, so any feishu.send({ text }) without an explicit target routes into whatever active card the session has. Empty to is a normal case (reply-in-thread, default chat), not an edge — so this silently changes destination on a common path.

activeCards is a module-scope Map, and cleanup depends on four hook paths in reply-dispatcher.ts all firing. There's no integration test for that. The next lifecycle refactor will silently leak the Map.

Before redesigning, could you share a trace or screenshot of the actual scenario, and why you went with runtime routing over the higher layers? That'd anchor the conversation in the real case.

@evandance evandance added feature request New feature or request messaging src/messaging/ + src/card/ — message rendering, cards, streaming changes requested Need do changes labels May 12, 2026
@zhulijin1991
Copy link
Copy Markdown
Contributor Author

Thanks, agreed. The bug is real, but the runtime reroute makes feishu.send too implicit and the empty-to case is a bad default to overload.

The observed scenario was: the agent finished a turn while a streaming card was still active, then sent final text separately, leaving both a running card and a separate final-text message. I will add that trace/screenshot to the PR description before redesigning.

For the fix direction, I will step back from the module-scope activeCards routing and try a higher layer instead: either tighten the tool description/turn policy so the agent does not double-send while a card is active, or have the dispatcher finalize/update the active card before allowing a normal send. That should keep send behavior predictable.

@zhulijin1991
Copy link
Copy Markdown
Contributor Author

Closing this in favor of #506. The replacement keeps send runtime behavior predictable and moves the fix to Feishu message-tool schema guidance instead of process-local active-card rerouting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes requested Need do changes feature request New feature or request messaging src/messaging/ + src/card/ — message rendering, cards, streaming

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants