fix(lib): always hash build context; record dirty flag (Greptile)

Address Greptile review on the build-provenance capture util:

- Always compute working_tree_hash (drop the "skip on clean commit"
  path). A `git status` clean tree can still contain .gitignore'd-but-not-
  .dockerignore'd files the commit can't reproduce; an always-present
  content hash identifies the exact shipped bytes and closes that gap.
- Guard the hash (_safe_working_tree_hash) so a permission error or
  filesystem race degrades to None instead of aborting the build — the
  module contract is that capture never raises into a build.
- Record dirtiness as a first-class `dirty` flag (surfaced as `source_dirty`
  / `dirty`) rather than overloading hash-presence, matching Go's
  vcs.modified and Nix's dirtyRev. None outside a git work tree.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: max-parke-scale

src/agentex/lib/cli/handlers/agent_handlers.py

@@ -279,7 +279,7 @@ def prepare_cloud_build_context(
         (staged_root / _BUILD_INFO_FILENAME).write_text(json.dumps(provenance.build_info(), indent=2, sort_keys=True))
         logger.info(
             f"Build provenance: commit={provenance.commit} ref={provenance.ref} "
-            f"clean_commit={provenance.is_clean_commit}"
+            f"working_tree_hash={provenance.working_tree_hash}"
         )
 
         # Compress the prepared context using the static zipped method

src/agentex/lib/utils/build_provenance.py

@@ -28,40 +28,38 @@
 
 @dataclass(frozen=True)
 class BuildProvenance:
-    """Source identity for one build. All fields degrade to ``None``.
+    """Source identity for one build; every field degrades to ``None``.
 
-    Exactly one identity anchors the build: a **clean committed tree** keys on
-    ``commit`` (``working_tree_hash`` is ``None``); anything else — a dirty tree
-    or a non-git context, neither of which a commit can address — carries a
-    ``working_tree_hash`` instead. So a non-null hash means "no clean commit to
-    point to," and ``is_clean_commit`` is the gate ``--require-clean`` checks.
+    ``working_tree_hash`` is the deterministic content hash of the build context
+    and is always computed — it identifies the exact bytes that shipped,
+    independent of git state. ``commit`` (+ ``ref`` / ``repo``) anchor those bytes
+    to source, and ``dirty`` records whether the work tree had uncommitted changes
+    at build time (``None`` outside a git work tree).
     """
 
     repo: Optional[str] = None
     commit: Optional[str] = None
     ref: Optional[str] = None
     subpath: Optional[str] = None
     working_tree_hash: Optional[str] = None
+    dirty: Optional[bool] = None
     author_name: Optional[str] = None
     author_email: Optional[str] = None
     build_timestamp: Optional[str] = None
 
-    @property
-    def is_clean_commit(self) -> bool:
-        return self.commit is not None and self.working_tree_hash is None
-
-    def source_fields(self) -> dict[str, str]:
+    def source_fields(self) -> dict[str, object]:
         """The ``source_*`` form fields for the cloud-build upload (None omitted)."""
         fields = {
             "source_repo": self.repo,
             "source_commit": self.commit,
             "source_ref": self.ref,
             "source_subpath": self.subpath,
             "working_tree_hash": self.working_tree_hash,
+            "source_dirty": self.dirty,
         }
         return {key: value for key, value in fields.items() if value is not None}
 
-    def build_info(self) -> dict[str, str]:
+    def build_info(self) -> dict[str, object]:
         """The ``build-info.json`` payload (runtime ``registration_metadata``).
 
         Overlapping keys match the server's ``DeploymentHistory`` type
@@ -75,6 +73,7 @@ def build_info(self) -> dict[str, str]:
             "branch_name": self.ref,
             "subpath": self.subpath,
             "working_tree_hash": self.working_tree_hash,
+            "dirty": self.dirty,
             "author_name": self.author_name,
             "author_email": self.author_email,
             "build_timestamp": self.build_timestamp,
@@ -167,6 +166,19 @@ def working_tree_hash(root: Path) -> str:
     return hashlib.sha256("\n".join(lines).encode("utf-8")).hexdigest()
 
 
+def _safe_working_tree_hash(root: Path) -> Optional[str]:
+    """``working_tree_hash`` that degrades to None — capture must never fail a build.
+
+    A permission error or filesystem race during the walk/stat/read would otherwise
+    raise out of capture and abort the build before the archive is even created.
+    """
+    try:
+        return working_tree_hash(root)
+    except Exception:
+        logger.warning("build-provenance: content hash failed; omitting", exc_info=True)
+        return None
+
+
 def capture_build_provenance(
     repo_path: Path, context_root: Path, content_root: Optional[Path] = None
 ) -> BuildProvenance:
@@ -176,19 +188,18 @@ def capture_build_provenance(
     relative to the repo root (which agent, in a monorepo). ``content_root`` is
     the directory hashed — the *staged*, post-``.dockerignore`` tree that actually
     ships; it defaults to ``context_root`` when there is no separate staging dir.
-    The content hash is computed unless a clean commit identifies the build (so:
-    for a dirty tree or a non-git context, but not for a clean committed tree).
+    ``working_tree_hash`` is always computed; git coordinates anchor it to source
+    when available.
     """
     timestamp = datetime.now(timezone.utc).isoformat()
     hash_root = content_root if content_root is not None else context_root
+    tree_hash = _safe_working_tree_hash(hash_root)
+
     repo_root = _git(repo_path, "rev-parse", "--show-toplevel")
     if repo_root is None:
-        # No git at all — the content hash is the only identity available.
-        logger.info("build-provenance: %s is not a git work tree; hashing context", repo_path)
-        return BuildProvenance(
-            working_tree_hash=working_tree_hash(hash_root),
-            build_timestamp=timestamp,
-        )
+        # No git — the content hash is the only identity available.
+        logger.info("build-provenance: %s is not a git work tree; content hash only", repo_path)
+        return BuildProvenance(working_tree_hash=tree_hash, build_timestamp=timestamp)
 
     repo_root_path = Path(repo_root)
     commit = _git(repo_root_path, "rev-parse", "HEAD")
@@ -207,17 +218,16 @@ def capture_build_provenance(
     except ValueError:
         subpath = None
 
-    # Hash unless a clean commit identifies the build: dirty tree, or an unborn
-    # HEAD with no commit yet, both fall back to the content hash.
+    # `git status --porcelain` is empty (→ _git returns None) for a clean tree.
     dirty = _git(repo_root_path, "status", "--porcelain") is not None
-    tree_hash = working_tree_hash(hash_root) if (dirty or commit is None) else None
 
     return BuildProvenance(
         repo=remote,
         commit=commit,
         ref=ref,
         subpath=subpath,
         working_tree_hash=tree_hash,
+        dirty=dirty,
         author_name=author_name,
         author_email=author_email,
         build_timestamp=timestamp,

tests/lib/test_build_provenance.py

@@ -130,26 +130,13 @@ def test_capture_clean_tree(tmp_path: Path) -> None:
     assert prov.repo == "github.com/scaleapi/demo"
     assert prov.ref == "main"
     assert prov.commit is not None and len(prov.commit) == 40
-    assert prov.working_tree_hash is None
-    assert prov.is_clean_commit is True
+    assert prov.working_tree_hash is not None  # always computed
+    assert prov.dirty is False
     assert prov.subpath is None
     assert prov.author_email == "dev@scale.com"
 
 
-def test_capture_dirty_tracked_modification(tmp_path: Path) -> None:
-    repo = _init_repo(tmp_path / "repo")
-    _write(repo, "main.py", "print(1)")
-    _commit_all(repo)
-    _write(repo, "main.py", "print(2)")  # modify tracked, do not commit
-
-    prov = capture_build_provenance(repo, repo)
-
-    assert prov.is_clean_commit is False
-    assert prov.working_tree_hash is not None
-    assert prov.commit is not None  # commit still recorded alongside the hash
-
-
-def test_capture_dirty_untracked_file_changes_hash(tmp_path: Path) -> None:
+def test_capture_untracked_file_changes_hash(tmp_path: Path) -> None:
     repo = _init_repo(tmp_path / "repo")
     _write(repo, "main.py", "print(1)")
     _commit_all(repo)
@@ -159,7 +146,7 @@ def test_capture_dirty_untracked_file_changes_hash(tmp_path: Path) -> None:
 
     # The stale-code guard: an untracked file is part of the build context, so it
     # must move the hash (a `git diff` of tracked files alone would miss it).
-    assert prov.is_clean_commit is False
+    assert prov.dirty is True
     assert prov.working_tree_hash == working_tree_hash(repo)
     assert working_tree_hash(repo) != _hash_without(repo, "scratch.py")
 
@@ -214,7 +201,7 @@ def test_capture_no_remote(tmp_path: Path) -> None:
 
     assert prov.repo is None
     assert prov.commit is not None
-    assert prov.is_clean_commit is True  # no remote, but a clean commit still anchors it
+    assert prov.working_tree_hash is not None  # always computed
 
 
 def test_capture_non_git_dir(tmp_path: Path) -> None:
@@ -226,12 +213,28 @@ def test_capture_non_git_dir(tmp_path: Path) -> None:
     assert prov.repo is None
     assert prov.commit is None
     assert prov.ref is None
-    # No commit to point to → the content hash is the identity.
+    # No commit → the content hash is the identity; dirtiness is undefined (no VCS).
     assert prov.working_tree_hash == working_tree_hash(plain)
-    assert prov.is_clean_commit is False
+    assert prov.dirty is None
     assert prov.build_timestamp is not None
 
 
+def test_capture_never_raises_when_hash_fails(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
+    import agentex.lib.utils.build_provenance as bp
+
+    plain = tmp_path / "plain"  # non-git → would hash, which we force to fail
+    _write(plain, "main.py", "print(1)")
+
+    def _boom(_root: Path) -> str:
+        raise OSError("permission denied")
+
+    monkeypatch.setattr(bp, "working_tree_hash", _boom)
+
+    prov = bp.capture_build_provenance(plain, plain)  # must not raise
+
+    assert prov.working_tree_hash is None
+
+
 def test_capture_monorepo_subpath(tmp_path: Path) -> None:
     repo = _init_repo(tmp_path / "repo")
     _write(repo, "agents/foo/main.py", "print(1)")