Skip to content

Commit 55be28d

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/query-top-limit-alias-fold
# Conflicts: # packages/metadata-protocol/src/protocol.ts
2 parents f972945 + af2a095 commit 55be28d

29 files changed

Lines changed: 2399 additions & 149 deletions
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
---
2+
"@objectstack/lint": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
feat(lint): L2 action-body writes to undeclared fields warn at author time (#4271)
7+
8+
The write-set lint that #4305 gave L2 hook bodies now covers the other surface
9+
that carries one. An action body is the same artefact: the same
10+
`HookBodySchema` union, parsed by the same `HookBodySchema.safeParse` in
11+
`actionBodyRunnerFactory`, run in the same QuickJS sandbox. So it fails the
12+
same way — `ctx.api.object('crm_deal').update({ stag: 'won' })` inside an
13+
action succeeds, returns success to the caller, and the unknown column simply
14+
never lands. Half the surface was still blind.
15+
16+
**New rule — `action-body-write-unknown-field` (advisory).** Wired into
17+
`REFERENCE_INTEGRITY_RULES`, so `os validate`, `os lint` and `os compile` all
18+
report it; it never blocks a build. Both places the runtime reads actions from
19+
are walked — top-level `actions` and `objects[].actions` — and a
20+
`defineStack`-merged action, which lives in both, is reported once at its
21+
authored path. That dedupe is by VALUE (bound object + name + body source), not
22+
by object identity the way `collectBundleActions` can afford: the suite runs on
23+
the schema-PARSED stack, and parsing rebuilds every node, so the two copies
24+
arrive as distinct objects that are merely equal. An identity check passes a
25+
shared-reference unit fixture and then reports the showcase app's one warning
26+
twice — which is exactly what it did before the end-to-end run caught it.
27+
28+
**Only the `ctx.api` write family carries over, and that is the point.** An
29+
action's `ctx.input` is its PARAMS bag (`input: unwrapProxyToPlain(actionCtx
30+
?.params)`), not a record, so resolving those names against object fields would
31+
flag every correctly-named parameter — a pure false-positive machine, and a
32+
false positive kills an advisory lint. `ctx.record` is not a write surface
33+
either: the runner hands the body a plain snapshot and never writes it back, so
34+
`ctx.record.x = …` is discarded for *declared* and undeclared fields alike —
35+
a different defect from "the unknown column vanishes", and flagging only its
36+
undeclared half would imply the declared half persists.
37+
38+
So the rule ships a declared **partition** of the shared
39+
`HOOK_BODY_WRITE_PATTERNS` rather than a second ledger:
40+
`ACTION_BODY_WRITE_PATTERN_IDS` (today: `api-crud-literal`) and
41+
`ACTION_BODY_WRITE_EXCLUSIONS` (`input-property-assign`,
42+
`input-object-assign`), each exclusion carrying its reason. The two halves are
43+
tested to cover the shared ledger exactly, so a fourth pattern landing on the
44+
hook side fails this rule's test until someone classifies it — silence is not a
45+
decision. Every applicable pattern is additionally proved end-to-end through
46+
the full validator (prefilter, pattern filter and field check included), and
47+
every exclusion is proved to be about applicability rather than an
48+
unextractable shape: the shared extractor still sees it, and this rule still
49+
reports nothing for it.
50+
51+
One extractor, one field index, one implicit-field set, shared with the hook
52+
rule rather than copied. The action rule is the same check on the other body
53+
surface, so a second copy of `IMPLICIT_FIELDS` would drift exactly the way the
54+
five hand-copied system-field lists #4330 collapsed did.
55+
56+
The lint stays off the kernel boot path, and lands one notch tighter than the
57+
hook side: the only applicable pattern is rooted at `ctx.api`, so an action
58+
body that never mentions it does not even parse, let alone load the ~9 MB
59+
TypeScript compiler. Guarded by `lazy-deps.test.ts`.
60+
61+
`@objectstack/spec`: `ScriptBodySchema` and `ActionSchema.body` now point at
62+
the action-side rule and spell out that `ctx.input` (params) and `ctx.record`
63+
(a discarded snapshot) are not record-write surfaces — doc comments only, no
64+
schema or generated-artifact change.
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
---
2+
"@objectstack/metadata-protocol": patch
3+
"@objectstack/objectql": patch
4+
"@objectstack/spec": patch
5+
"@objectstack/rest": patch
6+
---
7+
8+
fix(data): `searchFields` / `groupBy` / `aggregations` naming a field that does not exist are rejected, not silently degraded (#4254)
9+
10+
#4226 closed `sort` / `select` / `expand`; with the filter axis (#4134 / #4164 /
11+
#4181 / #4121) that made four field-naming read axes that either apply or fail.
12+
The same machine kept leaking on the remaining three, and each failure corrupted
13+
something the closed axes never touched:
14+
15+
```
16+
search=alpha&searchFields=no_such -> 200 MORE rows than the narrowing allowed
17+
groupBy=[no_such] -> 200 [{no_such: null, n: <true count>}] N groups collapsed into 1
18+
sum(no_such) -> 200 0 — indistinguishable from a real zero
19+
```
20+
21+
Each is now refused at the shared normalizer, so `GET /data/:object`,
22+
`POST /data/:object/query`, the export route and the runtime dispatcher give
23+
one answer instead of four.
24+
25+
- **`searchFields``400 INVALID_FIELD`.** The `select` failure with the sign
26+
flipped outward: the engine dropped unknown names and, when that emptied the
27+
override, fell back to the FULL searchable set — so a parameter that exists
28+
only to narrow a search widened it, and it changed which ROWS came back, not
29+
just which columns. Its only in-framework caller is `GET /data/:object/export`
30+
— the route whose `search` support just shipped so exports would stop
31+
downloading "the unsearched superset … in a file that looks authoritative";
32+
a typo'd `searchFields` did exactly that, one parameter over. Three causes,
33+
three messages, because the fixes differ (the split #4226 drew on expand): a
34+
name that is no field is a request typo; a REAL field outside the searchable
35+
set needs the object changed (its message names the declared
36+
`searchableFields` or the auto-default's type rule, whichever applies); and
37+
a `searchableFields` entry that names no field is a STALE DECLARATION — a
38+
bug on the object, called out as such because clients (objectui's list
39+
search) echo the declaration verbatim. The allowed set is resolved by the
40+
same `@objectstack/spec/data` function the engine's search expansion
41+
consumes (`resolveSearchFieldResolution`, moved from objectql), so the gate
42+
cannot drift from what search actually scans.
43+
- **`groupBy``400 INVALID_FIELD`.** The in-memory aggregation path projects
44+
an unknown column as `null` for every row, so all rows landed in ONE bucket
45+
whose count is the true row count — structurally perfect, identical to "this
46+
column really holds a single value". A chart draws one bar; nothing says the
47+
grouping never ran. Native SQL aggregation errors on the same input, so which
48+
backend a deployment sits on decided the answer — the "two routes, opposite
49+
answers" split, one axis over.
50+
- **`aggregations``400 INVALID_FIELD`.** `sum(<typo>)` folded a column of
51+
`undefined` to `0` — the exact number an empty quarter produces, in reports
52+
whose whole job is to be believed (`avg`/`min`/`max` answered `null` the same
53+
way). `count` with no `field` (or the `'*'` sentinel) is the one legitimate
54+
field-less form and passes.
55+
- **Unreadable SHAPES on the aggregation axes → `400 INVALID_QUERY`** — the
56+
standard-catalog code that had no emitter since it was written, like
57+
`INVALID_SORT` before #4226. A string `groupBy`, an entry naming no field, a
58+
function or `dateGranularity` outside the spec enums, a missing `alias`: each
59+
slipped past the `Array.isArray` routing guard (rows returned UNGROUPED) or
60+
computed a silent placeholder (`null` results, a column keyed `"undefined"`,
61+
one bucket per raw value under an unknown granularity).
62+
63+
Tiering is unchanged from #4226: registry + field map present → authoritative;
64+
no registry / no field map / legacy array field map → the NAME gates skip (shape
65+
gates still apply — they need no schema). The engine's own tolerance is
66+
untouched: internal callers reaching `engine.find()` / `engine.aggregate()`
67+
directly are unaffected. `@objectstack/rest` also stops logging
68+
`INVALID_FILTER` / `INVALID_SORT` / `INVALID_QUERY` rejections as
69+
"[REST] Unhandled error" — they are client mistakes the response already
70+
explains, as `INVALID_FIELD` always was.
71+
72+
Requests that name real fields are unaffected.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
---
3+
4+
ci(tooling): enforce the #4251 slot-lookup sweep ratchet with a counted baseline
5+
6+
The grandfather list shipped as paths in `eslint.config.mjs` under a comment
7+
reading "NEVER add an entry" — a promise nothing checked. ESLint cannot express
8+
a ratchet: an ignored file is ignored completely, so a NEW `getService<any>(…)`
9+
added to an already-listed file rode the old entry with `pnpm lint` green.
10+
11+
The list moves to `scripts/slot-lookup-baseline.json` (file → site count) and
12+
becomes the single source: its keys are the config's `ignores`, its values are
13+
what `pnpm check:slot-lookup` enforces. The script re-runs ESLint with the
14+
grandfathering lifted and matches reports by the rule's exact message, so the
15+
counter cannot drift from the rule. It fails on a new file, a grown count, a
16+
stale entry, and — via a key-set comparison against the merge base with main —
17+
on a file being ADDED to the list, which no count check can see.
18+
19+
Tooling and CI only; releases nothing.

.github/workflows/lint.yml

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ jobs:
2626
steps:
2727
- name: Checkout repository
2828
uses: actions/checkout@v7
29+
with:
30+
# The slot-lookup ratchet compares the baseline against its state at
31+
# the merge base with main — the only way to see a file being ADDED
32+
# to the grandfather list. A shallow clone has no merge base, and the
33+
# check would degrade to "not verified" on every run.
34+
fetch-depth: 0
2935

3036
- name: Setup Node.js
3137
uses: actions/setup-node@v7
@@ -57,6 +63,15 @@ jobs:
5763
- name: ESLint
5864
run: pnpm lint
5965

66+
# Slot-lookup sweep ratchet (#4251). `pnpm lint` above bans erasing a
67+
# service-lookup result to `any` across packages/, but the files still
68+
# holding pre-existing sites are grandfathered by path — and an ignored
69+
# file is ignored COMPLETELY, so new erasures added to one ride the old
70+
# entry in silence. This re-measures those files with the grandfathering
71+
# lifted and holds them to a per-file count, so the list can only shrink.
72+
- name: Slot-lookup ratchet
73+
run: pnpm check:slot-lookup
74+
6075
# Raw NUL guard (#3127): one literal U+0000 byte makes grep/ripgrep treat
6176
# the whole file as binary and silently return ZERO matches — the file drops
6277
# out of code search and out of every grep-based lint, with no error saying

content/docs/api/data-api.mdx

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,8 @@ Query records with filtering, sorting, selection, and pagination.
2424
| `top` | query | Max records to return. No default — omitting it returns all matching records. |
2525
| `skip` | query | Offset |
2626
| `expand` | query | Comma-separated list of relations to eager-load. Must name a reference field (`lookup` / `master_detail` / `user` / `tree`) — otherwise `400 INVALID_FIELD`. |
27-
| `search` | query | Full-text search query |
27+
| `search` | query | Full-text search term, scanned case-insensitively across the object's searchable fields (its declared `searchableFields`, or a text-like auto-default when none are declared). |
28+
| `searchFields` | query | Comma-separated subset of the searchable fields for `search` to scan — narrows the scan, never widens it. A name outside the searchable set is `400 INVALID_FIELD`. |
2829

2930
> **Note:** OData-style `$`-prefixed parameters (`$filter`, `$select`, `$orderby`, `$top`, `$skip`, `$expand`, `$count`, `$search`) are also accepted directly on this same endpoint as aliases — they're normalized internally to the parameter names above. There is no separate standalone OData endpoint.
3031
@@ -116,6 +117,46 @@ a direction that is neither `asc` nor `desc`) is `400 INVALID_SORT`.
116117
`GET /data/:object/:id` applies the same `select` and `expand` rules, so the
117118
list and single-record routes cannot disagree about one field map.
118119

120+
#### Neither is a search narrowing, a grouping, or an aggregation
121+
122+
The last three field-naming axes follow the same rule. Each of these used to
123+
answer `200` with something that looked exactly like a served query — and each
124+
corrupts something the earlier axes do not:
125+
126+
| Request | Result |
127+
|:---|:---|
128+
| `?search=alpha&searchFields=title` | scans only `title` |
129+
| `?search=alpha&searchFields=no_such_field` | `400 INVALID_FIELD` |
130+
| `?search=alpha&searchFields=amount` | `400 INVALID_FIELD` — real field, but not searchable |
131+
| `groupBy: ["status"]` | one bucket per status value |
132+
| `groupBy: ["no_such_field"]` | `400 INVALID_FIELD` |
133+
| `aggregations: [{function:"sum", field:"amount", alias:"total"}]` | the real total |
134+
| `aggregations: [{function:"sum", field:"no_such_field", alias:"total"}]` | `400 INVALID_FIELD` |
135+
| `aggregations: [{function:"count", alias:"n"}]` | `count(*)` — the one legitimate field-less form |
136+
137+
- **`searchFields`** — the only parameter whose failure changed which **rows**
138+
came back. An unknown name used to be dropped, and an override left empty
139+
fell back to scanning *every* searchable column: a parameter that exists
140+
only to **narrow** a search failed by **widening** it. Three causes get
141+
three messages, because the fixes differ: a name that is no field (a typo in
142+
the request), a real field outside the searchable set (declare it in
143+
`searchableFields`), and a `searchableFields` entry that names no field (a
144+
stale declaration — the bug is on the object, and clients that echo the
145+
declaration verbatim are told so).
146+
- **`groupBy`** — an unknown column projected `null` for every row, so all
147+
rows fell into **one bucket** whose count is the true row count:
148+
structurally perfect, indistinguishable from a column that really holds a
149+
single value. A chart draws one bar and nothing says the grouping never ran.
150+
- **`aggregations`**`sum` over an unknown column folded blanks to **`0`**,
151+
the exact number a genuinely empty quarter produces (`avg`/`min`/`max`
152+
answered `null` the same way), in reports whose whole job is to be believed.
153+
154+
A `groupBy` / `aggregations` value the spec cannot read at all — a bare string
155+
instead of an array, an entry that names no field, a function or date
156+
granularity outside the spec's enums, a missing `alias` — is
157+
`400 INVALID_QUERY`: those shapes were silently ignored, returning **ungrouped
158+
raw rows** with nothing to say the aggregation never happened.
159+
119160
**Response**:
120161
```json
121162
{

content/docs/api/error-catalog.mdx

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,10 @@ ObjectStack uses a structured error system with **9 error categories** and **53
7474
**Cause:** A field name in the request does not exist on the target object. On a
7575
list read this also covers an unreserved query parameter — `GET /data/:object`
7676
reads those as field filters, so one naming no field could only match zero
77-
records and is rejected rather than answered with an empty page.
77+
records and is rejected rather than answered with an empty page — plus every
78+
other read axis that names a field: `select`, `expand` (a real field that holds
79+
no reference gets its own message), `searchFields` (a real field outside the
80+
searchable set gets its own message), `groupBy`, and `aggregations[].field`.
7881
**Fix:** Check the object schema for valid field names. Use `os meta get object <name>` to inspect the object's fields. If the name was meant as a
7982
*parameter* rather than a field, use the real one — page size is `top` / `$top`
8083
/ `limit`, not `pageSize` / `perPage`; the response's `error` names the
@@ -117,8 +120,20 @@ substitute. See the [Data API](/docs/api/data-api).
117120
**Retry:** `no_retry`
118121

119122
### `INVALID_QUERY`
120-
**Cause:** The query structure is malformed.
121-
**Fix:** Check the query syntax against the [Query Cheat Sheet](/docs/data-modeling/queries).
123+
**Cause:** A `groupBy` / `aggregations` value the spec cannot read: a bare
124+
string where an array belongs, an entry that names no field, an aggregation
125+
function or `dateGranularity` outside the spec's enums, or a missing `alias`.
126+
(Field names that simply don't exist on the object are `INVALID_FIELD`
127+
instead.)
128+
**Fix:** Check the aggregation shapes against the
129+
[Query Cheat Sheet](/docs/data-modeling/queries) — e.g.
130+
`{ "groupBy": ["status"], "aggregations": [{ "function": "sum", "field":
131+
"amount", "alias": "total" }] }`. `count` is the only function that may omit
132+
`field`.
133+
**Why it is an error and not an empty result:** every one of these shapes used
134+
to be silently ignored or mis-read — rows came back **ungrouped**, or an
135+
unknown function computed `null` — in a response indistinguishable from a
136+
served aggregation.
122137
**Retry:** `no_retry`
123138

124139
### `INVALID_FILTER`

content/docs/data-modeling/queries.mdx

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,19 @@ three (only `count`/`sum`/`avg`/`min`/`max` are mapped), and the in-memory drive
364364
silently returns `null` for them. Avoid these three on SQL- or memory-backed objects.
365365
</Callout>
366366

367+
<Callout type="info">
368+
Over the REST/protocol ingress, `groupBy` and `aggregations` are validated
369+
before they reach any driver. A field the object does not have is
370+
`400 INVALID_FIELD` — the in-memory fallback used to collapse an unknown
371+
`groupBy` column into one `null`-keyed bucket, and to answer `sum(<typo>)`
372+
with `0`. A value the spec cannot read (a non-array, an entry naming no
373+
field, a function or `dateGranularity` outside the enums above, a missing
374+
`alias`) is `400 INVALID_QUERY` — those shapes used to be ignored, returning
375+
ungrouped raw rows. `count` with no `field` (or `field: "*"`) is the one
376+
legitimate field-less form and passes. Internal callers reaching
377+
`engine.aggregate()` directly are unaffected.
378+
</Callout>
379+
367380
### Aggregation Example
368381

369382
```typescript
@@ -449,6 +462,17 @@ Only `query` and `fields` are implemented. The engine expands `search` into a dr
449462
`operator`.
450463
</Callout>
451464

465+
<Callout type="info">
466+
`fields` (and its query-parameter spelling, `?searchFields=` / `?$searchFields=`) can only
467+
**narrow** the scan within the server-resolved searchable set — the object's declared
468+
`searchableFields`, or a text-like auto-default when none are declared. Over the
469+
REST/protocol ingress a name outside that set is `400 INVALID_FIELD`, with distinct
470+
messages for a field that does not exist and a real field that is not searchable. The
471+
engine used to drop unknown names and fall back to the full searchable set — a parameter
472+
that exists to narrow a search, silently widening it. Internal callers reaching
473+
`engine.find()` directly are unaffected.
474+
</Callout>
475+
452476
### Pinyin recall (Chinese deployments)
453477

454478
When pinyin search is enabled (`OS_SEARCH_PINYIN_ENABLED` — auto-on when the stack's

0 commit comments

Comments
 (0)