Skip to content

Commit 95be461

Browse files
os-zhuangclaude
andcommitted
merge: main (#4344 action-body write lint, #4351/#4355 hook-bodies docs)
Both landed on the same surface while this branch was open, so the merge is a reconciliation rather than a textual resolution: - #4344 built the same `actions` + `objects[].actions` walk, with the same value-dedupe of defineStack's merged copies (it found the double-report the same way). Two walks over one surface would drift silently, so its `collectActionBodies` is now exported (module-level, not via the barrel — same posture as BODY_WRITE_SYSTEM_FIELDS), carries the bound object, and this rule consumes it. The duplicate walk and its helpers are deleted. - The two rules split one write surface and answer different questions, so the suite, the barrel and the spec JSDoc now say which is which: `ctx.api` writes → does this field exist (did-you-mean); `ctx.record` writes → none of them land, declared or not (no did-you-mean, because the name is not the bug). - #4351/#4355 rewrote the hook-bodies write-set section this branch also edited. Its write-shape table listed three shapes and said nothing about `ctx.record`, which this branch now checks — added the row plus the note that it is a different defect, and that the record rule's statically-opaque cases are covered by the runtime recorder instead. Verified after the merge: lint 642, spec 7160, runtime 989, cli 628, spec check:generated 8/8, typecheck + eslint clean, showcase 0 findings clean and exactly 1 with the repro planted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2 parents 34e836c + 0449c63 commit 95be461

103 files changed

Lines changed: 3753 additions & 494 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: 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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
---
3+
4+
docs(hook-bodies): the write-set section said the write side had no static checking, which #4305/#4344 made false (#4351) — rewritten as an accurate coverage description (three literal patterns, advisory `hook-body-write-unknown-field` / `action-body-write-unknown-field`, and the shapes the parser must skip, so a missing warning is not read as proof of correctness), the "accepted gap with no planned closure" conclusion retracted (#3700 stays accurate history; the gap was closed from the other end, by parsing the write set), the runtime paragraph corrected against measured behaviour, and the three now-stale cross-links in `hooks.mdx` updated to the renamed anchor. Releases nothing.
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
---
2+
'@objectstack/spec': patch
3+
'@objectstack/rest': minor
4+
'@objectstack/runtime': minor
5+
---
6+
7+
Request bodies are now checked against the schemas the API catalog declares for them (#3899, the request-side dual of #3877).
8+
9+
**Routes that now answer `400 VALIDATION_FAILED` + `fields[]` for a body violating their declared `requestSchema`** (previously the body was consumed raw, and a malformed one silently executed different semantics):
10+
11+
- `POST /data/:object/query` — body must be a QueryAST (`FindDataRequestSchema`); a garbage body used to degrade into an unfiltered full read. The path `object` is now pinned into the forwarded query (a body `object` can no longer contradict the path).
12+
- `POST /data/:object` / `PATCH /data/:object/:id` — body must be a record object (`CreateDataRequestSchema` / `UpdateDataRequestSchema`).
13+
- `POST /data/:object/batch` — body must be a `BatchUpdateRequestSchema` (`operation` + `records[]`).
14+
- `POST /data/:object/createMany` — body must be a bare JSON array of records (`CreateManyDataRequestSchema`); `{ records: [...] }` (updateMany's envelope) is rejected with a pointer.
15+
- `POST /notifications/read` — body must be `{ ids: string[] }` (`MarkNotificationsReadRequestSchema`); a misnamed key used to become `markRead(userId, [])` — a 200 no-op that never cleared the badge.
16+
17+
**Dispatcher automation routes now validate their bodies** (no catalog schema; hand-written guards):
18+
19+
- `POST /automation` and `PUT /automation/:name` require a flow-definition object, and POST requires a non-empty `name` — a mistyped `name` used to register the flow under the key `undefined` and echo 200.
20+
- `POST /automation/:name/toggle` is strictly `{ enabled?: boolean }` — `{"enable": false}` (one letter off) used to ENABLE the flow and answer 200 `{enabled: true}`; it is now a 400 naming the offending key. An empty body still means enable.
21+
22+
**`QuerySchema` now declares the search contract ADR-0061 actually serves** (additive): `search` accepts the canonical bare query string as well as the structured `FullTextSearch` form, and the server-validated `searchFields` narrowing is formally declared. Previously the schema declared only the object form while every surface (and the ADR's own conformance proof) sent the string — drift that surfaced the moment request bodies started being validated.
23+
24+
**Catalog corrections in `@objectstack/spec` (`plugin-rest-api.zod.ts`)** — documentation-only tables:
25+
26+
- `DEFAULT_NOTIFICATION_ROUTES` drops the four device/preferences endpoints — those server routes were removed in #3612 (never built), yet the table kept declaring them, `requestSchema` and all.
27+
- `DEFAULT_AUTOMATION_ROUTES`' trigger endpoint path is corrected `/trigger` → `/trigger/:name` (the mounted path; the flow name rides the path) and its `AutomationTriggerRequestSchema` declaration is removed — that schema never described this route's wire shape.
28+
- `DEFAULT_DATA_CRUD_ROUTES` gains the `POST /:object/query` entry (mounted since forever, previously undeclared), repoints create/update to the schemas the routes actually validate (`CreateDataRequestSchema` / `UpdateDataRequestSchema` — the old `CreateRequestSchema`/`UpdateRequestSchema` names described a `{ data }` envelope the wire never had), and drops `requestSchema` from GET/DELETE entries (path/query-bound inputs; nothing can violate them as a body).
29+
- New gates: catalog `requestSchema`/`responseSchema` strings must resolve to real exported Zod schemas, `requestSchema` may only sit on body-carrying methods, and every declared `requestSchema` on a mounted route has a violating-body → 400 conformance case (`packages/rest` + `packages/runtime` request-schema-gate suites).
30+
31+
Migration: clients that already send the documented shapes are unaffected. If you relied on a malformed body being silently accepted (e.g. posting `{ records: [...] }` to `createMany`, a non-boolean `enabled` to toggle, or an off-schema analytics/query body), fix the request to the declared shape — the 400's `fields[]` names each offending key.
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: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
---
3+
4+
ci(dx): the type-check coverage ratchet now judges the test layer too (#4311). #4324 counted a package covered the moment it declared a `typecheck` script — but 27 of the 48 it waved through `exclude` `**/*.test.ts` from the very `tsconfig.json` that `tsc --noEmit` reads, hiding 568 test files and 1451 errors behind a green check (`@objectstack/spec` alone: 272 files, 902 errors). A new `TESTS_COVERED` invariant fails any package that excludes its own tests without a measured `TEST_DEBT` entry, and the gate now prints both layers so the src figure can never again stand in for the whole. Nine packages that were already clean with tests included had the exclusion dropped outright; nine more graduate out of DEBT after repairing 44 NodeNext imports missing their `.js` extension — a repair that closed 110 errors (most of the "noise" tier was one unresolved import degrading symbols to `any`) and exposed 12 real defects in `service-settings` that those broken imports had been masking. Dev scripts, tests and CI only; releases nothing.

‎AGENTS.md‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,19 @@ workspace package declares a `typecheck` script or carries a measured DEBT/EXEMP
2929
in `scripts/check-type-check-coverage.mjs`. New packages must arrive covered; a package
3030
that graduates deletes its ledger entry in the same PR.
3131

32+
**Do not `exclude` `*.test.ts` / `*.spec.ts` from a package's `tsconfig.json`.** `tsc
33+
--noEmit` reads that config, so an exclusion there hides the tests from the check the
34+
`typecheck` script advertises — a green gate over source nothing read, which is the
35+
#4311 defect itself. The ratchet's `TESTS_COVERED` invariant fails on any new exclusion;
36+
the packages that already had one carry a measured `TEST_DEBT` entry and graduate by
37+
dropping the exclusion.
38+
39+
One trap worth knowing before you read any of these counts: under `moduleResolution:
40+
NodeNext` a relative import missing its `.js` extension does not resolve, every symbol it
41+
names becomes `any`, and the callbacks over those symbols then report TS7006 "implicitly
42+
any". A pile of TS7006 is usually one broken import upstream, not a package that needs
43+
type annotations — fix the extension first and re-measure.
44+
3245
### Running the dev server
3346

3447
| Scenario | Command | Notes |

‎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
{

0 commit comments

Comments
 (0)