Skip to content

Commit b1ff43a

Browse files
committed
Merge origin/main into claude/global-actions-unreachable-rw7hpk
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
2 parents b934f96 + 1d5dc46 commit b1ff43a

65 files changed

Lines changed: 3834 additions & 334 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): action bodies execute under a real execution context — every owner-scoped write no longer dies FORBIDDEN
6+
7+
An action body's `ctx.api` was never bound. The sandbox's `buildSandboxApi`
8+
walked its whole fallback chain — no `actionCtx.api`, and the raw `ObjectQL`
9+
engine has no `.object()` (that lives on `ScopedContext`, reachable only via
10+
`engine.createContext()`, which the action path never called) — and landed on a
11+
repo facade that proxied every call to the engine with **no `context`**.
12+
`ctx.engine` had the identical hole.
13+
14+
Context-less is not "trusted", it is **identity-less**, and identity-less is
15+
strictly worse than either coherent posture: plugin-sharing's write gate
16+
short-circuits on `!context.userId` (no user to own the record) and its bypass
17+
needs `context.isSystem` (never set). So a `type: 'script'` action whose body
18+
called `ctx.api.object('crm_case').update(...)` failed with
19+
`FORBIDDEN: insufficient privileges to update crm_case` — **as the built-in
20+
admin** — while the `[action-audit]` line on the same request announced
21+
RLS-bypassing TRUSTED execution. Objects with a `public` sharing model, no
22+
owner field, or a bypass listing passed the gate early, so only *some* actions
23+
broke and the defect read as object-dependent flakiness.
24+
25+
Both dispatch paths (REST `/actions/:object/:action` and MCP `run_action`) now
26+
bind `ctx.api` to `engine.createContext(...)` and thread the same envelope
27+
through `ctx.engine`, matching what hook bodies already get from the engine's
28+
`buildHookApi`. The envelope is the caller's `ExecutionContext` elevated with
29+
`isSystem: true` — the posture the action surface already documents and gates
30+
for at invoke time (the ADR-0066 D4 capability gate and the `ai.exposed` gate
31+
are what admit a body to trusted execution). The caller's fields are spread
32+
first, so a body's writes stay attributable (`userId` stamps
33+
`created_by`/`updated_by`), org-scoped (`tenantId` stamps the org column and
34+
drives driver-level tenant isolation), and joined to an open transaction —
35+
rather than the unattributable, org-less rows a bare `{ isSystem: true }` would
36+
write.
37+
38+
No authoring change is required: `ctx.api.object(name)` inside a `body` now
39+
does what the docs always said it does. Bodies that worked before (public /
40+
owner-less objects) are unaffected apart from their writes now being correctly
41+
attributed and org-stamped.

‎.changeset/actions-global-key-and-failure-status.md‎

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,7 @@
33
"@objectstack/client": minor
44
---
55

