fix(mcp): make i18n:false delete and meta cleanup safe

.changeset/i18n-false-meta-safety.md

@@ -0,0 +1,30 @@
+---
+"@contentrain/mcp": patch
+---
+
+fix(mcp): make i18n:false delete and meta cleanup safe
+
+Two bugs a project hit while cleaning up an `i18n: false` collection, plus a
+source-hygiene fix surfaced along the way.
+
+**`content_delete` no longer destroys content when handed a locale.** On an
+`i18n: false` model, passing a non-default `locale` was destructive: the locale
+mapped onto `data.json` and the default-locale meta, so the call emptied the
+shared content and deleted the wrong meta file while the locale actually named
+kept its stray meta — the opposite of the request. Content is locale-agnostic
+here, so a locale-scoped delete is now rejected with a clear error (both in the
+plan API and the legacy path). Omit `locale` to delete the entry.
+
+**`contentrain_validate` with `fix: true` now clears the meta layout mismatch it
+warns about.** The "Meta layout mismatch" warning had no remediation, so `fixed`
+stayed `0`. The fix is deterministic and never decides a status: when the
+default-locale meta is authoritative the redundant strays are pruned; when only
+a stray exists it is migrated to the default path so the record is preserved;
+several strays with no default is left for the author to resolve. Consolidation
+runs before the orphan-content pass and gates that pass's draft fabrication, so
+a real published record is never replaced by a fabricated draft and then deleted
+on a later run.
+
+Also replaced two raw NUL bytes in the validator source (a Map-key separator)
+with a `\u0000` escape — identical at runtime, but the source is now plain text
+instead of being classified as binary by grep/diff/editors.

docs/packages/mcp.md

@@ -174,9 +174,19 @@ delivery: a collection entry is served only when its status is `published`.
 
 ::: tip Non-i18n models
 A model with `i18n: false` keeps all content in one `data.json`, so it has
-exactly one meta record — at the **default locale**, never `data.json`. Saving
-under a different locale does not move it. `contentrain_validate` warns when
-stray per-locale meta files exist from older versions.
+exactly one meta record — at the **default locale**, never `data.json`.
+
+- **A locale is meaningless here, so it is rejected, not guessed.**
+  `contentrain_content_delete` refuses a `locale`-scoped delete on an
+  `i18n: false` model: the locale would otherwise map onto `data.json` and the
+  default-locale meta, deleting the shared content and the wrong meta while a
+  stray per-locale meta stayed behind. Omit `locale` to delete the entry.
+- **`contentrain_validate` warns about stray per-locale meta** left by older
+  versions, and **`fix: true` cleans it up deterministically**: when the
+  default-locale meta is authoritative it prunes the extras (no status is
+  merged, so a `published` record is never downgraded); when only a stray
+  exists it is migrated to the default path so the record is preserved. Several
+  strays with no default is ambiguous — it is left for you to resolve by hand.
 :::
 
 ### Media Tools (Provider Media Facet)

packages/mcp/src/core/content-manager.ts

@@ -241,6 +241,19 @@ export async function deleteContent(
   model: ModelDefinition,
   opts: DeleteOpts,
 ): Promise<string[]> {
+  // A non-i18n model stores content locale-agnostically in data.json with a
+  // single meta record at the default locale, so a locale-scoped delete is
+  // meaningless and destructive (the locale maps onto data.json + the default
+  // meta, deleting the shared content and the wrong meta). Reject it; mirrors
+  // the same guard in the plan API (`planContentDelete`).
+  if (!model.i18n && opts.locale) {
+    throw new Error(
+      `Model "${model.id}" has i18n disabled — its content is locale-agnostic (stored in data.json), `
+      + 'so a locale-scoped delete is invalid. Omit "locale" to delete the entry, '
+      + 'or run contentrain_validate fix:true to remove stray per-locale meta files.',
+    )
+  }
+
   const removed: string[] = []
   const cDir = resolveContentDir(projectRoot, model)
 

packages/mcp/src/core/ops/content-delete.ts

@@ -65,6 +65,22 @@ export async function planContentDelete(
   input: ContentDeleteInput,
 ): Promise<ContentDeletePlan> {
   const { model } = input
+
+  // A non-i18n model stores content locale-agnostically in data.json and keeps
+  // exactly one meta record, pinned to the default locale. A locale-scoped
+  // delete is therefore meaningless — and actively destructive: the locale
+  // silently maps onto data.json and the default-locale meta, so passing a
+  // non-default locale deletes the shared content and the wrong meta while the
+  // named locale's stray meta is never touched. Reject it; stray per-locale
+  // meta is cleaned up by `contentrain_validate fix:true`.
+  if (!model.i18n && input.locale) {
+    throw new Error(
+      `Model "${model.id}" has i18n disabled — its content is locale-agnostic (stored in data.json), `
+      + 'so a locale-scoped delete is invalid. Omit "locale" to delete the entry, '
+      + 'or run contentrain_validate fix:true to remove stray per-locale meta files.',
+    )
+  }
+
   const changes: FileChange[] = []
   const removed: string[] = []
 

packages/mcp/src/core/validator/project.ts

@@ -264,6 +264,15 @@ async function validateCollectionModel(
     }
   }
 
+  // Consolidate a non-i18n model's meta layout BEFORE the orphan checks below.
+  // The reverse orphan-content check fabricates a draft default-locale meta for
+  // any content entry lacking meta at the default locale. If a stray holds the
+  // real (e.g. published) record under a non-default locale, running that check
+  // first would mint a draft default and let this pass then delete the real
+  // record. Consolidating first preserves the authoritative record.
+  const strayResult = await checkStrayNonI18nMeta(reader, projectRoot, model, config, issues, fix)
+  fixed += strayResult.fixed
+
   // Orphan meta check
   for (const locale of locales) {
     const metaRelPath = metaFilePath(model, locale, config.locales.default)
@@ -311,7 +320,12 @@ async function validateCollectionModel(
           entry: entryId,
           message: `Orphan content: entry "${entryId}" has no metadata`,
         })
-        if (fix && projectRoot) {
+        // Skip fabrication while a non-i18n model's meta is stuck in an
+        // unresolved stray: the content already has meta (misplaced), so minting
+        // a draft default here would let the stray-consolidation pass then delete
+        // the real record on a subsequent run. The mismatch warning already asks
+        // the agent to resolve the layout by hand.
+        if (fix && projectRoot && !strayResult.unresolved) {
           await writeMeta(projectRoot, model, { locale, entryId, defaultLocale: config.locales.default }, {
             status: 'draft',
             source: 'import',
@@ -346,49 +360,85 @@ async function validateCollectionModel(
     }
   }
 
-  await checkStrayNonI18nMeta(reader, model, config, issues)
-
   return { entries: entriesChecked, fixed }
 }
 
 /**
- * Flag meta files a non-i18n model should not have.
+ * Flag — and, with `fix`, remediate — meta files a non-i18n model should not have.
  *
  * Such a model keeps all content in one `data.json` and therefore exactly one
  * meta record, at the default locale. Earlier writes derived the meta path from
  * the caller's locale, so saving under a non-default locale left a second meta
- * file — and readers disagreed about which was authoritative. Reported rather
- * than auto-removed: the stray may hold the only `published` status in the
- * project, so deleting it silently could unpublish content.
+ * file, and readers disagreed about which was authoritative.
+ *
+ * The `fix` remediation is deterministic and never decides a status:
+ *   - default-locale meta present  → the strays are redundant, so delete them
+ *     (the default-locale record stays authoritative — no status is merged).
+ *   - default-locale meta absent, exactly one stray → that stray holds the only
+ *     record, so migrate it to the default path (move) rather than orphan the
+ *     content.
+ *   - default-locale meta absent, several strays → which is authoritative is
+ *     ambiguous, so leave the warning for the agent to resolve by hand.
+ *
+ * Returns the count of files remediated and whether strays remain unresolved.
+ * `unresolved` gates the caller's orphan-content fabrication: while a non-i18n
+ * model's meta still lives in a stray, the content is not truly orphaned, so
+ * minting a draft default-locale record would both be wrong and set up a trap
+ * (a later fix pass would then treat the real stray as redundant and delete it).
  */
 async function checkStrayNonI18nMeta(
   reader: RepoReader,
+  projectRoot: string | undefined,
   model: ModelDefinition,
   config: ContentrainConfig,
   issues: ValidationError[],
-): Promise<void> {
-  if (model.i18n) return
+  fix: boolean,
+): Promise<{ fixed: number, unresolved: boolean }> {
+  if (model.i18n) return { fixed: 0, unresolved: false }
 
+  const metaDir = `.contentrain/meta/${model.id}`
   const expected = `${config.locales.default}.json`
   let files: string[]
   try {
-    files = await reader.listDirectory(`.contentrain/meta/${model.id}`)
+    files = await reader.listDirectory(metaDir)
   } catch {
-    return
+    return { fixed: 0, unresolved: false }
   }
 
   const strays = files.filter(f => f.endsWith('.json') && f !== expected)
-  if (strays.length === 0) return
+  if (strays.length === 0) return { fixed: 0, unresolved: false }
 
   issues.push({
     severity: 'warning',
     model: model.id,
     message:
       `Meta layout mismatch: "${model.id}" has i18n disabled, so its content lives in a single data.json `
       + `and its meta belongs at ${expected} alone — but [${strays.join(', ')}] also exist. `
-      + 'Readers may disagree about which file is authoritative. Merge any status you want to keep into '
-      + `${expected}, then remove the extras.`,
+      + 'Readers may disagree about which file is authoritative. Run contentrain_validate fix:true to prune '
+      + `the extras (the default-locale meta stays authoritative).`,
   })
+
+  if (!fix || !projectRoot) return { fixed: 0, unresolved: true }
+
+  const defaultMeta = await readJsonViaReader<Record<string, unknown>>(reader, `${metaDir}/${expected}`)
+
+  if (defaultMeta === null) {
+    // No authoritative default. A single stray holds the only record — keep it
+    // by moving it to the default path. Several strays with no default is
+    // ambiguous, so do not guess: leave the warning standing (unresolved).
+    if (strays.length !== 1) return { fixed: 0, unresolved: true }
+    const stray = strays[0]!
+    const content = await readJsonViaReader<Record<string, unknown>>(reader, `${metaDir}/${stray}`)
+    if (content === null) return { fixed: 0, unresolved: true }
+    await writeJson(join(projectRoot, metaDir, expected), content)
+    await rm(join(projectRoot, metaDir, stray), { force: true })
+    return { fixed: 1, unresolved: false }
+  }
+
+  // Default-locale meta is authoritative → the strays are redundant. Each is a
+  // distinct file, so removing them in parallel is safe.
+  await Promise.all(strays.map(stray => rm(join(projectRoot, metaDir, stray), { force: true })))
+  return { fixed: strays.length, unresolved: false }
 }
 
 async function validateSingletonModel(
@@ -676,14 +726,18 @@ async function validateDocumentModel(
   // `unique` compares an entry against its siblings, so it needs the whole set —
   // validating one file at a time is why `unique` was silently a no-op for
   // documents, which is exactly where every shipped template declares it.
+  // Keyed by `${slug}\u0000${locale}`. The NUL separator can appear in neither
+  // a slug nor a locale, so composite keys never collide. Written as the escape
+  // `\u0000` (not a raw NUL byte) so the source stays plain text — a literal NUL
+  // makes tools classify this file as binary and breaks grep/diff.
   const rawByKey = new Map<string, string>()
   const frontmatterByLocale: Record<string, Record<string, Record<string, unknown>>> = {}
   for (const slug of slugs) {
     if (slug.startsWith('.')) continue
     for (const locale of locales) {
       const raw = await readTextViaReader(reader, documentFilePath(model, locale, slug))
       if (!raw) continue
-      rawByKey.set(`${slug}${locale}`, raw)
+      rawByKey.set(`${slug}\u0000${locale}`, raw)
       const { frontmatter } = parseFrontmatter(raw)
       frontmatterByLocale[locale] ??= {}
       frontmatterByLocale[locale][slug] = frontmatter
@@ -695,7 +749,7 @@ async function validateDocumentModel(
 
     for (const locale of locales) {
       const filePath = documentFilePath(model, locale, slug)
-      const raw = rawByKey.get(`${slug}${locale}`) ?? null
+      const raw = rawByKey.get(`${slug}\u0000${locale}`) ?? null
 
       if (!raw) {
         if (model.i18n) {

packages/mcp/src/tools/workflow.ts

@@ -64,10 +64,10 @@ export function registerWorkflowTools(
   // ─── contentrain_validate ───
   server.tool(
     'contentrain_validate',
-    'Validate project content against model schemas. Detects required field violations, type mismatches, broken relations, secret leaks, i18n parity issues, and more. If fix:true, auto-fixes structural issues (canonical sort, orphan meta, missing locale files) — do NOT manually edit .contentrain/ files.',
+    'Validate project content against model schemas. Detects required field violations, type mismatches, broken relations, secret leaks, i18n parity issues, and more. If fix:true, auto-fixes structural issues (canonical sort, orphan meta, missing locale files, stray non-i18n meta layout) — do NOT manually edit .contentrain/ files.',
     {
       model: z.string().optional().describe('Model ID to validate (omit for all models)'),
-      fix: z.boolean().optional().describe('Auto-fix structural issues (canonical sort, orphan meta, missing locale files). Default: false'),
+      fix: z.boolean().optional().describe('Auto-fix structural issues (canonical sort, orphan meta, missing locale files, stray non-i18n meta layout). Default: false'),
     },
     TOOL_ANNOTATIONS['contentrain_validate']!,
     async (input) => {

packages/mcp/tests/core/content.test.ts

@@ -507,7 +507,9 @@ describe('deleteContent', () => {
       { id: 'delete-me', locale: 'en', data: { name: 'Goner' } },
     ], config)
 
-    const removed = await deleteContent(testDir, collectionModel, { id: 'delete-me', locale: 'en' })
+    // collectionModel is i18n:false — content is locale-agnostic, so the delete
+    // is NOT locale-scoped (a locale-scoped delete is now rejected; see below).
+    const removed = await deleteContent(testDir, collectionModel, { id: 'delete-me', defaultLocale: 'en' })
     expect(removed).toHaveLength(1)
     expect(removed[0]).toContain('delete-me')
 
@@ -519,6 +521,25 @@ describe('deleteContent', () => {
     expect(content!['delete-me']).toBeUndefined()
   })
 
+  it('rejects a locale-scoped delete on a non-i18n collection', async () => {
+    await writeContent(testDir, collectionModel, [
+      { id: 'keep-me', locale: 'en', data: { name: 'Keeper' } },
+    ], config)
+
+    // Passing a locale on an i18n:false model used to empty data.json and delete
+    // the default-locale meta while leaving the named locale's stray meta intact.
+    // It is now rejected outright.
+    await expect(
+      deleteContent(testDir, collectionModel, { id: 'keep-me', locale: 'tr', defaultLocale: 'en' }),
+    ).rejects.toThrow(/i18n disabled/)
+
+    // The rejected call must touch nothing — the entry survives in data.json.
+    const content = await readJson<Record<string, unknown>>(
+      join(contentrainDir(testDir), 'content', 'blog', 'authors', 'data.json'),
+    )
+    expect(content!['keep-me']).toBeDefined()
+  })
+
   it('removes document slug directory', async () => {
     await writeContent(testDir, documentModel, [
       { slug: 'to-delete', locale: 'en', data: { title: 'Delete Me', slug: 'to-delete', body: '# Gone' } },

packages/mcp/tests/core/ops/content-delete.test.ts

@@ -0,0 +1,75 @@
+import { describe, expect, it } from 'vitest'
+import type { ModelDefinition } from '@contentrain/types'
+import { planContentDelete } from '../../../src/core/ops/content-delete.js'
+import type { RepoReader } from '../../../src/core/contracts/index.js'
+
+// The non-i18n locale guard rejects before any reader access, so a reader that
+// throws on every call proves the rejection happens up front — no file is read
+// or planned for removal.
+const throwingReader: RepoReader = {
+  async readFile() { throw new Error('reader must not be touched') },
+  async listDirectory() { throw new Error('reader must not be touched') },
+  async fileExists() { throw new Error('reader must not be touched') },
+}
+
+const nonI18nCollection: ModelDefinition = {
+  id: 'authors',
+  name: 'Authors',
+  kind: 'collection',
+  domain: 'blog',
+  i18n: false,
+  fields: { name: { type: 'string', required: true } },
+}
+
+const i18nCollection: ModelDefinition = {
+  ...nonI18nCollection,
+  id: 'guides',
+  i18n: true,
+}
+
+describe('planContentDelete — non-i18n locale guard', () => {
+  it('rejects a locale-scoped delete on a non-i18n model without touching the store', async () => {
+    await expect(
+      planContentDelete(throwingReader, {
+        model: nonI18nCollection,
+        id: 'abc123',
+        locale: 'en',
+        defaultLocale: 'tr',
+      }),
+    ).rejects.toThrow(/i18n disabled/)
+  })
+
+  it('allows a locale-free delete on a non-i18n model', async () => {
+    // No locale → the guard passes and the reader IS used. An empty project
+    // yields an empty plan; we only assert the guard did not fire.
+    const emptyReader: RepoReader = {
+      async readFile() { throw new Error('File not found') },
+      async listDirectory() { return [] },
+      async fileExists() { return false },
+    }
+    const plan = await planContentDelete(emptyReader, {
+      model: nonI18nCollection,
+      id: 'abc123',
+      defaultLocale: 'tr',
+    })
+    expect(plan.result).toEqual([])
+    expect(plan.changes).toEqual([])
+  })
+
+  it('still allows a locale-scoped delete on an i18n model', async () => {
+    // i18n models legitimately store content per locale, so the locale must
+    // pass through. An empty project yields an empty plan (no throw).
+    const emptyReader: RepoReader = {
+      async readFile() { throw new Error('File not found') },
+      async listDirectory() { return [] },
+      async fileExists() { return false },
+    }
+    const plan = await planContentDelete(emptyReader, {
+      model: i18nCollection,
+      id: 'abc123',
+      locale: 'en',
+      defaultLocale: 'tr',
+    })
+    expect(plan.result).toEqual([])
+  })
+})