From 0068e34644e9413efa565414357d843634d0bb27 Mon Sep 17 00:00:00 2001 From: tegwick Date: Thu, 10 Sep 2026 14:11:45 +0200 Subject: [PATCH] fix: bind projection retries and validate exact receipts Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a07ff8-19d0-7820-b4d0-1353833cb7fc --- src/repo_manager/projection_sync.py | 44 ++++++++++--- tests/test_fast_work_records.py | 66 +++++++++++++++++-- ...GR-WP-0017-projection-retry-convergence.md | 58 ++++++++++++++++ 3 files changed, 156 insertions(+), 12 deletions(-) create mode 100644 workplans/RMGR-WP-0017-projection-retry-convergence.md diff --git a/src/repo_manager/projection_sync.py b/src/repo_manager/projection_sync.py index 74558da..4254ff0 100644 --- a/src/repo_manager/projection_sync.py +++ b/src/repo_manager/projection_sync.py @@ -2,6 +2,7 @@ from __future__ import annotations +import hashlib import json import subprocess import tempfile @@ -152,14 +153,27 @@ def sync_repository_projection( if not commit: return {"ok": False, "status": "failed", "error": "repository HEAD is unavailable"} + request_path = f"/repos/{slug}/work-record-projection/reconcile" + request_body = { + "expected_commit": commit, + "acknowledge_retirements": acknowledge_retirements, + } + # The review acknowledgement changes request semantics at the same commit. + # Bind the key to the complete POST, retaining identical keys for retries. + request_digest = hashlib.sha256(json.dumps( + {"method": "POST", "path": request_path, "body": request_body}, + sort_keys=True, separators=(",", ":"), + ).encode()).hexdigest() pending = { "schema": "repo-manager.pending-work-record-projection.v1", "repo_slug": slug, "expected_commit": commit, "api_base": api_base.rstrip("/"), + "acknowledge_retirements": acknowledge_retirements, + "expected_instance_label": expected_instance_label, } headers = { - "Idempotency-Key": f"rmgr-projection:{slug}:{commit}", + "Idempotency-Key": f"rmgr-projection:v2:{request_digest}", "X-StateHub-Source-Agent": "repo-manager", } try: @@ -192,11 +206,8 @@ def sync_repository_projection( "pending_path": str(path), } response = client.post( - f"/repos/{slug}/work-record-projection/reconcile", - json={ - "expected_commit": commit, - "acknowledge_retirements": acknowledge_retirements, - }, + request_path, + json=request_body, headers=headers, ) response.raise_for_status() @@ -224,8 +235,25 @@ def sync_repository_projection( "error": detail, } - _pending_path(repo_root).unlink(missing_ok=True) - outcome_status = receipt.get("outcome", {}).get("status", "applied") + # A successful HTTP response alone is not an exact-commit primary receipt. + outcome = receipt.get("outcome") if isinstance(receipt, dict) else None + if not ( + isinstance(receipt, dict) + and receipt.get("schema") == "state-hub.repository-projection-reconcile.v1" + and receipt.get("instance_role") == "primary" + and receipt.get("instance_label") == identity.get("instance_label") + and receipt.get("expected_commit") == commit + and receipt.get("derived_commit") == commit + and isinstance(outcome, dict) + and outcome.get("repo_slug") == slug + and outcome.get("commit") == commit + and outcome.get("status") in {"applied", "noop", "refused"} + ): + return {"ok": False, "status": "invalid_receipt", "repo_slug": slug, + "commit": commit, "error": "response does not bind the requested primary/repository/commit"} + outcome_status = outcome["status"] + if outcome_status in {"applied", "noop"}: + _pending_path(repo_root).unlink(missing_ok=True) return { "ok": outcome_status in {"applied", "noop"}, "status": outcome_status, diff --git a/tests/test_fast_work_records.py b/tests/test_fast_work_records.py index cdaffe4..8020f7e 100644 --- a/tests/test_fast_work_records.py +++ b/tests/test_fast_work_records.py @@ -1,10 +1,12 @@ from __future__ import annotations +import json import subprocess from datetime import date from pathlib import Path import httpx +import pytest from repo_manager.commands.task import add_adhoc_task, mutate_task from repo_manager.commands.workplan import create_workplan @@ -173,10 +175,7 @@ def test_sync_uses_two_requests_and_exact_pushed_commit(tmp_path: Path) -> None: ) return httpx.Response( 200, - json={ - "outcome": {"status": "applied", "counts": {"created": 1}}, - "derived_commit": payload["expected_commit"], - }, + json=_receipt(payload), ) result = sync_repository_projection( @@ -234,3 +233,62 @@ def test_sync_queues_instead_of_writing_to_wrong_instance(tmp_path: Path) -> Non assert result["status"] == "queued" assert result["reason"] == "wrong_instance" assert Path(result["pending_path"]).is_file() + + +def _receipt(payload, **overrides): + return { + "schema": "state-hub.repository-projection-reconcile.v1", + "instance_role": "primary", "instance_label": "railiance01", + "expected_commit": payload["expected_commit"], + "derived_commit": payload["expected_commit"], + "outcome": {"repo_slug": "pilot", "commit": payload["expected_commit"], + "status": "applied", "counts": {"created": 1}}, + **overrides, + } + + +def test_reviewed_retry_has_distinct_key_and_identical_retry_replays(tmp_path): + repo = _fixture(tmp_path, remote=True) + seen = {} + keys = [] + + def handler(request): + if request.url.path == "/state/health": + return httpx.Response(200, json={"instance_role": "primary", "instance_label": "railiance01"}) + payload = json.loads(request.content) + key = request.headers["Idempotency-Key"] + keys.append(key) + if key in seen and seen[key] != payload: + return httpx.Response(409, json={"detail": "idempotency key reused for different payload"}) + seen[key] = payload + result = _receipt(payload) + result["outcome"]["status"] = "applied" if payload["acknowledge_retirements"] else "refused" + return httpx.Response(200, json=result) + + kwargs = {"api_base": "http://hub.test", "push": True, "transport": httpx.MockTransport(handler)} + refused = sync_repository_projection(repo, **kwargs) + admitted = sync_repository_projection(repo, acknowledge_retirements=True, **kwargs) + replay = sync_repository_projection(repo, acknowledge_retirements=True, **kwargs) + assert refused["status"] == "refused" + assert admitted["status"] == replay["status"] == "applied" + assert keys[0] != keys[1] == keys[2] + assert len(seen) == 2 + + +@pytest.mark.parametrize("overrides", [ + {"expected_commit": "f" * 40}, {"derived_commit": "e" * 40}, + {"instance_role": "cache"}, {"instance_label": "workstation"}, + {"schema": "unrelated-receipt/v1"}, {"outcome": {"status": "applied"}}, +]) +def test_sync_refuses_inconsistent_receipt(tmp_path, overrides): + repo = _fixture(tmp_path, remote=True) + + def handler(request): + if request.url.path == "/state/health": + return httpx.Response(200, json={"instance_role": "primary", "instance_label": "railiance01"}) + return httpx.Response(200, json=_receipt(json.loads(request.content), **overrides)) + + result = sync_repository_projection(repo, api_base="http://hub.test", push=True, + transport=httpx.MockTransport(handler)) + assert result["ok"] is False + assert result["status"] == "invalid_receipt" diff --git a/workplans/RMGR-WP-0017-projection-retry-convergence.md b/workplans/RMGR-WP-0017-projection-retry-convergence.md new file mode 100644 index 0000000..f5dca9b --- /dev/null +++ b/workplans/RMGR-WP-0017-projection-retry-convergence.md @@ -0,0 +1,58 @@ +--- +id: RMGR-WP-0017 +type: workplan +title: "Make reviewed projection retries and receipts reliable" +domain: infotech +repo: repo-manager +status: active +owner: codex +topic_slug: infotech +created: "2026-09-10" +updated: "2026-09-10" +related: [RMGR-WP-0012, HFACT-WP-0001, STATE-WP-0090] +--- + +Follow-up to HFACT-WP-0001-T02's observed HTTP 409 when a reviewed retirement +acknowledgement changes at the same source commit. File/Forge authority, primary +identity, exact commit and explicit retirement acknowledgement stay unchanged. + +## Bind retries to the complete request + +```task +id: RMGR-WP-0017-T01 +status: done +priority: high +``` + +Digest the method, repository route and full POST body into the idempotency key. +Identical retries retain their key; an explicit review acknowledgement uses a +different key. Retain acknowledgement and expected instance in queued metadata. + +## Verify the returned primary and commit + +```task +id: RMGR-WP-0017-T02 +status: done +priority: high +``` + +Reject incomplete or mismatched primary/repository/commit receipts instead of +reporting success or discarding pending state. Keep the two-request fast path. + +## Prove convergence and synchronize + +```task +id: RMGR-WP-0017-T03 +status: todo +priority: high +``` + +Run real-Git/mocked-transport regressions, the full suite and targeted lint. +Commit/sync; verify exact primary receipts on the selected factory chain using +the corrected client. STATE-WP-0090 owns the server's description/note fix and +live projection convergence. Preserve all existing identities and source files. + +Validation: all seven new regressions failed on the original client. The fixed +client passes all 188 repository tests and targeted Ruff checks. Identical +requests replay; changed reviewed payloads do not collide. Exact receipt checks +retain the two-request path. Live synchronization evidence remains T03.