Skip to content

fix: register checkpoint handlers when CheckpointConfig is created#5320

Merged
greysonlalonde merged 5 commits into
mainfrom
fix/checkpoint-handler-registration
Apr 7, 2026
Merged

fix: register checkpoint handlers when CheckpointConfig is created#5320
greysonlalonde merged 5 commits into
mainfrom
fix/checkpoint-handler-registration

Conversation

@greysonlalonde

@greysonlalonde greysonlalonde commented Apr 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Checkpoint event handlers were never registered on the event bus during normal execution. The lazy registration in _resolve only ran from _find_checkpoint, which is called from the handler itself — a chicken-and-egg problem. Checkpoints were silently never written.

Adds a model_validator on CheckpointConfig that calls _ensure_handlers_registered when any config is created, including from checkpoint=True.

Test plan

  • Crew(checkpoint=True) writes checkpoints on task_completed
  • Crew(checkpoint=CheckpointConfig(...)) writes checkpoints
  • crewai checkpoint list shows written checkpoints

Note

Medium Risk
Changes checkpoint initialization to eagerly register event-bus handlers when checkpoint is enabled, which affects runtime side effects during model validation for Crew/Flow/Agent construction.

Overview
Fixes automatic checkpointing being silently inactive by ensuring checkpoint event handlers are registered as soon as checkpointing is configured.

Crew, Flow, and BaseAgent now run a BeforeValidator on the checkpoint field that coerces checkpoint=True into a CheckpointConfig and triggers handler registration. CheckpointConfig also adds an after model_validator to register handlers whenever a config instance is created, preventing the previous chicken-and-egg lazy registration path.

Reviewed by Cursor Bugbot for commit bee1506. Bugbot is set up for automated code reviews on this repo. Configure here.

@iris-clawd iris-clawd 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.

Review: Fix checkpoint handler registration

Good catch — classic chicken-and-egg bug. The fix is clean and minimal.

🟡 Side effect in model_validator

Registering global event bus handlers from a pydantic model_validator is a side effect that runs on every instantiation, including:

  • Deserialization (model_validate_json, model_validate)
  • Test fixtures that create CheckpointConfig for unit tests
  • Multiple CheckpointConfig instances in the same process

The _ensure_handlers_registered function is presumably idempotent (the name suggests it), so repeat calls should be fine. But worth confirming that the guard in _ensure_handlers_registered is threadsafe — if two Crews with checkpoint=True are created concurrently, you don't want double registration.

🟡 Circular import risk

The lazy import (from crewai.state.checkpoint_listener import _ensure_handlers_registered) inside the validator mitigates this, but checkpoint_config.pycheckpoint_listener.py already has a dependency via _find_checkpoint importing CheckpointConfig. This creates a circular import path: checkpoint_config → checkpoint_listener → checkpoint_config. The lazy import in the validator should break the cycle at runtime, but it's fragile — a future top-level import in either file could trigger an ImportError.

✅ What looks good

  • Minimal fix for a real bug (checkpoints silently never written)
  • Lazy import avoids immediate circular dependency
  • model_validator(mode="after") is the right hook — config is fully populated

The circular import fragility is the main concern. If you wanted to be safer, you could register the handlers from the Crew/Flow init instead of the config validator. But for alpha, this works. 💬 167

@iris-clawd iris-clawd 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.

Approving — review notes are suggestions for later hardening. Good to land. ✅

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 50f1096. Configure here.

Comment thread lib/crewai/src/crewai/state/checkpoint_config.py
@github-actions github-actions Bot added the size/S label Apr 7, 2026

@iris-clawd iris-clawd 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.

Re-review after new commits

Scope expanded: now includes a _coerce_checkpoint BeforeValidator wired into Crew, Flow, and BaseAgent. Good improvement over the first version.

✅ What's better

  • _coerce_checkpoint centralizes the True → CheckpointConfig() coercion that was probably scattered or missing before. Clean pattern.
  • Handler registration now happens both in the BeforeValidator (for checkpoint=True on Crew/Flow/Agent) and in the model_validator on CheckpointConfig itself (for direct instantiation). Belt and suspenders — good.

🟡 Double registration path

Both _coerce_checkpoint and CheckpointConfig._register_handlers call _ensure_handlers_registered(). When checkpoint=True is passed to a Crew:

  1. _coerce_checkpoint fires → creates CheckpointConfig() → calls _ensure_handlers_registered()
  2. CheckpointConfig.__init__ triggers _register_handlers model_validator → calls _ensure_handlers_registered() again

Harmless if idempotent, but redundant. Could drop the call from _coerce_checkpoint since the model_validator always covers it. Minor.

🟢 Import note

The _coerce_checkpoint function is imported at module level in crew.py, flow.py, and base_agent.py. Since it only does a lazy import of checkpoint_listener inside the function body (not at import time), this is safe. Good.

Looks solid. 💬 169

@iris-clawd iris-clawd 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.

Re-approving with updated commits. ✅

@greysonlalonde greysonlalonde merged commit c0f3151 into main Apr 7, 2026
49 checks passed
@greysonlalonde greysonlalonde deleted the fix/checkpoint-handler-registration branch April 7, 2026 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants