diff --git a/packages/host/app/services/store.ts b/packages/host/app/services/store.ts index 820ee42c47..ca0d878f75 100644 --- a/packages/host/app/services/store.ts +++ b/packages/host/app/services/store.ts @@ -32,6 +32,8 @@ import { isSingleCardDocument, isLinkableCollectionDocument, resolveFileDefCodeRef, + X_BOXEL_JOB_PRIORITY_HEADER, + userInitiatedPriority, Deferred, delay, mergeRelationships, @@ -123,6 +125,77 @@ let waiter = buildWaiter('store-service'); const realmEventsLogger = logger('realm:events'); const storeLogger = logger('store'); +// Companion to `jobIdHeader()` (re-exported from +// `../lib/prerender-fetch-headers`). Policy is two-state, gated by +// `__boxelDuringPrerender`, not by the presence of +// `__boxelJobPriority`: +// +// 1. Inside a prerender tab: forward the worker job's priority as-is. +// The render-runner injects `__boxelJobPriority` alongside +// `__boxelJobId` on each visit — a priority of 0 is meaningful +// (the originating job is system-initiated background indexing) +// and must be preserved, not upgraded. Sub-`prerenderModule` +// calls fired by `_federated-search` for a `lookupDefinition` +// cache miss inherit this priority so they don't outrun the +// parent. If `__boxelJobPriority` is missing here (older +// render-runner build, test fixture, etc.) treat as 0 — the +// safe default for prerender-context work. +// +// 2. Outside a prerender tab (the host SPA in a real user's browser): +// stamp `userInitiatedPriority` (10). User clicks driving a +// search are by definition user-initiated work and should outrank +// background indexing on the realm-server's PagePool. Without +// this, a user search whose definition lookup misses the modules +// cache would fire its sub-prerender at priority 0 and queue +// behind concurrent indexing fan-out. +// +// External (non-host) HTTP callers — anything that doesn't run in +// the host SPA's JS runtime — bypass this helper entirely and set +// `X-Boxel-Job-Priority` directly on their request if they care. +// This helper covers the host SPA only. +// +// Both globals are checked with `=== true` / strict-number rather +// than truthy coercion: `__boxelDuringPrerender` is typed as a +// boolean and a stray truthy string from a future code path +// shouldn't silently flip the policy from "user-priority" to +// "preserve 0." +// Pure resolver — exported for the unit test in +// `tests/integration/job-priority-header-test.ts`. See the comment +// above for the policy rationale; the function is the literal +// translation of that policy to numbers. +export function resolveOutboundJobPriority({ + duringPrerender, + jobPriority, +}: { + duringPrerender: unknown; + jobPriority: unknown; +}): number { + let valid = + typeof jobPriority === 'number' && + Number.isSafeInteger(jobPriority) && + jobPriority >= 0 + ? jobPriority + : undefined; + if (duringPrerender === true) { + return valid ?? 0; + } + return valid ?? userInitiatedPriority; +} + +function jobPriorityHeader(): Record { + let g = globalThis as unknown as { + __boxelDuringPrerender?: boolean; + __boxelJobPriority?: number; + }; + return { + [X_BOXEL_JOB_PRIORITY_HEADER]: String( + resolveOutboundJobPriority({ + duringPrerender: g.__boxelDuringPrerender, + jobPriority: g.__boxelJobPriority, + }), + ), + }; +} const queryFieldSeedFromSearchSymbol = Symbol.for( 'cardstack-query-field-seed-from-search', ); @@ -835,6 +908,7 @@ export default class StoreService extends Service implements StoreInterface { ...duringPrerenderHeaders(), ...consumingRealmHeader(), ...jobIdHeader(), + ...jobPriorityHeader(), }, body: JSON.stringify({ ...query, realms }), }, @@ -904,6 +978,7 @@ export default class StoreService extends Service implements StoreInterface { ...duringPrerenderHeaders(), ...consumingRealmHeader(), ...jobIdHeader(), + ...jobPriorityHeader(), }, body: JSON.stringify({ ...query, realms }), }, diff --git a/packages/host/tests/unit/job-priority-header-test.ts b/packages/host/tests/unit/job-priority-header-test.ts new file mode 100644 index 0000000000..bc2cbd9e69 --- /dev/null +++ b/packages/host/tests/unit/job-priority-header-test.ts @@ -0,0 +1,150 @@ +import { module, test } from 'qunit'; + +import { userInitiatedPriority } from '@cardstack/runtime-common'; + +import { resolveOutboundJobPriority } from '@cardstack/host/services/store'; + +// Pure-resolver tests for the policy that decides what +// `X-Boxel-Job-Priority` value the host SPA stamps on outbound +// `_federated-search` calls. The function is module-internal logic +// extracted so its policy can be pinned without acceptance-test +// scaffolding. +// +// Two states gated by `__boxelDuringPrerender`: +// - inside prerender → forward (preserve 0) +// - outside prerender → user-initiated (10) by default +module('Unit | job-priority-header | resolveOutboundJobPriority', function () { + module('outside a prerender tab (user / API caller)', function () { + test('returns userInitiatedPriority when no global is set', function (assert) { + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: undefined, + jobPriority: undefined, + }), + userInitiatedPriority, + ); + }); + + test('returns userInitiatedPriority when __boxelDuringPrerender is false', function (assert) { + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: false, + jobPriority: undefined, + }), + userInitiatedPriority, + ); + }); + + test('honors an explicit override on __boxelJobPriority', function (assert) { + // Batch / scripting tooling running in the host SPA can set the + // global before issuing a fetch; outside a prerender tab we still + // forward what they set rather than overriding to user priority. + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: undefined, + jobPriority: 3, + }), + 3, + ); + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: false, + jobPriority: 0, + }), + 0, + 'override with 0 is preserved (not coerced to user priority)', + ); + }); + + test('rejects a truthy but non-boolean __boxelDuringPrerender — uses strict === true', function (assert) { + // If `__boxelDuringPrerender` somehow ended up as a stringy + // truthy value (e.g. set by a future code path that didn't + // coerce), the policy must NOT silently flip to "forward 0"; + // a real user-facing fetch would then queue behind background + // indexing. The check is `=== true` for exactly this reason. + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: 'yes', + jobPriority: undefined, + }), + userInitiatedPriority, + ); + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: 1, + jobPriority: undefined, + }), + userInitiatedPriority, + ); + }); + }); + + module('inside a prerender tab', function () { + test('forwards an explicit __boxelJobPriority of 10', function (assert) { + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: 10, + }), + 10, + ); + }); + + test('forwards an explicit __boxelJobPriority of 0 — must NOT upgrade', function (assert) { + // System-initiated indexing has priority 0. A + // `_federated-search` fired by the card render must preserve + // that or its sub-prerenders would outrank the parent job. + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: 0, + }), + 0, + ); + }); + + test('defaults to 0 when __boxelJobPriority is missing (older render-runner / test fixture)', function (assert) { + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: undefined, + }), + 0, + ); + }); + + test('rejects malformed __boxelJobPriority values', function (assert) { + // Non-number / negative / non-integer values fall through to + // the default for the active branch. + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: -1, + }), + 0, + ); + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: 1.5, + }), + 0, + ); + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: true, + jobPriority: '10', + }), + 0, + ); + assert.strictEqual( + resolveOutboundJobPriority({ + duringPrerender: false, + jobPriority: -1, + }), + userInitiatedPriority, + 'malformed value outside prerender → user-initiated default', + ); + }); + }); +}); diff --git a/packages/realm-server/handlers/handle-search.ts b/packages/realm-server/handlers/handle-search.ts index a612827941..0d47ace9c0 100644 --- a/packages/realm-server/handlers/handle-search.ts +++ b/packages/realm-server/handlers/handle-search.ts @@ -22,6 +22,8 @@ import { import type { JobScopedSearchCache } from '../job-scoped-search-cache'; import { PRERENDER_JOB_ID_HEADER, + PRERENDER_JOB_PRIORITY_HEADER, + sanitizeJobPriorityHeader, sanitizePrerenderJobId, } from '../prerender/prerender-constants'; @@ -53,14 +55,27 @@ export default function handleSearch(opts?: { } let cacheOnlyDefinitions = ctxt.get(DURING_PRERENDER_HEADER).length > 0; - let searchOpts = cacheOnlyDefinitions - ? { cacheOnlyDefinitions: true } - : undefined; + // The host's `_federated-search` fetch wrapper stamps + // `x-boxel-job-priority` while rendering inside a prerender tab. + // Threading it into search opts here lets `CachingDefinitionLookup` + // sub-prerenders (fired when a `type:` filter misses the modules + // cache) inherit the originating job's priority instead of silently + // dropping to 0. User / API callers don't stamp the header, so the + // value is `null` for live traffic — falls back to priority 0 + // (system-initiated default), same observable behavior as today. + let jobPriority = sanitizeJobPriorityHeader( + ctxt.get(PRERENDER_JOB_PRIORITY_HEADER), + ); + let searchOpts: { cacheOnlyDefinitions?: true; priority?: number } = {}; + if (cacheOnlyDefinitions) searchOpts.cacheOnlyDefinitions = true; + if (jobPriority !== null) searchOpts.priority = jobPriority; + let normalizedSearchOpts = + Object.keys(searchOpts).length > 0 ? searchOpts : undefined; let runSearch = () => searchRealms( realmList.map((realmURL) => realmByURL.get(realmURL)), cardsQuery, - searchOpts, + normalizedSearchOpts, ); // Job-scoped cache. Gated on: diff --git a/packages/realm-server/prerender/page-pool.ts b/packages/realm-server/prerender/page-pool.ts index f2f2e953bb..ca755fe920 100644 --- a/packages/realm-server/prerender/page-pool.ts +++ b/packages/realm-server/prerender/page-pool.ts @@ -1870,6 +1870,55 @@ export class PagePool { let releaseTab = await commandeered.queue.acquire(signal, priority); return { entry: commandeered, reused: false, releaseTab, tabStartupMs }; } + // module / command callers must produce the reserved tab the + // file-admission cap is supposed to keep room for. The cap + // bounds file workload at `affinityTabMax − 1`, leaving global- + // pool headroom for a non-file call — but the headroom is only + // a reservation, not a spawned tab. Without this synchronous + // refill+retry, the call falls through to the busy-tab fallback + // below and queues behind the file render that's awaiting this + // sub-render: the self-referential prerender deadlock. + // `#ensureStandbyPool` respects `#maxPages` via + // `#prepareSlotForStandby`, so this can't oversubscribe the + // global pool. Note this path only fires under + // `entryList.length < #affinityTabMax` — the at-cap case (every + // tab held by file renders, no dynamic-expansion budget) still + // falls through to busy-tab below. That residual deadlock + // requires either operator-side capacity tuning or the high- + // priority tier escape hatch beneath this branch. + // + // We await `#ensureStandbyPool` UNCONDITIONALLY here (not gated + // on `current < desired`). Reason: `#currentStandbyCount` = + // `#standbys.size + #creatingStandbys`. If the file render that + // arrived just before this caller consumed the only standby and + // its post-acquire `#kickStandbyRefill` is already creating a + // replacement, `creatingStandbys > 0` inflates `current` to + // meet `desired` while `#standbys.size` is still 0. A + // `current < desired` guard would skip the await; the + // subsequent `commandeerDormantTab(standbyOnly:true)` would + // then fail to find a real standby and the caller would fall + // through to the busy-tab branch — exactly the deadlock this + // change is meant to prevent. Two scenarios produce no-op + // behavior: (a) the pool is genuinely healthy with + // `#standbys.size >= desired` and no refill in flight — + // `#ensureStandbyPoolInternal`'s loop returns at line 1266 + // (`current >= desired`); (b) a refill is in flight — + // `#ensureStandbyPool` returns the existing `#ensuringStandbys` + // promise via dedup at line 1242 and we wait for it. Both + // produce the right shape: no spurious creation when not needed, + // wait when needed. + if (queue !== 'file' && entryList.length < this.#affinityTabMax) { + let startedAt = Date.now(); + await this.#ensureStandbyPool(); + tabStartupMs += Date.now() - startedAt; + let refilled = this.#commandeerDormantTab(affinityKey, { + standbyOnly: true, + }); + if (refilled) { + let releaseTab = await refilled.queue.acquire(signal, priority); + return { entry: refilled, reused: false, releaseTab, tabStartupMs }; + } + } // No orphan, no commandeer-able tab/standby. If we got here // through the dynamic-expansion escape hatch, drive an // expansion + fresh spawn so the saturated module/command @@ -1924,6 +1973,18 @@ export class PagePool { // #ensureStandbyPool()` in `getPage` made this ordering implicit; // now we make it explicit here so brand-new affinities still get // a fresh standby in preference to busy-tab queueing. + // + // The gate here is intentional and asymmetric with the non-file + // spawn-branch above. There the deadlock cost of skipping is + // unbounded (the caller queues on the very tab whose work it + // blocks), so we await unconditionally. Here the cost of + // skipping is only a cross-affinity-steal hop — itself a + // designed fallback — so paying an extra microtask for a no-op + // await is the wrong trade. It also shifts microtask ordering + // against a concurrent same-affinity file caller arriving on + // an idle tab, which the + // `queues same-realm request when tab is transitioning` test in + // `prerendering-test.ts` pins. if (this.#currentStandbyCount() < this.#desiredStandbyCount()) { let startedAt = Date.now(); await this.#ensureStandbyPool(); diff --git a/packages/realm-server/prerender/prerender-constants.ts b/packages/realm-server/prerender/prerender-constants.ts index 671cb4dc09..7e2ac03f54 100644 --- a/packages/realm-server/prerender/prerender-constants.ts +++ b/packages/realm-server/prerender/prerender-constants.ts @@ -42,6 +42,16 @@ export function sanitizePrerenderRequestId( // imports keep working unchanged. export { X_BOXEL_JOB_ID_HEADER as PRERENDER_JOB_ID_HEADER } from '@cardstack/runtime-common'; +// Worker-job priority of the request that triggered this prerender. +// Producer side stamps this header so any sub-`prerenderModule` the +// host fires during render inherits the originating priority instead +// of silently dropping to 0 — see prerender-headers.ts for the full +// chain rationale. +export { + X_BOXEL_JOB_PRIORITY_HEADER as PRERENDER_JOB_PRIORITY_HEADER, + sanitizeJobPriorityHeader, +} from '@cardstack/runtime-common'; + // Stamped on the host's outbound _federated-search / _search calls // when the host SPA detects it's running inside a prerender tab. The // prerender server signals "you are in a prerender" by injecting diff --git a/packages/realm-server/prerender/remote-prerenderer.ts b/packages/realm-server/prerender/remote-prerenderer.ts index 824e23364c..2018e44e9e 100644 --- a/packages/realm-server/prerender/remote-prerenderer.ts +++ b/packages/realm-server/prerender/remote-prerenderer.ts @@ -11,6 +11,7 @@ import { } from '@cardstack/runtime-common'; import { PRERENDER_JOB_ID_HEADER, + PRERENDER_JOB_PRIORITY_HEADER, PRERENDER_REQUEST_ID_HEADER, PRERENDER_SERVER_DRAINING_STATUS_CODE, PRERENDER_SERVER_STATUS_DRAINING, @@ -74,7 +75,11 @@ export function createRemotePrerenderer( let endpoint = new URL(path, prerenderURL); // jobId is request metadata, not part of the validated body — strip // it out before sending so the prerender-server's payload schema - // doesn't need to know about it. + // doesn't need to know about it. `priority` IS part of the validated + // body (existing PagePool plumbing), so we read it without stripping + // and additionally forward it as a header so the host's + // `_federated-search` fetch wrapper can re-stamp it on sub-prerender + // requests fired during render. let { jobId, ...attributesWithoutJobId } = attributes as Record< string, any @@ -87,6 +92,12 @@ export function createRemotePrerenderer( }; let sanitizedJobId = typeof jobId === 'string' ? sanitizePrerenderJobId(jobId) : null; + let priorityHeaderValue = + typeof attributes.priority === 'number' && + Number.isSafeInteger(attributes.priority) && + attributes.priority >= 0 + ? String(attributes.priority) + : null; // CS-10872: one correlation ID per logical client call, reused on // retries so operators can follow the full manager/prerender-server // log story for the same intent rather than hunting through N @@ -111,6 +122,9 @@ export function createRemotePrerenderer( ...(sanitizedJobId ? { [PRERENDER_JOB_ID_HEADER]: sanitizedJobId } : {}), + ...(priorityHeaderValue + ? { [PRERENDER_JOB_PRIORITY_HEADER]: priorityHeaderValue } + : {}), }, body: JSON.stringify(body), signal: ac.signal, diff --git a/packages/realm-server/prerender/render-runner.ts b/packages/realm-server/prerender/render-runner.ts index 985f9d0194..91fa9f7098 100644 --- a/packages/realm-server/prerender/render-runner.ts +++ b/packages/realm-server/prerender/render-runner.ts @@ -775,22 +775,35 @@ export class RenderRunner { // The check lives inside the try so `finally { release() }` frees // the tab slot if the caller aborted during the getPage handoff. throwIfAborted(signal, 'queued'); - // Single CDP round-trip that sets both the session auth and the - // indexing job id on the page. The job id surfaces to the host's - // `_federated-search` fetch wrapper via `globalThis.__boxelJobId` - // — the realm-server's handle-search gate pairs it with + // Single CDP round-trip that sets the session auth and the + // indexing job's id + priority on the page. Both surface to the + // host's `_federated-search` fetch wrapper via + // `globalThis.__boxelJobId` / `__boxelJobPriority` — the + // realm-server's handle-search gate pairs the id with // `x-boxel-consuming-realm` to decide whether to consult the - // JobScopedSearchCache. Always overwrite (including with - // undefined) so a tab reused across multiple visits never bleeds - // a prior visit's job id into the next render. + // JobScopedSearchCache, and threads the priority into + // `LookupContext.priority` so any sub-`prerenderModule` fired by + // `CachingDefinitionLookup` for a missed definition inherits the + // originating priority instead of silently dropping to 0. Always + // overwrite (including with undefined) so a tab reused across + // multiple visits never bleeds a prior visit's values into the + // next render. await page.evaluate( - (sessionAuth: string, id: string | undefined) => { + ( + sessionAuth: string, + id: string | undefined, + jobPriority: number | undefined, + ) => { localStorage.setItem('boxel-session', sessionAuth); (globalThis as unknown as { __boxelJobId?: string }).__boxelJobId = id; + ( + globalThis as unknown as { __boxelJobPriority?: number } + ).__boxelJobPriority = jobPriority; }, auth, jobId, + priority, ); // defense-in-depth: clear any stale file render data left on globalThis // from a prior visit before we start running passes. diff --git a/packages/realm-server/server.ts b/packages/realm-server/server.ts index 46890553bc..8578f2be63 100644 --- a/packages/realm-server/server.ts +++ b/packages/realm-server/server.ts @@ -395,7 +395,7 @@ export class RealmServer { cors({ origin: '*', allowHeaders: - 'Authorization, Content-Type, If-Match, If-None-Match, X-Requested-With, X-Boxel-Client-Request-Id, X-Boxel-Assume-User, X-HTTP-Method-Override, X-Boxel-Disable-Module-Cache, X-Filename, X-Boxel-During-Prerender, X-Boxel-Consuming-Realm, X-Boxel-Job-Id, X-Grafana-Device-Id, X-Grafana-Action', + 'Authorization, Content-Type, If-Match, If-None-Match, X-Requested-With, X-Boxel-Client-Request-Id, X-Boxel-Assume-User, X-HTTP-Method-Override, X-Boxel-Disable-Module-Cache, X-Filename, X-Boxel-During-Prerender, X-Boxel-Consuming-Realm, X-Boxel-Job-Id, X-Boxel-Job-Priority, X-Grafana-Device-Id, X-Grafana-Action', allowMethods: 'GET,HEAD,PUT,POST,DELETE,PATCH,OPTIONS,QUERY', // Cache the preflight response for 24 h. Without this @koa/cors // omits Access-Control-Max-Age and Chrome falls back to its diff --git a/packages/realm-server/tests/definition-lookup-test.ts b/packages/realm-server/tests/definition-lookup-test.ts index d4f792ff2b..b60bc9e71e 100644 --- a/packages/realm-server/tests/definition-lookup-test.ts +++ b/packages/realm-server/tests/definition-lookup-test.ts @@ -99,16 +99,34 @@ module(basename(__filename), function () { let mockRemotePrerenderer: Prerenderer; let dbAdapter: PgAdapter; let prerenderModuleCalls: number = 0; + let prerenderModulePriorities: (number | undefined)[] = []; + let forceEmptyDefinitions: boolean = false; let virtualNetwork: VirtualNetwork; hooks.beforeEach(async () => { prerenderModuleCalls = 0; + prerenderModulePriorities = []; + forceEmptyDefinitions = false; }); hooks.before(async () => { virtualNetwork = createVirtualNetwork(); mockRemotePrerenderer = { async prerenderModule(args: ModulePrerenderArgs) { prerenderModuleCalls++; + prerenderModulePriorities.push(args.priority); + if (forceEmptyDefinitions) { + return Promise.resolve({ + id: 'example-id', + status: 'ready' as const, + nonce: '12345', + isShimmed: false, + lastModified: +new Date(), + createdAt: +new Date(), + deps: [], + definitions: {}, + error: undefined, + }) as Promise; + } let moduleURL = new URL(args.url); let modulePathWithoutExtension = moduleURL.href.replace(/\.gts$/, ''); return Promise.resolve({ @@ -317,6 +335,100 @@ module(basename(__filename), function () { ); }); + test('getCachedDefinitions forwards priority to prerenderModule', async function (assert) { + await dbAdapter.execute('DELETE FROM modules'); + + await definitionLookup.getCachedDefinitions(`${realmURL}person.gts`, { + priority: 10, + }); + assert.strictEqual( + prerenderModuleCalls, + 1, + 'prerenderModule was called once', + ); + assert.deepEqual( + prerenderModulePriorities, + [10], + 'priority 10 was forwarded to prerenderModule', + ); + + // A subsequent call with a different priority on a cached entry must + // not fire the prerenderer again — cache short-circuits. + await definitionLookup.getCachedDefinitions(`${realmURL}person.gts`, { + priority: 0, + }); + assert.strictEqual( + prerenderModuleCalls, + 1, + 'cached entry returned without re-invoking the prerenderer', + ); + + // Default-priority call (no opts) must end up at priority `undefined` + // at the prerenderer, which the prerender server reads as 0. + await definitionLookup.invalidate(`${realmURL}person.gts`); + await definitionLookup.getCachedDefinitions(`${realmURL}person.gts`); + assert.strictEqual(prerenderModuleCalls, 2); + assert.deepEqual( + prerenderModulePriorities, + [10, undefined], + 'omitted priority forwards as undefined (default = 0 on the server)', + ); + }); + + test('getCachedDefinitions caches non-card modules as no-card markers', async function (assert) { + await dbAdapter.execute('DELETE FROM modules'); + forceEmptyDefinitions = true; + + let first = await definitionLookup.getCachedDefinitions( + `${realmURL}person.gts`, + ); + assert.strictEqual( + prerenderModuleCalls, + 1, + 'prerenderModule was called once', + ); + assert.ok(first, 'returned a cache entry'); + assert.deepEqual( + first?.definitions, + {}, + 'no-card module returns empty definitions', + ); + + // The empty-definitions row must persist to the modules table and be + // treated as a valid cache hit on the next lookup — the prerenderer + // must not fire a second time. + let second = await definitionLookup.getCachedDefinitions( + `${realmURL}person.gts`, + ); + assert.strictEqual( + prerenderModuleCalls, + 1, + 'second call short-circuited at the cache; prerenderModule still called once', + ); + assert.deepEqual( + second?.definitions, + {}, + 'second call returned the same empty-definitions row', + ); + + // Confirm the row really is in the database (not a transient in-memory + // entry that survived only the in-flight dedupe window). + let rows = await dbAdapter.execute( + `SELECT url, definitions FROM modules WHERE resolved_realm_url = $1`, + { bind: [realmURL] }, + ); + assert.strictEqual( + rows.length, + 1, + 'modules table has exactly one row for the realm', + ); + assert.deepEqual( + (rows[0] as { definitions: Record }).definitions, + {}, + 'persisted row has empty definitions', + ); + }); + test('invalidates cached module after module update', async function (assert) { await dbAdapter.execute('DELETE FROM modules'); diff --git a/packages/realm-server/tests/page-pool-standby-refill-test.ts b/packages/realm-server/tests/page-pool-standby-refill-test.ts index a8ec2400b7..d94fe68553 100644 --- a/packages/realm-server/tests/page-pool-standby-refill-test.ts +++ b/packages/realm-server/tests/page-pool-standby-refill-test.ts @@ -228,4 +228,67 @@ module(basename(__filename), function (hooks) { resB.release(); gate.unblockAll(); }); + + // The file-admission cap reserves a slot per-affinity for module / command + // work, but only as a *cap* on file callers — it doesn't itself produce + // the reserved tab. `#selectEntryForAffinity` for a non-file caller used + // to fall through to `#selectLeastPendingTab` when both the orphan-claim + // and the standby-commandeer paths missed, queueing the caller behind + // the busy file render that was waiting on it: the self-referential + // prerender deadlock. The fix unconditionally awaits `#ensureStandbyPool` + // and retries commandeer for module / command callers under the + // per-affinity cap, regardless of priority. + test('module call on an existing affinity with all owned tabs busy materializes a fresh tab', async function (assert) { + let gate = makeManualGate(); + let browserManager = makeBrowserStub({ gate: gate.gate }); + let pool = makePool({ maxPages: 2, browserManager }); + + // File render acquires the only tab on affinity A and holds it. + let held = await pool.getPage('A', 'file'); + assert.ok(held.pageId, 'first getPage on A returned a page'); + + // A module sub-render arrives for the same affinity while A1 is busy. + // Pre-fix: this would queue behind A1 on its per-tab queue (the + // deadlock state). Post-fix: a fresh tab is materialized. + let modCall = pool.getPage('A', 'module'); + + // Race the module call against the held tab — if the test ever hangs + // waiting on `modCall`, that's the deadlock regression returning. + // Bound with a 2s timeout so a regression fails the test cleanly. + let timeoutFired = false; + let timeoutHandle: NodeJS.Timeout | undefined; + let timer = new Promise<'timeout'>((resolve) => { + timeoutHandle = setTimeout(() => { + timeoutFired = true; + resolve('timeout'); + }, 2000); + }); + let winner = await Promise.race([ + modCall.then((r) => ({ kind: 'mod' as const, result: r })), + timer, + ]); + if (timeoutHandle) clearTimeout(timeoutHandle); + + assert.false( + timeoutFired, + 'module call did not block on the held file-render tab (no deadlock)', + ); + if (winner !== 'timeout') { + assert.strictEqual( + winner.kind, + 'mod', + 'module call resolved without waiting on the file render', + ); + // The module render must land on a different tab than the held one + // (which is still in use by the file render). + assert.notStrictEqual( + winner.result.pageId, + held.pageId, + 'module call got its own tab, distinct from the held file-render tab', + ); + winner.result.release(); + } + held.release(); + gate.unblockAll(); + }); }); diff --git a/packages/runtime-common/definition-lookup.ts b/packages/runtime-common/definition-lookup.ts index e6a736259f..d4aadee869 100644 --- a/packages/runtime-common/definition-lookup.ts +++ b/packages/runtime-common/definition-lookup.ts @@ -258,8 +258,21 @@ export interface PopulateCoordinator { waitForKey(coalesceKey: string, timeoutMs: number): Promise; } +// Public option shape for definition lookup calls. `priority` is forwarded to +// the prerender server when a cache miss requires a sub-prerender — same +// numeric scale as worker-job priority (0 = system-initiated background, +// 10 = userInitiatedPriority). Callers in the indexer thread their job +// priority through here so user-initiated reindex work doesn't silently +// downgrade to background priority for its module sub-renders. +export interface DefinitionLookupOptions { + priority?: number; +} + export interface DefinitionLookup { - lookupDefinition(codeRef: ResolvedCodeRef): Promise; + lookupDefinition( + codeRef: ResolvedCodeRef, + opts?: DefinitionLookupOptions, + ): Promise; // Like lookupDefinition but does not trigger a prerenderer call or // populate missing definitions. It may still perform lookup-context // resolution (including remote visibility probing) before reading from the @@ -274,6 +287,7 @@ export interface DefinitionLookup { forRealm(realm: LocalRealm): DefinitionLookup; getCachedDefinitions( moduleUrl: string, + opts?: DefinitionLookupOptions, ): Promise; getCachedDefinitionsBatch( query: DefinitionCacheEntryQuery, @@ -282,6 +296,13 @@ export interface DefinitionLookup { interface LookupContext { requestingRealm?: LocalRealm; + // Worker-job priority forwarded into sub-`prerenderModule` calls for + // definition cache misses. Origin is the `x-boxel-job-priority` + // header on `_federated-search` calls during a prerendered card + // render — `handle-search` sanitizes the header and passes it into + // `RealmIndexQueryEngine`'s search opts; the search engine threads + // it here when it calls `lookupDefinition`. + priority?: number; } export class CachingDefinitionLookup implements DefinitionLookup { @@ -339,8 +360,13 @@ export class CachingDefinitionLookup implements DefinitionLookup { this.#populateCoordinator = populateCoordinator; } - async lookupDefinition(codeRef: ResolvedCodeRef): Promise { - return await this.lookupDefinitionWithContext(codeRef); + async lookupDefinition( + codeRef: ResolvedCodeRef, + opts?: DefinitionLookupOptions, + ): Promise { + return await this.lookupDefinitionWithContext(codeRef, { + ...(opts?.priority !== undefined ? { priority: opts.priority } : {}), + }); } async lookupCachedDefinition( @@ -385,6 +411,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { async getCachedDefinitions( moduleUrl: string, + opts?: DefinitionLookupOptions, ): Promise { let canonicalModuleURL = canonicalURL(moduleUrl); let context = await this.buildLookupContext(canonicalModuleURL); @@ -405,6 +432,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope, cacheUserId, prerenderUserId, + priority: opts?.priority, }); } @@ -419,6 +447,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope: CacheScope; cacheUserId: string; prerenderUserId: string; + priority?: number; }): Promise { let key = inFlightKey(args); let existing = this.#inFlight.get(key); @@ -492,6 +521,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope: CacheScope; cacheUserId: string; prerenderUserId: string; + priority?: number; }, coordinator: PopulateCoordinator, ): Promise { @@ -540,6 +570,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope, cacheUserId, prerenderUserId, + priority, }: { moduleURL: string; realmURL: string; @@ -547,6 +578,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope: CacheScope; cacheUserId: string; prerenderUserId: string; + priority?: number; }): Promise { // Snapshot invalidation generations BEFORE the first await. // clearRealmDefinitions (and any future synchronous bump) runs entirely before @@ -584,6 +616,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { candidateURL, realmURL, prerenderUserId, + priority, ); if ( response.status === 'error' && @@ -756,6 +789,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { cacheScope, cacheUserId, prerenderUserId, + priority: contextOpts?.priority, }); if (!moduleEntry) { @@ -991,9 +1025,11 @@ export class CachingDefinitionLookup implements DefinitionLookup { async lookupDefinitionForRealm( codeRef: ResolvedCodeRef, realm: LocalRealm, + opts?: DefinitionLookupOptions, ): Promise { return await this.lookupDefinitionWithContext(codeRef, { requestingRealm: realm, + ...(opts?.priority !== undefined ? { priority: opts.priority } : {}), }); } @@ -1088,6 +1124,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { moduleUrl: string, realmURL: string, userId: string, + priority?: number, ): Promise { let permissions = await fetchUserPermissions(this.#dbAdapter, { userId }); let auth = this.#createPrerenderAuth(userId, permissions); @@ -1097,6 +1134,7 @@ export class CachingDefinitionLookup implements DefinitionLookup { realm: realmURL, url: moduleUrl, auth, + priority, }); } @@ -1707,8 +1745,15 @@ class RealmScopedDefinitionLookup implements DefinitionLookup { this.#realm = realm; } - async lookupDefinition(codeRef: ResolvedCodeRef): Promise { - return await this.#inner.lookupDefinitionForRealm(codeRef, this.#realm); + async lookupDefinition( + codeRef: ResolvedCodeRef, + opts?: DefinitionLookupOptions, + ): Promise { + return await this.#inner.lookupDefinitionForRealm( + codeRef, + this.#realm, + opts, + ); } async lookupCachedDefinition( @@ -1741,8 +1786,9 @@ class RealmScopedDefinitionLookup implements DefinitionLookup { async getCachedDefinitions( moduleUrl: string, + opts?: DefinitionLookupOptions, ): Promise { - return await this.#inner.getCachedDefinitions(moduleUrl); + return await this.#inner.getCachedDefinitions(moduleUrl, opts); } async getCachedDefinitionsBatch( diff --git a/packages/runtime-common/index-runner.ts b/packages/runtime-common/index-runner.ts index 4ca0edf81d..15c9061ff2 100644 --- a/packages/runtime-common/index-runner.ts +++ b/packages/runtime-common/index-runner.ts @@ -8,6 +8,7 @@ import { Memoize } from 'typescript-memoize'; import { logger, + hasCardExtension, hasExecutableExtension, SupportedMimeType, jobIdentity, @@ -205,7 +206,17 @@ export class IndexRunner { await current.#dependencyResolver.orderInvalidationsByDependencies( invalidations, ); - await current.preWarmModulesTable(invalidations); + // Pre-warm the modules cache. Combines per-row deps (which catch + // most modules used during a from-scratch pass) with the realm- + // wide `.gts` / `.gjs` sweep (which catches sibling card modules + // referenced by string in templates — the typical + // `` + // pattern). The filesystem-mtimes walk was already paid by + // discoverInvalidations above; we just filter and reuse it. + let allRealmCardModules = Object.keys( + discoverResult.filesystemMtimes, + ).filter(hasCardExtension); + await current.preWarmModulesTable(invalidations, allRealmCardModules); let resumedRows = current.batch.resumedRows; let resumedSkipped = 0; current.#onProgress?.({ @@ -373,7 +384,14 @@ export class IndexRunner { } current.#scheduleClearCacheForNextRender(); } - await current.preWarmModulesTable(invalidations); + // Pre-warm: combine per-row deps with a realm-wide `.gts`/`.gjs` + // sweep. Incremental skips `discoverInvalidations` so the + // filesystem-mtimes walk hasn't happened yet — call it here. + // Typical realm sizes make this < 200 ms; one call per job. + let incrementalMtimes = await current.#reader.mtimes(); + let allRealmCardModules = + Object.keys(incrementalMtimes).filter(hasCardExtension); + await current.preWarmModulesTable(invalidations, allRealmCardModules); let hrefs = urls.map((u) => u.href); let resumedRows = current.batch.resumedRows; @@ -587,11 +605,38 @@ export class IndexRunner { // Failures here are warned but do not fail the batch — a mid-render // sub-prerender will still fire on demand if pre-warm misses a // module. - private async preWarmModulesTable(invalidations: URL[]): Promise { - if (invalidations.length === 0) { + private async preWarmModulesTable( + invalidations: URL[], + allRealmCardModules: string[] = [], + ): Promise { + if (invalidations.length === 0 && allRealmCardModules.length === 0) { return; } let preWarmStart = Date.now(); + + // Base layer: every `.gts` / `.gjs` file in the realm, regardless of + // whether it appears in this batch's invalidation set. Catches sibling + // card modules referenced by *string* in templates (e.g. + // ``) + // — those don't appear in any instance's runtime `deps`. Without + // this layer the search fires a same-affinity `prerenderModule` + // mid-card-render at lookup time, which is the wait-shape the + // PagePool's tab-materialization for module/command callers is + // meant to relieve. + // + // `.gts` / `.gjs` only is an optimization, not a correctness gate: + // `.ts` / `.js` files CAN host `CardDef` (e.g. command-input + // cards). If pre-warm misses such a module, the on-demand + // `lookupDefinition` read-through during the visit fires a + // `prerenderModule` for it — safe because the PagePool now + // materializes a tab for the sub-prerender instead of queueing it + // behind the render that triggered the lookup. Restricting the + // sweep to the extensions where cards live almost exclusively + // avoids paying the prerender cost on every reindex for files that + // rarely define a card (typical realms have many helper `.ts` + // files alongside their cards). + let toWarm = new Set(allRealmCardModules); + let hrefs = invalidations.map((u) => u.href); let existingRows = await this.batch.getDependencyRows(hrefs); let bestByUrl = new Map(); @@ -604,13 +649,15 @@ export class IndexRunner { } } - let toWarm = new Set(); let novelJsonUrls: URL[] = []; for (let url of invalidations) { // Module files in the invalidation set are deps that instances // in the same batch will consume — pre-warm them directly. This // covers from-scratch and atomic-update batches where most rows - // have no prior `deps` data yet. + // have no prior `deps` data yet. Unlike the realm-wide layer + // above, this includes `.ts` / `.js` helpers — only the ones the + // batch is actually touching, so cost is bounded by invalidation + // size rather than realm size. if (hasExecutableExtension(url.href)) { toWarm.add(url.href); } @@ -649,7 +696,9 @@ export class IndexRunner { let failed = 0; for (let moduleUrl of toWarm) { try { - await this.#definitionLookup.getCachedDefinitions(moduleUrl); + await this.#definitionLookup.getCachedDefinitions(moduleUrl, { + priority: this.#jobPriority, + }); } catch { failed += 1; } diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index 2a0e21c7a6..11ff54faa2 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -645,6 +645,19 @@ export * from './pr-manifest'; export * from './file-def-code-ref'; export const executableExtensions = ['.js', '.gjs', '.ts', '.gts']; +// Extensions covered by the realm-wide pre-warm sweep that primes the +// modules cache before the visit loop. This is an optimization, not a +// correctness gate: a `.ts` / `.js` file CAN host a `CardDef` +// (e.g. command-input cards), and if pre-warm misses one the on-demand +// `lookupDefinition` cache read-through fires a `prerenderModule` for +// it during the visit. The PagePool's tab-materialization for +// module/command callers makes that on-demand path safe (the sub- +// prerender gets its own tab instead of queueing behind the render +// that triggered it). Restricting the sweep to `.gts` / `.gjs` — where +// cards live almost exclusively in practice — avoids paying the +// prerender cost on every index for a file type that rarely contains +// card definitions. +export const cardExtensions = ['.gts', '.gjs']; export { createResponse } from './create-response'; export * from './db-queries/db-types'; @@ -1007,6 +1020,15 @@ export function hasExecutableExtension(path: string): boolean { return false; } +export function hasCardExtension(path: string): boolean { + for (let extension of cardExtensions) { + if (path.endsWith(extension)) { + return true; + } + } + return false; +} + export function trimExecutableExtension( input: RealmResourceIdentifier, ): RealmResourceIdentifier { diff --git a/packages/runtime-common/prerender-headers.ts b/packages/runtime-common/prerender-headers.ts index 91f1f2e220..db5fccff14 100644 --- a/packages/runtime-common/prerender-headers.ts +++ b/packages/runtime-common/prerender-headers.ts @@ -42,3 +42,36 @@ export function sanitizeConsumingRealmHeader( if (!trimmed) return null; return REALM_URL_PATTERN.test(trimmed) ? trimmed : null; } + +// HTTP header carrying the worker-job priority of the request that +// triggered the prerender. Threaded from `pg-queue` job priority → +// `remote-prerenderer` → prerender-server → render-runner → page +// (`globalThis.__boxelJobPriority`) → host's `_federated-search` fetch +// wrapper → realm-server's `handle-search`. The realm-server forwards +// it into `LookupContext.priority` so any sub-`prerenderModule` fired +// by `CachingDefinitionLookup` for a missed definition inherits the +// originating job's priority instead of silently dropping to 0. +// +// Same scale as worker-job priority — 0 = system-initiated, 10 = +// userInitiatedPriority — small non-negative integers. +export const X_BOXEL_JOB_PRIORITY_HEADER = 'x-boxel-job-priority'; + +// Sanitize the inbound job-priority header value. The producer side +// stringifies a small non-negative integer; the consumer side must +// reject anything that isn't a base-10 integer in a reasonable range +// before passing it on as a number. Rejecting an out-of-range value +// returns `null` rather than clamping so an upstream regression +// (e.g. someone sending a bogus value) surfaces as "no priority" — +// safer than silently substituting a plausible-looking integer. +const JOB_PRIORITY_MAX = 1_000_000; +export function sanitizeJobPriorityHeader( + raw: string | null | undefined, +): number | null { + if (typeof raw !== 'string') return null; + let trimmed = raw.trim(); + if (!trimmed) return null; + if (!/^\d+$/.test(trimmed)) return null; + let n = Number(trimmed); + if (!Number.isSafeInteger(n) || n < 0 || n > JOB_PRIORITY_MAX) return null; + return n; +} diff --git a/packages/runtime-common/realm-index-query-engine.ts b/packages/runtime-common/realm-index-query-engine.ts index f1dcfcd88e..fcd573d3fc 100644 --- a/packages/runtime-common/realm-index-query-engine.ts +++ b/packages/runtime-common/realm-index-query-engine.ts @@ -83,6 +83,13 @@ type Options = { // when file-meta responses are served during card prerendering (the single // prerender semaphore permit is already held). cacheOnlyDefinitions?: boolean; + // Worker-job priority threaded from the caller (typically + // `_federated-search`'s `x-boxel-job-priority` header, set by the + // host's fetch wrapper during a prerendered card render). Forwarded + // to `lookupDefinition` so any sub-`prerenderModule` fired for a + // missed `type:` filter resolution inherits the originating + // priority. Defaults to 0 when absent. + priority?: number; } & QueryOptions; type SearchResult = SearchResultDoc | SearchResultError; @@ -381,7 +388,9 @@ export class RealmIndexQueryEngine { return false; } } else { - definition = await this.#definitionLookup.lookupDefinition(codeRef); + definition = await this.#definitionLookup.lookupDefinition(codeRef, { + ...(opts?.priority !== undefined ? { priority: opts.priority } : {}), + }); } if (!definition) { return false; @@ -715,7 +724,9 @@ export class RealmIndexQueryEngine { if (opts?.cacheOnlyDefinitions) { return await this.#definitionLookup.lookupCachedDefinition(codeRef); } - return await this.#definitionLookup.lookupDefinition(codeRef); + return await this.#definitionLookup.lookupDefinition(codeRef, { + ...(opts?.priority !== undefined ? { priority: opts.priority } : {}), + }); } // Populate query-based relationship fields using pre-extracted metadata