From 1fb29495615d4def4a01fa0ec9159c100e7e8763 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 04:45:17 +0900 Subject: [PATCH 1/4] fix: give reviewer write-scoped command tools --- crates/workdir/src/delegation.rs | 43 +++++++++++++++++++++++++++++- crates/worker/src/spawn/tool.rs | 35 ++++++++++++------------ docs/development/work-items.md | 2 +- resources/flows/coder-review.dcdl | 4 +-- resources/prompts/role/coder.md | 2 +- resources/prompts/role/reviewer.md | 2 +- 6 files changed, 64 insertions(+), 24 deletions(-) diff --git a/crates/workdir/src/delegation.rs b/crates/workdir/src/delegation.rs index f16f86ee..4a5b313f 100644 --- a/crates/workdir/src/delegation.rs +++ b/crates/workdir/src/delegation.rs @@ -311,7 +311,10 @@ impl DelegatingWorkdirSession { if !self.capabilities.supports(WorkdirSessionCapability::Read) || (writable && (!self.capabilities.supports(WorkdirSessionCapability::Write) - || !self.capabilities.supports(WorkdirSessionCapability::Edit))) + || !self.capabilities.supports(WorkdirSessionCapability::Edit) + || !self + .capabilities + .supports(WorkdirSessionCapability::Command))) { return Err(WorkdirError::Denied( "parent workdir session cannot delegate the requested capabilities".into(), @@ -342,6 +345,7 @@ impl DelegatingWorkdirSession { if writable { delegated.push(WorkdirSessionCapability::Write); delegated.push(WorkdirSessionCapability::Edit); + delegated.push(WorkdirSessionCapability::Command); } Ok(WorkdirSessionCapabilities::from_capabilities(delegated)) } @@ -929,6 +933,43 @@ mod tests { .delegate(request("leased", WorkdirDelegationPermission::Write)) .await .unwrap(); + assert!( + child + .capabilities + .supports(WorkdirSessionCapability::Command) + ); + let command = child + .scoped_session + .start_command(CommandRequest { + command: "printf child-command".into(), + timeout_secs: 5, + output_limit: 1024, + tool_call_id: Some("delegated-child-command".into()), + }) + .await + .unwrap(); + let command_output = child + .scoped_session + .command_output(CommandOutputRequest { + handle: command, + cursor: 0, + limit: 1024, + wait: true, + }) + .await + .unwrap(); + assert_eq!(command_output.content, "child-command"); + assert!( + parent + .start_command(CommandRequest { + command: "printf parent-command".into(), + timeout_secs: 5, + output_limit: 1024, + tool_call_id: Some("blocked-parent-command".into()), + }) + .await + .is_err() + ); assert!(matches!( parent.write(write("leased/file", "parent")).await, diff --git a/crates/worker/src/spawn/tool.rs b/crates/worker/src/spawn/tool.rs index 842160e9..3d2d8782 100644 --- a/crates/worker/src/spawn/tool.rs +++ b/crates/worker/src/spawn/tool.rs @@ -329,13 +329,13 @@ fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolErro "reviewer handoff requires the explicit effective profile builtin:reviewer".to_string(), )); } - if input + if !input .scope .iter() .any(|rule| matches!(rule.permission, PermissionInput::Write)) { return Err(ToolError::InvalidArgument( - "Merge Request Reviewer SubWorkers must have read-only delegated scope".to_string(), + "Merge Request Reviewer SubWorkers must include writable delegated scope".to_string(), )); } Ok(()) @@ -1008,28 +1008,28 @@ mod tests { } #[test] - fn reviewer_handoff_requires_explicit_builtin_profile_and_read_only_scope() { + fn reviewer_handoff_requires_explicit_builtin_profile_and_writable_scope() { let valid: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({ "name":"reviewer","task":"review","profile":"builtin:reviewer", - "scope":[{"target":"work","permission":"read"}], + "scope":[{"target":"work","permission":"write"}], "review":{"ticket_id":"T1"} })) .unwrap(); assert!(validate_reviewer_handoff(&valid).is_ok()); let wrong_profile: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({ "name":"reviewer","task":"review","profile":"builtin:coder", - "scope":[{"target":"work","permission":"read"}], - "review":{"ticket_id":"T1"} - })) - .unwrap(); - assert!(validate_reviewer_handoff(&wrong_profile).is_err()); - let writable: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({ - "name":"reviewer","task":"review","profile":"builtin:reviewer", "scope":[{"target":"work","permission":"write"}], "review":{"ticket_id":"T1"} })) .unwrap(); - assert!(validate_reviewer_handoff(&writable).is_err()); + assert!(validate_reviewer_handoff(&wrong_profile).is_err()); + let read_only: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({ + "name":"reviewer","task":"review","profile":"builtin:reviewer", + "scope":[{"target":"work","permission":"read"}], + "review":{"ticket_id":"T1"} + })) + .unwrap(); + assert!(validate_reviewer_handoff(&read_only).is_err()); } fn abs_rule(path: &Path, permission: Permission) -> ScopeRule { @@ -1079,7 +1079,7 @@ extract_threshold = 4000 } #[tokio::test] - async fn reviewer_profile_spawns_and_notifies_parent_controller() { + async fn reviewer_profile_write_scope_exposes_command_tools_and_notifies_parent_controller() { let runtime = TempDir::new().unwrap(); let workspace_root = runtime.path().join("project"); let available_profiles = write_project_profile_registry( @@ -1140,7 +1140,7 @@ extract_threshold = 4000 "task": "review immutable commit", "scope": [{ "target": ".", - "permission": "read", + "permission": "write", "recursive": true }] }); @@ -1171,11 +1171,10 @@ extract_threshold = 4000 let record = registry .get_internal("reviewer-child") .expect("Internal reviewer registry record"); - assert!(record.installed_tools.iter().any(|name| name == "Read")); - for denied in ["Write", "Edit", "Bash"] { + for required in ["Read", "Write", "Edit", "Glob", "Grep", "Bash"] { assert!( - !record.installed_tools.iter().any(|name| name == denied), - "read-only child unexpectedly received {denied}: {:?}", + record.installed_tools.iter().any(|name| name == required), + "write-scoped child is missing {required}: {:?}", record.installed_tools ); } diff --git a/docs/development/work-items.md b/docs/development/work-items.md index ebd29c50..9f2f7926 100644 --- a/docs/development/work-items.md +++ b/docs/development/work-items.md @@ -242,7 +242,7 @@ Implementation normally happens in a child git worktree created by the Orchestra ### 5. Review -The assigned Coder launches the Reviewer as an actual direct-child `builtin:reviewer` SubWorker with read-only scope and a structured handoff bound to the current immutable Merge Request revision. Server authority revalidates the parent assignment, Runtime-owned child session, effective profile, one-shot review attempt, and revision; prose output is not approval. +The assigned Coder launches the Reviewer as an actual direct-child `builtin:reviewer` SubWorker with write scope, so it can use the Workdir command tools required for inspection and validation, and a structured handoff bound to the current immutable Merge Request revision. Server authority revalidates the parent assignment, Runtime-owned child session, effective profile, one-shot review attempt, and revision; prose output is not approval. The Reviewer records the structured result with `MergeRequestReview`. Request changes requires a new immutable revision and a fresh child attempt. The Orchestrator uses `MergeRequestReadinessCheck` and then `MergeRequestComplete` for guarded integration with operation-id dedupe/CAS semantics; Flow transitions are not completion authority. diff --git a/resources/flows/coder-review.dcdl b/resources/flows/coder-review.dcdl index e90b63df..9e4c2814 100644 --- a/resources/flows/coder-review.dcdl +++ b/resources/flows/coder-review.dcdl @@ -15,7 +15,7 @@ }; review = { - instructions = "Use the current Ticket Merge Request as review authority. Confirm its immutable source selector resolves to the exact committed implementation HEAD, then spawn one actual direct-child SubWorker with profile builtin:reviewer, read-only scope, and a structured review handoff bound to the current immutable Merge Request revision. The trusted spawn layer records `ReviewRequested`; do not place commit/ref identity, capability material, or a prewritten verdict in model input. The child must commit MergeRequestReview; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition."; + instructions = "Use the current Ticket Merge Request as review authority. Confirm its immutable source selector resolves to the exact committed implementation HEAD, then spawn one actual direct-child SubWorker with profile builtin:reviewer, write scope for Workdir inspection and command validation, and a structured review handoff bound to the current immutable Merge Request revision. The trusted spawn layer records `ReviewRequested`; do not place commit/ref identity, capability material, or a prewritten verdict in model input. The child must commit MergeRequestReview; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition."; transitions = { approved = { target = "complete"; @@ -29,7 +29,7 @@ }; fix = { - instructions = "Resolve every open Reviewer finding on the same Ticket work branch, rerun the validation affected by the fixes, commit the corrected implementation as a new revision, and preserve concrete evidence. Publish only the updated Ticket work branch with a normal non-force push, verify that the configured repository provider resolves the published source ref to the exact new HEAD, and update the linked Merge Request so its current revision records that same subject. Request review from a fresh read-only Reviewer child so the trusted spawn layer captures the new immutable subject. Do not rewrite the previously reviewed commit, claim approval from the prior request_changes review, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected committed revision is published and ready for a new independent review."; + instructions = "Resolve every open Reviewer finding on the same Ticket work branch, rerun the validation affected by the fixes, commit the corrected implementation as a new revision, and preserve concrete evidence. Publish only the updated Ticket work branch with a normal non-force push, verify that the configured repository provider resolves the published source ref to the exact new HEAD, and update the linked Merge Request so its current revision records that same subject. Request review from a fresh Reviewer child with write scope so it can use the Workdir command tools required for inspection and validation while the trusted spawn layer captures the new immutable subject. Do not rewrite the previously reviewed commit, claim approval from the prior request_changes review, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected committed revision is published and ready for a new independent review."; transitions = { review = { target = "review"; diff --git a/resources/prompts/role/coder.md b/resources/prompts/role/coder.md index b8e2dbd3..7f4fdeaa 100644 --- a/resources/prompts/role/coder.md +++ b/resources/prompts/role/coder.md @@ -6,6 +6,6 @@ Before opening a Merge Request, publish only the committed Ticket work branch wi {% include "common.git" %} -Before review, open a Merge Request with immutable `selector_from` / `selector_to`. Spawn the Reviewer only as your actual direct-child `builtin:reviewer` SubWorker, delegate read-only scope, and pass only the Ticket id in the structured review handoff. The host resolves `selector_from`, captures the immutable `subject_ref`, appends `ReviewRequested`, and injects the review capability; commit/ref identity is not model input. Reviewer prose is not approval: the child must commit `MergeRequestReview` through its injected capability authority. +Before review, open a Merge Request with immutable `selector_from` / `selector_to`. Spawn the Reviewer only as your actual direct-child `builtin:reviewer` SubWorker, delegate write scope so it can use the Workdir command tools required for inspection and validation, and pass only the Ticket id in the structured review handoff. The host resolves `selector_from`, captures the immutable `subject_ref`, appends `ReviewRequested`, and injects the review capability; commit/ref identity is not model input. Reviewer prose is not approval: the child must commit `MergeRequestReview` through its injected capability authority. A request-changes result requires a freshly published immutable subject and a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request subject has authoritative approval, keep that source ref immutable, leave concise implementation evidence on the Ticket when useful, and hand off integration to the Orchestrator. Do not update the target selector. Do not call `MergeRequestComplete`. diff --git a/resources/prompts/role/reviewer.md b/resources/prompts/role/reviewer.md index 24677be6..c9cb1383 100644 --- a/resources/prompts/role/reviewer.md +++ b/resources/prompts/role/reviewer.md @@ -1,6 +1,6 @@ You are the Ticket Reviewer role running as an actual Runtime-owned direct child of the assigned Coder. -Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only, never as a supplied verdict. Review the host-captured `ReviewRequested.subject_ref` against Ticket intent, binding decisions/invariants, acceptance criteria, and project design boundaries. Use read-only inspection and focused validation; do not merge, close, mutate the Workdir, update a repository ref, or take over implementation. +Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only, never as a supplied verdict. Review the host-captured `ReviewRequested.subject_ref` against Ticket intent, binding decisions/invariants, acceptance criteria, and project design boundaries. Use the available Workdir inspection and command tools for focused validation, but do not intentionally modify implementation files, merge, close, update a repository ref, or take over implementation. Your prose response is not review authority. Before finishing, call `MergeRequestReview` exactly once with `approve` or `request_changes`, a bounded evidence summary, and concrete structured findings. Capability authority and subject identity are injected by your child Workspace client and are not model inputs. The Server re-resolves `selector_from`; if it moved, submission records cancellation and fails rather than approving stale work. From 87ecbcb113c579e81317d4c97a2c967b1d64d3a7 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 08:09:31 +0900 Subject: [PATCH 2/4] fix: validate non-worker ticket assignments --- crates/workspace-server/src/store.rs | 90 +++++++++++++++++++++++++--- 1 file changed, 81 insertions(+), 9 deletions(-) diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index c49960af..faf84096 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -6459,6 +6459,25 @@ fn validate_workspace_resource_references(conn: &Connection) -> Result<()> { fn workspace_resource_reference_diagnostics(conn: &Connection) -> Result> { let mut diagnostics = Vec::new(); + let current_assignment_reference_sql = + if column_exists(conn, "ticket_current_worker_assignments", "role")? + && column_exists(conn, "ticket_worker_assignments", "role")? + { + "SELECT current.workspace_id || '/' || current.assignment_id \ + FROM ticket_current_worker_assignments AS current \ + WHERE NOT EXISTS (SELECT 1 FROM ticket_worker_assignments AS assignment \ + WHERE assignment.workspace_id = current.workspace_id \ + AND assignment.ticket_id = current.ticket_id \ + AND assignment.role = current.role \ + AND assignment.assignment_id = current.assignment_id) LIMIT 100" + } else { + "SELECT current.workspace_id || '/' || current.assignment_id \ + FROM ticket_current_worker_assignments AS current \ + WHERE NOT EXISTS (SELECT 1 FROM ticket_worker_assignments AS assignment \ + WHERE assignment.workspace_id = current.workspace_id \ + AND assignment.ticket_id = current.ticket_id \ + AND assignment.assignment_id = current.assignment_id) LIMIT 100" + }; for (table, repository_nullable) in [ ("workdir_registry", false), ("artifacts", true), @@ -6531,14 +6550,7 @@ fn workspace_resource_reference_diagnostics(conn: &Connection) -> Result Date: Tue, 25 Aug 2026 09:39:33 +0900 Subject: [PATCH 3/4] fix: block cleanup for assigned workers --- crates/workspace-server/src/server.rs | 121 ++++++++++++++++++++++++-- crates/workspace-server/src/store.rs | 27 ++++++ 2 files changed, 143 insertions(+), 5 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index dc8d2b58..8be3de6f 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -7934,9 +7934,19 @@ fn build_runtime_cleanup_plan( let links = api .store .list_worker_workdir_links(&api.config.workspace_id, &record.worker)?; + let current_assignment = api.store.get_current_ticket_role_assignment_for_worker( + &api.config.workspace_id, + &record.worker, + )?; let is_running = live_running_worker_ids.contains(&record.worker); let pinned = record.retention_state == "pinned"; - let blocking_reason = if pinned { + let blocking_reason = if let Some(assignment) = current_assignment { + Some(format!( + "worker has current Ticket assignment `{}` (`{}`)", + assignment.ticket_id, + assignment.role.as_str() + )) + } else if pinned { Some("worker is pinned".to_string()) } else if is_running { Some("worker is running".to_string()) @@ -8114,6 +8124,24 @@ async fn execute_runtime_cleanup( .iter() .filter(|candidate| worker_targets.contains(candidate.target_id.as_str())) { + let worker = RuntimeWorkerRef::new( + candidate.runtime_id.clone(), + candidate.runtime_worker_id.clone(), + ); + if let Some(assignment) = api + .store + .get_current_ticket_role_assignment_for_worker(&api.config.workspace_id, &worker)? + { + return Err(cleanup_api_error( + runtime_id, + "workspace_cleanup_worker_assigned", + &format!( + "Worker is assigned to Ticket `{}` as `{}` and cannot be deleted", + assignment.ticket_id, + assignment.role.as_str() + ), + )); + } if let Some(reason) = &candidate.blocking_reason { return Err(cleanup_api_error( runtime_id, @@ -8129,10 +8157,6 @@ async fn execute_runtime_cleanup( )); } parse_runtime_worker_id_for_registry(&candidate.runtime_worker_id)?; - let worker = RuntimeWorkerRef::new( - candidate.runtime_id.clone(), - candidate.runtime_worker_id.clone(), - ); let session_lock = current_worker_session_lock(api, &worker); let _session_guard = session_lock.lock().await; close_current_worker_session_locked(api, &worker).await?; @@ -18134,6 +18158,43 @@ mod tests { runtime_worker_id.to_string() } + fn seed_cleanup_worker_assignment( + api: &WorkspaceApi, + runtime_worker_id: &str, + ticket_id: &str, + ) { + let conn = rusqlite::Connection::open(&api.config.database_path).unwrap(); + crate::store::configure_sqlite(&conn).unwrap(); + conn.execute( + "INSERT INTO typed_tickets ( + workspace_id, ticket_id, slug, title, status, kind, priority, body, + workflow_state, workflow_state_explicit + ) VALUES (?1, ?2, ?2, ?2, 'open', 'task', 'normal', '', 'inprogress', 1)", + rusqlite::params![api.config.workspace_id, ticket_id], + ) + .unwrap(); + api.store + .set_current_ticket_role_assignment( + &TicketRoleAssignmentRecord { + workspace_id: api.config.workspace_id.clone(), + ticket_id: ticket_id.to_string(), + assignment_id: format!("assignment-{ticket_id}"), + role: TicketAssignmentRole::Coder, + principal: TicketAssignmentPrincipal::Worker { + runtime_id: "runtime-test".to_string(), + worker_id: runtime_worker_id.to_string(), + }, + assigned_by: "test".to_string(), + assigned_at: "2026-08-25T00:00:00Z".to_string(), + }, + None, + &format!("event-{ticket_id}"), + &format!("operation-{ticket_id}"), + false, + ) + .unwrap(); + } + fn seed_test_repository(api: &WorkspaceApi, repository_id: &str) { if api .store @@ -18431,6 +18492,56 @@ mod tests { ); } + #[tokio::test] + async fn cleanup_blocks_assigned_worker_before_runtime_deletion() { + let workspace = tempfile::tempdir().unwrap(); + init_clean_git_workspace(workspace.path()); + let api = test_api(workspace.path()).await; + let worker_id = seed_cleanup_worker(&api, 3, "normal"); + seed_cleanup_worker_assignment(&api, &worker_id, "ticket-assigned"); + + let plan = build_runtime_cleanup_plan(&api, "runtime-test") + .unwrap_or_else(|err| panic!("cleanup plan: {}", err.error)); + let candidate = plan + .workers + .iter() + .find(|candidate| candidate.worker_id == worker_id) + .unwrap(); + assert_eq!( + candidate.blocking_reason.as_deref(), + Some("worker has current Ticket assignment `ticket-assigned` (`coder`)") + ); + let request = ExecuteRuntimeCleanupRequest { + expected_plan_revision: plan.revision.clone(), + expected_plan_digest: plan.digest.clone(), + worker_target_ids: vec![candidate.target_id.clone()], + workdir_target_ids: Vec::new(), + confirm_dirty_discard_target_ids: Vec::new(), + }; + + let error = execute_runtime_cleanup(&api, "runtime-test", request) + .await + .unwrap_err(); + assert!( + matches!( + error.error, + Error::RuntimeOperationFailed { ref code, .. } + if code == "workspace_cleanup_worker_assigned" + ), + "unexpected cleanup error: {:?}", + error.error + ); + assert!( + api.store + .get_worker_registry( + &api.config.workspace_id, + &RuntimeWorkerRef::new("runtime-test", worker_id), + ) + .unwrap() + .is_some() + ); + } + #[tokio::test] async fn cleanup_execution_requires_dirty_confirmation_and_deletes_removed_record() { let workspace = tempfile::tempdir().unwrap(); diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index faf84096..b09bcdac 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -1049,6 +1049,11 @@ pub trait ControlPlaneStore: Send + Sync { workspace_id: &str, ticket_id: &str, ) -> Result>; + fn get_current_ticket_role_assignment_for_worker( + &self, + workspace_id: &str, + worker: &RuntimeWorkerRef, + ) -> Result>; fn get_current_ticket_role_assignment( &self, workspace_id: &str, @@ -3610,6 +3615,28 @@ impl ControlPlaneStore for SqliteWorkspaceStore { }) } + fn get_current_ticket_role_assignment_for_worker( + &self, + workspace_id: &str, + worker: &RuntimeWorkerRef, + ) -> Result> { + self.with_conn(|conn| { + let sql = ticket_role_assignment_select_sql( + "WHERE current.workspace_id = ?1 \ + AND current.principal_kind = 'worker' \ + AND current.runtime_id = ?2 AND current.worker_id = ?3 \ + ORDER BY a.assigned_at, a.assignment_id LIMIT 1", + ); + Ok(conn + .query_row( + &sql, + params![workspace_id, worker.runtime_id, worker.worker_id], + read_ticket_role_assignment_record, + ) + .optional()?) + }) + } + fn set_current_ticket_role_assignment( &self, record: &TicketRoleAssignmentRecord, From 9a05bfa0c3fadc8523e1d13e2935408db929d489 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 25 Aug 2026 09:52:21 +0900 Subject: [PATCH 4/4] feat: allow ticket implementation cancellation --- crates/workspace-server/src/server.rs | 227 +++++++++++ crates/workspace-server/src/store.rs | 359 +++++++++++++----- .../tickets/[ticketId]/+page.svelte | 34 ++ 3 files changed, 524 insertions(+), 96 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 8be3de6f..7017e637 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1827,6 +1827,10 @@ pub fn build_router(api: WorkspaceApi) -> Router { "/api/w/{workspace_id}/tickets/{id}/assignments/{role}", put(scoped_set_ticket_assignment).delete(scoped_clear_ticket_assignment), ) + .route( + "/api/w/{workspace_id}/tickets/{id}/implementation-cancellations", + post(scoped_cancel_ticket_implementation), + ) .route( "/api/w/{workspace_id}/tickets/{id}/state", post(scoped_transition_ticket_state), @@ -3176,6 +3180,14 @@ struct SetTicketRoleAssignmentRequest { expected_assignment_id: Option, } +#[derive(Debug, Deserialize)] +#[serde(deny_unknown_fields)] +struct CancelTicketImplementationRequest { + operation_id: String, + assignment_id: String, + reason: String, +} + #[derive(Debug, Default, Deserialize)] #[serde(deny_unknown_fields)] struct ClearTicketRoleAssignmentQuery { @@ -3373,6 +3385,113 @@ async fn scoped_clear_ticket_assignment( assignment: None, })) } + +async fn scoped_cancel_ticket_implementation( + State(api): State, + AxumPath(path): AxumPath, + Json(request): Json, +) -> ApiResult> { + validate_workspace_scope(&api, &path.workspace_id)?; + let ticket = api.authority.ticket(&path.id)?; + let operation_id = require_ticket_assignment_value("operation_id", request.operation_id)?; + let assignment_id = require_ticket_assignment_value("assignment_id", request.assignment_id)?; + let reason = require_ticket_assignment_value("reason", request.reason)?; + if reason.len() > 512 { + return Err(Error::InvalidInput( + "implementation cancellation reason must be at most 512 bytes".to_string(), + ) + .into()); + } + + if !matches!( + ticket.state.as_str(), + state if state == TicketWorkflowState::InProgress.as_str() + || state == TicketWorkflowState::Ready.as_str() + ) { + return Err(Error::TicketAssignmentConflict(format!( + "implementation cancellation requires an inprogress Ticket; current state is {}", + ticket.state + )) + .into()); + } + + let current = api.store.get_current_ticket_role_assignment( + &path.workspace_id, + &ticket.id, + TicketAssignmentRole::Coder, + )?; + if let Some(assignment) = current.filter(|value| value.assignment_id == assignment_id) + && let TicketAssignmentPrincipal::Worker { + runtime_id, + worker_id, + } = assignment.principal + { + if api + .store + .get_ticket_assignment_operation(&path.workspace_id, &operation_id)? + .is_some() + { + return Err(Error::TicketAssignmentConflict(format!( + "operation `{operation_id}` was already used for another Ticket assignment mutation" + )) + .into()); + } + let worker = RuntimeWorkerRef::new(runtime_id, worker_id); + cancel_ticket_coder_worker(&api, &worker, &reason).await?; + } + + let cancelled = api.store.cancel_current_ticket_coder_assignment( + &path.workspace_id, + &ticket.id, + &assignment_id, + &new_id("tasev"), + &new_id("tev"), + &operation_id, + "workspace-web", + &Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true), + &reason, + )?; + if !cancelled { + return Err(Error::TicketAssignmentConflict(format!( + "assignment `{assignment_id}` is not the current Coder implementation" + )) + .into()); + } + browser_ticket_detail(&api, &ticket.id) +} + +async fn cancel_ticket_coder_worker( + api: &WorkspaceApi, + worker: &RuntimeWorkerRef, + reason: &str, +) -> ApiResult<()> { + let session_lock = current_worker_session_lock(api, worker); + let _session_guard = session_lock.lock().await; + match api.runtime.cancel_worker( + worker, + WorkerLifecycleRequest { + reason: Some(format!("Ticket implementation cancelled: {reason}")), + ticket_assignment: None, + }, + ) { + Ok(result) if result.state == WorkerOperationState::Accepted => {} + Ok(result) => { + return Err(ApiError::with_diagnostics( + Error::RuntimeOperationFailed { + runtime_id: worker.runtime_id.clone(), + code: "workspace_ticket_implementation_cancel_rejected".to_string(), + message: "Runtime did not cancel the assigned Coder Worker".to_string(), + }, + result.diagnostics, + )); + } + Err(RuntimeRegistryError::UnknownWorker { .. }) => {} + Err(error) => return Err(error.into_error().into()), + } + close_current_worker_session_locked(api, worker).await?; + Ok(()) +} + fn validate_ticket_assignment_state( api: &WorkspaceApi, assignment: &WorkerTicketAssignmentRequest, @@ -16015,6 +16134,114 @@ mod tests { assert_eq!(replayed_clear.assignment, None); } + #[tokio::test] + async fn implementation_cancellation_cancels_coder_and_returns_ticket_to_ready() { + let dir = tempfile::tempdir().unwrap(); + let api = test_api(dir.path()).await; + let worker = api + .runtime + .spawn_worker( + EMBEDDED_WORKER_RUNTIME_ID, + test_create_binding(), + WorkerSpawnRequest { + requested_worker_name: Some("cancelled-coder".to_string()), + intent: WorkerSpawnIntent::TicketRole { + ticket_id: "implementation-cancellation".to_string(), + role: TicketWorkerRole::Coder, + }, + acceptance: WorkerSpawnAcceptanceRequirement::RunAccepted { + expected_segments: 0, + }, + profile: ProfileSelector::Builtin("builtin:coder".to_string()), + ticket_assignment: None, + initial_submit: Vec::new(), + working_directory_request: None, + resolved_working_directory_request: None, + resolved_working_directory: None, + resolved_config_bundle: None, + resolved_worker_observation_enabled: false, + resolved_worker_observation_grants: Vec::new(), + resolved_workspace_api: Some(test_worker_workspace_api( + EMBEDDED_WORKER_RUNTIME_ID, + )), + resolved_memory_settings: Some(test_worker_memory_settings()), + resolved_control_operation: None, + }, + ) + .unwrap() + .worker + .unwrap() + .worker; + let worker = RuntimeWorkerRef::new(EMBEDDED_WORKER_RUNTIME_ID, worker.worker_id); + api.store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: worker.clone(), + display_name: "Cancelled Coder".to_string(), + profile: Some("builtin:coder".to_string()), + retention_state: "normal".to_string(), + transcript_ref: None, + session_ref: None, + summary_ref: None, + diagnostics_ref: None, + created_at: TEST_CREATED_AT.to_string(), + updated_at: TEST_CREATED_AT.to_string(), + }) + .unwrap(); + let backend = browser_ticket_backend(&api).unwrap(); + let mut input = ticket::NewTicket::new("Implementation cancellation"); + input.workflow_state = Some(TicketWorkflowState::InProgress); + let ticket = backend.create(input).unwrap(); + api.store + .set_current_ticket_coder_assignment( + &TicketCoderAssignmentRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + ticket_id: ticket.id.clone(), + assignment_id: "cancelled-assignment".to_string(), + worker: worker.clone(), + assigned_by: "test-user".to_string(), + assigned_at: TEST_CREATED_AT.to_string(), + }, + None, + "cancelled-assignment-event", + "cancelled-assignment-operation", + false, + ) + .unwrap(); + let path = || { + AxumPath(ScopedRecordPath { + workspace_id: TEST_WORKSPACE_ID.to_string(), + id: ticket.id.clone(), + }) + }; + let request = || { + Json(CancelTicketImplementationRequest { + operation_id: "cancel-implementation-operation".to_string(), + assignment_id: "cancelled-assignment".to_string(), + reason: "redo with the corrected design".to_string(), + }) + }; + + let Json(cancelled) = + scoped_cancel_ticket_implementation(State(api.clone()), path(), request()) + .await + .unwrap(); + assert_eq!(cancelled.state, TicketWorkflowState::Ready.as_str()); + assert!(cancelled.current_coder.is_none()); + assert!( + !cancelled + .assignments + .iter() + .any(|assignment| assignment.role == "coder") + ); + assert_eq!(api.runtime.worker(&worker).unwrap().state, "cancelled"); + + let Json(replayed) = scoped_cancel_ticket_implementation(State(api), path(), request()) + .await + .unwrap(); + assert_eq!(replayed.state, TicketWorkflowState::Ready.as_str()); + } + #[tokio::test] async fn authenticated_worker_ticket_mutation_notifies_current_assignment() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index b09bcdac..1192bc83 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -1086,6 +1086,18 @@ pub trait ControlPlaneStore: Send + Sync { occurred_at: &str, reason: Option<&str>, ) -> Result; + fn cancel_current_ticket_coder_assignment( + &self, + workspace_id: &str, + ticket_id: &str, + assignment_id: &str, + assignment_event_id: &str, + state_event_id: &str, + operation_id: &str, + actor: &str, + occurred_at: &str, + reason: &str, + ) -> Result; fn get_current_ticket_coder_assignment( &self, workspace_id: &str, @@ -4084,106 +4096,110 @@ impl ControlPlaneStore for SqliteWorkspaceStore { ) -> Result { self.with_conn_mut(|conn| { let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; - let current = read_ticket_role_assignment_by_id(&tx, workspace_id, assignment_id)?; - let Some(current) = current.filter(|value| { - value.ticket_id == ticket_id && value.role == role - }) else { - return Ok(false); - }; - let principal_json = serde_json::to_string(¤t.principal) - .map_err(|error| Error::Store(format!("serialize Ticket assignment principal: {error}")))?; - let mut hasher = Sha256::new(); - for value in [ - "ticket-role-assignment:clear:v1", - workspace_id, - ticket_id, - role.as_str(), - assignment_id, - principal_json.as_str(), - actor, - reason.unwrap_or(""), - ] { - hasher.update(value.as_bytes()); - hasher.update([0]); - } - let fingerprint = hasher - .finalize() - .iter() - .map(|byte| format!("{byte:02x}")) - .collect::(); - let (principal_id, runtime_id, worker_id) = match ¤t.principal { - TicketAssignmentPrincipal::User { account_id } => { - (Some(account_id.as_str()), None, None) - } - TicketAssignmentPrincipal::Worker { - runtime_id, - worker_id, - } => (None, Some(runtime_id.as_str()), Some(worker_id.as_str())), - TicketAssignmentPrincipal::WorkspaceAgent { agent_key } => { - (Some(agent_key.as_str()), None, None) - } - }; - let inserted = tx.execute( - "INSERT OR IGNORE INTO ticket_assignment_operations ( - workspace_id, operation_id, action, ticket_id, role, principal_kind, - principal_id, runtime_id, worker_id, assignment_id, - expected_assignment_id, created_at, request_fingerprint - ) VALUES (?1, ?2, 'unassign', ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9, ?10, ?11)", - params![ + let cleared = clear_current_ticket_role_assignment_in_tx( + &tx, workspace_id, - operation_id, ticket_id, - role.as_str(), - current.principal.kind(), - principal_id, - runtime_id, - worker_id, + role, assignment_id, + event_id, + operation_id, + actor, occurred_at, - fingerprint, - ], - )?; - if inserted == 0 { - let persisted: String = tx.query_row( - "SELECT request_fingerprint FROM ticket_assignment_operations - WHERE workspace_id = ?1 AND operation_id = ?2", - params![workspace_id, operation_id], + reason, + "ticket-role-assignment:clear:v1", + )?; + tx.commit()?; + Ok(cleared) + }) + } + + fn cancel_current_ticket_coder_assignment( + &self, + workspace_id: &str, + ticket_id: &str, + assignment_id: &str, + assignment_event_id: &str, + state_event_id: &str, + operation_id: &str, + actor: &str, + occurred_at: &str, + reason: &str, + ) -> Result { + self.with_conn_mut(|conn| { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let state: String = tx.query_row( + "SELECT workflow_state FROM typed_tickets + WHERE workspace_id = ?1 AND ticket_id = ?2", + params![workspace_id, ticket_id], |row| row.get(0), )?; - if persisted != fingerprint { + if !matches!(state.as_str(), "inprogress" | "ready") { return Err(Error::TicketAssignmentConflict(format!( - "operation `{operation_id}` was already used for different Ticket assignment input" + "implementation cancellation requires an inprogress Ticket; current state is `{state}`" ))); } - tx.commit()?; - return Ok(true); - } - let deleted = tx.execute( - "DELETE FROM ticket_current_worker_assignments - WHERE workspace_id = ?1 AND ticket_id = ?2 AND role = ?3 AND assignment_id = ?4", - params![workspace_id, ticket_id, role.as_str(), assignment_id], - )?; - if deleted != 0 { - tx.execute( - "INSERT INTO ticket_worker_assignment_events ( - workspace_id, ticket_id, role, event_id, action, assignment_id, - previous_assignment_id, actor, created_at, operation_id, reason - ) VALUES (?1, ?2, ?3, ?4, 'unassigned', NULL, ?5, ?6, ?7, ?8, ?9)", - params![ - workspace_id, - ticket_id, - role.as_str(), - event_id, - assignment_id, - actor, - occurred_at, - operation_id, - reason, - ], + let cleared = clear_current_ticket_role_assignment_in_tx( + &tx, + workspace_id, + ticket_id, + TicketAssignmentRole::Coder, + assignment_id, + assignment_event_id, + operation_id, + actor, + occurred_at, + Some(reason), + "ticket-role-assignment:cancel-implementation:v1", )?; - } + if !cleared { + return Ok(false); + } + if state == "inprogress" { + let event_index: i64 = tx.query_row( + "SELECT COALESCE(MAX(event_index), -1) + 1 FROM typed_ticket_events + WHERE workspace_id = ?1 AND ticket_id = ?2", + params![workspace_id, ticket_id], + |row| row.get(0), + )?; + tx.execute( + "INSERT INTO typed_ticket_events ( + workspace_id, ticket_id, event_index, kind, author, at, + from_state, to_state, reason, state_field, heading, body + ) VALUES (?1, ?2, ?3, 'state_changed', ?4, ?5, + 'inprogress', 'ready', ?6, 'state', + 'Implementation cancelled', '')", + params![workspace_id, ticket_id, event_index, actor, occurred_at, reason], + )?; + for (key, value) in [ + ("event_id", state_event_id), + ("assignment_id", assignment_id), + ("assignment_role", "coder"), + ("operation_id", operation_id), + ] { + tx.execute( + "INSERT INTO typed_ticket_event_attributes ( + workspace_id, ticket_id, event_index, key, value + ) VALUES (?1, ?2, ?3, ?4, ?5)", + params![workspace_id, ticket_id, event_index, key, value], + )?; + } + let updated = tx.execute( + "UPDATE typed_tickets SET workflow_state = 'ready', + workflow_state_explicit = 1, + queued_by = NULL, queued_at = NULL, updated_at = ?3 + WHERE workspace_id = ?1 AND ticket_id = ?2 + AND workflow_state = 'inprogress'", + params![workspace_id, ticket_id, occurred_at], + )?; + if updated != 1 { + return Err(Error::TicketAssignmentConflict( + "Ticket state changed during implementation cancellation".to_string(), + )); + } + } tx.commit()?; - Ok(deleted != 0) + Ok(true) }) } @@ -5540,6 +5556,117 @@ fn validate_ticket_assignment_role_principal( } } +#[allow(clippy::too_many_arguments)] +fn clear_current_ticket_role_assignment_in_tx( + tx: &rusqlite::Transaction<'_>, + workspace_id: &str, + ticket_id: &str, + role: TicketAssignmentRole, + assignment_id: &str, + event_id: &str, + operation_id: &str, + actor: &str, + occurred_at: &str, + reason: Option<&str>, + fingerprint_domain: &str, +) -> Result { + let current = read_ticket_role_assignment_by_id(tx, workspace_id, assignment_id)?; + let Some(current) = current.filter(|value| value.ticket_id == ticket_id && value.role == role) + else { + return Ok(false); + }; + let principal_json = serde_json::to_string(¤t.principal) + .map_err(|error| Error::Store(format!("serialize Ticket assignment principal: {error}")))?; + let mut hasher = Sha256::new(); + for value in [ + fingerprint_domain, + workspace_id, + ticket_id, + role.as_str(), + assignment_id, + principal_json.as_str(), + actor, + reason.unwrap_or(""), + ] { + hasher.update(value.as_bytes()); + hasher.update([0]); + } + let fingerprint = hasher + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect::(); + let (principal_id, runtime_id, worker_id) = match ¤t.principal { + TicketAssignmentPrincipal::User { account_id } => (Some(account_id.as_str()), None, None), + TicketAssignmentPrincipal::Worker { + runtime_id, + worker_id, + } => (None, Some(runtime_id.as_str()), Some(worker_id.as_str())), + TicketAssignmentPrincipal::WorkspaceAgent { agent_key } => { + (Some(agent_key.as_str()), None, None) + } + }; + let inserted = tx.execute( + "INSERT OR IGNORE INTO ticket_assignment_operations ( + workspace_id, operation_id, action, ticket_id, role, principal_kind, + principal_id, runtime_id, worker_id, assignment_id, + expected_assignment_id, created_at, request_fingerprint + ) VALUES (?1, ?2, 'unassign', ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?9, ?10, ?11)", + params![ + workspace_id, + operation_id, + ticket_id, + role.as_str(), + current.principal.kind(), + principal_id, + runtime_id, + worker_id, + assignment_id, + occurred_at, + fingerprint, + ], + )?; + if inserted == 0 { + let persisted: String = tx.query_row( + "SELECT request_fingerprint FROM ticket_assignment_operations + WHERE workspace_id = ?1 AND operation_id = ?2", + params![workspace_id, operation_id], + |row| row.get(0), + )?; + if persisted != fingerprint { + return Err(Error::TicketAssignmentConflict(format!( + "operation `{operation_id}` was already used for different Ticket assignment input" + ))); + } + return Ok(true); + } + let deleted = tx.execute( + "DELETE FROM ticket_current_worker_assignments + WHERE workspace_id = ?1 AND ticket_id = ?2 AND role = ?3 AND assignment_id = ?4", + params![workspace_id, ticket_id, role.as_str(), assignment_id], + )?; + if deleted != 0 { + tx.execute( + "INSERT INTO ticket_worker_assignment_events ( + workspace_id, ticket_id, role, event_id, action, assignment_id, + previous_assignment_id, actor, created_at, operation_id, reason + ) VALUES (?1, ?2, ?3, ?4, 'unassigned', NULL, ?5, ?6, ?7, ?8, ?9)", + params![ + workspace_id, + ticket_id, + role.as_str(), + event_id, + assignment_id, + actor, + occurred_at, + operation_id, + reason, + ], + )?; + } + Ok(deleted != 0) +} + fn current_ticket_worker_assignment_select_sql() -> String { "SELECT a.workspace_id, a.ticket_id, a.assignment_id, a.runtime_id, a.worker_id, \ a.assigned_by, a.assigned_at \ @@ -11052,18 +11179,58 @@ INSERT INTO worker_registry ( ); assert!( store - .clear_current_ticket_role_assignment( + .cancel_current_ticket_coder_assignment( + "workspace-role", + &ticket.meta.id, + "coder-manual-1", + "event-cancel-coder", + "event-cancel-state", + "op-cancel-coder", + "user", + "2026-09-01T00:03:00Z", + "implementation needs to be redone", + ) + .unwrap() + ); + assert!( + store + .cancel_current_ticket_coder_assignment( + "workspace-role", + &ticket.meta.id, + "coder-manual-1", + "event-cancel-coder-replay", + "event-cancel-state-replay", + "op-cancel-coder", + "user", + "2026-09-01T00:03:30Z", + "implementation needs to be redone", + ) + .unwrap(), + "same operation must be idempotent after the assignment is cleared" + ); + let cancelled_ticket = + ticket::TicketBackend::show(&backend, ticket.meta.id.clone().into()).unwrap(); + assert_eq!( + cancelled_ticket.meta.workflow_state, + ticket::TicketWorkflowState::Ready + ); + assert_eq!( + cancelled_ticket + .events + .last() + .and_then(|event| event.attributes.get("assignment_id")) + .map(String::as_str), + Some("coder-manual-1") + ); + assert!( + store + .get_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() + .is_none() ); assert!( store diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte index a0afba90..d5add100 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -55,6 +55,10 @@ let readyOperationKey = $state(null); let manualRuntimeId = $state(""); let manualWorkerId = $state(""); + let cancellationReason = $state(""); + const coderAssignment = $derived( + ticket.assignments.find((assignment) => assignment.role === "coder") ?? null, + ); const selectedRepository = $derived( (loadedRepositories?.items ?? []).find((repository: RepositorySummary) => repository.id === repositoryId) ?? null, ); @@ -162,6 +166,18 @@ }); } + async function cancelImplementation(event: SubmitEvent): Promise { + event.preventDefault(); + if (!coderAssignment || !cancellationReason.trim()) return; + if ( + await mutate("cancel-implementation", "/implementation-cancellations", { + operation_id: crypto.randomUUID(), + assignment_id: coderAssignment.assignment_id, + reason: cancellationReason.trim(), + }) + ) cancellationReason = ""; + } + async function saveEdit(event: SubmitEvent) { event.preventDefault(); if ( @@ -416,6 +432,24 @@ {/if} + {#if ticket.state === "inprogress" && coderAssignment} +
+ Cancel implementation +
+

+ Cancel the assigned Coder, remove its assignment, and return this Ticket to ready. +

+ + +
+
+ {/if} {#if ticket.assignment_diagnostics.length > 0} {#each ticket.assignment_diagnostics as diagnostic}

{diagnostic}