From 0ec14b8b62319cb30119c6dbaee78221a9b0fcaa Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Tue, 28 Jul 2026 22:45:50 +0500 Subject: [PATCH 01/20] fix(nextly): keep the recording generation monotonic across resets --- .../__tests__/recording-policy.test.ts | 23 +++++++++++++++++++ .../src/domains/webhooks/recording-policy.ts | 8 ++++++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts b/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts index 2573324cc..de9a6075d 100644 --- a/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts +++ b/packages/nextly/src/domains/webhooks/__tests__/recording-policy.test.ts @@ -2,6 +2,7 @@ import { afterEach, describe, it, expect } from "vitest"; import { applyStoredRecordingDecisions, + currentRecordingGeneration, isRecordingDisabledByConfig, clearWebhookRecording, isWebhookRecordingEnabled, @@ -194,4 +195,26 @@ describe("webhook recording policy", () => { expect(isRecordingDisabledByConfig("collection", "posts")).toBe(false); expect(isRecordingDisabledByConfig("collection", "never-seen")).toBe(false); }); + + it("rejects a refresh captured before a reset, even when the counter collides", () => { + // ABA: a background read captures generation N, then `clearServices()` + // resets and the next boot performs the SAME number of writes, so a + // zero-based counter lands back on exactly N. The stale snapshot — taken + // against the previous adapter and config — would then pass the guard and + // wipe every `db` decision the new boot had just published. + // + // Constructed so both epochs reach the identical count; that collision is + // the whole point, and without a monotonic counter this test fails. + resetWebhookRecordingPolicy(); + setWebhookRecording("collection", "old-epoch", false, "db"); + const captured = currentRecordingGeneration(); + + resetWebhookRecordingPolicy(); + setWebhookRecording("collection", "enquiries", false, "db"); + + applyStoredRecordingDecisions([], captured); + + // The stale snapshot was discarded, so the new boot's opt-out survives. + expect(isWebhookRecordingEnabled("collection", "enquiries")).toBe(false); + }); }); diff --git a/packages/nextly/src/domains/webhooks/recording-policy.ts b/packages/nextly/src/domains/webhooks/recording-policy.ts index f02ad0b96..11dce8474 100644 --- a/packages/nextly/src/domains/webhooks/recording-policy.ts +++ b/packages/nextly/src/domains/webhooks/recording-policy.ts @@ -294,6 +294,12 @@ export function resetWebhookRecordingPolicy(): void { storedRefresh.refresher = null; storedRefresh.readAtMs = 0; storedRefresh.queued = false; - storedRefresh.generation = 0; + // Advance rather than zero. A refresh that captured generation N before this + // reset may still be in flight; zeroing would let the next boot's ordinary + // increments climb back to N, so that stale read — taken against the previous + // adapter and config — would pass the guard and replace every `db` decision + // the new boot had just published, re-enabling recording for an opted-out + // entity. A counter that only ever increases can never be matched again. + storedRefresh.generation += 1; storedRefresh.now = () => Date.now(); } From cd0773af7603baf0b57d1dcd4e09d575b5f332cb Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Tue, 28 Jul 2026 23:09:13 +0500 Subject: [PATCH 02/20] fix(nextly): stop localized writes overwriting default-locale content --- .changeset/localized-write-companion-guard.md | 27 +++ ...nc-localized-companion.integration.test.ts | 174 ++++++++++++++ packages/nextly/src/cli/commands/db-sync.ts | 8 + packages/nextly/src/cli/commands/dev-build.ts | 77 ++++++ .../nextly/src/cli/commands/dev-watcher.ts | 7 + .../services/collection-mutation-service.ts | 26 ++ ...rite-without-companion.integration.test.ts | 227 ++++++++++++++++++ .../src/domains/i18n/runtime/companion-io.ts | 20 +- .../services/single-mutation-service.ts | 28 +++ 9 files changed, 590 insertions(+), 4 deletions(-) create mode 100644 .changeset/localized-write-companion-guard.md create mode 100644 packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts create mode 100644 packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts diff --git a/.changeset/localized-write-companion-guard.md b/.changeset/localized-write-companion-guard.md new file mode 100644 index 000000000..0696b8644 --- /dev/null +++ b/.changeset/localized-write-companion-guard.md @@ -0,0 +1,27 @@ +--- +"nextly": patch +"create-nextly-app": patch +"@nextlyhq/admin": patch +"@nextlyhq/admin-css": patch +"@nextlyhq/blocks-engine": patch +"@nextlyhq/ui": patch +"@nextlyhq/adapter-drizzle": patch +"@nextlyhq/adapter-postgres": patch +"@nextlyhq/adapter-mysql": patch +"@nextlyhq/adapter-sqlite": patch +"@nextlyhq/storage-s3": patch +"@nextlyhq/storage-uploadthing": patch +"@nextlyhq/storage-vercel-blob": patch +"@nextlyhq/plugin-form-builder": patch +"@nextlyhq/plugin-page-builder": patch +"@nextlyhq/plugin-seo": patch +"@nextlyhq/plugin-sdk": patch +"@nextlyhq/eslint-config": patch +"@nextlyhq/prettier-config": patch +"@nextlyhq/telemetry": patch +"@nextlyhq/tsconfig": patch +--- + +Enabling localization on a collection that already had content could silently overwrite the default language. `nextly db:sync` flips the collection to localized in its own process, so a running app could believe the collection was localized — showing the language switcher — before its translations table existed. Saving a translation then wrote over the original-language values and changed the entry's URL, while reporting success. + +`db:sync` and the dev config watcher now create the translations table in the same run, for collections, singles and components alike. If that table is somehow still missing, a write in a non-default language is refused with a clear message instead of overwriting content. Writing the default language before the table exists is unchanged. diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts new file mode 100644 index 000000000..9e6a8acf7 --- /dev/null +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -0,0 +1,174 @@ +/** + * `nextly db:sync` must create the companion `_locales` table in the SAME run. + * + * The push pipeline deliberately does not manage companion tables + * (`managed-tables.isCompanionTable`), and `ensureCompanionTable` — written as + * the db:sync/dev-boot counterpart to migration-owned creation — used to be + * called only at boot. Because `db:sync` runs in its own CLI process, it flipped + * the registry's `localized` flag and left companion creation to whenever the app + * next started. A server already running then read `localized: 1`, registered the + * companion in its runtime registry, and rendered the whole localization UI over + * a table that did not exist. Writes in that window overwrote the default + * language. + * + * So the assertion is specifically that the table exists when the sync sequence + * returns, not that it exists eventually. + * + * This drives the real `syncCollections` → `syncSingles` → `syncComponents` → + * `ensureLocalizedCompanions` sequence against a real SQLite file, because + * ORDER is the thing that broke: a companion carries a foreign key to its main + * table, so running the hook before singles and components are pushed creates + * nothing for them. What it does NOT prove is that `db-sync.ts` and + * `dev-watcher.ts` still call the hook — that wiring is a single line in each, + * checked by reading them. + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { + defineCollection, + defineConfig, + defineSingle, + text, +} from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { getDialectTables } from "../../../database/index"; +import { SchemaRegistry } from "../../../database/schema-registry"; +import { createLogger } from "../../utils/logger"; +import { + ensureLocalizedCompanions, + syncCollections, + syncComponents, + syncSingles, +} from "../dev-build"; +import { ensureCoreTables } from "../dev-server"; + +import type { DrizzleAdapter } from "@nextlyhq/adapter-drizzle/types"; +import type { CLIDatabaseAdapter } from "../../utils/adapter"; +import type { CommandContext } from "../../program"; +import type { LoadConfigResult } from "../../utils/config-loader"; +import type { ResolvedDevOptions } from "../db-sync"; + +let dir: string; +let adapter: Awaited> | undefined; +let previousDialect: string | undefined; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "nextly-dbsync-companion-")); + previousDialect = process.env.DB_DIALECT; +}); + +afterEach(async () => { + await adapter?.disconnect(); + adapter = undefined; + // Integration files share one fork, so a dialect left set here would be read + // by every file that follows. + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; + rmSync(dir, { recursive: true, force: true }); +}); + +/** Physical presence, probed the same way the write path probes it. */ +async function tableExists(name: string): Promise { + try { + await adapter?.executeQuery(`SELECT 1 FROM "${name}" LIMIT 0`); + return true; + } catch { + return false; + } +} + +/** The command's own sequence, minus the parts that read from disk. */ +async function runSync(config: LoadConfigResult["config"]): Promise { + process.env.DB_DIALECT = "sqlite"; + adapter = await createAdapter({ + type: "sqlite", + url: `file:${join(dir, "test.db")}`, + } as Parameters[0]); + + const logger = createLogger({ quiet: true }); + const options = { cwd: dir, autoSync: true } as ResolvedDevOptions; + const context = { logger, options: {}, cwd: dir } as CommandContext; + // The real adapter is a DrizzleAdapter; `CLIDatabaseAdapter` declares only the + // handful of methods the command signatures need, so the command file converts + // between them exactly like this. + const cli = adapter as unknown as CLIDatabaseAdapter; + const drizzleAdapter = adapter as unknown as DrizzleAdapter; + + // Without a resolver the ORM cannot address `dynamic_collections`, so the sync + // fails before it reaches anything this test is about. + const registry = new SchemaRegistry("sqlite"); + registry.registerStaticSchemas(getDialectTables("sqlite")); + drizzleAdapter.setTableResolver(registry); + await ensureCoreTables(cli, options, context); + + const configResult = { config } as LoadConfigResult; + await syncCollections(configResult, cli, options, context); + await syncSingles(configResult, cli, options, context); + await syncComponents(configResult, cli, options, context); + await ensureLocalizedCompanions(config, cli, context); +} + +describe("db:sync creates localized companion tables in-process (integration)", () => { + it("creates a localized collection's companion in the same run", async () => { + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + collections: [ + defineCollection({ + slug: "posts", + localized: true, + fields: [text({ name: "title", localized: true })], + }), + ], + }) + ); + + expect(await tableExists("dc_posts")).toBe(true); + // The assertion that fails without the fix: the push pipeline creates the + // main table and nothing in this process creates the companion. + expect(await tableExists("dc_posts_locales")).toBe(true); + }); + + it("creates a localized single's companion, which needs singles synced first", async () => { + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + singles: [ + defineSingle({ + slug: "homepage", + localized: true, + fields: [text({ name: "headline", localized: true })], + }), + ], + }) + ); + + // Singles use the `single_` prefix (`resolve-entity-table.GROUPS`). Asserting + // the main table too means a prefix change cannot make the companion check + // vacuously pass against a name nothing ever creates. + expect(await tableExists("single_homepage")).toBe(true); + expect(await tableExists("single_homepage_locales")).toBe(true); + }); + + it("leaves a non-localized collection with no companion", async () => { + await runSync( + defineConfig({ + collections: [ + defineCollection({ + slug: "logs", + fields: [text({ name: "title" })], + }), + ], + }) + ); + + expect(await tableExists("dc_logs")).toBe(true); + // Creating companions unconditionally would strand a dead table in every + // project that does not use localization. + expect(await tableExists("dc_logs_locales")).toBe(false); + }); +}); diff --git a/packages/nextly/src/cli/commands/db-sync.ts b/packages/nextly/src/cli/commands/db-sync.ts index abc965a94..d391d75e4 100644 --- a/packages/nextly/src/cli/commands/db-sync.ts +++ b/packages/nextly/src/cli/commands/db-sync.ts @@ -85,6 +85,7 @@ import { import { formatDuration } from "../utils/logger"; import { + ensureLocalizedCompanions, performPermissionSeeding, syncCollections, syncComponents, @@ -302,6 +303,13 @@ export async function runDbSync( await syncSingles(configResult, adapter, options, context); await syncComponents(configResult, adapter, options, context); + // Step 5.6: Create the `_locales` companion of every localized entity, which + // the push pipeline does not manage. Runs after all three syncs because the + // companion references its main table. Without this, `db:sync` left the + // registry saying "localized" with no table to hold translations until the + // app next booted, and writes in that window overwrote the default language. + await ensureLocalizedCompanions(configResult.config, adapter, context); + if (collectionCount === 0) { logger.warn("No collections defined in config"); logger.info("Add collections to your nextly.config.ts to get started."); diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index a5fae2e5c..c5138d529 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -761,3 +761,80 @@ async function handleRemovedComponents( } } } + +/** + * Create the companion `_locales` table for every localized collection, single + * and component in the config. + * + * The push pipeline deliberately does not manage companion tables (see + * `managed-tables.isCompanionTable`), and `ensureCompanionTable` — the intended + * db:sync/dev-boot counterpart to migration-owned creation — was only ever called + * at boot. Because `db:sync` runs in its own CLI process, it flipped the + * registry's `localized` flag and left creation to whenever the app next started; + * a server already running then rendered the whole localization UI over a table + * that did not exist. + * + * MUST be called after `syncCollections`, `syncSingles` AND `syncComponents`: + * the companion carries a foreign key to its main table, so running it earlier + * fails for whichever entity types have not been pushed yet. Idempotent — + * `ensureCompanionTable` returns early when the table is already there — so + * running it on every sync costs one probe per localized entity. + */ +export async function ensureLocalizedCompanions( + config: LoadConfigResult["config"], + adapter: CLIDatabaseAdapter, + context: CommandContext +): Promise { + const { logger } = context; + const dialect = adapter.getCapabilities().dialect; + const { ensureCompanionTable } = await import( + "../../domains/i18n/runtime/companion-io" + ); + const { resolveEntityTable } = await import( + "../../domains/i18n/migration/resolve-entity-table" + ); + + const groups = [ + config.collections ?? [], + config.singles ?? [], + config.components ?? [], + ]; + + for (const group of groups) { + for (const raw of group) { + const entity = raw as { + slug?: string; + localized?: boolean; + status?: boolean; + fields?: { name: string; type: string; localized?: boolean }[]; + }; + if (!entity.slug || entity.localized !== true) continue; + const resolved = resolveEntityTable(config, entity.slug); + if (!resolved) continue; + // `ensureCompanionTable` resolves even on failure (a first boot may not have + // the main table yet), so the reporter — not a try/catch — is what surfaces + // a real problem. + // `CLIDatabaseAdapter` under-declares the runtime object, which is a real + // DrizzleAdapter — the same conversion this file already uses for the sync + // service above. `ensureCompanionTable` needs `executeQuery`, which the + // declared CLI interface omits. + await ensureCompanionTable( + adapter as unknown as DrizzleAdapter, + { + slug: entity.slug, + tableName: resolved.tableName, + fields: entity.fields ?? [], + dialect, + status: entity.status === true, + }, + error => { + logger.warn( + `Could not create the translations table for "${entity.slug}" (${resolved.companionTableName}). ` + + `Writes in a non-default locale will be refused until it exists: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + } + ); + } + } +} diff --git a/packages/nextly/src/cli/commands/dev-watcher.ts b/packages/nextly/src/cli/commands/dev-watcher.ts index f2f64ff10..70dba2bc4 100644 --- a/packages/nextly/src/cli/commands/dev-watcher.ts +++ b/packages/nextly/src/cli/commands/dev-watcher.ts @@ -16,6 +16,7 @@ import type { LoadConfigResult } from "../utils/config-loader"; import type { ResolvedDevOptions } from "./db-sync"; import { + ensureLocalizedCompanions, performPermissionSeeding, syncCollections, syncComponents, @@ -72,6 +73,12 @@ export function createDebouncedSync( await syncSingles(configToSync, adapter, options, context); await syncComponents(configToSync, adapter, options, context); + // Turning on localization is a config edit, so it arrives through this + // watcher as often as through `db:sync`. The companion table is not part of + // the push pipeline, and creating it here rather than at the next boot is + // what keeps a running server from advertising localization it cannot store. + await ensureLocalizedCompanions(configToSync.config, adapter, context); + // Sync user_ext table (always — handles both code and UI fields) await syncUserFields(configToSync, adapter, options, context); diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index ca2c63395..1a200a70f 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -942,6 +942,32 @@ export class CollectionMutationService extends BaseService { // `migrate`, the dev auto-sync leaves localized columns on the MAIN table (Option B), so // writes must go there — return null and let the localized values flow to main as today. if (!(await this.companionTableExists(companion.companionTableName))) { + // CREATE is unaffected: a new entry has no other language's values to lose, + // so its translatable values legitimately sit on the main table until the + // companion exists — the documented pre-migration fallback. + // + // UPDATE is where content dies. The row already holds the default + // language's values on main, so letting a NON-default locale fall through + // overwrites them with the translation and regenerates the slug from it, + // silently and with a success response. The window is real: `db:sync` + // flips the registry's `localized` flag in its own process while the + // running server has yet to create the companion. Refuse rather than + // destroy. + const requested = resolveRequestedLocale(this.localization, locale); + if (!isCreate && requested !== this.localization.defaultLocale) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this collection yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + collection: collectionName, + locale: requested, + defaultLocale: this.localization.defaultLocale, + companionTable: companion.companionTableName, + }, + }); + } return null; } diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts new file mode 100644 index 000000000..8434bd924 --- /dev/null +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -0,0 +1,227 @@ +/** + * A localized write with no companion table must not destroy content. + * + * The dangerous state is a TRANSITION: a collection created WITHOUT localization + * keeps its translatable columns on the main table, and enabling localization + * later moves them to `dc__locales`. `nextly db:sync` runs in its own + * process — it flips the registry's `localized` flag but historically left + * companion creation to the next boot — so a running server can believe the + * collection is localized, render the full locale switcher, and still have no + * companion table. + * + * Before this guard, a write in a NON-default locale fell through to the main + * table in that window, overwriting the default language's values and + * regenerating the slug from the translation, while reporting success. + * Reproduced on published alpha.43 during the task 006 walk. + * + * The default locale legitimately writes to the main table until the companion + * exists ("Option B"), so that path must keep working unchanged. + * + * File-backed SQLite: `createTestNextly` disconnects the adapter as it boots, so + * an in-memory database would not survive the second boot that performs the + * transition. + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { defineCollection, text } from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { + createTestNextly, + getConfiguredTestDialects, + type TestNextly, +} from "../../../plugins/test-nextly"; + +import type { CollectionsHandler } from "../../../services/collections-handler"; + +let dir: string; +let dbPath: string; +let current: TestNextly | undefined; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "nextly-i18n-window-")); + dbPath = join(dir, "test.db"); +}); + +afterEach(async () => { + await current?.destroy(); + current = undefined; + rmSync(dir, { recursive: true, force: true }); +}); + +const localization = { locales: ["en", "es"], defaultLocale: "en" }; + +/** Same collection, with localization off (columns on main) or on (companion). */ +const posts = (localized: boolean) => + defineCollection({ + slug: "posts", + localized, + fields: [text({ name: "title", localized: true })], + }); + +async function boot(localized: boolean): Promise { + process.env.DB_DIALECT = "sqlite"; + const adapter = await createAdapter({ + type: "sqlite", + url: `file:${dbPath}`, + } as Parameters[0]); + return createTestNextly({ + adapter, + collections: [posts(localized)], + localization, + }); +} + +/** + * Read the PHYSICAL main-table title. Once the collection is localized the + * registered runtime schema omits translatable columns, so `adapter.select` + * would not return the column that the bug overwrites — the raw read is the + * only thing that shows what actually landed on disk. + */ +async function physicalTitle(handle: TestNextly): Promise { + const rows = await handle.adapter.executeQuery<{ title: string }>( + "SELECT title FROM dc_posts LIMIT 1" + ); + return rows[0]?.title; +} + +/** Re-open in the localized state, then remove the companion to recreate the window. */ +async function enterWindow(): Promise { + await current?.destroy(); + current = undefined; + const handle = await boot(true); + await handle.adapter.executeQuery("DROP TABLE IF EXISTS dc_posts_locales"); + return handle; +} + +describe("localized write without a companion table (integration)", () => { + it("refuses a non-default-locale write instead of overwriting the default", async () => { + // Boot 1: not localized, so `title` lives on the main table. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "posts", overrideAccess: true }, + { title: "How to Build a Blog" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + current = await enterWindow(); + + const translated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "posts", + entryId: id, + overrideAccess: true, + locale: "es", + }, + { title: "Cómo crear un blog" } + ); + + // The write must not report success, and it must be OUR refusal rather than + // a driver error escaping — a raw "no such table" would also be a failure + // here while meaning the guard never ran. + expect(translated.success).toBe(false); + expect(translated.statusCode).toBe(409); + expect(translated.message).toMatch(/Translations are not ready/); + + // ...and the default-language content must be untouched. This is the + // assertion that fails without the guard: `es` landed on the main row. + expect(await physicalTitle(current)).toBe("How to Build a Blog"); + }); + + it("still writes the default locale to the main table when no companion exists", async () => { + // "Option B" preserved: before the companion is created the default language + // legitimately lives on the main table, so this path must be unchanged. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "posts", overrideAccess: true }, + { title: "First" } + ); + const id = (created.data as { id: string }).id; + + current = await enterWindow(); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "posts", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "First (edited)" } + ); + + expect(updated.success).toBe(true); + expect(await physicalTitle(current)).toBe("First (edited)"); + }); +}); + +/** + * The guard has to hold on every dialect, and the SQLite cases above cannot show + * that. What decides whether it fires is `companionTableExists`, which probes + * with a dialect-quoted `SELECT 1` and swallows the failure — and on Postgres a + * statement that errors inside an open transaction poisons the rest of it, so + * "does a refusal come back at all, and is it ours" is a question only a real + * server answers. + * + * One boot is enough here, so the harness can provision a throwaway database per + * dialect: dropping the companion after the entry exists reproduces the same + * missing-table state the db:sync transition creates. The collection is localized + * from the start, so the translatable value never sat on the main table and there + * is nothing to overwrite — without the guard these return an opaque 500 from the + * driver rather than losing content. Proving the DEFAULT language's content + * survives still needs the two-boot SQLite case above, which is the only one that + * starts with the value on the main table. + */ +describe.each(getConfiguredTestDialects())( + "localized write without a companion table on %s (integration)", + dialect => { + it("refuses a non-default-locale update once the companion is gone", async () => { + current = await createTestNextly({ + dialect, + collections: [posts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "posts", overrideAccess: true, locale: "en" }, + { title: "Original" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_posts_locales`" : '"dc_posts_locales"'}` + ); + + const translated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "posts", + entryId: id, + overrideAccess: true, + locale: "es", + }, + { title: "Traducción" } + ); + + expect(translated.success).toBe(false); + expect(translated.statusCode).toBe(409); + expect(translated.message).toMatch(/Translations are not ready/); + }); + } +); diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index b2f7e2152..c1534ed00 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -257,7 +257,12 @@ export async function ensureCompanionTable( fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; - } + }, + /** + * Notified when creation fails. Optional so existing callers are unchanged; + * without it the previous swallow-and-retry-next-boot behaviour is preserved. + */ + onError?: (error: unknown) => void ): Promise { const companionTableName = `${args.tableName}_locales`; try { @@ -281,8 +286,15 @@ export async function ensureCompanionTable( for (const stmt of statements) { await adapter.executeQuery(stmt); } - } catch { - // Best-effort: main table may not exist yet on a very first boot — the companion - // will be created on the next boot (or by `nextly migrate`). + } catch (error) { + // Best-effort: the main table may not exist yet on a very first boot, where the + // companion is created on the next boot (or by `nextly migrate`). That case is + // expected and self-healing. Anything else is NOT — a persistent failure here + // leaves the entity marked localized with no place to store translations, and + // swallowing it silently is how that state went unnoticed. Report it through + // the optional reporter so a caller (db:sync, boot) can surface it; the + // function still resolves, because refusing to boot over a companion is worse + // than booting with non-default-locale writes refused. + onError?.(error); } } diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 23af7a3a2..23ca7ea5c 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -865,6 +865,34 @@ export class SingleMutationService extends BaseService { companion.companionTableName ) : false; + // A localized single whose companion table does not exist yet has nowhere + // to put a NON-default locale's values: the split below moves them out of + // the main payload and the companion upsert is then skipped, so the + // translation is silently dropped while the write reports success. The + // window is real — `db:sync` flips the registry's `localized` flag in its + // own process before the running server creates the companion — so refuse + // rather than discard the user's content. Default-locale values stay on + // the main table until the companion exists, which is intended. + if ( + companion && + !companionPhysicallyExists && + this.localization && + writeLocale !== undefined && + writeLocale !== this.localization.defaultLocale + ) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + single: singleMeta.slug, + locale: writeLocale, + defaultLocale: this.localization.defaultLocale, + companionTable: companion.companionTableName, + }, + }); + } // Same pre-transaction, pooled probe for the auto-create default seed: it // is keyed on the DEFAULT locale (not the write locale), so it needs its // own existence check rather than reusing `companionPhysicallyExists`. From c7f7fdb6af2648603156fb9209933f66a2157fec Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 03:23:26 +0500 Subject: [PATCH 03/20] fix(nextly): seed the companion so existing content stays readable --- .changeset/localized-write-companion-guard.md | 8 +- ...nc-localized-companion.integration.test.ts | 26 +++ packages/nextly/src/cli/commands/dev-build.ts | 58 ++++--- packages/nextly/src/di/register.ts | 19 ++- .../services/collection-mutation-service.ts | 24 +-- ...rite-without-companion.integration.test.ts | 83 ++++++++++ .../src/domains/i18n/migration/generate-up.ts | 85 ++++++---- .../src/domains/i18n/runtime/companion-io.ts | 149 ++++++++++++++++-- 8 files changed, 377 insertions(+), 75 deletions(-) diff --git a/.changeset/localized-write-companion-guard.md b/.changeset/localized-write-companion-guard.md index 0696b8644..ab1618169 100644 --- a/.changeset/localized-write-companion-guard.md +++ b/.changeset/localized-write-companion-guard.md @@ -22,6 +22,10 @@ "@nextlyhq/tsconfig": patch --- -Enabling localization on a collection that already had content could silently overwrite the default language. `nextly db:sync` flips the collection to localized in its own process, so a running app could believe the collection was localized — showing the language switcher — before its translations table existed. Saving a translation then wrote over the original-language values and changed the entry's URL, while reporting success. +Turning on localization for a collection that already had content could lose that content in two ways. -`db:sync` and the dev config watcher now create the translations table in the same run, for collections, singles and components alike. If that table is somehow still missing, a write in a non-default language is refused with a clear message instead of overwriting content. Writing the default language before the table exists is unchanged. +Existing entries could go blank. Once the translations table existed, every localized field was read from it, and nothing had copied the current values across — so titles and body text disappeared from the admin, from lists and from filters, while the values sat untouched in the database. Those values are now copied into the default language when the table is created, so existing content stays exactly as it was. Nothing is deleted: the copy leaves the originals in place. + +Saving a translation could overwrite the original language. `nextly db:sync` marks a collection as localized in a separate process from the running app, so the app could show the language switcher before its translations table existed, and saving a translation then wrote over the original-language values and changed the entry's URL while reporting success. `db:sync` and the dev config watcher now prepare that table in the same run, for collections, singles and components alike, and a translation saved before it exists is refused with a clear message instead of overwriting anything. Writing the default language before the table exists is unchanged. + +Collections and singles that set a custom `dbName` are now handled correctly here too; previously their translations table could be created against a table name that does not exist. And a database that is unreachable or refusing connections is no longer reported as a missing translations table. diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts index 9e6a8acf7..63d9fed6c 100644 --- a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -154,6 +154,32 @@ describe("db:sync creates localized companion tables in-process (integration)", expect(await tableExists("single_homepage_locales")).toBe(true); }); + it("resolves a custom dbName the way the runtime does", async () => { + // A collection's `dbName` is force-prefixed with `dc_` by the canonical + // resolver, so `dbName: "notes"` lives at `dc_notes`. Deriving the table name + // by pasting a prefix onto the slug, or by taking `dbName` verbatim, produces + // `notes_locales` with a foreign key to a `notes` table that does not exist — + // the create fails, the warning is swallowed, and the entity is left marked + // localized with nowhere to put translations. + await runSync( + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + collections: [ + defineCollection({ + slug: "field-notes", + dbName: "notes", + localized: true, + fields: [text({ name: "title", localized: true })], + }), + ], + }) + ); + + expect(await tableExists("dc_notes")).toBe(true); + expect(await tableExists("dc_notes_locales")).toBe(true); + expect(await tableExists("notes_locales")).toBe(false); + }); + it("leaves a non-localized collection with no companion", async () => { await runSync( defineConfig({ diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index c5138d529..a6204b158 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -790,27 +790,48 @@ export async function ensureLocalizedCompanions( const { ensureCompanionTable } = await import( "../../domains/i18n/runtime/companion-io" ); - const { resolveEntityTable } = await import( - "../../domains/i18n/migration/resolve-entity-table" + // Each entity kind resolves its physical table differently, and a custom + // `dbName` is where they diverge: collections and singles force their `dc_` / + // `single_` prefix onto it (and singles normalize the identifier), while + // components honour it verbatim. A single shared "prefix + slug" rule would + // point at a table the runtime never created — `dbName: "forms"` on a + // collection lives at `dc_forms`, so the companion would be built as + // `forms_locales` with a foreign key to a table that does not exist. + const { resolveCollectionTableName, resolveComponentTableName } = + await import("../../domains/schema/utils/resolve-table-name"); + const { resolveSingleTableName } = await import( + "../../domains/singles/services/resolve-single-table-name" ); - const groups = [ - config.collections ?? [], - config.singles ?? [], - config.components ?? [], + interface LocalizableEntity { + slug?: string; + dbName?: string; + localized?: boolean; + status?: boolean; + fields?: { name: string; type: string; localized?: boolean }[]; + } + + const groups: [LocalizableEntity[], (e: LocalizableEntity) => string][] = [ + [ + (config.collections ?? []) as LocalizableEntity[], + e => resolveCollectionTableName(e.slug!, e.dbName), + ], + [ + (config.singles ?? []) as LocalizableEntity[], + e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), + ], + [ + (config.components ?? []) as LocalizableEntity[], + e => resolveComponentTableName(e.slug!, e.dbName), + ], ]; - for (const group of groups) { - for (const raw of group) { - const entity = raw as { - slug?: string; - localized?: boolean; - status?: boolean; - fields?: { name: string; type: string; localized?: boolean }[]; - }; + const defaultLocale = config.localization?.defaultLocale; + + for (const [group, resolveTableName] of groups) { + for (const entity of group) { if (!entity.slug || entity.localized !== true) continue; - const resolved = resolveEntityTable(config, entity.slug); - if (!resolved) continue; + const tableName = resolveTableName(entity); // `ensureCompanionTable` resolves even on failure (a first boot may not have // the main table yet), so the reporter — not a try/catch — is what surfaces // a real problem. @@ -822,14 +843,15 @@ export async function ensureLocalizedCompanions( adapter as unknown as DrizzleAdapter, { slug: entity.slug, - tableName: resolved.tableName, + tableName, fields: entity.fields ?? [], dialect, status: entity.status === true, + defaultLocale, }, error => { logger.warn( - `Could not create the translations table for "${entity.slug}" (${resolved.companionTableName}). ` + + `Could not prepare the translations table for "${entity.slug}" (${tableName}_locales). ` + `Writes in a non-default locale will be refused until it exists: ` + `${error instanceof Error ? error.message : String(error)}` ); diff --git a/packages/nextly/src/di/register.ts b/packages/nextly/src/di/register.ts index d73d82a47..a6f493335 100644 --- a/packages/nextly/src/di/register.ts +++ b/packages/nextly/src/di/register.ts @@ -489,7 +489,10 @@ export async function registerServices( container.registerSingleton("adapter", () => adapter); - const schemaRegistry = await initializeSchemaRegistry(adapter); + const schemaRegistry = await initializeSchemaRegistry( + adapter, + config.localization?.defaultLocale + ); // Publish the webhook recording policy from the config INDEPENDENTLY of the // schema registry. `registerConfigTablesInResolver` (below) only runs when the @@ -972,7 +975,13 @@ async function resolveAdapter( * and `dynamic_components` DB tables and are generated at runtime. */ async function initializeSchemaRegistry( - adapter: DrizzleAdapter + adapter: DrizzleAdapter, + /** + * The language existing main-table values belong to. Passed down so a companion + * created here for an entity that already has content is seeded from that + * content instead of appearing empty, which would read as null everywhere. + */ + defaultLocale?: string ): Promise { try { const { SchemaRegistry } = await import("../database/schema-registry"); @@ -1043,6 +1052,7 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: hasStatus === true, + defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1094,6 +1104,7 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: hasStatus === true, + defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1143,6 +1154,7 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: false, + defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1712,6 +1724,7 @@ async function syncCodeFirstCollections( fields, dialect: syncDialect, status: desired.status === true, + defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1911,6 +1924,7 @@ async function syncCodeFirstComponents( fields: compConfig.fields as { name: string; type: string }[], dialect, status: false, + defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -2177,6 +2191,7 @@ async function reconcileSingleTablesForBoot( fields: fields, dialect, status: hasStatus, + defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 1a200a70f..59ac5df59 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -89,6 +89,7 @@ import { isValidLocale, resolveRequestedLocale, } from "../../i18n/resolve-locale"; +import { companionTableExists as sharedCompanionTableExists } from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; import { @@ -875,20 +876,21 @@ export class CollectionMutationService extends BaseService { ); } - /** Whether the companion `_locales` table physically exists (migration has run). */ + /** + * Whether the companion `_locales` table physically exists (migration has run). + * + * Delegates to the shared probe, which returns false only for a dialect-verified + * missing-table error and rethrows anything else. Catching every exception here + * turned a connection timeout or a permission error into "the table is absent", + * which the callers below read as a schema state: one refuses the write with a + * 409 telling the operator to re-run sync, the other decides the collection has + * no publish lifecycle. Both are the wrong answer to "the database is + * unreachable", and both hide a failure the caller would otherwise retry. + */ private async companionTableExists( companionTableName: string ): Promise { - const q = - this.adapter.dialect === "mysql" - ? `\`${companionTableName}\`` - : `"${companionTableName}"`; - try { - await this.adapter.executeQuery(`SELECT 1 FROM ${q} LIMIT 0`); - return true; - } catch { - return false; - } + return sharedCompanionTableExists(this.adapter, companionTableName); } /** diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 8434bd924..5d217f524 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -167,6 +167,89 @@ describe("localized write without a companion table (integration)", () => { }); }); +describe("enabling localization on existing content (integration)", () => { + it("keeps the default language readable once the companion appears", async () => { + // Creating the companion is not enough. Once it exists, a read resolves each + // localized field through it; with no default-locale row it overlays null, so + // the entity's existing content disappears from every read, list and filter + // while the values still sit on the main table. The companion has to be + // SEEDED from main, which is what the boot/db:sync path skipped. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "posts", overrideAccess: true }, + { title: "Original" } + ); + const id = (created.data as { id: string }).id; + + // Two re-boots, because the boot that flips `localized` in the registry is + // not the boot that creates the companion: the registry is read before the + // code-first sync writes the flag. That lag is exactly the window this branch + // closes for `db:sync`; here it just means the companion appears on the boot + // after, and the seed has to run then. + await current.destroy(); + current = await boot(true); + await current.destroy(); + current = await boot(true); + + const read = await current + .getService("collectionsHandler") + .getEntry({ collectionName: "posts", entryId: id, overrideAccess: true }); + + // Without the seed this is `null`: the companion exists and is empty. + expect((read.data as { title?: unknown }).title).toBe("Original"); + // The values must also still be on main — seeding copies, it does not move. + // Dropping those columns is the destructive half of the transition and stays + // behind the schema pipeline's confirmation. + expect(await physicalTitle(current)).toBe("Original"); + }); + + it("seeds once and does not duplicate rows on later boots", async () => { + // The seed runs on every boot and sync, so it must be gated on the companion + // being empty. Without that gate each boot would insert another default-locale + // row and the composite primary key would start rejecting writes. + current = await boot(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "posts", overrideAccess: true }, + { title: "Original" } + ); + const id = (created.data as { id: string }).id; + + await current.destroy(); + current = await boot(true); + + // Translate, so the companion is no longer empty, then re-boot. + await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "posts", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "Edited in the companion" } + ); + await current.destroy(); + current = await boot(true); + + const read = await current + .getService("collectionsHandler") + .getEntry({ collectionName: "posts", entryId: id, overrideAccess: true }); + expect((read.data as { title?: unknown }).title).toBe( + "Edited in the companion" + ); + + const rows = await current.adapter.executeQuery<{ n: number }>( + "SELECT COUNT(*) AS n FROM dc_posts_locales" + ); + expect(Number(rows[0]?.n)).toBe(1); + }); +}); + /** * The guard has to hold on every dialect, and the SQLite cases above cannot show * that. What decides whether it fires is `companionTableExists`, which probes diff --git a/packages/nextly/src/domains/i18n/migration/generate-up.ts b/packages/nextly/src/domains/i18n/migration/generate-up.ts index 97be1cef8..208a34755 100644 --- a/packages/nextly/src/domains/i18n/migration/generate-up.ts +++ b/packages/nextly/src/domains/i18n/migration/generate-up.ts @@ -68,44 +68,71 @@ export function buildLocalizationUpSql(spec: CompanionMigrationSpec): string { export function buildLocalizationUpStatements( spec: CompanionMigrationSpec ): string[] { - const { dialect, mainTable, companionTable, defaultLocale, columns } = spec; + const { dialect, mainTable } = spec; const create = buildCompanionCreateStatement(spec); - // Only columns already on the main table can be seeded from or dropped. A field added and - // localized in the same save is in `columns` (so the companion gets it) but not on main, so - // it is excluded from the SELECT and the DROP. Undefined `columnsOnMain` means "all" — the - // file-migration path, where every localized column pre-exists on main. + const seedStatement = buildCompanionSeedStatement(spec); + const seed = seedStatement ? [seedStatement] : []; + + const drops = columnsStillOnMain(spec).map( + c => + `ALTER TABLE ${q(mainTable, dialect)} DROP COLUMN ${q(c.name, dialect)}` + ); + + return [create, ...seed, ...drops]; +} + +/** + * The subset of `spec.columns` that physically exists on the main table, and so can be seeded + * from or dropped. A field added and localized in the same save is in `columns` (the companion + * needs it) but not here — there is nothing on main to copy or remove. Undefined `columnsOnMain` + * means "all", which is the file-migration path where every localized column pre-exists. + */ +function columnsStillOnMain( + spec: CompanionMigrationSpec +): CompanionMigrationSpec["columns"] { const onMainSet = spec.columnsOnMain && new Set(spec.columnsOnMain); - const onMain = onMainSet - ? columns.filter(c => onMainSet.has(c.name)) - : columns; + return onMainSet + ? spec.columns.filter(c => onMainSet.has(c.name)) + : spec.columns; +} + +/** + * The `INSERT ... SELECT` that copies the main table's existing values into the companion as + * default-locale rows, or null when there is nothing to copy. + * + * Split out from {@link buildLocalizationUpStatements} because seeding and dropping are not + * always wanted together. Creating the companion without this INSERT is what made existing + * content vanish: reads resolve a localized field through the companion once the table is + * there, find no row for the default locale, and overlay null — the values are still on the + * main table, but nothing returns them. + * + * Dropping the main columns is a separate, destructive step that the schema pipeline gates + * behind an explicit confirmation, so a caller that only needs the content to stay visible + * (dev boot, `db:sync`) takes this statement alone and leaves the columns in place. + */ +export function buildCompanionSeedStatement( + spec: CompanionMigrationSpec +): string | null { + const { dialect, mainTable, companionTable, defaultLocale } = spec; + const onMain = columnsStillOnMain(spec); + // An INSERT with an empty value list is invalid SQL, and with no pre-existing translatable + // columns and no status there is no content to preserve either. + if (onMain.length === 0 && !spec.status) return null; + // A leading ", " per column, so an empty set contributes nothing to the column lists. const onMainCols = onMain.map(c => `, ${q(c.name, dialect)}`).join(""); - - // When the collection has Draft/Published, the seeded default-locale rows carry the existing - // main row's `status` into the companion `_status` so enabling localization doesn't silently + // When the entity has Draft/Published, the seeded default-locale rows carry the existing main + // row's `status` into the companion `_status` so enabling localization does not silently // un-publish live content. const statusInsertCol = spec.status ? `, ${q("_status", dialect)}` : ""; const statusSelectCol = spec.status ? `, ${q("status", dialect)}` : ""; - // Skip the seed entirely when there is nothing on main to copy — no pre-existing translatable - // columns and no status. An INSERT with an empty value list would be invalid SQL, and there - // is no existing content to preserve. - const seed = - onMain.length > 0 || spec.status - ? [ - `INSERT INTO ${q(companionTable, dialect)} ` + - `(${q("_parent", dialect)}, ${q("_locale", dialect)}${statusInsertCol}${onMainCols}) ` + - `SELECT ${q("id", dialect)}, ${lit(defaultLocale)}${statusSelectCol}${onMainCols} ` + - `FROM ${q(mainTable, dialect)}`, - ] - : []; - - const drops = onMain.map( - c => - `ALTER TABLE ${q(mainTable, dialect)} DROP COLUMN ${q(c.name, dialect)}` + return ( + `INSERT INTO ${q(companionTable, dialect)} ` + + `(${q("_parent", dialect)}, ${q("_locale", dialect)}${statusInsertCol}${onMainCols}) ` + + `SELECT ${q("id", dialect)}, ${lit(defaultLocale)}${statusSelectCol}${onMainCols} ` + + `FROM ${q(mainTable, dialect)}` ); - - return [create, ...seed, ...drops]; } diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index c1534ed00..03dcb4a68 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -243,11 +243,13 @@ export async function companionHasStatusColumn( /** * Boot/db:sync helper: physically create the companion `_locales` table if it does - * not already exist. Idempotent and safe to run on every boot — a no-op once the table exists (or - * when the entity has no translatable fields). This is the db:sync/dev-boot counterpart to the - * migration-owned companion creation (`nextly migrate`), so a code-first localized collection / - * single / component gets a working companion without a manual migrate step. Best-effort: a - * failure (e.g. main table not yet created) is swallowed so it retries on the next boot. + * not already exist, and seed it from the main table when localization is being turned on for an + * entity that already holds content. Idempotent and safe to run on every boot — a no-op once the + * table exists and is seeded (or when the entity has no translatable fields). This is the + * db:sync/dev-boot counterpart to the migration-owned companion creation (`nextly migrate`), so a + * code-first localized collection / single / component gets a working companion without a manual + * migrate step. Best-effort: a failure (e.g. main table not yet created) is swallowed so it + * retries on the next boot. */ export async function ensureCompanionTable( adapter: CompanionWriteAdapter, @@ -257,6 +259,12 @@ export async function ensureCompanionTable( fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; + /** + * The language existing main-table values belong to. Supplying it enables the seed below; + * without it the companion is created empty, which is the pre-existing behaviour and is only + * correct for an entity that never held content on main. + */ + defaultLocale?: string; }, /** * Notified when creation fails. Optional so existing callers are unchanged; @@ -266,7 +274,10 @@ export async function ensureCompanionTable( ): Promise { const companionTableName = `${args.tableName}_locales`; try { - if (await companionTableExists(adapter, companionTableName)) return; + const alreadyExists = await companionTableExists( + adapter, + companionTableName + ); // Lazy import avoids a cycle (reconcile-companion → migration helpers). const { buildCompanionReconcileStatements } = await import( "../migration/reconcile-companion" @@ -274,18 +285,30 @@ export async function ensureCompanionTable( const localizedNames = new Set( resolveLocalizedFieldNames(args.fields, true) ); - const statements = buildCompanionReconcileStatements({ + const localizedFields = args.fields.filter(f => localizedNames.has(f.name)); + if (!alreadyExists) { + const statements = buildCompanionReconcileStatements({ + slug: args.slug, + tableName: args.tableName, + oldLocalized: [], + newLocalized: localizedFields, + dialect: args.dialect, + status: args.status === true, + companionExists: false, + }); + for (const stmt of statements) { + await adapter.executeQuery(stmt); + } + } + await seedCompanionFromMain(adapter, { slug: args.slug, tableName: args.tableName, - oldLocalized: [], - newLocalized: args.fields.filter(f => localizedNames.has(f.name)), + companionTableName, + localizedFields, dialect: args.dialect, status: args.status === true, - companionExists: false, + defaultLocale: args.defaultLocale, }); - for (const stmt of statements) { - await adapter.executeQuery(stmt); - } } catch (error) { // Best-effort: the main table may not exist yet on a very first boot, where the // companion is created on the next boot (or by `nextly migrate`). That case is @@ -298,3 +321,103 @@ export async function ensureCompanionTable( onError?.(error); } } + +/** + * Copy the main table's existing values into the companion as default-locale rows. + * + * Creating the companion is not enough on its own. Once the table exists, a read resolves each + * localized field through it, finds no row for the default locale, and overlays null — so an + * entity that already had content shows empty fields everywhere while the values sit untouched on + * the main table. Enabling localization on existing content therefore made that content + * invisible, not merely unwritable. + * + * Deliberately narrow, because this runs unattended on every boot and sync: + * + * - only when the companion is EMPTY. A companion with rows has been through a real transition + * (or holds translations), and re-seeding it would resurrect main-table values over them. + * - only for localized columns that are STILL on the main table, probed one by one. After the + * columns are dropped there is nothing to copy, and the probe is the only portable way to ask. + * - it does NOT drop those columns afterwards. That is the destructive half of the transition + * and the schema pipeline gates it behind an explicit confirmation; making the content visible + * again does not require it, and doing it here would route around that gate. + */ +async function seedCompanionFromMain( + adapter: CompanionWriteAdapter, + args: { + slug: string; + tableName: string; + companionTableName: string; + localizedFields: CompanionFieldLike[]; + dialect: SupportedDialect; + status: boolean; + defaultLocale?: string; + } +): Promise { + if (!args.defaultLocale || args.localizedFields.length === 0) return; + + const { deriveCompanionSpec } = await import( + "../migration/derive-companion-spec" + ); + const spec = deriveCompanionSpec({ + slug: args.slug, + dbName: args.tableName, + fields: args.localizedFields, + dialect: args.dialect, + defaultLocale: args.defaultLocale, + collectionLocalized: true, + status: args.status, + }); + if (!spec) return; + + if (!(await companionIsEmpty(adapter, args.companionTableName))) return; + + const columnsOnMain: string[] = []; + for (const column of spec.columns) { + if (await mainHasColumn(adapter, args.tableName, column.name)) { + columnsOnMain.push(column.name); + } + } + if (columnsOnMain.length === 0) return; + + const { buildCompanionSeedStatement } = await import( + "../migration/generate-up" + ); + const seed = buildCompanionSeedStatement({ ...spec, columnsOnMain }); + if (seed) await adapter.executeQuery(seed); +} + +/** Whether the companion holds no rows, i.e. nothing a seed could overwrite. */ +async function companionIsEmpty( + adapter: CompanionWriteAdapter, + companionTableName: string +): Promise { + const table = + adapter.dialect === "mysql" + ? `\`${companionTableName}\`` + : `"${companionTableName}"`; + const rows = await adapter.executeQuery(`SELECT 1 FROM ${table} LIMIT 1`); + return rows.length === 0; +} + +/** Whether a physical column is still present on the main table. */ +async function mainHasColumn( + adapter: CompanionWriteAdapter, + tableName: string, + columnName: string +): Promise { + const isMysql = adapter.dialect === "mysql"; + const table = isMysql ? `\`${tableName}\`` : `"${tableName}"`; + const column = isMysql ? `\`${columnName}\`` : `"${columnName}"`; + try { + await adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); + return true; + } catch { + // Catching everything is safe HERE, unlike a probe a write gates on: the + // caller has already run a query against this connection (the emptiness + // check, which does not catch), so an unreachable database has surfaced + // before this point. What remains is a question about one column of one + // table, and treating any answer but "yes" as "do not seed from it" only + // ever narrows the copy — never writes the wrong thing. + return false; + } +} From 638dc16be246864a00d6d82f483e956be59beec6 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 05:24:02 +0500 Subject: [PATCH 04/20] fix(nextly): backfill missing default-locale rows and respect no-auto-sync --- ...nc-localized-companion.integration.test.ts | 44 ++++----- packages/nextly/src/cli/commands/db-sync.ts | 8 +- packages/nextly/src/cli/commands/dev-build.ts | 8 +- .../nextly/src/cli/commands/dev-watcher.ts | 6 +- packages/nextly/src/di/register.ts | 6 +- ...rite-without-companion.integration.test.ts | 89 +++++++++++++++---- .../src/domains/i18n/migration/generate-up.ts | 20 ++++- .../src/domains/i18n/runtime/companion-io.ts | 38 +++----- 8 files changed, 150 insertions(+), 69 deletions(-) diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts index 63d9fed6c..fd7e03523 100644 --- a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -119,7 +119,7 @@ describe("db:sync creates localized companion tables in-process (integration)", localization: { locales: ["en", "es"], defaultLocale: "en" }, collections: [ defineCollection({ - slug: "posts", + slug: "dbsync_posts", localized: true, fields: [text({ name: "title", localized: true })], }), @@ -127,10 +127,10 @@ describe("db:sync creates localized companion tables in-process (integration)", }) ); - expect(await tableExists("dc_posts")).toBe(true); + expect(await tableExists("dc_dbsync_posts")).toBe(true); // The assertion that fails without the fix: the push pipeline creates the // main table and nothing in this process creates the companion. - expect(await tableExists("dc_posts_locales")).toBe(true); + expect(await tableExists("dc_dbsync_posts_locales")).toBe(true); }); it("creates a localized single's companion, which needs singles synced first", async () => { @@ -139,7 +139,7 @@ describe("db:sync creates localized companion tables in-process (integration)", localization: { locales: ["en", "es"], defaultLocale: "en" }, singles: [ defineSingle({ - slug: "homepage", + slug: "dbsync_homepage", localized: true, fields: [text({ name: "headline", localized: true })], }), @@ -147,27 +147,27 @@ describe("db:sync creates localized companion tables in-process (integration)", }) ); - // Singles use the `single_` prefix (`resolve-entity-table.GROUPS`). Asserting - // the main table too means a prefix change cannot make the companion check - // vacuously pass against a name nothing ever creates. - expect(await tableExists("single_homepage")).toBe(true); - expect(await tableExists("single_homepage_locales")).toBe(true); + // Singles are force-prefixed with `single_` by `resolveSingleTableName`. + // Asserting the main table too means a prefix change cannot make the companion + // check vacuously pass against a name nothing ever creates. + expect(await tableExists("single_dbsync_homepage")).toBe(true); + expect(await tableExists("single_dbsync_homepage_locales")).toBe(true); }); it("resolves a custom dbName the way the runtime does", async () => { // A collection's `dbName` is force-prefixed with `dc_` by the canonical - // resolver, so `dbName: "notes"` lives at `dc_notes`. Deriving the table name - // by pasting a prefix onto the slug, or by taking `dbName` verbatim, produces - // `notes_locales` with a foreign key to a `notes` table that does not exist — - // the create fails, the warning is swallowed, and the entity is left marked - // localized with nowhere to put translations. + // resolver, so `dbName: "dbsync_notes"` lives at `dc_dbsync_notes`. Taking + // `dbName` verbatim instead builds `dbsync_notes_locales` with a foreign key to + // a `dbsync_notes` table that does not exist — the create fails, the warning is + // swallowed, and the entity is left marked localized with nowhere to put + // translations. await runSync( defineConfig({ localization: { locales: ["en", "es"], defaultLocale: "en" }, collections: [ defineCollection({ - slug: "field-notes", - dbName: "notes", + slug: "dbsync_field_notes", + dbName: "dbsync_notes", localized: true, fields: [text({ name: "title", localized: true })], }), @@ -175,9 +175,9 @@ describe("db:sync creates localized companion tables in-process (integration)", }) ); - expect(await tableExists("dc_notes")).toBe(true); - expect(await tableExists("dc_notes_locales")).toBe(true); - expect(await tableExists("notes_locales")).toBe(false); + expect(await tableExists("dc_dbsync_notes")).toBe(true); + expect(await tableExists("dc_dbsync_notes_locales")).toBe(true); + expect(await tableExists("dbsync_notes_locales")).toBe(false); }); it("leaves a non-localized collection with no companion", async () => { @@ -185,16 +185,16 @@ describe("db:sync creates localized companion tables in-process (integration)", defineConfig({ collections: [ defineCollection({ - slug: "logs", + slug: "dbsync_logs", fields: [text({ name: "title" })], }), ], }) ); - expect(await tableExists("dc_logs")).toBe(true); + expect(await tableExists("dc_dbsync_logs")).toBe(true); // Creating companions unconditionally would strand a dead table in every // project that does not use localization. - expect(await tableExists("dc_logs_locales")).toBe(false); + expect(await tableExists("dc_dbsync_logs_locales")).toBe(false); }); }); diff --git a/packages/nextly/src/cli/commands/db-sync.ts b/packages/nextly/src/cli/commands/db-sync.ts index d391d75e4..4ac791ce5 100644 --- a/packages/nextly/src/cli/commands/db-sync.ts +++ b/packages/nextly/src/cli/commands/db-sync.ts @@ -308,7 +308,13 @@ export async function runDbSync( // companion references its main table. Without this, `db:sync` left the // registry saying "localized" with no table to hold translations until the // app next booted, and writes in that window overwrote the default language. - await ensureLocalizedCompanions(configResult.config, adapter, context); + // + // Gated on the same flag as the rest of the schema push: this issues DDL and + // can copy rows, so `--no-auto-sync` — chosen precisely to keep physical + // schema changes in migration files — must suppress it too. + if (options.autoSync !== false) { + await ensureLocalizedCompanions(configResult.config, adapter, context); + } if (collectionCount === 0) { logger.warn("No collections defined in config"); diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index a6204b158..bd3bb3832 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -301,6 +301,12 @@ export async function syncComponents( fields: component.fields, description: component.description, admin: component.admin, + // Persist i18n through db:sync, as the singles mapping above already does. + // Runtime field-group writes gate on the stored flag, so leaving it unset + // made `db:sync` record a localized field group as non-localized: + // translatable values kept being treated as shared main-table fields, and a + // save in a non-default locale overwrote the default one. + localized: component.localized === true, configPath: `${STORAGE_FORMAT.configPathDir}/${component.slug}.ts`, }) ); @@ -821,7 +827,7 @@ export async function ensureLocalizedCompanions( e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), ], [ - (config.components ?? []) as LocalizableEntity[], + (config.fieldGroups ?? []) as LocalizableEntity[], e => resolveComponentTableName(e.slug!, e.dbName), ], ]; diff --git a/packages/nextly/src/cli/commands/dev-watcher.ts b/packages/nextly/src/cli/commands/dev-watcher.ts index 70dba2bc4..b7586911b 100644 --- a/packages/nextly/src/cli/commands/dev-watcher.ts +++ b/packages/nextly/src/cli/commands/dev-watcher.ts @@ -77,7 +77,11 @@ export function createDebouncedSync( // watcher as often as through `db:sync`. The companion table is not part of // the push pipeline, and creating it here rather than at the next boot is // what keeps a running server from advertising localization it cannot store. - await ensureLocalizedCompanions(configToSync.config, adapter, context); + // Suppressed under `--no-auto-sync` for the same reason the rest of the + // push is: it issues DDL and can copy rows. + if (options.autoSync !== false) { + await ensureLocalizedCompanions(configToSync.config, adapter, context); + } // Sync user_ext table (always — handles both code and UI fields) await syncUserFields(configToSync, adapter, options, context); diff --git a/packages/nextly/src/di/register.ts b/packages/nextly/src/di/register.ts index a6f493335..4408eb045 100644 --- a/packages/nextly/src/di/register.ts +++ b/packages/nextly/src/di/register.ts @@ -489,9 +489,13 @@ export async function registerServices( container.registerSingleton("adapter", () => adapter); + // `transformedConfig`, not `config`: plugin config transformers run before this + // and may supply or override `localization`, and every other companion call + // site reads the transformed value. Taking the raw config here would create + // unseeded companions on the registry path while the code-first paths seed. const schemaRegistry = await initializeSchemaRegistry( adapter, - config.localization?.defaultLocale + transformedConfig.localization?.defaultLocale ); // Publish the webhook recording policy from the config INDEPENDENTLY of the diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 5d217f524..89d2a4f07 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -12,7 +12,7 @@ * Before this guard, a write in a NON-default locale fell through to the main * table in that window, overwriting the default language's values and * regenerating the slug from the translation, while reporting success. - * Reproduced on published alpha.43 during the task 006 walk. + * Reproduced on published 0.0.2-alpha.43. * * The default locale legitimately writes to the main table until the companion * exists ("Option B"), so that path must keep working unchanged. @@ -57,7 +57,7 @@ const localization = { locales: ["en", "es"], defaultLocale: "en" }; /** Same collection, with localization off (columns on main) or on (companion). */ const posts = (localized: boolean) => defineCollection({ - slug: "posts", + slug: "i18nwin_posts", localized, fields: [text({ name: "title", localized: true })], }); @@ -83,7 +83,7 @@ async function boot(localized: boolean): Promise { */ async function physicalTitle(handle: TestNextly): Promise { const rows = await handle.adapter.executeQuery<{ title: string }>( - "SELECT title FROM dc_posts LIMIT 1" + "SELECT title FROM dc_i18nwin_posts LIMIT 1" ); return rows[0]?.title; } @@ -93,7 +93,9 @@ async function enterWindow(): Promise { await current?.destroy(); current = undefined; const handle = await boot(true); - await handle.adapter.executeQuery("DROP TABLE IF EXISTS dc_posts_locales"); + await handle.adapter.executeQuery( + "DROP TABLE IF EXISTS dc_i18nwin_posts_locales" + ); return handle; } @@ -104,7 +106,7 @@ describe("localized write without a companion table (integration)", () => { const created = await current .getService("collectionsHandler") .createEntry( - { collectionName: "posts", overrideAccess: true }, + { collectionName: "i18nwin_posts", overrideAccess: true }, { title: "How to Build a Blog" } ); expect(created.success).toBe(true); @@ -116,7 +118,7 @@ describe("localized write without a companion table (integration)", () => { .getService("collectionsHandler") .updateEntry( { - collectionName: "posts", + collectionName: "i18nwin_posts", entryId: id, overrideAccess: true, locale: "es", @@ -143,7 +145,7 @@ describe("localized write without a companion table (integration)", () => { const created = await current .getService("collectionsHandler") .createEntry( - { collectionName: "posts", overrideAccess: true }, + { collectionName: "i18nwin_posts", overrideAccess: true }, { title: "First" } ); const id = (created.data as { id: string }).id; @@ -154,7 +156,7 @@ describe("localized write without a companion table (integration)", () => { .getService("collectionsHandler") .updateEntry( { - collectionName: "posts", + collectionName: "i18nwin_posts", entryId: id, overrideAccess: true, locale: "en", @@ -178,7 +180,7 @@ describe("enabling localization on existing content (integration)", () => { const created = await current .getService("collectionsHandler") .createEntry( - { collectionName: "posts", overrideAccess: true }, + { collectionName: "i18nwin_posts", overrideAccess: true }, { title: "Original" } ); const id = (created.data as { id: string }).id; @@ -195,7 +197,11 @@ describe("enabling localization on existing content (integration)", () => { const read = await current .getService("collectionsHandler") - .getEntry({ collectionName: "posts", entryId: id, overrideAccess: true }); + .getEntry({ + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + }); // Without the seed this is `null`: the companion exists and is empty. expect((read.data as { title?: unknown }).title).toBe("Original"); @@ -205,6 +211,47 @@ describe("enabling localization on existing content (integration)", () => { expect(await physicalTitle(current)).toBe("Original"); }); + it("backfills entries the companion is missing, not just an empty one", async () => { + // The seed is per-row, not per-table. A companion that already holds a row for + // ONE entry — someone edited a single entry in the default language — must not + // stop every other entry from being backfilled; gating on "the companion is + // empty" would leave the rest reading null permanently. + current = await boot(false); + const handler = () => + current!.getService("collectionsHandler"); + const ids: string[] = []; + for (const title of ["First", "Second", "Third"]) { + const created = await handler().createEntry( + { collectionName: "i18nwin_posts", overrideAccess: true }, + { title } + ); + ids.push((created.data as { id: string }).id); + } + + await current.destroy(); + current = await boot(true); + await current.destroy(); + current = await boot(true); + + // Put the companion into the partial state: keep one row, drop the others, so + // the table is non-empty but two entries have no default-locale row. + await current.adapter.executeQuery( + `DELETE FROM dc_i18nwin_posts_locales WHERE _parent <> '${ids[0]}'` + ); + + await current.destroy(); + current = await boot(true); + + for (const [index, title] of ["First", "Second", "Third"].entries()) { + const read = await handler().getEntry({ + collectionName: "i18nwin_posts", + entryId: ids[index], + overrideAccess: true, + }); + expect((read.data as { title?: unknown }).title).toBe(title); + } + }); + it("seeds once and does not duplicate rows on later boots", async () => { // The seed runs on every boot and sync, so it must be gated on the companion // being empty. Without that gate each boot would insert another default-locale @@ -213,7 +260,7 @@ describe("enabling localization on existing content (integration)", () => { const created = await current .getService("collectionsHandler") .createEntry( - { collectionName: "posts", overrideAccess: true }, + { collectionName: "i18nwin_posts", overrideAccess: true }, { title: "Original" } ); const id = (created.data as { id: string }).id; @@ -226,7 +273,7 @@ describe("enabling localization on existing content (integration)", () => { .getService("collectionsHandler") .updateEntry( { - collectionName: "posts", + collectionName: "i18nwin_posts", entryId: id, overrideAccess: true, locale: "en", @@ -238,13 +285,17 @@ describe("enabling localization on existing content (integration)", () => { const read = await current .getService("collectionsHandler") - .getEntry({ collectionName: "posts", entryId: id, overrideAccess: true }); + .getEntry({ + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + }); expect((read.data as { title?: unknown }).title).toBe( "Edited in the companion" ); const rows = await current.adapter.executeQuery<{ n: number }>( - "SELECT COUNT(*) AS n FROM dc_posts_locales" + "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales" ); expect(Number(rows[0]?.n)).toBe(1); }); @@ -280,21 +331,25 @@ describe.each(getConfiguredTestDialects())( const created = await current .getService("collectionsHandler") .createEntry( - { collectionName: "posts", overrideAccess: true, locale: "en" }, + { + collectionName: "i18nwin_posts", + overrideAccess: true, + locale: "en", + }, { title: "Original" } ); expect(created.success).toBe(true); const id = (created.data as { id: string }).id; await current.adapter.executeQuery( - `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_posts_locales`" : '"dc_posts_locales"'}` + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_posts_locales`" : '"dc_i18nwin_posts_locales"'}` ); const translated = await current .getService("collectionsHandler") .updateEntry( { - collectionName: "posts", + collectionName: "i18nwin_posts", entryId: id, overrideAccess: true, locale: "es", diff --git a/packages/nextly/src/domains/i18n/migration/generate-up.ts b/packages/nextly/src/domains/i18n/migration/generate-up.ts index 208a34755..31434f2df 100644 --- a/packages/nextly/src/domains/i18n/migration/generate-up.ts +++ b/packages/nextly/src/domains/i18n/migration/generate-up.ts @@ -113,7 +113,17 @@ function columnsStillOnMain( * (dev boot, `db:sync`) takes this statement alone and leaves the columns in place. */ export function buildCompanionSeedStatement( - spec: CompanionMigrationSpec + spec: CompanionMigrationSpec, + options?: { + /** + * Skip main rows that already have a default-locale companion row, making the + * statement safe to re-run and safe on a PARTIALLY seeded companion. Gating the + * whole seed on an empty companion instead would strand every other row the + * moment one row got a companion entry — one edited entry, and the rest keep + * reading null forever. + */ + onlyMissing?: boolean; + } ): string | null { const { dialect, mainTable, companionTable, defaultLocale } = spec; const onMain = columnsStillOnMain(spec); @@ -129,10 +139,16 @@ export function buildCompanionSeedStatement( const statusInsertCol = spec.status ? `, ${q("_status", dialect)}` : ""; const statusSelectCol = spec.status ? `, ${q("status", dialect)}` : ""; + const where = options?.onlyMissing + ? ` WHERE NOT EXISTS (SELECT 1 FROM ${q(companionTable, dialect)} ` + + `WHERE ${q(companionTable, dialect)}.${q("_parent", dialect)} = ${q(mainTable, dialect)}.${q("id", dialect)} ` + + `AND ${q(companionTable, dialect)}.${q("_locale", dialect)} = ${lit(defaultLocale)})` + : ""; + return ( `INSERT INTO ${q(companionTable, dialect)} ` + `(${q("_parent", dialect)}, ${q("_locale", dialect)}${statusInsertCol}${onMainCols}) ` + `SELECT ${q("id", dialect)}, ${lit(defaultLocale)}${statusSelectCol}${onMainCols} ` + - `FROM ${q(mainTable, dialect)}` + `FROM ${q(mainTable, dialect)}${where}` ); } diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 03dcb4a68..b62e0fe63 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -333,8 +333,10 @@ export async function ensureCompanionTable( * * Deliberately narrow, because this runs unattended on every boot and sync: * - * - only when the companion is EMPTY. A companion with rows has been through a real transition - * (or holds translations), and re-seeding it would resurrect main-table values over them. + * - only for main rows that have NO default-locale companion row yet, so it never overwrites a + * translation and is safe to re-run. Row-level rather than table-level: a companion that + * already holds one row (someone edited a single entry) must not stop every other row from + * being backfilled, or those stay unreadable forever. * - only for localized columns that are STILL on the main table, probed one by one. After the * columns are dropped there is nothing to copy, and the probe is the only portable way to ask. * - it does NOT drop those columns afterwards. That is the destructive half of the transition @@ -369,8 +371,6 @@ async function seedCompanionFromMain( }); if (!spec) return; - if (!(await companionIsEmpty(adapter, args.companionTableName))) return; - const columnsOnMain: string[] = []; for (const column of spec.columns) { if (await mainHasColumn(adapter, args.tableName, column.name)) { @@ -382,23 +382,13 @@ async function seedCompanionFromMain( const { buildCompanionSeedStatement } = await import( "../migration/generate-up" ); - const seed = buildCompanionSeedStatement({ ...spec, columnsOnMain }); + const seed = buildCompanionSeedStatement( + { ...spec, columnsOnMain }, + { onlyMissing: true } + ); if (seed) await adapter.executeQuery(seed); } -/** Whether the companion holds no rows, i.e. nothing a seed could overwrite. */ -async function companionIsEmpty( - adapter: CompanionWriteAdapter, - companionTableName: string -): Promise { - const table = - adapter.dialect === "mysql" - ? `\`${companionTableName}\`` - : `"${companionTableName}"`; - const rows = await adapter.executeQuery(`SELECT 1 FROM ${table} LIMIT 1`); - return rows.length === 0; -} - /** Whether a physical column is still present on the main table. */ async function mainHasColumn( adapter: CompanionWriteAdapter, @@ -412,12 +402,12 @@ async function mainHasColumn( await adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); return true; } catch { - // Catching everything is safe HERE, unlike a probe a write gates on: the - // caller has already run a query against this connection (the emptiness - // check, which does not catch), so an unreachable database has surfaced - // before this point. What remains is a question about one column of one - // table, and treating any answer but "yes" as "do not seed from it" only - // ever narrows the copy — never writes the wrong thing. + // Catching everything is safe HERE, unlike a probe a write gates on. Treating + // any answer but "yes" as "do not copy from this column" only ever narrows the + // seed; it can never write the wrong value. And a failure that is not a + // missing column — an unreachable database — does not get swallowed overall, + // because the seed statement that follows runs on the same connection and + // propagates to the caller's reporter. return false; } } From b9acaf21fc0efc62907c9636a3d66bcd230189ad Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 07:54:36 +0500 Subject: [PATCH 05/20] fix(nextly): seed only during the localization transition --- packages/nextly/src/cli/commands/dev-build.ts | 8 +- .../services/collection-mutation-service.ts | 29 ++-- ...rite-without-companion.integration.test.ts | 67 ++++++++- .../src/domains/i18n/runtime/companion-io.ts | 140 +++++++++++++++--- .../i18n/writes-create.integration.test.ts | 31 +++- .../services/single-mutation-service.ts | 21 ++- 6 files changed, 247 insertions(+), 49 deletions(-) diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index bd3bb3832..0ab91af97 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -798,8 +798,8 @@ export async function ensureLocalizedCompanions( ); // Each entity kind resolves its physical table differently, and a custom // `dbName` is where they diverge: collections and singles force their `dc_` / - // `single_` prefix onto it (and singles normalize the identifier), while - // components honour it verbatim. A single shared "prefix + slug" rule would + // `single_` prefix onto it (and singles normalize the identifier), while field + // groups take no override at all. A single shared "prefix + slug" rule would // point at a table the runtime never created — `dbName: "forms"` on a // collection lives at `dc_forms`, so the companion would be built as // `forms_locales` with a foreign key to a table that does not exist. @@ -828,7 +828,9 @@ export async function ensureLocalizedCompanions( ], [ (config.fieldGroups ?? []) as LocalizableEntity[], - e => resolveComponentTableName(e.slug!, e.dbName), + // Field groups derive their table from the slug alone — unlike collections + // and singles they carry no `dbName` override. + e => resolveComponentTableName(e.slug!), ], ]; diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 59ac5df59..1678dd96b 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -944,19 +944,26 @@ export class CollectionMutationService extends BaseService { // `migrate`, the dev auto-sync leaves localized columns on the MAIN table (Option B), so // writes must go there — return null and let the localized values flow to main as today. if (!(await this.companionTableExists(companion.companionTableName))) { - // CREATE is unaffected: a new entry has no other language's values to lose, - // so its translatable values legitimately sit on the main table until the - // companion exists — the documented pre-migration fallback. + // The main table carries no language of its own, so anything written there + // while the companion is missing is later read as the DEFAULT language — + // that is the assumption the companion seed makes when it copies those + // columns across. A write in another language therefore has nowhere honest + // to go, and both ways of letting it through lose content: // - // UPDATE is where content dies. The row already holds the default - // language's values on main, so letting a NON-default locale fall through - // overwrites them with the translation and regenerates the slug from it, - // silently and with a success response. The window is real: `db:sync` - // flips the registry's `localized` flag in its own process while the - // running server has yet to create the companion. Refuse rather than - // destroy. + // UPDATE overwrites. The row already holds the default language on main, + // so a non-default write replaces it and regenerates the slug from the + // translation, silently and with a success response. + // + // CREATE mis-files. The values land on main, and the seed then copies + // them into the default language's row — so Spanish text is served as + // English, and Spanish itself has no translation at all. + // + // The window is real: `db:sync` flips the registry's `localized` flag in + // its own process while the running server has yet to create the companion. + // Refuse either way; the default language still writes to main, which is + // the documented pre-migration fallback. const requested = resolveRequestedLocale(this.localization, locale); - if (!isCreate && requested !== this.localization.defaultLocale) { + if (requested !== this.localization.defaultLocale) { throw NextlyError.conflict({ reason: "state", message: diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 89d2a4f07..495b4ee7d 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -62,7 +62,10 @@ const posts = (localized: boolean) => fields: [text({ name: "title", localized: true })], }); -async function boot(localized: boolean): Promise { +async function boot( + localized: boolean, + defaultLocale = localization.defaultLocale +): Promise { process.env.DB_DIALECT = "sqlite"; const adapter = await createAdapter({ type: "sqlite", @@ -71,7 +74,7 @@ async function boot(localized: boolean): Promise { return createTestNextly({ adapter, collections: [posts(localized)], - localization, + localization: { ...localization, defaultLocale }, }); } @@ -252,6 +255,66 @@ describe("enabling localization on existing content (integration)", () => { } }); + it("does not fabricate rows from stale columns after the default language changes", async () => { + // The main columns are only the default language DURING the transition. Once + // real translations exist, writes go to the companion and those columns stop + // being updated — so re-pointing `defaultLocale` at another language must not + // seed it from them, or the new default serves stale text from the old one and + // suppresses the fallback that would have shown the real value. + current = await boot(false); + const handler = () => + current!.getService("collectionsHandler"); + const ids: string[] = []; + for (const title of ["Translated later", "Never translated"]) { + const created = await handler().createEntry( + { collectionName: "i18nwin_posts", overrideAccess: true }, + { title } + ); + ids.push((created.data as { id: string }).id); + } + + await current.destroy(); + current = await boot(true); + await current.destroy(); + current = await boot(true); + + // One real translation, which is what marks the transition as finished. The + // OTHER entry is the exposed one: it has no Spanish row, so a seed keyed on + // the new default would invent one from its stale English column. + const translated = await handler().updateEntry( + { + collectionName: "i18nwin_posts", + entryId: ids[0], + overrideAccess: true, + locale: "es", + }, + { title: "Texto en español" } + ); + expect(translated.success).toBe(true); + + // Now Spanish becomes the default. Main still holds the English text. + await current.destroy(); + current = await boot(true, "es"); + + const untranslated = await handler().getEntry({ + collectionName: "i18nwin_posts", + entryId: ids[1], + overrideAccess: true, + locale: "es", + }); + // Without the guard this reads "Never translated" — English served as Spanish. + expect((untranslated.data as { title?: unknown }).title).not.toBe( + "Never translated" + ); + + // Only the two English rows and the one real Spanish translation exist; no + // Spanish row was fabricated for the untranslated entry. + const rows = await current.adapter.executeQuery<{ n: number }>( + "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales WHERE _locale = 'es'" + ); + expect(Number(rows[0]?.n)).toBe(1); + }); + it("seeds once and does not duplicate rows on later boots", async () => { // The seed runs on every boot and sync, so it must be gated on the companion // being empty. Without that gate each boot would insert another default-locale diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index b62e0fe63..ccfa6b710 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -286,19 +286,33 @@ export async function ensureCompanionTable( resolveLocalizedFieldNames(args.fields, true) ); const localizedFields = args.fields.filter(f => localizedNames.has(f.name)); - if (!alreadyExists) { - const statements = buildCompanionReconcileStatements({ - slug: args.slug, - tableName: args.tableName, - oldLocalized: [], - newLocalized: localizedFields, - dialect: args.dialect, - status: args.status === true, - companionExists: false, - }); - for (const stmt of statements) { - await adapter.executeQuery(stmt); - } + // An EXISTING companion still has to be reconciled. The schema push pipeline + // deliberately skips companion tables, so marking another field localized (or + // turning on Draft/Published) adds a column the companion never gets: reads and + // writes then address a shape the physical table does not have, and the seed + // below would reference a column that is not there. Passing the columns the + // companion actually has as `oldLocalized` makes this an ADD of exactly what is + // missing. Dropping columns is left out on purpose — a column that disappeared + // from the config still holds content, and removing it is the pipeline's gated, + // confirmable job rather than something an unattended boot should decide. + const oldLocalized = alreadyExists + ? await presentCompanionFields( + adapter, + companionTableName, + localizedFields + ) + : []; + const statements = buildCompanionReconcileStatements({ + slug: args.slug, + tableName: args.tableName, + oldLocalized, + newLocalized: localizedFields, + dialect: args.dialect, + status: args.status === true, + companionExists: alreadyExists, + }); + for (const stmt of statements) { + await adapter.executeQuery(stmt); } await seedCompanionFromMain(adapter, { slug: args.slug, @@ -333,10 +347,18 @@ export async function ensureCompanionTable( * * Deliberately narrow, because this runs unattended on every boot and sync: * - * - only for main rows that have NO default-locale companion row yet, so it never overwrites a + * - only while the companion holds NO row in a non-default language. That is what separates a + * transition still in progress from an entity that has been localized for a while, and the + * two need opposite treatment. Mid-transition the main columns are the default language and + * backfilling them is the whole point. Once real translations exist, the main columns are + * frozen leftovers that stopped being updated when writes moved to the companion — seeding + * from them would be fabrication. Changing `localization.defaultLocale` afterwards is the + * sharp case: switching `en` to `fr` would otherwise invent French rows holding stale English + * text and suppress the fallback that would have shown the real value. + * - only for main rows that have NO row in the default language yet, so it never overwrites a * translation and is safe to re-run. Row-level rather than table-level: a companion that - * already holds one row (someone edited a single entry) must not stop every other row from - * being backfilled, or those stay unreadable forever. + * already holds one such row (someone edited a single entry mid-transition) must not stop + * every other row from being backfilled, or those stay unreadable forever. * - only for localized columns that are STILL on the main table, probed one by one. After the * columns are dropped there is nothing to copy, and the probe is the only portable way to ask. * - it does NOT drop those columns afterwards. That is the destructive half of the transition @@ -371,6 +393,10 @@ async function seedCompanionFromMain( }); if (!spec) return; + // The transition test. See the note above: once any other language has a row, + // the main columns are stale leftovers rather than the default language. + if (await companionHasTranslations(adapter, args)) return; + const columnsOnMain: string[] = []; for (const column of spec.columns) { if (await mainHasColumn(adapter, args.tableName, column.name)) { @@ -389,6 +415,47 @@ async function seedCompanionFromMain( if (seed) await adapter.executeQuery(seed); } +/** + * The subset of `fields` whose physical column the companion already has. Feeding this back as + * `oldLocalized` turns the reconcile into "add what is missing", with no dependency on knowing + * the entity's previous config. + */ +async function presentCompanionFields( + adapter: CompanionWriteAdapter, + companionTableName: string, + fields: CompanionFieldLike[] +): Promise { + const present: CompanionFieldLike[] = []; + for (const field of fields) { + if ( + await mainHasColumn(adapter, companionTableName, toColumn(field.name)) + ) { + present.push(field); + } + } + return present; +} + +/** + * Whether the companion already holds a row in some language other than the default — the signal + * that this entity is past its transition and its main columns are no longer the default language. + */ +async function companionHasTranslations( + adapter: CompanionWriteAdapter, + args: { companionTableName: string; defaultLocale?: string } +): Promise { + const isMysql = adapter.dialect === "mysql"; + const table = isMysql + ? `\`${args.companionTableName}\`` + : `"${args.companionTableName}"`; + const locale = isMysql ? "`_locale`" : `"_locale"`; + const rows = await adapter.executeQuery( + `SELECT 1 FROM ${table} WHERE ${locale} <> ${adapter.dialect === "postgresql" ? "$1" : "?"} LIMIT 1`, + [args.defaultLocale] + ); + return rows.length > 0; +} + /** Whether a physical column is still present on the main table. */ async function mainHasColumn( adapter: CompanionWriteAdapter, @@ -401,13 +468,38 @@ async function mainHasColumn( try { await adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); return true; - } catch { - // Catching everything is safe HERE, unlike a probe a write gates on. Treating - // any answer but "yes" as "do not copy from this column" only ever narrows the - // seed; it can never write the wrong value. And a failure that is not a - // missing column — an unreachable database — does not get swallowed overall, - // because the seed statement that follows runs on the same connection and - // propagates to the caller's reporter. - return false; + } catch (error) { + // Only a verified "no such column" answers this question with `false`. Swallowing + // everything would turn an unreachable database or a missing SELECT grant into + // "none of these columns exist", and the caller then finds nothing to copy and + // returns BEFORE running the seed — so no statement would fail, no reporter would + // fire, and `ensureCompanionTable` would report success over a companion that is + // still empty and content that still reads as null. Anything else propagates. + if (isMissingColumnError(error)) return false; + throw error; } } + +// The dialect's own wording for a column that is not there: Postgres +// `column "x" does not exist`, SQLite `no such column: x`, MySQL +// `Unknown column 'x' in 'field list'`. Column-specific on purpose — a bare +// `does not exist` also matches a missing table or database, which must +// propagate rather than be read as an absent column. +function isMissingColumnError(error: unknown): boolean { + // Drizzle reports `Failed query: ...` and keeps the driver's own wording on + // `cause`, so both levels have to be read or a real missing column would be + // misread as an unrelated failure and thrown. + const message = [ + error instanceof Error ? error.message : String(error), + error instanceof Error && error.cause instanceof Error + ? error.cause.message + : "", + ] + .join(" ") + .toLowerCase(); + return ( + /column .* does not exist/.test(message) || + message.includes("no such column") || + /unknown column/.test(message) + ); +} diff --git a/packages/nextly/src/domains/i18n/writes-create.integration.test.ts b/packages/nextly/src/domains/i18n/writes-create.integration.test.ts index 629ede016..7f8a8d69f 100644 --- a/packages/nextly/src/domains/i18n/writes-create.integration.test.ts +++ b/packages/nextly/src/domains/i18n/writes-create.integration.test.ts @@ -132,7 +132,7 @@ describe("createEntry — localized write routing (M5a)", () => { expect(rows).toEqual([{ _locale: "en", heading: "Hello" }]); }); - it("dev (no companion table): localized value stays on the main table, write succeeds", async () => { + it("dev (no companion table): the DEFAULT language stays on the main table, write succeeds", async () => { const t = await boot(); const handler = handlerOf(t); const adapter = t.adapter as unknown as { @@ -146,13 +146,38 @@ describe("createEntry — localized write routing (M5a)", () => { await adapter.executeQuery( 'ALTER TABLE "dc_pages" ADD COLUMN "heading" text' ); + const res = await handler.createEntry( + { collectionName: "pages", locale: "en", overrideAccess: true }, + { title: "T", heading: "Hello" } + ); + expect(res.success).toBe(true); + const rows = await adapter.executeQuery('SELECT "heading" FROM "dc_pages"'); + expect(rows).toEqual([{ heading: "Hello" }]); + }); + + it("dev (no companion table): a NON-default language is refused", async () => { + const t = await boot(); + const handler = handlerOf(t); + const adapter = t.adapter as unknown as { + executeQuery: (sql: string) => Promise[]>; + }; + await adapter.executeQuery('DROP TABLE IF EXISTS "dc_pages_locales"'); + await adapter.executeQuery( + 'ALTER TABLE "dc_pages" ADD COLUMN "heading" text' + ); + // The main table has no language of its own: whatever sits there is read as the + // DEFAULT language, and that is what the companion seed copies across when the + // table appears. Letting `de` land on main would publish German text as English + // and leave German itself with no translation, so the write is refused instead. const res = await handler.createEntry( { collectionName: "pages", locale: "de", overrideAccess: true }, { title: "T", heading: "Hallo" } ); - expect(res.success).toBe(true); + expect(res.success).toBe(false); + expect(res.statusCode).toBe(409); + expect(res.message).toMatch(/Translations are not ready/); const rows = await adapter.executeQuery('SELECT "heading" FROM "dc_pages"'); - expect(rows).toEqual([{ heading: "Hallo" }]); + expect(rows).toEqual([]); }); }); diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 23ca7ea5c..97cb92133 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -1082,12 +1082,21 @@ export class SingleMutationService extends BaseService { // row, not the main table. `companion` and `writeLocale` were resolved // above (before validation); the split reuses them. Done inside the // closure so a retry re-splits the freshly-timestamped payload. - const { main: mainPayload, companion: companionData } = companion - ? splitLocalizedWrite(updatePayload, companion.localizedFields) - : { - main: updatePayload, - companion: {} as Record, - }; + // Only split when the companion physically exists. Splitting first and + // then skipping the companion upsert (which is gated on the same flag + // below) would drop the translatable values on the floor: they leave + // `mainPayload` and are never written anywhere, so the write reports + // success having saved nothing. While the table is absent those values + // belong on the main table, which is the same pre-migration fallback + // collections use — and by then the write locale is guaranteed to be + // the default one, because the guard above refuses any other. + const { main: mainPayload, companion: companionData } = + companion && companionPhysicallyExists + ? splitLocalizedWrite(updatePayload, companion.localizedFields) + : { + main: updatePayload, + companion: {} as Record, + }; // per-locale status. The status the companion row carries — // from `updatePayload` (not `mainPayload`, which may have `status` From 0bcc5307750e22676f96f3078785bae464edbd98 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 08:40:03 +0500 Subject: [PATCH 06/20] refactor(nextly): read companion shape through schema introspection --- .../src/domains/i18n/runtime/companion-io.ts | 136 +++++++----------- 1 file changed, 53 insertions(+), 83 deletions(-) diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index ccfa6b710..31dcbd6f0 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -126,6 +126,42 @@ interface CompanionWriteAdapter { executeQuery(sql: string, params?: unknown[]): Promise; } +/** + * What provisioning additionally needs: the Drizzle handle, so the physical shape of a table can + * be read through the shared introspection helper instead of being guessed at with probe queries. + * Kept separate from {@link CompanionWriteAdapter} so the read/write helpers, which never + * introspect, keep their narrower contract. + */ +interface CompanionProvisionAdapter extends CompanionWriteAdapter { + getDrizzle(): T; +} + +/** + * The physical columns of `tableNames`, keyed by table, via the same introspection the schema + * pipeline uses. One round trip answers for every table and column at once, and — unlike a + * `SELECT ... LIMIT 0` probe — a failure is a real failure rather than being indistinguishable + * from "that column is not there". + */ +async function readPhysicalColumns( + adapter: CompanionProvisionAdapter, + tableNames: string[] +): Promise>> { + const { introspectLiveSnapshot } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const snapshot = await introspectLiveSnapshot( + adapter.getDrizzle(), + adapter.dialect, + tableNames + ); + return new Map( + snapshot.tables.map(table => [ + table.name, + new Set(table.columns.map(column => column.name)), + ]) + ); +} + /** * Upsert the companion `_locales` row for `(parentId, locale)` with the provided localized * columns. Only the supplied columns are written; other locales' rows and other columns on this @@ -252,7 +288,7 @@ export async function companionHasStatusColumn( * retries on the next boot. */ export async function ensureCompanionTable( - adapter: CompanionWriteAdapter, + adapter: CompanionProvisionAdapter, args: { slug: string; tableName: string; @@ -295,12 +331,15 @@ export async function ensureCompanionTable( // missing. Dropping columns is left out on purpose — a column that disappeared // from the config still holds content, and removing it is the pipeline's gated, // confirmable job rather than something an unattended boot should decide. + // One introspection answers both questions below: which localized columns the + // companion already has, and which are still on the main table for the seed. + const physical = await readPhysicalColumns(adapter, [ + args.tableName, + companionTableName, + ]); + const companionColumns = physical.get(companionTableName) ?? new Set(); const oldLocalized = alreadyExists - ? await presentCompanionFields( - adapter, - companionTableName, - localizedFields - ) + ? localizedFields.filter(f => companionColumns.has(toColumn(f.name))) : []; const statements = buildCompanionReconcileStatements({ slug: args.slug, @@ -315,6 +354,7 @@ export async function ensureCompanionTable( await adapter.executeQuery(stmt); } await seedCompanionFromMain(adapter, { + mainColumns: physical.get(args.tableName) ?? new Set(), slug: args.slug, tableName: args.tableName, companionTableName, @@ -359,8 +399,8 @@ export async function ensureCompanionTable( * translation and is safe to re-run. Row-level rather than table-level: a companion that * already holds one such row (someone edited a single entry mid-transition) must not stop * every other row from being backfilled, or those stay unreadable forever. - * - only for localized columns that are STILL on the main table, probed one by one. After the - * columns are dropped there is nothing to copy, and the probe is the only portable way to ask. + * - only for localized columns that are STILL on the main table, read from the introspected + * shape. After the columns are dropped there is nothing left to copy. * - it does NOT drop those columns afterwards. That is the destructive half of the transition * and the schema pipeline gates it behind an explicit confirmation; making the content visible * again does not require it, and doing it here would route around that gate. @@ -368,6 +408,8 @@ export async function ensureCompanionTable( async function seedCompanionFromMain( adapter: CompanionWriteAdapter, args: { + /** Physical columns the main table currently has, from the caller's introspection. */ + mainColumns: Set; slug: string; tableName: string; companionTableName: string; @@ -397,12 +439,9 @@ async function seedCompanionFromMain( // the main columns are stale leftovers rather than the default language. if (await companionHasTranslations(adapter, args)) return; - const columnsOnMain: string[] = []; - for (const column of spec.columns) { - if (await mainHasColumn(adapter, args.tableName, column.name)) { - columnsOnMain.push(column.name); - } - } + const columnsOnMain = spec.columns + .filter(column => args.mainColumns.has(column.name)) + .map(column => column.name); if (columnsOnMain.length === 0) return; const { buildCompanionSeedStatement } = await import( @@ -415,27 +454,6 @@ async function seedCompanionFromMain( if (seed) await adapter.executeQuery(seed); } -/** - * The subset of `fields` whose physical column the companion already has. Feeding this back as - * `oldLocalized` turns the reconcile into "add what is missing", with no dependency on knowing - * the entity's previous config. - */ -async function presentCompanionFields( - adapter: CompanionWriteAdapter, - companionTableName: string, - fields: CompanionFieldLike[] -): Promise { - const present: CompanionFieldLike[] = []; - for (const field of fields) { - if ( - await mainHasColumn(adapter, companionTableName, toColumn(field.name)) - ) { - present.push(field); - } - } - return present; -} - /** * Whether the companion already holds a row in some language other than the default — the signal * that this entity is past its transition and its main columns are no longer the default language. @@ -455,51 +473,3 @@ async function companionHasTranslations( ); return rows.length > 0; } - -/** Whether a physical column is still present on the main table. */ -async function mainHasColumn( - adapter: CompanionWriteAdapter, - tableName: string, - columnName: string -): Promise { - const isMysql = adapter.dialect === "mysql"; - const table = isMysql ? `\`${tableName}\`` : `"${tableName}"`; - const column = isMysql ? `\`${columnName}\`` : `"${columnName}"`; - try { - await adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); - return true; - } catch (error) { - // Only a verified "no such column" answers this question with `false`. Swallowing - // everything would turn an unreachable database or a missing SELECT grant into - // "none of these columns exist", and the caller then finds nothing to copy and - // returns BEFORE running the seed — so no statement would fail, no reporter would - // fire, and `ensureCompanionTable` would report success over a companion that is - // still empty and content that still reads as null. Anything else propagates. - if (isMissingColumnError(error)) return false; - throw error; - } -} - -// The dialect's own wording for a column that is not there: Postgres -// `column "x" does not exist`, SQLite `no such column: x`, MySQL -// `Unknown column 'x' in 'field list'`. Column-specific on purpose — a bare -// `does not exist` also matches a missing table or database, which must -// propagate rather than be read as an absent column. -function isMissingColumnError(error: unknown): boolean { - // Drizzle reports `Failed query: ...` and keeps the driver's own wording on - // `cause`, so both levels have to be read or a real missing column would be - // misread as an unrelated failure and thrown. - const message = [ - error instanceof Error ? error.message : String(error), - error instanceof Error && error.cause instanceof Error - ? error.cause.message - : "", - ] - .join(" ") - .toLowerCase(); - return ( - /column .* does not exist/.test(message) || - message.includes("no such column") || - /unknown column/.test(message) - ); -} From 904690b15f594afcc5a4e9d3df285b3375223601 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 09:53:42 +0500 Subject: [PATCH 07/20] fix(nextly): provision companions on the reload path and guard production --- packages/nextly/src/cli/commands/dev-build.ts | 6 ++ ...rite-without-companion.integration.test.ts | 42 +++++--- .../src/domains/i18n/runtime/companion-io.ts | 92 ++++++++++++------ ...ngle-without-companion.integration.test.ts | 92 ++++++++++++++++++ packages/nextly/src/init/reload-config.ts | 97 ++++++++++++++++++- 5 files changed, 284 insertions(+), 45 deletions(-) create mode 100644 packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index 0ab91af97..ce3c968da 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -792,6 +792,12 @@ export async function ensureLocalizedCompanions( context: CommandContext ): Promise { const { logger } = context; + // Same policy `performAutoSync` applies: production never gets unattended schema + // changes, and this issues DDL and can copy rows. Enforced here rather than at + // each call site so no caller can reintroduce the hole. In production the + // companion is `nextly migrate`'s job, and the write guard keeps a non-default + // write from destroying content until it runs. + if (process.env.NODE_ENV === "production") return; const dialect = adapter.getCapabilities().dialect; const { ensureCompanionTable } = await import( "../../domains/i18n/runtime/companion-io" diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 495b4ee7d..cf9cf6b9f 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -214,11 +214,16 @@ describe("enabling localization on existing content (integration)", () => { expect(await physicalTitle(current)).toBe("Original"); }); - it("backfills entries the companion is missing, not just an empty one", async () => { - // The seed is per-row, not per-table. A companion that already holds a row for - // ONE entry — someone edited a single entry in the default language — must not - // stop every other entry from being backfilled; gating on "the companion is - // empty" would leave the rest reading null permanently. + it("leaves a partly populated companion alone, keeping the main values intact", async () => { + // Once ANY per-locale row exists, the main columns can no longer be declared to + // be the default language: nothing records which language they hold, and the + // rows present may be in another one. Backfilling the remaining entries would + // therefore risk labelling their content as a language it is not. + // + // The cost is that those entries keep reading null until an operator acts, and + // that is the right side to err on — unreadable content is still intact on the + // main table and recoverable, whereas a mislabelled translation is silently + // wrong. This test pins both halves: no backfill, and no data loss. current = await boot(false); const handler = () => current!.getService("collectionsHandler"); @@ -245,14 +250,25 @@ describe("enabling localization on existing content (integration)", () => { await current.destroy(); current = await boot(true); - for (const [index, title] of ["First", "Second", "Third"].entries()) { - const read = await handler().getEntry({ - collectionName: "i18nwin_posts", - entryId: ids[index], - overrideAccess: true, - }); - expect((read.data as { title?: unknown }).title).toBe(title); - } + // The entry whose row survived still reads; the other two do not get one. + const kept = await handler().getEntry({ + collectionName: "i18nwin_posts", + entryId: ids[0], + overrideAccess: true, + }); + expect((kept.data as { title?: unknown }).title).toBe("First"); + + const rows = await current.adapter.executeQuery<{ n: number }>( + "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales" + ); + expect(Number(rows[0]?.n)).toBe(1); + + // Nothing was lost: the values the other two entries had are still on main, + // so an operator can complete the transition without recovering from backups. + const physical = await current.adapter.executeQuery<{ title: string }>( + "SELECT title FROM dc_i18nwin_posts ORDER BY title" + ); + expect(physical.map(r => r.title)).toEqual(["First", "Second", "Third"]); }); it("does not fabricate rows from stale columns after the default language changes", async () => { diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 31dcbd6f0..5b6942cda 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -349,6 +349,15 @@ export async function ensureCompanionTable( dialect: args.dialect, status: args.status === true, companionExists: alreadyExists, + // The PHYSICAL `_status` state, which is what decides whether turning + // Draft/Published on for an already-localized entity needs the column added. + // Omitting it left the companion without `_status` while the runtime schema + // was built with `hasStatus: true`, so the next per-locale status read or + // write hit a missing column. Undefined while the table does not exist yet, + // where the CREATE already includes it. + companionHasStatus: alreadyExists + ? companionColumns.has("_status") + : undefined, }); for (const stmt of statements) { await adapter.executeQuery(stmt); @@ -387,18 +396,20 @@ export async function ensureCompanionTable( * * Deliberately narrow, because this runs unattended on every boot and sync: * - * - only while the companion holds NO row in a non-default language. That is what separates a - * transition still in progress from an entity that has been localized for a while, and the - * two need opposite treatment. Mid-transition the main columns are the default language and - * backfilling them is the whole point. Once real translations exist, the main columns are - * frozen leftovers that stopped being updated when writes moved to the companion — seeding - * from them would be fabrication. Changing `localization.defaultLocale` afterwards is the - * sharp case: switching `en` to `fr` would otherwise invent French rows holding stale English - * text and suppress the fallback that would have shown the real value. - * - only for main rows that have NO row in the default language yet, so it never overwrites a - * translation and is safe to re-run. Row-level rather than table-level: a companion that - * already holds one such row (someone edited a single entry mid-transition) must not stop - * every other row from being backfilled, or those stay unreadable forever. + * - only while the companion is COMPLETELY EMPTY. Nothing weaker is sound. The main columns + * carry no record of which language they hold, so the only safe moment to declare them the + * default is when no per-locale content exists at all and there is therefore nothing that + * could be mislabelled. Narrowing this to "no row in a NON-default language" looks equivalent + * and is not: a companion holding only partial French rows, read after `defaultLocale` moves + * from `en` to `fr`, contains no non-French row, so stale English main columns would be + * seeded as French. Inferring the transition from the CURRENT default is what makes that + * possible, so this does not infer it at all. + * The cost is that a companion which already holds one row is never backfilled, and any entry + * still missing its default-language row stays unreadable until the operator acts. That is the + * right side to err on: unreadable content is intact on the main table and recoverable, while + * a fabricated translation is silently wrong and nearly undetectable. + * - even then, only for main rows with no row in the default language, so a retry or a + * concurrent write cannot produce a duplicate. * - only for localized columns that are STILL on the main table, read from the introspected * shape. After the columns are dropped there is nothing left to copy. * - it does NOT drop those columns afterwards. That is the destructive half of the transition @@ -406,7 +417,7 @@ export async function ensureCompanionTable( * again does not require it, and doing it here would route around that gate. */ async function seedCompanionFromMain( - adapter: CompanionWriteAdapter, + adapter: CompanionProvisionAdapter, args: { /** Physical columns the main table currently has, from the caller's introspection. */ mainColumns: Set; @@ -435,9 +446,9 @@ async function seedCompanionFromMain( }); if (!spec) return; - // The transition test. See the note above: once any other language has a row, - // the main columns are stale leftovers rather than the default language. - if (await companionHasTranslations(adapter, args)) return; + // The transition test. See the note above: any per-locale content at all means + // the main columns can no longer be safely declared to be the default language. + if (!(await companionIsEmpty(adapter, args))) return; const columnsOnMain = spec.columns .filter(column => args.mainColumns.has(column.name)) @@ -454,22 +465,41 @@ async function seedCompanionFromMain( if (seed) await adapter.executeQuery(seed); } +/** The slice of a Drizzle handle this needs: one bounded read of one table. */ +interface CompanionRowReader { + select(): { + from(table: unknown): { limit(n: number): PromiseLike }; + }; +} + /** - * Whether the companion already holds a row in some language other than the default — the signal - * that this entity is past its transition and its main columns are no longer the default language. + * Whether the companion holds no rows at all — the only state in which the main columns can be + * declared to be the default language without inferring anything (see the note on the seed). + * + * Goes through Drizzle rather than assembling SQL: the companion table object is built from the + * same descriptor the runtime registers, so quoting and dialect differences are the ORM's problem + * rather than being re-implemented here. */ -async function companionHasTranslations( - adapter: CompanionWriteAdapter, - args: { companionTableName: string; defaultLocale?: string } +async function companionIsEmpty( + adapter: CompanionProvisionAdapter, + args: { + slug: string; + tableName: string; + localizedFields: CompanionFieldLike[]; + dialect: SupportedDialect; + status: boolean; + } ): Promise { - const isMysql = adapter.dialect === "mysql"; - const table = isMysql - ? `\`${args.companionTableName}\`` - : `"${args.companionTableName}"`; - const locale = isMysql ? "`_locale`" : `"_locale"`; - const rows = await adapter.executeQuery( - `SELECT 1 FROM ${table} WHERE ${locale} <> ${adapter.dialect === "postgresql" ? "$1" : "?"} LIMIT 1`, - [args.defaultLocale] - ); - return rows.length > 0; + const companion = buildCompanionRuntimeTable({ + slug: args.slug, + tableName: args.tableName, + fields: args.localizedFields, + dialect: args.dialect, + localized: true, + status: args.status, + }); + if (!companion) return false; + const db = adapter.getDrizzle(); + const rows = await db.select().from(companion.table).limit(1); + return rows.length === 0; } diff --git a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts new file mode 100644 index 000000000..ecd580729 --- /dev/null +++ b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts @@ -0,0 +1,92 @@ +/** + * A localized Single whose companion table is missing must still persist a write in the + * DEFAULT language. + * + * While the companion is absent, translatable values belong on the main table — the same + * pre-migration fallback collections use. But a localized Single's registered runtime table is + * generated WITHOUT its translatable columns (`runtime-schema-generator` skips them as + * companion-owned), so keeping those values in the main payload writes them through a schema + * that does not declare them. If the ORM drops such keys, the update reports success while + * saving nothing, and the submitted content is gone with no error anywhere. + * + * The physical read is the point: the runtime schema omits the column, so an API read cannot + * distinguish "saved" from "silently dropped". + */ +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { defineSingle, text } from "../../../config"; +import { createAdapter } from "../../../database/factory"; +import { + createTestNextly, + type TestNextly, +} from "../../../plugins/test-nextly"; + +let dir: string; +let dbPath: string; +let current: TestNextly | undefined; + +beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), "nextly-single-window-")); + dbPath = join(dir, "test.db"); +}); + +afterEach(async () => { + await current?.destroy(); + current = undefined; + rmSync(dir, { recursive: true, force: true }); +}); + +const localization = { locales: ["en", "es"], defaultLocale: "en" }; + +const settings = (localized: boolean) => + defineSingle({ + slug: "swin_settings", + localized, + fields: [text({ name: "headline", localized: true })], + }); + +async function boot(localized: boolean): Promise { + process.env.DB_DIALECT = "sqlite"; + const adapter = await createAdapter({ + type: "sqlite", + url: `file:${dbPath}`, + } as Parameters[0]); + return createTestNextly({ + adapter, + singles: [settings(localized)], + localization, + }); +} + +describe("localized single without a companion table (integration)", () => { + it("persists a default-language write to the main table", async () => { + // Boot 1: not localized, so `headline` is an ordinary main-table column. + current = await boot(false); + await current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Original" }, + } as Parameters[0]); + + // Re-open localized, then remove the companion to recreate the window. + await current.destroy(); + current = await boot(true); + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS single_swin_settings_locales" + ); + + await current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Edited while the companion was missing" }, + locale: "en", + } as Parameters[0]); + + const rows = await current.adapter.executeQuery<{ headline: string }>( + "SELECT headline FROM single_swin_settings" + ); + expect(rows[0]?.headline).toBe("Edited while the companion was missing"); + }); +}); diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 76a9475de..936aad08e 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -98,7 +98,10 @@ type LoggerLike = { // adapter-drizzle would couple this module to the adapter package. interface AdapterLike { readonly dialect: "postgresql" | "mysql" | "sqlite"; - getDrizzle(): unknown; + getDrizzle(): T; + // Needed to provision the localized companion below: creating it is DDL, and + // seeding it from the main table is a write. + executeQuery(sql: string, params?: unknown[]): Promise; } type CollectionDef = { @@ -552,6 +555,87 @@ function republishRecordingPolicies( ); } +/** + * Create and seed the `_locales` companion of every localized collection, single and field group + * in the reloaded config. + * + * The reload path is the `next dev` counterpart to the CLI's `ensureLocalizedCompanions`: it is + * where a config edit lands when the app is running under plain `next dev` rather than + * `nextly db:sync --watch`. `ensureCompanionTable` is idempotent, so entities that already have + * their companion cost one introspection each. + * + * Never throws: a companion that cannot be provisioned must not take down a config reload. The + * write guard in the mutation services is what protects content in the meantime. + */ +async function ensureLocalizedCompanionsForReload( + adapter: AdapterLike, + config: { + collections?: unknown[]; + singles?: unknown[]; + fieldGroups?: unknown[]; + localization?: { defaultLocale?: string }; + } +): Promise { + // Same policy the CLI applies: production schema changes belong to `nextly migrate`. + if (process.env.NODE_ENV === "production") return; + + const { ensureCompanionTable } = await import( + "../domains/i18n/runtime/companion-io" + ); + const { resolveCollectionTableName, resolveComponentTableName } = + await import("../domains/schema/utils/resolve-table-name"); + const { resolveSingleTableName } = await import( + "../domains/singles/services/resolve-single-table-name" + ); + + type Localizable = { + slug?: string; + dbName?: string; + localized?: boolean; + status?: boolean; + fields?: { name: string; type: string; localized?: boolean }[]; + }; + const groups: [Localizable[], (e: Localizable) => string][] = [ + [ + (config.collections ?? []) as Localizable[], + e => resolveCollectionTableName(e.slug!, e.dbName), + ], + [ + (config.singles ?? []) as Localizable[], + e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), + ], + [ + (config.fieldGroups ?? []) as Localizable[], + e => resolveComponentTableName(e.slug!), + ], + ]; + + const defaultLocale = config.localization?.defaultLocale; + for (const [entities, resolveTableName] of groups) { + for (const entity of entities) { + if (!entity.slug || entity.localized !== true) continue; + await ensureCompanionTable( + adapter, + { + slug: entity.slug, + tableName: resolveTableName(entity), + fields: entity.fields ?? [], + dialect: adapter.dialect, + status: entity.status === true, + defaultLocale, + }, + error => { + console.warn( + `[nextly] Could not prepare the translations table for "${entity.slug}". ` + + `Writes in a non-default locale will be refused until it exists: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + } + ); + } + } +} + // Reload entry point. resolver is optional and exists primarily for tests. // dispatcher is also test-only: injects a fake PromptDispatcher (e.g., one // that records prompts and auto-confirms) so tests don't need a real TTY. @@ -636,6 +720,7 @@ async function runReload(opts?: { singles?: SingleDef[]; fieldGroups?: ComponentDef[]; webhookAuditEnabled?: boolean; + localization?: { defaultLocale?: string }; } | undefined; let previousFieldTypes: PluginFieldType[] | undefined; @@ -1405,6 +1490,16 @@ async function runReload(opts?: { singles: singleSynced && componentSynced, }); + // Physically provision the `_locales` companion of every localized entity + // before the runtime descriptors below register it. `next dev` routes config + // edits here rather than through the CLI watcher, so without this, turning on + // localization during ordinary HMR registered a companion the database did + // not have: the admin rendered the full localization UI, non-default writes + // were refused, and the main columns could still be dropped before anything + // had copied them across. Seeding is part of the same call, which is why it + // must run before the drop the schema apply may perform. + await ensureLocalizedCompanionsForReload(adapter, newConfig); + // Pre-compute fresh Drizzle table objects for all affected collections, // singles, and components. Synchronous (schema generation, no DB I/O). // Shared between the cache-refresh blocks below so we don't generate twice. From ca77085514826db5022306f1175ce3e3b1986b2c Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 10:24:35 +0500 Subject: [PATCH 08/20] fix(nextly): carry a newly localized field's shared value to the companion --- ...rite-without-companion.integration.test.ts | 59 ++++++++++++++++++ .../src/domains/i18n/runtime/companion-io.ts | 62 ++++++++++++++++++- 2 files changed, 120 insertions(+), 1 deletion(-) diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index cf9cf6b9f..278ca9e32 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -62,6 +62,17 @@ const posts = (localized: boolean) => fields: [text({ name: "title", localized: true })], }); +/** Adds a second field that starts SHARED and later becomes localized. */ +const postsWithTagline = (taglineLocalized: boolean) => + defineCollection({ + slug: "i18nwin_posts", + localized: true, + fields: [ + text({ name: "title", localized: true }), + text({ name: "tagline", localized: taglineLocalized }), + ], + }); + async function boot( localized: boolean, defaultLocale = localization.defaultLocale @@ -331,6 +342,54 @@ describe("enabling localization on existing content (integration)", () => { expect(Number(rows[0]?.n)).toBe(1); }); + it("carries a newly localized field's shared value onto the companion", async () => { + // A field that was SHARED lives on the main table and applies to every language. + // Marking it localized adds a companion column, and that column starts empty — + // so without a backfill the value stops being returned the moment the field + // becomes translatable, even though it is still sitting on the main table. + // + // This runs in the state the general seed refuses (the companion already holds + // rows), and is safe for a different reason: the column is brand new, so there + // is no per-locale content in it that could be mislabelled. + process.env.DB_DIALECT = "sqlite"; + const openWith = async (taglineLocalized: boolean): Promise => { + const adapter = await createAdapter({ + type: "sqlite", + url: `file:${dbPath}`, + } as Parameters[0]); + return createTestNextly({ + adapter, + collections: [postsWithTagline(taglineLocalized)], + localization, + }); + }; + + current = await openWith(false); + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "i18nwin_posts", overrideAccess: true }, + { title: "Title", tagline: "Shared tagline" } + ); + const id = (created.data as { id: string }).id; + + // Localize `tagline`. The companion already exists and holds the title's rows, + // so this is precisely the case the empty-companion seed skips. + await current.destroy(); + current = await openWith(true); + await current.destroy(); + current = await openWith(true); + + const read = await current + .getService("collectionsHandler") + .getEntry({ + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + }); + expect((read.data as { tagline?: unknown }).tagline).toBe("Shared tagline"); + }); + it("seeds once and does not duplicate rows on later boots", async () => { // The seed runs on every boot and sync, so it must be gated on the companion // being empty. Without that gate each boot would insert another default-locale diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 5b6942cda..741ae8142 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -362,8 +362,26 @@ export async function ensureCompanionTable( for (const stmt of statements) { await adapter.executeQuery(stmt); } + const mainColumns = physical.get(args.tableName) ?? new Set(); + // Columns the reconcile just ADDED to an existing companion. They arrive empty, + // so a field that was SHARED until now — its value on the main table, applying + // to every language — would start reading as null the moment it becomes + // localized. Copy it across before anything reads through the new column. + if (alreadyExists) { + const added = localizedFields + .map(f => toColumn(f.name)) + .filter( + column => !companionColumns.has(column) && mainColumns.has(column) + ); + await backfillAddedCompanionColumns(adapter, { + companionTableName, + tableName: args.tableName, + columns: added, + defaultLocale: args.defaultLocale, + }); + } await seedCompanionFromMain(adapter, { - mainColumns: physical.get(args.tableName) ?? new Set(), + mainColumns, slug: args.slug, tableName: args.tableName, companionTableName, @@ -465,6 +483,48 @@ async function seedCompanionFromMain( if (seed) await adapter.executeQuery(seed); } +/** + * Copy the main table's value into companion columns the reconcile has just added, for the + * DEFAULT language's rows only. + * + * This is narrower than the general seed and safe for the reason the general seed is not. The + * column is brand new on the companion, so no per-locale content exists in it that could be + * mislabelled, and the value being copied was SHARED until this save — language-neutral by + * definition, not a translation of anything. Whether the companion holds other rows is therefore + * irrelevant here, which is why this runs even in the state the seed refuses. + * + * Default language only: other languages fall back to it, so writing the same value into each + * would duplicate content rather than preserve it. + * + * Correlated scalar subquery because one statement then covers every dialect — verified on + * SQLite, Postgres and MySQL, where a join form would have needed three spellings. + */ +async function backfillAddedCompanionColumns( + adapter: CompanionWriteAdapter, + args: { + companionTableName: string; + tableName: string; + columns: string[]; + defaultLocale?: string; + } +): Promise { + if (!args.defaultLocale || args.columns.length === 0) return; + const isMysql = adapter.dialect === "mysql"; + const q = (id: string) => (isMysql ? `\`${id}\`` : `"${id}"`); + const companion = q(args.companionTableName); + const main = q(args.tableName); + const placeholder = adapter.dialect === "postgresql" ? "$1" : "?"; + + for (const column of args.columns) { + await adapter.executeQuery( + `UPDATE ${companion} SET ${q(column)} = ` + + `(SELECT ${q(column)} FROM ${main} WHERE ${main}.${q("id")} = ${companion}.${q("_parent")}) ` + + `WHERE ${companion}.${q("_locale")} = ${placeholder}`, + [args.defaultLocale] + ); + } +} + /** The slice of a Drizzle handle this needs: one bounded read of one table. */ interface CompanionRowReader { select(): { From c3d470ec4c38d2c5100d8c3c86daa1872e0d215b Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 12:26:10 +0500 Subject: [PATCH 09/20] fix(nextly): keep unattended companion reconciliation additive and gated --- ...nc-localized-companion.integration.test.ts | 42 +++++++++++++ packages/nextly/src/cli/commands/dev-build.ts | 3 + ...rite-without-companion.integration.test.ts | 59 ------------------- .../src/domains/i18n/runtime/companion-io.ts | 36 ++++++++++- packages/nextly/src/init/reload-config.ts | 3 + 5 files changed, 81 insertions(+), 62 deletions(-) diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts index fd7e03523..d6e72a019 100644 --- a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -180,6 +180,48 @@ describe("db:sync creates localized companion tables in-process (integration)", expect(await tableExists("dbsync_notes_locales")).toBe(false); }); + it("carries a newly localized field's shared value onto an existing companion", async () => { + // A field that was SHARED lives on the main table and applies to every language. + // Marking it localized adds a companion column, and that column starts empty — so + // without a backfill the value stops being returned the moment the field becomes + // translatable, even though it is still on the main table. + // + // Driven through `db:sync` rather than app boot on purpose: reconciling an + // EXISTING companion is opt-in, and boot deliberately does not opt in, because it + // would otherwise run ALTER TABLE in production off a metadata change. + const withTagline = (taglineLocalized: boolean) => + defineConfig({ + localization: { locales: ["en", "es"], defaultLocale: "en" }, + collections: [ + defineCollection({ + slug: "dbsync_notes2", + localized: true, + fields: [ + text({ name: "title", localized: true }), + text({ name: "tagline", localized: taglineLocalized }), + ], + }), + ], + }); + + await runSync(withTagline(false)); + // A row with the shared value on main and a default-locale companion row, which + // is what makes the `onlyMissing` seed skip it. + await adapter!.executeQuery( + `INSERT INTO "dc_dbsync_notes2" ("id", "title", "slug", "tagline") VALUES ('n1', 'T', 't', 'Shared tagline')` + ); + await adapter!.executeQuery( + `INSERT INTO "dc_dbsync_notes2_locales" ("_parent", "_locale", "title") VALUES ('n1', 'en', 'T')` + ); + + await runSync(withTagline(true)); + + const rows = await adapter!.executeQuery<{ tagline: string | null }>( + `SELECT "tagline" FROM "dc_dbsync_notes2_locales" WHERE "_locale" = 'en'` + ); + expect(rows[0]?.tagline).toBe("Shared tagline"); + }); + it("leaves a non-localized collection with no companion", async () => { await runSync( defineConfig({ diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index ce3c968da..7fec0a16e 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -862,6 +862,9 @@ export async function ensureLocalizedCompanions( dialect, status: entity.status === true, defaultLocale, + // These two callers already refuse to run in production and are gated on + // auto-sync, so they are the ones allowed to ALTER an existing companion. + reconcileExisting: true, }, error => { logger.warn( diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 278ca9e32..cf9cf6b9f 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -62,17 +62,6 @@ const posts = (localized: boolean) => fields: [text({ name: "title", localized: true })], }); -/** Adds a second field that starts SHARED and later becomes localized. */ -const postsWithTagline = (taglineLocalized: boolean) => - defineCollection({ - slug: "i18nwin_posts", - localized: true, - fields: [ - text({ name: "title", localized: true }), - text({ name: "tagline", localized: taglineLocalized }), - ], - }); - async function boot( localized: boolean, defaultLocale = localization.defaultLocale @@ -342,54 +331,6 @@ describe("enabling localization on existing content (integration)", () => { expect(Number(rows[0]?.n)).toBe(1); }); - it("carries a newly localized field's shared value onto the companion", async () => { - // A field that was SHARED lives on the main table and applies to every language. - // Marking it localized adds a companion column, and that column starts empty — - // so without a backfill the value stops being returned the moment the field - // becomes translatable, even though it is still sitting on the main table. - // - // This runs in the state the general seed refuses (the companion already holds - // rows), and is safe for a different reason: the column is brand new, so there - // is no per-locale content in it that could be mislabelled. - process.env.DB_DIALECT = "sqlite"; - const openWith = async (taglineLocalized: boolean): Promise => { - const adapter = await createAdapter({ - type: "sqlite", - url: `file:${dbPath}`, - } as Parameters[0]); - return createTestNextly({ - adapter, - collections: [postsWithTagline(taglineLocalized)], - localization, - }); - }; - - current = await openWith(false); - const created = await current - .getService("collectionsHandler") - .createEntry( - { collectionName: "i18nwin_posts", overrideAccess: true }, - { title: "Title", tagline: "Shared tagline" } - ); - const id = (created.data as { id: string }).id; - - // Localize `tagline`. The companion already exists and holds the title's rows, - // so this is precisely the case the empty-companion seed skips. - await current.destroy(); - current = await openWith(true); - await current.destroy(); - current = await openWith(true); - - const read = await current - .getService("collectionsHandler") - .getEntry({ - collectionName: "i18nwin_posts", - entryId: id, - overrideAccess: true, - }); - expect((read.data as { tagline?: unknown }).tagline).toBe("Shared tagline"); - }); - it("seeds once and does not duplicate rows on later boots", async () => { // The seed runs on every boot and sync, so it must be gated on the companion // being empty. Without that gate each boot would insert another default-locale diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 741ae8142..0a2901cf5 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -301,6 +301,19 @@ export async function ensureCompanionTable( * correct for an entity that never held content on main. */ defaultLocale?: string; + /** + * Allow ALTERing a companion that already exists — adding a newly localized column, or + * `_status` when Draft/Published is turned on — and backfilling those columns. + * + * Off by default, and that default is load-bearing. Plain app boot calls this + * unconditionally, with none of the auto-sync or production gating the CLI and reload paths + * apply, so reconciling here would let a production deployment run `ALTER TABLE` off a + * metadata change instead of waiting for `nextly migrate` — and a runtime role without DDL + * rights would fail silently, then register a schema describing columns that do not exist. + * Creating a MISSING companion stays unconditional: that is the pre-existing boot contract, + * and it adds a table rather than altering one. + */ + reconcileExisting?: boolean; }, /** * Notified when creation fails. Optional so existing callers are unchanged; @@ -338,6 +351,11 @@ export async function ensureCompanionTable( companionTableName, ]); const companionColumns = physical.get(companionTableName) ?? new Set(); + // Reconciling an EXISTING companion is opt-in (see `reconcileExisting`). When it + // is off, treat the current columns as the desired ones so the reconcile emits + // nothing for a table that is already there — boot then only ever CREATEs. + const reconcileExisting = args.reconcileExisting === true; + if (alreadyExists && !reconcileExisting) return; const oldLocalized = alreadyExists ? localizedFields.filter(f => companionColumns.has(toColumn(f.name))) : []; @@ -355,9 +373,21 @@ export async function ensureCompanionTable( // was built with `hasStatus: true`, so the next per-locale status read or // write hit a missing column. Undefined while the table does not exist yet, // where the CREATE already includes it. - companionHasStatus: alreadyExists - ? companionColumns.has("_status") - : undefined, + // Supplied only when it can ADD. Passing the physical state while `status` + // is off makes the reconcile emit an unconditional `DROP COLUMN _status`, + // which would discard every locale's publication state — and `db:sync` + // persists the new metadata BEFORE its destructive prompt, so that drop + // would happen even when the operator declined the prompt. Additive only, + // matching how localized columns are treated a few lines above: removing + // one is the confirmed transition's job, not an unattended sync's. + companionHasStatus: + alreadyExists && args.status === true + ? companionColumns.has("_status") + : undefined, + // Needed for the `_status` ADD to carry each default-locale row's real status + // across from the main table. Without it the column lands on its `draft` + // default, quietly unpublishing content that was live. + defaultLocale: args.defaultLocale, }); for (const stmt of statements) { await adapter.executeQuery(stmt); diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 936aad08e..e354da690 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -623,6 +623,9 @@ async function ensureLocalizedCompanionsForReload( dialect: adapter.dialect, status: entity.status === true, defaultLocale, + // These two callers already refuse to run in production and are gated on + // auto-sync, so they are the ones allowed to ALTER an existing companion. + reconcileExisting: true, }, error => { console.warn( From 197e2cc6f380daf90cb5d375df17d99e1dec6321 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 13:04:10 +0500 Subject: [PATCH 10/20] fix(nextly): provision companions before the HMR schema apply --- ...rite-without-companion.integration.test.ts | 8 +++++++ .../src/domains/i18n/runtime/companion-io.ts | 16 +++++++++++++- ...ngle-without-companion.integration.test.ts | 8 +++++++ packages/nextly/src/init/reload-config.ts | 21 ++++++++++--------- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index cf9cf6b9f..06cbec7a5 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -40,8 +40,14 @@ import type { CollectionsHandler } from "../../../services/collections-handler"; let dir: string; let dbPath: string; let current: TestNextly | undefined; +// The harness snapshots DB_DIALECT when IT creates an adapter. These tests build +// their own first, so the snapshot captures the already-overwritten "sqlite" and +// never puts the real dialect back — which, in a single-fork run, would make every +// later file resolve environment-backed schema behaviour as SQLite. +let previousDialect: string | undefined; beforeEach(() => { + previousDialect = process.env.DB_DIALECT; dir = mkdtempSync(join(tmpdir(), "nextly-i18n-window-")); dbPath = join(dir, "test.db"); }); @@ -49,6 +55,8 @@ beforeEach(() => { afterEach(async () => { await current?.destroy(); current = undefined; + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; rmSync(dir, { recursive: true, force: true }); }); diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 0a2901cf5..33a269f87 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -411,6 +411,7 @@ export async function ensureCompanionTable( }); } await seedCompanionFromMain(adapter, { + companionJustCreated: !alreadyExists, mainColumns, slug: args.slug, tableName: args.tableName, @@ -476,6 +477,8 @@ async function seedCompanionFromMain( dialect: SupportedDialect; status: boolean; defaultLocale?: string; + /** True when the caller created this companion in the same call. */ + companionJustCreated?: boolean; } ): Promise { if (!args.defaultLocale || args.localizedFields.length === 0) return; @@ -496,7 +499,18 @@ async function seedCompanionFromMain( // The transition test. See the note above: any per-locale content at all means // the main columns can no longer be safely declared to be the default language. - if (!(await companionIsEmpty(adapter, args))) return; + // + // Skipped when THIS call just created the companion, because then the transition + // is known rather than inferred and there is nothing to misread. It also has to + // be skipped: `db:sync` updates the registry before this runs, so a server that + // is already up can write a locale row the moment the CREATE commits, and one + // such row would otherwise cancel the backfill for every other document. The + // `onlyMissing` predicate protects that concurrent row on its own. + if ( + args.companionJustCreated !== true && + !(await companionIsEmpty(adapter, args)) + ) + return; const columnsOnMain = spec.columns .filter(column => args.mainColumns.has(column.name)) diff --git a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts index ecd580729..37c9732a3 100644 --- a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts @@ -28,8 +28,14 @@ import { let dir: string; let dbPath: string; let current: TestNextly | undefined; +// The harness snapshots DB_DIALECT when IT creates an adapter. These tests build +// their own first, so the snapshot captures the already-overwritten "sqlite" and +// never puts the real dialect back — which, in a single-fork run, would make every +// later file resolve environment-backed schema behaviour as SQLite. +let previousDialect: string | undefined; beforeEach(() => { + previousDialect = process.env.DB_DIALECT; dir = mkdtempSync(join(tmpdir(), "nextly-single-window-")); dbPath = join(dir, "test.db"); }); @@ -37,6 +43,8 @@ beforeEach(() => { afterEach(async () => { await current?.destroy(); current = undefined; + if (previousDialect === undefined) delete process.env.DB_DIALECT; + else process.env.DB_DIALECT = previousDialect; rmSync(dir, { recursive: true, force: true }); }); diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index e354da690..d5d324df3 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -1200,6 +1200,17 @@ async function runReload(opts?: { // not surface as a schema diff — so run the idempotent metadata sync before // returning, otherwise a metadata-only edit (e.g. toggling `versions`) would // not persist until the dev server restarts. + // Provision the `_locales` companion of every localized entity BEFORE anything + // touches the schema. Two orderings matter here and both are load-bearing: + // + // - before the apply, because enabling localization asks the pipeline to DROP + // the translatable columns from the main table, and the seed copies out of + // those columns. Running afterwards would find them already gone. + // - before the `!hasChanges` return, because that drop is classified unsafe and + // deferred, which leaves `hasChanges` false — so the exact transition this + // exists to support would return early and never provision anything. + await ensureLocalizedCompanionsForReload(adapter, newConfig); + if (!hasChanges) { // Only sync when the schema is genuinely in step (every entity had a zero-op // diff). If a real schema change was deferred (unsafe/needs review) or a diff @@ -1493,16 +1504,6 @@ async function runReload(opts?: { singles: singleSynced && componentSynced, }); - // Physically provision the `_locales` companion of every localized entity - // before the runtime descriptors below register it. `next dev` routes config - // edits here rather than through the CLI watcher, so without this, turning on - // localization during ordinary HMR registered a companion the database did - // not have: the admin rendered the full localization UI, non-default writes - // were refused, and the main columns could still be dropped before anything - // had copied them across. Seeding is part of the same call, which is why it - // must run before the drop the schema apply may perform. - await ensureLocalizedCompanionsForReload(adapter, newConfig); - // Pre-compute fresh Drizzle table objects for all affected collections, // singles, and components. Synchronous (schema generation, no DB I/O). // Shared between the cache-refresh blocks below so we don't generate twice. From 4e2317e83cac08b2d6a18ee29864485eae7d64fb Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 13:06:44 +0500 Subject: [PATCH 11/20] fix(nextly): refuse localized field-group writes without a companion --- .../services/field-group-mutation-service.ts | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index be12663e2..2361e04c4 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -28,6 +28,7 @@ import type { SanitizedLocalizationConfig } from "../../i18n/config/types"; import { resolveRequestedLocale } from "../../i18n/resolve-locale"; import { buildCompanionSchema, + companionTableExists, splitLocalizedWrite, upsertCompanionRow, } from "../../i18n/runtime/companion-io"; @@ -183,6 +184,27 @@ export class FieldGroupMutationService extends BaseService { ): Promise { if (Object.keys(companionData).length === 0) return; const writeLocale = resolveRequestedLocale(this.localization!, locale); + // Refuse rather than fail opaquely when the companion is not there yet. By this + // point the translatable values have already been split OUT of the main payload, + // so there is nowhere else for them to go: the upsert below would hit a missing + // table and surface a raw database error, and because the component path is not + // transactional the shared fields may already have been committed. A localized + // field group embedded under a NON-localized parent reaches here even when the + // collection and single guards pass, since those only check their own companion. + if ( + !(await companionTableExists(writeAdapter, schema.companionTableName)) + ) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + fieldGroupTable: schema.companionTableName, + locale: writeLocale, + }, + }); + } await upsertCompanionRow( writeAdapter, schema.companionTableName, From addc310f2c5dd7ce2bffd874a162478b10bede9a Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 16:09:49 +0500 Subject: [PATCH 12/20] fix(nextly): re-provision companions after the HMR apply creates tables --- packages/nextly/src/init/reload-config.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index d5d324df3..b18cc554a 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -1390,6 +1390,16 @@ async function runReload(opts?: { }); if (applyResult.success) { + // Provision again, now that the apply has created any brand-new main tables. + // The pre-apply call above cannot help an entity that did not exist yet: its + // companion carries a foreign key to a main table the pipeline had not created, + // so the CREATE failed and was swallowed, while the metadata and runtime schema + // were still published as localized — leaving non-default writes refused until + // another reload. Both calls are needed and both are idempotent: the earlier one + // seeds transitions while the main columns still exist, this one creates + // companions for entities that are new in this reload. + await ensureLocalizedCompanionsForReload(adapter, newConfig); + // Publish each scope's recording policy only AFTER its field-tree metadata // sync succeeds (see the assignment after the syncs below): the DDL applied, // but if a sync then fails, activating the new decision while the mutation From 8ab99d8fbd1d4b73a4edf9a5d0727983bc201789 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 17:17:26 +0500 Subject: [PATCH 13/20] refactor(nextly): split companion seeding out of the write guard --- .changeset/localized-write-companion-guard.md | 8 +- ...nc-localized-companion.integration.test.ts | 42 --- packages/nextly/src/cli/commands/dev-build.ts | 6 - packages/nextly/src/di/register.ts | 23 +- ...rite-without-companion.integration.test.ts | 215 +----------- .../src/domains/i18n/migration/generate-up.ts | 99 ++---- .../src/domains/i18n/runtime/companion-io.ts | 317 +----------------- packages/nextly/src/init/reload-config.ts | 31 +- 8 files changed, 47 insertions(+), 694 deletions(-) diff --git a/.changeset/localized-write-companion-guard.md b/.changeset/localized-write-companion-guard.md index ab1618169..60d9b21a5 100644 --- a/.changeset/localized-write-companion-guard.md +++ b/.changeset/localized-write-companion-guard.md @@ -22,10 +22,8 @@ "@nextlyhq/tsconfig": patch --- -Turning on localization for a collection that already had content could lose that content in two ways. +Saving a translation could overwrite the original language. `nextly db:sync` marks a collection as localized in a separate process from the running app, so the app could show the language switcher before its translations table existed — and a translation saved in that window wrote over the original-language values and changed the entry's URL, while reporting success. -Existing entries could go blank. Once the translations table existed, every localized field was read from it, and nothing had copied the current values across — so titles and body text disappeared from the admin, from lists and from filters, while the values sat untouched in the database. Those values are now copied into the default language when the table is created, so existing content stays exactly as it was. Nothing is deleted: the copy leaves the originals in place. +The translations table is now prepared during `db:sync` and during a dev config reload, for collections, singles and field groups alike. If it is still missing, a write in a non-default language is refused with a clear message instead of overwriting anything, and the same refusal now covers singles and embedded field groups rather than only collections. Writing the default language before the table exists is unchanged. -Saving a translation could overwrite the original language. `nextly db:sync` marks a collection as localized in a separate process from the running app, so the app could show the language switcher before its translations table existed, and saving a translation then wrote over the original-language values and changed the entry's URL while reporting success. `db:sync` and the dev config watcher now prepare that table in the same run, for collections, singles and components alike, and a translation saved before it exists is refused with a clear message instead of overwriting anything. Writing the default language before the table exists is unchanged. - -Collections and singles that set a custom `dbName` are now handled correctly here too; previously their translations table could be created against a table name that does not exist. And a database that is unreachable or refusing connections is no longer reported as a missing translations table. +Collections and singles that set a custom `dbName` are handled correctly here too; previously their translations table could be created against a table name that does not exist. And a database that is unreachable or refusing connections is no longer reported as a missing translations table. diff --git a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts index d6e72a019..fd7e03523 100644 --- a/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts +++ b/packages/nextly/src/cli/commands/__tests__/db-sync-localized-companion.integration.test.ts @@ -180,48 +180,6 @@ describe("db:sync creates localized companion tables in-process (integration)", expect(await tableExists("dbsync_notes_locales")).toBe(false); }); - it("carries a newly localized field's shared value onto an existing companion", async () => { - // A field that was SHARED lives on the main table and applies to every language. - // Marking it localized adds a companion column, and that column starts empty — so - // without a backfill the value stops being returned the moment the field becomes - // translatable, even though it is still on the main table. - // - // Driven through `db:sync` rather than app boot on purpose: reconciling an - // EXISTING companion is opt-in, and boot deliberately does not opt in, because it - // would otherwise run ALTER TABLE in production off a metadata change. - const withTagline = (taglineLocalized: boolean) => - defineConfig({ - localization: { locales: ["en", "es"], defaultLocale: "en" }, - collections: [ - defineCollection({ - slug: "dbsync_notes2", - localized: true, - fields: [ - text({ name: "title", localized: true }), - text({ name: "tagline", localized: taglineLocalized }), - ], - }), - ], - }); - - await runSync(withTagline(false)); - // A row with the shared value on main and a default-locale companion row, which - // is what makes the `onlyMissing` seed skip it. - await adapter!.executeQuery( - `INSERT INTO "dc_dbsync_notes2" ("id", "title", "slug", "tagline") VALUES ('n1', 'T', 't', 'Shared tagline')` - ); - await adapter!.executeQuery( - `INSERT INTO "dc_dbsync_notes2_locales" ("_parent", "_locale", "title") VALUES ('n1', 'en', 'T')` - ); - - await runSync(withTagline(true)); - - const rows = await adapter!.executeQuery<{ tagline: string | null }>( - `SELECT "tagline" FROM "dc_dbsync_notes2_locales" WHERE "_locale" = 'en'` - ); - expect(rows[0]?.tagline).toBe("Shared tagline"); - }); - it("leaves a non-localized collection with no companion", async () => { await runSync( defineConfig({ diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index 7fec0a16e..53bdb5852 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -840,8 +840,6 @@ export async function ensureLocalizedCompanions( ], ]; - const defaultLocale = config.localization?.defaultLocale; - for (const [group, resolveTableName] of groups) { for (const entity of group) { if (!entity.slug || entity.localized !== true) continue; @@ -861,10 +859,6 @@ export async function ensureLocalizedCompanions( fields: entity.fields ?? [], dialect, status: entity.status === true, - defaultLocale, - // These two callers already refuse to run in production and are gated on - // auto-sync, so they are the ones allowed to ALTER an existing companion. - reconcileExisting: true, }, error => { logger.warn( diff --git a/packages/nextly/src/di/register.ts b/packages/nextly/src/di/register.ts index 4408eb045..d73d82a47 100644 --- a/packages/nextly/src/di/register.ts +++ b/packages/nextly/src/di/register.ts @@ -489,14 +489,7 @@ export async function registerServices( container.registerSingleton("adapter", () => adapter); - // `transformedConfig`, not `config`: plugin config transformers run before this - // and may supply or override `localization`, and every other companion call - // site reads the transformed value. Taking the raw config here would create - // unseeded companions on the registry path while the code-first paths seed. - const schemaRegistry = await initializeSchemaRegistry( - adapter, - transformedConfig.localization?.defaultLocale - ); + const schemaRegistry = await initializeSchemaRegistry(adapter); // Publish the webhook recording policy from the config INDEPENDENTLY of the // schema registry. `registerConfigTablesInResolver` (below) only runs when the @@ -979,13 +972,7 @@ async function resolveAdapter( * and `dynamic_components` DB tables and are generated at runtime. */ async function initializeSchemaRegistry( - adapter: DrizzleAdapter, - /** - * The language existing main-table values belong to. Passed down so a companion - * created here for an entity that already has content is seeded from that - * content instead of appearing empty, which would read as null everywhere. - */ - defaultLocale?: string + adapter: DrizzleAdapter ): Promise { try { const { SchemaRegistry } = await import("../database/schema-registry"); @@ -1056,7 +1043,6 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: hasStatus === true, - defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1108,7 +1094,6 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: hasStatus === true, - defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1158,7 +1143,6 @@ async function initializeSchemaRegistry( fields: fields as { name: string; type: string }[], dialect, status: false, - defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1728,7 +1712,6 @@ async function syncCodeFirstCollections( fields, dialect: syncDialect, status: desired.status === true, - defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -1928,7 +1911,6 @@ async function syncCodeFirstComponents( fields: compConfig.fields as { name: string; type: string }[], dialect, status: false, - defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" @@ -2195,7 +2177,6 @@ async function reconcileSingleTablesForBoot( fields: fields, dialect, status: hasStatus, - defaultLocale: transformedConfig.localization?.defaultLocale, }); const { buildCompanionRuntimeTable } = await import( "../domains/i18n/runtime/companion-registration" diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 06cbec7a5..81c12b15f 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -70,10 +70,7 @@ const posts = (localized: boolean) => fields: [text({ name: "title", localized: true })], }); -async function boot( - localized: boolean, - defaultLocale = localization.defaultLocale -): Promise { +async function boot(localized: boolean): Promise { process.env.DB_DIALECT = "sqlite"; const adapter = await createAdapter({ type: "sqlite", @@ -82,7 +79,7 @@ async function boot( return createTestNextly({ adapter, collections: [posts(localized)], - localization: { ...localization, defaultLocale }, + localization, }); } @@ -180,214 +177,6 @@ describe("localized write without a companion table (integration)", () => { }); }); -describe("enabling localization on existing content (integration)", () => { - it("keeps the default language readable once the companion appears", async () => { - // Creating the companion is not enough. Once it exists, a read resolves each - // localized field through it; with no default-locale row it overlays null, so - // the entity's existing content disappears from every read, list and filter - // while the values still sit on the main table. The companion has to be - // SEEDED from main, which is what the boot/db:sync path skipped. - current = await boot(false); - const created = await current - .getService("collectionsHandler") - .createEntry( - { collectionName: "i18nwin_posts", overrideAccess: true }, - { title: "Original" } - ); - const id = (created.data as { id: string }).id; - - // Two re-boots, because the boot that flips `localized` in the registry is - // not the boot that creates the companion: the registry is read before the - // code-first sync writes the flag. That lag is exactly the window this branch - // closes for `db:sync`; here it just means the companion appears on the boot - // after, and the seed has to run then. - await current.destroy(); - current = await boot(true); - await current.destroy(); - current = await boot(true); - - const read = await current - .getService("collectionsHandler") - .getEntry({ - collectionName: "i18nwin_posts", - entryId: id, - overrideAccess: true, - }); - - // Without the seed this is `null`: the companion exists and is empty. - expect((read.data as { title?: unknown }).title).toBe("Original"); - // The values must also still be on main — seeding copies, it does not move. - // Dropping those columns is the destructive half of the transition and stays - // behind the schema pipeline's confirmation. - expect(await physicalTitle(current)).toBe("Original"); - }); - - it("leaves a partly populated companion alone, keeping the main values intact", async () => { - // Once ANY per-locale row exists, the main columns can no longer be declared to - // be the default language: nothing records which language they hold, and the - // rows present may be in another one. Backfilling the remaining entries would - // therefore risk labelling their content as a language it is not. - // - // The cost is that those entries keep reading null until an operator acts, and - // that is the right side to err on — unreadable content is still intact on the - // main table and recoverable, whereas a mislabelled translation is silently - // wrong. This test pins both halves: no backfill, and no data loss. - current = await boot(false); - const handler = () => - current!.getService("collectionsHandler"); - const ids: string[] = []; - for (const title of ["First", "Second", "Third"]) { - const created = await handler().createEntry( - { collectionName: "i18nwin_posts", overrideAccess: true }, - { title } - ); - ids.push((created.data as { id: string }).id); - } - - await current.destroy(); - current = await boot(true); - await current.destroy(); - current = await boot(true); - - // Put the companion into the partial state: keep one row, drop the others, so - // the table is non-empty but two entries have no default-locale row. - await current.adapter.executeQuery( - `DELETE FROM dc_i18nwin_posts_locales WHERE _parent <> '${ids[0]}'` - ); - - await current.destroy(); - current = await boot(true); - - // The entry whose row survived still reads; the other two do not get one. - const kept = await handler().getEntry({ - collectionName: "i18nwin_posts", - entryId: ids[0], - overrideAccess: true, - }); - expect((kept.data as { title?: unknown }).title).toBe("First"); - - const rows = await current.adapter.executeQuery<{ n: number }>( - "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales" - ); - expect(Number(rows[0]?.n)).toBe(1); - - // Nothing was lost: the values the other two entries had are still on main, - // so an operator can complete the transition without recovering from backups. - const physical = await current.adapter.executeQuery<{ title: string }>( - "SELECT title FROM dc_i18nwin_posts ORDER BY title" - ); - expect(physical.map(r => r.title)).toEqual(["First", "Second", "Third"]); - }); - - it("does not fabricate rows from stale columns after the default language changes", async () => { - // The main columns are only the default language DURING the transition. Once - // real translations exist, writes go to the companion and those columns stop - // being updated — so re-pointing `defaultLocale` at another language must not - // seed it from them, or the new default serves stale text from the old one and - // suppresses the fallback that would have shown the real value. - current = await boot(false); - const handler = () => - current!.getService("collectionsHandler"); - const ids: string[] = []; - for (const title of ["Translated later", "Never translated"]) { - const created = await handler().createEntry( - { collectionName: "i18nwin_posts", overrideAccess: true }, - { title } - ); - ids.push((created.data as { id: string }).id); - } - - await current.destroy(); - current = await boot(true); - await current.destroy(); - current = await boot(true); - - // One real translation, which is what marks the transition as finished. The - // OTHER entry is the exposed one: it has no Spanish row, so a seed keyed on - // the new default would invent one from its stale English column. - const translated = await handler().updateEntry( - { - collectionName: "i18nwin_posts", - entryId: ids[0], - overrideAccess: true, - locale: "es", - }, - { title: "Texto en español" } - ); - expect(translated.success).toBe(true); - - // Now Spanish becomes the default. Main still holds the English text. - await current.destroy(); - current = await boot(true, "es"); - - const untranslated = await handler().getEntry({ - collectionName: "i18nwin_posts", - entryId: ids[1], - overrideAccess: true, - locale: "es", - }); - // Without the guard this reads "Never translated" — English served as Spanish. - expect((untranslated.data as { title?: unknown }).title).not.toBe( - "Never translated" - ); - - // Only the two English rows and the one real Spanish translation exist; no - // Spanish row was fabricated for the untranslated entry. - const rows = await current.adapter.executeQuery<{ n: number }>( - "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales WHERE _locale = 'es'" - ); - expect(Number(rows[0]?.n)).toBe(1); - }); - - it("seeds once and does not duplicate rows on later boots", async () => { - // The seed runs on every boot and sync, so it must be gated on the companion - // being empty. Without that gate each boot would insert another default-locale - // row and the composite primary key would start rejecting writes. - current = await boot(false); - const created = await current - .getService("collectionsHandler") - .createEntry( - { collectionName: "i18nwin_posts", overrideAccess: true }, - { title: "Original" } - ); - const id = (created.data as { id: string }).id; - - await current.destroy(); - current = await boot(true); - - // Translate, so the companion is no longer empty, then re-boot. - await current - .getService("collectionsHandler") - .updateEntry( - { - collectionName: "i18nwin_posts", - entryId: id, - overrideAccess: true, - locale: "en", - }, - { title: "Edited in the companion" } - ); - await current.destroy(); - current = await boot(true); - - const read = await current - .getService("collectionsHandler") - .getEntry({ - collectionName: "i18nwin_posts", - entryId: id, - overrideAccess: true, - }); - expect((read.data as { title?: unknown }).title).toBe( - "Edited in the companion" - ); - - const rows = await current.adapter.executeQuery<{ n: number }>( - "SELECT COUNT(*) AS n FROM dc_i18nwin_posts_locales" - ); - expect(Number(rows[0]?.n)).toBe(1); - }); -}); - /** * The guard has to hold on every dialect, and the SQLite cases above cannot show * that. What decides whether it fires is `companionTableExists`, which probes diff --git a/packages/nextly/src/domains/i18n/migration/generate-up.ts b/packages/nextly/src/domains/i18n/migration/generate-up.ts index 31434f2df..97be1cef8 100644 --- a/packages/nextly/src/domains/i18n/migration/generate-up.ts +++ b/packages/nextly/src/domains/i18n/migration/generate-up.ts @@ -68,87 +68,44 @@ export function buildLocalizationUpSql(spec: CompanionMigrationSpec): string { export function buildLocalizationUpStatements( spec: CompanionMigrationSpec ): string[] { - const { dialect, mainTable } = spec; + const { dialect, mainTable, companionTable, defaultLocale, columns } = spec; const create = buildCompanionCreateStatement(spec); - const seedStatement = buildCompanionSeedStatement(spec); - const seed = seedStatement ? [seedStatement] : []; - - const drops = columnsStillOnMain(spec).map( - c => - `ALTER TABLE ${q(mainTable, dialect)} DROP COLUMN ${q(c.name, dialect)}` - ); - - return [create, ...seed, ...drops]; -} - -/** - * The subset of `spec.columns` that physically exists on the main table, and so can be seeded - * from or dropped. A field added and localized in the same save is in `columns` (the companion - * needs it) but not here — there is nothing on main to copy or remove. Undefined `columnsOnMain` - * means "all", which is the file-migration path where every localized column pre-exists. - */ -function columnsStillOnMain( - spec: CompanionMigrationSpec -): CompanionMigrationSpec["columns"] { + // Only columns already on the main table can be seeded from or dropped. A field added and + // localized in the same save is in `columns` (so the companion gets it) but not on main, so + // it is excluded from the SELECT and the DROP. Undefined `columnsOnMain` means "all" — the + // file-migration path, where every localized column pre-exists on main. const onMainSet = spec.columnsOnMain && new Set(spec.columnsOnMain); - return onMainSet - ? spec.columns.filter(c => onMainSet.has(c.name)) - : spec.columns; -} - -/** - * The `INSERT ... SELECT` that copies the main table's existing values into the companion as - * default-locale rows, or null when there is nothing to copy. - * - * Split out from {@link buildLocalizationUpStatements} because seeding and dropping are not - * always wanted together. Creating the companion without this INSERT is what made existing - * content vanish: reads resolve a localized field through the companion once the table is - * there, find no row for the default locale, and overlay null — the values are still on the - * main table, but nothing returns them. - * - * Dropping the main columns is a separate, destructive step that the schema pipeline gates - * behind an explicit confirmation, so a caller that only needs the content to stay visible - * (dev boot, `db:sync`) takes this statement alone and leaves the columns in place. - */ -export function buildCompanionSeedStatement( - spec: CompanionMigrationSpec, - options?: { - /** - * Skip main rows that already have a default-locale companion row, making the - * statement safe to re-run and safe on a PARTIALLY seeded companion. Gating the - * whole seed on an empty companion instead would strand every other row the - * moment one row got a companion entry — one edited entry, and the rest keep - * reading null forever. - */ - onlyMissing?: boolean; - } -): string | null { - const { dialect, mainTable, companionTable, defaultLocale } = spec; - const onMain = columnsStillOnMain(spec); - // An INSERT with an empty value list is invalid SQL, and with no pre-existing translatable - // columns and no status there is no content to preserve either. - if (onMain.length === 0 && !spec.status) return null; - + const onMain = onMainSet + ? columns.filter(c => onMainSet.has(c.name)) + : columns; // A leading ", " per column, so an empty set contributes nothing to the column lists. const onMainCols = onMain.map(c => `, ${q(c.name, dialect)}`).join(""); - // When the entity has Draft/Published, the seeded default-locale rows carry the existing main - // row's `status` into the companion `_status` so enabling localization does not silently + + // When the collection has Draft/Published, the seeded default-locale rows carry the existing + // main row's `status` into the companion `_status` so enabling localization doesn't silently // un-publish live content. const statusInsertCol = spec.status ? `, ${q("_status", dialect)}` : ""; const statusSelectCol = spec.status ? `, ${q("status", dialect)}` : ""; - const where = options?.onlyMissing - ? ` WHERE NOT EXISTS (SELECT 1 FROM ${q(companionTable, dialect)} ` + - `WHERE ${q(companionTable, dialect)}.${q("_parent", dialect)} = ${q(mainTable, dialect)}.${q("id", dialect)} ` + - `AND ${q(companionTable, dialect)}.${q("_locale", dialect)} = ${lit(defaultLocale)})` - : ""; + // Skip the seed entirely when there is nothing on main to copy — no pre-existing translatable + // columns and no status. An INSERT with an empty value list would be invalid SQL, and there + // is no existing content to preserve. + const seed = + onMain.length > 0 || spec.status + ? [ + `INSERT INTO ${q(companionTable, dialect)} ` + + `(${q("_parent", dialect)}, ${q("_locale", dialect)}${statusInsertCol}${onMainCols}) ` + + `SELECT ${q("id", dialect)}, ${lit(defaultLocale)}${statusSelectCol}${onMainCols} ` + + `FROM ${q(mainTable, dialect)}`, + ] + : []; - return ( - `INSERT INTO ${q(companionTable, dialect)} ` + - `(${q("_parent", dialect)}, ${q("_locale", dialect)}${statusInsertCol}${onMainCols}) ` + - `SELECT ${q("id", dialect)}, ${lit(defaultLocale)}${statusSelectCol}${onMainCols} ` + - `FROM ${q(mainTable, dialect)}${where}` + const drops = onMain.map( + c => + `ALTER TABLE ${q(mainTable, dialect)} DROP COLUMN ${q(c.name, dialect)}` ); + + return [create, ...seed, ...drops]; } diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 33a269f87..914e5bed8 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -126,42 +126,6 @@ interface CompanionWriteAdapter { executeQuery(sql: string, params?: unknown[]): Promise; } -/** - * What provisioning additionally needs: the Drizzle handle, so the physical shape of a table can - * be read through the shared introspection helper instead of being guessed at with probe queries. - * Kept separate from {@link CompanionWriteAdapter} so the read/write helpers, which never - * introspect, keep their narrower contract. - */ -interface CompanionProvisionAdapter extends CompanionWriteAdapter { - getDrizzle(): T; -} - -/** - * The physical columns of `tableNames`, keyed by table, via the same introspection the schema - * pipeline uses. One round trip answers for every table and column at once, and — unlike a - * `SELECT ... LIMIT 0` probe — a failure is a real failure rather than being indistinguishable - * from "that column is not there". - */ -async function readPhysicalColumns( - adapter: CompanionProvisionAdapter, - tableNames: string[] -): Promise>> { - const { introspectLiveSnapshot } = await import( - "../../schema/pipeline/diff/introspect-live" - ); - const snapshot = await introspectLiveSnapshot( - adapter.getDrizzle(), - adapter.dialect, - tableNames - ); - return new Map( - snapshot.tables.map(table => [ - table.name, - new Set(table.columns.map(column => column.name)), - ]) - ); -} - /** * Upsert the companion `_locales` row for `(parentId, locale)` with the provided localized * columns. Only the supplied columns are written; other locales' rows and other columns on this @@ -288,32 +252,13 @@ export async function companionHasStatusColumn( * retries on the next boot. */ export async function ensureCompanionTable( - adapter: CompanionProvisionAdapter, + adapter: CompanionWriteAdapter, args: { slug: string; tableName: string; fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; - /** - * The language existing main-table values belong to. Supplying it enables the seed below; - * without it the companion is created empty, which is the pre-existing behaviour and is only - * correct for an entity that never held content on main. - */ - defaultLocale?: string; - /** - * Allow ALTERing a companion that already exists — adding a newly localized column, or - * `_status` when Draft/Published is turned on — and backfilling those columns. - * - * Off by default, and that default is load-bearing. Plain app boot calls this - * unconditionally, with none of the auto-sync or production gating the CLI and reload paths - * apply, so reconciling here would let a production deployment run `ALTER TABLE` off a - * metadata change instead of waiting for `nextly migrate` — and a runtime role without DDL - * rights would fail silently, then register a schema describing columns that do not exist. - * Creating a MISSING companion stays unconditional: that is the pre-existing boot contract, - * and it adds a table rather than altering one. - */ - reconcileExisting?: boolean; }, /** * Notified when creation fails. Optional so existing callers are unchanged; @@ -323,10 +268,7 @@ export async function ensureCompanionTable( ): Promise { const companionTableName = `${args.tableName}_locales`; try { - const alreadyExists = await companionTableExists( - adapter, - companionTableName - ); + if (await companionTableExists(adapter, companionTableName)) return; // Lazy import avoids a cycle (reconcile-companion → migration helpers). const { buildCompanionReconcileStatements } = await import( "../migration/reconcile-companion" @@ -334,93 +276,18 @@ export async function ensureCompanionTable( const localizedNames = new Set( resolveLocalizedFieldNames(args.fields, true) ); - const localizedFields = args.fields.filter(f => localizedNames.has(f.name)); - // An EXISTING companion still has to be reconciled. The schema push pipeline - // deliberately skips companion tables, so marking another field localized (or - // turning on Draft/Published) adds a column the companion never gets: reads and - // writes then address a shape the physical table does not have, and the seed - // below would reference a column that is not there. Passing the columns the - // companion actually has as `oldLocalized` makes this an ADD of exactly what is - // missing. Dropping columns is left out on purpose — a column that disappeared - // from the config still holds content, and removing it is the pipeline's gated, - // confirmable job rather than something an unattended boot should decide. - // One introspection answers both questions below: which localized columns the - // companion already has, and which are still on the main table for the seed. - const physical = await readPhysicalColumns(adapter, [ - args.tableName, - companionTableName, - ]); - const companionColumns = physical.get(companionTableName) ?? new Set(); - // Reconciling an EXISTING companion is opt-in (see `reconcileExisting`). When it - // is off, treat the current columns as the desired ones so the reconcile emits - // nothing for a table that is already there — boot then only ever CREATEs. - const reconcileExisting = args.reconcileExisting === true; - if (alreadyExists && !reconcileExisting) return; - const oldLocalized = alreadyExists - ? localizedFields.filter(f => companionColumns.has(toColumn(f.name))) - : []; const statements = buildCompanionReconcileStatements({ slug: args.slug, tableName: args.tableName, - oldLocalized, - newLocalized: localizedFields, + oldLocalized: [], + newLocalized: args.fields.filter(f => localizedNames.has(f.name)), dialect: args.dialect, status: args.status === true, - companionExists: alreadyExists, - // The PHYSICAL `_status` state, which is what decides whether turning - // Draft/Published on for an already-localized entity needs the column added. - // Omitting it left the companion without `_status` while the runtime schema - // was built with `hasStatus: true`, so the next per-locale status read or - // write hit a missing column. Undefined while the table does not exist yet, - // where the CREATE already includes it. - // Supplied only when it can ADD. Passing the physical state while `status` - // is off makes the reconcile emit an unconditional `DROP COLUMN _status`, - // which would discard every locale's publication state — and `db:sync` - // persists the new metadata BEFORE its destructive prompt, so that drop - // would happen even when the operator declined the prompt. Additive only, - // matching how localized columns are treated a few lines above: removing - // one is the confirmed transition's job, not an unattended sync's. - companionHasStatus: - alreadyExists && args.status === true - ? companionColumns.has("_status") - : undefined, - // Needed for the `_status` ADD to carry each default-locale row's real status - // across from the main table. Without it the column lands on its `draft` - // default, quietly unpublishing content that was live. - defaultLocale: args.defaultLocale, + companionExists: false, }); for (const stmt of statements) { await adapter.executeQuery(stmt); } - const mainColumns = physical.get(args.tableName) ?? new Set(); - // Columns the reconcile just ADDED to an existing companion. They arrive empty, - // so a field that was SHARED until now — its value on the main table, applying - // to every language — would start reading as null the moment it becomes - // localized. Copy it across before anything reads through the new column. - if (alreadyExists) { - const added = localizedFields - .map(f => toColumn(f.name)) - .filter( - column => !companionColumns.has(column) && mainColumns.has(column) - ); - await backfillAddedCompanionColumns(adapter, { - companionTableName, - tableName: args.tableName, - columns: added, - defaultLocale: args.defaultLocale, - }); - } - await seedCompanionFromMain(adapter, { - companionJustCreated: !alreadyExists, - mainColumns, - slug: args.slug, - tableName: args.tableName, - companionTableName, - localizedFields, - dialect: args.dialect, - status: args.status === true, - defaultLocale: args.defaultLocale, - }); } catch (error) { // Best-effort: the main table may not exist yet on a very first boot, where the // companion is created on the next boot (or by `nextly migrate`). That case is @@ -433,177 +300,3 @@ export async function ensureCompanionTable( onError?.(error); } } - -/** - * Copy the main table's existing values into the companion as default-locale rows. - * - * Creating the companion is not enough on its own. Once the table exists, a read resolves each - * localized field through it, finds no row for the default locale, and overlays null — so an - * entity that already had content shows empty fields everywhere while the values sit untouched on - * the main table. Enabling localization on existing content therefore made that content - * invisible, not merely unwritable. - * - * Deliberately narrow, because this runs unattended on every boot and sync: - * - * - only while the companion is COMPLETELY EMPTY. Nothing weaker is sound. The main columns - * carry no record of which language they hold, so the only safe moment to declare them the - * default is when no per-locale content exists at all and there is therefore nothing that - * could be mislabelled. Narrowing this to "no row in a NON-default language" looks equivalent - * and is not: a companion holding only partial French rows, read after `defaultLocale` moves - * from `en` to `fr`, contains no non-French row, so stale English main columns would be - * seeded as French. Inferring the transition from the CURRENT default is what makes that - * possible, so this does not infer it at all. - * The cost is that a companion which already holds one row is never backfilled, and any entry - * still missing its default-language row stays unreadable until the operator acts. That is the - * right side to err on: unreadable content is intact on the main table and recoverable, while - * a fabricated translation is silently wrong and nearly undetectable. - * - even then, only for main rows with no row in the default language, so a retry or a - * concurrent write cannot produce a duplicate. - * - only for localized columns that are STILL on the main table, read from the introspected - * shape. After the columns are dropped there is nothing left to copy. - * - it does NOT drop those columns afterwards. That is the destructive half of the transition - * and the schema pipeline gates it behind an explicit confirmation; making the content visible - * again does not require it, and doing it here would route around that gate. - */ -async function seedCompanionFromMain( - adapter: CompanionProvisionAdapter, - args: { - /** Physical columns the main table currently has, from the caller's introspection. */ - mainColumns: Set; - slug: string; - tableName: string; - companionTableName: string; - localizedFields: CompanionFieldLike[]; - dialect: SupportedDialect; - status: boolean; - defaultLocale?: string; - /** True when the caller created this companion in the same call. */ - companionJustCreated?: boolean; - } -): Promise { - if (!args.defaultLocale || args.localizedFields.length === 0) return; - - const { deriveCompanionSpec } = await import( - "../migration/derive-companion-spec" - ); - const spec = deriveCompanionSpec({ - slug: args.slug, - dbName: args.tableName, - fields: args.localizedFields, - dialect: args.dialect, - defaultLocale: args.defaultLocale, - collectionLocalized: true, - status: args.status, - }); - if (!spec) return; - - // The transition test. See the note above: any per-locale content at all means - // the main columns can no longer be safely declared to be the default language. - // - // Skipped when THIS call just created the companion, because then the transition - // is known rather than inferred and there is nothing to misread. It also has to - // be skipped: `db:sync` updates the registry before this runs, so a server that - // is already up can write a locale row the moment the CREATE commits, and one - // such row would otherwise cancel the backfill for every other document. The - // `onlyMissing` predicate protects that concurrent row on its own. - if ( - args.companionJustCreated !== true && - !(await companionIsEmpty(adapter, args)) - ) - return; - - const columnsOnMain = spec.columns - .filter(column => args.mainColumns.has(column.name)) - .map(column => column.name); - if (columnsOnMain.length === 0) return; - - const { buildCompanionSeedStatement } = await import( - "../migration/generate-up" - ); - const seed = buildCompanionSeedStatement( - { ...spec, columnsOnMain }, - { onlyMissing: true } - ); - if (seed) await adapter.executeQuery(seed); -} - -/** - * Copy the main table's value into companion columns the reconcile has just added, for the - * DEFAULT language's rows only. - * - * This is narrower than the general seed and safe for the reason the general seed is not. The - * column is brand new on the companion, so no per-locale content exists in it that could be - * mislabelled, and the value being copied was SHARED until this save — language-neutral by - * definition, not a translation of anything. Whether the companion holds other rows is therefore - * irrelevant here, which is why this runs even in the state the seed refuses. - * - * Default language only: other languages fall back to it, so writing the same value into each - * would duplicate content rather than preserve it. - * - * Correlated scalar subquery because one statement then covers every dialect — verified on - * SQLite, Postgres and MySQL, where a join form would have needed three spellings. - */ -async function backfillAddedCompanionColumns( - adapter: CompanionWriteAdapter, - args: { - companionTableName: string; - tableName: string; - columns: string[]; - defaultLocale?: string; - } -): Promise { - if (!args.defaultLocale || args.columns.length === 0) return; - const isMysql = adapter.dialect === "mysql"; - const q = (id: string) => (isMysql ? `\`${id}\`` : `"${id}"`); - const companion = q(args.companionTableName); - const main = q(args.tableName); - const placeholder = adapter.dialect === "postgresql" ? "$1" : "?"; - - for (const column of args.columns) { - await adapter.executeQuery( - `UPDATE ${companion} SET ${q(column)} = ` + - `(SELECT ${q(column)} FROM ${main} WHERE ${main}.${q("id")} = ${companion}.${q("_parent")}) ` + - `WHERE ${companion}.${q("_locale")} = ${placeholder}`, - [args.defaultLocale] - ); - } -} - -/** The slice of a Drizzle handle this needs: one bounded read of one table. */ -interface CompanionRowReader { - select(): { - from(table: unknown): { limit(n: number): PromiseLike }; - }; -} - -/** - * Whether the companion holds no rows at all — the only state in which the main columns can be - * declared to be the default language without inferring anything (see the note on the seed). - * - * Goes through Drizzle rather than assembling SQL: the companion table object is built from the - * same descriptor the runtime registers, so quoting and dialect differences are the ORM's problem - * rather than being re-implemented here. - */ -async function companionIsEmpty( - adapter: CompanionProvisionAdapter, - args: { - slug: string; - tableName: string; - localizedFields: CompanionFieldLike[]; - dialect: SupportedDialect; - status: boolean; - } -): Promise { - const companion = buildCompanionRuntimeTable({ - slug: args.slug, - tableName: args.tableName, - fields: args.localizedFields, - dialect: args.dialect, - localized: true, - status: args.status, - }); - if (!companion) return false; - const db = adapter.getDrizzle(); - const rows = await db.select().from(companion.table).limit(1); - return rows.length === 0; -} diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index b18cc554a..af99c3e32 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -610,7 +610,6 @@ async function ensureLocalizedCompanionsForReload( ], ]; - const defaultLocale = config.localization?.defaultLocale; for (const [entities, resolveTableName] of groups) { for (const entity of entities) { if (!entity.slug || entity.localized !== true) continue; @@ -622,10 +621,6 @@ async function ensureLocalizedCompanionsForReload( fields: entity.fields ?? [], dialect: adapter.dialect, status: entity.status === true, - defaultLocale, - // These two callers already refuse to run in production and are gated on - // auto-sync, so they are the ones allowed to ALTER an existing companion. - reconcileExisting: true, }, error => { console.warn( @@ -1200,17 +1195,6 @@ async function runReload(opts?: { // not surface as a schema diff — so run the idempotent metadata sync before // returning, otherwise a metadata-only edit (e.g. toggling `versions`) would // not persist until the dev server restarts. - // Provision the `_locales` companion of every localized entity BEFORE anything - // touches the schema. Two orderings matter here and both are load-bearing: - // - // - before the apply, because enabling localization asks the pipeline to DROP - // the translatable columns from the main table, and the seed copies out of - // those columns. Running afterwards would find them already gone. - // - before the `!hasChanges` return, because that drop is classified unsafe and - // deferred, which leaves `hasChanges` false — so the exact transition this - // exists to support would return early and never provision anything. - await ensureLocalizedCompanionsForReload(adapter, newConfig); - if (!hasChanges) { // Only sync when the schema is genuinely in step (every entity had a zero-op // diff). If a real schema change was deferred (unsafe/needs review) or a diff @@ -1390,14 +1374,13 @@ async function runReload(opts?: { }); if (applyResult.success) { - // Provision again, now that the apply has created any brand-new main tables. - // The pre-apply call above cannot help an entity that did not exist yet: its - // companion carries a foreign key to a main table the pipeline had not created, - // so the CREATE failed and was swallowed, while the metadata and runtime schema - // were still published as localized — leaving non-default writes refused until - // another reload. Both calls are needed and both are idempotent: the earlier one - // seeds transitions while the main columns still exist, this one creates - // companions for entities that are new in this reload. + // Create the `_locales` companion of every localized entity, now that the apply + // has produced their main tables. `next dev` routes config edits here rather + // than through the CLI watcher, so without this an entity turned localized under + // ordinary HMR had its companion registered in the runtime registry while the + // database had no such table — non-default writes were then refused until a + // restart. Runs after the apply because the companion carries a foreign key to + // its main table, which a brand-new entity does not have before it. await ensureLocalizedCompanionsForReload(adapter, newConfig); // Publish each scope's recording policy only AFTER its field-tree metadata From 25a1f5d76739b583c70197d4d853f65a06c0ae95 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 18:32:38 +0500 Subject: [PATCH 14/20] fix(nextly): probe the companion before splitting a localized write --- packages/nextly/src/cli/commands/dev-build.ts | 25 +++++- .../services/field-group-mutation-service.ts | 90 +++++++++++-------- .../src/domains/i18n/runtime/companion-io.ts | 8 +- .../services/single-mutation-service.ts | 48 ++++++++++ packages/nextly/src/init/reload-config.ts | 7 ++ 5 files changed, 136 insertions(+), 42 deletions(-) diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index 53bdb5852..2d96da8c7 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -16,7 +16,11 @@ import { teardownEntityComponentData } from "../../domains/field-groups/services import { teardownEntityI18n } from "../../domains/i18n/migration/teardown-entity-i18n"; // Resolve the versioning config so `db:sync` persists it (parity with boot/HMR). import { resolveVersionsConfig } from "../../domains/versions/resolve-config"; -import { describeError, immediateMessage } from "../../errors/index"; +import { + describeError, + immediateMessage, + NextlyError, +} from "../../errors/index"; import { STORAGE_FORMAT } from "../../schemas/storage-format"; import { CollectionSyncService } from "../../services/collections/collection-sync-service"; import type { CollectionSyncResultWithValidation } from "../../services/collections/collection-sync-service"; @@ -840,6 +844,7 @@ export async function ensureLocalizedCompanions( ], ]; + const failures: string[] = []; for (const [group, resolveTableName] of groups) { for (const entity of group) { if (!entity.slug || entity.localized !== true) continue; @@ -861,13 +866,29 @@ export async function ensureLocalizedCompanions( status: entity.status === true, }, error => { - logger.warn( + logger.error( `Could not prepare the translations table for "${entity.slug}" (${tableName}_locales). ` + `Writes in a non-default locale will be refused until it exists: ` + `${error instanceof Error ? error.message : String(error)}` ); + failures.push(entity.slug!); } ); } } + // Fail the command. Exiting 0 here would tell deployment automation the schema is + // in step while the registry advertises localization the database cannot store, so + // every translation write is refused — unlike a failure from the main auto-sync + // pipeline, which does stop the run. Boot and the HMR reload stay best-effort by + // passing no reporter: they must not refuse to start over a companion. + if (failures.length > 0) { + throw NextlyError.conflict({ + reason: "state", + message: `Could not prepare the translations table for: ${failures.join(", ")}. Writes in a non-default locale will be refused until it exists.`, + logContext: { + cause: "companion-provisioning-failed", + entities: failures, + }, + }); + } } diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index 2361e04c4..7904f92ac 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -100,14 +100,19 @@ export class FieldGroupMutationService extends BaseService { * The caller writes `main` to the instance row and, after it has the instance id, upserts * the companion via {@link upsertLocalizedComponent}. */ - private splitLocalizedComponent( + private async splitLocalizedComponent( meta: DynamicFieldGroupRecord, - data: Record - ): { + data: Record, + locale: string | undefined, + writeAdapter: { + dialect: SupportedDialect; + executeQuery(sql: string, params?: unknown[]): Promise; + } = this.adapter + ): Promise<{ schema: ReturnType; main: Record; companion: Record; - } { + }> { if (!this.localization || meta.localized !== true) { return { schema: null, main: data, companion: {} }; } @@ -119,6 +124,32 @@ export class FieldGroupMutationService extends BaseService { status: false, }); if (!schema) return { schema: null, main: data, companion: {} }; + // Probe BEFORE splitting. Splitting first and then discovering the companion is + // absent strands the translatable values: they leave the main payload and the + // upsert that would have taken them is skipped, so the write reports success + // having saved nothing. Checking here also means the refusal is raised before + // the caller opens its transaction, where the adapters' `classifyError` would + // turn a `NextlyError` into an opaque database error and lose the 409. + if ( + !(await companionTableExists(writeAdapter, schema.companionTableName)) + ) { + const writeLocale = resolveRequestedLocale(this.localization, locale); + if (writeLocale !== this.localization.defaultLocale) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + fieldGroupTable: schema.companionTableName, + locale: writeLocale, + }, + }); + } + // Default language: keep the values on the existing main-table column, the + // same pre-companion fallback collections and singles preserve. + return { schema: null, main: data, companion: {} }; + } const { main, companion } = splitLocalizedWrite( data, schema.localizedFields @@ -184,27 +215,6 @@ export class FieldGroupMutationService extends BaseService { ): Promise { if (Object.keys(companionData).length === 0) return; const writeLocale = resolveRequestedLocale(this.localization!, locale); - // Refuse rather than fail opaquely when the companion is not there yet. By this - // point the translatable values have already been split OUT of the main payload, - // so there is nowhere else for them to go: the upsert below would hit a missing - // table and surface a raw database error, and because the component path is not - // transactional the shared fields may already have been committed. A localized - // field group embedded under a NON-localized parent reaches here even when the - // collection and single guards pass, since those only check their own companion. - if ( - !(await companionTableExists(writeAdapter, schema.companionTableName)) - ) { - throw NextlyError.conflict({ - reason: "state", - message: - "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", - logContext: { - cause: "localized-write-without-companion", - fieldGroupTable: schema.companionTableName, - locale: writeLocale, - }, - }); - } await upsertCompanionRow( writeAdapter, schema.companionTableName, @@ -414,9 +424,10 @@ export class FieldGroupMutationService extends BaseService { // i18n: split translatable values out of the main comp_ write — they live on the // companion. `main === data` when the component isn't localized (unchanged path). - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - data + data, + locale ); let instanceId: string; @@ -516,9 +527,10 @@ export class FieldGroupMutationService extends BaseService { ); // i18n: split translatable values out of the main comp_ write (companion-owned). - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - data + data, + locale ); let instanceId: string; @@ -607,9 +619,10 @@ export class FieldGroupMutationService extends BaseService { // i18n: split translatable values out per instance (companion-owned). The // diff-by-id update keeps the instance id stable, so companion rows for OTHER // locales survive a re-save in one locale. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - instance + instance, + locale ); await this.prepareInstanceForWrite( @@ -721,9 +734,10 @@ export class FieldGroupMutationService extends BaseService { const instance = instances[i]; const instanceId = instance.id; // i18n: split translatable values out (companion-owned) per instance. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, - instance + instance, + locale ); await this.prepareInstanceForWrite( @@ -888,9 +902,10 @@ export class FieldGroupMutationService extends BaseService { const componentFields = meta.fields; const instanceId = instance.id; // i18n: split translatable values out per instance using its own component meta. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( meta, - instance + instance, + locale ); await this.prepareInstanceForWrite( @@ -1042,9 +1057,10 @@ export class FieldGroupMutationService extends BaseService { const componentFields = meta.fields; const instanceId = instance.id; // i18n: split translatable values out per instance using its own component meta. - const { schema, main, companion } = this.splitLocalizedComponent( + const { schema, main, companion } = await this.splitLocalizedComponent( meta, - instance + instance, + locale ); await this.prepareInstanceForWrite( diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 914e5bed8..0859a63ac 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -243,9 +243,11 @@ export async function companionHasStatusColumn( /** * Boot/db:sync helper: physically create the companion `_locales` table if it does - * not already exist, and seed it from the main table when localization is being turned on for an - * entity that already holds content. Idempotent and safe to run on every boot — a no-op once the - * table exists and is seeded (or when the entity has no translatable fields). This is the + * not already exist. CREATION ONLY — the table is left empty, so an entity that already holds + * content on its main table will read null for every localized field until that content is + * copied across; a successful return is not evidence that it was. Idempotent and safe to run on + * every boot — a no-op once the table exists (or when the entity has no translatable fields). + * This is the * db:sync/dev-boot counterpart to the migration-owned companion creation (`nextly migrate`), so a * code-first localized collection / single / component gets a working companion without a manual * migrate step. Best-effort: a failure (e.g. main table not yet created) is swallowed so it diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 97cb92133..abd6c63fb 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -229,6 +229,30 @@ export class SingleMutationService extends BaseService { // Webhook helpers // ============================================================ + /** + * Whether the main table still carries a localized column — the only state in which + * the pre-companion fallback can actually persist anything. Probed rather than read + * from the runtime schema, because that schema already omits the column for a Single + * localized from creation, which is exactly the case being distinguished. + */ + private async mainHasLocalizedColumn( + tableName: string, + columnName: string | undefined + ): Promise { + if (!columnName) return false; + const isMysql = this.adapter.dialect === "mysql"; + const table = isMysql ? `\`${tableName}\`` : `"${tableName}"`; + const column = isMysql ? `\`${columnName}\`` : `"${columnName}"`; + try { + await this.adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); + return true; + } catch { + // A missing column is the answer this asks for. Any other failure surfaces on + // the write that follows, on the same connection. + return false; + } + } + /** * Read a Single's FULL companion translation state for one locale, keyed by * companion column — every localized field carries its stored value or is @@ -1090,6 +1114,30 @@ export class SingleMutationService extends BaseService { // belong on the main table, which is the same pre-migration fallback // collections use — and by then the write locale is guaranteed to be // the default one, because the guard above refuses any other. + // The fallback is only real when the main table STILL has those columns. + // A Single localized from creation (or one whose migration already ran) + // keeps them only on the companion, and the registered runtime table omits + // them — so putting the values back into `mainPayload` writes keys the ORM + // does not recognise: they are ignored, `updated_at` is still set, and the + // write reports success having saved nothing. + if (companion && !companionPhysicallyExists) { + const onMain = await this.mainHasLocalizedColumn( + singleMeta.tableName, + companion.localizedFields[0]?.column + ); + if (!onMain) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + single: singleMeta.slug, + companionTable: companion.companionTableName, + }, + }); + } + } const { main: mainPayload, companion: companionData } = companion && companionPhysicallyExists ? splitLocalizedWrite(updatePayload, companion.localizedFields) diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index af99c3e32..76b220e0f 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -1195,6 +1195,13 @@ async function runReload(opts?: { // not surface as a schema diff — so run the idempotent metadata sync before // returning, otherwise a metadata-only edit (e.g. toggling `versions`) would // not persist until the dev server restarts. + // Also provision on the no-DDL path. A missing `_locales` table produces no schema + // diff — companion tables are excluded from it — so `hasChanges` stays false and the + // reload would return before ever reaching the call after the apply, which is exactly + // the missing-companion state this repairs. Idempotent, so running it here and after + // a successful apply costs one existence probe per localized entity. + await ensureLocalizedCompanionsForReload(adapter, newConfig); + if (!hasChanges) { // Only sync when the schema is genuinely in step (every entity had a zero-op // diff). If a real schema change was deferred (unsafe/needs review) or a diff From b8bdba5f3a8e0cc0c12b8493dc2a999dbc6f881e Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Wed, 29 Jul 2026 21:23:44 +0500 Subject: [PATCH 15/20] fix(nextly): verify main columns before the pre-companion fallback While a companion table is missing, a write in the default language is meant to stay on the main table. That is only possible where the main table still carries the translatable columns. An entity localized from creation keeps them solely on the companion and its generated runtime table omits them, so the fallback handed those keys to an ORM that does not declare them: they were dropped, `updated_at` still moved, and the write reported success having saved nothing. Collections, singles and field groups now prove the column is physically present before taking that fallback, and refuse with the same 409 when it is not. The check goes through a shared `mainTableHasColumn` built on `introspectLiveSnapshot` rather than a `SELECT ... LIMIT 0` probe: a probe cannot tell a missing column from an unreachable database, so a transient failure surfaced as a misleading "translations are not ready" refusal. The checks resolve before the caller opens its transaction. Running them inside would borrow a second connection while the transaction holds one, which on a small pool means waiting for a connection that cannot be released until it finishes; it also keeps a refusal exactly as raised rather than passing it through the adapter's error classification on the way out. Field groups get `assertLocalizedFieldGroupsWritable` for that, wired at all three transactional callers, and their in-transaction splits probe on the transaction's own connection, signalled by an explicit optional `txAdapter` rather than by adapter identity. --- .changeset/localized-write-companion-guard.md | 4 +- .../services/collection-mutation-service.ts | 51 ++++- .../services/field-group-data-service.ts | 12 ++ .../services/field-group-mutation-service.ts | 119 ++++++++++- ...rite-without-companion.integration.test.ts | 189 +++++++++++++++++- .../src/domains/i18n/runtime/companion-io.ts | 51 +++++ ...ngle-without-companion.integration.test.ts | 67 +++++++ .../services/single-mutation-service.ts | 146 ++++++-------- 8 files changed, 544 insertions(+), 95 deletions(-) diff --git a/.changeset/localized-write-companion-guard.md b/.changeset/localized-write-companion-guard.md index 60d9b21a5..de454ff6d 100644 --- a/.changeset/localized-write-companion-guard.md +++ b/.changeset/localized-write-companion-guard.md @@ -24,6 +24,8 @@ Saving a translation could overwrite the original language. `nextly db:sync` marks a collection as localized in a separate process from the running app, so the app could show the language switcher before its translations table existed — and a translation saved in that window wrote over the original-language values and changed the entry's URL, while reporting success. -The translations table is now prepared during `db:sync` and during a dev config reload, for collections, singles and field groups alike. If it is still missing, a write in a non-default language is refused with a clear message instead of overwriting anything, and the same refusal now covers singles and embedded field groups rather than only collections. Writing the default language before the table exists is unchanged. +The translations table is now prepared during `db:sync` and during a dev config reload, for collections, singles and field groups alike. If it is still missing, a write in a non-default language is refused with a clear message instead of overwriting anything, and the same refusal now covers singles and embedded field groups rather than only collections. + +Writing the default language before the table exists still goes to the main table as before. The one exception is content that was localized from the start, whose translatable values have never had a main-table column to fall back to: saving that while the translations table is missing used to fail with a database error, and now reports the same clear message as the case above. Collections and singles that set a custom `dbName` are handled correctly here too; previously their translations table could be created against a table name that does not exist. And a database that is unreachable or refusing connections is no longer reported as a missing translations table. diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 1678dd96b..dfaeb080e 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -89,7 +89,10 @@ import { isValidLocale, resolveRequestedLocale, } from "../../i18n/resolve-locale"; -import { companionTableExists as sharedCompanionTableExists } from "../../i18n/runtime/companion-io"; +import { + companionTableExists as sharedCompanionTableExists, + mainTableHasColumn, +} from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; import { @@ -977,6 +980,32 @@ export class CollectionMutationService extends BaseService { }, }); } + // The default language keeps the fallback, but only where it can actually + // work. A collection localized from creation keeps its translatable columns + // solely on the companion, and the generated main-table schema omits them, so + // returning null here would leave those values in the main payload for a table + // that has no columns for them. That write cannot land: it reaches the driver + // and fails as a 500. Refusing here says the same thing in terms the caller can + // act on, and says it before anything is attempted. + const fallbackPossible = await mainTableHasColumn( + this.adapter, + // The companion is always `
_locales`, so the main table is its stem. + companion.companionTableName.replace(/_locales$/, ""), + companion.localizedFields[0]?.column + ); + if (!fallbackPossible) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this collection yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + collection: collectionName, + locale: requested, + companionTable: companion.companionTableName, + }, + }); + } return null; } @@ -2231,6 +2260,16 @@ export class CollectionMutationService extends BaseService { // collection opted out of recording), so the post-commit fast drain is // scheduled only for a write that recorded something. let recorded = false; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection and deadlock a single-connection pool, and a NextlyError raised in + // the callback is reclassified by the adapter into an opaque database error — + // so the actionable 409 would never reach the caller. + await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + }); await this.adapter.transaction(async tx => { const rawEntry = await tx.insert(tableName, entryData, { returning: "*", @@ -4233,6 +4272,16 @@ export class CollectionMutationService extends BaseService { // resolves, so a rolled-back attempt (a version conflict) or a commit // failure never flags a durable event that isn't there. let recorded = false; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection and deadlock a single-connection pool, and a NextlyError raised in + // the callback is reclassified by the adapter into an opaque database error — + // so the actionable 409 would never reach the caller. + await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + }); await withVersionConflictRetry(() => this.adapter.transaction(async tx => { recorded = false; diff --git a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts index b4781f936..327d0ed15 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts @@ -131,6 +131,18 @@ export class FieldGroupDataService { return this.mutationService.saveComponentDataInTransaction(tx, params); } + /** + * Verify every localized field group in a payload can be written, BEFORE the caller opens its + * transaction. See {@link FieldGroupMutationService.assertLocalizedFieldGroupsWritable} — this + * cannot run inside the transaction without risking pool starvation, and answering it first + * keeps a refusal exactly as raised. + */ + assertLocalizedFieldGroupsWritable( + params: Pick + ): Promise { + return this.mutationService.assertLocalizedFieldGroupsWritable(params); + } + deleteComponentData(params: DeleteComponentDataParams): Promise { return this.mutationService.deleteComponentData(params); } diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index 7904f92ac..6e69e1864 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -29,6 +29,7 @@ import { resolveRequestedLocale } from "../../i18n/resolve-locale"; import { buildCompanionSchema, companionTableExists, + mainTableHasColumn, splitLocalizedWrite, upsertCompanionRow, } from "../../i18n/runtime/companion-io"; @@ -104,10 +105,14 @@ export class FieldGroupMutationService extends BaseService { meta: DynamicFieldGroupRecord, data: Record, locale: string | undefined, - writeAdapter: { + // Present only on the in-transaction path, where the `*InTx` variants pass the + // transaction's own adapter. Its presence is therefore the signal for "a parent + // transaction is already open", which is what decides whether introspecting from + // here is safe. Omitted on the pooled path. + txAdapter?: { dialect: SupportedDialect; executeQuery(sql: string, params?: unknown[]): Promise; - } = this.adapter + } ): Promise<{ schema: ReturnType; main: Record; @@ -116,6 +121,7 @@ export class FieldGroupMutationService extends BaseService { if (!this.localization || meta.localized !== true) { return { schema: null, main: data, companion: {} }; } + const writeAdapter = txAdapter ?? this.adapter; const schema = buildCompanionSchema({ slug: meta.slug, tableName: meta.tableName, @@ -127,9 +133,10 @@ export class FieldGroupMutationService extends BaseService { // Probe BEFORE splitting. Splitting first and then discovering the companion is // absent strands the translatable values: they leave the main payload and the // upsert that would have taken them is skipped, so the write reports success - // having saved nothing. Checking here also means the refusal is raised before - // the caller opens its transaction, where the adapters' `classifyError` would - // turn a `NextlyError` into an opaque database error and lose the 409. + // having saved nothing. Checking here also means the refusal is raised before the + // caller opens its transaction, so it leaves exactly as raised rather than through + // the adapter's error classification, which rewraps anything that is not already a + // `DatabaseError`. if ( !(await companionTableExists(writeAdapter, schema.companionTableName)) ) { @@ -146,8 +153,35 @@ export class FieldGroupMutationService extends BaseService { }, }); } - // Default language: keep the values on the existing main-table column, the - // same pre-companion fallback collections and singles preserve. + // Default language keeps the pre-companion fallback — but only where it can + // actually work. A field group whose main `comp_*` table never had these + // columns (localized from creation, or already migrated) would otherwise take + // this branch and hand the values to a table with nowhere to put them, which + // fails at the driver as a 500. Refusing turns that into an answer the caller + // can act on. + // Only introspect on the pooled path. Inside a parent transaction this would + // borrow a second connection and deadlock a single-connection pool — and it does + // not need to, because `assertLocalizedFieldGroupsWritable` has already answered + // the same question before that transaction opened. + const fallbackPossible = + txAdapter !== undefined || + (await mainTableHasColumn( + this.adapter, + meta.tableName, + schema.localizedFields[0]?.column + )); + if (!fallbackPossible) { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this field group yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + fieldGroupTable: schema.companionTableName, + locale: writeLocale, + }, + }); + } return { schema: null, main: data, companion: {} }; } const { main, companion } = splitLocalizedWrite( @@ -294,6 +328,59 @@ export class FieldGroupMutationService extends BaseService { } } + /** + * Answer, BEFORE the caller opens its transaction, whether every localized field group in + * this payload can actually be written. + * + * Two reasons it cannot wait until the write itself. The probes borrow a connection from the + * pool, so running them inside the parent transaction waits for a connection that cannot be + * released until that transaction finishes; and answering here keeps a refusal exactly as + * raised, rather than passing it through the adapter's error classification on the way out of + * a transaction callback, which rewraps anything that is not already a `DatabaseError`. + * + * Idempotent and read-only — it introspects and probes, and writes nothing. + */ + async assertLocalizedFieldGroupsWritable( + params: Pick + ): Promise { + if (!this.localization) return; + for (const field of params.fields) { + if (!isFieldGroupField(field)) continue; + const value = params.data[field.name]; + if (value === undefined || value === null) continue; + // Mirror `saveComponentData`'s dispatch exactly, including its precedence: a field + // carrying both `components` and `component` is written as a dynamic zone, so + // deciding `component` first here would check a type the write never touches. + // + // For a dynamic zone the type travels per instance, so the PAYLOAD decides what is + // written, not the field's list of permitted types. Walking the permitted list + // instead would probe types absent from this write — and would refuse a perfectly + // good save whenever some other permitted type happened to be missing its + // companion. Deduplicated, because a zone commonly repeats one type. + const slugs = new Set(); + if (field.components && field.components.length > 0) { + for (const instance of Array.isArray(value) ? value : []) { + const type = (instance as Record | null)?.[ + STORAGE_FORMAT.wireTypeKey + ]; + if (typeof type === "string" && field.components.includes(type)) { + slugs.add(type); + } + } + } else if (field.component) { + slugs.add(field.component); + } + for (const slug of slugs) { + const meta = await this.registryService.getComponentBySlug(slug); + if (!meta || meta.localized !== true) continue; + // Reuse the same split the write performs: it raises the 409 when the + // companion is missing and the fallback is unavailable, which is exactly the + // decision needed here — and on the pooled adapter, outside any transaction. + await this.splitLocalizedComponent(meta, {}, params.locale); + } + } + } + async saveComponentDataInTransaction( tx: TransactionContext, params: SaveComponentDataParams @@ -530,7 +617,11 @@ export class FieldGroupMutationService extends BaseService { const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, data, - locale + locale, + // Probe on the TRANSACTION's connection, never the pool: the parent + // transaction already holds one, and a single-connection pool would + // deadlock waiting for a second. + this.txWriteAdapter(tx) ); let instanceId: string; @@ -737,7 +828,11 @@ export class FieldGroupMutationService extends BaseService { const { schema, main, companion } = await this.splitLocalizedComponent( componentMeta, instance, - locale + locale, + // Probe on the TRANSACTION's connection, never the pool: the parent + // transaction already holds one, and a single-connection pool would + // deadlock waiting for a second. + this.txWriteAdapter(tx) ); await this.prepareInstanceForWrite( @@ -1060,7 +1155,11 @@ export class FieldGroupMutationService extends BaseService { const { schema, main, companion } = await this.splitLocalizedComponent( meta, instance, - locale + locale, + // Probe on the TRANSACTION's connection, never the pool: the parent + // transaction already holds one, and a single-connection pool would + // deadlock waiting for a second. + this.txWriteAdapter(tx) ); await this.prepareInstanceForWrite( diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 81c12b15f..de3d3331d 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -27,7 +27,12 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; -import { defineCollection, text } from "../../../config"; +import { + defineCollection, + defineFieldGroup, + fieldGroup, + text, +} from "../../../config"; import { createAdapter } from "../../../database/factory"; import { createTestNextly, @@ -70,6 +75,26 @@ const posts = (localized: boolean) => fields: [text({ name: "title", localized: true })], }); +/** + * Same shape plus a genuinely SHARED field. `localized: false` is load-bearing: in a + * localized collection a text field localizes by default (`defaultLocalizedForType`), so + * without the explicit flag `author` would live on the companion too and the main table + * would have no column at all. + * + * That distinction is the whole point of this fixture. With a real shared column the + * UPDATE still has something valid to write, which is the shape that can commit a partial + * row rather than failing outright. + */ +const mixedPosts = (localized: boolean) => + defineCollection({ + slug: "i18nwin_mixed", + localized, + fields: [ + text({ name: "title", localized: true }), + text({ name: "author", localized: false }), + ], + }); + async function boot(localized: boolean): Promise { process.env.DB_DIALECT = "sqlite"; const adapter = await createAdapter({ @@ -237,5 +262,167 @@ describe.each(getConfiguredTestDialects())( expect(translated.statusCode).toBe(409); expect(translated.message).toMatch(/Translations are not ready/); }); + + it("refuses a default-locale update when the main table never had the column", async () => { + // The other half of the same window. The default language keeps the pre-companion + // fallback, so the split hands its values back to the main payload — but a + // collection localized from creation never had those columns on the main table. + // Without the guard the write reaches the driver and dies there: measured at 409 + // with it, 500 without, on every dialect. The content is not lost, but the caller + // is told nothing it can act on. + current = await createTestNextly({ + dialect, + collections: [posts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_posts", + overrideAccess: true, + locale: "en", + }, + { title: "Original" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_posts_locales`" : '"dc_i18nwin_posts_locales"'}` + ); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_posts", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "Edited" } + ); + + expect(updated.success).toBe(false); + expect(updated.statusCode).toBe(409); + expect(updated.message).toMatch(/Translations are not ready/); + }); + + it("does not half-apply the write when a shared field accompanies the translation", async () => { + // A real shared column alongside the translatable one is the shape most likely to + // commit a partial write, because `author` on its own would make a perfectly valid + // UPDATE. Measured: it still does not — the statement carries `title` too, the main + // table has no such column, and the whole statement fails as a 500. So the guard's + // contribution here is an actionable 409 in place of a driver error, not the + // prevention of a silent half-write. + // + // The `author` assertion holds that line: were the write ever to become partial, + // keeping the shared half and dropping the translation, this is what would catch it. + current = await createTestNextly({ + dialect, + collections: [mixedPosts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_mixed", + overrideAccess: true, + locale: "en", + }, + { title: "Original", author: "Ada" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_mixed_locales`" : '"dc_i18nwin_mixed_locales"'}` + ); + + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_mixed", + entryId: id, + overrideAccess: true, + locale: "en", + }, + { title: "Edited", author: "Grace" } + ); + + expect(updated.success).toBe(false); + expect(updated.statusCode).toBe(409); + + // The refusal has to be total. A half-applied write that keeps the shared field and + // drops the translation is exactly the loss this guard exists to prevent. + const rows = await current.adapter.executeQuery<{ author: string }>( + `SELECT author FROM ${dialect === "mysql" ? "`dc_i18nwin_mixed`" : '"dc_i18nwin_mixed"'}` + ); + expect(rows[0]?.author).toBe("Ada"); + }); } ); + +/** + * A dynamic zone permits several field-group types, but a write only touches the ones its + * payload actually contains. The pre-transaction guard has to scope itself the same way. + * + * Scoping it to the PERMITTED list instead refuses a perfectly good save whenever any other + * allowed type is missing its companion — a type this write was never going to touch. The + * parent collection here is NOT localized, which is also the case that reaches this guard + * without passing through the collection or single ones. + */ +describe("dynamic zone whose unused field group has no companion (integration)", () => { + it("saves a block type whose companion exists while another permitted type's is missing", async () => { + current = await createTestNextly({ + fieldGroups: [ + defineFieldGroup({ + slug: "znok", + localized: true, + fields: [text({ name: "heading", localized: true })], + }), + defineFieldGroup({ + slug: "znbroken", + localized: true, + fields: [text({ name: "caption", localized: true })], + }), + ], + collections: [ + defineCollection({ + slug: "i18nwin_zone", + fields: [ + text({ name: "title" }), + fieldGroup({ + name: "layout", + components: ["znok", "znbroken"], + repeatable: true, + }), + ], + }), + ], + localization, + }); + + // Only the type this write does NOT use loses its companion. + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS comp_znbroken_locales" + ); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { collectionName: "i18nwin_zone", overrideAccess: true, locale: "en" }, + { + title: "Page", + layout: [{ _componentType: "znok", heading: "Hello" }], + } + ); + + expect(created.success).toBe(true); + }); +}); diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 0859a63ac..aaf04a587 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -176,6 +176,57 @@ export async function upsertCompanionRow( ); } +/** + * Adapter surface for asking about a table's PHYSICAL shape: the Drizzle handle, which the + * shared introspection helper needs. Separate from {@link CompanionWriteAdapter} so the + * read/write helpers, which never introspect, keep their narrower contract. + */ +export interface CompanionIntrospectAdapter extends CompanionWriteAdapter { + getDrizzle(): T; +} + +/** + * Whether the main table still physically carries `columnName`. + * + * This answers one question for every entity type: **can the pre-companion fallback actually + * persist anything?** While the companion is missing, a write in the default language is meant + * to stay on the main table — but that is only true for an entity whose columns are still + * there. An entity localized from creation (or one whose migration has run) keeps them only on + * the companion, and its registered runtime table omits them, so the write carries keys the + * table has no columns for. Measured on all three dialects, that surfaces as a driver error + * and a 500 rather than a wrong value — the write does not quietly commit. Answering the + * question up front turns that opaque failure into a refusal the caller can act on. + * + * Goes through the same introspection the schema pipeline uses rather than a `SELECT ... LIMIT 0` + * probe. That matters beyond convention: a probe cannot tell "this column does not exist" from + * "the database is unreachable", so a transient failure would read as a missing column and + * produce a misleading "translations are not ready" refusal instead of the real error. + * Introspection fails loudly, and this deliberately does not catch. + * + * MUST be called before the caller opens its transaction. It borrows a connection from the pool, + * so running it inside one waits for a connection that cannot be released until that transaction + * finishes — starvation on a small pool. Resolving it first also keeps a refusal exactly as + * raised: errors leaving a transaction callback pass through the adapter's error classification, + * which rewraps anything that is not already a `DatabaseError`. + */ +export async function mainTableHasColumn( + adapter: CompanionIntrospectAdapter, + tableName: string, + columnName: string | undefined +): Promise { + if (!columnName) return false; + const { introspectLiveSnapshot } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const snapshot = await introspectLiveSnapshot( + adapter.getDrizzle(), + adapter.dialect, + [tableName] + ); + const table = snapshot.tables.find(t => t.name === tableName); + return table?.columns.some(c => c.name === columnName) === true; +} + // Whether a probe error is a verified "this TABLE does not exist" for the // current dialect, as opposed to a transient/connection/permission error or a // different missing resource (a missing DATABASE, schema, column, or role). The diff --git a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts index 37c9732a3..e32ef75c5 100644 --- a/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/singles/__tests__/localized-single-without-companion.integration.test.ts @@ -22,9 +22,12 @@ import { defineSingle, text } from "../../../config"; import { createAdapter } from "../../../database/factory"; import { createTestNextly, + getConfiguredTestDialects, type TestNextly, } from "../../../plugins/test-nextly"; +import type { SingleEntryService } from "../services/single-entry-service"; + let dir: string; let dbPath: string; let current: TestNextly | undefined; @@ -97,4 +100,68 @@ describe("localized single without a companion table (integration)", () => { ); expect(rows[0]?.headline).toBe("Edited while the companion was missing"); }); + + it("refuses a default-language write when the main table never had the column", async () => { + // Localized from creation, so `headline` only ever existed on the companion and the + // main table has no column to fall back to. The case above cannot show this: there + // the Single started life unlocalized, so its legacy main column is still present and + // the fallback genuinely works. + current = await boot(true); + await current.adapter.executeQuery( + "DROP TABLE IF EXISTS single_swin_settings_locales" + ); + + await expect( + current.nextly.updateSingle({ + slug: "swin_settings", + data: { headline: "Edited while the companion was missing" }, + locale: "en", + } as Parameters[0]) + ).rejects.toThrow(/Translations are not ready/); + }); }); + +/** + * The same refusal, on a real server, for a Single localized from creation — the case the + * SQLite pair above cannot cover, because there the Single starts life unlocalized and so + * keeps a usable main column. + * + * This pins the refusal down to an actionable 409 on every dialect. It asserts the status + * code rather than the message because the message is the weaker signal: when a + * `NextlyError` reaches an adapter's `classifyError` it is rewrapped, but the original + * `message` is copied onto the wrapper, so the text alone cannot tell a clean refusal from + * a rewrapped one. + * + * Note what this does NOT cover: the write path also resolves this before opening its + * transaction, so the probe cannot wait on a connection the transaction itself holds. That + * matters only on a single-connection pool, where the symptom is a hang rather than a + * wrong value, so it is left to the pool's own configuration rather than pinned here. + */ +describe.each(getConfiguredTestDialects())( + "localized single without a companion table on %s (integration)", + dialect => { + it("refuses a default-language write with an actionable 409", async () => { + current = await createTestNextly({ + dialect, + singles: [settings(true)], + localization, + }); + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`single_swin_settings_locales`" : '"single_swin_settings_locales"'}` + ); + + const result = await current + .getService("singleEntryService") + .update( + "swin_settings", + { headline: "Edited while the companion was missing" }, + { locale: "en" } + ); + + expect(result.success).toBe(false); + expect(result.statusCode).toBe(409); + expect(result.message).toMatch(/Translations are not ready/); + }); + } +); diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index abd6c63fb..6181eb4e1 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -81,6 +81,7 @@ import { import { buildCompanionSchema, companionTableExists, + mainTableHasColumn, splitLocalizedWrite, upsertCompanionRow, type CompanionSchema, @@ -229,30 +230,6 @@ export class SingleMutationService extends BaseService { // Webhook helpers // ============================================================ - /** - * Whether the main table still carries a localized column — the only state in which - * the pre-companion fallback can actually persist anything. Probed rather than read - * from the runtime schema, because that schema already omits the column for a Single - * localized from creation, which is exactly the case being distinguished. - */ - private async mainHasLocalizedColumn( - tableName: string, - columnName: string | undefined - ): Promise { - if (!columnName) return false; - const isMysql = this.adapter.dialect === "mysql"; - const table = isMysql ? `\`${tableName}\`` : `"${tableName}"`; - const column = isMysql ? `\`${columnName}\`` : `"${columnName}"`; - try { - await this.adapter.executeQuery(`SELECT ${column} FROM ${table} LIMIT 0`); - return true; - } catch { - // A missing column is the answer this asks for. Any other failure surfaces on - // the write that follows, on the same connection. - return false; - } - } - /** * Read a Single's FULL companion translation state for one locale, keyed by * companion column — every localized field carries its stored value or is @@ -889,34 +866,53 @@ export class SingleMutationService extends BaseService { companion.companionTableName ) : false; - // A localized single whose companion table does not exist yet has nowhere - // to put a NON-default locale's values: the split below moves them out of - // the main payload and the companion upsert is then skipped, so the - // translation is silently dropped while the write reports success. The - // window is real — `db:sync` flips the registry's `localized` flag in its - // own process before the running server creates the companion — so refuse - // rather than discard the user's content. Default-locale values stay on - // the main table until the companion exists, which is intended. - if ( - companion && - !companionPhysicallyExists && - this.localization && - writeLocale !== undefined && - writeLocale !== this.localization.defaultLocale - ) { - throw NextlyError.conflict({ - reason: "state", - message: - "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", - logContext: { - cause: "localized-write-without-companion", - single: singleMeta.slug, - locale: writeLocale, - defaultLocale: this.localization.defaultLocale, - companionTable: companion.companionTableName, - }, - }); + // A localized single whose companion table does not exist yet has nowhere to put + // a NON-default locale's values: the split below moves them out of the main + // payload and the companion upsert is then skipped, so the translation is + // silently dropped while the write reports success. The window is real — + // `db:sync` flips the registry's `localized` flag in its own process before the + // running server creates the companion — so refuse rather than discard. + // + // The DEFAULT locale keeps the pre-companion fallback, but only where it can + // actually work: an entity localized from creation keeps its translatable + // columns solely on the companion, and its registered runtime table omits them, + // so those keys would be dropped by the ORM while `updated_at` still moved. + // + // Both checks run HERE, before `adapter.transaction` opens. Introspecting from + // inside it would borrow a second connection while the transaction holds one, + // which on a small pool means waiting for a connection that cannot be released + // until this transaction finishes. Resolving it first also keeps the refusal + // exactly as raised: errors leaving a transaction callback pass through the + // adapter's error classification, which rewraps anything that is not already a + // `DatabaseError`. + if (companion && !companionPhysicallyExists && this.localization) { + // Captured so the closure below keeps the narrowed type. + const defaultLocale = this.localization.defaultLocale; + const refuse = (): never => { + throw NextlyError.conflict({ + reason: "state", + message: + "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", + logContext: { + cause: "localized-write-without-companion", + single: singleMeta.slug, + locale: writeLocale, + defaultLocale, + companionTable: companion.companionTableName, + }, + }); + }; + if (writeLocale !== undefined && writeLocale !== defaultLocale) { + refuse(); + } + const fallbackPossible = await mainTableHasColumn( + this.adapter, + singleMeta.tableName, + companion.localizedFields[0]?.column + ); + if (!fallbackPossible) refuse(); } + // Same pre-transaction, pooled probe for the auto-create default seed: it // is keyed on the DEFAULT locale (not the write locale), so it needs its // own existence check rather than reusing `companionPhysicallyExists`. @@ -927,6 +923,19 @@ export class SingleMutationService extends BaseService { ) : false; let updatedRows: SingleDocument[]; + // Verify every localized field group in this payload can actually be written + // BEFORE the transaction opens. Inside it the probes would borrow a second + // connection while the transaction holds one, which on a small pool means + // waiting for a connection that cannot be released until it finishes. It also + // keeps the refusal exactly as raised: errors leaving a transaction callback + // pass through the adapter's error classification, which rewraps anything that + // is not already a `DatabaseError`. + await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fieldConfigs, + data: componentFieldData, + locale: options.locale, + }); + try { // Retry the whole update+capture transaction on a version_no allocation // race; the re-run re-reads the max. The single UPDATE is deterministic. @@ -1107,37 +1116,10 @@ export class SingleMutationService extends BaseService { // above (before validation); the split reuses them. Done inside the // closure so a retry re-splits the freshly-timestamped payload. // Only split when the companion physically exists. Splitting first and - // then skipping the companion upsert (which is gated on the same flag - // below) would drop the translatable values on the floor: they leave - // `mainPayload` and are never written anywhere, so the write reports - // success having saved nothing. While the table is absent those values - // belong on the main table, which is the same pre-migration fallback - // collections use — and by then the write locale is guaranteed to be - // the default one, because the guard above refuses any other. - // The fallback is only real when the main table STILL has those columns. - // A Single localized from creation (or one whose migration already ran) - // keeps them only on the companion, and the registered runtime table omits - // them — so putting the values back into `mainPayload` writes keys the ORM - // does not recognise: they are ignored, `updated_at` is still set, and the - // write reports success having saved nothing. - if (companion && !companionPhysicallyExists) { - const onMain = await this.mainHasLocalizedColumn( - singleMeta.tableName, - companion.localizedFields[0]?.column - ); - if (!onMain) { - throw NextlyError.conflict({ - reason: "state", - message: - "Translations are not ready for this single yet. Restart the app (or re-run `nextly db:sync`) to create its translation table, then try again.", - logContext: { - cause: "localized-write-without-companion", - single: singleMeta.slug, - companionTable: companion.companionTableName, - }, - }); - } - } + // then skipping the companion upsert (gated on the same flag below) would + // drop the translatable values on the floor. While the table is absent + // they stay on the main table — the pre-companion fallback — which the + // pre-transaction guard above has already proven is actually possible. const { main: mainPayload, companion: companionData } = companion && companionPhysicallyExists ? splitLocalizedWrite(updatePayload, companion.localizedFields) From d0968b4e6720473f4c659b43e1cada02eca1dae0 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 30 Jul 2026 00:42:51 +0500 Subject: [PATCH 16/20] fix(nextly): resolve companion existence before the write transaction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Probing for a companion from inside a write transaction is not safe. The probe asks about a relation that may not exist, and PostgreSQL marks the entire transaction aborted the moment a statement errors — so although the probe catches it and correctly reports "absent", every statement after it fails with `current transaction is aborted`. `assertLocalizedFieldGroupsWritable` now returns the existence it resolved, and that map is threaded through `saveComponentDataInTransaction` into the three in-transaction variants, which read it instead of asking again. It is a required parameter rather than an optional one: inside a transaction there is no safe way to work the answer out, so a caller that cannot supply it should not compile. Per-write probes drop from `1 + 2K` to `1 + K` for K distinct localized field group types. Provisioning on the config-reload path now skips entities whose own schema change was deferred. Creating a companion for a transition that was not applied is worse than leaving it absent: the later Schema Builder apply finds the companion already present, takes the plain reconcile branch instead of seeding, and the existing default-locale values are lost. Tracked per entity rather than as a single flag, so entities whose schema is in step are still provisioned on the same pass. `db:sync` also reconciles an existing companion's columns rather than only creating a missing table. Marking a further field localized on an already localized entity adds the column to the main table while the companion keeps its old shape, and the write then splits that value into a column that is not there. Additive only — an unattended sync must not drop a column, because `db:sync` persists registry metadata before its destructive prompt — and confined to `db:sync`, leaving the reload path creation-only. --- packages/nextly/src/cli/commands/dev-build.ts | 27 ++- .../services/collection-mutation-service.ts | 47 +++--- .../services/field-group-data-service.ts | 24 ++- .../services/field-group-mutation-service.ts | 155 +++++++++++++----- .../src/domains/i18n/runtime/companion-io.ts | 78 +++++++++ .../services/single-mutation-service.ts | 14 +- packages/nextly/src/init/reload-config.ts | 46 +++++- 7 files changed, 313 insertions(+), 78 deletions(-) diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index 2d96da8c7..5ce17325f 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -803,7 +803,7 @@ export async function ensureLocalizedCompanions( // write from destroying content until it runs. if (process.env.NODE_ENV === "production") return; const dialect = adapter.getCapabilities().dialect; - const { ensureCompanionTable } = await import( + const { ensureCompanionTable, reconcileCompanionColumns } = await import( "../../domains/i18n/runtime/companion-io" ); // Each entity kind resolves its physical table differently, and a custom @@ -874,6 +874,31 @@ export async function ensureLocalizedCompanions( failures.push(entity.slug!); } ); + // Creating the companion is not enough on its own. `ensureCompanionTable` returns + // immediately once the table is there, so marking a FURTHER field localized on an + // entity that is already localized leaves the companion a column short — while the + // main-table sync above has already added that column to `comp_*` / `dc_*`. The write + // then splits the value into a companion column that does not exist. Additive only, + // and confined to `db:sync`: the reload path stays creation-only, because a running + // deployment must not alter its schema off a config edit. + await reconcileCompanionColumns( + adapter as unknown as DrizzleAdapter, + { + slug: entity.slug, + tableName, + fields: entity.fields ?? [], + dialect, + status: entity.status === true, + }, + error => { + logger.error( + `Could not update the translations table for "${entity.slug}" (${tableName}_locales). ` + + `Newly translatable fields will fail to save until it matches: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + failures.push(entity.slug!); + } + ); } } // Fail the command. Exiting 0 here would tell deployment automation the schema is diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index dfaeb080e..1b7339775 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -2265,11 +2265,12 @@ export class CollectionMutationService extends BaseService { // connection and deadlock a single-connection pool, and a NextlyError raised in // the callback is reclassified by the adapter into an opaque database error — // so the actionable 409 would never reach the caller. - await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ - fields: fields as unknown as FieldConfig[], - data: componentFieldData, - locale: params.locale, - }); + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + })) ?? new Map(); await this.adapter.transaction(async tx => { const rawEntry = await tx.insert(tableName, entryData, { returning: "*", @@ -2312,15 +2313,19 @@ export class CollectionMutationService extends BaseService { this.fieldGroupDataService && Object.keys(componentFieldData).length > 0 ) { - await this.fieldGroupDataService.saveComponentDataInTransaction(tx, { - parentId: entry.id as string, - parentTable: tableName, - fields: fields as unknown as FieldConfig[], - data: componentFieldData, - // i18n: thread the write locale so an embedded localized component writes - // translatable fields to its companion within the same transaction. - locale: params.locale, - }); + await this.fieldGroupDataService.saveComponentDataInTransaction( + tx, + { + parentId: entry.id as string, + parentTable: tableName, + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + // i18n: thread the write locale so an embedded localized component writes + // translatable fields to its companion within the same transaction. + locale: params.locale, + }, + fieldGroupPresence + ); } // Write many-to-many junction rows inside the transaction so a junction @@ -4277,11 +4282,12 @@ export class CollectionMutationService extends BaseService { // connection and deadlock a single-connection pool, and a NextlyError raised in // the callback is reclassified by the adapter into an opaque database error — // so the actionable 409 would never reach the caller. - await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ - fields: fields as unknown as FieldConfig[], - data: componentFieldData, - locale: params.locale, - }); + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fields as unknown as FieldConfig[], + data: componentFieldData, + locale: params.locale, + })) ?? new Map(); await withVersionConflictRetry(() => this.adapter.transaction(async tx => { recorded = false; @@ -4605,7 +4611,8 @@ export class CollectionMutationService extends BaseService { // i18n: thread the write locale so an embedded localized component writes // translatable fields to its companion within the same transaction. locale: params.locale, - } + }, + fieldGroupPresence ); } diff --git a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts index 327d0ed15..ec81aa4b8 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-data-service.ts @@ -11,6 +11,7 @@ import { FieldGroupMutationService, type SaveComponentDataParams, type DeleteComponentDataParams, + type FieldGroupCompanionPresence, } from "./field-group-mutation-service"; import { FieldGroupQueryService, @@ -126,20 +127,31 @@ export class FieldGroupDataService { saveComponentDataInTransaction( tx: TransactionContext, - params: SaveComponentDataParams + params: SaveComponentDataParams, + presence: FieldGroupCompanionPresence ): Promise { - return this.mutationService.saveComponentDataInTransaction(tx, params); + return this.mutationService.saveComponentDataInTransaction( + tx, + params, + presence + ); } /** * Verify every localized field group in a payload can be written, BEFORE the caller opens its - * transaction. See {@link FieldGroupMutationService.assertLocalizedFieldGroupsWritable} — this - * cannot run inside the transaction without risking pool starvation, and answering it first - * keeps a refusal exactly as raised. + * transaction, and return what it found out. See + * {@link FieldGroupMutationService.assertLocalizedFieldGroupsWritable} — this cannot run inside + * the transaction without risking pool starvation, and answering it first keeps a refusal + * exactly as raised. + * + * The returned map is not optional bookkeeping: it must be handed to + * `saveComponentDataInTransaction`, because that is the only way the in-transaction write can + * learn whether a companion exists without probing for it, which would abort the transaction + * on PostgreSQL when it does not. */ assertLocalizedFieldGroupsWritable( params: Pick - ): Promise { + ): Promise { return this.mutationService.assertLocalizedFieldGroupsWritable(params); } diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index 6e69e1864..7cac30115 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -79,6 +79,18 @@ function isFieldGroupField(field: FieldConfig): field is FieldGroupFieldConfig { return field.type === STORAGE_FORMAT.fieldType; } +/** + * Whether each localized field group's companion table physically exists, keyed by slug and + * resolved BEFORE the write transaction opens. + * + * This answers a question that cannot be asked again once a transaction is open. Probing for + * a relation that does not exist raises an error, and PostgreSQL marks the entire transaction + * aborted the moment one does — so although the probe catches it and reports "absent", every + * following statement fails with `current transaction is aborted`. Carrying the pre-resolved + * answer in is what lets the in-transaction path know without asking. + */ +export type FieldGroupCompanionPresence = ReadonlyMap; + export class FieldGroupMutationService extends BaseService { private readonly registryService: FieldGroupRegistryService; @@ -105,23 +117,35 @@ export class FieldGroupMutationService extends BaseService { meta: DynamicFieldGroupRecord, data: Record, locale: string | undefined, - // Present only on the in-transaction path, where the `*InTx` variants pass the - // transaction's own adapter. Its presence is therefore the signal for "a parent - // transaction is already open", which is what decides whether introspecting from - // here is safe. Omitted on the pooled path. - txAdapter?: { - dialect: SupportedDialect; - executeQuery(sql: string, params?: unknown[]): Promise; + // Present only on the in-transaction path. Both members travel together by design: + // inside a transaction the companion's existence must ALREADY be known, because asking + // again means probing a possibly-absent relation, and on PostgreSQL that aborts the + // whole transaction. Omitted on the pooled path, which is free to probe. + tx?: { + adapter: { + dialect: SupportedDialect; + executeQuery( + sql: string, + params?: unknown[] + ): Promise; + }; + presence: FieldGroupCompanionPresence; } ): Promise<{ schema: ReturnType; main: Record; companion: Record; + /** Whether the companion physically exists, so callers can reuse it without re-probing. */ + companionExists: boolean; }> { if (!this.localization || meta.localized !== true) { - return { schema: null, main: data, companion: {} }; + return { + schema: null, + main: data, + companion: {}, + companionExists: false, + }; } - const writeAdapter = txAdapter ?? this.adapter; const schema = buildCompanionSchema({ slug: meta.slug, tableName: meta.tableName, @@ -129,17 +153,32 @@ export class FieldGroupMutationService extends BaseService { dialect: this.adapter.dialect, status: false, }); - if (!schema) return { schema: null, main: data, companion: {} }; - // Probe BEFORE splitting. Splitting first and then discovering the companion is - // absent strands the translatable values: they leave the main payload and the - // upsert that would have taken them is skipped, so the write reports success - // having saved nothing. Checking here also means the refusal is raised before the + if (!schema) + return { + schema: null, + main: data, + companion: {}, + companionExists: false, + }; + // Resolve existence BEFORE splitting. Splitting first and then discovering the + // companion is absent strands the translatable values: they leave the main payload + // and the upsert that would have taken them is skipped, so the write reports success + // having saved nothing. Resolving here also means any refusal is raised before the // caller opens its transaction, so it leaves exactly as raised rather than through // the adapter's error classification, which rewraps anything that is not already a // `DatabaseError`. - if ( - !(await companionTableExists(writeAdapter, schema.companionTableName)) - ) { + // + // Inside a transaction the answer is READ, never probed. A probe against a missing + // relation aborts the entire transaction on PostgreSQL — the error is caught here but + // the connection is already poisoned, so the fallback write that follows would die + // with `current transaction is aborted`. `?? true` is unreachable in practice, since + // the pre-transaction pass walks exactly the slugs this write touches; assuming + // "provisioned" is the safe way to be wrong, because it fails loudly and atomically + // instead of quietly writing translatable values to the wrong table. + const companionExists = tx + ? (tx.presence.get(meta.slug) ?? true) + : await companionTableExists(this.adapter, schema.companionTableName); + if (!companionExists) { const writeLocale = resolveRequestedLocale(this.localization, locale); if (writeLocale !== this.localization.defaultLocale) { throw NextlyError.conflict({ @@ -164,7 +203,7 @@ export class FieldGroupMutationService extends BaseService { // not need to, because `assertLocalizedFieldGroupsWritable` has already answered // the same question before that transaction opened. const fallbackPossible = - txAdapter !== undefined || + tx !== undefined || (await mainTableHasColumn( this.adapter, meta.tableName, @@ -182,7 +221,7 @@ export class FieldGroupMutationService extends BaseService { }, }); } - return { schema: null, main: data, companion: {} }; + return { schema: null, main: data, companion: {}, companionExists }; } const { main, companion } = splitLocalizedWrite( data, @@ -195,6 +234,7 @@ export class FieldGroupMutationService extends BaseService { schema, main, companion: this.serializeCompanionValues(companion, schema, meta.fields), + companionExists, }; } @@ -342,8 +382,9 @@ export class FieldGroupMutationService extends BaseService { */ async assertLocalizedFieldGroupsWritable( params: Pick - ): Promise { - if (!this.localization) return; + ): Promise { + const presence = new Map(); + if (!this.localization) return presence; for (const field of params.fields) { if (!isFieldGroupField(field)) continue; const value = params.data[field.name]; @@ -371,19 +412,32 @@ export class FieldGroupMutationService extends BaseService { slugs.add(field.component); } for (const slug of slugs) { + if (presence.has(slug)) continue; const meta = await this.registryService.getComponentBySlug(slug); if (!meta || meta.localized !== true) continue; // Reuse the same split the write performs: it raises the 409 when the // companion is missing and the fallback is unavailable, which is exactly the // decision needed here — and on the pooled adapter, outside any transaction. - await this.splitLocalizedComponent(meta, {}, params.locale); + // Its verdict on existence is then carried into the transaction, so the write + // never has to ask a question that would abort the transaction to answer. + const { companionExists } = await this.splitLocalizedComponent( + meta, + {}, + params.locale + ); + presence.set(slug, companionExists); } } + return presence; } async saveComponentDataInTransaction( tx: TransactionContext, - params: SaveComponentDataParams + params: SaveComponentDataParams, + // Resolved by `assertLocalizedFieldGroupsWritable` before this transaction opened. + // Required rather than optional: inside a transaction there is no safe way to work it + // out, since probing a missing companion aborts the transaction on PostgreSQL. + presence: FieldGroupCompanionPresence ): Promise { const { parentId, parentTable, fields, data, locale } = params; @@ -414,6 +468,7 @@ export class FieldGroupMutationService extends BaseService { field, data: fieldData, locale, + presence, }); } else if (field.component) { if (field.repeatable) { @@ -424,6 +479,7 @@ export class FieldGroupMutationService extends BaseService { componentSlug: field.component, data: fieldData, locale, + presence, }); } else { await this.saveSingleComponentInTx(tx, { @@ -433,6 +489,7 @@ export class FieldGroupMutationService extends BaseService { componentSlug: field.component, data: fieldData as ComponentInstanceData, locale, + presence, }); } } @@ -585,10 +642,18 @@ export class FieldGroupMutationService extends BaseService { componentSlug: string; data: ComponentInstanceData; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, componentSlug, data, locale } = - params; + const { + parentId, + parentTable, + fieldName, + componentSlug, + data, + locale, + presence, + } = params; try { const componentMeta = @@ -618,10 +683,10 @@ export class FieldGroupMutationService extends BaseService { componentMeta, data, locale, - // Probe on the TRANSACTION's connection, never the pool: the parent - // transaction already holds one, and a single-connection pool would - // deadlock waiting for a second. - this.txWriteAdapter(tx) + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); let instanceId: string; @@ -791,10 +856,18 @@ export class FieldGroupMutationService extends BaseService { componentSlug: string; data: unknown; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, componentSlug, data, locale } = - params; + const { + parentId, + parentTable, + fieldName, + componentSlug, + data, + locale, + presence, + } = params; if (!Array.isArray(data)) { this.logger.warn("Repeatable component data is not an array", { @@ -829,10 +902,10 @@ export class FieldGroupMutationService extends BaseService { componentMeta, instance, locale, - // Probe on the TRANSACTION's connection, never the pool: the parent - // transaction already holds one, and a single-connection pool would - // deadlock waiting for a second. - this.txWriteAdapter(tx) + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); await this.prepareInstanceForWrite( @@ -1083,9 +1156,11 @@ export class FieldGroupMutationService extends BaseService { field: FieldGroupFieldConfig; data: unknown; locale?: string; + presence: FieldGroupCompanionPresence; } ): Promise { - const { parentId, parentTable, fieldName, field, data, locale } = params; + const { parentId, parentTable, fieldName, field, data, locale, presence } = + params; const allowedSlugs = field.components ?? []; const instances = field.repeatable @@ -1156,10 +1231,10 @@ export class FieldGroupMutationService extends BaseService { meta, instance, locale, - // Probe on the TRANSACTION's connection, never the pool: the parent - // transaction already holds one, and a single-connection pool would - // deadlock waiting for a second. - this.txWriteAdapter(tx) + // Never probe from in here: the answer was resolved before this transaction + // opened, because asking now would mean querying a possibly-absent relation, + // and on PostgreSQL that aborts the transaction outright. + { adapter: this.txWriteAdapter(tx), presence } ); await this.prepareInstanceForWrite( diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index aaf04a587..96120550e 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -209,6 +209,84 @@ export interface CompanionIntrospectAdapter extends CompanionWriteAdapter { * raised: errors leaving a transaction callback pass through the adapter's error classification, * which rewraps anything that is not already a `DatabaseError`. */ +/** + * Add localized columns an EXISTING companion is missing. No-op when the companion is absent — + * creating it is {@link ensureCompanionTable}'s job. + * + * Needed because a companion is created once and then never revisited, while the entity's field + * list keeps moving. Marking a further field localized on an already-localized entity leaves the + * companion a column short, and the write splits that value straight into a column that is not + * there. + * + * **Additive only, deliberately.** A field that stops being localized leaves its companion column + * in place rather than dropping it: this runs unattended, and `db:sync` persists registry metadata + * BEFORE its destructive prompt, so a drop here would execute even for an operator who then + * declined the change. An unused column is recoverable; a dropped one is not. + * + * Issues DDL, so it belongs to `db:sync` and never to boot — a running deployment must not alter + * its own schema because a config file changed. + */ +export async function reconcileCompanionColumns( + adapter: CompanionIntrospectAdapter, + args: { + slug: string; + tableName: string; + fields: CompanionFieldLike[]; + dialect: SupportedDialect; + status?: boolean; + }, + onError?: (error: unknown) => void +): Promise { + const companionTableName = `${args.tableName}_locales`; + try { + if (!(await companionTableExists(adapter, companionTableName))) return; + + const localizedNames = new Set( + resolveLocalizedFieldNames(args.fields, true) + ); + const desired = args.fields.filter(f => localizedNames.has(f.name)); + if (desired.length === 0) return; + + const { introspectLiveSnapshot } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const snapshot = await introspectLiveSnapshot( + adapter.getDrizzle(), + adapter.dialect, + [companionTableName] + ); + const present = new Set( + snapshot.tables + .find(t => t.name === companionTableName) + ?.columns.map(c => c.name) ?? [] + ); + // Nothing missing — the overwhelmingly common case, and worth leaving before the + // statement builder runs. + if (desired.every(f => present.has(toColumn(f.name)))) return; + + // Feed the canonical builder the columns that already exist as `oldLocalized`, so what it + // emits is exactly the difference. `old` is a subset of `new` by construction here, which + // is what keeps the result additive. + const { buildCompanionReconcileStatements } = await import( + "../migration/reconcile-companion" + ); + const statements = buildCompanionReconcileStatements({ + slug: args.slug, + tableName: args.tableName, + oldLocalized: desired.filter(f => present.has(toColumn(f.name))), + newLocalized: desired, + dialect: args.dialect, + status: args.status === true, + companionExists: true, + }); + for (const stmt of statements) { + await adapter.executeQuery(stmt); + } + } catch (error) { + onError?.(error); + } +} + export async function mainTableHasColumn( adapter: CompanionIntrospectAdapter, tableName: string, diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 6181eb4e1..7c1f30127 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -930,11 +930,12 @@ export class SingleMutationService extends BaseService { // keeps the refusal exactly as raised: errors leaving a transaction callback // pass through the adapter's error classification, which rewraps anything that // is not already a `DatabaseError`. - await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ - fields: fieldConfigs, - data: componentFieldData, - locale: options.locale, - }); + const fieldGroupPresence = + (await this.fieldGroupDataService?.assertLocalizedFieldGroupsWritable({ + fields: fieldConfigs, + data: componentFieldData, + locale: options.locale, + })) ?? new Map(); try { // Retry the whole update+capture transaction on a version_no allocation @@ -1569,7 +1570,8 @@ export class SingleMutationService extends BaseService { fields: fieldConfigs, data: attemptComponentData, locale: options.locale, - } + }, + fieldGroupPresence ); } diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 76b220e0f..93896b66b 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -574,7 +574,19 @@ async function ensureLocalizedCompanionsForReload( singles?: unknown[]; fieldGroups?: unknown[]; localization?: { defaultLocale?: string }; - } + }, + // `:` for every entity whose schema change was classified unsafe (or whose diff + // threw) this cycle, so it was NOT applied. Those must be skipped: creating a companion for + // a transition that has not happened is worse than leaving it absent. The Schema Builder + // later applies the real transition, and `buildCompanionTransitionStatements` decides + // whether to SEED the existing main-table values by looking at whether the companion is + // already there. Finding one — empty, created from a config that was never applied — sends + // it down the plain reconcile branch instead, and the default-locale content is lost. + // + // Deliberately per entity rather than a single "something was deferred" flag: entities whose + // schema IS in step still need provisioning on this pass, and skipping them wholesale + // reintroduces the missing-companion window this function exists to close. + deferred: ReadonlySet = new Set() ): Promise { // Same policy the CLI applies: production schema changes belong to `nextly migrate`. if (process.env.NODE_ENV === "production") return; @@ -595,24 +607,30 @@ async function ensureLocalizedCompanionsForReload( status?: boolean; fields?: { name: string; type: string; localized?: boolean }[]; }; - const groups: [Localizable[], (e: Localizable) => string][] = [ + // The kind prefixes the `deferred` keys, because a collection and a single may share a slug + // and only one of them may have been deferred. + const groups: [string, Localizable[], (e: Localizable) => string][] = [ [ + "collection", (config.collections ?? []) as Localizable[], e => resolveCollectionTableName(e.slug!, e.dbName), ], [ + "single", (config.singles ?? []) as Localizable[], e => resolveSingleTableName({ slug: e.slug!, dbName: e.dbName }), ], [ + "fieldGroup", (config.fieldGroups ?? []) as Localizable[], e => resolveComponentTableName(e.slug!), ], ]; - for (const [entities, resolveTableName] of groups) { + for (const [kind, entities, resolveTableName] of groups) { for (const entity of entities) { if (!entity.slug || entity.localized !== true) continue; + if (deferred.has(`${kind}:${entity.slug}`)) continue; await ensureCompanionTable( adapter, { @@ -1032,6 +1050,10 @@ async function runReload(opts?: { // metadata-only sync below must be skipped rather than persist unmigrated // schema metadata; it retries on the next clean reload or restart. let deferredSchemaChange = false; + // Which entities were deferred, as `:`. The flag above answers "may the + // metadata-only sync run at all"; this answers "may THIS entity be provisioned", which is a + // per-entity question — see `ensureLocalizedCompanionsForReload`. + const deferredEntities = new Set(); const desiredCollections: Record = {}; for (const target of targets) { @@ -1076,6 +1098,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`collection:${target.slug}`); desiredCollections[target.slug] = entry; continue; } @@ -1087,6 +1110,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`collection:${target.slug}`); } } @@ -1128,6 +1152,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`single:${target.slug}`); desiredSingles[target.slug] = entry; continue; } @@ -1139,6 +1164,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping single '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`single:${target.slug}`); } } @@ -1176,6 +1202,7 @@ async function runReload(opts?: { `Builder to confirm with resolutions, or revert the config edit.` ); deferredSchemaChange = true; + deferredEntities.add(`fieldGroup:${target.slug}`); desiredComponents[target.slug] = entry; continue; } @@ -1187,6 +1214,7 @@ async function runReload(opts?: { `[Nextly HMR] Skipping component '${target.slug}' due to error during diff: ${msg}` ); deferredSchemaChange = true; + deferredEntities.add(`fieldGroup:${target.slug}`); } } @@ -1200,7 +1228,11 @@ async function runReload(opts?: { // reload would return before ever reaching the call after the apply, which is exactly // the missing-companion state this repairs. Idempotent, so running it here and after // a successful apply costs one existence probe per localized entity. - await ensureLocalizedCompanionsForReload(adapter, newConfig); + await ensureLocalizedCompanionsForReload( + adapter, + newConfig, + deferredEntities + ); if (!hasChanges) { // Only sync when the schema is genuinely in step (every entity had a zero-op @@ -1388,7 +1420,11 @@ async function runReload(opts?: { // database had no such table — non-default writes were then refused until a // restart. Runs after the apply because the companion carries a foreign key to // its main table, which a brand-new entity does not have before it. - await ensureLocalizedCompanionsForReload(adapter, newConfig); + await ensureLocalizedCompanionsForReload( + adapter, + newConfig, + deferredEntities + ); // Publish each scope's recording policy only AFTER its field-tree metadata // sync succeeds (see the assignment after the syncs below): the DDL applied, From 380a2572c550c79b1b53340d95386f1dd589bb90 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 30 Jul 2026 02:58:31 +0500 Subject: [PATCH 17/20] fix(nextly): scope the companion guard to payloads that need it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A dynamic zone defaults to `repeatable: false`, and then its payload is a single object rather than an array. The pre-transaction check read that as "no instances", so the slug never reached the presence map and the write went looking for the companion table from inside the transaction — the exact failure the check exists to prevent, and on PostgreSQL an aborted transaction. The cause was two hand-mirrored normalizations drifting apart, so both now go through one `resolveZoneInstances`: the check and the write can no longer disagree about what a payload holds. The in-transaction default changes with it, from "assume provisioned" to "assume absent" — the two are not equally safe to guess wrong, because absent takes the fallback and fails loudly on the main table whereas provisioned splits values into a table that may not exist. The guard also refused more than it needed to. It rejected every non-default locale write while the companion was missing, without first asking whether the payload contained any translatable field at all. A shared-only edit touches neither the absent table nor the default language, so it is now allowed through at all three sites. Membership is decided by the canonical split, which accepts either the camelCase field name or the snake_case companion column. Companion reconciliation now tracks `_status` alongside the translatable columns, since switching Draft/Published on after the companion was created otherwise leaves per-locale status writes targeting a column that is not there. Reported as wanted whenever it already exists, so the reconcile can add a missing `_status` but never drop one from an unattended sync. The config-reload path reconciles as well as creates. `ensureCompanionTable` returns immediately for an existing table, so marking a further field localized took the no-DDL path and left the companion a column short. Safe there despite issuing DDL: the production guard at the top of that function has already returned, so it runs only under `next dev`. --- .../services/collection-mutation-service.ts | 16 +++ .../services/field-group-mutation-service.ts | 75 ++++++++--- ...rite-without-companion.integration.test.ts | 120 ++++++++++++++++++ .../src/domains/i18n/runtime/companion-io.ts | 71 +++++++---- .../services/single-mutation-service.ts | 17 ++- packages/nextly/src/init/reload-config.ts | 28 +++- 6 files changed, 283 insertions(+), 44 deletions(-) diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 1b7339775..1c1b413ac 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -92,6 +92,7 @@ import { import { companionTableExists as sharedCompanionTableExists, mainTableHasColumn, + splitLocalizedWrite, } from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; @@ -965,6 +966,21 @@ export class CollectionMutationService extends BaseService { // its own process while the running server has yet to create the companion. // Refuse either way; the default language still writes to main, which is // the documented pre-migration fallback. + // Before refusing anything: a payload carrying nothing companion-owned has no stake + // in the missing table. There is no translatable value to strand and none to + // overwrite, so a shared-only edit — a PATCH of one non-translatable field, say — is + // entirely safe and must not be blocked. Both refusals below are about protecting + // translatable values, and this payload has none. + // + // Membership goes through the canonical split rather than a hand-rolled key check, + // because it accepts either the camelCase field name or the snake_case companion + // column, and collection writes arrive already converted to snake_case. + const { companion: localizedInPayload } = splitLocalizedWrite( + entryData, + companion.localizedFields + ); + if (Object.keys(localizedInPayload).length === 0) return null; + const requested = resolveRequestedLocale(this.localization, locale); if (requested !== this.localization.defaultLocale) { throw NextlyError.conflict({ diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index 7cac30115..f6cd20ea4 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -79,6 +79,26 @@ function isFieldGroupField(field: FieldConfig): field is FieldGroupFieldConfig { return field.type === STORAGE_FORMAT.fieldType; } +/** + * The component instances a dynamic-zone payload actually contains. + * + * A repeatable zone sends an array; a non-repeatable one — the supported default — sends a + * single object. The pre-transaction check and the write itself both need this answer, and + * mirroring the normalisation by hand in each is what let them drift: the check read a + * non-repeatable object as "no instances", so that slug never reached the presence map, and the + * write then went looking for a companion table from inside the transaction. That is precisely + * the failure the check exists to prevent. + * + * Deriving it once means the two cannot disagree about what a payload holds. + */ +function resolveZoneInstances( + field: FieldGroupFieldConfig, + value: unknown +): ComponentInstanceData[] { + const raw = field.repeatable ? value : [value]; + return Array.isArray(raw) ? (raw as ComponentInstanceData[]) : []; +} + /** * Whether each localized field group's companion table physically exists, keyed by slug and * resolved BEFORE the write transaction opens. @@ -170,15 +190,40 @@ export class FieldGroupMutationService extends BaseService { // // Inside a transaction the answer is READ, never probed. A probe against a missing // relation aborts the entire transaction on PostgreSQL — the error is caught here but - // the connection is already poisoned, so the fallback write that follows would die - // with `current transaction is aborted`. `?? true` is unreachable in practice, since - // the pre-transaction pass walks exactly the slugs this write touches; assuming - // "provisioned" is the safe way to be wrong, because it fails loudly and atomically - // instead of quietly writing translatable values to the wrong table. + // the connection is already poisoned, so the fallback write that follows would die with + // `current transaction is aborted`. + // + // `resolveZoneInstances` makes the map complete by construction, so a miss means the two + // paths have drifted again. It defaults to "absent" rather than "provisioned" because the + // two are not equally safe to guess wrong: "absent" takes the fallback, which writes to the + // main table and fails loudly there if the columns are gone, whereas "provisioned" splits + // the values out and upserts into a table that may not exist — reintroducing the very abort + // this parameter was added to prevent. const companionExists = tx - ? (tx.presence.get(meta.slug) ?? true) + ? (tx.presence.get(meta.slug) ?? false) : await companionTableExists(this.adapter, schema.companionTableName); if (!companionExists) { + // A payload carrying nothing companion-owned has no stake in the missing table: no + // translatable value to strand, none to overwrite. A shared-only edit is therefore safe + // and must not be refused. Note this is evaluated on the REAL payload, so the + // pre-transaction pass — which calls with `{}` purely to reach the existence decision — + // is unaffected and still records presence for every slug the write touches. + const { companion: localizedInPayload } = splitLocalizedWrite( + data, + schema.localizedFields + ); + if ( + Object.keys(data).length > 0 && + Object.keys(localizedInPayload).length === 0 + ) { + return { + schema: null, + main: data, + companion: {}, + companionExists: false, + }; + } + const writeLocale = resolveRequestedLocale(this.localization, locale); if (writeLocale !== this.localization.defaultLocale) { throw NextlyError.conflict({ @@ -400,7 +445,7 @@ export class FieldGroupMutationService extends BaseService { // companion. Deduplicated, because a zone commonly repeats one type. const slugs = new Set(); if (field.components && field.components.length > 0) { - for (const instance of Array.isArray(value) ? value : []) { + for (const instance of resolveZoneInstances(field, value)) { const type = (instance as Record | null)?.[ STORAGE_FORMAT.wireTypeKey ]; @@ -988,11 +1033,11 @@ export class FieldGroupMutationService extends BaseService { const { parentId, parentTable, fieldName, field, data, locale } = params; const allowedSlugs = field.components ?? []; - const instances = field.repeatable - ? (data as ComponentInstanceData[]) - : [data as ComponentInstanceData]; + // Shared with the pre-transaction check, so the two cannot disagree about which + // instances this payload holds. + const instances = resolveZoneInstances(field, data); - if (!Array.isArray(instances)) { + if (instances.length === 0 && data !== null && data !== undefined) { this.logger.warn("Multi-component data is not an array", { fieldName }); return; } @@ -1163,11 +1208,11 @@ export class FieldGroupMutationService extends BaseService { params; const allowedSlugs = field.components ?? []; - const instances = field.repeatable - ? (data as ComponentInstanceData[]) - : [data as ComponentInstanceData]; + // Shared with the pre-transaction check, so the two cannot disagree about which + // instances this payload holds. + const instances = resolveZoneInstances(field, data); - if (!Array.isArray(instances)) { + if (instances.length === 0 && data !== null && data !== undefined) { this.logger.warn("Multi-component data is not an array", { fieldName }); return; } diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index de3d3331d..1d2a89a38 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -377,6 +377,126 @@ describe.each(getConfiguredTestDialects())( * parent collection here is NOT localized, which is also the case that reaches this guard * without passing through the collection or single ones. */ +/** + * A dynamic zone defaults to `repeatable: false`, and then its payload is a single OBJECT rather + * than an array. The pre-transaction check and the write must agree about that, or the check + * silently covers nothing: reading the object as "no instances" leaves the slug out of the + * presence map, and the write goes looking for the companion table from inside the transaction — + * which is the failure the check exists to prevent, and on PostgreSQL aborts the transaction. + * + * The refusal must therefore still be the actionable 409, not a driver error. + */ +describe.each(getConfiguredTestDialects())( + "non-repeatable dynamic zone with no companion on %s (integration)", + dialect => { + it("refuses with a 409 rather than a database failure", async () => { + current = await createTestNextly({ + dialect, + fieldGroups: [ + defineFieldGroup({ + slug: "znsingle", + localized: true, + fields: [text({ name: "heading", localized: true })], + }), + ], + collections: [ + defineCollection({ + slug: "i18nwin_single", + fields: [ + text({ name: "title" }), + // No `repeatable`, so the payload below is one object, not an array. + fieldGroup({ name: "hero", components: ["znsingle"] }), + ], + }), + ], + localization, + }); + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`comp_znsingle_locales`" : '"comp_znsingle_locales"'}` + ); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_single", + overrideAccess: true, + locale: "es", + }, + { + title: "Page", + hero: { _componentType: "znsingle", heading: "Hola" }, + } + ); + + expect(created.success).toBe(false); + expect(created.statusCode).toBe(409); + expect(created.message).toMatch(/Translations are not ready/); + }); + } +); + +/** + * The guard exists to protect translatable values, so a payload that carries none of them has + * nothing to protect. Editing only a shared field while the companion is missing touches neither + * the absent table nor the default language, and refusing it would block a perfectly safe write — + * an editor changing one non-translatable field in a non-default locale would simply be stuck. + */ +describe.each(getConfiguredTestDialects())( + "shared-only write with no companion on %s (integration)", + dialect => { + it("permits a non-default-locale edit that touches no translatable field", async () => { + current = await createTestNextly({ + dialect, + collections: [mixedPosts(true)], + localization, + }); + + const created = await current + .getService("collectionsHandler") + .createEntry( + { + collectionName: "i18nwin_mixed", + overrideAccess: true, + locale: "en", + }, + { title: "Original", author: "Ada" } + ); + expect(created.success).toBe(true); + const id = (created.data as { id: string }).id; + + await current.adapter.executeQuery( + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_mixed_locales`" : '"dc_i18nwin_mixed_locales"'}` + ); + + // `author` is the only shared field; `title` is companion-owned and absent here. + const updated = await current + .getService("collectionsHandler") + .updateEntry( + { + collectionName: "i18nwin_mixed", + entryId: id, + overrideAccess: true, + locale: "es", + }, + { author: "Grace" } + ); + + expect( + updated.success + ? "ok" + : `refused ${updated.statusCode}: ${updated.message}` + ).toBe("ok"); + + const rows = await current.adapter.executeQuery<{ author: string }>( + `SELECT author FROM ${dialect === "mysql" ? "`dc_i18nwin_mixed`" : '"dc_i18nwin_mixed"'}` + ); + expect(rows[0]?.author).toBe("Grace"); + }); + } +); + describe("dynamic zone whose unused field group has no companion (integration)", () => { it("saves a block type whose companion exists while another permitted type's is missing", async () => { current = await createTestNextly({ diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 96120550e..cfa663e9b 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -185,30 +185,6 @@ export interface CompanionIntrospectAdapter extends CompanionWriteAdapter { getDrizzle(): T; } -/** - * Whether the main table still physically carries `columnName`. - * - * This answers one question for every entity type: **can the pre-companion fallback actually - * persist anything?** While the companion is missing, a write in the default language is meant - * to stay on the main table — but that is only true for an entity whose columns are still - * there. An entity localized from creation (or one whose migration has run) keeps them only on - * the companion, and its registered runtime table omits them, so the write carries keys the - * table has no columns for. Measured on all three dialects, that surfaces as a driver error - * and a 500 rather than a wrong value — the write does not quietly commit. Answering the - * question up front turns that opaque failure into a refusal the caller can act on. - * - * Goes through the same introspection the schema pipeline uses rather than a `SELECT ... LIMIT 0` - * probe. That matters beyond convention: a probe cannot tell "this column does not exist" from - * "the database is unreachable", so a transient failure would read as a missing column and - * produce a misleading "translations are not ready" refusal instead of the real error. - * Introspection fails loudly, and this deliberately does not catch. - * - * MUST be called before the caller opens its transaction. It borrows a connection from the pool, - * so running it inside one waits for a connection that cannot be released until that transaction - * finishes — starvation on a small pool. Resolving it first also keeps a refusal exactly as - * raised: errors leaving a transaction callback pass through the adapter's error classification, - * which rewraps anything that is not already a `DatabaseError`. - */ /** * Add localized columns an EXISTING companion is missing. No-op when the companion is absent — * creating it is {@link ensureCompanionTable}'s job. @@ -260,9 +236,20 @@ export async function reconcileCompanionColumns( .find(t => t.name === companionTableName) ?.columns.map(c => c.name) ?? [] ); + // `_status` is tracked alongside the translatable columns: switching Draft/Published on + // AFTER the companion was created leaves it without the column, and later per-locale + // status writes then target something that is not there. + const hasStatus = present.has("_status"); + const wantStatus = args.status === true; + // Nothing missing — the overwhelmingly common case, and worth leaving before the - // statement builder runs. - if (desired.every(f => present.has(toColumn(f.name)))) return; + // statement builder runs. `_status` only counts as missing when it is wanted and absent; + // wanted-and-present and unwanted are both already in step. + if ( + desired.every(f => present.has(toColumn(f.name))) && + !(wantStatus && !hasStatus) + ) + return; // Feed the canonical builder the columns that already exist as `oldLocalized`, so what it // emits is exactly the difference. `old` is a subset of `new` by construction here, which @@ -276,7 +263,13 @@ export async function reconcileCompanionColumns( oldLocalized: desired.filter(f => present.has(toColumn(f.name))), newLocalized: desired, dialect: args.dialect, - status: args.status === true, + // Report status as wanted whenever the column is already there, so the builder can ADD a + // missing `_status` but never DROP an existing one. Draft/Published being switched OFF + // must not delete per-locale status from an unattended sync: `db:sync` persists registry + // metadata BEFORE its destructive prompt, so the drop would run even for an operator who + // went on to decline it. Removing it stays the gated pipeline's job. + status: wantStatus || hasStatus, + companionHasStatus: hasStatus, companionExists: true, }); for (const stmt of statements) { @@ -287,6 +280,30 @@ export async function reconcileCompanionColumns( } } +/** + * Whether the main table still physically carries `columnName`. + * + * This answers one question for every entity type: **can the pre-companion fallback actually + * persist anything?** While the companion is missing, a write in the default language is meant + * to stay on the main table — but that is only true for an entity whose columns are still + * there. An entity localized from creation (or one whose migration has run) keeps them only on + * the companion, and its registered runtime table omits them, so the write carries keys the + * table has no columns for. Measured on all three dialects, that surfaces as a driver error + * and a 500 rather than a wrong value — the write does not quietly commit. Answering the + * question up front turns that opaque failure into a refusal the caller can act on. + * + * Goes through the same introspection the schema pipeline uses rather than a `SELECT ... LIMIT 0` + * probe. That matters beyond convention: a probe cannot tell "this column does not exist" from + * "the database is unreachable", so a transient failure would read as a missing column and + * produce a misleading "translations are not ready" refusal instead of the real error. + * Introspection fails loudly, and this deliberately does not catch. + * + * MUST be called before the caller opens its transaction. It borrows a connection from the pool, + * so running it inside one waits for a connection that cannot be released until that transaction + * finishes — starvation on a small pool. Resolving it first also keeps a refusal exactly as + * raised: errors leaving a transaction callback pass through the adapter's error classification, + * which rewraps anything that is not already a `DatabaseError`. + */ export async function mainTableHasColumn( adapter: CompanionIntrospectAdapter, tableName: string, diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 7c1f30127..e71f52cad 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -885,7 +885,22 @@ export class SingleMutationService extends BaseService { // exactly as raised: errors leaving a transaction callback pass through the // adapter's error classification, which rewraps anything that is not already a // `DatabaseError`. - if (companion && !companionPhysicallyExists && this.localization) { + // A payload carrying nothing companion-owned has no stake in the missing table: no + // translatable value to strand, none to overwrite. Refusing a shared-only edit would + // block a write that is entirely safe, so it is allowed through to the main table. + // Membership goes through the canonical split, which accepts either the camelCase field + // name or the snake_case companion column. + const payloadTouchesCompanion = + companion !== null && + Object.keys( + splitLocalizedWrite(data, companion.localizedFields).companion + ).length > 0; + if ( + companion && + !companionPhysicallyExists && + this.localization && + payloadTouchesCompanion + ) { // Captured so the closure below keeps the narrowed type. const defaultLocale = this.localization.defaultLocale; const refuse = (): never => { diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 93896b66b..2d4a6d0f8 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -591,7 +591,7 @@ async function ensureLocalizedCompanionsForReload( // Same policy the CLI applies: production schema changes belong to `nextly migrate`. if (process.env.NODE_ENV === "production") return; - const { ensureCompanionTable } = await import( + const { ensureCompanionTable, reconcileCompanionColumns } = await import( "../domains/i18n/runtime/companion-io" ); const { resolveCollectionTableName, resolveComponentTableName } = @@ -648,6 +648,32 @@ async function ensureLocalizedCompanionsForReload( ); } ); + // Creating the table is not enough on its own: `ensureCompanionTable` returns + // immediately when one already exists, so marking a FURTHER field localized on an + // already-localized entity takes the no-DDL path, syncs its metadata, and leaves the + // companion a column short — the write then splits that value into a column that is not + // there. The CLI sync reconciles for the same reason; the HMR path needs it too. + // + // Safe here despite issuing DDL, because the production guard at the top of this + // function has already returned: this runs only under `next dev`. The reconcile is + // additive, so it never removes a column even when a field stops being localized. + await reconcileCompanionColumns( + adapter, + { + slug: entity.slug, + tableName: resolveTableName(entity), + fields: entity.fields ?? [], + dialect: adapter.dialect, + status: entity.status === true, + }, + error => { + console.warn( + `[nextly] Could not update the translations table for "${entity.slug}". ` + + `Newly translatable fields may fail to save until it is in step: ` + + `${error instanceof Error ? error.message : String(error)}` + ); + } + ); } } } From f149baafc09af66f8a646d8a09b5509030663707 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 30 Jul 2026 03:35:17 +0500 Subject: [PATCH 18/20] fix(nextly): back out the shared-only bypass and backfill localized status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deciding per write whether a payload "touches the companion" turned out to need more than the payload. Three ways it was wrong, each a way to lose content: - Hooks run after the decision. A field-level `beforeChange` that adds a localized value to an otherwise shared-only update is persisted from the post-hook payload, so a non-default-locale write reached the main table and overwrote the default locale's values. - `_status` is companion-owned per locale. A status-only PATCH therefore looked shared-only, and the status landed on the main row — unpublishing every locale at once on the collection path. - The field-group preflight passes `{}` to reach its existence decision, so the bypass never applied there and embedded groups kept refusing anyway. The refusal is unconditional again. What it costs is an editor being unable to change a shared field while the companion is missing, which is a transient state that `db:sync` or a reload resolves; what it buys back is three paths that could destroy or hide content. The optimization needs the recorded transition state to be done safely, so it waits for that rather than being inferred per write. Companion reconciliation now passes `defaultLocale`, which is what lets the builder emit its default-locale status backfill. ADD COLUMN seeds every existing companion row at 'draft' including the default-locale row, but that row's status IS the main row's and may already be 'published'. Without the backfill, enabling Draft/Published on an entity that already has content made all of it read as draft and drop out of published localized reads until each row was republished by hand. Also corrects documentation that still claimed the reload path seeds companions. It creates and reconciles; existing content stays where it is, so a successful reload is not evidence that default-locale data was carried across. --- packages/nextly/src/cli/commands/dev-build.ts | 8 ++- .../services/collection-mutation-service.ts | 16 ----- .../services/field-group-mutation-service.ts | 21 ------ ...rite-without-companion.integration.test.ts | 70 ++++++++----------- .../src/domains/i18n/runtime/companion-io.ts | 14 ++++ .../services/single-mutation-service.ts | 17 +---- packages/nextly/src/init/reload-config.ts | 19 +++-- 7 files changed, 66 insertions(+), 99 deletions(-) diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index 5ce17325f..bb3d8611e 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -879,8 +879,8 @@ export async function ensureLocalizedCompanions( // entity that is already localized leaves the companion a column short — while the // main-table sync above has already added that column to `comp_*` / `dc_*`. The write // then splits the value into a companion column that does not exist. Additive only, - // and confined to `db:sync`: the reload path stays creation-only, because a running - // deployment must not alter its schema off a config edit. + // Additive only. The reload path reconciles too — it is dev-only, behind its own + // production return — so the two stay in step. await reconcileCompanionColumns( adapter as unknown as DrizzleAdapter, { @@ -889,6 +889,10 @@ export async function ensureLocalizedCompanions( fields: entity.fields ?? [], dialect, status: entity.status === true, + // Lets the reconcile backfill the default-locale row's status from the main row + // when `_status` has to be added; without it, already-published content reads as + // draft and drops out of published localized reads after a sync. + defaultLocale: config.localization?.defaultLocale, }, error => { logger.error( diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 1c1b413ac..1b7339775 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -92,7 +92,6 @@ import { import { companionTableExists as sharedCompanionTableExists, mainTableHasColumn, - splitLocalizedWrite, } from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; @@ -966,21 +965,6 @@ export class CollectionMutationService extends BaseService { // its own process while the running server has yet to create the companion. // Refuse either way; the default language still writes to main, which is // the documented pre-migration fallback. - // Before refusing anything: a payload carrying nothing companion-owned has no stake - // in the missing table. There is no translatable value to strand and none to - // overwrite, so a shared-only edit — a PATCH of one non-translatable field, say — is - // entirely safe and must not be blocked. Both refusals below are about protecting - // translatable values, and this payload has none. - // - // Membership goes through the canonical split rather than a hand-rolled key check, - // because it accepts either the camelCase field name or the snake_case companion - // column, and collection writes arrive already converted to snake_case. - const { companion: localizedInPayload } = splitLocalizedWrite( - entryData, - companion.localizedFields - ); - if (Object.keys(localizedInPayload).length === 0) return null; - const requested = resolveRequestedLocale(this.localization, locale); if (requested !== this.localization.defaultLocale) { throw NextlyError.conflict({ diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index f6cd20ea4..ab5cafa6c 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -203,27 +203,6 @@ export class FieldGroupMutationService extends BaseService { ? (tx.presence.get(meta.slug) ?? false) : await companionTableExists(this.adapter, schema.companionTableName); if (!companionExists) { - // A payload carrying nothing companion-owned has no stake in the missing table: no - // translatable value to strand, none to overwrite. A shared-only edit is therefore safe - // and must not be refused. Note this is evaluated on the REAL payload, so the - // pre-transaction pass — which calls with `{}` purely to reach the existence decision — - // is unaffected and still records presence for every slug the write touches. - const { companion: localizedInPayload } = splitLocalizedWrite( - data, - schema.localizedFields - ); - if ( - Object.keys(data).length > 0 && - Object.keys(localizedInPayload).length === 0 - ) { - return { - schema: null, - main: data, - companion: {}, - companionExists: false, - }; - } - const writeLocale = resolveRequestedLocale(this.localization, locale); if (writeLocale !== this.localization.defaultLocale) { throw NextlyError.conflict({ diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 1d2a89a38..5ee90be7e 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -437,66 +437,56 @@ describe.each(getConfiguredTestDialects())( } ); -/** - * The guard exists to protect translatable values, so a payload that carries none of them has - * nothing to protect. Editing only a shared field while the companion is missing touches neither - * the absent table nor the default language, and refusing it would block a perfectly safe write — - * an editor changing one non-translatable field in a non-default locale would simply be stuck. - */ describe.each(getConfiguredTestDialects())( - "shared-only write with no companion on %s (integration)", + "non-repeatable dynamic zone with no companion on %s (integration)", dialect => { - it("permits a non-default-locale edit that touches no translatable field", async () => { + it("refuses with a 409 rather than a database failure", async () => { current = await createTestNextly({ dialect, - collections: [mixedPosts(true)], + fieldGroups: [ + defineFieldGroup({ + slug: "znsingle", + localized: true, + fields: [text({ name: "heading", localized: true })], + }), + ], + collections: [ + defineCollection({ + slug: "i18nwin_single", + fields: [ + text({ name: "title" }), + // No `repeatable`, so the payload below is one object, not an array. + fieldGroup({ name: "hero", components: ["znsingle"] }), + ], + }), + ], localization, }); - const created = await current - .getService("collectionsHandler") - .createEntry( - { - collectionName: "i18nwin_mixed", - overrideAccess: true, - locale: "en", - }, - { title: "Original", author: "Ada" } - ); - expect(created.success).toBe(true); - const id = (created.data as { id: string }).id; - await current.adapter.executeQuery( - `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`dc_i18nwin_mixed_locales`" : '"dc_i18nwin_mixed_locales"'}` + `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`comp_znsingle_locales`" : '"comp_znsingle_locales"'}` ); - // `author` is the only shared field; `title` is companion-owned and absent here. - const updated = await current + const created = await current .getService("collectionsHandler") - .updateEntry( + .createEntry( { - collectionName: "i18nwin_mixed", - entryId: id, + collectionName: "i18nwin_single", overrideAccess: true, locale: "es", }, - { author: "Grace" } + { + title: "Page", + hero: { _componentType: "znsingle", heading: "Hola" }, + } ); - expect( - updated.success - ? "ok" - : `refused ${updated.statusCode}: ${updated.message}` - ).toBe("ok"); - - const rows = await current.adapter.executeQuery<{ author: string }>( - `SELECT author FROM ${dialect === "mysql" ? "`dc_i18nwin_mixed`" : '"dc_i18nwin_mixed"'}` - ); - expect(rows[0]?.author).toBe("Grace"); + expect(created.success).toBe(false); + expect(created.statusCode).toBe(409); + expect(created.message).toMatch(/Translations are not ready/); }); } ); - describe("dynamic zone whose unused field group has no companion (integration)", () => { it("saves a block type whose companion exists while another permitted type's is missing", async () => { current = await createTestNextly({ diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index cfa663e9b..ccef75c13 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -210,6 +210,13 @@ export async function reconcileCompanionColumns( fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; + /** + * The configured default locale. Needed only when `_status` has to be ADDed: the builder + * backfills the default-locale companion row's status from the main row, which is the row + * whose status it actually is. Omitting it leaves already-published content reading as + * draft. + */ + defaultLocale?: string; }, onError?: (error: unknown) => void ): Promise { @@ -270,6 +277,13 @@ export async function reconcileCompanionColumns( // went on to decline it. Removing it stays the gated pipeline's job. status: wantStatus || hasStatus, companionHasStatus: hasStatus, + // Required for the builder to emit its default-locale status backfill. ADD COLUMN seeds + // every existing companion row at 'draft', including the default-locale row — but the + // default locale's status IS the main row's, which may already be 'published'. Without + // this, enabling Draft/Published on an entity that already has content makes all of it + // read as draft and vanish from published localized reads until each row is republished + // by hand. + defaultLocale: args.defaultLocale, companionExists: true, }); for (const stmt of statements) { diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index e71f52cad..7c1f30127 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -885,22 +885,7 @@ export class SingleMutationService extends BaseService { // exactly as raised: errors leaving a transaction callback pass through the // adapter's error classification, which rewraps anything that is not already a // `DatabaseError`. - // A payload carrying nothing companion-owned has no stake in the missing table: no - // translatable value to strand, none to overwrite. Refusing a shared-only edit would - // block a write that is entirely safe, so it is allowed through to the main table. - // Membership goes through the canonical split, which accepts either the camelCase field - // name or the snake_case companion column. - const payloadTouchesCompanion = - companion !== null && - Object.keys( - splitLocalizedWrite(data, companion.localizedFields).companion - ).length > 0; - if ( - companion && - !companionPhysicallyExists && - this.localization && - payloadTouchesCompanion - ) { + if (companion && !companionPhysicallyExists && this.localization) { // Captured so the closure below keeps the narrowed type. const defaultLocale = this.localization.defaultLocale; const refuse = (): never => { diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 2d4a6d0f8..4ed984d10 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -99,8 +99,9 @@ type LoggerLike = { interface AdapterLike { readonly dialect: "postgresql" | "mysql" | "sqlite"; getDrizzle(): T; - // Needed to provision the localized companion below: creating it is DDL, and - // seeding it from the main table is a write. + // Needed to provision the localized companion below: creating the table and adding + // columns to it are both DDL, and the status backfill that accompanies a new `_status` + // column is a write. executeQuery(sql: string, params?: unknown[]): Promise; } @@ -556,8 +557,14 @@ function republishRecordingPolicies( } /** - * Create and seed the `_locales` companion of every localized collection, single and field group - * in the reloaded config. + * Create, and bring into step, the `_locales` companion of every localized collection, single + * and field group in the reloaded config. + * + * It does NOT seed existing content into the companion. `ensureCompanionTable` is + * creation-only and leaves whatever is already on the main table where it is, so a successful + * reload is not evidence that default-locale data has been carried across — enabling + * localization on an entity that already has content still leaves that content unreadable + * until the transition seeds it. Copying it is the gated pipeline's job. * * The reload path is the `next dev` counterpart to the CLI's `ensureLocalizedCompanions`: it is * where a config edit lands when the app is running under plain `next dev` rather than @@ -665,6 +672,10 @@ async function ensureLocalizedCompanionsForReload( fields: entity.fields ?? [], dialect: adapter.dialect, status: entity.status === true, + // Lets the reconcile backfill the default-locale row's status from the main row + // when `_status` has to be added; without it, already-published content reads as + // draft and drops out of published localized reads. + defaultLocale: config.localization?.defaultLocale, }, error => { console.warn( From c13ec6116f9844cbee8032daec38d98e03183244 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 30 Jul 2026 09:53:56 +0500 Subject: [PATCH 19/20] fix(nextly): drop the status reconcile, which cannot be made retryable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adding `_status` to an existing companion is one statement and backfilling the default-locale row from the main row is another, and physical shape cannot tell the two apart afterwards. When the ADD lands and the backfill does not, every later run sees the column present, concludes the companion is in step, and returns — leaving previously published content reading as draft while reporting success. MySQL commits DDL implicitly, so the pair cannot be made atomic there either. Deciding this correctly needs a record of whether the backfill has run, which is state the system does not keep yet. Until it does, switching Draft/Published on for an already-localized entity belongs to the migration path: that fails loudly on a missing column, which is a better outcome than silently hiding published rows. The localized-column reconcile stays, because it has the opposite property. A missing column is visible on every run, so a partial apply simply completes on the next one. Retryability is the line between the two, and it is now stated where the decision is made. --- packages/nextly/src/cli/commands/dev-build.ts | 4 -- .../src/domains/i18n/runtime/companion-io.ts | 52 +++++++------------ packages/nextly/src/init/reload-config.ts | 4 -- 3 files changed, 20 insertions(+), 40 deletions(-) diff --git a/packages/nextly/src/cli/commands/dev-build.ts b/packages/nextly/src/cli/commands/dev-build.ts index bb3d8611e..c139efd1e 100644 --- a/packages/nextly/src/cli/commands/dev-build.ts +++ b/packages/nextly/src/cli/commands/dev-build.ts @@ -889,10 +889,6 @@ export async function ensureLocalizedCompanions( fields: entity.fields ?? [], dialect, status: entity.status === true, - // Lets the reconcile backfill the default-locale row's status from the main row - // when `_status` has to be added; without it, already-published content reads as - // draft and drops out of published localized reads after a sync. - defaultLocale: config.localization?.defaultLocale, }, error => { logger.error( diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index ccef75c13..63d51b057 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -210,13 +210,6 @@ export async function reconcileCompanionColumns( fields: CompanionFieldLike[]; dialect: SupportedDialect; status?: boolean; - /** - * The configured default locale. Needed only when `_status` has to be ADDed: the builder - * backfills the default-locale companion row's status from the main row, which is the row - * whose status it actually is. Omitting it leaves already-published content reading as - * draft. - */ - defaultLocale?: string; }, onError?: (error: unknown) => void ): Promise { @@ -243,20 +236,25 @@ export async function reconcileCompanionColumns( .find(t => t.name === companionTableName) ?.columns.map(c => c.name) ?? [] ); - // `_status` is tracked alongside the translatable columns: switching Draft/Published on - // AFTER the companion was created leaves it without the column, and later per-locale - // status writes then target something that is not there. + // `_status` is deliberately NOT reconciled here. Adding the column is one statement and + // backfilling the default-locale row from the main row is another, and the pair cannot be + // made retryable from physical shape alone: when the ADD lands and the backfill does not, + // every later run sees the column present, concludes the companion is in step, and leaves + // previously published content reading as draft while reporting success. MySQL commits DDL + // implicitly, so the two cannot be made atomic there either. + // + // Deciding it correctly needs a record of whether the backfill has run — the localization + // transition state this codebase does not keep yet. Until it does, switching Draft/Published + // on for an already-localized entity belongs to the migration path, which fails loudly on a + // missing column rather than silently hiding published rows. + // + // The localized-column reconcile below has no such weakness: a missing column is visible on + // every run, so a partial apply simply finishes on the next one. const hasStatus = present.has("_status"); - const wantStatus = args.status === true; - // Nothing missing — the overwhelmingly common case, and worth leaving before the - // statement builder runs. `_status` only counts as missing when it is wanted and absent; - // wanted-and-present and unwanted are both already in step. - if ( - desired.every(f => present.has(toColumn(f.name))) && - !(wantStatus && !hasStatus) - ) - return; + // Nothing missing — the overwhelmingly common case, and worth leaving before the statement + // builder runs. + if (desired.every(f => present.has(toColumn(f.name)))) return; // Feed the canonical builder the columns that already exist as `oldLocalized`, so what it // emits is exactly the difference. `old` is a subset of `new` by construction here, which @@ -270,20 +268,10 @@ export async function reconcileCompanionColumns( oldLocalized: desired.filter(f => present.has(toColumn(f.name))), newLocalized: desired, dialect: args.dialect, - // Report status as wanted whenever the column is already there, so the builder can ADD a - // missing `_status` but never DROP an existing one. Draft/Published being switched OFF - // must not delete per-locale status from an unattended sync: `db:sync` persists registry - // metadata BEFORE its destructive prompt, so the drop would run even for an operator who - // went on to decline it. Removing it stays the gated pipeline's job. - status: wantStatus || hasStatus, + // Report the companion's ACTUAL status shape on both sides, so the builder sees no status + // change and emits only the column difference — never an ADD or DROP of `_status`. + status: hasStatus, companionHasStatus: hasStatus, - // Required for the builder to emit its default-locale status backfill. ADD COLUMN seeds - // every existing companion row at 'draft', including the default-locale row — but the - // default locale's status IS the main row's, which may already be 'published'. Without - // this, enabling Draft/Published on an entity that already has content makes all of it - // read as draft and vanish from published localized reads until each row is republished - // by hand. - defaultLocale: args.defaultLocale, companionExists: true, }); for (const stmt of statements) { diff --git a/packages/nextly/src/init/reload-config.ts b/packages/nextly/src/init/reload-config.ts index 4ed984d10..0868f7fe4 100644 --- a/packages/nextly/src/init/reload-config.ts +++ b/packages/nextly/src/init/reload-config.ts @@ -672,10 +672,6 @@ async function ensureLocalizedCompanionsForReload( fields: entity.fields ?? [], dialect: adapter.dialect, status: entity.status === true, - // Lets the reconcile backfill the default-locale row's status from the main row - // when `_status` has to be added; without it, already-published content reads as - // draft and drops out of published localized reads. - defaultLocale: config.localization?.defaultLocale, }, error => { console.warn( From e33135fb1d45b68eb8bcb0ff7291e5e408511e99 Mon Sep 17 00:00:00 2001 From: Mobeen Abdullah Date: Thu, 30 Jul 2026 10:40:37 +0500 Subject: [PATCH 20/20] fix(nextly): check every localized column and survive concurrent provisioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fallback check proved one column existed and inferred the rest. A partially migrated main table — an older field keeping its legacy column while a newer localized field never had one — passed on the first column and then failed at the driver on a later one, which is the opaque error this check exists to replace. `mainTableHasColumns` now requires all of them, at all three call sites. Provisioning tolerates losing a race. `db:sync` and a dev boot or HMR reload provision the same companions, and neither `CREATE TABLE` nor `ADD COLUMN` is idempotent here, so whichever process arrived second failed the whole sync even though the table it wanted now existed. Both paths re-read the schema after a failure and treat the wanted shape being present as success, whoever produced it. Decided by re-introspecting rather than by matching driver error text, so a real failure that happens to mention the table is not swallowed. Enabling Draft/Published on a companion that predates it no longer reports success. Reconciling `_status` is unsafe here — the ADD and the default-locale back-fill cannot be retried as a pair — but returning quietly was worse: the caller persisted `status: true` and every later per-locale status read hit a column that was not there. It now reports through the error channel, so the sync exits non-zero and names `nextly migrate`. Only that direction; status switched off with the column still present is harmless. Also removes a duplicated test block that ran the non-repeatable dynamic-zone case twice per dialect. --- .../services/collection-mutation-service.ts | 6 +- .../services/field-group-mutation-service.ts | 6 +- ...rite-without-companion.integration.test.ts | 50 ------------- .../src/domains/i18n/runtime/companion-io.ts | 75 ++++++++++++++++--- .../services/single-mutation-service.ts | 6 +- 5 files changed, 74 insertions(+), 69 deletions(-) diff --git a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts index 1b7339775..9967900fc 100644 --- a/packages/nextly/src/domains/collections/services/collection-mutation-service.ts +++ b/packages/nextly/src/domains/collections/services/collection-mutation-service.ts @@ -91,7 +91,7 @@ import { } from "../../i18n/resolve-locale"; import { companionTableExists as sharedCompanionTableExists, - mainTableHasColumn, + mainTableHasColumns, } from "../../i18n/runtime/companion-io"; import { assembleDocument } from "../../versions/assemble-document"; import { captureInTx } from "../../versions/capture-in-tx"; @@ -987,11 +987,11 @@ export class CollectionMutationService extends BaseService { // that has no columns for them. That write cannot land: it reaches the driver // and fails as a 500. Refusing here says the same thing in terms the caller can // act on, and says it before anything is attempted. - const fallbackPossible = await mainTableHasColumn( + const fallbackPossible = await mainTableHasColumns( this.adapter, // The companion is always `
_locales`, so the main table is its stem. companion.companionTableName.replace(/_locales$/, ""), - companion.localizedFields[0]?.column + companion.localizedFields.map(f => f.column) ); if (!fallbackPossible) { throw NextlyError.conflict({ diff --git a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts index ab5cafa6c..8bcc9ce6a 100644 --- a/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts +++ b/packages/nextly/src/domains/field-groups/services/field-group-mutation-service.ts @@ -29,7 +29,7 @@ import { resolveRequestedLocale } from "../../i18n/resolve-locale"; import { buildCompanionSchema, companionTableExists, - mainTableHasColumn, + mainTableHasColumns, splitLocalizedWrite, upsertCompanionRow, } from "../../i18n/runtime/companion-io"; @@ -228,10 +228,10 @@ export class FieldGroupMutationService extends BaseService { // the same question before that transaction opened. const fallbackPossible = tx !== undefined || - (await mainTableHasColumn( + (await mainTableHasColumns( this.adapter, meta.tableName, - schema.localizedFields[0]?.column + schema.localizedFields.map(f => f.column) )); if (!fallbackPossible) { throw NextlyError.conflict({ diff --git a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts index 5ee90be7e..02764f179 100644 --- a/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts +++ b/packages/nextly/src/domains/i18n/__tests__/localized-write-without-companion.integration.test.ts @@ -437,56 +437,6 @@ describe.each(getConfiguredTestDialects())( } ); -describe.each(getConfiguredTestDialects())( - "non-repeatable dynamic zone with no companion on %s (integration)", - dialect => { - it("refuses with a 409 rather than a database failure", async () => { - current = await createTestNextly({ - dialect, - fieldGroups: [ - defineFieldGroup({ - slug: "znsingle", - localized: true, - fields: [text({ name: "heading", localized: true })], - }), - ], - collections: [ - defineCollection({ - slug: "i18nwin_single", - fields: [ - text({ name: "title" }), - // No `repeatable`, so the payload below is one object, not an array. - fieldGroup({ name: "hero", components: ["znsingle"] }), - ], - }), - ], - localization, - }); - - await current.adapter.executeQuery( - `DROP TABLE IF EXISTS ${dialect === "mysql" ? "`comp_znsingle_locales`" : '"comp_znsingle_locales"'}` - ); - - const created = await current - .getService("collectionsHandler") - .createEntry( - { - collectionName: "i18nwin_single", - overrideAccess: true, - locale: "es", - }, - { - title: "Page", - hero: { _componentType: "znsingle", heading: "Hola" }, - } - ); - - expect(created.success).toBe(false); - expect(created.statusCode).toBe(409); - expect(created.message).toMatch(/Translations are not ready/); - }); - } -); describe("dynamic zone whose unused field group has no companion (integration)", () => { it("saves a block type whose companion exists while another permitted type's is missing", async () => { current = await createTestNextly({ diff --git a/packages/nextly/src/domains/i18n/runtime/companion-io.ts b/packages/nextly/src/domains/i18n/runtime/companion-io.ts index 63d51b057..4f24a6614 100644 --- a/packages/nextly/src/domains/i18n/runtime/companion-io.ts +++ b/packages/nextly/src/domains/i18n/runtime/companion-io.ts @@ -214,13 +214,11 @@ export async function reconcileCompanionColumns( onError?: (error: unknown) => void ): Promise { const companionTableName = `${args.tableName}_locales`; + // Hoisted so the concurrency recheck in the catch can reuse it. + const localizedNames = new Set(resolveLocalizedFieldNames(args.fields, true)); + const desired = args.fields.filter(f => localizedNames.has(f.name)); try { if (!(await companionTableExists(adapter, companionTableName))) return; - - const localizedNames = new Set( - resolveLocalizedFieldNames(args.fields, true) - ); - const desired = args.fields.filter(f => localizedNames.has(f.name)); if (desired.length === 0) return; const { introspectLiveSnapshot } = await import( @@ -252,6 +250,26 @@ export async function reconcileCompanionColumns( // every run, so a partial apply simply finishes on the next one. const hasStatus = present.has("_status"); + // Draft/Published was switched on after this companion was created, so `_status` is now + // required and absent. Reconciling it is unsafe for the reasons above — but returning + // quietly is worse than either: the caller persists `status: true`, reports success, and + // every later per-locale status read or write hits a column that is not there. Report it + // so `db:sync` exits non-zero and the operator learns now rather than at the next publish. + // + // Only this direction is a problem. Status switched OFF while the column remains is + // harmless: the column is simply unused, and the additive policy keeps it. + if (args.status === true && !hasStatus) { + onError?.( + new Error( + `The translations table ${companionTableName} predates Draft/Published being enabled ` + + `for "${args.slug}", so it has no _status column. Run \`nextly migrate\` to add it: ` + + `an unattended sync cannot, because adding the column and back-filling the ` + + `default locale's status cannot be retried safely if only the first half lands.` + ) + ); + return; + } + // Nothing missing — the overwhelmingly common case, and worth leaving before the statement // builder runs. if (desired.every(f => present.has(toColumn(f.name)))) return; @@ -278,12 +296,32 @@ export async function reconcileCompanionColumns( await adapter.executeQuery(stmt); } } catch (error) { + // Same check-then-act window as the create path: a concurrent sync or reload may have added + // the very columns this run was adding, and `ADD COLUMN` is not idempotent. Re-introspect + // and treat "the shape we wanted is now present" as success, whoever produced it. + try { + const { introspectLiveSnapshot: reread } = await import( + "../../schema/pipeline/diff/introspect-live" + ); + const after = await reread(adapter.getDrizzle(), adapter.dialect, [ + companionTableName, + ]); + const now = new Set( + after.tables + .find(t => t.name === companionTableName) + ?.columns.map(c => c.name) ?? [] + ); + if (desired.every(f => now.has(toColumn(f.name)))) return; + } catch { + // Fall through to reporting the original error: if we cannot even re-read the table, + // the reconcile genuinely did not succeed. + } onError?.(error); } } /** - * Whether the main table still physically carries `columnName`. + * Whether the main table still physically carries ALL of `columnNames`. * * This answers one question for every entity type: **can the pre-companion fallback actually * persist anything?** While the companion is missing, a write in the default language is meant @@ -306,12 +344,13 @@ export async function reconcileCompanionColumns( * raised: errors leaving a transaction callback pass through the adapter's error classification, * which rewraps anything that is not already a `DatabaseError`. */ -export async function mainTableHasColumn( +export async function mainTableHasColumns( adapter: CompanionIntrospectAdapter, tableName: string, - columnName: string | undefined + columnNames: readonly (string | undefined)[] ): Promise { - if (!columnName) return false; + const wanted = columnNames.filter((c): c is string => Boolean(c)); + if (wanted.length === 0) return false; const { introspectLiveSnapshot } = await import( "../../schema/pipeline/diff/introspect-live" ); @@ -321,7 +360,13 @@ export async function mainTableHasColumn( [tableName] ); const table = snapshot.tables.find(t => t.name === tableName); - return table?.columns.some(c => c.name === columnName) === true; + if (!table) return false; + const present = new Set(table.columns.map(c => c.name)); + // EVERY column, not just one. A partially migrated main table — an older field keeping its + // legacy column while a newer localized field never had one — would otherwise pass on the + // first column and then fail at the driver on a later one, which is the opaque 500 this + // check exists to replace with an actionable refusal. + return wanted.every(c => present.has(c)); } // Whether a probe error is a verified "this TABLE does not exist" for the @@ -439,6 +484,16 @@ export async function ensureCompanionTable( await adapter.executeQuery(stmt); } } catch (error) { + // Another process may have created it between the probe and the CREATE — `db:sync` and a + // dev boot/HMR reload provision the same companions, and `CREATE TABLE` is not idempotent + // here. Losing that race is a success: the table the caller wanted now exists. Confirmed by + // re-checking rather than by reading the error text, so this cannot swallow a real failure + // that happens to mention the table. + if ( + await companionTableExists(adapter, companionTableName).catch(() => false) + ) { + return; + } // Best-effort: the main table may not exist yet on a very first boot, where the // companion is created on the next boot (or by `nextly migrate`). That case is // expected and self-healing. Anything else is NOT — a persistent failure here diff --git a/packages/nextly/src/domains/singles/services/single-mutation-service.ts b/packages/nextly/src/domains/singles/services/single-mutation-service.ts index 7c1f30127..1e47a55bf 100644 --- a/packages/nextly/src/domains/singles/services/single-mutation-service.ts +++ b/packages/nextly/src/domains/singles/services/single-mutation-service.ts @@ -81,7 +81,7 @@ import { import { buildCompanionSchema, companionTableExists, - mainTableHasColumn, + mainTableHasColumns, splitLocalizedWrite, upsertCompanionRow, type CompanionSchema, @@ -905,10 +905,10 @@ export class SingleMutationService extends BaseService { if (writeLocale !== undefined && writeLocale !== defaultLocale) { refuse(); } - const fallbackPossible = await mainTableHasColumn( + const fallbackPossible = await mainTableHasColumns( this.adapter, singleMeta.tableName, - companion.localizedFields[0]?.column + companion.localizedFields.map(f => f.column) ); if (!fallbackPossible) refuse(); }