51 lines
5.1 KiB
Markdown
51 lines
5.1 KiB
Markdown
# Fix: api-migrations SurrealQL ledger CREATE silently dropped under SurrealDB v3
|
|
|
|
**Package**: `@suaveplan/api-migrations`
|
|
**Status**: PROPOSED
|
|
**Type**: bug-fix
|
|
|
|
## Why
|
|
|
|
`createSurrealqlAdapter`'s `insertApplied` issued `CREATE ${table}:$name SET ...`. SurrealDB v3's parser does NOT substitute `$name` in the record-id position — the token is consumed as a literal ident, the CREATE is accepted, but no row is created. The migration body still executes (it goes through `adapter.execute`) and the advisory lock works, but the ledger is silently empty. The runner therefore re-executes every "applied" migration on the next boot, defeating the entire purpose of the ledger.
|
|
|
|
Surfaced while shipping `@suaveplan/api-plugin-database`: the integration test confirmed the migration body applied, then read the ledger directly via `INFO FOR DB` and found nothing. A workaround was added in api-plugin-database; the underlying bug lives here.
|
|
|
|
The same shape is reused by `deleteApplied` (`DELETE ${table}:${id}`), which silently drops any id that isn't a bare SurrealQL ident, breaking rollback for filenames like `001_initial`.
|
|
|
|
## What changes
|
|
|
|
- MODIFIED: `createSurrealqlAdapter.insertApplied` — switched record-id production from `${table}:$name` (silently dropped under v3) to a backtick-quoted record-id literal `\`<table>:\`<escaped-id>\`\`` spliced into the SQL. Backticks inside the id are doubled. Field values stay parameterised via named bindings.
|
|
- MODIFIED: `createSurrealqlAdapter.deleteApplied` — same v3-safe record-id form.
|
|
- MODIFIED: `createSurrealqlAdapter.selectApplied` — `bareId` helper strips the `${table}:` prefix and any wrapping backticks from returned SurrealDB ids so the value that round-trips into `deleteApplied(table, id)` is the bare migration name. Object-form `RecordId { tb, id }` is unwrapped to its bare id when stringy.
|
|
- MODIFIED: `createSurrealqlAdapter.insertApplied` — `appliedAt` is now bound as the raw `Date` object so the SDK CBOR-encodes it as a SurrealDB `datetime`. The prior `record.appliedAt.toISOString()` form is rejected by v3 with `InternalError: Couldn't coerce value for field 'appliedAt'` (the DDL declares the column `TYPE datetime`). Second silent bug surfaced by the regression test.
|
|
- ADDED: `vitest.integration.config.ts` + `test:integration` script + `tests/integration/global-setup.ts` + `tests/integration/integration-setup.ts` mirroring the `@suaveplan/surrealdb` pattern.
|
|
- ADDED: `tests/integration/surrealql-ledger.integration.test.ts` — three live-SurrealDB-v3 regression tests (silent CREATE drop, second-run skip, rollback round-trip).
|
|
|
|
## Out of scope
|
|
|
|
- Postgres / sqlite adapter changes — only SurrealQL is affected.
|
|
- Adding a new error code for "invalid table identifier" — the runner only passes the configured tableName (default `_migrations`), and table-ident validation would require expanding `MIGRATION_CODES` which is a wider spec change. The fix splices `table` directly into the SQL exactly as the original code did; no new injection surface is introduced.
|
|
- Reverting the `INFO FOR DB` workaround inside `@suaveplan/api-plugin-database` — a different session is shipping that package and may want to keep the workaround until they re-verify. See `## Follow-up` below.
|
|
|
|
## Source mapping
|
|
|
|
This change is a bug fix against an already-shipped genesis package — no `../suaveplan/enterprise/` source applies. The shipped reference for the v3-safe record-id form is `packages/auth/better-auth-surrealdb/src/id-strategy/id-strategy.ts::prepareRecordId` (line 112), which produces the same backtick-quoted record-ref shape and is proven against the same `testing-integration-surrealdb` testcontainer.
|
|
|
|
The behavior of `$param` in SurrealDB v3 record-id position is documented in the SurrealQL CREATE statement reference (https://surrealdb.com/docs/surrealql/statements/create) — only bare idents or backticked / angle-bracketed literals are valid in the post-`:` position; parameter references are evaluated as values, not as id constructors.
|
|
|
|
## Verification approach
|
|
|
|
- `src/adapters/surrealql/surrealql.test.ts` — unit assertions on the emitted SurrealQL string (regression: `not.toContain("$name SET")`, positive: `toContain("CREATE _migrations:\`001\` SET")`, backtick escape, rowToRecord normalisation).
|
|
- `tests/integration/surrealql-ledger.integration.test.ts` — three real-SurrealDB-v3 tests verifying:
|
|
- the ledger row is queryable IN THE SAME CONNECTION after the migration body succeeds,
|
|
- a second `runFromDirectory` call is a no-op (ledger is read back),
|
|
- `rollback()` removes the ledger row written by `insertApplied`.
|
|
|
|
## Migration impact
|
|
|
|
Internal to the package — the `MigrationAdapter` public surface is unchanged. Consumers (`@suaveplan/api-plugin-database` and any direct callers) see corrected behavior on the next install. No breaking changes.
|
|
|
|
## Follow-up
|
|
|
|
`@suaveplan/api-plugin-database` currently uses an `INFO FOR DB` workaround at its full-boot integration test to confirm a migration applied without trusting the ledger. With this fix, that workaround is no longer necessary and the next touch on api-plugin-database can revert it. Out of scope for this change because a parallel session owns api-plugin-database.
|