diff --git a/.claude/skills/running-tend/SKILL.md b/.claude/skills/running-tend/SKILL.md index 77771cfbed..6054ee1f17 100644 --- a/.claude/skills/running-tend/SKILL.md +++ b/.claude/skills/running-tend/SKILL.md @@ -75,6 +75,15 @@ cargo test --test integration # integration tests only CI runs on Linux, Windows, and macOS. +## Rework a test that reaches for its environment + +A test that leans on inherited state — the process CWD, an ambient env var — +sets up its own instead, via `TestRepo::with_initial_commit()` plus a tempdir, +the way most worktrunk tests already do. Guarding it with an early return +(**Don't "fix" tests by adding skip guards** in `/tend-ci-runner:fix-a-bug`) +drops the coverage rather than restoring it. This governs every workflow that +fixes a test here, not just issue triage. + ## Session Log Paths The artifact directory is named after the agent's working directory, and from @@ -243,28 +252,6 @@ across every repo. It's the project's preferred extension point. 5. Link to the [aliases docs](https://worktrunk.dev/extending/#aliases) and [tips & patterns](https://worktrunk.dev/tips-patterns/). -### Don't fix tests by adding skip guards - -When a test fails because production code or test setup can't handle some -scenario, fix the production code or rework the test setup. Don't add an -early-return skip — that removes the safety net while looking like a fix. -If a triage fix reaches for `let Ok(_) = ... else { return };`, a newly-added -`if !path.exists() { return; }`, or a fresh `#[ignore]`, stop and ask what -production behavior is actually broken. - -If the test relies on inherited environment (process CWD, ambient env -vars), rework it to set up its own — most worktrunk tests already do this -via `TestRepo::with_initial_commit()` plus a tempdir. - -### Same-root-cause-class triage - -The "work on the existing PR if it addresses the same problem" rule keys -on the same test. It doesn't catch a different test failing for the same -underlying reason. Group failing tests by root-cause class before writing -a fix; if an outstanding PR addresses any test in the class, wait for it -to merge and re-run, then mirror its approach for any sites still failing -rather than opening a parallel PR with a weaker fix. - ## Weekly Maintenance: MSRV & Toolchain Bump both MSRV and the development toolchain to **latest stable − 1**. When diff --git a/src/commands/list/collect/tasks.rs b/src/commands/list/collect/tasks.rs index fa1d3c716a..a171b1a52b 100644 --- a/src/commands/list/collect/tasks.rs +++ b/src/commands/list/collect/tasks.rs @@ -837,7 +837,7 @@ pub(super) fn parse_working_tree_status(status_output: &str) -> (WorkingTreeStat has_staged = true; } - if index_status == 'R' { + if index_status == 'R' || worktree_status == 'R' { has_renamed = true; } @@ -991,4 +991,16 @@ mod tests { assert!(!has_tracked_changes("?? artifact.bin")); assert!(has_tracked_changes("?? artifact.bin\n M src/main.rs")); } + + #[test] + fn worktree_rename_is_reported_as_dirty() { + let (status, is_dirty, has_conflicts) = parse_working_tree_status(" R old.txt -> new.txt"); + + assert_eq!( + status, + WorkingTreeStatus::new(false, false, false, true, false) + ); + assert!(is_dirty); + assert!(!has_conflicts); + } } diff --git a/src/git/parse.rs b/src/git/parse.rs index 69dc339e1d..4854085374 100644 --- a/src/git/parse.rs +++ b/src/git/parse.rs @@ -160,7 +160,7 @@ pub fn parse_porcelain_z(output: &str) -> Vec { files.push(path.to_string()); // For renames (R) and copies (C), the next NUL-separated field is the old path - if (status.starts_with('R') || status.starts_with('C')) + if status.contains(['R', 'C']) && let Some(old_path) = entries.next() { files.push(old_path.to_string()); @@ -193,7 +193,7 @@ pub fn parse_untracked_files(status_output: &str) -> Vec { } // Skip old path for renames/copies (we don't care about them here) - if status.starts_with('R') || status.starts_with('C') { + if status.contains(['R', 'C']) { entries.next(); } } @@ -434,6 +434,13 @@ mod tests { assert_eq!(files, vec!["keep.rs", "new.rs", "old.rs", "untracked.txt"]); } + #[test] + fn test_parse_porcelain_z_worktree_rename() { + let output = " R new.rs\0old.rs\0?? untracked.txt\0"; + let files = parse_porcelain_z(output); + assert_eq!(files, vec!["new.rs", "old.rs", "untracked.txt"]); + } + #[test] fn test_parse_porcelain_z_spaces_in_path() { let output = " M path with spaces/file name.rs\0"; @@ -480,6 +487,16 @@ mod tests { assert_eq!(files, vec!["untracked.txt"]); } + #[test] + fn test_parse_untracked_files_skips_worktree_rename_old_path() { + // A rename/copy in either status column has an extra old-path field. + // Make that old path look like another rename so a parser that treats + // it as a status entry will consume the real untracked record. + let output = " R new.rs\0R old.rs\0?? untracked.txt\0"; + let files = parse_untracked_files(output); + assert_eq!(files, vec!["untracked.txt"]); + } + #[test] fn test_parse_untracked_files_no_untracked() { let output = " M modified.rs\0A added.rs\0";