Compare commits
6 Commits
ff88be2e91
...
v0.55.0
| Author | SHA1 | Date | |
|---|---|---|---|
| c3efe893c3 | |||
| 9275348c45 | |||
| 242831a4a0 | |||
| f0533dc073 | |||
| f7b93d797c | |||
| 24596842ea |
@@ -23,6 +23,95 @@ skip versions are the composition of each intervening adjacent
|
|||||||
release's steps in order — no A-to-B path is pre-computed beyond
|
release's steps in order — no A-to-B path is pre-computed beyond
|
||||||
that.
|
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
|
||||||
|
created via OTC; no operator action required.**
|
||||||
|
|
||||||
|
`auth.provision_user` (the Gitea-OAuth callback path) matched an existing
|
||||||
|
`users` row only by `gitea_id`. A human who signed in first via the OTC
|
||||||
|
email path owns an email-only row (`gitea_id` NULL); when they later signed
|
||||||
|
in via Gitea OAuth with the same email, the lookup missed and the code
|
||||||
|
INSERTed a new row — colliding on the `idx_users_email` case-insensitive
|
||||||
|
unique index and raising `IntegrityError`, which 500-ed the callback. Now,
|
||||||
|
when no `gitea_id` row exists, `provision_user` reconciles by email and links
|
||||||
|
the OAuth identity (`gitea_id` / `gitea_login` / profile) onto the existing
|
||||||
|
row — the mirror of the OTC linker, which already links an OAuth-era email row
|
||||||
|
on first OTC sign-in. The §6.1 owner-zero bootstrap is applied on link
|
||||||
|
(matching the fresh-insert path); the row's `permission_state` is preserved so
|
||||||
|
linking never changes a human's admission status as a side effect.
|
||||||
|
|
||||||
|
## 0.54.0 — 2026-06-09
|
||||||
|
|
||||||
|
**Minor — the AI "Ask" affordance now works from an entry's canonical view
|
||||||
|
(§8.12); no operator action required.**
|
||||||
|
|
||||||
|
AI chat is an editing activity (a turn can emit proposed edits) and only runs
|
||||||
|
on an edit branch. On an entry's canonical `main` view the right column is the
|
||||||
|
human-discussion panel, not the chat panel — so invoking "Ask" (the selection
|
||||||
|
tooltip or the prompt bar) from the reading view pushed a chat turn that had
|
||||||
|
nowhere to render: it silently did nothing. Surfaced by dogfooding a non-default
|
||||||
|
project's spec entry on a live deployment.
|
||||||
|
|
||||||
|
- **Asking while reading "just works" (Option B).** When Ask is invoked from
|
||||||
|
the canonical view, the app transparently cuts an edit branch via the same
|
||||||
|
dispatch as Start Contributing (`start-edit-branch` for a super-draft,
|
||||||
|
`promote-to-branch` for an active RFC — idempotent, so it reuses an existing
|
||||||
|
edit branch rather than double-cutting), navigates onto that branch so the
|
||||||
|
chat panel mounts, and runs the question (text + the selected quote) there
|
||||||
|
once the branch's chat thread has resolved. The viewer lands in the chat with
|
||||||
|
the streaming answer, quote intact.
|
||||||
|
- **Graceful degradation.** A signed-out viewer keeps the read-only path
|
||||||
|
(sign-in prompt; Ask disabled) and a viewer who cannot contribute has the
|
||||||
|
branch cut rejected server-side and the error surfaced — no silent no-op and
|
||||||
|
no spurious branch. Asking from a branch already, and Flag on the canonical
|
||||||
|
view (which still opens a human discussion thread), are unchanged.
|
||||||
|
|
||||||
|
Frontend-only: the branch chat endpoints already worked on a branch. No
|
||||||
|
database migration and no configuration change.
|
||||||
|
|
||||||
## 0.53.0 — 2026-06-09
|
## 0.53.0 — 2026-06-09
|
||||||
|
|
||||||
**Minor — the branch/edit/body subsystem is now three-tier (project /
|
**Minor — the branch/edit/body subsystem is now three-tier (project /
|
||||||
|
|||||||
@@ -1022,6 +1022,20 @@ the user on it in contribute mode. New-branch naming defaults to an
|
|||||||
auto-generated value (user-renamable); the exact format is an
|
auto-generated value (user-renamable); the exact format is an
|
||||||
implementation detail.
|
implementation detail.
|
||||||
|
|
||||||
|
The AI chat is an editing activity — a turn can emit `<change>`
|
||||||
|
proposals — so it runs on an edit branch, not on read-only main, whose
|
||||||
|
right column is the human-discussion surface (§8.12 is branch-scoped).
|
||||||
|
Invoking the AI **Ask** affordance (the selection tooltip's prompt, or
|
||||||
|
the prompt bar) from main therefore transparently cuts an edit branch
|
||||||
|
via the same dispatch as "Start Contributing" (promote-to-branch for an
|
||||||
|
active RFC, start-edit-branch for a super-draft per §9.5; idempotent, so
|
||||||
|
an existing edit branch is reused rather than a second one cut), lands
|
||||||
|
the user on it, and runs the question — text plus any selected quote —
|
||||||
|
as the branch's first chat turn. A viewer who cannot contribute has the
|
||||||
|
branch cut rejected and the error surfaced (no branch is created); a
|
||||||
|
signed-out viewer keeps the §8.7 read-only path. **Flag** from main is
|
||||||
|
unaffected — it opens a human discussion thread on main, not a branch.
|
||||||
|
|
||||||
Discuss vs. contribute is an *intent* affordance, not a *permission*
|
Discuss vs. contribute is an *intent* affordance, not a *permission*
|
||||||
affordance. A user without contribute access to a branch sees the
|
affordance. A user without contribute access to a branch sees the
|
||||||
toggle disabled, with a sign-in or request-access path (see §8.7).
|
toggle disabled, with a sign-in or request-access path (see §8.7).
|
||||||
|
|||||||
+27
-3
@@ -145,6 +145,19 @@ def provision_user(config: Config, profile: dict[str, Any]) -> SessionUser:
|
|||||||
|
|
||||||
c = db.conn()
|
c = db.conn()
|
||||||
existing = c.execute("SELECT * FROM users WHERE gitea_id = ?", (gitea_id,)).fetchone()
|
existing = c.execute("SELECT * FROM users WHERE gitea_id = ?", (gitea_id,)).fetchone()
|
||||||
|
# No row for this gitea_id yet. A prior OTC sign-in (`provision_or_link_user`)
|
||||||
|
# may have created an email-only row with this same email and `gitea_id`
|
||||||
|
# NULL; `idx_users_email` is a unique index, so a blind INSERT below would
|
||||||
|
# raise IntegrityError and 500 the OAuth callback. Reconcile by email: link
|
||||||
|
# the OAuth identity onto that existing row instead. Mirror of the OTC
|
||||||
|
# linker, which links an OAuth-era email row on first OTC sign-in.
|
||||||
|
linked = False
|
||||||
|
if existing is None and email:
|
||||||
|
existing = c.execute(
|
||||||
|
"SELECT * FROM users WHERE email = ? COLLATE NOCASE AND gitea_id IS NULL LIMIT 1",
|
||||||
|
(email,),
|
||||||
|
).fetchone()
|
||||||
|
linked = existing is not None
|
||||||
if existing is None:
|
if existing is None:
|
||||||
role = "owner" if config.owner_gitea_login and login == config.owner_gitea_login else "contributor"
|
role = "owner" if config.owner_gitea_login and login == config.owner_gitea_login else "contributor"
|
||||||
# v0.8.0: a fresh OAuth-provisioned user is also subject to
|
# v0.8.0: a fresh OAuth-provisioned user is also subject to
|
||||||
@@ -166,15 +179,26 @@ def provision_user(config: Config, profile: dict[str, Any]) -> SessionUser:
|
|||||||
permission_state = "granted"
|
permission_state = "granted"
|
||||||
else:
|
else:
|
||||||
user_id = existing["id"]
|
user_id = existing["id"]
|
||||||
role = existing["role"]
|
# On a fresh link this OAuth sign-in is the row's first, so apply the
|
||||||
|
# §6.1 owner-zero bootstrap (matching the INSERT path). For a row already
|
||||||
|
# matched by gitea_id the role is settled — preserve it. `permission_state`
|
||||||
|
# is preserved either way: linking an OAuth identity onto an existing
|
||||||
|
# email row must not silently change the human's admission status.
|
||||||
|
if linked and config.owner_gitea_login and login == config.owner_gitea_login:
|
||||||
|
role = "owner"
|
||||||
|
else:
|
||||||
|
role = existing["role"]
|
||||||
permission_state = existing["permission_state"] or "granted"
|
permission_state = existing["permission_state"] or "granted"
|
||||||
|
# Setting `gitea_id` matters only on the link path (it was NULL); on the
|
||||||
|
# gitea_id-matched path it re-writes the same value. `role` is likewise a
|
||||||
|
# no-op there. Always refresh the mutable profile fields + last_seen.
|
||||||
c.execute(
|
c.execute(
|
||||||
"""
|
"""
|
||||||
UPDATE users
|
UPDATE users
|
||||||
SET gitea_login = ?, email = ?, display_name = ?, avatar_url = ?, last_seen_at = datetime('now')
|
SET gitea_id = ?, gitea_login = ?, email = ?, display_name = ?, avatar_url = ?, role = ?, last_seen_at = datetime('now')
|
||||||
WHERE id = ?
|
WHERE id = ?
|
||||||
""",
|
""",
|
||||||
(login, email, display, avatar, user_id),
|
(gitea_id, login, email, display, avatar, role, user_id),
|
||||||
)
|
)
|
||||||
|
|
||||||
return SessionUser(
|
return SessionUser(
|
||||||
|
|||||||
@@ -798,7 +798,7 @@ class Reconciler:
|
|||||||
log.info("reconciler: starting sweep")
|
log.info("reconciler: starting sweep")
|
||||||
try:
|
try:
|
||||||
try:
|
try:
|
||||||
await registry_mod.refresh_registry(self._config, self._gitea)
|
await registry_mod.refresh_registry(self._config, self._gitea, prune=True)
|
||||||
except Exception:
|
except Exception:
|
||||||
log.exception("reconciler: registry refresh failed; keeping last-good projects")
|
log.exception("reconciler: registry refresh failed; keeping last-good projects")
|
||||||
await refresh_meta_repo(self._config, self._gitea)
|
await refresh_meta_repo(self._config, self._gitea)
|
||||||
|
|||||||
+1
-1
@@ -133,7 +133,7 @@ async def lifespan(app: FastAPI):
|
|||||||
# (loud-fail per separation-of-concerns); the reconciler sweep keeps it
|
# (loud-fail per separation-of-concerns); the reconciler sweep keeps it
|
||||||
# fresh thereafter and tolerates a later bad PR.
|
# fresh thereafter and tolerates a later bad PR.
|
||||||
try:
|
try:
|
||||||
await registry_mod.refresh_registry(config, gitea)
|
await registry_mod.refresh_registry(config, gitea, prune=True)
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
raise RuntimeError(
|
raise RuntimeError(
|
||||||
f"registry mirror failed at startup ({config.registry_repo_full}/projects.yaml): {e}"
|
f"registry mirror failed at startup ({config.registry_repo_full}/projects.yaml): {e}"
|
||||||
|
|||||||
@@ -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:
|
def default_content_repo(config: Config) -> str | None:
|
||||||
"""The content repo the single-corpus mirror reads, from the default
|
"""The content repo the single-corpus mirror reads, from the default
|
||||||
project's row (filled by the registry mirror). Replaces the retired
|
project's row (filled by the registry mirror). Replaces the retired
|
||||||
|
|||||||
+23
-2
@@ -333,11 +333,18 @@ async def _mirror_named_collections(config: Config, gitea: Gitea, doc: RegistryD
|
|||||||
_upsert_named_collection(proj, subdir, ce, sha)
|
_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.
|
"""Mirror REGISTRY_REPO/projects.yaml into projects + deployment.
|
||||||
|
|
||||||
Idempotent. Raises RegistryError on a missing/invalid file and GiteaError
|
Idempotent. Raises RegistryError on a missing/invalid file and GiteaError
|
||||||
on transport failure; the caller chooses fatal-vs-tolerated.
|
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(
|
item = await gitea.get_contents(
|
||||||
config.gitea_org, config.registry_repo, "projects.yaml", ref="main"
|
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))
|
apply_registry(doc, sha, projects_mod.resolved_default_id(config))
|
||||||
# §22 S2: discover + upsert named collections from each content repo.
|
# §22 S2: discover + upsert named collections from each content repo.
|
||||||
await _mirror_named_collections(config, gitea, doc, sha)
|
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 "",
|
||||||
|
)
|
||||||
|
|||||||
@@ -0,0 +1,107 @@
|
|||||||
|
"""§6 hardening — the Gitea-OAuth `auth.provision_user` reconciles by email.
|
||||||
|
|
||||||
|
Regression cover for a §9-surfaced fragility: a human who signed in first via
|
||||||
|
OTC owns an email-only `users` row (`gitea_id` NULL). When they later sign in
|
||||||
|
via Gitea OAuth with the SAME email, `provision_user` used to match only by
|
||||||
|
`gitea_id`, miss the OTC row, and INSERT a new row — colliding on the
|
||||||
|
`idx_users_email` unique index and 500-ing the callback. The fix links the OAuth
|
||||||
|
identity onto the existing email row (the mirror of the OTC linker).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from test_propose_vertical import ( # noqa: F401
|
||||||
|
FakeGitea,
|
||||||
|
app_with_fake_gitea,
|
||||||
|
provision_user_row,
|
||||||
|
tmp_env,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _cfg():
|
||||||
|
from app.config import load_config
|
||||||
|
return load_config()
|
||||||
|
|
||||||
|
|
||||||
|
def test_oauth_links_onto_existing_otc_email_row(app_with_fake_gitea):
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from app import auth, db
|
||||||
|
|
||||||
|
app, _fake = app_with_fake_gitea
|
||||||
|
with TestClient(app):
|
||||||
|
# An OTC-first user: email-only row, gitea_id NULL, still 'pending'.
|
||||||
|
db.conn().execute(
|
||||||
|
"""
|
||||||
|
INSERT INTO users (gitea_id, gitea_login, email, display_name, avatar_url, role, permission_state)
|
||||||
|
VALUES (NULL, NULL, ?, ?, '', 'contributor', 'pending')
|
||||||
|
""",
|
||||||
|
("dual@example.com", "dual"),
|
||||||
|
)
|
||||||
|
otc_id = db.conn().execute(
|
||||||
|
"SELECT id FROM users WHERE email = ? COLLATE NOCASE", ("dual@example.com",)
|
||||||
|
).fetchone()["id"]
|
||||||
|
|
||||||
|
# Now the same human signs in via Gitea OAuth (new gitea_id, same email).
|
||||||
|
# Case-different email proves the NOCASE match.
|
||||||
|
user = auth.provision_user(
|
||||||
|
_cfg(),
|
||||||
|
{"id": 9001, "login": "dualgitea", "email": "Dual@Example.com",
|
||||||
|
"full_name": "Dual User", "avatar_url": "http://x/a.png"},
|
||||||
|
)
|
||||||
|
|
||||||
|
# Linked onto the SAME row — no second row, no 500.
|
||||||
|
assert user.user_id == otc_id
|
||||||
|
rows = db.conn().execute(
|
||||||
|
"SELECT id, gitea_id, gitea_login, permission_state, role FROM users WHERE email = ? COLLATE NOCASE",
|
||||||
|
("dual@example.com",),
|
||||||
|
).fetchall()
|
||||||
|
assert len(rows) == 1
|
||||||
|
assert rows[0]["id"] == otc_id
|
||||||
|
assert rows[0]["gitea_id"] == 9001 # OAuth identity attached
|
||||||
|
assert rows[0]["gitea_login"] == "dualgitea"
|
||||||
|
assert rows[0]["permission_state"] == "pending" # admission state preserved
|
||||||
|
assert rows[0]["role"] == "contributor"
|
||||||
|
|
||||||
|
|
||||||
|
def test_oauth_provisions_fresh_user_when_email_matches_no_one(app_with_fake_gitea):
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from app import auth, db
|
||||||
|
|
||||||
|
app, _fake = app_with_fake_gitea
|
||||||
|
with TestClient(app):
|
||||||
|
user = auth.provision_user(
|
||||||
|
_cfg(),
|
||||||
|
{"id": 9100, "login": "freshoauth", "email": "fresh@example.com",
|
||||||
|
"full_name": "Fresh", "avatar_url": ""},
|
||||||
|
)
|
||||||
|
row = db.conn().execute(
|
||||||
|
"SELECT gitea_id, gitea_login, role, permission_state FROM users WHERE id = ?",
|
||||||
|
(user.user_id,),
|
||||||
|
).fetchone()
|
||||||
|
assert row["gitea_id"] == 9100
|
||||||
|
assert row["gitea_login"] == "freshoauth"
|
||||||
|
# Reaching provision_user means admission passed → granted (unchanged).
|
||||||
|
assert row["permission_state"] == "granted"
|
||||||
|
|
||||||
|
|
||||||
|
def test_returning_oauth_user_matched_by_gitea_id_not_duplicated(app_with_fake_gitea):
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
from app import auth, db
|
||||||
|
|
||||||
|
app, _fake = app_with_fake_gitea
|
||||||
|
with TestClient(app):
|
||||||
|
provision_user_row(user_id=55, login="returning", role="contributor")
|
||||||
|
before = db.conn().execute("SELECT COUNT(*) AS n FROM users").fetchone()["n"]
|
||||||
|
|
||||||
|
user = auth.provision_user(
|
||||||
|
_cfg(),
|
||||||
|
{"id": 55, "login": "returning-renamed", "email": "returning@test",
|
||||||
|
"full_name": "Returning", "avatar_url": ""},
|
||||||
|
)
|
||||||
|
after = db.conn().execute("SELECT COUNT(*) AS n FROM users").fetchone()["n"]
|
||||||
|
|
||||||
|
assert user.user_id == 55
|
||||||
|
assert after == before # matched by gitea_id; no new row
|
||||||
|
row = db.conn().execute(
|
||||||
|
"SELECT gitea_login FROM users WHERE id = ?", (55,)
|
||||||
|
).fetchone()
|
||||||
|
assert row["gitea_login"] == "returning-renamed" # profile refreshed
|
||||||
@@ -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
|
||||||
@@ -0,0 +1,53 @@
|
|||||||
|
import { test, expect } from './lib/fixtures.js'
|
||||||
|
import { signIn, OWNER_EMAIL } from './lib/auth.js'
|
||||||
|
import { dismissCookies } from './lib/ui.js'
|
||||||
|
|
||||||
|
// §8.12 Option B — asking-while-reading on the canonical `main` view.
|
||||||
|
//
|
||||||
|
// Before this fix, the AI "Ask" affordance had no chat surface on main (the
|
||||||
|
// human-discussion panel renders there, not the chat panel), so asking from the
|
||||||
|
// reading view silently did nothing. Option B transparently cuts an edit branch,
|
||||||
|
// navigates to it, and runs the question there.
|
||||||
|
//
|
||||||
|
// This spec asserts the model-independent core: the branch cut + navigation
|
||||||
|
// (old code did NOTHING here — the silent no-op), and that the question turn
|
||||||
|
// actually FIRED against the branch. The turn's outcome is environment-specific
|
||||||
|
// — PPE/prod (a model is configured) streams an answer and the optimistic
|
||||||
|
// question persists; the Tier-1 stack has no AI provider, so the turn errors
|
||||||
|
// with "Chat failed". Either outcome proves the turn fired; we match both so the
|
||||||
|
// one spec passes in both environments. The quote-passthrough + answer rendering
|
||||||
|
// are covered deterministically by the RFCView unit tests (model mocked).
|
||||||
|
//
|
||||||
|
// Driven through the prompt bar, which shares handlePrompt → handleMainAsk with
|
||||||
|
// the selection tooltip's onAsk. Runs against the faceted `bdd` collection entry
|
||||||
|
// (collection-scoped — the same three-tier path G-15 hardened).
|
||||||
|
const ENTRY = '/p/ohm/c/bdd/e/checkout-returning'
|
||||||
|
|
||||||
|
test('Ask from the canonical view cuts an edit branch and lands the question in chat', async ({ page }) => {
|
||||||
|
await signIn(page, OWNER_EMAIL)
|
||||||
|
await page.goto(ENTRY)
|
||||||
|
await dismissCookies(page)
|
||||||
|
|
||||||
|
// We start on the canonical (main) view: read-only, no chat surface — the
|
||||||
|
// discuss-mode banner is the canonical-view tell, and the URL has no branch.
|
||||||
|
await expect(page.locator('.discuss-mode-banner')).toContainText('read-only')
|
||||||
|
expect(new URL(page.url()).searchParams.get('branch')).toBeNull()
|
||||||
|
|
||||||
|
// Ask a question from the prompt bar.
|
||||||
|
const question = `What problem does this entry solve? (${Date.now()})`
|
||||||
|
await page.locator('.prompt-input').fill(question)
|
||||||
|
await page.getByRole('button', { name: 'Ask', exact: true }).click()
|
||||||
|
|
||||||
|
// Option B: an edit branch is cut and we navigate onto it (no silent no-op).
|
||||||
|
await expect(async () => {
|
||||||
|
expect(new URL(page.url()).searchParams.get('branch')).toMatch(/^edit/)
|
||||||
|
}).toPass({ timeout: 30_000 })
|
||||||
|
|
||||||
|
// The question turn fired against the new branch: either the optimistic
|
||||||
|
// question is showing (model answered / answering) or the turn surfaced a
|
||||||
|
// chat error (no AI provider in Tier-1). Both prove it ran — the pre-fix
|
||||||
|
// silent no-op showed neither.
|
||||||
|
await expect(
|
||||||
|
page.getByText(question).or(page.getByText(/Chat failed|No AI providers/i)),
|
||||||
|
).toBeVisible({ timeout: 30_000 })
|
||||||
|
})
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "rfc-app-frontend",
|
"name": "rfc-app-frontend",
|
||||||
"private": true,
|
"private": true,
|
||||||
"version": "0.53.0",
|
"version": "0.55.0",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"dev": "vite",
|
"dev": "vite",
|
||||||
|
|||||||
@@ -93,6 +93,15 @@ export default function RFCView({ viewer }) {
|
|||||||
// surface to expose.
|
// surface to expose.
|
||||||
const editorRef = useRef(null)
|
const editorRef = useRef(null)
|
||||||
const originalSourceLinesRef = useRef([])
|
const originalSourceLinesRef = useRef([])
|
||||||
|
// §8.12 Option B — asking-while-reading on `main`. When Ask is invoked from
|
||||||
|
// the canonical view we transparently cut an edit branch, navigate to it, and
|
||||||
|
// run the question there once its view (and main_thread_id) loads. The pending
|
||||||
|
// question is stashed here and fired by the pending-ask effect below — we
|
||||||
|
// can't push optimistic chat messages before branchView.main_thread_id exists.
|
||||||
|
const pendingAskRef = useRef(null) // { text, quote, branch } | null
|
||||||
|
// Live ref to submitChatTurn so the (intentionally narrow-dep) message-load
|
||||||
|
// effect can fire the stashed §8.12 main-view ask with the latest closure.
|
||||||
|
const submitChatTurnRef = useRef(null)
|
||||||
const [editorContent, setEditorContent] = useState('')
|
const [editorContent, setEditorContent] = useState('')
|
||||||
// Mirror of the live CM6 doc for the Contribute-mode preview pane,
|
// Mirror of the live CM6 doc for the Contribute-mode preview pane,
|
||||||
// debounced so the preview doesn't re-render on every keystroke.
|
// debounced so the preview doesn't re-render on every keystroke.
|
||||||
@@ -237,8 +246,21 @@ export default function RFCView({ viewer }) {
|
|||||||
// Load chat messages whenever the branch's main thread id resolves.
|
// Load chat messages whenever the branch's main thread id resolves.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!branchView?.main_thread_id) return
|
if (!branchView?.main_thread_id) return
|
||||||
loadAllMessages(slug, branchParam, branchView.threads).then(setMessages)
|
loadAllMessages(slug, branchParam, branchView.threads).then(loaded => {
|
||||||
}, [branchView?.main_thread_id, slug, branchParam])
|
setMessages(loaded)
|
||||||
|
// §8.12 Option B — run the stashed main-view question now that the freshly
|
||||||
|
// cut branch's messages have loaded. Firing here (rather than in a parallel
|
||||||
|
// effect) appends the turn AFTER the loaded set, so this late message load
|
||||||
|
// can't clobber the optimistic chat turn. Land in contribute mode to
|
||||||
|
// surface any edits the turn proposes (AI chat is an editing activity).
|
||||||
|
const pending = pendingAskRef.current
|
||||||
|
if (pending && pending.branch === branchView.branch_name && pending.branch === branchParam) {
|
||||||
|
pendingAskRef.current = null
|
||||||
|
setMode('contribute')
|
||||||
|
submitChatTurnRef.current?.(pending.text, pending.quote)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}, [branchView?.main_thread_id, branchView?.branch_name, slug, branchParam])
|
||||||
|
|
||||||
// Selection wiring (§8.12). MarkdownPreview reports {text, coords}
|
// Selection wiring (§8.12). MarkdownPreview reports {text, coords}
|
||||||
// sourced from window.getSelection(); the SelectionTooltip
|
// sourced from window.getSelection(); the SelectionTooltip
|
||||||
@@ -335,8 +357,12 @@ export default function RFCView({ viewer }) {
|
|||||||
}, [manualCountdown, flushManualBuffer])
|
}, [manualCountdown, flushManualBuffer])
|
||||||
|
|
||||||
// ── Start contributing ─────────────────────────────────────────────────
|
// ── Start contributing ─────────────────────────────────────────────────
|
||||||
|
// Returns the branch the viewer should now be on (the freshly cut branch on
|
||||||
|
// main, or the current branch on a mode-flip), or null if no branch was cut
|
||||||
|
// (signed-out → login redirect, or the server rejected the cut). §8.12's
|
||||||
|
// main-view Ask reuses this dispatch and consumes the returned branch name.
|
||||||
const handleStartContributing = useCallback(async () => {
|
const handleStartContributing = useCallback(async () => {
|
||||||
if (!viewer) { window.location.href = '/auth/login'; return }
|
if (!viewer) { window.location.href = '/auth/login'; return null }
|
||||||
if (branchParam === 'main') {
|
if (branchParam === 'main') {
|
||||||
try {
|
try {
|
||||||
// §9.5 dispatch: super-drafts cut a meta-repo edit branch via
|
// §9.5 dispatch: super-drafts cut a meta-repo edit branch via
|
||||||
@@ -345,18 +371,34 @@ export default function RFCView({ viewer }) {
|
|||||||
? await startEditBranch(slug)
|
? await startEditBranch(slug)
|
||||||
: await promoteToBranch(slug)
|
: await promoteToBranch(slug)
|
||||||
setSearchParams({ branch: branch_name })
|
setSearchParams({ branch: branch_name })
|
||||||
|
return branch_name
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
setError(err.message)
|
setError(err.message)
|
||||||
|
return null
|
||||||
}
|
}
|
||||||
return
|
|
||||||
}
|
}
|
||||||
// Non-main: pure mode flip per §8.14.
|
// Non-main: pure mode flip per §8.14.
|
||||||
if (pendingDiscussChanges.length > 0) {
|
if (pendingDiscussChanges.length > 0) {
|
||||||
setPendingDiscussChanges([])
|
setPendingDiscussChanges([])
|
||||||
}
|
}
|
||||||
setMode('contribute')
|
setMode('contribute')
|
||||||
|
return branchParam
|
||||||
}, [viewer, slug, branchParam, pendingDiscussChanges, setSearchParams, isSuperDraft])
|
}, [viewer, slug, branchParam, pendingDiscussChanges, setSearchParams, isSuperDraft])
|
||||||
|
|
||||||
|
// ── §8.12 Option B — Ask invoked from the canonical `main` view ─────────
|
||||||
|
// AI chat is an editing activity and only runs on an edit branch, so asking
|
||||||
|
// while reading transparently cuts one (same dispatch as Start Contributing),
|
||||||
|
// navigates to it, and stashes the question; the pending-ask effect fires it
|
||||||
|
// once the new branch's view (and its main_thread_id) has resolved. Degrades
|
||||||
|
// gracefully: signed-out → login; a viewer who can't contribute has the cut
|
||||||
|
// rejected server-side → the error surfaces and we neither navigate nor chat.
|
||||||
|
const handleMainAsk = useCallback(async (text, quote) => {
|
||||||
|
if (!viewer) { window.location.href = '/auth/login'; return }
|
||||||
|
const branch = await handleStartContributing()
|
||||||
|
if (!branch || branch === 'main') return // cut failed → setError already ran
|
||||||
|
pendingAskRef.current = { text, quote: quote || null, branch }
|
||||||
|
}, [viewer, handleStartContributing])
|
||||||
|
|
||||||
// ── Submit a chat turn (prompt bar or selection tooltip) ───────────────
|
// ── Submit a chat turn (prompt bar or selection tooltip) ───────────────
|
||||||
const submitChatTurn = useCallback(async (text, quote) => {
|
const submitChatTurn = useCallback(async (text, quote) => {
|
||||||
if (!branchView?.main_thread_id || isStreaming) return
|
if (!branchView?.main_thread_id || isStreaming) return
|
||||||
@@ -427,16 +469,24 @@ export default function RFCView({ viewer }) {
|
|||||||
}
|
}
|
||||||
}, [slug, branchParam, branchView?.main_thread_id, isStreaming, viewer, selectedModel, mode])
|
}, [slug, branchParam, branchView?.main_thread_id, isStreaming, viewer, selectedModel, mode])
|
||||||
|
|
||||||
|
// Keep submitChatTurnRef pointed at the latest submitChatTurn so the
|
||||||
|
// message-load effect's §8.12 main-view ask uses the current branch closure.
|
||||||
|
useEffect(() => { submitChatTurnRef.current = submitChatTurn }, [submitChatTurn])
|
||||||
|
|
||||||
const handlePrompt = useCallback((text, sel) => {
|
const handlePrompt = useCallback((text, sel) => {
|
||||||
const quote = sel?.text || null
|
const quote = sel?.text || null
|
||||||
|
// On the canonical view there is no chat surface; Ask cuts an edit branch
|
||||||
|
// and runs the question there (§8.12 Option B). On a branch, ask directly.
|
||||||
|
if (branchParam === 'main') { handleMainAsk(text, quote); return }
|
||||||
submitChatTurn(text, quote)
|
submitChatTurn(text, quote)
|
||||||
}, [submitChatTurn])
|
}, [branchParam, handleMainAsk, submitChatTurn])
|
||||||
|
|
||||||
const handleTooltipAsk = useCallback(async (textOrNull, quote) => {
|
const handleTooltipAsk = useCallback(async (textOrNull, quote) => {
|
||||||
if (textOrNull === null) { setSelection(null); return }
|
if (textOrNull === null) { setSelection(null); return }
|
||||||
setSelection(null)
|
setSelection(null)
|
||||||
|
if (branchParam === 'main') { await handleMainAsk(textOrNull, quote); return }
|
||||||
await submitChatTurn(textOrNull, quote)
|
await submitChatTurn(textOrNull, quote)
|
||||||
}, [submitChatTurn])
|
}, [branchParam, handleMainAsk, submitChatTurn])
|
||||||
|
|
||||||
const handleTooltipFlag = useCallback(async (label, quote) => {
|
const handleTooltipFlag = useCallback(async (label, quote) => {
|
||||||
if (!viewer) { window.location.href = '/auth/login'; return }
|
if (!viewer) { window.location.href = '/auth/login'; return }
|
||||||
|
|||||||
@@ -0,0 +1,220 @@
|
|||||||
|
import React from 'react'
|
||||||
|
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||||
|
import { render, screen, fireEvent, waitFor } from '@testing-library/react'
|
||||||
|
import { MemoryRouter, Routes, Route } from 'react-router-dom'
|
||||||
|
|
||||||
|
// §8.12 Option B — asking-while-reading on the canonical `main` view. The AI
|
||||||
|
// "Ask" affordance (selection tooltip + prompt bar) has no chat surface on
|
||||||
|
// main (RFCDiscussionPanel renders there, not ChatPanel). These tests cover the
|
||||||
|
// transparent branch-cut: Ask on main cuts an edit branch, navigates to it, and
|
||||||
|
// runs the question there; Ask on a branch is unchanged; a rejected cut degrades
|
||||||
|
// gracefully; Flag on main still opens a discussion thread.
|
||||||
|
|
||||||
|
const getRFC = vi.fn()
|
||||||
|
const getRFCMain = vi.fn()
|
||||||
|
const getBranch = vi.fn()
|
||||||
|
const listModels = vi.fn()
|
||||||
|
const getCollection = vi.fn()
|
||||||
|
const getThreadMessages = vi.fn()
|
||||||
|
const promoteToBranch = vi.fn()
|
||||||
|
const startEditBranch = vi.fn()
|
||||||
|
const streamChatTurn = vi.fn()
|
||||||
|
const createThread = vi.fn()
|
||||||
|
|
||||||
|
vi.mock('../api', () => ({
|
||||||
|
getRFC: (...a) => getRFC(...a),
|
||||||
|
getRFCMain: (...a) => getRFCMain(...a),
|
||||||
|
getBranch: (...a) => getBranch(...a),
|
||||||
|
listModels: (...a) => listModels(...a),
|
||||||
|
getCollection: (...a) => getCollection(...a),
|
||||||
|
getThreadMessages: (...a) => getThreadMessages(...a),
|
||||||
|
promoteToBranch: (...a) => promoteToBranch(...a),
|
||||||
|
startEditBranch: (...a) => startEditBranch(...a),
|
||||||
|
streamChatTurn: (...a) => streamChatTurn(...a),
|
||||||
|
createThread: (...a) => createThread(...a),
|
||||||
|
acceptChange: vi.fn(), declineChange: vi.fn(), editMetadata: vi.fn(),
|
||||||
|
manualFlush: vi.fn(), reaskChange: vi.fn(), resolveThread: vi.fn(),
|
||||||
|
setBranchVisibility: vi.fn(), claimOwnership: vi.fn(),
|
||||||
|
retireRFC: vi.fn(), unretireRFC: vi.fn(),
|
||||||
|
}))
|
||||||
|
|
||||||
|
vi.mock('../lib/entryPaths', () => ({
|
||||||
|
entryPath: () => '/p/ohm/c/rfc-app/e/the-entry',
|
||||||
|
entryPrPath: () => '/pr',
|
||||||
|
useProjectId: () => 'ohm',
|
||||||
|
useCollectionId: () => 'rfc-app',
|
||||||
|
}))
|
||||||
|
vi.mock('../lib/analytics', () => ({ EVENTS: { RFC_VIEWED: 'rfc_viewed' }, track: vi.fn() }))
|
||||||
|
|
||||||
|
// Stub the heavy children; expose just the callbacks/data the flow needs.
|
||||||
|
vi.mock('./MarkdownSourceEditor.jsx', () => ({ default: () => null }))
|
||||||
|
vi.mock('./MarkdownPreview.jsx', () => ({ default: () => <div data-testid="preview" /> }))
|
||||||
|
vi.mock('./RFCDiscussionPanel.jsx', () => ({ default: () => <div data-testid="discussion-panel" /> }))
|
||||||
|
vi.mock('./ChatPanel.jsx', () => ({
|
||||||
|
default: ({ messages }) => (
|
||||||
|
<div data-testid="chat-panel">
|
||||||
|
{(messages || []).map((m, i) => (
|
||||||
|
<div key={i} data-role={m.role}>{m.text}</div>
|
||||||
|
))}
|
||||||
|
</div>
|
||||||
|
),
|
||||||
|
}))
|
||||||
|
vi.mock('./SelectionTooltip.jsx', () => ({
|
||||||
|
default: ({ onAsk, onFlag, disabled }) => (
|
||||||
|
<div data-testid="tooltip">
|
||||||
|
<button data-testid="tooltip-ask" disabled={disabled}
|
||||||
|
onClick={() => onAsk('Why this approach?', 'the selected quote')}>ask</button>
|
||||||
|
<button data-testid="tooltip-flag"
|
||||||
|
onClick={() => onFlag('confusing', 'the selected quote')}>flag</button>
|
||||||
|
</div>
|
||||||
|
),
|
||||||
|
}))
|
||||||
|
vi.mock('./PromptBar.jsx', () => ({
|
||||||
|
default: ({ onSubmit, disabled }) => (
|
||||||
|
<button data-testid="prompt-submit" disabled={disabled}
|
||||||
|
onClick={() => onSubmit('Why this approach?', { text: 'the selected quote' })}>send</button>
|
||||||
|
),
|
||||||
|
}))
|
||||||
|
vi.mock('./ChangePanel.jsx', () => ({ default: () => null, diffWords: () => [] }))
|
||||||
|
vi.mock('./PRModal.jsx', () => ({ default: () => null }))
|
||||||
|
vi.mock('./GraduateDialog.jsx', () => ({ default: () => null }))
|
||||||
|
vi.mock('./InvitationsModal.jsx', () => ({ default: () => null }))
|
||||||
|
vi.mock('./MetadataFieldsPanel.jsx', () => ({ default: () => null }))
|
||||||
|
|
||||||
|
import RFCView from './RFCView.jsx'
|
||||||
|
|
||||||
|
const ACTIVE_ENTRY = {
|
||||||
|
id: 'RFC-0001', title: 'The Entry', state: 'active',
|
||||||
|
owners: [], meta: {}, proposed_use_case: '', tags: [],
|
||||||
|
}
|
||||||
|
const MAIN_VIEW = { slug: 'the-entry', branches: [], open_prs: [] }
|
||||||
|
|
||||||
|
function branchPayload(branch, { main_thread_id = 't-1' } = {}) {
|
||||||
|
return {
|
||||||
|
slug: 'the-entry', title: 'The Entry', branch_name: branch, body: '# Body\n',
|
||||||
|
body_sha: 'abc', main_thread_id, threads: [], changes: [],
|
||||||
|
visibility: {}, grants: [], creator: 'me',
|
||||||
|
capabilities: { can_contribute: true, can_change_branch_settings: true },
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
const VIEWER = { gitea_login: 'me', role: 'contributor' }
|
||||||
|
|
||||||
|
function renderView(viewer = VIEWER, initialBranch = null) {
|
||||||
|
const path = initialBranch ? `/e/the-entry?branch=${initialBranch}` : '/e/the-entry'
|
||||||
|
return render(
|
||||||
|
<MemoryRouter initialEntries={[path]}>
|
||||||
|
<Routes>
|
||||||
|
<Route path="/e/:slug" element={<RFCView viewer={viewer} />} />
|
||||||
|
</Routes>
|
||||||
|
</MemoryRouter>,
|
||||||
|
)
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('RFCView — §8.12 main-view Ask (Option B)', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks()
|
||||||
|
getRFC.mockResolvedValue(ACTIVE_ENTRY)
|
||||||
|
getRFCMain.mockResolvedValue(MAIN_VIEW)
|
||||||
|
listModels.mockResolvedValue({ models: [{ id: 'claude' }], default: 'claude' })
|
||||||
|
getCollection.mockResolvedValue({ fields: null })
|
||||||
|
getThreadMessages.mockResolvedValue({ messages: [] })
|
||||||
|
getBranch.mockImplementation((_slug, branch) => Promise.resolve(branchPayload(branch)))
|
||||||
|
promoteToBranch.mockResolvedValue({ branch_name: 'edit-me-1' })
|
||||||
|
startEditBranch.mockResolvedValue({ branch_name: 'edit-the-entry' })
|
||||||
|
createThread.mockResolvedValue({ thread_id: 1, message_id: 1 })
|
||||||
|
streamChatTurn.mockImplementation(async (_slug, _branch, _threadId, { text, quote }, cb) => {
|
||||||
|
cb.onChunk?.('Here is the answer.')
|
||||||
|
cb.onChanges?.({ message_id: 42 })
|
||||||
|
cb.onDone?.()
|
||||||
|
return { assistantId: 42, userMsgId: 41 }
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
it('cuts an edit branch and runs the question (with the quote) there', async () => {
|
||||||
|
renderView()
|
||||||
|
await screen.findByTestId('discussion-panel') // we start on main
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId('prompt-submit'))
|
||||||
|
|
||||||
|
// The active-RFC dispatch is promote-to-branch.
|
||||||
|
await waitFor(() => expect(promoteToBranch).toHaveBeenCalledWith('the-entry'))
|
||||||
|
|
||||||
|
// Once the new branch view loads, the turn runs there — not on main —
|
||||||
|
// with the selected quote intact.
|
||||||
|
await waitFor(() => expect(streamChatTurn).toHaveBeenCalled())
|
||||||
|
const [slug, branch, threadId, payload] = streamChatTurn.mock.calls[0]
|
||||||
|
expect(slug).toBe('the-entry')
|
||||||
|
expect(branch).toBe('edit-me-1')
|
||||||
|
expect(threadId).toBe('t-1')
|
||||||
|
expect(payload).toMatchObject({ text: 'Why this approach?', quote: 'the selected quote' })
|
||||||
|
|
||||||
|
// The chat panel is now mounted (we left main) and shows the streamed answer.
|
||||||
|
const panel = await screen.findByTestId('chat-panel')
|
||||||
|
await waitFor(() => expect(panel).toHaveTextContent('Here is the answer.'))
|
||||||
|
})
|
||||||
|
|
||||||
|
it('uses start-edit-branch for a super-draft entry', async () => {
|
||||||
|
getRFC.mockResolvedValue({ ...ACTIVE_ENTRY, state: 'super-draft' })
|
||||||
|
getBranch.mockImplementation((_slug, branch) =>
|
||||||
|
Promise.resolve(branchPayload(branch === 'main' ? 'main' : 'edit-the-entry')))
|
||||||
|
renderView()
|
||||||
|
await screen.findByTestId('discussion-panel')
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId('tooltip-ask'))
|
||||||
|
|
||||||
|
await waitFor(() => expect(startEditBranch).toHaveBeenCalledWith('the-entry'))
|
||||||
|
expect(promoteToBranch).not.toHaveBeenCalled()
|
||||||
|
await waitFor(() => expect(streamChatTurn).toHaveBeenCalled())
|
||||||
|
expect(streamChatTurn.mock.calls[0][1]).toBe('edit-the-entry')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('asking from an existing branch runs directly, without cutting a new one', async () => {
|
||||||
|
renderView(VIEWER, 'edit-me-1')
|
||||||
|
await screen.findByTestId('chat-panel')
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId('prompt-submit'))
|
||||||
|
|
||||||
|
await waitFor(() => expect(streamChatTurn).toHaveBeenCalled())
|
||||||
|
expect(promoteToBranch).not.toHaveBeenCalled()
|
||||||
|
expect(startEditBranch).not.toHaveBeenCalled()
|
||||||
|
expect(streamChatTurn.mock.calls[0][1]).toBe('edit-me-1')
|
||||||
|
})
|
||||||
|
|
||||||
|
it('surfaces an error and does not chat when the branch cut is rejected', async () => {
|
||||||
|
promoteToBranch.mockRejectedValue(new Error('not a contributor'))
|
||||||
|
renderView()
|
||||||
|
await screen.findByTestId('discussion-panel')
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId('prompt-submit'))
|
||||||
|
|
||||||
|
await waitFor(() => expect(promoteToBranch).toHaveBeenCalled())
|
||||||
|
// Explicit error path — not a silent no-op — and no spurious chat turn.
|
||||||
|
expect(await screen.findByText(/not a contributor/)).toBeInTheDocument()
|
||||||
|
expect(streamChatTurn).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('Flag on the canonical view still creates a discussion thread (no branch cut)', async () => {
|
||||||
|
renderView()
|
||||||
|
await screen.findByTestId('discussion-panel')
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId('tooltip-flag'))
|
||||||
|
|
||||||
|
await waitFor(() => expect(createThread).toHaveBeenCalled())
|
||||||
|
const [, branch, body] = createThread.mock.calls[0]
|
||||||
|
expect(branch).toBe('main')
|
||||||
|
expect(body).toMatchObject({ thread_kind: 'flag' })
|
||||||
|
expect(promoteToBranch).not.toHaveBeenCalled()
|
||||||
|
expect(streamChatTurn).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
|
||||||
|
it('signed-out viewer gets a read-only path on main — no prompt bar, Ask disabled, no cut', async () => {
|
||||||
|
renderView(null)
|
||||||
|
await screen.findByTestId('discussion-panel')
|
||||||
|
|
||||||
|
expect(screen.queryByTestId('prompt-submit')).toBeNull()
|
||||||
|
expect(screen.getByTestId('tooltip-ask')).toBeDisabled()
|
||||||
|
expect(promoteToBranch).not.toHaveBeenCalled()
|
||||||
|
expect(startEditBranch).not.toHaveBeenCalled()
|
||||||
|
})
|
||||||
|
})
|
||||||
Reference in New Issue
Block a user