Skip to content

fix(aws-strands): halt the Strands loop on frontend tools instead of muting the wire - #2334

Merged
contextablemark merged 6 commits into
mainfrom
mark/strands-py-halt-stops-loop
Aug 6, 2026
Merged

fix(aws-strands): halt the Strands loop on frontend tools instead of muting the wire#2334
contextablemark merged 6 commits into
mainfrom
mark/strands-py-halt-stops-loop

Conversation

@contextablemark

Copy link
Copy Markdown
Contributor

Opened at @YiyuanMiao's request — this is our half of the Option 1 split agreed in #2172. It carries the two hunks that overlapped their PR (Fix #1 halt-break, Fix #2 keep-message) plus two they deliberately left to us, with tests. Once this is up they'll drop their overlapping hunks and rebase #2172 around it.

The bug

A frontend (proxy) tool returns a successful "Forwarded to client" placeholder server-side, so Strands has every reason to run another model cycle on that placeholder — and another. The adapter gated event emission only, and drained the rest of the run silently (if halt_event_stream: continue).

Measured with a scripted model over the real event loop — two parallel frontend calls plus three retries — 5 model invocations, 4 of them after the halt latched. All invisible to the client:

  • frontend tool calls the client never receives, so it can never answer them
  • real backend tool side effects, repeated
  • a phantom assistant turn persisted to the session store, so the next turn starts from a transcript claiming the question was already answered
  • RUN_FINISHED queued behind that work: 3.2s of dead air at 0.8s/cycle, and single-agent Strands has no cycle cap, so a model that keeps retrying the read yields no terminal event at all (25 cycles in 20s, still running)
  • a fault on an invisible cycle surfaces as RUN_ERROR instead

What changed

1. Stop the loop instead of draining itbreak, plus an explicit aclose() on the halt path. A bare break leaves the async generator suspended at a yield where ag_running is False, so the pre-existing "already exhausted?" guard takes the pass branch, deferring teardown to GC and leaving the model stream open. @YiyuanMiao independently reproduced this; it's why the break alone isn't sufficient.

2. Don't drop the halting batch's backend results. The halt latched on the tool-result message and skipped it wholesale, which in a mixed frontend+backend batch discarded the backend results: TOOL_CALL_RESULT, the MESSAGES_SNAPSHOT splice (the only path into client-side history, since TOOL_CALL_RESULT is emitted role-less by design), and state_from_result / custom_result_handler. No new machinery needed — the per-item loop already skips frontend placeholders; the message just has to reach it. Then break at end of batch.

Safe against the session-manager reconcile: the halt latches only after Strands appended the assistant toolUse + placeholder toolResult, and MessageAddedEvent drives sync_agent as well as append_message, so both the placeholder and the wire→native map are already durable. Verified against a real FileSessionManager.

Measured, before and after

drain this PR
backend TOOL_CALL_RESULT on the wire never emitted
model cycles the client can't see 4 0
backend tool executions per client-visible call
dead air before the terminal 6.92s 0.00s
phantom messages persisted 1 0

Also confirmed live against real gpt-5.4: the model does batch a frontend and a backend tool in a single cycle, and on the released build the backend result never reaches the client with 2.6s of dead air — after this, it's emitted with none.

Tests

12 regression tests driving the real adapter over a real strands.Agent with a stub model (no network, no credentials), so cycle counts are evidence about the loop rather than about a mocked stream. Includes a two-run round-trip that rebuilds client history from the emitted MESSAGES_SNAPSHOT and asserts the replayed transcript has no orphan toolUse — the failure that lands one run later and at the model provider rather than in our code.

5 of the 12 fail against main. The two that pass either way pin already-correct behaviour: parallel batches still emit in full, and continue_after_frontend_call still runs further cycles.

For #2172

Two sub-points @YiyuanMiao deliberately left for this PR because they're entangled with the halt-path teardown: the args_streamer branch still emitting the frontend ToolCallEnd immediately, and moving their safety flush into finally. Both touch the region this PR rewrites — happy to take them here as a follow-up commit, or leave them for #2172 once this lands. Your call, and thanks for the unusually careful review cycle.

🤖 Generated with Claude Code

contextablemark and others added 5 commits August 4, 2026 20:33
… mute the wire

