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: diff --git a/AGENTS.md b/AGENTS.md index 4801816..0ef0fe8 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,19 +50,19 @@ 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 (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 (71 tests) - managedLifecycle.test.ts Managed install with real file I/O (26 tests) - mcpConfig.test.ts MCP config with real temp directories (16 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) 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) 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 (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) @@ -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 a5d3303..5c5554d 100644 --- a/README.md +++ b/README.md @@ -59,11 +59,12 @@ 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`) -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. @@ -71,11 +72,11 @@ 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-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 @@ -119,11 +120,11 @@ Click it to see full diagnostics, including per-editor MCP configuration status | **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 -`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 --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 @@ -143,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 | @@ -202,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. @@ -257,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). @@ -275,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/batchApply.ts b/src/commands/batchApply.ts index 80bbe9b..6ac5c3c 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", @@ -36,10 +37,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. 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 { - 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. */ @@ -47,9 +53,166 @@ 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 BatchUnchangedMutation { + readonly path: string; + readonly op: string; +} + +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[]; +} + +/** 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; + files_renamed?: unknown; + match_count?: unknown; + refused?: unknown; + mutations?: 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, + 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) + }; + } 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 (`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); + if (!report) { + return { + warning: false, + message: `Batch apply completed: ${operationCount} operation(s) applied.` + }; + } + + const tally = describeBatchFileTally(report); + 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}. ${missed.length} operation(s) were not applied: ${missed.join(", ")}.` + }; + } + 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`); + } + if (report.filesRenamed > 0) { + parts.push(`${report.filesRenamed} file(s) renamed`); + } + 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"; + } + return reason.replaceAll("_", " "); } export async function batchApply(): Promise { @@ -108,10 +271,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/src/commands/configureMcp.ts b/src/commands/configureMcp.ts index 5433321..3afef7c 100644 --- a/src/commands/configureMcp.ts +++ b/src/commands/configureMcp.ts @@ -2,9 +2,15 @@ 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 { getPatchloomLog, writeUserVisibleOutput } from "../logging/outputChannel.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 { @@ -26,7 +32,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 })); @@ -72,20 +78,21 @@ 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"); } }); } catch (error) { - await vscode.window.showErrorMessage(`Failed to configure MCP: ${formatError(error)}`); + const message = formatMcpConfigureFailureMessage(error); + const log = getPatchloomLog(); + writeUserVisibleOutput(log, message); + log?.show(); + await vscode.window.showErrorMessage(message); + if (error instanceof McpConfigureError && error.completed.some((result) => result.changed)) { + await refreshStatusBar(); + } return; } @@ -100,3 +107,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/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 512d590..aa9cef8 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,18 @@ import { } from "../logging/outputChannel.js"; import { formatCliOutput, formatError, formatQuickActionCliOutput, mergePatchloomEnv } from "../util.js"; import { activeWorkspaceFolder, describeWorkspaceEnvironment } from "../workspace/readiness.js"; +import { + isRealPathInsideWorkspace, + resolveWorkspaceRelativePath, + sameRealFilePath +} from "../workspace/pathContainment.js"; + +export { + isPathInsideWorkspace, + isRealPathInsideWorkspace, + resolveWorkspaceRelativePath, + sameRealFilePath +} from "../workspace/pathContainment.js"; const execFileAsync = promisify(execFile); const STRUCTURED_FILE_EXTENSIONS = new Set([ @@ -32,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 { @@ -96,6 +117,32 @@ 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. + */ +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 } @@ -360,8 +407,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) { @@ -392,8 +439,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) { @@ -501,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 { @@ -555,6 +602,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; } @@ -632,6 +680,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; } @@ -665,6 +714,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; } @@ -698,6 +748,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; } @@ -1185,6 +1236,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."); } @@ -1237,6 +1290,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."); } @@ -1272,6 +1327,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; } @@ -1648,7 +1704,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, @@ -1663,6 +1719,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)}`, @@ -1704,11 +1761,28 @@ 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. - return !( + // Exit 3 is a soft no-op for text replace (pattern not in the file). + // 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 ( 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 { @@ -1930,88 +2004,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; @@ -2072,7 +2064,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.`, @@ -2129,13 +2121,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/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); } } 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/src/mcp/config.ts b/src/mcp/config.ts index c6b789f..a37ebd0 100644 --- a/src/mcp/config.ts +++ b/src/mcp/config.ts @@ -1,8 +1,15 @@ +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"; -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; @@ -19,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; @@ -46,6 +74,14 @@ 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) @@ -75,23 +111,49 @@ export async function configureMcpTargets(inputs: McpApplyInputs): Promise 0; + + if (hasText && isPlainObject(existingRoot) && stableJson(currentEntry) === stableJson(entry)) { + results.push({ + ...target, + exists: true, + configured: true, + changed: false + }); + continue; + } - if (changed) { + 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 - }); + results.push({ + ...target, + exists: content !== undefined, + configured: true, + changed: true + }); + } 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); + } } return results; @@ -111,6 +173,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", @@ -144,8 +211,52 @@ export function buildPatchloomMcpEntry( return entry; } +function mcpConfigRoot(target: McpTarget, workspaceFolderPath?: string): string | undefined { + if (target.kind === "windsurf-user") { + return path.dirname(target.filePath); + } + return workspaceFolderPath; +} + +function mcpConfigPathIsContained(target: McpTarget, workspaceFolderPath?: string): boolean { + const root = mcpConfigRoot(target, workspaceFolderPath); + if (!root) { + return false; + } + // A user Windsurf directory that does not exist yet has nothing to follow. + if (target.kind === "windsurf-user" && !existsSync(root)) { + return true; + } + return isRealPathInsideWorkspace(root, target.filePath); +} + +function assertMcpConfigWriteContained(target: McpTarget, workspaceFolderPath?: string): void { + if (mcpConfigPathIsContained(target, workspaceFolderPath)) { + return; + } + const root = mcpConfigRoot(target, workspaceFolderPath); + throw new Error( + root + ? `Refusing to write MCP config ${target.filePath} because it resolves outside ${root}` + : `Refusing to write MCP config ${target.filePath} because it resolves outside the workspace` + ); +} + 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( @@ -154,7 +265,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 { @@ -192,6 +303,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 {}; @@ -205,9 +338,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/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/src/workspace/pathContainment.ts b/src/workspace/pathContainment.ts new file mode 100644 index 0000000..9538ac3 --- /dev/null +++ b/src/workspace/pathContainment.ts @@ -0,0 +1,123 @@ +import { realpathSync } from "node:fs"; +import * as path from "node:path"; + +function pathEscapesRoot(relativePath: string): boolean { + return relativePath === ".." || relativePath.startsWith(`..${path.sep}`); +} + +function isResolvedPathInsideRoot(root: string, absolutePath: string): boolean { + const resolvedRoot = path.resolve(root); + const resolvedPath = path.resolve(absolutePath); + const fold = process.platform === "win32" || process.platform === "darwin" + ? (value: string) => 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); +} + +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)) { + return undefined; + } + 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) { + 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/batchApply.test.ts b/test/unit/batchApply.test.ts index f717ff3..5c6f63f 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"; @@ -15,7 +16,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"); @@ -54,6 +56,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 +75,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", () => { @@ -124,15 +139,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+)", () => { @@ -168,11 +179,111 @@ 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 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, + 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 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, + 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); }); 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..165f34a 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"; @@ -400,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, @@ -768,54 +788,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, 3, "should return vscode-workspace, cursor-workspace, windsurf-user"); - const vscode = targets.find((t) => t.kind === "vscode-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(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/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"); + }); +}); diff --git a/test/unit/mcpConfig.test.ts b/test/unit/mcpConfig.test.ts index c046bb0..08348fd 100644 --- a/test/unit/mcpConfig.test.ts +++ b/test/unit/mcpConfig.test.ts @@ -3,10 +3,13 @@ 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, + formatMcpConfigureFailureMessage, inspectMcpTargets, + McpConfigureError, resolveMcpTargets } from "../../src/mcp/config.js"; @@ -119,13 +122,111 @@ 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 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"); @@ -157,6 +258,116 @@ 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 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({ @@ -394,3 +605,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"; +} diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 3bf2a40..d187a10 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -30,12 +30,17 @@ import { buildMdUpsertBulletQuickAction, buildPatchApplyQuickAction, buildPatchMergeQuickAction, + patchCommandMadeNoChanges, buildReplaceQuickAction, buildSearchQuickAction, buildTidyQuickAction, buildUndoQuickAction, isAllowedPreviewMiss, previewMissMessage, + DOC_SET_SELECTOR_PROMPT, + DOC_SET_SELECTOR_PLACEHOLDER, + DOC_UPDATE_SELECTOR_PROMPT, + DOC_UPDATE_SELECTOR_PLACEHOLDER, isMarkdownPath, isEnvDocumentPath, isStructuredDocumentPath, @@ -44,9 +49,11 @@ import { stageExternalPatchInWorkspace, formatUndoFailureMessage, presentPatchMergeOutcome, + filesWithoutMatchNoneMessage, presentSearchOutcome, presentUndoSuccess, resolveWorkspaceRelativePath, + sameRealFilePath, retargetQuickAction, serializePatchloomArgs, withEndOfOptions, @@ -201,16 +208,63 @@ 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); }); +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); }); +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"), + 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); @@ -510,6 +564,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", () => { @@ -810,8 +894,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); }); @@ -819,7 +903,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); @@ -831,8 +915,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); }); @@ -840,8 +924,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); }); @@ -849,11 +933,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", () => { @@ -877,6 +976,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, { @@ -963,6 +1066,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-")); 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/); }); 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. 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.