fix(cli): parse os package publish manifest ids through PackageSchema.manifestId - #16889
Conversation
…ma.manifestId
`packages/cli/src/commands/package/publish.ts` carried its own
`MANIFEST_ID_RE` and tested `--manifest-id` (and the derived id) against it,
while the contract this repo declares for the column it publishes into is
`PackageSchema.manifestId` in `packages/spec/src/cloud/package.zod.ts`. The
local copy was looser on every axis, so the preflight admitted what the
control plane refuses.
- Delete `MANIFEST_ID_RE`. Both paths — the explicit `--manifest-id` /
`objectstack.manifest.json` check and the derive path in
`deriveManifestId` — now parse through the imported schema.
- The derive path's extra `explicit.includes('.')` condition is dropped: the
schema subsumes it (its pattern needs at least two segments). That
condition is why the two paths disagreed with each other as well as with
the declaration.
- The refusal text is quoted from the schema's own `invalid_format` issue and
its `.describe()`, so it can no longer state a contract that does not exist.
- A derived id the schema rejects (`slugify` has no letter-first rule, so an
app named `2024 App` derives `local.2024-app`) is refused before any network
call, naming where the id came from and how to set one. It is deliberately
not normalised: `manifestId` is immutable once published.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8
📓 Docs Drift CheckThis PR changes 1 package(s): 11 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 1 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 22 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin a6204a5d87b55f0755380c58f6f4d7745b1ce704 && git checkout a6204a5d87b55f0755380c58f6f4d7745b1ce704
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 142c01c88ee1be4c24e20f7b803a511a00c992fb 8756045d1a4b82cc711e09d105d2443abace1af7 && git checkout -B drift-repro 142c01c88ee1be4c24e20f7b803a511a00c992fb && git merge --no-ff 8756045d1a4b82cc711e09d105d2443abace1af7
node scripts/docs-audit/affected-docs.mjs --json 142c01c88ee1be4c24e20f7b803a511a00c992fb
|
Fixes #16520
Clause-②: noos package publishcarried its ownMANIFEST_ID_REand validated--manifest-id(and the derived id) against it, while the contract this repo declares for the column it publishes into isPackageSchema.manifestIdinpackages/spec/src/cloud/package.zod.ts. The local copy was looser on every axis, so the preflight admitted what the control plane refuses — and its error text, when it did fire, named a contract that does not exist.The remedy is the one the maintainer's ruling on cloud#1932 already settled — 「cloud#1932 不普查,直接按协议修改。」 under 「本项目以协议为基准。」 — import, never transcription.
CreatePackageRequestSchemain the same spec file already reaches forPackageSchema.shape.manifestIdthe same way (verified by symbol at current head, not by line number).验收 — evidence against each item
1.
MANIFEST_ID_REdeleted entirely; both paths routed through the schema.The constant is gone from the file (
git grep MANIFEST_ID_RE packages/clireturns nothing). Both sites now ask the imported schema:deriveManifestIdadoptsartifact.manifest.idonly whenisManifestId(explicit), which isPackageSchema.shape.manifestId.safeParse(...).success;run()parses the supplied or derived id through the same schema before any network call.The derive path's extra
explicit.includes('.')condition is removed, not preserved: the schema's pattern requires at least two segments, so a dotless id can never parse and the condition is subsumed. Dropping it narrows nothing and widens nothing.2.
deriveManifestIdclosed. It previously trustedartifact.manifest.idthrough the same loose test, socom.acme.repair_deskand friends were forwarded unchanged.ManifestSchema.idis a barez.string(), so an artifact may carry any shape at all; the deriver now adopts it only when the control plane would accept it, and otherwise falls through to the existinglocal.SLUGfallback.3. The producer side — handled by refusing, deliberately not by normalising.
slugifylowercases, collapses non-alphanumeric runs to hyphens, trims and truncates, with no letter-first rule, so a manifest named2024 Appderiveslocal.2024-app— digit-first, and rejected by the schema. That id is now refused before any network call, with a message that names where it came from and how to set one (--manifest-id,manifestIdinobjectstack.manifest.json, ormanifest.idin the config).It is not normalised into some other id, and that is a decision rather than an omission:
manifestIdis declared immutable once set ("renaming a package requires creating a new package"), so a normaliser mints a permanent, globally unique identifier the author never wrote and cannot rename afterwards. "Prefer failing to falling back" (AGENTS.md → Route & surface ownership §3) and Prime Directive #12's "reject it at authoring/publish so the error surfaces loudly" both point the same way. Nothing is lost by refusing: that publish does not work today either — it fails one round trip later, with a worse message. cloud PR #2032'smanifestIdSegmentis named in the card as one worked normalisation; it could not be read from this seat (see the note at the end) and is not what this PR does.4. Error text quoted from the schema. The refusal is built from the schema's own
invalid_formatissue message plus its.describe(); no second description of the pattern is written anywhere in the command. The retired sentence is what was lying: a user actually stopped by the CLI reada-z0-9._-, "fixed" their id to something likecom.acme.repair_desk— accepted locally, refused by the server. Following the error message led to a second error. A test asserts the new text contains the schema's own issue message and its description, and a sibling test asserts the stringa-z0-9._-no longer appears in the output.5. Per-path assertions, per shape.
packages/cli/test/package-publish-manifest-id.test.tsasserts all six shapes on both paths:crmcom.acme.repair_deskCOM.ACME.CRM9foo.barcom..acmecom.acme.Triage's correction is carried into the suite as data rather than as prose:
RELAXATIONSrecords, per shape, whether the derive path admitted it before the fix — five did,crmdid not, because of that dot condition. The two ablations below reproduce that asymmetry mechanically.On the derive path "refuses" means the illegal
manifest.idis not adopted: the deriver falls through tolocal.SLUG(its pre-existing behaviour for any id it will not take), and each case asserts both halves — the deriver returns{ id: 'local.acme-crm', source: 'artifact-manifest-name' }, and end to end themanifest_idthat reaches the wire is neither the illegal shape nor anything the schema rejects.6. Negative control — mandatory, and present on both paths.
com.acme.crmstill publishes, bytes unchanged: on the explicit pathcalls[0].body.manifest_idis exactlycom.acme.crm, and on the derive pathderiveManifestIdreturns exactly{ id: 'com.acme.crm', source: 'artifact-manifest-id' }and the same value reaches the wire. A second control covers the derived-but-legal case (Acme CRMpublishes aslocal.acme-crm).A re-transcription of the schema's regex would turn every refusal above green while reproducing this card's cause exactly, so the suite also carries a source pin: no fully anchored regex literal in
publish.tsmay matchcom.acme.crm— any local rule about manifest ids must, which is what makes it a rule about manifest ids. The pin runs with its own positive control beside it (the retired rule as text, and a transcription of the schema's own pattern — the scanner finds both), and additionally asserts that the source still containsPackageSchema.shape.manifestId, so the absence is a fix and not a deletion.7.
Clause-②re-declared from the delivered diff:no. The diff narrows a local preflight's accept set back onto the contract this repo declares. No id that is rejected today becomes valid: the deriver's fallback behaviour is unchanged, no normaliser was added, and the only condition removed (includes('.')) is subsumed by the schema. The single behaviour change in the other direction is that ids the server was going to reject are now refused locally.8. Out of scope, untouched: cloud's three producers (cloud PR #2032) and
create_package'sREVERSE_DOMAIN_RE.Reverse verification
Both legs were run from the committed implementation, each proving the mutation reached disk (blob hash before/after plus an on-disk marker count) and each restored with
git checkout HEAD -- PATH, verified bygit diff HEADandgit status --porcelainboth empty and the blob hash back to the HEAD blob. Baseline: 22 passed.isManifestIdbody replaced by the retired rule plus the call site's dot conditioncrmstays green, which is exactly triage's asymmetry; the source pin firesexplainManifestIddecided by the retired rule, returning the retired sentenceOne test stayed green under the explicit-path ablation and that is correct rather than a gap: "no longer states the contract the CLI invented" asserts the text, and under that mutation the command does not refuse
com.acme.repair_deskat all, so nothing is printed. Its sibling in the same block — which asserts the schema's own issue message is present — is one of the nine that went red.Tests and gates
Measured at
8756045d(the final commit) in a worktree branched fromc930f8597. Heavy runs went throughscripts/pm/os-verify-lock.sh; the box is shared, so the wall-clock figures below are shared-box seconds.pnpm --filter @objectstack/cli exec vitest run --project unit— 187 files passed, 2577 passed | 6 expected fail (2583), exit 0. Theintegrationtier is declared to CI: this diff touches no spawn entry point, notest/helpers/serve-process.tsand no driver/kernel boot path, and the new file lands inunitunderpackages/cli/vitest-tiers.ts(test/vitest-tiers-partition.test.tspasses).pnpm --filter @objectstack/cli typecheck— exit 0.check:test-typecheckreports the ledger unchanged (3 files / 28 errors, shrink-only), andtsconfig.test.jsonincludestest/**/*, so the new file is in a tsc program rather than merely on disk.pnpm --filter '@objectstack/cli^...' build— exit 0 (the dependency closure, so every verdict above resolves workspace imports through real.d.tsrather than through an unbuilt world).pnpm lint(eslint . --no-inline-config) — exit 0 over 6368 files, 0 findings, run whole rather than narrowed, at8756045d. File count read from--format json; the population is eslint's own fromeslint.config.mjs.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack(it derives its own change set from the merge base): 59 commands, 57 run green. Exit codes were captured before any pipe.Two are NOT MEASURED, reported as failed measurements rather than as passes:
pnpm check:dual-build-cjs-loads—PREREQUISITE NOT MET(exit 3): it reads built output and several packages outside this diff's closure have nodist/. It needs a whole-workspace build, which is CI's run.pnpm check:type-check-debt— its prerequisite cleared after a rebuild, but the repo-wide--re-measureexceeds this container's ~10-minute foreground cap. Its siblingpnpm check:type-check-coverageran green.Three gates first reported
NOTHING was measuredagainst stale build state in this worktree (check:i18n,check:i18n-coverage,check:i18n-walk-parity) and are green after rebuilding@objectstack/metadata-core,@objectstack/cliand the four connector packages — the prescription in AGENTS.md → Multi-agent discipline §9, not a finding.Two things this seat could not verify, stated as such
objectui(positive control) returns refs,objectstack-ai/cloudanswerscould not read Username, and a repository that does not exist answers identically — so the probe cannot separate private from absent, and a REST probe returns 403 for readable repositories too (that 403 is the proxy, not GitHub). This does not block the change: the fix aligns the CLI to this repo's own declared schema, which is correct whatever cloud is doing. Cloud governs the urgency of the conflict window, not the direction of the fix. The card's "once it lands" is therefore not restated here as confirmed.origin/mainhas moved since the branch point (9 commits,c930f8597→a5d4e286b6). None of them touchespackages/cli/src/commands/package/publish.tsorpackages/spec/src/cloud/package.zod.ts; several touch other files inpackages/cli. This branch was not merged withmain, so the joint verdict is the merge queue's rebuilt generation.Docs drift advisory
Re-derived rather than inherited. Angle-bracket placeholders below are spelled without their brackets (
--manifest-id ID,local.SLUG) because this surface mutates tag-shaped fragments.1. The advisory's own number, re-derived on a clean tree. The bot flagged a provenance defect in its own run ("that checkout carried uncommitted changes"). Re-run on the exact tree it names —
a6204a5d87, the merge of head8756045d1ainto base142c01c88e— in a detached worktree whosegit status --porcelainwas empty before the run,node scripts/docs-audit/affected-docs.mjs --json 142c01c88ereportscomputedOn.dirty: falseand 12 docs (11 hand-written + 1 release-owned) across 11 anchors — the same rows, in the same order, with the same anchor attributions as the comment. On this PR the bot's number was exact; the provenance warning did not corrupt it here. The 11 anchors are the 9 symbols this diff adds or removes (MANIFEST_ID_REamong them, as a deletion), the literalcom.acme.crm, and theos package publishcommand.2. Every id-shaped literal in
content/, parsed through the declaration. Swept all 406 files undercontent/(not just the 12 rows) with five queries, each candidate parsed throughPackageSchema.shape.manifestIdimported frompackages/spec/src/cloud/package.zod.tsat this tree — the same declaration the CLI now uses, not a transcription of it. Discrimination control on the parser:com.acme.crmaccepted,com.acme.repair_deskrejected,crmrejected.--manifest-idcom.acme.token anywhere incontent/local.token anywhere incontent/No real id literal in
content/is falsified. All seven "rejects" are placeholders or prose, not values a reader would copy: the flag metavariableIDin the options table, the wordandfrom the sentence "falls back to --manifest-id and then to…", the prose fragment "local.+ a slug of the artifact name", the placeholderlocal.SLUG, and two unrelatedlocal.occurrences inkernel/cluster.mdxandreferences/automation/control-flow.mdxthat are not manifest ids at all. Every concrete id the docs print in a publish context —com.acme.crm,com.acme.encryption,com.acme.pii— parses green. Control for the fence query: 2027 fences scanned corpus-wide, 5 contain a publish invocation, so the single row is a reading and not an empty scan.3.
content/docs/releases/v9.mdx— read, not edited. No fact in it is falsified by this diff. Stated explicitly because the answer is a negative: itsos package publishpassage covers publish-then-install and the--visibilitydefault (private/org/marketplace, defaulting toorg) and says nothing about manifest ids, their shape, what the command accepts or refuses, or its error text. Nothing to hand back for a docs-only PR or an issue from that page.4. The retired error text appears nowhere in the docs. The sentence this PR deletes named
a-z0-9._-; searchingcontent/for that contract, forExpected reverse-domainand forInvalid manifest-idreturns 0 hits. Paired control sharing the vocabulary:reverse-domain/reverse domainreturns 22 lines across 12 files, all read. So the zero is a reading, not a broken query. Of the tenos package publishpages, none states what the command accepts or refuses for a manifest id; the two rule-bearing rows aredeployment/cli.mdx(the--manifest-idoptions row) andpublish-and-preview.mdx(the two-POST sequence), and both state the precedence —artifact.manifest.id, elselocal.plus a slug — without stating the condition under which the fallback is taken. Both remain true.One incompleteness, reported rather than fixed here (a docs-only change is not a rider on a code PR):
deployment/cli.mdx's--manifest-idrow presentslocal.plus a slug as an unconditional default. After this change a derived id the schema rejects — a manifest named2024 Appderiveslocal.2024-app, digit-first — is refused before any network call, so the default can now fail loudly. The row does not say so. It was not falsified (it never claimed the fallback always succeeds), which is why this is a follow-up and not a correction.5. The advisory's stated blind spot, answered by hand. The advisory says a page stating a rule by its inputs shares no identifier with the emitter that implements it, and this diff is emitter-side, so such a page cannot be listed on any run. Searched by hand for a page that states the id rule by its inputs:
reverse-domain/reverse domain(22 lines, all read — 20 in generatedreferences/**carrying the schema's own description, plusprotocol/kernel/index.mdxanddeployment/cli.mdx, both consistent), thenpackage id/manifest id(74 further lines, all generated field descriptions with no shape rule in them). Exactly one page incontent/states an id rule by its inputs:plugins/development.mdx§ Plugin ID Format, which gives a pattern allowing a single segment and calls out uppercase, spaces and underscores. That is the Studio plugin id, a different contract fromPackageSchema.manifestId, and this diff does not touch it. It is also the control this hand search needed: the search does surface a by-inputs rule statement in the vocabulary that would have failed, so the zero for package-manifest-id rules is a reading rather than an empty grep.验收备注
noted, not filed:packages/cli/src/commands/package/publish.tskeepsNAMESPACE_REas a hand-copied mirror ofManifestSchema.shape.namespace/PackageSchema.shape.namespace. Unlike the manifest-id copy it is pinned —test/package-publish-namespace.test.tsasserts the CLI rule and the spec schema agree value by value — so it is a maintained copy rather than a silent fork, and converting it is a different change from this card. Whoever next touches the publish preflight is the one who meets it.filed as #16891:os package publishsilently substitutes a derived id when the artifact declares amanifest.idthe publish path will not use — the declared, authorable value is discarded with nothing said, and the substitute is a permanent immutable identifier. This PR makes that path reachable for MORE values (it narrows whichmanifest.idvalues are adopted), so it is named here rather than left implicit; whether the fallback should warn, refuse, or be enforced at the producer is a decision this seat has no ruling for.noted, not filed:content/docs/deployment/cli.mdx's--manifest-idoptions row presentslocal.plus a slug as an unconditional default; after this change a derived id the schema rejects is refused before any network call, so the default can fail loudly and the row does not say so. Not falsified, so not corrected here — a docs-only change is not a rider on a code PR. Successor: the next docs-accuracy pass scoped to this page, which the drift advisory on this PR already lists.Generated by Claude Code