feat(mcp)!: enforce the field constraints the schema already accepted#84
Merged
Conversation
Only 4 of the 27 field types had any validation beyond `typeof`: email, url, select and relation. The other 17 were indistinguishable from `string` at runtime — a `slug` field accepted "Hello World!!", a `date` field accepted "yesterday", and `integer` accepted 3.7. The types existed for the Studio UI and the emitted TS, and stopped meaning anything after that. New `validateSemanticType(value, type)`, called from validateFieldValue after the type match. It reuses what the codebase already owned rather than inventing rules: `validateSlug` (with its path-traversal check) for slug, and the same date parse `validator/schedule.ts` has always done for publish_at/expire_at. Severity follows one rule. A *definitional* check is an error — a slug that does not match SLUG_PATTERN is not a slug, an unparseable date is not a date. A *heuristic* is a warning, because the pattern is an approximation and a real value can sit outside it (international phone formats, CSS colour keywords). email/url keep the warning severity they already had; the tests pin it on purpose. `rating` is deliberately left alone — its scale is never declared, so any range would be invented. Also `validateAccept` + `isMediaType`: `accept` was declared on FieldDef and read by nothing. It is now checked against the value's file extension, at warning severity, and the message says that is what it is — the stored value is a path, so the real MIME type is only knowable at the provider's ingest. An extension we cannot map to a MIME type is not evidence of a violation, so it stays silent rather than guessing. `maxSize` is untouched here on purpose: byte length is not knowable from a path. Tests: 110 in packages/types pass; the rules are exercised through @contentrain/mcp's validateContent fixtures, where stubbing validateSemanticType to a no-op turns 10 red.
The reported case — `emails: ["not-an-email"]` producing zero errors — was one
symptom of a parallel implementation. `validateArrayItemType` was a second type
switch that knew 10 of the 27 types and checked only `typeof`, so:
- items never saw min/max/pattern/options/required — only their JS kind;
- `items` given as a FieldDef with a non-object type (`{type:'array',
items:{type:'string', max:50}}`) matched neither the string-form branch nor
the object+fields branch, so it was validated by nothing at all — while the
type emitter rendered the constraint as real;
- `integer` rejected 3.7 inside an array but accepted it as a scalar, because
the two switches disagreed.
Items now recurse through `validateField` itself, which already knows every
type, relations, and nested objects. One rule set for a type wherever it
appears, and the second switch is deleted. `ctx` is deliberately not passed to
items: `unique` compares an entry against its siblings, which means nothing for
positions inside one array. A depth cap bounds items-inside-items.
Two deliberate contract changes:
- Item type errors now carry validateFieldValue's message ("Type mismatch:
expected string, got number") rather than "must be a string". The field path
still names the index.
- Nested object errors are qualified by their parent — `seo.title`, not `title`,
which was ambiguous with a top-level field of the same name. The old
array-of-object branch already did this; unifying spread it to plain objects
too, and dropping the prefix would have been a regression the fixtures caught.
email/url heuristics move to @contentrain/types with the rest of the semantic
rules, so scalars and items share them.
Tests: 39 in entry.test.ts. New cases cover the reported bug, a type the old
switch never knew (date), the items-FieldDef black hole, options on items, and
the depth cap. entry.test.ts:171,201 updated for the two changes above.
`unique` is gated on a validation context carrying the entry's siblings, and
only the collection validator ever passed one. Documents were validated one file
at a time, so the check silently did nothing — which is exactly where it is most
often declared: every shipped template writes
`slug: { type: 'slug', required: true, unique: true }` on a document model
(templates/blog.ts, docs.ts, ecommerce.ts, saas.ts).
validateDocumentModel now reads each document once up front, keeps the
frontmatter per locale, and validates with `{ allEntries, currentEntryId: slug }`.
The pre-pass replaces the read inside the loop rather than adding one, so a
remote provider still pays one fetch per file.
Uniqueness stays scoped to a locale: the same value in en and tr is a
translation, not a collision.
Nested objects now receive `ctx` too (entry.ts recursion dropped it), so
`unique` inside `object.fields` is checked rather than quietly skipped.
Singletons are handled in the model_save commit that follows: a singleton holds
one record per locale, so `unique` has nothing to compare against and is
rejected at schema level rather than given a fake implementation here.
Tests: tests/core/validator/unique.test.ts (4) — duplicate detection, distinct
values, self-comparison, and locale scoping. Removing the ctx turns the first red.
`accept`, `maxSize` and `default` were declared on FieldDef, accepted by the zod
schema, stored in the model — and read by nothing. `options` on a non-select was
accepted and then ignored at validation time, because the refine only checked
select ⇒ options and never the reverse. A constraint that silently does nothing
is worse than no constraint: the author stops looking.
model_save now rejects a property declared where it cannot apply — `options` off
select, `items` off array, `fields` off object, `accept`/`maxSize` off media,
`unique` on a singleton (one record per locale, so nothing to compare against).
Plus `min > max`, and a `pattern` that does not compile: left to validation time
that degraded to a per-entry warning and silently disabled the rule.
The field schema is now `.strict()`. z.object *strips* unknown keys by default,
so a typo'd `requird: true` vanished without a word and the field quietly lost
the constraint its author thought they had declared.
Nested `fields`/`items` are validated recursively. They were typed `z.unknown()`
and the loop was top-level only, so a nested invalid type, a nested select
without options, or a nested bad field name all sailed through.
`default` is checked for coherence — right type for its field, and within its
own `options`. It is not written into content: supplying values is the agent's
job, and quietly materialising them would be MCP making a content decision.
Two things MCP cannot enforce now say so instead of hiding, via a new warnings
channel (`validateModelDefinition` returns `{errors, warnings}`; model_save
surfaces `schema_warnings`):
- `maxSize` — MCP stores a path and never sees the bytes. The provider enforces
it at ingest; provider.ts already assigns it that duty.
- `max` on a media field — it limits the *path string length*, not the file,
which is almost certainly not what the author meant.
Tests: tests/core/model-definition.test.ts (23) — validateModelDefinition had no
tests at all. Templates and the model tool still pass unchanged.
content_save ran plan → commit → validate → report. An invalid value was written to git and auto-merged, and the caller learned about it from a string in next_steps while `status` still said "committed". Nothing on the write path ever consulted model.fields before the bytes landed, so required fields, patterns, select options and secrets were all advisory after the fact. Validation now runs on the pending changes and blocks. The move itself is mechanical — the existing call already used an OverlayReader over `plan.changes`, which needs nothing from the commit — but two things had to be decided: **Only this save's entries are fatal.** validateProject checks the whole model, so blocking on every error would let one pre-existing bad entry hold up an unrelated, valid save by a caller who may not be able to fix it. `touchesSavedEntries` filters to the entries in the call — by id for collections, slug for documents, locale for singletons and dictionaries. Everything else is reported, not enforced. **Warnings pass.** email/url/colour/phone patterns and the `accept` extension check are approximations; a legitimate value can sit outside one. They come back in the response as `warnings` instead of failing the write. The sharpest consequence is one that falls out rather than being designed: a secret can no longer reach git. detectSecrets is an error, and errors no longer commit. Four contentrain_validate fixtures broke, and the break was the signal: each planted invalid content *through content_save* to prove validate reports it, and that is exactly what the gate now prevents. They plant it on disk instead — the way a legacy project or a hand-edit leaves it — which is the case that still matters: the gate stops new problems, it does not clean a repository. Tests: tests/tools/content-gate.test.ts (6). They assert on the repository, not the response — a gate that only *claims* to have blocked is the bug being fixed — so they check the content file is empty and no branch was created. Also covered: a valid save still commits, a warning still commits, a secret does not, and a broken sibling entry does not block a good save.
schema-rules.md listed accept, maxSize and default as working, and media-rules.md told agents 'do not silently accept oversized files' — a rule with no machine backing. The table now names who enforces each property, and says plainly that maxSize is the provider's job at ingest and that accept is an extension check. Essentials and the MCP docs page gain the write-gate contract.
✅ Deploy Preview for contentrain-ai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A project reported that
items,acceptandmaxSizeare accepted on a field but never enforced —emails: ["not-an-email"]andaccept: "image/jpeg"against a.webpboth produced zero errors.The report was right. It was also the visible tip: 4 of 27 field types had any validation beyond
typeof, three constraints were read by nothing,uniquedidn't work where the templates declare it, and none of it blocked a write anyway.Same through-line as #82: the tool accepts input, reports success, and silently does not do what it claims. A constraint that isn't a constraint is worse than no constraint — the author stops looking.
Studio delegates to this validator, so it picks all of this up for free.
What was wrong
plan → commit → validate → report. An invalid value landed in git, was auto-merged, and the caller learned about it from a string innext_stepswhilestatussaid"committed".bulkimported no validator at all.typeofcheckstype: 'slug'ignored theSLUG_PATTERNwe already own — and every template declaresslug: { type: 'slug' }, so"Hello World!!"validated clean.date/datetimeunparsed, thoughschedule.tsalready did exactly that parse for meta.typeofonly. Items never sawmin/max/pattern/options.itemsas a FieldDef with a non-object type ({type:'array', items:{type:'string', max:50}}) matched no branch at all — while the type emitter rendered it as real.integerrejected3.7in an array but accepted it as a scalar.uniquewas a no-op on documentsaccept/maxSize/defaultread by nothingschema-rules.md.media-rules.mdeven told agents "do not silently accept oversized files" — a rule with no machine backing.optionson a non-select silently ignoredz.objectstrips unknown keys, so a typo'drequird: truevanished without a word.What changed
The write gate. Validation runs on the pending changes and refuses to commit on any error —
isError, no branch, nothing to clean up. Two calls had to be made:validateProjectchecks the whole model; blocking on everything would let one pre-existing bad entry hold up an unrelated valid save by someone who may not be able to fix it. Scoped by id/slug/locale to the entries in the call.The sharpest consequence wasn't designed, it fell out: a secret can no longer reach git.
One rule set per type. Array items recurse through
validateFielditself instead of a parallel switch — closing the 10-of-27 hole, the items-FieldDef black hole, the missing constraints on items, and theintegersplit, while deleting code.Semantic types, reusing what we own (
validateSlug,schedule.ts's date parse). Severity follows one rule: definitional → error (slug,date,datetime,integer,percent); heuristic → warning (color,phone).email/urlkeep their existing warning — the tests pin it deliberately.ratingis left alone: its scale is never declared, so any range would be invented.The dead constraints, honestly.
acceptis enforced by extension-sniff and the message says that's what it is.defaultis coherence-checked at model_save but not written into content — supplying values is the agent's job.maxSizecannot be enforced by MCP (it holds a path, never bytes; LocalProvider has no media facet;MediaProvider.getkeys on id, not path), so model_save says so and points at the provider. The docs are corrected.model_save rejects what it won't enforce — including
uniqueon a singleton, which holds one record per locale and so has nothing to compare against. Nestedfields/itemsare validated recursively; they werez.unknown()and unchecked.Tests
72 new across 4 files.
validateModelDefinitionand the CDN document query had no tests at all.Each fix was verified by reverting it and watching the tests go red — stubbing
validateSemanticTypeto a no-op turns 10 red, including both of the reporter's cases. The gate tests assert on the repository, not the response — a gate that only claims to have blocked is the bug being fixed.Two fixtures broke, and the break was the signal. Four
contentrain_validatetests planted invalid content through content_save to prove validate reports it — precisely what the gate now prevents. They plant it on disk instead, which is the case that still matters: the gate stops new problems, it does not clean a repository.One regression I introduced, caught by an existing fixture: unifying items dropped the
blocks[0].titlepath prefix. Fixed, and taken one step further — top-level object errors are now qualified too (seo.title), where a baretitlewas ambiguous with a real top-level field.Green: MCP validator 51, model-definition 23, gate 6, workflow 15, content/apply/normalize 51, bulk/server/conformance 39, types 110, SDK 260 + type tests. oxlint 0/0 across 172 files,
tscclean in mcp/types/cli/sdk.Breaking
@contentrain/mcpmajor,@contentrain/typesminor.content_saverejects content it previously committed. Runcontentrain_validatebefore upgrading to see what would now be blocked. Blast radius is limited to the entries in each call.model_saverejects models it previously accepted (unknown keys,min > max,optionson a non-select,uniqueon a singleton).validateModelDefinitionreturns{ errors, warnings }instead ofstring[].validateFieldValue's message; the field path is unchanged.One consequence worth naming: a broken relation now blocks the save, so a target must exist before the entry referencing it. Within a single
content_savecall the overlay resolves them, so batches are fine; across calls it means ordering. That follows from relation integrity already being an error invalidateProject— flagging it because it's a real workflow change, not a bug.🤖 Generated with Claude Code