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 {
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
let initial_task = if let Some(ref incomplete_session) = resuming_session {

View File

@@ -32,6 +32,8 @@ pub struct CommonFlags {
pub project: Option<PathBuf>,
/// Resume a specific session by ID
pub resume: Option<String>,
/// Skip the plan approval gate for plan tools
pub skip_plan_tool_check: bool,
}
#[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)
#[arg(long, value_name = "PATH")]
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 {
@@ -179,6 +186,7 @@ impl Cli {
acd: self.acd,
project: self.project.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 {
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
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,
/// Whether plan mode is active (gate blocks file changes without approved plan)
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)
baseline_dirty_files: std::collections::HashSet<String>,
/// Manager for async research tasks
@@ -226,6 +229,7 @@ impl<W: UiWriter> Agent<W> {
auto_memory: false,
acd_enabled: false,
in_plan_mode: false,
skip_plan_tool_check: false,
baseline_dirty_files: std::collections::HashSet::new(),
pending_research_manager: pending_research::PendingResearchManager::new(),
loaded_toolsets: std::collections::HashSet::new(),
@@ -1688,6 +1692,11 @@ impl<W: UiWriter> Agent<W> {
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).
///
/// 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;
// 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 ApprovalGateResult::Blocked { message } =
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 => {
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> {
messages
.iter()
.map(|msg| {
json!({
"role": match msg.role {
let mut result = Vec::new();
for msg in messages {
// Tool result messages: OpenAI expects role "tool" with tool_call_id
if let Some(ref tool_call_id) = msg.tool_result_id {
result.push(json!({
"role": "tool",
"tool_call_id": tool_call_id,
"content": msg.content,
}));
continue;
}
let role = match msg.role {
MessageRole::System => "system",
MessageRole::User => "user",
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,
})
})
.collect()
}));
}
result
}
fn convert_tools(tools: &[Tool]) -> Vec<serde_json::Value> {