From a26462647258cc5f96a8711266661b2c19c9ee88 Mon Sep 17 00:00:00 2001 From: Codex Date: Wed, 3 Jun 2026 13:27:59 +0000 Subject: [PATCH] fix: guide minimax away from sed patch fallbacks --- scripts/src/apply-patch-v2.ts | 119 +++++++++++++++++++-- scripts/ssh-argv-guidance-contract-test.ts | 64 +++++++++++ 2 files changed, 177 insertions(+), 6 deletions(-) diff --git a/scripts/src/apply-patch-v2.ts b/scripts/src/apply-patch-v2.ts index 43b842f5..89f147f2 100644 --- a/scripts/src/apply-patch-v2.ts +++ b/scripts/src/apply-patch-v2.ts @@ -12,6 +12,9 @@ export interface UpdateChunk { oldLines: string[]; newLines: string[]; contextLinePairs: Array<{ oldIndex: number; newIndex: number }>; + contextLineCount: number; + addedLineCount: number; + deletedLineCount: number; isEndOfFile: boolean; } @@ -177,6 +180,24 @@ export function parseApplyPatchV2(patchText: string): PatchParseResult { index += 1; continue; } + if (line === beginMarker) { + pushUniqueHint( + hints, + "apply-patch hint: ignored nested MiniMax-style Begin Patch marker", + `apply-patch hint: ignored nested MiniMax-style Begin Patch marker on line ${index + 1}; keep exactly one ${beginMarker}/${endMarker} envelope around all hunks.`, + ); + index += 1; + continue; + } + if (line === endMarker) { + pushUniqueHint( + hints, + "apply-patch hint: ignored nested MiniMax-style End Patch marker", + `apply-patch hint: ignored nested MiniMax-style End Patch marker on line ${index + 1}; keep exactly one ${beginMarker}/${endMarker} envelope around all hunks.`, + ); + index += 1; + continue; + } if (line.startsWith(addFileMarker)) { const filePath = validatePatchPath(line.slice(addFileMarker.length), index + 1); index += 1; @@ -331,13 +352,13 @@ async function applyPatchV2Hunks(executor: ApplyPatchV2Executor, hunks: PatchHun try { if (hunk.kind === "add") { await applyWrite(hunk.path, hunk.content); - changed.push(`A ${hunk.path}`); + pushChanged(changed, `A ${hunk.path}`); outcomes.push({ ...outcomeBase(hunk, index), status: "applied", change: `A ${hunk.path}` }); continue; } if (hunk.kind === "delete") { await applyDelete(hunk.path); - changed.push(`D ${hunk.path}`); + pushChanged(changed, `D ${hunk.path}`); outcomes.push({ ...outcomeBase(hunk, index), status: "applied", change: `D ${hunk.path}` }); continue; } @@ -345,13 +366,13 @@ async function applyPatchV2Hunks(executor: ApplyPatchV2Executor, hunks: PatchHun const update = deriveUpdatedContent(hunk.path, originalContent, hunk.chunks); if (hunk.movePath !== null && hunk.movePath !== hunk.path) { await applyWrite(hunk.movePath, update.newContent); - changed.push(`M ${hunk.movePath}`); + pushChanged(changed, `M ${hunk.movePath}`); await applyDelete(hunk.path); outcomes.push({ ...outcomeBase(hunk, index), action: "move", status: "applied", change: `M ${hunk.movePath}` }); continue; } await applyWrite(hunk.path, update.newContent); - changed.push(`M ${hunk.path}`); + pushChanged(changed, `M ${hunk.path}`); outcomes.push({ ...outcomeBase(hunk, index), status: "applied", change: `M ${hunk.path}` }); } catch (error) { const partialChanges = changed.slice(changedBefore); @@ -373,6 +394,10 @@ async function applyPatchV2Hunks(executor: ApplyPatchV2Executor, hunks: PatchHun return { changed, outcomes }; } +function pushChanged(changed: string[], item: string): void { + if (!changed.includes(item)) changed.push(item); +} + function outcomeBase(hunk: PatchHunk, index: number): Omit { if (hunk.kind === "update" && hunk.movePath !== null && hunk.movePath !== hunk.path) { return { hunk: index + 1, action: "move", path: hunk.path, targetPath: hunk.movePath }; @@ -412,9 +437,27 @@ function appendExpectedLinesFailureHint(lines: string[], details: Record): void { + const diagnostics = recordValue(cause.diagnostics); + if (diagnostics === null) return; + const candidates = Array.isArray(diagnostics.firstExpectedLineCandidates) ? diagnostics.firstExpectedLineCandidates.filter((item): item is number => typeof item === "number") : []; + const firstExpectedLine = typeof diagnostics.firstExpectedLine === "string" ? diagnostics.firstExpectedLine : ""; + if (firstExpectedLine.length > 0 && candidates.length > 0) { + const suffix = Boolean(diagnostics.firstExpectedLineCandidatesTruncated) ? "+" : ""; + lines.push(`First expected line appears near target line(s): ${candidates.join(", ")}${suffix}`); + } + if (typeof diagnostics.bestPrefixMatchedLines === "number" && diagnostics.bestPrefixMatchedLines > 0 && typeof diagnostics.bestPrefixStartLine === "number") { + lines.push(`Best partial context match: ${diagnostics.bestPrefixMatchedLines} expected line(s) matched starting near line ${diagnostics.bestPrefixStartLine}.`); + } + if (diagnostics.likelyMissingAddedPrefixes === true) { + lines.push("Hint: this hunk looks like a large insertion whose new lines were written as context. Prefix every inserted line with +, keep only a few real existing context lines before/after the insertion, and regenerate the patch instead of editing it with sed."); + } +} + function recordValue(value: unknown): Record | null { return typeof value === "object" && value !== null && !Array.isArray(value) ? value as Record : null; } @@ -836,7 +879,7 @@ function validatePatchPath(value: string, line: number): string { function isFileHeader(line: string): boolean { const trimmed = line.trim(); - return trimmed.startsWith(addFileMarker) || trimmed.startsWith(deleteFileMarker) || trimmed.startsWith(updateFileMarker) || trimmed === endMarker; + return trimmed.startsWith(addFileMarker) || trimmed.startsWith(deleteFileMarker) || trimmed.startsWith(updateFileMarker) || trimmed === beginMarker || trimmed === endMarker; } function isUpdateChunkHeader(line: string): boolean { @@ -873,10 +916,14 @@ function parseUpdateChunk(lines: string[], startIndex: number, allowMissingConte const contextLinePairs: Array<{ oldIndex: number; newIndex: number }> = []; let parsed = 0; let isEndOfFile = false; + let contextLineCount = 0; + let addedLineCount = 0; + let deletedLineCount = 0; function pushContextLine(value: string): void { contextLinePairs.push({ oldIndex: oldLines.length, newIndex: newLines.length }); oldLines.push(value); newLines.push(value); + contextLineCount += 1; } while (index < lines.length - 1) { const line = lines[index] ?? ""; @@ -893,8 +940,10 @@ function parseUpdateChunk(lines: string[], startIndex: number, allowMissingConte pushContextLine(line.slice(1)); } else if (marker === "+") { newLines.push(line.slice(1)); + addedLineCount += 1; } else if (marker === "-") { oldLines.push(line.slice(1)); + deletedLineCount += 1; } else if (line.length === 0) { pushContextLine(""); } else { @@ -909,7 +958,7 @@ function parseUpdateChunk(lines: string[], startIndex: number, allowMissingConte index += 1; } if (parsed === 0) throw new ApplyPatchV2Error("update chunk does not contain any lines", { line: startIndex + 1 }); - return { chunk: { changeContext, sourceStartLine, oldLines, newLines, contextLinePairs, isEndOfFile }, nextIndex: index }; + return { chunk: { changeContext, sourceStartLine, oldLines, newLines, contextLinePairs, contextLineCount, addedLineCount, deletedLineCount, isEndOfFile }, nextIndex: index }; } function parseUnifiedDiffHunkHeader(line: string): { oldStart: number } | null { @@ -962,6 +1011,7 @@ function computeReplacements(filePath: string, originalLines: string[], chunks: path: filePath, chunk: chunkIndex + 1, expected: chunk.oldLines.join("\n"), + diagnostics: expectedLineDiagnostics(originalLines, chunk, preferredStart), }); } replacements.push([found, pattern.length, preserveMatchedContextLines(originalLines, found, newLines, chunk.contextLinePairs, pattern.length)]); @@ -971,6 +1021,63 @@ function computeReplacements(filePath: string, originalLines: string[], chunks: return replacements; } +function expectedLineDiagnostics(originalLines: string[], chunk: UpdateChunk, preferredStart: number): Record { + const firstExpectedLine = chunk.oldLines.find((line) => line.trim().length > 0) ?? ""; + const firstExpectedLineCandidates = firstExpectedLine.length === 0 ? [] : candidateLineNumbers(originalLines, firstExpectedLine, 8); + const prefix = bestPrefixMatch(originalLines, chunk.oldLines, firstExpectedLine, preferredStart); + return { + firstExpectedLine, + firstExpectedLineCandidates, + firstExpectedLineCandidatesTruncated: firstExpectedLine.length > 0 && candidateLineNumbers(originalLines, firstExpectedLine, 9).length > firstExpectedLineCandidates.length, + bestPrefixMatchedLines: prefix.matchedLines, + bestPrefixStartLine: prefix.startLine, + likelyMissingAddedPrefixes: likelyMissingAddedPrefixes(chunk, prefix.matchedLines), + }; +} + +function candidateLineNumbers(lines: string[], expectedLine: string, limit: number): number[] { + const result: number[] = []; + for (let index = 0; index < lines.length; index += 1) { + if (lineEquivalent(lines[index] ?? "", expectedLine)) { + result.push(index + 1); + if (result.length >= limit) break; + } + } + return result; +} + +function bestPrefixMatch(lines: string[], expectedLines: string[], firstExpectedLine: string, preferredStart: number): { startLine: number | null; matchedLines: number } { + let best = { startLine: null as number | null, matchedLines: 0 }; + if (expectedLines.length === 0) return best; + for (let index = Math.max(0, preferredStart); index < lines.length; index += 1) { + if (firstExpectedLine.length > 0 && !lineEquivalent(lines[index] ?? "", firstExpectedLine)) continue; + let matched = 0; + while (index + matched < lines.length && matched < expectedLines.length && lineEquivalent(lines[index + matched] ?? "", expectedLines[matched] ?? "")) matched += 1; + if (matched > best.matchedLines) best = { startLine: index + 1, matchedLines: matched }; + if (matched === expectedLines.length) break; + } + if (best.matchedLines > 0 || preferredStart <= 0) return best; + for (let index = 0; index < Math.min(preferredStart, lines.length); index += 1) { + if (firstExpectedLine.length > 0 && !lineEquivalent(lines[index] ?? "", firstExpectedLine)) continue; + let matched = 0; + while (index + matched < lines.length && matched < expectedLines.length && lineEquivalent(lines[index + matched] ?? "", expectedLines[matched] ?? "")) matched += 1; + if (matched > best.matchedLines) best = { startLine: index + 1, matchedLines: matched }; + } + return best; +} + +function likelyMissingAddedPrefixes(chunk: UpdateChunk, bestPrefixMatchedLines: number): boolean { + if (chunk.deletedLineCount > 0) return false; + if (chunk.oldLines.length < 8) return false; + if (chunk.addedLineCount > 2) return false; + if (chunk.contextLineCount < 8) return false; + return bestPrefixMatchedLines > 0 && bestPrefixMatchedLines < chunk.oldLines.length; +} + +function lineEquivalent(left: string, right: string): boolean { + return left === right || left.trimEnd() === right.trimEnd() || left.trim() === right.trim() || normalizeLine(left) === normalizeLine(right); +} + function preserveMatchedContextLines(originalLines: string[], found: number, newLines: string[], contextLinePairs: UpdateChunk["contextLinePairs"], matchedOldLength: number): string[] { if (contextLinePairs.length === 0) return newLines; const result = [...newLines]; diff --git a/scripts/ssh-argv-guidance-contract-test.ts b/scripts/ssh-argv-guidance-contract-test.ts index 91b04245..2aac5b09 100644 --- a/scripts/ssh-argv-guidance-contract-test.ts +++ b/scripts/ssh-argv-guidance-contract-test.ts @@ -1037,6 +1037,70 @@ export async function runSshArgvGuidanceContract(): Promise { failedCompoundVisibleV2.stderr, ); + const missingPlusLargeInsertVisibleV2 = await applyPatchV2FixtureAttempt([ + "*** Begin Patch", + "*** Update File: access-control.test.ts", + "@@", + " });", + "", + " test(\"cloud api accepts read-only workspace.build / debug.download sub-actions without --reason\", async () => {", + " const receivedJobs = [];", + " const executor = createServer(async (request, response) => {", + " const body = await requestJson(request);", + " receivedJobs.push({ url: request.url, body });", + " response.writeHead(200, { \"content-type\": \"application/json; charset=utf-8\" });", + " response.end(JSON.stringify({ accepted: true, status: \"completed\" }));", + " });", + " });", + "", + " test(\"cloud api routes device-pod probe GET requests through executor jobs\", async () => {", + "*** End Patch", + "", + ].join("\n"), { + "access-control.test.ts": [ + "test(\"cloud api bounds device-pod job output payloads\", async () => {", + "});", + "", + "test(\"cloud api routes device-pod probe GET requests through executor jobs\", async () => {", + "});", + "", + ].join("\n"), + }, { stderrOutput: true }); + assertCondition(missingPlusLargeInsertVisibleV2.exitCode === 1 && missingPlusLargeInsertVisibleV2.error === null, "v2 should still reject unsafe large insertion hunks whose added lines are missing + prefixes", missingPlusLargeInsertVisibleV2); + assertCondition( + missingPlusLargeInsertVisibleV2.stderr.includes("First expected line appears near target line(s): 2, 5") + && missingPlusLargeInsertVisibleV2.stderr.includes("Best partial context match: 2 expected line(s) matched") + && missingPlusLargeInsertVisibleV2.stderr.includes("large insertion whose new lines were written as context") + && missingPlusLargeInsertVisibleV2.stderr.includes("regenerate the patch instead of editing it with sed"), + "v2 missing-plus failure should diagnose the MiniMax sed-regression pattern explicitly", + missingPlusLargeInsertVisibleV2.stderr, + ); + + const nestedEnvelopeV2 = await applyPatchV2FixtureAttempt([ + "*** Begin Patch", + "*** Update File: nested-envelope.txt", + "@@", + " alpha", + "*** End Patch", + "*** Begin Patch", + "*** Update File: nested-envelope.txt", + "@@", + "-beta", + "+BETA", + "*** End Patch", + "", + ].join("\n"), { + "nested-envelope.txt": "alpha\nbeta\n", + }, { stderrOutput: true }); + assertCondition(nestedEnvelopeV2.exitCode === 0 && nestedEnvelopeV2.error === null, "v2 should accept MiniMax-style nested patch envelopes between hunks", nestedEnvelopeV2); + assertCondition(nestedEnvelopeV2.files["nested-envelope.txt"] === "alpha\nBETA\n", "v2 nested-envelope compatibility should still apply the later hunk", nestedEnvelopeV2); + assertCondition( + nestedEnvelopeV2.stderr.includes("ignored nested MiniMax-style End Patch marker") + && nestedEnvelopeV2.stderr.includes("ignored nested MiniMax-style Begin Patch marker"), + "v2 nested-envelope compatibility should emit canonical envelope hints", + nestedEnvelopeV2.stderr, + ); + const addBeforeFailedUpdateV2 = await applyPatchV2FixtureAttempt([ "*** Begin Patch", "*** Add File: hwpod",