Python: fix(core): preserve checkpoint identity across filesystem aliases by he-yufeng · Pull Request #9466 · microsoft/agent-framework

Yufeng He (he-yufeng) · 2026-10-16T21:16:49Z

Motivation & Context

FileCheckpointStorage must not silently overwrite, read, or delete a checkpoint when filesystem case folding or Unicode normalization makes two requested IDs resolve to one file. The filename returned by the filesystem is not a reliable checkpoint identity: HFS+ can return a decomposed spelling even for a checkpoint originally saved with an NFC ID.

Description & Review Guide

What are the major changes? Save operations retain the folded destination queue and repeat collision verification under destination ownership before replacement. Save, load, and delete now compare the requested ID with the ID persisted in the checkpoint, so a same-ID NFC round trip works while a genuinely different NFC/NFD or case-variant ID is rejected. Load performs existence checking, directory verification, and JSON reading in one worker call. Delete checks for a missing destination before any variant scan, then verifies identity in its worker under the deletion lock.
What is the impact of these changes? Invalid aliases fail closed instead of replacing or exposing another checkpoint. A missing independent ID still returns False from delete, and a valid checkpoint remains loadable, updatable, and deletable when the filesystem changes its filename spelling. Directory scans no longer run synchronously on the event-loop thread. There is no public API change.
What do you want reviewers to focus on? The distinction between filename spelling and persisted identity, collision verification under destination ownership, and the save/load/delete worker boundaries. Regression coverage includes concurrent variant saves, decomposed directory listings, absent variants on case-sensitive filesystems, and deterministic slow-directory heartbeat checks.

Related Issue

Fixes #9444

AI Assistance

No material AI assistance was used.
This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: implementation, regression tests, and this PR description. The author reviewed the final diff and ran the targeted checkpoint tests, Ruff, formatting, and Pyright checks.

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 links to an agreed issue with no competing open PR, or the Related Issue section documents a trivial-change or repository-automation exception.
This is not a breaking change. If it is a breaking change, add the breaking change label (or add "[BREAKING]" to the title prefix, before or after any language prefix). A workflow keeps the label and title prefix in sync automatically.

…on save On case-insensitive filesystems (default Windows/macOS), Round-A.json and round-a.json are the same file. FileCheckpointStorage.save accepted both spellings, so the second save silently replaced the first checkpoint and reads of the first ID then returned the other workflow's state, with the first workflow's listing left empty. save now fails closed: when the destination exists under a differently-cased spelling, the save raises WorkflowCheckpointException naming the collision instead of replacing a different checkpoint's data. Case-sensitive filesystems skip the scan entirely (the variant path does not exist there), and re-saving the exact same ID stays a normal update everywhere. Tests: a conformance regression pins the rejection, the first checkpoint's survival, and the same-ID update path; red without the guard on a case-insensitive volume.

Copilot

🟡 Changes recommended

The collision guard still permits data loss in supported scenarios and can reject valid updates, while the test probe introduces a collection race.

4 open issues

What changed in this PR

This PR addresses #9444 by attempting to prevent case-variant checkpoint IDs from overwriting stored snapshots.

Changes:

Adds a filename-collision guard before checkpoint encoding and saving.
Adds regression coverage for collision rejection, snapshot preservation, and same-ID updates.

File	Description
python/packages/core/tests/workflow/test_checkpoint_storage_conformance.py	Adds filesystem detection and collision regression coverage.
python/packages/core/agent_framework/_workflows/_checkpoint.py	Checks for case-variant filenames before saving.

_{Review effort: Balanced}

Yufeng He (he-yufeng) · 2026-10-17T09:31:02Z

