Skip to content

Fix authPlugin's uuid7 id pin: config, not a hardcoded constant - #1550

Merged
borisno2 merged 2 commits into
mainfrom
claude/jolly-turing-7yazfu
Sep 13, 2026
Merged

Fix authPlugin's uuid7 id pin: config, not a hardcoded constant#1550
borisno2 merged 2 commits into
mainfrom
claude/jolly-turing-7yazfu

Conversation

@borisno2

Copy link
Copy Markdown
Member

Summary

authPlugin hardcoded db.idField: 'uuid7' on every Auth list it injects, with no escape hatch:

  • An app-level db.idField default (e.g. cuid2, set to match a live better-auth install whose id columns already hold non-uuid text ids) never reached the Auth lists — the generator kept emitting a pg/uuid column regardless, breaking adopt-better-auth-tables's whole reason for existing.

  • The extendList branch (for a list the app declares itself under one of the derived keys) never applied the pin at all, so it could silently end up on a different idField than the injected branch, while the adapter's supportsUUIDs: true / supportsNumericIds: false were hardcoded regardless of which branch actually produced the list.

  • authPlugin gains an idField?: 'uuid7' | 'cuid2' option. Resolution order: explicit authPlugin({ idField }) → the app's own db.idField default → 'uuid7'. 'int autoincrement' is refused at config time (the Auth adapter treats every id as a string).

  • The addList and extendList branches are now required to agree: an app-declared list whose own resolved idField disagrees with the plugin's throws, naming both values.

  • adoptBetterAuthTables() grows a matching idField option, so adopting a text/cuid2-shaped live install is one line: adoptBetterAuthTables({ idField: 'cuid2' }).

  • The adapter's supportsUUIDs/supportsNumericIds (authIdCapabilities) are now derived from the resolved strategy instead of hardcoded, so they can never contradict the emitted column type.

  • ADR-0048 and ADR-0060 amended in place; new ADR-0070 records the decision. Changeset added for @opensaas/stack-auth (minor).

Test plan

  • New/updated unit tests in packages/auth/tests/config.test.ts, adopt-better-auth-tables.test.ts, and a new adapter-id-capabilities.test.ts
  • pnpm test in packages/auth — 518 passed, 81 skipped (pre-existing skips)
  • pnpm test in packages/core — 1757 passed
  • pnpm typecheck and pnpm build (root, turbo) — all 16 tasks succeed
  • pnpm lint / pnpm format / pnpm manypkg fix — clean

Closes #1239

🤖 Generated with Claude Code

https://claude.ai/code/session_01218dPJQasRpbcWL9w9fiZ8


Generated by Claude Code

…ide it

authPlugin hardcoded db.idField: 'uuid7' on every Auth list it injects, with
no escape hatch, so an app-level db.idField default (e.g. cuid2, to adopt a
live better-auth install with text ids) never reached the Auth lists. The
extendList branch also never applied the pin, so an app-declared list could
silently disagree with the adapter's hardcoded supportsUUIDs/supportsNumericIds.

authPlugin now resolves idField from an explicit authPlugin({ idField }),
else the app's own db.idField default, else 'uuid7' — refusing a resolution
of 'int autoincrement' (the Auth adapter treats every id as a string) and
throwing when an app-declared list's own idField disagrees with the Auth
lists' resolved strategy. The adapter's supportsUUIDs/supportsNumericIds are
now derived from that resolved strategy instead of hardcoded.

Closes #1239

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01218dPJQasRpbcWL9w9fiZ8
@vercel

vercel Bot commented Sep 13, 2026

Copy link
Copy Markdown

Deployment failed for project stack-docs with the following error:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day").

Learn More: https://vercel.com/open-saas?upgradeToPro=build-rate-limit

@changeset-bot

changeset-bot Bot commented Sep 13, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: dd57cf8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@opensaas/stack-auth Minor
@opensaas/stack-cli Minor
@opensaas/stack-core Minor
@opensaas/stack-rag Minor
@opensaas/stack-storage-s3 Minor
@opensaas/stack-storage-vercel Minor
@opensaas/stack-storage Minor
@opensaas/stack-tiptap Minor
@opensaas/stack-ui Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copy link
Copy Markdown
Member Author

The Vercel status check on this PR (stack-docs deployment) is failing with:

Resource is limited - try again in 24 hours (more than 100, code: "api-deployments-free-per-day")

That's the org's Vercel free-tier daily deployment quota being exhausted, not anything in this diff — this PR touches no docs content, and the failure names a resource limit rather than a build error. Nothing here for this PR to fix; it should clear on its own once the daily quota resets (or if/when the project moves to a paid plan). The real CI checks (test, e2e (postgres), e2e (dev-database), example-gates) are still running — I'll keep watching those.


Generated by Claude Code

@borisno2 borisno2 left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Single-pass review (no Agent-tool fan-out / subagent verify pass) of PR #1550 (959b078).

Worked through the diff line-by-line, traced resolveAuthIdField/authIdCapabilities against resolveIdStrategy/listIdColumn in core, checked the extendList id-agreement refusal against every fallback combination (explicit authPlugin({idField}), app-wide default, per-list override, singleton, int autoincrement), read the ADR-0048/0060 amendments and the new ADR-0070, and built+ran the full packages/auth suite (518 passed, 81 pre-existing skips) plus the three directly-relevant test files to confirm no regression.

No correctness bugs found — the id-resolution logic, the addList/extendList agreement check, and the derived supportsUUIDs/supportsNumericIds are internally consistent and well covered by the new tests. Left two low-severity notes inline (a reuse/drift-risk observation and a minor error-message attribution nit); neither blocks the change.


Generated by Claude Code

Comment thread packages/auth/src/config/plugin.ts
Comment thread packages/auth/src/config/plugin.ts Outdated

Copy link
Copy Markdown
Member Author

Review: #1550 — authPlugin's id strategy resolution (fixes #1239)

I cloned the PR branch, read the full diff, and traced the resolution logic against resolveIdStrategy/listIdColumn in packages/core/src/contract/derive.ts and id-boundary.ts. Summary against the five points requested:

1. resolveAuthIdField and the addList/extendList agreement check — correct

resolveAuthIdField(idField, appDefault) computes idField ?? appDefault ?? 'uuid7' and refuses 'int autoincrement'. This is exactly core's resolveIdStrategy's fallback chain (listConfig.db?.idField ?? config.db.idField ?? 'uuid7', minus the singleton case, which doesn't apply here). I walked the fallback matrix (explicit authPlugin({ idField }), app-wide default only, neither set, an app-wide 'int autoincrement' default with no override) and each resolves as documented.

The addList branch applies the single resolved idField uniformly to every entry the loop iterates — not just the four base models, but also the optional RateLimit list and any plugin-declared table (e.g. MCP OAuth tables) — since derive-auth-lists.ts sets no db.idField of its own on any of them, so this is a real fix, not one scoped only to the four base models.

The extendList agreement check (listIdColumn(context.config, listName)?.strategy !== idField) correctly generalizes beyond the direct "app default is int autoincrement" case: if only a single app-declared list under a derived key sets its own db.idField: 'int autoincrement' while the global default is fine, the agreement check still catches it (the earlier resolveAuthIdField refusal only fires when the global default resolves badly). I confirmed by tracing both paths that these two checks compose without a gap.

One rough edge, not a bug: if the app's global db.idField is 'int autoincrement' but an app-declared list (e.g. User) already sets its own valid override (e.g. 'cuid2'), resolveAuthIdField still throws immediately on the bad global default before the agreement check ever runs — the developer must add authPlugin({ idField: 'cuid2' }) explicitly even though one list already has the "right" value. This is defensible (nothing tells the plugin what strategy the other, not-yet-declared Auth lists like Session/Account should use just because User happens to override), but worth being aware isn't caught by a more targeted error message.

2. authIdCapabilities and reading only the user model's strategy — safe

authIdCapabilities derives { supportsUUIDs: strategy === 'uuid7', supportsNumericIds: false } from listIdColumn(config, registry.user)?.strategy. Reading only the user model to represent all Auth lists is safe because of the agreement check in point 1: every list in authLists (base models, RateLimit, plugin tables) is guaranteed to resolve to the same idField — either it's freshly added with the uniformly-resolved value, or it pre-existed and was checked to agree with that same value, or config-time construction throws. There's no path where user and, say, Session could disagree at the point authIdCapabilities runs. supportsNumericIds: false is correctly unconditional since 'int autoincrement' can never survive resolution.

3. New tests exercise the actual bug — yes

  • inherits the app default idField on every injected list when authPlugin sets none directly regresses the original defect (previously hardcoded uuid7 silently overrode a cuid2 app default; this test would have failed against the pre-fix code).
  • refuses an app default that resolves to int autoincrement and refuses an app-declared list whose own idField disagrees with the Auth lists exercise the two new refusals, not just the happy path.
  • adopts a live install on a text/cuid2 id strategy via adoptBetterAuthTables({ idField }) is a reasonable integration-level check of the adoption flow authPlugin's uuid7 pin overrides an app-level db.idField and misses the extendList branch #1239 was actually about.
  • authIdCapabilities tests cover uuid7 (default), explicit cuid2, and app-wide cuid2 default — the three inputs that matter to that function.

These read as real regression coverage, not restatements of the implementation.

4. ADR consistency — consistent

ADR-0070's decision section, and the ADR-0048/0060 blockquote amendments, match the shipped code point-for-point: the resolution order, the 'int autoincrement' refusal, the extendList-never-touches-db invariant (ADR-0013 precedent), and authIdCapabilities reading the user model's strategy to speak for all Auth lists. I didn't find any claim in the ADRs the diff doesn't back up.

5. Edge cases — plugin-derived tables and RateLimit — no gap found

Confirmed by reading derive-auth-lists.ts and the addList/extendList loop in plugin.ts: the resolved idField is applied to every entry getAuthLists(...) returns, which already includes RateLimit and any better-auth-plugin-declared table, not just the four base models — so there's no special-casing gap for those. The extendList agreement check is likewise generic over listName, so an app that itself declares RateLimit (or a plugin table) under its derived key gets the same mismatch protection as User.

Minor, non-blocking notes (already left inline)

  1. packages/auth/src/config/plugin.ts:19resolveAuthIdField re-implements resolveIdStrategy's exact fallback chain rather than reusing it. I confirmed resolveIdStrategy itself isn't exported from @opensaas/stack-core (only listIdColumn, which needs an existing list config and can't answer "what would a not-yet-added list resolve to"), so this duplication is closer to unavoidable than sloppy given the current export surface — but it is a drift risk if core's fallback order ever changes. Worth a cross-reference comment or exporting resolveIdStrategy for reuse.
  2. packages/auth/src/config/plugin.ts:108 — the mismatch error always says the conflicting list "is declared by the application," but the check only tests context.config.lists[listName] truthiness. If an earlier-running plugin (not the application) registered that list key first, the message misattributes the conflict. Not a functional bug — just a wording nit if that scenario is reachable in practice.

Verdict

No blocking issues found. No regressions in the packages/auth test suite. The two notes above are optional polish, not required changes.


Generated by Claude Code

Cross-reference resolveIdStrategy in the reimplemented fallback chain, and
soften the extendList mismatch error to not assume the application (rather
than an earlier-running plugin) declared the conflicting list.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01218dPJQasRpbcWL9w9fiZ8
@vercel

vercel Bot commented Sep 13, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
stack-docs Ready Ready Preview Sep 13, 2026 12:13pm UTC

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Core Package Coverage (./packages/core)

Status Category Percentage Covered / Total
🟢 Lines 93.11% (🎯 81%) 3814 / 4096
🟢 Statements 91.1% (🎯 76%) 4304 / 4724
🟢 Functions 95.43% (🎯 78%) 837 / 877
🟢 Branches 85.77% (🎯 71%) 2894 / 3374
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for UI Package Coverage (./packages/ui)

Status Category Percentage Covered / Total
🔵 Lines 78.7% 244 / 310
🔵 Statements 78.43% 251 / 320
🔵 Functions 69.81% 74 / 106
🔵 Branches 67.51% 160 / 237
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for CLI Package Coverage (./packages/cli)

Status Category Percentage Covered / Total
🔵 Lines 82.28% 1992 / 2421
🔵 Statements 82% 2142 / 2612
🔵 Functions 87.91% 342 / 389
🔵 Branches 75.17% 1066 / 1418
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Auth Package Coverage (./packages/auth)

Status Category Percentage Covered / Total
🔵 Lines 91.42% 288 / 315
🔵 Statements 90.16% 321 / 356
🔵 Functions 96.1% 74 / 77
🔵 Branches 82.87% 271 / 327
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/auth/src/config/adopt-better-auth-tables.ts 100% 100% 100% 100%
packages/auth/src/config/plugin.ts 100% 100% 100% 100%
packages/auth/src/config/types.ts 0% 0% 0% 0%
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Storage Package Coverage (./packages/storage)

Status Category Percentage Covered / Total
🔵 Lines 90.11% 301 / 334
🔵 Statements 90.27% 334 / 370
🔵 Functions 96.9% 94 / 97
🔵 Branches 86.44% 319 / 369
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for RAG Package Coverage (./packages/rag)

Status Category Percentage Covered / Total
🔵 Lines 91.98% 597 / 649
🔵 Statements 91.35% 655 / 717
🔵 Functions 97.54% 119 / 122
🔵 Branches 85.65% 424 / 495
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)

Status Category Percentage Covered / Total
🔵 Lines 100% 40 / 40
🔵 Statements 100% 40 / 40
🔵 Functions 100% 9 / 9
🔵 Branches 100% 19 / 19
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)

Status Category Percentage Covered / Total
🔵 Lines 100% 68 / 68
🔵 Statements 100% 71 / 71
🔵 Functions 100% 15 / 15
🔵 Branches 97.87% 46 / 47
File CoverageNo changed files found.
Generated in workflow #2494 for commit dd57cf8 by the Vitest Coverage Report Action

@borisno2
borisno2 merged commit b79da07 into main Sep 13, 2026
8 checks passed
@borisno2
borisno2 deleted the claude/jolly-turing-7yazfu branch September 13, 2026 21:17
borisno2 pushed a commit that referenced this pull request Sep 13, 2026
…llision

0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md (#1550)
and 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md
(#1548) landed within a minute of each other and both claimed 0070,
tripping check:adr-duplicates on every PR against main.

Renumbered the id-strategy ADR to 0071 (2 filename-link updates in ADR-0048
and ADR-0060) rather than the client-identity one, which has 7+ scattered
bare "(ADR-0070)" comment references across packages/core, packages/cli and
packages/auth that would otherwise need updating instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6
borisno2 pushed a commit that referenced this pull request Sep 13, 2026
…llision

0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md (#1550)
and 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md
(#1548) landed within a minute of each other and both claimed 0070,
tripping check:adr-duplicates on every PR against main.

Renumbered the id-strategy ADR to 0071 (2 filename-link updates in ADR-0048
and ADR-0060) rather than the client-identity one, which has 7+ scattered
bare "(ADR-0070)" comment references across packages/core, packages/cli and
packages/auth that would otherwise need updating instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6
borisno2 added a commit that referenced this pull request Sep 13, 2026
…llision (#1555)

0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md (#1550)
and 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md
(#1548) landed within a minute of each other and both claimed 0070,
tripping check:adr-duplicates on every PR against main.

Renumbered the id-strategy ADR to 0071 (2 filename-link updates in ADR-0048
and ADR-0060) rather than the client-identity one, which has 7+ scattered
bare "(ADR-0070)" comment references across packages/core, packages/cli and
packages/auth that would otherwise need updating instead.


Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

Co-authored-by: Claude <noreply@anthropic.com>
borisno2 added a commit that referenced this pull request Sep 14, 2026
* Fix read-restricted embedding fields re-embedding on every write (#1282)

The generation hook's regeneration skip read the stored hash off the
write's own Field-Visibility-filtered `item`, which strips a field the
session cannot read. For a read-restricted embedding() field this hash
always read back undefined, so every later write regenerated the
embedding regardless of whether the source text changed.

Adds readPluginOwnedRow to @opensaas/stack-core/extend — the read-side
twin of writePluginOwnedField (ADR-0068) — so the RAG plugin can read
the row's real, persisted columns for the regeneration check instead
of the caller's projection of it.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UGrzpcxbUT9PCsvDRbTpgi

* docs(adr): renumber the auth idField ADR to 0071 to resolve a 0070 collision

0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md (#1550)
and 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md
(#1548) landed within a minute of each other and both claimed 0070,
tripping check:adr-duplicates on every PR against main.

Renumbered the id-strategy ADR to 0071 (2 filename-link updates in ADR-0048
and ADR-0060) rather than the client-identity one, which has 7+ scattered
bare "(ADR-0070)" comment references across packages/core, packages/cli and
packages/auth that would otherwise need updating instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

* Address review: classify the read refusal, fix the read-doc comment

- generation-failure.ts: HandlelessPluginFieldReadError was missing
  from the refused-access classification set, so the wiring defect it
  names would have mis-reported as a per-row transient failure instead
  of the deduplicated standing-defect message its write-side twin gets.
  Renamed REFUSED_WRITE_ERRORS/isRefusedWrite to
  REFUSED_ACCESS_ERRORS/isRefusedAccess now that it covers both sides.
- plugin.ts: readForRegenerationCheck's comment claimed it reads only
  the metadata column; readPluginOwnedRow has no column projection and
  always fetches the whole row. Corrected, and noted the added
  id-scoped read per write in the changeset.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UGrzpcxbUT9PCsvDRbTpgi

---------

Co-authored-by: Claude <noreply@anthropic.com>
borisno2 added a commit that referenced this pull request Sep 14, 2026
…n lane (#1554)

* fix(auth): route databaseHooks context.adapter through the transaction lane

better-auth hands a databaseHooks before/after hook a GenericEndpointContext
whose .context.adapter is the Auth adapter's ROOT instance — better-auth
never swaps that reference for the transaction-bound one (only its own
AsyncLocalStorage, read via getCurrentAdapter, does). A hook that queried
through it during sign-up therefore ran on the outer lane while the sign-up
transaction held the database's only connection: a hang on the Dev database,
and a read outside the transaction (surviving a rollback) on pooled Postgres.

The root instance now reads its lane from the same AsyncLocalStorage store
the transaction-bound instance does. Both instances run inside the same
boundLane.run(...) call for the life of the transaction, so a hook reaching
through context.context.adapter lands on the transaction-bound connection
without needing to touch better-auth's own ALS routing.

Closes #1252

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

* docs(adr): renumber the auth idField ADR to 0071 to resolve a 0070 collision

0070-the-auth-plugins-id-strategy-is-config-not-a-hardcoded-pin.md (#1550)
and 0070-the-generated-clients-identity-is-a-process-wide-registry-entry.md
(#1548) landed within a minute of each other and both claimed 0070,
tripping check:adr-duplicates on every PR against main.

Renumbered the id-strategy ADR to 0071 (2 filename-link updates in ADR-0048
and ADR-0060) rather than the client-identity one, which has 7+ scattered
bare "(ADR-0070)" comment references across packages/core, packages/cli and
packages/auth that would otherwise need updating instead.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

* docs(auth): drop the now-fixed databaseHooks-outside-transaction limit

docs/content/reference/auth.md's "Known limits" section and the pending
#1251 changeset (.changeset/silent-moons-gather.md) both still described
the databaseHooks-hook-runs-outside-the-transaction limit this PR closes.
Left unedited, both would have shipped in the same release as this PR's
own changeset claiming it fixed — a contradiction in the same changelog
entry.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

* fix(auth): doc-comment phrasing nit from review

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HEEWCWcKXFFYY3PXE2zBH6

---------

Co-authored-by: Claude <noreply@anthropic.com>
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.

authPlugin's uuid7 pin overrides an app-level db.idField and misses the extendList branch

2 participants