diff --git a/src/index.ts b/src/index.ts index be81b46..c47d4f5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -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 = { @@ -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, @@ -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, ): 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 { @@ -1249,7 +1263,11 @@ export async function applyPatch(cwd: string, patchText: string): Promise 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: [ @@ -1439,8 +1459,9 @@ export function createApplyPatchTool(): ApplyPatchToolDefinition { 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.", diff --git a/test/index.test.ts b/test/index.test.ts index 8bc23e2..97aaab2 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -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( @@ -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();