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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/core/checkpoints/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ async function getInitializedCheckpointService(
}
}

export async function checkpointSave(cline: Task) {
export async function checkpointSave(cline: Task, force = false) {
const service = getCheckpointService(cline)

if (!service) {
Expand All @@ -169,7 +169,7 @@ export async function checkpointSave(cline: Task) {
telemetryService.captureCheckpointCreated(cline.taskId)

// Start the checkpoint process in the background.
return service.saveCheckpoint(`Task: ${cline.taskId}, Time: ${Date.now()}`).catch((err) => {
return service.saveCheckpoint(`Task: ${cline.taskId}, Time: ${Date.now()}`, { allowEmpty: force }).catch((err) => {
console.error("[Cline#checkpointSave] caught unexpected error, disabling checkpoints", err)
cline.enableCheckpoints = false
})
Expand Down
4 changes: 2 additions & 2 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1713,8 +1713,8 @@ export class Task extends EventEmitter<ClineEvents> {

// Checkpoints

public async checkpointSave() {
return checkpointSave(this)
public async checkpointSave(force: boolean = false) {
return checkpointSave(this, force)
}

public async checkpointRestore(options: CheckpointRestoreOptions) {
Expand Down
5 changes: 5 additions & 0 deletions src/core/tools/newTaskTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ export async function newTaskTool(
return
}

if (cline.enableCheckpoints) {
cline.checkpointSave(true)
await delay(350)
}

// Preserve the current mode so we can resume with it later.
cline.pausedModeSlug = (await provider.getState()).mode ?? defaultModeSlug

Expand Down
12 changes: 9 additions & 3 deletions src/services/checkpoints/ShadowCheckpointService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,17 +214,23 @@ export abstract class ShadowCheckpointService extends EventEmitter {
return this.shadowGitConfigWorktree
}

public async saveCheckpoint(message: string): Promise<CheckpointResult | undefined> {
public async saveCheckpoint(
message: string,
options?: { allowEmpty?: boolean },
): Promise<CheckpointResult | undefined> {
try {
this.log(`[${this.constructor.name}#saveCheckpoint] starting checkpoint save`)
this.log(
`[${this.constructor.name}#saveCheckpoint] starting checkpoint save (allowEmpty: ${options?.allowEmpty ?? false})`,
)

if (!this.git) {
throw new Error("Shadow git repo not initialized")
}

const startTime = Date.now()
await this.stageAll(this.git)
const result = await this.git.commit(message)
const commitArgs = options?.allowEmpty ? { "--allow-empty": null } : undefined
const result = await this.git.commit(message, commitArgs)
const isFirst = this._checkpoints.length === 0
const fromHash = this._checkpoints[this._checkpoints.length - 1] ?? this.baseHash!
const toHash = result.commit || fromHash
Expand Down
175 changes: 175 additions & 0 deletions src/services/checkpoints/__tests__/ShadowCheckpointService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -632,5 +632,180 @@ describe.each([[RepoPerTaskCheckpointService, "RepoPerTaskCheckpointService"]])(
expect(checkpointHandler).not.toHaveBeenCalled()
})
})

describe(`${klass.name}#saveCheckpoint with allowEmpty option`, () => {
it("creates checkpoint with allowEmpty=true even when no changes", async () => {
// No changes made, but force checkpoint creation
const result = await service.saveCheckpoint("Empty checkpoint", { allowEmpty: true })

expect(result).toBeDefined()
expect(result?.commit).toBeTruthy()
expect(typeof result?.commit).toBe("string")
})

it("does not create checkpoint with allowEmpty=false when no changes", async () => {
const result = await service.saveCheckpoint("No changes checkpoint", { allowEmpty: false })

expect(result).toBeUndefined()
})

it("does not create checkpoint by default when no changes", async () => {
const result = await service.saveCheckpoint("Default behavior checkpoint")

expect(result).toBeUndefined()
})

it("creates checkpoint with changes regardless of allowEmpty setting", async () => {
await fs.writeFile(testFile, "Modified content for allowEmpty test")

const resultWithAllowEmpty = await service.saveCheckpoint("With changes and allowEmpty", { allowEmpty: true })
expect(resultWithAllowEmpty?.commit).toBeTruthy()

await fs.writeFile(testFile, "Another modification for allowEmpty test")

const resultWithoutAllowEmpty = await service.saveCheckpoint("With changes, no allowEmpty")
expect(resultWithoutAllowEmpty?.commit).toBeTruthy()
})

it("emits checkpoint event for empty commits when allowEmpty=true", async () => {
const checkpointHandler = jest.fn()
service.on("checkpoint", checkpointHandler)

const result = await service.saveCheckpoint("Empty checkpoint event test", { allowEmpty: true })

expect(checkpointHandler).toHaveBeenCalledTimes(1)
const eventData = checkpointHandler.mock.calls[0][0]
expect(eventData.type).toBe("checkpoint")
expect(eventData.toHash).toBe(result?.commit)
expect(typeof eventData.duration).toBe("number")
expect(typeof eventData.isFirst).toBe("boolean") // Can be true or false depending on checkpoint history
})

it("does not emit checkpoint event when no changes and allowEmpty=false", async () => {
// First, create a checkpoint to ensure we're not in the initial state
await fs.writeFile(testFile, "Setup content")
await service.saveCheckpoint("Setup checkpoint")

// Reset the file to original state
await fs.writeFile(testFile, "Hello, world!")
await service.saveCheckpoint("Reset to original")

// Now test with no changes and allowEmpty=false
const checkpointHandler = jest.fn()
service.on("checkpoint", checkpointHandler)

const result = await service.saveCheckpoint("No changes, no event", { allowEmpty: false })

expect(result).toBeUndefined()
expect(checkpointHandler).not.toHaveBeenCalled()
})

it("handles multiple empty checkpoints correctly", async () => {
const commit1 = await service.saveCheckpoint("First empty checkpoint", { allowEmpty: true })
expect(commit1?.commit).toBeTruthy()

const commit2 = await service.saveCheckpoint("Second empty checkpoint", { allowEmpty: true })
expect(commit2?.commit).toBeTruthy()

// Commits should be different
expect(commit1?.commit).not.toBe(commit2?.commit)
})

it("logs correct message for allowEmpty option", async () => {
const logMessages: string[] = []
const testService = await klass.create({
taskId: "log-test",
shadowDir: path.join(tmpDir, `log-test-${Date.now()}`),
workspaceDir: service.workspaceDir,
log: (message: string) => logMessages.push(message),
})
await testService.initShadowGit()

await testService.saveCheckpoint("Test logging with allowEmpty", { allowEmpty: true })

const saveCheckpointLogs = logMessages.filter(msg =>
msg.includes("starting checkpoint save") && msg.includes("allowEmpty: true")
)
expect(saveCheckpointLogs).toHaveLength(1)

await testService.saveCheckpoint("Test logging without allowEmpty")

const defaultLogs = logMessages.filter(msg =>
msg.includes("starting checkpoint save") && msg.includes("allowEmpty: false")
)
expect(defaultLogs).toHaveLength(1)
})

it("maintains checkpoint history with empty commits", async () => {
// Create a regular checkpoint
await fs.writeFile(testFile, "Regular change")
const regularCommit = await service.saveCheckpoint("Regular checkpoint")
expect(regularCommit?.commit).toBeTruthy()

// Create an empty checkpoint
const emptyCommit = await service.saveCheckpoint("Empty checkpoint", { allowEmpty: true })
expect(emptyCommit?.commit).toBeTruthy()

// Create another regular checkpoint
await fs.writeFile(testFile, "Another regular change")
const anotherCommit = await service.saveCheckpoint("Another regular checkpoint")
expect(anotherCommit?.commit).toBeTruthy()

// Verify we can restore to the empty checkpoint
await service.restoreCheckpoint(emptyCommit!.commit)
expect(await fs.readFile(testFile, "utf-8")).toBe("Regular change")

// Verify we can restore to other checkpoints
await service.restoreCheckpoint(regularCommit!.commit)
expect(await fs.readFile(testFile, "utf-8")).toBe("Regular change")

await service.restoreCheckpoint(anotherCommit!.commit)
expect(await fs.readFile(testFile, "utf-8")).toBe("Another regular change")
})

it("handles getDiff correctly with empty commits", async () => {
// Create a regular checkpoint
await fs.writeFile(testFile, "Content before empty")
const beforeEmpty = await service.saveCheckpoint("Before empty")
expect(beforeEmpty?.commit).toBeTruthy()

// Create an empty checkpoint
const emptyCommit = await service.saveCheckpoint("Empty checkpoint", { allowEmpty: true })
expect(emptyCommit?.commit).toBeTruthy()

// Get diff between regular commit and empty commit
const diff = await service.getDiff({
from: beforeEmpty!.commit,
to: emptyCommit!.commit
})

// Should have no differences since empty commit doesn't change anything
expect(diff).toHaveLength(0)
})

it("works correctly in integration with new task workflow", async () => {
// Simulate the new task workflow where we force a checkpoint even with no changes
// This tests the specific use case mentioned in the git commit

// Start with a clean state (no pending changes)
const initialState = await service.saveCheckpoint("Check initial state")
expect(initialState).toBeUndefined() // No changes, so no commit

// Force a checkpoint for new task (this is the new functionality)
const newTaskCheckpoint = await service.saveCheckpoint("New task checkpoint", { allowEmpty: true })
expect(newTaskCheckpoint?.commit).toBeTruthy()

// Verify the checkpoint was created and can be restored
await fs.writeFile(testFile, "Work done in new task")
const workCommit = await service.saveCheckpoint("Work in new task")
expect(workCommit?.commit).toBeTruthy()

// Restore to the new task checkpoint
await service.restoreCheckpoint(newTaskCheckpoint!.commit)

// File should be back to original state
expect(await fs.readFile(testFile, "utf-8")).toBe("Hello, world!")
})
})
},
)