Skip to content

fix(service-analytics)!: refuse an analytics order key that names no member the query selects - #21314

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21267-analytics-order-key-selected
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/issue-21267-analytics-order-key-selected

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21267
Clause-②: no (narrowing)

What changed

The analytics door now refuses an order key that names no member the query selects, with 400 INVALID_FIELD, on the native-SQL and the ObjectQL face alike, before either strategy runs. This is triage's ruling (a) on the card (comment 5943018488).

The rule. An order key must be a column the answer carries: one of the query's own dimensions entries, one of its measures entries, or a timeDimensions entry that carries a granularity. It must be spelled exactly as it is selected. A timeDimensions entry with only a dateRange bounds the rows and is not a column, so it is not orderable. The spelling is exact because the answer keys each column by the spelling the caller sent (a cube-qualified measure keeps its qualifier), and both strategies write the key into ORDER BY as that column's name. The member-bearing keys were read off AnalyticsQuerySchema in packages/spec/src/data/analytics.zod.ts: measures, dimensions and timeDimensions[].dimension. where names filter members, not columns. Spec is not edited.

  • order-key-door.ts (new, internal to the package, not exported from index.ts). assertOrderKeysSelected(query) throws the door's existing member-gate envelope: code: 'INVALID_FIELD', status: 400, param: 'order', field (the first offending key). The message names every offending key and every member the query does select, and says the query was not run. ⛔ No new error code.
  • One call site: AnalyticsService.callCtx. That is the one seam query() (/analytics/query, and every query DatasetExecutor runs) and generateSql() (/analytics/sql) share, so both doors and both faces answer alike. It runs after ensureCube (an unknown cube still answers 404 first) and after the admission verdicts (object, stored metadata body, field read, field query). It runs before the read scopes are resolved and before any strategy is selected. ⛔ No per-strategy copy.
  • One definition of "projected". ObjectQLStrategy.projectedDimensions (which groups, maps rows and describes fields[]) now delegates to the door module's projectedDimensions. The door and the strategy read the same rule for which time dimensions are columns.

Measured: POST /api/v1/analytics/query and /sql on the real dispatcher route

Setup: AnalyticsServicePlugin over a real ObjectQL engine and SqlDriver, a signed-in caller, the real dispatcher-plugin mount, SQLite in memory and a private PostgreSQL 16.14. The cube over deal declares no join. owner is a lookup whose reference is a person object that also declares note and amount. The ObjectQL face is the same plugin narrowed to the engine-aggregate path. Before: origin/main at 3196ef1a1. After: this branch's service-analytics build at 4362776a3. The scratch probe was deleted.

query native SQLite, before → after native PostgreSQL, before → after ObjectQL face (both drivers), before → after
dimensions: ['owner.email'], order: { note } 500 DATABASE_ERROR → 400 INVALID_FIELD 500 (42702, note ambiguous) → 400 200 → 400
dimensions: ['note'], order: { amount } 200, ordered by an arbitrary row → 400 500 (42803, must appear in GROUP BY) → 400 200 → 400
dimensions: ['note'], order: { 'owner.email' } 500 → 400 500 (42703, no such column) → 400 200 → 400
CONTROL order: { note: 'desc' } (selected dimension) 200 → 200 200 → 200 200 → 200
CONTROL order: { amount_sum: 'desc' } (selected measure) 200 → 200 200 → 200 200 → 200

/sql (the dry run) answered 200 on every refused row before, with a statement whose ORDER BY names a column the statement does not select. It now answers the same 400. The control cells are byte-identical before and after: 16 cells (2 drivers × 2 faces × 2 routes × 2 controls), comparing status, rows and the echoed statement.

