Skip to content

fix(client): the four packages READ members declare the stage their door is declared at (#17536) - #19323

Merged
os-project-manager merged 7 commits into
mainfrom
claude/issue-17536-client-either-stage-widening
Sep 20, 2026
Merged

os-project-manager merged 7 commits into
mainfrom
claude/issue-17536-client-either-stage-widening

Conversation

@os-project-manager

@os-project-manager os-project-manager commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #17536

Clause-②: yes

The four packages READ members of @objectstack/client — ObjectStackClient.packages.list / .get and their ScopedEnvironmentClient twins — returned InstalledPackage, the AUTHORING manifest stage, while both read doors have been declared at EITHER stage since PR #17517. A response the server is declared able to send was one this SDK's own types said could not arrive.

packages/spec is the one contract between producers and consumers (Prime Directive #12) and packages/client is a consumer of it, so the consumer's declaration is what moves. All four now declare InstalledPackageAtEitherStage from @objectstack/spec/api.

Direction ruled by triage at 5735293092, taking ① widen the client. ⛔ Narrowing the read door was excluded by rule, not by preference: AGENTS.md rule 13 makes reversing PR #17517's landed decision a new ADR, which is domain:spec's work and not this card's. ⛔ Nothing under packages/spec is touched here.

The semver level, and the reading behind it

⚠️ The dispatch carried a reserved exit: measure the level honestly, and if it needs major, stop and report rather than land. It was measured, both legs, on this branch.

leg command exit what it says
major on @objectstack/client node scripts/check-changeset-no-major.mjs --base origin/main 1 ⛔ REFUSED — "Every publishable package is in the Changesets fixed (lockstep) group, so a single major promotes the ENTIRE monorepo… During the launch window ship breaking changes as minor instead."
minor on @objectstack/client (what this PR carries) same command 0 accepted

⇒ This does not need a major; it needs a minor plus the two carriers that stand in for the level during the launch window. check-changeset-no-major.mjs's own header states the convention and its end condition: "During the launch window it is NOT the carrier… The mandatory information carriers for breaking-ness in the meantime are the BREAKING banner the author writes in the changeset body and the ADR-0087 migration-ledger disposition… THIS GUARD IS WHAT GETS DISARMED AT GA." The window is open on this tree: there is no .changeset/pre.json, so the RC exemption is not in force and the guard is armed — which is what the exit-1 leg above demonstrates rather than assumes.

⛔ The level was not lowered to quiet a gate, and the breaking-ness was not dropped to reach a lower level. Both carriers are present and both were driven:

  • the changeset body carries a BREAKING banner;
  • node scripts/check-adr-0087-registration.mjs --base origin/main exits 0 and reports [BREAKING] not-required (no-migration-prescription). type-surface-only is deliberately NOT claimed, and the changeset writes out why it is unavailable on the merits: that category is for a published TYPE-surface NARROWING moving off an erased type, and at the merge base all four members carried a concrete annotation.

Under strict semver this is breaking, and the changeset says so in words. What the launch-window convention decides is only which field carries that fact.

What a consumer pays, measured

The element is a union of two whole, CLOSED stages that differ in exactly one key, manifest. Every other member of the row — id, name, version, status, enabled, installedAt, … — is common to both branches and reads exactly as before, so code that touches only those needs no change. Code that reaches INTO manifest separates the stages first: the authoring stage's objects are glob STRINGS, the assembled stage's are object DEFINITIONS, and the compiler now says so at the call site instead of letting a glob-shaped read compile against a row carrying definitions.

In this repository the consumer cost is zero sites outside packages/client itself. No other workspace package calls either read member; the only in-tree mentions are packages/client/README.md (a bare await client.packages.list(); with no member read, type-checked green by check:skill-examples against the rebuilt declarations) and the pins in this PR.

The three WRITE members did not move. install / enable / disable answer the row their own request contract produced — PackageInstallRequestSchema declares manifest: ManifestSchema — and PR #17517 moved the read doors alone. That asymmetry is a measurement, not an oversight, and it is pinned.

Tests — type-level, with a control that can fail

packages/client/src/return-type-precision.test.ts is the only place a return-type move CAN be pinned: nothing about the runtime values changed, so a runtime test is green either way.

  • direction 1 — all four read members now ADMIT an assembled-stage row;
  • the control — the same assignment against the unmoved packages.install is a used @ts-expect-error. If AssembledInstalledPackage were assignable to InstalledPackage after all, that suppression would go unused (TS2578) and direction 1 would be exposed as passing vacuously;
  • direction 2 — a manifest belonging to NEITHER stage is still refused, so this line reddens if anyone ever "widens" these members to any, unknown or an open shape.

Ablation (restore packages/client/src/index.ts to blob c12b554d20 — the blob it carried before 21e6b9887c, a fixed anchor rather than the moving origin/main, keep the test file, run tsc --noEmit -p tsconfig.test.json): exit 2, ten errors — the five toEqualTypeOf pins naming the union (TS2344) and five TS2322: the four direction-1 assignments plus the new #19324 gap pin. Zero TS2578 — both controls stay USED in both states. The restore was verified by blob identity against HEAD with git diff HEAD empty; both @ts-expect-error controls stay USED in the ablated run, which is why they are labelled GREEN IN BOTH STATES rather than offered as evidence.

Gates

Derived from the actual diff with node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack on the merged head, every exit code captured to disk before any pipe, and reconciled with --ran:

58 derived, 58 run, 0 NOT-MEASURED, 0 UNRUN  — all 58 exit 0
pnpm --filter '@objectstack/client^...' build   :: exit 0
pnpm --filter @objectstack/client typecheck     :: exit 0  (tsc + check:test-typecheck, 0 errors)
pnpm --filter @objectstack/client test          :: exit 0  (48 files / 566 tests)
pnpm lint                                        :: exit 0  — the FULL unnarrowed repo union, at 4fe43c0c0f

Two of the 58 first answered PREREQUISITE NOT MET (check:skill-examples, check:dual-build-cjs-loads), which is NOT MEASURED and not a pass: both read built output and the packages had no dist. They were built and re-run, and both then exited 0 on the merits — check:skill-examples type-checks 258 prose examples across three surfaces, including the client SDK's 23 blocks, against the rebuilt declarations.

origin/main was merged once (e3b3cdd2df) before this PR opened; build state was refreshed and the whole derived set plus the lint union were re-run on the merged head.

Acceptance notes

⚠️ One finding, in packages/spec, reported rather than filed or fixed — this lane does not touch that package and does not file cards. Measured on this branch, and it bears on how much the union actually buys a consumer:

AssembledInstalledPackage's manifest resolves to an index-signature type (Record of string to unknown) in the PUBLISHED TypeScript type, not to the assembled body's declared shape. The cause is the cast in packages/spec/src/api/package-api.zod.ts: AssembledPackageRecordBodySchema is built on (AssembledPackageBodySchema as unknown as z.ZodObject(z.ZodRawShape)), whose z.input is an index signature. Driven with tsc: InstalledPackage IS assignable to AssembledInstalledPackage (an authoring manifest satisfies an index signature), so the assembled arm absorbs the authoring arm at the type level, and a member read off the assembled stage's manifest arrives as unknown rather than its declared type. The runtime Zod parse is unaffected — the schema still checks the assembled body member by member, exactly as its own docblock says. This is the same failure family check:exported-any exists for ("the snapshot records that an export exists, never what it resolves to"), reached by a different spelling. Dedupe words: AssembledPackageRecordBodySchema, ZodRawShape, package-api.zod, assembled manifest type erosion, z.input index signature.

noted, not filed: @objectstack/client re-exports neither InstalledPackage nor InstalledPackageAtEitherStage, so a consumer writing an explicit annotation reaches into @objectstack/spec/api for it. That is unchanged by this PR — the SDK has never re-exported the package row — and adding a re-export would be a new published export on a package this card is only correcting. Carrier: none; no queued PR or lane touches this surface.

noted, not filed: the stale comment triage measured on packages.get (it claimed GetInstalledPackageResponseSchema is data: InstalledPackageSchema) is corrected in this PR rather than filed, per that ruling's explicit instruction. It is corrected in place with the reading that falsified it, not deleted.


Generated by Claude Code


Generated by Claude Code

…r is declared at

`packages.get` / `packages.list` — global and environment-scoped, four read
members in all — returned `InstalledPackage`, the AUTHORING manifest stage,
while `ListInstalledPackagesResponseSchema.packages` and
`GetInstalledPackageResponseSchema.data` have been declared
`InstalledPackageAtEitherStageSchema` since PR #17517. A response the server is
declared able to send was one this SDK's own types said could not arrive.

`packages/spec` is the one contract and `packages/client` is a consumer of it,
so the consumer's declaration moves. The three WRITE members keep
`InstalledPackage`: their own request contract declares `manifest:
ManifestSchema`, and PR #17517 moved the read doors alone.

Also corrects a comment measured stale on the same member: it claimed
`GetInstalledPackageResponseSchema` is `data: InstalledPackageSchema`.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf
…trol

Direction 1 pins that all four read members now admit an assembled-stage row;
the `@ts-expect-error` on `packages.install` is the same assignment against the
unmoved write door and is what makes direction 1 non-vacuous. Direction 2 pins
that the union is two CLOSED stages: a `manifest` belonging to neither is still
refused, so a later widening to `any` / `unknown` reddens here.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 20, 2026
@github-actions

github-actions Bot commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/environment-routing.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))
  • content/docs/concepts/north-star.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))
  • content/docs/deployment/publish-and-preview.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))
  • content/docs/deployment/single-project-mode.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))
  • content/docs/protocol/kernel/http-protocol.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))
  • content/docs/ui/forms.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/implementation-status.mdx (via /environments/:environmentId (route, a path literal in a comment in packages))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see

Coarse fallback — 15 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 13d52947d81aca235133ee9619d80723a7c63348 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 6c5485d1ad793432fba20565ada522864d1ac826 — the merge of head 4fe43c0c0f9f8687d77886777ff9d786d504c47a into base 13d52947d81aca235133ee9619d80723a7c63348, 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 6c5485d1ad793432fba20565ada522864d1ac826 && git checkout 6c5485d1ad793432fba20565ada522864d1ac826
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 13d52947d81aca235133ee9619d80723a7c63348 4fe43c0c0f9f8687d77886777ff9d786d504c47a && git checkout -B drift-repro 13d52947d81aca235133ee9619d80723a7c63348 && git merge --no-ff 4fe43c0c0f9f8687d77886777ff9d786d504c47a

node scripts/docs-audit/affected-docs.mjs --json 13d52947d81aca235133ee9619d80723a7c63348

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

Copy link
Copy Markdown
Collaborator Author

Served-tier: CONTRACT_REVIEW_TIER

Contract review

Head 39ba6e6171653b18fa1da8fc5f8be75c3281af8e (branch claude/issue-17536-client-either-stage-widening, base main, draft; 3 files, +250/−25). Isolated at-tier review of record: the ruling at 5735293092 (① widen the client) was read at source and is ⛔ not re-opened here; this review judges whether the diff implements it faithfully and what it costs. Every reading below was taken on a private clone of the head sha in this seat's scratchpad (merge-base with origin/main 81e12e18 = e3b3cdd2, identical to the API compare), with packages/spec and packages/core built from source (tsup + DTS) so the client resolves the spec through its real exports map, and tsc 6.0.3 under the package's own tsconfig.test.json strictness. The shared checkout was not written to.

