Skip to content

fix(spec)!: an aggregation entry refuses an undeclared key by name on every door, REST matching MCP (#28609) - #28706

Merged
objectstack-fleet[bot] merged 8 commits into
mainfrom
claude/project-thread-i0hitl
Nov 1, 2026
Merged

objectstack-fleet[bot] merged 8 commits into
mainfrom
claude/project-thread-i0hitl

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Requested by Leehom · project thread

Fixes #28609

Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — the published skills (skills/objectstack-query, skills/objectstack-api) and the docs teach only the declared entry members (function, field, alias, filter) and never an extra entry key, so nothing they teach changes; the refusal itself names the key and the spelling to send.

维护者速读

  • 改了什么: 查询里的聚合条目(aggregations[] 的每一项)现在在 spec 源头就关闭:写了 spec 没声明的键(比如拼错的 bogusKey,或把条目自己的 filter 写成 where)会被点名拒绝。REST、运行时分发器和 MCP 的 aggregate_records 读同一个 schema,拒绝文字一字不差。
  • 为什么改: 上一张卡(MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550)让 MCP 先拒绝了这种键,而 REST 仍然悄悄丢掉它照常返回 200,同一个条目在两个入口得到两种结论。
  • 风险与回滚: 发过多余条目键的调用方(REST、分发器、进程内)会从 200 变成 400 VALIDATION_FAILED;只含声明键的查询不变。回滚即还原 query.zod.ts 一处和 MCP 一处。
  • 先看: packages/spec/src/data/query.zod.ts(AggregationNodeSchema 变成 strictObject)、packages/mcp/src/mcp-http-tools.ts(删掉 MCP 自己那份条目副本,直接用 spec 节点)、packages/rest/src/aggregation-entry-undeclared-key-door.test.ts。
  • 两分钟验证: pnpm --filter @objectstack/rest exec vitest run src/aggregation-entry-undeclared-key-door.test.ts,预期 5 个用例全绿(3 个拒绝、2 个对照)。
  • 需要你做的: 无,席位落地。

What changes

  • AggregationNodeSchema is closed at the producer (packages/spec/src/data/query.zod.ts). It is now a strictObject with the unknown-key campaign's message (Unrecognized key(s) on this aggregation entry: KEY.), near-miss hints where / filters / conditions → filter and as → alias, and a history sentence. The retired distinct keeps its retiredKey() tombstone and answers that sentence, never the generic refusal (strictObject keeps tombstones out of suggestions). Every door that parses the node inherits it: POST /api/v1/data/:object/query on RestServer and on the runtime dispatcher (both parse FindDataRequestSchema), and EngineAggregateOptionsSchema.aggregations.
  • MCP folds into it (packages/mcp/src/mcp-http-tools.ts). aggregate_records took each entry as its own strictToolInput copy of the node's four members (MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550), a second closed key list. It now takes AggregationNodeSchema whole, so the refusal is the spec node's own sentence, the same one REST prints, and distinct answers its tombstone on both doors (MCP used to give it the generic unknown-key refusal). filter still reaches the bridge unchanged. tools/list now lists distinct as a schema nothing satisfies (not: {}) with its [REMOVED] text, and the member descriptions are the spec's.
  • Strictness ledger: data/ strip count 64 → 63 (regenerated), and the hand prose rows for query.zod.ts re-read and updated to say the node is strict.
  • Changeset .changeset/28609-aggregation-entry-strict.md: @objectstack/spec and @objectstack/mcp patch, fix(spec)!:, BREAKING banner, both declaration lines, ADR-0087 not-required (no-migration-prescription) (a query aggregation entry is a request surface, never stored metadata). It carries one sentence superseding the pending MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550 changeset's "the REST query door is untouched".

Tests

  • packages/rest/src/aggregation-entry-undeclared-key-door.test.ts (new): the real door (RestServer → findData → ObjectQL.aggregate → sqlite SqlDriver). bogusKey, entry where, entry as: 400 VALIDATION_FAILED, fields[] at query.aggregations.1 carrying exactly AggregationNodeSchema.safeParse(entry)'s message, driver never read. Controls: distinct keeps its tombstone; declared-only entries (count, sum, filtered count) answer the same grouped numbers.
  • packages/runtime/src/domains/data-write-request-gate.test.ts: one row in QUERY_REFUSED, so both doors (RestServer and the dispatcher catch-all) refuse it and the cross-door parity block asserts they answer alike.
  • packages/rest/src/request-schema-gate.conformance.test.ts: one more violating body on the /query route.
  • packages/mcp/src/mcp-http-tools.aggregation-entry.test.ts: refusals now contain the spec node's message; distinct gives the tombstone sentence and no "Unrecognized key"; tools/list pin updated (distinct present, not: {}).
  • packages/spec/src/data/query.test.ts: new block for the closed node (slip, four hints, tombstone not suggested, closure through QuerySchema and EngineAggregateOptionsSchema, declared-only control); two stale comments that said the node is non-strict corrected.

Acceptance notes

  • Premise held. Measured on base 0b6a52e8 through the REST door: all three entry slips answered 200 with the grouped numbers.
  • Metadata-protocol measured none, not the order's expected narrowed. AGENTS.md defines the line as the change to what a metadata author can write. check:authorable-surface stayed green (the node's key list is unchanged; strictness is not on that surface), the spec's own docblock calls QueryAST a request surface never stored in metadata, and the claim recorded none as expected. So the changeset is patch with the BREAKING banner, the shape AGENTS.md prescribes for a non-protocol breaking change.
  • Analytics re-judged: service-analytics builds its aggregations in the method dialect for IDataEngine and never imports or parses AggregationNodeSchema, so nothing there moves. No open PR touched query.zod.ts or the MCP data tools at branch time or before this PR.
  • dropped-refinements.baseline.json did not move (the order's census hypothesis): strictness is not a refinement; data/AggregationNode still records only filter.lazy.
  • Fixture sweep: every aggregation-entry literal in the tree (code, tests, skills, docs) was scanned for keys outside the five declared; the only hits are deliberate (func pins in driver tests that call drivers directly, fn in a plugin-security AST test, the analytics method dialect), none of which goes through this schema.

Verification

All readings at head 09f089de unless named (the merge of origin/main plus the ledger regeneration; nothing of this card moved after the suites below).

Red first, on base 0b6a52e

pnpm --filter @objectstack/rest exec vitest run src/aggregation-entry-undeclared-key-door.test.ts with the spec unchanged: 3 failed / 2 passed. bogusKey, entry where and entry as each answered 200 (expected undefined to be 400, the handler wrote the grouped rows with no status); the distinct tombstone and the declared-only control passed. With the fix: 5/5.

Ablation (one-shot, not a kept test)

node scripts/ablation-replace.mjs turned the node back into a plain z.object(shape) (anchor hit 1/1), pnpm --filter @objectstack/spec build, ablation-dist-preflight spec ablationShape found the marker in 2 built files. Reds: REST door 3 + REST gate table 1, runtime two-door rows 2, MCP entry file 3, spec block 4 (13 in all); every control stayed green. Restore leg: blob equal to HEAD, git diff HEAD empty, rebuilt, --absent preflight clean over 588 built files.

Package suites and dogfood
  • @objectstack/spec: 869 files, 24570 passed (before the last spec-test commit; that file then 96/96 on its own).
  • @objectstack/rest: 364 files, 6915 passed, 332 skipped.
  • @objectstack/runtime (after building @objectstack/runtime...): 504 files, 8189 passed, 19 skipped.
  • @objectstack/mcp: 48 files, 612 passed.
  • Full dogfood (owed by the (narrowing) arm), pnpm --filter @objectstack/dogfood test under the lock after building @objectstack/dogfood^...: VERDICT command-exit 0, 369 passed / 3 skipped files, 2981 passed / 28 skipped tests.
Derived gates

node scripts/pm/dispatch-gates.mjs --ran on the final head: 85 derived families accounted for, all with exit codes, 0 NOT-MEASURED, every exit 0. Two needed a second run: check:doc-authoring first red on an issue id in the node's customer-facing history sentence (removed, then green), and check-plugin-teardown-shape --self-test exit 3 until its fixture commit was fetched (then green). Also green on the final head: check:strictness-ledger, check:authorable-surface, check:api-surface (public API surface unchanged), check:docs, check:type-check-debt, check:dual-build-cjs-loads, check-adr-0087-registration --base origin/main. Repo-wide lint and the test:repo project are CI's.

Human Minimum

  • 1. Schemas start as Zod; packages/spec holds no runtime logic.
  • 2. Every name I added uses the casing for its kind.
  • 3. No formatter ran; pnpm lint passes (CI).
  • 4. One issue, one branch off an up-to-date main, one pull request.
  • 5. A change that publishes carries a changeset, and both declaration lines are filled in here and in the changeset.
  • 6. packages/spec touched: its generated artifacts are regenerated and committed.
  • 7. No hand edits to content/docs/references/, packages/*/CHANGELOG.md or content/docs/releases/.
  • 8. Title, body and comments are in English (the maintainer summary block is the lane's required Chinese section).

🤖 Generated with Claude Code

https://claude.ai/code/session_01FEkQ5m158G1nvmxNYr7Jwq


Generated by Claude Code

… every door

AggregationNodeSchema becomes a strictObject (the unknown-key campaign's
message, hints where/filters/conditions -> filter and as -> alias); the
retired distinct keeps its tombstone sentence. REST POST /data/:object/query
answered 200 with the key stripped; it now answers 400 VALIDATION_FAILED
naming the key, before any read.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5m158G1nvmxNYr7Jwq
…node whole

The tool's own closed copy of the entry folds into AggregationNodeSchema,
which now refuses undeclared keys at the producer: one refusal, the REST
door's words, and the retired distinct answers its tombstone on both doors.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5m158G1nvmxNYr7Jwq
…d aggregation-entry key

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5m158G1nvmxNYr7Jwq
…facing history sentence

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FEkQ5m158G1nvmxNYr7Jwq
@github-actions

github-actions Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/mcp, @objectstack/spec, touching 5 documentable anchor(s).

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

  • content/docs/api/client-sdk.mdx (via data.query (sdk, the route ledger binds it to POST /api/v1/data/:object/query, selected by route anchor /data/:object/query; the route ledger binds it to POST /data/:object/query))
  • content/docs/api/data-api.mdx (via /data/:object/query (route, a path literal in a comment on a changed line))
  • content/docs/api/wire-format.mdx (via /data/:object/query (route, a path literal in a comment on a changed line))
  • content/docs/data-modeling/queries.mdx (via /data/:object/query (route, a path literal in a comment on a changed line))
  • content/docs/kernel/runtime-services/data-service.mdx (via data.query (sdk, the route ledger binds it to POST /api/v1/data/:object/query, selected by route anchor /data/:object/query; the route ledger binds it to POST /data/:object/query), /data/:object/query (route, a path literal in a comment on a changed line))
  • content/docs/protocol/objectql/query-syntax.mdx (via /data/:object/query (route, a path literal in a comment on a changed line))

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

  • content/docs/releases/v17/17-5.mdx (via /data/:object/query (route, a path literal in a comment on a changed line))
  • content/docs/releases/v17/17-6.mdx (via /data/:object/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
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 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; 100 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 — 173 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 da6f0ce35da60bf81e79890b227815a989f9b165 → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json da6f0ce35da60bf81e79890b227815a989f9b165

⚠️ 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 da6f0ce35da60bf81e79890b227815a989f9b165 → 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

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 09f089de54fbb0fd6adaacc09b021d3ad47509c9
Local-runs: none
Scope: packages/spec/src/data/query.zod.ts, packages/mcp/src/mcp-http-tools.ts, packages/mcp/src/mcp-http-tools.aggregation-entry.test.ts, packages/rest/src/aggregation-entry-undeclared-key-door.test.ts, packages/rest/src/request-schema-gate.conformance.test.ts, packages/runtime/src/domains/data-write-request-gate.test.ts, packages/spec/src/data/query.test.ts, docs/audits/2026-07-unknown-key-strictness-ledger.md, docs/audits/2026-07-unknown-key-strictness-ledger.counts/data.md + .changeset/28609-aggregation-entry-strict.md, packages/spec/src/shared/strict-object.ts (strictObject, strictObjectError, acceptsNothing), packages/spec/src/shared/suggestions.zod.ts (strictUnknownKeyError), packages/spec/src/api/protocol.zod.ts (FindDataRequestSchema), packages/spec/src/data/data-engine.zod.ts (EngineAggregateOptionsSchema, QueryWithTransportSchema, expand), packages/spec/src/api/contract.zod.ts, packages/spec/src/data/driver.zod.ts, packages/spec/src/api/odata.zod.ts, packages/rest/src/crud-routes.ts (the /query gate), packages/runtime/src/domains/data.ts (the dispatcher's twin), packages/services/service-analytics/src/plugin.ts (the method to function bridge), ui/packages/core/src/utils/object-aggregate-query.ts, packages/spec/authorable-surface/data.json, packages/spec/authorable-surface.base.json, packages/spec/dropped-refinements.baseline.json, packages/spec/json-schema.manifest/data.json, packages/spec/api-surface/data.json, packages/spec/scripts/build-schemas.ts (reachabilityRootTypes, reachableVia), scripts/check-changeset-no-major.mjs, scripts/pm/clause2-line.mjs, AGENTS.md Post-Task step 3

Face: schema (record-recognisers --paths on the PR's ten changed paths returns hits = packages/spec/src/data/query.zod.ts, a published schema under packages/spec/src/**) and narrowing (the PR body and the changeset both carry Clause-②: no (narrowing) line-initial, so the floor is every path of the diff outside the prose faces; the changeset is the one prose-face path and is read for ② only). Inputs: the diff 49a6a5c3...09f089de (eight commits, including the merge of origin/main at 5d07aaa and the ledger regeneration at 09f089d), card #28609 with triage 6557790954 and claim 6557859202, the seat's order, the dev report 6559293491, the PR body, PR #28600 (merged 20dcc07) and its review 6555135229, the dev's evidence directory (red-first, green, ablation logs) read as data, and the head's check-runs from the REST API at 2026-11-01T14:39Z. PR author is objectstack-fleet[bot], so no ③ author note is owed.

① Derived judgments

  1. The node is closed in the campaign's own wording, and the tombstone survives. AggregationNodeSchema is now lazySchema(() => strictObject({ surface: 'this aggregation entry', history, aliases: { where, filters, conditions → filter; as → alias } }, shape)). strictObject (strict-object.ts:541) is closedObject(z.object(shape, { error: strictObjectError(options, shape) }).strict()), and strictObjectError builds strictUnknownKeyError with knownKeys = the shape's keys minus those for which acceptsNothing holds, so the retired distinct (retiredKey(), kept in the shape) stays off the rename candidate list while writing it still raises its own invalid_type tombstone. The spec test pins all three halves: the unrecognized_keys message Unrecognized key(s) on this aggregation entry: \bogusKey`., the four hints in the `key` → `canonical`spelling, anddistinctansweringinvalid_typeonly whiledistincis not told aboutdistinct. The history sentence was re-worded in 3f302825 to drop the issue id (check:doc-authoring`); it is prose inside the error map and moves no accept set. Judged correct.
  2. Every consumer of the node is accounted for. In packages/spec the node is held only by QuerySchema.aggregations and EngineAggregateOptionsSchema.aggregations (data-engine.zod.ts:483). QuerySchema is reached by FindDataRequestSchema through QueryWithTransportSchema (api/protocol.zod.ts:2477, data-engine.zod.ts:1312), by ODataQuerySchema (a GET projection, no entry literal arrives from a query string), by the expand / $expand members of data-engine.zod.ts, and as declared inputs of the contract registry (api/contract.zod.ts:421, 511) and the driver contract tuples (data/driver.zod.ts). The doors that parse it: POST /api/v1/data/:object/query on RestServer (crud-routes.ts:363 FindDataRequestSchema.safeParse), the runtime dispatcher's twin (domains/data.ts:247), and MCP aggregate_records, which now takes z.array(AggregationNodeSchema) whole. No other non-test parse / safeParse of QuerySchema, QueryWithTransportSchema, EngineAggregateOptionsSchema or AggregationNodeSchema exists in packages/, ui/ or apps/. The engine (objectql/src/engine.ts), the four drivers and metadata-protocol/src/protocol.ts name the schema in comments only; rejectUnknownEngineOptions closes the aggregate verb's top level and never recurses into entries. service-analytics hands engine.aggregate entries it builds itself ({ function, field, alias, filter? }, plugin.ts:378–420, the method dialect re-parsed through AggregationFunction), so nothing there moves. ui/packages/core sends { function, field, alias }; the aggregate: { field, function, groupBy } literals in objectui and hotcrm belong to the widget schemas (object-metric / object-chart), not this node. The skills and docs aggregation examples carry only declared keys (swept skills/objectstack-query/**, skills/objectstack-api/**, content/docs/api/data-api.mdx, protocol/objectql/query-syntax.mdx, data-modeling/queries.mdx). Judged complete.
  3. One refusal, word for word, and no second spelling in MCP. The REST door test pins fields[].message with toBe(AggregationNodeSchema.safeParse(entry).error.issues[0].message) for bogusKey, entry where and entry as; the MCP test pins toContain of the same helper's output for bogusKey and where (the SDK wraps it as Invalid arguments for tool aggregate_records: … at aggregations[N], the same sentence inside). aggregationEntrySchema() and its strictToolInput({ surface: 'this aggregate_records aggregation entry', … }) are deleted from mcp-http-tools.ts; a search of packages/mcp/src and the MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550 dogfood file on the head finds no remaining aggregationEntrySchema, this aggregate_records aggregation entry or other entry-level key list (the top-level strictToolInput that closes the tool's own keys is a different surface and is unchanged, pinned by the metrics → aggregations control). distinct now answers its tombstone on MCP too (not.toContain('Unrecognized key')), and tools/list advertises it as not: {} with the [REMOVED] text; the fix(mcp)!: an aggregate_records aggregation entry carries its filter to the engine and refuses an undeclared key by name #28600 review's ③ note that the two doors prescribed distinct differently is closed by this. Judged correct.
  4. Location and ordering on REST. query.aggregations.N sits beside the siblings the runtime two-door table already pins (query.limit, query.joins, query) and beside the tombstone's own query.aggregations.1.distinct; the new runtime row asserts both doors answer alike at query.aggregations.0. The REST door test spies the sqlite driver's find and aggregate and pins { aggregate: 0, find: 0 } on every refusal, and the runtime row says findData never asked. The refusal is the gate's, before any read. Judged correct.
  5. Controls. Declared-only entries (count, sum, filtered count) answer the same grouped numbers through the real door (c1: { n: 2, s: 500, big: 1 }, c2: { n: 3, s: 1250, big: 2 }); the spec control parses three declared-only entries unchanged; MCP's forwarding controls (entries without a filter reach the bridge byte-identical, top-level posture unchanged) are untouched from MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550. dropped-refinements.baseline.json is unchanged and correctly so: strictness is not a refinement, and data/AggregationNode still records filter.lazy only.

② Semver level

  • Clause-②: no (narrowing) is the right arm. No key, value or export is added; an entry key the REST and dispatcher doors stripped is refused by name. Both carriers (PR body and changeset) carry it line-initial.
  • Metadata-protocol: none is honest, and the order's expected narrowed was a hypothesis the dev was right to measure against. The line is defined (AGENTS.md Post-Task 3, clause2-line.mjs:502) as the change to what a metadata author can write, with none covering every non-protocol change, REST included. Three readings agree the node is not on an author's surface: (a) no registered metadata-type schema under packages/spec/src/ui/** imports QuerySchema or AggregationNodeSchema (dataset, dashboard and component import AggregationFunction / DateGranularity only, and carry their own closed aggregation shapes: GridAggregationSchema { field, type }, the dataset measures, the widget aggregate block), so the node is unreachable from the metadata-type roots that build-schemas.ts's reachableVia BFS starts from, by construction of the import graph; (b) no non-test code re-parses a stored item through QuerySchema or EngineAggregateOptionsSchema (①.2), so a stored report, dataset, dashboard or view cannot carry an entry that this refusal reads on save or on read; (c) the spec's own docblock and the MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550 distinct retirement entry already dispose of QueryAST as a request surface under ADR-0087. The one precedent the other way, engine(filter): NowAnchorSchema is declared closed, but a direct find() where tolerates an extra key on a $now anchor while aggregate() and metadata filters refuse it #26413 (narrowed for the $now anchor), narrowed FilterConditionSchema, which a view's saved filter does carry; this node has no such carrier. Two cautions for the record, neither a must-fix: check:authorable-surface being green is not the evidence for none, because the live authorable-surface/data.json enumerates every emitted def's keys (it lists data/Query:*, api/FindDataRequest:* and data/EngineAggregateOptions:* alongside data/AggregationNode:*) and the gate's own header says a refused value is invisible to it; and this review decided reachability by reading the import graph and the parse sites, not by running the BFS (Local-runs: none). Under scripts/check-changeset-no-major.mjs a declared none must grade every introduced entry patch and move no authorable-surface or authorable-defaults shard: both hold, and Check Changeset is green on the head.
  • Levels and banners are consistent. @objectstack/spec and @objectstack/mcp at patch, the fix(spec)!: bang in the summary and in the PR title, the **BREAKING** accept-set narrowing banner, Before / Now / Unchanged / Who sees it / The remedy, and exactly one ADR-0087 marker not-required (no-migration-prescription). This is the shape AGENTS.md prescribes for a non-protocol breaking change (patch, banner and disposition kept), the same shape fix(mcp)!: an aggregate_records aggregation entry carries its filter to the engine and refuses an undeclared key by name #28600 shipped and its review accepted, and action door: forwarding a type:api action drops the target's error.details and userMessage, and the ADR-0104 param check refuses approval decision outputs (outputs.KEY), so console step 3 (#27611) would regress #27738 states the rule in its own body. The not-required category is right: no key, export or type is added, removed or renamed, and a request body is not a stored row for a conversion to rewrite. The changeset's one supersession sentence over the pending MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550 entry ("the REST query door is untouched") is correct and wanted.
  • Changeset wording, not a must-fix: "Who sees it" names "a REST, dispatcher or in-process caller". No in-process door parses the node (①.2: engine.aggregate is closed at the top level only, and service-analytics sends declared keys), so the in-process clause describes a caller who parses QuerySchema or EngineAggregateOptionsSchema itself. The REST and dispatcher halves are exact. The seat may ask for the clause to be tightened; the review does not turn on it.

③ Boundary flags

  • Dev open_questions: none. One out_of_scope_findings row: open PR feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688 also edits docs/audits/2026-07-unknown-key-strictness-ledger.md. Read on the head of feat(spec)!: an authored connector entry refuses a top-level key it does not declare; the connector read withholds URL credential parts and a non-object auth #28688 (91b48bc): it rewrites the integration/ row (around line 1375) and the integration.md counts shard (3 → 4 sites); this PR rewrites the query.zod.ts rows (lines 719, 1260, 1287) and the data.md shard (strip 64 → 63). The hunks do not overlap, the counts shards are per directory and check:strictness-ledger sums them at read time, so whichever lands second merges clean and the gate stays green; the landing seat re-runs check:strictness-ledger after the second landing as the dev proposed. Not a must-fix.
  • The dev's departure from the order's narrowed. Declared in the PR body's Acceptance notes and in the dev report with its reasons; judged honest in ②. The claim had already recorded none as expected, so the carriers, the claim and the measurement agree; the order's hypothesis was the one that did not survive measurement.
  • Generated surfaces. The strictness ledger counts (data.md) are regenerated on the merged tree (09f089d, a regen-only commit) and the hand rows are re-read; dropped-refinements.baseline.json correctly does not move; json-schema/ is gitignored (built, not committed) and the committed json-schema.manifest/data.json is a def-name ratchet that this change does not touch; api-surface/data.json and the signatures file carry the same symbol set. The gates that read them (check:authorable-surface, check:strictness-ledger, check:api-surface, check:docs, check-regen-pending) run in lint.yml, and every lint.yml job is green on the head at 2026-11-01T14:39Z (Lint · repo & tooling gates, Lint · CI, release & engine gates, Lint · docs & code guards, Lint · ESLint, the three scoped jobs, Lint · PM dispatch-gates), as is Type Check · source gates.
  • Ablation and red-first, read from the evidence directory as data. Red-first on base 0b6a52e through the real door: 3 failed / 2 passed, each failure expected undefined to be 400 (the handler wrote rows with no status); green 5/5 at head. Ablation turned the node back into z.object(ablationShape) (anchor 1 → 0, marker found in 2 built spec files), and reds fell exactly where the assertions predict: REST door 3 + REST gate table 1 (4 of 21), runtime two-door rows 2 (of 76), MCP 3 (bogusKey, where, tools/list; the distinct case stays green under a plain object because the tombstone still fires, which is the correct reading), spec block 4 (of 96), 13 in all, every control green; restore blob equal to the then-HEAD 484652dd and git diff HEAD empty. The head's query.zod.ts blob is 524d8598 because 3f30282 later re-worded the history sentence; that commit moves prose inside the error map only, so the ablation reading carries to the head.
  • CI at this read (2026-11-01T14:39Z): no check red. Completed green: Check Changeset, Governed Surface Queue Guard, PR-body guards, Check PR Size, Dogfood Regression Gate, Spec property liveness, Flag docs affected by code changes, Check Documentation Links, Build UI Deps, Type Check · source gates, every lint.yml job. Still in progress: Build Core, the eight Test Core shards, the sixteen Test UI shards, Type Check UI, Type Check · workspace, Type Check · debt ledger, Type Check · consumer gates, Build & E2E UI, Live E2E UI. The landing pre-check (② of the review reference: every check green, not a required subset) is the seat's to re-read before the queue; this record does not stand in for it. The PR is a draft with no auto-merge armed.
  • Skills-docs: none holds (①.2 sweep). The docs drift bot flagged nothing new beyond fix(mcp)!: an aggregate_records aggregation entry carries its filter to the engine and refuses an undeclared key by name #28600's count_distinct literals.

④ Design judgment

  • packages/spec/src/data/query.zod.ts: against the published spec on main, the narrowing takes no declared capability away at any consumer: every key now refused was stripped before it reached the engine, so no computed answer depended on it, and the one key a caller plausibly meant (where as the entry's own filter) is named with its spelling instead of silently widening the aggregation to the whole group. It makes the spec's own refusal real on the doors that were lying by omission, and it makes the ledger's query.zod.ts row true where the MCP: aggregate_records silently drops an aggregation entry's filter, so a conditional count answers the unconditional total (REST honours it) #28550 close had left one entry shape with two verdicts. The close sits at the producer, so the MCP copy could be deleted rather than kept in sync (Prime Directive Add comprehensive test suite for Zod schema validation #12 direction); the aliases table carries over the exact four near-misses MCP had curated, and the history sentence is the campaign's one-sentence why. Holds against strictObject / strictUnknownKeyError as cited and against retiredKey()'s tombstone contract.
  • packages/mcp/src/mcp-http-tools.ts: z.array(AggregationNodeSchema).min(1) replaces the local copy; the handler still forwards aggregations whole to the bridge (unchanged lines), so filter reaches engine.aggregate as fix(mcp)!: an aggregate_records aggregation entry carries its filter to the engine and refuses an undeclared key by name #28600 established and nothing in packages/mcp evaluates it. The only visible change for an MCP client is the refusal's sentence (now the spec's) and distinct answering its prescription; tools/list declares both. Holds.
  • The five test files judge the scenario the card accepts against: the real REST door down to a sqlite driver with read spies (REST), the catch-all dispatcher's parity row (runtime), the catalog gate table (REST conformance), the tool over a recording bridge (MCP), and the node itself through QuerySchema and EngineAggregateOptionsSchema (spec). The two door tests share one nodeRefusal helper, which is what makes "word for word" a pinned fact rather than a sentence in the body. Holds.
  • docs/audits/2026-07-unknown-key-strictness-ledger.md and .counts/data.md: the generated numbers follow the site count (one strip left query.zod.ts, 4 → 3, data/ 64 → 63, wire/open bucket 63 → 62), and the hand rows state the new verdict without rewriting 一个被静默丢弃的排序键对外部调用方仍然是静默的:direction 该 400 还是继续被丢掉(#4674 第 4 项) #4721's history. Holds.

No must-fix found.

Implemented-by: claude/project-thread-i0hitl
Reviewed-by: session_0186vfjHHMsRXC9EUT9hTFMo

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

domain:server#2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo

席位核对

ACCEPT · domain:server seat 2 · os-project-manager · session_0186vfjHHMsRXC9EUT9hTFMo · 2026-11-01T14:40Z · head 09f089de · #28609

The maintainer's standing authorisation for landing reviewed PRs, verbatim: 「按照 pm-dispatch 规则复核过的都授权」.

  1. Shape:
    • A draft against main with Fixes #28609.
    • It carries patch for @objectstack/spec and the MCP package, with the BREAKING banner, a fix(spec)! summary and the ADR-0087 not-required marker.
    • check-governed-merges --pr 28706 reads NOT governed, +325 / -75. Landing tier S.
    • The claim (6557859202) names this PR's branch, claude/project-thread-i0hitl.
  2. Against the order:
    • AggregationNodeSchema refuses an undeclared key by name in the unknown-key campaign's wording, with hints, and distinct keeps its tombstone.
    • The REST query door and the dispatcher answer 400 VALIDATION_FAILED at query.aggregations.N before any read. MCP aggregate_records takes the spec node whole, with its local closed copy removed, so there is one sentence on both doors.
    • The red-first run on base answered 200 for 3 slips. Controls are green. The ablation reds 13 cases across spec, REST, runtime and MCP.
    • The departure from the order's expected narrowed: the dev measured Metadata-protocol: none. The review confirms the node is unreachable from any registered metadata-type root, so no stored item changes meaning. none is honest.
  3. Contract review: PASS on this head, 6559419790 (CONTRACT_REVIEW_TIER, schema and narrowing faces).
  4. Gates:
    • The dev reports 85 of 85 derived gates exit 0 and a green full dogfood battery.
    • CI on this head was in progress with 0 red at review, and auto-merge waits for it.
  5. Overlap:
  6. Landing: pr_ready + automerge_enable through the relay.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review November 1, 2026 14:41
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge November 1, 2026 14:41
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Nov 1, 2026
Merged via the queue into main with commit 36fa4ff Nov 1, 2026
66 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/project-thread-i0hitl branch November 1, 2026 15:13
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 protocol:data size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

spec/rest: the query door's aggregation entry still strips an undeclared key, while MCP's aggregate_records now refuses it by name (#28550)

1 participant