What the ObjectQL face answered today (triage's first measurement). It answered 200 for all three rows on both drivers. That is because its execution never applies order at all, not because it orders by anything. The selected-dimension control asked for desc and came back x, y, z on SQLite and z, x, y on PostgreSQL. Its echoed statement shows the same ORDER BY the native face could not run. See Acceptance notes.

Census (before the code change)

  • examples/** and packages/apps/**: zero hits. No shipped dashboard widget carries sortBy / sortOrder. The only sortBy text is a comment in examples/app-todo/src/dashboards/task.dashboard.ts:21. No report carries order. No dataset or cube carries one, and no page builds an analytics query with one. packages/apps/** issues no analytics query at all. The order: hits that do exist (active-projects.page.ts:28, review-queue.page.ts:34, the two task.view.ts files, embed-objectql/src/index.ts:58) are record-query sort entries on the data API, not analytics.
  • examples/app-showcase/src/ui/pages/command-center.page.ts (held by finding(examples): the app-showcase command-center KPI tiles write object-metric filter as a record, which the spec's own ComponentPropsMap['object-metric'].filter refuses ("takes the ViewFilterRule ARRAY form") #21251): no hit, so nothing to report. Its KPIs are object-metric scalars (aggregate with no order, lines 147–152). Its charts are dataset-bound with no sortBy (lines 158–167). Its work queue is an object-grid (line 172). Not edited.
  • objectui at the pinned .objectui-sha 31971ff1e28f: no hit. Its one /analytics/query sender is ObjectStackAdapter.aggregate() in packages/data-objectstack/src/index.ts. Its payload (lines 6278–6306) carries cube, measures, dimensions and where, never order. git grep for analytics.query( and for a cube: payload finds no other sender. Dashboard sortBy reaches the dataset door (selection.order), not this route.
  • In-repo producers: none besides the dataset executor. No AI tool, plugin or service other than DatasetExecutor calls the analytics query door with an order. The executor pushes an order down only when the selection is one query and every key is a dimension or unfiltered measure that query selects (canPushDownWindow, dataset-executor.ts:1136–1143). The dataset door refuses an unselected selection.order key itself first (resolveOrdering, 400 DATASET_INVALID). So this door never refuses a dataset selection the dataset door accepted.

A census pin was not added. The population of shipped analytics queries with an order is zero, and a pin over an empty population asserts nothing. A pin that walks future dashboards would be a new gate, which the dispatch's axes default to "no". The dataset door already refuses an unselected widget sortBy at runtime.

Pins

New file: packages/services/service-analytics/src/__tests__/order-key-selected.test.ts. It uses the plugin's own composition over a real engine, a SQLite cell and a PostgreSQL cell (a named skip without OS_TEST_POSTGRES_URL), and both faces. Each refusal pin runs both doors (query and generateSql) on both faces. It asserts code, status, param and field, that the message names the key and every selected member, and that nothing ran (zero raw statements and zero engine aggregates). There are 16 tests, 8 per cell.

1–3. The card's three rows are refused 400 INVALID_FIELD on both faces and both doors.
4. A timeDimensions entry that only sets a dateRange is not a column, so ordering by it is refused.
5. Exact spelling: a cube-qualified spelling of a measure selected bare is refused.
6. CONTROL: ordering by a selected dimension is served. The native face returns the requested order (z, y, x) and the ObjectQL face the same groups. /sql shows ORDER BY "note" DESC.
7. CONTROL: ordering by a selected measure is served. The native face returns x 15, y 7, z 1.
8. CONTROL: a bucketed time dimension is a column, so ordering by it is served (three month buckets).

Existing pins that now also hold this door's slot. A key naming a field the caller may not read still gets the field gate's 403, because the door runs after admission. That is pinned, unchanged and green, by field-read-admission-gate.test.ts ("an order key", "an expression member as an order key"), field-query-admission-gate.test.ts ("a masked field as an order key") and the route-level packages/rest/src/analytics-field-permission-gate.test.ts:300. A first draft placed the door ahead of admission. Those six service-level cases went red (a 400 replacing the 403), and the rest route pin would have too. That placement was dropped rather than the fixtures rewritten.

Ablations

The pin file imports the subject by relative path (../plugin.js), so each run reads src and there is no dist leg. Each mutation went through scripts/ablation-replace.mjs in WRAP mode, with an outer trap restore on EXIT INT TERM against the absolute path. Both ran from committed 4362776a3 with the PostgreSQL cell live. Predictions were written before each run.

ablation mutation (order-key-door.ts) predicted observed
A1 the refusal removed: if (unselected.length === 0) return; becomes a test that is always true (greater than or equal to 0) 10 red (pins 1–5 on each cell), 6 green (the controls) 10 failed / 6 passed, exactly pins 1–5 × 2
A2 the refusal widened to selected members: unselected = keys 6 red (the three controls on each cell), 10 green 6 failed / 10 passed, exactly the controls × 2

Each mutation landed: anchor 1 → 0, and the blob changed from 21103a6b4884 to A1 488d0e6274bb and A2 5c62c68e0315. Each was restored and proven: the blob equals the HEAD blob 21103a6b4884, and git diff HEAD is empty.

Docs

git grep over content/docs/** (excluding releases/) and skills/** for analytics order, sortBy and orderBy found no sentence or example this change makes false. Dashboards (ui/dashboards.mdx:161, skills/objectstack-ui/rules/dashboards.md:349) and reports (data-modeling/analytics.mdx:140) already say the sort key must be selected. The README example orders by a selected measure. Two sentences document the rule where /analytics/query had none:

  • content/docs/api/data-api.mdx, POST /analytics/query: a new callout after the where callout. It states the rule and the 400, says /sql refuses the same keys, and says "To order by a member, select it."
  • packages/services/service-analytics/README.md:100: the order row's empty description becomes the rule.

Changeset

.changeset/21267-analytics-order-key-selected.md: @objectstack/service-analytics minor, with Clause-②: no (narrowing), a BREAKING note, and the ADR-0087 disposition not-required (no-migration-prescription) with its reasons. See Deviations for minor.

Verification at 34d48294a

34d48294a is this branch's head. It carries origin/main at 4e530568a merged in (no overlap with this diff's files). Install and the full build (turbo run build, 72/72) were refreshed after the merge.

  • pnpm --filter @objectstack/service-analytics test, with the PostgreSQL cells live: 166 files, 3843 passed, 0 skipped, 0 failed. typecheck (tsc --noEmit): exit 0, and its program lists all 4 touched .ts files (--listFiles).
  • Consumer radius, against the post-merge build with OS_TEST_POSTGRES_URL set. These are the test files that drive the analytics service or its routes in packages/rest (21 files), packages/runtime (19), packages/drivers/driver-memory (26), packages/drivers/driver-sql (2) and packages/plugins/plugin-security (1). Results: 292/292, 551/551, 848/848, 60 passed / 1 skipped (a pre-existing skip in sql-driver-13714-aggregate-alias-single-identifier.test.ts), and 262/262. No failures. NOT MEASURED: the packages/qa/dogfood analytics files. They boot the example apps, which carry no analytics order (census above), and are left to the required Dogfood Regression Gate.
  • Gates: node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands derives 91 commands at 34d48294a, and all 91 exit 0. --ran reconciliation: "91 derived, 91 run, 0 NOT-MEASURED, 0 UNRUN", with every exit code recorded. No run left a managed block in AGENTS.md.
  • Lint, a declared narrowing: eslint --no-inline-config --format json over the 4 touched .ts files at 34d48294a reports 4 files, 0 errors and 0 warnings. ① All 4 are inside the population eslint.config.mjs lints (**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}, minus NEVER_LINTED). The .md / .mdx files match no files glob ("File ignored because no matching configuration was supplied"). ② The count of 4 is read from the JSON output. ③ The config enables no type-aware linting (--print-config gives parserOptions {"ecmaVersion":"latest","sourceType":"module"}, with no project), so this diff cannot move a verdict on an untouched file. The repo-wide pnpm lint is left to CI.

Acceptance notes

  • The ObjectQL face never applies order, limit or offset on /analytics/query, while its echoed statement says it does. This is a separate defect, reported to the seat and not handled here. ObjectQLStrategy.execute() hands engine.aggregate no ordering or window (objectql-strategy.ts:304–321), but its generateSql renders ORDER BY / LIMIT / OFFSET (:634–639). On the default composition every bucketed time-dimension query lands on this face, because the native face declines granularity. Measured at 34d48294a, SQLite and PostgreSQL 16.14: timeDimensions: [{ dimension: 'closed_on', granularity: 'month' }], order: { closed_on: 'desc' }, limit: 1 answers all three month rows, ascending on SQLite and 03, 05, 04 on PostgreSQL. The dataset door is unaffected: DatasetExecutor re-applies ordering and the window over the assembled grid.
  • The admission gate's order position (namedQueryFields) stays load-bearing. It still decides the 403 for an unselected key over a field the caller may not read, because the door runs after it.
  • The /sql dry run's answer changes only for a refused query: 200 with an unrunnable statement becomes 400.

Deviations


Generated by Claude Code

claude added 4 commits October 2, 2026 03:04
…member the query selects

An `order` key must be a column the answer carries: a `dimensions` entry,
a `measures` entry, or a `timeDimensions` entry with a `granularity`,
spelled exactly as selected. Anything else is refused INVALID_FIELD / 400
at the analytics door (query and the generateSql dry run), after
ensureCube and before strategy selection, so the native-SQL and the
ObjectQL face answer alike. The ObjectQL strategy's projection rule moves
into the door module so both read one definition.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…admission verdicts

callCtx is the one seam query() and generateSql() share, so the door has
one call site. Running it after the object, stored-metadata-body, field
read and field query gates keeps the 403 a field the caller may not read
gets in every position, the order key included.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…dd its changeset

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added the size/m label Oct 2, 2026
@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/service-analytics, touching 9 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/services/service-analytics/README.md), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

15 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx (via revenue_sum (literal, a string literal in a comment on a changed line))
  • content/docs/api/data-api.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected), revenue_sum (literal, a string literal in a comment on a changed line))
  • content/docs/api/error-catalog.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/api/error-handling-server.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/automation/hook-bodies.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/data-modeling/queries.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/data-modeling/schema-design.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/deployment/validating-metadata.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/kernel/contracts/data-engine.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/permissions/authorization.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/protocol/kernel/error-handling.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/protocol/objectql/index.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/protocol/objectql/query-syntax.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/protocol/objectql/types.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/ui/views.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))

⛔ 4 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v17/17-0.mdx (via ObjectQLStrategy (symbol, a top-level class))
  • content/docs/releases/v17/17-3.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/releases/v17/17-5.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected))
  • content/docs/releases/v17/17-6.mdx (via INVALID_FIELD (literal, a string literal in assertOrderKeysSelected), /api/v1/analytics/query (route, a path literal in a comment on a changed line))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/services/service-analytics/README.md) — pages documenting those are invisible to this run
  • 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 — 10 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 1371dc980cdf0d3128bee4a5441f2c6bec18f008 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 64cf7e8457094712e72a3e3b3d485db06bf5409a — the merge of head 34d48294a17830298d61b016c0282ae183e98ce4 into base 1371dc980cdf0d3128bee4a5441f2c6bec18f008, 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 64cf7e8457094712e72a3e3b3d485db06bf5409a && git checkout 64cf7e8457094712e72a3e3b3d485db06bf5409a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1371dc980cdf0d3128bee4a5441f2c6bec18f008 34d48294a17830298d61b016c0282ae183e98ce4 && git checkout -B drift-repro 1371dc980cdf0d3128bee4a5441f2c6bec18f008 && git merge --no-ff 34d48294a17830298d61b016c0282ae183e98ce4

node scripts/docs-audit/affected-docs.mjs --json 1371dc980cdf0d3128bee4a5441f2c6bec18f008

⚠️ 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 1371dc980cdf0d3128bee4a5441f2c6bec18f008 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

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