From 4c18295893a225d502366b2029fb30270c4fc3e3 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 2 Jun 2026 11:51:11 +0200 Subject: [PATCH 1/4] Add ember-test-waiters to markdown rendering pipeline The markdown card-reference, Mermaid, and KaTeX rendering runs in async operations kicked off by modifiers and ember-concurrency tasks after the initial render settles, so `settled()` could not wait for them. The acceptance tests compensated with manual 10-15s `waitFor`/`waitUntil` timeouts, which were slow and flaky. Base card defs run in the card sandbox and cannot import `@ember/test-waiters` directly, so this extracts the existing waiter injection plumbing out of fetcher.ts into a shared runtime-common `test-waiters` module that exposes a lazy `buildWaiter`/`waitForPromise` backed by the host-injected implementation (no-op in production). `base/default-templates/markdown.gts` now registers a `markdown-rendering` waiter around the KaTeX and Mermaid lazy-load tasks and the deferred card-slot collection, with a teardown guard so a destroyed modifier can't leave a waiter pending. The acceptance test manual timeouts are replaced with `settled()`. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/base/default-templates/markdown.gts | 117 ++++++++++++------ .../acceptance/markdown-file-def-test.gts | 106 ++++++---------- packages/runtime-common/fetcher.ts | 34 +---- packages/runtime-common/index.ts | 1 + packages/runtime-common/test-waiters.ts | 59 +++++++++ 5 files changed, 177 insertions(+), 140 deletions(-) create mode 100644 packages/runtime-common/test-waiters.ts diff --git a/packages/base/default-templates/markdown.gts b/packages/base/default-templates/markdown.gts index b1793366e41..974c754e26e 100644 --- a/packages/base/default-templates/markdown.gts +++ b/packages/base/default-templates/markdown.gts @@ -10,6 +10,7 @@ import LinkOffIcon from '@cardstack/boxel-icons/link-off'; import { bfmBlockFormatAndSize, + buildWaiter, cardTypeName, extractMermaidBlocks, processKatexPlaceholders, @@ -41,6 +42,11 @@ function wrapTablesHtml(html: string | null | undefined): string { return doc.body.innerHTML; } +// Lets `settled()` wait for the async markdown rendering work (Mermaid/KaTeX +// lazy-loading and the deferred card-slot collection) that is kicked off by +// modifiers and ember-concurrency tasks after the initial render settles. +const markdownRenderingWaiter = buildWaiter('markdown-rendering'); + type CardSlotFormat = 'atom' | 'embedded' | 'fitted' | 'isolated'; type SlotState = 'resolved' | 'loading' | 'unresolved'; @@ -168,6 +174,7 @@ export default class MarkDownTemplate extends GlimmerComponent<{ let linkedCards = this.args.linkedCards; let baseUrl = this.args.cardReferenceBaseUrl; let pendingUpdate = false; + let pendingToken: unknown = undefined; // On the very first modifier run linkedCards is likely still loading // (empty []) so we skip unresolved Pills to avoid flashing them for // refs that will soon resolve. On subsequent runs (linkedCards changed) @@ -269,22 +276,28 @@ export default class MarkDownTemplate extends GlimmerComponent<{ // re-render → observer fires again. let updateSlots = () => { pendingUpdate = false; - let nextSlots = collectSlots(); - let didChange = - nextSlots.length !== this.renderSlots.length || - nextSlots.some((slot, index) => { - let current = this.renderSlots[index]; - if (!current || current.element !== slot.element) return true; - if (current.kind !== slot.kind) return true; - if (current.state !== slot.state) return true; - if (current.format !== slot.format) return true; - if (current.card !== slot.card) return true; - if (current.url !== slot.url) return true; - return String(current.style ?? '') !== String(slot.style ?? ''); - }); - - if (didChange) { - this.renderSlots = nextSlots; + let token = pendingToken; + pendingToken = undefined; + try { + let nextSlots = collectSlots(); + let didChange = + nextSlots.length !== this.renderSlots.length || + nextSlots.some((slot, index) => { + let current = this.renderSlots[index]; + if (!current || current.element !== slot.element) return true; + if (current.kind !== slot.kind) return true; + if (current.state !== slot.state) return true; + if (current.format !== slot.format) return true; + if (current.card !== slot.card) return true; + if (current.url !== slot.url) return true; + return String(current.style ?? '') !== String(slot.style ?? ''); + }); + + if (didChange) { + this.renderSlots = nextSlots; + } + } finally { + markdownRenderingWaiter.endAsync(token); } }; @@ -293,15 +306,26 @@ export default class MarkDownTemplate extends GlimmerComponent<{ return; } pendingUpdate = true; + pendingToken = markdownRenderingWaiter.beginAsync(); scheduleOnce('afterRender', this, updateSlots); }; scheduleUpdate(); + // End any in-flight waiter token on teardown so a destroyed modifier + // (e.g. the scheduled update never flushed) cannot leave `settled()` + // hanging. `updateSlots` clears `pendingToken` first, so this only fires + // for a still-pending update. + let endPendingToken = () => { + let token = pendingToken; + pendingToken = undefined; + markdownRenderingWaiter.endAsync(token); + }; + // MutationObserver re-collects slots when the DOM is reconstructed // (e.g. after browser back-navigation rebuilds the element's children). if (typeof MutationObserver === 'undefined') { - return; + return endPendingToken; } let observer = new MutationObserver(scheduleUpdate); @@ -310,7 +334,10 @@ export default class MarkDownTemplate extends GlimmerComponent<{ subtree: true, }); - return () => observer.disconnect(); + return () => { + observer.disconnect(); + endPendingToken(); + }; }, ); @@ -328,11 +355,16 @@ export default class MarkDownTemplate extends GlimmerComponent<{ } _loadKatexTask = task({ drop: true }, async () => { - let loadKatex = (globalThis as any).__loadKatex; - if (typeof loadKatex !== 'function') { - return; + let token = markdownRenderingWaiter.beginAsync(); + try { + let loadKatex = (globalThis as any).__loadKatex; + if (typeof loadKatex !== 'function') { + return; + } + this._katex = await loadKatex(); + } finally { + markdownRenderingWaiter.endAsync(token); } - this._katex = await loadKatex(); }); // ── Mermaid lazy loading + pre-rendering ── @@ -361,27 +393,32 @@ export default class MarkDownTemplate extends GlimmerComponent<{ return; } - let mermaid = await loadMermaid(); - mermaid.initialize({ - startOnLoad: false, - securityLevel: 'strict', - theme: 'default', - }); + let token = markdownRenderingWaiter.beginAsync(); + try { + let mermaid = await loadMermaid(); + mermaid.initialize({ + startOnLoad: false, + securityLevel: 'strict', + theme: 'default', + }); - let svgs = new Map(); - for (let block of blocks) { - try { - let { svg } = await mermaid.render( - `mermaid-${++this._mermaidIdCounter}`, - block, - ); - svgs.set(block, svg); - } catch { - // skip failed blocks + let svgs = new Map(); + for (let block of blocks) { + try { + let { svg } = await mermaid.render( + `mermaid-${++this._mermaidIdCounter}`, + block, + ); + svgs.set(block, svg); + } catch { + // skip failed blocks + } } - } - this._mermaidSvgs = svgs; + this._mermaidSvgs = svgs; + } finally { + markdownRenderingWaiter.endAsync(token); + } }); getCardComponent = (card: BaseDef) => getComponent(card); diff --git a/packages/host/tests/acceptance/markdown-file-def-test.gts b/packages/host/tests/acceptance/markdown-file-def-test.gts index b2f312cd184..4e91db69b03 100644 --- a/packages/host/tests/acceptance/markdown-file-def-test.gts +++ b/packages/host/tests/acceptance/markdown-file-def-test.gts @@ -1,10 +1,10 @@ import { click, fillIn, + settled, triggerEvent, triggerKeyEvent, visit, - waitFor, waitUntil, } from '@ember/test-helpers'; @@ -336,7 +336,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}bfm-test.md`, }); - await waitFor('[data-test-pet-atom]', { timeout: 10000 }); + await settled(); assert .dom('[data-test-pet-atom]') @@ -388,12 +388,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}bfm-fallback.md`, }); - await waitUntil( - () => - document.querySelector('[data-test-markdown-bfm-unresolved-inline]') !== - null, - { timeout: 10000 }, - ); + await settled(); assert .dom('[data-test-markdown-bfm-unresolved-inline]') @@ -427,9 +422,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}bfm-test.md`, }); - await waitFor('[data-test-markdown-bfm-inline-card]', { timeout: 10000 }); - await waitFor('[data-test-markdown-bfm-block-card]', { timeout: 10000 }); - await waitFor('[data-test-card-url-bar-input]'); + await settled(); let urlInput = document.querySelector( '[data-test-card-url-bar-input]', @@ -452,16 +445,17 @@ module('Acceptance | markdown BFM card references', function (hooks) { await click('[data-test-markdown-bfm-inline-card]'); - await waitUntil(() => { - let currentValue = - ( - document.querySelector( - '[data-test-card-url-bar-input]', - ) as HTMLInputElement | null - )?.value ?? ''; - return currentValue !== startingValue; - }); - + let navigatedValue = + ( + document.querySelector( + '[data-test-card-url-bar-input]', + ) as HTMLInputElement | null + )?.value ?? ''; + assert.notStrictEqual( + navigatedValue, + startingValue, + 'clicking the inline card reference navigates the URL bar', + ); assert .dom('[data-test-card-url-bar-input]') .hasValue(`${testRealmURL}Pet/mango.json`); @@ -473,44 +467,29 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}bfm-test.md`, }); - await waitFor('[data-test-pet-atom]', { timeout: 10000 }); - await waitFor('[data-test-pet-embedded]', { timeout: 10000 }); - await waitFor('[data-test-card-url-bar-input]'); + await settled(); - // Wait for the overlay click handler to be bound (cursor: pointer is set - // by the Overlays component). CI runners under load need longer for the - // render cycles before the handler binds. - await waitUntil( - () => { - let el = document.querySelector( + // The overlay click handler (cursor: pointer set by the Overlays component) + // is bound once the card-reference slots resolve, which the markdown + // rendering waiter now tracks — so `settled()` above is sufficient. + assert.strictEqual( + ( + document.querySelector( '[data-test-markdown-bfm-inline-card]', - ) as HTMLElement | null; - return el?.style.cursor === 'pointer'; - }, - { - timeout: 15000, - timeoutMessage: 'overlay click handler (cursor:pointer) was not bound', - }, + ) as HTMLElement | null + )?.style.cursor, + 'pointer', + 'overlay click handler (cursor:pointer) is bound', ); await click('[data-test-markdown-bfm-inline-card]'); - await waitUntil( - () => { - let currentValue = - ( - document.querySelector( - '[data-test-card-url-bar-input]', - ) as HTMLInputElement | null - )?.value ?? ''; - return currentValue === `${testRealmURL}Pet/mango.json`; - }, - { - timeout: 15000, - timeoutMessage: - 'URL bar did not navigate to Pet/mango.json after click', - }, - ); + assert + .dom('[data-test-card-url-bar-input]') + .hasValue( + `${testRealmURL}Pet/mango.json`, + 'URL bar navigates to Pet/mango.json after click', + ); // Navigate back to the markdown file via the URL bar (code-mode navigation // uses replaceState, so history.back() has no entry to return to). @@ -524,9 +503,6 @@ module('Acceptance | markdown BFM card references', function (hooks) { 'Enter', ); - await waitFor('[data-test-pet-atom]', { timeout: 10000 }); - await waitFor('[data-test-pet-embedded]', { timeout: 10000 }); - assert .dom('[data-test-pet-atom]') .exists( @@ -552,8 +528,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { ], }); - await waitFor('[data-test-markdown-bfm-inline-card]', { timeout: 10000 }); - await waitFor('[data-test-markdown-bfm-block-card]', { timeout: 10000 }); + await settled(); await triggerEvent('[data-test-markdown-bfm-inline-card]', 'mouseenter'); assert @@ -584,11 +559,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}mermaid-test.md`, }); - // Wait for mermaid to lazy-load and render the diagram into an SVG. - await waitUntil(() => document.querySelector('pre.mermaid svg') !== null, { - timeout: 15000, - timeoutMessage: 'Mermaid diagram was not rendered as SVG within timeout', - }); + await settled(); assert .dom('pre.mermaid svg') @@ -608,14 +579,7 @@ module('Acceptance | markdown BFM card references', function (hooks) { codePath: `${testRealmURL}math-test.md`, }); - // Wait for KaTeX to lazy-load and render the math expressions. - await waitUntil( - () => document.querySelector('.math-placeholder .katex') !== null, - { - timeout: 15000, - timeoutMessage: 'KaTeX did not render math expressions within timeout', - }, - ); + await settled(); assert .dom('.math-placeholder .katex') diff --git a/packages/runtime-common/fetcher.ts b/packages/runtime-common/fetcher.ts index 666c83701a7..0811048c323 100644 --- a/packages/runtime-common/fetcher.ts +++ b/packages/runtime-common/fetcher.ts @@ -1,5 +1,8 @@ +import { buildWaiter, waitForPromise } from './test-waiters'; import type { VirtualNetwork } from './virtual-network'; +const fetcherWaiter = buildWaiter('fetcher'); + export type FetcherMiddlewareHandler = ( req: Request, next: (onwardReq: Request) => Promise, @@ -42,11 +45,11 @@ export function fetcher( ? urlOrRequest : new Request(urlOrRequest, init); - let token = beginAsync(); + let token = fetcherWaiter.beginAsync(); try { return responseWithWaiters(await buildNext(middlewareStack)(request)); } finally { - endAsync(token); + fetcherWaiter.endAsync(token); } }; return instance; @@ -104,30 +107,3 @@ function responseWithWaiters(response: Response): Response { }, }); } - -let waitForPromise: Waiters['waitForPromise'] = (p) => { - return p; -}; - -let beginAsync = (): unknown => { - return 'token'; -}; - -let endAsync = (_token: unknown): void => { - // pass -}; - -export interface Waiters { - buildWaiter(label: string): { - beginAsync(): unknown; - endAsync(token: unknown): void; - }; - waitForPromise(promise: Promise, label?: string): Promise; -} - -export function useTestWaiters(w: Waiters) { - ({ waitForPromise } = w); - let waiter = w.buildWaiter('fetcher'); - beginAsync = waiter.beginAsync.bind(waiter); - endAsync = waiter.endAsync.bind(waiter); -} diff --git a/packages/runtime-common/index.ts b/packages/runtime-common/index.ts index 98514b58384..7e92b6c20fe 100644 --- a/packages/runtime-common/index.ts +++ b/packages/runtime-common/index.ts @@ -695,6 +695,7 @@ export * from './stream'; export * from './realm'; export * from './realm-index-updater'; export * from './fetcher'; +export * from './test-waiters'; export * from './scoped-css'; export * from './html-utils'; export * from './utils'; diff --git a/packages/runtime-common/test-waiters.ts b/packages/runtime-common/test-waiters.ts new file mode 100644 index 00000000000..09728bade9f --- /dev/null +++ b/packages/runtime-common/test-waiters.ts @@ -0,0 +1,59 @@ +// Test-waiter plumbing for code that runs inside the card sandbox (e.g. base +// card defs and shared runtime-common utilities), which cannot import +// `@ember/test-waiters` directly. The host test setup injects the real +// implementation once via `useTestWaiters`; outside of tests (and before +// injection) everything here is a no-op so production code is unaffected. + +export interface Waiters { + buildWaiter(label: string): { + beginAsync(): unknown; + endAsync(token: unknown): void; + }; + waitForPromise(promise: Promise, label?: string): Promise; +} + +let injectedWaiters: Waiters | undefined; + +export function useTestWaiters(w: Waiters) { + injectedWaiters = w; +} + +export interface TestWaiter { + beginAsync(): unknown; + endAsync(token: unknown): void; +} + +// Returns a waiter whose real `@ember/test-waiters` backing is resolved lazily. +// This lets modules build their waiter at import time (before the host has had +// a chance to inject the real implementation) without registering a stray +// waiter in production. +export function buildWaiter(label: string): TestWaiter { + let real: ReturnType | undefined; + let resolve = () => { + if (!real && injectedWaiters) { + real = injectedWaiters.buildWaiter(label); + } + return real; + }; + return { + beginAsync() { + return resolve()?.beginAsync(); + }, + endAsync(token: unknown) { + if (token === undefined) { + return; + } + resolve()?.endAsync(token); + }, + }; +} + +export function waitForPromise( + promise: Promise, + label?: string, +): Promise { + if (injectedWaiters) { + return injectedWaiters.waitForPromise(promise, label); + } + return promise; +} From 9b284704b2dd8a72220752d2376ddbdf922e9f24 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 2 Jun 2026 13:11:38 +0200 Subject: [PATCH 2/4] Clean up leaked @test-prefix/ realm mapping in render-service test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit render-service-test's throwaway VirtualNetworks register @test-prefix/ in the process-global card-reference prefix registry, which outlives them and leaked into sibling modules — realm indexing then collected a spurious @test-prefix/ dependency alias. Unregister the prefix in afterEach. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../host/tests/unit/services/render-service-test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/host/tests/unit/services/render-service-test.ts b/packages/host/tests/unit/services/render-service-test.ts index e56e2922b06..90c38eab87e 100644 --- a/packages/host/tests/unit/services/render-service-test.ts +++ b/packages/host/tests/unit/services/render-service-test.ts @@ -3,6 +3,7 @@ import { module, test } from 'qunit'; import { Deferred, VirtualNetwork, + unregisterCardReferencePrefix, type RealmResourceIdentifier, type SingleCardDocument, type SingleFileMetaDocument, @@ -32,7 +33,17 @@ function makeVN(): VirtualNetwork { return vn; } -module('Unit | Service | render-service', function () { +module('Unit | Service | render-service', function (hooks) { + // `makeVN()` registers `@test-prefix/` in the process-global card-reference + // prefix registry (via VirtualNetwork.addRealmMapping's backward-compat + // bridge). That registry outlives this module's throwaway VNs, so without + // cleanup the mapping leaks into sibling test modules — e.g. realm indexing + // then unresolves `http://test-realm/test/...` URLs back through the stale + // prefix and collects a spurious `@test-prefix/...` dependency alias. + hooks.afterEach(function () { + unregisterCardReferencePrefix(prefix); + }); + test('CardStoreWithErrors resolves registered prefix ids for card and file lookups', function (assert) { let store = new CardStoreWithErrors(globalThis.fetch, makeVN()); let card = {} as CardDef; From 5e4e69e4d2e2c3a262f118abba4e226e4b9e1491 Mon Sep 17 00:00:00 2001 From: Matic Jurglic Date: Tue, 2 Jun 2026 13:35:09 +0200 Subject: [PATCH 3/4] ci(host): scope JUnit merge to the current run attempt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The host-merge-reports-and-publish job downloaded JUnit reports with `pattern: host-test-report-*`. That glob also matched the job's own `host-test-report-merged` output and every prior attempt's shard reports, and download-artifact has no attempt filter — so re-running a flaky shard re-merged the failed attempt's results (including its errors) back into the published "Host Test Results" check. The aggregate could never go green on a re-run even though all shards passed. Prefix the per-shard artifact name with the run attempt and scope the merge download pattern to that attempt, so each merge sees only the current attempt's shard reports and no longer matches the merged output artifact. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/ci-host.yaml | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci-host.yaml b/.github/workflows/ci-host.yaml index 66d441fdab0..c8f1c46569b 100644 --- a/.github/workflows/ci-host.yaml +++ b/.github/workflows/ci-host.yaml @@ -260,7 +260,14 @@ jobs: uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 if: ${{ !cancelled() }} with: - name: host-test-report-${{ matrix.shardIndex }} + # Scope the artifact name to the run attempt. Artifacts are retained + # per attempt under one run_id, and download-artifact (in the merge + # job) has no attempt filter — so a plain `host-test-report-N` name + # lets a re-run's merge pull this attempt's report alongside the + # failed prior attempt's, re-injecting stale failures into the + # published "Host Test Results" check. The attempt prefix keeps the + # merge scoped to the current attempt. + name: host-test-report-${{ github.run_attempt }}-${{ matrix.shardIndex }} path: junit/host-${{ matrix.shardIndex }}.xml retention-days: 30 - name: Extract memory report @@ -393,7 +400,11 @@ jobs: uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8.0.1 with: path: all-host-reports - pattern: host-test-report-* + # Only the current attempt's shard reports. A bare `host-test-report-*` + # also matched the `host-test-report-merged` output below and every + # prior attempt's shard reports, so re-running a flaky shard re-merged + # the failed attempt's results and the check could never go green. + pattern: host-test-report-${{ github.run_attempt }}-* merge-multiple: true - run: ls From 0eb55bc9dc85dbc9870881ae800f745620aa89ee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 13:06:37 +0000 Subject: [PATCH 4/4] Re-export useTestWaiters and Waiters from fetcher.ts for backwards compatibility --- packages/runtime-common/fetcher.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/runtime-common/fetcher.ts b/packages/runtime-common/fetcher.ts index 0811048c323..efc49231fc7 100644 --- a/packages/runtime-common/fetcher.ts +++ b/packages/runtime-common/fetcher.ts @@ -107,3 +107,7 @@ function responseWithWaiters(response: Response): Response { }, }); } + +// Re-exported for backwards compatibility – these were previously defined here +// and may be consumed via deep imports from this module. +export { useTestWaiters, type Waiters } from './test-waiters';