From 4e7410f90ba7cd196cfb0b91b49c8a8fbfadecc0 Mon Sep 17 00:00:00 2001 From: Ben Stull Date: Mon, 8 Jun 2026 22:39:46 -0700 Subject: [PATCH] =?UTF-8?q?fix(=C2=A722/G-15):=20make=20the=20branch/edit/?= =?UTF-8?q?body=20subsystem=20three-tier=20aware=20(v0.53.0)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §22 migrated the READ/catalog path to the three-tier (project/collection) model but the WRITE/branch/body subsystem still hardcoded the default project's default collection — resolving every meta-resident entry to the default content repo at rfcs/.md, ignoring the entry's project (its own content repo) and collection (a /rfcs/ prefix). An entry outside the default collection rendered a blank canonical body and every edit/PR/ body-write path hit the wrong file. - New single resolver: projects.content_repo_for_collection() + projects.entry_location(config, cid, slug) -> (org, repo, md_path) (collection -> project -> content_repo + subfolder; falls back to the default repo for a legacy/unknown collection). - Every entry write/read path resolves via it: api_branches (body GET + all branch write paths), api_prs (pr-draft/open/merge/withdraw/review/ resolution-branch), api_graduation (graduate/claim/retire/unretire + orchestrator + state-flip), api.py mark-reviewed + idea-PR merge/decline/ withdraw + proposal preview, api_metadata. bot.open_metadata_pr and bot.mark_entry_reviewed gained a file_path param. refresh_meta_branches, the webhook corpus-refresh dispatch, and the hygiene branch-delete now span every project's content repo, not just the default. - New additive collection-scoped body-read routes: GET /api/projects/{pid}/collections/{cid}/rfcs/{slug}/main and .../branches/{branch} disambiguate a slug across collections (G-5) and read the entry's own repo. Slug-only routes kept (now collection-aware via the cached row). Frontend getRFCMain/getBranch take optional pid+cid; RFCView threads them (mirrors the v0.52.1 entry-detail fix). No migration, no config change. Existing default-collection entries unaffected. Tests: backend resolver + collection-scoped branch/body + graduate- in-subfolder write path; frontend api unit. backend 677 / frontend 60 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 55 +++++ VERSION | 2 +- backend/app/api.py | 32 ++- backend/app/api_branches.py | 87 ++++++-- backend/app/api_graduation.py | 84 ++++---- backend/app/api_metadata.py | 15 +- backend/app/api_prs.py | 10 +- backend/app/bot.py | 23 ++- backend/app/cache.py | 118 ++++++----- backend/app/hygiene.py | 9 +- backend/app/projects.py | 31 +++ backend/app/webhooks.py | 17 +- .../test_g15_branch_collection_scoped.py | 192 ++++++++++++++++++ backend/tests/test_g15_entry_location.py | 108 ++++++++++ frontend/package.json | 2 +- frontend/src/api.branchbody.test.js | 40 ++++ frontend/src/api.js | 20 +- frontend/src/components/RFCView.jsx | 27 +-- 18 files changed, 720 insertions(+), 152 deletions(-) create mode 100644 backend/tests/test_g15_branch_collection_scoped.py create mode 100644 backend/tests/test_g15_entry_location.py create mode 100644 frontend/src/api.branchbody.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index f13d30b..120825d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,61 @@ 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.53.0 — 2026-06-09 + +**Minor — the branch/edit/body subsystem is now three-tier (project / +collection) aware (gap G-15); no operator action required.** + +§22 migrated the READ/catalog path to the three-tier model, but the +WRITE/branch/body subsystem still hardcoded the default project's default +collection: it resolved every meta-resident entry to the default project's +content repo at `rfcs/.md`, ignoring the entry's project (its own +content repo) and its collection (a `/rfcs/` prefix). As a result +an entry outside the default project's default collection rendered a **blank +canonical body** (the git-backed `GET …/branches/main` read the wrong file) +and every edit / PR / body-write path hit the wrong repo. Surfaced by +dogfooding a separate project on a live deployment. + +- **Single resolver.** New `projects.content_repo_for_collection(cid)` and + `projects.entry_location(config, cid, slug) → (org, repo, md_path)` resolve + an entry's git location from its collection (collection → project → + content_repo, plus the collection subfolder), falling back to the default + project's repo for a legacy / unknown collection. Every write/read path now + shares this resolver instead of the hardcoded default. +- **Every entry write/read path is collection-resolved:** the branch-body GET + and all branch write paths (`api_branches`: start-edit-branch, accept / + decline / reask, manual-flush, promote-to-branch, metadata, branch chat), + the PR family (`api_prs`: pr-draft / open-pr / merge / withdraw / review / + resolution-branch), graduation (`api_graduation`: graduate / claim / retire + / unretire and the module-level orchestrator + state-flip helpers), + mark-reviewed, the idea-PR merge / decline / withdraw + proposal preview, + and metadata edit/bulk/migrate (`api_metadata`). `bot.open_metadata_pr` and + `bot.mark_entry_reviewed` gained a `file_path` parameter (was hardcoded to + the repo root). The branch cache (`refresh_meta_branches`), the webhook + corpus-refresh dispatch, and the hygiene branch-delete now span **every** + project's content repo, not just the default. +- **Collection-scoped body-read routes** (new, additive): + `GET /api/projects/{pid}/collections/{cid}/rfcs/{slug}/main` and + `…/rfcs/{slug}/branches/{branch}` disambiguate a slug that exists in two + collections (G-5) and read the entry's own repo. The slug-only routes are + kept and are now collection-aware via the entry's cached row, so existing + clients are unaffected. +- **Frontend.** `getRFCMain` / `getBranch` take optional `projectId` + + `collectionId` and call the scoped routes when present (RFCView threads its + `pid`/`cid`, mirroring the v0.52.1 entry-detail fix); both fall back to the + slug-only routes otherwise. + +No database migration and no configuration change. Existing default-collection +entries are unaffected (the resolver returns the default repo and a repo-root +path for them). + +_Known boundary:_ the branch / thread / change / PR cache tables remain +slug-keyed, so a slug that genuinely exists in two collections is still +ambiguous on the slug-only *write* routes; the collection-scoped body-read +routes resolve it for the canonical view, and the resolver makes every write +target the correct repo via the entry's own collection. Full slug +de-duplication across collections on the write family is tracked separately. + ## 0.52.3 — 2026-06-09 **Patch — unmatched routes render a 404 instead of a blank page or a diff --git a/VERSION b/VERSION index d572c9b..7f422a1 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.52.3 \ No newline at end of file +0.53.0 diff --git a/backend/app/api.py b/backend/app/api.py index 2157c5d..1cc7307 100644 --- a/backend/app/api.py +++ b/backend/app/api.py @@ -919,14 +919,18 @@ def make_router( raise HTTPException(404, "Not found") if row["state"] != "active" or not row["unreviewed"]: raise HTTPException(409, "Entry is not an unreviewed active entry") + # §22/G-15: write to the entry's project content_repo + collection + # subfolder, not the deployment default. + org, meta_repo, md_path = projects_mod.entry_location(config, collection_id, slug) try: await bot.mark_entry_reviewed( viewer.as_actor(), - org=config.gitea_org, - meta_repo=(projects_mod.default_content_repo(config) or ""), + org=org, + meta_repo=meta_repo, slug=slug, reviewed_by=viewer.gitea_login, reviewed_at=entry_mod.today(), + file_path=md_path, ) except GiteaError as e: raise HTTPException(502, f"Gitea: {e.detail}") @@ -1031,7 +1035,19 @@ def make_router( # Read the proposed entry file from the head branch. slug = row["rfc_slug"] head = row["head_branch"] - result = await gitea.read_file(config.gitea_org, (projects_mod.default_content_repo(config) or ""), f"rfcs/{slug}.md", ref=head) + # §22/G-15: the proposal lives in its project's content_repo under its + # collection's `/rfcs/`. cached_prs carries project_id but not + # collection_id, so resolve the repo from the project and locate the file + # by trying each of the project's collection subfolders (default first). + repo = (projects_mod.content_repo(row["project_id"]) + or projects_mod.default_content_repo(config) or "") + result = None + for col in collections_mod.list_collections(row["project_id"], include_unlisted=True): + sub = col["subfolder"] or "" + cand = f"{sub}/rfcs/{slug}.md" if sub else f"rfcs/{slug}.md" + result = await gitea.read_file(config.gitea_org, repo, cand, ref=head) + if result: + break entry_payload: dict[str, Any] | None = None if result: text, _sha = result @@ -1245,7 +1261,9 @@ def make_router( await bot.merge_idea_pr( user.as_actor(), org=config.gitea_org, - meta_repo=(projects_mod.default_content_repo(config) or ""), + # §22/G-15: the idea PR lives in its project's content_repo. + meta_repo=(projects_mod.content_repo(row["project_id"]) + or projects_mod.default_content_repo(config) or ""), pr_number=pr_number, slug=row["rfc_slug"], ) @@ -1265,7 +1283,8 @@ def make_router( await bot.decline_idea_pr( user.as_actor(), org=config.gitea_org, - meta_repo=(projects_mod.default_content_repo(config) or ""), + meta_repo=(projects_mod.content_repo(row["project_id"]) + or projects_mod.default_content_repo(config) or ""), pr_number=pr_number, slug=row["rfc_slug"], comment=body.comment, @@ -1289,7 +1308,8 @@ def make_router( await bot.withdraw_idea_pr( user.as_actor(), org=config.gitea_org, - meta_repo=(projects_mod.default_content_repo(config) or ""), + meta_repo=(projects_mod.content_repo(row["project_id"]) + or projects_mod.default_content_repo(config) or ""), pr_number=pr_number, slug=row["rfc_slug"], ) diff --git a/backend/app/api_branches.py b/backend/app/api_branches.py index 2a66b66..c071867 100644 --- a/backend/app/api_branches.py +++ b/backend/app/api_branches.py @@ -29,7 +29,7 @@ from fastapi import APIRouter, HTTPException, Request from fastapi.responses import StreamingResponse from pydantic import BaseModel, Field -from . import auth, cache, chat as chat_layer, db, entry as entry_mod, funder, metadata as metadata_mod, models_resolver, projects as projects_mod +from . import auth, cache, chat as chat_layer, collections as collections_mod, db, entry as entry_mod, funder, metadata as metadata_mod, models_resolver, projects as projects_mod from .bot import Bot from .config import Config from .gitea import Gitea, GiteaError @@ -170,10 +170,7 @@ def make_router( # open edit branches, open meta-repo body-edit and metadata PRs. # ------------------------------------------------------------------- - @router.get("/api/rfcs/{slug}/main") - async def get_rfc_main(slug: str, request: Request) -> dict[str, Any]: - viewer = auth.current_user(request) - rfc = _require_rfc(slug, viewer) + async def _main_payload(rfc, slug: str, viewer) -> dict[str, Any]: if rfc["state"] not in ("active", "super-draft"): raise HTTPException(409, f"RFC is {rfc['state']}") @@ -294,6 +291,23 @@ def make_router( "pre_graduation_history": pre_grad, } + @router.get("/api/rfcs/{slug}/main") + async def get_rfc_main(slug: str, request: Request) -> dict[str, Any]: + viewer = auth.current_user(request) + return await _main_payload(_require_rfc(slug, viewer), slug, viewer) + + @router.get("/api/projects/{project_id}/collections/{collection_id}/rfcs/{slug}/main") + async def get_rfc_main_scoped( + project_id: str, collection_id: str, slug: str, request: Request, + ) -> dict[str, Any]: + # §22/G-15: collection-scoped canonical-body read. Disambiguates a slug + # that exists in two collections (G-5) and resolves the entry's own + # content repo / subfolder via `_repo_for`/`_file_path_for`. + viewer = auth.current_user(request) + _check_collection_in_project(project_id, collection_id) + rfc = _require_rfc(slug, viewer, collection_id=collection_id) + return await _main_payload(rfc, slug, viewer) + # The bare `GET /api/rfcs//branches/` is declared # at the *bottom* of this router so the more-specific deeper GET # routes — `branches/{branch:path}/threads` and @@ -489,6 +503,7 @@ def make_router( org=owner, meta_repo=repo, slug=slug, + file_path=path, new_file_contents=new_content, prior_sha=prior_sha, pr_title=pr_title, @@ -1048,10 +1063,7 @@ def make_router( # else, including slashed branch names like `foo/bar`. # ------------------------------------------------------------------- - @router.get("/api/rfcs/{slug}/branches/{branch:path}") - async def get_branch_view(slug: str, branch: str, request: Request) -> dict[str, Any]: - viewer = auth.current_user(request) - rfc = _require_rfc_with_repo(slug, viewer) + async def _branch_view_payload(rfc, slug: str, branch: str, viewer) -> dict[str, Any]: if not _can_read_branch(slug, branch, viewer): raise HTTPException(403, "Branch is private") @@ -1118,12 +1130,48 @@ def make_router( "capabilities": capabilities, } + @router.get("/api/rfcs/{slug}/branches/{branch:path}") + async def get_branch_view(slug: str, branch: str, request: Request) -> dict[str, Any]: + viewer = auth.current_user(request) + rfc = _require_rfc_with_repo(slug, viewer) + return await _branch_view_payload(rfc, slug, branch, viewer) + + @router.get("/api/projects/{project_id}/collections/{collection_id}/rfcs/{slug}/branches/{branch:path}") + async def get_branch_view_scoped( + project_id: str, collection_id: str, slug: str, branch: str, request: Request, + ) -> dict[str, Any]: + # §22/G-15: collection-scoped branch-body read (the canonical-body GET + # RFCView renders). Disambiguates a slug across collections (G-5) and + # reads the entry's own content repo / subfolder. + viewer = auth.current_user(request) + _check_collection_in_project(project_id, collection_id) + rfc = _require_rfc_with_repo(slug, viewer, collection_id=collection_id) + return await _branch_view_payload(rfc, slug, branch, viewer) + # ------------------------------------------------------------------ # Permission + state helpers (closures, share `config` etc.) # ------------------------------------------------------------------ - def _require_rfc(slug: str, viewer): - row = db.conn().execute("SELECT *, (SELECT c.project_id FROM collections c WHERE c.id = cached_rfcs.collection_id) AS project_id FROM cached_rfcs WHERE slug = ?", (slug,)).fetchone() + def _check_collection_in_project(project_id: str, collection_id: str) -> None: + """§22/G-15: a collection-scoped route 404s when the collection isn't in + the named project (matches api_metadata's guard).""" + if collections_mod.project_of_collection(collection_id) != project_id: + raise HTTPException(404, "Collection not in project") + + def _require_rfc(slug: str, viewer, collection_id: str | None = None): + # §22/G-15: when a collection_id is supplied (the collection-scoped + # body-read routes), scope the lookup to that collection so a slug that + # exists in two collections resolves unambiguously (G-5); otherwise the + # legacy slug-only lookup picks the entry by slug alone. + if collection_id is not None: + row = db.conn().execute( + "SELECT *, (SELECT c.project_id FROM collections c WHERE c.id = cached_rfcs.collection_id) AS project_id " + "FROM cached_rfcs WHERE slug = ? AND collection_id = ?", + (slug, collection_id)).fetchone() + else: + row = db.conn().execute( + "SELECT *, (SELECT c.project_id FROM collections c WHERE c.id = cached_rfcs.collection_id) AS project_id " + "FROM cached_rfcs WHERE slug = ?", (slug,)).fetchone() if row is None: raise HTTPException(404, "RFC not found") # §22.5 visibility gate (subtractive, §22.7): a gated project's entries @@ -1131,13 +1179,13 @@ def make_router( auth.require_project_readable(viewer, row["project_id"]) return row - def _require_rfc_with_repo(slug: str, viewer): + def _require_rfc_with_repo(slug: str, viewer, collection_id: str | None = None): """Used by every branch-scoped endpoint. Under the meta-only topology (§1) the meta repo is the implicit target for every entry — super-draft and active alike — so there is no per-RFC repo check. The name is retained for call-site stability; a withdrawn entry is still rejected.""" - row = _require_rfc(slug, viewer) + row = _require_rfc(slug, viewer, collection_id) if row["state"] == "withdrawn": raise HTTPException(409, "RFC is withdrawn") return row @@ -1183,14 +1231,23 @@ def make_router( return _is_meta_branch_name(branch) def _repo_for(rfc, branch: str = "main") -> tuple[str, str]: + # §22/G-15: a meta-resident entry's repo is its COLLECTION's project + # content_repo (collection → project → content_repo), not the deployment + # default — so an entry in a non-default project reads/writes its own + # repo. `entry_location` falls back to the default repo for a legacy / + # unknown collection, preserving the single-corpus behaviour. if _is_meta_target(rfc, branch): - return config.gitea_org, (projects_mod.default_content_repo(config) or "") + org, repo, _ = projects_mod.entry_location(config, rfc["collection_id"], rfc["slug"]) + return org, repo owner, repo = rfc["repo"].split("/", 1) return owner, repo def _file_path_for(rfc, branch: str = "main") -> str: + # §22/G-15: path is the collection's `/rfcs/.md` + # (repo root `rfcs/.md` for a default collection). if _is_meta_target(rfc, branch): - return f"rfcs/{rfc['slug']}.md" + _, _, path = projects_mod.entry_location(config, rfc["collection_id"], rfc["slug"]) + return path return RFC_FILE_PATH def _extract_body(rfc, file_contents: str, branch: str = "main") -> str: diff --git a/backend/app/api_graduation.py b/backend/app/api_graduation.py index 6161cf6..6afc516 100644 --- a/backend/app/api_graduation.py +++ b/backend/app/api_graduation.py @@ -344,12 +344,14 @@ def make_router( # Dual-read the meta-repo entry once (§22.4a sidecar-aware) — we need its # git state for the graduation commit and the body to carry through # unchanged (meta-only keeps the body in the entry, §13.3). - st = await metadata_mod.read_entry_from_git( - gitea, config.gitea_org, - (projects_mod.default_content_repo(config) or ""), f"rfcs/{slug}.md", - ) + # §22/G-15: resolve the repo+path from the entry's COLLECTION, not the + # deployment default, so an entry in a non-default project graduates in + # its own content repo / collection subfolder. + org, meta_repo, md_path = projects_mod.entry_location( + config, rfc["collection_id"], slug) + st = await metadata_mod.read_entry_from_git(gitea, org, meta_repo, md_path) if st is None: - raise HTTPException(409, f"Meta entry rfcs/{slug}.md not found on main") + raise HTTPException(409, f"Meta entry {md_path} not found on main") super_draft_entry = st.entry arbiters = json.loads(rfc["arbiters_json"] or "[]") or owners[:1] @@ -378,7 +380,7 @@ def make_router( extra=dict(super_draft_entry.extra), ) graduation_files = metadata_mod.write_entry_files( - f"rfcs/{slug}.md", graduated_entry, st) + md_path, graduated_entry, st) state = _new_active( slug, rfc_id=rfc_id, owners=owners, arbiters=arbiters, @@ -398,6 +400,7 @@ def make_router( coro = _orchestrate( config=config, gitea=gitea, bot=bot, actor=viewer.as_actor(), state=state, + meta_repo=meta_repo, graduation_files=graduation_files, ) if request.query_params.get("_sync") == "1": @@ -478,21 +481,21 @@ def make_router( if already: raise HTTPException(409, f"A claim PR is already open: #{already['pr_number']}") - st = await metadata_mod.read_entry_from_git( - gitea, config.gitea_org, - (projects_mod.default_content_repo(config) or ""), f"rfcs/{slug}.md", - ) + # §22/G-15: resolve repo+path from the entry's collection, not the default. + org, meta_repo, md_path = projects_mod.entry_location( + config, rfc["collection_id"], slug) + st = await metadata_mod.read_entry_from_git(gitea, org, meta_repo, md_path) if st is None: - raise HTTPException(409, f"Meta entry rfcs/{slug}.md not found on main") + raise HTTPException(409, f"Meta entry {md_path} not found on main") if viewer.gitea_login in st.entry.owners: return {"ok": True, "noop": True} ent = metadata_mod.apply_values( st.entry, {"owners": st.entry.owners + [viewer.gitea_login]}) - files = metadata_mod.write_entry_files(f"rfcs/{slug}.md", ent, st) + files = metadata_mod.write_entry_files(md_path, ent, st) try: pr = await bot.open_claim_pr( viewer.as_actor(), - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=org, meta_repo=meta_repo, slug=slug, files=files, ) @@ -521,12 +524,15 @@ def make_router( 403, "Only this RFC's owners or a site owner may retire it" ) prior_state = rfc["state"] - st = await _read_meta_entry(slug) + # §22/G-15: resolve repo+path from the entry's collection, not the default. + org, meta_repo, md_path = projects_mod.entry_location( + config, rfc["collection_id"], slug) + st = await _read_meta_entry(org, meta_repo, md_path) entry = metadata_mod.apply_values(st.entry, {"state": "retired"}) - files = metadata_mod.write_entry_files(f"rfcs/{slug}.md", entry, st) + files = metadata_mod.write_entry_files(md_path, entry, st) await _run_state_flip( config=config, gitea=gitea, bot=bot, actor=viewer.as_actor(), - slug=slug, files=files, + meta_repo=meta_repo, slug=slug, files=files, verb="retire", target_state="retired", ) _audit( @@ -550,14 +556,17 @@ def make_router( viewer = auth.require_user(request) if viewer.role != "owner": raise HTTPException(403, "Only a site owner may un-retire an RFC") - _require_retired(slug) + rfc = _require_retired(slug) restored = _prior_state_before_retire(slug) - st = await _read_meta_entry(slug) + # §22/G-15: resolve repo+path from the entry's collection, not the default. + org, meta_repo, md_path = projects_mod.entry_location( + config, rfc["collection_id"], slug) + st = await _read_meta_entry(org, meta_repo, md_path) entry = metadata_mod.apply_values(st.entry, {"state": restored}) - files = metadata_mod.write_entry_files(f"rfcs/{slug}.md", entry, st) + files = metadata_mod.write_entry_files(md_path, entry, st) await _run_state_flip( config=config, gitea=gitea, bot=bot, actor=viewer.as_actor(), - slug=slug, files=files, + meta_repo=meta_repo, slug=slug, files=files, verb="unretire", target_state=restored, ) _audit( @@ -598,16 +607,14 @@ def make_router( raise HTTPException(409, f"RFC is {row['state']}, not retired") return row - async def _read_meta_entry(slug: str): + async def _read_meta_entry(org: str, repo: str, md_path: str): """Dual-read an entry from meta-main → EntryGitState (sidecar-aware, §22.4a). A migrated body-only `.md` reads cleanly; never raises on bad - metadata (INV-3).""" - st = await metadata_mod.read_entry_from_git( - gitea, config.gitea_org, - (projects_mod.default_content_repo(config) or ""), f"rfcs/{slug}.md", - ) + metadata (INV-3). `org`/`repo`/`md_path` are collection-resolved by the + caller (§22/G-15) so a non-default project's entry reads its own repo.""" + st = await metadata_mod.read_entry_from_git(gitea, org, repo, md_path) if st is None: - raise HTTPException(409, f"Meta entry rfcs/{slug}.md not found on main") + raise HTTPException(409, f"Meta entry {md_path} not found on main") return st async def _refresh_catalog() -> None: @@ -636,6 +643,7 @@ async def _orchestrate( bot: Bot, actor: Actor, state: GraduationState, + meta_repo: str, graduation_files: list[dict], ) -> None: """Open the flip PR, then merge it. Two steps, no transaction: @@ -654,7 +662,7 @@ async def _orchestrate( try: pr = await bot.open_graduation_pr( actor, - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=config.gitea_org, meta_repo=meta_repo, slug=state.slug, files=graduation_files, rfc_id=state.rfc_id, @@ -673,14 +681,15 @@ async def _orchestrate( try: await bot.merge_graduation_pr( actor, - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=config.gitea_org, meta_repo=meta_repo, pr_number=state.new_pr_number, head_branch=state.graduation_branch or "", slug=state.slug, rfc_id=state.rfc_id, ) except GiteaError as e: await _fail(state, "merge_pr", f"Gitea: {e.detail}") - await _cleanup_unmerged(config=config, bot=bot, actor=actor, state=state) + await _cleanup_unmerged( + config=config, bot=bot, actor=actor, state=state, meta_repo=meta_repo) await _finish_failed(state, failed_at="merge_pr", on_behalf_of=actor.gitea_login) return await _done(state, "merge_pr", f"PR #{state.new_pr_number} merged") @@ -723,7 +732,7 @@ async def _orchestrate( async def _cleanup_unmerged( - *, config: Config, bot: Bot, actor: Actor, state: GraduationState, + *, config: Config, bot: Bot, actor: Actor, state: GraduationState, meta_repo: str, ) -> None: """A merge failure leaves the flip PR open on its `graduate--` branch. Close the PR and delete the branch so failed attempts don't @@ -735,7 +744,7 @@ async def _cleanup_unmerged( try: await bot.close_graduation_pr( actor, - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=config.gitea_org, meta_repo=meta_repo, pr_number=state.new_pr_number, head_branch=state.graduation_branch or "", slug=state.slug, reason="graduation merge failed", @@ -748,7 +757,7 @@ async def _cleanup_unmerged( await bot.delete_branch( actor, owner=config.gitea_org, - repo=(projects_mod.default_content_repo(config) or ""), + repo=meta_repo, branch=branch_name, slug=state.slug, action_kind="delete_post_merge_branch", @@ -843,6 +852,7 @@ async def _run_state_flip( gitea: Gitea, bot: Bot, actor: Actor, + meta_repo: str, slug: str, files: list[dict], verb: str, @@ -858,7 +868,7 @@ async def _run_state_flip( try: pr = await bot.open_retire_flip_pr( actor, - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=config.gitea_org, meta_repo=meta_repo, slug=slug, files=files, verb=verb, target_state=target_state, ) @@ -869,7 +879,7 @@ async def _run_state_flip( try: await bot.merge_retire_flip_pr( actor, - org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + org=config.gitea_org, meta_repo=meta_repo, pr_number=pr_number, head_branch=head_branch, slug=slug, verb=verb, ) @@ -878,12 +888,12 @@ async def _run_state_flip( # accumulate (mirrors graduation's `_cleanup_unmerged`). try: await bot.close_graduation_pr( - actor, org=config.gitea_org, meta_repo=(projects_mod.default_content_repo(config) or ""), + actor, org=config.gitea_org, meta_repo=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=(projects_mod.default_content_repo(config) or ""), + actor, owner=config.gitea_org, repo=meta_repo, branch=head_branch, slug=slug, action_kind="delete_post_merge_branch", reason=f"{verb} merge failed", diff --git a/backend/app/api_metadata.py b/backend/app/api_metadata.py index ef8d565..f6ef3e9 100644 --- a/backend/app/api_metadata.py +++ b/backend/app/api_metadata.py @@ -54,8 +54,13 @@ def _apply_op(entry: Any, op: str, field: str, value: Any) -> Any: def make_router(config: Config, gitea: Gitea, bot: Bot) -> APIRouter: router = APIRouter() - def _content_repo() -> tuple[str, str]: - return config.gitea_org, (projects_mod.default_content_repo(config) or "") + def _content_repo(collection_id: str) -> tuple[str, str]: + # §22/G-15: the COLLECTION's project content_repo, not the deployment + # default — an entry in a non-default project writes its own repo. + # Falls back to the default repo for an unknown collection. + repo = (projects_mod.content_repo_for_collection(collection_id) + or (projects_mod.default_content_repo(config) or "")) + return config.gitea_org, repo def _md_path(collection_id: str, slug: str) -> str: sub = collections_mod.subfolder_of(collection_id) or "" @@ -83,7 +88,7 @@ def make_router(config: Config, gitea: Gitea, bot: Bot) -> APIRouter: if unknown: raise HTTPException(422, f"Unknown field(s): {', '.join(sorted(unknown))}") - org, repo = _content_repo() + org, repo = _content_repo(collection_id) md_path = _md_path(collection_id, slug) st = await metadata_mod.read_entry_from_git(gitea, org, repo, md_path) if st is None: @@ -144,7 +149,7 @@ def make_router(config: Config, gitea: Gitea, bot: Bot) -> APIRouter: if body.op in ("add", "remove") and fields[body.field].get("type") != "tags": raise HTTPException(422, f"op {body.op} requires a tags field") - org, repo = _content_repo() + org, repo = _content_repo(collection_id) applied: list[str] = [] rejected: list[dict[str, str]] = [] all_ops: list[dict[str, Any]] = [] @@ -195,7 +200,7 @@ def make_router(config: Config, gitea: Gitea, bot: Bot) -> APIRouter: raise HTTPException(404, "Collection not in project") if not auth.is_collection_superuser(viewer, collection_id): raise HTTPException(403, "Owner access required to migrate a collection") - org, repo = _content_repo() + org, repo = _content_repo(collection_id) subfolder = collections_mod.subfolder_of(collection_id) or "" try: result = await metadata_mod.migrate_collection( diff --git a/backend/app/api_prs.py b/backend/app/api_prs.py index d71629c..f6e9818 100644 --- a/backend/app/api_prs.py +++ b/backend/app/api_prs.py @@ -690,14 +690,20 @@ def make_router( return not rfc["repo"] def _owner_repo(rfc) -> tuple[str, str]: + # §22/G-15: a meta-resident entry's repo is its COLLECTION's project + # content_repo, not the deployment default (entry_location falls back to + # the default for a legacy/unknown collection). if _is_meta_resident(rfc): - return config.gitea_org, (projects_mod.default_content_repo(config) or "") + org, repo, _ = projects_mod.entry_location(config, rfc["collection_id"], rfc["slug"]) + return org, repo owner, repo = rfc["repo"].split("/", 1) return owner, repo def _file_path_for(rfc) -> str: + # §22/G-15: the collection's `/rfcs/.md`. if _is_meta_resident(rfc): - return f"rfcs/{rfc['slug']}.md" + _, _, path = projects_mod.entry_location(config, rfc["collection_id"], rfc["slug"]) + return path return RFC_FILE_PATH def _extract_body(rfc, file_contents: str) -> str: diff --git a/backend/app/bot.py b/backend/app/bot.py index f1f363b..8b683b5 100644 --- a/backend/app/bot.py +++ b/backend/app/bot.py @@ -450,17 +450,19 @@ class Bot: org: str, meta_repo: str, slug: str, + file_path: str, new_file_contents: str, prior_sha: str, pr_title: str, pr_description: str, ) -> dict: """Per §9.5: a metadata-pane edit (title or tags) on a super-draft - opens a tiny meta-repo PR that touches only the frontmatter of - `rfcs/.md`. One commit, one PR, easy to triage. The branch - name uses the dash-separated `metadata--<6hex>` shape — same - routing-friendly form Slice 4 picked for edit branches per the - §19.2 path-routing candidate. + opens a tiny meta-repo PR that touches only the frontmatter of the + entry's `.md`. `file_path` is the collection-resolved path (§22/G-15: + `/rfcs/.md`), not assumed to be at the repo root. One + commit, one PR, easy to triage. The branch name uses the dash-separated + `metadata--<6hex>` shape — same routing-friendly form Slice 4 + picked for edit branches per the §19.2 path-routing candidate. """ import secrets @@ -471,7 +473,7 @@ class Bot: result = await self._gitea.update_file( org, meta_repo, - f"rfcs/{slug}.md", + file_path, content=new_file_contents, sha=prior_sha, message=commit_message, @@ -1210,13 +1212,18 @@ class Bot: slug: str, reviewed_by: str, reviewed_at: str, + file_path: str | None = None, ) -> None: """Clear §22.4c unreviewed on an active entry by writing its metadata sidecar on main (§22.4a). Dual-reads the entry (so a migrated body-only `.md` doesn't crash) and lazy-migrates a legacy `.md` to body-only in the same commit. Stamps the §6.5 On-behalf-of trailer and writes an - actions-log row, mirroring the graduation stamp's bot-write shape.""" - path = f"rfcs/{slug}.md" + actions-log row, mirroring the graduation stamp's bot-write shape. + + `file_path` is the collection-resolved entry path (§22/G-15: + `/rfcs/.md`); it defaults to the repo-root + `rfcs/.md` for the legacy single-corpus / default-collection case.""" + path = file_path or f"rfcs/{slug}.md" st = await metadata_mod.read_entry_from_git(self._gitea, org, meta_repo, path) if st is None: raise GiteaError(404, f"{path} not found") diff --git a/backend/app/cache.py b/backend/app/cache.py index 538689a..dde6d36 100644 --- a/backend/app/cache.py +++ b/backend/app/cache.py @@ -426,73 +426,85 @@ async def refresh_meta_branches(config: Config, gitea: Gitea) -> None: of slashes per the §19.2 path-routing candidate. """ org = config.gitea_org - repo = projects_mod.default_content_repo(config) - if not repo: - log.warning("refresh_meta_branches: default project has no content_repo yet; skipping") - return - try: - branches = await gitea.list_branches(org, repo) - except GiteaError as e: - log.warning("refresh_meta_branches: %s", e) + # §22/G-15: scan EVERY project's content_repo (not just the default), so an + # entry in a non-default project gets its edit branches + synthesized `main` + # row cached and its branch dropdown / has-commits-ahead check work. Mirrors + # refresh_meta_pulls' per-project loop. + prows = db.conn().execute( + "SELECT id, content_repo FROM projects WHERE content_repo IS NOT NULL AND content_repo != ''" + ).fetchall() + if not prows: + log.warning("refresh_meta_branches: no projects with a content_repo yet; skipping") return - meta_main_sha = "" - meta_main_ts = None edit_keys_seen: set[tuple[str, str]] = set() - for b in branches: - name = b.get("name") or "" - head_sha = (b.get("commit") or {}).get("id") or "" - last_commit_at = (b.get("commit") or {}).get("timestamp") - if name == "main": - meta_main_sha = head_sha - meta_main_ts = last_commit_at + for prow in prows: + repo = prow["content_repo"] + try: + branches = await gitea.list_branches(org, repo) + except GiteaError as e: + log.warning("refresh_meta_branches: %s (%s)", e, repo) continue - slug = _slug_from_branch_name(name) - if not slug: - continue - rfc = db.conn().execute( - "SELECT state, repo FROM cached_rfcs WHERE slug = ?", (slug,) - ).fetchone() - # Meta-only topology (§1): edit branches live on the meta repo for - # every meta-resident entry — super-drafts and active RFCs alike - # (active RFCs are graduated in place and keep editing here, §13). - # A legacy per-RFC repo (repo set) is the only thing excluded. - if not rfc or rfc["repo"] or rfc["state"] not in ("super-draft", "active"): - continue - edit_keys_seen.add((slug, name)) - db.conn().execute( - """ - INSERT INTO cached_branches (rfc_slug, branch_name, head_sha, state, last_commit_at) - VALUES (?, ?, ?, 'open', ?) - ON CONFLICT(collection_id, rfc_slug, branch_name) DO UPDATE SET - head_sha = excluded.head_sha, - state = CASE WHEN cached_branches.state = 'closed' THEN 'closed' ELSE 'open' END, - last_commit_at = excluded.last_commit_at - """, - (slug, name, head_sha, last_commit_at), - ) - # Synthesize a per-slug `main` row for every super-draft entry, so the - # §10.1 has-commits-ahead check in api_prs.py works uniformly. The - # head_sha is the meta-repo main's tip — every super-draft edit branch - # diverges from this single point. - if meta_main_sha: - super_drafts = db.conn().execute( - "SELECT slug FROM cached_rfcs " - "WHERE repo IS NULL AND state IN ('super-draft', 'active')" - ).fetchall() - for r in super_drafts: + meta_main_sha = "" + meta_main_ts = None + for b in branches: + name = b.get("name") or "" + head_sha = (b.get("commit") or {}).get("id") or "" + last_commit_at = (b.get("commit") or {}).get("timestamp") + if name == "main": + meta_main_sha = head_sha + meta_main_ts = last_commit_at + continue + slug = _slug_from_branch_name(name) + if not slug: + continue + rfc = db.conn().execute( + "SELECT state, repo FROM cached_rfcs WHERE slug = ?", (slug,) + ).fetchone() + # Meta-only topology (§1): edit branches live on the content repo for + # every meta-resident entry — super-drafts and active RFCs alike + # (active RFCs are graduated in place and keep editing here, §13). + # A legacy per-RFC repo (repo set) is the only thing excluded. + if not rfc or rfc["repo"] or rfc["state"] not in ("super-draft", "active"): + continue + edit_keys_seen.add((slug, name)) db.conn().execute( """ INSERT INTO cached_branches (rfc_slug, branch_name, head_sha, state, last_commit_at) - VALUES (?, 'main', ?, 'open', ?) + VALUES (?, ?, ?, 'open', ?) ON CONFLICT(collection_id, rfc_slug, branch_name) DO UPDATE SET head_sha = excluded.head_sha, + state = CASE WHEN cached_branches.state = 'closed' THEN 'closed' ELSE 'open' END, last_commit_at = excluded.last_commit_at """, - (r["slug"], meta_main_sha, meta_main_ts), + (slug, name, head_sha, last_commit_at), ) + # Synthesize a per-slug `main` row for this project's super-draft/active + # entries, so the §10.1 has-commits-ahead check works uniformly. The + # head_sha is this content repo's main tip — every edit branch in the + # project diverges from that single point. + if meta_main_sha: + super_drafts = db.conn().execute( + "SELECT r.slug AS slug FROM cached_rfcs r " + "JOIN collections c ON c.id = r.collection_id " + "WHERE r.repo IS NULL AND r.state IN ('super-draft', 'active') " + " AND c.project_id = ?", + (prow["id"],), + ).fetchall() + for r in super_drafts: + db.conn().execute( + """ + INSERT INTO cached_branches (rfc_slug, branch_name, head_sha, state, last_commit_at) + VALUES (?, 'main', ?, 'open', ?) + ON CONFLICT(collection_id, rfc_slug, branch_name) DO UPDATE SET + head_sha = excluded.head_sha, + last_commit_at = excluded.last_commit_at + """, + (r["slug"], meta_main_sha, meta_main_ts), + ) + # Mark previously-known edit branches that disappeared as deleted per # §11.5 / §12. Keep the row so chat history survives the branch's # deletion in Gitea. diff --git a/backend/app/hygiene.py b/backend/app/hygiene.py index 7431966..6885521 100644 --- a/backend/app/hygiene.py +++ b/backend/app/hygiene.py @@ -293,20 +293,21 @@ async def _delete_branch_via_bot( (we leave the branch row in place — a subsequent reconciler sweep will reconcile or the operator can intervene).""" rfc = db.conn().execute( - "SELECT state, repo FROM cached_rfcs WHERE slug = ?", (slug,) + "SELECT state, repo, collection_id FROM cached_rfcs WHERE slug = ?", (slug,) ).fetchone() if rfc is None: log.warning("hygiene: cannot delete %s/%s — slug missing from cache", slug, branch) return False if not rfc["repo"]: - repo = projects_mod.default_content_repo(config) + # §22/G-15: the edit/graduation branch lives on the entry's COLLECTION's + # project content_repo, not the deployment default. + owner, repo, _ = projects_mod.entry_location(config, rfc["collection_id"], slug) if not repo: log.warning( - "hygiene: default project has no content_repo; skipping branch delete for %s/%s", + "hygiene: no content_repo resolved; skipping branch delete for %s/%s", slug, branch, ) return False - owner = config.gitea_org elif "/" in rfc["repo"]: owner, repo = rfc["repo"].split("/", 1) else: diff --git a/backend/app/projects.py b/backend/app/projects.py index 885c18f..68ad8a4 100644 --- a/backend/app/projects.py +++ b/backend/app/projects.py @@ -201,6 +201,37 @@ def content_repo(project_id: str) -> str | None: return row["content_repo"] if row and row["content_repo"] else None +def content_repo_for_collection(collection_id: str) -> str | None: + """The content repo a collection's entries live in (§22 three-tier write + path, G-15): collection → project → content_repo. None if the collection or + its project is unknown/unset. The per-collection successor to + `default_content_repo` for the WRITE path — an entry in a non-default + project must read/write that project's repo, not the deployment default.""" + from . import collections as collections_mod + pid = collections_mod.project_of_collection(collection_id) + return content_repo(pid) if pid else None + + +def entry_location(config: Config, collection_id: str, slug: str) -> tuple[str, str, str]: + """The git location `(gitea_org, content_repo, md_path)` of an entry, resolved + from its collection (§22 three-tier, G-15). + + Repo: the collection's project content_repo, falling back to the deployment + default project's repo when the collection (or its project) is unknown — so a + legacy/single-corpus entry still resolves to a usable location rather than an + empty repo. Path: `/rfcs/.md`, or `rfcs/.md` at the + repo root for a default (subfolder-less) collection. + + This is the single resolver the branch/edit/body/metadata/graduation write + paths share, replacing the hardcoded `default_content_repo` + `rfcs/.md`. + """ + from . import collections as collections_mod + repo = content_repo_for_collection(collection_id) or (default_content_repo(config) or "") + sub = collections_mod.subfolder_of(collection_id) + rfcs_dir = f"{sub}/rfcs" if sub else "rfcs" + return config.gitea_org, repo, f"{rfcs_dir}/{slug}.md" + + def project_initial_state(project_id: str) -> str: """§22.4b landing state for new entries in a project's default collection (the per-corpus field moved down to the collection in migration 029). diff --git a/backend/app/webhooks.py b/backend/app/webhooks.py index 56d0412..591aa6e 100644 --- a/backend/app/webhooks.py +++ b/backend/app/webhooks.py @@ -80,10 +80,17 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: payload = {} repo_full = (payload.get("repository") or {}).get("full_name") or "" registry_full = f"{config.gitea_org}/{config.registry_repo}" - content_repo = projects_mod.default_content_repo(config) - if not content_repo: - log.warning("webhook: default project content_repo is unknown; corpus refresh skipped") - content_full = f"{config.gitea_org}/{content_repo}" if content_repo else None + # §22/G-15: a corpus push can land on ANY project's content_repo, not + # just the default — recognise the full set so a non-default project's + # push triggers the (multi-project) corpus/branch/PR refresh. + content_fulls = { + f"{config.gitea_org}/{r['content_repo']}" + for r in db.conn().execute( + "SELECT content_repo FROM projects " + "WHERE content_repo IS NOT NULL AND content_repo != ''") + } + if not content_fulls: + log.warning("webhook: no project content_repo is known; corpus refresh skipped") try: if repo_full == registry_full: # §22.2: a registry-repo push re-mirrors the projects table. @@ -94,7 +101,7 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: await registry_mod.refresh_registry(config, gitea) except registry_mod.RegistryError: log.exception("registry webhook: invalid projects.yaml; keeping last-good") - elif content_full and (repo_full == content_full or not repo_full): + elif content_fulls and (repo_full in content_fulls or not repo_full): await cache.refresh_meta_repo(config, gitea) await cache.refresh_meta_branches(config, gitea) await cache.refresh_meta_pulls(config, gitea) diff --git a/backend/tests/test_g15_branch_collection_scoped.py b/backend/tests/test_g15_branch_collection_scoped.py new file mode 100644 index 0000000..3007bc9 --- /dev/null +++ b/backend/tests/test_g15_branch_collection_scoped.py @@ -0,0 +1,192 @@ +"""G-15 — the branch/body subsystem is three-tier (project/collection) aware. + +Before G-15 the branch-body GET resolved every meta-resident entry to the +default project's content repo at `rfcs/.md`, so an entry in a named +collection (subfolder) or a non-default project's repo rendered a BLANK +canonical body and its edit/PR/body-write paths hit the wrong file. These tests +seed entries outside the default collection and assert the collection-scoped +body-read routes (and the now-collection-aware slug-only routes) read the +correct repo + subfolder. +""" +from __future__ import annotations + +import asyncio + +from app import cache as cache_mod, db, gitea as gitea_mod +from app.config import load_config + +from fastapi.testclient import TestClient # noqa: E402 +from test_propose_vertical import ( # noqa: E402,F401 + app_with_fake_gitea, tmp_env, provision_user_row, sign_in_as, +) + + +def _entry_md(slug, title, state="active"): + return f"---\nslug: {slug}\ntitle: {title}\nstate: {state}\n---\nthe canonical body\n" + + +def _add_features_collection(): + """A named 'features' collection (subfolder 'features') under the seeded + default project — same content repo ('meta'), distinct subfolder.""" + db.conn().execute( + "INSERT OR REPLACE INTO collections (id, project_id, type, subfolder, " + "initial_state, visibility, name, created_at, updated_at) VALUES " + "('features','default','document','features','super-draft','public','Features', " + "datetime('now'), datetime('now'))") + + +def _add_distinct_project(): + """A second project with its OWN content repo + a default (root) collection + — the live OHM dogfood shape (rfc-app project at rfc-app-content).""" + db.conn().execute( + "INSERT OR REPLACE INTO projects (id, name, content_repo, visibility, updated_at) " + "VALUES ('rfc-app','RFC App','rfc-app-content','public', datetime('now'))") + db.conn().execute( + "INSERT OR REPLACE INTO collections (id, project_id, type, subfolder, " + "initial_state, visibility, name, created_at, updated_at) VALUES " + "('rfc-app','rfc-app','document','','super-draft','public','RFC App', " + "datetime('now'), datetime('now'))") + + +def _mirror(): + cfg = load_config() + asyncio.run(cache_mod.refresh_meta_repo(cfg, gitea_mod.Gitea(cfg))) + + +# --- named collection (subfolder) under the default project ------------------- + +def test_branch_view_named_collection_reads_subfolder(app_with_fake_gitea): + app, fake = app_with_fake_gitea + with TestClient(app) as client: + _add_features_collection() + fake.files[("wiggleverse", "meta", "main", "features/rfcs/feat.md")] = { + "content": _entry_md("feat", "Feature Entry"), "sha": "sf"} + _mirror() + # cached_rfcs is keyed by the named collection. + assert db.conn().execute( + "SELECT collection_id FROM cached_rfcs WHERE slug='feat'" + ).fetchone()["collection_id"] == "features" + + # Collection-scoped branch view renders the body (was blank pre-G-15). + r = client.get( + "/api/projects/default/collections/features/rfcs/feat/branches/main") + assert r.status_code == 200, r.text + assert r.json()["body"] == "the canonical body\n" + + # The slug-only legacy route is now collection-aware too: it resolves + # the entry's own subfolder via the cached row, so it also renders. + r2 = client.get("/api/rfcs/feat/branches/main") + assert r2.status_code == 200, r2.text + assert r2.json()["body"] == "the canonical body\n" + + +def test_main_view_named_collection_scoped_route(app_with_fake_gitea): + app, fake = app_with_fake_gitea + with TestClient(app) as client: + _add_features_collection() + fake.files[("wiggleverse", "meta", "main", "features/rfcs/feat.md")] = { + "content": _entry_md("feat", "Feature Entry"), "sha": "sf"} + _mirror() + r = client.get("/api/projects/default/collections/features/rfcs/feat/main") + assert r.status_code == 200, r.text + assert r.json()["title"] == "Feature Entry" + + +# --- non-default project with a DISTINCT content repo (the OHM dogfood) ------- + +def test_branch_view_distinct_project_repo(app_with_fake_gitea): + app, fake = app_with_fake_gitea + with TestClient(app) as client: + _add_distinct_project() + fake.files[("wiggleverse", "rfc-app-content", "main", "rfcs/scoped.md")] = { + "content": _entry_md("scoped", "Scoped Admin IA"), "sha": "s1"} + _mirror() + assert db.conn().execute( + "SELECT collection_id FROM cached_rfcs WHERE slug='scoped'" + ).fetchone()["collection_id"] == "rfc-app" + + # Scoped read resolves the entry's OWN content repo (rfc-app-content). + r = client.get( + "/api/projects/rfc-app/collections/rfc-app/rfcs/scoped/branches/main") + assert r.status_code == 200, r.text + assert r.json()["body"] == "the canonical body\n" + + # And the slug-only route resolves the right repo via the cached row. + r2 = client.get("/api/rfcs/scoped/branches/main") + assert r2.status_code == 200, r2.text + assert r2.json()["body"] == "the canonical body\n" + + +# --- guards + regression ------------------------------------------------------ + +def test_scoped_branch_route_404_for_collection_outside_project(app_with_fake_gitea): + app, _ = app_with_fake_gitea + with TestClient(app) as client: + r = client.get( + "/api/projects/default/collections/nope/rfcs/x/branches/main") + assert r.status_code == 404 + + +def _seed_super_draft_in_collection(fake, *, slug, collection_id, subfolder, owners): + import json as _json + + import yaml + md_path = f"{subfolder}/rfcs/{slug}.md" if subfolder else f"rfcs/{slug}.md" + fm = {"slug": slug, "title": slug.title(), "state": "super-draft", "id": None, + "repo": None, "proposed_by": owners[0], "proposed_at": "2026-05-23", + "graduated_at": None, "graduated_by": None, + "owners": owners, "arbiters": owners[:1], "tags": []} + body = "the body\n" + text = f"---\n{yaml.safe_dump(fm, sort_keys=False).rstrip()}\n---\n\n{body}" + sha = fake._next_sha() + fake.files[("wiggleverse", "meta", "main", md_path)] = {"content": text, "sha": sha} + db.conn().execute( + "INSERT OR REPLACE INTO cached_rfcs (slug, title, state, rfc_id, repo, " + "proposed_by, proposed_at, owners_json, arbiters_json, tags_json, body, " + "body_sha, collection_id, last_main_commit_at, last_entry_commit_at) " + "VALUES (?,?, 'super-draft', NULL, NULL, ?, '2026-05-23', ?, ?, '[]', ?, ?, ?, " + "datetime('now'), datetime('now'))", + (slug, slug.title(), owners[0], _json.dumps(owners), _json.dumps(owners[:1]), + body, sha, collection_id)) + + +def test_graduate_in_named_collection_writes_to_subfolder(app_with_fake_gitea): + """G-15 write path: graduating a super-draft that lives in a named + collection flips the entry in that collection's `/rfcs/.md`, + not the default `rfcs/.md`.""" + app, fake = app_with_fake_gitea + with TestClient(app) as client: + _add_features_collection() + provision_user_row(user_id=1, login="ben", role="owner") + _seed_super_draft_in_collection( + fake, slug="gradme", collection_id="features", subfolder="features", + owners=["ben"]) + sign_in_as(client, user_id=1, gitea_login="ben", display_name="Ben", + role="owner", email="ben@x") + r = client.post("/api/rfcs/gradme/graduate?_sync=1", + json={"rfc_id": "RFC-0007", "owners": ["ben"]}) + assert r.status_code == 200, r.text + # The flip landed in the collection's subfolder, not the repo root. + sc = fake.files.get( + ("wiggleverse", "meta", "main", "features/rfcs/gradme.meta.yaml")) + assert sc is not None, "sidecar not written under the collection subfolder" + import yaml as _yaml + assert _yaml.safe_load(sc["content"])["state"] == "active" + # Nothing was written to the default repo-root path. + assert ("wiggleverse", "meta", "main", "rfcs/gradme.meta.yaml") not in fake.files + + +def test_default_collection_entry_still_renders(app_with_fake_gitea): + """Regression: the default-collection path (repo root `rfcs/.md` in + the default content repo) is unchanged by the G-15 resolution.""" + app, fake = app_with_fake_gitea + with TestClient(app) as client: + fake.files[("wiggleverse", "meta", "main", "rfcs/base.md")] = { + "content": _entry_md("base", "Baseline"), "sha": "sb"} + _mirror() + r = client.get("/api/rfcs/base/branches/main") + assert r.status_code == 200, r.text + assert r.json()["body"] == "the canonical body\n" + r2 = client.get("/api/projects/default/collections/default/rfcs/base/branches/main") + assert r2.status_code == 200, r2.text + assert r2.json()["body"] == "the canonical body\n" diff --git a/backend/tests/test_g15_entry_location.py b/backend/tests/test_g15_entry_location.py new file mode 100644 index 0000000..b7cba74 --- /dev/null +++ b/backend/tests/test_g15_entry_location.py @@ -0,0 +1,108 @@ +"""G-15 — the §22 three-tier write-path resolver. + +`projects.content_repo_for_collection` and `projects.entry_location` resolve an +entry's git location (org, content_repo, md_path) from its *collection* +(collection → project → content_repo, plus the collection subfolder) instead of +the deployment default. This is the keystone the branch/edit/body/graduation +write paths share so an entry outside the default project's default collection +reads/writes the correct file. +""" +from __future__ import annotations + +import tempfile +from pathlib import Path + +from app import collections as collections_mod, db, projects as projects_mod +from app.config import Config + + +def _db() -> Config: + cfg = Config( + gitea_url="x", gitea_bot_user="x", gitea_bot_token="x", gitea_org="wiggleverse", + registry_repo="registry", oauth_client_id="x", + oauth_client_secret="x", app_url="x", secret_key="x", + database_path=Path(tempfile.mkdtemp(prefix="g15loc-")) / "t.db", + owner_gitea_login="x", webhook_secret="x", + ) + db.run_migrations(cfg) + if db._CONN is not None: + db._CONN.close() + db._CONN = None + db.init(cfg) + return cfg + + +def _seed(): + # Default project (its content_repo is the deployment default) + a second + # project with a DISTINCT content_repo, each with a default + a named + # (subfolder) collection. + db.conn().execute( + "INSERT OR REPLACE INTO projects (id, name, content_repo, visibility, updated_at) " + "VALUES ('default','Default','meta','public', datetime('now'))") + db.conn().execute( + "INSERT OR REPLACE INTO projects (id, name, content_repo, visibility, updated_at) " + "VALUES ('rfc-app','RFC App','rfc-app-content','public', datetime('now'))") + rows = [ + ("default", "default", ""), + ("features", "default", "features"), + ("rfc-app", "rfc-app", ""), + ("specs", "rfc-app", "specs"), + ] + for cid, pid, sub in rows: + db.conn().execute( + "INSERT OR REPLACE INTO collections (id, project_id, type, subfolder, " + "initial_state, visibility, created_at, updated_at) VALUES " + "(?,?, 'document', ?, 'super-draft','public', datetime('now'), datetime('now'))", + (cid, pid, sub)) + + +def test_content_repo_for_collection_resolves_per_project(): + _db() + _seed() + # Default project's collections → the default content repo. + assert projects_mod.content_repo_for_collection("default") == "meta" + assert projects_mod.content_repo_for_collection("features") == "meta" + # The second project's collections → its own content repo. + assert projects_mod.content_repo_for_collection("rfc-app") == "rfc-app-content" + assert projects_mod.content_repo_for_collection("specs") == "rfc-app-content" + + +def test_content_repo_for_collection_unknown_is_none(): + _db() + _seed() + assert projects_mod.content_repo_for_collection("nope") is None + + +def test_entry_location_default_collection_repo_root(): + cfg = _db() + _seed() + org, repo, path = projects_mod.entry_location(cfg, "default", "alpha") + assert (org, repo, path) == ("wiggleverse", "meta", "rfcs/alpha.md") + + +def test_entry_location_named_collection_uses_subfolder(): + cfg = _db() + _seed() + org, repo, path = projects_mod.entry_location(cfg, "features", "beta") + assert (org, repo, path) == ("wiggleverse", "meta", "features/rfcs/beta.md") + + +def test_entry_location_other_project_distinct_repo(): + cfg = _db() + _seed() + # Named collection in a non-default project: distinct repo AND subfolder. + org, repo, path = projects_mod.entry_location(cfg, "specs", "gamma") + assert (org, repo, path) == ("wiggleverse", "rfc-app-content", "specs/rfcs/gamma.md") + # Default collection of the non-default project: distinct repo, repo root. + org, repo, path = projects_mod.entry_location(cfg, "rfc-app", "delta") + assert (org, repo, path) == ("wiggleverse", "rfc-app-content", "rfcs/delta.md") + + +def test_entry_location_unknown_collection_falls_back_to_default_repo(): + cfg = _db() + _seed() + # An entry whose collection_id is missing/unknown must still resolve to a + # usable location (the deployment default repo, repo root) rather than an + # empty repo — the legacy single-corpus behaviour. + org, repo, path = projects_mod.entry_location(cfg, "nope", "epsilon") + assert (org, repo, path) == ("wiggleverse", "meta", "rfcs/epsilon.md") diff --git a/frontend/package.json b/frontend/package.json index 7c77605..f0dacbf 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.52.3", + "version": "0.53.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/api.branchbody.test.js b/frontend/src/api.branchbody.test.js new file mode 100644 index 0000000..338a655 --- /dev/null +++ b/frontend/src/api.branchbody.test.js @@ -0,0 +1,40 @@ +// §22/G-15 — getRFCMain/getBranch build collection-scoped URLs when a project + +// collection are supplied (so an entry outside the default collection reads its +// own content repo / subfolder), and fall back to the slug-only routes otherwise. +import { describe, it, expect, vi, afterEach } from 'vitest' +import { getRFCMain, getBranch } from './api.js' + +function mockFetch() { + const fn = vi.fn(async () => ({ ok: true, status: 200, json: async () => ({}) })) + global.fetch = fn + return fn +} + +afterEach(() => { vi.restoreAllMocks() }) + +describe('G-15 collection-scoped branch/body URLs', () => { + it('getRFCMain scopes to project+collection when both are given', async () => { + const f = mockFetch() + await getRFCMain('login', 'rfc-app', 'specs') + expect(f).toHaveBeenCalledWith('/api/projects/rfc-app/collections/specs/rfcs/login/main') + }) + + it('getRFCMain falls back to the slug-only route without a collection', async () => { + const f = mockFetch() + await getRFCMain('login') + expect(f).toHaveBeenCalledWith('/api/rfcs/login/main') + }) + + it('getBranch scopes to project+collection and encodes the branch', async () => { + const f = mockFetch() + await getBranch('login', 'edit/foo', 'rfc-app', 'specs') + expect(f).toHaveBeenCalledWith( + '/api/projects/rfc-app/collections/specs/rfcs/login/branches/edit%2Ffoo') + }) + + it('getBranch falls back to the slug-only route without a collection', async () => { + const f = mockFetch() + await getBranch('login', 'main') + expect(f).toHaveBeenCalledWith('/api/rfcs/login/branches/main') + }) +}) diff --git a/frontend/src/api.js b/frontend/src/api.js index 911fc63..1f59e2b 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -448,11 +448,27 @@ export async function listModels(slug) { return jsonOrThrow(await fetch(`/api/rfcs/${slug}/models`)) } -export async function getRFCMain(slug) { +export async function getRFCMain(slug, projectId, collectionId) { + // §22/G-15: when the caller knows the entry's project + collection, read the + // collection-scoped route so a slug that exists in two collections resolves + // unambiguously and the entry's own content repo / subfolder is used. The + // bare-slug form stays for back-compat (default-collection callers). + if (projectId && collectionId) { + return jsonOrThrow(await fetch( + `/api/projects/${projectId}/collections/${collectionId}/rfcs/${slug}/main` + )) + } return jsonOrThrow(await fetch(`/api/rfcs/${slug}/main`)) } -export async function getBranch(slug, branch) { +export async function getBranch(slug, branch, projectId, collectionId) { + // §22/G-15: collection-scoped branch-body read (the canonical-body GET); see + // getRFCMain for the rationale. Falls back to the slug-only route. + if (projectId && collectionId) { + return jsonOrThrow(await fetch( + `/api/projects/${projectId}/collections/${collectionId}/rfcs/${slug}/branches/${encodeURIComponent(branch)}` + )) + } return jsonOrThrow(await fetch( `/api/rfcs/${slug}/branches/${encodeURIComponent(branch)}` )) diff --git a/frontend/src/components/RFCView.jsx b/frontend/src/components/RFCView.jsx index 3d12773..663ece7 100644 --- a/frontend/src/components/RFCView.jsx +++ b/frontend/src/components/RFCView.jsx @@ -221,8 +221,8 @@ export default function RFCView({ viewer }) { setSelection(null) setMode('discuss') - getRFCMain(slug).then(setMainView).catch(err => setError(err.message)) - getBranch(slug, branchParam) + getRFCMain(slug, pid, cid).then(setMainView).catch(err => setError(err.message)) + getBranch(slug, branchParam, pid, cid) .then(view => { setBranchView(view) setEditorContent(view.body || '') @@ -231,7 +231,8 @@ export default function RFCView({ viewer }) { setChanges(view.changes || []) }) .catch(err => setError(err.message)) - }, [slug, branchParam, entry]) + // §22/G-15: pid+cid scope the body reads; re-run if the collection changes. + }, [slug, branchParam, entry, pid, cid]) // Load chat messages whenever the branch's main thread id resolves. useEffect(() => { @@ -266,7 +267,7 @@ export default function RFCView({ viewer }) { paragraphCount: manualPending?.paragraphCount || 1, }) if (!res.noop) { - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) setChanges(fresh.changes || []) setPreviewContent(fresh.body || '') @@ -408,7 +409,7 @@ export default function RFCView({ viewer }) { )) } // Re-pull authoritative state: changes have been materialized server-side. - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setChanges(fresh.changes || []) // If we're in discuss mode and the new turn produced pending AI changes, // surface them as discuss-mode buffered count. @@ -447,7 +448,7 @@ export default function RFCView({ viewer }) { anchor_payload: { quote }, label, }) - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) loadAllMessages(slug, branchParam, fresh.threads).then(setMessages) } catch (err) { @@ -463,7 +464,7 @@ export default function RFCView({ viewer }) { // body. The proper tracked-changes overlay lives in Phase 3's // preview pane; with CM6 as the source editor there is no HTML // surface to inject `` into. - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) setChanges(fresh.changes || []) setEditorContent(fresh.body || '') @@ -477,7 +478,7 @@ export default function RFCView({ viewer }) { const handleDecline = useCallback(async (changeId) => { try { await apiDecline(slug, branchParam, changeId) - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setChanges(fresh.changes || []) } catch (err) { setError(err.message) @@ -487,7 +488,7 @@ export default function RFCView({ viewer }) { const handleReask = useCallback(async (changeId) => { try { await reaskChange(slug, branchParam, changeId) - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) setChanges(fresh.changes || []) loadAllMessages(slug, branchParam, fresh.threads).then(setMessages) @@ -499,7 +500,7 @@ export default function RFCView({ viewer }) { const handleResolveThread = useCallback(async (threadId) => { try { await resolveThread(slug, branchParam, threadId) - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) loadAllMessages(slug, branchParam, fresh.threads).then(setMessages) } catch (err) { @@ -977,7 +978,7 @@ export default function RFCView({ viewer }) { current={branchView.visibility} onClose={() => setShowVisibility(false)} onSaved={async () => { - const fresh = await getBranch(slug, branchParam) + const fresh = await getBranch(slug, branchParam, pid, cid) setBranchView(fresh) setShowVisibility(false) }} @@ -993,7 +994,7 @@ export default function RFCView({ viewer }) { setShowGraduateDialog(false) // The catalog row and the RFC view now reflect `active`. getRFC(pid, slug, cid).then(setEntry).catch(() => {}) - getRFCMain(slug).then(setMainView).catch(() => {}) + getRFCMain(slug, pid, cid).then(setMainView).catch(() => {}) }} /> )} @@ -1016,7 +1017,7 @@ export default function RFCView({ viewer }) { setShowMetadataPane(false) // Refresh main view so the new open PR surfaces in the // breadcrumb meta count immediately. - getRFCMain(slug).then(setMainView).catch(() => {}) + getRFCMain(slug, pid, cid).then(setMainView).catch(() => {}) navigate(entryPrPath(pid, slug, prNumber)) }} />