Skip to content

Commit 0870165

Browse files
committed
feat(checkpoints): per-step change cards and changeCardDetail setting (B3a, #1375)
1 parent 410591e commit 0870165

37 files changed

Lines changed: 905 additions & 57 deletions

packages/types/src/global-settings.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
} from "./provider-settings.js"
1313
import { telemetrySettingsSchema } from "./telemetry.js"
1414
import { toolNamesSchema } from "./tool.js"
15+
import { changeCardDetailSchema, type ChangeCardDetail } from "./message.js"
1516
import { type Keys } from "./type-fu.js"
1617
import { languagesSchema } from "./vscode.js"
1718

@@ -106,6 +107,14 @@ export const DEFAULT_CHECKPOINT_TIMEOUT_SECONDS = 15
106107
*/
107108
export const DEFAULT_PER_WRITE_CHECKPOINTS = true
108109

110+
/**
111+
* Default detail level for per-step change cards (B3a).
112+
* "summary" keeps cards compact (file list with +/− counts; the UI fetches
113+
* diffs lazily); "full" carries the unified diff inline per file.
114+
* @default "summary"
115+
*/
116+
export const DEFAULT_CHANGE_CARD_DETAIL: ChangeCardDetail = "summary"
117+
109118
/**
110119
* GlobalSettings
111120
*/
@@ -213,6 +222,13 @@ export const globalSettingsSchema = z.object({
213222
* @default true
214223
*/
215224
perWriteCheckpoints: z.boolean().optional(),
225+
/**
226+
* Detail level for per-step change cards: "full" includes the unified diff
227+
* inline for every changed file, "summary" carries only the file list with
228+
* +/− counts (diffs are fetched lazily by the UI).
229+
* @default "summary"
230+
*/
231+
changeCardDetail: changeCardDetailSchema.optional(),
216232

217233
ttsEnabled: z.boolean().optional(),
218234
ttsSpeed: z.number().optional(),

packages/types/src/message.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ export function isNonBlockingAsk(ask: ClineAsk): ask is NonBlockingAsk {
134134
* - `mcp_server_response`: Response received from MCP server
135135
* - `subtask_result`: Result of a completed subtask
136136
* - `checkpoint_saved`: Indicates a checkpoint has been saved
137+
* - `change_card`: Per-step change card summarizing the files a completed tool step wrote (B3a)
137138
* - `rooignore_error`: Error related to .rooignore file processing
138139
* - `diff_error`: Error occurred while applying a diff/patch
139140
* - `condense_context`: Context condensation/summarization has started
@@ -162,6 +163,7 @@ export const clineSays = [
162163
"mcp_server_response",
163164
"subtask_result",
164165
"checkpoint_saved",
166+
"change_card",
165167
"rooignore_error",
166168
"diff_error",
167169
"condense_context",
@@ -235,6 +237,49 @@ export const contextTruncationSchema = z.object({
235237

236238
export type ContextTruncation = z.infer<typeof contextTruncationSchema>
237239

240+
/**
241+
* ChangeCard
242+
*
243+
* Payload of the per-step change card (B3a). The extension host emits one
244+
* `say: "change_card"` message per completed tool write step, keyed by the
245+
* shadow-git checkpoint the step produced. The JSON payload (see
246+
* {@link ChangeCardData}) is carried in the message `text` field, the same
247+
* way tool approval messages carry their serialized ClineSayTool.
248+
*
249+
* `detail: "full"` carries the unified diff inline for every file so the UI
250+
* can render it directly; `detail: "summary"` carries only the file list with
251+
* +/− counts and the UI fetches diffs lazily (B3b). Auto-approved steps are
252+
* always emitted with `detail: "summary"` regardless of the user setting.
253+
*/
254+
export const changeCardDetailSchema = z.enum(["full", "summary"])
255+
256+
export type ChangeCardDetail = z.infer<typeof changeCardDetailSchema>
257+
258+
export const changeCardFileSchema = z.object({
259+
path: z.string(),
260+
additions: z.number(),
261+
deletions: z.number(),
262+
/**
263+
* Unified diff for this file. Only present when the card was emitted with
264+
* `detail: "full"`; summary cards leave it out to stay compact.
265+
*/
266+
diff: z.string().optional(),
267+
})
268+
269+
export type ChangeCardFile = z.infer<typeof changeCardFileSchema>
270+
271+
export const changeCardSchema = z.object({
272+
/** Opaque step identifier, reserved for future tool-step tracking. */
273+
stepId: z.string().optional(),
274+
/** Checkpoint commit SHAs produced by the step (one per per-write checkpoint). */
275+
checkpointIds: z.array(z.string()),
276+
files: z.array(changeCardFileSchema),
277+
totalFiles: z.number(),
278+
detail: changeCardDetailSchema,
279+
})
280+
281+
export type ChangeCardData = z.infer<typeof changeCardSchema>
282+
238283
/**
239284
* ClineMessage
240285
*

packages/types/src/vscode-extension-host.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import type { ProviderSettings, ProviderSettingsEntry } from "./provider-setting
55
import type { HistoryItem } from "./history.js"
66
import type { ModeConfig, PromptComponent } from "./mode.js"
77
import type { Experiments } from "./experiment.js"
8-
import type { ClineMessage, QueuedMessage } from "./message.js"
8+
import type { ChangeCardDetail, ClineMessage, QueuedMessage } from "./message.js"
99
import type { MarketplaceItem, MarketplaceInstalledMetadata, InstallMarketplaceItemOptions } from "./marketplace.js"
1010
import type { TodoItem } from "./todo.js"
1111
import type { CloudUserInfo, CloudOrganizationMembership, OrganizationAllowList, ShareVisibility } from "./cloud.js"
@@ -349,6 +349,7 @@ export type ExtensionState = Pick<
349349
enableCheckpoints: boolean
350350
checkpointTimeout: number // Timeout for checkpoint initialization in seconds (default: 15)
351351
perWriteCheckpoints: boolean
352+
changeCardDetail: ChangeCardDetail
352353
maxOpenTabsContext: number // Maximum number of VSCode open tabs to include in context (0-500)
353354
maxWorkspaceFiles: number // Maximum number of files to include in current working directory details (0-500)
354355
showRooIgnoredFiles: boolean // Whether to show .rooignore'd files in listings
Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
import { describe, expect, it } from "vitest"
2+
3+
import type { ChangeCardData } from "@roo-code/types"
4+
5+
import {
6+
buildChangeCard,
7+
buildChangeCardPayload,
8+
isAutoApprovedStep,
9+
resolveChangeCardDetail,
10+
type ChangeCardWrite,
11+
} from "../changeCard"
12+
13+
describe("changeCard (B3a)", () => {
14+
function write(overrides: Partial<ChangeCardWrite> = {}): ChangeCardWrite {
15+
return {
16+
path: "src/a.ts",
17+
diffStats: { additions: 2, deletions: 1 },
18+
diff: "--- a/src/a.ts\n+++ b/src/a.ts\n@@ -1 +1,2 @@\n-old\n+new-1\n+new-2",
19+
...overrides,
20+
}
21+
}
22+
23+
describe("isAutoApprovedStep", () => {
24+
it("returns false for an empty step", () => {
25+
expect(isAutoApprovedStep([])).toBe(false)
26+
})
27+
28+
it("returns true only when every write was auto-approved", () => {
29+
expect(isAutoApprovedStep([write({ autoApproved: true }), write({ autoApproved: true })])).toBe(true)
30+
expect(isAutoApprovedStep([write({ autoApproved: true }), write()])).toBe(false)
31+
expect(isAutoApprovedStep([write()])).toBe(false)
32+
})
33+
})
34+
35+
describe("resolveChangeCardDetail", () => {
36+
it("forces summary for auto-approved steps even when the setting is full", () => {
37+
const writes = [write({ autoApproved: true })]
38+
expect(resolveChangeCardDetail(writes, "full")).toBe("summary")
39+
expect(resolveChangeCardDetail(writes, undefined)).toBe("summary")
40+
})
41+
42+
it("follows the setting for interactive steps, defaulting to summary when unset", () => {
43+
const writes = [write()]
44+
expect(resolveChangeCardDetail(writes, "full")).toBe("full")
45+
expect(resolveChangeCardDetail(writes, "summary")).toBe("summary")
46+
expect(resolveChangeCardDetail(writes, undefined)).toBe("summary")
47+
})
48+
})
49+
50+
describe("buildChangeCard", () => {
51+
it("carries the inline diff per file for full detail on a multi-file step", () => {
52+
const card = buildChangeCard(
53+
"sha-1",
54+
[write(), write({ path: "src/b.ts", diffStats: { additions: 1, deletions: 0 }, diff: "+b" })],
55+
"full",
56+
)
57+
58+
expect(card).toEqual({
59+
checkpointIds: ["sha-1"],
60+
files: [
61+
{
62+
path: "src/a.ts",
63+
additions: 2,
64+
deletions: 1,
65+
diff: "--- a/src/a.ts\n+++ b/src/a.ts\n@@ -1 +1,2 @@\n-old\n+new-1\n+new-2",
66+
},
67+
{ path: "src/b.ts", additions: 1, deletions: 0, diff: "+b" },
68+
],
69+
totalFiles: 2,
70+
detail: "full",
71+
})
72+
})
73+
74+
it("omits the diff per file for summary detail (lazy fetch is B3b)", () => {
75+
const card = buildChangeCard("sha-1", [write()], "summary")
76+
77+
expect(card.files).toEqual([{ path: "src/a.ts", additions: 2, deletions: 1 }])
78+
expect(card.files[0]).not.toHaveProperty("diff")
79+
expect(card.detail).toBe("summary")
80+
expect(card.totalFiles).toBe(1)
81+
})
82+
83+
it("defaults missing diffStats to zero counts and keeps full detail without diff for a write without one", () => {
84+
const card = buildChangeCard("sha-1", [write({ diffStats: undefined, diff: undefined })], "full")
85+
86+
expect(card.files[0]).toEqual({ path: "src/a.ts", additions: 0, deletions: 0 })
87+
})
88+
})
89+
90+
describe("buildChangeCardPayload", () => {
91+
it("resolves the detail level and builds the payload in one call", () => {
92+
// The expectations are typed against the shared ChangeCardData
93+
// contract in @roo-code/types, so the builder's output is checked
94+
// against the same single source of truth the webview consumes.
95+
// Interactive step with the full setting: diff inline.
96+
const full: ChangeCardData = buildChangeCardPayload("sha-1", [write()], "full")
97+
expect(full.detail).toBe("full")
98+
expect(full.files[0].diff).toBe("--- a/src/a.ts\n+++ b/src/a.ts\n@@ -1 +1,2 @@\n-old\n+new-1\n+new-2")
99+
100+
// Auto-approved step with the full setting: compact summary, no diff.
101+
const compact: ChangeCardData = buildChangeCardPayload("sha-1", [write({ autoApproved: true })], "full")
102+
expect(compact.detail).toBe("summary")
103+
expect(compact.files[0]).not.toHaveProperty("diff")
104+
expect(compact.checkpointIds).toEqual(["sha-1"])
105+
expect(compact.totalFiles).toBe(1)
106+
})
107+
})
108+
})

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ interface ProviderLike {
6666
context: { globalStorageUri: { fsPath: string } }
6767
log: (...args: unknown[]) => void
6868
postMessageToWebview: (...args: unknown[]) => void
69+
getState: () => Promise<Record<string, unknown>>
6970
}
7071

7172
interface TaskLike {
@@ -74,14 +75,19 @@ interface TaskLike {
7475
checkpointService: ServiceLike
7576
checkpointServiceInitializing: boolean
7677
providerRef: { deref: () => ProviderLike | undefined }
78+
say: (...args: unknown[]) => Promise<void>
7779
}
7880

7981
describe("checkpointSave change-journal wiring (B2)", () => {
8082
let tmpStorageDir: string
8183
let saveCheckpointSpy: Mock
8284
let mockProvider: ProviderLike
8385
let mockTask: TaskLike
84-
const write: CheckpointWriteInfo = { path: "src/foo.ts", operation: "create", diffStats: { additions: 3, deletions: 0 } }
86+
const write: CheckpointWriteInfo = {
87+
path: "src/foo.ts",
88+
operation: "create",
89+
diffStats: { additions: 3, deletions: 0 },
90+
}
8591

8692
beforeEach(async () => {
8793
tmpStorageDir = await fs.mkdtemp(path.join(os.tmpdir(), "b2-journal-wiring-"))
@@ -90,6 +96,8 @@ describe("checkpointSave change-journal wiring (B2)", () => {
9096
context: { globalStorageUri: { fsPath: tmpStorageDir } },
9197
log: vi.fn(),
9298
postMessageToWebview: vi.fn(),
99+
// B3a: the card emission reads the live settings through getState.
100+
getState: vi.fn().mockResolvedValue({}),
93101
}
94102
// Structural test double for Task (the class is not instantiated at
95103
// this unit layer); the cast is safe because the fields checkpointSave
@@ -100,6 +108,10 @@ describe("checkpointSave change-journal wiring (B2)", () => {
100108
checkpointService: { isInitialized: true, saveCheckpoint: saveCheckpointSpy },
101109
checkpointServiceInitializing: false,
102110
providerRef: { deref: () => mockProvider },
111+
// B3a: the card emission calls task.say; a resolved double keeps the
112+
// test double complete instead of letting the emission take the
113+
// error path.
114+
say: vi.fn().mockResolvedValue(undefined),
103115
}
104116
})
105117

0 commit comments

Comments
 (0)