From ad658788013bd702fe8ac52d649a384dd6252617 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 07:59:07 -0700 Subject: [PATCH 1/4] docs: add implementation plan for cargo fmt CI fix Co-Authored-By: Claude Opus 4.6 --- docs/plans/2026-03-10-fix-cargo-fmt.md | 62 ++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 docs/plans/2026-03-10-fix-cargo-fmt.md diff --git a/docs/plans/2026-03-10-fix-cargo-fmt.md b/docs/plans/2026-03-10-fix-cargo-fmt.md new file mode 100644 index 0000000..4fb929f --- /dev/null +++ b/docs/plans/2026-03-10-fix-cargo-fmt.md @@ -0,0 +1,62 @@ +# Fix cargo fmt CI Failure Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use trycycle-executing to implement this plan task-by-task. + +**Goal:** Fix all `cargo fmt` formatting violations so the GitHub Actions CI pipeline passes. + +**Architecture:** Run `cargo fmt` to auto-format all Rust source files, then verify the full CI check sequence (`fmt --check`, `clippy`, `test`) passes. No logic changes — formatting only. + +**Tech Stack:** Rust toolchain (cargo fmt, clippy, cargo test) + +**Decision: Use `cargo fmt` auto-format rather than manual edits.** +All 17 diffs across 5 files are standard rustfmt line-length and argument-wrapping reformats. Running `cargo fmt` is idiomatic, deterministic, and guaranteed to produce the exact output `cargo fmt -- --check` expects. Manual editing would be error-prone and pointless. + +--- + +### Task 1: Run cargo fmt to auto-format all files + +**Files:** +- Modify: `src/commands/init.rs` (2 formatting diffs — long method chain and long `eprintln!` call) +- Modify: `src/commands/ls.rs` (4 formatting diffs — long `println!` and multi-arg `assert!` calls) +- Modify: `src/config.rs` (5 formatting diffs — long method chains and long `.args()` arrays) +- Modify: `src/platform.rs` (1 formatting diff — `unwrap_or_else` chain wrapping) +- Modify: `tests/e2e.rs` (5 formatting diffs — `.args()` arrays, `temp_dir().join()` chains, compact `assert!`) + +**Step 1: Run cargo fmt** + +```bash +cargo fmt +``` + +Expected: exits 0, no output. All 5 files are reformatted in place. + +**Step 2: Verify cargo fmt --check passes** + +```bash +cargo fmt -- --check +``` + +Expected: exits 0 with no output (no remaining diffs). + +**Step 3: Run cargo clippy** + +```bash +cargo clippy --all-targets -- -D warnings +``` + +Expected: exits 0. Formatting changes cannot introduce clippy warnings, but this mirrors the CI step that was previously blocked by the format failure. + +**Step 4: Run cargo test** + +```bash +cargo test +``` + +Expected: exits 0. Formatting changes cannot break tests, but this mirrors the CI step and confirms the full `check` job would pass. + +**Step 5: Commit** + +```bash +git add src/commands/init.rs src/commands/ls.rs src/config.rs src/platform.rs tests/e2e.rs +git commit -m "style: apply cargo fmt to fix CI formatting check" +``` From d3fbbf564b3dbd13541110d72596f65b8b67e019 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:03:57 -0700 Subject: [PATCH 2/4] test: add test plan for cargo fmt CI fix Co-Authored-By: Claude Opus 4.6 --- .../2026-03-10-fix-cargo-fmt-test-plan.md | 73 +++++++++++++++++++ 1 file changed, 73 insertions(+) create mode 100644 docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md diff --git a/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md b/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md new file mode 100644 index 0000000..19cd5ea --- /dev/null +++ b/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md @@ -0,0 +1,73 @@ +# Fix cargo fmt CI Failure — Test Plan + +## Strategy reconciliation + +The testing strategy (run `cargo fmt -- --check`, `cargo clippy`, `cargo test`) maps 1:1 to the CI workflow steps in `.github/workflows/ci.yml`. The implementation plan makes only formatting changes via `cargo fmt` — no logic, no new interfaces, no external dependencies. The strategy holds as-is with no adjustments needed. + +**Source of truth:** `.github/workflows/ci.yml` defines the exact commands and flags the CI pipeline runs. All assertions below mirror those commands. + +## Harness requirements + +None. All tests use the local Rust toolchain (`cargo fmt`, `cargo clippy`, `cargo test`) which is already available. No test harnesses need to be built. + +## Test plan + +### 1. Formatting check passes with zero diffs + +- **Name:** `cargo fmt --check` produces no diffs after formatting +- **Type:** invariant +- **Harness:** `cargo fmt -- --check` (same command as CI) +- **Preconditions:** `cargo fmt` has been run on the worktree +- **Actions:** Run `cargo fmt -- --check` +- **Expected outcome:** Exit code 0, empty stdout. Source of truth: `.github/workflows/ci.yml` line 27 (`cargo fmt -- --check`). Any non-zero exit or diff output means the fix is incomplete. +- **Interactions:** None. Pure formatting check, no compilation or linking. + +### 2. Clippy passes with no warnings + +- **Name:** Clippy lints pass after formatting changes +- **Type:** integration +- **Harness:** `cargo clippy --all-targets -- -D warnings` (same command as CI) +- **Preconditions:** Source files have been reformatted by `cargo fmt` +- **Actions:** Run `cargo clippy --all-targets -- -D warnings` +- **Expected outcome:** Exit code 0, no warnings or errors. Source of truth: `.github/workflows/ci.yml` line 29 (`cargo clippy --all-targets -- -D warnings`). While formatting changes cannot introduce clippy warnings, this step was previously blocked by the format failure, so it must be verified to confirm the full CI job would pass. +- **Interactions:** Exercises the full Rust compilation pipeline. Any pre-existing clippy issue unrelated to formatting would surface here. + +### 3. Test suite passes after formatting changes + +- **Name:** All tests pass after formatting changes +- **Type:** integration +- **Harness:** `cargo test` (same command as CI) +- **Preconditions:** Source files have been reformatted by `cargo fmt` +- **Actions:** Run `cargo test` +- **Expected outcome:** Exit code 0, all tests pass. Source of truth: `.github/workflows/ci.yml` line 31 (`cargo test`). Formatting changes cannot alter logic, but this confirms the third CI step passes end-to-end. +- **Interactions:** Compiles and runs all unit and integration tests including `tests/e2e.rs` (which is one of the reformatted files). The e2e tests invoke the compiled binary as a subprocess, so this exercises the full build-and-run path. + +### 4. Only formatting changes in the diff + +- **Name:** Diff contains only whitespace/line-wrapping changes, no logic modifications +- **Type:** invariant +- **Harness:** `git diff` inspection +- **Preconditions:** `cargo fmt` has been run but changes are not yet committed (or compare the commit diff) +- **Actions:** Run `git diff -- src/ tests/` and inspect the output +- **Expected outcome:** All hunks contain only whitespace, indentation, and line-break changes. No identifier, keyword, operator, or string literal changes. The affected files are exactly: `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`. Source of truth: the `cargo fmt -- --check` output captured before the fix, which showed 17 diffs across these 5 files — all line-wrapping reformats. +- **Interactions:** None. + +### 5. Correct files are modified + +- **Name:** Exactly the 5 expected files are changed, no others +- **Type:** boundary +- **Harness:** `git diff --name-only` +- **Preconditions:** `cargo fmt` has been run +- **Actions:** Run `git diff --name-only` (or `git diff --name-only HEAD~1` after commit) +- **Expected outcome:** Output contains exactly these 5 paths (in any order): `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`. No other files are modified. Source of truth: the `cargo fmt -- --check` output which identified diffs in exactly these files. +- **Interactions:** None. + +## Coverage summary + +**Covered:** +- All three CI `check` job steps (`fmt --check`, `clippy`, `test`) are verified with the exact same commands and flags used in CI. +- Diff correctness: only formatting changes, only expected files. + +**Excluded (per strategy):** +- The `install-script` CI job (`tests/test_install.sh`) is not affected by Rust source formatting and is excluded. Risk: none — that job is independent and was not failing. +- No cross-platform or Docker-based Linux test verification. The CI runs on `ubuntu-latest`; local verification runs on macOS. Risk: negligible for formatting-only changes, since `rustfmt` output is platform-independent. From 4fbb6501206af761c701f0d71850e7a57df1fc37 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:05:55 -0700 Subject: [PATCH 3/4] style: apply cargo fmt and fix clippy collapsible-if warnings Run cargo fmt to fix all 17 formatting diffs across 5 files that were failing the CI format check. Also collapse nested if-let chains in tests/e2e.rs to satisfy clippy::collapsible_if. Co-Authored-By: Claude Opus 4.6 --- src/commands/init.rs | 11 ++++-- src/commands/ls.rs | 22 +++++++++--- src/config.rs | 42 +++++++++++++++++++---- src/platform.rs | 5 ++- tests/e2e.rs | 79 ++++++++++++++++++-------------------------- 5 files changed, 97 insertions(+), 62 deletions(-) diff --git a/src/commands/init.rs b/src/commands/init.rs index fa26fc0..6337d1a 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -56,7 +56,10 @@ pub fn is_devproxy_process(pid: i32) -> bool { // On Linux, read /proc//exe symlink. let exe = std::fs::read_link(format!("/proc/{pid}/exe")); match exe { - Ok(path) => path.file_name().map(|n| n == "devproxy" || n == "devproxy-daemon").unwrap_or(false), + Ok(path) => path + .file_name() + .map(|n| n == "devproxy" || n == "devproxy-daemon") + .unwrap_or(false), Err(_) => false, } } @@ -328,7 +331,11 @@ pub fn install_daemon_binary(src: &std::path::Path, dest: &std::path::Path) -> R super::update::prepare_binary(dest)?; - eprintln!("{} daemon binary installed at {}", "ok:".green(), dest.display()); + eprintln!( + "{} daemon binary installed at {}", + "ok:".green(), + dest.display() + ); Ok(()) } diff --git a/src/commands/ls.rs b/src/commands/ls.rs index ff4f2f0..ad6ca09 100644 --- a/src/commands/ls.rs +++ b/src/commands/ls.rs @@ -29,7 +29,12 @@ pub async fn run() -> Result<()> { .unwrap_or(3) .max(3); // at least "URL".len() - println!(" {: String { if composite.len() <= 63 { return composite; } - composite.chars().take(63).collect::().trim_end_matches('-').to_string() + composite + .chars() + .take(63) + .collect::() + .trim_end_matches('-') + .to_string() } /// Find a free ephemeral port @@ -487,7 +492,12 @@ services: .output() .unwrap(); std::process::Command::new("git") - .args(["remote", "add", "origin", "https://github.com/user/my-cool-app.git"]) + .args([ + "remote", + "add", + "origin", + "https://github.com/user/my-cool-app.git", + ]) .current_dir(dir.path()) .output() .unwrap(); @@ -504,7 +514,12 @@ services: .output() .unwrap(); std::process::Command::new("git") - .args(["remote", "add", "origin", "git@github.com:user/another-app.git"]) + .args([ + "remote", + "add", + "origin", + "git@github.com:user/another-app.git", + ]) .current_dir(dir.path()) .output() .unwrap(); @@ -573,16 +588,29 @@ services: #[test] fn compose_slug_basic() { - assert_eq!(compose_slug("swift-penguin", "devproxy"), "swift-penguin-devproxy"); + assert_eq!( + compose_slug("swift-penguin", "devproxy"), + "swift-penguin-devproxy" + ); } #[test] fn compose_slug_truncates_to_63_chars() { let long_app = "a".repeat(100); let result = compose_slug("swift-penguin", &long_app); - assert!(result.len() <= 63, "composite slug must fit in a DNS label: len={}", result.len()); - assert!(!result.ends_with('-'), "should not end with hyphen after truncation"); - assert!(result.starts_with("swift-penguin-"), "should preserve the random slug prefix"); + assert!( + result.len() <= 63, + "composite slug must fit in a DNS label: len={}", + result.len() + ); + assert!( + !result.ends_with('-'), + "should not end with hyphen after truncation" + ); + assert!( + result.starts_with("swift-penguin-"), + "should preserve the random slug prefix" + ); } #[test] diff --git a/src/platform.rs b/src/platform.rs index dc234c3..87b4192 100644 --- a/src/platform.rs +++ b/src/platform.rs @@ -57,9 +57,8 @@ pub fn generate_launchagent_plist( // launchd provides a minimal PATH (/usr/bin:/bin:/usr/sbin:/sbin) that // excludes /usr/local/bin (Docker on macOS) and ~/.local/bin. Include // the common paths so the daemon can find docker, etc. - let path_value = std::env::var("PATH").unwrap_or_else(|_| { - "/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin".to_string() - }); + let path_value = std::env::var("PATH") + .unwrap_or_else(|_| "/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin".to_string()); let path_value = xml_escape(&path_value); let mut env_entries = format!( diff --git a/tests/e2e.rs b/tests/e2e.rs index 6f3586c..f2a8ab6 100644 --- a/tests/e2e.rs +++ b/tests/e2e.rs @@ -72,7 +72,12 @@ fn copy_fixtures(test_name: &str) -> PathBuf { .output() .expect("git init failed"); Command::new("git") - .args(["remote", "add", "origin", "https://github.com/test/e2e-fixture.git"]) + .args([ + "remote", + "add", + "origin", + "https://github.com/test/e2e-fixture.git", + ]) .current_dir(&dest) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -1291,17 +1296,16 @@ fn test_launchd_socket_activation() { // Signal-based cleanup for fallback path let pid_path = config_dir.join("daemon.pid"); - if pid_path.exists() { - if let Ok(pid_str) = std::fs::read_to_string(&pid_path) { - if let Ok(pid) = pid_str.trim().parse::() { - unsafe { - libc::kill(pid, libc::SIGTERM); - } - std::thread::sleep(Duration::from_millis(500)); - unsafe { - libc::kill(pid, libc::SIGKILL); - } - } + if pid_path.exists() + && let Ok(pid_str) = std::fs::read_to_string(&pid_path) + && let Ok(pid) = pid_str.trim().parse::() + { + unsafe { + libc::kill(pid, libc::SIGTERM); + } + std::thread::sleep(Duration::from_millis(500)); + unsafe { + libc::kill(pid, libc::SIGKILL); } } @@ -1357,14 +1361,10 @@ fn test_version_works_with_daemon_running() { /// from the CLI binary path. #[test] fn test_init_daemon_binary_path_separation() { - let config_dir = std::env::temp_dir().join(format!( - "devproxy-config-path-sep-{}", - std::process::id() - )); - let data_dir = std::env::temp_dir().join(format!( - "devproxy-data-path-sep-{}", - std::process::id() - )); + let config_dir = + std::env::temp_dir().join(format!("devproxy-config-path-sep-{}", std::process::id())); + let data_dir = + std::env::temp_dir().join(format!("devproxy-data-path-sep-{}", std::process::id())); if config_dir.exists() { std::fs::remove_dir_all(&config_dir).unwrap(); } @@ -1378,13 +1378,7 @@ fn test_init_daemon_binary_path_separation() { // Run init with daemon (uses fallback spawn since DEVPROXY_NO_SOCKET_ACTIVATION=1) let output = Command::new(devproxy_bin()) - .args([ - "init", - "--domain", - TEST_DOMAIN, - "--port", - &port.to_string(), - ]) + .args(["init", "--domain", TEST_DOMAIN, "--port", &port.to_string()]) .env("DEVPROXY_CONFIG_DIR", &config_dir) .env("DEVPROXY_DATA_DIR", &data_dir) .env("DEVPROXY_NO_SOCKET_ACTIVATION", "1") @@ -1394,10 +1388,7 @@ fn test_init_daemon_binary_path_separation() { let stderr = String::from_utf8_lossy(&output.stderr); eprintln!("init stderr: {stderr}"); - assert!( - output.status.success(), - "init should succeed: {stderr}" - ); + assert!(output.status.success(), "init should succeed: {stderr}"); // Verify daemon binary was installed at the data dir path let daemon_bin = data_dir.join("devproxy-daemon"); @@ -1430,17 +1421,16 @@ fn test_init_daemon_binary_path_separation() { // Clean up daemon let pid_path = config_dir.join("daemon.pid"); - if pid_path.exists() { - if let Ok(pid_str) = std::fs::read_to_string(&pid_path) { - if let Ok(pid) = pid_str.trim().parse::() { - unsafe { - libc::kill(pid, libc::SIGTERM); - } - std::thread::sleep(Duration::from_millis(500)); - unsafe { - libc::kill(pid, libc::SIGKILL); - } - } + if pid_path.exists() + && let Ok(pid_str) = std::fs::read_to_string(&pid_path) + && let Ok(pid) = pid_str.trim().parse::() + { + unsafe { + libc::kill(pid, libc::SIGTERM); + } + std::thread::sleep(Duration::from_millis(500)); + unsafe { + libc::kill(pid, libc::SIGKILL); } } @@ -1452,10 +1442,7 @@ fn test_init_daemon_binary_path_separation() { /// and produce the same output as the CLI binary. #[test] fn test_daemon_binary_matches_cli_binary() { - let data_dir = std::env::temp_dir().join(format!( - "devproxy-data-match-{}", - std::process::id() - )); + let data_dir = std::env::temp_dir().join(format!("devproxy-data-match-{}", std::process::id())); if data_dir.exists() { std::fs::remove_dir_all(&data_dir).unwrap(); } From 48e41c914d1cb3b4de23e3213e7fe3008062ccc4 Mon Sep 17 00:00:00 2001 From: Chris Fenton Date: Tue, 10 Mar 2026 08:09:41 -0700 Subject: [PATCH 4/4] docs: update planning docs to reflect clippy collapsible-if fix The plan and test plan previously claimed "formatting only" and "no identifier, keyword, or operator changes," which was inaccurate after the collapsible-if fix was needed. Update both docs to accurately describe the clippy lint fix as a semantically equivalent AST-level change alongside the cargo fmt formatting changes. Co-Authored-By: Claude Opus 4.6 --- docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md | 16 ++++++++-------- docs/plans/2026-03-10-fix-cargo-fmt.md | 8 +++++--- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md b/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md index 19cd5ea..a3540c0 100644 --- a/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md +++ b/docs/plans/2026-03-10-fix-cargo-fmt-test-plan.md @@ -2,7 +2,7 @@ ## Strategy reconciliation -The testing strategy (run `cargo fmt -- --check`, `cargo clippy`, `cargo test`) maps 1:1 to the CI workflow steps in `.github/workflows/ci.yml`. The implementation plan makes only formatting changes via `cargo fmt` — no logic, no new interfaces, no external dependencies. The strategy holds as-is with no adjustments needed. +The testing strategy (run `cargo fmt -- --check`, `cargo clippy`, `cargo test`) maps 1:1 to the CI workflow steps in `.github/workflows/ci.yml`. The implementation plan makes formatting changes via `cargo fmt` and fixes pre-existing `clippy::collapsible_if` warnings in `tests/e2e.rs` that were previously masked by the format failure. No new interfaces or external dependencies. **Source of truth:** `.github/workflows/ci.yml` defines the exact commands and flags the CI pipeline runs. All assertions below mirror those commands. @@ -29,8 +29,8 @@ None. All tests use the local Rust toolchain (`cargo fmt`, `cargo clippy`, `carg - **Harness:** `cargo clippy --all-targets -- -D warnings` (same command as CI) - **Preconditions:** Source files have been reformatted by `cargo fmt` - **Actions:** Run `cargo clippy --all-targets -- -D warnings` -- **Expected outcome:** Exit code 0, no warnings or errors. Source of truth: `.github/workflows/ci.yml` line 29 (`cargo clippy --all-targets -- -D warnings`). While formatting changes cannot introduce clippy warnings, this step was previously blocked by the format failure, so it must be verified to confirm the full CI job would pass. -- **Interactions:** Exercises the full Rust compilation pipeline. Any pre-existing clippy issue unrelated to formatting would surface here. +- **Expected outcome:** Exit code 0, no warnings or errors. Source of truth: `.github/workflows/ci.yml` line 29 (`cargo clippy --all-targets -- -D warnings`). This step was previously blocked by the format failure. Pre-existing clippy warnings (e.g., `clippy::collapsible_if` in `tests/e2e.rs`) must also be fixed to achieve a clean pass. +- **Interactions:** Exercises the full Rust compilation pipeline. Any pre-existing clippy issue unrelated to formatting will surface here and must be addressed. ### 3. Test suite passes after formatting changes @@ -42,14 +42,14 @@ None. All tests use the local Rust toolchain (`cargo fmt`, `cargo clippy`, `carg - **Expected outcome:** Exit code 0, all tests pass. Source of truth: `.github/workflows/ci.yml` line 31 (`cargo test`). Formatting changes cannot alter logic, but this confirms the third CI step passes end-to-end. - **Interactions:** Compiles and runs all unit and integration tests including `tests/e2e.rs` (which is one of the reformatted files). The e2e tests invoke the compiled binary as a subprocess, so this exercises the full build-and-run path. -### 4. Only formatting changes in the diff +### 4. Only formatting and clippy-lint changes in the diff -- **Name:** Diff contains only whitespace/line-wrapping changes, no logic modifications +- **Name:** Diff contains only whitespace/line-wrapping changes and semantically equivalent clippy lint fixes - **Type:** invariant - **Harness:** `git diff` inspection -- **Preconditions:** `cargo fmt` has been run but changes are not yet committed (or compare the commit diff) +- **Preconditions:** `cargo fmt` has been run and clippy warnings have been fixed, but changes are not yet committed (or compare the commit diff) - **Actions:** Run `git diff -- src/ tests/` and inspect the output -- **Expected outcome:** All hunks contain only whitespace, indentation, and line-break changes. No identifier, keyword, operator, or string literal changes. The affected files are exactly: `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`. Source of truth: the `cargo fmt -- --check` output captured before the fix, which showed 17 diffs across these 5 files — all line-wrapping reformats. +- **Expected outcome:** In `src/` files, all hunks contain only whitespace, indentation, and line-break changes. In `tests/e2e.rs`, hunks also include collapsing nested `if`/`if let` into `if ... && let ...` chains (a semantically equivalent AST-level change to satisfy `clippy::collapsible_if`). The affected files are exactly: `src/commands/init.rs`, `src/commands/ls.rs`, `src/config.rs`, `src/platform.rs`, `tests/e2e.rs`. - **Interactions:** None. ### 5. Correct files are modified @@ -66,7 +66,7 @@ None. All tests use the local Rust toolchain (`cargo fmt`, `cargo clippy`, `carg **Covered:** - All three CI `check` job steps (`fmt --check`, `clippy`, `test`) are verified with the exact same commands and flags used in CI. -- Diff correctness: only formatting changes, only expected files. +- Diff correctness: only formatting and semantically equivalent clippy-lint changes, only expected files. **Excluded (per strategy):** - The `install-script` CI job (`tests/test_install.sh`) is not affected by Rust source formatting and is excluded. Risk: none — that job is independent and was not failing. diff --git a/docs/plans/2026-03-10-fix-cargo-fmt.md b/docs/plans/2026-03-10-fix-cargo-fmt.md index 4fb929f..f4cf1e6 100644 --- a/docs/plans/2026-03-10-fix-cargo-fmt.md +++ b/docs/plans/2026-03-10-fix-cargo-fmt.md @@ -4,13 +4,15 @@ **Goal:** Fix all `cargo fmt` formatting violations so the GitHub Actions CI pipeline passes. -**Architecture:** Run `cargo fmt` to auto-format all Rust source files, then verify the full CI check sequence (`fmt --check`, `clippy`, `test`) passes. No logic changes — formatting only. +**Architecture:** Run `cargo fmt` to auto-format all Rust source files, then fix any pre-existing `clippy` lint warnings that were previously masked by the format failure, and verify the full CI check sequence (`fmt --check`, `clippy`, `test`) passes. **Tech Stack:** Rust toolchain (cargo fmt, clippy, cargo test) **Decision: Use `cargo fmt` auto-format rather than manual edits.** All 17 diffs across 5 files are standard rustfmt line-length and argument-wrapping reformats. Running `cargo fmt` is idiomatic, deterministic, and guaranteed to produce the exact output `cargo fmt -- --check` expects. Manual editing would be error-prone and pointless. +**Note (discovered during implementation):** After `cargo fmt` fixes the format check, `cargo clippy` reveals 4 pre-existing `clippy::collapsible_if` warnings in `tests/e2e.rs` that were previously masked because CI failed at the earlier `fmt --check` step. These require collapsing nested `if`/`if let` chains into single `if ... && let ...` expressions — a semantically equivalent but AST-level change, not just whitespace. + --- ### Task 1: Run cargo fmt to auto-format all files @@ -44,7 +46,7 @@ Expected: exits 0 with no output (no remaining diffs). cargo clippy --all-targets -- -D warnings ``` -Expected: exits 0. Formatting changes cannot introduce clippy warnings, but this mirrors the CI step that was previously blocked by the format failure. +Expected: exits 0. If pre-existing clippy warnings surface (previously masked by the format failure), fix them before proceeding. **Step 4: Run cargo test** @@ -58,5 +60,5 @@ Expected: exits 0. Formatting changes cannot break tests, but this mirrors the C ```bash git add src/commands/init.rs src/commands/ls.rs src/config.rs src/platform.rs tests/e2e.rs -git commit -m "style: apply cargo fmt to fix CI formatting check" +git commit -m "style: apply cargo fmt and fix clippy collapsible-if warnings" ```