diff --git a/.changeset/declared-unique-index-not-legacy.md b/.changeset/declared-unique-index-not-legacy.md new file mode 100644 index 0000000000..a074022f56 --- /dev/null +++ b/.changeset/declared-unique-index-not-legacy.md @@ -0,0 +1,31 @@ +--- +"@objectstack/driver-sql": patch +--- + +fix(driver-sql): a currently-declared unique index is never legacy debt — index drift no longer ping-pongs (#3955) + +An object may declare both a tenant-scoped field-level `unique: true` and an +object-level single-column unique index on the same column: + +```ts +email: Field.email({ unique: true }), +indexes: [{ fields: ['email'], unique: true }], +``` + +The declared index materializes under `buildIndexName` as +`uniq__` — which is also one of the two spellings +`legacyUniqueIndexNames` looks for when hunting pre-#3696 platform-wide +uniques. The detector therefore read an index the current metadata declares +as legacy debt and proposed replacing it with the tenant composite (which +the same sync had already created). + +The resulting plan never converged: `apply` dropped the declared index, the +next `plan` reported it missing and recreated it, and the one after that +called it legacy again — an unbounded drop/create cycle on a live unique +index, every round rendered as a "safe" change. + +`legacyUniqueReplacements` now takes the object's `declaredIndexes` and +filters their normalized names out of the legacy candidate set, so an index +metadata declares today is never mistaken for debt. Genuinely legacy indexes +are still retired, including the knex-spelled `
__unique` when +only the `uniq_…` spelling is declared. diff --git a/.changeset/migrate-search-companion-parity.md b/.changeset/migrate-search-companion-parity.md new file mode 100644 index 0000000000..6e44384cb0 --- /dev/null +++ b/.changeset/migrate-search-companion-parity.md @@ -0,0 +1,31 @@ +--- +"@objectstack/types": patch +"@objectstack/runtime": patch +"@objectstack/cli": patch +--- + +fix(runtime,cli,types): `os migrate` and the dev runtime now share one `__search` companion schema view (#3955) + +On a zh-locale deployment the dev runtime provisions the hidden `__search` +pinyin companion column (ADR-0098) on every eligible object, but the +`os migrate plan`/`apply` boot went through `createStandaloneStack`, which +never derived the locale-gated pinyin decision from the compiled artifact. +Its metadata therefore lacked every companion column, and `migrate plan` +reported each live `__search` column of a dev-created database as a +destructive orphan — with `--allow-destructive` as the printed remediation, +which would have dropped live feature columns. + +- `@objectstack/types`: new `collectConfiguredLocales(i18n)` and + `stampSearchPinyinEnabled(i18n)` — the single resolve-and-stamp helper for + `OS_SEARCH_PINYIN_ENABLED`. An explicit env value still wins; only a + positive locale-derived decision is stamped. +- `@objectstack/runtime`: `createStandaloneStack` stamps the decision from + the artifact's `i18n` before any plugin constructs a `SchemaRegistry`, and + surfaces `i18n` on its result like `requires`/`objects`/`manifest`. +- `@objectstack/cli`: the `serve`/`dev` boot now stamps through the same + shared helper (behaviour unchanged), so create/serve and plan/apply cannot + compute different schema views of the same source tree. + +A fresh CLI-created database is now also born with the same `__search` +columns the dev runtime would provision, instead of acquiring them on the +next dev boot. diff --git a/docs/adr/0098-pinyin-search-companion-column.md b/docs/adr/0098-pinyin-search-companion-column.md index 65af56c943..353c7f43c6 100644 --- a/docs/adr/0098-pinyin-search-companion-column.md +++ b/docs/adr/0098-pinyin-search-companion-column.md @@ -31,13 +31,19 @@ time**. 1. **Locale-gated platform switch, no field metadata.** `OS_SEARCH_PINYIN_ENABLED` (resolved by `resolveSearchPinyinEnabled()` in - `@objectstack/types`) gates the feature end-to-end. When unset, the CLI - boot path derives the default from the stack's configured locales (any - `zh-*` → on) and stamps the decision back into the env var so every - consumer — the per-engine `SchemaRegistry` and the plugin gate — reads the - same single decision. No field-level `pinyin` marker exists, so there is - no declared-but-unenforced dead metadata (ADR-0049) and no half-state - where a field "pretends" to support pinyin. + `@objectstack/types`) gates the feature end-to-end. When unset, every boot + path that sees the stack config derives the default from its configured + locales (any `zh-*` → on) and stamps the decision back into the env var + (shared `stampSearchPinyinEnabled()` helper) so every consumer — the + per-engine `SchemaRegistry` and the plugin gate — reads the same single + decision. There are exactly two such paths: the CLI `serve`/`dev` boot + (from `objectstack.config.ts`) and `createStandaloneStack` (from the + compiled artifact's `i18n` — `os migrate plan`/`apply`, embedders). A path + that resolved without stamping would compute a schema view without the + companion columns; that is how `os migrate` once flagged live `__search` + columns as destructive orphans (#3955). No field-level `pinyin` marker + exists, so there is no declared-but-unenforced dead metadata (ADR-0049) + and no half-state where a field "pretends" to support pinyin. 2. **Materialization set ≠ search set: one column per object.** Only the ADR-0079 display/name field (`resolveDisplayField`) feeds the hidden diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index 2b0fd5c10d..58fa4b8345 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -9,7 +9,7 @@ import { bundleRequire } from 'bundle-require'; import { loadConfig, BUNDLE_REQUIRE_EXTERNALS } from '../utils/config.js'; import { isHostConfig, shouldBootWithLibrary } from '../utils/plugin-detection.js'; import { resolveDriverType, resolveStorageDefinition, UnsupportedDriverError } from '../utils/storage-driver.js'; -import { readEnvWithDeprecation, resolveMultiOrgEnabled, resolveTenancyPosture, resolveAllowDegradedTenancy, isMcpServerEnabled, resolveSearchPinyinEnabled, isModuleNotFoundError } from '@objectstack/types'; +import { readEnvWithDeprecation, resolveMultiOrgEnabled, resolveTenancyPosture, resolveAllowDegradedTenancy, isMcpServerEnabled, stampSearchPinyinEnabled, isModuleNotFoundError } from '@objectstack/types'; import { PLATFORM_CAPABILITY_TOKENS } from '@objectstack/spec/kernel'; import { missingProviderMessage } from '../utils/capability-preflight.js'; import { resolveObjectStackHome } from '@objectstack/runtime'; @@ -726,22 +726,16 @@ export default class Serve extends Command { } // Pinyin search recall (#2486): locale-gated platform capability. When // `OS_SEARCH_PINYIN_ENABLED` is unset, the default derives from the - // stack's configured locales (any `zh-*` → on). This is the ONE place - // that sees the stack config, so the resolved decision is stamped back - // into the env var — every later consumer (each engine's SchemaRegistry - // provisioning the `__search` companion column, the plugin's own gate) - // reads the same answer via the no-arg `resolveSearchPinyinEnabled()`. - { - const i18nCfg = (config as any).i18n ?? {}; - const configuredLocales = [ - i18nCfg.defaultLocale, - i18nCfg.fallbackLocale, - ...(Array.isArray(i18nCfg.supportedLocales) ? i18nCfg.supportedLocales : []), - ].filter((l: unknown): l is string => typeof l === 'string'); - if (resolveSearchPinyinEnabled({ locales: configuredLocales })) { - process.env.OS_SEARCH_PINYIN_ENABLED = 'true'; - if (!requires.includes('pinyin-search')) requires.push('pinyin-search'); - } + // stack's configured locales (any `zh-*` → on), and the resolved + // decision is stamped back into the env var — every later consumer + // (each engine's SchemaRegistry provisioning the `__search` companion + // column, the plugin's own gate) reads the same answer via the no-arg + // `resolveSearchPinyinEnabled()`. The shared `stampSearchPinyinEnabled` + // helper is also what `createStandaloneStack` stamps from the compiled + // artifact, so serve/dev and `os migrate plan`/`apply` cannot compute + // different schema views of the same source tree (#3955). + if (stampSearchPinyinEnabled((config as any).i18n)) { + if (!requires.includes('pinyin-search')) requires.push('pinyin-search'); } // Default capability slate — every preset except `minimal` gets the // foundational services (queue + job + cache + settings + email + diff --git a/packages/cli/src/utils/schema-migrate.integration.test.ts b/packages/cli/src/utils/schema-migrate.integration.test.ts index ccd7dfb4e2..ec1d92386d 100644 --- a/packages/cli/src/utils/schema-migrate.integration.test.ts +++ b/packages/cli/src/utils/schema-migrate.integration.test.ts @@ -117,3 +117,103 @@ describe('bootSchemaStack + migrate engine (integration)', () => { } }, 30_000); }); + +/** + * #3955 — `os migrate` and the dev runtime must compute ONE schema view. + * + * A zh-locale deployment's dev runtime provisions the hidden `__search` + * pinyin companion column (ADR-0098) on every eligible object. The migrate + * boot goes through `createStandaloneStack`, which — before the fix — never + * derived the locale-gated pinyin decision from the artifact, so its metadata + * lacked every companion column and `os migrate plan` reported each live + * `__search` column as a destructive orphan, with `--allow-destructive` as + * the printed remediation. Following that advice would have dropped live + * feature columns. + */ +describe('bootSchemaStack — dev-provisioned __search companions are not orphans (#3955)', () => { + let dir: string; + let dbFile: string; + const savedEnv: Record = {}; + + beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-mig-pinyin-')); + mkdirSync(join(dir, 'dist'), { recursive: true }); + mkdirSync(join(dir, 'data'), { recursive: true }); + dbFile = join(dir, 'data', 'app.db'); + + // Compiled-artifact stand-in for a Chinese deployment: the i18n block is + // exactly what `os build` compiles out of `objectstack.config.ts`. + writeFileSync( + join(dir, 'dist', 'objectstack.json'), + JSON.stringify({ + id: 'mig_pinyin_smoke', + name: 'Migrate Pinyin Smoke', + i18n: { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'], fallbackLocale: 'en' }, + objects: [ + { + name: 'mig_person', + fields: { + name: { type: 'text', required: true }, + email: { type: 'text' }, + }, + }, + ], + }), + ); + + // Seed the database the way a dev-runtime first boot leaves it: the + // object's columns PLUS the provisioned `__search` companion. + const seed = new SqlDriver({ client: 'better-sqlite3', connection: { filename: dbFile }, useNullAsDefault: true }); + const k = (seed as any).knex; + await k.schema.createTable('mig_person', (t: any) => { + t.string('id').primary(); + t.timestamp('created_at'); + t.timestamp('updated_at'); + t.string('name').notNullable(); + t.string('email'); + t.string('__search'); + }); + await k('mig_person').insert({ id: '1', name: '张伟', email: 'zw@example.com', __search: 'zhangwei zw' }); + await k.destroy(); + + savedEnv.OS_ARTIFACT_PATH = process.env.OS_ARTIFACT_PATH; + savedEnv.NODE_ENV = process.env.NODE_ENV; + savedEnv.OS_SEARCH_PINYIN_ENABLED = process.env.OS_SEARCH_PINYIN_ENABLED; + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); + process.env.NODE_ENV = 'production'; + // The defect precondition: nothing external decided pinyin — the boot must + // derive it from the artifact's locales, exactly like serve/dev does. + delete process.env.OS_SEARCH_PINYIN_ENABLED; + }); + + afterAll(() => { + process.env.OS_ARTIFACT_PATH = savedEnv.OS_ARTIFACT_PATH; + process.env.NODE_ENV = savedEnv.NODE_ENV; + if (savedEnv.OS_SEARCH_PINYIN_ENABLED === undefined) delete process.env.OS_SEARCH_PINYIN_ENABLED; + else process.env.OS_SEARCH_PINYIN_ENABLED = savedEnv.OS_SEARCH_PINYIN_ENABLED; + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('the migrate boot provisions the companion in metadata, so plan reports no __search drift', async () => { + const stack = await bootSchemaStack({ databaseUrl: `file:${dbFile}` }); + try { + expect(stack.driver).toBeTruthy(); + + // The fix, observed at the seam: the artifact's zh-CN locale was stamped + // into the pinyin decision, so the migrate boot's OWN metadata carries + // the companion column — the same schema view the dev runtime serves. + const managed = (stack.driver as any).managedObjectFields.get('mig_person'); + expect(managed, 'mig_person must be metadata-managed').toBeDefined(); + expect(Object.keys(managed)).toContain('__search'); + + // And the defect, gone: no drift on __search — before the fix this was a + // destructive drop_column "orphaned" finding pointing at --allow-destructive. + const drift = await stack.driver!.detectManagedDrift(); + const searchDrift = drift.filter((d) => d.column === '__search'); + expect(searchDrift).toEqual([]); + expect(drift.filter((d) => d.category === 'destructive')).toEqual([]); + } finally { + await stack.shutdown(); + } + }, 30_000); +}); diff --git a/packages/plugins/driver-sql/src/schema-drift.ts b/packages/plugins/driver-sql/src/schema-drift.ts index 5b3adecd65..760cd4ee06 100644 --- a/packages/plugins/driver-sql/src/schema-drift.ts +++ b/packages/plugins/driver-sql/src/schema-drift.ts @@ -510,28 +510,50 @@ export interface LegacyUniqueReplacement { * names to look for and the composite that replaces them. `unique: 'global'` * fields are excluded — their single-column index is the declared intent now, * not legacy debt. + * + * `declaredIndexes` are excluded the same way, and for the same reason (#3955). + * An object may declare a single-column unique index alongside a tenant-scoped + * field-level `unique: true` — `email: { unique: true }` plus + * `indexes: [{ fields: ['email'], unique: true }]`. That declared index + * materializes under {@link buildIndexName}, which is *also* one of the two + * spellings {@link legacyUniqueIndexNames} looks for, so without this filter the + * detector reads an index metadata declares TODAY as pre-#3696 debt and proposes + * dropping it. The plan then never converges: apply drops the declared index, + * the next plan reports it missing and recreates it, and the one after that + * calls it legacy again — an unbounded drop/create cycle on a live unique index. + * An index the current metadata declares is by definition not legacy. */ export function legacyUniqueReplacements(args: { table: string; fields: Record; tenantField: string | null; physicalColumns: Set; + declaredIndexes?: Array<{ name?: string; fields?: string[]; unique?: boolean | 'global' }>; }): LegacyUniqueReplacement[] { - const { table, fields, tenantField, physicalColumns } = args; + const { table, fields, tenantField, physicalColumns, declaredIndexes } = args; if (!tenantField) return []; // Nothing was ever mis-scoped on a tenant-less table. // Without a physical tenant column there is no composite to replace the // legacy index with, and dropping it unreplaced would remove the constraint // outright rather than relax it. Leave it alone. if (!physicalColumns.has(tenantField)) return []; + // Normalized through the same helper the create path uses, so "what the + // declared index is named" is answered once, not guessed at twice. + const declaredNames = new Set( + (Array.isArray(declaredIndexes) ? declaredIndexes : []) + .map((idx) => normalizeDeclaredIndex(table, idx)?.name) + .filter((n): n is string => typeof n === 'string'), + ); const out: LegacyUniqueReplacement[] = []; for (const [name, field] of Object.entries(fields ?? {})) { if (!isUniqueDeclared(field?.unique)) continue; if (isGlobalUnique(field.unique)) continue; if (name === tenantField || !physicalColumns.has(name)) continue; + const legacyNames = legacyUniqueIndexNames(table, name).filter((n) => !declaredNames.has(n)); + if (legacyNames.length === 0) continue; const columns = [tenantField, name]; out.push({ column: name, - legacyNames: legacyUniqueIndexNames(table, name), + legacyNames, replacement: { name: buildIndexName(table, columns, true), columns, unique: true }, }); } diff --git a/packages/plugins/driver-sql/src/sql-driver-index-drift.test.ts b/packages/plugins/driver-sql/src/sql-driver-index-drift.test.ts index b3feec1191..6e323e95f9 100644 --- a/packages/plugins/driver-sql/src/sql-driver-index-drift.test.ts +++ b/packages/plugins/driver-sql/src/sql-driver-index-drift.test.ts @@ -153,6 +153,98 @@ describe('SqlDriver index drift (#3728)', () => { }); }); + // ── #3955: a DECLARED single-column unique is not legacy debt ───────────── + // + // An object may declare both a tenant-scoped field-level `unique: true` and + // an object-level single-column unique index on the same column (HotCRM's + // `crm_contact` does exactly this: `email: { unique: true }` plus + // `indexes: [{ fields: ['email'], unique: true }]`). The declared index + // materializes under `buildIndexName` — `uniq_
_` — which is also + // one of the two spellings the legacy detector looks for. It was therefore + // reported as pre-#3696 debt to be dropped, and the plan never converged. + describe('a currently-declared single-column unique index is never called legacy (#3955)', () => { + const contactMeta = [ + { + name: 'hp_contact', + fields: { + organization_id: { type: 'string' }, + email: { type: 'string', unique: true }, + last_name: { type: 'string' }, + }, + indexes: [ + { fields: ['email'], unique: true }, + { fields: ['last_name'] }, + ], + }, + ]; + + it('builds both indexes and then reports NO drift on a fresh table', async () => { + const driver = makeDriver(); + await driver.initObjects(contactMeta); + + // Both are current intent: the tenant composite from the field-level + // `unique: true`, and the verbatim declared global unique. + const uniques = await uniqueIndexColumns('hp_contact'); + expect(uniques['uniq_hp_contact_organization_id_email']).toEqual(['organization_id', 'email']); + expect(uniques['uniq_hp_contact_email']).toEqual(['email']); + + // Before the fix this reported `replace_unique_index` — proposing to drop + // the index the very same sync had just created from metadata. + expect(await driver.detectManagedDrift()).toHaveLength(0); + }); + + it('does not ping-pong: applying the plan converges instead of recreating work', async () => { + const driver = makeDriver(); + await driver.initObjects(contactMeta); + + // Round 1: nothing to do. Round 2 (the old loop's second half) likewise — + // the declared index is still there, so nothing reports it missing. + for (let round = 0; round < 3; round++) { + const drift = await driver.detectManagedDrift(); + expect(drift, `round ${round + 1} should be clean`).toHaveLength(0); + await driver.applyMigrationEntries(drift, { allowDestructive: false }); + } + + const uniques = await uniqueIndexColumns('hp_contact'); + expect(Object.keys(uniques).sort()).toEqual([ + 'uniq_hp_contact_email', + 'uniq_hp_contact_organization_id_email', + ]); + }); + + it('still retires a genuinely legacy index when metadata declares no such index', async () => { + // The #3728 behaviour must survive the filter: the same `uniq_
_` + // spelling IS legacy when nothing in metadata declares it. + const driver = makeDriver(); + await seedLegacyGlobalUnique('uniq_product_code'); + await driver.initObjects(productMeta); // no declared `indexes[]` + + const drift = await driver.detectManagedDrift(); + const entry = drift.find((d) => d.op.type === 'replace_unique_index'); + expect(entry, 'a legacy index nobody declares must still be retired').toBeDefined(); + expect((entry!.op as any).dropIndexNames).toEqual(['uniq_product_code']); + }); + + it('retires the knex-spelled legacy index even when the buildIndexName spelling is declared', async () => { + // Only the declared spelling is exempt. A pre-#3696 `
__unique` + // left over from the old createColumn path is still debt. + const driver = makeDriver(); + await knexInstance.schema.createTable('hp_contact', (t: any) => { + t.string('id').primary(); + t.string('organization_id'); + t.string('email'); + t.string('last_name'); + }); + await knexInstance.raw('CREATE UNIQUE INDEX hp_contact_email_unique ON hp_contact (email)'); + await driver.initObjects(contactMeta); + + const drift = await driver.detectManagedDrift(); + const entry = drift.find((d) => d.op.type === 'replace_unique_index'); + expect(entry).toBeDefined(); + expect((entry!.op as any).dropIndexNames).toEqual(['hp_contact_email_unique']); + }); + }); + // ── Applying it through `os migrate apply` ──────────────────────────────── describe('applyMigrationEntries', () => { diff --git a/packages/plugins/driver-sql/src/sql-driver.ts b/packages/plugins/driver-sql/src/sql-driver.ts index 66767fb567..03dcbf897f 100644 --- a/packages/plugins/driver-sql/src/sql-driver.ts +++ b/packages/plugins/driver-sql/src/sql-driver.ts @@ -3041,7 +3041,9 @@ export class SqlDriver implements IDataDriver { return diffManagedIndexes({ table: tableName, expected: expectedIndexes({ table: tableName, fields, tenantField, declaredIndexes, physicalColumns }), - legacy: legacyUniqueReplacements({ table: tableName, fields, tenantField, physicalColumns }), + // `declaredIndexes` goes to BOTH: it is what the table should have, and + // therefore also what must never be mistaken for legacy debt (#3955). + legacy: legacyUniqueReplacements({ table: tableName, fields, tenantField, physicalColumns, declaredIndexes }), physical: await this.introspectIndexes(tableName), }); } diff --git a/packages/runtime/src/standalone-stack.test.ts b/packages/runtime/src/standalone-stack.test.ts index b72635d15a..7f0f71ff29 100644 --- a/packages/runtime/src/standalone-stack.test.ts +++ b/packages/runtime/src/standalone-stack.test.ts @@ -121,6 +121,67 @@ describe('createStandaloneStack — surfaces app RBAC from the artifact (ADR-005 }, BOOT_TIMEOUT); }); +// #3955 — the standalone boot must share the serve/dev boot's locale-gated +// pinyin decision. `os migrate plan`/`apply` boot through this factory with the +// compiled artifact as the ONLY config in sight; before the stamp below, that +// boot resolved `resolveSearchPinyinEnabled()` env-first-only → off, computed a +// schema view WITHOUT the `__search` companion columns the dev runtime +// provisions, and reported every live companion column of a dev-created +// database as a destructive orphan (`drop_column`). +describe('createStandaloneStack — stamps the locale-derived pinyin decision from the artifact (#3955)', () => { + const originalEnv = process.env.OS_SEARCH_PINYIN_ENABLED; + let dir: string; + + beforeAll(() => { + dir = mkdtempSync(join(tmpdir(), 'os-standalone-pinyin-')); + }); + afterAll(() => { + try { rmSync(dir, { recursive: true, force: true }); } catch { /* noop */ } + if (originalEnv === undefined) delete process.env.OS_SEARCH_PINYIN_ENABLED; + else process.env.OS_SEARCH_PINYIN_ENABLED = originalEnv; + }); + + function writeArtifact(name: string, i18n?: unknown): string { + const p = join(dir, name); + writeFileSync(p, JSON.stringify({ ...ARTIFACT, ...(i18n ? { i18n } : {}) }), 'utf-8'); + return p; + } + + it('a zh-* locale in the artifact stamps OS_SEARCH_PINYIN_ENABLED before plugins boot, and i18n is surfaced', async () => { + delete process.env.OS_SEARCH_PINYIN_ENABLED; + const i18n = { defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'], fallbackLocale: 'en' }; + const result = await createStandaloneStack({ + artifactPath: writeArtifact('zh.objectstack.json', i18n), + databaseUrl: 'memory://standalone-pinyin-zh', + }); + // The stamp is what each engine's SchemaRegistry (constructed later, at + // kernel start, without config access) reads to decide whether to + // provision the `__search` companion column. + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBe('true'); + // And the config-shaped result carries i18n like requires/objects/manifest, + // so the CLI artifact-serve merge sees the same stack config keys. + expect(result.i18n).toEqual(i18n); + }, BOOT_TIMEOUT); + + it('a non-Chinese artifact leaves the env untouched (companion stays off)', async () => { + delete process.env.OS_SEARCH_PINYIN_ENABLED; + await createStandaloneStack({ + artifactPath: writeArtifact('en.objectstack.json', { defaultLocale: 'en', supportedLocales: ['en'] }), + databaseUrl: 'memory://standalone-pinyin-en', + }); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBeUndefined(); + }, BOOT_TIMEOUT); + + it('an explicit OS_SEARCH_PINYIN_ENABLED=false survives a zh-* artifact (operator override wins)', async () => { + process.env.OS_SEARCH_PINYIN_ENABLED = 'false'; + await createStandaloneStack({ + artifactPath: writeArtifact('zh-override.objectstack.json', { supportedLocales: ['zh-CN'] }), + databaseUrl: 'memory://standalone-pinyin-override', + }); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBe('false'); + }, BOOT_TIMEOUT); +}); + // ADR-0062 D1 (#3826) — the standalone `default` datasource is a DECLARATION. // The stack no longer constructs a driver: it translates the database URL into // a `{ driver, config }` definition carried by `DefaultDatasourcePlugin`, which diff --git a/packages/runtime/src/standalone-stack.ts b/packages/runtime/src/standalone-stack.ts index a4d4f2b3d6..ee2ac06d27 100644 --- a/packages/runtime/src/standalone-stack.ts +++ b/packages/runtime/src/standalone-stack.ts @@ -33,7 +33,7 @@ import { resolve as resolvePath } from 'node:path'; import { mkdirSync } from 'node:fs'; import { homedir } from 'node:os'; import { z } from 'zod'; -import { readEnvWithDeprecation } from '@objectstack/types'; +import { readEnvWithDeprecation, stampSearchPinyinEnabled } from '@objectstack/types'; import { loadArtifactBundle, isHttpUrl } from './load-artifact-bundle.js'; /** @@ -108,6 +108,16 @@ export interface StandaloneStackResult { requires?: string[]; objects?: any[]; manifest?: any; + /** + * The stack's `i18n` config as compiled into the artifact. Surfaced so a + * caller wrapping this result as a `defineStack()`-shaped config (the CLI + * artifact-serve path) drives the SAME locale-gated decisions the + * config-load path drives — notably the pinyin-search default + * (`stampSearchPinyinEnabled`, #3955). The boot itself already stamps the + * decision; this keeps the surfaced config shape complete for consumers + * that re-derive it. + */ + i18n?: any; /** * App-declared RBAC metadata, surfaced so the CLI (`serve`/`dev`/`start`) * can wire it without a host `objectstack.config.ts`. The `serve` command @@ -283,6 +293,18 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro unwrapEnvelope: true, }); + // Locale-gated pinyin search (#2486 / #3955): the compiled artifact carries + // the stack's `i18n` config, and it is the ONLY config this boot ever sees — + // `os migrate plan`/`apply` and embedders never load `objectstack.config.ts`. + // Resolve the same locale-derived decision the CLI serve boot resolves and + // stamp it into `OS_SEARCH_PINYIN_ENABLED` BEFORE any plugin constructs a + // SchemaRegistry, so this boot provisions the same `__search` companion + // columns as the dev runtime. Without the stamp, `os migrate` diffed a + // dev-created database against a schema view missing every companion column + // and flagged the live columns as destructive orphans (#3955). No artifact → + // no locales → the env-first resolver decides, same as before. + stampSearchPinyinEnabled(artifactBundle?.i18n); + const plugins: any[] = [ // MUST precede ObjectQLPlugin: its start() connects the default driver // through the datasource connection service, and ObjectQLPlugin.start() @@ -331,6 +353,8 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro Array.isArray(artifactBundle?.permissions) ? artifactBundle.permissions : undefined; const positions: any[] | undefined = Array.isArray(artifactBundle?.positions) ? artifactBundle.positions : undefined; + const i18n: any | undefined = + artifactBundle?.i18n && typeof artifactBundle.i18n === 'object' ? artifactBundle.i18n : undefined; return { plugins, @@ -343,5 +367,6 @@ export async function createStandaloneStack(config?: StandaloneStackConfig): Pro ...(manifest ? { manifest } : {}), ...(permissions ? { permissions } : {}), ...(positions ? { positions } : {}), + ...(i18n ? { i18n } : {}), }; } diff --git a/packages/types/src/env.test.ts b/packages/types/src/env.test.ts index a0c4590822..66c7e9ad7f 100644 --- a/packages/types/src/env.test.ts +++ b/packages/types/src/env.test.ts @@ -3,12 +3,14 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { _resetEnvDeprecationWarnings, + collectConfiguredLocales, readEnvWithDeprecation, resolveAllowDegradedTenancy, resolveSearchPinyinEnabled, resolveSandboxTimeoutMs, isMcpServerEnabled, resolveMcpStdioAutoStart, + stampSearchPinyinEnabled, } from './env.js'; describe('readEnvWithDeprecation', () => { @@ -176,6 +178,53 @@ describe('resolveSearchPinyinEnabled (#2486)', () => { }); }); +describe('collectConfiguredLocales / stampSearchPinyinEnabled (#3955)', () => { + const original = process.env.OS_SEARCH_PINYIN_ENABLED; + afterEach(() => { + if (original === undefined) delete process.env.OS_SEARCH_PINYIN_ENABLED; + else process.env.OS_SEARCH_PINYIN_ENABLED = original; + }); + + it('collects defaultLocale, fallbackLocale and supportedLocales; garbage collapses to []', () => { + expect( + collectConfiguredLocales({ defaultLocale: 'en', fallbackLocale: 'en', supportedLocales: ['en', 'zh-CN'] }), + ).toEqual(['en', 'en', 'en', 'zh-CN']); + expect(collectConfiguredLocales({ supportedLocales: ['en', 42, null] })).toEqual(['en']); + expect(collectConfiguredLocales(undefined)).toEqual([]); + expect(collectConfiguredLocales(null)).toEqual([]); + expect(collectConfiguredLocales('zh-CN')).toEqual([]); + }); + + it('stamps OS_SEARCH_PINYIN_ENABLED=true when a zh-* locale is configured and env is unset', () => { + delete process.env.OS_SEARCH_PINYIN_ENABLED; + expect(stampSearchPinyinEnabled({ defaultLocale: 'en', supportedLocales: ['en', 'zh-CN'] })).toBe(true); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBe('true'); + // Downstream no-arg consumers (per-engine SchemaRegistry, the plugin + // gate) now read the same decision — the whole point of the stamp. + expect(resolveSearchPinyinEnabled()).toBe(true); + }); + + it('leaves the env untouched (and returns false) for a non-Chinese config', () => { + delete process.env.OS_SEARCH_PINYIN_ENABLED; + expect(stampSearchPinyinEnabled({ defaultLocale: 'en', supportedLocales: ['en', 'ja-JP'] })).toBe(false); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBeUndefined(); + expect(stampSearchPinyinEnabled(undefined)).toBe(false); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBeUndefined(); + }); + + it('an explicit env opt-out beats the locale-derived default and is not overwritten', () => { + process.env.OS_SEARCH_PINYIN_ENABLED = 'false'; + expect(stampSearchPinyinEnabled({ supportedLocales: ['zh-CN'] })).toBe(false); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBe('false'); + }); + + it('an explicit env opt-in wins even with no zh locale configured', () => { + process.env.OS_SEARCH_PINYIN_ENABLED = 'true'; + expect(stampSearchPinyinEnabled({ supportedLocales: ['en'] })).toBe(true); + expect(process.env.OS_SEARCH_PINYIN_ENABLED).toBe('true'); + }); +}); + describe('MCP switches — HTTP surface vs stdio auto-start are decoupled (#3167)', () => { const origServer = process.env.OS_MCP_SERVER_ENABLED; const origServerLegacy = process.env.MCP_SERVER_ENABLED; diff --git a/packages/types/src/env.ts b/packages/types/src/env.ts index 9b810fcd5e..be3633cc2d 100644 --- a/packages/types/src/env.ts +++ b/packages/types/src/env.ts @@ -297,10 +297,12 @@ export function resolveOrgLimit(): number | undefined { * 3. No env var and no `zh-*` locale → off. OSS / non-Chinese deployments * never load `pinyin-pro` and pay zero compute cost. * - * Hosts that know the stack's i18n config (the CLI `serve` boot path) resolve - * once with locales and stamp the decision back into the env, so downstream - * consumers constructed without config access (per-engine SchemaRegistry) - * read the same answer via the no-arg form. + * Hosts that know the stack's i18n config — the CLI `serve` boot path AND the + * standalone artifact boot (`createStandaloneStack`, which `os migrate` + * plan/apply and embedders go through) — resolve once with locales and stamp + * the decision back into the env via {@link stampSearchPinyinEnabled}, so + * downstream consumers constructed without config access (per-engine + * SchemaRegistry) read the same answer via the no-arg form (#3955). */ export function resolveSearchPinyinEnabled(opts?: { locales?: readonly string[] }): boolean { const raw = readEnvWithDeprecation('OS_SEARCH_PINYIN_ENABLED', [], { silent: true }); @@ -310,6 +312,57 @@ export function resolveSearchPinyinEnabled(opts?: { locales?: readonly string[] return (opts?.locales ?? []).some((l) => /^zh([-_]|$)/i.test(String(l ?? '').trim())); } +/** + * The locales a stack's `i18n` config declares — `defaultLocale`, + * `fallbackLocale`, then `supportedLocales`. Accepts the config loosely typed + * (`unknown`) so any boot path can pass whatever its stack config or compiled + * artifact carries without importing spec schemas; non-string entries and a + * non-object config collapse to `[]`. + */ +export function collectConfiguredLocales(i18n: unknown): string[] { + const cfg = (i18n && typeof i18n === 'object' ? i18n : {}) as { + defaultLocale?: unknown; + fallbackLocale?: unknown; + supportedLocales?: unknown; + }; + return [ + cfg.defaultLocale, + cfg.fallbackLocale, + ...(Array.isArray(cfg.supportedLocales) ? cfg.supportedLocales : []), + ].filter((l): l is string => typeof l === 'string'); +} + +/** + * Resolve the pinyin-search decision from a stack's `i18n` config and stamp a + * positive result back into `OS_SEARCH_PINYIN_ENABLED` (#2486, #3955). + * + * Every boot path that SEES the stack config must stamp, because consumers + * constructed later without config access (each engine's `SchemaRegistry` + * provisioning the `__search` companion column, the `plugin-pinyin-search` + * gate) read the decision through the no-arg + * {@link resolveSearchPinyinEnabled}. A boot path that skips the stamp + * computes a schema view WITHOUT the companion columns — which is how + * `os migrate` came to flag the dev runtime's live `__search` columns as + * destructive orphans (#3955). Call sites: the CLI `serve`/`dev` boot + * (`objectstack.config.ts`) and `createStandaloneStack` (compiled artifact — + * `os migrate plan`/`apply`, embedders). + * + * An explicit `OS_SEARCH_PINYIN_ENABLED` always wins — the resolver reads it + * before consulting locales, so the stamp only materializes the + * locale-derived default. Only a positive decision is written: "unset" and + * "off" read identically through the no-arg resolver, and leaving the var + * untouched keeps a later boot free to re-derive from ITS config. + */ +export function stampSearchPinyinEnabled(i18n: unknown): boolean { + const enabled = resolveSearchPinyinEnabled({ locales: collectConfiguredLocales(i18n) }); + // Write through `globalThis` like `readEnvWithDeprecation` reads — this + // package has no Node type dependency (edge-safe); no env object → no stamp. + const env = (globalThis as { process?: { env?: Record } }) + .process?.env; + if (enabled && env) env.OS_SEARCH_PINYIN_ENABLED = 'true'; + return enabled; +} + /** * SINGLE decision point for a sandbox script-runner DEFAULT (ms), resolved from * the environment (framework#3259 / ADR-0102).