init: scaffold bunfig.toml with minimumReleaseAge by default#30532
init: scaffold bunfig.toml with minimumReleaseAge by default#30532robobun wants to merge 3 commits into
Conversation
bun init now writes a bunfig.toml with [install].minimumReleaseAge set to 86400 (1 day) for the blank, library, and react templates. An existing bunfig.toml is never overwritten, and --minimal skips it.
WalkthroughThe PR adds supply-chain protection to Changesbunfig.toml supply-chain protection
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@test/cli/init/init.test.ts`:
- Around line 352-355: The test currently only checks for the presence of the
minimumReleaseAge key in the generated bunfig, which is brittle; update the
assertions in the init test (the block that reads bunfig.toml and the similar
block at the other location) to assert the exact default value instead of just
the key — for example replace expect(bunfig).toContain("minimumReleaseAge") with
an equality or contains check that matches the full setting like
expect(bunfig).toContain("minimumReleaseAge = 86400") or use a regex to assert
the numeric value 86400 so the contract is locked to that default.
- Around line 375-393: Replace the manual for...of loop that creates tests for
each flag with a parameterized describe.each([...]) block so each value in
["--react", "--react=tailwind", "--react=shadcn"] becomes a separate sub-suite;
move the existing test body (which uses tempDirWithFiles, Bun.spawn with cmd
[bunExe(), "init", flag], reading bunfig via fs.readFileSync and assertions for
"[install]", "minimumReleaseAge", and "[serve.static]") into the parameterized
test callback and reference the parameter (flag) instead of the loop variable;
ensure the test timeout (30_000) and env bunEnv/stdio settings remain unchanged
and that the test name interpolates the flag for clarity.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 62cfbec6-b566-4b62-a95d-1215d8bbc8f9
📒 Files selected for processing (8)
docs/quickstart.mdxdocs/runtime/templating/init.mdxsrc/cli/init/bunfig.default.tomlsrc/cli/init/react-app/bunfig.tomlsrc/cli/init/react-shadcn/bunfig.tomlsrc/cli/init/react-tailwind/bunfig.tomlsrc/cli/init_command.zigtest/cli/init/init.test.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
The implementation looks clean and follows the existing .gitignore/README.md pattern, but this changes the default bun init output for every new project and the 1d vs 2d vs 3d default is still an open question (plus the alternative approach in #28729), so a maintainer should sign off — CI is also currently red on build-zig/build-cpp.
Extended reasoning...
Overview
This PR makes bun init scaffold a bunfig.toml with [install].minimumReleaseAge = 86400 for all non-minimal templates. It touches: a new src/cli/init/bunfig.default.toml asset, three existing React template bunfig.toml files, ~10 lines in src/cli/init_command.zig (new embedded asset + write_bunfig step that mirrors the existing .gitignore/README handling), two doc pages, and test/cli/init/init.test.ts (snapshot updates + 5 new cases).
Security risks
None introduced by the code itself — it writes a static embedded file to a freshly scaffolded project directory and never overwrites an existing bunfig.toml. No injection, auth, or path-handling concerns. The intent of the change is itself a security hardening (supply-chain protection).
Level of scrutiny
Medium-high. The Zig and template changes are mechanical and low-risk in isolation, but this is a user-facing product decision: every bun init will now produce an extra file and every scaffolded project will reject packages published in the last 24h. The author explicitly flags the default value (1d / 2d / 3d) as an open question, and the duplicate-PR bot points to #28729 which solves the same problem via a runtime default rather than a scaffolded file. A maintainer should decide which approach (and which value) is preferred.
Other factors
- CI is red on commit 898c4fe — build-zig and build-cpp are failing across all targets per robobun. The subsequent autofix.ci commit may or may not resolve this, but it should be green before merge.
- CodeRabbit left two minor nits (assert exact
86400value; usedescribe.each). Neither is blocking. - Test coverage for the new behavior is good (writes file, respects existing file, skips on
--minimal, present in all three React variants). - The
bunfig.tomlis written before the scaffoldingbun installruns in the blank/library path, so the initial install is covered — but note that in the React path (write files and run bun dev),bunfig.tomlis written viacreateNew(O_EXCL) as the first template file, which also precedes the install, so that's consistent.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
test/cli/init/init.test.ts (1)
375-393: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse
describe.each()for the React flag parameterization.The manual
for...ofloop should be converted todescribe.each()to match the test style guide.As per coding guidelines: "Use describe.each() for parameterized tests".♻️ Suggested refactor
- for (const flag of ["--react", "--react=tailwind", "--react=shadcn"]) { - test(`bun init ${flag} bunfig.toml has minimumReleaseAge`, async () => { + describe.each(["--react", "--react=tailwind", "--react=shadcn"])("bun init %s", flag => { + test("bunfig.toml has minimumReleaseAge", async () => { const temp = tempDirWithFiles(`bun-init-bunfig-${flag.replaceAll(/[^a-z]/g, "-")}`, {}); @@ expect(bunfig).toContain("[serve.static]"); }, 30_000); - } + });🤖 Prompt for 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. In `@test/cli/init/init.test.ts` around lines 375 - 393, Replace the manual for...of loop that creates tests for each React flag with a describe.each([...]) block so tests follow the style guide; use the same flag array ["--react", "--react=tailwind", "--react=shadcn"] in describe.each, move the test body that calls tempDirWithFiles, Bun.spawn (cmd: [bunExe(), "init", flag]), awaits exited, reads bunfig via fs.readFileSync(path.join(temp, "bunfig.toml"), "utf8") and asserts the same expectations (contains "[install]", "minimumReleaseAge = 86400", "[serve.static]") into the parameterized it/test callback, and preserve the 30_000 timeout and test name template (e.g., `bun init ${flag} bunfig.toml has minimumReleaseAge`).
🤖 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 `@test/cli/init/init.test.ts`:
- Around line 343-351: Change each Bun.spawn call that currently sets stdio to
["ignore","inherit","inherit"] (the ones creating { exited }) to pipe
stdout/stderr instead (e.g., stdio: ["ignore","pipe","pipe"]), capture the
output streams (read stdout/stderr text from the spawn result) and assert
expected stdout/stderr contents before asserting expect(await exited).toBe(0);
specifically update the spawn invocation and follow it with reading the
spawnResult.stdout/stderr into strings and performing expectations on those
strings (use the same bunExe(), temp, bunEnv variables) prior to the final
exit-code assertion; apply the same pattern to the other spawn blocks that
mirror this one.
---
Duplicate comments:
In `@test/cli/init/init.test.ts`:
- Around line 375-393: Replace the manual for...of loop that creates tests for
each React flag with a describe.each([...]) block so tests follow the style
guide; use the same flag array ["--react", "--react=tailwind", "--react=shadcn"]
in describe.each, move the test body that calls tempDirWithFiles, Bun.spawn
(cmd: [bunExe(), "init", flag]), awaits exited, reads bunfig via
fs.readFileSync(path.join(temp, "bunfig.toml"), "utf8") and asserts the same
expectations (contains "[install]", "minimumReleaseAge = 86400",
"[serve.static]") into the parameterized it/test callback, and preserve the
30_000 timeout and test name template (e.g., `bun init ${flag} bunfig.toml has
minimumReleaseAge`).
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: 8d388635-5e48-4253-8838-cf6830a57e0f
📒 Files selected for processing (1)
test/cli/init/init.test.ts
| const { exited } = Bun.spawn({ | ||
| cmd: [bunExe(), "init", "-y"], | ||
| cwd: temp, | ||
| stdio: ["ignore", "inherit", "inherit"], | ||
| env: bunEnv, | ||
| }); | ||
|
|
||
| expect(await exited).toBe(0); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Capture spawned process output before final exit-code assertion.
These new tests assert exited directly with inherited stdio, which weakens failure diagnostics. Prefer piping output and checking expected output before asserting the exit code.
♻️ Suggested pattern (apply to each new spawn block)
- const { exited } = Bun.spawn({
+ const { exited, stderr } = Bun.spawn({
cmd: [bunExe(), "init", "-y"],
cwd: temp,
- stdio: ["ignore", "inherit", "inherit"],
+ stdio: ["ignore", "inherit", "pipe"],
env: bunEnv,
});
- expect(await exited).toBe(0);
+ const [exitCode, stderrText] = await Promise.all([exited, stderr.text()]);
+ if (exitCode !== 0) {
+ expect(stderrText).toBe("");
+ }
+ expect(exitCode).toBe(0);Also applies to: 362-370, 379-387
🤖 Prompt for 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.
In `@test/cli/init/init.test.ts` around lines 343 - 351, Change each Bun.spawn
call that currently sets stdio to ["ignore","inherit","inherit"] (the ones
creating { exited }) to pipe stdout/stderr instead (e.g., stdio:
["ignore","pipe","pipe"]), capture the output streams (read stdout/stderr text
from the spawn result) and assert expected stdout/stderr contents before
asserting expect(await exited).toBe(0); specifically update the spawn invocation
and follow it with reading the spawnResult.stdout/stderr into strings and
performing expectations on those strings (use the same bunExe(), temp, bunEnv
variables) prior to the final exit-code assertion; apply the same pattern to the
other spawn blocks that mirror this one.
|
Closing — enabling |
There was a problem hiding this comment.
Implementation looks clean and well-tested, but this changes the default bun init output for every new project and the author is explicitly asking for input on the 1d vs 2d vs 3d value (and there's overlap with #28729's approach) — worth a human signing off on the product decision.
Extended reasoning...
Overview
This PR makes bun init scaffold a bunfig.toml containing [install].minimumReleaseAge = 86400 for all templates. Touches: a new embedded asset (src/cli/init/bunfig.default.toml), the three React template bunfig.toml files, ~10 lines in src/cli/init_command.zig to register and conditionally write the asset, two docs pages, and test/cli/init/init.test.ts (snapshot updates + 5 new cases).
Security risks
None. The change writes a static, embedded TOML file into a freshly scaffolded directory. No user input flows into the file path or contents, no auth/crypto/permissions code is touched. The feature itself is a security hardening (supply-chain delay).
Level of scrutiny
The code is low-risk and mechanical — write_bunfig is wired identically to the existing write_gitignore / write_readme steps (gated on !minimal and !existsZ("bunfig.toml")), and test coverage is thorough (creation, no-overwrite, --minimal exclusion, all three React variants, exact value assertion per CodeRabbit's now-resolved comment).
The behavior change, however, is user-facing and global: every bun init will now emit an extra file and constrain the initial install. That's a product/default decision rather than a bug fix or refactor.
Other factors
- The author explicitly flags the default value as open ("Happy to change to 2 days … or 3 days"), inviting maintainer input.
- A bot flagged potential overlap with #28729, which sets the same default at the runtime level instead of via scaffolded config — a human should decide which approach (or both) is desired.
- No CODEOWNERS apply to the touched paths; CodeRabbit's two comments are resolved.
Given the open product question and the overlap with another PR, I'm deferring rather than approving, even though I see nothing wrong with the implementation itself.
1 participant
What
bun initnow writes abunfig.tomlwith[install].minimumReleaseAgeenabled for all templates:bunfig.tomlis created (previously none was written)bunfig.tomltemplates gain the[install]section above their[serve.static]section--minimal: skipped, consistent with.gitignore/README.mdbunfig.tomlis never overwrittenThe file is written before the scaffolding
bun installruns, so the initial install is also protected.How
src/cli/init/bunfig.default.toml— new default assetsrc/cli/init/react-{app,tailwind,shadcn}/bunfig.toml— added[install]sectionsrc/cli/init_command.zig— register asset, addwrite_bunfigstep (gated on!minimaland!existsZ("bunfig.toml"))docs/runtime/templating/init.mdx,docs/quickstart.mdx— listbunfig.tomlin generated filesTests
test/cli/init/init.test.ts— 15 pass, 0 fail. New cases:bun initwritesbunfig.tomlcontaining[install]+minimumReleaseAgebunfig.tomlis not overwritten--minimaldoes not createbunfig.toml--react/--react=tailwind/--react=shadcnbunfig includes both[install]and[serve.static]bunfig.tomlin directory listingsNotes
The default value is 86400 seconds (1 day). Happy to change to 2 days (aligning with #30529) or 3 days (matching the docs examples) — it's a one-line change to the template files.