From d3a89bcff0ededaa026769b1c4c22ece627643cb Mon Sep 17 00:00:00 2001 From: Ayman Hamed Date: Tue, 4 Aug 2026 17:03:14 +0300 Subject: [PATCH] =?UTF-8?q?feat(tools):=20apply=5Fpatch=20=E2=80=94=20git-?= =?UTF-8?q?style=20unified-diff=20edit=20tool?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a targeted edit tool so agents stop reading+rewriting whole files to change a few lines. - UnifiedDiff: Foundation-only parser + applier. Applies each hunk by matching its context (the @@ line numbers are hints, not authority), with nearest-hint selection and cumulative offset across hunks. All-or-nothing: any hunk that doesn't match leaves the file untouched. Preserves trailing-newline state. - PatchFileTool (apply_patch): reads an existing file, applies a unified diff, writes atomically. requiresConfirmation (the diff shows in the approval sheet). Clear errors name the first unmatched hunk so the model can re-read and regenerate. Robustness: wrong @@ numbers still apply via context (the property that makes LLM-generated diffs usable). Tests cover offset drift, nearest-hint, context not found → unchanged, trailing-newline preservation, insertion/removal. Full suite: 155 green. Co-Authored-By: Claude Opus 4.8 --- .../SwiftAgentKitTools/FileSystemTools.swift | 73 ++++++++ Sources/SwiftAgentKitTools/UnifiedDiff.swift | 165 ++++++++++++++++++ .../SwiftAgentKitToolsTests.swift | 35 ++++ .../UnifiedDiffTests.swift | 112 ++++++++++++ 4 files changed, 385 insertions(+) create mode 100644 Sources/SwiftAgentKitTools/UnifiedDiff.swift create mode 100644 Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift diff --git a/Sources/SwiftAgentKitTools/FileSystemTools.swift b/Sources/SwiftAgentKitTools/FileSystemTools.swift index 8e2ba6c..d2a1b32 100644 --- a/Sources/SwiftAgentKitTools/FileSystemTools.swift +++ b/Sources/SwiftAgentKitTools/FileSystemTools.swift @@ -103,6 +103,79 @@ public struct FileWriteTool: AgentTool { } } +/// Apply a git-style unified diff to a single existing file. Confirmation +/// required — it mutates the disk. Applies by matching each hunk's context +/// (line numbers are treated as hints), so a diff whose `@@` numbers drifted +/// still applies. Prefer this over rewriting a whole file with `write_file`. +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). \ + 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). Prefer \ + this over `write_file` for changes to an existing file. Requires approval. + """ + public let parameters = ToolParameters( + properties: [ + "path": ToolParameterProperty(type: "string", description: "File to patch (a leading ~ is expanded)."), + "patch": ToolParameterProperty(type: "string", description: "A unified diff (git/diff -u). Only text hunks; no binary/rename."), + ], + required: ["path", "patch"] + ) + + public var requiresConfirmation: Bool { true } + + public init() {} + + public func execute(parameters: [String: Any]) async throws -> AgentToolResult { + guard let raw = (parameters["path"] as? String), !raw.isEmpty else { + return .error(toolCallId: "", toolName: name, message: "apply_patch requires a `path`.") + } + guard let patch = parameters["patch"] as? String, !patch.isEmpty else { + return .error(toolCallId: "", toolName: name, message: "apply_patch requires a `patch` (a unified diff).") + } + let path = expandPath(raw) + 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.") + } + guard let source = String(data: data, encoding: .utf8) else { + return .error(toolCallId: "", toolName: name, message: "Not a UTF-8 text file: \(raw)") + } + guard let hunks = UnifiedDiff.parse(patch) else { + return .error(toolCallId: "", toolName: name, + message: "The `patch` isn't a valid unified diff (no @@ hunks found).") + } + + switch UnifiedDiff.apply(hunks, to: source) { + case .failure(let err): + switch err { + case .hunkNotFound(let index, let preview): + return .error(toolCallId: "", toolName: name, message: """ + Hunk \(index + 1) didn't match \(raw) — the surrounding lines weren't found: "\(preview)". \ + Re-read the file and regenerate the diff against its current contents. Nothing was changed. + """) + case .cannotAnchor(let index): + return .error(toolCallId: "", toolName: name, message: """ + Hunk \(index + 1) is an insertion whose line number is past the end of \(raw). \ + Add a line of context so it can be anchored. Nothing was changed. + """) + } + case .success(let patched): + do { + try Data(patched.utf8).write(to: URL(fileURLWithPath: path), options: .atomic) + } catch { + return .error(toolCallId: "", toolName: name, message: "Write failed: \(error.localizedDescription)") + } + let added = hunks.reduce(0) { $0 + $1.lines.filter { if case .add = $0 { return true }; return false }.count } + let removed = hunks.reduce(0) { $0 + $1.lines.filter { if case .remove = $0 { return true }; return false }.count } + return .success(toolCallId: "", toolName: name, + result: "Applied \(hunks.count) hunk\(hunks.count == 1 ? "" : "s") (+\(added)/-\(removed)) to \(raw).") + } + } +} + /// List a directory's entries. Unconfirmed (read-only). public struct ListDirTool: AgentTool { public let name = "list_dir" diff --git a/Sources/SwiftAgentKitTools/UnifiedDiff.swift b/Sources/SwiftAgentKitTools/UnifiedDiff.swift new file mode 100644 index 0000000..aaab629 --- /dev/null +++ b/Sources/SwiftAgentKitTools/UnifiedDiff.swift @@ -0,0 +1,165 @@ +// +// UnifiedDiff.swift +// SwiftAgentKitTools +// +// A small, Foundation-only unified-diff parser and applier. Applies hunks by +// matching their context (line numbers in `@@` headers are treated as hints, +// not authority), so a patch whose line numbers drifted still applies — the +// reliability property that makes LLM-generated diffs usable. Pure and +// side-effect free, so it unit-tests directly on strings. +// + +import Foundation + +enum UnifiedDiff { + + /// One line within a hunk body. + enum Line: Equatable { + case context(String) + case remove(String) + case add(String) + } + + /// A single `@@ … @@` hunk. + struct Hunk: Equatable { + /// 1-based old-file start line from the header (a hint for locating the hunk). + var oldStart: Int + var lines: [Line] + + /// Lines expected to exist in the source (context + removed), in order. + var before: [String] { + lines.compactMap { + switch $0 { + case .context(let s), .remove(let s): return s + case .add: return nil + } + } + } + /// Lines after applying the hunk (context + added), in order. + var after: [String] { + lines.compactMap { + switch $0 { + case .context(let s), .add(let s): return s + case .remove: return nil + } + } + } + } + + enum ApplyError: Error, Equatable { + /// A hunk's context/removed block was found nowhere in the source. + case hunkNotFound(index: Int, preview: String) + /// An insertion-only hunk couldn't be anchored (its line number is out of range). + case cannotAnchor(index: Int) + } + + // MARK: - Parse + + /// Parse unified-diff text into hunks. File headers (`diff --git`, `index`, + /// `--- `, `+++ `) are tolerated and ignored — the caller already knows the + /// target path. Returns nil if there are no hunks. + static func parse(_ patch: String) -> [Hunk]? { + var hunks: [Hunk] = [] + var current: Hunk? + + func flush() { + if let c = current, !c.lines.isEmpty { hunks.append(c) } + current = nil + } + + // Split into lines, dropping trailing empties (the patch string's own + // trailing newline — a blank *context* line in a real diff is " ", not ""). + var rawLines = patch.components(separatedBy: "\n") + while rawLines.last == "" { rawLines.removeLast() } + + for raw in rawLines { + if raw.hasPrefix("@@") { + flush() + current = Hunk(oldStart: parseOldStart(raw) ?? 1, lines: []) + continue + } + // Ignore file headers whether or not we're inside a hunk yet. + if raw.hasPrefix("diff --git") || raw.hasPrefix("index ") + || raw.hasPrefix("--- ") || raw.hasPrefix("+++ ") { + continue + } + guard current != nil else { continue } // skip preamble before the first hunk + if raw.hasPrefix("\\") { continue } // "\ No newline at end of file" + + if raw.isEmpty { + current?.lines.append(.context("")) // tolerate a bare blank context line + } else { + let body = String(raw.dropFirst()) + switch raw.first { + case "+": current?.lines.append(.add(body)) + case "-": current?.lines.append(.remove(body)) + case " ": current?.lines.append(.context(body)) + default: continue // unknown line — ignore + } + } + } + flush() + return hunks.isEmpty ? nil : hunks + } + + /// Extract the old-file start line from an `@@ -a,b +c,d @@` header. + private static func parseOldStart(_ header: String) -> Int? { + guard let dash = header.firstIndex(of: "-") else { return nil } + let rest = header[header.index(after: dash)...] + let digits = rest.prefix { $0.isNumber } + return Int(digits) + } + + // MARK: - Apply + + /// Apply hunks to `source`, matching each by context near its hint line. + /// All-or-nothing: any failure returns an error and no partial result. + static func apply(_ hunks: [Hunk], to source: String) -> Result { + let hasTrailingNewline = source.hasSuffix("\n") + var lines = source.components(separatedBy: "\n") + if hasTrailingNewline { lines.removeLast() } // drop the empty element after the final "\n" + + var offset = 0 // cumulative shift from prior hunks + + for (i, hunk) in hunks.enumerated() { + let before = hunk.before + let after = hunk.after + let hint = max(0, hunk.oldStart - 1 + offset) + + if before.isEmpty { + // Pure insertion — anchor at the hinted line. + guard hint <= lines.count else { return .failure(.cannotAnchor(index: i)) } + lines.insert(contentsOf: after, at: hint) + offset += after.count + continue + } + + guard let match = locate(before, in: lines, near: hint) else { + return .failure(.hunkNotFound(index: i, preview: preview(before))) + } + lines.replaceSubrange(match..<(match + before.count), with: after) + offset += after.count - before.count + } + + var result = lines.joined(separator: "\n") + if hasTrailingNewline { result += "\n" } + return .success(result) + } + + /// Find the start index where `block` occurs contiguously in `lines`, + /// choosing the occurrence nearest `hint`. Line numbers are hints only. + private static func locate(_ block: [String], in lines: [String], near hint: Int) -> Int? { + guard !block.isEmpty, block.count <= lines.count else { return nil } + var matches: [Int] = [] + for start in 0...(lines.count - block.count) { + if Array(lines[start..<(start + block.count)]) == block { + matches.append(start) + } + } + return matches.min { abs($0 - hint) < abs($1 - hint) } + } + + private static func preview(_ block: [String]) -> String { + block.prefix(3).joined(separator: " ⏎ ").prefix(120).description + } +} diff --git a/Tests/SwiftAgentKitToolsTests/SwiftAgentKitToolsTests.swift b/Tests/SwiftAgentKitToolsTests/SwiftAgentKitToolsTests.swift index 663c285..b2c23cd 100644 --- a/Tests/SwiftAgentKitToolsTests/SwiftAgentKitToolsTests.swift +++ b/Tests/SwiftAgentKitToolsTests/SwiftAgentKitToolsTests.swift @@ -38,6 +38,41 @@ private func tempDir() -> URL { #expect(FileReadTool().requiresConfirmation == false) } +@Test func applyPatchEditsFileInPlace() async throws { + let dir = tempDir() + let path = dir.appendingPathComponent("code.txt").path + _ = try await FileWriteTool().execute(parameters: ["path": path, "content": "alpha\nbeta\ngamma\n"]) + + let patch = "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n" + let result = try await PatchFileTool().execute(parameters: ["path": path, "patch": patch]) + #expect(result.isError == false) + + let read = try await FileReadTool().execute(parameters: ["path": path]) + #expect(read.result == "alpha\nBETA\ngamma\n") +} + +@Test func applyPatchMissingFileErrors() async throws { + let path = tempDir().appendingPathComponent("nope.txt").path + let result = try await PatchFileTool().execute(parameters: ["path": path, "patch": "@@ -1 +1 @@\n-x\n+y\n"]) + #expect(result.isError == true) +} + +@Test func applyPatchNonMatchingHunkLeavesFileUnchanged() async throws { + let dir = tempDir() + let path = dir.appendingPathComponent("code.txt").path + _ = try await FileWriteTool().execute(parameters: ["path": path, "content": "a\nb\nc\n"]) + + let result = try await PatchFileTool().execute(parameters: ["path": path, "patch": "@@ -1,2 +1,2 @@\n x\n-y\n+Y\n"]) + #expect(result.isError == true) + + let read = try await FileReadTool().execute(parameters: ["path": path]) + #expect(read.result == "a\nb\nc\n") // untouched +} + +@Test func applyPatchRequiresConfirmation() { + #expect(PatchFileTool().requiresConfirmation == true) +} + @Test func readMissingFileErrors() async throws { let result = try await FileReadTool().execute(parameters: ["path": "/no/such/file.xyz"]) #expect(result.isError == true) diff --git a/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift b/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift new file mode 100644 index 0000000..698289a --- /dev/null +++ b/Tests/SwiftAgentKitToolsTests/UnifiedDiffTests.swift @@ -0,0 +1,112 @@ +import Testing +import Foundation +@testable import SwiftAgentKitTools + +struct UnifiedDiffTests { + + private func applied(_ patch: String, to source: String) -> String? { + guard let hunks = UnifiedDiff.parse(patch) else { return nil } + if case .success(let out) = UnifiedDiff.apply(hunks, to: source) { return out } + return nil + } + + // MARK: - Parse + + @Test func ignoresFileHeadersAndParsesHunk() { + let patch = """ + diff --git a/f.txt b/f.txt + index 111..222 100644 + --- a/f.txt + +++ b/f.txt + @@ -1,3 +1,3 @@ + a + -b + +B + c + """ + let hunks = UnifiedDiff.parse(patch) + #expect(hunks?.count == 1) + #expect(hunks?[0].oldStart == 1) + #expect(hunks?[0].before == ["a", "b", "c"]) + #expect(hunks?[0].after == ["a", "B", "c"]) + } + + @Test func malformedReturnsNil() { + #expect(UnifiedDiff.parse("not a diff at all") == nil) + #expect(UnifiedDiff.parse("") == nil) + } + + // MARK: - Apply + + @Test func singleHunkReplace() { + let source = "a\nb\nc\n" + let patch = "@@ -1,3 +1,3 @@\n a\n-b\n+B\n c\n" + #expect(applied(patch, to: source) == "a\nB\nc\n") + } + + /// The core reliability guarantee: wrong @@ line numbers still apply because + /// we match on context, not the header numbers. + @Test func wrongLineNumbersStillApplyViaContext() { + let source = "one\ntwo\nthree\nfour\nfive\n" + // Claims the change is at line 99, but context locates it at "three". + let patch = "@@ -99,3 +99,3 @@\n two\n-three\n+THREE\n four\n" + #expect(applied(patch, to: source) == "one\ntwo\nTHREE\nfour\nfive\n") + } + + @Test func multipleHunksWithOffsetDrift() { + let source = "1\n2\n3\n4\n5\n6\n" + // First hunk inserts a line, shifting later line numbers; second still applies. + let patch = """ + @@ -1,2 +1,3 @@ + 1 + +1.5 + 2 + @@ -5,2 +5,2 @@ + 5 + -6 + +six + """ + #expect(applied(patch, to: source) == "1\n1.5\n2\n3\n4\n5\nsix\n") + } + + @Test func contextNotFoundIsHunkNotFoundAndChangesNothing() { + let source = "a\nb\nc\n" + let patch = "@@ -1,2 +1,2 @@\n x\n-y\n+Y\n" + let hunks = UnifiedDiff.parse(patch)! + let result = UnifiedDiff.apply(hunks, to: source) + guard case .failure(let err) = result else { Issue.record("expected failure"); return } + if case .hunkNotFound(let index, _) = err { #expect(index == 0) } + else { Issue.record("expected hunkNotFound, got \(err)") } + } + + @Test func pureInsertionAtContext() { + let source = "start\nend\n" + let patch = "@@ -1,1 +1,2 @@\n start\n+middle\n" + #expect(applied(patch, to: source) == "start\nmiddle\nend\n") + } + + @Test func removeOnlyHunk() { + let source = "keep\ndrop\nkeep2\n" + let patch = "@@ -1,3 +1,2 @@\n keep\n-drop\n keep2\n" + #expect(applied(patch, to: source) == "keep\nkeep2\n") + } + + @Test func preservesNoTrailingNewline() { + let source = "a\nb" // no trailing newline + let patch = "@@ -1,2 +1,2 @@\n a\n-b\n+B\n" + #expect(applied(patch, to: source) == "a\nB") + } + + @Test func preservesTrailingNewline() { + let source = "a\nb\n" + let patch = "@@ -1,2 +1,2 @@\n a\n-b\n+B\n" + #expect(applied(patch, to: source) == "a\nB\n") + } + + @Test func picksOccurrenceNearestHint() { + // "x" appears twice; hint (line 4) should pick the second one. + let source = "x\ny\nz\nx\nw\n" + let patch = "@@ -4,1 +4,1 @@\n-x\n+X\n" + #expect(applied(patch, to: source) == "x\ny\nz\nX\nw\n") + } +}