Compare commits

...

2 Commits

Author SHA1 Message Date
Jochen
ccb8383f6b fix plan check catch-22 2026-03-23 21:04:39 +11:00
Jochen
c343dfa2f0 fix openai tool calls, also plan bug where plan is empty. 2026-03-23 20:49:58 +11:00
6 changed files with 80 additions and 14 deletions

View File

@@ -207,6 +207,9 @@ pub async fn run_agent_mode(
if flags.acd { if flags.acd {
agent.set_acd_enabled(true); agent.set_acd_enabled(true);
} }
if flags.skip_plan_tool_check {
agent.set_skip_plan_tool_check(true);
}
// If resuming a session, restore context and TODO // If resuming a session, restore context and TODO
let initial_task = if let Some(ref incomplete_session) = resuming_session { let initial_task = if let Some(ref incomplete_session) = resuming_session {

View File

@@ -32,6 +32,8 @@ pub struct CommonFlags {
pub project: Option<PathBuf>, pub project: Option<PathBuf>,
/// Resume a specific session by ID /// Resume a specific session by ID
pub resume: Option<String>, pub resume: Option<String>,
/// Skip the plan approval gate for plan tools
pub skip_plan_tool_check: bool,
} }
#[derive(Parser, Clone)] #[derive(Parser, Clone)]
@@ -161,6 +163,11 @@ pub struct Cli {
/// Load a project from the given path at startup (like /project but without auto-prompt) /// Load a project from the given path at startup (like /project but without auto-prompt)
#[arg(long, value_name = "PATH")] #[arg(long, value_name = "PATH")]
pub project: Option<PathBuf>, pub project: Option<PathBuf>,
/// Skip the plan approval gate check for plan tools (plan_read, plan_write, plan_approve).
/// Without this flag, plan tools are subject to the same approval gate as other tools.
#[arg(long)]
pub skip_plan_tool_check: bool,
} }
impl Cli { impl Cli {
@@ -179,6 +186,7 @@ impl Cli {
acd: self.acd, acd: self.acd,
project: self.project.clone(), project: self.project.clone(),
resume: self.resume.clone(), resume: self.resume.clone(),
skip_plan_tool_check: self.skip_plan_tool_check,
} }
} }
} }

View File

