refactor(data)!: a select-list entry is a field name — the nested-select object form is removed (#4196) - #4268
Merged
Conversation
…ect object form is removed (#4196) `FieldNodeSchema` declared two forms for one entry of `QueryAST['fields']`: a string, and `{ field, fields?, alias? }` for a nested select on a relationship. The object form was declared-but-inert. Nothing produced it, and nothing read `.fields` or `.alias`: - `objectql/engine.ts` — formula projection and both known-field filters treat the list as `string[]`; one already reached for `String(f).split('.')[0]`, which is what defensiveness against an unhandled shape looks like. - `driver-sql` `select()`, `driver-memory` `projectFields` — same. - `driver-mongodb` keyed its projection with the entry itself, so an object entry asked for a column literally named `"[object Object]"`. #4194 taught it to read `.field` because the newly-precise type made the old line a compile error — defensive handling of a shape nothing sends, not evidence of life. - The REST ingress stringified each entry before comparing it to the field map, so the same entry came back as `400 INVALID_FIELD: Unknown field '[object Object]'` — a rejection naming something the caller never wrote. So an author who wrote `fields: [{ field: 'owner', fields: ['name'] }]` got it accepted by validation and then dropped or mangled, depending on the driver (ADR-0078 silently-inert declaration; ADR-0049 enforce-or-remove). The capability it described is already served by `expand`, which the engine resolves through batch `$in` queries. Removing the second spelling rather than lowering it into the first is Prime Directive #12: one capability, one contract. `FieldNode` is now `string`; `FieldNodeSchema` is `z.string()` with an error map that answers an object entry with the FROM → TO prescription instead of zod's "expected string, received object" — and only an object, since telling the author of `fields: [42]` that their entry "was removed" would misinform. `z.input` becomes `string`, so `tsc` fails at the authoring site first. `assertProjectionFieldsExist` gains the shape axis, judged BEFORE the object's field map: the entry is wrong about its shape, not about this object, and a registry-less host (which the name gate gives up on) would otherwise hand it to a driver that cannot read it. Registered as an ADR-0087 D3 semantic migration (`query-field-node-object-form-retired`) rather than a D2 conversion: `QueryAST` is a request shape, never stored in stack metadata, so the chain has no source to rewrite. Same treatment as `EnhancedApiError.fieldErrors` and `BatchOptions.validateOnly` on this step. Two spec tests that pinned the object form as parseable are inverted — they proved the DECLARATION, which is exactly how a declared-but-inert shape survives a green suite. New pins cover the prescription, the non-object mistake keeping zod's own message, and the `expand` replacement; the list path gets three more in the #4226 conformance suite. No runtime behaviour changes for anything that ever worked; the defensive unwrapping the drivers had grown against a shape nothing sends goes with it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EskBioEkCCJpGz3SvPQDxx
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
documentation
Contributor
📓 Docs Drift CheckThis PR changes 6 package(s): 111 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…ed-select-form-tygnzg
…d FieldNode form #4252 landed while this branch was open and pinned the INPUT half of every recursive schema — including two probes that assert the `{ field, fields, alias }` select entry is assignable. Both changes merge without a git conflict and are jointly wrong: the exact shape AGENTS.md's multi-agent discipline #10 warns about. `FieldNode` is no longer recursive, so `FieldNodeSchema` infers both halves itself rather than carrying a `z.ZodType<Output, Input>` annotation. Its probes stay anyway — this file's job is that the input side is CHECKED, and a `z.string()` someone re-widens to `z.ZodType<FieldNode>` fails here exactly as a dropped type parameter would. The positive probe becomes a dotted path (the replacement the removal prescribes) and the object form joins the negative side. `wiredFieldNode` needs no change: `z.input<typeof FieldNodeSchema>` is `string`, so `42` is still the error the suppression expects. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EskBioEkCCJpGz3SvPQDxx
os-zhuang
marked this pull request as ready for review
July 31, 2026 02:12
os-zhuang
pushed a commit
that referenced
this pull request
Jul 31, 2026
…false (#3438) The boot line that names an open value-shape gate (#4253) fired on every deployment there is, and stated something untrue on any deployment that had already settled the posture with an environment switch. Both are one failure: an advisory that speaks where it does not apply is exactly how readers learn to ignore it, and the line's whole value is that a warn-first deployment is told once, credibly, which command ends warn mode. Neither defect is reachable from the suite that shipped with the announcement. `engine.test.ts` mocks `./registry`, and a mocked registry hands the engine exactly the fields the test wrote — so these cases get a sibling file built on the genuine `registerApp → registry` path, which is the only way a test sees what a deployment sees. `objectHasCoveredValueField` — the dormancy short-circuit meant to spare an object with no covered field the flag query — tested raw type membership, while the real registry INJECTS covered-type fields into every object it registers: `organization_id` and `owner_id` (both `system`), `created_by` and `updated_by` (both in `SKIP_FIELDS`), four `lookup`s. `validateRecord` skips every one of them before it reaches the value-shape check, so the short-circuit answered `true` for literally every object, never fired, and its WeakMap memoized a constant. It now counts by the validator's own `isScannableValueShapeField` — the predicate the scanner already imports. Three readings of "a covered field" drifting by one clause is how a gate ends up governing fields nothing enforces, which is the shape #4235 argued against. The announcement also consulted no environment switch, though both postures it reports on short-circuit ahead of the deployment flag it reads. Under `OS_DATA_VALUE_SHAPE_STRICT_ENABLED` enforcement is already on for both classes, so "checked but NOT enforced here" was false; under either opt-out the operator chose leniency deliberately, so naming a migration that cannot change what they get is noise. Each gate consults its own pair — `mediaPostureSetByEnv` and `valueShapePostureSetByEnv`, siblings for the reason `mediaStrictEffective` and `valueShapeStrictEffective` are siblings, since the opt-outs are per-class while the opt-in opens both. Cheapest test first, so a kernel with nothing to say still reaches no flag query. Enforcement is unchanged: same gates, same flags, same default. The flag read was already memoized per process, so what this corrects is the property ADR-0104 states, not a user-visible cost. Verified: objectql 1345 (7 new), cli 628, spec check:generated 8/8, objectql build, check:release-notes and check:doc-authoring pass, ESLint clean. One pre-existing failure on main is untouched by this branch and reproduces with zero of its source changes: `query-expression-conformance.test.ts` > "the removed nested-select object form is refused BY NAME", from #4268. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016zgA8CQMJeJbFEjnbib1Uv
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 #4196.
FieldNodeSchemadeclared two forms for one entry ofQueryAST['fields']:The issue's recommended disposition was remove, and the build agreed — nothing refuted it.
Why the object form had to go
No producer, and no consumer that read
.fieldsor.alias. Every reader on the path treats the list asstring[]:objectql/engine.ts— formula projection + both known-field filtersString(f).split('.')[0], which is what defensiveness against an unhandled shape looks like)driver-sqlselect(),driver-memoryprojectFieldsdriver-mongodb"[object Object]". #4194 taught it to read.fieldbecause the newly-precise type made the old line a compile error — defensive handling of a shape nothing sends, not evidence of lifeassertProjectionFieldsExist)400 INVALID_FIELD: Unknown field '[object Object]', a rejection naming something the caller never wroteSo
fields: [{ field: 'owner', fields: ['name'] }]was accepted by validation and then dropped or mangled depending on the driver — ADR-0078's silently-inert declaration, ADR-0049's enforce-or-remove. The capability it described is already served byexpand, resolved through batch$inqueries. Deleting the second spelling rather than lowering it into the first is Prime Directive #12: one capability, one contract.I also checked the other end of the wire:
objectstack-ai/objectuihas its own unrelated internalFieldNode(aQueryASTNodewithtype: 'field'), and none of its 23fields: [{ field: …sites is a select list — they are form sections, grid grouping, user filters and error envelopes. No producer there either.FROM → TO
fields: [{ field: 'owner', fields: ['name'] }]expand: { owner: { object: 'user', fields: ['name'] } }fields: [{ field: 'owner' }]fields: ['owner']fields: ['owner.name'](dotted path)fields: [{ field: 'total', alias: 't' }]aggregations/windowFunctions— they carry the livealiasThe one-line fix: a
fields[]entry is a string.The retirement kit
FieldNodeisstring;FieldNodeSchemaisz.string()with an error map answering an object entry with the prescription above instead of zod's "expected string, received object". Only an object gets it — telling the author offields: [42]that their entry "was removed" would misinform.z.inputbecomesstring, sotscfails at the authoring site first.assertProjectionFieldsExistgains the shape axis, judged before the object's field map: the entry is wrong about its shape, not about this object, and a registry-less host (which the name gate gives up on) would otherwise hand it to a driver that cannot read it.query-field-node-object-form-retired) rather than a D2 conversion +os migrate metastep — deliberately.QueryASTis a request shape, never stored in stack metadata, so the chain has no source to rewrite; same treatment asEnhancedApiError.fieldErrorsandBatchOptions.validateOnlyon this step. The protocol-18 rationale is extended to say so.as string[]casts andString(f)coercions the drivers and engine had grown against a shape nothing sends go with it.expandreplacement. Three more in the REST 列表：sort/select/expand指向不存在的字段时被静默丢弃（filter 轴已收口，这三条轴还没有） #4226 list-path conformance suite, including that a dotted path still works.query-syntax.mdxloses its "the engine drops non-string entries" row (there is nothing to drop now) and itsFieldNodetype block;data-engine.mdxand the generated references follow. Strictness ledger re-counted (query.zod.ts10 → 9z.object(sites).majorfor@objectstack/spec, carrying the FROM → TO table and the one-line fix.No liveness-ledger entry is involved:
FieldNodeis not a governed metadata type, and the union's members were never inauthorable-surface.json(the walker does not descend unions), so no baseline moved on that axis.The
mainmerge caught a jointly-wrong pair (second commit)#4252 landed while this branch was open, pinning the INPUT half of every recursive schema — including two probes asserting the
{ field, fields, alias }entry is assignable (recursive-schema-input-assertions.ts). The two changes merge with no git conflict and are jointly wrong: exactly the shape AGENTS.md multi-agent discipline #10 describes, and the reason for the pull-main-and-re-run-before-merging rule.FieldNodeis no longer recursive, soFieldNodeSchemainfers both halves itself rather than carrying az.ZodType<Output, Input>annotation. Its probes stay anyway — the file's job is that the input side is checked, and az.string()someone re-widens toz.ZodType<FieldNode>fails there exactly as a dropped type parameter would. The positive probe becomes a dotted path; the object form joins the negative side.wiredFieldNodeneeds no change (z.inputisstring, so42is still the expected error).Verification
pnpm test— green pre-merge (132/132). Re-run twice after mergingmain; both runs tripped a different wall-clock flake in a package this diff does not touch —metadata-core's ReDoS budget (expected 52.1 to be less than 50) andplugin-hono-server's graceful-drain — each passing in isolation on re-run, on a container running many suites in parallel. Neither package appears in this diff's 18 files. Remote CI is the referee.packages/specgate individually, post-merge:check:liveness,check:empty-state,check:authorable-surface,check:docs,check:api-surface,check:exported-any,check:spec-changes,check:upgrade-guide,check:variant-docs,check:strictness-ledger— all pass, pluscheck:generatedreporting all 8 generated artifacts up to date.gen:spec-changes/gen:upgrade-guideproduce no diff, and that is correct rather than a miss: both artifacts are bounded byprotocolVersion: 17.0.0, so the whole protocol-18 step (this change,requireAuth把 public 从"全局开关的副产品"升级为声明式能力,然后删掉 api.requireAuth 开关 #3963,validateOnlyBatchOptions.validateOnly 声明了 dry-run 但从不实现 —— "预演"会真实落库(PD #10) #4052,flow-retry-max-retries-requiredflow errorHandling.maxRetries 有两个默认值:schema 声明 0、引擎回退 3 —— 重试次数取决于流程有没有过 schema #4247) renders only whenPROTOCOL_VERSIONbumps.Out-of-scope finding — filed and already closed
check:generatedfailed its own ledger reconciliation when I hit it (check:strictness-ledger, added by #4232, classified in neitherGATEDnorNO_GENERATOR). Filed as #4267; #4252 had fixed it onmainminutes earlier, so the issue is closed as already-done rather than carried here.