fix(test): fix race conditions in next-pages dev-server HMR test

[robobun]

Summary

• Fix race condition where console message listeners were attached AFTER triggering page reload/HMR, causing the test to miss messages and hang
• After HMR swap, wait for DOM to reflect new component and re-query element handles (old handles become stale after React re-renders)
• Accumulate stdout chunks for more robust dev server URL/"ready" detection
• Increase inner puppeteer subprocess timeout from 30s to 90s for slow CI

Test plan

• Verify test passes on Linux x64
• Verify test passes on macOS x64
• Verify test passes on macOS ARM64

🤖 Generated with Claude Code

[robobun]

^{Updated 1:17 PM PT - Mar 6th, 2026}

❌ Your commit 097cedd8 has 1 failures in Build #38757 (All Failures):

• test/js/node/child_process/child_process.test.ts - code 1 on 🪟 11 aarch64

---

🧪   To try this PR locally:

bunx bun-pr 27840

That installs a local version of the PR into your bun-27840 executable, so you can run:

bun-27840 --bun

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Two integration tests were updated: the Puppeteer test now pre-attaches console listeners, awaits them alongside navigation/reload/HMR, and re-queries DOM handles after HMR; the dev-server test now accumulates stdout when detecting readiness and extends startup timeouts from 30s to 90s.

Changes

Cohort / File(s)	Summary
Puppeteer race condition fixes test/integration/next-pages/test/dev-server-puppeteer.ts	Import adjustment; attach console listeners before navigation/reload/HMR and await their promises (use Promise.all); conditionally enable pipe on non-macOS; poll for DOM rebuild after HMR and re-acquire element handles before assertions.
Dev server initialization improvements test/integration/next-pages/test/dev-server.test.ts	Introduce accumulated stdout buffer for readiness detection (search accumulated, lowercased output) and increase dev server startup timeouts from 30s to 90s.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main changes: fixing race conditions in the next-pages dev-server HMR test, which aligns with the primary objectives of the pull request.
Description check	✅ Passed	The description covers the required template sections and provides clear details about what the PR does, but lacks explicit information about how the code changes were verified.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/integration/next-pages/test/dev-server.test.ts (1)

139-149: ⚠️ Potential issue | 🟠 Major

Replace timer-driven watchdog logic with condition-driven waiting.

Line 139 and Line 149 still use setTimeout to control test flow. This is explicitly disallowed for tests and keeps flakiness tied to wall-clock timing.

Suggested refactor (condition-based timeout without setTimeout)

