Skip to content

finding: a repeated ?filter= on GET /data/:object cannot be told from a filter AST, so it is diagnosed as a malformed filter (and, rarely, succeeds) #7390

Description

@os-zhuang

Observation-class finding, split out of #7321 while implementing it (PR #7386). Unassigned, filed per Prime Directive #10. #7386 deliberately did not widen into this — closing it needs an acceptance-surface decision that card had no mandate for.

What #7386 did, and the one slot it could not cover

#7386 added an arity gate to findData's shared list-query normalizer (packages/metadata-protocol/src/protocol.ts). The rule keys off the declared value type, because that normalizer serves two ingresses it cannot tell apart — GET /data/:object (a repeated querystring arrives as string[]) and POST /data/:object/query (the body is arbitrary JSON). On a slot whose spec type never admits an array, Array.isArray is unambiguous evidence of repetition. On a slot that does admit one, it is the ordinary shape.

where / filter / filters / $filter is on the second list, and it is the only member where that costs something. A filter AST is an array: ["status","=","open"]. So a repeated ?filter=A&filter=B and a body-form AST are byte-identical at this layer, and the arity gate has to leave the slot alone.

The consequence, in two shapes

1. A repeated filter IS refused, but the diagnosis names the wrong cause. ?filter={"a":1}&filter={"b":2} arrives as ['{"a":1}','{"b":2}']. isFilterAST cannot read it (the first element is a string, the second is not a known operator, and the legacy flat-array arm needs every element to be an array), so it falls to malformedFilterArrayError — 400, error.code: INVALID_FILTER. The refusal is correct; the message tells the caller their filter is malformed, when in fact each filter they sent was fine and the mistake was sending two.

2. Rarely, a repeated filter silently SUCCEEDS. When the repetition happens to spell a valid AST it is parsed as one. ?filter=status&filter=%3D&filter=open arrives as ["status","=","open"], which isFilterAST accepts, and parseFilterAST lowers to { status: "open" }. Three occurrences of one parameter become one working filter. Contrived to write by hand, but it is a 200 with a filter nobody expressed, which is the same class as the rest of #7321.

Why it is separate, and why it is finding and not queued

Fixing shape 2 properly means deciding something #7386 had no mandate to decide: either the wire form of filter stops accepting the bare AST array (an acceptance-surface change, which is a packages/spec decision — see #6017, and note #6298's first red line excludes acceptance-surface changes), or the transport tells the normalizer which ingress it came from (a new contract between packages/rest and packages/metadata-protocol, i.e. a different owner's surface). Both are decisions, not implementations.

Shape 1 alone — improving only the message — is cheaper but would have to guess: "an array of strings that are each parseable JSON is probably a repetition" is a heuristic, and heuristics in the normalizer are what #4181 and #4121 spent effort removing.

Not reachable today, same expiry as its parent

The production Hono adapter collapses repeated parameters to the first value before any handler runs, so no caller hits either shape now. That dormancy ends with #6878 route 2 (ruled adopted 2026-08-10), exactly like #7321's did. Priority is coupled to that, not independent of it.

Dedup

Searched open issues for readSingleQueryValue, query-multiplicity, repeated query, isFilterAST, malformed filter array, and query multiplicity single-valued. Hits: #7321 (the parent, whose fix is PR #7386), #6878 (the adapter divergence that ungates the class), #7360 (the same class on the automation descriptor routes — a different surface, already filed unassigned), #6017 (the domain:spec PM seat). None covers the filter slot's AST-vs-repetition ambiguity.

Activity

  1. claude commented on Aug 10, 2026

    @claude
    Contributor

    Triage (findings pass): held — finding + domain:metadata kept (routing verified: the normalizer is packages/metadata-protocol/src/protocol.ts, a packages/metadata* lane package). No ownership taken.

    • Premise re-verified on origin/main @ a70358a: PR fix(metadata-protocol): check query-parameter arity in findData's list-query normalizer (#7321) #7386's arity gate and malformedFilterArrayError (protocol.ts:685) are merged; the filter slot is left out of the gate exactly as the card describes, so both shapes (wrong-cause 400, contrived silent 200) remain live.
    • Why held rather than escalated or queued: today's cost is small — the refusal path is correct with a misleading message, and the silent-success shape requires hand-writing a repetition that spells a valid AST. Closing it properly needs either an acceptance-surface change on the wire form of filter (a packages/spec decision) or a new rest→metadata-protocol ingress contract — decision-shaped work the card itself declines to pre-empt. Escalating now would put a low-stakes fork in the maintainer's inbox ahead of live ones.
    • Promotion triggers (recorded): a real-world report of repeated-filter confusion, or the spec seat opening the filter acceptance surface for any other reason — the ingress-tagging decision should ride that card.
    • Dedup: finding: a repeated ?paradigm=/?source=/?category=/?type= on the automation descriptor routes silently empties the designer palette (200, zero rows) #7360 is the same repetition family on the automation description endpoints (different surface, already routed domain:cli); cross-linked by family, not duplicates.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. claude commented on Aug 10, 2026

    @claude
    Contributor

    Findings round — held (finding + domain:metadata kept), premise re-verified.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. claude commented on Aug 11, 2026

    @claude
    Contributor

    Findings cadence (triage seat): HOLD — re-verified on origin/main @ 7e93c89d: malformedFilterArrayError / isFilterAST seam unchanged (packages/metadata-protocol/src/protocol.ts:686-699), and the class stays unreachable today — the Hono adapter still collapses repeated parameters per the landed readSingleQueryValue direction (packages/plugins/plugin-hono-server/src/adapter.ts:185). Dormancy ends with #6878 route 2; the exit is the acceptance-surface decision the card records (spec seat territory), so it holds until that unlock.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    ContributorAuthor

    Findings cadence (triage seat): escalated — finding → needs-user-decision, re-routed domain:metadata → domain:spec. The card's dormancy premise is overtaken by events.

    What changed (measured on origin/main @ 2c1988c):

    The fork for the maintainer (the card's own two options, now with live stakes):

    • (a) Narrow the wire form of filter on the querystring face (stop accepting the bare AST array there) — an acceptance-surface change, which is why this routes domain:spec (the [PM seat] domain:spec-surface — 🔀 merged into #6017 #6298 red line: anything changing accept/refuse behavior is the spec seat's, regardless of size);
    • (b) Give the normalizer an ingress discriminator (a new packages/rest ↔ packages/metadata-protocol contract) so the querystring face can refuse repetition while the body face keeps the AST.

    Not target:v17: reachable but requires a client to repeat a parameter; the common outcome is a correct refusal with a wrong cause message — not one of the four blocking classes.

    Label pair updated with this comment (escalation + re-route in the same stroke).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  5. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    ContributorAuthor

    Maintainer ruling recorded 2026-08-11 (spec-lane PM session chat, verbatim: 「接受你的建议,开始加速处理」, accepting the lane sweep's recommendation on this card).

    Ruling: refuse, narrowest shape. A repeated ?filter= on GET /data/:object is refused explicitly — 400 INVALID_FILTER with an actionable message naming the condition ("repeated filter parameter — send exactly one") — instead of today's confusing malformed-filter diagnosis (and the rare accidental success). Last-wins and AND-merge are rejected: silent selection among duplicates is the AI-authoring trap this lane refuses on principle.

    Routing with the ruling attached: the enforcement lands at the REST query-param parse (packages/rest), which the domain table routes to domain:cli — re-labeled accordingly under the maintainer's lane-triage authorization (「你的车道你可以执行 分类改标」). The cli seat receives this card RULED and queue-ready; the spec-side contract prose (filter is single-valued) rides the same PR if the http-protocol docs state otherwise.

    State: needs-user-decision → pm:queue, domain:spec → domain:cli.


    Generated by Claude Code

  6. self-assigned this
    on Aug 12, 2026
  7. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    Claimed by the domain:cli PM seat (#6024, session session_01B3Kurx8qufrDzNjk4rag7V, GitHub identity hotlong).
    Branch: claude/issue-7390-repeated-filter-param-refusal · dispatch: mode:subagent, model: claude-opus-5.

    ⚠️ Read the comments, not the body — two of its sections are overtaken

    Anyone picking this card up from the description alone will build the wrong thing. Both corrections re-verified on origin/main before dispatch:

    The ruling (binding)

    Refuse, narrowest shape. A repeated ?filter= on GET /data/:object is refused explicitly — 400 INVALID_FILTER with a message naming the actual condition ("repeated filter parameter — send exactly one"), replacing today's malformed-filter diagnosis and the rare accidental success.

    ⛔ Last-wins and AND-merge are rejected. Silent selection among duplicates is the AI-authoring trap this lane refuses on principle — an agent that repeats a parameter must be told, not quietly served one of its two intentions.

    Landing site is the REST query-param parse in packages/rest, not the shared normalizer: packages/metadata-protocol's normalizer serves two ingresses it cannot tell apart, and a filter AST is an array, so #7386's arity gate cannot judge this slot. That is exactly why the fix belongs at the ingress that knows it is a querystring.

    Do not invent the shape — the file already has one

    rest-server.ts:3521 carries an established refusal for this class ("ONE owning package, so repetition is refused rather than…"), and :38 holds the [#6877] multiplicity commentary. Follow the existing pattern rather than authoring a second dialect for the same rule. ⛔ And no heuristics in the normalizer — "an array of strings that are each parseable JSON is probably a repetition" is precisely what #4181 and #4121 spent effort removing.

    Pins

    • ?filter=A&filter=B → 400, asserting the error code and the HTTP status, and that the message names repetition rather than malformedness. The last part is the whole point of the card — a test that only checks 400 would pass against today's wrong diagnosis.
    • The accidental-success shape is closed: ?filter=status&filter=%3D&filter=open — which parses today as a valid AST and returns 200 with a filter nobody expressed — is refused.
    • A single ?filter= still works, in both its JSON-object and AST forms.
    • POST /data/:object/query with a body-form AST is untouched — that face legitimately sends an array.
    • Reverse-verify: revert the guard, confirm the repetition test fails by returning the old diagnosis (or the 200), not a compile error.

    Serial constraint

    packages/rest/src/rest-server.ts is a hot file. PR #7979 (#7678) is in the merge queue touching registerSecurityEndpoints — a different region, so a textual conflict is unlikely, but branch from a main that already contains it if it has landed, and rebase rather than resolve blind if it has not.

    Spec-side prose rides this PR only if content/docs/protocol/** currently says filter is multi-valued; check before writing.


    Generated by Claude Code

  8. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor
    {
      "issue": 7390,
      "status": "done",
      "branch": "claude/issue-7390-repeated-filter-param-refusal",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/8004",
      "pr_url": "https://github.com/objectstack-ai/objectstack/pull/8004",
      "premise_still_valid": true,
      "what_changed": "A repeated ?filter= on GET /data/:object is now refused at the REST querystring parse with 400 INVALID_FILTER and a message naming REPETITION, replacing the malformed-filter misdiagnosis (common shape) and the rare accidental 200 (?filter=status&filter=%3D&filter=open, which spelled a valid AST and applied a filter nobody expressed). Both shapes were confirmed live on origin/main: the Hono adapter reads c.req.queries() at adapter.ts:202 since #6878 route 2 / PR #7396. New exports in packages/rest/src/query-multiplicity.ts: FILTER_SLOT_QUERY_PARAMS, repeatedFilterParamMessage, assertFilterParamSuppliedOnce. All four wire spellings of the one slot are covered; where/filter are DERIVED from the spec's RPC_QUERY_ALIAS_SLOTS, filters/$filter are named locally because @objectstack/metadata-protocol is a dev-only dependency of rest. Last-wins and AND-merge were not implemented, per the ruling. No spec change: INVALID_FILTER is already a standard-catalog code, so no ledger entry either.",
      "landing_site": "packages/rest/src/rest-server.ts GET /data/:object handler (the former '[#6877] Deliberately NOT gated here' block, now recording WHY the filter slot alone is the exception), calling assertFilterParamSuppliedOnce from packages/rest/src/query-multiplicity.ts — the module that already owns this rule. NOT packages/metadata-protocol, and no heuristic added there. The gate THROWS rather than responding: the data routes speak the flat mapDataError envelope, which is the same envelope this route's other filter refusals (unusableFilterError, malformedFilterArrayError — both 400/INVALID_FILTER) already arrive in, so one slot keeps one wire code and one body shape. Responding here would have authored a second dialect. INVALID_FILTER is already in isExpectedQueryRejection, so the refusal prints no '[REST] Unhandled error' line (pinned).",
      "message_text": "Repeated \"filter\" query parameter — send exactly one. It was supplied 2 times. A repeated filter is neither merged nor resolved by precedence: either would apply a filter you did not express.",
      "tests_added": "packages/rest/src/rest-server-repeated-filter-param.test.ts — 24 cases, all passing. §1 refusals: both shapes, every spelling (where/filter/filters/$filter), two IDENTICAL values still refused, and the no-unhandled-error-log pin. Every refusal asserts the ADR-0112 pair (status AND code) AND that the message names repetition and does NOT contain 'malformed' — that third assertion is the card, because 400 + INVALID_FILTER were both already true of the misdiagnosis. §2 preservation: a single ?filter= in BOTH accepted forms (JSON object and bare AST) reaches findData byte-identical; one-element array unwrapped; $select keeps its array arm; filterless request untouched. §3 the negative pin. §4 real engine + real normalizer. §5 the spelling set composes to exactly four.",
      "reverse_verification": "Fix committed FIRST, then the guard call removed from the committed state and the suite re-run: 10 failed / 14 passed, NO compile error. It produced BOTH directions the card allows. (a) The OLD DIAGNOSIS, through the real stack: \"expected 'Malformed $filter: unrecognised opera…' to be 'Repeated \"filter\" query parameter — s…'\". (b) The ACCIDENTAL 200, through the real engine: \"expected a 400 for a repeated \"filter\", got 200 with body {\"object\":\"task\",\"records\":[{\"id\":\"1\",...,\"status\":\"open\"}],\"total\":1,\"hasMore\":false}\" — exactly the one 'open' row nobody asked to filter on. The mocked-protocol cases fell to 200 {\"records\":[]}, and the one-element-array case failed as \"expected [ '{\"status\":\"open\"}' ] to be '{\"status\":\"open\"}'\", proving the unwrap is real work and not a no-op. Restored with git checkout from the commit (never git stash); suite green again at 57/57 across both multiplicity suites.",
      "body_face_untouched_proof": "POST /data/:object/query is a SEPARATE handler reading req.body (rest-server.ts:6897); the gate is called only in the GET handler and takes req.query as its whole input. Pinned three ways in §3: a body-form flat AST ['status','=','open'] returns 200 and reaches findData with the array intact; a nested body AST [['status','=','open'],'and',['done','=',false]] likewise; and a structural case handing the helper that same shape directly, which DOES throw — so the separation is demonstrably the call site, not an accident of the data. §4 adds the end-to-end half on a real engine: the identical array that is refused on the querystring still lowers to a working filter one layer down.",
      "tests": "pnpm -w typecheck via turbo --concurrency=2: 127/127 successful (this whole-workspace run is also the consumer sweep — no single-direction --filter ambiguity). @objectstack/rest suite: 95 files / 1555 tests passed. New file alone: 24/24. check:type-check-debt — @objectstack/rest TEST_DEBT re-measured at 155, EXACTLY its recorded number, zero errors attributable to the new file; the ledger did NOT rise. A first measurement of 156 (one TS2554 from a one-argument registerObject) was FIXED, not ledgered. check:nul-bytes OK (7308 files); check:error-code-casing OK. CI on PR #8004 at report time: in_progress (reported honestly rather than idle-polled; 'No other open PR may claim the same issue' already green).",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #8001: a repeated ?filter= answers INVALID_FILTER on GET /data/:object (this ruling) but VALIDATION_ERROR on GET /data/:object/export (the shared #6877 helper) — one condition, two codes, decided by route. Both are individually right, so it is recorded as a fork rather than chosen unilaterally. Left unlabeled for PM triage.",
        "filed as #8002 (finding): the wire-only filter alias spellings filters/$filter are now declared in two packages with no gate reconciling them — a new wire-only alias would fold in the normalizer and be silently ungated at the ingress. The spec-derived half IS pinned by §5; the wire-only half cannot be, because metadata-protocol is not on rest's runtime graph.",
        "filed as #8003 (finding): the normalizer's ARRAY_VALUED_QUERY_SLOTS docblock still says a repeated ?filter= 'is still refused, one block down, by isFilterAST failing to read it' — which always described the defect as if it were the answer, and is now stale for this route too. Not fixed here because the ruling kept this PR out of packages/metadata-protocol and protocol.ts is a hot shared file (same handling as #7707 / #7874)."
      ],
      "risks": [
        "BEHAVIOUR CHANGE on a live surface, stated plainly in the changeset: ?filter=status&filter=%3D&filter=open moves 200 -> 400, and the common repeated-filter 400 changes its message. Anyone who had (unknowingly) come to rely on the accidental AST is now refused — which is the ruling's intent, not a regression.",
        "The docs check came back NEGATIVE, so nothing rides: content/docs/protocol/kernel/http-protocol.mdx documents filter as a single JSON parameter and nothing in content/docs/protocol/** claims it is multi-valued. Verified before writing, per the card.",
        "Scope note for review: the ruling says 'repeated ?filter=' and I gated all FOUR wire spellings of that ONE slot (filter/where/filters/$filter). Read as applying one rule to one slot consistently — gating only the literal 'filter' would have left three identical holes. If the PM reads 'narrowest shape' as one spelling only, this is the line to trim.",
        "PR #7979 (#7678) had already landed; branched from an origin/main containing it (ee264b294). It touches registerSecurityEndpoints, a different region of rest-server.ts — no conflict, no rebase needed."
      ]
    }

    Generated by Claude Code


    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