fix(build): write BUILD_ID via writeBundle so App Router builds emit it

[james-elicx]

What regressed

The nightly Next.js Deploy Suite regressed by ~700 tests between run 27057467752 (2143 passing) and run 27081060115 (1444 passing). Nearly every App Router fixture failed at setup with:

Failed to extract build ID from dist/server/index.js
Custom deploy script failed:  undefined (1)

Root cause

Two interacting facts:

1. The vinext:build-id plugin wrote dist/server/BUILD_ID from a closeBundle hook. closeBundle does not fire during the multi-environment createBuilder().buildApp() pipeline used for App Router production builds, so the file was silently never written for pure App Router apps. Only hybrid apps (which run a second vite.build() pass with a fresh plugin instance) and Pages Router apps produced the file.

2. The e2e deploy harness (scripts/e2e-deploy.sh → read_build_id) falls back to regex-parsing the buildId out of dist/server/index.js when BUILD_ID is absent. That fallback worked while server bundles were unminified — until perf(build): minify server build environments by default #1777 (perf(build): minify server build environments by default) landed between the two runs. Minification mangles the get buildId() { return "..." } / buildId = "..." patterns the regex relies on, so the fallback started returning no match and every App Router deploy failed at setup.

Confirmed by reproducing locally: a pure App Router buildApp() did not emit dist/server/BUILD_ID, and the regex returns NO MATCH against the minified index.js.

Fix 1 — emit BUILD_ID reliably

Switch the vinext:build-id plugin from closeBundle to writeBundle (mirroring the already-working vinext:image-config plugin). writeBundle fires for every emitted bundle across all environments; the existing one-time write guard ensures the file is written exactly once. Verified dist/server/BUILD_ID is now emitted for pure App Router, hybrid, and Pages Router builds.

Fix 2 — one build ID across all plugin instances

While reviewing Fix 1 we found a pre-existing divergence: a single vinext build can instantiate vinext() more than once (App Router buildApp() + the separate Pages Router vite.build() for hybrid apps). With no user generateBuildId, each instance resolved its own random UUID, so the App Router runtime, Pages runtime, prerender manifest, and BUILD_ID file could each get a different ID — risking ISR/seed-cache key mismatches for hybrid apps.

The CLI now resolves the build ID once (honoring the user's generateBuildId) and publishes it via __VINEXT_SHARED_BUILD_ID; the plugin adopts it unless the user supplied their own generateBuildId. resolveBuildId()'s standalone semantics are unchanged, so this build-only coordination never leaks into dev or tests.

Verified on the hybrid app-router-cloudflare example — the App Router RSC/SSR runtime, Pages entry.js, vinext-prerender.json, and dist/server/BUILD_ID now all carry the same build ID.

Tests

tests/app-router-production-build.test.ts gains two assertions on the buildApp() path:

• dist/server/BUILD_ID is emitted and non-empty (the regression guard).
• With __VINEXT_SHARED_BUILD_ID set, the emitted file and the value baked into the runtime bundle both equal the shared ID.

✓ tests/app-router-production-build.test.ts (4 tests)
✓ tests/next-config.test.ts / prerender.test.ts (252 tests)
✓ tests/deploy / build-optimization / features / pages-router / static-export / app-router-static-export (927 tests)

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/@vinext/cloudflare@1810

npm i https://pkg.pr.new/vinext@1810

commit: 54da50b

[github-actions]

Example	Preview	Production	Original
app-router-cloudflare	preview	production
pages-router-cloudflare	preview	production
app-router-playground	preview	production	original
realworld-api-rest	preview	production
nextra-docs-template	preview	production
benchmarks	preview	production
hackernews	preview	production	original
workers-cache	preview	production
web	preview	production

[james-elicx]

/bigbonk review

[ask-bonk]

@james-elicx Bonk workflow was cancelled.

View workflow run · To retry, trigger Bonk again.

[james-elicx]

/bigbonk review for issues

[ask-bonk]

/bigbonk review

Reviewed both fixes. Fix 1 (closeBundle → writeBundle) is correct — it mirrors the existing vinext:server-manifest plugin (index.ts:4383), which already uses writeBundle for the same multi-environment reason, and the one-time write guard is preserved. Tests pass locally (tests/app-router-production-build.test.ts, 4/4).

I found one real correctness issue in Fix 2.

🔴 The !rawConfig?.generateBuildId guard reintroduces the divergence it's meant to fix

In index.ts:1227:

const sharedBuildId = process.env.__VINEXT_SHARED_BUILD_ID;
if (sharedBuildId && sharedBuildId.length > 0 && !rawConfig?.generateBuildId) {
  nextConfig = { ...nextConfig, buildId: sharedBuildId };
}

The comment justifies skipping adoption when the user supplies generateBuildId:

a user-provided generateBuildId is authoritative and already shared because every instance calls it

That assumption is false for two supported cases:

1. generateBuildId returns null. This is documented Next.js behavior (and explicitly tested in tests/next-config.test.ts:1711 "falls back to a random UUID when generateBuildId returns null"). When it returns null, resolveBuildId() returns safeUUID() (config/next-config.ts:951) — a fresh random UUID per plugin instance. So for a hybrid app+pages build with generateBuildId: () => null, the App Router buildApp() instance and the hybrid Pages vite.build() instance each mint their own UUID — exactly the ISR/seed-cache key mismatch this PR set out to eliminate. The guard skips adoption because the function exists, even though its result is non-deterministic.

2. Non-deterministic generateBuildId (e.g. reads a timestamp, counter, or anything with side effects). "Every instance calls it" only coincides when the function is pure and deterministic.

Suggested fix

The CLI's resolvedNextConfig.buildId (cli.ts:455) is already the fully-resolved, authoritative ID — it ran resolveBuildId(config.generateBuildId) honoring the user's function including the null→UUID fallback. So the plugin can simply always adopt the shared ID when it's set, dropping the generateBuildId guard entirely:

const sharedBuildId = process.env.__VINEXT_SHARED_BUILD_ID;
if (sharedBuildId) {
  nextConfig = { ...nextConfig, buildId: sharedBuildId };
}

This is both simpler and correct for all three cases (no generateBuildId, generateBuildId returning a string, and generateBuildId returning null / being non-deterministic). The standalone resolveBuildId() semantics stay unchanged since the env var is only ever set by the build CLI.

If there's a deliberate reason to keep the guard (e.g. wanting the user's function to win even when it diverges per-call), that intent should be documented — but it contradicts the PR's stated goal of "one build ID across all plugin instances."

