Skip to content

Fix OOM in aspire-version-placeholders build hook#1318

Merged
IEvangelist merged 2 commits into
release/13.5from
dapine/fix-version-placeholders-oom
Jun 30, 2026
Merged

Fix OOM in aspire-version-placeholders build hook#1318
IEvangelist merged 2 commits into
release/13.5from
dapine/fix-version-placeholders-oom

Conversation

@IEvangelist

Copy link
Copy Markdown
Member

Problem

pnpm build:production on release/13.5 (e.g. CI for #1233) crashes during the aspire-version-placeholders integration's astro:build:done hook:

[build] Waiting for integration "aspire-version-placeholders", hook "astro:build:done"...
FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory
[ELIFECYCLE] Command failed with exit code 134.

Example run: https://github.com/microsoft/aspire.dev/actions/runs/28447182778/job/84299577028

Root cause

replaceAspireVersionPlaceholdersInDirectory walked the entire dist tree with a single recursive Promise.all. Every directory recursion and every file readFile started concurrently, so the contents of every .html/.md/.txt file across all locales — tens of thousands of files, including the large llms-full.txt assets — were held in memory simultaneously, exhausting the default ~4 GB Node heap.

Why the walk was mostly redundant

The replacement work itself is trivial (only two placeholders). Tracing where placeholders actually need replacing:

Artifact Source Already replaced before dist?
.html pages rendered via markdown.remarkPlugins (remarkAspireVersionPlaceholders runs before expressive-code renders code blocks) yes
llms*.txt starlight-llms-txt sources rendered HTML via render(entry) yes
reference/**/*.md generated from API/sample data, not docs content yes (no placeholders)
per-page *.md copies starlight-page-actions viteStaticCopys raw src/content/docs/** through a regex-only transform that bypasses remark no

The remarkAspireVersionPlaceholders plugin is already wired in and is the canonical, render-time, memory-safe mechanism. The only generated artifact that still carries raw %ASPIRE_VERSION% placeholders is the per-page .md copies emitted by starlight-page-actions.

Fix

  1. Scope the post-build pass to .md files only (the page-actions copies). The bulk of dist (.html + the large .txt assets) is no longer re-read.
  2. Bound concurrency — stream the .md files through a small worker pool (default 16) instead of an unbounded recursive Promise.all. Peak memory is now proportional to the concurrency limit, not the total file count.

Replacement semantics are unchanged.

Tests

Extended tests/unit/aspire-version-placeholders.vitest.test.ts:

  • .md copies are replaced; .html/.txt/.mdx/.json are left untouched (locks in the scoping decision).
  • Recursive traversal across nested directories with more files than the concurrency limit (exercises the bounded worker pool).

vitest 4/4 pass; ESLint clean.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

The astro:build:done hook re-walked the entire dist tree with a single
recursive Promise.all, holding the contents of every .html/.md/.txt file
(tens of thousands across all locales, including the large llms-full.txt
assets) in memory at once. On the full production build that exhausted the
default ~4 GB Node heap and crashed with 'JavaScript heap out of memory'.

That broad walk was almost entirely redundant. The remarkAspireVersionPlaceholders
remark plugin is already wired into markdown.remarkPlugins, so placeholders
are replaced before render: .html pages are correct, and llms*.txt is sourced
from rendered HTML via render(entry). The reference/**/*.md endpoints come from
API/sample data, not docs content. The only generated artifact that still
contains raw placeholders is the per-page .md copies emitted by
starlight-page-actions, which viteStaticCopy's raw src/content/docs/** through
a regex-only transform that bypasses the remark pipeline.

Scope the post-build pass to .md files only and stream them through a bounded
worker pool (default concurrency 16). Peak memory is now proportional to the
concurrency limit, and the bulk of dist (.html plus the large .txt assets) is
no longer re-read. Replacement semantics are unchanged.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 30, 2026 14:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an out-of-memory failure during pnpm build:production by narrowing and throttling the aspire-version-placeholders post-build replacement pass so it no longer reads (and holds) large portions of the dist output concurrently.

Changes:

  • Scope the post-build replacement pass to .md files (intended to target starlight-page-actions Markdown copies).
  • Replace unbounded recursive Promise.all traversal with a bounded worker-pool concurrency model.
  • Extend unit tests to lock in the scoping decision and exercise bounded traversal across nested directories.

Reviewed changes

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

File Description
src/frontend/config/aspire-version-placeholders-integration.mjs Restricts placeholder replacement to .md files and processes them with bounded concurrency to avoid OOM.
src/frontend/tests/unit/aspire-version-placeholders.vitest.test.ts Adds tests covering extension scoping and recursive traversal beyond the concurrency limit.

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

Comment thread src/frontend/config/aspire-version-placeholders-integration.mjs
Comment thread src/frontend/tests/unit/aspire-version-placeholders.vitest.test.ts Outdated
- Guard against a non-finite/0/negative concurrency value: normalize to a
  finite positive integer (falling back to the default) before computing the
  worker count, so a stray NaN can't collapse the pool to an empty array and
  silently skip every file. Adds a regression test passing NaN.
- Reword the scoping test comment so it no longer implies the seeded
  .html/.txt/.mdx fixtures were already replaced; clarify the assertion is that
  this pass intentionally leaves every non-.md extension untouched.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@aspire-repo-bot

Copy link
Copy Markdown
Contributor

Frontend HTML artifact ready

The latest frontend build uploaded the frontend-dist artifact for PR #1318. Use the VS Code button below to open this PR with GitHub Artifacts Explorer and browse the built HTML locally.

VS Code: Open PR #1318 artifacts

This comment updates automatically when a new frontend build artifact is uploaded.

@IEvangelist IEvangelist merged commit 9d2a467 into release/13.5 Jun 30, 2026
10 checks passed
@IEvangelist IEvangelist deleted the dapine/fix-version-placeholders-oom branch June 30, 2026 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants