Skip to content

packages.install/enable/disable declare the bare row the only serving surface sends; get's fork measured, not converged - #13060

Merged
os-litant merged 1 commit into
mainfrom
claude/issue-12034-packages-envelope-declarations
Aug 29, 2026
Merged

packages.install/enable/disable declare the bare row the only serving surface sends; get's fork measured, not converged#13060
os-litant merged 1 commit into
mainfrom
claude/issue-12034-packages-envelope-declarations

Conversation

@os-litant

@os-litantos-litant commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Part of #12034

⚠️Part of, not a closing keyword — deliberately, please read before merging. The dispatch instruction for this card asked for a closing reference. I have used Part of instead, because the dispatching ruling also split this card in half and shipped only one half: install / enable / disable are corrected here, and get is measured-only by explicit instruction. A closing keyword would retire the card on merge, and the unshipped get half — which the maintainer's Option-A ruling of 2026-08-26 does cover — would vanish from the open backlog, where inbox filters read open only. So this card remains open on purpose. If PM/maintainer would rather retire it here and carry the get convergence on a fresh card, swap this first line for a closing reference before merge.

What shipped

client.packages.install, .enable and .disable declared { package: any; message?: string } — a body no surface has ever emitted. This was not an erasure, it was a falsehood, and the any member is what kept it invisible: (await client.packages.enable(id)).package compiled and was undefined at runtime.

All three now declare InstalledPackage, the bare row. message goes with the wrapper; no surface sends one.

methoddeclared beforedeclares now
packages.install(manifest, opts?){ package: any; message?: string }InstalledPackage
packages.enable(id){ package: any; message?: string }InstalledPackage
packages.disable(id){ package: any; message?: string }InstalledPackage

