diff --git a/scripts/src/apply-patch-v2.test.ts b/scripts/src/apply-patch-v2.test.ts new file mode 100644 index 00000000..c5455697 --- /dev/null +++ b/scripts/src/apply-patch-v2.test.ts @@ -0,0 +1,247 @@ +import { createHash } from "node:crypto"; +import { Readable, Writable } from "node:stream"; +import { describe, expect, test } from "bun:test"; +import { + ApplyPatchV2Error, + runApplyPatchV2, + type ApplyPatchV2BulkReplacementWritePlan, + type ApplyPatchV2FileSystem, +} from "./apply-patch-v2"; + +class CaptureWritable extends Writable { + chunks: Buffer[] = []; + + _write(chunk: Buffer | string, _encoding: BufferEncoding, callback: (error?: Error | null) => void): void { + this.chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)); + callback(); + } + + text(): string { + return Buffer.concat(this.chunks).toString("utf8"); + } +} + +function sha256Hex(text: string): string { + return createHash("sha256").update(text, "utf8").digest("hex"); +} + +function applyReplacementPlan(original: string, plan: ApplyPatchV2BulkReplacementWritePlan): string { + const lines = original.split("\n"); + if (lines.at(-1) === "") lines.pop(); + for (const [start, oldLength, newLines] of [...plan.replacements].sort((left, right) => right[0] - left[0])) { + lines.splice(start, oldLength, ...newLines); + } + return lines.length === 0 ? "" : `${lines.join("\n")}\n`; +} + +describe("apply-patch v2 fs bulk update path", () => { + test("single fs update uses readFiles and applyReplacementsBulk instead of whole-file write", async () => { + const files = new Map([["docs/test.md", "alpha\nold\nomega\n"]]); + const calls: string[] = []; + const fs: ApplyPatchV2FileSystem = { + async stat(path) { + calls.push(`stat:${path}`); + throw new Error("stat should not be used by the single-file bulk path"); + }, + async readBlock(path) { + calls.push(`readBlock:${path}`); + throw new Error("readBlock should not be used by the single-file bulk path"); + }, + async writeFile(path) { + calls.push(`writeFile:${path}`); + throw new Error("writeFile should not be used for update hunks with replacement bulk"); + }, + async deleteFile(path) { + calls.push(`deleteFile:${path}`); + }, + async readFiles(paths) { + calls.push(`readFiles:${paths.join(",")}`); + return new Map(paths.map((path) => [path, files.get(path) ?? ""])); + }, + async applyReplacementsBulk(paths, plans) { + const targets = Array.from(paths); + calls.push(`applyReplacementsBulk:${targets.join(",")}`); + for (const target of targets) { + const plan = plans.get(target); + if (plan === undefined) throw new Error(`missing plan for ${target}`); + const next = applyReplacementPlan(files.get(target) ?? "", plan); + expect(Buffer.byteLength(next, "utf8")).toBe(plan.finalBytes); + expect(sha256Hex(next)).toBe(plan.finalSha256); + files.set(target, next); + } + }, + }; + const stdout = new CaptureWritable(); + const stderr = new CaptureWritable(); + + const exitCode = await runApplyPatchV2({ + executor: { fs }, + stdin: Readable.from([[ + "*** Begin Patch", + "*** Update File: docs/test.md", + "@@", + " alpha", + "-old", + "+new", + " omega", + "*** End Patch", + ].join("\n")]), + stdout, + stderr, + timing: { transport: "local", route: "D601:win/F/Work/ConStart" }, + }); + + expect(exitCode).toBe(0); + expect(files.get("docs/test.md")).toBe("alpha\nnew\nomega\n"); + expect(calls).toEqual(["readFiles:docs/test.md", "applyReplacementsBulk:docs/test.md"]); + expect(stdout.text()).toContain("M docs/test.md"); + expect(stderr.text()).toContain("\"remoteOperationCounts\":{\"fs.readFiles\":1,\"fs.applyReplacementsBulk\":1}"); + }); + + test("bulk read failure falls back to block reads but still writes through replacement bulk", async () => { + const files = new Map([ + ["a.md", "first\nold-a\nlast\n"], + ["b.md", "first\nold-b\nlast\n"], + ]); + const calls: string[] = []; + const fs: ApplyPatchV2FileSystem = { + async stat(path) { + calls.push(`stat:${path}`); + const text = files.get(path); + if (text === undefined) throw new Error(`missing ${path}`); + return { bytes: Buffer.byteLength(text, "utf8"), sha256: sha256Hex(text) }; + }, + async readBlock(path, blockIndex, blockBytes) { + calls.push(`readBlock:${path}:${blockIndex}:${blockBytes}`); + const text = files.get(path); + if (text === undefined) throw new Error(`missing ${path}`); + return blockIndex === 0 ? Buffer.from(text, "utf8") : Buffer.alloc(0); + }, + async writeFile(path) { + calls.push(`writeFile:${path}`); + throw new Error("writeFile should not be used after bulk read fallback"); + }, + async deleteFile(path) { + calls.push(`deleteFile:${path}`); + }, + async readFiles(paths) { + calls.push(`readFiles:${paths.join(",")}`); + throw new ApplyPatchV2Error("windows apply-patch fs operation failed: read-bulk-b64", { + operation: "read-bulk-b64", + targetCount: paths.length, + remoteElapsedMs: 60000, + landed: false, + }); + }, + async applyReplacementsBulk(paths, plans) { + const targets = Array.from(paths); + calls.push(`applyReplacementsBulk:${targets.join(",")}`); + for (const target of targets) { + const plan = plans.get(target); + if (plan === undefined) throw new Error(`missing plan for ${target}`); + files.set(target, applyReplacementPlan(files.get(target) ?? "", plan)); + } + }, + }; + const stdout = new CaptureWritable(); + const stderr = new CaptureWritable(); + + const exitCode = await runApplyPatchV2({ + executor: { fs }, + stdin: Readable.from([[ + "*** Begin Patch", + "*** Update File: a.md", + "@@", + " first", + "-old-a", + "+new-a", + " last", + "*** Update File: b.md", + "@@", + " first", + "-old-b", + "+new-b", + " last", + "*** End Patch", + ].join("\n")]), + stdout, + stderr, + timing: { transport: "local", route: "D601:win/F/Work/ConStart" }, + }); + + expect(exitCode).toBe(0); + expect(files.get("a.md")).toBe("first\nnew-a\nlast\n"); + expect(files.get("b.md")).toBe("first\nnew-b\nlast\n"); + expect(calls).toEqual([ + "readFiles:a.md,b.md", + "stat:a.md", + "readBlock:a.md:0:45000", + "stat:b.md", + "readBlock:b.md:0:45000", + "applyReplacementsBulk:a.md,b.md", + ]); + expect(stdout.text()).toContain("M a.md"); + expect(stdout.text()).toContain("M b.md"); + expect(stderr.text()).toContain("\"remoteFailureCount\":1"); + }); + + test("remote fs failure prints operation details", async () => { + const fs: ApplyPatchV2FileSystem = { + async stat() { + throw new Error("stat should not be used"); + }, + async readBlock() { + throw new Error("readBlock should not be used"); + }, + async writeFile() { + throw new Error("writeFile should not be used"); + }, + async deleteFile() { + throw new Error("deleteFile should not be used"); + }, + async readFiles(paths) { + return new Map(paths.map((path) => [path, "alpha\nold\nomega\n"])); + }, + async applyReplacementsBulk() { + throw new ApplyPatchV2Error("windows apply-patch fs operation failed: apply-replacements-bulk-stdin", { + operation: "apply-replacements-bulk-stdin", + route: "D601:win/F/Work/ConStart", + targetCount: 1, + inputBytes: 190, + expectedBytes: "22", + expectedSha256: "abc123", + remoteElapsedMs: 60000, + landed: false, + exitCode: 124, + stderrTail: "UNIDESK_SSH_RUNTIME_TIMEOUT timeoutSeconds=60", + }); + }, + }; + const stdout = new CaptureWritable(); + const stderr = new CaptureWritable(); + + const exitCode = await runApplyPatchV2({ + executor: { fs }, + stdin: Readable.from([[ + "*** Begin Patch", + "*** Update File: docs/test.md", + "@@", + " alpha", + "-old", + "+new", + " omega", + "*** End Patch", + ].join("\n")]), + stdout, + stderr, + timing: { transport: "local", route: "D601:win/F/Work/ConStart" }, + }); + + expect(exitCode).toBe(1); + expect(stdout.text()).toBe(""); + const err = stderr.text(); + expect(err).toContain("Remote operation: operation=apply-replacements-bulk-stdin route=D601:win/F/Work/ConStart targetCount=1 inputBytes=190 expectedBytes=22 sha256=abc123 elapsedMs=60000 landed=false exitCode=124"); + expect(err).toContain("Remote stderr tail:"); + expect(err).toContain("UNIDESK_SSH_RUNTIME_TIMEOUT timeoutSeconds=60"); + }); +}); diff --git a/scripts/src/apply-patch-v2.ts b/scripts/src/apply-patch-v2.ts index 6bdc7e2f..7e945b03 100644 --- a/scripts/src/apply-patch-v2.ts +++ b/scripts/src/apply-patch-v2.ts @@ -632,6 +632,7 @@ function isBulkReadFailure(error: unknown): boolean { } function shouldUseBulkUpdatePath(executor: ApplyPatchV2Executor, hunks: PatchHunk[]): boolean { + const isFsBulkPath = executor.fs !== undefined; if (executor.fs !== undefined) { if (executor.fs.readFiles === undefined || executor.fs.applyReplacementsBulk === undefined) return false; } else if (!executor.run) { @@ -643,7 +644,7 @@ function shouldUseBulkUpdatePath(executor: ApplyPatchV2Executor, hunks: PatchHun if (paths.has(hunk.path)) return false; paths.add(hunk.path); } - return paths.size >= 2; + return isFsBulkPath ? paths.size >= 1 : paths.size >= 2; } async function applyPatchV2UpdateHunksBulk(executor: ApplyPatchV2Executor, hunks: Array>): Promise { @@ -733,6 +734,7 @@ function formatApplyPatchFailure(error: unknown): string { const outcomes = Array.isArray(details.outcomes) ? details.outcomes as ApplyPatchV2Outcome[] : []; const lines = [`${message.trimEnd()}`]; appendParserFailureHint(lines, message, details); + appendRemoteOperationFailureDetails(lines, details); appendExpectedLinesFailureHint(lines, details); if (outcomes.length > 1 || outcomes.some((item) => item.status === "applied")) { lines.push("Patch status:"); @@ -757,6 +759,72 @@ function appendParserFailureHint(lines: string[], message: string, details: Reco } } +function appendRemoteOperationFailureDetails(lines: string[], details: Record): void { + const remote = remoteOperationFailureRecord(details); + if (remote === null) return; + const summaryFields = [ + ["operation", stringField(remote, "operation")], + ["route", stringField(remote, "route")], + ["file", stringField(remote, "filePath") ?? stringField(remote, "path")], + ["targetCount", numberOrStringField(remote, "targetCount")], + ["inputBytes", numberOrStringField(remote, "inputBytes")], + ["expectedBytes", numberOrStringField(remote, "expectedBytes")], + ["sha256", stringField(remote, "expectedSha256") ?? stringField(remote, "sha256")], + ["elapsedMs", numberOrStringField(remote, "remoteElapsedMs")], + ["landed", booleanOrStringField(remote, "landed")], + ["exitCode", numberOrStringField(remote, "exitCode")], + ].filter(([, value]) => value !== undefined && value !== null && value !== ""); + if (summaryFields.length > 0) { + lines.push(`Remote operation: ${summaryFields.map(([key, value]) => `${key}=${value}`).join(" ")}`); + } + const stderrTail = stringField(remote, "stderrTail") ?? stringField(remote, "stderr"); + if (stderrTail !== undefined && stderrTail.trim().length > 0) { + lines.push("Remote stderr tail:"); + appendQuotedBlock(lines, stderrTail.trim(), 8, 1200); + } + const stdoutTail = stringField(remote, "stdoutTail") ?? stringField(remote, "stdout"); + if (stdoutTail !== undefined && stdoutTail.trim().length > 0) { + lines.push("Remote stdout tail:"); + appendQuotedBlock(lines, stdoutTail.trim(), 6, 900); + } +} + +function remoteOperationFailureRecord(details: Record): Record | null { + if (typeof details.operation === "string") return details; + const cause = recordValue(details.cause); + if (cause !== null) { + const nested = remoteOperationFailureRecord(cause); + if (nested !== null) return nested; + } + const failed = recordValue(details.failed); + const failedError = recordValue(failed?.error); + const failedDetails = recordValue(failedError?.details); + if (failedDetails !== null) { + const nested = remoteOperationFailureRecord(failedDetails); + if (nested !== null) return nested; + } + return null; +} + +function stringField(record: Record, key: string): string | undefined { + const value = record[key]; + return typeof value === "string" ? value : undefined; +} + +function numberOrStringField(record: Record, key: string): string | undefined { + const value = record[key]; + if (typeof value === "number" && Number.isFinite(value)) return String(value); + if (typeof value === "string" && value.length > 0) return value; + return undefined; +} + +function booleanOrStringField(record: Record, key: string): string | undefined { + const value = record[key]; + if (typeof value === "boolean") return String(value); + if (typeof value === "string" && value.length > 0) return value; + return undefined; +} + function appendExpectedLinesFailureHint(lines: string[], details: Record): void { const cause = recordValue(details.cause) ?? details; const expected = typeof cause.expected === "string" ? cause.expected : ""; @@ -941,12 +1009,16 @@ async function readRemoteText(executor: ApplyPatchV2Executor, target: string): P async function readRemoteTextsBulk(executor: ApplyPatchV2Executor, targets: string[]): Promise> { if (targets.length === 0) return new Map(); - if (executor.fs?.readFiles !== undefined && targets.length > 1) { - const files = await executor.fs.readFiles(targets); - for (const target of targets) { - if (!files.has(target)) throw new ApplyPatchV2Error("remote apply-patch v2 fs bulk read omitted target", { target }); + if (executor.fs?.readFiles !== undefined) { + try { + const files = await executor.fs.readFiles(targets); + for (const target of targets) { + if (!files.has(target)) throw new ApplyPatchV2Error("remote apply-patch v2 fs bulk read omitted target", { target }); + } + return files; + } catch (error) { + if (!isBulkReadFailure(error)) throw error; } - return files; } if (executor.fs || targets.length === 1) { const files = new Map(); diff --git a/scripts/src/remote-ssh.ts b/scripts/src/remote-ssh.ts index 92ed2f99..b95e3e03 100644 --- a/scripts/src/remote-ssh.ts +++ b/scripts/src/remote-ssh.ts @@ -7,6 +7,7 @@ import { type UniDeskConfig } from "./config"; import { type RemoteCliOptions } from "./remote-options"; import { buildWindowsPowerShellInvocation, + createWindowsApplyPatchFileSystem, createSshStdoutForwarder, formatSshFailureHint, formatSshRuntimeTimeoutHint, @@ -901,9 +902,9 @@ async function runRemoteSshOverFrontend(session: FrontendSession, target: string }); } if ((normalizedArgs[0] ?? "") === "apply-patch") { - const executor: ApplyPatchV2Executor = { - run: (command, input) => runRemoteSshWebSocketCapture(session, invocation, command, input), - }; + const executor: ApplyPatchV2Executor = invocation.route.plane === "win" + ? { fs: createWindowsApplyPatchFileSystem(invocation, (remoteCommand, input) => runRemoteSshWebSocketCaptureRemoteCommand(session, invocation, remoteCommand, input)) } + : { run: (command, input) => runRemoteSshWebSocketCapture(session, invocation, command, input) }; return await runApplyPatchV2({ executor, stdin: process.stdin, diff --git a/scripts/src/remote.ts b/scripts/src/remote.ts index 0e6709bc..15cbfdea 100644 --- a/scripts/src/remote.ts +++ b/scripts/src/remote.ts @@ -8,6 +8,7 @@ import { summarizeMicroserviceHealthResponse, summarizeMicroserviceObservation, import { parseNetworkPerfOptions, runNetworkPerf } from "./network-perf"; import { buildWindowsPowerShellInvocation, + createWindowsApplyPatchFileSystem, createSshStdoutForwarder, formatSshFailureHint, formatSshRuntimeTimeoutHint, @@ -1552,9 +1553,9 @@ async function runRemoteSshOverFrontend(session: FrontendSession, target: string }); } if ((normalizedArgs[0] ?? "") === "apply-patch") { - const executor: ApplyPatchV2Executor = { - run: (command, input) => runRemoteSshWebSocketCapture(session, invocation, command, input), - }; + const executor: ApplyPatchV2Executor = invocation.route.plane === "win" + ? { fs: createWindowsApplyPatchFileSystem(invocation, (remoteCommand, input) => runRemoteSshWebSocketCaptureRemoteCommand(session, invocation, remoteCommand, input)) } + : { run: (command, input) => runRemoteSshWebSocketCapture(session, invocation, command, input) }; return await runApplyPatchV2({ executor, stdin: process.stdin, diff --git a/scripts/src/ssh.ts b/scripts/src/ssh.ts index 25b8f7c9..f2ed05fc 100644 --- a/scripts/src/ssh.ts +++ b/scripts/src/ssh.ts @@ -5,6 +5,7 @@ import { tmpdir } from "node:os"; import { join } from "node:path"; import { type UniDeskConfig, repoRoot } from "./config"; import { + ApplyPatchV2Error, decodeApplyPatchV2BulkRead, formatApplyPatchV2BulkReplacementPayload, isApplyPatchV2HelpArgs, @@ -2860,18 +2861,32 @@ type WindowsApplyPatchFsOperation = const windowsApplyPatchWriteB64ChunkChars = 12_000; -function createWindowsApplyPatchFileSystem(config: UniDeskConfig, invocation: ParsedSshInvocation): ApplyPatchV2FileSystem { +export function createWindowsApplyPatchFileSystem(invocation: ParsedSshInvocation, runRemoteCommand: (remoteCommand: string, input?: string) => Promise): ApplyPatchV2FileSystem { async function checked(operation: WindowsApplyPatchFsOperation, args: string[], input?: string): Promise { const command = buildWindowsPowerShellInvocation(windowsApplyPatchFsScript(invocation.route.workspace, operation, args)); - const result = await runSshCaptureRemoteCommand(config, invocation, command, input); + const startedAtMs = Date.now(); + let result: SshCaptureResult; + try { + result = await runRemoteCommand(command, input); + } catch (error) { + throw new ApplyPatchV2Error(`windows apply-patch fs operation failed: ${operation}`, windowsApplyPatchFsFailureDetails({ + operation, + args, + input, + invocation, + remoteElapsedMs: Math.max(0, Date.now() - startedAtMs), + cause: error instanceof Error ? { name: error.name, message: error.message } : { message: String(error) }, + })); + } if (result.exitCode === 0) return result; - throw new Error(`windows apply-patch fs operation failed: ${operation} ${JSON.stringify({ - route: invocation.route.raw, - args: args.slice(0, 4), - exitCode: result.exitCode, - stdout: result.stdout.slice(-2000), - stderr: result.stderr.slice(-4000), - })}`); + throw new ApplyPatchV2Error(`windows apply-patch fs operation failed: ${operation}`, windowsApplyPatchFsFailureDetails({ + operation, + args, + input, + invocation, + result, + remoteElapsedMs: Math.max(0, Date.now() - startedAtMs), + })); } return { @@ -2920,6 +2935,52 @@ function createWindowsApplyPatchFileSystem(config: UniDeskConfig, invocation: Pa }; } +function windowsApplyPatchFsFailureDetails(options: { + operation: WindowsApplyPatchFsOperation; + args: string[]; + input?: string; + invocation: ParsedSshInvocation; + result?: SshCaptureResult; + remoteElapsedMs: number; + cause?: Record; +}): Record { + const { operation, args, input, invocation, result } = options; + const inputBytes = input === undefined ? 0 : Buffer.byteLength(input, "utf8"); + const expected = windowsApplyPatchExpectedWrite(operation, args); + const targetCount = windowsApplyPatchBulkTargetCount(operation, args); + return { + operation, + route: invocation.route.raw, + ...(operation === "read-bulk-b64" || operation === "apply-replacements-bulk-stdin" ? {} : { filePath: args[0] ?? "" }), + ...(targetCount === null ? {} : { targetCount }), + args: args.slice(0, 4), + inputBytes, + ...(inputBytes === 0 ? {} : { inputSha256: createHash("sha256").update(input, "utf8").digest("hex") }), + ...(expected.expectedBytes === undefined ? {} : { expectedBytes: expected.expectedBytes }), + ...(expected.expectedSha256 === undefined ? {} : { expectedSha256: expected.expectedSha256 }), + remoteElapsedMs: options.remoteElapsedMs, + landed: false, + ...(result === undefined ? {} : { + exitCode: result.exitCode, + stdoutTail: result.stdout.slice(-2000), + stderrTail: result.stderr.slice(-4000), + }), + ...(options.cause === undefined ? {} : { cause: options.cause }), + }; +} + +function windowsApplyPatchExpectedWrite(operation: WindowsApplyPatchFsOperation, args: string[]): { expectedBytes?: string; expectedSha256?: string } { + if (operation === "write-b64-stdin") return { expectedBytes: args[1], expectedSha256: args[2] }; + if (operation === "write-b64-commit") return { expectedBytes: args[2], expectedSha256: args[3] }; + return {}; +} + +function windowsApplyPatchBulkTargetCount(operation: WindowsApplyPatchFsOperation, args: string[]): number | null { + if (operation !== "read-bulk-b64" && operation !== "apply-replacements-bulk-stdin") return null; + const count = Number(args[0]); + return Number.isSafeInteger(count) && count >= 0 ? count : null; +} + function windowsApplyPatchFsScript(basePath: string | null, operation: WindowsApplyPatchFsOperation, args: string[]): string { const target = args[0] ?? ""; const arg1 = args[1] ?? ""; @@ -3362,7 +3423,7 @@ export async function runSsh(config: UniDeskConfig, providerId: string, args: st if (operationName === "apply-patch") { const applyPatch = effectiveApplyPatchV2Invocation(invocation, normalizedArgs.slice(1)); const executor: ApplyPatchV2Executor = applyPatch.invocation.route.plane === "win" - ? { fs: createWindowsApplyPatchFileSystem(config, applyPatch.invocation) } + ? { fs: createWindowsApplyPatchFileSystem(applyPatch.invocation, (remoteCommand, input) => runSshCaptureRemoteCommand(config, applyPatch.invocation, remoteCommand, input)) } : { run: (command, input) => runSshCaptureCommand(config, applyPatch.invocation, command, input) }; return await runApplyPatchV2({ executor,