Skip to content

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

Open
Naveen Chatlapalli (1aifanatic) wants to merge 2 commits into
microsoft:mainfrom
1aifanatic:contrib/7338-workflow-activity-ids
Open

Naveen Chatlapalli (1aifanatic) wants to merge 2 commits into
microsoft:mainfrom
1aifanatic:contrib/7338-workflow-activity-ids

Conversation

@1aifanatic

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

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 package-configured Pyright pass. 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 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

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?

2 participants