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 <noreply@anthropic.com> Assistant: claude-code Assistant-Model: sonnet Assistant-Process: 169987@bnt-lap001 Assistant-Session: 322ef1ef-9048-4021-8570-b6d6f6347999
This commit is contained in:
parent
71ae259877
commit
c93628a017
6 changed files with 294 additions and 7 deletions
6
SCOPE.md
6
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
|
||||
|
|
|
|||
|
|
@ -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"}:
|
||||
|
|
|
|||
|
|
@ -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"],
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
106
tests/test_reissue.py
Normal file
106
tests/test_reissue.py
Normal file
|
|
@ -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"]
|
||||
103
workplans/APPROVAL-WP-0003-reissue-expired-approvals.md
Normal file
103
workplans/APPROVAL-WP-0003-reissue-expired-approvals.md
Normal file
|
|
@ -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.
|
||||
Loading…
Add table
Add a link
Reference in a new issue