From e02bac110d7b8061bbc9ebe04f6a08dd7770ce68 Mon Sep 17 00:00:00 2001 From: Don Garrett Date: Fri, 2 Oct 2026 22:38:13 +0000 Subject: [PATCH] Publish only the work: leave build artifacts out and report them (#80) The clone gets the standard build artifacts in its own exclude file, so they never show as changes, even to a model's `git add -A`. The harness's commits leave the same patterns out even when the project tracks them, as the sandbox's main did. What was left out is reported in the publish note and the PR description. Found again in bring-up run 3 (sandbox PR #208). Fixes #80. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01PPQ14L3LikyefCMFBqEbKE --- nisse-dev/src/nisse_dev/agent.py | 1 + nisse-dev/src/nisse_dev/git.py | 66 ++++++++++++++++++++++++++++-- nisse-dev/src/nisse_dev/publish.py | 36 ++++++++++++++-- nisse-dev/tests/test_git.py | 49 ++++++++++++++++++++++ nisse-dev/tests/test_publish.py | 20 +++++++++ 5 files changed, 164 insertions(+), 8 deletions(-) diff --git a/nisse-dev/src/nisse_dev/agent.py b/nisse-dev/src/nisse_dev/agent.py index 995cc87..935ba72 100644 --- a/nisse-dev/src/nisse_dev/agent.py +++ b/nisse-dev/src/nisse_dev/agent.py @@ -217,6 +217,7 @@ def prepare_workspace( if not repo.exists(): repo.clone(remote, credential=credential) repo.configure_identity(account, f"{account}@users.noreply.local") + repo.install_excludes() # build artifacts are never the work (#80) return repo diff --git a/nisse-dev/src/nisse_dev/git.py b/nisse-dev/src/nisse_dev/git.py index 4df2292..56f5596 100644 --- a/nisse-dev/src/nisse_dev/git.py +++ b/nisse-dev/src/nisse_dev/git.py @@ -33,6 +33,35 @@ class GitError(RuntimeError): pass +# Build artifacts and caches that are never the work (D4, #80). Kept out of +# every commit the harness makes, tracked or not, and excluded in the clone +# so they don't even show as changes. Conservative on purpose: a pattern here +# must never match a project's real source. +ARTIFACT_PATTERNS = ( + "__pycache__/", + "*.py[cod]", + ".pytest_cache/", + ".mypy_cache/", + ".ruff_cache/", + ".tox/", + ".nox/", + "*.egg-info/", + ".coverage", + "htmlcov/", + ".venv/", + "node_modules/", + ".DS_Store", +) +_EXCLUDES_MARK = "# nisse-dev: build artifacts are never the work (D4)" + + +def _pathspec(pattern: str) -> str: + """A pathspec leaving ``pattern`` out wherever it occurs in the tree.""" + glob = pattern.rstrip("/") + tail = "/**" if pattern.endswith("/") else "" + return f":(exclude,glob)**/{glob}{tail}" + + @dataclass(frozen=True) class HTTPSToken: """A push-only account's token, for HTTPS remotes (D4).""" @@ -84,6 +113,7 @@ class GitRepo: *args: str, credential: PushCredential | None = None, cwd: str | None = None, + raw: bool = False, ) -> str: quoted = " ".join(shlex.quote(a) for a in args) command, stdin, secrets = f"git {quoted}", None, {} @@ -117,7 +147,8 @@ class GitRepo: f"git {args[0]} failed: " f"{result.stderr.strip() or result.stdout.strip()}" ) - return result.stdout.strip() + # Raw for output whose leading whitespace means something (porcelain). + return result.stdout if raw else result.stdout.strip() # -- inspection ------------------------------------------------------- @@ -191,15 +222,42 @@ class GitRepo: return False return True + def install_excludes(self) -> None: + """List the build artifacts in the clone's own exclude file, so they + never appear as changes. Local to the clone: nothing in the project + changes. Idempotent.""" + path = shlex.quote(str(self.root / ".git" / "info" / "exclude")) + block = "\n".join([_EXCLUDES_MARK, *ARTIFACT_PATTERNS]) + result = self.host.execute( + f'mkdir -p -- "$(dirname -- {path})" && touch -- {path} && ' + f"grep -qxF {shlex.quote(_EXCLUDES_MARK)} {path} || " + f"cat >> {path}", + stdin=block, + timeout_s=TIMEOUT_S, + ) + if not result.ok: + raise GitError(f"could not set excludes: {result.stderr.strip()}") + def commit_all(self, message: str) -> str | None: - """Returns the new commit sha, or None when there was nothing to - commit — an empty milestone is a fact to report, not an error.""" + """Commit the work: every change except build artifacts, which are + left out even when the project tracks them (#80). Returns the new + commit sha, or None when there was nothing to commit — an empty + milestone is a fact to report, not an error.""" if not self.has_changes(): return None - self._git("add", "-A") + self._git("add", "-A", "--", ".", *(_pathspec(p) for p in ARTIFACT_PATTERNS)) + staged = self._git("diff", "--cached", "--name-only") + if not staged: + return None self._git("commit", "-q", "-m", message) return self.head_sha() + def left_out(self) -> list[str]: + """Changes not in the last commit: build artifacts the harness left + out, to be reported rather than silently dropped (D4).""" + out = self._git("status", "--porcelain", "--untracked-files=all", raw=True) + return sorted(line[3:] for line in out.splitlines() if line) + def push( self, remote_url: str, branch: str, *, credential: PushCredential | None = None ) -> None: diff --git a/nisse-dev/src/nisse_dev/publish.py b/nisse-dev/src/nisse_dev/publish.py index a23c3ee..219eb50 100644 --- a/nisse-dev/src/nisse_dev/publish.py +++ b/nisse-dev/src/nisse_dev/publish.py @@ -7,6 +7,7 @@ PR exists rather than repeating the push-and-open — the spec's own example of an idempotent transition reconciled against external truth. """ +from collections.abc import Sequence from dataclasses import dataclass from nisse_dev.git import GitRepo, PushCredential @@ -80,6 +81,8 @@ class MilestonePublisher: return PublishResult(None, "no changes to publish") # The executor committed its work itself; publish that. sha = self._git.head_sha() + # D4: what was left out (build artifacts) is reported, not dropped. + left_out = self._git.left_out() intent_id = self._store.begin_intent( OPEN_PR_INTENT, @@ -99,17 +102,19 @@ class MilestonePublisher: return PublishResult( existing, f"PR #{existing.number} was already open for this milestone; " - f"pushed to it: {existing.url}", + f"pushed to it: {existing.url}" + _left_out_note(left_out), ) pr = self._client.open_pull_request( self._repo, head=branch, base=self._base_branch, title=f"Milestone {milestone}: {project}", - body=_pr_body(milestone, plan, service_url), + body=_pr_body(milestone, plan, service_url, left_out), ) self._store.commit_intent(intent_id, cause=f"opened PR #{pr.number}") - return PublishResult(pr, f"opened PR #{pr.number}: {pr.url}") + return PublishResult( + pr, f"opened PR #{pr.number}: {pr.url}" + _left_out_note(left_out) + ) def published_pull_request( self, task_id: str, milestone: int @@ -161,7 +166,21 @@ class MilestonePublisher: return sha -def _pr_body(milestone: int, plan: str, service_url: str | None) -> str: +def _left_out_note(paths: list[str]) -> str: + if not paths: + return "" + shown = ", ".join(paths[:5]) + ( + f" and {len(paths) - 5} more" if len(paths) > 5 else "" + ) + return f" (left out {len(paths)} build artifact(s): {shown})" + + +def _pr_body( + milestone: int, + plan: str, + service_url: str | None, + left_out: Sequence[str] = (), +) -> str: parts = [ f"Milestone {milestone}, implemented by nisse-dev against the approved plan.", "", @@ -174,4 +193,13 @@ def _pr_body(milestone: int, plan: str, service_url: str | None) -> str: # so a reviewer can exercise it. Nothing reports one yet; the browser # tool (D7) is what will. parts += ["", "## Running test service", "", service_url] + if left_out: + parts += [ + "", + "## Left out", + "", + "Build artifacts in the workspace, never part of the work:", + "", + *(f"- `{path}`" for path in left_out), + ] return "\n".join(parts) diff --git a/nisse-dev/tests/test_git.py b/nisse-dev/tests/test_git.py index a2b8e5e..369b23a 100644 --- a/nisse-dev/tests/test_git.py +++ b/nisse-dev/tests/test_git.py @@ -195,3 +195,52 @@ def test_an_https_token_answers_git_through_the_helper( assert asked.stdout == "username=[redacted]\npassword=[redacted]\n" stored = ssh_host.execute(f"{body} store", secrets={"NISSE_PUSH_TOKEN": "x"}) assert stored.stdout == "" + + +# --- build artifacts are never the work (#80, D4) ---------------------------- + + +def test_artifacts_are_left_out_even_when_the_project_tracks_them( + repo: GitRepo, +) -> None: + # The sandbox's main tracks .pyc files (an earlier run published them). + cache = repo.root / "pkg" / "__pycache__" + cache.mkdir(parents=True) + (repo.root / "pkg" / "m.py").write_text("x = 1\n") + (cache / "m.cpython-312.pyc").write_bytes(b"old") + repo._git("add", "-A") + repo._git("commit", "-qm", "a project that tracks its caches") + + # A test run: the tracked .pyc changes, new caches appear, and the work. + (cache / "m.cpython-312.pyc").write_bytes(b"new") + (cache / "m.cpython-310.pyc").write_bytes(b"x") + (repo.root / ".pytest_cache").mkdir() + (repo.root / ".pytest_cache" / "v").write_text("x") + (repo.root / "pkg" / "m.py").write_text("x = 2\n") + + assert repo.commit_all("the work") is not None + committed = repo._git("show", "--name-only", "--format=", "HEAD").split() + assert committed == ["pkg/m.py"] + assert repo.left_out() == [ + ".pytest_cache/v", + "pkg/__pycache__/m.cpython-310.pyc", + "pkg/__pycache__/m.cpython-312.pyc", + ] + + +def test_only_artifacts_changed_means_nothing_to_commit(repo: GitRepo) -> None: + (repo.root / "__pycache__").mkdir() + (repo.root / "__pycache__" / "a.pyc").write_bytes(b"x") + assert repo.commit_all("nothing real") is None + + +def test_excludes_hide_new_artifacts_and_install_once(repo: GitRepo) -> None: + repo.install_excludes() + repo.install_excludes() + exclude = (repo.root / ".git" / "info" / "exclude").read_text() + assert exclude.count("__pycache__/") == 1 + (repo.root / "node_modules").mkdir() + (repo.root / "node_modules" / "x.js").write_text("x") + (repo.root / "real.py").write_text("x") + # Even the model's own `git add -A` now skips them. + assert repo._git("status", "--porcelain", "--untracked-files=all") == "?? real.py" diff --git a/nisse-dev/tests/test_publish.py b/nisse-dev/tests/test_publish.py index 5e744d2..234158d 100644 --- a/nisse-dev/tests/test_publish.py +++ b/nisse-dev/tests/test_publish.py @@ -90,6 +90,9 @@ class FakeGit: def commit_all(self, message: str) -> str | None: return self.repo.commit_all(message) + def left_out(self) -> list[str]: + return self.repo.left_out() + def push(self, remote_url: str, branch: str, *, credential: object = None) -> None: self.pushed.append(branch) self.remote[branch] = self.repo.head_sha() @@ -147,6 +150,23 @@ def test_a_branch_named_after_the_task_does_not_block_publishing( assert git.pushed == ["demo-abc-milestone-1"] +def test_left_out_artifacts_are_reported_not_published( + store: StateStore, tmp_path: Path +) -> None: + git = FakeGit(tmp_path / "work") + (tmp_path / "work" / "__pycache__").mkdir() + (tmp_path / "work" / "__pycache__" / "work.cpython-310.pyc").write_bytes(b"x") + client = FakeGitea() + result = build(store, git, client).publish( + task_id="t", milestone=1, project="demo", plan="p" + ) + assert "left out 1 build artifact(s): __pycache__/work.cpython-310.pyc" in ( + result.note + ) + assert "## Left out" in client.opened[0]["body"] + assert "`__pycache__/work.cpython-310.pyc`" in client.opened[0]["body"] + + def test_intent_is_recorded_then_committed(store: StateStore, tmp_path: Path) -> None: build(store, FakeGit(tmp_path / "work"), FakeGitea()).publish( task_id="t", milestone=1, project="demo", plan="p"