diff --git a/crates/g3-cli/src/interactive.rs b/crates/g3-cli/src/interactive.rs index 5c53e14..addea60 100644 --- a/crates/g3-cli/src/interactive.rs +++ b/crates/g3-cli/src/interactive.rs @@ -52,6 +52,18 @@ pub fn build_prompt(in_multiline: bool, in_plan_mode: bool, agent_name: Option<& } } +/// Prepare user input for plan mode, prepending "Create a plan: " if this is the first message. +/// Returns the (possibly modified) input and whether the flag should be reset. +pub fn prepare_plan_mode_input(input: &str, is_first_plan_message: bool, in_plan_mode: bool) -> (String, bool) { + if in_plan_mode && is_first_plan_message { + // Prepend "Create a plan: " and signal to reset the flag + (format!("Create a plan: {}", input), true) + } else { + // No modification needed + (input.to_string(), false) + } +} + /// Check if the input is an approval command (for plan mode). /// /// Recognizes: "a", "approve", "approved", and common misspellings. @@ -191,6 +203,9 @@ pub async fn run_interactive( // Track plan mode state (start in plan mode for non-agent mode) let mut in_plan_mode = !from_agent_mode; + // Track if this is the first message in plan mode (to prepend "Create a plan: ") + let mut is_first_plan_message = in_plan_mode; + // Sync agent's plan mode state with CLI state agent.set_plan_mode(in_plan_mode); @@ -275,8 +290,13 @@ pub async fn run_interactive( // Reprint input with formatting reprint_formatted_input(&input, &prompt); + // Prepend "Create a plan: " for first message in plan mode + let (final_input, should_reset) = prepare_plan_mode_input(&input, is_first_plan_message, in_plan_mode); + if should_reset { + is_first_plan_message = false; + } execute_user_input( - &mut agent, &input, show_prompt, show_code, &output, from_agent_mode + &mut agent, &final_input, show_prompt, show_code, &output, from_agent_mode ).await; } else { // Single line input @@ -328,6 +348,7 @@ pub async fn run_interactive( CommandResult::EnterPlanMode => { in_plan_mode = true; agent.set_plan_mode(true); + is_first_plan_message = true; continue; } } @@ -336,8 +357,13 @@ pub async fn run_interactive( // Reprint input with formatting reprint_formatted_input(&input, &prompt); + // Prepend "Create a plan: " for first message in plan mode + let (final_input, should_reset) = prepare_plan_mode_input(&input, is_first_plan_message, in_plan_mode); + if should_reset { + is_first_plan_message = false; + } execute_user_input( - &mut agent, &input, show_prompt, show_code, &output, from_agent_mode + &mut agent, &final_input, show_prompt, show_code, &output, from_agent_mode ).await; } } @@ -529,5 +555,65 @@ mod tests { assert!(is_approval_input("yes!")); assert!(is_approval_input("ok,")); } + + // Tests for prepare_plan_mode_input + + #[test] + fn test_prepare_plan_mode_input_happy_path_first_message() { + // Happy path: First message in plan mode gets "Create a plan: " prefix + let (result, should_reset) = prepare_plan_mode_input("fix the bug", true, true); + assert_eq!(result, "Create a plan: fix the bug"); + assert!(should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_negative_second_message() { + // Negative: Second message (is_first_plan_message = false) should NOT get prefix + let (result, should_reset) = prepare_plan_mode_input("fix the bug", false, true); + assert_eq!(result, "fix the bug"); + assert!(!should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_negative_not_in_plan_mode() { + // Negative: Not in plan mode should NOT get prefix even if is_first_plan_message is true + let (result, should_reset) = prepare_plan_mode_input("fix the bug", true, false); + assert_eq!(result, "fix the bug"); + assert!(!should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_negative_neither_condition() { + // Negative: Neither in plan mode nor first message + let (result, should_reset) = prepare_plan_mode_input("fix the bug", false, false); + assert_eq!(result, "fix the bug"); + assert!(!should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_boundary_empty_input() { + // Boundary: Empty input would get prefix, but in practice empty input + // is filtered out by the caller before reaching this function. + // This test documents the function's behavior in isolation. + let (result, should_reset) = prepare_plan_mode_input("", true, true); + assert_eq!(result, "Create a plan: "); + assert!(should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_boundary_whitespace_input() { + // Boundary: Whitespace-only input gets prefix preserved + let (result, should_reset) = prepare_plan_mode_input(" ", true, true); + assert_eq!(result, "Create a plan: "); + assert!(should_reset); + } + + #[test] + fn test_prepare_plan_mode_input_boundary_multiline_input() { + // Boundary: Multiline input gets prefix on first line only + let (result, should_reset) = prepare_plan_mode_input("line1\nline2\nline3", true, true); + assert_eq!(result, "Create a plan: line1\nline2\nline3"); + assert!(should_reset); + } } diff --git a/crates/g3-cli/src/ui_writer_impl.rs b/crates/g3-cli/src/ui_writer_impl.rs index fd78f3b..4551606 100644 --- a/crates/g3-cli/src/ui_writer_impl.rs +++ b/crates/g3-cli/src/ui_writer_impl.rs @@ -716,6 +716,7 @@ impl UiWriter for ConsoleUiWriter { #[derive(serde::Deserialize)] struct PlanCompact { plan_id: String, + #[allow(dead_code)] revision: u32, approved_revision: Option, items: Vec, @@ -731,6 +732,7 @@ impl UiWriter for ConsoleUiWriter { #[serde(default)] evidence: Vec, #[serde(default)] + #[allow(dead_code)] notes: Option, } #[derive(serde::Deserialize)] diff --git a/crates/g3-core/src/tools/plan.rs b/crates/g3-core/src/tools/plan.rs index 02a10b0..e57b5e6 100644 --- a/crates/g3-core/src/tools/plan.rs +++ b/crates/g3-core/src/tools/plan.rs @@ -1010,8 +1010,34 @@ pub fn check_plan_approval_gate(session_id: &str, working_dir: Option<&str>) -> // Check if a plan exists and whether it's approved let plan = match read_plan(session_id) { Ok(Some(plan)) => plan, - Ok(None) => return ApprovalGateResult::Allowed, // No plan, allow - Err(_) => return ApprovalGateResult::Allowed, // Can't read plan, allow + Ok(None) => { + // No plan exists - check if there are file changes that need blocking + let status_output = std::process::Command::new("git") + .args(["status", "--porcelain"]) + .current_dir(dir) + .output(); + + let output = match status_output { + Ok(o) if o.status.success() => String::from_utf8_lossy(&o.stdout).to_string(), + _ => return ApprovalGateResult::Allowed, + }; + + if output.trim().is_empty() { + return ApprovalGateResult::Allowed; // No changes, allow + } + + // There are file changes but no plan - block and require plan creation + return ApprovalGateResult::Blocked { + message: "⚠️ IMPLEMENTATION BLOCKED\n\n\ + You attempted to modify files without creating a plan first.\n\n\ + Before implementing, you must:\n\ + 1. Create a plan with `plan_write`\n\ + 2. Get the plan approved by the user\n\n\ + Do not attempt to implement until the plan is approved.".to_string(), + reverted_files: vec![], + }; + } + Err(_) => return ApprovalGateResult::Allowed, // Can't read plan, allow (error case) }; if plan.is_approved() { @@ -1424,12 +1450,40 @@ items: [] } #[test] - fn test_approval_gate_no_plan() { - // With a non-existent session, there's no plan, so it should allow - let result = check_plan_approval_gate("nonexistent-session-xyz", Some(".")); + fn test_approval_gate_no_plan_no_changes() { + // With a non-existent session and no uncommitted changes, it should allow. + // We use a temp dir that's a fresh git repo with no changes. + let temp_dir = tempfile::tempdir().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(temp_dir.path()) + .output() + .unwrap(); + let result = check_plan_approval_gate("nonexistent-session-xyz", Some(temp_dir.path().to_str().unwrap())); assert!(matches!(result, ApprovalGateResult::Allowed)); } + #[test] + fn test_approval_gate_no_plan_with_changes() { + // With a non-existent session but uncommitted changes, it should block. + let temp_dir = tempfile::tempdir().unwrap(); + std::process::Command::new("git") + .args(["init"]) + .current_dir(temp_dir.path()) + .output() + .unwrap(); + // Create an untracked file to simulate changes + std::fs::write(temp_dir.path().join("new_file.txt"), "test content").unwrap(); + + let result = check_plan_approval_gate("nonexistent-session-xyz", Some(temp_dir.path().to_str().unwrap())); + assert!(matches!(result, ApprovalGateResult::Blocked { .. })); + + // Verify the blocking message mentions creating a plan + if let ApprovalGateResult::Blocked { message, .. } = result { + assert!(message.contains("plan_write")); + } + } + #[test] fn test_approval_gate_not_git_repo() { // /tmp is typically not a git repo