Skip to content

feat: add checkpoint before every user message #7074

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

feat: add checkpoint before every user message #7074

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
faff050
feat: add checkpoint before every user message
roomote-agent Aug 14, 2025
06d75cc
feat: save checkpoints immediately when users submit messages
roomote-agent Aug 14, 2025
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
12 changes: 12 additions & 0 deletions src/core/task/Task.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,18 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
// results.
const finalUserContent = [...parsedUserContent, { type: "text" as const, text: environmentDetails }]

// Save checkpoint before adding user message to conversation history
if (this.enableCheckpoints) {
try {
Copy link
Author

Choose a reason for hiding this comment

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

Consider extracting this checkpoint logic to a separate method like saveCheckpointBeforeUserMessage() for better code organization and potential reuse. The try-catch block with error handling could be its own private method.

await this.checkpointSave(true)
} catch (error) {
console.error(
Copy link
Author

Choose a reason for hiding this comment

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

Consider adding telemetry for checkpoint failures. When checkpoint saving fails, we're only logging to console. Should we also capture this in telemetry to track how often checkpoints fail in production?

`[Task#recursivelyMakeClineRequests] Error saving checkpoint before user message: ${error.message}`,
Copy link
Author

Choose a reason for hiding this comment

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

The error message could be more specific by including the task ID for better debugging context, e.g., [Task#${this.taskId}#recursivelyMakeClineRequests]

error,
)
}
}

await this.addToApiConversationHistory({ role: "user", content: finalUserContent })
TelemetryService.instance.captureConversationMessage(this.taskId, "user")

Expand Down
142 changes: 142 additions & 0 deletions src/core/task/__tests__/Task.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1613,5 +1613,147 @@ describe("Cline", () => {
consoleErrorSpy.mockRestore()
})
})

describe("Checkpoint before user messages", () => {
it("should save checkpoint before adding user message to conversation history", async () => {
Copy link
Author

Choose a reason for hiding this comment

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

Excellent test coverage! The three test cases comprehensively cover the happy path, error handling, and disabled state. Good use of spies to verify the order of operations (checkpoint before conversation history).

const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
enableCheckpoints: true,
startTask: false,
})

// Mock checkpointSave method
const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined)

// Mock addToApiConversationHistory
const addToApiConversationHistorySpy = vi
.spyOn(task as any, "addToApiConversationHistory")
.mockResolvedValue(undefined)

// Mock other required methods
vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined)
vi.spyOn(task.api, "createMessage").mockReturnValue({
async *[Symbol.asyncIterator]() {
yield { type: "text", text: "response" }
},
} as any)

// Mock clineMessages to avoid errors
task.clineMessages = [
{
ts: Date.now(),
type: "say",
say: "api_req_started",
text: JSON.stringify({ request: "test" }),
},
]

// Call recursivelyMakeClineRequests which should trigger checkpoint save
await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }])

// Verify checkpoint was saved before adding to conversation history
expect(checkpointSaveSpy).toHaveBeenCalledWith(true)
expect(checkpointSaveSpy).toHaveBeenCalledBefore(addToApiConversationHistorySpy)
expect(addToApiConversationHistorySpy).toHaveBeenCalled()
})

it("should handle checkpoint save errors gracefully", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
enableCheckpoints: true,
startTask: false,
})

// Mock checkpointSave to throw an error
const checkpointError = new Error("Checkpoint save failed")
const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockRejectedValue(checkpointError)

// Mock console.error to verify error logging
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})

// Mock addToApiConversationHistory to verify it still gets called
const addToApiConversationHistorySpy = vi
.spyOn(task as any, "addToApiConversationHistory")
.mockResolvedValue(undefined)

// Mock other required methods
vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined)
vi.spyOn(task.api, "createMessage").mockReturnValue({
async *[Symbol.asyncIterator]() {
yield { type: "text", text: "response" }
},
} as any)

// Mock clineMessages
task.clineMessages = [
{
ts: Date.now(),
type: "say",
say: "api_req_started",
text: JSON.stringify({ request: "test" }),
},
]

// Call recursivelyMakeClineRequests
await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }])

// Verify checkpoint save was attempted
expect(checkpointSaveSpy).toHaveBeenCalledWith(true)

// Verify error was logged
expect(consoleErrorSpy).toHaveBeenCalledWith(
expect.stringContaining("Error saving checkpoint before user message"),
checkpointError,
)

// Verify conversation history was still updated despite checkpoint error
expect(addToApiConversationHistorySpy).toHaveBeenCalled()

// Restore console.error
consoleErrorSpy.mockRestore()
})

it("should not save checkpoint when checkpoints are disabled", async () => {
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
task: "test task",
enableCheckpoints: false,
startTask: false,
})

// Mock checkpointSave method
const checkpointSaveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue(undefined)

// Mock other required methods
vi.spyOn(task as any, "addToApiConversationHistory").mockResolvedValue(undefined)
vi.spyOn(task as any, "saveClineMessages").mockResolvedValue(undefined)
vi.spyOn(task.api, "createMessage").mockReturnValue({
async *[Symbol.asyncIterator]() {
yield { type: "text", text: "response" }
},
} as any)

// Mock clineMessages
task.clineMessages = [
{
ts: Date.now(),
type: "say",
say: "api_req_started",
text: JSON.stringify({ request: "test" }),
},
]

// Call recursivelyMakeClineRequests
await task.recursivelyMakeClineRequests([{ type: "text", text: "test user message" }])

// Verify checkpoint was NOT saved
expect(checkpointSaveSpy).not.toHaveBeenCalled()
})
})
})
})
Loading