Skip to content

Commit 52dcd64

Browse files
os-zhuangclaude
andcommitted
feat(spec): reject script actions with no executable binding + showcase execution tests
Prevents the class of bug fixed in #2169, where `showcase_mark_done` declared `type: 'script'` but carried neither a `body` nor a `target`. AppPlugin only registers an engine handler for actions with a runnable binding, so the action fell through to the `'*'` wildcard lookup and failed at invocation with `Action '<name>' on object '*' not found` — a soft failure that build- and shape-level tests never caught. Two complementary layers: 1. Author/compile-time guard (root cause): `ActionSchema` now requires `body || target` when `type === 'script'` (a `superRefine`, mirroring the existing "non-script types require `target`" rule). `os build` / `defineAction` now reject the broken shape immediately, for every bundle. Verified against the full monorepo build — every shipped bundle still compiles, so this only rejects configurations that were already non-functional at runtime. Existing spec fixtures that relied on the looser rule were updated. 2. Execution-path test (catch-net): examples/app-showcase/test/actions.test.ts drives the real `actionBodyRunnerFactory` + QuickJS sandbox against the shipped actions — asserting Mark Done produces a handler and writes `{ done: true, progress: 100 }`. The prior coverage test only checked that each ActionType *appeared* in the bundle, which is what let #2169 ship. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3d89caa commit 52dcd64

5 files changed

Lines changed: 188 additions & 19 deletions

File tree

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
spec(action): a `script` action must declare an executable binding — reject at
6+
author/compile time when it has neither an inline `body` nor a `target`.
7+
8+
A `type: 'script'` action with no `body` and no `target` registers no runtime
9+
handler: `AppPlugin` skips it, and invoking it falls through to the wildcard
10+
lookup and fails with `Action '<name>' on object '*' not found` (the #2169
11+
"Mark Done" bug). The shape was schema-valid and passed coverage tests, so the
12+
break only surfaced when a user clicked the button.
13+
14+
`ActionSchema` now enforces the invariant via `superRefine`: `script` requires
15+
`body || target` (mirroring the existing "non-script types require `target`"
16+
rule). `body`-bound actions are auto-registered by the runtime; `target`-bound
17+
actions name a function wired imperatively (e.g. via `onEnable`). This only
18+
rejects configurations that were already non-functional at runtime — verified
19+
against the full monorepo build (every shipped bundle still compiles).
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import { describe, it, expect } from 'vitest';
4+
import { actionBodyRunnerFactory, QuickJSScriptRunner } from '@objectstack/runtime';
5+
6+
import { allActions, MarkDoneAction } from '../src/actions/index.js';
7+
8+
/**
9+
* Execution-path coverage for declared actions.
10+
*
11+
* The `coverage.test.ts` check only asserts that every `ActionType` *appears*
12+
* in the bundle — a `type: 'script'` action with no executable handler passes
13+
* it. That blind spot shipped the #2169 bug: `showcase_mark_done` declared
14+
* `type: 'script'` but carried neither a `body` nor a `target`, so AppPlugin
15+
* registered no engine handler and clicking "Mark Done" failed at runtime with
16+
* `Action 'showcase_mark_done' on object '*' not found`.
17+
*
18+
* These tests drive the **real** runtime path — `actionBodyRunnerFactory` +
19+
* the QuickJS sandbox, the exact bridge AppPlugin uses — against the actions as
20+
* shipped. A body that fails to parse, references the wrong field, or is missing
21+
* entirely fails here, not in production.
22+
*/
23+
describe('showcase actions — executability', () => {
24+
const runner = new QuickJSScriptRunner();
25+
26+
it('every declared `script` action is executable (has a body or a target)', () => {
27+
// Mirrors the platform invariant enforced by ActionSchema: a script action
28+
// must be bound to *something* runnable. `target` actions are wired
29+
// imperatively (e.g. via onEnable); `body` actions are auto-registered.
30+
const scriptActions = allActions.filter((a) => a.type === 'script');
31+
expect(scriptActions.length).toBeGreaterThan(0);
32+
for (const a of scriptActions) {
33+
expect(
34+
Boolean((a as { body?: unknown }).body) || Boolean((a as { target?: unknown }).target),
35+
`script action '${a.name}' has neither body nor target — it cannot be invoked`,
36+
).toBe(true);
37+
}
38+
});
39+
40+
it('the runtime produces a handler for Mark Done (regression: #2169)', () => {
41+
const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'showcase' });
42+
const handler = factory(MarkDoneAction as never);
43+
expect(typeof handler).toBe('function');
44+
});
45+
46+
it('Mark Done flips `done` + `progress` via the sandboxed body', async () => {
47+
// Capture what the action writes through the proxied ObjectQL engine.
48+
let written: { object: string; data: Record<string, unknown> } | undefined;
49+
const ql = {
50+
object: (object: string) => ({
51+
update: async (data: Record<string, unknown>) => {
52+
written = { object, data };
53+
return { id: data.id };
54+
},
55+
}),
56+
};
57+
58+
const factory = actionBodyRunnerFactory(runner, { ql, appId: 'showcase' });
59+
const handler = factory(MarkDoneAction as never);
60+
expect(typeof handler).toBe('function');
61+
62+
const result = await handler!({
63+
recordId: 'task_1',
64+
record: { id: 'task_1', status: 'in_progress', progress: 40, done: false },
65+
params: {},
66+
user: { id: 'u1' },
67+
});
68+
69+
// It updates the right object with the completion fields — and deliberately
70+
// does NOT touch `status` (the state-machine only permits in_review -> done).
71+
expect(written?.object).toBe('showcase_task');
72+
expect(written?.data).toMatchObject({ id: 'task_1', done: true, progress: 100 });
73+
expect(written?.data).not.toHaveProperty('status');
74+
expect(result).toEqual({ ok: true, id: 'task_1' });
75+
});
76+
77+
it('a body-less `script` action yields no handler (the #2169 failure mode)', () => {
78+
// Documents exactly what used to ship: with neither body nor target the
79+
// runtime has nothing to register, so the HTTP action route falls into the
80+
// wildcard fallback. ActionSchema now rejects this at author time; this
81+
// asserts the runtime half of the contract.
82+
const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'showcase' });
83+
const handler = factory({ name: 'broken', object: 'showcase_task' } as never);
84+
expect(handler).toBeUndefined();
85+
});
86+
});

packages/spec/src/stack.test.ts

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -728,6 +728,7 @@ describe('defineStack - Map Format Support', () => {
728728
approve_deal: {
729729
label: 'Approve Deal',
730730
type: 'script',
731+
target: 'noop',
731732
},
732733
},
733734
};
@@ -1062,7 +1063,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
10621063
{ name: 'task', fields: { title: { type: 'text' as const } } },
10631064
],
10641065
actions: [
1065-
{ name: 'approve_task', label: 'Approve', objectName: 'task' },
1066+
{ name: 'approve_task', label: 'Approve', objectName: 'task', target: 'noop' },
10661067
],
10671068
};
10681069

@@ -1078,8 +1079,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
10781079
{ name: 'deal', fields: { amount: { type: 'number' as const } } },
10791080
],
10801081
actions: [
1081-
{ name: 'close_deal', label: 'Close Deal', objectName: 'deal' },
1082-
{ name: 'reopen_deal', label: 'Reopen Deal', objectName: 'deal' },
1082+
{ name: 'close_deal', label: 'Close Deal', objectName: 'deal', target: 'noop' },
1083+
{ name: 'reopen_deal', label: 'Reopen Deal', objectName: 'deal', target: 'noop' },
10831084
],
10841085
};
10851086

@@ -1096,8 +1097,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
10961097
{ name: 'project', fields: { name: { type: 'text' as const } } },
10971098
],
10981099
actions: [
1099-
{ name: 'complete_task', label: 'Complete', objectName: 'task' },
1100-
{ name: 'archive_project', label: 'Archive', objectName: 'project' },
1100+
{ name: 'complete_task', label: 'Complete', objectName: 'task', target: 'noop' },
1101+
{ name: 'archive_project', label: 'Archive', objectName: 'project', target: 'noop' },
11011102
],
11021103
};
11031104

@@ -1115,7 +1116,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11151116
{ name: 'task', fields: { title: { type: 'text' as const } } },
11161117
],
11171118
actions: [
1118-
{ name: 'global_action', label: 'Global' },
1119+
{ name: 'global_action', label: 'Global', target: 'noop' },
11191120
],
11201121
};
11211122

@@ -1130,8 +1131,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11301131
{ name: 'task', fields: { title: { type: 'text' as const } } },
11311132
],
11321133
actions: [
1133-
{ name: 'approve_task', label: 'Approve', objectName: 'task' },
1134-
{ name: 'global_search', label: 'Search' },
1134+
{ name: 'approve_task', label: 'Approve', objectName: 'task', target: 'noop' },
1135+
{ name: 'global_search', label: 'Search', target: 'noop' },
11351136
],
11361137
};
11371138

@@ -1149,11 +1150,11 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11491150
{
11501151
name: 'task',
11511152
fields: { title: { type: 'text' as const } },
1152-
actions: [{ name: 'inline_action', label: 'Inline' }],
1153+
actions: [{ name: 'inline_action', label: 'Inline', target: 'noop' }],
11531154
},
11541155
],
11551156
actions: [
1156-
{ name: 'merged_action', label: 'Merged', objectName: 'task' },
1157+
{ name: 'merged_action', label: 'Merged', objectName: 'task', target: 'noop' },
11571158
],
11581159
};
11591160

@@ -1170,7 +1171,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11701171
{ name: 'task', fields: { title: { type: 'text' as const } } },
11711172
],
11721173
actions: [
1173-
{ name: 'approve_task', label: 'Approve', objectName: 'task' },
1174+
{ name: 'approve_task', label: 'Approve', objectName: 'task', target: 'noop' },
11741175
],
11751176
};
11761177

@@ -1186,7 +1187,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11861187
{ name: 'task', fields: { title: { type: 'text' as const } } },
11871188
],
11881189
actions: [
1189-
{ name: 'approve_deal', label: 'Approve', objectName: 'nonexistent_object' },
1190+
{ name: 'approve_deal', label: 'Approve', objectName: 'nonexistent_object', target: 'noop' },
11901191
],
11911192
};
11921193

@@ -1198,7 +1199,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
11981199
const config = {
11991200
manifest: baseManifest,
12001201
actions: [
1201-
{ name: 'approve_deal', label: 'Approve', objectName: 'deal' },
1202+
{ name: 'approve_deal', label: 'Approve', objectName: 'deal', target: 'noop' },
12021203
],
12031204
};
12041205

0 commit comments

Comments
 (0)