Skip to content

Commit f61c8cf

Browse files
fix(spec,metadata-protocol)!: 排序节点写 direction 不再静默排反方向,两扇门同一个 change 关 (#4721) (#4922)
* feat(spec,metadata-protocol)!: reject a sort node spelling its direction `direction` (#4721) `SortNodeSchema` was a plain `z.object`, so zod's `.strip` default applied: SortNodeSchema.parse({ field: 'updated_at', direction: 'desc' }) → { field: 'updated_at', order: 'asc' } The foreign key was discarded, `order` fell back to `asc`, and the sort ran the OPPOSITE way under an ordinary success — with `limit`, a different set of rows, with no signal anywhere in the response. `direction` is not a typo: it is `IReportService.orderBy`'s live vocabulary, which plugin-auth's objectql adapter already translates by hand. A translation known to be necessary and enforced nowhere is the ADR-0049 shape. Both doors onto that shape are closed here, in one change: - `SortNodeSchema` (spec/src/data/query.zod.ts) → `strictObject` with `aliases: { direction: 'order' }`, so the rejection carries the translation. Edit distance can never bridge `direction` → `order`, so a bare "unrecognized key" would leave the caller where the silent strip did. - `normalizeSortNodes` (metadata-protocol/src/protocol.ts) — the ingress every external `orderBy` funnels through — refuses `{ field, direction }` with 400 INVALID_SORT naming `order` and quoting the corrected node. Closing only the schema would repeat the #1535/#4522 door asymmetry: `SortNodeSchema.parse` is reachable by three paths the REST normalizer never sees, and the normalizer runs ahead of any QueryAST parse. Deliberately NOT in scope: `QuerySchema`'s top level stays non-strict (`QuerySchema.safeParse({object:'sales', nonsenseKey:1}).success === true`) — tracked in the #4001 campaign map for its own batch. The `{field: direction}` map form is untouched: there `direction` is an ordinary column name, and refusing it would be the mirror-image bug. Strictness ledger: `query.zod.ts` keeps its `open` class for the four dialect sites; `SortNodeSchema` is carved out as authorable (4 strip of 5), which also resolves the recorded classification conflict — the FILE was the wrong unit. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 * docs(spec): keep SortNodeSchema's page description short; rationale moves to line comments `build-docs.ts` takes the FIRST `/** */` block in a `.zod.ts` file as the reference page's description, and joins every line of it with a blank line. The long #4721 rationale therefore rendered as a 56-line wall at the top of `content/docs/references/data/query.mdx`, where a customer reads what a sort node IS — not why one schema in the file is strict. The prose is unchanged, it is just `//` instead of `/** */` so the generator cannot pick it up, with a note at the top saying why it must stay that way. Regenerated: query.mdx (two lines), and the two skill reference indexes, which grew transitive entries because query.zod.ts now imports shared/strict-object. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Ehu85kbvMcrNTUJjwxvLJ9 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 2f1e2a5 commit f61c8cf

10 files changed

Lines changed: 458 additions & 30 deletions

File tree

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/metadata-protocol": major
4+
---
5+
6+
feat(spec,metadata-protocol)!: a sort node spelling its direction `direction` is a 400, not a silently reversed page (#4721)
7+
8+
**FROM → TO:** `orderBy: [{ field: 'updated_at', direction: 'desc' }]`
9+
`orderBy: [{ field: 'updated_at', order: 'desc' }]`. One word. If you are on the
10+
`{field, direction}` shape because you moved code over from
11+
`IReportService.orderBy`, that contract is unchanged — it is `orderBy` on the
12+
QueryAST / `EngineQueryOptions` axis that has always been `{field, order}`.
13+
14+
## What was wrong
15+
16+
`SortNodeSchema` was a plain `z.object`, so zod's default `.strip` applied.
17+
Measured on `main` before this change:
18+
19+
```
20+
SortNodeSchema.parse({ field: 'updated_at', direction: 'desc' })
21+
→ { field: 'updated_at', order: 'asc' }
22+
```
23+
24+
`direction` was discarded and `order` fell back to its `asc` default. The sort
25+
therefore ran in the **opposite** direction and the request succeeded. Paired
26+
with `limit` — which is how a caller asks for "the latest N" — that is not a
27+
reordered page but a **different set of rows**, returned under an ordinary 200
28+
with nothing in the response to distinguish it from the answer that was asked
29+
for.
30+
31+
`direction` is not a typo. It is the live vocabulary of a neighbouring contract,
32+
`IReportService.orderBy` (`@objectstack/spec/contracts`), and
33+
`plugin-auth/objectql-adapter.ts` already translates between the two by hand — a
34+
translation known to be necessary and enforced nowhere, which is the ADR-0049
35+
shape.
36+
37+
## What changed
38+
39+
Both doors onto that shape, in one change:
40+
41+
1. **`SortNodeSchema`** (`spec/src/data/query.zod.ts`) is now `strictObject`
42+
with `aliases: { direction: 'order' }`. An unknown key is rejected, and
43+
`direction` specifically gets the translation in the error message — edit
44+
distance can never bridge `direction``order`, so a bare "unrecognized key"
45+
would leave the caller exactly where the silent strip did.
46+
2. **`normalizeSortNodes`** (`metadata-protocol/src/protocol.ts`), the ingress
47+
every REST/RPC `orderBy` funnels through, refuses `{ field, direction }` with
48+
`400 INVALID_SORT` naming `order` and quoting the corrected node. Closing only
49+
the schema would repeat the door asymmetry of #1535/#4522: `SortNodeSchema` is
50+
reachable by three paths the REST normalizer never sees.
51+
52+
| `orderBy` you send | Before | After |
53+
|:--|:--|:--|
54+
| `[{ field: 'x', order: 'desc' }]` | descending | unchanged — descending |
55+
| `[{ field: 'x', direction: 'desc' }]` | **200, ascending** | `400 INVALID_SORT`, message names `order` |
56+
| `[{ field: 'x', order: 'desc', direction: 'asc' }]` | 200, descending | `400 INVALID_SORT` |
57+
| `'-x'` / `['-x']` / `{ x: 'desc' }` | descending | unchanged |
58+
| `{ direction: 'desc' }` (the `{field: direction}` map) | sorts by column `direction` | unchanged — a column may legitimately be called `direction` |
59+
60+
Scope is deliberately narrow: **`QuerySchema`'s top level is untouched** and
61+
still accepts undeclared keys (`QuerySchema.safeParse({ object: 'sales',
62+
nonsenseKey: 1 }).success === true`). That is tracked in the #4001 campaign map
63+
for its own batch, not smuggled in here.
64+
65+
Related: #4674, #4720, #4363, #4371, #4001, ADR-0049.

content/docs/references/data/query.mdx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,9 @@ description: Query protocol schemas
77

88
Sort Node
99

10-
Represents "Order By".
10+
Represents "Order By" — one `\{ field, order \}` pair. Unknown keys are
11+
12+
REJECTED (#4721); spell the direction `order`, never `direction`.
1113

1214
<Callout type="info">
1315
**Source:** `packages/spec/src/data/query.zod.ts`

docs/audits/2026-07-unknown-key-strictness-ledger.md

Lines changed: 35 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -510,7 +510,8 @@ not verdicts).
510510
| `external-lookup.zod.ts` | 12 | mixed (p) | authored config + wire results |
511511
| `seed-loader.zod.ts` | 12 | mixed (p) | seed file shapes are authored; loader state is runtime |
512512
| `field.zod.ts` | 11 | authorable | partially strict |
513-
| `filter.zod.ts` / `query.zod.ts` | 11+5 | open | query dialect — user data flows through; validated semantically elsewhere. `query.zod.ts` dropped one site in #4196: `FieldNodeSchema`'s nested-select object form was declared-but-inert and narrowed to `z.string()`, so the union's second member is gone. Four more left in #4286 with the `joins`/`windowFunctions` removals: `JoinNodeBaseSchema`, `WindowFunctionNodeSchema`, and `WindowSpecSchema`'s two blocks (outer + `frame`) were deleted with their clusters. Class unchanged |
513+
| `filter.zod.ts` | 11 | open | query dialect — user data flows through the predicate values; validated semantically elsewhere |
514+
| `query.zod.ts` | 5 | open, **except `SortNodeSchema` → authorable** | Blanket `open` was the imprecise verdict here, not the strictness. Four sites are the dialect proper (`BaseQuerySchema`, `AggregationNodeSchema`, `FullTextSearchSchema`, `GroupByNodeSchema`'s object arm) and keep the class. `SortNodeSchema` is not dialect: a closed two-key tuple `{field, order}` with **no user-data face at all** — so #4721 carved it out and it is **strict as of #4721** (`strictObject` + `aliases: { direction: 'order' }`). What that bought, measured on `main` first: `SortNodeSchema.parse({field, direction:'desc'})` → `{field, order:'asc'}` — the sort ran the OTHER WAY, and with `limit` that is a different set of rows under an ordinary 200. Per the 11:41Z ruling on #4721 this is a NEW door, not the completion of #4371's: that check is a hand-written top-level allowlist in `objectql/src/engine.ts` (`rejectUnknownEngineOptions`) that never recurses into `orderBy[]`, and `QuerySchema` itself is **not** strict (probe: `QuerySchema.safeParse({object:'sales', nonsenseKey:1}).success === true`) — top-level strictness is #4001's, tracked separately. Site history: one site dropped in #4196 (`FieldNodeSchema`'s nested-select object form narrowed to `z.string()`); four more in #4286 with the `joins`/`windowFunctions` removals (`JoinNodeBaseSchema`, `WindowFunctionNodeSchema`, `WindowSpecSchema`'s outer + `frame`) |
514515
| `driver-nosql.zod.ts` / `driver.zod.ts` / `driver-sql.zod.ts` | 10+9+2 | wire | driver capability contracts |
515516
| `datasource.zod.ts` | 6 | authorable | **strict as of #4001 data step** — all 6: `DatasourceSchema` (+ `pool` / `ssl`), `ExternalDatasourceSettingsSchema` (+ `validation`), `DriverDefinitionSchema`. **#4583 B/C dropped two more sites**: the `healthCheck` and `retryPolicy` blocks are gone — nothing scheduled a probe and nothing retried, so their strictness was validating a shape no code consumed. `config` stays `z.record` **at this level** by construction (per-driver shapes), but is no longer unchecked: **#4410** made `DatasourceSchema`'s refinement parse it against the contract for the declared driver (`driver/config-registry.zod.ts`), so the openness here is a shape this level cannot express rather than the absence of one. This row used to add "the driver's own `configSchema` validates them", which was false until #4410 landed the parse site it names. #4410 extended the same parse to each `readReplicas` entry; **#4468 retired that key** — no driver ever opened a replica connection and no query path splits reads from writes, so the entries were being checked against a contract nothing would apply. Strictness makes a dropped key loud; it cannot make a slot live, and a *precisely validated* dead slot is the more convincing lie | **#4583 dropped the ninth site**: `DatasourceCapabilities` is gone — eleven flags no code read, on a block whose strictness was the clearest case of this row's own closing sentence. `readOnly` in particular was *precisely validated* and completely inert, and had been relocated twice (#4410, #4465) toward somewhere it might be enforced; the shipped CRM example called a datasource a read replica on the strength of it while writes went through. Class unchanged
516517
| `driver/memory.zod.ts` / `driver/mongo.zod.ts` / `driver/postgres.zod.ts` | 6+1+1 | authorable | The per-driver shapes for the `config` slot — what an author actually writes under `datasource.config` (`host`, `port`, `filename`). **Undeclared here until the coverage walk went recursive** (see below): a subdirectory was invisible to the gate, so these sites sat outside the map while the map reported full coverage. **Strict as of #4410**, which is also what unblocked them: this row previously read "strictness here would enforce nothing" because nothing parsed `datasource.config` against these schemas and both `*DriverSpec.configSchema` literals were `{}`. Now `DatasourceSchema` parses `config` against them, and the same schemas project onto `configSchema` and onto the Studio connection form. (#4410 also ran the parse over each `readReplicas` entry; #4468 retired that key outright — see the row above.) `postgres.zod.ts` drops a site: its `ssl` was a `boolean | {ca, cert, key, …}` union, and the object arm is gone — certificates now live in the datasource-level `ssl` block (declared, strict, and until #4410 read by nobody), leaving `config.ssl` as the on/off shorthand. That narrowing is forced by the same projection: the Studio form renders anything that is not boolean/enum/number as a TEXT INPUT, so a union here would have produced a wizard whose every `ssl` value the new gate rejects. `memory.zod.ts` keeps 6 but loses two KEYS — `indexes` / `maxRecordsPerObject`, which `InMemoryDriverConfig` has no field for, removed under ADR-0049 rather than blessed by the new gate |
@@ -670,7 +671,7 @@ reverse pin above). Worth noting for the next batch that "resolve a row" has two
670671
exits, and the reverse pin cannot tell them apart — only the changeset and the
671672
triage row record which one was taken.
672673

673-
#### `data/`121 strip of 162
674+
#### `data/`120 strip of 162
674675

675676
| File | Strip | Sites | Class | Batch |
676677
|---|---|---|---|---|
@@ -684,14 +685,14 @@ triage row record which one was taken.
684685
| `analytics.zod.ts` | 8 | 8 | mixed (p) | `Metric` / `Dimension` / `Cube` / `AnalyticsQuery` — cube definitions are authored; needs a per-schema read |
685686
| `document.zod.ts` | 8 | 8 | wire (p) | `DocumentTemplate` / `ESignatureConfig` read authorable on their face — the `(p)` is unresolved, verify before scheduling either way |
686687
| `driver/memory.zod.ts` | 5 | 6 | authorable | The persistence-adapter union under `datasource.config`; `datasource.config` HAS been parsed against these since #4410, so strictness here now binds |
687-
| `query.zod.ts` | 5 | 5 | open | ⚠️ **classification conflict — see #4721.** The row calls the query dialect `open`; #4721 asks for `SortNodeSchema.strict()`. Both cannot be right. Resolve the class before writing code |
688+
| `query.zod.ts` | 4 | 5 | open | ~~⚠️ classification conflict — see #4721~~ **RESOLVED (11:41Z ruling, closed by #4721).** The conflict was real and the answer was that per-FILE classification was the imprecise instrument: `SortNodeSchema` was carved out as `authorable` and closed (`strictObject` + `aliases: { direction: 'order' }`), the other 4 sites keep `open`. Those 4 are the dialect proper — `BaseQuerySchema`, `AggregationNodeSchema`, `FullTextSearchSchema`, `GroupByNodeSchema`'s object arm — and `BaseQuerySchema`'s own top-level strictness is #4001's to schedule, deliberately **not** taken by #4721 |
688689
| `external-catalog.zod.ts` | 4 | 4 | wire (p) | **out of scope** |
689690
| `hook.zod.ts` | 4 | 6 | wire | **out of scope**`HookContextSchema` + `.session`/`.provenance`/`.user` are the runtime shape handed to a handler; verified in the data step |
690691
| `field.zod.ts` | 3 | 11 | authorable | `LocationCoordinates` / `CurrencyValue` / `Address` — field VALUE shapes, not field config; check whether they are record data (→ open) before closing |
691692
| `driver-sql.zod.ts` | 2 | 2 | wire | **out of scope** |
692693
| `field-value.zod.ts` | 1 | 2 | mixed (p) | `LocationValueSchema` — record data, very likely **open**; its sibling `FileValueSchema` is already `z.looseObject` |
693694

694-
**Authorable strip in `data/`: ~22 firm** (`object` 14 + `driver/memory` 5 + `field` 3), **plus ~33 needing a per-schema verdict** (`external-lookup` 12, `seed-loader` 12, `analytics` 8, `field-value` 1). 66 are wire/open and out of the ruling's forced scope.
695+
**Authorable strip in `data/`: ~22 firm** (`object` 14 + `driver/memory` 5 + `field` 3), **plus ~33 needing a per-schema verdict** (`external-lookup` 12, `seed-loader` 12, `analytics` 8, `field-value` 1). 65 are wire/open and out of the ruling's forced scope — 66 until #4721 closed `query.zod.ts`'s `SortNodeSchema`, which is the one row in this directory where the per-schema read moved a site OUT of `open` rather than confirming it.
695696

696697
#### `security/` — 13 strip of 20
697698

@@ -789,25 +790,39 @@ is the confirmation the campaign's own progress log was missing.
789790
estimated — read it, do not re-derive it, and do not plan off `strictObject(`
790791
occurrence counts (finding 19 explains what that undercounts).
791792

792-
Two things in that map need a decision before any code is written, and both
793-
are classification questions rather than implementation ones:
794-
795-
- **`data/query.zod.ts` is classed `open`, and #4721 asks for
796-
`SortNodeSchema.strict()`.** Both cannot be right. Measured, so the decision
797-
is made against facts rather than recollection: `SortNodeSchema.parse({
798-
field, direction: 'desc' })` returns `{ field, order: 'asc' }` — the wrong
799-
rows, with no signal — and #4721's premise that the top level already
800-
rejects unknown option keys is true but of a **different mechanism**:
801-
#4371's check is a hand-written allowlist in `objectql/src/engine.ts`
802-
(`rejectUnknownEngineOptions`) that iterates `Object.entries(bag)` at the
803-
top level only. It is a bespoke guard at one door, which is this campaign's
804-
finding 17 exactly, so "same invariant, one level down" is not available as
805-
a justification — closing the sort node is a *new* door, not the completion
806-
of an existing one.
793+
Two things in that map needed a decision before any code was written, and
794+
both were classification questions rather than implementation ones. The first
795+
is now settled:
796+
797+
- ~~**`data/query.zod.ts` is classed `open`, and #4721 asks for
798+
`SortNodeSchema.strict()`.**~~ **SETTLED (2026-08-03 11:41Z ruling; closed
799+
by #4721.)** Measured, so the decision was made against facts rather than
800+
recollection: `SortNodeSchema.parse({ field, direction: 'desc' })` returns
801+
`{ field, order: 'asc' }` — the wrong rows, with no signal — while #4721's
802+
premise that the top level already rejects unknown option keys turned out
803+
to be about a **different mechanism** and, on re-measurement, not even true
804+
of the schema: #4371's check is a hand-written allowlist in
805+
`objectql/src/engine.ts` (`rejectUnknownEngineOptions`) that iterates
806+
`Object.entries(bag)` at the top level only, and `QuerySchema` itself is not
807+
strict (`QuerySchema.safeParse({object:'sales', nonsenseKey:1}).success ===
808+
true`). That is finding 17 exactly — a bespoke guard at one door — so "same
809+
invariant, one level down" was **not** available as a justification, and the
810+
ruling does not use it: closing the sort node is a **new** door.
811+
812+
**The answer was that the FILE was the wrong unit.** `open` was awarded to
813+
the query dialect because user data flows through predicate values;
814+
`SortNodeSchema` is a closed two-key tuple with no user-data face, so it was
815+
re-classed `authorable` and closed while the other four sites kept `open`.
816+
The generalisable part: when a blanket per-file class collides with a
817+
per-schema finding, **suspect the blanket first** — this ledger classifies
818+
sites, and a file is only a convenient bag of them. Both doors were closed
819+
in the same change (`SortNodeSchema` + `normalizeSortNodes` in
820+
`metadata-protocol`), per finding 6's asymmetry.
821+
807822
- **`ui/app.zod.ts`'s `BaseNavItemSchema`** is the base that the strict
808823
discriminated-union members `.extend()`. Finding 16 is the warning: closing
809824
a base closes every extension of it, including any that is deliberately a
810-
wire shape.
825+
wire shape. **Still open.**
811826

812827
Done in the registered-types batch: `strictObject` (`shared/strict-object.ts`)
813828
replaced the four-part wiring recipe, and `seed` + `doc` became the first two

0 commit comments

Comments
 (0)