Skip to content

Commit ed765a3

Browse files
roomote[bot]roomotehannesrudolphdaniel-lxs
authored
feat(checkpoints): create checkpoint when user sends a message (#7713)
* feat(checkpoints): create checkpoint on user message send * fix(checkpoints): suppress implicit user-message checkpoint row; keep current checkpoint updated without a chat row * Fix checkpoint suppression for user messages - Propagate suppressMessage flag through event chain properly - Update ChatView to check checkpoint metadata for suppressMessage flag - Ensure checkpoint messages are created but not rendered when suppressed - Fix bug where checkpointSave(false) should have been checkpointSave(true) * fix: only create checkpoint on user message when files have changed - Changed allowEmpty from true to false in checkpointSave call - Checkpoints will now only be created when there are actual file changes - This avoids creating empty commits in the shadow git repository * test: update checkpoint test to include suppressMessage parameter - Fixed test expectation to match the new function signature - saveCheckpoint now expects both allowEmpty and suppressMessage parameters --------- Co-authored-by: Roo Code <[email protected]> Co-authored-by: Hannes Rudolph <[email protected]> Co-authored-by: Daniel Riccio <[email protected]>
1 parent ae01a90 commit ed765a3

File tree

6 files changed

+57
-19
lines changed

6 files changed

+57
-19
lines changed

src/core/checkpoints/__tests__/checkpoint.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ describe("Checkpoint functionality", () => {
111111
// saveCheckpoint should have been called
112112
expect(mockCheckpointService.saveCheckpoint).toHaveBeenCalledWith(
113113
expect.stringContaining("Task: test-task-id"),
114-
{ allowEmpty: true },
114+
{ allowEmpty: true, suppressMessage: false },
115115
)
116116

117117
// Result should contain the commit hash

src/core/checkpoints/index.ts

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,13 +131,26 @@ async function checkGitInstallation(
131131
task.checkpointServiceInitializing = false
132132
})
133133

134-
service.on("checkpoint", ({ fromHash: from, toHash: to }) => {
134+
service.on("checkpoint", ({ fromHash: from, toHash: to, suppressMessage }) => {
135135
try {
136-
provider?.postMessageToWebview({ type: "currentCheckpointUpdated", text: to })
136+
// Always update the current checkpoint hash in the webview, including the suppress flag
137+
provider?.postMessageToWebview({
138+
type: "currentCheckpointUpdated",
139+
text: to,
140+
suppressMessage: !!suppressMessage,
141+
})
137142

138-
task.say("checkpoint_saved", to, undefined, undefined, { from, to }, undefined, {
139-
isNonInteractive: true,
140-
}).catch((err) => {
143+
// Always create the chat message but include the suppress flag in the payload
144+
// so the chatview can choose not to render it while keeping it in history.
145+
task.say(
146+
"checkpoint_saved",
147+
to,
148+
undefined,
149+
undefined,
150+
{ from, to, suppressMessage: !!suppressMessage },
151+
undefined,
152+
{ isNonInteractive: true },
153+
).catch((err) => {
141154
log("[Task#getCheckpointService] caught unexpected error in say('checkpoint_saved')")
142155
console.error(err)
143156
})
@@ -164,7 +177,7 @@ async function checkGitInstallation(
164177
}
165178
}
166179

167-
export async function checkpointSave(task: Task, force = false) {
180+
export async function checkpointSave(task: Task, force = false, suppressMessage = false) {
168181
const service = await getCheckpointService(task)
169182

170183
if (!service) {
@@ -174,10 +187,12 @@ export async function checkpointSave(task: Task, force = false) {
174187
TelemetryService.instance.captureCheckpointCreated(task.taskId)
175188

176189
// Start the checkpoint process in the background.
177-
return service.saveCheckpoint(`Task: ${task.taskId}, Time: ${Date.now()}`, { allowEmpty: force }).catch((err) => {
178-
console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
179-
task.enableCheckpoints = false
180-
})
190+
return service
191+
.saveCheckpoint(`Task: ${task.taskId}, Time: ${Date.now()}`, { allowEmpty: force, suppressMessage })
192+
.catch((err) => {
193+
console.error("[Task#checkpointSave] caught unexpected error, disabling checkpoints", err)
194+
task.enableCheckpoints = false
195+
})
181196
}
182197

183198
export type CheckpointRestoreOptions = {

src/core/task/Task.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -890,6 +890,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
890890
this.askResponseText = text
891891
this.askResponseImages = images
892892

893+
// Create a checkpoint whenever the user sends a message.
894+
// Use allowEmpty=true to ensure a checkpoint is recorded even if there are no file changes.
895+
// Suppress the checkpoint_saved chat row for this particular checkpoint to keep the timeline clean.
896+
if (askResponse === "messageResponse") {
897+
void this.checkpointSave(false, true)
898+
}
899+
893900
// Mark the last follow-up question as answered
894901
if (askResponse === "messageResponse" || askResponse === "yesButtonClicked") {
895902
// Find the last unanswered follow-up message using findLastIndex
@@ -2774,8 +2781,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
27742781

27752782
// Checkpoints
27762783

2777-
public async checkpointSave(force: boolean = false) {
2778-
return checkpointSave(this, force)
2784+
public async checkpointSave(force: boolean = false, suppressMessage: boolean = false) {
2785+
return checkpointSave(this, force, suppressMessage)
27792786
}
27802787

27812788
public async checkpointRestore(options: CheckpointRestoreOptions) {

src/services/checkpoints/ShadowCheckpointService.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
200200

201201
public async saveCheckpoint(
202202
message: string,
203-
options?: { allowEmpty?: boolean },
203+
options?: { allowEmpty?: boolean; suppressMessage?: boolean },
204204
): Promise<CheckpointResult | undefined> {
205205
try {
206206
this.log(
@@ -221,7 +221,13 @@ export abstract class ShadowCheckpointService extends EventEmitter {
221221
const duration = Date.now() - startTime
222222

223223
if (result.commit) {
224-
this.emit("checkpoint", { type: "checkpoint", fromHash, toHash, duration })
224+
this.emit("checkpoint", {
225+
type: "checkpoint",
226+
fromHash,
227+
toHash,
228+
duration,
229+
suppressMessage: options?.suppressMessage ?? false,
230+
})
225231
}
226232

227233
if (result.commit) {

src/services/checkpoints/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export interface CheckpointEventMap {
2828
fromHash: string
2929
toHash: string
3030
duration: number
31+
suppressMessage?: boolean
3132
}
3233
restore: { type: "restore"; commitHash: string; duration: number }
3334
error: { type: "error"; error: Error }

webview-ui/src/components/chat/ChatView.tsx

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -859,10 +859,19 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
859859
// Remove the 500-message limit to prevent array index shifting
860860
// Virtuoso is designed to efficiently handle large lists through virtualization
861861
const newVisibleMessages = modifiedMessages.filter((message) => {
862-
// Filter out checkpoint_saved messages that are associated with user messages
863-
if (message.say === "checkpoint_saved" && message.text) {
864-
// Use O(1) Set lookup instead of O(n) array search
865-
if (userMessageCheckpointHashes.has(message.text)) {
862+
// Filter out checkpoint_saved messages that should be suppressed
863+
if (message.say === "checkpoint_saved") {
864+
// Check if this checkpoint has the suppressMessage flag set
865+
if (
866+
message.checkpoint &&
867+
typeof message.checkpoint === "object" &&
868+
"suppressMessage" in message.checkpoint &&
869+
message.checkpoint.suppressMessage
870+
) {
871+
return false
872+
}
873+
// Also filter out checkpoint messages associated with user messages (legacy behavior)
874+
if (message.text && userMessageCheckpointHashes.has(message.text)) {
866875
return false
867876
}
868877
}

0 commit comments

Comments
 (0)