Skip to content

test: run action and hook bodies through the real QuickJS sandbox (#575 A1) - #576

Merged
yinlianghui merged 1 commit into
mainfrom
claude/issue-575-a1-quickjs-zjoss3
Jul 31, 2026
Merged

test: run action and hook bodies through the real QuickJS sandbox (#575 A1)#576
yinlianghui merged 1 commit into
mainfrom
claude/issue-575-a1-quickjs-zjoss3

Conversation

@yinlianghui

Copy link
Copy Markdown
Collaborator

Description

Issue #575 item A1 only — the action-sandbox test gap.

The action side had no sandbox harness. test/global-actions.test.ts ran each body.source through new Function, which proves what a body computes and is structurally blind to everything the runtime imposes on it: no module scope, a JSON-only boundary, a capability gate, and a ctx.api built by the engine rather than by the author. So the one constraint this repo relies on by name — the shared factory in src/objects/_line-item-price-fill.ts is safe only while its handler body never reads a factory parameter — lived in a comment, with nothing anywhere failing if it were violated.

test/helpers/action-sandbox.ts runs nothing of its own: it hands bodies to the same QuickJSScriptRunner + actionBodyRunnerFactory / hookBodyRunnerFactory the runtime wires at boot, lowered by the same extractHookBody pass the CLI build uses. The only stand-in is the engine underneath — an ObjectQL-shaped recorder, deliberately without .object() so the runtime builds the repo facade itself and a body meets the real update(data, options) signature. Its two hand-written rules are pinned against a real ObjectQL on the in-memory driver, the way test/hook-query-predicate.test.ts pins the read predicate.

Verified against main @ 3ed27b26 before starting: the gap still existed.

Type of Change

  • New tests / test infrastructure
  • Documentation update (the constraint comment in _line-item-price-fill.ts now names its guard)

Related Issues

Related to #575 (item A1 only — B-group items untouched, A2–A4 untouched)

Changes Made

  • test/helpers/action-sandbox.ts (new) — real QuickJS harness: runActionBody builds the exact actionCtx both dispatch paths in @objectstack/runtime build (params: { ...params, recordId, objectName }, and no object key, so ctx.object is undefined in the sandbox); runHookBody executes a hook's shipped lowered body rather than its handler closure; extractSandboxBody is the CLI build's own lowering pass; makeSandboxEngine is the recording engine.
  • test/action-sandbox.test.ts (new, 52 tests) —
    • the stub engine's update contract asserted against a real ObjectQL on the in-memory driver, then the same two calls made through the runtime's facade inside QuickJS;
    • the sandbox boundary: an undeclared capability is denied, a body has no module scope, the dispatcher's params arrive as input;
    • all 7 script action bodies executed, with behavioural assertions on clone_opportunity, mark_primary, create_campaign and send_email;
    • the price-fill constraint as an executable guard on both line-item objects, with a negative control (a factory whose handler reads a factory parameter is rejected, naming the identifier) and a consequence control (such a body ReferenceErrors in the VM);
    • the generalisation the build only mentions in passing: all 24 callables still lower to metadata-only bodies.
  • test/global-actions.test.ts — same 18 assertions, now executed by the real sandbox instead of new Function.
  • src/objects/_line-item-price-fill.ts — the sandbox note points at its guard, and records why a test has to do this: when extraction fails the build does not — it silently keeps the handler bundled and the hook stops being deployable as pure metadata.
  • changeset added.

Finding recorded, not fixed

mass_update_stage calls ctx.api.object(...).update(id, { stage }), but ctx.api is the engine repo facade, whose update takes (data, options) — the id lands in the data slot and the engine rejects the write with Update requires an ID or options.multi=true on every invocation. Nothing before this harness could say so: the metadata is valid, the body parses, and #508 already documents the action as having no live invocation path. Fixing a shipped action is outside A1, so test/action-sandbox.test.ts asserts the current behaviour under a test name that says what it is; whoever revives the action will land on that assertion.

Same shape, larger and out of scope here: _hook-api.ts documents update: (id, doc) and nine hooks call it that way, while the kernel's ScopedContext / repo facade both take (data, options). Worth its own issue.

Testing

  • pnpm verify green end to end (validate → typecheck → lint → hygiene → build → test): 532 passed, 2 skipped, 26 files
  • Coverage thresholds still met (stmts 95.21 / branch 80.31 / funcs 95.12 / lines 99.59)
  • New tests added, including a negative control that fails when the guard is violated
  • No new dependencies — quickjs-emscripten arrives via @objectstack/runtime, already a direct dependency; lockfile untouched

Checklist

Additional Notes

Cost: ~50 ms per sandboxed run (a fresh WASM context each time, as the runtime does). The suite went from 3.6 s to 9.3 s wall clock overall.

Convention 5 from the issue — re-run pnpm verify on main after merge — still applies.


Generated by Claude Code

The action side had no sandbox harness. `test/global-actions.test.ts` ran
each `body.source` through `new Function`, which proves what a body computes
and is structurally blind to everything the runtime imposes on it: no module
scope, a JSON-only boundary, a capability gate, and a `ctx.api` built by the
engine rather than by the author. So the one constraint this repo relies on
by name — the shared factory in `src/objects/_line-item-price-fill.ts` is
safe only while its handler body never reads a factory parameter — lived in a
comment, with nothing anywhere failing if it were violated.

`test/helpers/action-sandbox.ts` runs nothing of its own: it hands bodies to
the same `QuickJSScriptRunner` + `actionBodyRunnerFactory` /
`hookBodyRunnerFactory` the runtime wires at boot, lowered by the same
`extractHookBody` pass the CLI build uses. The only stand-in is the engine
underneath — an ObjectQL-shaped recorder, deliberately without `.object()` so
the runtime builds the repo facade itself and a body meets the real
`update(data, options)` signature. Its two hand-written rules are pinned
against a real ObjectQL on the in-memory driver, the way
`hook-query-predicate.test.ts` pins the read predicate.

`test/action-sandbox.test.ts` then executes all seven script action bodies,
shows the capability gate denying an undeclared capability, shows a body has
no module scope, and turns the price-fill constraint into a guard with a
negative control — plus the generalisation the build only mentions in
passing: all 24 callables still lower to metadata-only bodies, and a hook
that stops doing so is silently bundled instead, not caught.

`global-actions.test.ts` keeps its assertions and switches to the real
executor. One finding it surfaced is recorded, not fixed: `mass_update_stage`
calls `update(id, doc)`, which the engine facade rejects on every invocation
(#508 already documents the action as having no live invocation path).

Refs #575.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ku5mr51i1UNcbq9VNbaNQb
@vercel

vercel Bot commented Jul 31, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
hotcrm Ignored Ignored Jul 31, 2026 2:48pm

Request Review

@yinlianghui
yinlianghui marked this pull request as ready for review July 31, 2026 15:01
@yinlianghui
yinlianghui merged commit a4a5ac0 into main Jul 31, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants