From 1bd280a11d7f2f9c28f019c830cc010ffe31706c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 13:27:38 -0700 Subject: [PATCH 1/8] feat: add live tests for CLI 0.29-0.31 Quick Action paths - Numeric selector via buildDocGetQuickAction - search -L all-hits no_matches + formatCliOutput stderr - create parent-path via builder, YAML alias doc set, agent-rules core+mode Signed-off-by: Sebastien Tardif --- AGENTS.md | 4 +- test/unit/initializeProject.test.ts | 21 +++++ test/unit/patchloomCli.test.ts | 119 +++++++++++++++++++++++++++- 3 files changed, 140 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 3c617f9..e263892 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,11 +51,11 @@ test/ batchApply.test.ts Batch template and operation count parsing (16 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (66 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) - initializeProject.test.ts Status display, agents file classification, formatError (41 tests) + initializeProject.test.ts Status display, agents file classification, formatError (43 tests) managedLifecycle.test.ts Managed install with real file I/O (22 tests) mcpConfig.test.ts MCP config with real temp directories (12 tests) outputChannel.test.ts Output channel logging wrapper (10 tests) - patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (40 tests incl. e2e) + patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (44 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 (62 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index c1907d9..62d877a 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -176,6 +176,27 @@ test("formatCliOutput surfaces numeric selector invalid_input (CLI 0.30+)", () = ); }); +test("formatCliOutput surfaces no_matches kind from stderr (CLI 0.29+)", () => { + const stderr = JSON.stringify({ + ok: false, + error: "no files without matches for 'TODO' in /tmp/ws", + error_kind: "no_matches" + }); + assert.equal( + formatCliOutput({ exitCode: 3, stdout: "", stderr }), + "no_matches: no files without matches for 'TODO' in /tmp/ws" + ); +}); + +test("formatCliOutput falls through when stdout starts with { but is not JSON", () => { + const result = formatCliOutput({ + exitCode: 1, + stdout: "{not valid json", + stderr: "fallback text" + }); + assert.equal(result, "fallback text {not valid json"); +}); + test("formatCliOutput appends suggested_op when present (CLI 0.27+)", () => { const stdout = JSON.stringify({ ok: false, diff --git a/test/unit/patchloomCli.test.ts b/test/unit/patchloomCli.test.ts index 24b9751..2421485 100644 --- a/test/unit/patchloomCli.test.ts +++ b/test/unit/patchloomCli.test.ts @@ -22,10 +22,13 @@ import { assessPatchloomCompatibility, resolvePatchloomStatusWithInputs } from "../../src/binary/patchloom.js"; -import { classifyAgentsFile } from "../../src/commands/initializeProject.js"; +import { classifyAgentsFile, generateAgentRules } from "../../src/commands/initializeProject.js"; import { buildStatusDetails, preferredStatusAction } from "../../src/commands/showStatus.js"; import { + buildCreateQuickAction, + buildDocGetQuickAction, buildDocMergeQuickAction, + buildDocSetQuickAction, buildApplyFragmentQuickAction, buildInsertAfterMatchQuickAction, buildReplaceQuickAction, @@ -35,6 +38,7 @@ import { } from "../../src/commands/quickActions.js"; import { configureMcpTargets, inspectMcpTargets } from "../../src/mcp/config.js"; import { performManagedInstall } from "../../src/install/managed.js"; +import { formatCliOutput } from "../../src/util.js"; const execFileAsync = promisify(execFile); @@ -605,6 +609,73 @@ describe("patchloom CLI integration", async () => { }); }); + test("search --files-without-match all-hits is no_matches via Quick Action args (CLI 0.29+)", async (t) => { + const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 5000 }); + const version = parsePatchloomVersion(`${stdout}${stderr}`); + if (!version || comparePatchloomVersions(version, "0.29.0") < 0) { + t.skip(`requires patchloom >= 0.29.0 (found ${version ?? "unknown"})`); + return; + } + + await withTempDir(async (dir) => { + await fs.writeFile(path.join(dir, "a.txt"), "TODO\n", "utf8"); + await fs.writeFile(path.join(dir, "b.txt"), "TODO\n", "utf8"); + + const action = buildSearchQuickAction(dir, "TODO", undefined, { filesWithoutMatch: true }); + try { + await execFileAsync(binaryPath, ["--json", ...action.args], { timeout: 5000 }); + assert.fail("all-hit --files-without-match should fail"); + } catch (error) { + const failed = error as { code?: number; stdout?: string; stderr?: string }; + assert.equal(failed.code, 3, "no-match exit code should be 3"); + const payload = `${failed.stdout ?? ""}${failed.stderr ?? ""}`; + assert.match(payload, /no_matches/); + assert.match(payload, /no files without matches/); + } + }); + }); + + test("doc get numeric-compare selector via Quick Action args (CLI 0.30+)", async (t) => { + const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 5000 }); + const version = parsePatchloomVersion(`${stdout}${stderr}`); + if (!version || comparePatchloomVersions(version, "0.30.0") < 0) { + t.skip(`requires patchloom >= 0.30.0 (found ${version ?? "unknown"})`); + return; + } + + await withTempDir(async (dir) => { + const file = path.join(dir, "servers.json"); + await fs.writeFile( + file, + JSON.stringify({ servers: [{ name: "a", port: 9000 }, { name: "b", port: 80 }] }), + "utf8" + ); + + const getAction = buildDocGetQuickAction(file, "servers[port>8000]"); + const result = await execFileAsync(binaryPath, ["--json", ...getAction.args], { timeout: 5000 }); + const parsed = JSON.parse(result.stdout) as { value?: { port?: number } }; + assert.equal(parsed.value?.port, 9000, "numeric compare should return the 9000 entry"); + + const badAction = buildDocGetQuickAction(file, "servers[port>abc]"); + try { + await execFileAsync(binaryPath, ["--json", ...badAction.args], { timeout: 5000 }); + assert.fail("non-numeric comparison operand should fail"); + } catch (error) { + const failed = error as { code?: number; stdout?: string; stderr?: string }; + const payload = `${failed.stdout ?? ""}${failed.stderr ?? ""}`; + assert.match(payload, /invalid_input/); + assert.match(payload, /must be numeric/); + const formatted = formatCliOutput({ + exitCode: typeof failed.code === "number" ? failed.code : 1, + stdout: failed.stdout ?? "", + stderr: failed.stderr ?? "" + }); + assert.match(formatted, /invalid_input/); + assert.match(formatted, /must be numeric/); + } + }); + }); + test("create through a file parent is invalid_input (CLI 0.31+)", async (t) => { const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 5000 }); const version = parsePatchloomVersion(`${stdout}${stderr}`); @@ -617,9 +688,10 @@ describe("patchloom CLI integration", async () => { const parent = path.join(dir, "notdir"); await fs.writeFile(parent, "file\n", "utf8"); const dest = path.join(parent, "child.txt"); + const action = buildCreateQuickAction(dest, "x"); try { - await execFileAsync(binaryPath, ["--json", "create", dest, "--content", "x", "--apply"], { + await execFileAsync(binaryPath, ["--json", ...action.args], { timeout: 5000 }); assert.fail("create through a file parent should fail"); @@ -632,6 +704,49 @@ describe("patchloom CLI integration", async () => { }); }); + test("doc set YAML alias via Quick Action args (CLI 0.31+)", async (t) => { + const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 5000 }); + const version = parsePatchloomVersion(`${stdout}${stderr}`); + if (!version || comparePatchloomVersions(version, "0.31.0") < 0) { + t.skip(`requires patchloom >= 0.31.0 (found ${version ?? "unknown"})`); + return; + } + + await withTempDir(async (dir) => { + const file = path.join(dir, "svc.yaml"); + await fs.writeFile(file, [ + "shared: &shared", + " timeout: 30", + "service_a: *shared", + "" + ].join("\n"), "utf8"); + + const action = buildDocSetQuickAction(file, "service_a.retries", "3"); + await execFileAsync(binaryPath, withApplyFlag(action.args), { timeout: 5000 }); + + const content = await fs.readFile(file, "utf8"); + assert.match(content, /<<: \*shared/, "alias merge key should be preserved"); + assert.match(content, /retries/, "new field should be written onto the alias"); + }); + }); + + test("generateAgentRules core surface honors mode (CLI 0.29+)", async (t) => { + const { stdout, stderr } = await execFileAsync(binaryPath, ["--version"], { timeout: 5000 }); + const version = parsePatchloomVersion(`${stdout}${stderr}`); + if (!version || comparePatchloomVersions(version, "0.29.0") < 0) { + t.skip(`requires patchloom >= 0.29.0 (found ${version ?? "unknown"})`); + return; + } + + await withTempDir(async (dir) => { + const mcp = await generateAgentRules(binaryPath, dir, { surface: "core", mode: "mcp" }); + const cli = await generateAgentRules(binaryPath, dir, { surface: "core", mode: "cli" }); + assert.match(mcp, /^#/m, "core mcp rules should be markdown"); + assert.match(cli, /^#/m, "core cli rules should be markdown"); + assert.notEqual(mcp, cli, "core mcp and cli texts should differ"); + }); + }); + test("doc append adds an item to a JSON array", async () => { await withTempDir(async (dir) => { const file = path.join(dir, "config.json"); From 315ba61467a63b4c1c00d8bf7f3e48b4e8a92f2a Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 13:35:10 -0700 Subject: [PATCH 2/8] fix: reuse workspace path helper and surface CLI envelopes on initialize - Create Quick Action now uses resolveWorkspaceRelativePath - generateAgentRules throws formatCliOutput so error_kind and suggested_op appear - readTextFileIfExists treats only FileNotFound as missing AGENTS.md Signed-off-by: Sebastien Tardif --- src/commands/initializeProject.ts | 46 +++++++++++++++++---- src/commands/quickActions.ts | 7 ++-- test/unit/initializeProject.test.ts | 64 ++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 11 deletions(-) diff --git a/src/commands/initializeProject.ts b/src/commands/initializeProject.ts index 789e974..f5ab255 100644 --- a/src/commands/initializeProject.ts +++ b/src/commands/initializeProject.ts @@ -3,7 +3,7 @@ import { promisify } from "node:util"; import type * as VSCode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; import { getPatchloomLog } from "../logging/outputChannel.js"; -import { formatError } from "../util.js"; +import { formatCliOutput, formatError } from "../util.js"; import { activeWorkspaceFolder } from "../workspace/readiness.js"; const execFileAsync = promisify(execFile); @@ -160,16 +160,31 @@ export function buildAgentRulesArgs(options: AgentRulesOptions = {}): string[] { return args; } +export interface GenerateAgentRulesDeps { + readonly execFile?: ( + file: string, + args: readonly string[], + options: { + cwd?: string; + timeout?: number; + maxBuffer?: number; + windowsHide?: boolean; + } + ) => Promise<{ stdout: string; stderr: string }>; +} + export async function generateAgentRules( binaryPath: string, cwd: string, - options: AgentRulesOptions = {} + options: AgentRulesOptions = {}, + deps: GenerateAgentRulesDeps = {} ): Promise { const log = getPatchloomLog(); const args = buildAgentRulesArgs(options); log?.logCommand(binaryPath, args, cwd); + const run = deps.execFile ?? execFileAsync; try { - const { stdout, stderr } = await execFileAsync(binaryPath, args, { + const { stdout, stderr } = await run(binaryPath, args, { cwd, timeout: 10_000, maxBuffer: 1024 * 1024, @@ -178,9 +193,23 @@ export async function generateAgentRules( log?.logResult(0, stdout, stderr); return stdout.endsWith("\n") ? stdout : `${stdout}\n`; } catch (error) { - log?.logResult(1, "", formatError(error)); - throw error; + const candidate = error as { code?: unknown; stdout?: unknown; stderr?: unknown }; + const exitCode = typeof candidate.code === "number" ? candidate.code : 1; + const stdout = typeof candidate.stdout === "string" ? candidate.stdout : ""; + const stderr = typeof candidate.stderr === "string" && candidate.stderr.length > 0 + ? candidate.stderr + : formatError(error); + log?.logResult(exitCode, stdout, stderr); + throw new Error(formatCliOutput({ exitCode, stdout, stderr })); + } +} + +/** True only for VS Code FileNotFound (missing file). Other FS errors must propagate. */ +export function isMissingFileError(error: unknown): boolean { + if (typeof error !== "object" || error === null) { + return false; } + return "code" in error && (error as { code: unknown }).code === "FileNotFound"; } async function readTextFileIfExists(uri: VSCode.Uri): Promise { @@ -188,8 +217,11 @@ async function readTextFileIfExists(uri: VSCode.Uri): Promise { } const absolutePath = path.resolve(folder.uri.fsPath, relativePath.trim()); - const relative = path.relative(folder.uri.fsPath, absolutePath); - if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) { - await vscode.window.showWarningMessage("File path must stay inside the workspace folder."); + try { + resolveWorkspaceRelativePath(folder.uri.fsPath, absolutePath); + } catch (error) { + await vscode.window.showWarningMessage(formatError(error)); return; } diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index 62d877a..d45e5c2 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -5,7 +5,8 @@ import { MINIMUM_SUPPORTED_PATCHLOOM_VERSION } from "../../src/binary/patchloom. import { buildAgentRulesArgs, classifyAgentsFile, - generateAgentRules + generateAgentRules, + isMissingFileError } from "../../src/commands/initializeProject.js"; import { buildStatusDetails, preferredStatusAction } from "../../src/commands/showStatus.js"; import { buildPatchloomMcpEntry, configureMcpTargets, inspectMcpTargets } from "../../src/mcp/config.js"; @@ -678,3 +679,64 @@ test("generateAgentRules logs error to output channel on CLI failure", async () setPatchloomLog(undefined); } }); + +test("generateAgentRules surfaces formatCliOutput envelope on CLI failure", async () => { + const logged: { exitCode: number; stdout: string; stderr: string }[] = []; + setPatchloomLog({ + log() {}, + logCommand() {}, + logResult(exitCode, stdout, stderr) { logged.push({ exitCode, stdout, stderr }); }, + show() {}, + dispose() {} + }); + const stdout = JSON.stringify({ + ok: false, + error: "selector uses wildcard/predicate, which is not valid for doc.set (single path only)", + error_kind: "invalid_input", + suggested_op: "doc.update", + applied: false + }); + const execError = Object.assign(new Error("Command failed: patchloom agent-rules"), { + code: 1, + stdout, + stderr: "" + }); + try { + await assert.rejects( + () => generateAgentRules("/fake/patchloom", "/tmp", {}, { + execFile: async () => { + throw execError; + } + }), + (err: Error) => { + assert.match(err.message, /invalid_input/); + assert.match(err.message, /suggested_op: doc\.update/); + return true; + } + ); + assert.equal(logged.length, 1, "logResult should be called once on failure"); + assert.match(`${logged[0].stdout}\n${logged[0].stderr}`, /invalid_input|suggested_op/); + } finally { + setPatchloomLog(undefined); + } +}); + +test("isMissingFileError is true for FileNotFound-shaped errors", () => { + assert.equal(isMissingFileError({ code: "FileNotFound" }), true); + assert.equal(isMissingFileError({ + code: "FileNotFound", + name: "EntryNotFound (FileSystemError)" + }), true); +}); + +test("isMissingFileError is false for permission-shaped errors", () => { + assert.equal(isMissingFileError({ code: "NoPermissions" }), false); + assert.equal(isMissingFileError({ code: "Unavailable" }), false); +}); + +test("isMissingFileError is false for generic Error", () => { + assert.equal(isMissingFileError(new Error("EACCES: permission denied")), false); + assert.equal(isMissingFileError("disk full"), false); + assert.equal(isMissingFileError(null), false); + assert.equal(isMissingFileError(undefined), false); +}); From 0ec0c4f83f9b2dbece7e1361afd0b1f40414d60c Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 13:48:36 -0700 Subject: [PATCH 3/8] fix: honor patchloom.env and make CLI errors actionable Pass patchloom.env to CLI spawn sites and honor patchloom.trace.server for command/result logging. Surface next-step hints on path, missing file, markdown, and MCP timeout errors. Catch AGENTS.md permission errors and preview failures instead of failing the command uncaught. Signed-off-by: Sebastien Tardif --- src/commands/batchApply.ts | 16 ++-- src/commands/initializeProject.ts | 34 +++++-- src/commands/quickActions.ts | 44 +++++++--- src/commands/verifyMcp.ts | 14 ++- src/logging/outputChannel.ts | 58 ++++++++++++ src/util.ts | 21 +++++ test/unit/initializeProject.test.ts | 132 +++++++++++++++++++++++++++- test/unit/outputChannel.test.ts | 72 ++++++++++++++- test/unit/quickActions.test.ts | 6 +- walkthrough/initialize.md | 7 +- 10 files changed, 362 insertions(+), 42 deletions(-) diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index fac536f..9b752dc 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -1,8 +1,8 @@ import { execFile } from "node:child_process"; import type * as VSCode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; -import { formatCliOutput } from "../util.js"; -import { getPatchloomLog } from "../logging/outputChannel.js"; +import { formatCliOutput, mergePatchloomEnv } from "../util.js"; +import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult } from "../logging/outputChannel.js"; import { activeWorkspaceFolder } from "../workspace/readiness.js"; // Batch replace is PATH OLD NEW (not CLI `replace OLD --new NEW path`). See CLI 0.18+ batch --help. @@ -65,11 +65,13 @@ export async function batchApply(): Promise { const plan = doc.getText(); const log = getPatchloomLog(); + const runtime = await getPatchloomRuntimeConfig(); + const env = mergePatchloomEnv(process.env, runtime.extraEnv); const args = buildBatchApplyArgs(); - log?.logCommand(binaryPath, args, folder.uri.fsPath); + logCliCommand(log, runtime.trace, binaryPath, args, folder.uri.fsPath); - const result = await executePatchloomWithStdin(binaryPath, args, folder.uri.fsPath, plan); - log?.logResult(result.exitCode, result.stdout, result.stderr); + const result = await executePatchloomWithStdin(binaryPath, args, folder.uri.fsPath, plan, env); + logCliResult(log, runtime.trace, result.exitCode, result.stdout, result.stderr); if (result.exitCode !== 0) { log?.show(); @@ -96,11 +98,13 @@ function executePatchloomWithStdin( binaryPath: string, args: readonly string[], cwd: string, - stdin: string + stdin: string, + env: NodeJS.ProcessEnv ): Promise { return new Promise((resolve) => { const child = execFile(binaryPath, [...args], { cwd, + env, timeout: 30_000, maxBuffer: 8 * 1024 * 1024, windowsHide: true diff --git a/src/commands/initializeProject.ts b/src/commands/initializeProject.ts index f5ab255..85d5bfa 100644 --- a/src/commands/initializeProject.ts +++ b/src/commands/initializeProject.ts @@ -2,8 +2,8 @@ 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 } from "../logging/outputChannel.js"; -import { formatCliOutput, formatError } from "../util.js"; +import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult } from "../logging/outputChannel.js"; +import { formatCliOutput, formatError, mergePatchloomEnv } from "../util.js"; import { activeWorkspaceFolder } from "../workspace/readiness.js"; const execFileAsync = promisify(execFile); @@ -72,12 +72,16 @@ export async function initializeProject(): Promise { return; } + const runtime = await getPatchloomRuntimeConfig(); let rules: string; try { rules = await generateAgentRules(binaryPath, folder.uri.fsPath, { mode: modePick.mode, platform: platformPick.platform, surface: surfacePick.surface + }, { + extraEnv: runtime.extraEnv, + trace: runtime.trace }); } catch (error) { await vscode.window.showErrorMessage(`Failed to run patchloom agent-rules in ${folder.name}: ${formatError(error)}`); @@ -90,8 +94,15 @@ export async function initializeProject(): Promise { } const agentsUri = vscode.Uri.joinPath(folder.uri, "AGENTS.md"); - const existingContent = await readTextFileIfExists(agentsUri); - const state = classifyAgentsFile(existingContent, rules); + let existingContent: string | undefined; + let state: AgentsFileState; + try { + existingContent = await readTextFileIfExists(agentsUri); + state = classifyAgentsFile(existingContent, rules); + } catch (error) { + await vscode.window.showErrorMessage(`Could not read AGENTS.md: ${formatError(error)}`); + return; + } if (state === "missing") { await vscode.workspace.fs.writeFile(agentsUri, encoder.encode(rules)); @@ -169,8 +180,12 @@ export interface GenerateAgentRulesDeps { timeout?: number; maxBuffer?: number; windowsHide?: boolean; + env?: NodeJS.ProcessEnv; } ) => Promise<{ stdout: string; stderr: string }>; + readonly extraEnv?: Record; + readonly processEnv?: NodeJS.ProcessEnv; + readonly trace?: string; } export async function generateAgentRules( @@ -181,16 +196,19 @@ export async function generateAgentRules( ): Promise { const log = getPatchloomLog(); const args = buildAgentRulesArgs(options); - log?.logCommand(binaryPath, args, cwd); + const env = mergePatchloomEnv(deps.processEnv ?? process.env, deps.extraEnv); + const trace = deps.trace ?? "off"; + logCliCommand(log, trace, binaryPath, args, cwd); const run = deps.execFile ?? execFileAsync; try { const { stdout, stderr } = await run(binaryPath, args, { cwd, timeout: 10_000, maxBuffer: 1024 * 1024, - windowsHide: true + windowsHide: true, + env }); - log?.logResult(0, stdout, stderr); + logCliResult(log, trace, 0, stdout, stderr); return stdout.endsWith("\n") ? stdout : `${stdout}\n`; } catch (error) { const candidate = error as { code?: unknown; stdout?: unknown; stderr?: unknown }; @@ -199,7 +217,7 @@ export async function generateAgentRules( const stderr = typeof candidate.stderr === "string" && candidate.stderr.length > 0 ? candidate.stderr : formatError(error); - log?.logResult(exitCode, stdout, stderr); + logCliResult(log, trace, exitCode, stdout, stderr); throw new Error(formatCliOutput({ exitCode, stdout, stderr })); } } diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index e5e931c..3e7a39d 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -5,8 +5,8 @@ import * as path from "node:path"; import { promisify } from "node:util"; import type * as VSCode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; -import { getPatchloomLog } from "../logging/outputChannel.js"; -import { formatCliOutput, formatError } from "../util.js"; +import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult } from "../logging/outputChannel.js"; +import { formatCliOutput, formatError, mergePatchloomEnv } from "../util.js"; import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; const execFileAsync = promisify(execFile); @@ -745,7 +745,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -783,7 +783,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -821,7 +821,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -859,7 +859,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -897,7 +897,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -935,7 +935,7 @@ export async function runQuickAction(): Promise { if (!isMarkdownPath(target.absolutePath)) { await vscode.window.showWarningMessage( - `${target.relativePath} is not a markdown file.` + `${target.relativePath} is not a markdown file. Pick a .md, .markdown, or .mdx file.` ); return; } @@ -1393,7 +1393,16 @@ async function previewAndMaybeApply( const vscode = await import("vscode"); const originalDocument = await vscode.workspace.openTextDocument(target.uri); const originalContent = await fs.readFile(target.absolutePath, "utf8"); - const preview = await buildPreviewDocument(binaryPath, action, originalContent, originalDocument.languageId); + let preview: VSCode.TextDocument | undefined; + try { + preview = await buildPreviewDocument(binaryPath, action, originalContent, originalDocument.languageId); + } catch (error) { + getPatchloomLog()?.show(); + await vscode.window.showErrorMessage( + `Patchloom failed while previewing changes to ${target.relativePath}: ${formatError(error)}` + ); + return; + } if (!preview) { await vscode.window.showInformationMessage(`No changes to preview for ${target.relativePath}.`); return; @@ -1547,7 +1556,9 @@ export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath const resolvedPath = path.resolve(absolutePath); const relativePath = path.relative(resolvedRoot, resolvedPath); if (!relativePath || relativePath.startsWith("..") || path.isAbsolute(relativePath)) { - throw new Error("File path must stay inside the current workspace folder."); + 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("/"); } @@ -1569,7 +1580,9 @@ async function ensureWorkspaceFileReady(target: WorkspaceFileTarget): Promise { const finalArgs = options.contain === false ? [...args] : withContainFlag(args); const log = getPatchloomLog(); - log?.logCommand(binaryPath, finalArgs, cwd); + const runtime = await getPatchloomRuntimeConfig(); + const env = mergePatchloomEnv(process.env, runtime.extraEnv); + logCliCommand(log, runtime.trace, binaryPath, finalArgs, cwd); try { const { stdout, stderr } = await execFileAsync(binaryPath, finalArgs, { cwd, + env, timeout: 30_000, maxBuffer: 8 * 1024 * 1024, windowsHide: true }); const result: PatchloomCommandResult = { exitCode: 0, stdout, stderr }; - log?.logResult(result.exitCode, result.stdout, result.stderr); + logCliResult(log, runtime.trace, result.exitCode, result.stdout, result.stderr); return result; } catch (error) { const execFailure = asExecFailure(error); @@ -1629,7 +1645,7 @@ async function executePatchloom( ? execFailure.stderr || execFailure.message : formatError(error) }; - log?.logResult(result.exitCode, result.stdout, result.stderr); + logCliResult(log, runtime.trace, result.exitCode, result.stdout, result.stderr); return result; } } diff --git a/src/commands/verifyMcp.ts b/src/commands/verifyMcp.ts index 2c956fd..3caf709 100644 --- a/src/commands/verifyMcp.ts +++ b/src/commands/verifyMcp.ts @@ -1,6 +1,7 @@ import { spawn } from "node:child_process"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; -import { getPatchloomLog } from "../logging/outputChannel.js"; +import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand } from "../logging/outputChannel.js"; +import { mergePatchloomEnv } from "../util.js"; export interface VerifyMcpInputs { readonly binaryPath: string; @@ -55,12 +56,17 @@ export async function verifyMcpServer(inputs: VerifyMcpInputs): Promise { +async function spawnMcpServer(binaryPath: string): Promise { + const runtime = await getPatchloomRuntimeConfig(); + const env = mergePatchloomEnv(process.env, runtime.extraEnv); + logCliCommand(getPatchloomLog(), runtime.trace, binaryPath, ["mcp-server"], process.cwd()); + return new Promise((resolve) => { const child = spawn(binaryPath, ["mcp-server"], { stdio: ["pipe", "pipe", "pipe"], timeout: 10_000, - windowsHide: true + windowsHide: true, + env }); let stdout = ""; @@ -79,7 +85,7 @@ function spawnMcpServer(binaryPath: string): Promise { const timer = setTimeout(() => { finish({ ok: false, - message: "MCP server did not respond within 10 seconds." + message: "MCP server did not respond within 10 seconds. Open Patchloom: Show Output, then run patchloom mcp-server --help in a terminal." }); }, 10_000); diff --git a/src/logging/outputChannel.ts b/src/logging/outputChannel.ts index dd655ae..8ab68e7 100644 --- a/src/logging/outputChannel.ts +++ b/src/logging/outputChannel.ts @@ -1,3 +1,61 @@ +import { shouldLogCliCommands, shouldLogCliStreams } from "../util.js"; + +export interface PatchloomRuntimeConfig { + readonly extraEnv: Record | undefined; + readonly trace: string; +} + +export async function getPatchloomRuntimeConfig(): Promise { + const vscode = await import("vscode"); + const config = vscode.workspace.getConfiguration("patchloom"); + return { + extraEnv: asStringEnv(config.get("env")), + trace: config.get("trace.server", "off") ?? "off" + }; +} + +function asStringEnv(value: unknown): Record | undefined { + if (value === undefined || value === null || typeof value !== "object" || Array.isArray(value)) { + return undefined; + } + const extra: Record = {}; + for (const [key, raw] of Object.entries(value as Record)) { + if (typeof raw === "string") { + extra[key] = raw; + } + } + return extra; +} + +export function logCliCommand( + log: PatchloomLog | undefined, + trace: string, + binary: string, + args: readonly string[], + cwd: string +): void { + if (shouldLogCliCommands(trace)) { + log?.logCommand(binary, args, cwd); + } +} + +export function logCliResult( + log: PatchloomLog | undefined, + trace: string, + exitCode: number, + stdout: string, + stderr: string +): void { + if (!shouldLogCliCommands(trace)) { + return; + } + if (shouldLogCliStreams(trace)) { + log?.logResult(exitCode, stdout, stderr); + return; + } + log?.logResult(exitCode, "", ""); +} + export interface OutputChannelLike { appendLine(value: string): void; show(preserveFocus?: boolean): void; diff --git a/src/util.ts b/src/util.ts index c5cf291..44f43d5 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,3 +1,24 @@ +/** Overlay `patchloom.env` onto a process env. Empty/undefined extra is identity. */ +export function mergePatchloomEnv( + base: NodeJS.ProcessEnv, + extra: Record | undefined +): NodeJS.ProcessEnv { + if (extra === undefined || Object.keys(extra).length === 0) { + return base; + } + return { ...base, ...extra }; +} + +/** `messages` and `verbose` log command lines and exit codes. */ +export function shouldLogCliCommands(trace: string): boolean { + return trace === "messages" || trace === "verbose"; +} + +/** Only `verbose` dumps CLI stdout/stderr into the output channel. */ +export function shouldLogCliStreams(trace: string): boolean { + return trace === "verbose"; +} + export function formatError(error: unknown): string { if (error instanceof Error && error.message.length > 0) { return error.message; diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index d45e5c2..8c67263 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -11,7 +11,13 @@ import { import { buildStatusDetails, preferredStatusAction } from "../../src/commands/showStatus.js"; import { buildPatchloomMcpEntry, configureMcpTargets, inspectMcpTargets } from "../../src/mcp/config.js"; import { setPatchloomLog } from "../../src/logging/outputChannel.js"; -import { formatCliOutput, formatError } from "../../src/util.js"; +import { + formatCliOutput, + formatError, + mergePatchloomEnv, + shouldLogCliCommands, + shouldLogCliStreams +} from "../../src/util.js"; test("formatError extracts message from Error instances", () => { assert.equal(formatError(new Error("disk full")), "disk full"); @@ -29,6 +35,46 @@ test("formatError falls back to String for Error with empty message", () => { assert.equal(formatError(err), String(err)); }); +test("mergePatchloomEnv is identity when extra is undefined", () => { + const base = { PATH: "/usr/bin", HOME: "/home/user" }; + assert.equal(mergePatchloomEnv(base, undefined), base); +}); + +test("mergePatchloomEnv is identity when extra is empty", () => { + const base = { PATH: "/usr/bin", HOME: "/home/user" }; + assert.equal(mergePatchloomEnv(base, {}), base); +}); + +test("mergePatchloomEnv adds new keys from extra", () => { + const merged = mergePatchloomEnv( + { PATH: "/usr/bin" }, + { PATCHLOOM_LOG: "debug" } + ); + assert.equal(merged.PATH, "/usr/bin"); + assert.equal(merged.PATCHLOOM_LOG, "debug"); +}); + +test("mergePatchloomEnv overwrites matching keys from extra", () => { + const merged = mergePatchloomEnv( + { PATH: "/usr/bin", PATCHLOOM_LOG: "info" }, + { PATCHLOOM_LOG: "debug" } + ); + assert.equal(merged.PATH, "/usr/bin"); + assert.equal(merged.PATCHLOOM_LOG, "debug"); +}); + +test("shouldLogCliCommands is true for messages and verbose", () => { + assert.equal(shouldLogCliCommands("off"), false); + assert.equal(shouldLogCliCommands("messages"), true); + assert.equal(shouldLogCliCommands("verbose"), true); +}); + +test("shouldLogCliStreams is true only for verbose", () => { + assert.equal(shouldLogCliStreams("off"), false); + assert.equal(shouldLogCliStreams("messages"), false); + assert.equal(shouldLogCliStreams("verbose"), true); +}); + // --- #36: direct unit tests for formatCliOutput --- test("formatCliOutput merges stderr and stdout into a single line", () => { @@ -651,6 +697,85 @@ test("buildAgentRulesArgs includes --surface core (CLI 0.24+)", () => { ); }); +test("generateAgentRules merges extraEnv into execFile env", async () => { + let seenEnv: NodeJS.ProcessEnv | undefined; + const output = await generateAgentRules("/fake/patchloom", "/tmp", {}, { + extraEnv: { PATCHLOOM_LOG: "debug" }, + processEnv: { PATH: "/usr/bin", HOME: "/home/user" }, + execFile: async (_file, _args, options) => { + seenEnv = options.env; + return { stdout: "# rules\n", stderr: "" }; + } + }); + assert.equal(output, "# rules\n"); + assert.ok(seenEnv); + assert.equal(seenEnv.PATH, "/usr/bin"); + assert.equal(seenEnv.HOME, "/home/user"); + assert.equal(seenEnv.PATCHLOOM_LOG, "debug"); +}); + +test("generateAgentRules logs command and streams when trace is verbose", async () => { + const commands: { binary: string; args: readonly string[]; cwd: string }[] = []; + const logged: { exitCode: number; stdout: string; stderr: string }[] = []; + setPatchloomLog({ + log() {}, + logCommand(binary, args, cwd) { commands.push({ binary, args, cwd }); }, + logResult(exitCode, stdout, stderr) { logged.push({ exitCode, stdout, stderr }); }, + show() {}, + dispose() {} + }); + try { + await generateAgentRules("/fake/patchloom", "/tmp", {}, { + trace: "verbose", + execFile: async () => ({ stdout: "# rules\n", stderr: "note" }) + }); + assert.equal(commands.length, 1); + assert.equal(commands[0].binary, "/fake/patchloom"); + assert.deepEqual(commands[0].args, ["agent-rules"]); + assert.equal(logged.length, 1); + assert.equal(logged[0].stdout, "# rules\n"); + assert.equal(logged[0].stderr, "note"); + } finally { + setPatchloomLog(undefined); + } +}); + +test("generateAgentRules skips CLI logs when trace is off", async () => { + let commands = 0; + let results = 0; + setPatchloomLog({ + log() {}, + logCommand() { commands += 1; }, + logResult() { results += 1; }, + show() {}, + dispose() {} + }); + try { + await generateAgentRules("/fake/patchloom", "/tmp", {}, { + execFile: async () => ({ stdout: "# rules\n", stderr: "" }) + }); + assert.equal(commands, 0); + assert.equal(results, 0); + } finally { + setPatchloomLog(undefined); + } +}); + +test("generateAgentRules extraEnv overwrites processEnv keys", async () => { + let seenEnv: NodeJS.ProcessEnv | undefined; + await generateAgentRules("/fake/patchloom", "/tmp", {}, { + extraEnv: { PATCHLOOM_LOG: "debug" }, + processEnv: { PATH: "/usr/bin", PATCHLOOM_LOG: "info" }, + execFile: async (_file, _args, options) => { + seenEnv = options.env; + return { stdout: "# rules\n", stderr: "" }; + } + }); + assert.ok(seenEnv); + assert.equal(seenEnv.PATCHLOOM_LOG, "debug"); + assert.equal(seenEnv.PATH, "/usr/bin"); +}); + test("generateAgentRules logs error to output channel on CLI failure", async () => { const logged: { exitCode: number; stdout: string; stderr: string }[] = []; const commands: { binary: string; args: readonly string[]; cwd: string }[] = []; @@ -663,7 +788,9 @@ test("generateAgentRules logs error to output channel on CLI failure", async () }); try { await assert.rejects( - () => generateAgentRules("/nonexistent/patchloom", "/tmp", { mode: "mcp", platform: "linux" }), + () => generateAgentRules("/nonexistent/patchloom", "/tmp", { mode: "mcp", platform: "linux" }, { + trace: "verbose" + }), (err: Error) => { assert.match(err.message, /ENOENT|not found|No such file/i); return true; @@ -704,6 +831,7 @@ test("generateAgentRules surfaces formatCliOutput envelope on CLI failure", asyn try { await assert.rejects( () => generateAgentRules("/fake/patchloom", "/tmp", {}, { + trace: "verbose", execFile: async () => { throw execError; } diff --git a/test/unit/outputChannel.test.ts b/test/unit/outputChannel.test.ts index ebca18d..5a94a2e 100644 --- a/test/unit/outputChannel.test.ts +++ b/test/unit/outputChannel.test.ts @@ -1,6 +1,12 @@ import assert from "node:assert/strict"; import test from "node:test"; -import { createPatchloomLog, getPatchloomLog, setPatchloomLog } from "../../src/logging/outputChannel.js"; +import { + createPatchloomLog, + getPatchloomLog, + logCliCommand, + logCliResult, + setPatchloomLog +} from "../../src/logging/outputChannel.js"; test("createPatchloomLog lazily creates channel on first log call", () => { let channelCreated = false; @@ -169,3 +175,67 @@ test("logResult handles multiline stderr", () => { assert.ok(lines.some(l => l === "stderr: error line 2")); assert.ok(lines.some(l => l.includes("Exit code: 1"))); }); + +test("logCliResult skips when trace is off", () => { + let called = false; + const log = createPatchloomLog(() => ({ + appendLine() { called = true; }, + show() {}, + dispose() {} + })); + + logCliResult(log, "off", 1, "stdout-body", "stderr-body"); + assert.equal(called, false); +}); + +test("logCliCommand skips when trace is off", () => { + let called = false; + const log = createPatchloomLog(() => ({ + appendLine() { called = true; }, + show() {}, + dispose() {} + })); + + logCliCommand(log, "off", "/bin/patchloom", ["agent-rules"], "/tmp"); + assert.equal(called, false); +}); + +test("logCliCommand writes when trace is messages", () => { + const lines: string[] = []; + const log = createPatchloomLog(() => ({ + appendLine(value: string) { lines.push(value); }, + show() {}, + dispose() {} + })); + + logCliCommand(log, "messages", "/bin/patchloom", ["agent-rules"], "/tmp"); + assert.ok(lines.some(l => l.includes("agent-rules"))); +}); + +test("logCliResult omits streams when trace is messages", () => { + const lines: string[] = []; + const log = createPatchloomLog(() => ({ + appendLine(value: string) { lines.push(value); }, + show() {}, + dispose() {} + })); + + logCliResult(log, "messages", 1, "stdout-body", "stderr-body"); + assert.equal(lines.length, 1); + assert.ok(lines[0].includes("Exit code: 1")); + assert.ok(!lines.some(l => l.includes("stdout-body") || l.includes("stderr-body"))); +}); + +test("logCliResult dumps streams when trace is verbose", () => { + const lines: string[] = []; + const log = createPatchloomLog(() => ({ + appendLine(value: string) { lines.push(value); }, + show() {}, + dispose() {} + })); + + logCliResult(log, "verbose", 0, "stdout-body", "stderr-body"); + assert.ok(lines.some(l => l === "stdout-body")); + assert.ok(lines.some(l => l === "stderr: stderr-body")); + assert.ok(lines.some(l => l.includes("Exit code: 0"))); +}); diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 56d0bee..46b56ad 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -342,21 +342,21 @@ test("resolveWorkspaceRelativePath accepts nested subdirectory", () => { test("resolveWorkspaceRelativePath rejects traversal with ..", () => { assert.throws( () => resolveWorkspaceRelativePath("/workspace/demo", "/workspace/demo/../../etc/passwd"), - { message: "File path must stay inside the current workspace folder." } + { message: "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." } ); }); test("resolveWorkspaceRelativePath rejects absolute path outside workspace", () => { assert.throws( () => resolveWorkspaceRelativePath("/workspace/demo", "/tmp/evil.txt"), - { message: "File path must stay inside the current workspace folder." } + { message: "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." } ); }); test("resolveWorkspaceRelativePath rejects workspace root itself", () => { assert.throws( () => resolveWorkspaceRelativePath("/workspace/demo", "/workspace/demo"), - { message: "File path must stay inside the current workspace folder." } + { message: "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." } ); }); diff --git a/walkthrough/initialize.md b/walkthrough/initialize.md index 0c02748..405f3e9 100644 --- a/walkthrough/initialize.md +++ b/walkthrough/initialize.md @@ -11,10 +11,9 @@ CLI 0.29+, the core pack text follows the mode you pick. ## What AGENTS.md Contains -- Project structure and conventions -- Build and test commands -- Coding style guidelines -- Architecture notes +`AGENTS.md` is Patchloom agent-rules text: how to call the CLI and MCP +tools, command examples, and integration notes. It is not a generic +project overview (structure, build commands, or coding style). The file is placed at the root of your workspace and works with GitHub Copilot, Claude Code, Grok, Cursor, and other AI coding tools. From 94115c794d099f7caa53765c8953c6f1cd02049d Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 13:56:00 -0700 Subject: [PATCH 4/8] chore: pin vscode types to engines and audit devDependencies Pin @types/vscode to ~1.90.0 so the lock cannot float past engines.vscode (^1.90.0). vsce rejects a declared @types/vscode range newer than engines. Bump @types/node to 26.4.0. Audit the full graph in CI (including devDependencies) because the production graph is empty. Refresh AGENTS.md unit test counts to match grep. Signed-off-by: Sebastien Tardif --- .github/workflows/security.yml | 4 ++-- AGENTS.md | 6 +++--- package-lock.json | 16 ++++++++-------- package.json | 4 ++-- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index c51bc7a..2e288ea 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -35,8 +35,8 @@ jobs: - name: Setup Node.js uses: ./.github/actions/setup-node - - name: Audit production dependencies - run: npm audit --audit-level=high --omit=dev + - name: Audit dependencies + run: npm audit --audit-level=high trivy: name: Trivy vulnerability scan diff --git a/AGENTS.md b/AGENTS.md index e263892..e9dd6ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,11 +51,11 @@ test/ batchApply.test.ts Batch template and operation count parsing (16 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (66 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) - initializeProject.test.ts Status display, agents file classification, formatError (43 tests) + initializeProject.test.ts Status display, agents file classification, formatError (57 tests) managedLifecycle.test.ts Managed install with real file I/O (22 tests) mcpConfig.test.ts MCP config with real temp directories (12 tests) - outputChannel.test.ts Output channel logging wrapper (10 tests) - patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (44 tests incl. e2e) + outputChannel.test.ts Output channel logging wrapper (15 tests) + patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (46 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 (62 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) diff --git a/package-lock.json b/package-lock.json index 77bac3c..5bd164d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,8 +10,8 @@ "license": "MIT", "devDependencies": { "@types/mocha": "^10.0.10", - "@types/node": "^26.2.0", - "@types/vscode": "^1.90.0", + "@types/node": "^26.4.0", + "@types/vscode": "~1.90.0", "@vscode/test-electron": "^3.1.0", "@vscode/vsce": "^3.0.0", "fast-check": "^4.9.0", @@ -1676,9 +1676,9 @@ "license": "MIT" }, "node_modules/@types/node": { - "version": "26.2.0", - "resolved": "https://registry.npmjs.org/@types/node/-/node-26.2.0.tgz", - "integrity": "sha512-5IviulTZeRNp2vAJ514cc/HUlY5nZ9fCbq9DMyC52BrhFZACo3nI0R7qBxhQmo/d27NFe96ur/b7Wwxklda+kg==", + "version": "26.4.0", + "resolved": "https://registry.npmjs.org/@types/node/-/node-26.4.0.tgz", + "integrity": "sha512-faiGnoIrLH/V8cibOMEAZ8pMw6oXqSukl29ra4mN8GdaB2ZewzeaLj+INpV5N+Z1eKWzY+IzaIZH2EIR6YZRNQ==", "dev": true, "license": "MIT", "dependencies": { @@ -1711,9 +1711,9 @@ } }, "node_modules/@types/vscode": { - "version": "1.125.0", - "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.125.0.tgz", - "integrity": "sha512-0icm/ZQAaism87P0ekHqi4/Ju9du+Tm0RUW+y7vqRsxY2cY0FNRX1nAnaW7nT6npPt2tfHiheZ55Zm9UhqonFA==", + "version": "1.90.0", + "resolved": "https://registry.npmjs.org/@types/vscode/-/vscode-1.90.0.tgz", + "integrity": "sha512-oT+ZJL7qHS9Z8bs0+WKf/kQ27qWYR3trsXpq46YDjFqBsMLG4ygGGjPaJ2tyrH0wJzjOEmDyg9PDJBBhWg9pkQ==", "dev": true, "license": "MIT" }, diff --git a/package.json b/package.json index 5f81778..a9cb35e 100644 --- a/package.json +++ b/package.json @@ -261,8 +261,8 @@ }, "devDependencies": { "@types/mocha": "^10.0.10", - "@types/node": "^26.2.0", - "@types/vscode": "^1.90.0", + "@types/node": "^26.4.0", + "@types/vscode": "~1.90.0", "@vscode/test-electron": "^3.1.0", "@vscode/vsce": "^3.0.0", "fast-check": "^4.9.0", From e56f61d795cb675d3759529bd6c7cf6992a01b22 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 14:04:57 -0700 Subject: [PATCH 5/8] fix: restrict patchloom.env and compile UI tests in CI Only PATCHLOOM_* keys overlay the CLI env. Workspace env is ignored in untrusted folders. Patch merge keeps --contain when the patch lives in the workspace. CI compiles out-uitest before ExTester. Signed-off-by: Sebastien Tardif --- .github/workflows/ci.yml | 2 +- AGENTS.md | 4 ++-- README.md | 2 +- package.json | 3 ++- src/commands/quickActions.ts | 17 +++++++++++--- src/logging/outputChannel.ts | 9 ++++++-- src/util.ts | 36 +++++++++++++++++++++++++++-- test/unit/initializeProject.test.ts | 35 ++++++++++++++++++++++++++++ test/unit/quickActions.test.ts | 6 +++++ 9 files changed, 102 insertions(+), 12 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 375e6b1..c50693a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -157,7 +157,7 @@ jobs: uses: ./.github/actions/setup-node - name: Compile extension and tests - run: npm run compile && npm run compile-tests + run: npm run compile && npm run compile-tests && npm run compile-uitests - name: Run extension integration tests (Linux) if: runner.os == 'Linux' diff --git a/AGENTS.md b/AGENTS.md index e9dd6ce..ad24684 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -51,13 +51,13 @@ test/ batchApply.test.ts Batch template and operation count parsing (16 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (66 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) - initializeProject.test.ts Status display, agents file classification, formatError (57 tests) + initializeProject.test.ts Status display, agents file classification, formatError (61 tests) managedLifecycle.test.ts Managed install with real file I/O (22 tests) mcpConfig.test.ts MCP config with real temp directories (12 tests) outputChannel.test.ts Output channel logging wrapper (15 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (46 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 (62 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (63 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (9 tests) suite/ diff --git a/README.md b/README.md index 14d6048..16b43fe 100644 --- a/README.md +++ b/README.md @@ -157,7 +157,7 @@ The extension detects outdated CLI builds and warns with upgrade guidance. It re | `patchloom.showStatusBar` | `true` | Show a status bar item reporting whether Patchloom is available. | | `patchloom.enable` | `true` | Enable the extension. When disabled, the status bar is hidden and background checks are skipped. | | `patchloom.trace.server` | `"off"` | Trace level for CLI output (`off`, `messages`, `verbose`). | -| `patchloom.env` | `{}` | Additional environment variables passed to the CLI (e.g., `{"PATCHLOOM_LOG": "debug"}`). | +| `patchloom.env` | `{}` | Extra `PATCHLOOM_*` variables for the CLI (for example `{"PATCHLOOM_LOG": "debug"}`). Workspace values apply only in a trusted folder. | | `patchloom.managedInstall.autoUpdate` | `true` | Automatically check for CLI updates on activation. | --- diff --git a/package.json b/package.json index a9cb35e..757601b 100644 --- a/package.json +++ b/package.json @@ -227,7 +227,8 @@ "patchloom.env": { "type": "object", "default": {}, - "markdownDescription": "Additional environment variables passed to the Patchloom CLI. For example, `{\"PATCHLOOM_LOG\": \"debug\"}` enables debug logging in the CLI." + "restricted": true, + "markdownDescription": "Additional `PATCHLOOM_*` environment variables passed to the CLI (for example `{\"PATCHLOOM_LOG\": \"debug\"}`). Workspace values apply only in a trusted folder. Loader keys such as PATH and LD_PRELOAD are ignored." }, "patchloom.managedInstall.autoUpdate": { "type": "boolean", diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 3e7a39d..107ae64 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -994,8 +994,10 @@ export async function runQuickAction(): Promise { } const action = buildPatchMergeQuickAction(patchUri[0].fsPath, allowConflicts.allow); - // Patch files may live outside the workspace; skip --contain so external patches work. - const result = await executePatchloom(binaryPath, action.args, folder.uri.fsPath, { contain: false }); + // External patch files are meta-inputs; --contain rejects them. Keep the + // write sandbox when the patch itself lives inside the workspace. + const contain = isPathInsideWorkspace(folder.uri.fsPath, patchUri[0].fsPath); + const result = await executePatchloom(binaryPath, action.args, folder.uri.fsPath, { contain }); const log = getPatchloomLog(); if (result.exitCode === 8) { @@ -1013,7 +1015,7 @@ export async function runQuickAction(): Promise { { label: "Undo last change", description: "Restore files from the last patchloom backup", - detail: "Runs `patchloom undo`", + detail: "Runs `patchloom undo --apply`", run: async () => { const folder = await activeWorkspaceFolder({ promptIfMany: true, @@ -1563,6 +1565,15 @@ export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath return relativePath.split(path.sep).join("/"); } +export function isPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { + try { + resolveWorkspaceRelativePath(workspaceRoot, absolutePath); + return true; + } catch { + return false; + } +} + function toWorkspaceFileTarget(folder: VSCode.WorkspaceFolder, absolutePath: string): WorkspaceFileTarget { const relativePath = resolveWorkspaceRelativePath(folder.uri.fsPath, absolutePath); diff --git a/src/logging/outputChannel.ts b/src/logging/outputChannel.ts index 8ab68e7..0e08d4d 100644 --- a/src/logging/outputChannel.ts +++ b/src/logging/outputChannel.ts @@ -1,4 +1,4 @@ -import { shouldLogCliCommands, shouldLogCliStreams } from "../util.js"; +import { resolvePatchloomEnvFromInspect, shouldLogCliCommands, shouldLogCliStreams } from "../util.js"; export interface PatchloomRuntimeConfig { readonly extraEnv: Record | undefined; @@ -8,8 +8,13 @@ export interface PatchloomRuntimeConfig { export async function getPatchloomRuntimeConfig(): Promise { const vscode = await import("vscode"); const config = vscode.workspace.getConfiguration("patchloom"); + const envSource = resolvePatchloomEnvFromInspect( + vscode.workspace.isTrusted, + config.inspect("env"), + config.get("env") + ); return { - extraEnv: asStringEnv(config.get("env")), + extraEnv: asStringEnv(envSource), trace: config.get("trace.server", "off") ?? "off" }; } diff --git a/src/util.ts b/src/util.ts index 44f43d5..e2d9dda 100644 --- a/src/util.ts +++ b/src/util.ts @@ -1,4 +1,8 @@ -/** Overlay `patchloom.env` onto a process env. Empty/undefined extra is identity. */ +/** Overlay allowed `patchloom.env` keys onto a process env. Only `PATCHLOOM_*` keys apply. */ +export function isAllowedPatchloomEnvKey(key: string): boolean { + return key.startsWith("PATCHLOOM_"); +} + export function mergePatchloomEnv( base: NodeJS.ProcessEnv, extra: Record | undefined @@ -6,7 +10,35 @@ export function mergePatchloomEnv( if (extra === undefined || Object.keys(extra).length === 0) { return base; } - return { ...base, ...extra }; + const allowed: Record = {}; + for (const [key, value] of Object.entries(extra)) { + if (isAllowedPatchloomEnvKey(key)) { + allowed[key] = value; + } + } + if (Object.keys(allowed).length === 0) { + return base; + } + return { ...base, ...allowed }; +} + +/** + * Workspace/folder env applies only in a trusted workspace. Untrusted + * folders may still set user/global `patchloom.env`. + */ +export function resolvePatchloomEnvFromInspect( + isTrusted: boolean, + inspect: { + globalValue?: unknown; + workspaceValue?: unknown; + workspaceFolderValue?: unknown; + } | undefined, + merged: unknown +): unknown { + if (isTrusted) { + return merged; + } + return inspect?.globalValue; } /** `messages` and `verbose` log command lines and exit codes. */ diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index 8c67263..723b03c 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -14,7 +14,9 @@ import { setPatchloomLog } from "../../src/logging/outputChannel.js"; import { formatCliOutput, formatError, + isAllowedPatchloomEnvKey, mergePatchloomEnv, + resolvePatchloomEnvFromInspect, shouldLogCliCommands, shouldLogCliStreams } from "../../src/util.js"; @@ -63,6 +65,39 @@ test("mergePatchloomEnv overwrites matching keys from extra", () => { assert.equal(merged.PATCHLOOM_LOG, "debug"); }); +test("mergePatchloomEnv ignores loader keys such as PATH and LD_PRELOAD", () => { + const merged = mergePatchloomEnv( + { PATH: "/usr/bin" }, + { PATH: "/tmp/evil/bin", LD_PRELOAD: "/tmp/evil.so", PATCHLOOM_LOG: "debug" } + ); + assert.equal(merged.PATH, "/usr/bin"); + assert.equal(merged.LD_PRELOAD, undefined); + assert.equal(merged.PATCHLOOM_LOG, "debug"); +}); + +test("isAllowedPatchloomEnvKey accepts only PATCHLOOM_ prefix", () => { + assert.equal(isAllowedPatchloomEnvKey("PATCHLOOM_LOG"), true); + assert.equal(isAllowedPatchloomEnvKey("PATH"), false); + assert.equal(isAllowedPatchloomEnvKey("DYLD_INSERT_LIBRARIES"), false); +}); + +test("resolvePatchloomEnvFromInspect uses merged env when trusted", () => { + const merged = { PATCHLOOM_LOG: "workspace" }; + const inspect = { globalValue: { PATCHLOOM_LOG: "user" }, workspaceValue: merged }; + assert.deepEqual(resolvePatchloomEnvFromInspect(true, inspect, merged), merged); +}); + +test("resolvePatchloomEnvFromInspect ignores workspace env when untrusted", () => { + const inspect = { + globalValue: { PATCHLOOM_LOG: "user" }, + workspaceValue: { PATH: "/tmp/evil" } + }; + assert.deepEqual( + resolvePatchloomEnvFromInspect(false, inspect, inspect.workspaceValue), + { PATCHLOOM_LOG: "user" } + ); +}); + test("shouldLogCliCommands is true for messages and verbose", () => { assert.equal(shouldLogCliCommands("off"), false); assert.equal(shouldLogCliCommands("messages"), true); diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 46b56ad..c8af1e5 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -28,6 +28,7 @@ import { buildUndoQuickAction, isMarkdownPath, isStructuredDocumentPath, + isPathInsideWorkspace, resolveWorkspaceRelativePath, retargetQuickAction, withApplyFlag, @@ -339,6 +340,11 @@ test("resolveWorkspaceRelativePath accepts nested subdirectory", () => { assert.equal(rel, "a/b/c/d.txt"); }); +test("isPathInsideWorkspace is true only for paths under the workspace", () => { + assert.equal(isPathInsideWorkspace("/workspace/demo", "/workspace/demo/changes.patch"), true); + assert.equal(isPathInsideWorkspace("/workspace/demo", "/tmp/outside.patch"), false); +}); + test("resolveWorkspaceRelativePath rejects traversal with ..", () => { assert.throws( () => resolveWorkspaceRelativePath("/workspace/demo", "/workspace/demo/../../etc/passwd"), From fa26fdb4972f5beeb85ac04ad13020c8a4790713 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 14:11:06 -0700 Subject: [PATCH 6/8] fix: treat .. as a path component and compile UI tests in npm test Filenames such as ..changes.patch stay inside the workspace sandbox. isPathInsideWorkspace compares resolved prefixes (case-fold on macOS/Windows). npm test now compiles out-uitest so it matches CI. Signed-off-by: Sebastien Tardif --- AGENTS.md | 4 ++-- package.json | 2 +- src/commands/quickActions.ts | 20 +++++++++++++------- test/unit/quickActions.test.ts | 7 +++++++ 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ad24684..9d5a159 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,7 +15,7 @@ Patchloom for VS Code is the official VS Code extension for [Patchloom](https:// | `npm run test:extension` | Run VS Code extension integration tests | | `npm run test:ui` | Run ExTester UI tests (downloads VS Code if needed) | | `npm run test:all` | Compile + unit tests + extension integration tests | -| `npm run test` | Compile + compile-tests + unit tests | +| `npm run test` | Compile + compile-tests + compile-uitests + unit tests | | `npm run test:coverage` | Unit tests with line coverage (80% threshold) | | `npm run package` | Package the `.vsix` using `@vscode/vsce` | | `npm run check` | Full CI gate: test + coverage + package | @@ -57,7 +57,7 @@ test/ outputChannel.test.ts Output channel logging wrapper (15 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (46 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 (63 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (64 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (9 tests) suite/ diff --git a/package.json b/package.json index 757601b..96e7900 100644 --- a/package.json +++ b/package.json @@ -247,7 +247,7 @@ "test:coverage": "node --test --experimental-test-coverage --test-coverage-lines=80 --test-coverage-exclude='out-test/src/extension*' --test-coverage-exclude='out-test/src/commands/showStatus*' --test-coverage-exclude='out-test/src/commands/configureMcp*' --test-coverage-exclude='out-test/src/commands/setupWorkspace*' --test-coverage-exclude='out-test/src/commands/batchApply*' --test-coverage-exclude='out-test/src/commands/quickActions*' --test-coverage-exclude='out-test/src/status/statusBar*' --test-coverage-exclude='out-test/test/unit/patchloomCli*' ./out-test/test/unit/*.test.js", "test:extension": "node ./out-test/test/suite/runExtensionTests.js", "test:ui": "npm run compile && npm run compile-uitests && extest setup-and-run './out-uitest/test/ui/*.test.js' --code_version max --extensions_dir .vscode-test/extensions", - "test": "npm run compile && npm run compile-tests && npm run test:unit", + "test": "npm run compile && npm run compile-tests && npm run compile-uitests && npm run test:unit", "test:all": "npm run compile && npm run compile-tests && npm run test:unit && npm run test:extension", "vscode:prepublish": "npm run compile", "package": "vsce package", diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 107ae64..7052a2f 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -1553,11 +1553,15 @@ 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 || relativePath.startsWith("..") || path.isAbsolute(relativePath)) { + 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." ); @@ -1566,12 +1570,14 @@ export function resolveWorkspaceRelativePath(workspaceRoot: string, absolutePath } export function isPathInsideWorkspace(workspaceRoot: string, absolutePath: string): boolean { - try { - resolveWorkspaceRelativePath(workspaceRoot, absolutePath); - return true; - } catch { - return false; - } + 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}`); } function toWorkspaceFileTarget(folder: VSCode.WorkspaceFolder, absolutePath: string): WorkspaceFileTarget { diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index c8af1e5..823178d 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -343,6 +343,13 @@ test("resolveWorkspaceRelativePath accepts nested subdirectory", () => { test("isPathInsideWorkspace is true only for paths under the workspace", () => { assert.equal(isPathInsideWorkspace("/workspace/demo", "/workspace/demo/changes.patch"), true); assert.equal(isPathInsideWorkspace("/workspace/demo", "/tmp/outside.patch"), false); + assert.equal(isPathInsideWorkspace("/workspace/demo", "/workspace/demo/..changes.patch"), true); + assert.equal(isPathInsideWorkspace("/workspace/demo", "/workspace/other/fix.patch"), false); +}); + +test("resolveWorkspaceRelativePath allows a filename that starts with ..", () => { + const rel = resolveWorkspaceRelativePath("/workspace/demo", "/workspace/demo/..changes.patch"); + assert.equal(rel, "..changes.patch"); }); test("resolveWorkspaceRelativePath rejects traversal with ..", () => { From eb2c1e5d7db09acbd392700b26de23b482d675ae Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 14:18:55 -0700 Subject: [PATCH 7/8] fix: activate the extension before ExTester status-bar polls UI tests now compile in CI. The suite opens a workspace without AGENTS.md, so content-based activation never ran and before() timed out. Invoke Show Status first to load the extension. Signed-off-by: Sebastien Tardif --- test/ui/extension.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/ui/extension.test.ts b/test/ui/extension.test.ts index 08fc809..16b8066 100644 --- a/test/ui/extension.test.ts +++ b/test/ui/extension.test.ts @@ -79,8 +79,11 @@ describe("Patchloom Extension UI", function () { this.timeout(90_000); await VSBrowser.instance.waitForWorkbench(); workbench = new Workbench(); - // Wait for the extension to activate and create its status bar item - await poll(findPatchloomStatus, 15_000); + // Content-based activation (AGENTS.md / .patchloom.toml) may not fire in + // the ExTester workspace. A Patchloom command loads the extension so the + // status bar item exists before the suite runs. + await workbench.executeCommand("Patchloom: Show Status"); + await poll(findPatchloomStatus, 30_000); }); describe("Status bar", function () { From f88e3b2abcae865aba3435b3d0d8c33f646beddd Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 27 Aug 2026 14:24:07 -0700 Subject: [PATCH 8/8] fix: wait for real Quick Action picks in ExTester The command palette InputBox was treated as the Quick Action picker when getQuickPicks() returned empty labels. Keep polling until a Patchloom notification or a Replace/Tidy pick appears. Signed-off-by: Sebastien Tardif --- test/ui/extension.test.ts | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/test/ui/extension.test.ts b/test/ui/extension.test.ts index 16b8066..1af7823 100644 --- a/test/ui/extension.test.ts +++ b/test/ui/extension.test.ts @@ -156,11 +156,14 @@ describe("Patchloom Extension UI", function () { const picks = await input.getQuickPicks(); const labels: string[] = []; for (const pick of picks) labels.push(await pick.getLabel()); - return { kind: "input", labels }; + if (labels.some((label) => label.includes("Replace") || label.includes("Tidy"))) { + return { kind: "input", labels }; + } + return undefined; } catch { return undefined; // keep polling } - }); + }, 15_000); assert.ok(result, "Quick Action should produce a notification or quick pick"); if (result.kind === "input") {