fix(registrar): bind workplan files to their records in the same pass
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 <noreply@anthropic.com> Assistant: claude-code Assistant-Model: opus Assistant-Process: 2583210@bnt-lap001 Assistant-Session: f2bff2d5-e9b2-4338-92ca-10282a927006
This commit is contained in:
parent
667bac3080
commit
bdb981be2b
2 changed files with 167 additions and 1 deletions
|
|
@ -21,7 +21,7 @@ import httpx
|
||||||
|
|
||||||
from repo_manager.gitops import GitError, commit_paths, push_ff
|
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.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
|
from repo_manager.record_identity import scan_record_identities
|
||||||
|
|
||||||
LOCK_PATH = Path("/tmp/repo-manager-identifier-registrar.lock")
|
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
|
}, 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(
|
def _check_primary(
|
||||||
api_base: str, *, allow_unverified: bool = False
|
api_base: str, *, allow_unverified: bool = False
|
||||||
) -> tuple[dict[str, Any], str | None]:
|
) -> 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:
|
if not any(before.values()) and not repair_projection_id and not bootstrap_empty_projection:
|
||||||
evidence["missing_after"] = before
|
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)
|
return RegistrarResult("noop", evidence, None, cid)
|
||||||
|
|
||||||
git_evidence, git_error = _check_git(repo)
|
git_evidence, git_error = _check_git(repo)
|
||||||
|
|
@ -737,4 +800,6 @@ def registrar_reconcile(
|
||||||
cid,
|
cid,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
evidence["workplan_bindings"] = _sync_workplan_bindings(repo, api_base, repo.name)
|
||||||
|
|
||||||
return RegistrarResult("applied", evidence, None, cid)
|
return RegistrarResult("applied", evidence, None, cid)
|
||||||
|
|
|
||||||
|
|
@ -612,3 +612,104 @@ class TestCheckPrimary:
|
||||||
)
|
)
|
||||||
_, err = rr._check_primary("http://hub", allow_unverified=True)
|
_, err = rr._check_primary("http://hub", allow_unverified=True)
|
||||||
assert err is None
|
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
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue