feat: promote external collaborators to first-class role#582
Merged
Conversation
Replaces the two-field representation (workspace_membership.is_external
bool + role='member') with a single role='external' value across the
schema, API, and frontend. Externals share the unified seat pool with
members per the PRD — no separate cap, policy-only differences.
- Drop workspace_membership.is_external and workspace_invite.include_org_membership;
add 'external' to both role enums (snapshot updated)
- Centralise role checks via ROLE_HIERARCHY; remove effective_workspace_role()
- Unify seat counting through compute_effective_seat_state (covers derived
org admins, so admin billing rollup, /usage, and enforcement all agree)
- Enforce the role='external' ⟺ no org_membership invariant at every
write path (invites, onboarding, accept-by-id/hash, heal)
- workspace_settings role dropdown rejects both directions of the
external boundary instead of only one
- Frontend: rename guest → external in roles.ts, invite wizards, member
lists, and badges; filter org-member picker by role string instead
of the optional is_external field
- Admin usage-and-billing: Seats column and footer now show
members + externals (the unified pool the overage charges against)
Refs: docs/adr/0003-external-as-role.md,
docs/prds/external-role-and-unified-seat-counting.md
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis PR migrates workspace collaboration access from a ChangesExternal Role Unification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
echo/frontend/src/routes/organisation/OrganisationRoute.tsx (1)
837-843:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale terminology: "guests" should be "externals"
yo this copy still says "guests" but we just shipped the whole external role migration. ship it consistent fam.
<Text size="xs" c="dimmed"> <Trans> Admins can reach every workspace in this organisation. Members - and guests only see the workspaces they've been given access + and externals only see the workspaces they've been given access to. </Trans> </Text>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@echo/frontend/src/routes/organisation/OrganisationRoute.tsx` around lines 837 - 843, Update the stale UI copy in OrganisationRoute by replacing the word "guests" with "externals" inside the Trans-wrapped text (found in OrganisationRoute.tsx where the Text component renders that description). Ensure the Trans string is updated exactly to "Admins can reach every workspace in this organisation. Members and externals only see the workspaces they've been given access to." and adjust any related translation keys/translations if your i18n backend requires updating.echo/server/dembrane/api/v2/onboarding.py (1)
107-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
invited_byis still used later but no longer fetched.Both the cap-blocked path and the invite-accepted path read
invite.get("invited_by"). After this change those notifications lose their recipient and silently stop firing. Addinvited_byback to the fields list or remove the downstream dependency.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@echo/server/dembrane/api/v2/onboarding.py` around lines 107 - 112, The patch removed "invited_by" from the fields list but downstream code still calls invite.get("invited_by") (used in the cap-blocked and invite-accepted notification paths), causing missing recipients; restore "invited_by" to the fields array in onboarding.py (the same dict that currently contains "id", "workspace_id", "role", "expires_at") so those notification paths can read invite.get("invited_by"), or alternatively remove/replace the invite.get("invited_by") usages in the cap-blocked and invite-accepted logic if you intend to stop relying on that field.echo/server/dembrane/api/v2/orgs.py (1)
202-250:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSubtract internal org members from this external count.
_count_external_in_organisation()says it counts users withrole="external"and noorg_membership, but the implementation only filtersworkspace_membership.role == "external". If a stale row survives during migration/heal, this header count will over-report whilelist_org_members()hides the same user viainternal_set, so the two org screens drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@echo/server/dembrane/api/v2/orgs.py` around lines 202 - 250, _count_external_in_organisation currently only counts workspace_membership rows with role="external" and doesn't exclude users who still have an org_membership; fetch org membership user IDs for this org (via async_directus.get_items on "org_membership" with filter org_id=_eq org_id and deleted_at _null and fields ["user_id"]), build a set of internal org user_ids, and subtract that set from the external workspace user_id set before returning the count; update references in the function (_count_external_in_organisation, async_directus.get_items, workspace_membership, org_membership) and ensure you guard against non-list returns and missing user_id keys as the existing code does.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@echo/scripts/seed_dev.py`:
- Line 715: The log message prints a misleading seeded email; update the print
call that currently outputs "grace@external" to the actual seeded account
"grace@seed.dembrane.dev" (i.e., modify the print(...) invocation that logs
"external grace@external on Acme/Default" so it shows "external
grace@seed.dembrane.dev on Acme/Default") to ensure seed output matches the real
seeded account.
In `@echo/server/dembrane/api/v2/onboarding.py`:
- Around line 137-143: Move the seat-cap check so it only runs when a new
membership will actually be created: check for an existing workspace_membership
(and its accepted_at state) before calling assert_can_add_seat() and only call
assert_can_add_seat() when no accepted membership exists and you are about to
create one; apply the same change to the duplicate section around the 229-252
logic. Ensure you reference and use the existing workspace_membership
variable/lookup and the accepted_at field to decide whether to skip the cap
check so retries or partial writes don’t 402 an idempotent invite.
- Around line 133-135: The current invite-processing loop sets
invite_role/is_external and then writes external workspace rows even if an
earlier invite already created org_membership; before creating or inserting an
external membership row, re-check whether org_membership already exists for that
user+org (e.g., query the same org_membership lookup used when creating internal
rows) and skip or reject the invite if org_membership is present so you never
create role="external" for users who already have org_membership; apply the same
guard to the other invite handling block that mirrors this logic (the later
block referenced in the comment).
---
Outside diff comments:
In `@echo/frontend/src/routes/organisation/OrganisationRoute.tsx`:
- Around line 837-843: Update the stale UI copy in OrganisationRoute by
replacing the word "guests" with "externals" inside the Trans-wrapped text
(found in OrganisationRoute.tsx where the Text component renders that
description). Ensure the Trans string is updated exactly to "Admins can reach
every workspace in this organisation. Members and externals only see the
workspaces they've been given access to." and adjust any related translation
keys/translations if your i18n backend requires updating.
In `@echo/server/dembrane/api/v2/onboarding.py`:
- Around line 107-112: The patch removed "invited_by" from the fields list but
downstream code still calls invite.get("invited_by") (used in the cap-blocked
and invite-accepted notification paths), causing missing recipients; restore
"invited_by" to the fields array in onboarding.py (the same dict that currently
contains "id", "workspace_id", "role", "expires_at") so those notification paths
can read invite.get("invited_by"), or alternatively remove/replace the
invite.get("invited_by") usages in the cap-blocked and invite-accepted logic if
you intend to stop relying on that field.
In `@echo/server/dembrane/api/v2/orgs.py`:
- Around line 202-250: _count_external_in_organisation currently only counts
workspace_membership rows with role="external" and doesn't exclude users who
still have an org_membership; fetch org membership user IDs for this org (via
async_directus.get_items on "org_membership" with filter org_id=_eq org_id and
deleted_at _null and fields ["user_id"]), build a set of internal org user_ids,
and subtract that set from the external workspace user_id set before returning
the count; update references in the function (_count_external_in_organisation,
async_directus.get_items, workspace_membership, org_membership) and ensure you
guard against non-list returns and missing user_id keys as the existing code
does.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbed86b1-d7c5-4d99-a336-53deacb257d8
📒 Files selected for processing (48)
echo/directus/sync/snapshot/fields/workspace_invite/include_org_membership.jsonecho/directus/sync/snapshot/fields/workspace_invite/role.jsonecho/directus/sync/snapshot/fields/workspace_membership/is_external.jsonecho/directus/sync/snapshot/fields/workspace_membership/role.jsonecho/docs/adr/0003-external-as-role.mdecho/docs/prds/external-role-and-unified-seat-counting.mdecho/frontend/src/components/organisation/OrganisationInviteWizard.tsxecho/frontend/src/components/project/ProjectUsageAndSharing.tsxecho/frontend/src/components/settings/MyAccessCard.tsxecho/frontend/src/components/workspace/OrganisationUsageRollup.tsxecho/frontend/src/components/workspace/SeatCapBanner.tsxecho/frontend/src/components/workspace/UsageCard.tsxecho/frontend/src/components/workspace/WorkspaceInviteWizard.tsxecho/frontend/src/hooks/useWorkspace.tsecho/frontend/src/hooks/useWorkspaceUsage.tsecho/frontend/src/lib/roles.tsecho/frontend/src/routes/admin/AdminSettingsRoute.tsxecho/frontend/src/routes/onboarding/OnboardingRoute.tsxecho/frontend/src/routes/organisation/OrganisationRoute.tsxecho/frontend/src/routes/project/ProjectsHome.tsxecho/frontend/src/routes/workspaces/WorkspaceSelectorRoute.tsxecho/frontend/src/routes/workspaces/WorkspaceSettingsRoute.tsxecho/scripts/backfill_direct_memberships.pyecho/scripts/create_schema.pyecho/scripts/matrix_smoke.pyecho/scripts/seed_dev.pyecho/server/dembrane/api/project.pyecho/server/dembrane/api/template.pyecho/server/dembrane/api/v2/access_requests.pyecho/server/dembrane/api/v2/admin.pyecho/server/dembrane/api/v2/bff/_access.pyecho/server/dembrane/api/v2/invites.pyecho/server/dembrane/api/v2/me.pyecho/server/dembrane/api/v2/middleware.pyecho/server/dembrane/api/v2/onboarding.pyecho/server/dembrane/api/v2/orgs.pyecho/server/dembrane/api/v2/projects.pyecho/server/dembrane/api/v2/schemas.pyecho/server/dembrane/api/v2/workspace_settings.pyecho/server/dembrane/api/v2/workspaces.pyecho/server/dembrane/inheritance.pyecho/server/dembrane/policies.pyecho/server/dembrane/seat_capacity.pyecho/server/tests/test_onboarding.pyecho/server/tests/test_policies.pyecho/server/tests/test_seat_capacity.pyecho/server/tests/test_tier_capacity.pyecho/server/tests/test_usage_gates_api.py
💤 Files with no reviewable changes (4)
- echo/directus/sync/snapshot/fields/workspace_invite/include_org_membership.json
- echo/frontend/src/hooks/useWorkspace.ts
- echo/directus/sync/snapshot/fields/workspace_membership/is_external.json
- echo/scripts/backfill_direct_memberships.py
- onboarding: gate seat cap only on new memberships so retries don't 402
existing members; promote external→member when user already has
org_membership to preserve the ADR-0003 invariant; restore invited_by
in invite fields so cap-blocked/accepted notifications have a recipient
- orgs: subtract internal org members from _count_external_in_organisation
so the header count stays in sync with list_org_members during heal
- ui: "guests" → "externals" in OrganisationRoute access-rules copy
- seed: fix misleading grace@external log line to match seeded email
spashii
approved these changes
May 21, 2026
github-merge-queue Bot
pushed a commit
that referenced
this pull request
May 25, 2026
fix: repair regressions from sidebar PR (#585) The sidebar PR overlapped with #577, #581, #582, and #583 and introduced a few cross-cutting issues this commit addresses: - Restore the "hide Project defaults for external-only users" gate in the new sidebar's UserSettingsView. PR #582/#583 added this filter to the old UserSettingsRoute; when the sidebar moved navigation into a new view component, the NavItem rendered unconditionally and external-only users could click through to an empty pane. - Drop schema-step tests that imported the deleted scripts/create_schema.py one-shot seeding script. The committed snapshot under directus/sync/snapshot/ is the source of truth now, so those structural guards were checking a script that no longer exists. - Update AGENTS.md to remove the create_schema.py reference and point at the snapshot directory. - Populate ref_workspace_id on PROJECT_SHARE_ROLE_CHANGED (project_sharing.py), REPORT_READY, and REPORT_FAILED (tasks.py). The new useNotifications href builder requires both ref_workspace_id and ref_project_id to produce /w/{ws}/projects/... links; without it those notifications rendered as dead clicks. - Strip the per-request org_membership lookup in get_workspace_context. PR #582 enforces role='external' ⟺ no org_membership at every write path, and on_organisation_member_removed cascades workspace_membership soft-deletes correctly, so the read-side normalisation was redundant per-request DB overhead. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * User settings view now fetches and displays workspace access information, conditionally showing relevant configuration options based on access level. * Notifications for project sharing and report status now include workspace context information for better organization. * **Documentation** * Updated deployment guide with clarified schema snapshot and migration script procedures. * **Tests** * Removed legacy schema step validation tests from test suite. <!-- review_stack_entry_start --> [](https://app.coderabbit.ai/change-stack/Dembrane/echo/pull/586?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack) <!-- review_stack_entry_end --> <!-- end of auto-generated comment: release notes by coderabbit.ai -->
](https://app.coderabbit.ai/change-stack/Dembrane/echo/pull/586?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack){kind=link}
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.
Replaces the two-field representation (workspace_membership.is_external
bool + role='member') with a single role='external' value across the
schema, API, and frontend. Externals share the unified seat pool with
members per the PRD — no separate cap, policy-only differences.
Refs: docs/adr/0003-external-as-role.md,
docs/prds/external-role-and-unified-seat-counting.md
Summary by CodeRabbit
Release Notes
New Features
Documentation