Skip to content

Commit 8d628a3

Browse files
committed
feat(cli): require --allow-non-empty to scaffold into a non-empty dir
Anthony's call on the open behavioral question: bare iii project init should error (not warn) when the target dir contains anything other than hidden dotfiles or iii-managed paths, and require --allow-non-empty to override. - New flag: --allow-non-empty on InitArgs. - New helper: check_directory_state() runs after create_dir_all and before scaffolder-core's copy_template (which uses fs::write and would otherwise silently overwrite the user's config.yaml/.gitignore). - Re-init is always allowed: if .iii/project.ini exists, the directory is treated as a known iii project and the guard is skipped. - "Empty enough" rule: hidden dotfiles (.git/, .env.example, etc.) and the data/ runtime directory don't count as user content. - Error message includes a sample of the offending names plus a clear fix line ("pass --allow-non-empty …"). Also DRYs the .iii/project.ini parsing per Anthony's review: read_existing_project_id and resolve_device_id_for_docker now share read_project_ini_field(root, key). Tests: 3 new e2e tests - project_init_errors_on_non_empty_dir_without_override (README.md present) - project_init_with_allow_non_empty_succeeds (override works, files preserved) - project_init_into_dir_with_only_hidden_files_succeeds (.git/ alone is fine) Replaces project_init_does_not_clobber_existing_config which tested the old (now incorrect) "silent skip-existing" behavior.
1 parent 30c1374 commit 8d628a3

2 files changed

Lines changed: 146 additions & 19 deletions

File tree

engine/src/cli/project/mod.rs

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,14 @@ pub struct InitArgs {
6868
/// Auto-confirm all prompts (non-interactive mode).
6969
#[arg(short, long)]
7070
pub yes: bool,
71+
72+
/// Allow scaffolding into a non-empty directory. Without this flag, init
73+
/// errors out if the target dir contains anything other than hidden
74+
/// dotfiles (e.g. `.git/`) or iii-managed paths (`.iii/`, `data/`).
75+
/// Re-running init in a directory with `.iii/project.ini` is always
76+
/// allowed (idempotent re-init).
77+
#[arg(long = "allow-non-empty")]
78+
pub allow_non_empty: bool,
7179
}
7280

7381
impl InitArgs {
@@ -131,6 +139,15 @@ async fn run_init(args: InitArgs) -> i32 {
131139
);
132140
}
133141

142+
if let Err(e) = check_directory_state(&root, args.allow_non_empty) {
143+
crate::cli::telemetry::send_project_init_failed("non_empty_dir", &e);
144+
return print_err(
145+
"target directory is not empty",
146+
&e,
147+
"pass --allow-non-empty to scaffold into an existing project, or pick a different directory",
148+
);
149+
}
150+
134151
let device_id = iii::workers::telemetry::environment::get_or_create_device_id();
135152
let project_name = root
136153
.file_name()
@@ -429,26 +446,28 @@ async fn persist_project_ini(
429446
}
430447

431448
fn read_existing_project_id(root: &Path) -> Option<String> {
449+
read_project_ini_field(root, "project_id")
450+
}
451+
452+
/// Read a single key from `.iii/project.ini` (flat or `[project]`-prefixed
453+
/// format), returning `None` when the file is absent, unreadable, or the key
454+
/// is missing/empty. The format-tolerant parser is shared between
455+
/// `read_existing_project_id` (used by re-init) and
456+
/// `resolve_device_id_for_docker` (used by the docker generator).
457+
fn read_project_ini_field(root: &Path, key: &str) -> Option<String> {
432458
let path = root.join(".iii").join("project.ini");
433459
let contents = std::fs::read_to_string(path).ok()?;
460+
let prefix = format!("{key}=");
434461
contents
435462
.lines()
436-
.find_map(|l| l.trim().strip_prefix("project_id="))
463+
.find_map(|l| l.trim().strip_prefix(&prefix))
437464
.map(|v| v.trim().to_string())
438465
.filter(|v| !v.is_empty())
439466
}
440467

441468
fn resolve_device_id_for_docker(root: &Path) -> String {
442-
let path = root.join(".iii").join("project.ini");
443-
let ini_exists = path.exists();
444-
let contents = std::fs::read_to_string(&path).ok();
445-
let device_id = contents.as_ref().and_then(|s| {
446-
s.lines()
447-
.find_map(|l| l.trim().strip_prefix("device_id="))
448-
.map(|v| v.trim().to_string())
449-
.filter(|v| !v.is_empty())
450-
});
451-
match device_id {
469+
let ini_exists = root.join(".iii").join("project.ini").exists();
470+
match read_project_ini_field(root, "device_id") {
452471
Some(id) => id,
453472
None => {
454473
if ini_exists {
@@ -488,6 +507,59 @@ fn resolve_root(dir: Option<&str>) -> Result<PathBuf, String> {
488507
}
489508
}
490509

510+
/// Reject scaffolding into a non-empty directory unless the user opted in via
511+
/// `--allow-non-empty`, OR the directory is already an iii project (has
512+
/// `.iii/project.ini`). Hidden dotfiles (`.git/`, `.gitignore`, etc.) and
513+
/// the `data/` runtime directory are not considered "non-empty content" —
514+
/// they're either dev tooling or iii-managed state.
515+
fn check_directory_state(root: &Path, allow_non_empty: bool) -> Result<(), String> {
516+
if !root.exists() {
517+
return Ok(());
518+
}
519+
if !root.is_dir() {
520+
return Err(format!("{} exists but is not a directory", root.display()));
521+
}
522+
// Idempotent re-init: an existing project.ini means we're scaffolding
523+
// into a directory we previously initialized. Always allowed.
524+
if root.join(".iii").join("project.ini").exists() {
525+
return Ok(());
526+
}
527+
if allow_non_empty {
528+
return Ok(());
529+
}
530+
let entries: Vec<String> = match std::fs::read_dir(root) {
531+
Ok(rd) => rd
532+
.filter_map(|e| e.ok())
533+
.map(|e| e.file_name().to_string_lossy().into_owned())
534+
.filter(|name| {
535+
// Hidden files/dirs (.git, .env.example, etc.) and iii's
536+
// own runtime data directory are not "user content" for
537+
// the purpose of this check.
538+
!name.starts_with('.') && name != "data"
539+
})
540+
.collect(),
541+
Err(e) => return Err(format!("read {}: {e}", root.display())),
542+
};
543+
if entries.is_empty() {
544+
Ok(())
545+
} else {
546+
let mut sample = entries.clone();
547+
sample.sort();
548+
let preview: Vec<String> = sample.iter().take(5).cloned().collect();
549+
let suffix = if sample.len() > 5 {
550+
format!(", and {} more", sample.len() - 5)
551+
} else {
552+
String::new()
553+
};
554+
Err(format!(
555+
"{} contains {}{}",
556+
root.display(),
557+
preview.join(", "),
558+
suffix
559+
))
560+
}
561+
}
562+
491563
fn print_err(problem: &str, cause: &str, fix: &str) -> i32 {
492564
eprintln!("{} {}", "error:".red().bold(), problem);
493565
eprintln!(" {} {}", "cause:".dimmed(), cause);

engine/tests/project_init_e2e.rs

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -206,22 +206,77 @@ fn project_generate_docker_uses_existing_project_ini_device_id() {
206206
}
207207

208208
#[test]
209-
fn project_init_does_not_clobber_existing_config() {
209+
fn project_init_errors_on_non_empty_dir_without_override() {
210+
// A pre-existing user file in the target directory should make init
211+
// refuse to scaffold (we'd otherwise silently overwrite it via
212+
// scaffolder-core's copy_template).
210213
let dir = tempdir().unwrap();
211-
std::fs::write(dir.path().join("config.yaml"), "existing: yes\n").unwrap();
214+
std::fs::write(dir.path().join("README.md"), "# my project\n").unwrap();
212215
let out = iii_bin()
213216
.args(["project", "init", "--template-dir"])
214217
.arg(fixtures())
215218
.arg("--directory")
216219
.arg(dir.path())
217220
.output()
218221
.expect("failed to run iii");
219-
assert!(out.status.success());
220-
// copy_template uses fs::write which overwrites, so a stale fixture
221-
// would clobber. We assert the file is non-empty (the bare template
222-
// should always produce something).
223-
let cfg = std::fs::read_to_string(dir.path().join("config.yaml")).unwrap();
224-
assert!(!cfg.is_empty());
222+
assert!(
223+
!out.status.success(),
224+
"init should refuse a non-empty directory without --allow-non-empty"
225+
);
226+
let stderr = String::from_utf8_lossy(&out.stderr);
227+
assert!(
228+
stderr.contains("not empty"),
229+
"expected non-empty error message:\n{stderr}"
230+
);
231+
assert!(
232+
stderr.contains("--allow-non-empty"),
233+
"error should suggest --allow-non-empty:\n{stderr}"
234+
);
235+
let readme = std::fs::read_to_string(dir.path().join("README.md")).unwrap();
236+
assert_eq!(readme, "# my project\n", "user file must be untouched");
237+
}
238+
239+
#[test]
240+
fn project_init_with_allow_non_empty_succeeds() {
241+
let dir = tempdir().unwrap();
242+
std::fs::write(dir.path().join("README.md"), "# my project\n").unwrap();
243+
let out = iii_bin()
244+
.args(["project", "init", "--allow-non-empty", "--template-dir"])
245+
.arg(fixtures())
246+
.arg("--directory")
247+
.arg(dir.path())
248+
.output()
249+
.expect("failed to run iii");
250+
assert!(
251+
out.status.success(),
252+
"init --allow-non-empty failed: {}",
253+
String::from_utf8_lossy(&out.stderr)
254+
);
255+
assert!(dir.path().join(".iii").join("project.ini").exists());
256+
assert!(dir.path().join("config.yaml").exists());
257+
// README.md isn't in the bare template's file list, so the user's copy
258+
// is left in place.
259+
assert!(dir.path().join("README.md").exists());
260+
}
261+
262+
#[test]
263+
fn project_init_into_dir_with_only_hidden_files_succeeds() {
264+
// A directory that only contains hidden dotfiles (e.g. a freshly
265+
// `git init`'d project) should not trigger the non-empty guard.
266+
let dir = tempdir().unwrap();
267+
std::fs::create_dir_all(dir.path().join(".git")).unwrap();
268+
let out = iii_bin()
269+
.args(["project", "init", "--template-dir"])
270+
.arg(fixtures())
271+
.arg("--directory")
272+
.arg(dir.path())
273+
.output()
274+
.expect("failed to run iii");
275+
assert!(
276+
out.status.success(),
277+
"init in a dir with only .git/ should succeed: {}",
278+
String::from_utf8_lossy(&out.stderr)
279+
);
225280
}
226281

227282
#[test]

0 commit comments

Comments
 (0)