Skip to content

add --dry-run arg #402

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
mrshmllow wants to merge 2 commits into
base: trunk
Choose a base branch
from
Draft

add --dry-run arg #402

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ec9ce8f
add --dry-run arg
mrshmllow Mar 12, 2026
46e3fe0
remove clone bound on goal fn
mrshmllow Mar 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 87 additions & 21 deletions crates/cli/src/apply.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,7 @@ fn read_apply_targets_from_stdin() -> Result<(Vec<String>, Vec<Name>)> {
}))
}

fn resolve_targets(
on: &[ApplyTarget],
modifiers: &mut SubCommandModifiers,
) -> Result<(HashSet<String>, HashSet<Name>)> {
fn resolve_targets(on: &[ApplyTarget]) -> Result<(HashSet<String>, HashSet<Name>)> {
on.iter()
.try_fold((HashSet::new(), HashSet::new()), |result, target| {
let (mut tags, mut names) = result;
Expand All @@ -67,8 +64,8 @@ fn resolve_targets(
names.insert(name.clone());
}
ApplyTarget::Stdin => {
modifiers.non_interactive = true;
let (found_tags, found_names) = read_apply_targets_from_stdin()?;

names.extend(found_names);
tags.extend(found_tags);
}
Expand Down Expand Up @@ -100,14 +97,20 @@ pub async fn apply<F>(
args: CommonVerbArgs,
partition: Partitions,
make_goal: F,
mut modifiers: SubCommandModifiers,
modifiers: SubCommandModifiers,
) -> Result<()>
where
F: Fn(&Name, &Node) -> Goal,
{
let location = Arc::new(location);

let (tags, names) = resolve_targets(&args.on, &mut modifiers)?;
// stdin implies non_interactive
let mut modifiers = modifiers;
if args.on.iter().any(|t| matches!(t, ApplyTarget::Stdin)) {
modifiers.non_interactive = true;
}

let (tags, names) = resolve_targets(&args.on)?;

let selected_names: Vec<_> = hive
.nodes
Expand All @@ -121,28 +124,93 @@ where
.map(|(name, _)| name.clone())
.collect();

let num_selected = selected_names.len();

let partitioned_names = partition_arr(selected_names, &partition);

if num_selected != partitioned_names.len() {
info!(
"Partitioning reduced selected number of nodes from {num_selected} to {}",
partitioned_names.len()
);
}

STATUS
.lock()
.add_many(&partitioned_names.iter().collect::<Vec<_>>());

if args.dry_run {
dry_activate_nodes(
hive,
&partitioned_names,
make_goal,
&location,
&should_quit,
modifiers,
);

return Ok(());
Comment on lines +133 to +143
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't exit dry-run with STATUS still populated.

STATUS.add_many(...) has already run before this branch, but the dry-run path returns before anything can drain or clear those entries. That leaves stale global status behind and can corrupt later progress output from the same process.

Suggested fix 
-    STATUS
-        .lock()
-        .add_many(&partitioned_names.iter().collect::<Vec<_>>());

     if args.dry_run {
         dry_activate_nodes(
             hive,
             &partitioned_names,
             make_goal,
             &location,
             &should_quit,
             modifiers,
         );

         return Ok(());
     }
+
+    STATUS
+        .lock()
+        .add_many(&partitioned_names.iter().collect::<Vec<_>>());
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@crates/cli/src/apply.rs` around lines 133 - 143, The dry-run branch returns
early leaving STATUS populated; before returning from the args.dry_run path in
apply (where dry_activate_nodes(...) is called), drain or clear the global
STATUS entries that were added (the ones added by STATUS.add_many earlier) so
they don't persist and corrupt later output. Locate the dry-run branch around
the args.dry_run check and add a call to the same STATUS cleanup/drain routine
used after real runs (or explicitly call STATUS.clear()/drain method)
immediately before the return to ensure global status is reset.

}

apply_nodes(
hive,
&partitioned_names,
make_goal,
location,
should_quit,
args.parallel,
modifiers,
)
.await
}

fn dry_activate_nodes<F>(
hive: &Hive,
names: &[Name],
make_goal: F,
location: &Arc<HiveLocation>,
should_quit: &Arc<AtomicBool>,
modifiers: SubCommandModifiers,
) where
F: Fn(&Name, &Node) -> Goal,
{
for name in names {
let node = hive.nodes.get(name).unwrap();

let goal = make_goal(name, node);
let plan = plan_for_node(
node,
name.clone(),
&goal,
location.clone(),
&modifiers,
should_quit.clone(),
);

let goal_str = match &goal {
Goal::Build => "Build".to_string(),
Goal::Apply(args) => format!("Apply {:?}", args.goal),
};

println!("Node: {name}");
println!("Goal: {goal_str}");
println!("Steps:");
for step in &plan.steps {
println!(" - {step}");
}
println!();
}
}

async fn apply_nodes<F>(
hive: &mut Hive,
names: &[Name],
make_goal: F,
location: Arc<HiveLocation>,
should_quit: Arc<AtomicBool>,
parallel: usize,
modifiers: SubCommandModifiers,
) -> Result<()>
where
F: Fn(&Name, &Node) -> Goal,
{
let mut set = hive
.nodes
.iter_mut()
.filter(|(name, _)| partitioned_names.contains(name))
.filter(|(name, _)| names.contains(name))
.map(|(name, node)| {
let goal = make_goal(name, node);

let plan = plan_for_node(
node,
name.clone(),
Expand All @@ -159,7 +227,7 @@ where
error!("There are no nodes selected for deployment");
}

let futures = futures::stream::iter(set).buffer_unordered(args.parallel);
let futures = futures::stream::iter(set).buffer_unordered(parallel);
let result = futures.collect::<Vec<_>>().await;

let (successful, errors): (Vec<_>, Vec<_>) =
Expand All @@ -179,9 +247,7 @@ where
}

if !errors.is_empty() {
// clear the status bar if we are about to print error messages
STATUS.lock().clear(&mut stderr());

return Err(NodeErrors(
errors
.into_iter()
Expand Down
4 changes: 4 additions & 0 deletions crates/cli/src/cli.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,10 @@ pub struct CommonVerbArgs {

#[arg(short, long, default_value_t = 10, value_parser=more_than_zero)]
pub parallel: usize,

/// Print the plan for each node without executing it.
#[arg(long, default_value_t = false)]
pub dry_run: bool,
}

#[allow(clippy::struct_excessive_bools)]
Expand Down
Loading