Skip to content

Commit 1a25f4a

Browse files
claude[bot]claude
andauthored
fix(runtime): GET /api/v1/packages/:id honours ?version= instead of silently ignoring it (#17668)
* 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> * test(runtime): pin the ?version= scope pair on GET /packages/:id (#17416) The discriminating pin (status + the field that names the version, and no package row on the refusal) beside the lit control (the unversioned read still answers exactly as it did). A green on only the new path would also pass with the ordinary read broken, and this door is the route's single implementation. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com> * chore: changeset for the ?version= scope on GET /packages/:id (#17416) Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com> * test(runtime): compare ?version= response bodies against ONE installed row (#17416) CI's Test Core (2/6) failed §3 on a 1 ms timestamp difference. Cause measured: `get()` built a fresh host per call, so the package was installed twice, and `SchemaRegistry.installPackage` stamps `installedAt`/`updatedAt` from one `new Date()` per install. Two hosts therefore hold two rows whose stamps differ whenever the installs straddle a millisecond boundary — a flake, not a product defect: nothing on the read path reads a clock (`toPackageResponse` is an allowlist copy, `withWritableVerdict` a spread, and `success()` adds no timestamp), so one install cannot produce two answers. Every case that compares two response bodies now issues both requests through one host, which is the shape §2's byte-identical pin already had and the reason it stayed green in the same shard. §1's pair moves onto one host too: the criterion is «the SAME request with and without `?version=`», and two hosts let a difference come from the rows instead of from the parameter. §4's array-vs-string pin carried the identical latent flake and is repaired with it. Whole-body `toEqual` is preserved everywhere — the stamps stay in the comparison, which is what makes "the same request" mean the same response. Claude-Session: https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0fb6f97 commit 1a25f4a

3 files changed

Lines changed: 381 additions & 1 deletion

File tree

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
'@objectstack/runtime': minor
3+
---
4+
5+
fix(runtime): `GET /api/v1/packages/:id` honours `?version=` instead of silently ignoring it (#17416)
6+
7+
The route accepted a `?version=` query parameter and the only surface serving it
8+
never read the parameter. A caller asking for a version that is not installed
9+
was answered `200` with the **installed** row, and nothing in the status,
10+
headers or body distinguished that from a version-scoped read that actually
11+
happened.
12+
13+
The parameter is not hypothetical traffic: `ScopedEnvironmentClient.packages.get`
14+
(`@objectstack/client`) declares `version?: string` and appends it, so the SDK
15+
has been sending a parameter the runtime dropped. The handler that honoured it
16+
— the REST registrar's twin of this route — was removed with the duplicate
17+
response shape, and the dispatcher's `/packages` domain never had that read to
18+
inherit.
19+
20+
```
21+
FROM GET /api/v1/packages/com.acme.crm?version=99.0.0 (1.0.0 installed)
22+
-> 200 { data: { manifest: { version: "1.0.0" }, … } }
23+
24+
TO GET /api/v1/packages/com.acme.crm?version=99.0.0
25+
-> 404 { error: { message: "Package 'com.acme.crm' version '99.0.0' not
26+
found — installed version is '1.0.0'" } }
27+
```
28+
29+
**What does not change.** The unversioned read is untouched, down to the row and
30+
the writability verdict it stamps — pinned as the lit control beside the new
31+
assertions, because a green on only the scoped path would also pass with the
32+
ordinary read broken. `?version=` naming the installed version is served
33+
exactly as the unversioned read is, and so is `?version=latest`: the deleted
34+
handler read `requested.value || 'latest'` and its store resolved `latest` to
35+
the newest row, so "no version" and "`latest`" named one request there and name
36+
one request here. An id the registry does not hold keeps its existing 404
37+
wording whether or not `?version=` rode along — a package that is not installed
38+
cannot be at the wrong version.
39+
40+
**This is request-side only.** The response shape is not touched, so the route
41+
still answers with exactly one body shape; comparison is exact string equality
42+
on the version, the same predicate the durable package store uses (`AND version
43+
= ?`), so the two answers to "is this package at version v" cannot drift into
44+
semver-range semantics at one of them.
45+
46+
A repeated `?version=a&version=b` is no longer resolved by silently choosing
47+
one — it is answered with a refusal naming what was seen. The repo's one rule
48+
for a repeated single-valued parameter answers `400 VALIDATION_ERROR` and is
49+
the right end state for this door too; it is not restated here, because the
50+
helper that owns that rule and its message is not exported from
51+
`@objectstack/rest`.
Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `GET /api/v1/packages/:id?version=` is a VERSION-SCOPED read, and its answer
5+
* is distinguishable from the unversioned one (#17416).
6+
*
7+
* ## The defect
8+
*
9+
* The route accepted `?version=` and the only surface serving it never read the
10+
* parameter. `ScopedEnvironmentClient.packages.get(id, version)` declares
11+
* `version?: string` and appends it, so `?version=99.0.0` against an installed
12+
* `1.0.0` was answered `200` with the `1.0.0` row — and no status, header or
13+
* field told the caller which of the two reads it got. The handler that
14+
* honoured it (with the store predicate `AND version = ?`) went with the REST
15+
* twin in #14503 / #16628; this dispatcher domain never had it to inherit.
16+
*
17+
* ## What is pinned, and why each half is here
18+
*
19+
* The card's acceptance criterion is a PAIR, because either half alone passes
20+
* for the wrong reason:
21+
*
22+
* - **§1 the discriminating pin** — the same request with and without
23+
* `?version=<a version that is not installed>` must not produce the same
24+
* response. Asserted as `status` PLUS the discriminating field (the error
25+
* message naming the version, and the body carrying no package row), ⛔
26+
* never as "it returned 200": that reading is precisely what hid this
27+
* defect for the whole of its life.
28+
* - **§2 the lit control** — the unversioned read still answers exactly as it
29+
* did, down to the row it serves. A green that only exercised the new
30+
* scoped path would also be green if the ordinary read were broken, and
31+
* this door is the single implementation of the route.
32+
*
33+
* §3 pins the two requests that mean "the installed row" — no parameter and
34+
* `?version=latest` — as ONE request, which is the contract the deleted
35+
* handler published (`requested.value || 'latest'`). §4 pins the repeated
36+
* parameter as not-a-silent-success: `?version=a&version=b` carries two
37+
* conflicting intents, and the answer names what it saw rather than choosing.
38+
* ⚠️ §4 asserts the DEFECT CLASS is closed (no `200` with the installed row),
39+
* deliberately not the exact status, because the repo's one rule for a repeated
40+
* single-valued parameter answers `400 VALIDATION_ERROR` and is unreachable
41+
* from this package today — see `readRequestedVersion`'s header in
42+
* `packages.ts`. So this pin stays green when that rule lands here.
43+
*
44+
* ## The harness
45+
*
46+
* A real {@link SchemaRegistry} behind the real {@link HttpDispatcher}, the way
47+
* `packages-writable-verdict.test.ts` and `packages-readonly-gate.test.ts` next
48+
* door do it — so the rows under test are what `installPackage` actually
49+
* produces and the answers are the ones the composed door gives.
50+
*/
51+
52+
import { describe, it, expect } from 'vitest';
53+
import { SchemaRegistry } from '@objectstack/objectql';
54+
import { HttpDispatcher } from '../http-dispatcher.js';
55+
56+
const PKG = 'com.acme.crm';
57+
/** The version actually installed — the row every read below can legitimately serve. */
58+
const INSTALLED = '1.0.0';
59+
/** The card's own repro value: a version this registry does not hold. */
60+
const ABSENT = '99.0.0';
61+
62+
function make() {
63+
const registry = new SchemaRegistry({ logLevel: 'silent' } as any);
64+
registry.installPackage({ id: PKG, name: PKG, version: INSTALLED, scope: 'project', type: 'app' } as any);
65+
const objectql = { registry, manifests: new Map<string, any>() };
66+
const kernel: any = {
67+
context: { getService: (name: string) => (name === 'objectql' ? objectql : null) },
68+
};
69+
return new HttpDispatcher(kernel);
70+
}
71+
72+
/** Holds the ADR-0106 D4 read set; the caller gate is not this file's subject. */
73+
const reader = (): any => ({
74+
request: {},
75+
environmentId: 'pkg-get-version-scope-test',
76+
executionContext: { userId: 'u_admin', isSystem: false, systemPermissions: ['manage_metadata', 'studio.access'] },
77+
});
78+
79+
/**
80+
* `GET /packages/:id` against an EXISTING host.
81+
*
82+
* ⛔ Every case that compares two RESPONSE BODIES must issue both requests
83+
* through this, against ONE `make()`. `SchemaRegistry.installPackage` stamps
84+
* `installedAt` and `updatedAt` from a single `new Date()` per install
85+
* (`packages/objectql/src/registry.ts`), and both are declared record fields
86+
* that `toPackageResponse` carries to the wire. So two hosts hold two rows
87+
* whose stamps differ whenever the installs straddle a millisecond boundary,
88+
* and a whole-body `toEqual` between them fails on the clock rather than on
89+
* anything this door did — a flake that passes on a re-run and comes back.
90+
*
91+
* One host makes it deterministic rather than merely likelier: both responses
92+
* are projections of ONE row, so there is no second install and no second
93+
* clock read to disagree. Nothing on the read path reads a clock at all —
94+
* neither `toPackageResponse` (an allowlist copy) nor `withWritableVerdict` (a
95+
* spread) nor the dispatcher's `success()` envelope — so with one install the
96+
* stamps cannot move, at any scheduling.
97+
*
98+
* ⛔ The repair for such a failure is this shape, ⛔ never dropping the two
99+
* stamps out of the comparison: whole-body equality is what makes «the same
100+
* request» mean the same RESPONSE rather than the same status.
101+
*/
102+
async function read(dispatcher: HttpDispatcher, query: Record<string, unknown> | undefined) {
103+
const r = await dispatcher.handlePackages(`/${PKG}`, 'GET', undefined, query, reader());
104+
return { status: r.response?.status ?? 200, body: r.response?.body };
105+
}
106+
107+
/** One request on a host of its own — for the cases that compare against no other body. */
108+
async function get(query: Record<string, unknown>) {
109+
return read(make(), query);
110+
}
111+
112+
describe('#17416 GET /packages/:id — ?version= scopes the read', () => {
113+
describe('§1 the discriminating pin — with and without ?version= are not the same answer', () => {
114+
it('a non-installed ?version= is NOT answered with the installed row', async () => {
115+
// ONE host: the criterion is «the SAME request with and without
116+
// `?version=`», so both answers have to be about the same row —
117+
// two hosts would let a difference come from the rows instead.
118+
const host = make();
119+
const scoped = await read(host, { version: ABSENT });
120+
const unscoped = await read(host, {});
121+
122+
// The discriminating field, not the status alone.
123+
expect(scoped.status).toBe(404);
124+
expect(scoped.body?.error?.message).toContain(ABSENT);
125+
expect(scoped.body?.error?.message).toContain(INSTALLED);
126+
// ⛔ No package row rode out on the refusal.
127+
expect(scoped.body?.data).toBeUndefined();
128+
129+
// ...and the SAME request without the parameter is a different answer.
130+
expect(unscoped.status).toBe(200);
131+
expect(unscoped.body?.data?.manifest?.version).toBe(INSTALLED);
132+
expect(scoped.status).not.toBe(unscoped.status);
133+
expect(scoped.body).not.toEqual(unscoped.body);
134+
});
135+
136+
it('the installed version IS served when it is the one asked for', async () => {
137+
const r = await get({ version: INSTALLED });
138+
expect(r.status).toBe(200);
139+
expect(r.body?.data?.manifest?.id).toBe(PKG);
140+
expect(r.body?.data?.manifest?.version).toBe(INSTALLED);
141+
});
142+
143+
it('an id this registry does not hold keeps the wording packages-single-door pins', async () => {
144+
const dispatcher = make();
145+
const r = await dispatcher.handlePackages('/com.absent.pkg', 'GET', undefined, { version: ABSENT }, reader());
146+
expect(r.response?.status).toBe(404);
147+
// ⛔ The id 404 is NOT re-worded by the version scope: a package that
148+
// is not here cannot be "at the wrong version".
149+
expect(r.response?.body?.error?.message).toBe(`Package 'com.absent.pkg' not found`);
150+
});
151+
});
152+
153+
describe('§2 the lit control — the unversioned read is untouched', () => {
154+
it('answers 200 with the installed row and its writability verdict', async () => {
155+
const r = await get({});
156+
expect(r.status).toBe(200);
157+
expect(r.body?.success).toBe(true);
158+
expect(r.body?.data?.manifest?.id).toBe(PKG);
159+
expect(r.body?.data?.manifest?.version).toBe(INSTALLED);
160+
// The #14375 verdict the door has always stamped, still stamped.
161+
expect(r.body?.data?.writable).toBe(true);
162+
});
163+
164+
it('is byte-identical to the read with no query object at all', async () => {
165+
const host = make();
166+
const withEmpty = await read(host, {});
167+
const withNone = await read(host, undefined);
168+
expect(withNone.status).toBe(200);
169+
expect(withNone.body).toEqual(withEmpty.body);
170+
});
171+
});
172+
173+
describe('§3 `latest` and absent name the SAME request', () => {
174+
it('?version=latest serves the installed row, exactly as no parameter does', async () => {
175+
const host = make();
176+
const latest = await read(host, { version: 'latest' });
177+
const unscoped = await read(host, {});
178+
expect(latest.status).toBe(200);
179+
expect(latest.body).toEqual(unscoped.body);
180+
});
181+
});
182+
183+
describe('§4 a repeated ?version= is not resolved silently', () => {
184+
it('two conflicting values are not answered 200 with the installed row', async () => {
185+
const r = await get({ version: [ABSENT, INSTALLED] });
186+
// The defect class: a success carrying a row the caller did not ask for.
187+
expect(r.status).not.toBe(200);
188+
expect(r.body?.data).toBeUndefined();
189+
// It says what it saw rather than choosing one of the two.
190+
expect(r.body?.error?.message).toContain('supplied 2 times');
191+
});
192+
193+
it('ONE occurrence encoded as a one-element array is one occurrence', async () => {
194+
const host = make();
195+
const arr = await read(host, { version: [INSTALLED] });
196+
const str = await read(host, { version: INSTALLED });
197+
expect(arr.status).toBe(200);
198+
expect(arr.body).toEqual(str.body);
199+
});
200+
201+
it('a one-element array naming an absent version still refuses', async () => {
202+
const r = await get({ version: [ABSENT] });
203+
expect(r.status).toBe(404);
204+
expect(r.body?.error?.message).toContain(ABSENT);
205+
});
206+
});
207+
});

0 commit comments

Comments
 (0)