Skip to content

Commit 3339570

Browse files
easonLiangWorldedtecheasonLiangWorldedtech
committed
test(core): assert the edit type open() actually saw
The two rows in file existence detection asserted only the value handle() ended on. That is satisfied by an assignment placed below the open call, and the order is what the file's content depends on: open() reads this.editType to decide whether the target exists and empties the file for anything that is not "modify" (DiffViewProvider.ts:127, 154-158, 167-168). The provider mock now records the field at the call and both rows assert the recorded value. Control: moving WriteToFileTool.ts:437 below the open at 517 fails exactly those two rows - expected [ undefined ] to deeply equal [ modify ] and [ create ] - while the pre-existing final-value assertions still pass, which is the gap the review named. 78 passed, 5 skipped with the production head untouched.
1 parent d11e250 commit 3339570

1 file changed

Lines changed: 14 additions & 1 deletion

File tree

‎src/core/tools/__tests__/writeToFileTool.spec.ts‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,11 @@ describe("writeToFileTool", () => {
112112
let mockHandleError: ReturnType<typeof vi.fn<HandleError>>
113113
let mockPushToolResult: ReturnType<typeof vi.fn<PushToolResult>>
114114
let toolResult: ToolResponse | undefined
115+
// The edit type each open() call actually saw. open() reads this.editType to decide
116+
// whether the target exists and empties the file for anything that is not "modify"
117+
// (DiffViewProvider.ts:127, 154-158, 167-168), so the value at the call, not the value
118+
// after handle() returns, is what the user's content depends on.
119+
const editTypeAtOpen: Array<"create" | "modify" | undefined> = []
115120

116121
beforeEach(() => {
117122
vi.clearAllMocks()
@@ -158,6 +163,7 @@ describe("writeToFileTool", () => {
158163
mockCline.rooIgnoreController = {
159164
validateAccess: vi.fn().mockReturnValue(true),
160165
}
166+
editTypeAtOpen.length = 0
161167
mockCline.diffViewProvider = {
162168
editType: undefined,
163169
isEditing: false,
@@ -166,7 +172,10 @@ describe("writeToFileTool", () => {
166172
// where the contract says it must be set.
167173
approvedSaveCommitted: false,
168174
originalContent: "",
169-
open: vi.fn().mockResolvedValue(undefined),
175+
open: vi.fn(() => {
176+
editTypeAtOpen.push(mockCline.diffViewProvider.editType)
177+
return Promise.resolve(undefined)
178+
}),
170179
update: vi.fn().mockResolvedValue(undefined),
171180
reset: vi.fn().mockResolvedValue(undefined),
172181
revertChanges: vi.fn().mockResolvedValue(undefined),
@@ -305,12 +314,16 @@ describe("writeToFileTool", () => {
305314
await executeWriteFileTool({}, { fileExists: true })
306315

307316
expect(mockCline.diffViewProvider.editType).toBe("modify")
317+
// The value open() saw, not just the value the run ended on: an assignment moved
318+
// below the open would leave the final state correct and the file emptied.
319+
expect(editTypeAtOpen).toEqual(["modify"])
308320
})
309321

310322
it("installs create as the edit type for a target that does not exist yet", async () => {
311323
await executeWriteFileTool({}, { fileExists: false })
312324

313325
expect(mockCline.diffViewProvider.editType).toBe("create")
326+
expect(editTypeAtOpen).toEqual(["create"])
314327
})
315328

316329
it("uses cached editType without filesystem check", async () => {

0 commit comments

Comments
 (0)