diff options
| author | soryu <soryu@soryu.co> | 2026-01-31 22:53:28 +0000 |
|---|---|---|
| committer | soryu <soryu@soryu.co> | 2026-01-31 22:54:50 +0000 |
| commit | 44bb3fe07ab191abd8260af6975bc175c223878e (patch) | |
| tree | 1d7dd73756345f3671af32cc84b9b4235d34d173 /makima/src/db/repository.rs | |
| parent | a6e36a8bfecb9ebe6c7b135b9e01557f7ebc3e58 (diff) | |
| download | soryu-44bb3fe07ab191abd8260af6975bc175c223878e.tar.gz soryu-44bb3fe07ab191abd8260af6975bc175c223878e.zip | |
feat: Add contract management system improvements (Phase 1)makima/contract-management-improvements
- Add docs/contract-management-spec.md with full system design
- Add docs/plans/implementation-plan.md with 5-phase rollout plan
- Add validate_deliverable() function and use in mark_deliverable_complete
- Add PhaseChangeResult enum and change_contract_phase_with_version() with FOR UPDATE locking
- Enforce phase_guard at API level for all callers
This addresses critical issues in contract management:
- Deliverable validation to prevent marking non-existent deliverables complete
- Version conflict detection for phase changes with row locking
- Phase guard enforcement at API level (applies to all callers including supervisors)
- Comprehensive specification and implementation plan for future phases
Diffstat (limited to 'makima/src/db/repository.rs')
| -rw-r--r-- | makima/src/db/repository.rs | 107 |
1 files changed, 104 insertions, 3 deletions
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 // ============================================================================= |
