refactor(agent-hypervisor)!: make session methods synchronous - #3645
Open
Tionne Smith (electricwolfemarshmallowhypertext) wants to merge 3 commits into
Conversation
Tionne Smith (electricwolfemarshmallowhypertext)
requested a review
from MohammadHaroonAbuomar
as a code owner
August 7, 2026 13:28
Tionne Smith (electricwolfemarshmallowhypertext)
requested a review
from liamcrumm
as a code owner
August 7, 2026 13:28
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
github-actions
Bot
added
documentation
|
🟡 Contributor Check: MEDIUM
Automated check by AGT Contributor Check. |
github-actions
Bot
added
the
needs-review:MEDIUM
PR Review Summary
Verdict: AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims. |
Contributor
There was a problem hiding this comment.
Pull request overview
Syncs the Hypervisor session lifecycle API (6 methods) to remove async-without-await footguns, updates repo call sites (docs/tests/examples/API), and adds a runtime guard in SagaOrchestrator to ensure step executors/compensators return awaitables.
TL;DR: 1 blocker, 2 warnings. Fix #1 and this ships.
| # | Sev | Issue | Where |
|---|---|---|---|
| 1 | Block | Awaitable guard rejects valid non-coroutine awaitables | SagaOrchestrator._ensure_awaitable |
| 2 | Warn | Quick start missing required creator_did (example will raise TypeError) |
docs/packages/agent-runtime.md |
| 3 | Warn | Quick start missing required creator_did (example will raise TypeError) |
agent-governance-python/agent-runtime/README.md |
#1: Validate generic awaitables (e.g., __await__) instead of only asyncio.isfuture/iscoroutine.
Changes:
- Convert
Hypervisorsession lifecycle methods fromasync def→def. - Update Python call sites (API server, tests, examples, benchmarks) to remove
await. - Add an executor/compensator awaitable return-type guard in
SagaOrchestrator.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/packages/agent-runtime.md | Update runtime quick start to sync session creation |
| docs/packages/agent-os.md | Update runtime session lifecycle example to sync calls |
| docs/packages/agent-hypervisor.md | Update hypervisor lifecycle examples to sync calls |
| agent-governance-python/agent-runtime/README.md | Update runtime README example to sync session creation |
| agent-governance-python/agent-os/README.md | Update Agent OS README lifecycle example to sync calls |
| agent-governance-python/agent-hypervisor/tests/unit/test_cli.py | Remove awaits in fixture session setup |
| agent-governance-python/agent-hypervisor/tests/test_agent_manager.py | Remove awaits across hypervisor unit tests |
| agent-governance-python/agent-hypervisor/tests/integration/test_scenarios.py | Remove awaits in integration scenarios |
| agent-governance-python/agent-hypervisor/tests/integration/test_hypervisor_e2e.py | Remove awaits in E2E hypervisor tests |
| agent-governance-python/agent-hypervisor/src/hypervisor/saga/orchestrator.py | Add runtime awaitable guard for step execution/compensation |
| agent-governance-python/agent-hypervisor/src/hypervisor/core.py | Make session lifecycle methods synchronous |
| agent-governance-python/agent-hypervisor/src/hypervisor/api/server.py | Update FastAPI handlers to call sync hypervisor methods |
| agent-governance-python/agent-hypervisor/README.md | Update README lifecycle examples to sync calls |
| agent-governance-python/agent-hypervisor/examples/docker-compose/app/server.py | Update example server to call sync hypervisor methods |
| agent-governance-python/agent-hypervisor/examples/demo.py | Update demo flows to call sync hypervisor methods |
| agent-governance-python/agent-hypervisor/docs/api-reference.md | Update API reference examples to sync calls |
| agent-governance-python/agent-hypervisor/benchmarks/bench_hypervisor.py | Update benchmarks to call sync lifecycle methods |
| agent-governance-python/agent-compliance/examples/governed_agent.py | Update compliance example to call sync runtime session creation |
Comment on lines
+238
to
+240
| def _ensure_awaitable(self, result: Any, name: str) -> None: | ||
| if not asyncio.isfuture(result) and not asyncio.iscoroutine(result): | ||
| raise TypeError(f"{name} must return an awaitable") |
Comment on lines
+57
to
59
| session = hv.create_session( | ||
| config=SessionConfig(consistency_mode=ConsistencyMode.EVENTUAL) | ||
| ) |
Comment on lines
+51
to
53
| session = hv.create_session( | ||
| config=SessionConfig(consistency_mode=ConsistencyMode.EVENTUAL) | ||
| ) |
MohammadHaroonAbuomar
requested changes
Aug 7, 2026
MohammadHaroonAbuomar
left a comment
MohammadHaroonAbuomar
left a comment
Collaborator
There was a problem hiding this comment.
- PR title: fails the required Validate PR title check; use the conventional format with the breaking marker, e.g. refactor(agent-hypervisor)!: make session methods synchronous.
- BREAKING_CHANGES.md: no entry, though the six converted methods ship in the published agent-governance-toolkit-core wheel (the pyproject force-includes hypervisor/src), so external consumers awaiting them break. Add an entry with a how-to-update line (drop the await).
Minor:
- a small test for _ensure_awaitable would make the checklist claim true; everything else verified clean: zero stale awaited call sites repo-wide, 640/640 hypervisor tests on the sync API, no fail-open - the conversion actually removes the forgot-await silent no-op class. Direction is maintainer-ordered per issue #3178.
Comment on lines
+238
to
+240
| def _ensure_awaitable(self, result: Any, name: str) -> None: | ||
| if not asyncio.isfuture(result) and not asyncio.iscoroutine(result): | ||
| raise TypeError(f"{name} must return an awaitable") |
Collaborator
There was a problem hiding this comment.
_ensure_awaitable accepts only asyncio.isfuture/iscoroutine, rejecting generic await-only awaitables that asyncio.wait_for accepted before this change (verified both sides) - use inspect.isawaitable. Also raise the guard's TypeError outside the retry try: currently a misconfigured sync executor is re-invoked through every retry with backoff (3 calls / 3s probed), repeating side effects before surfacing.
Tionne Smith (electricwolfemarshmallowhypertext)
changed the title
Contributor
Author
|
Addressed the review items:
Validation:
Ready for re-review. |
Imran Siddique (imran-siddique)
previously approved these changes
Aug 12, 2026
Imran Siddique (imran-siddique)
left a comment
Imran Siddique (imran-siddique)
left a comment
Collaborator
There was a problem hiding this comment.
Re-reviewed current head 1c76630f. The approved sync direction is implemented consistently across the six Hypervisor methods and direct repository call sites.
The subsequent blockers are all addressed:
- generic
__await__objects are accepted viainspect.isawaitable; - sync executor contract violations are rejected before retry/backoff, preventing repeated side effects;
- both behaviors have focused regression tests;
- the breaking API change is documented with the correct “drop
await” migration; - runtime examples now pass the required
creator_did; - unrelated async Saga APIs remain awaited.
The content is approved. GitHub currently reports the branch as not mergeable, so any current base conflict/check gate still needs to be resolved before merge.
Tionne Smith (electricwolfemarshmallowhypertext)
requested a review
from MohammadHaroonAbuomar
August 19, 2026 12:38
Tionne Smith (electricwolfemarshmallowhypertext)
force-pushed
the
marshmallow/hypervisor-sync-surface-current
branch
2 times, most recently
from
August 25, 2026 20:27
066a48c to
1c76630
Compare
Signed-off-by: Electric Wolfe Marshmallow Hypertext <tionne@gmail.com>
Signed-off-by: Electric Wolfe Marshmallow Hypertext <tionne@gmail.com>
Signed-off-by: Electric Wolfe Marshmallow Hypertext <tionne@gmail.com>
Tionne Smith (electricwolfemarshmallowhypertext)
force-pushed
the
marshmallow/hypervisor-sync-surface-current
branch
from
August 25, 2026 20:47
1c76630 to
4c065da
Compare
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.
Related Issue
Fixes #3178.
Problem & Solution
The Hypervisor session surface mixed sync and async methods even though six methods had no internal await points.
This PR implements the maintainer-approved sync direction for:
create_sessionjoin_sessionactivate_sessionterminate_sessionverify_behaviormonitor_sessionsDirect call sites were updated across the current repository. Unrelated same-name async APIs were left unchanged.
SagaOrchestrator.execute_stepandcompensateremain async, with a narrow runtime guard requiring executor/compensator results to be awaitable.Impact on Your Work
Removes the mixed sync/async Hypervisor API footgun and makes the session surface consistent with the existing synchronous helpers and callers.
Timeline
None.
Alternatives Considered
Keeping the six methods async was rejected because they contain no internal await points and the maintainer explicitly selected the synchronous direction in #3178.
Type of Change
Package(s) Affected
Core & runtime:
Governance & security:
Platform & tooling:
CLI plugins:
Shared / other:
Testing
Unit Testing
Focused Hypervisor test suite:
102 passed, 5 skippedManual Testing
py_compilepassed for edited Python filesgit diff --checkpassedmesh.create_session(...)await remains intentionally unchangedChecklist
Attribution & Prior Art
Prior art / related projects (if any):
None.
AI Assistance
If AI tools materially shaped this change, briefly note what was used:
AI-assisted code review and repository search were used during implementation; all changes were reviewed and validated manually before submission.
IP, Patents, and Licensing