fix(actions): reach global actions at their real registration key, and 404 an action that never dispatched (#3913) - #3930
Merged
Conversation
…ailures with a real status (#3913) Two independent defects that compounded into "global actions are unreachable, and when one fails you are told it succeeded". 1 — registration key vs lookup key. Both writers register an objectName-less action 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, and the miss now reports the ROUTED object rather than whichever probe ran last. 2 — every handler failure was wrapped as transport success. Both exits called deps.success(), which always emits {status: 200, body: {success: true, data}}, so a denial or a thrown handler went out as HTTP 200 {success:true, data: {success:false, error}} and any caller that skipped the inner envelope swallowed it. Failures now exit through the dispatcher's error path: 404 unregistered, 400 for a deliberate action-body throw (the mapping @objectstack/rest's mapDataError already uses for the same SandboxError) and for a rejected flow (code FLOW_FAILED), the error's own .status when it carries one, 400 + fields[] for a ValidationError, 500 otherwise. The success envelope is unchanged. client.actions.invoke/invokeGlobal still do not throw — they fold the new non-2xx shape into the same {success, data?, error?} result and keep honouring the pre-#3913 200-with-inner-failure shape, so a current SDK still talks to an older server. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AvZj6cLX7APd7roh2eK4F
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 34 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
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
os-zhuang
changed the title
This was referenced Jul 29, 2026
os-zhuang
added a commit
that referenced
this pull request
Jul 30, 2026
… single wrap (#3962) (#3969) Fixes #3962 — the platform decision classifying the 200-on-failure wire as a bug, not a contract. The 200-with-inner-envelope shape was never designed: no ADR or doc specified it, it originated as the catch block reusing deps.success(), and /actions was the only route of 12 that double-wrapped. Five defects traced back to that one extra layer. Contract now, identical to /data: - ran, returned → 200 {success:true, data: <handler return value>} (single wrap) - ran, rejected → 400, semantic code on error.code (VALIDATION_FAILED with fields[] in details; FLOW_FAILED for a rejected flow) - never dispatched → 404 / 403 / 400 / 503 (unchanged, #3930/#3951) - crashed → 500 (unchanged, #3951; name-based discriminator now selects 400 vs 500) actions-validation-envelope.test.ts pinned the 200 "so flipping it later is a conscious, documented break"; #3962 is that decision and the flipped test cites it. Integrates ADR-0110 (#3958/#3987 — name identity, undeclared refusal with no opt-out) and #3971 (semantic error.code, details.code promotion) from main. client.actions.invoke/invokeGlobal still never throw: every failure status folds into {success:false, error}, success reads the single wrap, and a narrow legacy heuristic (boolean success, no foreign keys) keeps a current SDK correct against pre-#3962 servers. Migration (raw-HTTP callers): branch on the status; on 200, data is the handler's return value directly. SDK callers need no change.
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.
Fixes #3913.
Defect 1 — registration key vs lookup key
Both writers register an objectName-less action under the literal
'global'—AppPlugin(action.object || 'global') andObjectQLPlugin.actionObjectKey. The REST fallback probed'*', andengine.executeActionis an exact-stringMaplookup with no wildcard semantics, so that probe could only ever miss:POST /api/v1/actions/global/log_callworked by accident (the path segment happened to spell the registration key);POST /api/v1/actions//log_callnever worked at all, and neither did falling back from an object-scoped route to a global handler.'global'is now the canonical key (GLOBAL_ACTION_OBJECT_KEY).actionHandlerObjectKeys(objectName)→[routed object, 'global', '*'], deduped — serves both the REST route and the MCPrun_actionbridge, which had the same'*'rotation. The legacy'*'stays last so a handler registered directly against it still resolves./actions//:action) routes at'global'instead of 400-ing.Defect 2 — "no such action" was reported as a success
The not-found exit called
deps.success(...), which always emits{status: 200, body: {success: true, data}}. A request naming an action that does not exist came back as:{"success":true,"data":{"success":false,"error":"Action 'log_call' on object '*' not found"}}Any caller that did not hand-unwrap the INNER envelope read the outer
success: trueand reported a success that never happened — including the shipped console (fixed on that side in objectstack-ai/objectui#2963).That exit is a 404 now, and it names the routed object rather than whichever probe ran last.
Where the line is
Did a handler run?
throw, flow rejection,ValidationErrordata: {success: false, error, code?, fields?}— unchanged, #3937's envelope#3937 is right that a handler which ran and rejected is a business outcome and belongs in the payload. That reasoning does not reach an unregistered action: nothing ran, there is no outcome to report, and it is indistinguishable from a typo'd URL. The route already answers every other pre-dispatch failure with a status, so the not-found exit is joining them rather than opening a new contract.
Reverted from the original version of this PR to honour #3937: the 400 for an action-body throw, the 400 +
FLOW_FAILEDfor a rejected flow, and theerrorFromThrown500 fallback.Client SDK
client.actions.invoke/invokeGlobalstill do not throw — and the catch added here is now load-bearing rather than cosmetic.client.fetchthrows on every non-2xx, so without it the routes that just gained a status would start propagating exceptions into callers that only ever checkedresult.success.Testing
pnpm buildgreen (71 tasks).actions-validation-envelope.test.ts, which this branch previously broke.http-dispatcher.actions-global-key.test.ts(13 cases). The engine double is deliberately faithful on the one point that matters: a realMapkeyed<object>:<action>that throwsengine.ts's verbatim miss message — a double that resolved any key would have passed while the bug was live. Two of its cases assert the 200 side of the line, so the 404 cannot later be widened into "every action failure is a 4xx" by someone reading only this file.Migration
A caller that hand-checked
data.successis unaffected. A caller that treated any 200 as success now sees a 404 for an action that does not exist — which is the bug being fixed. Callers going through@objectstack/clientneed no change.🤖 Generated with Claude Code
https://claude.ai/code/session_011AvZj6cLX7APd7roh2eK4F