6-
fix(actions): reach global actions at their real registration key, and stop serving handler failures as HTTP 200 (#3913)
7-
8-
Two independent defects that compounded into "global actions are unreachable,
9-
and when one fails you are told it succeeded".
6+
fix(actions): reach global actions at their real registration key, and 404 an action that never dispatched (#3913)
107

118
**1 — the registration key and the lookup key disagreed.** Both writers
129
register an objectName-less action under the literal `'global'`: `AppPlugin`
@@ -29,36 +26,30 @@ single-segment path (`/actions//:action`) routes at `'global'` instead of
2926
400-ing. A handler registered directly under `'*'` still resolves; the doc
3027
comments that called `'global'` a "wildcard" are corrected at every site.
3128

32-
**2 — every handler failure was wrapped as transport success.** Both the
33-
success and the failure exit called `deps.success(...)`, which always emits
34-
`{status: 200, body: {success: true, data}}`. So any failure — a denial, a
35-
missing action, a deliberate `throw` in an action body — went out as:
29+
**2 — "no such action" was reported as a success.** The not-found exit called
30+
`deps.success(...)`, which always emits `{status: 200, body: {success: true,
31+
data}}`, so a request naming an action that does not exist came back as:
3632

3733
```json
3834
{"success":true,"data":{"success":false,"error":"Action 'log_call' on object '*' not found"}}
3935
```
4036

4137
Every caller that did not hand-unwrap the INNER envelope read the outer
42-
`success: true` and reported a success that never happened. Failures now exit
43-
through the dispatcher's error path with a real status:
44-
45-
| Failure | Status |
46-
|:---|:---|
47-
| No handler registered under any key | **404**, naming the routed object rather than whichever probe ran last |
48-
| Deliberate `throw` from an action body (`SandboxError`) | **400** with the business message — the mapping `@objectstack/rest`'s `mapDataError` has always used for the identical error |
49-
| A `flow` action whose flow ran and rejected | **400**, `code: FLOW_FAILED` |
50-
| Error carrying its own `.status` (e.g. a `FORBIDDEN`) | that status |
51-
| Record `ValidationError` | **400** with `fields[]` (#3918 parity) |
52-
| Anything else | **500** |
53-
54-
The **success** envelope is unchanged (`{success: true, data: {success: true,
55-
data}}`), and `client.actions.invoke` / `invokeGlobal` still do **not** throw —
56-
they fold the new non-2xx shape back into the same `{ success, data?, error? }`
57-
result, with `error` as a plain string, and keep honouring the pre-#3913
58-
`200 + inner success:false` shape so a current SDK can still talk to an older
59-
server.
60-
61-
**Migration:** anything reading `response.data.success` off a raw HTTP call
62-
should read the HTTP status (or the top-level `success`) instead — a failed
63-
action is no longer a 200. Callers going through `@objectstack/client` need no
64-
change.
38+
`success: true` and reported a success that never happened — including the
39+
shipped console, which showed a green toast (fixed on that side in
40+
objectui#2963). Nothing **dispatched** there, so it is a **404** now, joining
41+
the answers this route already gives a status: 403 denied, 400 wrong action
42+
type, 503 unavailable. The miss also names the **routed** object rather than
43+
whichever probe ran last (the old fallback said `on object '*'`, an object the
44+
caller never asked for).
45+
46+
A handler that **ran and rejected** is unchanged: HTTP 200 with
47+
`data: {success: false, error, code?, fields?}`. That is a business outcome,
48+
not a transport error, and #3937 pins it. The line is "did a handler run" —
49+
below it the payload, above it the status.
50+
51+
`client.actions.invoke` / `invokeGlobal` still do **not** throw. `client.fetch`
52+
throws on every non-2xx, so `invoke` now catches and folds a dispatch failure
53+
into the same `{ success, data?, error? }` result with `error` as a plain
54+
string — otherwise the routes that just gained a status would have started
55+
propagating exceptions into callers that only ever checked `result.success`.
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
"@objectstack/client": patch
3+
---
4+
5+
fix(client): normalize both server error envelopes so `err.code` / `err.fields` mean one thing (#3918 follow-up)
6+
7+
Two envelopes are in play and they disagree about where the semantic code and
8+
the per-field list live:
9+
10+
```
11+
@objectstack/rest, flat:
12+
{ error, code: 'VALIDATION_FAILED', fields: [...] }
13+
14+
runtime dispatcher, wrapped:
15+
{ success: false, error: { message, code: 400,
16+
details: { code: 'VALIDATION_FAILED', fields: [...] } } }
17+
```
18+
19+
`error.code` in the **wrapped** form is the HTTP status, not a semantic code.
20+
The client read it straight through, so `err.code` was the **number 400** where
21+
the flat envelope gave `'VALIDATION_FAILED'` — meaning the branch our own docs
22+
teach,
23+
24+
```js
25+
if (err.code === 'VALIDATION_FAILED') err.fields.forEach(…)
26+
```
27+
28+
never matched on a dispatcher-served surface, and the field list (put on the
29+
wire for those routes by #3918) was unreachable at `err.details.error.details.fields`.
30+
31+
Now normalized at the throw site:
32+
33+
- **`err.code` is always the semantic string.** It is read from the flat
34+
`code`, else the wrapped `error.details.code`, else a *string* `error.code` —
35+
a numeric value is never reported as a code. The HTTP status is on
36+
`err.httpStatus`, where it always was.
37+
- **`err.fields` is the per-field list** whenever the server sent one, from
38+
either envelope. It is left **unset** (not `[]`) when there is none, so
39+
`if (err.fields)` is a safe test for "this failure is field-anchored".
40+
- **`err.details`** prefers a top-level `details` (unchanged), then the wrapped
41+
envelope's own `details`, then the whole body. The flat envelope has no
42+
top-level `details` and so keeps falling through to the whole body exactly as
43+
before — only the wrapped shape changes, and only from "the entire response"
44+
to the structured object it actually carries.
45+
46+
**Behaviour change worth noting:** code that read `err.code` from a
47+
dispatcher-served route previously got a number and now gets a string (or
48+
`undefined` where the server sent no semantic code). Nothing in this repo did —
49+
`err.httpStatus` was always the correct source for the status, and remains
50+
untouched — but a consumer that branched on `err.code === 400` should move to
51+
`err.httpStatus === 400`.
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(runtime): route every domain `catch` through `errorFromThrown` so status and `fields[]` survive (#3918 follow-up)
6+
7+
#3867 taught `dispatcher-plugin`'s `errorResponseBase` to read an error's
8+
`status` (not just `statusCode`), and #3918 taught
9+
`HttpDispatcher.errorFromThrown` the `VALIDATION_FAILED` shape. Both fixes were
10+
invisible to a whole tier of handlers underneath them: the domain modules each
11+
caught their own errors and called `deps.error(e.message, e.statusCode || 500)`
12+
directly, bypassing `errorFromThrown` entirely — 13 call sites, 9 of them in
13+
`/packages` alone.
14+
15+
The consequence on `/packages`, `/meta/_drafts`, `/ui`, `/security` and the
16+
`/mcp` transport:
17+
18+
- **A deliberate status was downgraded to 500.** Every protocol-layer domain
19+
error in this codebase carries its HTTP status as `status`, not `statusCode`
20+
(`OBJECT_NOT_FOUND`, `RECORD_NOT_FOUND`, `CLONE_DISABLED`, plugin-sharing's
21+
`FORBIDDEN`, …) — the exact read #3867 fixed one tier up. So a 404 these
22+
routes meant to return arrived as a 500, and the message was dragged through
23+
the 5xx leak sanitiser on the way out.
24+
- **A `ValidationError` still lost its `fields[]`** and its 400, re-opening
25+
#3918 on precisely the routes it was filed against.
26+
27+
Every one of those catches now calls `deps.errorFromThrown(e, …)`, so both
28+
fixes finally reach the routes that need them. Deliberate per-route fallbacks
29+
are preserved rather than flattened to 500: the `/meta` save fallback keeps
30+
**501** (that branch is reached only when the protocol has no `saveMetaItem`,
31+
so "unsupported" is the honest default) and the `/meta` two-part lookup keeps
32+
**404** — but a validation failure on either now answers 400 with its fields
33+
instead of being swallowed by the fallback.
34+
35+
`domains/keys.ts` is deliberately **not** converted: it discards the underlying
36+
error on purpose, because the message could echo row contents. Its literal
37+
`'Failed to create API key'` is the correct answer there and stays.
38+
39+
No behaviour change for errors that already carried `statusCode` — that read is
40+
preserved, only widened.
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
---
2+
"@objectstack/runtime": patch
3+
---
4+
5+
fix(actions): seed a flow action's params with the row id, like the trigger route does (#3915 follow-up)
6+
7+
#3915 gave the REST `/actions/:object/:action` route its flow dispatch and
8+
documented it as "equivalent to `POST /api/v1/automation/:target/trigger`,
9+
without having to know the flow name". A real run showed that claim did not
10+
hold: the params bag carried the subject record's fields — so `id` — but never
11+
`recordId`. The CRM's own `crm_convert_lead` action declares
12+
`recordIdParam: 'recordId'` and its flow reads `{recordId}`, so invoking it
13+
through the actions endpoint reached the automation engine and then died at its
14+
first node:
15+
16+
```
17+
Flow 'crm_convert_lead_wizard' failed: Node 'get_lead' failed: get_record:
18+
refusing to run — 1 filter condition(s) resolved to nothing … `{recordId}` (at id)
19+
```
20+
21+
while the identical run through `/automation/crm_convert_lead_wizard/trigger`
22+
paused normally on its first screen. Only a live invocation surfaced it — the
23+
unit tests mock `automation.execute`, so they pinned the call shape without
24+
noticing the bag was missing the key flows actually read.
25+
26+
`dispatchFlowAction` now seeds the row id under the same keys
27+
`domains/automation.ts` seeds for the trigger route — `recordId` and the
28+
`<objectName>Id` camelCase alias — plus the action's own declared
29+
`recordIdParam` (sourced from `recordIdField`, default `id`) when it names a
30+
third key. Explicit action params still win over every seed, and the seeding
31+
applies to the MCP `run_action` path too, which shared the same gap. A declared
32+
`recordIdParam` that no dispatcher honoured was the `declared ≠ enforced` shape
33+
in miniature.
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
'@objectstack/driver-sql': minor
3+
'@objectstack/runtime': minor
4+
'@objectstack/cli': minor
5+
---
6+
7+
`os migrate` no longer touches the database before you confirm, and refuses a
8+
SQLite database another process is using (#3917).
9+
10+
**Nothing is written before the prompt.** `plan` called itself a dry run and
11+
`apply` gated on `[y/N]`, but both booted the full plugin set first — and boot
12+
schema-sync issued create-table/add-column DDL (plus the artifact's inline seed
13+
wrote rows) against the target database before either promise was kept.
14+
`SqlDriver` gains `setDeferredDdl` / `previewDeferredSchemaWork` /
15+
`flushDeferredSchemaDdl`: while armed, `initObjects` still registers every
16+
in-memory map drift detection depends on but records the physical work instead
17+
of performing it. Both commands boot with it armed, render the held-back work
18+
as a `New (additive)` section of the plan, and `apply` performs it only after
19+
confirmation. `os meta resync` / `os migrate files-to-references` keep the old
20+
behaviour — they need the tables to exist.
21+
22+
**Occupancy check.** A live `os dev`/`os serve` holding the same SQLite file is
23+
the usual way a migration goes wrong: the migration is transactional and swaps
24+
tables inside the file, but the running server keeps prepared statements and a
25+
schema cookie the migration invalidates. `os migrate` now probes the target
26+
before booting — `PRAGMA locking_mode = EXCLUSIVE` + `BEGIN IMMEDIATE` under
27+
`busy_timeout = 0`, which reports `SQLITE_BUSY` when another connection is
28+
*attached*, not merely writing. (`wal_checkpoint(TRUNCATE)` only sees an active
29+
writer, and `-wal`/`-shm` presence cannot tell a live server from a crashed one;
30+
both are encoded as tests.) `apply` refuses with exit 1 — `error: database_busy`
31+
under `--json` — unless the new `--force` flag is passed; `plan` warns and
32+
continues, since it writes nothing either way. SQLite only: Postgres and MySQL
33+
take their own server-side locks.
34+
35+
`@objectstack/runtime` also exports `resolveStandaloneDatabase()`, so a caller
36+
can resolve the database target with the same precedence the boot uses without
37+
building the stack, and `createStandaloneStack` accepts `skipSeedData`.
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/spec": patch
3+
"@objectstack/service-analytics": patch
4+
---
5+
6+
feat(analytics): order the time axis by default, and give reports a sort declaration (#3916)
7+
8+
A matrix report with a date dimension across rendered its columns in arbitrary
9+
order — `2026-07-01, 2026-07-05, …, 2026-07-02`. Declaring `dateGranularity` on
10+
the dataset dimension made the bucket keys *sortable* (`2026-07`, `2026-Q3`)
11+
without making anything *sort* them, and the report author had no way to ask:
12+
`DatasetSelection.order` existed on the wire, but `ReportSchema` had no ordering
13+
field at all (dashboard widgets had their own `options.sortBy` channel; reports
14+
did not). Nothing in the chain supplied an order either — `resolveOrdering`
15+
returned `undefined` unless the selection carried one explicitly, the ObjectQL
16+
aggregate path has no ordering grammar so its buckets came back in Map-insertion
17+
order, and the pivot builds its column headers in row-arrival order.
18+
19+
- **A selected time dimension is now chronological by default.** When a
20+
selection states no `order` (and no `limit`, whose own fallback already
21+
ordered by every dimension), each selected dimension the cube types as `time`
22+
defaults to ASCENDING, in selection order. Bucket keys are minted sort-stable
23+
precisely so this works — `2026-07` sorts after `2026-06`, `2026-Q3` after
24+
`2026-Q1`. This lands on both strategy paths: a real `ORDER BY` where native
25+
SQL serves the query, and the executor's post-pass where a date-bucketed query
26+
is handed to the ObjectQL path. Null / empty buckets stay last, as everywhere
27+
else. Deliberately narrow: only time dimensions get a default, so grids with
28+
nothing wrong with them are not reordered.
29+
- **Reports can declare an ordering.** `ReportSchema.order` (and
30+
`blocks[].order` for a `joined` report) is a list of `{ by, direction }` sort
31+
keys, most significant first — an array, not a `Record`, because key order is
32+
the contract and JSON object key order should not have to be. `by` must name a
33+
dimension the report groups by (`rows` / `columns`) or a measure it displays
34+
(`values`); anything else fails at authoring time rather than becoming an
35+
ordering that silently does nothing. Duplicate keys are rejected. A `joined`
36+
report orders per block — declaring `order` on the container is an error.
37+
`reportSelectionOrder()` lowers the list into the `DatasetSelection.order` a
38+
renderer posts, and returns `undefined` for an empty list so the runtime's own
39+
defaults still apply.
40+
41+
An explicit `order` still wins outright — the chronological default is a
42+
default, not a policy, so "newest month first" is one declaration away.
43+
44+
`report.order` ships as `planned` + `authorWarn` in the liveness ledger: the
45+
framework half is complete and live (schema, lowering helper, executor), but
46+
objectui's `DatasetReportRenderer` does not yet carry `report.order` into the
47+
selection it posts. The default time-axis ordering needs no renderer change and
48+
is live now.

0 commit comments

Comments
 (0)