From bdb981be2bd4bedcbe5b1b5edf45e9090dce7f12 Mon Sep 17 00:00:00 2001 From: tegwick Date: Tue, 25 Aug 2026 15:39:11 +0200 Subject: [PATCH] fix(registrar): bind workplan files to their records in the same pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit backing_filename is what lets the read model tell a file-backed workplan from a hub-only orphan. It is written by PUT /workplans/index/bindings, which fix-consistency calls for workplans that already carry a UUID — but the registrar mints the UUID afterwards, so a freshly registered workplan stayed unbound until someone happened to run fix-consistency a second time. Nobody did: 278 of 800 workplans on central recorded no backing file, including four active and four ready. ADR-010 predicted this as the "broken links" class. The registrar now syncs bindings after minting, and on the noop path too — otherwise a record whose earlier bind failed stays unbound forever, because every later run returns early. Binding never fails the registration: the identifiers are already minted and committed, and a bind can be retried. Status is sent only when already canonical. The binding schema validates against the enum without normalising, so one legacy value 422s the whole batch; omitting beats guessing a mapping that could drift from canon. Refs CUST-WP-0068-T07 Co-Authored-By: Claude Opus 5 Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006 --- .../commands/registrar_reconcile.py | 67 +++++++++++- tests/test_registrar_reconcile.py | 101 ++++++++++++++++++ 2 files changed, 167 insertions(+), 1 deletion(-) diff --git a/src/repo_manager/commands/registrar_reconcile.py b/src/repo_manager/commands/registrar_reconcile.py index 30f1b82..b3606b1 100644 --- a/src/repo_manager/commands/registrar_reconcile.py +++ b/src/repo_manager/commands/registrar_reconcile.py @@ -21,7 +21,7 @@ import httpx from repo_manager.gitops import GitError, commit_paths, push_ff from repo_manager.parse.record import iter_record_files, parse_record_file -from repo_manager.parse.workplan import parse_workplan_file +from repo_manager.parse.workplan import iter_workplan_files, parse_workplan_file from repo_manager.record_identity import scan_record_identities LOCK_PATH = Path("/tmp/repo-manager-identifier-registrar.lock") @@ -176,6 +176,65 @@ def _check_git(repo: Path) -> tuple[dict[str, Any], str | None]: }, None +def _sync_workplan_bindings(repo: Path, api_base: str, repo_slug: str) -> dict[str, Any]: + """Record which file backs each workplan, immediately after minting its id. + + `backing_filename` is what makes a hub record re-derivable: without it the + read model cannot tell a file-backed workplan from a hub-only orphan. It is + written by `PUT /workplans/index/bindings`, which `fix-consistency` calls + for workplans that *already* carry a UUID — but the registrar mints the UUID + afterwards, so a freshly registered workplan stayed unbound until someone + happened to run `fix-consistency` a second time. Nobody did, and 35% of + central's workplans recorded no backing file as a result + (CUST-WP-0068-T07). + + Binding here closes the window rather than adding a step an operator has to + remember, which is how the gap opened in the first place. + """ + # The binding schema validates `status` against the canonical enum and does + # not normalise, so anything else is a 422 that would drop the whole batch. + # Status is optional metadata here; omit what we cannot vouch for rather + # than guess a mapping that could drift from canon. + canonical = { + "proposed", "ready", "active", "blocked", "backlog", "finished", "archived", + } + bindings: list[dict[str, Any]] = [] + for path in iter_workplan_files(repo): + parsed = parse_workplan_file(path, repo_root=repo) + if not parsed.state_hub_workstream_id: + continue + bindings.append( + { + "workplan_id": parsed.state_hub_workstream_id, + "filename": path.name, + "relative_path": str(path.relative_to(repo).as_posix()), + "repo_slug": repo_slug, + "archived": path.parent.name == "archived", + "status": parsed.status if parsed.status in canonical else None, + } + ) + if not bindings: + return {"skipped": "no workplans carry an identifier"} + try: + response = httpx.put( + f"{api_base.rstrip('/')}/workplans/index/bindings", + json={"bindings": bindings}, + timeout=30.0, + ) + response.raise_for_status() + payload = response.json() + except (httpx.HTTPError, ValueError) as exc: + # Never fail the registration for this: the identifiers are already + # minted and committed, and the binding can be re-synced. + return {"ok": False, "error": str(exc), "sent": len(bindings)} + updated = payload.get("updated", 0) + result = {"ok": True, "updated": updated, "sent": len(bindings)} + if updated < len(bindings): + # Expected while records exist in files but not yet on this hub. + result["unbound"] = len(bindings) - updated + return result + + def _check_primary( api_base: str, *, allow_unverified: bool = False ) -> tuple[dict[str, Any], str | None]: @@ -520,6 +579,10 @@ def registrar_reconcile( ) if not any(before.values()) and not repair_projection_id and not bootstrap_empty_projection: evidence["missing_after"] = before + # Bind on this path too. Nothing needs minting, but a previous run may + # have minted and failed to bind — without this, such records stay + # unbound forever because every later run lands here. + evidence["workplan_bindings"] = _sync_workplan_bindings(repo, api_base, repo.name) return RegistrarResult("noop", evidence, None, cid) git_evidence, git_error = _check_git(repo) @@ -737,4 +800,6 @@ def registrar_reconcile( cid, ) + evidence["workplan_bindings"] = _sync_workplan_bindings(repo, api_base, repo.name) + return RegistrarResult("applied", evidence, None, cid) diff --git a/tests/test_registrar_reconcile.py b/tests/test_registrar_reconcile.py index 1aa016a..478d479 100644 --- a/tests/test_registrar_reconcile.py +++ b/tests/test_registrar_reconcile.py @@ -612,3 +612,104 @@ class TestCheckPrimary: ) _, err = rr._check_primary("http://hub", allow_unverified=True) assert err is None + + +class TestWorkplanBindings: + """Registration must record the backing file in one pass (CUST-WP-0068-T07). + + `backing_filename` is what lets the read model tell a file-backed workplan + from a hub-only orphan. It used to be written only by a later + `fix-consistency` run, so freshly registered workplans stayed unbound and + 35% of central's workplans recorded no backing file. + """ + + @staticmethod + def _repo(tmp_path): + repo = tmp_path / "demo" + (repo / "workplans" / "archived").mkdir(parents=True) + (repo / "workplans" / "DEMO-WP-0001-a.md").write_text( + '---\nid: DEMO-WP-0001\ntype: workplan\nstatus: active\n' + 'state_hub_workstream_id: "11111111-1111-5111-8111-111111111111"\n---\n\n# a\n', + encoding="utf-8", + ) + (repo / "workplans" / "archived" / "DEMO-WP-0002-b.md").write_text( + '---\nid: DEMO-WP-0002\ntype: workplan\nstatus: finished\n' + 'state_hub_workstream_id: "22222222-2222-5222-8222-222222222222"\n---\n\n# b\n', + encoding="utf-8", + ) + (repo / "workplans" / "DEMO-WP-0003-unregistered.md").write_text( + "---\nid: DEMO-WP-0003\ntype: workplan\nstatus: proposed\n---\n\n# c\n", + encoding="utf-8", + ) + return repo + + def test_binds_every_identified_workplan(self, tmp_path, monkeypatch): + repo = self._repo(tmp_path) + sent = {} + + class R: + def raise_for_status(self): return None + def json(self): return {"updated": 2, "received": 2} + + def fake_put(url, json=None, timeout=None): + sent["url"] = url + sent["bindings"] = json["bindings"] + return R() + + monkeypatch.setattr(rr.httpx, "put", fake_put) + result = rr._sync_workplan_bindings(repo, "http://hub", "demo") + + assert result["ok"] and result["updated"] == 2 + assert sent["url"].endswith("/workplans/index/bindings") + by_id = {b["workplan_id"][:8]: b for b in sent["bindings"]} + # The workplan with no identifier cannot be bound and must be skipped, + # not sent with a null id. + assert set(by_id) == {"11111111", "22222222"} + assert by_id["11111111"]["relative_path"] == "workplans/DEMO-WP-0001-a.md" + assert by_id["11111111"]["archived"] is False + assert by_id["22222222"]["archived"] is True + assert by_id["22222222"]["status"] == "finished" + + def test_non_canonical_status_is_omitted_not_guessed(self, tmp_path, monkeypatch): + """The schema does not normalise, so a bad status 422s the whole batch.""" + repo = self._repo(tmp_path) + (repo / "workplans" / "DEMO-WP-0004-legacy.md").write_text( + '---\nid: DEMO-WP-0004\ntype: workplan\nstatus: done\n' + 'state_hub_workstream_id: "44444444-4444-5444-8444-444444444444"\n---\n\n# d\n', + encoding="utf-8", + ) + sent = {} + + class R: + def raise_for_status(self): return None + def json(self): return {"updated": 3, "received": 3} + + monkeypatch.setattr( + rr.httpx, "put", + lambda url, json=None, timeout=None: (sent.update(b=json["bindings"]), R())[1], + ) + rr._sync_workplan_bindings(repo, "http://hub", "demo") + legacy = [b for b in sent["b"] if b["workplan_id"].startswith("44444444")][0] + assert legacy["status"] is None + + def test_binding_failure_never_fails_registration(self, tmp_path, monkeypatch): + """Identifiers are already minted and committed; a bind can be retried.""" + repo = self._repo(tmp_path) + + def boom(*a, **k): + raise rr.httpx.HTTPError("hub unreachable") + + monkeypatch.setattr(rr.httpx, "put", boom) + result = rr._sync_workplan_bindings(repo, "http://hub", "demo") + assert result["ok"] is False and "unreachable" in result["error"] + + def test_reports_records_the_hub_does_not_have(self, tmp_path, monkeypatch): + repo = self._repo(tmp_path) + + class R: + def raise_for_status(self): return None + def json(self): return {"updated": 1, "received": 2} + + monkeypatch.setattr(rr.httpx, "put", lambda *a, **k: R()) + result = rr._sync_workplan_bindings(repo, "http://hub", "demo") + assert result["unbound"] == 1