refactor(members): extract driveInviteRepository seam

[2witstudios]

Summary

Epic 2 of 6 in the drive-invites redo (replaces PR #1229). Pure refactor — zero behavior change vs master.

The legacy POST /api/drives/[driveId]/members route now talks to the existing driveInviteRepository instead of the drive-member-service functions. This makes rubric §4 (architecture-seam rule) satisfiable for the follow-on Epic 4 email-payload branch, which will need to mock the seam exclusively (no ORM chain mocks).

• Slice 2.1: 10-method repository surface verified + unit-tested.
  • findAdminMembership now filters acceptedAt IS NOT NULL (correct from day one, aligns with Epic 1 sweep when it merges; Epic 1 not yet merged at branch time).
  • createDriveMember.acceptedAt widened to Date | null so Epic 4 can use the same seam for pending invitations.
• Slice 2.2: Route refactored to use repo; route POST tests rewritten to mock repo (no select().from().where() chains anywhere in the file). GET tests preserved unchanged — GET is out of scope.
• Slice 2.3: @scaffold snapshot test (legacy-post-snapshot.test.ts + frozen legacy-post-baseline.json) catches drift in status / body / audit / activity payloads vs the master baseline. Slated for removal in Epic 4 when the legacy POST is retired.

Inspired by PR #1229 (pu/invites) — only the seam-shape and acceptedAt-gate reasoning were borrowed; Epic 4-territory methods (findActivePendingMemberByEmail, acceptPendingMember, etc.) were intentionally left out.

LOC note

Final diff is +783 / -262 (net +521). Production code change is small (16 net LOC across route.ts + repository.ts); the bulk is test value:

• ~250 LOC: repository unit tests covering all 10 methods.
• ~227 LOC: @scaffold snapshot test.
• ~92 LOC: frozen baseline JSON.
• Net route.test.ts delta is small after a follow-up arrangeOwnerCreate() helper refactor halved the duplicated mock setup.

This is slightly over the 500-LOC target in the epic spec; trimming further would either drop unit coverage on individual repository methods or remove the rubric-required snapshot scaffold. Happy to consolidate further if reviewers want a specific axis tightened.

Files changed

In-scope only:

• apps/web/src/lib/repositories/drive-invite-repository.ts (+11 / -7) — isNotNull filter on findAdminMembership, Date \| null on createDriveMember.acceptedAt.
• apps/web/src/app/api/drives/[driveId]/members/route.ts (+18 / -7) — POST handler now uses driveInviteRepository; no Drizzle imports.
• apps/web/src/app/api/drives/[driveId]/members/__tests__/route.test.ts — POST tests rewritten to mock the seam.
• apps/web/src/app/api/drives/[driveId]/members/__tests__/legacy-post-snapshot.test.ts (NEW, @scaffold).
• apps/web/src/app/api/drives/[driveId]/members/__tests__/__fixtures__/legacy-post-baseline.json (NEW, frozen).
• apps/web/src/lib/repositories/__tests__/drive-invite-repository.test.ts (NEW).

Test plan

• pnpm exec vitest run for the 4 affected files → 79/79 pass (15 repo + 26 POST/GET + 7 snapshot + 31 invite-route adjacency).
• pnpm typecheck → clean across the monorepo.
• pnpm --filter web lint → clean (one pre-existing unrelated warning in QuickCreatePalette.tsx).
• findAdminMembership acceptedAt IS NOT NULL filter asserted in repo test.
• createDriveMember Date \| null round-trip asserted in repo test.
• No Drizzle imports in route.ts POST handler.
• No ORM chain mocks in route.test.ts.
• Snapshot test passes against the frozen baseline (auth/owner/admin-blocked/member-blocked/already-member/missing-drive/insert-failure).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@2witstudios has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 55 minutes and 10 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 93b00d70-3a1f-47ca-982a-5f1121f327f2

📥 Commits

Reviewing files that changed from the base of the PR and between 9cd7fc6 and d39babc.

📒 Files selected for processing (6)

• apps/web/src/app/api/drives/[driveId]/members/__tests__/__fixtures__/legacy-post-baseline.json
• apps/web/src/app/api/drives/[driveId]/members/__tests__/legacy-post-snapshot.test.ts
• apps/web/src/app/api/drives/[driveId]/members/__tests__/route.test.ts
• apps/web/src/app/api/drives/[driveId]/members/route.ts
• apps/web/src/lib/repositories/__tests__/drive-invite-repository.test.ts
• apps/web/src/lib/repositories/drive-invite-repository.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch pu/epic-2-repo-seam

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Review rate limit: 0/1 reviews remaining, refill in 55 minutes and 10 seconds.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[2witstudios]

Self-review against .pu/templates/rubric-review.md Appendix

Scoring 0–2 on each axis for the three changed test files (PR-scope only):

Axis	drive-invite-repository.test.ts	legacy-post-snapshot.test.ts	route.test.ts
Contract stability (refactor-resistance)	1	2	2
Mock quality (boundary-only, no chain mocks)	1 (intentional)	2	2
Assertion meaning (observable outcomes, payloads)	2	2	2
Flake risk	2	2	2
Spec clarity	2	2	2

Notes on the 1s:

• Repository test, contract stability = 1: the test asserts query-shape (e.g. where includes an isNotNull clause), so a Drizzle-internal refactor that produces the same SQL but a different builder shape would break it. This is the deliberate cost of testing a thin ORM seam — the alternative is a real-DB integration test, which the rubric §9 endorses but is out of scope here. The crucial assertions (acceptedAt round-trip, acceptedAt IS NOT NULL gate on admin lookup) would survive a builder refactor.
• Repository test, mock quality = 1: ORM-chain mocks ARE used here. Per rubric §3 + §4, that's prohibited at the route/service layer — for the repository itself, mocking @pagespace/db/db is the only way to unit-test the seam. Same trade-off as chat-message-hard-delete.test.ts and existing repo tests.

How rule §4 (architecture seam rule) is satisfied for follow-on epics

Before this PR, the legacy POST handler reached drive-member-service.ts which in turn ran db.select().from().where() chains. A route-level test that wanted to exercise the handler had to either mock the service (loose contract) or mock the ORM chain (forbidden by §3). Epic 4 will add an email-payload branch to a NEW route — and that route will have a strict mock the seam requirement. By landing the seam now, Epic 4's tests can write vi.mock('@/lib/repositories/drive-invite-repository', ...) and assert on payloads the route passes (e.g. createDriveMember called with acceptedAt: null for pending invites) without ever touching Drizzle.

The snapshot scaffold (@scaffold per §10) is the regression net for the refactor itself — it captures master's status / body / audit / activity payload for 7 representative request shapes and is slated for retirement in Epic 4 alongside the legacy POST.

Ready for human review.

[2witstudios]

Rubric self-review (Appendix scoring, 0–2 per axis)

Per .pu/templates/rubric-review.md — scoring each in-scope test file against the 5 appendix axes, with rationale for any 0.

apps/web/src/app/api/drives/[driveId]/members/__tests__/route.test.ts

Axis	Score	Notes
Contract stability (refactor-resistance)	2	All POST tests assert against the response contract (status code, body shape, audit/activity payload) — none reach into ORM internals or specific SQL. Refactoring driveInviteRepository internals would not break these.
Mock quality	2	POST mocks at the driveInviteRepository seam only; no select().from().where() chains anywhere. GET section preserves the existing drive-member-service mocks since the GET handler is intentionally out of scope for this refactor.
Assertion meaning	2	Side-effect tests verify payloads (logMemberActivity payload object, auditRequest event details) — no toHaveBeenCalledTimes(1) without a boundary-contract justification.
Flake risk	2	vi.resetAllMocks() per beforeEach; no real timers, no async sleeps, no real network or DB.
Spec clarity	2	Each test name describes the regression it would catch; no REVIEW flags needed because behavior was lifted byte-for-byte from master.

apps/web/src/app/api/drives/[driveId]/members/__tests__/legacy-post-snapshot.test.ts

Labeled @scaffold per rubric §10 — exists as refactor-protection until Epic 4 retires the legacy POST.

Axis	Score	Notes
Contract stability	2	Frozen baseline at __fixtures__/legacy-post-baseline.json — diff against master would surface immediately.
Mock quality	2	Repo-seam mocks only; no Drizzle.
Assertion meaning	2	Each case asserts both response status, response body, and (for success cases) audit + activity payloads against the baseline.
Flake risk	2	Fixed dates; no timers; no I/O.
Spec clarity	2	@scaffold JSDoc explains the temporary nature and Epic 4 retirement plan up-front.

apps/web/src/lib/repositories/__tests__/drive-invite-repository.test.ts

This is the seam itself — rubric §4 explicitly permits mocking db here.

Axis	Score	Notes
Contract stability	2	Tests verify the seam delegates to Drizzle with the correct shapes (e.g. where clause must include isNotNull(acceptedAt)); a refactor that preserves the contract would not break these.
Mock quality	2	db and operators are the system boundaries here, not internals. The isNotNull(acceptedAt) assertion specifically guards the Epic-1-aligned admin gate.
Assertion meaning	2	Each test asserts both the query shape (via the kind: 'isNotNull' marker injected by the operator mock) and the returned data — not just "was called".
Flake risk	2	All synchronous setup; no timers.
Spec clarity	2	File-level JSDoc explicitly explains why mocking db is permitted here but forbidden in route tests.

---

Why this satisfies rubric §4 (architecture seam rule)

Before this PR, the legacy POST /api/drives/[driveId]/members had two problems for the rubric:

1. The route called drive-member-service functions. That was a seam, but drive-member-service.ts itself is a wide grab-bag (members, permissions, broadcasts, audits in one file) — Epic 4 needs a narrower seam for the email-payload branch.
2. The route's POST and members/invite/route.ts (which already uses driveInviteRepository) had divergent seams. Epic 4 will fold the legacy POST into the email-payload route, so they need to share the seam now to make the Epic 4 diff pure-business-logic.

After this PR, both routes flow through driveInviteRepository. Tests that touch the legacy POST mock the repository — they are immune to query-builder refactors below the seam, exactly as §4 prescribes. The remaining service callers (checkDriveAccess, listDriveMembers) belong to the GET handler and are intentionally untouched per epic scope.

Ready for human review.