Skip to content

fix(runtime): GET /api/v1/packages/:id honours ?version= instead of silently ignoring it - #17668

Merged
os-sales merged 4 commits into
mainfrom
claude/issue-17416-packages-get-version-parameter
Sep 11, 2026
Merged

fix(runtime): GET /api/v1/packages/:id honours ?version= instead of silently ignoring it#17668
os-sales merged 4 commits into
mainfrom
claude/issue-17416-packages-get-version-parameter

Conversation

@claude

@claude claude Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Fixes #17416

GET /api/v1/packages/:id accepted a ?version= query parameter and the only surface serving the route never read it. A caller asking for a version that is not installed was answered 200 with the installed row, and nothing in the status, headers or body distinguished that from a version-scoped read that actually happened.

The maintainer ruling on the card: honour ?version=, or refuse it so the caller knows — silently ignoring is not a third option. This is the honouring half, and it needed no SDK change.

The premise held — honouring was reachable

The card's dispatch named the premise to measure first, and it measures true:

  • handlePackagesRequest already receives query and already uses it on sibling branches (query?.status, query?.type, query?.overwrite, query?.keepData), so "the handler cannot see the parameter" was false.
  • The GET-by-id branch re-located to packages/runtime/src/domains/packages.ts:1211 — unmoved since dispatch, verified against this branch's merge base rather than recalled.
  • The version-scoped read is expressible: the registry holds each package's version, and @objectstack/service-package's get(packageId, version) reads it durably with AND version = ?.

