From 661370201e10e4636f023aa4cfa6b0a7f1f68733 Mon Sep 17 00:00:00 2001 From: Eason Liang <136036952+easonLiangWorldedtech@users.noreply.github.com> Date: Mon, 9 Nov 2026 13:35:59 +0800 Subject: [PATCH] [Fix] teach #2343's apply_diff outside-workspace doubles about the hardened guarded write applyDiffTool.outsideWorkspace.spec.ts arrived with 1b9cb7d35 (#2343). Its doubles describe the tool as it behaved before the guarded-write series hardened apply_diff: an fs/promises double with readFile only, and a diffViewProvider.open() stubbed as a no-op over originalContent: "". The hardened tool stats around its own read to capture the version token its guarded publish consumes, and fails closed when open()'s re-read disagrees with that read. So on any branch that carries the hardened tool, execute() never reaches askApproval: 9 of the 11 rows fail ('expect(askApproval).toHaveBeenCalledTimes(1)' got 0 calls at spec:135, ':197', ':219') even though nothing about the outside-workspace decision changed. This touches the test file only; no production line changes. - fs double gets a stat (:26-31): rejecting, which is the case the tool already tolerates by publishing unguarded-by-token (the token is kept only when the file did not change between stat and read). - open() double records its re-read (:111-117): the real open() re-reads the file and keeps that text as the save baseline, which is exactly what the staleness check compares against. Measured (Windows box, pinned node 22.23.4, root-junction bins): - this tip: outsideWorkspace 11 passed (11), exit 0; tsc --noEmit 0 errors; eslint --max-warnings=0 0 messages on the file; prettier --check clean; eslint-suppressions.json untouched (delta empty). - without the hunks, in the 1408-head + main merge tree (tree 73632a8b3e57): 9 failed | 2 passed (11), the same 9 rows CI's platform-unit-test (windows-latest) reported in run 42565686409 job 128991579779. - with the stat hunk only: 7 failed | 4 passed. With both hunks: 11 passed (11), and that merged tree passes its whole core lane: 201 files, 4363 passed | 9 skipped (4372), exit 0. - negative control: main tip e487cd7b2a alone, that spec 11 passed (11) - the file is fine today; it is only unreadable against the hardened tool. #1408 (feat/guarded-write-wiring-s4b) is the pull request waiting on this: its windows arm is red against a file that is not on its branch, and its owner deliberately did not carry this file across, because both sides adding it with different content is an add/add conflict in the merge tree. --- .../applyDiffTool.outsideWorkspace.spec.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts b/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts index cbaa2433ca..bf27e65ce6 100644 --- a/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts +++ b/src/core/tools/__tests__/applyDiffTool.outsideWorkspace.spec.ts @@ -20,6 +20,12 @@ vi.mock("@roo-code/telemetry", () => ({ vi.mock("fs/promises", () => ({ default: { readFile: vi.fn(), + // ApplyDiffTool stats around its own read to capture the version token its guarded publish + // consumes, so the fs double has to carry a stat. Rejecting keeps the fixture honest about + // the case the code already tolerates: no stat, no token, publish unguarded-by-token. + stat: vi + .fn() + .mockRejectedValue(Object.assign(new Error("ENOENT: no such file or directory"), { code: "ENOENT" })), }, })) @@ -102,7 +108,12 @@ describe("ApplyDiffTool isOutsideWorkspace", () => { diffViewProvider: { editType: undefined, originalContent: "", - open: vi.fn().mockResolvedValue(undefined), + open: vi.fn().mockImplementation(async function () { + // The real open() re-reads the file and records that text as the save baseline; ApplyDiffTool + // compares that baseline against its own read and fails closed when they differ, so the + // double has to emulate the re-read instead of a no-op. + task.diffViewProvider.originalContent = "old\n" + }), update: vi.fn().mockResolvedValue(undefined), reset: vi.fn().mockResolvedValue(undefined), revertChanges: vi.fn().mockResolvedValue(undefined),