Backport: stop false stale-send guard (#2115 + prerequisite #2034) to release/v0.9.x - #2116
Conversation
Fixes a false-positive chat send guard that can show `New messages loaded — please review before sending` during normal same-tab usage after an agent response has already completed and is visible. The send guard now compares backend-derived messages against all locally visible comparable messages instead of only persisted/stored messages. This preserves the intended cross-tab stale-message warning while allowing the next same-tab send to proceed. Fixes #2033. ## Changes - Count all locally visible comparable chat messages when deciding whether the backend has unseen messages. - Apply the same comparable-message filtering to backend-derived messages before comparing counts. - Add mounted regression tests covering: - same-tab completed stream does not block the next send - true cross-tab/unseen messages still block - cross-tab/unseen messages still block after a same-tab stream ## Validation Automated: ```bash cd ui npm test -- --runTestsByPath src/components/chat/__tests__/ChatInterface.sendGuard.test.tsx --runInBand ``` Result: ```text Test Suites: 1 passed, 1 total Tests: 3 passed, 3 total ``` Previously also run successfully during branch validation: ```text src/lib/__tests__/messageHandlers.test.ts: 27 passed Full UI Jest suite: 268 passed ``` Manual: - Deployed/tested locally against `kind-kagent`. - Verified the next same-tab message sends without the false `New messages loaded` toast after a completed agent response. --------- Signed-off-by: Evan Rauner <raunerevan@gmail.com> Co-authored-by: Eitan Yarmush <eitan.yarmush@solo.io> (cherry picked from commit e1422ab) Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
## Description The chat send guard falsely blocked the next message with "New messages loaded — please review before sending" on essentially every turn after the first — sending twice was needed each time. It surfaced first on a tool-call turn, but the root cause affects every agent response. ## Root cause A2A marks per-message `contextId`/`taskId` as optional — the `Task` is the canonical carrier, so persisted history messages routinely store them as empty strings. After a turn completes the UI keeps the locally-streamed message copies (it does not reload from the backend), and those carry the task's real `contextId`/`taskId`, so the send guard keys them as `["task", contextId, taskId, contentSignature]`. The backend-extracted copies, with empty ids, fall back to `["message", <id>]` instead — and for converted tool messages that id is even regenerated (`uuidv4`) on every extraction. So a persisted agent message never matches its locally-streamed counterpart, the guard counts the backend as ahead of the local view, and blocks the send. Every turn has an agent response, so every send after the first tripped it. ## Fix Carry `contextId`/`taskId` on agent messages so the streamed and persisted copies key identically. Two layers: - **UI** (`ui/src/lib/messageHandlers.ts`): when flattening `task.history`, backfill `contextId`/`taskId` from the task for **every** extracted agent message (text and tool), treating `""` as absent (`||`, not `??`). This is the complete fix on its own — it repairs already-persisted sessions and works regardless of which runtime produced them. - **Runtime** (source, defense-in-depth, helps new sessions): stamp `contextId`/`taskId` onto agent messages at emission time, in both producers — Python `kagent-adk` (`convert_event_to_a2a_message`) and the Go ADK executor, now the default declarative runtime (`go/adk/pkg/a2a/executor.go`, via a `newAgentStatusEvent` seam). A2A allows omission, and remote agents are out of our control, so the UI backfill stays regardless. Scope is limited to agent messages; user messages are keyed by `messageId` and already match. ## Reproduction <img width="402" height="301" alt="image (7)" src="https://github.com/user-attachments/assets/136d194d-c195-4ba7-8319-9fc729a5a59d" /> 1. Send a message that triggers a tool call. 2. Send another message → blocked by the stale-send banner with no actual change. --------- Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io> (cherry picked from commit df828c0)
bb0332d to
999bb40
Compare
|
Warning Testing pausedMonthly snapshot limit reached. Update your plan for additional snapshots and to resume testing. |
0fe0390 to
999bb40
Compare
…/backport-send-guard-fix-v0.9.x Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
There was a problem hiding this comment.
Pull request overview
Backports the chat cross-tab stale-send guard fix to release/v0.9.x, preventing the UI from falsely blocking the next send with “New messages loaded — please review before sending” after normal same-tab turns (including tool-call turns). This aligns locally visible messages with backend-derived messages by keying on stable task/context identity and ensuring persisted agent messages carry (or are backfilled with) those ids.
Changes:
- UI: backfill missing/empty
contextId/taskIdon extracted agent history messages and update the send guard to compare backend-derived messages against locally visible messages using stable keys. - UI: add regression tests covering same-tab streaming, cross-tab unseen messages, and tool-call turn persistence.
- Runtime: stamp
contextId/taskIdonto emitted agent messages in both Python and Go producers, with unit tests.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| ui/src/lib/messageHandlers.ts | Backfills agent message ids during task-history flattening; adds comparable/keyed send-guard counting helpers. |
| ui/src/components/chat/ChatInterface.tsx | Switches send-guard inputs from stored-message counts to local visible message matching vs backend snapshot. |
| ui/src/components/chat/tests/ChatInterface.sendGuard.test.tsx | Adds mounted regression tests for send-guard behavior across same-tab, cross-tab, and tool-call scenarios. |
| python/packages/kagent-adk/src/kagent/adk/converters/event_converter.py | Stamps task_id/context_id on converted messages (defense-in-depth for new sessions). |
| python/packages/kagent-adk/tests/unittests/converters/test_event_converter.py | Adds unit test verifying stamped ids on converted messages. |
| go/adk/pkg/a2a/executor.go | Stamps ContextID/TaskID onto emitted agent messages via helper constructors. |
| go/adk/pkg/a2a/executor_test.go | Adds unit tests verifying id stamping on agent messages and status events. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (the task is the canonical carrier), but stamping them lets consumers | ||
| that flatten task.history into standalone messages key each message to | ||
| its task without backfilling. | ||
| context_id: Optional context ID stamped onto the message, as task_id. |
Summary
Backport of the chat send-guard fix to
release/v0.9.x. The send guard falsely blocked the next message with "New messages loaded — please review before sending" on essentially every turn after the first — sending twice was needed each time.This backport is two cherry-picks, in order:
#2034(e1422ab4) —fix(ui): avoid false stale chat send guard. Prerequisite.#2115(df828c0d) —Stop false stale-send guard after tool-call turns. The actual fix.Why #2034 is included (not just #2115)
release/v0.9.xshipped an older, count-based send guard: it blocks whenextractMessagesFromTasks(tasks).length > localMessageCount, comparing raw message counts. On that guard, the just-completed turn lives instreamingMessagesand is not counted inlocalMessageCount(which counts onlystoredMessages) until a block triggers a reload — so every send after the first blocks once, reloads, and succeeds on retry. That is the "send twice every message" symptom.#2115makes streamed and persisted messages key identically (contextId/taskId), which only matters for the key-based guard introduced by#2034. Applied alone to 0.9.x it would be a no-op (backfilling ids does not change a count). So#2034is cherry-picked first to bring the key-based guard, then#2115fixes it.Note for reviewers:
#2034is a behavioral change to the guard (count-based → key-based comparison of comparable messages), not only a bugfix. Both cherry-picks applied with no conflicts (0.9.x'sChatInterface.tsx/messageHandlers.tswere identical to#2034's parent).What #2115 fixes (on top of #2034)
Persisted agent messages omit
contextId/taskId(A2A optional fields; the task is the canonical carrier). The locally-streamed copies carry the task's real ids, so the key-based guard keys them as["task", …]while the backend-extracted copies fall back to["message", <id>](regenerated for tool messages). They never match, the backend looks ahead of the local view, and the send blocks on every turn with an agent response.The fix carries the ids on agent messages so the copies key identically:
ui/src/lib/messageHandlers.ts): when flatteningtask.history, backfillcontextId/taskIdfrom the task for every extracted agent message (text and tool), treating""as absent. Repairs already-persisted sessions regardless of producer.kagent-adk(convert_event_to_a2a_message) and the Go ADK executor (go/adk/pkg/a2a/executor.go).