Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
4 changes: 4 additions & 0 deletions crates/config/src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ pub struct ExternalAgentConfig {
#[serde(default)]
pub args: Vec<String>,
#[serde(default)]
pub models: Vec<String>,
#[serde(default)]
pub efforts: Vec<String>,
#[serde(default)]
pub env: HashMap<String, String>,
pub working_dir: Option<String>,
pub timeout_secs: Option<u64>,
Expand Down
17 changes: 17 additions & 0 deletions crates/config/src/schema/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,23 @@ enabled = true
assert_eq!(entry.wire_api, WireApi::ChatCompletions);
}

#[test]
fn external_agent_models_from_toml() {
let toml_str = r#"
[external_agents]
enabled = true

[external_agents.agents.claude-code]
binary = "claude"
models = ["claude-opus-4-8", "claude-sonnet-4-6"]
efforts = ["high", "xhigh"]
"#;
let config: MoltisConfig = toml::from_str(toml_str).unwrap();
let entry = config.external_agents.agents.get("claude-code").unwrap();
assert_eq!(entry.models, vec!["claude-opus-4-8", "claude-sonnet-4-6"]);
assert_eq!(entry.efforts, vec!["high", "xhigh"]);
}

#[test]
fn provider_entry_wire_api_skip_serializing_default() {
let entry = ProviderEntry::default();
Expand Down
4 changes: 4 additions & 0 deletions crates/config/src/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -693,6 +693,8 @@ port = {port} # Port number (auto-generated for this i
# [external_agents.agents.claude-code]
# binary = "claude" # Override binary path (default: look up on $PATH)
# args = ["-p", "--output-format", "json"]
# models = ["claude-opus-4-8", "claude-sonnet-4-6"] # Optional model choices shown in /model
# efforts = ["high", "xhigh"] # Optional effort choices shown in /model
# working_dir = "." # Override working directory
# timeout_secs = 300 # Session timeout
# use_tmux = false # Force tmux backend (vs direct PTY)
Expand All @@ -702,6 +704,8 @@ port = {port} # Port number (auto-generated for this i
# [external_agents.agents.codex]
# binary = "codex"
# args = ["app-server"]
# models = ["gpt-5.5", "gpt-5.4"]
# efforts = ["medium", "high", "xhigh"]

# [external_agents.agents.acp]
# binary = "/path/to/acp-agent"
Expand Down
2 changes: 2 additions & 0 deletions crates/config/src/validate/schema_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub(super) fn build_schema_map() -> KnownKeys {
Struct(HashMap::from([
("binary", Leaf),
("args", Array(Box::new(Leaf))),
("models", Array(Box::new(Leaf))),
("efforts", Array(Box::new(Leaf))),
("env", Map(Box::new(Leaf))),
("working_dir", Leaf),
("timeout_secs", Leaf),
Expand Down
4 changes: 4 additions & 0 deletions crates/config/src/validate/tests/agents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -347,9 +347,13 @@ enabled = true

[external_agents.agents.claude-code]
binary = "claude"
models = ["claude-opus-4-8", "claude-sonnet-4-6"]
efforts = ["high", "xhigh"]

[external_agents.agents.codex]
binary = "codex"
models = ["gpt-5.5", "gpt-5.4"]
efforts = ["medium", "high", "xhigh"]
"#;
let result = validate_toml_str(toml);
let warning = result
Expand Down
65 changes: 65 additions & 0 deletions crates/external-agents/src/runtimes/claude_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ impl ExternalAgentTransport for ClaudeCodeTransport {
spec.working_dir.clone(),
spec.timeout_secs,
spec.external_session_id.clone(),
spec.model.clone(),
spec.effort.clone(),
)))
}
}
Expand All @@ -85,6 +87,8 @@ struct ClaudeCodeSession {
working_dir: Option<PathBuf>,
timeout: Duration,
session_id: Option<String>,
model: Option<String>,
effort: Option<String>,
status: ExternalAgentStatus,
}

Expand All @@ -96,6 +100,8 @@ impl ClaudeCodeSession {
working_dir: Option<PathBuf>,
timeout_secs: Option<u64>,
session_id: Option<String>,
model: Option<String>,
effort: Option<String>,
) -> Self {
Self {
binary,
Expand All @@ -104,12 +110,26 @@ impl ClaudeCodeSession {
working_dir,
timeout: Duration::from_secs(timeout_secs.unwrap_or(300)),
session_id,
model,
effort,
status: ExternalAgentStatus::Idle,
}
}

fn args_for_turn(&self) -> Vec<String> {
let mut args = self.base_args.clone();
if let Some(model) = &self.model
&& !has_model_arg(&args)
{
args.push("--model".to_string());
args.push(model.clone());
}
if let Some(effort) = &self.effort
&& !has_effort_arg(&args)
{
args.push("--effort".to_string());
args.push(effort.clone());
}
if let Some(session_id) = &self.session_id
&& !has_resume_arg(&args)
{
Expand Down Expand Up @@ -221,6 +241,15 @@ fn has_resume_arg(args: &[String]) -> bool {
.any(|arg| matches!(arg.as_str(), "--resume" | "-r" | "--continue" | "-c"))
}

fn has_model_arg(args: &[String]) -> bool {
args.iter()
.any(|arg| matches!(arg.as_str(), "--model" | "-m"))
}

fn has_effort_arg(args: &[String]) -> bool {
args.iter().any(|arg| arg == "--effort")
}
Comment on lines +244 to +252

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 has_model_arg and has_effort_arg miss the --flag=value form

If a user configures args = ["--model=opus"] or args = ["--effort=low"] in their agent config (equals-sign form), these guards return false and a second conflicting flag is appended — e.g. ["--model=opus", "--model", "claude-opus-4"]. The codex version handles --model=value via strip_prefix("--config=") but the Claude Code version has no equivalent.

Suggested change
fn has_model_arg(args: &[String]) -> bool {
args.iter()
.any(|arg| matches!(arg.as_str(), "--model" | "-m"))
}
fn has_effort_arg(args: &[String]) -> bool {
args.iter().any(|arg| arg == "--effort")
}
fn has_model_arg(args: &[String]) -> bool {
args.iter().any(|arg| {
matches!(arg.as_str(), "--model" | "-m")
|| arg.starts_with("--model=")
|| arg.starts_with("-m=")
})
}
fn has_effort_arg(args: &[String]) -> bool {
args.iter()
.any(|arg| arg == "--effort" || arg.starts_with("--effort="))
}


#[cfg(test)]
mod tests {
use {
Expand Down Expand Up @@ -256,6 +285,8 @@ mod tests {
None,
None,
None,
None,
None,
);
assert!(!session.args_for_turn().iter().any(|arg| arg == "--resume"));
session.session_id = Some("sid".to_string());
Expand All @@ -277,6 +308,8 @@ mod tests {
None,
None,
Some("persisted".to_string()),
None,
None,
);

assert_eq!(session.external_session_id(), Some("persisted"));
Expand Down Expand Up @@ -314,6 +347,8 @@ printf '%s\n' '{"result":"ok","session_id":"sid-1"}'
None,
Some(5),
None,
None,
None,
);

let first = session
Expand All @@ -338,4 +373,34 @@ printf '%s\n' '{"result":"ok","session_id":"sid-1"}'
fs::remove_dir_all(dir)?;
Ok(())
}

#[test]
fn adds_model_arg_when_model_is_selected() {
let session = ClaudeCodeSession::new(
"claude".to_string(),
vec![
"-p".to_string(),
"--output-format".to_string(),
"json".to_string(),
],
HashMap::new(),
None,
None,
Some("sid".to_string()),
Some("opus".to_string()),
Some("xhigh".to_string()),
);

assert_eq!(session.args_for_turn(), vec![
"-p",
"--output-format",
"json",
"--model",
"opus",
"--effort",
"xhigh",
"--resume",
"sid"
]);
}
}
108 changes: 108 additions & 0 deletions crates/external-agents/src/runtimes/codex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl ExternalAgentTransport for CodexTransport {
} else {
spec.args.clone()
};
let args = args_with_model_and_effort(args, spec.model.as_deref(), spec.effort.as_deref());
Ok(Box::new(
CodexAppServerSession::start(
binary,
Expand Down Expand Up @@ -382,6 +383,75 @@ fn extract_usage(value: &Value) -> Option<crate::types::TokenUsage> {
})
}

fn args_with_model_and_effort(
mut args: Vec<String>,
model: Option<&str>,
effort: Option<&str>,
) -> Vec<String> {
let insert_at = args.iter().position(|arg| arg == "app-server").unwrap_or(0);
let mut inserts = Vec::new();
if let Some(model) = model
&& !has_model_arg(&args)
{
inserts.extend(["--model".to_string(), model.to_string()]);
}
if let Some(effort) = effort
&& !has_effort_arg(&args)
{
inserts.extend([
"-c".to_string(),
format!("model_reasoning_effort=\"{effort}\""),
]);
}
for (offset, arg) in inserts.into_iter().enumerate() {
args.insert(insert_at + offset, arg);
}
args
}

fn has_model_arg(args: &[String]) -> bool {
let mut iter = args.iter().peekable();
while let Some(arg) = iter.next() {
if matches!(arg.as_str(), "--model" | "-m") {
return true;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2 has_model_arg misses --model=value form in base args

Similar to the claude_code.rs issue: if a user puts --model=gpt-5.5 as a single element in their args config, the guard returns false and a second --model flag is injected before app-server. The equals-sign form is not covered by the matches!(arg.as_str(), "--model" | "-m") branch.

Suggested change
if matches!(arg.as_str(), "--model" | "-m") {
return true;
}
if matches!(arg.as_str(), "--model" | "-m")
|| arg.starts_with("--model=")
|| arg.starts_with("-m=")
{
return true;
}

if matches!(arg.as_str(), "--config" | "-c")
&& iter
.peek()
.is_some_and(|next| next.trim_start().starts_with("model="))
{
return true;
}
if arg
.strip_prefix("--config=")
.is_some_and(|value| value.trim_start().starts_with("model="))
{
return true;
}
}
false
}

fn has_effort_arg(args: &[String]) -> bool {
let mut iter = args.iter().peekable();
while let Some(arg) = iter.next() {
if matches!(arg.as_str(), "--config" | "-c")
&& iter
.peek()
.is_some_and(|next| next.trim_start().starts_with("model_reasoning_effort="))
{
return true;
}
if arg
.strip_prefix("--config=")
.is_some_and(|value| value.trim_start().starts_with("model_reasoning_effort="))
{
return true;
}
}
false
}

fn token_count_field(value: &Value, fields: &[&str]) -> Option<u32> {
fields.iter().find_map(|field| {
value
Expand Down Expand Up @@ -443,6 +513,44 @@ mod tests {
assert_eq!(camel.output_tokens, 55);
}

#[test]
fn inserts_model_and_effort_before_app_server_command() {
assert_eq!(
args_with_model_and_effort(
vec!["app-server".to_string()],
Some("gpt-5.5"),
Some("xhigh")
),
vec![
"--model",
"gpt-5.5",
"-c",
"model_reasoning_effort=\"xhigh\"",
"app-server"
]
);
assert_eq!(
args_with_model_and_effort(
vec![
"--model".to_string(),
"configured".to_string(),
"-c".to_string(),
"model_reasoning_effort=\"high\"".to_string(),
"app-server".to_string()
],
Some("ignored"),
Some("ignored"),
),
vec![
"--model",
"configured",
"-c",
"model_reasoning_effort=\"high\"",
"app-server"
]
);
}

#[tokio::test]
async fn codex_session_reuses_thread_for_multiple_prompts() -> anyhow::Result<()> {
let unique = SystemTime::now()
Expand Down
4 changes: 4 additions & 0 deletions crates/external-agents/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ pub struct ExternalAgentSpec {
pub kind: AgentTransportKind,
pub session_key: Option<String>,
pub external_session_id: Option<String>,
pub model: Option<String>,
pub effort: Option<String>,
pub binary: Option<String>,
pub args: Vec<String>,
pub env: HashMap<String, String>,
Expand All @@ -90,6 +92,8 @@ impl ExternalAgentSpec {
kind,
session_key: None,
external_session_id: None,
model: None,
effort: None,
binary: None,
args: Vec::new(),
env: HashMap::new(),
Expand Down
Loading