diff --git a/AGENTS.md b/AGENTS.md index 567aa88..6ed9130 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -18,9 +18,9 @@ Patchloom for VS Code is the official VS Code extension for [Patchloom](https:// | `npm run test` | Compile + compile-tests + compile-uitests + unit tests | | `npm run test:coverage` | Unit tests with line coverage (80% threshold) | | `npm run package` | Package the `.vsix` using `@vscode/vsce` | -| `npm run check` | Full CI gate: test + coverage + package | +| `npm run check` | Local gate: unit tests + coverage + package (required before commit) | -Always run `npm run check` before committing. +Always run `npm run check` before committing. Merge CI also runs extension integration tests (`test:extension`) and ExTester UI tests (`test:ui`). `npm run test:all` covers compile + unit + extension tests but does not include UI tests. ## Project structure @@ -48,16 +48,17 @@ 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 (16 tests) + batchApply.test.ts Batch template and operation count parsing (18 tests) binary.test.ts Binary discovery, managed install, compatibility, workspace env (66 tests) binaryDiscovery.test.ts Real executable discovery on PATH (13 tests) initializeProject.test.ts Status display, agents file classification, formatError (61 tests) managedLifecycle.test.ts Managed install with real file I/O (22 tests) mcpConfig.test.ts MCP config with real temp directories (12 tests) - outputChannel.test.ts Output channel logging wrapper (21 tests) + mcpRegister.test.ts Native MCP definition helper for binary path (2 tests) + outputChannel.test.ts Output channel logging wrapper (22 tests) patchloomCli.test.ts Patchloom CLI integration with real binary + managed install e2e MCP (46 tests incl. e2e) propertyBased.test.ts Property-based tests with fast-check (13 tests) - quickActions.test.ts Quick action command building, path containment, patch merge (71 tests) + quickActions.test.ts Quick action command building, path containment, patch merge (73 tests) verifyMcp.test.ts MCP server verify and JSON-RPC response parsing (15 tests) downloadIntegration.test.ts HTTP download, redirect, streaming SHA-256 (9 tests) suite/ @@ -126,7 +127,7 @@ All I/O-dependent functions accept an `inputs` object with injectable callbacks - No `any` types without justification. - Pure helpers with injected I/O for testability. - Keep `extension.ts` thin. No business logic in the entrypoint. -- `npm run check` is the full gate. Nothing merges unless it passes. +- `npm run check` is the local gate (unit tests + coverage + package). Run it before every commit. Merge CI also requires extension integration and UI tests. - All relative imports must use `.js` extensions (`from "./foo.js"`, not `from "./foo"`). Required by `moduleResolution: "node16"`. - All commits require a `Signed-off-by` line (DCO). Use `git commit -s`. - When adding commands to `package.json`, update the expected count in `test/suite/index.ts`. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 87d4c95..b6a8365 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -29,13 +29,15 @@ npm test | Script | Purpose | |--------|---------| -| `npm run check` | Full CI gate (run before every commit) | +| `npm run check` | Local gate: unit tests + coverage + package (run before every commit) | | `npm run compile` | Compile extension source | | `npm run compile-tests` | Compile test source | -| `npm test` | Compile + run all tests | +| `npm test` | Compile + compile-tests + compile-uitests + unit tests | | `npm run test:unit` | Unit tests only | | `npm run test:coverage` | Unit tests with 80% line coverage threshold | | `npm run test:extension` | VS Code extension integration tests | +| `npm run test:all` | Compile + unit tests + extension integration tests (does not include UI tests) | +| `npm run test:ui` | ExTester UI tests (downloads VS Code if needed; not part of `test:all`) | | `npm run package` | Package the `.vsix` | ### Running in VS Code diff --git a/README.md b/README.md index 27a69b2..70368e4 100644 --- a/README.md +++ b/README.md @@ -122,7 +122,7 @@ Workspace Quick Actions and Batch Apply pass `--contain` so CLI paths stay insid ### Output channel -Search, search `-L`, undo, and patch-merge success or conflict details always appear in the **Patchloom** output channel. Other CLI invocations, arguments, and I/O follow `patchloom.trace.server` (`off` by default; set `verbose` to dump every command). Run `Patchloom: Show Output` to open it. +Search hits, search `-L`, undo restore details, patch-merge results (including failures), and batch-apply results always appear in the **Patchloom** output channel. Diagnostic command traces (invocations, arguments, and other CLI I/O) follow `patchloom.trace.server` (`off` by default; set `verbose` to dump every command). Run `Patchloom: Show Output` to open it. ### Compatibility diagnostics @@ -156,7 +156,7 @@ The extension detects outdated CLI builds and warns with upgrade guidance. It re | `patchloom.path` | `""` | Absolute path to the Patchloom binary. When empty, the extension searches `PATH` and then the managed install location. | | `patchloom.showStatusBar` | `true` | Show a status bar item reporting whether Patchloom is available. | | `patchloom.enable` | `true` | Enable the extension. When disabled, the status bar is hidden and background checks are skipped. | -| `patchloom.trace.server` | `"off"` | Trace level for CLI output (`off`, `messages`, `verbose`). | +| `patchloom.trace.server` | `"off"` | Diagnostic CLI trace level (`off`, `messages`, `verbose`). User-facing search, undo, patch-merge, and batch-apply streams always go to Output. | | `patchloom.env` | `{}` | Extra `PATCHLOOM_*` variables for the CLI (for example `{"PATCHLOOM_LOG": "debug"}`). Workspace values apply only in a trusted folder. | | `patchloom.managedInstall.autoUpdate` | `true` | Automatically check for CLI updates on activation. | @@ -218,7 +218,7 @@ On CLI 0.20+, sole-path loads of binary or invalid UTF-8 files report `error_kin On CLI 0.22+, over-wide fuzzy matches can report `error_kind: fuzzy_span_suspicious`. Prefer an exact `old` string, structured `doc`/`md`/`ast` edits, or `apply-fragment` with a unique anchor. **Doc selector needs multi-match op** -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` (`doc.update` or `doc.delete_where`). The extension surfaces that hint in the Output channel and notifications. Use the multi-match op (or a concrete index path such as `items.0.val`). +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` (`doc.update` or `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`). **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`). @@ -230,7 +230,7 @@ Run `Patchloom: Configure MCP` and select the target editor config. Run `Patchloom: Show Status` to see persisted diagnostic details. If the managed binary is present but not usable, choose **Reinstall Patchloom** (or the status-bar action) to re-download from GitHub Releases. **Debugging CLI errors** -Run `Patchloom: Show Output` to inspect the channel. Search, undo, and patch-merge success or conflict details are always written there; set `patchloom.trace.server` to `verbose` for other CLI invocations, arguments, stdout, and stderr. +Run `Patchloom: Show Output` to inspect the channel. Search, search `-L`, undo restore details, patch-merge results (including failures), and batch-apply streams are always written there; set `patchloom.trace.server` to `verbose` for other CLI invocations, arguments, stdout, and stderr. --- diff --git a/package.json b/package.json index 96e7900..96e7bf2 100644 --- a/package.json +++ b/package.json @@ -222,7 +222,7 @@ "messages", "verbose" ], - "markdownDescription": "Trace level for Patchloom CLI output. `messages` logs command invocations and results. `verbose` includes full stdout/stderr." + "markdownDescription": "Trace level for diagnostic Patchloom CLI logging. `messages` logs command invocations and exit codes. `verbose` includes full stdout/stderr for every command. User-facing results (search hits, search `-L`, undo restore details, patch-merge streams, and batch-apply streams) always appear in the Output channel regardless of this setting." }, "patchloom.env": { "type": "object", diff --git a/src/commands/batchApply.ts b/src/commands/batchApply.ts index 9b752dc..edf9ed7 100644 --- a/src/commands/batchApply.ts +++ b/src/commands/batchApply.ts @@ -2,7 +2,13 @@ import { execFile } from "node:child_process"; import type * as VSCode from "vscode"; import { ensurePatchloomReadyOrNotify } from "../binary/patchloom.js"; import { formatCliOutput, mergePatchloomEnv } from "../util.js"; -import { getPatchloomLog, getPatchloomRuntimeConfig, logCliCommand, logCliResult } from "../logging/outputChannel.js"; +import { + getPatchloomLog, + getPatchloomRuntimeConfig, + logCliCommand, + logCliResult, + presentCliResultInOutput +} from "../logging/outputChannel.js"; import { activeWorkspaceFolder } from "../workspace/readiness.js"; // Batch replace is PATH OLD NEW (not CLI `replace OLD --new NEW path`). See CLI 0.18+ batch --help. @@ -28,6 +34,11 @@ export function parseBatchOperationCount(plan: string): number { return plan.split("\n").filter((line) => line.trim().length > 0).length; } +/** True when the plan has no non-empty operation lines. */ +export function isEmptyBatchPlan(plan: string): boolean { + return parseBatchOperationCount(plan) === 0; +} + /** CLI argv for Batch Apply. Global --contain first (CLI 0.10+ path guard). */ export function buildBatchApplyArgs(): string[] { return ["--contain", "batch", "--apply"]; @@ -64,6 +75,13 @@ export async function batchApply(): Promise { } const plan = doc.getText(); + if (isEmptyBatchPlan(plan)) { + await vscode.window.showWarningMessage( + "Batch plan is empty. Add at least one operation." + ); + return; + } + const log = getPatchloomLog(); const runtime = await getPatchloomRuntimeConfig(); const env = mergePatchloomEnv(process.env, runtime.extraEnv); @@ -74,7 +92,7 @@ export async function batchApply(): Promise { logCliResult(log, runtime.trace, result.exitCode, result.stdout, result.stderr); if (result.exitCode !== 0) { - log?.show(); + presentCliResultInOutput(log, result); await vscode.window.showErrorMessage( `Batch apply failed: ${formatCliOutput(result)}` ); @@ -82,7 +100,7 @@ export async function batchApply(): Promise { } const ops = parseBatchOperationCount(plan); - log?.show(); + presentCliResultInOutput(log, result); await vscode.window.showInformationMessage( `Batch apply completed: ${ops} operation(s) applied.` ); diff --git a/src/commands/managedInstall.ts b/src/commands/managedInstall.ts index 990f7e9..8ef7fc8 100644 --- a/src/commands/managedInstall.ts +++ b/src/commands/managedInstall.ts @@ -8,6 +8,7 @@ import { performManagedInstall } from "../install/managed.js"; import { getPatchloomLog } from "../logging/outputChannel.js"; +import { refreshMcpServerBinary } from "../mcp/register.js"; import { refreshStatusBar } from "../status/statusBar.js"; import { formatError } from "../util.js"; @@ -67,6 +68,7 @@ export async function installPatchloom(): Promise { log?.log(`Managed install complete: Patchloom ${result.version} at ${result.binaryPath}`); await refreshStatusBar(); + await refreshMcpServerBinary(); await vscode.window.showInformationMessage( `Patchloom ${result.version} installed successfully.` ); @@ -142,6 +144,7 @@ export async function updatePatchloom(): Promise { log?.log(`Managed update complete: Patchloom ${result.version} at ${result.binaryPath}`); await refreshStatusBar(); + await refreshMcpServerBinary(); await vscode.window.showInformationMessage( `Patchloom updated to ${result.version}.` ); @@ -194,6 +197,7 @@ export async function reinstallPatchloom(): Promise { log?.log(`Managed reinstall complete: Patchloom ${result.version} at ${result.binaryPath}`); await refreshStatusBar(); + await refreshMcpServerBinary(); await vscode.window.showInformationMessage( `Patchloom ${result.version} reinstalled successfully.` ); diff --git a/src/commands/quickActions.ts b/src/commands/quickActions.ts index 82eec6f..764e12e 100644 --- a/src/commands/quickActions.ts +++ b/src/commands/quickActions.ts @@ -52,6 +52,7 @@ export function presentPatchMergeOutcome( return "conflicts"; } if (result.exitCode !== 0) { + presentCliResultInOutput(log, result); return "error"; } presentCliResultInOutput(log, result); @@ -65,6 +66,17 @@ export function presentUndoSuccess( presentCliResultInOutput(log, result); } +export function formatUndoFailureMessage(result: { + exitCode: number; + stdout: string; + stderr: string; +}): string { + if (result.stderr.includes("no backup")) { + return "No patchloom backup to undo."; + } + return `Patchloom undo failed: ${formatCliOutput(result)}`; +} + interface WorkspaceFileTarget { readonly workspaceFolder: VSCode.WorkspaceFolder; readonly absolutePath: string; @@ -1047,7 +1059,6 @@ export async function runQuickAction(): Promise { if (outcome === "conflicts") { await vscode.window.showWarningMessage("Patch merge completed with unresolved conflicts. Check the output for details."); } else if (outcome === "error") { - log?.show(); await vscode.window.showErrorMessage(`Patch merge failed: ${formatCliOutput(result)}`); } else { await vscode.window.showInformationMessage("Patch merged successfully."); @@ -1081,10 +1092,7 @@ export async function runQuickAction(): Promise { const result = await executePatchloom(binaryPath, action.args, folder.uri.fsPath); if (result.exitCode !== 0) { - const message = result.stderr.includes("no backup") - ? "No patchloom backup to undo." - : `Patchloom undo failed: ${formatCliOutput(result)}`; - await vscode.window.showWarningMessage(message); + await vscode.window.showWarningMessage(formatUndoFailureMessage(result)); return; } diff --git a/src/install/managed.ts b/src/install/managed.ts index f04cc1f..1f990c6 100644 --- a/src/install/managed.ts +++ b/src/install/managed.ts @@ -424,6 +424,49 @@ export function assertTrustedManagedInstallDownloadUrl( } } +/** Hosts GitHub release downloads may redirect to after the first hop. */ +const TRUSTED_MANAGED_INSTALL_REDIRECT_HOSTS = new Set([ + "github.com", + "objects.githubusercontent.com", + "release-assets.githubusercontent.com", + "github-releases.githubusercontent.com" +]); + +/** + * Resolve a redirect Location against the current request URL. + * Handles absolute and relative Location values. + */ +export function resolveManagedInstallRedirectUrl( + currentUrl: string, + location: string +): string { + return new URL(location, currentUrl).href; +} + +/** + * Whether a redirect target is safe for managed-install downloads. + * Allows HTTPS only on known GitHub release asset hosts (not the first-hop + * /releases/download/ path allowlist, which real CDN redirects would fail). + */ +export function isTrustedManagedInstallRedirectUrl(url: string): boolean { + try { + const parsed = new URL(url); + return parsed.protocol === "https:" + && TRUSTED_MANAGED_INSTALL_REDIRECT_HOSTS.has(parsed.hostname); + } catch { + return false; + } +} + +export function assertTrustedManagedInstallRedirectUrl(url: string): void { + if (!isTrustedManagedInstallRedirectUrl(url)) { + throw new ManagedInstallVerificationError( + "untrusted-download-url", + `Managed install redirect target is not a trusted GitHub release host: ${url}` + ); + } +} + export async function clearManagedInstallStaging( inputs: ManagedInstallStagingCleanupInputs ): Promise { @@ -773,9 +816,16 @@ export type HttpGetFn = ( callback: (response: IncomingMessage) => void ) => ClientRequest; +export interface CreateDownloaderOptions { + readonly validateRedirect?: (url: string) => void; +} + export function createDownloader( - get: HttpGetFn + get: HttpGetFn, + options?: CreateDownloaderOptions ): (url: string, destPath: string, redirectsRemaining?: number) => Promise { + const validateRedirect = options?.validateRedirect; + function download(url: string, destPath: string, redirectsRemaining = 5): Promise { return new Promise((resolve, reject) => { const file = createWriteStream(destPath); @@ -786,7 +836,17 @@ export function createDownloader( reject(new Error(`Download failed: too many redirects for ${url}`)); return; } - download(response.headers.location, destPath, redirectsRemaining - 1).then(resolve, reject); + let nextUrl: string; + try { + nextUrl = resolveManagedInstallRedirectUrl(url, response.headers.location); + if (validateRedirect) { + validateRedirect(nextUrl); + } + } catch (error) { + reject(error); + return; + } + download(nextUrl, destPath, redirectsRemaining - 1).then(resolve, reject); return; } if (response.statusCode && response.statusCode >= 400) { @@ -819,7 +879,8 @@ export function createDownloader( } const defaultDownloadToFile = createDownloader( - https.get.bind(https) as HttpGetFn + https.get.bind(https) as HttpGetFn, + { validateRedirect: assertTrustedManagedInstallRedirectUrl } ); async function defaultExecCommand(cmd: string, args: string[], cwd: string): Promise { diff --git a/src/logging/outputChannel.ts b/src/logging/outputChannel.ts index dc678d5..6b15546 100644 --- a/src/logging/outputChannel.ts +++ b/src/logging/outputChannel.ts @@ -69,6 +69,10 @@ export function writeUserVisibleOutput( if (log === undefined || text === "") { return; } + if (log.writeUserOutput !== undefined) { + log.writeUserOutput(text); + return; + } log.log(text); } @@ -92,6 +96,7 @@ export interface PatchloomLog { log(message: string): void; logCommand(binary: string, args: readonly string[], cwd: string): void; logResult(exitCode: number, stdout: string, stderr: string): void; + writeUserOutput?(text: string): void; show(): void; dispose(): void; } @@ -141,6 +146,12 @@ export function createPatchloomLog( } ch.appendLine(`[${new Date().toISOString()}] Exit code: ${exitCode}`); }, + writeUserOutput(text: string): void { + const ch = ensureChannel(); + for (const line of text.split(/\r?\n/)) { + ch.appendLine(line); + } + }, show(): void { ensureChannel().show(true); }, diff --git a/src/mcp/register.ts b/src/mcp/register.ts index f4ae0ef..ab1e567 100644 --- a/src/mcp/register.ts +++ b/src/mcp/register.ts @@ -1,15 +1,40 @@ -import * as vscode from "vscode"; +import type * as VSCode from "vscode"; import { resolvePatchloomStatus } from "../binary/patchloom.js"; import { getPatchloomLog } from "../logging/outputChannel.js"; -export function registerMcpServerProvider(context: vscode.ExtensionContext): void { +/** Pure helper for native MCP definitions (no vscode). Empty when binary unknown. */ +export function mcpServerDefinitionsForBinary( + binaryPath: string | undefined +): readonly Record[] { + if (!binaryPath) { + return []; + } + return [ + { + label: "Patchloom MCP", + serverDefinition: { + type: "stdio", + command: binaryPath, + args: ["mcp-server"] + } + } + ]; +} + +let resolvedBinaryPath: string | undefined; +let providerRegistered = false; + +type LmWithMcpProvider = typeof VSCode.lm & { + registerMCPServerDefinitionProvider?: ( + id: string, + provider: { provideMCPServerDefinitions(): unknown[] } + ) => VSCode.Disposable; +}; + +export async function registerMcpServerProvider(context: VSCode.ExtensionContext): Promise { // vscode.lm.registerMCPServerDefinitionProvider is available in VS Code 1.100+ - const lm = vscode.lm as typeof vscode.lm & { - registerMCPServerDefinitionProvider?: ( - id: string, - provider: { provideMCPServerDefinitions(): unknown[] } - ) => vscode.Disposable; - }; + const vscode = await import("vscode"); + const lm = vscode.lm as LmWithMcpProvider; if (typeof lm.registerMCPServerDefinitionProvider !== "function") { return; } @@ -36,37 +61,45 @@ function buildMcpServerDefinition(): Record { }; } -export async function registerMcpServerProviderWithBinary(context: vscode.ExtensionContext): Promise { - const lm = vscode.lm as typeof vscode.lm & { - registerMCPServerDefinitionProvider?: ( - id: string, - provider: { provideMCPServerDefinitions(): unknown[] } - ) => vscode.Disposable; - }; - if (typeof lm.registerMCPServerDefinitionProvider !== "function") { - return; +/** Resolve CLI binary and update the path used by the native MCP provider. */ +export async function refreshMcpServerBinary(): Promise { + const status = await resolvePatchloomStatus(); + if (status.ready && status.binaryPath) { + resolvedBinaryPath = status.binaryPath; + } else { + resolvedBinaryPath = undefined; } +} - const status = await resolvePatchloomStatus(); - if (!status.ready || !status.binaryPath) { +/** + * Always register the native MCP provider when the API exists. + * Binary path is resolved at provide time and refreshed after managed install. + */ +export async function registerMcpServerProviderWithBinary(context: VSCode.ExtensionContext): Promise { + const vscode = await import("vscode"); + const lm = vscode.lm as LmWithMcpProvider; + if (typeof lm.registerMCPServerDefinitionProvider !== "function") { return; } - const binaryPath = status.binaryPath; - const disposable = lm.registerMCPServerDefinitionProvider("patchloom", { - provideMCPServerDefinitions() { - return [{ - label: "Patchloom MCP", - serverDefinition: { - type: "stdio", - command: binaryPath, - args: ["mcp-server"] - } - }]; - } - }); - context.subscriptions.push(disposable); + if (!providerRegistered) { + const disposable = lm.registerMCPServerDefinitionProvider("patchloom", { + provideMCPServerDefinitions() { + return [...mcpServerDefinitionsForBinary(resolvedBinaryPath)]; + } + }); + context.subscriptions.push(disposable); + providerRegistered = true; + const log = getPatchloomLog(); + log?.log("Registered Patchloom MCP server definition provider via native API"); + } + + await refreshMcpServerBinary(); const log = getPatchloomLog(); - log?.log(`Registered Patchloom MCP server via native API (binary: ${binaryPath})`); + if (resolvedBinaryPath) { + log?.log(`Patchloom MCP binary resolved: ${resolvedBinaryPath}`); + } else { + log?.log("Patchloom MCP binary not ready; provider will return empty until install/refresh"); + } } diff --git a/src/util.ts b/src/util.ts index e2d9dda..3af392a 100644 --- a/src/util.ts +++ b/src/util.ts @@ -109,7 +109,7 @@ function extractCliJsonError(stream: string): string | undefined { } // CLI 0.27+: fail-closed doc navigation may hint the multi-match sibling. if (typeof parsed.suggested_op === "string" && parsed.suggested_op.length > 0) { - message = `${message} (suggested_op: ${parsed.suggested_op})`; + message = `${message} (try ${parsed.suggested_op})`; } return message; } catch { diff --git a/test/unit/batchApply.test.ts b/test/unit/batchApply.test.ts index 47ab25b..3a1b181 100644 --- a/test/unit/batchApply.test.ts +++ b/test/unit/batchApply.test.ts @@ -3,6 +3,7 @@ import test from "node:test"; import { buildBatchApplyArgs, buildBatchTemplate, + isEmptyBatchPlan, parseBatchOperationCount } from "../../src/commands/batchApply.js"; @@ -53,6 +54,16 @@ test("parseBatchOperationCount counts a single operation", () => { assert.equal(parseBatchOperationCount('tidy.fix src/main.ts'), 1); }); +test("isEmptyBatchPlan is true for empty and whitespace-only plans", () => { + assert.equal(isEmptyBatchPlan(""), true); + assert.equal(isEmptyBatchPlan(" \n \n"), true); +}); + +test("isEmptyBatchPlan is false when at least one operation is present", () => { + assert.equal(isEmptyBatchPlan('tidy.fix src/main.ts'), false); + assert.equal(isEmptyBatchPlan('\nreplace a.txt "x" "y"\n'), false); +}); + // --- #34: snapshot-style template tests --- test("buildBatchTemplate replace line has file and quoted arguments", () => { diff --git a/test/unit/binary.test.ts b/test/unit/binary.test.ts index a53a155..1b84ea8 100644 --- a/test/unit/binary.test.ts +++ b/test/unit/binary.test.ts @@ -19,8 +19,10 @@ import { } from "../../src/binary/patchloom.js"; import { assertTrustedManagedInstallDownloadUrl, + assertTrustedManagedInstallRedirectUrl, buildManagedInstallReleaseAssets, isTrustedManagedInstallDownloadUrl, + isTrustedManagedInstallRedirectUrl, calculateSha256Hex, clearManagedInstallFailure, clearManagedInstallFailureRecord, @@ -37,6 +39,7 @@ import { promoteManagedInstallBinary, resolveManagedInstallChecksum, resolveManagedInstallPaths, + resolveManagedInstallRedirectUrl, resolveManagedInstallTransactionPaths, setManagedInstallFailure, isMuslLinux, @@ -979,6 +982,75 @@ test("isTrustedManagedInstallDownloadUrl respects custom repo parameter", () => ); }); +// --- managed install redirect URL trust tests --- + +test("resolveManagedInstallRedirectUrl resolves relative Location against current URL", () => { + const resolved = resolveManagedInstallRedirectUrl( + "https://github.com/patchloom/patchloom/releases/download/v0.1.0/archive.tar.xz", + "/foo" + ); + assert.equal(resolved, "https://github.com/foo"); + assert.equal(isTrustedManagedInstallRedirectUrl(resolved), true); +}); + +test("isTrustedManagedInstallRedirectUrl accepts GitHub release CDN hosts over HTTPS", () => { + assert.equal( + isTrustedManagedInstallRedirectUrl("https://objects.githubusercontent.com/github-production-release-asset/x"), + true + ); + assert.equal( + isTrustedManagedInstallRedirectUrl("https://release-assets.githubusercontent.com/x"), + true + ); + assert.equal( + isTrustedManagedInstallRedirectUrl("https://github-releases.githubusercontent.com/x"), + true + ); + assert.equal( + isTrustedManagedInstallRedirectUrl("https://github.com/patchloom/patchloom/releases/download/v0.1.0/x"), + true + ); +}); + +test("isTrustedManagedInstallRedirectUrl rejects non-HTTPS and untrusted hosts", () => { + assert.equal( + isTrustedManagedInstallRedirectUrl("https://evil.example/payload"), + false + ); + assert.equal( + isTrustedManagedInstallRedirectUrl("http://objects.githubusercontent.com/x"), + false + ); + assert.equal( + isTrustedManagedInstallRedirectUrl("http://github.com/x"), + false + ); + assert.equal(isTrustedManagedInstallRedirectUrl("not-a-url"), false); + assert.equal(isTrustedManagedInstallRedirectUrl(""), false); +}); + +test("assertTrustedManagedInstallRedirectUrl throws untrusted-download-url for evil hosts", () => { + assert.doesNotThrow(() => + assertTrustedManagedInstallRedirectUrl("https://objects.githubusercontent.com/x") + ); + assert.throws( + () => assertTrustedManagedInstallRedirectUrl("https://evil.example/payload"), + (error: unknown) => { + assert.ok(error instanceof ManagedInstallVerificationError); + assert.equal(error.reason, "untrusted-download-url"); + return true; + } + ); + assert.throws( + () => assertTrustedManagedInstallRedirectUrl("http://objects.githubusercontent.com/x"), + (error: unknown) => { + assert.ok(error instanceof ManagedInstallVerificationError); + assert.equal(error.reason, "untrusted-download-url"); + return true; + } + ); +}); + test("resolvePatchloomStatusWithInputs skips PATH and setting in untrusted workspaces", async () => { const expectedBinaryPath = path.join("/managed/install", "managed-bin", "patchloom"); const status = await resolvePatchloomStatusWithInputs({ diff --git a/test/unit/initializeProject.test.ts b/test/unit/initializeProject.test.ts index a2d5596..d4aa89b 100644 --- a/test/unit/initializeProject.test.ts +++ b/test/unit/initializeProject.test.ts @@ -290,7 +290,7 @@ test("formatCliOutput appends suggested_op when present (CLI 0.27+)", () => { }); assert.equal( formatCliOutput({ exitCode: 1, stdout, stderr: "" }), - "invalid_input: selector uses wildcard/predicate, which is not valid for doc.set (single path only) (suggested_op: doc.update)" + "invalid_input: selector uses wildcard/predicate, which is not valid for doc.set (single path only) (try doc.update)" ); }); @@ -887,7 +887,7 @@ test("generateAgentRules surfaces formatCliOutput envelope on CLI failure", asyn }), (err: Error) => { assert.match(err.message, /invalid_input/); - assert.match(err.message, /suggested_op: doc\.update/); + assert.match(err.message, /try doc\.update/); return true; } ); diff --git a/test/unit/mcpRegister.test.ts b/test/unit/mcpRegister.test.ts new file mode 100644 index 0000000..f89ace4 --- /dev/null +++ b/test/unit/mcpRegister.test.ts @@ -0,0 +1,20 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { mcpServerDefinitionsForBinary } from "../../src/mcp/register.js"; + +test("mcpServerDefinitionsForBinary returns empty when binary is undefined", () => { + assert.deepEqual(mcpServerDefinitionsForBinary(undefined), []); +}); + +test("mcpServerDefinitionsForBinary returns one stdio definition for a path", () => { + const defs = mcpServerDefinitionsForBinary("/opt/patchloom"); + assert.equal(defs.length, 1); + assert.deepEqual(defs[0], { + label: "Patchloom MCP", + serverDefinition: { + type: "stdio", + command: "/opt/patchloom", + args: ["mcp-server"] + } + }); +}); diff --git a/test/unit/outputChannel.test.ts b/test/unit/outputChannel.test.ts index feaefe6..5083262 100644 --- a/test/unit/outputChannel.test.ts +++ b/test/unit/outputChannel.test.ts @@ -291,3 +291,17 @@ test("presentCliResultInOutput skips empty streams", () => { test("presentCliResultInOutput no-ops when log is undefined", () => { presentCliResultInOutput(undefined, { stdout: "out", stderr: "err" }); }); + +test("writeUserVisibleOutput through createPatchloomLog writes raw lines without timestamp", () => { + const lines: string[] = []; + const log = createPatchloomLog(() => ({ + appendLine(value: string) { lines.push(value); }, + show() {}, + dispose() {} + })); + + writeUserVisibleOutput(log, "a\nb"); + + assert.deepEqual(lines, ["a", "b"]); + assert.ok(lines.every((line) => !line.includes("["))); +}); diff --git a/test/unit/quickActions.test.ts b/test/unit/quickActions.test.ts index 8a8b2d5..f31978b 100644 --- a/test/unit/quickActions.test.ts +++ b/test/unit/quickActions.test.ts @@ -29,6 +29,7 @@ import { isMarkdownPath, isStructuredDocumentPath, isPathInsideWorkspace, + formatUndoFailureMessage, presentPatchMergeOutcome, presentSearchOutcome, presentUndoSuccess, @@ -754,7 +755,7 @@ test("presentPatchMergeOutcome exit 0 writes streams + show, returns ok", () => assert.deepEqual(messages, ["merged 2 hunks", "SHOW"]); }); -test("presentPatchMergeOutcome exit 1 writes nothing, returns error", () => { +test("presentPatchMergeOutcome exit 1 writes streams + show, returns error", () => { const { log, messages } = fakeLog(); const kind = presentPatchMergeOutcome(log, { exitCode: 1, @@ -762,7 +763,7 @@ test("presentPatchMergeOutcome exit 1 writes nothing, returns error", () => { stderr: "failed" }); assert.equal(kind, "error"); - assert.deepEqual(messages, []); + assert.deepEqual(messages, ["out", "failed", "SHOW"]); }); test("presentUndoSuccess writes streams + show", () => { @@ -773,3 +774,25 @@ test("presentUndoSuccess writes streams + show", () => { }); assert.deepEqual(messages, ["restored a.ts", "info", "SHOW"]); }); + +test("formatUndoFailureMessage uses no-backup copy when stderr mentions no backup", () => { + assert.equal( + formatUndoFailureMessage({ + exitCode: 1, + stdout: "", + stderr: "error: no backup found for workspace" + }), + "No patchloom backup to undo." + ); +}); + +test("formatUndoFailureMessage prefixes other failures with Patchloom undo failed", () => { + assert.equal( + formatUndoFailureMessage({ + exitCode: 2, + stdout: "", + stderr: "permission denied" + }), + "Patchloom undo failed: permission denied" + ); +});