diff --git a/.changeset/field-constraints.md b/.changeset/field-constraints.md new file mode 100644 index 0000000..583134f --- /dev/null +++ b/.changeset/field-constraints.md @@ -0,0 +1,78 @@ +--- +"@contentrain/mcp": major +"@contentrain/types": minor +--- + +feat(mcp)!: enforce the field constraints the schema already accepted + +A project reported that `items`, `accept` and `maxSize` are accepted on a field but +never enforced — `emails: ["not-an-email"]` and `accept: "image/jpeg"` against a +`.webp` both produced zero errors. The report was right, and the surface was larger +than the three properties it named: **4 of 27 field types had any semantic +validation**, three constraints were read by nothing, and none of it blocked a write. + +A constraint that isn't a constraint is worse than no constraint — the author stops +looking. + +**`content_save` now validates before committing and refuses to write.** It ran +`plan → commit → validate → report`, so an invalid value landed in git, was +auto-merged, and the caller learned about it from a string in `next_steps` while +`status` still said `"committed"`. Validation now runs on the pending changes and +blocks on errors, returning `isError` and no commit. Warnings still pass — they are +heuristics, and a legitimate value can sit outside an approximate pattern. Only the +entries being saved are fatal: a pre-existing bad entry elsewhere in the model does +not hold up an unrelated save. + +**Array items share the scalar rule set.** They ran through a parallel type switch +that knew 10 of the 27 types and checked only `typeof`, so `min`/`max`/`pattern`/ +`options` never reached an item, and `items` given as a FieldDef with a non-object +type (`{type:'array', items:{type:'string', max:50}}`) matched no branch at all — +silently unvalidated, while the type emitter rendered it as real. Items now recurse +through the same validator, which also closes the `integer` split where `3.7` was +rejected inside an array but accepted as a scalar. + +**17 types were pure `typeof` checks.** `slug` now uses the `SLUG_PATTERN` the +codebase already owned — every shipped template declares `slug: { type: 'slug' }`, +so `"Hello World!!"` used to validate clean. `date`/`datetime` are parsed (the same +check `schedule.ts` already did for meta), `percent` is range-checked, and `color`/ +`phone` warn. Mechanical rules are errors; heuristics are warnings. `email`/`url` +keep their existing warning severity. `rating` is deliberately untouched — its scale +is never declared, so any range would be invented. + +**`unique` works on documents.** It was gated on a context only the collection +validator passed, so it was a no-op exactly where every shipped template declares it. +On singletons it is now rejected at model_save: the model holds one record per +locale, so there is nothing to compare against. + +**The dead constraints, handled honestly.** `accept` is enforced by extension-sniff +and says that is what it is. `default` is coherence-checked at model_save (right +type, within its own `options`) but not written into content. `maxSize` **cannot be +enforced by MCP** — it holds a path, never the bytes — so model_save now says so and +points at the provider, which owns the policy at ingest. The docs claimed all three +worked; they no longer do. + +**model_save rejects what it will not enforce.** `options` on a non-select, `items` +on a non-array, `accept`/`maxSize` on a non-media field, `min > max`, and an +uncompilable `pattern` are now errors instead of silent no-ops. Nested `fields`/ +`items` schemas are validated recursively — they were typed `z.unknown()` and never +checked. The field schema is `.strict()`: a typo'd constraint (`requird: true`) used +to be stripped without a word. + +BREAKING CHANGE: + +- `content_save` rejects content it previously committed. Run `contentrain_validate` + before upgrading to see what would now be blocked. +- `model_save` rejects models it previously accepted (unknown keys, `min > max`, + `options` on a non-select, `unique` on a singleton). +- `validateModelDefinition` returns `{ errors, warnings }` instead of `string[]`. +- Array-item type errors carry `validateFieldValue`'s message ("Type mismatch: + expected string, got number") instead of "must be a string". The field path is + unchanged. +- Nested object errors are qualified by their parent (`seo.title`, not `title`) — + a bare name was ambiguous with a top-level field. + +`@contentrain/types` gains `validateSemanticType`, `validateAccept` and +`isMediaType`; `validateFieldValue` now applies semantic and `accept` rules. + +Studio picks all of this up automatically — its `content-validation.ts` delegates to +this validator. diff --git a/docs/packages/mcp.md b/docs/packages/mcp.md index b7ca8fa..52c0a68 100644 --- a/docs/packages/mcp.md +++ b/docs/packages/mcp.md @@ -124,6 +124,38 @@ The MCP server exposes **24 tools** — 19 core + 5 media — organized by funct | `contentrain_apply` | Extract or reuse | Two-phase normalize: extract content or patch source files | | `contentrain_bulk` | Batch operations | Bulk locale copy, status updates, and deletes | +### Field constraints + +`contentrain_content_save` **validates before it writes**. A `severity: error` issue +on any entry in the call means nothing is committed — no branch, nothing to clean up. +Fix the values and call again. Warnings pass and come back in the response. + +The split is deliberate: + +- **Errors** are definitional. A `slug` that does not match `SLUG_PATTERN` is not a + slug; an unparseable `date` is not a date; `3.7` is not an `integer`. +- **Warnings** are heuristics. `email`, `url`, `color` and `phone` patterns are + approximations, and a legitimate value can sit outside one. + +Only the entries in the call are fatal. A pre-existing bad entry elsewhere in the +model is reported but does not block your save. + +Array items are validated by the same rules as a scalar of that type, so +`items: { type: 'string', max: 50 }` means what it looks like it means. + +::: warning What MCP does not enforce +`maxSize` is the one constraint MCP accepts but cannot check — it stores a path and +never sees the file. Your media provider enforces it at ingest, and `model_save` +returns a `schema_warnings` entry saying so. `accept` **is** checked, but against the +file extension only, which is why it warns rather than errors. +::: + +`model_save` rejects a constraint declared where it cannot apply — `options` on a +non-select, `items` on a non-array, `accept` on a non-media field, `unique` on a +singleton, `min > max`, an uncompilable `pattern` — instead of storing it and doing +nothing. Unknown keys are rejected too, so a typo'd `requird: true` is an error +rather than a silent no-op. + ### Publish status Entry status lives in `.contentrain/meta/`, not in content, and it decides CDN diff --git a/packages/mcp/src/core/apply-manager.ts b/packages/mcp/src/core/apply-manager.ts index a1bc1fe..693b064 100644 --- a/packages/mcp/src/core/apply-manager.ts +++ b/packages/mcp/src/core/apply-manager.ts @@ -395,8 +395,8 @@ export async function applyExtract( kind: ext.kind, fields: ext.fields as Record | undefined, }) - if (modelErrors.length > 0) { - validationErrors.push(...modelErrors.map(e => `[${ext.model}] ${e}`)) + if (modelErrors.errors.length > 0) { + validationErrors.push(...modelErrors.errors.map(e => `[${ext.model}] ${e}`)) } for (const entry of ext.entries) { diff --git a/packages/mcp/src/core/model-manager.ts b/packages/mcp/src/core/model-manager.ts index d6d754f..b4d780a 100644 --- a/packages/mcp/src/core/model-manager.ts +++ b/packages/mcp/src/core/model-manager.ts @@ -450,6 +450,10 @@ export const FIELD_TYPE_ENUM = [ /** * Shared Zod schema for field definitions. * Used by both model_save and normalize extract for full parity. + * + * `.strict()` is load-bearing: the default `z.object` *strips* unknown keys, so a + * typo'd constraint (`requird: true`) used to vanish without a word and the field + * silently lost the rule its author thought they had declared. */ export const fieldDefZodSchema: z.ZodType> = z.record(z.string(), z.object({ type: z.enum(FIELD_TYPE_ENUM).describe('Field type from the 27-type catalog'), @@ -466,7 +470,7 @@ export const fieldDefZodSchema: z.ZodType> = z.record(z. accept: z.string().optional(), maxSize: z.number().optional(), description: z.string().optional(), -}).refine( +}).strict().refine( (f) => { if ((f.type === 'relation' || f.type === 'relations') && !f.model) return false if (f.type === 'select' && (!f.options || f.options.length === 0)) return false @@ -479,13 +483,182 @@ export const fieldDefZodSchema: z.ZodType> = z.record(z. const VALID_FIELD_TYPES = new Set(FIELD_TYPE_ENUM) +/** Field types whose value is a path/URL to a media asset. */ +const MEDIA_FIELD_TYPES = new Set(['image', 'video', 'file']) +/** Bounds `items`/`fields` nesting; far above any real schema. */ +const MAX_SCHEMA_DEPTH = 10 + +export interface ModelDefinitionIssues { + /** Block the write. */ + errors: string[] + /** Surface to the caller; the write proceeds. */ + warnings: string[] +} + +interface RawFieldDef { + type?: string + required?: unknown + unique?: unknown + default?: unknown + min?: unknown + max?: unknown + pattern?: unknown + options?: unknown + model?: unknown + items?: unknown + fields?: unknown + accept?: unknown + maxSize?: unknown +} + +/** + * Check one field definition, recursing into `fields` and `items`. + * + * The governing rule: **do not accept a constraint that will not be enforced.** + * A constraint that silently does nothing is worse than no constraint, because + * the author stops looking. So a property declared where it cannot apply is an + * error, and a property we genuinely cannot enforce says so out loud. + */ +function checkFieldDef( + raw: unknown, + path: string, + modelKind: string, + errors: string[], + warnings: string[], + depth: number, +): void { + if (typeof raw !== 'object' || raw === null) { + errors.push(`Field "${path}": must be an object`) + return + } + if (depth > MAX_SCHEMA_DEPTH) { + errors.push(`Field "${path}": exceeds the maximum nesting depth of ${MAX_SCHEMA_DEPTH}`) + return + } + + const def = raw as RawFieldDef + const type = def.type + + if (!type || !VALID_FIELD_TYPES.has(type)) { + errors.push(`Field "${path}": invalid type "${type}"`) + return + } + + if ((type === 'relation' || type === 'relations') && !def.model) { + errors.push(`Field "${path}": ${type} type requires "model" property`) + } + if (type === 'select' && (!Array.isArray(def.options) || def.options.length === 0)) { + errors.push(`Field "${path}": select type requires non-empty "options" array`) + } + // The reverse direction was never checked, so `options` on a string field was + // accepted and then silently ignored at validation time. + if (def.options !== undefined && type !== 'select') { + errors.push(`Field "${path}": "options" only applies to select fields — it is ignored on "${type}"`) + } + if (def.items !== undefined && type !== 'array') { + errors.push(`Field "${path}": "items" only applies to array fields — it is ignored on "${type}"`) + } + if (def.fields !== undefined && type !== 'object') { + errors.push(`Field "${path}": "fields" only applies to object fields — it is ignored on "${type}"`) + } + if (def.accept !== undefined && !MEDIA_FIELD_TYPES.has(type)) { + errors.push(`Field "${path}": "accept" only applies to image/video/file fields — it is ignored on "${type}"`) + } + if (def.maxSize !== undefined && !MEDIA_FIELD_TYPES.has(type)) { + errors.push(`Field "${path}": "maxSize" only applies to image/video/file fields — it is ignored on "${type}"`) + } + + // A singleton holds one record per locale, so there is nothing for a value to + // be unique against. + if (def.unique === true && modelKind === 'singleton') { + errors.push(`Field "${path}": "unique" has no meaning on a singleton — the model holds a single record per locale`) + } + + if (typeof def.min === 'number' && typeof def.max === 'number' && def.min > def.max) { + errors.push(`Field "${path}": min (${def.min}) is greater than max (${def.max})`) + } + + // Compile the regex here rather than let it fail once per entry at validation + // time, where it degrades to a warning and silently disables the constraint. + if (typeof def.pattern === 'string') { + try { + // eslint-disable-next-line no-new + new RegExp(def.pattern) + } catch { + errors.push(`Field "${path}": "pattern" is not a valid regular expression — /${def.pattern}/`) + } + } + + if (def.default !== undefined) { + checkDefaultCoherence(def, type, path, errors) + } + + // `max` on a media field measures the length of the stored path string, not + // the file — almost certainly not what the author meant. + if (typeof def.max === 'number' && MEDIA_FIELD_TYPES.has(type)) { + warnings.push( + `Field "${path}": "max" on a ${type} field limits the length of the stored path string, not the file size. Use "maxSize" for bytes.`, + ) + } + + // Said plainly rather than accepted in silence: MCP holds a path, never the + // bytes, so it cannot check this. The provider owns the policy at ingest. + if (def.maxSize !== undefined && MEDIA_FIELD_TYPES.has(type)) { + warnings.push( + `Field "${path}": "maxSize" is not enforced by MCP — it has no access to the file. Your media provider enforces it when the asset is ingested.`, + ) + } + + if (def.fields !== undefined && typeof def.fields === 'object' && def.fields !== null) { + for (const [nested, nestedDef] of Object.entries(def.fields as Record)) { + if (!/^[a-z][a-z0-9_]*$/.test(nested)) { + errors.push(`Field "${path}.${nested}": invalid name — must be snake_case starting with letter`) + } + checkFieldDef(nestedDef, `${path}.${nested}`, modelKind, errors, warnings, depth + 1) + } + } + + if (typeof def.items === 'string') { + if (!VALID_FIELD_TYPES.has(def.items)) { + errors.push(`Field "${path}.items": invalid type "${def.items}"`) + } + } else if (def.items !== undefined) { + checkFieldDef(def.items, `${path}.items`, modelKind, errors, warnings, depth + 1) + } +} + +/** A default that its own field would reject is a schema bug, not a content one. */ +function checkDefaultCoherence(def: RawFieldDef, type: string, path: string, errors: string[]): void { + const value = def.default + const isString = typeof value === 'string' + const isNumber = typeof value === 'number' + + if (type === 'select' && Array.isArray(def.options) && isString && !def.options.includes(value)) { + errors.push(`Field "${path}": default "${value}" is not one of its own options [${(def.options as string[]).join(', ')}]`) + return + } + const wantsNumber = ['number', 'integer', 'decimal', 'percent', 'rating'].includes(type) + const wantsBoolean = type === 'boolean' + const wantsArray = type === 'array' + + if (wantsNumber && !isNumber) { + errors.push(`Field "${path}": default must be a number for type "${type}"`) + } else if (wantsBoolean && typeof value !== 'boolean') { + errors.push(`Field "${path}": default must be a boolean for type "${type}"`) + } else if (wantsArray && !Array.isArray(value)) { + errors.push(`Field "${path}": default must be an array for type "${type}"`) + } +} + /** * Validate a model definition before writing. - * Returns array of error messages (empty = valid). - * Used by both model_save tool and normalize extract. + * Used by both the model_save tool and normalize extract. */ -export function validateModelDefinition(input: { id: string; kind: string; fields?: Record }): string[] { +export function validateModelDefinition( + input: { id: string; kind: string; fields?: Record }, +): ModelDefinitionIssues { const errors: string[] = [] + const warnings: string[] = [] // ID format: kebab-case if (!/^[a-z0-9]+(?:-[a-z0-9]+)*$/.test(input.id)) { @@ -497,32 +670,14 @@ export function validateModelDefinition(input: { id: string; kind: string; field errors.push('Dictionary models cannot have fields. Dictionaries store flat key-value pairs.') } - // Fields validation if (input.fields) { for (const [fieldName, fieldDef] of Object.entries(input.fields)) { - const def = fieldDef as { type?: string; model?: unknown; options?: unknown } - - // Field name format if (!/^[a-z][a-z0-9_]*$/.test(fieldName)) { errors.push(`Field "${fieldName}": invalid name — must be snake_case starting with letter`) } - - // Type check - if (!def.type || !VALID_FIELD_TYPES.has(def.type)) { - errors.push(`Field "${fieldName}": invalid type "${def.type}"`) - } - - // Relation requires model - if ((def.type === 'relation' || def.type === 'relations') && !def.model) { - errors.push(`Field "${fieldName}": ${def.type} type requires "model" property`) - } - - // Select requires options - if (def.type === 'select' && (!def.options || !Array.isArray(def.options) || def.options.length === 0)) { - errors.push(`Field "${fieldName}": select type requires non-empty "options" array`) - } + checkFieldDef(fieldDef, fieldName, input.kind, errors, warnings, 0) } } - return errors + return { errors, warnings } } diff --git a/packages/mcp/src/core/validator/entry.ts b/packages/mcp/src/core/validator/entry.ts index 2e07940..0e8bdfb 100644 --- a/packages/mcp/src/core/validator/entry.ts +++ b/packages/mcp/src/core/validator/entry.ts @@ -51,6 +51,9 @@ export function validateContent( } } +/** Bounds `items`-inside-`items` nesting; far above any real schema. */ +const MAX_FIELD_DEPTH = 10 + function validateField( value: unknown, def: FieldDef, @@ -59,10 +62,15 @@ function validateField( entryId: string | undefined, fieldId: string, ctx?: ValidationContext, + depth = 0, ): ValidationError[] { const errors: ValidationError[] = [] const errCtx = { model: modelId, locale, entry: entryId, field: fieldId } + if (depth > MAX_FIELD_DEPTH) { + return [{ severity: 'error', ...errCtx, message: `${fieldId} exceeds the maximum nesting depth of ${MAX_FIELD_DEPTH}` }] + } + if (value !== null && value !== undefined && value !== '') { const secretErrors = detectSecrets(value) for (const e of secretErrors) { @@ -102,13 +110,6 @@ function validateField( } } - if (def.type === 'email' && typeof value === 'string' && !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(value)) { - errors.push({ severity: 'warning', ...errCtx, message: `${fieldId} may not be a valid email` }) - } - if (def.type === 'url' && typeof value === 'string' && !/^https?:\/\/.+/.test(value) && !value.startsWith('/')) { - errors.push({ severity: 'warning', ...errCtx, message: `${fieldId} may not be a valid URL` }) - } - if (def.type === 'relation' && def.model) { const targets = Array.isArray(def.model) ? def.model : [def.model] if (targets.length > 1) { @@ -168,66 +169,33 @@ function validateField( } } - if (def.type === 'array' && Array.isArray(value) && def.items && typeof def.items === 'string') { + // Array items run through `validateField` itself rather than a parallel type + // switch. The old switch knew 10 of the 27 field types and checked only + // `typeof`, so `min`/`max`/`pattern`/`options` never reached an item and an + // `items` given as a FieldDef with a non-object type matched no branch at all — + // silently unvalidated, while the type emitter rendered it as a real constraint. + // Recursing keeps one rule set for a type wherever it appears. + if (def.type === 'array' && Array.isArray(value) && def.items) { + const itemDef: FieldDef = typeof def.items === 'string' + ? { type: def.items as FieldDef['type'] } + : def.items for (let i = 0; i < value.length; i++) { - errors.push(...validateArrayItemType(value[i], def.items, errCtx, `${fieldId}[${i}]`)) + // `ctx` is deliberately dropped: `unique` compares an entry against its + // siblings, which has no meaning for positions inside one array. + errors.push(...validateField( + value[i], itemDef, modelId, locale, entryId, `${fieldId}[${i}]`, undefined, depth + 1, + )) } } if (def.type === 'object' && def.fields && typeof value === 'object' && value !== null && !Array.isArray(value)) { - const nested = validateContent(value as Record, def.fields, modelId, locale, entryId) - errors.push(...nested.errors) - } - - if ( - def.type === 'array' - && Array.isArray(value) - && def.items - && typeof def.items === 'object' - && def.items.type === 'object' - && def.items.fields - ) { - for (let i = 0; i < value.length; i++) { - if (typeof value[i] === 'object' && value[i] !== null) { - const nested = validateContent(value[i] as Record, def.items.fields, modelId, locale, entryId) - for (const e of nested.errors) { - errors.push({ ...e, field: `${fieldId}[${i}].${e.field}` }) - } - } + const nested = validateContent(value as Record, def.fields, modelId, locale, entryId, ctx) + // Qualify the nested path with the parent so an error names where it lives — + // `meta.title`, or `blocks[0].title` when this object is an array item. + for (const e of nested.errors) { + errors.push({ ...e, field: e.field ? `${fieldId}.${e.field}` : fieldId }) } } return errors } - -function validateArrayItemType( - value: unknown, - itemType: string, - errCtx: { model: string, locale: string, entry: string | undefined, field: string }, - fieldPath: string, -): ValidationError[] { - const ctx = { ...errCtx, field: fieldPath } - switch (itemType) { - case 'string': - case 'email': - case 'url': - case 'slug': - case 'image': - case 'video': - case 'file': - if (typeof value !== 'string') return [{ severity: 'error', ...ctx, message: `${fieldPath} must be a string` }] - break - case 'number': - case 'integer': - case 'decimal': - if (typeof value !== 'number') return [{ severity: 'error', ...ctx, message: `${fieldPath} must be a number` }] - if (itemType === 'integer' && !Number.isInteger(value)) { - return [{ severity: 'error', ...ctx, message: `${fieldPath} must be an integer` }] - } - break - case 'boolean': - if (typeof value !== 'boolean') return [{ severity: 'error', ...ctx, message: `${fieldPath} must be a boolean` }] - break - } - return [] -} diff --git a/packages/mcp/src/core/validator/project.ts b/packages/mcp/src/core/validator/project.ts index 62bdfe0..6346980 100644 --- a/packages/mcp/src/core/validator/project.ts +++ b/packages/mcp/src/core/validator/project.ts @@ -672,12 +672,30 @@ async function validateDocumentModel( // per-locale orphan/parity warnings. Iterate just the default locale. #8/Y3 const locales = model.i18n ? config.locales.supported : [config.locales.default] + // Read every document once up front and keep its frontmatter per locale. + // `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. + const rawByKey = new Map() + const frontmatterByLocale: Record>> = {} + 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) + const { frontmatter } = parseFrontmatter(raw) + frontmatterByLocale[locale] ??= {} + frontmatterByLocale[locale][slug] = frontmatter + } + } + for (const slug of slugs) { if (slug.startsWith('.')) continue for (const locale of locales) { const filePath = documentFilePath(model, locale, slug) - const raw = await readTextViaReader(reader, filePath) + const raw = rawByKey.get(`${slug}${locale}`) ?? null if (!raw) { if (model.i18n) { @@ -731,6 +749,10 @@ async function validateDocumentModel( fieldsWithoutBody, model.id, locale, + undefined, + // Siblings in the same locale, so `unique` has something to compare + // against. Keyed by slug — a document's identity is its slug. + { allEntries: frontmatterByLocale[locale] ?? {}, currentEntryId: slug }, ) for (const err of entryResult.errors) { issues.push({ ...err, slug }) diff --git a/packages/mcp/src/tools/content.ts b/packages/mcp/src/tools/content.ts index f107360..441b4e8 100644 --- a/packages/mcp/src/tools/content.ts +++ b/packages/mcp/src/tools/content.ts @@ -1,10 +1,12 @@ import type { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js' import { z } from 'zod' +import type { ValidationError } from '@contentrain/types' import type { ToolProvider } from '../server.js' import { readConfig, readVocabulary } from '../core/config.js' import { readModel } from '../core/model-manager.js' import { listContent } from '../core/content-manager.js' import { planContentDelete, planContentSave } from '../core/ops/index.js' +import type { ContentSaveEntryResult } from '../core/ops/types.js' import { LocalProvider } from '../providers/local/index.js' import { buildBranchName } from '../git/transaction.js' import { checkBranchHealth } from '../git/branch-lifecycle.js' @@ -141,6 +143,35 @@ export function registerContentTools( .map(r => r.id ?? r.slug ?? r.locale) .filter((v): v is string => Boolean(v)) + // Validate BEFORE committing. This used to run after the commit and only + // report, so an invalid value landed in git — and was auto-merged — while + // the response still said `status: "committed"` and buried the problem in a + // next_steps string. The OverlayReader layers the pending changes over the + // provider, so it needs nothing from the commit. + const validationResult = await validateProject( + new OverlayReader(provider, plan.changes), + { model: input.model }, + ) + + // Only this save's own entries are fatal. validateProject checks the whole + // model, so an unrelated pre-existing error elsewhere in it must not block + // a caller who is writing something valid — they may not even be able to + // fix it. Everything else stays in the response as context. + const blocking = validationResult.issues.filter( + i => i.severity === 'error' && touchesSavedEntries(i, plan.result), + ) + + if (blocking.length > 0) { + return { + content: [{ type: 'text' as const, text: JSON.stringify({ + error: 'Validation failed — nothing was written.', + issues: blocking.map(describeIssue), + hint: 'Fix the values and call contentrain_content_save again. Nothing was committed, so there is no branch to clean up.', + }, null, 2) }], + isError: true, + } + } + const branch = buildBranchName('content', input.model) const message = `[contentrain] content: ${input.model}` const contextPayload = { @@ -173,20 +204,15 @@ export function registerContentTools( } } - // Post-save validation — runs against an OverlayReader that layers the - // just-saved changes on top of the provider's base view. This validates - // the committed state for BOTH providers and BOTH workflow modes. The - // old local path validated `projectRoot` (the developer working tree), - // which in review mode (or before selectiveSync) had not yet received the - // feature-branch files — producing false "locale file missing" errors for - // content that was just created. #7 - const validationResult = await validateProject( - new OverlayReader(provider, plan.changes), - { model: input.model }, - ) - const allAdvisories = plan.result.flatMap(r => r.advisories ?? []) + // Warnings do not block — they are heuristics (a colour that may not parse, + // an extension that may contradict `accept`), and a legitimate value can sit + // outside an approximate pattern. They ride along in the response instead. + const warnings = validationResult.issues + .filter(i => i.severity === 'warning' && touchesSavedEntries(i, plan.result)) + .map(i => `${i.field ?? i.locale}: ${i.message}`) + return { content: [{ type: 'text' as const, text: JSON.stringify({ status: 'committed', @@ -197,14 +223,12 @@ export function registerContentTools( advisories: allAdvisories, advisory_note: 'Save succeeded. Review these warnings and consider consolidating duplicate values.', } : {}), - validation: { - valid: validationResult.valid, - errors: validationResult.issues.filter(i => i.severity === 'error').map(i => i.message), - }, + ...(warnings.length > 0 ? { warnings } : {}), + validation: { valid: true, errors: [] }, context_updated: true, next_steps: [ ...(allAdvisories.length > 0 ? ['ADVISORY: Duplicate values detected — review advisories above'] : []), - ...(!validationResult.valid ? ['WARNING: Content has validation errors — run contentrain_validate for details'] : []), + ...(warnings.length > 0 ? ['REVIEW: see warnings above — the save was not blocked by them'] : []), ...(model.kind === 'collection' ? ['Use contentrain_content_list to verify', 'Add more entries or publish'] : ['Use contentrain_content_list to verify']), @@ -381,3 +405,39 @@ export function registerContentTools( }, ) } + +/** Trim a validation issue to the fields that locate it, dropping empty ones. */ +function describeIssue(issue: ValidationError): Record { + const out: Record = {} + if (issue.entry) out['entry'] = issue.entry + if (issue.slug) out['slug'] = issue.slug + if (issue.locale) out['locale'] = issue.locale + if (issue.field) out['field'] = issue.field + out['message'] = issue.message + return out +} + +/** + * Does a validation issue belong to one of the entries this save is writing? + * + * The write gate blocks on errors, but `validateProject` inspects the whole model. + * Without this filter, a pre-existing bad entry somewhere else in the model would + * block an unrelated — and perfectly valid — save, leaving the caller stuck behind + * a problem they did not create and may not be able to fix. + * + * Matching is by identity: entry ID for collections, slug for documents, and + * locale alone for singletons and dictionaries, which have one record per locale. + */ +function touchesSavedEntries( + issue: { entry?: string, slug?: string, locale?: string }, + results: ContentSaveEntryResult[], +): boolean { + return results.some((r) => { + if (issue.locale && r.locale && issue.locale !== r.locale) return false + if (r.id) return issue.entry === r.id + if (r.slug) return issue.slug === r.slug + // Singleton / dictionary: the locale is the whole identity, and an issue with + // no entry or slug is about the record itself. + return !issue.entry && !issue.slug + }) +} diff --git a/packages/mcp/src/tools/model.ts b/packages/mcp/src/tools/model.ts index 54e9b69..ee223c2 100644 --- a/packages/mcp/src/tools/model.ts +++ b/packages/mcp/src/tools/model.ts @@ -47,7 +47,7 @@ export function registerModelTools( } } - const errors = validateModel(input) + const { errors, warnings: schemaWarnings } = validateModel(input) if (errors.length > 0) { return { content: [{ type: 'text' as const, text: JSON.stringify({ error: 'Validation failed', details: errors }) }], @@ -148,12 +148,18 @@ export function registerModelTools( action, model: input.id, validation: { valid: true, errors: [] }, + // Constraints the schema accepts but MCP does not enforce, said out loud + // rather than left for the author to discover in production. + ...(schemaWarnings.length > 0 ? { schema_warnings: schemaWarnings } : {}), git: { branch, action: workflowAction, commit: commitSha, ...(sync ? { sync } : {}) }, context_updated: true, content_path: contentPath + '/', ...(displayPath ? { example_file: displayPath } : {}), ...(importSnippet ? { import_snippet: importSnippet } : {}), - next_steps: ['Add content with contentrain_content_save'], + next_steps: [ + ...(schemaWarnings.length > 0 ? ['REVIEW: see schema_warnings — some declared constraints are not enforced by MCP'] : []), + 'Add content with contentrain_content_save', + ], }, null, 2) }], } }, diff --git a/packages/mcp/tests/core/model-definition.test.ts b/packages/mcp/tests/core/model-definition.test.ts new file mode 100644 index 0000000..eecf084 --- /dev/null +++ b/packages/mcp/tests/core/model-definition.test.ts @@ -0,0 +1,163 @@ +import { describe, expect, it } from 'vitest' +import { validateModelDefinition } from '../../src/core/model-manager.js' + +/** + * Schema-shape rules for model_save. + * + * The governing principle: do not accept a constraint that will not be enforced. + * `accept`, `maxSize` and `default` were declared, stored, and read by nothing; + * `options` on a non-select was accepted and silently ignored. A constraint that + * does nothing is worse than no constraint — the author stops looking. + */ + +const model = (fields: Record, kind = 'collection') => + validateModelDefinition({ id: 'posts', kind, fields }) + +describe('validateModelDefinition', () => { + it('accepts a well-formed model', () => { + const { errors, warnings } = model({ + title: { type: 'string', required: true, max: 120 }, + slug: { type: 'slug', required: true, unique: true }, + status: { type: 'select', options: ['draft', 'live'], default: 'draft' }, + }) + expect(errors).toEqual([]) + expect(warnings).toEqual([]) + }) + + describe('constraints declared where they cannot apply', () => { + it('rejects options on a non-select field', () => { + const { errors } = model({ title: { type: 'string', options: ['a', 'b'] } }) + expect(errors.some(e => /"options" only applies to select/.test(e))).toBe(true) + }) + + it('rejects items on a non-array field', () => { + const { errors } = model({ title: { type: 'string', items: 'string' } }) + expect(errors.some(e => /"items" only applies to array/.test(e))).toBe(true) + }) + + it('rejects fields on a non-object field', () => { + const { errors } = model({ title: { type: 'string', fields: { a: { type: 'string' } } } }) + expect(errors.some(e => /"fields" only applies to object/.test(e))).toBe(true) + }) + + it('rejects accept and maxSize on a non-media field', () => { + const { errors } = model({ title: { type: 'string', accept: 'image/*', maxSize: 100 } }) + expect(errors.some(e => /"accept" only applies to image\/video\/file/.test(e))).toBe(true) + expect(errors.some(e => /"maxSize" only applies to image\/video\/file/.test(e))).toBe(true) + }) + + it('rejects unique on a singleton — there is nothing to compare against', () => { + const { errors } = model({ title: { type: 'string', unique: true } }, 'singleton') + expect(errors.some(e => /"unique" has no meaning on a singleton/.test(e))).toBe(true) + }) + + it('allows unique on a collection and a document', () => { + expect(model({ sku: { type: 'string', unique: true } }, 'collection').errors).toEqual([]) + expect(model({ sku: { type: 'string', unique: true } }, 'document').errors).toEqual([]) + }) + }) + + describe('incoherent constraints', () => { + it('rejects min greater than max', () => { + const { errors } = model({ title: { type: 'string', min: 10, max: 5 } }) + expect(errors.some(e => /min \(10\) is greater than max \(5\)/.test(e))).toBe(true) + }) + + it('rejects a pattern that does not compile', () => { + // Left to validation time this degrades to a per-entry warning, silently + // disabling the constraint. + const { errors } = model({ title: { type: 'string', pattern: '[invalid' } }) + expect(errors.some(e => /not a valid regular expression/.test(e))).toBe(true) + }) + + it('rejects a default outside its own options', () => { + const { errors } = model({ status: { type: 'select', options: ['a', 'b'], default: 'z' } }) + expect(errors.some(e => /default "z" is not one of its own options/.test(e))).toBe(true) + }) + + it('rejects a default of the wrong type', () => { + const { errors } = model({ count: { type: 'number', default: 'lots' } }) + expect(errors.some(e => /default must be a number/.test(e))).toBe(true) + }) + }) + + describe('nested schemas', () => { + it('validates a nested object field', () => { + const { errors } = model({ + seo: { type: 'object', fields: { title: { type: 'bogus' } } }, + }) + expect(errors.some(e => /Field "seo.title": invalid type "bogus"/.test(e))).toBe(true) + }) + + it('validates a nested select without options', () => { + const { errors } = model({ + seo: { type: 'object', fields: { kind: { type: 'select' } } }, + }) + expect(errors.some(e => /Field "seo.kind": select type requires/.test(e))).toBe(true) + }) + + it('validates a nested field name', () => { + const { errors } = model({ + seo: { type: 'object', fields: { BadName: { type: 'string' } } }, + }) + expect(errors.some(e => /Field "seo.BadName": invalid name/.test(e))).toBe(true) + }) + + it('validates an items FieldDef', () => { + const { errors } = model({ + tags: { type: 'array', items: { type: 'select' } }, + }) + expect(errors.some(e => /Field "tags.items": select type requires/.test(e))).toBe(true) + }) + + it('validates an items type given as a string', () => { + const { errors } = model({ tags: { type: 'array', items: 'bogus' } }) + expect(errors.some(e => /Field "tags.items": invalid type "bogus"/.test(e))).toBe(true) + }) + + it('bounds runaway nesting', () => { + let field: Record = { type: 'string' } + for (let i = 0; i < 15; i++) field = { type: 'array', items: field } + const { errors } = model({ deep: field }) + expect(errors.some(e => /nesting depth/.test(e))).toBe(true) + }) + }) + + describe('constraints MCP cannot enforce are stated, not hidden', () => { + it('warns that maxSize is the provider’s job', () => { + const { errors, warnings } = model({ cover: { type: 'image', maxSize: 500_000 } }) + // Not an error — the constraint is legitimate, MCP just cannot check it. + expect(errors).toEqual([]) + expect(warnings.some(w => /"maxSize" is not enforced by MCP/.test(w))).toBe(true) + expect(warnings.some(w => /ingested/.test(w))).toBe(true) + }) + + it('warns that max on a media field measures the path, not the file', () => { + const { errors, warnings } = model({ cover: { type: 'image', max: 100 } }) + expect(errors).toEqual([]) + expect(warnings.some(w => /limits the length of the stored path string/.test(w))).toBe(true) + }) + + it('does not warn about max on an ordinary string field', () => { + const { warnings } = model({ title: { type: 'string', max: 100 } }) + expect(warnings).toEqual([]) + }) + }) + + describe('pre-existing rules still hold', () => { + it('rejects a non-kebab-case model id', () => { + const { errors } = validateModelDefinition({ id: 'Blog_Post', kind: 'collection', fields: {} }) + expect(errors.some(e => /must be kebab-case/.test(e))).toBe(true) + }) + + it('rejects fields on a dictionary', () => { + const { errors } = model({ title: { type: 'string' } }, 'dictionary') + expect(errors.some(e => /Dictionary models cannot have fields/.test(e))).toBe(true) + }) + + it('rejects a relation without a model', () => { + const { errors } = model({ author: { type: 'relation' } }) + expect(errors.some(e => /requires "model" property/.test(e))).toBe(true) + }) + }) +}) diff --git a/packages/mcp/tests/core/validator/entry.test.ts b/packages/mcp/tests/core/validator/entry.test.ts index a16ccc8..3a752b1 100644 --- a/packages/mcp/tests/core/validator/entry.test.ts +++ b/packages/mcp/tests/core/validator/entry.test.ts @@ -176,7 +176,9 @@ describe('validateContent', () => { keywords: { type: 'array', items: 'string' }, } const result = validateContent({ keywords: ['ok', 42] }, fields, 'blog', 'en', 'abc') - expect(result.errors.some(e => e.field === 'keywords[1]' && /must be a string/.test(e.message ?? ''))).toBe(true) + // Items now share the scalar rule set, so the message is validateFieldValue's. + // The path still names the offending index. + expect(result.errors.some(e => e.field === 'keywords[1]' && /expected string/.test(e.message ?? ''))).toBe(true) }) it('array of integers rejects float', () => { @@ -187,6 +189,175 @@ describe('validateContent', () => { expect(result.errors.some(e => e.field === 'counts[1]' && /integer/.test(e.message ?? ''))).toBe(true) }) + // Array items used to run through a parallel type switch that knew 10 of the 27 + // field types and checked only `typeof`. They now share the scalar rule set. + describe('array items share the scalar rule set', () => { + it('applies the email heuristic to items, not just scalars', () => { + // The reported case: items:'email' accepted anything string-shaped. + const fields: Record = { + emails: { type: 'array', items: 'email' }, + } + const result = validateContent({ emails: ['not-an-email'] }, fields, 'blog', 'en', 'abc') + expect(result.errors.some(e => e.field === 'emails[0]' && e.severity === 'warning')).toBe(true) + }) + + it('validates a type the old item switch never knew', () => { + const fields: Record = { + published: { type: 'array', items: 'date' }, + } + const result = validateContent({ published: ['2026-07-15', 'yesterday'] }, fields, 'blog', 'en', 'abc') + expect(result.errors.some(e => e.field === 'published[1]' && e.severity === 'error')).toBe(true) + expect(result.errors.some(e => e.field === 'published[0]')).toBe(false) + }) + + it('enforces constraints on an items FieldDef — the old black hole', () => { + // `items` as a FieldDef with a non-object type matched no branch at all. + const fields: Record = { + codes: { type: 'array', items: { type: 'string', max: 3, pattern: '^[a-z]+$' } }, + } + const result = validateContent({ codes: ['ok', 'toolong', 'AB'] }, fields, 'blog', 'en', 'abc') + expect(result.errors.some(e => e.field === 'codes[1]' && /maximum/.test(e.message ?? ''))).toBe(true) + expect(result.errors.some(e => e.field === 'codes[2]' && /pattern/.test(e.message ?? ''))).toBe(true) + expect(result.errors.some(e => e.field === 'codes[0]')).toBe(false) + }) + + it('enforces select options on items', () => { + const fields: Record = { + sizes: { type: 'array', items: { type: 'select', options: ['s', 'm', 'l'] } }, + } + const result = validateContent({ sizes: ['m', 'xxl'] }, fields, 'blog', 'en', 'abc') + expect(result.errors.some(e => e.field === 'sizes[1]' && e.severity === 'error')).toBe(true) + }) + + it('bounds runaway nesting instead of blowing the stack', () => { + let items: FieldDef = { type: 'string' } + for (let i = 0; i < 15; i++) items = { type: 'array', items } + let value: unknown = 'x' + for (let i = 0; i < 15; i++) value = [value] + + const result = validateContent({ deep: value }, { deep: items }, 'blog', 'en', 'abc') + expect(result.errors.some(e => /nesting depth/.test(e.message ?? ''))).toBe(true) + }) + }) + + // 17 of 27 types were pure typeof checks. Mechanical rules are errors; heuristics + // are warnings, because a legitimate value can sit outside an approximate pattern. + describe('semantic type rules', () => { + it('rejects a slug field that is not a slug', () => { + // Every shipped template declares slug: { type: 'slug', ... }. + const result = validateContent( + { slug: 'Hello World!!' }, { slug: { type: 'slug' } }, 'blog', 'en', 'abc', + ) + expect(result.errors.some(e => e.field === 'slug' && e.severity === 'error')).toBe(true) + expect(result.valid).toBe(false) + }) + + it('accepts a well-formed slug', () => { + const result = validateContent( + { slug: 'hello-world-2' }, { slug: { type: 'slug' } }, 'blog', 'en', 'abc', + ) + expect(result.errors).toEqual([]) + }) + + it('rejects a float in an integer scalar — matching the array-item rule', () => { + // The scalar path lumped integer with number, so 3.7 passed here while the + // identical value was rejected inside an array. + const result = validateContent( + { count: 3.7 }, { count: { type: 'integer' } }, 'blog', 'en', 'abc', + ) + expect(result.errors.some(e => e.field === 'count' && e.severity === 'error')).toBe(true) + }) + + it('rejects unparseable date and datetime', () => { + const fields: Record = { + on: { type: 'date' }, + at: { type: 'datetime' }, + } + const result = validateContent({ on: '15/07/2026', at: 'not-a-time' }, fields, 'blog', 'en', 'abc') + expect(result.errors.some(e => e.field === 'on' && e.severity === 'error')).toBe(true) + expect(result.errors.some(e => e.field === 'at' && e.severity === 'error')).toBe(true) + }) + + it('accepts valid date and datetime', () => { + const fields: Record = { + on: { type: 'date' }, + at: { type: 'datetime' }, + } + const result = validateContent( + { on: '2026-07-15', at: '2026-07-15T14:08:34.746Z' }, fields, 'blog', 'en', 'abc', + ) + expect(result.errors).toEqual([]) + }) + + it('rejects a percent outside 0-100', () => { + const result = validateContent( + { pct: 140 }, { pct: { type: 'percent' } }, 'blog', 'en', 'abc', + ) + expect(result.errors.some(e => e.field === 'pct' && e.severity === 'error')).toBe(true) + }) + + it('warns — never errors — on a heuristic colour check', () => { + const result = validateContent( + { brand: 'not a colour' }, { brand: { type: 'color' } }, 'blog', 'en', 'abc', + ) + expect(result.errors.some(e => e.field === 'brand' && e.severity === 'warning')).toBe(true) + // A heuristic must not fail the entry. + expect(result.valid).toBe(true) + }) + + it('accepts the colour forms an author actually writes', () => { + const fields: Record = { + a: { type: 'color' }, b: { type: 'color' }, c: { type: 'color' }, d: { type: 'color' }, + } + const result = validateContent( + { a: '#fff', b: '#1a2b3c', c: 'rgb(1, 2, 3)', d: 'rebeccapurple' }, fields, 'blog', 'en', 'abc', + ) + expect(result.errors).toEqual([]) + }) + + it('leaves rating alone — its scale is never declared', () => { + const result = validateContent( + { stars: 42 }, { stars: { type: 'rating' } }, 'blog', 'en', 'abc', + ) + expect(result.errors).toEqual([]) + }) + }) + + describe('accept on media fields', () => { + it('warns when the extension contradicts accept, and says it is a sniff', () => { + // The reported case: accept:"image/jpeg" with a .webp value. + const fields: Record = { + cover: { type: 'image', accept: 'image/jpeg' }, + } + const result = validateContent({ cover: 'media/original/a.webp' }, fields, 'blog', 'en', 'abc') + const issue = result.errors.find(e => e.field === 'cover') + expect(issue?.severity).toBe('warning') + expect(issue?.message).toContain('extension check') + expect(result.valid).toBe(true) + }) + + it('accepts a matching extension and a wildcard', () => { + const fields: Record = { + a: { type: 'image', accept: 'image/jpeg' }, + b: { type: 'image', accept: 'image/*' }, + c: { type: 'file', accept: '.pdf' }, + } + const result = validateContent( + { a: 'media/x.jpg', b: 'media/y.webp', c: 'media/z.pdf' }, fields, 'blog', 'en', 'abc', + ) + expect(result.errors).toEqual([]) + }) + + it('stays silent on an extension it cannot map to a MIME type', () => { + // An unknown extension is not evidence of a violation. + const fields: Record = { + cover: { type: 'image', accept: 'image/jpeg' }, + } + const result = validateContent({ cover: 'media/original/a.heic' }, fields, 'blog', 'en', 'abc') + expect(result.errors).toEqual([]) + }) + }) + it('nested object validation recurses into fields', () => { const fields: Record = { seo: { @@ -198,7 +369,9 @@ describe('validateContent', () => { } const result = validateContent({ seo: {} }, fields, 'blog', 'en', 'abc') expect(result.valid).toBe(false) - expect(result.errors.some(e => e.field === 'title' && e.severity === 'error')).toBe(true) + // The path is qualified by its parent — a bare `title` would be ambiguous with + // a top-level field of the same name. + expect(result.errors.some(e => e.field === 'seo.title' && e.severity === 'error')).toBe(true) }) it('array-of-object validation prefixes field path with index', () => { diff --git a/packages/mcp/tests/core/validator/unique.test.ts b/packages/mcp/tests/core/validator/unique.test.ts new file mode 100644 index 0000000..fd4a8b5 --- /dev/null +++ b/packages/mcp/tests/core/validator/unique.test.ts @@ -0,0 +1,115 @@ +import { describe, expect, it } from 'vitest' +import { validateProject } from '../../../src/core/validator/index.js' +import type { RepoReader } from '../../../src/core/contracts/index.js' + +/** + * `unique` was gated on a validation context that only the collection validator + * ever passed, so it was silently a no-op for documents — which is exactly where + * every shipped template declares it (`templates/blog.ts`, `docs.ts`, + * `ecommerce.ts`, `saas.ts` all carry `slug: { type: 'slug', unique: true }`). + */ + +function makeInMemoryReader(files: Record): RepoReader { + return { + async readFile(path) { + const content = files[path] + if (content === undefined) throw new Error(`File not found: ${path}`) + return content + }, + async listDirectory(path) { + const prefix = path.endsWith('/') ? path : `${path}/` + const children = new Set() + for (const filePath of Object.keys(files)) { + if (!filePath.startsWith(prefix)) continue + const rest = filePath.slice(prefix.length) + const firstSegment = rest.split('/')[0] + if (firstSegment) children.add(firstSegment) + } + return [...children].toSorted() + }, + async fileExists(path) { + if (Object.hasOwn(files, path)) return true + const prefix = path.endsWith('/') ? path : `${path}/` + return Object.keys(files).some(f => f.startsWith(prefix)) + }, + } +} + +const CONFIG = JSON.stringify({ + version: 1, + stack: 'nuxt', + workflow: 'review', + locales: { default: 'en', supported: ['en'] }, + domains: ['blog'], +}) + +const DOC_MODEL = JSON.stringify({ + id: 'posts', name: 'Posts', kind: 'document', domain: 'blog', i18n: true, + fields: { + title: { type: 'string', required: true }, + sku: { type: 'string', unique: true }, + }, +}) + +const doc = (title: string, sku: string) => `---\ntitle: ${title}\nsku: ${sku}\n---\n\nBody.\n` + +describe('unique on document models', () => { + it('flags a duplicate value across two documents', () => { + const reader = makeInMemoryReader({ + '.contentrain/config.json': CONFIG, + '.contentrain/models/posts.json': DOC_MODEL, + '.contentrain/content/blog/posts/first/en.md': doc('First', 'ABC-1'), + '.contentrain/content/blog/posts/second/en.md': doc('Second', 'ABC-1'), + }) + + return validateProject(reader).then((result) => { + const dupes = result.issues.filter(i => /must be unique/.test(i.message)) + expect(dupes.length).toBeGreaterThan(0) + expect(dupes[0]!.severity).toBe('error') + expect(dupes.some(i => i.field === 'sku')).toBe(true) + expect(result.valid).toBe(false) + }) + }) + + it('stays quiet when every document carries a distinct value', async () => { + const reader = makeInMemoryReader({ + '.contentrain/config.json': CONFIG, + '.contentrain/models/posts.json': DOC_MODEL, + '.contentrain/content/blog/posts/first/en.md': doc('First', 'ABC-1'), + '.contentrain/content/blog/posts/second/en.md': doc('Second', 'ABC-2'), + }) + + const result = await validateProject(reader) + expect(result.issues.filter(i => /must be unique/.test(i.message))).toEqual([]) + }) + + it('does not compare a document against itself', async () => { + const reader = makeInMemoryReader({ + '.contentrain/config.json': CONFIG, + '.contentrain/models/posts.json': DOC_MODEL, + '.contentrain/content/blog/posts/only/en.md': doc('Only', 'ABC-1'), + }) + + const result = await validateProject(reader) + expect(result.issues.filter(i => /must be unique/.test(i.message))).toEqual([]) + }) + + it('scopes uniqueness to one locale', async () => { + // The same sku in en and tr is a translation, not a collision. + const reader = makeInMemoryReader({ + '.contentrain/config.json': JSON.stringify({ + version: 1, + stack: 'nuxt', + workflow: 'review', + locales: { default: 'en', supported: ['en', 'tr'] }, + domains: ['blog'], + }), + '.contentrain/models/posts.json': DOC_MODEL, + '.contentrain/content/blog/posts/first/en.md': doc('First', 'ABC-1'), + '.contentrain/content/blog/posts/first/tr.md': doc('Birinci', 'ABC-1'), + }) + + const result = await validateProject(reader) + expect(result.issues.filter(i => /must be unique/.test(i.message))).toEqual([]) + }) +}) diff --git a/packages/mcp/tests/tools/content-gate.test.ts b/packages/mcp/tests/tools/content-gate.test.ts new file mode 100644 index 0000000..d4614f4 --- /dev/null +++ b/packages/mcp/tests/tools/content-gate.test.ts @@ -0,0 +1,196 @@ +import { describe, expect, it, beforeEach, afterEach, vi } from 'vitest' + +vi.setConfig({ testTimeout: 120000, hookTimeout: 120000 }) +import { join } from 'node:path' +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { simpleGit } from 'simple-git' +import { Client } from '@modelcontextprotocol/sdk/client/index.js' +import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js' +import { createServer } from '../../src/server.js' +import { readJson } from '../../src/util/fs.js' + +/** + * The write gate. content_save used to run plan → commit → validate → report: + * an invalid value landed in git, was auto-merged, and the caller learned about + * it from a string in `next_steps` while `status` still said "committed". + * + * These assert on the repository, not on the response — a gate that only claims + * to have blocked is the bug we are fixing. + */ + +let testDir: string +let client: Client + +async function initProject(dir: string): Promise { + const git = simpleGit(dir) + await git.init() + await git.addConfig('user.name', 'Test') + await git.addConfig('user.email', 'test@test.com') + await writeFile(join(dir, '.gitkeep'), '') + await git.add('.') + await git.commit('initial') +} + +async function createTestClient(projectRoot: string): Promise { + const server = createServer(projectRoot) + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair() + const c = new Client({ name: 'test-client', version: '1.0.0' }) + await Promise.all([c.connect(clientTransport), server.connect(serverTransport)]) + return c +} + +function parseResult(result: unknown): Record { + const content = (result as { content: Array<{ text: string }> }).content + return JSON.parse(content[0]!.text) as Record +} + +async function createModel(id: string, fields: Record): Promise { + await client.callTool({ + name: 'contentrain_model_save', + arguments: { id, name: id, kind: 'collection', domain: 'blog', i18n: true, fields }, + }) + client = await createTestClient(testDir) +} + +function contentFile(model: string, locale: string): Promise | null> { + return readJson(join(testDir, '.contentrain', 'content', 'blog', model, `${locale}.json`)) +} + +async function branches(): Promise { + return (await simpleGit(testDir).branchLocal()).all +} + +beforeEach(async () => { + testDir = await mkdtemp(join(tmpdir(), 'cr-gate-test-')) + await initProject(testDir) + client = await createTestClient(testDir) + await client.callTool({ name: 'contentrain_init', arguments: { locales: ['en'] } }) + client = await createTestClient(testDir) +}) + +afterEach(async () => { + await rm(testDir, { recursive: true, force: true }) +}) + +describe('contentrain_content_save write gate', () => { + it('writes nothing when a value is invalid', async () => { + await createModel('posts', { title: { type: 'string', required: true }, slug: { type: 'slug' } }) + const before = await branches() + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { + model: 'posts', + entries: [{ locale: 'en', data: { title: 'Hi', slug: 'Not A Slug!!' } }], + }, + }) + + const data = parseResult(result) + expect(data['status']).not.toBe('committed') + expect(data['error']).toContain('nothing was written') + + // The repository is the witness: no content, no new branch. + const content = await contentFile('posts', 'en') + expect(content ?? {}).toEqual({}) + expect(await branches()).toEqual(before) + }) + + it('names the offending field so the caller can fix it', async () => { + await createModel('posts', { title: { type: 'string', required: true }, slug: { type: 'slug' } }) + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { + model: 'posts', + entries: [{ locale: 'en', data: { title: 'Hi', slug: 'Not A Slug!!' } }], + }, + }) + + const issues = parseResult(result)['issues'] as Array> + expect(issues.some(i => i['field'] === 'slug')).toBe(true) + }) + + it('commits a valid entry', async () => { + await createModel('posts', { title: { type: 'string', required: true }, slug: { type: 'slug' } }) + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { + model: 'posts', + entries: [{ locale: 'en', data: { title: 'Hi', slug: 'a-real-slug' } }], + }, + }) + + expect(parseResult(result)['status']).toBe('committed') + expect(Object.keys(await contentFile('posts', 'en') ?? {})).toHaveLength(1) + }) + + it('lets a warning through and reports it', async () => { + // Heuristics must not block — a legitimate value can sit outside the pattern. + await createModel('people', { name: { type: 'string' }, contact: { type: 'email' } }) + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { + model: 'people', + entries: [{ locale: 'en', data: { name: 'Ada', contact: 'not-an-email' } }], + }, + }) + + const data = parseResult(result) + expect(data['status']).toBe('committed') + expect((data['warnings'] as string[]).some(w => /email/.test(w))).toBe(true) + expect(Object.keys(await contentFile('people', 'en') ?? {})).toHaveLength(1) + }) + + it('refuses to commit a secret', async () => { + // Falls out of the gate rather than being a special case: detectSecrets is an + // error, and errors no longer reach git. Pinned because it is the sharpest + // consequence of moving validation ahead of the commit. + await createModel('settings', { name: { type: 'string' }, token: { type: 'string' } }) + const before = await branches() + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { + model: 'settings', + entries: [{ locale: 'en', data: { name: 'API', token: 'sk_live_abc123secret' } }], + }, + }) + + expect(parseResult(result)['error']).toContain('nothing was written') + expect(await contentFile('settings', 'en') ?? {}).toEqual({}) + expect(await branches()).toEqual(before) + }) + + it('does not block a valid save because another entry is already broken', async () => { + // A pre-existing bad entry elsewhere in the model must not hold the caller + // hostage — they may not even be able to fix it. + await createModel('posts', { title: { type: 'string', required: true }, slug: { type: 'slug' } }) + + const first = await client.callTool({ + name: 'contentrain_content_save', + arguments: { model: 'posts', entries: [{ locale: 'en', data: { title: 'Good', slug: 'ok-one' } }] }, + }) + const goodId = (parseResult(first)['results'] as Array>)[0]!['id'] as string + + // Corrupt it behind MCP's back, the way a hand-edit or an older version would. + const path = join(testDir, '.contentrain', 'content', 'blog', 'posts', 'en.json') + const { writeJson } = await import('../../src/util/fs.js') + const existing = await readJson>(path) ?? {} + existing[goodId] = { title: 'Good', slug: 'BROKEN SLUG!!' } + await writeJson(path, existing) + const git = simpleGit(testDir) + await git.add('.') + await git.commit('corrupt entry') + client = await createTestClient(testDir) + + const result = await client.callTool({ + name: 'contentrain_content_save', + arguments: { model: 'posts', entries: [{ locale: 'en', data: { title: 'New', slug: 'ok-two' } }] }, + }) + + expect(parseResult(result)['status']).toBe('committed') + }) +}) diff --git a/packages/mcp/tests/tools/workflow.test.ts b/packages/mcp/tests/tools/workflow.test.ts index 15244da..f8e8b43 100644 --- a/packages/mcp/tests/tools/workflow.test.ts +++ b/packages/mcp/tests/tools/workflow.test.ts @@ -89,19 +89,34 @@ describe('contentrain_validate', () => { expect(branches.all).toContain('contentrain') }) + /** + * Write content straight to the working tree, bypassing content_save. + * + * content_save now refuses to commit content with validation errors, so the + * only way an invalid entry exists is if it predates the gate or was + * hand-edited. These fixtures reproduce exactly that: the gate stops *new* + * problems, it does not retroactively clean a repository, so + * `contentrain_validate` must still report the old ones. + */ + async function plantContent( + domain: string, + model: string, + perLocale: Record>, + ): Promise { + for (const [locale, data] of Object.entries(perLocale)) { + await writeJson(join(testDir, '.contentrain', 'content', domain, model, `${locale}.json`), data) + } + } + it('detects required field missing', async () => { client = await createModel(client, 'authors', 'collection', 'blog', { name: { type: 'string', required: true }, bio: { type: 'text' }, }) - // Save content with missing required field - await client.callTool({ - name: 'contentrain_content_save', - arguments: { - model: 'authors', - entries: [{ id: 'author-001', locale: 'en', data: { bio: 'A developer' } }], - }, + await plantContent('blog', 'authors', { + en: { 'author-001': { bio: 'A developer' } }, + tr: { 'author-001': { bio: 'A developer' } }, }) client = await createClient(testDir) @@ -134,13 +149,9 @@ describe('contentrain_validate', () => { category: { type: 'relation', model: 'categories' }, }) - // Save post with nonexistent category reference - await client.callTool({ - name: 'contentrain_content_save', - arguments: { - model: 'posts', - entries: [{ id: 'post-001', locale: 'en', data: { title: 'My Post', category: 'nonexistent-cat' } }], - }, + await plantContent('blog', 'posts', { + en: { 'post-001': { title: 'My Post', category: 'nonexistent-cat' } }, + tr: { 'post-001': { title: 'My Post', category: 'nonexistent-cat' } }, }) client = await createClient(testDir) @@ -197,15 +208,10 @@ describe('contentrain_validate', () => { token: { type: 'string' }, }) - await client.callTool({ - name: 'contentrain_content_save', - arguments: { - model: 'settings', - entries: [ - { locale: 'en', data: { api_endpoint: 'https://api.example.com', token: 'sk_live_abc123secret' } }, - { locale: 'tr', data: { api_endpoint: 'https://api.example.com', token: 'sk_live_abc123secret' } }, - ], - }, + // A singleton stores one flat object per locale — no entry-id keying. + await plantContent('system', 'settings', { + en: { api_endpoint: 'https://api.example.com', token: 'sk_live_abc123secret' }, + tr: { api_endpoint: 'https://api.example.com', token: 'sk_live_abc123secret' }, }) client = await createClient(testDir) @@ -230,15 +236,9 @@ describe('contentrain_validate', () => { email: { type: 'string', required: true, pattern: '^[^@]+@example\\\\.com$' }, }) - await client.callTool({ - name: 'contentrain_content_save', - arguments: { - model: 'authors', - entries: [ - { id: 'author-001', locale: 'en', data: { email: 'alice@outside.com' } }, - { id: 'author-001', locale: 'tr', data: { email: 'alice@outside.com' } }, - ], - }, + await plantContent('blog', 'authors', { + en: { 'author-001': { email: 'alice@outside.com' } }, + tr: { 'author-001': { email: 'alice@outside.com' } }, }) client = await createClient(testDir) @@ -505,17 +505,25 @@ describe('contentrain_validate', () => { }, }) + // content_save now refuses to write an entry with a missing required nested + // field, so the invalid state has to be planted the way a legacy project or a + // hand-edit would produce it: valid save first, then corrupt the file. await client.callTool({ name: 'contentrain_content_save', arguments: { model: 'pages', entries: [ - { id: 'page-a', locale: 'en', data: { seo: {} } }, - { id: 'page-a', locale: 'tr', data: { seo: {} } }, + { id: 'page-a', locale: 'en', data: { seo: { title: 'Fine' } } }, + { id: 'page-a', locale: 'tr', data: { seo: { title: 'Fine' } } }, ], }, }) + for (const locale of ['en', 'tr']) { + const path = join(testDir, '.contentrain', 'content', 'marketing', 'pages', `${locale}.json`) + await writeJson(path, { 'page-a': { seo: {} } }) + } + client = await createClient(testDir) const result = await client.callTool({ @@ -525,8 +533,10 @@ describe('contentrain_validate', () => { const data = parseResult(result) const issues = data['issues'] as Array> + // The path is qualified by its parent — a bare `title` would be ambiguous + // with a top-level field of the same name. const nestedError = issues.find(i => - i['severity'] === 'error' && i['field'] === 'title', + i['severity'] === 'error' && i['field'] === 'seo.title', ) expect(nestedError).toBeDefined() }) diff --git a/packages/rules/essential/contentrain-essentials.md b/packages/rules/essential/contentrain-essentials.md index 5c6d1d2..02a1e9e 100644 --- a/packages/rules/essential/contentrain-essentials.md +++ b/packages/rules/essential/contentrain-essentials.md @@ -34,6 +34,8 @@ MCP is **deterministic infrastructure**. The agent (you) is the **intelligence l - **NEVER** include system fields in content data: `id`, `status`, `source`, `updated_by`, `updated_at`, `createdAt`, `updatedAt` - **ALWAYS** use MCP tools — do not write `.contentrain/` JSON files directly - **Publishing is yours, not MCP's.** `contentrain_content_save` preserves an existing entry's `status`; only a brand-new entry starts at `draft`. To change publish state, call `contentrain_bulk update_status` deliberately — and note that on the CDN a collection entry is delivered only when its status is `published`. +- **Schema validity is MCP's.** `contentrain_content_save` validates before it writes and **refuses** the save on any error — nothing is committed, so there is no branch to clean up. Fix the values and call it again. Warnings (email/url/colour/phone heuristics, `accept` extension checks) do not block; they come back in the response. +- **A declared constraint is a real constraint.** `model_save` rejects a property declared where it cannot apply (`options` on a non-select, `accept` on a non-media field, `unique` on a singleton) rather than accepting it and doing nothing. The one exception is `maxSize`, which MCP cannot enforce — your media provider does, at ingest, and `model_save` says so. ## MCP Tools diff --git a/packages/rules/shared/media-rules.md b/packages/rules/shared/media-rules.md index e4423ac..11178f8 100644 --- a/packages/rules/shared/media-rules.md +++ b/packages/rules/shared/media-rules.md @@ -73,6 +73,12 @@ Never hand-build delivery URLs or guess the CDN base. Save the path or URL the m 3. If an image exceeds the size limit, instruct the user to compress or resize it before storing. Do not silently accept oversized files. + **This one is on you.** A `maxSize` on the field does not enforce it: MCP stores a + path and never sees the bytes, so it cannot check the size — your media provider + does, when the asset is ingested. `model_save` says as much when you declare + `maxSize`. A field's `accept` is checked, but only against the file **extension**, + which can lie. + 4. SVG files SHOULD be optimized (e.g., with SVGO) to remove editor metadata, comments, and unnecessary attributes. --- diff --git a/packages/rules/shared/schema-rules.md b/packages/rules/shared/schema-rules.md index 60e1986..269898f 100644 --- a/packages/rules/shared/schema-rules.md +++ b/packages/rules/shared/schema-rules.md @@ -92,22 +92,25 @@ A field definition describes one field in a model. Include only the properties t ### 3.1 Field Properties -| Property | Applicable Types | Description | -|----------|-----------------|-------------| -| `type` | ALL | **Required.** One of the 27 types. | -| `required` | ALL | Mark field as mandatory. Default: `false`. Omit if `false`. | -| `unique` | `string`, `email`, `slug`, `integer` | Enforce uniqueness within model. Default: `false`. Omit if `false`. | -| `default` | ALL | Default value. Omit if `null`. | -| `min` | string/text: char count; numbers: value; array: element count | Minimum constraint. | -| `max` | Same as `min` | Maximum constraint. | -| `pattern` | `string`, `text`, `code` | Regex validation pattern. | -| `options` | `select` ONLY | Fixed choices: `["draft", "published", "archived"]` | -| `model` | `relation`, `relations` ONLY | Target model ID. String or string array for polymorphic. | -| `items` | `array` ONLY | Element type: `"string"` or `{ "type": "object", "fields": {...} }` | -| `fields` | `object` ONLY | Nested field definitions. | -| `accept` | `image`, `video`, `file` | Allowed MIME types: `"image/png,image/jpeg"` | -| `maxSize` | `image`, `video`, `file` | Maximum file size in bytes. | -| `description` | ALL | Human-readable hint (shown in Studio UI tooltip, used as agent context). | +A property declared where it does not apply is **rejected by `model_save`** — it is +not silently ignored. The "Enforced by" column says who actually checks each one. + +| Property | Applicable Types | Enforced by | Description | +|----------|-----------------|-------------|-------------| +| `type` | ALL | content_save (blocks) | **Required.** One of the 27 types. | +| `required` | ALL | content_save (blocks) | Mark field as mandatory. Default: `false`. Omit if `false`. | +| `unique` | Any type, on `collection` and `document` models. **Not `singleton`** — it holds one record per locale, so there is nothing to be unique against. | content_save (blocks) | Enforce uniqueness within the model, per locale. Default: `false`. Omit if `false`. | +| `default` | ALL | model_save (coherence only) | Default value. Checked against its own `type` and `options`; **not** written into content — the agent supplies values. Omit if `null`. | +| `min` | string/text: char count; numbers: value; array: element count | content_save (blocks) | Minimum constraint. Must not exceed `max`. | +| `max` | Same as `min` | content_save (blocks) | Maximum constraint. On a media field this limits the **path string length**, not the file — use `maxSize` for bytes (model_save warns). | +| `pattern` | Any string-shaped type | content_save (blocks) | Regex validation pattern. Compiled at model_save, so a broken regex is rejected rather than silently disabling the rule. | +| `options` | `select` ONLY | content_save (blocks) | Fixed choices: `["draft", "published", "archived"]` | +| `model` | `relation`, `relations` ONLY | content_save (blocks) | Target model ID. String or string array for polymorphic. | +| `items` | `array` ONLY | content_save (blocks) | Element type: `"string"`, or a full FieldDef — `{ "type": "string", "max": 50 }`. Items are validated by the same rules as a scalar of that type. | +| `fields` | `object` ONLY | content_save (blocks) | Nested field definitions. Validated recursively, at model_save and at content_save. | +| `accept` | `image`, `video`, `file` | content_save (**warns**) | Allowed MIME types: `"image/png,image/jpeg"`, `"image/*"`, `".pdf"`. MCP checks the value's **file extension** — an approximation, since it holds a path, not the bytes. Your provider enforces the real MIME type at ingest. | +| `maxSize` | `image`, `video`, `file` | **Provider, at ingest — not MCP** | Maximum file size in bytes. MCP never sees the file, so it cannot check this; model_save says so. | +| `description` | ALL | — | Human-readable hint (shown in Studio UI tooltip, used as agent context). | ### 3.2 Omission Rules diff --git a/packages/types/src/index.ts b/packages/types/src/index.ts index 61cbeba..428af1a 100644 --- a/packages/types/src/index.ts +++ b/packages/types/src/index.ts @@ -564,10 +564,138 @@ function checkMinMax(value: unknown, fieldDef: FieldDef): string | null { return null } +// ─── Semantic type validation ─── + +/** Hex (#rgb/#rrggbb/#rrggbbaa), rgb()/rgba(), hsl()/hsla(), or a bare CSS keyword. */ +const COLOR_PATTERN = /^(#(?:[0-9a-fA-F]{3,4}|[0-9a-fA-F]{6}|[0-9a-fA-F]{8})|(?:rgb|hsl)a?\([^)]*\)|[a-zA-Z]+)$/ +/** Deliberately loose: digits, spaces, and the usual separators, 4-20 digits total. */ +const PHONE_PATTERN = /^\+?[\d\s().-]{4,}$/ +const EMAIL_PATTERN = /^[^\s@]+@[^\s@]+\.[^\s@]+$/ +const DATE_PATTERN = /^\d{4}-\d{2}-\d{2}$/ + +function isParsableDate(value: string): boolean { + return !Number.isNaN(new Date(value).getTime()) +} + +/** + * Rules a field type implies beyond its runtime `typeof`. + * + * Severity follows one rule: a check that is *definitional* is an error — a `slug` + * that does not match `SLUG_PATTERN` is not a slug. A check that is a *heuristic* is + * a warning, because the pattern is an approximation and a legitimate value may sit + * outside it (international phone formats, CSS colour keywords). + * + * `rating` is deliberately absent: its scale is never declared, so any range here + * would be invented. Declare `min`/`max` on the field instead. + * + * Returns `[]` for types whose only contract is their `typeof` (`string`, `text`, + * `code`, `icon`, `markdown`, `richtext`, `number`, `decimal`, `boolean`, media). + */ +export function validateSemanticType(value: unknown, type: FieldType): ValidationError[] { + if (typeof value === 'number') { + if (type === 'integer' && !Number.isInteger(value)) { + return [{ severity: 'error', message: `Value ${value} is not an integer` }] + } + if (type === 'percent' && (value < 0 || value > 100)) { + return [{ severity: 'error', message: `Percent ${value} is outside 0-100` }] + } + return [] + } + + if (typeof value !== 'string' || value === '') return [] + + switch (type) { + case 'slug': { + const err = validateSlug(value) + return err ? [{ severity: 'error', message: err }] : [] + } + case 'date': + return DATE_PATTERN.test(value) && isParsableDate(value) + ? [] + : [{ severity: 'error', message: `Invalid date "${value}": must be YYYY-MM-DD` }] + case 'datetime': + return isParsableDate(value) + ? [] + : [{ severity: 'error', message: `Invalid datetime "${value}": must be an ISO 8601 date-time` }] + case 'email': + return EMAIL_PATTERN.test(value) + ? [] + : [{ severity: 'warning', message: `"${value}" may not be a valid email` }] + case 'url': + return /^https?:\/\/.+/.test(value) || value.startsWith('/') + ? [] + : [{ severity: 'warning', message: `"${value}" may not be a valid URL` }] + case 'color': + return COLOR_PATTERN.test(value) + ? [] + : [{ severity: 'warning', message: `"${value}" may not be a valid color (expected hex, rgb(), hsl() or a keyword)` }] + case 'phone': + return PHONE_PATTERN.test(value) && (value.match(/\d/g)?.length ?? 0) >= 4 + ? [] + : [{ severity: 'warning', message: `"${value}" may not be a valid phone number` }] + default: + return [] + } +} + +/** Media field types — the ones whose value is a path/URL to an asset. */ +const MEDIA_TYPES = new Set(['image', 'video', 'file']) + +export function isMediaType(type: FieldType): boolean { + return MEDIA_TYPES.has(type) +} + +/** Extensions we can name a MIME type for. Deliberately small — this is a sniff. */ +const EXTENSION_MIME: Record = { + jpg: 'image/jpeg', jpeg: 'image/jpeg', png: 'image/png', gif: 'image/gif', + webp: 'image/webp', avif: 'image/avif', svg: 'image/svg+xml', ico: 'image/x-icon', + mp4: 'video/mp4', webm: 'video/webm', mov: 'video/quicktime', + pdf: 'application/pdf', zip: 'application/zip', json: 'application/json', + csv: 'text/csv', txt: 'text/plain', +} + +/** + * Check a media value's file extension against an `accept` list. + * + * This is an approximation and says so: the value is a path or URL, so the real + * MIME type is only knowable where the bytes are — the provider, at ingest + * (`provider.ts:MediaProvider.ingest` owns that policy). An extension can lie, and + * an unknown extension is not evidence of a violation. Hence: warning, never error, + * and silence when we cannot tell. + * + * `accept` follows the HTML input syntax: `image/jpeg`, `image/*`, `.jpg`, comma-separated. + */ +export function validateAccept(value: string, accept: string): ValidationError[] { + const path = value.split('?')[0]?.split('#')[0] ?? value + const ext = path.split('.').pop()?.toLowerCase() + if (!ext || ext === path.toLowerCase()) return [] + + const mime = EXTENSION_MIME[ext] + const patterns = accept.split(',').map(a => a.trim().toLowerCase()).filter(Boolean) + if (patterns.length === 0) return [] + + const matches = patterns.some((p) => { + if (p.startsWith('.')) return p.slice(1) === ext + if (p.endsWith('/*')) return mime ? mime.startsWith(p.slice(0, -1)) : false + return mime === p + }) + if (matches) return [] + + // An extension we have no MIME for is not evidence of a violation. + if (!mime && !patterns.some(p => p.startsWith('.'))) return [] + + return [{ + severity: 'warning', + message: `File extension ".${ext}" does not match accept "${accept}" (extension check — the provider enforces the real MIME type at ingest)`, + }] +} + /** * Validate a field value against its FieldDef schema. - * Checks: required, type match, min/max, pattern, select options. - * Does NOT check: secrets (use detectSecrets), uniqueness (stateful), relations (I/O). + * Checks: required, type match, semantic type rules, min/max, pattern, select + * options, and `accept` on media fields (by extension). + * Does NOT check: secrets (use detectSecrets), uniqueness (stateful), relations (I/O), + * or `maxSize` — byte length is not knowable from a stored path. */ export function validateFieldValue(value: unknown, fieldDef: FieldDef): ValidationError[] { const issues: ValidationError[] = [] @@ -584,6 +712,12 @@ export function validateFieldValue(value: unknown, fieldDef: FieldDef): Validati return issues } + issues.push(...validateSemanticType(value, fieldDef.type)) + + if (fieldDef.accept && isMediaType(fieldDef.type) && typeof value === 'string') { + issues.push(...validateAccept(value, fieldDef.accept)) + } + const minMaxErr = checkMinMax(value, fieldDef) if (minMaxErr) { issues.push({ severity: 'error', message: minMaxErr })