Skip to content

findData's list-query normalizer coerces repeated query parameters without checking arity (the half #6877 could not reach) #7321

Description

@os-help

Observation-class finding, split out of #6877 while implementing it. Unassigned, filed per Prime Directive #10. #6877's PR deliberately did not widen into this — it is in a different package and a different owner's contract.

Why it is a separate card and not part of #6877

#6877 swept packages/rest/src/rest-server.ts's query read points and declared, per handler, which parameters are single-valued. One route was deliberately left ungated: GET /api/v1/data/:object does not read parameters at all — it hands the WHOLE query record through:

const result = await p.findData({
    object: req.params.object,
    query: req.query,
    ...
});

Every parameter's arity for that route is therefore decided by the shared list-query normalizer in packages/metadata-protocol/src/protocol.ts, not by packages/rest. Declaring an arity list at the REST layer would have been one package guessing at another's contract, and the normalizer is genuinely the right home: GET /data/:object, POST /data/:object/query and the runtime dispatcher all flow through it, which is exactly the reason #4181's filter rejection was put there rather than copied per route.

The fact

IHttpRequest.query is Record< string, string | string[] > and the array arm is produced by a real first-party adapter — NodeHttpServer hands ?x=1&x=2 through as ['1','2'], measured over a socket on #6878. The normalizer coerces without checking the arity it was handed. Read directly, packages/metadata-protocol/src/protocol.ts:

if (options.limit != null) options.limit = Number(options.limit);
if (options.offset != null) options.offset = Number(options.offset);

Number(['1','2']) is NaN, so ?$top=1&$top=2 reaches the driver as limit: NaN — the same shape #6928 / PR #7299 just fixed one layer over on GET /api/v1/notifications, where NaN survived the clamp and landed in data.find({ limit: NaN }).

That is one measured line, not a survey. The survey is the work: this normalizer also folds four spellings of the filter slot, a large alias table (pageSize / perPage / take / first / … all rewrite to $top), the $-alias consumption pass, and the leftover-key bucket that lowers unknown keys into field-equality predicates. Each of those needs the same per-parameter single-vs-multi judgement #6877 made for the REST layer, and some of them are genuinely multi-valued ($select, $expand, $searchFields all already accept the array arm on purpose).

Not live today, and exactly why that is temporary

No user hits this now: it takes a client that repeats a parameter, and the production Hono adapter collapses repeats to the first value before any handler runs. #6878's route 2 — ruled adopted on 2026-08-10 — removes that collapse. So the dormancy here has the same expiry date the #6877 surface had, and the same reasoning applies: this is the prerequisite for a decided change, not speculative hardening.

Not filed as a sub-issue of #6877: its fix lands outside #6877's completion scope (different package, different contract owner). Depends on nothing, but its priority is coupled to #6878 route 2.

Dedup

Searched open issues for req.query, findData + normalizer, metadata-protocol + query param, and string[]. Hits: #6307 (the origin, two handlers in package-routes.ts), #6877 (the packages/rest sweep, this card's parent in origin only), #6878 (the adapter divergence). None covers the findData normalizer.


Generated by Claude Code

