Skip to content

Commit 8bc056f

Browse files
committed
fix(runtime): GET /packages/:id honours ?version= instead of ignoring it (#17416)
The route accepted `?version=` from the SDK and the only surface serving it never read the parameter: the caller got `200` with the installed row and nothing in the status, headers or body told that apart from a version-scoped read. The handler that honoured it went with the REST twin in #14503/#16628 and this dispatcher domain never had the read to inherit. Request-side only — the response shape is untouched (#12034 owns it), so the route still answers with exactly one body shape. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com>
1 parent d46deba commit 8bc056f

1 file changed

Lines changed: 123 additions & 1 deletion

File tree

‎packages/runtime/src/domains/packages.ts‎

Lines changed: 123 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,6 +531,102 @@ function withWritableVerdict<T extends { manifest?: { id?: unknown }; id?: unkno
531531
return { ...row, writable: isWritablePackage(engine, id) };
532532
}
533533

534+
/**
535+
* What `?version=` on `GET /packages/:id` asked for (#17416).
536+
*
537+
* ## The defect this exists to close
538+
*
539+
* The route ACCEPTED `?version=` and no surface serving it ever read the
540+
* parameter. `ScopedEnvironmentClient.packages.get(id, version)`
541+
* (`packages/client/src/index.ts`) declares `version?: string` and appends it,
542+
* so the caller was answered `200` with the INSTALLED row and nothing in the
543+
* status, headers or body told it apart from a version-scoped read that
544+
* actually happened. The handler that honoured it — the REST registrar's twin
545+
* of this route — went with the duplicate response shape in #14503 / #16628,
546+
* and this dispatcher domain never had that read to inherit.
547+
*
548+
* Maintainer ruling on #17416: honour the parameter, or refuse it so the caller
549+
* knows. ⛔ Silently ignoring is not a third option. This is the honouring
550+
* half, and it is a request-side change only: the response shape is untouched
551+
* (that surface is #12034's, bound separately), so one route still answers with
552+
* exactly one body shape.
553+
*
554+
* ## `latest` and "absent" are the SAME request, deliberately
555+
*
556+
* The deleted handler read `requested.value || 'latest'` and its store treated
557+
* `latest` as `ORDER BY created_at DESC LIMIT 1` — so "no version" and
558+
* "`?version=latest`" named one request there. They name one request here too:
559+
* the registry holds exactly one row per package id and it is the installed,
560+
* newest one. ⛔ Not a new sentinel invented at this door — continuity with the
561+
* contract the deleted door published.
562+
*
563+
* ## Why a repeated parameter is NOT resolved here
564+
*
565+
* `IHttpRequest.query` is `Record<string, string | string[]>` and the array arm
566+
* is live on every adapter this repo ships, so `?version=a&version=b` reaches
567+
* this door as `['a','b']` — a well-formed request carrying two conflicting
568+
* intents. Picking one silently is a wrong answer delivered as a success, which
569+
* is the defect class this card is about, so it is not done. The repo's ONE
570+
* rule for this condition answers `400 VALIDATION_ERROR`
571+
* (`refuseRepeatedQueryParams` / `repeatedQueryParamMessage` in
572+
* `packages/rest/src/query-multiplicity.ts`, whose header is the authority) and
573+
* that is the right end state for this door as well. ⛔ It is NOT restated
574+
* here: that module is not exported from `@objectstack/rest`'s barrel, so
575+
* calling it from this package would mean widening another package's public
576+
* surface, and a second copy of the rule with a second message is the drift its
577+
* own header forbids. Until the rule is reachable, this door says what it saw
578+
* and names no version — it does not answer `200` with the installed row.
579+
*
580+
* A one-element array is one occurrence encoded differently by an adapter and
581+
* is unwrapped, per that same rule's stated semantics.
582+
*/
583+
type RequestedVersion =
584+
| { readonly kind: 'unscoped' }
585+
| { readonly kind: 'exact'; readonly value: string }
586+
| { readonly kind: 'repeated'; readonly count: number };
587+
588+
function readRequestedVersion(raw: unknown): RequestedVersion {
589+
if (Array.isArray(raw)) {
590+
if (raw.length > 1) return { kind: 'repeated', count: raw.length };
591+
return readRequestedVersion(raw[0]);
592+
}
593+
if (typeof raw !== 'string') return { kind: 'unscoped' };
594+
// `latest` is the installed row — see this type's header.
595+
if (raw === 'latest') return { kind: 'unscoped' };
596+
return { kind: 'exact', value: raw };
597+
}
598+
599+
/** The one sentence this door answers a repeated `?version=` with. */
600+
function repeatedVersionMessage(id: string, count: number): string {
601+
return `Package '${id}' — the "version" query parameter was supplied ${count} times, `
602+
+ 'so this read names no single version. Supply it at most once.';
603+
}
604+
605+
/**
606+
* The version the registry's row for a package IS (#17416).
607+
*
608+
* `manifest.version` is read FIRST because it is the field the producer
609+
* actually writes: `SchemaRegistry.installPackage` stores a projection of the
610+
* manifest, and nothing in the registry populates `installedVersion` — which
611+
* `packages/spec/src/kernel/package-registry.zod.ts` declares `.optional()` and
612+
* documents as a mirror («Mirrors manifest.version for quick access»). So the
613+
* mirror is a fallback for a row some other producer filled in, ⛔ never a
614+
* tolerated alias for an off-spec spelling: both keys are declared, and a row
615+
* where they disagree is a producer defect this door cannot repair.
616+
*
617+
* Comparison is exact string equality — the same predicate the durable store
618+
* uses (`AND version = ?` in `@objectstack/service-package`), so the two
619+
* answers about "is this package at version v" cannot drift into semver range
620+
* semantics at one of them.
621+
*/
622+
function installedVersionOf(pkg: unknown): string | undefined {
623+
const src = pkg as { manifest?: { version?: unknown }; installedVersion?: unknown } | null;
624+
const fromManifest = src?.manifest?.version;
625+
if (typeof fromManifest === 'string' && fromManifest !== '') return fromManifest;
626+
const mirror = src?.installedVersion;
627+
return typeof mirror === 'string' && mirror !== '' ? mirror : undefined;
628+
}
629+
534630
export async function handlePackagesRequest(deps: DomainHandlerDeps, path: string, method: string, body: any, query: any, _context: HttpProtocolContext): Promise<HttpDispatcherResult> {
535631
const m = method.toUpperCase();
536632

@@ -1207,12 +1303,38 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin
12071303
}
12081304
}
12091305

