fix(mcp): stop publish status silently dropping content from the CDN#82
Merged
Conversation
content_save rebuilt an entry's meta from scratch on every write, so
editing a single field silently moved a `published` entry to `draft` —
and the next CDN build served the whole collection as `{}`. Found on a
live project: five published guides became two bytes of JSON, and
contentrain_validate reported zero errors throughout.
The sharpest tell was that the code already knew: content-save.ts
computed `action: 'updated'` for a pre-existing entry, then overwrote its
meta with defaultMeta() anyway — the distinction only ever reached the
response payload. Resetting status is a publish decision, and per this
repo's own split (MCP is deterministic infra, the agent is intelligence)
MCP should never have been making it.
- New `mergeEntryMeta(existing, data)` in meta-manager.ts. An existing
entry keeps `status`, `approved_by` and `version`; only an entry with
no prior meta starts at `draft`. `source`/`updated_by` describe this
write, so they are still stamped.
- All four kinds now read prior meta. Only the collection branch did
before — and even it discarded the entry's own record while preserving
its siblings.
- The same reset lived in a byte-equivalent second copy in
content-manager.ts, reached via contentrain_apply and scaffolding.
Both call sites now share the one helper, so a fix here cannot drift.
Tests: tests/core/content-save-meta.test.ts (10, plan layer, no git) and
5 new cases in tests/core/content.test.ts covering all four kinds,
sibling isolation, approved_by/version survival, and publish_at still
applying. Reverting mergeEntryMeta to a hardcoded draft turns 5 of them
red. Existing create-path assertions are untouched — they always
asserted draft on create, which stays correct.
update_status reported `updated: 10` while committing a two-line diff.
Five entry_ids across two locales persisted one status change per locale;
the other four were silently dropped, and the tool still returned
`status: "committed"`. copy_locale had the identical defect, writing one
meta record instead of N while reporting `copied: N`.
Root cause was `await Promise.all(updateOps)` over per-entry writeMeta
calls. metaPath resolves to the same `{locale}.json` for every entry ID,
and writeMeta is itself a read-modify-write of that whole file — so all N
calls read the same snapshot and rewrote the file wholesale. Last to
settle won. A plain sequential await loop would have been correct; the
Promise.all is precisely what broke it.
That matters beyond this call site: CLAUDE.md's quality gate says
"no-await-in-loop -> use Promise.all() for parallel I/O", and this is that
rule applied mechanically to shared-file writes. The rule is now qualified
— reads and distinct paths only, never a read-modify-write over shared
state — because otherwise the next author reintroduces this.
- New `writeMetaEntries()` does one read-modify-write per locale file and
returns the IDs actually written, so the race cannot be reintroduced by
looping writeMeta. Both operations use it.
- `updated` is now counted from what persisted, never from entry_ids.length,
and a run that matches nothing fails loudly instead of committing empty.
- Guard order fixed. The entry_ids check ran before the model-kind check,
so a singleton had no reachable path: omitting entry_ids failed with
"requires entry_ids", supplying them failed with "only supported for
collection models". Singletons and dictionaries are now supported (omit
entry_ids); documents are rejected with a reason.
- New optional `locale` scopes the change; previously every supported
locale was rewritten, so restoring one locale's status changed the other's.
Tests: tests/tools/bulk.test.ts (7) — the tool had no test file at all,
which is why both races shipped. Reintroducing the Promise.all turns
"persists every entry_id" red with the exact production symptom
(expected 'draft' to be 'published').
A model with i18n:false keeps all its content in one data.json, so it has
exactly one meta record. But the meta path was derived from the caller's
locale, so saving the same entry under two locales produced
meta/{id}/tr.json AND meta/{id}/en.json for one content file — and every
reader was left to guess which was authoritative.
paths.ts held the evidence 26 lines apart: contentFilePath's Pick includes
'i18n' and collapses to data.json; metaFilePath's Pick was 'id' | 'kind',
so i18n was not even in the type and the function structurally could not
branch on it. content-save then handed the same `locale` to both.
- metaFilePath now takes 'i18n' in the Pick plus a required defaultLocale,
making the mistake unrepresentable rather than merely fixed. tsc located
all eight call sites.
- meta-manager's metaPath gets the same treatment, which also removes its
hardcoded `'en'` fallback — that silently wrote to the wrong file on any
project whose default locale was not en.
- Fixes a second bug this asymmetry caused: non-i18n collection deletes
resolved locales from data.json, yielding the pseudo-locale "data", and
looked for meta/{id}/data.json — a file that never existed. The read
returned null, the loop continued, and the meta entry was orphaned on
every delete. metaFilePath now maps "data" onto the default locale.
- describe_format claimed "locale is ignored in file paths", which was
false for meta and left agents to infer the layout. It now states the
meta rule and carries a meta_pattern.
The validator already resolved non-i18n models against the default locale
only, so it agreed with the new convention and needed no behaviour change.
Tests: 3 cases in tests/core/content-save-meta.test.ts under a config
whose default (tr) is deliberately not supported[0] (en) — the exact shape
that made a real project's CDN serve non-i18n collections empty.
doctor reported "SDK client: Stale — run `contentrain generate`" permanently. Running generate did not clear it, which trains people to ignore a health check. It stat'd the two directories. A directory's mtime only moves when an entry is added, removed or renamed inside it — and generate rewrites index.mjs/index.d.ts/index.cjs in place, so .contentrain/client stops moving after the very first run. Meanwhile a selective sync recreates model files via `git checkout HEAD -- <path>`, which unlinks and recreates them and does bump .contentrain/models. So after any model save the comparison could never pass again. newestFileMtime() now walks each directory and compares the newest file mtime, which existing-file rewrites do update. The recursion covers client/data/*, where per-model modules land. The existing test only passed because it created a brand-new model file — the one case that bumps a directory mtime by design — so it never exercised the re-generate path where the bug lives. Tests: 2 new cases in tests/core/doctor.test.ts — one regenerating in place after a model save (red against the old directory-stat code), one where only a nested data file is newer.
contentrain_validate reported zero errors while a project's collections were being served empty from the CDN. It only ever checked whether meta existed, never what it said — so the entire class of failure behind the preceding three fixes was invisible to it. - Notice: drafts sitting alongside published entries in one collection. That is the fingerprint of a write that reset status. Advisory only — a genuine work-in-progress entry is indistinguishable, so this is a call for the agent to make. Never auto-fixed: publishing is a content decision, and MCP does not make those. - Warning: a non-i18n model with meta files outside the default locale. Reported, not auto-removed — a stray may hold the only `published` status in the project, so deleting it could unpublish content. The message names the file to merge into and the ones to drop. Docs: essentials now state that content_save preserves status and that the CDN only delivers `published` collection entries; the bulk skill documents the singleton/dictionary form and the locale scope; the MCP docs page gains a publish-status section. Tests: tests/core/validator/status-drift.test.ts (5), reader-backed and git-free, including the quiet cases so the checks cannot start shouting at healthy projects.
On the CDN, document(model).all() reads the model's _index, which carries
frontmatter only — the body lives in the per-slug document. But all() was
typed Promise<T[]>, and the generated document type declares `body: string`.
So `entry.body` type-checked, was undefined at runtime, and pages rendered
their headings and silently dropped their prose. No error, no warning, a
passing typecheck, and a "verified live on CDN" check that passed because
it only looked at anchors and the table of contents — both frontmatter.
The trap is that the bundled runtime's all() does carry bodies: it reads
the content files directly. Code written and tested against bundled
delivery compiled and passed, then rendered empty after a CDN switch. The
two modes genuinely differ in shape, so they no longer share a return type.
BREAKING CHANGE: CdnDocumentQuery.all() returns DocumentIndexEntry<T>[]
(Omit<T, 'body'>) and first() returns DocumentIndexEntry<T> | undefined.
Reading .body off either is now a compile error; use bySlug(slug), which
returns { frontmatter, body, html }. Runtime behaviour is unchanged — this
only makes the type tell the truth about what was already returned, so the
failure moves from a silently blank page to a build error.
Also fixes the unchecked `fetch<T[]>` assertion in http-transport that let
the lie through in the first place.
Tests: tests/cdn/document-query.test.ts (6) — the CDN document query had
no tests — plus tests/cdn/document-query.test-d.ts (4) pinning the type
contract. That needed real infrastructure: tsconfig.json only includes
`src`, so the tests were never type-checked and a @ts-expect-error in them
would have been decorative. vitest typecheck is now enabled against a
tsconfig.test.json, runs under the existing `pnpm test`, and reverting
DocumentIndexEntry to T fails with "Unused '@ts-expect-error' directive".
✅ Deploy Preview for contentrain-ai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Enabling vitest typecheck for the document index-entry contract made the typechecker read the whole tsconfig.test.json project — src plus tests — for the first time. tsconfig.json only includes `src`, so these two test files had never been type-checked and had drifted: - tests/runtime/singleton.test.ts — `interface Hero` has no implicit index signature, so Map<string, Hero> is not assignable to the accessor's Map<string, Record<string, unknown>>. A type alias does have one. - tests/cdn/bundle-preload.test.ts — mock.calls[1] is possibly undefined under noUncheckedIndexedAccess. Both are test-only; no source behaviour changes. They surfaced as "Unhandled Source Error" and failed the run while the 260 tests passed, which is exactly the point of turning the check on.
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.
Four publish-status bugs reported from a live project running the CDN, plus two more found while verifying them. Every one was confirmed in source, not inferred, and every fix has a test that goes red when the fix is reverted.
The common thread: each tool reported success while the CDN quietly stopped delivering content.
contentrain_validatereported zero errors throughout, and none of it is visible in bundled delivery — a project could carry all of it for months and discover it at the CDN switch.What was wrong
1.
contentrain_content_savesilently unpublished content (highest blast radius). It rebuilt meta from scratch on every write, so editing one field moved apublishedentry todraftand the next CDN build served the collection as{}. The code already knew it was an update —content-save.tscomputedaction: 'updated', then overwrote the meta anyway. Resetting status is a publish decision, and per our own split (MCP is deterministic infra, the agent is intelligence) MCP should never have been making it.The same reset lived in a byte-equivalent second copy in
content-manager.ts, reached viacontentrain_applyand scaffolding. The reporter did not see that one. Both now share onemergeEntryMeta.2.
contentrain_bulk update_statusdropped all but one write. It reportedupdated: 10while committing a two-line diff.Promise.allover per-entrywriteMetacalls, each a read-modify-write of the same{locale}.json— all N read one snapshot, last to settle won.The root cause is ours: CLAUDE.md's quality gate says "
no-await-in-loop→ usePromise.all()", and this is that rule applied mechanically to shared-file writes. A plain sequentialawaitwould have been correct. The rule is now qualified — reads and distinct paths only — because otherwise the next author reintroduces this.copy_localehad the identical race (not in the report).3. Non-i18n models had two meta files for one content file.
contentFilePath'sPickincludes'i18n';metaFilePath's was'id' | 'kind'— 26 lines apart in the same file — so meta structurally could not branch on i18n and followed the caller's locale instead. This also orphaned meta on every non-i18n collection delete (not in the report), which searched formeta/{id}/data.json, a file that never existed.4.
doctor's SDK freshness check was permanently "Stale". It compared directory mtimes;generaterewrites client files in place (never moving the dir), whilegit checkoutrecreates model files during a selective sync (which does).DX: CDN
document().all()returned body-less entries typed as having a body._indexcarries frontmatter only, butall()was typedT[]and the generated type declaresbody: string. Pages rendered headings and silently dropped their prose — type-checked, passed review, shipped. The trap is that the bundledall()does carry bodies, so code written against bundled delivery passed and then rendered empty on CDN.What changed
mergeEntryMeta— existing entries keepstatus/approved_by/version; only new entries startdraft. Both copies unified.writeMetaEntries— one read-modify-write per locale file, so the race can't be reintroduced by looping.updatedcounted from disk; a no-match run now fails loudly.localescope.metaFilePathtakesi18n+ a requireddefaultLocale, making the mistake unrepresentable. tsc found all 8 call sites. RemovesmetaPath's hardcoded'en'fallback too.newestFileMtime()— compares newest file under each dir.document().all()DocumentIndexEntry<T>(Omit<T, 'body'>). Breaking:.bodyis now a compile error → the failure moves from a blank page to a build error.Tests
33 new.
contentrain_bulkhad no test file at all, which is why both races shipped; the CDN document query had none either.Each fix was verified by reverting it and watching the test go red — the bulk test fails with the exact production symptom (
expected 'draft' to be 'published').The SDK type test needed real infrastructure:
tsconfig.jsononly includessrc, so tests were never type-checked and a@ts-expect-errorin them would have been decorative.vitest --typechecknow runs againsttsconfig.test.jsonunder the existingpnpm test.Green locally:
@contentrain/query260 + 4 type tests, MCP core 381,bulk+branch-lifecycle22 together, oxlint 0/0, tsc clean across mcp/sdk/cli/types.One thing for CI to confirm: a full-suite run showed two
tests/git/branch-lifecycle.test.tstimeouts under parallel load — that file allows 30s for tests that take 5-7s alone, and this suite is git-heavy and known-slow. Not caused by these changes (the file is untouched, and it passes alongsidebulk.test.ts), but adding a git-heavybulk.test.tsraises contention, sobulk.test.tswas trimmed to share setup where assertions allowed. CI runs on 2 cores, so it is the real check here.The open Studio PR makes the CDN enforce status for singletons/dictionaries (correct — they publish regardless today). But every singleton in the field is currently
draft, because bug 1 has been resetting them. Today that is harmless; the moment Studio enforces it, that content disappears from the CDN — and the stale-object sweep GCs the prior copy.Two correct fixes that regress when combined. Order:
@contentrain/mcpminor,@contentrain/querymajor).contentrain_validateto find the drift andcontentrain_bulk update_statusto restore it — which needs the singleton support in this PR. Without it there is no MCP route back; the reporter had to hand-edit.contentrain/meta/, which our own docs tell agents never to touch.The changeset carries this as a migration note.
Note on the Studio side
The reporter's diagnosis for bug 3 (CDN looking for
meta/{model}/data.json) was wrong — the builder substituteslocales[0], and a missing meta file over-publishes rather than zeroing out. The real cause wassupported[0]vsconfig.locales.default, which bites exactly the reporter's config (default: tr, supported: [en, tr]). That is fixed in the Studio PR.Worth flagging for later: the Studio PR's
resolveContentPathfix (ignoringlocale_strategy) and bug 3 here are two faces of one root cause — both repos compute paths independently. Consolidating into@contentrain/typesPATH_PATTERNSis a separate piece of work.🤖 Generated with Claude Code