From 8756045d1a4b82cc711e09d105d2443abace1af7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 8 Sep 2026 14:03:11 +0000 Subject: [PATCH] fix(cli): parse `os package publish` manifest ids through PackageSchema.manifestId MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `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 Claude-Session: https://claude.ai/code/session_015QE8qk46e5CHJxyQEUjbf8 --- .../cli-manifest-id-parse-through-schema.md | 13 + packages/cli/src/commands/package/publish.ts | 121 +++++- .../test/package-publish-manifest-id.test.ts | 355 ++++++++++++++++++ 3 files changed, 476 insertions(+), 13 deletions(-) create mode 100644 .changeset/cli-manifest-id-parse-through-schema.md create mode 100644 packages/cli/test/package-publish-manifest-id.test.ts diff --git a/.changeset/cli-manifest-id-parse-through-schema.md b/.changeset/cli-manifest-id-parse-through-schema.md new file mode 100644 index 0000000000..4cc6bf4c72 --- /dev/null +++ b/.changeset/cli-manifest-id-parse-through-schema.md @@ -0,0 +1,13 @@ +--- +"@objectstack/cli": patch +--- + +`os package publish` now decides what a manifest id is by parsing it through `PackageSchema.manifestId` — the schema for the very column it publishes into — instead of testing it against a hand-copied look-alike. + +The command carried its own rule (`MANIFEST_ID_RE`, a case-insensitive "starts alphanumeric, then any of a-z 0-9 dot underscore hyphen, up to 255 chars"), which is looser than the declared contract on every axis. The local preflight therefore **admitted what the control plane refuses**: a single segment (`crm`), an underscore (`com.acme.repair_desk`), upper case (`COM.ACME.CRM`), a digit-first segment (`9foo.bar`), an empty segment (`com..acme`) and a trailing dot (`com.acme.`). The preflight passed, the request went out, and the server answered `400`. Its error text, when it did fire, named a contract (`a-z0-9._-`) that does not exist — so a user who followed the message walked into a second refusal. + +- **One rule, both paths.** `MANIFEST_ID_RE` is deleted. The explicit `--manifest-id` / `objectstack.manifest.json` path and the derive path (`deriveManifestId`, which adopts `artifact.manifest.id`) now ask the same imported schema. They previously disagreed with each other as well as with the declaration: the derive path additionally required a dot, so a bare `crm` was blocked there and accepted on the explicit path. That extra condition is gone because the schema subsumes it — its pattern requires at least two segments. +- **The refusal text is quoted from the schema**, from its own `invalid_format` issue plus its `.describe()`, so it can no longer drift from the rule it describes. +- **A derived id the schema rejects is refused, not rewritten.** `slugify` has no letter-first rule, so an app named `2024 App` derives `local.2024-app` — digit-first, and rejected. That is now refused before any network call, with a message naming where the id came from and how to set one (`--manifest-id`, `manifestId` in `objectstack.manifest.json`, or `manifest.id`). It is deliberately not normalised into some other id: `manifestId` is immutable once published, and minting a different permanent global identifier than the inputs imply is worse than saying what is wrong. + +Publishing is unaffected for every id the control plane accepts — a legal reverse-domain id passes both paths with unchanged bytes. What changes is that the ids the server was going to reject are now refused locally, with the real rule in the message. diff --git a/packages/cli/src/commands/package/publish.ts b/packages/cli/src/commands/package/publish.ts index 172145c1c5..d4f4cc1ed7 100644 --- a/packages/cli/src/commands/package/publish.ts +++ b/packages/cli/src/commands/package/publish.ts @@ -29,11 +29,54 @@ import { readFile } from 'node:fs/promises'; import { resolve as resolvePath, basename, dirname, isAbsolute } from 'node:path'; import { Args, Command, Flags } from '@oclif/core'; +import { PackageSchema } from '@objectstack/spec/cloud'; import { printHeader, printKV, printSuccess, printError, printStep } from '../../utils/format.js'; import { DEFAULT_CLOUD_URL, tryReadCloudConfig } from '../../utils/cloud-config.js'; import { readErrorMessage } from '../../utils/response-envelope.js'; -const MANIFEST_ID_RE = /^[a-z0-9][a-z0-9._-]{0,254}$/i; +/** + * The one rule for a manifest id is `PackageSchema.manifestId` — the schema for + * the very column this command publishes into (`sys_package.manifest_id`). + * Imported, never transcribed: a second, hand-copied rule here is what let this + * preflight admit six shapes the control plane refuses (`com.acme.repair_desk`, + * `COM.ACME.CRM`, `9foo.bar`, `com..acme`, `com.acme.` and — on the explicit + * `--manifest-id` path — a bare `crm`), while telling the user the contract was + * `a-z0-9._-`. `CreatePackageRequestSchema` in `cloud/package.zod.ts` reaches + * for the same declaration the same way. + * + * Read through a function rather than a module-level constant so `PackageSchema` + * stays lazy (`lazySchema`): the first `.shape` access materialises the whole + * package schema, and `os` boots for many commands that never publish. + */ +function manifestIdSchema() { + return PackageSchema.shape.manifestId; +} + +/** True when `value` is a manifest id the control plane will accept. */ +export function isManifestId(value: string): boolean { + return manifestIdSchema().safeParse(value).success; +} + +/** + * Describe why `value` is not a manifest id, **quoting the schema** — its own + * `invalid_format` issue for the rule and its `.describe()` for the shape. + * + * Deliberately not hand-written: the hand-written sentence this replaces + * (`Expected reverse-domain form like 'com.acme.crm' (a-z0-9._-)`) named a + * contract that does not exist, so a user stopped by the CLI would "fix" their + * id into something like `com.acme.repair_desk` — accepted here, refused by the + * server. Following the error message led to a second error. + * + * Returns `undefined` when `value` is valid. + */ +function explainManifestId(value: string): string | undefined { + const schema = manifestIdSchema(); + const parsed = schema.safeParse(value); + if (parsed.success) return undefined; + const reasons = parsed.error.issues.map((issue) => issue.message).join('; '); + const expected = schema.description; + return expected ? `${reasons}. Expected: ${expected}` : reasons; +} /** * Mirror of `manifest.namespace`'s pattern in `@objectstack/spec` @@ -52,23 +95,68 @@ function slugify(input: string): string { .slice(0, 64) || 'app'; } +/** Where a derived manifest id came from — used to make the refusal actionable. */ +export type ManifestIdSource = 'artifact-manifest-id' | 'artifact-manifest-name' | 'artifact-filename'; + +export interface DerivedManifestId { + /** The id the publish would use. NOT guaranteed valid — the caller parses it. */ + id: string; + /** Which input produced it. */ + source: ManifestIdSource; +} + /** * Derive a reverse-domain manifest_id when the user hasn't passed --manifest-id. * Order of precedence: - * 1. artifact.manifest.id (if it looks like a reverse-domain id) + * 1. artifact.manifest.id (only when it is a manifest id the control plane accepts) * 2. local. * 3. local. + * + * Step 1 is gated by the schema, not by a local look-alike test: `manifest.id` + * is a bare `z.string()` in `ManifestSchema`, so an artifact may carry any + * shape at all, and the previous test forwarded `com.acme.repair_desk` and + * friends unchanged. The old extra `explicit.includes('.')` condition is gone + * because the schema subsumes it — its pattern requires at least two segments, + * so a dotless id can never parse. That is why a bare `crm` was already blocked + * here while the explicit `--manifest-id` path let it through: two paths, two + * strictnesses, neither of them the declared one. + * + * Steps 2 and 3 are the CLI's own invention and are **not** guaranteed valid: + * `slugify` has no letter-first rule, so a manifest named `2024 App` derives + * `local.2024-app`, which the schema rejects. That is refused at the single + * gate in `run()` with the source named, rather than normalised: `manifestId` + * is immutable once published ("renaming a package requires creating a new + * package"), so silently minting a different permanent global identifier than + * the one the inputs imply is worse than saying what is wrong. */ -function deriveManifestId(artifact: any, artifactPath: string): string { +export function deriveManifestId(artifact: any, artifactPath: string): DerivedManifestId { const explicit = artifact?.manifest?.id; - if (typeof explicit === 'string' && MANIFEST_ID_RE.test(explicit) && explicit.includes('.')) { - return explicit; + if (typeof explicit === 'string' && isManifestId(explicit)) { + return { id: explicit, source: 'artifact-manifest-id' }; } const name = artifact?.manifest?.name; if (typeof name === 'string' && name.trim()) { - return `local.${slugify(name)}`; + return { id: `local.${slugify(name)}`, source: 'artifact-manifest-name' }; + } + return { + id: `local.${slugify(basename(artifactPath).replace(/\.json$/i, ''))}`, + source: 'artifact-filename', + }; +} + +/** The remedy line for a refused manifest id, by where the id came from. */ +function manifestIdRemedy(source: ManifestIdSource | 'explicit'): string { + switch (source) { + case 'artifact-manifest-id': + case 'artifact-manifest-name': + return 'It was derived from the compiled artifact. Pass --manifest-id, set `manifestId` in ' + + 'objectstack.manifest.json, or fix `manifest.id` in objectstack.config.ts and rebuild.'; + case 'artifact-filename': + return 'It was derived from the artifact filename. Pass --manifest-id, set `manifestId` in ' + + 'objectstack.manifest.json, or give the app a `manifest.name` and rebuild.'; + default: + return 'Pass a --manifest-id the control plane accepts.'; } - return `local.${slugify(basename(artifactPath).replace(/\.json$/i, ''))}`; } /** @@ -322,14 +410,21 @@ export default class PackagePublish extends Command { const m = tplManifest?.data ?? {}; const baseDir = tplManifest?.baseDir ?? process.cwd(); - const manifestId = ( + // One gate for both paths — the explicitly supplied id and the derived one + // are parsed by the same schema the control plane parses `manifest_id` + // with. Keeping a looser local copy "to fail early" is what this command + // used to do, and it did not fail early: it PASSED early, and the server + // answered 400. + const supplied = flags['manifest-id'] - ?? (typeof m.manifestId === 'string' ? m.manifestId : undefined) - ?? deriveManifestId(artifact, artifactPath) - ).trim(); - if (!MANIFEST_ID_RE.test(manifestId)) { + ?? (typeof m.manifestId === 'string' ? m.manifestId : undefined); + const derived = supplied === undefined ? deriveManifestId(artifact, artifactPath) : undefined; + const manifestId = (supplied ?? derived!.id).trim(); + const manifestIdProblem = explainManifestId(manifestId); + if (manifestIdProblem !== undefined) { printError( - `Invalid manifest-id '${manifestId}'. Expected reverse-domain form like 'com.acme.crm' (a-z0-9._-).`, + `Invalid manifest-id '${manifestId}'. ${manifestIdProblem}. ` + + manifestIdRemedy(derived?.source ?? 'explicit'), ); this.exit(1); return; diff --git a/packages/cli/test/package-publish-manifest-id.test.ts b/packages/cli/test/package-publish-manifest-id.test.ts new file mode 100644 index 0000000000..715a673b37 --- /dev/null +++ b/packages/cli/test/package-publish-manifest-id.test.ts @@ -0,0 +1,355 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `os package publish` decides what a manifest id is by PARSING THROUGH the + * declaration, not by testing a hand-copied look-alike. + * + * Before this suite the command carried its own + * `MANIFEST_ID_RE = /^[a-z0-9][a-z0-9._-]{0,254}$/i` (spelled here as text, in + * `RETIRED_LOCAL_RULE`, so the anti-transcription pin below has a positive + * control to fire on). It was looser than `PackageSchema.manifestId` on every + * axis, so the local preflight ADMITTED what the control plane refuses. + * + * ## The two paths, and why they must be asserted separately + * + * There are two ways an id reaches the wire, and before the fix they had two + * different strictnesses — neither of them the declared one: + * + * explicit `--manifest-id X` (or `manifestId` in objectstack.manifest.json) + * -> tested against the local rule only. + * derive `deriveManifestId()` adopts `artifact.manifest.id` + * -> tested against the local rule AND `explicit.includes('.')`. + * + * That extra dot condition is why a bare `crm` was already blocked on the + * derive path while the explicit path let it through: five of the six shapes + * below held on both paths, `crm` on only one. After the fix both paths ask the + * same schema, and the dot condition is gone because the schema subsumes it + * (its pattern needs at least two segments). + * + * ## What separates a fix from a re-transcription + * + * Re-typing the schema's regex into this file would turn every refusal + * assertion below green while reproducing the defect exactly. Two things rule + * that out and neither is optional: the source pin (no second rule may live in + * the command) and the negative control (a legal id still publishes, bytes + * unchanged). + */ + +import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest'; +import { readFileSync } from 'node:fs'; +import { mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { dirname, join, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { PackageSchema } from '@objectstack/spec/cloud'; +import PackagePublish, { deriveManifestId, isManifestId } from '../src/commands/package/publish.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const PUBLISH_SRC = resolve(HERE, '../src/commands/package/publish.ts'); + +/** The declaration both paths must now answer to. */ +const MANIFEST_ID = PackageSchema.shape.manifestId; + +/** + * The six shapes the retired local rule admitted, with the pre-fix reading of + * WHICH path admitted each — triage's correction to the card, kept here because + * it is the reason every case below is asserted per path rather than once. + */ +const RELAXATIONS: ReadonlyArray<{ id: string; why: string; admittedOnDerivePathBefore: boolean }> = [ + { id: 'crm', why: 'single segment', admittedOnDerivePathBefore: false }, + { id: 'com.acme.repair_desk', why: 'underscore', admittedOnDerivePathBefore: true }, + { id: 'COM.ACME.CRM', why: 'upper case', admittedOnDerivePathBefore: true }, + { id: '9foo.bar', why: 'digit-first segment', admittedOnDerivePathBefore: true }, + { id: 'com..acme', why: 'empty segment', admittedOnDerivePathBefore: true }, + { id: 'com.acme.', why: 'trailing dot', admittedOnDerivePathBefore: true }, +]; + +/** The negative control: a legal reverse-domain id. */ +const LEGAL_ID = 'com.acme.crm'; + +type Call = { url: string; body: any }; + +function artifactJson(manifest: Record): string { + return JSON.stringify({ manifest, objects: [] }); +} + +/** Stub `fetch` so both publish POSTs succeed, and record what was sent. */ +function stubCloud(): Call[] { + const calls: Call[] = []; + vi.stubGlobal('fetch', vi.fn(async (url: string, init: any) => { + calls.push({ url, body: JSON.parse(init.body) }); + const data = url.endsWith('/versions') + ? { id: 'ver_1', version: '1.2.0', listing_status: 'draft' } + : { id: 'pkg_1', created: true, visibility: 'org' }; + return { ok: true, status: 200, statusText: 'OK', json: async () => ({ success: true, data }) } as any; + })); + return calls; +} + +describe('os package publish — the manifest-id rule is the spec manifest-id rule', () => { + let dir = ''; + const prevEnv = { + url: process.env.OS_CLOUD_URL, + key: process.env.OS_CLOUD_API_KEY, + id: process.env.OS_PACKAGE_MANIFEST_ID, + }; + const prevCwd = process.cwd(); + + beforeEach(() => { + delete process.env.OS_PACKAGE_MANIFEST_ID; + }); + + afterEach(async () => { + process.chdir(prevCwd); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + process.env.OS_CLOUD_URL = prevEnv.url; + process.env.OS_CLOUD_API_KEY = prevEnv.key; + if (prevEnv.id === undefined) delete process.env.OS_PACKAGE_MANIFEST_ID; + else process.env.OS_PACKAGE_MANIFEST_ID = prevEnv.id; + if (dir) await rm(dir, { recursive: true, force: true }); + dir = ''; + }); + + /** Write an artifact carrying `manifest` and chdir next to it. */ + async function artifactAt(manifest: Record): Promise { + dir = await mkdtemp(join(tmpdir(), 'package-publish-mid-')); + const path = join(dir, 'objectstack.json'); + await writeFile(path, artifactJson(manifest)); + process.chdir(dir); + process.env.OS_CLOUD_URL = 'http://cloud.test'; + process.env.OS_CLOUD_API_KEY = 'tok_123'; + return path; + } + + /** Run the command, capturing its printed output and its exit code. */ + async function runPublish(argv: string[]): Promise<{ exitCode?: number; output: string }> { + const output: string[] = []; + const sink = (...args: unknown[]) => { output.push(args.map(String).join(' ')); }; + vi.spyOn(console, 'error').mockImplementation(sink); + vi.spyOn(console, 'log').mockImplementation(sink); + let exitCode: number | undefined; + try { + await PackagePublish.run(argv); + } catch (err: any) { + exitCode = err?.oclif?.exit ?? err?.exitCode; + } + return { exitCode, output: output.join('\n') }; + } + + // ------------------------------------------------------------------------- + // The rule itself + // ------------------------------------------------------------------------- + + it('agrees with PackageSchema.manifestId on every case', () => { + const cases: ReadonlyArray = [ + [LEGAL_ID, true], + ['local.acme-crm', true], + ['a.b', true], + ['com.acme.crm-2', true], + ...RELAXATIONS.map(({ id }) => [id, false] as const), + ['', false], + ['com.acme.crm ', false], + ['-com.acme', false], + ]; + const disagreements = cases.filter(([value, expected]) => { + const cli = isManifestId(value); + const spec = MANIFEST_ID.safeParse(value).success; + return cli !== expected || spec !== expected; + }); + expect(disagreements).toEqual([]); + }); + + // ------------------------------------------------------------------------- + // Path 1 — the explicit `--manifest-id` check + // ------------------------------------------------------------------------- + + describe('explicit --manifest-id path', () => { + for (const { id, why } of RELAXATIONS) { + it(`refuses '${id}' (${why}) before any network call`, async () => { + const path = await artifactAt({ id: LEGAL_ID, name: 'Acme CRM', version: '1.2.0' }); + const calls = stubCloud(); + + const { exitCode, output } = await runPublish([path, '--manifest-id', id]); + + expect(exitCode).toBe(1); + expect(calls).toEqual([]); + expect(output).toContain(`Invalid manifest-id '${id}'`); + }); + } + + // Negative control. An implementation that merely re-transcribed the + // schema's regex would pass every refusal above; this is half of what + // separates the two (the source pin is the other half). + it(`still publishes the legal id '${LEGAL_ID}', bytes unchanged`, async () => { + const path = await artifactAt({ id: 'com.other.thing', name: 'Acme CRM', version: '1.2.0' }); + const calls = stubCloud(); + + await PackagePublish.run([path, '--manifest-id', LEGAL_ID]); + + expect(calls).toHaveLength(2); + expect(calls[0].url).toBe('http://cloud.test/api/v1/cloud/packages'); + expect(calls[0].body.manifest_id).toBe(LEGAL_ID); + }); + }); + + // ------------------------------------------------------------------------- + // Path 2 — the derive path (`artifact.manifest.id`) + // ------------------------------------------------------------------------- + + describe('derive path', () => { + for (const { id, why, admittedOnDerivePathBefore } of RELAXATIONS) { + it(`refuses to adopt '${id}' (${why}${admittedOnDerivePathBefore ? '' : ' — already blocked before the fix'})`, async () => { + // Unit half: the deriver does not forward the illegal shape… + const derived = deriveManifestId( + { manifest: { id, name: 'Acme CRM' } }, + '/nowhere/objectstack.json', + ); + expect(derived.id).not.toBe(id); + expect(derived).toEqual({ id: 'local.acme-crm', source: 'artifact-manifest-name' }); + expect(isManifestId(derived.id)).toBe(true); + + // …and end to end, nothing resembling it reaches the wire. + const path = await artifactAt({ id, name: 'Acme CRM', version: '1.2.0' }); + const calls = stubCloud(); + await PackagePublish.run([path]); + + expect(calls).toHaveLength(2); + expect(calls[0].body.manifest_id).not.toBe(id); + expect(MANIFEST_ID.safeParse(calls[0].body.manifest_id).success).toBe(true); + }); + } + + // Negative control on this path too. + it(`adopts the legal id '${LEGAL_ID}' unchanged`, async () => { + expect(deriveManifestId({ manifest: { id: LEGAL_ID, name: 'Acme CRM' } }, '/nowhere/objectstack.json')) + .toEqual({ id: LEGAL_ID, source: 'artifact-manifest-id' }); + + const path = await artifactAt({ id: LEGAL_ID, name: 'Acme CRM', version: '1.2.0' }); + const calls = stubCloud(); + await PackagePublish.run([path]); + + expect(calls).toHaveLength(2); + expect(calls[0].body.manifest_id).toBe(LEGAL_ID); + }); + + it('falls back to the artifact filename when the artifact names nothing usable', () => { + expect(deriveManifestId({ manifest: { id: 'com..acme' } }, '/tmp/build/objectstack.json')) + .toEqual({ id: 'local.objectstack', source: 'artifact-filename' }); + }); + }); + + // ------------------------------------------------------------------------- + // The producer half — the CLI also MAKES ids, and slugify has no + // letter-first rule (the filer's addendum). + // ------------------------------------------------------------------------- + + describe('a derived id the schema rejects is refused, not published and not rewritten', () => { + it("refuses the digit-first id derived from a manifest named '2024 App'", async () => { + const derived = deriveManifestId({ manifest: { name: '2024 App' } }, '/nowhere/objectstack.json'); + expect(derived).toEqual({ id: 'local.2024-app', source: 'artifact-manifest-name' }); + expect(isManifestId(derived.id)).toBe(false); + + const path = await artifactAt({ name: '2024 App', version: '1.2.0' }); + const calls = stubCloud(); + const { exitCode, output } = await runPublish([path]); + + expect(exitCode).toBe(1); + expect(calls).toEqual([]); + expect(output).toContain("Invalid manifest-id 'local.2024-app'"); + // The refusal says where the id came from and how to set one, because + // the user never typed this string. + expect(output).toContain('derived from the compiled artifact'); + expect(output).toContain('--manifest-id'); + // It is NOT normalised into some other permanent identifier: manifestId + // is immutable once published. + expect(output).not.toContain('local.a2024-app'); + }); + + it('publishes when the same derivation lands on a legal id', async () => { + const path = await artifactAt({ name: 'Acme CRM', version: '1.2.0' }); + const calls = stubCloud(); + + await PackagePublish.run([path]); + + expect(calls).toHaveLength(2); + expect(calls[0].body.manifest_id).toBe('local.acme-crm'); + }); + }); + + // ------------------------------------------------------------------------- + // The error text + // ------------------------------------------------------------------------- + + describe('the refusal text is quoted from the schema, not written a second time', () => { + it("reports the schema's own invalid_format issue and its description", async () => { + const rejected = MANIFEST_ID.safeParse('com.acme.repair_desk'); + expect(rejected.success).toBe(false); + const issue = rejected.success === false ? rejected.error.issues[0] : undefined; + expect(issue?.code).toBe('invalid_format'); + + const path = await artifactAt({ id: LEGAL_ID, name: 'Acme CRM', version: '1.2.0' }); + stubCloud(); + const { output } = await runPublish([path, '--manifest-id', 'com.acme.repair_desk']); + + expect(output).toContain(issue!.message); + expect(output).toContain(MANIFEST_ID.description!); + }); + + it('no longer states the contract the CLI invented', async () => { + const path = await artifactAt({ id: LEGAL_ID, name: 'Acme CRM', version: '1.2.0' }); + stubCloud(); + const { output } = await runPublish([path, '--manifest-id', 'com.acme.repair_desk']); + + // The retired sentence sent a stopped user to `com.acme.repair_desk` — + // accepted locally, refused by the server. Following the error message + // led to a second error. + expect(output).not.toContain('a-z0-9._-'); + }); + }); +}); + +// --------------------------------------------------------------------------- +// The source pin — no second copy of the rule may live in the command +// --------------------------------------------------------------------------- + +/** The rule this card deleted, as text so the scanner has a positive control. */ +const RETIRED_LOCAL_RULE = 'const MANIFEST_ID_RE = /^[a-z0-9][a-z0-9._-]{0,254}$/i;'; + +/** + * Every fully anchored regex literal in `code` that matches a legal manifest + * id. Any local rule for manifest ids must match `com.acme.crm` — that is what + * makes it a rule about manifest ids — so this finds a transcription without + * having to guess which transcription was written. + */ +function manifestIdShapedLiterals(code: string): string[] { + const literals = code.match(/\/\^(?:[^/\\\n]|\\.)*\$\/[a-z]*/g) ?? []; + return literals.filter((literal) => { + const lastSlash = literal.lastIndexOf('/'); + const source = literal.slice(1, lastSlash); + const flags = literal.slice(lastSlash + 1).replace(/[gy]/g, ''); + try { + return new RegExp(source, flags).test(LEGAL_ID); + } catch { + return false; + } + }); +} + +describe('publish.ts keeps no local copy of the manifest-id rule', () => { + it('scans for transcriptions (control: the retired rule is found)', () => { + expect(manifestIdShapedLiterals(RETIRED_LOCAL_RULE)).toHaveLength(1); + // …and on a transcription of the schema's own pattern, the shape a + // "fix" that re-types the regex would take. + expect(manifestIdShapedLiterals('const X = /^[a-z][a-z0-9-]*(\\.[a-z][a-z0-9-]*)+$/;')).toHaveLength(1); + }); + + it('finds none in the command source', () => { + const source = readFileSync(PUBLISH_SRC, 'utf8'); + expect(manifestIdShapedLiterals(source)).toEqual([]); + expect(source).not.toContain('MANIFEST_ID_RE'); + // …and the declaration is reached by import, which is what makes the + // absence above a fix rather than a deletion. + expect(source).toContain('PackageSchema.shape.manifestId'); + }); +});