Merge origin/master into makima/files-under-contracts-combined - resolve import conflictsmakima/files-under-contracts-combined

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

5 files changed, 234 insertions, 10 deletions

diff --git a/makima/src/server/handlers/contract_chat.rs b/makima/src/server/handlers/contract_chat.rs
index e2adb72..28c3436 100644
--- a/makima/src/server/handlers/contract_chat.rs
+++ b/makima/src/server/handlers/contract_chat.rs
@@ -20,7 +20,7 @@ use crate::db::{
 };
 use crate::llm::{
     all_templates, analyze_task_output, body_to_markdown, format_checklist_markdown,
-    format_parsed_tasks, get_phase_checklist, parse_tasks_from_breakdown,
+    format_parsed_tasks, parse_tasks_from_breakdown,
     claude::{self, ClaudeClient, ClaudeError, ClaudeModel},
     groq::{GroqClient, GroqError, Message, ToolCallResponse},
     parse_contract_tool_call, templates_for_phase, ContractToolRequest, FileInfo,
@@ -433,8 +433,8 @@ When a new contract is created or the user seems unsure:
 fn build_contract_context(contract: &crate::db::models::ContractWithRelations) -> String {
     let c = &contract.contract;
     let mut context = format!(
-        "Name: {}\nID: {}\nPhase: {}\nStatus: {}\n",
-        c.name, c.id, c.phase, c.status
+        "Name: {}\nID: {}\nPhase: {}\nStatus: {}\nContract Type: {}\nAutonomous Loop: {}\n",
+        c.name, c.id, c.phase, c.status, c.contract_type, c.autonomous_loop
     );
 
     if let Some(ref desc) = c.description {
@@ -455,12 +455,31 @@ fn build_contract_context(contract: &crate::db::models::ContractWithRelations) -
     }).collect();
 
     let has_repository = !contract.repositories.is_empty();
-    let phase_checklist = get_phase_checklist(&c.phase, &file_infos, &task_infos, has_repository);
+    let phase_checklist = crate::llm::get_phase_checklist_for_type(&c.phase, &file_infos, &task_infos, has_repository, &c.contract_type);
 
     // Add phase checklist to context
     context.push_str("\n");
     context.push_str(&format_checklist_markdown(&phase_checklist));
 
+    // Add deliverable check result for phase transition readiness
+    // Note: pr_url is not available in TaskSummary, so we pass None here
+    // Full PR checking should be done via the check_deliverables_met tool
+    let deliverable_check = crate::llm::check_deliverables_met(
+        &c.phase,
+        &c.contract_type,
+        &file_infos,
+        &task_infos,
+        has_repository,
+        None, // pr_url not available in TaskSummary
+    );
+
+    // Add deliverable prompt guidance
+    context.push_str(&crate::llm::generate_deliverable_prompt_guidance(
+        &c.phase,
+        &c.contract_type,
+        &deliverable_check,
+    ));
+
     // Files summary
     context.push_str(&format!("\n### Files ({} total)\n", contract.files.len()));
     if !contract.files.is_empty() {
@@ -1732,6 +1751,65 @@ async fn handle_contract_request(
                 };
             }
 
+            // Check if deliverables are met before allowing transition
+            let cwr = match get_contract_with_relations(pool, contract_id, owner_id).await {
+                Ok(Some(c)) => c,
+                Ok(None) | Err(_) => {
+                    // Fall through - we'll just skip the deliverables check
+                    return ContractRequestResult {
+                        success: false,
+                        message: "Failed to load contract for deliverables check".to_string(),
+                        data: None,
+                    };
+                }
+            };
+
+            let file_infos: Vec<FileInfo> = cwr.files.iter().map(|f| FileInfo {
+                id: f.id,
+                name: f.name.clone(),
+                contract_phase: f.contract_phase.clone(),
+            }).collect();
+
+            let task_infos: Vec<TaskInfo> = cwr.tasks.iter().map(|t| TaskInfo {
+                id: t.id,
+                name: t.name.clone(),
+                status: t.status.clone(),
+            }).collect();
+
+            let has_repository = !cwr.repositories.is_empty();
+            // Note: pr_url is not available in TaskSummary, so we skip PR checking here
+            // For simple contracts, the PR deliverable check will need to be done
+            // by fetching full task details if needed
+
+            let check_result = crate::llm::check_deliverables_met(
+                current_phase,
+                &contract.contract_type,
+                &file_infos,
+                &task_infos,
+                has_repository,
+                None, // pr_url not available in TaskSummary
+            );
+
+            // Block transition if deliverables are not met
+            if !check_result.deliverables_met {
+                return ContractRequestResult {
+                    success: false,
+                    message: format!(
+                        "Cannot advance to '{}' phase: deliverables not met. {}",
+                        new_phase, check_result.summary
+                    ),
+                    data: Some(json!({
+                        "status": "deliverables_not_met",
+                        "currentPhase": current_phase,
+                        "requestedPhase": new_phase,
+                        "deliverablesMet": false,
+                        "requiredDeliverables": check_result.required_deliverables,
+                        "missing": check_result.missing,
+                        "action": "Complete the missing deliverables before advancing to the next phase"
+                    })),
+                };
+            }
+
             // Check if phase_guard is enabled
             if contract.phase_guard {
                 // If user provided feedback, return it for the task to address
@@ -1816,8 +1894,8 @@ async fn handle_contract_request(
             // Update phase (either phase_guard is disabled, or user confirmed)
             match repository::change_contract_phase_for_owner(pool, contract_id, owner_id, &new_phase).await {
                 Ok(Some(updated)) => {
-                    // Get deliverables for the new phase
-                    let deliverables = crate::llm::get_phase_deliverables(&new_phase);
+                    // Get deliverables for the new phase (using contract type)
+                    let deliverables = crate::llm::get_phase_deliverables_for_type(&new_phase, &contract.contract_type);
 
                     // Build suggested files list
                     let suggested_files: Vec<serde_json::Value> = deliverables
@@ -1963,7 +2041,7 @@ async fn handle_contract_request(
                     }).collect();
 
                     let has_repository = !cwr.repositories.is_empty();
-                    let checklist = get_phase_checklist(&cwr.contract.phase, &file_infos, &task_infos, has_repository);
+                    let checklist = crate::llm::get_phase_checklist_for_type(&cwr.contract.phase, &file_infos, &task_infos, has_repository, &cwr.contract.contract_type);
 
                     ContractRequestResult {
                         success: true,
@@ -1993,6 +2071,82 @@ async fn handle_contract_request(
             }
         }
 
+        ContractToolRequest::CheckDeliverablesMet => {
+            match get_contract_with_relations(pool, contract_id, owner_id).await {
+                Ok(Some(cwr)) => {
+                    let file_infos: Vec<FileInfo> = cwr.files.iter().map(|f| FileInfo {
+                        id: f.id,
+                        name: f.name.clone(),
+                        contract_phase: f.contract_phase.clone(),
+                    }).collect();
+
+                    let task_infos: Vec<TaskInfo> = cwr.tasks.iter().map(|t| TaskInfo {
+                        id: t.id,
+                        name: t.name.clone(),
+                        status: t.status.clone(),
+                    }).collect();
+
+                    let has_repository = !cwr.repositories.is_empty();
+
+                    // Note: pr_url is not available in TaskSummary
+                    // For simple contracts needing PR checking, full task details would need to be fetched
+                    // For now, we pass None and the LLM can guide the user to ensure a PR exists
+
+                    let check_result = crate::llm::check_deliverables_met(
+                        &cwr.contract.phase,
+                        &cwr.contract.contract_type,
+                        &file_infos,
+                        &task_infos,
+                        has_repository,
+                        None, // pr_url not available in TaskSummary
+                    );
+
+                    // Check if we should auto-progress
+                    let auto_progress = crate::llm::should_auto_progress(
+                        &cwr.contract.phase,
+                        &cwr.contract.contract_type,
+                        &file_infos,
+                        &task_infos,
+                        has_repository,
+                        None, // pr_url not available in TaskSummary
+                        cwr.contract.autonomous_loop,
+                    );
+
+                    ContractRequestResult {
+                        success: true,
+                        message: check_result.summary.clone(),
+                        data: Some(json!({
+                            "deliverablesMet": check_result.deliverables_met,
+                            "readyToAdvance": check_result.ready_to_advance,
+                            "phase": check_result.phase,
+                            "nextPhase": check_result.next_phase,
+                            "requiredDeliverables": check_result.required_deliverables,
+                            "missing": check_result.missing,
+                            "summary": check_result.summary,
+                            "autoProgressRecommended": check_result.auto_progress_recommended,
+                            "autoProgress": {
+                                "shouldProgress": auto_progress.should_progress,
+                                "nextPhase": auto_progress.next_phase,
+                                "reason": auto_progress.reason,
+                                "action": format!("{:?}", auto_progress.action),
+                            },
+                            "autonomousLoop": cwr.contract.autonomous_loop,
+                        })),
+                    }
+                }
+                Ok(None) => ContractRequestResult {
+                    success: false,
+                    message: "Contract not found".to_string(),
+                    data: None,
+                },
+                Err(e) => ContractRequestResult {
+                    success: false,
+                    message: format!("Database error: {}", e),
+                    data: None,
+                },
+            }
+        }
+
         // =============================================================================
         // Task Derivation Handlers
         // =============================================================================
diff --git a/makima/src/server/handlers/contract_daemon.rs b/makima/src/server/handlers/contract_daemon.rs
index 13c5640..5b23831 100644
--- a/makima/src/server/handlers/contract_daemon.rs
+++ b/makima/src/server/handlers/contract_daemon.rs
@@ -280,7 +280,7 @@ pub async fn get_contract_checklist(
         Err(_) => false,
     };
 
-    let checklist = phase_guidance::get_phase_checklist(&contract.phase, &files, &tasks, has_repository);
+    let checklist = phase_guidance::get_phase_checklist_for_type(&contract.phase, &files, &tasks, has_repository, &contract.contract_type);
 
     Json(checklist).into_response()
 }
@@ -319,7 +319,7 @@ pub async fn get_contract_goals(
 
     match repository::get_contract_for_owner(pool, id, auth.owner_id).await {
         Ok(Some(contract)) => {
-            let deliverables = phase_guidance::get_phase_deliverables(&contract.phase);
+            let deliverables = phase_guidance::get_phase_deliverables_for_type(&contract.phase, &contract.contract_type);
             Json(ContractGoalsResponse {
                 description: contract.description,
                 phase: contract.phase,
@@ -491,7 +491,7 @@ pub async fn get_suggest_action(
         .map(|r| !r.is_empty())
         .unwrap_or(false);
 
-    let checklist = phase_guidance::get_phase_checklist(&contract.phase, &files, &tasks, has_repository);
+    let checklist = phase_guidance::get_phase_checklist_for_type(&contract.phase, &files, &tasks, has_repository, &contract.contract_type);
 
     // Determine suggested action based on checklist
     let (action, description) = if !checklist.suggestions.is_empty() {
diff --git a/makima/src/server/handlers/contracts.rs b/makima/src/server/handlers/contracts.rs
index 462b385..f16f33d 100644
--- a/makima/src/server/handlers/contracts.rs
+++ b/makima/src/server/handlers/contracts.rs
@@ -612,6 +612,9 @@ pub async fn delete_contract(
         }
     }
 
+    // Clean up any pending supervisor questions for this contract
+    state.remove_pending_questions_for_contract(id);
+
     // Clean up all task worktrees BEFORE deleting the contract
     // (because CASCADE delete will remove tasks from DB)
     cleanup_contract_worktrees(pool, &state, id).await;
diff --git a/makima/src/server/handlers/mesh.rs b/makima/src/server/handlers/mesh.rs
index 3d05f35..3d64eb4 100644
--- a/makima/src/server/handlers/mesh.rs
+++ b/makima/src/server/handlers/mesh.rs
@@ -482,6 +482,9 @@ pub async fn delete_task(
         }
     }
 
+    // Clean up any pending supervisor questions for this task
+    state.remove_pending_questions_for_task(id);
+
     match repository::delete_task_for_owner(pool, id, auth.owner_id).await {
         Ok(true) => StatusCode::NO_CONTENT.into_response(),
         Ok(false) => (
diff --git a/makima/src/server/state.rs b/makima/src/server/state.rs
index 5b75281..32c0af3 100644
--- a/makima/src/server/state.rs
+++ b/makima/src/server/state.rs
@@ -797,6 +797,70 @@ impl AppState {
         self.question_responses.remove(&question_id);
     }
 
+    /// Remove all pending questions for a specific task.
+    ///
+    /// This should be called when a task is deleted to clean up orphaned questions.
+    /// Returns the number of questions removed.
+    pub fn remove_pending_questions_for_task(&self, task_id: Uuid) -> usize {
+        // Collect question IDs to remove
+        let question_ids: Vec<Uuid> = self
+            .pending_questions
+            .iter()
+            .filter(|entry| entry.value().task_id == task_id)
+            .map(|entry| entry.value().question_id)
+            .collect();
+
+        let count = question_ids.len();
+
+        // Remove pending questions and their responses
+        for question_id in question_ids {
+            self.pending_questions.remove(&question_id);
+            self.question_responses.remove(&question_id);
+        }
+
+        if count > 0 {
+            tracing::info!(
+                task_id = %task_id,
+                count = count,
+                "Cleaned up pending questions for deleted task"
+            );
+        }
+
+        count
+    }
+
+    /// Remove all pending questions for a specific contract.
+    ///
+    /// This should be called when a contract is deleted to clean up orphaned questions.
+    /// Returns the number of questions removed.
+    pub fn remove_pending_questions_for_contract(&self, contract_id: Uuid) -> usize {
+        // Collect question IDs to remove
+        let question_ids: Vec<Uuid> = self
+            .pending_questions
+            .iter()
+            .filter(|entry| entry.value().contract_id == contract_id)
+            .map(|entry| entry.value().question_id)
+            .collect();
+
+        let count = question_ids.len();
+
+        // Remove pending questions and their responses
+        for question_id in question_ids {
+            self.pending_questions.remove(&question_id);
+            self.question_responses.remove(&question_id);
+        }
+
+        if count > 0 {
+            tracing::info!(
+                contract_id = %contract_id,
+                count = count,
+                "Cleaned up pending questions for deleted contract"
+            );
+        }
+
+        count
+    }
+
     /// Register a new daemon connection.
     ///
     /// Returns the connection_id for later reference.