+        """
+        if not file_path.exists():
+            return
+        expected_name = file_path.name


Correct, fixed in 183ae7b. resolve() on Windows would hand back the on-disk casing, so the guard now derives the expected filename from the requested checkpoint ID (f"{checkpoint_id}.json") instead of file_path.name; the resolved path is still used only for the containment check and the directory scan.

Yufeng He (he-yufeng) · 2026-10-17T09:31:04Z

+        except OSError:
+            # A transient listing/read failure must not turn a routine save into a
+            # hard failure; the write below still fails loudly if the path is bad.
+            return


Agreed, fail-closed is the right call for a guard whose entire purpose is preventing silent data loss. In 183ae7b an OSError from iterdir() now raises WorkflowCheckpointException ("the storage directory could not be listed") instead of skipping the scan.

Yufeng He (he-yufeng) · 2026-10-17T09:31:07Z

+            return
+        expected_name = file_path.name
+        try:
+            for sibling in file_path.parent.iterdir():


Right, enumeration order made same-ID updates depend on which spelling appeared first. The guard in 183ae7b now collects the directory names once and allows the save when the exact spelling already exists; the case-folded raise only fires when no exact match is present, so coexistence on case-sensitive volumes works regardless of iterdir() order.

Yufeng He (he-yufeng) · 2026-10-17T09:31:09Z

+@pytest.mark.skipif(
+    not _filesystem_is_case_insensitive(Path(__file__).parent),
+    reason="case-variant collision only occurs on case-insensitive filesystems",
+)
+async def test_file_save_rejects_case_variant_checkpoint_id(tmp_path: Path) -> None:
+    """A case-only variant ID must not silently replace the other checkpoint (#9444)."""


Fixed in 183ae7b: the probe now runs inside the test against tmp_path (unique per xdist worker, and the volume that actually holds the checkpoints) and calls pytest.skip when the volume is case-sensitive. The module-level skipif decorator is gone.

…ization Address Copilot review on the microsoft#9444 fix: derive the expected filename from the requested checkpoint ID instead of the resolved path (Windows resolve() canonicalizes to on-disk casing, which would accept the colliding save), allow same-ID updates when the exact spelling already exists on case-sensitive volumes, and fail closed when the directory cannot be listed. Move the case-sensitivity probe into the test body on tmp_path so parallel xdist workers cannot race a shared collection-time probe.

Somay kousis (Somay-kousis)

Ran the PR's conformance file on macOS 26.6 APFS (case-insensitive, default volume):

main 44fe223 + PR test file: 1 failed, 18 passed (test_file_save_rejects_case_variant_checkpoint_id fails)
PR head 183ae7b: 19 passed

So the case-only collision from #9444 is fixed here.

One input still reaches the same overwrite: IDs that differ by Unicode normalization. APFS treats NFC and NFD spellings as one file ("café-1" vs "café-1"), and str.casefold() does not normalize, so _assert_no_case_variant_collision finds no variant and the second save replaces the first.

Script (public API only, same shape as the issue's repro): two saves, then load(first_id).

== main 44fe223d
case: load('Round-A') -> id='round-a' wf=second-workflow state={'marker': 'second'}; first-workflow ids=[]
nfc/nfd: load('café-1') -> id='café-1' wf=second-workflow state={'marker': 'second'}; first-workflow ids=[]
case+nfd: load('Café-1') -> id='café-1' wf=second-workflow state={'marker': 'second'}; first-workflow ids=[]
== PR head 183ae7b7
case: second save rejected: WorkflowCheckpointException
nfc/nfd: load('café-1') -> id='café-1' wf=second-workflow state={'marker': 'second'}; first-workflow ids=[]
case+nfd: load('Café-1') -> id='café-1' wf=second-workflow state={'marker': 'second'}; first-workflow ids=[]

(the printed IDs look identical; the second one is NFD, 7 code points vs 6)

Comparing normalized keys in the variant scan closes it:

def _key(name: str) -> str:
    return unicodedata.normalize("NFC", name).casefold()

variant = next(
    (name for name in siblings if name != expected_name and _key(name) == _key(expected_name)),
    None,
)

With that change, all three rows above raise WorkflowCheckpointException, and test_checkpoint_storage_conformance.py plus test_checkpoint.py still pass. Could the new regression be parametrized over ("Round-A", "round-a"), (NFC "café-1", NFD "café-1") and (NFC "Café-1", NFD "café-1")?

Not checked: Windows/NTFS and HFS+.

…lision guard APFS and HFS+ treat NFC and NFD spellings of a name as one file, and str.casefold() does not normalize, so a checkpoint ID differing only by normalization form still reached the silent overwrite from microsoft#9444. Compare siblings under NFC + casefold and reject those variants too. Thanks to Somay-kousis for the on-device report and the fix sketch.

Yufeng He (he-yufeng) · 2026-10-17T21:51:59Z

Somay kousis (@Somay-kousis) Thanks for the on-device verification and for the normalization catch. Landed in cddc816.

The guard now compares siblings under unicodedata.normalize("NFC", name).casefold() (essentially your _key sketch), so NFC and NFD spellings of the same ID are rejected the same way case variants are. The regression is parametrized as you suggested: the existing case pair keeps its own test (it folds on both NTFS and APFS), and the two new pairs (NFC "café-1", NFD "café-1") and (NFC "Café-1", NFD "café-1") run behind a normalization-folding probe so they exercise the real overwrite path on APFS and skip cleanly on volumes where the two spellings legitimately coexist.

On your "not checked" list: NTFS is normalization-sensitive, so the NFD save finds no existing file, the guard returns early, and both checkpoints coexist without an overwrite, which is safe (if visually confusing); the reject only engages where the filesystem actually folds. Reproduced the hole before the fix on this machine (APFS): both new parametrized cases fail against the previous guard and pass now, and the full checkpoint family (153 tests across the five checkpoint test files) plus pyright stay green.

Eduard van Valkenburg (eavanvalkenburg)

The sequential save guard is much improved, but the file backend still permits variant-ID data loss and identity aliasing in supported operations.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-22T13:23:28Z

        return file_path

+    @staticmethod
+    def _assert_no_variant_collision(file_path: Path, checkpoint_id: CheckpointID) -> None:


Please enforce the same exact checkpoint-ID identity on load() and delete(). On a case/normalization-folding filesystem, after saving Round-A, _validate_file_path("round-a") resolves to the same existing file: load("round-a") returns the other checkpoint and delete("round-a") deletes Round-A. Under the fail-closed policy, a variant ID should not read or remove a checkpoint whose stored ID/spelling differs; add regressions for both operations.

Both operations now go through _assert_stored_spelling, which compares the directory's stored spelling against the requested one and refuses when only a case/normalization fold brings them together: load("round-a") raises rather than returning Round-A's state, and delete("round-a") raises rather than unlinking it. Same-ID access and genuinely distinct IDs on a case-sensitive volume are unchanged. Regressions are test_file_load_rejects_case_variant_checkpoint_id and test_file_delete_rejects_case_variant_checkpoint_id in the conformance file.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-22T13:23:28Z

        from ._checkpoint_encoding import decode_checkpoint_value, encode_checkpoint_value

        file_path = self._validate_file_path(checkpoint.checkpoint_id)
+        self._assert_no_variant_collision(file_path, checkpoint.checkpoint_id)


This check occurs before _enqueue_write, so two first-time variant saves can both observe no destination and pass before either write lands; the second atomic replace can still overwrite the first. Please serialize by the filesystem-folded collision key and repeat the guard after the predecessor completes/immediately before replacement, with a concurrent-save regression. Cross-process guarantees can remain a separately documented limitation, but same-process saves should not retain the corruption race.

Two changes close it. The destination queue is now keyed by the filesystem-folded name (NFC + casefold) instead of the raw path, so variant spellings of one checkpoint share one queue and the second save cannot begin until the first has actually landed. And _write_atomic repeats the collision guard under that ownership, immediately before the replace, so a variant sibling created by the predecessor is seen and the save fails closed. test_concurrent_variant_saves_do_not_overwrite gathers both first-time variant saves and asserts exactly one is rejected with the survivor's data intact. Cross-process saves stay what the module comment says: atomic replace, undefined winner.

…e variant saves

Yufeng He (he-yufeng) · 2026-10-23T09:27:29Z

Status note for the reviewer: the red Public API Compatibility check is not from this diff. griffe crashes while resolving the base snapshot's agent_framework._workflows._edge.EdgeGroup.builtin_type alias (_edge.py:315, base side), which is being fixed separately in #9614. The same red shows on my other open PR (#9547), which touches unrelated code. Once #9614 lands on main this check should pass here with no changes on my side; the workflow tests (1439 passed, including the three new regression tests) are the meaningful signal for this PR.

…ase-collision

Yufeng He (he-yufeng) · 2026-10-23T13:05:17Z

Green now: merged main (which carries #9614) into the branch, and Public API Compatibility passes on the new head alongside the workflow tests. The two review items from your last pass are in 76b34fa, unchanged by the merge.

Somay kousis (Somay-kousis) · 2026-10-25T16:14:17Z

Sorry for the late reply. Reran on e6f935d: the case-only, NFC/NFD and case+NFD pairs all reject the second save with WorkflowCheckpointException now. Thanks.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-26T12:19:36Z

+            (name for name in siblings if name != expected_name and _fs_key(name) == expected_key),
+            None,
+        )
+        if variant is None or expected_name in siblings:


Yufeng He (@he-yufeng), HFS+ stores filenames in decomposed Unicode, so saving the NFC ID café-1 succeeds but directory enumeration returns cafe\u0301-1.json. The original ID then fails both this exact-spelling check on a same-ID update and _assert_stored_spelling on load/delete. A successfully saved checkpoint therefore cannot be loaded, updated, or deleted using the ID returned by save(). Please distinguish filesystem normalization from a genuinely different checkpoint ID using the ID persisted in the checkpoint or equivalent identity metadata, and cover an NFC round trip on a decomposing filesystem. Simply normalizing both spellings would lose the intended distinction between genuinely different NFC/NFD IDs.

Implemented in 8126b01dd99ea9977eea188fe09d9949dbacc46f.

Exact directory spelling no longer bypasses identity verification. Save, load, and delete compare the requested ID with the persisted checkpoint_id; load reuses the JSON it already read instead of reading the file twice.

The new decomposed-listing regression proves both sides: a genuinely different NFD ID is rejected even when its filename exactly matches the directory entry, while the original NFC ID can still be saved, loaded, and deleted. This is a deterministic simulation of the HFS+ listing behavior, not a claim that I ran the test on an HFS+ volume.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-26T12:19:38Z

+            ) from exc
+        if expected_name in siblings:
+            return
+        variant = next((name for name in siblings if _fs_key(name) == _fs_key(expected_name)), None)


Yufeng He (@he-yufeng), this rejects a folded sibling without checking whether the requested path actually exists. On a case-sensitive filesystem, after saving Round-A, delete("round-a") now raises WorkflowCheckpointException, even though round-a.json is absent and the documented result for a missing ID is False. These are independent IDs on that filesystem, so there is no alias to prevent. Please skip the spelling rejection when the requested destination does not exist and add a case-sensitive regression confirming that deleting the absent variant returns False without touching Round-A.

Implemented in 8126b01dd99ea9977eea188fe09d9949dbacc46f.

Delete now checks whether the requested destination exists before doing any variant verification, all inside the worker under the deletion lock. An absent independent round-a.json therefore returns False without touching Round-A.json.

The case-sensitive regression is included in the checkpoint conformance file. It is the single skipped test in this APFS run, so I am not claiming a local case-sensitive execution; the remaining 111 tests in the two-file checkpoint sweep passed.

Eduard van Valkenburg (eavanvalkenburg)

Yufeng He (@he-yufeng) One additional material issue remains in python/packages/core/agent_framework/_workflows/_checkpoint.py:661. The new collision guards synchronously enumerate the entire storage directory before the first await in load(), delete(), and existing-ID save(). Single-checkpoint operations now perform O(N) filesystem work on the event-loop thread, which can stall unrelated workflows and cancellation handling on large or slow directories. Please move the directory verification into worker-side/offloaded execution while preserving the collision check under destination ownership, and add an async regression showing the event loop remains responsive during a large scan.

I am not repeating the two existing HFS+ normalization and absent-case-variant concerns; those remain open separately.

# Conflicts: # python/packages/core/agent_framework/_workflows/_checkpoint.py

Yufeng He (he-yufeng) · 2026-10-27T16:13:53Z

Eduard van Valkenburg (@eavanvalkenburg) The event-loop blocking is addressed in 8126b01dd99ea9977eea188fe09d9949dbacc46f.

Save no longer performs a pre-enqueue directory scan; its collision check remains worker-side under destination ownership. Load performs existence checking, JSON reading, and identity verification in one asyncio.to_thread() call. Delete performs its existence check and identity verification in the worker while holding the deletion lock, returning False before a scan when the requested path is absent.

A new save/load/delete parameterized regression streams 2,000 directory entries behind a deterministic thread-controlled delay and verifies that an event-loop heartbeat continues during each operation. The old load and delete paths fail this check; the new paths pass.

Final checkpoint sweep: 111 passed, 1 skipped. The skip is the real case-sensitive-filesystem delete regression on this APFS host. Ruff, formatting, and Pyright on the changed module all pass.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-28T15:50:46Z

Yufeng He (@he-yufeng) The collision queue key at python/packages/core/agent_framework/_workflows/_checkpoint.py:398 still includes the storage directory as a case-sensitive Path. On a case-insensitive macOS filesystem, Path.resolve() can preserve different parent spellings: storage instances targeting Checkpoints and checkpoints have samefile() == True but acquire independent destination queues. Concurrent first-time saves of Round-A and round-a can then both observe no destination, pass their worker-side guards, and replace one another. A native directory-identity probe and exact-head enqueue probe confirmed distinct keys with no predecessor; a deterministic save reproduction returned success twice with only one checkpoint surviving. Please key ownership by the storage directory's actual filesystem identity plus the folded filename, preserving independence for genuinely distinct case-sensitive directories, and add a two-storage-instance concurrent-save regression.

Eduard van Valkenburg (eavanvalkenburg) · 2026-10-29T10:49:38Z

Yufeng He (@he-yufeng) There is a separate load/delete race in _assert_stored_spelling() at python/packages/core/agent_framework/_workflows/_checkpoint.py:714-716. On a case-insensitive filesystem, load("round-a") can finish reading Round-A.json, then delete("Round-A") removes the file before the load enumerates the directory. variant is then None, so this early return skips the persisted-ID comparison and the load returns Round-A's snapshot instead of rejecting the alias. I reproduced this deterministically with the exact-head module and an in-memory filesystem; the same load without the deletion correctly raises.

Please validate the ID in the already-read JSON regardless of whether a matching directory entry remains, and add an interleaved load/delete regression.

Yufeng He (he-yufeng) · 2026-10-29T13:08:00Z

The directory verification already runs off the event loop. _assert_stored_spelling executes inside asyncio.to_thread (load/delete), and _assert_no_variant_collision executes inside run_in_executor (save). Neither call site enumerates the directory on the event-loop thread before the first await.

The async regression you asked for is already in the branch as test_file_directory_verification_stays_off_event_loop, parameterized over save/load/delete. It monkey-patches Path.iterdir to block for up to 5 seconds and asserts a heartbeat coroutine running on the event loop fires at least 3 times during the scan. All 3 cases pass on the current head; the full conformance file is 28 passed, 1 skipped.

If the O(N) scan itself is the concern independent of which thread it runs on, replacing iterdir() with a targeted lookup would be a different change: the collision check needs the full sibling set to detect case/normalization variants, so a per-file exists() cannot replace it. Happy to discuss a bounded-scan approach if the O(N) cost matters on your target workloads.

Copilot AI balanced review requested due to automatic review settings October 16, 2026 21:16

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 16, 2026 21:16 — with GitHub Actions Inactive

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 16, 2026 21:17 — with GitHub Actions Inactive

agent-framework-automation Bot added the python Usage: [Issues, PRs], Target: Python label Oct 16, 2026

Copilot started reviewing on behalf of Yufeng He (he-yufeng) October 16, 2026 21:17

View session

Copilot AI reviewed Oct 16, 2026

View reviewed changes

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 17, 2026 09:30 — with GitHub Actions Inactive

Somay kousis (Somay-kousis) reviewed Oct 17, 2026

View reviewed changes

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 17, 2026 21:51 — with GitHub Actions Inactive

Eduard van Valkenburg (eavanvalkenburg) requested changes Oct 22, 2026

View reviewed changes

Python: fix(core): deny case-fold aliases in load/delete and serializ…

76b34fa

…e variant saves

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 23, 2026 04:07 — with GitHub Actions Inactive

Merge remote-tracking branch 'origin/main' into fix/file-checkpoint-c…

e6f935d

…ase-collision

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 23, 2026 13:03 — with GitHub Actions Inactive

Eduard van Valkenburg (eavanvalkenburg) reviewed Oct 26, 2026

View reviewed changes

Eduard van Valkenburg (eavanvalkenburg) requested changes Oct 27, 2026

View reviewed changes

Yufeng He (he-yufeng) added 2 commits October 27, 2026 23:47

Merge remote-tracking branch 'origin/main' into HEAD

45d1241

# Conflicts: # python/packages/core/agent_framework/_workflows/_checkpoint.py

Python: fix(core): preserve checkpoint identity across filesystem folds

8126b01

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 27, 2026 16:08 — with GitHub Actions Inactive

Yufeng He (he-yufeng) changed the title ~~Python: fix(core): reject checkpoint IDs that collide by letter case on save~~ Python: fix(core): preserve checkpoint identity across filesystem aliases Oct 27, 2026

Yufeng He (he-yufeng) temporarily deployed to github-app-auth October 27, 2026 16:12 — with GitHub Actions Inactive

Conversation

Yufeng He (he-yufeng) commented Oct 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation & Context

Description & Review Guide

Related Issue

AI Assistance

Contribution Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Somay kousis (Somay-kousis) left a comment

Choose a reason for hiding this comment

Uh oh!

Yufeng He (he-yufeng) commented Oct 17, 2026

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yufeng He (he-yufeng) commented Oct 23, 2026

Uh oh!

Yufeng He (he-yufeng) commented Oct 23, 2026

Uh oh!

Somay kousis (Somay-kousis) commented Oct 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) left a comment

Choose a reason for hiding this comment

Uh oh!

Yufeng He (he-yufeng) commented Oct 27, 2026

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) commented Oct 28, 2026

Uh oh!

Eduard van Valkenburg (eavanvalkenburg) commented Oct 29, 2026

Uh oh!

Yufeng He (he-yufeng) commented Oct 29, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Yufeng He (he-yufeng) commented Oct 16, 2026 •

edited

Loading