From 681874cd6f481451810707bb5debdaf928628740 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 4 Oct 2026 22:23:19 -0700 Subject: [PATCH 01/25] test: lock MCP status words and JSON-RPC skip The status tests used /configured/, which also matches "not configured", so a configured editor shown as not configured still passed. The JSON-RPC skip test accepted any success object, including a decoy line with no jsonrpc field. Probe: the old regex matches "not configured". npm run check passed 449 tests, 92.58% line coverage, and vsce package. Signed-off-by: Sebastien Tardif --- test/unit/verifyMcp.test.ts | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/test/unit/verifyMcp.test.ts b/test/unit/verifyMcp.test.ts index 7123541..207180c 100644 --- a/test/unit/verifyMcp.test.ts +++ b/test/unit/verifyMcp.test.ts @@ -64,15 +64,23 @@ test("parseInitializeResponse returns undefined for non-JSON lines", () => { }); test("parseInitializeResponse skips non-jsonrpc lines", () => { - const data = '{"status":"ok"}\n' + JSON.stringify({ + const data = JSON.stringify({ + result: { serverInfo: { name: "decoy", version: "9.9.9" } } + }) + "\n" + JSON.stringify({ jsonrpc: "2.0", id: 1, - result: { protocolVersion: "2024-11-05", capabilities: {} } + result: { + protocolVersion: "2024-11-05", + serverInfo: { name: "patchloom", version: "0.2.0" }, + capabilities: {} + } }); const result = parseInitializeResponse(data); assert.ok(result); assert.equal(result.ok, true); + assert.equal(result.serverName, "patchloom"); + assert.equal(result.serverVersion, "0.2.0"); }); test("parseInitializeResponse handles multi-line output with blank lines", () => { @@ -173,9 +181,9 @@ test("buildStatusDetails shows per-editor MCP breakdown", () => { } ); - assert.match(details, /VS Code workspace.*configured/); - assert.match(details, /Cursor workspace.*not configured/); - assert.match(details, /Windsurf user.*not configured/); + assert.match(details, /MCP VS Code workspace: \u2713 configured/); + assert.match(details, /MCP Cursor workspace: \u2717 not configured/); + assert.match(details, /MCP Windsurf user: \u2717 not configured/); }); test("buildStatusDetails shows fallback when mcpTargets is undefined", () => { @@ -224,8 +232,8 @@ test("buildStatusDetails shows checkmark for configured targets", () => { } ); - assert.match(details, /\u2713/); - assert.match(details, /configured/); + assert.match(details, /MCP VS Code workspace: \u2713 configured/); + assert.doesNotMatch(details, /not configured/); }); test("buildStatusDetails shows X for unconfigured targets", () => { @@ -251,6 +259,6 @@ test("buildStatusDetails shows X for unconfigured targets", () => { } ); - assert.match(details, /\u2717/); - assert.match(details, /not configured/); + assert.match(details, /MCP Cursor workspace: \u2717 not configured/); + assert.doesNotMatch(details, /\u2713 configured/); }); From 7b18f677b9286087ffabbad906f0bc77ee855b19 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 4 Oct 2026 22:35:54 -0700 Subject: [PATCH 02/25] fix: refuse to rewrite an MCP config that could not be read Configure MCP treated every read error as a missing file, parsed that as an empty object, and wrote a patchloom-only config. A missing file is still created. Any other read error now fails the command before write. Status inspection still ignores read errors so the status bar can render. The README states the same contract. npm run check: 451 passed, line coverage 92.58%. Signed-off-by: Sebastien Tardif --- README.md | 2 +- src/commands/configureMcp.ts | 10 +---- src/mcp/config.ts | 27 +++++++++++- test/unit/mcpConfig.test.ts | 85 ++++++++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index a5d3303..1be2c85 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ Run `Patchloom: Setup Workspace` to walk through everything your project needs: - **Cursor** (`.cursor/mcp.json`) - **Windsurf** (`~/.codeium/windsurf/mcp_config.json`) -When configuring, pick **Full tool inventory** (default) or **Core pack**. Core sets `PATCHLOOM_MCP_SURFACE=core` on the server entry. Existing servers in JSON or JSONC (`//` comments, trailing commas) stay in the file. A config that is not an object is left unchanged and the command reports an error. +When configuring, pick **Full tool inventory** (default) or **Core pack**. Core sets `PATCHLOOM_MCP_SURFACE=core` on the server entry. Existing servers in JSON or JSONC (`//` comments, trailing commas) stay in the file. A config that is not an object is left unchanged and the command reports an error. If an existing file cannot be read, the command reports that error and does not replace it. A missing file is still created. CLI **0.37.0** exposes **66** MCP tools by default (including `explain_plan`, `tidy_check`, `list_files`, `apply_fragment`, `notebook_edit`, and `operation_schema`). The core pack is 12 tools on CLI 0.37+: `read_file`, `search_files`, `list_files`, `replace_text`, `batch_replace`, `doc_get`, `doc_set`, `doc_query`, `md_replace_section`, `execute_plan`, `operation_schema`, `server_info`. `doc` accepts `.jsonc` (comments stay on set), plus `.env`, `.ini`, and `.properties`. `doc_set` accepts `if_exists` to skip a missing file or selector. `apply_patch` accepts `apply=false` for a check-only preview. CLI `list-files` inventories ignore-aware paths the same way as MCP `list_files`. `search_files` accepts `files_without_match` (CLI 0.29+). `apply_patch` accepts unified diffs, Codex `*** Begin Patch`, and Aider SEARCH/REPLACE (CLI 0.30+). `doc_query` can list object keys and count array or object length (CLI 0.32+). Search, replace, and tidy dests accept cwd-only globs such as `*.txt` (CLI 0.33+; use `**/*.txt` or `--glob` for nested files). Absolute paths that resolve inside the MCP workspace root are allowed; empty paths, `../`, and outside paths still reject with stable `error_kind` peels. diff --git a/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index 5433321..faea4b4 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import * as vscode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; -import { configureMcpTargets, inspectMcpTargets } from "../mcp/config.js"; +import { configureMcpTargets, inspectMcpTargets, readMcpConfigText } from "../mcp/config.js"; import { refreshStatusBar } from "../status/statusBar.js"; import { formatError } from "../util.js"; import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; @@ -72,13 +72,7 @@ export async function configureMcp(): Promise { includeUserTarget: environment.supportsUserMcpConfig, patchloomPathSetting: binaryPath, mcpSurface: surfacePick.surface, - readFile: async (filePath) => { - try { - return await fs.readFile(filePath, "utf8"); - } catch { - return undefined; - } - }, + readFile: readMcpConfigText, writeFile: async (filePath, content) => { await fs.mkdir(path.dirname(filePath), { recursive: true }); await fs.writeFile(filePath, content, "utf8"); diff --git a/src/mcp/config.ts b/src/mcp/config.ts index c6b789f..73d07b8 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -1,3 +1,4 @@ +import * as fs from "node:fs/promises"; import * as path from "node:path"; import { parse, type ParseError } from "jsonc-parser"; import { configuredBinaryPathFromSetting } from "../binary/patchloom.js"; @@ -66,7 +67,7 @@ export async function inspectMcpTargets(inputs: McpInspectionInputs): Promise { - const readFile = inputs.readFile ?? defaultReadFile; + const readFile = inputs.readFile ?? readMcpConfigText; const patchloomCommand = configuredBinaryPathFromSetting(inputs.patchloomPathSetting) ?? "patchloom"; const includeKinds = inputs.includeKinds ? new Set(inputs.includeKinds) : undefined; const targets = resolveMcpTargets(inputs.workspaceFolderPath, inputs.homeDir, inputs.includeUserTarget) @@ -205,9 +206,31 @@ function parseJsonObject(content: string | undefined, filePath: string): Record< return { ...parsed }; } +function isEnoent(error: unknown): boolean { + return typeof error === "object" + && error !== null + && "code" in error + && (error as { code?: unknown }).code === "ENOENT"; +} + +/** + * Missing config is undefined. Any other read error throws so configure does + * not replace an unreadable file with a patchloom-only object. + */ +export async function readMcpConfigText(filePath: string): Promise { + try { + return await fs.readFile(filePath, "utf8"); + } catch (error) { + if (isEnoent(error)) { + return undefined; + } + throw error; + } +} + async function defaultReadFile(filePath: string): Promise { try { - return await (await import("node:fs/promises")).readFile(filePath, "utf8"); + return await fs.readFile(filePath, "utf8"); } catch { return undefined; } diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index c046bb0..8e0d423 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -394,3 +394,88 @@ test("configureMcpTargets handles empty config file", async () => { assert.ok((written.servers as Record).patchloom); }); }); + +test("configureMcpTargets does not replace a config it could not read", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + const filePath = path.join(vscodeDir, "mcp.json"); + await fs.mkdir(filePath, { recursive: true }); + + let writes = 0; + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + writeFile: async () => { + writes += 1; + } + }), + (err: unknown) => { + assert.equal(isEnoent(err), false); + return true; + } + ); + assert.equal(writes, 0); + + const inspected = await inspectMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeUserTarget: false + }); + const vscodeTarget = inspected.find((target) => target.kind === "vscode-workspace"); + assert.ok(vscodeTarget); + assert.equal(vscodeTarget.configured, false); + }); +}); + +test("configureMcpTargets leaves an unreadable config file unchanged", async (t) => { + if (process.platform === "win32" || process.getuid?.() === 0) { + t.skip("chmod 0 does not deny read for this process"); + return; + } + + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + const filePath = path.join(vscodeDir, "mcp.json"); + const original = `${JSON.stringify({ + servers: { other: { command: "other-server" } } + }, null, 2)}\n`; + await fs.writeFile(filePath, original, "utf8"); + await fs.chmod(filePath, 0); + + let writes = 0; + try { + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + writeFile: async () => { + writes += 1; + } + }), + (err: unknown) => { + assert.equal(isEnoent(err), false); + return true; + } + ); + assert.equal(writes, 0); + } finally { + await fs.chmod(filePath, 0o644); + } + + const after = await fs.readFile(filePath, "utf8"); + assert.equal(after, original); + }); +}); + +function isEnoent(error: unknown): boolean { + return typeof error === "object" + && error !== null + && "code" in error + && (error as { code?: unknown }).code === "ENOENT"; +} From 23a6623ceec78a84b17dc4e1e74ecaf68ca37f7c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 4 Oct 2026 22:42:14 -0700 Subject: [PATCH 03/25] ci: set a timeout on the path-filter job The changes job in ci.yml was the only workflow job without timeout-minutes. It runs dorny/paths-filter on pull requests. Five minutes matches the other short jobs. Signed-off-by: Sebastien Tardif --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d0ea18..50e59af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,6 +24,7 @@ jobs: # This pattern is used successfully in patchloom and attune-io/attune. if: github.event_name == 'pull_request' runs-on: ubuntu-latest + timeout-minutes: 5 permissions: pull-requests: read outputs: From 8dfae38b28524442f55237e4796937553381e1d8 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 10:29:31 -0700 Subject: [PATCH 04/25] fix: offer the portable workspace MCP config VS Code's Add Server flow prefers a workspace-root .mcp.json file with mcpServers, and the Agent Host reads that file directly. Configure MCP can write that target, including type stdio, and leaves .vscode/mcp.json alone unless that older target is selected. Sibling servers in an existing .mcp.json stay. A second configure of the same entry does not rewrite the file. npm run check: 452 passed, line coverage 92.62%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 1 + README.md | 5 ++- src/commands/configureMcp.ts | 17 ++++++++- src/mcp/config.ts | 28 ++++++++++++-- test/unit/binary.test.ts | 6 ++- test/unit/initializeProject.test.ts | 6 ++- test/unit/mcpConfig.test.ts | 57 +++++++++++++++++++++++++++++ walkthrough/configure-mcp.md | 7 +++- 8 files changed, 117 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4801816..dcaadec 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -133,6 +133,7 @@ All I/O-dependent functions accept an `inputs` object with injectable callbacks | Target | Config file | Key | |--------|------------|-----| | VS Code workspace | `.vscode/mcp.json` | `servers` | +| Portable workspace | `.mcp.json` | `mcpServers` | | Cursor workspace | `.cursor/mcp.json` | `mcpServers` | | Windsurf user | `~/.codeium/windsurf/mcp_config.json` | `mcpServers` | diff --git a/README.md b/README.md index 1be2c85..8312553 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,8 @@ Run `Patchloom: Setup Workspace` to walk through everything your project needs: `Patchloom: Configure MCP` injects the Patchloom MCP server into your editor's config. Supports: -- **VS Code** (`.vscode/mcp.json`) +- **VS Code** (`.vscode/mcp.json`, key `servers`) +- **Portable workspace** (`.mcp.json`, key `mcpServers`). Prefer this file for current VS Code and Copilot. The Agent Host reads it directly. The entry sets `"type": "stdio"`. - **Cursor** (`.cursor/mcp.json`) - **Windsurf** (`~/.codeium/windsurf/mcp_config.json`) @@ -75,7 +76,7 @@ The status bar shows MCP and binary readiness at a glance: - **$(check) Patchloom** when the binary is ready but MCP is not yet set up - **$(warning) Patchloom** when the binary is missing or needs an upgrade -Click it to see full diagnostics, including per-editor MCP configuration status (VS Code, Cursor, Windsurf). +Click it to see full diagnostics, including per-file MCP configuration status (VS Code, portable workspace, Cursor, Windsurf). ### Verify MCP Server diff --git a/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index faea4b4..a1b4c2c 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -26,7 +26,7 @@ export async function configureMcp(): Promise { const selectable = targets.map((target) => ({ label: target.label, description: target.filePath, - detail: target.configured ? "Already configured" : target.exists ? "Config file exists" : "Config file will be created", + detail: targetDetail(target), target })); @@ -94,3 +94,18 @@ export async function configureMcp(): Promise { await refreshStatusBar(); await vscode.window.showInformationMessage(summary || "Patchloom MCP setup completed."); } + +function targetDetail(target: { kind: string; configured: boolean; exists: boolean }): string { + const state = target.configured + ? "Already configured" + : target.exists + ? "Config file exists" + : "Config file will be created"; + if (target.kind === "portable-workspace") { + return `${state}. Preferred for current VS Code and Copilot`; + } + if (target.kind === "vscode-workspace") { + return `${state}. Older VS Code location`; + } + return state; +} diff --git a/src/mcp/config.ts b/src/mcp/config.ts index 73d07b8..fe1cbdf 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -3,7 +3,11 @@ import * as path from "node:path"; import { parse, type ParseError } from "jsonc-parser"; import { configuredBinaryPathFromSetting } from "../binary/patchloom.js"; -export type McpTargetKind = "vscode-workspace" | "cursor-workspace" | "windsurf-user"; +export type McpTargetKind = + | "vscode-workspace" + | "portable-workspace" + | "cursor-workspace" + | "windsurf-user"; export interface McpTarget { readonly kind: McpTargetKind; @@ -112,6 +116,11 @@ export function resolveMcpTargets( label: "VS Code workspace", filePath: path.join(workspaceFolderPath, ".vscode", "mcp.json") }, + { + kind: "portable-workspace", + label: "Portable workspace", + filePath: path.join(workspaceFolderPath, ".mcp.json") + }, { kind: "cursor-workspace", label: "Cursor workspace", @@ -146,7 +155,20 @@ export function buildPatchloomMcpEntry( } function usesMcpServersKey(kind: McpTargetKind): boolean { - return kind === "windsurf-user" || kind === "cursor-workspace"; + return kind === "windsurf-user" || kind === "cursor-workspace" || kind === "portable-workspace"; +} + +function entryForKind( + kind: McpTargetKind, + commandPath: string, + mcpSurface: McpSurface +): Record { + const entry = buildPatchloomMcpEntry(commandPath, mcpSurface); + // Current VS Code marks `type` required on portable stdio servers. + if (kind === "portable-workspace") { + return { type: "stdio", ...entry }; + } + return entry; } function withPatchloomEntry( @@ -155,7 +177,7 @@ function withPatchloomEntry( commandPath: string, mcpSurface: McpSurface = "full" ): Record { - const entry = buildPatchloomMcpEntry(commandPath, mcpSurface); + const entry = entryForKind(kind, commandPath, mcpSurface); if (usesMcpServersKey(kind)) { const servers = objectValue(config.mcpServers); return { diff --git a/test/unit/binary.test.ts b/test/unit/binary.test.ts index 70263c0..97bb1e7 100644 --- a/test/unit/binary.test.ts +++ b/test/unit/binary.test.ts @@ -407,7 +407,11 @@ test("describeWorkspaceEnvironment reports unverified for unknown remote names", test("resolveMcpTargets omits user config targets when disabled", () => { const targets = resolveMcpTargets("/workspace/demo", "/Users/demo", false); - assert.deepEqual(targets.map((target) => target.kind), ["vscode-workspace", "cursor-workspace"]); + assert.deepEqual(targets.map((target) => target.kind), [ + "vscode-workspace", + "portable-workspace", + "cursor-workspace" + ]); }); test("detectManagedInstallTarget maps supported platforms to release targets", () => { diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index ab6287a..5fef597 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -779,12 +779,16 @@ test("inspectMcpTargets reports configured targets", async () => { } }); - assert.equal(targets.length, 3, "should return vscode-workspace, cursor-workspace, windsurf-user"); + assert.equal(targets.length, 4, "should return vscode, portable, cursor, and windsurf"); const vscode = targets.find((t) => t.kind === "vscode-workspace"); + const portable = targets.find((t) => t.kind === "portable-workspace"); const cursor = targets.find((t) => t.kind === "cursor-workspace"); const windsurf = targets.find((t) => t.kind === "windsurf-user"); assert.ok(vscode); assert.equal(vscode.configured, true, "vscode-workspace should be configured"); + assert.ok(portable); + assert.equal(portable.filePath, path.join("/workspace/demo", ".mcp.json")); + assert.equal(portable.configured, false, "portable workspace should not be configured"); assert.ok(cursor); assert.equal(cursor.configured, false, "cursor-workspace should not be configured"); assert.ok(windsurf); diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index 8e0d423..fe1ddff 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -157,6 +157,63 @@ test("configureMcpTargets preserves existing servers in the config file", async }); }); +test("configureMcpTargets writes portable .mcp.json with mcpServers and stdio type", async () => { + await withTempDir(async (workspace) => { + const filePath = path.join(workspace, ".mcp.json"); + await fs.writeFile( + filePath, + `${JSON.stringify({ mcpServers: { other: { command: "other-server" } } }, null, 2)}\n`, + "utf8" + ); + const readFile = async (targetPath: string) => { + try { return await fs.readFile(targetPath, "utf8"); } catch { return undefined; } + }; + const writeFile = async (targetPath: string, content: string) => { + await fs.mkdir(path.dirname(targetPath), { recursive: true }); + await fs.writeFile(targetPath, content, "utf8"); + }; + const inputs = { + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["portable-workspace"] as const, + patchloomPathSetting: "/opt/patchloom", + mcpSurface: "core" as const, + readFile, + writeFile + }; + + const first = await configureMcpTargets(inputs); + assert.equal(first.length, 1); + assert.equal(first[0].changed, true); + assert.equal(first[0].filePath, filePath); + + const written = await readJson(filePath); + assert.equal(written.servers, undefined); + const servers = written.mcpServers as Record>; + assert.equal(servers.other.command, "other-server"); + assert.deepEqual(servers.patchloom, { + type: "stdio", + command: "/opt/patchloom", + args: ["mcp-server"], + env: { PATCHLOOM_MCP_SURFACE: "core" } + }); + await assert.rejects(() => fs.stat(path.join(workspace, ".vscode", "mcp.json"))); + + const second = await configureMcpTargets(inputs); + assert.equal(second[0].changed, false); + + const inspected = await inspectMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + readFile + }); + const portable = inspected.find((target) => target.kind === "portable-workspace"); + const vscodeTarget = inspected.find((target) => target.kind === "vscode-workspace"); + assert.equal(portable?.configured, true); + assert.equal(vscodeTarget?.configured, false); + }); +}); + test("configureMcpTargets writes Cursor config with mcpServers key", async () => { await withTempDir(async (workspace) => { await configureMcpTargets({ diff --git a/walkthrough/configure-mcp.md b/walkthrough/configure-mcp.md index f35bcc3..cfdaf1a 100644 --- a/walkthrough/configure-mcp.md +++ b/walkthrough/configure-mcp.md @@ -12,10 +12,13 @@ for your editor. Choose the **full** tool inventory or the **core** pack | Editor | Config file | Key | |--------|-------------|-----| | VS Code | `.vscode/mcp.json` | `servers` | +| Portable workspace | `.mcp.json` | `mcpServers` | | Cursor | `.cursor/mcp.json` | `mcpServers` | | Windsurf | `~/.codeium/windsurf/mcp_config.json` | `mcpServers` | -The command lists the editors it can configure. You pick one or more -targets, then Full vs Core. It does not write every editor on its own. +Prefer **Portable workspace** for current VS Code and Copilot. That file +lives at the workspace root and sets `"type": "stdio"`. The older VS Code +file remains available. You pick one or more targets, then Full vs Core. +The command does not write every editor on its own. See the [MCP setup guide](https://patchloom.github.io/patchloom/getting-started/mcp-setup.html) for advanced configuration. From 0f095a7a2d13237666c575c006f0d5b4b5854ec4 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 10:36:34 -0700 Subject: [PATCH 05/25] docs: match status bar text to the item the user sees The configured state is "$(plug) Patchloom" plus the detected version when known. The ready-without-MCP state is "$(check) Patchloom" plus that version. A missing or old CLI stays "$(warning) Patchloom". npm run check: 452 passed, line coverage 92.65%. Signed-off-by: Sebastien Tardif --- README.md | 4 ++-- walkthrough/install.md | 7 +++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8312553..55b4e82 100644 --- a/README.md +++ b/README.md @@ -72,8 +72,8 @@ CLI **0.37.0** exposes **66** MCP tools by default (including `explain_plan`, `t The status bar shows MCP and binary readiness at a glance: -- **$(plug) Patchloom MCP** when the MCP server is configured -- **$(check) Patchloom** when the binary is ready but MCP is not yet set up +- **$(plug) Patchloom**, with the detected version when known (`$(plug) Patchloom v0.37.0`), when an MCP config file is configured +- **$(check) Patchloom**, with the detected version when known, when the binary is ready and MCP is not configured - **$(warning) Patchloom** when the binary is missing or needs an upgrade Click it to see full diagnostics, including per-file MCP configuration status (VS Code, portable workspace, Cursor, Windsurf). diff --git a/walkthrough/install.md b/walkthrough/install.md index 07ae266..98ac298 100644 --- a/walkthrough/install.md +++ b/walkthrough/install.md @@ -50,5 +50,8 @@ cargo install patchloom curl --proto '=https' --tlsv1.2 -LsSf https://github.com/patchloom/patchloom/releases/latest/download/patchloom-installer.sh | sh ``` -After installation, the status bar shows a green checkmark when the CLI -is detected. +After installation, the status bar shows `$(check) Patchloom` when the +CLI is detected and MCP is not configured yet. It shows `$(plug) Patchloom` +when an MCP config file is configured, and `$(warning) Patchloom` when +the CLI is missing or too old. A detected version is appended on the +check and plug states. From 5b7245485bad589ca4a8ea1db1fab4202fc2a764 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 10:47:03 -0700 Subject: [PATCH 06/25] fix: refuse MCP config paths that leave the workspace Configure MCP followed a symlink at .mcp.json or .vscode and could replace a file outside the folder. Status refresh read that same path on activation. Both paths now use the realpath check quick actions already use. A Windsurf directory that does not exist yet is still created. A symlink to an outside file is not read and is not written. npm run check: 453 passed, line coverage 92.55%. Signed-off-by: Sebastien Tardif --- src/commands/quickActions.ts | 93 +++---------------------- src/mcp/config.ts | 42 ++++++++++++ src/workspace/pathContainment.ts | 84 +++++++++++++++++++++++ test/unit/initializeProject.test.ts | 102 ++++++++++++++++------------ test/unit/mcpConfig.test.ts | 53 +++++++++++++++ 5 files changed, 246 insertions(+), 128 deletions(-) create mode 100644 src/workspace/pathContainment.ts diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 512d590..415fbf7 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -1,5 +1,4 @@ import { execFile } from "node:child_process"; -import { realpathSync } from "node:fs"; import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; @@ -16,6 +15,16 @@ import { } from "../logging/outputChannel.js"; import { formatCliOutput, formatError, formatQuickActionCliOutput, mergePatchloomEnv } from "../util.js"; import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; +import { + isRealPathInsideWorkspace, + resolveWorkspaceRelativePath +} from "../workspace/pathContainment.js"; + +export { + isPathInsideWorkspace, + isRealPathInsideWorkspace, + resolveWorkspaceRelativePath +} from "../workspace/pathContainment.js"; const execFileAsync = promisify(execFile); const STRUCTURED_FILE_EXTENSIONS = new Set([ @@ -1930,88 +1939,6 @@ async function inputWorkspaceFileTarget(folder: VSCode.WorkspaceFolder): Promise } } -function pathEscapesWorkspace(relativePath: string): boolean { - return relativePath === ".." || relativePath.startsWith(`..${path.sep}`); -} - -export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath: string): string { - const resolvedRoot = path.resolve(workspaceRoot); - const resolvedPath = path.resolve(absolutePath); - const relativePath = path.relative(resolvedRoot, resolvedPath); - if (!relativePath || pathEscapesWorkspace(relativePath) || path.isAbsolute(relativePath)) { - throw new Error( - "File path must stay inside the current workspace folder. Use a path under this folder (for example src/app.ts), or open the folder that owns the file." - ); - } - return relativePath.split(path.sep).join("/"); -} - -function isResolvedPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { - const resolvedRoot = path.resolve(workspaceRoot); - const resolvedPath = path.resolve(absolutePath); - const fold = process.platform === "win32" || process.platform === "darwin" - ? (value: string) => value.toLowerCase() - : (value: string) => value; - const root = fold(resolvedRoot); - const target = fold(resolvedPath); - return target === root || target.startsWith(`${root}${path.sep}`); -} - -export function isPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { - return isResolvedPathInsideWorkspace(workspaceRoot, absolutePath); -} - -function isEnoent(error: unknown): boolean { - return typeof error === "object" - && error !== null - && "code" in error - && (error as { code?: unknown }).code === "ENOENT"; -} - -/** Real path of absolutePath, or of its nearest existing ancestor plus the missing suffix. */ -function realPathAllowingMissingSuffix(absolutePath: string): string | undefined { - const missing: string[] = []; - let current = path.resolve(absolutePath); - while (true) { - try { - const realAncestor = realpathSync(current); - if (missing.length === 0) { - return realAncestor; - } - return path.join(realAncestor, ...missing.slice().reverse()); - } catch (error) { - if (!isEnoent(error)) { - return undefined; - } - const parent = path.dirname(current); - if (parent === current) { - return undefined; - } - missing.push(path.basename(current)); - current = parent; - } - } -} - -/** - * True when the real path stays inside the workspace. - * A missing leaf uses the nearest existing ancestor's real path plus the - * missing suffix. Existing files that realpath outside stay rejected. - */ -export function isRealPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { - let realRoot: string; - try { - realRoot = realpathSync(workspaceRoot); - } catch { - return false; - } - const resolved = realPathAllowingMissingSuffix(absolutePath); - if (resolved === undefined) { - return false; - } - return isResolvedPathInsideWorkspace(realRoot, resolved); -} - export interface StagedExternalPatch { readonly patchPath: string; readonly cleanup: () => Promise; diff --git a/src/mcp/config.ts b/src/mcp/config.ts index fe1cbdf..1c97172 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -1,7 +1,9 @@ +import { existsSync } from "node:fs"; import * as fs from "node:fs/promises"; import * as path from "node:path"; import { parse, type ParseError } from "jsonc-parser"; import { configuredBinaryPathFromSetting } from "../binary/patchloom.js"; +import { isRealPathInsideWorkspace } from "../workspace/pathContainment.js"; export type McpTargetKind = | "vscode-workspace" @@ -51,6 +53,14 @@ export async function inspectMcpTargets(inputs: McpInspectionInputs): Promise value.toLowerCase() + : (value: string) => value; + const foldedRoot = fold(resolvedRoot); + const target = fold(resolvedPath); + return target === foldedRoot || target.startsWith(`${foldedRoot}${path.sep}`); +} + +export function isPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { + return isResolvedPathInsideRoot(workspaceRoot, absolutePath); +} + +function isEnoent(error: unknown): boolean { + return typeof error === "object" + && error !== null + && "code" in error + && (error as { code?: unknown }).code === "ENOENT"; +} + +/** Real path of absolutePath, or of its nearest existing ancestor plus the missing suffix. */ +function realPathAllowingMissingSuffix(absolutePath: string): string | undefined { + const missing: string[] = []; + let current = path.resolve(absolutePath); + while (true) { + try { + const realAncestor = realpathSync(current); + if (missing.length === 0) { + return realAncestor; + } + return path.join(realAncestor, ...missing.slice().reverse()); + } catch (error) { + if (!isEnoent(error)) { + return undefined; + } + const parent = path.dirname(current); + if (parent === current) { + return undefined; + } + missing.push(path.basename(current)); + current = parent; + } + } +} + +/** + * True when the real path stays inside the root. + * A missing leaf uses the nearest existing ancestor's real path plus the + * missing suffix. Existing files that realpath outside stay rejected. + */ +export function isRealPathInsideWorkspace(root: string, absolutePath: string): boolean { + let realRoot: string; + try { + realRoot = realpathSync(root); + } catch { + return false; + } + const resolved = realPathAllowingMissingSuffix(absolutePath); + if (resolved === undefined) { + return false; + } + return isResolvedPathInsideRoot(realRoot, resolved); +} + +export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath: string): string { + const resolvedRoot = path.resolve(workspaceRoot); + const resolvedPath = path.resolve(absolutePath); + const relativePath = path.relative(resolvedRoot, resolvedPath); + if (!relativePath || pathEscapesRoot(relativePath) || path.isAbsolute(relativePath)) { + throw new Error( + "File path must stay inside the current workspace folder. Use a path under this folder (for example src/app.ts), or open the folder that owns the file." + ); + } + return relativePath.split(path.sep).join("/"); +} diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index 5fef597..2f0910c 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -1,4 +1,6 @@ import assert from "node:assert/strict"; +import * as fs from "node:fs/promises"; +import * as os from "node:os"; import * as path from "node:path"; import test from "node:test"; import { MINIMUM_SUPPORTED_PATCHLOOM_VERSION } from "../../src/binary/patchloom.js"; @@ -768,58 +770,68 @@ test("buildPatchloomMcpEntry points at patchloom mcp-server", () => { }); test("inspectMcpTargets reports configured targets", async () => { - const targets = await inspectMcpTargets({ - workspaceFolderPath: "/workspace/demo", - homeDir: "/Users/demo", - readFile: async (filePath) => { - if (filePath.endsWith(path.join(".vscode", "mcp.json"))) { - return '{"servers":{"patchloom":{"command":"patchloom","args":["mcp-server"]}}}'; + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-mcp-inspect-")); + try { + const targets = await inspectMcpTargets({ + workspaceFolderPath: workspace, + homeDir: path.join(workspace, "home"), + readFile: async (filePath) => { + if (filePath.endsWith(path.join(".vscode", "mcp.json"))) { + return '{"servers":{"patchloom":{"command":"patchloom","args":["mcp-server"]}}}'; + } + return undefined; } - return undefined; - } - }); + }); - assert.equal(targets.length, 4, "should return vscode, portable, cursor, and windsurf"); - const vscode = targets.find((t) => t.kind === "vscode-workspace"); - const portable = targets.find((t) => t.kind === "portable-workspace"); - const cursor = targets.find((t) => t.kind === "cursor-workspace"); - const windsurf = targets.find((t) => t.kind === "windsurf-user"); - assert.ok(vscode); - assert.equal(vscode.configured, true, "vscode-workspace should be configured"); - assert.ok(portable); - assert.equal(portable.filePath, path.join("/workspace/demo", ".mcp.json")); - assert.equal(portable.configured, false, "portable workspace should not be configured"); - assert.ok(cursor); - assert.equal(cursor.configured, false, "cursor-workspace should not be configured"); - assert.ok(windsurf); - assert.equal(windsurf.configured, false, "windsurf-user should not be configured"); + assert.equal(targets.length, 4, "should return vscode, portable, cursor, and windsurf"); + const vscode = targets.find((t) => t.kind === "vscode-workspace"); + const portable = targets.find((t) => t.kind === "portable-workspace"); + const cursor = targets.find((t) => t.kind === "cursor-workspace"); + const windsurf = targets.find((t) => t.kind === "windsurf-user"); + assert.ok(vscode); + assert.equal(vscode.configured, true, "vscode-workspace should be configured"); + assert.ok(portable); + assert.equal(portable.filePath, path.join(workspace, ".mcp.json")); + assert.equal(portable.configured, false, "portable workspace should not be configured"); + assert.ok(cursor); + assert.equal(cursor.configured, false, "cursor-workspace should not be configured"); + assert.ok(windsurf); + assert.equal(windsurf.configured, false, "windsurf-user should not be configured"); + } finally { + await fs.rm(workspace, { recursive: true, force: true }); + } }); test("configureMcpTargets creates or updates only the selected target kinds", async () => { - const writes = new Map(); - const results = await configureMcpTargets({ - workspaceFolderPath: "/workspace/demo", - homeDir: "/Users/demo", - includeKinds: ["cursor-workspace"], - patchloomPathSetting: "/custom/patchloom", - readFile: async (filePath) => { - if (filePath.endsWith(path.join(".cursor", "mcp.json"))) { - return '{"servers":{"other":{"command":"other"}}}'; + const workspace = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-mcp-select-")); + try { + const writes = new Map(); + const results = await configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: path.join(workspace, "home"), + includeKinds: ["cursor-workspace"], + patchloomPathSetting: "/custom/patchloom", + readFile: async (filePath) => { + if (filePath.endsWith(path.join(".cursor", "mcp.json"))) { + return '{"servers":{"other":{"command":"other"}}}'; + } + return undefined; + }, + writeFile: async (filePath, content) => { + writes.set(filePath, content); } - return undefined; - }, - writeFile: async (filePath, content) => { - writes.set(filePath, content); - } - }); + }); - assert.equal(results.length, 1); - const cursorPath = path.join("/workspace/demo", ".cursor", "mcp.json"); - assert.equal(writes.has(cursorPath), true); - assert.equal(writes.has(path.join("/workspace/demo", ".vscode", "mcp.json")), false); - assert.match(writes.get(cursorPath) ?? "", /patchloom/); - assert.match(writes.get(cursorPath) ?? "", /mcp-server/); - assert.match(writes.get(cursorPath) ?? "", /other/); + assert.equal(results.length, 1); + const cursorPath = path.join(workspace, ".cursor", "mcp.json"); + assert.equal(writes.has(cursorPath), true); + assert.equal(writes.has(path.join(workspace, ".vscode", "mcp.json")), false); + assert.match(writes.get(cursorPath) ?? "", /patchloom/); + assert.match(writes.get(cursorPath) ?? "", /mcp-server/); + assert.match(writes.get(cursorPath) ?? "", /other/); + } finally { + await fs.rm(workspace, { recursive: true, force: true }); + } }); test("buildAgentRulesArgs omits default all modes", () => { diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index fe1ddff..3fabe68 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -214,6 +214,59 @@ test("configureMcpTargets writes portable .mcp.json with mcpServers and stdio ty }); }); +test("configureMcpTargets refuses a config symlink that leaves the workspace", async (t) => { + await withTempDir(async (workspace) => { + const outside = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-mcp-outside-")); + try { + const secret = path.join(outside, "secret.txt"); + await fs.writeFile(secret, "do-not-touch", "utf8"); + const linkPath = path.join(workspace, ".mcp.json"); + try { + await fs.symlink(secret, linkPath); + } catch { + t.skip("fs.symlink is not available on this platform"); + return; + } + + let reads = 0; + const inspected = await inspectMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeUserTarget: false, + readFile: async (filePath) => { + if (filePath === linkPath) { + reads += 1; + } + return undefined; + } + }); + assert.equal(reads, 0); + assert.equal( + inspected.find((target) => target.kind === "portable-workspace")?.configured, + false + ); + + let writes = 0; + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["portable-workspace"], + patchloomPathSetting: "patchloom", + writeFile: async () => { + writes += 1; + } + }), + /resolves outside/ + ); + assert.equal(writes, 0); + assert.equal(await fs.readFile(secret, "utf8"), "do-not-touch"); + } finally { + await fs.rm(outside, { recursive: true, force: true }); + } + }); +}); + test("configureMcpTargets writes Cursor config with mcpServers key", async () => { await withTempDir(async (workspace) => { await configureMcpTargets({ From 10215ea94a94b65a5ddb93985510502292ca1c52 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 11:16:08 -0700 Subject: [PATCH 07/25] fix: accept workspace paths that use a different symlink prefix On macOS, /var and /tmp are symlinks to /private. A Quick Action or Create could treat a file inside the folder as outside when the workspace URI and the file path used different prefixes. The relative path now falls back to the real directory only when that real path stays inside the workspace. npm run check: 454 passed, line coverage 92.56%. Signed-off-by: Sebastien Tardif --- src/workspace/pathContainment.ts | 33 +++++++++++++++++++++++++------- test/unit/quickActions.test.ts | 30 +++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/src/workspace/pathContainment.ts b/src/workspace/pathContainment.ts index 0b4aa69..b50e20b 100644 --- a/src/workspace/pathContainment.ts +++ b/src/workspace/pathContainment.ts @@ -71,14 +71,33 @@ export function isRealPathInsideWorkspace(root: string, absolutePath: string): b return isResolvedPathInsideRoot(realRoot, resolved); } -export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath: string): string { - const resolvedRoot = path.resolve(workspaceRoot); - const resolvedPath = path.resolve(absolutePath); - const relativePath = path.relative(resolvedRoot, resolvedPath); +const workspacePathError = + "File path must stay inside the current workspace folder. Use a path under this folder (for example src/app.ts), or open the folder that owns the file."; + +function relativeInsideRoot(root: string, absolutePath: string): string | undefined { + const relativePath = path.relative(path.resolve(root), path.resolve(absolutePath)); if (!relativePath || pathEscapesRoot(relativePath) || path.isAbsolute(relativePath)) { - throw new Error( - "File path must stay inside the current workspace folder. Use a path under this folder (for example src/app.ts), or open the folder that owns the file." - ); + return undefined; } return relativePath.split(path.sep).join("/"); } + +export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath: string): string { + const lexical = relativeInsideRoot(workspaceRoot, absolutePath); + if (lexical) { + return lexical; + } + // /var and /tmp are symlinks to /private on macOS. The folder URI and a + // pasted path can name one file with two prefixes. Accept that only when + // the real path is still inside the real workspace. + if (isRealPathInsideWorkspace(workspaceRoot, absolutePath)) { + const realFile = realPathAllowingMissingSuffix(absolutePath); + if (realFile) { + const viaReal = relativeInsideRoot(realpathSync(workspaceRoot), realFile); + if (viaReal) { + return viaReal; + } + } + } + throw new Error(workspacePathError); +} diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 3bf2a40..4e64271 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -510,6 +510,36 @@ test("resolveWorkspaceRelativePath rejects workspace root itself", () => { ); }); +test("resolveWorkspaceRelativePath accepts the real path of a symlinked workspace", async (t) => { + const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-rel-")); + const linkRoot = `${realRoot}-link`; + const outside = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-rel-out-")); + try { + await fs.writeFile(path.join(realRoot, "a.ts"), "x"); + try { + await fs.symlink(realRoot, linkRoot, "dir"); + } catch { + t.skip("fs.symlink is not available on this platform"); + return; + } + + assert.equal(resolveWorkspaceRelativePath(linkRoot, path.join(realRoot, "a.ts")), "a.ts"); + assert.equal(resolveWorkspaceRelativePath(realRoot, path.join(linkRoot, "a.ts")), "a.ts"); + assert.equal( + resolveWorkspaceRelativePath(linkRoot, path.join(realRoot, "src", "new.ts")), + "src/new.ts" + ); + assert.throws( + () => resolveWorkspaceRelativePath(linkRoot, path.join(outside, "x.ts")), + /must stay inside the current workspace folder/ + ); + } finally { + await fs.rm(realRoot, { recursive: true, force: true }); + await fs.rm(linkRoot, { force: true }); + await fs.rm(outside, { recursive: true, force: true }); + } +}); + // --- #32: edge-case builder tests --- test("buildSearchQuickAction with empty pattern produces valid args", () => { From aea69a54a330808dbed86684d560b4416cba5408 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 11:21:31 -0700 Subject: [PATCH 08/25] fix: match unsaved editors across symlink path prefixes Quick actions asked to save a dirty file only when the editor path and the workspace path were the same string. On macOS those strings differ for /var and /private/var, so a typed path skipped the save prompt and the later apply could overwrite unsaved edits. Path identity now uses realpath. npm run check: 455 passed, line coverage 92.59%. Signed-off-by: Sebastien Tardif --- src/commands/quickActions.ts | 15 +++++---------- src/workspace/pathContainment.ts | 20 ++++++++++++++++++++ test/unit/quickActions.test.ts | 26 ++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 415fbf7..7eb3733 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -17,13 +17,15 @@ import { formatCliOutput, formatError, formatQuickActionCliOutput, mergePatchloo import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; import { isRealPathInsideWorkspace, - resolveWorkspaceRelativePath + resolveWorkspaceRelativePath, + sameRealFilePath } from "../workspace/pathContainment.js"; export { isPathInsideWorkspace, isRealPathInsideWorkspace, - resolveWorkspaceRelativePath + resolveWorkspaceRelativePath, + sameRealFilePath } from "../workspace/pathContainment.js"; const execFileAsync = promisify(execFile); @@ -1999,7 +2001,7 @@ async function ensureWorkspaceFileReady(target: WorkspaceFileTarget): Promise sameFilePath(document.uri.fsPath, target.absolutePath)); + const openDocument = vscode.workspace.textDocuments.find((document) => sameRealFilePath(document.uri.fsPath, target.absolutePath)); if (openDocument?.isDirty) { const choice = await vscode.window.showWarningMessage( `${target.relativePath} has unsaved changes. Save it before running Patchloom quick actions.`, @@ -2056,13 +2058,6 @@ async function executePatchloom( } } -function sameFilePath(left: string, right: string): boolean { - const normalize = (value: string) => process.platform === "win32" - ? path.resolve(value).toLowerCase() - : path.resolve(value); - return normalize(left) === normalize(right); -} - function asExecFailure(error: unknown): (Error & { stdout: string; stderr: string; exitCode: number }) | undefined { if (!(error instanceof Error)) { return undefined; diff --git a/src/workspace/pathContainment.ts b/src/workspace/pathContainment.ts index b50e20b..9538ac3 100644 --- a/src/workspace/pathContainment.ts +++ b/src/workspace/pathContainment.ts @@ -82,6 +82,26 @@ function relativeInsideRoot(root: string, absolutePath: string): string | undefi return relativePath.split(path.sep).join("/"); } +/** + * True when both paths name the same file. realpath folds macOS /var and + * /tmp onto /private so an open editor matches a workspace-relative path. + * A path that does not exist yet is compared as a resolved string. + */ +export function sameRealFilePath(left: string, right: string): boolean { + return normalizeRealFilePath(left) === normalizeRealFilePath(right); +} + +function normalizeRealFilePath(value: string): string { + const resolved = path.resolve(value); + let normalized = resolved; + try { + normalized = realpathSync(resolved); + } catch { + normalized = resolved; + } + return process.platform === "win32" ? normalized.toLowerCase() : normalized; +} + export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath: string): string { const lexical = relativeInsideRoot(workspaceRoot, absolutePath); if (lexical) { diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 4e64271..09df3af 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -47,6 +47,7 @@ import { presentSearchOutcome, presentUndoSuccess, resolveWorkspaceRelativePath, + sameRealFilePath, retargetQuickAction, serializePatchloomArgs, withEndOfOptions, @@ -993,6 +994,31 @@ test("formatUndoFailureMessage prefixes other failures with Patchloom undo faile ); }); +test("sameRealFilePath matches files across a symlinked workspace root", async (t) => { + const realRoot = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-same-")); + const linkRoot = `${realRoot}-link`; + try { + const filePath = path.join(realRoot, "a.ts"); + await fs.writeFile(filePath, "x"); + const otherPath = path.join(realRoot, "b.ts"); + await fs.writeFile(otherPath, "y"); + try { + await fs.symlink(realRoot, linkRoot, "dir"); + } catch { + t.skip("fs.symlink is not available on this platform"); + return; + } + + assert.equal(sameRealFilePath(path.join(linkRoot, "a.ts"), filePath), true); + assert.equal(sameRealFilePath(filePath, path.join(linkRoot, "a.ts")), true); + assert.equal(sameRealFilePath(path.join(linkRoot, "a.ts"), otherPath), false); + assert.equal(sameRealFilePath("/workspace/demo/missing.ts", "/workspace/demo/missing.ts"), true); + } finally { + await fs.rm(realRoot, { recursive: true, force: true }); + await fs.rm(linkRoot, { force: true }); + } +}); + test("isRealPathInsideWorkspace follows symlinks and stays fail-closed", async (t) => { const workspaceRoot = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-realpath-ws-")); const outsideRoot = await fs.mkdtemp(path.join(os.tmpdir(), "patchloom-realpath-out-")); From e4d4ddff7935146f54d031482950d1fbea960b5f Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 11:29:08 -0700 Subject: [PATCH 09/25] fix: show the output channel when a command fails Create, doc get, doc keys, doc len, and undo returned on a non-zero exit before writing the CLI streams. Initialize, Configure MCP, Verify MCP, and managed install or update failures left the text in a toast or an unopened channel. With tracing off, the full error was easy to miss. Those failures now open the Patchloom output channel. npm run check: 455 passed, line coverage 92.53%. Signed-off-by: Sebastien Tardif --- src/commands/configureMcp.ts | 7 ++++++- src/commands/initializeProject.ts | 8 ++++++-- src/commands/managedInstall.ts | 3 +++ src/commands/quickActions.ts | 5 +++++ src/commands/verifyMcp.ts | 1 + 5 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index a1b4c2c..4787ff8 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -2,6 +2,7 @@ import * as fs from "node:fs/promises"; import * as path from "node:path"; import * as vscode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; +import { getPatchloomLog, writeUserVisibleOutput } from "../logging/outputChannel.js"; import { configureMcpTargets, inspectMcpTargets, readMcpConfigText } from "../mcp/config.js"; import { refreshStatusBar } from "../status/statusBar.js"; import { formatError } from "../util.js"; @@ -79,7 +80,11 @@ export async function configureMcp(): Promise { } }); } catch (error) { - await vscode.window.showErrorMessage(`Failed to configure MCP: ${formatError(error)}`); + const message = formatError(error); + const log = getPatchloomLog(); + writeUserVisibleOutput(log, message); + log?.show(); + await vscode.window.showErrorMessage(`Failed to configure MCP: ${message}`); return; } diff --git a/src/commands/initializeProject.ts b/src/commands/initializeProject.ts index 85d5bfa..123c977 100644 --- a/src/commands/initializeProject.ts +++ b/src/commands/initializeProject.ts @@ -2,7 +2,7 @@ import { execFile } from "node:child_process"; import { promisify } from "node:util"; import type * as VSCode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; -import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult } from "../logging/outputChannel.js"; +import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult, writeUserVisibleOutput } from "../logging/outputChannel.js"; import { formatCliOutput, formatError, mergePatchloomEnv } from "../util.js"; import { activeWorkspaceFolder } from "../workspace/readiness.js"; @@ -84,7 +84,11 @@ export async function initializeProject(): Promise { trace: runtime.trace }); } catch (error) { - await vscode.window.showErrorMessage(`Failed to run patchloom agent-rules in ${folder.name}: ${formatError(error)}`); + const message = formatError(error); + const log = getPatchloomLog(); + writeUserVisibleOutput(log, message); + log?.show(); + await vscode.window.showErrorMessage(`Failed to run patchloom agent-rules in ${folder.name}: ${message}`); return; } diff --git a/src/commands/managedInstall.ts b/src/commands/managedInstall.ts index faeee3b..f978a6f 100644 --- a/src/commands/managedInstall.ts +++ b/src/commands/managedInstall.ts @@ -92,6 +92,7 @@ export async function installPatchloom(): Promise { } catch (error) { const message = formatError(error); log?.log(`Managed install failed: ${message}`); + log?.show(); await vscode.window.showErrorMessage(`Failed to install Patchloom: ${message}`); } } @@ -171,6 +172,7 @@ export async function updatePatchloom(): Promise { } catch (error) { const message = formatError(error); log?.log(`Managed update failed: ${message}`); + log?.show(); await vscode.window.showErrorMessage(`Failed to update Patchloom: ${message}`); } } @@ -225,6 +227,7 @@ export async function reinstallPatchloom(): Promise { } catch (error) { const message = formatError(error); log?.log(`Managed reinstall failed: ${message}`); + log?.show(); await vscode.window.showErrorMessage(`Failed to reinstall Patchloom: ${message}`); } } diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 7eb3733..d24f9a0 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -566,6 +566,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action, folder.uri.fsPath); if (result.exitCode !== 0) { + presentCliResultInOutput(getPatchloomLog(), result); await vscode.window.showErrorMessage(`Patchloom create failed: ${formatCliOutput(result)}`); return; } @@ -643,6 +644,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action, target.workspaceFolder.uri.fsPath); if (result.exitCode !== 0) { + presentCliResultInOutput(getPatchloomLog(), result); await vscode.window.showErrorMessage(`Patchloom doc get failed: ${formatCliOutput(result)}`); return; } @@ -676,6 +678,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action, target.workspaceFolder.uri.fsPath); if (result.exitCode !== 0) { + presentCliResultInOutput(getPatchloomLog(), result); await vscode.window.showErrorMessage(`Patchloom doc keys failed: ${formatCliOutput(result)}`); return; } @@ -709,6 +712,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action, target.workspaceFolder.uri.fsPath); if (result.exitCode !== 0) { + presentCliResultInOutput(getPatchloomLog(), result); await vscode.window.showErrorMessage(`Patchloom doc len failed: ${formatCliOutput(result)}`); return; } @@ -1283,6 +1287,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action, folder.uri.fsPath); if (result.exitCode !== 0) { + presentCliResultInOutput(getPatchloomLog(), result); await vscode.window.showWarningMessage(formatUndoFailureMessage(result)); return; } diff --git a/src/commands/verifyMcp.ts b/src/commands/verifyMcp.ts index 3caf709..f039d1f 100644 --- a/src/commands/verifyMcp.ts +++ b/src/commands/verifyMcp.ts @@ -40,6 +40,7 @@ export async function verifyMcp(): Promise { if (result.ok) { await vscode.window.showInformationMessage(result.message); } else { + log?.show(); await vscode.window.showErrorMessage(result.message); } } From bf6b3c58e91a223435c031ced3d11b431e9e6161 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 11:48:50 -0700 Subject: [PATCH 10/25] fix: keep JSONC comments when configuring MCP Configure MCP parsed mcp.json and wrote it back with JSON.stringify. A comment or a trailing comma in .vscode/mcp.json disappeared the first time the patchloom server was added, and a matching entry was rewritten when its keys were in a different order. Existing files are now edited in place. A servers value that is not an object is left unchanged. npm run check: 457 passed, line coverage 92.56%. Signed-off-by: Sebastien Tardif --- src/mcp/config.ts | 55 ++++++++++++++++++++++++++----- test/unit/mcpConfig.test.ts | 65 +++++++++++++++++++++++++++++++++++-- 2 files changed, 110 insertions(+), 10 deletions(-) diff --git a/src/mcp/config.ts b/src/mcp/config.ts index 1c97172..8228ca7 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -1,7 +1,7 @@ import { existsSync } from "node:fs"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import { parse, type ParseError } from "jsonc-parser"; +import { applyEdits, modify, parse, type ParseError } from "jsonc-parser"; import { configuredBinaryPathFromSetting } from "../binary/patchloom.js"; import { isRealPathInsideWorkspace } from "../workspace/pathContainment.js"; @@ -93,20 +93,37 @@ export async function configureMcpTargets(inputs: McpApplyInputs): Promise 0; - if (changed) { - await inputs.writeFile(target.filePath, serialized); + if (hasText && isPlainObject(existingRoot) && stableJson(currentEntry) === stableJson(entry)) { + results.push({ + ...target, + exists: true, + configured: true, + changed: false + }); + continue; } + if (hasText && existingRoot !== undefined && !isPlainObject(existingRoot)) { + throw new Error(`Cannot update MCP config ${target.filePath}: "${key}" must be a JSON object`); + } + + // JSON.stringify of the parse drops comments and trailing commas. + const serialized = typeof content === "string" && content.trim().length > 0 + ? applyPatchloomEntry(content, key, entry) + : `${JSON.stringify(withPatchloomEntry(target.kind, original, patchloomCommand, mcpSurface), null, 2)}\n`; + await inputs.writeFile(target.filePath, serialized); + results.push({ ...target, exists: content !== undefined, configured: true, - changed + changed: true }); } @@ -257,6 +274,28 @@ function isPlainObject(value: unknown): value is Record { return value !== null && typeof value === "object" && !Array.isArray(value); } +function applyPatchloomEntry(content: string, key: string, entry: Record): string { + const edits = modify(content, [key, "patchloom"], entry, { + formattingOptions: { insertSpaces: true, tabSize: 2 } + }); + return applyEdits(content, edits); +} + +function stableJson(value: unknown): string { + if (value === undefined) { + return "undefined"; + } + if (value === null || typeof value !== "object") { + return JSON.stringify(value) ?? "null"; + } + if (Array.isArray(value)) { + return `[${value.map((item) => stableJson(item)).join(",")}]`; + } + const record = value as Record; + const keys = Object.keys(record).sort(); + return `{${keys.map((key) => `${JSON.stringify(key)}:${stableJson(record[key])}`).join(",")}}`; +} + function parseJsonObject(content: string | undefined, filePath: string): Record { if (!content || !content.trim()) { return {}; diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index 3fabe68..192e096 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -3,6 +3,7 @@ import * as fs from "node:fs/promises"; import * as os from "node:os"; import * as path from "node:path"; import test from "node:test"; +import { parse as parseJsonc } from "jsonc-parser"; import { buildPatchloomMcpEntry, configureMcpTargets, @@ -119,13 +120,73 @@ test("configureMcpTargets preserves sibling servers in JSONC mcp.json", async () } }); - const written = await readJson(filePath); - const servers = written.servers as Record; + const written = await fs.readFile(filePath, "utf8"); + assert.match(written, /\/\/ comment/); + const parsed = parseJsonc(written, [], { allowTrailingComma: true }) as Record; + const servers = parsed.servers as Record; assert.ok(servers.github, "existing github server should be preserved"); assert.ok(servers.patchloom, "patchloom server should be added"); }); }); +test("configureMcpTargets leaves a matching JSONC entry untouched", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + const filePath = path.join(vscodeDir, "mcp.json"); + const original = `{ + // owner note + "servers": { + "patchloom": { + "args": ["mcp-server"], + "command": "patchloom" + } + } +} +`; + await fs.writeFile(filePath, original, "utf8"); + + const results = await configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + readFile: async (targetPath) => fs.readFile(targetPath, "utf8"), + writeFile: async () => { + throw new Error("matching MCP config must not be rewritten"); + } + }); + + assert.equal(results[0].changed, false); + assert.equal(await fs.readFile(filePath, "utf8"), original); + }); +}); + +test("configureMcpTargets refuses a non-object servers value", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + const filePath = path.join(vscodeDir, "mcp.json"); + const original = `{ "servers": [] }\n`; + await fs.writeFile(filePath, original, "utf8"); + + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace"], + patchloomPathSetting: "patchloom", + readFile: async (targetPath) => fs.readFile(targetPath, "utf8"), + writeFile: async () => { + throw new Error("non-object servers value must not be overwritten"); + } + }), + /must be a JSON object/ + ); + assert.equal(await fs.readFile(filePath, "utf8"), original); + }); +}); + test("configureMcpTargets preserves existing servers in the config file", async () => { await withTempDir(async (workspace) => { const vscodeDir = path.join(workspace, ".vscode"); From 62c8753e87aaa9717be47167a93d0d2403180bb5 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:21:00 -0700 Subject: [PATCH 11/25] fix: do not count batch comments as applied operations patchloom batch skips blank lines and lines whose first non-whitespace character is #. Batch Apply counted those lines, so a comment-only plan still reported that operations were applied, and a commented plan over-counted. The count now matches the CLI, and a comment-only plan is refused before the command runs. npm run check: 458 passed, line coverage 92.55%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- README.md | 2 +- src/commands/batchApply.ts | 9 +++++++-- test/unit/batchApply.test.ts | 14 ++++++++++++++ 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dcaadec..fac5ef6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,7 @@ src/ workspace/readiness.ts Workspace readiness: environment detection, folder selection test/ unit/ Unit tests (node:test, dependency-injected, no VS Code API) - batchApply.test.ts Batch template and operation count parsing (21 tests) + batchApply.test.ts Batch template and operation count parsing (22 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (71 tests) diff --git a/README.md b/README.md index 55b4e82..7b9d3d6 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay insid ### Batch operations -`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --apply` so all changes land atomically. +`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --apply` so all changes land atomically. A line whose first non-whitespace character is `#` is a comment. Blank lines and comments are not operations. A plan that contains only those lines is not applied, and the completion message counts only real operations. ### Output channel diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index 80bbe9b..27bbc58 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -36,10 +36,15 @@ export function buildBatchTemplate(): string { } export const BATCH_APPLY_PROMPT = - "Edit the batch plan, then click Apply to execute all operations atomically. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; + "Edit the batch plan, then click Apply to execute all operations atomically. Lines whose first non-whitespace character is # are comments and are not applied. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; +/** Count operations `patchloom batch` will run. A leading BOM, blank lines, and `#` comments are ignored. */ export function parseBatchOperationCount(plan: string): number { - return plan.split("\n").filter((line) => line.trim().length > 0).length; + const body = plan.charCodeAt(0) === 0xfeff ? plan.slice(1) : plan; + return body.split("\n").filter((line) => { + const trimmed = line.trim(); + return trimmed.length > 0 && !trimmed.startsWith("#"); + }).length; } /** True when the plan has no non-empty operation lines. */ diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index f717ff3..53751fb 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -54,6 +54,18 @@ test("parseBatchOperationCount ignores blank lines between operations", () => { assert.equal(parseBatchOperationCount(plan), 2); }); +test("parseBatchOperationCount ignores # comments the CLI skips", () => { + const plan = [ + "# note", + " # indented", + 'replace a.txt "x" "y"', + "" + ].join("\n"); + assert.equal(parseBatchOperationCount(plan), 1); + assert.equal(parseBatchOperationCount("\uFEFF# only a comment\n"), 0); + assert.equal(parseBatchOperationCount("\uFEFFtidy.fix src/main.ts\n"), 1); +}); + test("parseBatchOperationCount counts a single operation", () => { assert.equal(parseBatchOperationCount('tidy.fix src/main.ts'), 1); }); @@ -61,6 +73,7 @@ test("parseBatchOperationCount counts a single operation", () => { test("isEmptyBatchPlan is true for empty and whitespace-only plans", () => { assert.equal(isEmptyBatchPlan(""), true); assert.equal(isEmptyBatchPlan(" \n \n"), true); + assert.equal(isEmptyBatchPlan("# comment only\n # still a comment\n"), true); }); test("isEmptyBatchPlan is false when at least one operation is present", () => { @@ -175,4 +188,5 @@ test("buildBatchApplyArgs prefixes global --contain before batch --apply", () => test("BATCH_APPLY_PROMPT names dotted doc.update and doc.delete_where shapes", () => { assert.match(BATCH_APPLY_PROMPT, /doc\.update PATH SELECTOR VALUE/); assert.match(BATCH_APPLY_PROMPT, /doc\.delete_where PATH SELECTOR PREDICATE/); + assert.match(BATCH_APPLY_PROMPT, /first non-whitespace character is #/); }); From e310959cf2aab28d13f718c2c63aaccda94bae68 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:24:09 -0700 Subject: [PATCH 12/25] docs: say patch apply keeps workspace containment The README said patch apply and patch merge skip containment when the patch file is outside the workspace. Those commands copy that file into the workspace and still pass --contain. npm run check: 458 passed, line coverage 92.57%. Signed-off-by: Sebastien Tardif --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 7b9d3d6..161d817 100644 --- a/README.md +++ b/README.md @@ -120,7 +120,7 @@ Click it to see full diagnostics, including per-file MCP configuration status (V | **Merge patch (three-way)** | Apply a stale patch using three-way merge (v0.2.0+) | | **Undo last change** | Restore files from the latest Patchloom backup session | -Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay inside the workspace root (CLI 0.10+). Containment is relative to the effective working directory (the workspace folder). Patch apply and patch merge skip containment when the patch file may live outside the workspace. +Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay inside the workspace root (CLI 0.10+). Containment is relative to the effective working directory (the workspace folder). Patch apply and patch merge copy a patch file that lives outside the workspace into a temporary directory inside it, then still pass `--contain`. A path inside that patch that leaves the workspace is rejected. ### Batch operations From 7fe30937bb3ce2d5cdddb73ae454f3f23bfc9d56 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:31:27 -0700 Subject: [PATCH 13/25] fix: report a missing markdown heading instead of no changes Quick Action preview treated exit 3 as "No changes to preview" for every command except doc update and delete-where. patchloom 0.37 uses exit 3 when an md section command cannot find the heading, so a typo looked like a no-op. Those previews now show the CLI error. npm run check: 459 passed, line coverage 92.56%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- README.md | 2 +- src/commands/quickActions.ts | 7 ++++++- test/unit/quickActions.test.ts | 15 +++++++++++++++ 4 files changed, 23 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fac5ef6..0230089 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (95 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (98 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/README.md b/README.md index 161d817..ec1f098 100644 --- a/README.md +++ b/README.md @@ -276,7 +276,7 @@ On CLI 0.22+, over-wide fuzzy matches can report `error_kind: fuzzy_span_suspici On CLI 0.27+, `doc set` / `doc ensure` / `doc delete` with a predicate or wildcard selector stay `error_kind: invalid_input` and may include `suggested_op`. Standalone CLI is `doc update` / `doc delete-where`; batch plans use `doc.update` / `doc.delete_where`. The extension surfaces that hint in notifications; full CLI text for search, undo, patch-merge, and batch apply is in the Output channel. Use the multi-match op (or a concrete index path such as `items.0.val`). The matching Quick Actions are **Update matching structured values** and **Delete matching array items**. **Ambiguous markdown heading** -On CLI 0.25+, section ops that match the same heading more than once report `error_kind: ambiguous`. Make the heading unique or use a level-qualified query (for example `## Rules`). +On CLI 0.25+, section ops that match the same heading more than once report `error_kind: ambiguous`. Make the heading unique or use a level-qualified query (for example `## Rules`). A heading that is absent is reported as not found, and the Quick Action preview shows that error. **MCP config not injected** Run `Patchloom: Configure MCP` and select the target editor config. diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index d24f9a0..34b7c17 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -1720,7 +1720,12 @@ export function isAllowedPreviewMiss(action: PlannedQuickAction, exitCode: numbe return false; } // PlannedQuickAction.args are operands only. executePatchloom adds --contain. - // doc update and delete-where: exit 3 is a path miss (key not found), not a soft no-op. + // Exit 3 is a soft no-op for text replace (pattern not in the file). It is a + // real miss for doc update / delete-where (selector matched nothing) and for + // markdown section ops (heading not found). Those must surface the CLI error. + if (action.args[0] === "md") { + return false; + } return !( action.args[0] === "doc" && (action.args[1] === "update" || action.args[1] === "delete-where") diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 09df3af..72d0346 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -212,6 +212,21 @@ test("isAllowedPreviewMiss allows doc set exit 3", () => { assert.equal(isAllowedPreviewMiss(action, 3), true); }); +test("isAllowedPreviewMiss rejects markdown heading miss exit 3", () => { + const actions = [ + buildMdReplaceSectionQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "x"), + buildMdTableAppendQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "| a | b |"), + buildMdUpsertBulletQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "x"), + buildMdInsertAfterHeadingQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "x"), + buildMdInsertAfterSectionQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "## FAQ"), + buildMdInsertBeforeHeadingQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "x") + ]; + for (const action of actions) { + assert.equal(action.args[0], "md"); + assert.equal(isAllowedPreviewMiss(action, 3), false); + } +}); + test("isAllowedPreviewMiss rejects exit 0", () => { const action = buildReplaceQuickAction("/workspace/demo/README.md", "old", "new"); assert.equal(isAllowedPreviewMiss(action, 0), false); From 8308b128b9b5f76aba6d4e703d715e2ff8b50453 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:35:27 -0700 Subject: [PATCH 14/25] fix: stop offering predicate selectors on doc set Set structured value asked for servers[port>8000]. patchloom 0.37 rejects that selector on doc set (exit 1, file unchanged) and accepts it on doc update and doc get. The set box now asks for a concrete path. The compare example stays on Update matching structured values and on Read structured value. npm run check: 460 passed, line coverage 92.57%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/quickActions.ts | 18 ++++++++++++++---- test/unit/quickActions.test.ts | 13 +++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 0230089..1aa8eea 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (98 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (99 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 34b7c17..0e92fd3 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -43,6 +43,16 @@ const MARKDOWN_FILE_EXTENSIONS = new Set([".md", ".markdown", ".mdx"]); export const STRUCTURED_DOCUMENT_LABEL = "JSON, JSONC, YAML, TOML, .env, INI, or .properties"; +/** `doc set` accepts a concrete path. A predicate is `doc update`. */ +export const DOC_SET_SELECTOR_PROMPT = + "Selector path. Use a concrete path such as scripts.test. A predicate such as servers[port>8000] goes to Update matching structured values."; +export const DOC_SET_SELECTOR_PLACEHOLDER = "scripts.test"; + +/** `doc update` is the multi-match write, including numeric compares. */ +export const DOC_UPDATE_SELECTOR_PROMPT = + "Selector path (wildcards, predicates, and numeric compares such as items[name=foo].v or servers[port>8000])"; +export const DOC_UPDATE_SELECTOR_PLACEHOLDER = "items[*].enabled or servers[port>8000]"; + export type TidyFix = "ensure-final-newline" | "trim-trailing-whitespace" | "normalize-eol-lf"; export interface PlannedQuickAction { @@ -371,8 +381,8 @@ export async function runQuickAction(): Promise { } const selector = await vscode.window.showInputBox({ - prompt: "Selector path (CLI 0.30+ accepts numeric compares such as servers[port>8000])", - placeHolder: "scripts.test or servers[port>8000]", + prompt: DOC_SET_SELECTOR_PROMPT, + placeHolder: DOC_SET_SELECTOR_PLACEHOLDER, validateInput: (value) => value.length > 0 ? undefined : "Selector is required." }); if (selector === undefined) { @@ -403,8 +413,8 @@ export async function runQuickAction(): Promise { } const selector = await vscode.window.showInputBox({ - prompt: "Selector path (wildcards and predicates such as items[*].enabled)", - placeHolder: "items[*].enabled or items[name=foo].v", + prompt: DOC_UPDATE_SELECTOR_PROMPT, + placeHolder: DOC_UPDATE_SELECTOR_PLACEHOLDER, validateInput: (value) => value.length > 0 ? undefined : "Selector is required." }); if (selector === undefined) { diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 72d0346..af79c3b 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -36,6 +36,10 @@ import { buildUndoQuickAction, isAllowedPreviewMiss, previewMissMessage, + DOC_SET_SELECTOR_PROMPT, + DOC_SET_SELECTOR_PLACEHOLDER, + DOC_UPDATE_SELECTOR_PROMPT, + DOC_UPDATE_SELECTOR_PLACEHOLDER, isMarkdownPath, isEnvDocumentPath, isStructuredDocumentPath, @@ -212,6 +216,15 @@ test("isAllowedPreviewMiss allows doc set exit 3", () => { assert.equal(isAllowedPreviewMiss(action, 3), true); }); +test("doc set selector copy sends predicates to doc update", () => { + assert.equal(DOC_SET_SELECTOR_PLACEHOLDER, "scripts.test"); + assert.match(DOC_SET_SELECTOR_PROMPT, /concrete path/); + assert.match(DOC_SET_SELECTOR_PROMPT, /Update matching structured values/); + assert.doesNotMatch(DOC_SET_SELECTOR_PROMPT, /accepts numeric compares/); + assert.match(DOC_UPDATE_SELECTOR_PROMPT, /servers\[port>8000\]/); + assert.match(DOC_UPDATE_SELECTOR_PLACEHOLDER, /servers\[port>8000\]/); +}); + test("isAllowedPreviewMiss rejects markdown heading miss exit 3", () => { const actions = [ buildMdReplaceSectionQuickAction("/workspace/demo/CHANGELOG.md", "## Missing", "x"), From 1586d4255486009bee8cd65c28cc66eb6a692adf Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:39:40 -0700 Subject: [PATCH 15/25] fix: report a missing structured key instead of no changes Quick Action preview treated exit 3 as "No changes to preview" for doc append, prepend, merge, and move. patchloom 0.37 uses that exit when the key is missing, so a bad path looked like a no-op. Those previews now show the CLI error. Delete of an absent key still exits 0, and text replace still treats exit 3 as no matches. npm run check: 461 passed, line coverage 92.58%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/quickActions.ts | 20 ++++++++++++++------ test/unit/quickActions.test.ts | 12 ++++++++++++ 3 files changed, 27 insertions(+), 7 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 1aa8eea..91d0de7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (99 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (100 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 0e92fd3..81b7fd6 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -1730,16 +1730,24 @@ export function isAllowedPreviewMiss(action: PlannedQuickAction, exitCode: numbe return false; } // PlannedQuickAction.args are operands only. executePatchloom adds --contain. - // Exit 3 is a soft no-op for text replace (pattern not in the file). It is a - // real miss for doc update / delete-where (selector matched nothing) and for - // markdown section ops (heading not found). Those must surface the CLI error. + // Exit 3 is a soft no-op for text replace (pattern not in the file). + // It is a real miss for markdown section ops (heading not found) and for + // doc update, delete-where, append, prepend, merge, and move (missing key). if (action.args[0] === "md") { return false; } - return !( + if ( action.args[0] === "doc" && - (action.args[1] === "update" || action.args[1] === "delete-where") - ); + (action.args[1] === "update" || + action.args[1] === "delete-where" || + action.args[1] === "append" || + action.args[1] === "prepend" || + action.args[1] === "merge" || + action.args[1] === "move") + ) { + return false; + } + return true; } export function previewMissMessage(action: PlannedQuickAction, relativePath: string): string { diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index af79c3b..ccc21b5 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -211,6 +211,18 @@ test("isAllowedPreviewMiss rejects doc delete-where exit 3", () => { assert.equal(isAllowedPreviewMiss(action, 3), false); }); +test("isAllowedPreviewMiss rejects missing-key exit 3 for append, prepend, merge, and move", () => { + const actions = [ + buildDocAppendQuickAction("/workspace/demo/data.json", "missing", "true"), + buildDocPrependQuickAction("/workspace/demo/data.json", "missing", "true"), + buildDocMergeQuickAction("/workspace/demo/data.json", "{\"a\":1}", "missing"), + buildDocMoveQuickAction("/workspace/demo/data.json", "missing", "dest") + ]; + for (const action of actions) { + assert.equal(isAllowedPreviewMiss(action, 3), false); + } +}); + test("isAllowedPreviewMiss allows doc set exit 3", () => { const action = buildDocSetQuickAction("/workspace/demo/package.json", "scripts.test", "vitest"); assert.equal(isAllowedPreviewMiss(action, 3), true); From 6058c24381a819b3fc7fbf8773bb037632f0aa15 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 12:43:59 -0700 Subject: [PATCH 16/25] fix: stop the batch template from overwriting the anchor line The default Batch Apply plan used replace PATH "anchor_line" --insert-after="...". patchloom 0.37 does not treat --insert-after as a batch flag, so that token becomes the new text and the matched line is replaced. The plan now uses file.prepend, which inserts a line. The other eleven template lines still apply on a matching tree. npm run check: 461 passed, line coverage 92.60%. Signed-off-by: Sebastien Tardif --- README.md | 2 +- src/commands/batchApply.ts | 3 ++- test/unit/batchApply.test.ts | 15 ++++++--------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index ec1f098..ca1faf2 100644 --- a/README.md +++ b/README.md @@ -258,7 +258,7 @@ The extension's MCP server is stdio. If you bind Streamable HTTP yourself (`patc On CLI 0.30+, `patch apply` (and MCP `apply_patch`) accepts unified diffs, Codex `*** Begin Patch`, and Aider SEARCH/REPLACE. Update and SEARCH matches must be unique unless you pass `--replace-all` (SEARCH/REPLACE only). The Quick Action **Apply patch (unified / Begin Patch / SEARCH-REPLACE)** builds `patch apply`. **Merge patch (three-way)** is still `patch merge` for stale unified diffs. **Batch replace shape** -Batch lines use `replace PATH OLD NEW` (and optional flags such as `--fuzzy`). Do not paste CLI form `replace OLD --new NEW path` into a batch plan; CLI 0.18+ returns a clear parse error with the PATH OLD NEW hint. +Batch lines use `replace PATH OLD NEW` (and optional flags such as `--fuzzy`). Do not paste CLI form `replace OLD --new NEW path` into a batch plan; CLI 0.18+ returns a clear parse error with the PATH OLD NEW hint. `--insert-after` is not a batch flag. The Quick Action **Insert text after match** uses it. In a batch plan that token is the new text, so the matched line is replaced. Prepend with `file.prepend PATH "text"`. **Batch doc.update / delete_where shape** Batch lines use `doc.update PATH SELECTOR VALUE` and `doc.delete_where PATH SELECTOR PREDICATE`. These are dotted batch ops (path, then selector, then value or predicate). diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index 27bbc58..1b85051 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -13,12 +13,13 @@ import { activeWorkspaceFolder } from "../workspace/readiness.js"; import { serializePatchloomArgs } from "./quickActions.js"; // Batch replace is PATH OLD NEW (not CLI `replace OLD --new NEW path`). See CLI 0.18+ batch --help. +// --insert-after is not a batch flag. The CLI stores that token as the new text. // doc.update / doc.delete_where are the multi-match siblings of doc.set / doc.delete // (CLI 0.27+ suggested_op hints this). export const BATCH_TEMPLATE = [ "replace src/example.ts \"old text\" \"new text\"", "replace src/example.ts \"typo_here\" \"fixed\" --fuzzy --min-fuzzy-score 0.80", - "replace src/example.ts \"anchor_line\" --insert-after=\"new sibling line\"", + "file.prepend src/example.ts \"header line\"", "doc.set package.json version \"2.0.0\"", "doc.set tsconfig.jsonc compilerOptions.strict true", "doc.update data.json \"items[*].enabled\" true", diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index 53751fb..054b4bd 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -15,7 +15,8 @@ test("buildBatchTemplate returns line-oriented format with twelve operations", ( assert.equal(lines.length, 12); assert.ok(lines[0].startsWith("replace "), "first line should be a replace operation"); assert.ok(lines[1].startsWith("replace ") && lines[1].includes("--fuzzy"), "second line should be fuzzy replace"); - assert.ok(lines[2].startsWith("replace ") && lines[2].includes("--insert-after"), "third line should be insert-after"); + assert.ok(lines[2].startsWith("file.prepend "), "third line should be file.prepend"); + assert.equal(lines.some((line) => line.includes("--insert-after")), false); assert.ok(lines[3].startsWith("doc.set "), "fourth line should be a doc.set operation"); assert.ok(lines[4].startsWith("doc.set ") && lines[4].includes("tsconfig.jsonc"), "fifth line should be JSONC doc.set"); assert.ok(lines[5].startsWith("doc.update "), "sixth line should be multi-match doc.update"); @@ -137,15 +138,11 @@ test("buildBatchTemplate doc.merge line uses path selector value (CLI 0.16 multi assert.match(mergeLine, /\s0\s/, "example should merge into document 0"); }); -test("buildBatchTemplate includes replace --insert-after example (CLI 0.16)", () => { +test("buildBatchTemplate uses file.prepend instead of a batch --insert-after token", () => { const lines = buildBatchTemplate().split("\n"); - const insertLine = lines.find((l) => l.includes("--insert-after")); - assert.ok(insertLine, "template should contain an insert-after example"); - assert.match( - insertLine, - /replace \S+ ".+" --insert-after=/, - "insert-after should use batch flag form path pattern --insert-after=payload" - ); + const prependLine = lines.find((line) => line.startsWith("file.prepend ")); + assert.equal(prependLine, "file.prepend src/example.ts \"header line\""); + assert.equal(lines.some((line) => line.includes("--insert-after")), false); }); test("buildBatchTemplate includes JSONC doc.set and directory file.rename (CLI 0.35+)", () => { From 30f6dd1d43861542d3475cf55120e87ac197aba2 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:06:09 -0700 Subject: [PATCH 17/25] fix: install the Windows release zip's root binary The 0.37.0 Windows zip extracts patchloom.exe at the archive root. Unix archives still nest the binary under patchloom-/. Managed install only looked in the nested directory, so Windows install tried to promote a binary that was never moved. Look in the nested path, then the archive root. When neither copy is present, report an extract failure instead of a replace failure. npm run check: 463 passed, line coverage 92.67%, vsce package succeeded. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/install/managed.ts | 31 +++++++---- test/unit/managedLifecycle.test.ts | 84 +++++++++++++++++++++++++++--- 3 files changed, 99 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 91d0de7..9a4eaf0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,7 +54,7 @@ test/ binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (71 tests) - managedLifecycle.test.ts Managed install with real file I/O (26 tests) + managedLifecycle.test.ts Managed install with real file I/O (28 tests) mcpConfig.test.ts MCP config with real temp directories (16 tests) managedInstall.test.ts Managed Update compares latest vs managed binary (10 tests) mcpRegister.test.ts Native MCP definition helper for binary path (6 tests) diff --git a/src/install/managed.ts b/src/install/managed.ts index fab76b8..d4f3631 100644 --- a/src/install/managed.ts +++ b/src/install/managed.ts @@ -662,17 +662,28 @@ export async function performManagedInstall(inputs: PerformManagedInstallInputs) format: target.archiveFormat }); - // cargo-dist archives extract to /patchloom, but promotion - // expects the binary at managed-bin/patchloom. Move it into place. - const extractedBinaryPath = path.join( - txPaths.stagingRoot, - `patchloom-${target.targetTriple}`, - managedBinaryName(platform) - ); - if (await defaultFileExists(extractedBinaryPath)) { - await defaultEnsureDir(path.dirname(txPaths.stagedBinaryPath)); - await defaultRenameFile(extractedBinaryPath, txPaths.stagedBinaryPath); + // Unix release archives nest the binary at patchloom-/patchloom. + // The Windows zip from the same release puts patchloom.exe at the archive root. + // Promotion expects managed-bin/patchloom(.exe). + const binaryName = managedBinaryName(platform); + const extractedCandidates = [ + path.join(txPaths.stagingRoot, `patchloom-${target.targetTriple}`, binaryName), + path.join(txPaths.stagingRoot, binaryName) + ]; + let extractedBinaryPath: string | undefined; + for (const candidate of extractedCandidates) { + if (await defaultFileExists(candidate)) { + extractedBinaryPath = candidate; + break; + } + } + if (!extractedBinaryPath) { + throw new Error( + `Extracted archive did not contain ${binaryName}. Looked in patchloom-${target.targetTriple}/${binaryName} and ${binaryName}.` + ); } + await defaultEnsureDir(path.dirname(txPaths.stagedBinaryPath)); + await defaultRenameFile(extractedBinaryPath, txPaths.stagedBinaryPath); report("installing"); await promoteManagedInstallBinary({ diff --git a/test/unit/managedLifecycle.test.ts b/test/unit/managedLifecycle.test.ts index 37bcc1b..7762958 100644 --- a/test/unit/managedLifecycle.test.ts +++ b/test/unit/managedLifecycle.test.ts @@ -453,10 +453,10 @@ test("performManagedInstall runs full pipeline with injected I/O", async () => { } }, extractArchive: async (inputs) => { - // Simulate extraction: create the binary in staging - const txPaths = resolveManagedInstallTransactionPaths(installRoot, target); - await fs.mkdir(path.dirname(txPaths.stagedBinaryPath), { recursive: true }); - await fs.writeFile(txPaths.stagedBinaryPath, "#!/bin/sh\necho patchloom 0.1.0\n", { mode: 0o755 }); + // Unix cargo-dist layout: patchloom-/patchloom + const extracted = path.join(inputs.destDir, `patchloom-${target.targetTriple}`, "patchloom"); + await fs.mkdir(path.dirname(extracted), { recursive: true }); + await fs.writeFile(extracted, "#!/bin/sh\necho patchloom 0.1.0\n", { mode: 0o755 }); }, readFileContent: async (filePath) => { return fs.readFile(filePath, "utf8"); @@ -549,9 +549,9 @@ test("performManagedInstall fetches latest version when none specified", async ( } }, extractArchive: async (inputs) => { - const txPaths = resolveManagedInstallTransactionPaths(installRoot, target); - await fs.mkdir(path.dirname(txPaths.stagedBinaryPath), { recursive: true }); - await fs.writeFile(txPaths.stagedBinaryPath, "binary-0.3.0", { mode: 0o755 }); + const extracted = path.join(inputs.destDir, `patchloom-${target.targetTriple}`, "patchloom"); + await fs.mkdir(path.dirname(extracted), { recursive: true }); + await fs.writeFile(extracted, "binary-0.3.0", { mode: 0o755 }); }, readFileContent: async (filePath) => fs.readFile(filePath, "utf8"), failurePersistence: { storageRoot: installRoot } @@ -560,3 +560,73 @@ test("performManagedInstall fetches latest version when none specified", async ( assert.equal(result.version, "0.3.0"); }); }); + +test("performManagedInstall promotes patchloom.exe from a flat Windows zip", async () => { + await withTempDir(async (installRoot) => { + const target = detectManagedInstallTarget("win32", "x64"); + assert.ok(target); + // SHA-256 of "fake-archive-content" + const checksumContent = "fd3d4b42292957ad0b649621615962140c857fbf7342038d6cc6b2b1ab8c3411 patchloom-x86_64-pc-windows-msvc.zip\n"; + + const result = await performManagedInstall({ + installRoot, + version: "0.37.0", + platform: "win32", + arch: "x64", + downloadFile: async (inputs) => { + await fs.mkdir(path.dirname(inputs.destPath), { recursive: true }); + if (inputs.url.endsWith(".sha256")) { + await fs.writeFile(inputs.destPath, checksumContent, "utf8"); + } else { + await fs.writeFile(inputs.destPath, "fake-archive-content", "utf8"); + } + }, + extractArchive: async (inputs) => { + // patchloom 0.37.0 Windows zip: patchloom.exe at the archive root. + await fs.writeFile(path.join(inputs.destDir, "patchloom.exe"), "windows-binary", "utf8"); + }, + readFileContent: async (filePath) => fs.readFile(filePath, "utf8"), + failurePersistence: { storageRoot: installRoot } + }); + + assert.equal(result.version, "0.37.0"); + assert.equal(result.target.targetTriple, "x86_64-pc-windows-msvc"); + assert.ok(result.binaryPath.endsWith(`${path.sep}patchloom.exe`)); + assert.equal(await fs.readFile(result.binaryPath, "utf8"), "windows-binary"); + }); +}); + +test("performManagedInstall reports a missing extracted binary", async () => { + await withTempDir(async (installRoot) => { + clearManagedInstallFailure(); + const checksumContent = "fd3d4b42292957ad0b649621615962140c857fbf7342038d6cc6b2b1ab8c3411 patchloom-x86_64-pc-windows-msvc.zip\n"; + + await assert.rejects( + () => performManagedInstall({ + installRoot, + version: "0.37.0", + platform: "win32", + arch: "x64", + downloadFile: async (inputs) => { + await fs.mkdir(path.dirname(inputs.destPath), { recursive: true }); + if (inputs.url.endsWith(".sha256")) { + await fs.writeFile(inputs.destPath, checksumContent, "utf8"); + } else { + await fs.writeFile(inputs.destPath, "fake-archive-content", "utf8"); + } + }, + extractArchive: async (inputs) => { + await fs.writeFile(path.join(inputs.destDir, "README.md"), "no binary", "utf8"); + }, + readFileContent: async (filePath) => fs.readFile(filePath, "utf8"), + failurePersistence: { storageRoot: installRoot } + }), + /did not contain patchloom\.exe/ + ); + + const failure = await loadManagedInstallFailure({ storageRoot: installRoot }); + assert.ok(failure); + assert.equal(failure.stage, "extract"); + assert.equal(failure.reason, "extract-failed"); + }); +}); From da42c0ca04004ee34d1c16cc44ad463bbb97c927 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:14:09 -0700 Subject: [PATCH 18/25] fix: name MCP targets already saved when configure stops Configure MCP wrote earlier config files, then a later target whose server map was not an object threw. The toast only said the command failed, so a saved VS Code config looked unchanged. The error now keeps the completed results, and the message says how many targets were updated before the stop. npm run check: 464 passed, line coverage 92.75%, vsce package. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/configureMcp.ts | 16 +++++-- src/mcp/config.ts | 89 ++++++++++++++++++++++++------------ test/unit/mcpConfig.test.ts | 40 ++++++++++++++++ 4 files changed, 112 insertions(+), 35 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9a4eaf0..7d5a2a4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,7 +55,7 @@ test/ binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (71 tests) managedLifecycle.test.ts Managed install with real file I/O (28 tests) - mcpConfig.test.ts MCP config with real temp directories (16 tests) + mcpConfig.test.ts MCP config with real temp directories (23 tests) managedInstall.test.ts Managed Update compares latest vs managed binary (10 tests) mcpRegister.test.ts Native MCP definition helper for binary path (6 tests) statusRefresh.test.ts Status and MCP refresh order after input change (1 test) diff --git a/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index 4787ff8..3afef7c 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -3,9 +3,14 @@ import * as path from "node:path"; import * as vscode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; import { getPatchloomLog, writeUserVisibleOutput } from "../logging/outputChannel.js"; -import { configureMcpTargets, inspectMcpTargets, readMcpConfigText } from "../mcp/config.js"; +import { + configureMcpTargets, + formatMcpConfigureFailureMessage, + inspectMcpTargets, + McpConfigureError, + readMcpConfigText +} from "../mcp/config.js"; import { refreshStatusBar } from "../status/statusBar.js"; -import { formatError } from "../util.js"; import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; export async function configureMcp(): Promise { @@ -80,11 +85,14 @@ export async function configureMcp(): Promise { } }); } catch (error) { - const message = formatError(error); + const message = formatMcpConfigureFailureMessage(error); const log = getPatchloomLog(); writeUserVisibleOutput(log, message); log?.show(); - await vscode.window.showErrorMessage(`Failed to configure MCP: ${message}`); + await vscode.window.showErrorMessage(message); + if (error instanceof McpConfigureError && error.completed.some((result) => result.changed)) { + await refreshStatusBar(); + } return; } diff --git a/src/mcp/config.ts b/src/mcp/config.ts index 8228ca7..a37ebd0 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -26,6 +26,27 @@ export interface McpTargetResult extends McpTargetStatus { readonly changed: boolean; } +/** A configure stop after zero or more targets were already written. */ +export class McpConfigureError extends Error { + readonly completed: readonly McpTargetResult[]; + + constructor(message: string, completed: readonly McpTargetResult[]) { + super(message); + this.name = "McpConfigureError"; + this.completed = completed; + } +} + +/** Toast text when configure stops. Names targets that were already saved. */ +export function formatMcpConfigureFailureMessage(error: unknown): string { + const message = error instanceof Error && error.message.length > 0 ? error.message : String(error); + const changed = error instanceof McpConfigureError + ? error.completed.filter((result) => result.changed).length + : 0; + const saved = changed > 0 ? `Updated ${changed} MCP config target(s). ` : ""; + return `${saved}Failed to configure MCP: ${message}`; +} + export interface McpInspectionInputs { readonly workspaceFolderPath?: string; readonly homeDir?: string; @@ -90,41 +111,49 @@ export async function configureMcpTargets(inputs: McpApplyInputs): Promise 0; - - if (hasText && isPlainObject(existingRoot) && stableJson(currentEntry) === stableJson(entry)) { + try { + assertMcpConfigWriteContained(target, inputs.workspaceFolderPath); + const content = await readFile(target.filePath); + const original = parseJsonObject(content, target.filePath); + const entry = entryForKind(target.kind, patchloomCommand, mcpSurface); + const key = usesMcpServersKey(target.kind) ? "mcpServers" : "servers"; + const existingRoot = original[key]; + const currentEntry = isPlainObject(existingRoot) ? existingRoot.patchloom : undefined; + const hasText = typeof content === "string" && content.trim().length > 0; + + if (hasText && isPlainObject(existingRoot) && stableJson(currentEntry) === stableJson(entry)) { + results.push({ + ...target, + exists: true, + configured: true, + changed: false + }); + continue; + } + + if (hasText && existingRoot !== undefined && !isPlainObject(existingRoot)) { + throw new Error(`Cannot update MCP config ${target.filePath}: "${key}" must be a JSON object`); + } + + // JSON.stringify of the parse drops comments and trailing commas. + const serialized = typeof content === "string" && content.trim().length > 0 + ? applyPatchloomEntry(content, key, entry) + : `${JSON.stringify(withPatchloomEntry(target.kind, original, patchloomCommand, mcpSurface), null, 2)}\n`; + await inputs.writeFile(target.filePath, serialized); + results.push({ ...target, - exists: true, + exists: content !== undefined, configured: true, - changed: false + changed: true }); - continue; - } - - if (hasText && existingRoot !== undefined && !isPlainObject(existingRoot)) { - throw new Error(`Cannot update MCP config ${target.filePath}: "${key}" must be a JSON object`); + } catch (error) { + if (error instanceof McpConfigureError) { + throw error; + } + const message = error instanceof Error && error.message.length > 0 ? error.message : String(error); + throw new McpConfigureError(message, results); } - - // JSON.stringify of the parse drops comments and trailing commas. - const serialized = typeof content === "string" && content.trim().length > 0 - ? applyPatchloomEntry(content, key, entry) - : `${JSON.stringify(withPatchloomEntry(target.kind, original, patchloomCommand, mcpSurface), null, 2)}\n`; - await inputs.writeFile(target.filePath, serialized); - - results.push({ - ...target, - exists: content !== undefined, - configured: true, - changed: true - }); } return results; diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index 192e096..08348fd 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -7,7 +7,9 @@ import { parse as parseJsonc } from "jsonc-parser"; import { buildPatchloomMcpEntry, configureMcpTargets, + formatMcpConfigureFailureMessage, inspectMcpTargets, + McpConfigureError, resolveMcpTargets } from "../../src/mcp/config.js"; @@ -187,6 +189,44 @@ test("configureMcpTargets refuses a non-object servers value", async () => { }); }); +test("configureMcpTargets keeps earlier target writes when a later target is not an object", async () => { + await withTempDir(async (workspace) => { + const vscodeDir = path.join(workspace, ".vscode"); + await fs.mkdir(vscodeDir, { recursive: true }); + await fs.writeFile(path.join(vscodeDir, "mcp.json"), "{}\n", "utf8"); + const portablePath = path.join(workspace, ".mcp.json"); + const portableOriginal = `{ "mcpServers": [] }\n`; + await fs.writeFile(portablePath, portableOriginal, "utf8"); + + await assert.rejects( + () => configureMcpTargets({ + workspaceFolderPath: workspace, + homeDir: workspace, + includeKinds: ["vscode-workspace", "portable-workspace"], + patchloomPathSetting: "patchloom", + readFile: async (targetPath) => fs.readFile(targetPath, "utf8"), + writeFile: async (targetPath, content) => { + await fs.writeFile(targetPath, content, "utf8"); + } + }), + (error: unknown) => { + assert.ok(error instanceof McpConfigureError); + assert.equal(error.completed.length, 1); + assert.equal(error.completed[0]?.kind, "vscode-workspace"); + assert.equal(error.completed[0]?.changed, true); + assert.match(formatMcpConfigureFailureMessage(error), /^Updated 1 MCP config target\(s\)\. Failed to configure MCP: Cannot update MCP config /); + assert.match(formatMcpConfigureFailureMessage(error), /"mcpServers" must be a JSON object$/); + return true; + } + ); + + const vscodeConfig = await readJson(path.join(vscodeDir, "mcp.json")); + const servers = vscodeConfig.servers as Record; + assert.equal(servers.patchloom?.command, "patchloom"); + assert.equal(await fs.readFile(portablePath, "utf8"), portableOriginal); + }); +}); + test("configureMcpTargets preserves existing servers in the config file", async () => { await withTempDir(async (workspace) => { const vscodeDir = path.join(workspace, ".vscode"); From b062a7e926fa7bba669a40db0794b50536bccbcc Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:23:59 -0700 Subject: [PATCH 19/25] fix: report batch replaces that matched nothing Batch Apply counted every plan line as applied when patchloom exited 0. CLI 0.37 keeps earlier writes when another replace matches nothing and records that on the JSON refused list, or as match_count 0 when the miss is on a file the plan also wrote. The command now requests --json and the toast names those refused operations. Live probe: doc.set plus a missing replace exited 0 and the message named b.txt (no matches). npm run check: 468 passed, line coverage 92.79%, vsce package. Signed-off-by: Sebastien Tardif --- AGENTS.md | 4 +- README.md | 4 +- src/commands/batchApply.ts | 135 +++++++++++++++++++++++++++++++++-- test/unit/batchApply.test.ts | 59 ++++++++++++++- 4 files changed, 190 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 7d5a2a4..a5aba53 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,7 @@ src/ initializeProject.ts Initialize Project command: generate/diff AGENTS.md managedInstall.ts Managed Install commands: install, update, reinstall Patchloom binary quickActions.ts Quick Action command: replace, tidy, doc set, search, create, append, prepend, doc get, doc keys, doc len, patch merge - batchApply.ts Batch Apply command: atomic multi-operation plan (line-oriented, not JSON) + batchApply.ts Batch Apply command: line-oriented plan; hard errors roll back; refused replaces are reported setupWorkspace.ts Setup Workspace command: guided readiness walkthrough showStatus.ts Show Status command: diagnostics display verifyMcp.ts Verify MCP Server command: JSON-RPC initialize probe @@ -50,7 +50,7 @@ src/ workspace/readiness.ts Workspace readiness: environment detection, folder selection test/ unit/ Unit tests (node:test, dependency-injected, no VS Code API) - batchApply.test.ts Batch template and operation count parsing (22 tests) + batchApply.test.ts Batch template and operation count parsing (26 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (71 tests) diff --git a/README.md b/README.md index ca1faf2..f1ab8d6 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay insid ### Batch operations -`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --apply` so all changes land atomically. A line whose first non-whitespace character is `#` is a comment. Blank lines and comments are not operations. A plan that contains only those lines is not applied, and the completion message counts only real operations. +`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --json --apply`. A hard error rolls the plan back. A replace that matches nothing is refused, other successful writes stay, and the completion message names those refused paths. A line whose first non-whitespace character is `#` is a comment. Blank lines and comments are not operations. A plan that contains only those lines is not applied. ### Output channel @@ -144,7 +144,7 @@ The extension detects outdated CLI builds and warns with upgrade guidance. It re | `Patchloom: Initialize Project` | Generate or diff `AGENTS.md` from `patchloom agent-rules` (mode, platform, surface full/core) | | `Patchloom: Configure MCP` | Inject Patchloom MCP server config (full or core tool surface) into editor config files | | `Patchloom: Quick Action` | Build a Patchloom CLI command from an interactive picker | -| `Patchloom: Batch Apply` | Open a batch plan and execute all operations atomically | +| `Patchloom: Batch Apply` | Open a batch plan, apply it, and report refused operations | | `Patchloom: Show Output` | Open the Patchloom output channel for CLI logs and diagnostics | | `Patchloom: Show Status` | Display binary readiness, version, compatibility, and workspace state | | `Patchloom: Verify MCP Server` | Spawn the MCP server and verify it responds to a JSON-RPC initialize request | diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index 1b85051..df84736 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -37,7 +37,7 @@ export function buildBatchTemplate(): string { } export const BATCH_APPLY_PROMPT = - "Edit the batch plan, then click Apply to execute all operations atomically. Lines whose first non-whitespace character is # are comments and are not applied. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; + "Edit the batch plan, then click Apply. A hard error rolls the plan back. A replace that matches nothing stays refused and does not undo other writes. Lines whose first non-whitespace character is # are comments and are not applied. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; /** Count operations `patchloom batch` will run. A leading BOM, blank lines, and `#` comments are ignored. */ export function parseBatchOperationCount(plan: string): number { @@ -53,9 +53,129 @@ export function isEmptyBatchPlan(plan: string): boolean { return parseBatchOperationCount(plan) === 0; } -/** CLI argv for Batch Apply. Flags come from serializePatchloomArgs, not from scanning operands. */ +/** CLI argv for Batch Apply. `--json` is how a refused replace is visible on exit 0. */ export function buildBatchApplyArgs(): string[] { - return serializePatchloomArgs({ args: ["batch"], apply: true, contain: true }); + return serializePatchloomArgs({ args: ["batch", "--json"], apply: true, contain: true }); +} + +export interface BatchRefusedOperation { + readonly path: string; + readonly reason: string; +} + +export interface BatchApplyReport { + readonly filesChanged: number; + readonly filesCreated: number; + readonly filesDeleted: number; + readonly matchCount: number | undefined; + readonly refused: readonly BatchRefusedOperation[]; +} + +/** Parse `patchloom batch --json` stdout. Undefined when the CLI did not return that object. */ +export function parseBatchApplyReport(stdout: string): BatchApplyReport | undefined { + const trimmed = stdout.trim(); + if (!trimmed.startsWith("{")) { + return undefined; + } + try { + const parsed = JSON.parse(trimmed) as { + files_changed?: unknown; + files_created?: unknown; + files_deleted?: unknown; + match_count?: unknown; + refused?: unknown; + }; + if (typeof parsed.files_changed !== "number") { + return undefined; + } + const refused: BatchRefusedOperation[] = []; + if (Array.isArray(parsed.refused)) { + for (const item of parsed.refused) { + if (!item || typeof item !== "object") { + continue; + } + const record = item as { path?: unknown; reason?: unknown }; + if (typeof record.path !== "string" || record.path.length === 0) { + continue; + } + refused.push({ + path: record.path, + reason: typeof record.reason === "string" && record.reason.length > 0 ? record.reason : "refused" + }); + } + } + return { + filesChanged: parsed.files_changed, + filesCreated: typeof parsed.files_created === "number" ? parsed.files_created : 0, + filesDeleted: typeof parsed.files_deleted === "number" ? parsed.files_deleted : 0, + matchCount: typeof parsed.match_count === "number" ? parsed.match_count : undefined, + refused + }; + } catch { + return undefined; + } +} + +export interface BatchApplyCompletion { + readonly warning: boolean; + readonly message: string; +} + +/** + * Exit 0 is not "every plan line was applied". CLI 0.37 keeps earlier writes + * when another replace matches nothing, and reports that on `refused` or + * `match_count: 0`. + */ +export function formatBatchApplyCompletion(stdout: string, operationCount: number): BatchApplyCompletion { + const report = parseBatchApplyReport(stdout); + if (!report) { + return { + warning: false, + message: `Batch apply completed: ${operationCount} operation(s) applied.` + }; + } + + const tally = describeBatchFileTally(report); + if (report.refused.length > 0) { + const details = report.refused + .map((item) => `${item.path} (${describeRefusedReason(item.reason)})`) + .join(", "); + return { + warning: true, + message: `Batch apply: ${tally}. ${report.refused.length} operation(s) were not applied: ${details}.` + }; + } + if (report.matchCount === 0) { + return { + warning: true, + message: `Batch apply: ${tally}. A replace in the plan matched nothing.` + }; + } + return { + warning: false, + message: `Batch apply completed: ${tally}.` + }; +} + +function describeBatchFileTally(report: BatchApplyReport): string { + const parts: string[] = []; + if (report.filesChanged > 0) { + parts.push(`${report.filesChanged} file(s) changed`); + } + if (report.filesCreated > 0) { + parts.push(`${report.filesCreated} file(s) created`); + } + if (report.filesDeleted > 0) { + parts.push(`${report.filesDeleted} file(s) deleted`); + } + return parts.length > 0 ? parts.join(", ") : "no files changed"; +} + +function describeRefusedReason(reason: string): string { + if (reason === "no_matches") { + return "no matches"; + } + return reason.replaceAll("_", " "); } export async function batchApply(): Promise { @@ -114,10 +234,13 @@ export async function batchApply(): Promise { } const ops = parseBatchOperationCount(plan); + const completion = formatBatchApplyCompletion(result.stdout, ops); presentCliResultInOutput(log, result); - await vscode.window.showInformationMessage( - `Batch apply completed: ${ops} operation(s) applied.` - ); + if (completion.warning) { + await vscode.window.showWarningMessage(completion.message); + return; + } + await vscode.window.showInformationMessage(completion.message); } interface BatchCommandResult { diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index 054b4bd..81edf90 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -4,6 +4,7 @@ import { BATCH_APPLY_PROMPT, buildBatchApplyArgs, buildBatchTemplate, + formatBatchApplyCompletion, isEmptyBatchPlan, parseBatchOperationCount } from "../../src/commands/batchApply.js"; @@ -178,12 +179,66 @@ test("buildBatchTemplate includes doc.delete_where multi-match example (CLI 0.27 ); }); -test("buildBatchApplyArgs prefixes global --contain before batch --apply", () => { - assert.deepEqual(buildBatchApplyArgs(), ["--contain", "batch", "--apply"]); +test("buildBatchApplyArgs prefixes global --contain before batch --json --apply", () => { + assert.deepEqual(buildBatchApplyArgs(), ["--contain", "batch", "--json", "--apply"]); +}); + +test("formatBatchApplyCompletion names a refused replace instead of claiming every line applied", () => { + const stdout = JSON.stringify({ + ok: true, + status: "success", + applied: true, + files_changed: 1, + files_created: 0, + files_deleted: 0, + refused: [{ path: "b.txt", match_mode: "exact", reason: "no_matches" }] + }); + const completion = formatBatchApplyCompletion(stdout, 2); + assert.equal(completion.warning, true); + assert.equal( + completion.message, + "Batch apply: 1 file(s) changed. 1 operation(s) were not applied: b.txt (no matches)." + ); +}); + +test("formatBatchApplyCompletion warns when a same-file replace reports match_count 0", () => { + const stdout = JSON.stringify({ + ok: true, + status: "success", + applied: true, + files_changed: 1, + files_created: 0, + files_deleted: 0, + match_count: 0 + }); + const completion = formatBatchApplyCompletion(stdout, 2); + assert.equal(completion.warning, true); + assert.match(completion.message, /A replace in the plan matched nothing/); + assert.match(completion.message, /1 file\(s\) changed/); +}); + +test("formatBatchApplyCompletion reports created files without a zero changed count", () => { + const stdout = JSON.stringify({ + ok: true, + files_changed: 0, + files_created: 1, + files_deleted: 0 + }); + const completion = formatBatchApplyCompletion(stdout, 1); + assert.equal(completion.warning, false); + assert.equal(completion.message, "Batch apply completed: 1 file(s) created."); +}); + +test("formatBatchApplyCompletion falls back to the plan count when stdout is not JSON", () => { + const completion = formatBatchApplyCompletion("applied 2 operations\n", 2); + assert.equal(completion.warning, false); + assert.equal(completion.message, "Batch apply completed: 2 operation(s) applied."); }); test("BATCH_APPLY_PROMPT names dotted doc.update and doc.delete_where shapes", () => { assert.match(BATCH_APPLY_PROMPT, /doc\.update PATH SELECTOR VALUE/); assert.match(BATCH_APPLY_PROMPT, /doc\.delete_where PATH SELECTOR PREDICATE/); assert.match(BATCH_APPLY_PROMPT, /first non-whitespace character is #/); + assert.match(BATCH_APPLY_PROMPT, /does not undo other writes/); + assert.equal(BATCH_APPLY_PROMPT.includes("execute all operations atomically"), false); }); From e40472d20b1566daab8df9619e2a36a662fb081c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:27:22 -0700 Subject: [PATCH 20/25] fix: report a batch doc.delete that removed nothing CLI 0.37 exits 0 when doc.delete does not remove a key, including when an earlier doc.set in the same plan already wrote. The JSON records mutations[].changed false and omits refused. The completion message now names that operation. Live probe: doc.set plus doc.delete of a missing key exited 0 and the message named d.json (doc.delete changed nothing). npm run check: 470 passed, line coverage 92.82%, vsce package. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- README.md | 2 +- src/commands/batchApply.ts | 48 +++++++++++++++++++++++++++++------- test/unit/batchApply.test.ts | 31 +++++++++++++++++++++++ 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a5aba53..822db37 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,7 @@ src/ workspace/readiness.ts Workspace readiness: environment detection, folder selection test/ unit/ Unit tests (node:test, dependency-injected, no VS Code API) - batchApply.test.ts Batch template and operation count parsing (26 tests) + batchApply.test.ts Batch template and operation count parsing (28 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (71 tests) diff --git a/README.md b/README.md index f1ab8d6..035d6a5 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay insid ### Batch operations -`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --json --apply`. A hard error rolls the plan back. A replace that matches nothing is refused, other successful writes stay, and the completion message names those refused paths. A line whose first non-whitespace character is `#` is a comment. Blank lines and comments are not operations. A plan that contains only those lines is not applied. +`Patchloom: Batch Apply` opens a line-oriented plan template where you can compose multiple operations (replace, fuzzy replace, `doc.set`, JSONC `doc.set`, multi-match `doc.update`, `doc.delete_where`, multi-doc `doc.merge`, file append, directory `file.rename`, markdown section inserts, tidy). The extension pipes the plan to `patchloom batch --json --apply`. A hard error rolls the plan back. A replace that matches nothing, or a structured delete that removes nothing, is reported, other successful writes stay, and the completion message names those operations. A line whose first non-whitespace character is `#` is a comment. Blank lines and comments are not operations. A plan that contains only those lines is not applied. ### Output channel diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index df84736..0d613aa 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -37,7 +37,7 @@ export function buildBatchTemplate(): string { } export const BATCH_APPLY_PROMPT = - "Edit the batch plan, then click Apply. A hard error rolls the plan back. A replace that matches nothing stays refused and does not undo other writes. Lines whose first non-whitespace character is # are comments and are not applied. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; + "Edit the batch plan, then click Apply. A hard error rolls the plan back. A replace that matches nothing, or a structured delete that removes nothing, is reported and does not undo other writes. Lines whose first non-whitespace character is # are comments and are not applied. Multi-match lines use dotted batch ops: doc.update PATH SELECTOR VALUE and doc.delete_where PATH SELECTOR PREDICATE."; /** Count operations `patchloom batch` will run. A leading BOM, blank lines, and `#` comments are ignored. */ export function parseBatchOperationCount(plan: string): number { @@ -63,12 +63,18 @@ export interface BatchRefusedOperation { readonly reason: string; } +export interface BatchUnchangedMutation { + readonly path: string; + readonly op: string; +} + export interface BatchApplyReport { readonly filesChanged: number; readonly filesCreated: number; readonly filesDeleted: number; readonly matchCount: number | undefined; readonly refused: readonly BatchRefusedOperation[]; + readonly unchanged: readonly BatchUnchangedMutation[]; } /** Parse `patchloom batch --json` stdout. Undefined when the CLI did not return that object. */ @@ -84,6 +90,7 @@ export function parseBatchApplyReport(stdout: string): BatchApplyReport | undefi files_deleted?: unknown; match_count?: unknown; refused?: unknown; + mutations?: unknown; }; if (typeof parsed.files_changed !== "number") { return undefined; @@ -109,7 +116,8 @@ export function parseBatchApplyReport(stdout: string): BatchApplyReport | undefi filesCreated: typeof parsed.files_created === "number" ? parsed.files_created : 0, filesDeleted: typeof parsed.files_deleted === "number" ? parsed.files_deleted : 0, matchCount: typeof parsed.match_count === "number" ? parsed.match_count : undefined, - refused + refused, + unchanged: unchangedMutations(parsed.mutations) }; } catch { return undefined; @@ -123,8 +131,8 @@ export interface BatchApplyCompletion { /** * Exit 0 is not "every plan line was applied". CLI 0.37 keeps earlier writes - * when another replace matches nothing, and reports that on `refused` or - * `match_count: 0`. + * when another replace matches nothing (`refused` or `match_count: 0`) or + * when `doc.delete` removes nothing (`mutations[].changed === false`). */ export function formatBatchApplyCompletion(stdout: string, operationCount: number): BatchApplyCompletion { const report = parseBatchApplyReport(stdout); @@ -136,13 +144,14 @@ export function formatBatchApplyCompletion(stdout: string, operationCount: numbe } const tally = describeBatchFileTally(report); - if (report.refused.length > 0) { - const details = report.refused - .map((item) => `${item.path} (${describeRefusedReason(item.reason)})`) - .join(", "); + const missed = [ + ...report.refused.map((item) => `${item.path} (${describeRefusedReason(item.reason)})`), + ...report.unchanged.map((item) => `${item.path} (${item.op} changed nothing)`) + ]; + if (missed.length > 0) { return { warning: true, - message: `Batch apply: ${tally}. ${report.refused.length} operation(s) were not applied: ${details}.` + message: `Batch apply: ${tally}. ${missed.length} operation(s) were not applied: ${missed.join(", ")}.` }; } if (report.matchCount === 0) { @@ -171,6 +180,27 @@ function describeBatchFileTally(report: BatchApplyReport): string { return parts.length > 0 ? parts.join(", ") : "no files changed"; } +function unchangedMutations(value: unknown): BatchUnchangedMutation[] { + if (!Array.isArray(value)) { + return []; + } + const unchanged: BatchUnchangedMutation[] = []; + for (const item of value) { + if (!item || typeof item !== "object") { + continue; + } + const record = item as { path?: unknown; op?: unknown; changed?: unknown }; + if (record.changed !== false || typeof record.path !== "string" || record.path.length === 0) { + continue; + } + unchanged.push({ + path: record.path, + op: typeof record.op === "string" && record.op.length > 0 ? record.op : "operation" + }); + } + return unchanged; +} + function describeRefusedReason(reason: string): string { if (reason === "no_matches") { return "no matches"; diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index 81edf90..361ab6d 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -201,6 +201,37 @@ test("formatBatchApplyCompletion names a refused replace instead of claiming eve ); }); +test("formatBatchApplyCompletion warns when doc.delete removes nothing", () => { + const stdout = JSON.stringify({ + ok: true, + status: "success", + applied: true, + files_changed: 1, + files_created: 0, + files_deleted: 0, + mutations: [{ path: "d.json", op: "doc.delete", changed: false, removed: 0 }] + }); + const completion = formatBatchApplyCompletion(stdout, 2); + assert.equal(completion.warning, true); + assert.equal( + completion.message, + "Batch apply: 1 file(s) changed. 1 operation(s) were not applied: d.json (doc.delete changed nothing)." + ); +}); + +test("formatBatchApplyCompletion does not warn when doc.delete removes a key", () => { + const stdout = JSON.stringify({ + ok: true, + files_changed: 1, + files_created: 0, + files_deleted: 0, + mutations: [{ path: "d.json", op: "doc.delete", changed: true, removed: 1 }] + }); + const completion = formatBatchApplyCompletion(stdout, 1); + assert.equal(completion.warning, false); + assert.equal(completion.message, "Batch apply completed: 1 file(s) changed."); +}); + test("formatBatchApplyCompletion warns when a same-file replace reports match_count 0", () => { const stdout = JSON.stringify({ ok: true, From 6db0a088a63e23ba534e8217ba91c17f981449c3 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:42:58 -0700 Subject: [PATCH 21/25] fix: warn when a patch apply or merge writes nothing patch apply and patch merge exit 0 with applied set to false when every hunk is already present. Quick Actions now request JSON and show a warning instead of a success toast. A patch that writes a file still reports success. npm run check: 472 passed, line coverage 92.83%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/quickActions.ts | 24 +++++++++++++++++++++++- test/unit/quickActions.test.ts | 32 ++++++++++++++++++++++++-------- 3 files changed, 48 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 822db37..e527db1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (100 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (102 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 81b7fd6..fb3da44 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -117,6 +117,23 @@ export function presentSearchOutcome( return "hits"; } +/** + * `patch apply` and `patch merge` exit 0 when a patch writes nothing + * (`applied: false`). That is not a successful apply. + */ +export function patchCommandMadeNoChanges(stdout: string): boolean { + const trimmed = stdout.trim(); + if (!trimmed.startsWith("{")) { + return false; + } + try { + const parsed = JSON.parse(trimmed) as { applied?: unknown }; + return parsed.applied === false; + } catch { + return false; + } +} + export function presentPatchMergeOutcome( log: PatchloomLog | undefined, result: { exitCode: number; stdout: string; stderr: string } @@ -1210,6 +1227,8 @@ export async function runQuickAction(): Promise { if (result.exitCode !== 0) { await vscode.window.showErrorMessage(`Patch apply failed: ${formatCliOutput(result)}`); + } else if (patchCommandMadeNoChanges(result.stdout)) { + await vscode.window.showWarningMessage("Patch made no changes."); } else { await vscode.window.showInformationMessage("Patch applied successfully."); } @@ -1262,6 +1281,8 @@ export async function runQuickAction(): Promise { await vscode.window.showWarningMessage("Patch merge completed with unresolved conflicts. Check the output for details."); } else if (outcome === "error") { await vscode.window.showErrorMessage(`Patch merge failed: ${formatCliOutput(result)}`); + } else if (patchCommandMadeNoChanges(result.stdout)) { + await vscode.window.showWarningMessage("Patch merge made no changes."); } else { await vscode.window.showInformationMessage("Patch merged successfully."); } @@ -1674,7 +1695,7 @@ export function buildMdInsertBeforeHeadingQuickAction(targetPath: string, headin } export function buildPatchApplyQuickAction(patchPath: string): PlannedQuickAction { - const args = withEndOfOptions(["patch", "apply"], [patchPath]); + const args = withEndOfOptions(["patch", "apply", "--json"], [patchPath]); return { title: `Apply patch ${path.basename(patchPath)}`, targetPath: patchPath, @@ -1689,6 +1710,7 @@ export function buildPatchMergeQuickAction(patchPath: string, allowConflicts: bo if (allowConflicts) { head.push("--allow-conflicts"); } + head.push("--json"); const args = withEndOfOptions(head, [patchPath]); return { title: `Merge patch ${path.basename(patchPath)}`, diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index ccc21b5..6c2b2de 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -30,6 +30,7 @@ import { buildMdUpsertBulletQuickAction, buildPatchApplyQuickAction, buildPatchMergeQuickAction, + patchCommandMadeNoChanges, buildReplaceQuickAction, buildSearchQuickAction, buildTidyQuickAction, @@ -881,8 +882,8 @@ test("buildPatchApplyQuickAction builds a patch apply command", () => { const action = buildPatchApplyQuickAction("/workspace/demo/changes.patch"); assert.equal(action.title, "Apply patch changes.patch"); - assert.deepEqual(action.targetArgIndices, [3]); - assert.deepEqual(action.args, ["patch", "apply", "--", "/workspace/demo/changes.patch"]); + assert.deepEqual(action.targetArgIndices, [4]); + assert.deepEqual(action.args, ["patch", "apply", "--json", "--", "/workspace/demo/changes.patch"]); assert.equal(action.apply, true); }); @@ -890,7 +891,7 @@ test("retargetQuickAction works with patch apply command", () => { const action = buildPatchApplyQuickAction("/workspace/demo/fix.patch"); const retargeted = retargetQuickAction(action, "/tmp/preview/fix.patch"); - assert.equal(retargeted.args[3], "/tmp/preview/fix.patch"); + assert.equal(retargeted.args[4], "/tmp/preview/fix.patch"); assert.equal(retargeted.args[0], "patch"); assert.equal(retargeted.args[1], "apply"); assert.equal(retargeted.apply, true); @@ -902,8 +903,8 @@ test("buildPatchMergeQuickAction builds a patch merge command", () => { const action = buildPatchMergeQuickAction("/workspace/demo/changes.patch", false); assert.equal(action.title, "Merge patch changes.patch"); - assert.deepEqual(action.targetArgIndices, [3]); - assert.deepEqual(action.args, ["patch", "merge", "--", "/workspace/demo/changes.patch"]); + assert.deepEqual(action.targetArgIndices, [4]); + assert.deepEqual(action.args, ["patch", "merge", "--json", "--", "/workspace/demo/changes.patch"]); assert.equal(action.apply, true); }); @@ -911,8 +912,8 @@ test("buildPatchMergeQuickAction includes allow-conflicts flag when enabled", () const action = buildPatchMergeQuickAction("/workspace/demo/stale.diff", true); assert.equal(action.title, "Merge patch stale.diff"); - assert.deepEqual(action.targetArgIndices, [4]); - assert.deepEqual(action.args, ["patch", "merge", "--allow-conflicts", "--", "/workspace/demo/stale.diff"]); + assert.deepEqual(action.targetArgIndices, [5]); + assert.deepEqual(action.args, ["patch", "merge", "--allow-conflicts", "--json", "--", "/workspace/demo/stale.diff"]); assert.equal(action.apply, true); }); @@ -920,11 +921,26 @@ test("retargetQuickAction works with patch merge command", () => { const action = buildPatchMergeQuickAction("/workspace/demo/fix.patch", false); const retargeted = retargetQuickAction(action, "/tmp/preview/fix.patch"); - assert.equal(retargeted.args[3], "/tmp/preview/fix.patch"); + assert.equal(retargeted.args[4], "/tmp/preview/fix.patch"); assert.equal(retargeted.args[0], "patch"); assert.equal(retargeted.args[1], "merge"); }); +test("patchCommandMadeNoChanges is true when patch JSON says applied false", () => { + const stdout = JSON.stringify({ ok: true, files: [], applied: false }); + assert.equal(patchCommandMadeNoChanges(stdout), true); +}); + +test("patchCommandMadeNoChanges is false when the patch applied", () => { + const stdout = JSON.stringify({ + ok: true, + files: [{ path: "a.txt", status: "applied" }], + applied: true + }); + assert.equal(patchCommandMadeNoChanges(stdout), false); + assert.equal(patchCommandMadeNoChanges("applied a.txt\n"), false); +}); + // --- append Quick Action (reflecting patchloom 0.4.0+) --- test("buildAppendQuickAction builds an append command", () => { From 4a1c8291482d5280e2d4a88dce4258e1bd5be0ed Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:47:38 -0700 Subject: [PATCH 22/25] fix: stop claiming every file contains a search miss Search files without a match exits 3 when the list is empty. An empty folder and a glob that matches nothing use that same exit, so the Quick Action no longer says every scanned file contains the pattern. npm run check: 473 passed, line coverage 92.83%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- README.md | 2 +- src/commands/quickActions.ts | 11 ++++++++++- test/unit/quickActions.test.ts | 5 +++++ 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e527db1..24baabe 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (102 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (103 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/README.md b/README.md index 035d6a5..5c5554d 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ On CLI 0.31+, create and rename check each parent of the destination before stag On CLI 0.30+, selectors accept `!=`, `>`, `>=`, `<`, `<=`, and `[!key]` (for example `servers[port>8000]`). A non-numeric compare reports `error_kind: invalid_input`. Use a number on the right-hand side, or a concrete index path. **Search files without match** -On CLI 0.29+, `search -L` / `--files-without-match` lists files that do not contain the pattern. Combining it with `--files-with-matches` or `--count` is `invalid_input`. When every scanned file contains the pattern, the CLI reports `error_kind: no_matches` and the text `no files without matches for 'PATTERN' in SCOPE` (that is not a content miss). +On CLI 0.29+, `search -L` / `--files-without-match` lists files that do not contain the pattern. Combining it with `--files-with-matches` or `--count` is `invalid_input`. When that list is empty, the CLI exits 3 with `error_kind: no_matches` and `no files without matches for 'PATTERN' in SCOPE`. An empty folder, a glob that matches nothing, and a folder where every file contains the pattern all look the same. The Quick Action says no files lack the pattern. **YAML mapping alias stayed an alias** On CLI 0.31+, `doc set` on a mapping that is only `service_a: *shared` writes a merge key plus local fields (`<<: *shared` and your new keys) instead of inlining the whole object. Sequence items (`- *shared`) still expand or stay not-applied. CLI 0.32+ also keeps the merge key when you grow an inherited array, delete an inherited field, or write an empty object. diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index fb3da44..7f4bc88 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -117,6 +117,15 @@ export function presentSearchOutcome( return "hits"; } +/** + * `--files-without-match` exits 3 when nothing is listed. An empty folder + * and a glob that matches nothing use that same exit, so this must not + * say every scanned file contains the pattern. + */ +export function filesWithoutMatchNoneMessage(pattern: string): string { + return `No files without matches for "${pattern}".`; +} + /** * `patch apply` and `patch merge` exit 0 when a patch writes nothing * (`applied: false`). That is not a successful apply. @@ -539,7 +548,7 @@ export async function runQuickAction(): Promise { const outcome = presentSearchOutcome(log, result); if (outcome === "none") { - await vscode.window.showInformationMessage(`Every scanned file contains "${pattern}".`); + await vscode.window.showInformationMessage(filesWithoutMatchNoneMessage(pattern)); } else if (outcome === "error") { await vscode.window.showErrorMessage(`Patchloom search failed: ${formatCliOutput(result)}`); } else { diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 6c2b2de..4af35ad 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -49,6 +49,7 @@ import { stageExternalPatchInWorkspace, formatUndoFailureMessage, presentPatchMergeOutcome, + filesWithoutMatchNoneMessage, presentSearchOutcome, presentUndoSuccess, resolveWorkspaceRelativePath, @@ -964,6 +965,10 @@ test("presentSearchOutcome exit 0 writes streams + show, returns hits", () => { assert.deepEqual(messages, ["file.ts:1:hit", "note", "SHOW"]); }); +test("filesWithoutMatchNoneMessage does not claim every file contains the pattern", () => { + assert.equal(filesWithoutMatchNoneMessage("TODO"), 'No files without matches for "TODO".'); +}); + test("presentSearchOutcome exit 3 writes nothing, returns none", () => { const { log, messages } = fakeLog(); const kind = presentSearchOutcome(log, { From cec619c05a6e0e13c73573f6a62604ccbff69dec Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 13:57:08 -0700 Subject: [PATCH 23/25] fix: stop suggesting doc update after that update missed A doc.update that matches nothing names itself in the error. The Quick Action toast treated that name as a suggestion to run the same action. A real "Use: doc update" hint still points at the Update action. npm run check: 475 passed, line coverage 92.83%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/util.ts | 34 +++++++++++++++++++++++------ test/unit/initializeProject.test.ts | 18 +++++++++++++++ 3 files changed, 46 insertions(+), 8 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 24baabe..e0a4b03 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -53,7 +53,7 @@ test/ batchApply.test.ts Batch template and operation count parsing (28 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) - initializeProject.test.ts Status display, agents file classification, formatError (71 tests) + initializeProject.test.ts Status display, agents file classification, formatError (73 tests) managedLifecycle.test.ts Managed install with real file I/O (28 tests) mcpConfig.test.ts MCP config with real temp directories (23 tests) managedInstall.test.ts Managed Update compares latest vs managed binary (10 tests) diff --git a/src/util.ts b/src/util.ts index 8d95098..67c8ada 100644 --- a/src/util.ts +++ b/src/util.ts @@ -87,11 +87,25 @@ const QUICK_ACTION_UPDATE_HINT = const QUICK_ACTION_DELETE_WHERE_HINT = '(try Quick Action "Delete matching array items" or CLI `doc delete-where`)'; +/** + * A suggestion ("use doc update", "try doc.delete_where"), not the name of + * the operation that just failed ("doc.update matched nothing"). + */ +function mentionsSuggestedCommand(raw: string, commands: readonly string[]): boolean { + const folded = raw.toLowerCase(); + return commands.some((command) => + folded.includes(`try ${command}`) || + folded.includes(`use ${command}`) || + folded.includes(`use: ${command}`) || + folded.includes(`retry with ${command}`) + ); +} + /** * Quick Action toast wrapper around formatCliOutput. Agents still parse the * CLI token from formatCliOutput; picker labels are for humans in the UI. - * Preview does not pass --json, so also scan human stderr for suggested_op - * tokens (doc.update, doc update, doc.delete_where, doc delete-where). + * Preview does not pass --json, so also scan human stderr for suggestion + * phrasing. The failing operation's own name is not a suggestion. */ export function formatQuickActionCliOutput(result: { exitCode: number; @@ -103,15 +117,21 @@ export function formatQuickActionCliOutput(result: { .replaceAll("(try doc.delete_where)", QUICK_ACTION_DELETE_WHERE_HINT); const raw = `${result.stderr}\n${result.stdout}`; - const mentionsUpdate = raw.includes("doc.update") || raw.includes("doc update"); - const mentionsDelete = - raw.includes("doc.delete_where") || raw.includes("doc delete-where"); + const suggestsUpdate = mentionsSuggestedCommand(raw, ["doc.update", "doc update"]); + const suggestsDelete = mentionsSuggestedCommand(raw, ["doc.delete_where", "doc delete-where"]); + const namesDeleteOp = raw.includes("doc.delete_where") || raw.includes("doc delete-where"); const mentionsSuggestedOp = raw.includes("suggested_op"); - if (!formatted.includes(QUICK_ACTION_UPDATE_HINT) && (mentionsUpdate || (mentionsSuggestedOp && !mentionsDelete))) { + if ( + !formatted.includes(QUICK_ACTION_UPDATE_HINT) && + (suggestsUpdate || (mentionsSuggestedOp && !namesDeleteOp)) + ) { formatted = `${formatted} ${QUICK_ACTION_UPDATE_HINT}`; } - if (!formatted.includes(QUICK_ACTION_DELETE_WHERE_HINT) && mentionsDelete) { + if ( + !formatted.includes(QUICK_ACTION_DELETE_WHERE_HINT) && + (suggestsDelete || (mentionsSuggestedOp && namesDeleteOp)) + ) { formatted = `${formatted} ${QUICK_ACTION_DELETE_WHERE_HINT}`; } return formatted; diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index 2f0910c..165f34a 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -402,6 +402,24 @@ test("formatQuickActionCliOutput maps human stderr doc.delete_where to the picke ); }); +test("formatQuickActionCliOutput does not suggest doc update when that operation matched nothing", () => { + const stderr = + "operation 1 (doc.update) failed: d.json: doc.update matched nothing for selector 'servers[port>8000]'"; + assert.equal( + formatQuickActionCliOutput({ exitCode: 3, stdout: "", stderr }), + stderr + ); +}); + +test("formatQuickActionCliOutput maps a colon Use: doc update hint to the picker label", () => { + const stderr = + "selector uses wildcard/predicate, which is not valid for doc.set (single path only). Use: doc update 'items[id=b].val' "; + assert.equal( + formatQuickActionCliOutput({ exitCode: 1, stdout: "", stderr }), + `${stderr} (try Quick Action "Update matching structured values" or CLI \`doc update\`)` + ); +}); + test("formatCliOutput keeps dotted suggested_op token for agents", () => { const stdout = JSON.stringify({ ok: false, From 3a4c80fbe1057d2d345a38ece34abdf226428f84 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 14:10:42 -0700 Subject: [PATCH 24/25] fix: report a missing fragment or insert anchor apply-fragment and insert-before or insert-after exit 3 when the anchor is not in the file. Preview treated that like a text replace with nothing to change. Those misses now show the CLI error. npm run check: 476 passed, line coverage 92.86%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/quickActions.ts | 10 +++++++--- test/unit/quickActions.test.ts | 11 +++++++++++ 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index e0a4b03..a39a6fb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (53 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (103 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (104 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (12 tests) packageEngines.test.ts engines.node major meets installed vsce and ovsx floors (1 test) diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 7f4bc88..aa9cef8 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -1762,9 +1762,13 @@ export function isAllowedPreviewMiss(action: PlannedQuickAction, exitCode: numbe } // PlannedQuickAction.args are operands only. executePatchloom adds --contain. // Exit 3 is a soft no-op for text replace (pattern not in the file). - // It is a real miss for markdown section ops (heading not found) and for - // doc update, delete-where, append, prepend, merge, and move (missing key). - if (action.args[0] === "md") { + // It is a real miss when an anchor or heading was not found: markdown + // section ops, apply-fragment, and insert-before / insert-after. + // Same for doc update, delete-where, append, prepend, merge, and move. + if (action.args[0] === "md" || action.args[0] === "apply-fragment") { + return false; + } + if (action.args.some((arg) => arg.startsWith("--insert-before=") || arg.startsWith("--insert-after="))) { return false; } if ( diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 4af35ad..d187a10 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -208,6 +208,17 @@ test("isAllowedPreviewMiss allows replace exit 3", () => { assert.equal(isAllowedPreviewMiss(action, 3), true); }); +test("isAllowedPreviewMiss rejects a missing apply-fragment or insert anchor", () => { + const actions = [ + buildApplyFragmentQuickAction("/workspace/demo/lib.rs", "after", "missing anchor", " let x = 2;"), + buildInsertAfterMatchQuickAction("/workspace/demo/lib.rs", "nope", "zzz"), + buildInsertBeforeMatchQuickAction("/workspace/demo/lib.rs", "nope", "zzz") + ]; + for (const action of actions) { + assert.equal(isAllowedPreviewMiss(action, 3), false); + } +}); + test("isAllowedPreviewMiss rejects doc delete-where exit 3", () => { const action = buildDocDeleteWhereQuickAction("/workspace/demo/data.json", "items", "name=stale"); assert.equal(isAllowedPreviewMiss(action, 3), false); From 75521c11c3731ae03672bc10e8646e0b7086116c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Mon, 5 Oct 2026 14:15:08 -0700 Subject: [PATCH 25/25] fix: report a batch rename when no file content changed A rename-only batch plan exits 0 with files_changed 0 and files_renamed 1. The toast said no files changed. It now counts renames. npm run check: 477 passed, line coverage 92.85%. Signed-off-by: Sebastien Tardif --- AGENTS.md | 2 +- src/commands/batchApply.ts | 7 +++++++ test/unit/batchApply.test.ts | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index a39a6fb..0ef0fe8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,7 +50,7 @@ src/ workspace/readiness.ts Workspace readiness: environment detection, folder selection test/ unit/ Unit tests (node:test, dependency-injected, no VS Code API) - batchApply.test.ts Batch template and operation count parsing (28 tests) + batchApply.test.ts Batch template and operation count parsing (29 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (76 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (73 tests) diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index 0d613aa..6ac5c3c 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -72,6 +72,7 @@ export interface BatchApplyReport { readonly filesChanged: number; readonly filesCreated: number; readonly filesDeleted: number; + readonly filesRenamed: number; readonly matchCount: number | undefined; readonly refused: readonly BatchRefusedOperation[]; readonly unchanged: readonly BatchUnchangedMutation[]; @@ -88,6 +89,7 @@ export function parseBatchApplyReport(stdout: string): BatchApplyReport | undefi files_changed?: unknown; files_created?: unknown; files_deleted?: unknown; + files_renamed?: unknown; match_count?: unknown; refused?: unknown; mutations?: unknown; @@ -115,6 +117,7 @@ export function parseBatchApplyReport(stdout: string): BatchApplyReport | undefi filesChanged: parsed.files_changed, filesCreated: typeof parsed.files_created === "number" ? parsed.files_created : 0, filesDeleted: typeof parsed.files_deleted === "number" ? parsed.files_deleted : 0, + filesRenamed: typeof parsed.files_renamed === "number" ? parsed.files_renamed : 0, matchCount: typeof parsed.match_count === "number" ? parsed.match_count : undefined, refused, unchanged: unchangedMutations(parsed.mutations) @@ -133,6 +136,7 @@ export interface BatchApplyCompletion { * Exit 0 is not "every plan line was applied". CLI 0.37 keeps earlier writes * when another replace matches nothing (`refused` or `match_count: 0`) or * when `doc.delete` removes nothing (`mutations[].changed === false`). + * A rename-only plan has `files_changed: 0` and `files_renamed: 1`. */ export function formatBatchApplyCompletion(stdout: string, operationCount: number): BatchApplyCompletion { const report = parseBatchApplyReport(stdout); @@ -177,6 +181,9 @@ function describeBatchFileTally(report: BatchApplyReport): string { if (report.filesDeleted > 0) { parts.push(`${report.filesDeleted} file(s) deleted`); } + if (report.filesRenamed > 0) { + parts.push(`${report.filesRenamed} file(s) renamed`); + } return parts.length > 0 ? parts.join(", ") : "no files changed"; } diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index 361ab6d..5c6f63f 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -248,6 +248,20 @@ test("formatBatchApplyCompletion warns when a same-file replace reports match_co assert.match(completion.message, /1 file\(s\) changed/); }); +test("formatBatchApplyCompletion reports a rename when no file content changed", () => { + const stdout = JSON.stringify({ + ok: true, + applied: true, + files_changed: 0, + files_created: 0, + files_deleted: 0, + files_renamed: 1 + }); + const completion = formatBatchApplyCompletion(stdout, 1); + assert.equal(completion.warning, false); + assert.equal(completion.message, "Batch apply completed: 1 file(s) renamed."); +}); + test("formatBatchApplyCompletion reports created files without a zero changed count", () => { const stdout = JSON.stringify({ ok: true,