From a43174d9013ccd931c60478baec94dba519ba37b Mon Sep 17 00:00:00 2001 From: DJ Date: Mon, 30 Mar 2026 19:48:13 -0700 Subject: [PATCH 1/7] ci: add test gate to release workflow Add a test job (fmt, clippy, cargo test) that must pass before the build matrix runs, ensuring tagged releases are never published from code that fails basic quality checks. Refs #11 Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/release.yml | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0934fa4..4b22ef2 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -42,9 +42,34 @@ jobs: exit 1 fi + test: + name: Test + needs: verify_tag_on_main + runs-on: ubuntu-24.04 + steps: + - name: Check out repository + uses: actions/checkout@v5 + + - name: Install Rust toolchain + uses: dtolnay/rust-toolchain@stable + with: + components: rustfmt, clippy + + - name: Restore Cargo cache + uses: Swatinem/rust-cache@v2 + + - name: Check formatting + run: cargo fmt --all --check + + - name: Run cargo clippy + run: cargo clippy --locked --all-targets -- -D warnings + + - name: Run cargo test + run: cargo test --locked + build: name: Build ${{ matrix.target }} - needs: verify_tag_on_main + needs: [verify_tag_on_main, test] runs-on: ${{ matrix.runs_on }} strategy: fail-fast: false From d297f407536fcecf4b808778ca60fbad3fd5ea38 Mon Sep 17 00:00:00 2001 From: mark-pro <20671988+mark-pro@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:14:58 -0400 Subject: [PATCH 2/7] ci: enforce clippy checks in workflows and fix existing warnings --- .github/workflows/ci.yml | 3 +++ .github/workflows/release.yml | 2 +- src/cli/branch/mod.rs | 5 ++--- src/cli/operation/mod.rs | 2 +- src/cli/sync/mod.rs | 12 ++++++------ src/core/clean/apply.rs | 1 + src/core/clean/plan.rs | 2 +- src/core/commit.rs | 5 ++--- src/core/git.rs | 5 ++--- src/core/merge/apply.rs | 4 +--- src/core/reparent/plan.rs | 5 ++--- src/core/store/types.rs | 1 + src/core/sync.rs | 23 ++++++++++++----------- 13 files changed, 35 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fc8c481..c09653f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,6 +32,9 @@ jobs: - name: Check formatting run: cargo fmt --all --check + - name: Run cargo clippy + run: cargo clippy --locked --all-targets -- -D warnings + - name: Run cargo check run: cargo check --locked --all-targets diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 4b22ef2..5c64ddd 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -65,7 +65,7 @@ jobs: run: cargo clippy --locked --all-targets -- -D warnings - name: Run cargo test - run: cargo test --locked + run: cargo test --locked --release build: name: Build ${{ matrix.target }} diff --git a/src/cli/branch/mod.rs b/src/cli/branch/mod.rs index ae52d39..bfa80de 100644 --- a/src/cli/branch/mod.rs +++ b/src/cli/branch/mod.rs @@ -19,13 +19,12 @@ pub struct BranchArgs { pub fn execute(args: BranchArgs) -> io::Result { let outcome = branch::run(&args.clone().into())?; - if outcome.status.success() { - if let Some(node) = &outcome.created_node { + if outcome.status.success() + && let Some(node) = &outcome.created_node { println!("Created and switched to '{}'.", node.branch_name); println!(); println!("{}", super::tree::render_branch_lineage(&outcome.lineage)); } - } Ok(CommandOutcome { status: outcome.status, diff --git a/src/cli/operation/mod.rs b/src/cli/operation/mod.rs index b57855f..2d6c714 100644 --- a/src/cli/operation/mod.rs +++ b/src/cli/operation/mod.rs @@ -45,7 +45,7 @@ impl AnimationTerminal { pub fn finish(&mut self, frame: &str) -> io::Result<()> { self.render(frame)?; - write!(self.stdout, "{ANSI_SHOW_CURSOR}\n")?; + writeln!(self.stdout, "{ANSI_SHOW_CURSOR}")?; self.stdout.flush()?; self.active = false; Ok(()) diff --git a/src/cli/sync/mod.rs b/src/cli/sync/mod.rs index 56da08a..ab19ba5 100644 --- a/src/cli/sync/mod.rs +++ b/src/cli/sync/mod.rs @@ -481,9 +481,9 @@ fn format_remote_push_plan(plan: &sync::RemotePushPlan) -> String { for action in &plan.actions { let action_label = match action.kind { - RemotePushActionKind::CreateRemoteBranch => "create", - RemotePushActionKind::UpdateRemoteBranch => "push", - RemotePushActionKind::ForceUpdateRemoteBranch => "force-push", + RemotePushActionKind::Create => "create", + RemotePushActionKind::Update => "push", + RemotePushActionKind::ForceUpdate => "force-push", }; lines.push(format!( "- {action_label} {} on {}", @@ -528,9 +528,9 @@ fn format_partial_remote_push_output(header: &str, outcome: &RemotePushOutcome) let mut lines = vec![header.to_string()]; for action in &outcome.pushed_actions { let action_label = match action.kind { - RemotePushActionKind::CreateRemoteBranch => "created", - RemotePushActionKind::UpdateRemoteBranch => "pushed", - RemotePushActionKind::ForceUpdateRemoteBranch => "force-pushed", + RemotePushActionKind::Create => "created", + RemotePushActionKind::Update => "pushed", + RemotePushActionKind::ForceUpdate => "force-pushed", }; lines.push(format!( "- {action_label} {} on {}", diff --git a/src/core/clean/apply.rs b/src/core/clean/apply.rs index 2fa1841..03ddfc3 100644 --- a/src/core/clean/apply.rs +++ b/src/core/clean/apply.rs @@ -293,6 +293,7 @@ where }) } +#[allow(clippy::too_many_arguments)] fn process_clean_candidate( session: &mut crate::core::store::StoreSession, original_branch: &str, diff --git a/src/core/clean/plan.rs b/src/core/clean/plan.rs index 73f7ea8..9e88ef3 100644 --- a/src/core/clean/plan.rs +++ b/src/core/clean/plan.rs @@ -255,7 +255,7 @@ fn apply_deleted_step_projection( let mut projected_state = state.clone(); for step in deleted_steps { - deleted_local::simulate_deleted_local_step(&mut projected_state, &step)?; + deleted_local::simulate_deleted_local_step(&mut projected_state, step)?; } Ok(projected_state) diff --git a/src/core/commit.rs b/src/core/commit.rs index 5a7a7b1..0e4ab37 100644 --- a/src/core/commit.rs +++ b/src/core/commit.rs @@ -196,12 +196,11 @@ fn parse_git_log_output(stdout: &str) -> Vec { fn parse_commit_metadata(remainder: &str) -> (bool, Vec, String) { let trimmed = remainder.trim_start(); - if let Some(decorated) = trimmed.strip_prefix('(') { - if let Some((decorations, title)) = decorated.split_once(") ") { + if let Some(decorated) = trimmed.strip_prefix('(') + && let Some((decorations, title)) = decorated.split_once(") ") { let (is_head, refs) = parse_decorations(decorations); return (is_head, refs, title.to_string()); } - } (false, Vec::new(), trimmed.to_string()) } diff --git a/src/core/git.rs b/src/core/git.rs index 5be1336..1ad0d78 100644 --- a/src/core/git.rs +++ b/src/core/git.rs @@ -208,12 +208,11 @@ where let text = String::from_utf8_lossy(&chunk[..read]); stderr_output.push_str(&text); - if let Some(progress) = parse_latest_rebase_progress(&stderr_output) { - if last_progress != Some(progress) { + if let Some(progress) = parse_latest_rebase_progress(&stderr_output) + && last_progress != Some(progress) { on_progress(progress)?; last_progress = Some(progress); } - } } let status = child.wait()?; diff --git a/src/core/merge/apply.rs b/src/core/merge/apply.rs index dc69573..8a7fda1 100644 --- a/src/core/merge/apply.rs +++ b/src/core/merge/apply.rs @@ -282,9 +282,7 @@ fn run_squash_merge( let commit_output = git::commit_with_message_file(&message_path); let remove_result = fs::remove_file(&message_path); let commit_output = commit_output?; - if let Err(err) = remove_result { - return Err(err); - } + remove_result?; Ok(commit_output) } diff --git a/src/core/reparent/plan.rs b/src/core/reparent/plan.rs index 38f62dd..bc66b5f 100644 --- a/src/core/reparent/plan.rs +++ b/src/core/reparent/plan.rs @@ -110,8 +110,8 @@ pub(crate) fn plan(options: &ReparentOptions) -> io::Result { let new_parent = branch::resolve_parent_ref(&session.state, &session.config, parent_branch_name)?; - if let ParentRef::Branch { node_id: parent_id } = new_parent { - if graph.active_descendant_ids(node.id).contains(&parent_id) { + if let ParentRef::Branch { node_id: parent_id } = new_parent + && graph.active_descendant_ids(node.id).contains(&parent_id) { return Err(io::Error::new( io::ErrorKind::InvalidInput, format!( @@ -120,7 +120,6 @@ pub(crate) fn plan(options: &ReparentOptions) -> io::Result { ), )); } - } let restack_plan = restack::previews_for_actions(&restack::plan_after_branch_reparent( &session.state, diff --git a/src/core/store/types.rs b/src/core/store/types.rs index 928964e..c50e9ec 100644 --- a/src/core/store/types.rs +++ b/src/core/store/types.rs @@ -348,6 +348,7 @@ pub enum ParentRef { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "type", rename_all = "snake_case")] +#[allow(clippy::enum_variant_names)] pub enum DaggerEvent { BranchCreated(BranchCreatedEvent), BranchAdopted(BranchAdoptedEvent), diff --git a/src/core/sync.rs b/src/core/sync.rs index 56f73fd..23ca6ad 100644 --- a/src/core/sync.rs +++ b/src/core/sync.rs @@ -93,9 +93,9 @@ pub struct SyncOutcome { #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum RemotePushActionKind { - CreateRemoteBranch, - UpdateRemoteBranch, - ForceUpdateRemoteBranch, + Create, + Update, + ForceUpdate, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -660,6 +660,7 @@ where ) } +#[allow(clippy::too_many_arguments)] fn execute_sync_restack_step( session: &mut crate::core::store::StoreSession, original_branch: &str, @@ -1179,10 +1180,10 @@ pub fn execute_remote_push_plan(plan: &RemotePushPlan) -> io::Result { + RemotePushActionKind::Create | RemotePushActionKind::Update => { git::push_branch_to_remote(&action.target)? } - RemotePushActionKind::ForceUpdateRemoteBranch => { + RemotePushActionKind::ForceUpdate => { git::force_push_branch_to_remote_with_lease(&action.target)? } }; @@ -1311,7 +1312,7 @@ fn plan_remote_push_action( else { return Ok(Some(RemotePushAction { target, - kind: RemotePushActionKind::CreateRemoteBranch, + kind: RemotePushActionKind::Create, })); }; @@ -1323,7 +1324,7 @@ fn plan_remote_push_action( if allow_force_update { return Ok(Some(RemotePushAction { target, - kind: RemotePushActionKind::ForceUpdateRemoteBranch, + kind: RemotePushActionKind::ForceUpdate, })); } @@ -1332,7 +1333,7 @@ fn plan_remote_push_action( if git::merge_base(&remote_tracking_branch_ref, branch_name)? == remote_oid { return Ok(Some(RemotePushAction { target, - kind: RemotePushActionKind::UpdateRemoteBranch, + kind: RemotePushActionKind::Update, })); } @@ -1384,12 +1385,12 @@ mod tests { assert_eq!(plan.actions[0].target.branch_name, "feat/auth"); assert_eq!( plan.actions[0].kind, - RemotePushActionKind::ForceUpdateRemoteBranch + RemotePushActionKind::ForceUpdate ); assert_eq!(plan.actions[1].target.branch_name, "feat/auth-ui"); assert_eq!( plan.actions[1].kind, - RemotePushActionKind::CreateRemoteBranch + RemotePushActionKind::Create ); assert!( plan.actions @@ -1420,7 +1421,7 @@ mod tests { assert_eq!(plan.actions[0].target.branch_name, "feat/auth"); assert_eq!( plan.actions[0].kind, - RemotePushActionKind::UpdateRemoteBranch + RemotePushActionKind::Update ); }); } From bb5296f4fe0a60cd9e5023a8b3c0892f7a4bfc5a Mon Sep 17 00:00:00 2001 From: mark-pro <20671988+mark-pro@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:16:41 -0400 Subject: [PATCH 3/7] ci: ensure clippy is explicitly installed in toolchain --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c09653f..20dfd74 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,7 +24,7 @@ jobs: - name: Install Rust toolchain uses: dtolnay/rust-toolchain@stable with: - components: rustfmt + components: rustfmt, clippy - name: Restore Cargo cache uses: Swatinem/rust-cache@v2 From 137110d6a476109beb38fadcab5935900335fd0e Mon Sep 17 00:00:00 2001 From: mark-pro <20671988+mark-pro@users.noreply.github.com> Date: Tue, 31 Mar 2026 21:19:18 -0400 Subject: [PATCH 4/7] style: run cargo fmt to fix clippy auto-fix formatting --- src/cli/branch/mod.rs | 11 ++++++----- src/core/commit.rs | 9 +++++---- src/core/git.rs | 9 +++++---- src/core/reparent/plan.rs | 19 ++++++++++--------- src/core/sync.rs | 15 +++------------ 5 files changed, 29 insertions(+), 34 deletions(-) diff --git a/src/cli/branch/mod.rs b/src/cli/branch/mod.rs index bfa80de..08087e8 100644 --- a/src/cli/branch/mod.rs +++ b/src/cli/branch/mod.rs @@ -20,11 +20,12 @@ pub fn execute(args: BranchArgs) -> io::Result { let outcome = branch::run(&args.clone().into())?; if outcome.status.success() - && let Some(node) = &outcome.created_node { - println!("Created and switched to '{}'.", node.branch_name); - println!(); - println!("{}", super::tree::render_branch_lineage(&outcome.lineage)); - } + && let Some(node) = &outcome.created_node + { + println!("Created and switched to '{}'.", node.branch_name); + println!(); + println!("{}", super::tree::render_branch_lineage(&outcome.lineage)); + } Ok(CommandOutcome { status: outcome.status, diff --git a/src/core/commit.rs b/src/core/commit.rs index 0e4ab37..962e3c0 100644 --- a/src/core/commit.rs +++ b/src/core/commit.rs @@ -197,10 +197,11 @@ fn parse_commit_metadata(remainder: &str) -> (bool, Vec, String) { let trimmed = remainder.trim_start(); if let Some(decorated) = trimmed.strip_prefix('(') - && let Some((decorations, title)) = decorated.split_once(") ") { - let (is_head, refs) = parse_decorations(decorations); - return (is_head, refs, title.to_string()); - } + && let Some((decorations, title)) = decorated.split_once(") ") + { + let (is_head, refs) = parse_decorations(decorations); + return (is_head, refs, title.to_string()); + } (false, Vec::new(), trimmed.to_string()) } diff --git a/src/core/git.rs b/src/core/git.rs index 1ad0d78..680ab1a 100644 --- a/src/core/git.rs +++ b/src/core/git.rs @@ -209,10 +209,11 @@ where stderr_output.push_str(&text); if let Some(progress) = parse_latest_rebase_progress(&stderr_output) - && last_progress != Some(progress) { - on_progress(progress)?; - last_progress = Some(progress); - } + && last_progress != Some(progress) + { + on_progress(progress)?; + last_progress = Some(progress); + } } let status = child.wait()?; diff --git a/src/core/reparent/plan.rs b/src/core/reparent/plan.rs index bc66b5f..c2ac26e 100644 --- a/src/core/reparent/plan.rs +++ b/src/core/reparent/plan.rs @@ -111,15 +111,16 @@ pub(crate) fn plan(options: &ReparentOptions) -> io::Result { branch::resolve_parent_ref(&session.state, &session.config, parent_branch_name)?; if let ParentRef::Branch { node_id: parent_id } = new_parent - && graph.active_descendant_ids(node.id).contains(&parent_id) { - return Err(io::Error::new( - io::ErrorKind::InvalidInput, - format!( - "cannot reparent '{}' onto descendant '{}'", - branch_name, parent_branch_name - ), - )); - } + && graph.active_descendant_ids(node.id).contains(&parent_id) + { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!( + "cannot reparent '{}' onto descendant '{}'", + branch_name, parent_branch_name + ), + )); + } let restack_plan = restack::previews_for_actions(&restack::plan_after_branch_reparent( &session.state, diff --git a/src/core/sync.rs b/src/core/sync.rs index 23ca6ad..7d22f1a 100644 --- a/src/core/sync.rs +++ b/src/core/sync.rs @@ -1383,15 +1383,9 @@ mod tests { assert_eq!(plan.actions.len(), 2); assert_eq!(plan.actions[0].target.branch_name, "feat/auth"); - assert_eq!( - plan.actions[0].kind, - RemotePushActionKind::ForceUpdate - ); + assert_eq!(plan.actions[0].kind, RemotePushActionKind::ForceUpdate); assert_eq!(plan.actions[1].target.branch_name, "feat/auth-ui"); - assert_eq!( - plan.actions[1].kind, - RemotePushActionKind::Create - ); + assert_eq!(plan.actions[1].kind, RemotePushActionKind::Create); assert!( plan.actions .iter() @@ -1419,10 +1413,7 @@ mod tests { assert_eq!(plan.actions.len(), 1); assert_eq!(plan.actions[0].target.branch_name, "feat/auth"); - assert_eq!( - plan.actions[0].kind, - RemotePushActionKind::Update - ); + assert_eq!(plan.actions[0].kind, RemotePushActionKind::Update); }); } From 5fa5d8ff07c580f267469f19111c86ed30a94c91 Mon Sep 17 00:00:00 2001 From: DJ Date: Tue, 31 Mar 2026 19:49:11 -0700 Subject: [PATCH 5/7] fix: resolve clippy collapsible_if lints Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli/tree/mod.rs | 8 ++++---- src/cli/tree/render.rs | 26 +++++++++++++------------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/cli/tree/mod.rs b/src/cli/tree/mod.rs index e8945d9..2e95956 100644 --- a/src/cli/tree/mod.rs +++ b/src/cli/tree/mod.rs @@ -43,10 +43,10 @@ pub(super) fn render_focused_context_tree( ) -> io::Result { let mut view = tree::focused_context_view(branch_name)?; - if let Some((current_branch_name, suffix)) = suffix_for_current_branch { - if view.current_branch_name.as_deref() == Some(current_branch_name) { - view.current_branch_suffix = Some(suffix.to_string()); - } + if let Some((current_branch_name, suffix)) = suffix_for_current_branch + && view.current_branch_name.as_deref() == Some(current_branch_name) + { + view.current_branch_suffix = Some(suffix.to_string()); } Ok(render::render_stack_tree(&view)) diff --git a/src/cli/tree/render.rs b/src/cli/tree/render.rs index b04d2fc..7c42f3f 100644 --- a/src/cli/tree/render.rs +++ b/src/cli/tree/render.rs @@ -26,20 +26,20 @@ pub fn render_stack_tree(view: &TreeView) -> String { .collect::>() .join("\n"); - if !view.is_current_visible { - if let Some(current_branch) = &view.current_branch_name { - let label = match &view.current_branch_suffix { - Some(suffix) => format!("{current_branch} {suffix}"), - None => current_branch.clone(), - }; + if !view.is_current_visible + && let Some(current_branch) = &view.current_branch_name + { + let label = match &view.current_branch_suffix { + Some(suffix) => format!("{current_branch} {suffix}"), + None => current_branch.clone(), + }; - rendered.push_str("\n\n"); - rendered.push_str(&format!( - "{} {}", - Accent::BranchRef.paint_ansi(markers::CURRENT_BRANCH), - Accent::BranchRef.paint_ansi(&label) - )); - } + rendered.push_str("\n\n"); + rendered.push_str(&format!( + "{} {}", + Accent::BranchRef.paint_ansi(markers::CURRENT_BRANCH), + Accent::BranchRef.paint_ansi(&label) + )); } rendered From cebeec05c620a1b4fcf87a801793a09c34df6430 Mon Sep 17 00:00:00 2001 From: DJ Date: Thu, 2 Apr 2026 04:55:23 -0700 Subject: [PATCH 6/7] fix: use correct RemotePushActionKind enum variants in tests After merging upstream/main, the RemotePushActionKind enum was renamed from CreateRemoteBranch to Create. Update test code to use the correct variant names (Create, Update, ForceUpdate). Co-Authored-By: Claude Opus 4.6 (1M context) --- src/core/sync.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/sync.rs b/src/core/sync.rs index aab4515..a1755eb 100644 --- a/src/core/sync.rs +++ b/src/core/sync.rs @@ -1900,14 +1900,14 @@ mod tests { remote_name: "origin".into(), branch_name: "feat/auth".into(), }, - kind: RemotePushActionKind::CreateRemoteBranch, + kind: RemotePushActionKind::Create, }, super::RemotePushAction { target: BranchPushTarget { remote_name: "origin".into(), branch_name: "feat/auth-ui".into(), }, - kind: RemotePushActionKind::CreateRemoteBranch, + kind: RemotePushActionKind::Create, }, ], }; @@ -1927,12 +1927,12 @@ mod tests { SyncStatus::PushingRemoteBranch { branch_name: "feat/auth".into(), remote_name: "origin".into(), - kind: RemotePushActionKind::CreateRemoteBranch, + kind: RemotePushActionKind::Create, }, SyncStatus::PushingRemoteBranch { branch_name: "feat/auth-ui".into(), remote_name: "origin".into(), - kind: RemotePushActionKind::CreateRemoteBranch, + kind: RemotePushActionKind::Create, }, ] ); From 418894a3bc23e23c3ce4cbb06d1040f3c4560a7c Mon Sep 17 00:00:00 2001 From: DJ Date: Thu, 2 Apr 2026 11:02:29 -0700 Subject: [PATCH 7/7] fix: use correct RemotePushActionKind enum variants in render.rs The enum was renamed from CreateRemoteBranch/UpdateRemoteBranch/ForceUpdateRemoteBranch to Create/Update/ForceUpdate but render.rs still used the old names. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/cli/sync/render.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cli/sync/render.rs b/src/cli/sync/render.rs index 777e907..9c9199e 100644 --- a/src/cli/sync/render.rs +++ b/src/cli/sync/render.rs @@ -358,9 +358,9 @@ fn format_status_text(status: &SyncStatus) -> String { } => format!( "{} {branch_name} on {remote_name}", match kind { - RemotePushActionKind::CreateRemoteBranch => "Creating remote branch", - RemotePushActionKind::UpdateRemoteBranch => "Pushing", - RemotePushActionKind::ForceUpdateRemoteBranch => "Force-pushing", + RemotePushActionKind::Create => "Creating remote branch", + RemotePushActionKind::Update => "Pushing", + RemotePushActionKind::ForceUpdate => "Force-pushing", } ), SyncStatus::DeletingBranch { branch_name } => format!("Deleting {branch_name}"),