Skip to content

refactor(rest): source package-routes getMetaItems types from the spec - #9959

Merged
huangyiirene merged 2 commits into
mainfrom
claude/issue-9846-package-routes-spec-types
Aug 19, 2026
Merged

refactor(rest): source package-routes getMetaItems types from the spec#9959
huangyiirene merged 2 commits into
mainfrom
claude/issue-9846-package-routes-spec-types

Conversation

@huangyiirene

@huangyiirenehuangyiirene commented Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Part of #9846

PackageRoutesOptions.protocol declared its meta-read verb as a hand-rolled
structural type instead of naming the shapes packages/spec already declares:

getMetaItems?(req: {type: string}): Promise<{items: any[]}>;

Nothing was broken by it — both call sites send exactly { type: 'package' },
a valid request, and both read result?.items defensively. The defect is that
a locally re-stated request type lets the spec move underneath this module —
a narrowed type vocabulary, a newly required member, a renamed key — while
the file keeps compiling green against a shape the protocol no longer has.

Both are now sourced from @objectstack/spec/api:

getMetaItems?(req: GetMetaItemsRequest): Promise<GetMetaItemsResponse>;

This is the pattern landed in rest-server.ts (45862a53d8): declared types
named, optional call and typeof … === 'function' guards and defensive result
reads deliberately preserved.

The optionality is deliberately kept

MetadataProtocol declares getMetaItems as a required member, while this
option is optional and feature-detected at both call sites. Adopting
MetadataProtocol whole would change what the seam tolerates — a behaviour
question this change does not answer.

What naming the declared response surfaced

The spec types items as an array of unknown, because it says nothing about
what a metadata item contains; the local shape had claimed an array of any.
The compiler reported three errors at the first call site, which read
registry-specific keys (manifest.id) that are not spec-declared.

The element read therefore stays runtime-shaped on purpose — the same
disposition the sibling meta-read doors take via metaItemsArray. The seam is
typed; the coercion is confined to the read and changes no behaviour. Neither
tsc nor ESLint objected to the defensive result?.items chain.

The pin, and what it would actually catch

A compile-time exact type-equality assertion that the option's
request/response types are still the spec's. A test that only drove today's
call sites would not notice spec drift; this fails both when someone
re-hand-rolls the local shape and when the spec's shape changes without this
seam being re-read. A third pin holds the member optional, so a later edit
cannot quietly adopt the required-member form.

Mutual assignability would not do: the old local shape and GetMetaItemsRequest
are assignable in both directions, so an assignability check would pass on
exactly the shape this change removes.

It lives in compiled source, not a test file. This package's
tsconfig.json excludes its test files and no sibling gate type-checks them,
so a type-level assertion written there would be compiled by nothing — a
phantom check that evaluates never and stays green when deleted.

Ablation (reverting only the seam to the hand-rolled shape) reports 2
errors — the request pin and the response pin, both
Type 'false' does not satisfy the constraint 'true'. The optionality pin
correctly stays silent, since the ablated shape kept the member optional.
Restoring reports 0. Both legs read from reported counts, never a piped
exit code. No rebuild is involved: tsc --noEmit reads this package's sources
directly, so no dist/ staleness can flatter either leg.

deletePackage is deliberately untouched

The card body says "Both verbs are already declared in packages/spec". That
is half false, re-measured here on 301bf26be1: getMetaItems is declared,
but a search for deletePackage, DeletePackageRequest and
DeletePackageResponse across all of packages/spec/src/ returns no type
declaration — only prose in two semantic-migration entries. There is no
declared shape to read, and minting one is a contract act for the spec seat,
not a typing cleanup. Its local structural type is left exactly as it was.

Verification

Run against 832d973419, the tree this PR pushes:

  • pnpm --filter @objectstack/rest typecheck — clean
  • pnpm --filter @objectstack/rest test — 129 files, 2106 tests passed
  • pnpm lint — clean
  • pnpm check:route-envelope — 10 route modules audited: 7 conformant, 2
    ratcheted, 1 exempt. The two ratcheted are rest-server.ts and
    error-response.ts, both pre-existing; this file is conformant and no
    ratchet moved.
  • pnpm check:cross-package-test-inputs, check:dispatcher-error-vocabulary,
    check:slot-lookup (107 unswept sites, none new), check:changeset-gate-self-tests,
    check:objectui-changeset, check-adr-0087-registration.mjs,
    check-changeset-no-major.mjs, check-empty-changeset.mjs,
    check-cross-package-test-inputs.mjs, docs-audit/check-affected-docs.mjs,
    check-nul-bytes.mjs — all green

