fix(spec,lint): a formula field in searchableFields is refused loudly (#6674) - #7103
Merged
Merged
Conversation
…#6674) #4254 closed the fail-open on the unknown-name axis. The same shape survived one axis over, on names that are perfectly real: the declared branch of `resolveSearchFieldResolution` filtered by EXISTENCE only, so a `formula` field declared in `searchableFields` entered the allowed set — and the #4254 ingress gate, which reads that same set, accepted it for exactly that reason. A formula value is computed on read and no driver materializes a column for it, so the `$contains` the engine expands `$search` into has nothing to scan. Measured: 0 rows on driver-memory, 0 rows WITH NO ERROR on driver-sql. The declaration read as search coverage and delivered none. - spec (the deciding face): the declared branch filters on existence AND scannability; new `SEARCH_VIRTUAL_TYPES` / `isVirtualSearchField` are the one judgment resolution, gate and linter share. The resolution stays non-throwing — internal callers never pass an ingress, which is why #4254 put the loudness at the ingress. - metadata-protocol: 400 INVALID_FIELD under its own reason, split out before the declared/auto branch because both of those messages are wrong for it. - lint: a build error on the object's own set as well as a view's narrowing, under the existing `searchable-field-unsearchable` rule. The storage-not-taste carve-out is kept and pinned by controls in all three packages: a declared `json` / `lookup` column is still executed, because it has a column and CAN match. Corpus sweep of objectstack + objectui + cloud: zero authored declarations affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 3 package(s): 106 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 7 release-owned page(s) also reference the affected code. These are read-only:
|
github-actions
Bot
added
documentation
…ports `check:api-surface` (inside the TypeScript Type Check job) judged the public surface "0 breaking, 2 added" and asked for the snapshot. Both additions are intentional and are the design's centre: `SEARCH_VIRTUAL_TYPES` and `isVirtualSearchField` are the ONE judgment the spec resolution, the #4254 ingress gate and the linter all read, so that they cannot drift about which field types have a stored column. Snapshot delta is exactly those two names in api-surface/data.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
…rchable-formula-fields
… baseline PR #7090 landed `check:export-origins` after this branch was cut, so the merge of `origin/main` brought a required gate the branch had never satisfied: the `data` shard was stale for the two exports this PR adds. `pnpm --filter @objectstack/spec gen:export-origins` — one shard rewritten, two added lines, both resolving to the single declaration site `src/data/search-fields.ts`. No re-homed origin and no second origin for an existing name, so this is not the #4411 dual-source trap the gate warns about; `check:dual-source-exports` agrees (0 new). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PiRUoQkTSBBmpyXBY3cVn2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #6674
The #4254 fail-open, surviving one axis over on names that are perfectly real.
resolveSearchFieldResolution's declared branch filtered entries by EXISTENCEonly, so a
formulafield declared insearchableFieldsentered the allowedset — and the #4254 ingress gate, which reads that same set, accepted it for
exactly that reason.
Ruling this implements (triage promotion, quoted verbatim)
The wording half (#6673) is a different card and is NOT folded in — its two hint
strings are already fixed on
mainand are untouched here.1. Corpus count — the STOP gate, run first
Zero authored
searchableFieldsdeclarations name a formula-typed field, inany of the three repos. Nothing in the tree changes verdict, so the STOP does not
fire and no fixture needed correcting.
Two passes per repo, the second deliberately over-inclusive so a hit cannot hide:
A = same-file (the object's
fieldsand itssearchableFieldsin one file,the authoritative reading); B = every formula field NAME found anywhere in the
repo matched against every
searchableFieldsentry found anywhere, which catchescross-file declarations pass A cannot see — at the cost of matching unrelated
objects that share a field name.
searchableFieldssearchableFieldsdeclarations / entries4e6ca32d85e52495485cbd3Control probes (#4878 — proving the scan saw the trees rather than reading an
empty directory): every ingredient count above is non-zero in all three repos —
the scan found
searchableFieldsdeclarations and formula-typed fields andresolved entry names in each. A scan that saw nothing would have reported zeros
in those columns, not just in the hit columns.
The three pass-B rows, adjudicated individually — all the same false positive:
packages/objectql/src/query-expression-conformance.test.tsdeclaressearchableFields: ['title']on a fixture whosetitleistype: 'text'; theformula-typed
titlelives on an unrelated fixture object inpackages/spec/src/ai/solution-blueprint.test.ts. Different objects, same fieldname — which is exactly the over-inclusion pass B trades for cross-file reach.
Not a hit.
The two authored example-app files carrying
searchableFields(
examples/app-showcase/src/data/objects/account.object.ts,examples/app-showcase/src/ui/pages/renewals-pipeline.page.ts) declare no formulaentries.
2. Premise — reproduced before implementing
Predicted directions were written down before any probe ran; all six matched.
4e6ca32d8resolveSearchFieldResolution, declared formula{"allowed":["name","project_name_formula"],"source":"declared"}{"allowed":["name","project_name"],"source":"auto"}expandSearchToFilter('Apollo', …)$containsover the virtual column{"$or":[{"name":{"$contains":"Apollo"}},{"project_name_formula":{"$contains":"Apollo"}}]}validateSearchableFields[]— and the list-view narrowing was silent too, because the declared set had admitted the entrysearchFields=name) 1 row$containserr = null(control: 1 row)P1 reproduces the card's transcript byte for byte. P6 is the one worth naming: the
SQL backend does not error on the missing column, it answers — so the failure is
indistinguishable from "no record matches".
Why it can never match: a formula value is computed on read and no driver
materializes a column for it —
driver-sql/src/schema-drift.tsfieldHasColumn,driver-turso/src/remote-transport.ts("Virtual — no column"), andobjectql/src/engine.tsbuildFormulaPlan, which excludes formula fields fromthe projection it sends down precisely because the driver would fail on the name.
3. What changed, and where the deciding face is
The triage routing comment named spec's resolution semantics as the deciding face
and the lint edit as the enforcement tail. That held: the fix is a change to what
a declaration is ADMITTED to say, with two enforcement faces made loud from the
same judgment.
packages/spec/src/data/search-fields.ts— the deciding face. The declaredbranch now filters on existence AND scannability. New
SEARCH_VIRTUAL_TYPES(exactly
formula, pinned) andisVirtualSearchField— one judgment, soresolution, gate and linter cannot drift about which types have a column, the
same one-source move REST 读路径：
searchFields/groupBy/aggregations指向不存在的字段时被静默降级（#4226 收口后剩下的三条轴） #4254 made between gate and engine. The function staysnon-throwing on purpose: it is consulted on every search by internal callers
(hooks, flows, registry-less hosts) that never pass an ingress, which is exactly
why REST 读路径：
searchFields/groupBy/aggregations指向不存在的字段时被静默降级（#4226 收口后剩下的三条轴） #4254 put the loudness at the ingress rather than in the resolution.packages/metadata-protocol/src/protocol.ts—400 INVALID_FIELDunder itsown reason class, split out before the declared/auto branch because both of
those messages are wrong for it: "outside the declared set" is false when the
entry IS in the list, and the auto-default's "declare
searchableFieldstochoose the searchable set explicitly" would instruct the author to write the
very declaration being refused. The message names the field, its type, that the
value is computed on read and never stored, and the fix.
packages/lint/src/validate-searchable-fields.ts— a build error atauthoring time, on the object's own
searchableFieldsas well as a view'snarrowing, under the existing
searchable-field-unsearchablerule (no new ruleid, so no new suppression surface to document).
The carve-out that is deliberately kept. #4830 left the canonical surface
existence-only on the grounds that a declaration is the author's choice and the
engine executes it. That reasoning still holds for everything with a column: a
declared
jsonorlookupentry runs a$containsover the stored JSON text orthe stored foreign key — narrow, rarely useful, but a scan that CAN match, so it
is neither a 400 nor a finding. The dividing line this PR draws is storage, not
search quality, and each of the three packages carries a control pinning it, so
a later reading of #6674 as "the declared branch is type-filtered after all" goes
red rather than silently rejecting metadata the runtime accepts (ADR-0072 D1).
4. Reverse verification — direction predicted first, then measured
Predicted: the new pins go RED with the fix removed; neighbours stay GREEN. Run
per face, in order, each restored before the next.
spec/data/search-fields.tsmetadata-protocol/src/protocol.tslint/src/validate-searchable-fields.tsThe controls staying green in runs 2 and 3 is the informative half: they show the
refusal messages are load-bearing, not merely a side effect of the spec drop — with
the spec change alone the entry is dropped from
allowedbut the gate answers withthe wrong neighbour message, which the pins reject.
Acceptance/rejection matrix, all measured against a real
ObjectQLengine:$searchFieldsINVALID_FIELD(code+statusasserted per ADR-0112/#6142)name)?search=on the same objectjson/lookupentry5. Stock compatibility (acceptance tightening)
Measured on the load path, for an already-published object whose
searchableFieldscarries a formula field:searchableFieldsis stillz.array(z.string()); this is a resolution and enforcement rule, not a shaperule. The object registers, and every non-search operation is untouched.
?search=returns the SAME rows. The dropped entry matched none ofthem before, so the result set is byte-identical (pinned as a control above).
200with no rowsto
400 INVALID_FIELD. This includes objectui's list search, which echoesschema.searchableFieldsverbatim as$searchFields— that is the real-worldblast radius, and the corpus count above bounds it at zero.
searchableFieldsis ENTIRELY formula entries filters to empty and fallsthrough to the auto-default, exactly as an all-stale declaration has since
REST 读路径：
searchFields/groupBy/aggregations指向不存在的字段时被静默降级（#4226 收口后剩下的三条轴） #4254. Search widens there, from "matched nothing, ever" to the auto-defaultset. It is not left silent: the linter reports the declaration as a build error.
6. Verification run
All heavy runs under
flock /tmp/os-heavy-verify.lock.(Re-run after the merge lap — see section 7.)
Gates from
.github/workflows/lint.yml, one by one — all green:check:nul-bytes(6527 files, plus a manualgrep -naPcontrol-byte self-scan ofevery file touched),
check:adr-0087-registration --base origin/main("this PR adds no declared-breaking changeset" — the same disposition #4254's
enforcement widening was graded, hence
patch),check:empty-changeset,check:error-code-casing,check:route-envelope,check:doc-authoring,check:quick-reference-counts,check:skill-frame-sync,check:docs-audit-scope,check:adr-anchors, and spec'scheck:generated --reconcile-only,check:authorable-surface,check:docs,check:skill-docs,check:skill-refs,check:skill-examples,check:api-surface,check:exported-any,check:dual-source-exports,check:spec-changes,check:upgrade-guide,check:liveness,check:export-origins.pnpm gen:schema && pnpm gen:docsre-run because thesearchableFieldsdescribechanged; the only committed generated delta is that one row in
content/docs/references/data/object.mdx.One CI lap was spent on a gate this list originally missed. The first push
went red on TypeScript Type Check at
check:api-surface: the two new publicexports on
./datawere judged "0 breaking, 2 added" with the snapshot notregenerated. Both additions are intentional and are the design's centre —
SEARCH_VIRTUAL_TYPESandisVirtualSearchFieldare the one judgment the specresolution, the ingress gate and the linter all read. Fixed in
ce9e454acbypnpm --filter @objectstack/spec gen:api-surface; the snapshot delta is exactlythose two names in
packages/spec/api-surface/data.json, and the remainingspec-surface gates were swept in the same lap rather than discovered one at a
time.
One changeset (
patch× spec / metadata-protocol / lint).7. Merge lap onto post-#7090 main
PR #7090 landed
check:export-originsafter this branch was cut — a requiredgate the branch had never satisfied, and one that would have bounced it from the
merge queue. Merged
origin/main(f5a9bc2f3) in: no source conflicts;generated artifacts were regenerated from the merged tree rather than
hand-merged.
gen:export-origins→ one shard rewritten, and the diff is exactly the twonames this PR adds, both resolving to the single declaration site
src/data/search-fields.ts#…. No re-homed origin and no second origin for anexisting name, so this is not the spec 同名双源:两个
MetadataWatchEvent形状不同、分挂两个子路径入口,其中 kernel 版零消费方(ADR-0049 enforce-or-remove) #4411 dual-source trap the gate's own messagewarns about —
check:dual-source-exportsagrees (0 accepted dual-source).check:export-origins✓ (4993 exports across 16 entry points resolve exactly as recorded, self-test passing),check:generated --reconcile-only✓ (now20 check: + 14 gen:scripts, all classified),check:api-surface✓(
unchanged— the earlier snapshot still holds on the merged tree),check:dual-source-exports✓.the point of re-running: spec
355 files / 9281 tests, lint68 / 1775,metadata-protocol
66 / 852, objectql163 / 2798— all passed. Themetadata-protocol rise matters most: main's org 作用域的 flow overlay 只在「本进程内发布后」绑定触发器,重启后静默失绑——冷启动两条读路径都把 organization_id 非空的行滤掉了 #6190 work touched the same
protocol.tsthis PR edits, and its tests pass alongside the new gate's.Head
b3d8e80b4. CI re-converged on it: 26/26 checks green (Console Pin Gateskipped), ESLint ✓ 18:15:05Z and TypeScript Type Check — the job that runscheck:export-origins— ✓ 18:25:57Z.Notes for the lander
origin/mainatf5a9bc2f3, so the branch carries the test(spec): export-surface pins compare a build-time baseline instead of running tsc (#4796) #7090 gateand satisfies it. Generated artifacts were regenerated, never hand-merged.
Generated by Claude Code