-    var pid: number, exited;
-    let timeout = setTimeout(() => {
-      if (timeout && pid) {
-        process.kill?.(pid);
-        pid = 0;
-
-        if (dev_server_pid) {
-          process?.kill?.(dev_server_pid);
-          dev_server_pid = undefined;
-        }
-      }
-    }, 90000).unref();
-
-    ({ exited, pid } = Bun.spawn([bunExe(), "test/dev-server-puppeteer.ts", baseUrl], {
+    const proc = Bun.spawn([bunExe(), "test/dev-server-puppeteer.ts", baseUrl], {
       cwd: root,
       env: bunEnv,
       stdio: ["ignore", "inherit", "inherit"],
     }));
-
-    expect(await exited).toBe(0);
-    pid = 0;
-    clearTimeout(timeout);
-    // `@ts-expect-error`
-    timeout = undefined;
+    const result = await Promise.race([
+      proc.exited.then(code => ({ kind: "exit" as const, code })),
+      Bun.sleep(90_000).then(() => ({ kind: "timeout" as const })),
+    ]);
+    if (result.kind === "timeout") {
+      process.kill?.(proc.pid);
+      if (dev_server_pid) {
+        process.kill?.(dev_server_pid);
+        dev_server_pid = undefined;
+      }
+      throw new Error("dev-server-puppeteer timed out");
+    }
+    expect(result.code).toBe(0);

As per coding guidelines "Do not use setTimeout in tests; instead await the condition to be met - you are testing the CONDITION, not TIME PASSING".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration/next-pages/test/dev-server.test.ts` around lines 139 - 149,
The test currently uses a setTimeout-based watchdog assigned to the variable
timeout which kills pid and dev_server_pid after 90s; replace this timer-driven
logic with a condition-driven wait that awaits the expected condition (e.g.,
process exit, server ready, or PID becoming falsy) with a bounded Promise
timeout helper instead of using setTimeout directly. Locate the setTimeout call
that references timeout, pid, and dev_server_pid and remove it, then implement
an await waitForCondition helper (or reuse existing test utilities) that polls
the condition or listens for the child-process 'exit'/'close' events and rejects
after a configurable max duration; when the condition resolves, perform the same
cleanup (process.kill for pid/dev_server_pid) synchronously. Ensure the helper
is used in the same scope where timeout, pid, and dev_server_pid are defined so
cleanup behavior and variable mutations remain identical.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/integration/next-pages/test/dev-server-puppeteer.ts`:
- Around line 112-115: The test starts a console waiter (reload_promise =
waitForConsoleMessage(p, /counter a/)) but calls p.reload({}) without awaiting
it, creating a race and possible unhandled rejection; change the logic to await
both the reload and the console waiter together (e.g., use Promise.all on
p.reload() and reload_promise) so that p.reload()'s rejection is observed;
update the code around reload_promise, waitForConsoleMessage, and p.reload to
await both promises atomically.

---

Outside diff comments:
In `@test/integration/next-pages/test/dev-server.test.ts`:
- Around line 139-149: The test currently uses a setTimeout-based watchdog
assigned to the variable timeout which kills pid and dev_server_pid after 90s;
replace this timer-driven logic with a condition-driven wait that awaits the
expected condition (e.g., process exit, server ready, or PID becoming falsy)
with a bounded Promise timeout helper instead of using setTimeout directly.
Locate the setTimeout call that references timeout, pid, and dev_server_pid and
remove it, then implement an await waitForCondition helper (or reuse existing
test utilities) that polls the condition or listens for the child-process
'exit'/'close' events and rejects after a configurable max duration; when the
condition resolves, perform the same cleanup (process.kill for
pid/dev_server_pid) synchronously. Ensure the helper is used in the same scope
where timeout, pid, and dev_server_pid are defined so cleanup behavior and
variable mutations remain identical.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 9d66ef03-6ce1-469b-894d-28cd539a9f29

📥 Commits

Reviewing files that changed from the base of the PR and between 3832c85 and 5268986d99a12fbefb5025315ecb716f89a538f6.

📒 Files selected for processing (2)

• test/integration/next-pages/test/dev-server-puppeteer.ts
• test/integration/next-pages/test/dev-server.test.ts

[claude]

LGTM — straightforward test-only fixes for race conditions and flaky CI timeouts.

Extended reasoning...

Overview

This PR modifies two test files (dev-server-puppeteer.ts and dev-server.test.ts) under test/integration/next-pages/test/. The changes: (1) pre-attach console message listeners before triggering reload/HMR to eliminate race conditions, (2) wait for DOM to reflect the new component after HMR and re-query stale element handles, (3) accumulate stdout chunks instead of per-chunk matching for dev server readiness detection, and (4) bump an inner subprocess timeout from 30s to 90s.

Security risks

None. These are test-only changes with no production code impact.

Level of scrutiny

Low scrutiny warranted. All changes are confined to integration test files, follow established Puppeteer testing patterns (fire-and-forget navigation while awaiting console events), and the fixes are mechanically straightforward. The CodeRabbit suggestion to Promise.all the p.reload() is a minor nit — the fire-and-forget pattern is consistent with how p.goto(url) is already used earlier in the same file (line 83), and any reload failure would surface as a test timeout.

Other factors

No production code touched, no CODEOWNERS concerns, no outstanding human reviewer comments. The accumulated-stdout fix in the test harness is a clear improvement over per-chunk matching which could split the URL or "ready" keyword across chunk boundaries.

[claude]

LGTM — straightforward test reliability fixes for race conditions and stale handles.

Extended reasoning...

Overview

This PR modifies two test files (dev-server-puppeteer.ts and dev-server.test.ts) in the next-pages integration test suite. The changes fix race conditions where console listeners were attached after triggering page reload/HMR, add DOM readiness checks after HMR swaps, accumulate stdout chunks for more robust readiness detection, and bump an inner subprocess timeout from 30s to 90s.

Security risks

None. These are test-only changes with no production code impact.

Level of scrutiny

Low scrutiny warranted. All changes are confined to integration test files, follow established Puppeteer testing patterns, and the fixes are logically sound (set up listener before action, re-query stale handles, accumulate partial chunks). The timeout margin nit (90s inner vs 100s outer) is flagged as an inline comment but is unlikely to cause issues in practice since cleanup takes well under 10s.

Other factors

The label: claude tag indicates this was AI-generated, but the changes are small, well-scoped, and correct. CodeRabbit flagged the unawaited p.reload({}) but that pattern pre-existed this PR and is a common Puppeteer idiom when racing a reload against a console listener. The accumulated stdout buffer fix correctly handles the case where the URL or "ready" string is split across chunk boundaries.

[claude]

🟡 Nit: The inner kill timeout was bumped from 30s to 90s (line 149) but the outer bun:test timeout stayed at 100s (line 163), leaving only a 10s margin for cleanup (down from 70s). On the slow CI machines that motivated this change, the cleanup sequence (SIGTERM, browser close, process exit, exited promise resolution) could potentially exceed 10s, causing a confusing bun:test timeout instead of the intended clean subprocess kill. Consider bumping the outer timeout to 120_000 or 150_000 to restore a comfortable margin.

Extended reasoning...

What the bug is

The inner setTimeout safety timeout at line 149 was increased from 30,000ms to 90,000ms as part of this PR to accommodate slow CI machines. However, the outer bun:test timeout at line 163 was left unchanged at 100,000ms. This reduces the margin between the two timeouts from 70 seconds (100s - 30s) to just 10 seconds (100s - 90s).

How the timeout interaction works

The design intent is clear: the inner timeout is a safety mechanism that kills the puppeteer subprocess if the test hangs, while the outer bun:test timeout is the hard deadline for the entire test. The inner timeout should always fire well before the outer one, giving the test code time to cleanly handle the subprocess termination. When the inner timeout fires at 90s, the following cleanup sequence must complete within the remaining 10s:

1. process.kill(pid) sends SIGTERM to the puppeteer subprocess
2. The subprocess SIGTERM handler calls b?.close?.() to close the Chrome browser
3. The subprocess waits 100ms then calls process.exit(0)
4. expect(await exited) resolves in the test
5. The test assertion runs and the test completes

Step-by-step proof with timing

Before this PR: inner timeout = 30s, outer timeout = 100s, margin = 70s. Even on the slowest CI machine, 70 seconds is more than enough for cleanup.

After this PR: inner timeout = 90s, outer timeout = 100s, margin = 10s. While the cleanup sequence should normally complete in well under 1 second, the entire premise of this PR is that these tests run on slow CI machines. On such machines, Chrome browser shutdown (b?.close?.()) can be slow, process signal delivery can be delayed, and the event loop may be sluggish. If cleanup takes more than 10 seconds, the bun:test timeout fires first, producing a confusing generic timeout error instead of the clean subprocess kill the inner timeout was designed to provide.

Impact

The practical impact is low — cleanup will almost certainly complete in under 10 seconds even on slow CI. However, the whole point of the inner timeout is to provide a cleaner failure mode than bun:test generic timeout. With only a 10s margin, there is a non-trivial chance on stressed CI that the outer timeout fires first, defeating the purpose of the inner safety timeout.

Suggested fix

Bump the outer bun:test timeout at line 163 from 100_000 to 120_000 or 150_000 to restore a comfortable margin. This is a trivial change that preserves the intended timeout hierarchy.

[claude]

LGTM — test-only race condition fixes with correct patterns; the timeout margin nit is minor and shouldn't block.

Extended reasoning...

Overview

This PR modifies two test files (dev-server-puppeteer.ts and dev-server.test.ts) in the next-pages integration test suite. The changes fix race conditions where console message listeners were attached after triggering page reload/HMR, add DOM readiness waits and stale handle re-acquisition after HMR, accumulate stdout for more robust dev server readiness detection, and bump an inner subprocess timeout from 30s to 90s.

Security risks

None. These are purely test infrastructure changes with no impact on production code, user-facing behavior, or security-sensitive paths.

Level of scrutiny

Low scrutiny is appropriate. The changes are confined to integration test files, follow well-established Puppeteer patterns (pre-attaching listeners before triggering events, waitForFunction for DOM readiness, re-querying element handles after re-renders), and the fixes directly address the documented race conditions.

Other factors

I previously left a minor nit about the inner timeout (90s) being close to the outer bun:test timeout (100s), leaving only a 10s margin for cleanup. This is a legitimate observation but has low practical impact — cleanup should complete well within 10s even on slow CI. The core race condition fixes are correct and valuable. CodeRabbit also reviewed and found no actionable issues remaining. No CODEOWNERS are affected since these are test files.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

test/integration/next-pages/test/dev-server.test.ts (1)

139-149: ⚠️ Potential issue | 🟠 Major

Replace timer-based watchdog with await-based process lifecycle handling.

Line 149 still uses setTimeout for test control. This is brittle and conflicts with the test guideline; prefer await exited with deterministic cleanup in try/finally and rely on the test timeout configuration.

As per coding guidelines: "Do not use setTimeout in tests; instead await the condition to be met - you are testing the CONDITION, not TIME PASSING".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration/next-pages/test/dev-server.test.ts` around lines 139 - 149,
Replace the brittle setTimeout watchdog with deterministic await-based process
lifecycle handling: remove the setTimeout/unref block that references timeout,
pid, dev_server_pid and process.kill, and instead ensure the child process's
exit is awaited (use the process/exited promise or attach an 'exit'/'close'
listener and await it) within a try/finally around the test body; in the finally
block, perform cleanup by checking pid and dev_server_pid and invoking
process.kill if still running, then clear/reset those variables—use the child
process's exit promise rather than sleeping to control test completion.

♻️ Duplicate comments (1)

test/integration/next-pages/test/dev-server-puppeteer.ts (1)

112-114: ⚠️ Potential issue | 🟠 Major

Await the reload promise atomically with the console waiter.

At Line 113, p.reload({}) is fired but not awaited. If navigation fails, the rejection can be missed while reload_promise hangs waiting for logs.

Suggested fix

   const reload_promise = waitForConsoleMessage(p, /counter a/);
-  p.reload({});
-  await reload_promise;
+  await Promise.all([
+    reload_promise,
+    p.reload({ waitUntil: "domcontentloaded" }),
+  ]);

#!/bin/bash
# Verify whether reload() calls in this file are awaited or coordinated.
rg -nP --type=ts -C2 '\breload\s*\(' test/integration/next-pages/test/dev-server-puppeteer.ts

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@test/integration/next-pages/test/dev-server-puppeteer.ts` around lines 112 -
114, The test fires p.reload({}) without awaiting it, which can miss navigation
rejections while reload_promise (from waitForConsoleMessage) waits; change the
coordination to await both atomically — e.g., use Promise.all to run
waitForConsoleMessage (reload_promise) and p.reload({}) together so any
navigation error rejects the awaited operation; update the code around the
reload_promise and p.reload calls (references: reload_promise,
waitForConsoleMessage, p.reload) to await both concurrently.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@test/integration/next-pages/test/dev-server.test.ts`:
- Around line 67-75: The test currently calls loaded() as soon as
accumulated.includes("ready"), which can fire before baseUrl is detected and
cause undefined baseUrl; update the readiness check in the dev-server harness
(the block that updates accumulated, match, baseUrl, hasLoaded and calls
loaded()) to only resolve when BOTH a "ready" signal is seen and baseUrl has
been set (i.e., require accumulated.toLowerCase().includes("ready") && baseUrl)
before setting hasLoaded = true and calling loaded(); keep the existing logic
that extracts baseUrl via accumulated.match(/http:\/\/localhost:\d+/) so baseUrl
is available when the combined condition is evaluated.

---

Outside diff comments:
In `@test/integration/next-pages/test/dev-server.test.ts`:
- Around line 139-149: Replace the brittle setTimeout watchdog with
deterministic await-based process lifecycle handling: remove the
setTimeout/unref block that references timeout, pid, dev_server_pid and
process.kill, and instead ensure the child process's exit is awaited (use the
process/exited promise or attach an 'exit'/'close' listener and await it) within
a try/finally around the test body; in the finally block, perform cleanup by
checking pid and dev_server_pid and invoking process.kill if still running, then
clear/reset those variables—use the child process's exit promise rather than
sleeping to control test completion.

---

Duplicate comments:
In `@test/integration/next-pages/test/dev-server-puppeteer.ts`:
- Around line 112-114: The test fires p.reload({}) without awaiting it, which
can miss navigation rejections while reload_promise (from waitForConsoleMessage)
waits; change the coordination to await both atomically — e.g., use Promise.all
to run waitForConsoleMessage (reload_promise) and p.reload({}) together so any
navigation error rejects the awaited operation; update the code around the
reload_promise and p.reload calls (references: reload_promise,
waitForConsoleMessage, p.reload) to await both concurrently.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: fae0dd5a-61c1-44b0-a967-ba3de53e6e9f

📥 Commits

Reviewing files that changed from the base of the PR and between ad8e0067d2546c67f7b06f207524b04d8c8e0776 and 76b3508.

📒 Files selected for processing (2)

• test/integration/next-pages/test/dev-server-puppeteer.ts
• test/integration/next-pages/test/dev-server.test.ts

[claude]

LGTM — test-only race condition fixes with clear intent and no production impact.

Extended reasoning...

Overview

This PR modifies two test files (dev-server-puppeteer.ts and dev-server.test.ts) in the Next.js pages integration test suite. The changes fix race conditions where console listeners were attached after triggering navigation/reload/HMR, accumulate stdout for more robust dev server readiness detection, add browser launch retries, wait for DOM rebuilds after HMR before re-querying elements, and bump the inner subprocess timeout from 30s to 90s.

Security risks

None. This PR only touches test infrastructure — no production code, no auth paths, no user-facing behavior.

Level of scrutiny

Low scrutiny warranted. These are straightforward test reliability improvements following well-established Puppeteer patterns (Promise.all for navigation + listener, waitForFunction for DOM polling, retry loops for flaky browser launches). The changes are self-contained within integration test files.

Other factors

All reviewer feedback from CodeRabbit has been addressed (reload awaiting via Promise.all, baseUrl gate on readiness check). My previous nit about the tight margin between the inner 90s kill timeout and outer 100s bun:test timeout remains unaddressed but is genuinely low-impact — cleanup will almost certainly complete well within 10s even on slow CI. No production code is affected, no CODEOWNERS are involved, and the changes are mechanically sound.

[claude]

LGTM — straightforward test-only race condition fixes with no production code impact.

Extended reasoning...

Overview

This PR modifies two test files (dev-server-puppeteer.ts and dev-server.test.ts) to fix race conditions in Next.js dev-server HMR integration tests. The changes include: attaching console listeners before triggering navigation/reload/HMR (using Promise.all), re-querying stale DOM handles after HMR, adding a waitForFunction for Tailwind CSS styles, accumulating stdout for robust readiness detection, retry logic for browser launch, and increasing the inner subprocess timeout.

Security risks

None. This PR only touches integration test code — no production code, no auth, no crypto, no user-facing behavior.

Level of scrutiny

Low scrutiny warranted. These are test reliability improvements with clear intent. The patterns used (Promise.all for concurrent awaits, waitForFunction for polling, retry loops) are standard Puppeteer test idioms. The accumulated stdout buffer fix and && baseUrl guard (addressing a CodeRabbit finding) are both correct.

Other factors

Previous review issues from CodeRabbit (unwaited reload, baseUrl race) have been addressed in follow-up commits. My prior nit about the tight timeout margin (90s inner vs 100s outer) remains unaddressed but is very low impact — cleanup will almost certainly complete in well under 10s. The new inline nit about missing a post-HMR CSS wait is also very low impact given the implicit delays from intervening async operations. Overall the PR is a clear improvement to test robustness on slow CI machines.

[claude]

🟡 The PR adds a waitForFunction to poll for Tailwind CSS computed styles before the initial-load style assertions (lines 108-113) but does not add a similar wait after HMR before the post-HMR style assertions (lines 175-185). After HMR, Counter2 uses rounded-br-full (vs Counter1's rounded-bl-full), so the CSS changes. For consistency with the initial-load pattern, consider adding a waitForFunction polling for borderBottomRightRadius === "9999px" before the post-HMR style block.

Extended reasoning...

The inconsistency

The PR introduces a waitForFunction at lines 108-113 that polls until getComputedStyle(el).borderBottomLeftRadius === "9999px" before the initial-load style assertions, with the comment "On slow CI machines, stylesheets may not be loaded when the page first renders." However, after HMR at lines 159-165, the code only waits for the DOM text content to update (Count B:) and re-queries the element handle — it does not wait for the new Tailwind CSS class (rounded-br-full) to be reflected in computed styles before asserting on them at lines 175-185.

Step-by-step proof

1. Counter1.txt uses rounded-bl-full class, Counter2.txt uses rounded-br-full class
2. HMR triggers: copyFileSync(Counter2.txt, Counter.tsx) at line 155
3. The test waits for the HMR console message (line 156) and DOM text update (lines 159-163)
4. Lines 167-173 perform click/getCount assertions (7 async round-trips)
5. Lines 175-185 assert borderBottomRightRadius === "9999px" without an explicit CSS wait

The Tailwind JIT config scans ./src/**/*.{js,ts,jsx,tsx,mdx} — and .txt files are NOT in the content glob. So when Counter2.txt is copied to Counter.tsx, Tailwind JIT must regenerate CSS to include the new rounded-br-full utility class. This CSS HMR update could theoretically arrive after the JS HMR update.

Why practical impact is very low

The refutation raises valid points: the initial-load CSS wait addresses a fundamentally different scenario (external stylesheet HTTP fetch on first page load) versus HMR (where CSS changes are injected inline via WebSocket). Additionally, between the HMR DOM wait and the style assertions, there are 7 async Puppeteer round-trips (4 getCount calls + 3 click operations) that provide hundreds of milliseconds of implicit delay. Next.js also typically delivers CSS and JS HMR updates together or in rapid succession.

Why it is still worth noting

The same slow-CI reasoning that justified the initial-load CSS wait applies in principle here too. The PR explicitly acknowledges that slow CI machines can have timing issues with CSS — yet the post-HMR path doesn't apply the same defensive pattern. Adding await p.waitForFunction(() => { const el = document.querySelector("#counter-fixture"); return el && getComputedStyle(el).borderBottomRightRadius === "9999px"; }); before line 175 would make the code consistent and marginally more robust, at negligible cost.

Suggested fix

Add a waitForFunction polling for borderBottomRightRadius === "9999px" after the HMR DOM wait (after line 164) and before the style assertions (line 175), mirroring the initial-load pattern at lines 108-113.

[claude]

🟡 Nit: Line 25 uses const { execSync } = require("child_process") inside a conditional block, which violates the test/CLAUDE.md convention (line 218-220) to avoid dynamic require and use module-scope imports. Move this to import { execSync } from "child_process" at the top of the file and only wrap the execSync(...) usage in the platform conditional.

Extended reasoning...

What the issue is

The file test/integration/next-pages/test/dev-server-puppeteer.ts at line 25 uses a dynamic require call inside a conditional block:

if (process.platform === "darwin") {
  try {
    const { execSync } = require("child_process");
    // ...
  } catch {}
}

The convention being violated

The project’s test/CLAUDE.md (lines 218-220) explicitly states:

Avoid dynamic import & require

Only use dynamic import or require when the test is specifically testing something related to dynamic import or require. Otherwise, always use module-scope import statements.

This require("child_process") is not testing dynamic require behavior — it’s just importing a built-in module for use in a macOS quarantine workaround.

Step-by-step proof

1. test/CLAUDE.md line 218 says "Avoid dynamic import & require"
2. Line 220 clarifies: only use dynamic import/require when testing that specific feature
3. The code at line 25 uses require("child_process") inside an if (process.platform === "darwin") block
4. This is not testing dynamic require — it’s using execSync to run xattr to remove macOS quarantine attributes
5. Therefore it violates the stated convention

Impact

This is purely a coding convention issue. The dynamic require works correctly at runtime — child_process is a built-in Node/Bun module that always resolves successfully and has zero import cost. There is no functional bug here.

Suggested fix

Add a static import at the top of the file alongside the other imports:

import { execSync } from "child_process";

Then simplify the conditional block to only wrap the usage:

if (process.platform === "darwin") {
  try {
    const cachePath = join(process.env.HOME || "~", ".cache", "puppeteer");
    execSync(`xattr -rd com.apple.quarantine "${cachePath}" 2>/dev/null || true`, { stdio: "ignore" });
  } catch {}
}

This is a trivial one-line change that brings the code in line with the project’s stated conventions.