diff --git a/CHANGELOG.md b/CHANGELOG.md index eff4eef..8180a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,103 @@ 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.5.0 — 2026-05-27 + +**Minor — no operator action required.** This release wires the +PR-less per-RFC discussion surface (roadmap item #3, SPEC §10.10). +An RFC's main view now carries a discussion panel distinct from PR +comments and branch chat; contribution — proposing edits the document +will land — still requires opening a PR via the §10.1 affordance. +The substrate is the existing `threads` / `thread_messages` tables; +rows with `threads.branch_name IS NULL` scope to the RFC's main view. +No schema migration is required. + +### Added + +- **PR-less discussion endpoints** (`backend/app/api_discussion.py`) + mounted at `/api/rfcs//discussion/...`: + - `GET /api/rfcs//discussion/threads` — list threads where + `threads.branch_name IS NULL`. Anonymous-readable per the v0.3.0 + contract; the default whole-doc chat thread is materialized + lazily on first read. + - `POST /api/rfcs//discussion/threads` — open a new + discussion thread (`thread_kind='chat'`, `anchor_kind='whole-doc'`, + `branch_name=NULL`). Body: optional `label`, optional first + `message`. Requires contributor role. + - `GET /api/rfcs//discussion/threads//messages` + — read messages on a discussion thread. Anonymous-readable. + - `POST /api/rfcs//discussion/threads//messages` + — post a message. Body: `text`, optional `quote`. Requires + contributor role. + - `POST /api/rfcs//discussion/threads//resolve` + — resolve a discussion thread. Permission per §10.10: thread + creator, RFC owner / arbiter, or app admin / owner. +- **`RFCDiscussionPanel.jsx`** — the right-column surface on the RFC + view when the viewer is on `main`. Composer requires sign-in; + Cmd/Ctrl+Enter sends. Multiple threads surface as pill-shaped + tabs above the message feed. A "New thread" affordance opens a + fresh thread on the same RFC. +- **SPEC `§10.10` PR-less discussion vs. contribution** — settles + the distinction between discussion (RFC-scoped, no PR, both + anonymous-readable and contributor-writeable) and contribution + (still requires a PR via §10.1). Also extends `§5`'s `threads` + table commentary so the null-branch interpretation is documented + as actively used rather than reserved scaffold. +- **SPEC `§17`** — lists the five new `discussion/...` endpoints + in the illustrative table. + +### Changed + +- **`backend/app/chat.py`** — `_fan_out_chat` now passes + `branch_name` straight through to the notify chokepoint instead of + coercing `None` to `"main"`. The notifications row carries the + null through, which preserves the §15.7 reconciler's keying on + `(rfc_slug, branch_name)` for the eventual discussion-side + chat-seen advance (deferred to a §19.2 candidate). Existing + branch-scoped chat continues to pass non-null branch names; the + change is invisible to that path. +- **`backend/app/notify.py`** — `fan_out_chat_message`'s + `branch_name` parameter is now typed `str | None` to match the + v0.5.0 PR-less shape. Routing rules are unchanged; the inbox prose + renders identically whether the chat lives on a branch or on the + RFC's discussion surface, which is the right honest signal. +- **`RFCView.jsx`** — the right-column panel is now conditional: + when `branchParam === 'main'`, render `RFCDiscussionPanel` + (the new PR-less surface); otherwise render the existing + `ChatPanel` (branch chat unchanged). + +### §19.2 candidates surfaced + +- PR-less discussion: range / paragraph anchors (the data model + permits them; the UI is the deferred part). +- PR-less discussion: distinct notification `event_kind`s + (`open_rfc_discussion_thread`, etc.) if usage shows contributors + want to filter discussion-vs-branch in the §15.2 inbox. +- PR-less discussion: chat-seen cursor closing the §15.7 + reconciliation loop for the new surface. +- PR-less discussion: AI participant invocation (discussion-only, + no `` block side-effects). +- PR-less discussion: anonymous-read polish to match the v0.6.0 + write-gate hardening (item #4). + +### Upgrade steps (from 0.4.0) + +1. The deployment **MUST** rebuild the frontend (`npm install && + npm run build`) so `RFCDiscussionPanel.jsx` ships in the bundle. + No new env vars; existing `frontend/.env` is sufficient. +2. The deployment **MUST** restart the backend so the new + `api_discussion` router mounts. No schema migration runs — the + `threads` table already supports `branch_name IS NULL` per §5, + and the v0.5.0 build is the first to write rows in that shape. +3. The deployment **MAY** announce the new surface to its + contributors: the RFC view's main page now carries a discussion + panel below the document. Existing branch chat and PR review + surfaces are unchanged. +4. The deployment **SHOULD NOT** expect a hardening of the + anonymous-write gate in v0.5.0 — that lands in v0.6.0 (item #4). + v0.5.0's write paths already refuse anonymous posts, so no + pre-emptive operator action is needed. + ## 0.4.0 — 2026-05-27 **Minor — no operator action required beyond rebuild + restart.** The diff --git a/SPEC.md b/SPEC.md index ba3f9e6..1a3c1c4 100644 --- a/SPEC.md +++ b/SPEC.md @@ -253,13 +253,18 @@ and exact columns are illustrative; the implementing session can adjust. - `threads` — every conversation in the system, whether scoped to an RFC's main view, a branch, or a span within a branch's document. Columns: `id`, `rfc_slug`, `branch_name` (nullable — null means scoped to the - RFC's main view), `anchor_kind` (`whole-doc` | `range` | `paragraph`), + RFC's main view, the PR-less per-RFC discussion surface per §10's + closing note; non-null means scoped to a branch's work, including + PR-comment threads), `anchor_kind` (`whole-doc` | `range` | `paragraph`), `anchor_payload` (JSON: serialized ProseMirror range or paragraph id), `thread_kind` (`chat` | `flag` | `review` — `review` is the diff-anchored PR-review thread defined in §10.4), `label` (short human-authored summary; for flags this is the entire content), `state` (`open` | `resolved` | `stale`), `created_by`, `created_at`, `resolved_at`, `resolved_by`. - Visibility is derived from the underlying branch (§11.1). + Visibility is derived from the underlying branch (§11.1); for the + null-branch PR-less discussion surface, visibility follows the RFC + (anonymous read open per the §14 / v0.3.0 contract, write requires + contributor per §6.1, tightened toward anon-write-refused in v0.6.0). - `thread_messages` — the actual chat content for `chat`-kind threads. Columns: `id`, `thread_id`, `role` (`user` | `assistant` | `system`), `author_user_id` (nullable; null for assistant), `model_id` (nullable; @@ -1639,6 +1644,40 @@ framework's evidence unit; admitting plumbing commits — "fix merge conflict with main" — into that timeline would dilute the signal each commit is meant to carry. +### 10.10 PR-less discussion vs. contribution + +PR comments and branch chat (§10.4, §8.4) are PR-scoped: they live on +the `threads` rows whose `branch_name` names a branch (the PR's head +or, pre-PR, a feature branch). They are the right surface for *this +specific proposed change*. They are not the right surface for "what +about this part of the RFC overall?" or "have we considered…?" — a +question that doesn't yet warrant cutting a branch and that would +distort a PR's review timeline if it landed there. + +The RFC view carries a **discussion surface** distinct from PR +comments. Its substrate is `threads` rows whose `branch_name IS NULL` +(§5) — the same conversation table used by branch chat, with the +nullable column doing the segregating. Posting a discussion thread or +message does not open a PR; the §1 chokepoint is unaffected because +chat messages never produced Git writes. Contribution — proposing +edits the document will land — still requires opening a PR via the +§10.1 affordance. + +The distinction in one line: **discussion is what the RFC is for; +contribution is how the RFC changes.** Either is honest; conflating +them was the failure mode of generic-PR-comments-as-only-conversation. + +Reads on the discussion surface follow §14 / the v0.3.0 anonymous-read +contract: anyone can see the conversation. Writes require contributor +role per §6.1 (v0.5.0 implements the gate; v0.6.0 — item #4 — hardens +adjacent surfaces to match). The notification routing reuses the +existing `chat_message_in_participated_thread` / +`chat_reply_to_my_message` event kinds with `branch_name=null` on the +fan-out row; the §15.7 reconciler and §15 inbox prose render +identically whether the chat lives on a branch or on the RFC's +discussion surface. A distinct `open_rfc_discussion_thread` event +kind is a §19.2 candidate if evidence demands the split. + --- ## 11. Branches and PRs: visibility, contribute, lifecycle @@ -2552,6 +2591,26 @@ The follow-up session will refine this. A minimal starting set: - `POST /api/rfcs//branches//threads//resolve` — resolve a thread per §8.12; permission per the rules in that section. +- `GET /api/rfcs//discussion/threads` — list threads on the + RFC's PR-less discussion surface per §10.10 (rows where + `threads.branch_name IS NULL`). Anonymous-readable per the v0.3.0 + anonymous-read contract; the default whole-doc chat thread is + materialized lazily on first read, mirroring the §8.12 branch-chat + default. +- `POST /api/rfcs//discussion/threads` — open a discussion + thread per §10.10. Body: optional `label` (short summary), optional + first `message`. Writes require contributor role; anonymous viewers + receive 401. Thread is created with `anchor_kind='whole-doc'`, + `thread_kind='chat'`, `branch_name=NULL`. +- `GET /api/rfcs//discussion/threads//messages` — + read messages on a discussion thread. Anonymous-readable. +- `POST /api/rfcs//discussion/threads//messages` — + post a message into a discussion thread per §10.10. Body: `text`, + optional `quote`. Writes require contributor role. +- `POST /api/rfcs//discussion/threads//resolve` — + resolve a discussion thread per §10.10; permission collapses to the + thread creator, any RFC owner / arbiter per §6.3, and any app + admin / owner per §6.1. - `POST /api/rfcs//branches//open-pr` — open a PR per §10.1; body carries the AI-drafted (and possibly edited) title and description. @@ -3287,6 +3346,55 @@ binding. ("operators MAY configure their monitoring to probe `/api/health`; the endpoint is unauthenticated by design"). §17 now lists the endpoint in its illustrative table.* +- **PR-less discussion: range and paragraph anchors.** v0.5.0 lands + the structural discussion surface (§10.10) but constrains every + PR-less thread to `anchor_kind='whole-doc'` — the data model permits + `range` and `paragraph` anchors (§5) but the UI work to surface a + passage-anchored thread on a non-branch view is the deferred half. + The natural follow-on is a margin-icon affordance on the main view + matching §8.12's branch-side surface, with the anchor stored on the + null-branch thread. Earns its session when discussion volume warrants + the precision; v0.5.0's flat surface is sufficient for most "have we + considered…?" gestures. Touches §10.10 and §8.12. +- **PR-less discussion: distinct notification event_kinds.** v0.5.0 + routes per-RFC discussion messages through the existing + `chat_message_in_participated_thread` and `chat_reply_to_my_message` + event kinds with `branch_name=null` on the fan-out row. The inbox + prose reads identically whether the chat lives on a branch or on the + RFC's discussion surface, which is honest signal: the conversation + shape is the same; only the scope differs. A future session may + introduce `open_rfc_discussion_thread` / `post_rfc_discussion_message` + if evidence shows contributors want to filter discussion-vs-branch + chat distinctly in the §15.2 inbox. Touches §15.1 (the event_kind + enum), §15.2 (the inbox filter chips), and §10.10. +- **PR-less discussion: chat-seen cursor.** §15.7 commits the + `branch_chat_seen` cursor for branch-scoped chat. The PR-less + discussion surface has no equivalent cursor in v0.5.0; the inbox + reconciler's keying on `(rfc_slug, branch_name)` does match a + null-branch advance, but no write path advances it. A natural + follow-on is a sibling table — `rfc_discussion_seen` or a + null-branch row on `branch_chat_seen` — that the discussion panel + advances on read, closing the §15.7 reconciliation loop for + discussion-surface notifications. Defer-able until inbox volume on + the new surface shows it matters. +- **PR-less discussion: AI participation.** v0.5.0's discussion + surface is human-only — no AI participant invocation, no `` + block parsing, no per-thread model picker. The branch chat (§8.12) + retains the §18 AI surface. The natural follow-on is wiring the AI + participant into discussion threads (the model picker, the + `Ask Claude` button on a selection tooltip) without enabling + document edits — a discussion-only AI turn produces only chat + content, no `changes` row, no commit. Contribution still requires a + PR; the AI's discussion-side help is just better prompts. Touches + §10.10, §8.12, and §18. +- **PR-less discussion: anonymous read polish.** v0.5.0 inherits the + v0.3.0 anonymous-read contract — anyone can read; only signed-in + contributors can write. The composer affordance for anonymous + viewers ("Sign in to comment.") matches the existing read-only-bar + treatment but the surface has not yet been audited for the v0.6.0 + hardening that tightens write gates app-wide. The §19.2 "public + face of discuss mode" entry overlaps; this entry is its discussion- + surface variant. - **Deployment-supplied subject framing.** The framework was built with one deployment in mind (OHM, standardizing natural-language vocabulary), but the substrate generalizes to any domain that diff --git a/VERSION b/VERSION index 1d0ba9e..8f0916f 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.0 +0.5.0 diff --git a/backend/app/api.py b/backend/app/api.py index b00bd32..d7bc6c7 100644 --- a/backend/app/api.py +++ b/backend/app/api.py @@ -20,6 +20,7 @@ from pydantic import BaseModel, Field from . import ( api_admin, api_branches, + api_discussion, api_graduation, api_notifications, api_prs, @@ -81,6 +82,12 @@ def make_router( # the §15.8 mute typeahead) and the §6/§17 admin surfaces # (role, write-mute, audit-log, graduation-readiness queue). router.include_router(api_admin.make_router(config)) + # v0.5.0: §5 / §7 / §10 — PR-less per-RFC discussion endpoints. + # The substrate is the existing threads/thread_messages tables; + # rows whose branch_name IS NULL scope to the RFC's main view. + # Contribution still requires a PR (api_prs above); this surface + # is for discussion that does not yet warrant a branch. + router.include_router(api_discussion.make_router()) # --------------------------------------------------------------- # §17: /api/health — unauthenticated post-flight probe. diff --git a/backend/app/api_discussion.py b/backend/app/api_discussion.py new file mode 100644 index 0000000..81d6e2f --- /dev/null +++ b/backend/app/api_discussion.py @@ -0,0 +1,330 @@ +"""§5 / §7 / §10 — PR-less per-RFC discussion endpoints (v0.5.0). + +This module surfaces the discussion-without-PR shape committed by the +roadmap's item #3. The substrate is the existing `threads` / +`thread_messages` pair from §5: rows whose `branch_name` is NULL are +scoped to the RFC's main view (the schema comment on the column says +exactly this; until now no write path produced such rows). This module +is the read+write surface for those rows. + +Contribution still requires a PR: the §10 PR flow is unchanged, the +branch-scoped chat in `api_branches.py` is unchanged, and accept / +decline of AI `` blocks still lives on a branch. What this +module adds is the "discuss freely about the RFC, no branch yet" surface +— a place to drop a question, a flag-style observation, or a multi-turn +conversation that does not yet warrant cutting a branch. + +Auth shape mirrors the v0.3.0 anonymous-read contract: reads are open, +writes require `auth.require_contributor`. Item #4 ("anon discuss/ +contribute off-limits") tightens the read gate in v0.6.0; v0.5.0's +write gate already holds the line. + +Notification routing reuses the existing `fan_out_chat_message` path +with `branch_name=None`; the `notifications.branch_name` column is +nullable, and the inbox row prose ("@alice posted a chat message on +") renders identically whether the chat lives on a branch +or on the RFC's discussion surface. The existing +`chat_message_in_participated_thread` / `chat_reply_to_my_message` +event kinds carry both shapes; introducing a parallel +`open_rfc_discussion_thread` / `post_rfc_discussion_message` enum pair +would split routing without adding signal. The §15 §19.2 candidate +"distinct event_kinds for PR-less discussion" notes the option for a +future session if evidence demands the split. +""" +from __future__ import annotations + +import json +import logging +from typing import Any + +from fastapi import APIRouter, HTTPException, Request +from pydantic import BaseModel, Field + +from . import auth, chat as chat_layer, db + +log = logging.getLogger(__name__) + + +# --------------------------------------------------------------------------- +# Request bodies +# --------------------------------------------------------------------------- + + +class DiscussionThreadCreateBody(BaseModel): + """A discussion thread is a `thread_kind='chat'`, `anchor_kind='whole-doc'`, + `branch_name=NULL` row. Anchored-range / per-paragraph threads on the + RFC discussion surface are a §19.2 candidate — the schema supports + them; the UI work to surface a range-anchor on a non-branch view is + the deferred part. v0.5.0 keeps the shape narrow.""" + label: str | None = Field(default=None, max_length=400) + message: str | None = Field(default=None, max_length=20_000) + + +class DiscussionMessageBody(BaseModel): + text: str = Field(min_length=1, max_length=20_000) + quote: str | None = Field(default=None, max_length=2000) + + +# --------------------------------------------------------------------------- +# Router +# --------------------------------------------------------------------------- + + +def make_router() -> APIRouter: + router = APIRouter() + + # ------------------------------------------------------------------- + # GET /api/rfcs//discussion/threads + # Lists every PR-less thread on the RFC. The default whole-doc thread + # is materialized lazily on first list (mirroring the §8.12 branch- + # chat default-thread treatment) so the UI always has a target for + # the compose-message affordance. + # ------------------------------------------------------------------- + + @router.get("/api/rfcs/{slug}/discussion/threads") + async def list_discussion_threads(slug: str, request: Request) -> dict[str, Any]: + viewer = auth.current_user(request) + _require_rfc_readable(slug) + # Ensure the default whole-doc discussion thread exists. We mint + # it on first read regardless of viewer (anonymous viewers can + # trigger the creation — the row's `created_by` is null in that + # case, mirroring `_ensure_branch_chat_thread`). + _ensure_discussion_thread(slug, viewer) + rows = db.conn().execute( + """ + SELECT id, anchor_kind, anchor_payload, thread_kind, label, state, + created_by, created_at, resolved_at, resolved_by + FROM threads + WHERE rfc_slug = ? AND branch_name IS NULL + ORDER BY id + """, + (slug,), + ).fetchall() + return {"items": [_serialize_thread(r) for r in rows]} + + # ------------------------------------------------------------------- + # POST /api/rfcs//discussion/threads + # Open a fresh discussion thread. Writes require require_contributor + # — anonymous viewers can read but cannot open a thread, per item + # #4's hardening anticipated in v0.6.0 (we already enforce it here + # to avoid the open window). + # ------------------------------------------------------------------- + + @router.post("/api/rfcs/{slug}/discussion/threads") + async def create_discussion_thread( + slug: str, body: DiscussionThreadCreateBody, request: Request + ) -> dict[str, Any]: + viewer = auth.require_contributor(request) + _require_rfc_readable(slug) + cur = db.conn().execute( + """ + INSERT INTO threads + (rfc_slug, branch_name, anchor_kind, anchor_payload, + thread_kind, label, created_by) + VALUES (?, NULL, 'whole-doc', NULL, 'chat', ?, ?) + """, + (slug, body.label, viewer.user_id), + ) + thread_id = cur.lastrowid + message_id = None + if body.message: + message_id = chat_layer.append_user_message( + thread_id=thread_id, + author_user_id=viewer.user_id, + text=body.message, + quote=None, + ) + return {"thread_id": thread_id, "message_id": message_id} + + # ------------------------------------------------------------------- + # GET /api/rfcs//discussion/threads//messages + # ------------------------------------------------------------------- + + @router.get("/api/rfcs/{slug}/discussion/threads/{thread_id}/messages") + async def get_discussion_thread_messages( + slug: str, thread_id: int, request: Request + ) -> dict[str, Any]: + _viewer = auth.current_user(request) + _require_rfc_readable(slug) + thread = _require_discussion_thread(slug, thread_id) + rows = db.conn().execute( + """ + SELECT m.id, m.role, m.author_user_id, + u.gitea_login AS author_login, + u.display_name AS author_display, + m.model_id, m.text, m.quote, m.created_at + FROM thread_messages m + LEFT JOIN users u ON u.id = m.author_user_id + WHERE m.thread_id = ? + ORDER BY m.id + """, + (thread_id,), + ).fetchall() + return { + "thread": _serialize_thread(thread), + "messages": [_serialize_message(r) for r in rows], + } + + # ------------------------------------------------------------------- + # POST /api/rfcs//discussion/threads//messages + # ------------------------------------------------------------------- + + @router.post("/api/rfcs/{slug}/discussion/threads/{thread_id}/messages") + async def post_discussion_message( + slug: str, thread_id: int, body: DiscussionMessageBody, request: Request + ) -> dict[str, Any]: + viewer = auth.require_contributor(request) + _require_rfc_readable(slug) + _require_discussion_thread(slug, thread_id) + message_id = chat_layer.append_user_message( + thread_id=thread_id, + author_user_id=viewer.user_id, + text=body.text, + quote=body.quote, + ) + return {"ok": True, "message_id": message_id} + + # ------------------------------------------------------------------- + # POST /api/rfcs//discussion/threads//resolve + # ------------------------------------------------------------------- + + @router.post("/api/rfcs/{slug}/discussion/threads/{thread_id}/resolve") + async def resolve_discussion_thread( + slug: str, thread_id: int, request: Request + ) -> dict[str, Any]: + viewer = auth.require_contributor(request) + rfc = _require_rfc_readable(slug) + thread = _require_discussion_thread(slug, thread_id) + if not _can_resolve(rfc, thread, viewer): + raise HTTPException( + 403, + "Only the thread creator, an RFC owner/arbiter, or an app admin/owner may resolve", + ) + db.conn().execute( + """ + UPDATE threads + SET state = 'resolved', + resolved_by = ?, + resolved_at = datetime('now') + WHERE id = ? + """, + (viewer.user_id, thread_id), + ) + return {"ok": True, "thread_id": thread_id} + + return router + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _require_rfc_readable(slug: str): + """Per the v0.3.0 anonymous-read contract: any cached RFC is readable + by anyone. Withdrawn entries refuse reads of every shape — same rule + `_require_rfc_with_repo` in `api_branches.py` follows.""" + row = db.conn().execute( + "SELECT * FROM cached_rfcs WHERE slug = ?", (slug,) + ).fetchone() + if row is None: + raise HTTPException(404, "RFC not found") + if row["state"] == "withdrawn": + raise HTTPException(409, "RFC is withdrawn") + return row + + +def _require_discussion_thread(slug: str, thread_id: int): + """A discussion thread is one whose (rfc_slug, branch_name) = (slug, + NULL). Refuse cleanly if the thread id resolves to a branch-scoped + thread instead — that lookup belongs on the branch endpoints.""" + row = db.conn().execute( + """ + SELECT * FROM threads + WHERE id = ? AND rfc_slug = ? AND branch_name IS NULL + """, + (thread_id, slug), + ).fetchone() + if not row: + raise HTTPException(404, "Discussion thread not found") + return row + + +def _ensure_discussion_thread(slug: str, viewer) -> int: + """Per the §8.12 lazy-create pattern, materialize a default whole-doc + chat thread on the RFC's discussion surface on first read. Created_by + is null when an anonymous viewer triggers creation — the thread is + structurally owned by the RFC, not by whoever opened the view.""" + row = db.conn().execute( + """ + SELECT id FROM threads + WHERE rfc_slug = ? AND branch_name IS NULL + AND anchor_kind = 'whole-doc' AND thread_kind = 'chat' + ORDER BY id LIMIT 1 + """, + (slug,), + ).fetchone() + if row: + return row["id"] + cur = db.conn().execute( + """ + INSERT INTO threads + (rfc_slug, branch_name, anchor_kind, thread_kind, label, created_by) + VALUES (?, NULL, 'whole-doc', 'chat', NULL, ?) + """, + (slug, viewer.user_id if viewer else None), + ) + return cur.lastrowid + + +def _can_resolve(rfc, thread, viewer) -> bool: + if viewer is None: + return False + if viewer.role in ("owner", "admin"): + return True + owners = json.loads(rfc["owners_json"] or "[]") + arbiters = json.loads(rfc["arbiters_json"] or "[]") + if viewer.gitea_login in owners or viewer.gitea_login in arbiters: + return True + if thread["created_by"] == viewer.user_id: + return True + return False + + +# --------------------------------------------------------------------------- +# Serializers — mirror api_branches.py's shape +# --------------------------------------------------------------------------- + + +def _serialize_thread(row) -> dict[str, Any]: + payload = row["anchor_payload"] + try: + anchor = json.loads(payload) if payload else None + except Exception: + anchor = None + return { + "id": row["id"], + "anchor_kind": row["anchor_kind"], + "anchor_payload": anchor, + "thread_kind": row["thread_kind"], + "label": row["label"], + "state": row["state"], + "created_by": row["created_by"], + "created_at": row["created_at"], + "resolved_at": row["resolved_at"] if "resolved_at" in row.keys() else None, + "resolved_by": row["resolved_by"] if "resolved_by" in row.keys() else None, + } + + +def _serialize_message(row) -> dict[str, Any]: + return { + "id": row["id"], + "role": row["role"], + "author_user_id": row["author_user_id"], + "author_login": row["author_login"], + "author_display": row["author_display"], + "model_id": row["model_id"], + "text": row["text"], + "quote": row["quote"], + "created_at": row["created_at"], + } diff --git a/backend/app/chat.py b/backend/app/chat.py index 13538af..f97d577 100644 --- a/backend/app/chat.py +++ b/backend/app/chat.py @@ -168,10 +168,15 @@ def _fan_out_chat(thread_id: int, author_user_id: int, message_id: int) -> None: ).fetchone() if pr_row: pr_number = pr_row["pr_number"] + # v0.5.0 (§5 / §10 — PR-less discussion): a thread with + # branch_name IS NULL is scoped to the RFC's main view. Pass None + # through to the notify chokepoint so the notifications row keeps + # `branch_name` null — coercing it to "main" would misroute the + # §15.7 chat-seen reconciler (which keys on branch_name). notify.fan_out_chat_message( actor_user_id=author_user_id, rfc_slug=row["rfc_slug"], - branch_name=row["branch_name"] or "main", + branch_name=row["branch_name"], thread_id=thread_id, message_id=message_id, is_review_thread=(row["thread_kind"] == "review"), diff --git a/backend/app/notify.py b/backend/app/notify.py index 610195d..96b8922 100644 --- a/backend/app/notify.py +++ b/backend/app/notify.py @@ -212,7 +212,7 @@ def fan_out_chat_message( *, actor_user_id: int, rfc_slug: str, - branch_name: str, + branch_name: str | None, thread_id: int, message_id: int, is_review_thread: bool = False, @@ -227,6 +227,14 @@ def fan_out_chat_message( (state='watching', i.e. full stream) get a churn-class `chat_message_in_participated_thread`. The two are union'd so a user who is both gets only the personal-direct row. + + v0.5.0: `branch_name` may be None — that is the PR-less per-RFC + discussion shape (`threads.branch_name IS NULL`, §5). The + notifications row carries the null through; the inbox prose renders + identically whether the chat lives on a branch or on the RFC's + discussion surface, and the §15.7 reconciler keys on + (rfc_slug, branch_name) so a null branch correctly matches the + PR-less discussion's eventual chat-seen-equivalent advance. """ _bump_auto_watch(actor_user_id, rfc_slug) diff --git a/backend/tests/test_discussion_vertical.py b/backend/tests/test_discussion_vertical.py new file mode 100644 index 0000000..c4e93ac --- /dev/null +++ b/backend/tests/test_discussion_vertical.py @@ -0,0 +1,237 @@ +"""End-to-end integration tests for the v0.5.0 PR-less discussion +surface — roadmap item #3, "discussion without PR; contribution requires +PR." + +The vertical: an active RFC exists; the discussion endpoints under +`/api/rfcs//discussion/...` open threads with +`threads.branch_name IS NULL`, post messages into them, and surface +them on subsequent reads. Branch-scoped threads (the §8.12 surface) +remain segregated. Anonymous viewers can read; only signed-in +contributors can write. +""" +from __future__ import annotations + +import pytest + +# Reuse the harness from Slice 1 / Slice 2. +from test_propose_vertical import ( # noqa: F401 — fixtures land via import + FakeGitea, + app_with_fake_gitea, + provision_user_row, + sign_in_as, + tmp_env, +) +from test_rfc_view_vertical import seed_active_rfc, SEED_BODY + + +# --------------------------------------------------------------------------- +# Tests +# --------------------------------------------------------------------------- + + +def test_create_and_post_to_pr_less_discussion_thread(app_with_fake_gitea): + """The vertical: signed-in contributor opens a thread on the RFC's + discussion surface, posts a message, and the thread + message + surface on subsequent reads with branch_name IS NULL.""" + from fastapi.testclient import TestClient + from app import db + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=1, login="alice", role="contributor") + seed_active_rfc(fake, slug="ohm", title="OHM", body=SEED_BODY) + sign_in_as(client, user_id=1, gitea_login="alice", display_name="Alice", role="contributor") + + # Listing materializes the default whole-doc thread. + r = client.get("/api/rfcs/ohm/discussion/threads") + assert r.status_code == 200, r.text + items = r.json()["items"] + assert len(items) == 1 + default_thread_id = items[0]["id"] + assert items[0]["anchor_kind"] == "whole-doc" + assert items[0]["thread_kind"] == "chat" + + # Open an additional discussion thread with a first message. + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"label": "Question about §3", "message": "Is consent baked into the trait model?"}, + ) + assert r.status_code == 200, r.text + payload = r.json() + thread_id = payload["thread_id"] + message_id = payload["message_id"] + assert thread_id is not None and message_id is not None + + # Confirm the row carries branch_name IS NULL (the PR-less shape). + row = db.conn().execute( + "SELECT rfc_slug, branch_name, thread_kind, anchor_kind, created_by FROM threads WHERE id = ?", + (thread_id,), + ).fetchone() + assert row["rfc_slug"] == "ohm" + assert row["branch_name"] is None + assert row["thread_kind"] == "chat" + assert row["anchor_kind"] == "whole-doc" + assert row["created_by"] == 1 + + # The thread surfaces on the list endpoint alongside the default. + r = client.get("/api/rfcs/ohm/discussion/threads") + ids = [t["id"] for t in r.json()["items"]] + assert default_thread_id in ids + assert thread_id in ids + + # Posting a reply on the new thread persists and returns the id. + r = client.post( + f"/api/rfcs/ohm/discussion/threads/{thread_id}/messages", + json={"text": "Following up — see §3.2."}, + ) + assert r.status_code == 200, r.text + reply_id = r.json()["message_id"] + + # The messages read endpoint returns both messages in order. + r = client.get(f"/api/rfcs/ohm/discussion/threads/{thread_id}/messages") + assert r.status_code == 200 + messages = r.json()["messages"] + assert [m["id"] for m in messages] == [message_id, reply_id] + assert messages[0]["author_login"] == "alice" + assert messages[0]["text"].startswith("Is consent") + + +def test_anonymous_can_read_but_cannot_post_discussion(app_with_fake_gitea): + """Per the v0.3.0 anonymous-read contract: reads on the discussion + surface are open; write attempts return 401. v0.6.0 (item #4) will + tighten the read gate — v0.5.0 holds the write line so there is no + open window between releases.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + seed_active_rfc(fake, slug="ohm", title="OHM", body=SEED_BODY) + + # Seed the discussion thread + first message as Alice. + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor") + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"message": "First."}, + ) + assert r.status_code == 200 + thread_id = r.json()["thread_id"] + + # Drop the session — viewer is anonymous now. + client.cookies.clear() + + # Reads are open. + r = client.get("/api/rfcs/ohm/discussion/threads") + assert r.status_code == 200 + assert any(t["id"] == thread_id for t in r.json()["items"]) + + r = client.get(f"/api/rfcs/ohm/discussion/threads/{thread_id}/messages") + assert r.status_code == 200 + assert len(r.json()["messages"]) >= 1 + + # Writes refuse 401. + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"message": "Drive-by."}, + ) + assert r.status_code == 401 + + r = client.post( + f"/api/rfcs/ohm/discussion/threads/{thread_id}/messages", + json={"text": "Drive-by reply."}, + ) + assert r.status_code == 401 + + +def test_discussion_threads_and_branch_threads_are_segregated(app_with_fake_gitea): + """A branch-scoped thread (the §8.12 surface, branch_name='main' or a + feature branch) MUST NOT surface on the discussion endpoint, which + is keyed on branch_name IS NULL. The two surfaces share a table; the + null-filter is what segregates them.""" + from fastapi.testclient import TestClient + from app import db + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=3, login="alice", role="contributor") + seed_active_rfc(fake, slug="ohm", title="OHM", body=SEED_BODY) + sign_in_as(client, user_id=3, gitea_login="alice", display_name="Alice", role="contributor") + + # Manually materialize a branch-scoped thread on a feature branch. + db.conn().execute( + """ + INSERT INTO threads + (rfc_slug, branch_name, anchor_kind, thread_kind, label, created_by) + VALUES ('ohm', 'alice-draft-aa00', 'whole-doc', 'chat', NULL, 3) + """ + ) + # And one on the discussion surface. + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"message": "Discussion-surface message."}, + ) + assert r.status_code == 200 + discussion_thread_id = r.json()["thread_id"] + + # The discussion list contains the null-branch thread (plus the + # default whole-doc) and excludes the feature-branch thread. + r = client.get("/api/rfcs/ohm/discussion/threads") + assert r.status_code == 200 + ids = [t["id"] for t in r.json()["items"]] + assert discussion_thread_id in ids + # Feature-branch thread MUST NOT surface. + branch_thread_row = db.conn().execute( + "SELECT id FROM threads WHERE branch_name = 'alice-draft-aa00'" + ).fetchone() + assert branch_thread_row is not None + assert branch_thread_row["id"] not in ids + + +def test_discussion_thread_resolve_permissions(app_with_fake_gitea): + """A thread's creator can resolve it; an unrelated contributor cannot; + an admin / owner / RFC-owner can. Mirrors §8.12's resolution rule for + branch-scoped threads.""" + from fastapi.testclient import TestClient + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=4, login="alice", role="contributor") + provision_user_row(user_id=5, login="bob", role="contributor") + provision_user_row(user_id=6, login="ben", role="owner") + seed_active_rfc(fake, slug="ohm", title="OHM", body=SEED_BODY) + + sign_in_as(client, user_id=4, gitea_login="alice", display_name="Alice", role="contributor") + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"label": "Alice's thread", "message": "..."}, + ) + thread_id = r.json()["thread_id"] + + # Unrelated contributor refused. + sign_in_as(client, user_id=5, gitea_login="bob", display_name="Bob", role="contributor") + r = client.post(f"/api/rfcs/ohm/discussion/threads/{thread_id}/resolve") + assert r.status_code == 403 + + # Creator allowed. + sign_in_as(client, user_id=4, gitea_login="alice", display_name="Alice", role="contributor") + r = client.post(f"/api/rfcs/ohm/discussion/threads/{thread_id}/resolve") + assert r.status_code == 200 + + # Open another thread, resolve it as the owner. + r = client.post( + "/api/rfcs/ohm/discussion/threads", + json={"label": "Another thread", "message": "..."}, + ) + thread_id2 = r.json()["thread_id"] + sign_in_as(client, user_id=6, gitea_login="ben", display_name="Ben", role="owner") + r = client.post(f"/api/rfcs/ohm/discussion/threads/{thread_id2}/resolve") + assert r.status_code == 200 + + +def test_discussion_404_on_unknown_rfc(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + r = client.get("/api/rfcs/nonexistent/discussion/threads") + assert r.status_code == 404 diff --git a/frontend/package-lock.json b/frontend/package-lock.json index 7a70921..ee1899c 100644 --- a/frontend/package-lock.json +++ b/frontend/package-lock.json @@ -1,12 +1,12 @@ { "name": "rfc-app-frontend", - "version": "0.2.1", + "version": "0.5.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "rfc-app-frontend", - "version": "0.2.1", + "version": "0.5.0", "dependencies": { "@codemirror/commands": "^6.10.3", "@codemirror/lang-markdown": "^6.5.0", diff --git a/frontend/package.json b/frontend/package.json index e7d105d..b68f1f4 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.4.0", + "version": "0.5.0", "type": "module", "scripts": { "dev": "vite", diff --git a/frontend/src/App.css b/frontend/src/App.css index a577e39..87e77c0 100644 --- a/frontend/src/App.css +++ b/frontend/src/App.css @@ -1776,3 +1776,91 @@ .grad-queue-link:hover strong { text-decoration: underline; } .muted { color: #6b7280; } .error { color: #b91c1c; } + +/* v0.5.0 — PR-less per-RFC discussion panel (RFCDiscussionPanel.jsx). + Visual neighbor of .chat-panel but distinct: discussion lives on the + RFC, branch chat lives on the branch. Same flex column shape so it + slots cleanly into the existing .right-panel container. +*/ +.discussion-panel { + flex: 1; display: flex; flex-direction: column; + overflow: hidden; min-height: 0; +} +.discussion-header { + padding: 10px 14px; + border-bottom: 1px solid #f0f0ee; + background: #fafafa; + display: flex; flex-direction: column; gap: 4px; +} +.discussion-header-title { font-size: 12px; color: #555; font-weight: 600; } +.discussion-header-meta { font-size: 11px; color: #888; } +.discussion-thread-tabs { + display: flex; gap: 4px; flex-wrap: wrap; + padding: 6px 14px; + border-bottom: 1px solid #f0f0ee; + background: #fcfcfb; +} +.discussion-thread-tab { + background: #fff; border: 1px solid #e5e5e0; cursor: pointer; + font-size: 11px; color: #555; + padding: 3px 8px; border-radius: 999px; +} +.discussion-thread-tab.active { + background: #eef2ff; border-color: #5b5bd6; color: #3737a0; +} +.discussion-thread-tab.resolved { opacity: 0.6; } +.discussion-messages { + flex: 1; overflow-y: auto; + padding: 14px; + display: flex; flex-direction: column; gap: 10px; +} +.discussion-empty { + flex: 1; display: flex; align-items: center; justify-content: center; + text-align: center; padding: 24px; +} +.discussion-empty p { + font-size: 13px; color: #999; line-height: 1.6; max-width: 280px; +} +.discussion-error { + background: #fee; border: 1px solid #fcc; color: #b91c1c; + padding: 8px 10px; border-radius: 4px; font-size: 12px; +} +.discussion-message { display: flex; flex-direction: column; gap: 3px; } +.discussion-message-meta { + display: flex; gap: 8px; font-size: 11px; color: #888; +} +.discussion-message-author { color: #5b5bd6; font-weight: 500; } +.discussion-message-quote { + font-size: 11px; color: #666; font-style: italic; + border-left: 2px solid #ddd; padding-left: 8px; margin-bottom: 2px; +} +.discussion-message-body { + font-size: 13px; color: #222; line-height: 1.5; + white-space: pre-wrap; word-wrap: break-word; + background: #f7f7f5; padding: 8px 10px; border-radius: 6px; +} +.discussion-message.system .discussion-system-bubble { + font-size: 12px; color: #888; font-style: italic; + text-align: center; padding: 4px 0; +} +.discussion-composer { + border-top: 1px solid #f0f0ee; + padding: 10px 14px; + background: #fafafa; + display: flex; flex-direction: column; gap: 6px; +} +.discussion-composer-textarea { + width: 100%; resize: vertical; min-height: 60px; + font-family: inherit; font-size: 13px; + border: 1px solid #ddd; border-radius: 4px; + padding: 6px 8px; +} +.discussion-composer-textarea:focus { + outline: none; border-color: #5b5bd6; +} +.discussion-composer-actions { + display: flex; gap: 8px; align-items: center; justify-content: flex-end; +} +.discussion-readonly { + font-size: 12px; color: #666; padding: 4px 0; +} diff --git a/frontend/src/api.js b/frontend/src/api.js index c96a37b..1901a93 100644 --- a/frontend/src/api.js +++ b/frontend/src/api.js @@ -197,6 +197,52 @@ export async function resolveThread(slug, branch, threadId) { return jsonOrThrow(res) } +// ── v0.5.0: PR-less per-RFC discussion (§5 / §10) ──────────────────────── +// +// The substrate is `threads.branch_name IS NULL` — the same threads +// table the branch chat uses, with a null branch the schema already +// supported. Contribution still requires a PR (api_prs / openPR), so +// these endpoints are read+write for discussion only. + +export async function listDiscussionThreads(slug) { + return jsonOrThrow(await fetch(`/api/rfcs/${slug}/discussion/threads`)) +} + +export async function createDiscussionThread(slug, { label = null, message = null } = {}) { + const res = await fetch(`/api/rfcs/${slug}/discussion/threads`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ label, message }), + }) + return jsonOrThrow(res) +} + +export async function getDiscussionThreadMessages(slug, threadId) { + return jsonOrThrow(await fetch( + `/api/rfcs/${slug}/discussion/threads/${threadId}/messages`, + )) +} + +export async function postDiscussionMessage(slug, threadId, { text, quote = null }) { + const res = await fetch( + `/api/rfcs/${slug}/discussion/threads/${threadId}/messages`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ text, quote }), + }, + ) + return jsonOrThrow(res) +} + +export async function resolveDiscussionThread(slug, threadId) { + const res = await fetch( + `/api/rfcs/${slug}/discussion/threads/${threadId}/resolve`, + { method: 'POST' }, + ) + return jsonOrThrow(res) +} + // ── Slice 4: super-draft body editing (§9.5) ───────────────────────────── export async function startEditBranch(slug, body = {}) { diff --git a/frontend/src/components/RFCDiscussionPanel.jsx b/frontend/src/components/RFCDiscussionPanel.jsx new file mode 100644 index 0000000..9d812c8 --- /dev/null +++ b/frontend/src/components/RFCDiscussionPanel.jsx @@ -0,0 +1,290 @@ +// RFCDiscussionPanel.jsx — v0.5.0's PR-less per-RFC discussion surface. +// +// Roadmap item #3: an RFC's main view now has a discussion surface +// distinct from PR comments and from branch chat. The substrate is the +// existing threads/thread_messages tables — rows with +// `threads.branch_name IS NULL` scope to "the RFC, no branch yet." +// +// Reused as the right-column panel on `branchParam === 'main'`. Branch +// chat (ChatPanel.jsx) keeps its existing role for branch-scoped work, +// including PRs. Contribution remains gated behind opening a PR — +// nothing here writes to the document. + +import { useCallback, useEffect, useRef, useState } from 'react' +import { + createDiscussionThread, + getDiscussionThreadMessages, + listDiscussionThreads, + postDiscussionMessage, + resolveDiscussionThread, +} from '../api' + +export default function RFCDiscussionPanel({ slug, viewer }) { + const [threads, setThreads] = useState([]) + const [messagesByThread, setMessagesByThread] = useState({}) + const [composer, setComposer] = useState('') + const [activeThreadId, setActiveThreadId] = useState(null) + const [error, setError] = useState(null) + const [sending, setSending] = useState(false) + const bottomRef = useRef(null) + + // Pull threads + messages on mount / slug change. + useEffect(() => { + if (!slug) return + let cancelled = false + setError(null) + setThreads([]) + setMessagesByThread({}) + setActiveThreadId(null) + listDiscussionThreads(slug) + .then(async ({ items }) => { + if (cancelled) return + setThreads(items || []) + // Pre-load messages for each thread. The list is small (per-RFC, + // not per-branch) so a fan-out fetch is fine; §19.2 candidate + // for paging if a hot RFC accumulates lots of threads. + const collected = {} + for (const t of items || []) { + try { + const { messages } = await getDiscussionThreadMessages(slug, t.id) + collected[t.id] = messages + } catch { + collected[t.id] = [] + } + } + if (!cancelled) { + setMessagesByThread(collected) + // Default the active thread to the system's lazy whole-doc + // default (the first row with anchor_kind='whole-doc' and + // no label) so the composer wires to a real id immediately. + const dflt = (items || []).find( + t => t.anchor_kind === 'whole-doc' && !t.label, + ) + setActiveThreadId(dflt?.id || items?.[0]?.id || null) + } + }) + .catch(err => { if (!cancelled) setError(err.message) }) + return () => { cancelled = true } + }, [slug]) + + // Scroll to bottom when messages land in the active thread. + useEffect(() => { + bottomRef.current?.scrollIntoView({ behavior: 'smooth' }) + }, [activeThreadId, messagesByThread[activeThreadId]?.length]) + + const handleSend = useCallback(async () => { + if (!viewer) { window.location.href = '/auth/login'; return } + const text = composer.trim() + if (!text || sending) return + setSending(true) + setError(null) + try { + // If no thread yet, mint one with the message as its first turn. + if (!activeThreadId) { + const { thread_id, message_id } = await createDiscussionThread(slug, { message: text }) + // Re-pull authoritative state — the default whole-doc thread + // existed pre-this call (the GET creates it lazily), so we + // either get the existing default's id back from the new + // thread's row or the prior default; either way the list call + // is the source of truth. + const { items } = await listDiscussionThreads(slug) + setThreads(items || []) + const { messages } = await getDiscussionThreadMessages(slug, thread_id) + setMessagesByThread(prev => ({ ...prev, [thread_id]: messages })) + setActiveThreadId(thread_id) + void message_id + } else { + const { message_id } = await postDiscussionMessage(slug, activeThreadId, { text }) + const { messages } = await getDiscussionThreadMessages(slug, activeThreadId) + setMessagesByThread(prev => ({ ...prev, [activeThreadId]: messages })) + void message_id + } + setComposer('') + } catch (err) { + setError(err.message) + } finally { + setSending(false) + } + }, [composer, sending, viewer, slug, activeThreadId]) + + const handleNewThread = useCallback(async () => { + if (!viewer) { window.location.href = '/auth/login'; return } + setError(null) + try { + const { thread_id } = await createDiscussionThread(slug, { label: null, message: null }) + const { items } = await listDiscussionThreads(slug) + setThreads(items || []) + setActiveThreadId(thread_id) + setMessagesByThread(prev => ({ ...prev, [thread_id]: [] })) + } catch (err) { + setError(err.message) + } + }, [viewer, slug]) + + const handleResolve = useCallback(async (threadId) => { + if (!viewer) return + setError(null) + try { + await resolveDiscussionThread(slug, threadId) + const { items } = await listDiscussionThreads(slug) + setThreads(items || []) + } catch (err) { + setError(err.message) + } + }, [viewer, slug]) + + const onKeyDown = useCallback((e) => { + if (e.key === 'Enter' && (e.metaKey || e.ctrlKey)) { + e.preventDefault() + handleSend() + } + }, [handleSend]) + + const activeThread = threads.find(t => t.id === activeThreadId) || null + const activeMessages = messagesByThread[activeThreadId] || [] + const openThreads = threads.filter(t => t.state === 'open') + + return ( +
+
+ + Discussion Beta + + + {openThreads.length} open thread{openThreads.length === 1 ? '' : 's'} + {' · '}contribution requires a PR + +
+ + {threads.length > 1 && ( +
+ {threads.map(t => ( + + ))} +
+ )} + +
+ {error &&
{error}
} + {activeMessages.length === 0 && !error && ( +
+

+ {viewer + ? 'No discussion yet. Be the first to comment — discussion lives here without opening a PR. To propose an edit, use Start Contributing above.' + : 'No discussion yet. Sign in to comment. Discussion lives here without opening a PR; proposed edits still flow through PRs.'} +

+
+ )} + {activeMessages.map(msg => ( + + ))} +
+
+ +
+ {viewer ? ( + <> +