Skip to content

fix(client): bind packages.get to the bare InstalledPackage row on both the global and scoped clients - #17419

Merged
os-justin merged 2 commits into
mainfrom
claude/issue-12034-packages-get-single-type
Sep 10, 2026
Merged

fix(client): bind packages.get to the bare InstalledPackage row on both the global and scoped clients#17419
os-justin merged 2 commits into
mainfrom
claude/issue-12034-packages-get-single-type

Conversation

@os-justin

@os-justin os-justin commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Fixes #12034

Binds packages.get to the bare InstalledPackage row on both the global ObjectStackClient and ScopedEnvironmentClient, replacing a { package } envelope that no mounted surface emits.

Implements the maintainer's Option A ruling of 2026-09-09 (verbatim 「同意」), recorded on the card. The whole remaining card ships here, hence a closing keyword — unlike PR #13060, which correctly used Part of when only install / enable / disable shipped.

Clause-②: yes

The ruling's three items, as delivered

# ruled delivered
1 REST registrar's two sendOk sites fold source into the row already satisfied, by deletion — no producer edit in this PR
2 ScopedEnvironmentClient.packages.get binding and its pin move in the same PR done — binding, pin, and the false rationale beside it
3 client.packages.get binds the single true type done

Item 1 was delivered by another card, and I verified that rather than assuming it

The ruling named packages/rest/src/package-routes.ts:817 and :880. That file is 506 lines — those offsets are off the end of it. PR #16628 (card #14503, merged 2026-09-08) removed the REST read routes outright instead of converging them.

Verified on the merge base rather than taken from the card: registerPackageRoutes builds exactly one route and hands that same array to mountDirectRoutes, which is also what it returns as the description of what it mounted —

const routes: readonly DirectMountedRoute[] = [publishRoute];
return mountDirectRoutes(server, routes);

so POST /packages/publish is the registrar's whole surface, and the only { package } left in that file belongs to the publish response, a different client method this card does not touch. ⇒ The two-surface fork is gone, the dispatcher is the sole implementation, and it already sends the shape Option A ruled for. This card was SDK-side only.

Why the type is now bindable, and why it is this type

The dispatcher's /packages domain builds the detail body and every list row with one expression:

withWritableVerdict(qlService, toPackageResponse(pkg))

GET /packages/:id returns it directly; GET /packages maps it over the collection. So binding get to InstalledPackage makes two doors of one domain agree — it is not a fresh claim about either. packages.list has declared InstalledPackage[] since #11925, and @objectstack/spec's own GetInstalledPackageResponseSchema declares data: InstalledPackageSchema and always has. The declaration was the only thing out of step.

The scoped method is the sharper half. Its rationale in source read: "only the REST registrar serves the scoped path — so the { package } envelope declared here is the one that route actually sends." #16628 falsified it. The mount is still there (direct-mount-composition.ts registers the registrar at both {base}/packages and {base}/environments/:environmentId/packages) — it simply serves no read any more. What serves the scoped path is the dispatcher via the @objectstack/hono catch-all, which strips the /environments/:environmentId prefix and is, per http-dispatcher.ts, the only entry that hands dispatch() a still-scoped path. ⇒ Between #16628 and this PR the scoped declaration described a body emitted nowhere, with a real InstalledPackage behind the member rather than an any — so .package reads there looked type-safe and returned undefined. That stale rationale is corrected here too; leaving it would pin a false explanation next to a corrected binding.

Evidence — the pins are red before and green after

A runtime test cannot observe a return-type narrowing, so the evidence is type-level. Ablation: fix committed first, then packages/client/src/index.ts restored to the merge base with the pin file kept, mutation proven on disk by blob hash (25f5c960… == BASE, != HEAD), restored under an EXIT/INT/TERM trap and verified byte-identical afterwards (git diff HEAD empty, on-disk hash == HEAD blob).

npx tsc -p tsconfig.test.json --noEmit, same command both legs:

leg exit errors
ablated (declarations reverted) 2 5, all in return-type-precision.test.ts
restored 0 0

The five:

(335,75) TS2344  scoped pin in returnTypePrecisionPins11925
(572,75) TS2344  client.packages.get pin
(573,75) TS2344  scoped.packages.get pin
(613,5)  TS2578  Unused '@ts-expect-error'   <- .package on the global get
(615,5)  TS2578  Unused '@ts-expect-error'   <- .package on the scoped get

The two TS2578 rows are the point. The guard that previously sat beside the scoped binding was labelled "GREEN IN BOTH STATES — regression guard, not red-before evidence", and its claim ("the scoped detail route answers { package }, not the bare row") is now false. It is not reworded in place: a line that is red before this card cannot also be green in both states of #11925, and that paragraph is the record of #11925's ablation. It is removed with that reasoning written down, and the scoped method's direction-2 evidence moves to returnTypePrecisionPins12034, beside the three siblings that fail identically.

These suppressions are compiled, not phantom. tsc --noEmit on the package's main config does not include this file (--listFiles count: 0); tsconfig.test.json does (count: 1), and pnpm --filter @objectstack/client typecheck runs both it and check:test-typecheck.

Blast radius