1210-
// GET /packages/:id → get package
1306+
// GET /packages/:id[?version=] → get package, scoped to a version when
1307+
// one is asked for (#17416 — see `readRequestedVersion`).
12111308
if (parts.length === 1 && m === 'GET') {
12121309
const denied = requireReadCapability(deps, _context); if (denied) return denied;
12131310
const id = decodeURIComponent(parts[0]);
12141311
const pkg = registry.getPackage(id);
12151312
if (!pkg) return { handled: true, response: deps.error(`Package '${id}' not found`, 404) };
1313+
// [#17416] The version scope is applied AFTER the id lookup, so an
1314+
// id this registry does not hold keeps answering the wording
1315+
// `packages-single-door.test.ts` pins — `Package '<id>' not found`,
1316+
// whether or not `?version=` rode along. Only a package that IS
1317+
// here can be at the wrong version.
1318+
const requested = readRequestedVersion(query?.version);
1319+
if (requested.kind === 'repeated') {
1320+
return {
1321+
handled: true,
1322+
response: deps.error(repeatedVersionMessage(id, requested.count), 404),
1323+
};
1324+
}
1325+
if (requested.kind === 'exact') {
1326+
const present = installedVersionOf(pkg);
1327+
if (present !== requested.value) {
1328+
return {
1329+
handled: true,
1330+
response: deps.error(
1331+
`Package '${id}' version '${requested.value}' not found`
1332+
+ (present ? ` — installed version is '${present}'` : ''),
1333+
404,
1334+
),
1335+
};
1336+
}
1337+
}
12161338
// [#14375] Same verdict, same predicate as the list door — and
12171339
// [#14309] the same project-then-stamp order, for the same reason.
12181340
return {

0 commit comments

Comments
 (0)