From 560226dea2af1996a2fadaa4eaf053a0a30b09d6 Mon Sep 17 00:00:00 2001 From: Hare Date: Thu, 20 Aug 2026 11:06:31 +0900 Subject: [PATCH] fix: enforce assignment workspace references --- crates/workspace-server/src/retention.rs | 9 + crates/workspace-server/src/server.rs | 51 ++++ crates/workspace-server/src/store.rs | 287 +++++++++++++++++- .../workspace-schema-migrations.md | 3 +- 4 files changed, 343 insertions(+), 7 deletions(-) diff --git a/crates/workspace-server/src/retention.rs b/crates/workspace-server/src/retention.rs index 3b34a6db..58d2baf8 100644 --- a/crates/workspace-server/src/retention.rs +++ b/crates/workspace-server/src/retention.rs @@ -1082,6 +1082,11 @@ mod tests { ) VALUES('w',?1,'r','one','builtin:coder','normal','created','rev1')", [worker_id().to_string()], )?; + c.execute( + "INSERT INTO typed_tickets (workspace_id, ticket_id, slug, title, status, kind, priority, body, workflow_state, workflow_state_explicit) \ + VALUES ('w', 'ticket', 'ticket', 'Ticket', 'open', 'task', 'normal', '', 'planning', 1)", + [], + )?; Ok(()) }) .unwrap(); @@ -1267,7 +1272,11 @@ mod tests { fn purge_tombstone_commit_is_idempotent() { let s = setup(); s.with_conn(|conn| { + conn.execute("INSERT INTO typed_tickets(workspace_id,ticket_id,slug,title,status,kind,priority,body,workflow_state,workflow_state_explicit) VALUES('w','ticket-old','ticket-old','Old Ticket','open','task','normal','','planning',1)", [])?; + conn.execute("INSERT INTO worker_registry(workspace_id,worker_id,runtime_id,display_name,profile,retention_state,created_at,updated_at) VALUES('w','1','r','old worker','builtin:coder','normal','created','rev1')", [])?; conn.execute("INSERT INTO ticket_worker_assignments(workspace_id,ticket_id,assignment_id,runtime_id,worker_id,assigned_by,assigned_at) VALUES('w','ticket-old','assignment-old','r','1','test','t')", [])?; + conn.execute("DELETE FROM worker_registry WHERE workspace_id='w' AND runtime_id='r' AND worker_id='1'", [])?; + conn.execute("DELETE FROM typed_tickets WHERE workspace_id='w' AND ticket_id='ticket-old'", [])?; Ok(()) }).unwrap(); let p = s.plan_worker_removal(&req(), &inv()).unwrap(); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 7cdfac14..578ad3d0 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -14556,6 +14556,21 @@ mod tests { .create(ticket::NewTicket::new("Assigned Ticket")) .unwrap(); let ticket_id = created.id; + api.store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: RuntimeWorkerRef::new("embedded", "42"), + display_name: "Worker 42".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 assignment = TicketWorkerAssignmentRecord { workspace_id: TEST_WORKSPACE_ID.to_string(), ticket_id: ticket_id.clone(), @@ -14650,6 +14665,24 @@ mod tests { .unwrap() .worker .unwrap(); + api.store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: RuntimeWorkerRef::new( + EMBEDDED_WORKER_RUNTIME_ID, + source_worker.worker.worker_id.clone(), + ), + display_name: "Source Worker".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 recipient_worker = api .runtime .spawn_worker( @@ -14660,6 +14693,24 @@ mod tests { .unwrap() .worker .unwrap(); + api.store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.to_string(), + worker: RuntimeWorkerRef::new( + EMBEDDED_WORKER_RUNTIME_ID, + recipient_worker.worker.worker_id.clone(), + ), + display_name: "Recipient Worker".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 ticket_ref = backend .create(ticket::NewTicket::new("Notify assigned Worker")) diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 742c51a4..8069d018 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -1075,6 +1075,7 @@ impl SqliteWorkspaceStore { Error::Store(format!("Merge Request schema verification failed: {error}")) })?; validate_workspace_resource_references(&conn)?; + verify_workspace_resource_constraints(&conn)?; Ok(Self { conn: Arc::new(Mutex::new(conn)), }) @@ -5128,6 +5129,30 @@ CREATE UNIQUE INDEX ux_worker_workdir_attachment_reservation_id Ok(()) } +fn verify_workspace_resource_constraints(conn: &Connection) -> Result<()> { + if current_schema_version(conn)? < 39 { + return Ok(()); + } + for trigger in [ + "ticket_worker_assignments_validate_insert", + "ticket_worker_assignments_validate_update", + "ticket_worker_assignment_events_validate_insert", + "ticket_assignment_operations_validate_insert", + ] { + let exists = conn.query_row( + "SELECT EXISTS(SELECT 1 FROM sqlite_schema WHERE type = 'trigger' AND name = ?1)", + [trigger], + |row| row.get::<_, bool>(0), + )?; + if !exists { + return Err(Error::Store(format!( + "Workspace resource constraint trigger `{trigger}` is missing" + ))); + } + } + Ok(()) +} + fn validate_workspace_resource_references(conn: &Connection) -> Result<()> { let diagnostics = workspace_resource_reference_diagnostics(conn)?; if diagnostics.is_empty() { @@ -5211,6 +5236,55 @@ fn workspace_resource_reference_diagnostics(conn: &Connection) -> Result ' || assignment.ticket_id \ + FROM ticket_worker_assignments AS assignment \ + WHERE NOT EXISTS (SELECT 1 FROM typed_tickets AS ticket \ + WHERE ticket.workspace_id = assignment.workspace_id \ + AND ticket.ticket_id = assignment.ticket_id) LIMIT 100", + ), + ( + "ticket_worker_assignments.worker_id", + "SELECT assignment.workspace_id || '/' || assignment.assignment_id || ' -> ' || assignment.runtime_id || '/' || assignment.worker_id \ + FROM ticket_worker_assignments AS assignment \ + WHERE NOT EXISTS (SELECT 1 FROM worker_registry AS worker \ + WHERE worker.workspace_id = assignment.workspace_id \ + AND worker.runtime_id = assignment.runtime_id \ + AND worker.worker_id = assignment.worker_id) LIMIT 100", + ), + ( + "ticket_current_worker_assignments.assignment_id", + "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 \ + AND assignment.runtime_id = current.runtime_id \ + AND assignment.worker_id = current.worker_id) LIMIT 100", + ), + ( + "ticket_worker_assignment_events.assignment_id", + "SELECT event.workspace_id || '/' || event.event_id \ + FROM ticket_worker_assignment_events AS event \ + WHERE (event.assignment_id IS NOT NULL AND NOT EXISTS (\ + SELECT 1 FROM ticket_worker_assignments AS assignment \ + WHERE assignment.workspace_id = event.workspace_id \ + AND assignment.assignment_id = event.assignment_id)) \ + OR (event.previous_assignment_id IS NOT NULL AND NOT EXISTS (\ + SELECT 1 FROM ticket_worker_assignments AS assignment \ + WHERE assignment.workspace_id = event.workspace_id \ + AND assignment.assignment_id = event.previous_assignment_id)) LIMIT 100", + ), + ( + "ticket_assignment_operations.ticket_id", + "SELECT operation.workspace_id || '/' || operation.operation_id || ' -> ' || operation.ticket_id \ + FROM ticket_assignment_operations AS operation \ + WHERE NOT EXISTS (SELECT 1 FROM typed_tickets AS ticket \ + WHERE ticket.workspace_id = operation.workspace_id \ + AND ticket.ticket_id = operation.ticket_id) LIMIT 100", + ), ( "artifacts.ticket_id", "SELECT artifact.workspace_id || '/' || artifact.artifact_id || ' -> ' || artifact.ticket_id \ @@ -5247,6 +5321,16 @@ fn workspace_resource_reference_diagnostics(conn: &Connection) -> Result panic!("missing assignment constraint trigger must fail closed"), + Err(error) => error, + }; + assert!( + error + .to_string() + .contains("ticket_worker_assignments_validate_insert"), + "{error}" + ); + } + #[test] fn migration_plan_lists_workspace_reference_violations_without_mutating_source() { let temp = tempfile::tempdir().unwrap(); @@ -8243,6 +8424,17 @@ INSERT INTO typed_ticket_relations ( INSERT INTO objective_ticket_links ( workspace_id, objective_id, ticket_id, kind, created_at ) VALUES ('workspace-b', 'objective-a', 'ticket-b', 'tracks', '2026-01-01'); +INSERT INTO worker_registry ( + workspace_id, runtime_id, worker_id, display_name, retention_state, created_at, updated_at +) VALUES + ('workspace-a', 'runtime-a', '00000000-0000-7000-8000-000000000001', 'Worker A', 'normal', '2026-01-01', '2026-01-01'), + ('workspace-b', 'runtime-b', '00000000-0000-7000-8000-000000000002', 'Worker B', 'normal', '2026-01-01', '2026-01-01'); +INSERT INTO ticket_worker_assignments ( + workspace_id, ticket_id, assignment_id, runtime_id, worker_id, assigned_by, assigned_at +) VALUES ( + 'workspace-b', 'ticket-b', 'assignment-cross-worker', 'runtime-a', + '00000000-0000-7000-8000-000000000001', 'tester', '2026-01-01' +); "#, ) .unwrap(); @@ -8257,12 +8449,19 @@ INSERT INTO objective_ticket_links ( error.contains("objective_ticket_links.objective_id"), "{error}" ); + assert!( + error.contains("ticket_worker_assignments.worker_id"), + "{error}" + ); + assert!(error.contains("assignment-cross-worker"), "{error}"); assert_eq!(current_schema_version(&conn).unwrap(), 38); conn.execute("DELETE FROM typed_ticket_relations", []) .unwrap(); conn.execute("DELETE FROM objective_ticket_links", []) .unwrap(); + conn.execute("DELETE FROM ticket_worker_assignments", []) + .unwrap(); apply_migrations_through(&conn, 39).unwrap(); assert_eq!(current_schema_version(&conn).unwrap(), 39); @@ -8286,6 +8485,53 @@ INSERT INTO objective_ticket_links ( [], ); assert!(cross_objective_link.is_err()); + let cross_ticket_assignment = conn.execute( + "INSERT INTO ticket_worker_assignments \ + (workspace_id, ticket_id, assignment_id, runtime_id, worker_id, assigned_by, assigned_at) \ + VALUES ('workspace-b', 'ticket-a', 'assignment-cross-ticket', 'runtime-b', \ + '00000000-0000-7000-8000-000000000002', 'tester', '2026-01-01')", + [], + ); + assert!(cross_ticket_assignment.is_err()); + let cross_worker_assignment = conn.execute( + "INSERT INTO ticket_worker_assignments \ + (workspace_id, ticket_id, assignment_id, runtime_id, worker_id, assigned_by, assigned_at) \ + VALUES ('workspace-b', 'ticket-b', 'assignment-cross-worker', 'runtime-a', \ + '00000000-0000-7000-8000-000000000001', 'tester', '2026-01-01')", + [], + ); + assert!(cross_worker_assignment.is_err()); + conn.execute( + "INSERT INTO ticket_worker_assignments \ + (workspace_id, ticket_id, assignment_id, runtime_id, worker_id, assigned_by, assigned_at) \ + VALUES ('workspace-b', 'ticket-b', 'assignment-b', 'runtime-b', \ + '00000000-0000-7000-8000-000000000002', 'tester', '2026-01-01')", + [], + ) + .unwrap(); + let mismatched_current_assignment = conn.execute( + "INSERT INTO ticket_current_worker_assignments \ + (workspace_id, ticket_id, assignment_id, runtime_id, worker_id, updated_at) \ + VALUES ('workspace-b', 'ticket-b', 'assignment-b', 'runtime-a', \ + '00000000-0000-7000-8000-000000000001', '2026-01-01')", + [], + ); + assert!(mismatched_current_assignment.is_err()); + let cross_assignment_event = conn.execute( + "INSERT INTO ticket_worker_assignment_events \ + (workspace_id, ticket_id, event_id, action, assignment_id, actor, created_at) \ + VALUES ('workspace-a', 'ticket-a', 'event-cross-assignment', 'assigned', \ + 'assignment-b', 'tester', '2026-01-01')", + [], + ); + assert!(cross_assignment_event.is_err()); + let cross_operation_ticket = conn.execute( + "INSERT INTO ticket_assignment_operations \ + (workspace_id, operation_id, action, ticket_id, created_at) \ + VALUES ('workspace-b', 'operation-cross-ticket', 'assign', 'ticket-a', '2026-01-01')", + [], + ); + assert!(cross_operation_ticket.is_err()); assert!( conn.execute( "DELETE FROM repositories WHERE workspace_id = 'workspace-a' AND repository_id = 'repo-a'", @@ -8321,6 +8567,26 @@ INSERT INTO objective_ticket_links ( .unwrap(), 0 ); + conn.execute( + "DELETE FROM worker_registry WHERE workspace_id = 'workspace-b' AND worker_id = '00000000-0000-7000-8000-000000000002'", + [], + ) + .unwrap(); + conn.execute( + "DELETE FROM typed_tickets WHERE workspace_id = 'workspace-b' AND ticket_id = 'ticket-b'", + [], + ) + .unwrap(); + assert_eq!( + conn.query_row( + "SELECT COUNT(*) FROM ticket_worker_assignments WHERE workspace_id = 'workspace-b'", + [], + |row| row.get::<_, i64>(0), + ) + .unwrap(), + 1, + "historical assignments survive Worker and Ticket retention deletion" + ); conn.execute( "DELETE FROM workspaces WHERE workspace_id = 'workspace-b'", [], @@ -8335,6 +8601,15 @@ INSERT INTO objective_ticket_links ( .unwrap(), 0 ); + assert_eq!( + conn.query_row( + "SELECT COUNT(*) FROM ticket_worker_assignments WHERE workspace_id = 'workspace-b'", + [], + |row| row.get::<_, i64>(0), + ) + .unwrap(), + 0 + ); let foreign_key_failures: i64 = conn .query_row("SELECT COUNT(*) FROM pragma_foreign_key_check", [], |row| { diff --git a/docs/development/workspace-schema-migrations.md b/docs/development/workspace-schema-migrations.md index 5e878942..e9e2caeb 100644 --- a/docs/development/workspace-schema-migrations.md +++ b/docs/development/workspace-schema-migrations.md @@ -20,7 +20,8 @@ The Workspace Server owns one control-plane SQLite database. Schema changes are Start exactly one instance of the new Server binary against the database. Startup applies migration 39 in one SQLite transaction after the Ticket and Merge Request component schemas are available. The migration: - rebuilds Ticket, Objective, assignment, Artifact, and human-key tables with Workspace-scoped composite identity; -- adds composite foreign keys for repository, Ticket, Objective, Worker, relation-target, and assignment references; +- adds composite foreign keys for repository, Ticket, Objective, Worker, relation-target, and current-assignment references; +- validates new historical assignment/event references with SQLite triggers while allowing those audit rows to survive later Ticket or Worker retention deletion; reservation operation ids remain intentionally unconstrained until their resources exist; - checks the rebuilt schema with `PRAGMA foreign_key_check` before recording the schema version; and - restores `PRAGMA foreign_keys = ON` whether the transaction commits or rolls back.