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"