Skip to content

fix(metadata-protocol)!: the protocol install primitive parses manifest.id, and the duplicate door parses its target id (#19417) - #19574

Merged
os-project-manager merged 1 commit into
mainfrom
claude/issue-19417-protocol-install-door-parses-manifest-id
Sep 21, 2026
Merged

os-project-manager merged 1 commit into
mainfrom
claude/issue-19417-protocol-install-door-parses-manifest-id

Conversation

@os-project-manager

Copy link
Copy Markdown
Collaborator

Fixes #19417

Clause-②: no (narrowing)

Fixes, not Part of, and the reason is measured rather than assumed: the landing record 5754746826 held this card open for exactly one thing — the packages/metadata-protocol seam, protocol.ts building dupManifest with id: request.targetPackageId and writing it through installPackage with no ManifestSchema parse. That seam is closed here. Every other ask on the card already landed with #19473 and is verified present on this branch's base: the HTTP door's ManifestSchema.shape.id.safeParse gate, the refusal pins, and the REVERSED domain-handler-registry.test.ts pin ([#19417] POST /packages refuses 'pkg-a' — the id the pattern refuses (the REVERSED pin)). Nothing on the card is left standing, so merging it should close it.

Diff measured between b3615f1a4cd7f3ff59ff0548530daa042627f732 (the merge base, origin/main at branch creation) and 3128642fd60833d130844364f1b5e6c872efafb5 (head). 3 files, +465 / -3.

⏳ The measurement the order asked for FIRST — and it is not what the seam note feared

The order's blocking question: does the platform itself mint package ids that MANIFEST_ID_PATTERN refuses, through this door? Enforcing here narrows a live door whose docblock says it serves packages that never take the defineStack path.

Measured answer: no — no first-party code mints a refused id through this door. The instrument and its controls:

probe reading control
non-test callers of protocol.installPackage, whole repo 2, both opened the same instrument returns the registry-direct call sites it must not confuse them with (objectql/src/engine.ts, service-package/src/index.ts) — lit
caller 1 — packages/runtime/src/domains/packages.ts (POST /packages) the ManifestSchema.shape.id.safeParse gate #19473 landed sits above the protocolSvc.installPackage(...) call in the same branch, unconditionally ⇒ only conforming ids reach the primitive from there the branch's other gates (!pkgId, the version leg) are present in the same read
caller 2 — duplicatePackage (same file) id is request.targetPackageId, caller-supplied: its only non-test caller is POST /packages/:id/duplicate, which reads it from the request body and checks only that it is non-empty duplicatePackage non-test callers = 1, enumerated
first-party id MINTING sites os init stamps com.example. + manifestIdSlug(name), and manifestIdSlug forces a letter-initial segment — conforming by construction. The shipped example manifests are com.example.crm / .showcase / .todo defineStack occurrence counts in examples/ are non-zero — lit
the Studio UI (objectui 0cf2d6644bdb96a9a6784ef801ee6a60a5306bd8, cloned to read it) both duplicate dialogs prefill the source id with -copy appended — conforming whenever the source is. Nothing auto-generates a refused id targetPackageId occurrence count in objectui is non-zero, every hit opened

⚠️ The seat's two failed probes are not repeated here: packages/studio/src and packages/setup/src do not exist in this tree, and a packages/*/src pathspec is dead against git grep. Every reading above names a real path and carries a control that hit.

⇒ the narrowing refuses caller input, never a value the platform produces. No grandfather clause is invented, and nothing is routed around.

The fix, and why it is on the primitive

ObjectStackProtocolImplementation.installPackage spread the request into any and handed it to SchemaRegistry.installPackage with a second as any. grep ManifestSchema over its 22,686 lines returned exactly 2 hits, both comments — no parse anywhere in the file.

#19473 is not this door, and it is also not unrelated — measured, because both readings matter. It landed in packages/runtime/src/domains/packages.ts, and that HTTP door does route through protocol.installPackage when the protocol service resolves. So its gate protects that one caller and nothing else: duplicatePackage is a second caller and an embedder holding the protocol object is a third. Gating a door buys that door; this gate is on the method every caller passes.

Three changes, all in packages/metadata-protocol/src/protocol.ts:

  1. installPackage parses the raw manifest.id through ManifestSchema.shape.id — by reference, never a copy of the grammar — ahead of the spread, the version default and the namespace derivation. The refusal is the declaration's own sentence (manifestIdRefusal), surfaced rather than reworded, and the throw carries statusCode: 400 so an HTTP boundary answers 400 rather than the 500 an unannotated throw earns (resolveThrownHttpError). statusCode is the spelling this file already uses for its 404.
  2. duplicatePackage parses its target id at the top of the method. It has to be there, not only in installPackage: the manifest write below sits inside a deliberately best-effort catch {}, so a refusal raised only there would be swallowed and the caller would read success: true on a package with no manifest row — a silent partial state, strictly worse than the status quo this card set out to close. The position also honours the POST /packages/:id/duplicate 对一个启动中的代码包返回 success:false, copiedCount:0, failedCount:0(空 base)—— ADR-0070 D4「复制成可写 base」对代码包是否本就该拷贝其对象? #14451 rule already on this door: refuse before the mint, or the empty shell is left behind. The key named is targetPackageId, the path the caller actually wrote.
  3. One assumption, one implementation. Both namespace derivations on the duplicate path moved from a raw id.split('.').pop() to the spec helper deriveNamespaceFromPackageId that installPackage already used. This is not cosmetic: the target namespace is spliced into every copied object name as namespace + '_' + short, and an object name is /^[a-z_][a-z0-9_]*$/ (packages/spec/src/data/object.zod.ts). The Studio's own default duplicate id — the source id with -copy appended — therefore derived leave-copy and minted leave-copy_ticket, a name the object declaration refuses. The helper answers leave_copy. The source side is the same rule read backwards: the prefix those rows actually carry is the one installPackage stamped, so matching them with the raw split found nothing and the copy landed under the SOURCE's names — the collision the re-namespacing exists to prevent. An explicit targetNamespace still wins untouched; when neither an explicit nor a derivable namespace exists, the door refuses loudly naming targetNamespace as the remedy instead of renaming rows with an empty prefix.

Both directions pinned

packages/metadata-protocol/src/protocol.install-manifest-id.test.ts, 18 cases, all passing:

  • refusal — four refused ids (a bare word, an underscore inside a segment, the empty string, a digit-initial segment), a manifest with no id at all, and a whitespace-padded conforming id. Each asserts the 400 tag, the message identical to manifestIdRefusal itself (a pin that retyped the sentence would go green on a reworded fourth sentence for one rule), and that neither writer ran — not the in-memory registry, not the durable publish.
  • lit controlcom.example.crm, com.example.my-erp and org.apache.superset still install, registry called exactly once with the id intact.
  • the repair survives — the refusal for com.example.my_erp still contains com.example.my-erp, the mechanical repair the declaration verifies before offering.
  • duplicate door — the same four refused targets are refused with nothing minted and nothing scanned (registry.installPackage, engine.find and saveMetaItem all uncalled), against a lit control where a conforming target duplicates rows and all.
  • namespace — the Studio default yields leave_copy and writes leave_copy_ticket; an explicit targetNamespace wins; an underivable one refuses naming the remedy.

Reverse verification — direction predicted BEFORE running, both legs restored and proven

Run through scripts/ablation-replace.mjs, which proves the mutation reached disk by anchor count and blob hash and proves the restore against HEAD (no bare git checkout --). The subject resolves from src here — the suite imports ./protocol.js inside its own package — so dist is not on this resolution path.

ablation predicted measured
A — delete the installPackage id gate the installPackage refusal pins turn RED; the lit controls and every duplicate-door pin stay GREEN 7 failed / 11 passed. Exactly the installPackage arm, including the repair pin I had not counted; both control groups green. Blob be9dd23ad9c88981aea29d39, restored to be9dd23ad9c8, git diff HEAD empty
B — put the raw split('.').pop() back for targetNs only the two namespace pins turn RED 2 failed / 16 passed, exactly those two. Blob be9dd23ad9c8613737784401, restored to be9dd23ad9c8, git diff HEAD empty

Gates — 61 commands, harvested at this head

node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack at 3128642fd6 (the script derived the change set itself from the merge base; ⛔ not a hand-written path list). 58 green, 0 red, 3 NOT MEASURED — then one of the three was converted by building its prerequisite, leaving 59 green / 2 NOT MEASURED.

Green, by name: check-adr-0087-registration (+ self-test), check-changeset-no-major (+ self-test), check-ci-filter-parity, check-closing-keyword-parity (+ self-test), check-comment-mask-adoption (+ self-test), check-comment-mask-corpus, check-empty-changeset (+ self-test), check-keyed-text-bounds (+ self-test), check-platform-object-tenancy-census (+ self-test), check-plugin-teardown-shape (+ self-test), check-registry-log-declared (+ self-test), check-rest-log-spy-declared (+ self-test), check-system-context-census (+ self-test), check-undeclared-dep-imports (+ self-test), docs-audit/check-affected-docs, docs-audit/check-drift-comment, pm/release-rehearsal-clone --self-test, spec check:duration-unit-keys, check:changeset-gate-self-tests, check:cross-package-test-inputs, check:dispatcher-error-vocabulary, check:doc-authoring, check:driver-memory-census, check:dts-closure, check:durability-log-level, check:engine-double-contract, check:filter-alias-parity, check:gitlink-declared, check:issue-citations, check:lean-entry-closure, check:logger-receiver-detach, check:nul-bytes, check:objectql-double-limit, check:objectui-changeset, check:org-identifier, check:page-declaration-shape, check:pm-changeset-deadline-census, check:published-files, check:query-options-erasure, check:refd-timer-probe, check:slot-lookup, check:sourcemap-no-sources-content, check:test-source-alias, check:tier-file-adoption, check:type-check-coverage, check:watch-hint-literal, check:where-matcher.

check:lean-entry-closure first exited 3 (PREREQUISITE NOT MET — it loads built entry points and @objectstack/objectql had no dist); after turbo run build --filter=@objectstack/objectql it measured green: 2 published conditions, 15 packages, admitted set held exactly.

NOT MEASURED, and recorded as such — neither a pass nor a failure:

  • check:dual-build-cjs-loadsexit 3, PREREQUISITE NOT MET: it reads built output and 68 packages have no dist/. That needs a whole-repo build, which is CI's Build Core.
  • check:type-check-debtexit 3, same class: --re-measure refuses to record a number against an unbuilt closure, since an unresolved import invents TS2307/TS7006 and erases the real debt. lint.yml builds the closure before this step.

Plus, beyond the harvest: pnpm --filter '@objectstack/metadata-protocol^...' build green; the package's full suite green (185 files, 2,645 tests, 0 failures — no existing pin moved); pnpm --filter @objectstack/metadata-protocol typecheck green, with --listFiles confirming the new test file is in the program; the three @objectstack/objectql suites that drive a REAL protocol instance green (29 tests); and the repo-wide union pnpm lint (eslint . --no-inline-config) green at 3128642fd6 — the union, so no narrowing had to be proven.

Scope held, and what is deliberately left standing

  • The id leg alone. InstallPackageRequestSchema / ManifestSchema are still not parsed whole here; the residual classes the HTTP door's own docblock records are untouched and are each their own narrowing.
  • packages/spec does not move, and was never opened: the declaration was already right, and the order fenced it out.
  • Boot-time and in-process installs are unaffected — they reach SchemaRegistry.installPackage / ObjectQL.registerApp directly and never pass this primitive. Versionless and namespace-less manifests still install; their defaults simply run behind the id gate instead of ahead of it.

Acceptance notes — observed, ⛔ not fixed here

  1. objectui declares the same rule, looser. packages/app-shell/src/views/studio-design/packages-io.ts exports PACKAGE_ID_RE = /^[a-z][a-z0-9_.-]*(\.[a-z0-9_-]+)+$/, which admits underscores and digit-initial segments; MANIFEST_ID_PATTERN is /^[a-z][a-z0-9-]*(\.[a-z][a-z0-9-]*)+$/ and its own TSDoc says underscores are NOT admitted. Both the package-create dialog and the duplicate dialog validate against the looser copy, so the Studio accepts com.example.my_erp and the server has refused it since fix(runtime): POST /api/v1/packages parses the manifest's id leg #19473 — a second declaration of one rule, in the sibling repo. Reported for a card of its own; ⛔ not touched from here.
  2. duplicatePackage's explicit targetNamespace is still unvalidated. An explicitly passed my-ns is spliced into object names as my-ns_x, which the object declaration refuses. This change only aligned the DERIVED default, which is the seam the order named.
  3. reassignOrphanedMetadata reads targetPackageId without parsing it, the same positional read one method over. Left alone deliberately: it rebinds rows to an EXISTING package rather than minting one, so it is a different question about a different door.

Generated by Claude Code

…rimitive (#19417)

`ObjectStackProtocolImplementation.installPackage` spread the request into
`any` and handed it to `SchemaRegistry.installPackage` with a second `as any`,
so an id `MANIFEST_ID_PATTERN` refuses installed and persisted while
`defineStack()`, `os build`, `os validate` and the publish face all refused the
same id. #19473 closed the HTTP door, which is one CALLER of this primitive;
`duplicatePackage` is a second and an embedder is a third.

The gate asks the declaration by reference (`ManifestSchema.shape.id`) and
surfaces its own sentence (`manifestIdRefusal`) rather than rewording it, ahead
of every write and every derivation. `duplicatePackage` parses its target id at
the top of the method, because its manifest write sits inside a best-effort
`catch {}` that would otherwise swallow the refusal and report success.

Both namespace derivations on the duplicate path move from a raw
`id.split('.').pop()` to the spec helper `deriveNamespaceFromPackageId`, the
one `installPackage` already used: the target namespace is spliced into every
copied object name, and the Studio's default `<sourceId>-copy` derived
`leave-copy`, minting names the object declaration refuses.

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 21, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/metadata-protocol, touching 5 documentable anchor(s).

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

  • content/docs/api/declarative-endpoints.mdx (via manifest.id (literal, a string literal in installPackage))
  • content/docs/deployment/cli.mdx (via manifest.id (literal, a string literal in installPackage))
  • content/docs/deployment/publish-and-preview.mdx (via manifest.id (literal, a string literal in installPackage))
  • content/docs/getting-started/quick-reference.mdx (via manifest.id (literal, a string literal in installPackage))
  • content/docs/kernel/contracts/metadata-service.mdx (via /api/v1/packages/:packageId (route, bridged from symbol installPackage — its route source's handler names it))
  • content/docs/protocol/kernel/error-handling.mdx (via /api/v1/packages/:packageId (route, bridged from symbol installPackage — its route source's handler names it))
  • content/docs/protocol/kernel/http-protocol.mdx (via manifest.id (literal, a string literal in installPackage))
  • content/docs/ui/apps.mdx (via manifest.id (literal, a string literal in installPackage))

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

  • content/docs/releases/v12.mdx (via installPackage (symbol, a method of class ObjectStackProtocolImplementation))
  • content/docs/releases/v17/17-0.mdx (via /api/v1/packages/:packageId (route, bridged from symbol installPackage — its route source's handler names it))
  • content/docs/releases/v17/17-4.mdx (via /api/v1/packages/:packageId (route, bridged from symbol installPackage — its route source's handler names it))

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
  • the SDK route bridge reached 60 of 215 client-bound route-ledger rows — the other 155 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 155: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 100 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 11 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 b3615f1a4cd7f3ff59ff0548530daa042627f732packageMentionDocs.

Which tree this was computed on

This run read content/docs from 8b05264c5bddd8a7e866bd62cc115880b997b4df — the merge of head 3128642fd60833d130844364f1b5e6c872efafb5 into base b3615f1a4cd7f3ff59ff0548530daa042627f732, 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 8b05264c5bddd8a7e866bd62cc115880b997b4df && git checkout 8b05264c5bddd8a7e866bd62cc115880b997b4df
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin b3615f1a4cd7f3ff59ff0548530daa042627f732 3128642fd60833d130844364f1b5e6c872efafb5 && git checkout -B drift-repro b3615f1a4cd7f3ff59ff0548530daa042627f732 && git merge --no-ff 3128642fd60833d130844364f1b5e6c872efafb5

node scripts/docs-audit/affected-docs.mjs --json b3615f1a4cd7f3ff59ff0548530daa042627f732

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

@os-project-manager
os-project-manager marked this pull request as ready for review September 21, 2026 09:59
@os-project-manager
os-project-manager added this pull request to the merge queue Sep 21, 2026
Merged via the queue into main with commit 2306a75 Sep 21, 2026
36 checks passed
@os-project-manager
os-project-manager deleted the claude/issue-19417-protocol-install-door-parses-manifest-id branch September 21, 2026 10:14
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