fix: stabilize Windows test harness

[BigSimmo]

Summary

• make temporary test and runner cleanup resilient to transient Windows file handles
• repair signed-image auth transitions so post-sign-out 401s do not overwrite the signed-out state
• make signed-url and PDF virtualization jsdom scheduling deterministic
• harden CLS, Playwright, Lighthouse, and test-run coordination paths; record the completed review

Verification

• npm run verify:pr-local — PASS (runtime, lock parity, changed-file formatting, lint, typecheck, full Vitest, build, offline RAG fixtures)
• jsdom stability: four independent runs of the repaired pair — 60/60 passed
• npm run format — PASS before commit

Notes

• The push-time guard skipped duplicate lint/typecheck because another worktree held the repository heavy-run lease. The same checks had already passed in verify:pr-local on this exact implementation commit.
• No provider-backed or production checks were run.

Summary by CodeRabbit

• Bug Fixes

  • Prevented signed-out or stale requests from incorrectly triggering session-expiration handling.
  • Improved cross-platform route-path handling and temporary-file cleanup.

• Reliability

  • Added retryable cleanup for temporary files and directories.
  • Improved browser and server run cleanup, process management, port selection, and validation.

• Tests

  • Strengthened authentication, visualization, browser, and cross-platform test coverage.
  • Updated the scripts index and branch review records.

[supabase]

This pull request has been ignored for the connected project sjrfecxgysukkwxsowpy because there are no changes detected in supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

---

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

[coderabbitai]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in:26 minutes

Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: adc8ec66-d314-4d39-b021-911531217bee

📥 Commits

Reviewing files that changed from the base of the PR and between 1d1b5e1 and 1b2df38.

📒 Files selected for processing (7)

• docs/scripts-index.md
• scripts/lib/http-readiness.mjs
• scripts/measure-cls-attribution.mjs
• tests/http-readiness.test.ts
• tests/issues-report.test.ts
• tests/measure-cls-attribution-contract.test.ts
• tests/test-runner-safety.test.ts

📝 Walkthrough

Walkthrough

The pull request hardens filesystem cleanup across scripts and tests, improves CLS attribution runner port and lifecycle handling, prevents stale 401 responses from expiring signed-out sessions, stabilizes browser and path tests, and adds a branch reconciliation workflow.

Changes

Runner and test reliability

Layer / File(s)	Summary
Retryable filesystem cleanup scripts/retryable-fs.mjs, scripts/run-lighthouse-budget.mjs, scripts/run-playwright.mjs, scripts/test-run-lock.mjs, tests/*	Added bounded retry handling for transient removal failures and applied it across runner and temporary-fixture cleanup paths.
CLS runner lifecycle and project validation scripts/measure-cls-attribution.mjs, tests/measure-cls-attribution-contract.test.ts	Added managed-port selection, project identity validation, heavy-run locking, owned process termination, tracked resources, and centralized teardown.
Signed-image authentication state handling src/components/clinical-dashboard/use-signed-image-url.ts, tests/auth-signed-url-cache.dom.test.tsx	401 handling now requires an authenticated identity and an active consumer. Tests use act, controlled promises, and shared event/frame helpers.
Deterministic browser and path tests tests/document-viewer-page-virtualization.dom.test.tsx, scripts/check-bundle-budget.mjs, tests/bundle-budget.test.ts	Viewer tests explicitly control animation frames and microtasks. Manifest paths and fixture lookups normalize Windows separators.
Main-branch reconciliation .github/workflows/merge-main-pr-1868.yml, docs/branch-review-ledger.md	Added a guarded workflow that merges main, resolves the expected ledger conflict, validates the result, and pushes the branch.
Script index docs/scripts-index.md	Updated the documented script count from 227 to 228.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

• BigSimmo/Database#699 — Related test-runner hardening and cleanup retries.
• BigSimmo/Database#1432 — Related Playwright runner safety and cleanup changes.
• BigSimmo/Database#1374 — Related signed-image URL authentication and cache behavior.

Suggested labels:codex

Suggested reviewers:cursoragent

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 19.44% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies the primary change: stabilizing the Windows test harness.
Description check	✅ Passed	The description covers the main changes and verification results, but it omits the required Risk and rollout section and explicit UI-gate status.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch codex/windows-test-portability

---

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit:91fc30bca7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/measure-cls-attribution.mjs`:
- Around line 242-252: Update the heavy-run cleanup flow around
stopOwnedProcessTree and removePathSync so process termination and path removal
execute within nested try/finally blocks, ensuring cleanup errors cannot bypass
lock release. Invoke lock.release() from the outer finally while preserving the
existing cleanup order and behavior.
- Around line 141-150: Add a request-level timeout in the polling request around
the visible .get call, destroying the request when it expires and scheduling
poll() again so an incomplete response cannot block indefinitely; ensure the
timeout is cleared when the response ends or the request errors. Add a
regression test using a server that accepts the request but never ends its
response, verifying polling resumes and the deadline/lease path completes.
In `@tests/test-runner-safety.test.ts`:
- Around line 104-112: Update the unsafeRecursiveRemoval check in the “requires
bounded retries” test so it inspects each individual rmSync call’s options
object and requires a positive maxRetries value within that same object,
regardless of option order. Prevent later calls, comments, or strings from
satisfying the guard, while preserving acceptance of valid bounded recursive
cleanup.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: db5137d6-524b-4335-9010-7927edbce7ff

📥 Commits

Reviewing files that changed from the base of the PR and between be5c7f5 and 8fd8c0e.

📒 Files selected for processing (34)

• docs/scripts-index.md
• scripts/check-bundle-budget.mjs
• scripts/measure-cls-attribution.mjs
• scripts/retryable-fs.mjs
• scripts/run-lighthouse-budget.mjs
• scripts/run-playwright.mjs
• scripts/test-run-lock.mjs
• src/components/clinical-dashboard/use-signed-image-url.ts
• tests/adopt-visual-baselines.test.ts
• tests/auth-signed-url-cache.dom.test.tsx
• tests/bundle-budget.test.ts
• tests/check-lighthouse-budget.test.ts
• tests/client-secret-surface.test.ts
• tests/codex-autofix-workflow.test.ts
• tests/codex-cloud-setup.test.ts
• tests/compare-retrieval-eval.test.ts
• tests/design-sync-contract.test.ts
• tests/design-system-adoption.test.ts
• tests/document-viewer-page-virtualization.dom.test.tsx
• tests/flake-ledger.test.ts
• tests/function-grants.test.ts
• tests/guard-push.test.ts
• tests/installed-lock-parity.test.ts
• tests/local-presence.test.ts
• tests/measure-cls-attribution-contract.test.ts
• tests/playwright-browser-preflight.test.ts
• tests/pr-handoff-stop.test.ts
• tests/primary-checkout-lease.test.ts
• tests/reconciliation-evidence-pack.test.ts
• tests/reconciliation-preflight.test.ts
• tests/resolve-tsx-cli.test.ts
• tests/search-results-band-adoption.test.ts
• tests/setup-codex-worktree.test.ts
• tests/test-runner-safety.test.ts

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/merge-main-pr-1868.yml:
- Around line 63-80: Replace the direct Path-based reconstruction in the
docs/branch-review-ledger.md conflict branch with the repository’s ledger
command, preserving the configured merge-driver process and existing rows. Keep
the validation that identifies the PR `#1868` row, but use the established ledger
workflow to resolve and stage the file instead of Path.write_text.
- Around line 106-114: Remove the automatic commit and push behavior from the
workflow step following “Remove helper, commit, and push,” including git commit
and git push to codex/windows-test-portability. Leave branch updates to an
authorized, confirmation-gated non-bot process and avoid any provider/API
mutation in this workflow.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 7c911bd2-14df-4ed7-818d-36ed9b72ae87

📥 Commits

Reviewing files that changed from the base of the PR and between 8fd8c0e and 1d1b5e1.

📒 Files selected for processing (2)

• .github/workflows/merge-main-pr-1868.yml
• docs/branch-review-ledger.md

[Copilot]

Pull request overview

Stabilizes the repo’s Windows-heavy test/runner harness by making temporary-path cleanup tolerant of transient file handles, tightening test determinism in jsdom scheduling, and hardening runner coordination paths (Playwright/Lighthouse/CLS) to reduce flake risk.

Changes:

• Introduce a shared removePathSync helper with bounded retries and adopt it (or equivalent rmSync retry options) across scripts and tests that delete ephemeral state.
• Make jsdom-based tests more deterministic (animation-frame/idling control) and prevent stale/signed-out auth transitions from overwriting signed-out state.
• Harden runner/coordination tooling and related contracts (bundle-budget path normalization; CLS harness port selection and isolation checks) and update docs/ledger.

Reviewed changes

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

Show a summary per file

File	Description
tests/test-runner-safety.test.ts	Adds removePathSync usage and new safety assertions guarding recursive cleanup patterns.
tests/setup-codex-worktree.test.ts	Adds bounded rmSync retries for temp worktree fixtures.
tests/search-results-band-adoption.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/resolve-tsx-cli.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/reconciliation-preflight.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/reconciliation-evidence-pack.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/primary-checkout-lease.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/pr-handoff-stop.test.ts	Skips bash-hook contract test on Windows to avoid false negatives + adds bounded cleanup retries.
tests/playwright-browser-preflight.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/measure-cls-attribution-contract.test.ts	Extends contract coverage for managed-port selection and shared runner safety boundaries.
tests/local-presence.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/installed-lock-parity.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/guard-push.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/function-grants.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/flake-ledger.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/document-viewer-page-virtualization.dom.test.tsx	Makes jsdom scheduling deterministic by explicitly controlling RAF/idle flush points.
tests/design-system-adoption.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/design-sync-contract.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/compare-retrieval-eval.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/codex-cloud-setup.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/codex-autofix-workflow.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/client-secret-surface.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/check-lighthouse-budget.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
tests/bundle-budget.test.ts	Improves cross-platform path handling in fake FS fixtures + adds bounded rmSync retries.
tests/auth-signed-url-cache.dom.test.tsx	Hardens signed-url cache/auth lifecycle tests for deterministic state changes and scheduling.
tests/adopt-visual-baselines.test.ts	Adds bounded rmSync retries for temp fixture cleanup.
src/components/clinical-dashboard/use-signed-image-url.ts	Prevents 401s from sign-out/stale identities from triggering session-expired state; fixes effect deps.
scripts/test-run-lock.mjs	Switches ephemeral coordinator cleanup from rmSync to removePathSync for Windows-tolerant deletion.
scripts/run-playwright.mjs	Uses removePathSync for runner cleanup to avoid transient handle failures.
scripts/run-lighthouse-budget.mjs	Uses removePathSync for runner cleanup and report clearing to avoid transient handle failures.
scripts/retryable-fs.mjs	New helper providing bounded retry semantics for path removal on Windows/transient handles.
scripts/measure-cls-attribution.mjs	Hardens CLS measurement harness: managed-port selection, project identity validation, heavy-run lock usage, safer cleanup.
scripts/check-bundle-budget.mjs	Normalizes diagnostic paths to forward slashes for stable cross-platform output/assertions.
docs/scripts-index.md	Updates scripts count in the docs index to reflect new scripts additions.
docs/branch-review-ledger.md	Appends the completed review record for this PR work.
.github/workflows/build-pr-1868-review-fixes.yml	Adds a PR-specific workflow intended to apply/verify “review fixes” on a named branch.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[github-actions]

CI triage

CI failed on this PR. Automated classification of the 2 failed job(s):

• Production UI (3) — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.
• PR required — needs investigation: inspect the failing step and uploaded diagnostics; rerun only after classifying the cause.

Compared with main CI run #10239 (cancelled).

Classification is evidence routing, not permission to ignore a failure. Exact quarantined Playwright identities remain governed by the flake ledger.