Skip to content

Commit 09ee21c

Browse files
os-zhuangclaude
andauthored
fix(spec): constrain ViewFilterRuleSchema.value by operator (#6227) (#7114)
* fix(spec): constrain ViewFilterRuleSchema.value by operator (#6227) `value` was declared string | number | boolean | null | (string|number)[] with no coupling to `operator`, so a set operator carrying a scalar — { field: 'stage', operator: 'not_in', value: 'won' } — published cleanly and then answered 400 INVALID_FILTER when the view was rendered (#5869 / PR #6209 closed that runtime half). The author was gone by then. The tightening mirrors `assertListComparandShapes` exactly, three constraints one for one: in/not_in require an array (any length), between requires exactly two bounds, every other operator is unchanged. It goes no further because #5685 ruled a schema stricter than the runtime to be the wrong side — so `in: []`, `equals: ['a','b']`, `contains: 5` and `is_empty: ''` all still parse. `superRefine` rather than a discriminated union, measured: z.discriminatedUnion cannot be constructed against `z.preprocess(normalizeFilterOperator, z.enum(…))` at all, and a Zod 4 refinement adds no JSON-Schema structure while leaving `.shape` and the ZodObject class intact for all five carriers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 * chore(spec): regenerate the baselines the #6227 tightening moves Five artifacts, all mechanical: - api-surface/ui.json + export-origins/ui.json — the two new exported operator vocabularies (VIEW_FILTER_LIST_VALUE_OPERATORS, VIEW_FILTER_PAIR_VALUE_OPERATORS). - spec-changes.json + docs/protocol-upgrade-guide.md — the ADR-0087 step-17 ledger entry `view-filter-rule-value-shaped-by-operator`. - content/docs/references/ui/view.mdx — the reworded `value` description. ⚠️ api-surface/ and export-origins/ are regenerated from a dist built from THIS source. Generating them against the stale dist this worktree inherited silently DELETED `JobRunOutcome (interface)` from contracts.json — a live export, and by build-api-surface.ts's own rule a breaking removal. Neither generator asserts dist freshness even though check-dev-prereqs.mjs already defines it by content hash; filed as #7122. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 * test(metadata-protocol): the alias-fold fixture picks a value shape its operator can carry (#6227) `canonicalizes every alias the spec still folds` loops over every entry of VIEW_FILTER_OPERATOR_ALIASES and parsed each with one hard-coded `value: 'x'`. Three of those aliases (nin / notin / notIn) fold to `not_in`, so after the #6227 tightening the rule is refused before the fold can be observed: path ["filter", 0, "value"] Operator "not_in" on field "status" requires an ARRAY of values. The suite's subject is the alias FOLD, not the value shape — the scalar was incidental — so each alias now takes a value from its operator's family, read from the spec's exported VIEW_FILTER_LIST_VALUE_OPERATORS / VIEW_FILTER_PAIR_VALUE_OPERATORS rather than a fresh local list. A list-valued operator added later therefore cannot silently reintroduce this. Every alias in the table is still covered. Not a real consumer emitting refused shapes, and not a deliberate pin of the old acceptance. It escaped the consumer sweep because the rule is built with a LOOP VARIABLE, invisible to an `operator: '<literal>'` grep; a repo-wide re-grep for variable-operator filter fixtures finds this one site and no other. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 445a0c2 commit 09ee21c

10 files changed

Lines changed: 614 additions & 7 deletions

File tree

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
---
2+
"@objectstack/spec": minor
3+
---
4+
5+
feat(spec): a view filter rule's `value` must have the shape its OPERATOR can execute (#6227)
6+
7+
<!-- adr-0087: registered view-filter-rule-value-shaped-by-operator -->
8+
9+
`ViewFilterRuleSchema.value` was declared
10+
`string | number | boolean | null | (string | number)[]` with **no coupling to
11+
`operator`**, so every operator accepted every shape. A set operator carrying a
12+
scalar — `{ field: 'stage', operator: 'not_in', value: 'won' }` — was a
13+
spec-valid view filter rule. It published cleanly, and then failed when someone
14+
opened the view.
15+
16+
That made the failure two-stage. #5869 / PR #6209 had already closed the runtime
17+
half: `assertListComparandShapes` refuses the lowered `{ stage: { $nin: 'won' } }`
18+
with a named `400 INVALID_FILTER` (a `500 DATABASE_ERROR` before it). Correct
19+
refusal, wrong moment — by then the author is long gone, and the view had been
20+
sitting in the store looking valid. The authoring surface now refuses the same
21+
shapes at publish time, so the feedback reaches the person who can act on it.
22+
23+
**The tightening mirrors the runtime gate exactly — three constraints, one for
24+
one, and deliberately nothing more:**
25+
26+
| operator | `value` must be | why |
27+
|---|---|---|
28+
| `in` / `not_in` (and the `nin` / `notIn` / `notin` spellings) | an array, any length | lowers to `$in` / `$nin` |
29+
| `between` | exactly `[min, max]` | lowers to `$between` |
30+
| everything else | unchanged | the query path does not judge them |
31+
32+
It goes no further on purpose. #5685 already ruled on the opposite error — a
33+
schema stricter than the runtime "in ways the runtime deliberately allows" was
34+
found to be the wrong side and was widened to match — so these all still parse:
35+
36+
- `in: []` — an empty list is a declared predicate ("matches nothing" /
37+
"matches everything"), and both drivers say so.
38+
- `equals: ['a', 'b']` — lowers to a deep-equality comparand every backend answers.
39+
- `contains: 5` — no backend refuses it.
40+
- `is_empty: ''` — the null predicates take their direction from the operator
41+
**name**; `convertComparison` ignores the value position, and the ObjectUI
42+
client deliberately sends a truthy placeholder there.
43+
44+
The refusal names the operator, the field, the shape received and the shape to
45+
write:
46+
47+
```
48+
Operator "not_in" on field "stage" requires an ARRAY of values. Received a
49+
string ("won"). "not_in" tests membership of a list — write ["won"] for a single
50+
value, or use "not_equals" to compare against it. An empty list [] is allowed and
51+
is a real predicate. This is refused at authoring time because the query path
52+
refuses it too (400 INVALID_FILTER, #5869).
53+
```
54+
55+
**Migration.** A filter rule whose operator is `in`, `not_in` or `between` and
56+
whose `value` is not an array of the right arity now fails to parse; `os validate`
57+
and `os lint` report each one by path. Wrap a single value in a list
58+
(`value: 'won'``value: ['won']`) or complete the range's second bound.
59+
60+
Two checks are worth doing where they look unnecessary. A rule reading
61+
`operator: 'in', value: ''` is an **unfinished** row, not a filter — decide what
62+
it was meant to select rather than mechanically rewriting it to `[""]`, which is
63+
a real and different predicate. And a view that already carried one of these
64+
shapes **was never returning filtered rows**: it answered `400 INVALID_FILTER` on
65+
render, so re-check what the view is supposed to show rather than assuming the
66+
old result set was correct.
67+
68+
**Metadata at rest is not rewritten, and there is no D2 conversion.** The read
69+
path does not re-validate stored rows, so no stored view becomes unreadable; what
70+
changes is that re-saving one is refused at the write gate, naming `value`. A
71+
conversion was considered and rejected: this shape was never written by any
72+
first-party producer (measured — every `in` / `not_in` rule across this repo,
73+
`objectui` and `cloud` already carries an array) and has never executed, so
74+
coercing it at load would be the platform guessing intent rather than replaying a
75+
rename — and it cannot guess honestly, since `between: 5` has no defensible
76+
second bound.
77+
78+
Two operator vocabularies are now exported —
79+
`VIEW_FILTER_LIST_VALUE_OPERATORS` and `VIEW_FILTER_PAIR_VALUE_OPERATORS` — so a
80+
producer can ask the question the schema asks instead of keeping its own copy.

content/docs/references/ui/view.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ View filter rule
829829
| :--- | :--- | :--- | :--- |
830830
| **field** | `string` || Field name to filter on |
831831
| **operator** | `Enum<'equals' \| 'not_equals' \| 'contains' \| 'not_contains' \| 'starts_with' \| 'ends_with' \| 'greater_than' \| 'less_than' \| 'greater_than_or_equal' \| … +10 more>` || Filter operator |
832-
| **value** | `string \| number \| boolean \| null \| (string \| number)[]` | optional | Filter value |
832+
| **value** | `string \| number \| boolean \| null \| (string \| number)[]` | optional | Filter value. The accepted SHAPE depends on the operator: `in` / `not_in` take an array (any length, including []), `between` takes exactly [min, max], every other operator takes a scalar. The unary operators (is_empty / is_not_empty / is_null / is_not_null) take their direction from the operator name and ignore this key. |
833833

834834
### Allowed Values: `ViewFilterRule.operator`
835835

docs/protocol-upgrade-guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,9 @@ The same descriptor loses a key in this step, and the pairing is the point (#674
428428
- **`import-run-automations-declared-default-corrected`**`api.ImportRequest runAutomations — the declared default of the key on BOTH import bodies, POST /api/v1/data/:object/import (ImportRequest) and its async twin POST /api/v1/data/:object/import/jobs (CreateImportJobRequest, which IS the same schema object). It was declared default(false) and described as "off by default for bulk"; it is now default(true), which is what the server has always done` → an explicit runAutomations: false on any import request that is meant to load rows without firing triggers/hooks. That spelling is unchanged and has always been the only one the server read — what changes is that omitting the key now DECLARES what it already DID. Callers who want automations on need write nothing
429429
- Why not automatic: A DECLARATION corrected to match a runtime that did not move — the inverse of a behaviour flip, and registered here for the reason protocol 12's `rest-requireauth-default-flip` and this major's `action-descriptor-resume-authority-default-flip` are: whether a given import was meant to fire triggers is a judgment no transform can make, so the prescription is a TODO rather than a rewrite. The server decides in import-prepare.ts with `body?.runAutomations !== false`, i.e. an omitted flag runs automations, and has since #2922 — automations always ran on import historically (the engine ignored the flag entirely before then), so opt-out was made the explicit act, matching platform convention. The schema said the opposite in both machine-readable and human-readable form, and both SHIPPED: `.default(false)` in `@objectstack/spec`'s JSON Schema, and the describe prose in the published reference tables for both defs. ⚠️ Nothing in this repo reconciled the two and NO deployed caller changes behaviour: no request path parses an import body through this schema — the route reads the raw body, and the sole reference to `CreateImportJobRequestSchema` is the declarative `ImportJobApiContracts` catalog entry, a declaration and not a parse. That is exactly why this needed a ruling rather than a docs edit: the divergence was unobservable in-tree and observable only to a consumer OUTSIDE it. A client or SDK that validated its request through the published schema materialised `runAutomations: false` from the declared default and sent it explicitly, and the server honoured it — so the same request body produced opposite behaviour depending on whether the caller validated before sending, with the validating caller silently losing its triggers. Nothing rejected it, nothing warned, and the reference page told an author the wrong thing in the other direction. There is deliberately NO schema tombstone and no D2 conversion: no key is removed, and an HTTP request body is neither authored nor persisted — the same disposition `notification-list-cursor-retired` (#6361) takes for the sibling default on this major, and `batch-options-validate-only-retired` before it. The declared move itself is recorded mechanically, per key, in DEFAULT_CHANGES_BY_MAJOR[17] (#4666), whose `from`/`to` fingerprints are re-derived on every build. Maintainer ruling 2026-08-09 (#6704, disposition A: the spec follows the runtime). ADR-0049 / ADR-0078.
430430
- Done when: Every import request of yours that must NOT fire triggers sends `runAutomations: false` explicitly, rather than omitting the key and trusting the old declared default. The check is worth doing precisely where it looks unnecessary: if you build the body by parsing it through `ImportRequestSchema` (or the published JSON Schema) and then send the PARSED object, your bulk loads were running with automations OFF and will now run with them ON — that is the only class whose behaviour changes, and it changes toward what an unvalidated caller always got. ⚠️ Behaviour on the wire is deliberately UNCHANGED and should be verified as such: a body that omits `runAutomations` fired triggers before this change and fires them after, and `runAutomations: false` turns them off before and after. Nothing starts being refused — the route never validated this body against the schema and does not begin to. `dryRun` is unaffected and still runs NO automations whatever the flag says (#6037).
431+
- **`view-filter-rule-value-shaped-by-operator`**`ui.ViewFilterRule value — the third key of a view filter rule, on every carrier of ViewFilterRuleSchema: ListView.filter, a list view tab filter, Page.filterBy, a related-list component filter and a lookup picker filter. It accepted any declared scalar or array for EVERY operator; the accepted shape is now decided by the rule operator — in / not_in require an array, between requires exactly two bounds, and every other operator is unchanged` → an ARRAY for in / not_in (a single value becomes a one-element list: value: "won" becomes value: ["won"]), and a two-element [min, max] array for between. The empty list [] stays legal for in / not_in and keeps its meaning. Nothing else moves: a scalar operator carrying an array, a string operator carrying a number, and a unary operator carrying an ignored value all still parse
432+
- Why not automatic: A publish-time gate catching up to a query-time one, not a new rule. #5869 / PR #6209 closed the RUNTIME half: `assertListComparandShapes` (@objectstack/objectql, filter-comparand-shape.ts) refuses a lowered `{ stage: { $nin: "won" } }` with a named 400 INVALID_FILTER, and before that it was a 500. The authoring surface stayed silent, so the failure was two-stage: the view published cleanly and only broke when someone opened it. That file names this very schema as the reachable authoring source of the defect. The tightening MIRRORS that gate exactly — three constraints, one for one — and deliberately goes no further, because #5685 already ruled on the opposite error: a schema stricter than the runtime "in ways the runtime deliberately allows" was the WRONG side and was widened to match. So `in: []` is still accepted (a declared predicate both drivers implement), `equals: ["a","b"]` is still accepted (it lowers to a deep-equality comparand), and `is_empty: ""` is still accepted (the null predicates take their direction from the operator NAME — convertComparison ignores the value position, and the ObjectUI client deliberately sends a truthy placeholder there). ⚠️ Metadata AT REST is deliberately NOT rewritten, and there is no D2 conversion. A D2 entry replays a shape the platform once WROTE and renamed; this shape was never written by any first-party producer (every in / not_in rule in this repo, in objectui and in the cloud repo already carries an array — measured) and has never EXECUTED, since it 400s on first render today. Coercing it at load would be the platform guessing intent rather than replaying a rename, and it cannot guess honestly: value: "" would become the predicate [""] (a real filter on the empty string) rather than the "not filled in yet" a console row means, and between: 5 has no defensible second bound at all. The read path does not re-validate stored rows (applyConversionsToStoredItem never validates, by its own contract), so no stored view becomes unreadable; what changes is that RE-SAVING such a view is refused at the write gate naming `value`, instead of storing a filter that 400s. ADR-0049 / ADR-0078 / ADR-0112.
433+
- Done when: Grep your authored views, pages and related-list components for a filter rule whose operator is in, not_in or between (including the alias spellings nin / notIn / notin) and whose value is not an array of the right arity, then wrap or complete it. `os validate` / `os lint` now report each one by path with the operator, the received shape and the corrected shape, so the sweep is mechanical rather than by eye. Two checks are worth doing where it looks unnecessary: a rule reading `operator: "in", value: ""` is an UNFINISHED row, not a filter — decide what it was meant to select rather than mechanically rewriting it to [""], which is a real and different predicate. And a view that already carried one of these shapes was never returning filtered rows: it answered 400 INVALID_FILTER on render (#5869), so re-check what the view is supposed to show rather than assuming the old result set was correct.
431434

432435
---
433436

packages/metadata-protocol/src/protocol.graft-normalized-operators.test.ts

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,13 @@
1919
* actually validates against.
2020
*/
2121
import { describe, it, expect } from 'vitest';
22-
import { ViewMetadataSchema, VIEW_FILTER_OPERATORS, VIEW_FILTER_OPERATOR_ALIASES } from '@objectstack/spec/ui';
22+
import {
23+
ViewMetadataSchema,
24+
VIEW_FILTER_OPERATORS,
25+
VIEW_FILTER_OPERATOR_ALIASES,
26+
VIEW_FILTER_LIST_VALUE_OPERATORS,
27+
VIEW_FILTER_PAIR_VALUE_OPERATORS,
28+
} from '@objectstack/spec/ui';
2329
import { graftNormalizedOperators } from './protocol.js';
2430

2531
/** Graft through the real spec schema, the way `saveMetaItem` does. */
@@ -31,6 +37,30 @@ function graftThroughSchema(authored: unknown): unknown {
3137
return graftNormalizedOperators(authored, parsed.data);
3238
}
3339

40+
/**
41+
* A `value` whose SHAPE the given canonical operator can carry (#6227).
42+
*
43+
* This suite's subject is the ALIAS FOLD, and the fold is only observable on a
44+
* rule that PARSES. Since #6227 the spec couples `value` to `operator` — the
45+
* three aliases that fold to `not_in` (`nin` / `notin` / `notIn`) need an array,
46+
* and a range needs two bounds — so a single hard-coded scalar would be refused
47+
* for a reason that has nothing to do with what is being tested.
48+
*
49+
* Read from the spec's own exported vocabularies rather than a local list, so a
50+
* list-valued operator added later cannot silently reintroduce the breakage:
51+
* that is the same "one declared vocabulary, not N dialects" rule the alias
52+
* table itself exists to serve.
53+
*/
54+
function valueFor(canonicalOperator: string): unknown {
55+
if ((VIEW_FILTER_LIST_VALUE_OPERATORS as readonly string[]).includes(canonicalOperator)) {
56+
return ['x'];
57+
}
58+
if ((VIEW_FILTER_PAIR_VALUE_OPERATORS as readonly string[]).includes(canonicalOperator)) {
59+
return ['x', 'y'];
60+
}
61+
return 'x';
62+
}
63+
3464
/** Flattened runtime view overlay — the shape a console personalization PUT sends. */
3565
const view = (filter: unknown, extra: Record<string, unknown> = {}) => ({
3666
name: 'showcase_task.open',
@@ -53,9 +83,9 @@ describe('graftNormalizedOperators — through the real view metadata schema', (
5383
it('canonicalizes every alias the spec still folds', () => {
5484
const canonical = new Set<string>(VIEW_FILTER_OPERATORS);
5585
const stillLegacy: string[] = [];
56-
for (const alias of Object.keys(VIEW_FILTER_OPERATOR_ALIASES)) {
86+
for (const [alias, foldsTo] of Object.entries(VIEW_FILTER_OPERATOR_ALIASES)) {
5787
const out = graftThroughSchema(
58-
view([{ field: 'status', operator: alias, value: 'x' }]),
88+
view([{ field: 'status', operator: alias, value: valueFor(foldsTo) }]),
5989
) as { filter: Array<{ operator: string }> };
6090
if (!canonical.has(out.filter[0].operator)) stillLegacy.push(alias);
6191
}

packages/spec/api-surface/ui.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,10 @@
327327
"UserFiltersParsed (type)",
328328
"UserFiltersSchema (const)",
329329
"VIEW_CONSOLE_ROW_DECORATIONS (const)",
330+
"VIEW_FILTER_LIST_VALUE_OPERATORS (const)",
330331
"VIEW_FILTER_OPERATORS (const)",
331332
"VIEW_FILTER_OPERATOR_ALIASES (const)",
333+
"VIEW_FILTER_PAIR_VALUE_OPERATORS (const)",
332334
"VIEW_METADATA_BRANCHES (const)",
333335
"VIEW_METADATA_MEMBERS (const)",
334336
"VIEW_WRITE_PATH_IDENTITY_KEYS (const)",

packages/spec/export-origins/ui.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,8 +327,10 @@
327327
"UserFiltersParsed": "src/ui/view.zod.ts#UserFiltersParsed (type)",
328328
"UserFiltersSchema": "src/ui/view.zod.ts#UserFiltersSchema (const)",
329329
"VIEW_CONSOLE_ROW_DECORATIONS": "src/ui/view.zod.ts#VIEW_CONSOLE_ROW_DECORATIONS (const)",
330+
"VIEW_FILTER_LIST_VALUE_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_LIST_VALUE_OPERATORS (const)",
330331
"VIEW_FILTER_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_OPERATORS (const)",
331332
"VIEW_FILTER_OPERATOR_ALIASES": "src/ui/view.zod.ts#VIEW_FILTER_OPERATOR_ALIASES (const)",
333+
"VIEW_FILTER_PAIR_VALUE_OPERATORS": "src/ui/view.zod.ts#VIEW_FILTER_PAIR_VALUE_OPERATORS (const)",
332334
"VIEW_METADATA_BRANCHES": "src/ui/view.zod.ts#VIEW_METADATA_BRANCHES (const)",
333335
"VIEW_METADATA_MEMBERS": "src/ui/view.zod.ts#VIEW_METADATA_MEMBERS (const)",
334336
"VIEW_WRITE_PATH_IDENTITY_KEYS": "src/ui/view.zod.ts#VIEW_WRITE_PATH_IDENTITY_KEYS (const)",

0 commit comments

Comments
 (0)