Minor

• Test gap: the new test (tests/app-router-production-build.test.ts) covers the no-generateBuildId path but not the generateBuildId paths. A regression test with generateBuildId: () => null plus __VINEXT_SHARED_BUILD_ID set would have caught the issue above and guards against future regressions.
• Env var lifetime: cli.ts:455 sets process.env.__VINEXT_SHARED_BUILD_ID and never clears it. Fine for the CLI (process exits after build), but it's a process-global mutation. Worth a one-line note that it's intentionally not cleaned up, or clearing it in a finally for robustness if buildApp() is ever reused in-process.

[ask-bonk]

Posted the review on PR #1810. Summary of findings:

Review summary

Fix 1 (closeBundle → writeBundle): ✅ Correct. It mirrors the existing vinext:server-manifest plugin which already uses writeBundle for the same multi-environment reason, preserves the one-time write guard, and tests pass locally (4/4).

Fix 2 (shared build ID): 🔴 One real correctness issue.

The guard !rawConfig?.generateBuildId at index.ts:1227 reintroduces the divergence it's meant to fix in two supported cases:

1. generateBuildId returns null — a documented, tested Next.js behavior (tests/next-config.test.ts:1711). In that case resolveBuildId() returns a fresh random UUID per plugin instance (config/next-config.ts:951), so a hybrid app+pages build gets divergent IDs across the buildApp() and hybrid vite.build() instances — exactly the ISR/seed-cache mismatch the PR targets. The guard skips adoption because the function exists, ignoring that its result isn't deterministic.

2. Non-deterministic generateBuildId (timestamps, counters, side effects).

Suggested fix: drop the guard and always adopt the shared ID when set, since the CLI's resolvedNextConfig.buildId is already the fully-resolved authoritative value (including the null→UUID fallback). Simpler and correct for all cases.

Minor: test gap (no coverage of the generateBuildId paths) and the CLI env var is never cleaned up.

github run

[james-elicx]

Thanks for the review — the generateBuildId correctness issue is a real one, fixed in 54da50b.

Fix: dropped the !rawConfig?.generateBuildId guard. The plugin now always adopts __VINEXT_SHARED_BUILD_ID when set, exactly as suggested. The CLI's resolvedNextConfig.buildId is already the authoritative value (it ran resolveBuildId(config.generateBuildId) once, including the documented null→UUID fallback), so a per-instance re-resolution should never win. This is correct for all cases: no generateBuildId, generateBuildId returning a string, and generateBuildId returning null / being non-deterministic.

