coulomb/repo-manager
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(registrar): make --confirm-primary assert authority, not liveness

Browse source 
_check_primary accepted any instance reporting status=ok and db=connected.
A local cache and the central hub both satisfied that for seven weeks while
every registration went to the cache — a liveness check wearing an authority
check's name.

It now requires the hub to declare instance_role=primary. An instance that
declares nothing is refused with a message naming what to set; proceeding
anyway requires an explicit --allow-unverified-primary rather than a silent
default.

Four tests cover the logic directly; the existing suite stubbed _check_primary
and never exercised it.

Refs CUST-WP-0067-T03

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:
tegwick 2026-08-25 10:32:51 +02:00
parent 68943631d8
commit 10bdb683cb
3 changed files with 105 additions and 11 deletions
Download patch file Download diff file Expand all files Collapse all files

7
src/repo_manager/cli.py
View file

@ -64,6 +64,12 @@ def main(argv: list[str] | None = None) -> int:
help="Confirm that --api-base is the authoritative hub",
)
p_registrar.add_argument("--push", action="store_true", help="Push the registrar commit")
p_registrar.add_argument(
"--allow-unverified-primary",
action="store_true",
help="Proceed when the hub declares no instance role. Explicit on purpose: "
"the alternative is silently trusting whatever answered.",
)
registrar_mode = p_registrar.add_mutually_exclusive_group()
registrar_mode.add_argument(
"--repair-workplan",
@ -517,6 +523,7 @@ def main(argv: list[str] | None = None) -> int:
api_base=args.api_base,
statehub_bin=args.statehub_bin,
confirm_primary=args.confirm_primary,
allow_unverified_primary=args.allow_unverified_primary,
push=args.push,
repair_workplan=args.repair_workplan,
bootstrap_empty_projection=args.bootstrap_empty_projection,

35
src/repo_manager/commands/registrar_reconcile.py
View file

@ -176,7 +176,16 @@ def _check_git(repo: Path) -> tuple[dict[str, Any], str | None]:
}, None
def _check_primary(api_base: str) -> tuple[dict[str, Any], str | None]:
def _check_primary(
api_base: str, *, allow_unverified: bool = False
) -> tuple[dict[str, Any], str | None]:
"""Verify the target is the authoritative hub, not merely a healthy one.
This used to assert only status/db, which every instance satisfies — a
liveness check wearing an authority check's name. A local cache and central
both passed it for seven weeks while onboarding went to the cache
(CUST-WP-0067-T03, ADR-010).
"""
try:
response = httpx.get(f"{api_base.rstrip('/')}/state/health", timeout=10.0)
response.raise_for_status()
@ -185,7 +194,24 @@ def _check_primary(api_base: str) -> tuple[dict[str, Any], str | None]:
return {}, f"primary State Hub health check failed: {exc}"
if payload.get("status") != "ok" or payload.get("db") != "connected":
return payload, "State Hub is not healthy and database-connected"
return payload, None
role = payload.get("instance_role")
label = payload.get("instance_label") or api_base
if role == "primary":
return payload, None
if role in (None, "unknown"):
if allow_unverified:
return payload, None
return payload, (
f"{api_base} does not declare an instance role, so it cannot be "
"confirmed as the authoritative hub. Set STATE_HUB_INSTANCE_ROLE="
"primary on the central deployment. To proceed without that "
"guarantee, pass --allow-unverified-primary explicitly."
)
return payload, (
f"{api_base} reports instance_role={role!r} ({label}); refusing to treat "
"it as the authoritative hub"
)
def _workplan_projection_id(repo: Path, canonical_id: str) -> str | None:
@ -410,6 +436,7 @@ def registrar_reconcile(
api_base: str = "http://127.0.0.1:8000",
statehub_bin: str | None = None,
confirm_primary: bool = False,
allow_unverified_primary: bool = False,
push: bool = False,
repair_workplan: str | None = None,
bootstrap_empty_projection: bool = False,
@ -502,7 +529,9 @@ def registrar_reconcile(
"rejected", evidence, {"code": "git_precondition_failed", "message": git_error}, cid
)
health, health_error = _check_primary(api_base)
health, health_error = _check_primary(
api_base, allow_unverified=allow_unverified_primary
)
evidence["state_hub_health"] = health
if health_error:
return RegistrarResult(

74
tests/test_registrar_reconcile.py
View file

@ -172,7 +172,7 @@ def test_linked_closed_workplan_does_not_reopen_historical_task_gaps(tmp_path: P
def test_rejects_dirty_or_unsynced_repository(tmp_path: Path, monkeypatch) -> None:
repo = _fixture(tmp_path)
monkeypatch.setattr(rr, "_check_primary", lambda _api: ({"status": "ok", "db": "connected"}, None))
monkeypatch.setattr(rr, "_check_primary", lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None))
(repo / "note.txt").write_text("dirty", encoding="utf-8")
dirty = rr.registrar_reconcile(repo, confirm_primary=True)
assert dirty.status == "rejected"
@ -242,7 +242,7 @@ status: accepted
def test_scopes_registrar_env_and_commits_assigned_ids(tmp_path: Path, monkeypatch) -> None:
repo = _fixture(tmp_path)
monkeypatch.setattr(rr, "_check_primary", lambda _api: ({"status": "ok", "db": "connected"}, None))
monkeypatch.setattr(rr, "_check_primary", lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None))
def fake_run(command, *, env):
assert command[1:3] == ["fix-consistency", "--path"]
@ -282,7 +282,7 @@ def test_accepts_unrelated_assessment_fail_after_exact_requested_verification(
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
def fake_run(command, *, env):
@ -335,7 +335,7 @@ def test_statehub_timeout_returns_structured_failure(tmp_path: Path, monkeypatch
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
monkeypatch.setattr(
rr,
@ -379,7 +379,7 @@ def test_repairs_an_already_identified_workplan_projection(tmp_path: Path, monke
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
monkeypatch.setattr(
rr,
@ -428,7 +428,7 @@ def test_repair_fails_when_exact_projection_remains_absent(tmp_path: Path, monke
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
monkeypatch.setattr(
rr,
@ -476,7 +476,7 @@ def test_bootstraps_and_verifies_a_completely_empty_projection(tmp_path: Path, m
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
monkeypatch.setattr(
rr,
@ -530,7 +530,7 @@ def test_empty_projection_bootstrap_refuses_existing_rows(tmp_path: Path, monkey
monkeypatch.setattr(
rr,
"_check_primary",
lambda _api: ({"status": "ok", "db": "connected"}, None),
lambda _api, **_: ({"status": "ok", "db": "connected", "instance_role": "primary"}, None),
)
monkeypatch.setattr(
rr,
@ -554,3 +554,61 @@ def test_empty_projection_bootstrap_refuses_existing_rows(tmp_path: Path, monkey
assert result.status == "rejected"
assert result.error and result.error["code"] == "bootstrap_precondition_failed"
class TestCheckPrimary:
"""_check_primary must assert authority, not just liveness (CUST-WP-0067-T03).
The previous version accepted any instance reporting status=ok and
db=connected. A local cache and the central hub both satisfied that for
seven weeks while every registration went to the cache.
"""
@staticmethod
def _resp(payload):
class R:
def raise_for_status(self):
return None
def json(self):
return payload
return R()
def test_accepts_declared_primary(self, monkeypatch):
monkeypatch.setattr(
rr.httpx, "get",
lambda *a, **k: self._resp(
{"status": "ok", "db": "connected", "instance_role": "primary"}
),
)
_, err = rr._check_primary("http://hub")
assert err is None
def test_rejects_declared_cache(self, monkeypatch):
monkeypatch.setattr(
rr.httpx, "get",
lambda *a, **k: self._resp(
{"status": "ok", "db": "connected", "instance_role": "cache",
"instance_label": "workstation"}
),
)
_, err = rr._check_primary("http://hub")
assert err and "cache" in err
def test_rejects_healthy_instance_that_declares_nothing(self, monkeypatch):
"""A healthy hub is not thereby the authoritative one."""
monkeypatch.setattr(
rr.httpx, "get",
lambda *a, **k: self._resp({"status": "ok", "db": "connected"}),
)
_, err = rr._check_primary("http://hub")
assert err and "does not declare an instance role" in err
def test_unverified_requires_an_explicit_opt_in(self, monkeypatch):
monkeypatch.setattr(
rr.httpx, "get",
lambda *a, **k: self._resp({"status": "ok", "db": "connected"}),
)
_, err = rr._check_primary("http://hub", allow_unverified=True)
assert err is None