Skip to content

Commit 8cfc0c8

Browse files
os-zhuangclaude
andcommitted
fix(seed-summary): count the marketplace rehydrate heal in the boot banner, and make the counter accumulate
The boot-banner Seeds line (#3435) was fed by a `seed-summary` kernel counter only AppPlugin's config-app seed wrote to. The marketplace rehydrate heal (#3421) seeds an installed package's sample data in the same boot-quiet window but wrote nothing — so a marketplace package's healed rows, and critically its empty "installed but 0 rows" state, were absent from the banner. Two latent bugs blocked simply folding it in: 1. The counter read its prior total via `(ctx as any).kernel?.getService`, but the PluginContext has no `.kernel` handle — the read was always undefined, so every write was a blind overwrite, never an accumulation. 2. registerService throws on a duplicate name, so the second writer's registration threw and was silently dropped. 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 seed-source order. AppPlugin and the marketplace heal both use it. The heal reports healed rows and forces a non-zero rejected on a zero-row heal so "installed but 0 rows" escalates to the banner warning. Verified on showcase: fresh boot reads "292 inserted · 6 updated" (130 config + 162 HotCRM heal) instead of "130 · 6". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 2ba560a commit 8cfc0c8

7 files changed

Lines changed: 276 additions & 19 deletions

File tree

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
"@objectstack/cloud-connection": patch
3+
---
4+
5+
fix(seed-summary): count the marketplace rehydrate heal in the boot banner, and make the counter actually accumulate
6+
7+
The boot-banner `Seeds:` line (#3435) was fed by a `seed-summary` kernel
8+
counter that only AppPlugin's config-app seed wrote to. The marketplace
9+
rehydrate heal (#3421) — which seeds an installed package's sample data onto a
10+
fresh/empty database in the same boot-quiet window — wrote nothing, so a
11+
marketplace package's healed rows, and critically its EMPTY "installed but 0
12+
rows" state, were absent from the banner (the exact class of bug the line
13+
exists to catch). On the showcase the banner read `130 inserted · 6 updated`
14+
while HotCRM silently healed 162 more rows next to it.
15+
16+
Two latent bugs in the counter blocked simply folding the heal in:
17+
18+
1. It read the prior total through `(ctx as any).kernel?.getService(...)`, but
19+
the plugin `PluginContext` has no `.kernel` handle (kernel.ts builds it with
20+
`getService` / `registerService` only) — so the read was always `undefined`
21+
and each write was a blind overwrite, never an accumulation.
22+
2. `registerService(name, …)` THROWS on a duplicate name, so the second writer's
23+
registration threw (caught → silently dropped). Whichever seed source ran
24+
last simply won; combined with (1) the "accumulates across apps" intent
25+
never worked.
26+
27+
Fix: a shared `accumulateSeedSummary(ctx, delta)` in `@objectstack/types`
28+
registers ONE mutable counter object and mutates it in place — race- and
29+
cache-safe regardless of which seed source (config app or marketplace heal)
30+
runs first. Both AppPlugin and the marketplace heal now use it. The marketplace
31+
heal reports healed rows as inserted/updated, and forces a non-zero `rejected`
32+
when it lands zero rows so the "installed but 0 rows" state escalates to the
33+
banner's yellow warning.
34+
35+
Verified on the showcase: fresh boot now reads `292 inserted · 6 updated`
36+
(130 config + 162 HotCRM heal) instead of `130 · 6`.

packages/cloud-connection/src/marketplace-install-local-heal.test.ts

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,17 +53,28 @@ function makeRawApp() {
5353

5454
function makeCtx(rawApp: any, services: Record<string, any>) {
5555
const hooks = new Map<string, any>();
56+
// Dynamically-registered services (e.g. the `seed-summary` counter the boot
57+
// banner reads, #3435). Faithful to the real kernel: getService THROWS when
58+
// absent, registerService THROWS on a duplicate — the exact behaviours
59+
// accumulateSeedSummary's register-once-then-mutate design has to survive.
60+
const registered = new Map<string, any>();
5661
return {
5762
ctx: {
5863
hook: (e: string, h: any) => hooks.set(e, h),
5964
getService: (name: string) => {
6065
if (name === 'http-server') return { getRawApp: () => rawApp };
66+
if (registered.has(name)) return registered.get(name);
6167
const svc = services[name];
6268
if (svc === undefined) throw new Error(`no ${name}`);
6369
return svc;
6470
},
71+
registerService: (name: string, value: any) => {
72+
if (registered.has(name)) throw new Error(`Service '${name}' already registered`);
73+
registered.set(name, value);
74+
},
6575
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
6676
},
77+
seedSummary: () => registered.get('seed-summary'),
6778
fire: async () => { await hooks.get('kernel:ready')?.(); },
6879
};
6980
}
@@ -134,11 +145,11 @@ async function rehydrateWith(entryOverrides: Record<string, any>, findRows: Reco
134145
});
135146
const rawApp = makeRawApp();
136147
const services = makeServices(findRows);
137-
const { ctx, fire } = makeCtx(rawApp, services);
148+
const { ctx, fire, seedSummary } = makeCtx(rawApp, services);
138149
const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
139150
await plugin.start(ctx as any);
140151
await fire();
141-
return { rawApp, services, ctx };
152+
return { rawApp, services, ctx, seedSummary };
142153
}
143154

144155
describe('rehydrate sample-data healing', () => {
@@ -234,3 +245,60 @@ describe('rehydrate sample-data healing', () => {
234245
expect(new LocalManifestSource(dir).read(MANIFEST.id)?.withSampleData).toBe(true);
235246
});
236247
});
248+
249+
// The boot banner's `Seeds:` line (#3435) is fed by a `seed-summary` kernel
250+
// counter that AppPlugin populates for the config-app seed. The marketplace
251+
// rehydrate heal runs in the same boot-quiet window but wrote nothing to it, so
252+
// a marketplace package's healed rows — or, critically, its EMPTY state (the
253+
// original "installed but 0 rows" bug) — were absent from the banner. These pin
254+
// the fold-in.
255+
describe('rehydrate heal feeds the boot-banner seed-summary (#3430 follow-up)', () => {
256+
it('adds healed rows to the seed-summary counter', async () => {
257+
seedResult = { summary: { totalInserted: 3, totalUpdated: 1, totalSkipped: 2 }, errors: [] };
258+
const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] });
259+
expect(seedSummary()).toMatchObject({ inserted: 3, updated: 1, skipped: 2, rejected: 0 });
260+
});
261+
262+
it('escalates an empty heal (0 rows) to a non-zero rejected — the "installed but 0 rows" signal', async () => {
263+
// Empty DB, heal runs, loader lands nothing and reports its failures as
264+
// skips (0 hard errors). The banner must still flag it, so rejected is
265+
// forced to at least 1.
266+
seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
267+
const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] });
268+
expect(seedSummary().rejected).toBeGreaterThanOrEqual(1);
269+
expect(seedSummary().inserted).toBe(0);
270+
});
271+
272+
it('surfaces the loader\'s own error count as rejected when a heal errors', async () => {
273+
seedResult = { summary: { totalInserted: 0, totalUpdated: 0, totalSkipped: 0 }, errors: [{ message: 'locked' }, { message: 'locked' }] };
274+
const { seedSummary } = await rehydrateWith({}, { crm_x: [], crm_y: [] });
275+
expect(seedSummary().rejected).toBe(2);
276+
});
277+
278+
it('does NOT touch the counter when heal is skipped (data already present)', async () => {
279+
const { seedSummary } = await rehydrateWith({}, { crm_x: [{ id: 'a' }], crm_y: [] });
280+
expect(seedSummary()).toBeUndefined();
281+
});
282+
283+
it('does NOT touch the counter for a purged package', async () => {
284+
const { seedSummary } = await rehydrateWith({ sampleDataPurged: true }, { crm_x: [], crm_y: [] });
285+
expect(seedSummary()).toBeUndefined();
286+
});
287+
288+
it('accumulates on top of a config-app seed-summary already on the kernel', async () => {
289+
// Simulate AppPlugin having seeded first: pre-load the counter, then let
290+
// the marketplace heal add to it (the banner shows one combined line).
291+
seedResult = { summary: { totalInserted: 5, totalUpdated: 0, totalSkipped: 0 }, errors: [] };
292+
new LocalManifestSource(dir).write({
293+
packageId: 'pkg_1', versionId: 'pkgv_1', manifestId: MANIFEST.id, version: MANIFEST.version,
294+
manifest: MANIFEST, installedAt: '2026-01-01T00:00:00.000Z', installedBy: 'admin', withSampleData: false,
295+
});
296+
const rawApp = makeRawApp();
297+
const { ctx, fire, seedSummary } = makeCtx(rawApp, makeServices({ crm_x: [], crm_y: [] }));
298+
(ctx as any).registerService('seed-summary', { inserted: 130, updated: 6, skipped: 0, rejected: 0 });
299+
const plugin = new MarketplaceInstallLocalPlugin({ controlPlaneUrl: 'off', storageDir: dir });
300+
await plugin.start(ctx as any);
301+
await fire();
302+
expect(seedSummary()).toMatchObject({ inserted: 135, updated: 6 });
303+
});
304+
});

packages/cloud-connection/src/marketplace-install-local-plugin.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
*/
4343

4444
import type { Plugin, PluginContext } from '@objectstack/core';
45-
import { resolveMultiOrgEnabled } from '@objectstack/types';
45+
import { resolveMultiOrgEnabled, accumulateSeedSummary } from '@objectstack/types';
4646
import { resolveCloudUrl } from './cloud-url.js';
4747
import { resolveMarketplacePublicBaseUrl } from './marketplace-public-url.js';
4848
import { LocalManifestSource, type InstalledManifestEntry } from './local-manifest-source.js';
@@ -253,11 +253,22 @@ export class MarketplaceInstallLocalPlugin implements Plugin {
253253
entry.sampleDataPurged = false;
254254
try { this.ledger.write(entry); } catch { /* non-fatal */ }
255255
ctx.logger?.info?.(`[MarketplaceInstallLocal] healed sample data for ${entry.manifestId}: inserted=${summary.inserted} updated=${summary.updated} errors=${summary.errors}`);
256+
// #3430 follow-up: fold the heal into the boot banner's Seeds
257+
// line (the shared `seed-summary` counter #3435 added for the
258+
// config seed) so a marketplace package's healed rows are
259+
// visible too.
260+
accumulateSeedSummary(ctx as any, { inserted: summary.inserted, updated: summary.updated, skipped: summary.skipped, rejected: summary.errors });
256261
} else {
257262
ctx.logger?.warn?.(`[MarketplaceInstallLocal] sample-data heal for ${entry.manifestId} landed no rows${summary.errorSample ? ` — first error: ${summary.errorSample}` : ''}`);
263+
// The "installed but 0 rows" state — the exact thing the banner
264+
// Seeds line exists to catch. Force a non-zero `rejected` so it
265+
// escalates to the yellow warning even when the loader reported
266+
// its failures as skips rather than hard errors.
267+
accumulateSeedSummary(ctx as any, { skipped: summary.skipped, rejected: Math.max(summary.errors, 1) });
258268
}
259269
} catch (err: any) {
260270
ctx.logger?.warn?.(`[MarketplaceInstallLocal] sample-data heal failed for ${entry.manifestId}: ${err?.message ?? err}`);
271+
accumulateSeedSummary(ctx as any, { rejected: 1 });
261272
}
262273
};
263274

