Skip to content

fix(runtime): type the packages-domain protocol service handle so undeclared request keys are compile errors - #15215

Merged
os-litant merged 6 commits into
mainfrom
claude/issue-13598-protocol-service-handle-typing
Sep 4, 2026
Merged

fix(runtime): type the packages-domain protocol service handle so undeclared request keys are compile errors#15215
os-litant merged 6 commits into
mainfrom
claude/issue-13598-protocol-service-handle-typing

Conversation

@os-litant

@os-litant os-litant commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #13598

The packages domain reached getMetaItems / saveMetaItem — and ten other protocol verbs — through a service handle typed any, so nothing downstream of that seam compiled against a contract. The #11006 series' end state ("an undeclared key in a request literal is a compile error") stopped one seam short here. This narrows the slot once, at one helper, and keeps every runtime capability probe.

What was measured, before writing anything

The card's anchors (~:504-531) and triage's count ("四处", :308 :373 :683 :706) are both stale. Re-derived on this branch's base 25a59bd with triage's own instrument:

$ git grep -c "resolveService(_context, 'protocol')\|resolveService(context, 'protocol')" \
      HEAD -- packages/runtime/src/domains/packages.ts
12

:404 :469 :795 :818 :840 :861 :911 :934 :996 :1046 :1139 :1236twelve, not four, matching the dispatching PM's own re-measurement anchor for anchor. Two carry the any on the declaration rather than the call site (:404 const protocolSvc: any = …, :1236 const protocol: any = …), which a call-site-only fix would have left standing. A broader control on the same file (grep -c resolveService) returns 14: the twelve plus one comment mention and one objectql resolve, so the twelve is the whole population and not a pattern that happened to match a subset.

The seam, and why the type is not on the slot

DomainHandlerDeps.resolveService already expresses a per-name return type — its first overload is keyed on keyof ServiceSlotContracts. 'protocol' is deliberately absent from that map, and the reason is written at both ends: the contract's own comment ("real services with no written contract, so they keep today's any rather than being given a shape here that nothing verifies") and this file's #9960 note. Mapping the slot would be a packages/spec change that has to answer for the whole slot, including the eight verbs no contract declares — and it would assert that a filled slot is a MetadataProtocol, whose members are mostly required, which is exactly what the runtime probes exist to deny.

So the narrowing happens at the consumer, once, in the shape domains/mcp.ts already uses for the same slot — its McpMergedMetadataRead, a Pick of MetadataProtocol's getMetaItems:

The eight verbs with no declared request shape anywhere (publishPackageDrafts, discardPackageDrafts, listCommits, revertCommit, rollbackToPackageCommit, reassignOrphanedMetadata, duplicatePackage, updatePackage) keep an explicit any request. @objectstack/metadata-protocol types them inline on the implementation class and exports nothing for them; writing a structural type here would be a private restatement nothing verifies — the thing #9846 retired one file over. What their entries still buy is the verb name: a misspelt verb is now a compile error where the any handle took any spelling at all.

The guards STAY

Not one typeof protocol.VERB === 'function' probe is removed or weakened. A host may occupy this slot with a partial object; that Partial is what makes the type agree with the probes instead of contradicting them. The type answers "is this key declared?", the probe answers "did this host bring the verb?" — two different questions, both still asked. Section 2 of the new pin drives a real dispatcher whose protocol brings none of the verbs and asserts the seven documented 501s.

