Skip to content

bun init --react: don't overwrite existing README.md - #35164

Closed
robobun wants to merge 2 commits into
mainfrom
claude/farm/b9587333/init-react-readme-overwrite
Closed

robobun wants to merge 2 commits into
mainfrom
claude/farm/b9587333/init-react-readme-overwrite

Conversation

@robobun

@robobun robobun commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator

Fixes #2892.

Repro

D=$(mktemp -d) && cd $D && mkdir src
printf 'MY README - do not lose me\n' > README.md
printf '{"name":"mine"}\n' > package.json
printf 'export const mine = 1\n' > src/index.ts
CI=true bun init -y --react </dev/null | grep -E 'skipping|README|src/index'
head -1 README.md

Before:

 ○ package.json (already exists, skipping)
 ○ tsconfig.json (already exists, skipping)
 + README.md
 ○ src/index.ts (already exists, skipping)
# bun-react-template          <- user's README gone

After:

 ○ package.json (already exists, skipping)
 ○ tsconfig.json (already exists, skipping)
 ○ README.md (already exists, skipping)
 ○ src/index.ts (already exists, skipping)
MY README - do not lose me

Cause

write_files_and_run_bun_dev in src/runtime/cli/init_command.rs routes every template file through Assets::create_new (O_CREAT|O_EXCL → EEXIST → skip message) except README.md, which is special-cased through Assets::create_with_contents so {name}/{bunVersion} can be substituted. That helper opened with O_CREAT|O_TRUNC, silently clobbering an existing file. The blank-template path already guards README via exists_z("README.md"|"README"|...); the react writer skipped that guard.

Fix

Substitute the placeholders first, then write through create_new like every other file in the loop. Existing README.md now hits the same EEXIST → skip path. Also skip when README/README.txt/README.mdx exist, matching the blank template. The now-dead create_with_contents/create_full_with_contents helpers (only ever used for this one call, and the O_TRUNC footgun itself) are removed.

Verification

  • New test bun init --react does not overwrite an existing README.md fails on canary (+ README.md, contents clobbered), passes with this change.
  • New test bun init --react into an empty dir still writes a templated README.md confirms fresh dirs still get the substituted template.
  • Full test/cli/init/init.test.ts suite: 17 pass.

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/init/init.test.ts

The react template writer routed README.md through Assets::create_with_contents
(O_CREAT|O_TRUNC) to substitute {name}/{bunVersion}, while every other template
file used Assets::create_new (O_CREAT|O_EXCL) and printed 'already exists,
skipping' on EEXIST. Running 'bun init --react' in a directory that already had
a README.md silently truncated the user's README to the template stub.

Substitute the placeholders up front and write through create_new like the
other files, so README.md gets the same EEXIST guard and skip message. Also
skip when README/README.txt/README.mdx exist, matching the blank template.
The now-unused create_with_contents/create_full_with_contents helpers (which
only existed for this one call and were the O_TRUNC footgun) are removed.

Fixes #2892
@robobun

robobun commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: Ready for review. Diff is green; CI red is unrelated to this change.

Reproduced with:

mkdir src && printf 'MY README\n' > README.md && printf 'export const x=1\n' > src/index.ts
CI=true bun init -y --react </dev/null

README.md is truncated to # bun-react-template while src/index.ts and every other existing file print (already exists, skipping).

PR: #35164

CI (build #78020): test/cli/init/init.test.ts passes on all lanes. Remaining red is unrelated to bun init:

  • test/js/node/test/parallel/test-net-connect-memleak.js and test-gc-http-client-connaborted.js: GC-timing tests also failing on main #77601.
  • darwin-26-aarch64: The number of VMs exceeds the system limit (CI infra, no test ran).
  • webview-chrome / require-cache / no-orphans / s3: flaky, passed on retry.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026 •

Copy link
Copy Markdown
Contributor

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: 32 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

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: ASSERTIVE

Plan: Pro

Run ID: 77d99b75-f088-4236-a911-ca0ce4b09e62

📥 Commits

Reviewing files that changed from the base of the PR and between 47597ab and 75ae67a.

📒 Files selected for processing (2)
  • src/runtime/cli/init_command.rs
  • test/cli/init/init.test.ts

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

@robobun

robobun commented Jul 22, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 2:15 PM PT - Jul 22nd, 2026

❌ @autofix-ci[bot], your commit 75ae67a has 3 failures in Build #78020 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 35164

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

bun-35164 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — routes the react-template README through the same O_CREAT|O_EXCL path as every other file, so an existing README now yields the standard skip message instead of being truncated.

What was reviewed:

  • Confirmed create_with_contents/create_full_with_contents have no remaining callers after removal.
  • The synthesized Err(crate::Error::Sys(EEXIST)) for alt-README names hits the existing matches! skip branch and prints README.md (already exists, skipping).
  • Assets::create_new takes &[u8] (not &'static), so passing the substituted Vec<u8> is fine.
  • Two new tests cover both the regression (existing README preserved) and the happy path (fresh dir still gets templated README with name/version substituted).
Extended reasoning...

Overview

Changes write_files_and_run_bun_dev in src/runtime/cli/init_command.rs so that README.md is written via Assets::create_new (O_CREAT|O_EXCL) like every other template file, instead of the special-cased create_with_contents helper that opened with O_TRUNC and clobbered existing files. Template substitution now happens up front into a local buffer. Also matches the blank-template behavior of skipping when README/README.txt/README.mdx already exist. The two dead helpers (create_with_contents, create_full_with_contents) — which existed solely for this one call site and were the O_TRUNC footgun — are deleted. Two tests are added to test/cli/init/init.test.ts.

Security risks

None. This is scaffolding output for bun init; the change strictly narrows what gets written (never truncates an existing file where it previously would).

Level of scrutiny

Low-to-medium. bun init is not a hot path or security-sensitive; the change is a small localized refactor that unifies README handling with the existing per-file loop. The fix is at the right layer (the one caller that bypassed the EEXIST guard) rather than adding a defensive check downstream.

Other factors

  • Grepped for the removed helper names across src/ — no remaining references, so the dead-code deletion is complete.
  • The manually-constructed crate::Error::Sys(bun_errno::SystemErrno::EEXIST) matches the exact shape the existing error handler already pattern-matches on (init_command.rs:1638), so the alt-README-name case produces the same "(already exists, skipping)" line as a real EEXIST from open().
  • Assets::create_new signature is (&ZStr, &[u8]) with no 'static bound on contents, so the local Vec<u8> from substitute is a valid argument; ZStr::from_slice_with_nul(b"README.md\0") mirrors the existing ZStr::from_static(b"CLAUDE.md\0") usage in the same file.
  • Tests follow file conventions (tempDirWithFiles, initEnv, concurrent pipe drain via Promise.all, exit code asserted last, issue URL comment on the regression test) and assert both the stdout skip message and the on-disk file contents.

@robobun

robobun commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator Author

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-22, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.

@robobun robobun closed this Sep 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants