diff --git a/CHANGELOG.md b/CHANGELOG.md index 579bab3..7b6bc65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,48 @@ 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.55.0 — 2026-06-09 + +**Minor — the registry reconcile now prunes projects the registry no longer +declares; read the upgrade note.** + +The registry mirror was additive-only: it upserted the projects/collections a +deployment's `projects.yaml` declares but never removed ones it had dropped. +Re-pinning a deployment to a different registry (or editing `projects.yaml` to +remove a project) therefore stranded the old project's collections and its +cached entries in the database — dead rows that, at scale, starved writes (a +deployment's PPE accumulated ~1,200 orphaned entry rows this way and had to be +reset by hand). + +- **Prune-on-reconcile.** `registry.refresh_registry(..., prune=True)` now calls + `projects.prune_absent_projects`, which deletes any project absent from the + freshly-parsed registry together with its collections and every project-scoped + row (entries + their satellites, threads + messages, changes, PRs, stars, + watches, …) in one transaction, with FK enforcement off for the multi-table + delete and a `PRAGMA foreign_key_check` backstop that rolls the whole thing + back rather than leave a dangling reference. +- **Whole-project granularity, and only the default-safe triggers.** A project + still present in the registry is never touched here (pruning individual entries + within a present project belongs to the best-effort content cache). The + deployment's **default project is never pruned**, whatever the registry says. + Pruning runs only on the two full-reconcile triggers — **startup** and the + **periodic reconciler sweep** — never on incidental refreshes (a collection + create, a registry webhook), so an in-app refresh can't wipe a project. +- **Safe against a transient registry read.** The prune is reached only after a + successful parse of a non-empty registry: `parse_registry` rejects an empty + project list and the registry read raises on a missing file / transport error + (the reconciler keeps last-good on that error). A flaky read can't trigger a + wipe. + +**Upgrade note (no steps required for a correctly-configured deployment).** On +the first startup after upgrading, any project that exists in the database but is +**absent from your `projects.yaml`** will be pruned. For a deployment whose +database was only ever populated from its current registry this is a no-op (every +DB project is in `projects.yaml`). If you have deliberately out-of-band projects +in the database (there is no supported way to create one — every project goes +through `projects.yaml`), add them to the registry before upgrading. No database +migration and no configuration change. + ## 0.54.1 — 2026-06-09 **Patch — the Gitea-OAuth sign-in no longer 500s when an account was first diff --git a/VERSION b/VERSION index 1942d77..316ba4b 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.54.1 +0.55.0 diff --git a/backend/app/cache.py b/backend/app/cache.py index dde6d36..1689f43 100644 --- a/backend/app/cache.py +++ b/backend/app/cache.py @@ -798,7 +798,7 @@ class Reconciler: log.info("reconciler: starting sweep") try: try: - await registry_mod.refresh_registry(self._config, self._gitea) + await registry_mod.refresh_registry(self._config, self._gitea, prune=True) except Exception: log.exception("reconciler: registry refresh failed; keeping last-good projects") await refresh_meta_repo(self._config, self._gitea) diff --git a/backend/app/main.py b/backend/app/main.py index 418e51f..c7c673e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -133,7 +133,7 @@ async def lifespan(app: FastAPI): # (loud-fail per separation-of-concerns); the reconciler sweep keeps it # fresh thereafter and tolerates a later bad PR. try: - await registry_mod.refresh_registry(config, gitea) + await registry_mod.refresh_registry(config, gitea, prune=True) except Exception as e: raise RuntimeError( f"registry mirror failed at startup ({config.registry_repo_full}/projects.yaml): {e}" diff --git a/backend/app/projects.py b/backend/app/projects.py index 68ad8a4..aa14f6e 100644 --- a/backend/app/projects.py +++ b/backend/app/projects.py @@ -181,6 +181,98 @@ def reconcile_default_collection_id(config: Config) -> None: ) +def prune_absent_projects(config: Config, present_ids: set[str]) -> list[str]: + """Delete projects the registry no longer declares, plus their collections + and every project-scoped row. Whole-project granularity only. + + `present_ids` is the authoritative set of project ids from the just-parsed + registry. Safety is structural: the only caller (`registry.refresh_registry`) + raises on a read/transport error, and `parse_registry` rejects an empty + project list — so this is reached ONLY after a real, non-empty registry has + been parsed. A transient registry-read failure therefore can't trigger a wipe. + The default project is never pruned regardless of what the registry says. + + Whole-project, not per-collection/entry: a project still present keeps all its + rows here. Pruning individual entries within a present project belongs to the + best-effort content cache (whose list can transiently fail), not to this. + + Mirrors `restamp_default_project`'s FK-off + `foreign_key_check` pattern. After + 029 the entry-corpus tables key on `collection_id` (not `project_id`), so the + delete spans three discovered sets — project_id-keyed tables, collection_id- + keyed tables (keyed to the stale projects' collections), and the lone non-keyed + descendant `thread_messages` (FK→threads) — in one transaction with FK + enforcement off and a `foreign_key_check` backstop that rolls back on any + dangling reference (so an incomplete delete fails loudly rather than corrupts). + Returns the pruned project ids (empty when nothing is stale). + """ + keep = set(present_ids) + keep.add(resolved_default_id(config)) + keep.add(DEFAULT_PROJECT_ID) + conn = db.conn() + stale = [ + r["id"] for r in conn.execute("SELECT id FROM projects").fetchall() + if r["id"] not in keep + ] + if not stale: + return [] + + stale_collections = [ + r["id"] for r in conn.execute( + f"SELECT id FROM collections WHERE project_id IN ({','.join('?' * len(stale))})", + stale, + ).fetchall() + ] + tables = [r["name"] for r in conn.execute("SELECT name FROM sqlite_master WHERE type='table'")] + + def _has(table: str, col: str) -> bool: + return any(c["name"] == col for c in conn.execute(f"PRAGMA table_info({table})")) + + pid_tables = [t for t in tables if t != "projects" and _has(t, "project_id")] + cid_tables = [t for t in tables if _has(t, "collection_id")] + p_ph = ",".join("?" * len(stale)) + c_ph = ",".join("?" * len(stale_collections)) if stale_collections else "" + + conn.execute("PRAGMA foreign_keys = OFF") + try: + conn.execute("BEGIN") + # Non-keyed descendant: thread_messages hangs off threads(id), which is + # project_id-keyed below. Clear it first by thread lineage. + conn.execute( + f"DELETE FROM thread_messages WHERE thread_id IN " + f"(SELECT id FROM threads WHERE project_id IN ({p_ph}))", + stale, + ) + if stale_collections: + for t in cid_tables: + conn.execute( + f"DELETE FROM {t} WHERE collection_id IN ({c_ph})", stale_collections + ) + for t in pid_tables: # includes collections + cached_prs + threads/changes/… + conn.execute(f"DELETE FROM {t} WHERE project_id IN ({p_ph})", stale) + conn.execute(f"DELETE FROM projects WHERE id IN ({p_ph})", stale) + violations = conn.execute("PRAGMA foreign_key_check").fetchall() + if violations: + conn.execute("ROLLBACK") + raise RuntimeError( + f"prune left foreign-key violations: {[tuple(v) for v in violations]}" + ) + conn.execute("COMMIT") + except Exception: + try: + conn.execute("ROLLBACK") + except Exception: + pass + raise + finally: + conn.execute("PRAGMA foreign_keys = ON") + log.warning( + "prune: removed %d project(s) absent from the registry: %s " + "(%d collection(s), across %d project- + %d collection-keyed tables)", + len(stale), stale, len(stale_collections), len(pid_tables), len(cid_tables), + ) + return stale + + def default_content_repo(config: Config) -> str | None: """The content repo the single-corpus mirror reads, from the default project's row (filled by the registry mirror). Replaces the retired diff --git a/backend/app/registry.py b/backend/app/registry.py index e2504de..d4b7652 100644 --- a/backend/app/registry.py +++ b/backend/app/registry.py @@ -333,11 +333,18 @@ async def _mirror_named_collections(config: Config, gitea: Gitea, doc: RegistryD _upsert_named_collection(proj, subdir, ce, sha) -async def refresh_registry(config: Config, gitea: Gitea) -> None: +async def refresh_registry(config: Config, gitea: Gitea, *, prune: bool = False) -> None: """Mirror REGISTRY_REPO/projects.yaml into projects + deployment. Idempotent. Raises RegistryError on a missing/invalid file and GiteaError on transport failure; the caller chooses fatal-vs-tolerated. + + `prune=True` additionally removes projects the registry no longer declares + (and their collections + entries) — see `projects.prune_absent_projects`. + It is OFF by default and enabled only on the full-reconcile triggers + (startup + the periodic sweep), never on incidental refreshes (a collection + create, a webhook) that don't change the project set — keeping the + destructive sweep on the few code paths that mean "reconcile to the registry." """ item = await gitea.get_contents( config.gitea_org, config.registry_repo, "projects.yaml", ref="main" @@ -355,4 +362,18 @@ async def refresh_registry(config: Config, gitea: Gitea) -> None: apply_registry(doc, sha, projects_mod.resolved_default_id(config)) # §22 S2: discover + upsert named collections from each content repo. await _mirror_named_collections(config, gitea, doc, sha) - log.info("registry: mirrored %d project(s) at %s", len(doc.projects), sha) + # Prune projects the registry no longer declares (additive-only was a §9 + # fragility — a re-pin to a new registry stranded the old projects' entries, + # starving writes). Safe here: we only reach this line after a successful + # parse of a non-empty registry (parse_registry rejects an empty project list + # and the read above raises on transport failure), so a transient failure + # never prunes. Whole-project granularity; the default project is never pruned. + pruned = ( + projects_mod.prune_absent_projects(config, {e.id for e in doc.projects}) + if prune else [] + ) + log.info( + "registry: mirrored %d project(s) at %s%s", + len(doc.projects), sha, + f"; pruned {len(pruned)} absent: {pruned}" if pruned else "", + ) diff --git a/backend/tests/test_prune_absent_projects.py b/backend/tests/test_prune_absent_projects.py new file mode 100644 index 0000000..430bdd8 --- /dev/null +++ b/backend/tests/test_prune_absent_projects.py @@ -0,0 +1,163 @@ +"""§9 hardening — the registry reconcile prunes projects the registry no longer +declares (was additive-only). + +A re-pin to a new registry used to strand the old projects' rows (PPE's +stale-ecomm 1238-row entry cache starving writes). `prune_absent_projects` now +deletes a removed project together with its collections and every project-scoped +row, at whole-project granularity, with a `foreign_key_check` backstop. The +safety guard is structural: `refresh_registry` raises on a read/transport error +and `parse_registry` rejects an empty project list, so the prune is reached only +after a real, non-empty registry parse — a transient failure can't wipe data. +""" +from __future__ import annotations + +import tempfile +from pathlib import Path + +import pytest + +import app.db as db +from app import projects, registry + + +class _Cfg: + def __init__(self, path, default_id): + self.database_path = path + self.default_project_id = default_id + + +def _two_project_db(monkeypatch, default_id="ohm"): + """A multi-project DB: default project 'ohm' (collection 'default') + a second + project 'ecomm' (collection 'ecomm'), each with a rich set of project- and + collection-keyed rows, plus the non-keyed `thread_messages` descendant.""" + path = str(Path(tempfile.mkdtemp()) / "t.db") + cfg = _Cfg(path, default_id) + db.run_migrations(cfg) + monkeypatch.setattr(db, "_CONN", db.connect(path)) + conn = db.conn() + conn.execute("DELETE FROM collections") + conn.execute("DELETE FROM projects") + conn.execute("INSERT INTO projects (id,name,content_repo,visibility) VALUES ('ohm','OHM','ohm-content','public')") + conn.execute("INSERT INTO projects (id,name,content_repo,visibility) VALUES ('ecomm','Ecomm','ecomm-content','public')") + conn.execute("INSERT INTO collections (id,project_id,type,subfolder,initial_state,visibility,name) " + "VALUES ('default','ohm','document','','super-draft','public','OHM')") + conn.execute("INSERT INTO collections (id,project_id,type,subfolder,initial_state,visibility,name) " + "VALUES ('ecomm','ecomm','bdd','ecomm','super-draft','public','Ecomm')") + conn.execute("INSERT INTO users (id,gitea_login,display_name,role) VALUES (1,'a','A','contributor')") + + def seed(proj, coll, slug): + conn.execute("INSERT INTO cached_rfcs (slug,title,state,collection_id) VALUES (?,?,'active',?)", + (slug, slug.title(), coll)) + conn.execute("INSERT INTO cached_branches (rfc_slug,branch_name,collection_id) VALUES (?, 'main', ?)", + (slug, coll)) + conn.execute("INSERT INTO cached_prs (rfc_slug,pr_kind,repo,pr_number,title,state,project_id) " + "VALUES (?, 'rfc_branch', ?, 1, 't', 'open', ?)", (slug, proj, proj)) + conn.execute("INSERT INTO stars (user_id,rfc_slug,collection_id) VALUES (1,?,?)", (slug, coll)) + conn.execute("INSERT INTO watches (user_id,rfc_slug,state,set_by,collection_id) VALUES (1,?,'watching','explicit',?)", + (slug, coll)) + conn.execute("INSERT INTO changes (rfc_slug,branch_name,kind,original,proposed,project_id) " + "VALUES (?, 'main', 'ai', 'o', 'p', ?)", (slug, proj)) + conn.execute("INSERT INTO notifications (recipient_user_id,event_kind,project_id) VALUES (1,'x',?)", (proj,)) + cur = conn.execute("INSERT INTO threads (rfc_slug,anchor_kind,thread_kind,project_id) " + "VALUES (?, 'whole-doc', 'chat', ?)", (slug, proj)) + conn.execute("INSERT INTO thread_messages (thread_id,role,text) VALUES (?, 'user', 'hi')", (cur.lastrowid,)) + + seed("ohm", "default", "human") + seed("ecomm", "ecomm", "cart") + assert conn.execute("PRAGMA foreign_key_check").fetchall() == [] # seed is FK-clean + return cfg, conn + + +_PRUNED_TABLES = [ + "cached_rfcs", "cached_branches", "cached_prs", "stars", "watches", + "changes", "notifications", "threads", +] + + +def test_prune_removes_absent_project_and_all_its_data(monkeypatch): + cfg, conn = _two_project_db(monkeypatch, default_id="ohm") + pruned = projects.prune_absent_projects(cfg, {"ohm"}) # ecomm absent + assert pruned == ["ecomm"] + + # ecomm: project, collection, and every keyed row gone. + assert conn.execute("SELECT 1 FROM projects WHERE id='ecomm'").fetchone() is None + assert conn.execute("SELECT 1 FROM collections WHERE id='ecomm'").fetchone() is None + for t in _PRUNED_TABLES: + col = "collection_id" if t in ("cached_rfcs", "cached_branches", "stars", "watches") else None + if col: + assert conn.execute(f"SELECT COUNT(*) n FROM {t} WHERE collection_id='ecomm'").fetchone()["n"] == 0, t + else: + assert conn.execute(f"SELECT COUNT(*) n FROM {t} WHERE project_id='ecomm'").fetchone()["n"] == 0, t + # The non-keyed descendant (thread_messages on ecomm's thread) is gone too. + assert conn.execute("SELECT COUNT(*) n FROM thread_messages").fetchone()["n"] == 1 # only ohm's remains + + # ohm (the default) is fully intact. + assert conn.execute("SELECT 1 FROM projects WHERE id='ohm'").fetchone() is not None + assert conn.execute("SELECT 1 FROM collections WHERE id='default'").fetchone() is not None + assert conn.execute("SELECT COUNT(*) n FROM cached_rfcs WHERE slug='human'").fetchone()["n"] == 1 + assert conn.execute("SELECT COUNT(*) n FROM threads WHERE rfc_slug='human'").fetchone()["n"] == 1 + + # FK integrity intact after the prune. + assert conn.execute("PRAGMA foreign_key_check").fetchall() == [] + + +def test_prune_is_noop_when_all_projects_present(monkeypatch): + cfg, conn = _two_project_db(monkeypatch, default_id="ohm") + assert projects.prune_absent_projects(cfg, {"ohm", "ecomm"}) == [] + assert conn.execute("SELECT COUNT(*) n FROM projects").fetchone()["n"] == 2 + assert conn.execute("SELECT COUNT(*) n FROM cached_rfcs").fetchone()["n"] == 2 + + +def test_prune_never_removes_the_default_project(monkeypatch): + # Even with an EMPTY present set (which the real caller can't produce — + # parse_registry rejects an empty registry), the default project survives. + cfg, conn = _two_project_db(monkeypatch, default_id="ohm") + pruned = projects.prune_absent_projects(cfg, set()) + assert "ohm" not in pruned and "ecomm" in pruned + assert conn.execute("SELECT 1 FROM projects WHERE id='ohm'").fetchone() is not None + assert conn.execute("SELECT 1 FROM collections WHERE id='default'").fetchone() is not None + assert conn.execute("PRAGMA foreign_key_check").fetchall() == [] + + +def test_empty_registry_is_rejected_so_prune_is_never_reached(): + # The structural safety guard: a parse that would yield no projects raises, + # so refresh_registry never reaches the prune with an empty present set. + with pytest.raises(registry.RegistryError): + registry.parse_registry("deployment:\n name: x\nprojects: []\n") + + +def test_incidental_refresh_does_not_prune(app_with_fake_gitea): # noqa: F811 + """An incidental re-mirror (the registry webhook, prune=False) must NOT prune + a project absent from projects.yaml — only the full-reconcile triggers + (startup + periodic sweep) prune. Guards the scoping decision so an in-app + refresh can't wipe a project.""" + import hashlib + import hmac + import json as _json + + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + # A project present in the DB but not in the fake registry's projects.yaml. + db.conn().execute( + "INSERT INTO projects (id,name,content_repo,visibility) " + "VALUES ('ghost','Ghost','ghost-content','public')" + ) + body = _json.dumps({"repository": {"full_name": "wiggleverse/registry"}}).encode() + secret = "test-webhook-secret-for-signature-verification" + sig = hmac.new(secret.encode(), body, hashlib.sha256).hexdigest() + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={"X-Gitea-Event": "push", "X-Gitea-Signature": sig, + "Content-Type": "application/json"}, + ) + assert r.status_code == 200 + # The incidental re-mirror left the ghost in place (prune is off here). + assert db.conn().execute("SELECT 1 FROM projects WHERE id='ghost'").fetchone() is not None + + +# app_with_fake_gitea fixture import for the integration test above. +from test_propose_vertical import app_with_fake_gitea, tmp_env # noqa: E402,F401 diff --git a/frontend/package.json b/frontend/package.json index 4790ae4..7775513 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.54.1", + "version": "0.55.0", "type": "module", "scripts": { "dev": "vite",