fix: refuse to overwrite existing minimal.toml without --force - #1053
Merged
norrietaylor merged 2 commits intoJul 29, 2026
Conversation
run_init_flow wrote minimal.toml unconditionally and reported "Created" even when it replaced an existing file. On non-TTY stdin confirm() read EOF as its default true, so `min init -y` and `--no-input` silently overwrote a user's edited config with no backup. Add an existence check that refuses the overwrite unless the new --force flag is passed, mirroring `min session destroy --all`, which already refuses non-interactively and names the flag to proceed. The plan banner and result line now report "overwrite"/"Updated" when an existing file is being replaced. Refs: #1032
📝 WalkthroughWalkthrough
ChangesInit overwrite control
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
Comment |
norrietaylor
marked this pull request as ready for review
July 29, 2026 18:39
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/minimal/src/lib.rs (1)
2533-2555: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMake the no-force write atomic.
A file can be created after
exists()and beforestd::fs::write()—especially while the confirmation prompt is open—then be truncated without--force. Usecreate_new(true)for the non-force path so the filesystem enforces the no-overwrite policy.Proposed fix
- std::fs::write(&plan.toml_path, &plan.content) - .with_context(|| format!("writing {}", plan.toml_path.display()))?; + if force { + std::fs::write(&plan.toml_path, &plan.content) + .with_context(|| format!("writing {}", plan.toml_path.display()))?; + } else { + use std::io::Write as _; + + let mut output = match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&plan.toml_path) + { + Ok(output) => output, + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => bail!( + "refusing to overwrite existing {} without --force", + plan.toml_path.display() + ), + Err(error) => { + return Err(error).with_context(|| format!("writing {}", plan.toml_path.display())) + } + }; + output + .write_all(plan.content.as_bytes()) + .with_context(|| format!("writing {}", plan.toml_path.display()))?; + }🤖 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 `@crates/minimal/src/lib.rs` around lines 2533 - 2555, Update the write flow around the `exists` check and `std::fs::write` so non-force writes use an atomic create-new operation that fails if the target appears after the check or confirmation prompt. Preserve overwrite behavior when `force` is enabled, and retain the existing `with_context` error reporting for the write operation.
🤖 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.
Outside diff comments:
In `@crates/minimal/src/lib.rs`:
- Around line 2533-2555: Update the write flow around the `exists` check and
`std::fs::write` so non-force writes use an atomic create-new operation that
fails if the target appears after the check or confirmation prompt. Preserve
overwrite behavior when `force` is enabled, and retain the existing
`with_context` error reporting for the write operation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a59d1ffe-e07b-4215-a26a-1342cfc25bf7
📒 Files selected for processing (1)
crates/minimal/src/lib.rs
norrietaylor
enabled auto-merge (squash)
July 29, 2026 18:52
norrietaylor
approved these changes
Jul 29, 2026
norrietaylor
deleted the
inbox-patch/init-overwrite-guard-13f1897f9fe157d9
branch
July 29, 2026 19:13
norrietaylor
added a commit
that referenced
this pull request
Jul 29, 2026
The merge commit c0f9a4f resolved conflicts in crates/minimal/src/lib.rs by taking the pre-merge side, silently reverting two fixes that had landed on main: - #1053 — the `min init` refuse-to-overwrite guard: `InitArgs::force`, the bail on an existing `minimal.toml`, and the Updated/Created wording. Without it `min init` overwrites an existing `minimal.toml` with no backup and no prompt, which is the data loss #1032 was filed for. - #1060 — the `SHELL=/bin/sh` pin on the ssh ProxyCommand and the `SendEnv` forwarding of LANG/LC_*/TZ. Without the pin, attach dies at "banner exchange ... Broken pipe" for any user whose $SHELL is a bare name (fish) or absent from the ssh context. Neither revert was caught by the suite: there is no test for the init guard, and `interactive_attach_requires_a_tty_on_stdin` only asserts the error contains "not a TTY" and "--command", which holds either way. Rebuilt lib.rs from main and reapplied only the intended change, so the diff against main is now exactly the `hide = true` on `AttachArgs::command` plus the non-TTY error reword: +4/-3, was +9/-51. Verified: rustfmt clean; `cargo build -p minimal --locked` ok; `min session attach --help` no longer lists `-c`/`--command` while `--command` still parses; `min init --help` lists `--force` again. `cargo test -p minimal` cannot run on macOS (dev-deps pull minimald -> procfs/caps, Linux-only), so the suite is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
norrietaylor
added a commit
that referenced
this pull request
Jul 29, 2026
…surface (#1071) * chore(minimal): hide `min session attach --command` from help Add `hide = true` to the `#[arg(long, short)]` attribute on `AttachArgs::command` so `-c` / `--command` no longer appear in `min session attach --help`, `-h`, or shell completions. The flag stays functional for existing scripted callers; the option promises a general remote exec it cannot deliver (no PTY, only three daemon commands accepted), so it should not be advertised for discovery. The non-TTY attach error still points stuck callers at `--command`; reword it to frame the flag as a deliberate hidden escape hatch rather than drop the reference, keeping the advice actionable now that the flag is off the help surface. * fix(minimal): restore #1053 and #1060 clobbered by the merge commit The merge commit c0f9a4f resolved conflicts in crates/minimal/src/lib.rs by taking the pre-merge side, silently reverting two fixes that had landed on main: - #1053 — the `min init` refuse-to-overwrite guard: `InitArgs::force`, the bail on an existing `minimal.toml`, and the Updated/Created wording. Without it `min init` overwrites an existing `minimal.toml` with no backup and no prompt, which is the data loss #1032 was filed for. - #1060 — the `SHELL=/bin/sh` pin on the ssh ProxyCommand and the `SendEnv` forwarding of LANG/LC_*/TZ. Without the pin, attach dies at "banner exchange ... Broken pipe" for any user whose $SHELL is a bare name (fish) or absent from the ssh context. Neither revert was caught by the suite: there is no test for the init guard, and `interactive_attach_requires_a_tty_on_stdin` only asserts the error contains "not a TTY" and "--command", which holds either way. Rebuilt lib.rs from main and reapplied only the intended change, so the diff against main is now exactly the `hide = true` on `AttachArgs::command` plus the non-TTY error reword: +4/-3, was +9/-51. Verified: rustfmt clean; `cargo build -p minimal --locked` ok; `min session attach --help` no longer lists `-c`/`--command` while `--command` still parses; `min init --help` lists `--force` again. `cargo test -p minimal` cannot run on macOS (dev-deps pull minimald -> procfs/caps, Linux-only), so the suite is left to CI. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: gominimal-aw-bot[bot] <281738952+gominimal-aw-bot[bot]@users.noreply.github.com> Co-authored-by: Norrie Taylor <91171431+norrietaylor@users.noreply.github.com> Co-authored-by: Norrie Taylor <norrie@minimal.dev> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1032
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABKmgbEQ
run_init_flowwroteminimal.tomlunconditionally and reported "Created" even when it replaced an existing file; on non-TTY stdinconfirm()returns its defaulttrueon EOF, somin init -yand--no-inputsilently overwrote an edited config with no backup. This adds an existence check that refuses the overwrite unless the new--forceflag is passed, mirroringmin session destroy --all, which already refuses non-interactively and names the flag to proceed (the shared helper was extracted in #682; informed by #682). The plan banner and result line now report "overwrite"/"Updated" when replacing; fresh-create behaviour is unchanged, matching the issue's scope. No natural unit-test hook exists — the helper needs a live graph checkout — so the guard is proven by the verification gate below.Verification
cargo fmt --all --check — clean
cargo clippy --workspace --locked -- -D warnings — 0 warnings
cargo build --workspace --locked — ok
cargo test --workspace --locked — ok (minimal crate 131 passed; 0 failed across workspace)
Note
Refuse to overwrite existing
minimal.tomlwithout--forceinmin initrun_init_flowin lib.rs now checks ifminimal.tomlalready exists and exits with an error unless--forceis passed.forcefield toInitArgsand threads it throughcmd_initintorun_init_flow.min initon a directory with an existingminimal.tomlnow fails by default instead of prompting to overwrite.Macroscope summarized 3cb8f52.
Summary by CodeRabbit
New Features
--forceoption tomin init, allowing existing configuration files to be overwritten.Bug Fixes
--forceis provided.