Repository navigation
Python: discard pending State on failed/cancelled superstep (#7859) - #8306
LI (ktz03) wants to merge 10 commits into
Conversation
Call State.discard() on runner failure and cancellation paths so staged writes cannot leak into a later successful run on the same Workflow. Fixes microsoft#7859.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
agent-framework-automation
Bot
added
the
python
There was a problem hiding this comment.
🟡 Changes recommended
Cleanup can still be skipped during event streaming and cancellation outside the polling block.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Prevents failed workflow supersteps from leaking staged Python state into later runs.
Changes:
- Discards pending state on failure and cancellation paths.
- Adds a workflow-reuse regression test for failures.
File summaries
| File | Description |
|---|---|
_runner.py |
Adds pending-state cleanup paths. |
test_runner.py |
Tests state isolation after failure. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Scope cancel/BaseException cleanup across the full superstep until commit - Discard pending state before yielding failure events - Add cancellation regression that stages state then reuses the workflow
GitHub Actions
Inactive
- Discard in finally so raising executor cleanup cannot skip it - Defer executor_failed yields until after discard - Block Workflow.run reuse while superstep cleanup is in progress
GitHub Actions
Inactive
- Discard in finally so raising executor cleanup cannot skip it - Defer executor_failed yields until after discard - Block Workflow.run reuse while superstep cleanup is in progress
GitHub Actions
Inactive
Reject concurrent cancel/run while ResponseStream cleanup or another exclusive entry holds the workflow, and hold _exclusive_run_hold for the cancel continuation.
GitHub Actions
Inactive
GitHub Actions
Inactive
|
Eduard van Valkenburg (@eavanvalkenburg) Updated PR body to the repository template and addressed the latest review feedback on this head — please re-review when convenient. |
GitHub Actions
Inactive
|
Please fix the failing CI/CD checks. |
|
Thanks for the contribution. The current head is not green: |
Commit declarative response headers before DeclarativeActionError so Runner State.discard (microsoft#7859) does not drop them. Type WorkflowContext in the discard regression tests so ty accepts yield_output.
GitHub Actions
Inactive
|
Addressed on
Workflow runs for this head are currently waiting on approval-gated Actions. |
…staging Keep cancel_pending_requests setup inside try/finally so normalize_tools failures cannot leave _exclusive_run_hold stuck. Discard residual pending State when checkpoint preparation does not complete, and only mark the superstep committed after checkpoint prep returns.
GitHub Actions
Inactive
GitHub Actions
Inactive
Rebase microsoft#7859 discard/exclusive-hold semantics onto main's asyncio.wait runner loop so cancelled supersteps release iteration/event tasks instead of hanging, while still discarding uncommitted State before the next run.
GitHub Actions
Inactive
| self._assign_response_headers(state, result) | ||
| # Runner discards pending State when the superstep fails (#7859 / #8306). | ||
| # Commit first so diagnostic headers remain readable after the error. | ||
| state.commit() |
There was a problem hiding this comment.
LI (@ktz03), state.commit() runs even when this action has no responseHeaders; in that path _assign_response_headers() is a no-op, so this commits all pending workflow state immediately before the error is raised. Those failed-superstep writes can then leak into a later run, which is the bug this PR is intended to prevent. Please only preserve diagnostic header state when headers were actually configured or written, and add a regression asserting that a failed HTTP action without responseHeaders discards pending state.
|
LI (@ktz03) Thanks for the contribution. The failed-superstep state-leak concern remains outstanding without updates for more than three business days, so I’m closing this for now. Please reopen once failed HTTP actions without |
|
The requested fix is available on the source branch in commit 8fd977f. Failed HTTP actions now discard pending state, including when responseHeaders is not configured. All four regression cases fail before the fix and pass afterward. Fresh validation: 1,095 declarative tests and all 50 HTTP tests passed; Ruff and strict Pyright checks passed. I don’t see a Reopen button on this PR. Could a maintainer please reopen it so the updated head can be reviewed? |
Motivation & Context
State.set()stages writes in_pending, but a failed or cancelled superstep previously left those writes intact. Because aWorkflowreuses the same runner/state acrossrun()calls, a later successful run couldcommit()stale pending writes from a prior failed/cancelled superstep.Fixes #7859.
Description & Review Guide
finallyso raising executor cleanup cannot skip it); deferexecutor_failedyields until after discard; blockWorkflow.runreuse while cleanup is in progress; lockcancel_pending_requestswith the same active/cleanup hold asrun().RunnerImpl.run_until_convergencecleanup ordering andWorkflow.cancel_pending_requests/_is_run_active.Related Issue
Fixes #7859
No other open PR targets this issue.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.