Skip to content

Stop false stale-send guard after tool-call turns#2115

Merged
EItanya merged 4 commits into
mainfrom
iplay88keys/fix-send-guard-tool-call-false-positive
Jun 30, 2026
Merged

Stop false stale-send guard after tool-call turns#2115
EItanya merged 4 commits into
mainfrom
iplay88keys/fix-send-guard-tool-call-false-positive

Conversation

@iplay88keys

@iplay88keys iplay88keys commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

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

image (7)
  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>
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
@chromatic-com

chromatic-com Bot commented Jun 30, 2026

Copy link
Copy Markdown

Warning

Testing paused

Monthly snapshot limit reached. Update your plan for additional snapshots and to resume testing.

Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
… guard

Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
@iplay88keys iplay88keys marked this pull request as ready for review June 30, 2026 18:09
Copilot AI review requested due to automatic review settings June 30, 2026 18:09

Copilot AI 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.

Pull request overview

Fixes a false-positive “stale-send” guard in the chat UI by ensuring agent messages extracted from persisted task.history key identically to their locally-streamed counterparts (especially after tool-call turns), preventing the next send from being incorrectly blocked.

Changes:

  • UI: backfills contextId/taskId for extracted agent messages (treating "" as absent) so send-guard keys match between streamed and persisted messages.
  • UI tests: adds regression coverage for same-tab tool-call and text turns persisted with empty agent ids.
  • Runtimes (defense-in-depth): stamps task_id/context_id onto emitted agent messages in both the Python ADK converter and the Go ADK executor, with accompanying unit tests.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
ui/src/lib/messageHandlers.ts Backfills agent contextId/taskId when extracting persisted history so send-guard keys align with streamed messages.
ui/src/components/chat/tests/ChatInterface.sendGuard.test.tsx Adds regression tests covering the previously-failing same-tab persisted tool-call and text scenarios.
python/packages/kagent-adk/src/kagent/adk/converters/event_converter.py Stamps task_id/context_id onto converted agent messages at emission time.
python/packages/kagent-adk/tests/unittests/converters/test_event_converter.py Verifies converted messages carry task_id/context_id.
go/adk/pkg/a2a/executor.go Centralizes agent message creation to stamp request ContextID/TaskID on emitted agent messages.
go/adk/pkg/a2a/executor_test.go Tests that the new Go executor seam stamps context/task ids onto agent messages and working 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.
@EItanya EItanya merged commit df828c0 into main Jun 30, 2026
30 checks passed
@EItanya EItanya deleted the iplay88keys/fix-send-guard-tool-call-false-positive branch June 30, 2026 18:26
dimetron added a commit to dimetron/kagent that referenced this pull request Jun 30, 2026
…-widgets

Resolve the send-guard conflict in
ui/src/components/chat/__tests__/ChatInterface.sendGuard.test.tsx by keeping
this branch's high-water-mark guard suite. Upstream kagent-dev#2115 fixed the
content-signature guard's tool-call false positive, but this branch already
replaced that guard with a content-independent high-water mark (counts persisted
history items), which is immune to the same class of false positives — including
the MCP App widget payload serialization differences that regressed the
content-signature guard. The upstream test additions exercise the removed
content-signature helpers, so they are dropped here.

Keep upstream's non-conflicting changes: extractMessagesFromTasks contextId/taskId
backfill (messageHandlers.ts), and the A2A executor / Python event_converter id
stamping (defense-in-depth, harmless under the high-water-mark guard).

Signed-off-by: Dmytro Rashko <dimetron@me.com>
iplay88keys added a commit that referenced this pull request Jun 30, 2026
## 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)
EItanya added a commit that referenced this pull request Jul 1, 2026
… release/v0.9.x (#2116)

## 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.x` shipped an older, **count-based** send guard: it blocks
when `extractMessagesFromTasks(tasks).length > localMessageCount`,
comparing raw message counts. On that guard, the just-completed turn
lives in `streamingMessages` and is not counted in `localMessageCount`
(which counts only `storedMessages`) 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.

`#2115` makes 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 `#2034` is cherry-picked
first to bring the key-based guard, then `#2115` fixes it.

Note for reviewers: `#2034` is 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's
`ChatInterface.tsx`/`messageHandlers.ts` were 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** (`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.
Repairs already-persisted sessions regardless of producer.
- **Runtime** (source): stamp the ids at emission time in both producers
— Python `kagent-adk` (`convert_event_to_a2a_message`) and the Go ADK
executor (`go/adk/pkg/a2a/executor.go`).

---------

Signed-off-by: Evan Rauner <raunerevan@gmail.com>
Signed-off-by: Jeremy Alvis <jeremy.alvis@solo.io>
Co-authored-by: erauner12 <59383957+erauner12@users.noreply.github.com>
Co-authored-by: Eitan Yarmush <eitan.yarmush@solo.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants