Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async checkpoints #1346

Merged
merged 2 commits into from
Mar 3, 2025
Merged

Async checkpoints #1346

merged 2 commits into from
Mar 3, 2025

Conversation

cte
Copy link
Collaborator

@cte cte commented Mar 3, 2025

Context

Implement an EventEmitter in ShadowCheckpointService so we can call it's methods without awaiting and have a convenient way to listening for completion without blocking the chat UI.

I'm also deleting the LocalCheckpointService since in retrospect I think it was a bad idea not to have full isolation from the local git repo. However, some of the techniques could can be reused in a variant of the ShadowCheckpointService that is branch-per-task instead of repo-per-task. Cline is dabbling with a similar optimization.

Note that saveCheckpoint will throw if a tool action triggers a checkpoint save and the service isn't initialized yet. As a follow-up we can optionally surface this error to user.

Open question; if checkpoints are problematic should this code attempt to update your settings so that checkpoints are effectively off by default going forward?

Implementation

Screenshots

before after

How to Test

Get in Touch


Important

Replaces LocalCheckpointService with async ShadowCheckpointService using event-driven checkpoints in Cline.ts.

  • Behavior:
    • Introduces ShadowCheckpointService in Cline.ts for async checkpoint handling.
    • Removes LocalCheckpointService and CheckpointServiceFactory.
    • Checkpoints are now event-driven using CheckpointEventEmitter.
    • saveCheckpoint and restoreCheckpoint methods in ShadowCheckpointService emit events.
  • Tests:
    • Adds tests for ShadowCheckpointService in ShadowCheckpointService.test.ts.
    • Tests cover checkpoint creation, restoration, and event emission.
  • Misc:
    • Updates CheckpointSaved.tsx to use new checkpoint schema.
    • Removes strategy and version from checkpointSchema in schema.ts.

This description was created by Ellipsis for e227a9e. It will automatically update as commits are pushed.

@cte cte requested a review from mrubens as a code owner March 3, 2025 20:29
Copy link

changeset-bot bot commented Mar 3, 2025

⚠️ No Changeset found

Latest commit: e227a9e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. enhancement New feature or request labels Mar 3, 2025

this.isInitialized = true

this.emit("initialize", {
Copy link

Choose a reason for hiding this comment

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

Consider adding inline JSDoc comments for the emitted 'initialize' event to clarify what data (e.g. workspaceDir, baseHash, created, duration) callers can expect.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 3, 2025
@cte cte merged commit 939e4fc into main Mar 3, 2025
11 checks passed
@cte cte deleted the cte/async-checkpoints branch March 3, 2025 20:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request lgtm This PR has been approved by a maintainer size:XXL This PR changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants