diff --git a/scripts/src/apply-patch-v2.ts b/scripts/src/apply-patch-v2.ts index 867911b8..43b842f5 100644 --- a/scripts/src/apply-patch-v2.ts +++ b/scripts/src/apply-patch-v2.ts @@ -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: ` 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 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): 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 | null { + return typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record : 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; diff --git a/scripts/src/remote.ts b/scripts/src/remote.ts index e4fc5328..503b4e9a 100644 --- a/scripts/src/remote.ts +++ b/scripts/src/remote.ts @@ -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 { 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); } diff --git a/scripts/src/ssh.ts b/scripts/src/ssh.ts index 9d11f84f..734a5b85 100644 --- a/scripts/src/ssh.ts +++ b/scripts/src/ssh.ts @@ -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 ${normalizedArgs.join(" ")}\`. Keep -- only inside operations such as \`script -- \` or \`exec -- \`.\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 { - 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(); diff --git a/scripts/ssh-argv-guidance-contract-test.ts b/scripts/ssh-argv-guidance-contract-test.ts index bbcc7308..91b04245 100644 --- a/scripts/ssh-argv-guidance-contract-test.ts +++ b/scripts/ssh-argv-guidance-contract-test.ts @@ -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 { 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 { 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 { }); 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 { 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 { 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);