Skip to content

fix: keep membership owners atomic - #1481

Merged
kang-heewon merged 3 commits into
trunkfrom
fix/1463-atomic-owner-mutation
Jul 25, 2026
Merged

fix: keep membership owners atomic#1481
kang-heewon merged 3 commits into
trunkfrom
fix/1463-atomic-owner-mutation

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Jul 25, 2026

Copy link
Copy Markdown
Member

Outcome

Membership owner removal, demotion, and ownership transfer now use explicit atomic store contracts. Competing mutations serialize per tenant, preserve at least one owner, and map last-owner or serialization losers to stable membership Problems instead of allowing a validation/write race.

The in-memory and Drizzle adapters implement the contracts. Drizzle locks the current owner set and uses conditional DML, including a single ownership-transfer update. A digest-pinned PostgreSQL 16.10 service now runs the real READ COMMITTED, REPEATABLE READ, and SERIALIZABLE concurrency suite in the required CI validation job.

Custom MembershipStore adapters must implement the new atomic mutation and transfer methods; a minor changeset and generated API documentation record that compatibility requirement. The validation-only MembershipOwnerGuard is deprecated for write paths.

Fixes #1463

Verification

  • @croco/membership-core — 77/77 tests
  • @croco/membership-drizzle — 22/22 unit tests
  • PostgreSQL 16.10 concurrency suite — 5/5 tests across READ COMMITTED, REPEATABLE READ, and SERIALIZABLE
  • Affected dependency build — 28/28 tasks
  • Package lint and typecheck — passed
  • Public API surface — 111 package snapshots match
  • Generated API docs — 112/112 build tasks; drift check passed
  • Repository verification — passed, 21/22 applicable gates
  • Pre-push workspace tests — 226/226 tasks
  • Pre-push workspace typecheck — 225/225 tasks
  • Changeset-required gate — affected packages covered

Review gates

  • Correctness and regression: PASS — deterministic barriers prove concurrent removal and demotion allow one winner; a stale-role transfer schedule cannot remove the new final owner; real PostgreSQL sessions prove row-lock behavior and stable Problem mapping at all supported isolation levels.
  • API, security, compatibility, and release: PASS — required store methods and discriminated results are exported, snapshots and generated docs agree, serialization failures are explicit conflicts, the validation-only guard is deprecated, and minor changesets cover both published packages.
  • Maintainability and minimality: PASS — both store implementations and the duplicate service/manager entry points share the same contract; changes are limited to membership behavior, focused tests, CI enforcement, release metadata, and generated membership docs.
  • Independent adversarial review: APPROVE — the final review cleared atomicity, stale reads, transfer semantics, Problem mapping, public API, changesets, documentation, and required PostgreSQL CI enforcement.

Residual risk

Direct consumers of low-level save or delete can still bypass owner invariants; write paths that enforce ownership must use mutateOwner or transferOwnership. Custom persistent adapters must provide equivalent tenant-scoped serialization and normalize serialization failures to conflict.

@coderabbitai

coderabbitai Bot commented Jul 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@kang-heewon, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ed24400d-bd12-4a46-9cb6-8d5fdf5e6f7e

📥 Commits

