diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 61026fea..a7713d6f 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -2713,7 +2713,7 @@ struct CurrentWorkerWorkdirAttachmentResponse { attached: bool, } -#[derive(Debug, Deserialize)] +#[derive(Debug, Clone, Deserialize)] struct ScopedRecordPath { workspace_id: String, id: String, @@ -3698,9 +3698,7 @@ struct BrowserAppendTicketEventRequest { #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] -struct BrowserQueueTicketRequest { - queued_by: Option, -} +struct BrowserQueueTicketRequest {} #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] @@ -3829,6 +3827,15 @@ async fn scoped_transition_ticket_state( ).into()); } let current = api.authority.ticket(&path.id)?; + if request.state == TicketWorkflowState::InProgress + && current.state != TicketWorkflowState::InProgress.as_str() + { + return Err(Error::TicketAssignmentConflict( + "generic Ticket state mutation cannot enter inprogress; use Queue acceptance or atomic ready-state Coder assignment" + .to_string(), + ) + .into()); + } let mut change = TicketStateChange::new( current.state, request.state.as_str(), @@ -3881,22 +3888,20 @@ async fn scoped_mark_ticket_ready_from_browser( async fn scoped_queue_ticket( State(api): State, AxumPath(path): AxumPath, - Json(request): Json, + Json(_request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let queued_by = request.queued_by.as_deref().unwrap_or("web"); - browser_ticket_backend(&api)? - .queue_ready(TicketIdOrSlug::Id(path.id.clone()), queued_by) - .map_err(Error::from)?; - let Json(ticket) = browser_ticket_detail(&api, &path.id)?; - notify_ticket_recipients( + let _ = execute_ticket_rest_operation( &api, &path.workspace_id, - &path.id, - TicketWorkflowState::Ready.as_str(), - ticket.state.as_str(), - None, - ); + HeaderMap::new(), + TicketBackendOperation::QueueReady { + id: TicketIdOrSlug::Id(path.id.clone()), + queued_by: "workspace-web".to_string(), + }, + ) + .await?; + let Json(ticket) = browser_ticket_detail(&api, &path.id)?; Ok(Json(ticket)) } @@ -15949,6 +15954,68 @@ mod tests { assert_eq!(invalid_source.status(), StatusCode::BAD_REQUEST); } + #[tokio::test] + async fn generic_browser_state_mutation_cannot_bypass_assignment_start() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api(dir.path()).await; + let backend = browser_ticket_backend(&api).unwrap(); + let ticket = backend + .create(ticket::NewTicket::new("Generic transition guard")) + .unwrap(); + let path = ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + id: ticket.id.clone(), + }; + + let planning = scoped_transition_ticket_state( + State(api.clone()), + AxumPath(path.clone()), + Json(BrowserTransitionTicketStateRequest { + state: TicketWorkflowState::InProgress, + reason: None, + body: None, + author: None, + }), + ) + .await + .unwrap_err() + .into_response(); + assert_eq!(planning.status(), StatusCode::CONFLICT); + assert_eq!( + backend + .show(ticket.id.clone().into()) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Planning + ); + + let mut ready_input = ticket::NewTicket::new("Ready transition guard"); + ready_input.workflow_state = Some(TicketWorkflowState::Ready); + let ready = backend.create(ready_input).unwrap(); + let ready_result = scoped_transition_ticket_state( + State(api.clone()), + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + id: ready.id.clone(), + }), + Json(BrowserTransitionTicketStateRequest { + state: TicketWorkflowState::InProgress, + reason: None, + body: None, + author: None, + }), + ) + .await + .unwrap_err() + .into_response(); + assert_eq!(ready_result.status(), StatusCode::CONFLICT); + assert_eq!( + backend.show(ready.id.into()).unwrap().meta.workflow_state, + TicketWorkflowState::Ready + ); + } + #[tokio::test] async fn queue_requires_orchestrator_role_and_records_assignment_fence() { let dir = tempfile::tempdir().unwrap(); @@ -15962,6 +16029,19 @@ mod tests { let ticket = backend.create(input).unwrap(); let path = (TEST_WORKSPACE_ID.to_string(), ticket.id.clone()); + let legacy_missing = scoped_queue_ticket( + State(api.clone()), + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + id: ticket.id.clone(), + }), + Json(BrowserQueueTicketRequest {}), + ) + .await + .unwrap_err() + .into_response(); + assert_eq!(legacy_missing.status(), StatusCode::CONFLICT); + let missing = scoped_queue_ticket_record( State(api.clone()), AxumPath(path.clone()), @@ -16623,18 +16703,17 @@ mod tests { .await .unwrap(); assert_eq!(ready.state, "ready"); + assign_test_orchestrator(&api, &ticket_id); let Json(queued) = scoped_queue_ticket( State(api.clone()), AxumPath(path()), - Json(BrowserQueueTicketRequest { - queued_by: Some("browser-user".to_string()), - }), + Json(BrowserQueueTicketRequest {}), ) .await .unwrap(); assert_eq!(queued.state, "queued"); - assert_eq!(queued.queued_by.as_deref(), Some("browser-user")); + assert_eq!(queued.queued_by.as_deref(), Some("workspace-web")); let Json(closed) = scoped_close_ticket( State(api), AxumPath(path()), diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index aeaea00b..bb7b4022 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -3648,6 +3648,9 @@ impl ControlPlaneStore for SqliteWorkspaceStore { }; self.with_conn_mut(|conn| { let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + if let Some(worker) = record.principal.worker() { + ensure_worker_assignment_available(&tx, &record.workspace_id, &worker)?; + } let existing_operation: Option<(String, Option)> = tx .query_row( @@ -3866,6 +3869,9 @@ impl ControlPlaneStore for SqliteWorkspaceStore { self.with_conn_mut(|conn| { let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + if let Some(worker) = record.principal.worker() { + ensure_worker_assignment_available(&tx, &record.workspace_id, &worker)?; + } let existing_operation: Option<(String, Option)> = tx .query_row( "SELECT request_fingerprint, assignment_id FROM ticket_assignment_operations @@ -5374,6 +5380,32 @@ fn read_worker_control_grant_by_operation( .map_err(Error::from) } +fn ensure_worker_assignment_available( + conn: &Connection, + workspace_id: &str, + worker: &RuntimeWorkerRef, +) -> Result<()> { + let removal_blocks_assignment: bool = conn.query_row( + "SELECT EXISTS( + SELECT 1 FROM worker_removal_operations + WHERE workspace_id = ?1 AND runtime_id = ?2 AND worker_id = ?3 + AND state IN ('executing', 'failed', 'succeeded') + UNION ALL + SELECT 1 FROM worker_tombstones + WHERE workspace_id = ?1 AND runtime_id = ?2 AND worker_id = ?3 + )", + params![workspace_id, worker.runtime_id, worker.worker_id], + |row| row.get(0), + )?; + if removal_blocks_assignment { + return Err(Error::TicketAssignmentConflict(format!( + "Worker {}/{} is being retained or has been removed", + worker.runtime_id, worker.worker_id + ))); + } + Ok(()) +} + fn read_ticket_role_assignment_record( row: &rusqlite::Row<'_>, ) -> rusqlite::Result { @@ -10630,7 +10662,51 @@ INSERT INTO worker_registry ( TicketAssignmentRole::Coder, ) .unwrap(), - Some(coder) + Some(coder.clone()) + ); + assert!( + store + .delete_worker_registry("workspace-role", &worker.worker) + .is_err(), + "active role assignment must prevent Worker removal" + ); + assert!( + store + .clear_current_ticket_role_assignment( + "workspace-role", + &ticket.meta.id, + TicketAssignmentRole::Coder, + "coder-manual-1", + "event-clear-coder", + "op-clear-coder", + "user", + "2026-09-01T00:03:00Z", + Some("test removal guard"), + ) + .unwrap() + ); + assert!( + store + .delete_worker_registry("workspace-role", &worker.worker) + .unwrap() + ); + let removed_worker_assignment = TicketRoleAssignmentRecord { + assignment_id: "contributor-removed-worker".to_string(), + role: TicketAssignmentRole::Contributor, + assigned_at: "2026-09-01T00:04:00Z".to_string(), + ..coder + }; + assert!( + store + .set_current_ticket_role_assignment( + &removed_worker_assignment, + None, + "event-removed-worker", + "op-removed-worker", + false, + ) + .is_err(), + "removed/tombstoned Worker cannot become a new role principal" ); }