Repository navigation
fix(metadata-protocol): duplicatePackage parses an explicit targetNamespace through the manifest namespace declaration - #19829
Conversation
…espace through the manifest namespace declaration An explicit `targetNamespace` was taken raw and spliced into every copied object name, while the derived default already had to satisfy the namespace charset. Both branches now pass one `ManifestSchema.shape.namespace` parse before anything is scanned or minted, refusing with the declaration's own sentence and the status-derived 400 VALIDATION_ERROR the derived branch already answered. Claude-Session: https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr Co-authored-by: Claude <noreply@anthropic.com>
📓 Docs Drift Check1 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to list — not a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run. What this run could not see
Coarse fallback — 11 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 0e2edb0fd68861017d9a980f5b730f73d6c715d3 && git checkout 0e2edb0fd68861017d9a980f5b730f73d6c715d3
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 a27d6d9348277ae8e8e276712eed7a391e389962 && git checkout -B drift-repro 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 && git merge --no-ff a27d6d9348277ae8e8e276712eed7a391e389962
node scripts/docs-audit/affected-docs.mjs --json 1cacfe4a425dc0c2cdd4eb5d0088d5b3f5a16c92 |
…owing (minor) An explicit targetNamespace outside the manifest.namespace declaration used to be accepted verbatim and is now refused, so the changeset declares Clause-② no (narrowing), ships minor, carries the BREAKING banner naming the refused values, and states its ADR-0087 disposition. Claude-Session: https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Rendered by an isolated at-tier reviewer subagent that was fed the card, its rulings and this PR only, and adopted by the ① Derived judgments
② Semver level
③ Boundary flags
Implemented-by: VERDICT: PASS Generated by Claude Code |
…ce (objectstack-ai#19842) Fixes objectstack-ai#19823 Clause-②: no (narrowing) ## Measurement first — committed before the fix (`8d145637a8`) Triage (comment 5792299650) ordered the three predictions recorded MEASURED or REFUTED before any fix. The instrument is `packages/drivers/driver-turso/src/turso-remote-deferred-ddl.test.ts`, first committed as a characterisation of the unrefused behaviour. It replays, against a remote-mode `TursoDriver` over `makeLibsqlSqliteStub` (a real SQLite database wearing the `@libsql/client` interface) wrapped in a recorder that logs every statement the transport sends, the exact driver calls a `deferSchemaDdl` boot makes: 1. `setDeferredDdl(true)`: the CLI's `DeferSchemaDdlPlugin.init`. 2. `syncSchemasBatch(...)`: `ObjectQLPlugin.start()`'s boot sync. It takes the batch door because this driver answers `supports.batchSchemaSync === true` and has the method, which the test also asserts. 3. `syncSchema(...)`: the composed-host coverage pass (`engine.syncObjectSchema`) that `plan` / `apply` run. 4. `previewDeferredSchemaWork()` / `flushDeferredSchemaDdl()`: what `plan` prints, and what `apply` performs after its confirm prompt. Seed: an existing remote table `probe` missing one declared column and holding a naive datetime (`2025-07-28 00:00:00`), plus a declared object `fresh` with no table. Run on base `1cacfe4a42`: 6 of 6 characterisation tests green. | prediction | door | verdict | evidence (recorded statements, disk) | |:--|:--|:--|:--| | (a) the dry run performs DDL | `syncSchemasBatch` (engine boot sync) | **MEASURED** | `CREATE TABLE "fresh" (...)`, `ALTER TABLE "probe" ADD COLUMN "why" TEXT` | | (a) | `syncSchema` / `initObjects` | **MEASURED** | the same CREATE and ALTER | | (b) the dry run rewrites rows | `syncSchemasBatch` | **REFUTED** | no row write; the naive value is still on disk | | (b) | `syncSchema` / `initObjects` | **MEASURED** | `update "probe" set "at" = (case ... end) where rowid in (select ...)`; the row now reads `2025-07-28T00:00:00.000Z` | | (c) the plan reports no pending work | every door | **MEASURED** | `previewDeferredSchemaWork()` answers `[]`, `flushDeferredSchemaDdl()` answers `[]`, `deferredSchemaObjectCount` is 0 | **Triage's exits: neither fires.** Exit one (all three REFUTED) does not: the plan path reaches DDL on every door. Exit two (a destructive statement) does not: no door emitted a `DROP` or a type change. The only row writes are the canonical backfill's `update`, which rewrites a value's spelling and not the instant it names. **Not measured end to end:** the `os migrate plan` binary itself against a live libsql remote. The CLI's build closure is 58 workspace packages. The driver half is measured above. The CLI half is read from source on `1cacfe4a42`: - `packages/cli/src/utils/schema-migrate.ts` `DeferSchemaDdlPlugin.init` calls `setDeferredDdl(true)` in Phase 1. - `packages/core/src/kernel.ts` `bootstrap()` rethrows an init error unwrapped, and `Runtime.start()` is `kernel.bootstrap()`. - `plan` prints `error.message`. Under `--json` it emits `error` plus `code` through `errorCodeFields`. ## Dispatch assumptions, measured - **A1 — confirmed, and widened.** The `isRemote` arms of `syncSchema` and `initObjects` do run remote DDL plus `backfillRemoteCanonicalTemporalQuietly()`, and never read the flag (`deferredDdl`: 0 hits under `packages/drivers/driver-turso/src/` on base). The engine's boot sync reaches neither of them, though. It takes a third door, `TursoDriver.syncSchemasBatch`, whose remote arm forwards straight to `RemoteTransport.syncSchemasBatch`: DDL without the backfill. So on the ordinary `plan` path the DDL is certain, and the backfill arrives through the coverage pass. - **A2 — confirmed.** The CLI refuses only on `typeof driver.setDeferredDdl !== 'function'`. The measurement shows the inherited setter accepting `true` on the remote face without a throw. - **A3 — five commands arm deferral; all five carry a dry-run or confirm-before-change promise.** So the refusal disables nothing that promised to write, and there is no `needs_decision` stop. Enumerated from `deferSchemaDdl: true` under `packages/cli/src/commands/**`. `setDeferredDdl` has no other caller in this repository. | command | its promise | on remote Turso before this PR | after | |:--|:--|:--|:--| | `os migrate plan` | dry run, "never mutates the schema" | DDL (plus the backfill on the coverage pass), then "no pending work", exit 0 | refused, exit 1, nothing sent | | `os migrate apply` | "nothing is written before you say yes" | DDL during boot, then the confirm prompt; the flush reports `[]` | refused, exit 1 | | `os migrate duplicates` | read-only inventory; "cannot change the install it is describing" | DDL during boot | refused (`boot_failed` plus the detail) | | `os migrate account-issuer` | read-only pre-flight | DDL during boot | refused | | `os migrate multi-value-columns` | dry run by default; `--apply` promises "the only statements this command may run are the remedy's" | DDL during boot, in both modes | refused | - **A4 — confirmed.** `previewDeferredSchemaWork` / `flushDeferredSchemaDdl` read `deferredSchemaObjects`, which only the Knex `SqlDriver.initObjects` fills. On remote both answer `[]`, per the table above. `remote-canonical-backfill.ts` already said so in prose. Through the CLI, a Turso URL always builds a **remote** driver. `standalone-stack.ts` hands the driver `{ url, authToken }` with no `syncUrl`, and a `file:` URL is classified `sqlite`. So all five commands refuse on every Turso URL the CLI accepts. ## The fix `TursoDriver` overrides `setDeferredDdl`: arming (`true`) in `remote` transport mode throws before any statement is sent. Disarming is accepted, and `local` / `replica` delegate to `SqlDriver` unchanged. The refusal (`refuseRemoteDeferredDdl`, beside the transaction and auto-number refusals) answers `code: 'NOT_IMPLEMENTED'`, `status: 501`. That is a `StandardErrorCode` member and the envelope this transport already uses for its other capability gaps, so there is **no new error code**. **Whose message the operator reads — measured, not assumed.** The CLI's own refusal ("does not support deferred schema DDL ... Upgrade @objectstack/driver-sql") cannot fire, because the method exists. The driver's throw propagates out of `DeferSchemaDdlPlugin.init` unwrapped, and the command prints its `message`. So the driver's own message is the operator contract, and the CLI is untouched: `packages/cli/src/utils/schema-migrate.ts` and `packages/cli/src/commands/migrate/*` were read only. Its first sentence: > Deferred schema DDL is not supported by the Turso REMOTE transport (this datasource's transport mode is `remote`), so a command that promises a dry run or a confirmation before any schema change cannot keep that promise against it. The rest says why (remote DDL is immediate, and a remote sync rewrites temporal values in place), what it replaced, and what to do instead: preview against a local SQLite copy (a `file:` URL), or let an ordinary `os serve` / `os start` boot perform the additive sync. Why at the setter: every deferring caller passes through it, and it runs before any schema work. A refused arm has sent nothing, and it leaves the driver un-armed, so an ordinary boot sync on the same instance is unchanged. Honouring the deferral remotely (recording objects, a remote preview and flush) is new capability with no measured pull, and it is not attempted here. ## Tests — `turso-remote-deferred-ddl.test.ts` (8 tests) - **Refusal envelope:** `toBeInstanceOf(Error)`, `code === 'NOT_IMPLEMENTED'`, `status === 501`, and the message starts with the first sentence above, spelled out in the test rather than imported. - **The measurement's own scenario now performs NOTHING:** the full deferred-boot replay against remote pending work rejects with the envelope. The recorder holds **zero statements** (so zero DDL and zero row writes), and the disk is byte-for-byte what the command found: the tables, the columns and the naive value. - **Disarm:** `setDeferredDdl(false)` is accepted and sends nothing. - **Lit control, remote ordinary boot (deferral NOT armed):** after a refused arm on the same driver, `syncSchemasBatch` still emits the CREATE and the ALTER, and `syncSchema` still runs the backfill `update` (the stored value becomes `2025-07-28T00:00:00.000Z`). - **Lit controls, `local` and `replica`:** arming is accepted. The same replay records instead of performing (`fresh` absent, `why` absent, `deferredSchemaObjectCount` 2). The preview lists `create_table fresh [label]` and `add_columns probe [why]`. The flush performs exactly the previewed work, and the libsql client carried no DDL. - **Pin:** `supports.batchSchemaSync === true` and `syncSchemasBatch` is a function, the two facts the engine ANDs to pick the batch door. Package suite at `67be9850fd`: `pnpm --filter @objectstack/driver-turso exec vitest run --maxWorkers=2` gave **56 files / 1299 tests passed**. `pnpm --filter @objectstack/driver-turso typecheck` exited 0, and `tsc --noEmit --listFiles` includes the new file (56 test files in the program). ## Ablation — committed fix, then removed, then restored At HEAD `67be9850fd`, through `scripts/ablation-replace.mjs` in WRAP mode, with a shell `trap` restoring `git checkout HEAD --` on the absolute path. The anchor was `if (deferred && this.isRemote) refuseRemoteDeferredDdl();`, replaced by a marker comment. - **On disk:** anchor 1 to 0, marker 0 to 1, blob `79960fb8a08f` to `d16059b40f0a`. The in-mutation `grep -c` read anchor 0 and marker 1. The subject is imported from `src/` (a relative `./turso-driver.js`), so no `dist/` leg applies. - **Direction predicted before the run:** the three tests that need the refusal go red (the envelope, performs-NOTHING, and the refused-arm control), and the five that do not stay green. - **Observed:** `3 failed | 5 passed (8)`, exactly those three: `expected null to be an instance of Error`, then `expected undefined to be 'NOT_IMPLEMENTED'` twice. - **Restore:** the blob after restore equals HEAD (`79960fb8a08f`), `git diff HEAD` is empty, and `git status --porcelain` is empty. ## Gates — derived on the final commit `67be9850fd` `node scripts/pm/dispatch-gates.mjs --commands` (no paths) derived **61** commands. Every exit code was captured before any pipe, and each command was recorded with it: - **58 exited 0.** Among them: `check:adr-0087-registration --base origin/main` accepted the changeset as `[BREAKING+clause-②-narrowing] not-required (no-migration-prescription)`, and `check:changeset-no-major` reported no `major` (the level axis is not applicable locally, since there is no PR payload). Also green: `check:empty-changeset`, `check:doc-authoring`, `check:nul-bytes`, `check:object-def-param-keys`, `check:published-files`, `check:dts-closure`, `check:sourcemap-no-sources-content`, `check:test-source-alias`, `check:type-check-coverage`, `check:cross-package-test-inputs` and `check:engine-double-contract`. - **3 exited 3, NOT MEASURED (PREREQUISITE NOT MET):** `check:dual-build-cjs-loads` and `check:type-check-debt` need the whole workspace built, and `check:lean-entry-closure` needs `objectql/dist`. They are declared to CI. A narrow probe of the half this diff touches: the built `driver-turso` `dist/index.js` loads under `require` and `dist/index.mjs` under `import`, and both export `TursoDriver` (exit 0). - **`--ran` reconciliation:** `✓ dispatch-gates --ran: 61 derived famil(ies) accounted for — 58 run, 3 NOT-MEASURED (3 DERIVED from a recorded exit 3).` It reported 0 UNRUN. - **CLI integration tier:** declared to CI. No spawn entry or CLI file is touched. **Driver conformance ledger (lane commitment)**, identical before the first edit and after the final commit: - `OK — 50 covered cell(s), 0 in the DEBT ledger, 0 exempt.` - Dialect axis: `8 conformance suite(s) ... 7 run the matrix, 1 declare named cell(s), 0 in the DIALECT ledger.` No DEBT added. ## Changeset judgement — a declared narrowing A remote `setDeferredDdl(true)` used to resolve, and a remote `os migrate plan` used to exit 0. Both now refuse. That narrows what the published driver accepts, so this PR follows the shape of this seat's sibling PR objectstack-ai#19829: `Clause-②: no (narrowing)`, a `minor` bump for `@objectstack/driver-turso`, a **BREAKING** banner, and the ADR-0087 disposition `not-required (no-migration-prescription)`. Nothing authorable is removed or renamed, and `setDeferredDdl` keeps its name and signature. The claim carries bare `Clause-②: no`, and its own rationale calls this change a narrowing, so the arm is added and the base value is unchanged. ## Acceptance notes - `content/docs/deployment/cli.mdx`, section "Nothing is written before you confirm", does not mention that a remote Turso datasource is refused. It is incomplete rather than wrong: plan and apply still write nothing. Successor: none. - External callers of `setDeferredDdl` on a remote `TursoDriver` outside this repository (for example the cloud repository) were NOT MEASURED, because that repository is not reachable from this container. Inside this repository the only caller is the CLI plugin. - Two reproducible defects surfaced during the measurement. They are **not fixed here** and are reported to the seat for filing: the remote batch door's missing read-coercion registration and backfill, and remote `detectManagedDrift()` reading the dummy Knex connection. --- _Generated by [Claude Code](https://claude.ai/code/session_01TEhopqrWQYBycZzyJHpAZr)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
Fixes #19577
Clause-②: no (narrowing)
What changed
ObjectStackProtocolImplementation.duplicatePackage(packages/metadata-protocol/src/protocol.ts, located by symbol) resolved its target namespace asrequest.targetNamespace ?? deriveNamespaceFromPackageId(request.targetPackageId), and the loud400fired only on!targetNs. So an explicittargetNamespacecrossed no gate. That value is written as the copy'smanifest.namespaceand spliced into every copied object name as${targetNs}_${short}, which meanstargetNamespace: 'my-ns'mintedmy-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:
ManifestSchema.shape.namespace(exported from@objectstack/spec/kernel, already imported byprotocol.tsfor the id gate). It is themanifest.namespacerule itself (/^[a-z][a-z0-9_]{1,19}$/,packages/spec/src/kernel/manifest.zod.ts), not a copied regex. The parse input istargetNs ?? '', because the declaration is.optional()and would otherwise pass an absent value.Invalid package namespace 'my-ns' on+ backticktargetNamespacebacktick +. 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 itsCannot 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.{ statusCode: 400 }with nocode, exactly as the id refusal above them does. An HTTP boundary (resolveThrownHttpError) therefore answers400 VALIDATION_ERRORon both branches, which is the status-derived code the derived branch already answered.targetNamespacestill wins untouched …") is rewritten to state the new truth: the explicit value still wins over the default, but it is parsed.Dispatch assumptions, measured
c11852406the construct is exactly as described. The ablation below shows the explicit branch resolving for every value the declaration refuses.deriveNamespaceFromPackageIdsanitises toward. It tests a module-privateNAMESPACE_REinpackages/spec/src/kernel/namespace-prefix.ts, which is byte-identical to the manifest declaration's regex but not exported. The nearest published declaration isManifestSchema.shape.namespace, and that is what this PR uses. The same shape was used for the id:ManifestSchema.shape.id.declaredCodeis absent.POST /api/v1/packages/:id/duplicate(packages/runtime/src/domains/packages.ts) forwards a stringtargetNamespaceverbatim to this method and maps the throw witherrorFromThrown(e, 500). That route therefore answers400 VALIDATION_ERRORwith no route change.Tests
New file:
packages/metadata-protocol/src/protocol.duplicate-package-target-namespace.test.ts. The existingprotocol.install-manifest-id.test.tsis untouched and still green, derived-branch refusal included.my-ns, uppercase, leading digit, leading underscore, 1 char, 21 chars, padded' leave2 ', and''. Each case asserts:resolveThrownHttpError:status 400,code VALIDATION_ERROR,declaredCodeundefined;ManifestSchema.shape.namespace.safeParseand not retyped;leave2,leave_copy, 20 chars and 2 chars. Each still duplicates,manifest.namespaceequals the value, and the written object names are exactly${ns}_ticket.targetNamespaceremedy.Runs (all at HEAD
5e843ab81, throughscripts/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 --listFilesincludes the new test file (1 hit).pnpm --filter @objectstack/metadata-protocol build && pnpm --filter @objectstack/objectql exec vitest run --maxWorkers=2 src/protocol-package-lifecycle.test.tsgave10 passed. That test is a downstream caller passing the conformingtargetNamespace: 'iojn2'through the built dist. The runtime integration suitepackage-duplicate-adopt-org-scope.integration.test.tsalso usesiojn2only; 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 anchorif (targetNs == null || !declaredTargetNs.success) {withif (!targetNs) {, which is the base guard, so the new parse is removed.74ee23265ed4→63700594eaa0. The subject resolves via the relative./protocol.js, so it runs fromsrc/and needed no rebuild.Tests 7 failed | 6 passed (13). All 7 explicit refusal cases failed withexpected the call to be refused, but it resolved.''(the base!targetNsguard already caught it), the 4 lit controls, and the derived case.74ee23265ed4equals the HEAD blob, andgit diff HEADis empty.Gates
node scripts/pm/dispatch-gates.mjs --commandswas run on HEAD5e843ab81: 61 commands, each run with its exit captured before any pipe.--ranverdict:✓ 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-closurefirst answered exit 3 (PREREQUISITE NOT MET: objectql dist absent). It was re-run afterpnpm exec turbo run build --filter=@objectstack/objectql --concurrency=2and gave exit 0 (2 published condition(s) measured from a real load). The record carries the rerun.pnpm check:dual-build-cjs-loads. Reason: PREREQUISITE NOT MET, since it reads the built output of every package (68 lack dist here). A fullpnpm builddoes not fit the foreground cap, and this diff changes nopackage.json,exportsor build config. CI runs it on a fresh full build.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 owntypecheckexits 0.Narrowed lint (measured).
pnpm exec eslint --no-inline-config --format jsonwas run over the two changed TS files: exit 0, and the JSON reports 2 files, 0 errors, 0 warnings.eslint.config.mjs:files: ['**/*.{ts,tsx,mts,cts,js,jsx,mjs,cjs}']plus thepackages/**/*.{ts,…}blocks. The changeset.mdis in nofilesglob, so these two files are the whole lintable part of the diff.parserOptions.project, no typed rules; stated ateslint.config.mjsaround 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:minorfor@objectstack/metadata-protocol, declaringClause-②: no (narrowing), a BREAKING for callers banner and anadr-0087: not-required (no-migration-prescription)disposition. It was re-graded frompatchin a patch round on the seat's call, following the same door's precedent PR #19574. The claim'sClause-②VALUE (no) is unchanged; the(narrowing)arm carries the direction the changeset gates read.Acceptance notes
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')andsafeParse('_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.tskeeps a privateNAMESPACE_REthat duplicates the manifest declaration's regex. It is a second declaration of one rule; no drift is observed today. Carrier: none.sys,base,system) are named in themanifest.namespaceTSDoc but are not part of its regex, sotargetNamespace: 'sys'passes this parse. This PR validates the charset declaration only. Whether asyscopy is refused downstream is NOT MEASURED. Carrier: none.manifest.idpositionally and never parses the body throughManifestSchema— POST /packages answers 201 to ids thatMANIFEST_ID_PATTERN(spec,defineStack,os build, the publish face) refuses #19417 derived-seam work is untouched apart from its refusal sentence now coming from the declaration.Generated by Claude Code