The frontend-tool halt gated event emission only. A proxy tool returns a
SUCCESSFUL "Forwarded to client" placeholder, so Strands kept running model
cycles on placeholder input while the adapter silently discarded their events.

Measured with a scripted model over the real event loop (2 parallel frontend
calls + 3 retries): 5 model invocations, 4 of them after the halt latched.
Costs, all invisible to the client:

  * frontend tool calls the client never receives, so it can never answer them
  * real backend tool side effects
  * a phantom assistant turn persisted to the session store, so the next turn
    starts from a transcript claiming the question was already answered
  * RUN_FINISHED queued behind those cycles — 3.2s of dead air at 0.8s/cycle,
    and single-agent Strands has no cycle cap, so a model that keeps retrying
    the read yields NO terminal event at all (25 cycles in 20s, still running)
  * a fault on an invisible cycle surfaces as RUN_ERROR instead

Stop the loop instead, then close the generator explicitly: a break leaves it
SUSPENDED at a yield where ag_running is False, which the existing
exhausted-generator guard reads as "already closed", deferring teardown to GC
and leaving the model stream open.

Safe against the session-manager reconcile: the halt latches only after Strands
appended the assistant toolUse + placeholder toolResult, and MessageAddedEvent
drives sync_agent as well as append_message, so both the placeholder and the
wire->native map are already durable. Verified against a real
FileSessionManager. This is why the TS abort needed
persistHaltedFrontendPlaceholders and Python does not — TS cancels inside the
tool-execution cycle, before Strands appends.

Adds 7 regression tests driving the real adapter over a real strands.Agent with
a stub model (no network, no credentials). 5 fail without this change; the two
that pass either way pin already-correct behavior — parallel batches still emit
in full, and continue_after_frontend_call still runs further cycles.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…l halts

A parallel batch mixing a frontend tool with backend tools resolves into ONE
tool-result message. The frontend-tool halt latched on that message and skipped
it wholesale, so every backend result in the batch was lost permanently:

  * the client's tool card never resolves — it sits in "running" forever
  * the result never reaches MESSAGES_SNAPSHOT, which is the only path into
    client-side history (TOOL_CALL_RESULT is emitted role-less by design and is
    explicitly NOT history), so consumers that persist from the event stream
    hold a transcript whose toolUse has no toolResult. With
    replay_history_into_strands and no session manager that transcript is
    replayed straight to the model provider, which rejects an orphan tool_use
  * state_from_result and custom_result_handler never fire, so an app mapping
    backend results into shared state loses the update with no error anywhere

None of this needed new machinery: the per-item loop already skips frontend
placeholders. The message just has to be allowed to reach it. Also break at the
end of the batch rather than relying on the check at the top of the loop, so
termination doesn't depend on Strands yielding one more event after it.

Measured on a repro of the reported stream (frontend get_current_canvas_yaml +
backend aws___run_script in one batch), across the three builds:

                          drain     halt-only   halt + this
  backend TOOL_CALL_RESULT  never     never       emitted
  backend executions        3x        1x          1x
  dead air before terminal  6.92s     1.51s       0.00s

Adds 4 regression tests, all failing before this change. One of them pins an
interaction the change makes reachable: stop_streaming_after_result halts from
inside the per-item loop, and that loop now runs on a halting batch where it
previously could not, so its suppression of later items must still hold.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…oisons

The halting-batch tests assert on the events one run emits. They cannot see the
failure that lands one run LATER and at the model provider rather than in our
code: with no session manager the adapter does `strands_agent.messages =
_build_strands_history(input_data.messages)`, and that builder transcribes
straight through with no orphan handling. Whatever the client persisted becomes
the literal transcript, so a dropped backend result means turn 2 carries a
toolUse with no toolResult — which Bedrock and Anthropic both reject.

Drives two real runs through the adapter. Turn 1 is the halting mixed batch;
client-side history is then rebuilt FROM the emitted MESSAGES_SNAPSHOT rather
than hand-written, so the test asserts the property that actually matters — the
event stream carries enough to reconstruct a servable transcript — instead of
begging the question. Turn 2 replays it and captures what the stub model
receives, asserting no orphan toolUse and that the backend result is present so
the model can answer from it rather than re-running the tool.

Fails against the previous commit with: toolUse with no toolResult:
['run_script'].

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Python Preview Packages

Version 0.0.0.dev1786000891 published to TestPyPI.

