Skip to content

fix(cli): preserve intra-turn text/tool order for ACP sessions#505

Merged
tiann merged 1 commit intotiann:mainfrom
junmo-kim:feat/acp-interleave-order
Apr 21, 2026
Merged

fix(cli): preserve intra-turn text/tool order for ACP sessions#505
tiann merged 1 commit intotiann:mainfrom
junmo-kim:feat/acp-interleave-order

Conversation

@junmo-kim
Copy link
Copy Markdown
Contributor

Problem

When using hapi gemini (ACP backend), text chunks that arrive before a
tool call are rendered after all tool blocks in the web UI. A session
producing text → tool_call → tool_result → text arrives at the Hub and
Web in the order tool_call, tool_result, text, so the assistant's
reasoning appears below the tool output rather than above it.

This is an intra-turn ordering issue — distinct from the turn-completion
timing addressed in #356.

Solution

AcpMessageHandler.handleUpdate now calls flushText() immediately
before dispatching a tool_call or plan update — a new tool
invocation or plan closes the preceding text segment.

tool_call_update (tool result / progress) does NOT flush: it is a
lifecycle event on an already-open tool call, not a boundary between
text segments. Flushing there would split a text segment that spans
the tool's execution and break the cumulative-delta dedup inside
appendTextChunk.

The cumulative-delta dedup, audience filtering, rate-limit parsing,
and internal-event suppression in AcpMessageHandler are untouched.
AcpSdkBackend's waitForSessionUpdateQuiet / turn_complete logic
is not modified.

Tests

  • The old ordering test is replaced by an equivalent case asserting
    ['text', 'tool_call', 'tool_result'].
  • Interleave scenarios: text → tool and text → tool → text → tool.
  • Dedup-across-boundary: cumulative dedup still works inside each text
    segment separated by tool events.
  • Regression: text streamed between tool_call and tool_call_update
    is preserved and emitted after tool_result, not duplicated or
    fragmented.
  • All other existing tests (audience filter, rate-limit, internal-
    event, overlap dedup) are unchanged and pass.

Flush buffered assistant text before emitting tool_call and plan
updates so the CLI preserves the order in which an ACP agent produced
them. tool_call_update events do not flush: they are lifecycle events
on an already-open tool call, and flushing there would split a text
segment that spans the tool's execution. Cumulative-delta dedup,
rate-limit, and internal-event filters in AcpMessageHandler are
untouched; only the previously implicit "text is emitted at turn
boundary" contract is relaxed.

Existing tests covering ordering are reworked to assert the new
contract, and new cases cover interleaved text/tool sequences, dedup
across tool boundaries, and dedup preservation when text arrives
between tool_call and tool_call_update.
Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

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

Findings

  • No actionable findings on the added/modified lines.

Summary

  • Review mode: initial
  • No diff-attributable correctness, security, or regression issues found in the latest PR diff.
  • Residual risk/testing gap: not run in automation here; coverage is strong for text -> tool_call -> tool_result ordering and dedup boundaries, but I did not verify against a live ACP provider stream.

Testing

  • Not run (automation)

HAPI Bot

@tiann tiann merged commit 3405b56 into tiann:main Apr 21, 2026
2 checks passed
@junmo-kim junmo-kim deleted the feat/acp-interleave-order branch April 21, 2026 13:30
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.

2 participants