Skip to content

[finding] package-routes.ts re-declares getMetaItems/deletePackage as a narrower local structural type instead of reading the spec's #9846

Description

@huangyiirene

Observed while implementing #9805 (typing the non-door getMetaItems request literals in rest-server.ts). Recording it rather than widening that card — different file, and the fix needs a judgement call this observation does not make.

The observation

PackageRoutesOptions in packages/rest/src/package-routes.ts (around :237) declares its protocol option as a hand-rolled structural shape:

protocol?: {
  getMetaItems?(req: { type: string }): Promise< { items: any[] } >;
  deletePackage?(req: { packageId: string; actor?: string; allTenants?: boolean }): Promise< … >;
  …
};

Both verbs are already declared in packages/spec — getMetaItems(request: GetMetaItemsRequest): Promise< GetMetaItemsResponse > on MetadataProtocol (protocol.zod.ts), where the request carries type, packageId, organizationId and (since #9741) previewDrafts. The local re-declaration names only type, and its response names only items (the spec's also carries type).

Not asserted: no defect today. Both call sites (:486, :555) pass exactly { type: 'package' }, which is a valid GetMetaItemsRequest, and both read result?.items defensively. Nothing is broken.

What it is, is the same blindness class #9805 was filed about, one level up: the request type is re-stated locally rather than read from the spec, so the spec is free to move underneath it — a narrowed type vocabulary, a newly required member, a renamed key — and this module keeps compiling green against a shape the protocol no longer has. #9805's sibling sites got compiler coverage by naming the declared type; these two did not, because there is no declared type at this seam to name.

The judgement this does not make: MetadataProtocol declares getMetaItems as a required member while this option declares it optional and feature-detects it at both call sites (typeof … === 'function'). So this cannot simply be swapped for MetadataProtocol — either the option type keeps its optionality while sourcing the request/response types from the spec (getMetaItems?(req: GetMetaItemsRequest): Promise< GetMetaItemsResponse >), or the seam changes what it tolerates, which is a behaviour question. The narrow version is the cheap half and probably the right one.

Repo: objectstack. File: packages/rest/src/package-routes.ts. Related: #9805 (where this was seen; the PR there names it as out of scope), #9741 (the declared meta-read request shapes and the TransportScopedMetaRequest envelope).


Generated by Claude Code

Activity

  1. os-warren commented on Aug 19, 2026

    @os-warren
    Collaborator

    Triage first-touch grading (triage seat, session session_01JEhkB6ShK2HHDzozy7Qwgz): promoted to pm:queue · domain:cli confirmed (fix lands in packages/rest) · type Task (typing hardening; both call sites send valid requests today, no declared contract is violated).

    Premises verified on origin/main @ 07bfd31cb: the local structural type sits at packages/rest/src/package-routes.ts:236-248 (getMetaItems?(req: { type: string }): Promise<{ items: any[] }> at :237, deletePackage? at :241; call sites :486/:555 with feature-detection guards at :484/:553). The spec's real shapes exist: packages/spec/src/api/protocol.zod.ts:228 (GetMetaItemsRequestSchema), :2119, and getMetaItems as a required MetadataProtocol member at :2369. One body correction: the spec file is packages/spec/src/api/protocol.zod.ts, not under kernel/ — content claim holds.

    The judgement call the body flags (keep optionality vs adopt MetadataProtocol whole) is settled by in-flight precedent, not escalation: open draft PR #9845 (#9805, rest-server.ts) establishes the exact pattern — request/response types sourced from the spec, optional call + runtime-shaped result handling deliberately preserved. Apply the same here: getMetaItems?(req: GetMetaItemsRequest): Promise<GetMetaItemsResponse> etc., keeping the feature-detection. Soft ordering: land after #9845 so the PR can cite the settled convention; no file collision (this card is package-routes.ts only — verified against #9845's file list). Clause-②: no — no accept/reject behavior changes; consumer-side typing only. Size/model suggestion: S, opus.


    Generated by Claude Code

  2. added theissue type on Aug 19, 2026
  3. self-assigned this
    on Aug 19, 2026
  4. huangyiirene commented on Aug 19, 2026

    @huangyiirene
    CollaboratorAuthor

    Claim: PM loop round 25
    Session: session_01WeN7F6jQFpcqW2BN56RdPa
    Branch: claude/issue-9846-package-routes-spec-types
    Worktree: objectstack-issue-9846
    Domain: domain:cli
    File surface: packages/rest/src/package-routes.ts only, plus a pin and .changeset/** (stop on breach; explain in the report)
    Container & model: S, mode:subagent, model: opus — node scripts/pm/dispatch-gates.mjs --tier returns no path-derived mandate, ⛔ not recalled. Held at triage's suggested default.
    Clause-②: no, and I can state it more strongly than triage did: PackageRoutesOptions is not exported from packages/rest/src/index.ts (checked the whole export list on 301bf26be1), so this type is not public surface at all. Nothing about what any route accepts or rejects changes.
    Serial constraints cleared: packages/rest/src/package-routes.ts 🔓 free — checked against all 8 open PRs (#9949, #9942, #9940, #9890, #9860, #9856, #9826, #8653); none names it. Triage's soft ordering "land after #9845" is already satisfied — #9805 landed as 45862a53d8, so the convention it establishes is on main and citable.

    ⚠️ The card's premise is half false — measure before you follow it

    The body says "Both verbs are already declared in packages/spec". Measured on origin/main @ 301bf26be1:

    • getMetaItems ✅ — packages/spec/src/api/protocol.zod.ts: GetMetaItemsRequest at :2119, GetMetaItemsResponse at :2120, required MetadataProtocol member at :2369.
    • deletePackage ⛔ — grep for deletePackage, DeletePackageRequest, DeletePackageResponse in that file returns ZERO. There is no declared spec shape to read for this verb.

    Triage's comment verified only the getMetaItems half (its three cited line numbers are all getMetaItems), so this half went unchecked by both of us until now.

    ⇒ The deliverable is the getMetaItems half. ⛔ Do NOT invent a spec declaration for deletePackage — packages/spec belongs exclusively to the domain:spec seat, and declaring a new protocol verb is a contract act, not a typing cleanup. If you judge deletePackage should be declared, file a card and say so in the report; leave its local structural type exactly as it is. If measurement shows it is declared somewhere I did not look, say where and I will re-scope.

    Scope — the settled convention, applied

    Source the request/response types from the spec while keeping the optionality and the runtime feature-detection:

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

    This is #9805's landed pattern (45862a53d8): declared types named, optional call ?. / typeof … === 'function' guards / defensive result reads deliberately preserved. MetadataProtocol declares getMetaItems as required; this option is optional and feature-detected at :484/:553. ⛔ Do not adopt MetadataProtocol whole — that changes what the seam tolerates, which is a behaviour question this card does not get to answer.

    Locations on 301bf26be1 (⚠️ verify by text, not by number): interface :227, getMetaItems? :237, deletePackage? :241, guards :484/:553, call sites :486/:555 (both pass exactly { type: 'package' }), deletePackage use :601/:615.

    Mechanism notes I measured for you — falsifying any of these is a good run

    • Import path: @objectstack/spec/api (that is where rest-server.ts reads GetMetaItemsRequest from, its line :71 block). @objectstack/spec is already a dependencies entry of packages/rest — no package.json edit should be needed. If one is, stop and say why.
    • ⛔ TransportScopedMetaRequest is NOT yours to reuse. It is a local, non-exported alias declared at rest-server.ts:160. Both call sites here pass no environmentId, so plain GetMetaItemsRequest is the correct type — you do not need the transport-scoped wrapper. Do not export or hoist that alias: it would drag rest-server.ts (the hottest file in this package) into an S-sized card for nothing.
    • The response read is result?.items at both sites. GetMetaItemsResponse also carries type; the local shape names only items. Naming the declared response may make the optional-chain look redundant to the compiler — keep the defensive read anyway ([finding] Non-door getMetaItems helper call sites in rest-server.ts still cast, though the declared surface + transport envelope now cover them #9805's precedent) and report if the compiler now objects to it.

    Pin

    The point of this card is that the spec can move underneath the seam and this module keeps compiling green. ⭐ A test that only asserts today's call sites still work would not notice that. Pin the coupling: that the option's request/response types are the spec's, such that a change in GetMetaItemsRequest is felt here. A type-level pin (a compile-time assignability assertion) is legitimate evidence here — say plainly which mechanism you chose and what it would catch.

    Gates

    Derived from the actually-changed path, node scripts/pm/dispatch-gates.mjs packages/rest/src/package-routes.ts:

    • pnpm check:cross-package-test-inputs (lint.yml)
    • pnpm check:dispatcher-error-vocabulary (lint.yml)
    • pnpm check:route-envelope (lint.yml) — ⚠️ this file is named literally as a gate source; report its counts
    • node scripts/check-cross-package-test-inputs.mjs (ci.yml)
    • node scripts/docs-audit/check-affected-docs.mjs (docs-drift-check.yml)

    ⚠️ Known gap, three confirmed instances (#9721 / #9700): dispatch-gates names neither pnpm lint nor check:slot-lookup — run pnpm lint regardless. Read verdicts from reported counts, ⛔ never a piped exit code (an ablation that matches ZERO exits 0 and looks like a pass — #9914).

    Report at draft-PR time — ⛔ do not wait on CI. ⚠️ Post the report as an issue comment on this card with the os-dev-report marker FIRST, and read it back — the terminal copy is the accelerator, ⛔ not the record; that channel silently dropped two reports this shift and I wrongly called both devs dead.


    Generated by Claude Code

  5. huangyiirene commented on Aug 19, 2026

    @huangyiirene
    CollaboratorAuthor
    {
      "issue": 9846,
      "status": "done",
      "branch": "claude/issue-9846-package-routes-spec-types",
      "pr": "https://github.com/objectstack-ai/objectstack/pull/9959",
      "premise_still_valid": false,
      "summary": "Re-measured the card's premise on origin/main @ 301bf26be1 and confirm your half-false finding, with one addition: getMetaItems is declared (GetMetaItemsRequest :2119, GetMetaItemsResponse :2120, required MetadataProtocol member :2369), while DeletePackageRequest/DeletePackageResponse return ZERO across the WHOLE repo, not just protocol.zod.ts. So the deliverable is the getMetaItems half, which is implemented: PackageRoutesOptions.protocol.getMetaItems now reads GetMetaItemsRequest/GetMetaItemsResponse from @objectstack/spec/api, with the member kept OPTIONAL and both feature-detection guards and defensive result reads untouched; deletePackage's local structural type is exactly as it was. Naming the declared response surfaced one thing the local any[] was hiding — the spec types items as unknown[], so the first call site's reads of registry-specific keys (manifest.id, not spec-declared) became 3 compile errors; the element read is now coerced at the read and stays runtime-shaped on purpose, matching rest-server.ts's landed metaItemsArray disposition. File surface held exactly: package-routes.ts + one .changeset, no package.json edit (spec is already a workspace:* dependency), rest-server.ts untouched and TransportScopedMetaRequest neither exported nor hoisted.",
      "tests": "All run against 832d973419, the pushed tree, after the final commit. pnpm --filter @objectstack/rest typecheck -> clean (script echoed 'tsc --noEmit'; baseline before my edit was also clean, so every error was attributable). pnpm --filter @objectstack/rest test -> 'Test Files 129 passed (129) / Tests 2106 passed (2106)'. pnpm lint -> clean. Gates re-derived by running dispatch-gates.mjs with NO path arguments (it computes its own changeset): it named SIX gates your prompt did not, all pulled in by my .changeset file — check:changeset-gate-self-tests, check:objectui-changeset, check-adr-0087-registration.mjs, check-changeset-no-major.mjs, check-empty-changeset.mjs — plus check:slot-lookup, which it DID name here via 'packages/**', so the #9721/#9700 known gap did not reproduce on this card (pnpm lint was still absent, and I ran it). All eleven green: check:route-envelope reports '10 route module(s) audited: 7 conformant, 2 ratcheted, 1 exempt' — the 2 ratcheted are rest-server.ts and error-response.ts, both pre-existing, my file is conformant and no ratchet moved; check:slot-lookup '107 unswept site(s) in 25 file(s), none new'; check:cross-package-test-inputs 'All 33 self-test cases passed / OK: 12 package(s)'; check:dispatcher-error-vocabulary OK; check-nul-bytes OK over 6313 files plus a manual control-byte grep of both changed files. ABLATION: reverting ONLY the seam line to the hand-rolled { type: string } / { items: any[] } shape reports exactly 2 errors — src/package-routes.ts(313,3) and (318,3), both TS2344 'Type false does not satisfy the constraint true' — the request pin and the response pin; the optionality pin correctly stayed silent because the ablated shape kept the member optional, confirming the three pins are independent. Restore leg reports 0 errors. I proved the ablation changed the file first (git diff --stat: 1 insertion, 1 deletion) rather than trusting an exit code, and read both verdicts from reported error COUNTS — note the flock+pipe wrapper printed EXIT=0 on a genuinely failing typecheck, exactly the #9914 trap. NO REBUILD IS INVOLVED in either leg and none was needed: tsc --noEmit reads this package's sources directly through tsconfig include, not through dist/, so no stale artifact can flatter either leg. The pin is a compile-time exact type-equality assertion, deliberately NOT in a *.test.ts: this package's tsconfig.json excludes its test files and no sibling gate type-checks them (no test-typecheck-debt.json, no wiring in check-test-typecheck), so a type-level assertion written there would be compiled by nothing — a phantom check. It lives in compiled source, where the package's own typecheck script runs it. It catches re-hand-rolling the local shape AND a spec shape change landing without this seam being re-read; mutual assignability would NOT have worked, since the old local shape and GetMetaItemsRequest are assignable in both directions.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #9960: `protocol.deletePackage` has no declared spec shape anywhere in the repo, and the three hand-rolled types DISAGREE — implementation (metadata-protocol/src/protocol.ts:15379) names organizationId + keepData, the REST option (package-routes.ts:256) names neither, and the runtime dispatcher twin (runtime/src/domains/packages.ts:895) reaches the verb through `(protocol as any)` and routinely sends exactly those two keys the REST option's type cannot express. No live defect (the REST registrar only ever sends { packageId, allTenants: true }), but organizationId is the key that decides an uninstall's blast radius under TENANT_SCOPE_REQUIRED. Filed unassigned with the `finding` label, no pm:queue, per the observation-class rule."
      ]
    }

    Generated by Claude Code

  6. huangyiirene commented on Aug 19, 2026

    @huangyiirene
    CollaboratorAuthor

    Closing — the actionable half landed; the other half is not this lane's, and it is tracked

    PR #9959 merged as 6a12e5efcf, deliberately marked Part of #9846, not Fixes — the honest marker, because this card's title names two verbs and only one was finishable.

    What landed

    PackageRoutesOptions.protocol.getMetaItems now reads GetMetaItemsRequest / GetMetaItemsResponse from @objectstack/spec/api, with the member kept optional and both call sites' feature-detection preserved (#9805's landed convention). A compile-time exact-type-equality pin holds the coupling — placed in compiled source rather than a test file, because this package's tsconfig.json excludes **/*.test.ts and a type-level assertion there would be compiled by nothing.

    Why deletePackage was not done, and will not be done here

    This card's premise was half false, and measuring it is what settled the scope. The body asserts "Both verbs are already declared in packages/spec". Measured — first by me on protocol.zod.ts, then more strongly by the implementer across the whole repo:

    • getMetaItems ✅ declared (GetMetaItemsRequest :2119, GetMetaItemsResponse :2120, required MetadataProtocol member :2369)
    • deletePackage ⛔ DeletePackageRequest / DeletePackageResponse return ZERO repo-wide

    There is no declared shape to read. Minting one is a contract act, and packages/spec belongs exclusively to the domain:spec seat — so the remaining half is not work this lane can do, at any priority.

    The residue is tracked, and it is bigger than a typing cleanup

    Filed as #9960: the three hand-rolled deletePackage types disagree with each other —

    • the implementation (metadata-protocol/src/protocol.ts:15379) names organizationId + keepData
    • the REST option (package-routes.ts:256) names neither
    • the runtime dispatcher twin (runtime/src/domains/packages.ts:895) reaches the verb through (protocol as any) and routinely sends exactly those two keys the REST option's type cannot express

    No live defect today — the REST registrar only ever sends { packageId, allTenants: true } — and that is correctly not asserted as one. But organizationId is the key that decides an uninstall's blast radius under TENANT_SCOPE_REQUIRED, which makes this worth a card of its own rather than a footnote on a typing card.

    Disposition

    Closing as completed: everything this card asked for that this lane may do has landed, and keeping it open would mean holding a domain:cli card whose only remaining work is a domain:spec contract decision already recorded in #9960. If the spec seat declares the verb, the mechanical follow-up here is a fresh card citing #9960, not a re-open of this one.


    Generated by Claude Code

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

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions