From 0ffde0cd0923e37901c55a78c8b5fd1f124ae68b Mon Sep 17 00:00:00 2001 From: gensokyo-dev Date: Sat, 14 Nov 2026 09:48:48 +0000 Subject: [PATCH] fix(desktop): admit a build witness folder by the record's own rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The witness seed accepted its folder with a readdir check that follows symlinks, treats a missing path as usable, and reports a regular-file parent as "not empty". The record is later saved through planOutputTree, so a seed could accept a folder the save then refused. The seed now admits the folder with planOutputTree too, and returns its refusal. Fixes #9291 The MCP boundary test is from #9292 by viniciusdsandrade; its parallel admission helper is not taken, since planOutputTree already owns the rule and export already uses it. Co-Authored-By: Vinícius DS Andrade <95101505+viniciusdsandrade@users.noreply.github.com> Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- .../desktop/DesktopBuildWitness.ts | 26 ++++++-------- tests/boundary/mcp/buildWitnessMcp.test.ts | 35 ++++++++++++++++++- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/application/desktop/DesktopBuildWitness.ts b/src/application/desktop/DesktopBuildWitness.ts index 4390bccc85..9d079ba53a 100644 --- a/src/application/desktop/DesktopBuildWitness.ts +++ b/src/application/desktop/DesktopBuildWitness.ts @@ -1,9 +1,9 @@ import { createHash, randomUUID } from "node:crypto"; -import { readdir } from "node:fs/promises"; import { z } from "zod"; import { AnalysisError } from "../../domain/analysisErrorBase.js"; +import { planOutputTree } from "./DesktopOutputTree.js"; import { AnalysisInputError } from "../../domain/analysisErrorCore.js"; import { parseDesktopTask, @@ -115,14 +115,6 @@ const recordDirectoryRefusal = ( ) : undefined; -/** Whether a folder can take the record: absent, or empty. */ -const usableFolder = async (directory: string): Promise => - readdir(directory).then( - (entries) => entries.length === 0, - (cause: unknown) => - (cause as NodeJS.ErrnoException | undefined)?.code === "ENOENT", - ); - /** * A witness from its seed on: `open` while stages are checked, `saving` * once a stage ended it and until its record is written, which a repeat of @@ -543,13 +535,15 @@ export class DesktopBuildWitness { "Give directory, a new or empty folder on this host for the record and recording.", ), ); - if (!(await usableFolder(directory))) - return err( - refused( - `${directory} exists and is not empty.`, - "Give a new or empty folder.", - ), - ); + // Admit the folder by the same rule the record is saved under, so the + // seed cannot accept a folder (a symlink, a file, a denied path) that the + // save would refuse. + try { + await planOutputTree(directory, { operation: CHECK, field: "directory" }); + } catch (cause: unknown) { + if (cause instanceof AnalysisError) return err(cause); + throw cause; + } let differs: string | undefined; if (this.#bundle !== undefined) { try { diff --git a/tests/boundary/mcp/buildWitnessMcp.test.ts b/tests/boundary/mcp/buildWitnessMcp.test.ts index aec41e35bb..1f983a0edd 100644 --- a/tests/boundary/mcp/buildWitnessMcp.test.ts +++ b/tests/boundary/mcp/buildWitnessMcp.test.ts @@ -1,4 +1,4 @@ -import { mkdir, readFile, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, rm, symlink, writeFile } from "node:fs/promises"; import { join } from "node:path"; import type { CallToolResult } from "@modelcontextprotocol/server"; @@ -636,6 +636,39 @@ it("refuses a witness without a folder for its record, or one already holding fi ).toContain("not empty"); }); +it("refuses an unusable witness folder before recording", async () => { + const { check, computer, directory } = await witnessed(); + const root = await createTestTempDirectory("witness-unusable-"); + const regularFile = join(root, "file"); + await writeFile(regularFile, "occupied"); + const candidates = [regularFile, join(regularFile, "out")]; + if (process.platform !== "win32") { + const emptyFolder = join(root, "empty"); + await mkdir(emptyFolder); + const emptyLink = join(root, "empty-link"); + await symlink(emptyFolder, emptyLink); + const danglingLink = join(root, "dangling-link"); + await symlink(join(root, "absent"), danglingLink); + candidates.push(emptyLink, danglingLink); + } + for (const candidate of candidates) { + const response = await check({ stage: "seed", directory: candidate }); + const { error } = parseMcpToolError(response); + expect(error.code).toBe("invalid_request"); + expect(error.message).not.toContain("not empty"); + expect(error.details?.issues).toMatchObject([ + { path: ["directory"], reason: "invalid_value" }, + ]); + } + expect(computer.recordingRequests).toEqual([]); + const seed = parsed(await check({ stage: "seed", directory })); + expect(seed.witness).toMatchObject({ + stage: "seed", + held: true, + next_stage: "after_edit", + }); +}); + it("retries a seed whose state read failed, with the recording it began", async () => { const witness = await witnessed(); witness.app.failGets = 1;