PagePool: materialize the reserved module/command tab; plumb priority through definition lookups

[habdelra]

Why

Staging investigation of stuck indexing on ctse/ambitious-piranha (914 rows) and ctse/prudent-octopus (255 rows) found every errored row hitting the same fingerprint in timing_diagnostics:

• renderStage = waiting-stability
• queryLoadsInFlight > 0, all with source: search-resource:*
• cardDocLoadsInFlight = [], inFlightModuleImports = []
• affinitySnapshot.sameAffinityActivity carrying {kind: module, queue: module, state: queued} entries on the same affinity as the stuck card render

That is the documented self-referential prerender deadlock from the indexing-diagnostics skill rubric:

1. A card render (e.g. PolicyTrackingDashboard.json) renders and the template fires <Search @query={{filter: {type: {module: '.../cohort.gts', name: 'Cohort'}}}}> against _federated-search.
2. The search needs the Cohort card definition to plan SQL.
3. CachingDefinitionLookup.lookupDefinition misses (the modules table doesn't have a row for cohort.gts).
4. It spawns a prerenderModule call for cohort.gts — same affinity.
5. The affinity already has its tabs held by the dashboard render that's awaiting the search. The sub-module-prerender queues on the per-tab queue.
6. The dashboard render waits on the search → which waits on the definition → which waits on the queued module render → which waits on the dashboard's tab. Deadlock.
7. After 90s, the render-timeout fires and the row gets written as has_error.

The cycle was reproduced in 6 distinct erroring rows in ambitious-piranha and 2 in prudent-octopus. Every recent indexing job for ambitious-piranha has been rejected for ~similar reasons — the deadlock can also kill the whole batch when it gets caught by the manager-side 150s /prerender-visit timeout (job 410276 today fits that shape).

What this PR changes

Two narrowly-scoped fixes that together close the deadlock. Conceptually independent enough to review separately, but they belong in one PR because they fix the same underlying bug from two angles.

1. PagePool.#selectEntryForAffinity: materialize the reserved module/command tab

The file-admission cap (`#fileAdmissionCap = max(1, affinityTabMax - 1)`) is supposed to keep one tab's worth of global-pool headroom per affinity for module/command work. But the cap only limits file callers — it doesn't produce the reserved tab. The existing spawn branch in #selectEntryForAffinity had only three ways to actually create a tab for a non-file caller:

1. Claim an orphan context (only if the affinity had a previously-evicted shared BrowserContext lying around).
2. Commandeer a dormant standby (only if the standby pool happened to have a spare).
3. The canExpandPastAffinityCap high-priority tier expansion (dormant in default config — requires PRERENDER_PAGE_POOL_HIGH_PRIORITY_MAX and PRERENDER_HIGH_PRIORITY_THRESHOLD both set).

When (1) and (2) both miss and (3) is dormant, the call falls through to #selectLeastPendingTab(entryList) and queues on the existing busy tab. That is the deadlock state.

The fix adds a fourth path between the failed commandeer and the canExpandPastAffinityCap block: if the caller is non-file (queue !== 'file') and the affinity is under cap (`entryList.length < affinityTabMax`), synchronously await #ensureStandbyPool and retry commandeer. Same shape as the brand-new-affinity branch (`entryList.length === 0`) immediately below — generalized to existing affinities with at least one already-busy tab.

The fix is independent of priority. The reservation invariant says module/command work always gets a tab when one is available under the per-affinity cap; gating that on priority would mean the invariant only holds for some callers, which is the bug, not the contract.

Why this is safe at scale:

• #ensureStandbyPool respects #maxPages via #prepareSlotForStandby (caps total contexts at #maxPages + 1, LRU-evicts if needed). It cannot oversubscribe the global pool.
• #commandeerDormantTab({ standbyOnly: true }) only succeeds against a real standby — no cross-affinity steal.
• File callers still acquire #fileAdmissionCap upstream of #selectEntryForAffinity, so cross-realm fairness for file workload is unchanged.

2. CachingDefinitionLookup: plumb priority through definition sub-renders

Today getModuleDefinitionsViaPrerenderer calls prerenderer.prerenderModule({ affinityType, affinityValue, realm, url, auth }) — no priority argument. That means user-initiated reindex work (priority 10) silently downgrades to priority 0 for its definition-lookup sub-prerenders. The high-priority tier expansion (the third tab-creation path above) is gated on caller priority, so this silent downgrade also prevents the existing escape hatch from firing when it could have.

Added a narrow public option type DefinitionLookupOptions = { priority?: number } and threaded it through:

• DefinitionLookup.getModuleCacheEntry(url, opts?) (interface + both implementations)
• loadModuleCacheEntry / loadModuleCacheEntryUncached / loadModuleCacheEntryCoordinated (private args)
• getModuleDefinitionsViaPrerenderer(url, realmURL, userId, priority?)
• prerenderer.prerenderModule({ ..., priority })

prerenderModule and downstream already accepted and honored priority end-to-end (render-runner.ts passes it to getPage which uses it for per-tab queue / admission semaphore / global render semaphore priority-aware dequeue). The chain was plumbing-only on the lookup side.

LookupContext (the existing internal type with { requestingRealm }) was the wrong public surface — it carries internal lookup state callers shouldn't see. The new DefinitionLookupOptions is the narrow public option type.

Callers updated:

• IndexRunner.preWarmModulesTable now passes { priority: this.#jobPriority }. this.#jobPriority is already threaded from the worker job's JobInfo through to IndexRunner (existing infrastructure; we just stopped throwing it away on the last hop).
• RealmIndexQueryEngine.searchCards HTTP path: no notion of request priority on the realm-server side; passes undefined (defaults to 0), preserving current behavior.

What this does NOT do

• Does not address the broader "pre-warm walks the wrong modules" problem — that's coming in PR B. PR B uses reader.mtimes() to enumerate every .gts/.gjs in the realm and pre-warms all of them (not just the ones in per-row deps). PR B reduces how often the page-pool deadlock-prevention path even has to fire.
• Does not parallelize pre-warm. That's PR C, which lands after PR B's measurement establishes a baseline.
• Does not touch clearRealmCache's post-reindex realm-wide wipe (which re-empties the modules table after every full reindex). Out-of-scope follow-up.

Tests

tests/page-pool-standby-refill-test.ts

New test: 'module call on an existing affinity with all owned tabs busy materializes a fresh tab'.

Holds a file render on affinity A (one tab, not released), then fires a module call on A. Asserts the module call resolves promptly with a different pageId than the held tab — both that no deadlock occurred and that the module render landed on its own tab. Bounded by a 2s timer so a regression fails the test cleanly instead of hanging.

tests/definition-lookup-test.ts

Two new tests:

• 'getModuleCacheEntry forwards priority to prerenderModule' — captures the priority arg the mock prerenderer receives. Asserts:
  • An explicit { priority: 10 } forwards as 10.
  • A subsequent call on a cached entry short-circuits at the cache (priority isn't even consulted).
  • An invalidation + omitted priority forwards as undefined (which the prerender server treats as 0).
• 'getModuleCacheEntry caches non-card modules as no-card markers' — direct contract test for the no-card-marker behavior PR B's cost analysis depends on. Stubs the mock prerenderer to return { status: 'ready', definitions: {} }. Asserts:
  • The empty-definitions row persists to the modules table (verified by direct SQL).
  • A second getModuleCacheEntry short-circuits at the cache without re-invoking the prerenderer.

Both tests live in the existing definition-lookup-test.ts module structure and run in <100ms each.

Verification

Local (full realm-server suite takes too long for in-loop validation — relying on CI for full regression):

```
$ pnpm --filter @cardstack/realm-server run test --filter 'DefinitionLookup'
ok 1 .. 28 (all 28 DefinitionLookup tests pass, including 2 new)
$ pnpm --filter @cardstack/realm-server run test --filter 'module call on an existing'
ok 1 page-pool-standby-refill-test.ts > module call on an existing affinity with all owned tabs busy materializes a fresh tab
```

Both pnpm --filter @cardstack/runtime-common run lint and pnpm --filter @cardstack/realm-server run lint:js clean for the files this PR touches. (The pre-existing prettier baseline in generated realms/.../bxl-chunks/*.ts files is untouched by this PR.)

After merge + staging deploy, verification will be:

• Trigger _full-reindex on ctse/prudent-octopus and ctse/ambitious-piranha.
• Check boxel_index rows for those realms: `SELECT count(*) FILTER (WHERE has_error) FROM boxel_index WHERE realm_url = ?` — expect zero (vs. 8 today across the two realms).
• Spot-check timing_diagnostics->'affinitySnapshot'->'sameAffinityActivity' on the heaviest rows (dashboards, cohorts): no {state: queued} entries.
• Grep prerender-server logs for the dashboard's requestId=…: confirm any sub-module-prerender ran on a fresh pageId distinct from the dashboard's pageId.

Test plan

• CI passes on the new branch
• Staging deploy + _full-reindex of ctse/prudent-octopus lands with zero errored rows
• Staging deploy + _full-reindex of ctse/ambitious-piranha lands with zero errored rows
• Spot-check affinitySnapshot.sameAffinityActivity on the heaviest rows post-fix — no state: queued entries

🤖 Generated with Claude Code

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1cf2626ea3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

Pull request overview

This PR addresses a prerender/indexing deadlock where same-affinity module/command sub-renders can get queued behind a busy file render that is itself waiting on the sub-render (self-referential wait). It does this by ensuring a reserved module/command tab is actually materialized when available, and by propagating job priority through definition-lookup-triggered sub-prerenders so higher-priority work doesn’t silently downgrade.

Changes:

• PagePool: add a synchronous standby-refill + retry-commandeer path for non-file callers under the per-affinity cap, to ensure the reserved module/command tab is actually created.
• DefinitionLookup: introduce DefinitionLookupOptions { priority?: number } and thread it through to prerenderer.prerenderModule({ priority }).
• Tests: add coverage for the standby-refill behavior and for priority plumbing + empty-definitions (“no-card marker”) caching.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/runtime-common/index-runner.ts	Passes the indexing job priority into module pre-warm definition lookups.
packages/runtime-common/definition-lookup.ts	Adds DefinitionLookupOptions and forwards priority through module cache population/prerender.
packages/realm-server/prerender/page-pool.ts	Adds a refill+retry path to materialize a standby for non-file callers under the affinity cap.
packages/realm-server/tests/page-pool-standby-refill-test.ts	Adds a regression test intended to ensure a module call on a busy affinity gets a fresh tab.
packages/realm-server/tests/definition-lookup-test.ts	Adds tests for priority forwarding and caching empty-definitions module rows.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[github-actions]

Preview deployments

• Host staging preview

• Host production preview

Host Test Results

    1 files      1 suites   1h 48m 29s ⏱️
2 673 tests 2 658 ✅ 15 💤 0 ❌
2 692 runs  2 677 ✅ 15 💤 0 ❌

Results for commit de0548f.

Realm Server Test Results

    1 files  ±    0      1 suites  +1   8m 44s ⏱️ + 8m 44s
1 414 tests +1 414  1 414 ✅ +1 414  0 💤 ±0  0 ❌ ±0 
1 501 runs  +1 501  1 501 ✅ +1 501  0 💤 ±0  0 ❌ ±0 

Results for commit de0548f. ± Comparison against earlier commit 572f941.

[backspace]

The test plan suggests this:

• Staging deploy + _full-reindex of ctse/prudent-octopus lands with zero errored rows
• Staging deploy + _full-reindex of ctse/ambitious-piranha lands with zero errored rows

but it doesn’t look like that happened?

[habdelra]

The test plan suggests this:

• Staging deploy + _full-reindex of ctse/prudent-octopus lands with zero errored rows
• Staging deploy + _full-reindex of ctse/ambitious-piranha lands with zero errored rows

but it doesn’t look like that happened?

with all the merges that happen (especially the day before sprint demo), i'm unsure I could get thru a full reindex without an independent merge reverting my deploy. i think its easier to merge, test, and then adjust. our workers are setup such that if indexing gets interrupted by a deploy, it should pick up where it left off after the deploy is finished