Skip to content

Invalidate the StatefulExecutorCache on checkpoint restore - #745

Open
PratikDhanave wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:invalidate-cache-on-checkpoint-restore
Open

Invalidate the StatefulExecutorCache on checkpoint restore#745
PratikDhanave wants to merge 1 commit into
microsoft:mainfrom
PratikDhanaveFork:invalidate-cache-on-checkpoint-restore

Conversation

@PratikDhanave

Copy link
Copy Markdown
Contributor

What

StatefulExecutorCache caches its typed state value after the first read (when
concurrent runs are disabled and the caller does not skip the cache). The only
way to drop that cached value was Reset(), whose func() error signature does
not match the executor's OnCheckpointRestoredFunc hook.

On an in-place checkpoint restore, restoreCheckpointCore imports the checkpoint
state and then calls notifyCheckpointLoaded, which fires each executor's
OnCheckpointRestored. A shared stateful executor that had already cached kept
cached == true holding the pre-restore value, so the next ReadState returned
the stale value instead of the state imported from the checkpoint.

This adds StatefulExecutorCache.OnCheckpointRestored(*Context) error, which
invalidates the cache. Its signature matches Executor.OnCheckpointRestoredFunc,
so a shared stateful executor can wire it directly into the restore hook that the
runner already invokes.

Why

This matches the .NET/Python Agent Framework semantics, where in-memory executor
state derived from workflow state is dropped when a checkpoint is restored, so
the executor observes exactly the restored state on the next read. Restoring an
earlier checkpoint while still trusting a newer cached aggregate is a correctness
bug across SDKs.

How it is tested

TestStatefulExecutorCache_CheckpointRestoreInvalidatesCache builds a workflow
from the shared aggregatorBinding pattern, runs a (capturing the post-a
checkpoint) then b so the cache holds a+b, restores in place to the post-a
checkpoint, and sends c. With the hook wired the aggregate is [a, a+b, a+c];
without it the cache stays stale and yields a+b+c. The existing cache and
reset tests continue to pass.

@PratikDhanave
PratikDhanave requested a review from a team as a code owner July 24, 2026 04:02
Copilot AI review requested due to automatic review settings July 24, 2026 04:02

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.

Pull request overview

Adds a StatefulExecutorCache hook that can be directly wired into an executor’s checkpoint-restore lifecycle so cached typed state is invalidated after an in-place checkpoint restore (preventing stale reads).

Changes:

  • Introduce StatefulExecutorCache.OnCheckpointRestored(*Context) error, which clears the cache by delegating to Reset().
  • Wire the new hook into the shared “aggregator” test binding via Executor.OnCheckpointRestoredFunc.
  • Add an end-to-end test covering in-place checkpoint restore behavior to ensure the post-restore read observes restored state, not stale cached state.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
workflow/executor.go Adds a cache-invalidation hook compatible with Executor.OnCheckpointRestoredFunc to prevent stale cached state after restore.
workflow/executor_test.go Wires the hook in a shared binding and adds a regression test validating correct behavior across checkpoint restore + resume.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

This comment has been minimized.

@github-actions github-actions Bot added parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs labels Jul 24, 2026
StatefulExecutorCache keeps an executor-local value after its first read, and
the only invalidation is Reset(), whose signature does not match the executor's
OnCheckpointRestored hook. A shared stateful executor restored in place to an
earlier checkpoint therefore kept its pre-restore cached value, and the next
read returned the stale state instead of the imported checkpoint state.

Add OnCheckpointRestored to StatefulExecutorCache. Its signature matches
Executor.OnCheckpointRestoredFunc so it can be wired into the restore hook that
notifyCheckpointLoaded already fires, clearing the cache so the next read
observes the restored state. This mirrors the .NET/Python cache behavior, where
in-memory executor state is dropped on checkpoint restore.
@PratikDhanave
PratikDhanave force-pushed the invalidate-cache-on-checkpoint-restore branch from a92b1e5 to 648ba36 Compare July 24, 2026 09:30
@github-actions

Copy link
Copy Markdown
Contributor

Cross-Repo Parity Review ✅

PR summary: Adds StatefulExecutorCache.OnCheckpointRestored(*Context) error — a new exported method that calls Reset() to invalidate the executor-local cache after a checkpoint restore.

Scope

This PR exports one new method on StatefulExecutorCache[T]. Its signature matches the existing Executor.OnCheckpointRestoredFunc hook shape, so callers can wire it directly as the restore callback.

Cross-repo parity assessment

The OnCheckpointRestored lifecycle concept is well-established in the upstream .NET codebase (dotnet/src/Microsoft.Agents.AI.Workflows/), appearing in HandoffAgentExecutor, GroupChatManager, RoundRobinGroupChatManager, MagenticOrchestrator, and others. The Go implementation of this hook is consistent in purpose: notify an executor component when workflow state is restored from a checkpoint so it can discard stale in-memory state.

Neither Python nor .NET exposes a direct StatefulExecutorCache equivalent (Go's generic typed-state helper is Go-specific), so cache-invalidation-on-restore is an internal implementation detail surfaced in a Go-idiomatic way. No semantic divergence from upstream.

Labels added: public-api-change (exported method added), parity-approved (no cross-repo inconsistency found).

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • awmgmcpg

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "awmgmcpg"

See Network Configuration for more information.

Generated by Go API Consistency Review Agent · 36.5 AIC · ⌖ 5.01 AIC · ⊞ 5.9K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

parity-approved Go API consistency review found no parity issues public-api-change Pull Request changes public APIs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants