diff --git a/.changeset/marketplace-heal-seed-summary.md b/.changeset/marketplace-heal-seed-summary.md new file mode 100644 index 0000000000..ef186b0ade --- /dev/null +++ b/.changeset/marketplace-heal-seed-summary.md @@ -0,0 +1,36 @@ +--- +"@objectstack/cloud-connection": patch +--- + +fix(seed-summary): count the marketplace rehydrate heal in the boot banner, and make the counter actually accumulate + +The boot-banner `Seeds:` line (#3435) was fed by a `seed-summary` kernel +counter that only AppPlugin's config-app seed wrote to. The marketplace +rehydrate heal (#3421) — which seeds an installed package's sample data onto a +fresh/empty database in the same boot-quiet window — wrote nothing, so a +marketplace package's healed rows, and critically its EMPTY "installed but 0 +rows" state, were absent from the banner (the exact class of bug the line +exists to catch). On the showcase the banner read `130 inserted · 6 updated` +while HotCRM silently healed 162 more rows next to it. + +Two latent bugs in the counter blocked simply folding the heal in: + +1. It read the prior total through `(ctx as any).kernel?.getService(...)`, but + the plugin `PluginContext` has no `.kernel` handle (kernel.ts builds it with + `getService` / `registerService` only) — so the read was always `undefined` + and each write was a blind overwrite, never an accumulation. +2. `registerService(name, …)` THROWS on a duplicate name, so the second writer's + registration threw (caught → silently dropped). Whichever seed source ran + last simply won; combined with (1) the "accumulates across apps" intent + never worked. + +Fix: a shared `accumulateSeedSummary(ctx, delta)` in `@objectstack/types` +registers ONE mutable counter object and mutates it in place — race- and +cache-safe regardless of which seed source (config app or marketplace heal) +runs first. Both AppPlugin and the marketplace heal now use it. The marketplace +heal reports healed rows as inserted/updated, and forces a non-zero `rejected` +when it lands zero rows so the "installed but 0 rows" state escalates to the +banner's yellow warning. + +Verified on the showcase: fresh boot now reads `292 inserted · 6 updated` +(130 config + 162 HotCRM heal) instead of `130 · 6`. diff --git a/packages/cloud-connection/src/marketplace-install-local-heal.test.ts b/packages/cloud-connection/src/marketplace-install-local-heal.test.ts index 3781dddf97..9b63b374ce 100644 --- a/packages/cloud-connection/src/marketplace-install-local-heal.test.ts +++ b/packages/cloud-connection/src/marketplace-install-local-heal.test.ts @@ -53,17 +53,28 @@ function makeRawApp() { function makeCtx(rawApp: any, services: Record) { const hooks = new Map(); + // Dynamically-registered services (e.g. the `seed-summary` counter the boot + // banner reads, #3435). Faithful to the real kernel: getService THROWS when + // absent, registerService THROWS on a duplicate — the exact behaviours + // accumulateSeedSummary's register-once-then-mutate design has to survive. + const registered = new Map(); return { ctx: { hook: (e: string, h: any) => hooks.set(e, h), getService: (name: string) => { if (name === 'http-server') return { getRawApp: () => rawApp }; + if (registered.has(name)) return registered.get(name); const svc = services[name]; if (svc === undefined) throw new Error(`no ${name}`); return svc; }, + registerService: (name: string, value: any) => { + if (registered.has(name)) throw new Error(`Service '${name}' already registered`); + registered.set(name, value); + }, logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() }, }, + seedSummary: () => registered.get('seed-summary'), fire: async () => { await hooks.get('kernel:ready')?.(); }, }; } @@ -134,11 +145,11 @@ async function rehydrateWith(entryOverrides: Record, findRows: Reco }); const rawApp = makeRawApp(); const services = makeServices(findRows); - const { ctx, fire } = makeCtx(rawApp, services); + const { ctx, fire, seedSummary } = makeCtx(rawApp, services); const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); await plugin.start(ctx as any); await fire(); - return { rawApp, services, ctx }; + return { rawApp, services, ctx, seedSummary }; } describe('rehydrate sample-data healing', () => { @@ -234,3 +245,60 @@ describe('rehydrate sample-data healing', () => { expect(new LocalManifestSource(dir).read(MANIFEST.id)?.withSampleData).toBe(true); }); }); + +// The boot banner's `Seeds:` line (#3435) is fed by a `seed-summary` kernel +// counter that AppPlugin populates for the config-app seed. The marketplace +// rehydrate heal runs in the same boot-quiet window but wrote nothing to it, so +// a marketplace package's healed rows — or, critically, its EMPTY state (the +// original "installed but 0 rows" bug) — were absent from the banner. These pin +// the fold-in. +describe('rehydrate heal feeds the boot-banner seed-summary (#3430 follow-up)', () => { + it('adds healed rows to the seed-summary counter', async () => { + seedResult = { summary: { totalInserted: 3, totalUpdated: 1, totalSkipped: 2 }, errors: [] }; + const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] }); + expect(seedSummary()).toMatchObject({ inserted: 3, updated: 1, skipped: 2, rejected: 0 }); + }); + + it('escalates an empty heal (0 rows) to a non-zero rejected — the "installed but 0 rows" signal', async () => { + // Empty DB, heal runs, loader lands nothing and reports its failures as + // skips (0 hard errors). The banner must still flag it, so rejected is + // forced to at least 1. + seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [] }; + const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] }); + expect(seedSummary().rejected).toBeGreaterThanOrEqual(1); + expect(seedSummary().inserted).toBe(0); + }); + + it('surfaces the loader\'s own error count as rejected when a heal errors', async () => { + seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [{ message: 'locked' }, { message: 'locked' }] }; + const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] }); + expect(seedSummary().rejected).toBe(2); + }); + + it('does NOT touch the counter when heal is skipped (data already present)', async () => { + const { seedSummary } = await rehydrateWith({}, { crm_x: [{ id: 'a' }], crm_y: [] }); + expect(seedSummary()).toBeUndefined(); + }); + + it('does NOT touch the counter for a purged package', async () => { + const { seedSummary } = await rehydrateWith({ sampleDataPurged: true }, { crm_x: [], crm_y: [] }); + expect(seedSummary()).toBeUndefined(); + }); + + it('accumulates on top of a config-app seed-summary already on the kernel', async () => { + // Simulate AppPlugin having seeded first: pre-load the counter, then let + // the marketplace heal add to it (the banner shows one combined line). + seedResult = { summary: { totalInserted: 5, totalUpdated: 0, totalSkipped: 0 }, errors: [] }; + new LocalManifestSource(dir).write({ + packageId: 'pkg_1', versionId: 'pkgv_1', manifestId: MANIFEST.id, version: MANIFEST.version, + manifest: MANIFEST, installedAt: '2026-01-01T00:00:00.000Z', installedBy: 'admin', withSampleData: false, + }); + const rawApp = makeRawApp(); + const { ctx, fire, seedSummary } = makeCtx(rawApp, makeServices({ crm_x: [], crm_y: [] })); + (ctx as any).registerService('seed-summary', { inserted: 130, updated: 6, skipped: 0, rejected: 0 }); + const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir }); + await plugin.start(ctx as any); + await fire(); + expect(seedSummary()).toMatchObject({ inserted: 135, updated: 6 }); + }); +}); diff --git a/packages/cloud-connection/src/marketplace-install-local-plugin.ts b/packages/cloud-connection/src/marketplace-install-local-plugin.ts index efa332c087..0b4ad5c3be 100644 --- a/packages/cloud-connection/src/marketplace-install-local-plugin.ts +++ b/packages/cloud-connection/src/marketplace-install-local-plugin.ts @@ -42,7 +42,7 @@ */ import type { Plugin, PluginContext } from '@objectstack/core'; -import { resolveMultiOrgEnabled } from '@objectstack/types'; +import { resolveMultiOrgEnabled, accumulateSeedSummary } from '@objectstack/types'; import { resolveCloudUrl } from './cloud-url.js'; import { resolveMarketplacePublicBaseUrl } from './marketplace-public-url.js'; import { LocalManifestSource, type InstalledManifestEntry } from './local-manifest-source.js'; @@ -253,11 +253,22 @@ export class MarketplaceInstallLocalPlugin implements Plugin { entry.sampleDataPurged = false; try { this.ledger.write(entry); } catch { /* non-fatal */ } ctx.logger?.info?.(`[MarketplaceInstallLocal] healed sample data for ${entry.manifestId}: inserted=${summary.inserted} updated=${summary.updated} errors=${summary.errors}`); + // #3430 follow-up: fold the heal into the boot banner's Seeds + // line (the shared `seed-summary` counter #3435 added for the + // config seed) so a marketplace package's healed rows are + // visible too. + accumulateSeedSummary(ctx as any, { inserted: summary.inserted, updated: summary.updated, skipped: summary.skipped, rejected: summary.errors }); } else { ctx.logger?.warn?.(`[MarketplaceInstallLocal] sample-data heal for ${entry.manifestId} landed no rows${summary.errorSample ? ` — first error: ${summary.errorSample}` : ''}`); + // The "installed but 0 rows" state — the exact thing the banner + // Seeds line exists to catch. Force a non-zero `rejected` so it + // escalates to the yellow warning even when the loader reported + // its failures as skips rather than hard errors. + accumulateSeedSummary(ctx as any, { skipped: summary.skipped, rejected: Math.max(summary.errors, 1) }); } } catch (err: any) { ctx.logger?.warn?.(`[MarketplaceInstallLocal] sample-data heal failed for ${entry.manifestId}: ${err?.message ?? err}`); + accumulateSeedSummary(ctx as any, { rejected: 1 }); } }; diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index c8be1caded..178dbd5c6a 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -2,7 +2,7 @@ import { Plugin, PluginContext, wireAuthoredTranslationSync } from '@objectstack/core'; import { assertProtocolCompat } from '@objectstack/metadata-core'; -import { resolveMultiOrgEnabled } from '@objectstack/types'; +import { resolveMultiOrgEnabled, accumulateSeedSummary } from '@objectstack/types'; import { SeedLoaderService } from './seed-loader.js'; import { loadDisabledPackageIds } from './package-state-store.js'; import type { IMetadataService, II18nService } from '@objectstack/spec/contracts'; @@ -847,21 +847,15 @@ export class AppPlugin implements Plugin { // reach `os dev` output — info is under the default warn // level, and the serve boot-quiet window swallows stdout // — so without this a fixture can lose most of its rows - // with no signal at all. Accumulates across apps. - try { - const kernelRef: any = (ctx as any).kernel; - const prev = (() => { - try { return kernelRef?.getService?.('seed-summary') as any; } catch { return undefined; } - })(); - const summary = { - inserted: (prev?.inserted ?? 0) + totalInserted, - updated: (prev?.updated ?? 0) + totalUpdated, - skipped: (prev?.skipped ?? 0) + totalSkipped, - rejected: (prev?.rejected ?? 0) + totalErrored, - }; - if (kernelRef?.registerService) kernelRef.registerService('seed-summary', summary); - else if (typeof (ctx as any).registerService === 'function') (ctx as any).registerService('seed-summary', summary); - } catch { /* banner summary is best-effort */ } + // with no signal at all. Accumulates across every seed + // source (config apps AND the marketplace rehydrate heal, + // #3430) via the shared register-once-then-mutate counter. + accumulateSeedSummary(ctx as any, { + inserted: totalInserted, + updated: totalUpdated, + skipped: totalSkipped, + rejected: totalErrored, + }); if (result.success) { ctx.logger.info('[Seeder] Seed loading complete', { inserted: totalInserted, diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index ca4ca3701d..b713a1d312 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -2,6 +2,7 @@ export * from './env.js'; export * from './module-not-found.js'; +export * from './seed-summary.js'; // Placeholder for Kernel interface to avoid circular dependency // The actual Kernel implementation will satisfy this interface. diff --git a/packages/types/src/seed-summary.test.ts b/packages/types/src/seed-summary.test.ts new file mode 100644 index 0000000000..c6ffa22c74 --- /dev/null +++ b/packages/types/src/seed-summary.test.ts @@ -0,0 +1,72 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// accumulateSeedSummary is the shared boot-banner seed counter fed from more +// than one place (AppPlugin's config seed AND the marketplace rehydrate heal). +// The tricky part is the kernel it runs against: registerService THROWS on a +// duplicate name, and getService caches — so a naive read-modify-register loses +// the second writer's count entirely (the bug that made the marketplace heal +// invisible in the banner). These pin the register-once-then-mutate contract. + +import { describe, it, expect } from 'vitest'; +import { accumulateSeedSummary, SEED_SUMMARY_SERVICE } from './seed-summary.js'; + +/** Kernel double: registerService throws on duplicate; getService throws when absent. */ +function fakeKernelCtx() { + const services = new Map(); + return { + getService: (name: string) => { + if (!services.has(name)) throw new Error(`Service '${name}' not found`); + return services.get(name); + }, + registerService: (name: string, value: any) => { + if (services.has(name)) throw new Error(`Service '${name}' already registered`); + services.set(name, value); + }, + _peek: () => services.get(SEED_SUMMARY_SERVICE), + }; +} + +describe('accumulateSeedSummary', () => { + it('creates the counter on first use', () => { + const ctx = fakeKernelCtx(); + accumulateSeedSummary(ctx, { inserted: 130, updated: 6 }); + expect(ctx._peek()).toEqual({ inserted: 130, updated: 6, skipped: 0, rejected: 0 }); + }); + + it('ACCUMULATES a second writer instead of throwing on re-register (the marketplace-heal bug)', () => { + const ctx = fakeKernelCtx(); + accumulateSeedSummary(ctx, { inserted: 130, updated: 6 }); // config seed + accumulateSeedSummary(ctx, { inserted: 162 }); // marketplace heal + // Both counts land — the second call must NOT be swallowed by the + // kernel's "already registered" throw. + expect(ctx._peek()).toEqual({ inserted: 292, updated: 6, skipped: 0, rejected: 0 }); + }); + + it('mutates the SAME object a prior reader already holds (cache-safe)', () => { + const ctx = fakeKernelCtx(); + accumulateSeedSummary(ctx, { inserted: 1 }); + const held = ctx.getService(SEED_SUMMARY_SERVICE); // a reader caches this reference + accumulateSeedSummary(ctx, { rejected: 3 }); + // The already-held reference reflects the later write. + expect(held).toEqual({ inserted: 1, updated: 0, skipped: 0, rejected: 3 }); + }); + + it('sums rejected across sources so the banner warning fires on any failure', () => { + const ctx = fakeKernelCtx(); + accumulateSeedSummary(ctx, { inserted: 130 }); + accumulateSeedSummary(ctx, { rejected: 5 }); + expect(ctx._peek().rejected).toBe(5); + }); + + it('is best-effort — never throws even when the ctx cannot store services', () => { + const broken = { + getService: () => { throw new Error('boom'); }, + registerService: () => { throw new Error('nope'); }, + }; + expect(() => accumulateSeedSummary(broken, { inserted: 1 })).not.toThrow(); + }); + + it('tolerates a ctx missing getService/registerService entirely', () => { + expect(() => accumulateSeedSummary({}, { inserted: 1 })).not.toThrow(); + }); +}); diff --git a/packages/types/src/seed-summary.ts b/packages/types/src/seed-summary.ts new file mode 100644 index 0000000000..dc8a364dae --- /dev/null +++ b/packages/types/src/seed-summary.ts @@ -0,0 +1,75 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Boot-banner seed counters (#3415 / #3430). + * + * Data seeding runs inside serve's boot-quiet stdout window, and SeedLoader's + * own logs sit under the default `warn` level — so without a banner line a + * fixture (or a marketplace package) can silently lose most of its rows. The + * CLI reads this counter after `runtime.start()` and prints a `Seeds:` line, + * escalating to a warning when `rejected > 0`. + * + * The counter is fed from more than one place — AppPlugin's config-app seed AND + * the marketplace rehydrate heal — which is exactly why {@link accumulateSeedSummary} + * exists: naive `registerService(name, {…})` twice THROWS ("already + * registered"), and the plugin `getService` cache means a re-registered value + * would be invisible to a reader that already read the old one. So we register + * ONE mutable object and mutate it in place; every writer shares that object + * and the CLI reads the live total. + */ + +/** Running seed totals for the boot banner. */ +export interface SeedSummaryCounters { + inserted: number; + updated: number; + skipped: number; + /** Records dropped by validation/reference errors — the silent-loss case. */ + rejected: number; +} + +/** Kernel service name under which the shared counter object lives. */ +export const SEED_SUMMARY_SERVICE = 'seed-summary'; + +/** + * Add a seed outcome to the shared boot-banner counter, creating it on first + * use. Best-effort and never throws. + * + * Race/ordering safety: the counter is a single object registered once (the + * first writer wins the registration; a loser catches the "already registered" + * throw and fetches the winner's object). Every writer then MUTATES that shared + * object in place, so the value is correct regardless of which seed source runs + * first and independent of the kernel's `getService` cache. + * + * `ctx` is a plugin context exposing `getService` / `registerService` (the + * standard PluginContext — note it has no `.kernel` handle, so callers must not + * reach through one). + */ +export function accumulateSeedSummary( + ctx: { getService?: (name: string) => any; registerService?: (name: string, value: any) => void }, + delta: Partial, +): void { + try { + const read = (): SeedSummaryCounters | undefined => { + // getService throws when the service is absent — treat as "none yet". + try { return ctx.getService?.(SEED_SUMMARY_SERVICE); } catch { return undefined; } + }; + let counter = read(); + if (!counter) { + const fresh: SeedSummaryCounters = { inserted: 0, updated: 0, skipped: 0, rejected: 0 }; + try { + ctx.registerService?.(SEED_SUMMARY_SERVICE, fresh); + counter = fresh; + } catch { + // Lost the registration race — another writer got there first. + // Mutate their object so both writers' counts land. + counter = read() ?? fresh; + } + } + counter.inserted += delta.inserted ?? 0; + counter.updated += delta.updated ?? 0; + counter.skipped += delta.skipped ?? 0; + counter.rejected += delta.rejected ?? 0; + } catch { + /* best-effort — a seed summary must never break a boot */ + } +}