Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 29 additions & 8 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,11 +97,13 @@ export type ApplyPatchFailure = {
filePath: string;
operation: ApplyPatchOperation;
message: string;
code?: string | undefined;
};

export type ApplyPatchRecoveryInstructions = {
mustReadFiles: string[];
mustNotReadFiles: string[];
failedFiles: string[];
};

export type ApplyPatchResult = {
Expand Down Expand Up @@ -1207,7 +1209,11 @@ async function applyParsedPatchDetailed(
fuzz += hunkFuzz;
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
failures.push({ filePath: hunk.filePath, operation: hunk.type, message });
const code =
error && typeof error === "object" && "code" in error && typeof error.code === "string"
? error.code
: undefined;
failures.push({ filePath: hunk.filePath, operation: hunk.type, message, code });
}
await notifyApplyPatchProgress(onProgress, {
applied: appliedFiles.length,
Expand All @@ -1221,20 +1227,28 @@ async function applyParsedPatchDetailed(
appliedFiles,
failures,
hasPartialSuccess: appliedFiles.length > 0 && failures.length > 0,
recoveryInstructions: { mustReadFiles: [], mustNotReadFiles: [] },
recoveryInstructions: { mustReadFiles: [], mustNotReadFiles: [], failedFiles: [] },
details: { fuzz },
};
result.recoveryInstructions = createRecoveryInstructions(result);
return result;
}

function isRereadCandidate(failure: ApplyPatchFailure): boolean {
// ENOENT (missing file), EACCES/EPERM (permission), ENOTDIR/EISDIR (path) are
// not context mismatches — rereading will not fix them. Only context-line
// failures (no code, thrown by replaceChunks) benefit from a reread.
return failure.code === undefined;
}

function createRecoveryInstructions(
result: Pick<ApplyPatchResult, "appliedFiles" | "failures">,
): ApplyPatchRecoveryInstructions {
const mustReadFiles = [...new Set(result.failures.map((failure) => failure.filePath))];
const mustReadFiles = [...new Set(result.failures.filter(isRereadCandidate).map((failure) => failure.filePath))];
const mustReadFileSet = new Set(mustReadFiles);
const failedFileSet = new Set(result.failures.map((failure) => failure.filePath));
const mustNotReadFiles = [...new Set(result.appliedFiles.filter((filePath) => !mustReadFileSet.has(filePath)))];
return { mustReadFiles, mustNotReadFiles };
return { mustReadFiles, mustNotReadFiles, failedFiles: [...failedFileSet] };
}

export async function applyPatch(cwd: string, patchText: string): Promise<string[]> {
Expand All @@ -1249,7 +1263,11 @@ export async function applyPatch(cwd: string, patchText: string): Promise<string
appliedFiles.push(appliedFile);
} catch (error) {
const message = error instanceof Error ? error.message : String(error);
const failure = { filePath: hunk.filePath, operation: hunk.type, message } satisfies ApplyPatchFailure;
const code =
error && typeof error === "object" && "code" in error && typeof error.code === "string"
? error.code
: undefined;
const failure = { filePath: hunk.filePath, operation: hunk.type, message, code } satisfies ApplyPatchFailure;
const result: ApplyPatchResult = {
summaries,
appliedFiles,
Expand Down Expand Up @@ -1430,17 +1448,20 @@ export function createApplyPatchTool(): ApplyPatchToolDefinition {
},
);
if (result.failures.length > 0) {
const failureLines = result.failures.map(
(failure) => `- ${failure.filePath} (${failure.operation}): ${failure.message}`,
);
const mustReadFiles = result.recoveryInstructions.mustReadFiles;
const failed = mustReadFiles.join(", ");
const mustReadText = mustReadFiles.join(" and ");
return {
content: [
{
type: "text",
text: [
result.hasPartialSuccess ? "apply_patch partially failed." : "apply_patch failed.",
`Failed: ${failed}`,
`Recovery: MUST read ${mustReadText} before retrying.`,
"Failed:",
...failureLines,
mustReadFiles.length > 0 ? `Recovery: MUST read ${mustReadText} before retrying.` : "",
result.appliedFiles.length > 0
? "Earlier file actions in this patch were already applied."
: "No file actions were applied.",
Expand Down
48 changes: 47 additions & 1 deletion test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -894,7 +894,8 @@ EOF`;
// then
const text = result.content.find((block) => block.type === "text")?.text ?? "";
expect(text).toContain("apply_patch partially failed.");
expect(text).toContain("Failed: broken.txt");
expect(text).toContain("Failed:");
expect(text).toContain("- broken.txt (update):");
expect(text).toContain("Recovery: MUST read broken.txt before retrying.");
expect(text).toContain("Earlier file actions in this patch were already applied.");
expect(text).toContain(
Expand Down Expand Up @@ -925,6 +926,51 @@ EOF`;
expect(text).toContain("No file actions were applied.");
});

it("#given update of missing file #when executed #then discloses ENOENT reason without reread advice", async () => {
// given
const directory = await createTempDirectory();
const patch = `*** Begin Patch
*** Update File: missing.txt
@@
-old
+new
*** End Patch`;

// when
const result = await createApplyPatchTool().execute("apply-patch-test", { input: patch }, undefined, undefined, {
cwd: directory,
} as never);

// then
const text = result.content.find((block) => block.type === "text")?.text ?? "";
expect(text).toContain("missing.txt");
expect(text).toContain("ENOENT");
expect(text).not.toContain("MUST read");
});

it("#given context mismatch on existing file #when executed #then discloses reason with reread advice", async () => {
// given
const directory = await createTempDirectory();
await writeFile(path.join(directory, "exists.txt"), "line\n", "utf-8");
const patch = `*** Begin Patch
*** Update File: exists.txt
@@
-missing
+new
*** End Patch`;

// when
const result = await createApplyPatchTool().execute("apply-patch-test", { input: patch }, undefined, undefined, {
cwd: directory,
} as never);

// then
const text = result.content.find((block) => block.type === "text")?.text ?? "";
expect(text).toContain("exists.txt");
expect(text).toContain("MUST read exists.txt");
expect(text).toMatch(/expected lines|context|find/i);
});

it("#given concurrent patches to different lines in one file #when applied #then preserves both updates", async () => {
// given
const directory = await createTempDirectory();
Expand Down