fix: improve minimax apply-patch compatibility

This commit is contained in:
Codex
2026-06-03 13:07:24 +00:00
parent d012fe9a5e
commit 67a0446e51
4 changed files with 177 additions and 27 deletions
+71 -11
View File
@@ -11,6 +11,7 @@ export interface UpdateChunk {
sourceStartLine: number | null;
oldLines: string[];
newLines: string[];
contextLinePairs: Array<{ oldIndex: number; newIndex: number }>;
isEndOfFile: boolean;
}
@@ -120,9 +121,9 @@ export function applyPatchV2HelpPayload() {
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 +.",
"Update File uses @@ or @@ context markers, followed by context lines starting with space and changed lines starting with - or +; unified-diff line-range headers are accepted with hints for MiniMax compatibility.",
"Update File uses @@ or @@ context markers, followed by context lines starting with one extra space prefix and changed lines starting with - or +; for a column-0 source line `const x`, write ` const x`, and for a two-space-indented source line write three spaces total. Unified-diff line-range headers are accepted with hints for MiniMax compatibility.",
"Prefer `trans <route> apply-patch < /tmp/patch.diff` for long patches, Windows paths, or quoting-sensitive content.",
"MiniMax compatibility: stray @@ or unprefixed content inside Add File, and extra hunk/body lines after Delete File, are accepted with stderr hints."
"MiniMax compatibility: stray @@ or unprefixed content inside Add File, unprefixed Update File context lines, and extra hunk/body lines after Delete File, are accepted with stderr hints."
],
examples: {
addFile: [
@@ -394,6 +395,7 @@ function formatApplyPatchFailure(error: unknown): string {
const details = error instanceof ApplyPatchV2Error ? error.details : {};
const outcomes = Array.isArray(details.outcomes) ? details.outcomes as ApplyPatchV2Outcome[] : [];
const lines = [`${message.trimEnd()}`];
appendExpectedLinesFailureHint(lines, details);
if (outcomes.length > 1 || outcomes.some((item) => item.status === "applied")) {
lines.push("Patch status:");
appendOutcomeSection(lines, "Applied before failure:", outcomes.filter((item) => item.status === "applied"));
@@ -402,6 +404,37 @@ function formatApplyPatchFailure(error: unknown): string {
return `${lines.join("\n")}\n`;
}
function appendExpectedLinesFailureHint(lines: string[], details: Record<string, unknown>): void {
const cause = recordValue(details.cause) ?? details;
const expected = typeof cause.expected === "string" ? cause.expected : "";
if (expected.length === 0) return;
const path = typeof cause.path === "string" ? cause.path : "target file";
const chunk = typeof cause.chunk === "number" ? ` hunk ${cause.chunk}` : "";
lines.push(`Expected lines for ${path}${chunk}:`);
appendQuotedBlock(lines, expected, 20, 1600);
lines.push("Hint: re-read the target file around this hunk. In Update File hunks, every context line needs a leading space prefix; for a column-0 source line like `]);`, write ` ]);`.");
}
function recordValue(value: unknown): Record<string, unknown> | null {
return typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record<string, unknown> : null;
}
function appendQuotedBlock(lines: string[], text: string, maxLines: number, maxChars: number): void {
const sourceLines = text.split("\n");
let usedChars = 0;
let omitted = 0;
for (const [index, line] of sourceLines.entries()) {
const quoted = JSON.stringify(line);
if (index >= maxLines || usedChars + quoted.length > maxChars) {
omitted = sourceLines.length - index;
break;
}
lines.push(` ${quoted}`);
usedChars += quoted.length;
}
if (omitted > 0) lines.push(` ... ${omitted} more expected line(s) omitted`);
}
function appendOutcomeSection(lines: string[], title: string, outcomes: ApplyPatchV2Outcome[]): void {
if (outcomes.length === 0) return;
lines.push(title);
@@ -806,6 +839,14 @@ function isFileHeader(line: string): boolean {
return trimmed.startsWith(addFileMarker) || trimmed.startsWith(deleteFileMarker) || trimmed.startsWith(updateFileMarker) || trimmed === endMarker;
}
function isUpdateChunkHeader(line: string): boolean {
return line === emptyChangeContextMarker || line.startsWith(changeContextMarker) || parseUnifiedDiffHunkHeader(line) !== null;
}
function pushUniqueHint(hints: string[], prefix: string, hint: string): void {
if (!hints.some((existing) => existing.startsWith(prefix))) hints.push(hint);
}
function parseUpdateChunk(lines: string[], startIndex: number, allowMissingContext: boolean, filePath: string, hints: string[]): { chunk: UpdateChunk; nextIndex: number } {
let index = startIndex;
let changeContext: string | null = null;
@@ -829,11 +870,18 @@ function parseUpdateChunk(lines: string[], startIndex: number, allowMissingConte
const oldLines: string[] = [];
const newLines: string[] = [];
const contextLinePairs: Array<{ oldIndex: number; newIndex: number }> = [];
let parsed = 0;
let isEndOfFile = false;
function pushContextLine(value: string): void {
contextLinePairs.push({ oldIndex: oldLines.length, newIndex: newLines.length });
oldLines.push(value);
newLines.push(value);
}
while (index < lines.length - 1) {
const line = lines[index] ?? "";
if (isFileHeader(line)) break;
if (parsed > 0 && isUpdateChunkHeader(line)) break;
if (line === eofMarker) {
if (parsed === 0) throw new ApplyPatchV2Error("update chunk does not contain any lines", { line: index + 1 });
isEndOfFile = true;
@@ -842,25 +890,26 @@ function parseUpdateChunk(lines: string[], startIndex: number, allowMissingConte
}
const marker = line[0] ?? "";
if (marker === " ") {
oldLines.push(line.slice(1));
newLines.push(line.slice(1));
pushContextLine(line.slice(1));
} else if (marker === "+") {
newLines.push(line.slice(1));
} else if (marker === "-") {
oldLines.push(line.slice(1));
} else if (line.length === 0) {
oldLines.push("");
newLines.push("");
} else if (parsed > 0) {
break;
pushContextLine("");
} else {
throw new ApplyPatchV2Error("unexpected line in update chunk", { line: index + 1, text: line });
pushUniqueHint(
hints,
`apply-patch hint: accepted unprefixed Update File context line in ${filePath}`,
`apply-patch hint: accepted unprefixed Update File context line in ${filePath} on line ${index + 1}; prefix context lines with one extra space in addition to source indentation.`,
);
pushContextLine(line);
}
parsed += 1;
index += 1;
}
if (parsed === 0) throw new ApplyPatchV2Error("update chunk does not contain any lines", { line: startIndex + 1 });
return { chunk: { changeContext, sourceStartLine, oldLines, newLines, isEndOfFile }, nextIndex: index };
return { chunk: { changeContext, sourceStartLine, oldLines, newLines, contextLinePairs, isEndOfFile }, nextIndex: index };
}
function parseUnifiedDiffHunkHeader(line: string): { oldStart: number } | null {
@@ -915,13 +964,24 @@ function computeReplacements(filePath: string, originalLines: string[], chunks:
expected: chunk.oldLines.join("\n"),
});
}
replacements.push([found, pattern.length, newLines]);
replacements.push([found, pattern.length, preserveMatchedContextLines(originalLines, found, newLines, chunk.contextLinePairs, pattern.length)]);
lineIndex = found + pattern.length;
}
assertNonOverlappingReplacements(filePath, replacements, originalLines.length);
return replacements;
}
function preserveMatchedContextLines(originalLines: string[], found: number, newLines: string[], contextLinePairs: UpdateChunk["contextLinePairs"], matchedOldLength: number): string[] {
if (contextLinePairs.length === 0) return newLines;
const result = [...newLines];
for (const pair of contextLinePairs) {
if (pair.oldIndex >= matchedOldLength || pair.newIndex >= result.length) continue;
const originalLine = originalLines[found + pair.oldIndex];
if (originalLine !== undefined) result[pair.newIndex] = originalLine;
}
return result;
}
function seekSequenceWithFallback(lines: string[], pattern: string[], preferredStart: number, fallbackStart: number, eof: boolean): number | null {
const preferred = seekSequence(lines, pattern, preferredStart, eof);
if (preferred !== null || preferredStart === fallbackStart) return preferred;
+9 -5
View File
@@ -11,9 +11,11 @@ import {
formatSshFailureHint,
formatSshRuntimeTimeoutHint,
formatSshRuntimeTimingHint,
normalizeSshOperationArgs,
parseSshInvocation,
remoteCommandForRoute,
sshFailureHint,
sshRouteSeparatorCompatibilityHint,
sshRoutePayloadCwd,
sshRuntimeTimeoutHint,
sshRuntimeTimeoutMs,
@@ -1276,21 +1278,23 @@ export function remoteSshFrontendPlanForTest(target: string, args: string[]): Re
async function runRemoteSshOverFrontend(session: FrontendSession, target: string | undefined, args: string[]): Promise<number> {
if (!target) throw new Error("remote ssh requires a route, for example: bun scripts/cli.ts --main-server-ip 74.48.78.17 ssh D601 hostname");
const invocation = parseSshInvocation(target, args);
if (isSshFileTransferOperation(args)) {
const normalizedArgs = normalizeSshOperationArgs(args);
process.stderr.write(sshRouteSeparatorCompatibilityHint(args, normalizedArgs));
const invocation = parseSshInvocation(target, normalizedArgs);
if (isSshFileTransferOperation(normalizedArgs)) {
const executor: SshRemoteCommandExecutor = {
runRemoteCommand: (remoteCommand, input) => runRemoteSshWebSocketCaptureRemoteCommand(session, invocation, remoteCommand, input),
};
return await runSshFileTransferOperation(invocation, args, executor, {
return await runSshFileTransferOperation(invocation, normalizedArgs, executor, {
buildRouteCommand: remoteCommandForRoute,
buildWindowsPowerShellCommand: buildWindowsPowerShellInvocation,
});
}
if ((args[0] ?? "") === "apply-patch") {
if ((normalizedArgs[0] ?? "") === "apply-patch") {
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: args.slice(1) });
return await runApplyPatchV2({ executor, stdin: process.stdin, stdout: process.stdout, stderr: process.stderr, argv: normalizedArgs.slice(1) });
}
return runRemoteSshWebSocket(session, invocation);
}
+24 -10
View File
@@ -911,6 +911,17 @@ export function parseSshArgs(args: string[]): ParsedSshArgs {
};
}
export function normalizeSshOperationArgs(args: string[]): string[] {
return args[0] === "--" ? args.slice(1) : args;
}
export function sshRouteSeparatorCompatibilityHint(rawArgs: string[], normalizedArgs: string[]): string {
if (rawArgs === normalizedArgs || rawArgs[0] !== "--") return "";
const operation = normalizedArgs[0] ?? "";
const operationText = operation.length === 0 ? "operation" : `operation \`${operation}\``;
return `UNIDESK_SSH_HINT route-level -- is ignored before ${operationText}; canonical form is \`trans <route> ${normalizedArgs.join(" ")}\`. Keep -- only inside operations such as \`script -- <command>\` or \`exec -- <command>\`.\n`;
}
function validateDirectArgvCommand(commandName: string, toolArgs: string[]): void {
if (toolArgs.length !== 1) return;
const command = toolArgs[0] ?? "";
@@ -927,16 +938,17 @@ function looksLikeShellCommandString(value: string): boolean {
export function parseSshInvocation(target: string, args: string[]): ParsedSshInvocation {
const route = parseSshRoute(target);
const operationArgs = normalizeSshOperationArgs(args);
if (route.plane === "k3s") {
return { providerId: route.providerId, route, parsed: parseK3sRouteArgs(route, args) };
return { providerId: route.providerId, route, parsed: parseK3sRouteArgs(route, operationArgs) };
}
if (route.plane === "win") {
return { providerId: route.providerId, route, parsed: parseWinRouteArgs(route, args) };
return { providerId: route.providerId, route, parsed: parseWinRouteArgs(route, operationArgs) };
}
if ((args[0] ?? "") === "k3s") {
throw new Error(`ssh k3s shorthand is unsupported; use route syntax instead: trans ${route.providerId}:k3s ${args.slice(1).join(" ")}`.trim());
if ((operationArgs[0] ?? "") === "k3s") {
throw new Error(`ssh k3s shorthand is unsupported; use route syntax instead: trans ${route.providerId}:k3s ${operationArgs.slice(1).join(" ")}`.trim());
}
return { providerId: route.providerId, route, parsed: parseSshArgs(args) };
return { providerId: route.providerId, route, parsed: parseSshArgs(operationArgs) };
}
export function parseSshRoute(target: string): ParsedSshRoute {
@@ -2512,14 +2524,16 @@ function writeChunkedStdin(stdin: NodeJS.WritableStream, input: string): void {
}
export async function runSsh(config: UniDeskConfig, providerId: string, args: string[]): Promise<number> {
const invocation = parseSshInvocation(providerId, args);
const normalizedArgs = normalizeSshOperationArgs(args);
process.stderr.write(sshRouteSeparatorCompatibilityHint(args, normalizedArgs));
const invocation = parseSshInvocation(providerId, normalizedArgs);
const parsed = invocation.parsed;
const operationName = args[0] ?? "";
if (isSshFileTransferOperation(args)) {
const operationName = normalizedArgs[0] ?? "";
if (isSshFileTransferOperation(normalizedArgs)) {
const executor: SshRemoteCommandExecutor = {
runRemoteCommand: (remoteCommand, input) => runSshCaptureRemoteCommand(config, invocation, remoteCommand, input),
};
return await runSshFileTransferOperation(invocation, args, executor, {
return await runSshFileTransferOperation(invocation, normalizedArgs, executor, {
buildRouteCommand: remoteCommandForRoute,
buildWindowsPowerShellCommand: buildWindowsPowerShellInvocation,
});
@@ -2533,7 +2547,7 @@ export async function runSsh(config: UniDeskConfig, providerId: string, args: st
stdin: process.stdin,
stdout: process.stdout,
stderr: process.stderr,
argv: args.slice(1),
argv: normalizedArgs.slice(1),
});
}
const startedAtMs = Date.now();
+73 -1
View File
@@ -13,11 +13,13 @@ import {
formatSshFailureHint,
formatSshRuntimeTimeoutHint,
formatSshRuntimeTimingHint,
normalizeSshOperationArgs,
parseSshArgs,
parseSshInvocation,
remoteApplyPatchSource,
shellArgv,
sshFailureHint,
sshRouteSeparatorCompatibilityHint,
sshShellCompatibilityPrelude,
sshUserToolPathPrelude,
sshRuntimeTimeoutHint,
@@ -402,6 +404,20 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
assertCondition(hostWorkspaceLongForm.route.workspace === "/home/ubuntu/workspace/hwlab-dev", "host: workspace route must parse as the same location model", hostWorkspaceLongForm);
assertCondition(hostWorkspaceLongForm.parsed.remoteCommand === "'git' 'status' '--short'", "host workspace argv operation must stay argv-quoted", hostWorkspaceLongForm);
const routeSeparatorArgs = normalizeSshOperationArgs(["--", "apply-patch"]);
assertCondition(JSON.stringify(routeSeparatorArgs) === JSON.stringify(["apply-patch"]), "route-level -- before an operation should be ignored for MiniMax compatibility", routeSeparatorArgs);
const routeSeparatorHint = sshRouteSeparatorCompatibilityHint(["--", "apply-patch"], routeSeparatorArgs);
assertCondition(routeSeparatorHint.includes("route-level -- is ignored") && routeSeparatorHint.includes("canonical form"), "route-level -- compatibility should emit a canonical hint", routeSeparatorHint);
const hostRouteSeparatorApplyPatch = parseSshInvocation("D601:/tmp", ["--", "apply-patch"]);
assertCondition(hostRouteSeparatorApplyPatch.parsed.requiresStdin === true && hostRouteSeparatorApplyPatch.parsed.remoteCommand === null, "host route-level -- apply-patch should still use local v2 apply-patch", hostRouteSeparatorApplyPatch);
const hostRouteSeparatorRg = parseSshInvocation("D601:/tmp", ["--", "rg", "-ne", "DEVICE_JOB_READ_ONLY_SUB_ACTIONS", "-ne", "ACCESS_SCHEMA_STATEMENTS", "internal/cloud/access-control.ts"]);
assertCondition(
hostRouteSeparatorRg.parsed.invocationKind === "argv"
&& hostRouteSeparatorRg.parsed.remoteCommand === "'rg' '-ne' 'DEVICE_JOB_READ_ONLY_SUB_ACTIONS' '-ne' 'ACCESS_SCHEMA_STATEMENTS' 'internal/cloud/access-control.ts'",
"host route-level -- rg should preserve argv quoting instead of turning regex | into a remote shell pipe",
hostRouteSeparatorRg,
);
const winCmd = parseSshInvocation("D601:win", ["cmd", "ver"]);
assertCondition(winCmd.route.plane === "win" && winCmd.route.workspace === null, "win route must parse as the Windows cmd plane", winCmd);
const winCmdScript = decodeWinEncodedCommand(winCmd.parsed.remoteCommand);
@@ -517,6 +533,8 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
assertCondition(directScriptCommand.invocationKind === "argv", "script -- command form must run as direct argv without stdin", directScriptCommand);
assertCondition(directScriptCommand.remoteCommand === "'sed' '-n' '1,2p' 'file.txt'", "script -- command form must preserve dash-prefixed command args", directScriptCommand);
assertCondition(directScriptCommand.requiresStdin === false, "script -- command form must not wait for stdin", directScriptCommand);
const routeSeparatorBeforeScript = parseSshInvocation("D601:/tmp", ["--", "script", "--", "sed", "-n", "1,2p", "file.txt"]);
assertCondition(routeSeparatorBeforeScript.parsed.remoteCommand === "'sed' '-n' '1,2p' 'file.txt'", "route-level -- compatibility must not remove the command-local script -- separator", routeSeparatorBeforeScript);
const directScriptOneLiner = parseSshArgs(["script", "--", "cd /root/hwlab && git status --short --branch"]);
assertCondition(directScriptOneLiner.invocationKind === "helper", "script -- single-string command should run through a remote shell", directScriptOneLiner);
@@ -757,6 +775,56 @@ 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 unprefixedUpdateContextV2 = await applyPatchV2FixtureAttempt([
"*** Begin Patch",
"*** Update File: internal/cloud/access-control.ts",
"@@",
" \"io.uart.jsonrpc\"",
"]);",
"+const DEVICE_JOB_READ_ONLY_SUB_ACTIONS = new Set([",
"+ \"status\",",
"+ \"output\"",
"+]);",
"const ACCESS_SCHEMA_STATEMENTS = Object.freeze([",
"*** End Patch",
"",
].join("\n"), {
"internal/cloud/access-control.ts": [
" \"io.uart.write\",",
" \"io.uart.jsonrpc\"",
"]);",
"const ACCESS_SCHEMA_STATEMENTS = Object.freeze([",
"]);",
"",
].join("\n"),
}, { stderrOutput: true });
assertCondition(unprefixedUpdateContextV2.exitCode === 0 && unprefixedUpdateContextV2.error === null, "v2 should accept MiniMax-style unprefixed Update File context lines", unprefixedUpdateContextV2);
assertCondition(
unprefixedUpdateContextV2.files["internal/cloud/access-control.ts"]?.includes("const DEVICE_JOB_READ_ONLY_SUB_ACTIONS = new Set(["),
"v2 should apply the update when MiniMax omits context prefixes for column-0 lines",
unprefixedUpdateContextV2,
);
assertCondition(
unprefixedUpdateContextV2.stderr.includes("accepted unprefixed Update File context line in internal/cloud/access-control.ts")
&& unprefixedUpdateContextV2.stderr.includes("one extra space in addition to source indentation"),
"v2 MiniMax-style Update File compatibility should emit canonical syntax hints",
unprefixedUpdateContextV2.stderr,
);
const unprefixedFirstUpdateContextV2 = await applyPatchV2FixtureAttempt([
"*** Begin Patch",
"*** Update File: zero-column.ts",
"const MUTATING_INTENTS = new Set([",
"+ \"workspace.apply-patch\",",
"]);",
"*** End Patch",
"",
].join("\n"), {
"zero-column.ts": "const MUTATING_INTENTS = new Set([\n]);\n",
}, { stderrOutput: true });
assertCondition(unprefixedFirstUpdateContextV2.exitCode === 0 && unprefixedFirstUpdateContextV2.error === null, "v2 should accept an omitted first @@ plus unprefixed column-0 context", unprefixedFirstUpdateContextV2);
assertCondition(unprefixedFirstUpdateContextV2.files["zero-column.ts"] === "const MUTATING_INTENTS = new Set([\n \"workspace.apply-patch\",\n]);\n", "v2 omitted-@@ compatibility should preserve column-0 context correctly", unprefixedFirstUpdateContextV2);
const manyLines = Array.from({ length: 6200 }, (_, index) => {
if (index === 4) return "HEAD old";
if (index === 3099) return "MIDDLE old";
@@ -959,11 +1027,13 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
assertCondition(
failedCompoundVisibleV2.stderr.includes("failed to find expected lines")
&& failedCompoundVisibleV2.stderr.includes("Applied before failure:")
&& failedCompoundVisibleV2.stderr.includes("Expected lines for second.txt hunk 1:")
&& failedCompoundVisibleV2.stderr.includes("\"missing second\"")
&& failedCompoundVisibleV2.stderr.includes("M first.txt")
&& failedCompoundVisibleV2.stderr.includes("Failed:")
&& failedCompoundVisibleV2.stderr.includes("hunk 2 update second.txt")
&& !failedCompoundVisibleV2.stderr.includes("third.txt"),
"v2 failed CLI path should print Codex-style stderr plus applied/failed summary and stop before later hunks",
"v2 failed CLI path should print Codex-style stderr plus expected lines, applied/failed summary, and stop before later hunks",
failedCompoundVisibleV2.stderr,
);
@@ -1353,6 +1423,8 @@ export async function runSshArgvGuidanceContract(): Promise<JsonRecord> {
const frontendRemoteHostPatchPlan = remoteSshFrontendPlanForTest("D601", ["apply-patch"]);
assertCondition(frontendRemoteHostPatchPlan.requiresStdin === true && frontendRemoteHostPatchPlan.remoteCommand === null && !String(frontendRemoteHostPatchPlan.wrappedRemoteCommand ?? "").includes("UNIDESK_SSH_TOOL_DIR"), "frontend apply-patch plan must stay a local v2 engine operation and not bootstrap legacy helpers", frontendRemoteHostPatchPlan);
const frontendRemoteHostPatchSeparatorPlan = remoteSshFrontendPlanForTest("D601", ["--", "apply-patch"]);
assertCondition(frontendRemoteHostPatchSeparatorPlan.requiresStdin === true && frontendRemoteHostPatchSeparatorPlan.remoteCommand === null, "frontend apply-patch plan should also accept route-level -- before apply-patch", frontendRemoteHostPatchSeparatorPlan);
const frontendRemoteV1Plan = remoteSshFrontendPlanForTest("D601:/tmp", ["apply-patch-v1"]);
assertCondition(String(frontendRemoteV1Plan.wrappedRemoteCommand ?? "").includes("UNIDESK_SSH_TOOL_DIR=/tmp/unidesk-ssh-tools"), "frontend apply-patch-v1 must bootstrap the remote apply_patch helper", frontendRemoteV1Plan);