From 224142867367adf00f7397615b73e3080dfc88b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Wed, 20 May 2026 12:13:45 +0800 Subject: [PATCH 1/5] test(hooks/init): add red test for copilot-instructions preservation Adds a TDD red test that asserts pre-existing user content in .github/copilot-instructions.md must survive 'rtk init --copilot'. Currently fails because run_copilot() uses write_if_changed() which truncates the file. Refs #1964 --- src/hooks/init.rs | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 21da7c748..bfd3fabd1 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -5634,4 +5634,44 @@ mod tests { "RTK end marker must be removed" ); } + + #[test] + fn test_copilot_init_preserves_existing_instructions() { + use std::env; + + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let user_content = "# My Copilot Instructions\n\n\ + Always respond in Spanish.\n\ + Never suggest npm; prefer pnpm.\n"; + fs::write(&instructions_path, user_content).unwrap(); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + let result = run_copilot(InitContext::default()); + env::set_current_dir(&original_cwd).unwrap(); + result.unwrap(); + + let final_content = fs::read_to_string(&instructions_path).unwrap(); + + assert!( + final_content.contains("Always respond in Spanish."), + "User custom rule was destroyed. Got: {final_content}" + ); + assert!( + final_content.contains("Never suggest npm; prefer pnpm."), + "User custom rule was destroyed. Got: {final_content}" + ); + assert!( + final_content.contains(RTK_BLOCK_START), + "RTK block start marker missing" + ); + assert!( + final_content.contains(RTK_BLOCK_END), + "RTK block end marker missing" + ); + } } From d10816516b4c199b06af18278ab53c76d26c2d87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Wed, 20 May 2026 12:15:05 +0800 Subject: [PATCH 2/5] fix(hooks/init): preserve user content in copilot-instructions.md run_copilot() previously called write_if_changed() which truncated any pre-existing .github/copilot-instructions.md, destroying user-authored Copilot rules. This routes copilot-instructions.md through upsert_rtk_block() (the same idempotent marker-block helper already used for CLAUDE.md and AGENTS.md). The COPILOT_INSTRUCTIONS constant is wrapped in ... markers so subsequent inits replace only RTK-owned content; the rest of the file is preserved verbatim. Malformed pre-existing blocks (opening marker without closing) are refused with a remediation message rather than silently rewritten. Fixes #1964, #1891 --- src/hooks/init.rs | 86 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index bfd3fabd1..1f95c80b6 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3682,7 +3682,8 @@ const COPILOT_HOOK_JSON: &str = r#"{ } "#; -const COPILOT_INSTRUCTIONS: &str = r#"# RTK — Token-Optimized CLI +const COPILOT_INSTRUCTIONS: &str = r#" +# RTK — Token-Optimized CLI **rtk** is a CLI proxy that filters and compresses command outputs, saving 60-90% tokens. @@ -3707,8 +3708,82 @@ rtk gain --history # Per-command savings history rtk discover # Find missed rtk opportunities rtk proxy # Run raw (no filtering) but track usage ``` + "#; +/// Upsert the RTK marker block in `copilot-instructions.md`. +/// +/// Preserves user content outside the ` ... +/// ` markers; only RTK-owned content between the +/// markers is added/updated/left untouched depending on prior state. +/// Refuses to modify malformed files (opening marker without closing). +fn upsert_copilot_instructions(path: &Path, ctx: InitContext) -> Result<()> { + let InitContext { verbose, dry_run } = ctx; + + let existing = if path.exists() { + fs::read_to_string(path) + .with_context(|| format!("Failed to read {}", path.display()))? + } else { + String::new() + }; + + let (new_content, action) = upsert_rtk_block(&existing, COPILOT_INSTRUCTIONS); + + match action { + RtkBlockUpsert::Added => { + if dry_run { + println!( + "[dry-run] would add Copilot instructions to {}", + path.display() + ); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + if verbose > 0 { + eprintln!("Added Copilot instructions to {}", path.display()); + } + } + } + RtkBlockUpsert::Updated => { + if dry_run { + println!( + "[dry-run] would update Copilot instructions in {}", + path.display() + ); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + if verbose > 0 { + eprintln!("Updated Copilot instructions in {}", path.display()); + } + } + } + RtkBlockUpsert::Unchanged => { + if verbose > 0 { + eprintln!( + "Copilot instructions already up to date: {}", + path.display() + ); + } + } + RtkBlockUpsert::Malformed => { + eprintln!( + "[warn] Found '{}' without closing marker in {}", + RTK_BLOCK_START, + path.display() + ); + eprintln!(" Action: Manually remove the incomplete block, then re-run:"); + eprintln!(" rtk init --copilot"); + anyhow::bail!( + "Refusing to modify malformed copilot-instructions.md at {}", + path.display() + ); + } + } + + Ok(()) +} + /// Entry point for `rtk init --copilot` pub fn run_copilot(ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; @@ -3724,14 +3799,9 @@ pub fn run_copilot(ctx: InitContext) -> Result<()> { let hook_path = hooks_dir.join("rtk-rewrite.json"); write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; - // 2. Write instructions + // 2. Upsert RTK marker block in copilot-instructions.md (preserves user content) let instructions_path = github_dir.join("copilot-instructions.md"); - write_if_changed( - &instructions_path, - COPILOT_INSTRUCTIONS, - "Copilot instructions", - ctx, - )?; + upsert_copilot_instructions(&instructions_path, ctx)?; if dry_run { print_dry_run_footer(); From 544a5f446bac2b81311ad3e28940d291ef7c7ce0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Wed, 20 May 2026 12:15:51 +0800 Subject: [PATCH 3/5] test(hooks/init): cover idempotency, stale-block, dry-run, fresh, malformed paths Adds five additional tests around copilot-instructions.md upsert: - idempotent re-init (no duplicated blocks, identical content) - stale RTK block in-place update with surrounding user content preserved - dry-run never creates or modifies the file - fresh install creates file with RTK marker block when no file exists - malformed pre-existing block (opening marker without closing) is refused with a clear error rather than silently rewritten Refs #1964 --- src/hooks/init.rs | 148 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 148 insertions(+) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 1f95c80b6..8c1b12862 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -5744,4 +5744,152 @@ mod tests { "RTK block end marker missing" ); } + + #[test] + fn test_copilot_init_idempotent_repeats() { + use std::env; + + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + run_copilot(InitContext::default()).unwrap(); + let after_first = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); + + run_copilot(InitContext::default()).unwrap(); + let after_second = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); + env::set_current_dir(&original_cwd).unwrap(); + + assert_eq!( + after_first, after_second, + "Second init must be a no-op (idempotent)" + ); + + let count_start = after_first.matches(RTK_BLOCK_START).count(); + let count_end = after_first.matches(RTK_BLOCK_END).count(); + assert_eq!( + count_start, 1, + "RTK_BLOCK_START must appear once, got {count_start}" + ); + assert_eq!( + count_end, 1, + "RTK_BLOCK_END must appear once, got {count_end}" + ); + } + + #[test] + fn test_copilot_init_updates_stale_block() { + use std::env; + + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let stale = format!( + "# Project rules\n\nUse rg.\n\n{}\n# OLD RTK CONTENT\nrtk foo\n{}\n", + RTK_BLOCK_START, RTK_BLOCK_END + ); + fs::write(&instructions_path, &stale).unwrap(); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + run_copilot(InitContext::default()).unwrap(); + env::set_current_dir(&original_cwd).unwrap(); + + let updated = fs::read_to_string(&instructions_path).unwrap(); + + assert!( + updated.contains("Use rg."), + "User content outside the block must be preserved" + ); + assert!( + !updated.contains("# OLD RTK CONTENT"), + "Stale RTK block content must be removed" + ); + assert!( + updated.contains("rtk cargo test"), + "Fresh COPILOT_INSTRUCTIONS content must be present" + ); + } + + #[test] + fn test_copilot_init_dry_run_no_write() { + use std::env; + + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + assert!(!instructions_path.exists()); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + let ctx = InitContext { + dry_run: true, + ..InitContext::default() + }; + run_copilot(ctx).unwrap(); + env::set_current_dir(&original_cwd).unwrap(); + + assert!( + !instructions_path.exists(), + "Dry-run must not create copilot-instructions.md" + ); + } + + #[test] + fn test_copilot_init_fresh_install_creates_file() { + use std::env; + + let temp = TempDir::new().unwrap(); + let instructions_path = temp + .path() + .join(".github") + .join("copilot-instructions.md"); + assert!(!instructions_path.exists()); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + run_copilot(InitContext::default()).unwrap(); + env::set_current_dir(&original_cwd).unwrap(); + + assert!( + instructions_path.exists(), + "Fresh install must create copilot-instructions.md" + ); + let content = fs::read_to_string(&instructions_path).unwrap(); + assert!(content.contains(RTK_BLOCK_START)); + assert!(content.contains(RTK_BLOCK_END)); + assert!(content.contains("rtk cargo test")); + } + + #[test] + fn test_copilot_init_refuses_malformed_block() { + use std::env; + + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let malformed = format!("# My rules\n\n{}\nincomplete RTK block\n", RTK_BLOCK_START); + fs::write(&instructions_path, &malformed).unwrap(); + + let original_cwd = env::current_dir().unwrap(); + env::set_current_dir(temp.path()).unwrap(); + let result = run_copilot(InitContext::default()); + env::set_current_dir(&original_cwd).unwrap(); + + assert!( + result.is_err(), + "Malformed file must cause an error, not silent rewrite" + ); + + let after = fs::read_to_string(&instructions_path).unwrap(); + assert_eq!(after, malformed, "File must not be modified when malformed"); + } } From 194a1b42159fc6d17ba7768fa0361a54edb7b133 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Wed, 20 May 2026 12:18:52 +0800 Subject: [PATCH 4/5] refactor(hooks/init): extract run_copilot_at for parallel-safe tests run_copilot(ctx) is now a thin wrapper around run_copilot_at(base, ctx); tests pass a TempDir path so they no longer mutate process-global cwd. This eliminates flakes when cargo test runs in parallel (the default). The public API is unchanged; only the internal seam moves. Refs #1964 --- src/hooks/init.rs | 76 +++++++++++++++-------------------------------- 1 file changed, 24 insertions(+), 52 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 8c1b12862..9371ea4e9 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -3721,8 +3721,7 @@ fn upsert_copilot_instructions(path: &Path, ctx: InitContext) -> Result<()> { let InitContext { verbose, dry_run } = ctx; let existing = if path.exists() { - fs::read_to_string(path) - .with_context(|| format!("Failed to read {}", path.display()))? + fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))? } else { String::new() }; @@ -3784,15 +3783,25 @@ fn upsert_copilot_instructions(path: &Path, ctx: InitContext) -> Result<()> { Ok(()) } -/// Entry point for `rtk init --copilot` +/// Entry point for `rtk init --copilot`. +/// +/// Installs in the current working directory's `.github/` subdirectory. pub fn run_copilot(ctx: InitContext) -> Result<()> { + run_copilot_at(Path::new("."), ctx) +} + +/// Same as [`run_copilot`] but operates relative to an explicit base path. +/// +/// Used by tests to avoid mutating process-global `cwd` (which is racy under +/// `cargo test`'s default parallel execution). +fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { let InitContext { dry_run, .. } = ctx; - // Install in current project's .github/ directory - let github_dir = Path::new(".github"); + let github_dir = base.join(".github"); let hooks_dir = github_dir.join("hooks"); if !dry_run { - fs::create_dir_all(&hooks_dir).context("Failed to create .github/hooks/ directory")?; + fs::create_dir_all(&hooks_dir) + .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; } // 1. Write hook config @@ -5707,8 +5716,6 @@ mod tests { #[test] fn test_copilot_init_preserves_existing_instructions() { - use std::env; - let temp = TempDir::new().unwrap(); let github_dir = temp.path().join(".github"); fs::create_dir_all(&github_dir).unwrap(); @@ -5719,11 +5726,7 @@ mod tests { Never suggest npm; prefer pnpm.\n"; fs::write(&instructions_path, user_content).unwrap(); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); - let result = run_copilot(InitContext::default()); - env::set_current_dir(&original_cwd).unwrap(); - result.unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); let final_content = fs::read_to_string(&instructions_path).unwrap(); @@ -5747,20 +5750,15 @@ mod tests { #[test] fn test_copilot_init_idempotent_repeats() { - use std::env; - let temp = TempDir::new().unwrap(); let github_dir = temp.path().join(".github"); fs::create_dir_all(&github_dir).unwrap(); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); - run_copilot(InitContext::default()).unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); let after_first = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); - run_copilot(InitContext::default()).unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); let after_second = fs::read_to_string(github_dir.join("copilot-instructions.md")).unwrap(); - env::set_current_dir(&original_cwd).unwrap(); assert_eq!( after_first, after_second, @@ -5781,8 +5779,6 @@ mod tests { #[test] fn test_copilot_init_updates_stale_block() { - use std::env; - let temp = TempDir::new().unwrap(); let github_dir = temp.path().join(".github"); fs::create_dir_all(&github_dir).unwrap(); @@ -5794,10 +5790,7 @@ mod tests { ); fs::write(&instructions_path, &stale).unwrap(); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); - run_copilot(InitContext::default()).unwrap(); - env::set_current_dir(&original_cwd).unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); let updated = fs::read_to_string(&instructions_path).unwrap(); @@ -5817,23 +5810,15 @@ mod tests { #[test] fn test_copilot_init_dry_run_no_write() { - use std::env; - let temp = TempDir::new().unwrap(); - let github_dir = temp.path().join(".github"); - fs::create_dir_all(&github_dir).unwrap(); - - let instructions_path = github_dir.join("copilot-instructions.md"); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); assert!(!instructions_path.exists()); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); let ctx = InitContext { dry_run: true, ..InitContext::default() }; - run_copilot(ctx).unwrap(); - env::set_current_dir(&original_cwd).unwrap(); + run_copilot_at(temp.path(), ctx).unwrap(); assert!( !instructions_path.exists(), @@ -5843,19 +5828,11 @@ mod tests { #[test] fn test_copilot_init_fresh_install_creates_file() { - use std::env; - let temp = TempDir::new().unwrap(); - let instructions_path = temp - .path() - .join(".github") - .join("copilot-instructions.md"); + let instructions_path = temp.path().join(".github").join("copilot-instructions.md"); assert!(!instructions_path.exists()); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); - run_copilot(InitContext::default()).unwrap(); - env::set_current_dir(&original_cwd).unwrap(); + run_copilot_at(temp.path(), InitContext::default()).unwrap(); assert!( instructions_path.exists(), @@ -5869,8 +5846,6 @@ mod tests { #[test] fn test_copilot_init_refuses_malformed_block() { - use std::env; - let temp = TempDir::new().unwrap(); let github_dir = temp.path().join(".github"); fs::create_dir_all(&github_dir).unwrap(); @@ -5879,10 +5854,7 @@ mod tests { let malformed = format!("# My rules\n\n{}\nincomplete RTK block\n", RTK_BLOCK_START); fs::write(&instructions_path, &malformed).unwrap(); - let original_cwd = env::current_dir().unwrap(); - env::set_current_dir(temp.path()).unwrap(); - let result = run_copilot(InitContext::default()); - env::set_current_dir(&original_cwd).unwrap(); + let result = run_copilot_at(temp.path(), InitContext::default()); assert!( result.is_err(), From 35a540c34b93f3fcb224edb98051dbad5264735a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E6=9D=8E=E5=86=A0=E8=BE=B0?= Date: Thu, 21 May 2026 00:01:20 +0800 Subject: [PATCH 5/5] refactor(hooks/init): share write_rtk_block, unify malformed handling Address review feedback on #1976: - Extract write_rtk_block() shared dispatcher: eliminates the duplicated 4-arm RtkBlockUpsert match between run_claude_md_mode and the former upsert_copilot_instructions (now inlined). Both callers stay under the ~60-line guideline. - Unify malformed handling: both paths now bail!() with a diagnostic and the exact recovery command. CLAUDE.md previously warned and exited 0, silently skipping the OpenCode plugin step; behaviour is now consistent. - Reorder run_copilot_at: upsert copilot-instructions.md BEFORE writing the hook config so a malformed file aborts the install without leaving a stale .github/hooks/rtk-rewrite.json on disk. Regression coverage: - test_claude_md_mode_refuses_malformed_block mirrors the existing copilot malformed test against the shared dispatcher contract. - test_copilot_init_malformed_leaves_no_hook_on_disk pins the new write order so a future re-order regression is caught. cargo fmt / clippy --all-targets / test --bin rtk: clean (1909 passed). --- src/hooks/init.rs | 300 ++++++++++++++++++++++++---------------------- 1 file changed, 159 insertions(+), 141 deletions(-) diff --git a/src/hooks/init.rs b/src/hooks/init.rs index 9371ea4e9..9af21dc57 100644 --- a/src/hooks/init.rs +++ b/src/hooks/init.rs @@ -1482,72 +1482,22 @@ fn run_claude_md_mode(global: bool, install_opencode: bool, ctx: InitContext) -> eprintln!("Writing rtk instructions to: {}", path.display()); } - if path.exists() { - let existing = fs::read_to_string(&path)?; - // upsert_rtk_block handles all 4 cases: add, update, unchanged, malformed - let (new_content, action) = upsert_rtk_block(&existing, RTK_INSTRUCTIONS); - - match action { - RtkBlockUpsert::Added => { - if dry_run { - println!("[dry-run] would add rtk instructions to {}", path.display()); - } else { - fs::write(&path, new_content)?; - println!("[ok] Added rtk instructions to existing {}", path.display()); - } - } - RtkBlockUpsert::Updated => { - if dry_run { - println!( - "[dry-run] would update rtk instructions in {}", - path.display() - ); - } else { - fs::write(&path, new_content)?; - println!("[ok] Updated rtk instructions in {}", path.display()); - } - } - RtkBlockUpsert::Unchanged => { - if !dry_run { - println!( - "[ok] {} already contains up-to-date rtk instructions", - path.display() - ); - } - return Ok(()); - } - RtkBlockUpsert::Malformed => { - eprintln!( - "[warn] Warning: Found '{}' without closing marker in {}", - RTK_BLOCK_START, - path.display() - ); + let recovery_cmd = if global { + "rtk init -g --claude-md" + } else { + "rtk init --claude-md" + }; - if let Some((line_num, _)) = existing - .lines() - .enumerate() - .find(|(_, line)| line.contains(RTK_BLOCK_START)) - { - eprintln!(" Location: line {}", line_num + 1); - } + let action = write_rtk_block( + &path, + RTK_INSTRUCTIONS, + "rtk instructions", + recovery_cmd, + ctx, + )?; - eprintln!(" Action: Manually remove the incomplete block, then re-run:"); - if global { - eprintln!(" rtk init -g --claude-md"); - } else { - eprintln!(" rtk init --claude-md"); - } - return Ok(()); - } - } - } else if dry_run { - println!( - "[dry-run] would create {} with rtk instructions", - path.display() - ); - } else { - fs::write(&path, RTK_INSTRUCTIONS)?; - println!("[ok] Created {} with rtk instructions", path.display()); + if matches!(action, RtkBlockUpsert::Unchanged) { + return Ok(()); } if global { @@ -2416,6 +2366,85 @@ fn upsert_rtk_block(content: &str, block: &str) -> (String, RtkBlockUpsert) { } } +/// Idempotently write an RTK-owned marker block into `path`, preserving user content. +/// +/// Reads the file (if any), passes it through [`upsert_rtk_block`], and writes the +/// result back via [`atomic_write`]. Refuses to modify files containing an opening +/// marker without a matching closing marker (bails with a diagnostic and the exact +/// `recovery_cmd` to re-run after manual cleanup). +/// +/// Returns the [`RtkBlockUpsert`] action so callers can branch on whether anything +/// was actually changed (e.g., to skip post-install steps on `Unchanged`). +/// +/// `label` is shown in user-facing messages (e.g., `"rtk instructions"`, +/// `"Copilot instructions"`). +fn write_rtk_block( + path: &Path, + block: &str, + label: &str, + recovery_cmd: &str, + ctx: InitContext, +) -> Result { + let InitContext { dry_run, .. } = ctx; + + let existing = if path.exists() { + fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))? + } else { + String::new() + }; + + let (new_content, action) = upsert_rtk_block(&existing, block); + + match action { + RtkBlockUpsert::Added => { + if dry_run { + println!("[dry-run] would add {} to {}", label, path.display()); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + println!("[ok] Added {} to {}", label, path.display()); + } + } + RtkBlockUpsert::Updated => { + if dry_run { + println!("[dry-run] would update {} in {}", label, path.display()); + } else { + atomic_write(path, &new_content) + .with_context(|| format!("Failed to write {}", path.display()))?; + println!("[ok] Updated {} in {}", label, path.display()); + } + } + RtkBlockUpsert::Unchanged => { + if !dry_run { + println!("[ok] {} already up to date in {}", label, path.display()); + } + } + RtkBlockUpsert::Malformed => { + eprintln!( + "[warn] Found '{}' without closing marker in {}", + RTK_BLOCK_START, + path.display() + ); + if let Some((line_num, _)) = existing + .lines() + .enumerate() + .find(|(_, line)| line.contains(RTK_BLOCK_START)) + { + eprintln!(" Location: line {}", line_num + 1); + } + eprintln!(" Action: Manually remove the incomplete block, then re-run:"); + eprintln!(" {recovery_cmd}"); + anyhow::bail!( + "Refusing to modify malformed {} at {}", + label, + path.display() + ); + } + } + + Ok(action) +} + /// Patch CLAUDE.md: add @RTK.md, migrate if old block exists fn patch_claude_md(path: &Path, ctx: InitContext) -> Result { let InitContext { verbose, dry_run } = ctx; @@ -3711,78 +3740,6 @@ rtk proxy # Run raw (no filtering) but track usage "#; -/// Upsert the RTK marker block in `copilot-instructions.md`. -/// -/// Preserves user content outside the ` ... -/// ` markers; only RTK-owned content between the -/// markers is added/updated/left untouched depending on prior state. -/// Refuses to modify malformed files (opening marker without closing). -fn upsert_copilot_instructions(path: &Path, ctx: InitContext) -> Result<()> { - let InitContext { verbose, dry_run } = ctx; - - let existing = if path.exists() { - fs::read_to_string(path).with_context(|| format!("Failed to read {}", path.display()))? - } else { - String::new() - }; - - let (new_content, action) = upsert_rtk_block(&existing, COPILOT_INSTRUCTIONS); - - match action { - RtkBlockUpsert::Added => { - if dry_run { - println!( - "[dry-run] would add Copilot instructions to {}", - path.display() - ); - } else { - atomic_write(path, &new_content) - .with_context(|| format!("Failed to write {}", path.display()))?; - if verbose > 0 { - eprintln!("Added Copilot instructions to {}", path.display()); - } - } - } - RtkBlockUpsert::Updated => { - if dry_run { - println!( - "[dry-run] would update Copilot instructions in {}", - path.display() - ); - } else { - atomic_write(path, &new_content) - .with_context(|| format!("Failed to write {}", path.display()))?; - if verbose > 0 { - eprintln!("Updated Copilot instructions in {}", path.display()); - } - } - } - RtkBlockUpsert::Unchanged => { - if verbose > 0 { - eprintln!( - "Copilot instructions already up to date: {}", - path.display() - ); - } - } - RtkBlockUpsert::Malformed => { - eprintln!( - "[warn] Found '{}' without closing marker in {}", - RTK_BLOCK_START, - path.display() - ); - eprintln!(" Action: Manually remove the incomplete block, then re-run:"); - eprintln!(" rtk init --copilot"); - anyhow::bail!( - "Refusing to modify malformed copilot-instructions.md at {}", - path.display() - ); - } - } - - Ok(()) -} - /// Entry point for `rtk init --copilot`. /// /// Installs in the current working directory's `.github/` subdirectory. @@ -3804,14 +3761,22 @@ fn run_copilot_at(base: &Path, ctx: InitContext) -> Result<()> { .with_context(|| format!("Failed to create {} directory", hooks_dir.display()))?; } - // 1. Write hook config + // 1. Upsert RTK marker block in copilot-instructions.md (preserves user content). + // Done BEFORE writing the hook config so a malformed file aborts the install + // without leaving a stale hook on disk. + let instructions_path = github_dir.join("copilot-instructions.md"); + write_rtk_block( + &instructions_path, + COPILOT_INSTRUCTIONS, + "Copilot instructions", + "rtk init --copilot", + ctx, + )?; + + // 2. Write hook config (only reached if the upsert above succeeded). let hook_path = hooks_dir.join("rtk-rewrite.json"); write_if_changed(&hook_path, COPILOT_HOOK_JSON, "Copilot hook config", ctx)?; - // 2. Upsert RTK marker block in copilot-instructions.md (preserves user content) - let instructions_path = github_dir.join("copilot-instructions.md"); - upsert_copilot_instructions(&instructions_path, ctx)?; - if dry_run { print_dry_run_footer(); } else { @@ -5864,4 +5829,57 @@ mod tests { let after = fs::read_to_string(&instructions_path).unwrap(); assert_eq!(after, malformed, "File must not be modified when malformed"); } + + #[test] + fn test_copilot_init_malformed_leaves_no_hook_on_disk() { + // Regression: a malformed copilot-instructions.md aborted the install + // mid-way, but the hook config had already been written. The upsert + // now runs first, so the hook config must not appear when the upsert + // bails. + let temp = TempDir::new().unwrap(); + let github_dir = temp.path().join(".github"); + fs::create_dir_all(&github_dir).unwrap(); + + let instructions_path = github_dir.join("copilot-instructions.md"); + let malformed = format!("# My rules\n\n{}\nincomplete RTK block\n", RTK_BLOCK_START); + fs::write(&instructions_path, &malformed).unwrap(); + + let hook_path = github_dir.join("hooks").join("rtk-rewrite.json"); + + let result = run_copilot_at(temp.path(), InitContext::default()); + + assert!(result.is_err(), "Malformed file must cause a hard error"); + assert!( + !hook_path.exists(), + "Hook config must not be written when the upsert aborts: {}", + hook_path.display() + ); + } + + #[test] + fn test_claude_md_mode_refuses_malformed_block() { + // Mirrors `test_copilot_init_refuses_malformed_block`: a malformed + // CLAUDE.md previously emitted a warning and exited 0, silently + // skipping the OpenCode plugin step. The shared `write_rtk_block` + // dispatcher now bails for both paths. + let tmp = TempDir::new().unwrap(); + with_claude_dir_override(&tmp, |claude_dir| { + let claude_md = claude_dir.join(CLAUDE_MD); + let malformed = format!( + "# Existing notes\n\n{}\nincomplete RTK block\n", + RTK_BLOCK_START + ); + fs::write(&claude_md, &malformed).unwrap(); + + let result = run_claude_md_mode(true, false, InitContext::default()); + + assert!( + result.is_err(), + "Malformed CLAUDE.md must cause a hard error, not silent skip" + ); + + let after = fs::read_to_string(&claude_md).unwrap(); + assert_eq!(after, malformed, "File must not be modified when malformed"); + }); + } }