feat: Add content engine - #747
Conversation
`publishedCondition` took `Record<string, PgColumn>`, so passing the columns of a content type without publication compiled and then compared columns that do not exist. It now takes the two columns it actually reads, which a `ContentModel`'s column map satisfies only when `publication` is enabled. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`field.slug({ source: "title" })` generates a NOT NULL, unique-indexed
varchar and derives its value from a text field when the create payload
omits it. Supplied values are normalised the same way, so the rules hold
whoever wrote them.
An update never re-derives the slug - only an explicit `slug` in the
patch moves it, which is what keeps published URLs stable. Nothing
auto-suffixes: `slugify` is deterministic, uniqueness belongs to the
index, and a clash surfaces as the existing 23505 -> 409 mapping.
A slug that folds to nothing (CJK, emoji, punctuation) throws the new
`ContentInputError`, which the generated routes turn into a 400 carrying
the actionable message.
The example plugin gains a slug on `example.article`, with the two-step
backfill migration a populated table actually needs.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`publicApi: { enabled: true, path, fields }` opts a content type into a
generated public read surface. It requires `publication` and exactly one
exposed slug field, both checked at definition time, so enabling
publication still exposes nothing on its own.
`fields` is a strict allowlist with no wildcard, and `searchableFields`,
`filterableFields` and `orderableFields` must each be a subset of it -
that is what stops a filter or a sort being used to probe a column the
response omits. User fields are rejected outright (a user field resolves
to a person), and so is `status`, which is a constant once every row is
published.
Paths are validated as a single lowercase segment, `admin` is reserved
because the admin gate is a substring test, and two content types
claiming the same path fail at registry validation naming both plugins.
`ContentPublicSelect` carries exactly the allowlisted keys, with an
exposed relation projected to `{ id, label }`. The matching Zod schemas
are what the public SELECT will be built from, so a private field never
leaves Postgres rather than being fetched and deleted.
Named `publicApi` rather than `public`: the latter is a reserved word in
strict mode and cannot be destructured in `defineContentType`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`model.publicService(c)` is a separate object from `model.service`, not a filtered view of it: there is no create, update, delete, publish or unpublish to omit, so a public write is not something you can reach by accident. It is `undefined` unless the content type has a `publicApi`. Two invariants make it safe. The published predicate is applied inside every method rather than passed in, so there is no argument a caller can forget - `ContentPublicFindManyArgs` has no `where` and no `includeDrafts`. And the SELECT is built from `publicApi.fields`, so a private column never leaves Postgres. The single exception is `id`, which the pagination cursor reads off the row; it is dropped from the projection unless the allowlist names it, and that boundary is tested. Filters, search and ordering each go through the public allowlist rather than the admin one, so a column that is readable is not automatically queryable and a private column cannot be probed sideways. `resolveReferenceTargets` moves to `server/references.ts` so both services resolve relation labels identically, and `publicationColumns` narrows an erased column map for the predicate - the generic case Phase 0's narrower parameter type made explicit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two GET routes per public content type, mounted by a top-level
`buildContentPublicModule`:
GET /api/{pluginId}/content/{publicApi.path}/
GET /api/{pluginId}/content/{publicApi.path}/{slug}
Public by omission, exactly like every other public route in VitNode: no
`adminStaffPermission` and no `/admin/` in the path, so the global admin
gate never sees them. The route tests install no session at all, which
is what makes "it answered anyway" a real assertion.
A draft, an unpublished row, a cleared publication date and a typo are
all the same 404 - a 403 would confirm the record exists.
`orderBy` is a literal enum of the public allowlist, so a column that is
orderable in the AdminCP but not published is a 400. Private fields are
absent from the filter schema entirely, so they cannot reach the query
builder even as a rejected value.
The module deliberately registers no `contentTypes`: `buildApiPlugin`
collects them recursively and a second registration would throw
"Duplicate content type id". That trap has its own test.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
One row action that flips with the row's state - Publish for a draft, Unpublish for a published row - rather than two buttons with a dead one. Gated by `can_publish`, never by `can_edit`, so a role can be trusted to write drafts without being trusted to put them on the internet. Confirmation dialog, loading and disabled state from the existing `ConfirmActionAlertDialog` (which now takes `submitVariant`, since `destructive` is right for a delete and wrong for a publish), success and failure toasts with a description, and a table refresh through the server action's `revalidatePath`. Both routes are idempotent, so a double click is a 200 with `changed: false` rather than an error - the button needs no guard. The edit dialog gets a read-only status line rather than a second publish control: `status` and `publishedAt` are not in the form schema, and two competing mutation paths in one dialog is how a form ends up fighting its own state. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Cache tags are pure strings in `@vitnode/core/content`, so an app can tag
its own fetches and `"use cache"` functions with exactly the same values
and get invalidated alongside the generated pages. `revalidateTag` lives
in the new `@vitnode/core/content/next` entrypoint, the only module in
the engine that imports `next/*` - `content/` and `content/server/` are
loaded by `apps/api` and drizzle-kit, where that throws. A test walks the
import graph and asserts the rule rather than trusting it.
Format is `content:{contentTypeId}:{scope}[:{key}]`, clamped to Next's
256-character limit with the same FNV-1a fingerprint the index names use
(now shared in `content/fingerprint.ts`), so two 160-character slugs
cannot collapse onto one tag.
`contentInvalidationTags` is pure, so the whole matrix is a table test:
a draft created or edited touches nothing, publish/unpublish/delete-once-
published expire list + item + slug, and a slug change expires the old
URL and the new one. Nothing global is ever invalidated.
Invalidation is triggered by the AdminCP server actions, after the write
returns - never by the service, which may be inside an uncommitted
transaction, may not be running under Next, and owns no request scope.
The update path reads the row *before* writing so it knows the slug it
is replacing; that read is skipped for content types with no public API.
`RawApiFetchArgs["options"]` gains an explicit `next` field: Next's
augmentation of the global `RequestInit` is not visible where the package
compiles, and widening the shared fetcher beats bypassing it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four new pages - Slug field, Public API, Public Content Service and Caching - and updates to the nine existing ones that were describing a world without them. The four names are now used consistently and kept apart: Admin Content Service, Public Content Service, generated Admin API, generated public API. `service.mdx` is retitled and says which of the two it is. Three claims that were true last week and are not any more: the "publish buttons are not here yet" callout in admincp.mdx, the "no generated public read endpoint" section in limitations.mdx, and the "until the generated public read layer lands" framing in publication.mdx. Every page repeats the rule that matters: publication alone exposes nothing, public content is opt-in twice over, public writes are never generated, and a direct service call emits no event and expires no cache tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ContentTableView` passed only `admin.list.orderableFields` to `DataTable`, so `id`, `createdAt`, `updatedAt` and - with publication - `status` and `publishedAt` had no sort control, even though the generated route has always allowed them. Both sides now read `orderableColumns(definition)`, the client-safe helper the route already builds its `orderBy` enum from, so the two lists cannot drift. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Ten findings from the PR #733 review, verified against the installed Next 16.3.0-preview.9 declarations rather than assumed. Caching - `contentPublicFetch` now sends `cache: "force-cache"`. Caching is opt-in in Next 16, so without it the tags decorated a response that was never stored. The opt-in is on this function only; `rawApiFetch` is untouched. - `revalidateContent` takes a mode. `immediate` (the default) calls `updateTag`; `stale-while-revalidate` keeps `revalidateTag(tag, "max")`. Unpublish, delete and a moved slug must not serve the old response even once, so they expire immediately; an edit to a published row that kept its URL is the one case that stays stale-while-revalidate. Public projection - An exposed relation is `{ id }`. The label came from the target's `admin.titleField` - administrative metadata that may name a field the target never publishes, on a row that may itself be a draft. The public service now joins nothing at all, so a target table is never read. Admin labels are unchanged. Correctness - The slug backfill truncates the base before appending the row id. A slug can already fill `varchar(160)`, so `slug || '-' || id` overflowed the column and failed the migration on exactly the rows the statement was rescuing. - `contentPublicFetch` takes `PublicContentTypeDefinition`, so a content type without `publicApi` is a compile error instead of a request to `/api/{pluginId}/content//`. - The detail path is `encodeURIComponent`d. Policy - `publicApi.path` collisions are scoped to `pluginId + path`. The route is `/api/{pluginId}/content/{path}`, so two plugins publishing "articles" do not collide; rejecting them failed an app's boot over a name neither author can see. Inside one plugin it is still an error. - `publicService.findById` is kept as direct-plugin API - an event payload carries a `contentId`, not a slug. It applies the predicate centrally and returns the public projection, and no numeric-id route is generated. - `publishedCondition`'s comment no longer claims the engine generates no public route. Tests - New: `fetch.server.test.ts` (cache mode, tags, path encoding), `revalidate.server.test.ts` (which Next API each mode calls). - `mutation-api.test.ts` drives the real `revalidate.server` against a mocked `next/cache` and asserts the function, not just the tag list. - The Postgres suite seeds duplicate titles, a title filling the whole column and a non-Latin one *between* migrations, so the committed backfill runs against real data, and asserts no relation label reaches a response. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The duplicate pass rescued a row by appending its id - but the value it built is itself a slug, so it could land on a natural one that was left alone: id=1 "Foo 2" -> "foo-2" unique, untouched id=2 "Foo" -> "foo" -> "foo-2" rescued onto row 1 id=3 "Foo" -> "foo" -> "foo-3" The id fallback had the same hole: a title that normalises to nothing becomes its bare id, which collides with a row whose title genuinely normalised to that number. Neither surfaced until `CREATE UNIQUE INDEX` failed two statements later, halfway through a deploy. Dropping the `WHERE` and suffixing every row removes the class of bug rather than the instance. Every value is `<base>-<id>`, or `<id>` when the title left nothing behind, so two can only be equal if their ids are - and ids are the primary key. No loop, no second pass, nothing random. The cost is that unambiguous rows are suffixed too. That is migration-only: runtime create and update still return 409 on a collision and never rewrite a URL an author chose. The Postgres suite seeds both collision scenarios with explicit ids - they only reproduce at particular ids, so the sequence must not pick them - alongside the existing long-title and non-Latin cases, and replays the committed migration over them. Restoring the old predicate fails that suite on the unique index. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SearchModel` derived a document's owner from `c.get("plugin")`, and a rebuild
runs inside the core cron request - so the same record was stored as
`@vitnode/example` when a mutation route indexed it and `core` when a rebuild
did. The Elasticsearch adapter made it worse by hardcoding `pluginId: "core"`
in `toSource`, so the mirrored document could disagree with the canonical row.
`SearchDocument` gains an optional `pluginId`, and ownership is resolved in one
place - `SearchModel.resolveOwner` - as document, then request, then `"core"`.
An explicit owner now wins over the request, `pluginId` joins the conflict
`set` so a rebuild can repair a row written before its indexer declared one,
and the adapter serializes what it was given.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`SearchIndexer.load` returned documents only, and the rebuild treated an empty
array as end-of-source. A page can read rows and project none of them - every
row on it published with an unusable title, say - so the rebuild stopped there
and never reached the valid records behind it.
`load` now returns `{ documents, itemsRead }`. The rebuild advances its cursor
by `itemsRead` and terminates only on `itemsRead === 0`, which also keeps
multi-document-per-item indexers correct: a document count was never a source
count. Every implementation in the repository is updated, so there is no
transitional union to remove later.
The rebuild also stamps the registering plugin on any document that names no
owner, and generated indexers carry their plugin id into the mapper - the queue
drains inside the core cron request, where the request's plugin is not the
owner.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The status route raised a collection's source total to `Math.max(total, indexed)` and the UI called anything with `indexed >= total` healthy. So 10 documents for 9 published records was rewritten to 10/10, 100%, "Indexed" - the one state that most needs attention was the one guaranteed to be hidden. Both counts are now reported as measured, and a collection is "Indexed" only when they match exactly; any mismatch in either direction is stale. Coverage can read past 100% because that is the signal, while the progress bar is clamped so it cannot draw past its track. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ResolvedContentSearchConfig.enabled` was `boolean`, so a searchable definition only satisfied `SearchableContentTypeDefinition` after an `as` - and the type test asserted the cast rather than the behaviour. `defineContentType` now infers the whole `search` argument as one type parameter and reads the literal back off it, so `enabled` resolves to `true` or `false` and the public type needs no assertion. Inferring the object rather than its parts is what makes this work: an intersection member is not an inference site, and the field rules stay in the parameter's *constraint*, which is checked once `TPublicField` is resolved. `titleField` also has to be non-nullable now, at compile time and at runtime: a `null` heading is not a search result, and a record without one is skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`syncContentSearch` caught the search engine's error and then awaited
`c.get("log").error(...)`, which writes to the database - so a logger that was
down for the same reason the engine was turned a committed content write into an
HTTP 500. That is exactly the guarantee the feature documents it keeps.
Logging is now wrapped the way `LocalEventsAdapter` wraps its own, falling back
to the console, and the returned outcome still carries the original search error
rather than the logger's.
The Postgres suite also gains a case for a published row the mapper cannot
project: page one reads it and yields nothing, and `itemsRead` is what carries
the rebuild through to the valid rows behind it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents plugin ownership across live sync and both rebuild shapes, the difference between source rows and produced documents, why an empty document page does not end a rebuild, the real meaning of the AdminCP counts (including over-indexed as stale, and malformed data as under-indexed), and that a logger failure is as harmless to the mutation as a search engine failure. States plainly that there is no durable retry mechanism. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Stage 3 changed `load` to return `SearchIndexerPage`, which is the right contract but broke every external plugin using the documented public import - a bare `SearchDocument[]` no longer compiled. `load` now returns `SearchIndexerLoadResult`, and one helper decides what a result means. A page passes through; a non-empty array reports the requested limit, because that is what the old rebuild advanced by and an array carries no source count - `documents.length` would skip rows for any indexer that emits several documents per item. An empty array is the only end signal it has, which is exactly why the array form cannot express "rows read, none projected" and is deprecated rather than merely older. `ContentSearchIndexer` pins the generated adapter to the page result, so the widened contract does not weaken what the engine itself guarantees. A blank `pluginId` is now treated as absent everywhere ownership is resolved, through a shared `searchDocumentOwner`, since it became public input. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A collection with documents but no registered indexer was reported as owned by core, because `indexer?.pluginId ?? "core"` had no other source to consult. The rows themselves know better: they carry the plugin that wrote them. The coverage query now selects `pluginId` alongside the counts, and ownership resolves as registered indexer, then stored owner, then `"unknown"`. The registered indexer stays canonical - it is what the next rebuild will stamp on the rows - so a disagreement resolves in its favour rather than reporting a mismatch nobody can act on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Covers the preferred page result and what `itemsRead` counts, the deprecated array result and the one guarantee it cannot make, and how the AdminCP names the owner of a collection whose indexer is gone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`total` fell back to the indexed count when no indexer was registered, so a collection nothing can rebuild reported 11/11, 100%, "Indexed" - the fallback matching itself read as full coverage. The status response now carries `hasIndexer`, taken from whether an indexer was found and from nothing else: rows knowing which plugin wrote them says nothing about whether anything can write them again. `total` is `null` when there is no indexer to ask, so coverage is absent rather than invented, and the UI gains an `orphaned` status that is decided before the counts are compared. An indexer without the optional `count` still falls back to the indexed count - that is the documented meaning of leaving `count` out, and it is a registered collection either way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`Reindex` on an orphaned collection deleted it: the task cleared the item type and then rebuilt nothing, because the filtered indexer list was empty. The rebuild route now answers 404 for an item type no indexer claims, and the task refuses it before `search.clear` runs. Both checks are needed - the route gives the AdminCP an immediate, explainable failure, while the task also covers direct queue dispatches and a job queued while the indexer was still registered. A full rebuild is untouched: it clears the whole index and refills every registered indexer, so orphaned documents are still removed by it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two additions to the generated AdminCP, both opt-in and both defaulting to
exactly what a content type does today.
`admin.create.mode` and `admin.edit.mode` take `"dialog"` or `"page"`, and
default to `"dialog"` - so nothing about an existing content type moves. Page
mode is served by the *same* catch-all route as the list, resolved
exact-content-type-first so an id ending in `.create` keeps its own screen:
/admin/content/blog/post list
/admin/content/blog/post/create create
/admin/content/blog/post/42/edit edit
The Create button and the row pencil become links rather than dialogs that
mount and redirect, and both pages check `can_view` plus their own permission
on the server - a URL typed into the address bar answers the way the button
would have. A create hands over to the new record's edit page when there is
one, using the id the mutation now returns.
`forms.layout` on the frontend registration lets a plugin place the fields
itself. It is presentation only: the engine keeps the schema, the validation,
the defaults, the mutation, the version precondition, the structured errors,
the toast, the invalidation, the events, the search write and the delivery
effects. `ContentFormField` renders the element the engine already built -
including its field override - so overrides and layouts compose.
The layout is a client component referenced from a *server* config, so its
props are serialisable and everything else reaches it through client context.
A `renderField(name)` callback would read better and would be a server closure,
which cannot cross that boundary at all.
Two fixes fall out of the same work:
- the locale tabs rendered no inputs at all. `TranslationPanel` handed AutoForm
bare field ids, and AutoForm renders nothing for a field with no component -
so every localized content type had a form nobody could type into.
- `GET /{id}` now answers with `labels`, the way the list already did. It is
the read a form makes, and a relation picker showing `3` instead of a name
was the only thing stopping page mode from opening on a complete record.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The blog becomes a Content Engine consumer and stops being a CRUD
implementation. Two content types replace six route files, three lib files, two
admin screens, two create/edit dialogs, a search indexer and a hand-written slug
uniqueness check.
blog.category dialog create/edit, colour field override, colour cell
override, relation target
blog.post page create/edit, custom layout, AutoFormEditor, category
relation, author, publication, editorial, search, delivery
The ids stay `blog.post` and `blog.category`, the tables stay `blog_posts` and
`blog_categories`, the fields stay `categoryId` and `authorId`, and the
permission modules stay `posts` and `categories` - so every existing role, every
foreign key and every stored permission still addresses the right thing.
"Article" is what the AdminCP calls it, because that is what people call it.
The migration is additive. Nothing is dropped and no record moves:
- the text moves out of `core_languages_words` into the two generated
translation tables, one row per language that genuinely had one,
- every existing article becomes `published` with `publishedAt = createdAt` -
they were all publicly readable before, and that is the one publication fact
the old schema can prove. No revision history is invented,
- a record with no default-locale translation gets one built from a value it
already has, rather than being left unreadable,
- only rows that were actually copied are deleted from the old storage.
A PostgreSQL suite seeds a pre-migration install - two categories with different
colours, three articles, an author, rich bodies, existing slugs and a Polish
translation - runs the committed migration over it, and reads everything back
through the engine's own services.
Search and events stop being duplicated. The `search` block replaces
`api/lib/search.ts`, which emitted a document per *enabled language* whether or
not a translation existed. The blog's own event names survive as adapters over
`content.blog.*`, so one mutation still means one announcement -
`blog.post.deleted` loses `categoryId`, because the row is gone by then and
inventing one would be a lie in an audit trail.
The legacy admin URLs redirect. The two public read routes are removed: they
read `core_languages_words`, which no longer holds the data, and the article's
generated public API is a better answer at the same `/blog/` prefix.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A new page for dialog-vs-page and custom layouts, with the blog as the worked example of both ends: a category with a colour picker and a colour cell, and an article with a page-mode editor and `AutoFormEditor`. Says plainly what a layout is not: it decides where the fields are, and the Content Engine decides what happens when you press Save. The blog guide gains its new architecture and an upgrade section naming the two deliberate breaks, and the events reference now marks the blog's own event names as compatibility adapters over `content.blog.*`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A relation picker takes its labels from the target's `admin.titleField`, which has to be a shared column - and every text field on a blog category is localized, so there is none. The generic picker fell back to identifiers, and `#3` is not a category anybody recognises. The label, and only the label, now comes from the generated admin list route with `?locale=`, which already returns each row's translation. The relation is untouched: a real foreign key, a real `onDelete: "restrict"`, validated by the generated schemas, and the combobox stores the same identifier it always did. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit 6063236. The override reached a server action, and `config.tsx` cannot: the `vitnode` CLI loads it with jiti to enumerate a plugin's routes and messages, so anything `server-only` in its static graph throws - which broke `vitnode init` outright, before the first migration ran. That is the constraint core's own overrides already respect, and the reason `ContentField` is handed a `loadOptions` callback rather than importing one. A lazy import would have hidden the trap rather than removed it. So the limitation stands and is written down instead: a relation label comes from a **shared** column on the target, and a localized content type has none, so the article's category picker labels its options `#3`. Resolving a label from the translation table is a Content Engine change, not a plugin workaround. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Base UI's `Button` assumes it renders a real `<button>` unless told otherwise,
so every one of the five link-rendered buttons page mode introduced - Create,
the row pencil, the two Back links and a layout's Cancel - warned at runtime and
lost the button semantics that forms and assistive technology rely on.
`nativeButton={false}` is what the AdminCP's existing link-buttons already pass.
The tests said nothing, and that was the more interesting bug: the `Link` mock
returned a bare `<a href>` and dropped every other prop, so Base UI's ref never
attached, its check never ran, and the assertion passed by not looking. The mock
now spreads what it is given, and both suites fail on an unexpected
`console.error` - verified by reintroducing the missing prop and watching them
go red.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4e6799740
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // block. There is nothing to publish and there never will be, so cancel | ||
| // rather than retrying until the queue gives up - an error every ten | ||
| // minutes forever is not a useful way to report a config change. | ||
| if (!entry || !editorialService) { |
There was a problem hiding this comment.
When a deployment disables only editorial.scheduling while retaining the editorial workflow, editorialService remains defined, so this guard lets previously queued tasks continue to the publish/unpublish call below. As a result, content can still go live or be withdrawn after scheduling was explicitly turned off; treat model.definition.editorial.scheduling.enabled === false as unregistered and cancel the pending schedule.
Useful? React with 👍 / 👎.
| */ | ||
| export const clearSearchDebugAdminRoute = buildRoute({ | ||
| pluginId: CONFIG_PLUGIN.pluginId, | ||
| adminStaffPermission: { module: "system", permission: "can_view" }, |
There was a problem hiding this comment.
A staff role granted only system:can_view for diagnostic access can POST to this endpoint and permanently remove every indexed document in an unmanaged collection. Since the system permission catalog already distinguishes read access from mutating operations, gate this destructive route behind a dedicated mutation permission rather than the read-only status permission.
Useful? React with 👍 / 👎.
| export default function ErrorBoundary({ error, retry }: RouteErrorViewProps) { | ||
| return <RouteErrorView error={error} retry={retry} />; |
There was a problem hiding this comment.
Next App Router invokes error components with { error, reset }, not { error, retry }; consequently retry is undefined here and clicking “Try again” throws instead of recovering. Accept reset and pass it to the shared view; the same incorrect signature is copied into the other new route boundaries and both global-error templates.
AGENTS.md reference: apps/docs/AGENTS.md:L3-L5
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7817399e03
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| DELETE FROM "core_languages_words" w | ||
| USING "core_languages" l | ||
| WHERE w."pluginCode" = '@vitnode/blog' | ||
| AND w."tableName" IN ('blog_categories', 'blog_posts') | ||
| AND l."code" = w."languageCode"; |
There was a problem hiding this comment.
On upgrades with existing posts, the old indexer has already stored documents under itemType: "blog_post", while the Content Engine now writes and deletes only blog.post. This migration removes the legacy language rows but neither rekeys nor clears the old canonical/provider documents, so old search results continue pointing at the removed /blog/{categoryId}/{slug} URLs and later edits can produce duplicate old/new hits until an administrator manually performs a full rebuild. Include an automatic search migration or rebuild that clears both the canonical table and the active provider.
Useful? React with 👍 / 👎.
| return await c.get("db").transaction(async tx => { | ||
| const pending = await pendingForItem(itemId, tx); |
There was a problem hiding this comment.
When publish and unpublish requests for the same item run concurrently, both transactions can read an empty pending set here and then insert different (item, action) rows, so the partial unique index does not make either transaction conflict. This bypasses contentScheduleTimingError and can commit an unpublish scheduled before its publish, causing the later publish to leave content live contrary to the requested final state; lock a common per-item resource before reading the pending schedules.
Useful? React with 👍 / 👎.
| export const blogPostContentType = defineContentType({ | ||
| id: "blog.post", |
There was a problem hiding this comment.
The generated search documents use the content type id blog.post, but packages/vitnode/src/views/search/registry.tsx still registers only blog_post. Consequently every newly indexed blog result falls back to the “unknown” renderer, and the Blog Posts filter continues querying the obsolete collection instead of the new one; update the renderer key and its filter coverage alongside this id change.
Useful? React with 👍 / 👎.
| locale: resolved.locale, | ||
| }); | ||
|
|
||
| return c.json(resolution, 200); |
There was a problem hiding this comment.
When callers omit ?locale=, this route resolves the response from Accept-Language, but the response carries neither Vary: Accept-Language nor Content-Language. A shared HTTP cache can therefore reuse a Polish resolution for an English request to the same URL; the item and sitemap handlers have the same omission. Return the locale headers used by the ordinary public routes for all negotiated delivery responses.
Useful? React with 👍 / 👎.
| const ContentForm = dynamic(async () => | ||
| import("./content-form").then(mod => ({ default: mod.ContentForm })), | ||
| ); |
There was a problem hiding this comment.
Replace this next/dynamic declaration—and the matching one in edit-action.tsx—with React.lazy; the form is explicitly content-heavy and is already rendered inside Suspense, while the repository convention requires that loading pattern for heavy dialogs.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Reject an empty collection identifier
When an API caller sends { "itemType": "" }, this schema accepts it, both truthiness checks treat it as omitted, and the route dispatches an empty payload. The rebuild task consequently calls search.clear(undefined) and rebuilds the entire index rather than rejecting the invalid collection-scoped request, which can cause an unexpectedly expensive and disruptive global rebuild. Require a non-empty itemType, as the adjacent clear-search schema already does.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export const blogApiPlugin = () => | ||
| buildApiPlugin({ | ||
| pluginId: CONFIG_PLUGIN.pluginId, | ||
| modules: [adminModule, categoriesModule, postsModule], | ||
| searchIndexers: [blogPostSearchIndexer], | ||
| permissionStaff: { | ||
| moderator: { | ||
| posts: ["can_edit", "can_delete"], | ||
| }, | ||
| admin: { | ||
| posts: [ | ||
| "can_view", | ||
| { | ||
| permission: "can_create", | ||
| dependsOn: ["can_view"], | ||
| }, | ||
| "can_edit", | ||
| "can_delete", | ||
| ], | ||
| categories: ["can_view", "can_create", "can_edit", "can_delete"], | ||
| }, | ||
| }, | ||
| modules: [ |
There was a problem hiding this comment.
The previous plugin config declared moderator.posts with can_edit and can_delete, but this replacement supplies no moderator catalog entries, and withContentPermissions only derives entries for admin. When any restricted moderator entry is saved, updatePermissionsStaffAdminRoute filters its grants against that catalog and silently removes the existing @vitnode/blog:posts permissions; those permissions also cannot be granted to new moderators. Preserve the moderator declaration alongside the generated admin permissions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Require a mutation permission to rebuild search
A staff role granted only system:can_view for integration diagnostics can POST to this route repeatedly and queue full rebuilds. The worker begins by calling search.clear(itemType), so this can temporarily empty the public search index and impose repeated database/provider load despite the role having no mutation permission; gate rebuilding behind a dedicated mutation permission rather than the read-only status permission.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const locale = localeOf.get(values._languageId as number); | ||
| // A translation whose language row has been deleted has no locale to | ||
| // index under. Skipped rather than guessed - a document under an invented | ||
| // code is one nothing would ever query. | ||
| if (locale === undefined) return []; |
There was a problem hiding this comment.
When a previously published locale is changed to enabled: false, public content and delivery routes reject that locale, but this rebuild path still creates its search document because it only checks whether the language exists. syncContentLocalizedSearch similarly maps every language without filtering isEnabled, so later edits re-upsert the document; the unauthenticated search route can consequently return the disabled translation's title and content via ?lang=<locale>. Filter disabled languages in both rebuild and live synchronization so rebuilding also removes stale documents.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1cfae19364
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| }); | ||
|
|
||
| if (!response.ok) break; |
There was a problem hiding this comment.
When the delivery API returns a non-2xx response, this silently breaks the loop and returns the entries collected so far. In particular, a failure on the first request leaves cursor as null, so the result is { entries: [], truncated: false }, indistinguishable from a successfully generated empty sitemap; a transient API error can therefore publish an empty or incomplete sitemap without alerting the caller. Throw the fetch error or mark the result as truncated/failed.
Useful? React with 👍 / 👎.
| export const ensureContentPreviewSecret = async ( | ||
| db: ContentDatabase, | ||
| ): Promise<string> => { | ||
| cached ??= readOrCreate(db).catch((error: unknown) => { |
There was a problem hiding this comment.
Once any request populates this process-wide cache, deleting or replacing the content_preview row no longer rotates the key for that running process. During an emergency rotation, existing instances continue accepting compromised tokens and minting links with the old key, while restarted instances use the new key, making preview links succeed or fail depending on which instance handles the request. Use a bounded cache or an explicit production invalidation mechanism so all processes converge on the rotated database value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a28cc736c7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // The same helper the interactive routes use, so a scheduled publish and a | ||
| // clicked one are indistinguishable to every listener and to the index. | ||
| const { event, search, searchByLocale } = await contentEditorialEffects( |
There was a problem hiding this comment.
When an effects run succeeds at search synchronization but fails at another effect such as cache delivery, the whole task is retried; if the item is edited, unpublished, or republished before that retry, this call replays the frozen older outcome and can overwrite the newer search document (for example, an old publish retry can re-index content that was subsequently unpublished). Check the current item version/state before applying stateful effects, or track completed effects so a partial retry cannot replay an obsolete search mutation.
Useful? React with 👍 / 👎.
| const after = await contentPublicLocaleStates(c, model, payload.itemId, { | ||
| row, | ||
| }); | ||
| const before = await contentPublicLocaleStates(c, model, payload.itemId, { |
There was a problem hiding this comment.
For localized content, these reads occur after the transition has committed and therefore use the current translation rows rather than those that existed when the scheduled publish/unpublish ran. If a translation is deleted, unpublished, or has its slug changed before this effects task executes, both snapshots lose the former public locale/slug; that intervening translation mutation sees the base already unpublished and invalidates no public tags, so a cached withdrawn page can remain reachable. Persist the per-locale before/after invalidation data in the queued payload while the transition is committed.
Useful? React with 👍 / 👎.
| | --- | --- | | ||
| | `GET /api/@vitnode/blog/posts` | `GET /api/@vitnode/blog/content/blog` | | ||
| | `GET /api/@vitnode/blog/categories` | removed - categories have no public URL | | ||
| | `/admin/blog/posts`, `/admin/blog/categories` | redirect to the generated screens | |
There was a problem hiding this comment.
On upgrade, /admin/blog/posts and /admin/blog/categories do not redirect as documented: this commit deletes both page handlers, and a repository-wide search finds no replacement redirect or route for either path. Existing bookmarks and links therefore resolve to 404 instead of the generated Content Engine screens; retain lightweight redirect pages or add equivalent redirects before advertising this migration behavior.
Useful? React with 👍 / 👎.
1 participant
Improving Documentation
pnpm lint:fixto fix formatting issues before opening the PR.Description
What?
Why?