Skip to content

Commit d705454

Browse files
authored
fix(publish): scope entry-route runtime assets to their own template (#320)
Every entry route on a site served one arbitrary page's scripts. `getLatestPublishedSiteSnapshot` is `order by data_rows.created_at asc limit 1` — the first published page ever created. It exists to carry `site_json` for routes that are not themselves pages, and its `runtime_assets_json` rode along by accident. Entry routes took it verbatim, so `assetScopeAppliesToPage` was never evaluated on that path at all: a script scoped to two pages shipped on all ten entry routes, and had the oldest page carried no scripts the same bug would have shipped a scoped script nowhere. The 404 route inherited the same manifest, and so did the publish-time bake, which is what most requests actually serve. That getter no longer carries runtime assets, so a miss now degrades to "no scripts" rather than "someone else's scripts". `entryTemplateSnapshot.ts` resolves the manifest from the page that actually renders — the innermost template in the chain — and treats its absence as authoritative too, so a manifest arriving from anywhere else cannot survive onto a route it was never scoped to. Also fixed, found on the way: `composeTemplateChain` built its result field by field and dropped `Page.template`, and `assetScopeAppliesToPage` gates its `templates` branch on `Boolean(page.template)`. A stylesheet scoped to an entry template therefore never applied to that template's own routes — the one place it was meant to. And `applyStatus` called `updateSelectedEntry` for whatever row it published, active or not, silently retargeting the workspace. That discarded whatever an author had open and unsaved, and left a tool loop of `set_document_fields → set_document_status` writing one document behind itself, so every field write after the first was refused. It now uses `applyEntryUpdate`, which selects only a row that was already active — matching the guard the non-agent paths have always had. The field tools' descriptions state the active-document precondition they have always enforced. Test: an entry route takes its template's manifest, ignores a stale one, keeps the site document it resolved against, and falls back cleanly with no template; a composed template keeps its config so a template-scoped asset matches; publishing another document leaves the active one alone and a following write to it still lands. Verified live: pack 020's page-scoped script now appears on `/` and `/circuit` only, where it previously appeared on those plus all ten entry routes.
1 parent 19b04b6 commit d705454

12 files changed

Lines changed: 396 additions & 14 deletions

File tree

server/ai/tools/content/writeTools.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ const setDocumentFieldTool: AiTool = {
125125
execution: 'browser',
126126
requiredCapabilities: DOCUMENT_EDIT_CAPS,
127127
description:
128-
"Write one field on a document. `value` shape depends on the field type (read content_get_collection_schema first if unsure): text/longText/richText/url/email → string; number → number; boolean → boolean; date/dateTime → ISO string; select → option id; multiSelect → option id[]; media → { id } or { id }[]; relation → { rowId } or { rowId }[]; body → markdown string. Bridge converts markdown ↔ Tiptap automatically for body.",
128+
"Write one field on a document. The document MUST be the active one — call content_set_active_document first, or the write is refused. (content_create_document leaves the new document active, so create-then-fill needs no extra call.) `value` shape depends on the field type (read content_get_collection_schema first if unsure): text/longText/richText/url/email → string; number → number; boolean → boolean; date/dateTime → ISO string; select → option id; multiSelect → option id[]; media → { id } or { id }[]; relation → { rowId } or { rowId }[]; body → markdown string. Bridge converts markdown ↔ Tiptap automatically for body.",
129129
inputSchema: SetDocumentFieldInput,
130130
}
131131

@@ -144,7 +144,7 @@ const setDocumentFieldsTool: AiTool = {
144144
execution: 'browser',
145145
requiredCapabilities: DOCUMENT_EDIT_CAPS,
146146
description:
147-
'Batch-write multiple fields on one document. `fields` is Record<fieldId, value>; same per-type shapes as content_set_document_field. Prefer this when generating a whole post (title + slug + body + seo* in one call).',
147+
'Batch-write multiple fields on one document. The document MUST be the active one — call content_set_active_document first, or the write is refused. `fields` is Record<fieldId, value>; same per-type shapes as content_set_document_field. Prefer this when generating a whole post (title + slug + body + seo* in one call), and when filling several documents in sequence set each one active before writing to it.',
148148
inputSchema: SetDocumentFieldsInput,
149149
}
150150

server/publish/bakeDataRows.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { renderPublishedDataRowTemplate } from './publicRenderer'
3232
import { applyPublishedHtmlPipeline } from './publishedHtmlPipeline'
3333
import { writeArtefact } from './staticArtefact'
3434
import { getLatestSnapshotForVersion } from './publishedSnapshotCache'
35+
import { snapshotForEntryRoute } from './entryTemplateSnapshot'
3536

3637
interface DataRowBakeResult {
3738
/** Routes successfully baked into the slot. */
@@ -91,7 +92,10 @@ export async function bakePublishedDataRowArtefacts(
9192
const row = await getPublishedDataRowByRoute(db, route.tableRouteBase, route.rowSlug)
9293
if (!row) continue
9394
const syntheticUrl = new URL(`http://localhost${urlPath}`)
94-
const rendered = await renderPublishedDataRowTemplate(siteSnapshot, row, {
95+
// Runtime assets come from this table's entry template, not from the
96+
// arbitrary page the site-wide snapshot happens to name.
97+
const snapshot = await snapshotForEntryRoute(db, siteSnapshot, route.tableSlug)
98+
const rendered = await renderPublishedDataRowTemplate(snapshot, row, {
9599
db,
96100
url: syntheticUrl,
97101
publishVersion,
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/**
2+
* Runtime assets for a route rendered through a template.
3+
*
4+
* Runtime assets are per-PAGE: `publishSite.ts` bundles them once per row in
5+
* `site.pages`, honouring each script's `scope` through
6+
* `assetScopeAppliesToPage`. Entry routes (`/rooms/<slug>`) are not pages —
7+
* they are rendered per request from a template page plus a data row — so they
8+
* have no manifest of their own and have to borrow their template's.
9+
*
10+
* They used to borrow whatever `getLatestPublishedSiteSnapshot` returned, which
11+
* is `order by data_rows.created_at asc limit 1`: the first published page ever
12+
* created. That getter exists to supply `site_json`; its `runtime_assets_json`
13+
* rode along by accident. So every entry route on a site served one arbitrary
14+
* page's scripts, and the scope predicate was never consulted on that path at
15+
* all — a script scoped to two pages shipped on all of them, and a script the
16+
* oldest page did not carry never shipped anywhere.
17+
*
18+
* `getLatestPublishedSiteSnapshot` no longer carries runtime assets, so the
19+
* failure mode if this resolution misses is "no scripts", never "someone
20+
* else's scripts".
21+
*/
22+
import type { DbClient } from '../db/client'
23+
import type { PublishedPageSnapshot } from '../repositories/publish'
24+
import { getPublishedPageSnapshotById } from '../repositories/publish'
25+
import { resolveNotFoundTemplate, resolveTemplateChain } from '@core/templates/templateMatching'
26+
27+
/**
28+
* `siteSnapshot` with the runtime manifest of the page that will actually
29+
* render `pageId`. The site document is kept from `siteSnapshot` so it stays
30+
* the one the caller resolved its template chain against, even if a publish
31+
* lands between the two reads.
32+
*/
33+
async function withRuntimeAssetsOfPage(
34+
db: DbClient,
35+
siteSnapshot: PublishedPageSnapshot,
36+
pageId: string,
37+
): Promise<PublishedPageSnapshot> {
38+
const own = await getPublishedPageSnapshotById(db, pageId)
39+
// The rendering page's manifest is authoritative, INCLUDING when it has
40+
// none. Merging only the present case would let a manifest that arrived on
41+
// `siteSnapshot` from anywhere else survive onto a route it was never
42+
// scoped to — which is the whole defect.
43+
const { runtimeAssets: _discarded, ...withoutAssets } = siteSnapshot
44+
return own?.runtimeAssets
45+
? { ...withoutAssets, runtimeAssets: own.runtimeAssets }
46+
: withoutAssets
47+
}
48+
49+
/**
50+
* Snapshot to render an entry route of `tableSlug` with — the innermost
51+
* template in its chain supplies the runtime manifest.
52+
*/
53+
export async function snapshotForEntryRoute(
54+
db: DbClient,
55+
siteSnapshot: PublishedPageSnapshot,
56+
tableSlug: string,
57+
): Promise<PublishedPageSnapshot> {
58+
const chain = resolveTemplateChain(siteSnapshot.site, { kind: 'entry', tableSlug })
59+
const innermost = chain[chain.length - 1]
60+
if (!innermost) return siteSnapshot
61+
return await withRuntimeAssetsOfPage(db, siteSnapshot, innermost.id)
62+
}
63+
64+
/** Snapshot to render the 404 route with — same reasoning as entry routes. */
65+
export async function snapshotForNotFoundRoute(
66+
db: DbClient,
67+
siteSnapshot: PublishedPageSnapshot,
68+
): Promise<PublishedPageSnapshot> {
69+
const template = resolveNotFoundTemplate(siteSnapshot.site)
70+
if (!template) return siteSnapshot
71+
return await withRuntimeAssetsOfPage(db, siteSnapshot, template.id)
72+
}

server/publish/publicRouter.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ import {
7676
import { NOT_FOUND_ARTEFACT_URL_PATH, readArtefact } from './staticArtefact'
7777
import { getOrRender, peek } from './renderCache'
7878
import { getLatestSnapshotForVersion } from './publishedSnapshotCache'
79+
import { snapshotForEntryRoute, snapshotForNotFoundRoute } from './entryTemplateSnapshot'
7980
import { getPublishVersion } from './publishState'
8081
import { canonicalRenderQuery } from './loopPrefetch'
8182

@@ -173,7 +174,10 @@ async function resolvePublicRoute(
173174
// full-site parse.
174175
const siteSnapshot = await getLatestSnapshotForVersion(db, getPublishVersion())
175176
if (!siteSnapshot) return { kind: 'not-found' }
176-
return { kind: 'row', snapshot: siteSnapshot, row }
177+
// That snapshot carries no runtime manifest — an entry route takes the one
178+
// belonging to the template that actually renders it.
179+
const snapshot = await snapshotForEntryRoute(db, siteSnapshot, row.tableSlug)
180+
return { kind: 'row', snapshot, row }
177181
}
178182

179183
const redirect = await getDataRowRedirectByRoute(db, route.tableRouteBase, route.rowSlug)
@@ -320,8 +324,10 @@ export async function renderNotFoundResponse(
320324
return new Response(warm.body, { headers: warm.headers, status: 404 })
321325
}
322326

323-
const snapshot = await getLatestSnapshotForVersion(db, getPublishVersion())
324-
if (!snapshot || !resolveNotFoundTemplate(snapshot.site)) return null
327+
const siteSnapshot = await getLatestSnapshotForVersion(db, getPublishVersion())
328+
if (!siteSnapshot || !resolveNotFoundTemplate(siteSnapshot.site)) return null
329+
// Same as entry routes: the 404 template supplies its own runtime manifest.
330+
const snapshot = await snapshotForNotFoundRoute(db, siteSnapshot)
325331

326332
const syntheticUrl = new URL(NOT_FOUND_ARTEFACT_URL_PATH, url.origin)
327333
const cached = await getOrRender(cacheKey, async () => {

server/publish/publishRow.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
type PreviousPublishedRoute,
2727
} from '../repositories/data/publish'
2828
import { getLatestPublishedSiteSnapshot } from '../repositories/publish'
29+
import { snapshotForEntryRoute } from './entryTemplateSnapshot'
2930
import { renderPublishedDataRowTemplate } from './publicRenderer'
3031
import { applyPublishedHtmlPipeline } from './publishedHtmlPipeline'
3132
import { removeArtefactInPlace, updateArtefactInPlace } from './staticArtefact'
@@ -132,7 +133,10 @@ async function writeDataRowArtefact(
132133

133134
const newPath = publicDataPath(tableInfo.tableRouteBase, publishedRow.slug)
134135
const syntheticUrl = new URL(`http://localhost${newPath}`)
135-
const rendered = await renderPublishedDataRowTemplate(siteSnapshot, publishedDataRow, {
136+
// Runtime assets come from this table's entry template, not from the
137+
// arbitrary page the site-wide snapshot happens to name.
138+
const snapshot = await snapshotForEntryRoute(db, siteSnapshot, tableInfo.tableSlug)
139+
const rendered = await renderPublishedDataRowTemplate(snapshot, publishedDataRow, {
136140
db,
137141
url: syntheticUrl,
138142
publishVersion,

server/repositories/publish.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,13 +323,25 @@ export async function getPublishedPageSnapshotById(
323323
return rows[0] ? snapshotFromQueryRow(rows[0]) : null
324324
}
325325

326+
/**
327+
* Any published page's snapshot, used purely as a carrier for `site_json` —
328+
* routes that are not themselves pages (entry routes, the 404) need the site
329+
* document to resolve their template chain.
330+
*
331+
* It deliberately does NOT carry runtime assets. Those are per-page, and the
332+
* arbitrary page this returns (the first created, per the `order by`) is
333+
* almost never the page that renders the request. Letting its
334+
* `runtime_assets_json` ride along meant every entry route on a site served
335+
* one unrelated page's scripts, with the scope predicate never consulted.
336+
* Callers needing a manifest resolve the page that actually renders and take
337+
* its own — see `server/publish/entryTemplateSnapshot.ts`.
338+
*/
326339
export async function getLatestPublishedSiteSnapshot(
327340
db: DbClient,
328341
): Promise<PublishedPageSnapshot | null> {
329342
const { rows } = await db<SnapshotQueryRow>`
330343
select data_rows.id as row_id,
331344
site_snapshots.site_json,
332-
data_row_versions.runtime_assets_json,
333345
site_snapshots.importmap_body,
334346
site_snapshots.importmap_sha256
335347
from data_rows
@@ -341,5 +353,6 @@ export async function getLatestPublishedSiteSnapshot(
341353
order by data_rows.created_at asc
342354
limit 1
343355
`
344-
return rows[0] ? snapshotFromQueryRow(rows[0]) : null
356+
const row = rows[0]
357+
return row ? snapshotFromQueryRow({ ...row, runtime_assets_json: null }) : null
345358
}

src/__tests__/data/contentAdmin.test.tsx

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,78 @@ describe('ContentPage', () => {
970970
expect(params.get('row')).toBe('article_2')
971971
})
972972

973+
it('publishing another document does not steal the active document', async () => {
974+
// `applyStatus` used to call `updateSelectedEntry` for whatever row it
975+
// published, active or not. That retargeted the workspace — discarding the
976+
// author's unsaved draft — and left a tool loop of
977+
// `set_document_fields → set_document_status` writing one document behind
978+
// itself, so every field write after the first was refused.
979+
const postA = makeRow('post_a', 'posts', { title: 'Open post', slug: 'open-post', seoTitle: '' })
980+
const postB = makeRow('post_b', 'posts', { title: 'Other post', slug: 'other-post', seoTitle: '' })
981+
982+
globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit) => {
983+
const url = String(input)
984+
const method = init?.method ?? 'GET'
985+
986+
if (url === '/admin/api/cms/data/tables' && method === 'GET') {
987+
return json({ tables: [makeTable('posts', 'Posts', 'posts', '/posts', 'Post', 'Posts')] })
988+
}
989+
if (url === '/admin/api/cms/data/tables/posts/rows' && method === 'GET') {
990+
return json({ rows: [postA, postB] })
991+
}
992+
if (url === '/admin/api/cms/data/rows/post_a' && method === 'GET') return json({ row: postA })
993+
if (url === '/admin/api/cms/data/rows/post_b' && method === 'GET') return json({ row: postB })
994+
if (url === '/admin/api/cms/data/rows/post_a' && method === 'PATCH') {
995+
const body = JSON.parse(String(init?.body))
996+
return json({ row: makeRow('post_a', 'posts', body.cells) })
997+
}
998+
if (url === '/admin/api/cms/data/rows/post_b/publish' && method === 'POST') {
999+
return json({ row: { ...postB, status: 'published', publishedAt: '2026-05-01T10:02:00.000Z' } })
1000+
}
1001+
if (url === '/admin/api/cms/data/authors' && method === 'GET') return json({ authors: [] })
1002+
if (url === '/admin/api/cms/media' && method === 'GET') return json({ assets: [] })
1003+
1004+
const ambient = ambientFetchFallback(url)
1005+
if (ambient) return ambient
1006+
return json({ error: `Unhandled ${method} ${url}` }, 500)
1007+
}
1008+
1009+
render(
1010+
<AdminTestProviders>
1011+
<ContentPage />
1012+
</AdminTestProviders>,
1013+
)
1014+
expect(await screen.findByRole('region', { name: 'Posts' })).toBeDefined()
1015+
1016+
// Each call gets its own act() so React commits in between and the bridge's
1017+
// workspace ref refreshes. Batching them hides the bug: the ref would still
1018+
// hold the pre-publish workspace and the last write would pass either way.
1019+
let statusResult: Awaited<ReturnType<typeof executeContentTool>> | null = null
1020+
let writeResult: Awaited<ReturnType<typeof executeContentTool>> | null = null
1021+
await act(async () => {
1022+
await executeContentTool('content_set_active_document', { documentId: 'post_a' })
1023+
})
1024+
await act(async () => {
1025+
// Publish the OTHER document…
1026+
statusResult = await executeContentTool('content_set_document_status', {
1027+
documentId: 'post_b',
1028+
status: 'published',
1029+
})
1030+
})
1031+
await act(async () => {
1032+
// …post_a must still be the active document, so this write must land.
1033+
writeResult = await executeContentTool('content_set_document_field', {
1034+
documentId: 'post_a',
1035+
fieldId: 'seoTitle',
1036+
value: 'Still mine',
1037+
})
1038+
})
1039+
1040+
expect(statusResult?.ok).toBe(true)
1041+
expect(writeResult?.ok).toBe(true)
1042+
expect(String(writeResult?.error ?? '')).not.toContain('not the active doc')
1043+
})
1044+
9731045
it('uses content-specific rail panels instead of editor-only panels', async () => {
9741046
render(
9751047
<AdminTestProviders>
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/**
2+
* Entry routes take their runtime manifest from the template that renders
3+
* them, not from whatever page the site-wide snapshot happens to name.
4+
*
5+
* `getLatestPublishedSiteSnapshot` is `order by data_rows.created_at asc limit
6+
* 1` — the first published page ever created. It exists to carry `site_json`,
7+
* and its `runtime_assets_json` used to ride along. Entry routes are rendered
8+
* from a template plus a data row rather than from a page, so they inherited
9+
* that manifest wholesale: every entry route on a site served one arbitrary
10+
* page's scripts, and `assetScopeAppliesToPage` was never consulted on the
11+
* path at all.
12+
*
13+
* Over-inclusion was what surfaced in practice — a script scoped to two pages
14+
* shipping on ten entry routes — but the same bug under-includes just as
15+
* easily: if the oldest page carries no scripts, a scoped script reaches
16+
* nothing.
17+
*/
18+
import { describe, expect, it } from 'bun:test'
19+
import type { DbClient } from '../../../server/db'
20+
import type { PublishedPageSnapshot } from '../../../server/repositories/publish'
21+
import { snapshotForEntryRoute } from '../../../server/publish/entryTemplateSnapshot'
22+
import { makeSite } from '../fixtures'
23+
24+
const postsTarget = { kind: 'postTypes' as const, tableSlugs: ['posts'] }
25+
26+
function runtimeAssets(name: string) {
27+
return {
28+
scripts: [{ publicPath: `/_instatic/assets/${name}/classic/001-${name}.js`, placement: 'body-end' as const }],
29+
}
30+
}
31+
32+
/** A site whose first page is unrelated and whose second is the entry template. */
33+
function siteSnapshot(): PublishedPageSnapshot {
34+
const site = makeSite()
35+
site.pages[0].id = 'oldest-page'
36+
site.pages.push({
37+
...structuredClone(site.pages[0]),
38+
id: 'post-template',
39+
slug: 'post-template',
40+
template: { enabled: true, target: postsTarget, priority: 10 },
41+
})
42+
return { cmsSnapshotVersion: 1, pageRowId: 'oldest-page', site }
43+
}
44+
45+
/** Stands in for the DB: only `getPublishedPageSnapshotById` is reached. */
46+
function dbReturning(byPageId: Record<string, unknown>): DbClient {
47+
const fake = (async (_strings: TemplateStringsArray, ...params: unknown[]) => {
48+
const pageId = String(params[0])
49+
const assets = byPageId[pageId]
50+
return assets
51+
? { rows: [{ row_id: pageId, site_json: makeSite(), runtime_assets_json: assets }], rowCount: 1 }
52+
: { rows: [], rowCount: 0 }
53+
}) as unknown as DbClient
54+
return fake
55+
}
56+
57+
describe('entry-route runtime manifest', () => {
58+
it('uses the entry template\'s own manifest, not the site snapshot\'s', async () => {
59+
const snapshot = siteSnapshot()
60+
const db = dbReturning({ 'post-template': runtimeAssets('template') })
61+
62+
const resolved = await snapshotForEntryRoute(db, snapshot, 'posts')
63+
64+
expect(resolved.runtimeAssets?.scripts[0]?.publicPath).toContain('001-template.js')
65+
})
66+
67+
it('serves no scripts when the template has none, rather than another page\'s', async () => {
68+
// Reproduce the old shape: a site snapshot already carrying the oldest
69+
// page's manifest, and a template with none of its own. The old code
70+
// passed that straight through, so every entry route on the site served
71+
// `001-oldest.js`.
72+
const snapshot = { ...siteSnapshot(), runtimeAssets: runtimeAssets('oldest') }
73+
const db = dbReturning({})
74+
75+
const resolved = await snapshotForEntryRoute(db, snapshot, 'posts')
76+
77+
expect(resolved.runtimeAssets?.scripts[0]?.publicPath ?? '').not.toContain('001-oldest.js')
78+
})
79+
80+
it('overrides a stale manifest on the site snapshot with the template\'s', async () => {
81+
const snapshot = { ...siteSnapshot(), runtimeAssets: runtimeAssets('oldest') }
82+
const db = dbReturning({ 'post-template': runtimeAssets('template') })
83+
84+
const resolved = await snapshotForEntryRoute(db, snapshot, 'posts')
85+
86+
expect(resolved.runtimeAssets?.scripts[0]?.publicPath).toContain('001-template.js')
87+
})
88+
89+
it('leaves the site document untouched so the resolved chain still applies', async () => {
90+
const snapshot = siteSnapshot()
91+
const db = dbReturning({ 'post-template': runtimeAssets('template') })
92+
93+
const resolved = await snapshotForEntryRoute(db, snapshot, 'posts')
94+
95+
expect(resolved.site).toBe(snapshot.site)
96+
})
97+
98+
it('falls back to the site snapshot when the table has no entry template', async () => {
99+
const snapshot = siteSnapshot()
100+
const db = dbReturning({ 'post-template': runtimeAssets('template') })
101+
102+
const resolved = await snapshotForEntryRoute(db, snapshot, 'no-such-table')
103+
104+
expect(resolved).toBe(snapshot)
105+
})
106+
})

0 commit comments

Comments
 (0)