fix(cli): a config-booted app keeps its module code — action handlers reach the engine again (#4095) - #4129
Conversation
… reach the engine again (#4095) `os serve <config>` 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 <config>` 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 1 package(s): 18 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…gs-visibility-12kkwh
|
Blocked on a red Verified on pristine git worktree add /tmp/main-check origin/main
cd /tmp/main-check && node scripts/check-wildcard-fallthrough.mjs
# ✗ wildcard fall-through guard (#4116) → 1 problem(s)Cause — a merge-order collision between two landed PRs
'packages/adapters/hono/src/index.ts:all `${prefix}/storage/*`': {
ratchet: '#4117 — terminal, same shape as #4088; adapter has no in-repo consumer',
},Each was green on its own base; together the declaration is stale. Since the guard runs on every PR, this currently blocks all of them, not just this one. Why I have not fixed it hereThe remedy is the two-line deletion the guard's own message asks for, but it is in another change's guard and there is a real choice behind it — drop the stale This branch has Generated by Claude Code |
…ails on main
`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
…gs-visibility-12kkwh # Conflicts: # scripts/check-wildcard-fallthrough.mjs
…oduced #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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HrRNgrWaRtggzmrHpbomyh
Closes #4095.
The loss
os serve <config>callscreateStandaloneStack(), which readsdist/objectstack.jsonand returns a ready-madeAppPluginfor the app. That satisfied serve's "does the host already wrap itself?" guard, so thenew AppPlugin(config)built from the loaded module — the only one carrying the module'sonEnable— was skipped. A JSON artifact cannot hold a function, so the app booted with all of its metadata and none of its code.Two probes nail it. First, what
createStandaloneStack()actually returns once the artifact resolves:Second, serve at the branch that discards it:
The hook survives the entire config chain —
mod.default || mod, the named-export preservation, the docs graft, bothmergeBootConfigcalls — and is then dropped by the guard. Exactly the observed split: metadata survives, code does not.examples/app-todois correctly authored — it declarestarget: 'completeTask', registerstodo_task:completeTask, and exportsonEnable. The ADR-0110 D5 inventory had been naming all eight actions since #4012 made boot warnings visible, and it was right.#4110 has since landed on this same seam from the other side, and its own comment corroborates the diagnosis — "the artifact path never hit it only because
createStandaloneStackappends ITS AppPlugin after the engine". It fixed the ordering; the code loss is untouched by it (see "Re-verified against the new base" below).The fix
Serve grafts the module's executable members onto the app bundle already registered, instead of dropping them along with the wrap.
AppPluginexecutes travels —onEnableand thefunctionsmap that string-named hook/job handlers resolve against.onDisableis deliberately excluded: it is declared inpackages/specbutgrepfinds no kernel, runtime or service that ever calls it, so grafting it would wire a hook nothing runs.manifest.id, so a host composing severalAppPlugins 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 rather than guess when there are several.isAppPluginLikeis shared between the guard and the graft, so the decision to skip the wrap and the compensation for skipping it cannot drift apart.Verification — the user-visible thing actually works
Live HTTP against a real
examples/app-todoboot:POST /api/v1/actions/todo_task/complete_task404 RESOURCE_NOT_FOUND — Action 'complete_task' on object 'todo_task' not found{"success":true}POST …/export_csv{"success":true,"data":"subject,status,priority,…\nLearn ObjectStack,completed,…"}— real seeded rows[action-governance]boot warningThe intermediate state was informative too: with an empty body the handler returned
400 VALIDATION_ERROR — Update requires an ID, i.e. the failure moved from the router into the handler's own logic.Tests
onDisable-must-not-travel, and the degenerate inputs a boot can hand it.bin/run-dev.jswith a fixture whose action is executable only ifonEnableran, and asserts the inventory reports nothing. It also asserts the boot reached its banner, so "no warning" cannot pass vacuously. It uses the sharedtest/helpers/serve-process.tsharness fix(cli,core,metadata,runtime): the platform boots with no application —os serveno longer needs a compiled artifact (#4085) #4110 extracted rather than carrying a second copy.Its
os compilestep is retained, with its justification inverted: it was originally a workaround for config-boot being unable to start without an artifact (#4085, now fixed), and it is kept because the artifact is what makescreateStandaloneStack()contribute the artifact-derivedAppPluginthis regression is about. Without it, serve wraps the config directly,onEnableruns for the trivial reason, and the test would pass while exercising nothing.Re-verified against the new base
mainmoved by 12 commits mid-review (#4110, #4133, #4117, #4120, #4114, #4136 …), including two that touch this exact area, so the whole thing was re-checked rather than assumed:mainin at001c01e.packages/cli/src/commands/serve.tstaken verbatim fromorigin/main, the e2e fails withthe action's handler went unregistered. fix(cli,core,metadata,runtime): the platform boots with no application —os serveno longer needs a compiled artifact (#4085) #4110 fixed the plugin ordering; the code loss is a different branch of the sameif, and remains.eslint srcclean;check:wildcard-fallthroughgreen (7 yielding / 0 ratcheted / 5 exempt).I earlier carried a two-line fix here for the wildcard guard that was failing on
main— that is now dropped: #4133 fixed the same staleMOUNTSentry independently and better (it records why the storage twin disappeared), so the merge takes main's version and my commit contributes nothing. Nothing in this PR touchesscripts/any more.Relationship to #4085 — checked, and they were not the same bug
I verified this before #4085 was fixed:
os serve <config>could not boot withoutdist/objectstack.json, and that reproduced unchanged with this fix applied — a separate defect on the other side of the same fork. #4110 has since fixed it, from the ordering angle, which confirms the split. The verdict and the lead I left are on #4085.