From fb650dea5638d9bdb3c32521c0b3436365c5ffce Mon Sep 17 00:00:00 2001 From: Ayman Hamed Date: Fri, 9 Oct 2026 23:26:07 +0300 Subject: [PATCH] apply_patch: one file per call, folders named as folders A diff spanning two files had every hunk tried against `path`, so the second file's hunk "didn't match" (Naseem 2026-10-09: PomoApp.swift and project.pbxproj in one patch). It is now refused with both names and nothing changed; the description says one file per call. A folder path says it is a folder instead of suggesting write_file. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../SwiftAgentKitTools/FileSystemTools.swift | 18 ++++++++ Sources/SwiftAgentKitTools/UnifiedDiff.swift | 16 +++++++ .../UnifiedDiffTests.swift | 46 +++++++++++++++++++ 3 files changed, 80 insertions(+) diff --git a/Sources/SwiftAgentKitTools/FileSystemTools.swift b/Sources/SwiftAgentKitTools/FileSystemTools.swift index 32d497b..ddf3afd 100644 --- a/Sources/SwiftAgentKitTools/FileSystemTools.swift +++ b/Sources/SwiftAgentKitTools/FileSystemTools.swift @@ -188,6 +188,7 @@ public struct PatchFileTool: AgentTool { public let name = "apply_patch" public let description = """ Edit an existing file by applying a unified diff (git / `diff -u` format). \ + One file per call: `path` is that file; to change two files, call it twice. \ Provide the smallest diff that makes the change — one or more `@@` hunks with \ a few lines of surrounding context; `-` lines are removed, `+` lines added. \ Line numbers in `@@` headers may be approximate (matched by context, tolerant \ @@ -238,6 +239,23 @@ public struct PatchFileTool: AgentTool { if let policy, let reason = policy.blockReason(for: path) { return .error(toolCallId: "", toolName: name, message: "Refusing to patch \(raw): \(reason).") } + // One file per call. A diff spanning two files had all its hunks tried + // against `path`, and the second file's hunk "didn't match" (Naseem, + // 2026-10-09: PomoApp.swift + project.pbxproj in one patch). + let targets = UnifiedDiff.targets(in: patch) + if targets.count > 1 { + return .error(toolCallId: "", toolName: name, message: """ + This patch changes \(targets.count) files (\(targets.joined(separator: ", "))); apply_patch takes \ + one file per call. Nothing was changed. Send one apply_patch per file, each with that file's `path`. + """) + } + var isDir: ObjCBool = false + if FileManager.default.fileExists(atPath: path, isDirectory: &isDir), isDir.boolValue { + return .error(toolCallId: "", toolName: name, message: """ + \(raw) is a folder. `path` must be the file the diff changes\ + \(targets.first.map { " (this diff names \($0))" } ?? ""). Nothing was changed. + """) + } guard let data = FileManager.default.contents(atPath: path) else { return .error(toolCallId: "", toolName: name, message: "Cannot read file to patch: \(raw). Use write_file to create a new file.") diff --git a/Sources/SwiftAgentKitTools/UnifiedDiff.swift b/Sources/SwiftAgentKitTools/UnifiedDiff.swift index 4a6aafc..4bd984f 100644 --- a/Sources/SwiftAgentKitTools/UnifiedDiff.swift +++ b/Sources/SwiftAgentKitTools/UnifiedDiff.swift @@ -109,6 +109,22 @@ enum UnifiedDiff { return hunks.isEmpty ? nil : hunks } + /// The files a diff changes, from its `+++` headers (a leading `a/`/`b/` + /// dropped, /dev/null skipped), in order and distinct. Empty when the diff + /// has no headers — the tool's `path` names its one file. + static func targets(in patch: String) -> [String] { + var out: [String] = [] + for raw in patch.components(separatedBy: "\n") where raw.hasPrefix("+++ ") { + var name = String(raw.dropFirst(4)) + if let tab = name.firstIndex(of: "\t") { name = String(name[.. Int? { guard let dash = header.firstIndex(of: "-") else { return nil } diff --git a/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift b/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift index 32cda0f..26b4185 100644 --- a/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift +++ b/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift @@ -149,4 +149,50 @@ struct UnifiedDiffTests { #expect(nearby.contains("line10")) #expect(nearby.contains("10 |")) // numbered, so the model can re-anchor } + + // MARK: - One file per call (Naseem, 2026-10-09) + + /// A model put PomoApp.swift's and project.pbxproj's hunks in one patch + /// with path=PomoApp.swift; the pbxproj hunk "didn't match" the Swift file. + @Test func targetsListsEachFileTheDiffChanges() { + let patch = """ + --- a/Pomo/PomoApp.swift + +++ b/Pomo/PomoApp.swift + @@ -1,1 +1,1 @@ + -a + +b + --- a/Pomo.xcodeproj/project.pbxproj + +++ b/Pomo.xcodeproj/project.pbxproj + @@ -1,1 +1,1 @@ + -c + +d + """ + #expect(UnifiedDiff.targets(in: patch) == ["Pomo/PomoApp.swift", "Pomo.xcodeproj/project.pbxproj"]) + #expect(UnifiedDiff.targets(in: "--- a/f.txt\n+++ b/f.txt\n@@ -1 +1 @@\n-a\n+b") == ["f.txt"]) + #expect(UnifiedDiff.targets(in: "@@ -1 +1 @@\n-a\n+b").isEmpty) + } + + @Test func aMultiFilePatchIsRefusedWithBothNames() async throws { + let dir = FileManager.default.temporaryDirectory.appendingPathComponent("sak-patch-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + let file = dir.appendingPathComponent("a.txt") + try "a\n".write(to: file, atomically: true, encoding: .utf8) + let patch = "--- a/a.txt\n+++ b/a.txt\n@@ -1 +1 @@\n-a\n+A\n--- a/b.txt\n+++ b/b.txt\n@@ -1 +1 @@\n-b\n+B" + let r = try await PatchFileTool().execute(parameters: ["path": file.path, "patch": patch]) + #expect(r.isError) + #expect(r.result.contains("a.txt") && r.result.contains("b.txt")) + #expect(r.result.contains("one file per call")) + #expect(try String(contentsOf: file, encoding: .utf8) == "a\n") + } + + @Test func aFolderPathIsNamedAsAFolder() async throws { + let dir = FileManager.default.temporaryDirectory.appendingPathComponent("sak-patch-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: dir) } + let r = try await PatchFileTool().execute(parameters: ["path": dir.path, "patch": "@@ -1 +1 @@\n-a\n+b"]) + #expect(r.isError) + #expect(r.result.contains("is a folder")) + #expect(!r.result.contains("Use write_file")) + } }