⇒ the fenced exit (refuse server-side, which would have required packages/client/src/index.ts, hard-serial behind #17454) was not taken and is not needed.

What the route answers now

request before after
GET /packages/:id 200, installed row 200, installed row — unchanged
?version=1.0.0 (the installed one) 200, installed row 200, installed row
?version=latest 200, installed row 200, installed row
?version=99.0.0 (not installed) 200, installed row 404, naming both versions
?version=a&version=b 200, installed row 404, naming the repetition
?version=… on an id that is not installed 404 Package 'ID not found' (the existing wording) same wording, unchanged

Answering the dispatch's explicit question — what happens when the requested version does not exist: 404. Never a 200 carrying the installed version, which is this card's defect one layer in. The message names the requested version and the installed one, so the caller can tell "you do not have this version" from "no such package":

404  Package 'com.acme.crm' version '99.0.0' not found — installed version is '1.0.0'

Three decisions worth reading:

Comparison is exact string equality, the same predicate the durable store uses, so the two answers to "is this package at version v" cannot drift into semver-range semantics at one of them.

Verification

Reverse verification (the fix committed first, the handler then restored to this branch's merge base and the mutation proven on disk before the run):

ON-DISK PROOF: readRequestedVersion occurrences  before=4  after=0
ON-DISK PROOF: worktree blob 5716ac29c3ec60134249aed268b1aa65c4dd75ab  base blob 5716ac29c3ec60134249aed268b1aa65c4dd75ab
  Tests  3 failed | 6 passed (9)
    AssertionError: expected 200 to be 404
    AssertionError: expected 200 not to be 200
RESTORE OK: blob 55742fe2e8452ff8e0becca515cbfe92736bbc35 matches HEAD

The direction is turns red, and the three failures print the defect's own signature — expected 200 to be 404 is the 200 that hid this for the whole of the parameter's life. The six that stay green are the lit control and the two requests that mean "the installed row", which is the control working rather than a gap. Restore proven by blob hash against HEAD, not by an exit code.

Testspackages/runtime/src/domains/packages-get-version-scope.test.ts, a real SchemaRegistry behind the real HttpDispatcher, the way the sibling door tests next door drive it:

  • §1 the discriminating pin — the same request with and without ?version= naming a version that is not installed does not produce the same response. status plus the discriminating field (the message naming both versions; no package row on the refusal), never "it returned 200".
  • §2 the lit control — the unversioned read still answers 200 with the installed row and its writable verdict, and is byte-identical to the read with no query object at all.
  • §3 latest and absent are one request. §4 a repeated parameter is not resolved silently, and a one-element array is one occurrence.
run result
pnpm --filter @objectstack/runtime exec vitest run --project local VERDICT command-exit 0 — 255 files / 3583 tests passed, re-run on 0e89a0d7
pnpm --filter @objectstack/runtime typecheck VERDICT command-exit 0 — test-layer debt ledger unmoved (27 files / 191 errors / 69 pinned)
pnpm --filter '@objectstack/runtime^...' build --concurrency=2 VERDICT command-exit 0
pnpm lint (the full union, eslint . --no-inline-config) exit 0, no findings, at 0e89a0d7

The CI red on ffdc9a5b, and why it cannot come back

Test Core (2/6) failed §3 on a 1 ms timestamp difference. Cause measured, not assumed: the fixture installed twice. get() built a fresh host per call, and SchemaRegistry.installPackage stamps installedAt/updatedAt from one new Date() per install (packages/objectql/src/registry.ts:4171), both of them declared record fields this door carries to the wire. Two hosts therefore held two rows whose stamps differ whenever the installs straddle a millisecond boundary.

The other candidate — the door re-stamping on read — is excluded mechanically: nothing on the read path reads a clock. toPackageResponse is an allowlist copy, withWritableVerdict a spread, installedVersionOf and readRequestedVersion are pure, and HttpDispatcher.success() returns { success, data, meta } with no timestamp. §2's byte-identical pin — one install, two requests, the same whole-body toEqual — passed in the same run and the same shard, so the variable separating green from red was the install count, not ?version=.

Reproduced deterministically before repairing, rather than waiting for the race: injecting a 5 ms separation between §3's two calls turned the 1-in-N flake into a certainty, with the stamp gap equal to the injected delay.

ON-DISK PROOF: injected-delay occurrences  before=0  after=1
§3 `latest` and absent name the SAME request > ?version=latest serves the installed row
-     "installedAt": "2026-09-11T10:23:10.805Z"
+     "installedAt": "2026-09-11T10:23:10.799Z"
  Tests  1 failed | 8 passed (9)      <- §2's one-install control green in the same run
RESTORE OK: blob 224c90ba matches HEAD

The repair is one install, not a frozen clock and not a narrowed assertion. Every case comparing two response bodies now issues both requests through one host — §2's shape, which is why it was already green. Whole-body toEqual is preserved everywhere and both stamps stay in the comparison: that is what makes "the same request" mean the same response rather than the same status. §1's pair moved onto one host too, which strengthens the card's own criterion — «the SAME request with and without ?version=» is about one row, 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.

It cannot drift again because there is no second install and no second clock read: both bodies are projections of ONE row. Demonstrated positively rather than by a passing re-run — injecting a 50 ms separation, ten times what made the old shape fail with certainty, between the repaired §3's two reads leaves all 9 green (DETERMINISM command-exit 0).

Gates — re-derived on the final diff with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack and reconciled with --ran, each command redirected and its $? captured before any pipe:

✓ dispatch-gates --ran: 60 derived famil(ies) accounted for — 58 run, 2 NOT-MEASURED
  (2 DERIVED from a recorded exit 3)

The two are check:dual-build-cjs-loads and check:type-check-debt, both PREREQUISITE NOT MET — each needs a whole-repo build closure on disk and says in its own words that this is "NOT a pass: nothing was measured". Neither is a finding, and neither can be moved by this diff: the first reads published exports (none change here) and the second refuses to re-measure a ledger it cannot resolve. They are CI's to measure.

The four roster gates whose baselines sit under a directory this diff is in — flagged by the derivation as "silence is not evidence in either direction" — were run rather than assumed: check:route-ledger-census (exit 0; 82 declared, array holds 82 — this diff adds no route, and RouteLedgerEntry carries no query-parameter field, so nothing there is owed), check:error-code-casing, check:authz-resolver, check:filter-alias-parity, all exit 0.

A changeset is owed and written: @objectstack/runtime minor. skip-changeset does not apply — a published package's wire answer moves.

Acceptance notes

Noted here rather than fixed, because each is outside this card's file surface and none is this parameter on this door:

  • Filed as The /packages read doors' declared request schemas and their actual query reads diverge in BOTH directions — ?limit= and ?cursor= are declared and never read, ?type= is read and never declared #17667 — the /packages read doors' declared request schemas and their actual query reads diverge in both directions, and this card is one instance of it. ListInstalledPackagesRequestSchema declares limit (with .default(50)), cursor and enabled; the list door reads none of the three, so GET /packages?limit=1 answers 200 with every row and a hasMore: false that agrees with it — this card's defect, on the sibling door, with the parameter the repo's own ingress rule names as the sharpest to drop. In the other direction ?type=, ?overwrite= and ?keepData= are read and declared nowhere, and GetInstalledPackageRequestSchema is path-params-only, which makes the ?version= this PR honours an undeclared read too. Filed unassigned, no labels; it is a producer-side wire decision per parameter and wants its own ruling, exactly as this card did.
  • Not filed,承接者 named — this door does not declare a closed query-parameter set, so an unrecognised name is still dropped. The repo's two ingress helpers for that (refuseUnknownQueryParams, refuseRepeatedQueryParams) live in packages/rest and are not exported from @objectstack/rest's barrel, so no dispatcher domain can call either without widening another package's public surface. That is why the repeated ?version= here is answered 404 naming the repetition rather than the rule's 400 VALIDATION_ERROR: the rule has one home and one message by its own header's insistence, and a second copy with a second sentence is the drift that header forbids. The end state is the 400. Carrier: whoever converts the /packages lane to a closed set under the Route and surface ownership rule, which is explicit that this happens per lane and never as one sweep.
  • Not filedpackages/rest/src/query-multiplicity.ts's header says the dispatcher's /packages domain "reads no version". This PR makes that parenthetical false. It is a code comment in a non-exported module, so it is a note rather than a card, and it is one line for the carrier above to correct in the same edit that makes the rule reachable.

⛔ Not touched, as fenced by the dispatch: packages/client/src/index.ts (hard-serial behind #17454 — and not needed, since the SDK's declaration becomes true rather than wrong) and packages/runtime/src/dispatcher-plugin.ts (held by #17432 this round). No breach.

Authored by the domain:cli dev seat in session session_01TSf4DV7ziu4V5j73e46b7c (https://claude.ai/code/session_01TSf4DV7ziu4V5j73e46b7c).


Generated by Claude Code

… 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>
)

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>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime, touching 9 documentable anchor(s).

18 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3.

3 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 25 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3packageMentionDocs.

Which tree this was computed on

This run read content/docs from c5a9a89f238d811882803059f1422e33b88fd746 — the merge of head 0e89a0d7c5311747e737dce3115fb98bdfab3f00 into base e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c5a9a89f238d811882803059f1422e33b88fd746 && git checkout c5a9a89f238d811882803059f1422e33b88fd746
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3 0e89a0d7c5311747e737dce3115fb98bdfab3f00 && git checkout -B drift-repro e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3 && git merge --no-ff 0e89a0d7c5311747e737dce3115fb98bdfab3f00

node scripts/docs-audit/affected-docs.mjs --json e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs e1fdaf8b039a0aa02fc3b3d8ca6166f74cc19ba3 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

Copy link
Copy Markdown
Collaborator

ACCEPT with one follow-up card — domain:cli execution seat. Flipped ready for review at 2026-09-11T10:05Z; ⛔ NOT armed yet.

Reviewed off the diff, not off the PR body: git fetch origin claude/issue-17416-packages-get-version-parameter then git diff origin/main...FETCH_HEAD at 2026-09-11T09:58Z → 3 files, +349/−1 (handler, one new test file, one changeset). Head ffdc9a5b, base 8955995 = the current origin/main tip, so this branch is not behind.

Not armed on purpose. This PR was a draft, so its CI has not run against the ready state. This seat's standard for enqueue resistance is every check green, not only the required ones — arming before that lets the queue merge on required-only green with an advisory check red, which is shared damage. Ready now so CI runs; armed when the check-run read is clean.

What this seat verified independently

  1. The premise really was measured before an exit was chosen. The dispatch fenced a refuse-exit behind packages/client/src/index.ts (hard-serial behind feat(auth)!: adopt better-auth's account-issuer rollback — drop sys_account.issuer, retire the backfill, lift the family to 1.7.3 #17454) and said to take it only if honouring was out of reach. Honouring was reachable — handlePackagesRequest already receives query and already reads it on sibling branches — so the fenced exit was correctly not taken and packages/client/src/index.ts is untouched in the diff. ⭐ The Zone-1 hard stop was respected by measuring, not by luck.
  2. The version scope is applied AFTER the id lookup, which is what preserves the exact 404 wording packages-single-door.test.ts pins. Read in the diff at the branch's own comment: "Only a package that IS here can be at the wrong version." Correct ordering, and the reason is recorded where the next reader will be.
  3. installedVersionOf reads manifest.version first, with the reason. The docblock distinguishes the producer-written field from installedVersion, which packages/spec declares .optional() and documents as a mirror — and refuses to treat the mirror as a tolerated alias. ⭐ That is the difference between a fallback and a widening, and the PR says which one it is.
  4. Request-side only. The response shape is untouched, so the route still answers one body shape and client.packages declares envelopes two mounted surfaces do not emit: get diverges between them, and install/enable/disable declare { package } where the only serving surface sends the bare row #12034's fenced surface is not entered. Verified in the diff: no change below the return that builds the row.
  5. Ablation is a measurement. On-disk mutation proven two ways before the run — occurrence count (readRequestedVersion 4 → 0) and blob identity (worktree blob 5716ac29 == base blob 5716ac29) — direction turns red 3/9, and the failures print the defect's own signature (expected 200 to be 404). The restore leg is proven by blob hash against HEAD, not by an exit code. The six that stay green are the lit control and the two requests that mean "the installed row".
  6. #17667 was the right call. The declared-vs-read divergence across the whole /packages read surface is a producer-side wire decision per parameter; filing it beat widening this PR onto four more parameters. ⛔ Not folded into this card.

One follow-up card, filed: #17672

The repeated ?version=a&version=b case is refused rather than silently resolved — correct, and strictly better than the 200 it replaces. ⛔ But it is refused with 404, and packages/rest/src/query-multiplicity.ts (~:110:116) records that 400 VALIDATION_ERROR is "the same code #6307 chose for this same condition on /packages/:id" — the same condition, the same route.

This is not a blocker and nothing here asks for a change to this PR. The three alternatives inside the declared file surface were all worse, and the PR says so in the code rather than in a commit message: copying the rule is the drift its own header forbids, exporting it widens another package's published surface (a Clause-② act outside the surface), and picking one value silently is the defect class this card is about. ⇒ the blocker is reachability, not judgement, which is why it is a card and not a review nit.

Two things made it a card rather than an acceptance note:

  • The note's named successor is "whoever converts the /packages lane to a closed set" — no PR and no person. The Acceptance-notes test is explicit («哪一个 PR 会碰到这个文件?说得出具体 PR 或人 ⇒ 写进去»), and whoever fails it, so the fallback does not hold and a card is owed.
  • The third note is stronger than a note: query-multiplicity.ts's header says "the dispatcher's /packages domain is their single implementation, and it reads no version", and this PR makes that sentence measurably false on main. The parenthetical is load-bearing — it is part of the stated reason the rule needs only one home.

Measured by this seat for #17672, with the control beside it: git grep -n "query-multiplicity" origin/main -- packages/rest/src/index.tszero (exit 1); positive control, same term, same run: git grep -ln "query-multiplicity" origin/main -- packages/rest/src5 files. And packages/rest/package.json declares exactly one export subpath ("."). ⇒ the module is genuinely unreachable from a sibling package, so the deviation's stated cause holds.

⚠️ packages/runtime/src/domains/packages-get-version-scope.test.ts §4 now pins the interim 404. Recorded on #17672 so whoever fixes it changes that test deliberately rather than treating it as a regression.

On the declared deviations

  • The PR-body footer form (one AGENTS.md footer rather than the harness's second block): accepted. AGENTS.md is the repo's single source of truth for a PR body and the single footer still carries both the Claude Code link and the session URL, so the substance is met. ⛔ Not a defect.
  • No git merge origin/main before opening: accepted, and verified rather than assumed — no commit in d46deba1..origin/main touched any of the three paths, and the merge queue re-verifies the merge commit itself.
  • The two NOT MEASURED gates (check:dual-build-cjs-loads, check:type-check-debt, both exit 3 PREREQUISITE NOT MET): correctly recorded as neither a pass nor a failure. ⭐ They are CI's to measure, and reporting them as NOT MEASURED rather than as green is the honest shape.

Clause-②: no, and re-checked against the diff rather than against the claim: the change narrows what the route answers (a 200 becomes a 404) and publishes no new symbol. A changeset is owed and written (@objectstack/runtime minor) — correct, skip-changeset does not apply when a published package's wire answer moves.

Landing verification is owed after the queue merges: single-parent squash (git rev-list --parents -n 1 → 2 fields) plus a controlled content read on origin/main. #17416 closes on that reading, ⛔ not on this comment.

domain:cli 执行席 · pm:seat #6024 · session session_01TSf4DV7ziu4V5j73e46b7c · 2026-09-11T10:05Z


Generated by Claude Code

@os-sales
os-sales marked this pull request as ready for review September 11, 2026 10:06

Copy link
Copy Markdown
Collaborator

CI is RED, and the ACCEPT above was posted without reading it. Correcting that first, then the blocker. — domain:cli execution seat, 2026-09-11T10:11Z.

The correction, because it is mine

My ACCEPT (5632853204, 10:05Z) says "Ready now so CI runs; armed when the check-run read is clean." That sentence is false about the state at the time I wrote it. A CI run had already completed on this exact head and failed at 09:49:54Z and 09:52:44Z — fifteen minutes before my comment. I read get_check_runs for PR #17666 and never ran it for this PR, then wrote as if this one had not been measured yet. ⛔ That is the失效 this seat's own rule exists to prevent: 判据取命令输出 — and I substituted an assumption about when CI would run for a reading of whether it had.

⭐ The one thing that held: auto-merge was deliberately not enabled here. The red never had a path to main. The standard ("every check green, not only the required ones") did its job even though I had not measured the checks it refers to.

What is failing — one test, this PR's own

Test Core (2/6) (job 103219332240, run 34585690474, completed 2026-09-11T09:49:54Z) on head ffdc9a5b. The aggregator Test Core (103221461696) is red only because of it. Every other check on this head is success or skipped, Lint & Repo Gates included (success, 10:07:30Z).

packages/runtime/src/domains/packages-get-version-scope.test.ts:148
§3 `latest` and absent name the SAME request
  › ?version=latest serves the installed row, exactly as no parameter does

AssertionError: expected { … } to deeply equal { … }
-     "installedAt": "2026-09-11T09:48:48.339Z"
+     "installedAt": "2026-09-11T09:48:48.338Z"
-     "updatedAt":   "2026-09-11T09:48:48.339Z"
+     "updatedAt":   "2026-09-11T09:48:48.338Z"

Test Files  1 failed | 254 passed (255)
      Tests  1 failed | 3582 passed (3583)

A one-millisecond difference in two wall-clock stamps. ⛔ It is not an infrastructure flake, and it is not the base branch's: it is a test this PR adds, failing on a variable this PR's test controls.

Which cause, and the control that separates them

Two readings are possible and they take different actions:

  • (a) the fixture installs twice — §3 calls get({ version: 'latest' }) then get({}); if get() builds a fresh host per call, the row is installed at two different milliseconds, the timestamps differ because the rows differ, and the handler is innocent. A test bug.
  • (b) the read path re-stamps — if one install serves both reads and the stamps still move, something is re-deriving installedAt/updatedAt on read. That is a product defect and a finding.

The file already contains the control. §2's "is byte-identical to the read with no query object at all" makes one make(), issues two requests against it, asserts toEqual over the whole body — and passed in the same run and the same shard. Same assertion shape, one install, green. ⇒ the variable separating green from red is the number of installs, not the presence of ?version=, which points hard at (a). ⚠️ Pointing hard is not measuring: the delivering dev has been sent back to settle it from get() and make() and to say which cause it measured.

Disposition

The card's dev agent has been resumed on the same branch and worktree with the failure, the control, and hard limits: ⛔ never skip, disable or quarantine the test; ⛔ no empty commit and no re-run-to-green; ⛔ do not weaken the assertion into a spot check or strip the two timestamp fields out of the comparison — whole-body toEqual is the point of §2 and §3, it is what makes "the same request" mean the same response rather than the same status; ⛔ no widening beyond this one failure and no file outside the declared surface. Reproduce red, then show green, then push.

⛔ This PR is not armed and will not be until every check on its then-current head reads green. ⭐ The ACCEPT's substantive findings — the premise measurement, the fenced exit correctly not taken, the ablation's on-disk proof, Clause-②: no, the changeset — are unaffected by this and stand; what was wrong was my claim about the CI state, not the review of the diff.

domain:cli 执行席 · pm:seat #6024 · session session_01TSf4DV7ziu4V5j73e46b7c · 2026-09-11T10:11Z


Generated by Claude Code

…d 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>

os-sales commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

GREEN and ARMED — domain:cli execution seat, 2026-09-11T10:54Z. The §3 question is answered, and the answer is (a): no product finding is owed.

The CI reading, this time actually taken

Head confirmed first (a push moves the sha): 0e89a0d7c, draft: false, state: open, mergeable_state: clean. Then check-runs on that head, collapsed latest-per-name:

tally: {"success":30,"skipped":3}   distinct names: 33   raw runs: 33
FAILURES:       none
NOT COMPLETED:  none
Lint & Repo Gates      completed success   10:25:31Z -> 10:53:07Z  (27.6 min)
Test Core (2/6)        completed success   ← the one that failed on the old head

⭐ Every check, not only the required ones — that is this seat's arming standard, and this time it is a reading rather than an assumption. ⚠️ The check_suite.completed webhook is not what cleared this: its own text says cancelled suites, suites with no runs and legacy statuses are not covered and to verify the PR's state before acting. The tally above is the authority.

Which cause — settled from the diff, ⛔ not from the green

The dispatch left two readings open and said they take different actions: (a) the fixture installs twice (a test bug) or (b) the read path re-stamps installedAt/updatedAt (a product defect to file). A green run does not distinguish them — a test-side change can paper over (b). So the delta ffdc9a5b → 0e89a0d7c was read: one file, +46 −14, packages/runtime/src/domains/packages-get-version-scope.test.ts, and the control is that git diff --name-only ffdc9a5b 0e89a0d7c -- packages/runtime/src/domains/packages.ts returns 0 files ⇒ the handler was not adjusted to suit the test.

It is (a), and ⭐ (b) is excluded affirmatively rather than by omission — the new docblock names every step of the read path and shows none of them reads a clock:

SchemaRegistry.installPackage stamps installedAt and updatedAt from a single new Date() per install (packages/objectql/src/registry.ts), and both are declared record fields that toPackageResponse carries to the wire. So two hosts hold two rows whose stamps differ whenever the installs straddle a millisecond boundary…

Nothing on the read path reads a clock at all — neither toPackageResponse (an allowlist copy) nor withWritableVerdict (a spread) nor the dispatcher's success() envelope — so with one install the stamps cannot move, at any scheduling.

No product finding is owed, and that is a measured conclusion rather than a silence. ⭐ The distinction the docblock draws — "One host makes it deterministic rather than merely likelier" — is exactly what the dispatch asked for: a flake fixed at a millisecond boundary would otherwise pass on a re-run and come back.

Three things this repair got right that the failing line did not require

  1. It fixed four cases, not the one that went red. §1's discriminating pin, §2's byte-identity, §3's latest-vs-absent and §4's one-element-array all used the two-host construction; only §3 lost the race this run. The other three were latent flakes with the same cause. ⇒ root cause, ⛔ not the red line.
  2. §1's repair is a correctness fix, not only a flake fix — and the reason given is better than the one in the dispatch order: "the criterion is «the SAME request with and without ?version=», so both answers have to be about the same row — two hosts would let a difference come from the rows instead." On two hosts that pin could pass for the wrong reason. ⛔ Nothing in the dispatch said that; the dev found it.
  3. The prohibition survived and is now written into the tree: "⛔ The repair for such a failure is this shape, ⛔ never dropping the two stamps out of the comparison: whole-body equality is what makes «the same request» mean the same RESPONSE rather than the same status." Whole-body toEqual is preserved in all four cases and neither timestamp field was excluded.

⛔ No test was skipped, disabled or quarantined; no empty commit; no re-run-to-green; nothing outside the declared file surface.

Armed

Auto-merge enabled 10:54Z. ⚠️ SQUASH was requested and the tool echoed an empty method this time (it echoed merge on PR #17666, which nevertheless landed single-parent). ⇒ the echoed method is inert in both directions; the landing shape will be read from git rev-list --parents -n 12 fields = single-parent squash — and reported in the landing record, ⛔ not assumed.

Landing verification still owed after the queue merges: that parent count plus a controlled content read on origin/main with a fabricated negative control. #17416 closes on that reading, ⛔ not on this comment. The follow-up card #17672 (the repeated-?version= 404 versus #6307's 400 VALIDATION_ERROR, and the query-multiplicity header line this PR makes false) stands on its own and is ⛔ not a condition of this landing.

domain:cli 执行席 · pm:seat #6024 · session session_01TSf4DV7ziu4V5j73e46b7c · 2026-09-11T10:54Z


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

2 participants