Repository navigation
Commit 4112752
fix(metadata-protocol): duplicatePackage parses an explicit targetNamespace through the manifest namespace declaration (#19829)
Fixes #19577
Clause-②: no (narrowing)
## What changed
`ObjectStackProtocolImplementation.duplicatePackage`
(`packages/metadata-protocol/src/protocol.ts`, located by symbol)
resolved its target namespace as `request.targetNamespace ??
deriveNamespaceFromPackageId(request.targetPackageId)`, and the loud
`400` fired only on `!targetNs`. So an explicit `targetNamespace`
crossed no gate. That value is written as the copy's
`manifest.namespace` and spliced into every copied object name as
`${targetNs}_${short}`, which means `targetNamespace: 'my-ns'` minted
`my-ns_ticket`, a name the object declaration refuses.
Both branches now pass **one** parse, before the source scan and before
the target package record is minted:
- **The declaration, by reference.** `ManifestSchema.shape.namespace`
(exported from `@objectstack/spec/kernel`, already imported by
`protocol.ts` for the id gate). It is the `manifest.namespace` rule
itself (`/^[a-z][a-z0-9_]{1,19}$/`,
`packages/spec/src/kernel/manifest.zod.ts`), not a copied regex. The
parse input is `targetNs ?? ''`, because the declaration is
`.optional()` and would otherwise pass an absent value.
- **Refused, not sanitised.** An explicit value the declaration refuses
is refused. It is never rewritten the way the derivation sanitises an
id.
- **The declaration's sentence.** The refusal opens with the key and the
value, then carries the declaration's own issue message verbatim. For
example: `Invalid package namespace 'my-ns' on` + backtick
`targetNamespace` backtick + `. Namespace must be 2-20 chars, lowercase
alphanumeric + underscore. It becomes the copy's manifest.namespace and
the prefix of every copied object name.` The derived branch keeps its
`Cannot derive a package namespace from 'ID'. Pass targetNamespace
explicitly.` opener. Its reworded rule clause is replaced by the same
declaration sentence. Neither message opens with a bracketed tag.
- **No new error code.** Both refusals throw `{ statusCode: 400 }` with
no `code`, exactly as the id refusal above them does. An HTTP boundary
(`resolveThrownHttpError`) therefore answers `400 VALIDATION_ERROR` on
both branches, which is the status-derived code the derived branch
already answered.
- The comment that blessed the gap ("An explicitly declared
`targetNamespace` still wins untouched …") is rewritten to state the new
truth: the explicit value still wins over the default, but it is parsed.
## Dispatch assumptions, measured
- **A1 (confirmed).** On base `c11852406` the construct is exactly as
described. The ablation below shows the explicit branch resolving for
every value the declaration refuses.
- **A2 (refuted in part).** There is **no** exported namespace validator
or regex that `deriveNamespaceFromPackageId` sanitises toward. It tests
a module-private `NAMESPACE_RE` in
`packages/spec/src/kernel/namespace-prefix.ts`, which is byte-identical
to the manifest declaration's regex but not exported. The nearest
published declaration is `ManifestSchema.shape.namespace`, and that is
what this PR uses. The same shape was used for the id:
`ManifestSchema.shape.id`.
- **A3 (confirmed).** The comment is rewritten.
- **A4 (confirmed).** No new code is minted. The explicit refusal reuses
the derived branch's status-only shape, and the tests assert
`declaredCode` is absent.
- **Producer location.** The fix is at the producer. `POST
/api/v1/packages/:id/duplicate`
(`packages/runtime/src/domains/packages.ts`) forwards a string
`targetNamespace` verbatim to this method and maps the throw with
`errorFromThrown(e, 500)`. That route therefore answers `400
VALIDATION_ERROR` with no route change.
## Tests
New file:
`packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts`.
The existing `protocol.install-manifest-id.test.ts` is **untouched** and
still green, derived-branch refusal included.
- **Refusal, 8 cases.** Hyphen `my-ns`, uppercase, leading digit,
leading underscore, 1 char, 21 chars, padded `' leave2 '`, and `''`.
Each case asserts:
- the envelope via `resolveThrownHttpError`: `status 400`, `code
VALIDATION_ERROR`, `declaredCode` undefined;
- that the message **starts with** its first sentence;
- that the message contains the declaration's own issue message, read
from `ManifestSchema.shape.namespace.safeParse` and not retyped;
- that nothing was installed, scanned or saved.
- **Lit control, 4 cases.** `leave2`, `leave_copy`, 20 chars and 2
chars. Each still duplicates, `manifest.namespace` equals the value, and
the written object names are exactly `${ns}_ticket`.
- **Derived branch, 1 case.** Same envelope, and the declaration's
sentence follows the `targetNamespace` remedy.
Runs (all at HEAD `5e843ab81`, through `scripts/pm/os-verify-lock.sh`):
- `pnpm --filter '@objectstack/metadata-protocol^...' build`: VERDICT
command-exit 0.
- `pnpm --filter @objectstack/metadata-protocol exec vitest run
--maxWorkers=2 src/protocol.duplicate-package-target-namespace.test.ts
src/protocol.install-manifest-id.test.ts
src/protocol.bracketed-refusal-opener-absence.test.ts`: `Test Files 3
passed (3)`, `Tests 36 passed (36)`. The bracketed-opener pin is green.
- `pnpm --filter @objectstack/metadata-protocol exec vitest run
--maxWorkers=2` (whole package): `Test Files 188 passed | 3 skipped
(191)`, `Tests 2671 passed | 19 skipped (2690)`, VERDICT command-exit 0.
- `pnpm --filter @objectstack/metadata-protocol typecheck`: exit 0. `tsc
--noEmit --listFiles` includes the new test file (1 hit).
- Consumer check: `pnpm --filter @objectstack/metadata-protocol build &&
pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2
src/protocol-package-lifecycle.test.ts` gave `10 passed`. That test is a
downstream caller passing the conforming `targetNamespace: 'iojn2'`
through the built dist. The runtime integration suite
`package-duplicate-adopt-org-scope.integration.test.ts` also uses
`iojn2` only; it is declared to CI and was not run here.
**Ablation (one-shot, after commit `5e843ab81`).** Tool: `node
scripts/ablation-replace.mjs`, which restores itself on EXIT/INT/TERM,
wrapped in the verify lock. It replaced the anchor `if (targetNs == null
|| !declaredTargetNs.success) {` with `if (!targetNs) {`, which is the
base guard, so the new parse is removed.
- On disk: anchor x1 → x0, replacement x0 → x1, blob `74ee23265ed4` →
`63700594eaa0`. The subject resolves via the relative `./protocol.js`,
so it runs from `src/` and needed no rebuild.
- Result: `Tests 7 failed | 6 passed (13)`. All 7 explicit refusal cases
failed with `expected the call to be refused, but it resolved`.
- The 6 that stayed green are what was predicted: `''` (the base
`!targetNs` guard already caught it), the 4 lit controls, and the
derived case.
- Restore: blob after restore `74ee23265ed4` equals the HEAD blob, and
`git diff HEAD` is empty.
## Gates
`node scripts/pm/dispatch-gates.mjs --commands` was run on HEAD
`5e843ab81`: 61 commands, each run with its exit captured before any
pipe. `--ran` verdict: `✓ dispatch-gates --ran: 61 derived famil(ies)
accounted for — 59 run, 2 NOT-MEASURED (2 DERIVED from a recorded exit
3).`
- `pnpm check:lean-entry-closure` first answered exit 3 (PREREQUISITE
NOT MET: objectql dist absent). It was re-run after `pnpm exec turbo run
build --filter=@objectstack/objectql --concurrency=2` and gave exit 0
(`2 published condition(s) measured from a real load`). The record
carries the rerun.
- NOT MEASURED: `pnpm check:dual-build-cjs-loads`. Reason: PREREQUISITE
NOT MET, since it reads the built output of every package (68 lack dist
here). A full `pnpm build` does not fit the foreground cap, and this
diff changes no `package.json`, `exports` or build config. CI runs it on
a fresh full build.
- NOT MEASURED: `pnpm check:type-check-debt`. Reason: PREREQUISITE NOT
MET, since it needs the whole packages build closure. The only package
touched is `@objectstack/metadata-protocol`, whose own `typecheck` exits
0.
```text
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-changeset-no-major.mjs --self-test :: exit 0
node scripts/check-ci-filter-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-adoption.mjs :: exit 0
node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-keyed-text-bounds.mjs :: exit 0
node scripts/check-keyed-text-bounds.mjs --self-test :: exit 0
node scripts/check-platform-object-tenancy-census.mjs :: exit 0
node scripts/check-platform-object-tenancy-census.mjs --self-test :: exit 0
node scripts/check-plugin-teardown-shape.mjs :: exit 0
node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0
node scripts/check-registry-log-declared.mjs :: exit 0
node scripts/check-registry-log-declared.mjs --self-test :: exit 0
node scripts/check-rest-log-spy-declared.mjs :: exit 0
node scripts/check-rest-log-spy-declared.mjs --self-test :: exit 0
node scripts/check-system-context-census.mjs :: exit 0
node scripts/check-system-context-census.mjs --self-test :: exit 0
node scripts/check-undeclared-dep-imports.mjs :: exit 0
node scripts/check-undeclared-dep-imports.mjs --self-test :: exit 0
node scripts/docs-audit/check-affected-docs.mjs :: exit 0
node scripts/docs-audit/check-drift-comment.mjs :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:dispatcher-error-vocabulary :: exit 0
pnpm check:doc-authoring :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:dts-closure :: exit 0
pnpm check:dual-build-cjs-loads :: exit 3
pnpm check:durability-log-level :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:filter-alias-parity :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:issue-citations :: exit 0
pnpm check:lean-entry-closure :: exit 0
pnpm check:logger-receiver-detach :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:objectql-double-limit :: exit 0
pnpm check:objectui-changeset :: exit 0
pnpm check:org-identifier :: exit 0
pnpm check:page-declaration-shape :: exit 0
pnpm check:pm-changeset-deadline-census :: exit 0
pnpm check:published-files :: exit 0
pnpm check:query-options-erasure :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:slot-lookup :: exit 0
pnpm check:sourcemap-no-sources-content :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:tier-file-adoption :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 3
pnpm check:watch-hint-literal :: exit 0
pnpm check:where-matcher :: exit 0
```
**Narrowed lint (measured).** `pnpm exec eslint --no-inline-config
--format json` was run over the two changed TS files: exit 0, and the
JSON reports 2 files, 0 errors, 0 warnings.
- ① Population, from `eslint.config.mjs`: `files:
['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']` plus the
`packages/**/*.{ts,…}` blocks. The changeset `.md` is in no `files`
glob, so these two files are the whole lintable part of the diff.
- ② File count: 2, read from the JSON output.
- ③ Invariance: the config never enables type-aware linting (no
`parserOptions.project`, no typed rules; stated at `eslint.config.mjs`
around line 327). The baselines it reads
(`scripts/slot-lookup-baseline.json`,
`scripts/query-options-erasure-baseline.json`) are untouched. So this
diff cannot move any verdict on an untouched file.
## Changeset
`.changeset/19577-duplicate-package-explicit-namespace.md`: `minor` for
`@objectstack/metadata-protocol`, declaring `Clause-②: no (narrowing)`,
a **BREAKING for callers** banner and an `adr-0087: not-required
(no-migration-prescription)` disposition. It was re-graded from `patch`
in a patch round on the seat's call, following the same door's precedent
PR #19574. The claim's `Clause-②` VALUE (`no`) is unchanged; the
`(narrowing)` arm carries the direction the changeset gates read.
## Acceptance notes
- **The declaration's sentence under-states its own regex.**
`ManifestSchema.shape.namespace`'s message, `Namespace must be 2-20
chars, lowercase alphanumeric + underscore`, and its TSDoc rule (`2-20
characters, lowercase letters, digits, and underscores only`) both omit
"starts with a lowercase letter". Measured: `safeParse('1leave')` and
`safeParse('_leave')` refuse with that sentence, although both values
satisfy every clause of it. This PR surfaces the sentence verbatim, as
the id precedent requires, so the explicit-refusal message inherits the
gap. The derived-branch message used to spell the leading-letter clause
itself. That is a spec-side fix outside this card's file surface, and it
is reported to the seat as a finding.
- `packages/spec/src/kernel/namespace-prefix.ts` keeps a private
`NAMESPACE_RE` that duplicates the manifest declaration's regex. It is a
second declaration of one rule; no drift is observed today. Carrier:
none.
- Reserved namespaces (`sys`, `base`, `system`) are named in the
`manifest.namespace` TSDoc but are not part of its regex, so
`targetNamespace: 'sys'` passes this parse. This PR validates the
charset declaration only. Whether a `sys` copy is refused downstream is
NOT MEASURED. Carrier: none.
- #19576 (install-local door) is not addressed here. The #19417
derived-seam work is untouched apart from its refusal sentence now
coming from the declaration.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 71ef221 commit 4112752
3 files changed
Lines changed: 235 additions & 12 deletions
File tree
- .changeset
- packages/metadata-protocol/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
19371 | 19371 | | |
19372 | 19372 | | |
19373 | 19373 | | |
19374 | | - | |
19375 | | - | |
| 19374 | + | |
| 19375 | + | |
| 19376 | + | |
| 19377 | + | |
| 19378 | + | |
| 19379 | + | |
| 19380 | + | |
| 19381 | + | |
| 19382 | + | |
| 19383 | + | |
| 19384 | + | |
| 19385 | + | |
| 19386 | + | |
| 19387 | + | |
| 19388 | + | |
| 19389 | + | |
| 19390 | + | |
| 19391 | + | |
| 19392 | + | |
19376 | 19393 | | |
19377 | 19394 | | |
19378 | | - | |
19379 | | - | |
19380 | | - | |
19381 | | - | |
19382 | | - | |
19383 | | - | |
19384 | | - | |
| 19395 | + | |
| 19396 | + | |
| 19397 | + | |
| 19398 | + | |
| 19399 | + | |
| 19400 | + | |
| 19401 | + | |
19385 | 19402 | | |
19386 | 19403 | | |
19387 | | - | |
19388 | | - | |
19389 | | - | |
| 19404 | + | |
| 19405 | + | |
| 19406 | + | |
| 19407 | + | |
| 19408 | + | |
| 19409 | + | |
| 19410 | + | |
| 19411 | + | |
19390 | 19412 | | |
19391 | 19413 | | |
19392 | 19414 | | |
| |||
0 commit comments