Skip to content

Commit 49c9a1a

Browse files
authored
fix(cli): a config-booted app keeps its module code — action handlers reach the engine again (#4095) (#4129)
`os serve <config>` calls `createStandaloneStack()`, which reads `dist/objectstack.json` and returns 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. 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; serve carried the 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 executes travel — `onEnable` and the `functions` map that string-named hook/job handlers resolve against. `onDisable` is excluded: it is declared in packages/spec but no kernel, runtime or service calls it. - The artifact stays the metadata source of truth. Neither side is a superset (the artifact carries ADR-0046 packaged docs the config never has, 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; it refuses rather than guess. - A bundle's own value always wins, so a host that wrapped itself is untouched. - Code that finds no bundle is reported with a boot warning naming the consequence instead of vanishing — that silent drop is what hid this. Verified end to end: 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 on the shared serve harness. Both fail against the pre-fix command — re-confirmed against merged main, where taking serve.ts from origin/main still reproduces the loss (#4110 fixed the plugin ORDERING; the code loss is a different branch of the same `if`). Closes #4095
1 parent d4720ca commit 49c9a1a

5 files changed

Lines changed: 537 additions & 3 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
---
2+
'@objectstack/cli': patch
3+
---
4+
5+
**A config-booted app no longer loses its `onEnable` — every `script` action's
6+
handler reaches the engine again instead of 404'ing at dispatch (#4095).**
7+
8+
`os serve <config>` calls `createStandaloneStack()`, which reads
9+
`dist/objectstack.json` and returns a ready-made `AppPlugin` for the app. That
10+
satisfied serve's "does the host already wrap itself with an AppPlugin?" guard,
11+
so the `new AppPlugin(config)` built from the LOADED MODULE — the only one
12+
carrying the module's `onEnable` — was skipped. A JSON artifact cannot hold a
13+
function, so the app booted with all of its metadata and none of its code.
14+
15+
On `examples/app-todo` that meant eight declared `script` actions, zero
16+
registered handlers, and every button answering
17+
`404 Action 'complete_task' on object 'todo_task' not found`. The example is
18+
correctly authored: it declares `target: 'completeTask'`, registers
19+
`todo_task:completeTask`, and exports `onEnable`. serve carried that hook intact
20+
all the way to the branch that discarded it.
21+
22+
Serve now grafts the module's executable members onto the app bundle already
23+
registered, rather than dropping them with the wrap:
24+
25+
- Only members `AppPlugin` actually executes travel — `onEnable` and the
26+
`functions` map that string-named hook/job handlers resolve against. (`onDisable`
27+
is deliberately excluded: it is declared in `packages/spec` but no kernel,
28+
runtime or service ever calls it, so grafting it would wire a hook nothing
29+
runs.)
30+
- The artifact stays the metadata source of truth. Neither side is a superset —
31+
the artifact carries compile-time enrichment the config never has (ADR-0046
32+
packaged docs, which serve already grafts the other way) — so this moves code
33+
only, and never metadata.
34+
- Targeting is by `manifest.id`, so a host composing several `AppPlugin`s can
35+
never have one app's handlers attached to another. With no id to match, it
36+
falls back to the single app bundle present and refuses when there are several.
37+
- A bundle's own value always wins, so a host that wrapped itself on purpose is
38+
untouched.
39+
- Code that finds no bundle to land on is now reported with a boot warning naming
40+
the consequence ("they 404 at dispatch") instead of vanishing. That silent drop
41+
is what hid this.
42+
43+
Verified end to end on `examples/app-todo`: `POST /api/v1/actions/todo_task/complete_task`
44+
went from `404 RESOURCE_NOT_FOUND` to `{"success":true}`, `export_csv` now returns
45+
real CSV, and the `[action-governance]` boot warning naming all eight actions is
46+
gone. 14 unit cases pin the graft and — as importantly — the cases where it must
47+
refuse; one end-to-end case boots a real stack through `bin/run-dev.js` and fails
48+
against the pre-fix command.
49+
50+
Note that `os serve <config>` still cannot boot at all when `dist/objectstack.json`
51+
is absent (#4085, `Service 'manifest' is async - use await`). That was verified to
52+
be a **separate** defect on the other side of the same fork, not this one: the
53+
failure reproduces unchanged with this fix applied.

packages/cli/src/commands/serve.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { missingProviderMessage } from '../utils/capability-preflight.js';
1616
import { resolveObjectStackHome } from '@objectstack/runtime';
1717
import { LOG_LEVELS, resolveLogLevel, readLogLevelEnv } from '../utils/log-level.js';
1818
import { BootLogCapture, isVerboseBootLevel } from '../utils/boot-log-capture.js';
19+
import { graftAuthoredRuntimeMembers, isAppPluginLike } from '../utils/graft-runtime-hooks.js';
1920
import { redactConnectionUrl, describeDriverConnection } from '../utils/connection-display.js';
2021
import {
2122
printHeader,
@@ -1047,9 +1048,7 @@ export default class Serve extends Command {
10471048
// To avoid double-registration when the host already wraps itself with
10481049
// an AppPlugin (e.g. apps/objectos's dev-workspace stack), we skip if
10491050
// any plugin in `plugins[]` is already an AppPlugin instance.
1050-
const hasAppPluginAlready = plugins.some(
1051-
(p: any) => p && (p.type === 'app' || p.constructor?.name === 'AppPlugin' || (p.name && typeof p.name === 'string' && p.name.startsWith('plugin.app.')))
1052-
);
1051+
const hasAppPluginAlready = plugins.some(isAppPluginLike);
10531052
const configHasMetadata = !!(
10541053
config.objects || config.manifest || config.apps || config.flows || config.apis
10551054
);
@@ -1085,6 +1084,30 @@ export default class Serve extends Command {
10851084
+ ' Its objects/flows will NOT be served. Fix the config (or pin an AppPlugin in `plugins`).',
10861085
));
10871086
}
1087+
} else if (hasAppPluginAlready) {
1088+
// #4095 — skipping the wrap above also discards the authored module's
1089+
// CODE. On the config-boot path the bundle already in `plugins[]` came
1090+
// from `createStandaloneStack()` reading `dist/objectstack.json`, and a
1091+
// JSON artifact cannot carry a function: the app booted with every
1092+
// `script` action DECLARED and no handler registered, so each one 404'd
1093+
// at dispatch. Move the executable members onto that bundle (the
1094+
// bundle's own value always wins, so a host that wrapped itself on
1095+
// purpose is untouched) and say so out loud when they have nowhere to go
1096+
// — that silent drop is what hid this.
1097+
// Success is silent: on the config-boot path this is now the normal
1098+
// route by which handlers reach the engine, and the observable proof is
1099+
// that the actions dispatch.
1100+
const graft = graftAuthoredRuntimeMembers(plugins, config);
1101+
if (graft.orphaned.length > 0) {
1102+
console.warn(chalk.yellow(
1103+
` ⚠ ${relativeConfig} exports ${graft.orphaned.join(' / ')} but no app bundle claimed `
1104+
+ `${graft.orphaned.length === 1 ? 'it' : 'them'}`
1105+
+ `${graft.reason === 'ambiguous-app-plugin'
1106+
? ' — several apps are registered and the config declares no manifest.id to match'
1107+
: ' — no registered app bundle has a matching manifest.id'}`
1108+
+ '. Action handlers registered there will NOT be reachable (they 404 at dispatch).',
1109+
));
1110+
}
10881111
}
10891112

10901113
// 3b. Auto-register I18nServicePlugin if config contains translations/i18n
Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* framework#4095 — a config-booted app lost its `onEnable`, so every `script`
5+
* action was declared and none was executable.
6+
*
7+
* `os serve <config>` calls `createStandaloneStack()`, which reads
8+
* `dist/objectstack.json` and returns a ready-made `AppPlugin`. That tripped
9+
* serve's "does the host already wrap itself?" guard, so the
10+
* `new AppPlugin(config)` built from the loaded MODULE — the only one carrying
11+
* the module's `onEnable` — never ran. A JSON artifact cannot hold a function,
12+
* so `examples/app-todo` booted with eight action declarations and zero
13+
* handlers and every button answered `404 Action 'complete_task' on object
14+
* 'todo_task' not found`.
15+
*
16+
* These pin the graft that repairs it, and — just as important — the cases where
17+
* it must REFUSE, since attaching one app's handlers to another app's bundle
18+
* would be worse than the bug.
19+
*/
20+
21+
import { describe, it, expect } from 'vitest';
22+
import {
23+
GRAFTABLE_RUNTIME_MEMBERS,
24+
graftAuthoredRuntimeMembers,
25+
isAppPluginLike,
26+
} from './graft-runtime-hooks.js';
27+
28+
/** An AppPlugin as `createStandaloneStack()` builds it: artifact metadata, no code. */
29+
const artifactApp = (id: string, extra: Record<string, unknown> = {}) => ({
30+
name: `plugin.app.${id}`,
31+
type: 'app',
32+
bundle: {
33+
manifest: { id },
34+
objects: [{ name: 'x_task' }],
35+
actions: [{ name: 'do_thing', type: 'script', target: 'doThing' }],
36+
...extra,
37+
},
38+
});
39+
40+
/** The authored module as serve assembles it — metadata PLUS the code half. */
41+
const authoredConfig = (id: string, extra: Record<string, unknown> = {}) => ({
42+
manifest: { id },
43+
objects: [{ name: 'x_task' }],
44+
onEnable: async () => {},
45+
...extra,
46+
});
47+
48+
describe('isAppPluginLike', () => {
49+
it('recognises every shape serve\'s own guard recognised', () => {
50+
// Must stay in lockstep with the guard that decides to SKIP the wrap, or the
51+
// skip and the graft that compensates for it disagree.
52+
expect(isAppPluginLike({ type: 'app' })).toBe(true);
53+
expect(isAppPluginLike({ name: 'plugin.app.com.example.todo' })).toBe(true);
54+
class AppPlugin { }
55+
expect(isAppPluginLike(new AppPlugin())).toBe(true);
56+
});
57+
58+
it('is not fooled by ordinary plugins or non-objects', () => {
59+
expect(isAppPluginLike({ name: 'com.objectstack.metadata' })).toBe(false);
60+
expect(isAppPluginLike({ name: 'com.objectstack.engine.objectql' })).toBe(false);
61+
expect(isAppPluginLike({ type: 'service' })).toBe(false);
62+
expect(isAppPluginLike(null)).toBe(false);
63+
expect(isAppPluginLike(undefined)).toBe(false);
64+
expect(isAppPluginLike('plugin.app.x')).toBe(false);
65+
});
66+
});
67+
68+
describe('graftAuthoredRuntimeMembers', () => {
69+
it('moves onEnable onto the artifact-derived bundle — the #4095 repair', () => {
70+
const app = artifactApp('com.example.todo');
71+
const config = authoredConfig('com.example.todo');
72+
const result = graftAuthoredRuntimeMembers([{ name: 'com.objectstack.metadata' }, app], config);
73+
74+
expect(result.grafted).toEqual(['onEnable']);
75+
expect(result.orphaned).toEqual([]);
76+
expect(result.appId).toBe('com.example.todo');
77+
// The load-bearing assertion: AppPlugin reads this at start().
78+
expect(typeof (app.bundle as any).onEnable).toBe('function');
79+
expect((app.bundle as any).onEnable).toBe(config.onEnable);
80+
});
81+
82+
it('moves the `functions` map too — string-named hook/job handlers are code as well', () => {
83+
const app = artifactApp('com.example.todo');
84+
const config = authoredConfig('com.example.todo', { functions: { doThing: () => 1 } });
85+
const result = graftAuthoredRuntimeMembers([app], config);
86+
87+
expect(result.grafted).toEqual(['onEnable', 'functions']);
88+
expect(typeof (app.bundle as any).functions.doThing).toBe('function');
89+
});
90+
91+
it('grafts nothing the artifact never lost', () => {
92+
// Only executable members travel. Metadata is the artifact's job, and the
93+
// artifact is the richer source (it carries compile-time docs the config
94+
// never has), so nothing else may be copied over it.
95+
const app = artifactApp('com.example.todo', { docs: [{ name: 'readme' }] });
96+
const config = authoredConfig('com.example.todo', { objects: [{ name: 'DIFFERENT' }] });
97+
graftAuthoredRuntimeMembers([app], config);
98+
99+
expect((app.bundle as any).objects).toEqual([{ name: 'x_task' }]);
100+
expect((app.bundle as any).docs).toEqual([{ name: 'readme' }]);
101+
});
102+
103+
it('never overwrites a hook the bundle already has', () => {
104+
// A host that wrapped itself on purpose already decided what runs.
105+
const own = async () => {};
106+
const app = artifactApp('com.example.todo', { onEnable: own });
107+
const result = graftAuthoredRuntimeMembers([app], authoredConfig('com.example.todo'));
108+
109+
expect(result.grafted).toEqual([]);
110+
expect(result.reason).toBe('already-present');
111+
expect((app.bundle as any).onEnable).toBe(own);
112+
});
113+
114+
it('matches by manifest.id, so one app never gets another app\'s handlers', () => {
115+
const todo = artifactApp('com.example.todo');
116+
const crm = artifactApp('com.example.crm');
117+
const result = graftAuthoredRuntimeMembers([todo, crm], authoredConfig('com.example.crm'));
118+
119+
expect(result.grafted).toEqual(['onEnable']);
120+
expect(result.appId).toBe('com.example.crm');
121+
expect((crm.bundle as any).onEnable).toBeTypeOf('function');
122+
expect((todo.bundle as any).onEnable).toBeUndefined();
123+
});
124+
125+
it('refuses rather than guess when the authored id matches nothing', () => {
126+
// Grafting onto "the only candidate" here would wire the wrong app.
127+
const other = artifactApp('com.example.other');
128+
const result = graftAuthoredRuntimeMembers([other], authoredConfig('com.example.todo'));
129+
130+
expect(result.grafted).toEqual([]);
131+
expect(result.orphaned).toEqual(['onEnable']);
132+
expect(result.reason).toBe('no-app-plugin');
133+
expect((other.bundle as any).onEnable).toBeUndefined();
134+
});
135+
136+
it('falls back to the single app bundle when the config declares no manifest id', () => {
137+
const app = artifactApp('com.example.todo');
138+
const result = graftAuthoredRuntimeMembers([app], { objects: [], onEnable: async () => {} });
139+
expect(result.grafted).toEqual(['onEnable']);
140+
});
141+
142+
it('refuses when there is no id AND several candidates', () => {
143+
const result = graftAuthoredRuntimeMembers(
144+
[artifactApp('com.example.todo'), artifactApp('com.example.crm')],
145+
{ objects: [], onEnable: async () => {} },
146+
);
147+
expect(result.grafted).toEqual([]);
148+
expect(result.orphaned).toEqual(['onEnable']);
149+
expect(result.reason).toBe('ambiguous-app-plugin');
150+
});
151+
152+
it('reports orphaned code when no app bundle is registered at all', () => {
153+
// The silent-drop case that hid #4095 — the caller has to be able to shout.
154+
const result = graftAuthoredRuntimeMembers(
155+
[{ name: 'com.objectstack.metadata' }],
156+
authoredConfig('com.example.todo'),
157+
);
158+
expect(result.orphaned).toEqual(['onEnable']);
159+
expect(result.reason).toBe('no-app-plugin');
160+
});
161+
162+
it('stays quiet for a config that carries no code', () => {
163+
const app = artifactApp('com.example.todo');
164+
const result = graftAuthoredRuntimeMembers([app], { manifest: { id: 'com.example.todo' } });
165+
expect(result).toEqual({ grafted: [], orphaned: [], reason: 'no-authored-members' });
166+
});
167+
168+
it('tolerates the degenerate inputs a boot can hand it', () => {
169+
for (const plugins of [undefined, [], [null], [undefined]]) {
170+
expect(() => graftAuthoredRuntimeMembers(plugins as any, authoredConfig('x'))).not.toThrow();
171+
}
172+
for (const authored of [undefined, null, 'nope', 42, {}]) {
173+
expect(graftAuthoredRuntimeMembers([artifactApp('x')], authored).grafted).toEqual([]);
174+
}
175+
// An app plugin with no bundle at all must not throw.
176+
expect(
177+
graftAuthoredRuntimeMembers([{ type: 'app' }], authoredConfig('x')).reason,
178+
).toBe('no-app-plugin');
179+
});
180+
181+
it('grafts only members AppPlugin actually executes', () => {
182+
// `onDisable` is declared in packages/spec but called by no kernel, runtime
183+
// or service — grafting it would wire a hook nothing runs.
184+
expect([...GRAFTABLE_RUNTIME_MEMBERS]).toEqual(['onEnable', 'functions']);
185+
186+
const app = artifactApp('com.example.todo');
187+
graftAuthoredRuntimeMembers([app], authoredConfig('com.example.todo', {
188+
onDisable: async () => {},
189+
}));
190+
expect((app.bundle as any).onDisable).toBeUndefined();
191+
});
192+
});

0 commit comments

Comments
 (0)