Skip to content

fix(objectql,metadata-protocol): refuse aggregate queries that answered a wrong table, and an empty aggregate list - #25935

Merged
objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-6vh3z0
Oct 26, 2026
Merged

objectstack-fleet[bot] merged 1 commit into
mainfrom
claude/project-thread-6vh3z0

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

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 aggregations list with 404 OBJECT_NOT_FOUND for a registered object. All four now answer 400, naming the position. The claim is 6407789444; the seat claim on #25819, 6408744205, names the same branch. Branch claude/project-thread-6vh3z0 is the card thread's harness-pinned branch: one commit, 46a0a7df, on main cb0a2fcf4f.

What changes

#25818, at the engine's aggregate entry (packages/objectql/src/aggregate-query-shape-door.ts, called from engine.ts right after the structured-JSON group door and the aggregate × field-type door):

query before (main cb0a2fcf4f, through findData) now
groupBy: [{ field: 'name', dateGranularity: 'month' }] on a text field 200 [{ name: null, n: 3 }] 400 INVALID_FIELD, path: groupBy.0.dateGranularity
the same on a number field 200 [{ amount: '1970-01', n: 3 }] 400 INVALID_FIELD
the same on a time field 200 [{ at_time: null, n: 3 }] 400 INVALID_FIELD
aggregations: [{ count, alias: 'n' }, { sum amount, alias: 'n' }] 200 [{ n: 60 }] 400 INVALID_QUERY, path: aggregations.1.alias
groupBy: ['status'], aggregations: [{ count, alias: 'status' }] 200 [{ status: 2 }, { status: 1 }] 400 INVALID_QUERY, path: aggregations.0.alias
two group entries projecting one column (opened_on by month and by year) 200 [{ opened_on: '2026', n: 1 }, { opened_on: '2026', n: 2 }] 400 INVALID_QUERY, path: groupBy.1.field
  • "A date field" is the declared type's class, read by declaredFieldClasses (having-filter.ts), which is the reading in-memory-aggregation.ts's calendarDayGroupFields buckets a date field by. A date (CALENDAR_DATE_TYPES) and a datetime (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), a formula, or a registry-less host. driver-sql's pushdown reads its own column registry (temporalFieldKind), so the engine door judges before either face.
  • A column name is a group entry's name, or alias ?? field for the object form (the name applyInMemoryAggregation and driver-sql project), and an aggregation's alias. An alias moves the group column, so groupBy: [{ field: 'status', alias: 's' }] with an aggregation aliased status is served.
  • Every door agrees because the check sits in engine.aggregate: the REST query route and its dispatcher twin (both through findData), the analytics ObjectQL strategy, the MCP data bridge, a flow or a hook. The engine's INVALID_FIELD / INVALID_QUERY with status: 400 reaches the wire through mapDataError unchanged (pinned below).
  • The two group entries projecting one column is the same defect class as the alias collision (one row column, two values) and closes in the same walk, so it rides here as a bounded in-place fix (same class, mechanical, same files, same gate family). Its base answer is the last row of the table above.

#25819, at findData (packages/metadata-protocol/src/protocol.ts, after assertAggregationFieldsExist): an aggregate axis sent as an empty list, with no non-empty axis beside it, answers 400 INVALID_QUERY, naming 'query.aggregations' (or 'query.groupBy', or both) as empty. { groupBy: ['status'], aggregations: [] } and { groupBy: [], aggregations: [count] } are served as before.

  • Where the 404 came from (measured, the card had not traced it): the routing read [] as "no aggregate" and handed the key to engine.find, whose unknown-option refusal (engine.ts:928 on base, called from find at :15190) is an uncoded Error reading "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 plus not) read that as 404 OBJECT_NOT_FOUND, "Object 'acct' is not registered".
  • What [] means on each door. REST and dispatcher (through findData): the 404 above, now 400. { groupBy: [] } alone took the same path to the same 404, so the refusal covers it too. engine.aggregate called 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).
  • No shipped caller sends [] 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 the count node; ObjectStackAdapter.aggregate forwards its caller's list; the analytics strategy calls engine.aggregate, not findData.

protocol.ts hunk: @@ -13435,0 +13436,31 @@, inside findData. 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 asserts code, status and httpStatus, the path and param, and that no driver read happened. Controls: date and datetime buckets with their real buckets, a formula and an undeclared bucket, an alias that frees the field name, two buckets of one field under their own aliases. A REST-parity block runs findData and mapDataError for the three card shapes and aggregate query: an empty aggregations: [] answers 404 OBJECT_NOT_FOUND "Object … is not registered" for a registered object #25819's empty list (wire 400, never 404).
  • 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.
  • Red first. With engine.ts and protocol.ts reverted to main (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 test then typecheck: lock VERDICT command-exit 0, Test Files 264 passed | 3 skipped, Tests 21496 passed | 19 skipped.
  • pnpm --filter @objectstack/objectql test then typecheck: VERDICT command-exit 0, Test Files 468 passed, Tests 8725 passed.
  • Consumer spot-check (not owed; the engine refuses shapes it used to serve): the 31 @objectstack/rest test files that name aggregations or dateGranularity (31 passed, 516 tests) and pnpm --filter @objectstack/service-analytics test (185 files passed, 4517 tests).
  • eslint --no-inline-config packages/objectql packages/metadata-protocol: exit 0.
  • Derived gates: dispatch-gates --commands derived 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; after git fetch --shallow-since=2026-07-21) and check:dual-build-cjs-loads (exit 3 before a full turbo run build, 121 of 121 tasks).
  • Dogfood: n/a (no ! in the title, no narrowing arm).

Acceptance notes

  • Uncoded engine option refusal reads as an unknown object. rejectUnknownEngineOptions throws a bare Error whose words quote the object name, so any such refusal that reaches a REST data door is mapped to 404 OBJECT_NOT_FOUND by 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.
  • The engine door's no-axis answer. engine.aggregate(object, {}) and { aggregations: [] } return the raw rows (ObjectRepository.aggregate defaults its query to {}). The REST door now refuses the same body. No shipped in-process caller was measured sending it. Observation only, no carrier.
  • A formula under 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 one null bucket. Observation only, no carrier.

Generated by Claude Code

…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
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 26, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/metadata-protocol, @objectstack/objectql, touching 20 documentable anchor(s).

32 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json 2a728ba2acf52c13d1918c2afca7b79bf391e1f5.

⛔ 14 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 26 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 2a728ba2acf52c13d1918c2afca7b79bf391e1f5 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6c2989e94eff3ab12ac015638f16df317de43384 — the merge of head 46a0a7dff1b7a87832027add95573531ea8fbf97 into base 2a728ba2acf52c13d1918c2afca7b79bf391e1f5, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# 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

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 2a728ba2acf52c13d1918c2afca7b79bf391e1f5 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对 · ACCEPT · domain:server seat 1 · os-project-manager · session_01R1q2xdtYgU6FKrZ7LgjPCC · 2026-10-26T04:51Z

Head 46a0a7dff1, report 6408493005, claim 6407789444.

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

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 26, 2026 05:22
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 26, 2026 05:22
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 26, 2026
Merged via the queue into main with commit 7a04ba2 Oct 26, 2026
53 of 54 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-6vh3z0 branch October 26, 2026 05:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants