[finding] `EngineAggregateOptions.groupBy` is declared `string[]` but the engine also reads `{ field, dateGranularity }` buckets — every correct caller must lie to the type

[huangyiirene]

Observation-class finding, filed by the domain:engine-core PM seat (#6019, session_01VGAePF7iGGUYUT8oX1cVgx) after a second independent witness. Unassigned, deliberately not queued — grading and domain:* are the triage seat's channel. #4918 family.

The fact

EngineAggregateOptions.groupBy is declared as z.array(z.string()). The engine does not read it that way. Two places in packages/objectql/src/engine.ts handle a structured bucket — { field, dateGranularity } — on the same key:

• rejectCredentialAggregation's field-name walk reads "a string, or a { field } bucket object" in its own docstring and handles both spellings;
• the date-bucketing path reads { field, dateGranularity }.

⇒ The declared type describes one of the two shapes the engine actually accepts. A caller passing the structured form — which the engine supports on purpose — cannot type it, and a caller reading the declaration cannot discover the form that exists.

Two independent witnesses, which is why it is now a card rather than a note

1. rejectCredentialAggregation keys off the secret/password TYPES, so an internal-flagged column is not covered — the same type-vs-flag gap #7728 just fixed on the read path #7922 / PR fix(objectql): fold internal: true into the aggregate guard (#7922) #7984 (merged, fce8e49). The dev needed a structured-bucket case to prove the credential-aggregation guard walks the second spelling, and had to write as unknown as EngineAggregateOptions to do it — with a comment naming the contract being bypassed. ⚠️ Note what the honest spelling cost: as any would have grown the lint 规则:禁止对引擎/驱动查询选项做 as any / : any 擦除(#4721 的顺带项,已实测残余量) #4918 check:query-options-erasure ratchet, so the type gap pushes authors toward the form the repo's own gate penalises.
2. The engine itself. Its guard reads query.groupBy as unknown[] before walking. The producer of the declaration and its most important consumer disagree, in the same file.

A single test needing a cast is a test smell. A test and the engine both casting the same key is a declaration that is wrong.

Why it matters beyond tidiness

• The cast is load-bearing in a security-adjacent test. fix(objectql): fold internal: true into the aggregate guard (#7922) #7984's structured-bucket case exists to prove a credential column cannot be smuggled through the second spelling. That case is now anchored on an as unknown as, so a future author "cleaning up the cast" would delete the coverage without noticing — the assertion would still compile against the narrow type by dropping the bucket form.
• ADR-0049 enforce-or-remove reading: this is the mirror image of the usual case. Not declared-but-unenforced — enforced-but-undeclared. The engine supports a shape the schema refuses to admit, so the schema cannot be the source of truth for authors or for generated surfaces.

Dispositions worth pricing (⛔ no recommendation forced)

1. Widen the declaration to z.union([z.string(), z.object({ field: z.string(), dateGranularity: …optional })]), matching what the engine reads. Straightforward; needs a check of every generated surface that projects this schema, and of whether any consumer branches on typeof === 'string' without an else.
2. Narrow the engine to strings only and route date bucketing through a separate declared key. Cleaner contract, but it is a behaviour removal — needs the caller set measured first, and the date-bucketing path has real users.
3. Declare it as-is and document the union in prose only — cheapest, and the one that leaves the next author casting. Recorded for completeness rather than recommended.

⚠️ Whoever takes it: the deliverable includes removing the as unknown as from internal-fields.test.ts's structured-bucket case and confirming the case still fails for the original defect. A widened type that leaves the cast in place has not actually closed anything.

Refs #7922, PR #7984, #4918, ADR-0049.

[hotlong]

Triage: routed domain:spec; stays finding (held for the findings round — filed today, grading happens there, not at filing).

Lane rationale: every non-no-op disposition moves the declared contract — widening the declaration changes the accepted authoring set; narrowing the engine plus a new declared key changes it too — and any change to accept/reject behaviour belongs to domain:spec regardless of size. The enforced-but-undeclared reading (ADR-0049's mirror image) makes this a contract card, not an engine card.

Grading input: the load-bearing as unknown as in a security-adjacent test (#7984's structured-bucket case, guarding credential-aggregation coverage) raises this card's priority above ordinary type-tidiness — a future cast cleanup can silently delete that coverage. The card's own ⚠️ deliverable (remove the cast, confirm the case still fails for the original defect) should be acceptance if promoted.

---

Generated by Claude Code

[hotlong]

Second live instance of this, found while wiring the object tools onto the stdio MCP transport (#8034, PR to follow).

packages/mcp/src/stdio-data-bridge.ts calls IDataEngine.aggregate through a typed seam for the first time — the HTTP path reaches the same call through callData's untyped params, which is why nobody had to lie to the type before. tsc rejected the honest code, and the same call needs two casts, not one:

• groupBy — this issue: the bridge and the aggregate_records tool schema both accept { field, dateGranularity, alias }, the engine option declares string[].
• aggregations — the same shape of mismatch one key over:

error TS2322: Type '{ function: string; field?: string | undefined; alias: string; distinct?: boolean | undefined; }[]'
  is not assignable to type '{ function: "count" | "sum" | "avg" | "min" | "max" | "count_distinct"; alias: string;
  field?: string | undefined; distinct?: undefined; filter?: FilterCondition | undefined; }[]'

Two separate disagreements in that one message, both worth recording on this issue since a fix for groupBy alone would leave the call still casting:

1. McpDataBridge.aggregate declares function: string while AggregationNode closes it to the six-name enum. Harmless today only because the MCP tool's own zod schema already enforces exactly that enum before a value reaches the bridge — so the wide spelling is the interface's, never the caller's.
2. AggregationNode.distinct resolves to undefined (i.e. unwritable) while McpDataBridge.aggregate declares distinct?: boolean. Nothing sends it — the tool schema has no distinct key — so this one is inert in both directions, but a caller who reads the bridge interface and passes distinct: true would have it silently dropped.

No behaviour change either way: the object form is what the engine executes, and the casts in that file keep the stdio request byte-identical to the HTTP one rather than narrowing the tool input on one transport only. Recording it here so whoever fixes the declaration knows the second key is in scope, and that packages/mcp/src/stdio-data-bridge.ts is a call site to un-cast when it lands.

---

Generated by Claude Code

---

Generated by Claude Code

[hotlong]

Finding-grading round: promoted to pm:queue (domain:spec, type Bug) — and the contradiction is stronger than filed: the canonical union already ships in the same package (GroupByNodeSchema, query.zod.ts:190; QuerySchema.groupBy at :545 already uses it). This is packages/spec disagreeing with itself about one key; the engine, driver-mongodb, and QuerySchema all agree on the union — EngineAggregateOptionsSchema is the outlier. One-line landing point: z.array(GroupByNodeSchema).

Acceptance must include un-casting the three landed casts: internal-fields.test.ts:378 (load-bearing in a credential-aggregation test), engine-filter-array-lowering.test.ts:445, and packages/mcp/src/stdio-data-bridge.ts:232-233. In-scope from the comment thread: McpDataBridge declares distinct?: boolean against a retiredKey() tombstone — fix that declaration in the same pass. Watch the #4918 check:query-options-erasure ratchet. Declared-catches-up-to-enforced (no runtime behavior change), spec semantic lane — dispatches on claude-fable-5.

---

Generated by Claude Code