From d8afe6473864532956f1c2ca3133eafd47e26be6 Mon Sep 17 00:00:00 2001 From: MAJDOUB Khalid Date: Wed, 10 Jun 2026 00:10:59 +0200 Subject: [PATCH 1/3] fix: collect worker changed paths --- src/forge_loop/sandbox/__init__.py | 2 + src/forge_loop/sandbox/changed_paths.py | 56 +++++++++++++++ tests/test_sandbox_changed_paths.py | 93 +++++++++++++++++++++++++ 3 files changed, 151 insertions(+) create mode 100644 src/forge_loop/sandbox/changed_paths.py create mode 100644 tests/test_sandbox_changed_paths.py diff --git a/src/forge_loop/sandbox/__init__.py b/src/forge_loop/sandbox/__init__.py index c5d987e..1a7e2aa 100644 --- a/src/forge_loop/sandbox/__init__.py +++ b/src/forge_loop/sandbox/__init__.py @@ -1,5 +1,6 @@ """Sandbox and capability policy contracts.""" +from forge_loop.sandbox.changed_paths import worker_changed_paths from forge_loop.sandbox.policy import ( CapabilityPolicy, FilesystemScope, @@ -21,5 +22,6 @@ "mcp_allow_patterns", "policy_hash", "render_capability_policy", + "worker_changed_paths", "write_root_violations", ] diff --git a/src/forge_loop/sandbox/changed_paths.py b/src/forge_loop/sandbox/changed_paths.py new file mode 100644 index 0000000..79e84a2 --- /dev/null +++ b/src/forge_loop/sandbox/changed_paths.py @@ -0,0 +1,56 @@ +"""Collect a worker worktree's changed files from git output.""" + +from __future__ import annotations + +import logging +import os +from collections.abc import Callable + +logger = logging.getLogger(__name__) + +GitOutputRunner = Callable[[tuple[str, ...], str], str] + + +def _changed_path(worktree_path: str, name: str) -> str: + return os.path.normpath(os.path.abspath(os.path.join(worktree_path, name.strip()))) + + +def worker_changed_paths( + run_git: GitOutputRunner, + worktree_path: str, + *, + base_ref: str, +) -> tuple[str, ...]: + """Absolute normalized paths changed in ``worktree_path`` relative to ``base_ref``. + + Uses injected git execution only: tracked modifications come from + ``git diff --name-only `` and untracked, non-ignored files from + ``git ls-files --others --exclude-standard``. Any git failure returns an + empty tuple so collection cannot abort the runner tick. + """ + cwd = os.fspath(worktree_path) + try: + diff = run_git(("git", "diff", "--name-only", base_ref), cwd) + others = run_git(("git", "ls-files", "--others", "--exclude-standard"), cwd) + except Exception as exc: # noqa: BLE001 — best-effort diff collection, never crash the tick + logger.warning( + "worker changed-path collection failed for worktree=%s base_ref=%s: %s", + cwd, + base_ref, + exc, + exc_info=True, + ) + return () + + seen: set[str] = set() + changed: list[str] = [] + for output in (diff, others): + for name in output.splitlines(): + if not name.strip(): + continue + path = _changed_path(cwd, name) + if path in seen: + continue + seen.add(path) + changed.append(path) + return tuple(changed) diff --git a/tests/test_sandbox_changed_paths.py b/tests/test_sandbox_changed_paths.py new file mode 100644 index 0000000..9a18146 --- /dev/null +++ b/tests/test_sandbox_changed_paths.py @@ -0,0 +1,93 @@ +"""Unit tests for worker changed-path collection from git output (issue #442).""" + +from __future__ import annotations + +import logging +import os +from pathlib import Path + +import pytest + +from forge_loop.sandbox.changed_paths import worker_changed_paths + + +def test_worker_changed_paths_is_exported_from_sandbox_package() -> None: + from forge_loop.sandbox import worker_changed_paths as exported + + assert exported is worker_changed_paths + + +class FakeGit: + def __init__( + self, + *, + diff: str = "", + others: str = "", + raise_on: tuple[str, ...] = (), + ) -> None: + self.calls: list[tuple[tuple[str, ...], str]] = [] + self._outputs = {"diff": diff, "ls-files": others} + self._raise_on = set(raise_on) + + def __call__(self, argv: tuple[str, ...], cwd: str) -> str: + self.calls.append((argv, cwd)) + command = argv[1] + if command in self._raise_on: + raise RuntimeError(f"{command} failed") + return self._outputs[command] + + +def _abs(worktree: Path, *names: str) -> tuple[str, ...]: + return tuple(os.path.normpath(os.path.abspath(str(worktree / name))) for name in names) + + +@pytest.mark.parametrize( + ("diff", "others", "expected"), + [ + ("src/a.py\nsrc/b.py\n", "new.txt\n", ("src/a.py", "src/b.py", "new.txt")), + ( + "src/a.py\nshared.txt\n", + "shared.txt\nnew.txt\n", + ("src/a.py", "shared.txt", "new.txt"), + ), + ("./src/../src/a.py\nsub/./x\n", "sub/../new.txt\n", ("src/a.py", "sub/x", "new.txt")), + ], +) +def test_worker_changed_paths_collects_deduplicates_and_normalizes( + tmp_path: Path, diff: str, others: str, expected: tuple[str, ...] +) -> None: + git = FakeGit(diff=diff, others=others) + + assert worker_changed_paths(git, str(tmp_path), base_ref="origin/trunk") == _abs( + tmp_path, *expected + ) + + +def test_worker_changed_paths_empty_output_is_empty_tuple(tmp_path: Path) -> None: + git = FakeGit(diff="\n \n", others="") + + assert worker_changed_paths(git, str(tmp_path), base_ref="origin/trunk") == () + + +@pytest.mark.parametrize(("raise_on", "diff"), [("diff", ""), ("ls-files", "src/a.py\n")]) +def test_worker_changed_paths_returns_empty_and_logs_when_git_raises( + tmp_path: Path, caplog: pytest.LogCaptureFixture, raise_on: str, diff: str +) -> None: + git = FakeGit(diff=diff, raise_on=(raise_on,)) + + caplog.set_level(logging.WARNING, logger="forge_loop.sandbox.changed_paths") + + assert worker_changed_paths(git, str(tmp_path), base_ref="origin/trunk") == () + assert "worktree=" in caplog.text + assert "base_ref=origin/trunk" in caplog.text + + +def test_worker_changed_paths_invokes_expected_git_commands(tmp_path: Path) -> None: + git = FakeGit(diff="", others="") + + worker_changed_paths(git, str(tmp_path), base_ref="origin/base") + + assert git.calls == [ + (("git", "diff", "--name-only", "origin/base"), str(tmp_path)), + (("git", "ls-files", "--others", "--exclude-standard"), str(tmp_path)), + ] From f08fb1a33c2c04c219db24707eb802843d806916 Mon Sep 17 00:00:00 2001 From: MAJDOUB Khalid Date: Wed, 10 Jun 2026 00:16:12 +0200 Subject: [PATCH 2/3] fix: verify worker changed paths (#442) From 103c386bc3a8c2ace4072f8b2c5380ecc45bd3f9 Mon Sep 17 00:00:00 2001 From: MAJDOUB Khalid Date: Wed, 10 Jun 2026 00:24:47 +0200 Subject: [PATCH 3/3] fix: preserve spaced worker changed paths (#442) --- src/forge_loop/sandbox/changed_paths.py | 2 +- tests/test_sandbox_changed_paths.py | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/forge_loop/sandbox/changed_paths.py b/src/forge_loop/sandbox/changed_paths.py index 79e84a2..57aa5c5 100644 --- a/src/forge_loop/sandbox/changed_paths.py +++ b/src/forge_loop/sandbox/changed_paths.py @@ -12,7 +12,7 @@ def _changed_path(worktree_path: str, name: str) -> str: - return os.path.normpath(os.path.abspath(os.path.join(worktree_path, name.strip()))) + return os.path.normpath(os.path.abspath(os.path.join(worktree_path, name))) def worker_changed_paths( diff --git a/tests/test_sandbox_changed_paths.py b/tests/test_sandbox_changed_paths.py index 9a18146..e89852c 100644 --- a/tests/test_sandbox_changed_paths.py +++ b/tests/test_sandbox_changed_paths.py @@ -69,6 +69,14 @@ def test_worker_changed_paths_empty_output_is_empty_tuple(tmp_path: Path) -> Non assert worker_changed_paths(git, str(tmp_path), base_ref="origin/trunk") == () +def test_worker_changed_paths_preserves_significant_path_spaces(tmp_path: Path) -> None: + git = FakeGit(diff=" allowed/escape.py\nallowed/ok.py \n") + + assert worker_changed_paths(git, str(tmp_path), base_ref="origin/trunk") == _abs( + tmp_path, " allowed/escape.py", "allowed/ok.py " + ) + + @pytest.mark.parametrize(("raise_on", "diff"), [("diff", ""), ("ls-files", "src/a.py\n")]) def test_worker_changed_paths_returns_empty_and_logs_when_git_raises( tmp_path: Path, caplog: pytest.LogCaptureFixture, raise_on: str, diff: str