Coerce string patch hunks
Authored by
mfwolffe <wolffemf@dukes.jmu.edu>
- SHA
8f1b6b4c7122ce0147d11c647f796cb8c1c60a74- Parents
-
107215b - Tree
5d3e8f2
8f1b6b4
8f1b6b4c7122ce0147d11c647f796cb8c1c60a74107215b
5d3e8f2| Status | File | + | - |
|---|---|---|---|
| M |
src/loader/runtime/safeguard_services.py
|
5 | 2 |
| M |
src/loader/tools/file_tools.py
|
8 | 4 |
| M |
src/loader/tools/fs_safety.py
|
30 | 1 |
| M |
src/loader/utils/file_mutations.py
|
11 | 8 |
| M |
tests/test_bash_operator_surfaces.py
|
12 | 0 |
| M |
tests/test_expanded_tools.py
|
27 | 0 |
| M |
tests/test_safeguard_services.py
|
25 | 0 |
src/loader/runtime/safeguard_services.pymodified@@ -8,6 +8,8 @@ from dataclasses import dataclass | ||
| 8 | 8 | from difflib import get_close_matches |
| 9 | 9 | from pathlib import Path |
| 10 | 10 | |
| 11 | +from ..tools.fs_safety import coerce_structured_patch_payload | |
| 12 | + | |
| 11 | 13 | TEXT_REWRITE_SUFFIXES = frozenset( |
| 12 | 14 | { |
| 13 | 15 | ".c", |
@@ -953,7 +955,8 @@ class PreActionValidator: | ||
| 953 | 955 | if not html_declared_file_result.valid: |
| 954 | 956 | return html_declared_file_result |
| 955 | 957 | |
| 956 | - has_hunks = isinstance(hunks, list) and bool(hunks) | |
| 958 | + structured_hunks = coerce_structured_patch_payload(hunks) | |
| 959 | + has_hunks = bool(structured_hunks) | |
| 957 | 960 | has_raw_patch = isinstance(raw_patch, str) and bool(raw_patch.strip()) |
| 958 | 961 | if not has_hunks and not has_raw_patch: |
| 959 | 962 | return ValidationResult( |
@@ -965,7 +968,7 @@ class PreActionValidator: | ||
| 965 | 968 | |
| 966 | 969 | html_placeholder_result = self._validate_html_placeholder_patch( |
| 967 | 970 | str(file_path), |
| 968 | - hunks, | |
| 971 | + structured_hunks, | |
| 969 | 972 | raw_patch, |
| 970 | 973 | ) |
| 971 | 974 | if not html_placeholder_result.valid: |
src/loader/tools/file_tools.pymodified@@ -10,6 +10,7 @@ from .base import ConfirmationRequired, Tool, ToolResult | ||
| 10 | 10 | from .fs_safety import ( |
| 11 | 11 | StructuredPatchHunk, |
| 12 | 12 | apply_structured_patch, |
| 13 | + coerce_structured_patch_payload, | |
| 13 | 14 | ensure_safe_to_read, |
| 14 | 15 | ensure_safe_to_write, |
| 15 | 16 | make_structured_patch, |
@@ -514,7 +515,7 @@ class PatchTool(Tool): | ||
| 514 | 515 | async def execute( |
| 515 | 516 | self, |
| 516 | 517 | file_path: str, |
| 517 | - hunks: list[dict[str, Any]] | None = None, | |
| 518 | + hunks: list[dict[str, Any]] | dict[str, Any] | str | None = None, | |
| 518 | 519 | patch: str | None = None, |
| 519 | 520 | **kwargs: Any, |
| 520 | 521 | ) -> ToolResult: |
@@ -555,14 +556,17 @@ class PatchTool(Tool): | ||
| 555 | 556 | original_content = await asyncio.to_thread(path.read_text) |
| 556 | 557 | original_lines = original_content.splitlines() |
| 557 | 558 | raw_patch = patch or kwargs.get("diff") or kwargs.get("patch_text") |
| 559 | + structured_hunks = coerce_structured_patch_payload(hunks) | |
| 558 | 560 | parsed_hunks: list[StructuredPatchHunk] |
| 559 | - if hunks: | |
| 561 | + if structured_hunks: | |
| 560 | 562 | parsed_hunks = [ |
| 561 | - StructuredPatchHunk.from_dict_with_original( | |
| 563 | + hunk | |
| 564 | + if isinstance(hunk, StructuredPatchHunk) | |
| 565 | + else StructuredPatchHunk.from_dict_with_original( | |
| 562 | 566 | hunk, |
| 563 | 567 | original_lines=original_lines, |
| 564 | 568 | ) |
| 565 | - for hunk in hunks | |
| 569 | + for hunk in structured_hunks | |
| 566 | 570 | ] |
| 567 | 571 | elif isinstance(raw_patch, str) and raw_patch.strip(): |
| 568 | 572 | parsed_hunks = parse_unified_diff_patch(raw_patch) |
src/loader/tools/fs_safety.pymodified@@ -2,9 +2,10 @@ | ||
| 2 | 2 | |
| 3 | 3 | from __future__ import annotations |
| 4 | 4 | |
| 5 | +import json | |
| 6 | +import re | |
| 5 | 7 | from dataclasses import asdict, dataclass |
| 6 | 8 | from pathlib import Path |
| 7 | -import re | |
| 8 | 9 | |
| 9 | 10 | MAX_READ_SIZE = 10 * 1024 * 1024 |
| 10 | 11 | MAX_WRITE_SIZE = 10 * 1024 * 1024 |
@@ -82,6 +83,34 @@ class StructuredPatchHunk: | ||
| 82 | 83 | ) |
| 83 | 84 | |
| 84 | 85 | |
| 86 | +def coerce_structured_patch_payload( | |
| 87 | + value: object, | |
| 88 | +) -> list[dict[str, object] | StructuredPatchHunk]: | |
| 89 | + """Normalize structured patch payloads from native tool callers. | |
| 90 | + | |
| 91 | + Some local models serialize the `hunks` array as a JSON string even though | |
| 92 | + the tool schema asks for an array. Treat that as recoverable instead of | |
| 93 | + rejecting an otherwise valid patch call. | |
| 94 | + """ | |
| 95 | + | |
| 96 | + if isinstance(value, str): | |
| 97 | + if not value.strip(): | |
| 98 | + return [] | |
| 99 | + try: | |
| 100 | + value = json.loads(value) | |
| 101 | + except json.JSONDecodeError: | |
| 102 | + return [] | |
| 103 | + | |
| 104 | + if isinstance(value, StructuredPatchHunk): | |
| 105 | + return [value] | |
| 106 | + if isinstance(value, dict): | |
| 107 | + return [value] | |
| 108 | + if not isinstance(value, list): | |
| 109 | + return [] | |
| 110 | + | |
| 111 | + return [item for item in value if isinstance(item, (dict, StructuredPatchHunk))] | |
| 112 | + | |
| 113 | + | |
| 85 | 114 | def resolve_workspace_path( |
| 86 | 115 | raw_path: str, |
| 87 | 116 | *, |
src/loader/utils/file_mutations.pymodified@@ -13,6 +13,7 @@ from rich.text import Text | ||
| 13 | 13 | |
| 14 | 14 | from ..tools.fs_safety import ( |
| 15 | 15 | StructuredPatchHunk, |
| 16 | + coerce_structured_patch_payload, | |
| 16 | 17 | make_structured_patch, |
| 17 | 18 | parse_unified_diff_patch, |
| 18 | 19 | ) |
@@ -183,11 +184,12 @@ def _coerce_optional_text(value: Any) -> str | None: | ||
| 183 | 184 | |
| 184 | 185 | |
| 185 | 186 | def _coerce_patch_hunks(value: Any) -> list[StructuredPatchHunk]: |
| 186 | - if not isinstance(value, list): | |
| 187 | + patch_items = coerce_structured_patch_payload(value) | |
| 188 | + if not patch_items: | |
| 187 | 189 | return [] |
| 188 | 190 | |
| 189 | 191 | hunks: list[StructuredPatchHunk] = [] |
| 190 | - for item in value: | |
| 192 | + for item in patch_items: | |
| 191 | 193 | if isinstance(item, StructuredPatchHunk): |
| 192 | 194 | hunks.append(item) |
| 193 | 195 | elif isinstance(item, dict): |
@@ -289,12 +291,13 @@ def _render_preview_summary(preview: FileMutationPreview) -> Text: | ||
| 289 | 291 | |
| 290 | 292 | stats = [] |
| 291 | 293 | if preview.added_lines: |
| 292 | - stats.append(("+%d" % preview.added_lines, "green")) | |
| 294 | + stats.append((f"+{preview.added_lines}", "green")) | |
| 293 | 295 | if preview.removed_lines: |
| 294 | - stats.append(("-%d" % preview.removed_lines, "red")) | |
| 296 | + stats.append((f"-{preview.removed_lines}", "red")) | |
| 295 | 297 | if preview.context_lines: |
| 296 | - stats.append(("%d context" % preview.context_lines, "dim")) | |
| 297 | - stats.append(("%d hunk%s" % (preview.hunk_count, "" if preview.hunk_count == 1 else "s"), "dim")) | |
| 298 | + stats.append((f"{preview.context_lines} context", "dim")) | |
| 299 | + hunk_suffix = "" if preview.hunk_count == 1 else "s" | |
| 300 | + stats.append((f"{preview.hunk_count} hunk{hunk_suffix}", "dim")) | |
| 298 | 301 | |
| 299 | 302 | text.append("\n") |
| 300 | 303 | for index, (label, style) in enumerate(stats): |
@@ -343,8 +346,8 @@ def _iter_rendered_patch_lines( | ||
| 343 | 346 | for hunk in hunks: |
| 344 | 347 | rendered.append( |
| 345 | 348 | Text( |
| 346 | - "@@ -%d,%d +%d,%d @@\n" | |
| 347 | - % (hunk.old_start, hunk.old_lines, hunk.new_start, hunk.new_lines), | |
| 349 | + f"@@ -{hunk.old_start},{hunk.old_lines} " | |
| 350 | + f"+{hunk.new_start},{hunk.new_lines} @@\n", | |
| 348 | 351 | style="dim", |
| 349 | 352 | ) |
| 350 | 353 | ) |
tests/test_bash_operator_surfaces.pymodified@@ -2,6 +2,7 @@ | ||
| 2 | 2 | |
| 3 | 3 | from __future__ import annotations |
| 4 | 4 | |
| 5 | +import json | |
| 5 | 6 | from types import SimpleNamespace |
| 6 | 7 | |
| 7 | 8 | import pytest |
@@ -276,6 +277,17 @@ def test_build_file_mutation_preview_accepts_replacement_block_hunks() -> None: | ||
| 276 | 277 | assert preview.structured_patch[0].lines[0].startswith("+ <svg") |
| 277 | 278 | |
| 278 | 279 | |
| 280 | +def test_build_file_mutation_preview_accepts_json_encoded_hunks() -> None: | |
| 281 | + tool_args = _replacement_block_patch_tool_args() | |
| 282 | + tool_args["hunks"] = json.dumps(tool_args["hunks"]) | |
| 283 | + | |
| 284 | + preview = build_file_mutation_preview("patch", tool_args=tool_args) | |
| 285 | + | |
| 286 | + assert preview is not None | |
| 287 | + assert preview.structured_patch[0].old_start == 42 | |
| 288 | + assert preview.structured_patch[0].new_lines == 5 | |
| 289 | + | |
| 290 | + | |
| 279 | 291 | def test_cli_print_tool_call_renders_bash_panel_without_truncating(monkeypatch: pytest.MonkeyPatch) -> None: |
| 280 | 292 | console = Console(record=True, width=120) |
| 281 | 293 | monkeypatch.setattr(cli_main_module, "console", console) |
tests/test_expanded_tools.pymodified@@ -38,6 +38,33 @@ async def test_patch_tool_applies_structured_hunks(temp_dir: Path) -> None: | ||
| 38 | 38 | assert result.metadata["structured_patch"] |
| 39 | 39 | |
| 40 | 40 | |
| 41 | +@pytest.mark.asyncio | |
| 42 | +async def test_patch_tool_accepts_json_encoded_structured_hunks( | |
| 43 | + temp_dir: Path, | |
| 44 | +) -> None: | |
| 45 | + target = temp_dir / "sample.txt" | |
| 46 | + target.write_text("alpha\nbeta\ngamma\n") | |
| 47 | + tool = PatchTool(workspace_root=temp_dir) | |
| 48 | + | |
| 49 | + result = await tool.execute( | |
| 50 | + file_path=str(target), | |
| 51 | + hunks=json.dumps( | |
| 52 | + [ | |
| 53 | + { | |
| 54 | + "old_start": 2, | |
| 55 | + "old_lines": 1, | |
| 56 | + "new_start": 2, | |
| 57 | + "new_lines": 1, | |
| 58 | + "lines": ["-beta", "+beta from json string"], | |
| 59 | + } | |
| 60 | + ] | |
| 61 | + ), | |
| 62 | + ) | |
| 63 | + | |
| 64 | + assert result.is_error is False | |
| 65 | + assert target.read_text() == "alpha\nbeta from json string\ngamma\n" | |
| 66 | + | |
| 67 | + | |
| 41 | 68 | @pytest.mark.asyncio |
| 42 | 69 | async def test_patch_tool_rejects_context_mismatch(temp_dir: Path) -> None: |
| 43 | 70 | target = temp_dir / "sample.txt" |
tests/test_safeguard_services.pymodified@@ -2,6 +2,7 @@ | ||
| 2 | 2 | |
| 3 | 3 | from __future__ import annotations |
| 4 | 4 | |
| 5 | +import json | |
| 5 | 6 | import tempfile |
| 6 | 7 | from pathlib import Path |
| 7 | 8 | |
@@ -353,6 +354,30 @@ def test_pre_action_validator_allows_patch_string_without_hunks() -> None: | ||
| 353 | 354 | assert result == ValidationResult(valid=True) |
| 354 | 355 | |
| 355 | 356 | |
| 357 | +def test_pre_action_validator_allows_json_encoded_patch_hunks() -> None: | |
| 358 | + validator = PreActionValidator() | |
| 359 | + | |
| 360 | + result = validator.validate( | |
| 361 | + "patch", | |
| 362 | + { | |
| 363 | + "file_path": "notes.txt", | |
| 364 | + "hunks": json.dumps( | |
| 365 | + [ | |
| 366 | + { | |
| 367 | + "old_start": 1, | |
| 368 | + "old_lines": 1, | |
| 369 | + "new_start": 1, | |
| 370 | + "new_lines": 1, | |
| 371 | + "lines": ["-old", "+new"], | |
| 372 | + } | |
| 373 | + ] | |
| 374 | + ), | |
| 375 | + }, | |
| 376 | + ) | |
| 377 | + | |
| 378 | + assert result == ValidationResult(valid=True) | |
| 379 | + | |
| 380 | + | |
| 356 | 381 | def test_pre_action_validator_blocks_placeholder_html_write(tmp_path: Path) -> None: |
| 357 | 382 | validator = PreActionValidator() |
| 358 | 383 | |