fix(runtime): a repeated ?version= on GET /packages/:id answers 400 VALIDATION_ERROR from the one shared rule, and @objectstack/rest publishes it (#17672) - #17815
Conversation
…ALIDATION_ERROR from the one shared rule (#17672) The door refused a repeated `?version=` with `404` and a sentence of its own, so a request-shape error was indistinguishable from the two genuine not-founds the same door answers. The repo's one rule for this condition already answers `400 VALIDATION_ERROR` in the ADR-0112 nested body; what blocked #17668 from calling it was that `packages/rest/src/query-multiplicity.ts` is reachable from nowhere outside its package. - `@objectstack/rest`'s barrel publishes `repeatedQueryParamMessage` and `refuseRepeatedQueryParams`, with the entry recording which half is portable across a package boundary and which is not. - The dispatcher's `/packages` domain calls the message function and drops its local copy; `deps.error(msg, 400)` derives `VALIDATION_ERROR`. - The module header's "and it reads no `version`" parenthetical is corrected — false since #17668 landed, and load-bearing prose about why the rule has one home. - `packages-get-version-scope.test.ts` §4's pin is deliberately changed from the interim `404` to the end state, and §5 pins the distinction the card is about. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
…bility Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift CheckThis PR changes 2 package(s): 18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 32 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin f79df6bbb98718c2c5f06670b7fb86d6763a751c && git checkout f79df6bbb98718c2c5f06670b7fb86d6763a751c
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 2070a1d865d98cd0f0299441f36d278c45ecdfdf 2969466d0cf86443511ede8986995ebccef36ab7 && git checkout -B drift-repro 2070a1d865d98cd0f0299441f36d278c45ecdfdf && git merge --no-ff 2969466d0cf86443511ede8986995ebccef36ab7
node scripts/docs-audit/affected-docs.mjs --json 2070a1d865d98cd0f0299441f36d278c45ecdfdf
|
Comment-only, on the pin file. The docs-drift advisory on the PR surfaced two pages that bear on this diff, both re-read on the branch: - `content/docs/api/client-sdk.mdx` already documents `VALIDATION_ERROR` / 400 for "a repeated query parameter", so the door contradicted a published page for as long as it answered 404 — a stronger justification than the precedent alone, and nothing to edit there. - `content/docs/kernel/contracts/metadata-service.mdx` states that a missing id on this route answers `404 RESOURCE_NOT_FOUND` with the message `Package 'ID' not found`. Both halves are already asserted here; the comment names the page so the pin says what it protects. No release-owned page is touched. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
|
| job | conclusion |
|---|---|
Type Check · source gates |
success |
Type Check · workspace |
cancelled |
Type Check · debt ledger |
cancelled |
Type Check · consumer gates |
cancelled |
Lint & Repo Gates |
cancelled |
TypeScript Type Check (aggregator) |
failure |
⇒ zero shards failed. One passed, four were cancelled, and the aggregator reports failure over cancellations — the ruled fail-closed behaviour of this check. ⛔ Never filed and ⛔ never "fixed": the maintainer refused to whitelist lifecycle values (2026-08-07), #3668 rewired it and #6082 counts shard attestations.
Why they were cancelled
| reading | result |
|---|---|
| the run itself | 34681268539 has run-level conclusion: cancelled, ⛔ not failure |
| the head it ran on | 4f530786ee88f2f663d8c23af9e3777bc58cb984 |
| this PR's head now | 2969466d0cf86443511ede8986995ebccef36ab7, pushed 07:42:11Z |
| CI on the current head | 11 success, 3 skipped, 17 still running, 0 red |
⇒ the dev pushed again while that run was in flight, GitHub's concurrency group cancelled it, and the aggregator closed fail-closed over the cancellation. The red belongs to a head that no longer exists on this branch.
⛔ No re-run. Re-running a superseded head measures nothing, and "flake" is not the diagnosis here — the cause is named and it is a cancellation with a cause, not an unexplained failure. The check that matters is the one on 2969466d0c, which is still running and carries no red.
Generated by Claude Code
|
ACCEPT — Gate state48 checks: 41 ⭐ The one thing no pressure made acceptable — checked first
And the file goes further than not-copying: The header line owed on every branch
Both genuine 404s survive, and they are pinned on the DISCRIMINATING field
⭐ And the test comments now name the two published pages the pins protect (
|
Contract reviewHead reviewed:
1. What the published surface gains — read off the BUILT artefact, not the source
repeatedQueryParamMessage(name: string, count: number): string
refuseRepeatedQueryParams(req: any, res: any, names: readonly string[]): booleanBoth are primitive-signature. Absent from the built 2. Does anything ELSE in the diff widen a published surface?
⇒ one declared widening, nothing undeclared. ⛔ 3. The premise that bent, and why it does not change the verdictTriage's premise ② has two halves. The second holds (§1). The first — 「导出那两个符号就够」 — bends: only Verdict on the open question — A, and the reason is jurisdictional, not aesthetic
A. ⛔ B would narrow a ruling, and this seat does not narrow rulings; that is the same rule that made the dev hand it up. The footgun B was reaching for is already answered where it does damage — the export site carries a GateThe clause-② declaration is legible and both carriers agree ( Reviewed-by: Generated by Claude Code |
⛔
|
| step | act | reading |
|---|---|---|
| 1 | POST …/ccr/convert_to_draft |
{"draft":true} ⇒ timeline gains convert_to_draft 08:33:57Z and removed_from_merge_queue 08:34:02Z — the dequeue actually happened, which is the half the guard warns is easy to skip |
| 2 | strip the carrier from the PR | labels documentation, needs:contract-review, size/m, tests, tooling → documentation, size/m, tests, tooling, read back |
| 3 | verify both carriers | card #17672 → domain:cli, pm:dispatched, priority:p2 (already clear); PR → clear ⇒ both |
| 4 | check-clause2-carriers --pair 17815 |
✓ "the clause-② declaration is readable in the fixed spelling and both carriers agree, and a review of record names this head" |
| 5 | re-enqueue | ready_for_review 08:34:22Z → added_to_merge_queue 08:34:23Z |
⇒ the PR is back in the queue with both carriers clear and a verdict naming this head. ⛔ Nothing about the diff changed, ⛔ no check was edited, and ⛔ the review was not re-run to paper over the sequence.
What this red was NOT
⛔ Not a failure of this PR's code — the same merge group's CI, Spec Liveness Check and Closing-Target Claim Guard all passed, and CI on the head is 41 green / 7 skipped / 0 red. ⛔ Not the fail-closed-over-cancelled aggregator pattern, which is a different red with a different cause (that one was diagnosed separately at 5644530293). This one was a real refusal of a real violation, and reading the job log rather than the check name is what told the two apart.
Generated by Claude Code
Fixes #17672
Clause-②: yes
GET /api/v1/packages/:id?version=a&version=banswered404. This repo already had a landed answer for exactly that condition on exactly that route —400 VALIDATION_ERRORin the ADR-0112 nested body — and one implementation of it. What blocked PR #17668 from calling it was reachability:@objectstack/restdeclares a single export subpath andquery-multiplicity.tswas not on it.Triage ruled route 1 at
5643017415and refused routes 2 and 3. This branch executes route 1. ⛔ No copy of the rule was made inpackages/runtime, under any framing.1. The repro, driven first — one host, four requests
Before any edit, on the branch point
310760d22, through the realHttpDispatcherand a realSchemaRegistry:The card said the two answers were indistinguishable by status. Measured, they were indistinguishable by
error.codeas well — all three refusals are404 RESOURCE_NOT_FOUND, so a client branching on either field could not separate a request-shape error from a not-found.After, same harness, same four requests:
The full body is the dispatcher's declared envelope,
{ success: false, error: { code: 'VALIDATION_ERROR', message, httpStatus: 400 } }.VALIDATION_ERRORis derived bybuildApiErrorfromstandardErrorCodeForHttpStatus(400), the standard catalog's member for 400. ⛔ Nothing inpackages/specmoves.2.⚠️ Premise ② was measured, and it does not hold in the reading the route-1 wording implies
Triage's named premise: 「导出那两个符号就够,且导出它们不会连带把该模块的内部面一起拉上公开面」.
Half two holds. Built
packages/rest/dist/index.d.tsgains exactly two declarations, both of primitives:SingleQueryRead,readSingleQueryValue,FILTER_SLOT_QUERY_PARAMS,assertFilterParamSuppliedOnceandrepeatedFilterParamMessageare all absent from the published surface (grep over the built.d.ts). The module's internal surface is not dragged along.Half one is where the premise bends. Nothing MORE than those two symbols is needed — so the hard fork's trigger (「若可达性需要导出的不止那两个符号」) does not fire, and this branch does not stop. But only ONE of the two is callable at this door, and the reason is structural rather than stylistic:
refuseRepeatedQueryParamswrites the answer itself,res.status(400).json(…). A dispatcher domain has nores— it RETURNS{ handled, response }, and every error body on that surface is built bybuildApiError. Driven, with the gate given a capturingres:So the gate's body is not a legal body on this wire surface. The message function is the portable half; the gate is not. The door therefore calls
repeatedQueryParamMessageand builds its body throughdeps.error(msg, 400).refuseRepeatedQueryParamsis exported anyway, and that is deliberate rather than accidental. The ruling names both symbols, and narrowing a ruling is a report rather than a dev's decision — so both are published, the barrel entry records which half travels and which does not, and this paragraph hands the question to the at-tier contract review that this PR waits on. If review decides a published symbol with no cross-package consumer should not ship, droppingrefuseRepeatedQueryParamsfrom the barrel is a one-line change that touches nothing else in this diff.Clause-②: yesis ⛔ NOT downgraded:packages/rest's published surface widens on either outcome.3. The answer comes from the shared implementation — a control that can fail
The committed pin asserts the wire text BY DERIVATION (
toBe(repeatedQueryParamMessage('version', 2))), so it cannot fail when the shared sentence moves. The falsifiable control is an ablation, run from the committed state:packages/rest/src/query-multiplicity.ts, replacingthis endpoint will not choose between conflicting values.withABLATION-17672 the shared sentence moved.grep -cof the removed text1 → 0, of the injected text0 → 1;git hash-objectof the pathe6887a55… → 1482e095…, against theHEADblob hashe6887a55….packages/rest's own literal pin,rest-server-query-multiplicity.test.ts:15 failed | 17 passed, exit 1.message="The \"version\" query parameter was supplied 2 times. Supply it at most once — ABLATION-17672 the shared sentence moved.", and a temporary literal assertion on the door went red. ⇒ the door's wire text follows the shared module's source.13 passed, exit 0, which is the behaviour it is written for.git checkout HEAD -- ABSOLUTE-PATHfrom anEXIT INT TERMtrap;git diff HEADfor that path empty afterwards.packages/runtime/vitest.config.tsaliases@objectstack/restto../rest/src/index.ts, so this suite resolves the specifier to SOURCE.scripts/ablation-dist-preflight.mjsis the preflight for the dist-resolved case and does not apply here — the ablation reaching the door with no build is itself the proof that source is the resolved path.4.⚠️ A pin was changed deliberately
packages/runtime/src/domains/packages-get-version-scope.test.ts§4.As written for #17416 it asserted only⚠️ §4 asserts the DEFECT CLASS is closed (no
status !== 200and that no installed row rode out, and its own docblock said the status was left out on purpose: 「200with the installed row), deliberately not the exact status … So this pin stays green when that rule lands here.」404: measured, it did not. It was written loose enough to survive this fix, and it would have stayed green through it. ⛔ That is not a reason to leave it — a door whose contract is400 VALIDATION_ERRORshould have a pin that says so, and a loose pin that survives both answers cannot tell a reader which one is the contract.So §4 now pins the END state: the status, the code, the ADR-0112 nested body, and the message by derivation from
@objectstack/rest's function. §5 is new and pins the card's actual criterion — the three refusals read400 VALIDATION_ERROR/404 RESOURCE_NOT_FOUND/404 RESOURCE_NOT_FOUNDand are mutually distinguishable. ⛔ The404it replaces is not treated as existing contract: it was the interim answer of an unreachable rule.5. ⭐ The one line owed on every branch
packages/rest/src/query-multiplicity.tssaid the dispatcher's/packagesdomain "reads noversion" — false since #17668 landed, and load-bearing, since it is part of why the rule needs only one home. The paragraph now states that the domain does read it, that the rule's home neither moved nor split, and that it serves two doors with one message. The⚠️clause under it records which half is portable across a package boundary, so the next author at a dispatcher domain does not reach for the gate.6. Two published pages, re-read on this branch — one was already right, one is a live condition
A docs-drift advisory on this PR listed 21 pages. ⛔ Its row count is not an instruction to edit anything; two rows actually bear on this diff, and both were re-read here rather than taken on trust.
⭐
content/docs/api/client-sdk.mdxalready documented the end state. Its error table reads:⇒ a published page has been giving
400 VALIDATION_ERRORfor a repeated query parameter while this door answered404 RESOURCE_NOT_FOUNDfor exactly that condition. So this is not only a landed-precedent argument: the door contradicted a page the platform ships. ⛔ Nothing to edit there — the code now agrees with a page that was already correct, which is the opposite of drift.content/docs/kernel/contracts/metadata-service.mdxis a live condition, not a mention. Its route table says ofGET /api/v1/packages/:id: "a missing id answers404 RESOURCE_NOT_FOUND, messagePackage 'ID' not found". That sentence stays true only if the missing-id refusal survives with its exact message. §1 asserts both halves —toBeon the message and onRESOURCE_NOT_FOUND— and the new ordering pin asserts the message again with a repeated?version=riding along, so moving either turns a test red instead of silently falsifying that page. The test comment names the page, so the pin says what it protects. ⛔ No docs edit is owed.⛔⚠️ And one gap carried forward rather than papered over: the advisory reports that
content/docs/releases/**is read-only and untouched.packages/rest/src/index.tsyielded no anchor, so pages documenting what this PR exported there are invisible to that run — "not listed" is not "not affected" for that file, and nothing here should be read as coverage of it.7. Not a breaking change, measured rather than assumed
The
404being replaced was introduced by #17668 (1a25f4a8d).git merge-base --is-ancestor 1a25f4a8d '@objectstack/runtime@17.4.0'exits 1; two control commits from that tag's own history answer exit 0 on the same predicate, in a checkout wheregit rev-parse --is-shallow-repositoryisfalse. ⇒ it has never been published, so no released consumer can have branched on it. Changeset:@objectstack/restminor (published exports),@objectstack/runtimepatch.8. Verification
Run on the final commit,
git rev-parse --short HEAD=2969466d0— the branch is4f530786e(a merge oforigin/main43df8db3a, which touches only CI workflow and script paths, none overlapping this diff) plus one comment-only commit naming the two pages in §6. The whole table below was re-run on2969466d0, ⛔ not carried over from the earlier head.pnpm --filter @objectstack/rest testpnpm --filter @objectstack/runtime testpnpm --filter @objectstack/runtime test:repopnpm --filter @objectstack/rest --filter @objectstack/runtime typecheckcheck:test-typecheckpnpm --filter '@objectstack/runtime^...' build+ both changed packagespnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*'pnpm lint(repo-wide,eslint . --no-inline-config)scripts/pm/dispatch-gates.mjs)2969466d0(family set identical), reconciled with--rancarrying an exit code per family, all 0On the earlier head two gates first answered
exit 3—PREREQUISITE NOT MET, which each states in its own text is ⛔ not a pass and not a finding.check:dual-build-cjs-loadsneeded a whole-workspacedistand went to 0 after the full build;check:type-check-debtOOM'd under aNODE_OPTIONSheap cap tighter than the one it declares for itself, and went to 0 re-run without it. Both are plain 0 in the2969466d0sweep above (5 ledger entries re-measured, 55 raw tsc errors, none above its recorded number). Also run, because the derivation flags their rosters as sitting under a changed path:check:error-status-conformance,check:error-code-casing,check:route-ledger-census,check:authz-resolver,check:published-readme-exports— all exit 0.Acceptance notes
Filed — #17813:
metadata-protocol'srepeatedQueryParamErrordocblock says its wording ispackages/rest's "verbatim so a caller … is told the same thing twice", and driven, the two sentences differ at the first quoted character (The "top"vsThe 'top'). Found because this card made the message importable for the first time; ⛔ not fixable here —@objectstack/metadata-protocoldoes not depend on@objectstack/restin either direction that would allow the import, so it is a layering decision. That card is not addressed by this PR and remains open.noted, not filed — the multiplicity check sits AFTER the id lookup, so
GET /packages/UNKNOWN-ID?version=a&version=bstill answers404 Package '…' not foundrather than400. That is #17416's deliberate ordering ("only a package that IS here can be at the wrong version") and this card moved the status of a refusal, not the order of two refusals. It is now pinned explicitly in §1 rather than left incidental, so a future re-ordering is a visible decision. Successor: the contract review on this PR, or whoever converts the/packageslane to a closed parameter set — which is #17667's territory, currentlyneeds-user-decision.noted, not filed —
readRequestedVersionin the domain still implements the count-not-shape predicate thatreadSingleQueryValueimplements inpackages/rest(length > 1refuses,length === 1unwraps,length === 0is absent). Importing the predicate too would be a third exported symbol plus its result type, which is exactly the widening the fork instruction guards; the door's copy also carries thelatestsentinel, which is domain semantics the shared reader has no business knowing. Left as is, with the shared MESSAGE — the drift surface the module's header actually names — imported. Successor: none identified; this is a note for the reviewer of this diff and nothing else is queued against these lines.Generated by Claude Code