Repository navigation
[BREAKING] Python: Reject unsafe functional step replay - #9391
Atharva Vichare (atty57) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Nested legacy replay can still silently return the wrong cached result when arguments are fingerprintable.
1 open finding
What changed in this PR
Adds fail-loud safeguards for functional workflow replay when step arguments lack stable identities.
Changes:
- Rejects unsafe checkpoint and HITL replay for opaque arguments.
- Validates legacy cache hits and replayable results.
- Adds regression tests and updates replay documentation.
| File | Description |
|---|---|
python/packages/core/agent_framework/_workflows/_functional.py |
Adds replay-safety guards and documentation. |
python/packages/core/tests/workflow/test_functional_workflow.py |
Adds opaque-argument replay regression tests. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
Eduard van Valkenburg (@eavanvalkenburg) thanks for the plan. The nested legacy-replay failure and the opaque-argument example share one root cause, and one small change fixes both without rejecting any legacy cache hit. Findings and a sketch of the change below. Root cause. Change. This is suggestion 1 of the issue: keep the order-based counters, but scope them to the calling step.
The other variant in suggestion 1, advancing the children's counters on a cache hit, would need the call counts stored with each cached entry. Entries from earlier releases lack them, so it would not help the legacy case. It was not prototyped. What was run.
Legacy-format entries (
Test files.
Compatibility boundary.
Not covered yet: concurrent sibling calls under different parents, a nested Sketch of the change (against c4de86d)--- a/python/packages/core/agent_framework/_workflows/_functional.py
+++ b/python/packages/core/agent_framework/_workflows/_functional.py
@@ module level, after _active_run_ctx @@
_active_run_ctx: ContextVar[RunContext | None] = ContextVar("_active_run_ctx", default=None)
+# Scope of the running step body (hash of its cache key); empty at workflow level.
+_active_step_scope: ContextVar[str] = ContextVar("_active_step_scope", default="")
@@ RunContext.__init__ @@
- self._step_call_counters: dict[str, int] = {}
+ self._step_call_counters: dict[tuple[str, str], int] = {}
@@ RunContext._get_legacy_step_cache_key, _get_automatic_step_cache_key @@
def _get_legacy_step_cache_key(self, step_name: str) -> str:
- idx = self._step_call_counters.get(step_name, 0)
- self._step_call_counters[step_name] = idx + 1
- return f"{step_name}::{idx}"
+ scope = _active_step_scope.get()
+ idx = self._step_call_counters.get((scope, step_name), 0)
+ self._step_call_counters[(scope, step_name)] = idx + 1
+ return f"{scope}/{step_name}::{idx}" if scope else f"{step_name}::{idx}"
def _get_automatic_step_cache_key(self, step_name: str, wrapper_identity: str, identity: str) -> str:
+ scope = _active_step_scope.get()
+ if scope:
+ identity = f"{scope}/{identity}"
counter_key = (step_name, wrapper_identity, identity)
@@ StepWrapper.__call__ @@
auto_request_info_index_before = ctx._auto_request_info_index
+ scope_token = _active_step_scope.set(hashlib.sha256(cache_key.encode("utf-8")).hexdigest()[:16])
try:
result = await self._func(*call_args, **call_kwargs)
except Exception as exc:
...
raise
+ finally:
+ _active_step_scope.reset(scope_token)
@@ the workflow's _execute @@
token = _active_run_ctx.set(ctx)
+ scope_reset_token = _active_step_scope.set("")
release_task_tracking = _track_workflow_tasks()
...
finally:
release_task_tracking()
+ _active_step_scope.reset(scope_reset_token)
_active_run_ctx.reset(token) |

Motivation & Context
Bypassing a cached outer
@stepskips its inner call counters. A later nested or top-level call can then silently reuse an earlier invocation's result from an order-only cache. Legacy checkpoints remain unsafe even when current arguments are fingerprintable or callers supply explicit replay keys; opaque arguments could also produce unsafe new checkpoints.Description & Review Guide
replay_keyfor checkpoint/HITL replay. Legacy order-only checkpoints are no longer replayable, including flat sequential checkpoints, and must be regenerated in a new run. Existing versioned checkpoints continue to replay. Sequential execution without replay still supports opaque arguments. With explicit keys, nested crash recovery returns[['A', 'B']]and only rerunsb.This chooses the fail-loud fix from issue option #3 over automatically scoping counters (option #1), keeping the fix focused and requiring explicit invocation identity for opaque inputs.
Related Issue
Fixes #9385
Maintainer agreement on this direction is pending; no competing open PR was found before opening this PR.
AI Assistance
AI assistance details: Issue analysis, implementation, tests, docstrings, and this PR description.
Contribution Checklist
breaking changelabel (or add "[BREAKING]" to the title prefix, before or after any language prefix) — a workflow keeps the label and title prefix in sync automatically.The title flags the intentional compatibility change for legacy order-only checkpoints. All 168 functional workflow tests pass, including the nested and mixed nested/direct legacy replay regressions. Ruff lint/format, source and test Pyright, and test mypy checks pass on the touched files. The full
poe checkis left to CI; maintainer agreement is pending.