From 620926b83447646eb39cea863e877f8e042d8c74 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Sun, 7 Jun 2026 21:54:52 -0700 Subject: [PATCH] =?UTF-8?q?fix(=C2=A722):=20heal=20mig-029=20vs=20registry?= =?UTF-8?q?-mirror=20collection-id=20divergence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On a deployment that already had >=2 projects when migration 029 ran, 029 seeds the default project's collection id as the project id (e.g. 'ohm'), but the registry mirror expects 'default' -> it inserted a duplicate empty 'default' collection, orphaning the entries. Adds projects.reconcile_default_collection_id() -- the collection-grain twin of restamp_default_project -- run at startup BEFORE the mirror so it merges onto the canonical 'default' collection instead of duplicating. Renames the divergent collection id and cascades collection_id across all keyed tables (FK-off atomic rename + foreign_key_check). Idempotent; no-op on fresh / single-project / already-aligned deploys. Seam tests show the duplicate forms without the fix and merges with it. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/app/main.py | 6 + backend/app/projects.py | 89 ++++++++++ .../test_reconcile_default_collection.py | 156 ++++++++++++++++++ 3 files changed, 251 insertions(+) create mode 100644 backend/tests/test_reconcile_default_collection.py diff --git a/backend/app/main.py b/backend/app/main.py index 953caf5..3e0acd6 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -102,6 +102,12 @@ async def lifespan(app: FastAPI): db.run_migrations(config) db.init(config) gitea = Gitea(config) + # §22 framework heal: reconcile a divergent default-project collection id + # (migration 029's ≥2-projects seed names it after the project, e.g. 'ohm', + # but the mirror expects 'default') BEFORE the mirror runs, so the mirror + # merges onto the canonical 'default' collection instead of duplicating it. + # Idempotent no-op on fresh / single-project / already-aligned deployments. + projects.reconcile_default_collection_id(config) # §22.2: mirror the registry before anything reads projects/content_repo. # First boot has no last-good rows, so a missing/invalid registry is fatal # (loud-fail per separation-of-concerns); the reconciler sweep keeps it diff --git a/backend/app/projects.py b/backend/app/projects.py index e6d56d8..885c18f 100644 --- a/backend/app/projects.py +++ b/backend/app/projects.py @@ -92,6 +92,95 @@ def restamp_default_project(config: Config) -> None: DEFAULT_PROJECT_ID, target, len(pid_tables)) +def reconcile_default_collection_id(config: Config) -> None: + """Heal the §22 migration-029 vs registry-mirror divergence for the default + project's collection id on a multi-project deployment. + + Migration 029 seeds each project's default collection id as the literal + 'default' only when the DB holds a single project at migration time; with + ≥2 projects it falls back to the *project id* (avoiding a PK collision — + 029 can't read DEFAULT_PROJECT_ID, there is no env in SQL). But the registry + mirror (`registry._default_collection_id`) expects the deployment's default + project to own the collection id 'default'. On an upgrade whose DB already + held ≥2 projects when 029 ran, the default project's collection is therefore + named after the project (e.g. 'ohm'), and the next mirror would INSERT a + second, empty 'default' collection instead of merging — duplicating the + default corpus and orphaning the entries (which point at 'ohm'). + + This is the collection-grain twin of `restamp_default_project`. Run at + startup BEFORE the registry mirror so the canonical 'default' collection + already exists when the mirror upserts (merge, not duplicate). Renames the + divergent collection's id to 'default' and cascades `collection_id` across + every collection-keyed table, with FK enforcement off for the atomic rename + and a `foreign_key_check` backstop before commit. Idempotent; a no-op on + fresh / single-project / already-aligned deployments. + """ + from .collections import DEFAULT_COLLECTION_ID + + target = resolved_default_id(config) + if target == DEFAULT_COLLECTION_ID: # default project already owns 'default' + return + conn = db.conn() + # The 029 ≥2-projects seed names the default project's collection after the + # project itself; the canonical id the mirror expects is 'default'. + divergent = conn.execute( + "SELECT 1 FROM collections WHERE id = ? AND project_id = ? LIMIT 1", + (target, target), + ).fetchone() + if not divergent: + return + if conn.execute( + "SELECT 1 FROM collections WHERE id = ? LIMIT 1", (DEFAULT_COLLECTION_ID,) + ).fetchone(): + # A 'default' collection already exists (e.g. a prior buggy mirror left a + # duplicate). Don't auto-merge data — that needs care; leave both for + # operator cleanup and log loudly. + log.warning( + "reconcile: default project %r owns both a %r and a 'default' " + "collection; skipping auto-rename (manual merge required)", + target, target, + ) + return + + cid_tables = [ + t["name"] + for t in conn.execute("SELECT name FROM sqlite_master WHERE type='table'") + if any(c["name"] == "collection_id" + for c in conn.execute(f"PRAGMA table_info({t['name']})")) + ] + conn.execute("PRAGMA foreign_keys = OFF") + try: + conn.execute("BEGIN") + conn.execute( + "UPDATE collections SET id = ? WHERE id = ?", + (DEFAULT_COLLECTION_ID, target), + ) + for t in cid_tables: + conn.execute( + f"UPDATE {t} SET collection_id = ? WHERE collection_id = ?", + (DEFAULT_COLLECTION_ID, target), + ) + violations = conn.execute("PRAGMA foreign_key_check").fetchall() + if violations: + conn.execute("ROLLBACK") + raise RuntimeError( + f"reconcile 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.info( + "reconcile: renamed default-project collection %r -> 'default' across %d tables", + target, len(cid_tables), + ) + + 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/tests/test_reconcile_default_collection.py b/backend/tests/test_reconcile_default_collection.py new file mode 100644 index 0000000..adc028f --- /dev/null +++ b/backend/tests/test_reconcile_default_collection.py @@ -0,0 +1,156 @@ +"""§22 framework bug — migration-029 vs registry-mirror collection-id +divergence for a multi-project deployment's default project. + +Migration 029 seeds the default project's collection id as the literal +'default' only when one project exists at migration time; with ≥2 projects it +falls back to the *project id*. The registry mirror expects the default +project to own the collection id 'default'. On an upgrade whose DB already +held ≥2 projects when 029 ran, the default project's collection is therefore +named after the project (e.g. 'ohm'), and the next mirror would INSERT a +second, empty 'default' collection. `reconcile_default_collection_id` heals +the divergence at startup, before the mirror, so the mirror merges instead of +duplicating. This is the collection-grain twin of `restamp_default_project`. +""" +from __future__ import annotations + +import tempfile +from pathlib import Path + +import app.db as db +from app import projects, registry + +_TWO_PROJECT_REGISTRY = """ +deployment: + name: Open Human Model + tagline: t +projects: + - id: ohm + name: Open Human Model + type: document + content_repo: ohm-content + visibility: public + - id: ecomm + name: Ecomm + type: bdd + content_repo: ecomm-content + visibility: public +""" + + +def _collection_ids_for(conn, project_id): + return {r["id"] for r in conn.execute( + "SELECT id FROM collections WHERE project_id = ?", (project_id,))} + + +class _Cfg: + def __init__(self, path, default_id): + self.database_path = path + self.default_project_id = default_id + + +def _divergent_multiproject(monkeypatch, default_id="ohm"): + """A DB in the post-029 divergent state: the default project ('ohm') owns a + collection whose id is the project id (the 029 ≥2-projects seed), a second + project ('ecomm') owns its own collection, and NO 'default' collection + exists. Entry rows point at the divergent collection_id='ohm'.""" + path = str(Path(tempfile.mkdtemp()) / "t.db") + cfg = _Cfg(path, default_id) + db.run_migrations(cfg) # seeds bootstrap 'default' project + 'default' collection + monkeypatch.setattr(db, "_CONN", db.connect(path)) + conn = db.conn() + # Drop the single-project bootstrap seed and rebuild the divergent + # multi-project state 029 would have produced on an upgrade. + conn.execute("DELETE FROM collections") + conn.execute("DELETE FROM projects") + conn.execute("INSERT INTO projects (id,name,content_repo,visibility) " + "VALUES ('ohm','Open Human Model','ohm-content','public')") + conn.execute("INSERT INTO projects (id,name,content_repo,visibility) " + "VALUES ('ecomm','Ecomm','ecomm-content','public')") + # 029 ≥2-projects seed: collection id == project id. + conn.execute("INSERT INTO collections (id,project_id,type,subfolder,initial_state,visibility,name) " + "VALUES ('ohm','ohm','document','','super-draft','public','Open Human Model')") + 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')") + # Entry data for the default project lives in the divergent 'ohm' collection. + conn.execute("INSERT INTO cached_rfcs (slug,title,state,collection_id) VALUES ('human','Human','active','ohm')") + conn.execute("INSERT INTO rfc_collaborators (rfc_slug,user_id,role_in_rfc,collection_id) " + "VALUES ('human',1,'contributor','ohm')") + conn.execute("INSERT INTO stars (user_id,rfc_slug,collection_id) VALUES (1,'human','ohm')") + # The 'ecomm' collection has its own entry. + conn.execute("INSERT INTO cached_rfcs (slug,title,state,collection_id) VALUES ('cart','Cart','active','ecomm')") + return cfg, conn + + +def test_reconcile_renames_divergent_default_collection_to_default(monkeypatch): + cfg, conn = _divergent_multiproject(monkeypatch, default_id="ohm") + projects.reconcile_default_collection_id(cfg) + # The default project's collection id is now the canonical 'default'. + assert conn.execute("SELECT 1 FROM collections WHERE id='ohm'").fetchone() is None + row = conn.execute("SELECT project_id FROM collections WHERE id='default'").fetchone() + assert row is not None and row["project_id"] == "ohm" + # Entry rows cascaded onto 'default'. + assert conn.execute("SELECT collection_id FROM cached_rfcs WHERE slug='human'").fetchone()["collection_id"] == "default" + assert conn.execute("SELECT collection_id FROM rfc_collaborators WHERE rfc_slug='human'").fetchone()["collection_id"] == "default" + assert conn.execute("SELECT collection_id FROM stars WHERE rfc_slug='human'").fetchone()["collection_id"] == "default" + # The non-default 'ecomm' collection is untouched (mirror + 029 agree on it). + assert conn.execute("SELECT 1 FROM collections WHERE id='ecomm'").fetchone() is not None + assert conn.execute("SELECT collection_id FROM cached_rfcs WHERE slug='cart'").fetchone()["collection_id"] == "ecomm" + # FK integrity intact after the rename. + assert conn.execute("PRAGMA foreign_key_check").fetchall() == [] + + +def test_reconcile_is_idempotent(monkeypatch): + cfg, conn = _divergent_multiproject(monkeypatch, default_id="ohm") + projects.reconcile_default_collection_id(cfg) + projects.reconcile_default_collection_id(cfg) # second call: already aligned → no-op + assert conn.execute("SELECT project_id FROM collections WHERE id='default'").fetchone()["project_id"] == "ohm" + assert conn.execute("SELECT COUNT(*) c FROM cached_rfcs WHERE collection_id='default'").fetchone()["c"] == 1 + + +def test_reconcile_noop_when_default_id_is_literal_default(monkeypatch): + # Single-project deployment, no DEFAULT_PROJECT_ID: 029 already seeded + # 'default' and the mirror agrees — nothing to reconcile. + path = str(Path(tempfile.mkdtemp()) / "t.db") + cfg = _Cfg(path, "") # resolves to 'default' + db.run_migrations(cfg) + monkeypatch.setattr(db, "_CONN", db.connect(path)) + conn = db.conn() + projects.reconcile_default_collection_id(cfg) + assert conn.execute("SELECT 1 FROM collections WHERE id='default'").fetchone() is not None + + +def test_mirror_duplicates_without_reconcile(monkeypatch): + # Demonstrates the bug: the mirror on the divergent state inserts a SECOND + # 'default' collection for the default project (alongside the 029 'ohm'). + cfg, conn = _divergent_multiproject(monkeypatch, default_id="ohm") + doc = registry.parse_registry(_TWO_PROJECT_REGISTRY) + registry.apply_registry(doc, registry_sha="s1", default_id="ohm") + assert _collection_ids_for(conn, "ohm") == {"ohm", "default"} # duplicate! + + +def test_reconcile_then_mirror_merges_no_duplicate(monkeypatch): + # With the fix: reconcile before the mirror → the mirror merges onto the + # canonical 'default' collection; the default project owns exactly one. + cfg, conn = _divergent_multiproject(monkeypatch, default_id="ohm") + projects.reconcile_default_collection_id(cfg) + doc = registry.parse_registry(_TWO_PROJECT_REGISTRY) + registry.apply_registry(doc, registry_sha="s1", default_id="ohm") + assert _collection_ids_for(conn, "ohm") == {"default"} + assert _collection_ids_for(conn, "ecomm") == {"ecomm"} + # The default project's corpus entry is intact under 'default'. + assert conn.execute("SELECT collection_id FROM cached_rfcs WHERE slug='human'").fetchone()["collection_id"] == "default" + # The mirror refreshed the merged collection's metadata (type from registry). + assert conn.execute("SELECT type FROM collections WHERE id='default'").fetchone()["type"] == "document" + + +def test_reconcile_skips_when_default_collection_already_exists(monkeypatch): + # A prior buggy mirror already created a 'default' collection alongside the + # divergent 'ohm' one: don't auto-merge data — leave both for operator cleanup. + cfg, conn = _divergent_multiproject(monkeypatch, default_id="ohm") + conn.execute("INSERT INTO collections (id,project_id,type,subfolder,initial_state,visibility,name) " + "VALUES ('default','ohm','document','','super-draft','public','dup')") + projects.reconcile_default_collection_id(cfg) + # Both still present (no destructive auto-merge). + assert conn.execute("SELECT 1 FROM collections WHERE id='ohm'").fetchone() is not None + assert conn.execute("SELECT 1 FROM collections WHERE id='default'").fetchone() is not None