Skip to content

Commit aa6bc19

Browse files
committed
feat(checkpoints): per-task change journal with torn-tail repair (B2, #1375)
1 parent b674c42 commit aa6bc19

10 files changed

Lines changed: 931 additions & 56 deletions
Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
import fs from "fs/promises"
2+
import os from "os"
3+
import path from "path"
4+
5+
import { afterEach, beforeEach, describe, expect, it } from "vitest"
6+
7+
import { appendChange, journalPath, loadChanges, type ChangeJournalEntry } from "../changeJournal"
8+
9+
describe("changeJournal", () => {
10+
const taskId = "test-task"
11+
12+
let tmpRoot: string
13+
14+
beforeEach(async () => {
15+
tmpRoot = await fs.mkdtemp(path.join(os.tmpdir(), "b2-journal-"))
16+
})
17+
18+
afterEach(async () => {
19+
await fs.rm(tmpRoot, { recursive: true, force: true })
20+
})
21+
22+
function entry(overrides: Partial<ChangeJournalEntry> = {}): ChangeJournalEntry {
23+
return {
24+
path: "src/foo.ts",
25+
operation: "create",
26+
checkpointId: "abc12345",
27+
...overrides,
28+
}
29+
}
30+
31+
describe("appendChange", () => {
32+
it("writes one JSON line per entry with the documented field shape", async () => {
33+
await appendChange(tmpRoot, taskId, entry({ operation: "create", checkpointId: "aaa" }))
34+
35+
const raw = await fs.readFile(journalPath(tmpRoot, taskId), "utf8")
36+
const lines = raw.split("\n").filter((line) => line !== "")
37+
expect(lines).toHaveLength(1)
38+
const parsed = JSON.parse(lines[0]) as ChangeJournalEntry
39+
expect(parsed.path).toBe("src/foo.ts")
40+
expect(parsed.operation).toBe("create")
41+
expect(parsed.checkpointId).toBe("aaa")
42+
})
43+
44+
it("appends multiple entries sequentially", async () => {
45+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "a" }))
46+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "b" }))
47+
48+
const raw = await fs.readFile(journalPath(tmpRoot, taskId), "utf8")
49+
expect(raw.split("\n").filter((line) => line !== "")).toHaveLength(2)
50+
})
51+
})
52+
53+
describe("loadChanges", () => {
54+
it("returns [] for an absent journal file", async () => {
55+
expect(await loadChanges(tmpRoot, taskId)).toEqual([])
56+
})
57+
58+
it("returns [] for an empty journal file", async () => {
59+
await fs.mkdir(path.dirname(journalPath(tmpRoot, taskId)), { recursive: true })
60+
await fs.writeFile(journalPath(tmpRoot, taskId), "")
61+
62+
expect(await loadChanges(tmpRoot, taskId)).toEqual([])
63+
})
64+
65+
it("parses all entries in order with a clean tail", async () => {
66+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "x" }))
67+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "y" }))
68+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "z" }))
69+
70+
const result = await loadChanges(tmpRoot, taskId)
71+
expect(result).toHaveLength(3)
72+
expect(result[0].checkpointId).toBe("x")
73+
expect(result[1].checkpointId).toBe("y")
74+
expect(result[2].checkpointId).toBe("z")
75+
})
76+
77+
it("parses a journal whose final line has no trailing newline", async () => {
78+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "ok" }))
79+
80+
// Rewrite the file without the trailing newline of the last line.
81+
const filePath = journalPath(tmpRoot, taskId)
82+
const content = (await fs.readFile(filePath, "utf8")).replace(/\n$/, "")
83+
await fs.writeFile(filePath, content)
84+
85+
const result = await loadChanges(tmpRoot, taskId)
86+
expect(result).toHaveLength(1)
87+
expect(result[0].checkpointId).toBe("ok")
88+
})
89+
90+
it("discards a torn final line and returns the complete entries", async () => {
91+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "ok" }))
92+
93+
// Append a second line truncated mid-content, with no trailing newline.
94+
await fs.appendFile(journalPath(tmpRoot, taskId), '{"path":"src/half.ts","operation":"upd')
95+
96+
const result = await loadChanges(tmpRoot, taskId)
97+
expect(result).toHaveLength(1)
98+
expect(result[0].checkpointId).toBe("ok")
99+
})
100+
101+
it("skips a corrupt middle line and still loads the later valid entries", async () => {
102+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "ok" }))
103+
104+
// Corrupt the first line in place, then append a valid entry after it.
105+
await fs.writeFile(
106+
journalPath(tmpRoot, taskId),
107+
'"{"path":"src/corrupt.ts","operation":"update"\n' + JSON.stringify(entry({ checkpointId: "after" })) + "\n",
108+
)
109+
110+
const result = await loadChanges(tmpRoot, taskId)
111+
expect(result).toHaveLength(1)
112+
expect(result[0].checkpointId).toBe("after")
113+
})
114+
115+
it("does not throw when the entire journal is torn", async () => {
116+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "first" }))
117+
118+
// Truncate to a single character — definitely invalid JSON.
119+
await fs.writeFile(journalPath(tmpRoot, taskId), "{")
120+
121+
expect(await loadChanges(tmpRoot, taskId)).toEqual([])
122+
})
123+
124+
it("includes diffStats when present", async () => {
125+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "s", diffStats: { additions: 5, deletions: 2 } }))
126+
127+
const result = await loadChanges(tmpRoot, taskId)
128+
expect(result[0].diffStats).toEqual({ additions: 5, deletions: 2 })
129+
})
130+
131+
it("omits diffStats when not provided", async () => {
132+
await appendChange(tmpRoot, taskId, entry({ checkpointId: "n" }))
133+
134+
const result = await loadChanges(tmpRoot, taskId)
135+
expect(result[0].diffStats).toBeUndefined()
136+
})
137+
})
138+
})
Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,206 @@
1+
import fs from "fs/promises"
2+
import os from "os"
3+
import path from "path"
4+
5+
import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from "vitest"
6+
7+
import type { Task } from "../../task/Task"
8+
import { loadChanges } from "../changeJournal"
9+
import { checkpointSave, type CheckpointWriteInfo } from "../index"
10+
11+
// Mock the VS Code API surface (index.ts imports vscode at module level).
12+
vi.mock("vscode", () => ({
13+
window: {
14+
showErrorMessage: vi.fn(),
15+
showInformationMessage: vi.fn(),
16+
createTextEditorDecorationType: vi.fn(() => ({})),
17+
},
18+
Uri: {
19+
file: vi.fn((p: string) => ({ fsPath: p })),
20+
parse: vi.fn((uri: string) => ({ with: vi.fn(() => ({})) })),
21+
},
22+
commands: {
23+
executeCommand: vi.fn(),
24+
},
25+
}))
26+
27+
vi.mock("@roo-code/telemetry", () => ({
28+
TelemetryService: {
29+
instance: {
30+
captureCheckpointCreated: vi.fn(),
31+
captureCheckpointRestored: vi.fn(),
32+
captureCheckpointDiffed: vi.fn(),
33+
},
34+
},
35+
}))
36+
37+
vi.mock("../../../utils/path", () => ({
38+
getWorkspacePath: vi.fn(() => "/test/workspace"),
39+
}))
40+
41+
vi.mock("../../../utils/git", () => ({
42+
checkGitInstalled: vi.fn().mockResolvedValue(true),
43+
}))
44+
45+
vi.mock("../../../i18n", () => ({
46+
t: vi.fn((key: string) => key),
47+
}))
48+
49+
vi.mock("p-wait-for", () => ({
50+
default: vi.fn(),
51+
}))
52+
53+
// The real service would require a git repo; the checkpointSave under test
54+
// only needs the pre-initialized service on the task.
55+
vi.mock("../../../services/checkpoints")
56+
57+
const TASK_ID = "journal-test-task"
58+
const COMMIT = "test-commit-hash"
59+
60+
interface ServiceLike {
61+
isInitialized: boolean
62+
saveCheckpoint: (...args: unknown[]) => Promise<unknown>
63+
}
64+
65+
interface ProviderLike {
66+
context: { globalStorageUri: { fsPath: string } }
67+
log: (...args: unknown[]) => void
68+
postMessageToWebview: (...args: unknown[]) => void
69+
}
70+
71+
interface TaskLike {
72+
taskId: string
73+
enableCheckpoints: boolean
74+
checkpointService: ServiceLike
75+
checkpointServiceInitializing: boolean
76+
providerRef: { deref: () => ProviderLike | undefined }
77+
}
78+
79+
describe("checkpointSave change-journal wiring (B2)", () => {
80+
let tmpStorageDir: string
81+
let saveCheckpointSpy: Mock
82+
let mockProvider: ProviderLike
83+
let mockTask: TaskLike
84+
const write: CheckpointWriteInfo = { path: "src/foo.ts", operation: "create", diffStats: { additions: 3, deletions: 0 } }
85+
86+
beforeEach(async () => {
87+
tmpStorageDir = await fs.mkdtemp(path.join(os.tmpdir(), "b2-journal-wiring-"))
88+
saveCheckpointSpy = vi.fn().mockResolvedValue({ commit: COMMIT })
89+
mockProvider = {
90+
context: { globalStorageUri: { fsPath: tmpStorageDir } },
91+
log: vi.fn(),
92+
postMessageToWebview: vi.fn(),
93+
}
94+
// Structural test double for Task (the class is not instantiated at
95+
// this unit layer); the cast is safe because the fields checkpointSave
96+
// reads are exactly these.
97+
mockTask = {
98+
taskId: TASK_ID,
99+
enableCheckpoints: true,
100+
checkpointService: { isInitialized: true, saveCheckpoint: saveCheckpointSpy },
101+
checkpointServiceInitializing: false,
102+
providerRef: { deref: () => mockProvider },
103+
}
104+
})
105+
106+
afterEach(async () => {
107+
vi.restoreAllMocks()
108+
await fs.rm(tmpStorageDir, { recursive: true, force: true })
109+
})
110+
111+
it("appends exactly one journal line referencing the B1 checkpoint id for a per-write save", async () => {
112+
await checkpointSave(mockTask as Task, false, true, write)
113+
114+
const entries = await loadChanges(tmpStorageDir, TASK_ID)
115+
expect(entries).toHaveLength(1)
116+
expect(entries[0]).toEqual({
117+
path: "src/foo.ts",
118+
operation: "create",
119+
checkpointId: COMMIT,
120+
diffStats: { additions: 3, deletions: 0 },
121+
})
122+
123+
// The raw file holds exactly one JSON line.
124+
const journalFile = path.join(tmpStorageDir, "tasks", TASK_ID, "checkpoints", "changes.jsonl")
125+
const raw = await fs.readFile(journalFile, "utf8")
126+
expect(raw.split("\n").filter((line) => line !== "")).toHaveLength(1)
127+
})
128+
129+
it("omits diffStats in the journal entry when not provided", async () => {
130+
await checkpointSave(mockTask as Task, false, true, { path: "src/bar.ts", operation: "update" })
131+
132+
const entries = await loadChanges(tmpStorageDir, TASK_ID)
133+
expect(entries).toHaveLength(1)
134+
expect(entries[0].path).toBe("src/bar.ts")
135+
expect(entries[0].operation).toBe("update")
136+
expect(entries[0].checkpointId).toBe(COMMIT)
137+
expect(entries[0].diffStats).toBeUndefined()
138+
})
139+
140+
it("does not write a journal entry for non-write checkpoint saves (task-start baseline)", async () => {
141+
await checkpointSave(mockTask as Task)
142+
143+
const journalFile = path.join(tmpStorageDir, "tasks", TASK_ID, "checkpoints", "changes.jsonl")
144+
await expect(fs.stat(journalFile)).rejects.toThrow()
145+
expect(await loadChanges(tmpStorageDir, TASK_ID)).toEqual([])
146+
})
147+
148+
it("appends one entry per file change for a multi-file write (apply-patch shape)", async () => {
149+
await checkpointSave(mockTask as Task, false, true, [
150+
{ path: "src/a.ts", operation: "create" },
151+
{ path: "src/b.ts", operation: "update" },
152+
{ path: "src/c.ts", operation: "delete" },
153+
])
154+
155+
const entries = await loadChanges(tmpStorageDir, TASK_ID)
156+
expect(entries).toHaveLength(3)
157+
// Every entry references the single checkpoint of the whole patch.
158+
expect(entries.map((entry) => entry.checkpointId)).toEqual([COMMIT, COMMIT, COMMIT])
159+
expect(entries.map((entry) => entry.path)).toEqual(["src/a.ts", "src/b.ts", "src/c.ts"])
160+
expect(entries.map((entry) => entry.operation)).toEqual(["create", "update", "delete"])
161+
})
162+
163+
it("keeps the existing error-swallowing behavior and skips the journal on save failure", async () => {
164+
saveCheckpointSpy.mockRejectedValueOnce(new Error("git exploded"))
165+
166+
await expect(checkpointSave(mockTask as Task, false, true, write)).resolves.toBeUndefined()
167+
expect(mockTask.enableCheckpoints).toBe(false)
168+
expect(await loadChanges(tmpStorageDir, TASK_ID)).toEqual([])
169+
})
170+
171+
it("does not write a journal entry when the checkpoint save is a no-op (empty commit)", async () => {
172+
saveCheckpointSpy.mockResolvedValueOnce(undefined)
173+
174+
await checkpointSave(mockTask as Task, false, true, write)
175+
176+
expect(await loadChanges(tmpStorageDir, TASK_ID)).toEqual([])
177+
expect(mockTask.enableCheckpoints).toBe(true)
178+
})
179+
180+
it("does not crash when the provider has no globalStorageDir", async () => {
181+
mockTask.providerRef = { deref: () => undefined }
182+
183+
await expect(checkpointSave(mockTask as Task, false, true, write)).resolves.toMatchObject({ commit: COMMIT })
184+
expect(mockTask.enableCheckpoints).toBe(true)
185+
expect(await loadChanges(tmpStorageDir, TASK_ID)).toEqual([])
186+
})
187+
188+
it("logs and continues when the journal cannot be written (checkpoints stay enabled)", async () => {
189+
// Block the per-task checkpoint dir so the journal mkdir/append fails.
190+
const taskDir = path.join(tmpStorageDir, "tasks", TASK_ID)
191+
await fs.mkdir(taskDir, { recursive: true })
192+
await fs.writeFile(path.join(taskDir, "checkpoints"), "blocker")
193+
194+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => undefined)
195+
196+
// The journal failure is swallowed (logged, not rethrown), so the
197+
// checkpoint result still resolves exactly as without journaling.
198+
await expect(checkpointSave(mockTask as Task, false, true, write)).resolves.toMatchObject({ commit: COMMIT })
199+
200+
expect(mockTask.enableCheckpoints).toBe(true)
201+
expect(consoleErrorSpy).toHaveBeenCalledWith(
202+
expect.stringContaining("failed to append change journal entry"),
203+
expect.anything(),
204+
)
205+
})
206+
})

0 commit comments

Comments
 (0)