Skip to content

Commit 720b2bf

Browse files
easonLiangWorldedtecheasonLiangWorldedtech
committed
fix(checkpoints): keep the task-start baseline from broadcasting a current checkpoint
The baseline save records the workspace state before the task writes anything. Broadcasting it as the webview current checkpoint makes every completed task offer a See New Changes affordance pointing at a commit that holds no user change. Only that broadcast is suppressed, and only for that save: the commit is still produced with allowEmpty and still recorded in checkpoint history, so rollback keeps its baseline restore point. Saves a user can act on, including the per-write checkpoints and the post-loop save, keep broadcasting normally.
1 parent e3dd44c commit 720b2bf

6 files changed

Lines changed: 171 additions & 16 deletions

File tree

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

Lines changed: 111 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ describe("Checkpoint functionality", () => {
131131
// saveCheckpoint should have been called
132132
expect(mockCheckpointService.saveCheckpoint).toHaveBeenCalledWith(
133133
expect.stringContaining("Task: test-task-id"),
134-
{ allowEmpty: true, suppressMessage: false },
134+
{ allowEmpty: true, suppressMessage: false, suppressBroadcast: false },
135135
)
136136

137137
// Result should contain the commit hash
@@ -473,6 +473,116 @@ describe("Checkpoint functionality", () => {
473473
})
474474
})
475475

