fix(rest): refuse a repeated ?filter= as a repetition, not as a malformed filter (#7390)

[hotlong]

Fixes#7390

Implements the maintainer ruling recorded on the issue 2026-08-11 — quoted verbatim and untranslated:

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.

What was wrong

Two of the issue description's sections were overtaken by events before this was picked up; both were re-verified on origin/main before implementing.

"Not reachable today" had expired.packages/plugins/plugin-hono-server/src/adapter.ts:202 reads c.req.queries() ?? {} since #6878 route 2 landed (PR #7396), so repeated parameters reach findData's normalizer as string[] and both shapes in the body were live.

The normalizer structurally cannot judge this slot.#7386's arity gate keys off each slot's declared value type, and a filter AST is an array — ["status","=","open"]. The normalizer serves two ingresses it cannot tell apart (GET /data/:object, where a repeat is string[], and POST /data/:object/query, whose body is arbitrary JSON), so on this one slot Array.isArray is not evidence of anything.

request	before	after
?filter={"a":1}&filter={"b":2}	400 INVALID_FILTER, diagnosed as malformed	400 INVALID_FILTER, diagnosed as repetition
?filter=status&filter=%3D&filter=open	200, applying {status:"open"}	400 INVALID_FILTER

The first was the common shape and its message was actively misleading — both filters the caller sent were well-formed, and the response told them to check the AST syntax that was never wrong. The second is contrived to write by hand but is the sharper defect: three occurrences of one parameter happened to spell a valid AST, so the request succeeded while applying a filter nobody expressed.

Where it lands, and why not the obvious place

At the REST querystring parse (packages/rest), not in the shared normalizer. The querystring ingress is the only layer that knows it is a querystring — and there an array on the filter slot is a repeated parameter and can be nothing else. That keeps the normalizer free of the heuristic ("an array of strings each parseable as JSON is probably a repetition") that #4181 and #4121 spent effort removing, and leaves the body face alone.

packages/spec does not move: INVALID_FILTER is already a standard-catalog code (errors.zod.ts), so no ledger entry is needed either.

The shape follows the file's existing one

The gate lives in query-multiplicity.ts, the module that already owns this rule ([#6877]), and reuses its readSingleQueryValue count-not-shape semantics — so a one-element array is one occurrence and is unwrapped rather than refused.

It throws rather than responding, which is the one deliberate difference from its sibling refuseRepeatedQueryParams. The data routes speak the flat mapDataError envelope, and that is the envelope this route's other filter refusals already arrive in — unusableFilterError and malformedFilterArrayError both throw 400 / INVALID_FILTER and are shaped by the handler's own catch. Throwing from inside the same try gives one slot one wire code and one body shape, whether the filter was unreadable or sent twice; responding here would have authored a second dialect for one condition. INVALID_FILTER is already in isExpectedQueryRejection's vocabulary, so the refusal does not print an "[REST] Unhandled error" line.

All four wire spellings of the one slot are covered. where / filter are derived from the spec's own RPC_QUERY_ALIAS_SLOTS ("the ONE place the alias to canonical mapping is declared"), so a spelling added there reaches the gate; filters / $filter are wire-only and named locally, because @objectstack/metadata-protocol is a dev-only dependency and no runtime import of its table exists. That asymmetry is filed as #8002.

Pins

packages/rest/src/rest-server-repeated-filter-param.test.ts, 24 cases:

• §1 refusals — both shapes, on every spelling, plus two identical values (still two occurrences), plus a check that the refusal is not logged as an unhandled fault. Every case asserts the ADR-0112 pair (statusandcode) and that the message names repetition and does not say "malformed". That third assertion is the point: 400 + INVALID_FILTER were both already true of the misdiagnosis, so a status-only test passes green against the bug.
• §2 preservation — a single ?filter= in both accepted forms (JSON object and bare AST) reaches findData byte-identical; a one-element array is unwrapped; $select keeps its array arm; a filterless request is untouched.
• §3 the negative pin — POST /data/:object/query with a body-form AST (flat and nested) is forwarded, proving the ingress was gated and not the parser.
• §4 the layer below, on a real engine and the real normalizer: handed the repetition directly it still answers Malformed $filter: unrecognised operator..., and handed the accidental AST directly it still returns the wrong row happily. Those two are the reverse verification kept as tests rather than as a transcript — they document why deleting the gate is not a cleanup.
• §5 — the spelling set composes to exactly the four wire spellings.

Reverse verification

Guard removed from the committed state, suite re-run: 10 failed / 14 passed, no compile error, and it produced both directions the card allows for.

The old diagnosis, through the real stack:

AssertionError: expected 'Malformed $filter: unrecognised opera...'
to be 'Repeated "filter" query parameter - s...'

The accidental 200, through the real engine — the request returning exactly the one open row nobody asked to filter on:

expected a 400 for a repeated "filter", got 200 with body
{"object":"task","records":[{"id":"1",...,"status":"open"}],"total":1,"hasMore":false}

The mocked-protocol cases fell to 200 {"records":[]}, and the one-element-array case failed as expected [ '{"status":"open"}' ] to be '{"status":"open"}' — so the unwrap is real work, not a no-op. Guard restored with git checkout from the commit (never git stash); suite green again.

Verification

• pnpm -w typecheck (turbo, --concurrency=2): 127/127 successful. That whole-workspace run is also the consumer sweep — no single-direction --filter ambiguity.
• @objectstack/rest suite: 95 files / 1555 tests passed.
• check:type-check-debt: @objectstack/rest TEST_DEBT re-measured at 155, exactly its recorded number, with zero errors attributable to the new file. The entry has no margin by design, so a first measurement of 156 (one TS2554 from a one-argument registerObject) was fixed, not ledgered.
• check:nul-bytes OK; check:error-code-casing OK.

Not in this PR

content/docs/protocol/** was checked before writing anything: kernel/http-protocol.mdx documents filter as a single JSON parameter and nothing claims it is multi-valued, so no spec-side prose rides this change.

Out-of-scope findings

Filed unassigned per Prime Directive #10, none fixed here:

• A repeated ?filter= answers two different error codes depending on which data route received it #8001 — a repeated ?filter= answers INVALID_FILTER on this route and VALIDATION_ERROR on GET /data/:object/export. Both are individually right (this one by ruling, that one by the shared packages/rest 的其它 req.query.* 读取点同样把 string | string[] 当字符串用(#6307 的未扩大部分) #6877 helper); recording the fork rather than choosing for the maintainer.
• finding: the wire-only filter alias spellings (filters / $filter) are now declared in two packages, with no gate reconciling them #8002 — filters / $filter are now declared in two packages with no gate reconciling them.
• finding: the list-query normalizer's where arity note still says a repeated ?filter= is refused by isFilterAST failing #8003 — the normalizer's where arity note still says a repeated ?filter= is refused by isFilterAST failing, which was always describing the defect as the answer and is now stale for this route too.

---

Generated by Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Actions	Updated (UTC)
objectstack	Ignored		Aug 12, 2026 10:57am

[github-actions]

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/rest.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

• content/docs/ai/connect-mcp.mdx(via @objectstack/rest)
• content/docs/api/error-handling-server.mdx(via @objectstack/rest)
• content/docs/api/index.mdx(via @objectstack/rest)
• content/docs/permissions/authentication.mdx(via @objectstack/rest)
• content/docs/permissions/system-context.mdx(via packages/rest)
• content/docs/plugins/index.mdx(via @objectstack/rest)
• content/docs/plugins/packages.mdx(via @objectstack/rest)
• content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/rest)
• content/docs/protocol/kernel/i18n-standard.mdx(via packages/rest)

⛔ 3 release-owned page(s) also reference the affected code. These are read-only:

• content/docs/releases/implementation-status.mdx(via @objectstack/rest)
• content/docs/releases/v12.mdx(via @objectstack/rest)
• content/docs/releases/v17.mdx(via @objectstack/rest)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

[hotlong]

PM review — domain:cli seat (#6024). Verdict: accept, pending CI. No change requests.

Every pin is here, including the one that decides whether this card was actually closed: the message must name repetition, not malformedness. The PR states the reason better than the dispatch did — "400 + INVALID_FILTER were both already true of the misdiagnosis, so a status-only test passes green against the bug." That is exactly why a toBe(400) test would have shipped a green suite over an unchanged defect.

Both stale premises were re-verified rather than taken from the claim comment, which is the right handling for a card whose body inverts its own verdict.

Two things beyond the ask

§4 keeps the reverse verification as tests, not as a transcript. Handed the repetition directly, the real normalizer still answers Malformed $filter: unrecognised operator…; handed the accidental AST, it still returns the wrong row happily. So the file now documents why deleting this gate is not a cleanup — the layer below is measured, in CI, to be unable to do this job. A future simplification that removes the gate turns those red instead of finding a green suite and a plausible-looking diff.

Throwing instead of responding is the correct deviation, and the reasoning generalizes.unusableFilterError and malformedFilterArrayError already throw 400 / INVALID_FILTER into the handler's own catch, so throwing gives this slot one wire code and one body shape whether the filter was unreadable or sent twice. Responding directly would have authored a second dialect for a single condition — the exact defect class this lane has closed five times today. Deviating from the sibling helper to avoid that is a better read of the rule than following it would have been.

Also correct: where / filterderived from RPC_QUERY_ALIAS_SLOTS rather than transcribed, with the filters / $filter asymmetry named and filed (#8002) instead of quietly hand-copied. The dispatch's ⛔ on heuristics is respected — the querystring ingress knows it is a querystring, so no guessing is needed anywhere.

Ledger discipline held

@objectstack/rest first measured 156 — one TS2554 from a one-argument registerObject in the new test — and it was fixed, not ledgered, back to the recorded 155. That entry has no margin by design and the PR says so.

The three findings are correctly scoped

#8001 is the one I want on the record: a repeated ?filter= now answers INVALID_FILTER here and VALIDATION_ERROR on GET /data/:object/export. Both are individually right — this one by the maintainer's ruling, that one by the shared #6877 helper — and reconciling them is a choice about which vocabulary owns the condition. Recording the fork instead of picking one is right; picking would have been a ruling made inside an implementation.

#8003 is a sharp catch: the normalizer's where arity note says a repeated ?filter= is refused by isFilterAST failing — which was always describing the defect as though it were the answer, and is now stale for this route on top of that.

Docs correctly not touched: kernel/http-protocol.mdx already documents filter as a single JSON parameter, checked before writing rather than after.

---

Generated by Claude Code

[hotlong]

One addendum to the review above — not a change request, and not a reason to pull this out of the queue.

The docs check in this PR went to content/docs/protocol/kernel/http-protocol.mdx, which was a reasonable place to look and correctly reported nothing claiming filter is multi-valued. The page that turned out to matter is a different one:

content/docs/api/data-api.mdx:22 is the row that documents this parameter and its rejection behaviour:

filter | query | Filter expression (JSON). filters also accepted for backward compatibility. Malformed JSON is rejected with 400 INVALID_FILTER — never ignored.

That sentence stays true — this PR does not change malformed-JSON handling. But the page now under-describes the parameter it documents: a second condition answers the same code on the same slot, and it is the condition this PR exists to make legible. Leaving it out reproduces #7390's own gap one layer up — the caller is still not told that sending it twice is the problem.

Incomplete, not incorrect, so it is filed as #8005 rather than churning a queued PR for one clause.

Worth noting for the drift advisory's own credibility: data-api.mdx is not among the 9 docs it listed here, despite being the most relevant. That is the second instance today of the mechanism recorded in #7967 — the check maps docs to packages by textual mention, so a page describing a package's behaviour without naming the package is invisible to it. Measured with a control (VALIDATION_ERROR → 4 files under content/docs/api/), so the miss is real rather than a bad query on my side.

Verdict unchanged: accept, auto-merge on.

---

Generated by Claude Code

[hotlong]

PM ruling on the scope question raised in the report — gating all four spellings is correct. Do not trim.

The report flags this honestly and asks rather than assumes:

the ruling says "repeated ?filter=" and I gated all FOUR wire spellings of that ONE slot … If the PM reads "narrowest shape" as one spelling only, this is the line to trim.

"Narrowest shape" modifies the remedy, not the coverage. It was the maintainer distinguishing refuse from the alternatives on the table — last-wins, AND-merge, and the acceptance-surface change that would have narrowed filter's wire form. It says nothing about how many spellings reach the slot, because the alternatives it was ruling out were all about what to do, not where.

where / filter / filters / $filter are one slot. Gating only the literal filter would leave three identical holes, reachable by any caller who used an alias — the same defect under a different name, and a fourth instance of the one-rule-N-implementations divergence this lane has closed five times today. A refusal that depends on which synonym the caller happened to type is not narrower; it is just inconsistent.

Deriving where / filter from the spec's RPC_QUERY_ALIAS_SLOTS rather than transcribing them is what makes this hold: the coverage tracks the declaration, so a spelling added there is gated the day it is declared. The filters / $filter half genuinely cannot be derived — @objectstack/metadata-protocol is not on rest's runtime graph — and naming that asymmetry in #8002 instead of hand-waving it is the right disposition.

#8001 stays unlabeled and ungraded, as filed. One condition answering INVALID_FILTER here and VALIDATION_ERROR on /data/:object/export is a question about which vocabulary owns the condition — a ruling, not an implementation. Declining to pick it inside this PR was correct, and this seat is not picking it either; grading belongs to the triage seat.

The behaviour-change risk is stated the way it should be: ?filter=status&filter=%3D&filter=open moves 200 → 400, and anyone who had unknowingly come to depend on that accidental AST is now refused. That is the ruling's intent, and the changeset says so plainly rather than describing this as a pure bug fix.

Verdict unchanged: accept, auto-merge on.

---

Generated by Claude Code