feat(support): conversations + conversation_messages tables with workspace-membership RLS#28
Merged
Conversation
…RLS (US-066) Add a NEW table pair for the support-conversation surface (Epic E), deliberately separate from threads/messages. conversations carries the ADR-0002 workspace-membership boundary (EXISTS against workspace_membership on workspace_id + auth.uid(), membership presence only - role appears in no predicate); conversation_messages delegates to its parent conversation's membership, mirroring how messages delegates to threads. The leak-proof owner-only threads/messages predicate is left completely untouched - no kind discriminator, no policy edits - keeping the two trust models in separate tables (PRD Risk #3 / ADR-0004). - columns per the US-066 AC (incl. claimed_by/claimed_at handoff fields) - RLS enabled on both; SELECT/INSERT/UPDATE/DELETE all membership-gated - indexes: conversations(workspace_id, status), conversation_messages(conversation_id, created_at asc) - backend/test_us066_conversations_rls.py: cross-workspace zero-leak RLS test with same-workspace positive control; asserts no conversations policy references wm.role and that threads stays owner-only - AGENTS.md: record the two-trust-model invariant
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Retrieval eval — PR vs
|
| Mode | recall@5 | MRR | nDCG@5 |
|---|---|---|---|
| vector | 0.860 (±0.000) | 0.772 (±0.000) | 0.779 (±0.000) |
| keyword | 0.110 (±0.000) | 0.120 (±0.000) | 0.112 (±0.000) |
| hybrid | 0.860 (±0.000) | 0.759 (±0.000) | 0.769 (±0.000) |
Per-category recall@5
| Mode | single_chunk | multi_hop | adversarial | paraphrase |
|---|---|---|---|---|
| vector | 0.900 (±0.000) | 0.933 (±0.000) | 0.600 (±0.000) | 1.000 (±0.000) |
| keyword | 0.250 (±0.000) | 0.033 (±0.000) | 0.000 (±0.000) | 0.000 (±0.000) |
| hybrid | 0.900 (±0.000) | 0.933 (±0.000) | 0.600 (±0.000) | 1.000 (±0.000) |
Comment is updated in place on each push by .github/workflows/retrieval-eval.yml (US-035). Comment-only — never blocks the build.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer's goal was to implement story US-066: a pure database-migration change adding a new conversations and conversation_messages table pair governed by workspace-membership row-level security, as the foundation for the support-conversation surface (Epic E). They imposed a hard design constraint that this must be a brand-new table pair, not an extension of the existing owner-only threads/messages tables, which had to stay completely untouched (no kind discriminator and no policy edits to the leak-proof predicate). They specified exact columns for both tables, RLS enabled on both with conversations policies using the ADR-0002 membership EXISTS clause (with role appearing in no predicate) and child messages delegating to the parent conversation's workspace membership, two specific indexes, and an automated cross-workspace zero-leak test, all while keeping scope strictly to US-066 and not starting US-067+. After the work was committed, the developer ran the no-mistakes validation pipeline and, when it flagged foreign-key delete-behavior questions, decided that bot_user_id and claimed_by should use ON DELETE SET NULL while workspace_id stays with no cascade (matching the documents.workspace_id precedent), instructing the agent to apply that and continue. Following repeated validation-infrastructure crashes attributed to resource exhaustion rather than a code defect, the developer's final stated intent was to retry the no-mistakes validation run on the verified-green branch at commit 2ddad52 now that machine load had been reduced.
What Changed
20260623120000_init_conversations.sqlcreating a newconversations+conversation_messagestable pair (deliberately separate from the owner-onlythreads/messagespair, nokinddiscriminator). Both tables enable RLS with workspace-membership policies built on the ADR-0002EXISTSclause againstworkspace_membership(presence only —roleappears in no predicate), child messages delegating to the parent conversation's workspace; FKsbot_user_idandclaimed_byuseON DELETE SET NULLwhileworkspace_idstaysNO ACTION, plus a(workspace_id, status)agent-queue index and a(conversation_id, created_at)transcript index.backend/test_us066_conversations_rls.py, a cross-workspace zero-leak suite asserting members see only their own workspace's conversations and child messages through real PostgREST + RLS.AGENTS.mdandCLAUDE.md, and sync the Phase-2 PRD status to mark the US-066 migration as landed.Risk Assessment
✅ Low: A well-bounded, additive, thoroughly-documented migration whose RLS faithfully mirrors an existing leak-proof precedent and is pinned by an exact cross-workspace zero-leak test; the only findings are a data-integrity nit and a rare-path performance note.
Testing
Validated against the running local Supabase (DB :54322) that already has the target migration 2ddad52 applied. Baseline: confirmed the four conversation FK constraints and the documents.workspace_id precedent via pg_constraint introspection. Ran the project's own cross-workspace RLS test (9 exact assertions pass through the real PostgREST + RLS path, with a W2-member positive control proving the zeros are genuine). Wrote and ran a transaction-rolled-back FK demo showing bot_user_id/claimed_by SET NULL on user deletion (conversation preserved) and a blocked workspace delete (no cascade). Captured a psql schema snapshot confirming FK actions, RLS-enabled state, both indexes, role-free conversations policies, and untouched owner-only threads policies. No UI surface exists (pure DB migration), so evidence is CLI transcripts and a persisted-schema snapshot rather than screenshots. All checks pass; working tree clean and no DB residue.
Evidence: Cross-workspace zero-leak RLS test transcript (9 assertions)
conversations: 8 policy predicates, none reference role conversation_messages: every policy delegates to parent conversation RLS enabled on both conversations and conversation_messages both required indexes present threads policies remain owner-only (no workspace_membership branch) U1 list /conversations -> {C1} only (1 row) U1 direct read of C2 by id -> 0 rows (RLS-hidden) U1 sees C1's message, 0 of C2's (child delegation holds) positive control: U2 (W2 member) reads C2 -> the zeros are real OK: US-066 passed — 9 exact assertions; conversations + conversation_messages enforce the ADR-0002 membership boundary with zero cross-workspace leak and no role predicateEvidence: FK ON DELETE behavior demo output (second-commit decision)
STEP 1 delete bot user -> conversation SURVIVES, bot_user_id -> NULL ✓ STEP 2 delete claimed_by agent -> conversation SURVIVES, claimed_by -> NULL ✓ STEP 3 delete owning workspace -> BLOCKED by FK (no cascade) ✓ ForeignKeyViolationError: update or delete on table "workspaces" violates foreign key constraint "conversations_workspace_id_fkey" on table "conversations" OK: FK ON DELETE behavior matches commit 2ddad52 — bot_user_id & claimed_by SET NULL (conversation preserved), workspace_id no-cascade (delete restricted, mirrors documents.workspace_id).Evidence: Persisted-schema snapshot (FK actions, RLS, indexes, role-free + threads-untouched policies)
FK ON DELETE: bot_user_id=SET NULL, claimed_by=SET NULL, workspace_id=NO ACTION, conversation_id=CASCADE | documents.workspace_id=NO ACTION (precedent) | RLS enabled on both tables | both required indexes present | conversations policies: references_role=f for all | threads policies: has_ws_membership_branch=f for allEvidence: FK delete-behavior demo script
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
supabase/migrations/20260623120000_init_conversations.sql:34-status(line 34) andchannel(line 36) are free-text with no CHECK constraint, unlikerole(line 21 of the file / the message role column) and the priorworkspace_membership.rolewhich both pin closed enums via CHECK. Because the agent-queue index is(workspace_id, status)and the queue list filters onstatus, an application bug writing a typo'd or mis-cased value (e.g. 'Active', 'resolve' vs 'resolved') would silently drop a conversation out of every status-filtered query with no DB-level guard. Considercheck (status in ('active','escalated','resolved',...))and a channel CHECK, or confirm the value set is intentionally left open for the foundation migration.supabase/migrations/20260623120000_init_conversations.sql:32-bot_user_id(line 32) andclaimed_by(line 37) useON DELETE SET NULLbut neither column is indexed. Deleting a row fromauth.usersforces Postgres to sequentially scanconversationsto find and null-out matching rows. This is a rare admin/lifecycle operation on a table that is unlikely to be large, so the cost is minor - noting it to complete the ON-DELETE-behavior reasoning from commit 2ddad52, not as a merge blocker.✅ **Test** - passed
✅ No issues found.
DATABASE_URL=postgresql://postgres:postgres@127.0.0.1:54322/postgres python -m backend.test_us066_conversations_rls— 9 exact cross-workspace zero-leak assertions through real PostgREST + RLS (W1 member sees only C1, direct read of C2 returns 0 rows, child-message delegation, W2-member positive control)python fk_delete_behavior_demo.py— behavioral proof that deleting bot user / claimed_by agent SET NULL the FK columns while preserving the conversation, and deleting the owning workspace is blocked (NO ACTION), all rolled back to leave the shared DB untouchedpsql schema snapshot — FK ON DELETE actions (bot_user_id/claimed_by=SET NULL, workspace_id=NO ACTION matching documents.workspace_id, conversation_id=CASCADE), RLS enabled on both tables, both required indexes present, no conversations policy references wm.role, threads policies remain owner-only with no workspace_membership branchsupabase/migrations/20260623120000_init_conversations.sql:1- The new migration header and US-066's acceptance criteria cite "ADR-0004 + ADR-0008" as the design record (echoed in AGENTS.md's "PRD Risk Module 4 (US-016 + US-017): metadata extraction & filtered retrieval #3 / ADR-0004" reference), but no docs/adr/0004-.md or docs/adr/0008-.md files exist - only 0001, 0002, 0007 are present. This is a pre-existing pattern (ADR-0003/0005/0006 are likewise referenced without files), but this change adds new dangling references. Authoring the ADRs requires the design owner's rationale, which I cannot invent in a docs-sync pass.✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.