Skip to content

Python: scope executor activity IDs to workflow runs - #8549

Merged
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
1aifanatic:contrib/7338-workflow-activity-ids
Sep 23, 2026
Merged

Python: scope executor activity IDs to workflow runs#8549
Eduard van Valkenburg (eavanvalkenburg) merged 3 commits into
microsoft:mainfrom
1aifanatic:contrib/7338-workflow-activity-ids

Conversation

@1aifanatic

@1aifanatic Naveen Chatlapalli (1aifanatic) commented Sep 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Motivation & Context

Executor activity snapshots currently reuse IDs such as executor:researcher across workflow runs. A UI or message store keyed by message ID can overwrite the activity from an earlier turn.

Description & Review Guide

  • What are the major changes? Prefix the executor activity message ID with the length-prefixed AG-UI run ID so unrestricted component strings cannot collide. Add regression cases for completion and failure paths across two runs and two executors, plus the delimiter-collision case.
  • What is the impact of these changes? Progress and terminal snapshots for an executor still update the same activity within a run, while distinct runs have distinct activity IDs. The executor identity remains available in content.executor_id; no public API or dependency is added.
  • What do you want reviewers to focus on? Within-run update identity and separation across runs. The original two cases fail before run scoping; the delimiter case fails before length-prefixing. The complete AG-UI package suite passes: 1,370 passed, 14 skipped (12 existing deprecation/AsyncMock warnings). Changed-file Ruff lint/format and the repository test-typing task for ag-ui pass all five gating checkers (mypy, pyrefly, ty, zuban, and Pyright). Cross-package tests were not run.

Related Issue

Fixes #7338. Checked the issue discussion, cross-references, and open PRs; no competing implementation was found. The related #7429 concerns lifecycle/thread correlation and explicitly excludes activity message IDs.

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.

Copilot AI balanced review requested due to automatic review settings September 19, 2026 21:41
@1aifanatic
Naveen Chatlapalli (1aifanatic) marked this pull request as ready for review September 19, 2026 21:41
@agent-framework-automation agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Sep 19, 2026
Copilot AI reviewed Sep 19, 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

The delimiter-based ID format permits collisions between unrestricted run and executor IDs.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)
What changed in this PR

Scopes executor activity IDs to workflow runs to prevent cross-run snapshot overwrites.

Changes:

  • Prefixes activity IDs with the run ID.
  • Adds completion and failure regression coverage across runs and executors.
File Description
python/​packages/​ag-ui/​agent_framework_ag_ui/​_workflow_run.py Adds run-scoped executor activity IDs.
python/​packages/​ag-ui/​tests/​ag_ui/​test_workflow_run.py Tests activity identity and run separation.

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

Comment thread python/packages/ag-ui/agent_framework_ag_ui/_workflow_run.py Outdated
@eavanvalkenburg

Eduard van Valkenburg (eavanvalkenburg) commented Sep 21, 2026

Copy link
Copy Markdown
Member

Naveen Chatlapalli (@1aifanatic), thanks for the contribution. The current head is not green: Test Typing Checks and Merge Gatekeeper are failing. Please fix the typing failure and re-request review once the current head is green.

@1aifanatic

Naveen Chatlapalli (1aifanatic) commented Sep 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Fixed in 6992ea0. The regression tests now narrow events with isinstance(event, ActivitySnapshotEvent), so message_id and content are accessed on the concrete event type rather than BaseEvent.

I reproduced the CI diagnostics from Test Typing Checks and ran the repository's test-typing task for ag-ui with all five gating checkers: mypy, pyrefly, ty, zuban, and Pyright all pass. The full AG-UI package suite also passes (1,370 passed, 14 skipped), and Ruff lint/format checks pass. The new head is pushed; I will re-request review once its required CI checks are green.

The new-head workflows currently show action_required, including Python - Code Quality. A maintainer needs to approve those runs before I can confirm green CI and re-request review.

eavanvalkenburg
Eduard van Valkenburg (eavanvalkenburg) approved these changes Sep 23, 2026
View reviewed changes

@eavanvalkenburg Eduard van Valkenburg (eavanvalkenburg) left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Naveen Chatlapalli (@1aifanatic) Reviewed the current head; no blocking findings.

Merged via the queue into microsoft:main with commit e636a72 Sep 23, 2026
43 checks passed

This branch was successfully deployed

1 active deployment
github-app-auth — 6992ea0f Deployed Sep 22, 2026 by 1aifanatic via add_label #23545
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

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
@moonbox3 Evan Mattson (moonbox3) Awaiting requested review from moonbox3 moonbox3 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 TaoChenOSU is a code owner
@giles17 Giles Odigwe (giles17) Awaiting requested review from giles17 giles17 is a code owner

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: [AGUI] BaseMessage message_id uniqueness semantics - per-thread or globally unique or something else?

3 participants

@1aifanatic @eavanvalkenburg