daonanchao/vibe-kanban
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix command builder whitespace handling (#2018)

Browse Source 
* Fix command builder whitespace handling

Add a seprate command for parameters that need to be split on whitespace boundaries, and default to not splitting.

The previous behaviour always split parameters, which caused copilot CLI to fail when a project had whitespace in the repository directory name; `error: too many arguments. Expected 0 arguments but got 1;.

The issue was not specific to copilot altough it only occured in copilot due to an implementation detail for copilot where it recevied a directory name which happened to contain the project name.

* make extend_shell_params private
This commit is contained in:
Solomon
2026-01-15 20:08:09 +00:00
committed by GitHub
parent 82a5dbecbc
commit daa2690500
12 changed files with 80 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
crates/executors/src/command.rs
View File

@@ -16,6 +16,8 @@ pub enum CommandBuildError {
EmptyCommand,
#[error("failed to quote command: {0}")]
QuoteError(#[from] shlex::QuoteError),
#[error("invalide shell parameters: {0}")]
InvalidShellParams(String),
}
#[derive(Debug, Clone)]
@@ -90,6 +92,27 @@ impl CommandBuilder {
self
}
fn extend_shell_params<I>(mut self, more: I) -> Result<Self, CommandBuildError>
where
I: IntoIterator,
I::Item: Into<String>,
{
let joined = more
.into_iter()
.map(|p| p.into())
.collect::<Vec<String>>()
.join(" ");
let extra: Vec<String> = split_command_line(&joined)
.map_err(|err| CommandBuildError::InvalidShellParams(format!("{joined}: {err}")))?;
match &mut self.params {
Some(p) => p.extend(extra),
None => self.params = Some(extra),
}
Ok(self)
}
pub fn extend_params<I>(mut self, more: I) -> Self
where
I: IntoIterator,
@@ -115,19 +138,20 @@ impl CommandBuilder {
}
fn build(&self, additional_args: &[String]) -> Result<CommandParts, CommandBuildError> {
let mut parts = split_command_line(&self.simple_join(additional_args))?;
let program = parts.remove(0);
Ok(CommandParts::new(program, parts))
}
fn simple_join(&self, additional_args: &[String]) -> String {
let mut parts = vec![self.base.clone()];
let mut parts = vec![];
let base_parts = split_command_line(&self.base)?;
parts.extend(base_parts);
if let Some(ref params) = self.params {
parts.extend(params.clone());
}
parts.extend(additional_args.iter().cloned());
parts.join(" ")
if parts.is_empty() {
return Err(CommandBuildError::EmptyCommand);
}
let program = parts.remove(0);
Ok(CommandParts::new(program, parts))
}
}
@@ -148,15 +172,18 @@ fn split_command_line(input: &str) -> Result<Vec<String>, CommandBuildError> {
}
}
pub fn apply_overrides(builder: CommandBuilder, overrides: &CmdOverrides) -> CommandBuilder {
pub fn apply_overrides(
builder: CommandBuilder,
overrides: &CmdOverrides,
) -> Result<CommandBuilder, CommandBuildError> {
let builder = if let Some(ref base) = overrides.base_command_override {
builder.override_base(base.clone())
} else {
builder
};
if let Some(ref extra) = overrides.additional_params {
builder.extend_params(extra.clone())
builder.extend_shell_params(extra.clone())
} else {
builder
Ok(builder)
}
}

8
crates/executors/src/executors/amp.rs
View File

@@ -9,7 +9,7 @@ use ts_rs::TS;
use workspace_utils::msg_store::MsgStore;
use crate::{
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -33,7 +33,7 @@ pub struct Amp {
}
impl Amp {
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let mut builder = CommandBuilder::new("npx -y @sourcegraph/amp@0.0.1764777697-g907e30")
.params(["--execute", "--stream-json"]);
if self.dangerously_allow_all.unwrap_or(false) {
@@ -51,7 +51,7 @@ impl StandardCodingAgentExecutor for Amp {
prompt: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_parts = self.build_command_builder().build_initial()?;
let command_parts = self.build_command_builder()?.build_initial()?;
let (executable_path, args) = command_parts.into_resolved().await?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
@@ -88,7 +88,7 @@ impl StandardCodingAgentExecutor for Amp {
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
// 1) Fork the thread synchronously to obtain new thread id
let builder = self.build_command_builder();
let builder = self.build_command_builder()?;
let fork_line = builder.build_follow_up(&[
"threads".to_string(),
"fork".to_string(),

8
crates/executors/src/executors/claude.rs
View File

@@ -24,7 +24,7 @@ use self::{
};
use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, CommandParts, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, CommandParts, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -76,7 +76,7 @@ pub struct ClaudeCode {
}
impl ClaudeCode {
async fn build_command_builder(&self) -> CommandBuilder {
async fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
// If base_command_override is provided and claude_code_router is also set, log a warning
if self.cmd.base_command_override.is_some() && self.claude_code_router.is_some() {
tracing::warn!(
@@ -169,7 +169,7 @@ impl StandardCodingAgentExecutor for ClaudeCode {
prompt: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_builder = self.build_command_builder().await;
let command_builder = self.build_command_builder().await?;
let command_parts = command_builder.build_initial()?;
self.spawn_internal(current_dir, prompt, command_parts, env)
.await
@@ -182,7 +182,7 @@ impl StandardCodingAgentExecutor for ClaudeCode {
session_id: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_builder = self.build_command_builder().await;
let command_builder = self.build_command_builder().await?;
let command_parts = command_builder.build_follow_up(&[
"--fork-session".to_string(),
"--resume".to_string(),

10
crates/executors/src/executors/codex.rs
View File

@@ -46,7 +46,7 @@ use self::{
};
use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, CommandParts, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, CommandParts, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, ExecutorExitResult, SpawnedChild,
@@ -174,7 +174,7 @@ impl StandardCodingAgentExecutor for Codex {
prompt: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_parts = self.build_command_builder().build_initial()?;
let command_parts = self.build_command_builder()?.build_initial()?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let action = CodexSessionAction::Chat {
prompt: combined_prompt,
@@ -190,7 +190,7 @@ impl StandardCodingAgentExecutor for Codex {
session_id: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_parts = self.build_command_builder().build_follow_up(&[])?;
let command_parts = self.build_command_builder()?.build_follow_up(&[])?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let action = CodexSessionAction::Chat {
prompt: combined_prompt,
@@ -242,7 +242,7 @@ impl StandardCodingAgentExecutor for Codex {
session_id: Option<&str>,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let command_parts = self.build_command_builder().build_initial()?;
let command_parts = self.build_command_builder()?.build_initial()?;
let review_target = ReviewTarget::Custom {
instructions: prompt.to_string(),
};
@@ -259,7 +259,7 @@ impl Codex {
"npx -y @openai/codex@0.77.0"
}
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let mut builder = CommandBuilder::new(Self::base_command());
builder = builder.extend_params(["app-server"]);
if self.oss.unwrap_or(false) {

8
crates/executors/src/executors/copilot.rs
View File

@@ -22,7 +22,7 @@ use uuid::Uuid;
use workspace_utils::{msg_store::MsgStore, path::get_vibe_kanban_temp_dir};
use crate::{
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -55,7 +55,7 @@ pub struct Copilot {
}
impl Copilot {
fn build_command_builder(&self, log_dir: &str) -> CommandBuilder {
fn build_command_builder(&self, log_dir: &str) -> Result<CommandBuilder, CommandBuildError> {
let mut builder = CommandBuilder::new("npx -y @github/copilot@0.0.375").params([
"--no-color",
"--log-level",
@@ -106,7 +106,7 @@ impl StandardCodingAgentExecutor for Copilot {
) -> Result<SpawnedChild, ExecutorError> {
let log_dir = Self::create_temp_log_dir(current_dir).await?;
let command_parts = self
.build_command_builder(&log_dir.to_string_lossy())
.build_command_builder(&log_dir.to_string_lossy())?
.build_initial()?;
let (program_path, args) = command_parts.into_resolved().await?;
@@ -149,7 +149,7 @@ impl StandardCodingAgentExecutor for Copilot {
) -> Result<SpawnedChild, ExecutorError> {
let log_dir = Self::create_temp_log_dir(current_dir).await?;
let command_parts = self
.build_command_builder(&log_dir.to_string_lossy())
.build_command_builder(&log_dir.to_string_lossy())?
.build_follow_up(&["--resume".to_string(), session_id.to_string()])?;
let (program_path, args) = command_parts.into_resolved().await?;

8
crates/executors/src/executors/cursor.rs
View File

@@ -16,7 +16,7 @@ use workspace_utils::{
};
use crate::{
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -53,7 +53,7 @@ impl CursorAgent {
"cursor-agent"
}
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let mut builder =
CommandBuilder::new(Self::base_command()).params(["-p", "--output-format=stream-json"]);
@@ -79,7 +79,7 @@ impl StandardCodingAgentExecutor for CursorAgent {
) -> Result<SpawnedChild, ExecutorError> {
mcp::ensure_mcp_server_trust(self, current_dir).await;
let command_parts = self.build_command_builder().build_initial()?;
let command_parts = self.build_command_builder()?.build_initial()?;
let (executable_path, args) = command_parts.into_resolved().await?;
@@ -118,7 +118,7 @@ impl StandardCodingAgentExecutor for CursorAgent {
mcp::ensure_mcp_server_trust(self, current_dir).await;
let command_parts = self
.build_command_builder()
.build_command_builder()?
.build_follow_up(&["--resume".to_string(), session_id.to_string()])?;
let (executable_path, args) = command_parts.into_resolved().await?;

8
crates/executors/src/executors/droid.rs
View File

@@ -10,7 +10,7 @@ use ts_rs::TS;
use workspace_utils::msg_store::MsgStore;
use crate::{
command::CommandParts,
command::{CommandBuildError, CommandBuilder, CommandParts},
env::ExecutionEnv,
executors::{AppendPrompt, ExecutorError, SpawnedChild, StandardCodingAgentExecutor},
logs::utils::EntryIndexProvider,
@@ -83,7 +83,7 @@ pub struct Droid {
}
impl Droid {
pub fn build_command_builder(&self) -> crate::command::CommandBuilder {
pub fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
use crate::command::{CommandBuilder, apply_overrides};
let mut builder =
CommandBuilder::new("droid exec").params(["--output-format", "stream-json"]);
@@ -145,7 +145,7 @@ impl StandardCodingAgentExecutor for Droid {
prompt: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let droid_command = self.build_command_builder().build_initial()?;
let droid_command = self.build_command_builder()?.build_initial()?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
spawn_droid(droid_command, &combined_prompt, current_dir, env, &self.cmd).await
@@ -164,7 +164,7 @@ impl StandardCodingAgentExecutor for Droid {
))
})?;
let continue_cmd = self
.build_command_builder()
.build_command_builder()?
.build_follow_up(&["--session-id".to_string(), forked_session_id.clone()])?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);

8
crates/executors/src/executors/gemini.rs
View File

@@ -10,7 +10,7 @@ use workspace_utils::msg_store::MsgStore;
pub use super::acp::AcpAgentHarness;
use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -35,7 +35,7 @@ pub struct Gemini {
}
impl Gemini {
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let mut builder = CommandBuilder::new("npx -y @google/gemini-cli@0.23.0");
if let Some(model) = &self.model {
@@ -67,7 +67,7 @@ impl StandardCodingAgentExecutor for Gemini {
) -> Result<SpawnedChild, ExecutorError> {
let harness = AcpAgentHarness::new();
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let gemini_command = self.build_command_builder().build_initial()?;
let gemini_command = self.build_command_builder()?.build_initial()?;
let approvals = if self.yolo.unwrap_or(false) {
None
} else {
@@ -94,7 +94,7 @@ impl StandardCodingAgentExecutor for Gemini {
) -> Result<SpawnedChild, ExecutorError> {
let harness = AcpAgentHarness::new();
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let gemini_command = self.build_command_builder().build_follow_up(&[])?;
let gemini_command = self.build_command_builder()?.build_follow_up(&[])?;
let approvals = if self.yolo.unwrap_or(false) {
None
} else {

6
crates/executors/src/executors/opencode.rs
View File

@@ -11,7 +11,7 @@ use workspace_utils::msg_store::MsgStore;
use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, ExecutorExitResult, SpawnedChild,
@@ -47,7 +47,7 @@ pub struct Opencode {
}
impl Opencode {
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let builder = CommandBuilder::new("npx -y opencode-ai@1.1.3")
// Pass hostname/port as separate args so OpenCode treats them as explicitly set
// (it checks `process.argv.includes(\"--port\")` / `\"--hostname\"`).
@@ -64,7 +64,7 @@ impl Opencode {
) -> Result<SpawnedChild, ExecutorError> {
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let command_parts = self.build_command_builder().build_initial()?;
let command_parts = self.build_command_builder()?.build_initial()?;
let (program_path, args) = command_parts.into_resolved().await?;
let mut command = Command::new(program_path);

8
crates/executors/src/executors/qwen.rs
View File

@@ -9,7 +9,7 @@ use workspace_utils::msg_store::MsgStore;
use crate::{
approvals::ExecutorApprovalService,
command::{CmdOverrides, CommandBuilder, apply_overrides},
command::{CmdOverrides, CommandBuildError, CommandBuilder, apply_overrides},
env::ExecutionEnv,
executors::{
AppendPrompt, AvailabilityInfo, ExecutorError, SpawnedChild, StandardCodingAgentExecutor,
@@ -33,7 +33,7 @@ pub struct QwenCode {
}
impl QwenCode {
fn build_command_builder(&self) -> CommandBuilder {
fn build_command_builder(&self) -> Result<CommandBuilder, CommandBuildError> {
let mut builder = CommandBuilder::new("npx -y @qwen-code/qwen-code@0.2.1");
if self.yolo.unwrap_or(false) {
@@ -56,7 +56,7 @@ impl StandardCodingAgentExecutor for QwenCode {
prompt: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let qwen_command = self.build_command_builder().build_initial()?;
let qwen_command = self.build_command_builder()?.build_initial()?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let harness = AcpAgentHarness::with_session_namespace("qwen_sessions");
let approvals = if self.yolo.unwrap_or(false) {
@@ -83,7 +83,7 @@ impl StandardCodingAgentExecutor for QwenCode {
session_id: &str,
env: &ExecutionEnv,
) -> Result<SpawnedChild, ExecutorError> {
let qwen_command = self.build_command_builder().build_follow_up(&[])?;
let qwen_command = self.build_command_builder()?.build_follow_up(&[])?;
let combined_prompt = self.append_prompt.combine_prompt(prompt);
let harness = AcpAgentHarness::with_session_namespace("qwen_sessions");
let approvals = if self.yolo.unwrap_or(false) {

5
crates/server/src/error.rs
View File

@@ -10,7 +10,7 @@ use db::models::{
workspace::WorkspaceError,
};
use deployment::{DeploymentError, RemoteClientNotConfigured};
use executors::executors::ExecutorError;
use executors::{command::CommandBuildError, executors::ExecutorError};
use git2::Error as Git2Error;
use services::services::{
config::{ConfigError, EditorOpenError},
@@ -76,6 +76,8 @@ pub enum ApiError {
Conflict(String),
#[error("Forbidden: {0}")]
Forbidden(String),
#[error(transparent)]
CommandBuilder(#[from] CommandBuildError),
}
impl From<&'static str> for ApiError {
@@ -124,6 +126,7 @@ impl IntoResponse for ApiError {
ApiError::Deployment(_) => (StatusCode::INTERNAL_SERVER_ERROR, "DeploymentError"),
ApiError::Container(_) => (StatusCode::INTERNAL_SERVER_ERROR, "ContainerError"),
ApiError::Executor(_) => (StatusCode::INTERNAL_SERVER_ERROR, "ExecutorError"),
ApiError::CommandBuilder(_) => (StatusCode::INTERNAL_SERVER_ERROR, "CommandBuildError"),
ApiError::Database(_) => (StatusCode::INTERNAL_SERVER_ERROR, "DatabaseError"),
ApiError::Worktree(_) => (StatusCode::INTERNAL_SERVER_ERROR, "WorktreeError"),
ApiError::Config(_) => (StatusCode::INTERNAL_SERVER_ERROR, "ConfigError"),

2
crates/server/src/routes/task_attempts/codex_setup.rs
View File

@@ -78,7 +78,7 @@ pub async fn run_codex_setup(
async fn get_setup_helper_action(codex: &Codex) -> Result<ExecutorAction, ApiError> {
let mut login_command = CommandBuilder::new(Codex::base_command());
login_command = login_command.extend_params(["login"]);
login_command = apply_overrides(login_command, &codex.cmd);
login_command = apply_overrides(login_command, &codex.cmd)?;
let (program_path, args) = login_command
.build_initial()