diff --git a/CHANGELOG.md b/CHANGELOG.md index ad84b89..4cbdad7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,57 @@ skip versions are the composition of each intervening adjacent release's steps in order — no A-to-B path is pre-computed beyond that. +## 0.32.0 — 2026-06-01 + +**Minor — graduation's integer RFC number is now optional, and RFC +owners + site owners can retire (soft-delete) RFCs. Schema migration +`025_retired_state.sql` runs automatically on deploy; a frontend rebuild +applies the UI.** + +Two changes to the §13 lifecycle: + +1. **Optional number at graduation (§13.2/§13.3).** Graduation no longer + hard-requires a valid `RFC-NNNN`. The Graduate dialog's integer-ID + field is now optional: it is still pre-filled with the next free + number as a *suggestion*, but the graduating owner may clear it and + graduate with **no number**. When blank, the entry flips to `active` + with `id: null` and the **slug remains the canonical identifier** + (§2.3). `GET …/graduate/check` treats a blank id as valid (`ok:true`); + `POST …/graduate` accepts a blank/absent `rfc_id` and only validates + the `^RFC-\d{4,}$` regex + collision check when a number *is* supplied. + The catalog and RFC view render number-less active entries by their + slug/title (no "RFC-undefined"). RFC-0001's existing number is + grandfathered — `cached_rfcs.rfc_id` was already nullable, so no data + migration was needed for this part. + +2. **Retire / un-retire — soft delete (§3, §3.1, §13.7).** A fourth entry + state, `retired`, is added. An RFC's own owners (frontmatter) and site + `owner`-role holders — **not** app admins — may retire an entry + (`POST /api/rfcs//retire`); it flips to `retired` via an + auto-merged meta-repo PR, leaving the body and every other field + (including any integer id) intact. A retired entry is removed from + **every** browsing surface: the catalog, the RFC/discussion/branch + views (404), and link pickers/search. It is *not* hard-deleted — the + entry stays in `rfcs/` (git is truth). Un-retire + (`POST /api/rfcs//unretire`) restores the prior state and is + **site-owner-only**, so a soft-delete is always recoverable by the + operator but an RFC owner cannot reverse their own retirement. Site + owners find retired entries via a new owner-gated admin surface + (`GET /api/admin/retired-rfcs`, the "Retired" tab) and un-retire from + there or by navigating directly to the entry. + +Upgrade steps: + +- **MUST** run the migration. `025_retired_state.sql` rebuilds + `cached_rfcs` to widen its `state` CHECK constraint to include + `retired` (SQLite cannot alter a CHECK in place). It runs automatically + at startup via the forward-only migration runner; existing rows are + preserved, and the table is reconstructible from Gitea by the + reconciler regardless (§4). No operator action beyond a normal deploy. +- **SHOULD** rebuild the frontend so the optional-id Graduate dialog, the + Retire affordance, and the owner-only "Retired" admin tab are present. +- No config or secret changes. + ## 0.31.4 — 2026-06-01 **Patch — bug fix + UI polish: secondary buttons that were invisible on diff --git a/SPEC.md b/SPEC.md index d204abe..0242b02 100644 --- a/SPEC.md +++ b/SPEC.md @@ -160,19 +160,31 @@ an idea costs nothing in identifier space. ## 3. RFC states -There are three canonical states stored in entry frontmatter: +There are four canonical states stored in entry frontmatter: - **`super-draft`** — the entry exists in the meta repo's `rfcs/` directory. Anyone signed in can chat on it; anyone can claim ownership; an owner is required before graduation. -- **`active`** — the entry has been graduated: it carries an integer - `id` (`RFC-NNNN`) and `graduated_at`/`graduated_by`. It lives in the - same `rfcs/.md` entry it always did — graduation is an in-place - state flip (§13), not a move. Branches, PRs, and conversation happen on - the meta repo against that entry, exactly as they did while it was a +- **`active`** — the entry has been graduated: it carries + `graduated_at`/`graduated_by` and, **optionally**, an integer `id` + (`RFC-NNNN`). The integer id is not required — graduation may proceed + with no number, in which case `id` stays null and the **slug remains + the canonical identifier** (§2.3, §13.2). It lives in the same + `rfcs/.md` entry it always did — graduation is an in-place state + flip (§13), not a move. Branches, PRs, and conversation happen on the + meta repo against that entry, exactly as they did while it was a super-draft. `repo:` stays null (§1). - **`withdrawn`** — pulled before becoming canonical. Stays in the directory as historical record, hidden from default views, filterable in. +- **`retired`** — soft-deleted. The entry stays in the `rfcs/` directory + as historical record (git is truth — nothing is hard-deleted), but it + is **removed from every UI surface**: it does not appear in the + catalog, is not addressable through the ordinary RFC view, and is + excluded from link pickers and search. Unlike `withdrawn` (which is + merely hidden-by-default and filterable back in), `retired` is not + surfaced anywhere a contributor browses. Retiring is for entries an + owner wants gone from the working set; it is reversible only by a site + owner (§13.7). A fourth concept — **`idea`** — is not stored in frontmatter. It is the *derived view* of "there is an open PR against the meta repo proposing @@ -192,12 +204,28 @@ super-draft ──[graduate, owner or admin]─────▶ active super-draft ──[withdraw, proposer or owner/admin]──▶ withdrawn active ──[withdraw, owner/admin]─────────────▶ withdrawn withdrawn ──[reopen, owner/admin]────────────▶ super-draft +super-draft ──[retire, RFC owner or site owner]──▶ retired +active ──[retire, RFC owner or site owner]───▶ retired +retired ──[un-retire, site owner]────────────▶ prior state ``` Every transition is a commit to the meta repo. State history is auditable through `git log rfcs/.md`. The app maintains a separate audit log for "who clicked the button" (see §6.5). +**Who may retire.** Retire (§13.7) is deliberately *narrower* than +withdraw: it is available to the RFC's own `owners` (frontmatter) and to +holders of the site **`owner`** role only — **not** to app admins. It is +the one destructive-feeling action in the lifecycle (the entry leaves the +working set entirely), so the authority to perform it is held closer than +withdraw's owner/admin set. Un-retire — bringing a retired entry back to +the state it held before — is held tighter still: **site owners only**. +An RFC owner can retire their own entry but cannot bring it back; that +asymmetry is intentional, so a soft-delete is always recoverable by the +site's operator. The entry file is never removed from `rfcs/`, so a +retired entry remains recoverable by editing frontmatter directly even if +the app surfaces were lost. + ### 3.2 State change side-effects Changing state in the meta repo entry is the *only* required operation @@ -2034,9 +2062,14 @@ broadening rather than a precondition for the proposer's own RFC.) Clicking "Graduate" opens a small dialog with two editable fields: -- **Integer ID** — pre-filled as `max(existing integer IDs) + 1`, - formatted as `RFC-NNNN`. Editable to allow gap reservations but the - default is just the next number. +- **Integer ID** — **optional.** Pre-filled as `max(existing integer + IDs) + 1`, formatted as `RFC-NNNN`, purely as a *suggestion* the + graduating owner may keep, change (to reserve gaps), or **clear + entirely**. Leaving it blank graduates the entry to `active` with no + number — `id` stays null and the slug remains the canonical identifier + (§2.3). The field's helper text states this: blank means "graduate + without a number." A blank id is the default-accepted case, not an + error. - **Initial owners** — pre-filled from the entry's `owners:`, with an "add owner" picker. Must have at least one. @@ -2044,17 +2077,20 @@ Clicking "Graduate" opens a small dialog with two editable fields: the meta-only topology.) Each field validates inline as the admin types, with a short -debounce, against the catalog cache and a regex — integer-ID -collision against existing IDs, the at-least-one-owner constraint on -the picker. Errors render as a short line of text beneath the -offending field. The integer-ID collision check is re-issued -atomically server-side on confirm, since a concurrent graduation -could land between dialog-open and submit. While any field is -invalid, the confirm button is disabled and its tooltip names the -first blocker specifically — "Integer ID 42 is already taken," "Add -at least one initial owner" — the same grammar the precondition -popover below uses, so the dialog and the gate read as one surface -rather than two competing styles. +debounce, against the catalog cache and a regex. The integer-ID field +is valid when it is **blank** (graduate without a number) *or* matches +`^RFC-\d{4,}$` and is not already taken by another entry; the owners +picker enforces the at-least-one constraint. Errors render as a short +line of text beneath the offending field. The integer-ID collision +check is re-issued atomically server-side on confirm, since a +concurrent graduation could land between dialog-open and submit (only +relevant when a number was supplied — a blank id can never collide). +While any field is invalid, the confirm button is disabled and its +tooltip names the first blocker specifically — "Integer ID 42 is +already taken," "Add at least one initial owner" — the same grammar the +precondition popover below uses, so the dialog and the gate read as one +surface rather than two competing styles. A missing number is **never** +a blocker. The dialog's confirm button is also disabled when the entry has no owners. The disabled button opens a small popover on hover or click @@ -2070,14 +2106,16 @@ kept, so they coexist with graduation. Confirming the dialog runs a single operation as the bot: open a PR against the meta repo that re-serializes `rfcs/.md` with -`state: active`, `id: RFC-NNNN`, `graduated_at: `, -`graduated_by: `, and the `owners:` from the dialog — -**leaving the body unchanged** — then auto-merge it (the admin who -clicked is the merge actor). The webhook flow updates the SQLite cache -and the catalog row transitions per §7.2. +`state: active`, `id: RFC-NNNN` **or `id: null` when no number was +supplied**, `graduated_at: `, `graduated_by: `, and the `owners:` from the dialog — **leaving the body +unchanged** — then auto-merge it (the admin who clicked is the merge +actor). The webhook flow updates the SQLite cache and the catalog row +transitions per §7.2. When `id` is null, the catalog and RFC view +identify the entry by its slug (no "RFC-NNNN" chip is rendered). ``` -super-draft entry ──[graduate]──▶ same entry, state: active, id assigned +super-draft entry ──[graduate]──▶ same entry, state: active, id assigned OR null (body unchanged, repo: null, lives in rfcs/.md throughout) ``` @@ -2144,6 +2182,41 @@ the canonical home in the app. RFC-0001 is therefore an ordinary meta-only active RFC like any other; no grandfathered per-repo path remains in the code. +### 13.7 Retire (soft delete) + +Retire takes an entry out of the working set without destroying it. It +is the same shape as the other lifecycle transitions — an in-place +frontmatter flip on the meta entry, committed via an auto-merged bot PR +(§13.3's machinery, reused) — but with two distinguishing rules: + +- **Authority (§3.1).** Only the RFC's own `owners` (frontmatter) and + holders of the site `owner` role may retire. App admins may *not* + (this is the one lifecycle action where admin authority does not + apply). Un-retire is **site owners only**: an RFC owner can retire but + cannot reverse it, so every soft-delete remains recoverable by the + operator. +- **Visibility.** A `retired` entry is removed from **every** browsing + surface — the catalog (`GET /api/rfcs`), the ordinary RFC view (`GET + /api/rfcs/` returns 404 for everyone except a site owner, so the + un-retire affordance has somewhere to live), link pickers, and search. + This is stronger than `withdrawn`, which is merely hidden-by-default + and filterable back in. + +The flip sets `state: retired` and leaves every other field — including +the integer `id`, if one was assigned — untouched, so an un-retire +restores the entry exactly as it was. Retire is allowed from +`super-draft` or `active`; the prior state is recorded in the audit log +(`retire` action) so un-retire (`unretire` action) can restore it. +Because the entry file is never removed from `rfcs/`, the meta repo +remains the source of truth and the reconciler reproduces the +`retired` state on every sweep. + +A site owner finds retired entries through an admin surface (a +"Retired" list, owner-gated) and un-retires from there; the entry then +reappears in the catalog under its restored state. The integer `id`, +once assigned, is never reclaimed or reissued by retire/un-retire +(gap-tolerant allocation per §2.3). + --- ## 14. Outside the RFC view: landing and the philosophy surface diff --git a/VERSION b/VERSION index a8a0217..9eb2aa3 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.31.4 +0.32.0 diff --git a/backend/app/api.py b/backend/app/api.py index 0d77109..676c23c 100644 --- a/backend/app/api.py +++ b/backend/app/api.py @@ -657,12 +657,19 @@ def make_router( return {"items": items} @router.get("/api/rfcs/{slug}") - async def get_rfc(slug: str) -> dict[str, Any]: + async def get_rfc(slug: str, request: Request) -> dict[str, Any]: row = db.conn().execute( "SELECT * FROM cached_rfcs WHERE slug = ?", (slug,) ).fetchone() if row is None: raise HTTPException(404, "Not found") + # §13.7: a retired entry is removed from every browsing surface. + # The sole exception is a site owner, so the un-retire affordance + # has somewhere to live; everyone else gets a plain 404. + if row["state"] == "retired": + viewer = auth.current_user(request) + if viewer is None or viewer.role != "owner": + raise HTTPException(404, "Not found") payload = _serialize_rfc(row) # Roadmap #26: surface the optional propose-time use case on the # RFC view. The idea PR closes on merge, but the canonical row in diff --git a/backend/app/api_admin.py b/backend/app/api_admin.py index f2edf23..3e217ee 100644 --- a/backend/app/api_admin.py +++ b/backend/app/api_admin.py @@ -829,6 +829,57 @@ def make_router(config: Config) -> APIRouter: items_blocked.append(payload) return {"ready": items_ready, "blocked": items_blocked} + # ----- §13.7: retired (soft-deleted) entries — site owners only ----- + + @router.get("/api/admin/retired-rfcs") + async def retired_rfcs(request: Request) -> dict[str, Any]: + # Retired entries are hidden from every browsing surface (§13.7); + # this owner-gated list is how a site owner discovers them to + # un-retire. Admins do not get this surface — un-retire authority is + # site-owner-only, so neither is the list that feeds it. + viewer = auth.require_admin(request) + if viewer.role != "owner": + raise HTTPException(403, "Site owner role required") + rows = db.conn().execute( + """ + SELECT slug, title, rfc_id, owners_json, tags_json, + proposed_at, updated_at + FROM cached_rfcs + WHERE state = 'retired' + ORDER BY updated_at DESC + """ + ).fetchall() + items = [] + for r in rows: + # The state it would return to on un-retire, mirroring + # api_graduation._prior_state_before_retire's audit lookup. + prior = db.conn().execute( + """ + SELECT details FROM actions + WHERE rfc_slug = ? AND action_kind = 'retire' + ORDER BY id DESC LIMIT 1 + """, + (r["slug"],), + ).fetchone() + restored_state = None + if prior and prior["details"]: + try: + restored_state = json.loads(prior["details"]).get("prior_state") + except (ValueError, TypeError): + restored_state = None + if restored_state not in ("super-draft", "active"): + restored_state = "active" if r["rfc_id"] else "super-draft" + items.append({ + "slug": r["slug"], + "title": r["title"], + "id": r["rfc_id"], + "owners": json.loads(r["owners_json"] or "[]"), + "tags": json.loads(r["tags_json"] or "[]"), + "retired_at": r["updated_at"], + "restores_to": restored_state, + }) + return {"items": items} + # ----- User search (typeahead for §15.8 mute add) ----- @router.get("/api/users/search") diff --git a/backend/app/api_discussion.py b/backend/app/api_discussion.py index 8153096..755ae22 100644 --- a/backend/app/api_discussion.py +++ b/backend/app/api_discussion.py @@ -256,6 +256,10 @@ def _require_rfc_readable(slug: str): raise HTTPException(404, "RFC not found") if row["state"] == "withdrawn": raise HTTPException(409, "RFC is withdrawn") + # §13.7: a retired entry is soft-deleted — refuse reads of every shape + # (a 404, not a 409: the entry is not surfaced anywhere a browser looks). + if row["state"] == "retired": + raise HTTPException(404, "RFC not found") return row diff --git a/backend/app/api_graduation.py b/backend/app/api_graduation.py index 6933145..69bf13c 100644 --- a/backend/app/api_graduation.py +++ b/backend/app/api_graduation.py @@ -19,7 +19,10 @@ Routes (§17): - GET /api/rfcs//blocking-prs (informational; no longer a graduation precondition) -Plus the §13.1 claim PR endpoint (POST /api/rfcs//claim). +Plus the §13.1 claim PR endpoint (POST /api/rfcs//claim) and the +§13.7 retire / un-retire endpoints (POST /api/rfcs//retire, +POST /api/rfcs//unretire) — soft-delete and its site-owner-only +reversal, each a single in-place frontmatter flip via an auto-merged PR. The orchestrator runs in-process — each in-flight graduation lives in a small `GraduationState` keyed by slug, with an asyncio.Queue feeding the @@ -74,7 +77,7 @@ class StepState: @dataclass class GraduationState: slug: str - rfc_id: str + rfc_id: str | None # null when graduating without a number (§13.2) owners: list[str] arbiters: list[str] steps: list[StepState] @@ -113,7 +116,7 @@ def _get_active(slug: str) -> GraduationState | None: return _active.get(slug) -def _new_active(slug: str, *, rfc_id: str, +def _new_active(slug: str, *, rfc_id: str | None, owners: list[str], arbiters: list[str]) -> GraduationState: state = GraduationState( slug=slug, rfc_id=rfc_id, @@ -164,7 +167,12 @@ def _rfc_id_taken(rfc_id: str, *, excluding_slug: str) -> bool: class GraduateBody(BaseModel): - rfc_id: str = Field(min_length=5, max_length=40) + # §13.2: the integer id is OPTIONAL. Blank/absent → graduate with no + # number (id stays null, the slug is the canonical identifier per + # §2.3). When supplied it must match ^RFC-\d{4,}$ and be free — both + # are checked in the handler, not by the field bound (an empty string + # is a legitimate value here). + rfc_id: str | None = Field(default=None, max_length=40) owners: list[str] = Field(min_length=1) @@ -257,14 +265,19 @@ def make_router( candidate_id = (request.query_params.get("id") or "").strip() owners = json.loads(rfc["owners_json"] or "[]") - # ID field + # ID field — §13.2: the integer id is OPTIONAL. A blank id is + # valid and means "graduate without a number" (id stays null, the + # slug is canonical per §2.3). Only a *non-blank* id is held to the + # RFC-NNNN regex and the collision check. id_payload: dict[str, Any] = {"value": candidate_id, "ok": True, "error": None} if not candidate_id: - id_payload["ok"] = False - id_payload["error"] = "Integer ID is required" + pass # blank is the default-accepted case, not an error elif not _is_valid_rfc_id(candidate_id): id_payload["ok"] = False - id_payload["error"] = "ID must look like RFC-NNNN (at least four digits)" + id_payload["error"] = ( + "ID must look like RFC-NNNN (at least four digits), " + "or leave blank to graduate without a number" + ) elif _rfc_id_taken(candidate_id, excluding_slug=slug): id_payload["ok"] = False id_payload["error"] = f"Integer ID {candidate_id} is already taken" @@ -315,14 +328,18 @@ def make_router( # §13.2 atomic re-validation. The dialog's debounced check runs # client-side as the admin types; this is the authoritative check # that closes the dialog-open-to-confirm race on the integer ID. - rfc_id = body.rfc_id.strip() + # The id is OPTIONAL: a blank/absent id graduates with no number + # (id stays null, slug is canonical per §2.3). Only a supplied id + # is held to the regex and the collision re-check. + rfc_id = (body.rfc_id or "").strip() or None owners = [o.strip() for o in body.owners if o.strip()] if not owners: raise HTTPException(422, "Add at least one initial owner") - if not _is_valid_rfc_id(rfc_id): - raise HTTPException(422, "ID must look like RFC-NNNN (at least four digits)") - if _rfc_id_taken(rfc_id, excluding_slug=slug): - raise HTTPException(409, f"Integer ID {rfc_id} is already taken") + if rfc_id is not None: + if not _is_valid_rfc_id(rfc_id): + raise HTTPException(422, "ID must look like RFC-NNNN (at least four digits)") + if _rfc_id_taken(rfc_id, excluding_slug=slug): + raise HTTPException(409, f"Integer ID {rfc_id} is already taken") # Read the meta-repo entry once — we need the file's sha for the # graduation PR's update_file call and the body to carry through @@ -482,6 +499,70 @@ def make_router( await cache.refresh_meta_pulls(config, gitea) return {"pr_number": pr["number"], "slug": slug, "branch_name": pr["head"]["ref"]} + # ------------------------------------------------------------------- + # §13.7: POST /api/rfcs//retire + # Soft-delete an entry — flip its frontmatter `state` to `retired` via + # an auto-merged meta-repo PR (§13.3 machinery reused). RFC owners and + # site owners only — NOT app admins (§3.1). Allowed from super-draft or + # active; the body and every other field (including the integer id) are + # kept, so the entry stays recoverable in git and un-retire restores it + # exactly. + # ------------------------------------------------------------------- + + @router.post("/api/rfcs/{slug}/retire") + async def retire_rfc(slug: str, request: Request) -> dict[str, Any]: + viewer = auth.require_contributor(request) + rfc = _require_retirable(slug) + if not _can_retire(rfc, viewer): + raise HTTPException( + 403, "Only this RFC's owners or a site owner may retire it" + ) + prior_state = rfc["state"] + entry, sha = await _read_meta_entry(slug) + entry.state = "retired" + await _run_state_flip( + config=config, gitea=gitea, bot=bot, actor=viewer.as_actor(), + slug=slug, new_contents=entry_mod.serialize(entry), prior_sha=sha, + verb="retire", target_state="retired", + ) + _audit( + viewer.user_id, viewer.gitea_login, "retire", + rfc_slug=slug, + details={"prior_state": prior_state, "rfc_id": entry.id}, + ) + await _refresh_catalog() + return {"ok": True, "slug": slug, "state": "retired"} + + # ------------------------------------------------------------------- + # §13.7: POST /api/rfcs//unretire + # Bring a retired entry back to the state it held before retirement. + # Site owners only (§3.1) — tighter than retire itself, so a soft-delete + # is always recoverable by the operator but an RFC owner cannot reverse + # their own retirement. + # ------------------------------------------------------------------- + + @router.post("/api/rfcs/{slug}/unretire") + async def unretire_rfc(slug: str, request: Request) -> dict[str, Any]: + viewer = auth.require_user(request) + if viewer.role != "owner": + raise HTTPException(403, "Only a site owner may un-retire an RFC") + _require_retired(slug) + restored = _prior_state_before_retire(slug) + entry, sha = await _read_meta_entry(slug) + entry.state = restored + await _run_state_flip( + config=config, gitea=gitea, bot=bot, actor=viewer.as_actor(), + slug=slug, new_contents=entry_mod.serialize(entry), prior_sha=sha, + verb="unretire", target_state=restored, + ) + _audit( + viewer.user_id, viewer.gitea_login, "unretire", + rfc_slug=slug, + details={"restored_state": restored}, + ) + await _refresh_catalog() + return {"ok": True, "slug": slug, "state": restored} + # ------------------------------------------------------------------- # Helpers # ------------------------------------------------------------------- @@ -494,6 +575,45 @@ def make_router( raise HTTPException(409, f"RFC is {row['state']}, not super-draft") return row + def _require_retirable(slug: str): + row = db.conn().execute("SELECT * FROM cached_rfcs WHERE slug = ?", (slug,)).fetchone() + if row is None: + raise HTTPException(404, "RFC not found") + if row["state"] not in ("super-draft", "active"): + raise HTTPException(409, f"RFC is {row['state']}, cannot be retired") + return row + + def _require_retired(slug: str): + row = db.conn().execute("SELECT * FROM cached_rfcs WHERE slug = ?", (slug,)).fetchone() + if row is None: + raise HTTPException(404, "RFC not found") + if row["state"] != "retired": + raise HTTPException(409, f"RFC is {row['state']}, not retired") + return row + + async def _read_meta_entry(slug: str) -> tuple[entry_mod.Entry, str]: + fetched = await gitea.read_file( + config.gitea_org, config.meta_repo, f"rfcs/{slug}.md", ref="main", + ) + if fetched is None: + raise HTTPException(409, f"Meta entry rfcs/{slug}.md not found on main") + text, file_sha = fetched + try: + return entry_mod.parse(text), file_sha + except Exception as e: + raise HTTPException(500, f"Meta entry malformed: {e}") + + async def _refresh_catalog() -> None: + # Inline refresh so the catalog reflects the flip immediately; the + # webhook/reconciler path is the steady-state per §4.1. A refresh + # failure does not unwind the merge — the reconciler catches up. + try: + await cache.refresh_meta_repo(config, gitea) + await cache.refresh_meta_branches(config, gitea) + await cache.refresh_meta_pulls(config, gitea) + except Exception as e: + log.warning("retire/unretire cache refresh failed for slug: %s", e) + return router @@ -673,6 +793,100 @@ def _can_graduate(rfc, viewer) -> bool: return viewer.gitea_login in owners or viewer.gitea_login in arbiters +def _can_retire(rfc, viewer) -> bool: + """§3.1: retire is narrower than graduate/withdraw — RFC owners + (frontmatter) and site `owner`-role holders only, NOT app admins.""" + if viewer is None: + return False + if viewer.role == "owner": # site owner + return True + owners = json.loads(rfc["owners_json"] or "[]") + return viewer.gitea_login in owners + + +def _prior_state_before_retire(slug: str) -> str: + """§13.7: the state to restore on un-retire. Read it from the most + recent `retire` audit row's `prior_state`; if that's missing (e.g. the + entry was retired out of band), fall back to inferring from the cached + row — an assigned integer id implies it was `active`, else `super-draft`. + """ + row = db.conn().execute( + """ + SELECT details FROM actions + WHERE rfc_slug = ? AND action_kind = 'retire' + ORDER BY id DESC LIMIT 1 + """, + (slug,), + ).fetchone() + if row and row["details"]: + try: + prior = json.loads(row["details"]).get("prior_state") + if prior in ("super-draft", "active"): + return prior + except (ValueError, TypeError): + pass + cached = db.conn().execute( + "SELECT rfc_id FROM cached_rfcs WHERE slug = ?", (slug,) + ).fetchone() + return "active" if (cached and cached["rfc_id"]) else "super-draft" + + +async def _run_state_flip( + *, + config: Config, + gitea: Gitea, + bot: Bot, + actor: Actor, + slug: str, + new_contents: str, + prior_sha: str, + verb: str, + target_state: str, +) -> None: + """§13.7: open + merge a retire / un-retire frontmatter flip PR. Runs + inline (no SSE — the flip is a single quick state change, unlike the + multi-step graduation that streams progress). On an open failure + nothing was created; on a merge failure the half-open PR/branch is + cleaned up. Either failure raises 502 so the caller does not record the + transition as having happened.""" + try: + pr = await bot.open_retire_flip_pr( + actor, + org=config.gitea_org, meta_repo=config.meta_repo, + slug=slug, new_file_contents=new_contents, prior_sha=prior_sha, + verb=verb, target_state=target_state, + ) + except GiteaError as e: + raise HTTPException(502, f"Gitea: {e.detail}") + pr_number = pr["number"] + head_branch = pr["head"]["ref"] + try: + await bot.merge_retire_flip_pr( + actor, + org=config.gitea_org, meta_repo=config.meta_repo, + pr_number=pr_number, head_branch=head_branch, + slug=slug, verb=verb, + ) + except GiteaError as e: + # Clean up the unmerged flip PR/branch so failed attempts don't + # accumulate (mirrors graduation's `_cleanup_unmerged`). + try: + await bot.close_graduation_pr( + actor, org=config.gitea_org, meta_repo=config.meta_repo, + pr_number=pr_number, head_branch=head_branch, + slug=slug, reason=f"{verb} merge failed", + ) + await bot.delete_branch( + actor, owner=config.gitea_org, repo=config.meta_repo, + branch=head_branch, slug=slug, + action_kind="delete_post_merge_branch", + reason=f"{verb} merge failed", + ) + except Exception: + log.exception("%s cleanup failed for %s", verb, slug) + raise HTTPException(502, f"Gitea: {e.detail}") + + def _audit( actor_user_id: int | None, on_behalf_of: str | None, diff --git a/backend/app/bot.py b/backend/app/bot.py index 0a874b0..4436a92 100644 --- a/backend/app/bot.py +++ b/backend/app/bot.py @@ -706,22 +706,24 @@ class Bot: slug: str, new_file_contents: str, prior_sha: str, - rfc_id: str, + rfc_id: str | None, owners: list[str], ) -> dict: """§13.3 (meta-only): open a PR against the meta repo that flips the - entry's frontmatter to `state: active` with the integer `id` and - graduation stamps — **keeping the body unchanged** (§1 meta-only - topology; no repo is created and no body is stripped). Branch name - uses the `graduate--<6hex>` shape — dash-separated like the - other meta-repo branches per the §19.2 path-routing candidate. + entry's frontmatter to `state: active` with the graduation stamps + and — **optionally** — the integer `id`, **keeping the body + unchanged** (§1 meta-only topology; no repo is created and no body + is stripped). When `rfc_id` is None the entry graduates without a + number (id stays null, slug is canonical per §2.3, §13.2). Branch + name uses the `graduate--<6hex>` shape — dash-separated like + the other meta-repo branches per the §19.2 path-routing candidate. """ import secrets branch = f"graduate-{slug}-{secrets.token_hex(3)}" await self._gitea.create_branch(org, meta_repo, branch, from_branch="main") ae = actor.email or f"{actor.gitea_login}@users.noreply" - commit_subject = f"Graduate {slug} → {rfc_id}" + commit_subject = f"Graduate {slug} → {rfc_id}" if rfc_id else f"Graduate {slug} (no number)" commit_message = _stamp_single(commit_subject, actor) result = await self._gitea.update_file( org, meta_repo, f"rfcs/{slug}.md", @@ -736,11 +738,12 @@ class Bot: or result.get("content", {}).get("sha") or "" ) - pr_title = f"Graduate {slug} → {rfc_id}" + pr_title = f"Graduate {slug} → {rfc_id}" if rfc_id else f"Graduate {slug} (no number)" owners_str = ", ".join(owners) if owners else "(none)" + id_line = f"- ID: `{rfc_id}`\n" if rfc_id else "- ID: (none — identified by slug)\n" pr_body_text = ( f"Graduates super-draft `{slug}` to active.\n\n" - f"- ID: `{rfc_id}`\n" + f"{id_line}" f"- Owners: {owners_str}\n\n" f"This is an in-place state flip per the meta-only topology\n" f"(SPEC §1, §13.3): the entry `rfcs/{slug}.md` keeps its body and\n" @@ -772,7 +775,7 @@ class Bot: pr_number: int, head_branch: str, slug: str, - rfc_id: str, + rfc_id: str | None, ) -> None: """§13.3 step 4: auto-merge the graduation PR with the admin as merge actor. Distinct action_kind so the audit log carries the @@ -788,7 +791,7 @@ class Bot: reports the transient state (belt-and-suspenders against the same race appearing in a different shape). """ - subject = f"Graduate {slug} → {rfc_id}" + subject = f"Graduate {slug} → {rfc_id}" if rfc_id else f"Graduate {slug} (no number)" body = _trailer(actor) try: @@ -809,6 +812,105 @@ class Bot: details={"rfc_id": rfc_id}, ) + # ----- §13.7 retire / un-retire: open + merge a state-flip PR ----- + + async def open_retire_flip_pr( + self, + actor: Actor, + *, + org: str, + meta_repo: str, + slug: str, + new_file_contents: str, + prior_sha: str, + verb: str, + target_state: str, + ) -> dict: + """§13.7: open a PR flipping `rfcs/.md` to `state: + ` — for retire (`verb='retire'`, target `retired`) + or un-retire (`verb='unretire'`, target the restored prior state). + Only the frontmatter `state` changes; the body and every other + field (including the integer `id`) are kept, so an un-retire + restores the entry exactly. Branch shape mirrors graduation's + `--<6hex>`. + """ + import secrets + + branch = f"{verb}-{slug}-{secrets.token_hex(3)}" + await self._gitea.create_branch(org, meta_repo, branch, from_branch="main") + ae = actor.email or f"{actor.gitea_login}@users.noreply" + verb_title = "Retire" if verb == "retire" else "Un-retire" + commit_subject = f"{verb_title} {slug}" + commit_message = _stamp_single(commit_subject, actor) + result = await self._gitea.update_file( + org, meta_repo, f"rfcs/{slug}.md", + content=new_file_contents, + sha=prior_sha, + message=commit_message, + branch=branch, + author_name=actor.display_name, author_email=ae, + ) + commit_sha = ( + result.get("commit", {}).get("sha") + or result.get("content", {}).get("sha") + or "" + ) + pr_title = f"{verb_title} {slug}" + pr_body_text = ( + f"{verb_title}s `{slug}` (state → `{target_state}`).\n\n" + f"This is an in-place frontmatter flip per SPEC §13.7 — the\n" + f"entry `rfcs/{slug}.md` keeps its body and every other field;\n" + f"only `state` changes." + ) + _subject, pr_body = _stamp("", pr_body_text, actor) + pr = await self._gitea.create_pull( + org, meta_repo, + title=pr_title, body=pr_body, head=branch, base="main", + ) + _log( + actor, + f"{verb}_pr_open", + rfc_slug=slug, + branch_name=branch, + pr_number=pr["number"], + bot_commit_sha=commit_sha, + details={"pr_title": pr_title, "target_state": target_state}, + ) + return pr + + async def merge_retire_flip_pr( + self, + actor: Actor, + *, + org: str, + meta_repo: str, + pr_number: int, + head_branch: str, + slug: str, + verb: str, + ) -> None: + """§13.7: auto-merge the retire / un-retire flip PR (same + mergeable-wait + retry shape as `merge_graduation_pr`).""" + verb_title = "Retire" if verb == "retire" else "Un-retire" + subject = f"{verb_title} {slug}" + body = _trailer(actor) + try: + await self._gitea.wait_for_mergeable(org, meta_repo, pr_number) + except TimeoutError as e: + raise GiteaError(409, str(e)) from e + await _merge_with_retry( + self._gitea, org, meta_repo, pr_number, + merge_message_title=subject, merge_message_body=body, + ) + _log( + actor, + f"{verb}_pr_merge", + rfc_slug=slug, + branch_name=head_branch, + pr_number=pr_number, + details={}, + ) + # ----- §13.3 (meta-only): cleanup of an unmerged flip PR ----- async def close_graduation_pr( diff --git a/backend/app/entry.py b/backend/app/entry.py index 27fad95..7d5d2a6 100644 --- a/backend/app/entry.py +++ b/backend/app/entry.py @@ -30,7 +30,7 @@ _ABSENT = object() class Entry: slug: str title: str - state: str = "super-draft" # super-draft | active | withdrawn + state: str = "super-draft" # super-draft | active | withdrawn | retired (§3, §13.7) id: str | None = None # 'RFC-NNNN' or None repo: str | None = None proposed_by: str = "" diff --git a/backend/migrations/025_retired_state.sql b/backend/migrations/025_retired_state.sql new file mode 100644 index 0000000..0c9ae15 --- /dev/null +++ b/backend/migrations/025_retired_state.sql @@ -0,0 +1,61 @@ +-- §3 / §13.7: add the `retired` soft-delete state to cached_rfcs. +-- +-- SQLite cannot ALTER a CHECK constraint in place, so we rebuild the table +-- with the expanded constraint and copy the rows across. cached_rfcs is a +-- §4 cache (reconstructible from Gitea by the reconciler), and nothing +-- holds a foreign key into it, so the rebuild is safe; we preserve the +-- existing rows anyway to avoid a needless full re-read on upgrade. +-- +-- The rebuilt table must carry EVERY column cached_rfcs has accumulated, +-- including the ones added by later migrations via ALTER TABLE ADD COLUMN: +-- 009_per_rfc_models -> models_json +-- 010_funder -> funder_login +-- 021_proposed_use_case -> proposed_use_case +-- They are appended last (matching the live column order) and copied +-- across explicitly so nothing is dropped. +-- +-- The migration runner wraps this file in a single BEGIN/COMMIT, so the +-- swap is atomic. + +CREATE TABLE cached_rfcs_new ( + slug TEXT PRIMARY KEY, + title TEXT NOT NULL, + state TEXT NOT NULL CHECK (state IN ('super-draft', 'active', 'withdrawn', 'retired')), + rfc_id TEXT, -- 'RFC-NNNN' or NULL (NULL is also valid for an active RFC graduated without a number, §13.2) + repo TEXT, -- 'org/repo' or NULL; always NULL under the meta-only topology (§1) + proposed_by TEXT, + proposed_at TEXT, + graduated_at TEXT, + graduated_by TEXT, + owners_json TEXT NOT NULL DEFAULT '[]', + arbiters_json TEXT NOT NULL DEFAULT '[]', + tags_json TEXT NOT NULL DEFAULT '[]', + body TEXT, + body_sha TEXT, + last_main_commit_at TEXT, + last_entry_commit_at TEXT, + updated_at TEXT NOT NULL DEFAULT (datetime('now')), + models_json TEXT, -- 009_per_rfc_models + funder_login TEXT, -- 010_funder + proposed_use_case TEXT -- 021_proposed_use_case +); + +INSERT INTO cached_rfcs_new + (slug, title, state, rfc_id, repo, proposed_by, proposed_at, + graduated_at, graduated_by, owners_json, arbiters_json, tags_json, + body, body_sha, last_main_commit_at, last_entry_commit_at, updated_at, + models_json, funder_login, proposed_use_case) +SELECT + slug, title, state, rfc_id, repo, proposed_by, proposed_at, + graduated_at, graduated_by, owners_json, arbiters_json, tags_json, + body, body_sha, last_main_commit_at, last_entry_commit_at, updated_at, + models_json, funder_login, proposed_use_case + FROM cached_rfcs; + +DROP TABLE cached_rfcs; +ALTER TABLE cached_rfcs_new RENAME TO cached_rfcs; + +CREATE INDEX idx_cached_rfcs_state ON cached_rfcs (state); +CREATE INDEX idx_cached_rfcs_last_active ON cached_rfcs ( + COALESCE(last_main_commit_at, last_entry_commit_at) DESC +); diff --git a/backend/tests/test_graduation_vertical.py b/backend/tests/test_graduation_vertical.py index 5e03376..2cd3321 100644 --- a/backend/tests/test_graduation_vertical.py +++ b/backend/tests/test_graduation_vertical.py @@ -514,6 +514,120 @@ def test_edit_branch_surfaces_normally_after_graduation(app_with_fake_gitea): f"edit branch not in branches: {[b['name'] for b in d['branches']]}" +def test_graduate_check_accepts_blank_id(app_with_fake_gitea): + """§13.2 (optional number): a blank id is VALID — it means "graduate + without a number." `can_submit` stays true (owners are set), and an + absent `id` param behaves the same as an explicit empty string.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + seed_owned_super_draft(fake, slug="ohm", title="OHM", pitch=PITCH, owners=["ben"]) + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner") + + # Explicit empty id. + d = client.get("/api/rfcs/ohm/graduate/check", params={"id": ""}).json() + assert d["id"]["ok"] is True + assert d["id"]["error"] is None + assert d["can_submit"] is True + + # No id param at all — same default-accepted shape. + d = client.get("/api/rfcs/ohm/graduate/check").json() + assert d["id"]["ok"] is True + assert d["can_submit"] is True + + # A malformed (non-blank) id is still rejected. + d = client.get("/api/rfcs/ohm/graduate/check", params={"id": "RFC-xx"}).json() + assert d["id"]["ok"] is False + assert d["can_submit"] is False + + +def test_graduate_without_number_flips_to_active_null_id_by_slug(app_with_fake_gitea): + """§13.2/§13.3 (optional number): graduating with a blank id flips the + entry to `active` with `id: null`. The slug is the canonical identifier; + the catalog shows the entry as active with no number, and the audit row + records rfc_id null.""" + from fastapi.testclient import TestClient + from app import db, entry as entry_mod + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + seed_owned_super_draft(fake, slug="ohm", title="Open Human Model", + pitch=PITCH, owners=["ben"], arbiters=["ben"]) + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner", email="ben@test") + + # Blank rfc_id → graduate without a number. + r = client.post( + "/api/rfcs/ohm/graduate?_sync=1", + json={"rfc_id": "", "owners": ["ben"]}, + ) + assert r.status_code == 200, r.text + d = r.json() + assert d["succeeded"] is True + assert d["rfc_id"] is None + + # Meta entry: active, id null, body kept, graduation stamped. + meta_text = fake.files[("wiggleverse", "meta", "main", "rfcs/ohm.md")]["content"] + graduated = entry_mod.parse(meta_text) + assert graduated.state == "active" + assert graduated.id is None + assert graduated.graduated_by == "ben" + assert graduated.graduated_at + assert "Open Human Model is a framework" in graduated.body + + # Cache flipped to active with a null rfc_id. + cached = db.conn().execute( + "SELECT state, rfc_id FROM cached_rfcs WHERE slug = 'ohm'" + ).fetchone() + assert cached["state"] == "active" + assert cached["rfc_id"] is None + + # Catalog: present as active, identified by slug (no number). + items = client.get("/api/rfcs").json()["items"] + ohm = next(i for i in items if i["slug"] == "ohm") + assert ohm["state"] == "active" + assert ohm["id"] is None + + # Audit: graduate_complete with rfc_id null. + complete = db.conn().execute( + """ + SELECT details FROM actions + WHERE rfc_slug = 'ohm' AND action_kind = 'graduate_complete' + ORDER BY id DESC LIMIT 1 + """ + ).fetchone() + assert complete is not None + assert _json.loads(complete["details"])["rfc_id"] is None + + +def test_graduate_with_number_unchanged_when_id_absent_field(app_with_fake_gitea): + """Omitting the rfc_id field entirely is treated the same as blank — + graduates without a number — so older clients that drop the field don't + break, and the supplied-number path stays exactly as before.""" + from fastapi.testclient import TestClient + from app import entry as entry_mod + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + seed_owned_super_draft(fake, slug="ohm", title="OHM", pitch=PITCH, owners=["ben"]) + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner") + + r = client.post("/api/rfcs/ohm/graduate?_sync=1", json={"owners": ["ben"]}) + assert r.status_code == 200, r.text + assert r.json()["rfc_id"] is None + graduated = entry_mod.parse( + fake.files[("wiggleverse", "meta", "main", "rfcs/ohm.md")]["content"] + ) + assert graduated.state == "active" + assert graduated.id is None + + def test_claim_opens_meta_pr(app_with_fake_gitea): """§13.1: any signed-in contributor can claim ownership of an unclaimed super-draft; the result is a meta-repo PR diff --git a/backend/tests/test_retire_vertical.py b/backend/tests/test_retire_vertical.py new file mode 100644 index 0000000..0561436 --- /dev/null +++ b/backend/tests/test_retire_vertical.py @@ -0,0 +1,257 @@ +"""End-to-end integration tests for the §13.7 retire (soft-delete) flow. + +Retire is an in-place frontmatter flip on the meta entry (state → +`retired`) committed via an auto-merged PR — the same machinery as +graduation, reused. The distinguishing rules under test: + + * Authority (§3.1): RFC owners (frontmatter) and site `owner`-role + holders may retire; app admins may NOT. Un-retire is site-owners-only. + * Visibility (§13.7): a retired entry drops out of the catalog + (`GET /api/rfcs`), and `GET /api/rfcs/` 404s for everyone except + a site owner (so the un-retire affordance has a surface). + * Reversibility: a site owner can un-retire, restoring the prior state + (and keeping the integer id intact); an RFC owner cannot. + +These walk against the in-process FakeGitea from test_propose_vertical.py, +reusing the super-draft seed + sync graduation seam from the graduation +suite. +""" +from __future__ import annotations + +import json as _json + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + grant_rfc_collaborator, + provision_user_row, + sign_in_as, + tmp_env, +) +from test_super_draft_vertical import seed_super_draft # noqa: F401 +from test_graduation_vertical import PITCH, seed_owned_super_draft # noqa: F401 + + +def _catalog_slugs(client) -> set[str]: + return {i["slug"] for i in client.get("/api/rfcs").json()["items"]} + + +def test_rfc_owner_can_retire_and_entry_leaves_every_surface(app_with_fake_gitea): + """An RFC owner (frontmatter, role contributor) retires their own + super-draft. The entry flips to `retired`, drops out of the catalog, + and `GET /api/rfcs/` 404s for them (they are not a site owner).""" + from fastapi.testclient import TestClient + from app import db, entry as entry_mod + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="carol", role="contributor") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"], arbiters=["carol"]) + sign_in_as(client, user_id=2, gitea_login="carol", + display_name="Carol", role="contributor") + + assert "ohm" in _catalog_slugs(client) + + r = client.post("/api/rfcs/ohm/retire") + assert r.status_code == 200, r.text + assert r.json()["state"] == "retired" + + # Meta entry on main: state retired, body + fields kept. + meta = entry_mod.parse( + fake.files[("wiggleverse", "meta", "main", "rfcs/ohm.md")]["content"] + ) + assert meta.state == "retired" + assert "carol" in meta.owners + + # Cache flipped; gone from the catalog. + cached = db.conn().execute( + "SELECT state FROM cached_rfcs WHERE slug = 'ohm'" + ).fetchone() + assert cached["state"] == "retired" + assert "ohm" not in _catalog_slugs(client) + + # The RFC owner is NOT a site owner → 404 on the entry read. + assert client.get("/api/rfcs/ohm").status_code == 404 + + # Audit row records the prior state for un-retire. + row = db.conn().execute( + "SELECT details FROM actions WHERE rfc_slug='ohm' AND action_kind='retire' ORDER BY id DESC LIMIT 1" + ).fetchone() + assert row is not None + assert _json.loads(row["details"])["prior_state"] == "super-draft" + + +def test_site_owner_sees_retired_entry_but_admin_and_others_404(app_with_fake_gitea): + """`GET /api/rfcs/` for a retired entry: site owner gets 200 + (so the un-retire UI has a surface); an admin, a non-owner contributor, + and an anonymous viewer all get 404.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + provision_user_row(user_id=2, login="carol", role="contributor") + provision_user_row(user_id=3, login="dave", role="admin") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + + sign_in_as(client, user_id=2, gitea_login="carol", + display_name="Carol", role="contributor") + assert client.post("/api/rfcs/ohm/retire").status_code == 200 + + # Site owner: 200. + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner") + r = client.get("/api/rfcs/ohm") + assert r.status_code == 200, r.text + assert r.json()["state"] == "retired" + + # Admin (not site owner): 404. + sign_in_as(client, user_id=3, gitea_login="dave", + display_name="Dave", role="admin") + assert client.get("/api/rfcs/ohm").status_code == 404 + + # Non-owner contributor: 404. + provision_user_row(user_id=4, login="erin", role="contributor") + sign_in_as(client, user_id=4, gitea_login="erin", + display_name="Erin", role="contributor") + assert client.get("/api/rfcs/ohm").status_code == 404 + + +def test_admin_cannot_retire(app_with_fake_gitea): + """§3.1: retire authority excludes app admins. An admin who is not an + RFC owner gets 403 — the one lifecycle action where admin authority + does not apply.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=3, login="dave", role="admin") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + sign_in_as(client, user_id=3, gitea_login="dave", + display_name="Dave", role="admin") + r = client.post("/api/rfcs/ohm/retire") + assert r.status_code == 403, r.text + + +def test_non_owner_contributor_cannot_retire(app_with_fake_gitea): + """A signed-in contributor who is not an RFC owner gets 403.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=4, login="erin", role="contributor") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + sign_in_as(client, user_id=4, gitea_login="erin", + display_name="Erin", role="contributor") + assert client.post("/api/rfcs/ohm/retire").status_code == 403 + + +def test_site_owner_can_retire_active_and_unretire_restores_active_with_id(app_with_fake_gitea): + """Round-trip on an active RFC: graduate (with a number) → retire → + un-retire. The integer id survives, and un-retire restores `active`.""" + from fastapi.testclient import TestClient + from app import db, entry as entry_mod + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["ben"], arbiters=["ben"]) + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner", email="ben@test") + + # Graduate with a number. + assert client.post("/api/rfcs/ohm/graduate?_sync=1", + json={"rfc_id": "RFC-0042", "owners": ["ben"]}).status_code == 200 + + # Retire (site owner). + assert client.post("/api/rfcs/ohm/retire").json()["state"] == "retired" + assert "ohm" not in _catalog_slugs(client) + + # Un-retire (site owner) restores active, id intact. + r = client.post("/api/rfcs/ohm/unretire") + assert r.status_code == 200, r.text + assert r.json()["state"] == "active" + + meta = entry_mod.parse( + fake.files[("wiggleverse", "meta", "main", "rfcs/ohm.md")]["content"] + ) + assert meta.state == "active" + assert meta.id == "RFC-0042" + + cached = db.conn().execute( + "SELECT state, rfc_id FROM cached_rfcs WHERE slug = 'ohm'" + ).fetchone() + assert cached["state"] == "active" + assert cached["rfc_id"] == "RFC-0042" + assert "ohm" in _catalog_slugs(client) + + +def test_rfc_owner_cannot_unretire(app_with_fake_gitea): + """Un-retire is site-owner-only: an RFC owner who could retire cannot + bring it back (the soft-delete is recoverable only by the operator).""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="carol", role="contributor") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + sign_in_as(client, user_id=2, gitea_login="carol", + display_name="Carol", role="contributor") + assert client.post("/api/rfcs/ohm/retire").status_code == 200 + # Same RFC owner tries to un-retire → 403. + assert client.post("/api/rfcs/ohm/unretire").status_code == 403 + + +def test_admin_retired_list_is_site_owner_only(app_with_fake_gitea): + """GET /api/admin/retired-rfcs lists retired entries for a site owner; + an admin (who lacks un-retire authority) gets 403.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="ben", role="owner") + provision_user_row(user_id=2, login="carol", role="contributor") + provision_user_row(user_id=3, login="dave", role="admin") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + sign_in_as(client, user_id=2, gitea_login="carol", + display_name="Carol", role="contributor") + assert client.post("/api/rfcs/ohm/retire").status_code == 200 + + # Admin: 403. + sign_in_as(client, user_id=3, gitea_login="dave", + display_name="Dave", role="admin") + assert client.get("/api/admin/retired-rfcs").status_code == 403 + + # Site owner: 200, sees ohm with its restore target. + sign_in_as(client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner") + r = client.get("/api/admin/retired-rfcs") + assert r.status_code == 200, r.text + items = r.json()["items"] + ohm = next(i for i in items if i["slug"] == "ohm") + assert ohm["restores_to"] == "super-draft" + + +def test_retired_entry_refuses_discussion_reads(app_with_fake_gitea): + """A retired entry refuses content reads of every shape (§13.7) — the + discussion/branch surfaces 404/409 rather than serve a soft-deleted RFC.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="carol", role="contributor") + seed_owned_super_draft(fake, slug="ohm", title="OHM", + pitch=PITCH, owners=["carol"]) + sign_in_as(client, user_id=2, gitea_login="carol", + display_name="Carol", role="contributor") + assert client.post("/api/rfcs/ohm/retire").status_code == 200 + + # The /main branch surface no longer serves it. + assert client.get("/api/rfcs/ohm/main").status_code in (404, 409) diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 0ff7bbf..2f4349d 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "rfc-app-frontend", - "version": "0.24.0", + "version": "0.33.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "rfc-app-frontend", - "version": "0.24.0", + "version": "0.33.0", "dependencies": { "@amplitude/unified": "^1.1.9", "@codemirror/commands": "^6.10.3", diff --git a/frontend/package.json b/frontend/package.json index ac13081..222c400 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.31.4", + "version": "0.32.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/api.js b/frontend/src/api.js index f99e7e9..6d25944 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -530,6 +530,22 @@ export async function listBlockingPRs(slug) { return jsonOrThrow(await fetch(`/api/rfcs/${slug}/blocking-prs`)) } +// §13.7 retire (soft delete). RFC owners + site owners may retire; only +// site owners may un-retire. The backend gates both regardless of UI. +export async function retireRFC(slug) { + return jsonOrThrow(await fetch(`/api/rfcs/${slug}/retire`, { method: 'POST' })) +} + +export async function unretireRFC(slug) { + return jsonOrThrow(await fetch(`/api/rfcs/${slug}/unretire`, { method: 'POST' })) +} + +// §13.7: the site-owner-only list of retired entries (for the admin +// "Retired" surface, where un-retire lives). +export async function listRetiredRFCs() { + return jsonOrThrow(await fetch('/api/admin/retired-rfcs')) +} + export async function graduateCheck(slug, { id }) { // Meta-only topology (§13.2): two fields — integer id + owners. No // repo name to validate. diff --git a/frontend/src/components/Admin.jsx b/frontend/src/components/Admin.jsx index ca424e2..94cdc7d 100644 --- a/frontend/src/components/Admin.jsx +++ b/frontend/src/components/Admin.jsx @@ -24,6 +24,8 @@ import { addAllowlistEmail, removeAllowlistEmail, createUserInvite, + listRetiredRFCs, + unretireRFC, } from '../api.js' import { EVENTS, track } from '../lib/analytics.js' @@ -42,12 +44,18 @@ const TABS = [ ] export default function Admin({ viewer }) { + // §13.7: the "Retired" surface (un-retire) is site-owner-only. The + // backend gates /api/admin/retired-rfcs and /unretire on the owner role + // regardless; we only show the link to owners so admins aren't offered + // a tab that would 403. + const isSiteOwner = viewer.role === 'owner' + const tabs = isSiteOwner ? [...TABS, { path: 'retired', label: 'Retired' }] : TABS return (
+ {actionError && ( +
Retire failed: {actionError}
+ )} {claimError && (
Claim failed: {claimError}
)}