From 784d0589cd3127816757baa0ba7021e0d1fb1487 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:11:41 +0000 Subject: [PATCH 1/4] fix(service-package): sys_packages on the managed migration path, and the boot honours OS_SKIP_SCHEMA_SYNC PackageServicePlugin.init() declares sys_packages as the package.storage service (presence probe + the boot's own ensurePackageTable); bootSchemaStack lists it as create_table when absent and creates it on flush, so os migrate plan / apply cover it. start() then skips its DDL when the engine resolved skipSchemaSync, logging one info line. Claude-Session: https://claude.ai/code/session_01CeRCvvvjxbxu2iYJAc5Cce Co-authored-by: Claude --- packages/cli/src/utils/schema-migrate.ts | 86 +++++++++++++ .../services/service-package/src/index.ts | 115 ++++++++++++++++-- .../service-package/src/package-table.ts | 90 +++++++++++++- 3 files changed, 274 insertions(+), 17 deletions(-) diff --git a/packages/cli/src/utils/schema-migrate.ts b/packages/cli/src/utils/schema-migrate.ts index a3b8cb6ef2b..95a7327f97f 100644 --- a/packages/cli/src/utils/schema-migrate.ts +++ b/packages/cli/src/utils/schema-migrate.ts @@ -244,6 +244,82 @@ function sortPendingSchemaWork(work: PendingSchemaWork[]): PendingSchemaWork[] { return work.sort((a, b) => a.table.localeCompare(b.table) || a.kind.localeCompare(b.kind)); } +/** + * [#23022] The kernel service under which `PackageServicePlugin.init()` + * declares `sys_packages` — `@objectstack/service-package`'s + * `PACKAGE_STORAGE_SERVICE`, read by name because the package exports no + * constant for it. + */ +const PACKAGE_STORAGE_SERVICE = 'package.storage'; + +/** What {@link PACKAGE_STORAGE_SERVICE} answers with (`service-package`'s `PackageStorageDeclaration`). */ +interface PackageStorageDeclaration { + readonly table: string; + readonly columns: readonly string[]; + /** Read-only: `true` present, `false` absent, `undefined` could not tell. */ + present(): Promise; + /** The package registry's own `CREATE TABLE IF NOT EXISTS` and index. */ + ensure(): Promise; +} + +/** + * [#23022] `sys_packages` as pending schema work, when it is absent. + * + * ## Why the plan needs this + * + * `PackageServicePlugin.start()` creates `sys_packages` with raw DDL, because + * the table cannot be a declared object (`service-package`'s + * `package-table.ts` records the measurement: the driver's managed tables key + * on `id` alone, and this one keys on `(id, version)`). So no driver deferral + * ever listed it, and a deployment that sets `OS_SKIP_SCHEMA_SYNC=1` — whose + * boot now creates nothing — was left with no way to get it from the documented + * path. The plugin declares the table in `init()`, which a migrate boot runs, + * and this lists it as `create_table` beside the objects' tables; `apply`'s + * flush then runs the plugin's own statements, so `apply` creates exactly what + * the boot creates. + * + * `null` when there is nothing to plan: the plugin is not composed, or the + * table exists. When its presence could not be READ, that is not absence: a + * note says the table was not examined, and nothing is planned. + */ +async function planPackageTable( + kernel: unknown, + notes: string[], +): Promise<{ storage: PackageStorageDeclaration; work: PendingSchemaWork } | null> { + let service: unknown; + try { + service = (kernel as { getService?: (name: string) => unknown }).getService?.(PACKAGE_STORAGE_SERVICE); + } catch { + return null; // not composed: this boot's deployment serves no package registry + } + if (service === undefined || service === null) return null; + const storage = service as Partial; + if ( + typeof storage.table !== 'string' || !Array.isArray(storage.columns) + || typeof storage.present !== 'function' || typeof storage.ensure !== 'function' + ) { + throw new Error( + `The "${PACKAGE_STORAGE_SERVICE}" service does not have the shape this CLI reads (table, columns, present, ` + + 'ensure), so sys_packages cannot be planned. @objectstack/cli and @objectstack/service-package must come ' + + 'from the same release.', + ); + } + const declared = storage as PackageStorageDeclaration; + const present = await declared.present(); + if (present === true) return null; + if (present === undefined) { + notes.push( + `${declared.table} (the package registry's table) was NOT examined: its presence could not be read on this ` + + 'database, so this run does not list it and does not create it.', + ); + return null; + } + return { + storage: declared, + work: { table: declared.table, kind: 'create_table', columns: [...declared.columns] }, + }; +} + /** * Arms deferred-DDL mode on EVERY SQL driver the boot connects, before any of * them can schema-sync (#3917, #21391). @@ -925,6 +1001,10 @@ export async function bootSchemaStack( for (const d of deferral?.drivers ?? []) { if (d.previewDeferredSchemaWork) pendingSchemaWork.push(...(await d.previewDeferredSchemaWork())); } + // [#23022] …and the one table outside the object set, `sys_packages`, which + // no driver deferral can see. Read-only here; `flushSchemaDdl` creates it. + let packageTable = deferral ? await planPackageTable(kernel, composition.notes) : null; + if (packageTable) pendingSchemaWork.push(packageTable.work); sortPendingSchemaWork(pendingSchemaWork); // [#21529] The tables the held-back sync would create: measured absent. const absentTables = new Set( @@ -1008,6 +1088,12 @@ export async function bootSchemaStack( for (const d of deferral?.drivers ?? []) { if (d.flushDeferredSchemaDdl) performed.push(...(await d.flushDeferredSchemaDdl())); } + // [#23022] The package registry's own DDL — the statements its boot runs. + if (packageTable) { + await packageTable.storage.ensure(); + performed.push(packageTable.work); + packageTable = null; + } // [#21529] The tables exist now; nothing is measured absent any more. absentTables.clear(); deferral?.release(); diff --git a/packages/services/service-package/src/index.ts b/packages/services/service-package/src/index.ts index 16ac106e458..9c6cfa75f2b 100644 --- a/packages/services/service-package/src/index.ts +++ b/packages/services/service-package/src/index.ts @@ -5,7 +5,13 @@ import { createHash } from 'node:crypto'; import { checkProtocolCompat } from '@objectstack/metadata-core'; import type { ObjectStackManifest } from '@objectstack/spec/kernel'; import type { IDataEngine } from '@objectstack/spec/contracts'; -import { ensurePackageTable, PACKAGE_TABLE_STATEMENTS, resolvePackageTableDialect } from './package-table.js'; +import { + ensurePackageTable, + PACKAGE_TABLE_COLUMNS, + PACKAGE_TABLE_STATEMENTS, + probePackageTable, + resolvePackageTableDialect, +} from './package-table.js'; export interface PackageMetadata { objects?: any[]; @@ -341,6 +347,59 @@ function isSeamUnreadable(error: unknown): boolean { ); } +/** + * [#23022] Whether the engine's `ObjectQLPlugin` resolved `skipSchemaSync` (the + * option, else `OS_SKIP_SCHEMA_SYNC=1`). `ObjectQLPlugin.init()` publishes that + * value on the engine instance as an undeclared property (#22984), so it is + * probed here: an engine nothing published it on (a test double, a foreign + * engine) reads as not set. ⛔ Never a second read of the environment: the + * option and the env var must not disagree between the engine's boot and this + * plugin's. + */ +function bootSchemaSyncSkipped(engine: IDataEngine): boolean { + return (engine as unknown as { skipSchemaSync?: unknown }).skipSchemaSync === true; +} + +/** + * [#23022] The ONE line `start()` prints when that flag is set, at `info`: the + * operator declared this, so nothing is degraded. If the table is in fact + * missing, a publish reports the missing-table failure on the write that needs + * it. + */ +const PACKAGE_TABLE_SKIPPED_LINE = + 'PackageServicePlugin: sys_packages NOT provisioned at boot — the boot\'s schema sync is skipped ' + + '(`skipSchemaSync` / OS_SKIP_SCHEMA_SYNC=1), declaring this deployment\'s DDL managed out-of-band, so this ' + + 'plugin issues none either. `os migrate apply` creates it when it is missing (`os migrate plan` lists it as ' + + '`create_table`).'; + +/** + * [#23022] The kernel service under which `init()` declares the storage this + * plugin owns outside the object model, for `os migrate plan` / `apply` + * (`packages/cli/src/utils/schema-migrate.ts` reads it by this name). + * + * Why a service, and why from `init()`: a migrate boot composes this plugin for + * its declarations only — `init()` runs, `start()` and every lifecycle hook do + * not — and `sys_packages` is not an object, so the driver's deferred sync that + * plans and creates every object's table never sees it (`package-table.ts` + * says why it cannot be one). Deliberately a service name rather than an + * export, so it adds no name to this package's declared surface; the CLI's + * integration test boots this real plugin through the migrate funnel, which is + * what holds the two ends together. + */ +const PACKAGE_STORAGE_SERVICE = 'package.storage'; + +/** [#23022] What {@link PACKAGE_STORAGE_SERVICE} answers with. */ +interface PackageStorageDeclaration { + /** The table, as a `create_table` line names it. */ + readonly table: 'sys_packages'; + /** Its columns, in DDL order. */ + readonly columns: readonly string[]; + /** Read-only: `true` present, `false` absent, `undefined` could not tell (`probePackageTable`). */ + present(): Promise; + /** Create the table and its index when absent — the boot's own `ensurePackageTable`. */ + ensure(): Promise; +} + /** * Package Management Service Plugin * @@ -351,6 +410,23 @@ export class PackageServicePlugin implements Plugin { name = 'package-service'; async init(ctx: PluginContext): Promise { + // [#23022] Declare `sys_packages` where a migrate boot can see it. The + // engine is resolved when the declaration is USED, never here: `init()` + // may run before the engine's plugin has registered it. + const engine = (): IDataEngine => { + const objectql = ctx.getService('objectql'); + if (!objectql || !objectql.execute) { + throw new Error('ObjectQL service with execute() support is required for the sys_packages table'); + } + return objectql; + }; + const storage: PackageStorageDeclaration = { + table: 'sys_packages', + columns: PACKAGE_TABLE_COLUMNS, + present: () => probePackageTable(engine()), + ensure: async () => { await ensurePackageTable(engine()); }, + }; + ctx.registerService(PACKAGE_STORAGE_SERVICE, storage); // Service will be registered in start() after ObjectQL is available ctx.logger.debug('Package service plugin initialized'); } @@ -371,18 +447,31 @@ export class PackageServicePlugin implements Plugin { // never created on MySQL and nothing above `debug` said so. "Already // exists" is no longer an error at all (see `ensurePackageTable`), so // anything arriving here is a real refusal, and the start fails with it. - try { - await this.ensureTable(objectql, logger); - } catch (error) { - logger.error( - 'Failed to create the sys_packages table — the database refused the DDL. This plugin\'s start() fails and ' - + 'no `package` service is registered: under the kernel\'s default rollback the boot fails, and a kernel ' - + 'that continues past a failed plugin installs packages in memory only. The refusal is attached here and ' - + 'in the driver\'s log line; make the database accept it (its permissions, or a dialect the driver ' - + 'names), then restart.', - error as Error, - ); - throw error; + // + // [#23022] FIRST: a deployment that declared its DDL managed out-of-band + // (`skipSchemaSync` / OS_SKIP_SCHEMA_SYNC=1, "use after running migrations + // manually") gets none from here either — the value the engine's own boot + // resolved, as `plugin-audit` and `service-messaging` read it (#22984). No + // existence probe either: the flag protects a cold-start budget. Safe only + // because the documented path covers the table: `os migrate plan` lists it + // and `os migrate apply` creates it, through the declaration `init()` + // registers above. The service below is still registered. + if (bootSchemaSyncSkipped(objectql)) { + logger.info(PACKAGE_TABLE_SKIPPED_LINE); + } else { + try { + await this.ensureTable(objectql, logger); + } catch (error) { + logger.error( + 'Failed to create the sys_packages table — the database refused the DDL. This plugin\'s start() fails and ' + + 'no `package` service is registered: under the kernel\'s default rollback the boot fails, and a kernel ' + + 'that continues past a failed plugin installs packages in memory only. The refusal is attached here and ' + + 'in the driver\'s log line; make the database accept it (its permissions, or a dialect the driver ' + + 'names), then restart.', + error as Error, + ); + throw error; + } } // Create the package service diff --git a/packages/services/service-package/src/package-table.ts b/packages/services/service-package/src/package-table.ts index 811a391e295..ee3a13c76aa 100644 --- a/packages/services/service-package/src/package-table.ts +++ b/packages/services/service-package/src/package-table.ts @@ -21,6 +21,20 @@ import type { IDataEngine } from '@objectstack/spec/contracts'; * `(id, version)` key and `TEXT` timestamps, so fresh and adopted databases * would disagree on the key. Carrying `(id, version)` that way needs a data * migration, so the table keeps its own DDL — spelled per dialect below. + * + * ## How `os migrate plan` / `apply` reach that DDL anyway (#23022) + * + * Not being an object kept the table off the managed migration path, so a + * deployment that sets `OS_SKIP_SCHEMA_SYNC=1` ("use after running migrations + * manually") had no documented way to get it. The plugin now DECLARES the table + * in `init()`, the phase a migrate boot runs, as its `package.storage` service: + * {@link probePackageTable} is the plan's read-only "is it there?", and + * {@link ensurePackageTable} is what `apply` runs. These are the boot's own + * statements, so a table `apply` creates is the one the boot creates. + * Re-measured for that card on SQLite: a declared `sys_packages` still keys on + * `id` alone (a second version refused, `SQLITE_CONSTRAINT_PRIMARYKEY`) and + * types the timestamps `datetime`, while the same object adopted over an + * existing table reports no drift — fresh and adopted databases would disagree. */ /** @@ -155,6 +169,35 @@ export const PACKAGE_TABLE_STATEMENTS: Readonly = new Map([ + ['sqlite', "SELECT 1 AS present FROM sqlite_master WHERE type = 'table' AND name = 'sys_packages' LIMIT 1"], + [ + 'postgres', + 'SELECT 1 AS present FROM information_schema.tables ' + + "WHERE table_schema = current_schema() AND table_name = 'sys_packages' LIMIT 1", + ], + [ + 'mysql', + 'SELECT 1 AS present FROM information_schema.tables ' + + "WHERE table_schema = DATABASE() AND table_name = 'sys_packages' LIMIT 1", + ], +]); + /** * The statement set for each dialect name `SqlDriver.dialectName` answers * with. A name not in this table — `'unknown'`, or no answer at all — takes @@ -196,16 +239,55 @@ const STATEMENT_SET_FOR_DRIVER_DIALECT: ReadonlyMap * driver registration could contradict. */ export function resolvePackageTableDialect(objectql: IDataEngine): PackageTableDialect { - let named: unknown; + const named = defaultDriverDialectName(objectql); + return (named === undefined ? undefined : STATEMENT_SET_FOR_DRIVER_DIALECT.get(named)) ?? 'standard'; +} + +/** + * The `dialectName` the default driver answers with, or `undefined` when it + * names none. The one read {@link resolvePackageTableDialect} and + * {@link probePackageTable} share, for the reasons the former documents. + */ +function defaultDriverDialectName(objectql: IDataEngine): string | undefined { try { const driverName = objectql.getDefaultDriverName?.(); const driver = driverName === undefined ? undefined : objectql.getDriverByName?.(driverName); - named = (driver as { dialectName?: unknown } | undefined)?.dialectName; + const named = (driver as { dialectName?: unknown } | undefined)?.dialectName; + return typeof named === 'string' ? named : undefined; } catch { // A registry or getter that throws names no dialect. - named = undefined; + return undefined; } - return (typeof named === 'string' ? STATEMENT_SET_FOR_DRIVER_DIALECT.get(named) : undefined) ?? 'standard'; +} + +/** + * [#23022] Does `sys_packages` exist on the database {@link ensurePackageTable} + * writes to? Read-only: one catalog SELECT, never DDL. This is the question + * `os migrate plan` asks before it lists the table as `create_table`. + * + * `undefined` means "could not tell", and it is never read as absent: + * + * - the default driver names no dialect with a catalog probe (`'unknown'`, a + * non-SQL driver, a host with no driver registry), so no SQL is sent; or + * - the seam accepted the probe and returned no result set at all — the read + * guard's case (commit ab47f6974): a seam that cannot answer has not + * answered "absent". + * + * A probe the database REFUSES throws, whole. + */ +export async function probePackageTable(objectql: IDataEngine): Promise { + const named = defaultDriverDialectName(objectql); + const probe = named === undefined ? undefined : PRESENCE_PROBE_FOR_DRIVER_DIALECT.get(named); + if (probe === undefined) return undefined; + const result = await objectql.execute!({ sql: probe }); + if (!isResultSet(result)) return undefined; + return hasRows(result); +} + +/** A raw SELECT's answer in one of the shapes the drivers return: rows, `{ rows }`, or mysql2's tuple. */ +function isResultSet(result: unknown): boolean { + if (Array.isArray(result)) return true; + return Array.isArray((result as { rows?: unknown } | null | undefined)?.rows); } /** One row or more in a result set, across the shapes a raw SELECT returns. */ From e55a1442fb96fb40311e51e011b7df3a0e2ea15c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:20:04 +0000 Subject: [PATCH 2/4] test(service-package): pin the package.storage declaration and the skipSchemaSync boot skip Claude-Session: https://claude.ai/code/session_01CeRCvvvjxbxu2iYJAc5Cce Co-authored-by: Claude --- .../service-package/src/managed-path.test.ts | 253 ++++++++++++++++++ .../service-package/src/package-table.test.ts | 6 +- 2 files changed, 257 insertions(+), 2 deletions(-) create mode 100644 packages/services/service-package/src/managed-path.test.ts diff --git a/packages/services/service-package/src/managed-path.test.ts b/packages/services/service-package/src/managed-path.test.ts new file mode 100644 index 00000000000..73be56f7f60 --- /dev/null +++ b/packages/services/service-package/src/managed-path.test.ts @@ -0,0 +1,253 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #23022 — `sys_packages` reaches the documented migration path, and then the + * boot honours `OS_SKIP_SCHEMA_SYNC`. + * + * ## What was broken + * + * `start()` ran its raw `CREATE TABLE IF NOT EXISTS sys_packages` whatever the + * flag said ("When `1`, skip the implicit `db:sync` on boot. Use after running + * migrations manually."), and the managed path did not cover the table at all: + * `os migrate plan --json` did not list it and `os migrate apply` did not + * create it. Honouring the flag alone would have left a flag deployment with no + * `sys_packages`, so the order is load-bearing: the declaration first, then the + * skip. + * + * ## What this suite holds + * + * 1. `init()` declares the table as the `package.storage` service: a + * read-only presence probe per dialect, and the boot's own + * `ensurePackageTable` — on a real SQLite database, the table it creates is + * the `(id, version)` table with its index. + * 2. `start()` reads the value the engine published (`skipSchemaSync`), and + * when it is set issues no SQL at all, says so once at `info`, and still + * registers the `package` service. Unset, it creates the table as before. + * + * The CLI end — `os migrate plan` listing the table and `apply` creating it, + * through this real plugin composed by the migrate funnel — is + * `packages/cli/src/utils/schema-migrate.package-table.integration.test.ts`. + */ + +import { describe, it, expect } from 'vitest'; +import { DatabaseSync } from 'node:sqlite'; +import { PackageServicePlugin, type PackageService } from './index.js'; +import { PACKAGE_TABLE_COLUMNS, PACKAGE_TABLE_STATEMENTS } from './package-table.js'; + +const PACKAGE_STORAGE_SERVICE = 'package.storage'; + +interface Declaration { + table: string; + columns: readonly string[]; + present(): Promise; + ensure(): Promise; +} + +/** A real `node:sqlite` database behind an engine whose default driver says `sqlite`, recording every statement. */ +function sqliteEngine(extra: Record = {}) { + const db = new DatabaseSync(':memory:'); + const seen: string[] = []; + const engine: any = { + getDefaultDriverName: () => 'default', + getDriverByName: () => ({ dialectName: 'sqlite' }), + async execute({ sql, args }: { sql: string; args?: unknown[] }) { + seen.push(sql); + const stmt = db.prepare(sql); + return /^\s*select/i.test(sql) ? stmt.all(...((args ?? []) as any[])) : stmt.run(...((args ?? []) as any[])); + }, + ...extra, + }; + return { db, engine, seen }; +} + +/** An engine whose default driver names `dialectName`, answering every statement with `answer`. */ +function recordingEngine(dialectName: unknown, answer: (sql: string) => unknown) { + const seen: string[] = []; + const engine: any = { + getDefaultDriverName: () => 'default', + getDriverByName: () => ({ dialectName }), + async execute({ sql }: { sql: string }) { + seen.push(sql); + return answer(sql); + }, + }; + return { engine, seen }; +} + +function pluginContext(engine: unknown) { + const logs: Array<{ level: string; msg: string }> = []; + const services = new Map(); + const ctx: any = { + logger: { + debug: (msg: string) => logs.push({ level: 'debug', msg }), + info: (msg: string) => logs.push({ level: 'info', msg }), + warn: (msg: string) => logs.push({ level: 'warn', msg }), + error: (msg: string) => logs.push({ level: 'error', msg }), + }, + getService: (n: string) => (n === 'objectql' ? engine : services.get(n)), + registerService: (n: string, s: unknown) => { services.set(n, s); }, + }; + return { ctx, logs, services }; +} + +function tableShape(db: DatabaseSync) { + return { + columns: db.prepare("SELECT name, type, \"notnull\" AS not_null FROM pragma_table_info('sys_packages') ORDER BY cid").all(), + pk: db.prepare("SELECT name FROM pragma_table_info('sys_packages') WHERE pk > 0 ORDER BY pk").all(), + indexes: db.prepare("SELECT name FROM sqlite_master WHERE type = 'index' AND name = 'idx_packages_latest'").all(), + }; +} + +const hasTable = (db: DatabaseSync) => + db.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'sys_packages'").all().length === 1; + +// --------------------------------------------------------------------------- +// 1. init() declares the table for the migrate path +// --------------------------------------------------------------------------- + +describe('[#23022] init() declares sys_packages as the package.storage service', () => { + it('a real SQLite database: absent → ensure() creates the (id, version) table and its index → present', async () => { + const { db, engine, seen } = sqliteEngine(); + const { ctx, services } = pluginContext(engine); + await new PackageServicePlugin().init(ctx); + + const storage = services.get(PACKAGE_STORAGE_SERVICE) as Declaration; + expect(storage.table).toBe('sys_packages'); + expect([...storage.columns]).toEqual([...PACKAGE_TABLE_COLUMNS]); + + expect(await storage.present()).toBe(false); + // The probe is a read: it sent one SELECT and created nothing. + expect(seen).toHaveLength(1); + expect(seen[0]).toMatch(/^SELECT /); + expect(hasTable(db)).toBe(false); + + await storage.ensure(); + expect(seen.slice(1)).toEqual([ + PACKAGE_TABLE_STATEMENTS.standard.createTable, + PACKAGE_TABLE_STATEMENTS.standard.createLatestIndex, + ]); + expect(await storage.present()).toBe(true); + expect(tableShape(db)).toEqual({ + columns: PACKAGE_TABLE_COLUMNS.map((name) => ({ + name, + type: 'TEXT', + not_null: ['created_at', 'updated_at'].includes(name) ? 0 : 1, + })), + pk: [{ name: 'id' }, { name: 'version' }], + indexes: [{ name: 'idx_packages_latest' }], + }); + }); + + it('the declared columns are the CREATE TABLE columns of every dialect, in order', () => { + for (const set of Object.values(PACKAGE_TABLE_STATEMENTS)) { + const columns = [...set.createTable.matchAll(/^\s*([a-z_]+)\s+[A-Z]/gm)].map((m) => m[1]); + expect(columns).toEqual([...PACKAGE_TABLE_COLUMNS]); + } + }); + + it.each([ + ['sqlite', /FROM sqlite_master WHERE type = 'table' AND name = 'sys_packages'/], + ['postgres', /FROM information_schema\.tables WHERE table_schema = current_schema\(\) AND table_name = 'sys_packages'/], + ['mysql', /FROM information_schema\.tables WHERE table_schema = DATABASE\(\) AND table_name = 'sys_packages'/], + ])('a %s default driver is asked its own catalog', async (dialectName, probe) => { + const { engine, seen } = recordingEngine(dialectName, () => ({ rows: [] })); + const { ctx, services } = pluginContext(engine); + await new PackageServicePlugin().init(ctx); + expect(await (services.get(PACKAGE_STORAGE_SERVICE) as Declaration).present()).toBe(false); + expect(seen).toHaveLength(1); + expect(seen[0]).toMatch(probe); + }); + + it.each([ + ['a row', [{ present: 1 }], true], + ['no row', [], false], + ['{ rows: [row] }', { rows: [{ present: 1 }] }, true], + ["mysql2's [rows, fields] with a row", [[{ present: 1 }], []], true], + ["mysql2's [rows, fields] with none", [[], []], false], + ])('the catalog answers %s', async (_label, answer, present) => { + const { engine } = recordingEngine('mysql', () => answer); + const { ctx, services } = pluginContext(engine); + await new PackageServicePlugin().init(ctx); + expect(await (services.get(PACKAGE_STORAGE_SERVICE) as Declaration).present()).toBe(present); + }); + + it.each([ + ["the driver names 'unknown'", 'unknown'], + ['a driver that names no dialect (memory, MongoDB)', undefined], + ])('could not tell is undefined, never absent, and sends no SQL: %s', async (_label, dialectName) => { + const { engine, seen } = recordingEngine(dialectName, () => []); + const { ctx, services } = pluginContext(engine); + await new PackageServicePlugin().init(ctx); + expect(await (services.get(PACKAGE_STORAGE_SERVICE) as Declaration).present()).toBeUndefined(); + expect(seen).toEqual([]); + }); + + it('a seam that returns no result set is undefined, never absent', async () => { + const { engine } = recordingEngine('sqlite', () => null); + const { ctx, services } = pluginContext(engine); + await new PackageServicePlugin().init(ctx); + expect(await (services.get(PACKAGE_STORAGE_SERVICE) as Declaration).present()).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// 2. start() honours the value the engine resolved +// --------------------------------------------------------------------------- + +describe('[#23022] start() issues no DDL when the engine resolved skipSchemaSync', () => { + it('flag set: no SQL at all, the table stays absent, ONE info line, and the package service is still registered', async () => { + const { db, engine, seen } = sqliteEngine({ skipSchemaSync: true }); + const { ctx, logs, services } = pluginContext(engine); + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + + // Hydration reads `list()` and finds no table; no DDL reached the database. + expect(seen.filter((sql) => !/^\s*SELECT/i.test(sql))).toEqual([]); + expect(hasTable(db)).toBe(false); + + const lines = logs.filter((l) => l.level === 'info' && l.msg.includes('sys_packages')); + expect(lines).toHaveLength(1); + expect(lines[0].msg).toContain('NOT provisioned at boot'); + expect(lines[0].msg).toContain('OS_SKIP_SCHEMA_SYNC'); + expect(lines[0].msg).toContain('os migrate apply'); + expect(logs.filter((l) => l.level === 'warn' || l.level === 'error')).toEqual([]); + expect(typeof (services.get('package') as PackageService | undefined)?.publish).toBe('function'); + }); + + it('flag set, then the documented path: the declaration creates the table the boot did not', async () => { + const { db, engine } = sqliteEngine({ skipSchemaSync: true }); + const { ctx, services } = pluginContext(engine); + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + expect(hasTable(db)).toBe(false); + + await (services.get(PACKAGE_STORAGE_SERVICE) as Declaration).ensure(); + expect(hasTable(db)).toBe(true); + const out = await (services.get('package') as PackageService).publish({ + manifest: { id: 'com.acme.crm', version: '1.0.0' } as any, + metadata: {}, + }); + expect(out).toEqual({ success: true }); + }); + + it.each([ + ['unset', {}], + ['false', { skipSchemaSync: false }], + ["the string 'true' (only the boolean the engine publishes counts)", { skipSchemaSync: 'true' }], + ])('CONTROL — flag %s: the boot creates the table as before, and says nothing about skipping', async (_label, extra) => { + const { db, engine, seen } = sqliteEngine(extra); + const { ctx, logs } = pluginContext(engine); + const plugin = new PackageServicePlugin(); + await plugin.init(ctx); + await plugin.start(ctx); + + expect(seen.slice(0, 2)).toEqual([ + PACKAGE_TABLE_STATEMENTS.standard.createTable, + PACKAGE_TABLE_STATEMENTS.standard.createLatestIndex, + ]); + expect(hasTable(db)).toBe(true); + expect(logs.some((l) => l.msg.includes('NOT provisioned at boot'))).toBe(false); + }); +}); diff --git a/packages/services/service-package/src/package-table.test.ts b/packages/services/service-package/src/package-table.test.ts index 0af664e902e..69d52fbb512 100644 --- a/packages/services/service-package/src/package-table.test.ts +++ b/packages/services/service-package/src/package-table.test.ts @@ -157,8 +157,10 @@ function pluginContext(engine: unknown) { error: (msg: string, err?: unknown) => logs.push({ level: 'error', msg, err }), }, getService: (n: string) => (n === 'objectql' ? engine : undefined), - registerService: (_n: string, s: PackageService) => { - registered = s; + // [#23022] `init()` registers `package.storage` too; this captures only + // the `package` service `start()` registers. + registerService: (n: string, s: PackageService) => { + if (n === 'package') registered = s; }, }; return { ctx, logs, registered: () => registered }; From 6f0d23fde6c45aff6d00049fb31b5a0d8a43b896 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:36:47 +0000 Subject: [PATCH 3/4] test(cli): os migrate plan lists sys_packages and apply creates it, through the real funnel Claude-Session: https://claude.ai/code/session_01CeRCvvvjxbxu2iYJAc5Cce Co-authored-by: Claude --- ...-migrate.package-table.integration.test.ts | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 packages/cli/src/utils/schema-migrate.package-table.integration.test.ts diff --git a/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts b/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts new file mode 100644 index 00000000000..1b442391432 --- /dev/null +++ b/packages/cli/src/utils/schema-migrate.package-table.integration.test.ts @@ -0,0 +1,157 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #23022 — `os migrate plan` lists `sys_packages` and `os migrate apply` + * creates it, with `OS_SKIP_SCHEMA_SYNC=1` in the environment. + * + * ## What was broken + * + * `sys_packages` is the one table outside the object set: + * `PackageServicePlugin.start()` creates it with raw DDL (it keys on + * `(id, version)`, which a declared object cannot carry). A migrate boot runs a + * provider's `init()` and never its `start()`, and no driver deferral sees a + * table no object declares, so the plan never listed it and `apply` never + * created it. A deployment that sets the flag — "skip the implicit `db:sync` on + * boot. Use after running migrations manually." — therefore had no documented + * way to get it, and the boot could not honour the flag without stranding the + * deployment. Measured on `9f4b652e3` through `os migrate plan --json` and + * `os migrate apply --yes --json`: no `sys_packages` in `pending`, and absent + * after the apply. + * + * ## What this holds, through the REAL funnel + * + * The boot is `bootSchemaStack` exactly as the two commands call it, composing + * the real `PackageServicePlugin` the way `os serve` mounts it (the always-on + * `package-registry` provider), so the two ends of the `package.storage` + * declaration — the plugin's `init()` and this CLI's reader — meet here: + * + * 1. plan (a never-started project): `create_table sys_packages` is pending, + * with the table's seven columns, and no database file is brought into + * existence; + * 2. apply: the boot creates no `sys_packages` before the flush (the table is + * still absent while the plan is on screen), the flush creates it and + * reports it, and the table is the one the boot's own DDL creates — + * `PRIMARY KEY (id, version)`, `TEXT` timestamps, `idx_packages_latest`; + * 3. a plan over the migrated database lists nothing for it. + * + * The flag is SET for every boot here: the documented path must not depend on + * it. That the boot itself creates nothing under the flag is + * `@objectstack/service-package`'s `managed-path.test.ts`. + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { existsSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import Database from 'better-sqlite3'; +// Module-top side-effect loads, paid during COLLECTION rather than inside a +// clocked test body: `bootSchemaStack` reaches these through dynamic +// `import()`s that resolve through `dist/` (`check:test-source-alias`). +import '@objectstack/runtime'; +import '@objectstack/objectql'; +import { bootSchemaStack, type SchemaStack } from './schema-migrate.js'; + +const COLUMNS = ['id', 'version', 'manifest', 'metadata', 'hash', 'created_at', 'updated_at']; + +/** The env vars that outrank the unified project default (#6469). */ +const OVERRIDING_ENV = ['OS_DATABASE_URL', 'DATABASE_URL', 'TURSO_DATABASE_URL', 'OS_DATABASE_DRIVER', 'OS_HOME']; +const SET_ENV: Record = { + OS_SKIP_SCHEMA_SYNC: '1', + OS_TELEMETRY_DB: '0', + NODE_ENV: 'production', +}; + +describe('os migrate plan / apply cover sys_packages, flag set (#23022)', () => { + let dir: string; + let dbFile: string; + let stack: SchemaStack | null = null; + const savedEnv: Record = {}; + + const boot = (mode: 'plan' | 'apply'): Promise => bootSchemaStack({ + jsonOutput: false, + projectRoot: dir, + databaseUrl: `file:${dbFile}`, + deferSchemaDdl: true, + ...(mode === 'plan' ? { readOnlyProbe: true } : {}), + composeHostStack: true, + composeServedPlatform: true, + }); + + const packageWork = (s: SchemaStack) => s.pendingSchemaWork.filter((w) => w.table === 'sys_packages'); + + /** Read the file with a connection of OUR OWN — never the booted stack's. */ + function readTable() { + const db = new Database(dbFile, { readonly: true }); + try { + return { + present: db.prepare("SELECT name FROM sqlite_master WHERE type = 'table' AND name = 'sys_packages'").all().length === 1, + columns: db.prepare("SELECT name, type, \"notnull\" AS not_null FROM pragma_table_info('sys_packages') ORDER BY cid").all(), + pk: db.prepare("SELECT name FROM pragma_table_info('sys_packages') WHERE pk > 0 ORDER BY pk").all(), + index: db.prepare("SELECT name FROM pragma_index_info('idx_packages_latest') ORDER BY seqno").all(), + }; + } finally { + db.close(); + } + } + + beforeEach(() => { + dir = mkdtempSync(join(tmpdir(), 'os-23022-')); + dbFile = join(dir, 'app.db'); + writeFileSync(join(dir, 'objectstack.config.ts'), [ + 'export default {', + " manifest: { id: 'com.example.os23022', name: 'os23022', version: '0.0.0', type: 'app' },", + " objects: [{ name: 'os23022_note', fields: { title: { type: 'text' } } }],", + '};', + '', + ].join('\n')); + for (const key of [...OVERRIDING_ENV, ...Object.keys(SET_ENV), 'OS_ARTIFACT_PATH']) savedEnv[key] = process.env[key]; + for (const key of OVERRIDING_ENV) delete process.env[key]; + Object.assign(process.env, SET_ENV); + process.env.OS_ARTIFACT_PATH = join(dir, 'dist', 'objectstack.json'); // deliberately absent + }); + + afterEach(async () => { + try { await stack?.shutdown(); } catch { /* torn down either way */ } + stack = null; + for (const [key, value] of Object.entries(savedEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + try { rmSync(dir, { recursive: true, force: true }); } catch { /* ignore */ } + }); + + it('plan lists it, apply creates the boot\'s own table, and a plan after that lists nothing for it', async () => { + // 1. plan — a never-started project. + stack = await boot('plan'); + expect(stack.composition.servedBoot).toEqual({ mirrored: true }); + expect(packageWork(stack)).toEqual([{ table: 'sys_packages', kind: 'create_table', columns: COLUMNS }]); + await stack.shutdown(); + stack = null; + expect(existsSync(dbFile), 'a dry run brought the database into existence').toBe(false); + + // 2. apply — nothing for it before the flush, the boot's table after it. + stack = await boot('apply'); + expect(packageWork(stack)).toEqual([{ table: 'sys_packages', kind: 'create_table', columns: COLUMNS }]); + expect(readTable().present, 'the migrate boot created sys_packages before the operator confirmed').toBe(false); + const created = await stack.flushSchemaDdl(); + expect(created.filter((w) => w.table === 'sys_packages')).toEqual([ + { table: 'sys_packages', kind: 'create_table', columns: COLUMNS }, + ]); + await stack.shutdown(); + stack = null; + expect(readTable()).toEqual({ + present: true, + columns: COLUMNS.map((name) => ({ + name, + type: 'TEXT', + not_null: ['created_at', 'updated_at'].includes(name) ? 0 : 1, + })), + pk: [{ name: 'id' }, { name: 'version' }], + index: [{ name: 'id' }, { name: 'created_at' }], + }); + + // 3. plan again — the table is there, so nothing is pending for it. + stack = await boot('plan'); + expect(packageWork(stack)).toEqual([]); + }, 300_000); +}); From e644d53aaaa72745d25b37cf8de311936f1d6dda Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 12 Oct 2026 13:44:12 +0000 Subject: [PATCH 4/4] changeset: sys_packages on the migrate path, and the boot honours OS_SKIP_SCHEMA_SYNC for it Claude-Session: https://claude.ai/code/session_01CeRCvvvjxbxu2iYJAc5Cce Co-authored-by: Claude --- .changeset/23022-sys-packages-managed-then-flag.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .changeset/23022-sys-packages-managed-then-flag.md diff --git a/.changeset/23022-sys-packages-managed-then-flag.md b/.changeset/23022-sys-packages-managed-then-flag.md new file mode 100644 index 00000000000..963711b4337 --- /dev/null +++ b/.changeset/23022-sys-packages-managed-then-flag.md @@ -0,0 +1,14 @@ +--- +'@objectstack/service-package': patch +'@objectstack/cli': patch +--- + +`os migrate plan` / `apply` cover `sys_packages`, and the boot honours `OS_SKIP_SCHEMA_SYNC` for it + +Clause-②: no + +`sys_packages`, the package registry's table, is created by `PackageServicePlugin.start()` with its own DDL, because it keys on `(id, version)` and a declared object cannot carry that key. So it was never on the managed migration path: `os migrate plan` did not list it and `os migrate apply` did not create it. A deployment that sets `OS_SKIP_SCHEMA_SYNC=1` ("Use after running migrations manually") still had it created at boot, and had no documented way to create it otherwise. + +- **The migration path covers it.** `PackageServicePlugin.init()` declares the table as its `package.storage` kernel service: a read-only catalog probe (SQLite, PostgreSQL and MySQL each ask their own catalog) and the boot's own create statements. `os migrate plan` lists `sys_packages` as `create_table` when it is absent, and `os migrate apply` creates it, with the same statements the boot runs, so the table and its `idx_packages_latest` index come out exactly as a boot creates them. Whether the flag is set does not matter to either command. Where the table's presence cannot be read (a driver that names no SQL dialect), the run says it did not examine the table, and neither lists nor creates it. +- **Then the boot honours the flag.** With the flag set, `PackageServicePlugin` issues no DDL and no existence probe at boot, and logs one `info` line naming `sys_packages` and the flag, and pointing at `os migrate apply`. The `package` service is still registered. The value read is the one `ObjectQLPlugin` resolved (the `skipSchemaSync` option, else the environment variable), as `plugin-audit` and `service-messaging` read it. On a flag deployment, run `os migrate apply` before the first boot that needs the table. +- **What is unchanged.** With the flag unset, the boot creates the table exactly as before. An existing `sys_packages` table is not altered by either command; the plan lists nothing for it. No type or export changes.