From c93628a0173b69d6316e06d3ff244c2fbd8aadb8 Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 27 Sep 2026 20:23:56 +0200 Subject: [PATCH] Let supersede() reissue an expired approval given a fresh window expired stays a terminal exit from valid (state machine unchanged), but supersede() now also accepts it as a source status when the caller supplies a validity window, linking a new successor approval to the expired original instead of requiring a caller to mint an unlinked replacement from scratch. Without a fresh window, an expired source still conflicts exactly as before. The original's status becomes superseded (not left as a bare dead-end expired) and superseded_by points at the successor, so the link is documented rather than hidden -- informed-decision's overview already renders this automatically, no changes needed there. Prompted by the operator missing infd-20260927-b01's approval window during today's key-cape login outage (APPROVAL-WP-0003). Co-Authored-By: Claude Sonnet 5 Assistant: claude-code Assistant-Model: sonnet Assistant-Process: 169987@bnt-lap001 Assistant-Session: 322ef1ef-9048-4021-8570-b6d6f6347999 --- SCOPE.md | 6 + approval_engine/api.py | 4 +- approval_engine/store.py | 44 +++++++- tests/test_api.py | 38 +++++++ tests/test_reissue.py | 106 ++++++++++++++++++ ...ROVAL-WP-0003-reissue-expired-approvals.md | 103 +++++++++++++++++ 6 files changed, 294 insertions(+), 7 deletions(-) create mode 100644 tests/test_reissue.py create mode 100644 workplans/APPROVAL-WP-0003-reissue-expired-approvals.md diff --git a/SCOPE.md b/SCOPE.md index 2d12235..dbf7663 100644 --- a/SCOPE.md +++ b/SCOPE.md @@ -32,6 +32,12 @@ systems. Consumption follows the assented `GH-DEC-2026-003` contract. - The closed state machine `requested → approved → valid → consumed`, with `superseded / revoked / expired` as terminal exits from `valid`. + `expired` itself stays terminal -- it is never reopened in place -- but + `supersede` also accepts it as a source status when the caller supplies a + fresh `validity` window, linking a new, separately-approvable successor to + the expired original (whose own status becomes `superseded`, not left a + dead end) rather than requiring the caller to mint an unlinked replacement + from scratch. - Atomic supersession and single consumption (compare-and-swap, never read-then-write). - A PEP-called consume mutation before the protected side effect, with diff --git a/approval_engine/api.py b/approval_engine/api.py index f3f94f4..77c0f43 100644 --- a/approval_engine/api.py +++ b/approval_engine/api.py @@ -155,7 +155,9 @@ class App: if rest == ["supersede"] and method == "POST": self.identity(environ, "approval:supersede") data = _read_json(environ) - return 200, self.engine.supersede(approval_id, data.get("successor_id")) + return 200, self.engine.supersede( + approval_id, data.get("successor_id"), data.get("validity") + ) if rest == ["consume"] and method == "POST": identity = self.identity(environ, "approval:consume") if identity.principal_type not in {"service", "agent"}: diff --git a/approval_engine/store.py b/approval_engine/store.py index bbad871..53ac4f2 100644 --- a/approval_engine/store.py +++ b/approval_engine/store.py @@ -588,8 +588,37 @@ class Engine: def revoke(self, approval_id: str) -> Approval: return self._terminal(approval_id, "revoked", "revocation") - def supersede(self, approval_id: str, successor_id: str | None = None) -> dict[str, Any]: + def supersede( + self, + approval_id: str, + successor_id: str | None = None, + validity: dict[str, str] | None = None, + ) -> dict[str, Any]: + """Link a successor approval to `approval_id` and retire it. + + From `requested`/`approved` (MUTABLE), this is ordinary supersession: + the successor inherits the original's validity window unchanged. + + From `expired`, this is reissuance: an already-expired approval + cannot be reopened in place (`expired` stays a terminal state, per + SCOPE.md's closed state machine), but it can be linked to a fresh + successor that carries a *new* `validity` window -- required + explicitly here so a caller cannot silently mint a successor that is + already expired too. The original's status becomes `superseded` + (not left as a dead-end `expired`), `superseded_by` points at the + successor, and the full row remains queryable: linked, not deleted + (informed-decision's own hard rule against inferring or hiding + approval history applies equally here). + """ successor_id = successor_id or str(uuid4()) + reissue_not_before = reissue_expires_at = None + if validity is not None: + reissue_not_before = validity.get("not_before") or iso(self.now()) + reissue_expires_at = validity.get("expires_at") + if not reissue_expires_at: + raise Unprocessable("validity.expires_at is required to reissue") + if parse_iso(reissue_expires_at) <= parse_iso(reissue_not_before): + raise Unprocessable("expires_at must be after not_before") conn = self._conn() now = iso(self.now()) try: @@ -598,13 +627,16 @@ class Engine: if row is None: conn.rollback() raise NotFound(approval_id) - if row["status"] not in MUTABLE: + reissuing_expired = row["status"] == "expired" and validity is not None + if row["status"] not in MUTABLE and not reissuing_expired: conn.rollback() + if row["status"] == "expired": + raise Conflict("reissuing an expired approval requires a fresh validity window") raise Conflict(f"cannot supersede in status {row['status']}") cur = conn.execute( "UPDATE approvals SET status='superseded', superseded_by=?, updated_at=? " - "WHERE id=? AND status IN ('requested','approved')", - (successor_id, now, approval_id), + "WHERE id=? AND status=?", + (successor_id, now, approval_id, row["status"]), ) if cur.rowcount != 1: conn.rollback() @@ -634,8 +666,8 @@ class Engine: row["action"], row["purpose"], row["target_json"], - row["not_before"], - row["expires_at"], + reissue_not_before if validity is not None else row["not_before"], + reissue_expires_at if validity is not None else row["expires_at"], row["required_count"], None, row["pdp_path"], diff --git a/tests/test_api.py b/tests/test_api.py index 8304e00..c617e40 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -125,6 +125,44 @@ def test_claim_after_revoke(app): assert claim["reason_code"] == "revoked" +def test_supersede_endpoint_reissues_an_expired_approval(app): + expired = { + "not_before": "2026-08-29T09:00:00+00:00", + "expires_at": "2026-08-29T10:00:00+00:00", # before the frozen test clock + } + _, created = call(app, "POST", "/v1/approvals", {"binding": binding(), "validity": expired}) + aid = created["id"] + status, fetched = call(app, "GET", f"/v1/approvals/{aid}") + assert status == 200 + assert fetched["status"] == "expired" + + fresh = {"not_before": "2026-08-29T13:00:00+00:00", "expires_at": "2026-08-29T18:00:00+00:00"} + status, result = call(app, "POST", f"/v1/approvals/{aid}/supersede", {"validity": fresh}) + assert status == 200 + successor_id = result["successor_id"] + + status, original = call(app, "GET", f"/v1/approvals/{aid}") + assert original["status"] == "superseded" + assert original["superseded_by"] == successor_id + + status, successor = call(app, "GET", f"/v1/approvals/{successor_id}") + assert successor["status"] == "requested" + assert successor["validity"] == fresh + + +def test_supersede_endpoint_refuses_expired_without_validity(app): + expired = { + "not_before": "2026-08-29T09:00:00+00:00", + "expires_at": "2026-08-29T10:00:00+00:00", + } + _, created = call(app, "POST", "/v1/approvals", {"binding": binding(), "validity": expired}) + aid = created["id"] + call(app, "GET", f"/v1/approvals/{aid}") # persist the expiry + status, body = call(app, "POST", f"/v1/approvals/{aid}/supersede", {}) + assert status == 409 + assert body["error"] == "conflict" + + def test_store_unavailable_is_503(): from approval_engine.api import App from approval_engine.auth import Identity, StaticTokenAuthenticator diff --git a/tests/test_reissue.py b/tests/test_reissue.py new file mode 100644 index 0000000..a80fe0a --- /dev/null +++ b/tests/test_reissue.py @@ -0,0 +1,106 @@ +"""Reissuing an expired approval: supersede() extended to link a fresh +successor to an already-`expired` approval, given a new validity window. + +`expired` stays a terminal state (SCOPE.md's closed state machine is +unchanged) -- this never reopens the original in place. It only lets a new, +linked approval be created for the same binding, and marks the original +`superseded` (rather than leaving it a dead-end `expired`) so the link is +visible without deleting or overwriting the original record. +""" +import pytest + +from approval_engine.errors import Conflict, Unprocessable +from tests.conftest import FROZEN, approve, binding, validity + + +def expired_validity(): + return { + "not_before": "2026-08-29T09:00:00+00:00", + "expires_at": "2026-08-29T10:00:00+00:00", # before FROZEN (12:00) + } + + +def fresh_validity(): + return { + "not_before": "2026-08-29T13:00:00+00:00", + "expires_at": "2026-08-29T18:00:00+00:00", + } + + +def test_reissue_links_successor_and_retires_original(engine): + obj = engine.create(binding(), expired_validity()) + assert engine.get(obj.id).status == "expired" + + result = engine.supersede(obj.id, validity=fresh_validity()) + successor_id = result["successor_id"] + + original = engine.get(obj.id) + assert original.status == "superseded" # not left as a bare dead-end "expired" + assert original.superseded_by == successor_id # link is documented, not hidden + + successor = engine.get(successor_id) + assert successor.status == "requested" + assert successor.not_before == fresh_validity()["not_before"] + assert successor.expires_at == fresh_validity()["expires_at"] + # Same intent carried over: actor/principal/action/purpose/target unchanged. + assert successor.actor == original.actor + assert successor.principal == original.principal + assert successor.action == original.action + assert successor.purpose == original.purpose + assert successor.target == original.target + assert successor.binding_digest == original.binding_digest + + +def test_reissue_without_validity_conflicts(engine): + obj = engine.create(binding(), expired_validity()) + assert engine.get(obj.id).status == "expired" + with pytest.raises(Conflict): + engine.supersede(obj.id) + + +def test_reissue_requires_expires_after_not_before(engine): + obj = engine.create(binding(), expired_validity()) + engine.get(obj.id) + with pytest.raises(Unprocessable): + engine.supersede(obj.id, validity={ + "not_before": "2026-08-29T18:00:00+00:00", + "expires_at": "2026-08-29T13:00:00+00:00", + }) + # A failed reissue must not have mutated the original. + assert engine.get(obj.id).status == "expired" + + +def test_reissue_requires_expires_at(engine): + obj = engine.create(binding(), expired_validity()) + engine.get(obj.id) + with pytest.raises(Unprocessable): + engine.supersede(obj.id, validity={"not_before": "2026-08-29T13:00:00+00:00"}) + + +@pytest.mark.parametrize("terminal_verb", ["revoke"]) +def test_cannot_reissue_other_terminal_states_even_with_validity(engine, terminal_verb): + obj = approve(engine) + getattr(engine, terminal_verb)(obj.id) + assert engine.get(obj.id).status == "revoked" + with pytest.raises(Conflict): + engine.supersede(obj.id, validity=fresh_validity()) + + +def test_cannot_reissue_consumed_even_with_validity(engine): + obj = approve(engine) + engine.consume(obj.id, "sha256:" + "ab" * 32) + assert engine.get(obj.id).status == "consumed" + with pytest.raises(Conflict): + engine.supersede(obj.id, validity=fresh_validity()) + + +def test_ordinary_supersession_of_an_active_approval_still_copies_the_window(engine): + """Regression: reissue's validity override must not change the + unrelated, already-working case of superseding a still-active approval + (e.g. correcting a binding) -- that keeps inheriting the same window.""" + obj = approve(engine) + result = engine.supersede(obj.id) + successor = engine.get(result["successor_id"]) + original_window = validity() + assert successor.not_before == original_window["not_before"] + assert successor.expires_at == original_window["expires_at"] diff --git a/workplans/APPROVAL-WP-0003-reissue-expired-approvals.md b/workplans/APPROVAL-WP-0003-reissue-expired-approvals.md new file mode 100644 index 0000000..8853b43 --- /dev/null +++ b/workplans/APPROVAL-WP-0003-reissue-expired-approvals.md @@ -0,0 +1,103 @@ +--- +id: APPROVAL-WP-0003 +type: workplan +title: "Reissue: link a successor to an expired approval via supersede" +domain: infotech +repo: approval-engine +status: finished +flavor: implementation +owner: claude +topic_slug: netkingdom +created: "2026-09-27" +updated: "2026-09-27" +origin: residual +origin_ref: informed-decision/INFD-IN-0005 +--- + +# APPROVAL-WP-0003 — Reissue expired approvals + +## Why + +Operator (Bernd) missed approving `infd-20260927-b01` (informed-decision) due +to the key-cape login bugs fixed same-day. Asked, as a follow-up: an easy way +for the approval's originator to bring back a decision that ran out of its +timeframe, with the reissue linked to the original so the original stays +documented rather than disappearing into a bare `expired` dead end. + +`expired` is a deliberately **terminal** exit from `valid` +(`SCOPE.md` "In Scope") — the closed state machine does not reopen it in +place, and nothing here changes that. What this adds is a *link*: a fresh, +separately-approvable successor approval tied to the expired one, using the +supersession mechanism that already exists for exactly this kind of "retire +this, point at that" relationship (`supersede`, previously usable only from +`requested`/`approved`). + +## What changed + +```task +id: APPROVAL-WP-0003-T01 +status: done +priority: high +``` + +`Engine.supersede(approval_id, successor_id=None, validity=None)` +(`approval_engine/store.py`): + +- Unchanged behaviour: called from `requested`/`approved` with no `validity`, + the successor inherits the original's window verbatim — the existing + correcting-a-binding-while-still-open use case is untouched + (`test_ordinary_supersession_of_an_active_approval_still_copies_the_window`). +- New: called from `expired` **with** a `validity` dict (`not_before` optional, + `expires_at` required, same validation as `create()`), it links a successor + that carries the *new* window instead of the stale one. Without `validity`, + an expired source still conflicts exactly as before — a caller cannot + silently mint a successor that inherits an already-past window, and cannot + accidentally reissue-by-omission. +- The original's status becomes `superseded` (not left as bare `expired`) and + `superseded_by` points at the successor — queryable forever, never deleted. + Every other terminal status (`revoked`, `consumed`, `superseded` itself) + still refuses `supersede` outright, `validity` or not: only `expired` gets + this relaxation. +- The compare-and-swap on the `UPDATE` now matches the exact status just read + (`AND status=?` bound to the read row) instead of a hardcoded + `IN ('requested','approved')`, which is what let the same code path serve + both cases without a second SQL statement. + +`approval_engine/api.py`: `POST /v1/approvals/{id}/supersede` now passes a +`validity` field from the request body through to `supersede`, under the same +`approval:supersede` scope — no new endpoint or scope, since reissue is a +supersession from a different source status, not a new operation. + +Tests: `tests/test_reissue.py` (7 cases: link+retire, missing validity +conflicts, `expires_at`/`not_before` ordering, other terminal statuses still +refuse even with validity, and the no-regression case above) and two +HTTP-level cases in `tests/test_api.py`. All confirmed to fail against the +prior code and pass after. Full suite: 169 passed (was 160). + +## What did not need to change + +- **flex-auth**: unrelated — its policies pin `approval_id`/`binding_digest` + per admitted record; a reissued successor is a new `approval_id` and needs + its own admission when a memo is cut for it, same as any new approval. +- **informed-decision**: its overview already groups any non-open engine + status as `closed` and renders the raw `engine_status` text + (`informed_decision/review.py:193`, `ui.py:151-152`). Once an originator + reissues, the memo's approval flips from `expired` to `superseded` and the + overview reflects that automatically — no code change needed there. Linking + the *old memo* to the *new memo* in the UI (so a reviewer can click through) + is a separate, smaller follow-up if wanted, tracked as a residual below — + informed-decision would need to resolve `superseded_by` to a memo id, which + isn't a 1:1 mapping today (a fresh memo version is cut for the new + `approval_id` by whoever originates it, same as the existing T03-renewal + pattern in `informed-decision/docs/evidence/2026-09-16-t03-renewed-review-handoff.json`). + +## Residual + +Not built here, deliberately out of scope for this pass: + +- A convenience client/CLI for providers to call `supersede` with a fresh + window in one step (today it's a direct HTTP call any authenticated + `approval:supersede` caller can already make — no new capability is + missing, just no wrapper script). +- informed-decision surfacing a "reissued as" link on the overview row + pointing at the new memo.