feat(org): treat platform admins as members in internal organizations#3378
feat(org): treat platform admins as members in internal organizations#3378tofikwest wants to merge 5 commits into
Conversation
Adds `Organization.isInternal` so platform-operated organizations can run compliance with their platform-admin staff as real participants — assignable, counted toward compliance progress, and included in notifications and device rollups. Behavior is unchanged for every other organization (flag defaults to false; admin-only toggle on the org admin screen). Centralizes the rule behind a single `isOrgParticipant` predicate (`@trycompai/auth/participation`, mirrored for the Trigger.dev bundle), replacing the scattered inline `user.role === 'admin'` participation checks across assignment/approval, compliance counting, notifications, reminders, and device queries. Platform-admin access guards are intentionally untouched. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
All reported issues were addressed across 41 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…idation, keep dep boundary - run-browser-automation: use the shared `orgParticipantMemberWhere` helper instead of an inline copy, so it matches the other task notifiers (uses the role constant + owner exception) and still includes everyone in internal orgs. - update-admin-organization DTO: reject an explicit `null` isInternal at validation (400) instead of letting it reach the non-null column (500). - org-participation (app): stop re-exporting `isOrgParticipant` from the module that imports `@db/server`; consumers import it from the dependency-free `org-participation-rule` so server-only code can't leak into client/Trigger bundles. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb
|
@cubic-dev-ai review it |
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 41 files
Confidence score: 3/5
apps/app/src/lib/org-participation-rule.tsduplicatesisOrgParticipantinstead of using@trycompai/auth/participation, which creates a real drift risk where org-participation checks can behave differently across services and cause inconsistent access decisions after future updates — align both implementations now (or add a strict parity test plus a tracked follow-up if bundling constraints prevent immediate import).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/app/src/lib/org-participation-rule.ts">
<violation number="1" location="apps/app/src/lib/org-participation-rule.ts:17">
P2: `isOrgParticipant` is duplicated in `apps/app` instead of being imported from `@trycompai/auth/participation`. Even if importing the shared subpath is blocked by Trigger.dev bundling constraints, the two implementations can drift silently. Consider verifying whether the subpath avoids the `better-auth` bundling issue, and if duplication must stay, add a cross-package sync test so CI fails when the implementations diverge.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| */ | ||
| export const PLATFORM_ADMIN_ROLE = 'admin'; | ||
|
|
||
| export function isOrgParticipant( |
There was a problem hiding this comment.
P2: isOrgParticipant is duplicated in apps/app instead of being imported from @trycompai/auth/participation. Even if importing the shared subpath is blocked by Trigger.dev bundling constraints, the two implementations can drift silently. Consider verifying whether the subpath avoids the better-auth bundling issue, and if duplication must stay, add a cross-package sync test so CI fails when the implementations diverge.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/lib/org-participation-rule.ts, line 17:
<comment>`isOrgParticipant` is duplicated in `apps/app` instead of being imported from `@trycompai/auth/participation`. Even if importing the shared subpath is blocked by Trigger.dev bundling constraints, the two implementations can drift silently. Consider verifying whether the subpath avoids the `better-auth` bundling issue, and if duplication must stay, add a cross-package sync test so CI fails when the implementations diverge.</comment>
<file context>
@@ -0,0 +1,23 @@
+ */
+export const PLATFORM_ADMIN_ROLE = 'admin';
+
+export function isOrgParticipant(
+ userRole: string | null | undefined,
+ { orgIsInternal }: { orgIsInternal: boolean },
</file context>
…rule, guard boundaries - generate-vendor-mitigation: fail closed — only assign the author when the member actually exists in this org (add `author &&` guard) so a missed lookup can never write an unknown/cross-org member id. - SelectAssignee: use the shared `isOrgParticipant` predicate instead of an inline copy, so the picker can't drift from the backend rule. - lib/org-participation.ts: add `import 'server-only'` (matches compliance.ts / permissions.server.ts) so this DB-backed helper can't be pulled into a client bundle. - Add a drift guard test asserting the app-local participation rule stays identical to `@trycompai/auth/participation`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb
|
Reviewed the latest cubic pass (5 issues). Verdict + resolution below — fixes in Fixed (valid):
Not a real / new issue — marking as won't-fix:
All source typechecks clean (only the pre-existing |
|
@cubic-dev-ai review it |
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
All reported issues were addressed across 41 files
Confidence score: 5/5
- Safe to merge after the addressed issues were fixed.
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…r, skip needless query - orgParticipantMemberWhere: make it a faithful Prisma translation of isOrgParticipant — for non-internal orgs exclude only platform admins (include null roles; drop the owner carve-out, which the isInternal flag now supersedes). Removes the split source of truth between the predicate and the query fragment. - generate-vendor-mitigation: add `deactivated: false` to the author lookup so a deactivated member can't be auto-assigned (matches the other trigger tasks). - filterComplianceMembers (app + api): skip the `getOrgIsInternal` query when the member list has no platform admins — the flag can't change the result there. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb
|
Third cubic pass (3 issues) — all addressed in
Source typechecks clean (only the pre-existing |
|
@cubic-dev-ai review it |
@tofikwest I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
5 issues found across 41 files
Confidence score: 2/5
apps/api/src/utils/org-participation.ts(orgParticipantMemberWhere) can overwrite an existinguserrelation filter with a bare spread, so callers may silently get broader or narrower membership matches than intended. That’s a concrete regression risk for authorization/visibility logic—merge only after composing this with existinguserconstraints (e.g.,AND) and adding a regression test for pre-filtered callers.apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsxupdates only local state afterisInternalis toggled, so editing the currently active org can leaveOrgInternalProviderwith stale semantics until refresh. Merging as-is risks inconsistent access behavior across the app—trigger a route revalidation/provider refresh after a successful toggle before merging.apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsxapplies a high-impactisInternalchange on a single click without confirmation, despite changing org-wide membership rules. An accidental toggle could immediately alter who can participate—add an explicit confirmation (or secondary safeguard) before applying the mutation.apps/api/src/trigger/browser-automation/run-browser-automation.tsandapps/app/src/trigger/tasks/task/weekly-task-reminder.tsnow diverge in participation behavior and add extra org lookups, which can produce inconsistent recipient/task selection and unnecessary latency. Align both paths to the centralized participation predicate and remove the duplicate org fetch pattern to de-risk behavior drift.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx">
<violation number="1" location="apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx:52">
P1: After successfully toggling `isInternal`, the component only updates local `useState`. When an admin edits the currently active organization, the surrounding `OrgInternalProvider` in the server layout retains the old `organization.isInternal` value because the layout isn't refreshed. Downstream consumers such as `useOrgIsInternal()` (e.g., assignee pickers) will continue showing the stale flag until a full page reload.
The component already receives `currentOrgId` but does not use it. Consider calling `router.refresh()` after a successful update when `org.id === currentOrgId`, so the server layout re-renders and the provider reflects the new value. You will also need to import `useRouter` from `next/navigation`.</violation>
<violation number="2" location="apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx:135">
P2: The `isInternal` toggle changes core membership semantics for the whole organization, and the UI itself warns to leave it off for customer organizations. Because it fires immediately on a single click with no confirmation, an accidental toggle could incorrectly include platform admins in a customer org's compliance and assignment flows. Consider using an `AlertDialog` confirmation (already available in `@trycompai/design-system` and used for timeline deletion in `TimelineCard.tsx`) before enabling (and possibly disabling) this setting.</violation>
</file>
<file name="apps/api/src/utils/org-participation.ts">
<violation number="1" location="apps/api/src/utils/org-participation.ts:51">
P1: `orgParticipantMemberWhere` returns a bare spread fragment that overwrites existing `user` filters when callers already constrain the `user` relation. For non-internal orgs the returned `{ user: { OR: [...] } }` replaces, rather than intersects with, any existing `user` key in the `where` object.
Concrete impact: in the mention notifiers (`comment-mention-notifier.service.ts` and `task-item-mention-notifier.service.ts`) the `user: { id: { in: mentionedUserIds } }` filter is silently dropped, so the query returns **all** non-platform-admin members instead of only the mentioned users. This causes mention emails/notifications to be sent to the wrong recipient set.
You could make the helper safe to spread by wrapping its filter in `AND` so Prisma intersects it with any existing conditions rather than overwriting them.</violation>
</file>
<file name="apps/api/src/trigger/browser-automation/run-browser-automation.ts">
<violation number="1" location="apps/api/src/trigger/browser-automation/run-browser-automation.ts:47">
P2: The browser-automation task now performs two sequential organization queries before starting the parallel `task`/`members` fetch. `orgParticipantMemberWhere` calls `getOrgIsInternal`, which itself does `db.organization.findUnique({ select: { isInternal: true } })`. Because the first `findUnique` only selects `name`, the second query is unavoidable and executed serially, delaying all downstream work.
Consider either selecting `isInternal` alongside `name` in the first query, or moving the organization fetch back into the `Promise.all` (after `participantWhere`) so it runs in parallel with `task` and `members` rather than sequentially before them.</violation>
</file>
<file name="apps/app/src/trigger/tasks/task/weekly-task-reminder.ts">
<violation number="1" location="apps/app/src/trigger/tasks/task/weekly-task-reminder.ts:108">
P2: The `!isOwner` check alongside `isOrgParticipant` creates an inconsistency with the centralized participation predicate. `isOrgParticipant` only considers the global `user.role` and `orgIsInternal`, so the extra `isOwner` bypass means platform admins who hold an `owner` member role still receive digest emails in non-internal orgs — behavior that differs from four other call sites (`compliance.ts`, `SelectAssignee.tsx`, `policy-acknowledgment-digest-helpers.ts`, `generate-vendor-mitigation.ts`) which apply `isOrgParticipant` without an owner override. Because the PR’s goal is to centralize participation logic, this call site either should align with the predicate exactly, or the owner exception should be lifted into `isOrgParticipant` itself and documented so the rule is applied consistently everywhere.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
| return; | ||
| } | ||
|
|
||
| toast.success( |
There was a problem hiding this comment.
P1: After successfully toggling isInternal, the component only updates local useState. When an admin edits the currently active organization, the surrounding OrgInternalProvider in the server layout retains the old organization.isInternal value because the layout isn't refreshed. Downstream consumers such as useOrgIsInternal() (e.g., assignee pickers) will continue showing the stale flag until a full page reload.
The component already receives currentOrgId but does not use it. Consider calling router.refresh() after a successful update when org.id === currentOrgId, so the server layout re-renders and the provider reflects the new value. You will also need to import useRouter from next/navigation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx, line 52:
<comment>After successfully toggling `isInternal`, the component only updates local `useState`. When an admin edits the currently active organization, the surrounding `OrgInternalProvider` in the server layout retains the old `organization.isInternal` value because the layout isn't refreshed. Downstream consumers such as `useOrgIsInternal()` (e.g., assignee pickers) will continue showing the stale flag until a full page reload.
The component already receives `currentOrgId` but does not use it. Consider calling `router.refresh()` after a successful update when `org.id === currentOrgId`, so the server layout re-renders and the provider reflects the new value. You will also need to import `useRouter` from `next/navigation`.</comment>
<file context>
@@ -55,10 +49,32 @@ export function OrganizationDetail({
return;
}
+ toast.success(
+ next ? 'Background checks now required' : 'Background checks bypassed for this organization',
+ );
</file context>
| user: { OR: [{ role: { not: PLATFORM_ADMIN_ROLE } }, { role: null }] }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
P1: orgParticipantMemberWhere returns a bare spread fragment that overwrites existing user filters when callers already constrain the user relation. For non-internal orgs the returned { user: { OR: [...] } } replaces, rather than intersects with, any existing user key in the where object.
Concrete impact: in the mention notifiers (comment-mention-notifier.service.ts and task-item-mention-notifier.service.ts) the user: { id: { in: mentionedUserIds } } filter is silently dropped, so the query returns all non-platform-admin members instead of only the mentioned users. This causes mention emails/notifications to be sent to the wrong recipient set.
You could make the helper safe to spread by wrapping its filter in AND so Prisma intersects it with any existing conditions rather than overwriting them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/utils/org-participation.ts, line 51:
<comment>`orgParticipantMemberWhere` returns a bare spread fragment that overwrites existing `user` filters when callers already constrain the `user` relation. For non-internal orgs the returned `{ user: { OR: [...] } }` replaces, rather than intersects with, any existing `user` key in the `where` object.
Concrete impact: in the mention notifiers (`comment-mention-notifier.service.ts` and `task-item-mention-notifier.service.ts`) the `user: { id: { in: mentionedUserIds } }` filter is silently dropped, so the query returns **all** non-platform-admin members instead of only the mentioned users. This causes mention emails/notifications to be sent to the wrong recipient set.
You could make the helper safe to spread by wrapping its filter in `AND` so Prisma intersects it with any existing conditions rather than overwriting them.</comment>
<file context>
@@ -0,0 +1,53 @@
+): Promise<Prisma.MemberWhereInput> {
+ if (await getOrgIsInternal(organizationId)) return {};
+ return {
+ user: { OR: [{ role: { not: PLATFORM_ADMIN_ROLE } }, { role: null }] },
+ };
+}
</file context>
| <Switch | ||
| checked={isInternal} | ||
| disabled={savingInternal} | ||
| onCheckedChange={handleToggleInternal} |
There was a problem hiding this comment.
P2: The isInternal toggle changes core membership semantics for the whole organization, and the UI itself warns to leave it off for customer organizations. Because it fires immediately on a single click with no confirmation, an accidental toggle could incorrectly include platform admins in a customer org's compliance and assignment flows. Consider using an AlertDialog confirmation (already available in @trycompai/design-system and used for timeline deletion in TimelineCard.tsx) before enabling (and possibly disabling) this setting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/app/(app)/[orgId]/admin/organizations/[adminOrgId]/components/OrganizationDetail.tsx, line 135:
<comment>The `isInternal` toggle changes core membership semantics for the whole organization, and the UI itself warns to leave it off for customer organizations. Because it fires immediately on a single click with no confirmation, an accidental toggle could incorrectly include platform admins in a customer org's compliance and assignment flows. Consider using an `AlertDialog` confirmation (already available in `@trycompai/design-system` and used for timeline deletion in `TimelineCard.tsx`) before enabling (and possibly disabling) this setting.</comment>
<file context>
@@ -110,6 +119,25 @@ export function OrganizationDetail({
+ <Switch
+ checked={isInternal}
+ disabled={savingInternal}
+ onCheckedChange={handleToggleInternal}
+ aria-label="Internal organization"
+ />
</file context>
| // Use the shared participation rule so this path stays aligned with the | ||
| // other task notifiers: internal (platform-operated) orgs include platform | ||
| // admins; other orgs exclude them (except platform admins who are owners). | ||
| const organization = await db.organization.findUnique({ |
There was a problem hiding this comment.
P2: The browser-automation task now performs two sequential organization queries before starting the parallel task/members fetch. orgParticipantMemberWhere calls getOrgIsInternal, which itself does db.organization.findUnique({ select: { isInternal: true } }). Because the first findUnique only selects name, the second query is unavoidable and executed serially, delaying all downstream work.
Consider either selecting isInternal alongside name in the first query, or moving the organization fetch back into the Promise.all (after participantWhere) so it runs in parallel with task and members rather than sequentially before them.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/trigger/browser-automation/run-browser-automation.ts, line 47:
<comment>The browser-automation task now performs two sequential organization queries before starting the parallel `task`/`members` fetch. `orgParticipantMemberWhere` calls `getOrgIsInternal`, which itself does `db.organization.findUnique({ select: { isInternal: true } })`. Because the first `findUnique` only selects `name`, the second query is unavoidable and executed serially, delaying all downstream work.
Consider either selecting `isInternal` alongside `name` in the first query, or moving the organization fetch back into the `Promise.all` (after `participantWhere`) so it runs in parallel with `task` and `members` rather than sequentially before them.</comment>
<file context>
@@ -40,11 +41,15 @@ async function sendTaskStatusChangeEmails(params: {
+ // Use the shared participation rule so this path stays aligned with the
+ // other task notifiers: internal (platform-operated) orgs include platform
+ // admins; other orgs exclude them (except platform admins who are owners).
+ const organization = await db.organization.findUnique({
+ where: { id: organizationId },
+ select: { name: true },
</file context>
| .split(',') | ||
| .map((r) => r.trim()) | ||
| .includes('owner'); | ||
| if ( |
There was a problem hiding this comment.
P2: The !isOwner check alongside isOrgParticipant creates an inconsistency with the centralized participation predicate. isOrgParticipant only considers the global user.role and orgIsInternal, so the extra isOwner bypass means platform admins who hold an owner member role still receive digest emails in non-internal orgs — behavior that differs from four other call sites (compliance.ts, SelectAssignee.tsx, policy-acknowledgment-digest-helpers.ts, generate-vendor-mitigation.ts) which apply isOrgParticipant without an owner override. Because the PR’s goal is to centralize participation logic, this call site either should align with the predicate exactly, or the owner exception should be lifted into isOrgParticipant itself and documented so the rule is applied consistently everywhere.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/app/src/trigger/tasks/task/weekly-task-reminder.ts, line 108:
<comment>The `!isOwner` check alongside `isOrgParticipant` creates an inconsistency with the centralized participation predicate. `isOrgParticipant` only considers the global `user.role` and `orgIsInternal`, so the extra `isOwner` bypass means platform admins who hold an `owner` member role still receive digest emails in non-internal orgs — behavior that differs from four other call sites (`compliance.ts`, `SelectAssignee.tsx`, `policy-acknowledgment-digest-helpers.ts`, `generate-vendor-mitigation.ts`) which apply `isOrgParticipant` without an owner override. Because the PR’s goal is to centralize participation logic, this call site either should align with the predicate exactly, or the owner exception should be lifted into `isOrgParticipant` itself and documented so the rule is applied consistently everywhere.</comment>
<file context>
@@ -97,6 +99,21 @@ export const weeklyTaskReminder = schedules.task({
+ .split(',')
+ .map((r) => r.trim())
+ .includes('owner');
+ if (
+ !isOrgParticipant(member.user.role, {
+ orgIsInternal: org.isInternal,
</file context>
What
Adds an
Organization.isInternalflag so platform-operated organizations (e.g. our own) can run compliance with their platform-admin staff as real members — assignable, counted toward compliance progress, and included in notifications, reminders, and device rollups.For every other organization the behavior is unchanged: the flag defaults to
false, and with it off the new code returns exactly the same result as before. It's an admin-only toggle on the org admin screen.Why
Platform admins (global staff accounts) are intentionally excluded from an organization's day-to-day membership everywhere in the product. That's correct for the orgs they only enter to support. It's wrong for a platform-operated org, where those same accounts are the real members and need to run compliance like anyone else.
How
isOrgParticipant(userRole, { orgIsInternal }), in@trycompai/auth(new@trycompai/auth/participationsubpath so it stays dependency-light; mirrored in a dependency-free app helper for the Trigger.dev bundle).user.role === 'admin'participation checks across assignment/approval, compliance counting, notifications/reminders, and device queries with that single rule.isInternaladded to the admin org update endpoint + a "Platform settings → Internal organization" switch on the org detail screen.Data
20260708120000_add_organization_is_internaladdsisInternal BOOLEAN NOT NULL DEFAULT false.Tests
Rollout
Flag is off everywhere on merge. Enable per-org from the admin screen (or
PATCH /v1/admin/organizations/:id { isInternal: true }) only for platform-operated orgs.🤖 Generated with Claude Code
https://claude.ai/code/session_01UWG1YQ8KsjefFAvRV7pdtb
Summary by cubic
Treat platform admins as real members in internal (platform-operated) organizations via
Organization.isInternal, and centralize participation with a single rule applied across API, app, and Trigger.dev paths. Removes the previous “owner” exception for platform admins in customer orgs; internal orgs include everyone.New Features
isInternaladded to admin org update DTO and endpoint (explicitnullnow 400s).isOrgParticipantin@trycompai/auth/participation(mirrored in the app for Trigger.dev) replaces inline checks across assignments/approvals, compliance, notifications/reminders, device and Fleet endpoints, browser automation, and digest schedules.orgParticipantMemberWherenow exactly mirrors the predicate (excludes only platform admins for non-internal orgs; includes all for internal).Migration
Organization.isInternal BOOLEAN NOT NULL DEFAULT false(migration20260708120000_add_organization_is_internal).PATCH /v1/admin/organizations/:id { isInternal: true }.Written for commit 77df89c. Summary will update on new commits.