Merge pull request #52 from soryu-co/makima/contract-management-improvements

feat: Add contract management system improvements (Phase 1)

2 files changed, 135 insertions, 3 deletions

diff --git a/makima/src/db/models.rs b/makima/src/db/models.rs
index a6b5b05..636d81a 100644
--- a/makima/src/db/models.rs
+++ b/makima/src/db/models.rs
@@ -1808,6 +1808,37 @@ pub struct ChangePhaseRequest {
     /// User feedback for changes (used when not confirming)
     #[serde(skip_serializing_if = "Option::is_none")]
     pub feedback: Option<String>,
+    /// Expected version for optimistic locking. If provided, the phase change
+    /// will only succeed if the current contract version matches.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub expected_version: Option<i32>,
+}
+
+/// Result of a phase change operation, supporting explicit conflict detection.
+#[derive(Debug, Clone)]
+pub enum PhaseChangeResult {
+    /// Phase change succeeded, returning the updated contract
+    Success(Contract),
+    /// Version conflict: the contract was modified concurrently
+    VersionConflict {
+        /// The version the client expected
+        expected: i32,
+        /// The actual current version in the database
+        actual: i32,
+        /// The current phase of the contract
+        current_phase: String,
+    },
+    /// Validation failed (e.g., invalid phase transition)
+    ValidationFailed {
+        /// Human-readable reason for the failure
+        reason: String,
+        /// List of missing requirements for the phase transition
+        missing_requirements: Vec<String>,
+    },
+    /// The caller is not authorized to change this contract's phase
+    Unauthorized,
+    /// The contract was not found
+    NotFound,
 }
 
 /// Response for phase transition when phase_guard is enabled
diff --git a/makima/src/db/repository.rs b/makima/src/db/repository.rs
index 8c7ea23..b7c5af1 100644
--- a/makima/src/db/repository.rs
+++ b/makima/src/db/repository.rs
@@ -11,9 +11,10 @@ use super::models::{
     ContractTypeTemplateRecord, ConversationMessage, ConversationSnapshot, CreateContractRequest,
     CreateFileRequest, CreateTaskRequest, CreateTemplateRequest, Daemon, DaemonTaskAssignment,
     DaemonWithCapacity, DeliverableDefinition, File, FileSummary, FileVersion, HistoryEvent,
-    HistoryQueryFilters, MeshChatConversation, MeshChatMessageRecord, PhaseConfig, PhaseDefinition,
-    RedTeamNotification, SupervisorState, Task, TaskCheckpoint, TaskEvent, TaskSummary,
-    UpdateContractRequest, UpdateFileRequest, UpdateTaskRequest, UpdateTemplateRequest,
+    HistoryQueryFilters, MeshChatConversation, MeshChatMessageRecord, PhaseChangeResult,
+    PhaseConfig, PhaseDefinition, RedTeamNotification, SupervisorState, Task, TaskCheckpoint,
+    TaskEvent, TaskSummary, UpdateContractRequest, UpdateFileRequest, UpdateTaskRequest,
+    UpdateTemplateRequest,
 };
 
 /// Repository error types.
@@ -2676,6 +2677,9 @@ pub async fn delete_contract_for_owner(
 }
 
 /// Change contract phase and record event.
+///
+/// This is the simple version without version checking. Use `change_contract_phase_with_version`
+/// for explicit version conflict detection.
 pub async fn change_contract_phase_for_owner(
     pool: &PgPool,
     id: Uuid,
@@ -2723,6 +2727,103 @@ pub async fn change_contract_phase_for_owner(
     Ok(contract)
 }
 
+/// Change contract phase with explicit version checking for conflict detection.
+///
+/// Uses `SELECT ... FOR UPDATE` to lock the row and prevent race conditions.
+/// Returns `PhaseChangeResult::VersionConflict` if the expected version doesn't match.
+pub async fn change_contract_phase_with_version(
+    pool: &PgPool,
+    id: Uuid,
+    owner_id: Uuid,
+    new_phase: &str,
+    expected_version: Option<i32>,
+) -> Result<PhaseChangeResult, sqlx::Error> {
+    // Start a transaction to ensure atomicity with row locking
+    let mut tx = pool.begin().await?;
+
+    // Lock the row with SELECT FOR UPDATE and get current state
+    let existing: Option<Contract> = sqlx::query_as::<_, Contract>(
+        r#"
+        SELECT *
+        FROM contracts
+        WHERE id = $1 AND owner_id = $2
+        FOR UPDATE
+        "#,
+    )
+    .bind(id)
+    .bind(owner_id)
+    .fetch_optional(&mut *tx)
+    .await?;
+
+    let Some(existing) = existing else {
+        tx.rollback().await?;
+        return Ok(PhaseChangeResult::NotFound);
+    };
+
+    // Check version if provided (optimistic locking)
+    if let Some(expected) = expected_version {
+        if existing.version != expected {
+            tx.rollback().await?;
+            return Ok(PhaseChangeResult::VersionConflict {
+                expected,
+                actual: existing.version,
+                current_phase: existing.phase,
+            });
+        }
+    }
+
+    // Validate the phase transition is allowed
+    let valid_phases = existing.valid_phase_ids();
+    if !valid_phases.contains(&new_phase.to_string()) {
+        tx.rollback().await?;
+        return Ok(PhaseChangeResult::ValidationFailed {
+            reason: format!(
+                "Invalid phase '{}' for contract type '{}'",
+                new_phase, existing.contract_type
+            ),
+            missing_requirements: vec![format!(
+                "Phase must be one of: {}",
+                valid_phases.join(", ")
+            )],
+        });
+    }
+
+    let previous_phase = existing.phase.clone();
+
+    // Update phase with version increment
+    let contract = sqlx::query_as::<_, Contract>(
+        r#"
+        UPDATE contracts
+        SET phase = $3, version = version + 1, updated_at = NOW()
+        WHERE id = $1 AND owner_id = $2
+        RETURNING *
+        "#,
+    )
+    .bind(id)
+    .bind(owner_id)
+    .bind(new_phase)
+    .fetch_one(&mut *tx)
+    .await?;
+
+    // Record event
+    sqlx::query(
+        r#"
+        INSERT INTO contract_events (contract_id, event_type, previous_phase, new_phase)
+        VALUES ($1, 'phase_change', $2, $3)
+        "#,
+    )
+    .bind(id)
+    .bind(&previous_phase)
+    .bind(new_phase)
+    .execute(&mut *tx)
+    .await?;
+
+    // Commit the transaction
+    tx.commit().await?;
+
+    Ok(PhaseChangeResult::Success(contract))
+}
+
 // =============================================================================
 // Contract Repository Functions
 // =============================================================================