[miguel-heygen]

What changes

Every CLI browser test now starts Chrome with launchManagedBrowser and closes it with closeManagedBrowser (packages/cli/src/browser/launch.ts). That close gives Chrome 1 s, then kills it and waits for the exit. A bare browser.close() has no such limit: when the DevTools connection wedges it can wait out Puppeteer's multi-minute protocol timeout (the engine says the same at packages/engine/src/services/browserManager.ts:406).

scripts/check-cli-process-ownership.mjs, which already runs in bun run lint and the commit hooks, now also fails when a CLI test calls <something>browser.close() or puppeteer.launch(), so a new test cannot slip back.

Why

On Windows CI, colorGradingShaderStages.browser.test.ts failed twice with Hook timed out in 30000ms at afterAll(async () => { await browser?.close(); }), once in a merge-queue run, with every test in the file passed. It launches Chrome on software GL outside the CLI's owner. The other 14 files close the same way, so they move too instead of waiting for each to flake.

Changes

• 8 tests that called puppeteer.launch now call launchManagedBrowser(puppeteer, ...) with the same options, and close with closeManagedBrowser.
• 7 tests that got their browser from CLI code (already launched through the owner) now close it with closeManagedBrowser instead of browser.close().
• The ownership check gains listUnmanagedTestBrowsers for *.test.ts files; process-exit rules for source files are unchanged. A new case in check-cli-process-ownership.test.mjs fails without the rule.
• No shared wrapper: the files find Chrome three different ways (ensureBrowser, PUPPETEER_EXECUTABLE_PATH, findSystemBrowser), and launchManagedBrowser already is the one owner.

Verification

• All 15 files on Linux with the CLI's own Chrome (headless shell 152): 15 files, 122 tests passed, none skipped, 4 runs in a row.
• The check exits 0 on this branch. Run on the unconverted colorGradingShaderStages test from main it reports both sites (:106 calls puppeteer.launch(), :122 calls browser.close()). Putting await browser.close(); back into a converted test fails it with the file and line. Run over all 382 CLI test files on main, the rule flags exactly the 23 sites in these 15 files.
• tsc --noEmit (cli), oxfmt, oxlint, comment citations and the comment ratchet: all exit 0.

Not in this PR

Several CLI commands still close a managed browser with a bare browser.close() (compare, snapshot, layout, validate, motion-shot, grade-compare, the capture watchdog and capture attempt, the composition prelaunch). Same risk in shipped code; that change and extending this check to source files come in the next PR.

[jrusso1020]

The 15 test files consistently use the managed launcher/closer without weakening their assertions. packages/cli/src/browser/launch.ts:229 gives normal shutdown one second, then sends SIGKILL and waits for exit/process disappearance with a second one-second ceiling; it does not just abandon Chrome.

Blocker — one converted test still reaches an unbounded browser close before its new cleanup. packages/cli/src/capture/captureCompositionFrame.prelaunch.browser.test.ts:38 exercises the 800px prelaunch against a 640px composition. openSettledCompositionPage calls prelaunched.take(); the mismatched key enters discard() and awaits bare browser.close() at packages/cli/src/capture/captureCompositionFrame.ts:157. The test has not entered its try/finally at line 44 yet, so the new managed closes at lines 50–51 cannot rescue a wedged DevTools close. An exact-function probe with a never-resolving close confirms mismatch take() remains pending. Route this indirect cleanup through the bounded owner too (including the disconnected matching-key path at line 166), and add a regression with a hung close. Deferring other production callers is reasonable, but this helper is directly exercised by one of this PR's 15 tests.

Nonblocking guard limitation: scripts/check-cli-process-ownership.mjs:45 matches receiver text ending in browser, rather than browser identity. Probes flag await browser.close() and optional chaining but accept page.browser().close(), destructured/aliased close, computed access, context.close(), and even the early?.close() spelling replaced in this PR. Production files are correctly excluded from this new check. Extend the intended browser variants and document context-close scope; the rule currently prevents only the named spellings.

Audited: all 17 changed files and the shared close implementation; traced the capture/prelaunch integration. Validation: four checker tests passed with only its TypeScript import redirected to the installed dependency; targeted syntax/scope probes and exact-function mismatch probe executed. A fresh independent pass reproduced the prelaunch gap and exercised the exact close functions with real Node child processes: normal close, hung close (SIGKILL after ~1 second), rejected close, already-exited process, and concurrent deduplication passed. No full 15-suite or Windows run by me. No prior reviews/comments were present on my initial check.

Verdict: REQUEST CHANGES
Reasoning: The direct teardown conversions are sound, but the prelaunch test can still wait on the same unbounded close before reaching its new cleanup.

— hf-review