Python: Fix AG-UI approval thread aliases - #6908
Merged
Merged
Conversation
github-actions
Bot
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes AG-UI approval resolution for stateful providers (e.g., Foundry) where the provider-advertised conversation_id can differ from the client-supplied thread_id, causing pending approvals to be unresolvable on resume and leading to missing tool outputs.
Changes:
- Register each pending approval under both the original client thread id and the provider conversation id (as aliases to the same registry entry).
- Consume/remove all aliases when an approval response is validated to prevent replay across either id.
- Add a regression test that resumes approvals using either id and verifies replay prevention.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py | Adds alias-aware pending-approval registration/removal and updates eviction + approval consumption to operate on shared alias entries. |
| python/packages/ag-ui/tests/ag_ui/test_approval_thread_id_mismatch.py | New regression test covering approval resolution via client thread id vs provider conversation id and preventing replay via the alternate id. |
Contributor
Author
|
@microsoft-github-policy-service agree |
godququ5-code
marked this pull request as ready for review
July 3, 2026 19:53
moonbox3
reviewed
Jul 7, 2026
Contributor
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
moonbox3
approved these changes
Jul 7, 2026
godququ5-code
force-pushed
the
godququ5/issue-6894
branch
from
July 7, 2026 13:14
165a16b to
6911161
Compare
# Conflicts: # python/packages/ag-ui/agent_framework_ag_ui/_agent_run.py
giles17
approved these changes
Jul 10, 2026
RudrenduPaul
added a commit
to RudrenduPaul/toolgovern
that referenced
this pull request
Jul 18, 2026
Today's require-approval gate is a synchronous, in-process callback with a 30s fail-closed timeout and no way to resume a decision out of band. Add PendingApprovalRegistry (TS + Python) as a keyed, server-generated-id store for require-approval decisions, wired into governTool()/govern_tool() alongside the existing synchronous onApprovalRequired path (unchanged for callers who don't opt in). - registerPending()/get()/resolvePending() plus alias tolerance (registerAlias) so a rewritten thread/conversation id still resolves to the same pending entry -- modeled on the bug fixed in microsoft/agent-framework#6908. - resolvePending() never creates a new entry for an unrecognized id -- closes the resume-token bypass Corridor's security bot found in langchain-ai/langgraph#8169's human_approval() helper. - Edited arguments supplied to resolvePending() are re-run through the classifier before an "allow" is accepted; a still-risky edit is denied even after approval, with a real test proving it on both sides. - governTool()/govern_tool() persist a pending approval BEFORE invoking the synchronous handler. A genuine handler answer (allow/deny) closes the entry out as terminal; a fail-closed default (no handler, timeout, or a throwing handler) leaves it open for later resolution -- otherwise the async path would be permanently unreachable. - resumePendingApproval()/resume_pending_approval() closes the loop: given the resolved outcome, it actually invokes the tool and appends one trace entry with approvedBy populated end-to-end, not only on the sync path. 60 new TypeScript tests, 53 new Python tests, all existing suites green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #6894
Root cause
For stateful providers such as Foundry, AG-UI can receive a request with a stable client thread id, then stream a provider conversation id back to the caller. Pending approval requests were registered only under the post-stream provider id, so clients that resumed with the original client thread id could not match the pending approval and the tool was never executed.
Test plan
uv run pytest packages/ag-ui/tests/ag_ui/test_approval_thread_id_mismatch.py packages/ag-ui/tests/ag_ui/test_agent_wrapper_comprehensive.py -k "approval" -qcd packages/ag-ui && uv run poe testcd packages/ag-ui && uv run poe syntaxcd packages/ag-ui && uv run poe pyright