fix(runtime,cli,types,driver-sql): one schema view for serve/dev and migrate — __search companions + declared unique indexes (#3955)

.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_<table>_<column>` — 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 `<table>_<column>_unique` when
+only the `uniq_…` spelling is declared.

.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.

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

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 +

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<string, string | undefined> = {};
+
+ 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);
+});

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<string, any>;
  tenantField: string | null;
  physicalColumns: Set<string>;
+ 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<any>(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 },
  });
  }

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_<table>_<col>` — 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_<table>_<col>`
+ // 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 `<table>_<col>_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', () => {