Reviewing files that changed from the base of the PR and between 96b6b80 and 949f9f5.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (30)
  • .changeset/atomic-membership-owners.md
  • .github/workflows/ci.yml
  • packages/docs/src/content/docs/api/membership-core/src/classes/InMemoryMembershipStore.md
  • packages/docs/src/content/docs/api/membership-core/src/classes/MembershipOwnerGuard.md
  • packages/docs/src/content/docs/api/membership-core/src/classes/MembershipService.md
  • packages/docs/src/content/docs/api/membership-core/src/classes/MembershipStore.md
  • packages/docs/src/content/docs/api/membership-core/src/type-aliases/MembershipOwnerMutationInput.md
  • packages/docs/src/content/docs/api/membership-core/src/type-aliases/MembershipOwnerMutationResult.md
  • packages/docs/src/content/docs/api/membership-core/src/type-aliases/MembershipOwnershipTransferInput.md
  • packages/docs/src/content/docs/api/membership-core/src/type-aliases/MembershipOwnershipTransferResult.md
  • packages/docs/src/content/docs/api/membership-drizzle/src/classes/DrizzleMembershipStore.md
  • packages/membership-core/README.md
  • packages/membership-core/src/index.ts
  • packages/membership-core/src/libs/InMemoryMembershipStore.ts
  • packages/membership-core/src/libs/MembershipManager.ts
  • packages/membership-core/src/libs/MembershipOwnerGuard.ts
  • packages/membership-core/src/libs/MembershipService.ts
  • packages/membership-core/src/libs/MembershipStore.ts
  • packages/membership-core/src/libs/types.ts
  • packages/membership-core/src/tests/MembershipManager.spec.ts
  • packages/membership-core/src/tests/MembershipService.spec.ts
  • packages/membership-core/src/tests/MembershipStore.spec.ts
  • packages/membership-drizzle/README.md
  • packages/membership-drizzle/package.json
  • packages/membership-drizzle/src/libs/DrizzleMembershipStore.ts
  • packages/membership-drizzle/src/tests/DrizzleMembershipStore.postgres.spec.ts
  • packages/membership-drizzle/src/tests/DrizzleMembershipStore.spec.ts
  • public-api-surface.snapshot.json
  • scripts/tests/ci-workflow.spec.ts
  • scripts/workflow-verification-contract.mts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/1463-atomic-owner-mutation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kang-heewon
kang-heewon force-pushed the fix/1463-atomic-owner-mutation branch from a54f5e9 to 9a697ca Compare July 25, 2026 06:33
@github-actions

github-actions Bot commented Jul 25, 2026

Copy link
Copy Markdown

📊 Benchmark Results

✅ All benchmarks passed

Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 8.8μs 30.0ms 8.2μs +7.1% -
CrocoApp lambdaHandler (10 controllers) 367.4μs 50.0ms 258.4μs +42.1% -
Lambda cold-start simulation 443.7μs 80.0ms 418.1μs +6.1% -
Lambda cold-start with headers 402.3μs 80.0ms 369.7μs +8.8% -
Lambda cold-start with binary body 374.0μs 80.0ms 339.1μs +10.3% -
Lambda cold-start with query params 322.7μs 80.0ms 301.3μs +7.1% -
Lambda cold-start with authorizer context 321.2μs 80.0ms 299.8μs +7.1% -
Lambda cold-start realistic scenario 322.5μs 80.0ms 299.2μs +7.8% -
EventBusConfig.start (10 handlers) 1.5μs 10.0ms 1.4μs +7.7% -
EventPublisher.publishNow single event 1.8μs 2.0ms 1.7μs +6.5% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.1μs -11.2% -
Container.get singleton (cold) 80.0μs 5.0ms 70.3μs +13.9% -
Container.register × 50 components 3.4ms 10.0ms 3.2ms +6.1% -
Container.validate (50 components) 3.9ms 20.0ms 3.4ms +14.4% -
Container.get singleton (warm) 1.6μs 500.0μs 1.6μs -1.8% -
TelemetryRuntime.init (lambda preset) 2.3μs 200.0ms 1.1ms -99.8% -
lambdaPreset config creation 1.5μs 2.0ms 1.4μs +5.0% -

Updated: 2026-07-25T07:17:35.157Z · Commit: 9bfe554

@kang-heewon
kang-heewon force-pushed the fix/1463-atomic-owner-mutation branch from b271cfc to 949f9f5 Compare July 25, 2026 07:08
@kang-heewon
kang-heewon merged commit 239c077 into trunk Jul 25, 2026
15 of 16 checks passed
@kang-heewon
kang-heewon deleted the fix/1463-atomic-owner-mutation branch July 25, 2026 08:25
Sign up for free to 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.

[membership-core] Make last-owner validation and removal atomic

1 participant