diff --git a/.changeset/app-plugin-flat-bundle-seed-double-collect.md b/.changeset/app-plugin-flat-bundle-seed-double-collect.md new file mode 100644 index 0000000000..f64edf62b9 --- /dev/null +++ b/.changeset/app-plugin-flat-bundle-seed-double-collect.md @@ -0,0 +1,32 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): a flat-manifest bundle no longer collects every seed dataset twice + +`AppPlugin.start()` collects seed data from two locations — the top-level +`data` field, then the legacy `manifest.data` for backward compatibility. The +legacy read resolves its base as `this.bundle.manifest || this.bundle`, so on a +FLAT bundle — manifest fields written directly on the bundle rather than nested +under `manifest:`, a shape `AppPlugin` supports by design and this repo's own +tests construct — it re-read the very array the top-level read had just +contributed. Every dataset landed in the collection twice. + +`mergeSeedDatasets` is a plain `push` with no de-duplication, so both copies +reached the shared `seed-datasets` registry, the inline boot seed, and every +later per-org replay. For an `upsert` dataset with an `externalId` the second +pass is idempotent and the cost is doubled work; for a `mode: 'insert'` dataset +it is the dataset APPLIED TWICE per boot — measured here as two `insert` calls +for one record. + +The legacy read now carries the same reference guard its sibling collector has +always carried: `loadTranslations()` performs the identical two-location read +and skips the legacy half when `manifest.translations` IS the array the top +level already contributed. That asymmetry between the two collectors was the +whole defect, so the repair is the sibling's guard rather than a third spelling +of the same idea. + +⛔ Not a removal of the legacy read: a bundle whose `manifest.data` is a +genuinely different array from its top-level `data` still contributes both, and +a bundle that nests its manifest is unaffected either way. Nothing is added to +or removed from any published surface. diff --git a/packages/runtime/src/app-plugin.seed.test.ts b/packages/runtime/src/app-plugin.seed.test.ts index dd058db099..4f3b56fe8c 100644 --- a/packages/runtime/src/app-plugin.seed.test.ts +++ b/packages/runtime/src/app-plugin.seed.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { AppPlugin } from './app-plugin'; +import { readSeedDatasets } from './seed-datasets.js'; import { readSeedSettlement } from './seed-settlement.js'; import type { PluginContext } from '@objectstack/core'; @@ -234,3 +235,114 @@ describe('AppPlugin inline-seed settle signal (app:seeded, #2996)', () => { }); }); }); + +/** + * #15262 — a FLAT bundle (manifest fields written directly on the bundle, no + * `manifest:` key) had every seed dataset collected TWICE. + * + * `start()` reads seed data from two locations: the top-level `data` field and + * the legacy `manifest.data`. The legacy read resolves its base as + * `this.bundle.manifest || this.bundle`, so on a flat bundle it re-reads the + * very array the top-level read just contributed. `mergeSeedDatasets` is a + * plain `push` with no de-duplication, so both copies reach the shared + * `seed-datasets` registry AND the inline loader — for a `mode: 'insert'` + * dataset that is the row applied twice per boot, not merely doubled work. + * + * The sibling two-location collector for `translations` (`loadTranslations`) + * has always carried the reference guard that makes the same read safe. These + * tests pin BOTH halves of that guard: the flat bundle collects once, and a + * bundle whose `manifest.data` is a genuinely DIFFERENT array still collects + * both — a fix that simply deleted the legacy read would pass the first + * assertion and fail the second. + */ +describe('AppPlugin flat-bundle seed collection (#15262)', () => { + const OLD_BUDGET = process.env.OS_INLINE_SEED_BUDGET_MS; + const OLD_MULTI = process.env.OS_MULTI_ORG_ENABLED; + + let insert: ReturnType; + + /** A context with a real service map, so `seed-datasets` can be read back. */ + const makeContext = (): PluginContext => { + const services = new Map(); + return { + logger: { info: vi.fn(), error: vi.fn(), warn: vi.fn(), debug: vi.fn() }, + registerService: vi.fn((name: string, svc: unknown) => { + if (services.has(name)) throw new Error(`service '${name}' already registered`); + services.set(name, svc); + }), + getService: vi.fn((name: string) => { + if (name === 'objectql') return { insert }; + if (services.has(name)) return services.get(name); + return undefined; // `metadata` absent → basic-insert fallback + }), + getServices: vi.fn(() => new Map()), + hook: vi.fn(), + trigger: vi.fn(), + } as unknown as PluginContext; + }; + + /** The shape the card names: NO `manifest` key, a top-level `data` array. */ + const flatBundle = () => ({ + id: 'com.test.flat-seed', + name: 'flat-seed', + data: [ + { + object: 'sys_user', + mode: 'insert', + records: [{ id: 'flat_u1', name: 'Flat One' }], + }, + ], + }); + + beforeEach(() => { + delete process.env.OS_MULTI_ORG_ENABLED; + process.env.OS_INLINE_SEED_BUDGET_MS = '8000'; + insert = vi.fn(async () => undefined); + }); + + afterEach(() => { + if (OLD_BUDGET === undefined) delete process.env.OS_INLINE_SEED_BUDGET_MS; + else process.env.OS_INLINE_SEED_BUDGET_MS = OLD_BUDGET; + if (OLD_MULTI === undefined) delete process.env.OS_MULTI_ORG_ENABLED; + else process.env.OS_MULTI_ORG_ENABLED = OLD_MULTI; + }); + + it('collects a flat bundle seed dataset ONCE, not once per read location', async () => { + const ctx = makeContext(); + + await new AppPlugin(flatBundle()).start(ctx); + + expect(readSeedDatasets(ctx)).toHaveLength(1); + }); + + it('applies a mode:insert record ONCE per boot on a flat bundle', async () => { + const ctx = makeContext(); + + await new AppPlugin(flatBundle()).start(ctx); + + // The row consequence, not the collection count: the doubled dataset + // was applied twice by the inline seed — the #3434 end state reached + // by a different route. + expect(insert).toHaveBeenCalledTimes(1); + expect(insert).toHaveBeenCalledWith('sys_user', { id: 'flat_u1', name: 'Flat One' }, expect.anything()); + }); + + it('still collects BOTH sources when manifest.data is a genuinely different array', async () => { + const ctx = makeContext(); + const bundle = { + manifest: { + id: 'com.test.nested-seed', + data: [{ object: 'sys_user', mode: 'insert', records: [{ id: 'legacy_u1' }] }], + }, + data: [{ object: 'sys_user', mode: 'insert', records: [{ id: 'top_u1' }] }], + }; + + await new AppPlugin(bundle).start(ctx); + + // Anti-vacuity: the legacy fallback is GUARDED, not removed. + expect(readSeedDatasets(ctx)).toHaveLength(2); + expect(insert).toHaveBeenCalledTimes(2); + expect(insert).toHaveBeenCalledWith('sys_user', { id: 'top_u1' }, expect.anything()); + expect(insert).toHaveBeenCalledWith('sys_user', { id: 'legacy_u1' }, expect.anything()); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index aa24315ee2..7dfa5287c2 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -1152,8 +1152,21 @@ export class AppPlugin implements Plugin { } // 2. Legacy: `manifest.data` (backward compatibility) + // + // [#15262] The reference guard is what makes the two-location read + // safe, and it is the same guard the sibling `translations` collector + // in `loadTranslations()` has always carried. On a FLAT bundle — + // manifest fields written directly on the bundle, no `manifest:` key, + // a shape `AppPlugin` supports by design (see the constructor's + // `bundle?.manifest || bundle`) — this base resolves to the bundle + // itself, so without the check step 2 re-reads the very array step 1 + // just contributed and every dataset lands twice. `mergeSeedDatasets` + // is a plain `push` with no de-duplication, so both copies reach the + // shared registry, the inline loader and every later per-org replay: + // for a `mode: 'insert'` dataset that is the row applied twice per + // boot, not merely doubled work. const manifest = this.bundle.manifest || this.bundle; - if (manifest && Array.isArray(manifest.data)) { + if (manifest && Array.isArray(manifest.data) && manifest.data !== this.collections.data) { seedDatasets.push(...manifest.data); }