① Derived judgments

Scope — count right, only READ members moved. At head, packages/client/src/index.ts declares InstalledPackageAtEitherStage on exactly four members: ObjectStackClient.packages.list (:2488), .get (:2557), ScopedEnvironmentClient.packages.list (:7955), .get (:8004) — 12 occurrences of the symbol at head against 0 on origin/main. The four write-side members still declare Promise<InstalledPackage>: install (:2594), enable (:2629), disable (:2647), update (:2673); their request contract is PackageInstallRequestSchema.manifest: ManifestSchema (package-api.zod.ts:279), so keeping them at the authoring stage is right. The card's triage appendix measured four bindings across two surfaces; the diff moves those four and no other. Accept-set: unchanged — the diff touches return annotations, the matching unwrapResponse / _unwrap generics and docblocks only; no request parameter, no new export from @objectstack/client, nothing under packages/spec.

Public-surface movement, each pinned. ObjectStackClient.packages.list element → pin :320 (TS2344 under ablation); .get → :602; ScopedEnvironmentClient.packages.list → :207; .get → :356 and :604; the four direction-1 admissions → :705 :706 :707 :709 (TS2322 under ablation); the write side's non-movement → the install @ts-expect-error control, used in both states.

Test controls — they can fail, measured. Ablation reproduced: src/index.ts restored to the merge-base blob c12b554d (0 hits of the widened symbol), pin file kept, tsc --noEmit → exit 2, nine errors in the pin file (TS2344 ×5 at :207/:320/:356/:602/:604, TS2322 ×4 at :705–:709) and 0 TS2578, so both suppressions stay used in both states (the PR body's 696–700 are line numbers from an earlier commit; cosmetic). Control-can-fail, driven three ways: (i) a member declared at the union makes the install-shaped suppression go unused — TS2578 fires (probe P7); (ii) AssembledInstalledPackage → InstalledPackage is refused (P2), so the install control is used for the reason its docblock states; (iii) get widened to Promise<any> in the worktree fires TS2578 at :743 (the direction-2 suppression) and at :644 (an older #12034 pin). At head the pin file contributes 0 errors.

The open finding #19324 holds — driven, not relayed — and it reaches into this diff's text. Probes at head, published declarations:

probe result
P1 const x: AssembledInstalledPackage = authoringRow compiles — the assembled arm absorbs the authoring arm
P8 const x: AssembledInstalledPackage = eitherStageRow compiles — the whole union assigns to the assembled arm alone
P4 type of AssembledInstalledPackage['manifest'] Record<string, unknown>
P3 type of either.manifest.objects unknown
P9/P10/P11 { ...authoringRow, manifest: { bogus: 1, objects: 'not-even-an-array' } } against the union AND against Awaited<ReturnType<typeof client.packages.get>>; also manifest: {} all compile — the declaration admits ANY object-shaped manifest
P5 if (Array.isArray(pkg.manifest.objects)) {…} else {…} pkg.manifest is the SAME union in BOTH branches — narrows nothing
P6 return pkg.manifest.objects as string[] | undefined error, but as unknown is not assignable, ⛔ not as a stage discrimination
runtime control, built spec: InstalledPackageAtEitherStageSchema.safeParse(bogusRow).success false (authoring-shaped row: true) — the Zod union IS strict; only the TYPE is tolerant

Root cause is deeper than the cast #19324 names: AssembledPackageBodySchema is annotated z.ZodType<Record<string, unknown>, Record<string, unknown>> at packages/spec/src/stack.zod.ts:1283 deliberately (#14513 — TS7056 and the declaration-chunk/heap ceiling; its docblock reads «What a consumer loses is the static field typing inside an assembled body»). The cast in package-api.zod.ts only carries that decision through. ⛔ Not this PR's to fix, and #19324's owner should read stack.zod.ts:1283 before treating the cast as removable.

What that does to the diff. The widening itself is implemented faithfully — the four declarations now name the type the two doors are declared at, which is what the ruling ordered. But four passages in the diff claim what the published type does not deliver, and the same PR's own acceptance notes record the erosion:

  1. Changeset: «A row belonging to neither stage is refused by both branches and therefore by the declaration; ⛔ this is not a tolerant shape» — false at the type level (P9–P11); true only at runtime.
  2. Changeset and the list docblock: «the compiler now says so at the call site», the assembled stage's objects being «object DEFINITIONS» — the compiler refuses a glob-shaped read by unknown (P6), not by stage, and the assembled arm carries no definition type at all.
  3. ObjectStackClient.packages.list docblock prescribes Array.isArray(pkg.manifest.objects) ? … : … to separate the stages — it narrows nothing (P5), and it cannot separate them at runtime either: both stages' objects are arrays (kernel/manifest.zod.ts:423 z.array(z.string()) vs stack.zod.ts:316 z.array(ObjectSchema)). That is a shipped .d.ts docblock prescribing a discriminator that does not discriminate.
  4. Test docblock on direction 2: «a manifest belonging to NEITHER stage is refused by both branches … reddens if these members are ever widened to any / unknown / an open shape» — the pin refuses a string primitive only; the declaration is already an open shape for object manifests.

During the launch window the changeset body is, in the gate's own words, «the only signal there is», and it ships as CHANGELOG.md, where a factual error later needs a dedicated docs-only PR (AGENTS.md, Documentation Guardrails). Keep the claim as narrow as what is enforced.

② Semver level and changeset

.changeset/17536-client-packages-read-doors-either-stage.md: "@objectstack/client": minor, Clause-②: yes, BREAKING banner, <!-- adr-0087: not-required (no-migration-prescription) … -->.

⚠️ The level and carriers are right; the changeset body is what ① faults — its consumer-facing paragraph describes a type-level discrimination the published type does not provide.

③ Boundary flags

  • CI, both layers, at 39ba6e61. Check runs collapsed latest-per-name: all seven required contexts success (Lint & Repo Gates was in_progress on my first read and completed success at 11:48:12Z). Check suites: 13 github-actions suites completed success/skipped, 0 failed; 4 third-party app suites (vercel, fly-io, claude, cloudflare-workers-and-pages) sit queued with 0 runs — not a pass, not a failure, not required. main has moved since the PR's base (596090ef → 81e12e18, behind by 9); the queue rebuilds on landing.
  • NOT MEASURED by this seat: the full pnpm lint union and the client's own dist/check:exported-any-returns (I built spec and core only; Lint & Repo Gates, Build Core and TypeScript Type Check are green in CI and stand in).
  • Governed surfaces: none touched (3 paths, all under packages/client/ and .changeset/). Not Tier H, not Tier S.
  • [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324: its cause attribution should be corrected by its owner (see stack.zod.ts:1283, feat(cli+spec): compile a project of N packages into one packages[] artifact, with the assembled package body declared (ADR-0130 D4 producer, #14242 B) #14513) — reported here, ⛔ not filed by this seat, and ⛔ not this PR's fix.
  • Shared identity: the PR body footer, the dev report and the claim all carry the session id this review is instructed to sign; recorded as the accepted blind spot AGENTS.md names, not judged.
  • Docs Drift Check listed 6 pages via a route literal inside a docblock — advisory noise; no doc change is owed by this diff.

Remedy (narrow, text-only, no packages/spec change)

Rewrite the changeset's «What the union is / what a consumer does» paragraphs, the ObjectStackClient.packages.list and .get docblocks, and the direction-2 docblock in return-type-precision.test.ts to say what the union buys today: on the assembled arm manifest is Record<string, unknown> (spec's deliberate #14513 annotation; #19324 tracks it), so a read into manifest on the union is unknown; narrow with a packages/spec parse (InstalledPackageSchema.safeParse / AssembledInstalledPackageSchema.safeParse), ⛔ not with Array.isArray. Either drop the «not a tolerant shape» claim from the type-level statements or add a pin that records the object-tolerance as the KNOWN gap tied to #19324 (it reddens the day spec repairs it). Then re-review on the new head.

Implemented-by: claude/issue-17536-client-either-stage-widening
Reviewed-by: session_01QCdUBjM47SxioST9z5Zwdf

Verdict: FAIL — the four declarations move exactly as ruled and the level is the convention correctly applied, but the diff ships changeset and docblock text asserting a stage discrimination and a closed shape that the published type measurably does not provide, with the PR's own notes recording why. Fix the text, keep the types, re-review.


Generated by Claude Code

… type-level gap (#17536)

The at-tier contract review of PR #19323 FAILed this diff on its TEXT, not its
types: four passages asserted a stage discrimination and a closed shape the
published declaration measurably does not provide. The declarations stay exactly
as ruled; the prose around them is corrected, and the gap it used to hide is
pinned.

Measured at this head with `tsc` against the built spec, in a private worktree:

  AssembledInstalledPackage['manifest']            Record<string, unknown>
  (union).manifest.objects                         unknown
  { ...authoringRow, manifest: { bogus: 1, objects: 'not-even-an-array' } }
      against the union AND against
      Awaited<ReturnType<typeof client.packages.get>>          compiles
  `if (Array.isArray(pkg.manifest.objects))`       same union in BOTH branches
  manifest: 'com.acme.crm@1.0.0' (a string)        refused by both branches

  runtime control, built spec:
  InstalledPackageAtEitherStageSchema.safeParse(bogusRow).success   false
  ... .safeParse(rowWithNoObjects) against each stage schema        both true

So: the runtime parse is strict, the TYPE is tolerant of any object manifest,
and `Array.isArray` separates the stages on neither level — at runtime both
stages' `objects` are arrays.

- changeset: drops the "not a tolerant shape" and "the compiler now says so"
  claims; states the runtime/type asymmetry, names #19324 and its root cause
  (`packages/spec/src/stack.zod.ts:1283`, #14513), and replaces the prescribed
  discriminator with a worked `packages/spec` parse.
- `ObjectStackClient.packages.list` docblock: same correction, at the door a
  consumer actually reads; `Array.isArray` is now the documented wrong answer.
- the import-site comment: the closedness belongs to the RUNTIME declaration.
- direction-2 test docblock: says what that pin measures — a string primitive —
  and what it does not.
- new pin `objectToleranceGap19324`: the object tolerance recorded as the
  behaviour it is, with no suppression, so tsc reds on it the day #19324 closes.
- the WRITE-member count was wrong in three places: four members stayed, not
  three (`install`, `enable`, `disable`, `update`).

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf
…17536)

Adding the gap pin changes the count the docblock states, so the count is
re-driven rather than left stale: index.ts restored to blob `c12b554d20` (the
blob it carried before 21e6b98 — a fixed anchor, not a moving `origin/main`),
`tsc --noEmit -p tsconfig.test.json` → exit 2, TEN errors (TS2344 x5, TS2322 x5)
and 0 TS2578, so both controls stay used in both states. The restore leg was
proved by hash: the file came back to `7f819e89ae` with `git diff HEAD` empty.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QCdUBjM47SxioST9z5Zwdf

Copy link
Copy Markdown
Collaborator Author

Patch round pushed — the review's FAIL was text-only, and so is the fix

New head 4fe43c0c0f9f8687d77886777ff9d786d504c47a (previous 39ba6e6171653b18fa1da8fc5f8be75c3281af8e), still draft. Nothing under packages/spec; no declared return, no test-pin type, no changeset level touched. No label, assignee or ready-flip write.

The four passages named in the review of record (5749642796) now say what the union buys today:

  1. changeset, "what the union is" — the "not a tolerant shape" claim is dropped; the runtime/type asymmetry is stated instead, with the measurement (strict parse, tolerant type) and with [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324 and its root cause named.
  2. changeset, "what a consumer does" — "the compiler now says so" is replaced by the honest reading: a manifest member read off the union is unknown, so a caller narrows by PARSING the row with a packages/spec schema. Worked example included.
  3. ObjectStackClient.packages.list docblock — the prescribed Array.isArray(pkg.manifest.objects) discriminator is now the documented WRONG answer, with the reason it separates nothing on either level (same union in both branches; both stages' objects are arrays at runtime). This is the row that reaches a user, so it carries the worked parse.
  4. direction-2 test docblock — says what that pin measures, a string primitive, and what it does not.

Taken up as well: the optional pin. objectToleranceGap19324 records the object tolerance as the behaviour it is, deliberately with no suppression, so tsc reds on that line the day #19324 closes. The ablation was re-driven because the pin changes its count — restored to the fixed blob c12b554d20 rather than a moving origin/main, exit 2, ten errors (TS2344 x5, TS2322 x5), zero TS2578, so both controls stay used in both states.

Two text defects of the same class were corrected in passing, both in these three files: the import-site comment repeated the "never a tolerant shape" overclaim, and the WRITE-member count read "three" in three places when four members stayed (install, enable, disable, update).

Gates at the new head: all 58 derived families run with each exit code captured to disk before any pipe and reconciled with --ran (58 run, 0 NOT-MEASURED), plus pnpm lint as the full unnarrowed repo union — all exit 0. Two families first answered PREREQUISITE NOT MET and went green after their build prerequisites were built.

Full report, including the verbatim replacement text for every passage: 5749910797 on #17536. The PR body was deliberately not patched — two stale spots in it are itemised there for the seat.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Served-tier: CONTRACT_REVIEW_TIER

Contract review

Head-sha: 4fe43c0c0f9f8687d77886777ff9d786d504c47a

PR #19323 · card #17536 · branch claude/issue-17536-client-either-stage-widening · base main, merge-base 74fb2f7a80 · draft · 3 files, +350/−25. Isolated at-tier review of record for THIS head. The previous record (5749642796, FAIL, bound to head 39ba6e6171) was read for the passages it challenged and is not inherited; the ruling at 5735293092 (① widen the client) is the direction and is not re-opened.

Notation: angle brackets are written as square brackets throughout (Record[string, unknown] stands for the source's angle-bracket spelling) because the comment sanitizer eats tag-shaped fragments.

Where every reading was taken: a private worktree at 4fe43c0c0f under this reviewer's scratchpad (the shared checkout was not written to), pnpm install --frozen-lockfile --offline, @objectstack/spec and @objectstack/core built from source at that head (tsup + DTS), tsc 6.0.3 under packages/client/tsconfig.test.json strictness (a scoped program of src/index.ts + src/return-type-precision.test.ts + one probe file, same compiler options), node v22.22.2 for the runtime parses against the built dist. Nothing in the implementer's round report (5749910797) was taken on trust; every number below is this reviewer's own.

① Derived judgments

Scope — the load-bearing claim holds: text-only, plus one pin. git diff --name-status 74fb2f7a80..4fe43c0c0f: A .changeset/17536-client-packages-read-doors-either-stage.md, M packages/client/src/index.ts, M packages/client/src/return-type-precision.test.ts; -- packages/spec matches 0 paths. The patch round (39ba6e6171..531e1225d6) is +142/−42 in those same three files. Judged right.

Public surface, measured with an exact-type predicate (an Equals conditional type, not mere assignability):

  • the four READ members declare exactly the union: client.packages.get and scoped.packages.get ≡ InstalledPackageAtEitherStage; client.packages.list and scoped.packages.list ≡ { packages: InstalledPackageAtEitherStage[]; total: number } (envelope unchanged) — right, this is what the ruling ordered and what ListInstalledPackagesResponseSchema.packages (package-api.zod.ts:212) and GetInstalledPackageResponseSchema.data (:236) are declared at;
  • the four WRITE members are untouched: install, enable, disable, update ≡ Promise[InstalledPackage] — right (PackageInstallRequestSchema declares manifest: ManifestSchema, package-api.zod.ts:279); uninstall answers { id, success, message? } and is correctly not counted;
  • accept set unchanged: no request parameter moved; @objectstack/client re-exports neither InstalledPackage nor the union (0 export hits for either name), as the changeset states;
  • in-repo consumer cost: 0 call sites of .packages.list( / .packages.get( outside packages/client (the two packages/runtime hits are docblock mentions); packages/client/README.md:250 is a bare await client.packages.list(); with no member read.

Every shipped measurement re-driven — each one matched:

shipped claim (changeset / list docblock / pin file) reading at 4fe43c0c0f
the assembled arm's manifest is Record[string, unknown] exact (Equals) — true
pkg.manifest.objects on the union is unknown exact — true, both as an indexed type and on a declared value
a manifest of { bogus: 1, objects: 'not-even-an-array' } and of {} COMPILE against the declared return compile against all four read members and against the bare union (6 assignments, 0 errors)
Array.isArray(pkg.manifest.objects) separates the stages on NEITHER level type: pkg.manifest ≡ the same union in both branches, and an assignment to the authoring Manifest is refused in both (both suppressions used); runtime: the authoring objects parses to an array of strings, the assembled to an array of objects — both arrays
InstalledPackageAtEitherStageSchema.safeParse refuses a neither-stage row built spec: the bogus manifest → success: false; {} → false; a string manifest → false; a stage-clean authoring row → true; a stage-clean assembled row → true
a row with no objects parses as either stage InstalledPackageSchema → true, AssembledInstalledPackageSchema → true, the union → true
the worked parse-based narrowing separates the stages authoring row vs AssembledInstalledPackageSchema → false (manifest.objects.0: expected object, received string); assembled row vs InstalledPackageSchema → false (expected string, received object)
a string manifest is refused by both branches (the direction-2 pin) refused; suppression used
AssembledInstalledPackage is not assignable to InstalledPackage (why the install control is used) refused; and the reverse assigns — the authoring arm, and the whole union, assign to the assembled arm — the erosion the acceptance notes describe
ablation: index.ts at blob c12b554d20, exit 2, ten errors, TS2344 x5, TS2322 x5, 0 TS2578 git restore --source=21e6b9887c^ → blob c12b554d20, 0 hits of the widened symbol; tsc exit 2, 10 errors: TS2344 at :207 :320 :356 :604 :606, TS2322 at :723 :724 :725 :727 (direction 1) and :787 (the gap pin); 0 TS2578; restored to blob 7f819e89ae, git status --porcelain clean apart from this reviewer's probe files
the direction-2 suppression reddens if a member is widened to any client.packages.get widened to Promise[any] → TS2578 at :762 (direction 2) and at :646 (the #12034 .package pin), TS2344 at :604; restored by blob
objectToleranceGap19324 reddens the day the assembled arm is typed control: against a simulated union whose assembled arm is a closed shape, the same literal is refused (suppression used) — the pin is capable of failing, it carries no suppression at head, so that red will surface as the comment says
root cause packages/spec/src/stack.zod.ts:1283, #14513, TS7056 line 1283 is export const AssembledPackageBodySchema: z.ZodType[Record[string, unknown], Record[string, unknown]]; its docblock names #14513 and TS7056 and the declaration-chunk/heap ceiling
objects is z.array(z.string()) at the authoring stage and z.array(ObjectSchema) at the assembled one kernel/manifest.zod.ts:423; stack.zod.ts:316 inside STACK_DEFINITION_COLLECTIONS_SHAPE, which assembledPackageBodyShape() (:1167) picks from
type-surface-only predicate 4 narrowed-from-erased reads the base annotation through isErasedType check-adr-0087-registration.mjs:2876, :2920, :3283
.changeset/pre.json absent, guard armed absent at the head; the major leg in ② demonstrates it

The two in-passing corrections — in scope. (a) The import-site comment in index.ts repeated «never a tolerant shape»; that is one of the type-level statements the previous record's remedy said to drop, in a file already in the diff — the same defect, not a new one. (b) «three» WRITE members → «four»: measured above, four members answer Promise[InstalledPackage]; the count was wrong in the diff's own prose and is corrected only in prose. Neither touches a declaration, a pin type, a new file or packages/spec; both are declared as deviations in the round report. Judged right. (The pre-existing #12034 docblock at return-type-precision.test.ts:555 still says «The three packages WRITE verbs» — that is #12034's own statement about the three it bound, not this card's text.)

Two precision notes, non-blocking — no shipped sentence is false, but a reader should know exactly what is measured:

  1. The pin-file docblock says the runtime refusal of «that same row» «is pinned beside its producer in packages-read-delete-response-conformance.test.ts». That test (:264) pins a NEITHER-stage row whose objects MIXES a glob with a definition, through the real door — the same class, not the literal { bogus: 1, objects: 'not-even-an-array' } row. The literal row's runtime refusal is measured here and in the round report and pinned nowhere; the class refusal is pinned there. Acceptable as written; a later touch could say «a neither-stage row (a mixed objects array)».
  2. The worked example's assembled branch comments parsed.data.manifest — the ASSEMBLED stage, object definitions. That is a runtime statement: measured, parsed.data.manifest after AssembledInstalledPackageSchema.safeParse is still Record[string, unknown] and parsed.data.manifest.objects is still unknown (the same [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324 gap, post-parse), while the authoring branch's authoring.manifest.objects is typed string[] | undefined. The surrounding text already says the type cannot carry the narrowing today, so this is not an overclaim; it is the one place a consumer could expect static typing the assembled arm does not have. Worth one clause when the gap pin is next touched; not a reason to fail this head.

② Semver level

.changeset/17536-client-packages-read-doors-either-stage.md: "@objectstack/client": minor, Clause-②: yes, a BREAKING banner, and adr-0087: not-required (no-migration-prescription).

③ Boundary flags

  • CI at 4fe43c0c0f (check runs read after 13:08Z, all completed): all seven required contexts success — Lint & Repo Gates (completed 13:08:25Z), TypeScript Type Check, Test Core, Dogfood Regression Gate, Build Core, Temporal Conformance (live PG + MySQL), Governed Surface Queue Guard; no check run concluded failure; Console Pin Gate, Build Docs and Packed-tarball smoke (opt-in) skipped by path filter.
  • Governed surfaces: none of the three paths is one (packages/client/**, .changeset/**). Not Tier H, not Tier S.
  • Not measured by this seat: the full pnpm lint union and the devDependency dists (hono, runtime, rest, …) — my full tsconfig.test.json run therefore reports 86 TS2307/TS7006 errors in OTHER files, all module-not-found for packages I did not build, and 0 in index.ts and 0 in the pin file; CI's green TypeScript Type Check and Lint & Repo Gates stand in for the whole program.
  • Dev flags / open_questions (round report 5749910797): (1) the PR body's Ablation paragraph is stale in two ways (anchor origin/main adf4b18777 and count «nine») — answered: the docblock and the pin file carry the correct measurement (blob c12b554d20, ten, 0 TS2578); the body is the seat's to edit and is not a landing blocker; (2) the body's Gates block names 39ba6e6171 — same disposition, re-sha when next touched. Neither is escalated.
  • Out-of-scope findings the round recorded (the union's branches are not mutually exclusive for a row without objects — confirmed above; two gates answer PREREQUISITE NOT MET until a build exists; [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324's cause attribution corrected at 5749650656) — read, none bears on this head, none filed by this seat.
  • [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324 stays open and is correctly not this PR's to fix; the gap pin gives its closer a red to find.
  • Shared identity: the claim, the PR and this record's Reviewed-by all carry the seat's session; the implementation identity is the branch (mode:subagent), so this is not a SELF-REVIEW pair.

Implemented-by: claude/issue-17536-client-either-stage-widening
Reviewed-by: session_01QCdUBjM47SxioST9z5Zwdf

VERDICT: PASS — the four declarations are exactly what the ruling ordered and what the doors are declared at; every measurement the shipped text now makes was re-driven at this head and matched; the level is the launch-window convention correctly applied with both carriers present; the two in-passing corrections are within the diff's own prose. Two non-blocking precision notes are recorded in ① for the next touch.


Generated by Claude Code

Copy link
Copy Markdown
Collaborator Author

Clause-② provenance — at-tier review of record PASS at head 4fe43c0c0f9f8687d77886777ff9d786d504c47a; both carriers cleared

⛔ This is NOT a clearance to land. It records which record cleared which gate, on which head.

record comment 5750018962, 2026-09-20T13:12:40Z
head judged 4fe43c0c0f9f8687d77886777ff9d786d504c47a
verdict PASS
carriers needs:contract-review removed from card #17536 and PR #19323 in that order; check-clause2-carriers.mjs --pair 19323 exits 0, row C6-RECORD naming 5750018962 on this head

Why this record exists at all, and why the previous clear did not survive

The gate was cleared once already, at 11:58:08Z. Then the head moved 39ba6e6171 → 4fe43c0c0f. check-clause2-carriers --pair 19323 caught that as exit 4 / row C3, in its own words: "the review that cleared this gate judged a different tree, so the clear no longer covers what would land."

⭐ A review of record binds to the head it judged — not to the PR. The re-hang is the seat's act (the checker refuses to write a gate label, correctly: hanging or clearing one from a checker would be 自查放行). Carriers were re-hung at 12:48Z and a fresh isolated reviewer was commissioned against the new head.

Precondition ① was read from the record itself

The checker states plainly that it reports the record's existence, ⛔ never its verdict — "whether it reads PASS is precondition ① of the landing check and stays human." So it was read directly: first line is Served-tier: CONTRACT_REVIEW_TIER (the constant name, ⛔ not a model id), ## Contract review present, head sha as a code span, line-initial Implemented-by: and Reviewed-by:, no model identifier anywhere in 13737 bytes, and **VERDICT: PASS**.

Tier

CONTRACT_REVIEW_TIER = claude-fable-5-1, re-derived from scripts/pm/dispatch-gates.mjs:11930 at origin/main rather than recalled. This seat serves claude-opus-5 on both session_context.model and last_served_model ⇒ under tier ⇒ the review was commissioned as an isolated at-tier subagent and ⛔ was not self-reviewed. The reviewer was instructed to re-drive every reading rather than repeat the implementer's numbers; it reports doing so in its own worktree, and its ablation independently reproduced ten errors (TS2344 ×5, TS2322 ×5, zero TS2578) — the same count the patch round measured.

⚠️ One correction to the record, not affecting the verdict

The record's ③ disposition reports the PR body's Ablation paragraph and Gates sha as stale. They were, when the reviewer read them — but they were patched at ~13:00Z, before the record posted at 13:12:40Z. Verified on the live body just now: **ten** errors present, **nine** errors absent, fixed blob anchor c12b554d20 present, moving anchor adf4b18777 absent, at 4fe43c0c0f present, at 39ba6e6171 absent.

⇒ ⛔ nothing is owed on that item; a later reader should not chase it. The reviewer itself scoped it correctly as the seat's, not a landing blocker.

Two non-blocking findings the record raised — carried, ⛔ not silently dropped

Both are recorded on card #17536 rather than actioned here. The reviewer judged both non-blocking with reasoning, and ⛔ this seat serves under tier and does not overturn an at-tier judgement to add a round.

  1. the pin-file docblock says a refusal is "pinned beside its producer" — the cited test pins the class (a neither-stage row with a mixed objects array), ⛔ not the specific literal whose runtime refusal is measured but pinned nowhere;
  2. the worked example's assembled branch reads as a type statement where it is only a runtime one — post-safeParse, parsed.data.manifest is still Record<string, unknown> and .objects still unknown, because the [finding] AssembledInstalledPackage.manifest erodes to an index-signature type in the published .d.ts, so the assembled arm absorbs the authoring arm #19324 gap survives the parse.

⭐ Both are gated on the same future moment, and the gap pin's own comment already names it: when #19324 closes, tsc reds on that pin, and whoever answers that red tightens this guidance. ⇒ that is the right carrier for both.


Generated by Claude Code

@os-project-manager
os-project-manager marked this pull request as ready for review September 20, 2026 13:16
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 20, 2026
Merged via the queue into main with commit be7382d Sep 20, 2026
50 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-17536-client-either-stage-widening branch September 20, 2026 13:48
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

2 participants