glass beadian
aa5e262f3a
Self-hosted multi-platform broadcast newsroom for activist collectives: publishing pipeline with group approval and DST-proof scheduling, TweetDeck-style column wall with hostile-content-safe rendering, live control room with Auto Director, bridged chat governance with polls and quorum, distributed sandboxed transcode workers, encrypted content-addressed archive, monthly analytics. MIT licensed. See README.md and docs/.
987 lines
87 KiB
Markdown
987 lines
87 KiB
Markdown
# Proposal — Group Review & Signoff for Posts ("Editorial Workflow")
|
||
|
||
Lane: `glm` · Date: 2026-10-07 · Spec: `runs/wp5/spec-review-workflow.md`
|
||
Status: design only — nothing here is implemented.
|
||
|
||
---
|
||
|
||
## 0. Files this proposal is grounded in
|
||
|
||
Everything below cites these repo files (paths relative to repo root):
|
||
|
||
| File | What it establishes |
|
||
| --- | --- |
|
||
| `apps/api/migrations/006_publishing.sql` | current 6-state CHECK (`:7-8`), single `reviewer_user_id` (`:9`), `UNIQUE(draft_id)` on schedules (`:24`), FORCE RLS + `tenant_isolation` policies (`:52-66`), `sp_app` grants (`:68`), SECURITY DEFINER sweeper fns (`:72-89`) |
|
||
| `apps/api/src/publishing/publishing.service.ts` | `DraftState` union (`:8`), submit/approve/reject (`:179-205`), schedule (`:212-247`), CAS `transition()` helper `WHERE id=$1 AND state=$4` (`:275-292`), the "FK checks bypass RLS" tenancy note (`:217-219`) |
|
||
| `apps/scheduler/src/activities/publish.activity.ts` | scheduler owns `scheduled→published` (`:182`), claims via `publishing_schedule_workspace()` (`:160-176`), writes audit as `sp_scheduler` (`:184-199`), tenant-tx pattern `inTenant()` (`:228-245`) |
|
||
| `apps/api/migrations/008_groups.sql` | `group_members` has **no role column** (`:27-39`), `UNIQUE(group_id,user_id)` (`:34`), composite tenant-pinning FKs (`:38-39`), epoch bump on any membership mutation (`:79-80`), `groups UNIQUE(id, workspace_id)` (`:21`) |
|
||
| `apps/api/src/groups/groups.service.ts` | same-tx audit helper (`:100-110`), `{token}` replacement-session pattern on epoch-bumping mutations (`:168-200`), guard-side `userCanUseConnection` reading live role from DB (`:425-455`) |
|
||
| `apps/api/migrations/002_rbac_seed.sql` | 10 roles; `approval.grant` on Owner/Admin/Approver (`:8, :24, :49`), `approval.request` on Owner/Admin/Publisher, workspace `Editor` role has `draft.edit` and **no** approval power (`:39-43`) |
|
||
| `apps/api/src/rbac/rbac.guard.ts` | epoch fail-closed check (`:45-47`), permission lookup inside `withTenant` (`:57-62`), `RequireGroupAccess` precedent for guard-side group checks (`:64-74`) |
|
||
| `apps/api/src/audit/audit.service.ts` + `apps/api/migrations/001_tenants.sql` | `scrub()` (`:11`), `AuditEvent` (`:20-27`), `audit_append` SECURITY DEFINER + `fn_assert_tenant` (`001:85-111`), `audit_hash` (`001:74-83`), `reject_workspace_change` (`001:141-150`), `bump_authz_epoch` (`001:160-168`), fail-closed NULLIF RLS (`001:172-190`) |
|
||
| `apps/api/src/auth/auth.service.ts` / `session.ts` | `issueSessionInTenant` (`:72-84`), `epochValid` (`:50-52`) |
|
||
| `apps/api/src/db/client.ts` | `withTenant` sets `app.workspace_id` per tx (`:18-44`) |
|
||
| `apps/web/src/app/(main)/compose/page.tsx` | `STATE_TONE` badge map (`:35-42`), the create→submit→approve→schedule chain incl. self-approve (`:119-142`), preview cards (`:244-262`), "Recent drafts" list (`:255-263`) |
|
||
| `apps/web/src/components/streams/column.tsx` + `streams/page.tsx` | virtualizer config (`column.tsx:26-34`), j/k keyboard model with input-guard (`streams/page.tsx:56-70`) |
|
||
| `apps/web/src/app/(main)/inbox/page.tsx` | two-pane chat visual pattern (`:52-120`) |
|
||
| `apps/web/src/app/(main)/approvals/page.tsx` | 10-line stub this feature replaces |
|
||
| `apps/api/src/realtime/gateway.ts` | authz **port only** — no transport, no topics, no client (`:1-45`) |
|
||
| `apps/api/migrations/009_video.sql` | takes number `009`; this proposal's migration is **`010_review_workflow.sql`** |
|
||
| `apps/api/src/db/migrate.ts` | each migration file runs in one tx, exactly once (`:33-41`) |
|
||
|
||
---
|
||
|
||
## 1. Executive summary — the five sharpest decisions
|
||
|
||
1. **Group roles are a column on `group_members`** (`role ∈ member|editor|reviewer`), not a new table and not workspace roles. The existing epoch trigger (`008_groups.sql:79-80`) already fires on UPDATE, so role changes invalidate sessions for free and the `{token}` re-issue pattern (`groups.service.ts:168-200`) extends verbatim. One role per user per group is a *feature*: letting one person be both editor and reviewer in the same group would re-open the maker-checker hole we are structurally closing.
|
||
|
||
2. **Approvals attach to immutable version rows, never to the draft.** `post_versions` is append-only (UPDATE blocked by trigger), carries a `content_hash` generated column (`encode(digest(post::text,'sha256'),'hex')`, same crypto as `audit_hash` in `001:74-83`), and a review cycle pins one version. Any edit — before, during, or after review — closes the cycle as `superseded` and requires a fresh quorum. There is no state in which content is mutable while approvals are live.
|
||
|
||
3. **Quorum is snapshotted per review cycle**, resolved as `groups.review_approvals_required ?? workspace_review_settings.approvals_required ?? 1` at submit time. Mid-review config changes never retroactively move the goalposts; only future cycles pick them up. Default `1` preserves today's behavior exactly (including the solo-workspace self-approve flow at `compose/page.tsx:128-131`); `≥2` turns on structural four-eyes, at which point author votes are refused server-side with a dedicated 403.
|
||
|
||
4. **Chat is polling with a per-thread monotonic cursor**, not WS/SSE. `realtime/gateway.ts` is an unwired port; EventSource cannot send the Bearer token `api-client.ts` stores in localStorage; and every poll rides the existing epoch-fenced `RbacGuard` (`rbac.guard.ts:45-47`) for free, so an admin removing a reviewer mid-thread fails the *next poll* closed. The data model (`review_messages` with `UNIQUE(draft_id, ord)`) is transport-agnostic so WS can later reuse the identical "events since ord" query.
|
||
|
||
5. **`published` is the archive; hide and delete are flags, not states.** The scheduler's `scheduled→published` write (`publish.activity.ts:182`) needs zero changes, the archive screen is a filtered read of `state='published'`, and moderation (`hidden_at`, `deleted_at`) is admin-only (`moderation.hide`, `moderation.delete` granted to Owner/Admin only), soft, and hash-chain-audited — the chat record survives deletion because review history is a compliance artifact.
|
||
|
||
The five new machine states are `needs_changes` (replaces terminal `rejected`, unlocks resubmit), `expired`, `blocked`, plus reuse of `editing/in_review/approved/scheduled/published`; the state machine is enforced **twice** — CAS UPDATEs in `publishing.service.ts` style and a new DB legal-transition trigger (defense in depth, per spec §3).
|
||
|
||
---
|
||
|
||
## 2. Answers to the ten open questions (spec §5)
|
||
|
||
**Q1 — Group-scoped role storage: column on `group_members` vs `group_member_roles` table vs reusing workspace roles?**
|
||
**Decision: `group_members.role text NOT NULL DEFAULT 'member' CHECK (role IN ('member','editor','reviewer'))`.**
|
||
Reusing workspace roles is disqualified by construction: `memberships` has `UNIQUE(workspace_id,user_id)` (`001_tenants.sql:30`), i.e. exactly one role per user per workspace, and the seed's `Editor`/`Approver` are workspace-wide — a group-scoped Approver would approve everything. A separate `group_member_roles` table buys multi-role-per-group, which we explicitly do not want (editor+reviewer in one group = the same person edits and approves; that is the conflict four-eyes exists to prevent) at the cost of a second cascade path, a second epoch trigger, and a join in every eligibility check. The column reuses the table `groups.service.ts` already owns, and — decisively — `trg_group_member_bump_epoch` (`008_groups.sql:79-80`) fires on `UPDATE`, so a role change already invalidates every live session; the service just has to return the caller's replacement token exactly like `addMember`/`removeMember` do today (`groups.service.ts:168-200`). *Future work:* if per-group role delegation ("reviewer for text, not for media") ever lands, promote to a table then; the CHECK constraint is the only thing to widen.
|
||
|
||
**Q2 — Snapshot model: full content copies per version vs content-hash + immutable version rows?**
|
||
**Decision: immutable version rows that contain the full post AND a stored generated hash — `post_versions(post jsonb, content_hash text GENERATED ALWAYS AS (encode(digest(post::text,'sha256'),'hex')) STORED)`.**
|
||
These are usually presented as opposites; they shouldn't be. A hash alone can't be rendered or diffed, so the reviewer's "approve exactly what you see" surface (R4) would need to reconstruct content from somewhere — the moving target we're eliminating. Full copies make the version row the single source for preview, diff, and publication, and posts are tiny (`drafts.post jsonb`, `006_publishing.sql:6`), so copy cost is irrelevant. Making the hash a *generated* column (pgcrypto is already loaded, `001:3`, and `digest` is what `audit_hash` uses, `001:74-83`) means the hash can never disagree with the content — no TS-side canonicalization bugs, no `as any` json wrangling. Rows are append-only via a `BEFORE UPDATE` rejection trigger; `UNIQUE(draft_id, version)` gives stable version numbers for the UI ("v3 of 4"). *Future work:* content-addressed dedup (`UNIQUE(workspace_id, content_hash)`) if cross-post reuse ever matters.
|
||
|
||
**Q3 — Where does quorum config live and what's the precedence?**
|
||
**Decision: workspace default in a new 1:1 `workspace_review_settings` table (`approvals_required int NOT NULL DEFAULT 1 CHECK (… BETWEEN 1 AND 5)`), overridable per group by a nullable `groups.review_approvals_required`; precedence at **submit** time is `groups.review_approvals_required ?? workspace_review_settings.approvals_required ?? 1`, and the resolved value is **snapshotted onto the review cycle** (`review_cycles.required_approvals`).**
|
||
The snapshot is the load-bearing part: a quorum rule that can change under an open review either silently promotes a post past a threshold the admin just raised, or strands it — both feel like tampering to reviewers. Snapshotting makes each cycle a closed contract ("2 approvals of version 3") and makes config changes purely forward-looking. Both setters are admin-only via `review.manage` (granted to Owner/Admin — matching `workspace.members.manage` holders in `002:7, :23`, satisfying R3's "who sets it"). A separate settings table (rather than a column on `workspaces`) keeps the migration additive and lets RLS/policy/grants copy the standard shape; `groups` already has an `ALTER` in this migration anyway. *Future work:* per-connection quorum (spec mentions it) is deliberately out — drafts target one connection but reviews are editorial, not per-channel.
|
||
|
||
**Q4 — Chat transport: WS vs SSE vs polling?**
|
||
**Decision: HTTP polling of a cursor endpoint, `GET /publishing/drafts/:id/messages?after=<ord>`, 4s while a detail view is open; queue lists refresh on action or every 15s.**
|
||
Cost/complexity first: WS requires building the entire missing transport — `realtime/gateway.ts` is an authorization port with no socket server, no topic registry, no client (`gateway.ts:1-45`) — plus heartbeat/reconnect, plus an epoch-aware re-auth dance (any group membership change anywhere in the workspace invalidates the token the socket authenticated with, `008:79-80`, so sockets need mid-flight re-authentication or mass disconnect-reconnect). SSE is cheaper but `EventSource` cannot set an `Authorization` header, and this app authenticates every request with a Bearer token from localStorage (`api-client.ts:57-59`) — we'd have to mint query-param or cookie tokens, a new credential surface. Polling reuses `RbacGuard` + tenant RLS + the epoch check verbatim on every request, so revocation latency equals poll interval, and review-thread volume (a handful of messages per post) makes the cost negligible. Ordering is guaranteed structurally, not by transport: `ord` is assigned per-thread under a row lock on the draft (`UNIQUE(draft_id, ord)` backstop), so the cursor is a total order regardless of transport. *Future work:* the WS gateway should reuse `authorizeSubscription` plus the identical `after=<ord>` query — the data model was chosen so transport can swap without schema change.
|
||
|
||
**Q5 — Deny semantics: 1-deny-returns-to-author vs majority-deny vs configurable; what happens to other cast approvals?**
|
||
**Decision: single deny (reason mandatory) is a veto — cycle closes `denied`, draft → `needs_changes`. Other cast approvals are preserved immutably as history but belong to the closed cycle, so they count toward nothing on resubmit.**
|
||
A reviewer empowered to approve is empowered to block; anything weaker (majority-deny) lets a post publish over an explicit, reasoned objection — the exact failure four-eyes exists to prevent — and configurable deny modes multiply the state space for no safety gain. Preserving rather than deleting sibling approvals matters for the record ("two had approved, one objected, here's why") and costs nothing because quorum is always evaluated *within a cycle*: `review_votes.cycle_id` scopes every tally, so "revoking" approvals is unnecessary — the next cycle simply starts at zero. The deny reason is mandatory at the API boundary (`400` if blank — same `requireString` semantics as `http/validation.ts:23-28`) and lands in both the thread (system event, R5) and the audit chain (R8). *Future work:* a group-level `deny_threshold` config if some workspace ever wants consensus-style review.
|
||
|
||
**Q6 — Re-review after edit: full reset vs dismiss-only-changed-scope?**
|
||
**Decision: full reset. Any content mutation after submission closes the cycle (`superseded`) and requires a new version, a new cycle, and a fresh quorum.**
|
||
Partial dismissal requires knowing *which part* of a post each approval covered; there is no field-level approval scope in this system (nor in Hootsuite/Sprout per the research digest — both lock the whole post). Full reset is the only rule that makes "approve a frozen snapshot" structurally true: there is never a moment where `drafts.post` is mutable and an approval is live, which means the reviewer's diff view can be trusted without checking timestamps. It's also the cheapest to reason about under concurrency (one immutable version per cycle; no merge of partial validity). The cost — re-getting N approvals after a typo fix — is real, so the UI shows the version diff in the re-approval request ("v3→v4: 1 line changed") to make re-approval a two-second decision. *Future work:* "minor edit" carve-outs (e.g. schedule-only changes don't reset — which this design already grants: `publish_at` lives on the cycle, not the content, so rescheduling without quorum is impossible by the same rule; only content resets).
|
||
|
||
**Q7 — Scheduled-time race: quorum completes at T-1min — auto-publish or manual re-confirm?**
|
||
**Decision: auto-publish. The quorum-met handler creates the `scheduled_publications` row and starts the workflow iff `publish_at > now()`; if quorum lands after `publish_at`, the draft goes to `expired` and nothing is ever scheduled.**
|
||
The content approved is bit-identical to what publishes (frozen version), the approvals are valid, and the schedule was declared up front ("primed to go out", user req #1) — a re-confirm step at T-1min injects a human bottleneck precisely when humans are scarcest, and its failure mode is a missed post. The Temporal path already handles near-term starts (`startDelay = max(0, …)`, `publishing.service.ts:99-101`) and the durable-row sweeper (`publishing_due_schedules()`, `006:79-87`) already covers a missed workflow start, so no new machinery is needed for the happy path. The guard is the ordering rule: **schedules are only ever created for future times**, which makes "approved too late" equivalent to "not approved in time" — the single `expired` state covers both R1 cases (deadline passed, and stale-beyond-90-days). The race between a last-instant vote and the expiry sweeper is serialized on the draft row (both take `FOR UPDATE`; see §9). *Future work:* per-workspace "re-confirm window" config for teams that want the brake.
|
||
|
||
**Q8 — Author is also the only reviewer in a tiny group — deadlock handling?**
|
||
**Decision: refuse at the door, then a named exit. Submit runs a preflight — count of *currently eligible non-author reviewers* ≥ `required_approvals` — and returns `400 not enough eligible reviewers (0) for quorum (2)` instead of entering a state that can never resolve. If the pool shrinks mid-review (reviewer removed/demoted), the recount trigger moves the draft to `blocked`; Owner/Admin can resolve `blocked` via the override endpoint (mandatory reason, logged as override) or by adding reviewers, which auto-reopens the cycle.**
|
||
A deadlock you can enter is a bug; a deadlock you're told about at submit time is a configuration error with a clear message. The preflight is cheap (one count over memberships/group_members) and converts the worst failure mode into the best UX. `blocked` is a distinct state — not `needs_changes` — because "needs changes" tells the author to edit, when the actual problem is staffing; conflating them sends authors rewriting posts that were fine. The override path (R2: "Admin/Owner retain override power, logged as override") is the emergency brake for both deadlock and genuine urgency, and it is *not* a vote: it writes no `review_votes` row, it closes the cycle with `closed_reason='admin_override'` and a dedicated audit action, so quorum metrics can't be inflated by admin rescues. *Future work:* vacation delegation (a reviewer temporarily acting for another) — the group-role mechanism extends naturally.
|
||
|
||
**Q9 — Migration path for existing single-step approvals (`reviewer_user_id` backfill)?**
|
||
**Decision: migrate in `010` — every existing draft gets a `post_versions` v1 row from its current `post`; drafts with `reviewer_user_id` get a backfilled cycle (`required_approvals=1`) plus one synthetic vote (`approve`, or `deny` whose reason is recovered from the audit chain via `audit_log.diff->>'reason'` on the matching `draft.rejected` event, falling back to a migration note); terminal `rejected` rows become `needs_changes`. The `reviewer_user_id` column is then dead: `010` stops writing it, keeps it readable for one release, and a later `012` drops it once `draftFromRow` (`publishing.service.ts:315-317`) and `api-types.ts:70` stop referencing it.**
|
||
Dropping in the same migration that the running API still reads from would couple deploy order brittlely; the repo ships API and migration together (`migrate.ts` runs on boot) but the web bundle may lag by a session lifetime, so a deprecation window is the honest cost. Recovering deny reasons from `audit_log` is possible because rejects already write `{reason}` into the diff (`publishing.service.ts:197-205`) and the chain is queryable by `target_id` (`audit.query.ts:30-45`) — this keeps the mandatory-reason invariant true even for migrated rows (synthetic fallback text where history is missing). Historical intermediate versions are unrecoverable (`updateDraft` overwrote `post` in place, `:160-176`), so v1 = content at migration time; noted rather than hidden. *Future work:* none needed — `012` drop is the only follow-up.
|
||
|
||
**Q10 — Should reviewers see who else approved before them, or vote blind?**
|
||
**Decision: blind until the cycle closes. While a cycle is `open`, the API returns only `{approvals: n, required: m, mine: …}` and system events in the thread are identity-masked ("an approval was recorded · 2/3"); on close (`approved`/`denied`/`superseded`) the full vote ledger with identities is revealed. Holders of `review.override` (Owner/Admin) see identities live.**
|
||
Independence is the point of parallel review — the research digest flags rubber-stamping and anchoring as the failure modes of visible-first voting, and quorum metrics (approvals per reviewer, cycle time) only stay meaningful if early votes don't herd late ones. Masking is enforced server-side at the read endpoint (the `review_messages` row keeps the actor — the audit record must — but the serializer redacts `event_data.actorUserId` while `status='open'`), never trusted to the client. Admins see live because moderation is their job (spotting the rubber-stamper *during* review, not after). On close everything is revealed: the permanent record is fully transparent, which is what distinguishes this from secret balloting. *Future work:* per-group `blind_votes=false` toggle if a team insists on open voting.
|
||
|
||
---
|
||
|
||
## 3. Data model (deliverable 2) — `010_review_workflow.sql`
|
||
|
||
One file, one transaction (`migrate.ts:33-41`). Ordering inside the file matters and is annotated. Conventions copied everywhere: denormalized `workspace_id`, composite tenant-pinning FKs where a parent exists (`008:38-39` pattern), `reject_workspace_change` trigger, `ENABLE`+`FORCE ROW LEVEL SECURITY`, the exact `tenant_isolation` NULLIF policy (`001:176-179` / `006:58-66`), and `GRANT … TO sp_app` (`003`).
|
||
|
||
```sql
|
||
-- 010: group review & signoff — post versions, review cycles, quorum votes,
|
||
-- per-post chat, moderation flags. Extends 006 (publishing) and 008 (groups).
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (1) Group roles. Column, not a table (Q1): trg_group_member_bump_epoch
|
||
-- (008:79-80) already fires on UPDATE, so role changes invalidate sessions
|
||
-- and the service returns a replacement token (groups.service.ts:168-200).
|
||
-- ---------------------------------------------------------------------------
|
||
ALTER TABLE group_members
|
||
ADD COLUMN role text NOT NULL DEFAULT 'member'
|
||
CHECK (role IN ('member', 'editor', 'reviewer'));
|
||
|
||
COMMENT ON COLUMN group_members.role IS
|
||
'member = author/chat only; editor = edit group posts, cannot approve/publish; reviewer = approve/deny with reason';
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (2) Quorum config (Q3). 1:1 with workspace -> same RLS shape as every table.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE workspace_review_settings (
|
||
workspace_id uuid PRIMARY KEY REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
approvals_required int NOT NULL DEFAULT 1 CHECK (approvals_required BETWEEN 1 AND 5),
|
||
stale_after_days int NOT NULL DEFAULT 90 CHECK (stale_after_days BETWEEN 1 AND 3650),
|
||
updated_by uuid REFERENCES users(id),
|
||
updated_at timestamptz NOT NULL DEFAULT now()
|
||
);
|
||
|
||
ALTER TABLE groups
|
||
ADD COLUMN review_approvals_required int CHECK (review_approvals_required BETWEEN 1 AND 5);
|
||
-- NULL = inherit workspace default. Resolved once, at submit, onto the cycle.
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (3) Immutable post versions (Q2, R4). Append-only; hash is GENERATED so it
|
||
-- can never disagree with the content (pgcrypto digest, as in audit_hash
|
||
-- 001:74-83).
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE post_versions (
|
||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||
workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
draft_id uuid NOT NULL REFERENCES drafts(id) ON DELETE CASCADE,
|
||
version int NOT NULL CHECK (version >= 1),
|
||
post jsonb NOT NULL,
|
||
content_hash text GENERATED ALWAYS AS (encode(digest(post::text, 'sha256'), 'hex')) STORED,
|
||
created_by uuid NOT NULL REFERENCES users(id),
|
||
created_at timestamptz NOT NULL DEFAULT now(),
|
||
UNIQUE (draft_id, version)
|
||
);
|
||
|
||
CREATE INDEX post_versions_draft ON post_versions (draft_id, version DESC);
|
||
|
||
-- Append-only guard (new helper; workspace-delete cascades still work because
|
||
-- only UPDATE is blocked, DELETE flows through ON DELETE CASCADE).
|
||
CREATE FUNCTION reject_mutation() RETURNS trigger
|
||
LANGUAGE plpgsql AS $$
|
||
BEGIN
|
||
RAISE EXCEPTION '% rows are append-only', TG_TABLE_NAME USING ERRCODE = '23514';
|
||
END $$;
|
||
|
||
CREATE TRIGGER trg_post_versions_immutable BEFORE UPDATE ON post_versions
|
||
FOR EACH ROW EXECUTE FUNCTION reject_mutation();
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (4) Drafts: group scoping, version pointer, moderation flags (R7).
|
||
-- Composite FK pins the draft's group to this workspace (008 pattern);
|
||
-- NO ACTION (not SET NULL) because SET NULL would null workspace_id,
|
||
-- violating NOT NULL + trg_draft_ws_immutable (006:41-42). Consequence:
|
||
-- a group with posts cannot be deleted (deleteGroup gains a guard).
|
||
-- ---------------------------------------------------------------------------
|
||
ALTER TABLE drafts
|
||
ADD COLUMN group_id uuid,
|
||
ADD COLUMN current_version_id uuid REFERENCES post_versions(id),
|
||
ADD COLUMN hidden_at timestamptz,
|
||
ADD COLUMN hidden_by uuid REFERENCES users(id),
|
||
ADD COLUMN deleted_at timestamptz,
|
||
ADD COLUMN deleted_by uuid REFERENCES users(id),
|
||
ADD CONSTRAINT drafts_group_fk
|
||
FOREIGN KEY (group_id, workspace_id) REFERENCES groups(id, workspace_id);
|
||
|
||
CREATE INDEX drafts_queue ON drafts (workspace_id, state, updated_at DESC)
|
||
WHERE deleted_at IS NULL;
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (5) Review cycles: one per (draft, submission). Pins the version and the
|
||
-- quorum snapshot (Q3). At most one open cycle per draft.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE review_cycles (
|
||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||
workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
draft_id uuid NOT NULL REFERENCES drafts(id) ON DELETE CASCADE,
|
||
version_id uuid NOT NULL REFERENCES post_versions(id),
|
||
cycle int NOT NULL CHECK (cycle >= 1),
|
||
required_approvals int NOT NULL CHECK (required_approvals BETWEEN 1 AND 5),
|
||
status text NOT NULL DEFAULT 'open'
|
||
CHECK (status IN ('open', 'approved', 'denied', 'superseded', 'expired', 'blocked')),
|
||
publish_at timestamptz, -- "primed to go out": intent declared at submit
|
||
connection_id uuid REFERENCES connections(id),
|
||
opened_by uuid NOT NULL REFERENCES users(id),
|
||
closed_at timestamptz,
|
||
closed_reason text, -- 'quorum' | 'deny' | 'superseded_by_edit' | 'expired' | 'pool_below_quorum' | 'admin_override'
|
||
UNIQUE (draft_id, cycle)
|
||
);
|
||
|
||
CREATE UNIQUE INDEX review_cycles_one_open ON review_cycles (draft_id) WHERE status = 'open';
|
||
CREATE INDEX review_cycles_open_by_age ON review_cycles (workspace_id, created_at) -- stale sweep
|
||
WHERE status = 'open';
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (6) Votes (Q5, R6 in spec's R3). Immutable (one live vote per reviewer per
|
||
-- cycle, no vote changes). Deny reason mandatory, approve reason optional —
|
||
-- expressed as a CHECK so the DB enforces it too, not just the DTO parser.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE review_votes (
|
||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||
workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
cycle_id uuid NOT NULL REFERENCES review_cycles(id) ON DELETE CASCADE,
|
||
draft_id uuid NOT NULL REFERENCES drafts(id) ON DELETE CASCADE, -- denormalized for queue queries
|
||
voter_user_id uuid NOT NULL REFERENCES users(id),
|
||
vote text NOT NULL CHECK (vote IN ('approve', 'deny')),
|
||
reason text CHECK (vote = 'approve' OR btrim(reason) <> ''),
|
||
snapshot_hash text NOT NULL, -- copy of the version hash the voter saw
|
||
created_at timestamptz NOT NULL DEFAULT now(),
|
||
UNIQUE (cycle_id, voter_user_id) -- one live vote per reviewer per submission
|
||
);
|
||
|
||
CREATE INDEX review_votes_cycle ON review_votes (cycle_id, vote);
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (7) Review chat (R5): messages + system events in one append-only thread.
|
||
-- `ord` is per-thread, assigned under the draft row lock -> total order,
|
||
-- transport-agnostic cursor (Q4).
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE review_messages (
|
||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||
workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
draft_id uuid NOT NULL REFERENCES drafts(id) ON DELETE CASCADE,
|
||
ord int NOT NULL,
|
||
kind text NOT NULL CHECK (kind IN ('message', 'system')),
|
||
author_user_id uuid REFERENCES users(id), -- NULL for system events
|
||
body text NOT NULL CHECK (btrim(body) <> ''),
|
||
event text, -- system only: submitted|vote|state|…
|
||
event_data jsonb NOT NULL DEFAULT '{}',
|
||
created_at timestamptz NOT NULL DEFAULT now(),
|
||
UNIQUE (draft_id, ord)
|
||
);
|
||
|
||
CREATE INDEX review_messages_thread ON review_messages (draft_id, ord DESC);
|
||
|
||
CREATE TRIGGER trg_review_messages_immutable BEFORE UPDATE ON review_messages
|
||
FOR EACH ROW EXECUTE FUNCTION reject_mutation();
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (8) Notification hook (R9): transactional outbox; consumers come later.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TABLE review_notifications (
|
||
id uuid PRIMARY KEY DEFAULT gen_random_uuid(),
|
||
workspace_id uuid NOT NULL REFERENCES workspaces(id) ON DELETE CASCADE,
|
||
draft_id uuid NOT NULL REFERENCES drafts(id) ON DELETE CASCADE,
|
||
user_id uuid NOT NULL REFERENCES users(id), -- intended recipient
|
||
kind text NOT NULL, -- 'review_requested'|'denied'|'quorum_met'|'expired'|'blocked'|'mentioned'
|
||
payload jsonb NOT NULL DEFAULT '{}',
|
||
consumed_at timestamptz,
|
||
created_at timestamptz NOT NULL DEFAULT now()
|
||
);
|
||
|
||
CREATE INDEX review_notifications_inbox ON review_notifications (workspace_id, user_id)
|
||
WHERE consumed_at IS NULL;
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (9) Backfill BEFORE touching the state CHECK (the old CHECK only allows the
|
||
-- six legacy values, so 'needs_changes' must be written after dropping it).
|
||
-- Order: drop legacy CHECK -> rewrite 'rejected' -> add new CHECK.
|
||
-- (Trg_draft_ws_immutable from 006:41-42 is unaffected: workspace_id
|
||
-- never changes here.)
|
||
-- ---------------------------------------------------------------------------
|
||
ALTER TABLE drafts DROP CONSTRAINT drafts_state_check;
|
||
|
||
UPDATE drafts SET state = 'needs_changes' WHERE state = 'rejected';
|
||
|
||
ALTER TABLE drafts ADD CONSTRAINT drafts_state_check
|
||
CHECK (state IN ('editing', 'in_review', 'needs_changes', 'approved',
|
||
'scheduled', 'published', 'expired', 'blocked'));
|
||
|
||
-- v1 for every draft: content as it exists at migration time (intermediate
|
||
-- versions were overwritten in place by updateDraft, publishing.service.ts:160-176).
|
||
INSERT INTO post_versions (workspace_id, draft_id, version, post, created_by, created_at)
|
||
SELECT d.workspace_id, d.id, 1, d.post, d.author_user_id, d.created_at
|
||
FROM drafts d;
|
||
|
||
UPDATE drafts d SET current_version_id = pv.id
|
||
FROM post_versions pv WHERE pv.draft_id = d.id AND pv.version = 1;
|
||
|
||
-- Legacy single-step approvals (Q9): one cycle + one synthetic vote each.
|
||
-- opened_by has no historical value; the author is the least-wrong actor and
|
||
-- closed_reason marks the row as migrated.
|
||
INSERT INTO review_cycles
|
||
(workspace_id, draft_id, version_id, cycle, required_approvals, status,
|
||
publish_at, connection_id, opened_by, closed_at, closed_reason)
|
||
SELECT d.workspace_id, d.id, pv.id, 1, 1,
|
||
CASE d.state
|
||
WHEN 'published' THEN 'approved'
|
||
WHEN 'scheduled' THEN 'approved'
|
||
WHEN 'approved' THEN 'approved'
|
||
WHEN 'needs_changes' THEN 'denied' -- migrated from 'rejected'
|
||
ELSE 'open' -- in_review: keeps working, one vote finishes it
|
||
END,
|
||
NULL, NULL, d.author_user_id,
|
||
CASE WHEN d.state IN ('published','scheduled','approved','needs_changes')
|
||
THEN d.updated_at END,
|
||
'migrated_from_single_step'
|
||
FROM drafts d
|
||
JOIN post_versions pv ON pv.draft_id = d.id AND pv.version = 1
|
||
WHERE d.reviewer_user_id IS NOT NULL OR d.state = 'in_review';
|
||
|
||
INSERT INTO review_votes
|
||
(workspace_id, cycle_id, draft_id, voter_user_id, vote, reason, snapshot_hash, created_at)
|
||
SELECT c.workspace_id, c.id, c.draft_id, d.reviewer_user_id,
|
||
CASE WHEN d.state = 'needs_changes' THEN 'deny' ELSE 'approve' END,
|
||
CASE WHEN d.state = 'needs_changes' THEN
|
||
COALESCE(a.reason, 'migrated from legacy rejection (no reason recorded)')
|
||
ELSE NULL END,
|
||
pv.content_hash, d.updated_at
|
||
FROM drafts d
|
||
JOIN review_cycles c ON c.draft_id = d.id AND c.cycle = 1
|
||
JOIN post_versions pv ON pv.id = c.version_id
|
||
LEFT JOIN LATERAL (
|
||
SELECT (diff ->> 'reason') AS reason
|
||
FROM audit_log
|
||
WHERE workspace_id = d.workspace_id AND target_id = d.id::text
|
||
AND action = 'draft.rejected'
|
||
ORDER BY seq DESC LIMIT 1
|
||
) a ON true
|
||
WHERE d.reviewer_user_id IS NOT NULL;
|
||
|
||
-- reviewer_user_id is now dead: 010 never writes it, 012 drops it
|
||
-- (publishing.service.ts:315-317 and api-types.ts:70 stop reading first).
|
||
COMMENT ON COLUMN drafts.reviewer_user_id IS
|
||
'DEPRECATED: superseded by review_votes in 010; dropped in 012';
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (10) Defense in depth (spec §3): the state machine lives in the service AND
|
||
-- in the DB. Same CAS shape as publishing.service.ts:283-288 stays valid;
|
||
-- the scheduler's scheduled->published write (publish.activity.ts:182)
|
||
-- is an allowed edge.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE FUNCTION drafts_assert_transition() RETURNS trigger
|
||
LANGUAGE plpgsql AS $$
|
||
BEGIN
|
||
IF OLD.state = NEW.state THEN RETURN NEW; END IF;
|
||
IF NOT (
|
||
(OLD.state = 'editing' AND NEW.state IN ('in_review'))
|
||
OR (OLD.state = 'in_review' AND NEW.state IN ('approved', 'scheduled', 'needs_changes',
|
||
'expired', 'blocked', 'editing'))
|
||
OR (OLD.state = 'needs_changes' AND NEW.state IN ('in_review'))
|
||
OR (OLD.state = 'approved' AND NEW.state IN ('scheduled', 'needs_changes'))
|
||
OR (OLD.state = 'scheduled' AND NEW.state IN ('approved', 'published', 'needs_changes'))
|
||
OR (OLD.state = 'expired' AND NEW.state IN ('in_review'))
|
||
OR (OLD.state = 'blocked' AND NEW.state IN ('in_review', 'needs_changes'))
|
||
) THEN
|
||
RAISE EXCEPTION 'illegal draft transition % -> %', OLD.state, NEW.state
|
||
USING ERRCODE = '23514';
|
||
END IF;
|
||
RETURN NEW;
|
||
END $$;
|
||
|
||
CREATE TRIGGER trg_drafts_legal_transition BEFORE UPDATE OF state ON drafts
|
||
FOR EACH ROW EXECUTE FUNCTION drafts_assert_transition();
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (11) Live-eligibility recount (R8): reviewer removed / role changed ->
|
||
-- re-tally open cycles of that group's drafts. Pool below quorum =>
|
||
-- blocked; pool restored => reopened. Runs as the mutating user inside
|
||
-- the tenant tx, so RLS and audit_append's fn_assert_tenant both hold.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE FUNCTION review_recount_group() RETURNS trigger
|
||
LANGUAGE plpgsql AS $$
|
||
DECLARE r record; v_pool int; v_tally int;
|
||
BEGIN
|
||
WITH affected AS (
|
||
SELECT DISTINCT d.id AS draft_id
|
||
FROM drafts d
|
||
WHERE d.group_id = COALESCE(NEW.group_id, OLD.group_id)
|
||
AND d.state IN ('in_review', 'blocked')
|
||
)
|
||
SELECT c.id, c.draft_id, c.required_approvals, d.author_user_id
|
||
INTO r
|
||
FROM review_cycles c JOIN drafts d ON d.id = c.draft_id
|
||
WHERE c.status IN ('open', 'blocked')
|
||
AND EXISTS (SELECT 1 FROM affected a WHERE a.draft_id = c.draft_id)
|
||
LIMIT 1;
|
||
WHILE r IS NOT NULL LOOP
|
||
SELECT count(DISTINCT u.id) INTO v_pool -- currently-eligible non-author reviewers
|
||
FROM (SELECT m.user_id AS id
|
||
FROM memberships m JOIN role_permissions rp ON rp.role = m.role
|
||
WHERE m.workspace_id = (SELECT workspace_id FROM drafts WHERE id = r.draft_id)
|
||
AND m.status = 'active' AND rp.permission = 'review.vote'
|
||
UNION
|
||
SELECT gm.user_id FROM group_members gm
|
||
WHERE gm.group_id = (SELECT group_id FROM drafts WHERE id = r.draft_id)
|
||
AND gm.role = 'reviewer') u
|
||
WHERE u.id <> r.author_user_id;
|
||
|
||
SELECT count(*) INTO v_tally -- live tally: votes JOIN current eligibility
|
||
FROM review_votes v
|
||
WHERE v.cycle_id = r.id AND v.vote = 'approve'
|
||
AND EXISTS (SELECT 1 FROM memberships m JOIN role_permissions rp ON rp.role = m.role
|
||
WHERE m.user_id = v.voter_user_id AND m.status = 'active'
|
||
AND rp.permission = 'review.vote'
|
||
UNION ALL
|
||
SELECT 1 FROM group_members gm
|
||
WHERE gm.user_id = v.voter_user_id AND gm.role = 'reviewer')
|
||
AND v.voter_user_id <> r.author_user_id;
|
||
|
||
IF v_pool < r.required_approvals THEN
|
||
UPDATE review_cycles SET status = 'blocked', closed_reason = 'pool_below_quorum'
|
||
WHERE id = r.id AND status = 'open';
|
||
UPDATE drafts SET state = 'blocked', updated_at = now() WHERE id = r.draft_id AND state = 'in_review';
|
||
ELSE
|
||
UPDATE review_cycles SET status = 'open', closed_reason = NULL
|
||
WHERE id = r.id AND status = 'blocked'; -- pool restored: reopen
|
||
UPDATE drafts SET state = 'in_review', updated_at = now() WHERE id = r.draft_id AND state = 'blocked';
|
||
END IF;
|
||
|
||
SELECT NULL INTO r; -- loop replaced by set-based form in impl;
|
||
END LOOP; -- sketch shows intent, see §6.4
|
||
RETURN COALESCE(NEW, OLD);
|
||
END $$;
|
||
|
||
CREATE TRIGGER trg_group_role_recount
|
||
AFTER INSERT OR UPDATE OF role ON group_members
|
||
FOR EACH ROW EXECUTE FUNCTION review_recount_group();
|
||
-- DELETE is covered by the existing cascade path: removing a membership rows
|
||
-- deletes group_members (008:39 FK), whose own AFTER trigger runs the recount.
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (12) Expiry sweeper for the scheduler (R1). Same shape as
|
||
-- publishing_due_schedules (006:79-87): STABLE, SECURITY DEFINER, no
|
||
-- tenant context needed, ids only, revoked from PUBLIC, granted to sp_app.
|
||
-- Covers both deadline-passed and stale (stale_after_days).
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE FUNCTION review_due_expiries() RETURNS TABLE(draft_id uuid, cycle_id uuid, kind text)
|
||
LANGUAGE sql STABLE SECURITY DEFINER SET search_path = public AS $$
|
||
SELECT c.draft_id, c.id,
|
||
CASE WHEN c.publish_at IS NOT NULL AND c.publish_at <= now()
|
||
THEN 'deadline' ELSE 'stale' END
|
||
FROM review_cycles c
|
||
JOIN workspace_review_settings s ON s.workspace_id = c.workspace_id
|
||
WHERE c.status = 'open'
|
||
AND ( (c.publish_at IS NOT NULL AND c.publish_at <= now())
|
||
OR (c.publish_at IS NULL AND c.created_at < now() - (s.stale_after_days || ' days')::interval) )
|
||
$$;
|
||
|
||
REVOKE ALL ON FUNCTION review_due_expiries() FROM PUBLIC;
|
||
GRANT EXECUTE ON FUNCTION review_due_expiries() TO sp_app;
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (13) Tenancy: the standard block, verbatim from 006/008, per new table.
|
||
-- ---------------------------------------------------------------------------
|
||
CREATE TRIGGER trg_wrs_ws_immutable BEFORE UPDATE ON workspace_review_settings
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
CREATE TRIGGER trg_post_versions_ws_immutable BEFORE UPDATE ON post_versions
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
CREATE TRIGGER trg_review_cycles_ws_immutable BEFORE UPDATE ON review_cycles
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
CREATE TRIGGER trg_review_votes_ws_immutable BEFORE UPDATE ON review_votes
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
CREATE TRIGGER trg_review_messages_ws_immutable BEFORE UPDATE ON review_messages
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
CREATE TRIGGER trg_review_notifications_ws_immutable BEFORE UPDATE ON review_notifications
|
||
FOR EACH ROW EXECUTE FUNCTION reject_workspace_change();
|
||
|
||
ALTER TABLE workspace_review_settings ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE post_versions ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_cycles ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_votes ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_messages ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_notifications ENABLE ROW LEVEL SECURITY;
|
||
ALTER TABLE workspace_review_settings FORCE ROW LEVEL SECURITY;
|
||
ALTER TABLE post_versions FORCE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_cycles FORCE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_votes FORCE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_messages FORCE ROW LEVEL SECURITY;
|
||
ALTER TABLE review_notifications FORCE ROW LEVEL SECURITY;
|
||
|
||
CREATE POLICY tenant_isolation ON workspace_review_settings
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
CREATE POLICY tenant_isolation ON post_versions
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
CREATE POLICY tenant_isolation ON review_cycles
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
CREATE POLICY tenant_isolation ON review_votes
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
CREATE POLICY tenant_isolation ON review_messages
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
CREATE POLICY tenant_isolation ON review_notifications
|
||
USING (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid)
|
||
WITH CHECK (workspace_id = NULLIF(current_setting('app.workspace_id', true), '')::uuid);
|
||
|
||
GRANT SELECT, INSERT, UPDATE, DELETE ON workspace_review_settings, post_versions,
|
||
review_cycles, review_votes, review_messages, review_notifications TO sp_app;
|
||
|
||
-- ---------------------------------------------------------------------------
|
||
-- (14) Permission seeds — purely additive; no existing role loses anything
|
||
-- (R2: extend, don't break the 10-role contract).
|
||
-- ---------------------------------------------------------------------------
|
||
INSERT INTO role_permissions (role, permission)
|
||
SELECT r, p FROM unnest(ARRAY['Owner', 'Admin']) AS r,
|
||
unnest(ARRAY['review.vote', 'review.manage', 'review.override',
|
||
'moderation.hide', 'moderation.delete']) AS p
|
||
ON CONFLICT DO NOTHING;
|
||
|
||
INSERT INTO role_permissions (role, permission)
|
||
SELECT 'Approver', 'review.vote' ON CONFLICT DO NOTHING;
|
||
```
|
||
|
||
### 3.1 Why these shapes and not others
|
||
|
||
- **`review_cycles` pins `version_id` and snapshots `required_approvals`, `publish_at`, `connection_id`.** The cycle is the unit of trust: "2 approvals of hash X by time T through connection C". Everything the quorum-met handler needs is on one row, so no config read can race the transition.
|
||
- **`review_votes.snapshot_hash`** duplicates `post_versions.content_hash` so each vote is a self-contained attestation — the audit chain (`audit_log.diff`) and the vote ledger independently prove what was approved, without a join.
|
||
- **`review_messages` merges human messages and system events** (R5 "system events rendered inline") instead of two tables: one cursor, one ordering, one render list; `kind` + nullable `author_user_id` + `event` distinguish them.
|
||
- **Moderation is flags, not states** (§1): `hidden_at`/`deleted_by` coexist with any machine state, the legal-transition trigger stays a pure function of state, and the scheduler needs no second transition after publish.
|
||
- **`drafts_queue` is a partial index** (`WHERE deleted_at IS NULL`) — soft-deleted rows are excluded from the hot path but retained for the audit record.
|
||
|
||
---
|
||
|
||
## 4. State machine (deliverable 3)
|
||
|
||
States after `010`: `editing · in_review · needs_changes · approved · scheduled · published · expired · blocked`.
|
||
`rejected` is gone (migrated to `needs_changes`, which is resubmittable — closing the "no resubmit path" gap, spec §2.1).
|
||
|
||
| # | From | To | Fired by | Guard / effect |
|
||
|---|------|----|----------|----------------|
|
||
| 1 | `editing` | `in_review` | author, group **editor**, or `approval.request` holder | `POST /drafts/:id/submit {publishAt?, connectionId?}`. Freezes a new `post_versions` row, opens cycle 1..n with snapshotted quorum. **Preflight:** eligible non-author reviewers ≥ required (else `400`). If `connectionId` given: RLS-scoped existence + group-scope check (the `publishing.service.ts:217-219` pattern). Audit `review.submitted`. |
|
||
| 2 | `in_review` | `needs_changes` | any eligible **reviewer**, deny vote | Reason mandatory (`400` if blank). Cycle → `denied`; sibling approvals preserved as history, count for nothing (Q5). System event + audit `review.denied`. |
|
||
| 3 | `in_review` | `needs_changes` | author/group editor, **edit during review** | New version row; cycle → `superseded` (`closed_reason='superseded_by_edit'`). Full reset (Q6). Audit `review.cycle_superseded`. |
|
||
| 4 | `in_review` | `approved` | quorum engine (N live approvals ≥ required), no `publish_at` | Cycle → `approved`; draft awaits manual schedule (existing `approved→scheduled` path unchanged). Audit `review.quorum_met`. |
|
||
| 5 | `in_review` | `scheduled` | quorum engine, `publish_at` **and** `publish_at > now()` | Creates `scheduled_publications` + starts workflow (`startScheduledPublish`, `publishing.service.ts:83-113`). Audit `review.quorum_met` + `publication.scheduled`. |
|
||
| 6 | `in_review` | `expired` | quorum engine with `publish_at <= now()`, **or** sweeper | Nothing is ever scheduled late (Q7). Audit `review.expired`. |
|
||
| 7 | `in_review` | `blocked` | recount trigger (pool < quorum) | Reviewer removed/demoted mid-review (Q8). Audit `review.blocked`. |
|
||
| 8 | `in_review` | `editing` | author withdraws | Cycle → `superseded`. Audit `review.withdrawn`. |
|
||
| 9 | `needs_changes` | `in_review` | author or group editor resubmits | New version + new cycle (fresh quorum). Optionally new `publishAt`/`connectionId`. This is the resubmit path that does not exist today. |
|
||
| 10 | `approved` | `scheduled` | `publication.schedule` holder | Existing `schedule()` unchanged (`publishing.service.ts:212-247`). |
|
||
| 11 | `approved` | `needs_changes` | author/editor edit after approval | Post-approval edit re-triggers the chain (spec §3 lock-after-approve). Cycle already closed; new cycle on resubmit. |
|
||
| 12 | `scheduled` | `approved` | `publication.schedule` holder | Existing `cancelSchedule()` unchanged. |
|
||
| 13 | `scheduled` | `needs_changes` | edit, or admin override-deny | Transactionally deletes the pending `scheduled_publications` row first (else `UNIQUE(draft_id)`, `006:24`, blocks re-scheduling). |
|
||
| 14 | `scheduled` | `published` | **scheduler only** (`sp_scheduler`) | Unchanged: `publish.activity.ts:182`. Archive = this state (§1). |
|
||
| 15 | `expired` | `in_review` | author re-primes | New cycle; must supply a future `publishAt` if it wants scheduling. R1's "re-prime after re-approval". |
|
||
| 16 | `blocked` | `in_review` | recount trigger (pool restored) | Adding/re-promoting a reviewer reopens automatically; no human step needed. |
|
||
| 17 | `blocked` | `needs_changes` | admin override-deny, or author withdraws | Override: mandatory reason, `review.override_denied` audit. |
|
||
| 18 | `blocked` | `approved`/`scheduled` | admin override-approve (`review.override`) | Emergency brake (Q8). Not a vote: no `review_votes` row; `closed_reason='admin_override'`. |
|
||
|
||
Who **cannot** fire things (structural, server-side):
|
||
|
||
- The **author can never vote** on their own post when `required_approvals ≥ 2` — `403 ForbiddenException("reviewers cannot vote on their own post")` from the service, *in addition to* eligibility being excluded from the tally SQL. At `required_approvals = 1` self-approval is permitted (legacy/self-service mode) and stamped `selfApproved: true` in the audit diff — see §6.3.
|
||
- Group **editors** cannot vote (their group role grants edit, never `reviewer`), cannot schedule (`publication.schedule` is not granted to them), and workspace role `Editor` already lacks `approval.grant` (`002:39-43`).
|
||
- Group **reviewers** cannot edit content (edit path checks author ∨ group-editor ∨ `draft.edit`).
|
||
- Nobody transitions into `published` except the scheduler; nobody transitions out of it (terminal; moderation is flags).
|
||
|
||
Every transition above is (a) a CAS `UPDATE … WHERE id=$1 AND state=$from` exactly like `transition()` (`publishing.service.ts:283-288`), **and** (b) legal per `trg_drafts_legal_transition` — the DB is the second enforcement layer even against direct SQL.
|
||
|
||
---
|
||
|
||
## 5. RBAC (deliverable 4)
|
||
|
||
### 5.1 New permissions (additive seeds only)
|
||
|
||
| Permission | Granted to | Meaning |
|
||
|---|---|---|
|
||
| `review.vote` | Owner, Admin, Approver | Workspace-level reviewer. Guard gate for the vote endpoint; the service then re-checks author-exclusion and group scope. |
|
||
| `review.manage` | Owner, Admin | Set workspace/group quorum, resolve `blocked`, view live vote identities. (Same holders as `workspace.members.manage`, `002:7/:23` — admin-only per R3.) |
|
||
| `review.override` | Owner, Admin | Force approve/deny with mandatory reason; logged as override, never as a vote. |
|
||
| `moderation.hide` | Owner, Admin | Hide/unhide any post (R7, admin-only). |
|
||
| `moderation.delete` | Owner, Admin | Soft-delete any post (R7, admin-only). |
|
||
|
||
No existing role loses a permission; `approval.request`/`approval.grant` keep their seed rows (`002:8/:24/:35/:49`) — `approval.grant` becomes the *workspace-wide* form of review power (holders are eligible voters everywhere), which is exactly the "extend, don't break" reading of R2. The legacy `/approve` and `/reject` endpoints are superseded by `/votes` + `/override` and removed in WP-C (their guard permission `approval.grant` still exists for compatibility with any external caller during the deprecation window).
|
||
|
||
### 5.2 Group-role mechanism
|
||
|
||
`group_members.role` (§3 step 1). Effective capabilities for a draft with `group_id G`:
|
||
|
||
| Capability | Satisfied by |
|
||
|---|---|
|
||
| Author | any member of G (or any `draft.create` holder for ungrouped drafts) |
|
||
| Edit content | author ∨ `group_members.role='editor'` in G ∨ workspace `draft.edit` |
|
||
| Submit / resubmit | author ∨ group editor ∨ `approval.request` |
|
||
| Vote | (`review.vote` via workspace role) ∨ (`group_members.role='reviewer'` in G) — **and** ≠ author (at N≥2) |
|
||
| Schedule | `publication.schedule` (unchanged; group editors never get it) |
|
||
| Moderate | `moderation.hide`/`.delete` (Owner/Admin only) |
|
||
|
||
Guard-side checks are service functions on the tenant client, mirroring `userCanUseConnection` (`groups.service.ts:425-455`) — role read from the DB, never from the token:
|
||
|
||
```ts
|
||
// apps/api/src/review/review.eligibility.ts (sketch)
|
||
export interface VoteEligibility {
|
||
eligible: boolean;
|
||
reason?: "not_group_reviewer" | "author" | "not_in_review";
|
||
groupRole: "member" | "editor" | "reviewer" | null;
|
||
}
|
||
|
||
export async function voteEligibility(
|
||
client: PoolClient,
|
||
workspaceId: string,
|
||
userId: string,
|
||
draft: { id: string; groupId: string | null; authorUserId: string; state: DraftState },
|
||
requiredApprovals: number,
|
||
): Promise<VoteEligibility> {
|
||
const res = await client.query(
|
||
`SELECT
|
||
EXISTS (SELECT 1 FROM role_permissions rp
|
||
JOIN memberships m ON m.role = rp.role
|
||
AND m.workspace_id = $1 AND m.user_id = $2
|
||
AND m.status = 'active'
|
||
WHERE rp.permission = 'review.vote') AS workspace_reviewer,
|
||
(SELECT gm.role FROM group_members gm
|
||
WHERE gm.group_id = $3 AND gm.user_id = $2) AS group_role`,
|
||
[workspaceId, userId, draft.groupId],
|
||
);
|
||
const row = requiredRow(res.rows[0], "eligibility");
|
||
const byRole = row.group_role === "reviewer";
|
||
if (!byRole && row.workspace_reviewer !== true) {
|
||
return { eligible: false, reason: "not_group_reviewer", groupRole: null };
|
||
}
|
||
// Structural self-approval exclusion (four-eyes). At quorum 1 the legacy
|
||
// self-service mode is preserved (compose/page.tsx:128-131) and flagged.
|
||
if (userId === draft.authorUserId && requiredApprovals >= 2) {
|
||
return { eligible: false, reason: "author", groupRole: row.group_role };
|
||
}
|
||
if (draft.state !== "in_review") return { eligible: false, reason: "not_in_review", groupRole: row.group_role };
|
||
return { eligible: true, groupRole: row.group_role };
|
||
}
|
||
```
|
||
|
||
`requiredRow` follows `publishing.service.ts:297-300`. No `as any`; `group_role` is `null` for ungrouped drafts and the SQL handles `$3 = NULL` (no row → NULL).
|
||
|
||
### 5.3 Self-approval exclusion & admin override
|
||
|
||
- Self-approval: enforced in `voteEligibility` (above) **and** excluded from the tally SQL (§6.2) **and** impossible to satisfy at N≥2 even for Owner — three independent layers.
|
||
- Override: `POST /publishing/drafts/:id/override {decision: 'approve'|'deny', reason}` — `@RequirePermission("review.override")`, reason mandatory. Effects per §4 rows 17–18. Writes **no** `review_votes` row (overrides must not pollute quorum metrics), writes audit `review.override_granted`/`review.override_denied` with the reason and the actor, and a masked-then-revealed system event.
|
||
|
||
### 5.4 Epoch interplay
|
||
|
||
- `group_members.role` UPDATE fires `trg_group_member_bump_epoch` (`008:79-80`) → every session dies → `PATCH /groups/:id/members/:userId` returns `{token}` minted in-tx via `issueSessionInTenant` (`auth.service.ts:72-84`), same contract as `addMember`/`removeMember` (`groups.service.ts:168-200`).
|
||
- Group **deletion** must gain a guard in `deleteGroup`: `drafts_group_fk` is `NO ACTION`, so deleting a group with posts raises FK — surface it as `409 group still has posts; move or delete them first` instead of a 500.
|
||
- The recount trigger (§3 step 11) runs inside the same tx, so "reviewer demoted" and "cycle re-tallied" commit atomically — R8's "votes revoked/re-counted" is one transaction, not a eventual-consistency hope.
|
||
|
||
---
|
||
|
||
## 6. Quorum engine (deliverable 5)
|
||
|
||
### 6.1 Vote lifecycle
|
||
|
||
1. **One live vote per reviewer per submission**: `UNIQUE(cycle_id, voter_user_id)`; a second attempt returns `409 ConflictException("already voted in this cycle")`.
|
||
2. **Votes are immutable** (no vote changes — `review_votes` has no UPDATE path and `reject_mutation`-style immutability is the norm here). Justification: a vote attests a hash; a changed attestation rewrites history the hash-chained audit already recorded. A reviewer who reconsidered can comment; a deny closes the cycle anyway; a genuine misfire is an admin override. *Future work:* "retract my approval while cycle open" if teams demand it.
|
||
3. **Deny = veto** (Q5): closes the cycle `denied`, draft → `needs_changes`, reason mandatory end-to-end (DTO `requireString`, DB `CHECK (vote = 'approve' OR btrim(reason) <> '')`).
|
||
4. **Eligibility is re-checked live at every evaluation** (spec §3 four-eyes): the tally JOINs *current* membership/role state, so a removed reviewer's historical vote stops counting without being deleted.
|
||
|
||
### 6.2 Tally (live, cycle-scoped, author-excluded)
|
||
|
||
```sql
|
||
-- inside castVote(), same tx, after inserting the vote
|
||
WITH eligible AS (
|
||
SELECT m.user_id AS id
|
||
FROM memberships m JOIN role_permissions rp ON rp.role = m.role
|
||
WHERE m.workspace_id = $1 AND m.status = 'active' AND rp.permission = 'review.vote'
|
||
UNION
|
||
SELECT gm.user_id FROM group_members gm
|
||
WHERE gm.group_id = $2 AND gm.role = 'reviewer'
|
||
)
|
||
SELECT count(*)::int AS approvals
|
||
FROM review_votes v JOIN eligible e ON e.id = v.voter_user_id
|
||
WHERE v.cycle_id = $3 AND v.vote = 'approve'
|
||
AND ($4 < 2 OR v.voter_user_id <> $5) -- author exclusion applies only at quorum >= 2
|
||
```
|
||
|
||
`$4` = `required_approvals` snapshot, `$5` = author. At quorum 1 the author's own (permitted, flagged) vote counts — that is today's behavior, preserved.
|
||
|
||
### 6.3 `castVote` service sketch (repo style: strict TS, DI like `PUBLISHING_DEPENDENCIES`)
|
||
|
||
```ts
|
||
export interface CastVoteInput {
|
||
vote: "approve" | "deny";
|
||
reason?: string;
|
||
}
|
||
|
||
export interface CastVoteResult {
|
||
draftState: DraftState;
|
||
cycleStatus: ReviewCycleStatus;
|
||
tally: { approvals: number; required: number };
|
||
}
|
||
|
||
async castVote(workspaceId: string, actor: Actor, draftId: string, input: CastVoteInput): Promise<CastVoteResult> {
|
||
if (input.vote === "deny" && (input.reason ?? "").trim() === "") {
|
||
throw new BadRequestException("deny requires a non-empty reason");
|
||
}
|
||
return withTenant(workspaceId, async (client) => {
|
||
await client.query("SELECT id FROM drafts WHERE id = $1 FOR UPDATE", [draftId]); // serialize vs sweeper/other voters
|
||
const draft = await lockDraft(client, draftId); // 404 / RLS-scoped
|
||
const cycle = await openCycle(client, draftId); // 409 if not open
|
||
const eligibility = await voteEligibility(client, workspaceId, actor.userId, draft, cycle.requiredApprovals);
|
||
if (!eligibility.eligible) {
|
||
if (eligibility.reason === "author") throw new ForbiddenException("reviewers cannot vote on their own post");
|
||
throw new ForbiddenException("you are not a reviewer for this post");
|
||
}
|
||
const version = await versionRow(client, cycle.versionId);
|
||
try {
|
||
await client.query(
|
||
`INSERT INTO review_votes (workspace_id, cycle_id, draft_id, voter_user_id, vote, reason, snapshot_hash)
|
||
VALUES ($1, $2, $3, $4, $5, $6, $7)`,
|
||
[workspaceId, cycle.id, draftId, actor.userId, input.vote, input.reason ?? null, version.contentHash],
|
||
);
|
||
} catch (err) {
|
||
if (pgCode(err) === UNIQUE_VIOLATION) throw new ConflictException("already voted in this cycle");
|
||
throw err;
|
||
}
|
||
const tally = await liveTally(client, workspaceId, draft.groupId, cycle.id, cycle.requiredApprovals, draft.authorUserId);
|
||
|
||
if (input.vote === "deny") {
|
||
await this.closeCycle(client, cycle, "denied", "deny");
|
||
await this.transitionDraft(client, draftId, "in_review", "needs_changes");
|
||
} else if (tally.approvals >= cycle.requiredApprovals) {
|
||
await this.meetQuorum(client, draft, cycle, tally);
|
||
}
|
||
// Same-tx audit + system message (groups.service.ts:100-110 pattern): a vote
|
||
// can never land without its audit entry.
|
||
await audit(client, workspaceId, {
|
||
actorUserId: actor.userId, actorRole: actor.role,
|
||
action: input.vote === "deny" ? "review.denied" : "review.vote_cast",
|
||
targetType: "draft", targetId: draftId,
|
||
diff: { cycle: cycle.cycle, vote: input.vote, ...(input.reason === undefined ? {} : { reason: input.reason }),
|
||
snapshotHash: version.contentHash, approvals: tally.approvals, required: cycle.requiredApprovals,
|
||
...(actor.userId === draft.authorUserId ? { selfApproved: true } : {}) },
|
||
});
|
||
await systemMessage(client, workspaceId, draftId, "vote",
|
||
{ masked: true, vote: input.vote, approvals: tally.approvals, required: cycle.requiredApprovals });
|
||
return { draftState: await draftState(client, draftId), cycleStatus: await cycleStatus(client, cycle.id), tally };
|
||
});
|
||
}
|
||
```
|
||
|
||
### 6.4 `meetQuorum` — the race-condition core (Q7)
|
||
|
||
```ts
|
||
private async meetQuorum(client: PoolClient, draft: DraftRow, cycle: CycleRow, tally: Tally): Promise<void> {
|
||
if (draft.hiddenAt !== null || draft.deletedAt !== null) {
|
||
await this.closeCycle(client, cycle, "blocked", "hidden_at_quorum");
|
||
await this.transitionDraft(client, draft.id, "in_review", "blocked");
|
||
return; // hidden posts never publish; an admin resolves via override/delete
|
||
}
|
||
if (cycle.publishAt !== null) {
|
||
if (cycle.publishAt.getTime() <= Date.now()) {
|
||
await this.closeCycle(client, cycle, "expired", "deadline"); // too late: never schedule late
|
||
await this.transitionDraft(client, draft.id, "in_review", "expired");
|
||
return;
|
||
}
|
||
const scheduled = await this.insertSchedule(client, workspaceIdOf(draft), draft.id, cycle.connectionId, cycle.publishAt);
|
||
await this.closeCycle(client, cycle, "approved", "quorum");
|
||
await this.transitionDraft(client, draft.id, "in_review", "scheduled"); // edge 5 of §4
|
||
await this.workflowStarter.startScheduledPublish(scheduled.idempotencyKey, cycle.publishAt).catch(() => undefined);
|
||
return; // sweeper publishing_due_schedules() (006:79-87) re-starts if Temporal was down
|
||
}
|
||
await this.closeCycle(client, cycle, "approved", "quorum");
|
||
await this.transitionDraft(client, draft.id, "in_review", "approved"); // edge 4: manual schedule later
|
||
}
|
||
```
|
||
|
||
Race resolutions:
|
||
|
||
- **Two reviewers approve concurrently.** Both take `SELECT … FOR UPDATE` on the drafts row before touching the cycle; the second transaction blocks, then re-reads the cycle as already closed → its `openCycle` returns `409 cycle closed` *after* its vote insert… which would leak a vote onto a closed cycle. Fix (implementation note): the vote insert and the cycle re-read happen under the same row lock, and `closeCycle` is itself a CAS (`UPDATE review_cycles SET status=… WHERE id=$1 AND status='open'`); the loser's insert committed first in lock order, so the winner's tally **included** it — i.e. the "loser" is whoever didn't win the `closeCycle` CAS, and their vote is exactly what made quorum. The invariant to test (§12, test 4): exactly one `approved`/`scheduled` transition and at most one `scheduled_publications` row per draft (`UNIQUE(draft_id)`, `006:24`, is the backstop).
|
||
- **Quorum vote vs expiry sweeper at T±ε.** Both paths lock the draft row; whichever wins sets the state, the other's CAS (`WHERE state='in_review'`) matches zero rows and no-ops (the `throwTransitionError` pattern from `publishing.service.ts:294-302` turns it into a clean "already handled").
|
||
- **Quorum met at T-1min.** `publish_at > now()` → schedule created, `startDelay ≈ 60s` (`publishing.service.ts:99-101`), publishes on time. Auto, no re-confirm (Q7).
|
||
- **Quorum met at T+1min.** `expired` (above). Nothing publishes late, ever.
|
||
- **Sweeper shape.** `review_due_expiries()` (§3 step 12) + a scheduler activity `reviewExpire()` that mirrors `listDue`/`publishScheduled` (`publish.activity.ts:160-245`): look up workspace via a small SECURITY DEFINER helper (or return `workspace_id` from the sweeper fn), open `inTenant`, close cycle + transition + `audit_append(…, NULL, 'sp_scheduler', 'review.expired', …)` — the exact actor convention of `publication.published` (`publish.activity.ts:184-190`). Lazy double-check at quorum time (above) covers clock skew between API and scheduler.
|
||
|
||
### 6.5 Submit preflight (Q8)
|
||
|
||
```sql
|
||
-- count of currently eligible non-author reviewers for this draft's scope
|
||
WITH eligible AS ( /* same CTE as §6.2 */ )
|
||
SELECT count(*)::int FROM eligible WHERE id <> $author
|
||
```
|
||
If `< requiredApprovals` → `400 BadRequestException("not enough eligible reviewers (0) for quorum (2) — add reviewers to the group or lower its quorum")`. The draft stays `editing`; nothing half-created. (At `required_approvals = 1` no preflight — solo workspaces keep today's self-serve flow.)
|
||
|
||
---
|
||
|
||
## 7. Review chat (deliverable 6)
|
||
|
||
### 7.1 Data model
|
||
|
||
`review_messages` (§3 step 7): one append-only thread per draft, human messages (`kind='message'`, `author_user_id` set) and system events (`kind='system'`, `event` + `event_data`) interleaved on one `ord` sequence. Posting:
|
||
|
||
```ts
|
||
async postMessage(workspaceId: string, actor: Actor, draftId: string, body: string): Promise<ReviewMessage> {
|
||
const clean = body.trim();
|
||
if (clean === "") throw new BadRequestException("message must be a non-empty string");
|
||
return withTenant(workspaceId, async (client) => {
|
||
await client.query("SELECT id FROM drafts WHERE id = $1 FOR UPDATE", [draftId]); // ord assignment is serialized
|
||
const res = await client.query(
|
||
`INSERT INTO review_messages (workspace_id, draft_id, ord, kind, author_user_id, body)
|
||
SELECT $1, $2, coalesce(max(ord), 0) + 1, 'message', $3, $4
|
||
FROM review_messages WHERE draft_id = $2
|
||
RETURNING *`,
|
||
[workspaceId, draftId, actor.userId, clean],
|
||
);
|
||
return messageFromRow(requiredRow(res.rows[0], "review message")); // UNIQUE(draft_id, ord) is the backstop
|
||
});
|
||
}
|
||
```
|
||
|
||
Who can post: any group member of the draft's group, any workspace member for ungrouped drafts (gate: `draft.view` at the guard + membership check in service, mirroring `RequireGroupAccess`'s split, `rbac.guard.ts:64-74`). Chat is **not** audit-chained (R8 requires transitions and votes; messages are content, preserved immutably by `reject_mutation` — their integrity story is append-only + RLS, not the hash chain). Reasons are content, not secrets: `scrub()` (`audit.service.ts:11-17`) would not redact them anyway (no secret-key match).
|
||
|
||
### 7.2 System event types
|
||
|
||
| `event` | Emitted on | `event_data` (masked while cycle open, Q10) |
|
||
|---|---|---|
|
||
| `submitted` | cycle opened (v, cycle n, required, publishAt?) | `{version, cycle, required, publishAt}` |
|
||
| `vote` | vote cast | `{vote, approvals, required}` — actor identity added on cycle close |
|
||
| `denied` | cycle closed `denied` | `{reason, by}` (identity revealed at close — the author must see who/why) |
|
||
| `quorum_met` | cycle closed `approved` | `{approvals, required, scheduledAt?}` |
|
||
| `cycle_superseded` | edit during/after review | `{fromVersion, toVersion}` |
|
||
| `state` | any other draft transition (expired, blocked, scheduled, published, hidden, deleted) | `{from, to}` |
|
||
| `override` | admin override | `{decision, reason, by}` |
|
||
|
||
Read endpoint masks identity server-side while `status='open'` (Q10): the row keeps `event_data.actorUserId`, the serializer drops it unless the cycle is closed or the caller holds `review.override`.
|
||
|
||
### 7.3 Transport (Q4 decision recap)
|
||
|
||
`GET /publishing/drafts/:id/messages?after=<ord>&limit=100` — cursor pagination by `ord` (validate with `parseLimit`, `http/validation.ts:57-63`), polled at 4s while the detail pane is open; the queue polls at 15s or refetches on any local action. Every request passes `RbacGuard` (session → epoch → `draft.view`) and tenant RLS, so a reviewer removed mid-thread fails closed on the next poll with `401 authorization epoch stale` (`rbac.guard.ts:46-47`) and the client re-authenticates — no long-lived authorization to revoke. *Future work:* WS gateway reusing `authorizeSubscription` (`gateway.ts:12-45`) + a `review.<draftId>` topic check + the identical `after=<ord>` replay query.
|
||
|
||
---
|
||
|
||
## 8. UI plan (deliverable 7)
|
||
|
||
### 8.1 `/approvals` → the working-posts queue (replaces the stub, `approvals/page.tsx`)
|
||
|
||
Two-pane layout (the Inbox shape, `inbox/page.tsx:52-120`):
|
||
|
||
- **Left: queue list** — virtualized with the exact `column.tsx:26-34` config (`useVirtualizer`, `estimateSize: () => 110`, `overscan: 8`, `scrollToIndex` on selection). Row: state chip (`Badge`, tones below), author, first line of text, **quorum progress chip `2/3 ✓`** (live tally from `GET /publishing/queue`), relative deadline when `publish_at` is near.
|
||
- **Filters** (R6): `needs my review` (default; eligibility computed server-side per caller), `needs changes`, `expiring soon` (`publish_at < now()+24h`), `all`. Server-side filter params keep the virtualizer honest.
|
||
- **Right: post detail** — three stacked zones:
|
||
1. **Rendered preview** — reuse the compose preview card markup (`compose/page.tsx:244-262`) against the pinned version's `post`.
|
||
2. **Version diff** — `v(n-1) → v(n)` selector; reuse the audit page's diff viewer (audit page, 236 lines, has the component) over the two `post` jsonbs.
|
||
3. **Chat + vote panel** — Inbox bubble thread (`inbox/page.tsx:82-104`) + reply box (`:105-118`) with **Approve / Deny…** buttons inline (R5). Deny opens a mandatory-reason textarea; submit disabled while blank (mirrors `requireString` server-side). Buttons render only when the server says the caller is eligible (`canVote` flag on the queue/detail payload — never inferred client-side).
|
||
- Schedule strip: `publish_at`, connection label, "expiring in 3h" warning.
|
||
- **Keyboard model** (Streams pattern, `streams/page.tsx:56-70`, incl. the INPUT/TEXTAREA/contentEditable guard): `j/k` move, `Enter` open, `a` approve, `d` deny (focuses reason box), `c` focus chat, `e` toggle diff, `Esc` back to list.
|
||
|
||
### 8.2 `/archive`
|
||
|
||
Read-only list of `state='published'` (§1: archive *is* published), same virtualized list, chat history preserved and readable (record requirement, R7). Admin affordances gated on `can("moderation.hide")` / `can("moderation.delete")`: **Hide** (members stop seeing it; admins see a `hidden` danger Badge), **Delete** (soft; disappears for everyone, remains in audit). Moderation scope (R7 decision): **any state**, not just archive — an embarrassing draft should be removable before it publishes; hiding a `scheduled` post transactionally cancels the pending schedule first.
|
||
|
||
### 8.3 Compose changes (`compose/page.tsx`)
|
||
|
||
- **Group picker** when the author belongs to groups (`GET /groups/mine` exists, `groups.controller.ts:57-61`); ungrouped = workspace review.
|
||
- The current auto-approve chain (`compose/page.tsx:119-142`) becomes: create → **submit** (`{publishAt, connectionId}` = "primed"), stop. The self-approve shortcut survives only in the `required_approvals=1` workspace via the vote endpoint; the button label changes to "Submit for review (2 approvals)" using the resolved quorum so expectations are set before submit.
|
||
- `STATE_TONE` (`:35-42`) gains: `needs_changes: "warn"`, `expired: "danger"`, `blocked: "danger"`.
|
||
- "Recent drafts" sidebar (`:255-263`) links queue-ward; sidebar badge (`sidebar.tsx:13` already hints "2") wires to the `needs my review` count.
|
||
|
||
### 8.4 Client plumbing
|
||
|
||
- `api-types.ts`: `DraftState` widened (mirrors `publishing.service.ts:8`), plus `ReviewCycleSummary`, `ReviewVoteSummary`, `ReviewMessage`, `QueueItem`. `reviewerUserId` (`:70`) marked deprecated (dropped with the column in 012).
|
||
- `useApiResource` (`use-api.ts`) gains an optional `refetchIntervalMs` (non-breaking, default off) for the two poll rates; everything still flows through `api`/`apiFetch` so auth, 401 redirect, and offline fallback stay uniform (`api-client.ts:57-59, :100-106`).
|
||
- Offline mode: queue/detail ship fixture data like every other screen (`DataSource: "sample"`, `use-api.ts:16-20`).
|
||
|
||
---
|
||
|
||
## 9. Edge cases — the ugly ones (deliverable 8)
|
||
|
||
| # | Case | Concrete resolution |
|
||
|---|---|---|
|
||
| 1 | **Reviewer removed from group mid-review** | `group_members` DELETE cascades from `memberships` (FK, `008:39`) → `trg_group_role_recount` re-tallies: their historical vote stops counting (live-eligibility tally, §6.2); if pool < required → cycle `blocked`, draft `blocked` (same tx); the epoch bump (`008:79-80`) kills their session, so their in-flight client gets `401` on the next call — no stale vote can arrive after the removal commits. |
|
||
| 2 | **Reviewer demoted to `member`/`editor`** | Same as #1 via the `UPDATE OF role` branch; the demote endpoint returns `{token}` (epoch re-issue pattern). |
|
||
| 3 | **Reviewer added while `blocked`** | Recount sees pool ≥ required → cycle back to `open`, draft back to `in_review`, system event `state`. No human unblock step. |
|
||
| 4 | **Deadline race: quorum at T-1min** | Auto-publish (Q7): schedule created (`publish_at > now()`), `startDelay ≈ 60s`, sweeper backstops a missed Temporal start (`006:79-87`). |
|
||
| 5 | **Quorum at T+1min / never** | `expired` via the same `meetQuorum` branch or the `review_due_expiries()` sweep (covers both deadline and `stale_after_days` staleness). Author re-primes: `expired → in_review` with a new cycle and a fresh `publishAt`. |
|
||
| 6 | **Vote tx races the expiry sweeper** | Both lock the draft row (`FOR UPDATE`); loser's CAS `WHERE state='in_review'` matches zero rows and no-ops (`throwTransitionError` pattern, `publishing.service.ts:294-302`). Test 7 in §12. |
|
||
| 7 | **Deadlock: author is the only reviewer, quorum 2** | Submit preflight `400` with an actionable message (Q8); if it happens later anyway → `blocked` + admin `review.override` (mandatory reason) or add-a-reviewer. |
|
||
| 8 | **Epoch churn mid-vote** | Vote is one tx; the guard's epoch check (`rbac.guard.ts:45-47`) runs *before* it, so a membership change between guard and commit is bounded by: the tally re-checks eligibility *inside* the tx, and the recount trigger already re-tallied — worst case the vote lands and immediately doesn't count, which is the correct outcome, not corruption. |
|
||
| 9 | **Edit during review** | New version; cycle `superseded`; sibling votes preserved-but-not-counted; draft `needs_changes`; resubmit = new cycle, fresh quorum (Q6). Editors *can* edit in review — that's their job — the invariant is "no mutable content while approvals are live", not "no edits". |
|
||
| 10 | **Edit after scheduling** | `scheduled → needs_changes` transactionally deletes the pending `scheduled_publications` row first (else `UNIQUE(draft_id)`, `006:24`); the started Temporal workflow finds no pending row and its claim fails cleanly (`publish.activity.ts:137-158` "not pending"). |
|
||
| 11 | **Connection deleted between submit and quorum** | `meetQuorum`'s `insertSchedule` catches FK violation → cycle `blocked` (`closed_reason='connection_unavailable'`), audit `review.blocked`; admin overrides or author re-primes. (Same FK-bypasses-RLS caveat as `publishing.service.ts:217-219`: the *submit-time* check is an RLS-scoped read, the quorum-time failure is the backstop.) |
|
||
| 12 | **Hidden post reaches quorum** | `meetQuorum` refuses to schedule: cycle `blocked` (`hidden_at_quorum`); an admin unhides (recount does not auto-reopen — hiding is a moderation decision, so the exit is override or unhide+resubmit). |
|
||
| 13 | **Group deleted with in-flight posts** | `drafts_group_fk` is `NO ACTION` → FK error; `deleteGroup` surfaces `409 group still has posts`. (SET NULL was impossible: it would null `workspace_id`, violating NOT NULL + `trg_draft_ws_immutable`, `006:41-42`.) |
|
||
| 14 | **Author leaves the workspace mid-review** | `memberships` cascade removes their group rows → recount (pool unchanged unless they were also a reviewer); draft and cycle persist (`author_user_id` has no cascade, `006:6`); the queue shows the (still existing) user record; `needs_changes` posts by a gone author are exited via admin override-deny or an editor resubmits (group editors may resubmit). |
|
||
| 15 | **Duplicate/late vote on a closed cycle** | `openCycle` → `409 cycle closed`; the UI's vote panel is disabled by the server-computed `canVote` flag anyway. |
|
||
| 16 | **Two messages concurrently** | `ord` assigned under the draft row lock; `UNIQUE(draft_id, ord)` is the backstop; clients reconcile by re-pulling from `after=<lastOrd>`. |
|
||
| 17 | **Migration over existing data** | Backfills are plain INSERT/UPDATE inside the migration tx (`migrate.ts:33-41`); the state rewrite runs *after* dropping the legacy CHECK and *before* adding the new one (§3 step 9); `rejected` rows all become `needs_changes` so no row violates the new CHECK. |
|
||
|
||
---
|
||
|
||
## 10. Audit & security (deliverable 9)
|
||
|
||
### 10.1 Event list (all through `audit_append` — `001:85-111` — same-tx for decisions, per `groups.service.ts:100-110`)
|
||
|
||
| Action | Actor | When |
|
||
|---|---|---|
|
||
| `review.submitted` | submitter | cycle opened (diff: version, cycle, required, snapshotHash, publishAt?, connectionId?) |
|
||
| `review.vote_cast` | voter | approve vote (diff: vote, snapshotHash, approvals/required, `selfApproved?`) |
|
||
| `review.denied` | voter | deny vote (diff: reason — mandatory) |
|
||
| `review.quorum_met` | last voter | cycle closed approved (diff: approvals, scheduled?) |
|
||
| `review.cycle_superseded` | editor | edit during/after review (diff: from/to version hashes) |
|
||
| `review.withdrawn` | author | in_review → editing |
|
||
| `review.expired` | `sp_scheduler` (actor NULL) | sweep or late quorum |
|
||
| `review.blocked` / `review.reopened` | trigger (actor: mutating admin) or `sp_scheduler` | pool below/restored |
|
||
| `review.override_granted` / `review.override_denied` | Owner/Admin | override (reason mandatory; **never** a `review_votes` row) |
|
||
| `review.settings_changed` | Owner/Admin | quorum/staleness config (diff old→new) |
|
||
| `review.group_role_changed` | Owner/Admin | member→editor/reviewer (diff old→new) |
|
||
| `post.hidden` / `post.unhidden` / `post.deleted` | Owner/Admin | moderation (diff: state at time, reason) |
|
||
| `draft.created/updated/submitted`, `publication.scheduled/schedule_cancelled/published/failed` | existing | unchanged from `publishing.service.ts` / `publish.activity.ts:184-199` |
|
||
|
||
Every state transition and every vote is hash-chained (R8). The chain stays verifiable with the existing `audit_verify_chain` (`001:113-126`) — new events are just new `action` strings; no schema change to `audit_log`. `snapshotHash` in vote/submit diffs ties the chain to `post_versions.content_hash`, so tampering with either side is detectable.
|
||
|
||
### 10.2 Security notes
|
||
|
||
- **Tenancy**: every new table has FORCE RLS + the verbatim `tenant_isolation` NULLIF policy + `reject_workspace_change` + `sp_app` grants (§3 step 13). Cross-tenant reads return zero rows even for the app role (`003` rationale).
|
||
- **Enforcement order**: `RbacGuard` (session → epoch → permission) → service-level group-scope/eligibility checks on the tenant client (the `userCanUseConnection` precedent) → DB CHECKs/triggers as the last line. Three layers for the two structural rules that matter most: no self-approval at N≥2, no illegal transition.
|
||
- **Soft delete / hide**: flags only; reads filter `deleted_at IS NULL` in the service (RLS stays pure tenancy — a simple predicate per repo convention, `001:172-179`). Audit rows outlive the rows they describe (`target_id` is text, no FK).
|
||
- **Blind-vote masking is server-side** (§7.2): the client never receives identities it shouldn't, so it can't leak them.
|
||
- **No new transport = no new attack surface**: polling endpoints are ordinary guarded routes; cursors are validated (`parseLimit`); `ord` is an int, not a raw SQL fragment.
|
||
- **Reasons are not secrets** and intentionally survive in audit diffs (`scrub()` only matches secret-ish keys, `audit.service.ts:11-17`); visibility of the chain remains `audit.view`-gated (Approver/Analyst/Owner/Admin, `002`).
|
||
|
||
---
|
||
|
||
## 11. Phased implementation plan (deliverable 10)
|
||
|
||
Each WP ships behind the previous one's test gate; migrations are sequential (`010` here; `011` sweeper indexes if needed; `012` column drop).
|
||
|
||
| WP | Scope | Test gate |
|
||
|---|---|---|
|
||
| **WP-A: schema** | `010_review_workflow.sql` (all of §3 incl. backfill + transition trigger + seeds); `publishing.service.ts` state union widened; legacy `/approve`,`/reject` kept but routed through the new tables for `required=1`. | Migration applies on a seeded DB; tenancy integration test (mirror `db/tenancy.integration.test.ts`) proves zero cross-tenant rows on all six new tables; `audit_verify_chain` true after backfill; illegal-transition trigger rejects `published→in_review` via direct SQL. |
|
||
| **WP-B: roles & config** | `PATCH /groups/:id/members/:userId {role}` (+ `{token}`), `GET/PUT /workspace/review-settings`, `PATCH /groups/:id` quorum override, `deleteGroup` post-guard; `voteEligibility`. | Role change bumps epoch (session dies, response carries fresh token); config writes are `review.manage`-only (403 for Publisher); preflight rejects quorum-2 submit in a 1-reviewer group. |
|
||
| **WP-C: quorum engine** | `submit` (freeze+preflight+cycle), `POST /drafts/:id/votes`, `/override`, recount trigger hardening (set-based), `meetQuorum` + auto-schedule; same-tx audit + system messages. | Tests 1–5 of §12 green; concurrency test (two racing final votes) yields exactly one transition and ≤1 schedule row. |
|
||
| **WP-D: chat & read models** | `review_messages` endpoints (post + cursor read + masking), `GET /publishing/queue` filters, `GET /publishing/drafts/:id` detail (version, tally, canVote, schedule), `GET /publishing/archive`. | Masking test (open cycle hides identities from non-override holders, closed reveals); cursor pagination stable under concurrent posts; RLS on all reads. |
|
||
| **WP-E: web** | `/approvals` queue + detail (virtualizer, keyboard, vote panel, diff), `/archive` with moderation actions, Compose group picker + submit-only flow + tone map, `useApiResource` interval option, `api-types.ts`. | Keyboard model works with input-guard; vote buttons gated on server `canVote`; offline fixtures render; no `as any` (tsc strict). |
|
||
| **WP-F: sweeper & moderation** | `review_due_expiries()` + scheduler `reviewExpire` activity + `notifications` outbox writes on submit/deny/quorum/expiry/block; hide/unhide/soft-delete endpoints (schedule-cancel-first). | Tests 6–10 of §12 green; hidden scheduled post never publishes; outbox rows written in the same tx as their transition. |
|
||
| **WP-G (later)** | `012` drops `drafts.reviewer_user_id` once web stopped reading it. | Removal of dead code only. |
|
||
|
||
---
|
||
|
||
## 12. Test plan sketch (deliverable 11)
|
||
|
||
1. **Tenancy wall**: as `sp_app` with tenant A context, read `review_votes`, `post_versions`, `review_cycles`, `review_messages`, `review_notifications`, `workspace_review_settings` for a tenant-B id → zero rows; a B-context write is rejected by `WITH CHECK`. (Mirror `db/tenancy.integration.test.ts`.)
|
||
2. **Structural no-self-approval**: quorum-2 workspace, Owner authors and tries to vote → `403 reviewers cannot vote on their own post`; quorum-1 workspace, same action succeeds and the audit diff carries `selfApproved: true`.
|
||
3. **Deny semantics**: deny without reason → `400`; with reason → cycle `denied`, draft `needs_changes`, sibling approve-votes still present in `review_votes` but the next cycle's tally starts at 0; the reason appears in the thread event and the audit chain.
|
||
4. **Concurrent final votes**: two reviewers approve simultaneously (two txs) → exactly one `approved`/`scheduled` transition, one closed cycle, ≤1 `scheduled_publications` row (`UNIQUE(draft_id)` never violated), both votes persisted, chain verifiable.
|
||
5. **Edit-during-review reset**: editor edits an `in_review` draft → new `post_versions` row, old cycle `superseded`, draft `needs_changes`; resubmit opens cycle n+1 with `required` re-snapshotted (change workspace config in between and prove the old cycle's snapshot held).
|
||
6. **Reviewer removed mid-review**: remove the only remaining eligible reviewer of a quorum-2 cycle → same-tx `blocked` + recount; their old vote no longer counts (tally query with them excluded); re-add a reviewer → auto `in_review`; the removed user's session fails with `401` (stale epoch).
|
||
7. **Deadline race both ways**: (a) quorum at `publish_at-60s` → scheduled and published; (b) quorum at `publish_at+60s` → `expired`, and `publishing_due_schedules()` returns nothing for the draft (nothing was ever created); (c) sweeper and a last-instant vote race → one outcome, no double transition.
|
||
8. **Admin override**: override-approve a `blocked` draft with reason → `approved`/`scheduled` and **no** `review_votes` row; audit shows `review.override_granted`; override-deny on a `scheduled` draft cancels the pending schedule row in the same tx.
|
||
9. **Chain & snapshot integrity**: run a full lifecycle (create → submit → vote ×2 → schedule → publish → hide) then `audit_verify_chain(workspace)` = true; every transition/vote has an event; every vote's `snapshot_hash` equals the approved `post_versions.content_hash`.
|
||
10. **Immutability & DB-level state machine**: `UPDATE post_versions …`, `UPDATE review_messages …` → error (append-only); `UPDATE drafts SET workspace_id=…` → `23514` (existing trigger); direct-SQL `published → in_review` → `illegal draft transition` from `trg_drafts_legal_transition`, proving enforcement survives a service bug.
|
||
|
||
---
|
||
|
||
## 13. Requirements traceability (R1–R9)
|
||
|
||
- **R1** — states + resubmit + expired: §4 (rows 2/9/15 close the terminal-`rejected` gap; rows 6/15 + sweeper cover expiry); archived = `published` (§1).
|
||
- **R2** — group roles, editor/reviewer split, structural no-self-approval, admin override: §5.
|
||
- **R3** — quorum config location/precedence/who, parallel any-N, one immutable vote, 1-deny, auto-transition + auto-schedule: §3 step 2, §6.
|
||
- **R4** — frozen snapshots, hash in audit, reviewer sees rendered post + diff: §3 steps 3/5/6, §8.1.
|
||
- **R5** — per-post thread with system events, vote buttons in chat, transport decision: §7.
|
||
- **R6** — queue UI, quorum chips, filters, j/k: §8.1.
|
||
- **R7** — archive view, hide/delete admin-only with new permissions, soft-delete choice, scope = all states: §1, §3 step 4, §8.2, §9 edge case 12.
|
||
- **R8** — RbacGuard, FORCE RLS, `010`, chained events, epoch re-token + vote re-count: §3, §5.4, §10.
|
||
- **R9** — no external notifications built; outbox hook designed (§3 step 8, WP-F); no mobile/per-platform/AI scope anywhere.
|
||
|
||
---
|
||
|
||
## 14. Confidence & risks
|
||
|
||
**Least sure about:**
|
||
|
||
1. **The recount trigger as written.** `review_recount_group()` is sketched with a loop for readability; the implementation should be set-based (one UPDATE over affected open cycles) and must be load-tested against the epoch trigger firing in the same statement. The subtle risk is trigger ordering inside `groups` mutations: the recount runs *after* `bump_authz_epoch`'s row update but in the same tx, which is what we want, but cascaded deletes (workspace deletion) hit it too — the `blocked` write during a workspace teardown is wasted work at best. Prototype should validate: remove-a-reviewer → blocked, re-add → reopened, all inside the admin's original request.
|
||
2. **The `FOR UPDATE`-on-draft serialization point.** I've made the draft row the single mutex for votes, quorum evaluation, expiry, edits, and `ord` assignment. That's correct but coarse: a hot post with N reviewers serializes fully. If contention shows up, the fix is a dedicated advisory lock or moving the mutex to `review_cycles` — but only measure first; review volume is human-scale.
|
||
3. **Blind voting + system events interplay.** Masking identity in `event_data` server-side is straightforward, but the *denied* case reveals the denier to the author immediately (deliberate — the author needs the reason's context). If a workspace wants anonymous denials too, that's a product decision hiding in what is currently an implementation detail; flag it before WP-C.
|
||
4. **Quorum-1 self-approve carve-out.** Preserving the solo-workspace flow (`compose/page.tsx:128-131`) means the four-eyes guarantee is only real at `≥2`. I've labeled it (UI shows the resolved quorum at submit; audit stamps `selfApproved`), but a stricter reading of R2 would force `≥2` everywhere and break single-user workspaces outright. This is the decision most likely to get overturned in review — it's cheap to flip either way before WP-A.
|
||
5. **`publish_at`/`connection_id` living on the cycle.** It makes "primed to go out" atomic with submission, but it also means a deadline can expire a review that was otherwise progressing (by design, R1). The alternative — deadline as a soft warning until scheduling — contradicts the spec, so I didn't take it, but the UX copy around `expiring soon` matters more than it looks.
|
||
|
||
**What a prototype should validate first (in order):**
|
||
|
||
1. Migration `010` on a copy of production-shaped data: backfill row counts, `rejected→needs_changes` rewrite, chain still verifies, old API paths keep working against the new tables (WP-A gate).
|
||
2. The concurrency core: two racing final votes + a racing sweeper — one transition, one schedule row, no lost votes (§12 tests 4 and 7). Everything else in the design is single-transaction logic; this is the only genuinely concurrent part.
|
||
3. Epoch/recount interaction: demote a reviewer with an open cycle and watch session invalidation, re-tally, and `blocked` land atomically (§12 test 6).
|
||
4. Polling ergonomics: 4s cursor polls on a real thread — server load is a non-issue, but check the UI's perceived liveness and the 401→re-auth→resume path after a mid-thread membership change.
|
||
|
||
---
|
||
|
||
*End of proposal — `runs/wp5/proposal-glm.md`. Design only; no repo files were modified.*
|