Repository navigation
fix(objectql,metadata-protocol): refuse aggregate queries that answered a wrong table, and an empty aggregate list - #25935
Conversation
…ed a wrong table, and an empty aggregate list engine.aggregate refuses a dateGranularity over a field declared neither a date nor a datetime (INVALID_FIELD, path groupBy.N.dateGranularity), and an aggregated row that would carry two columns under one name (INVALID_QUERY, path at the second position). findData refuses an empty aggregations or groupBy list with no non-empty axis beside it (INVALID_QUERY), which used to reach engine.find and come back as 404 OBJECT_NOT_FOUND. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018QKYDyfAfcAWGKY4UCv3e3
📓 Docs Drift CheckThis PR changes 2 package(s): 32 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 14 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 6c2989e94eff3ab12ac015638f16df317de43384 && git checkout 6c2989e94eff3ab12ac015638f16df317de43384
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2a728ba2acf52c13d1918c2afca7b79bf391e1f5 46a0a7dff1b7a87832027add95573531ea8fbf97 && git checkout -B drift-repro 2a728ba2acf52c13d1918c2afca7b79bf391e1f5 && git merge --no-ff 46a0a7dff1b7a87832027add95573531ea8fbf97
node scripts/docs-audit/affected-docs.mjs --json 2a728ba2acf52c13d1918c2afca7b79bf391e1f5
|
席位核对 · ACCEPT ·
|
| item | reading |
|---|---|
| Shape | draft, base main, Fixes #25818 + Fixes #25819, Clause-②: no and Metadata-protocol: none line-initial, patch changeset. |
| Files (6, +499/−0) | objectql/src/aggregate-query-shape-door.ts (new) + its hook in engine.ts (8 lines), the #25819 refusal in metadata-protocol/src/protocol.ts (one hunk at 13436, beside assertAggregationFieldsExist; disjoint from #25826 and from #25467 cut 12), two tests, one changeset: inside the claim's surface. No packages/spec/**, no classification edit, so no contract review is owed. Not governed (check-governed-merges --pr 25935), 499 changed lines. |
| Fix | at the entry of engine.aggregate, so every door that reaches the engine agrees (REST query and its dispatcher twin, analytics, MCP bridge, flows, hooks): dateGranularity over a field not declared date/datetime → INVALID_FIELD 400 with path: groupBy.N.dateGranularity (predicate reused: declaredFieldClasses); two aggregated-row columns under one name → INVALID_QUERY 400 naming both positions. #25819: an empty aggregations/groupBy with no non-empty axis → INVALID_QUERY 400 naming query.aggregations; the 404 is traced to engine.ts:928's uncoded unknown-option error read by data-error-classification.ts:2034. Seat's own read: the extra arm (two group entries projecting one column) is the same overwrite hazard found by measurement, bounded and in place. |
| Evidence | the module header's measured table on cb0a2fcf4f (six 200-with-wrong-data shapes); red-first tests; no shipped caller sends [] meaning plain rows (report). |
| Gates | 73/73 derived exit 0, --ran 0 NOT-MEASURED; both packages' test + typecheck green, lint 0. CI at this act: in progress. |
| Landing | ready + auto-merge through the relay once every check on this head is green or an expected skip. |
Generated by Claude Code
Requested by Leehom · project thread
Fixes #25818
Fixes #25819
Clause-②: no
Metadata-protocol: none
Ruling-ref: 6399831537
The aggregate door answered three malformed queries with a plausible wrong table under 200, and an empty
aggregationslist with404 OBJECT_NOT_FOUNDfor a registered object. All four now answer 400, naming the position. The claim is6407789444; the seat claim on #25819,6408744205, names the same branch. Branchclaude/project-thread-6vh3z0is the card thread's harness-pinned branch: one commit,46a0a7df, onmaincb0a2fcf4f.What changes
#25818, at the engine's
aggregateentry (packages/objectql/src/aggregate-query-shape-door.ts, called fromengine.tsright after the structured-JSON group door and the aggregate × field-type door):maincb0a2fcf4f, throughfindData)groupBy: [{ field: 'name', dateGranularity: 'month' }]on atextfield200 [{ name: null, n: 3 }]400 INVALID_FIELD,path: groupBy.0.dateGranularitynumberfield200 [{ amount: '1970-01', n: 3 }]400 INVALID_FIELDtimefield200 [{ at_time: null, n: 3 }]400 INVALID_FIELDaggregations: [{ count, alias: 'n' }, { sum amount, alias: 'n' }]200 [{ n: 60 }]400 INVALID_QUERY,path: aggregations.1.aliasgroupBy: ['status'], aggregations: [{ count, alias: 'status' }]200 [{ status: 2 }, { status: 1 }]400 INVALID_QUERY,path: aggregations.0.aliasopened_onby month and by year)200 [{ opened_on: '2026', n: 1 }, { opened_on: '2026', n: 2 }]400 INVALID_QUERY,path: groupBy.1.fielddeclaredFieldClasses(having-filter.ts), which is the readingin-memory-aggregation.ts'scalendarDayGroupFieldsbuckets adatefield by. Adate(CALENDAR_DATE_TYPES) and adatetime(INSTANT_TYPES) pass. A field the declaration cannot type is not judged: undeclared (the ingress door answers an unknown one first; a system column the map does not declare buckets as before), aformula, or a registry-less host.driver-sql's pushdown reads its own column registry (temporalFieldKind), so the engine door judges before either face.alias ?? fieldfor the object form (the nameapplyInMemoryAggregationanddriver-sqlproject), and an aggregation'salias. An alias moves the group column, sogroupBy: [{ field: 'status', alias: 's' }]with an aggregation aliasedstatusis served.engine.aggregate: the REST query route and its dispatcher twin (both throughfindData), the analytics ObjectQL strategy, the MCP data bridge, a flow or a hook. The engine'sINVALID_FIELD/INVALID_QUERYwithstatus: 400reaches the wire throughmapDataErrorunchanged (pinned below).#25819, at
findData(packages/metadata-protocol/src/protocol.ts, afterassertAggregationFieldsExist): an aggregate axis sent as an empty list, with no non-empty axis beside it, answers400 INVALID_QUERY, naming'query.aggregations'(or'query.groupBy', or both) as empty.{ groupBy: ['status'], aggregations: [] }and{ groupBy: [], aggregations: [count] }are served as before.[]as "no aggregate" and handed the key toengine.find, whose unknown-option refusal (engine.ts:928on base, called fromfindat:15190) is an uncodedErrorreading "find('acct') does not recognise option 'aggregations'…". The REST mapping's unknown-object limb (packages/types/src/data-error-classification.ts:2034: the quoted object name plusnot) read that as404 OBJECT_NOT_FOUND, "Object 'acct' is not registered".[]means on each door. REST and dispatcher (throughfindData): the 404 above, now 400.{ groupBy: [] }alone took the same path to the same 404, so the refusal covers it too.engine.aggregatecalled directly with neither axis returns the rows unchanged (the in-memory aggregator's documented no-axis answer); this PR does not change the engine door (see Acceptance notes).[]meaning plain rows, so the ruling's default holds. Measured producers:objectAggregateSpecQuery(@object-ui/core) always sends one node;compileListViewGroupQuery(@objectstack/spec/ui) always leads with thecountnode;ObjectStackAdapter.aggregateforwards its caller's list; the analytics strategy callsengine.aggregate, notfindData.protocol.tshunk:@@ -13435,0 +13436,31 @@, insidefindData. It does not overlap #25826 (lines 9021–9060) or #25467 cut 12 (23440–24900).Tests
packages/objectql/src/engine-aggregate-query-shape-door.test.ts(7 tests): every refusal assertscode,statusandhttpStatus, thepathandparam, and that no driver read happened. Controls:dateanddatetimebuckets with their real buckets, aformulaand an undeclared bucket, an alias that frees the field name, two buckets of one field under their own aliases. A REST-parity block runsfindDataandmapDataErrorfor the three card shapes and aggregate query: an emptyaggregations: []answers 404 OBJECT_NOT_FOUND "Object … is not registered" for a registered object #25819's empty list (wire400, never404).packages/metadata-protocol/src/protocol.empty-aggregate-axis.test.ts(6 tests): four refused shapes (code,status,param,object, engine never asked), plus the served controls.engine.tsandprotocol.tsreverted tomain(tree-only, restored from saved copies, the diff back to 39 lines before the commit): objectql suite 7 of 7 failed (the control's failure was a read count in the test, corrected); metadata-protocol suite 4 failed, 2 controls passed. With the fix: 7/7 and 6/6.All at
46a0a7dff1:pnpm --filter @objectstack/metadata-protocol testthentypecheck: lockVERDICT command-exit 0, Test Files 264 passed | 3 skipped, Tests 21496 passed | 19 skipped.pnpm --filter @objectstack/objectql testthentypecheck:VERDICT command-exit 0, Test Files 468 passed, Tests 8725 passed.@objectstack/resttest files that nameaggregationsordateGranularity(31 passed, 516 tests) andpnpm --filter @objectstack/service-analytics test(185 files passed, 4517 tests).eslint --no-inline-config packages/objectql packages/metadata-protocol: exit 0.dispatch-gates --commandsderived 73 families; all 73 ran with exit 0, reconciled by--ran(73 run, 0 NOT-MEASURED). Two needed their prerequisite first, then re-ran green:check-engine-split-ratio --days 90(exit 2 on the shallow clone; aftergit fetch --shallow-since=2026-07-21) andcheck:dual-build-cjs-loads(exit 3 before a fullturbo run build, 121 of 121 tasks).!in the title, no narrowing arm).Acceptance notes
rejectUnknownEngineOptionsthrows a bareErrorwhose words quote the object name, so any such refusal that reaches a REST data door is mapped to404 OBJECT_NOT_FOUNDby the quoted-name limb. After this PR no measured REST path reaches it (the empty-list path was the one measured). Observation only, no carrier.engine.aggregate(object, {})and{ aggregations: [] }return the raw rows (ObjectRepository.aggregatedefaults its query to{}). The REST door now refuses the same body. No shipped in-process caller was measured sending it. Observation only, no carrier.formulaunder a date bucket is not judged, because the declaration names no stored value class for it. On base, a number-expression formula bucketed by month answered onenullbucket. Observation only, no carrier.Generated by Claude Code