diff --git a/api/services/forge_projection.py b/api/services/forge_projection.py index 1df0d73..0a8da10 100644 --- a/api/services/forge_projection.py +++ b/api/services/forge_projection.py @@ -12,6 +12,7 @@ what makes the reset in `T03` verifiable: you can always ask what the projection from __future__ import annotations +import os import re import subprocess import tempfile @@ -38,6 +39,50 @@ class ForgeDeriveError(RuntimeError): """The repository could not be read from the forge.""" +class ForgeUnreadableError(ForgeDeriveError): + """Central is not permitted to read this repository — a policy, not a fault. + + `ADR-012`'s premise (the forge is the projection source) holds only for + repositories central can read, and until `STATE-WP-0084` nothing said so: + a private repository failed the same way a broken one did, so "cannot read" + and "does not exist" were indistinguishable from the outside. + + They must never be confused, because they authorise opposite things. A + repository that does not derive may have had its files removed deliberately; + a repository we cannot read tells us nothing at all about its files. Only + the first can justify retiring a record. + + Forgejo answers an unauthenticated request for a private repository with a + 404, so "not found" is classified as unreadable too. That is deliberate: the + two cases are genuinely indistinguishable at this layer, and the safe + reading of an ambiguous answer is the one that cannot destroy a record. + """ + + +# git says this in several ways depending on version, transport, and whether a +# credential helper is installed; all of them mean the same thing here. +_UNREADABLE_MARKERS = ( + "could not read username", + "could not read password", + "authentication failed", + "terminal prompts disabled", + "invalid username or password", + "403 forbidden", + "the requested url returned error: 403", + "the requested url returned error: 401", + "repository not found", + "remote: not found", + "does not appear to be a git repository", +) + + +def _is_unreadable(message: str) -> bool: + low = message.lower() + if "not found" in low and "fatal: repository" in low: + return True + return any(marker in low for marker in _UNREADABLE_MARKERS) + + @dataclass class DerivedTask: record_id: str @@ -63,11 +108,28 @@ class DerivedProjection: repo_slug: str commit: str workplans: list[DerivedWorkplan] = field(default_factory=list) + # False when the checkout has no `workplans/` directory at all. An empty + # projection then means "we did not find the records", which is not the same + # claim as "this repository has no records" — and only the second one could + # ever justify retiring anything. + records_source_present: bool = True @property def task_count(self) -> int: return sum(len(w.tasks) for w in self.workplans) + @property + def retirement_eligible(self) -> bool: + """Whether an absence in this projection is evidence of an absence. + + A projection that could not be read never reaches this: it raises. What + this rules out is the quieter case — a clone that succeeded and returned + nothing, which is what would have retired every record in a repository + had a clone ever come back empty instead of failing (`STATE-WP-0084-T01`; + the near-miss was `vergabe-teilnahme`). + """ + return self.records_source_present + def to_dict(self) -> dict[str, Any]: return { "schema": "state-hub.forge-projection.v1", @@ -75,6 +137,8 @@ class DerivedProjection: # Provenance is not optional: a projection that cannot name the # commit it came from cannot be audited (ADR-012 decision 2). "commit": self.commit, + "records_source_present": self.records_source_present, + "retirement_eligible": self.retirement_eligible, "workplans": [ { "record_id": w.record_id, @@ -100,8 +164,12 @@ class DerivedProjection: def _run_git(*args: str, cwd: str | None = None, timeout: float = 120.0) -> str: + # Without this a clone of a private repository blocks on a username prompt + # instead of failing, and an unattended derivation pass hangs rather than + # reporting. Failing fast is what makes the unreadable case observable. + env = {**os.environ, "GIT_TERMINAL_PROMPT": "0", "GIT_ASKPASS": "", "GCM_INTERACTIVE": "never"} proc = subprocess.run( - ["git", *args], cwd=cwd, capture_output=True, text=True, timeout=timeout + ["git", *args], cwd=cwd, capture_output=True, text=True, timeout=timeout, env=env ) if proc.returncode != 0: raise ForgeDeriveError((proc.stderr or proc.stdout).strip()[:400]) @@ -161,6 +229,7 @@ def derive_from_checkout(repo_root: Path, repo_slug: str, commit: str) -> Derive proj = DerivedProjection(repo_slug=repo_slug, commit=commit) wp_dir = repo_root / "workplans" if not wp_dir.is_dir(): + proj.records_source_present = False return proj for path in sorted(wp_dir.rglob("*.md")): if path.name.startswith("."): @@ -208,6 +277,14 @@ def derive_from_forge( _run_git(*args, url, tmp) except subprocess.TimeoutExpired as exc: raise ForgeDeriveError(f"clone timed out for {repo_slug}") from exc + except ForgeDeriveError as exc: + # Classify before propagating. A caller that cannot tell "not + # permitted" from "broken" will eventually treat one as the other. + if _is_unreadable(str(exc)): + raise ForgeUnreadableError( + f"{repo_slug} could not be read from the forge: {exc}" + ) from exc + raise commit = _run_git("rev-parse", "HEAD", cwd=tmp) return derive_from_checkout(Path(tmp), repo_slug, commit) @@ -241,6 +318,9 @@ class ProjectionDiff: missing: list[dict[str, Any]] = field(default_factory=list) # forge has, hub lacks stale: list[dict[str, Any]] = field(default_factory=list) # hub has, forge lacks differing: list[dict[str, Any]] = field(default_factory=list) # both, fields differ + # Set when the source could not support a claim of absence, so `stale` was + # deliberately left empty rather than computed (`STATE-WP-0084-T01`). + stale_withheld: str | None = None @property def clean(self) -> bool: @@ -261,6 +341,7 @@ class ProjectionDiff: "stale": len(self.stale), "differing": len(self.differing), }, + "stale_withheld": self.stale_withheld, "missing": self.missing, "stale": self.stale, "differing": self.differing, @@ -278,6 +359,14 @@ def diff_against_hub( is testable without a database and cannot accidentally mutate anything. """ d = ProjectionDiff(repo_slug=derived.repo_slug, commit=derived.commit) + if not derived.retirement_eligible: + # Compute what is missing and what differs as usual — those only ever + # add or correct. Absence is the one conclusion this source cannot + # support, so it is not drawn at all rather than drawn and then filtered. + d.stale_withheld = ( + "the checkout has no workplans/ directory, so an absent record is " + "unexplained rather than evidence of removal" + ) # Match on canonical identity, never on UUID. Most hub records still carry # pre-ADR-007 random identifiers, so a UUID-keyed comparison reports every @@ -314,6 +403,8 @@ def diff_against_hub( d.missing.append({"kind": "workplan", "record_id": w.record_id, "uuid": w.uuid}) for key, w in have_wp.items(): if key not in want_wp: + if d.stale_withheld: + continue uid = str(w["id"]) d.stale.append( { @@ -371,6 +462,8 @@ def diff_against_hub( ) for key, t in have.items(): if key not in want: + if d.stale_withheld: + continue uid = str(t["id"]) d.stale.append( {"kind": "task", "uuid": uid, "title": t.get("title"), @@ -397,7 +490,7 @@ def diff_against_hub( class ResetOutcome: repo_slug: str commit: str - status: str # applied | refused | noop + status: str # applied | refused | noop | unreadable created: list[str] = field(default_factory=list) updated: list[str] = field(default_factory=list) retired: list[str] = field(default_factory=list) @@ -460,7 +553,23 @@ async def reset_repository_projection( from api.models.task import Task from api.models.workplan import Workplan - derived = derived or derive_from_forge(repo_slug, forge_base=forge_base) + if derived is None: + try: + derived = derive_from_forge(repo_slug, forge_base=forge_base) + except ForgeUnreadableError as exc: + # Not an error: a statement about what central is permitted to see. + # Reported as its own status so a caller cannot mistake it for a + # repository whose records stopped deriving (`STATE-WP-0084-T01`). + out = ResetOutcome(repo_slug=repo_slug, commit="", status="unreadable") + out.refused.append( + {"reason": "repository could not be read from the forge", "slug": repo_slug, + "detail": str(exc)[:300]} + ) + out.notes.append( + "Nothing was changed and nothing was retired. This says nothing " + "about whether the repository's records still exist." + ) + return out outcome = ResetOutcome(repo_slug=repo_slug, commit=derived.commit, status="noop") repo = ( @@ -571,6 +680,27 @@ async def reset_repository_projection( r for k, r in matched.items() if k not in want and r.projection_retired_at is None ] + if stale and not derived.retirement_eligible: + # Acknowledgement cannot authorise this. The caller is confirming that + # records which stopped deriving should be retired; here nothing has + # been shown to have stopped deriving, because the source produced no + # records to compare against. Consenting to a conclusion is not the + # same as the evidence for it existing. + outcome.status = "refused" + for r in stale: + outcome.refused.append( + { + "reason": "source produced no records; absence is unexplained", + "slug": r.slug, + "status": r.status, + "backing_relative_path": r.backing_relative_path, + } + ) + outcome.notes.append( + "The checkout has no workplans/ directory. Retirement withheld " + "regardless of acknowledgement (STATE-WP-0084-T01)." + ) + return outcome if stale and not acknowledge_retirements: outcome.status = "refused" for r in stale: @@ -665,6 +795,11 @@ async def reset_repository_projection( class FleetResetOutcome: results: dict[str, dict[str, Any]] = field(default_factory=dict) errors: dict[str, str] = field(default_factory=dict) + # Kept apart from `errors` on purpose. Nine repositories sitting in an error + # bucket read as nine broken repositories; they were nine we were not + # allowed to read, which is a different thing to go and fix + # (`STATE-WP-0083-T04`, 2026-08-26). + unreadable: dict[str, str] = field(default_factory=dict) def to_dict(self) -> dict[str, Any]: by_status: dict[str, int] = {} @@ -672,9 +807,11 @@ class FleetResetOutcome: by_status[r["status"]] = by_status.get(r["status"], 0) + 1 return { "schema": "state-hub.fleet-projection-reset.v1", - "repositories": len(self.results) + len(self.errors), + "repositories": len(self.results) + len(self.errors) + len(self.unreadable), "by_status": by_status, "errored": len(self.errors), + "unreadable_count": len(self.unreadable), + "unreadable": self.unreadable, "totals": { k: sum(r["counts"][k] for r in self.results.values()) for k in ("created", "updated", "retired", "refused") @@ -719,7 +856,16 @@ async def reset_fleet_projection( await session.commit() else: await session.rollback() - outcome.results[slug] = result.to_dict() + if result.status == "unreadable": + detail = next( + (r.get("detail", "") for r in result.refused), "" + ) + outcome.unreadable[slug] = detail or "could not be read from the forge" + else: + outcome.results[slug] = result.to_dict() + except ForgeUnreadableError as exc: + # Reachable when the caller supplied its own derivation path. + outcome.unreadable[slug] = str(exc)[:300] except Exception as exc: # noqa: BLE001 - one repo must not end the pass outcome.errors[slug] = f"{type(exc).__name__}: {exc}"[:300] return outcome diff --git a/tests/test_forge_projection.py b/tests/test_forge_projection.py index 706be1a..ffa2e84 100644 --- a/tests/test_forge_projection.py +++ b/tests/test_forge_projection.py @@ -417,3 +417,118 @@ class TestSlugCollisionRefusal: assert out.status == "refused" assert out.refused[0]["reason"].startswith("slug already belongs") assert session.added == [] + + +class TestUnreadableIsNotMissing: + """STATE-WP-0084-T01. + + A repository central is not permitted to read and a repository whose + records no longer derive authorise opposite things. Every test here exists + to keep the retirement path unreachable from an answer that cannot support + it — by construction, not by the reset happening to fail first. + """ + + @pytest.mark.parametrize( + "stderr", + [ + "fatal: could not read Username for 'https://forgejo.coulomb.social'", + "remote: Invalid username or password.\nfatal: Authentication failed", + "fatal: could not read Username for 'https://f': terminal prompts disabled", + "fatal: repository 'https://forgejo.coulomb.social/rapp-openbao.git' not found", + "fatal: unable to access '...': The requested URL returned error: 403", + ], + ) + def test_permission_shaped_failures_are_classified_unreadable(self, stderr, monkeypatch): + def boom(*a, **k): + raise fp.ForgeDeriveError(stderr) + + monkeypatch.setattr(fp, "_run_git", boom) + with pytest.raises(fp.ForgeUnreadableError): + fp.derive_from_forge("rapp-openbao") + + def test_a_genuine_fault_stays_a_plain_error(self, monkeypatch): + def boom(*a, **k): + raise fp.ForgeDeriveError("fatal: early EOF\nfatal: index-pack failed") + + monkeypatch.setattr(fp, "_run_git", boom) + with pytest.raises(fp.ForgeDeriveError) as exc: + fp.derive_from_forge("demo") + assert not isinstance(exc.value, fp.ForgeUnreadableError) + + def test_unreadable_is_a_derive_error_so_old_callers_still_catch_it(self): + assert issubclass(fp.ForgeUnreadableError, fp.ForgeDeriveError) + + def test_a_checkout_without_workplans_cannot_evidence_absence(self, tmp_path): + (tmp_path / "bare").mkdir() + p = fp.derive_from_checkout(tmp_path / "bare", "bare", "abc") + assert p.workplans == [] + assert p.records_source_present is False + assert p.retirement_eligible is False + + def test_a_real_checkout_can(self, tmp_path): + p = fp.derive_from_checkout(_repo(tmp_path), "demo", "abc") + assert p.records_source_present is True + assert p.retirement_eligible is True + + def test_the_diff_withholds_stale_rather_than_computing_it(self): + derived = fp.DerivedProjection( + repo_slug="demo", commit="c0ffee", records_source_present=False + ) + hub = [{"id": "11111111-1111-1111-1111-111111111111", "slug": "demo-wp-0001", + "status": "active", "backing_relative_path": "workplans/a.md"}] + d = fp.diff_against_hub(derived, hub, {}) + assert d.stale == [] + assert d.would_remove == 0 + assert d.stale_withheld + + @pytest.mark.asyncio + async def test_an_empty_source_cannot_retire_even_when_acknowledged(self): + derived = fp.DerivedProjection( + repo_slug="demo", commit="c0ffee", records_source_present=False + ) + row = _Row(slug="demo-wp-0001", status="active", path="workplans/a.md") + session = _FakeSession(repo=_Repo(), rows=[row]) + out = await fp.reset_repository_projection( + session, "demo", derived=derived, acknowledge_retirements=True + ) + assert out.status == "refused" + assert out.retired == [] + assert row.projection_retired_at is None + assert out.refused[0]["reason"].startswith("source produced no records") + + @pytest.mark.asyncio + async def test_an_unreadable_repository_reports_unreadable_not_error(self, monkeypatch): + def boom(*a, **k): + raise fp.ForgeUnreadableError("rapp-openbao could not be read from the forge") + + monkeypatch.setattr(fp, "derive_from_forge", boom) + session = _FakeSession(repo=_Repo(), rows=[]) + out = await fp.reset_repository_projection(session, "rapp-openbao") + assert out.status == "unreadable" + assert out.retired == [] and out.created == [] and out.updated == [] + + @pytest.mark.asyncio + async def test_the_fleet_keeps_unreadable_out_of_the_error_bucket(self, monkeypatch): + async def fake(session, slug, **kw): + if slug == "private": + out = fp.ResetOutcome(repo_slug=slug, commit="", status="unreadable") + out.refused.append({"reason": "repository could not be read from the forge", + "slug": slug, "detail": "could not read Username"}) + return out + if slug == "broken": + raise RuntimeError("index-pack failed") + out = fp.ResetOutcome(repo_slug=slug, commit="c0ffee", status="applied") + out.updated.append("A-WP-0001") + return out + + monkeypatch.setattr(fp, "reset_repository_projection", fake) + sessions = [_FakeSession(repo=_Repo(), rows=[]) for _ in range(3)] + outcome = await fp.reset_fleet_projection( + TestFleetReset._Factory(sessions), ["ok", "private", "broken"] + ) + assert list(outcome.unreadable) == ["private"] + assert list(outcome.errors) == ["broken"] + assert "private" not in outcome.results + d = outcome.to_dict() + assert d["unreadable_count"] == 1 and d["errored"] == 1 + assert d["repositories"] == 3 diff --git a/workplans/STATE-WP-0084-forge-read-for-private-repositories.md b/workplans/STATE-WP-0084-forge-read-for-private-repositories.md index bcdb522..5800d8e 100644 --- a/workplans/STATE-WP-0084-forge-read-for-private-repositories.md +++ b/workplans/STATE-WP-0084-forge-read-for-private-repositories.md @@ -54,7 +54,7 @@ wait on `MASON-WP-0003-T02`. ```task id: STATE-WP-0084-T01 -status: todo +status: done priority: high state_hub_task_id: "e955067d-2152-59f7-b672-12755ceb26b0" ``` @@ -81,6 +81,33 @@ Acceptance: an unreadable repository is reported as unreadable; a clone that succeeds but returns nothing produces no retirement proposals; both cases are covered by tests that fail if the retirement path is reachable from either. +**Done 2026-08-26.** `api/services/forge_projection.py`: + +- `ForgeUnreadableError` subclasses `ForgeDeriveError`, so existing callers + keep catching it while new ones can tell the cases apart. A permission-shaped + git failure is classified into it; a genuine fault stays a plain error. + Forgejo's 404 for an unauthenticated private repository classifies as + unreadable, because the two are indistinguishable at this layer and the safe + reading of an ambiguous answer is the one that cannot destroy a record. +- `_run_git` runs with `GIT_TERMINAL_PROMPT=0`. Without it an unattended pass + blocks on a username prompt instead of failing, and an unreadable repository + is only observable if it fails. +- `DerivedProjection.records_source_present` / `.retirement_eligible` separate + "no records found" from "no records exist". A checkout with no `workplans/` + directory can no longer evidence an absence. +- `diff_against_hub` withholds `stale` rather than computing and filtering it, + and says so in `stale_withheld`. `would_remove` is 0 for such a source. +- `reset_repository_projection` returns `status="unreadable"` for a repository + it may not read, and refuses retirement from an ineligible source **even when + `acknowledge_retirements=True`** — consenting to a conclusion is not the same + as the evidence for it existing. +- `reset_fleet_projection` keeps unreadable repositories out of `errors`, in + their own bucket with its own count. Nine repositories in an error bucket + read as nine broken repositories; they were nine we were not allowed to read. + +13 tests in `TestUnreadableIsNotMissing` (`tests/test_forge_projection.py`); +58 pass across the forge/projection/backfill suites. + ## Deliver the credential to the pod ```task