fix: bind projection retries and validate exact receipts
Assistant: codex Assistant-Model: gpt-6-astra Assistant-Session: 01a07ff8-19d0-7820-b4d0-1353833cb7fc
This commit is contained in:
parent
84df076e95
commit
0068e34644
3 changed files with 156 additions and 12 deletions
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import hashlib
|
||||||
import json
|
import json
|
||||||
import subprocess
|
import subprocess
|
||||||
import tempfile
|
import tempfile
|
||||||
|
|
@ -152,14 +153,27 @@ def sync_repository_projection(
|
||||||
if not commit:
|
if not commit:
|
||||||
return {"ok": False, "status": "failed", "error": "repository HEAD is unavailable"}
|
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 = {
|
pending = {
|
||||||
"schema": "repo-manager.pending-work-record-projection.v1",
|
"schema": "repo-manager.pending-work-record-projection.v1",
|
||||||
"repo_slug": slug,
|
"repo_slug": slug,
|
||||||
"expected_commit": commit,
|
"expected_commit": commit,
|
||||||
"api_base": api_base.rstrip("/"),
|
"api_base": api_base.rstrip("/"),
|
||||||
|
"acknowledge_retirements": acknowledge_retirements,
|
||||||
|
"expected_instance_label": expected_instance_label,
|
||||||
}
|
}
|
||||||
headers = {
|
headers = {
|
||||||
"Idempotency-Key": f"rmgr-projection:{slug}:{commit}",
|
"Idempotency-Key": f"rmgr-projection:v2:{request_digest}",
|
||||||
"X-StateHub-Source-Agent": "repo-manager",
|
"X-StateHub-Source-Agent": "repo-manager",
|
||||||
}
|
}
|
||||||
try:
|
try:
|
||||||
|
|
@ -192,11 +206,8 @@ def sync_repository_projection(
|
||||||
"pending_path": str(path),
|
"pending_path": str(path),
|
||||||
}
|
}
|
||||||
response = client.post(
|
response = client.post(
|
||||||
f"/repos/{slug}/work-record-projection/reconcile",
|
request_path,
|
||||||
json={
|
json=request_body,
|
||||||
"expected_commit": commit,
|
|
||||||
"acknowledge_retirements": acknowledge_retirements,
|
|
||||||
},
|
|
||||||
headers=headers,
|
headers=headers,
|
||||||
)
|
)
|
||||||
response.raise_for_status()
|
response.raise_for_status()
|
||||||
|
|
@ -224,8 +235,25 @@ def sync_repository_projection(
|
||||||
"error": detail,
|
"error": detail,
|
||||||
}
|
}
|
||||||
|
|
||||||
_pending_path(repo_root).unlink(missing_ok=True)
|
# A successful HTTP response alone is not an exact-commit primary receipt.
|
||||||
outcome_status = receipt.get("outcome", {}).get("status", "applied")
|
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 {
|
return {
|
||||||
"ok": outcome_status in {"applied", "noop"},
|
"ok": outcome_status in {"applied", "noop"},
|
||||||
"status": outcome_status,
|
"status": outcome_status,
|
||||||
|
|
|
||||||
|
|
@ -1,10 +1,12 @@
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import json
|
||||||
import subprocess
|
import subprocess
|
||||||
from datetime import date
|
from datetime import date
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
|
import pytest
|
||||||
|
|
||||||
from repo_manager.commands.task import add_adhoc_task, mutate_task
|
from repo_manager.commands.task import add_adhoc_task, mutate_task
|
||||||
from repo_manager.commands.workplan import create_workplan
|
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(
|
return httpx.Response(
|
||||||
200,
|
200,
|
||||||
json={
|
json=_receipt(payload),
|
||||||
"outcome": {"status": "applied", "counts": {"created": 1}},
|
|
||||||
"derived_commit": payload["expected_commit"],
|
|
||||||
},
|
|
||||||
)
|
)
|
||||||
|
|
||||||
result = sync_repository_projection(
|
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["status"] == "queued"
|
||||||
assert result["reason"] == "wrong_instance"
|
assert result["reason"] == "wrong_instance"
|
||||||
assert Path(result["pending_path"]).is_file()
|
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"
|
||||||
|
|
|
||||||
58
workplans/RMGR-WP-0017-projection-retry-convergence.md
Normal file
58
workplans/RMGR-WP-0017-projection-retry-convergence.md
Normal file
|
|
@ -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.
|
||||||
Loading…
Add table
Add a link
Reference in a new issue