From cbc021d98310fe2087b30e11959941d741accc38 Mon Sep 17 00:00:00 2001 From: kmajdoub Date: Thu, 11 Jun 2026 01:35:44 +0200 Subject: [PATCH] fix(worker): shield planted .claude/settings.json from worker commits (#449) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A getadaptiq worker committed the planted policy file over the target repo's TRACKED .claude/settings.json — the operator's hooks and permissions would have been destroyed on merge (caught at review, PR salvaged). Root cause: plant_worker_settings overwrites the path, and nothing kept the resulting diff out of the worker's add -A. Shield at the git level so no worker cooperation is needed: - tracked file → git update-index --skip-worktree (overwrite invisible to status/diff/add) - untracked → append to info/exclude (worktree-local, resolved via --git-path so linked worktrees hit the right file) Best-effort on non-git dirs (unit fixtures) — the plant still applies. 2 new tests pin both cases end-to-end (real git repos, worker-style add -A stages nothing). 61/61 in worker suites; full suite green (known-bad test_critic_sdk excluded, pre-existing on trunk). Closes #449. Co-Authored-By: Claude Fable 5 --- src/forge_loop/worker_worktree.py | 47 ++++++++++++++++++++++++ tests/test_worker_worktree.py | 61 +++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/src/forge_loop/worker_worktree.py b/src/forge_loop/worker_worktree.py index 96e0028..6fbff37 100644 --- a/src/forge_loop/worker_worktree.py +++ b/src/forge_loop/worker_worktree.py @@ -166,9 +166,56 @@ def plant_worker_settings( settings_path.write_text(render_worker_settings(policy)) settings_path.chmod(0o444) cdir.chmod(0o555) + _shield_plant_from_git(worktree) _emit_worker_policy_event(events_file, worktree, policy) +_PLANT_REL = ".claude/settings.json" + + +def _shield_plant_from_git(worktree: Path) -> None: + """Keep the planted settings out of any worker commit (#449). + + A worker committed the plant over the target repo's TRACKED + ``.claude/settings.json`` — the operator's hooks and permissions + would have been destroyed on merge. Shield at the git level so no + worker cooperation is needed: + + * tracked file → ``git update-index --skip-worktree`` (the + overwrite never shows in status/diff/add -A) + * untracked → append to ``info/exclude`` (worktree-local, + never committed; resolved via ``--git-path`` so linked + worktrees hit the right file) + + Best-effort: a non-git target_dir (unit fixtures, bare staging + dirs) silently skips — the plant itself still applies.""" + git = lambda *a: subprocess.run( # noqa: E731 + ["git", *a], cwd=worktree, capture_output=True, text=True, + timeout=15, check=False, + ) + tracked = git("ls-files", "--error-unmatch", _PLANT_REL) + if tracked.returncode == 0: + git("update-index", "--skip-worktree", _PLANT_REL) + return + git_path = git("rev-parse", "--git-path", "info/exclude") + if git_path.returncode != 0: + return + exclude = Path(git_path.stdout.strip()) + if not exclude.is_absolute(): + exclude = worktree / exclude + try: + existing = exclude.read_text() if exclude.exists() else "" + if _PLANT_REL not in existing: + exclude.parent.mkdir(parents=True, exist_ok=True) + exclude.write_text( + existing + + ("" if existing.endswith("\n") or not existing else "\n") + + f"{_PLANT_REL}\n" + ) + except OSError: + return + + def _emit_worker_policy_event( events_file: Path | None, worktree: Path, diff --git a/tests/test_worker_worktree.py b/tests/test_worker_worktree.py index 7323281..57cf8bd 100644 --- a/tests/test_worker_worktree.py +++ b/tests/test_worker_worktree.py @@ -2,6 +2,8 @@ from __future__ import annotations +import subprocess + import json from pathlib import Path from typing import Any @@ -398,3 +400,62 @@ def test_plant_worker_settings_none_policy_withholds_all_secrets( lines = [json.loads(line) for line in events_file.read_text().splitlines() if line.strip()] enforced = [rec for rec in lines if rec.get("kind") == "worker_policy_enforced"] assert enforced[0]["withheld_secrets"] == ["DB_PASSWORD", "GITHUB_TOKEN"] + + +class TestPlantShieldedFromGit: + """#449 — the planted settings file must never reach a worker commit. + + A getadaptiq worker committed the plant over the operator's TRACKED + .claude/settings.json (hooks + permissions destroyed on merge). The + plant now shields the path at the git level, so even `git add -A` + inside the worker session stages nothing for it.""" + + def _repo(self, tmp_path): + repo = tmp_path / "repo" + repo.mkdir() + run = lambda *a: subprocess.run( # noqa: E731 + ["git", *a], cwd=repo, capture_output=True, text=True, check=True + ) + run("init", "-b", "main") + run("config", "user.email", "t@t") + run("config", "user.name", "t") + return repo, run + + def test_tracked_settings_overwrite_invisible_to_git(self, tmp_path): + repo, run = self._repo(tmp_path) + cdir = repo / ".claude" + cdir.mkdir() + (cdir / "settings.json").write_text('{"operator": "hooks"}') + (repo / "README.md").write_text("x") + run("add", "-A") + run("commit", "-m", "operator config") + + plant_worker_settings(repo, None) + + run2 = subprocess.run( + ["git", "status", "--porcelain", "--untracked-files=all"], + cwd=repo, capture_output=True, text=True, check=True, + ) + assert ".claude/settings.json" not in run2.stdout + # and a worker-style add-all stages nothing for it + subprocess.run(["git", "add", "-A"], cwd=repo, check=True) + staged = subprocess.run( + ["git", "diff", "--cached", "--name-only"], + cwd=repo, capture_output=True, text=True, check=True, + ) + assert ".claude/settings.json" not in staged.stdout + + def test_untracked_plant_excluded_from_add_all(self, tmp_path): + repo, run = self._repo(tmp_path) + (repo / "README.md").write_text("x") + run("add", "-A") + run("commit", "-m", "init") + + plant_worker_settings(repo, None) + + subprocess.run(["git", "add", "-A"], cwd=repo, check=True) + staged = subprocess.run( + ["git", "diff", "--cached", "--name-only"], + cwd=repo, capture_output=True, text=True, check=True, + ) + assert ".claude/settings.json" not in staged.stdout