Skip to content

test(codex): share one Bun transpiler cache across write-lock children - #5732

Merged
lidge-jun merged 1 commit into
devfrom
codex/260924-write-lock-shared-transpiler-cache
Sep 24, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/260924-write-lock-shared-transpiler-cache

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 24, 2026 •

Copy link
Copy Markdown
Owner

Summary

tests/codex-integration/codex-write-lock.test.ts failed twice on dev (ci.yml lane=all run 35953803435, windows 1/9, attempts 1 and 2) with the same error: one OS user and one home take ONE lock, case 0 timed out after 15000 ms waiting for the holder child's marker, with the child still running. No product code changed; #5730 added a test file, which shifted this file to the front of its batch on that shard.

The cause is in the fixture. The home-environment cases give each child a brand-new fake HOME/USERPROFILE, and Bun keeps its runtime transpiler cache under the user's home cache directory unless BUN_RUNTIME_TRANSPILER_CACHE_PATH is set: HOME on macOS/Linux and USERPROFILE on Windows (Bun 1.4.0). A child started with a fresh home writes a new @t@ cache (261 files, 4.1 MB, measured on macOS), so every holder in those cases re-transpiled the whole lock graph, and the describe's beforeAll warm-up did not help. The Windows logs show the split: in the same describe, children with the ambient home booted in 1.2-3.7 s, while the fake-home cases took 5-19 s.

This PR pins BUN_RUNTIME_TRANSPILER_CACHE_PATH to one per-process temp directory for the warm-up and for every child in the describe, and removes it in afterAll. The explicit override takes precedence on every platform. The lock identity under test is derived only from HOME/USERPROFILE, so every assertion is unchanged and no budget is raised.

Verification

  • Probe: a fresh HOME without the pin created 261 cache files under it; two fresh homes with the pin created none (all 261 went to the shared directory).

  • bun test tests/codex-integration/codex-write-lock.test.ts: 20 pass, 0 fail (case 0/1/2 in about 0.3 s each on macOS); the shared cache directory is removed afterwards.

  • bun run typecheck: pass.

  • The Windows proof is ci.yml lane=all on dev after merge (the owner verifies on dev).

  • Review: gpt-6-sol adversarial review NEAR-PASS (assertions unchanged; cache sharing and cleanup sound; Windows cache-home precision folded into the comment and this description).

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 24, 2026 04:42
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-24T04:44:55.615138Z edbaa81 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 24, 2026
@coderabbitai

coderabbitai Bot commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: a942ba1f-c276-45f7-854f-a7d5f019bcc5

📥 Commits

Reviewing files that changed from the base of the PR and between 74cd423 and edbaa81.

📒 Files selected for processing (1)
  • tests/codex-integration/codex-write-lock.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

The lock integration test now uses a temporary shared transpiler cache for spawned child processes. It warms the cache before the tests and removes it afterward.

Changes

Lock test cache

Layer / File(s) Summary
Cache setup and child spawning
tests/codex-integration/codex-write-lock.test.ts
The test creates a temporary transpiler cache, warms the child module graph, and passes the cache path to both child-spawning helpers. An afterAll hook removes the cache.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to edbaa

The cache change appears mergeable after normal checks. Windows verification remains planned, but the supplied evidence does not establish a failure that should block merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. 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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the change: the test now shares one Bun transpiler cache across write-lock child processes. It is concise, specific, and directly matches the pull request changes.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

The home-environment cases give every child a fresh fake HOME, and Bun keeps its
runtime transpiler cache under the HOME-derived cache directory, so each holder
re-transpiled the whole lock graph and the describe's warm-up paid for nothing.
On windows-latest those holders took 5-19 s against 1-4 s for the same child with
the ambient home, and "case 0" crossed INTERNAL_DEADLINE_MS twice in run
35953803435 after a new test file shifted it to the front of its batch.

Pin BUN_RUNTIME_TRANSPILER_CACHE_PATH to one per-process directory for the
warm-up and every child. The lock identity under test reads only HOME and
USERPROFILE, so no assertion changes.
@lidge-jun
lidge-jun force-pushed the codex/260924-write-lock-shared-transpiler-cache branch from edbaa81 to dcb6c8a Compare September 24, 2026 04:46
@lidge-jun
lidge-jun merged commit 8b562bf into dev Sep 24, 2026
5 of 7 checks passed
@lidge-jun
lidge-jun deleted the codex/260924-write-lock-shared-transpiler-cache branch September 24, 2026 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant