[Orgs] 2 Organization management UI + role-aware permission gating (stacked on #267) - #268
Open
amal66 wants to merge 35 commits into
Open
[Orgs] 2 Organization management UI + role-aware permission gating (stacked on #267)#268amal66 wants to merge 35 commits into
amal66 wants to merge 35 commits into
Conversation
Collaborator
Author
|
Went through the full diff — the core feature here is solid, but the PR bundles several independent changes worth calling out. Breakdown: 1. Backend pagination/search/sort (the stated scope)
2. Frontend lazy-load integration
3. Perf/schema work inside the migration that goes beyond pagination
4. State-management removal in
5. Shared
6. Drive-by logic fix in delete-selected (both pages)
Requested changes:
Happy to re-review quickly once that's done — #1 + #2 + the tests look good and I don't expect issues with the core approach. |
amal66
force-pushed
the
olp-pr/organizations-ui
branch
from
August 3, 2026 01:57
27208bc to
8cc1bf7
Compare
4 tasks
amal66
force-pushed
the
olp-pr/organizations-ui
branch
from
August 6, 2026 03:43
5157e02 to
3bc199e
Compare
amal66
marked this pull request as draft
August 10, 2026 16:19
amal66
force-pushed
the
olp-pr/organizations-ui
branch
from
August 16, 2026 20:56
3bc199e to
384c923
Compare
Collaborator
Author
Rebase round, 2026-08-16Rebased onto the updated #267 (itself on current
Two commits added on top:
Verification
|
amal66
force-pushed
the
olp-pr/organizations-ui
branch
3 times, most recently
from
August 18, 2026 13:19
ebddef7 to
35ab80c
Compare
|
|
Collaborator
Author
Live smoke evidence — 2026-08-18 rebaseRecorded end-to-end runs of the highest-risk RBAC behaviours against a full local stack (Next dev + backend + Supabase CLI stack, fresh browser contexts, two org users: an owner and a member). All five checks passed, and no request in any flow returned a 5xx. Each section below has an inline recording (GIF), key frames, and a link to the original video. Full asset set lives on
1 — Org lifecycle + last-owner denial (create org → add member by email → role Member↔Admin → leave as sole owner)
The leave attempt returns HTTP 409 (asserted on the DELETE response), the confirm popup closes, and the error renders inline in the org card:
2 — Overlap keeps editor (member is org viewer AND direct-shared on “Overlap Case”)Precondition verified in the DB row:
3 — Pure org viewer gating (org-only project + review, no direct share)Setup as owner (upload PDF → create “Reading Room Review”):
As the member (view-tier via org only): the project and review are visible, but Run, Documents (add-to-review) and Chat are all disabled, and the project’s upload control is greyed:
4 — Admin roster limits (owner row untouchable, no Owner grant)As an org admin: the owner’s row renders as a static pill with no role control, and the role menu for another member was asserted to contain no “Owner” option:
5 — Directory search with org rowsAs the member, project search and the assistant document-picker both return the org-shared rows; the run recorded zero 5xx responses:
Observations (not blockers, flagging for awareness)
Also re-verified on the rebased tips: backend 709/709 (incl. the 27 supabase-gated stack tests against a live stack), frontend suite green with coverage 99.82/97.19/100/100 vs floors 97/96/98/98. |
amal66
changed the title
Introduce an organizations tenant layer on top of the existing per-user model: every account gets an auto-provisioned personal org, orgs carry owner/admin/member RBAC via org_members, and teams group members inside an org. projects/documents/workflows/tabular_reviews gain a nullable org_id (ON DELETE SET NULL) so org membership becomes a third access branch alongside row ownership and shared_with emails — in the access helpers, the overview RPCs, and the org-aware /orgs REST module. Mechanical port of the organizations/RBAC feature from amal66/mike@main (b3166dd) onto the upstream layout. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CEguyEgXa9JjCciXCcVemC
…nches Give the three access branches a Drive-style role ladder instead of raw ok/isOwner flags: row owner → owner, shared_with email → editor, org owner/admin → manager, plain org member → viewer. A single capability matrix (lib/permissions.ts) maps roles to what routes may do — view, content.edit, docs.organize, structure.manage, members.manage, container.delete — and every project/document/review write route now declares the capability it needs instead of hand-rolling an owner check. This makes the ADR's 'org membership grants visibility, not ownership' promise real: plain org members are read-only (previously the org branch returned ok:true and most write routes gated on nothing beyond ok), and org owner/admins can curate content (manage folders, sharing, review structure) without being able to delete containers they don't own. Notable tightenings, all fail-closed: - folder rename/move/delete, doc-set/column edits on reviews, and clear-cells are manager+ (generalising the owner-only folder-delete gate that landed upstream in open-legal-products#193) - version pushes, edit resolution, chat, and review generation are editor+ (org viewers excluded) - project PATCH (metadata + sharing) is manager+, so org admins can manage without owning; project/review DELETE stays owner-only - GET /projects/:id and /people now go through checkProjectAccess (the roster previously 404'd for org members who could read the project) Detail responses expose access_role alongside is_owner so the client can render per-role affordances. can() is exhaustively unit-tested (role × capability), and route suites cover the new gates. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Org viewers could append messages to a colleague's chat — and burn LLM
credits doing it — because the existing-chat path only asked "can you
see this chat?", never "can you write to it?".
WHY THIS MATTERS
The organizations feature gives every plain org member visibility into
their org's projects: they map to the "viewer" project role
(backend/src/lib/access.ts, orgRoleToProjectRole). Visibility is the
point — but the capability matrix (backend/src/lib/permissions.ts) is
explicit that chatting is a WRITE:
capability | min role | covers
content.edit | editor | upload documents, push versions, CHAT, ...
Creating a chat already honored that: validateAccessibleProjectId gates
new project chats on can(projectRole, "content.edit"). But POST /chat
with an existing chat_id, and POST /chat/:chatId/generate-title, gated
only on getAccessibleChat — which returned the chat whenever
checkProjectAccess(...).ok was true. That is true for org viewers. So a
viewer could:
- append user messages into a colleague's project chat,
- trigger LLM generation (spending the owner's configured API keys),
- overwrite the chat's title via generate-title (an UPDATE on chats).
WHAT IS THE ACCESS-VS-CAPABILITY DISTINCTION
"Can you see this resource?" and "can you write to this resource?" are
different questions and must be answered by different checks:
- an ACCESS check resolves whether the caller has any standing at all
(owner / shared editor / org member) — it yields a role;
- a CAPABILITY check asks whether that role clears the bar for the
specific operation: can(role, "content.edit").
A route that stops after the access check silently grants its weakest
role the powers of its strongest. That is exactly the bug class here:
read-only endpoints (GET /chat/:chatId) and write endpoints (POST
/chat) shared one gate, so the gate had to be as permissive as the
reads — and the writes inherited that permissiveness.
HOW THE FIX WORKS
getAccessibleChat now returns the caller's ProjectRole along with the
chat, instead of flattening everything to "found / not found":
type ChatAccess =
| { ok: true; chat: AccessibleChat; projectRole: ProjectRole }
| { ok: false };
The chat owner maps to "owner"; for project chats everyone else
inherits their project role from checkProjectAccess. Chats without a
project_id remain reachable only by their owner (unchanged).
Each caller then declares the capability it needs:
- GET /chat/:chatId — read: any resolved role (project.view
semantics), so org viewers keep read access. Unchanged behavior.
- POST /chat with chat_id — write: rejects with 403 unless
can(projectRole, "content.edit"), mirroring the new-chat path.
- POST /chat/:chatId/generate-title — write (UPDATEs chats.title):
same 403 gate.
403 (not 404) is correct for viewers: they are allowed to know the chat
exists — they can read it — they just cannot modify it.
Note: routes/projectChat.ts does NOT have this hole — it already gates
the entire POST on content.edit before touching any chat row.
Tests (chat.routes.test.ts) add a table-aware supabase stub so the
caller's org role can be varied, and prove: viewer POST to an existing
chat → 403 with no LLM call; viewer generate-title → 403; chat owner
and org admin (manager) still stream successfully; admin generate-title
still works; viewer GET of the same chat still returns 200.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
With two or more owners in an org, an admin could demote an owner to
member (or remove them outright), because the membership mutations
checked the ACTOR's rank but never compared it to the TARGET's.
WHY THIS MATTERS
The role ladder is owner > admin > member. updateMember and
removeMember guarded three things:
- the actor can manage members at all (roleCanManage → owner/admin),
- "only an owner may GRANT the owner role" (no self-escalation),
- last-owner protection (never demote/remove the sole owner).
None of those look at who the target IS. So in an org with owners A and
B plus admin C, C could call updateMember(target=A, role="member"):
C passes roleCanManage, "member" is not "owner" so the grant check does
not fire, and B still counts as an owner so last-owner passes. Result:
an admin deposes an owner — and with both owners demoted one at a time,
an admin becomes the effective top of the org without ever holding the
owner role. That inverts the hierarchy the ladder is supposed to
encode.
WHAT IS AN ACTOR/TARGET RANK CHECK
Role-ladder authorization has two halves, and they are easy to conflate:
1. Does the ACTOR's role permit this KIND of operation?
("admins may manage members")
2. Does the ACTOR outrank-or-equal the TARGET of the operation?
("...but not members who outrank them")
Check (1) alone is enough for rank-neutral operations (creating a
team). Any operation aimed at another member also needs check (2),
otherwise every manager-tier role can act on the tier above it. The
existing "only an owner may grant owner" rule is the escalation half of
this idea; what was missing is the demotion half: only an owner may act
AGAINST an owner.
HOW THE FIX WORKS
Both mutations now fetch the target's role (they already did, for
last-owner counting) and add a rank guard before it:
if (targetRole === "owner" && actorRole !== "owner")
return { ok: false, kind: "forbidden" };
- updateMember: an admin demoting an owner → forbidden, regardless of
how many owners exist. Owner-on-owner changes still work.
- removeMember: an admin removing an owner → forbidden. An owner
leaving on their own is unaffected: self-leave implies
actorRole === targetRole === "owner", so the guard passes and the
existing last-owner protection still has the final say.
Since owner/admin/member is a three-rung ladder, this single condition
IS the full outrank-or-equal rule: owners outrank everyone, admins may
act on admins/members, and plain members cannot reach these functions
at all (roleCanManage already rejects them).
Tests (orgs.test.ts) seed a two-owner org so last-owner protection is
provably not what stops the attack: admin-demotes-owner → forbidden,
admin-removes-owner → forbidden, owner-demotes-owner still ok,
owner-self-leave with a second owner still ok, and the existing
last-owner cases stay green.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Adding an org or team member by email used the Supabase admin API's
listUsers({ perPage: 1000 }) and scanned the page in JavaScript. Any
user beyond the first 1000 accounts silently resolved to null, so the
route answered "No user with that email" for a user who exists.
WHY THIS MATTERS
List-and-scan lookups against an admin API are a scalability trap that
looks fine in every dev and demo environment:
- CORRECTNESS decays with growth. listUsers is paginated; a single
perPage: 1000 call is only "everyone" while the instance has fewer
than 1000 accounts. The day it crosses that line, membership adds
start failing for exactly the newest users — with a misleading 404
and no error anywhere in the logs.
- COST grows linearly. Even while it still works, resolving ONE email
means transferring up to 1000 user records and comparing each in
process, on every add-member call — O(n) network and CPU for what
an indexed table answers in O(log n).
- The admin auth API is a management surface, not a query engine. It
has no "find by email" filter here, which is the hint that
lookup-by-attribute belongs on a queryable table with an index.
The comment above the helper claimed it "mirrors the lookup pattern in
routes/projects.ts /people" — but that route actually uses the
user_profiles-based helpers in lib/userLookup.ts, not the admin API.
The codebase already had the right tool; this route just didn't use it.
WHAT THE INDEXED LOOKUP IS
lib/userLookup.ts maintains lookups over the user_profiles table, which
stores each user's normalized (lowercased, trimmed) email and is kept
in sync on auth events (syncProfileEmail). findProfileUserByEmail is a
single indexed query:
const { data } = await db
.from("user_profiles")
.select("user_id, email, display_name")
.eq("email", normalized)
.maybeSingle();
One row travels over the wire regardless of whether the instance has
100 users or 10 million, and "not found" is an honest answer instead of
an artifact of pagination.
HOW THE FIX WORKS
routes/orgs.ts's resolveUserIdByEmail now delegates to that helper:
const user = await findProfileUserByEmail(db, email);
return user?.id ?? null;
The contract is unchanged — null when no such user — so both callers
(POST /orgs/:orgId/members and POST /orgs/:orgId/teams/:teamId/members)
keep their existing 404 behavior for genuinely unknown emails, and the
stale comment is corrected. This was also the last listUsers scan in
the backend. Email normalization (trim + lowercase) lives inside the
helper, matching the semantics the old loop implemented by hand.
findProfileUserByEmail is covered by unit tests
(lib/__tests__/userLookup.test.ts: found via normalization, not-found,
and blank-input cases); the org routes have no route-level test file,
so the two-line delegation rides on that existing coverage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…API actually calls
WHY THIS MATTERS
PR migration 20260717_03 added the new org-membership visibility branch
("you can see rows tagged with an org you belong to") to
get_tabular_reviews_overview — but only to its legacy 3-argument
overload. The API never calls that overload. GET /tabular-review builds
nine named arguments (p_user_id, p_user_email, p_project_id, p_scope,
p_limit, p_offset, p_search_term, p_sort_key, p_sort_direction) in
lib/tabularReviewsOverview.ts, so PostgREST resolves the 9-argument
overload from 20260726_01 — which never got the org branch. Result: on
any install upgraded via migrations, org-shared reviews were readable
through the detail endpoints (access.ts allows them) yet invisible in
every list view. Fresh installs, which bootstrap from schema.sql, were
unaffected because schema.sql's 9-argument version already carries the
org branch — so fresh and migrated databases silently diverged, the
worst kind of bug: nothing errors, some deployments just show less data.
WHAT IS FUNCTION OVERLOADING IN POSTGRES
Postgres identifies a function by name AND argument types.
get_tabular_reviews_overview(text, text, text) and
get_tabular_reviews_overview(text, text, text, text, integer, integer,
text, text, text) are two completely independent functions that happen
to share a name. CREATE OR REPLACE matches on the full signature, so
replacing one overload never touches the other:
create or replace function f(a text) ... -- replaces f(text)
create or replace function f(a text, b int) ... -- separate function!
PostgREST resolves an RPC call to the overload whose named parameters
match the JSON body it received. A call with nine named keys can only
ever hit the 9-argument overload. That is why patching the 3-argument
overload in 20260717_03 was a no-op for the API.
HOW THE FIX WORKS
This migration re-declares both overloads exactly as backend/schema.sql
already defines them (the bodies are copied verbatim, not rewritten):
1. The 9-argument overload gains the two org-membership arms:
- accessible_projects: projects whose org_id is in an org the caller
belongs to (EXISTS against org_members) — so in-project reviews of
org colleagues become visible;
- visible_reviews: org-tagged reviews owned by someone else become
visible in the global list (p_project_id is null).
2. The 3-argument overload becomes the thin wrapper schema.sql uses —
it simply delegates to the 9-argument version with scope 'all' and an
effectively unlimited page — replacing 20260717_03's divergent
full-body copy. From now on there is a single source of truth for the
visibility predicate, and this file sorts after every earlier
overview migration, so replaying migrations from scratch also ends in
the correct state (previously 20260726_01 sorted after 20260717_03
and wiped the org branch on replay).
Signatures do not change, so CREATE OR REPLACE suffices — no DROP
FUNCTION, and the migration is safe to re-run. schema.sql needs no
change: it was already correct; this converges migrated installs onto
it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WHY THIS MATTERS
get_tabular_review_ids_overview backs GET /tabular-review/ids, the bulk
"select all matching" action: it returns id + owner for every review the
caller can see, so the client can select the whole filtered set without
fetching full payloads. For performance it does NOT delegate to
get_tabular_reviews_overview — it carries its own copy of the visibility
predicate, and its own migration (20260727_01) warns in its header:
"If the access/visibility rules in get_tabular_reviews_overview's
visible_reviews CTE ever change, mirror the change here too."
The organizations work changed exactly those rules — it added a third
visibility branch, "rows tagged with an org the caller belongs to",
alongside "row owner" and "shared_with email" — but this RPC was never
mirrored, neither in a migration nor in schema.sql. The user-visible
symptom is nasty because it is partial: an org member SEES a colleague's
org-shared reviews in the list, but "select all matching" silently
skips them, so bulk actions run over fewer rows than the visible
selection implies. Nothing errors; rows just go missing.
HOW THE FIX WORKS
Add the same two org-membership arms the paginated overview uses
(compare its 9-argument definition in schema.sql):
1. accessible_projects gains an EXISTS against org_members on
p.org_id — reviews living in an org colleague's project become
visible, exactly like email-shared projects already were:
or (
p.org_id is not null
and p.user_id <> p_user_id
and exists (
select 1 from public.org_members m
where m.org_id = p.org_id and m.user_id::text = p_user_id
)
)
2. The row filter gains the matching arm for org-tagged reviews in the
global list (p_project_id is null), keyed on tr.org_id.
The change is made in BOTH places that define this function — edited
in place in backend/schema.sql (fresh installs bootstrap from it) and
as new migration 20260805_02 (upgrades existing installs) — with
byte-identical function bodies, so the two install paths cannot
diverge. org_members.user_id is a uuid FK to auth.users while content
tables store user_id as text, hence the ::text casts. Org membership
grants visibility only; user_id still identifies the owner.
The signature is unchanged, so the migration is CREATE OR REPLACE only
and safe to re-run.
TESTS
The org-visibility integration suite (tabularPagination.supabase.test.ts)
now seeds two real auth users in one org — the FK to auth.users means
random UUIDs won't do — makes one a plain "member", and asserts the
member sees the colleague's in-project and standalone org reviews via
the ids RPC, plus a lockstep assertion that the ids RPC returns exactly
the set the paginated overview shows, which is the drift this bug class
is about.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…actor
WHY THIS MATTERS
The manager-gate test for POST /tabular-review/:reviewId/clear-cells was
written when the endpoint keyed cell resets on document_ids. After
rebasing onto main it hit the 400 validation branch ("row_ids is
required") before ever reaching the 403 role check it exists to prove,
so the RBAC gate went untested.
WHAT IS THE LOGICAL-ROWS REFACTOR
Main's folder-grouped tabular review work (open-legal-products#274) reshaped reviews around
logical review rows: a row can represent a folder of documents, not just
one document, so cell-level operations now address rows. clear-cells
accordingly takes row_ids instead of document_ids.
HOW THE FIX WORKS
The test now sends { row_ids: ["row-1"] }, a valid payload under the
current API, so the request passes validation and exercises the intended
assertion: an org editor without structure.manage receives 403 "Only a
review manager can clear cells". No production code changes.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…dels Rebasing onto current main surfaced a new instance of the overload trap this branch already fixed once for tabular reviews (20260816_04/05): since the org feature was written, main grew paginated overloads for the projects and workflows overviews (20260807_01), bulk select-all ids RPCs and a sidebar summaries RPC (20260812_01), and a TS-side directory search. Every one of them re-implements the visibility predicate, and every one of them predates the org concept — so an org member would see org content in the detail endpoints and legacy lists but not in the paginated lists, the sidebar, select-all, or the document picker. SQL (migration 20260816_06 + schema.sql in lockstep, byte-identical bodies): get_projects_overview (10-arg), get_project_ids_overview, get_project_summaries gain the same org-membership EXISTS arm the legacy overloads got in 20260816_03; get_workflows_overview (12-arg) and get_workflow_ids_overview gain the org_shared CTE from the 3-arg version, with scope 'shared' including org-shared rows. TS: handleProjectDirectorySearch (GET /projects?view=directory-search) adds the org branch to its inline access derivation via listUserOrgIds, mirroring listAccessibleProjectIds. GET /projects/:id/directory and /export already route through checkProjectAccess and inherit the org branch for free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e on docs and reviews ensureDocAccess and ensureReviewAccess checked the row's own org membership before falling through to the project check — but the project check is what carries the shared_with editor branch. A colleague who is BOTH a plain member of your org AND explicitly shared on your project (the normal in-firm sharing case) matched the org branch first and came out a viewer: every content.edit route (upload versions, rename, chat, accept/reject edits, generate) suddenly 403'd for someone main happily allowed. checkProjectAccess and the file's own precedence note already had it right: owner, then shared_with, then org. Reorder both helpers to match — project fall-through (share-aware) first, the row's own org branch after — while keeping the org branch's upgrade power: an org owner/admin is a manager even where the project branch only found viewer standing, and an org-tagged standalone row (no project) still resolves through its own org_id. Roles can now only strengthen as branches accumulate, never weaken. Also aligns ensureReviewAccess's email comparison with the trim() normalization the other helpers got in the sharing-email-case fix. Regression tests: the overlap case pins editor on both helpers, and an org admin pins manager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ment used PgArray syntax
GET /projects?view=directory-search builds its shared-projects query
with .contains("shared_with", [email]). supabase-js serializes a JS
array there as PostgREST's PgArray form ({email}), which Postgres
rejects for a jsonb column — "invalid input syntax for type json" —
so the endpoint returned 500 for any authenticated caller (the email
branch always runs). Pre-existing on main, but this branch adds the org
visibility arm to exactly this handler, so the document picker must
actually work to deliver it. Use the JSON containment idiom the sibling
listAccessibleProjectIds already uses:
.filter("shared_with", "cs", JSON.stringify([email])).
Verified live: owner and org-member searches both 500'd before, both
return the project after.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… lists they filter 20260816_06 ported the org-membership arm onto the second-generation paginated/ids/summary read-models, but the 20260812_01 batch it covered also shipped two dropdown-option companions that were missed: get_project_filter_options (the Practice and Owner dropdowns above the projects table) and get_workflow_filter_options (practice / language / jurisdiction above the workflows list). Both derive their option sets from a "visible rows" predicate that still only knew the owner and email-share arms. The symptom is a visible seam for org members: the list shows an org project, but its owner never appears in the Owner filter and its practice never appears in the Practice filter — rows you can see but cannot filter to. Filters and lists must be computed from the same visible set or the UI contradicts itself. The arms are copied verbatim from the 20260816_06 predicates. In the workflows RPC the org CTE keeps the workflow_shares NOT EXISTS dedup and is tagged source='shared', matching the overview's bucketing decision (shared-with-me and shared-via-my-org are one "shared" bucket from the caller's point of view), so p_scope filtering stays consistent between a list and its dropdowns. schema.sql carries the same bodies for fresh installs. Verified live: an org member's Owner dropdown now offers the org project's owner, and an org-shared workflow's practice appears under scope=all and scope=shared but not scope=owned. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ant can never demote The previous commit in this area (0096f38) fixed one direction of branch shadowing — an org "viewer" verdict no longer buries a shared_with editor — but the mirror direction survived: the shared_with branch was still an unconditional early return, so it shadowed STRONGER verdicts. Two concrete downgrades: * checkProjectAccess: an org owner/admin (manager tier) whose email someone also put in the project's shared_with resolved as editor — adding a grant stripped their folder-rename/delete, metadata, and sharing rights. * ensureReviewAccess: the review's own shared_with early-returned editor before the project was even consulted, so the PROJECT OWNER, politely added to a review's share list by a collaborator, lost owner-tier standing on that review (403 on title/columns edits). Both are privilege LOSS, not escalation — but they violate the invariant the role ladder promises: grants accumulate, they never subtract. That invariant is safe here precisely because shared_with entries are bare email strings with no per-entry role: there is no way to express a deliberately weaker share that strongest-wins could override. The fix replaces ordered early returns with an explicit merge: every matching branch derives its ProjectRole and the strongest one wins (strongerRole in lib/permissions.ts). ensureDocAccess/ensureReviewAccess skip the extra org lookup when the project verdict already folded in the same org's membership, so the common paths cost the same queries as before. This also retires the old code's dead upgrade ternary — "upgrade-only" is now a property of the merge, not of branch ordering. Regression tests pin the two downgrades above, plus the one fixture where the doc's own org actually decides the outcome (doc org differs from the project's org and only the former makes the caller a manager). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…containment misuses cdf6706 fixed the directory-search 500 caused by calling supabase-js .contains() on a jsonb column (it serializes a JS array as PostgREST's PgArray form {email}, which Postgres rejects for jsonb). A sweep for survivors of the same pattern found two, both with a worse failure mode than a 500 — silent data loss: * routes/audit.ts accessibleProjectIds used .contains on projects.shared_with, and the caller swallows query errors (shared.data ?? []), so GET /audit and GET /audit/export quietly omitted every event from projects shared with the caller — for every caller, on an audit surface whose whole point is completeness. * lib/userDataExport.ts passed the raw login email into the tabular_reviews containment while the projects query two lines up normalized it; shared_with entries are stored lowercased, so a mixed-case account's data export silently missed reviews shared with them. Both now use the JSON containment idiom the rest of the codebase settled on: .filter("shared_with", "cs", JSON.stringify([email])) with a trimmed, lowercased email. The audit test mock deliberately no longer implements .contains — a regression back to PgArray serialization fails the suite loudly instead of passing against a too-forgiving fake — and a new case pins the exact lowercased jsonb literal that goes over the wire. Verified live: an event in a project shared with the caller now appears in their /audit feed (it did not before the fix), and a stranger still sees nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every user gets a personal org at signup, and resolveContentOrgId tags every org-less row they create with it — that is what keeps all content tenant-scoped without demanding an explicit org on every write. The quiet consequence: personal-org membership IS visibility of the owner's entire private library, because all the org visibility arms (lists, detail access, RPCs) key off org_members. addMember had no personal guard. The org owner could POST a colleague into their own personal org — plausibly thinking of it as "my workspace, let me add my assistant" — and thereby share every private project, document, chat, review and workflow they had ever created, with no per-item consent and no UI hint of the blast radius. Explicit sharing (shared_with, real orgs) is the intended channel for that. addMember now refuses with a 400 validation error when the target org is personal. Teams need no equivalent guard: team membership requires org membership first, so a single-user org cannot grow a multi-user team. Verified live: POST /orgs/:personalOrgId/members returns 400; the 57-check RBAC matrix (real orgs) is unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Last-owner protection lived only in lib/orgs.ts as a read-then-act check: count the owners, then demote/remove if more than one remain. Two concurrent requests each removing a DIFFERENT owner both count two, both pass, and the org ends up with zero owners — a terminal state, because granting the owner role itself requires an owner, so no API call can ever repair it. Races between requests cannot be fixed in the application layer without a serialization point, so the invariant moves into the database: org_members_protect_last_owner (BEFORE DELETE OR UPDATE OF role) first locks the organizations row FOR UPDATE — concurrent owner departures on the same org serialize on that lock, and the loser re-counts after the winner committed, sees one owner left, and aborts with errcode 23514. The service layer maps that onto the same 409 the sequential check produces, so callers can't tell which layer stopped them. The trigger deliberately stands aside for the two legitimate cascades: org deletion (the organizations row is already gone inside the transaction, so the lock probe finds nothing) and direct auth-user deletion (the member's auth.users row is gone; security definer, like handle_new_user, so that probe works for any calling role). Account-deletion cleanup is reordered to match — and it's a fix in its own right: it used to DELETE the sole owner's membership first and promote an heir after, a window where the org exists ownerless (and which the trigger would now reject). It now promotes the heir before the membership goes, or deletes a memberless org outright and lets the cascade clean up. Verified live: direct SQL demote/delete of a sole owner both fail with "must keep at least one owner"; the API path still returns its 409. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…map to 409 Three service-layer gaps in the same file: * removeMember deleted only the org_members row. team_members has no FK to org membership (its cascades are team deletion and auth-user deletion), and the org-membership requirement is enforced only at addTeamMember time — so a removed member kept appearing on every one of that org's team rosters, silently violating the "teams group existing members" invariant. Removal now also deletes the target's team_members rows across that org's teams (and only that org's: their seats elsewhere are untouched, pinned by test). * The existence pre-checks in addMember / addTeamMember / createTeam race with concurrent inserts. The unique constraints backstop correctness, but the raw 23505 surfaced as a 500 db_error; it now maps onto the same 409 conflict the sequential path returns. * When the new last-owner trigger (20260816_08) fires under a race, its 23514 arrived as a 500; updateMember / removeMember now translate it into the last_owner result → the standard 409 with the standard message. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The signup trigger swallows every exception by design (a provisioning hiccup must never block signup) and the backfill migration ran exactly once. A user who slipped through both — trigger failed after the backfill's moment in time — had no personal org FOREVER: getPersonalOrgId returned null, every piece of content they created landed with org_id null, and GET /orgs returned an empty list, with no code path anywhere that would repair it. The same state reappears when account deletion clears a user's data but the final auth-delete fails. getPersonalOrgId now self-heals: when the lookup comes back empty it creates the org (named after the profile email, like the trigger) plus the owner membership. The partial unique index on (created_by) where personal makes the lazy create race-safe — the loser of a concurrent create re-reads the winner's row. A failed membership insert degrades softly: content tagging and owner-arm visibility never depended on membership, only GET /orgs does, and the next call cannot re-create a duplicate. Verified live: delete a user's personal org, create a workflow — the workflow lands in a freshly healed personal org and GET /orgs shows it again. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…accounts POST /orgs/:orgId/members and the team-member variant resolved the target email BEFORE checking the caller's standing in the org, and the two failure responses are distinguishable 404s: unknown email → "No user with that email", known email but caller not a manager → "Organization not found". Any authenticated user could therefore probe arbitrary emails for account existence against any orgId — an oracle, independent of whether the add could ever succeed. The routes now prove the caller holds a manage role in the org first and only then touch the email, so non-managers always get the same organization-not-found response regardless of what they probed with. The service layer re-checks the role afterwards; this commit only fixes the ordering of disclosure. Verified live: an outsider probing a real org with a nonexistent email gets "Organization not found", not email feedback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tail access 20260814_01 lowercased projects.shared_with but left tabular_reviews alone. Review shares have the same jsonb email-list shape and the same split consumers: the TypeScript detail check (ensureReviewAccess) lowercases both sides, but the list/ids RPCs compare the STORED entry against the caller's already-lowercased email via jsonb containment. A pre-normalization mixed-case entry therefore produced a ghost review — openable from a direct link, invisible in every list. New writes have been normalized for a while (routes/tabular.ts), so this is a one-pass data repair using the exact lateral-unnest recipe of 20260814_01: trim, lowercase, dedupe, preserve first-seen order, and touch only rows that actually change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rges The schema-drift check builds a real upgraded deployment (baseline schema.sql + migrations) and a real fresh install (current schema.sql) and diffs their fingerprints — and a function's stored body includes its comments verbatim, so even a rewrapped comment line reads as two deployments disagreeing about a function. Three such seams slipped into the last round: the two filter-options org-arm comments referenced the sibling migration number in the migration copy but not in schema.sql, and the last-owner trigger carried two inline comments in the migration that schema.sql's copy lacked. This aligns the text byte-for-byte (migration 20260816_07 adopts the schema.sql wording; schema.sql adopts the trigger's inline comments). Editing 20260816_07/08 is safe: they are part of this unmerged PR, not shipped history. Verified by replicating the CI check locally — scratch postgres, baseline 9a1277b + added migrations vs current schema.sql, fingerprints identical. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ting The client half of the org/RBAC feature: orgs become visible and usable, and the UI stops deriving permissions from a single is_owner boolean. - Settings → Organizations: create orgs, expand into a member roster (profile names/emails, role select with owner/admin/member, remove or leave with confirm; 409 last-owner protection surfaced inline) and teams (create/delete, add/remove members). Personal orgs stay hidden — they are plumbing, not a firm. - New project modal gains an Organization select (personal workspace default) so org-scoped projects can actually be created; sidebar user menu links to Organizations. - GET /orgs/:id/members and /teams responses enriched server-side with mirrored profile email/display_name (same source as /people) so the client never renders a bare user id. - lib/permissions.ts mirrors the server capability matrix; detail pages derive the caller's role from access_role (falling back to the is_owner list-row contract) and gate affordances through one canDo() seam: project details/sharing manager+, folder rename/move/delete manager+, doc rename/move and folder create editor+, review columns/document-set/clear-cells/details manager+, container delete owner-only. This also closes the previously unguarded folder-tree actions and un-read-onlys org admins. - OwnerOnlyPopup generalised with a requiredRole tier so denials name the role that can act, and now shows the owner's email when known. - Review details save only sends project_id when it changes (moving is owner-only server-side; managers editing a title must not 403). - Tests: client matrix + roleFrom contract, OrganizationsPage flows (create, roster, role change, error surfacing, member read-only), OwnerOnlyPopup tiers. Frontend suite 74 passed; backend 307 passed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WHY THIS MATTERS
The teams endpoint fetched team member rows with:
const [{ data: memberRows }, ...] = await Promise.all([...]);
Destructuring only `data` and never looking at `error` means a failed
query (network blip, RLS misconfiguration, dropped column after a bad
migration) does not fail the request — it returns HTTP 200 with every
team showing an empty member list. That is worse than a 500: the client
renders confidently wrong data, users think members were removed, and
nothing in monitoring fires because the status code says "success".
Every other Supabase query in this codebase checks `error` and maps it
to a 500; this one silently swallowed it.
WHAT IS A SILENT PARTIAL FAILURE
supabase-js never throws on query failure. Each query resolves to a
`{ data, error }` pair, and exactly one of the two is meaningful. If
you read `data` without checking `error`, a failure looks identical to
an empty result set (`data` is null, and `null ?? []` turns it into
[]). The type system cannot save you here — `data: T[] | null` is a
valid type either way — so the discipline of checking `error` after
every query is the only guard.
HOW THE FIX WORKS
Keep the Promise.all (the two queries are still independent), but bind
the whole response object instead of destructuring `data` away:
const [membersRes, { userById }] = await Promise.all([...]);
if (membersRes.error)
return void res.status(500).json({ detail: membersRes.error.message });
const memberRows = membersRes.data;
On failure the handler now responds 500 with the database error detail
— the same shape sendFailure() produces for "db_error" results from
lib/orgs.ts — so clients and monitoring see the truth instead of a
plausible-looking empty roster.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… a table scan
WHY THIS MATTERS
GET /orgs/:orgId/members and GET /orgs/:orgId/teams enriched their
rosters by calling loadProfileUsersByEmail(), which selects EVERY row
of user_profiles and builds two in-memory maps — just to look up the
handful of user_ids actually present in the roster. On a small dev
database this is invisible; in production it means every roster render
pulls the entire user directory over the wire, on every request, and
the cost grows with total signups rather than with org size. It is a
classic O(all users) query hiding behind an O(members) endpoint.
WHAT IS A TARGETED LOOKUP
Instead of "fetch everything, filter in application code", you push the
filter into the database with an IN clause:
select user_id, email, display_name
from user_profiles
where user_id in ('a...', 'b...', 'c...')
The database resolves this against the primary-key index on user_id and
returns only the rows requested. Transfer size and query time are now
proportional to the roster being rendered, not to the size of the user
base.
HOW THE FIX WORKS
lib/userLookup.ts gains loadProfileUsersByIds(db, userIds):
- de-duplicates and drops empty ids, short-circuits on an empty list
(no query at all for empty rosters);
- issues one .in("user_id", ids) select and returns a Map keyed by
user_id, in supabase style ({ userById, error }) so the route can map
a failure onto a 500 — Express 4 does not catch a thrown rejection
from an async handler.
Both org routes now collect the user_ids they already hold (member rows
/ team-member rows) and pass exactly those. In the /teams handler the
profile lookup consumes the roster query's output, so the two queries
run sequentially instead of in the previous Promise.all — the second
query is now so much smaller that this is still a large net win.
loadProfileUsersByEmail remains for callers that genuinely need the
email->user mapping across all profiles (projects/tabular people
endpoints).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… failures
WHY THIS MATTERS
The project role model says viewers can read documents but not add
them, and the header "Add" button already respects that (the parent
only wires it up when canDo("content.edit") is true). But DocTable had
two side doors around the button:
1. Drag-and-drop — handleDropCollectionFiles ran for any role, so a
viewer could drop files and kick off real upload requests.
2. The empty-state panel — its onClick called openAddDocuments
directly, opening the picker/modal for viewers too.
The backend correctly answers 403, but the catch block only did
console.error(err), so from the viewer's perspective the drop overlay
appeared, spinners ran, and then... nothing. Silent failure erodes
trust in the UI, and offering an action the user is not allowed to
perform is a permission-model inconsistency even when the server holds
the line. (Client-side gating is UX, not security — the backend check
remains the real enforcement.)
WHAT IS CAPABILITY GATING
Instead of sprinkling role names through components, the app maps roles
to capabilities once (frontend/src/app/lib/permissions.ts — e.g.
"content.edit" requires the editor rank) and components ask a single
question: allowed("content.edit")? DocTable wraps that in
requireCapability(capability, action, requiredRole), which returns
false AND raises the "you need the editor role to add documents" popup
via onOwnerOnlyAction — so every blocked path explains itself the same
way. When no canDo prop is passed (library/standalone contexts) the
check allows everything, preserving existing behavior outside projects.
HOW THE FIX WORKS
- handleDropCollectionFiles now begins with
if (!requireCapability("content.edit", "add documents", "editor"))
return;
which covers all three drop entry points (window-level drop, the
table-root drop zone, and the hidden file input) because they all
funnel through this one function.
- openAddDocuments performs the same check, covering the empty-state
click as well as any future caller of the published header action.
- The upload catch block still logs for debugging, but now also feeds
the existing WarningPopup channel:
setDocumentUploadWarning(detail ? `Upload failed: ${detail}` : ...)
using apiErrorDetail() to pull the backend's {detail} message, so a
403 (or any other failure) is shown to the user instead of vanishing
into the console.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…i coverage ratchet Main raised the src/app/lib coverage floors to 97/96/98/98 (PR open-legal-products#291) after this branch was written, and the floors are measured per function: eleven untested org endpoint wrappers dropped functions coverage to 93.9% and failed CI. Add the org wrappers — plus the org-scoped createProject variant — to the existing table-driven thin-wrapper suite, asserting route, method, body serialization, and the default member role. Coverage lands back at 100% functions / 100% lines. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ver enforces The review header's Documents action, the selection Delete action, and the drag-and-drop upload path all mutate the review's document set — which the backend gates at structure.manage (PATCH document_ids 403s below manager). Columns, clear-results, details and workflow already ran through requireStructure, but these three paths had no client gate: an editor got a silent console.error (add), an optimistic removal that snapped back with no explanation (delete), or worst, an upload that succeeded followed by an attach that 403'd — an orphaned document (drop). Route all three through requireStructure so the role popup explains the denial before any request fires, mirroring the DocTable drop-path gate. Also give the org page's two raw text inputs (new org, new team) an explicit h-10: SETTINGS_CONTROL_CLASS carries no height — it is meant to be wrapped by SettingsTextInput, which adds one — so the bare swap from accountGlassInputClassName left them shorter than every sibling control. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
roleFrom resolves the caller's role from whatever payload a component
holds: detail responses carry access_role (for every role, including
"owner"), list rows carry is_owner. A row with NEITHER field used to
fall through to "owner" — and such rows exist: PATCH handlers return
the bare DB row. After a column save the review state was replaced by
that bare row, roleFrom said "owner", and every client gate opened for
a manager — including the delete flow, whose server side is a 204
no-op for non-owners, so the UI showed "deleted" and navigated away
while the review lived on. A phantom destructive action.
The unknown case now resolves to "viewer", the same fail-closed
posture `can()` already takes for unknown roles: guessing high opens
gates the server will refuse; guessing low only hides an affordance
until real data arrives. The legitimate contracts are untouched —
access_role wins when present, is_owner true/false still mean
owner/editor.
Verified: new regression test pins roleFrom({}) === "viewer"; full
frontend suite 361/361, coverage above the src/app/lib floors.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rver enforces
This PR introduces the first users who can see content they cannot
write to (org "viewer"s), which turns every ungated mutation affordance
into a dead end: the click fires, the server 403s or 404s, and nothing
explains why. Sweep of the review view and project workspace against
the backend capability table:
* Review generation and cell regeneration are editor-tier server-side
(content.edit on /generate and /regenerate-cell). A new
requireContent gate mirrors the existing requireStructure pattern and
shows the role popup instead of a doomed request.
* Moving a review to another project is owner-only server-side ("Only
the review owner can move a review") but sat behind the manager-tier
details modal; a manager changing the selector got an unexplained
failed save with the modal stuck open. The changed-project save now
requires owner up front.
* New Chat / New Review in a project are editor-tier server-side
(content.edit on chat create and review create); both creation
callbacks now gate and explain. The role derivation moved above the
callbacks it feeds — a useCallback deps array reads its identifiers
at render time, so referencing a later `const` would throw at
runtime (temporal dead zone), not just lint.
* saveColumnsConfig merges the PATCH response into the previous review
state instead of replacing it: the response is the bare DB row with
no access_role/is_owner, and replacing dropped the caller's role
(the enabling half of the phantom-delete chain fixed alongside in
roleFrom).
* The drop-upload path refetches the document list even after a
mid-loop failure — each successful upload has already attached
itself server-side, so skipping the refetch left real documents
invisible until reload — and failures now surface in a warning
popup instead of only console.error.
Verified: frontend suite 361/361, lint and tsc clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… hide the rest The collection drop handler ran its uploads through Promise.all, which rejects on the first failure and discards every sibling result — but the sibling uploads it discards have already succeeded server-side. The user saw "Upload failed", yet half their files existed and only appeared after a reload, which reads as duplicate-upload bait. Promise.allSettled keeps both halves of the truth: fulfilled uploads land in the table immediately, and the first rejection still feeds the existing warning popup with its error detail. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…w chat, add-documents Fresh-eyes sweep of client gates against the backend capability table found three affordance classes still offered to roles the server rejects, all in the dead-end shape this PR eliminates elsewhere (click → 403 → console.error → nothing visible): * Document VERSION mutations in DocTable were entirely ungated. The server tiers them two ways: upload/rename are content.edit, but file-replace and delete are OWNER-only (documents.ts gates them on access.isOwner, the doc's creator) — a capability check cannot express the latter, so a requireDocOwnerForVersions guard gates on the row itself. Version upload failures also now surface in the warning popup, and a failed collection drop no longer overwrites the unsupported-files warning (both problems render together). * Review chat: sending is editor-tier (content.edit on POST /chat) but READING history is view-tier, so gating the chat toggle would be stricter than the server. Instead TRChatPanel grows a canSend prop — viewers get the transcript with a disabled composer and an explanatory placeholder rather than a send that always errors. * The review "Add documents" buttons opened the modal for anyone and only denied on submit; they now pre-gate with requireStructure, the same stop-before-the-modal pattern openNewReview already uses. Two robustness fixes in the same files: clearResultsForRows rolls its optimistic "pending" state back when the server call fails (cells no longer blank until reload), and the review-load effect carries a cancellation flag so a stale fetch can't clobber fresh state on rapid review switches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tually do
The members list rendered every management control for anyone with
canManage, but the server distinguishes admins from owners: an admin
cannot change or remove an OWNER, and cannot grant the owner role
(lib/orgs.ts rank guards). An admin therefore saw a live role select —
"Owner" option included — and a remove button on owner rows, and every
use died with the generic top-of-card 403. Owner rows now show the
plain badge to admins, and the owner option only renders for owners.
Three feedback fixes in the same flow:
* The last-owner 409 ("An organization must keep at least one owner.")
rendered in the card body BEHIND the still-open confirm modal — a
sole owner clicking Leave → Confirm watched a popup do nothing. The
popup now closes when the action settles, failure or success, so the
explanation is actually visible.
* run() serializes card actions by silently no-oping when one is busy;
AddUserInput then cleared its email field as if the add succeeded.
run() now reports whether it ran, and AddUserInput keeps the input
on a skipped or failed add (returning `false` from onAdd is the new,
backward-compatible signal — void still means success).
* The owner-tier popup said "the project owner" even for review-level
denials; the subject is now just "the owner".
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
POST /orgs/:id/members and POST .../teams/:id/members return the inserted org_members/team_members row — no email, no display_name. The wrappers declared the enriched roster shapes (OrgMember / OrgTeamMember with email + display_name as present-but-nullable), which happens to work today only because the org page discards the response and refetches the roster. The first caller to trust the declared type would read fields that are simply absent. Split the types: OrgMemberRow / OrgTeamMemberRow are what mutations return, OrgMember extends the row with the mirrored profile fields the GET roster endpoint attaches. Lying types are latent bugs; this makes the contract match the wire. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tion Main's shared-UI unification (open-legal-products#345) rebuilt SettingsSection on the shared GlassCardUI, which renders a fixed surface class and accepts no className — padding now belongs to an inner wrapper div, the pattern every other settings page adopted in that refactor. The org page still passed className="p-4"/"p-0" from before the rebase, which tsc rejects against the new prop type. The create-org card gains the standard inner p-4 wrapper; the org card drops the no-op p-0 (its rows carry their own padding). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
amal66
force-pushed
the
olp-pr/organizations-ui
branch
from
August 21, 2026 17:29
35ab80c to
13c7ae0
Compare
The tabular-review chat panel got a canSend gate, but the project chat page shipped role-blind: an org viewer opening a colleague's chat saw a fully enabled composer, and Send rendered their bubble plus an empty assistant bubble while the server refused the write — no error anywhere but the console. The explorer's Upload button and the window-level file drop had the same shape: enabled affordance, silent 404. ChatInput now takes canSend (default true, so the standalone assistant is untouched). False renders the same read-only composer the review panel uses — disabled textarea with the viewing-only placeholder, disabled send, attach/workflow entry points hidden, drop-uploads answered with the edit-access warning instead of a doomed request. The page derives the gate from the project detail's access_role (content.edit), with one exception: the chat's own creator keeps a live composer regardless of tier, because the server's ladder always lets a row's owner continue their own thread. While the project is still loading the gate stays open — enforcement is server-side either way, and flashing a disabled composer at every editor on load is worse than one refused send. Replication (before): as a plain org member, open another member's project chat and send a message — bubble appears, nothing answers, POST /projects/:id/chat 404s silently. After: composer reads "Viewing only — sending needs edit access" and is inert; the explorer upload button is disabled and a drop shows the editors-only popup. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0128b5tXqvX4GghBELHhyVnC
Collaborator
Author
Live smoke evidence — 2026-08-21 (rebased tips + composer fix
|
amal66
marked this pull request as ready for review
August 21, 2026 21:16
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
2 participants
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.
Stacked on #267
This PR is the frontend half of the organizations/RBAC feature: it sits on top of #267's branch and carries its two commits — review the top commit only (
feat(frontend): organization management UI + role-aware permission gating). If #267 merges first, this rebases to a single clean commit.#267 shipped the tenant layer and the project role ladder server-side, but nothing in the product surfaced it: no way to create an org, no way to see members, and a UI that still derives every permission from a single
is_ownerboolean (including the org-admin case where the server allows an action but the client renders read-only). This PR closes those UI/UX flows.What it adds
Settings → Organizations (new page, plus a sidebar entry)
409 An organization must keep at least one owner.last-owner protection.Org-scoped project creation
The New Project modal gains an Organization select (default: Personal workspace) populated from
GET /orgs. This is the write path that makes the whole feature real: until now nothing in the UI could create a project with anorg_id.Role-aware permission gating (replaces
is_owner-only logic)frontend/src/app/lib/permissions.tsmirrors the server's capability matrix, and detail pages derive the caller's role from theaccess_rolefield #267 added (falling back to the historicis_ownercontract on list rows). OnecanDo(capability)seam threads through the project workspace:members.manage(manager+)structure.manage(manager+)docs.organize(editor+)content.edit(editor+)structure.manage(manager+)Notable fixes this delivers:
OwnerOnlyPopupgrows arequiredRoletier ("Only the owner or a manager can rename folders.") and now shows the owner's email when known, so a denial tells the user who to ask.project_idwhen it actually changed — moving a review is owner-only server-side, and a manager editing just the title must not 403.Backend (small, additive)
GET /orgs/:orgId/membersandGET /orgs/:orgId/teamsare enriched with mirrored profileemail/display_name(same source as the projects/peopleendpoint) so the client never has to render a UUID.UX walkthrough (screenshots)
Captured against a live local stack (Supabase CLI + MinIO + backend + prod
next start), three users in one org: an owner, an admin (→ manager role on org projects), and a plain member (→ viewer).As the org owner
1 — Organization management (Settings → Organizations): roster with role selects and descriptions, teams with member chips.
2 — Creating a project inside the firm: the new Organization select in the New Project modal.
3 — Owner's project view: full toolset (Folder, Add documents, Actions).
4 — People with access modal.
As a plain org member (viewer)
5 — Same project, read-only: no Folder button, Add documents disabled.
6 — "View details": inputs disabled.
7 — Delete attempt: owner-only popup instead of a silent 404.
As an org admin (manager)
8 — Manager toolset: Folder and uploads available — the previous server-allows/client-forbids inconsistency is gone.
9 — "Edit details": editable, with Update action.
10 — Container deletion still owner-only, even for admins.
Testing
cd frontend && npm test→ 74 passed (10 files), including new suites: the client capability matrix asserted cell-by-cell against the server's, theroleFromfallback contract, OrganizationsPage flows (create org, roster rendering, role change, inline 409 surfacing, member read-only mode), and OwnerOnlyPopup tier copy.cd frontend && npx tsc --noEmit→ clean;npm run lint→ 0 errors;next build→ clean production build.cd backend && npx vitest run→ 307 passed (member/team enrichment covered by the orgs service suite).next start): org create → member add → role change → viewer/editor/manager gating in the project and review UIs.Provenance
All new code written for this PR (no fork port — the fork has no org UI). Follows the existing conventions: account settings tab rail,
AccountSectionglass cards,PillButton/ModalSelect/AddUserInputprimitives,apiRequestclient layer, theOwnerOnlyPopupguard idiom, and testing-library patterns from the frontend harness.🤖 Generated with Claude Code