From a97f82db2832d489dc555c823cc7e8a0f7cd7ca0 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Mon, 2 Nov 2026 18:22:40 +0530 Subject: [PATCH] fix(work): pin the format of patches the Host parses collect_git_delta and collect_diff run plain `git diff`, so the user's diff configuration decides the shape of the patch that parse_unified_diff renders in the Work attempt diff canvas and that the Work page shows as raw text: - diff.mnemonicPrefix=true turns `git diff HEAD` prefixes into c/ and w/, so the canvas names the file "w/app.py" and lists the real path a second time as a hunk-less "modified" entry; - color.diff=always wraps every line in ANSI escapes, so no header or hunk is recognised and every file shows 0 additions and 0 deletions; - diff.noprefix=true drops the a/ and b/ prefixes; - diff.external replaced collect_diff's patch, which, unlike the attempt delta, did not pass --no-ext-diff. Add PATCH_FORMAT_ARGS (--no-ext-diff, --no-color, --src-prefix=a/, --dst-prefix=b/) to server/local_git.py and use it for the committed and working-tree attempt patches and for collect_diff's patch. These options override the configuration for one command only, so the user's Git setup is left untouched. Fixes #279 --- server/local_git.py | 7 +++- server/work_artifact_registry.py | 6 +-- tests/test_local_git_provider_diff.py | 22 +++++++++++ tests/test_work_artifact_registry.py | 54 +++++++++++++++++++++++++++ 4 files changed, 85 insertions(+), 4 deletions(-) diff --git a/server/local_git.py b/server/local_git.py index cf1e6afc..3a34a1b8 100644 --- a/server/local_git.py +++ b/server/local_git.py @@ -8,6 +8,11 @@ CREATE_NO_WINDOW = 0x08000000 +# Patches are parsed and displayed by Amadeus, so their format must not follow +# the user's diff configuration: an external diff driver, forced color, or +# diff.noprefix/diff.mnemonicPrefix would change what the parser receives. +PATCH_FORMAT_ARGS = ("--no-ext-diff", "--no-color", "--src-prefix=a/", "--dst-prefix=b/") + async def run_git(cwd: str, args: list[str]) -> dict[str, Any]: path = Path(cwd).resolve() @@ -42,7 +47,7 @@ async def collect_diff(cwd: str, *, include_patch: bool = True) -> dict[str, Any head_probe = await run_git(resolved, ["rev-parse", "--verify", "HEAD"]) has_head = head_probe["returncode"] == 0 if include_patch: - diff_args = ["diff", "HEAD", "--"] if has_head else ["diff", "--"] + diff_args = ["diff", *PATCH_FORMAT_ARGS, "HEAD", "--"] if has_head else ["diff", *PATCH_FORMAT_ARGS, "--"] else: diff_args = ["diff", "--name-only", "HEAD", "--"] if has_head else ["diff", "--name-only", "--"] diff = await run_git(resolved, diff_args) diff --git a/server/work_artifact_registry.py b/server/work_artifact_registry.py index 6c699680..b7c998bf 100644 --- a/server/work_artifact_registry.py +++ b/server/work_artifact_registry.py @@ -11,7 +11,7 @@ from agent_host.work_ledger_store import WorkLedgerStore from agent_host.work_ledger_types import RunAttemptRecord, WorkItemRecord -from server.local_git import collect_diff, run_git +from server.local_git import PATCH_FORMAT_ARGS, collect_diff, run_git _HASH_LIMIT_BYTES = 64 * 1024 * 1024 @@ -421,13 +421,13 @@ async def collect_git_delta( if baseline_head and current_head and baseline_head != current_head: result = await run_git( repo_root, - ["diff", "--no-ext-diff", "--unified=3", baseline_head, current_head, "--", *changed_files], + ["diff", *PATCH_FORMAT_ARGS, "--unified=3", baseline_head, current_head, "--", *changed_files], ) if result["returncode"] == 0: committed_patch = result["stdout"] result = await run_git( repo_root, - ["diff", "--no-ext-diff", "--unified=3", current_head or "HEAD", "--", *changed_files], + ["diff", *PATCH_FORMAT_ARGS, "--unified=3", current_head or "HEAD", "--", *changed_files], ) if result["returncode"] == 0: working_patch = result["stdout"] diff --git a/tests/test_local_git_provider_diff.py b/tests/test_local_git_provider_diff.py index 156e51c6..88c04cc3 100644 --- a/tests/test_local_git_provider_diff.py +++ b/tests/test_local_git_provider_diff.py @@ -86,6 +86,27 @@ async def run() -> None: asyncio.run(run()) +def test_provider_diff_patch_ignores_user_diff_format_config() -> None: + async def run() -> None: + with _repo() as repo: + tracked = repo / "tracked.txt" + tracked.write_text("one\n", encoding="utf-8") + _git(repo, "add", "tracked.txt") + _git(repo, "commit", "-m", "initial") + _git(repo, "config", "diff.mnemonicPrefix", "true") + _git(repo, "config", "color.diff", "always") + tracked.write_text("two\n", encoding="utf-8") + + res = await _provider_diff(repo) + + patch = res["diff"]["patch"] + assert "\x1b[" not in patch + assert "diff --git a/tracked.txt b/tracked.txt" in patch + assert "--- a/tracked.txt\n+++ b/tracked.txt" in patch + + asyncio.run(run()) + + def test_provider_diff_handles_unborn_head() -> None: async def run() -> None: with _repo() as repo: @@ -201,6 +222,7 @@ async def run() -> None: def _main() -> None: test_provider_diff_uses_head_and_lists_untracked() test_provider_diff_preserves_non_ascii_and_spaced_paths() + test_provider_diff_patch_ignores_user_diff_format_config() test_provider_diff_handles_unborn_head() test_terminal_run_diff_uses_persisted_attempt_delta_not_current_cwd() test_codex_attempt_uses_provider_neutral_persisted_diff() diff --git a/tests/test_work_artifact_registry.py b/tests/test_work_artifact_registry.py index 03671668..d7fded40 100644 --- a/tests/test_work_artifact_registry.py +++ b/tests/test_work_artifact_registry.py @@ -15,6 +15,7 @@ sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) from agent_host.work_ledger_store import WorkLedgerStore +from server.unified_diff import parse_unified_diff from server.work_artifact_registry import WorkArtifactRegistry @@ -254,6 +255,59 @@ async def run() -> None: asyncio.run(run()) +@pytest.mark.parametrize( + ("key", "value"), + [ + ("diff.mnemonicPrefix", "true"), + ("diff.noprefix", "true"), + ("color.diff", "always"), + ], +) +def test_attempt_patch_ignores_user_diff_format_config(key: str, value: str) -> None: + async def run() -> None: + with tempfile.TemporaryDirectory(prefix="work_artifact_diff_config_") as temp: + root = Path(temp) / "project" + root.mkdir() + _git(root, "init") + _git(root, "config", "user.email", "amadeus-test@example.invalid") + _git(root, "config", "user.name", "Amadeus Test") + (root / "committed.txt").write_text("base\n", encoding="utf-8") + (root / "working.txt").write_text("base\n", encoding="utf-8") + _git(root, "add", ".") + _git(root, "commit", "-m", "baseline") + _git(root, "config", key, value) + + store = WorkLedgerStore(Path(temp) / "ledger.sqlite3") + project = store.create_or_get_project(root) + item = store.create_work_item(project.project_id, title="Edit files") + attempt = store.create_attempt(item.work_item_id, provider="codex", task="Edit files") + registry = WorkArtifactRegistry(store) + await registry.capture_baseline(attempt, item) + (root / "committed.txt").write_text("committed\n", encoding="utf-8") + _git(root, "commit", "-am", "provider commit") + (root / "working.txt").write_text("working\n", encoding="utf-8") + + current_attempt = store.get_attempt(attempt.attempt_id) + assert current_attempt is not None + delta = await registry.finalize_attempt(current_attempt, item) + store.close() + + assert "diff --git a/committed.txt b/committed.txt" in delta["committed_patch"] + assert "diff --git a/working.txt b/working.txt" in delta["working_patch"] + assert "\x1b[" not in delta["patch"] + parsed = parse_unified_diff( + delta["patch"], + changed_files=delta["changed_files"], + untracked=delta["untracked"], + ) + assert [(entry["path"], entry["additions"], entry["deletions"]) for entry in parsed["files"]] == [ + ("committed.txt", 1, 1), + ("working.txt", 1, 1), + ] + + asyncio.run(run()) + + def test_non_git_workspace_registers_new_files_without_claiming_preexisting_changes() -> None: async def run() -> None: with tempfile.TemporaryDirectory(prefix="work_artifact_filesystem_") as temp: