fix(runtime): carry code/fields across the sandbox boundary so form actions can anchor validation errors (#3918 follow-up) - #3937
Merged
os-zhuang merged 2 commits intoJul 29, 2026
Conversation
…ctions 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 to 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. `/actions` then surfaces them so a form can highlight the offending input. Its wire contract is deliberately UNCHANGED: status stays 200 and `success: false` remains the failure signal — that route has always reported business failure in the payload and every caller branches on `data.success`, so `code`/`fields` are purely additive and omitted when absent. Message channels are byte-identical: `.message` keeps the debug wrapper for server logs, `.innerMessage` stays the toast text. Also adds dispatcher-validation-error.real.test.ts, pinning both dispatcher exits against the REAL objectql ValidationError rather than a fixture — including its deliberate absence of `.status`, the assumption the whole #3918 fix rests on. Covered by 19 new cases across sandbox, /actions and the dispatcher exits; 7 of them fail on the pre-fix source. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeZ7DmSKjiKmQzYh8L5CJS
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/l
documentation
Contributor
📓 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:
|
The docs-drift check on #3937 flagged `automation/hook-bodies.mdx`. Nothing there was made stale by this branch, but the page is the reference for what a sandboxed body gets from `ctx.api`, and this branch gives it something new: a rejected call now carries `code` and `fields` alongside `name`/`message`. Adds a short "Errors from ctx.api" section under nested cross-object writes, covering the two properties, the worked `VALIDATION_FAILED` case, the fact that the passthrough is a deliberate ALLOWLIST (body code can read anything on a rejection, so host errors must not arrive with driver state or record payloads attached), and that an escaping or re-thrown error keeps the payload on the way back out to the action's HTTP response. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LeZ7DmSKjiKmQzYh8L5CJS
os-zhuang
pushed a commit
that referenced
this pull request
Jul 29, 2026
Resolves the `/actions` catch-block conflict with #3937, which landed the opposite decision to this branch's second half and asserted it in `actions-validation-envelope.test.ts` so it could not drift silently. #3937 is right about the case it addressed: a handler that RAN and rejected is a business outcome, not a transport error, and belongs in the payload. That reasoning does not extend to a request that never DISPATCHED — an unregistered action has no outcome to report and is indistinguishable from a typo'd URL. This route already answers the other pre-dispatch failures with a status (403 denied, 400 wrong action type, 503 unavailable), so the not-found exit joins them as a 404 and everything below the dispatch line keeps #3937's envelope, `code`/`fields` included. Reverted from this branch to honour that: the 400 for an action-body throw, the 400 + FLOW_FAILED for a rejected flow, and the errorFromThrown 500 fallback. The client-side catch in `actions.invoke` stays and is now load-bearing — `client.fetch` throws on every non-2xx, so without it the routes that just gained a status would start propagating exceptions into callers that only ever checked `result.success`. The wider 200-vs-4xx question is left open for #3913 rather than settled here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AvZj6cLX7APd7roh2eK4F
This was referenced Jul 29, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 29, 2026
…d 404 an action that never dispatched (#3913) (#3930) Object-less actions register under the literal 'global' (AppPlugin's `action.object || 'global'`, ObjectQLPlugin.actionObjectKey) but the REST fallback probed '*' — and engine.executeAction is an exact-string Map lookup with no wildcard semantics, so that probe could only miss. /actions/global/:action worked by accident (the path segment happened to spell the registration key); /actions//:action never worked, and neither did falling back from an object-scoped route to a global handler. 'global' is now canonical (GLOBAL_ACTION_OBJECT_KEY). One probe order — [routed object, 'global', '*'] via actionHandlerObjectKeys — serves both the REST route and the MCP run_action bridge, with the legacy '*' kept last so a handler registered directly against it still resolves. A single-segment path routes at 'global' instead of 400-ing. The not-found exit also called deps.success(), so "no such action" went out as HTTP 200 {success:true, data:{success:false, error}} and read as a success to any caller that skipped the inner envelope. Nothing DISPATCHED there, so it is a 404 now, joining the pre-dispatch answers this route already gives a status (403 denied, 400 wrong action type, 503 unavailable), and it names the routed object rather than whichever probe ran last. A handler that RAN and rejected is unchanged: HTTP 200 with {success:false, error, code?, fields?} — a business outcome, not a transport error, per #3937. The line is "did a handler run". client.actions.invoke/invokeGlobal still do not throw; the added catch is load-bearing since client.fetch throws on every non-2xx.
os-zhuang
added a commit
that referenced
this pull request
Jul 29, 2026
…ccess:false (#3913 follow-up) (#3951) #3937 settled that a failed action reports in the payload at HTTP 200 — "an action that fails is a normal outcome, not a transport error". That is a statement about the action REJECTING: a business rule saying no. The same exit was also covering a third case it never argued for. A TypeError in a handler, a driver blowing up, a sandbox timeout are not outcomes the action chose to report — they are the server failing to produce one. Serving them as 200 hid every handler crash from the layers that exist to catch server faults: gateway error rates, retry and circuit-breaker policy, APM auto-capture, alerting, fetch().ok. Those are 500 now, through the same errorFromThrown exit every other domain catch has used since #3925 — which also puts a driver dump behind the internal-error-leak sanitiser (#3867) instead of letting it reach the client verbatim in a 200 body. Nothing #3937 put in the payload moves. Rejection and crash are told apart by the error's NAME, the signal @objectstack/rest already uses on this exact distinction: a plain Error, a SandboxError carrying innerMessage, anything with code/fields, and a ValidationError by name are rejections; TypeError / ReferenceError / a driver's own class, and a SandboxError with no innerMessage (timeout, capability denial) are crashes. A throw with no name at all is not confidently a fault and keeps its 200 — deliberately the narrow direction. Also in the same exit: an error carrying its own status/statusCode (a plugin's FORBIDDEN with status 403) is served with it rather than buried in a 200 payload. Record ValidationErrors deliberately carry no .status, so #3937's cases never reach that branch. Documented in api/error-catalog.mdx (new Action Errors section with the full status table and the two-check pattern a raw fetch caller needs) and ui/actions.mdx.
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to the #3918 chain (#3920, #3925, #3927). Found by dogfooding the merged chain against a running app, not by reading code — this is the gap that made the original report's symptom survive all three merged PRs.
What dogfooding turned up
Submitting a record that fails validation through a form action came back as:
No status a client could branch on, no code, no
fields[]. The chain's dispatcher fixes could not help here: the field list was already gone before any dispatcher exit ran. It was lost at the QuickJS boundary, twice —quickjs-runner.ts, thectx.api.object(x).<op>()rejection):vm.newError({ name, message })dropped every other property, so a body reaching a recordValidationErrorsaw bare prose.`<name>: <message>`before the host ever saw it.Reproduced on the showcase app:
showcase_submit_signoffsetsstatus: 'sent'on an invoice whoseissued_onis empty, andissued_ondeclaresrequiredWhen: record.status in ['sent','paid'].The fix
Both hops now carry an explicit allowlist —
codeandfields— alongside the message, andSandboxErrorexposes them as.code/.fields./actionsthen surfaces them: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. There's a test asserting a
ValidationErrordecorated with{ internal: { dsn }, record: { ssn } }crosses with onlyname/message/code/fieldsvisible.The
/actionswire contract is deliberately unchangedStatus stays 200,
success: falseremains 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 ondata.success. Making it a 4xx would be a break in exchange for a strictly additive fix, so the fix is additive:codeandfieldsare omitted when absent, and a caller that ignores them sees exactly what it saw before. There's a test pinning this so it can't drift silently; flipping it later should be a conscious, documented decision.Message channels are byte-identical too:
.messagekeeps the`<kind> '<name>' threw:`debug wrapper for server logs,.innerMessagestays the plain toast text. The structured payload rides alongside them, never instead of them.Also: pinning the chain against the real
ValidationErrordispatcher-validation-error.real.test.tsdrives both dispatcher exits with the genuine objectqlValidationErrorinstead of a hand-built fixture — including asserting its deliberate absence of.status, which is the assumption the whole #3918 fix rests on.The existing fixture-based tests restate that contract (deliberately — they prove the duck-typing predicate accepts the shape, including for hand-thrown errors). These check it. Give
ValidationErrora.statusone day, or rename.fields, and every fixture test keeps passing while production quietly regresses; now a test fails instead.Verification
/actionsenvelope, and the real-ValidationErrorpinning.quickjs-runner.ts+actions.tsand re-running. (The real-ValidationErrorfile passes both ways by design: it asserts an existing contract.)Generated by Claude Code