Skip to content

Python: Don't emit response.completed after a cancelled Foundry hosting run - #8565

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
leilei3167:fix/issue-8564-cancellation-completed-event
Sep 21, 2026
Merged

Python: Don't emit response.completed after a cancelled Foundry hosting run#8565
Eduard van Valkenburg (eavanvalkenburg) merged 4 commits into
microsoft:mainfrom
leilei3167:fix/issue-8564-cancellation-completed-event

Conversation

@leilei3167

@leilei3167 leilei3167 commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

Motivation & Context

After a Foundry-hosted response is cancelled through cancellation_signal, _handle_inner_workflow and _handle_inner_agent stop their _SignalledIterator loop and return normally instead of raising. The post-loop code in _handle_prepared_response treats that normal return as a success and still yields response.completed (or response.incomplete). azure-ai-agentserver-responses happens to rewrite the terminal to cancelled on the wire, but anything sitting between the hosting handler and that layer (for example a subclass wrapping _handle_response to persist results) sees a successful terminal for a run that was actually cancelled.

Description & Review Guide

  • What are the major changes? In _handle_prepared_response, return without yielding a terminal event once cancellation_signal is observed set after the inner generator drains, instead of falling through to tracker.close() / emit_completed / emit_incomplete.
  • What is the impact of these changes? A cancelled run no longer emits response.completed; callers consuming the handler stream can rely on the absence of a terminal event (or the cancel-aware host layer's synthesized cancelled terminal) to distinguish a cancelled run from a finished one.
  • What do you want reviewers to focus on? Please double-check that skipping tracker.close() on the cancelled path is safe (the underlying _SignalledIterator/ResponseStream already close the wrapped iterator in their own finally blocks) and that this doesn't affect the shutdown/recovery path, which is gated separately.

Related Issue

Fixes #8564

Contribution Checklist

  • The code builds clean without any errors or warnings
  • All unit tests pass, and I have added new tests where possible
  • The PR follows the Contribution Guidelines
  • This PR is linked to an issue and there is no other open PR for this issue (see Related Issue above).
  • This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.

@leilei3167
Python: Don't emit response.completed after a cancelled Foundry hosti…
3a7e54a 
…ng run

Fixes microsoft#8564. When cancellation_signal fires, _handle_inner_workflow and
_handle_inner_agent stop their _SignalledIterator loop and return
normally instead of raising, so the post-loop code in
_handle_prepared_response treated the cancelled run as a success and
still emitted response.completed (or response.incomplete). Add an
early return once the inner generator drains so a cancelled run emits
no terminal event, letting the cancel-aware host layer synthesize the
cancelled terminal instead.
Copilot AI balanced review requested due to automatic review settings September 20, 2026 22:35
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Sep 20, 2026
Copilot AI reviewed Sep 20, 2026
View reviewed changes

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.

Copilot review overview

🟡 Changes recommended

Cancellation during tracker-closing events can still produce an incorrect terminal event.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 Medium severity

Open (1)
What changed in this PR

Prevents cancelled Foundry-hosted Python runs from emitting successful terminal events.

Changes:

  • Stops terminal emission when cancellation is detected.
  • Updates agent and workflow cancellation tests.
File Description
_responses.py Adds cancellation-aware terminal suppression.
test_responses.py Updates cancellation expectations and coverage.

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py Resolved
@leilei3167
Python: Recheck cancellation after draining tracker.close() before te…
d2813dd 
…rminal event

Copilot review on microsoft#8565 pointed out that the cancellation_signal check
added there runs before tracker.close(), but each event that loop
yields suspends the handler. A cancellation arriving while those
trailing *.done events are being consumed still fell through to
emit_completed/emit_incomplete, reproducing the microsoft#8564 race for a
narrower window. Recheck the signal once more immediately after
draining tracker.close(), before selecting the terminal event.
@leilei3167
Python: Gate cancellation-terminal suppression on client_cancelled
7ae1fe5 
Only skip the response.completed/incomplete terminal when the
cancellation_signal cause is an explicit /cancel (or non-bg disconnect),
i.e. context.client_cancelled is True. cancellation_signal also fires
for steering pressure with no cause flag set, so without this gate a
steered turn's partial output would be silently dropped instead of
completing normally. Update the existing explicit-cancel tests to set
client_cancelled alongside the signal, and add a regression test for
the steering case.
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 21, 2026
@eavanvalkenburg

Eduard van Valkenburg (eavanvalkenburg) commented Sep 21, 2026

Copy link
Copy Markdown
Member

leilei3167, thanks for the update. The current head now conflicts with main, so it cannot enter the merge queue. Please resolve the conflicts and re-request review once the updated head is green.

@leilei3167
Merge upstream main; keep cancellation terminal gates with incomplete…
155a0a2 
…_reason

Resolve conflict in foundry_hosting _responses: retain client_cancelled
recheck after tracker.close(), and adopt main's incomplete_reason path for
emit_incomplete so the branch can re-enter the merge queue.
@leilei3167

leilei3167 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the heads-up — merged latest main and resolved the conflict in _responses.py so the cancellation-terminal gates stay in place alongside the new incomplete_reason path. Ready for another look once CI is green.

Merged via the queue into microsoft:main with commit a1469d1 Sep 21, 2026
42 checks passed
ZedingZhang (ZedingZhang) pushed a commit to ZedingZhang/agent-framework that referenced this pull request Sep 29, 2026
@leilei3167
Python: Don't emit response.completed after a cancelled Foundry hosti…
b2668f5 
…ng run (microsoft#8565)

* Python: Don't emit response.completed after a cancelled Foundry hosting run

Fixes microsoft#8564. When cancellation_signal fires, _handle_inner_workflow and
_handle_inner_agent stop their _SignalledIterator loop and return
normally instead of raising, so the post-loop code in
_handle_prepared_response treated the cancelled run as a success and
still emitted response.completed (or response.incomplete). Add an
early return once the inner generator drains so a cancelled run emits
no terminal event, letting the cancel-aware host layer synthesize the
cancelled terminal instead.

* Python: Recheck cancellation after draining tracker.close() before terminal event

Copilot review on microsoft#8565 pointed out that the cancellation_signal check
added there runs before tracker.close(), but each event that loop
yields suspends the handler. A cancellation arriving while those
trailing *.done events are being consumed still fell through to
emit_completed/emit_incomplete, reproducing the microsoft#8564 race for a
narrower window. Recheck the signal once more immediately after
draining tracker.close(), before selecting the terminal event.

* Python: Gate cancellation-terminal suppression on client_cancelled

Only skip the response.completed/incomplete terminal when the
cancellation_signal cause is an explicit /cancel (or non-bg disconnect),
i.e. context.client_cancelled is True. cancellation_signal also fires
for steering pressure with no cause flag set, so without this gate a
steered turn's partial output would be silently dropped instead of
completing normally. Update the existing explicit-cancel tests to set
client_cancelled alongside the signal, and add a regression test for
the steering case.

This branch was previously deployed

1 inactive deployment
github-app-auth — 155a0a24 Deployed Sep 21, 2026 by leilei3167 via add_label #23420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@moonbox3 Evan Mattson (moonbox3) moonbox3 left review comments
Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) eavanvalkenburg approved these changes
@SergeyMenshykh SergeyMenshykh Awaiting requested review from SergeyMenshykh SergeyMenshykh is a code owner
@rogerbarreto Roger Barreto (rogerbarreto) Awaiting requested review from rogerbarreto rogerbarreto is a code owner
@westey-m westey (westey-m) Awaiting requested review from westey-m westey-m is a code owner
@jpalvarezl Jose Alvarez (jpalvarezl) Awaiting requested review from jpalvarezl jpalvarezl is a code owner
@TaoChenOSU Tao Chen (TaoChenOSU) Awaiting requested review from TaoChenOSU
@giles17 Giles Odigwe (giles17) Awaiting requested review from giles17

Assignees

No one assigned

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Python: Foundry hosting yields response.completed from the handler after a cancelled workflow run

4 participants

@leilei3167 @eavanvalkenburg @moonbox3