fix(runtime): carry code/fields across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up)

.changeset/sandbox-structured-error-passthrough.md

@@ -0,0 +1,61 @@
+---
+"@objectstack/runtime": patch
+---
+
+fix(runtime): carry `code` / `fields[]` across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up)
+
+Found by dogfooding the merged #3918 chain against a running app. Submitting a
+record that fails validation through a form **action** came back as:
+
+```
+HTTP 200
+{ "success": true, "data": { "success": false,
+                             "error": "ValidationError: issued_on is required" } }
+```
+
+No status a client could branch on, no code, no `fields[]`. The chain's
+dispatcher fixes could not help: the field list was already gone before any
+dispatcher exit ran. It was lost at the QuickJS boundary, twice —
+
+1. **host → VM.** `vm.newError({ name, message })` dropped every other property,
+   so a body reaching a record `ValidationError` through
+   `ctx.api.object(x).update(...)` saw bare prose.
+2. **VM → host.** The wrapper's reject handler flattened the error to the string
+   `<name>: <message>` before the host ever saw it.
+
+Both hops now carry an explicit **allowlist** — `code` and `fields` — alongside
+the message, and `SandboxError` exposes them as `.code` / `.fields`. The
+allowlist is a security boundary, not a style choice: host errors routinely hang
+driver state, connection details or whole record payloads off themselves, and
+anything crossing INTO the VM is readable by untrusted sandboxed code. Copying
+the error's own enumerable keys would leak all of it.
+
+`/actions` then surfaces them, so a form can highlight the offending input:
+
+```
+HTTP 200
+{ "success": true, "data": { "success": false,
+                             "error": "ValidationError: issued_on is required",
+                             "code": "VALIDATION_FAILED",
+                             "fields": [ { "field": "issued_on", "code": "required", … } ] } }
+```
+
+**The `/actions` wire contract is deliberately unchanged.** The status stays
+200 and `success: false` remains the failure signal: that route has always
+reported business failure in the payload (an action that "fails" is a normal
+outcome, not a transport error) and every caller branches on `data.success`.
+Making it a 4xx would be a break in exchange for a strictly additive fix, so the
+fix is additive — `code` and `fields` are simply omitted when absent, and a
+caller that ignores them sees exactly what it saw before.
+
+Message channels are byte-identical: `SandboxError.message` keeps the
+`<kind> '<name>' threw:` debug wrapper for server logs and `.innerMessage` stays
+the plain business text a toast shows. The structured payload rides alongside
+them, never instead of them.
+
+Also adds `dispatcher-validation-error.real.test.ts`, which pins both dispatcher
+exits against the **real** objectql `ValidationError` rather than a hand-built
+fixture — including its deliberate absence of `.status`, the assumption the
+whole #3918 fix rests on. The existing fixture-based tests restate that contract;
+these check it, so a future change to the class fails a test instead of quietly
+regressing production.

content/docs/automation/hook-bodies.mdx

@@ -151,6 +151,31 @@ Per-invocation budgets default to **250ms** (hooks) / **5000ms** (actions) of **
 
 A body may write *other* objects — e.g. `await ctx.api.object('parent').update({ ... })` from a child's `afterInsert`/`afterUpdate` (requires `api.write`). The target's own hooks fire too: the nested write runs in a **fresh sandbox VM** while the calling body is suspended, and this composes to any depth. This is the natural "when a child changes, roll the total up to the parent" automation — it does **not** need a denormalized, hand-maintained mirror field. Because each body's budget is **CPU time** (ADR-0102), the caller is **not** charged for the nested write's own run — so the stock 250ms default comfortably covers deep rollup chains, and you rarely need to raise `timeoutMs` (the spec still permits up to 30_000ms for a genuinely CPU-heavy body).
 
+### Errors from `ctx.api`
+
+A rejected `ctx.api` call gives your body the host error's `name` and `message`, plus two structured properties when the host supplied them:
+
+| Property | Meaning |
+|:---|:---|
+| `e.code` | The semantic code, e.g. `'VALIDATION_FAILED'` |
+| `e.fields` | Per-field validation envelopes — `{ field, code, message }[]` |
+
+```js
+try {
+  await ctx.api.object('invoice').update({ id: input.id, status: 'sent' });
+} catch (e) {
+  if (e.code === 'VALIDATION_FAILED') {
+    // e.fields → [{ field: 'issued_on', code: 'required', message: 'issued_on is required' }]
+    throw e;   // re-throwing keeps the payload; see below
+  }
+  throw e;
+}
+```
+
+Nothing else crosses into the VM. That is a deliberate **allowlist**, not an oversight: host errors routinely carry driver state, connection details or whole record payloads, and anything reachable on a rejection is readable by body code.
+
+An error your body lets escape — or re-throws — keeps `code` and `fields` on the way back out too, so an action's HTTP response can carry them (`data.code` / `data.fields`) and a form can highlight the offending input rather than only raising a toast. Errors you construct yourself are treated the same way: set `e.code` before throwing and it reaches the caller.
+
 ## L3 — Compiled modules (intentionally disabled)
 
 An earlier design allowed the CLI to emit a sibling `objectstack-runtime.<hash>.mjs` that `objectos` would `import()` at runtime. We removed that path because:

packages/runtime/src/dispatcher-validation-error.real.test.ts

@@ -0,0 +1,137 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #3918 follow-up — pin both dispatcher exits against the REAL `ValidationError`.
+ *
+ * `dispatcher-validation-error.test.ts` drives the exits with a hand-built
+ * fixture: an `Error` with `name = 'ValidationError'`, `code =
+ * 'VALIDATION_FAILED'` and a `fields[]`. That is deliberate — it proves the
+ * duck-typing predicate accepts the SHAPE, including for hand-thrown errors
+ * that never touched objectql.
+ *
+ * What it cannot prove is that the shape it asserts is still the shape objectql
+ * actually throws. The whole fix rests on three facts about that class —
+ * `.code`, `.fields[]`, and the deliberate ABSENCE of `.status` — and a fixture
+ * restates those facts rather than checking them. Give `ValidationError` a
+ * `.status` one day, or rename `.fields`, and every fixture-based test here
+ * keeps passing while production quietly regresses.
+ *
+ * So these construct the genuine article and send it through both exits. They
+ * are the tests that fail if the contract moves.
+ */
+
+import { describe, it, expect } from 'vitest';
+import { ValidationError } from '@objectstack/objectql';
+
+import { HttpDispatcher } from './http-dispatcher.js';
+import { createDispatcherPlugin } from './dispatcher-plugin.js';
+
+const FIELDS = [
+    { field: 'email', code: 'invalid_email' as const, message: 'email must be a valid email address' },
+    { field: 'name', code: 'required' as const, message: 'name is required' },
+];
+
+describe('#3918 — the real objectql ValidationError still has the shape the fix relies on', () => {
+    const err = new ValidationError(FIELDS);
+
+    it('carries `code` and `fields[]`', () => {
+        expect(err.code).toBe('VALIDATION_FAILED');
+        expect(err.fields).toEqual(FIELDS);
+        expect(err.name).toBe('ValidationError');
+    });
+
+    it('carries NO `status` / `statusCode` — which is why the boundary must supply 400', () => {
+        // If this ever fails, the dispatcher's `.status`-first precedence will
+        // start winning over the 400 default and these exits change behaviour
+        // silently. That is the assumption the whole fix is built on.
+        expect((err as any).status).toBeUndefined();
+        expect((err as any).statusCode).toBeUndefined();
+    });
+
+    it('carries no `issues` — the property the old `details` builder read', () => {
+        expect((err as any).issues).toBeUndefined();
+    });
+});
+
+// ---------------------------------------------------------------------------
+
+/** Exit 1 — the RETURNED path, through a route whose fallback is 500. */
+async function publishDrafts(thrown: unknown) {
+    const protocol = { publishPackageDrafts: async () => { throw thrown; } };
+    const objectql = { registry: {} };
+    const resolve = (name: string) =>
+        name === 'protocol' ? protocol : name === 'objectql' ? objectql : undefined;
+    const kernel: any = { getService: resolve, getServiceAsync: async (n: string) => resolve(n) };
+    const result: any = await new HttpDispatcher(kernel).dispatch(
+        'POST', '/packages/demo/publish-drafts', {}, {}, {} as any,
+    );
+    return result.response;
+}
+
+/** Exit 2 — the THROWN path, through the plugin's real route handler. */
+async function analyticsQuery(thrown: unknown) {
+    const handlers: Record<string, (req: any, res: any) => any> = {};
+    const rec = (verb: string) => (path: string, h: any) => { handlers[`${verb} ${path}`] = h; };
+    const server = {
+        get: rec('GET'), post: rec('POST'), put: rec('PUT'),
+        delete: rec('DELETE'), patch: rec('PATCH'),
+    };
+    const analytics = {
+        query: async () => { throw thrown; },
+        getMeta: async () => ({ cubes: [] }),
+        generateSql: async () => ({ sql: null }),
+    };
+    const kernel = {
+        getService: (n: string) => (n === 'analytics' ? analytics : undefined),
+        getServiceAsync: async (n: string) => (n === 'analytics' ? analytics : undefined),
+    };
+    const plugin = createDispatcherPlugin({ prefix: '/api/v1', securityHeaders: false });
+    await plugin.start?.({
+        getKernel: () => kernel,
+        getService: (n: string) => (n === 'http.server' ? server : undefined),
+        environmentId: undefined,
+        logger: { info() {}, warn() {}, error() {}, debug() {} },
+        hook: () => {}, on: () => {},
+    } as any);
+
+    const res: any = {
+        statusCode: undefined, body: undefined,
+        status(c: number) { res.statusCode = c; return res; },
+        header() { return res; },
+        json(b: any) { res.body = b; return res; },
+    };
+    await handlers['POST /api/v1/analytics/query']({ body: { cube: 'x', query: {} }, query: {} }, res);
+    return res;
+}
+
+describe('#3918 — both exits serve the real ValidationError as 400 + fields[]', () => {
+    it('errorFromThrown (returned path)', async () => {
+        const res = await publishDrafts(new ValidationError(FIELDS));
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.details).toEqual({ code: 'VALIDATION_FAILED', fields: FIELDS });
+        // The class builds its message from the human field messages — that is
+        // what a toast shows, so it must survive intact rather than being
+        // replaced by the 5xx sanitiser.
+        expect(res.body.error.message).toContain('email must be a valid email address');
+    });
+
+    it('errorResponseBase (thrown path)', async () => {
+        const res = await analyticsQuery(new ValidationError(FIELDS));
+
+        expect(res.statusCode).toBe(400);
+        expect(res.body.error.details).toEqual({ code: 'VALIDATION_FAILED', fields: FIELDS });
+        expect(res.body.error.message).toContain('name is required');
+        // Not a server fault: the errorReporter side-channel stays clear.
+        expect(res.__obsRecordedError).toBeUndefined();
+    });
+
+    it('an empty-fields ValidationError still answers 400, not 500', async () => {
+        // The class defaults its message to 'Validation failed' for an empty
+        // list; the status must not depend on the list being non-empty.
+        const res = await publishDrafts(new ValidationError([]));
+
+        expect(res.status).toBe(400);
+        expect(res.body.error.details.fields).toEqual([]);
+    });
+});

packages/runtime/src/domains/actions-validation-envelope.test.ts

@@ -0,0 +1,128 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #3918 follow-up — the LAST hop: what `/actions` puts on the wire.
+ *
+ * The sandbox now carries `code` / `fields` back out of the VM on a
+ * `SandboxError` (see `sandbox/error-passthrough.test.ts`). This pins the other
+ * half: the actions route surfacing them, which is what a form actually reads.
+ *
+ * Before, this envelope was the message string and nothing else, so a form
+ * action could raise a toast but never highlight the offending input — the
+ * symptom in the original report.
+ *
+ * **The status stays 200.** `/actions` has always reported business failure in
+ * the payload (`{ success: false }`), not as a transport error, and every
+ * caller branches on `data.success`. Making this a 4xx would be a wire-contract
+ * break in exchange for a strictly additive fix, so the fix is additive: the
+ * same envelope, now carrying what the client needs. That choice is asserted
+ * below so it cannot drift silently.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+
+import { HttpDispatcher } from '../http-dispatcher.js';
+import { SandboxError } from '../sandbox/quickjs-runner.js';
+
+const FIELDS = [
+    { field: 'issued_on', code: 'required', message: 'issued_on is required' },
+];
+
+const scriptAction = {
+    name: 'submit_signoff',
+    objectName: 'crm_invoice',
+    type: 'script',
+    body: { language: 'js', source: 'return 1;', capabilities: ['api.write'] },
+};
+
+/** A dispatcher whose action handler rejects with `thrown`. */
+function makeDispatcher(thrown: unknown) {
+    const objectDef = { name: 'crm_invoice', actions: [scriptAction] };
+    const ql: any = {
+        executeAction: vi.fn(async () => { throw thrown; }),
+        getSchema: (n: string) => (n === objectDef.name ? objectDef : undefined),
+        registry: { getObject: (n: string) => (n === objectDef.name ? objectDef : undefined), getItem: () => undefined },
+        find: vi.fn(async () => [{ id: 'inv_1', status: 'draft' }]),
+        insert: vi.fn(), update: vi.fn(), delete: vi.fn(),
+    };
+    const metadata: any = {
+        load: vi.fn(async () => null),
+        listObjects: vi.fn(async () => [objectDef]),
+        getObject: vi.fn(async () => objectDef),
+    };
+    const kernel: any = {
+        context: {
+            getService: (n: string) =>
+                n === 'objectql' || n === 'data' ? ql : n === 'metadata' ? metadata : null,
+        },
+    };
+    return new HttpDispatcher(kernel);
+}
+
+async function invoke(thrown: unknown) {
+    const res: any = await makeDispatcher(thrown).handleActions(
+        '/crm_invoice/submit_signoff/inv_1',
+        'POST',
+        {},
+        { request: {}, environmentId: 'platform', executionContext: { userId: 'u1', systemPermissions: [] } } as any,
+    );
+    return res.response;
+}
+
+/** What the sandbox now throws for a record validation failure. */
+const validationSandboxError = () =>
+    new SandboxError(
+        "action 'submit_signoff' threw: ValidationError: issued_on is required",
+        'ValidationError: issued_on is required',
+        { code: 'VALIDATION_FAILED', fields: FIELDS },
+    );
+
+describe('#3918 follow-up — /actions surfaces code + fields on a validation failure', () => {
+    it('carries `fields[]` so a form can anchor the error to the input', async () => {
+        const response = await invoke(validationSandboxError());
+
+        expect(response.body.data).toMatchObject({
+            success: false,
+            code: 'VALIDATION_FAILED',
+            fields: FIELDS,
+        });
+    });
+
+    it('keeps the human message as the toast text', async () => {
+        const response = await invoke(validationSandboxError());
+
+        expect(response.body.data.error).toBe('ValidationError: issued_on is required');
+    });
+
+    it('KEEPS HTTP 200 and `success: false` — the wire contract is unchanged', async () => {
+        // Deliberate: see the file header. If this flips to 4xx it must be a
+        // conscious, documented break, not a side effect.
+        const response = await invoke(validationSandboxError());
+
+        expect(response.status).toBe(200);
+        expect(response.body.success).toBe(true);   // dispatcher envelope
+        expect(response.body.data.success).toBe(false); // business outcome
+    });
+
+    it('omits `code` / `fields` entirely for an ordinary failure', async () => {
+        // Callers branch on presence; emitting empty values would claim a
+        // field-anchored failure that never happened.
+        const response = await invoke(
+            new SandboxError("action 'x' threw: boom", 'boom'),
+        );
+
+        expect(response.body.data).toEqual({ success: false, error: 'boom' });
+        expect('code' in response.body.data).toBe(false);
+        expect('fields' in response.body.data).toBe(false);
+    });
+
+    it('carries a code without fields when that is all the error had', async () => {
+        const response = await invoke(
+            new SandboxError("action 'x' threw: pick another", 'pick another', { code: 'DUPLICATE' }),
+        );
+
+        expect(response.body.data).toEqual({
+            success: false, error: 'pick another', code: 'DUPLICATE',
+        });
+    });
+});

packages/runtime/src/domains/actions.ts

@@ -272,6 +272,29 @@ export async function handleActionsRequest(deps: DomainHandlerDeps, path: string
         const inner: unknown = err?.innerMessage;
         const clientMsg = (typeof inner === 'string' && inner) ? inner : full;
         if (clientMsg !== full) console.error(`[action ${objectName}/${actionName}] ${full}`);
-        return { handled: true, response: deps.success({ success: false, error: clientMsg }) };
+        // [#3918 follow-up] Carry the structured payload when the failure was a
+        // record validation error. Until the sandbox learned to pass `code` /
+        // `fields` back OUT of the VM, this envelope was the message string and
+        // nothing else — so a form action could only ever raise a toast, never
+        // highlight the field the user actually got wrong, which is the symptom
+        // the original report was about.
+        //
+        // The HTTP status stays 200 and `success: false` remains the failure
+        // signal. `/actions` has always reported business failure in the payload
+        // (an action that "fails" is a normal outcome, not a transport error) and
+        // every existing caller branches on `data.success`; turning this into a
+        // 4xx would be a wire-contract break in exchange for a strictly additive
+        // fix.
+        const code: unknown = err?.code;
+        const fields: unknown = err?.fields;
+        return {
+            handled: true,
+            response: deps.success({
+                success: false,
+                error: clientMsg,
+                ...(typeof code === 'string' && code ? { code } : {}),
+                ...(Array.isArray(fields) ? { fields } : {}),
+            }),
+        };
     }
 }