diff --git a/.changeset/template-manifest-optional-manifest-id.md b/.changeset/template-manifest-optional-manifest-id.md new file mode 100644 index 0000000000..8551de19e6 --- /dev/null +++ b/.changeset/template-manifest-optional-manifest-id.md @@ -0,0 +1,50 @@ +--- +"@objectstack/spec": minor +--- + +feat(spec): `manifestId` is optional on `TemplateManifestSchema`, and every shipped template manifest is now parsed against it (#7319) + +`TemplateManifestSchema` describes the on-disk `objectstack.manifest.json`, and +the bundled blank template declares it as its `$schema` — but the file did not +satisfy it. The schema inherited `manifestId` from `CreatePackageRequestSchema` +as **required**, and no template has ever declared one. Measured on `main`, that +was the *only* complaint the shipped file produced: + +``` +manifestId: Invalid input: expected string, received undefined (invalid_type) +``` + +Nothing broke, because nothing parsed the file. `create-objectstack` reads and +rewrites it as raw JSON and does not depend on `@objectstack/spec` at all; +`objectstack package publish` also reads it raw, resolving the id as +`--manifest-id ?? manifest.manifestId ?? deriveManifestId(artifact, path)`. That +fallback is the measurement: on this file the id is a declarative **default**, +not a requirement — a template tree that declares none publishes fine, deriving +`local.` from the compiled artifact. + +**The key is now optional on the on-disk descriptor**, declared locally rather +than by loosening the shared base: + +- `TemplateManifestSchema` omits the inherited field and re-declares it + `.optional()`, reusing the publish field's value constraints — the same + omit-then-extend split `namespace` uses (#6861), in the other direction. A + malformed id is still rejected: optional is not unvalidated. +- **`CreatePackageRequestSchema` is untouched.** The publish request that reaches + the control plane still requires `manifestId` — the package row is addressed + by it and it is immutable once set. Widening the base would have made a publish + request with no package identity parse, which is the collapse the local + override exists to avoid. + +Authoring is unchanged in the accepting direction: a manifest that declares a +`manifestId` still parses exactly as before, and every other required key +(`displayName`, `name`, `specVersion`) is still required. + +**New gate — `check:template-manifests`.** Every `objectstack.manifest.json` +under `packages/create-objectstack/src/templates/` is parsed against +`TemplateManifestSchema` on every PR (unfiltered, in the required +`TypeScript Type Check` job). This is the check that would have caught both +drifts this file has now accumulated — #6861's silently stripped `namespace` and +this one — so the `$schema` line those files carry stops being an unverified +claim. It walks the template tree rather than a hand-kept file list, so a +template added later is covered on the day it lands, and it fails rather than +reporting success if it finds nothing to parse. diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index eb3a9b239e..660f2e3630 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -1053,6 +1053,20 @@ jobs: - name: Check the react-blocks contract is in sync with the spec run: pnpm --filter @objectstack/spec check:react-blocks + # The shipped `objectstack.manifest.json` files declare TemplateManifestSchema + # as their `$schema`, and until #7319 nothing read that claim: both consumers + # take the file as raw JSON (create-objectstack does not depend on the spec at + # all), so the schema and the artifact drifted twice without a red build — + # #6861's silently stripped `namespace`, then a required `manifestId` the blank + # template has never carried. One parse per shipped manifest closes the class. + # + # Reads `src/` and the template trees, so it needs no build and belongs in this + # pre-build group with the other source audits. No paths filter and required, + # for the standard reason: a filter on packages/create-objectstack/** would go + # dormant on exactly the PR that tightens the schema instead. + - name: Check every shipped template manifest satisfies TemplateManifestSchema + run: pnpm --filter @objectstack/spec check:template-manifests + # Example apps are AI-authoring reference templates; a red typecheck is a # bad signal to copy from. tsup transpiles them without a full typecheck, # so build alone will not catch type drift — typecheck them explicitly. diff --git a/content/docs/references/cloud/template-manifest.mdx b/content/docs/references/cloud/template-manifest.mdx index 0c07c43f1a..b864f86cc3 100644 --- a/content/docs/references/cloud/template-manifest.mdx +++ b/content/docs/references/cloud/template-manifest.mdx @@ -8,7 +8,14 @@ description: Template Manifest protocol schemas `objectstack.manifest.json` — on-disk descriptor for a template / package source tree. Strict projection of `CreatePackageRequestSchema` (server- managed fields excluded) plus scaffold-time extras (name slug, -specVersion, namespace, skills, preview, scaffold, readmePath). +specVersion, namespace, skills, preview, scaffold, readmePath), with +`manifestId` locally relaxed to OPTIONAL — this file is a source tree, not +a publish request (#7319; the field's own TSDoc carries the measurement). + +Every shipped `objectstack.manifest.json` is parsed against this schema by +`pnpm --filter @objectstack/spec check:template-manifests`, so the +`$schema` line those files carry is a verified claim rather than an +assertion nothing reads. **Source:** `packages/spec/src/cloud/template-manifest.zod.ts` @@ -34,7 +41,6 @@ objectstack.manifest.json — template / package source descriptor | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **manifestId** | `string` | ✅ | Globally unique reverse-domain package identifier (e.g. com.acme.crm) | | **displayName** | `string` | ✅ | Display name shown in Studio and Marketplace | | **description** | `string` | optional | Short package description | | **visibility** | `Enum<'private' \| 'org' \| 'marketplace'>` | optional | Package visibility: private = owner org only; org = all envs in owner org; marketplace = public registry | @@ -46,6 +52,7 @@ objectstack.manifest.json — template / package source descriptor | **publisher** | `Enum<'objectstack' \| 'partner' \| 'community' \| 'private'>` | optional | Package publisher provenance tier | | **isStarter** | `boolean` | optional | | | **translations** | `Record` | optional | Locale-keyed overrides; missing keys fall back to base columns | +| **manifestId** | `string` | optional | Optional declarative default for the published package id (reverse-domain, e.g. com.acme.crm). Absent on a template source tree: `objectstack package publish` falls back to --manifest-id and then to a derived `local.`. NOT optional on the publish request itself | | **name** | `string` | ✅ | CLI slug (kebab-case, no namespace prefix) | | **specVersion** | `string` | ✅ | Compatible @objectstack/spec semver range | | **namespace** | `string` | optional | Scaffold-only: the template’s own metadata namespace, rewritten by create-objectstack at scaffold time and read back as the fallback source for the template’s original namespace. NOT the publish namespace — publish reads that off the compiled artifact’s manifest.namespace (ADR-0048 addendum §A.2) | diff --git a/packages/spec/package.json b/packages/spec/package.json index f0815383ae..8f0035c419 100644 --- a/packages/spec/package.json +++ b/packages/spec/package.json @@ -220,6 +220,7 @@ "check:react-blocks": "tsx scripts/build-react-blocks-contract.ts --check", "check:react-declaration-parity": "tsx scripts/check-react-blocks-declaration-parity.ts", "check:skill-examples": "tsx scripts/check-skill-examples.ts --self-test && tsx scripts/check-skill-examples.ts", + "check:template-manifests": "tsx scripts/check-template-manifests.ts --self-test && tsx scripts/check-template-manifests.ts", "check:test-typecheck": "tsx ../../scripts/check-test-typecheck.mts --self-test && tsx ../../scripts/check-test-typecheck.mts --package packages/spec --project tsconfig.test.json", "gen:test-typecheck-debt": "tsx ../../scripts/check-test-typecheck.mts --update --package packages/spec --project tsconfig.test.json", "check:scripts-typecheck": "tsc --noEmit -p tsconfig.scripts.json", diff --git a/packages/spec/scripts/check-generated.ts b/packages/spec/scripts/check-generated.ts index 218bc8c1de..5ee6175ca3 100644 --- a/packages/spec/scripts/check-generated.ts +++ b/packages/spec/scripts/check-generated.ts @@ -154,6 +154,17 @@ const NO_GENERATOR: ReadonlyArray<{ check: string; why: string }> = [ { check: 'check:liveness', why: 'audits whether declared spec properties have a reader — no artifact' }, { check: 'check:empty-state', why: 'audits empty-state coverage — no artifact' }, { check: 'check:skill-examples', why: 'validates skill examples parse — no artifact' }, + // #7319. Reads `src/` and the shipped template trees and writes nothing: a + // failure is either a manifest to fix or a schema to fix, never a `gen:` to + // run. It audits the inverse direction from everything in GATED — those + // compare an artifact this package GENERATES against its source, this one + // compares a file another package SHIPS against the schema that claims to + // describe it. Two drifts had already accumulated in that blind spot (#6861's + // stripped `namespace`, #7319's required-but-absent `manifestId`). + { + check: 'check:template-manifests', + why: 'parses every shipped objectstack.manifest.json against TemplateManifestSchema — no artifact', + }, // Landed in #4177 while this ledger landed in #4183 — neither PR could see the // other, so `main` carried an unclassified script and this reconciliation was // failing on `main` itself. The doc it checks against is hand-written, so there diff --git a/packages/spec/scripts/check-template-manifests.ts b/packages/spec/scripts/check-template-manifests.ts new file mode 100644 index 0000000000..051def3753 --- /dev/null +++ b/packages/spec/scripts/check-template-manifests.ts @@ -0,0 +1,294 @@ +#!/usr/bin/env tsx +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Every shipped `objectstack.manifest.json` must parse against + * `TemplateManifestSchema` (#7319). + * + * WHY THIS EXISTS. `TemplateManifestSchema` is documented as the schema of the + * on-disk `objectstack.manifest.json`, and the bundled templates declare it as + * their `$schema`. Nothing read that claim. Both consumers of the file take it + * as raw JSON — `create-objectstack` does not depend on `@objectstack/spec` at + * all (deps: chalk, commander, tar), and `objectstack package publish` reads it + * with `JSON.parse` and per-key `typeof` guards — so the schema and the file it + * describes were free to disagree, and did, twice: + * + * #6861 — the schema STRIPPED `namespace`, a key the scaffolder writes, + * rewrites and reads back. Silent: `parse()` succeeded without it. + * #7319 — the schema REQUIRED `manifestId`, which the blank template has + * never declared. Measured on `origin/main` @ 3e8e669c0, the shipped + * file did not satisfy its own `$schema`, and the only complaint was + * `manifestId` — `invalid_type`, `expected string, received undefined`. + * + * Neither was reachable by any gate: the first is invisible by construction (a + * strip is a success), and the second is invisible because no live path parses + * this file at all. One parse per shipped manifest closes the class — the + * declaration and the artifact are compared, in the direction that fails loudly. + * + * WHAT IT PROVES, AND WHAT IT DOES NOT. It proves the files we ship satisfy the + * schema we publish for them. It says nothing about a manifest authored + * elsewhere (a remote template, a user's project): nothing validates those + * either, and #7319 deliberately did not add publish-time validation — the + * question there is a contract change, not a drift. + * + * Usage: + * pnpm --filter @objectstack/spec check:template-manifests + * pnpm --filter @objectstack/spec check:template-manifests --self-test + */ + +import { readdirSync, readFileSync, mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, join, relative } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import { TemplateManifestSchema } from '../src/cloud/template-manifest.zod'; + +const pkgRoot = join(dirname(fileURLToPath(import.meta.url)), '..'); +const repoRoot = join(pkgRoot, '..', '..'); + +/** + * Where the shipped manifests live. A DIRECTORY, walked recursively, never a + * hand-kept list of files: the whole point is that a template added tomorrow is + * covered on the day it lands, without anyone remembering this gate exists. + */ +const TEMPLATE_ROOT = join(repoRoot, 'packages/create-objectstack/src/templates'); + +const MANIFEST_FILENAME = 'objectstack.manifest.json'; + +/** The `$schema` these files claim. Verifying the claim is half the point. */ +const TEMPLATE_MANIFEST_SCHEMA_URL = 'https://schemas.objectstack.dev/template-manifest.json'; + +const rel = (p: string) => relative(repoRoot, p); + +/** Every `objectstack.manifest.json` under `root`, deepest paths included. */ +export function findManifests(root: string): string[] { + const found: string[] = []; + const walk = (dir: string) => { + let entries; + try { + entries = readdirSync(dir, { withFileTypes: true }); + } catch { + return; // missing dir — the caller's zero-file guard reports it + } + for (const e of entries.sort((a, b) => a.name.localeCompare(b.name))) { + const p = join(dir, e.name); + if (e.isDirectory()) { + if (e.name === 'node_modules' || e.name === 'dist') continue; + walk(p); + } else if (e.name === MANIFEST_FILENAME) { + found.push(p); + } + } + }; + walk(root); + return found; +} + +/** + * Parse one manifest. Returns the problems as printable lines — empty means it + * satisfies the schema it advertises. + */ +export function checkManifest(path: string): string[] { + let raw: string; + try { + raw = readFileSync(path, 'utf8'); + } catch (err) { + return [`cannot be read: ${(err as Error).message}`]; + } + + let data: unknown; + try { + data = JSON.parse(raw); + } catch (err) { + return [`is not valid JSON: ${(err as Error).message}`]; + } + + const problems: string[] = []; + + // The gate's authority comes from the FILENAME — every `objectstack.manifest.json` + // under the template root is judged against `TemplateManifestSchema`. So a file + // claiming some other `$schema` is not an exemption, it is a contradiction: one + // of the two statements is wrong and a reader cannot tell which. Absent is fine + // (the claim is optional); present and different is not. + const declared = (data as Record | null)?.$schema; + if (typeof declared === 'string' && declared !== TEMPLATE_MANIFEST_SCHEMA_URL) { + problems.push( + `declares $schema "${declared}", but this gate judges it against TemplateManifestSchema ` + + `("${TEMPLATE_MANIFEST_SCHEMA_URL}"). Reconcile the claim with the schema that governs the file.`, + ); + } + + const result = TemplateManifestSchema.safeParse(data); + if (!result.success) { + for (const issue of result.error.issues) { + const at = issue.path.length ? issue.path.join('.') : ''; + problems.push(`${at}: ${issue.message} (${issue.code})`); + } + } + + return problems; +} + +// ── self-test ─────────────────────────────────────────────────────────────── +// A gate over one file today is a gate that can rot into a no-op unnoticed, so +// its detection is proved against fixtures rather than against the corpus. The +// green case is the #7319 shape (no `manifestId`) and the red cases include the +// #6861 key (`namespace`), so both intents this schema carries are exercised. + +function selfTest(): never { + const failures: string[] = []; + const check = (ok: boolean, what: string) => { + if (!ok) failures.push(what); + }; + + const dir = mkdtempSync(join(tmpdir(), 'os-template-manifests-')); + try { + const write = (name: string, body: unknown) => { + const d = join(dir, name); + mkdirSync(d, { recursive: true }); + const p = join(d, MANIFEST_FILENAME); + writeFileSync(p, typeof body === 'string' ? body : JSON.stringify(body, null, 2), 'utf8'); + return p; + }; + + const base = { + $schema: TEMPLATE_MANIFEST_SCHEMA_URL, + name: 'blank', + namespace: 'blank', + specVersion: '^6.0.0', + displayName: 'Blank Starter', + }; + + // GREEN — the shipped shape: no `manifestId` at all (#7319). + const green = write('green', base); + check( + checkManifest(green).length === 0, + `green fixture: a manifest with no manifestId must PASS — that relaxation is #7319 itself; got ${JSON.stringify(checkManifest(green))}`, + ); + + // GREEN — a manifest that DOES declare a well-formed id. + const withId = write('with-id', { ...base, manifestId: 'com.acme.blank' }); + check( + checkManifest(withId).length === 0, + `with-id fixture: a well-formed reverse-domain manifestId must PASS; got ${JSON.stringify(checkManifest(withId))}`, + ); + + // RED — optional is not unvalidated: a malformed id is still a malformed id. + const badId = write('bad-id', { ...base, manifestId: 'Not A Reverse Domain' }); + check( + checkManifest(badId).some((p) => p.startsWith('manifestId:')), + 'bad-id fixture: a malformed manifestId must FAIL — optional must not mean unchecked', + ); + + // RED — the #6861 key. The gate that would have caught that drift is this one, + // so it has to actually judge `namespace` values. + const badNs = write('bad-namespace', { ...base, namespace: 'CRM-App' }); + check( + checkManifest(badNs).some((p) => p.startsWith('namespace:')), + 'bad-namespace fixture: a malformed namespace must FAIL (#6861 — the key is declared, so its values are judged)', + ); + + // RED — a genuinely required key is still required. + const { displayName: _dropped, ...noDisplayName } = base; + const missing = write('missing-required', noDisplayName); + check( + checkManifest(missing).some((p) => p.startsWith('displayName:')), + 'missing-required fixture: a missing displayName must FAIL — the relaxation is manifestId ONLY', + ); + + // RED — a contradictory `$schema` claim. + const wrongSchema = write('wrong-schema', { ...base, $schema: 'https://example.invalid/other.json' }); + check( + checkManifest(wrongSchema).some((p) => p.includes('declares $schema')), + 'wrong-schema fixture: a manifest claiming a different $schema must FAIL', + ); + + // RED — malformed JSON is a finding, not a skip. + const broken = write('broken', '{ not json'); + check( + checkManifest(broken).some((p) => p.includes('not valid JSON')), + 'broken fixture: malformed JSON must FAIL', + ); + + // Discovery walks recursively and finds exactly the manifests written above. + const discovered = findManifests(dir); + check( + discovered.length === 7, + `discovery: expected 7 fixture manifests, found ${discovered.length}`, + ); + + // The vacuous-green guard's input: an empty tree yields zero, which `main` + // turns into a failure rather than a silent pass. + const emptyDir = mkdtempSync(join(tmpdir(), 'os-template-manifests-empty-')); + try { + check(findManifests(emptyDir).length === 0, 'discovery: an empty tree must yield zero manifests'); + } finally { + rmSync(emptyDir, { recursive: true, force: true }); + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + + if (failures.length) { + for (const f of failures) console.error(`✗ self-test: ${f}`); + console.error(`\ncheck-template-manifests --self-test: ${failures.length} failure(s).\n`); + process.exit(1); + } + console.log( + '✅ self-test: accepts a manifest with no manifestId (#7319) and one with a well-formed id,\n' + + ' rejects a malformed manifestId / namespace (#6861), a missing required key, a\n' + + ' contradictory $schema and malformed JSON; discovery walks the template tree.', + ); + process.exit(0); +} + +function main(): void { + if (process.argv.includes('--self-test')) selfTest(); + + console.log(`🧪 Parsing every shipped ${MANIFEST_FILENAME} against TemplateManifestSchema...\n`); + + const manifests = findManifests(TEMPLATE_ROOT); + + // Vacuous-green guard. Zero files is far more likely to mean "the templates + // moved" than "we ship no templates", and a gate that checked nothing must not + // report success. + if (manifests.length === 0) { + console.error( + `✗ No ${MANIFEST_FILENAME} found under ${rel(TEMPLATE_ROOT)}.\n\n` + + ` Every bundled template ships one, so this is almost certainly a moved\n` + + ` directory rather than an empty one — point TEMPLATE_ROOT at wherever the\n` + + ` templates live now. A gate that parses nothing must not report success.\n`, + ); + process.exit(1); + } + + let bad = 0; + for (const path of manifests) { + const problems = checkManifest(path); + console.log(` ${problems.length === 0 ? '✓' : '✗'} ${rel(path)}`); + for (const p of problems) console.log(` ${p}`); + if (problems.length) bad++; + } + + if (bad === 0) { + console.log( + `\n✅ ${manifests.length} shipped manifest(s) satisfy TemplateManifestSchema — ` + + `their $schema line is a verified claim.`, + ); + return; + } + + console.error( + `\n✗ ${bad} of ${manifests.length} shipped manifest(s) do not satisfy the schema they declare.\n\n` + + ` Two fixes, and the right one depends on which side is wrong:\n` + + ` • the FILE is wrong — it omits a key the schema requires, or carries a bad\n` + + ` value. Fix the manifest.\n` + + ` • the SCHEMA is wrong — it demands something no template can supply, the way\n` + + ` it required a publish-time \`manifestId\` from a source tree (#7319). Fix\n` + + ` packages/spec/src/cloud/template-manifest.zod.ts, and keep the relaxation\n` + + ` LOCAL: CreatePackageRequestSchema is the publish request and must stay strict.\n`, + ); + process.exit(1); +} + +main(); diff --git a/packages/spec/src/cloud/template-manifest-id.test.ts b/packages/spec/src/cloud/template-manifest-id.test.ts new file mode 100644 index 0000000000..d66e76bd63 --- /dev/null +++ b/packages/spec/src/cloud/template-manifest-id.test.ts @@ -0,0 +1,112 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `manifestId` is OPTIONAL on the on-disk descriptor and REQUIRED on the + * publish request (#7319). + * + * `TemplateManifestSchema` is a projection of `CreatePackageRequestSchema`, and + * it inherited `manifestId` as required — so the bundled blank template did not + * satisfy the `$schema` it declares, with `manifestId` the only complaint + * (measured on `origin/main` @ 3e8e669c0). Nothing broke, because nothing parses + * the file: `objectstack package publish` reads it as raw JSON and resolves + * `--manifest-id ?? m.manifestId ?? deriveManifestId(...)`, which is also the + * measurement that the key is genuinely optional on this side. + * + * The fix is a LOCAL relaxation, and these pins hold both halves of it. Widening + * the shared base instead would have made the publish request accept a package + * with no identity — the same "one schema, two meanings" collapse the sibling + * `namespace` split (#6861, `package-namespace.test.ts`) exists to prevent, in + * the direction that costs more. + */ + +import { describe, it, expect } from 'vitest'; +import { CreatePackageRequestSchema } from './package.zod'; +import { TemplateManifestSchema } from './template-manifest.zod'; + +/** A template manifest that is valid except for whatever a case changes. */ +function templateManifest(overrides: Record = {}) { + return { + name: 'blank', + specVersion: '^6.0.0', + displayName: 'Blank Starter', + ...overrides, + }; +} + +/** A CreatePackageRequest that is valid except for the case under test. */ +function createRequest(overrides: Record = {}) { + return { + manifestId: 'com.acme.crm', + ownerOrgId: 'org_acme', + displayName: 'Acme CRM', + createdBy: 'usr_1', + ...overrides, + }; +} + +describe('TemplateManifestSchema relaxes manifestId to optional (#7319)', () => { + it('parses a manifest that declares no manifestId — the shipped shape', () => { + const result = TemplateManifestSchema.safeParse(templateManifest()); + expect(result.success).toBe(true); + expect(result.success && result.data.manifestId).toBeUndefined(); + }); + + it('still DECLARES the key — relaxed, not stripped', () => { + // A projection that dropped the field entirely would silently discard an id a + // template author did write, which is #6861's failure wearing the other hat. + expect(Object.keys(TemplateManifestSchema.shape)).toContain('manifestId'); + const parsed = TemplateManifestSchema.parse(templateManifest({ manifestId: 'com.acme.blank' })); + expect(parsed.manifestId).toBe('com.acme.blank'); + }); + + it('still JUDGES the value — optional is not unvalidated', () => { + const result = TemplateManifestSchema.safeParse(templateManifest({ manifestId: 'Not A Reverse Domain' })); + expect(result.success).toBe(false); + const issues = result.success ? [] : result.error.issues; + expect(issues).toHaveLength(1); + expect(issues[0].path).toEqual(['manifestId']); + expect(issues[0].code).toBe('invalid_format'); + }); + + it('relaxes manifestId ONLY — the other required keys are untouched', () => { + const result = TemplateManifestSchema.safeParse({ name: 'blank', specVersion: '^6.0.0' }); + expect(result.success).toBe(false); + const paths = result.success ? [] : result.error.issues.map((i) => i.path.join('.')); + expect(paths).toContain('displayName'); + expect(paths).not.toContain('manifestId'); + }); +}); + +describe('CreatePackageRequestSchema keeps manifestId REQUIRED (#7319)', () => { + it('rejects a publish request with no manifestId', () => { + const { manifestId: _dropped, ...withoutId } = createRequest(); + const result = CreatePackageRequestSchema.safeParse(withoutId); + expect(result.success).toBe(false); + const issues = result.success ? [] : result.error.issues; + expect(issues).toHaveLength(1); + expect(issues[0].path).toEqual(['manifestId']); + expect(issues[0].code).toBe('invalid_type'); + }); + + it('accepts one that carries it', () => { + expect(CreatePackageRequestSchema.safeParse(createRequest()).success).toBe(true); + }); + + it('is a DIFFERENT field instance from the template one, and keeps its own describe', () => { + // Two instances carrying two obligations. Collapsing them — by relaxing the + // base instead of overriding locally — is the regression these pins exist for. + const templateField = TemplateManifestSchema.shape.manifestId; + const publishField = CreatePackageRequestSchema.shape.manifestId; + expect(templateField).not.toBe(publishField); + + expect(publishField.safeParse(undefined).success).toBe(false); + expect(templateField.safeParse(undefined).success).toBe(true); + + expect(publishField.description).toBe( + 'Globally unique reverse-domain package identifier (e.g. com.acme.crm)', + ); + const templateDoc = templateField.description ?? ''; + expect(templateDoc).toMatch(/optional/i); + expect(templateDoc).toMatch(/NOT optional on the publish request/i); + }); +}); diff --git a/packages/spec/src/cloud/template-manifest.zod.ts b/packages/spec/src/cloud/template-manifest.zod.ts index e9d5cee92f..ce1baf3617 100644 --- a/packages/spec/src/cloud/template-manifest.zod.ts +++ b/packages/spec/src/cloud/template-manifest.zod.ts @@ -4,7 +4,14 @@ * `objectstack.manifest.json` — on-disk descriptor for a template / package * source tree. Strict projection of `CreatePackageRequestSchema` (server- * managed fields excluded) plus scaffold-time extras (name slug, - * specVersion, namespace, skills, preview, scaffold, readmePath). + * specVersion, namespace, skills, preview, scaffold, readmePath), with + * `manifestId` locally relaxed to OPTIONAL — this file is a source tree, not + * a publish request (#7319; the field's own TSDoc carries the measurement). + * + * Every shipped `objectstack.manifest.json` is parsed against this schema by + * `pnpm --filter @objectstack/spec check:template-manifests`, so the + * `$schema` line those files carry is a verified claim rather than an + * assertion nothing reads. */ import { z } from 'zod'; @@ -35,8 +42,43 @@ export const TemplateManifestSchema = lazySchema(() => // // Value constraints are reused from the publish field (one vocabulary, // §A.7); only the meaning differs, and the describe says so. - .omit({ ownerOrgId: true, createdBy: true, namespace: true }) + // + // `manifestId` is omitted and re-declared for the same reason in a + // different direction (#7319): the create-request field is REQUIRED and + // must stay so — a publish request with no package identity is not a + // request — while on this file the id is a declarative DEFAULT the + // publisher may or may not have written down. The relaxation is therefore + // stated HERE, locally, and `CreatePackageRequestSchema` is untouched; + // widening the shared base would have loosened the publish surface too. + .omit({ ownerOrgId: true, createdBy: true, namespace: true, manifestId: true }) .extend({ + /** + * Reverse-domain package id this source tree publishes as — a + * declarative default, NOT a requirement of the file. + * + * OPTIONAL here, required on `CreatePackageRequestSchema`. The split is + * measured, not stylistic: `objectstack package publish` reads this file + * as raw JSON and resolves the id as + * `--manifest-id ?? m.manifestId ?? deriveManifestId(artifact, path)` + * (`packages/cli/src/commands/package/publish.ts`), so a template tree + * that declares none publishes perfectly well — the fallback derives + * `local.` from the compiled artifact. The bundled blank template + * has shipped without the key since it was written, and the id it would + * carry is per-project anyway: `create-objectstack` stamps the identity + * at scaffold time, so a template-level literal would name a package + * nobody publishes (#7319 rejected exactly that fix). + * + * Required stays required where it means something. The publish request + * that reaches the control plane still carries a mandatory + * `manifestId` — the package row is addressed by it and it is immutable + * once set — and this local override does not reach that schema. + * + * Value constraints are reused from the publish field, as with + * `namespace` above: one vocabulary, only the obligation differs. + */ + manifestId: CreatePackageRequestSchema.shape.manifestId.optional().describe( + 'Optional declarative default for the published package id (reverse-domain, e.g. com.acme.crm). Absent on a template source tree: `objectstack package publish` falls back to --manifest-id and then to a derived `local.`. NOT optional on the publish request itself' + ), name: z.string().regex(/^[a-z][a-z0-9-]*$/) .describe('CLI slug (kebab-case, no namespace prefix)'), specVersion: z.string().describe('Compatible @objectstack/spec semver range'),