Test gap: added adopts the shared build ID even when generateBuildId is set — it builds with generateBuildId: () => null and a shared ID set, asserting both dist/server/BUILD_ID and the baked runtime value equal the shared ID. This fails under the old guard and guards against re-introduction.

Env var lifetime: documented at cli.ts that the var is intentionally not cleared — vinext build runs once and the process exits, so there's no in-process reuse to leak into, and it's namespaced to the build flow (never read by dev or standalone resolveBuildId).

resolveBuildId()'s standalone semantics are unchanged; tests/next-config.test.ts (170) and tests/app-router-production-build.test.ts (5/5) pass.

[james-elicx]

/bigbonk re-review for issues

[ask-bonk]

/bigbonk re-review

Re-reviewed at 54da50b after the previous round. The earlier correctness issue is resolved and I found no new blocking issues. ✅

Fix 2 — guard dropped, now correct for all cases ✅

The !rawConfig?.generateBuildId guard is gone; the plugin now always adopts __VINEXT_SHARED_BUILD_ID when set (index.ts:1230-1232). I traced the data flow end-to-end and it's sound:

• The CLI sets the var from resolvedNextConfig.buildId (cli.ts:460), which is the authoritative value: resolveNextConfig() → resolveBuildId(config.generateBuildId) (config/next-config.ts:1382), honoring the documented null → safeUUID() fallback (config/next-config.ts:951).
• Adoption happens inside the once-per-instance if (!nextConfig) guard, before every downstream consumer of nextConfig.buildId:
  • define of process.env.__VINEXT_BUILD_ID → baked runtime value (index.ts:1325)
  • dist/server/BUILD_ID write (index.ts:4347)
  • prerender/client-runtime manifest (index.ts:4497)
• So for a hybrid app+pages build, all four artifacts now carry the single shared ID even when generateBuildId is non-deterministic (() => null, timestamps, etc.). This closes exactly the divergence the prior review flagged.

Fix 1 — closeBundle → writeBundle ✅ (unchanged, still correct)

Consistent with the sibling vinext:server-manifest (index.ts:4384) and vinext:image-config (index.ts:4297) plugins, which already use writeBundle + a one-time write guard for the same multi-environment reason. The buildIdWritten guard is preserved.

Test coverage ✅

The new adopts the shared build ID even when generateBuildId is set test (generateBuildId: () => null + a shared ID) is a genuine regression guard: under the old guard the file would have received a per-instance UUID instead of sharedBuildId, failing toBe(sharedBuildId). finally restores the prior env value, so it won't leak across the serial test files. All 5 tests in tests/app-router-production-build.test.ts pass locally (24s).

Notes (non-blocking)

• length > 0 is dead-but-harmless. resolveBuildId() throws on an empty string (config/next-config.ts:962), so buildId can never be ""; the extra check is defensive only. Fine to keep.
• rscCompatibilityId still diverges per instance. createRscCompatibilityId() (index.ts:183) returns a fresh randomUUID() per plugin instance when no deploymentId is set, so a hybrid app+pages build still gets two different RSC-compat IDs. This is orthogonal to this PR (RSC-compat token, not the build/cache ID) and pre-existing — worth a follow-up issue if cross-router RSC-compat consistency matters, but not a blocker here.
• Env-var lifetime is documented at cli.ts:451-459 as intentionally not cleaned up. Reasonable given vinext build exits after the build; the only in-process consumer is the test, which manages it explicitly.

LGTM.

[ask-bonk]

Re-review posted to PR #1810. Summary:

The prior correctness issue is resolved and no new blocking issues found. ✅

• Fix 2 (shared build ID): The !rawConfig?.generateBuildId guard was dropped. The plugin now always adopts __VINEXT_SHARED_BUILD_ID when set. I traced the full data flow and confirmed it's sound — the CLI's resolvedNextConfig.buildId is the authoritative value (including the documented null → UUID fallback), and adoption happens before all four consumers (define runtime bake, BUILD_ID file, prerender manifest). This closes the divergence flagged earlier, including the non-deterministic generateBuildId case.

• Fix 1 (closeBundle → writeBundle): Consistent with the sibling vinext:server-manifest and vinext:image-config plugins; one-time write guard preserved.

• Tests: The new generateBuildId: () => null test is a genuine regression guard (would fail under the old guard). All 5 tests pass locally.

• Non-blocking notes: length > 0 is dead-but-harmless (buildId can never be empty); rscCompatibilityId still diverges per instance but is orthogonal/pre-existing; env-var lifetime is documented as intentional.

LGTM.

github run