fix(rest): refuse a repeated ?version= on GET/DELETE /packages/:id instead of handing the array to PackageService (#6307)

[os-project-manager]

Fixes#6307

What was wrong

IHttpRequest.query is declared Record<string, string | string[]> (packages/spec/src/contracts/http-server.ts) — a repeated query parameter arrives as an array. Both /api/v1/packages/:id handlers read it as a string and handed it to PackageService.get/delete, whose parameter is version?: string.

This is not "adding tolerance for off-spec input". The contract already declared the array shape. The consumer simply never handled a shape it was told to expect.

Observation-class: no user hits this today. It takes a client that repeats the parameter, and nothing in this repo or its SDK does.

Premise, measured on origin/main before writing any fix

Not inherited on faith — driven through the real registrar:

[GET] packageService.get('com.acme.crm', ["1.0.0","2.0.0"]) isArray = true
[DELETE] packageService.delete('com.acme.crm', ["1.0.0","2.0.0"]) isArray = true
[DELETE] protocol.deletePackage called? false
[DELETE] answered: 200 {"success":true,"data":{"message":"Deleted com.acme.crm@1.0.0,2.0.0"}}
[DELETE control, no version] protocol.deletePackage called? true

The DELETE line is the sharp one and it reproduces exactly as the card claims. if (!version && typeof options.protocol?.deletePackage === 'function') is what gates the full uninstall (#2747: the package's metadata rows, the durable sys_packages record, and the registered data-plane cleanups — plugin-security revoking its permission sets and bindings). Any truthy version skips it. So a repeated parameter silently narrowed the scope of the operation on a destructive verb, asked the durable registry to delete a version literally named 1.0.0,2.0.0, and answered 200.

And what the adapters actually produce

The fix's premise rests on the array being real, so both IHttpServer implementations were driven over a real socket with ?version=1.0.0&version=2.0.0&single=9:

adapter	req.query
NodeHttpServer (packages/qa/http-conformance, node:http)	{"version":["1.0.0","2.0.0"],"single":"9"}
HonoHttpServer (plugin-hono-server, production)	{"version":"1.0.0","single":"9"}

The array arm is produced by a real in-repo adapter. Hono collapses a repeat to the first value before a handler sees it — both are contract-legal (the union permits either), which is precisely why the consumer must handle the declared shape rather than depend on which server booted. Filed as a separate finding (#6878) rather than widened into this PR.

The fix — refuse the ambiguity, do not resolve it

?version=a&version=b is a well-formed request carrying two conflicting intents. Picking one silently is a wrong answer delivered as a 200, and on a destructive verb it silently changes what the operation does. Both verbs now answer:

400 { "success": false, "error": { "code": "VALIDATION_ERROR",
"message": "The \"version\" query parameter was supplied 2 times. Supply it at most
once — this endpoint will not choose between conflicting values." } }

The same rule on both verbs — one parameter, one answer; two different answers would be a new inconsistency. The tests assert the two bodies are toEqual, not merely both-400.

The code was established, not invented. Per ADR-0112 and this module's own docstring rule, generic conditions reuse the standard catalog rather than registering a synonym. "This request contradicts itself" is generic, so it is VALIDATION_ERROR — the catalog's generic validation failure, and already what HttpStatusErrorCodeMap names a bare 400. INVALID_FIELD was rejected: rest-server.ts branches on it as a record-field classifier, and reusing it here would conflate two vocabularies. MISSING_REQUIRED_FIELD (the card's tentative suggestion) is simply false — nothing is missing.

The rule is about multiplicity, not shape. The parameter may be supplied at most once. A one-element array is one occurrence encoded differently by an adapter and is accepted; an empty array is no occurrence. Two identical values are still two occurrences and are still refused — "at most one distinct value" would be a de-duplication rule no client can predict, while "supply it at most once" is checkable client-side without knowing our semantics.

The stop condition was checked, and did not trigger

The card said to stop and escalate if any real caller legitimately repeats version. Searched in-repo callers, docs and the SDK: client.packages.get builds ?version=${encodeURIComponent(version)} from a single version?: string; the CLI's package commands target the separate /cloud/packages/... surface. No caller can produce the new 400.

Verification

Nothing that works today moves. Single value, no value, and an empty ?version= are pinned byte-for-byte against the recorded pre-fix bodies, and the full-uninstall branch is pinned as still reached when no version is supplied at all.

Reverse verification — prediction written down before running: revert the source only, keep the tests, expect 9 red, direction "main resolves the ambiguity / mis-reads a one-element array where the fix refuses or unwraps".

	predicted	measured
red in package-routes-query-multiplicity.test.ts	8 of 14	8
red in package-envelope.conformance.test.ts	1 (the new case)	1
total red	9	9
tsc -p packages/rest/tsconfig.json	back to 2 × TS2345	2

Honestly excluded as evidence — green in both directions: the 6 no-regression cases in the new file (they guard against the fix moving something, which is a different job) and the 17 pre-existing envelope cases. One counting miss to record: I predicted 15 pre-existing conformance cases and there are 17 — an error about that file's size, not about the fix; the red count and its direction were exact.

Gates, on the tree merged with origin/main: pnpm lint clean, and the full check:* battery from .github/workflows/lint.yml — all 61 green. The two directly relevant to a new 4xx, check:route-envelope and check:error-code-casing, both pass. turbo run typecheck 120/120. Full @objectstack/rest suite: 1124 passed / 72 files.

Two runs first came back non-green for reasons that were not this diff, recorded so the next agent does not re-chase them: turbo run typecheck was OOM-killed (exit 137) on a box where sibling agents were building concurrently, and check:type-check-debt then refused to measure at all because that kill had left @objectstack/platform-objects without a built .d.ts. Rebuilding the closure at --concurrency=1 and re-running both gave 0 and 0. The refusal is the gate working correctly — it declines to record a number measured against an unbuilt world.

DEBT ledger (#4311), reported and not rewritten — no --lower, baseline untouched:

ledger	recorded	measured after this PR
DEBT['@objectstack/rest']	2 (code-tier 2 (TS2345))	0
TEST_DEBT['@objectstack/rest']	163	153 on main → 151 here (the new test file adds 0)

The gate reaches the same numbers on its own and names the consequence:

ℹ @objectstack/rest: DEBT records 2, and tsc now reports 0 -- graduation candidate.
Onboard it (add "typecheck": "tsc --noEmit", or drop the test exclusion, and delete
the ledger entry in the same PR). `--lower` deliberately leaves this one alone: 0 is
not a lower ceiling, it is a graduation…
ℹ @objectstack/rest: TEST_DEBT records 163, tsc now reports 151 (-12)

Those two TS2345were exactly this defect, so the package now compiles clean at zero. The graduation the gate suggests is a separate PR by its own description (it wants the ledger entry deleted and a typecheck script added in one change) and the ledger is not this PR's surface — flagged here for whoever tends #4311.

Changeset

minor on @objectstack/rest. Not patch: a request shape that used to answer 200 now answers 400, which is wire-visible. Not major: no signature changes, and nothing that can be reached from a supported client. The precedent followed is action-param-strict-unknown-keys (minor for "reject what used to be accepted"); the patch refusal changesets in the repo (analytics-filter-refusal-envelope &co.) all convert a 500 into a 400, which is a strictly weaker change than converting a 200.

Scope

packages/rest/src/package-routes.ts + its tests + the changeset. Nothing else. rest-server.ts and discovery.zod.ts (held by #6535 / #6714) untouched — confirmed by the diff.

The card's deliberate follow-on question — whether other req.query.* readers in packages/rest share this shape — was measured but not fixed here: 61 non-test readers, 9 of them guarded with typeof === 'string', the rest split between passing arrays through, String()-ing them into "a,b", and Number()-ing them into NaN. Filed as #6877 (unassigned) rather than widening this PR — and note that, unlike the two fixed here, none of those are among the frozen TS2345.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_017uFVNMmTxLpmfQYiuKM1Yx

---

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 9, 2026 3:09am

[github-actions]

📓 Docs Drift Check

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

11 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/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)
• 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)

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.