From 7501f39adea6eaf9188e44ddb412e03658c1f436 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 05:21:17 +0000 Subject: [PATCH] fix(runtime): seed persisted disabled packages before the empty-env early return (#5047) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AppPlugin.init` seeds the registry's initial-disabled set from `/package-state/.json` so every registration path installs operator-disabled packages disabled. That seed ran AFTER the empty-env early return, and an empty env (no app payload in the artifact) is exactly the hydration-only scenario: its packages all arrive later from `sys_packages` replay or an HTTP install. Result: on DB-driven environments the initial-disabled set stayed empty and disabled packages came back enabled on every restart. Move the seed above the return, next to the hook/action body runners and the authored-translation sync that are already hoisted for the same reason, and extract it into `seedPersistedDisabledPackages()` with the rationale attached. Tests (the blind spot that hid this — `package-state-store` and `setInitialDisabledPackageIds` had zero coverage repo-wide): - `package-state-store.test.ts` — round trip, per-environment isolation, missing/corrupt file degradation, env-id sanitization. - `app-plugin.disabled-seed.test.ts` — real LiteKernel + ObjectQLPlugin + empty-env AppPlugin over a real state file, asserting post-boot registration and the REAL `PackageServicePlugin` `sys_packages` replay both land disabled; plus the non-empty env's existing behavior. Reverse-verified: with the seed moved back below the return, exactly the two empty-env cases fail (`installed` / `enabled: true`) and the non-empty case stays green. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_018iARDqtrhQgz6fVHDeDkbQ --- .changeset/empty-env-disabled-package-seed.md | 26 ++ packages/runtime/package.json | 1 + .../src/app-plugin.disabled-seed.test.ts | 227 ++++++++++++++++++ packages/runtime/src/app-plugin.ts | 38 ++- .../runtime/src/package-state-store.test.ts | 146 +++++++++++ packages/runtime/vitest.config.ts | 5 + pnpm-lock.yaml | 3 + 7 files changed, 439 insertions(+), 7 deletions(-) create mode 100644 .changeset/empty-env-disabled-package-seed.md create mode 100644 packages/runtime/src/app-plugin.disabled-seed.test.ts create mode 100644 packages/runtime/src/package-state-store.test.ts diff --git a/.changeset/empty-env-disabled-package-seed.md b/.changeset/empty-env-disabled-package-seed.md new file mode 100644 index 0000000000..d14adf1388 --- /dev/null +++ b/.changeset/empty-env-disabled-package-seed.md @@ -0,0 +1,26 @@ +--- +"@objectstack/runtime": patch +--- + +fix(runtime): disabled packages no longer come back enabled after an empty-env restart (#5047) + +An operator who disables a package has that decision persisted to +`/package-state/.json`, and boot replays it by seeding +the registry's initial-disabled set **before** any package is registered — so +every registration path (boot-artifact decomposition, `sys_packages` +rehydration, HTTP install) installs those packages disabled. + +That seed ran inside `AppPlugin.init` **after** the empty-env early return. An +empty environment is one whose artifact carries no app payload — which is +exactly the environment where every package arrives later, from +`PackageServicePlugin`'s Phase 2 replay of `sys_packages` or from an HTTP +install. So on precisely those DB-driven environments the initial-disabled set +stayed empty, and a package the administrator had disabled came back **enabled** +on every restart, with no error anywhere: the disable had persisted correctly, +it was simply never read. + +The seed now runs before that return, alongside the default hook/action body +runners and the authored-translation sync, which are before it for the same +reason. Non-empty environments are unaffected — the seed still lands before the +manifest is decomposed — and the seed remains best-effort, degrading silently on +kernels with no engine. diff --git a/packages/runtime/package.json b/packages/runtime/package.json index 1b36f42724..38693fffff 100644 --- a/packages/runtime/package.json +++ b/packages/runtime/package.json @@ -50,6 +50,7 @@ "@objectstack/service-datasource": "workspace:*", "@objectstack/service-job": "workspace:*", "@objectstack/service-messaging": "workspace:*", + "@objectstack/service-package": "workspace:*", "typescript": "^6.0.3", "vitest": "^4.1.10" }, diff --git a/packages/runtime/src/app-plugin.disabled-seed.test.ts b/packages/runtime/src/app-plugin.disabled-seed.test.ts new file mode 100644 index 0000000000..da4d35a7ee --- /dev/null +++ b/packages/runtime/src/app-plugin.disabled-seed.test.ts @@ -0,0 +1,227 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Persisted package disable-state must survive a restart of an EMPTY env (#5047). + * + * The seed that carries an operator's "disable this package" decision across a + * restart works by filling the registry's initial-disabled set BEFORE the first + * `installPackage` call, so that every registration path installs those + * packages disabled. It used to run AFTER `AppPlugin.init`'s empty-env early + * return — and an empty env (an artifact with no app payload) is precisely the + * environment whose packages ALL arrive later, from `sys_packages` hydration or + * an HTTP install. So on exactly those envs the set stayed empty and every + * disabled package came back ENABLED on each restart. + * + * These tests boot a REAL kernel (LiteKernel + ObjectQLPlugin + AppPlugin) over + * a REAL state file, and drive the REAL `PackageServicePlugin` rehydration, so + * the regression is pinned end to end rather than against a re-implementation. + * + * Reverse verification for the fix: move `seedPersistedDisabledPackages(ctx)` + * back below the `if (this.empty)` return in `app-plugin.ts` and every + * empty-env case here fails (`installed` / `enabled: true`), while the + * non-empty case stays green. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { LiteKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import { PackageServicePlugin } from '@objectstack/service-package'; + +import { AppPlugin } from './app-plugin.js'; +import { setPackageDisabled } from './package-state-store.js'; + +const ENVIRONMENT_ID = 'env_disabled_seed'; +const DISABLED_ID = 'com.acme.reporting'; +const ENABLED_ID = 'com.acme.billing'; + +interface InstalledPackageView { + status?: string; + enabled?: boolean; +} +interface TestRegistry { + installPackage(manifest: Record): unknown; + getPackage(id: string): InstalledPackageView | undefined; +} + +let home: string; +const envSnapshot = { OS_HOME: process.env.OS_HOME, OS_ENVIRONMENT_ID: process.env.OS_ENVIRONMENT_ID }; + +function manifestFor(id: string) { + return { id, name: id, version: '1.0.0', type: 'application' }; +} + +/** + * Boot the composition an empty environment actually runs: the engine plus an + * AppPlugin whose bundle carries no app payload. + */ +async function bootEmptyEnv(): Promise<{ kernel: LiteKernel; registry: TestRegistry }> { + const kernel = new LiteKernel({ logger: { level: 'error' } }); + kernel.use(new ObjectQLPlugin({})); + kernel.use(new AppPlugin({}, { environmentId: ENVIRONMENT_ID, organizationId: 'org_test' })); + await kernel.bootstrap(); + const ql = kernel.getService<{ registry: TestRegistry }>('objectql'); + return { kernel, registry: ql.registry }; +} + +/** A PluginContext for PackageServicePlugin whose engine shares `registry`. */ +function packageServiceCtx(registry: TestRegistry, rows: Array>) { + const execute = vi.fn(async ({ sql }: { sql: string }) => { + if (/SELECT \* FROM sys_packages/i.test(sql)) return { rows }; + return { rows: [] }; // CREATE TABLE / INDEX / … + }); + const services = new Map([['objectql', { execute, registry }]]); + return { + logger: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn() }, + getService: (n: string) => services.get(n), + registerService: (n: string, s: unknown) => services.set(n, s), + } as never; +} + +function sysPackagesRow(manifest: Record) { + return { + id: manifest.id, + version: manifest.version, + manifest: JSON.stringify(manifest), + metadata: '{}', + hash: 'h', + created_at: 't', + updated_at: 't', + }; +} + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), 'os-disabled-seed-')); + process.env.OS_HOME = home; + delete process.env.OS_ENVIRONMENT_ID; +}); + +afterEach(() => { + rmSync(home, { recursive: true, force: true }); + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + if (envSnapshot.OS_ENVIRONMENT_ID === undefined) delete process.env.OS_ENVIRONMENT_ID; + else process.env.OS_ENVIRONMENT_ID = envSnapshot.OS_ENVIRONMENT_ID; +}); + +describe('empty-env boot seeds persisted package disable-state (#5047)', () => { + it('a package registered after boot lands DISABLED — the hydration-only path', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); // operator disabled it last run + + const { kernel, registry } = await bootEmptyEnv(); + // Nothing came from the (empty) artifact; this is the post-boot + // registration every package in such an env goes through. + registry.installPackage(manifestFor(DISABLED_ID)); + + expect(registry.getPackage(DISABLED_ID)).toMatchObject({ + status: 'disabled', + enabled: false, + }); + + await kernel.shutdown(); + }); + + it('seeds only the persisted ids — other packages still install enabled', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); + + const { kernel, registry } = await bootEmptyEnv(); + registry.installPackage(manifestFor(ENABLED_ID)); + + expect(registry.getPackage(ENABLED_ID)).toMatchObject({ + status: 'installed', + enabled: true, + }); + + await kernel.shutdown(); + }); + + it('a package replayed from sys_packages by PackageServicePlugin lands DISABLED', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); + + // Phase 1: the empty-env kernel boots and seeds the registry. + const { kernel, registry } = await bootEmptyEnv(); + + // Phase 2: the real rehydration replays the durable row into that same + // registry (ADR-0033 consolidation). + await new PackageServicePlugin().start( + packageServiceCtx(registry, [ + sysPackagesRow(manifestFor(DISABLED_ID)), + sysPackagesRow(manifestFor(ENABLED_ID)), + ]), + ); + + expect(registry.getPackage(DISABLED_ID)).toMatchObject({ + status: 'disabled', + enabled: false, + }); + expect(registry.getPackage(ENABLED_ID)).toMatchObject({ + status: 'installed', + enabled: true, + }); + + await kernel.shutdown(); + }); + + it('re-enabling clears the persisted state — the package comes back enabled', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, false); + + const { kernel, registry } = await bootEmptyEnv(); + registry.installPackage(manifestFor(DISABLED_ID)); + + expect(registry.getPackage(DISABLED_ID)).toMatchObject({ + status: 'installed', + enabled: true, + }); + + await kernel.shutdown(); + }); + + it('boots an empty env with no persisted state at all (nothing to seed)', async () => { + const { kernel, registry } = await bootEmptyEnv(); + registry.installPackage(manifestFor(DISABLED_ID)); + + expect(registry.getPackage(DISABLED_ID)).toMatchObject({ enabled: true }); + + await kernel.shutdown(); + }); + + // Guards the other direction of the reorder: moving the seed earlier must + // not change what a NON-empty env already did. + it('non-empty env keeps its existing behavior — bundle package installs disabled', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); + + const kernel = new LiteKernel({ logger: { level: 'error' } }); + kernel.use(new ObjectQLPlugin({})); + kernel.use( + new AppPlugin( + { id: DISABLED_ID, name: DISABLED_ID, version: '1.0.0', objects: [] }, + { environmentId: ENVIRONMENT_ID, organizationId: 'org_test' }, + ), + ); + await kernel.bootstrap(); + + const registry = kernel.getService<{ registry: TestRegistry }>('objectql').registry; + expect(registry.getPackage(DISABLED_ID)).toMatchObject({ + status: 'disabled', + enabled: false, + }); + + await kernel.shutdown(); + }); + + // The seed resolves `objectql` through getService; a kernel without an + // engine (metadata-only one-shot commands) must still boot. + it('degrades silently on a kernel with no engine at all', async () => { + setPackageDisabled(ENVIRONMENT_ID, DISABLED_ID, true); + + const kernel = new LiteKernel({ logger: { level: 'error' } }); + kernel.use(new AppPlugin({}, { environmentId: ENVIRONMENT_ID, organizationId: 'org_test' })); + + await expect(kernel.bootstrap()).resolves.toBeUndefined(); + await kernel.shutdown(); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index c0b35a2351..465c02bddb 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -193,6 +193,16 @@ export class AppPlugin implements Plugin { // up with (the core in-memory fallback included); idempotent across // multiple wirers via the ownership marker in core. wireAuthoredTranslationSync(ctx as any); + // Seed persisted package disable-state — also BEFORE the empty-env + // return (#5047). An empty env is EXACTLY the hydration-only scenario: + // the artifact ships no app payload, so every package in that + // environment arrives later from `sys_packages` (PackageServicePlugin's + // Phase 2 rehydrate) or from an HTTP install. Seeding after the return + // meant the registry's initial-disabled set stayed empty on those + // envs, and a package an operator had disabled came back ENABLED on + // every restart. The seed must land before ANY registration path runs, + // which is Phase 1, unconditionally. + this.seedPersistedDisabledPackages(ctx); if (this.empty) { ctx.logger.debug('[AppPlugin] empty env — no app payload, skipping init', { pluginName: this.name, @@ -223,11 +233,27 @@ export class AppPlugin implements Plugin { ? { ...this.bundle.manifest, ...this.bundle } : this.bundle; - // Seed persisted package disable-state into the registry BEFORE the - // manifest is decomposed, so disabled packages are installed disabled - // and stay hidden after restart. Honors every later registration path - // (boot artifact, marketplace rehydrate, import) via the registry's - // initial-disabled set. Best-effort — never block boot on this. + ctx.getService<{ register(m: any): void }>('manifest').register(servicePayload); + } + + /** + * Seed persisted package disable-state into the registry's initial-disabled + * set, so every later registration path — boot artifact decomposition, + * marketplace / `sys_packages` rehydrate, local import — installs those + * packages DISABLED and they stay hidden after a restart. + * + * Runs in init (Phase 1) and BEFORE the empty-env return (#5047), for the + * same reason the runners above do: the seed only works if it is in place + * before the FIRST `installPackage` call, and on an empty env every + * package arrives from Phase 2 hydration rather than from this bundle. + * On a non-empty env it still lands before the manifest is decomposed, + * because that decomposition happens at the `manifest.register()` call at + * the end of init. + * + * Best-effort — never block boot on this. Degrades silently on kernels + * with no engine (metadata-only one-shot commands, mock-engine tests). + */ + private seedPersistedDisabledPackages(ctx: PluginContext): void { try { const ql = ctx.getService<{ registry?: { setInitialDisabledPackageIds?: (ids: Iterable) => void } }>('objectql'); const setter = ql?.registry?.setInitialDisabledPackageIds; @@ -246,8 +272,6 @@ export class AppPlugin implements Plugin { error: (err as Error)?.message ?? String(err), }); } - - ctx.getService<{ register(m: any): void }>('manifest').register(servicePayload); } /** diff --git a/packages/runtime/src/package-state-store.test.ts b/packages/runtime/src/package-state-store.test.ts new file mode 100644 index 0000000000..f3ead4d521 --- /dev/null +++ b/packages/runtime/src/package-state-store.test.ts @@ -0,0 +1,146 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `package-state-store` unit contract (#5047). + * + * This store is the ONLY durable record of which packages an operator has + * disabled — the registry itself is in-memory and re-registers every package + * as enabled on each boot. It had zero test coverage, which is why the + * empty-env seed regression (see `app-plugin.disabled-seed.test.ts`) could + * exist unnoticed: nothing asserted either half of the round trip. + * + * The invariants pinned here: the round trip, per-environment isolation, and + * the two "never crash boot" degradations (missing file, corrupt file). + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { loadDisabledPackageIds, setPackageDisabled } from './package-state-store.js'; + +let home: string; +const envSnapshot = { OS_HOME: process.env.OS_HOME, OS_ENVIRONMENT_ID: process.env.OS_ENVIRONMENT_ID }; + +/** Absolute path of the state file the store is expected to use. */ +function stateFile(environmentId: string): string { + return join(home, 'package-state', `${environmentId}.json`); +} + +beforeEach(() => { + home = mkdtempSync(join(tmpdir(), 'os-package-state-')); + process.env.OS_HOME = home; + delete process.env.OS_ENVIRONMENT_ID; +}); + +afterEach(() => { + rmSync(home, { recursive: true, force: true }); + if (envSnapshot.OS_HOME === undefined) delete process.env.OS_HOME; + else process.env.OS_HOME = envSnapshot.OS_HOME; + if (envSnapshot.OS_ENVIRONMENT_ID === undefined) delete process.env.OS_ENVIRONMENT_ID; + else process.env.OS_ENVIRONMENT_ID = envSnapshot.OS_ENVIRONMENT_ID; +}); + +describe('package-state-store', () => { + it('returns an empty set when no state file exists', () => { + expect(loadDisabledPackageIds('env_local')).toEqual(new Set()); + }); + + it('round-trips a disable through the file and back', () => { + setPackageDisabled('env_local', 'com.acme.reporting', true); + + expect(loadDisabledPackageIds('env_local')).toEqual(new Set(['com.acme.reporting'])); + expect(JSON.parse(readFileSync(stateFile('env_local'), 'utf8'))).toEqual({ + disabled: ['com.acme.reporting'], + }); + }); + + it('re-enabling removes the id (and leaves the others alone)', () => { + setPackageDisabled('env_local', 'com.acme.reporting', true); + setPackageDisabled('env_local', 'com.acme.billing', true); + + setPackageDisabled('env_local', 'com.acme.reporting', false); + + expect(loadDisabledPackageIds('env_local')).toEqual(new Set(['com.acme.billing'])); + }); + + it('accumulates disables across calls and persists them sorted', () => { + setPackageDisabled('env_local', 'com.acme.zeta', true); + setPackageDisabled('env_local', 'com.acme.alpha', true); + + expect(JSON.parse(readFileSync(stateFile('env_local'), 'utf8')).disabled).toEqual([ + 'com.acme.alpha', + 'com.acme.zeta', + ]); + }); + + it('disabling the same id twice is idempotent', () => { + setPackageDisabled('env_local', 'com.acme.reporting', true); + setPackageDisabled('env_local', 'com.acme.reporting', true); + + expect(JSON.parse(readFileSync(stateFile('env_local'), 'utf8')).disabled).toEqual([ + 'com.acme.reporting', + ]); + }); + + it('re-enabling something that was never disabled is a no-op, not a crash', () => { + setPackageDisabled('env_local', 'com.acme.never', false); + + expect(loadDisabledPackageIds('env_local')).toEqual(new Set()); + }); + + // The whole reason the file is keyed by environment: a disable in staging + // must not hide the package in production (or in a sibling local env). + it('isolates state per environment', () => { + setPackageDisabled('env_staging', 'com.acme.reporting', true); + + expect(loadDisabledPackageIds('env_staging')).toEqual(new Set(['com.acme.reporting'])); + expect(loadDisabledPackageIds('env_production')).toEqual(new Set()); + }); + + it('falls back to OS_ENVIRONMENT_ID, then to `default`, when no id is passed', () => { + process.env.OS_ENVIRONMENT_ID = 'env_from_env_var'; + setPackageDisabled(undefined, 'com.acme.reporting', true); + expect(loadDisabledPackageIds()).toEqual(new Set(['com.acme.reporting'])); + expect(readFileSync(stateFile('env_from_env_var'), 'utf8')).toContain('com.acme.reporting'); + + delete process.env.OS_ENVIRONMENT_ID; + expect(loadDisabledPackageIds()).toEqual(new Set()); + setPackageDisabled(undefined, 'com.acme.billing', true); + expect(readFileSync(stateFile('default'), 'utf8')).toContain('com.acme.billing'); + }); + + // An environment id reaches this store from config / env vars, so it is not + // guaranteed path-safe. It must never escape the package-state directory. + it('sanitizes environment ids into a single flat file name', () => { + setPackageDisabled('../../etc/evil', 'com.acme.reporting', true); + + expect(loadDisabledPackageIds('../../etc/evil')).toEqual(new Set(['com.acme.reporting'])); + expect(readFileSync(stateFile('.._.._etc_evil'), 'utf8')).toContain('com.acme.reporting'); + }); + + // Boot reads this file. A half-written or hand-edited file must degrade to + // "nothing disabled" rather than throwing inside plugin init. + it('treats a corrupt state file as empty instead of throwing', () => { + mkdirSync(join(home, 'package-state'), { recursive: true }); + writeFileSync(stateFile('env_local'), '{ this is not json', 'utf8'); + + expect(() => loadDisabledPackageIds('env_local')).not.toThrow(); + expect(loadDisabledPackageIds('env_local')).toEqual(new Set()); + }); + + it('ignores a non-object payload and non-string entries', () => { + mkdirSync(join(home, 'package-state'), { recursive: true }); + writeFileSync(stateFile('env_local'), '"just a string"', 'utf8'); + expect(loadDisabledPackageIds('env_local')).toEqual(new Set()); + + writeFileSync(stateFile('env_local'), JSON.stringify({ disabled: ['ok', 42, null] }), 'utf8'); + expect(loadDisabledPackageIds('env_local')).toEqual(new Set(['ok'])); + }); + + it('creates the package-state directory on first write', () => { + expect(() => setPackageDisabled('env_local', 'com.acme.reporting', true)).not.toThrow(); + expect(readFileSync(stateFile('env_local'), 'utf8')).toContain('com.acme.reporting'); + }); +}); diff --git a/packages/runtime/vitest.config.ts b/packages/runtime/vitest.config.ts index a9066e43f4..4cb8dc468d 100644 --- a/packages/runtime/vitest.config.ts +++ b/packages/runtime/vitest.config.ts @@ -36,6 +36,11 @@ export default defineConfig({ // the #4567 regression (croner rejecting the expression envelope) is // reproduced by the actual scheduler rather than by a double. '@objectstack/service-job': path.resolve(__dirname, '../services/service-job/src/index.ts'), + // Dev-only: app-plugin.disabled-seed.test.ts drives the REAL + // `sys_packages` → registry rehydration (#5047), so the empty-env seed + // regression is proven against the actual hydration code rather than a + // re-implementation of it. + '@objectstack/service-package': path.resolve(__dirname, '../services/service-package/src/index.ts'), }, }, test: { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 15c90e3978..2707e8d4e4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1882,6 +1882,9 @@ importers: '@objectstack/service-messaging': specifier: workspace:* version: link:../services/service-messaging + '@objectstack/service-package': + specifier: workspace:* + version: link:../services/service-package typescript: specifier: ^6.0.3 version: 6.0.3