fix(coordination): close RealRuntime._current_instance parallel-invoke validation race (#40) - #47
Conversation
…e validation race RealRuntime is constructed once per Orchestra.run() and the orchestrator dispatches step() for every runnable branch as a concurrent asyncio task on that one shared object. step() stashed the per-branch agent instance on self._current_instance before the execute_step await, and _translate() read it back after the await to feed ValidationProcessor. Under parallel_invoke a sibling branch overwrote the shared attribute during the await, so a branch's coordination action was validated against a different agent's identity (and its outgoing topology edges) - a fabricated, misattributed "Agent X cannot invoke: [...]" failure (latent when fanned-out workers shared identical edges; surfaced non-deterministically when they differed). A direct DP-004 (branch isolation) violation. Thread the agent instance as an explicit parameter into _translate and delete self._current_instance, so the shared runtime holds zero per-step mutable state - conforming the lone outlier to the file's existing convention (execute_step / _build_content_only_diagnostic already take instance explicitly). Public Runtime Protocol and step(branch) signature unchanged; only the private _translate signature changes. Adds a deterministic no-LLM regression test that forces the racy interleave (asyncio.gather + a yielding execute_step) - RED on the pre-fix code, GREEN after. See ADR-010. Closes #40
There was a problem hiding this comment.
Code Review
This pull request resolves a critical concurrency bug (issue #40) in RealRuntime where concurrent step() calls could overwrite the shared _current_instance attribute, leading to cross-talk and incorrect validation of agent identities. The fix threads the per-branch agent instance explicitly as a parameter through _translate() and removes the shared state entirely. The changes are accompanied by ADR-010 documenting this architectural decision and a deterministic regression test to prevent future occurrences. I have no feedback to provide as the implementation is correct and well-documented.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Closes #40.
What
RealRuntimeis constructed once perOrchestra.run()and the orchestrator dispatchesstep()for every runnable branch as a concurrentasyncio.Taskon that one shared object.step()stashed the per-branch agent instance onself._current_instancebefore theexecute_stepawait, and_translate()read it back after the await to feedValidationProcessor.validate_coordination_action(agent=...). Underparallel_invokefan-out a sibling branch overwrote the shared attribute during the await, so a branch's coordination action was validated against a different agent's identity — and thus its outgoing topology edges — producing a fabricated, misattributed"Agent X cannot invoke: [...]"failure. Latent when fanned-out workers shared identical outgoing edges; surfaced non-deterministically when they differed. A direct DP-004 (branch isolation) violation.Fix
Thread the agent instance as an explicit parameter into
_translate(agent=instance) and deleteself._current_instance, so the shared runtime holds zero per-step mutable state — conforming the lone outlier to the file's existing convention (execute_step/_build_content_only_diagnosticalready takeinstanceexplicitly). Four edits inreal_runtime.py; no new mechanism.Public surface unchanged: the
RuntimeProtocol andstep(branch)signature are untouched; only the private_translatesignature changes.Tests
tests/coordination/orchestrator/test_real_runtime_parallel_race.py) forces the racy interleave viaasyncio.gather+ a yieldingexecute_step. Verified RED on pre-fix code (validator was handed a cross-attributed identity: [('AgentA', 'AgentC'), ('AgentC', 'AgentC')]) and GREEN after the fix.runtime._current_instance = instanceline fromtest_content_only_hard_limit.py(already dead pre-fix — overwritten on everystep()).parallel_invokereproducer is manual verification only (needs a provider key) — deliberately not committed as a skipped/flaky CI test.TRUNK-CRITICAL
real_runtime.pyis TRUNK-CRITICAL. This is a non-additive change to a private method signature + removal of a private attribute; the publicRuntimeProtocol is unchanged. Framework-team approval: rezaho, 2026-06-23 (recorded inADR-010§Approval). ADR:docs/architecture/framework/decisions/ADR-010-realruntime-per-branch-instance-threading.md.Multi-consumer justification
Pure correctness fix; the affected surface is everyone running
parallel_invokewith heterogeneous outgoing edges — third-partyOrchestra.run()users, MARSYS Cloud / Studio, the framework's own coordination examples, and Spren workflows. No Spren-specific path; no Spren type imported.Planning brief (private Spren repo):
docs/implementation/framework/sessions/v0.3.0/16-realruntime-current-instance-race.md.