packages/spec/src/** is untouched: every key these literals send was already declared, so this is consumer-side typing only.

Evidence: an undeclared key is now a compile error

Same instrument both directions, one injected key (bogusUndeclaredKey: true) in the ADR-0045 flip's saveMetaItem literal, each leg proven on disk by blob hash and restored the same way:

tree tsc --noEmit -p packages/runtime/tsconfig.json
base 25a59bd (blob e7e7bf85) exit 0, zero diagnostics
this branch (blob 4bd8a401) exit 2packages.ts(727,41): error TS2353: Object literal may only specify known properties, and 'bogusUndeclaredKey' does not exist in type '{ type: string; name: string; item: unknown; organizationId?: … }'

Both legs restored with git checkout HEAD -- "$REPO_ROOT/PATH" under an EXIT trap, each verified by git diff HEAD empty and git hash-object back to the HEAD blob.

packages-protocol-handle-typing.test.ts makes that measurement durable rather than one-shot. Its four @ts-expect-error directives are themselves checked — if the seam ever goes back to any they stop matching an error and tsc reports TS2578 — and they are not phantom checks: --listFiles puts the file in tsconfig.test.json's program (1 hit) and not in the build program (0 hits, while domains/packages.ts is 1 there), so the probe is directional rather than blind. The file carries no entry in test-typecheck-debt.json, which the ratchet reads as "any error it gains is red on arrival"; the ledger is unchanged at 27 files / 191 errors.

Ablation, direction predicted before running. Reverting domains/packages.ts to base (blob confirmed e7e7bf85 on disk, PackagesDomainProtocol grep 0, (protocol as any) grep 21) makes the pin red as TS2305 x1 + TS2578 x4 — the reversal shape, every directive going unused, which is why the directives are the pin and not expectTypeOf assertions. Restored and re-verified by hash. No dist/ is involved in this ablation and no rebuild is owed: the subject is reached by an intra-package relative import (./packages.js), so tsc and vitest both read it from source, with no vitest alias in play.

Verification

Everything below re-run at final HEAD 34a1740a, on a clean tree, AFTER merging origin/main (50d6c924) into the branch — every number here is from the merged tree, not from the pre-merge one.

  • pnpm --filter @objectstack/runtime test221 files / 3168 tests passed, including the 8 new ones.
  • pnpm --filter @objectstack/runtime typecheckexit 0 (tsc --noEmit plus check:test-typecheck: ledger held at 27 files / 191 errors / 69 pinned signatures).
  • Gate union derived with node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack --commands (never a hand-written path list) — 71 commands, all exit 0, each exit code captured before any pipe. Re-derived twice: once after the docs edit below (which pulled in 28 gates the first derivation did not name) and again after the merge, where it returns the identical 71 over the identical 4-path change set.
  • pnpm lint (repo-wide eslint . --no-inline-config, exactly as CI spells it) — exit 0 over 5,874 files, 0 errors, 0 warnings; both edited source files are in that population. Not a narrowed run.
  • Control-character scan over the diff (grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]') — no hits.

check:type-check-debt and check:dual-build-cjs-loads first answered PREREQUISITE NOT MET (exit 3 — not a pass and not a finding). Building @objectstack/runtime and the two other packages they named turned both into real measurements: the debt ratchet re-measured 16 ledger entries / 207 raw errors with none above its recorded number (17 / 217 before the merge), and the dual-build gate loaded 102 require entry points across 66 packages.

The docs hunk

content/docs/permissions/system-context.mdx moves three line anchors by exactly 96 — the height of the inserted type block — for census rows 52 and 55. Written by the gate's own node scripts/check-system-context-census.mjs --fix, which is what check:system-context-census demanded; no prose changed.

That page is a merge=os-regen generated artifact, so the merge went through the sanctioned sequence
(bash scripts/pm/os-regen-merge.sh, self-test green first): merge, take main's side, commit the merge, THEN regenerate
— never a regeneration while the tree is still in MERGE state. Verified afterwards that main's own advances on the page
survived: diffed against origin/main, the only difference is my two rows (52 and 55), with main's row 50
(rest-server.ts:4888 …) and row 56 (:138 / :189) intact.

⚠️ Expect GitHub to report this head dirty again. That page is re-anchored by every PR that shifts a line in a
runtime source file, and GitHub's server-side merge does not run the os-regen driver, so a textual conflict reappears
the moment main re-touches it — it did so once while this PR was being verified. git merge-tree --write-tree origin/main HEAD (which does honour the driver) exits 0. ⛔ Not a reason to force-push or rebase: the remedy at landing
time is the same scripts/pm/os-regen-merge.sh sequence.

Out of scope, deliberately

packages/runtime/src/domains/meta.ts reaches saveMetaItem through the same untyped handle. Same family, different file, and not folded in: it is not needed to make the packages-domain literals compile against the declared request types, and its literal is built field by field and separately pinned. Left as dispatched.

@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Sep 4, 2026
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

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

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

  • content/docs/concepts/metadata-lifecycle.mdx (via getMetaItems (literal, a string literal in Partial; a string literal on a changed line), saveMetaItem (literal, a string literal in Partial))
  • content/docs/data-modeling/drivers.mdx (via getMetaItem (literal, a string literal in Partial))
  • content/docs/deployment/environment-variables.mdx (via getMetaItems (literal, a string literal in Partial; a string literal on a changed line))
  • content/docs/deployment/validating-metadata.mdx (via saveMetaItem (literal, a string literal in Partial))
  • content/docs/kernel/cluster.mdx (via saveMetaItem (literal, a string literal in Partial))
  • content/docs/kernel/services-checklist.mdx (via getMetaItem (literal, a string literal in Partial), getMetaItems (literal, a string literal in Partial; a string literal on a changed line), saveMetaItem (literal, a string literal in Partial))
  • content/docs/permissions/authorization.mdx (via saveMetaItem (literal, a string literal in Partial))

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

  • content/docs/releases/v12.mdx (via installPackage (literal, a string literal in PackagesDomainProtocol))
  • content/docs/releases/v17.mdx (via deletePackage (symbol, a method of type PackagesDomainProtocol), duplicatePackage (symbol, a method of type PackagesDomainProtocol), publishPackageDrafts (symbol, a method of type PackagesDomainProtocol), saveMetaItem (literal, a string literal in Partial))

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 cross-cutting symbol(s) contributed no route anchor: resolveProtocol (26 routes)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 47 of 219 client-bound route-ledger rows — the other 172 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 172: 14 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • 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 — 24 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 50d6c924bb7cacbc934e8386d8523f1be40cc040packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8417bc388e079187252330af732e47acc7adfb82 — the merge of head 34a1740a63cacc9cb146c78be9a251595502eced into base 50d6c924bb7cacbc934e8386d8523f1be40cc040, 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 8417bc388e079187252330af732e47acc7adfb82 && git checkout 8417bc388e079187252330af732e47acc7adfb82
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 50d6c924bb7cacbc934e8386d8523f1be40cc040 34a1740a63cacc9cb146c78be9a251595502eced && git checkout -B drift-repro 50d6c924bb7cacbc934e8386d8523f1be40cc040 && git merge --no-ff 34a1740a63cacc9cb146c78be9a251595502eced

node scripts/docs-audit/affected-docs.mjs --json 50d6c924bb7cacbc934e8386d8523f1be40cc040

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

@os-litant
os-litant marked this pull request as ready for review September 4, 2026 06:56
@os-litant
os-litant enabled auto-merge September 4, 2026 06:56
@os-litant
os-litant added this pull request to the merge queue Sep 4, 2026
Merged via the queue into main with commit dd999ce Sep 4, 2026
39 checks passed
@os-litant
os-litant deleted the claude/issue-13598-protocol-service-handle-typing branch September 4, 2026 07:34
zhuangjianguo pushed a commit that referenced this pull request Sep 4, 2026
…ndeclared request keys are compile errors (#15215)

* wip(runtime): type the packages-domain protocol service handle

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

* wip(runtime): add the packages-domain protocol handle typing pin

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

* chore(changeset): patch note for the packages-domain protocol handle typing

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

* docs(permissions): re-anchor the system-context census rows moved by the typing block

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

* docs(permissions): regenerate the system-context census from the merged tree

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

---------

Co-authored-by: Claude <noreply@anthropic.com>
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