In-repo callers: zero. Re-measured rather than cited from the card, whose numbers came from a probe where test and control both returned 0. Using grep -r (not git grep, which reads only tracked files) over .ts/.tsx/.mts/.mjs/.js/.mdx: packages.get has 22 hits, every one of them the declarations themselves, their pins, the route-ledger row, or an unrelated Map.get() on a local named packages in build scripts. Positive controls fired — packages.list 17, packages.install 17, metadata.list 37 — and a fabricated packages.zzznosuch returned 0, so the zero is a reading and not a dead scan.

Pinned sibling: zero. objectui at the pinned .objectui-sha 53ded82bf was cloned and grepped: no SDK packages.* callers (control client. = 433 hits). The Console Pin Gate is not at risk.

Documentation examples: covered. check:skill-examples type-checks 258 prose blocks including 23 on the client SDK surface; a .package read in any of them would have gone red. It did not.

⚠️ None of this measures the real exposure. The published SDK's external consumers are the surface that breaks, and no in-repo scan can see them. (await client.packages.get(id)).package compiles today and will not after this. That is the card's purpose — the read was already undefined at runtime on every surface that serves this route — but it is a genuine compile-time break for anyone who wrote it, and "0 in-repo callers" must not be read as "no impact".

Changeset

minor, not major (check-changeset-no-major blocks major; precedent: client-unannotated-return-erasure.md #11925, client-precise-sdk-return-types.md #8140). It spells FROM → TO and carries the migration line: read the row directly, not .package.

Acceptance notes

Verification

dependency closure pnpm --filter '@objectstack/client^...' build — exit 0
full workspace build pnpm build — 73 successful, 73 total
typecheck pnpm --filter @objectstack/client typecheck — exit 0, incl. check:test-typecheck (0 files / 0 errors in debt)
tests pnpm --filter @objectstack/client test41 files, 492 tests, all passed
gates all 60 derived families run, every one exit 0; dispatch-gates --ran reconciles 60 derived / 60 run / 0 NOT-MEASURED (a derived zero — every family recorded an exit code and none is 3)
lint pnpm lint (eslint . --no-inline-config, whole repo, no narrowing) — exit 0, clean

All of the above were re-run on the final head 3543e521 (after git merge origin/main and a full rebuild): the ratchet families check:type-check-coverage, check:type-check-debt, check:dts-closure, check:published-files, check:dual-build-cjs-loads and check:skill-examples all exit 0 there, as do the client package's typecheck and its 492 tests. The gate family set was re-derived after the merge and is byte-identical to the pre-merge derivation — 60 families, none added, none removed.

Three gates first answered PREREQUISITE NOT MET (check:skill-examples exit 1, check:dual-build-cjs-loads exit 3, check:type-check-debt exit 3) because they read built dist/ and the workspace was not fully built. That is "nothing was measured", not a red gate — each says so in its own output. All three were re-run after pnpm build and are exit 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt


Generated by Claude Code

…th clients (#12034)

`client.packages.get` declared `{ package: any }` and its environment-scoped
twin declared `{ package: InstalledPackage }`. Neither body is emitted by any
surface: the runtime dispatcher's `/packages` domain has been the single
implementation of `GET /packages/:id` since #16628 removed the REST
registrar's read routes, and it answers the bare row —
`success(withWritableVerdict(qlService, toPackageResponse(pkg)))`, the same
expression it maps over every `list` row.

Maintainer ruling of 2026-09-09 (Option A) converged this route on the bare
row. The producer half arrived by subtraction rather than convergence, so this
change is SDK-side only.

Also corrects the rationale pinned beside the scoped binding, which reasoned
from a REST mount that #16628 deleted, and moves the scoped method's
direction-2 evidence to the pin function of this card, where it is red before
the fix rather than green in both states.

Clause-②: yes — a `.package` read compiles today and will not after this.

Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt
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 10, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

23 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 93447bbadf5c5f87d180bc5c007b727026eb71d0.

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

What this run could not see
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 14 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 93447bbadf5c5f87d180bc5c007b727026eb71d0packageMentionDocs.

Which tree this was computed on

This run read content/docs from ec75520080113d36fcb0012ca11e3c2a93548d17 — the merge of head 3543e521e0c5af2355648126f95983ad0b4f0805 into base 93447bbadf5c5f87d180bc5c007b727026eb71d0, 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 ec75520080113d36fcb0012ca11e3c2a93548d17 && git checkout ec75520080113d36fcb0012ca11e3c2a93548d17
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 93447bbadf5c5f87d180bc5c007b727026eb71d0 3543e521e0c5af2355648126f95983ad0b4f0805 && git checkout -B drift-repro 93447bbadf5c5f87d180bc5c007b727026eb71d0 && git merge --no-ff 3543e521e0c5af2355648126f95983ad0b4f0805

node scripts/docs-audit/affected-docs.mjs --json 93447bbadf5c5f87d180bc5c007b727026eb71d0

⚠️ 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 93447bbadf5c5f87d180bc5c007b727026eb71d0 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-justin
os-justin marked this pull request as ready for review September 10, 2026 13:36
@os-justin
os-justin enabled auto-merge September 10, 2026 13:36
@os-justin
os-justin added this pull request to the merge queue Sep 10, 2026
Merged via the queue into main with commit fb6a2de Sep 10, 2026
45 checks passed
@os-justin
os-justin deleted the claude/issue-12034-packages-get-single-type branch September 10, 2026 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants