From 82661b52ea2e81d04a3e85b7f18425c0ba193659 Mon Sep 17 00:00:00 2001 From: Josh Rotenberg Date: Thu, 15 Jan 2026 16:52:29 -0800 Subject: [PATCH 1/3] fix: compose commands producing "docker docker compose" (#233) The execute_command helper was passing "docker" as the command name for compose commands, but CommandExecutor also prepends the command name to args before running. This resulted in "docker docker compose ..." instead of "docker compose ...". Fix: Pass "compose" as the command name and skip the redundant "compose" prefix from args. Changes: - Fix execute_command to pass "compose" as command name for compose commands - Add regression tests in ps.rs and command.rs - Expand CI antipattern check to include src/command.rs Fixes #233 Co-Authored-By: Claude Opus 4.5 --- .github/workflows/ci.yml | 7 ++++--- src/command.rs | 40 +++++++++++++++++++++++++++++++++++++-- src/command/compose/ps.rs | 23 ++++++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6d8e2a5..166dce2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -50,10 +50,11 @@ jobs: if: matrix.rust == 'stable' && matrix.os == 'ubuntu-latest' run: | # Commands should use self.execute_command(args) not self.executor.execute_command("docker", args) - # The latter causes "docker docker " double-command bug - if grep -r 'executor\.execute_command("docker"' src/command/; then - echo "ERROR: Found executor.execute_command(\"docker\", ...) antipattern!" + # The latter causes "docker docker " double-command bug (issue #233) + if grep -rE 'executor\.execute_command\("docker"|execute_command\("docker",' src/command.rs src/command/; then + echo "ERROR: Found execute_command(\"docker\", ...) antipattern!" echo "Use self.execute_command(args) instead to avoid 'docker docker' bug." + echo "For compose commands, pass 'compose' as command name, not 'docker'." exit 1 fi diff --git a/src/command.rs b/src/command.rs index a2369e0..876fbb2 100644 --- a/src/command.rs +++ b/src/command.rs @@ -138,8 +138,10 @@ pub trait DockerCommand { // For compose commands, we need to handle "docker compose " // For regular commands, we handle "docker " if command_args.first() == Some(&"compose".to_string()) { - // This is a compose command - args are already formatted correctly - executor.execute_command("docker", command_args).await + // This is a compose command - pass "compose" as command name + // and remaining args (skip the "compose" prefix since it becomes the command name) + let remaining_args = command_args.into_iter().skip(1).collect(); + executor.execute_command("compose", remaining_args).await } else { // Regular docker command - first arg is the command name let command_name = command_args @@ -1076,4 +1078,38 @@ mod tests { assert!(empty_output.stdout_is_empty()); assert!(empty_output.stderr_is_empty()); } + + /// Regression test for issue #233: Verify that compose commands don't produce + /// "docker docker compose" when executed. The args returned by ComposeCommand + /// should start with "compose" (not "docker"), and the execute_command logic + /// should properly handle this by passing "compose" as the command name. + #[cfg(feature = "compose")] + #[test] + fn test_compose_command_args_structure() { + use crate::compose::ComposeUpCommand; + + let cmd = ComposeUpCommand::new() + .file("docker-compose.yml") + .detach() + .service("web"); + + let args = ComposeCommand::build_command_args(&cmd); + + // First arg must be "compose" - this becomes the command name + assert_eq!(args[0], "compose", "compose args must start with 'compose'"); + + // "docker" should never appear in these args - the runtime binary + // is added separately by CommandExecutor + assert!( + !args.iter().any(|arg| arg == "docker"), + "compose args should not contain 'docker': {:?}", + args + ); + + // Verify expected structure: compose [global opts] up [subcommand opts] [services] + assert!(args.contains(&"up".to_string()), "must contain subcommand"); + assert!(args.contains(&"--file".to_string()), "must contain --file"); + assert!(args.contains(&"--detach".to_string()), "must contain --detach"); + assert!(args.contains(&"web".to_string()), "must contain service name"); + } } diff --git a/src/command/compose/ps.rs b/src/command/compose/ps.rs index e55621a..6ffa1a1 100644 --- a/src/command/compose/ps.rs +++ b/src/command/compose/ps.rs @@ -380,4 +380,27 @@ mod tests { assert!(args.contains(&"my-project".to_string())); assert!(args.contains(&"--all".to_string())); } + + /// Regression test for issue #233: ComposePsCommand was failing because + /// the command was being built as "docker docker compose ..." instead of + /// "docker compose ...". This verifies that build_command_args does not + /// include "docker" since the runtime binary is added separately. + #[test] + fn test_compose_args_no_docker_prefix() { + let cmd = ComposePsCommand::new() + .file("/path/to/docker-compose.yaml") + .service("php"); + + let args = ComposeCommand::build_command_args(&cmd); + + // Args should start with "compose", not "docker" + assert_eq!(args[0], "compose"); + // "docker" should not appear anywhere in the args (the runtime binary + // is added separately by CommandExecutor) + assert!( + !args.iter().any(|arg| arg == "docker"), + "args should not contain 'docker': {:?}", + args + ); + } } From 0a4cb38345ffb5cac1ec46a0794b8001c9d33209 Mon Sep 17 00:00:00 2001 From: Josh Rotenberg Date: Thu, 15 Jan 2026 16:55:47 -0800 Subject: [PATCH 2/3] style: cargo fmt --- src/command.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/command.rs b/src/command.rs index 876fbb2..0aa11aa 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1109,7 +1109,13 @@ mod tests { // Verify expected structure: compose [global opts] up [subcommand opts] [services] assert!(args.contains(&"up".to_string()), "must contain subcommand"); assert!(args.contains(&"--file".to_string()), "must contain --file"); - assert!(args.contains(&"--detach".to_string()), "must contain --detach"); - assert!(args.contains(&"web".to_string()), "must contain service name"); + assert!( + args.contains(&"--detach".to_string()), + "must contain --detach" + ); + assert!( + args.contains(&"web".to_string()), + "must contain service name" + ); } } From 0aee9044630c41dcb84d0f220fba0831dedf32a7 Mon Sep 17 00:00:00 2001 From: Josh Rotenberg Date: Thu, 15 Jan 2026 17:01:39 -0800 Subject: [PATCH 3/3] style: fix clippy warnings in doc comments and format args --- src/command.rs | 7 +++---- src/command/compose/ps.rs | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/command.rs b/src/command.rs index 0aa11aa..de8de3f 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1080,8 +1080,8 @@ mod tests { } /// Regression test for issue #233: Verify that compose commands don't produce - /// "docker docker compose" when executed. The args returned by ComposeCommand - /// should start with "compose" (not "docker"), and the execute_command logic + /// "docker docker compose" when executed. The args returned by `ComposeCommand` + /// should start with "compose" (not "docker"), and the `execute_command` logic /// should properly handle this by passing "compose" as the command name. #[cfg(feature = "compose")] #[test] @@ -1102,8 +1102,7 @@ mod tests { // is added separately by CommandExecutor assert!( !args.iter().any(|arg| arg == "docker"), - "compose args should not contain 'docker': {:?}", - args + "compose args should not contain 'docker': {args:?}" ); // Verify expected structure: compose [global opts] up [subcommand opts] [services] diff --git a/src/command/compose/ps.rs b/src/command/compose/ps.rs index 6ffa1a1..8ec8221 100644 --- a/src/command/compose/ps.rs +++ b/src/command/compose/ps.rs @@ -381,9 +381,9 @@ mod tests { assert!(args.contains(&"--all".to_string())); } - /// Regression test for issue #233: ComposePsCommand was failing because + /// Regression test for issue #233: `ComposePsCommand` was failing because /// the command was being built as "docker docker compose ..." instead of - /// "docker compose ...". This verifies that build_command_args does not + /// "docker compose ...". This verifies that `build_command_args` does not /// include "docker" since the runtime binary is added separately. #[test] fn test_compose_args_no_docker_prefix() { @@ -399,8 +399,7 @@ mod tests { // is added separately by CommandExecutor) assert!( !args.iter().any(|arg| arg == "docker"), - "args should not contain 'docker': {:?}", - args + "args should not contain 'docker': {args:?}" ); } }