diff --git a/ts/packages/agents/code/package.json b/ts/packages/agents/code/package.json index c6f3d97c6f..60b3d2a153 100644 --- a/ts/packages/agents/code/package.json +++ b/ts/packages/agents/code/package.json @@ -60,6 +60,7 @@ }, "devDependencies": { "@typeagent/action-grammar-compiler": "workspace:*", + "@typeagent/action-schema": "workspace:*", "@typeagent/action-schema-compiler": "workspace:*", "@types/better-sqlite3": "7.6.13", "@types/debug": "^4.1.12", diff --git a/ts/packages/agents/code/src/codeActionHandler.ts b/ts/packages/agents/code/src/codeActionHandler.ts index caf2f7722c..0d4227cf00 100644 --- a/ts/packages/agents/code/src/codeActionHandler.ts +++ b/ts/packages/agents/code/src/codeActionHandler.ts @@ -5,6 +5,7 @@ import { WebSocketMessageV2 } from "@typeagent/websocket-utils"; import { CodeAgentWebSocketServer } from "./codeAgentWebSocketServer.js"; import { ActionContext, + ActionResult, AppAction, AppAgent, ReadinessReport, @@ -59,7 +60,7 @@ const sharedActiveSessions = new Set>(); const sharedPendingCalls: Map< number, { - resolve: (value?: undefined) => void; + resolve: (errorMessage?: string) => void; context?: ActionContext | undefined; } > = new Map(); @@ -99,7 +100,7 @@ type CodeActionContext = { pendingCall: Map< number, { - resolve: (value?: undefined) => void; + resolve: (errorMessage?: string) => void; context?: ActionContext | undefined; } >; @@ -187,6 +188,40 @@ function getCodeBindPort(): number { return n; } +// Detect an operational-failure result from the coda extension: a WebSocket +// `result` that is JSON encoding a plain object with exactly one key, +// `error` (a string) — the shape produced by handleReadActions' catch-all +// and explicit error returns. Any other shape (including the "OK"/"pong" +// strings used by non-action messages) is treated as success and yields +// undefined, so this only changes behavior for genuinely error-shaped results. +export function extractOperationalError(result: unknown): string | undefined { + if (typeof result !== "string") { + return undefined; + } + let parsed: unknown; + try { + parsed = JSON.parse(result); + } catch { + return undefined; + } + if ( + typeof parsed === "object" && + parsed !== null && + !Array.isArray(parsed) + ) { + const keys = Object.keys(parsed); + const errorValue = (parsed as Record).error; + if ( + keys.length === 1 && + keys[0] === "error" && + typeof errorValue === "string" + ) { + return errorValue; + } + } + return undefined; +} + // Wire the shared server's onMessage handler. Module-scoped because the // server itself is module-scoped — all sessions route their pending-call // completions through the same handler. @@ -204,7 +239,7 @@ function attachSharedOnMessage(server: CodeAgentWebSocketServer): void { if (context?.actionIO) { context.actionIO.setDisplay(data.result); } - resolve(); + resolve(extractOperationalError(data.result)); } } } catch (error) { @@ -497,9 +532,14 @@ export async function getActiveFileFromVSCode( // NOTE: pendingCall entry has no ActionContext because this isn’t a UI action agentContext.pendingCall.set(callId, { - resolve: (value?: any) => { + // This call site doesn't currently receive real ActiveFile + // payloads through the pending-call channel (the coda side has + // no "code/getActiveFile" handler, so it always falls through to + // the default "OK" response) — only the timeout/undefined path + // is exercised today. Any non-error result still resolves undefined. + resolve: () => { clearTimeout(t); - resolve(value as ActiveFile | undefined); + resolve(undefined); }, context: undefined as any, }); @@ -543,23 +583,26 @@ async function executeCodeAction( } const callId = nextSharedCallId++; - return new Promise((resolve) => { + return new Promise((resolve) => { const timeoutMs = 5000; const timeoutHandle = setTimeout(() => { if (agentContext.pendingCall.has(callId)) { agentContext.pendingCall.delete(callId); + const timeoutMessage = `No connected coda extension handled action "${action.actionName}". If multiple VS Code windows are open, reload the others (Ctrl+Shift+P → Developer: Reload Window) so they pick up the latest coda bundle.`; if (context.actionIO) { - context.actionIO.setDisplay( - `No connected coda extension handled action "${action.actionName}". If multiple VS Code windows are open, reload the others (Ctrl+Shift+P → Developer: Reload Window) so they pick up the latest coda bundle.`, - ); + context.actionIO.setDisplay(timeoutMessage); } - resolve(undefined); + resolve(createActionResultFromError(timeoutMessage)); } }, timeoutMs); agentContext.pendingCall.set(callId, { - resolve: (value?: undefined) => { + resolve: (errorMessage?: string) => { clearTimeout(timeoutHandle); - resolve(value); + resolve( + errorMessage !== undefined + ? createActionResultFromError(errorMessage) + : undefined, + ); }, context, }); diff --git a/ts/packages/agents/code/src/codeActionsSchema.ts b/ts/packages/agents/code/src/codeActionsSchema.ts index 5980b86cea..bb19db8278 100644 --- a/ts/packages/agents/code/src/codeActionsSchema.ts +++ b/ts/packages/agents/code/src/codeActionsSchema.ts @@ -14,6 +14,7 @@ export type CodeActions = | ListOpenEditorsAction | GetFileContentAction | GetWorkspaceChangesAction + | GetGitDiffAction | LaunchCopilotChatAction; export type CodeActivity = LaunchVSCodeAction; @@ -229,3 +230,44 @@ export type GetWorkspaceChangesAction = { actionName: "getWorkspaceChanges"; parameters: {}; }; + +// Get the actual Git diff for the workspace: changed files plus bounded unified-patch/hunk text, split into staged and unstaged sections by default. +// +// Complements getWorkspaceChanges (which only reports file paths and +// statuses) by returning real patch content, truncated with explicit +// metadata when a diff is large. +// +// Omit `base` (or pass "HEAD") to diff against HEAD: staged (index vs HEAD) +// and unstaged (working tree vs index) changes are reported as two separate +// sections. Pass `base` to instead diff the working tree against an +// arbitrary ref (branch, tag, or commit-ish), reported as one combined +// section since git does not distinguish staged/unstaged against a custom +// base. +// +// Each section reports: files (path, oldPath for renames/copies, status, +// binary, and bounded patch text -- omitted for binary files), plus +// filesTruncated/patchTruncated booleans when the file-count or byte-budget +// caps were hit, and filesOutsideWorkspace (only present when > 0) counting +// files dropped because they fall outside every open workspace folder (the +// git repository root can sit above the folder(s) actually opened). +// +// Example: +// User: show me my uncommitted changes +// Agent: { actionName: "getGitDiff", parameters: {} } +// +// Example: +// User: diff my branch against main +// Agent: { actionName: "getGitDiff", parameters: { base: "main" } } +export type GetGitDiffAction = { + actionName: "getGitDiff"; + parameters: { + // Git ref (branch, tag, or commit-ish, e.g. "main", "HEAD~3") to diff + // the working tree against. Omit (or pass "HEAD") to diff against + // HEAD and get separate staged/unstaged sections. + base?: string; + // Workspace-relative repository root or folder name to select which + // git repository to diff when more than one is open. Ignored (and + // unnecessary) when only one repository is open. + repository?: string; + }; +}; diff --git a/ts/packages/agents/code/test/codeActionErrorSemantics.spec.ts b/ts/packages/agents/code/test/codeActionErrorSemantics.spec.ts new file mode 100644 index 0000000000..8df73e9d54 --- /dev/null +++ b/ts/packages/agents/code/test/codeActionErrorSemantics.spec.ts @@ -0,0 +1,88 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Tests for `extractOperationalError`, the shape-detector that lets + * `executeCodeAction` distinguish a genuine operational failure reported by + * the coda VS Code extension (a WebSocket `result` that JSON-encodes + * `{ error: string }`) from a normal success payload, so failures surface as + * a failed `ActionResult` (via `createActionResultFromError`) instead of a + * success-shaped `ActionResult` whose display just happens to contain the + * word "error". + */ + +import { extractOperationalError } from "../src/codeActionHandler.js"; + +describe("extractOperationalError", () => { + test("detects the single-key { error: string } shape used by handleReadActions' catch-all", () => { + expect(extractOperationalError(JSON.stringify({ error: "boom" }))).toBe( + "boom", + ); + }); + + test("returns undefined for non-string results (e.g. already-parsed objects)", () => { + expect(extractOperationalError({ error: "boom" })).toBeUndefined(); + expect(extractOperationalError(undefined)).toBeUndefined(); + expect(extractOperationalError(42)).toBeUndefined(); + }); + + test("returns undefined for non-JSON strings (e.g. the 'OK'/'pong' acks)", () => { + expect(extractOperationalError("OK")).toBeUndefined(); + expect(extractOperationalError("pong")).toBeUndefined(); + }); + + test("returns undefined for JSON arrays and primitives", () => { + expect( + extractOperationalError(JSON.stringify(["error"])), + ).toBeUndefined(); + expect( + extractOperationalError(JSON.stringify("error")), + ).toBeUndefined(); + expect(extractOperationalError(JSON.stringify(null))).toBeUndefined(); + expect(extractOperationalError(JSON.stringify(5))).toBeUndefined(); + }); + + test("returns undefined for success-shaped JSON objects with unrelated keys", () => { + // Real success payloads for read actions (e.g. getGitDiff) are + // multi-key objects — must never be mistaken for an error. + expect( + extractOperationalError( + JSON.stringify({ files: [], truncated: false }), + ), + ).toBeUndefined(); + }); + + test("returns undefined when 'error' is present alongside other keys", () => { + // Only the exact single-key { error: string } shape produced by + // handleReadActions' explicit error returns counts — an object that + // happens to also carry an "error" field among real success data + // must not be treated as an operational failure. + expect( + extractOperationalError( + JSON.stringify({ error: "boom", files: [] }), + ), + ).toBeUndefined(); + }); + + test("returns undefined when 'error' is present but not a string", () => { + expect( + extractOperationalError(JSON.stringify({ error: 123 })), + ).toBeUndefined(); + expect( + extractOperationalError(JSON.stringify({ error: null })), + ).toBeUndefined(); + }); + + test("detects the JSON-encoded 'Did not handle the action' failure from handleVSCodeActions", () => { + // The unhandled-action fallback in handleVSCodeActions.ts must encode + // as { error: string } (not a bare string) so this failure path is + // classified the same way as every other operational error. + expect( + extractOperationalError( + JSON.stringify({ + error: 'Did not handle the action: "someUnknownAction"', + }), + ), + ).toBe('Did not handle the action: "someUnknownAction"'); + }); +}); diff --git a/ts/packages/agents/code/test/codeActionsSchemaDiscovery.spec.ts b/ts/packages/agents/code/test/codeActionsSchemaDiscovery.spec.ts new file mode 100644 index 0000000000..2eda58b508 --- /dev/null +++ b/ts/packages/agents/code/test/codeActionsSchemaDiscovery.spec.ts @@ -0,0 +1,61 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Proves that getGitDiff is discoverable through the same catalog pipeline + * external MCP clients use (e.g. commandExecutor's discover_agents/execute_action, + * which read schemas via Dispatcher.getAgentSchemas -> ParsedActionSchema.actionSchemas). + * + * This parses the real codeActionsSchema.ts source with the same + * @typeagent/action-schema APIs used to build/round-trip the compiled + * dist/codeSchema.pas.json catalog artifact (see + * packages/actionSchema/test/regen.spec.ts, which already round-trips this + * exact schema as part of its generic per-agent coverage). Rather than + * duplicate that generic round-trip, this test asserts the specific, + * externally-visible shape (name/description/parameters) of the new action. + */ + +import fs from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { + parseActionSchemaSource, + getActionDescription, + getParameterNames, +} from "@typeagent/action-schema"; + +// This spec compiles to dist/test/*.js, so the source tree (../src from the +// package root) is two levels up from there. +const schemaPath = fileURLToPath( + new URL("../../src/codeActionsSchema.ts", import.meta.url), +); + +describe("code agent action catalog discoverability", () => { + const source = fs.readFileSync(schemaPath, "utf-8"); + const parsed = parseActionSchemaSource( + source, + "code", + { action: "CodeActions", activity: "CodeActivity" }, + path.basename(schemaPath), + ); + + it("includes getGitDiff with a concise, standalone description", () => { + const actionDef = parsed.actionSchemas.get("getGitDiff"); + expect(actionDef).toBeDefined(); + expect(getActionDescription(actionDef!)).toBe( + "Get the actual Git diff for the workspace: changed files plus bounded unified-patch/hunk text, split into staged and unstaged sections by default.", + ); + }); + + it("exposes getGitDiff's base and repository parameters", () => { + const actionDef = parsed.actionSchemas.get("getGitDiff")!; + const names = getParameterNames(actionDef, () => undefined).sort(); + expect(names).toEqual(["parameters.base", "parameters.repository"]); + }); + + it("still includes getWorkspaceChanges (compatibility)", () => { + const actionDef = parsed.actionSchemas.get("getWorkspaceChanges"); + expect(actionDef).toBeDefined(); + expect(getActionDescription(actionDef!)).toBeTruthy(); + }); +}); diff --git a/ts/packages/coda/package.json b/ts/packages/coda/package.json index ad644fc723..bfa11ce6ba 100644 --- a/ts/packages/coda/package.json +++ b/ts/packages/coda/package.json @@ -28,9 +28,11 @@ "package": "mkdirp dist-pub && vsce package --allow-star-activation --allow-missing-repository --no-dependencies -o dist-pub/aisystems-coda.vsix", "prettier": "prettier --check . --ignore-path ../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../.prettierignore", - "pretest": "pnpm run build", + "test": "npm run test:local", "test-compile": "tsc -p ./src", + "pretest:full": "pnpm run build", "test:full": "vscode-test", + "test:local": "tsx --test --test-reporter=spec --test-reporter-destination=stdout --test-reporter=../../tools/scripts/nodeTestFailureReporter.mjs --test-reporter-destination=stdout test/*.spec.ts", "vscode:prepublish": "pnpm run esbuild-base --minify", "watch": "tsc -w" }, @@ -75,7 +77,8 @@ "esbuild": "^0.28.2", "mkdirp": "^3.0.1", "prettier": "^3.5.3", - "rimraf": "^6.0.1" + "rimraf": "^6.0.1", + "tsx": "^4.21.0" }, "engines": { "vscode": "^1.88.0" diff --git a/ts/packages/coda/src/gitDiffUtils.ts b/ts/packages/coda/src/gitDiffUtils.ts new file mode 100644 index 0000000000..744038e9d9 --- /dev/null +++ b/ts/packages/coda/src/gitDiffUtils.ts @@ -0,0 +1,328 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Pure, vscode-independent Git diff parsing/formatting helpers used by +// getGitDiff (see handleReadActions.ts). Kept free of any "vscode" import so +// they can be unit tested directly (with tsx --test) without the VS Code +// extension host. + +import { StringDecoder } from "node:string_decoder"; + +// Output bounds for getGitDiff. Real repos can have arbitrarily large diffs; +// these caps keep the ActionResult message small enough to round-trip over +// the WebSocket and fit in a reasoning agent's context, with explicit +// truncation metadata rather than silently dropping content. +export const MAX_DIFF_FILES = 200; +export const MAX_FILE_PATCH_BYTES = 20_000; +export const MAX_SECTION_PATCH_BYTES = 100_000; + +export type DiffFileEntry = { + path: string; + oldPath?: string; + status: string; + binary: boolean; + patch?: string; +}; + +export type DiffSection = { + files: DiffFileEntry[]; + filesTruncated: boolean; + patchTruncated: boolean; + // Count of files omitted because they fall outside every open workspace + // folder (only set, and only > 0, when that filtering actually dropped + // something -- see handleReadActions.ts). Absent otherwise. + filesOutsideWorkspace?: number; + // Count of "diff --git" blocks that failed to parse (e.g. an unexpected + // header shape) and were dropped rather than silently omitted with no + // trace. Absent when every block parsed successfully. + filesUnparsed?: number; +}; + +// Truncate patch text to at most maxBytes (UTF-8), returning the possibly- +// truncated text, the number of bytes it consumed, and whether it was cut. +// Uses StringDecoder rather than a raw byte slice so a cut that lands in the +// middle of a multi-byte character drops that trailing partial character +// instead of decoding it into a U+FFFD replacement character (which would +// also make the reported byte count wrong, since re-encoding U+FFFD is 3 +// bytes even when only 1 byte of the original sequence was kept). +export function boundPatchText( + text: string, + maxBytes: number, +): [text: string, bytes: number, truncated: boolean] { + const encoded = Buffer.from(text, "utf8"); + if (encoded.length <= maxBytes) { + return [text, encoded.length, false]; + } + const decoder = new StringDecoder("utf8"); + const truncated = decoder.write(encoded.subarray(0, maxBytes)); + return [truncated, Buffer.byteLength(truncated, "utf8"), true]; +} + +export function isBinaryDiffText(text: string): boolean { + return ( + /^Binary files .* differ/m.test(text) || + text.includes("GIT binary patch") + ); +} + +// Split raw unified-diff text into per-file blocks (on "diff --git " +// headers). Exported so callers can filter/inspect blocks (e.g. workspace +// containment in handleReadActions.ts) before/independent of parseUnifiedDiff. +export function splitDiffBlocks(text: string): string[] { + if (!text) { + return []; + } + return text.split(/(?=^diff --git )/m).filter((block) => block.length > 0); +} + +// Parse the raw unified-diff text returned by repo.diff() into per-file +// entries. Files are split on "diff --git " headers; status/binary/rename +// are inferred from the standard git diff header lines since diff() (unlike +// diffWith()) does not return a separate Change[] with status codes. +export function parseUnifiedDiff(text: string): DiffSection { + const blocks = splitDiffBlocks(text); + const filesTruncated = blocks.length > MAX_DIFF_FILES; + const limitedBlocks = blocks.slice(0, MAX_DIFF_FILES); + let remaining = MAX_SECTION_PATCH_BYTES; + let patchTruncated = false; + let filesUnparsed = 0; + const files: DiffFileEntry[] = []; + for (const block of limitedBlocks) { + const entry = parseDiffBlock(block); + if (!entry) { + filesUnparsed++; + continue; + } + if (!entry.binary && entry.patch) { + if (remaining <= 0) { + delete entry.patch; + patchTruncated = true; + } else { + const [boundedText, usedBytes, wasTruncated] = boundPatchText( + entry.patch, + Math.min(MAX_FILE_PATCH_BYTES, remaining), + ); + entry.patch = boundedText; + remaining -= usedBytes; + if (wasTruncated) { + patchTruncated = true; + } + } + } + files.push(entry); + } + return { + files, + filesTruncated, + patchTruncated, + ...(filesUnparsed > 0 && { filesUnparsed }), + }; +} + +// Confirms a block is a valid diff block (just checks for the header +// prefix; the two paths on this line are not reliably separable when an +// unquoted, plain-ASCII path contains a literal space, so actual path +// extraction uses the single-path-per-line forms below instead). +const DIFF_HEADER_RE = /^diff --git /m; +const LEGACY_HEADER_PATH_RE = /^diff --git a\/(.*) b\/(.*)$/m; + +const GIT_QUOTE_ESCAPES: Record = { + "\\": 0x5c, + '"': 0x22, + a: 0x07, + b: 0x08, + f: 0x0c, + n: 0x0a, + r: 0x0d, + t: 0x09, + v: 0x0b, +}; + +// Reverse git's diff-header path quoting. By default (core.quotepath=true, +// the git default) any path containing a non-ASCII UTF-8 byte -- i.e. any +// accented, CJK, or emoji filename -- is wrapped in double quotes with each +// non-printable/high-bit byte octal-escaped (`\NNN`), plus the usual C +// escapes (`\\`, `\"`, `\t`, ...). Without unquoting these, such paths fail +// to match and the file is silently dropped from the diff. `inner` must +// NOT include the surrounding quotes. +// +// A path can also be quoted for reasons other than non-ASCII bytes (e.g. it +// contains a literal `"` or tab) while `core.quotepath=false` is set, in +// which case any non-ASCII characters appear *unescaped* inside the quotes +// -- already-decoded JS string characters, not raw bytes. Octal/C escapes +// are collected as raw bytes (they may be a multi-byte UTF-8 sequence split +// across several `\NNN` escapes) and decoded as UTF-8 once flushed; any +// unescaped character is appended to the result as-is instead, so a +// surrogate pair for a character outside the BMP round-trips unchanged. +function unquoteGitHeaderPath(inner: string): string { + let result = ""; + let byteBuffer: number[] = []; + const flushBytes = () => { + if (byteBuffer.length > 0) { + result += Buffer.from(byteBuffer).toString("utf8"); + byteBuffer = []; + } + }; + for (let i = 0; i < inner.length; i++) { + const ch = inner[i]; + if (ch !== "\\" || i + 1 >= inner.length) { + if (ch.charCodeAt(0) > 0x7f) { + flushBytes(); + result += ch; + } else { + byteBuffer.push(ch.charCodeAt(0)); + } + continue; + } + const octal = inner.slice(i + 1, i + 4); + if (/^[0-7]{3}$/.test(octal)) { + byteBuffer.push(parseInt(octal, 8)); + i += 3; + continue; + } + const escaped = GIT_QUOTE_ESCAPES[inner[i + 1]]; + if (escaped !== undefined) { + byteBuffer.push(escaped); + i += 1; + continue; + } + // Unrecognized escape: keep the backslash literally; the next + // character is processed on its own next iteration. + byteBuffer.push(ch.charCodeAt(0)); + } + flushBytes(); + return result; +} + +// Extract the path from a single diff header line matched by `lineRe` (must +// have exactly one capture group spanning the rest of the line). Handles +// both the quoted (C-escaped) and bare forms, and returns undefined for a +// missing line or the `/dev/null` sentinel. +function extractHeaderLinePath( + block: string, + lineRe: RegExp, +): string | undefined { + const match = lineRe.exec(block); + if (!match) { + return undefined; + } + // Git appends a bare trailing tab to a `---`/`+++` path that contains a + // literal space (the classic unified-diff convention for disambiguating + // the filename from a would-be timestamp) -- after the closing quote + // for a quoted path. Strip it before the quote check below: a real + // trailing-tab byte in a filename is always quoted with the tab + // C-escaped as `\t` *inside* the quotes, so a bare trailing tab here is + // unambiguously this disambiguator, never part of the path itself. + // (`rename from`/`rename to`/`copy from`/`copy to` never get this tab, + // but stripping a tab that isn't there is a no-op, so this is safe for + // every caller.) + const raw = match[1].endsWith("\t") ? match[1].slice(0, -1) : match[1]; + if (raw === "/dev/null") { + return undefined; + } + if (raw.startsWith('"') && raw.endsWith('"')) { + return unquoteGitHeaderPath(raw.slice(1, -1)); + } + return raw; +} + +function stripPrefix(value: string, prefix: string): string; +function stripPrefix( + value: string | undefined, + prefix: string, +): string | undefined; +function stripPrefix( + value: string | undefined, + prefix: string, +): string | undefined { + if (value === undefined) { + return undefined; + } + return value.startsWith(prefix) ? value.slice(prefix.length) : value; +} + +const RENAME_FROM_RE = /^rename from (.+)$/m; +const RENAME_TO_RE = /^rename to (.+)$/m; +const COPY_FROM_RE = /^copy from (.+)$/m; +const COPY_TO_RE = /^copy to (.+)$/m; +const MINUS_RE = /^--- (.+)$/m; +const PLUS_RE = /^\+\+\+ (.+)$/m; + +// Parse a single "diff --git a/... b/..." block into a DiffFileEntry. Paths +// are read from the single-path-per-line rename/copy/---/+++ header lines +// (each independently quoted only when needed) rather than the combined +// "diff --git a/X b/Y" line, since that line is ambiguous to split when an +// unquoted (plain-ASCII) path contains a literal space. +export function parseDiffBlock(block: string): DiffFileEntry | undefined { + if (!DIFF_HEADER_RE.test(block)) { + return undefined; + } + const isRename = /^rename from /m.test(block); + const isCopy = /^copy from /m.test(block); + const isNew = + /^new file mode /m.test(block) || /^--- \/dev\/null/m.test(block); + const isDeleted = + /^deleted file mode /m.test(block) || + /^\+\+\+ \/dev\/null/m.test(block); + + let path: string | undefined; + let oldPath: string | undefined; + if (isRename || isCopy) { + oldPath = extractHeaderLinePath( + block, + isRename ? RENAME_FROM_RE : COPY_FROM_RE, + ); + path = extractHeaderLinePath( + block, + isRename ? RENAME_TO_RE : COPY_TO_RE, + ); + } + // ---/+++ lines are present for any block with an actual content diff + // (including a rename-with-changes), and are the unambiguous source for + // the current path when a rename/copy had no from/to lines to fall back + // on (they always do, but this keeps the two sources consistent). + path = + path ?? + stripPrefix(extractHeaderLinePath(block, PLUS_RE), "b/") ?? + stripPrefix(extractHeaderLinePath(block, MINUS_RE), "a/"); + if (path === undefined) { + // Rare fallback: a binary-diff or mode-only-change block has + // neither ---/+++ nor rename/copy lines (a rename/copy always + // emits "rename to"/"copy to", handled above). Both sides of the + // combined "diff --git a/X b/Y" header are therefore always the + // same path here, so extract it by finding where the first + // (quoted-or-not) path ends rather than trying to split two + // independently-quoted paths apart. + const quoted = /^diff --git "((?:\\.|[^"\\])*)"/m.exec(block); + if (quoted) { + path = stripPrefix(unquoteGitHeaderPath(quoted[1]), "a/"); + } else { + const legacy = LEGACY_HEADER_PATH_RE.exec(block); + if (!legacy) { + return undefined; + } + path = legacy[2]; + } + } + + let status: string; + if (isRename) { + status = "renamed"; + } else if (isCopy) { + status = "copied"; + } else if (isNew) { + status = "added"; + } else if (isDeleted) { + status = "deleted"; + } else { + status = "modified"; + } + const binary = isBinaryDiffText(block); + return { + path, + ...(oldPath !== undefined && { oldPath }), + status, + binary, + ...(!binary && { patch: block }), + }; +} diff --git a/ts/packages/coda/src/handleReadActions.ts b/ts/packages/coda/src/handleReadActions.ts index c81d44a4a2..b3239844a5 100644 --- a/ts/packages/coda/src/handleReadActions.ts +++ b/ts/packages/coda/src/handleReadActions.ts @@ -4,6 +4,12 @@ import * as vscode from "vscode"; import * as path from "path"; import { ActionResult } from "./helpers"; +import { + type DiffSection, + parseDiffBlock, + parseUnifiedDiff, + splitDiffBlocks, +} from "./gitDiffUtils"; // Read/introspection action names served here. Kept in sync with the read // actions in packages/agents/code/src/codeActionsSchema.ts. @@ -14,12 +20,15 @@ const READ_ACTIONS = new Set([ "listOpenEditors", "getFileContent", "getWorkspaceChanges", + "getGitDiff", ]); type ReadActionParameters = { fileName?: string; startLine?: number; endLine?: number; + base?: string; + repository?: string; }; type ReadAction = { @@ -44,6 +53,15 @@ type GitRepository = { workingTreeChanges: GitChange[]; indexChanges: GitChange[]; }; + // Raw unified diff text for the whole repo: `cached=true` is the staged + // diff (index vs HEAD, i.e. `git diff --cached`), `cached=false`/omitted + // is the unstaged diff (working tree vs index, i.e. `git diff`). + diff(cached?: boolean): Promise; + // Raw unified diff text for a path (or "." for the whole repo, which the + // git extension passes straight through as `git diff -- .`, byte- + // identical to a path-less `git diff ` run from the repo root) + // between the working tree and an arbitrary ref. + diffWith(ref: string, path: string): Promise; }; type GitApi = { @@ -83,6 +101,8 @@ export async function handleReadActions( return ok(await getFileContent(params)); case "getWorkspaceChanges": return ok(await getWorkspaceChanges()); + case "getGitDiff": + return ok(await getGitDiff(params)); default: return { handled: false, message: "" }; } @@ -283,7 +303,10 @@ async function getFileContent(params: { }; } -async function getWorkspaceChanges() { +// Acquire the built-in git extension's API, activating it if needed. Shared +// by getWorkspaceChanges and getGitDiff so both report the same error for a +// missing/inactive extension. +async function getGitApi(): Promise { const gitExtension = vscode.extensions.getExtension("vscode.git"); if (!gitExtension) { @@ -292,7 +315,14 @@ async function getWorkspaceChanges() { const exports = gitExtension.isActive ? gitExtension.exports : await gitExtension.activate(); - const api = exports.getAPI(1); + return exports.getAPI(1); +} + +async function getWorkspaceChanges() { + const api = await getGitApi(); + if ("error" in api) { + return api; + } const repositories = api.repositories.map((repo) => ({ root: vscode.workspace.asRelativePath(repo.rootUri, false), branch: repo.state.HEAD?.name, @@ -310,6 +340,104 @@ async function getWorkspaceChanges() { return { repositories }; } +async function getGitDiff(params: ReadActionParameters) { + const api = await getGitApi(); + if ("error" in api) { + return api; + } + if (api.repositories.length === 0) { + return { error: "No git repositories are open in this workspace." }; + } + const selected = selectRepository(api.repositories, params.repository); + if ("error" in selected) { + return selected; + } + const repo = selected.repository; + const root = vscode.workspace.asRelativePath(repo.rootUri, false); + const branch = repo.state.HEAD?.name; + const base = params.base?.trim(); + + if (!base || base === "HEAD") { + const [unstagedText, stagedText] = await Promise.all([ + repo.diff(false), + repo.diff(true), + ]); + return { + root, + branch, + base: "HEAD", + unstaged: parseContainedDiff(unstagedText, repo), + staged: parseContainedDiff(stagedText, repo), + }; + } + + // A single repo-rooted diff (rather than one diffWith(base, path) call + // per changed file) avoids N serial git spawns -- which for a large + // changeset can exceed the action's timeout -- and reuses the same + // parseUnifiedDiff bounding/binary-detection logic as the default (no + // base) path above instead of duplicating it. + let diffText: string; + try { + diffText = await repo.diffWith(base, "."); + } catch (err) { + return { + error: `Failed to diff against "${base}": ${err instanceof Error ? err.message : String(err)}`, + }; + } + return { + root, + branch, + base, + diff: parseContainedDiff(diffText, repo), + }; +} + +// Pick the repository to diff. With a single open repository, `repository` +// is ignored (nothing to disambiguate). With multiple, match by workspace- +// relative root, root folder name, or absolute fs path; report the available +// roots on an ambiguous/unmatched selector instead of guessing. +function selectRepository( + repositories: GitRepository[], + repository: string | undefined, +): { repository: GitRepository } | { error: string } { + if (repositories.length === 1) { + return { repository: repositories[0] }; + } + const roots = repositories.map((repo) => ({ + repo, + relative: vscode.workspace.asRelativePath(repo.rootUri, false), + })); + if (!repository) { + return { + error: `Multiple git repositories are open; specify "repository" as one of: ${roots.map((r) => r.relative).join(", ")}.`, + }; + } + const needle = repository.trim(); + const match = roots.find( + (r) => + r.relative === needle || + path.basename(r.relative) === needle || + r.repo.rootUri.fsPath === needle, + ); + if (!match) { + return { + error: `No open git repository matches "${repository}". Available: ${roots.map((r) => r.relative).join(", ")}.`, + }; + } + return { repository: match.repo }; +} + +// True if `candidateFsPath` is `rootFsPath` itself or nested under it. +function isWithinRoot(candidateFsPath: string, rootFsPath: string): boolean { + const rootWithSep = rootFsPath.endsWith(path.sep) + ? rootFsPath + : rootFsPath + path.sep; + return ( + candidateFsPath === rootFsPath || + candidateFsPath.startsWith(rootWithSep) + ); +} + // Resolve a workspace-relative path or bare file name to a Uri inside an open // workspace folder. Rejects paths that escape the workspace root via `..` or // absolute components (matching the containment check used when creating files). @@ -321,16 +449,68 @@ function resolveWorkspaceFile(fileName: string): vscode.Uri | undefined { const trimmed = fileName.trim(); for (const folder of folders) { const candidate = vscode.Uri.joinPath(folder.uri, trimmed); - const root = folder.uri.fsPath; - const rootWithSep = root.endsWith(path.sep) ? root : root + path.sep; - const target = candidate.fsPath; - if (target === root || target.startsWith(rootWithSep)) { + if (isWithinRoot(candidate.fsPath, folder.uri.fsPath)) { return candidate; } } return undefined; } +// The git extension's repository root can sit above the open workspace +// folder(s) -- e.g. this very monorepo's "ts/" folder is opened as a +// workspace folder while its git repository root is the parent directory -- +// so a diff can otherwise return patch content for files outside every +// folder the user actually opened. getFileContent/getDiagnostics both +// refuse such paths via resolveWorkspaceFile; do the same here for +// consistency by dropping out-of-workspace files from the result (recording +// how many were dropped) rather than silently including them. A rename/copy +// also checks `oldPath`: its destination can sit inside the workspace while +// its source (and the patch's old-side content) came from outside it, which +// would otherwise leak repo-root-relative paths and content the workspace +// boundary is meant to hide. +// +// Filtering happens on the raw diff blocks *before* parseUnifiedDiff applies +// MAX_DIFF_FILES/MAX_SECTION_PATCH_BYTES, so an out-of-workspace file never +// crowds an in-workspace one out of those caps. Skipped entirely when no +// workspace folder is open (nothing to contain to, and nothing meaningfully +// "outside" without one). +function parseContainedDiff( + diffText: string, + repo: GitRepository, +): DiffSection { + const folders = vscode.workspace.workspaceFolders; + if (!folders || folders.length === 0) { + return parseUnifiedDiff(diffText); + } + const isInsideWorkspace = (relativePath: string) => { + const absolute = vscode.Uri.joinPath(repo.rootUri, relativePath); + return folders.some((folder) => + isWithinRoot(absolute.fsPath, folder.uri.fsPath), + ); + }; + let filesOutsideWorkspace = 0; + const containedBlocks = splitDiffBlocks(diffText).filter((block) => { + const entry = parseDiffBlock(block); + if (!entry) { + // Leave blocks that fail to parse for parseUnifiedDiff to count + // as filesUnparsed rather than silently dropping them here too. + return true; + } + const inside = + isInsideWorkspace(entry.path) && + (entry.oldPath === undefined || isInsideWorkspace(entry.oldPath)); + if (!inside) { + filesOutsideWorkspace++; + } + return inside; + }); + const section = parseUnifiedDiff(containedBlocks.join("")); + if (filesOutsideWorkspace === 0) { + return section; + } + return { ...section, filesOutsideWorkspace }; +} + // Map the VS Code git API Status enum (numeric) to a readable name. function gitStatusName(status: number): string { const names: Record = { diff --git a/ts/packages/coda/src/handleVSCodeActions.ts b/ts/packages/coda/src/handleVSCodeActions.ts index e53c70ab5d..d96633e593 100644 --- a/ts/packages/coda/src/handleVSCodeActions.ts +++ b/ts/packages/coda/src/handleVSCodeActions.ts @@ -589,7 +589,15 @@ export async function handleVSCodeActions(action: any) { actionResult = handledResult; } else { actionResult.handled = false; - actionResult.message = `Did not handle the action: "${actionName}"`; + // JSON-encode as {error: ...} (matching every operational-failure + // shape returned by handleReadActions) rather than a bare string, + // so extractOperationalError on the code-agent side correctly + // classifies this as a failed ActionResult instead of silently + // treating it as success (JSON.parse on a bare string throws and + // is caught/ignored there). + actionResult.message = JSON.stringify({ + error: `Did not handle the action: "${actionName}"`, + }); } } diff --git a/ts/packages/coda/test/gitDiffUtils.spec.ts b/ts/packages/coda/test/gitDiffUtils.spec.ts new file mode 100644 index 0000000000..8bc977c6b0 --- /dev/null +++ b/ts/packages/coda/test/gitDiffUtils.spec.ts @@ -0,0 +1,431 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import test from "node:test"; +import assert from "node:assert/strict"; +import { + MAX_DIFF_FILES, + MAX_FILE_PATCH_BYTES, + MAX_SECTION_PATCH_BYTES, + boundPatchText, + isBinaryDiffText, + parseDiffBlock, + parseUnifiedDiff, +} from "../src/gitDiffUtils.js"; + +test("boundPatchText returns text unchanged when under the byte budget", () => { + const [text, bytes, truncated] = boundPatchText("hello", 100); + assert.equal(text, "hello"); + assert.equal(bytes, 5); + assert.equal(truncated, false); +}); + +test("boundPatchText truncates at the byte budget and reports truncation", () => { + const [text, bytes, truncated] = boundPatchText("0123456789", 4); + assert.equal(text, "0123"); + assert.equal(bytes, 4); + assert.equal(truncated, true); +}); + +test("boundPatchText drops a trailing multi-byte character split by the byte budget", () => { + // "é" is 2 bytes in UTF-8; a 1-byte budget can't fit it, so it should be + // dropped entirely rather than emitting a corrupt/replacement character. + const [text, bytes, truncated] = boundPatchText("é", 1); + assert.equal(text, ""); + assert.equal(bytes, 0); + assert.equal(truncated, true); +}); + +test("boundPatchText keeps a complete multi-byte character that fits exactly", () => { + const [text, bytes, truncated] = boundPatchText("aé", 3); + assert.equal(text, "aé"); + assert.equal(bytes, 3); + assert.equal(truncated, false); +}); + +test("isBinaryDiffText detects the standard git binary-diff marker", () => { + assert.equal( + isBinaryDiffText("Binary files a/img.png and b/img.png differ\n"), + true, + ); +}); + +test("isBinaryDiffText detects GIT binary patch payloads", () => { + assert.equal( + isBinaryDiffText("diff --git a/x b/x\nGIT binary patch\nliteral 10\n"), + true, + ); +}); + +test("isBinaryDiffText returns false for a normal text patch", () => { + assert.equal( + isBinaryDiffText("diff --git a/x b/x\n@@ -1 +1 @@\n-old\n+new\n"), + false, + ); +}); + +test("parseDiffBlock identifies a modified file", () => { + const block = [ + "diff --git a/src/foo.ts b/src/foo.ts", + "index 111..222 100644", + "--- a/src/foo.ts", + "+++ b/src/foo.ts", + "@@ -1 +1 @@", + "-old", + "+new", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "src/foo.ts"); + assert.equal(entry.oldPath, undefined); + assert.equal(entry.status, "modified"); + assert.equal(entry.binary, false); + assert.equal(entry.patch, block); +}); + +test("parseDiffBlock identifies a new (added) file", () => { + const block = [ + "diff --git a/src/new.ts b/src/new.ts", + "new file mode 100644", + "--- /dev/null", + "+++ b/src/new.ts", + "@@ -0,0 +1 @@", + "+content", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "added"); + assert.equal(entry.oldPath, undefined); +}); + +test("parseDiffBlock identifies a deleted file", () => { + const block = [ + "diff --git a/src/gone.ts b/src/gone.ts", + "deleted file mode 100644", + "--- a/src/gone.ts", + "+++ /dev/null", + "@@ -1 +0,0 @@", + "-content", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "deleted"); +}); + +test("parseDiffBlock identifies a rename and reports the old path", () => { + const block = [ + "diff --git a/src/old.ts b/src/new.ts", + "similarity index 100%", + "rename from src/old.ts", + "rename to src/new.ts", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "renamed"); + assert.equal(entry.path, "src/new.ts"); + assert.equal(entry.oldPath, "src/old.ts"); +}); + +test("parseDiffBlock identifies a copy and reports the source path", () => { + const block = [ + "diff --git a/src/orig.ts b/src/copy.ts", + "similarity index 100%", + "copy from src/orig.ts", + "copy to src/copy.ts", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "copied"); + assert.equal(entry.oldPath, "src/orig.ts"); +}); + +test("parseDiffBlock marks binary diffs and omits patch text", () => { + const block = [ + "diff --git a/img.png b/img.png", + "index 111..222 100644", + "Binary files a/img.png and b/img.png differ", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.binary, true); + assert.equal(entry.patch, undefined); +}); + +test("parseDiffBlock returns undefined for text with no diff --git header", () => { + assert.equal(parseDiffBlock("not a diff at all"), undefined); +}); + +test("parseUnifiedDiff returns an empty section for empty text", () => { + const section = parseUnifiedDiff(""); + assert.deepEqual(section, { + files: [], + filesTruncated: false, + patchTruncated: false, + }); +}); + +test("parseUnifiedDiff splits multiple file blocks and preserves order", () => { + const text = [ + "diff --git a/a.ts b/a.ts", + "index 1..2 100644", + "--- a/a.ts", + "+++ b/a.ts", + "@@ -1 +1 @@", + "-a", + "+a2", + "diff --git a/b.ts b/b.ts", + "index 3..4 100644", + "--- a/b.ts", + "+++ b/b.ts", + "@@ -1 +1 @@", + "-b", + "+b2", + "", + ].join("\n"); + const section = parseUnifiedDiff(text); + assert.equal(section.files.length, 2); + assert.equal(section.files[0].path, "a.ts"); + assert.equal(section.files[1].path, "b.ts"); + assert.equal(section.filesTruncated, false); + assert.equal(section.patchTruncated, false); +}); + +test("parseUnifiedDiff caps per-file patch bytes and reports patchTruncated", () => { + const bigLine = "+".concat("x".repeat(MAX_FILE_PATCH_BYTES + 1000)); + const text = [ + "diff --git a/big.ts b/big.ts", + "index 1..2 100644", + "--- a/big.ts", + "+++ b/big.ts", + "@@ -1 +1 @@", + bigLine, + "", + ].join("\n"); + const section = parseUnifiedDiff(text); + assert.equal(section.files.length, 1); + assert.equal(section.patchTruncated, true); + assert.ok( + Buffer.byteLength(section.files[0].patch ?? "", "utf8") <= + MAX_FILE_PATCH_BYTES, + ); +}); + +test("parseUnifiedDiff caps the total section byte budget across many files", () => { + // Each file's patch is small individually (well under the per-file cap) + // but there are enough of them to blow through the shared section budget. + const perFileBytes = 500; + const fileCount = Math.ceil(MAX_SECTION_PATCH_BYTES / perFileBytes) + 5; + const blocks: string[] = []; + for (let i = 0; i < fileCount; i++) { + blocks.push( + [ + `diff --git a/f${i}.ts b/f${i}.ts`, + "index 1..2 100644", + "--- a/f" + i + ".ts", + "+++ b/f" + i + ".ts", + "@@ -1 +1 @@", + "+" + "x".repeat(perFileBytes), + "", + ].join("\n"), + ); + } + const section = parseUnifiedDiff(blocks.join("")); + assert.equal(section.patchTruncated, true); + // Total bytes actually kept across all patches should not exceed the + // shared section budget (a small header/margin is allowed). + const totalPatchBytes = section.files.reduce( + (sum, f) => sum + Buffer.byteLength(f.patch ?? "", "utf8"), + 0, + ); + assert.ok(totalPatchBytes <= MAX_SECTION_PATCH_BYTES); +}); + +test("parseUnifiedDiff caps the number of files and reports filesTruncated", () => { + const blocks: string[] = []; + for (let i = 0; i < MAX_DIFF_FILES + 1; i++) { + blocks.push(`diff --git a/f${i}.ts b/f${i}.ts\nindex 1..2 100644\n`); + } + const text = blocks.join(""); + const section = parseUnifiedDiff(text); + assert.equal(section.files.length, MAX_DIFF_FILES); + assert.equal(section.filesTruncated, true); +}); + +test("parseDiffBlock unquotes a C-escaped non-ASCII header path", () => { + // git quotes a path (independently on each side) whenever it contains a + // non-ASCII UTF-8 byte, which is the default for accented/CJK/emoji + // filenames (core.quotepath=true). "café.txt" is octal-escaped as + // \303\251 (UTF-8 for "é"). + const block = [ + 'diff --git "a/caf\\303\\251.txt" "b/caf\\303\\251.txt"', + "index 111..222 100644", + '--- "a/caf\\303\\251.txt"', + '+++ "b/caf\\303\\251.txt"', + "@@ -1 +1 @@", + "-old", + "+new", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "café.txt"); + assert.equal(entry.status, "modified"); +}); + +test("parseDiffBlock unquotes a rename with a mixed quoted/unquoted header", () => { + // A rename from an ASCII to a non-ASCII name quotes only the side that + // needs it, and the from/to paths themselves are also independently + // quoted only when needed. + const block = [ + 'diff --git a/old.txt "b/caf\\303\\251 new.txt"', + "similarity index 100%", + "rename from old.txt", + 'rename to "caf\\303\\251 new.txt"', + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "renamed"); + assert.equal(entry.path, "café new.txt"); + assert.equal(entry.oldPath, "old.txt"); +}); + +test("parseDiffBlock unquotes a header path with a literal (unescaped) non-ASCII character", () => { + // With core.quotepath=false, a path is still quoted when it contains a + // structural character needing a C escape (e.g. a literal double quote), + // but any non-ASCII bytes appear literally rather than octal-escaped. + // Unescaped characters must round-trip as themselves, not be + // mis-decoded as raw UTF-8 bytes (e.g. "é" mistaken for byte 0xE9). + const block = [ + 'diff --git "a/caf\\"é.txt" "b/caf\\"é.txt"', + "index 111..222 100644", + '--- "a/caf\\"é.txt"', + '+++ "b/caf\\"é.txt"', + "@@ -1 +1 @@", + "-old", + "+new", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, 'caf"é.txt'); + assert.equal(entry.status, "modified"); +}); + +test("parseDiffBlock handles an unquoted (ASCII) path containing a space", () => { + // Real git appends a bare trailing tab to `---`/`+++` lines whenever the + // path contains a literal space (a unified-diff convention disambiguating + // the filename from a would-be timestamp) -- confirmed against real + // `git diff` output on a scratch repo. That tab must not become part of + // the reported path. + const block = [ + "diff --git a/my file.txt b/my file.txt", + "index 111..222 100644", + "--- a/my file.txt\t", + "+++ b/my file.txt\t", + "@@ -1 +1 @@", + "-old", + "+new", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "my file.txt"); + assert.equal(entry.status, "modified"); +}); + +test("parseDiffBlock strips the trailing tab from a quoted, space-containing path", () => { + // When a spaced path also needs C-quoting (e.g. it's also non-ASCII), + // git's disambiguating tab is appended *after* the closing quote -- + // confirmed against real `git diff` output. + const block = [ + 'diff --git "a/caf\\303\\251 space.txt" "b/caf\\303\\251 space.txt"', + "index 111..222 100644", + '--- "a/caf\\303\\251 space.txt"\t', + '+++ "b/caf\\303\\251 space.txt"\t', + "@@ -1 +1 @@", + "-old", + "+new", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "café space.txt"); + assert.equal(entry.status, "modified"); +}); + +test("parseDiffBlock does not add a trailing tab to rename from/to lines", () => { + // Confirmed against real `git diff` output: unlike ---/+++, rename + // from/to lines never get the disambiguating tab even for spaced paths. + const block = [ + "diff --git a/old name.txt b/new name.txt", + "similarity index 100%", + "rename from old name.txt", + "rename to new name.txt", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.status, "renamed"); + assert.equal(entry.path, "new name.txt"); + assert.equal(entry.oldPath, "old name.txt"); +}); + +test("parseDiffBlock unquotes a binary-diff header with a non-ASCII name (no ---/+++ lines)", () => { + // A binary diff has neither ---/+++ nor rename/copy lines, so the path + // must come from the combined "diff --git" header; both sides are + // always the same path here (a rename would emit rename from/to + // instead), and the quoted form must still be unquoted correctly. + const block = [ + 'diff --git "a/bild\\303\\244.bin" "b/bild\\303\\244.bin"', + "index 111..222 100644", + "Binary files a/bildä.bin and b/bildä.bin differ", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "bildä.bin"); + assert.equal(entry.binary, true); + assert.equal(entry.patch, undefined); +}); + +test("parseDiffBlock unquotes a mode-only-change header with a non-ASCII name", () => { + const block = [ + 'diff --git "a/caf\\303\\251.sh" "b/caf\\303\\251.sh"', + "old mode 100644", + "new mode 100755", + "", + ].join("\n"); + const entry = parseDiffBlock(block); + assert.ok(entry); + assert.equal(entry.path, "café.sh"); + assert.equal(entry.status, "modified"); +}); + +test("parseUnifiedDiff reports filesUnparsed for a block it cannot recover a path from", () => { + // A block with a header but no rename/copy/---/+++ lines and no + // "a/X b/Y" (or quoted) split possible is dropped, but counted rather + // than silently vanishing from the result. + const text = [ + "diff --git a/normal.ts b/normal.ts", + "index 1..2 100644", + "--- a/normal.ts", + "+++ b/normal.ts", + "@@ -1 +1 @@", + "-old", + "+new", + "diff --git weird-no-ab-split", + "old mode 100644", + "", + ].join("\n"); + const section = parseUnifiedDiff(text); + assert.equal(section.files.length, 1); + assert.equal(section.files[0].path, "normal.ts"); + assert.equal(section.filesUnparsed, 1); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 23f2e1807f..743fd132f0 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -2443,6 +2443,9 @@ importers: '@typeagent/action-grammar-compiler': specifier: workspace:* version: link:../../actionGrammarCompiler + '@typeagent/action-schema': + specifier: workspace:* + version: link:../../actionSchema '@typeagent/action-schema-compiler': specifier: workspace:* version: link:../../actionSchemaCompiler @@ -4467,6 +4470,9 @@ importers: rimraf: specifier: ^6.0.1 version: 6.0.1 + tsx: + specifier: ^4.21.0 + version: 4.21.0 packages/codeProcessor: dependencies: