docs: canonical Keystone → stack migration guide (#490)#522
Conversation
Expand guides/migrating-from-keystone.md into the comprehensive, single source of truth for Keystone -> stack migration, folding in the specs/keystone-migration.md recipes and reflecting shipped Area A-E behaviour: - Field-type, hook (pipeline timing), and access mapping tables. - Generator parity (Area A): keystoneCompat empty-string text defaults, timestamps off by default + db.timestamps opt-in, bare singleton id, select db.isNullable/db.enumName, honoured defaultValue. - context.graphql.run -> context.db.* summarised; links the hardened migrate-context-calls skill recipes (#488) rather than duplicating. - Image/file (Area C): links the non-destructive multi-column recipe (#479). - Auth adoption (Area D): links the adoptBetterAuthTables() recipe (#483). - Configurable output paths (Area E, #485) for side-by-side coexistence. - Multi-schema preservation + admin-UI parity notes. Reduce specs/keystone-migration.md to a design-notes pointer at the published guide. Trim the duplicated context.graphql.run section and Keystone field table in guides/migration.md to point at the canonical guide (no stale duplicates). Make the canonical guide the first migration entry in the docs nav. Docs-only change; link-check, docs build, lint, and format:check pass. https://claude.ai/code/session_01ULd2HCT8dUve9aa9gKi6sX
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
borisno2
left a comment
borisno2
left a comment
There was a problem hiding this comment.
Review — canonical Keystone → stack migration guide (#490)
Verdict: REQUEST CHANGES (posted as a Comment review because GitHub blocks a formal Approve/Request-changes when the reviewing identity authored the PR). One factual defect (the bigInt() mapping) should be fixed before merge; everything else is accurate and the consolidation is sound.
I cross-checked every config/API snippet against the shipped code on main (commit c97fb46). The vast majority verified clean. Findings below, ordered by severity.
🔴 Must fix
1. bigInt() → integer({ db: { nativeType: 'BigInt' } }) is incorrect (field-type table §2).
This is the author's open question #4, and the answer is: no, this is not the recommended mapping and as written it produces an invalid schema.
integer().getPrismaType()(packages/core/src/fields/index.ts:192-225) returns Prisma scalar typeIntand, whendb.nativeTypeis set, appends@db.<nativeType>→ so this emitssomeField Int? @db.BigInt.@db.BigIntis not a valid native-type attribute for the PrismaIntscalar. Prisma models BigInt as a scalar type (BigInt, which maps to JSbigint), not as a@db.annotation onInt.Int @db.BigIntwill failprisma validate/generate(P1012) on Postgres/MySQL/SQLite.- Even if it generated,
integer().getTypeScriptType()always returnstype: 'number'(line 230), so you'd silently losebigintprecision in TS. - The stack's own migration tooling does not treat this as the BigInt path:
packages/cli/src/migration/introspectors/prisma-introspector.ts:203maps PrismaBigInt→text()with the comment// No native support, andmigration-generator.ts:527special-casesBigInt/Decimal/Bytes. There is nobigInt()builder.
Recommendation: don't present integer({ db: { nativeType: 'BigInt' } }) as the mapping. Either (a) state that there is no native BigInt field and the tool maps it to text() (matching the introspector), and/or (b) recommend db.extendPrismaSchema to emit a real BigInt scalar column if the live DB has one. As written it will mislead migrators into an un-generatable schema.
🟡 Worth addressing
2. float() → decimal() mapping changes the column type (field-type table §2).
Keystone's float() is a Float (double) column; decimal() emits Decimal @db.Decimal(p,s) (fields/index.ts:342-378). Mapping float → decimal is a destructive column-type change, which cuts against the guide's central "Schema parity / no destructive changes" promise. There is no float() builder in the stack, so some workaround is unavoidable — but the table should flag that Float → Decimal is a type change (consider decimal or extendPrismaSchema/nativeType to keep a real float), not present it as a clean same-shape mapping like the rows above it.
🟢 Verified accurate (no change needed)
Spot-checked and confirmed against source:
- Generator parity (Area A):
db.keystoneCompat(empty-string non-null text default, opt-in, explicitdefaultValuewins, nullable/non-text untouched) —config/types.ts:1582,fields/index.ts:120-129.db.timestampsglobal + per-list override +P1012dedupe —types.ts:1369,1554. Bare singletonid Int @id— matches.selectdb.type/db.enumName(default<List><Field>)/db.isNullablesemantics and the→ status AccountNoteStatusType? @default(open)example output —types.ts:643-700,fields/index.ts:854-915. HonoureddefaultValue→@default(...)on text/integer/json/checkbox/decimal/timestamp — confirmed. - Query API (Area B):
defineFragment/runQuery/runQueryOne/ResultOf/RelationSelectorall exist with the documented signatures and the nested{ query, where, orderBy, take, skip }shape (query/index.ts), and are exported from the root@opensaas/stack-core(src/index.ts) — so the guide'simport { defineFragment, type ResultOf } from '@opensaas/stack-core'is correct. - Output paths (Area E):
OutputConfig.prismaSchema(defaultprisma/schema.prisma) andopensaasDir(default.opensaas) matchtypes.ts:2087; exported from root. - Auth (Area D) & image/file (Area C): correctly linked, not duplicated.
adoptBetterAuthTables()exists (packages/auth/src/config/adopt-better-auth-tables.ts). Image/file use@opensaas/stack-storage/fieldswithdb.columns: 'keystone'(storage/src/fields/index.ts). The "7 image columns / 3 file columns" claim and theavatar_url/resume_filename… example names matchIMAGE_COLUMN_PARTS/FILE_COLUMN_PARTSexactly (storage/src/utils/multi-column.ts). - Mapping tables: Access §4 —
itemis provided to operation access on update/delete (access/types.ts:270,304), so theaccess.item.*row is right. Hook §7 — the read pipeline (3 steps: DB → field access control → fieldresolveOutput) is correct; reads do not runafterOperation(context/index.tsfindUnique/findMany only filter fields + resolveOutput). Note this means the repo's rootCLAUDE.mdread-order list (which adds a 4th "field afterOperation" step) is the stale one — the guide is right. - Consolidation soundness: single canonical page;
migration.mdtrimmed to pointers while preserving the## Migrating context.graphql.runheading (line 877), socore-concepts/queries.md's two#migrating-contextgraphqlrunanchors (lines 14, 178) still resolve.specs/keystone-migration.mdreduced to a pointer + topic map; nav updated. - Tooling: CLI
MIGRATION_GUIDE_URL===https://stack.opensaas.au/docs/guides/migrating-from-keystone(commands/migrate.ts:23, asserted in tests),--with-ai/--type keystoneare valid flags, and/plugin install opensaas-migration@opensaas-stack-marketplacematches the marketplace (.claude-plugin/marketplace.json). Themigrate-context-callsRecipe 1–4 summaries (where-shape, connect/disconnect/set: [], "never write scalar FK — silently stripped", gql.tada/VariablesOf→factory, fragment→include + null-on-access-denied) all faithfully match the skill'sSKILL.md. - Links: all four ADR
blob/mainURLs (0004–0007) and bothspecs/*.mdtargets exist on disk.
Recommendations on the four flagged open questions
- Is
migrating-from-keystone.mdthe right canonical page? — Yes. The CLI'sMIGRATION_GUIDE_URLalready points at exactly this page (and a test locks the value), so extending it rather than restructuring aroundmigration.mdis the correct call. - Keep vs delete the
## Migrating context.graphql.runpointer section inmigration.md? — Keep it.core-concepts/queries.mdlinks into#migrating-contextgraphqlrunin two places; the heading must survive or those anchors 404. Leaving it as a short pointer is the right move. - GitHub
blob/mainURLs tospecs/anddocs/adr/. — Fine to keep. All targets exist and these aren't docs-site pages, so blob URLs are appropriate (link-check correctly doesn't validate them). Minor optional nit:blob/mainURLs aren't version-pinned, so they can drift if a file is renamed — acceptable for design-notes/ADR references. bigInt()mapping. — Not correct as written (see 🔴 #1). Needs revision before this ships as the public source of truth.
Summary: Strong, accurate, well-consolidated guide — the Area A–E technical content, query API, links, and tooling references all check out against shipped code. The single blocking item is the bigInt() field mapping (#1), which would lead a migrator to a schema Prisma rejects; the float()→decimal() row (#2) should also be caveated. Fix those two and this is good to merge. Holding for human approval per the HITL note.
https://claude.ai/code/session_01ULd2HCT8dUve9aa9gKi6sX
Generated by Claude Code
…#490) The migration guide's field-type table documented Keystone bigInt() → integer({ db: { nativeType: 'BigInt' } }), which is invalid: it emits Int @db.BigInt, a combination Prisma rejects (@db.BigInt is not a valid native type for the Int scalar), and the TS type stays number anyway. Rewrite the bigInt row to match what the stack actually does. The migration introspector (packages/cli/src/migration/introspectors/prisma-introspector.ts:203) maps Prisma BigInt → text() with a "No native support" comment and warns the field is mapped to text(); there is no bigInt() builder. Document that honestly, including the string-storage precision caveat. Also caveat the float() → decimal() row: there is no float() builder and decimal() is a type change (Float → Decimal, number → decimal.js Decimal), not byte-for-byte parity — call out the precision/behaviour difference instead of presenting a clean 1:1 mapping. Ran prettier --write on both touched docs to satisfy format:check. https://claude.ai/code/session_01ULd2HCT8dUve9aa9gKi6sX
|
Follow-up accuracy fix pushed to
I grounded the corrected mapping in the migration introspector:
This also supersedes the open "please confirm that's the recommended approach" question in the PR description for Checks (re-run on this branch after the change):
Note: https://claude.ai/code/session_01ULd2HCT8dUve9aa9gKi6sX Generated by Claude Code |
Coverage Report for Core Package Coverage (./packages/core)
File CoverageNo changed files found. |
Coverage Report for UI Package Coverage (./packages/ui)
File CoverageNo changed files found. |
Coverage Report for CLI Package Coverage (./packages/cli)
File CoverageNo changed files found. |
Coverage Report for Auth Package Coverage (./packages/auth)
File CoverageNo changed files found. |
Coverage Report for Storage Package Coverage (./packages/storage)
File CoverageNo changed files found. |
Coverage Report for RAG Package Coverage (./packages/rag)
File CoverageNo changed files found. |
Coverage Report for Storage S3 Package Coverage (./packages/storage-s3)
File CoverageNo changed files found. |
Coverage Report for Storage Vercel Package Coverage (./packages/storage-vercel)
File CoverageNo changed files found. |
2 participants
Implements #490
Part of #486
HITL — holds for human review/approval before merge
This is the public face of migration and spans every area, so per #490 it is human-in-the-loop: an agent drafted it, but the EM reviews and approves before merge. Do not merge without human approval.
What changed (docs-only)
Promotes a single canonical, published Keystone → stack migration guide as the source of truth, folding in the
specs/keystone-migration.mdrecipes and reflecting the shipped Area A–E behaviour.docs/content/guides/migrating-from-keystone.md— expanded from a thin landing page into the comprehensive canonical guide. Covers: overview, config translation, field-type mapping table, generator parity (Area A) —db.keystoneCompatempty-string text defaults, auto-timestamps OFF by default +db.timestampsopt-in (incl. per-list override +P1012dedupe), bare singletonid Int @id,selectdb.type/db.enumName/db.isNullable, honoureddefaultValue; access mapping; hook mapping with the real list/field pipeline timing;context.graphql.run→context.db.*summary that links the hardenedmigrate-context-callsskill recipes (Harden migrate-context-calls skill: where-shape, connect/disconnect, gql.tada, include/select (with fixtures) #488); image/file (Area C) links the non-destructive multi-column recipe (Rewrite image/file migration guidance to lead non-destructive; document Vercel Blob #479); auth adoption (Area D) links theadoptBetterAuthTables()recipe ("Adopt existing better-auth tables" preset + docs (app User vs auth identity linking) #483); configurable output paths (Area E, Configurable generator output paths: output config block + path resolver #485); multi-schema preservation + admin-UI parity; checklist; CLI + plugin tooling.specs/keystone-migration.md— reduced to a short design-notes pointer at the published guide, with a table mapping each topic (A–E) to where it is now maintained.docs/content/guides/migration.md— trimmed the duplicatedcontext.graphql.runrecipes and the partial KeystoneJS field table to short pointers at the canonical guide (no stale duplicates); kept it as the general multi-source AI-assisted walkthrough.docs/lib/navigation.ts— made "Migrating from Keystone" the first migration entry; relabelled the general one "Migration Guide (all sources)".Source of truth & link-only sections
guides/migrating-from-keystone.md. The CLI (MIGRATION_GUIDE_URL) already points at it.API verification
Every config/API snippet was cross-checked against shipped code on
main:packages/core/src/config/types.tsandpackages/core/src/fields/index.ts(db.keystoneCompat,db.timestampsglobal + per-list,selectdb.type/db.enumName/db.isNullable,db.schemas/db.schema/db.map,OutputConfig).packages/core/src/query/index.ts(defineFragment,runQuery/runQueryOne,ResultOf,RelationSelector).adoptBetterAuthTables()vspackages/authand the auth guide's adoption section.db.columns: 'keystone'vsspecs/keystone-image-migration.md+ ADR-0006.migrate-context-callsrecipe summaries vs the skill'sSKILL.md.packages/cli/src/generator/output-paths.ts.Checks
pnpm install --prefer-offline— cleanpnpm --filter opensaas-stack-docs link-check) — pass (5 required pages resolve, internal links in 4 migration docs valid)next build, runs link-check prebuild) — pass (all pages generated, incl./docs/guides/migrating-from-keystone)pnpm lint— pass (0 errors; only pre-existing warnings in untouched files)pnpm format:check— passNo changeset/plugin-version bump: docs-only, no
packages/*orclaude-plugins/*changes.For the reviewer to focus on
migrating-from-keystone.mdas canonical (rather than restructuring aroundmigration.md) because the CLI and link-check already treat it as canonical. Confirm that's the intended page.migration.mdtrimming: I kept the## Migrating context.graphql.runheading (now a short pointer) socore-concepts/queries.md's existing#migrating-contextgraphqlrunanchor link still resolves to a real section. Confirm you're happy keeping that anchor vs removing the section entirely.specs/anddocs/adr/: these point to GitHubblob/main/...URLs (they aren't docs-site pages, so the link-check doesn't verify them). Worth a sanity check that those paths are the intended targets.bigInt()mapping: there is no dedicatedbigInt()builder, so I documentedinteger({ db: { nativeType: 'BigInt' } })as the mapping — please confirm that's the recommended approach.https://claude.ai/code/session_01ULd2HCT8dUve9aa9gKi6sX
Generated by Claude Code