fix: expose apply-patch timing summary

This commit is contained in:
Codex
2026-06-06 18:12:58 +00:00
parent 3f26f4f359
commit 27acd2a98f
6 changed files with 234 additions and 5 deletions
+200 -2
View File
@@ -50,6 +50,7 @@ export interface ApplyPatchV2Options {
stdout: Writable;
stderr?: Writable;
argv?: string[];
timing?: ApplyPatchV2TimingOptions;
}
export interface ApplyPatchV2Executor {
@@ -75,6 +76,35 @@ export interface ApplyPatchV2RemoteResult {
stderr: string;
}
export interface ApplyPatchV2TimingOptions {
providerId?: string;
route?: string;
transport?: "backend-core-broker" | "frontend-websocket" | "local" | "virtual";
thresholdMs?: number;
}
export interface ApplyPatchV2TimingSummary {
code: "apply-patch-v2-timing";
level: "info" | "warning";
status: "succeeded" | "failed";
providerId?: string;
route?: string;
transport?: ApplyPatchV2TimingOptions["transport"];
durationMs: number;
thresholdMs: number;
slow: boolean;
patchBytes: number;
fileCount: number;
hunkCount: number;
changedCount: number;
remoteOperationCount: number;
remoteOperationCounts: Record<string, number>;
remoteElapsedMs: number;
remoteFailureCount: number;
message: string;
note: string;
}
export class ApplyPatchV2Error extends Error {
constructor(message: string, public readonly details: Record<string, unknown> = {}) {
super(message);
@@ -91,6 +121,13 @@ interface ApplyPatchV2Plan {
outcomes: ApplyPatchV2Outcome[];
}
export interface ApplyPatchV2RemoteMetrics {
remoteOperationCount: number;
remoteOperationCounts: Record<string, number>;
remoteElapsedMs: number;
remoteFailureCount: number;
}
const beginMarker = "*** Begin Patch";
const endMarker = "*** End Patch";
const environmentIdMarker = "*** Environment ID: ";
@@ -101,6 +138,7 @@ const moveToMarker = "*** Move to: ";
const emptyChangeContextMarker = "@@";
const changeContextMarker = "@@ ";
const eofMarker = "*** End of File";
const defaultApplyPatchV2SlowWarningMs = 10_000;
export function isApplyPatchV2HelpArgs(args: string[] = []): boolean {
const first = args[0] ?? "";
@@ -121,6 +159,24 @@ export function applyPatchV2HelpPayload() {
firstLine: beginMarker,
lastLine: endMarker
},
timing: {
stderrPrefix: "UNIDESK_APPLY_PATCH_TIMING",
fields: [
"durationMs",
"patchBytes",
"fileCount",
"hunkCount",
"changedCount",
"remoteOperationCount",
"remoteOperationCounts",
"remoteElapsedMs",
"remoteFailureCount",
"providerId",
"route",
"transport"
],
stdoutCompatibility: "Timing is written to stderr so success stdout remains Codex apply_patch-compatible."
},
rules: [
"Add File has no @@ hunk marker: put content immediately after `*** Add File: <path>` and prefix every content line with +.",
"A blank line in Add File is a line containing only +.",
@@ -302,20 +358,162 @@ export async function runApplyPatchV2(options: ApplyPatchV2Options): Promise<num
stderr.write("ssh apply-patch requires patch text on stdin.\n");
return 2;
}
const startedAtMs = Date.now();
const metrics = createApplyPatchV2RemoteMetrics();
const executor = instrumentApplyPatchV2Executor(options.executor, metrics);
let parsed: PatchParseResult | null = null;
let plan: ApplyPatchV2Plan | null = null;
try {
const parsed = parseApplyPatchV2(patchText);
const plan = await applyPatchV2Hunks(options.executor, parsed.hunks);
parsed = parseApplyPatchV2(patchText);
plan = await applyPatchV2Hunks(executor, parsed.hunks);
for (const hint of parsed.hints) stderr.write(`${hint}\n`);
options.stdout.write("Success. Updated the following files:\n");
for (const item of plan.changed) options.stdout.write(`${item}\n`);
stderr.write(formatApplyPatchV2TimingSummary(applyPatchV2TimingSummary({
status: "succeeded",
patchText,
parsed,
plan,
metrics,
startedAtMs,
options: options.timing,
})));
return 0;
} catch (error) {
const failureSummary = applyPatchV2TimingSummary({
status: "failed",
patchText,
parsed,
plan,
metrics,
startedAtMs,
options: options.timing,
});
if (options.stderr === undefined) throw error;
options.stderr.write(formatApplyPatchFailure(error));
options.stderr.write(formatApplyPatchV2TimingSummary(failureSummary));
return 1;
}
}
function createApplyPatchV2RemoteMetrics(): ApplyPatchV2RemoteMetrics {
return { remoteOperationCount: 0, remoteOperationCounts: {}, remoteElapsedMs: 0, remoteFailureCount: 0 };
}
function instrumentApplyPatchV2Executor(executor: ApplyPatchV2Executor, metrics: ApplyPatchV2RemoteMetrics): ApplyPatchV2Executor {
return {
...executor,
run: executor.run === undefined ? undefined : async (command, input) => {
const operation = applyPatchV2OperationName(command);
const started = Date.now();
metrics.remoteOperationCount += 1;
metrics.remoteOperationCounts[operation] = (metrics.remoteOperationCounts[operation] ?? 0) + 1;
try {
const result = await executor.run!(command, input);
metrics.remoteElapsedMs += Math.max(0, Date.now() - started);
if (result.exitCode !== 0) metrics.remoteFailureCount += 1;
return result;
} catch (error) {
metrics.remoteElapsedMs += Math.max(0, Date.now() - started);
metrics.remoteFailureCount += 1;
throw error;
}
},
fs: executor.fs === undefined ? undefined : instrumentApplyPatchV2FileSystem(executor.fs, metrics),
};
}
function instrumentApplyPatchV2FileSystem(fs: ApplyPatchV2FileSystem, metrics: ApplyPatchV2RemoteMetrics): ApplyPatchV2FileSystem {
return {
stat: (path) => recordApplyPatchV2FsOperation(metrics, "fs.stat", () => fs.stat(path)),
readBlock: (path, blockIndex, blockBytes) => recordApplyPatchV2FsOperation(metrics, "fs.readBlock", () => fs.readBlock(path, blockIndex, blockBytes)),
writeFile: (path, content) => recordApplyPatchV2FsOperation(metrics, "fs.writeFile", () => fs.writeFile(path, content)),
deleteFile: (path) => recordApplyPatchV2FsOperation(metrics, "fs.deleteFile", () => fs.deleteFile(path)),
};
}
async function recordApplyPatchV2FsOperation<T>(metrics: ApplyPatchV2RemoteMetrics, operation: string, run: () => Promise<T>): Promise<T> {
const started = Date.now();
metrics.remoteOperationCount += 1;
metrics.remoteOperationCounts[operation] = (metrics.remoteOperationCounts[operation] ?? 0) + 1;
try {
const result = await run();
metrics.remoteElapsedMs += Math.max(0, Date.now() - started);
return result;
} catch (error) {
metrics.remoteElapsedMs += Math.max(0, Date.now() - started);
metrics.remoteFailureCount += 1;
throw error;
}
}
function applyPatchV2OperationName(command: string[]): string {
const index = command.indexOf("unidesk-apply-patch-v2");
return index >= 0 ? command[index + 1] ?? "unknown" : command[0] ?? "unknown";
}
export function applyPatchV2TimingSummary(options: {
status: ApplyPatchV2TimingSummary["status"];
patchText: string;
parsed: PatchParseResult | null;
plan: ApplyPatchV2Plan | null;
metrics: ApplyPatchV2RemoteMetrics;
startedAtMs: number;
finishedAtMs?: number;
options?: ApplyPatchV2TimingOptions;
}): ApplyPatchV2TimingSummary {
const finishedAtMs = options.finishedAtMs ?? Date.now();
const durationMs = Math.max(0, Math.round(finishedAtMs - options.startedAtMs));
const thresholdMs = options.options?.thresholdMs ?? defaultApplyPatchV2SlowWarningMs;
const slow = durationMs > thresholdMs;
const patchBytes = Buffer.byteLength(options.patchText, "utf8");
const fileCount = countApplyPatchV2Files(options.parsed?.hunks ?? []);
const hunkCount = options.parsed?.hunks.length ?? 0;
const changedCount = options.plan?.changed.length ?? applyPatchV2ChangedCountFromOutcomes(options.plan?.outcomes ?? []);
const durationSeconds = Number((durationMs / 1000).toFixed(3));
const thresholdSeconds = Number((thresholdMs / 1000).toFixed(3));
return {
code: "apply-patch-v2-timing",
level: slow ? "warning" : "info",
status: options.status,
...(options.options?.providerId === undefined ? {} : { providerId: options.options.providerId }),
...(options.options?.route === undefined ? {} : { route: options.options.route }),
...(options.options?.transport === undefined ? {} : { transport: options.options.transport }),
durationMs,
thresholdMs,
slow,
patchBytes,
fileCount,
hunkCount,
changedCount,
remoteOperationCount: options.metrics.remoteOperationCount,
remoteOperationCounts: options.metrics.remoteOperationCounts,
remoteElapsedMs: Math.max(0, Math.round(options.metrics.remoteElapsedMs)),
remoteFailureCount: options.metrics.remoteFailureCount,
message: slow
? `apply-patch completed in ${durationSeconds}s, above the ${thresholdSeconds}s warning threshold; inspect remoteOperationCounts and remoteElapsedMs before repeating high-frequency patch work.`
: `apply-patch completed in ${durationSeconds}s.`,
note: "Timing summary is written to stderr so stdout remains Codex apply_patch-compatible.",
};
}
export function formatApplyPatchV2TimingSummary(summary: ApplyPatchV2TimingSummary): string {
return `UNIDESK_APPLY_PATCH_TIMING ${JSON.stringify(summary)}\n`;
}
function countApplyPatchV2Files(hunks: PatchHunk[]): number {
const files = new Set<string>();
for (const hunk of hunks) {
files.add(hunk.path);
if (hunk.kind === "update" && hunk.movePath !== null) files.add(hunk.movePath);
}
return files.size;
}
function applyPatchV2ChangedCountFromOutcomes(outcomes: ApplyPatchV2Outcome[]): number {
return outcomes.filter((outcome) => outcome.status === "applied").length;
}
async function applyPatchV2Hunks(executor: ApplyPatchV2Executor, hunks: PatchHunk[]): Promise<ApplyPatchV2Plan> {
if (hunks.length === 0) throw new ApplyPatchV2Error("No files were modified.");
+1 -1
View File
@@ -202,7 +202,7 @@ export function sshHelp(): unknown {
"When a one-line shell command is easier to type through the script path, `script -- '<command && command>'` runs that single string through the remote shell without waiting for stdin. When `script --` is followed by multiple tokens, it stays a direct argv form for commands such as `trans D601:/work script -- sed -n '1,20p' file`.",
"script and shell helper modes inject a tiny POSIX-compatible printf wrapper before user shell text, so portable printf headings such as `printf \"--- section ---\\n\"` work consistently under dash/sh and bash. Direct argv commands are unchanged.",
"For arbitrary stdin streams into a workload command, use a workload route plus `exec --stdin -- <command> ...`; this keeps the route as location-only and avoids heredoc/base64/tar shell wrapping.",
"`apply-patch` is the default remote text patch entry and uses the v2 local line-based patch engine with remote read/write operations, including Windows routes such as `D601:win/c/test`, so long Unicode/Chinese lines and pure insertion hunks avoid the legacy remote shell hunk parser. Its stdout/stderr follows Codex apply_patch text output rather than UniDesk JSON output; on multi-file failure, stderr lists applied hunks before the first failed hunk and the failed hunk, then stops like Codex apply_patch.",
"`apply-patch` is the default remote text patch entry and uses the v2 local line-based patch engine with remote read/write operations, including Windows routes such as `D601:win/c/test`, so long Unicode/Chinese lines and pure insertion hunks avoid the legacy remote shell hunk parser. Its stdout follows Codex apply_patch text output rather than UniDesk JSON output; stderr keeps Codex-style failure text and appends one `UNIDESK_APPLY_PATCH_TIMING` JSON summary with durationMs, patchBytes, fileCount, hunkCount, changedCount, remoteOperationCount, remoteOperationCounts and remoteElapsedMs so slow patch runs can be attributed without changing success stdout.",
"`upload` and `download` are the default whole-file transfer entries for non-text and generated files. They write through remote temp files, verify byte count and SHA-256 on both sides, and return `verification.automatic=true`, `verification.verified=true`, and `verification.match.{bytes,sha256}=true`; this JSON is the transfer integrity proof, so callers do not need a separate manual `sha256sum` check. The client falls back from a single stdin payload to bounded chunks before treating provider-gateway limits as a server-side problem.",
"`apply-patch-v1` is the only legacy fallback entry: it rejects low-context update hunks by default, reports the matched file:line for each hunk on stderr, and only accepts --allow-loose when the caller has manually reviewed an intentionally ambiguous insertion.",
"script defaults to target /bin/sh and inherits provider proxy variables such as HTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXY; it is for host/k3s POSIX shell only. Use --shell bash only for bash syntax such as pipefail, arrays, or [[ ... ]], not as a proxy workaround.",
+12 -1
View File
@@ -1294,7 +1294,18 @@ async function runRemoteSshOverFrontend(session: FrontendSession, target: string
const executor: ApplyPatchV2Executor = {
run: (command, input) => runRemoteSshWebSocketCapture(session, invocation, command, input),
};
return await runApplyPatchV2({ executor, stdin: process.stdin, stdout: process.stdout, stderr: process.stderr, argv: normalizedArgs.slice(1) });
return await runApplyPatchV2({
executor,
stdin: process.stdin,
stdout: process.stdout,
stderr: process.stderr,
argv: normalizedArgs.slice(1),
timing: {
providerId: invocation.providerId,
route: invocation.route.raw,
transport: "frontend-websocket",
},
});
}
return runRemoteSshWebSocket(session, invocation);
}
+5
View File
@@ -2655,6 +2655,11 @@ export async function runSsh(config: UniDeskConfig, providerId: string, args: st
stdout: process.stdout,
stderr: process.stderr,
argv: normalizedArgs.slice(1),
timing: {
providerId: invocation.providerId,
route: invocation.route.raw,
transport: "backend-core-broker",
},
});
}
const startedAtMs = Date.now();
+15 -1
View File
@@ -5,7 +5,7 @@ import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, wr
import os from "node:os";
import path from "node:path";
import { sshHelp } from "./src/help";
import { runApplyPatchV2 } from "./src/apply-patch-v2";
import { runApplyPatchV2, type ApplyPatchV2TimingSummary } from "./src/apply-patch-v2";
import { providerTriageRecommendedCrossChecks } from "./src/provider-triage";
import { extractRemoteCliOptions, remoteSshFrontendPlanForTest } from "./src/remote";
import { runSshFileTransferOperation, type SshFileTransferCommandBuilders, type SshRemoteCommandExecutor } from "./src/ssh-file-transfer";
@@ -227,6 +227,12 @@ async function applyPatchV2FixtureAttempt(patch: string, files: Record<string, s
return { stdout, stderr, exitCode, files: Object.fromEntries(state), commands, error };
}
function applyPatchTimingFromStderr(stderr: string): ApplyPatchV2TimingSummary {
const line = stderr.split(/\r?\n/u).find((item) => item.startsWith("UNIDESK_APPLY_PATCH_TIMING "));
assertCondition(line !== undefined, "apply-patch stderr must include UNIDESK_APPLY_PATCH_TIMING", stderr);
return JSON.parse(line!.slice("UNIDESK_APPLY_PATCH_TIMING ".length)) as ApplyPatchV2TimingSummary;
}
async function applyPatchV2ActualShellFixtureAttempt(
patch: string,
files: Record<string, string>,
@@ -787,6 +793,13 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
});
assertCondition(unifiedHeaderLineRangeV2.stderr.includes("accepted unified-diff hunk header in repeated-large.txt"), "v2 unified header compatibility should emit a canonical syntax hint", unifiedHeaderLineRangeV2.stderr);
const unifiedTiming = applyPatchTimingFromStderr(unifiedHeaderLineRangeV2.stderr);
assertCondition(unifiedTiming.code === "apply-patch-v2-timing" && unifiedTiming.status === "succeeded", "v2 apply-patch timing summary should identify successful runs", unifiedTiming);
assertCondition(unifiedTiming.patchBytes > 0 && unifiedTiming.fileCount === 1 && unifiedTiming.hunkCount === 1 && unifiedTiming.changedCount === 1, "v2 timing summary should expose patch/file/hunk/change counts", unifiedTiming);
assertCondition(unifiedTiming.remoteOperationCount === unifiedHeaderLineRangeV2.commands.length, "v2 timing summary should count remote operations", { unifiedTiming, commands: unifiedHeaderLineRangeV2.commands });
assertCondition((unifiedTiming.remoteOperationCounts.stat ?? 0) >= 1 && (unifiedTiming.remoteOperationCounts["read-b64-block"] ?? 0) >= 1 && Object.keys(unifiedTiming.remoteOperationCounts).some((key) => key.startsWith("write-b64")), "v2 timing summary should classify stat/read/write operations", unifiedTiming);
assertCondition(unifiedHeaderLineRangeV2.stdout.startsWith("Success. Updated the following files:"), "v2 timing summary must not change Codex-compatible success stdout", unifiedHeaderLineRangeV2.stdout);
const unprefixedUpdateContextV2 = await applyPatchV2FixtureAttempt([
"*** Begin Patch",
"*** Update File: internal/cloud/access-control.ts",
@@ -1553,6 +1566,7 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
assertCondition(helpText.includes("UNIDESK_SSH_HINT"), "ssh help must document structured failure hint", helpText);
assertCondition(helpText.includes("UNIDESK_SSH_RUNTIME_TIMEOUT") && helpText.includes("UNIDESK_TRAN_TIMEOUT_HINT") && helpText.includes("60s") && helpText.includes("submit-and-poll"), "ssh help must document top-level runtime timeout and short polling discipline", helpText);
assertCondition(helpText.includes("UNIDESK_SSH_TIMING") && helpText.includes("10s") && helpText.includes("slow successful calls are a distributed performance monitoring signal") && helpText.includes("Routine short calls do not emit timing noise"), "ssh help must document slow-only runtime timing hints", helpText);
assertCondition(helpText.includes("UNIDESK_APPLY_PATCH_TIMING") && helpText.includes("remoteOperationCounts") && helpText.includes("durationMs"), "ssh help must document apply-patch aggregate timing summary", helpText);
assertCondition(helpText.includes("must not add provider/plane directory locks") && helpText.includes("k8s/Tekton/Argo/Lease"), "ssh help must document tran's no-local-lock boundary", helpText);
const crossChecks = providerTriageRecommendedCrossChecks("D601");