@@ -193,6 +193,9 @@ async fn run_console_mode(
if cli.acd { if cli.acd {
agent.set_acd_enabled(true); agent.set_acd_enabled(true);
} }
if cli.skip_plan_tool_check {
agent.set_skip_plan_tool_check(true);
}
// Load CLI project if --project flag was specified // Load CLI project if --project flag was specified
let initial_project: Option<project::Project> = if let Some(ref project_path) = cli.project { let initial_project: Option<project::Project> = if let Some(ref project_path) = cli.project {

View File

@@ -166,6 +166,9 @@ pub struct Agent<W: UiWriter> {
acd_enabled: bool, acd_enabled: bool,
/// Whether plan mode is active (gate blocks file changes without approved plan) /// Whether plan mode is active (gate blocks file changes without approved plan)
in_plan_mode: bool, in_plan_mode: bool,
/// When true, plan tools (plan_read/write/approve) skip the approval gate.
/// Controlled by --skip-plan-tool-check CLI flag.
skip_plan_tool_check: bool,
/// Files that were already dirty when plan mode started (excluded from approval gate) /// Files that were already dirty when plan mode started (excluded from approval gate)
baseline_dirty_files: std::collections::HashSet<String>, baseline_dirty_files: std::collections::HashSet<String>,
/// Manager for async research tasks /// Manager for async research tasks
@@ -226,6 +229,7 @@ impl<W: UiWriter> Agent<W> {
auto_memory: false, auto_memory: false,
acd_enabled: false, acd_enabled: false,
in_plan_mode: false, in_plan_mode: false,
skip_plan_tool_check: false,
baseline_dirty_files: std::collections::HashSet::new(), baseline_dirty_files: std::collections::HashSet::new(),
pending_research_manager: pending_research::PendingResearchManager::new(), pending_research_manager: pending_research::PendingResearchManager::new(),
loaded_toolsets: std::collections::HashSet::new(), loaded_toolsets: std::collections::HashSet::new(),
@@ -1688,6 +1692,11 @@ impl<W: UiWriter> Agent<W> {
self.in_plan_mode self.in_plan_mode
} }
/// Set whether plan tools skip the approval gate.
pub fn set_skip_plan_tool_check(&mut self, skip: bool) {
self.skip_plan_tool_check = skip;
}
/// Check if the current plan is in a terminal state (all items done or blocked). /// Check if the current plan is in a terminal state (all items done or blocked).
/// ///
/// Returns true if: /// Returns true if:
@@ -3022,7 +3031,12 @@ Skip if nothing new. Be brief."#;
let result = self.execute_tool_inner_in_dir(tool_call, working_dir).await; let result = self.execute_tool_inner_in_dir(tool_call, working_dir).await;
// Check plan approval gate after tool execution (only in plan mode) // Check plan approval gate after tool execution (only in plan mode)
if self.in_plan_mode { // Plan tools (plan_read/write/approve) only skip the gate when
// --skip-plan-tool-check is passed; otherwise they are gated like
// every other tool.
let is_plan_tool = matches!(tool_call.tool.as_str(), "plan_write" | "plan_read" | "plan_approve");
let dominated_by_gate = if is_plan_tool { !self.skip_plan_tool_check } else { true };
if self.in_plan_mode && dominated_by_gate {
if let Some(session_id) = &self.session_id { if let Some(session_id) = &self.session_id {
if let ApprovalGateResult::Blocked { message } = if let ApprovalGateResult::Blocked { message } =
check_plan_approval_gate(session_id, working_dir, &self.baseline_dirty_files) check_plan_approval_gate(session_id, working_dir, &self.baseline_dirty_files)

View File

@@ -815,7 +815,7 @@ pub async fn execute_plan_read<W: UiWriter>(
} }
None => { None => {
ctx.ui_writer.print_plan_compact(None, None, false); ctx.ui_writer.print_plan_compact(None, None, false);
Ok(String::new()) Ok("No plan exists yet. Use plan_write to create one.".to_string())
} }
} }
} }

View File

@@ -415,19 +415,57 @@ impl LLMProvider for OpenAIProvider {
} }
fn convert_messages(messages: &[Message]) -> Vec<serde_json::Value> { fn convert_messages(messages: &[Message]) -> Vec<serde_json::Value> {
messages let mut result = Vec::new();
.iter() for msg in messages {
.map(|msg| { // Tool result messages: OpenAI expects role "tool" with tool_call_id
json!({ if let Some(ref tool_call_id) = msg.tool_result_id {
"role": match msg.role { result.push(json!({
"role": "tool",
"tool_call_id": tool_call_id,
"content": msg.content,
}));
continue;
}
let role = match msg.role {
MessageRole::System => "system", MessageRole::System => "system",
MessageRole::User => "user", MessageRole::User => "user",
MessageRole::Assistant => "assistant", MessageRole::Assistant => "assistant",
}, };
// Assistant messages with tool calls
if !msg.tool_calls.is_empty() {
let tool_calls: Vec<serde_json::Value> = msg.tool_calls.iter().map(|tc| {
json!({
"id": tc.id,
"type": "function",
"function": {
"name": tc.name,
"arguments": tc.input.to_string(),
}
})
}).collect();
let mut m = json!({
"role": role,
"tool_calls": tool_calls,
});
// Include content only if non-empty (OpenAI allows null/absent content
// on assistant messages that have tool_calls)
if !msg.content.is_empty() {
m["content"] = json!(msg.content);
}
result.push(m);
continue;
}
// Regular messages
result.push(json!({
"role": role,
"content": msg.content, "content": msg.content,
}) }));
}) }
.collect() result
} }
fn convert_tools(tools: &[Tool]) -> Vec<serde_json::Value> { fn convert_tools(tools: &[Tool]) -> Vec<serde_json::Value> {