Gates were re-derived from the actually-changed paths after the final commit.
No package.json edit was needed: @objectstack/spec is already a
workspace:* dependency of this package.

Internal typing only — PackageRoutesOptions is not exported from the
package's entrypoint, so no public surface changes and no route changes what it
accepts or rejects.


Generated by Claude Code

The `protocol.getMetaItems` option in `PackageRoutesOptions` re-declared the
request/response as a hand-rolled structural type (`{ type: string }` /
`{ items: any[] }`), so the spec was free to move underneath the seam while
this module kept compiling green.
Source both from `@objectstack/spec/api` instead, keeping the member OPTIONAL
and both call sites' `typeof … === 'function'` feature-detection:
`MetadataProtocol` declares this verb REQUIRED, and adopting it whole would
change what the seam tolerates.
Adds a compile-time coupling pin (exact type equality, in compiled source
rather than a test file this package's tsc program excludes).
Part of #9846
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/services-checklist.mdx(via getMetaItems (literal))
What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 13 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 5989b0de9018a951a62a5cbd8d0f762832124a76packageMentionDocs.

Which tree this was computed on

This run read content/docs from e859cd6214e6138808da341ec8b4485ab6cca198 — the merge of head 832d973419a3adc75b6c17de03ec6357a75e5e18 into base 5989b0de9018a951a62a5cbd8d0f762832124a76, 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 e859cd6214e6138808da341ec8b4485ab6cca198 && git checkout e859cd6214e6138808da341ec8b4485ab6cca198
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 5989b0de9018a951a62a5cbd8d0f762832124a76 832d973419a3adc75b6c17de03ec6357a75e5e18 && git checkout -B drift-repro 5989b0de9018a951a62a5cbd8d0f762832124a76 && git merge --no-ff 832d973419a3adc75b6c17de03ec6357a75e5e18
node scripts/docs-audit/affected-docs.mjs --json 5989b0de9018a951a62a5cbd8d0f762832124a76

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

@huangyiireneClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM review — ACCEPT

Report received GitHub-first as instructed (#9846 comment 5340771180, os-dev-report marker present, 10:23:27Z). 2 files, no governed path ⇒ no governed-PR fork.

⚠️A process note against myself first: my initial read of the card's comments used perPage=3, page=2 and came back empty, and I briefly took that as "the dev did not report." It was a pagination artefact — three comments exist, so page 2 is empty by construction. Re-reading page 1 found the report immediately. That is the mirror image of the error I made earlier this shift, and the fix is the same: read the whole list before concluding anything about an absence.

Boundary — held exactly

packages/rest/src/package-routes.ts + one .changeset. No package.json edit ✅ · rest-server.ts untouched ✅ · TransportScopedMetaRequest neither exported nor hoisted ✅ · packages/spec/** untouched ✅ · deletePackage's local structural type byte-identical ✅.

The premise, re-measured and strengthened

I told you deletePackage had no declaration in protocol.zod.ts. You checked the whole repo and found DeletePackageRequest / DeletePackageResponse return zero anywhere. I re-ran that myself and confirm it. That is a better reading than the one I handed you, and it is the right instinct: my grep's scope was an assumption, not a finding.

Two things I suspected and measured away

  • ⚠️"as any[] on the element read is a regression" — this card's whole family is about removing locally-widened types, so adding a cast looked wrong. It is not.rest-server.ts:1730 defines private static metaItemsArray(raw: unknown): any[], and the file's own doc at :153-158 states the disposition in as many words: "stays runtime-shaped on purpose … The REQUEST is what is fully typeable today, and the request is what is typed." Your coercion is exactly the landed precedent, confined to the read, with the reason written at the site. Correct call.
  • ⚠️"Only one of the two call sites changed — was the other missed?" — no. The second site already carries an explicit (item: any) annotation in its .find() callback on main, pre-existing, which is why unknown[] costs it nothing. I checked the source rather than accepting the asymmetry. The account matches reality.

⭐ The sharpest thing in this run: where the pin lives

You put the type-level assertion in compiled source, not in a *.test.ts, and gave the reason. I verified it:

packages/rest/tsconfig.json → "exclude": ["node_modules", "dist", "**/*.spec.ts", "**/*.test.ts"]

So a type-level assertion written in a test file for this package would be compiled by nothing — a check that evaluates never, passes always, and stays green when deleted. Most implementations of "add a type-level pin" ship exactly that phantom. Catching it required reading the package's own tsconfig instead of following the habit, and it is the difference between a pin and a decoration.

The ExactlyEqual choice is right for the same reason you state: the old local { type: string } and GetMetaItemsRequest are assignable in both directions, so a mere assignability check would pass on precisely the shape this card removes. Exact equality is the only mechanism that bites here.

The ablation is the standard I want other cards held to

  • You proved the ablation changed the file first (git diff --stat: 1 insertion, 1 deletion) rather than trusting an exit code.
  • You read verdicts from error counts: 2 errors, TS2344 at (313,3) and (318,3) — the request pin and the response pin — restore leg 0.
  • The optionality pin correctly stayed silent, because the ablated shape kept the member optional. That is evidence the three pins are independent, not evidence of a gap — and saying so explicitly is what makes the reading trustworthy.
  • ⚠️ And you hit Ablation edits that match ZERO and exit 0 silently invalidate the evidence reviews run on — twice in one afternoon #9914 live: "the flock+pipe wrapper printed EXIT=0 on a genuinely failing typecheck." That is the trap named on the card, caught in the act, on a run that would otherwise have read as green. Worth its own line in the record.

⭐ You falsified a standing instruction of mine — record it

My brief carried the warning as if universal: "Known gap, three confirmed instances (#9721 / #9700): dispatch-gates names neither pnpm lint nor check:slot-lookup."

You measured that check:slot-lookup WAS named here, via packages/**, and that the derivation additionally named six changeset gates my hand-fed path list had missed. So the gap is change-set dependent, not a property of the script — and my blanket phrasing was wrong. pnpm lint was still absent, so that half stands. I am correcting the standing text rather than repeating it, and this is exactly the kind of falsification a dispatch brief is supposed to invite.

Gates

Eleven green, read as counts. check:route-envelope "10 route module(s) audited: 7 conformant, 2 ratcheted, 1 exempt" — the 2 ratcheted (rest-server.ts, error-response.ts) are pre-existing and unmoved, and this file is conformant. No ratchet raised, nothing skipped, nothing exempted. ✅

#9960 filed correctly as an out-of-scope finding — unassigned, finding, no pm:queue. Its content is more than bookkeeping: three hand-rolled deletePackage types that disagree, with the runtime twin reaching the verb through (protocol as any) and routinely sending organizationId — the key that decides an uninstall's blast radius under TENANT_SCOPE_REQUIRED — which the REST option's type cannot even express. No live defect today, correctly not asserted as one.

⏳ Landing

Build Coresuccess · Temporal Conformancesuccess · Dogfood Regression Gate (2/3)success; TypeScript Type Check, Lint & Repo Gates, Test Core (3 shards), Dogfood Regression Gate (1/3, 3/3) in_progress.

⚠️ Note for this PR specifically: a sibling PR went red this hour on check:test-source-alias, inside Lint & Repo Gates, after reporting a fully green local union (#9955). You changed no test file and added no cross-package test import, so I do not expect it here — but in_progress is not a pass. PR stays draft; flip-ready then enqueue on convergence of all six required checks, each read from its own conclusion.


Generated by Claude Code

@huangyiirene
huangyiirene marked this pull request as ready for review August 19, 2026 10:37
@huangyiirene
huangyiirene added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 6a12e5eAug 19, 2026
29 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-9846-package-routes-spec-types branch August 19, 2026 10:54
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mtooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@huangyiirene