You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
A repeated ?version= on GET /packages/:id is refused with 404 where the landed precedent for that exact condition on that exact route is 400 VALIDATION_ERROR — and the one module that owns the rule is unreachable from the dispatcher package #17672
Filed by the domain:cli execution PM seat (pm:seat#6024, session session_01TSf4DV7ziu4V5j73e46b7c) out of PR #17668's acceptance review. ⛔ Not graded, no domain:*, no priority, no assignee — that is triage's.
What lands, and what is right about it
PR #17668 (card #17416) teaches GET /api/v1/packages/:id to honour ?version=. A repeated ?version=a&version=b is refused rather than silently resolved to one of the two — correct, and strictly better than the 200-with-the-installed-row it replaces. ⛔ Nothing here asks for that to be undone.
The defect
It is refused with 404, and this repo already has a landed answer for exactly this condition on exactly this route.
packages/rest/src/query-multiplicity.ts (~:110–:116), which its own header calls the authority for this condition:
400 with the ADR-0112 nested body { error: { code, message } } … VALIDATION_ERROR is not a new code: it is the standard catalog's member for 400 (spec/src/api/errors.zod.ts, standardErrorCodeForHttpStatus(400)), and the same code #6307 chose for this same condition on /packages/:id. Nothing in packages/spec moves.
⇒ A repeated query parameter is a request-shape error, and 404 says "not found". A client branching on status cannot tell "you asked for a version I do not have" (a real 404 this PR also introduces, correctly) from "your request named the parameter twice".
Why PR #17668 could not simply use the rule — in the landed code's own words
From the docblock the PR adds to packages/runtime/src/domains/packages.ts:
The repo's ONE rule for this condition answers 400 VALIDATION_ERROR (refuseRepeatedQueryParams / repeatedQueryParamMessage in packages/rest/src/query-multiplicity.ts, whose header is the authority) and that is the right end state for this door as well. ⛔ It is NOT restated here: that module is not exported from @objectstack/rest's barrel, so calling it from this package would mean widening another package's public surface, and a second copy of the rule with a second message is the drift its own header forbids.
That reasoning is sound and this seat accepted it: the three alternatives available inside that card's file surface were all worse (silently picking one value is the very defect class the card is about; copying the rule is the drift its header forbids; exporting it widens another package's published surface, which is a Clause-② act outside the declared surface). ⇒ The blocker is reachability, not judgement, which is why it is its own card rather than a review nit.
Measured by this seat on origin/main @ 89559950:
packages/rest/package.json declares exactly one export subpath: "." → dist/index.js / dist/index.cjs. There is no subpath through which a sibling package could reach this module.
git grep -n "query-multiplicity" origin/main -- packages/rest/src/index.ts → zero hits (exit 1). Positive control, same term, same run: git grep -ln "query-multiplicity" origin/main -- packages/rest/src → 5 files (query-allowlist.ts, query-multiplicity.ts, rest-server-query-multiplicity.test.ts, rest-server-repeated-filter-param.test.ts, rest-server.ts). The channel is alive, so the zero is a reading.
Second half — a landed sentence that PR #17668 makes false
The same module's header, explaining why the rule has one home (~:58–:63):
#6307 landed the first copy of this rule in package-routes.ts, on the ?version= of that registrar's package read/delete routes. Those routes are gone (#14503 — the dispatcher's /packages domain is their single implementation, and it reads no version), so the rule now has one home: here, for the rest-server.ts read points — ONE rule and one message, not a second implementation that drifts.
After PR #17668 merges, "it reads no version" is measurably false: that domain reads version and refuses a repeated one with its own message. ⚠️ The parenthetical is not decorative — it is part of the stated reason the rule needs only one home. The module now understates its own scope.
⛔ PR #17668 correctly did not edit it: packages/rest was outside that card's declared file surface. And per the Acceptance-notes test — "哪一个 PR 会碰到这个文件?说得出具体 PR 或人 ⇒ 写进去" — the successor it named is "whoever converts the /packages lane to a closed set", which names no PR and no person, so the note's fallback does not hold and a card is owed. That is this card.
GET /api/v1/packages/<installed-id>?version=a&version=b
→ 404 Package '<id>' — the "version" query parameter was supplied 2 times, so this read names no single version. Supply it at most once.
Expected, per the precedent above: 400 with the ADR-0112 nested body and VALIDATION_ERROR.
Pinned by packages/runtime/src/domains/packages-get-version-scope.test.ts §4, so whoever fixes this changes that test deliberately — ⚠️ it is a pin on the interim answer, not on the end state.
What a fix would decide (⛔ not decided here)
Make the rule reachable — export refuseRepeatedQueryParams / repeatedQueryParamMessage from @objectstack/rest's entry (or give the module a subpath) so the dispatcher's domains can call the one implementation. ⚠️ Widens a published surface ⇒ Clause-②: yes.
Move the refusal to where the rule lives — the ingress check happens in packages/rest before the dispatcher domain is reached. ⚠️ Changes which layer owns query-shape validation for dispatcher routes, and the Route and surface ownership rule is explicit that the closed-set conversion happens per lane and never as one sweep.
Rule that this door legitimately answers a different status and record why. ⚠️ Then the #6307 chose … on /packages/:id sentence and the reads no version parenthetical both need correcting, so the header edit is owed on this branch too.
Every branch owes the one-line header correction. ⇒ the header line is the part that is not a decision.
Not duplicates (checked, including closed; search channel returned 2 on-topic hits, so the zero for a duplicate is a reading)
Filed by the
domain:cliexecution PM seat (pm:seat#6024, sessionsession_01TSf4DV7ziu4V5j73e46b7c) out of PR #17668's acceptance review. ⛔ Not graded, nodomain:*, no priority, no assignee — that is triage's.What lands, and what is right about it
PR #17668 (card #17416) teaches
GET /api/v1/packages/:idto honour?version=. A repeated?version=a&version=bis refused rather than silently resolved to one of the two — correct, and strictly better than the200-with-the-installed-row it replaces. ⛔ Nothing here asks for that to be undone.The defect
It is refused with
404, and this repo already has a landed answer for exactly this condition on exactly this route.packages/rest/src/query-multiplicity.ts(~:110–:116), which its own header calls the authority for this condition:⇒ A repeated query parameter is a request-shape error, and
404says "not found". A client branching on status cannot tell "you asked for a version I do not have" (a real404this PR also introduces, correctly) from "your request named the parameter twice".Why PR #17668 could not simply use the rule — in the landed code's own words
From the docblock the PR adds to
packages/runtime/src/domains/packages.ts:That reasoning is sound and this seat accepted it: the three alternatives available inside that card's file surface were all worse (silently picking one value is the very defect class the card is about; copying the rule is the drift its header forbids; exporting it widens another package's published surface, which is a
Clause-②act outside the declared surface). ⇒ The blocker is reachability, not judgement, which is why it is its own card rather than a review nit.Measured by this seat on
origin/main@89559950:packages/rest/package.jsondeclares exactly one export subpath:"."→dist/index.js/dist/index.cjs. There is no subpath through which a sibling package could reach this module.git grep -n "query-multiplicity" origin/main -- packages/rest/src/index.ts→ zero hits (exit 1). Positive control, same term, same run:git grep -ln "query-multiplicity" origin/main -- packages/rest/src→ 5 files (query-allowlist.ts,query-multiplicity.ts,rest-server-query-multiplicity.test.ts,rest-server-repeated-filter-param.test.ts,rest-server.ts). The channel is alive, so the zero is a reading.Second half — a landed sentence that PR #17668 makes false
The same module's header, explaining why the rule has one home (~
:58–:63):After PR #17668 merges, "it reads no⚠️ The parenthetical is not decorative — it is part of the stated reason the rule needs only one home. The module now understates its own scope.
version" is measurably false: that domain readsversionand refuses a repeated one with its own message.⛔ PR #17668 correctly did not edit it:
packages/restwas outside that card's declared file surface. And per the Acceptance-notes test — "哪一个 PR 会碰到这个文件?说得出具体 PR 或人 ⇒ 写进去" — the successor it named is "whoever converts the/packageslane to a closed set", which names no PR and no person, so the note's fallback does not hold and a card is owed. That is this card.Repro (once PR #17668 is on
main)Expected, per the precedent above:
400with the ADR-0112 nested body andVALIDATION_ERROR.Pinned by⚠️ it is a pin on the interim answer, not on the end state.
packages/runtime/src/domains/packages-get-version-scope.test.ts§4, so whoever fixes this changes that test deliberately —What a fix would decide (⛔ not decided here)
refuseRepeatedQueryParams/repeatedQueryParamMessagefrom@objectstack/rest's entry (or give the module a subpath) so the dispatcher's domains can call the one implementation.Clause-②: yes.packages/restbefore the dispatcher domain is reached.#6307 chose … on /packages/:idsentence and thereads no versionparenthetical both need correcting, so the header edit is owed on this branch too.Every branch owes the one-line header correction. ⇒ the header line is the part that is not a decision.
Not duplicates (checked, including closed; search channel returned 2 on-topic hits, so the zero for a duplicate is a reading)
?limit=and?cursor=are declared and never read,?type=is read and never declared #17667 (open, filed by the GET /api/v1/packages/:id silently ignores ?version= — the only serving surface never reads it, and the handler that did was deleted with the REST twin #17416 dev) — the/packagesread doors' declared request schemas vs their actual query reads:?limit=/?cursor=/?enabled=declared and never read,?type=/?overwrite=/?keepData=read and never declared. A different artefact (the zod request schemas inpackages/spec) and a different fix./approvals/requests— decide whether the closed-parameter-set rule becomes ingress policy #7606 (closed) — unknown query parameters silently dropped, as an ingress-policy decision. This is repeated parameters, which already have a landed rule.?version=honouring itself, deliberately scoped to that one parameter on that one door.GET/DELETE /packages/:id把重复的?version=查询参数(string[])原样交给 PackageService #6307 (the precedent) — the original 400 for this condition, on routes that have since been removed by packages/rest package read routes claim to SHADOW their dispatcher twins, but the dispatcher answers /packages and /packages/:id on a stock showcase boot #14503.