Skip to content

feat: Make checkpoint on new task #3834

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

samhvw8
Copy link
Collaborator

@samhvw8 samhvw8 commented May 22, 2025

Ensures that invoking the newTaskTool always creates a checkpoint, even if no files have changed. This provides a consistent state snapshot before a sub-task is initiated.

Related GitHub Issue

Closes: #

Description

Test Procedure

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) has been removed.
  • Testing:
    • New and/or updated tests have been added to cover my changes.
    • All tests pass locally (npm test).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch.
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates.
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

image

Documentation Updates

Additional Notes

Get in Touch


Important

Ensure newTaskTool creates a checkpoint even if no files change by forcing empty commits.

  • Behavior:
    • newTaskTool in newTaskTool.ts now calls checkpointSave with force=true to ensure a checkpoint is created even if no files have changed.
    • checkpointSave in index.ts updated to accept a force parameter, allowing empty commits.
  • Services:
    • saveCheckpoint in ShadowCheckpointService.ts updated to handle allowEmpty option for empty commits.
  • Misc:
    • Minor delay added after checkpoint creation in newTaskTool.ts.

This description was created by Ellipsis for 10535b4. You can customize this summary. It will automatically update as commits are pushed.

@daniel-lxs
Copy link
Collaborator

daniel-lxs commented May 22, 2025

Hi again @samhvw8,
Do you think it makes sense to add a new unit test to ShadowCheckpointService.test.ts for the force empty commit behavior?

Overall I think this is a great idea, many times I wanted to revert the changes made by a subtask only to find out there was no checkpoint.

@samhvw8
Copy link
Collaborator Author

samhvw8 commented May 22, 2025 via email

@hannesrudolph
Copy link
Collaborator

@cte Can we please merge this for now? this functionality of checkpoints with orchestrator is not useful. This would be far better than the current state of things and we don't have the time for a more complex solution at the moment.

@hannesrudolph hannesrudolph moved this from New to PR [Greenlit] in Roo Code Roadmap May 23, 2025
@cte
Copy link
Collaborator

cte commented May 23, 2025

@cte Can we please merge this for now? this functionality of checkpoints with orchestrator is not useful. This would be far better than the current state of things and we don't have the time for a more complex solution at the moment.

Yes, I think we can merge it after it goes through code review, though as I mentioned before this isn't the way I want to solve this problem, so fair warning that there's a chance that I will revert this and re-implement it later.

Ensures that invoking the `newTaskTool` always creates a checkpoint, even if no files have changed. This provides a consistent state snapshot before a sub-task is initiated.
@samhvw8 samhvw8 force-pushed the feat/make-checkpoint-on-new-task branch from 10535b4 to b518212 Compare May 23, 2025 05:07
@samhvw8
Copy link
Collaborator Author

samhvw8 commented May 23, 2025

@daniel-lxs Roo help me added test, can you check it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR [Greenlit]
Development

Successfully merging this pull request may close these issues.

4 participants