Description
FileCheckpointStorage accepts at save time state that it refuses at load time, and the refusal is silent end to end. Four related behaviors, all verified on main @ 8e803b9 with the standalone repro below:
-
Save does not validate types; load does. save() encodes any picklable object with no whitelist check; the restricted-pickle whitelist is enforced only in load() via decode_checkpoint_value. A workflow state containing an application class (not registered via allowed_checkpoint_types / register_checkpoint_type) saves successfully, then fails to restore with WorkflowCheckpointException. The failure surfaces at restore time, potentially in a different process, days later. InMemoryCheckpointStorage round-trips the same state without error, so identical workflow code works under the default test storage and breaks against the durable one.
-
Blocked checkpoints become invisible to get_latest. list_checkpoints skips files that fail decode with only a log warning (pinned as graceful in test_file_checkpoint_storage_corrupted_file), and get_latest builds on it. A workflow whose saved checkpoint contains an unregistered type gets get_latest() is None, which per the CheckpointStorage protocol means "no checkpoints exist" — except one does exist (see 3). A resume flow driven by get_latest sees nothing and, combined with 1, nothing was ever loud.
-
The two listing APIs disagree. list_checkpoint_ids reads raw JSON without decoding, so it still reports the blocked checkpoint (1 id) while list_checkpoints and get_latest report none. Two APIs answering the same question differently is a contract split of its own.
-
Corrupt files make load() raise json.JSONDecodeError, not the documented WorkflowCheckpointException. The CheckpointStorage.load docstring says it raises WorkflowCheckpointException "if checkpoint decoding fails", but the JSON read itself is unwrapped, so a truncated or corrupt checkpoint file surfaces as a raw json.JSONDecodeError. The pinned test only covers list_checkpoints graceful handling, not load's exception type.
I am not sure how much of 1-3 is intended design (register-the-type is the documented remedy, and fail-open listing is pinned), so the questions I'd pose: should save() validate against the whitelist (or at least warn) so the failure is loud at capture time; should get_latest distinguish "no checkpoints" from "checkpoints exist but were skipped"; should list_checkpoint_ids and list_checkpoints agree; and should load wrap corrupt-file errors in the documented exception? Item 4 looks like a plain bug regardless of the others.
Found via a differential probe of the two storages; AI assistance disclosed; repro is standalone and offline.
Reproduction Steps
import asyncio, sys, tempfile
from dataclasses import dataclass, field
sys.path.insert(0, "python/packages/core")
from agent_framework._workflows._checkpoint import (
FileCheckpointStorage, InMemoryCheckpointStorage, WorkflowCheckpoint,
)
@dataclass
class ProbeState: # any application class not registered for checkpoint decode
counter: int = 0
cp = WorkflowCheckpoint(workflow_name="wf", graph_signature_hash="s", state={"e": ProbeState(7)})
async def main():
mem = InMemoryCheckpointStorage()
await mem.save(cp)
print(await mem.load(cp.checkpoint_id)) # loads fine, ProbeState(7)
with tempfile.TemporaryDirectory() as d:
fcs = FileCheckpointStorage(d)
await fcs.save(cp) # succeeds, no warning
try:
await fcs.load(cp.checkpoint_id) # WorkflowCheckpointException
except Exception as e:
print(type(e).__name__, str(e)[:90])
print(await fcs.list_checkpoints(workflow_name="wf")) # []
print(await fcs.get_latest(workflow_name="wf")) # None
print(await fcs.list_checkpoint_ids(workflow_name="wf")) # [id] -- disagrees
# item 4: corrupt file
with tempfile.TemporaryDirectory() as d:
fcs = FileCheckpointStorage(d)
cp2 = WorkflowCheckpoint(workflow_name="wf", graph_signature_hash="s", state={"x": 1})
await fcs.save(cp2)
p = __import__("pathlib").Path(d) / f"{cp2.checkpoint_id}.json"
raw = p.read_text(); p.write_text(raw[: len(raw) // 2])
try:
await fcs.load(cp2.checkpoint_id)
except Exception as e:
print(type(e).__name__) # json.JSONDecodeError
asyncio.run(main())
Environment
- agent-framework-core @ main 8e803b9 (editable, python/packages/core)
- Python 3.12, macOS, offline repro (no external services)
Description
FileCheckpointStorageaccepts at save time state that it refuses at load time, and the refusal is silent end to end. Four related behaviors, all verified on main @ 8e803b9 with the standalone repro below:Save does not validate types; load does.
save()encodes any picklable object with no whitelist check; the restricted-pickle whitelist is enforced only inload()viadecode_checkpoint_value. A workflow state containing an application class (not registered viaallowed_checkpoint_types/register_checkpoint_type) saves successfully, then fails to restore withWorkflowCheckpointException. The failure surfaces at restore time, potentially in a different process, days later.InMemoryCheckpointStorageround-trips the same state without error, so identical workflow code works under the default test storage and breaks against the durable one.Blocked checkpoints become invisible to
get_latest.list_checkpointsskips files that fail decode with only a log warning (pinned as graceful intest_file_checkpoint_storage_corrupted_file), andget_latestbuilds on it. A workflow whose saved checkpoint contains an unregistered type getsget_latest() is None, which per theCheckpointStorageprotocol means "no checkpoints exist" — except one does exist (see 3). A resume flow driven byget_latestsees nothing and, combined with 1, nothing was ever loud.The two listing APIs disagree.
list_checkpoint_idsreads raw JSON without decoding, so it still reports the blocked checkpoint (1 id) whilelist_checkpointsandget_latestreport none. Two APIs answering the same question differently is a contract split of its own.Corrupt files make
load()raisejson.JSONDecodeError, not the documentedWorkflowCheckpointException. TheCheckpointStorage.loaddocstring says it raisesWorkflowCheckpointException"if checkpoint decoding fails", but the JSON read itself is unwrapped, so a truncated or corrupt checkpoint file surfaces as a rawjson.JSONDecodeError. The pinned test only coverslist_checkpointsgraceful handling, notload's exception type.I am not sure how much of 1-3 is intended design (register-the-type is the documented remedy, and fail-open listing is pinned), so the questions I'd pose: should
save()validate against the whitelist (or at least warn) so the failure is loud at capture time; shouldget_latestdistinguish "no checkpoints" from "checkpoints exist but were skipped"; shouldlist_checkpoint_idsandlist_checkpointsagree; and shouldloadwrap corrupt-file errors in the documented exception? Item 4 looks like a plain bug regardless of the others.Found via a differential probe of the two storages; AI assistance disclosed; repro is standalone and offline.
Reproduction Steps
Environment