476+
describe("checkpointSave - task-start baseline broadcast suppression", () => {
477+
type CheckpointPayload = {
478+
type: "checkpoint"
479+
fromHash: string
480+
toHash: string
481+
duration: number
482+
suppressMessage?: boolean
483+
suppressBroadcast?: boolean
484+
}
485+
type PostedWebviewMessage = { type?: string; text?: string; suppressMessage?: boolean }
486+
487+
const broadcastPosts = (): PostedWebviewMessage[] =>
488+
mockProvider.postMessageToWebview.mock.calls
489+
.map(([message]: PostedWebviewMessage[]) => message)
490+
.filter((message: PostedWebviewMessage) => message?.type === "currentCheckpointUpdated")
491+
492+
const registerCheckpointHandler = async () => {
493+
// The handler is registered when the service is created, so drop the pre-made
494+
// double and let getCheckpointService wire it up against the same double.
495+
mockTask.checkpointService = undefined
496+
mockTask.checkpointServiceInitializing = false
497+
498+
await getCheckpointService(mockTask)
499+
500+
const registration = mockCheckpointService.on.mock.calls.find(
501+
([event]: [string]) => event === "checkpoint",
502+
) as [string, (payload: CheckpointPayload) => void] | undefined
503+
if (!registration) {
504+
throw new Error("the checkpoint handler was never registered on the service")
505+
}
506+
return registration[1]
507+
}
508+
509+
const emitOfLastSave = () => {
510+
const call = mockCheckpointService.saveCheckpoint.mock.calls.at(-1)
511+
if (!call) {
512+
throw new Error("no checkpoint save reached the service")
513+
}
514+
return call[1] as { suppressMessage?: boolean; suppressBroadcast?: boolean }
515+
}
516+
517+
it("threads the suppression flag to the service for the save that asks for it", async () => {
518+
await checkpointSave(mockTask, true, true, undefined, true)
519+
520+
expect(mockCheckpointService.saveCheckpoint).toHaveBeenCalledWith(
521+
expect.stringContaining("Task: test-task-id"),
522+
{ allowEmpty: true, suppressMessage: true, suppressBroadcast: true },
523+
)
524+
})
525+
526+
it("keeps allowEmpty in force so a clean workspace still commits the baseline", async () => {
527+
await expect(checkpointSave(mockTask, true, true, undefined, true)).resolves.toMatchObject({
528+
commit: "test-commit-hash",
529+
})
530+
})
531+
532+
it("suppresses only the current-checkpoint broadcast for the baseline save", async () => {
533+
const handler = await registerCheckpointHandler()
534+
mockCheckpointService.saveCheckpoint.mockResolvedValue({ commit: HASH_2 })
535+
536+
await checkpointSave(mockTask, true, true, undefined, true)
537+
const options = emitOfLastSave()
538+
handler({
539+
type: "checkpoint",
540+
fromHash: HASH_1,
541+
toHash: HASH_2,
542+
duration: 1,
543+
suppressMessage: options.suppressMessage,
544+
suppressBroadcast: options.suppressBroadcast,
545+
})
546+
547+
expect(broadcastPosts()).toHaveLength(0)
548+
expect(mockTask.say).toHaveBeenCalledWith(
549+
"checkpoint_saved",
550+
HASH_2,
551+
undefined,
552+
undefined,
553+
{ from: HASH_1, to: HASH_2, suppressMessage: true },
554+
undefined,
555+
{ isNonInteractive: true },
556+
)
557+
})
558+
559+
it("still broadcasts the current checkpoint after a real user checkpoint", async () => {
560+
// The user-facing shape at Task.ts:2485 - a save whose chat row is suppressed
561+
// but whose checkpoint the user can still roll back to and see changes for.
562+
const handler = await registerCheckpointHandler()
563+
mockCheckpointService.saveCheckpoint.mockResolvedValue({ commit: HASH_2 })
564+
565+
await checkpointSave(mockTask, false, true)
566+
const options = emitOfLastSave()
567+
handler({
568+
type: "checkpoint",
569+
fromHash: HASH_1,
570+
toHash: HASH_2,
571+
duration: 1,
572+
suppressMessage: options.suppressMessage,
573+
suppressBroadcast: options.suppressBroadcast,
574+
})
575+
576+
const broadcasts = broadcastPosts()
577+
expect(broadcasts).toHaveLength(1)
578+
expect(broadcasts[0]).toEqual({
579+
type: "currentCheckpointUpdated",
580+
text: HASH_2,
581+
suppressMessage: true,
582+
})
583+
})
584+
})
585+
476586
describe("getCheckpointService", () => {
477587
it("should return existing service if available", async () => {
478588
const service = await getCheckpointService(mockTask)

‎src/core/checkpoints/index.ts‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,23 @@ async function checkGitInstallation(
180180
task.checkpointServiceInitializing = false
181181
})
182182

183-
service.on("checkpoint", ({ fromHash: from, toHash: to, suppressMessage }) => {
183+
service.on("checkpoint", ({ fromHash: from, toHash: to, suppressMessage, suppressBroadcast }) => {
184184
try {
185185
sendCheckpointInitWarn(task)
186-
// Always update the current checkpoint hash in the webview, including the suppress flag
187-
provider?.postMessageToWebview({
188-
type: "currentCheckpointUpdated",
189-
text: to,
190-
suppressMessage: !!suppressMessage,
191-
})
186+
// The current-checkpoint broadcast is what the completion card renders its
187+
// "See New Changes" affordance from, so it is sent for every checkpoint a user
188+
// could act on. A baseline checkpoint is the workspace state before this task
189+
// wrote anything, so broadcasting it would make every completed task offer to
190+
// show changes that do not exist. Only the broadcast is suppressed: the commit
191+
// is still made and still recorded in history below, so rollback keeps its
192+
// baseline restore point.
193+
if (!suppressBroadcast) {
194+
provider?.postMessageToWebview({
195+
type: "currentCheckpointUpdated",
196+
text: to,
197+
suppressMessage: !!suppressMessage,
198+
})
199+
}
192200

193201
// Always create the chat message but include the suppress flag in the payload
194202
// so the chatview can choose not to render it while keeping it in history.
@@ -252,6 +260,7 @@ export async function checkpointSave(
252260
force = false,
253261
suppressMessage = false,
254262
write?: CheckpointWriteInfo | CheckpointWriteInfo[],
263+
suppressBroadcast = false,
255264
) {
256265
const service = await getCheckpointService(task)
257266

@@ -289,7 +298,11 @@ export async function checkpointSave(
289298

290299
// Start the checkpoint process in the background.
291300
return service
292-
.saveCheckpoint(`Task: ${task.taskId}, Time: ${Date.now()}`, { allowEmpty: force, suppressMessage })
301+
.saveCheckpoint(`Task: ${task.taskId}, Time: ${Date.now()}`, {
302+
allowEmpty: force,
303+
suppressMessage,
304+
suppressBroadcast,
305+
})
293306
.then(async (result) => {
294307
// Only a real commit produces entries (an empty or failed save resolves
295308
// to undefined / rejects), and non-write checkpoint calls (e.g. the

‎src/core/task/Task.ts‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3927,7 +3927,12 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
39273927
try {
39283928
// allowEmpty=true so a clean workspace still produces the baseline
39293929
// commit; awaited so the first per-write checkpoint cannot interleave.
3930-
const baseline = await this.checkpointSave(true, true)
3930+
// suppressBroadcast=true for this save only: the baseline is the workspace
3931+
// state before the task wrote anything, so broadcasting it as the current
3932+
// checkpoint makes every completed task show a "See New Changes" affordance
3933+
// pointing at a commit that holds no user change. The commit itself is still
3934+
// produced and remains the baseline restore point.
3935+
const baseline = await this.checkpointSave(true, true, true)
39313936
// The flag is set only once the baseline actually committed: a save
39323937
// that no-ops behind a failed or timed-out service initialization is
39333938
// retried on a later loop entry instead of being marked done without
@@ -6230,8 +6235,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
62306235

62316236
// Checkpoints
62326237

6233-
public async checkpointSave(force: boolean = false, suppressMessage: boolean = false) {
6234-
return checkpointSave(this, force, suppressMessage)
6238+
public async checkpointSave(force: boolean = false, suppressMessage: boolean = false, suppressBroadcast = false) {
6239+
return checkpointSave(this, force, suppressMessage, undefined, suppressBroadcast)
62356240
}
62366241

62376242
private buildCleanConversationHistory(

‎src/core/task/__tests__/Task.spec.ts‎

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7190,7 +7190,31 @@ describe("Cline", () => {
71907190
await taskAccess.initiateTaskLoop([])
71917191

71927192
expect(saveSpy).toHaveBeenCalledOnce()
7193-
expect(saveSpy).toHaveBeenCalledWith(true, true)
7193+
expect(saveSpy).toHaveBeenCalledWith(true, true, true)
7194+
})
7195+
7196+
it("produces the baseline commit while suppressing only its broadcast", async () => {
7197+
// The suppression is an extra argument to the same save, so the baseline still
7198+
// commits (allowEmpty) and the once-per-instance flag still flips only on that
7199+
// commit: a second loop entry gets no second save because the first committed.
7200+
const task = new Task({
7201+
provider: mockProvider,
7202+
apiConfiguration: mockApiConfig,
7203+
task: "suppressed baseline task",
7204+
startTask: false,
7205+
})
7206+
const taskAccess = getTaskTestAccess(task)
7207+
const saveSpy = vi.spyOn(task, "checkpointSave").mockResolvedValue({ commit: "baseline-commit" })
7208+
const state = await mockProvider.getState()
7209+
vi.spyOn(mockProvider, "getState").mockResolvedValue(state)
7210+
7211+
vi.spyOn(task, "recursivelyMakeClineRequests").mockResolvedValue(true)
7212+
7213+
await taskAccess.initiateTaskLoop([])
7214+
await taskAccess.initiateTaskLoop([])
7215+
7216+
expect(saveSpy).toHaveBeenCalledTimes(1)
7217+
expect(saveSpy).toHaveBeenCalledWith(true, true, true)
71947218
})
71957219

71967220
it("does not record a baseline when the task resumes from history", async () => {
@@ -7242,7 +7266,7 @@ describe("Cline", () => {
72427266
await taskAccess.initiateTaskLoop([])
72437267

72447268
expect(saveSpy).toHaveBeenCalledOnce()
7245-
expect(saveSpy).toHaveBeenCalledWith(true, true)
7269+
expect(saveSpy).toHaveBeenCalledWith(true, true, true)
72467270
})
72477271

72487272
it("does not record a baseline checkpoint when perWriteCheckpoints is disabled", async () => {
@@ -7385,7 +7409,7 @@ describe("Cline", () => {
73857409
await taskAccess.initiateTaskLoop([])
73867410

73877411
expect(saveSpy).toHaveBeenCalledOnce()
7388-
expect(saveSpy).toHaveBeenCalledWith(true, true)
7412+
expect(saveSpy).toHaveBeenCalledWith(true, true, true)
73897413
expect(requestSpy).toHaveBeenCalled()
73907414
})
73917415
})

‎src/services/checkpoints/ShadowCheckpointService.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
330330

331331
public async saveCheckpoint(
332332
message: string,
333-
options?: { allowEmpty?: boolean; suppressMessage?: boolean },
333+
options?: { allowEmpty?: boolean; suppressMessage?: boolean; suppressBroadcast?: boolean },
334334
): Promise<CheckpointResult | undefined> {
335335
try {
336336
this.log(
@@ -357,6 +357,7 @@ export abstract class ShadowCheckpointService extends EventEmitter {
357357
toHash,
358358
duration,
359359
suppressMessage: options?.suppressMessage ?? false,
360+
suppressBroadcast: options?.suppressBroadcast ?? false,
360361
})
361362
}
362363

‎src/services/checkpoints/types.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ export interface CheckpointEventMap {
2929
toHash: string
3030
duration: number
3131
suppressMessage?: boolean
32+
// set only by the task-start baseline save: the checkpoint is recorded, but it must not become the webview current checkpoint
33+
suppressBroadcast?: boolean
3234
}
3335
restore: { type: "restore"; commitHash: string; duration: number }
3436
error: { type: "error"; error: Error }

0 commit comments

Comments
 (0)