packages/runtime/src/app-plugin.ts

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import { Plugin, PluginContext, wireAuthoredTranslationSync } from '@objectstack/core';
44
import { assertProtocolCompat } from '@objectstack/metadata-core';
5-
import { resolveMultiOrgEnabled } from '@objectstack/types';
5+
import { resolveMultiOrgEnabled, accumulateSeedSummary } from '@objectstack/types';
66
import { SeedLoaderService } from './seed-loader.js';
77
import { loadDisabledPackageIds } from './package-state-store.js';
88
import type { IMetadataService, II18nService } from '@objectstack/spec/contracts';
@@ -847,21 +847,15 @@ export class AppPlugin implements Plugin {
847847
// reach `os dev` output — info is under the default warn
848848
// level, and the serve boot-quiet window swallows stdout
849849
// — so without this a fixture can lose most of its rows
850-
// with no signal at all. Accumulates across apps.
851-
try {
852-
const kernelRef: any = (ctx as any).kernel;
853-
const prev = (() => {
854-
try { return kernelRef?.getService?.('seed-summary') as any; } catch { return undefined; }
855-
})();
856-
const summary = {
857-
inserted: (prev?.inserted ?? 0) + totalInserted,
858-
updated: (prev?.updated ?? 0) + totalUpdated,
859-
skipped: (prev?.skipped ?? 0) + totalSkipped,
860-
rejected: (prev?.rejected ?? 0) + totalErrored,
861-
};
862-
if (kernelRef?.registerService) kernelRef.registerService('seed-summary', summary);
863-
else if (typeof (ctx as any).registerService === 'function') (ctx as any).registerService('seed-summary', summary);
864-
} catch { /* banner summary is best-effort */ }
850+
// with no signal at all. Accumulates across every seed
851+
// source (config apps AND the marketplace rehydrate heal,
852+
// #3430) via the shared register-once-then-mutate counter.
853+
accumulateSeedSummary(ctx as any, {
854+
inserted: totalInserted,
855+
updated: totalUpdated,
856+
skipped: totalSkipped,
857+
rejected: totalErrored,
858+
});
865859
if (result.success) {
866860
ctx.logger.info('[Seeder] Seed loading complete', {
867861
inserted: totalInserted,

packages/types/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
export * from './env.js';
44
export * from './module-not-found.js';
5+
export * from './seed-summary.js';
56

67
// Placeholder for Kernel interface to avoid circular dependency
78
// The actual Kernel implementation will satisfy this interface.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// accumulateSeedSummary is the shared boot-banner seed counter fed from more
4+
// than one place (AppPlugin's config seed AND the marketplace rehydrate heal).
5+
// The tricky part is the kernel it runs against: registerService THROWS on a
6+
// duplicate name, and getService caches — so a naive read-modify-register loses
7+
// the second writer's count entirely (the bug that made the marketplace heal
8+
// invisible in the banner). These pin the register-once-then-mutate contract.
9+
10+
import { describe, it, expect } from 'vitest';
11+
import { accumulateSeedSummary, SEED_SUMMARY_SERVICE } from './seed-summary.js';
12+
13+
/** Kernel double: registerService throws on duplicate; getService throws when absent. */
14+
function fakeKernelCtx() {
15+
const services = new Map<string, any>();
16+
return {
17+
getService: (name: string) => {
18+
if (!services.has(name)) throw new Error(`Service '${name}' not found`);
19+
return services.get(name);
20+
},
21+
registerService: (name: string, value: any) => {
22+
if (services.has(name)) throw new Error(`Service '${name}' already registered`);
23+
services.set(name, value);
24+
},
25+
_peek: () => services.get(SEED_SUMMARY_SERVICE),
26+
};
27+
}
28+
29+
describe('accumulateSeedSummary', () => {
30+
it('creates the counter on first use', () => {
31+
const ctx = fakeKernelCtx();
32+
accumulateSeedSummary(ctx, { inserted: 130, updated: 6 });
33+
expect(ctx._peek()).toEqual({ inserted: 130, updated: 6, skipped: 0, rejected: 0 });
34+
});
35+
36+
it('ACCUMULATES a second writer instead of throwing on re-register (the marketplace-heal bug)', () => {
37+
const ctx = fakeKernelCtx();
38+
accumulateSeedSummary(ctx, { inserted: 130, updated: 6 }); // config seed
39+
accumulateSeedSummary(ctx, { inserted: 162 }); // marketplace heal
40+
// Both counts land — the second call must NOT be swallowed by the
41+
// kernel's "already registered" throw.
42+
expect(ctx._peek()).toEqual({ inserted: 292, updated: 6, skipped: 0, rejected: 0 });
43+
});
44+
45+
it('mutates the SAME object a prior reader already holds (cache-safe)', () => {
46+
const ctx = fakeKernelCtx();
47+
accumulateSeedSummary(ctx, { inserted: 1 });
48+
const held = ctx.getService(SEED_SUMMARY_SERVICE); // a reader caches this reference
49+
accumulateSeedSummary(ctx, { rejected: 3 });
50+
// The already-held reference reflects the later write.
51+
expect(held).toEqual({ inserted: 1, updated: 0, skipped: 0, rejected: 3 });
52+
});
53+
54+
it('sums rejected across sources so the banner warning fires on any failure', () => {
55+
const ctx = fakeKernelCtx();
56+
accumulateSeedSummary(ctx, { inserted: 130 });
57+
accumulateSeedSummary(ctx, { rejected: 5 });
58+
expect(ctx._peek().rejected).toBe(5);
59+
});
60+
61+
it('is best-effort — never throws even when the ctx cannot store services', () => {
62+
const broken = {
63+
getService: () => { throw new Error('boom'); },
64+
registerService: () => { throw new Error('nope'); },
65+
};
66+
expect(() => accumulateSeedSummary(broken, { inserted: 1 })).not.toThrow();
67+
});
68+
69+
it('tolerates a ctx missing getService/registerService entirely', () => {
70+
expect(() => accumulateSeedSummary({}, { inserted: 1 })).not.toThrow();
71+
});
72+
});

packages/types/src/seed-summary.ts

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* Boot-banner seed counters (#3415 / #3430).
5+
*
6+
* Data seeding runs inside serve's boot-quiet stdout window, and SeedLoader's
7+
* own logs sit under the default `warn` level — so without a banner line a
8+
* fixture (or a marketplace package) can silently lose most of its rows. The
9+
* CLI reads this counter after `runtime.start()` and prints a `Seeds:` line,
10+
* escalating to a warning when `rejected > 0`.
11+
*
12+
* The counter is fed from more than one place — AppPlugin's config-app seed AND
13+
* the marketplace rehydrate heal — which is exactly why {@link accumulateSeedSummary}
14+
* exists: naive `registerService(name, {…})` twice THROWS ("already
15+
* registered"), and the plugin `getService` cache means a re-registered value
16+
* would be invisible to a reader that already read the old one. So we register
17+
* ONE mutable object and mutate it in place; every writer shares that object
18+
* and the CLI reads the live total.
19+
*/
20+
21+
/** Running seed totals for the boot banner. */
22+
export interface SeedSummaryCounters {
23+
inserted: number;
24+
updated: number;
25+
skipped: number;
26+
/** Records dropped by validation/reference errors — the silent-loss case. */
27+
rejected: number;
28+
}
29+
30+
/** Kernel service name under which the shared counter object lives. */
31+
export const SEED_SUMMARY_SERVICE = 'seed-summary';
32+
33+
/**
34+
* Add a seed outcome to the shared boot-banner counter, creating it on first
35+
* use. Best-effort and never throws.
36+
*
37+
* Race/ordering safety: the counter is a single object registered once (the
38+
* first writer wins the registration; a loser catches the "already registered"
39+
* throw and fetches the winner's object). Every writer then MUTATES that shared
40+
* object in place, so the value is correct regardless of which seed source runs
41+
* first and independent of the kernel's `getService` cache.
42+
*
43+
* `ctx` is a plugin context exposing `getService` / `registerService` (the
44+
* standard PluginContext — note it has no `.kernel` handle, so callers must not
45+
* reach through one).
46+
*/
47+
export function accumulateSeedSummary(
48+
ctx: { getService?: (name: string) => any; registerService?: (name: string, value: any) => void },
49+
delta: Partial<SeedSummaryCounters>,
50+
): void {
51+
try {
52+
const read = (): SeedSummaryCounters | undefined => {
53+
// getService throws when the service is absent — treat as "none yet".
54+
try { return ctx.getService?.(SEED_SUMMARY_SERVICE); } catch { return undefined; }
55+
};
56+
let counter = read();
57+
if (!counter) {
58+
const fresh: SeedSummaryCounters = { inserted: 0, updated: 0, skipped: 0, rejected: 0 };
59+
try {
60+
ctx.registerService?.(SEED_SUMMARY_SERVICE, fresh);
61+
counter = fresh;
62+
} catch {
63+
// Lost the registration race — another writer got there first.
64+
// Mutate their object so both writers' counts land.
65+
counter = read() ?? fresh;
66+
}
67+
}
68+
counter.inserted += delta.inserted ?? 0;
69+
counter.updated += delta.updated ?? 0;
70+
counter.skipped += delta.skipped ?? 0;
71+
counter.rejected += delta.rejected ?? 0;
72+
} catch {
73+
/* best-effort — a seed summary must never break a boot */
74+
}
75+
}

0 commit comments

Comments
 (0)