fix: optimize Windows apply-patch fs path

This commit is contained in:
Codex
2026-06-26 17:05:09 +00:00
parent 9bce21b9ef
commit 624904c383
5 changed files with 404 additions and 22 deletions
+247
View File
@@ -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<string, string>([["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<string, string>([
["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");
});
});
+78 -6
View File
@@ -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<Extract<PatchHunk, { kind: "update" }>>): Promise<ApplyPatchV2Plan> {
@@ -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<string, unknown>): 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<string, unknown>): Record<string, unknown> | 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<string, unknown>, key: string): string | undefined {
const value = record[key];
return typeof value === "string" ? value : undefined;
}
function numberOrStringField(record: Record<string, unknown>, 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<string, unknown>, 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<string, unknown>): 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<Map<string, string>> {
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<string, string>();
+4 -3
View File
@@ -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,
+4 -3
View File
@@ -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,
+71 -10
View File
@@ -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<SshCaptureResult>): ApplyPatchV2FileSystem {
async function checked(operation: WindowsApplyPatchFsOperation, args: string[], input?: string): Promise<SshCaptureResult> {
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<string, unknown>;
}): Record<string, unknown> {
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,