diff --git a/crates/tui/src/commands/mod.rs b/crates/tui/src/commands/mod.rs index 9a953be62..17635e0cb 100644 --- a/crates/tui/src/commands/mod.rs +++ b/crates/tui/src/commands/mod.rs @@ -1171,6 +1171,7 @@ mod tests { step: "keep checklist primary".to_string(), status: StepStatus::InProgress, }], + ..Default::default() }); } diff --git a/crates/tui/src/prompts/modes/plan.md b/crates/tui/src/prompts/modes/plan.md index bcbfb229f..01a80749d 100644 --- a/crates/tui/src/prompts/modes/plan.md +++ b/crates/tui/src/prompts/modes/plan.md @@ -8,6 +8,22 @@ the final plan; that is the handoff signal that lets the UI show the accept / re All writes and patches are blocked — you can read the world but you can't change it. Shell and code execution are unavailable. +Produce a grounded plan artifact through `update_plan`, not just a step list. Include what you +discovered during investigation so the user can make an informed decision: + +- `title` — short summary +- `objective` — what this plan aims to achieve +- `context_summary` — what you found during investigation +- `sources_used` — files, docs, commands, or sub-agents consulted +- `constraints` — user rules, repo constitution, mode limits, safety constraints +- `recommended_approach` — high-level approach and rationale +- `plan` — ordered execution steps with statuses (required) +- `verification_plan` — how to verify correctness +- `risks_and_unknowns` — known risks, assumptions, open questions +- `handoff_packet` — compact summary for Agent-mode execution + +All fields except `plan` are optional — only include what you actually discovered. + Use this mode to build a thorough plan. Spawn read-only sub-agents for parallel investigation. After `update_plan` presents the plan, wait for the user's next action instead of continuing to tool around in Plan mode. diff --git a/crates/tui/src/tools/plan.rs b/crates/tui/src/tools/plan.rs index 17caab4f6..b13a61255 100644 --- a/crates/tui/src/tools/plan.rs +++ b/crates/tui/src/tools/plan.rs @@ -54,11 +54,42 @@ pub struct PlanItemArg { } /// Update payload used by the plan tool. -#[derive(Debug, Clone, Serialize, Deserialize)] +/// +/// Backward-compatible: legacy callers that only send `plan` + optional +/// `explanation` still work. New v0.9.0 fields are all `Option` and +/// default to `None` / empty. +#[derive(Debug, Clone, Serialize, Deserialize, Default)] pub struct UpdatePlanArgs { #[serde(default)] - pub explanation: Option, + pub title: Option, + #[serde(default)] + pub objective: Option, + #[serde(default)] + pub context_summary: Option, + #[serde(default)] + pub sources_used: Vec, + #[serde(default)] + pub constraints: Vec, + #[serde(default)] + pub recommended_approach: Option, + #[serde(default)] pub plan: Vec, + #[serde(default)] + pub verification_plan: Option, + #[serde(default)] + pub risks_and_unknowns: Option, + #[serde(default)] + pub handoff_packet: Option, + #[serde(default)] + pub explanation: Option, +} + +/// Discriminant for plan artifacts serialised into the transcript. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum PlanKind { + /// Standard `update_plan` outcome (plan mode, hunt protocol, etc.). + Update, } // === Plan State === @@ -117,25 +148,77 @@ impl PlanStep { /// Serializable snapshot for display #[derive(Debug, Clone, Serialize)] pub struct PlanSnapshot { + pub kind: PlanKind, + #[serde(skip_serializing_if = "Option::is_none")] + pub title: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub objective: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub context_summary: Option, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub sources_used: Vec, + #[serde(skip_serializing_if = "Vec::is_empty")] + pub constraints: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub recommended_approach: Option, + #[serde(skip_serializing_if = "Option::is_none")] pub explanation: Option, pub items: Vec, + #[serde(skip_serializing_if = "Option::is_none")] + pub verification_plan: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub risks_and_unknowns: Option, + #[serde(skip_serializing_if = "Option::is_none")] + pub handoff_packet: Option, } /// State tracking for the current plan #[derive(Debug, Clone, Default)] pub struct PlanState { - explanation: Option, + title: Option, + objective: Option, + context_summary: Option, + sources_used: Vec, + constraints: Vec, + recommended_approach: Option, steps: Vec, + verification_plan: Option, + risks_and_unknowns: Option, + handoff_packet: Option, + explanation: Option, } impl PlanState { /// Check whether the plan is empty. #[must_use] pub fn is_empty(&self) -> bool { - self.steps.is_empty() && self.explanation.as_deref().unwrap_or("").is_empty() + self.steps.is_empty() + && self.explanation.as_deref().unwrap_or("").is_empty() + && self.title.as_deref().unwrap_or("").is_empty() + && self.objective.as_deref().unwrap_or("").is_empty() + && self.context_summary.as_deref().unwrap_or("").is_empty() + && self + .recommended_approach + .as_deref() + .unwrap_or("") + .is_empty() + && self.verification_plan.as_deref().unwrap_or("").is_empty() + && self.risks_and_unknowns.as_deref().unwrap_or("").is_empty() + && self.handoff_packet.as_deref().unwrap_or("").is_empty() + && self.sources_used.is_empty() + && self.constraints.is_empty() } pub fn update(&mut self, args: UpdatePlanArgs) { + self.title = args.title.filter(|s| !s.trim().is_empty()); + self.objective = args.objective.filter(|s| !s.trim().is_empty()); + self.context_summary = args.context_summary.filter(|s| !s.trim().is_empty()); + self.sources_used = args.sources_used; + self.constraints = args.constraints; + self.recommended_approach = args.recommended_approach.filter(|s| !s.trim().is_empty()); + self.verification_plan = args.verification_plan.filter(|s| !s.trim().is_empty()); + self.risks_and_unknowns = args.risks_and_unknowns.filter(|s| !s.trim().is_empty()); + self.handoff_packet = args.handoff_packet.filter(|s| !s.trim().is_empty()); self.explanation = args.explanation.filter(|s| !s.trim().is_empty()); let now = Instant::now(); @@ -186,6 +269,13 @@ impl PlanState { pub fn snapshot(&self) -> PlanSnapshot { PlanSnapshot { + kind: PlanKind::Update, + title: self.title.clone(), + objective: self.objective.clone(), + context_summary: self.context_summary.clone(), + sources_used: self.sources_used.clone(), + constraints: self.constraints.clone(), + recommended_approach: self.recommended_approach.clone(), explanation: self.explanation.clone(), items: self .steps @@ -195,9 +285,42 @@ impl PlanState { status: s.status.clone(), }) .collect(), + verification_plan: self.verification_plan.clone(), + risks_and_unknowns: self.risks_and_unknowns.clone(), + handoff_packet: self.handoff_packet.clone(), } } + #[allow(dead_code)] + pub fn title(&self) -> Option<&str> { + self.title.as_deref() + } + + #[allow(dead_code)] + pub fn objective(&self) -> Option<&str> { + self.objective.as_deref() + } + + #[allow(dead_code)] + pub fn context_summary(&self) -> Option<&str> { + self.context_summary.as_deref() + } + + #[allow(dead_code)] + pub fn sources_used(&self) -> &[String] { + &self.sources_used + } + + #[allow(dead_code)] + pub fn constraints(&self) -> &[String] { + &self.constraints + } + + #[allow(dead_code)] + pub fn recommended_approach(&self) -> Option<&str> { + self.recommended_approach.as_deref() + } + pub fn explanation(&self) -> Option<&str> { self.explanation.as_deref() } @@ -206,6 +329,21 @@ impl PlanState { &self.steps } + #[allow(dead_code)] + pub fn verification_plan(&self) -> Option<&str> { + self.verification_plan.as_deref() + } + + #[allow(dead_code)] + pub fn risks_and_unknowns(&self) -> Option<&str> { + self.risks_and_unknowns.as_deref() + } + + #[allow(dead_code)] + pub fn handoff_packet(&self) -> Option<&str> { + self.handoff_packet.as_deref() + } + /// Get counts of steps by status pub fn counts(&self) -> (usize, usize, usize) { let mut pending = 0; @@ -306,20 +444,42 @@ impl ToolSpec for UpdatePlanTool { } fn description(&self) -> &'static str { - "Update optional high-level strategy metadata for complex initiatives. Use checklist_write for primary Work progress; update_plan should capture phase-level approach changes, not duplicate checklist items. Each strategy step has a description and status (pending, in_progress, completed). Optionally include an explanation of the overall approach." + "Publish a structured implementation plan as a reviewable artifact. Include the investigation findings (sources, constraints, risks) so the user can make an informed decision. All fields except `plan` are optional — only provide what you actually discovered." } fn input_schema(&self) -> serde_json::Value { json!({ "type": "object", "properties": { - "explanation": { + "title": { + "type": "string", + "description": "Short title summarizing the plan" + }, + "objective": { + "type": "string", + "description": "What this plan aims to achieve" + }, + "context_summary": { + "type": "string", + "description": "Brief summary of the investigation context" + }, + "sources_used": { + "type": "array", + "description": "Files, docs, commands, or sub-agents consulted", + "items": { "type": "string" } + }, + "constraints": { + "type": "array", + "description": "User rules, repo constitution, mode limits, safety constraints", + "items": { "type": "string" } + }, + "recommended_approach": { "type": "string", - "description": "Optional high-level explanation of the plan or approach" + "description": "High-level approach and rationale" }, "plan": { "type": "array", - "description": "List of plan steps", + "description": "Ordered execution steps", "items": { "type": "object", "properties": { @@ -335,9 +495,36 @@ impl ToolSpec for UpdatePlanTool { }, "required": ["step", "status"] } + }, + "verification_plan": { + "type": "string", + "description": "How to verify the plan was executed correctly" + }, + "risks_and_unknowns": { + "type": "string", + "description": "Known risks, assumptions, and open questions" + }, + "handoff_packet": { + "type": "string", + "description": "Compact execution-ready summary for handoff to Agent mode" + }, + "explanation": { + "type": "string", + "description": "Optional high-level explanation (legacy, prefer recommended_approach)" } }, - "required": ["plan"] + "required": [ + "title", + "objective", + "context_summary", + "sources_used", + "constraints", + "recommended_approach", + "plan", + "verification_plan", + "risks_and_unknowns", + "handoff_packet" + ] }) } @@ -354,10 +541,23 @@ impl ToolSpec for UpdatePlanTool { input: serde_json::Value, _context: &ToolContext, ) -> Result { - let explanation = input - .get("explanation") - .and_then(|v| v.as_str()) - .map(std::string::ToString::to_string); + let opt_str = |key: &str| -> Option { + input + .get(key) + .and_then(|v| v.as_str()) + .map(|s| s.to_string()) + }; + let opt_strs = |key: &str| -> Vec { + input + .get(key) + .and_then(|v| v.as_array()) + .map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(|s| s.to_string())) + .collect() + }) + .unwrap_or_default() + }; let plan_items = input .get("plan") @@ -385,8 +585,17 @@ impl ToolSpec for UpdatePlanTool { } let args = UpdatePlanArgs { - explanation, + title: opt_str("title"), + objective: opt_str("objective"), + context_summary: opt_str("context_summary"), + sources_used: opt_strs("sources_used"), + constraints: opt_strs("constraints"), + recommended_approach: opt_str("recommended_approach"), plan: plan_args, + verification_plan: opt_str("verification_plan"), + risks_and_unknowns: opt_str("risks_and_unknowns"), + handoff_packet: opt_str("handoff_packet"), + explanation: opt_str("explanation"), }; let mut state = self.plan_state.lock().await; @@ -404,3 +613,117 @@ impl ToolSpec for UpdatePlanTool { ))) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn legacy_update_plan_still_works() { + let mut state = PlanState::default(); + state.update(UpdatePlanArgs { + explanation: Some("legacy plan".into()), + plan: vec![PlanItemArg { + step: "step one".into(), + status: StepStatus::Pending, + }], + ..Default::default() + }); + assert_eq!(state.explanation(), Some("legacy plan")); + assert_eq!(state.steps().len(), 1); + assert!(state.title().is_none()); + } + + #[test] + fn full_artifact_fields_parsed_and_stored() { + let mut state = PlanState::default(); + state.update(UpdatePlanArgs { + title: Some("Test Plan".into()), + objective: Some("Test objective".into()), + context_summary: Some("Investigated X".into()), + sources_used: vec!["file_a.rs".into(), "docs/guide.md".into()], + constraints: vec!["no shell access".into()], + recommended_approach: Some("Use incremental refactor".into()), + plan: vec![PlanItemArg { + step: "step one".into(), + status: StepStatus::InProgress, + }], + verification_plan: Some("Run tests".into()), + risks_and_unknowns: Some("Might break old API".into()), + handoff_packet: Some("Compact summary".into()), + ..Default::default() + }); + + assert_eq!(state.title(), Some("Test Plan")); + assert_eq!(state.objective(), Some("Test objective")); + assert_eq!(state.context_summary(), Some("Investigated X")); + assert_eq!(state.sources_used(), &["file_a.rs", "docs/guide.md"]); + assert_eq!(state.constraints(), &["no shell access"]); + assert_eq!( + state.recommended_approach(), + Some("Use incremental refactor") + ); + assert_eq!(state.steps().len(), 1); + assert_eq!(state.verification_plan(), Some("Run tests")); + assert_eq!(state.risks_and_unknowns(), Some("Might break old API")); + assert_eq!(state.handoff_packet(), Some("Compact summary")); + } + + #[test] + fn snapshot_skips_empty_fields() { + let mut state = PlanState::default(); + state.update(UpdatePlanArgs { + plan: vec![PlanItemArg { + step: "only step".into(), + status: StepStatus::Completed, + }], + ..Default::default() + }); + + let snapshot = state.snapshot(); + let json = serde_json::to_string(&snapshot).unwrap(); + // Verify that only non-empty fields appear. + assert!(json.contains("\"items\"")); + assert!(json.contains("\"step\"")); + // Empty/Optional fields should be absent. + assert!(!json.contains("\"title\"")); + assert!(!json.contains("\"sources_used\"")); + } + + #[test] + fn snapshot_includes_new_fields_when_present() { + let mut state = PlanState::default(); + state.update(UpdatePlanArgs { + title: Some("Rich Plan".into()), + verification_plan: Some("Verify with CI".into()), + plan: vec![], + ..Default::default() + }); + + let snapshot = state.snapshot(); + let json = serde_json::to_string(&snapshot).unwrap(); + assert!(json.contains("\"title\"")); + assert!(json.contains("Rich Plan")); + assert!(json.contains("\"verification_plan\"")); + } + + #[test] + fn is_empty_respects_new_fields() { + let mut state = PlanState::default(); + assert!(state.is_empty()); + + state.update(UpdatePlanArgs { + title: Some("Non-empty".into()), + ..Default::default() + }); + assert!(!state.is_empty(), "title should make plan non-empty"); + } + + #[test] + fn default_update_plan_args_is_usable() { + let args = UpdatePlanArgs::default(); + assert!(args.plan.is_empty()); + assert!(args.title.is_none()); + assert!(args.sources_used.is_empty()); + } +} diff --git a/crates/tui/src/tui/app.rs b/crates/tui/src/tui/app.rs index 3eb494f16..3b2531b38 100644 --- a/crates/tui/src/tui/app.rs +++ b/crates/tui/src/tui/app.rs @@ -5918,6 +5918,7 @@ mod tests { step: "step 1".to_string(), status: StepStatus::InProgress, }], + ..Default::default() }); assert!(!plan.is_empty()); }