From 461b58443c2f90021a22dd3124612a69ba579b35 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 10:44:28 +0000 Subject: [PATCH 1/3] =?UTF-8?q?fix(cli):=20a=20config-booted=20app=20keeps?= =?UTF-8?q?=20its=20module=20code=20=E2=80=94=20action=20handlers=20reach?= =?UTF-8?q?=20the=20engine=20again=20(#4095)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `os serve ` calls `createStandaloneStack()`, which reads `dist/objectstack.json` and returns a ready-made `AppPlugin` for the app. That satisfied serve's "does the host already wrap itself with an AppPlugin?" guard, so the `new AppPlugin(config)` built from the LOADED MODULE — the only one carrying the module's `onEnable` — was skipped. A JSON artifact cannot hold a function, so the app booted with all of its metadata and none of its code. On examples/app-todo that meant eight declared `script` actions, zero registered handlers, and every button answering `404 Action 'complete_task' on object 'todo_task' not found`. The example is correctly authored — it declares `target: 'completeTask'`, registers `todo_task:completeTask`, and exports `onEnable`; serve carried that hook intact all the way to the branch that discarded it. The ADR-0110 D5 inventory had been naming all eight since #4012 made boot warnings visible, and it was right. Serve now grafts the module's executable members onto the app bundle already registered instead of dropping them with the wrap: - Only members AppPlugin actually executes travel — `onEnable` and the `functions` map that string-named hook/job handlers resolve against. `onDisable` is deliberately excluded: it is declared in packages/spec but no kernel, runtime or service ever calls it, so grafting it would wire a hook nothing runs. - The artifact stays the metadata source of truth. Neither side is a superset — it carries compile-time enrichment the config never has (ADR-0046 packaged docs, which serve already grafts the other way) — so this moves code only. - Targeting is by `manifest.id`, so a host composing several AppPlugins can never have one app's handlers attached to another. With no id to match it falls back to the single app bundle present, and refuses when there are several. - A bundle's own value always wins, so a host that wrapped itself on purpose is untouched. - Code that finds no bundle to land on is now reported with a boot warning naming the consequence instead of vanishing. That silent drop is what hid this. `isAppPluginLike` is shared, so the guard that decides to skip the wrap and the graft that compensates for skipping it cannot disagree about what an app plugin is. Verified end to end on examples/app-todo: POST /api/v1/actions/todo_task/ complete_task went from 404 RESOURCE_NOT_FOUND to {"success":true}, export_csv returns real CSV, and the [action-governance] warning naming all eight is gone. 14 unit cases pin the graft and the cases where it must refuse; one end-to-end case boots a real stack through bin/run-dev.js. Both fail against the pre-fix command. CLI suite 86 files / 892 tests green; eslint src clean. `os serve ` still cannot boot without dist/objectstack.json (#4085) — verified to be a separate defect on the other side of the same fork, reproducing unchanged with this fix applied. Closes #4095 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh --- .changeset/app-runtime-hooks-artifact-boot.md | 53 +++++ packages/cli/src/commands/serve.ts | 29 ++- .../cli/src/utils/graft-runtime-hooks.test.ts | 192 ++++++++++++++++++ packages/cli/src/utils/graft-runtime-hooks.ts | 150 ++++++++++++++ .../test/serve-app-runtime-hooks.e2e.test.ts | 181 +++++++++++++++++ 5 files changed, 602 insertions(+), 3 deletions(-) create mode 100644 .changeset/app-runtime-hooks-artifact-boot.md create mode 100644 packages/cli/src/utils/graft-runtime-hooks.test.ts create mode 100644 packages/cli/src/utils/graft-runtime-hooks.ts create mode 100644 packages/cli/test/serve-app-runtime-hooks.e2e.test.ts diff --git a/.changeset/app-runtime-hooks-artifact-boot.md b/.changeset/app-runtime-hooks-artifact-boot.md new file mode 100644 index 0000000000..114e7d1677 --- /dev/null +++ b/.changeset/app-runtime-hooks-artifact-boot.md @@ -0,0 +1,53 @@ +--- +'@objectstack/cli': patch +--- + +**A config-booted app no longer loses its `onEnable` — every `script` action's +handler reaches the engine again instead of 404'ing at dispatch (#4095).** + +`os serve ` calls `createStandaloneStack()`, which reads +`dist/objectstack.json` and returns a ready-made `AppPlugin` for the app. That +satisfied serve's "does the host already wrap itself with an AppPlugin?" guard, +so the `new AppPlugin(config)` built from the LOADED MODULE — the only one +carrying the module's `onEnable` — was skipped. A JSON artifact cannot hold a +function, so the app booted with all of its metadata and none of its code. + +On `examples/app-todo` that meant eight declared `script` actions, zero +registered handlers, and every button answering +`404 Action 'complete_task' on object 'todo_task' not found`. The example is +correctly authored: it declares `target: 'completeTask'`, registers +`todo_task:completeTask`, and exports `onEnable`. serve carried that hook intact +all the way to the branch that discarded it. + +Serve now grafts the module's executable members onto the app bundle already +registered, rather than dropping them with the wrap: + +- Only members `AppPlugin` actually executes travel — `onEnable` and the + `functions` map that string-named hook/job handlers resolve against. (`onDisable` + is deliberately excluded: it is declared in `packages/spec` but no kernel, + runtime or service ever calls it, so grafting it would wire a hook nothing + runs.) +- The artifact stays the metadata source of truth. Neither side is a superset — + the artifact carries compile-time enrichment the config never has (ADR-0046 + packaged docs, which serve already grafts the other way) — so this moves code + only, and never metadata. +- Targeting is by `manifest.id`, so a host composing several `AppPlugin`s can + never have one app's handlers attached to another. With no id to match, it + falls back to the single app bundle present and refuses when there are several. +- A bundle's own value always wins, so a host that wrapped itself on purpose is + untouched. +- Code that finds no bundle to land on is now reported with a boot warning naming + the consequence ("they 404 at dispatch") instead of vanishing. That silent drop + is what hid this. + +Verified end to end on `examples/app-todo`: `POST /api/v1/actions/todo_task/complete_task` +went from `404 RESOURCE_NOT_FOUND` to `{"success":true}`, `export_csv` now returns +real CSV, and the `[action-governance]` boot warning naming all eight actions is +gone. 14 unit cases pin the graft and — as importantly — the cases where it must +refuse; one end-to-end case boots a real stack through `bin/run-dev.js` and fails +against the pre-fix command. + +Note that `os serve ` still cannot boot at all when `dist/objectstack.json` +is absent (#4085, `Service 'manifest' is async - use await`). That was verified to +be a **separate** defect on the other side of the same fork, not this one: the +failure reproduces unchanged with this fix applied. diff --git a/packages/cli/src/commands/serve.ts b/packages/cli/src/commands/serve.ts index d210d4e6ff..46eff69da6 100644 --- a/packages/cli/src/commands/serve.ts +++ b/packages/cli/src/commands/serve.ts @@ -16,6 +16,7 @@ import { missingProviderMessage } from '../utils/capability-preflight.js'; import { resolveObjectStackHome } from '@objectstack/runtime'; import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js'; import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js'; +import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js'; import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js'; import { printHeader, @@ -1014,9 +1015,7 @@ export default class Serve extends Command { // To avoid double-registration when the host already wraps itself with // an AppPlugin (e.g. apps/objectos's dev-workspace stack), we skip if // any plugin in `plugins[]` is already an AppPlugin instance. - const hasAppPluginAlready = plugins.some( - (p: any) => p && (p.type === 'app' || p.constructor?.name === 'AppPlugin' || (p.name && typeof p.name === 'string' && p.name.startsWith('plugin.app.'))) - ); + const hasAppPluginAlready = plugins.some(isAppPluginLike); const configHasMetadata = !!( config.objects || config.manifest || config.apps || config.flows || config.apis ); @@ -1028,6 +1027,30 @@ export default class Serve extends Command { } catch (e: any) { // silent } + } else if (hasAppPluginAlready) { + // #4095 — skipping the wrap above also discards the authored module's + // CODE. On the config-boot path the bundle already in `plugins[]` came + // from `createStandaloneStack()` reading `dist/objectstack.json`, and a + // JSON artifact cannot carry a function: the app booted with every + // `script` action DECLARED and no handler registered, so each one 404'd + // at dispatch. Move the executable members onto that bundle (the + // bundle's own value always wins, so a host that wrapped itself on + // purpose is untouched) and say so out loud when they have nowhere to go + // — that silent drop is what hid this. + // Success is silent: on the config-boot path this is now the normal + // route by which handlers reach the engine, and the observable proof is + // that the actions dispatch. + const graft = graftAuthoredRuntimeMembers(plugins, config); + if (graft.orphaned.length > 0) { + console.warn(chalk.yellow( + ` ⚠ ${relativeConfig} exports ${graft.orphaned.join(' / ')} but no app bundle claimed ` + + `${graft.orphaned.length === 1 ? 'it' : 'them'}` + + `${graft.reason === 'ambiguous-app-plugin' + ? ' — several apps are registered and the config declares no manifest.id to match' + : ' — no registered app bundle has a matching manifest.id'}` + + '. Action handlers registered there will NOT be reachable (they 404 at dispatch).', + )); + } } // 3b. Auto-register I18nServicePlugin if config contains translations/i18n diff --git a/packages/cli/src/utils/graft-runtime-hooks.test.ts b/packages/cli/src/utils/graft-runtime-hooks.test.ts new file mode 100644 index 0000000000..6e6f7ef243 --- /dev/null +++ b/packages/cli/src/utils/graft-runtime-hooks.test.ts @@ -0,0 +1,192 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * framework#4095 — a config-booted app lost its `onEnable`, so every `script` + * action was declared and none was executable. + * + * `os serve ` calls `createStandaloneStack()`, which reads + * `dist/objectstack.json` and returns a ready-made `AppPlugin`. That tripped + * serve's "does the host already wrap itself?" guard, so the + * `new AppPlugin(config)` built from the loaded MODULE — the only one carrying + * the module's `onEnable` — never ran. A JSON artifact cannot hold a function, + * so `examples/app-todo` booted with eight action declarations and zero + * handlers and every button answered `404 Action 'complete_task' on object + * 'todo_task' not found`. + * + * These pin the graft that repairs it, and — just as important — the cases where + * it must REFUSE, since attaching one app's handlers to another app's bundle + * would be worse than the bug. + */ + +import { describe, it, expect } from 'vitest'; +import { + GRAFTABLE_RUNTIME_MEMBERS, + graftAuthoredRuntimeMembers, + isAppPluginLike, +} from './graft-runtime-hooks.js'; + +/** An AppPlugin as `createStandaloneStack()` builds it: artifact metadata, no code. */ +const artifactApp = (id: string, extra: Record = {}) => ({ + name: `plugin.app.${id}`, + type: 'app', + bundle: { + manifest: { id }, + objects: [{ name: 'x_task' }], + actions: [{ name: 'do_thing', type: 'script', target: 'doThing' }], + ...extra, + }, +}); + +/** The authored module as serve assembles it — metadata PLUS the code half. */ +const authoredConfig = (id: string, extra: Record = {}) => ({ + manifest: { id }, + objects: [{ name: 'x_task' }], + onEnable: async () => {}, + ...extra, +}); + +describe('isAppPluginLike', () => { + it('recognises every shape serve\'s own guard recognised', () => { + // Must stay in lockstep with the guard that decides to SKIP the wrap, or the + // skip and the graft that compensates for it disagree. + expect(isAppPluginLike({ type: 'app' })).toBe(true); + expect(isAppPluginLike({ name: 'plugin.app.com.example.todo' })).toBe(true); + class AppPlugin { } + expect(isAppPluginLike(new AppPlugin())).toBe(true); + }); + + it('is not fooled by ordinary plugins or non-objects', () => { + expect(isAppPluginLike({ name: 'com.objectstack.metadata' })).toBe(false); + expect(isAppPluginLike({ name: 'com.objectstack.engine.objectql' })).toBe(false); + expect(isAppPluginLike({ type: 'service' })).toBe(false); + expect(isAppPluginLike(null)).toBe(false); + expect(isAppPluginLike(undefined)).toBe(false); + expect(isAppPluginLike('plugin.app.x')).toBe(false); + }); +}); + +describe('graftAuthoredRuntimeMembers', () => { + it('moves onEnable onto the artifact-derived bundle — the #4095 repair', () => { + const app = artifactApp('com.example.todo'); + const config = authoredConfig('com.example.todo'); + const result = graftAuthoredRuntimeMembers([{ name: 'com.objectstack.metadata' }, app], config); + + expect(result.grafted).toEqual(['onEnable']); + expect(result.orphaned).toEqual([]); + expect(result.appId).toBe('com.example.todo'); + // The load-bearing assertion: AppPlugin reads this at start(). + expect(typeof (app.bundle as any).onEnable).toBe('function'); + expect((app.bundle as any).onEnable).toBe(config.onEnable); + }); + + it('moves the `functions` map too — string-named hook/job handlers are code as well', () => { + const app = artifactApp('com.example.todo'); + const config = authoredConfig('com.example.todo', { functions: { doThing: () => 1 } }); + const result = graftAuthoredRuntimeMembers([app], config); + + expect(result.grafted).toEqual(['onEnable', 'functions']); + expect(typeof (app.bundle as any).functions.doThing).toBe('function'); + }); + + it('grafts nothing the artifact never lost', () => { + // Only executable members travel. Metadata is the artifact's job, and the + // artifact is the richer source (it carries compile-time docs the config + // never has), so nothing else may be copied over it. + const app = artifactApp('com.example.todo', { docs: [{ name: 'readme' }] }); + const config = authoredConfig('com.example.todo', { objects: [{ name: 'DIFFERENT' }] }); + graftAuthoredRuntimeMembers([app], config); + + expect((app.bundle as any).objects).toEqual([{ name: 'x_task' }]); + expect((app.bundle as any).docs).toEqual([{ name: 'readme' }]); + }); + + it('never overwrites a hook the bundle already has', () => { + // A host that wrapped itself on purpose already decided what runs. + const own = async () => {}; + const app = artifactApp('com.example.todo', { onEnable: own }); + const result = graftAuthoredRuntimeMembers([app], authoredConfig('com.example.todo')); + + expect(result.grafted).toEqual([]); + expect(result.reason).toBe('already-present'); + expect((app.bundle as any).onEnable).toBe(own); + }); + + it('matches by manifest.id, so one app never gets another app\'s handlers', () => { + const todo = artifactApp('com.example.todo'); + const crm = artifactApp('com.example.crm'); + const result = graftAuthoredRuntimeMembers([todo, crm], authoredConfig('com.example.crm')); + + expect(result.grafted).toEqual(['onEnable']); + expect(result.appId).toBe('com.example.crm'); + expect((crm.bundle as any).onEnable).toBeTypeOf('function'); + expect((todo.bundle as any).onEnable).toBeUndefined(); + }); + + it('refuses rather than guess when the authored id matches nothing', () => { + // Grafting onto "the only candidate" here would wire the wrong app. + const other = artifactApp('com.example.other'); + const result = graftAuthoredRuntimeMembers([other], authoredConfig('com.example.todo')); + + expect(result.grafted).toEqual([]); + expect(result.orphaned).toEqual(['onEnable']); + expect(result.reason).toBe('no-app-plugin'); + expect((other.bundle as any).onEnable).toBeUndefined(); + }); + + it('falls back to the single app bundle when the config declares no manifest id', () => { + const app = artifactApp('com.example.todo'); + const result = graftAuthoredRuntimeMembers([app], { objects: [], onEnable: async () => {} }); + expect(result.grafted).toEqual(['onEnable']); + }); + + it('refuses when there is no id AND several candidates', () => { + const result = graftAuthoredRuntimeMembers( + [artifactApp('com.example.todo'), artifactApp('com.example.crm')], + { objects: [], onEnable: async () => {} }, + ); + expect(result.grafted).toEqual([]); + expect(result.orphaned).toEqual(['onEnable']); + expect(result.reason).toBe('ambiguous-app-plugin'); + }); + + it('reports orphaned code when no app bundle is registered at all', () => { + // The silent-drop case that hid #4095 — the caller has to be able to shout. + const result = graftAuthoredRuntimeMembers( + [{ name: 'com.objectstack.metadata' }], + authoredConfig('com.example.todo'), + ); + expect(result.orphaned).toEqual(['onEnable']); + expect(result.reason).toBe('no-app-plugin'); + }); + + it('stays quiet for a config that carries no code', () => { + const app = artifactApp('com.example.todo'); + const result = graftAuthoredRuntimeMembers([app], { manifest: { id: 'com.example.todo' } }); + expect(result).toEqual({ grafted: [], orphaned: [], reason: 'no-authored-members' }); + }); + + it('tolerates the degenerate inputs a boot can hand it', () => { + for (const plugins of [undefined, [], [null], [undefined]]) { + expect(() => graftAuthoredRuntimeMembers(plugins as any, authoredConfig('x'))).not.toThrow(); + } + for (const authored of [undefined, null, 'nope', 42, {}]) { + expect(graftAuthoredRuntimeMembers([artifactApp('x')], authored).grafted).toEqual([]); + } + // An app plugin with no bundle at all must not throw. + expect( + graftAuthoredRuntimeMembers([{ type: 'app' }], authoredConfig('x')).reason, + ).toBe('no-app-plugin'); + }); + + it('grafts only members AppPlugin actually executes', () => { + // `onDisable` is declared in packages/spec but called by no kernel, runtime + // or service — grafting it would wire a hook nothing runs. + expect([...GRAFTABLE_RUNTIME_MEMBERS]).toEqual(['onEnable', 'functions']); + + const app = artifactApp('com.example.todo'); + graftAuthoredRuntimeMembers([app], authoredConfig('com.example.todo', { + onDisable: async () => {}, + })); + expect((app.bundle as any).onDisable).toBeUndefined(); + }); +}); diff --git a/packages/cli/src/utils/graft-runtime-hooks.ts b/packages/cli/src/utils/graft-runtime-hooks.ts new file mode 100644 index 0000000000..2237c4d1e6 --- /dev/null +++ b/packages/cli/src/utils/graft-runtime-hooks.ts @@ -0,0 +1,150 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +// --------------------------------------------------------------------------- +// Re-attach an authored config's CODE to the app bundle serve booted from the +// compiled artifact (#4095). +// +// On `os serve `, `createStandaloneStack()` reads +// `dist/objectstack.json` and returns a ready-made `AppPlugin` for the app. +// That satisfied serve's "does the host already wrap itself?" guard, so the +// `new AppPlugin(config)` built from the LOADED MODULE — the only one carrying +// the module's `onEnable` — was skipped. A JSON artifact cannot hold a +// function, so the app booted with all of its metadata and none of its code: +// `examples/app-todo` served eight `script` action declarations and zero +// handlers, and every one of its buttons answered +// `404 Action 'complete_task' on object 'todo_task' not found`. +// +// Neither side is a superset, which is why the answer is a graft rather than a +// swap. The artifact carries compile-time enrichment the config never has (ADR +// -0046 packaged docs — serve already grafts those the other way), and the +// module carries the code the artifact structurally cannot. So the artifact +// stays the metadata source of truth and the module contributes exactly the +// members the runtime EXECUTES. +// +// Deliberately narrow: only members `AppPlugin` actually reads are moved. The +// bundle's own value always wins — a host that wrapped itself on purpose is +// never overwritten — and a config whose code finds no bundle to land on is +// reported rather than dropped, which is the failure mode that made this +// invisible for so long. +// --------------------------------------------------------------------------- + +/** + * Bundle members `AppPlugin` executes, and which therefore cannot survive a + * trip through JSON: + * + * - `onEnable` — invoked at `start()` with the host context (`ctx.ql`), which + * is where apps register action handlers and drivers. + * - `functions` — the name → handler map that string-named hook and job + * handlers (`Hook.handler: 'foo'`) resolve against. + * + * `onDisable` is deliberately absent: it is declared in `packages/spec` but no + * kernel, runtime or service ever calls it, so grafting it would wire a hook + * nothing runs. + */ +export const GRAFTABLE_RUNTIME_MEMBERS = ['onEnable', 'functions'] as const; + +export type GraftableRuntimeMember = (typeof GRAFTABLE_RUNTIME_MEMBERS)[number]; + +export interface RuntimeGraftResult { + /** Members moved onto the target bundle. */ + grafted: GraftableRuntimeMember[]; + /** + * Members the authored config carries that found no bundle to land on — the + * silent-drop case, which the caller must surface rather than swallow. + */ + orphaned: GraftableRuntimeMember[]; + /** Why nothing was grafted, when that is the answer. */ + reason?: 'no-authored-members' | 'no-app-plugin' | 'ambiguous-app-plugin' | 'already-present'; + /** `manifest.id` of the bundle that received the graft. */ + appId?: string; +} + +/** + * serve's "is an app bundle already registered?" predicate. + * + * Kept here so the guard that decides to SKIP `new AppPlugin(config)` and the + * graft that compensates for skipping it can never disagree about what counts + * as an app plugin. + */ +export function isAppPluginLike(plugin: unknown): boolean { + if (!plugin || typeof plugin !== 'object') return false; + const p = plugin as { type?: unknown; name?: unknown; constructor?: { name?: string } }; + if (p.type === 'app') return true; + if (p.constructor?.name === 'AppPlugin') return true; + return typeof p.name === 'string' && p.name.startsWith('plugin.app.'); +} + +/** Whether `source` carries a usable value for `member`. */ +function carries(source: unknown, member: GraftableRuntimeMember): boolean { + if (!source || typeof source !== 'object') return false; + const value = (source as Record)[member]; + if (member === 'onEnable') return typeof value === 'function'; + // `functions` is a map or an array of `{ name, handler }` records. + return typeof value === 'object' && value !== null; +} + +function readManifestId(source: unknown): string | undefined { + if (!source || typeof source !== 'object') return undefined; + const manifest = (source as { manifest?: unknown }).manifest; + if (!manifest || typeof manifest !== 'object') return undefined; + const id = (manifest as { id?: unknown }).id; + return typeof id === 'string' && id.length > 0 ? id : undefined; +} + +/** + * Move the authored module's runtime members onto the already-registered app + * bundle they belong to, and report anything that had nowhere to go. + * + * Target selection is by `manifest.id`, so a host composing several + * `AppPlugin`s can never have one app's handlers attached to another. A config + * with no manifest id at all falls back to the single app bundle present, and + * refuses when there is more than one. + */ +export function graftAuthoredRuntimeMembers( + plugins: readonly unknown[] | undefined, + authored: unknown, +): RuntimeGraftResult { + const authoredMembers = GRAFTABLE_RUNTIME_MEMBERS.filter((m) => carries(authored, m)); + if (authoredMembers.length === 0) { + return { grafted: [], orphaned: [], reason: 'no-authored-members' }; + } + + const candidates = (plugins ?? []).filter(isAppPluginLike) as Array<{ bundle?: unknown }>; + if (candidates.length === 0) { + return { grafted: [], orphaned: authoredMembers, reason: 'no-app-plugin' }; + } + + const authoredId = readManifestId(authored); + let target: { bundle?: unknown } | undefined; + if (authoredId) { + // Identity match only. Falling back to "the only candidate" here would + // graft app A's handlers onto app B whenever the ids disagree. + target = candidates.find((p) => readManifestId(p.bundle) === authoredId); + if (!target) return { grafted: [], orphaned: authoredMembers, reason: 'no-app-plugin' }; + } else if (candidates.length === 1) { + target = candidates[0]; + } else { + return { grafted: [], orphaned: authoredMembers, reason: 'ambiguous-app-plugin' }; + } + + const bundle = target.bundle; + if (!bundle || typeof bundle !== 'object') { + return { grafted: [], orphaned: authoredMembers, reason: 'no-app-plugin' }; + } + + const grafted: GraftableRuntimeMember[] = []; + for (const member of authoredMembers) { + // The bundle's own value always wins: a host that wrapped itself on + // purpose already decided what should run. + if (carries(bundle, member)) continue; + (bundle as Record)[member] = (authored as Record)[member]; + grafted.push(member); + } + + return { + grafted, + orphaned: [], + reason: grafted.length === 0 ? 'already-present' : undefined, + appId: readManifestId(bundle) ?? authoredId, + }; +} diff --git a/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts b/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts new file mode 100644 index 0000000000..50b31b7a2b --- /dev/null +++ b/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts @@ -0,0 +1,181 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * framework#4095 — a config-booted app lost its `onEnable`, so every `script` + * action was DECLARED and none was EXECUTABLE. + * + * `os serve ` calls `createStandaloneStack()`, which reads + * `dist/objectstack.json` and hands back a ready-made `AppPlugin`. That satisfied + * serve's "does the host already wrap itself?" guard, so the + * `new AppPlugin(config)` built from the loaded MODULE — the only one carrying + * the module's `onEnable` — was skipped. A JSON artifact cannot hold a function, + * so the app booted with all of its metadata and none of its code. + * + * Only a test that drives the real command can catch that: the loss happens in + * the join between the artifact-derived stack and the loaded module, and neither + * half is wrong on its own. So this boots a real stack through `bin/run-dev.js` + * and reads its stdout. + * + * The oracle is the ADR-0110 D5 inventory (visible on the CLI since #4012): a + * `script` action whose handler is registered in `onEnable` reconciles as BOUND + * when the hook ran, and is reported under `[action-governance]` when it did + * not. Confirmed to fail against the pre-fix command, naming + * `hookfix_task:do_thing`. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { execFile, spawn } from 'node:child_process'; +import { promisify } from 'node:util'; +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const execFileP = promisify(execFile); +const HERE = resolve(fileURLToPath(import.meta.url), '..'); +const CLI = resolve(HERE, '../bin/run-dev.js'); +const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); + +/** + * An app whose action is only executable if its module-level `onEnable` runs: + * `do_thing` is a `script` action with no `body`, targeting a handler that + * nothing but `onEnable` registers. Compiling this yields an artifact carrying + * the declaration and — being JSON — none of the code. + */ +const CONFIG = ` +export const onEnable = async (ctx) => { + ctx.ql.registerAction('hookfix_task', 'doThing', () => ({ ok: true })); +}; + +export default { + manifest: { + id: 'com.example.hookfix', + namespace: 'hookfix', + version: '1.0.0', + type: 'app', + name: 'Runtime Hook Fixture', + }, + objects: [{ + name: 'hookfix_task', + label: 'Task', + sharingModel: 'private', + fields: { + title: { type: 'text', label: 'Title' }, + }, + actions: [{ + name: 'do_thing', + label: 'Do Thing', + type: 'script', + target: 'doThing', + }], + }], +}; +`; + +interface ServeRun { + stdout: string; + stderr: string; +} + +/** Boot `os serve` in `cwd`, collect output until the banner, then stop it. */ +function runServe( + cwd: string, + args: string[], + opts: { waitFor: RegExp; timeoutMs?: number }, +): Promise { + return new Promise((resolveRun, rejectRun) => { + const child = spawn(TSX, [CLI, 'serve', 'objectstack.config.ts', ...args], { + cwd, + env: { + ...process.env, + NO_COLOR: '1', + OS_DATABASE_URL: ':memory:', + OS_LOG_LEVEL: '', + OS_DISABLE_CONSOLE: '1', + }, + }); + + let stdout = ''; + let stderr = ''; + let settled = false; + + const finish = (err?: Error) => { + if (settled) return; + settled = true; + clearTimeout(timer); + try { + child.kill('SIGTERM'); + } catch { + /* already gone */ + } + if (err) rejectRun(err); + else resolveRun({ stdout, stderr }); + }; + + const timer = setTimeout( + () => + finish( + new Error( + `serve did not reach ${opts.waitFor} in time.\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`, + ), + ), + opts.timeoutMs ?? 180_000, + ); + + child.stdout.on('data', (d) => { + stdout += String(d); + if (opts.waitFor.test(stdout)) finish(); + }); + child.stderr.on('data', (d) => { + stderr += String(d); + }); + child.on('error', (err) => finish(err)); + child.on('exit', () => finish()); + }); +} + +let dir: string; + +beforeAll(async () => { + dir = mkdtempSync(join(tmpdir(), 'os-runtime-hooks-e2e-')); + writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); + // The artifact is what makes this test meaningful — it is the metadata-only + // stack `createStandaloneStack()` boots from, and the reason the module's code + // needed re-attaching. (It is also currently required to boot at all: #4085.) + await execFileP(TSX, [CLI, 'compile'], { + cwd: dir, + maxBuffer: 16 * 1024 * 1024, + env: { ...process.env, NO_COLOR: '1' }, + }); +}, 240_000); + +afterAll(() => { + if (dir) rmSync(dir, { recursive: true, force: true }); +}); + +describe('os serve — an app booted from its artifact keeps its module code (#4095)', () => { + it('runs onEnable, so the declared script action has a handler', async () => { + const port = String(40000 + Math.floor(Math.random() * 20000)); + const { stdout, stderr } = await runServe(dir, ['--port', port], { + waitFor: /Press Ctrl\+C to stop/, + }); + + const seen = `\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`; + + // Sanity: the boot has to have got far enough for the inventory to run at + // all, or "no warning" would be vacuously true. + expect(stdout, `serve never reached its banner${seen}`).toMatch(/Server is ready/); + + // The load-bearing assertion. Pre-fix this reported + // `{"count":1,"actions":["hookfix_task:do_thing"]}` — declared, no handler. + expect(stdout, `the action's handler went unregistered${seen}`).not.toContain( + '[action-governance]', + ); + expect(stdout).not.toContain('hookfix_task:do_thing'); + + // And the config's code was not merely tolerated in silence: nothing should + // report it as orphaned either. + expect(stdout).not.toContain('no app bundle claimed'); + expect(stderr).not.toContain('no app bundle claimed'); + }, 240_000); +}); From 257541d42063528a814a0b1569b354cf47f775ea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:02:01 +0000 Subject: [PATCH 2/3] =?UTF-8?q?fix(guard):=20drop=20the=20wildcard-mount?= =?UTF-8?q?=20entry=20#4112=20deleted=20=E2=80=94=20the=20guard=20fails=20?= =?UTF-8?q?on=20main?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/check-wildcard-fallthrough.mjs` declared the hono adapter's `${prefix}/storage/*` mount, which no longer exists: ✗ wildcard fall-through guard (#4116) packages/adapters/hono/src/index.ts:all `${prefix}/storage/*` DECLARED but not found by the scan. Moved, renamed or deleted? Update MOUNTS. #4112 removed that mount outright — the adapter now carries a "deliberately NOT mounted (#4087)" note in its place — because the wildcard claimed the whole `/storage` subtree and answered its own 404 for every path `service-storage` registers under it. That is the #4088 failure actually happening, so removal was the fix rather than a ratchet. #4112 and #4122 (which added this scan) were each green on their own base and landed in that order, leaving the entry declaring a deleted mount. Reproduced on pristine origin/main, so it is not specific to any branch — and since the guard runs on every PR, it blocks all of them: git worktree add /tmp/main-check origin/main cd /tmp/main-check && node scripts/check-wildcard-fallthrough.mjs # 1 problem(s) Removes the stale entry, which is what the guard's own message asks for. The sibling `${prefix}/auth/*` entry stays: that mount is still there (packages/adapters/hono/src/index.ts:279), so its ratchet is still live. The comment above them said "Both are the #4088 shape" and is now singular, with the storage mount's removal recorded so the next reader does not have to reconstruct why one of a documented pair disappeared. Verified: `pnpm check:wildcard-fallthrough` self-test 15 cases, then 6 yielding / 1 ratcheted / 5 exempt across 12 namespace-claiming mounts. The two steps that follow it in the same CI job and never got to run — check:release-notes, check:node-version — pass too. Refs #4116, #4117, #4112 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh --- scripts/check-wildcard-fallthrough.mjs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/scripts/check-wildcard-fallthrough.mjs b/scripts/check-wildcard-fallthrough.mjs index 0a37024dfd..8145e48097 100644 --- a/scripts/check-wildcard-fallthrough.mjs +++ b/scripts/check-wildcard-fallthrough.mjs @@ -147,17 +147,23 @@ const MOUNTS = { // ── Ratchet: real, tracked, NOT blessed ───────────────────────────────── // - // Both are the #4088 shape in an adapter that no in-repo package depends on, - // so nothing is broken today — but it ships, and ADR-0076's own trigger is an - // out-of-tree embedder (`../objectbase`'s gateway). An embedder mounting a - // route under either prefix hits exactly #4088. Found BY this scan; manual - // greps for the pattern had missed both. Tracked by #4117. + // The #4088 shape in an adapter that no in-repo package depends on, so nothing + // is broken today — but it ships, and ADR-0076's own trigger is an out-of-tree + // embedder (`../objectbase`'s gateway). An embedder mounting a route under + // this prefix hits exactly #4088. Found BY this scan; manual greps for the + // pattern had missed it. Tracked by #4117. + // + // This listed the adapter's `${prefix}/storage/*` mount too, until #4112 + // deleted that mount outright (see the "deliberately NOT mounted (#4087)" + // note in the adapter): the wildcard claimed the whole `/storage` subtree and + // answered its own 404 for every path `service-storage` registers under it — + // the #4088 failure actually happening, so the fix was removal rather than a + // ratchet. #4112 and the PR that added this scan (#4122) were each green on + // their own base and landed in that order, which left the entry declaring a + // mount that no longer existed and failed this guard on `main`. 'packages/adapters/hono/src/index.ts:all `${prefix}/auth/*`': { ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer', }, - 'packages/adapters/hono/src/index.ts:all `${prefix}/storage/*`': { - ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer', - }, }; /** HTTP-verb registrars plus `use`; anything that can claim a path pattern. */ From aaafef521df26f8fbc47fd62a63d54a887c75d7a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 30 Jul 2026 11:29:54 +0000 Subject: [PATCH 3/3] test(cli): move the #4095 e2e onto the shared serve harness main introduced MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #4110 extracted the spawn-the-real-`os serve` harness this test had inlined into `test/helpers/serve-process.ts`; use it instead of carrying a second copy. The `os compile` step stays, but its justification inverts. It was there because config-boot could not start at all without `dist/objectstack.json` (#4085) — now fixed — and it is kept for a positive reason: the artifact is what makes `createStandaloneStack()` contribute the artifact-derived AppPlugin whose missing code this regression is about. Without it serve wraps the config directly, `onEnable` runs for the trivial reason, and the test would pass while exercising nothing. Comment says so, so nobody removes it as leftover #4085 scaffolding. Re-verified against merged main: with `packages/cli/src/commands/serve.ts` taken from origin/main the case still fails ("the action's handler went unregistered"), so #4095 is live on the new base and this remains a real guard. Full CLI suite 88 files / 909 tests; eslint src clean. Refs #4095 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh --- .../test/serve-app-runtime-hooks.e2e.test.ts | 95 +++---------------- 1 file changed, 15 insertions(+), 80 deletions(-) diff --git a/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts b/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts index 50b31b7a2b..0155919f70 100644 --- a/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts +++ b/packages/cli/test/serve-app-runtime-hooks.e2e.test.ts @@ -11,30 +11,25 @@ * the module's `onEnable` — was skipped. A JSON artifact cannot hold a function, * so the app booted with all of its metadata and none of its code. * - * Only a test that drives the real command can catch that: the loss happens in - * the join between the artifact-derived stack and the loaded module, and neither - * half is wrong on its own. So this boots a real stack through `bin/run-dev.js` - * and reads its stdout. + * Only a test that drives the real command can catch it: the loss happens in the + * join between the artifact-derived stack and the loaded module, and neither half + * is wrong on its own. * * The oracle is the ADR-0110 D5 inventory (visible on the CLI since #4012): a * `script` action whose handler is registered in `onEnable` reconciles as BOUND - * when the hook ran, and is reported under `[action-governance]` when it did - * not. Confirmed to fail against the pre-fix command, naming - * `hookfix_task:do_thing`. + * when the hook ran, and is reported under `[action-governance]` when it did not. + * Confirmed to fail against the pre-fix command, naming `hookfix_task:do_thing`. */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; -import { execFile, spawn } from 'node:child_process'; +import { execFile } from 'node:child_process'; import { promisify } from 'node:util'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join, resolve } from 'node:path'; -import { fileURLToPath } from 'node:url'; +import { join } from 'node:path'; +import { runServe, randomPort, CLI, TSX } from './helpers/serve-process.js'; const execFileP = promisify(execFile); -const HERE = resolve(fileURLToPath(import.meta.url), '..'); -const CLI = resolve(HERE, '../bin/run-dev.js'); -const TSX = resolve(HERE, '../../../node_modules/.bin/tsx'); /** * An app whose action is only executable if its module-level `onEnable` runs: @@ -72,76 +67,17 @@ export default { }; `; -interface ServeRun { - stdout: string; - stderr: string; -} - -/** Boot `os serve` in `cwd`, collect output until the banner, then stop it. */ -function runServe( - cwd: string, - args: string[], - opts: { waitFor: RegExp; timeoutMs?: number }, -): Promise { - return new Promise((resolveRun, rejectRun) => { - const child = spawn(TSX, [CLI, 'serve', 'objectstack.config.ts', ...args], { - cwd, - env: { - ...process.env, - NO_COLOR: '1', - OS_DATABASE_URL: ':memory:', - OS_LOG_LEVEL: '', - OS_DISABLE_CONSOLE: '1', - }, - }); - - let stdout = ''; - let stderr = ''; - let settled = false; - - const finish = (err?: Error) => { - if (settled) return; - settled = true; - clearTimeout(timer); - try { - child.kill('SIGTERM'); - } catch { - /* already gone */ - } - if (err) rejectRun(err); - else resolveRun({ stdout, stderr }); - }; - - const timer = setTimeout( - () => - finish( - new Error( - `serve did not reach ${opts.waitFor} in time.\n--- stdout ---\n${stdout}\n--- stderr ---\n${stderr}`, - ), - ), - opts.timeoutMs ?? 180_000, - ); - - child.stdout.on('data', (d) => { - stdout += String(d); - if (opts.waitFor.test(stdout)) finish(); - }); - child.stderr.on('data', (d) => { - stderr += String(d); - }); - child.on('error', (err) => finish(err)); - child.on('exit', () => finish()); - }); -} - let dir: string; beforeAll(async () => { dir = mkdtempSync(join(tmpdir(), 'os-runtime-hooks-e2e-')); writeFileSync(join(dir, 'objectstack.config.ts'), CONFIG, 'utf8'); - // The artifact is what makes this test meaningful — it is the metadata-only - // stack `createStandaloneStack()` boots from, and the reason the module's code - // needed re-attaching. (It is also currently required to boot at all: #4085.) + // The artifact is REQUIRED by this test, and for a positive reason rather than + // as a workaround: it is what makes `createStandaloneStack()` contribute an + // artifact-derived `AppPlugin`, which is the bundle whose missing code this + // regression is about. Without it serve wraps the config directly (#4085/#4110 + // made that path boot), `onEnable` runs for the trivial reason, and the test + // would pass while exercising nothing. await execFileP(TSX, [CLI, 'compile'], { cwd: dir, maxBuffer: 16 * 1024 * 1024, @@ -155,8 +91,7 @@ afterAll(() => { describe('os serve — an app booted from its artifact keeps its module code (#4095)', () => { it('runs onEnable, so the declared script action has a handler', async () => { - const port = String(40000 + Math.floor(Math.random() * 20000)); - const { stdout, stderr } = await runServe(dir, ['--port', port], { + const { stdout, stderr } = await runServe(dir, ['--port', randomPort()], { waitFor: /Press Ctrl\+C to stop/, });