Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions .changeset/action-script-executable-guard.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
---
"@objectstack/spec": minor
---

spec(action): a `script` action must declare an executable binding — reject at
author/compile time when it has neither an inline `body` nor a `target`.

A `type: 'script'` action with no `body` and no `target` registers no runtime
handler: `AppPlugin` skips it, and invoking it falls through to the wildcard
lookup and fails with `Action '<name>' on object '*' not found` (the #2169
"Mark Done" bug). The shape was schema-valid and passed coverage tests, so the
break only surfaced when a user clicked the button.

`ActionSchema` now enforces the invariant via `superRefine`: `script` requires
`body || target` (mirroring the existing "non-script types require `target`"
rule). `body`-bound actions are auto-registered by the runtime; `target`-bound
actions name a function wired imperatively (e.g. via `onEnable`). This only
rejects configurations that were already non-functional at runtime — verified
against the full monorepo build (every shipped bundle still compiles).
86 changes: 86 additions & 0 deletions examples/app-showcase/test/actions.test.ts
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.

import { describe, it, expect } from 'vitest';
import { actionBodyRunnerFactory, QuickJSScriptRunner } from '@objectstack/runtime';

import { allActions, MarkDoneAction } from '../src/actions/index.js';

/**
* Execution-path coverage for declared actions.
*
* The `coverage.test.ts` check only asserts that every `ActionType` *appears*
* in the bundle — a `type: 'script'` action with no executable handler passes
* it. That blind spot shipped the #2169 bug: `showcase_mark_done` declared
* `type: 'script'` but carried neither a `body` nor a `target`, so AppPlugin
* registered no engine handler and clicking "Mark Done" failed at runtime with
* `Action 'showcase_mark_done' on object '*' not found`.
*
* These tests drive the **real** runtime path — `actionBodyRunnerFactory` +
* the QuickJS sandbox, the exact bridge AppPlugin uses — against the actions as
* shipped. A body that fails to parse, references the wrong field, or is missing
* entirely fails here, not in production.
*/
describe('showcase actions — executability', () => {
const runner = new QuickJSScriptRunner();

it('every declared `script` action is executable (has a body or a target)', () => {
// Mirrors the platform invariant enforced by ActionSchema: a script action
// must be bound to *something* runnable. `target` actions are wired
// imperatively (e.g. via onEnable); `body` actions are auto-registered.
const scriptActions = allActions.filter((a) => a.type === 'script');
expect(scriptActions.length).toBeGreaterThan(0);
for (const a of scriptActions) {
expect(
Boolean((a as { body?: unknown }).body) || Boolean((a as { target?: unknown }).target),
`script action '${a.name}' has neither body nor target — it cannot be invoked`,
).toBe(true);
}
});

it('the runtime produces a handler for Mark Done (regression: #2169)', () => {
const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'showcase' });
const handler = factory(MarkDoneAction as never);
expect(typeof handler).toBe('function');
});

it('Mark Done flips `done` + `progress` via the sandboxed body', async () => {
// Capture what the action writes through the proxied ObjectQL engine.
let written: { object: string; data: Record<string, unknown> } | undefined;
const ql = {
object: (object: string) => ({
update: async (data: Record<string, unknown>) => {
written = { object, data };
return { id: data.id };
},
}),
};

const factory = actionBodyRunnerFactory(runner, { ql, appId: 'showcase' });
const handler = factory(MarkDoneAction as never);
expect(typeof handler).toBe('function');

const result = await handler!({
recordId: 'task_1',
record: { id: 'task_1', status: 'in_progress', progress: 40, done: false },
params: {},
user: { id: 'u1' },
});

// It updates the right object with the completion fields — and deliberately
// does NOT touch `status` (the state-machine only permits in_review -> done).
expect(written?.object).toBe('showcase_task');
expect(written?.data).toMatchObject({ id: 'task_1', done: true, progress: 100 });
expect(written?.data).not.toHaveProperty('status');
expect(result).toEqual({ ok: true, id: 'task_1' });
});

it('a body-less `script` action yields no handler (the #2169 failure mode)', () => {
// Documents exactly what used to ship: with neither body nor target the
// runtime has nothing to register, so the HTTP action route falls into the
// wildcard fallback. ActionSchema now rejects this at author time; this
// asserts the runtime half of the contract.
const factory = actionBodyRunnerFactory(runner, { ql: {}, appId: 'showcase' });
const handler = factory({ name: 'broken', object: 'showcase_task' } as never);
expect(handler).toBeUndefined();
});
});
27 changes: 14 additions & 13 deletions packages/spec/src/stack.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -728,6 +728,7 @@ describe('defineStack - Map Format Support', () => {
approve_deal: {
label: 'Approve Deal',
type: 'script',
target: 'noop',
},
},
};
Expand DownExpand Up@@ -1062,7 +1063,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{ name: 'task', fields: { title: { type: 'text' as const } } },
],
actions: [
{ name: 'approve_task', label: 'Approve', objectName: 'task' },
{ name: 'approve_task', label: 'Approve', objectName: 'task', target: 'noop' },
],
};

Expand All@@ -1078,8 +1079,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{ name: 'deal', fields: { amount: { type: 'number' as const } } },
],
actions: [
{ name: 'close_deal', label: 'Close Deal', objectName: 'deal' },
{ name: 'reopen_deal', label: 'Reopen Deal', objectName: 'deal' },
{ name: 'close_deal', label: 'Close Deal', objectName: 'deal', target: 'noop' },
{ name: 'reopen_deal', label: 'Reopen Deal', objectName: 'deal', target: 'noop' },
],
};

Expand All@@ -1096,8 +1097,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{ name: 'project', fields: { name: { type: 'text' as const } } },
],
actions: [
{ name: 'complete_task', label: 'Complete', objectName: 'task' },
{ name: 'archive_project', label: 'Archive', objectName: 'project' },
{ name: 'complete_task', label: 'Complete', objectName: 'task', target: 'noop' },
{ name: 'archive_project', label: 'Archive', objectName: 'project', target: 'noop' },
],
};

Expand All@@ -1115,7 +1116,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{ name: 'task', fields: { title: { type: 'text' as const } } },
],
actions: [
{ name: 'global_action', label: 'Global' },
{ name: 'global_action', label: 'Global', target: 'noop' },
],
};

Expand All@@ -1130,8 +1131,8 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{ name: 'task', fields: { title: { type: 'text' as const } } },
],
actions: [
{ name: 'approve_task', label: 'Approve', objectName: 'task' },
{ name: 'global_search', label: 'Search' },
{ name: 'approve_task', label: 'Approve', objectName: 'task', target: 'noop' },
{ name: 'global_search', label: 'Search', target: 'noop' },
],
};

Expand All@@ -1149,11 +1150,11 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
{
name: 'task',
fields: { title: { type: 'text' as const } },
actions: [{ name: 'inline_action', label: 'Inline' }],
actions: [{ name: 'inline_action', label: 'Inline', target: 'noop' }],
},
],
actions: [
{ name: 'merged_action', label: 'Merged', objectName: 'task' },
{ name: 'merged_action', label: 'Merged', objectName: 'task', target: 'noop' },
],
};

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

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

Expand All@@ -1198,7 +1199,7 @@ describe('defineStack - Action Auto-Merge into Objects', () => {
const config = {
manifest: baseManifest,
actions: [
{ name: 'approve_deal', label: 'Approve', objectName: 'deal' },
{ name: 'approve_deal', label: 'Approve', objectName: 'deal', target: 'noop' },
],
};

Expand Down
Loading