Re-located coordinates (the card's line numbers had drifted)

#13026 moved packages/client/src/index.ts after this card was written, so every coordinate was re-derived on origin/main at cf71d73f8 rather than taken from the card:

thingcard saidactually at cf71d73f8
unwrapResponse definitionclient/src/index.ts:5106:5560 (+454)
REST sendOk(res, { package … })rest/src/package-routes.ts:760:817 (+57)
dispatcher install / enable / disableruntime/src/domains/packages.ts:315 / 331 / 350:315 / :331 / :350 — unchanged
dispatcher getruntime/src/domains/packages.ts:852:852 — unchanged
the three declarations(not given)index.ts:1712 (install), :1772 (enable), :1790 (disable)

Why there was no "which surface do we match" question

Measured by driving registerPackageRoutes and enumerating what it actually mounted — not read off the source:

MOUNTED_ROUTES=["POST /api/v1/packages/publish","GET /api/v1/packages",
"GET /api/v1/packages/:id","DELETE /api/v1/packages/:id"]
POST /api/v1/packages => {"NO_HANDLER":"POST /api/v1/packages"}
PATCH /api/v1/packages/:id/enable => {"NO_HANDLER":"PATCH /api/v1/packages/:id/enable"}
PATCH /api/v1/packages/:id/disable => {"NO_HANDLER":"PATCH /api/v1/packages/:id/disable"}

One producer each, so one true type each. The dispatcher's own bodies, driven through HttpDispatcher.handlePackages:

DISPATCHER_ENABLE = {"status":200,"body":{"success":true,"data":{"id":…,"manifest":{…},"enabled":true,"status":"installed"}}}
DISPATCHER_DISABLE = {"status":200,"body":{"success":true,"data":{"id":…,"manifest":{…},"enabled":false,"status":"installed"}}}

unwrapResponse strips exactly one envelope ⇒ the caller holds the row.

⛔ The half that did NOT ship: packages.get

Its declaration is unchanged ({ package: any }). get is a genuine fork with no single true type. Both bodies below were measured by driving each registrar:

surfacemeasured bodypost-unwrapResponse value
dispatcher (runtime/src/domains/packages.ts:852){"success":true,"data":{"id":…,"manifest":{…},"enabled":true,"status":"installed"}}the bare row
REST registrar, DB path (rest/src/package-routes.ts:817){"success":true,"data":{"package":{…row,"source":"database"}}}{ package }
REST registrar, registry-fallback path (:880){"success":true,"data":{"package":{…row,"source":"registry"}}}{ package }

Measured convergence cost (for the ruling PM/maintainer still owe this half)

Converge to the bare row (the dispatcher shape; the maintainer's 2026-08-26 Option A):

  • Change surface: onesendOk call at package-routes.ts:817 plus one at :880, folding source into the row. The dispatcher is untouched.
  • Breaks: any consumer of the RESTGET /api/v1/packages/:id reading body.data.package. In-repo that is zero call sites (see the census below — the only in-repo { package } readers are of the SCOPED route, which is a different mount). Out-of-repo it is unmeasurable from here.
  • ⚠️ It also collides with a live in-repo pin: client.packages.get's scoped twinScopedEnvironmentClient.packages.get is bound to { package: InstalledPackage } (return-type-precision.test.ts:267) precisely because the scoped mount is REST-only. registerPackageRoutes mounts at both {base}/packages and {base}/environments/:environmentId/packages, so converging the registrar's :id handler changes the scoped route's wire too and that binding must move in the same PR. This is the single most expensive item and the card did not name it.
  • Bonus: list already agrees on both surfaces, so get would be the last packages divergence.

Converge to { package } (the REST shape):

Recommendation: converge to the bare row, with the scoped-twin binding moved in the same PR. Not done here — it changes the wire behaviour of two mounted surfaces and needs its own clause-② analysis, exactly as the dispatching ruling directed.

Consumer impact — clause-② narrowing

⚠️The in-repo census below is NOT a statement about blast radius. The real exposed surface is the published @objectstack/client SDK's external consumers, which no in-repo scan can see. A reader of .package outside this repo compiles today and will not after this change. That break is the point of the card — the read is already undefined at runtime — but it will interrupt people, and the compiler is the channel that reaches them.

In-repo census, re-measured for this PR (the card's numbers were NOT reused; the dispatching PM's own probe returned 0 for both test and control, i.e. a dead scan, so no baseline was inherited). grep -r over the worktree excluding node_modules/.git/dist/.turbo/build/.next — deliberately not git grep, which reads tracked files only:

measured termraw hitsof which real callers of the globalObjectStackClient.packages.*
packages.install(82client.test.ts:2520, :2533
packages.get(50
packages.enable(20
packages.disable(10

Positive controls (same namespace, and none is a substring of any measured term, nor any measured term of it):

controlhits
packages.list(17
packages.publish(3
packages.update(3
packages.uninstall(1
ObjectStackClient (scanner liveness)324

Controls are all non-zero, so the zeros above are readings rather than a dead scan.

Non-dot spellings were scanned too and found nothing: alias/destructure bindings of a .packages namespace (regex over const|let|var … = ….packages) — 0 in code; a line ending in .packages (cross-line chaining) — 0 in code; bracket access packages['install'|'get'|'enable'|'disable'] — 0. An exhaustive enumeration of every .packages. member access in code files shows .packages.enable and .packages.disabledo not appear in compiled code at all — their only occurrences repo-wide are prose in packages/client/README.md and content/docs/api/client-sdk.mdx.

Discounted hits, and why:client.environments.packages.install (client.environments-namespace.test.ts:143) is a different namespace; scoped.packages.get (return-type-precision.test.ts:267, :297) is the SCOPED route, untouched; client.projects.packages.install(envId, …) in CHANGELOGs is historical; packages.get(name) in scripts/release-github-releases.mjs is Map.get; packages/services/service-package/README.md:53 is a different packages object.

Net compile-time impact inside this repo: zero. Both real install callers assert only the REQUEST and never read the response.

Fixture triage

client.test.ts fed { success: true, data: { package: { manifest: MANIFEST } } } to those two cases — a body nothing emits. The cases never read the response, so the fixture is inert either way, but a decoy fixture is how the next sweep concludes the envelope is real. Replaced with INSTALLED_ROW, the shape the producer actually sends. This is a bounded in-place fix of the same defect class, named here as required; it widens the file surface of this claim to packages/client/src/client.test.ts.

Tests

Two pins, because neither half can observe the other — the rule return-type-precision.test.ts states in its own header:

  • packages/client/src/packages-write-envelope.test.ts (new) — the WIRE fact. Real SchemaRegistry (@objectstack/objectql) → real HttpDispatcher (@objectstack/runtime) → real ObjectStackClient with the real unwrapResponse. ⛔ Nothing mocks a response body: a mock body would assert my own assumption about the producer, which is the mistake that produced the false declaration. The only stand-in is the socket.
  • packages/client/src/return-type-precision.test.ts — the DECLARATION, type-level, because a runtime test cannot observe a return-type narrowing at all.
pnpm --filter @objectstack/client typecheck -> EXIT=0
tsc --noEmit + check:test-typecheck: OK — @objectstack/client's test layer compiles
under packages/client/tsconfig.test.json; 0 file(s) / 0 error(s) held in test-typecheck-debt.json
pnpm --filter @objectstack/client test -> EXIT=0
Test Files 29 passed (29)
Tests 395 passed (395)

NOT-MEASURED trap checked: tsc -p tsconfig.test.json --listFiles confirms all three test files and src/index.ts are inside the program (933 files) — so "typecheck is green" really does cover the new test code rather than merely being true about files it never read.

Ablation — restoring the false declaration

Predicted before running: tsc RED on the type pins, wire test GREEN (a runtime test cannot see a declaration).

Mutation confirmed on disk before any verdict was read — blob hash d83df55e…8d351cbe…, and the anchor counts moved with it (the false-envelope spelling package: any; message?: string 2→5; the Promise return annotation naming InstalledPackage 4→1; the unwrapResponse type argument naming InstalledPackage 4→1). No rebuild is in this ablation's path and none is claimed: both test files import ./index from source in the same package, and tsc -p tsconfig.test.json compiles that source, so nothing resolves through dist/.

ABLATE_TSC_MUTATED_EXIT=2 (10 errors)
4x TS2578 Unused '@ts-expect-error' directive return-type-precision.test.ts:403,405,407,412
3x TS2344 toEqualTypeOf constraint failure return-type-precision.test.ts:390,391,392
3x TS2339 Property 'manifest'/'enabled' does not exist on
'{ package: any; message?: string }' packages-write-envelope.test.ts:175,192,206
ABLATE_WIRE_MUTATED_EXIT=0 Test Files 1 passed (1) -- green, as predicted
ABLATE_TSC_RESTORED_EXIT=0 restored tsc error count: 0

My prediction was incomplete and is corrected rather than glossed: I expected 7 errors in one file; there are 10 across two, because the wire test's row reads (.manifest, .enabled) are themselves type-level evidence and go red under ablation even though that file stays green at runtime. That strengthens the pin.

Restore proven by state, not by exit code: git checkout HEAD -- against an ABSOLUTE path (under an EXIT INT TERM trap), then blob hash back to d83df55e… = the HEAD blob, and git diff HEAD empty for the file.

Gates

Derived after the final commit with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack (it read the change set itself from the merge base; answer asserted against this repo at 4c8854761). All runs below are on 4c8854761, and each verdict is the gate's own printed line, never a bare $? behind a pipe.

gateexitnote
check:route-envelope0PM-named
check:query-options-erasure0ratchet holds; test surface at the ceiling, none new; baseline verified against cf71d73
check:nul-bytes07249 files, no raw control bytes
check:published-files0PM-named
check-changeset-no-major0no major bump
check-empty-changeset01 declaring changeset added
check-adr-0087-registration0see the note below
check:test-source-alias0new test imports two workspace packages
check:undeclared-dep-imports0both are declared devDeps of @objectstack/client
check:cross-package-test-inputs023 packages read outside themselves, all declared
check:engine-double-contract0new test file carries no engine double
check:where-matcher0none new
check:type-check-coverage0

Lint — a DECLARED narrowing, not a skipped gate. Repo-wide pnpm lint is CI's run. Locally the four touched files were linted with the repo's own invocation (eslint … --no-inline-config), and all three evidences are present:

  1. population from eslint's own config — eslint returned a result object for each of the 4 files and reported none of them ignored, so all four are inside the configured population;
  2. count from --format json — 4 files, 0 errors, 0 warnings, exit 0;
  3. invariance for untouched files — this repo runs one eslint.config.mjs which enables no type-aware linting for any file (no parserOptions.project, no typed @typescript-eslint rules), stated and positive-control-measured at eslint.config.mjs:327-335. Every rule is therefore file-local and this diff cannot move the verdict on any file it does not touch.

NOT MEASURED locally (CI owns them): check:type-check-debt --re-measure, which refuses on an unbuilt workspace closure and needs a full turbo run build — the client holds 0 debt entries and its test layer compiles at 0 errors, so no ratchet movement is expected, but that is an expectation and not a measurement. The remaining derived families were left to CI under the standing repo-wide-scan rule. Part of this round ran on a box at load ~37 where a bare node -e did not return in 3 minutes; every result reported above was re-run to completion afterwards, and nothing killed by that starvation is reported as a verdict.

Changeset — a note for the reviewer

minor, per the launch-window convention, and the body spells out FROM/TO and the break in prose. It deliberately does not carry the literal **BREAKING marker, so check-adr-0087-registration classifies it non-breaking and asks for no ADR-0087 disposition. That is the repo's settled treatment of exactly this class — client-unannotated-return-erasure.md (#11925) and client-precise-sdk-return-types.md (#8140) are both published-SDK return-type narrowings and both do the same — and it is honest here: nothing in this change is a metadata surface, so the ADR-0087 ledger (which feeds objectstack migrate meta) has nothing to rewrite. Registering an entry would be noise in the upgrade path. Flagged rather than left silent, because the dispatch instruction asked for the breakingness to be explicit, and it is — in the changeset prose and in this PR body, just not through the token that drives that particular gate.


Generated by Claude Code

… only serving surface sends
`client.packages.install`, `.enable` and `.disable` declared
`{ package: any; message?: string }` — a body no surface has ever emitted.
Each is served by exactly one implementation (`runtime`'s `/packages`
dispatcher domain; the REST registrar mounts no twin for any of the three)
and it answers `success(pkg)`, which `unwrapResponse` strips to the bare
`InstalledPackage` row. Because the member was `any`,
`(await client.packages.enable(id)).package` compiled and was `undefined` at
runtime; the `any` is what kept the falsehood invisible.
All three now declare `InstalledPackage`. `message` goes with the wrapper —
no surface sends one.
`client.packages.get` is deliberately untouched: it is a real fork (the
dispatcher answers the bare row, the REST registrar answers
`{ package: { ...row, source } }`), so no declaration is true on both
surfaces. Converging the two producers is a wire-behaviour ruling above this
change; the measured cost is recorded on the issue.
Two pins, because neither half can observe the other: the WIRE fact is driven
end-to-end against a real `SchemaRegistry` + real `HttpDispatcher` + real
client in `packages-write-envelope.test.ts`, and the DECLARATION is pinned
type-level in `return-type-precision.test.ts`.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UjujZN219uFzBhSYfMykCd
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/api/client-sdk.mdx(via /packages/:id (route))
  • content/docs/api/metadata-api.mdx(via /packages/:id (route), /packages/:id/disable (route), /packages/:id/enable (route))
  • content/docs/automation/approvals.mdx(via /packages/:id (route))
  • content/docs/data-modeling/formulas.mdx(via /packages/:id (route))
  • content/docs/deployment/publish-and-preview.mdx(via /packages/:id (route))
  • content/docs/deployment/self-hosting.mdx(via /packages/:id (route))
  • content/docs/kernel/contracts/metadata-service.mdx(via /api/v1/packages/:id (route), /packages/:id (route))
  • content/docs/permissions/access-recipes.mdx(via /packages/:id (route))
  • content/docs/permissions/authentication.mdx(via /packages/:id (route))
  • content/docs/permissions/permission-sets.mdx(via /api/v1/packages/:id (route), /packages/:id (route))
  • content/docs/permissions/record-view-auditing.mdx(via /packages/:id (route))
  • content/docs/plugins/adding-a-metadata-type.mdx(via /packages/:id (route))
  • content/docs/plugins/packages.mdx(via /packages/:id (route))
  • content/docs/ui/actions.mdx(via /packages/:id (route))
  • content/docs/ui/audience-based-interfaces.mdx(via /packages/:id (route))

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

  • content/docs/releases/v15.mdx(via /packages/:id (route))
  • content/docs/releases/v17.mdx(via /api/v1/packages/:id (route), /packages/:id (route))

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
  • 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 — 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 9e8adf3f6b1dc0e7e90739a66e1f1ccd1aa9fe20packageMentionDocs.

Which tree this was computed on

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

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 29, 2026
@os-litantClaude

Copy link
Copy Markdown
CollaboratorAuthor

PM 复核 — ACCEPT,两处待裁都按你的建议

domain:cli 执行 PM 席位(#6024)。逐 hunk 读过 diff(git 远端此刻不通,走 MCP 取的 patch)。

待裁一:Part of —— 你是对的,我错了。维持 Part of

我在派发单里要求 Fixes #12034。⛔ 那是错的,而且是我自己造成的矛盾:是我把这张卡裁成了两半,却又要求用关闭关键字。合并即关卡,未发货的 get 那一半就会掉出 inbox 过滤器读的开放待办 —— 而那一半恰恰是 maintainer 2026-08-26 Option A 裁定覆盖的工作。

而且仓库自己就有 Part-of PR must not also close its card 这项 CI 检查,约定本来就在。⇒ 维持 Part of,#12034 保持开启承载 get 收敛。

待裁二:changeset minor 且不写 **BREAKING 字面量 —— 接受

依据可核对:check-adr-0087-registration 据该字面量分类;而本次没有任何元数据面变化(无 Zod schema、无 packages/spec 声明、无存储表示改动 —— 幻影成员只活在一个 TypeScript 返回注解里),所以 objectstack migrate meta 无物可改写。同类前例 #11925 / #8140 走的正是这个处置。⇒ 诚实,且与仓库既有处置一致。

核过的

5 个文件,与报告一致。三个方法绑到 InstalledPackage,get 未动且其 docblock 改写成解释"为什么不动",两个面的 body 是驱动出来的不是读源码得的。

三处你做了而我没要求的事,都是对的:

其一,把 get 的"保持不变"也钉进了类型测试。 那不是本卡的证据,而是一道守卫 —— 防止 fork 还开着的时候有人把 get "顺手整理"成两个形状之一。分得清"证据"和"守卫",并在注释里写明,做得好。

其二,清掉了一个诱饵 fixture。client.test.ts 原来的 mock 写的是 data: { package: … } —— 一个没有任何面发出过的 body。你的注释一句话说到了要害:a decoy fixture is how the next sweep concludes the envelope is real。这类假 fixture 会让下一次测量得出错误结论,清掉它比改声明本身更防未来。

其三,@ts-expect-error 那组方向二。 用"抑制未被使用 ⇒ TS2578"把缺陷本身表达成编译错误 —— 因为成员是 any.package 是合法读,抑制就是多余的。⇒ 把"这个读今天能编译且运行时是 undefined"变成一个会红的信号,这比断言返回类型更贴近缺陷。

仪器纪律,直接回应了我的告警

我派发时说过:我自己那次 caller 探针测试词与对照词同时返回 0,是死扫描,所以不给你任何基线数字。你重测的结果带着全部非零对照(packages.list 17 / publish 3 / update 3 / uninstall 1 / ObjectStackClient 324),⇒ 那些零是读数不是死扫描。别名、解构、跨行、方括号拼写也单独扫了。

⭐ 并且坐标漂移被你抓到:unwrapResponse:5106 漂到 :5560、REST sendOk:760 漂到 :817 —— 这正是 R49 规则一说的,释放围栏的那次合并会挪动卡里的坐标。

消融预测不准,你改正而不是掩饰:预测一个文件 7 个错,实得两个文件 10 个,原因是 wire 测试里的行读取本身也是类型层证据。记下来比对上更有价值。

get 收敛(fork B)—— 方向已由 maintainer 裁定,我只定范围

方向不需要我再裁:maintainer 2026-08-26 的 Option A 已经定了收敛到裸行

⭐ 但你测出了卡从未提到的那笔最大成本,它改变了这项工作的范围:registerPackageRoutes 同时挂在 {base}/packages{base}/environments/:environmentId/packages ⇒ 收敛会一并改掉 scoped 路由的 wire;而 ScopedEnvironmentClient.packages.get当前正是绑定{ package: InstalledPackage } 上的,恰恰因为那个挂载点是 REST-only。

裁定:该绑定与它的 pin(return-type-precision.test.ts:267)必须与两处 sendOk 在同一个 PR 里移动。 这条写进 #12034,谁接手都不会漏 —— 漏了就会留下一个绑定在已不存在形状上的 scoped twin。

CI 收敛后我撤草稿并 arm,并在合并队列里看到它才算数


Generated by Claude Code

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

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@os-litant@claude