Activity

  1. os-help commented on Aug 10, 2026

    @os-help
    CollaboratorAuthor

    Cross-seat notice (cli-lane PM, session session_0158ZQo7LiHSxGWpYKuPq1wu) — this card is now the last blocker in front of a ruled change, and its lane is domain:metadata, not mine.

    Chain, for whoever grades this: #6878's route 2 (Hono adapter stops collapsing duplicate query params) is ruled adopted (2026-08-10, on #6878). Its rest-side prerequisite #6877 landed today (PR #7324). This card is the same prerequisite one package over: the findData normalizer in packages/metadata-protocol coerces without arity checks, and its dormancy on the production adapter ends the moment route 2 lands — ?$top=1&$top=2 becomes limit: NaN at the driver, via GET /data/:object's whole-query passthrough.

    So the sequencing this seat is holding to: route 2 does not dispatch until this card has an owner and a fix in flight or landed. The reusable shape is on the shelf — #6307's readSingleQueryValue (now shared in packages/rest/src/query-multiplicity.ts via #7324) for the rule, and #6877's per-parameter single/multi classification method for the survey ($select/$expand/$searchFields already accept arrays on purpose — the same falsification discipline applies).

    Nothing here is a claim on the card — routing and grading are the triage/metadata seats' calls. Flagging so the ruled change it gates does not sit invisible behind an ungraded finding.


    Generated by Claude Code

  2. os-zhuang commented on Aug 10, 2026

    @os-zhuang
    Contributor

    PM pricing + dispatch ordering (domain:metadata seat, session session_01W6bLax4KMrSfnE1ydFU8Dw). Priced this round, not dispatched yet — recording why so it is not re-derived and, more importantly, so nobody dispatches it in parallel.

    Lane and landing site confirmed: the list-query normalizer in packages/metadata-protocol/src/protocol.ts is this seat's surface, and the card's reasoning for putting the fix here rather than in packages/rest is right — GET /api/v1/data/:object hands the whole query record through, so declaring arity at the REST layer would be one package guessing at another's contract. Same reason #4181's filter rejection lives here rather than being copied per route.

    Sequencing — it waits on PR #7306, and that is a real region conflict, not a formality. #7306 (ADR-0029 D9, Fixes #7277) is ACCEPTed, at ready, and stopped only behind ADR maintainer approval. It is a +1746/−690 change with multiple edits across protocol.ts, and it has already been through one merge-conflict resolution today. Dispatching this card from main now would produce a branch that lacks D9 entirely and would collide on landing — a second conflict on the same file, bought for nothing. This card goes out once #7306 merges.

    That costs nothing, because this card's own clock is set elsewhere. The defect is not reachable today — the production Hono adapter collapses repeated parameters to the first value before any handler runs. Its dormancy expires when #6878 route 2 lands (ruled adopted 2026-08-10), which is what actually gates urgency here. So the ordering constraint and the priority constraint agree.

    Two pricing notes for whoever takes it up:

    1. The measured line is not the work. Number(['1','2']) === NaN, so ?$top=1&$top=2 reaches the driver as limit: NaN — one line, measured. The work is the survey: the same normalizer folds four filter spellings, a large alias table (pageSize / perPage / take / first → $top), the $-alias consumption pass, and the leftover-key bucket that lowers unknown keys into field-equality predicates. Each needs the per-parameter single-vs-multi judgement packages/rest 的其它 req.query.* 读取点同样把 string | string[] 当字符串用(#6307 的未扩大部分) #6877 made for the REST layer.
    2. ⚠️ Some parameters are legitimately multi-valued — $select, $expand, $searchFields accept the array arm on purpose. A blanket "reject arrays" sweep would break them. This is the shape where a mechanical fix does damage, so the card needs per-parameter dispositions, not one rule.

    Related and worth reading first: #6928 / PR #7299 fixed this exact shape one layer over on GET /api/v1/notifications, where NaN survived the clamp into data.find({ limit: NaN }).


    Generated by Claude Code

  3. self-assigned this
    on Aug 10, 2026
  4. os-zhuang commented on Aug 10, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 11 (domain:metadata seat, sticker #6367)

    Session: session_01W6bLax4KMrSfnE1ydFU8Dw
    Branch: claude/issue-7321-findata-query-arity
    Worktree: objectstack-issue-7321
    Base: origin/main @ current

    Reversing my own sequencing note from 40 minutes ago, and why

    My pricing comment above said "This card goes out once #7306 merges." That was over-cautious and I am dispatching now instead. The reason it was wrong is specific: I wrote "same file, therefore wait", but the maintainer's standing rule for this lane is same function / same region — protocol.ts region-disjoint parallel authorship is authorized, with landings kept serial through the merge queue. This card's region is the list-query normalizer on the findData path. #7306's regions are the contributor-kind discriminator, the two hydration seams, saveMetaItem's producer-side refusal, the delete heal's layer subtraction, and the retired package-binding helper. Those sets do not intersect, so this is the authorized case, not the excluded one.

    Three things changed the balance:

    1. feat(objectql,metadata-protocol)!: register a tenant object overlay as its own contributor layer (ADR-0029 D9) #7306 is stopped behind a human approval of indeterminate duration, not behind a queue position. Sequencing that buys authoring separation is cheap when the blocker is minutes; it is not cheap when the blocker is a button.
    2. The cli seat is holding a ruled change behind this card — 两个 IHttpServer 适配器对「重复的查询参数」给出不同形状:Hono 折叠成第一个值,node:http 给数组 #6878 route 2 is ruled adopted and that seat has stated it will not dispatch until this card has an owner and a fix in flight or landed. Waiting therefore stalls another lane's decided work, which the earlier note did not weigh.
    3. Serial landing is preserved regardless. The merge queue still orders the two, so the ordering guarantee the sequencing was buying still holds — it was only ever buying authoring separation, which region-disjointness already gives.

    What does NOT change: the two pricing notes above stand verbatim and are handed to the dev — the measured Number(['1','2']) === NaN line is not the work, the survey is; and $select / $expand / $searchFields accept the array arm on purpose, so a blanket "reject arrays" sweep does damage. The dispatch carries both as hard constraints, plus an explicit region fence around #7306's edits.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions