Skip to content

improvement(admin): update defaults for better UX - #6112

Merged
icecrasher321 merged 5 commits into
stagingfrom
improvement/admin-defs
Jul 31, 2026
Merged

improvement(admin): update defaults for better UX#6112
icecrasher321 merged 5 commits into
stagingfrom
improvement/admin-defs

Conversation

@icecrasher321

@icecrasher321icecrasher321 commented Jul 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

Admin routes dependent changes + perf improvements pass

Type of Change

  • Other: Code cleanup + Perf improvments

Testing

N/A

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercelBot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
docsSkippedSkippedJul 31, 2026 2:53am

Request Review

@cursor

cursorBot commented Jul 31, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches concurrent org membership, invitation state, credential creation, billing webhooks, and admin workspace moves—incorrect locking or compensation could deny access, double-grant seats, or leave partial billing state.

Overview
Hardens organization membership and workspace lifecycle mutations against races by introducing a shared organization → user billing identity → membership lock sequence (acquireOrganizationUserMutationLocks) and re-reading authoritative context after locks before writing.

Credential creation now plans workspace context inside the transaction, acquires those locks, re-validates write access (403/409 if org or permissions changed), and seeds memberships from the locked context. Personal env credential sync runs in one transaction behind the same user-identity fence with chunked inserts/deletes.

Workspace creation replaces ad-hoc billing-identity checks with lockWorkspaceCreationContext aligned to transfer/removal fencing; invitation accept/update/reject/revoke paths claim invitation and workspace advisory locks, then authorize from protected re-reads (lockInvitationForMutation, revokeInvitationAsAdmin, updateInvitation). Direct workspace grants use the same ordering and revoke pending grants in-transaction instead of post-hoc cancel. Failed invitation emails compensate with expected updatedAt / organization id guards; outbox adds coalesced reschedule for pending events on the same subject.

Admin workspace moves accept optional expectedOwnerId; attachable personal workspace selection uses ownedAttachableWorkspacesWhere. Seat counting excludes pending invites whose email already belongs to any organization. External org access removal clears per-user usage caps in the removal transaction. Enterprise webhook retries when Stripe create races ahead of paused-collection provisioning.

Smaller changes: chunkArray centralized in @sim/utils/helpers; subscription reads can forUpdate in caller transactions; invitation acceptance maps unexpected transaction failures to server-error instead of leaking throws.

Reviewed by Cursor Bugbot for commit 1f26f7f. Configure here.

@icecrasher321icecrasher321 changed the title improvement(admin): uupdate defaults for better UXimprovement(admin): update defaults for better UXJul 31, 2026
@greptile-apps

greptile-appsBot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR strengthens transactional consistency across administrative workspace, invitation, credential, billing, and cleanup operations.

  • Centralizes invitation updates, revocation, acceptance, and workspace-grant changes behind transactional helpers and coordinated locks.
  • Adds owner validation and invitation, credential, storage-payer, permission, and outbox migration handling to administrative workspace moves.
  • Reworks credential creation to re-authorize the caller after acquiring organization and user mutation locks.
  • Introduces batched cleanup utilities and updates log, soft-delete, task, billing, and outbox cleanup paths.
  • Consolidates workspace authorization and organization-membership behavior while expanding concurrency and rollback tests.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains in the eligible follow-up review scope.

Important Files Changed

FilenameOverview
apps/sim/lib/invitations/core.tsCentralizes invitation acceptance, updates, revocation, authorization, and concurrency control into transactional mutation flows.
apps/sim/lib/workspaces/admin-move.tsExpands administrative workspace moves to coordinate ownership validation, invitation migration, storage billing, permissions, credentials, and outbox work.
apps/sim/app/api/credentials/route.tsRe-authorizes credential creation under shared organization and user mutation locks before inserting credentials and memberships.
apps/sim/lib/billing/organizations/membership.tsAdds shared organization-user locking and coordinates membership mutations with invitation and workspace state.
apps/sim/lib/cleanup/batch-delete.tsIntroduces reusable bounded batch-selection and deletion machinery used by the revised cleanup jobs.
apps/sim/lib/core/outbox/service.tsUpdates outbox processing and coalescing behavior to support deferred administrative and invitation side effects.

Sequence Diagram

sequenceDiagram
participant Admin as Admin/API caller
participant Service as Transactional service
participant Locks as Advisory and row locks
participant DB as PostgreSQL
participant Outbox as Outbox worker
Admin->>Service: Move workspace or mutate invitation
Service->>Locks: Acquire invitation/workspace/org locks
Locks-->>Service: Mutation scope serialized
Service->>DB: Re-read authoritative state
Service->>Service: Re-authorize and validate expected state
alt State remains valid
Service->>DB: Apply coordinated mutations
Service->>Outbox: Enqueue post-commit work
DB-->>Admin: Commit result
else State changed or authorization lost
Service-->>Admin: Conflict or forbidden response
end
Loading

Reviews (2): Last reviewed commit: "refactor: address review nits on the adm..." | Re-trigger Greptile

Correct the attach lock-order comment. The order matches admin move, and what
makes it mandatory is invitation acceptance: it holds `workspace-invitations:<id>`
while waiting for the workspace row, so row-locking first (as this did)
deadlocks against it. The previous ownership-transfer justification did not
hold — that path takes the organization lock before its workspace rows too, so
the two agree on order rather than inverting.
Drop `cancelInvitation`. The `revokeInvitationAsAdmin` extraction left it with
no callers, and an unlocked, unauthorized `status = 'cancelled'` flip sitting
next to the fenced replacement is easy to reach for by mistake.
Drop the unused `executor` parameters from `hasWorkspaceAdminAccess` and
`isOrganizationAdminOrOwner`. No caller threads a transaction through either,
and the former goes back to delegating to `checkWorkspaceAccess` instead of
re-deriving the same permission itself.
Import `chunkArray` from `@sim/utils/helpers` everywhere and remove the
re-export from `batch-delete.ts`, so the symbol has one source rather than a
non-barrel shim plus the package.
Restore the bounded attachability check in `addDashboardOrganizationMember`:
scope the query to the selected ids instead of listing every attachable
workspace and scanning that array per selection.
Resolve the credential-creation permission through
`getEffectiveWorkspacePermission` rather than a second copy of the org-admin
derivation ladder, so the rule cannot drift from the shared resolver.
Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

bugbot run

@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

@greptile

Comment threadapps/sim/lib/billing/webhooks/enterprise.ts
`revokeInvitationWorkspaceGrant` lost its only caller when the DELETE route
moved to `revokeInvitationAsAdmin`, leaving a locked wrapper nothing invoked.
Remove it and fold its documentation into `revokeInvitationWorkspaceGrantTx`,
which direct grants and scoped revocation still call. The grant-revocation test
now drives the transactional form directly, so the sibling-grant and
final-grant-cancels behaviour it covers stays under test.
`isSameOrgMember` has had no caller since before this branch — direct grant
resolves membership through `getUserOrganization` inside its own transaction —
so it and its tests go too.
`getWorkspaceMembership` is no longer imported outside its module now that
credential creation reads `getCredentialCreationWorkspaceContext`; make it
module-private rather than leave it on the public surface.
Co-Authored-By: Claude <noreply@anthropic.com>
Each was checked by hand across every file type, including barrel re-exports
and string references, rather than taken from a static analyzer.
`isUserMemberOfOrganization` has no reference anywhere.
`reapplyPaidOrgJoinBillingForExistingMember` only ever ran from two
lock-ordering tests. The transaction-enlisted form it delegated to is what the
subscription webhooks call and what those tests actually assert on, so they now
drive it directly. The assertions are unchanged: the wrapper contributed a
transaction, an organization lock and a membership existence check, none of
which appear in the recorded operations.
`replaceLargeValueReferences` and `replaceLargeValueReferencesWithClient` are
both thin wrappers over `replaceLargeValueReferenceKeysWithClient`, which
execution logging, human-in-the-loop resume and the trace backfill all still
call. The single test covering a wrapper now composes the key collection itself
and targets that live helper.
Co-Authored-By: Claude <noreply@anthropic.com>
@icecrasher321

Copy link
Copy Markdown
CollaboratorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 1f26f7f. Configure here.

@icecrasher321
icecrasher321 merged commit 04a8c0a into stagingJul 31, 2026
26 checks passed
@waleedlatif1
waleedlatif1 deleted the improvement/admin-defs branch August 1, 2026 18:01
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@icecrasher321