Warning: These packages are built from contributor code that may not yet have been vetted for correctness or security. Install at your own risk and do not use in production.

Install with uv

Add the TestPyPI index to your pyproject.toml:

[[tool.uv.index]]
name = "testpypi"
url = "https://test.pypi.org/simple/"
explicit = true

Then install the packages you need:

# Core SDK
uv add 'ag-ui-protocol==0.0.0.dev1786000891' --index testpypi

# Integrations (each already depends on the matching ag-ui-protocol preview)
uv add 'ag-ui-langgraph==0.0.0.dev1786000891' --index testpypi
uv add 'ag-ui-crewai==0.0.0.dev1786000891' --index testpypi
# NOTE: ag-ui-agent-spec depends on pyagentspec (git-only, not on PyPI).
# You will need to install pyagentspec separately from its git repo.
uv add 'ag-ui-agent-spec==0.0.0.dev1786000891' --index testpypi
uv add 'ag_ui_adk==0.0.0.dev1786000891' --index testpypi
uv add 'ag_ui_strands==0.0.0.dev1786000891' --index testpypi

Install with pip

pip install \
  --index-url https://test.pypi.org/simple/ \
  --extra-index-url https://pypi.org/simple/ \
  ag-ui-protocol==0.0.0.dev1786000891

Use --extra-index-url https://pypi.org/simple/ so pip can resolve
transitive dependencies (pydantic, fastapi, etc.) from real PyPI.


Commit: f7dc8a5

@pkg-pr-new

pkg-pr-new Bot commented Aug 6, 2026

Copy link
Copy Markdown

Open in StackBlitz

@ag-ui/a2a-middleware

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/a2a-middleware@2334

@ag-ui/a2ui-middleware

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/a2ui-middleware@2334

@ag-ui/event-throttle-middleware

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/event-throttle-middleware@2334

@ag-ui/mcp-apps-middleware

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/mcp-apps-middleware@2334

@ag-ui/mcp-middleware

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/mcp-middleware@2334

@ag-ui/a2a

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/a2a@2334

@ag-ui/adk

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/adk@2334

@ag-ui/ag2

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/ag2@2334

@ag-ui/agno

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/agno@2334

@ag-ui/aws-strands

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/aws-strands@2334

@ag-ui/claude-agent-sdk

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/claude-agent-sdk@2334

@ag-ui/claude-managed-agents

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/claude-managed-agents@2334

@ag-ui/crewai

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/crewai@2334

@ag-ui/langchain

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/langchain@2334

@ag-ui/langgraph

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/langgraph@2334

@ag-ui/llamaindex

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/llamaindex@2334

@ag-ui/mastra

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/mastra@2334

@ag-ui/pydantic-ai

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/pydantic-ai@2334

@ag-ui/vercel-ai-sdk

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/vercel-ai-sdk@2334

@ag-ui/watsonx

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/watsonx@2334

@ag-ui/a2ui-toolkit

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/a2ui-toolkit@2334

create-ag-ui-app

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/create-ag-ui-app@2334

@ag-ui/client

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/client@2334

@ag-ui/core

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/core@2334

@ag-ui/encoder

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/encoder@2334

@ag-ui/proto

pnpm add https://pkg.pr.new/ag-ui-protocol/ag-ui/@ag-ui/proto@2334

commit: ef4b5ca

YiyuanMiao added a commit to YiyuanMiao/ag-ui that referenced this pull request Aug 6, 2026
…ol#2 to PR ag-ui-protocol#2334

Per the split agreed on ag-ui-protocol#2172, revert the Fix ag-ui-protocol#1 change (halt `break` back to
the original `continue`) so the halt-path fix — including the explicit
`aclose()` that `break` alone doesn't cover — lands via ag-ui-protocol#2334 instead.

Keep the pending_halt message-retention (no longer discarding the tool-result
message) since the deferred frontend-end flush (Fix ag-ui-protocol#3, retained here) depends
on it; update its comment to no longer reference the reverted break.

Signed-off-by: YiyuanMiao <miaoyiyuan31@gmail.com>
@contextablemark
contextablemark merged commit 0d2de4f into main Aug 6, 2026
46 checks passed
@contextablemark
contextablemark deleted the mark/strands-py-halt-stops-loop branch August 6, 2026 15:41
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