From 1517af0ea0e56b3c27513e04aeb09afe7d3a6645 Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Thu, 20 Aug 2026 19:07:57 -0400 Subject: [PATCH 1/2] POST /_screenshot-card persists captures to MediaCache and returns served URLs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The endpoint stops being ephemeral-only. A capture of an indexed card persists under the same canonical identity the GET _screenshot/ DSL resolves — instance URL, canonical spec hash, the instance's current index generation — so a POST-published capture serves on the durable URL immediately, gated realms included (publish-by-POST). The response gains captures: [{name, url, width, height, deviceScaleFactor, base64?}] while the top-level base64/width/height keep mirroring captures[0], so current-shape requests stay byte-compatible; includeBase64: false opts out of the bytes ahead of the default flipping once callers migrate to URLs. A request whose identity already has a ledger entry answers from the store with zero render work (dimensions now recorded on the ledger so the mirror never decodes bytes), an edited card misses its stale entry and re-captures at the current generation, and the wait is bounded like the GET DSL: over budget answers 503 + Retry-After while the job persists its capture anyway, so the retry is a pure ledger hit. Cards the index doesn't know still capture with the legacy response shape. Co-Authored-By: Claude Fable 5 --- packages/base/realm-config.gts | 8 +- ...63_schema.sql => 1787266260881_schema.sql} | 2 + ...60881_add-media-cache-ledger-dimensions.js | 17 + .../handlers/handle-screenshot-card.ts | 276 +++++++++++++++- packages/realm-server/main.ts | 1 + packages/realm-server/routes.ts | 8 + packages/realm-server/server.ts | 8 + .../tests/screenshot-card-test.ts | 305 +++++++++++++++++- packages/runtime-common/capture-spec.ts | 27 ++ packages/runtime-common/media-cache.ts | 39 +++ packages/runtime-common/realm.ts | 2 + .../runtime-common/tasks/screenshot-card.ts | 2 + 12 files changed, 679 insertions(+), 16 deletions(-) rename packages/host/config/schema/{1787258803063_schema.sql => 1787266260881_schema.sql} (99%) create mode 100644 packages/postgres/migrations/1787266260881_add-media-cache-ledger-dimensions.js diff --git a/packages/base/realm-config.gts b/packages/base/realm-config.gts index e34ca317c79..5c8c3ab70f3 100644 --- a/packages/base/realm-config.gts +++ b/packages/base/realm-config.gts @@ -689,9 +689,11 @@ export class RealmConfig extends CardDef { // unbounded spec space reachable with only realm read, so it is off // unless the realm turns it on; the gate blocks Chrome work only, never // serving — any capture whose canonical spec already has a MediaCache - // ledger entry streams regardless. Read from the realm's indexed config - // at request time, so editing this takes effect with the index update, - // no restart. + // ledger entry streams regardless, including one a write-holder published + // via POST /_screenshot-card (which persists under the same canonical + // identity the GET resolves). Read from the realm's indexed config at + // request time, so editing this takes effect with the index update, no + // restart. @field allowArbitraryScreenshots = contains(BooleanField); @field cardTitle = contains(StringField, { diff --git a/packages/host/config/schema/1787258803063_schema.sql b/packages/host/config/schema/1787266260881_schema.sql similarity index 99% rename from packages/host/config/schema/1787258803063_schema.sql rename to packages/host/config/schema/1787266260881_schema.sql index 9d88dcae222..420b0c59f94 100644 --- a/packages/host/config/schema/1787258803063_schema.sql +++ b/packages/host/config/schema/1787266260881_schema.sql @@ -88,6 +88,8 @@ size_bytes NOT NULL, created_at NOT NULL, last_accessed_at NOT NULL, + width INTEGER, + height INTEGER, PRIMARY KEY ( realm_url, source_url, capture_spec_hash, source_generation ) ); diff --git a/packages/postgres/migrations/1787266260881_add-media-cache-ledger-dimensions.js b/packages/postgres/migrations/1787266260881_add-media-cache-ledger-dimensions.js new file mode 100644 index 00000000000..ec7922b891f --- /dev/null +++ b/packages/postgres/migrations/1787266260881_add-media-cache-ledger-dimensions.js @@ -0,0 +1,17 @@ +exports.shorthands = undefined; + +// Pixel dimensions of the stored capture, recorded so a serving path that +// answers from the ledger (the POST endpoint's ledger-hit response mirrors +// the capture's width/height) never has to decode the image bytes. Nullable: +// rows written before dimensions were recorded simply lack them. + +exports.up = (pgm) => { + pgm.addColumns('media_cache_ledger', { + width: { type: 'integer' }, + height: { type: 'integer' }, + }); +}; + +exports.down = (pgm) => { + pgm.dropColumns('media_cache_ledger', ['width', 'height']); +}; diff --git a/packages/realm-server/handlers/handle-screenshot-card.ts b/packages/realm-server/handlers/handle-screenshot-card.ts index 19a6caae9fb..39752316c23 100644 --- a/packages/realm-server/handlers/handle-screenshot-card.ts +++ b/packages/realm-server/handlers/handle-screenshot-card.ts @@ -1,7 +1,22 @@ import type Koa from 'koa'; -import { isCaptureFormat } from '@cardstack/runtime-common'; -import { enqueueScreenshotCardJob } from '@cardstack/runtime-common/jobs/screenshot-card'; +import { + captureSpecHash, + ensureTrailingSlash, + findLiveInstanceGeneration, + findMediaCacheEntry, + isCaptureFormat, + screenshotURLFor, + type CaptureSpec, + type MediaCacheEntry, + type MediaCacheEntryKey, + type ScreenshotPrerenderResponse, +} from '@cardstack/runtime-common'; +import { + enqueueScreenshotCardJob, + estimateScreenshotQueueWait, + SCREENSHOT_SYNC_WAIT_BUDGET_MS, +} from '@cardstack/runtime-common/jobs/screenshot-card'; import { userInitiatedPriority } from '@cardstack/runtime-common/queue'; import { @@ -13,13 +28,46 @@ import { import type { CreateRoutesArgs } from '../routes.ts'; import type { RealmServerTokenClaim } from '../utils/jwt.ts'; +// One entry of the response's `captures` array: the durable served URL is +// the reference callers should embed (a re-capture rotates its bytes, never +// the URL); `base64` rides along by default until callers migrate to URLs +// (`includeBase64: false` opts out). `name` and `deviceScaleFactor` are +// null for ad-hoc captures — they populate for declared-screenshot batches +// and once the capture engine reports a scale factor. +interface CaptureResult { + name: string | null; + url: string; + width: number | null; + height: number | null; + deviceScaleFactor: number | null; + base64?: string; +} + /** * Handler for `POST /_screenshot-card`. * - * Enqueues a screenshot-card job via the queue system, waits for the result, - * and returns it. The job runs in a worker which calls - * `prerenderer.prerenderScreenshot(...)` after fetching the caller's - * realm permissions. + * Captures one card and persists the capture to the MediaCache under its + * canonical identity (instance URL × canonical capture spec × the + * instance's current index generation) — the same key the GET + * `_screenshot/` DSL resolves, so a capture published here serves on that + * route immediately, even on realms whose `allowArbitraryScreenshots` gate + * is closed (the gate blocks new GET-triggered captures, never serving). + * This endpoint itself is deliberately ungated: it is an authenticated + * surface with full captureSpec power under realm-read trust. + * + * A request whose canonical identity already has a ledger entry answers + * from the store with zero render work — which is also what lets a + * timed-out request resume: the job persists its capture even after the + * HTTP wait gives up (503 + Retry-After, bounded well under the ALB idle + * timeout), so the retry is a pure ledger hit. + * + * Response (201): `data.attributes` carries the raw capture fields — + * `status`, `base64`, `width`, `height`, `contentType` — for + * byte-compatibility with current-shape callers, plus `captures: + * [{name, url, width, height, deviceScaleFactor, base64?}]` when the + * capture persisted. A card the index doesn't know (or a server without a + * MediaCache store) still captures and returns the raw fields, just + * without `captures`. * * Request body (JSON:API): * ```json @@ -29,7 +77,8 @@ import type { RealmServerTokenClaim } from '../utils/jwt.ts'; * "attributes": { * "realmURL": "https://realm.example/user/workspace/", * "cardId": "https://realm.example/user/workspace/Person/fadhlan", - * "format": "isolated" + * "format": "isolated", + * "includeBase64": true * } * } * } @@ -40,6 +89,8 @@ import type { RealmServerTokenClaim } from '../utils/jwt.ts'; export default function handleScreenshotCard({ dbAdapter, queue, + mediaCacheAdapter, + screenshotSyncWaitMs = SCREENSHOT_SYNC_WAIT_BUDGET_MS, }: CreateRoutesArgs): (ctxt: Koa.Context, next: Koa.Next) => Promise { return async function (ctxt: Koa.Context, _next: Koa.Next) { let request = await fetchRequestFromContext(ctxt); @@ -58,7 +109,7 @@ export default function handleScreenshotCard({ ); } - let { realmURL, cardId, format } = attrs; + let { realmURL, cardId, format, includeBase64 } = attrs; if (!realmURL || typeof realmURL !== 'string') { return sendResponseForBadRequest(ctxt, 'realmURL is required'); } @@ -73,6 +124,19 @@ export default function handleScreenshotCard({ 'format must be "isolated" or "embedded"', ); } + if (includeBase64 !== undefined && typeof includeBase64 !== 'boolean') { + return sendResponseForBadRequest(ctxt, 'includeBase64 must be a boolean'); + } + let withBase64 = includeBase64 !== false; + let normalizedRealmURL = ensureTrailingSlash(realmURL); + // The persist identity (and the served URL) hang off the instance's + // location within its realm. + if (!cardId.startsWith(normalizedRealmURL)) { + return sendResponseForBadRequest(ctxt, 'cardId must be within realmURL'); + } + let spec: CaptureSpec = { format }; + let sourceURL = cardId.replace(/\.json$/, ''); + let instanceLocalPath = sourceURL.slice(normalizedRealmURL.length); let token = ctxt.state.token as RealmServerTokenClaim; if (!token?.user) { @@ -84,6 +148,45 @@ export default function handleScreenshotCard({ let userId = token.user; try { + // The canonical capture identity — resolvable only when the instance + // is indexed (the generation is part of the key) and this server has + // a store. Without either, the capture still runs; it just isn't + // persisted and the response carries no served URL. + let entryKey: MediaCacheEntryKey | undefined; + if (mediaCacheAdapter) { + let generation = await findLiveInstanceGeneration(dbAdapter, { + realmURL: normalizedRealmURL, + instanceURL: sourceURL, + }); + if (generation !== undefined) { + entryKey = { + realmURL: normalizedRealmURL, + sourceURL, + captureSpecHash: await captureSpecHash(spec), + sourceGeneration: generation, + }; + } + } + + if (entryKey) { + let entry = await findMediaCacheEntry(dbAdapter, entryKey); + if (entry) { + let response = await respondFromLedger({ + entry, + withBase64, + normalizedRealmURL, + instanceLocalPath, + spec, + mediaCacheAdapter: mediaCacheAdapter!, + }); + if (response) { + return await setContextResponse(ctxt, response); + } + // The entry's object was reclaimed between the ledger read and + // the stream open — fall through and re-capture. + } + } + let job = await enqueueScreenshotCardJob( { realmURL, @@ -91,14 +194,73 @@ export default function handleScreenshotCard({ runAs: userId, cardId, format, - persist: null, + persist: entryKey ? { ...entryKey, lane: 'on-demand' } : null, }, queue, dbAdapter, userInitiatedPriority, ); - let result = await job.done; + let timeoutHandle: ReturnType | undefined; + const timedOut = Symbol('sync-wait-timeout'); + let result: ScreenshotPrerenderResponse | typeof timedOut; + try { + result = await Promise.race([ + job.done, + new Promise((resolve) => { + timeoutHandle = setTimeout( + () => resolve(timedOut), + screenshotSyncWaitMs, + ); + timeoutHandle.unref?.(); + }), + ]); + } finally { + if (timeoutHandle) { + clearTimeout(timeoutHandle); + } + } + if (result === timedOut) { + // The job keeps running and (when persisting) lands its capture in + // the MediaCache, so the client's retry answers from the ledger + // with no second render. The retry hint is one average capture. + let estimate = await estimateScreenshotQueueWait( + dbAdapter, + `screenshot:${realmURL}`, + ); + return await setContextResponse( + ctxt, + new Response(null, { + status: 503, + headers: { + 'retry-after': String( + Math.max( + 1, + Math.ceil(Math.max(estimate.avgCaptureMs, 1000) / 1000), + ), + ), + }, + }), + ); + } + + let attributes: Record = { ...result }; + if (!withBase64) { + delete attributes.base64; + } + if (entryKey && result.status === 'ready') { + attributes.captures = [ + captureResult({ + withBase64, + base64: result.base64, + width: result.width ?? null, + height: result.height ?? null, + normalizedRealmURL, + instanceLocalPath, + spec, + }), + ]; + } await setContextResponse( ctxt, @@ -106,7 +268,7 @@ export default function handleScreenshotCard({ JSON.stringify({ data: { type: 'screenshot-card-result', - attributes: result, + attributes, }, }), { @@ -121,3 +283,95 @@ export default function handleScreenshotCard({ } }; } + +function captureResult({ + withBase64, + base64, + width, + height, + normalizedRealmURL, + instanceLocalPath, + spec, +}: { + withBase64: boolean; + base64: string | undefined; + width: number | null; + height: number | null; + normalizedRealmURL: string; + instanceLocalPath: string; + spec: CaptureSpec; +}): CaptureResult { + return { + name: null, + url: screenshotURLFor({ + realmURL: normalizedRealmURL, + instanceLocalPath, + spec, + }), + width, + height, + deviceScaleFactor: null, + ...(withBase64 && base64 !== undefined ? { base64 } : {}), + }; +} + +// A ledger hit answers with zero render work, in the same envelope a fresh +// capture produces. Returns undefined when the entry's object is gone from +// the store (reclaimed under a live row) so the caller re-captures. +async function respondFromLedger({ + entry, + withBase64, + normalizedRealmURL, + instanceLocalPath, + spec, + mediaCacheAdapter, +}: { + entry: MediaCacheEntry; + withBase64: boolean; + normalizedRealmURL: string; + instanceLocalPath: string; + spec: CaptureSpec; + mediaCacheAdapter: NonNullable; +}): Promise { + let base64: string | undefined; + if (withBase64) { + let stream = await mediaCacheAdapter.getStream(entry.objectKey); + if (!stream) { + return undefined; + } + let chunks: Buffer[] = []; + for await (let chunk of stream) { + chunks.push(Buffer.from(chunk)); + } + base64 = Buffer.concat(chunks).toString('base64'); + } else if (!(await mediaCacheAdapter.head(entry.objectKey))) { + return undefined; + } + let attributes: Record = { + status: 'ready', + width: entry.width, + height: entry.height, + contentType: entry.contentType, + ...(withBase64 ? { base64 } : {}), + captures: [ + captureResult({ + withBase64, + base64, + width: entry.width, + height: entry.height, + normalizedRealmURL, + instanceLocalPath, + spec, + }), + ], + }; + return new Response( + JSON.stringify({ + data: { type: 'screenshot-card-result', attributes }, + }), + { + status: 201, + headers: { 'Content-Type': 'application/vnd.api+json' }, + }, + ); +} diff --git a/packages/realm-server/main.ts b/packages/realm-server/main.ts index 0d1c2018b34..53f6697b8d3 100644 --- a/packages/realm-server/main.ts +++ b/packages/realm-server/main.ts @@ -668,6 +668,7 @@ const reportHostShellToManager = async () => { let server = new RealmServer({ realms, reconciler, + mediaCacheAdapter, virtualNetwork, matrixClient, realmsRootPath, diff --git a/packages/realm-server/routes.ts b/packages/realm-server/routes.ts index 8c13b159b21..82d0fa8f7e2 100644 --- a/packages/realm-server/routes.ts +++ b/packages/realm-server/routes.ts @@ -1,6 +1,7 @@ import type { DBAdapter, DefinitionLookup, + MediaCacheAdapter, QueuePublisher, Realm, VirtualNetwork, @@ -89,6 +90,13 @@ export type CreateRoutesArgs = { serverURL: string; dbAdapter: DBAdapter; definitionLookup: DefinitionLookup; + // MediaCache object store; absent means the POST screenshot endpoint + // captures without persisting (and returns no served URL). + mediaCacheAdapter?: MediaCacheAdapter; + // Bounded sync-wait budget for the POST screenshot endpoint. Defaults to + // SCREENSHOT_SYNC_WAIT_BUDGET_MS; tests shrink it to exercise the + // 503 + Retry-After path without holding real time. + screenshotSyncWaitMs?: number; matrixClient: MatrixClient; realmServerSecretSeed: string; grafanaSecret: string; diff --git a/packages/realm-server/server.ts b/packages/realm-server/server.ts index 9f423041ade..f4a72106a5d 100644 --- a/packages/realm-server/server.ts +++ b/packages/realm-server/server.ts @@ -11,6 +11,7 @@ import { type VirtualNetwork, type DBAdapter, type QueuePublisher, + type MediaCacheAdapter, DEFAULT_AUDIO_SIZE_LIMIT_BYTES, DEFAULT_CARD_SIZE_LIMIT_BYTES, DEFAULT_FILE_SIZE_LIMIT_BYTES, @@ -940,6 +941,7 @@ export class RealmServer { private dbAdapter: DBAdapter; private queue: QueuePublisher; private definitionLookup: DefinitionLookup; + private mediaCacheAdapter: MediaCacheAdapter | undefined; private assetsURL: URL; private getIndexHTML: () => Promise; private serverURL: URL; @@ -979,6 +981,7 @@ export class RealmServer { dbAdapter, queue, definitionLookup, + mediaCacheAdapter, assetsURL, getIndexHTML, matrixRegistrationSecret, @@ -1003,6 +1006,9 @@ export class RealmServer { dbAdapter: DBAdapter; queue: QueuePublisher; definitionLookup: DefinitionLookup; + // MediaCache object store shared with the realms this server mounts; + // absent means the POST screenshot endpoint captures without persisting. + mediaCacheAdapter?: MediaCacheAdapter; assetsURL: URL; getIndexHTML: () => Promise; matrixRegistrationSecret?: string; @@ -1056,6 +1062,7 @@ export class RealmServer { this.dbAdapter = dbAdapter; this.queue = queue; this.definitionLookup = definitionLookup; + this.mediaCacheAdapter = mediaCacheAdapter; this.assetsURL = assetsURL; this.getIndexHTML = getIndexHTML; this.matrixRegistrationSecret = matrixRegistrationSecret; @@ -1169,6 +1176,7 @@ export class RealmServer { createRoutes({ dbAdapter: this.dbAdapter, definitionLookup: this.definitionLookup, + mediaCacheAdapter: this.mediaCacheAdapter, serverURL: this.serverURL.href, matrixClient: this.matrixClient, realmServerSecretSeed: this.realmServerSecretSeed, diff --git a/packages/realm-server/tests/screenshot-card-test.ts b/packages/realm-server/tests/screenshot-card-test.ts index 78882c552ad..d7444036c80 100644 --- a/packages/realm-server/tests/screenshot-card-test.ts +++ b/packages/realm-server/tests/screenshot-card-test.ts @@ -4,7 +4,14 @@ import Koa from 'koa'; import Router from '@koa/router'; import supertest from 'supertest'; import { basename } from 'path'; -import { Deferred } from '@cardstack/runtime-common'; +import { + Deferred, + asExpressions, + captureSpecHash, + insert, + putMedia, + query, +} from '@cardstack/runtime-common'; import type { DBAdapter, QueuePublisher, @@ -13,12 +20,14 @@ import type { PgPrimitive, ScreenshotPrerenderResponse, } from '@cardstack/runtime-common'; +import type { PgAdapter } from '@cardstack/postgres'; import handleScreenshotCard from '../handlers/handle-screenshot-card.ts'; import type { CreateRoutesArgs } from '../routes.ts'; import { jwtMiddleware } from '../middleware/index.ts'; import { createJWT } from '../utils/jwt.ts'; -import { realmSecretSeed } from './helpers/index.ts'; +import { FakeMediaCacheAdapter } from './helpers/fake-media-cache-adapter.ts'; +import { realmSecretSeed, setupDB } from './helpers/index.ts'; module(basename(import.meta.filename), function () { module('/_screenshot-card endpoint', function () { @@ -252,5 +261,297 @@ module(basename(import.meta.filename), function () { ); assert.deepEqual(published, [], 'does not enqueue any job'); }); + + test('rejects a cardId outside the realm', async function (assert) { + let { queue, published } = makeQueue({ status: 'ready' }); + let app = buildApp(buildArgs(makeDbAdapter(), queue)); + let token = createJWT( + { user: '@someone:localhost', sessionRoom: '!room:localhost' }, + realmSecretSeed, + ); + + let response = await supertest(app.callback()) + .post('/_screenshot-card') + .set('Authorization', `Bearer ${token}`) + .send({ + data: { + attributes: { + realmURL: 'http://example.test/', + cardId: 'http://other.test/Person/fadhlan', + format: 'isolated', + }, + }, + }); + + assert.strictEqual(response.status, 400); + assert.ok(response.text.includes('cardId must be within realmURL')); + assert.deepEqual(published, [], 'does not enqueue any job'); + }); + }); + + module('/_screenshot-card persistence', function (hooks) { + const REALM_URL = 'http://example.test/'; + const CARD_ID = `${REALM_URL}Person/fadhlan`; + const PNG_BYTES = new TextEncoder().encode('stub-png-bytes'); + const PNG_BASE64 = Buffer.from(PNG_BYTES).toString('base64'); + const READY: ScreenshotPrerenderResponse = { + status: 'ready', + base64: PNG_BASE64, + width: 800, + height: 600, + contentType: 'image/png', + }; + + let dbAdapter: PgAdapter; + let adapter: FakeMediaCacheAdapter; + + setupDB(hooks, { + beforeEach: async (_dbAdapter: PgAdapter): Promise => { + dbAdapter = _dbAdapter; + adapter = new FakeMediaCacheAdapter(); + }, + }); + + function makePersistQueue(behavior: 'ready' | 'never'): { + queue: QueuePublisher; + published: Array>; + } { + let published: Array> = []; + let nextId = 1; + let queue: QueuePublisher = { + async publish( + args: QueuePublishArgs, + ): Promise> { + published.push(args as QueuePublishArgs); + let notifier = new Deferred(); + if (behavior === 'ready') { + notifier.fulfill(READY as unknown as TResult); + } + return { + id: nextId++, + get done() { + return notifier.promise; + }, + } as Job; + }, + async destroy() {}, + }; + return { queue, published }; + } + + function persistApp( + queue: QueuePublisher, + opts: { screenshotSyncWaitMs?: number } = {}, + ) { + let app = new Koa(); + let router = new Router(); + router.post( + '/_screenshot-card', + jwtMiddleware(realmSecretSeed, dbAdapter), + handleScreenshotCard({ + dbAdapter, + queue, + mediaCacheAdapter: adapter, + ...opts, + } as unknown as CreateRoutesArgs), + ); + app.use(router.routes()); + return app; + } + + async function seedInstanceRow(generation = 1) { + let { nameExpressions, valueExpressions } = asExpressions( + { + url: `${CARD_ID}.json`, + file_alias: CARD_ID, + realm_url: REALM_URL, + type: 'instance', + generation, + last_modified: Date.now(), + resource_created_at: Date.now(), + is_deleted: false, + pristine_doc: { attributes: {} }, + }, + { jsonFields: ['pristine_doc'] }, + ); + await query( + dbAdapter, + insert('boxel_index', nameExpressions, valueExpressions), + ); + } + + function post(app: Koa, attributes: Record) { + let token = createJWT( + { user: '@someone:localhost', sessionRoom: '!room:localhost' }, + realmSecretSeed, + ); + return supertest(app.callback()) + .post('/_screenshot-card') + .set('Authorization', `Bearer ${token}`) + .send({ data: { type: 'screenshot-card', attributes } }); + } + + test('a capture of an indexed card enqueues with the DSL-matching persist identity and returns a served URL', async function (assert) { + await seedInstanceRow(); + let { queue, published } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + assert.deepEqual((published[0]?.args as any)?.persist, { + realmURL: REALM_URL, + sourceURL: CARD_ID, + captureSpecHash: await captureSpecHash({ format: 'isolated' }), + sourceGeneration: 1, + lane: 'on-demand', + }); + + let attrs = response.body.data.attributes; + assert.strictEqual(attrs.status, 'ready'); + assert.strictEqual(attrs.base64, PNG_BASE64, 'top-level mirror intact'); + assert.strictEqual(attrs.width, 800); + assert.strictEqual(attrs.height, 600); + assert.deepEqual(attrs.captures, [ + { + name: null, + url: `${REALM_URL}_screenshot/Person/fadhlan`, + width: 800, + height: 600, + deviceScaleFactor: null, + base64: PNG_BASE64, + }, + ]); + }); + + test('a non-default format shows up in the served URL', async function (assert) { + await seedInstanceRow(); + let { queue } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'embedded', + }).expect(201); + + assert.strictEqual( + response.body.data.attributes.captures[0].url, + `${REALM_URL}_screenshot/Person/fadhlan?format=embedded`, + ); + }); + + test('includeBase64: false omits the bytes everywhere', async function (assert) { + await seedInstanceRow(); + let { queue } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + includeBase64: false, + }).expect(201); + + let attrs = response.body.data.attributes; + assert.false('base64' in attrs, 'no top-level base64'); + assert.false('base64' in attrs.captures[0], 'no per-capture base64'); + assert.strictEqual(attrs.width, 800, 'dimensions still mirror'); + }); + + test('a ledger hit answers with zero render work', async function (assert) { + await seedInstanceRow(); + await putMedia(dbAdapter, adapter, { + realmURL: REALM_URL, + sourceURL: CARD_ID, + captureSpecHash: await captureSpecHash({ format: 'isolated' }), + sourceGeneration: 1, + bytes: PNG_BYTES, + contentType: 'image/png', + lane: 'on-demand', + width: 800, + height: 600, + }); + let { queue, published } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + assert.deepEqual(published, [], 'no job was enqueued'); + let attrs = response.body.data.attributes; + assert.strictEqual(attrs.status, 'ready'); + assert.strictEqual(attrs.base64, PNG_BASE64, 'bytes come from the store'); + assert.strictEqual(attrs.width, 800); + assert.strictEqual(attrs.height, 600); + assert.strictEqual( + attrs.captures[0].url, + `${REALM_URL}_screenshot/Person/fadhlan`, + ); + }); + + test('an edited card misses the stale ledger entry and re-captures', async function (assert) { + await seedInstanceRow(2); + // A capture of generation 1 exists, but the instance has moved on. + await putMedia(dbAdapter, adapter, { + realmURL: REALM_URL, + sourceURL: CARD_ID, + captureSpecHash: await captureSpecHash({ format: 'isolated' }), + sourceGeneration: 1, + bytes: PNG_BYTES, + contentType: 'image/png', + lane: 'on-demand', + }); + let { queue, published } = makePersistQueue('ready'); + + await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + assert.strictEqual(published.length, 1, 'a fresh capture was enqueued'); + assert.strictEqual( + ((published[0]?.args as any)?.persist as { sourceGeneration: number }) + .sourceGeneration, + 2, + 'the persist identity carries the current generation', + ); + }); + + test('a wait that outruns the budget answers 503 + Retry-After', async function (assert) { + await seedInstanceRow(); + let { queue, published } = makePersistQueue('never'); + + let response = await post( + persistApp(queue, { screenshotSyncWaitMs: 50 }), + { realmURL: REALM_URL, cardId: CARD_ID, format: 'isolated' }, + ); + + assert.strictEqual(response.status, 503); + assert.ok(Number(response.headers['retry-after']) >= 1); + assert.strictEqual( + published.length, + 1, + 'the capture is in flight; its job persists the result for the retry', + ); + }); + + test('an unindexed card still captures, without persisting', async function (assert) { + let { queue, published } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + assert.strictEqual((published[0]?.args as any)?.persist, null); + let attrs = response.body.data.attributes; + assert.strictEqual(attrs.base64, PNG_BASE64, 'legacy shape intact'); + assert.false('captures' in attrs, 'no served URL without a persist'); + }); }); }); diff --git a/packages/runtime-common/capture-spec.ts b/packages/runtime-common/capture-spec.ts index ad292db1d2a..56d10b90340 100644 --- a/packages/runtime-common/capture-spec.ts +++ b/packages/runtime-common/capture-spec.ts @@ -96,3 +96,30 @@ export async function captureSpecHash(spec: CaptureSpec): Promise { new TextEncoder().encode(canonicalCaptureSpecString(spec)), ); } + +// The spec's canonical query string — '' for the all-defaults spec — so a +// served URL round-trips through `parseCaptureSpecParams` back to the same +// canonical form. +export function canonicalCaptureSpecQuery(spec: CaptureSpec): string { + let searchParams = new URLSearchParams(); + if (spec.format !== DEFAULT_CAPTURE_FORMAT) { + searchParams.set('format', spec.format); + } + let qs = searchParams.toString(); + return qs.length > 0 ? `?${qs}` : ''; +} + +// The durable served URL for one capture of one instance: the platform's +// only public screenshot URL form. A re-capture changes what this URL +// serves, never the URL itself. +export function screenshotURLFor({ + realmURL, + instanceLocalPath, + spec, +}: { + realmURL: string; + instanceLocalPath: string; + spec: CaptureSpec; +}): string { + return `${realmURL}_screenshot/${instanceLocalPath}${canonicalCaptureSpecQuery(spec)}`; +} diff --git a/packages/runtime-common/media-cache.ts b/packages/runtime-common/media-cache.ts index 7b3fa54eab5..70fb6632b67 100644 --- a/packages/runtime-common/media-cache.ts +++ b/packages/runtime-common/media-cache.ts @@ -85,6 +85,11 @@ export interface MediaCacheEntry extends MediaCacheEntryKey { lane: MediaCacheLane; contentType: string; sizeBytes: number; + // Pixel dimensions of the capture, recorded so serving paths that answer + // from the ledger never decode the bytes; null when the capture engine + // didn't report them. + width: number | null; + height: number | null; createdAt: number; lastAccessedAt: number; } @@ -129,11 +134,15 @@ export async function putMedia( sourceGeneration, sourceContentHash = null, lane, + width = null, + height = null, }: MediaCacheEntryKey & { bytes: Uint8Array; contentType: string; lane: MediaCacheLane; sourceContentHash?: string | null; + width?: number | null; + height?: number | null; }, ): Promise<{ objectKey: string; sizeBytes: number }> { let objectKey = await computeMediaCacheKey(bytes); @@ -160,6 +169,8 @@ export async function putMedia( lane, content_type: contentType, size_bytes: bytes.length, + width, + height, created_at: now, last_accessed_at: now, }); @@ -266,6 +277,8 @@ export async function findMediaCacheEntry( lane: MediaCacheLane; content_type: string; size_bytes: number | string; + width: number | null; + height: number | null; created_at: number | string; last_accessed_at: number | string; }[]; @@ -283,11 +296,37 @@ export async function findMediaCacheEntry( lane: row.lane, contentType: row.content_type, sizeBytes: Number(row.size_bytes), + width: row.width == null ? null : Number(row.width), + height: row.height == null ? null : Number(row.height), createdAt: Number(row.created_at), lastAccessedAt: Number(row.last_accessed_at), }; } +// The generation of a live indexed instance, addressable by either its +// extensionless card-id URL or its `.json` file URL — the capture-identity +// resolution a caller needs before it can compute a MediaCache key. Returns +// undefined when the instance is absent or tombstoned. +export async function findLiveInstanceGeneration( + dbAdapter: DBAdapter, + { realmURL, instanceURL }: { realmURL: string; instanceURL: string }, +): Promise { + let rows = (await query(dbAdapter, [ + `SELECT generation FROM boxel_index + WHERE (url =`, + param(instanceURL), + `OR file_alias =`, + param(instanceURL), + `) AND realm_url =`, + param(realmURL), + `AND type = 'instance' + AND (is_deleted = FALSE OR is_deleted IS NULL) + LIMIT 1`, + ] as Expression)) as { generation: number | string }[]; + let row = rows[0]; + return row == null ? undefined : Number(row.generation); +} + // --------------------------------------------------------------------------- // GC sweep read side — mirrors `prerender-html-reconcile.ts`: pure queries // plus a pure planning step; the enqueue/delete orchestration lives in the diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index 98fa5c1601b..a83ace7c45c 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -4263,6 +4263,8 @@ export class Realm { ...entryKey, bytes, contentType: outcome.contentType ?? 'image/png', + width: outcome.width ?? null, + height: outcome.height ?? null, lane: 'on-demand', }); entry = await findMediaCacheEntry(this.#dbAdapter, entryKey); diff --git a/packages/runtime-common/tasks/screenshot-card.ts b/packages/runtime-common/tasks/screenshot-card.ts index baafbcb8aef..4b1dd5caf23 100644 --- a/packages/runtime-common/tasks/screenshot-card.ts +++ b/packages/runtime-common/tasks/screenshot-card.ts @@ -130,6 +130,8 @@ const screenshotCard: Task = ({ ...persist, bytes, contentType: response.contentType ?? 'image/png', + width: response.width ?? null, + height: response.height ?? null, }); } catch (e: any) { log.error( From 406e77991d76098e09a70730a6ce5495d19f6739 Mon Sep 17 00:00:00 2001 From: Luke Melia Date: Mon, 24 Aug 2026 17:59:11 -0400 Subject: [PATCH 2/2] Address review: gate the ledger fast path on realm read, unify the liveness predicate, key twins by runAs - The POST ledger fast path (and the generation probe feeding it) now requires realm read for the JWT user, checked the way the realm itself checks it (exact rows plus * and users grants). Callers without read go straight to the render path, whose permissions the worker enforces. - findLiveInstanceGeneration shares the engine's liveness predicate (prerenderedJoin + effectiveHasError, now exported) so an effective-error instance can no longer persist a capture the GET route answers as a miss. - A POST ledger hit bumps last_accessed_at through the same guarded touch the GET serve uses, so POST-only consumers stay out of the GC's on-demand age-out. - The twin probe behind hasTwin matches runAs along with the persist target, mirroring the coalesce key, so a pending job under another render identity no longer waves a request past the congestion gate. - The enqueue, its concurrency group, and the retry estimate use the canonical realm URL, and cardId/realmURL go through URL normalization before the containment check and persist-identity derivation. Co-Authored-By: Claude Fable 5 --- .../handlers/handle-screenshot-card.ts | 89 ++++++-- .../tests/screenshot-card-test.ts | 208 +++++++++++++++++- packages/runtime-common/index-query-engine.ts | 9 +- .../runtime-common/jobs/screenshot-card.ts | 48 ++-- .../runtime-common/media-cache-serving.ts | 6 +- packages/runtime-common/media-cache.ts | 24 +- packages/runtime-common/realm.ts | 12 +- 7 files changed, 342 insertions(+), 54 deletions(-) diff --git a/packages/realm-server/handlers/handle-screenshot-card.ts b/packages/realm-server/handlers/handle-screenshot-card.ts index 39752316c23..8efd9d83dd4 100644 --- a/packages/realm-server/handlers/handle-screenshot-card.ts +++ b/packages/realm-server/handlers/handle-screenshot-card.ts @@ -3,15 +3,19 @@ import type Koa from 'koa'; import { captureSpecHash, ensureTrailingSlash, + fetchRealmPermissions, findLiveInstanceGeneration, findMediaCacheEntry, isCaptureFormat, screenshotURLFor, + touchMediaCacheEntryOnHit, type CaptureSpec, + type DBAdapter, type MediaCacheEntry, type MediaCacheEntryKey, type ScreenshotPrerenderResponse, } from '@cardstack/runtime-common'; +import RealmPermissionChecker from '@cardstack/runtime-common/realm-permission-checker'; import { enqueueScreenshotCardJob, estimateScreenshotQueueWait, @@ -52,8 +56,11 @@ interface CaptureResult { * `_screenshot/` DSL resolves, so a capture published here serves on that * route immediately, even on realms whose `allowArbitraryScreenshots` gate * is closed (the gate blocks new GET-triggered captures, never serving). - * This endpoint itself is deliberately ungated: it is an authenticated - * surface with full captureSpec power under realm-read trust. + * This endpoint skips that gate deliberately: it is an authenticated + * surface with full captureSpec power under realm-read trust. Realm read is + * enforced in two places: the ledger fast path (and the generation probe + * feeding it) checks it here, since it answers before any job exists; the + * render path relies on the worker task's permission check. * * A request whose canonical identity already has a ledger entry answers * from the store with zero render work — which is also what lets a @@ -89,6 +96,7 @@ interface CaptureResult { export default function handleScreenshotCard({ dbAdapter, queue, + matrixClient, mediaCacheAdapter, screenshotSyncWaitMs = SCREENSHOT_SYNC_WAIT_BUDGET_MS, }: CreateRoutesArgs): (ctxt: Koa.Context, next: Koa.Next) => Promise { @@ -128,14 +136,30 @@ export default function handleScreenshotCard({ return sendResponseForBadRequest(ctxt, 'includeBase64 must be a boolean'); } let withBase64 = includeBase64 !== false; - let normalizedRealmURL = ensureTrailingSlash(realmURL); + // Both URLs go through `new URL` resolution (dot segments, + // percent-encoding, default ports) before anything derives from them: + // the containment check below must mean real containment (a dotted + // `cardId` prefixed with the realm URL escapes a plain string-prefix + // test), and the persist identity must key an instance exactly the way + // the GET route's `paths.fileURL` derivation does. + let normalizedRealmURL: string; + let normalizedCardId: string; + try { + normalizedRealmURL = ensureTrailingSlash(new URL(realmURL).href); + normalizedCardId = new URL(cardId).href; + } catch { + return sendResponseForBadRequest( + ctxt, + 'realmURL and cardId must be valid absolute URLs', + ); + } // The persist identity (and the served URL) hang off the instance's // location within its realm. - if (!cardId.startsWith(normalizedRealmURL)) { + if (!normalizedCardId.startsWith(normalizedRealmURL)) { return sendResponseForBadRequest(ctxt, 'cardId must be within realmURL'); } let spec: CaptureSpec = { format }; - let sourceURL = cardId.replace(/\.json$/, ''); + let sourceURL = normalizedCardId.replace(/\.json$/, ''); let instanceLocalPath = sourceURL.slice(normalizedRealmURL.length); let token = ctxt.state.token as RealmServerTokenClaim; @@ -154,17 +178,35 @@ export default function handleScreenshotCard({ // persisted and the response carries no served URL. let entryKey: MediaCacheEntryKey | undefined; if (mediaCacheAdapter) { - let generation = await findLiveInstanceGeneration(dbAdapter, { - realmURL: normalizedRealmURL, - instanceURL: sourceURL, - }); - if (generation !== undefined) { - entryKey = { + // The ledger fast path and the generation probe feeding it answer + // from the store before any job exists, so the worker task's + // permission check never covers them — realm read is enforced here + // instead (ahead of the probe, which alone would leak instance + // existence on a private realm). Read is checked the way the realm + // itself checks it — exact rows plus the `*` and `users` grants. A + // caller without read goes straight to the render path, whose + // permissions the worker enforces, and never persists. + let permissions = await fetchRealmPermissions( + dbAdapter, + new URL(normalizedRealmURL), + ); + let mayRead = await new RealmPermissionChecker( + permissions, + matrixClient, + ).can(userId, 'read'); + if (mayRead) { + let generation = await findLiveInstanceGeneration(dbAdapter, { realmURL: normalizedRealmURL, - sourceURL, - captureSpecHash: await captureSpecHash(spec), - sourceGeneration: generation, - }; + instanceURL: sourceURL, + }); + if (generation !== undefined) { + entryKey = { + realmURL: normalizedRealmURL, + sourceURL, + captureSpecHash: await captureSpecHash(spec), + sourceGeneration: generation, + }; + } } } @@ -178,6 +220,7 @@ export default function handleScreenshotCard({ instanceLocalPath, spec, mediaCacheAdapter: mediaCacheAdapter!, + dbAdapter, }); if (response) { return await setContextResponse(ctxt, response); @@ -187,12 +230,15 @@ export default function handleScreenshotCard({ } } + // The canonical realm URL keys the per-realm serialization lane (the + // job's default concurrency group) so this surface and the GET lane — + // which keys off the realm's own URL — share one lane per realm. let job = await enqueueScreenshotCardJob( { - realmURL, + realmURL: normalizedRealmURL, realmUsername: userId, runAs: userId, - cardId, + cardId: normalizedCardId, format, persist: entryKey ? { ...entryKey, lane: 'on-demand' } : null, }, @@ -226,7 +272,7 @@ export default function handleScreenshotCard({ // with no second render. The retry hint is one average capture. let estimate = await estimateScreenshotQueueWait( dbAdapter, - `screenshot:${realmURL}`, + `screenshot:${normalizedRealmURL}`, ); return await setContextResponse( ctxt, @@ -325,6 +371,7 @@ async function respondFromLedger({ instanceLocalPath, spec, mediaCacheAdapter, + dbAdapter, }: { entry: MediaCacheEntry; withBase64: boolean; @@ -332,6 +379,7 @@ async function respondFromLedger({ instanceLocalPath: string; spec: CaptureSpec; mediaCacheAdapter: NonNullable; + dbAdapter: DBAdapter; }): Promise { let base64: string | undefined; if (withBase64) { @@ -347,6 +395,11 @@ async function respondFromLedger({ } else if (!(await mediaCacheAdapter.head(entry.objectKey))) { return undefined; } + // A hit consumed through this endpoint is a use like any GET serve: the + // same guarded bump (on-demand lane only, hourly-throttled) keeps a + // capture refreshed exclusively via POST from aging out of the GC's + // idle TTL while in active use. + await touchMediaCacheEntryOnHit(dbAdapter, entry); let attributes: Record = { status: 'ready', width: entry.width, diff --git a/packages/realm-server/tests/screenshot-card-test.ts b/packages/realm-server/tests/screenshot-card-test.ts index d7444036c80..be9f30441cb 100644 --- a/packages/realm-server/tests/screenshot-card-test.ts +++ b/packages/realm-server/tests/screenshot-card-test.ts @@ -9,9 +9,12 @@ import { asExpressions, captureSpecHash, insert, + insertPermissions, + param, putMedia, query, } from '@cardstack/runtime-common'; +import { estimateScreenshotQueueWait } from '@cardstack/runtime-common/jobs/screenshot-card'; import type { DBAdapter, QueuePublisher, @@ -20,6 +23,7 @@ import type { PgPrimitive, ScreenshotPrerenderResponse, } from '@cardstack/runtime-common'; +import type { MatrixClient } from '@cardstack/runtime-common/matrix-client'; import type { PgAdapter } from '@cardstack/postgres'; import handleScreenshotCard from '../handlers/handle-screenshot-card.ts'; @@ -304,11 +308,24 @@ module(basename(import.meta.filename), function () { let dbAdapter: PgAdapter; let adapter: FakeMediaCacheAdapter; + // The permission checker consults the matrix profile only for realms + // with a `users` grant; these tests seed exact-user rows, so the stub + // is never called. + let matrixClient = { + async getProfile() { + return null; + }, + } as unknown as MatrixClient; setupDB(hooks, { beforeEach: async (_dbAdapter: PgAdapter): Promise => { dbAdapter = _dbAdapter; adapter = new FakeMediaCacheAdapter(); + // The ledger fast path is gated on realm read; `@stranger:localhost` + // is deliberately left without permissions for the negative tests. + await insertPermissions(dbAdapter, new URL(REALM_URL), { + '@someone:localhost': ['read'], + }); }, }); @@ -351,6 +368,7 @@ module(basename(import.meta.filename), function () { handleScreenshotCard({ dbAdapter, queue, + matrixClient, mediaCacheAdapter: adapter, ...opts, } as unknown as CreateRoutesArgs), @@ -359,7 +377,10 @@ module(basename(import.meta.filename), function () { return app; } - async function seedInstanceRow(generation = 1) { + async function seedInstanceRow( + generation = 1, + opts: { hasError?: boolean } = {}, + ) { let { nameExpressions, valueExpressions } = asExpressions( { url: `${CARD_ID}.json`, @@ -371,8 +392,11 @@ module(basename(import.meta.filename), function () { resource_created_at: Date.now(), is_deleted: false, pristine_doc: { attributes: {} }, + ...(opts.hasError + ? { has_error: true, error_doc: { message: 'index error' } } + : {}), }, - { jsonFields: ['pristine_doc'] }, + { jsonFields: ['pristine_doc', 'error_doc'] }, ); await query( dbAdapter, @@ -380,9 +404,13 @@ module(basename(import.meta.filename), function () { ); } - function post(app: Koa, attributes: Record) { + function post( + app: Koa, + attributes: Record, + user = '@someone:localhost', + ) { let token = createJWT( - { user: '@someone:localhost', sessionRoom: '!room:localhost' }, + { user, sessionRoom: '!room:localhost' }, realmSecretSeed, ); return supertest(app.callback()) @@ -553,5 +581,177 @@ module(basename(import.meta.filename), function () { assert.strictEqual(attrs.base64, PNG_BASE64, 'legacy shape intact'); assert.false('captures' in attrs, 'no served URL without a persist'); }); + + test('a caller without realm read never touches the ledger', async function (assert) { + await seedInstanceRow(); + let ledgerBytes = new TextEncoder().encode('private-ledger-bytes'); + let ledgerBase64 = Buffer.from(ledgerBytes).toString('base64'); + await putMedia(dbAdapter, adapter, { + realmURL: REALM_URL, + sourceURL: CARD_ID, + captureSpecHash: await captureSpecHash({ format: 'isolated' }), + sourceGeneration: 1, + bytes: ledgerBytes, + contentType: 'image/png', + lane: 'on-demand', + width: 800, + height: 600, + }); + let { queue, published } = makePersistQueue('ready'); + + let response = await post( + persistApp(queue), + { realmURL: REALM_URL, cardId: CARD_ID, format: 'isolated' }, + '@stranger:localhost', + ).expect(201); + + assert.strictEqual( + published.length, + 1, + 'goes to the render path (whose permissions the worker enforces) instead of the ledger', + ); + assert.strictEqual( + (published[0]?.args as any)?.persist, + null, + 'no persist identity without realm read', + ); + let attrs = response.body.data.attributes; + assert.notStrictEqual( + attrs.base64, + ledgerBase64, + 'the stored capture bytes never reach a caller without read', + ); + assert.false( + 'captures' in attrs, + 'no served URL is disclosed without read', + ); + }); + + test('an errored instance captures without persisting', async function (assert) { + await seedInstanceRow(1, { hasError: true }); + let { queue, published } = makePersistQueue('ready'); + + let response = await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + // An errored instance can never serve on the GET `_screenshot/` route + // (its liveness gate excludes effective-error rows), so persisting + // here would return a served URL that 404s. + assert.strictEqual((published[0]?.args as any)?.persist, null); + assert.false('captures' in response.body.data.attributes); + }); + + test('a ledger hit refreshes a stale last-accessed stamp', async function (assert) { + await seedInstanceRow(); + await putMedia(dbAdapter, adapter, { + realmURL: REALM_URL, + sourceURL: CARD_ID, + captureSpecHash: await captureSpecHash({ format: 'isolated' }), + sourceGeneration: 1, + bytes: PNG_BYTES, + contentType: 'image/png', + lane: 'on-demand', + width: 800, + height: 600, + }); + // Age the entry past the touch throttle so the hit must bump it — + // otherwise a capture consumed only through this endpoint looks idle + // to the GC's on-demand TTL while in active use. + let staleStamp = Date.now() - 25 * 60 * 60 * 1000; + await query(dbAdapter, [ + `UPDATE media_cache_ledger SET last_accessed_at =`, + param(staleStamp), + `WHERE realm_url =`, + param(REALM_URL), + `AND source_url =`, + param(CARD_ID), + ]); + let { queue, published } = makePersistQueue('ready'); + + await post(persistApp(queue), { + realmURL: REALM_URL, + cardId: CARD_ID, + format: 'isolated', + }).expect(201); + + assert.deepEqual(published, [], 'answered from the ledger'); + let rows = (await query(dbAdapter, [ + `SELECT last_accessed_at FROM media_cache_ledger WHERE realm_url =`, + param(REALM_URL), + `AND source_url =`, + param(CARD_ID), + ])) as { last_accessed_at: string | number }[]; + assert.true( + Number(rows[0]?.last_accessed_at) > staleStamp, + 'the hit bumped last_accessed_at', + ); + }); + }); + + module('screenshot queue twin estimate', function (hooks) { + let dbAdapter: PgAdapter; + + setupDB(hooks, { + beforeEach: async (_dbAdapter: PgAdapter): Promise => { + dbAdapter = _dbAdapter; + }, + }); + + test('hasTwin requires the runAs the caller would render under', async function (assert) { + let concurrencyGroup = 'screenshot:http://example.test/'; + let persist = { + realmURL: 'http://example.test/', + sourceURL: 'http://example.test/Person/fadhlan', + captureSpecHash: 'abc123', + sourceGeneration: 1, + lane: 'on-demand', + }; + let { nameExpressions, valueExpressions } = asExpressions( + { + job_type: 'screenshot-card', + concurrency_group: concurrencyGroup, + args: { + cardId: persist.sourceURL, + format: 'isolated', + runAs: '@owner:localhost', + persist, + }, + }, + { jsonFields: ['args'] }, + ); + await query(dbAdapter, insert('jobs', nameExpressions, valueExpressions)); + + let twinKey = { + sourceURL: persist.sourceURL, + captureSpecHash: persist.captureSpecHash, + sourceGeneration: persist.sourceGeneration, + }; + let sameIdentity = await estimateScreenshotQueueWait( + dbAdapter, + concurrencyGroup, + { ...twinKey, runAs: '@owner:localhost' }, + ); + assert.true( + sameIdentity.hasTwin, + 'a same-runAs pending job is a joinable twin', + ); + + // A persist-target match under a different runAs is a job the caller + // cannot join (the coalesce key includes runAs), so reporting it as a + // twin would wave a gate-skipping request into a lane that then + // renders anyway. + let differentRunAs = await estimateScreenshotQueueWait( + dbAdapter, + concurrencyGroup, + { ...twinKey, runAs: '@someone-else:localhost' }, + ); + assert.false( + differentRunAs.hasTwin, + 'a different-runAs job is not a twin', + ); + }); }); }); diff --git a/packages/runtime-common/index-query-engine.ts b/packages/runtime-common/index-query-engine.ts index 7ada3e5dad7..3bbd6a46afa 100644 --- a/packages/runtime-common/index-query-engine.ts +++ b/packages/runtime-common/index-query-engine.ts @@ -2157,8 +2157,11 @@ function prerenderedTableFromOpts(opts: WIPOptions | undefined) { // prerendered_html row reads those as NULL (`ph.url IS NULL`) — no rendering // exists yet. Being keyed on the primary key the join is 1:1, so it never // fans out a `GROUP BY url` grouping. `icon_html` is not joined in: the icon -// renders in the index visit and lives on boxel_index. -function prerenderedJoin(opts: WIPOptions | undefined) { +// renders in the index visit and lives on boxel_index. Exported (with +// `effectiveHasError`) so `findLiveInstanceGeneration` in media-cache.ts can +// build its realm-scoped raw-SQL twin of `liveInstanceGeneration` from the +// same fragments instead of re-deriving the liveness predicate. +export function prerenderedJoin(opts?: WIPOptions) { return `LEFT JOIN ${prerenderedTableFromOpts( opts, )} AS ph ON ph.url = i.url AND ph.realm_url = i.realm_url AND ph.type = i.type`; @@ -2177,7 +2180,7 @@ function prerenderedJoin(opts: WIPOptions | undefined) { // the prerender_html event. const RENDER_ERROR_IS_CURRENT = `(ph.url IS NOT NULL AND ph.error_doc IS NOT NULL AND ph.generation >= i.generation)`; -function effectiveHasError(): string { +export function effectiveHasError(): string { return `(COALESCE(i.has_error, FALSE) OR ${RENDER_ERROR_IS_CURRENT})`; } diff --git a/packages/runtime-common/jobs/screenshot-card.ts b/packages/runtime-common/jobs/screenshot-card.ts index 356d11e1c74..bc9bc003fb0 100644 --- a/packages/runtime-common/jobs/screenshot-card.ts +++ b/packages/runtime-common/jobs/screenshot-card.ts @@ -22,12 +22,17 @@ export const SCREENSHOT_CARD_JOB_TIMEOUT_SEC = 60; // caller the twin's result verbatim. Queued and in-flight twins both join; // an in-flight join just registers a late waiter on the running job. // -// Only the ledger-backed GET lane coalesces. Its persist target pins the -// source generation, so a joined caller can never be handed a capture of a -// different revision. `POST /_screenshot-card` publishes with `persist: null` -// — a render-now request whose identity carries no freshness axis, so an -// in-flight twin could be up to a reservation-lease old and of a pre-edit -// card; those always insert. +// Only persist-carrying jobs coalesce — both surfaces publish them: the GET +// `_screenshot/` lane always, `POST /_screenshot-card` whenever the instance +// is indexed and the server has a store. A persist target pins the source +// generation, so a joined caller can never be handed a capture of a +// different revision. A `persist: null` job (unindexed card, or a server +// with no MediaCache) is a render-now request whose identity carries no +// freshness axis — an in-flight twin could be up to a reservation-lease old +// and of a pre-edit card — so those always insert. The `runAs` equality +// below keeps joins within one render identity: the GET lane renders as the +// realm owner and the POST lane as the requester, so cross-surface twins +// never join even when their persist targets match. function chooseScreenshotCardCoalesceDecision( context: QueueCoalesceContext, ): QueueCoalesceDecision { @@ -124,9 +129,10 @@ export interface ScreenshotQueueEstimate { // even starts. estimatedWaitMs: number; // True when a queued or in-flight job already carries this exact capture - // identity: the incoming request would coalesce onto it and cost no new - // Chrome work, so the caller skips the congestion pre-check rather than - // 503-ing a request the lane is about to satisfy for free. + // identity — persist target AND `runAs`, the full coalesce key: the + // incoming request would coalesce onto it and cost no new Chrome work, so + // the caller skips the congestion pre-check rather than 503-ing a request + // the lane is about to satisfy for free. hasTwin: boolean; } @@ -140,13 +146,19 @@ export interface ScreenshotQueueEstimate { export async function estimateScreenshotQueueWait( dbAdapter: DBAdapter, concurrencyGroup: string, - // The persist identity of the capture about to be requested. When a queued - // or in-flight job already matches it, the request coalesces rather than - // rendering, so `hasTwin` lets the caller bypass the congestion gate. + // The capture identity about to be requested: the persist target plus the + // `runAs` the caller would render under. When a queued or in-flight job + // already matches all of it, the request coalesces rather than rendering, + // so `hasTwin` lets the caller bypass the congestion gate. `runAs` must be + // part of the match because the coalesce join requires it — a persist-only + // match would report jobs the caller cannot actually join (a POST job runs + // as its requester, a GET job as the realm owner) and wave a + // gate-skipping request into a lane that then renders anyway. twinOf?: { sourceURL: string; captureSpecHash: string; sourceGeneration: number; + runAs: string; }, ): Promise { if (dbAdapter.kind !== 'pg') { @@ -182,10 +194,12 @@ export async function estimateScreenshotQueueWait( ] as Expression) as Promise<{ avg_ms: number | string | null }[]>, twinOf ? (query(dbAdapter, [ - // A pending/in-flight job carrying the same persist target (jsonb - // extraction, so the compares are text — sourceGeneration binds as - // a string to match). Confined to the GET lane's identity fields; - // POST jobs have no persist and never appear here. + // A pending/in-flight job carrying the same persist target and + // `runAs` (jsonb extraction, so the compares are text — + // sourceGeneration binds as a string to match). Mirrors the + // coalesce join's key exactly: both surfaces publish + // persist-carrying jobs, and only a same-`runAs` job is one the + // caller would join. `SELECT EXISTS ( SELECT 1 FROM jobs WHERE status = 'unfulfilled' @@ -198,6 +212,8 @@ export async function estimateScreenshotQueueWait( param(twinOf.captureSpecHash), `AND args->'persist'->>'sourceGeneration' =`, param(String(twinOf.sourceGeneration)), + `AND args->>'runAs' =`, + param(twinOf.runAs), `) AS has_twin`, ] as Expression) as Promise<{ has_twin: boolean }[]>) : Promise.resolve([{ has_twin: false }]), diff --git a/packages/runtime-common/media-cache-serving.ts b/packages/runtime-common/media-cache-serving.ts index 5f14fbc967c..7a9600a4b65 100644 --- a/packages/runtime-common/media-cache-serving.ts +++ b/packages/runtime-common/media-cache-serving.ts @@ -151,7 +151,9 @@ export const MEDIA_CACHE_TOUCH_THROTTLE_MS = 60 * 60 * 1000; // to one per `MEDIA_CACHE_TOUCH_THROTTLE_MS` per entry. Best-effort: a // failed bump must never fail a serve — the worst case is an in-use // on-demand capture looking idle to the GC one sweep early, and a later -// serve re-marks it. +// serve re-marks it. Exported (as `touchMediaCacheEntryOnHit`) so every +// surface that answers from the ledger — this route and the POST +// `_screenshot-card` fast path — marks use through the one guard. async function touch(dbAdapter: DBAdapter, entry: MediaCacheEntry) { if (entry.lane !== 'on-demand') { return; @@ -168,3 +170,5 @@ async function touch(dbAdapter: DBAdapter, entry: MediaCacheEntry) { ); } } + +export { touch as touchMediaCacheEntryOnHit }; diff --git a/packages/runtime-common/media-cache.ts b/packages/runtime-common/media-cache.ts index 70fb6632b67..24a412459c8 100644 --- a/packages/runtime-common/media-cache.ts +++ b/packages/runtime-common/media-cache.ts @@ -11,6 +11,7 @@ import { type Expression, } from './expression.ts'; import { uint8ArrayToHex } from './index.ts'; +import { effectiveHasError, prerenderedJoin } from './index-query-engine.ts'; const log = logger('media-cache'); @@ -306,21 +307,30 @@ export async function findMediaCacheEntry( // The generation of a live indexed instance, addressable by either its // extensionless card-id URL or its `.json` file URL — the capture-identity // resolution a caller needs before it can compute a MediaCache key. Returns -// undefined when the instance is absent or tombstoned. +// undefined when the instance is absent, tombstoned, or in effective-error +// state. +// +// "Live" here MUST mean what `IndexQueryEngine.liveInstanceGeneration` (the +// GET `_screenshot/` serving gate) means, or a capture persisted under this +// probe's generation resolves to a URL that route answers as a miss. The +// row predicate is that method's, built from the same exported fragments +// (`prerenderedJoin`, `effectiveHasError`), plus a realm scope this caller +// has and the engine's path does not need. export async function findLiveInstanceGeneration( dbAdapter: DBAdapter, { realmURL, instanceURL }: { realmURL: string; instanceURL: string }, ): Promise { let rows = (await query(dbAdapter, [ - `SELECT generation FROM boxel_index - WHERE (url =`, + `SELECT i.generation FROM boxel_index AS i ${prerenderedJoin()} + WHERE (i.url =`, param(instanceURL), - `OR file_alias =`, + `OR i.file_alias =`, param(instanceURL), - `) AND realm_url =`, + `) AND i.realm_url =`, param(realmURL), - `AND type = 'instance' - AND (is_deleted = FALSE OR is_deleted IS NULL) + `AND i.type = 'instance' + AND (i.is_deleted = FALSE OR i.is_deleted IS NULL) + AND NOT ${effectiveHasError()} LIMIT 1`, ] as Expression)) as { generation: number | string }[]; let row = rows[0]; diff --git a/packages/runtime-common/realm.ts b/packages/runtime-common/realm.ts index a83ace7c45c..9ad8a24c98c 100644 --- a/packages/runtime-common/realm.ts +++ b/packages/runtime-common/realm.ts @@ -4187,11 +4187,17 @@ export class Realm { }); } + // Render as the realm's owner — the same identity an index pass renders + // under. The requester already proved realm read; the capture is a + // realm-derived artifact, not a per-user view. Resolved ahead of the + // congestion pre-check because the twin probe matches on `runAs`. + let owner = await this.getRealmOwnerUserId(); + let concurrencyGroup = `screenshot:${this.url}`; let estimate = await estimateScreenshotQueueWait( this.#dbAdapter, concurrencyGroup, - entryKey, + { ...entryKey, runAs: owner }, ); // A request whose capture is already queued or rendering coalesces onto // that job (see `chooseScreenshotCardCoalesceDecision`) and costs no new @@ -4208,10 +4214,6 @@ export class Realm { ); } - // Render as the realm's owner — the same identity an index pass renders - // under. The requester already proved realm read; the capture is a - // realm-derived artifact, not a per-user view. - let owner = await this.getRealmOwnerUserId(); let job = await enqueueScreenshotCardJob( { realmURL: this.url,