From a2cd8601991827b29db82001cb10220f131382e3 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 17 Aug 2026 14:23:47 +0900 Subject: [PATCH 01/13] feat: gate ticket readiness on validated targets --- Cargo.lock | 1 + crates/ticket/Cargo.toml | 1 + crates/ticket/src/lib.rs | 689 +++++++++++++++--- crates/ticket/src/tool.rs | 174 +++-- crates/worker/src/controller.rs | 6 +- crates/worker/src/feature/builtin/ticket.rs | 54 +- crates/worker/src/shutdown_after_idle.rs | 24 +- crates/workspace-server/src/server.rs | 370 +++++++--- crates/yoi/src/ticket_cli.rs | 77 +- .../console/worker-console.ui.test.ts | 4 +- .../tickets/[ticketId]/+page.svelte | 81 +- 11 files changed, 1106 insertions(+), 375 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d76aaca..95ae1d04 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4417,6 +4417,7 @@ dependencies = [ "serde", "serde_json", "serde_yaml", + "sha2 0.11.0", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/crates/ticket/Cargo.toml b/crates/ticket/Cargo.toml index 3dbff58a..1f7f2c52 100644 --- a/crates/ticket/Cargo.toml +++ b/crates/ticket/Cargo.toml @@ -14,6 +14,7 @@ schemars = { workspace = true } serde = { workspace = true, features = ["derive"] } serde_json = { workspace = true } serde_yaml = "0.9.34" +sha2.workspace = true rusqlite.workspace = true thiserror.workspace = true tempfile.workspace = true diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 7abcf7a3..d44656cc 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -19,6 +19,7 @@ use project_record::{allocate_record_id, unix_epoch_millis_now, validate_record_ use rusqlite::{Connection, OptionalExtension, params}; use serde::{Deserialize, Serialize}; use serde_yaml::{Mapping as YamlMapping, Value as YamlValue}; +use sha2::{Digest, Sha256}; use thiserror::Error; pub mod config; @@ -80,6 +81,32 @@ pub enum TicketError { Locked { path: PathBuf }, #[error("ticket conflict: {0}")] Conflict(String), + #[error("ticket target repository is required")] + MissingTargetRepository, + #[error("ticket target repository `{0}` is not registered in this workspace")] + UnknownTargetRepository(String), + #[error("ticket target selector is required for repository `{0}`")] + MissingTargetSelector(String), + #[error( + "ticket target selector `{selector}` is invalid for repository `{repository_id}`: {reason}" + )] + InvalidTargetSelector { + repository_id: String, + selector: String, + reason: String, + }, + #[error("ticket target authority is unavailable")] + TargetAuthorityUnavailable, + #[error("stale ticket workflow state: expected `{expected}`, found `{actual}`")] + StaleWorkflowState { expected: String, actual: String }, + #[error("invalid ticket workflow transition `{from}` -> `{to}`")] + InvalidWorkflowTransition { from: String, to: String }, + #[error("ticket has unresolved blocking relations: {0}")] + BlockingRelations(String), + #[error( + "ticket operation key `{operation_key}` was reused with a different request fingerprint" + )] + OperationFingerprintMismatch { operation_key: String }, #[error("SQLite ticket backend error: {0}")] Sqlite(String), #[error("ticket parse error in {path}: {message}")] @@ -475,6 +502,35 @@ pub enum TicketTargetEdit { Clear, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct ResolvedTicketTarget { + pub repository_id: String, + pub ref_selector: String, +} + +/// Workspace-owned authority used to resolve and validate implementation targets. +/// +/// Ticket storage never infers repositories from cwd or repository paths. The +/// Workspace Backend supplies this boundary from its authoritative repository +/// catalog. Backends without it fail closed for ready/queue transitions. +pub trait TicketTargetAuthority: Send + Sync { + fn resolve_target( + &self, + workspace_id: &str, + repository_id: Option<&str>, + ref_selector: Option<&str>, + ) -> Result; +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TicketMarkReady { + pub operation_key: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub reason: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub author: Option, +} + impl TicketTargetEdit { fn validate(&self) -> Result<()> { if let Self::Set { @@ -504,6 +560,106 @@ fn validate_ticket_target(repository_id: Option<&str>, ref_selector: Option<&str Ok(()) } +fn resolve_ready_target( + authority: Option<&Arc>, + workspace_id: &str, + ticket: &Ticket, +) -> Result { + authority + .ok_or(TicketError::TargetAuthorityUnavailable)? + .resolve_target( + workspace_id, + ticket.meta.repository_id.as_deref(), + ticket.meta.ref_selector.as_deref(), + ) +} + +fn mark_ready_fingerprint( + ticket: &Ticket, + request: &TicketMarkReady, + target: &ResolvedTicketTarget, +) -> String { + let mut digest = Sha256::new(); + digest.update(b"ticket.mark-ready.v1\0"); + digest.update(ticket.meta.id.as_str().as_bytes()); + digest.update(b"\0planning\0"); + digest.update(target.repository_id.as_bytes()); + digest.update(b"\0"); + digest.update(target.ref_selector.as_bytes()); + digest.update(b"\0"); + if let Some(reason) = request.reason.as_deref() { + digest.update(reason.as_bytes()); + } + digest + .finalize() + .iter() + .map(|byte| format!("{byte:02x}")) + .collect() +} + +fn validate_mark_ready_replay(ticket: &Ticket, request: &TicketMarkReady) -> Result { + let Some(event) = ticket + .events + .iter() + .find(|event| event.attributes.get("operation_key") == Some(&request.operation_key)) + else { + return Ok(false); + }; + let target = ResolvedTicketTarget { + repository_id: event + .attributes + .get("repository_id") + .cloned() + .ok_or_else(|| { + TicketError::Conflict("mark-ready event is missing repository_id".to_owned()) + })?, + ref_selector: event + .attributes + .get("ref_selector") + .cloned() + .ok_or_else(|| { + TicketError::Conflict("mark-ready event is missing ref_selector".to_owned()) + })?, + }; + let fingerprint = mark_ready_fingerprint(ticket, request, &target); + if event.attributes.get("request_fingerprint") != Some(&fingerprint) { + return Err(TicketError::OperationFingerprintMismatch { + operation_key: request.operation_key.clone(), + }); + } + if ticket.meta.workflow_state != TicketWorkflowState::Ready + || ticket.meta.repository_id.as_deref() != Some(target.repository_id.as_str()) + || ticket.meta.ref_selector.as_deref() != Some(target.ref_selector.as_str()) + { + return Err(TicketError::StaleWorkflowState { + expected: TicketWorkflowState::Ready.as_str().to_owned(), + actual: ticket.meta.workflow_state.as_str().to_owned(), + }); + } + Ok(true) +} + +fn validate_generic_state_change( + current: TicketWorkflowState, + to: TicketWorkflowState, +) -> Result<()> { + if current == TicketWorkflowState::Planning && to == TicketWorkflowState::Ready + || current == TicketWorkflowState::Ready && to == TicketWorkflowState::Queued + { + return Err(TicketError::InvalidWorkflowTransition { + from: current.as_str().to_owned(), + to: to.as_str().to_owned(), + }); + } + if !TicketWorkflowState::is_role_transition(current, to) { + return Err(TicketError::InvalidWorkflowTransition { + from: current.as_str().to_owned(), + to: to.as_str().to_owned(), + }); + } + Ok(()) +} + #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct TicketItemEdit { pub title: Option, @@ -1524,12 +1680,7 @@ pub trait TicketBackend { change: TicketStateChange, ) -> Result<()>; fn set_workflow_state(&self, id: TicketIdOrSlug, change: TicketStateChange) -> Result<()>; - fn mark_intake_ready( - &self, - id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, - ) -> Result<()>; + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> Result; fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()>; fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()>; fn add_ticket_relation( @@ -1605,10 +1756,9 @@ pub enum TicketBackendOperation { id: TicketIdOrSlug, change: TicketStateChange, }, - MarkIntakeReady { + MarkReady { id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, + request: TicketMarkReady, }, QueueReady { id: TicketIdOrSlug, @@ -1710,13 +1860,8 @@ where backend.set_workflow_state(id, change)?; TicketBackendOperationResult::Unit } - TicketBackendOperation::MarkIntakeReady { - id, - summary, - change, - } => { - backend.mark_intake_ready(id, summary, change)?; - TicketBackendOperationResult::Unit + TicketBackendOperation::MarkReady { id, request } => { + TicketBackendOperationResult::Ticket(backend.mark_ready(id, request)?) } TicketBackendOperation::QueueReady { id, queued_by } => { backend.queue_ready(id, &queued_by)?; @@ -1756,10 +1901,25 @@ where }) } -#[derive(Debug, Clone)] +#[derive(Clone)] pub struct LocalTicketBackend { root: PathBuf, record_language: Option, + target_authority: Option>, +} + +impl fmt::Debug for LocalTicketBackend { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("LocalTicketBackend") + .field("root", &self.root) + .field("record_language", &self.record_language) + .field( + "target_authority", + &self.target_authority.as_ref().map(|_| "configured"), + ) + .finish() + } } impl LocalTicketBackend { @@ -1767,6 +1927,7 @@ impl LocalTicketBackend { Self { root: root.into(), record_language: None, + target_authority: None, } } @@ -1775,6 +1936,11 @@ impl LocalTicketBackend { self } + pub fn with_target_authority(mut self, authority: Arc) -> Self { + self.target_authority = Some(authority); + self + } + pub fn record_language(&self) -> Option<&str> { self.record_language.as_deref() } @@ -2076,6 +2242,16 @@ impl LocalTicketBackend { dir: &Path, change: &TicketStateChange, state_field: Option<&str>, + ) -> Result<()> { + self.append_state_changed_event_with_attributes(dir, change, state_field, &[]) + } + + fn append_state_changed_event_with_attributes( + &self, + dir: &Path, + change: &TicketStateChange, + state_field: Option<&str>, + extra_attributes: &[(&str, &str)], ) -> Result<()> { validate_state_change(change)?; let author = change.author.clone().unwrap_or_else(default_author); @@ -2087,6 +2263,7 @@ impl LocalTicketBackend { if let Some(state_field) = state_field { attrs.push(("field", state_field)); } + attrs.extend_from_slice(extra_attributes); self.append_thread_event( dir, TicketEventKind::StateChanged.as_str(), @@ -2247,6 +2424,7 @@ pub struct SqliteTicketBackend { record_language: Option, event_attributes: BTreeMap, mutation_hook: Option>, + target_authority: Option>, #[cfg(test)] full_ticket_load_count: Arc, } @@ -2263,6 +2441,10 @@ impl fmt::Debug for SqliteTicketBackend { "mutation_hook", &self.mutation_hook.as_ref().map(|_| "configured"), ) + .field( + "target_authority", + &self.target_authority.as_ref().map(|_| "configured"), + ) .finish() } } @@ -2275,6 +2457,7 @@ impl SqliteTicketBackend { record_language: None, event_attributes: BTreeMap::new(), mutation_hook: None, + target_authority: None, #[cfg(test)] full_ticket_load_count: Arc::new(AtomicUsize::new(0)), } @@ -2316,6 +2499,11 @@ impl SqliteTicketBackend { self } + pub fn with_target_authority(mut self, authority: Arc) -> Self { + self.target_authority = Some(authority); + self + } + pub fn db_path(&self) -> &Path { self.db_path.as_path() } @@ -3166,6 +3354,15 @@ impl TicketBackend for SqliteTicketBackend { validate_required_event_value("author", author)?; } let ticket_id = self.resolve_ticket_id(conn, id)?; + if edit.target.is_some() { + let current = self.load_ticket(conn, &ticket_id)?.meta.workflow_state; + if current != TicketWorkflowState::Planning { + return Err(TicketError::Conflict(format!( + "ticket implementation target is locked after planning (current state: {})", + current.as_str() + ))); + } + } let now = now_utc(); let mut body_edit_audit = TicketBodyEditAudit::None; if let Some(title) = edit.title.as_ref() { @@ -3331,10 +3528,41 @@ impl TicketBackend for SqliteTicketBackend { } fn set_workflow_state(&self, id: TicketIdOrSlug, change: TicketStateChange) -> Result<()> { + validate_state_change(&change)?; + let from = TicketWorkflowState::parse(&change.from).ok_or_else(|| { + TicketError::InvalidWorkflowTransition { + from: change.from.clone(), + to: change.to.clone(), + } + })?; + let to = TicketWorkflowState::parse(&change.to).ok_or_else(|| { + TicketError::InvalidWorkflowTransition { + from: change.from.clone(), + to: change.to.clone(), + } + })?; + validate_generic_state_change(from, to)?; self.with_write(|conn| { - validate_state_change(&change)?; let ticket_id = self.resolve_ticket_id(conn, id)?; - let to = TicketWorkflowState::parse(&change.to).ok_or_else(|| TicketError::Conflict(format!("unknown workflow_state '{}':", change.to)))?; + let current = self.load_ticket(conn, &ticket_id)?.meta.workflow_state; + if current != from { + return Err(TicketError::StaleWorkflowState { + expected: from.as_str().to_owned(), + actual: current.as_str().to_owned(), + }); + } + if from == TicketWorkflowState::Queued && to == TicketWorkflowState::InProgress { + let ticket = self.load_ticket(conn, &ticket_id)?; + let blockers = ticket + .relations + .blockers + .into_iter() + .filter(|blocker| !relation_blocker_allows_queue(blocker)) + .collect::>(); + if !blockers.is_empty() { + return Err(TicketError::BlockingRelations(format_relation_blockers(&blockers))); + } + } let at = now_utc(); self.insert_event(conn, &ticket_id, &TicketEvent { kind: TicketEventKind::StateChanged, author: Some(change.author.clone().unwrap_or_else(default_author)), at: Some(at.clone()), status: None, from: Some(change.from), to: Some(change.to), reason: Some(change.reason), state_field: Some("state".to_string()), heading: Some(TicketEventKind::StateChanged.heading()), body: change.body, references: change.references, attributes: BTreeMap::new() })?; conn.execute("UPDATE typed_tickets SET workflow_state = ?3, workflow_state_explicit = 1, updated_at = ?4, status = CASE WHEN ?3 = 'closed' THEN 'closed' ELSE status END WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, to.as_str(), at]).map_err(sqlite_err)?; @@ -3342,24 +3570,98 @@ impl TicketBackend for SqliteTicketBackend { }) } - fn mark_intake_ready( - &self, - id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, - ) -> Result<()> { - self.add_intake_summary(id.clone(), summary)?; - self.set_workflow_state(id, change) - } - - fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()> { + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> Result { + validate_required_event_value("operation_key", &request.operation_key)?; self.with_write(|conn| { let ticket_id = self.resolve_ticket_id(conn, id)?; let ticket = self.load_ticket(conn, &ticket_id)?; - if ticket.meta.workflow_state != TicketWorkflowState::Ready { return Err(TicketError::Conflict(format!("Ticket state is {}; only ready Tickets can be queued", ticket.meta.workflow_state.as_str()))); } + if validate_mark_ready_replay(&ticket, &request)? { + return Ok(ticket); + } + let target = resolve_ready_target( + self.target_authority.as_ref(), + &self.workspace_id, + &ticket, + )?; + let fingerprint = mark_ready_fingerprint(&ticket, &request, &target); + if ticket.meta.workflow_state != TicketWorkflowState::Planning { + return Err(TicketError::StaleWorkflowState { + expected: TicketWorkflowState::Planning.as_str().to_owned(), + actual: ticket.meta.workflow_state.as_str().to_owned(), + }); + } + let reason = request + .reason + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or("implementation target validated") + .to_owned(); let at = now_utc(); - conn.execute("UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, queued_by, at]).map_err(sqlite_err)?; - self.insert_event(conn, &ticket_id, &TicketEvent { kind: TicketEventKind::StateChanged, author: Some(queued_by.to_string()), at: Some(at), status: None, from: Some("ready".to_string()), to: Some("queued".to_string()), reason: Some("queued".to_string()), state_field: Some("state".to_string()), heading: Some(TicketEventKind::StateChanged.heading()), body: MarkdownText::new(format!("Queued for Orchestrator by {queued_by}.")), references: Vec::new(), attributes: BTreeMap::new() }) + self.insert_event( + conn, + &ticket_id, + &TicketEvent { + kind: TicketEventKind::StateChanged, + author: Some(request.author.unwrap_or_else(default_author)), + at: Some(at.clone()), + status: None, + from: Some(TicketWorkflowState::Planning.as_str().to_owned()), + to: Some(TicketWorkflowState::Ready.as_str().to_owned()), + reason: Some(reason), + state_field: Some("state".to_owned()), + heading: Some(TicketEventKind::StateChanged.heading()), + body: MarkdownText::new(format!( + "Implementation target `{}` at selector `{}` was validated and the Ticket was marked ready.", + target.repository_id, target.ref_selector + )), + references: Vec::new(), + attributes: BTreeMap::from([ + ("operation_key".to_owned(), request.operation_key), + ("request_fingerprint".to_owned(), fingerprint), + ("repository_id".to_owned(), target.repository_id.clone()), + ("ref_selector".to_owned(), target.ref_selector.clone()), + ]), + }, + )?; + conn.execute( + "UPDATE typed_tickets SET workflow_state = 'ready', workflow_state_explicit = 1, repository_id = ?3, ref_selector = ?4, updated_at = ?5 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'planning'", + params![self.workspace_id, ticket_id, target.repository_id, target.ref_selector, at], + ) + .map_err(sqlite_err)?; + self.load_ticket(conn, &ticket_id) + }) + } + + fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()> { + validate_required_event_value("queued_by", queued_by)?; + self.with_write(|conn| { + let ticket_id = self.resolve_ticket_id(conn, id)?; + let ticket = self.load_ticket(conn, &ticket_id)?; + if ticket.meta.workflow_state != TicketWorkflowState::Ready { + return Err(TicketError::StaleWorkflowState { + expected: TicketWorkflowState::Ready.as_str().to_owned(), + actual: ticket.meta.workflow_state.as_str().to_owned(), + }); + } + let target = resolve_ready_target( + self.target_authority.as_ref(), + &self.workspace_id, + &ticket, + )?; + let blockers = ticket + .relations + .blockers + .iter() + .filter(|blocker| !relation_blocker_allows_queue(blocker)) + .cloned() + .collect::>(); + if !blockers.is_empty() { + return Err(TicketError::BlockingRelations(format_relation_blockers(&blockers))); + } + let at = now_utc(); + conn.execute("UPDATE typed_tickets SET workflow_state = 'queued', workflow_state_explicit = 1, queued_by = ?3, queued_at = ?4, repository_id = ?5, ref_selector = ?6, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2 AND workflow_state = 'ready'", params![self.workspace_id, ticket_id, queued_by, at, target.repository_id, target.ref_selector]).map_err(sqlite_err)?; + self.insert_event(conn, &ticket_id, &TicketEvent { kind: TicketEventKind::StateChanged, author: Some(queued_by.to_string()), at: Some(at.clone()), status: None, from: Some("ready".to_string()), to: Some("queued".to_string()), reason: Some("queued".to_string()), state_field: Some("state".to_string()), heading: Some(TicketEventKind::StateChanged.heading()), body: MarkdownText::new(format!("Queued for Orchestrator by {queued_by}.")), references: Vec::new(), attributes: BTreeMap::from([("queued_by".to_owned(), queued_by.to_owned()), ("queued_at".to_owned(), at), ("repository_id".to_owned(), target.repository_id), ("ref_selector".to_owned(), target.ref_selector)]) }) }) } @@ -3676,6 +3978,15 @@ impl TicketBackend for LocalTicketBackend { let _lock = self.acquire_lock()?; let dir = self.find_ticket_dir(&id)?; let item = dir.join("item.md"); + if edit.target.is_some() { + let current = self.ticket_workflow_state_from_dir(&dir)?; + if current != TicketWorkflowState::Planning { + return Err(TicketError::Conflict(format!( + "ticket implementation target is locked after planning (current state: {})", + current.as_str() + ))); + } + } let mut content = fs::read_to_string(&item).map_err(|e| io_err(&item, e))?; let mut body_edit_audit = TicketBodyEditAudit::None; let mut updates = Vec::new(); @@ -3892,13 +4203,7 @@ impl TicketBackend for LocalTicketBackend { change.to )) })?; - if !TicketWorkflowState::is_role_transition(from, to) { - return Err(TicketError::Conflict(format!( - "workflow_state transition {} -> {} is not allowed through set_workflow_state; use dedicated planning-ready or queue APIs for gated transitions", - from.as_str(), - to.as_str() - ))); - } + validate_generic_state_change(from, to)?; let _lock = self.acquire_lock()?; let dir = self.find_ticket_dir(&id)?; if from == TicketWorkflowState::Queued && to == TicketWorkflowState::InProgress { @@ -3916,43 +4221,58 @@ impl TicketBackend for LocalTicketBackend { self.apply_workflow_state_change(&dir, from, to, change, &[]) } - fn mark_intake_ready( - &self, - id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, - ) -> Result<()> { - let from = TicketWorkflowState::parse(&change.from).ok_or_else(|| { - TicketError::Conflict(format!( - "invalid workflow_state transition source: {}", - change.from - )) - })?; - let to = TicketWorkflowState::parse(&change.to).ok_or_else(|| { - TicketError::Conflict(format!( - "invalid workflow_state transition target: {}", - change.to - )) - })?; - if !TicketWorkflowState::is_planning_ready_transition(from, to) { - return Err(TicketError::Conflict(format!( - "mark_intake_ready only allows state planning -> ready, got {} -> {}", - from.as_str(), - to.as_str() - ))); - } + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> Result { + validate_required_event_value("operation_key", &request.operation_key)?; let _lock = self.acquire_lock()?; let dir = self.find_ticket_dir(&id)?; - let current = self.ticket_workflow_state_from_dir(&dir)?; - if current != from { - return Err(TicketError::Conflict(format!( - "state changed concurrently: expected `{}`, found `{}`", - from.as_str(), - current.as_str() - ))); + let ticket = self.ticket_from_dir(&dir)?; + if validate_mark_ready_replay(&ticket, &request)? { + return Ok(ticket); } - self.append_intake_summary_event(&dir, &summary)?; - self.apply_workflow_state_change(&dir, from, to, change, &[]) + let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; + let fingerprint = mark_ready_fingerprint(&ticket, &request, &target); + if ticket.meta.workflow_state != TicketWorkflowState::Planning { + return Err(TicketError::StaleWorkflowState { + expected: TicketWorkflowState::Planning.as_str().to_owned(), + actual: ticket.meta.workflow_state.as_str().to_owned(), + }); + } + let reason = request + .reason + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .unwrap_or("implementation target validated"); + let mut change = TicketStateChange::new( + TicketWorkflowState::Planning.as_str(), + TicketWorkflowState::Ready.as_str(), + reason, + MarkdownText::new(format!( + "Implementation target `{}` at selector `{}` was validated and the Ticket was marked ready.", + target.repository_id, target.ref_selector + )), + ); + change.author = request.author.or_else(|| Some(default_author())); + self.append_state_changed_event_with_attributes( + &dir, + &change, + Some("state"), + &[ + ("operation_key", request.operation_key.as_str()), + ("request_fingerprint", fingerprint.as_str()), + ("repository_id", target.repository_id.as_str()), + ("ref_selector", target.ref_selector.as_str()), + ], + )?; + self.set_frontmatter_fields( + &dir.join("item.md"), + &[ + ("state", TicketWorkflowState::Ready.as_str()), + ("repository_id", target.repository_id.as_str()), + ("ref_selector", target.ref_selector.as_str()), + ], + )?; + self.ticket_from_dir(&dir) } fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()> { @@ -3961,14 +4281,16 @@ impl TicketBackend for LocalTicketBackend { let dir = self.find_ticket_dir(&id)?; let item = dir.join("item.md"); let meta = ticket_meta_for_dir(&dir, read_item_file(&item)?.frontmatter)?; + let ticket = self.ticket_from_dir(&dir)?; + let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; let blockers = self.relation_blockers_for_meta(&meta)?; let active_blockers = blockers .into_iter() .filter(|blocker| !relation_blocker_allows_queue(blocker)) .collect::>(); if !active_blockers.is_empty() { - return Err(TicketError::Conflict(format!( - "ticket {} has unresolved blocking relation(s): {}", + return Err(TicketError::BlockingRelations(format!( + "{}: {}", meta.id, format_relation_blockers(&active_blockers) ))); @@ -3986,7 +4308,12 @@ impl TicketBackend for LocalTicketBackend { TicketWorkflowState::Ready, TicketWorkflowState::Queued, change, - &[("queued_by", queued_by), ("queued_at", at.as_str())], + &[ + ("queued_by", queued_by), + ("queued_at", at.as_str()), + ("repository_id", target.repository_id.as_str()), + ("ref_selector", target.ref_selector.as_str()), + ], ) } @@ -4723,9 +5050,17 @@ fn invalid_ticket_record_reason(error: &TicketError) -> &'static str { TicketError::Locked { .. } => "ticket backend is locked", TicketError::Sqlite(_) => "could not read ticket record", TicketError::NotFound(_) => "ticket record is missing", - TicketError::Ambiguous { .. } | TicketError::Conflict(_) => { - "invalid ticket record metadata" - } + TicketError::Ambiguous { .. } + | TicketError::Conflict(_) + | TicketError::MissingTargetRepository + | TicketError::UnknownTargetRepository(_) + | TicketError::MissingTargetSelector(_) + | TicketError::InvalidTargetSelector { .. } + | TicketError::TargetAuthorityUnavailable + | TicketError::StaleWorkflowState { .. } + | TicketError::InvalidWorkflowTransition { .. } + | TicketError::BlockingRelations(_) + | TicketError::OperationFingerprintMismatch { .. } => "invalid ticket record metadata", } } @@ -6064,8 +6399,32 @@ mod tests { use super::*; use tempfile::TempDir; + #[derive(Debug)] + struct TestTargetAuthority; + + impl TicketTargetAuthority for TestTargetAuthority { + fn resolve_target( + &self, + _workspace_id: &str, + repository_id: Option<&str>, + ref_selector: Option<&str>, + ) -> Result { + let repository_id = repository_id.unwrap_or("main"); + if repository_id == "unknown" { + return Err(TicketError::UnknownTargetRepository( + repository_id.to_owned(), + )); + } + Ok(ResolvedTicketTarget { + repository_id: repository_id.to_owned(), + ref_selector: ref_selector.unwrap_or("develop").to_owned(), + }) + } + } + fn backend(dir: &TempDir) -> LocalTicketBackend { LocalTicketBackend::new(dir.path().join("tickets")) + .with_target_authority(Arc::new(TestTargetAuthority)) } fn assert_ticket_target_edit_semantics(backend: &B) { @@ -6574,6 +6933,91 @@ state: planning assert_ticket_target_edit_semantics(&backend); } + #[test] + fn sqlite_mark_ready_and_queue_enforce_target_and_blockers_atomically() { + let tmp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::open(tmp.path().join("workspace.db"), "workspace-test") + .unwrap() + .with_target_authority(Arc::new(TestTargetAuthority)); + let mut dependency = NewTicket::new("Dependency"); + dependency.repository_id = Some("main".to_owned()); + let dependency = backend.create(dependency).unwrap(); + let mut implementation = NewTicket::new("Implementation"); + implementation.repository_id = Some("main".to_owned()); + let implementation = backend.create(implementation).unwrap(); + backend + .add_ticket_relation( + TicketIdOrSlug::Id(implementation.id.clone()), + NewTicketRelation { + kind: TicketRelationKind::DependsOn, + target: dependency.id.clone(), + note: None, + author: Some("test".to_owned()), + }, + ) + .unwrap(); + let request = TicketMarkReady { + operation_key: "sqlite-ready".to_owned(), + reason: Some("target accepted".to_owned()), + author: Some("test".to_owned()), + }; + let ready = backend + .mark_ready( + TicketIdOrSlug::Id(implementation.id.clone()), + request.clone(), + ) + .unwrap(); + assert_eq!(ready.meta.ref_selector.as_deref(), Some("develop")); + let replay = backend + .mark_ready(TicketIdOrSlug::Id(implementation.id.clone()), request) + .unwrap(); + assert_eq!( + replay + .events + .iter() + .filter(|event| event.attributes.contains_key("operation_key")) + .count(), + 1 + ); + assert!(matches!( + backend.queue_ready( + TicketIdOrSlug::Id(implementation.id.clone()), + "orchestrator", + ), + Err(TicketError::BlockingRelations(_)) + )); + let after_rejection = backend + .show(TicketIdOrSlug::Id(implementation.id.clone())) + .unwrap(); + assert_eq!( + after_rejection.meta.workflow_state, + TicketWorkflowState::Ready + ); + assert!(!after_rejection.events.iter().any(|event| { + event.from.as_deref() == Some("ready") && event.to.as_deref() == Some("queued") + })); + backend + .close( + TicketIdOrSlug::Id(dependency.id), + MarkdownText::new("resolved"), + ) + .unwrap(); + backend + .queue_ready( + TicketIdOrSlug::Id(implementation.id.clone()), + "orchestrator", + ) + .unwrap(); + assert_eq!( + backend + .show(TicketIdOrSlug::Id(implementation.id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Queued + ); + } + #[test] fn sqlite_backend_persists_and_edits_ticket_target() { let tmp = TempDir::new().unwrap(); @@ -6917,24 +7361,24 @@ state: planning fn add_event_status_and_close_preserve_local_layout() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); - let ticket = backend.create(NewTicket::new("Flow Ticket")).unwrap(); + let mut input = NewTicket::new("Flow Ticket"); + input.repository_id = Some("main".to_owned()); + let ticket = backend.create(input).unwrap(); backend .add_event( TicketIdOrSlug::Id(ticket.id.clone()), NewTicketEvent::new(TicketEventKind::Plan, "Implementation plan."), ) .unwrap(); - let mut summary = TicketIntakeSummary::new("Ready for queue."); - summary.author = Some("test".to_string()); - let mut change = TicketStateChange::new( - "planning", - "ready", - "ready_for_queue", - MarkdownText::new("Ready for queue."), - ); - change.author = Some("test".to_string()); backend - .mark_intake_ready(TicketIdOrSlug::Id(ticket.id.clone()), summary, change) + .mark_ready( + TicketIdOrSlug::Id(ticket.id.clone()), + TicketMarkReady { + operation_key: "test-flow-ready".to_owned(), + reason: Some("ready_for_queue".to_owned()), + author: Some("test".to_owned()), + }, + ) .unwrap(); let current_item = tmp.path().join("tickets").join(&ticket.id).join("item.md"); assert!(current_item.exists()); @@ -7127,6 +7571,8 @@ state: planning let mut ready_input = NewTicket::new("Ready Workflow"); ready_input.workflow_state = Some(TicketWorkflowState::Ready); + ready_input.repository_id = Some("main".to_owned()); + ready_input.ref_selector = Some("develop".to_owned()); let ready = backend.create(ready_input).unwrap(); backend .queue_ready(TicketIdOrSlug::Id(ready.id.clone()), "workspace-panel") @@ -7192,41 +7638,58 @@ state: planning } #[test] - fn mark_intake_ready_records_summary_and_state_change() { + fn mark_ready_resolves_target_and_is_idempotent() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); - let ticket = backend.create(NewTicket::new("Planning Ready")).unwrap(); - let mut summary = TicketIntakeSummary::new("Concise accepted requirements."); - summary.author = Some("intake".to_string()); - let mut change = - TicketStateChange::new("planning", "ready", "accepted", "Ticket is ready to queue."); - change.author = Some("intake".to_string()); + let mut input = NewTicket::new("Planning Ready"); + input.repository_id = Some("main".to_owned()); + let ticket = backend.create(input).unwrap(); + let request = TicketMarkReady { + operation_key: "ready-op-1".to_owned(), + reason: Some("accepted".to_owned()), + author: Some("intake".to_owned()), + }; - backend - .mark_intake_ready(TicketIdOrSlug::Id(ticket.id.clone()), summary, change) + let first = backend + .mark_ready(TicketIdOrSlug::Id(ticket.id.clone()), request.clone()) .unwrap(); - let record = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); - assert_eq!(record.meta.workflow_state, TicketWorkflowState::Ready); - assert!( - record + let second = backend + .mark_ready(TicketIdOrSlug::Id(ticket.id.clone()), request) + .unwrap(); + assert_eq!(first.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!(first.meta.repository_id.as_deref(), Some("main")); + assert_eq!(first.meta.ref_selector.as_deref(), Some("develop")); + assert_eq!(first.events, second.events); + assert_eq!( + first .events .iter() - .any(|event| event.kind == TicketEventKind::IntakeSummary) + .filter(|event| { + event.kind == TicketEventKind::StateChanged + && event.from.as_deref() == Some("planning") + && event.to.as_deref() == Some("ready") + }) + .count(), + 1 ); - assert!(record.events.iter().any(|event| { - event.kind == TicketEventKind::StateChanged - && event.state_field.as_deref() == Some("state") - && event.from.as_deref() == Some("planning") - && event.to.as_deref() == Some("ready") - })); + assert!(matches!( + backend.mark_ready( + TicketIdOrSlug::Id(ticket.id), + TicketMarkReady { + operation_key: "ready-op-1".to_owned(), + reason: Some("different".to_owned()), + author: Some("intake".to_owned()), + }, + ), + Err(TicketError::OperationFingerprintMismatch { .. }) + )); } #[test] fn close_sets_state_closed() { let tmp = TempDir::new().unwrap(); let backend = backend(&tmp); - let mut input = NewTicket::new("Close Workflow"); - input.workflow_state = Some(TicketWorkflowState::Queued); + let input = NewTicket::new("Close Workflow"); let ticket = backend.create(input).unwrap(); backend diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 410f7556..158f9d8b 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -16,8 +16,9 @@ use crate::{ NewTicket, NewTicketEvent, NewTicketRelation, OrchestrationPlanKind, OrchestrationPlanRecord, Result as TicketResult, Ticket, TicketBackend, TicketBodyReplacement, TicketDoctorDiagnostic, TicketDoctorReport, TicketDoctorSeverity, TicketError, TicketEventKind, TicketIdOrSlug, - TicketIntakeSummary, TicketListState, TicketRef, TicketRelation, TicketRelationKind, - TicketRelationView, TicketStateChange, TicketSummary, TicketWorkflowState, default_author, + TicketIntakeSummary, TicketListState, TicketMarkReady, TicketRef, TicketRelation, + TicketRelationKind, TicketRelationView, TicketStateChange, TicketSummary, TicketWorkflowState, + default_author, }; const DEFAULT_LIST_LIMIT: usize = 50; @@ -42,7 +43,7 @@ pub const TICKET_BASE_TOOL_NAMES: [&str; 14] = [ "TicketPlan", "TicketDecision", "TicketImplementationReport", - "TicketIntakeReady", + "TicketMarkReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -77,7 +78,7 @@ pub const TICKET_TOOL_NAMES: [&str; 19] = [ "TicketPlan", "TicketDecision", "TicketImplementationReport", - "TicketIntakeReady", + "TicketMarkReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -106,7 +107,7 @@ pub const TICKET_MUTATING_TOOL_NAMES: [&str; 13] = [ "TicketPlan", "TicketDecision", "TicketImplementationReport", - "TicketIntakeReady", + "TicketMarkReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -132,9 +133,9 @@ const PLAN_DESCRIPTION: &str = "Append a typed Ticket plan event. `body` is Mark const DECISION_DESCRIPTION: &str = "Append a typed Ticket decision event. `body` is Markdown."; const IMPLEMENTATION_REPORT_DESCRIPTION: &str = "Append a typed Ticket implementation_report event. `body` is Markdown."; -const INTAKE_READY_DESCRIPTION: &str = "Mark an existing Ticket planning lane ready through the typed \ -Ticket backend. The tool appends a bounded `intake_summary`, appends a typed `state_changed` event \ -for `state`, and transitions state to `ready`."; +const MARK_READY_DESCRIPTION: &str = "Mark a planning Ticket ready through the typed Ticket backend. \ +The backend atomically validates and normalizes the persisted repository/ref target, records one typed \ +state_changed event, and transitions planning -> ready. `reason` is optional."; const QUEUE_DESCRIPTION: &str = "Queue a ready Ticket for Orchestrator routing through the typed \ Ticket backend. The backend performs the gated ready -> queued transition, records queued_by/queued_at, \ and rejects unresolved blocking relations."; @@ -174,7 +175,7 @@ fn base_tool_description(name: &str) -> &'static str { "TicketPlan" => PLAN_DESCRIPTION, "TicketDecision" => DECISION_DESCRIPTION, "TicketImplementationReport" => IMPLEMENTATION_REPORT_DESCRIPTION, - "TicketIntakeReady" => INTAKE_READY_DESCRIPTION, + "TicketMarkReady" => MARK_READY_DESCRIPTION, "TicketQueue" => QUEUE_DESCRIPTION, "TicketWorkflowState" => WORKFLOW_STATE_DESCRIPTION, "TicketClose" => CLOSE_DESCRIPTION, @@ -305,13 +306,8 @@ impl TicketBackend for TicketToolBackend { self.backend.set_workflow_state(id, change) } - fn mark_intake_ready( - &self, - id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, - ) -> TicketResult<()> { - self.backend.mark_intake_ready(id, summary, change) + fn mark_ready(&self, id: TicketIdOrSlug, request: TicketMarkReady) -> TicketResult { + self.backend.mark_ready(id, request) } fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> TicketResult<()> { @@ -559,17 +555,12 @@ struct TicketThreadEventParams { } #[derive(Debug, Deserialize, schemars::JsonSchema)] -struct TicketIntakeReadyParams { +struct TicketMarkReadyParams { /// Ticket id. ticket: String, - /// Concise bounded intake summary to append as a typed intake_summary event. - intake_summary: String, - /// Reason attached to the state_changed event. Defaults to `planning_ready`. + /// Optional reason attached to the state_changed event. #[serde(default)] reason: Option, - /// Optional state_changed body. If omitted, a concise default is used. - #[serde(default)] - state_change_body: Option, } #[derive(Debug, Deserialize, schemars::JsonSchema)] @@ -837,7 +828,7 @@ struct TicketImplementationReportTool { } #[derive(Clone)] -struct TicketIntakeReadyTool { +struct TicketMarkReadyTool { backend: TicketToolBackend, } @@ -899,6 +890,23 @@ impl Tool for TicketCreateTool { _ctx: llm_engine::tool::ToolExecutionContext, ) -> Result { let params: TicketCreateParams = parse_input("TicketCreate", input_json)?; + if params + .state + .is_some_and(|state| !matches!(state.into_state(), TicketWorkflowState::Planning)) + { + return Err(backend_error( + "TicketCreate", + TicketError::InvalidWorkflowTransition { + from: "creation".to_owned(), + to: params + .state + .expect("checked non-planning state") + .into_state() + .as_str() + .to_owned(), + }, + )); + } let mut input = NewTicket::new(params.title); if let Some(body) = params.body { input.body = MarkdownText::new(body); @@ -1115,40 +1123,33 @@ impl_ticket_thread_event_tool!( ); #[async_trait] -impl Tool for TicketIntakeReadyTool { +impl Tool for TicketMarkReadyTool { async fn execute( &self, input_json: &str, - _ctx: llm_engine::tool::ToolExecutionContext, + ctx: llm_engine::tool::ToolExecutionContext, ) -> Result { - let params: TicketIntakeReadyParams = parse_input("TicketIntakeReady", input_json)?; - let from = TicketWorkflowState::Planning; - let reason = params - .reason - .unwrap_or_else(|| "planning_ready".to_string()); - let body = params.state_change_body.unwrap_or_else(|| { - self.backend - .default_intake_ready_state_change_body(from.as_str()) - }); - let mut summary = TicketIntakeSummary::new(params.intake_summary); - summary.author = None; - let mut change = TicketStateChange::new( - from.as_str(), - TicketWorkflowState::Ready.as_str(), - reason, - body, - ); - change.author = None; - self.backend - .mark_intake_ready( + let params: TicketMarkReadyParams = parse_input("TicketMarkReady", input_json)?; + let ticket = self + .backend + .mark_ready( TicketIdOrSlug::Query(params.ticket.clone()), - summary, - change, + TicketMarkReady { + operation_key: format!("ticket-mark-ready:{}", ctx.call_id), + reason: params.reason, + author: None, + }, ) - .map_err(|error| backend_error("TicketIntakeReady", error))?; + .map_err(|error| backend_error("TicketMarkReady", error))?; Ok(json_output( format!("Marked ticket {} state ready", params.ticket), - json!({ "ticket": params.ticket, "state": "ready", "ok": true }), + json!({ + "ticket": ticket.meta.id, + "state": ticket.meta.workflow_state.as_str(), + "repository_id": ticket.meta.repository_id, + "ref_selector": ticket.meta.ref_selector, + "ok": true + }), )) } } @@ -1726,7 +1727,7 @@ fn input_schema(name: &str) -> Value { "TicketComment" | "TicketPlan" | "TicketDecision" | "TicketImplementationReport" => { serde_json::to_value(schemars::schema_for!(TicketThreadEventParams)) } - "TicketIntakeReady" => serde_json::to_value(schemars::schema_for!(TicketIntakeReadyParams)), + "TicketMarkReady" => serde_json::to_value(schemars::schema_for!(TicketMarkReadyParams)), "TicketQueue" => serde_json::to_value(schemars::schema_for!(TicketQueueParams)), "TicketWorkflowState" => { serde_json::to_value(schemars::schema_for!(TicketWorkflowStateParams)) @@ -1774,7 +1775,7 @@ impl_from_backend!(TicketCommentTool); impl_from_backend!(TicketPlanTool); impl_from_backend!(TicketDecisionTool); impl_from_backend!(TicketImplementationReportTool); -impl_from_backend!(TicketIntakeReadyTool); +impl_from_backend!(TicketMarkReadyTool); impl_from_backend!(TicketQueueTool); impl_from_backend!(TicketWorkflowStateTool); impl_from_backend!(TicketCloseTool); @@ -1801,7 +1802,7 @@ pub fn ticket_tools(backend: impl Into) -> Vec("TicketIntakeReady", backend.clone()), + tool_definition::("TicketMarkReady", backend.clone()), tool_definition::("TicketQueue", backend.clone()), tool_definition::("TicketWorkflowState", backend.clone()), tool_definition::("TicketClose", backend.clone()), @@ -1826,8 +1827,26 @@ mod tests { use super::*; use tempfile::TempDir; + #[derive(Debug)] + struct TestTargetAuthority; + + impl crate::TicketTargetAuthority for TestTargetAuthority { + fn resolve_target( + &self, + _workspace_id: &str, + repository_id: Option<&str>, + ref_selector: Option<&str>, + ) -> crate::Result { + Ok(crate::ResolvedTicketTarget { + repository_id: repository_id.unwrap_or("main").to_owned(), + ref_selector: ref_selector.unwrap_or("develop").to_owned(), + }) + } + } + fn backend(temp: &TempDir) -> LocalTicketBackend { LocalTicketBackend::new(temp.path().join("tickets")) + .with_target_authority(Arc::new(TestTargetAuthority)) } fn tool(definition: ToolDefinition) -> Arc { @@ -1877,7 +1896,7 @@ mod tests { "TicketPlan", "TicketDecision", "TicketImplementationReport", - "TicketIntakeReady", + "TicketMarkReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -2460,16 +2479,17 @@ mod tests { async fn ticket_workflow_tools_mark_ready_and_transition_state() { let temp = TempDir::new().unwrap(); let backend = backend(&temp); - let created = backend.create(NewTicket::new("Workflow Tool")).unwrap(); - let intake_ready = tool_by_name(backend.clone(), "TicketIntakeReady"); + let mut input = NewTicket::new("Workflow Tool"); + input.repository_id = Some("main".to_owned()); + let created = backend.create(input).unwrap(); + let intake_ready = tool_by_name(backend.clone(), "TicketMarkReady"); let workflow = tool_by_name(backend.clone(), "TicketWorkflowState"); intake_ready .execute( &json!({ "ticket": created.id.clone(), - "intake_summary": "Requirements accepted; implementation can be queued.", - "author": "intake-worker" + "reason": "requirements accepted" }) .to_string(), Default::default(), @@ -2512,12 +2532,12 @@ mod tests { let record = backend.show(TicketIdOrSlug::Id(created.id)).unwrap(); assert_eq!(record.meta.workflow_state, TicketWorkflowState::Done); - assert!( - record - .events - .iter() - .any(|event| event.kind == TicketEventKind::IntakeSummary) - ); + assert!(record.events.iter().any(|event| { + event.kind == TicketEventKind::StateChanged + && event.from.as_deref() == Some("planning") + && event.to.as_deref() == Some("ready") + && event.attributes.contains_key("request_fingerprint") + })); let transitions = record .events .iter() @@ -2661,7 +2681,11 @@ mod tests { ) .await .unwrap_err(); - assert!(ready_error.to_string().contains("not allowed")); + assert!( + ready_error + .to_string() + .contains("invalid ticket workflow transition") + ); let mut done_input = NewTicket::new("Backward Bypass"); done_input.workflow_state = Some(TicketWorkflowState::Done); @@ -2680,7 +2704,11 @@ mod tests { ) .await .unwrap_err(); - assert!(backward_error.to_string().contains("not allowed")); + assert!( + backward_error + .to_string() + .contains("invalid ticket workflow transition") + ); let mut queued_input = NewTicket::new("Skip Bypass"); queued_input.workflow_state = Some(TicketWorkflowState::Queued); @@ -2699,17 +2727,21 @@ mod tests { ) .await .unwrap_err(); - assert!(skip_error.to_string().contains("not allowed")); + assert!( + skip_error + .to_string() + .contains("invalid ticket workflow transition") + ); } #[tokio::test] - async fn ticket_intake_ready_tool_rejects_non_planning_ticket() { + async fn ticket_mark_ready_tool_rejects_non_planning_ticket() { let temp = TempDir::new().unwrap(); let backend = backend(&temp); let mut input = NewTicket::new("Already Ready"); input.workflow_state = Some(TicketWorkflowState::Ready); let created = backend.create(input).unwrap(); - let intake_ready = tool_by_name(backend.clone(), "TicketIntakeReady"); + let intake_ready = tool_by_name(backend.clone(), "TicketMarkReady"); let error = intake_ready .execute( @@ -2723,7 +2755,7 @@ mod tests { .await .unwrap_err(); - assert!(error.to_string().contains("state changed concurrently")); + assert!(error.to_string().contains("stale ticket workflow state")); let record = backend.show(TicketIdOrSlug::Id(created.id)).unwrap(); assert_eq!(record.meta.workflow_state, TicketWorkflowState::Ready); assert!(!record.events.iter().any(|event| { @@ -2867,7 +2899,7 @@ mod tests { "TicketPlan", "TicketDecision", "TicketImplementationReport", - "TicketIntakeReady", + "TicketMarkReady", "TicketQueue", "TicketRelationRecord", "TicketOrchestrationPlanRecord", diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index ebb07952..67d3ea01 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -18,7 +18,7 @@ use crate::runtime::dir::RuntimeDir; use crate::segment_log_sink::SegmentLogSink; use crate::shared_state::WorkerSharedState; use crate::shutdown_after_idle::{ - ShutdownAfterIdleRequest, TicketIntakeReadyShutdownHook, is_ticket_intake_role, + ShutdownAfterIdleRequest, TicketMarkReadyShutdownHook, is_ticket_intake_role, take_shutdown_request_after_status, }; use crate::spawn::registry::SpawnedWorkerRegistry; @@ -423,11 +423,11 @@ impl WorkerController { .await?; // Intake role Workers self-terminate only after a successful - // TicketIntakeReady turn has fully settled back to Idle. The request + // TicketMarkReady turn has fully settled back to Idle. The request // is transient controller state, not model-visible context or ticket // claim metadata. let shutdown_after_idle = ShutdownAfterIdleRequest::default(); - worker.add_post_tool_call_hook(TicketIntakeReadyShutdownHook::new( + worker.add_post_tool_call_hook(TicketMarkReadyShutdownHook::new( shutdown_after_idle.clone(), is_ticket_intake_role(worker.runtime_ticket_role()), )); diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index ac461c13..32283d6f 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -381,7 +381,6 @@ const READ_ONLY_TOOL_NAMES: &[&str] = &["QueryTicket", "ShowTicket"]; const AUTHORING_TOOL_NAMES: &[&str] = &[ "TicketCreate", "TicketEditItem", - "TicketQueue", "TicketClose", "TicketRelationRecord", "TicketRelationRemove", @@ -389,7 +388,7 @@ const AUTHORING_TOOL_NAMES: &[&str] = &[ const THREAD_TOOL_NAMES: &[&str] = &["TicketComment"]; -const INTAKE_TOOL_NAMES: &[&str] = &["TicketIntakeReady"]; +const INTAKE_TOOL_NAMES: &[&str] = &["TicketMarkReady"]; #[cfg(test)] const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ @@ -398,7 +397,6 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ "QueryTicket", "ShowTicket", "TicketComment", - "TicketQueue", "TicketClose", "TicketRelationRecord", "TicketRelationRemove", @@ -409,6 +407,7 @@ const WORKFLOW_TOOL_NAMES: &[&str] = &[ "QueryTicket", "ShowTicket", "TicketComment", + "TicketQueue", "TicketWorkflowState", "TicketClose", "TicketDependencyCheck", @@ -419,6 +418,7 @@ const WORKFLOW_TOOL_NAMES: &[&str] = &[ ]; const WORKFLOW_ADDITIONAL_TOOL_NAMES: &[&str] = &[ + "TicketQueue", "TicketWorkflowState", "TicketClose", "TicketDependencyCheck", @@ -894,16 +894,15 @@ impl WorkspaceHttpTicketBackend { TicketError::Conflict(format!("serialize Ticket workflow change: {error}")) })?), ), - TicketBackendOperation::MarkIntakeReady { - id, - summary, - change, - } => Self::request_unit( + TicketBackendOperation::MarkReady { id, request } => Self::request( client, WorkspaceRequestMethod::Post, - format!("{base}/{}/intake-ready", Self::ticket_path(&id)), - Some(serde_json::json!({ "summary": summary, "change": change })), - ), + format!("{base}/{}/workflow/mark-ready", Self::ticket_path(&id)), + Some(serde_json::to_value(request).map_err(|error| { + TicketError::Conflict(format!("serialize Ticket mark-ready request: {error}")) + })?), + ) + .map(TicketBackendOperationResult::Ticket), TicketBackendOperation::QueueReady { id, .. } => Self::request_unit( client, WorkspaceRequestMethod::Post, @@ -1119,22 +1118,15 @@ impl TicketBackend for WorkspaceHttpTicketBackend { } } - fn mark_intake_ready( + fn mark_ready( &self, id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, - ) -> TicketResult<()> { - match self.invoke(TicketBackendOperation::MarkIntakeReady { - id, - summary, - change, - })? { - TicketBackendOperationResult::Unit => Ok(()), - other => Err(TicketError::Conflict(format!( - "unexpected ticket backend response: {other:?}" - ))), - } + request: ticket::TicketMarkReady, + ) -> TicketResult { + expect_ticket_result!( + self.invoke(TicketBackendOperation::MarkReady { id, request }), + TicketBackendOperationResult::Ticket + ) } fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> TicketResult<()> { @@ -1303,13 +1295,13 @@ mod tests { assert_eq!(show.name, "ShowTicket"); assert!(show.input_schema["properties"]["event_limit"].is_object()); let tool_names = TicketFeatureAccess::workspace_authoring().tool_names(); - assert_eq!(tool_names.len(), 9); + assert_eq!(tool_names.len(), 8); assert!( tool_names.len() < 13, "authoring catalog must stay below the prior broad catalog" ); let workflow_names = TicketFeatureAccess::workflow().tool_names(); - assert_eq!(workflow_names.len(), 10); + assert_eq!(workflow_names.len(), 11); assert!( workflow_names.len() < 12, "workflow catalog must stay below the prior broad catalog" @@ -1390,7 +1382,7 @@ mod tests { .collect::>(); assert!(workspace_tools.contains(&"TicketCreate")); assert!(workspace_tools.contains(&"TicketEditItem")); - assert!(workspace_tools.contains(&"TicketQueue")); + assert!(!workspace_tools.contains(&"TicketQueue")); assert!(!workspace_tools.contains(&"TicketWorkflowState")); let orchestration = @@ -1406,7 +1398,7 @@ mod tests { assert!(orchestration_tools.contains(&"TicketRelationRecord")); assert!(orchestration_tools.contains(&"TicketOrchestrationPlanRecord")); assert!(!orchestration_tools.contains(&"TicketEditItem")); - assert!(!orchestration_tools.contains(&"TicketQueue")); + assert!(orchestration_tools.contains(&"TicketQueue")); let work_report = ticket_tools_feature_with_access(temp.path(), TicketFeatureAccess::work_report()); @@ -1519,8 +1511,8 @@ language = "Japanese" assert_eq!(installed, WORKSPACE_AUTHORING_TOOL_NAMES); assert!(installed.iter().any(|tool| *tool == "TicketCreate")); assert!(installed.iter().any(|tool| *tool == "TicketEditItem")); - assert!(installed.iter().any(|tool| *tool == "TicketQueue")); - assert!(!installed.iter().any(|tool| *tool == "TicketIntakeReady")); + assert!(!installed.iter().any(|tool| *tool == "TicketQueue")); + assert!(!installed.iter().any(|tool| *tool == "TicketMarkReady")); assert!(!installed.iter().any(|tool| *tool == "TicketWorkflowState")); assert!( !installed diff --git a/crates/worker/src/shutdown_after_idle.rs b/crates/worker/src/shutdown_after_idle.rs index bae83158..2f4d1fb3 100644 --- a/crates/worker/src/shutdown_after_idle.rs +++ b/crates/worker/src/shutdown_after_idle.rs @@ -9,7 +9,7 @@ use ticket::config::TicketRole; use crate::hook::{Hook, HookPostToolAction, PostToolCall, ToolResultSummary}; -const TICKET_INTAKE_READY_TOOL_NAME: &str = "TicketIntakeReady"; +const TICKET_MARK_READY_TOOL_NAME: &str = "TicketMarkReady"; #[derive(Clone, Default)] pub(crate) struct ShutdownAfterIdleRequest { @@ -42,12 +42,12 @@ pub(crate) fn take_shutdown_request_after_status( status == WorkerStatus::Idle && shutdown_after_idle.take() } -pub(crate) struct TicketIntakeReadyShutdownHook { +pub(crate) struct TicketMarkReadyShutdownHook { shutdown_after_idle: ShutdownAfterIdleRequest, eligible_ticket_intake_role: bool, } -impl TicketIntakeReadyShutdownHook { +impl TicketMarkReadyShutdownHook { pub(crate) fn new( shutdown_after_idle: ShutdownAfterIdleRequest, eligible_ticket_intake_role: bool, @@ -60,7 +60,7 @@ impl TicketIntakeReadyShutdownHook { fn observe_tool_result(&self, info: &ToolResultSummary) { if self.eligible_ticket_intake_role - && info.tool_name == TICKET_INTAKE_READY_TOOL_NAME + && info.tool_name == TICKET_MARK_READY_TOOL_NAME && !info.is_error { self.shutdown_after_idle.request(); @@ -69,7 +69,7 @@ impl TicketIntakeReadyShutdownHook { } #[async_trait] -impl Hook for TicketIntakeReadyShutdownHook { +impl Hook for TicketMarkReadyShutdownHook { async fn call(&self, info: &ToolResultSummary) -> HookPostToolAction { self.observe_tool_result(info); HookPostToolAction::Continue @@ -98,9 +98,9 @@ mod tests { #[test] fn successful_ticket_intake_ready_schedules_shutdown_after_idle_for_intake_role() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); + let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); - hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, false)); + hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, false)); assert!(request.is_requested()); assert!(request.take()); @@ -110,9 +110,9 @@ mod tests { #[test] fn failed_ticket_intake_ready_does_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); + let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); - hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, true)); + hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, true)); assert!(!request.is_requested()); } @@ -120,9 +120,9 @@ mod tests { #[test] fn non_intake_role_does_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketIntakeReadyShutdownHook::new(request.clone(), false); + let hook = TicketMarkReadyShutdownHook::new(request.clone(), false); - hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, false)); + hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, false)); assert!(!request.is_requested()); } @@ -130,7 +130,7 @@ mod tests { #[test] fn other_successful_tools_do_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); + let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); hook.observe_tool_result(&tool_result("ShowTicket", false)); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index c2d7654a..7c91bd5f 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1060,11 +1060,18 @@ impl WorkspaceApi { &self, request: &WorkerSpawnRequest, ) -> ApiResult<()> { - let selected_repository_id = + let (selected_repository_id, selected_ref_selector) = if let Some(working_directory) = request.resolved_working_directory_request.as_ref() { let repository_id = working_directory.repository.id.as_str(); self.require_workspace_repository(repository_id)?; - Some(repository_id.to_string()) + ( + Some(repository_id.to_string()), + working_directory + .repository + .selector + .as_deref() + .map(str::to_owned), + ) } else if let Some(claim) = request.resolved_working_directory.as_ref() { let workdir = self .store @@ -1076,20 +1083,48 @@ impl WorkspaceApi { ))) })?; self.require_workspace_repository(&workdir.repository_id)?; - Some(workdir.repository_id) + (Some(workdir.repository_id), workdir.creation_selector) } else { - None + (None, None) }; if let WorkerSpawnIntent::TicketRole { ticket_id, .. } = &request.intent { let ticket = self.authority.ticket(ticket_id)?; - if let Some(repository_id) = ticket.repository_id.as_deref() { - self.require_workspace_repository(repository_id)?; - if selected_repository_id.as_deref() != Some(repository_id) { - return Err(ApiError::from(Error::Config(format!( - "Ticket `{ticket_id}` targets repository `{repository_id}`, but the Worker launch does not resolve that repository in this Workspace" - )))); - } + // Workdir-less Ticket Workers cannot execute repository implementation. + // Preserve that control-plane launch while still validating any persisted + // target (including its Workspace ownership) when one exists. + if selected_repository_id.is_none() && ticket.repository_id.is_none() { + return Ok(()); + } + let repository_id = ticket.repository_id.as_deref().ok_or_else(|| { + ApiError::from(Error::Config( + "Ticket implementation target must be validated and persisted before spawning a Ticket Worker".to_owned(), + )) + })?; + let ref_selector = ticket.ref_selector.as_deref().ok_or_else(|| { + ApiError::from(Error::Config( + "Ticket implementation target selector must be validated and persisted before spawning a Ticket Worker".to_owned(), + )) + })?; + self.require_workspace_repository(repository_id)?; + self.repository_reader() + .observe_merge_target(repository_id, Some(ref_selector)) + .map_err(|error| { + ApiError::from(Error::Config(format!( + "Ticket implementation target is no longer resolvable: {error:?}" + ))) + })?; + if selected_repository_id.as_deref() != Some(repository_id) { + return Err(ApiError::from(Error::Config(format!( + "Ticket `{ticket_id}` targets repository `{repository_id}`, but the Worker launch resolves `{}`", + selected_repository_id.as_deref().unwrap_or("none") + )))); + } + if selected_ref_selector.as_deref() != Some(ref_selector) { + return Err(ApiError::from(Error::Config(format!( + "Ticket `{ticket_id}` targets selector `{ref_selector}`, but the Worker launch resolves `{}`", + selected_ref_selector.as_deref().unwrap_or("none") + )))); } } Ok(()) @@ -1315,8 +1350,8 @@ pub fn build_router(api: WorkspaceApi) -> Router { post(scoped_set_ticket_workflow_state), ) .route( - "/api/w/{workspace_id}/tickets/{id}/intake-ready", - post(scoped_prepare_ticket_intake_ready), + "/api/w/{workspace_id}/tickets/{id}/workflow/mark-ready", + post(scoped_mark_ticket_ready), ) .route( "/api/w/{workspace_id}/tickets/{id}/workflow/queue", @@ -1397,6 +1432,10 @@ pub fn build_router(api: WorkspaceApi) -> Router { "/api/w/{workspace_id}/tickets/{id}/state", post(scoped_transition_ticket_state), ) + .route( + "/api/w/{workspace_id}/tickets/{id}/ready", + post(scoped_mark_ticket_ready_from_browser), + ) .route( "/api/w/{workspace_id}/tickets/{id}/events", post(scoped_append_ticket_event), @@ -3107,6 +3146,55 @@ struct BrowserCloseTicketRequest { resolution: String, } +#[derive(Clone)] +struct WorkspaceTicketTargetAuthority { + api: WorkspaceApi, +} + +impl ticket::TicketTargetAuthority for WorkspaceTicketTargetAuthority { + fn resolve_target( + &self, + workspace_id: &str, + repository_id: Option<&str>, + ref_selector: Option<&str>, + ) -> ticket::Result { + if workspace_id != self.api.config.workspace_id { + return Err(ticket::TicketError::UnknownTargetRepository( + repository_id.unwrap_or_default().to_owned(), + )); + } + let repository_id = repository_id + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or(ticket::TicketError::MissingTargetRepository)?; + let repository = self + .api + .store + .get_repository(workspace_id, repository_id) + .map_err(|error| ticket::TicketError::Conflict(error.to_string()))? + .ok_or_else(|| { + ticket::TicketError::UnknownTargetRepository(repository_id.to_owned()) + })?; + let selector = ref_selector + .map(str::trim) + .filter(|value| !value.is_empty()) + .or(repository.default_ref.as_deref()) + .ok_or_else(|| ticket::TicketError::MissingTargetSelector(repository_id.to_owned()))?; + self.api + .repository_reader() + .observe_merge_target(repository_id, Some(selector)) + .map_err(|error| ticket::TicketError::InvalidTargetSelector { + repository_id: repository_id.to_owned(), + selector: selector.to_owned(), + reason: format!("{error:?}"), + })?; + Ok(ticket::ResolvedTicketTarget { + repository_id: repository_id.to_owned(), + ref_selector: selector.to_owned(), + }) + } +} + fn browser_ticket_backend(api: &WorkspaceApi) -> Result { let config = ticket::config::TicketConfig::load_workspace(&api.config.workspace_root) .map_err(|error| Error::Config(format!("load Ticket workspace settings: {error}")))?; @@ -3114,7 +3202,10 @@ fn browser_ticket_backend(api: &WorkspaceApi) -> Result { api.config.database_path.clone(), api.config.workspace_id.clone(), )? - .with_record_language(config.ticket_record_language())) + .with_record_language(config.ticket_record_language()) + .with_target_authority(Arc::new(WorkspaceTicketTargetAuthority { + api: api.clone(), + }))) } fn browser_ticket_detail(api: &WorkspaceApi, ticket_id: &str) -> ApiResult> { @@ -3208,6 +3299,25 @@ async fn scoped_append_ticket_event( browser_ticket_detail(&api, &path.id) } +async fn scoped_mark_ticket_ready_from_browser( + State(api): State, + AxumPath(path): AxumPath, + Json(request): Json, +) -> ApiResult> { + validate_workspace_scope(&api, &path.workspace_id)?; + browser_ticket_backend(&api)? + .mark_ready( + TicketIdOrSlug::Id(path.id.clone()), + ticket::TicketMarkReady { + operation_key: request.operation_key, + reason: request.reason, + author: Some("web".to_owned()), + }, + ) + .map_err(Error::from)?; + browser_ticket_detail(&api, &path.id) +} + async fn scoped_queue_ticket( State(api): State, AxumPath(path): AxumPath, @@ -3269,7 +3379,10 @@ async fn execute_worker_ticket_rest_operation( api.config.workspace_id.clone(), ) .map_err(Error::from)? - .with_record_language(config.ticket_record_language()); + .with_record_language(config.ticket_record_language()) + .with_target_authority(Arc::new(WorkspaceTicketTargetAuthority { + api: api.clone(), + })); let operation_kind = ticket_mutation_operation_kind(&operation); let is_mutation = operation_kind != "read"; let target = ticket_mutation_target(&operation).cloned(); @@ -3421,6 +3534,15 @@ async fn scoped_create_ticket_record( headers: HeaderMap, Json(input): Json, ) -> ApiResult> { + if input + .workflow_state + .is_some_and(|state| state != TicketWorkflowState::Planning) + { + return Err(settings_bad_request( + "ticket_create_state_bypass", + "Ticket creation must start in planning; use guarded workflow operations for later states", + )); + } let result = execute_worker_ticket_rest_operation( &api, &path.workspace_id, @@ -3534,9 +3656,10 @@ async fn scoped_add_ticket_intake_summary( } #[derive(Debug, Deserialize)] -struct TicketIntakeReadyRequest { - summary: ticket::TicketIntakeSummary, - change: TicketStateChange, +struct TicketMarkReadyRequest { + operation_key: String, + #[serde(default)] + reason: Option, } async fn scoped_set_ticket_state_field( @@ -3578,24 +3701,30 @@ async fn scoped_set_ticket_workflow_state( ticket_rest_unit(result) } -async fn scoped_prepare_ticket_intake_ready( +async fn scoped_mark_ticket_ready( State(api): State, AxumPath((workspace_id, id)): AxumPath<(String, String)>, headers: HeaderMap, - Json(request): Json, -) -> ApiResult { + Json(request): Json, +) -> ApiResult> { let result = execute_worker_ticket_rest_operation( &api, &workspace_id, headers, - TicketBackendOperation::MarkIntakeReady { + TicketBackendOperation::MarkReady { id: TicketIdOrSlug::Query(id), - summary: request.summary, - change: request.change, + request: ticket::TicketMarkReady { + operation_key: request.operation_key, + reason: request.reason, + author: None, + }, }, ) .await?; - ticket_rest_unit(result) + ticket_rest_result(result, |result| match result { + TicketBackendOperationResult::Ticket(ticket) => Some(ticket), + _ => None, + }) } async fn scoped_queue_ticket_record( @@ -4368,7 +4497,7 @@ fn ticket_mutation_target(operation: &TicketBackendOperation) -> Option<&TicketI | TicketBackendOperation::AddIntakeSummary { id, .. } | TicketBackendOperation::SetStateField { id, .. } | TicketBackendOperation::SetWorkflowState { id, .. } - | TicketBackendOperation::MarkIntakeReady { id, .. } + | TicketBackendOperation::MarkReady { id, .. } | TicketBackendOperation::QueueReady { id, .. } | TicketBackendOperation::Close { id, .. } | TicketBackendOperation::AddTicketRelation { id, .. } @@ -4409,12 +4538,7 @@ fn bind_worker_ticket_operation_source( | TicketBackendOperation::SetStateField { change, .. } | TicketBackendOperation::SetWorkflowState { change, .. } => change.author = Some(author), TicketBackendOperation::AddIntakeSummary { summary, .. } => summary.author = Some(author), - TicketBackendOperation::MarkIntakeReady { - summary, change, .. - } => { - summary.author = Some(author.clone()); - change.author = Some(author); - } + TicketBackendOperation::MarkReady { request, .. } => request.author = Some(author), TicketBackendOperation::QueueReady { queued_by, .. } => *queued_by = author, TicketBackendOperation::AddTicketRelation { relation, .. } => { relation.author = Some(author) @@ -4435,7 +4559,7 @@ fn ticket_mutation_operation_kind(operation: &TicketBackendOperation) -> &'stati TicketBackendOperation::AddIntakeSummary { .. } => "add_intake_summary", TicketBackendOperation::SetStateField { .. } => "set_state_field", TicketBackendOperation::SetWorkflowState { .. } => "set_workflow_state", - TicketBackendOperation::MarkIntakeReady { .. } => "mark_intake_ready", + TicketBackendOperation::MarkReady { .. } => "mark_ready", TicketBackendOperation::QueueReady { .. } => "queue_ready", TicketBackendOperation::Close { .. } => "close", TicketBackendOperation::AddTicketRelation { .. } => "add_relation", @@ -11883,7 +12007,26 @@ impl From for ApiError { ticket::TicketError::NotFound(_) => "ticket_not_found", ticket::TicketError::Ambiguous { .. } => "ticket_ambiguous", ticket::TicketError::Locked { .. } => "ticket_locked", - ticket::TicketError::Conflict(_) => "ticket_conflict", + ticket::TicketError::Conflict(_) + | ticket::TicketError::StaleWorkflowState { .. } + | ticket::TicketError::InvalidWorkflowTransition { .. } + | ticket::TicketError::BlockingRelations(_) + | ticket::TicketError::OperationFingerprintMismatch { .. } => "ticket_conflict", + ticket::TicketError::MissingTargetRepository => { + "ticket_target_repository_missing" + } + ticket::TicketError::UnknownTargetRepository(_) => { + "ticket_target_repository_unknown" + } + ticket::TicketError::MissingTargetSelector(_) => { + "ticket_target_selector_missing" + } + ticket::TicketError::InvalidTargetSelector { .. } => { + "ticket_target_selector_invalid" + } + ticket::TicketError::TargetAuthorityUnavailable => { + "ticket_target_authority_unavailable" + } ticket::TicketError::InvalidPathComponent(_) | ticket::TicketError::PathEscapesRoot { .. } => "invalid_ticket_request", ticket::TicketError::Io { .. } @@ -13549,7 +13692,7 @@ mod tests { config.repositories = vec![ConfiguredRepository { id: TEST_REPOSITORY_ID.to_string(), provider: "git".to_string(), - uri: ".".to_string(), + uri: workspace_root.display().to_string(), path: workspace_root, display_name: Some("Test Repository".to_string()), default_selector: Some("HEAD".to_string()), @@ -13706,7 +13849,7 @@ mod tests { fn init_clean_git_workspace(path: &std::path::Path) { for args in [ - vec!["init"], + vec!["init", "--initial-branch=develop"], vec!["config", "user.email", "test@example.invalid"], vec!["config", "user.name", "Yoi Test"], ] { @@ -13738,6 +13881,86 @@ mod tests { } } + #[tokio::test] + async fn mark_ready_resolves_workspace_target_and_closes_lifecycle_bypasses() { + let dir = tempfile::tempdir().unwrap(); + init_clean_git_workspace(dir.path()); + let api = test_api(dir.path()).await; + let backend = browser_ticket_backend(&api).unwrap(); + + let mut input = ticket::NewTicket::new("Validated target"); + input.repository_id = Some(TEST_REPOSITORY_ID.to_owned()); + input.ref_selector = Some("develop".to_owned()); + let ticket_ref = backend.create(input).unwrap(); + let request = ticket::TicketMarkReady { + operation_key: "ready-server-test".to_owned(), + reason: Some("target accepted".to_owned()), + author: Some("test".to_owned()), + }; + let ready = backend + .mark_ready(TicketIdOrSlug::Id(ticket_ref.id.clone()), request.clone()) + .unwrap(); + assert_eq!(ready.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!( + ready.meta.repository_id.as_deref(), + Some(TEST_REPOSITORY_ID) + ); + assert_eq!(ready.meta.ref_selector.as_deref(), Some("develop")); + assert_eq!( + backend + .mark_ready(TicketIdOrSlug::Id(ticket_ref.id.clone()), request) + .unwrap() + .events + .iter() + .filter(|event| event.attributes.contains_key("operation_key")) + .count(), + 1 + ); + assert!(matches!( + backend.edit_item( + TicketIdOrSlug::Id(ticket_ref.id.clone()), + ticket::TicketItemEdit { + target: Some(ticket::TicketTargetEdit::Set { + repository_id: TEST_REPOSITORY_ID.to_owned(), + ref_selector: Some("other".to_owned()), + }), + ..Default::default() + }, + ), + Err(ticket::TicketError::Conflict(_)) + )); + + let mut missing = ticket::NewTicket::new("Missing target"); + missing.repository_id = Some("unknown".to_owned()); + let missing = backend.create(missing).unwrap(); + assert!(matches!( + backend.mark_ready( + TicketIdOrSlug::Id(missing.id.clone()), + ticket::TicketMarkReady { + operation_key: "missing-repository".to_owned(), + reason: None, + author: None, + }, + ), + Err(ticket::TicketError::UnknownTargetRepository(_)) + )); + assert_eq!( + backend + .show(TicketIdOrSlug::Id(missing.id)) + .unwrap() + .meta + .workflow_state, + TicketWorkflowState::Planning + ); + assert!(matches!( + backend.set_workflow_state( + TicketIdOrSlug::Id(ticket_ref.id), + TicketStateChange::new("ready", "queued", "bypass", "must use TicketQueue",), + ), + Err(ticket::TicketError::InvalidWorkflowTransition { .. }) + )); + } + #[test] fn worker_source_actor_roles_use_canonical_vocabulary() { assert_eq!(worker_source_actor_role(true, false), "coder"); @@ -13779,6 +14002,7 @@ mod tests { #[tokio::test] async fn orchestrator_ticket_notifications_project_authoritative_post_mutation_state() { let dir = tempfile::tempdir().unwrap(); + init_clean_git_workspace(dir.path()); let (api, execution) = test_api_with_recording_backend(dir.path()).await; let source_worker = api .runtime @@ -13836,29 +14060,23 @@ mod tests { let orchestrator = started.worker.unwrap().worker; execution.take_inputs(); - let ticket = browser_ticket_backend(&api) - .unwrap() - .create(ticket::NewTicket::new("Bounded notification")) - .unwrap(); + let mut input = ticket::NewTicket::new("Bounded notification"); + input.repository_id = Some(TEST_REPOSITORY_ID.to_owned()); + input.ref_selector = Some("develop".to_owned()); + let ticket = browser_ticket_backend(&api).unwrap().create(input).unwrap(); let ticket_id = TicketIdOrSlug::Id(ticket.id.clone()); let operations = [ - TicketBackendOperation::SetWorkflowState { + TicketBackendOperation::MarkReady { id: ticket_id.clone(), - change: TicketStateChange::new( - "planning", - "ready", - "ready for implementation", - "test transition", - ), + request: ticket::TicketMarkReady { + operation_key: "notification-ready".to_owned(), + reason: Some("ready for implementation".to_owned()), + author: None, + }, }, - TicketBackendOperation::SetWorkflowState { + TicketBackendOperation::QueueReady { id: ticket_id.clone(), - change: TicketStateChange::new( - "ready", - "queued", - "queued for implementation", - "test transition", - ), + queued_by: "spoofed".to_owned(), }, TicketBackendOperation::SetWorkflowState { id: ticket_id.clone(), @@ -14263,30 +14481,11 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let api = test_api(dir.path()).await; let backend = browser_ticket_backend(&api).unwrap(); - let ticket_ref = backend - .create(ticket::NewTicket::new("Recover queued work")) - .unwrap(); - backend - .mark_intake_ready( - TicketIdOrSlug::Id(ticket_ref.id.clone()), - ticket::TicketIntakeSummary { - author: Some("intake".to_string()), - body: MarkdownText::new("Ready"), - references: Vec::new(), - }, - ticket::TicketStateChange { - from: "planning".to_string(), - to: "ready".to_string(), - reason: "ready".to_string(), - author: Some("intake".to_string()), - body: MarkdownText::new("Ready"), - references: Vec::new(), - }, - ) - .unwrap(); - backend - .queue_ready(TicketIdOrSlug::Id(ticket_ref.id.clone()), "browser-user") - .unwrap(); + let mut input = ticket::NewTicket::new("Recover queued work"); + input.workflow_state = Some(TicketWorkflowState::Queued); + input.repository_id = Some(TEST_REPOSITORY_ID.to_owned()); + input.ref_selector = Some("HEAD".to_owned()); + let ticket_ref = backend.create(input).unwrap(); *api.orchestrator_attention_fingerprint.lock().unwrap() = Some(ticket_ref.id.clone()); let Json(started) = scoped_start_workspace_orchestrator( @@ -14712,6 +14911,7 @@ mod tests { #[tokio::test] async fn ticket_browser_endpoints_mutate_typed_backend_and_return_thread() { let dir = tempfile::tempdir().unwrap(); + init_clean_git_workspace(dir.path()); let api = test_api(dir.path()).await; let ticket_ref = browser_ticket_backend(&api) .unwrap() @@ -14751,7 +14951,7 @@ mod tests { replace_all: false, target: Some(TicketTargetEdit::Set { repository_id: "main".to_string(), - ref_selector: Some("feature/api".to_string()), + ref_selector: Some("develop".to_string()), }), author: Some("browser-user".to_string()), }), @@ -14761,7 +14961,7 @@ mod tests { assert_eq!(edited.title, "Browser Ticket API edited"); assert_eq!(edited.body, "Updated from the Browser API."); assert_eq!(edited.repository_id.as_deref(), Some("main")); - assert_eq!(edited.ref_selector.as_deref(), Some("feature/api")); + assert_eq!(edited.ref_selector.as_deref(), Some("develop")); assert_eq!(edited.assignee, None); assert_eq!(edited.relations.outgoing.len(), 1); assert_eq!(edited.relations.outgoing[0].target, related_ticket_id); @@ -14782,14 +14982,12 @@ mod tests { event.kind == "comment" && event.body.as_deref() == Some("API comment") })); - let Json(ready) = scoped_transition_ticket_state( + let Json(ready) = scoped_mark_ticket_ready_from_browser( State(api.clone()), AxumPath(path()), - Json(BrowserTransitionTicketStateRequest { - state: TicketWorkflowState::Ready, - reason: Some("intake complete".to_string()), - body: Some("Ready for queue".to_string()), - author: Some("browser-user".to_string()), + Json(TicketMarkReadyRequest { + operation_key: "browser-ready".to_owned(), + reason: Some("intake complete".to_owned()), }), ) .await diff --git a/crates/yoi/src/ticket_cli.rs b/crates/yoi/src/ticket_cli.rs index cad60e55..62d9f1ad 100644 --- a/crates/yoi/src/ticket_cli.rs +++ b/crates/yoi/src/ticket_cli.rs @@ -12,8 +12,7 @@ use ticket::config::{ use ticket::{ LocalTicketBackend, MarkdownText, NewTicket, NewTicketEvent, NewTicketRelation, SqliteTicketBackend, TicketBackend, TicketDoctorSeverity, TicketEventKind, TicketIdOrSlug, - TicketIntakeSummary, TicketListQuery, TicketListState, TicketRelationKind, TicketSummary, - TicketWorkflowState, + TicketListQuery, TicketListState, TicketRelationKind, TicketSummary, TicketWorkflowState, }; const DEFAULT_LIST_LIMIT: usize = 50; @@ -630,8 +629,16 @@ fn state( let id = TicketIdOrSlug::Query(options.query.clone()); let target_state = match options.state { StateTarget::Planning => TicketWorkflowState::Planning, - StateTarget::Ready => TicketWorkflowState::Ready, - StateTarget::Queued => TicketWorkflowState::Queued, + StateTarget::Ready => { + return Err(TicketCliError::new( + "ready requires Workspace repository authority; use the Browser Mark ready action or TicketMarkReady", + )); + } + StateTarget::Queued => { + return Err(TicketCliError::new( + "queued is an Orchestrator operation; use TicketQueue after MarkReady succeeds", + )); + } StateTarget::InProgress => TicketWorkflowState::InProgress, StateTarget::Done => { return Err(TicketCliError::new( @@ -646,33 +653,16 @@ fn state( }; let current = backend.show(id.clone())?; let ticket_id = current.meta.id.clone(); - match target_state { - TicketWorkflowState::Ready => backend.mark_intake_ready( - id, - TicketIntakeSummary::new("Marked ready by `yoi ticket state`."), - ticket::TicketStateChange { - from: current.meta.workflow_state.as_str().to_string(), - to: TicketWorkflowState::Ready.as_str().to_string(), - reason: "cli_state".to_string(), - author: Some("yoi ticket".to_string()), - body: "Marked ready by `yoi ticket state`.\n".into(), - references: Vec::new(), - }, - )?, - TicketWorkflowState::Queued => backend.queue_ready(id, "yoi ticket")?, - _ => { - let from = current.meta.workflow_state; - let change = ticket::TicketStateChange { - from: from.as_str().to_string(), - to: target_state.as_str().to_string(), - reason: "cli_state".to_string(), - author: Some("yoi ticket".to_string()), - body: format!("State changed to `{}`.\n", target_state.as_str()).into(), - references: Vec::new(), - }; - backend.set_workflow_state(id, change)?; - } - } + let from = current.meta.workflow_state; + let change = ticket::TicketStateChange { + from: from.as_str().to_string(), + to: target_state.as_str().to_string(), + reason: "cli_state".to_string(), + author: Some("yoi ticket".to_string()), + body: format!("State changed to `{}`.\n", target_state.as_str()).into(), + references: Vec::new(), + }; + backend.set_workflow_state(id, change)?; Ok(success(format!( "state\t{}\t{}\n", ticket_id, @@ -1335,23 +1325,14 @@ mod tests { .contains(&format!("appended\t{}\timplementation_report", ticket_id)) ); - let ready = run(&temp, &["state", &ticket_id, "ready"]); - assert_eq!(ready.stdout, format!("state\t{}\tready\n", ticket_id)); - let ready_listed = run(&temp, &["list", "--state", "ready"]); - assert!(ready_listed.stdout.contains(&ticket_id)); - - let queued = run(&temp, &["state", &ticket_id, "queued"]); - assert_eq!(queued.stdout, format!("state\t{}\tqueued\n", ticket_id)); - let queued_listed = run(&temp, &["list", "--state", "queued"]); - assert!(queued_listed.stdout.contains(&ticket_id)); - - let inprogress = run(&temp, &["state", &ticket_id, "inprogress"]); - assert_eq!( - inprogress.stdout, - format!("state\t{}\tinprogress\n", ticket_id) - ); - let inprogress_listed = run(&temp, &["list", "--state", "inprogress"]); - assert!(inprogress_listed.stdout.contains(&ticket_id)); + let ready_error = parse_ticket_args(&args(&["state", &ticket_id, "ready"])) + .and_then(|cli| run_in_workspace(cli, temp.path())) + .unwrap_err(); + assert!(ready_error.to_string().contains("TicketMarkReady")); + let queue_error = parse_ticket_args(&args(&["state", &ticket_id, "queued"])) + .and_then(|cli| run_in_workspace(cli, temp.path())) + .unwrap_err(); + assert!(queue_error.to_string().contains("TicketQueue")); let done_error = parse_ticket_args(&args(&["state", &ticket_id, "done"])) .and_then(|cli| run_in_workspace(cli, temp.path())) diff --git a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts index 0a653e59..725746ef 100644 --- a/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts +++ b/web/workspace/src/lib/workspace/console/worker-console.ui.test.ts @@ -247,9 +247,11 @@ Deno.test("workspace Tickets surface provides Kanban and lifecycle controls", as assert( ticketDetailLoad.includes("/repositories") && ticketDetailPage.includes('mutate("state", "/state"') && + ticketDetailPage.includes('mutate("ready", "/ready"') && ticketDetailPage.includes('mutate("queue", "/queue"') && + ticketDetailPage.includes("targetCandidateValid") && + ticketDetailPage.includes("persistedTargetValid") && !ticketDetailPage.includes("/merge-request/merge") && - ticketDetailPage.includes("merged_result_commit") && !ticketDetailPage.includes('mutate("review", "/review"') && ticketDetailPage.includes('mutate("close", "/close"') && ticketDetailPage.includes("ticketWorkerLaunchHref") && 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 875f2472..a581a1b3 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte @@ -14,6 +14,7 @@ import type { ApiResult } from "$lib/workspace/api/http"; import type { RepositoryListResponse, + RepositorySummary, TicketDetail, } from "$lib/workspace/sidebar/types"; @@ -65,7 +66,9 @@ thread: MergeRequestThreadEvent[]; }; - const MUTABLE_TICKET_STATES = TICKET_STATES.filter((state) => state !== "done"); + const MUTABLE_TICKET_STATES = TICKET_STATES.filter((state) => + state !== "done" && state !== "ready" && state !== "queued" + ); const { data } = $props<{ data: { @@ -115,6 +118,27 @@ let resolution = $state(""); let busy = $state(null); let errorMessage = $state(null); + let readyOperationKey = $state(null); + const selectedRepository = $derived( + (loadedRepositories?.items ?? []).find((repository: RepositorySummary) => repository.id === repositoryId) ?? null, + ); + const effectiveRefSelector = $derived(refSelector.trim() || selectedRepository?.default_ref || ""); + const targetCandidateValid = $derived( + ticket.state === "planning" && + selectedRepository !== null && + (selectedRepository.diagnostics ?? []).length === 0 && + effectiveRefSelector.length > 0, + ); + const persistedTargetValid = $derived( + ticket.repository_id !== null && + ticket.ref_selector !== null && + (loadedRepositories?.items ?? []).some((repository: RepositorySummary) => + repository.id === ticket.repository_id && (repository.diagnostics ?? []).length === 0 + ), + ); + const implementationStartEligible = $derived( + persistedTargetValid && ticket.state !== "planning" && ticket.state !== "closed", + ); const ticketPath = $derived( workspaceApiPath( @@ -180,6 +204,33 @@ }, "PATCH"); } + async function markReady() { + if (!targetCandidateValid || busy) return; + if ( + ticket.repository_id !== repositoryId || + (ticket.ref_selector ?? "") !== refSelector.trim() + ) { + const saved = await mutate("target", "", { + target: { + action: "set", + repository_id: repositoryId, + ref_selector: refSelector.trim() || null, + }, + }, "PATCH"); + if (!saved) return; + } + readyOperationKey ??= crypto.randomUUID(); + if ( + await mutate("ready", "/ready", { + operation_key: readyOperationKey, + reason: transitionReason.trim() || null, + }) + ) { + readyOperationKey = null; + transitionReason = ""; + } + } + async function transition(event: SubmitEvent) { event.preventDefault(); if ( @@ -329,16 +380,19 @@

Assigned to {ticket.assignee ?? "Unassigned"}

- {#if orchestratorOnline} -

The Orchestrator is online. Start a role-specific Worker with the Ticket target below.

+ {#if orchestratorOnline && implementationStartEligible} +

The Orchestrator is online. Start a role-specific Worker with the validated Ticket target below.

Coder
{:else} -

Start the Workspace Orchestrator from the Ticket panel before launching Ticket Workers.

+

+ {orchestratorOnline + ? "Validate and persist the repository target before starting a Ticket Worker." + : "Start the Workspace Orchestrator from the Ticket panel before launching Ticket Workers."} +

-
{/if} @@ -347,15 +401,15 @@

Repository target

- -
@@ -374,8 +428,15 @@ Apply state - {#if ticket.state === "ready"} - + {#if !targetCandidateValid} +

Choose a healthy repository and an effective ref selector before marking ready.

+ {/if} + {:else if ticket.state === "ready"} + {/if} From f9e5fca67dc0c6a9d2225fb584dbbf90678dd870 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 17 Aug 2026 14:44:59 +0900 Subject: [PATCH 02/13] fix: preserve intake and companion ticket workflows --- crates/ticket/src/lib.rs | 44 +++++++++- crates/ticket/src/tool.rs | 96 ++++++++++++++++++++- crates/worker/src/controller.rs | 6 +- crates/worker/src/feature/builtin/ticket.rs | 21 +++-- crates/worker/src/shutdown_after_idle.rs | 24 +++--- crates/workspace-server/src/server.rs | 15 +++- 6 files changed, 178 insertions(+), 28 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index d44656cc..daf7350d 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -529,6 +529,8 @@ pub struct TicketMarkReady { pub reason: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub author: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub intake_summary: Option, } impl TicketTargetEdit { @@ -590,6 +592,16 @@ fn mark_ready_fingerprint( if let Some(reason) = request.reason.as_deref() { digest.update(reason.as_bytes()); } + if let Some(summary) = request.intake_summary.as_ref() { + digest.update(b"\0intake-summary\0"); + digest.update(summary.body.as_str().as_bytes()); + for reference in &summary.references { + digest.update(b"\0"); + digest.update(reference.kind.as_bytes()); + digest.update(b":"); + digest.update(reference.target.as_bytes()); + } + } digest .finalize() .iter() @@ -3598,6 +3610,28 @@ impl TicketBackend for SqliteTicketBackend { .unwrap_or("implementation target validated") .to_owned(); let at = now_utc(); + if let Some(mut summary) = request.intake_summary.clone() { + validate_intake_summary(&summary)?; + summary.author = request.author.clone().or(summary.author); + self.insert_event( + conn, + &ticket_id, + &TicketEvent { + kind: TicketEventKind::IntakeSummary, + author: summary.author, + at: None, + status: None, + from: None, + to: None, + reason: None, + state_field: None, + heading: Some(TicketEventKind::IntakeSummary.heading()), + body: summary.body, + references: summary.references, + attributes: BTreeMap::new(), + }, + )?; + } self.insert_event( conn, &ticket_id, @@ -4252,7 +4286,11 @@ impl TicketBackend for LocalTicketBackend { target.repository_id, target.ref_selector )), ); - change.author = request.author.or_else(|| Some(default_author())); + change.author = request.author.clone().or_else(|| Some(default_author())); + if let Some(mut summary) = request.intake_summary { + summary.author = request.author.clone().or(summary.author); + self.append_intake_summary_event(&dir, &summary)?; + } self.append_state_changed_event_with_attributes( &dir, &change, @@ -6960,6 +6998,7 @@ state: planning operation_key: "sqlite-ready".to_owned(), reason: Some("target accepted".to_owned()), author: Some("test".to_owned()), + intake_summary: None, }; let ready = backend .mark_ready( @@ -7377,6 +7416,7 @@ state: planning operation_key: "test-flow-ready".to_owned(), reason: Some("ready_for_queue".to_owned()), author: Some("test".to_owned()), + intake_summary: None, }, ) .unwrap(); @@ -7648,6 +7688,7 @@ state: planning operation_key: "ready-op-1".to_owned(), reason: Some("accepted".to_owned()), author: Some("intake".to_owned()), + intake_summary: None, }; let first = backend @@ -7679,6 +7720,7 @@ state: planning operation_key: "ready-op-1".to_owned(), reason: Some("different".to_owned()), author: Some("intake".to_owned()), + intake_summary: None, }, ), Err(TicketError::OperationFingerprintMismatch { .. }) diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 158f9d8b..87ee84b2 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -69,7 +69,7 @@ pub const TICKET_ORCHESTRATION_TOOL_NAMES: [&str; 5] = [ pub const TICKET_ORCHESTRATION_READ_ONLY_TOOL_NAMES: [&str; 2] = ["TicketRelationQuery", "TicketOrchestrationPlanQuery"]; -pub const TICKET_TOOL_NAMES: [&str; 19] = [ +pub const TICKET_TOOL_NAMES: [&str; 20] = [ "TicketCreate", "TicketEditItem", "QueryTicket", @@ -79,6 +79,7 @@ pub const TICKET_TOOL_NAMES: [&str; 19] = [ "TicketDecision", "TicketImplementationReport", "TicketMarkReady", + "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -100,7 +101,7 @@ pub const TICKET_READ_ONLY_TOOL_NAMES: [&str; 6] = [ "TicketOrchestrationPlanQuery", ]; -pub const TICKET_MUTATING_TOOL_NAMES: [&str; 13] = [ +pub const TICKET_MUTATING_TOOL_NAMES: [&str; 14] = [ "TicketCreate", "TicketEditItem", "TicketComment", @@ -108,6 +109,7 @@ pub const TICKET_MUTATING_TOOL_NAMES: [&str; 13] = [ "TicketDecision", "TicketImplementationReport", "TicketMarkReady", + "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -136,6 +138,9 @@ const IMPLEMENTATION_REPORT_DESCRIPTION: &str = const MARK_READY_DESCRIPTION: &str = "Mark a planning Ticket ready through the typed Ticket backend. \ The backend atomically validates and normalizes the persisted repository/ref target, records one typed \ state_changed event, and transitions planning -> ready. `reason` is optional."; +const INTAKE_READY_DESCRIPTION: &str = "Record a bounded intake summary and mark a planning Ticket ready. \ +The backend applies the same target validation and lock as TicketMarkReady and commits the summary, \ +state_changed event, effective target, and planning -> ready transition atomically."; const QUEUE_DESCRIPTION: &str = "Queue a ready Ticket for Orchestrator routing through the typed \ Ticket backend. The backend performs the gated ready -> queued transition, records queued_by/queued_at, \ and rejects unresolved blocking relations."; @@ -176,6 +181,7 @@ fn base_tool_description(name: &str) -> &'static str { "TicketDecision" => DECISION_DESCRIPTION, "TicketImplementationReport" => IMPLEMENTATION_REPORT_DESCRIPTION, "TicketMarkReady" => MARK_READY_DESCRIPTION, + "TicketIntakeReady" => INTAKE_READY_DESCRIPTION, "TicketQueue" => QUEUE_DESCRIPTION, "TicketWorkflowState" => WORKFLOW_STATE_DESCRIPTION, "TicketClose" => CLOSE_DESCRIPTION, @@ -563,6 +569,17 @@ struct TicketMarkReadyParams { reason: Option, } +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TicketIntakeReadyParams { + /// Ticket id. + ticket: String, + /// Concise bounded intake summary appended before the ready transition. + intake_summary: String, + /// Optional reason attached to the state_changed event. + #[serde(default)] + reason: Option, +} + #[derive(Debug, Deserialize, schemars::JsonSchema)] struct TicketQueueParams { /// Ticket id. @@ -832,6 +849,11 @@ struct TicketMarkReadyTool { backend: TicketToolBackend, } +#[derive(Clone)] +struct TicketIntakeReadyTool { + backend: TicketToolBackend, +} + #[derive(Clone)] struct TicketQueueTool { backend: TicketToolBackend, @@ -1138,6 +1160,7 @@ impl Tool for TicketMarkReadyTool { operation_key: format!("ticket-mark-ready:{}", ctx.call_id), reason: params.reason, author: None, + intake_summary: None, }, ) .map_err(|error| backend_error("TicketMarkReady", error))?; @@ -1154,6 +1177,39 @@ impl Tool for TicketMarkReadyTool { } } +#[async_trait] +impl Tool for TicketIntakeReadyTool { + async fn execute( + &self, + input_json: &str, + ctx: llm_engine::tool::ToolExecutionContext, + ) -> Result { + let params: TicketIntakeReadyParams = parse_input("TicketIntakeReady", input_json)?; + let ticket = self + .backend + .mark_ready( + TicketIdOrSlug::Query(params.ticket.clone()), + TicketMarkReady { + operation_key: format!("ticket-intake-ready:{}", ctx.call_id), + reason: params.reason, + author: None, + intake_summary: Some(TicketIntakeSummary::new(params.intake_summary)), + }, + ) + .map_err(|error| backend_error("TicketIntakeReady", error))?; + Ok(json_output( + format!("Marked ticket {} state ready after intake", params.ticket), + json!({ + "ticket": ticket.meta.id, + "state": ticket.meta.workflow_state.as_str(), + "repository_id": ticket.meta.repository_id, + "ref_selector": ticket.meta.ref_selector, + "ok": true + }), + )) + } +} + #[async_trait] impl Tool for TicketQueueTool { async fn execute( @@ -1728,6 +1784,7 @@ fn input_schema(name: &str) -> Value { serde_json::to_value(schemars::schema_for!(TicketThreadEventParams)) } "TicketMarkReady" => serde_json::to_value(schemars::schema_for!(TicketMarkReadyParams)), + "TicketIntakeReady" => serde_json::to_value(schemars::schema_for!(TicketIntakeReadyParams)), "TicketQueue" => serde_json::to_value(schemars::schema_for!(TicketQueueParams)), "TicketWorkflowState" => { serde_json::to_value(schemars::schema_for!(TicketWorkflowStateParams)) @@ -1776,6 +1833,7 @@ impl_from_backend!(TicketPlanTool); impl_from_backend!(TicketDecisionTool); impl_from_backend!(TicketImplementationReportTool); impl_from_backend!(TicketMarkReadyTool); +impl_from_backend!(TicketIntakeReadyTool); impl_from_backend!(TicketQueueTool); impl_from_backend!(TicketWorkflowStateTool); impl_from_backend!(TicketCloseTool); @@ -1803,6 +1861,7 @@ pub fn ticket_tools(backend: impl Into) -> Vec("TicketMarkReady", backend.clone()), + tool_definition::("TicketIntakeReady", backend.clone()), tool_definition::("TicketQueue", backend.clone()), tool_definition::("TicketWorkflowState", backend.clone()), tool_definition::("TicketClose", backend.clone()), @@ -1897,6 +1956,7 @@ mod tests { "TicketDecision", "TicketImplementationReport", "TicketMarkReady", + "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", "TicketClose", @@ -2558,6 +2618,38 @@ mod tests { ); } + #[tokio::test] + async fn ticket_intake_ready_records_summary_with_validated_target() { + let temp = TempDir::new().unwrap(); + let backend = backend(&temp); + let mut input = NewTicket::new("Intake Workflow"); + input.repository_id = Some("main".to_owned()); + let created = backend.create(input).unwrap(); + tool_by_name(backend.clone(), "TicketIntakeReady") + .execute( + &json!({ + "ticket": created.id.clone(), + "intake_summary": "Requirements and target are accepted.", + "reason": "intake_complete" + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + let record = backend.show(TicketIdOrSlug::Id(created.id)).unwrap(); + assert_eq!(record.meta.workflow_state, TicketWorkflowState::Ready); + assert_eq!(record.meta.ref_selector.as_deref(), Some("develop")); + assert_eq!( + record + .events + .iter() + .filter(|event| event.kind == TicketEventKind::IntakeSummary) + .count(), + 1 + ); + } + #[tokio::test] async fn ticket_workflow_tool_allows_return_to_planning_from_ready_and_queued() { let temp = TempDir::new().unwrap(); diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index 67d3ea01..ebb07952 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -18,7 +18,7 @@ use crate::runtime::dir::RuntimeDir; use crate::segment_log_sink::SegmentLogSink; use crate::shared_state::WorkerSharedState; use crate::shutdown_after_idle::{ - ShutdownAfterIdleRequest, TicketMarkReadyShutdownHook, is_ticket_intake_role, + ShutdownAfterIdleRequest, TicketIntakeReadyShutdownHook, is_ticket_intake_role, take_shutdown_request_after_status, }; use crate::spawn::registry::SpawnedWorkerRegistry; @@ -423,11 +423,11 @@ impl WorkerController { .await?; // Intake role Workers self-terminate only after a successful - // TicketMarkReady turn has fully settled back to Idle. The request + // TicketIntakeReady turn has fully settled back to Idle. The request // is transient controller state, not model-visible context or ticket // claim metadata. let shutdown_after_idle = ShutdownAfterIdleRequest::default(); - worker.add_post_tool_call_hook(TicketMarkReadyShutdownHook::new( + worker.add_post_tool_call_hook(TicketIntakeReadyShutdownHook::new( shutdown_after_idle.clone(), is_ticket_intake_role(worker.runtime_ticket_role()), )); diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index 32283d6f..4b2e7d16 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -381,6 +381,8 @@ const READ_ONLY_TOOL_NAMES: &[&str] = &["QueryTicket", "ShowTicket"]; const AUTHORING_TOOL_NAMES: &[&str] = &[ "TicketCreate", "TicketEditItem", + "TicketMarkReady", + "TicketQueue", "TicketClose", "TicketRelationRecord", "TicketRelationRemove", @@ -388,7 +390,7 @@ const AUTHORING_TOOL_NAMES: &[&str] = &[ const THREAD_TOOL_NAMES: &[&str] = &["TicketComment"]; -const INTAKE_TOOL_NAMES: &[&str] = &["TicketMarkReady"]; +const INTAKE_TOOL_NAMES: &[&str] = &["TicketIntakeReady"]; #[cfg(test)] const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ @@ -397,6 +399,8 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ "QueryTicket", "ShowTicket", "TicketComment", + "TicketMarkReady", + "TicketQueue", "TicketClose", "TicketRelationRecord", "TicketRelationRemove", @@ -407,7 +411,6 @@ const WORKFLOW_TOOL_NAMES: &[&str] = &[ "QueryTicket", "ShowTicket", "TicketComment", - "TicketQueue", "TicketWorkflowState", "TicketClose", "TicketDependencyCheck", @@ -418,7 +421,6 @@ const WORKFLOW_TOOL_NAMES: &[&str] = &[ ]; const WORKFLOW_ADDITIONAL_TOOL_NAMES: &[&str] = &[ - "TicketQueue", "TicketWorkflowState", "TicketClose", "TicketDependencyCheck", @@ -1295,13 +1297,13 @@ mod tests { assert_eq!(show.name, "ShowTicket"); assert!(show.input_schema["properties"]["event_limit"].is_object()); let tool_names = TicketFeatureAccess::workspace_authoring().tool_names(); - assert_eq!(tool_names.len(), 8); + assert_eq!(tool_names.len(), 10); assert!( tool_names.len() < 13, "authoring catalog must stay below the prior broad catalog" ); let workflow_names = TicketFeatureAccess::workflow().tool_names(); - assert_eq!(workflow_names.len(), 11); + assert_eq!(workflow_names.len(), 10); assert!( workflow_names.len() < 12, "workflow catalog must stay below the prior broad catalog" @@ -1382,7 +1384,7 @@ mod tests { .collect::>(); assert!(workspace_tools.contains(&"TicketCreate")); assert!(workspace_tools.contains(&"TicketEditItem")); - assert!(!workspace_tools.contains(&"TicketQueue")); + assert!(workspace_tools.contains(&"TicketQueue")); assert!(!workspace_tools.contains(&"TicketWorkflowState")); let orchestration = @@ -1398,7 +1400,7 @@ mod tests { assert!(orchestration_tools.contains(&"TicketRelationRecord")); assert!(orchestration_tools.contains(&"TicketOrchestrationPlanRecord")); assert!(!orchestration_tools.contains(&"TicketEditItem")); - assert!(orchestration_tools.contains(&"TicketQueue")); + assert!(!orchestration_tools.contains(&"TicketQueue")); let work_report = ticket_tools_feature_with_access(temp.path(), TicketFeatureAccess::work_report()); @@ -1511,8 +1513,9 @@ language = "Japanese" assert_eq!(installed, WORKSPACE_AUTHORING_TOOL_NAMES); assert!(installed.iter().any(|tool| *tool == "TicketCreate")); assert!(installed.iter().any(|tool| *tool == "TicketEditItem")); - assert!(!installed.iter().any(|tool| *tool == "TicketQueue")); - assert!(!installed.iter().any(|tool| *tool == "TicketMarkReady")); + assert!(installed.iter().any(|tool| *tool == "TicketQueue")); + assert!(installed.iter().any(|tool| *tool == "TicketMarkReady")); + assert!(!installed.iter().any(|tool| *tool == "TicketIntakeReady")); assert!(!installed.iter().any(|tool| *tool == "TicketWorkflowState")); assert!( !installed diff --git a/crates/worker/src/shutdown_after_idle.rs b/crates/worker/src/shutdown_after_idle.rs index 2f4d1fb3..bae83158 100644 --- a/crates/worker/src/shutdown_after_idle.rs +++ b/crates/worker/src/shutdown_after_idle.rs @@ -9,7 +9,7 @@ use ticket::config::TicketRole; use crate::hook::{Hook, HookPostToolAction, PostToolCall, ToolResultSummary}; -const TICKET_MARK_READY_TOOL_NAME: &str = "TicketMarkReady"; +const TICKET_INTAKE_READY_TOOL_NAME: &str = "TicketIntakeReady"; #[derive(Clone, Default)] pub(crate) struct ShutdownAfterIdleRequest { @@ -42,12 +42,12 @@ pub(crate) fn take_shutdown_request_after_status( status == WorkerStatus::Idle && shutdown_after_idle.take() } -pub(crate) struct TicketMarkReadyShutdownHook { +pub(crate) struct TicketIntakeReadyShutdownHook { shutdown_after_idle: ShutdownAfterIdleRequest, eligible_ticket_intake_role: bool, } -impl TicketMarkReadyShutdownHook { +impl TicketIntakeReadyShutdownHook { pub(crate) fn new( shutdown_after_idle: ShutdownAfterIdleRequest, eligible_ticket_intake_role: bool, @@ -60,7 +60,7 @@ impl TicketMarkReadyShutdownHook { fn observe_tool_result(&self, info: &ToolResultSummary) { if self.eligible_ticket_intake_role - && info.tool_name == TICKET_MARK_READY_TOOL_NAME + && info.tool_name == TICKET_INTAKE_READY_TOOL_NAME && !info.is_error { self.shutdown_after_idle.request(); @@ -69,7 +69,7 @@ impl TicketMarkReadyShutdownHook { } #[async_trait] -impl Hook for TicketMarkReadyShutdownHook { +impl Hook for TicketIntakeReadyShutdownHook { async fn call(&self, info: &ToolResultSummary) -> HookPostToolAction { self.observe_tool_result(info); HookPostToolAction::Continue @@ -98,9 +98,9 @@ mod tests { #[test] fn successful_ticket_intake_ready_schedules_shutdown_after_idle_for_intake_role() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); + let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); - hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, false)); + hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, false)); assert!(request.is_requested()); assert!(request.take()); @@ -110,9 +110,9 @@ mod tests { #[test] fn failed_ticket_intake_ready_does_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); + let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); - hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, true)); + hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, true)); assert!(!request.is_requested()); } @@ -120,9 +120,9 @@ mod tests { #[test] fn non_intake_role_does_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketMarkReadyShutdownHook::new(request.clone(), false); + let hook = TicketIntakeReadyShutdownHook::new(request.clone(), false); - hook.observe_tool_result(&tool_result(TICKET_MARK_READY_TOOL_NAME, false)); + hook.observe_tool_result(&tool_result(TICKET_INTAKE_READY_TOOL_NAME, false)); assert!(!request.is_requested()); } @@ -130,7 +130,7 @@ mod tests { #[test] fn other_successful_tools_do_not_schedule_shutdown_after_idle() { let request = ShutdownAfterIdleRequest::default(); - let hook = TicketMarkReadyShutdownHook::new(request.clone(), true); + let hook = TicketIntakeReadyShutdownHook::new(request.clone(), true); hook.observe_tool_result(&tool_result("ShowTicket", false)); diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 7c91bd5f..a843004d 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3312,6 +3312,7 @@ async fn scoped_mark_ticket_ready_from_browser( operation_key: request.operation_key, reason: request.reason, author: Some("web".to_owned()), + intake_summary: None, }, ) .map_err(Error::from)?; @@ -3660,6 +3661,8 @@ struct TicketMarkReadyRequest { operation_key: String, #[serde(default)] reason: Option, + #[serde(default)] + intake_summary: Option, } async fn scoped_set_ticket_state_field( @@ -3717,6 +3720,7 @@ async fn scoped_mark_ticket_ready( operation_key: request.operation_key, reason: request.reason, author: None, + intake_summary: request.intake_summary, }, }, ) @@ -4538,7 +4542,12 @@ fn bind_worker_ticket_operation_source( | TicketBackendOperation::SetStateField { change, .. } | TicketBackendOperation::SetWorkflowState { change, .. } => change.author = Some(author), TicketBackendOperation::AddIntakeSummary { summary, .. } => summary.author = Some(author), - TicketBackendOperation::MarkReady { request, .. } => request.author = Some(author), + TicketBackendOperation::MarkReady { request, .. } => { + request.author = Some(author.clone()); + if let Some(summary) = request.intake_summary.as_mut() { + summary.author = Some(author); + } + } TicketBackendOperation::QueueReady { queued_by, .. } => *queued_by = author, TicketBackendOperation::AddTicketRelation { relation, .. } => { relation.author = Some(author) @@ -13896,6 +13905,7 @@ mod tests { operation_key: "ready-server-test".to_owned(), reason: Some("target accepted".to_owned()), author: Some("test".to_owned()), + intake_summary: None, }; let ready = backend .mark_ready(TicketIdOrSlug::Id(ticket_ref.id.clone()), request.clone()) @@ -13940,6 +13950,7 @@ mod tests { operation_key: "missing-repository".to_owned(), reason: None, author: None, + intake_summary: None, }, ), Err(ticket::TicketError::UnknownTargetRepository(_)) @@ -14072,6 +14083,7 @@ mod tests { operation_key: "notification-ready".to_owned(), reason: Some("ready for implementation".to_owned()), author: None, + intake_summary: None, }, }, TicketBackendOperation::QueueReady { @@ -14988,6 +15000,7 @@ mod tests { Json(TicketMarkReadyRequest { operation_key: "browser-ready".to_owned(), reason: Some("intake complete".to_owned()), + intake_summary: None, }), ) .await From 4c31ea2228f52a935445d8f835fd67b993d2cc67 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 17 Aug 2026 14:59:24 +0900 Subject: [PATCH 03/13] fix: check queue state before local readiness --- crates/ticket/src/lib.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index daf7350d..a7829c56 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -4319,6 +4319,12 @@ impl TicketBackend for LocalTicketBackend { let dir = self.find_ticket_dir(&id)?; let item = dir.join("item.md"); let meta = ticket_meta_for_dir(&dir, read_item_file(&item)?.frontmatter)?; + if meta.workflow_state != TicketWorkflowState::Ready { + return Err(TicketError::StaleWorkflowState { + expected: TicketWorkflowState::Ready.as_str().to_owned(), + actual: meta.workflow_state.as_str().to_owned(), + }); + } let ticket = self.ticket_from_dir(&dir)?; let target = resolve_ready_target(self.target_authority.as_ref(), "local", &ticket)?; let blockers = self.relation_blockers_for_meta(&meta)?; @@ -7642,7 +7648,7 @@ state: planning assert!(matches!( backend.queue_ready(TicketIdOrSlug::Id(ticket.id.clone()), "workspace-panel"), - Err(TicketError::Conflict(_)) + Err(TicketError::StaleWorkflowState { .. }) )); let record = backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(); assert_eq!(record.meta.workflow_state, TicketWorkflowState::Planning); From 71cd58f86864cf9269b0d5fcc7e2ab88220e37c6 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 17 Aug 2026 22:53:09 +0900 Subject: [PATCH 04/13] feat: scope merge request tools by profile flags --- crates/manifest/src/config.rs | 103 +++++++++- crates/manifest/src/lib.rs | 24 +++ crates/manifest/src/profile.rs | 73 ++++++++ crates/worker/src/controller.rs | 15 ++ .../src/feature/builtin/merge_request.rs | 176 +++++++++++++++--- crates/worker/src/feature/builtin/ticket.rs | 28 --- crates/worker/src/prompt/catalog.rs | 33 ++++ crates/worker/src/prompt/system.rs | 43 +++++ crates/worker/src/worker.rs | 22 +++ docs/development/work-items.md | 9 +- resources/flows/coder-review.dcdl | 8 +- resources/profiles/base.dcdl | 7 + resources/profiles/coder.dcdl | 7 + resources/profiles/orchestrator.dcdl | 7 + resources/profiles/reviewer.dcdl | 7 + resources/prompts/catalog.dcdl | 2 + resources/prompts/common/merge-request.md | 19 ++ resources/prompts/role/coder.md | 4 +- resources/prompts/role/reviewer.md | 2 +- 19 files changed, 522 insertions(+), 67 deletions(-) create mode 100644 resources/prompts/common/merge-request.md diff --git a/crates/manifest/src/config.rs b/crates/manifest/src/config.rs index 8be096c7..33f2b295 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -18,9 +18,10 @@ use crate::model::{AuthRef, ModelManifest, ReasoningControl}; use crate::plugin::PluginConfig; use crate::{ CompactionConfig, EngineManifest, FeatureConfig, FeatureFlagConfig, FileUploadLimits, - McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig, ScopeConfig, - SessionConfig, SkillsConfig, TicketFeatureConfig, ToolOutputLimits, ToolPermissionConfig, - ToolPermissionRule, WebConfig, WorkerFeatureConfig, WorkerManifest, WorkerMeta, + McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig, + MergeRequestFeatureConfig, ScopeConfig, SessionConfig, SkillsConfig, TicketFeatureConfig, + ToolOutputLimits, ToolPermissionConfig, ToolPermissionRule, WebConfig, WorkerFeatureConfig, + WorkerManifest, WorkerMeta, }; /// Partial-form Worker manifest. Every field is optional; one or more @@ -97,6 +98,8 @@ pub struct FeatureConfigPartial { #[serde(default)] pub ticket: Option, #[serde(default)] + pub merge_request: Option, + #[serde(default)] pub orchestration: Option, #[serde(default)] pub plugins: Option, @@ -127,6 +130,11 @@ impl FeatureConfigPartial { FeatureFlagConfigPartial::merge, ), ticket: merge_option(self.ticket, other.ticket, TicketFeatureConfigPartial::merge), + merge_request: merge_option( + self.merge_request, + other.merge_request, + MergeRequestFeatureConfigPartial::merge, + ), orchestration: merge_option( self.orchestration, other.orchestration, @@ -216,6 +224,28 @@ impl TicketFeatureConfigPartial { } } +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(default, deny_unknown_fields)] +pub struct MergeRequestFeatureConfigPartial { + pub show: Option, + pub open: Option, + pub review: Option, + pub readiness_check: Option, + pub complete: Option, +} + +impl MergeRequestFeatureConfigPartial { + fn merge(self, other: Self) -> Self { + Self { + show: other.show.or(self.show), + open: other.open.or(self.open), + review: other.review.or(self.review), + readiness_check: other.readiness_check.or(self.readiness_check), + complete: other.complete.or(self.complete), + } + } +} + impl From for FeatureConfig { fn from(value: FeatureConfigPartial) -> Self { Self { @@ -247,6 +277,10 @@ impl From for FeatureConfig { .ticket .map(TicketFeatureConfig::from) .unwrap_or_default(), + merge_request: value + .merge_request + .map(MergeRequestFeatureConfig::from) + .unwrap_or_default(), orchestration: value .orchestration .map(FeatureFlagConfig::from) @@ -326,6 +360,30 @@ impl From for TicketFeatureConfigPartial { } } +impl From for MergeRequestFeatureConfig { + fn from(value: MergeRequestFeatureConfigPartial) -> Self { + Self { + show: value.show.unwrap_or_default(), + open: value.open.unwrap_or_default(), + review: value.review.unwrap_or_default(), + readiness_check: value.readiness_check.unwrap_or_default(), + complete: value.complete.unwrap_or_default(), + } + } +} + +impl From for MergeRequestFeatureConfigPartial { + fn from(value: MergeRequestFeatureConfig) -> Self { + Self { + show: Some(value.show), + open: Some(value.open), + review: Some(value.review), + readiness_check: Some(value.readiness_check), + complete: Some(value.complete), + } + } +} + impl From for FeatureConfigPartial { fn from(value: FeatureConfig) -> Self { Self { @@ -339,6 +397,7 @@ impl From for FeatureConfigPartial { objective: Some(value.objective.into()), manage_workdir: Some(value.manage_workdir.into()), ticket: Some(value.ticket.into()), + merge_request: Some(value.merge_request.into()), orchestration: Some(value.orchestration.into()), plugins: Some(value.plugins.into()), } @@ -1880,6 +1939,7 @@ worker_max_turns = 7 assert!(!manifest.feature.objective.enabled); assert!(!manifest.feature.manage_workdir.enabled); assert!(!manifest.feature.ticket.enabled); + assert!(!manifest.feature.merge_request.any()); } #[test] @@ -1899,6 +1959,13 @@ thread = false intake = false workflow = false +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1934,6 +2001,14 @@ enabled = false assert!(!manifest.feature.ticket.thread); assert!(!manifest.feature.ticket.intake); assert!(!manifest.feature.ticket.workflow); + assert_eq!( + manifest.feature.merge_request, + MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert!(!manifest.feature.orchestration.enabled); assert!(!manifest.feature.memory.enabled); assert!(!manifest.feature.memory.staging); @@ -1957,6 +2032,13 @@ thread = false intake = false workflow = false +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1968,6 +2050,11 @@ enabled = false thread = true workflow = true +[feature.merge_request] +open = true +review = false +readiness_check = true + [feature.orchestration] enabled = true @@ -2017,6 +2104,16 @@ enabled = true assert!(manifest.feature.ticket.thread); assert!(!manifest.feature.ticket.intake); assert!(manifest.feature.ticket.workflow); + assert_eq!( + manifest.feature.merge_request, + MergeRequestFeatureConfig { + show: true, + open: true, + review: false, + readiness_check: true, + complete: false, + } + ); assert!(manifest.feature.orchestration.enabled); assert!(manifest.feature.objective.enabled); assert!(manifest.feature.web.enabled); diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 1f6d1f74..7c7c418c 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -125,6 +125,8 @@ pub struct FeatureConfig { #[serde(default)] pub ticket: TicketFeatureConfig, #[serde(default)] + pub merge_request: MergeRequestFeatureConfig, + #[serde(default)] pub orchestration: FeatureFlagConfig, #[serde(default)] pub plugins: FeatureFlagConfig, @@ -143,6 +145,7 @@ impl Default for FeatureConfig { objective: FeatureFlagConfig::disabled(), manage_workdir: FeatureFlagConfig::disabled(), ticket: TicketFeatureConfig::default(), + merge_request: MergeRequestFeatureConfig::default(), orchestration: FeatureFlagConfig::disabled(), plugins: FeatureFlagConfig::disabled(), } @@ -252,6 +255,27 @@ pub struct TicketFeatureConfig { pub workflow: bool, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(deny_unknown_fields)] +pub struct MergeRequestFeatureConfig { + #[serde(default)] + pub show: bool, + #[serde(default)] + pub open: bool, + #[serde(default)] + pub review: bool, + #[serde(default)] + pub readiness_check: bool, + #[serde(default)] + pub complete: bool, +} + +impl MergeRequestFeatureConfig { + pub fn any(self) -> bool { + self.show || self.open || self.review || self.readiness_check || self.complete + } +} + /// External Agent Skills (`SKILL.md`) ingest configuration. Skills are /// loaded *only* from the directories listed here — there is no /// implicit `$config_dir/skills/` or builtin probe. Profile and Manifest diff --git a/crates/manifest/src/profile.rs b/crates/manifest/src/profile.rs index 65b8d99c..3ace3501 100644 --- a/crates/manifest/src/profile.rs +++ b/crates/manifest/src/profile.rs @@ -919,6 +919,37 @@ fn apply_role_profile( _ => serde_json::json!({ "enabled": true, "authoring": true, "thread": true }), }; value["feature"]["ticket"] = ticket; + let merge_request = match slug { + "coder" => serde_json::json!({ + "show": true, + "open": true, + "review": false, + "readiness_check": false, + "complete": false + }), + "reviewer" => serde_json::json!({ + "show": true, + "open": false, + "review": true, + "readiness_check": false, + "complete": false + }), + "orchestrator" => serde_json::json!({ + "show": true, + "open": false, + "review": false, + "readiness_check": true, + "complete": true + }), + _ => serde_json::json!({ + "show": false, + "open": false, + "review": false, + "readiness_check": false, + "complete": false + }), + }; + value["feature"]["merge_request"] = merge_request; } fn reject_manifest_shaped_profile(value: &serde_json::Value) -> Result<(), ProfileError> { @@ -1361,6 +1392,7 @@ mod tests { assert!(companion.feature.objective.enabled); assert!(!companion.feature.ticket.intake); assert!(!companion.feature.orchestration.enabled); + assert!(!companion.feature.merge_request.any()); assert_eq!( companion.compaction.as_ref().unwrap().threshold, Some(240000) @@ -1401,6 +1433,15 @@ mod tests { assert!(!orchestrator.feature.sub_worker.enabled); assert!(orchestrator.feature.worker.enabled); assert!(!orchestrator.feature.worker.direct_spawn); + assert_eq!( + orchestrator.feature.merge_request, + crate::MergeRequestFeatureConfig { + show: true, + readiness_check: true, + complete: true, + ..Default::default() + } + ); assert!(orchestrator.feature.ticket.enabled); assert!(orchestrator.feature.ticket.enabled); assert!(!orchestrator.feature.ticket.authoring); @@ -1424,6 +1465,14 @@ mod tests { assert!(coder.feature.sub_worker.enabled); assert!(coder.feature.flow.enabled); assert!(!coder.feature.worker.enabled); + assert_eq!( + coder.feature.merge_request, + crate::MergeRequestFeatureConfig { + show: true, + open: true, + ..Default::default() + } + ); assert!(coder.scope.allow.is_empty()); assert!(coder.delegation_scope.allow.is_empty()); assert_eq!(coder.model.ref_.as_deref(), Some("codex-oauth/gpt-5.5")); @@ -1442,6 +1491,14 @@ mod tests { assert!(!reviewer.feature.sub_worker.enabled); assert!(!reviewer.feature.flow.enabled); assert!(!reviewer.feature.worker.enabled); + assert_eq!( + reviewer.feature.merge_request, + crate::MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert!(reviewer.feature.ticket.enabled); assert!(reviewer.feature.ticket.enabled); assert!(!reviewer.feature.ticket.authoring); @@ -1604,6 +1661,14 @@ enabled = true authoring = false thread = false intake = false + +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1626,6 +1691,14 @@ enabled = false assert!(!resolved.manifest.feature.ticket.authoring); assert!(!resolved.manifest.feature.ticket.thread); assert!(!resolved.manifest.feature.ticket.intake); + assert_eq!( + resolved.manifest.feature.merge_request, + crate::MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert!(!resolved.manifest.feature.orchestration.enabled); assert_eq!( resolved.manifest.delegation_scope.allow[0].target, diff --git a/crates/worker/src/controller.rs b/crates/worker/src/controller.rs index ebb07952..e952a46b 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -769,6 +769,21 @@ where ), ); } + if feature_config.merge_request.any() { + let workspace_client = worker.workspace_client_handle(); + if !workspace_client.is_available() || workspace_client.workspace_id().is_none() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "Merge Request tools require Backend Workspace API authority", + )); + } + feature_registry.add_module( + crate::feature::builtin::merge_request::MergeRequestFeature::new( + workspace_client, + feature_config.merge_request, + ), + ); + } if feature_config.manage_workdir.enabled { // Workdir lifecycle is Workspace control-plane authority. The Worker // receives only the injected WorkspaceClient and never Runtime URLs, diff --git a/crates/worker/src/feature/builtin/merge_request.rs b/crates/worker/src/feature/builtin/merge_request.rs index b2dd5c57..832789d2 100644 --- a/crates/worker/src/feature/builtin/merge_request.rs +++ b/crates/worker/src/feature/builtin/merge_request.rs @@ -1,19 +1,41 @@ -use crate::feature::ToolDefinition; +use crate::feature::{ + FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution, + FeatureInstructionDeclaration, FeatureInstructionId, FeatureModule, ToolContribution, + ToolDeclaration, ToolDefinition, +}; use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod}; use async_trait::async_trait; use llm_engine::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; +use manifest::MergeRequestFeatureConfig; use schemars::JsonSchema; use serde::Deserialize; use serde_json::json; use std::sync::Arc; -pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[ - "MergeRequestShow", - "MergeRequestReadinessCheck", - "MergeRequestOpen", - "MergeRequestComplete", +pub const FEATURE_ID: &str = "merge_request"; +const FEATURE_NAME: &str = "Merge Request tools"; +const FEATURE_DESCRIPTION: &str = + "Operation-specific Merge Request workflow tools over Workspace authority."; +const FEATURE_INSTRUCTION_ID: &str = "merge_request.workflow"; +pub const FEATURE_PROMPT_REF: &str = "common.merge_request"; + +fn workflow_instruction() -> FeatureInstructionDeclaration { + FeatureInstructionDeclaration::new( + FeatureInstructionId::builtin(FEATURE_INSTRUCTION_ID), + FEATURE_PROMPT_REF, + "Operation-specific Merge Request workflow guidance", + ) + .expect("static Merge Request workflow instruction declaration is valid") +} + +const ALL_KINDS: [Kind; 5] = [ + Kind::Show, + Kind::Open, + Kind::Review, + Kind::Readiness, + Kind::Complete, ]; -pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit"; + #[derive(Clone, Copy)] enum Kind { Show, @@ -89,13 +111,23 @@ struct ReviewFindingInput { body: String, } impl Kind { + fn enabled(self, config: MergeRequestFeatureConfig) -> bool { + match self { + Self::Show => config.show, + Self::Open => config.open, + Self::Review => config.review, + Self::Readiness => config.readiness_check, + Self::Complete => config.complete, + } + } + fn name(self) -> &'static str { match self { Self::Show => "MergeRequestShow", Self::Readiness => "MergeRequestReadinessCheck", Self::Open => "MergeRequestOpen", Self::Complete => "MergeRequestComplete", - Self::Review => "MergeRequestReviewSubmit", + Self::Review => "MergeRequestReview", } } fn schema(self) -> serde_json::Value { @@ -218,24 +250,53 @@ fn definition(client: Arc, kind: Kind) -> ToolDefinition { ) }) } -pub fn common_tools(c: Arc) -> Vec { - vec![ - definition(c.clone(), Kind::Show), - definition(c.clone(), Kind::Readiness), - definition(c.clone(), Kind::Open), - definition(c, Kind::Complete), - ] +pub struct MergeRequestFeature { + client: Arc, + config: MergeRequestFeatureConfig, } -pub fn reviewer_tools(c: Arc) -> Vec { - if c.reviewer_context().is_some() { - vec![ - definition(c.clone(), Kind::Show), - definition(c, Kind::Review), - ] - } else { - vec![] + +impl MergeRequestFeature { + pub fn new(client: Arc, config: MergeRequestFeatureConfig) -> Self { + Self { client, config } + } + + fn kinds(&self) -> impl Iterator + '_ { + ALL_KINDS + .into_iter() + .filter(|kind| kind.enabled(self.config)) } } + +impl FeatureModule for MergeRequestFeature { + fn descriptor(&self) -> FeatureDescriptor { + let mut descriptor = FeatureDescriptor::builtin(FEATURE_ID, FEATURE_NAME) + .with_description(FEATURE_DESCRIPTION); + if self.config.any() { + descriptor = descriptor.with_instruction(workflow_instruction()); + } + for kind in self.kinds() { + descriptor = descriptor.with_tool(ToolDeclaration::new( + kind.name(), + description(kind.name()).unwrap_or("Merge Request operation."), + )); + } + descriptor + } + + fn install(&self, ctx: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError> { + if self.config.any() { + ctx.instructions() + .register(FeatureInstructionContribution::new(workflow_instruction()))?; + } + let mut tools = ctx.tools(); + for kind in self.kinds() { + let definition = definition(self.client.clone(), kind); + tools.register(ToolContribution::new(kind.name(), definition))?; + } + Ok(()) + } +} + pub fn description(n: &str) -> Option<&'static str> { match n { "MergeRequestShow" => Some("Read the selector-based Merge Request and append-only thread."), @@ -248,7 +309,7 @@ pub fn description(n: &str) -> Option<&'static str> { "MergeRequestComplete" => { Some("Complete using an approved review event and final target-ref evidence.") } - "MergeRequestReviewSubmit" => { + "MergeRequestReview" => { Some("Submit the injected Reviewer capability result for its captured subject ref.") } _ => None, @@ -257,6 +318,72 @@ pub fn description(n: &str) -> Option<&'static str> { #[cfg(test)] mod tests { use super::*; + use crate::feature::FeatureRegistryBuilder; + use crate::hook::HookRegistryBuilder; + use crate::worker::TestWorkspaceHttpClient; + + fn install(config: MergeRequestFeatureConfig) -> (Vec, Vec) { + let client: Arc = + Arc::new(TestWorkspaceHttpClient::new("workspace", "http://unused")); + let mut pending_tools = Vec::new(); + let mut hook_builder = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(MergeRequestFeature::new(client, config)) + .install_into_pending(&mut pending_tools, &mut hook_builder); + assert!(!report.has_errors(), "{}", report.error_message()); + ( + report.installed_tool_names(), + report + .installed_instruction_contributions() + .into_iter() + .map(|instruction| instruction.prompt_ref) + .collect(), + ) + } + + fn tool_names(config: MergeRequestFeatureConfig) -> Vec { + install(config).0 + } + + #[test] + fn flags_define_the_exact_registered_tool_surface() { + let coder = MergeRequestFeatureConfig { + show: true, + open: true, + ..Default::default() + }; + assert_eq!(tool_names(coder), ["MergeRequestShow", "MergeRequestOpen"]); + + let reviewer = MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + }; + assert_eq!( + tool_names(reviewer), + ["MergeRequestShow", "MergeRequestReview"] + ); + + let orchestrator = MergeRequestFeatureConfig { + show: true, + readiness_check: true, + complete: true, + ..Default::default() + }; + assert_eq!( + tool_names(orchestrator), + [ + "MergeRequestShow", + "MergeRequestReadinessCheck", + "MergeRequestComplete" + ] + ); + assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]); + let unspecified = install(MergeRequestFeatureConfig::default()); + assert!(unspecified.0.is_empty()); + assert!(unspecified.1.is_empty()); + } + #[test] fn schemas_hide_revision_and_commit_authority() { let schemas = [ @@ -276,6 +403,5 @@ mod tests { assert!(!j.contains(banned), "{banned} in {j}") } } - assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRequestReview")); } } diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index ac461c13..cb2fb57c 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -24,7 +24,6 @@ use ticket::{ tool::{TICKET_TOOL_NAMES, TicketToolBackend, ticket_tool_description, ticket_tools}, }; -use super::merge_request; use crate::feature::{ FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId, @@ -588,22 +587,6 @@ impl FeatureModule for TicketFeature { ticket_tool_description(name, self.record_language.as_deref()), )); } - if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend { - let names: Vec<&str> = if client.reviewer_context().is_some() { - vec![ - "MergeRequestShow", - merge_request::MERGE_REQUEST_REVIEW_TOOL_NAME, - ] - } else { - merge_request::MERGE_REQUEST_COMMON_TOOL_NAMES.to_vec() - }; - for name in names { - descriptor = descriptor.with_tool(ToolDeclaration::new( - name, - merge_request::description(name).unwrap_or("Merge Request operation."), - )); - } - } descriptor } @@ -661,17 +644,6 @@ impl FeatureModule for TicketFeature { }; tools.register(ToolContribution::new(name, definition))?; } - if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend { - let definitions = if client.reviewer_context().is_some() { - merge_request::reviewer_tools(client.clone()) - } else { - merge_request::common_tools(client.clone()) - }; - for definition in definitions { - let (meta, _) = definition(); - tools.register(ToolContribution::new(meta.name.clone(), definition))?; - } - } Ok(()) } } diff --git a/crates/worker/src/prompt/catalog.rs b/crates/worker/src/prompt/catalog.rs index 82518b90..53981905 100644 --- a/crates/worker/src/prompt/catalog.rs +++ b/crates/worker/src/prompt/catalog.rs @@ -527,6 +527,39 @@ mod tests { ); } + #[test] + fn merge_request_instruction_matches_the_exposed_operations() { + let catalog = PromptCatalog::builtins_only().unwrap(); + let coder = catalog + .render_name( + "common.merge_request", + Value::from_serialize(serde_json::json!({ + "tools": ["MergeRequestShow", "MergeRequestOpen"] + })), + ) + .unwrap(); + assert!(coder.contains("Reread the current Merge Request")); + assert!(coder.contains("Open the Merge Request only after")); + assert!(!coder.contains("Submit the authoritative verdict")); + assert!(!coder.contains("Complete integration only after")); + + let orchestrator = catalog + .render_name( + "common.merge_request", + Value::from_serialize(serde_json::json!({ + "tools": [ + "MergeRequestShow", + "MergeRequestReadinessCheck", + "MergeRequestComplete" + ] + })), + ) + .unwrap(); + assert!(orchestrator.contains("Use `MergeRequestReadinessCheck`")); + assert!(orchestrator.contains("Complete integration only after")); + assert!(!orchestrator.contains("Open the Merge Request only after")); + } + #[test] fn commit_capable_roles_classify_commits_by_change_type() { let catalog = PromptCatalog::builtins_only().unwrap(); diff --git a/crates/worker/src/prompt/system.rs b/crates/worker/src/prompt/system.rs index d043b703..32f291bd 100644 --- a/crates/worker/src/prompt/system.rs +++ b/crates/worker/src/prompt/system.rs @@ -319,6 +319,7 @@ fn append_trailing_section( #[cfg(test)] mod tests { use super::*; + use crate::feature::FeatureInstructionId; use chrono::TimeZone; use manifest::{Permission, ScopeConfig, ScopeRule}; use tempfile::TempDir; @@ -400,6 +401,48 @@ mod tests { } } + #[test] + fn merge_request_role_prompts_match_operation_specific_tool_surfaces() { + fn render(role: &str, tools: &[&str]) -> String { + let tmp = TempDir::new().unwrap(); + let scope = build_scope(tmp.path()); + let prompts = PromptCatalog::builtins_only().unwrap(); + let template = + SystemPromptTemplate::parse(role, PromptCatalogSource::builtins_only()).unwrap(); + let instruction = FeatureInstructionDeclaration::new( + FeatureInstructionId::builtin("merge_request.workflow"), + "common.merge_request", + "Merge Request workflow", + ) + .unwrap(); + let mut ctx = context(tmp.path(), &scope, &prompts); + ctx.tool_names = tools.iter().map(|name| (*name).to_string()).collect(); + ctx.feature_instructions = std::slice::from_ref(&instruction); + template.render(&ctx).unwrap() + } + + let coder = render("role.coder", &["MergeRequestShow", "MergeRequestOpen"]); + assert!(coder.contains("Open the Merge Request only after")); + assert!(!coder.contains("Complete integration only after")); + assert!(coder.contains("Do not call `MergeRequestComplete`")); + + let reviewer = render("role.reviewer", &["MergeRequestShow", "MergeRequestReview"]); + assert!(reviewer.contains("Submit the authoritative verdict")); + assert!(!reviewer.contains("Open the Merge Request only after")); + + let orchestrator = render( + "role.orchestrator", + &[ + "MergeRequestShow", + "MergeRequestReadinessCheck", + "MergeRequestComplete", + ], + ); + assert!(orchestrator.contains("Use `MergeRequestReadinessCheck`")); + assert!(orchestrator.contains("Complete integration only after")); + assert!(!orchestrator.contains("Submit the authoritative verdict")); + } + #[test] fn role_templates_are_selected_without_filesystem_resolution() { let loader = PromptCatalogSource::builtins_only(); diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 40a0ec7e..2830014f 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -5890,6 +5890,13 @@ model_id = "claude-sonnet-4-20250514" [engine] instruction = "saved" +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [[scope.allow]] target = "/snapshot/workspace" permission = "read" @@ -5912,6 +5919,13 @@ model_id = "claude-sonnet-4-20250514" [engine] instruction = "current" +[feature.merge_request] +show = true +open = true +review = true +readiness_check = true +complete = true + [[scope.allow]] target = "/current/workspace" permission = "write" @@ -5931,6 +5945,14 @@ permission = "write" .unwrap(); assert_eq!(restored.engine.instruction, "saved"); + assert_eq!( + restored.feature.merge_request, + manifest::MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert_eq!(restored.scope.allow.len(), 1); assert_eq!( restored.scope.allow[0].target, diff --git a/docs/development/work-items.md b/docs/development/work-items.md index 8e096fea..ebd29c50 100644 --- a/docs/development/work-items.md +++ b/docs/development/work-items.md @@ -31,14 +31,15 @@ Maintainers can inspect the local `.yoi/tickets/` files directly when debugging ## Ticket tools inside Workers -Workers with the Ticket built-in feature can use typed Ticket tools: +Workers with the Ticket and operation-specific Merge Request built-in features can use typed workflow tools: - `TicketCreate` - `QueryTicket` — bounded authoritative Ticket discovery with typed state/text/event/evidence/relation/Objective/time/attention filters, stable snippets, and cursor metadata. - `ShowTicket` — detailed authority for one Ticket, including item revision, bounded thread/event references, relations, linked Objectives, implementation reports, and current Merge Request/review evidence. - `TicketComment` -- `MergeRequestShow`, `MergeRequestOpen`, `MergeRequestAddRevision`, `MergeRequestComplete` -- `MergeRequestReviewSubmit` — available only inside the attested direct-child Reviewer attempt; attempt/revision capability material is not model input. +- Coder: `MergeRequestShow`, `MergeRequestOpen` +- Reviewer: `MergeRequestShow`, `MergeRequestReview` — available only inside the attested direct-child Reviewer request; grant and subject-ref capability material are not model input. +- Orchestrator: `MergeRequestShow`, `MergeRequestReadinessCheck`, `MergeRequestComplete` - `TicketClose` - `TicketRelationRecord` @@ -243,7 +244,7 @@ Implementation normally happens in a child git worktree created by the Orchestra 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 Reviewer records the structured result with `MergeRequestReviewSubmit`. Request changes requires a new immutable revision and a fresh child attempt. `MergeRequestComplete` performs guarded Ticket completion with operation-id dedupe/CAS semantics; Flow transitions are not completion authority. +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. Blockers must be fixed or explicitly escalated before merge-ready submission. diff --git a/resources/flows/coder-review.dcdl b/resources/flows/coder-review.dcdl index c191cd1b..c01b1d1d 100644 --- a/resources/flows/coder-review.dcdl +++ b/resources/flows/coder-review.dcdl @@ -15,7 +15,7 @@ }; review = { - instructions = "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 child must commit MergeRequestReviewSubmit; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition."; + instructions = "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 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"; @@ -39,17 +39,17 @@ }; complete = { - instructions = "Call MergeRequestComplete with a fresh operation_id and the approved current revision. The Server must revalidate current assignment, immutable revision, registered Reviewer attempt, and Ticket inprogress CAS. Only after the authoritative operation returns Ticket state done, request a Flow transition."; + instructions = "Leave concise implementation and validation evidence on the Ticket, then hand off the exact approved Merge Request revision to the Orchestrator for readiness and integration. Do not call MergeRequestComplete; Coder approval handoff is not Ticket completion authority. After durable handoff evidence exists, request a Flow transition."; transitions = { completed = { target = "done"; - condition = "MergeRequestComplete durably returned done for this exact operation_id and current approved revision. A Flow state or prose report alone is never sufficient."; + condition = "The exact approved Merge Request revision and implementation evidence have been durably handed off to the Orchestrator. A Flow state or prose report alone is never Ticket completion authority."; }; }; }; done = { - instructions = "The guarded Merge Request completion operation committed Ticket state done. Flow terminal state only reflects that durable authority."; + instructions = "The approved implementation has been handed off for Orchestrator-owned readiness and integration. Flow terminal state only reflects that handoff."; terminal = true; }; }; diff --git a/resources/profiles/base.dcdl b/resources/profiles/base.dcdl index 04148c7b..2e960d15 100644 --- a/resources/profiles/base.dcdl +++ b/resources/profiles/base.dcdl @@ -30,6 +30,13 @@ feature = { worker = { enabled = false; }; objective = { enabled = true; }; ticket = { enabled = true; authoring = true; thread = true; }; + merge_request = { + show = false; + open = false; + review = false; + readiness_check = false; + complete = false; + }; }; memory = { diff --git a/resources/profiles/coder.dcdl b/resources/profiles/coder.dcdl index 27a85776..93713b55 100644 --- a/resources/profiles/coder.dcdl +++ b/resources/profiles/coder.dcdl @@ -12,5 +12,12 @@ import "./base.dcdl" // { flow = { enabled = true; }; worker = { enabled = true; }; ticket = { enabled = true; thread = true; }; + merge_request = { + show = true; + open = true; + review = false; + readiness_check = false; + complete = false; + }; }; } diff --git a/resources/profiles/orchestrator.dcdl b/resources/profiles/orchestrator.dcdl index feb7f402..cb654b63 100644 --- a/resources/profiles/orchestrator.dcdl +++ b/resources/profiles/orchestrator.dcdl @@ -12,6 +12,13 @@ import "./base.dcdl" // { worker = { enabled = true; direct_spawn = false; }; manage_workdir = { enabled = true; }; ticket = { enabled = true; thread = true; workflow = true; }; + merge_request = { + show = true; + open = false; + review = false; + readiness_check = true; + complete = true; + }; orchestration = { enabled = true; }; }; } diff --git a/resources/profiles/reviewer.dcdl b/resources/profiles/reviewer.dcdl index 82270494..b41f9627 100644 --- a/resources/profiles/reviewer.dcdl +++ b/resources/profiles/reviewer.dcdl @@ -11,5 +11,12 @@ import "./base.dcdl" // { sub_worker = { enabled = false; }; worker = { enabled = false; }; ticket = { enabled = true; thread = false; }; + merge_request = { + show = true; + open = false; + review = true; + readiness_check = false; + complete = false; + }; }; } diff --git a/resources/prompts/catalog.dcdl b/resources/prompts/catalog.dcdl index 84dd4b2f..1144f3a3 100644 --- a/resources/prompts/catalog.dcdl +++ b/resources/prompts/catalog.dcdl @@ -6,6 +6,7 @@ let defaultDocument = import "./default.md"; commonLanguage = import "./common/language.md"; commonGit = import "./common/git.md"; +commonMergeRequest = import "./common/merge-request.md"; commonTickets = import "./common/tickets.md"; commonToolUsage = import "./common/tool-usage.md"; commonWorkerObservation = import "./common/worker-observation.md"; @@ -36,6 +37,7 @@ in common = { git = commonGit.content; language = commonLanguage.content; + merge_request = commonMergeRequest.content; tickets = commonTickets.content; tool_usage = commonToolUsage.content; worker_observation = commonWorkerObservation.content; diff --git a/resources/prompts/common/merge-request.md b/resources/prompts/common/merge-request.md new file mode 100644 index 00000000..4e7a8996 --- /dev/null +++ b/resources/prompts/common/merge-request.md @@ -0,0 +1,19 @@ +## Merge Request workflow + +Use only the exposed Merge Request operations; their availability expresses this Worker's workflow responsibility, not authorization to bypass Backend validation. +{% if "MergeRequestShow" in tools %} +- Reread the current Merge Request and append-only thread with `MergeRequestShow` before making review or integration decisions. +{% endif %} +{% if "MergeRequestOpen" in tools %} +- Open the Merge Request only after all intended changes are committed and the Workdir is clean. Use immutable source and target selectors; do not infer target authority from a branch name or cwd. +- Before requesting independent review, make the exact current MR revision authoritative. +{% endif %} +{% if "MergeRequestReview" in tools %} +- Review the exact current immutable MR revision independently. Submit the authoritative verdict through `MergeRequestReview`; prose alone is not approval. +{% endif %} +{% if "MergeRequestReadinessCheck" in tools %} +- Use `MergeRequestReadinessCheck` to resolve current refs and authoritative review readiness before integration. +{% endif %} +{% if "MergeRequestComplete" in tools %} +- Complete integration only after readiness confirms approval for the exact current revision and all target/ref guards pass. Merge completion is separate from implementation and review evidence. +{% endif %} diff --git a/resources/prompts/role/coder.md b/resources/prompts/role/coder.md index adfa0e54..29932418 100644 --- a/resources/prompts/role/coder.md +++ b/resources/prompts/role/coder.md @@ -4,6 +4,6 @@ Treat the first committed user message as the bounded Ticket/action context and {% 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 `MergeRequestReviewSubmit` 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 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. -A request-changes result requires a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. Complete only through `MergeRequestComplete` with a unique operation id, the approved `Review` event id, and final target-ref evidence; the Server re-resolves selectors, revalidates assignment, and fences Ticket state side effects. +A request-changes result requires a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request revision has authoritative approval, leave concise implementation evidence on the Ticket and hand off integration to the Orchestrator. Do not call `MergeRequestComplete`. diff --git a/resources/prompts/role/reviewer.md b/resources/prompts/role/reviewer.md index 112c412c..8f2c05cc 100644 --- a/resources/prompts/role/reviewer.md +++ b/resources/prompts/role/reviewer.md @@ -2,6 +2,6 @@ You are the Ticket Reviewer role running as an actual Runtime-owned direct child Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only. 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, or take over implementation. -Your prose response is not review authority. Before finishing, call `MergeRequestReviewSubmit` 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. +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. Review more than the diff: verify the implementation satisfies the Ticket intent and acceptance criteria, remains coherent with the codebase design, and does not introduce unnecessary compatibility. From bc835b8503aab1b6dc9155030c0a44d6e47e0e54 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 03:08:30 +0900 Subject: [PATCH 05/13] feat: add bounded Ticket and Objective query pages --- crates/ticket/src/lib.rs | 187 +++++- crates/ticket/src/sqlite_schema.rs | 40 +- crates/workspace-server/src/authority.rs | 619 ++++++++++++++++-- crates/workspace-server/src/records.rs | 19 + crates/workspace-server/src/server.rs | 29 +- crates/workspace-server/src/store.rs | 43 +- web/workspace/src/lib/generated/ticket-api.ts | 1 + 7 files changed, 849 insertions(+), 89 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 7abcf7a3..909ae183 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -93,6 +93,26 @@ fn io_err(path: impl Into, source: io::Error) -> TicketError { } } +fn read_ticket_summary_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let workflow_state = row.get::<_, String>(7)?; + Ok(TicketSummary { + id: row.get(0)?, + slug: row.get(1)?, + title: row.get(2)?, + status: ExtensibleTicketStatus::from(row.get::<_, String>(3)?.as_str()), + kind: row.get(4)?, + priority: row.get(5)?, + labels: Vec::new(), + readiness: row.get(6)?, + workflow_state: TicketWorkflowState::parse(&workflow_state) + .unwrap_or(TicketWorkflowState::Planning), + workflow_state_explicit: row.get::<_, i64>(8)? != 0, + queued_by: row.get(9)?, + queued_at: row.get(10)?, + updated_at: row.get(11)?, + }) +} + fn sqlite_err(error: impl std::fmt::Display) -> TicketError { TicketError::Sqlite(error.to_string()) } @@ -1404,6 +1424,26 @@ pub struct SqliteTicketListProjection { pub items: Vec, } +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct SqliteTicketListCursor { + pub updated_at: Option, + pub ticket_id: String, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct SqliteTicketListPageQuery { + pub states: Vec, + pub limit: usize, + pub after: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SqliteTicketListPage { + pub items: Vec, + pub has_more: bool, + pub next: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TicketInvalidRecord { pub label: String, @@ -2354,6 +2394,89 @@ impl SqliteTicketBackend { }) } + /// Lists one stable keyset-paginated Workspace summary page. + /// + /// Filtering, ordering, and `limit + 1` are applied by SQLite before the bounded blocker + /// hydration query. The returned cursor is storage data; callers must wrap it in their own + /// opaque, query-bound transport cursor. + pub fn list_workspace_projection_page( + &self, + query: SqliteTicketListPageQuery, + ) -> Result { + self.with_read(|conn| { + let states = serde_json::to_string( + &query + .states + .iter() + .map(|state| state.as_str()) + .collect::>(), + ) + .map_err(|error| TicketError::Sqlite(error.to_string()))?; + let cursor_updated_at = query + .after + .as_ref() + .and_then(|cursor| cursor.updated_at.clone()); + let cursor_id = query + .after + .as_ref() + .map(|cursor| cursor.ticket_id.as_str()); + let fetch_limit = query.limit.saturating_add(1); + let mut statement = conn + .prepare( + "SELECT ticket_id, slug, title, status, kind, priority, readiness, + workflow_state, workflow_state_explicit, queued_by, queued_at, updated_at + FROM typed_tickets AS ticket + WHERE workspace_id = ?1 + AND (json_array_length(?2) = 0 OR EXISTS ( + SELECT 1 FROM json_each(?2) AS state + WHERE state.value = ticket.workflow_state + )) + AND (?3 IS NULL OR COALESCE(ticket.updated_at, '') < COALESCE(?4, '') + OR (COALESCE(ticket.updated_at, '') = COALESCE(?4, '') + AND ticket.ticket_id > ?3)) + ORDER BY ticket.updated_at DESC, ticket.ticket_id ASC + LIMIT ?5", + ) + .map_err(sqlite_err)?; + let rows = statement + .query_map( + params![ + self.workspace_id, + states, + cursor_id, + cursor_updated_at, + i64::try_from(fetch_limit).unwrap_or(i64::MAX) + ], + read_ticket_summary_row, + ) + .map_err(sqlite_err)?; + let mut summaries = rows + .collect::, _>>() + .map_err(sqlite_err)?; + let has_more = summaries.len() > query.limit; + summaries.truncate(query.limit); + let next = has_more.then(|| { + let summary = summaries.last().expect("non-empty page with continuation"); + SqliteTicketListCursor { + updated_at: summary.updated_at.clone(), + ticket_id: summary.id.clone(), + } + }); + let blockers = self.list_workspace_blockers(conn, &summaries)?; + Ok(SqliteTicketListPage { + items: summaries + .into_iter() + .map(|summary| SqliteTicketListItem { + relation_blockers: blockers.get(&summary.id).cloned().unwrap_or_default(), + summary, + }) + .collect(), + has_more, + next, + }) + }) + } + fn list_workspace_summaries( &self, conn: &Connection, @@ -6745,6 +6868,66 @@ state: planning assert!(!debug.contains("full-artifact-marker")); } + #[test] + fn sqlite_workspace_projection_page_uses_stable_keyset_and_state_filter() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("workspace.db"); + let backend = SqliteTicketBackend::open(&db_path, "workspace-test").unwrap(); + let mut ids = Vec::new(); + for (title, state, updated_at) in [ + ("Newest", TicketWorkflowState::Ready, "2026-08-12T03:00:00Z"), + ("Middle", TicketWorkflowState::Ready, "2026-08-12T02:00:00Z"), + ( + "Oldest", + TicketWorkflowState::Planning, + "2026-08-12T01:00:00Z", + ), + ] { + let mut input = NewTicket::new(title); + input.workflow_state = Some(state); + let ticket = backend.create(input).unwrap(); + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at=?3 WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", ticket.id, updated_at], + ) + .unwrap(); + ids.push(ticket.id); + } + + let first = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready], + limit: 1, + after: None, + }) + .unwrap(); + assert_eq!(first.items[0].summary.id, ids[0]); + assert!(first.has_more); + + let mut inserted = NewTicket::new("Inserted"); + inserted.workflow_state = Some(TicketWorkflowState::Ready); + let inserted = backend.create(inserted).unwrap(); + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at='2026-08-12T04:00:00Z' WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", inserted.id], + ) + .unwrap(); + + let second = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready], + limit: 1, + after: first.next, + }) + .unwrap(); + assert_eq!(second.items[0].summary.id, ids[1]); + assert!(!second.has_more); + } + #[test] fn sqlite_workspace_projection_sql_shape_is_constant_for_ticket_count() { let source = include_str!("lib.rs"); @@ -6756,8 +6939,8 @@ state: planning .map(|offset| start + offset) .expect("following method"); let projection_source = &source[start..end]; - assert_eq!(projection_source.matches("self.with_read(").count(), 1); - assert_eq!(projection_source.matches(".prepare(").count(), 2); + assert_eq!(projection_source.matches("self.with_read(").count(), 2); + assert_eq!(projection_source.matches(".prepare(").count(), 3); let item_loop = projection_source .split("items: summaries") .nth(1) diff --git a/crates/ticket/src/sqlite_schema.rs b/crates/ticket/src/sqlite_schema.rs index 4f876294..ebbbf8f2 100644 --- a/crates/ticket/src/sqlite_schema.rs +++ b/crates/ticket/src/sqlite_schema.rs @@ -7,7 +7,7 @@ use crate::{Result, TicketError, sqlite_err}; const MIGRATION_TABLE: &str = "ticket_schema_migrations"; const MAX_SCHEMA_DIAGNOSTICS: usize = 32; -pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 3; +pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 4; #[derive(Clone, Copy)] struct Migration { @@ -32,6 +32,11 @@ const MIGRATIONS: &[Migration] = &[ name: "convert_legacy_reviews_to_comments", apply: retire_legacy_ticket_review_events, }, + Migration { + version: 4, + name: "add_ticket_query_indexes", + apply: add_ticket_query_indexes, + }, ]; #[derive(Clone, Copy)] @@ -507,6 +512,29 @@ fn retire_legacy_ticket_review_events(connection: &Connection) -> Result<()> { .map_err(sqlite_err) } +fn add_ticket_query_indexes(connection: &Connection) -> Result<()> { + connection + .execute_batch( + r#" + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_state_updated + ON typed_tickets(workspace_id, workflow_state, updated_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_updated + ON typed_tickets(workspace_id, updated_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_created + ON typed_tickets(workspace_id, created_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_title + ON typed_tickets(workspace_id, title COLLATE NOCASE, ticket_id); + CREATE INDEX IF NOT EXISTS typed_ticket_events_workspace_kind_ticket + ON typed_ticket_events(workspace_id, kind, ticket_id, event_index); + CREATE INDEX IF NOT EXISTS typed_ticket_relations_workspace_source_kind + ON typed_ticket_relations(workspace_id, ticket_id, kind, target); + CREATE INDEX IF NOT EXISTS typed_ticket_relations_workspace_target_kind + ON typed_ticket_relations(workspace_id, target, kind, ticket_id); + "#, + ) + .map_err(sqlite_err) +} + fn add_column_if_missing( connection: &Connection, table: &str, @@ -841,10 +869,10 @@ mod tests { verify_sqlite_ticket_schema(&connection).unwrap(); let versions = load_applied_migrations(&connection).unwrap(); - assert_eq!(versions.len(), 3); + assert_eq!(versions.len(), 4); assert_eq!( versions.get(&LATEST_SQLITE_TICKET_SCHEMA_VERSION), - Some(&"convert_legacy_reviews_to_comments".to_string()) + Some(&"add_ticket_query_indexes".to_string()) ); } @@ -989,7 +1017,7 @@ mod tests { .to_string() .contains("unsupported Ticket schema migration version 99") ); - assert_eq!(load_applied_migrations(&connection).unwrap().len(), 4); + assert_eq!(load_applied_migrations(&connection).unwrap().len(), 5); } #[test] @@ -1090,7 +1118,7 @@ mod tests { connection.execute("INSERT INTO typed_ticket_events (workspace_id,ticket_id,event_index,kind,author,at,status,heading,body) VALUES ('workspace-1','ticket-1',0,'review','reviewer','2026-08-11T00:00:00Z','approve','Review','legacy evidence')",[]).unwrap(); connection.execute("INSERT INTO typed_ticket_event_attributes (workspace_id,ticket_id,event_index,key,value) VALUES ('workspace-1','ticket-1',0,'result','approve')",[]).unwrap(); connection - .execute("DELETE FROM ticket_schema_migrations WHERE version=3", []) + .execute("DELETE FROM ticket_schema_migrations WHERE version>=3", []) .unwrap(); migrate_sqlite_ticket_schema(&connection).unwrap(); let (kind,status,heading,body):(String,Option,Option,Option)=connection.query_row("SELECT kind,status,heading,body FROM typed_ticket_events WHERE workspace_id='workspace-1' AND ticket_id='ticket-1' AND event_index=0",[],|row|Ok((row.get(0)?,row.get(1)?,row.get(2)?,row.get(3)?))).unwrap(); @@ -1129,6 +1157,6 @@ mod tests { let connection = Connection::open(database).unwrap(); verify_sqlite_ticket_schema(&connection).unwrap(); - assert_eq!(load_applied_migrations(&connection).unwrap().len(), 3); + assert_eq!(load_applied_migrations(&connection).unwrap().len(), 4); } } diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 9070db63..0f36665d 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -6,9 +6,11 @@ use merge_request::{ ReviewDecision, }; use project_record::{allocate_record_id, unix_epoch_millis_now}; +use rusqlite::{params_from_iter, types::Value as SqlValue}; use ticket::{ - SqliteTicketBackend, TicketBackend, TicketEvent, TicketIdOrSlug, TicketWorkspaceActionPriority, + SqliteTicketBackend, SqliteTicketListCursor, SqliteTicketListItem, SqliteTicketListPageQuery, + TicketBackend, TicketEvent, TicketIdOrSlug, TicketWorkflowState, TicketWorkspaceActionPriority, project_ticket_workspace_item, }; @@ -17,8 +19,9 @@ use crate::records::{ ObjectiveQueryItem, ObjectiveQueryRequest, ObjectiveQueryResponse, ObjectiveResourceSummary, ObjectiveShowRequest, ObjectiveSummary, ProjectRecordList, QueryPage, TicketAssignmentSummary, TicketDetail, TicketEventDetail, TicketEvidenceEvent, TicketEvidenceSummary, - TicketMergeRequestSummary, TicketQueryItem, TicketQueryRequest, TicketQueryResponse, - TicketShowRequest, TicketSummary, summarize_body, truncate_body, validate_project_id, + TicketListPageRequest, TicketMergeRequestSummary, TicketQueryItem, TicketQueryRequest, + TicketQueryResponse, TicketShowRequest, TicketSummary, TicketSummaryPage, summarize_body, + truncate_body, validate_project_id, }; use crate::store::{ ControlPlaneStore, MemoryDocumentRecord, MemoryStagingRecord, MemoryStagingResolutionRecord, @@ -43,6 +46,7 @@ impl WorkspaceAuthority for T where T: ObjectiveAuthority + TicketAuthority + pub trait TicketAuthority { fn list_tickets(&self, limit: usize) -> Result>; + fn list_ticket_page(&self, request: TicketListPageRequest) -> Result; fn query_tickets(&self, query: TicketQueryRequest) -> Result; fn ticket(&self, id: &str) -> Result; fn show_ticket(&self, id: &str, query: TicketShowRequest) -> Result; @@ -317,11 +321,318 @@ impl SqliteWorkspaceAuthority { }) } + fn query_ticket_candidate_ids( + &self, + query: &TicketQueryRequest, + sort: TicketQuerySort, + after: Option<&(String, String)>, + limit: usize, + ) -> Result> { + let mut values = vec![SqlValue::Text(self.workspace_id.clone())]; + let mut predicates = vec!["t.workspace_id=?1".to_string()]; + let mut bind = |value: SqlValue| { + values.push(value); + format!("?{}", values.len()) + }; + if !query.states.is_empty() { + let states = query + .states + .iter() + .map(|state| bind(SqlValue::Text(state.clone()))) + .collect::>(); + predicates.push(format!("t.workflow_state IN ({})", states.join(","))); + } + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + predicates.push(format!( + "(lower(t.title) LIKE {p} OR lower(t.body) LIKE {p} OR EXISTS ( + SELECT 1 FROM typed_ticket_events e + WHERE e.workspace_id=t.workspace_id AND e.ticket_id=t.ticket_id + AND lower(e.body) LIKE {p}))", + p = pattern + )); + } + if let Some(value) = &query.updated_after { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("COALESCE(t.updated_at,'')>={value}")); + } + if let Some(value) = &query.updated_before { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("COALESCE(t.updated_at,'')<={value}")); + } + if let Some(value) = &query.linked_objective_id { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("EXISTS (SELECT 1 FROM objective_ticket_links link WHERE link.workspace_id=t.workspace_id AND link.ticket_id=t.ticket_id AND link.objective_id={value})")); + } + if query.related_ticket_id.is_some() || query.relation_kind.is_some() { + let related = query + .related_ticket_id + .as_ref() + .map(|value| bind(SqlValue::Text(value.clone()))); + let kind = query + .relation_kind + .as_ref() + .map(|value| bind(SqlValue::Text(value.clone()))); + let related = related.map(|value| format!("AND ((r.ticket_id=t.ticket_id AND r.target={value}) OR (r.target=t.ticket_id AND r.ticket_id={value}))")).unwrap_or_default(); + let kind = kind + .map(|value| format!("AND r.kind={value}")) + .unwrap_or_default(); + predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_relations r WHERE r.workspace_id=t.workspace_id {related} {kind})")); + } + let blocker = "EXISTS (SELECT 1 FROM typed_ticket_relations relation + JOIN typed_tickets blocker ON blocker.workspace_id=relation.workspace_id + AND blocker.ticket_id=CASE WHEN relation.ticket_id=t.ticket_id THEN relation.target ELSE relation.ticket_id END + WHERE relation.workspace_id=t.workspace_id + AND ((relation.ticket_id=t.ticket_id AND relation.kind='depends_on') + OR (relation.target=t.ticket_id AND relation.kind='blocks')) + AND blocker.workflow_state NOT IN ('done','closed'))"; + let active_blocker = "EXISTS (SELECT 1 FROM typed_ticket_relations relation + JOIN typed_tickets blocker ON blocker.workspace_id=relation.workspace_id + AND blocker.ticket_id=CASE WHEN relation.ticket_id=t.ticket_id THEN relation.target ELSE relation.ticket_id END + WHERE relation.workspace_id=t.workspace_id + AND ((relation.ticket_id=t.ticket_id AND relation.kind='depends_on') + OR (relation.target=t.ticket_id AND relation.kind='blocks')) + AND blocker.workflow_state NOT IN ('queued','inprogress','done','closed'))"; + let report_index = "(SELECT max(event.event_index) FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind='implementation_report')"; + let edit_index = "(SELECT max(event.event_index) FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind='item_edit')"; + let current_report = format!( + "({report_index} IS NOT NULL AND ({edit_index} IS NULL OR {report_index}>={edit_index}))" + ); + let merge_request_id = "(SELECT relation.merge_request_id FROM merge_request_ticket_relations relation + JOIN merge_requests request ON request.workspace_id=relation.workspace_id AND request.merge_request_id=relation.merge_request_id + WHERE relation.workspace_id=t.workspace_id AND relation.ticket_id=t.ticket_id + ORDER BY CASE WHEN request.state='open' THEN 0 ELSE 1 END, request.created_at DESC LIMIT 1)"; + let review_decision = format!("(SELECT json_extract(event.payload_json,'$.decision') FROM merge_request_events event + WHERE event.workspace_id=t.workspace_id AND event.merge_request_id={merge_request_id} AND event.kind='review' + AND json_extract(event.payload_json,'$.subject_ref')=(SELECT selector_from FROM merge_requests request WHERE request.workspace_id=t.workspace_id AND request.merge_request_id={merge_request_id}) + AND NOT EXISTS (SELECT 1 FROM merge_request_events revoked WHERE revoked.workspace_id=event.workspace_id AND revoked.merge_request_id=event.merge_request_id AND revoked.kind='review_revoked' AND json_extract(revoked.payload_json,'$.review_event_id')=event.event_id) + ORDER BY event.sequence DESC LIMIT 1)"); + let review_status = format!( + "CASE WHEN {merge_request_id} IS NULL THEN 'none' WHEN {review_decision}='approve' THEN 'approved' WHEN {review_decision}='request_changes' THEN 'request_changes' ELSE 'pending' END" + ); + let has_commit = format!( + "({merge_request_id} IS NOT NULL OR EXISTS (SELECT 1 FROM typed_ticket_event_references reference WHERE reference.workspace_id=t.workspace_id AND reference.ticket_id=t.ticket_id AND reference.kind='commit'))" + ); + for event_kind in &query.event_kinds { + let event_kind = bind(SqlValue::Text(event_kind.clone())); + predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind={event_kind})")); + } + for evidence in &query.evidence { + predicates.push(match evidence.as_str() { + "merge_request" => format!("{merge_request_id} IS NOT NULL"), + "commit" => has_commit.clone(), + "approved_review" => format!("{review_status}='approved'"), + other => { + return Err(Error::InvalidRecordId(format!( + "unsupported evidence filter `{other}`" + ))); + } + }); + } + if let Some(status) = &query.review_status { + let status = if status == "unresolved_changes" { + "request_changes" + } else { + status.as_str() + }; + let status = bind(SqlValue::Text(status.to_string())); + predicates.push(format!("{review_status}={status}")); + } + for attention in &query.attention { + predicates.push(match attention.as_str() { + "done_not_closed" => "t.workflow_state='done'".to_string(), + "unresolved_review" | "unresolved_changes" => format!("{review_status}='request_changes'"), + "missing_commit" => format!("NOT {has_commit}"), + "blocked" => blocker.to_string(), + "unblocked" => format!("NOT {blocker}"), + "ready" => format!("t.workflow_state='ready' AND NOT {active_blocker}"), + "awaiting_review" => format!("t.workflow_state='inprogress' AND {current_report} AND {review_status}='pending'"), + "stale_after_rescope" => format!("{report_index} IS NOT NULL AND {edit_index}>{report_index}"), + "missing_evidence" => format!("NOT ({current_report} AND {has_commit} AND {review_status}='approved')"), + other => return Err(Error::InvalidRecordId(format!("unsupported attention filter `{other}`"))), + }); + } + let rank_expression = match sort { + TicketQuerySort::Priority => format!( + "CASE WHEN t.workflow_state='ready' AND NOT {active_blocker} THEN 0 WHEN t.workflow_state IN ('queued','inprogress') THEN 1 ELSE 2 END" + ), + TicketQuerySort::Relevance => { + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + format!( + "CASE WHEN lower(t.title) LIKE {pattern} THEN 0 WHEN lower(t.body) LIKE {pattern} THEN 1 WHEN EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND lower(event.body) LIKE {pattern}) THEN 2 ELSE 3 END" + ) + } else { + "3".to_string() + } + } + _ => "0".to_string(), + }; + if let Some((key, id)) = after { + match sort { + TicketQuerySort::UpdatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("(COALESCE(t.updated_at,'')<{key} OR (COALESCE(t.updated_at,'')={key} AND t.ticket_id>{id}))")); + } + TicketQuerySort::CreatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("(COALESCE(t.created_at,'')<{key} OR (COALESCE(t.created_at,'')={key} AND t.ticket_id>{id}))")); + } + TicketQuerySort::Title => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(lower(t.title)>{key} OR (lower(t.title)={key} AND t.ticket_id>{id}))" + )); + } + TicketQuerySort::Priority | TicketQuerySort::Relevance => { + let (rank, updated_at) = key.split_once('|').unwrap_or(("9", "")); + let rank = bind(SqlValue::Integer(rank.parse::().unwrap_or(9))); + let updated_at = bind(SqlValue::Text(updated_at.to_string())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("({rank_expression}>{rank} OR ({rank_expression}={rank} AND (COALESCE(t.updated_at,'')<{updated_at} OR (COALESCE(t.updated_at,'')={updated_at} AND t.ticket_id>{id}))))")); + } + } + } + let order = match sort { + TicketQuerySort::Title => "t.title COLLATE NOCASE ASC, t.ticket_id ASC".to_string(), + TicketQuerySort::CreatedDesc => "t.created_at DESC, t.ticket_id ASC".to_string(), + TicketQuerySort::UpdatedDesc => "t.updated_at DESC, t.ticket_id ASC".to_string(), + TicketQuerySort::Priority | TicketQuerySort::Relevance => { + format!("{rank_expression} ASC, t.updated_at DESC, t.ticket_id ASC") + } + }; + let limit = bind(SqlValue::Integer(i64::try_from(limit).unwrap_or(i64::MAX))); + let sql = format!( + "SELECT t.ticket_id FROM typed_tickets t WHERE {} ORDER BY {order} LIMIT {limit}", + predicates.join(" AND ") + ); + self.store.with_conn(|connection| { + let mut statement = connection.prepare(&sql)?; + let rows = statement.query_map(params_from_iter(values.iter()), |row| row.get(0))?; + Ok(rows.collect::, _>>()?) + }) + } + + fn query_objective_candidate_ids( + &self, + query: &ObjectiveQueryRequest, + sort: ObjectiveQuerySort, + after: Option<&(String, String)>, + limit: usize, + ) -> Result> { + let mut values = vec![SqlValue::Text(self.workspace_id.clone())]; + let mut predicates = vec!["o.workspace_id=?1".to_string()]; + let mut bind = |value: SqlValue| { + values.push(value); + format!("?{}", values.len()) + }; + if !query.states.is_empty() { + let states = query + .states + .iter() + .map(|state| bind(SqlValue::Text(state.clone()))) + .collect::>(); + predicates.push(format!("o.state IN ({})", states.join(","))); + } + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + predicates.push(format!( + "(lower(o.title) LIKE {pattern} OR lower(o.body_md) LIKE {pattern})" + )); + } + if let Some(value) = &query.updated_after { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("o.updated_at>={value}")); + } + if let Some(value) = &query.updated_before { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("o.updated_at<={value}")); + } + if let Some(value) = &query.linked_ticket_id { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("EXISTS (SELECT 1 FROM objective_ticket_links link WHERE link.workspace_id=o.workspace_id AND link.objective_id=o.objective_id AND link.ticket_id={value})")); + } + let relevance_rank = if let Some(text) = + query.query.as_deref().filter(|text| !text.is_empty()) + { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + format!( + "CASE WHEN lower(o.title) LIKE {pattern} THEN 0 WHEN lower(o.body_md) LIKE {pattern} THEN 1 ELSE 2 END" + ) + } else { + "2".to_string() + }; + if let Some((key, id)) = after { + match sort { + ObjectiveQuerySort::UpdatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(o.updated_at<{key} OR (o.updated_at={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::CreatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(o.created_at<{key} OR (o.created_at={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::Title => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(lower(o.title)>{key} OR (lower(o.title)={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::Relevance => { + let (rank, updated_at) = key.split_once('|').unwrap_or(("9", "")); + let rank = bind(SqlValue::Integer(rank.parse::().unwrap_or(9))); + let updated_at = bind(SqlValue::Text(updated_at.to_string())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("({relevance_rank}>{rank} OR ({relevance_rank}={rank} AND (o.updated_at<{updated_at} OR (o.updated_at={updated_at} AND o.objective_id>{id}))))")); + } + } + } + let order = match sort { + ObjectiveQuerySort::Title => { + "o.title COLLATE NOCASE ASC, o.objective_id ASC".to_string() + } + ObjectiveQuerySort::CreatedDesc => "o.created_at DESC, o.objective_id ASC".to_string(), + ObjectiveQuerySort::UpdatedDesc => "o.updated_at DESC, o.objective_id ASC".to_string(), + ObjectiveQuerySort::Relevance => { + format!("{relevance_rank} ASC, o.updated_at DESC, o.objective_id ASC") + } + }; + let limit = bind(SqlValue::Integer(i64::try_from(limit).unwrap_or(i64::MAX))); + let sql = format!( + "SELECT o.objective_id FROM objectives o WHERE {} ORDER BY {order} LIMIT {limit}", + predicates.join(" AND ") + ); + self.store.with_conn(|connection| { + let mut statement = connection.prepare(&sql)?; + let rows = statement.query_map(params_from_iter(values.iter()), |row| row.get(0))?; + Ok(rows.collect::, _>>()?) + }) + } + fn read_ticket_detail(&self, id: &str, request: TicketShowRequest) -> Result { validate_project_id(id)?; let ticket = self .ticket_backend .show(TicketIdOrSlug::Id(id.to_string()))?; + self.ticket_detail_from_ticket(ticket, request) + } + + fn ticket_detail_from_ticket( + &self, + ticket: ticket::Ticket, + request: TicketShowRequest, + ) -> Result { + let id = ticket.meta.id.as_str(); let (body, body_truncated) = truncate_body(ticket.document.body.as_str(), DETAIL_BODY_LIMIT); let event_limit = request @@ -458,42 +769,97 @@ impl TicketAuthority for SqliteWorkspaceAuthority { }) } + fn list_ticket_page(&self, request: TicketListPageRequest) -> Result { + let limit = request.limit.unwrap_or(30).clamp(1, 100); + let mut states = request.states; + states.sort(); + states.dedup(); + let parsed_states = states + .iter() + .map(|state| { + TicketWorkflowState::parse(state).ok_or_else(|| { + Error::InvalidRecordId(format!("unsupported ticket state `{state}`")) + }) + }) + .collect::>>()?; + let fingerprint = format!("ticket-summary:v1:states={}", states.join(",")); + let after = request + .cursor + .as_deref() + .map(|cursor| parse_ticket_summary_cursor(cursor, &fingerprint)) + .transpose()?; + let page = + self.ticket_backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: parsed_states, + limit, + after, + })?; + let items = page + .items + .into_iter() + .map(ticket_summary_from_sqlite_item) + .collect::>(); + let next_cursor = page + .next + .map(|position| make_ticket_summary_cursor(&fingerprint, position)); + Ok(TicketSummaryPage { + page: QueryPage { + limit, + returned: items.len(), + has_more: page.has_more, + next_cursor, + sort: "updated_desc".to_string(), + source_limit: None, + source_truncated: false, + }, + items, + invalid_records: Vec::new(), + record_authority: RECORD_SOURCE_WORKSPACE_SQLITE.to_string(), + }) + } + fn query_tickets(&self, query: TicketQueryRequest) -> Result { validate_ticket_query(&query)?; let limit = query.limit.unwrap_or(50).clamp(1, 100); let sort = normalize_ticket_sort(query.sort.as_deref(), query.query.is_some())?; + let fingerprint = ticket_query_fingerprint(&query, sort); let cursor = query .cursor .as_deref() - .map(parse_query_cursor) + .map(|cursor| parse_bound_query_cursor(cursor, &fingerprint)) .transpose()?; - let mut summaries = self.list_tickets(1_001)?.items; - let source_truncated = summaries.len() > 1_000; - summaries.truncate(1_000); + let candidate_limit = limit.saturating_add(1); + let candidate_ids = + self.query_ticket_candidate_ids(&query, sort, cursor.as_ref(), candidate_limit)?; + let source_truncated = candidate_ids.len() == candidate_limit; let mut items = Vec::new(); - for summary in summaries { - let detail = self.read_ticket_detail( - &summary.id, + for ticket_id in candidate_ids { + let authoritative = self + .ticket_backend + .show(TicketIdOrSlug::Id(ticket_id.clone()))?; + let summary = ticket_summary_from_ticket(&authoritative); + let authoritative_body = authoritative.document.body.clone(); + let authoritative_events = authoritative.events.clone(); + let detail = self.ticket_detail_from_ticket( + authoritative, TicketShowRequest { event_limit: Some(TICKET_EVENT_LIMIT), event_cursor: None, }, )?; - let authoritative = self - .ticket_backend - .show(TicketIdOrSlug::Id(summary.id.clone()))?; if ticket_matches_query( &summary, &detail, - authoritative.document.body.as_str(), - &authoritative.events, + authoritative_body.as_str(), + &authoritative_events, &query, ) { items.push(ticket_query_item( summary, &detail, - authoritative.document.body.as_str(), - &authoritative.events, + authoritative_body.as_str(), + &authoritative_events, &query, )); } @@ -505,7 +871,11 @@ impl TicketAuthority for SqliteWorkspaceAuthority { let has_more = items.len() > limit; items.truncate(limit); let next_cursor = has_more - .then(|| items.last().map(|item| make_ticket_cursor(item, sort))) + .then(|| { + items + .last() + .map(|item| make_ticket_cursor(item, sort, &fingerprint)) + }) .flatten(); Ok(TicketQueryResponse { page: QueryPage { @@ -514,7 +884,7 @@ impl TicketAuthority for SqliteWorkspaceAuthority { has_more, next_cursor, sort: sort.to_string(), - source_limit: Some(1_000), + source_limit: Some(candidate_limit), source_truncated, }, items, @@ -566,33 +936,36 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { )?; let limit = query.limit.unwrap_or(50).clamp(1, 100); let sort = normalize_objective_sort(query.sort.as_deref(), query.query.is_some())?; + let fingerprint = objective_query_fingerprint(&query, sort); let cursor = query .cursor .as_deref() - .map(parse_query_cursor) + .map(|cursor| parse_bound_query_cursor(cursor, &fingerprint)) .transpose()?; - let mut objectives = self.list_objectives(1_001)?.items; - let source_truncated = objectives.len() > 1_000; - objectives.truncate(1_000); + let candidate_limit = limit.saturating_add(1); + let objective_ids = + self.query_objective_candidate_ids(&query, sort, cursor.as_ref(), candidate_limit)?; + let source_truncated = objective_ids.len() == candidate_limit; let mut items = Vec::new(); - for objective in objectives { - let body_md = self.objective_record(&objective.id)?.body_md; - if !objective_matches_query(&objective, &body_md, &query) { - continue; - } + for objective_id in objective_ids { + let record = self.objective_record(&objective_id)?; let linked_tickets = self .store - .list_objective_ticket_links(&self.workspace_id, &objective.id)? + .list_objective_ticket_links(&self.workspace_id, &objective_id)? .into_iter() .map(|link| link.ticket_id) .collect::>(); - if query - .linked_ticket_id - .as_ref() - .is_some_and(|id| !linked_tickets.iter().any(|ticket_id| ticket_id == id)) - { - continue; - } + let body_md = record.body_md.clone(); + let objective = ObjectiveSummary { + id: record.objective_id, + title: record.title, + state: record.state, + created_at: Some(record.created_at), + updated_at: Some(record.updated_at), + summary: summarize_body(&body_md), + linked_tickets: linked_tickets.clone(), + record_source: RECORD_SOURCE_WORKSPACE_SQLITE.to_string(), + }; items.push(objective_query_item( objective, linked_tickets, @@ -607,7 +980,11 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { let has_more = items.len() > limit; items.truncate(limit); let next_cursor = has_more - .then(|| items.last().map(|item| make_objective_cursor(item, sort))) + .then(|| { + items + .last() + .map(|item| make_objective_cursor(item, sort, &fingerprint)) + }) .flatten(); Ok(ObjectiveQueryResponse { page: QueryPage { @@ -616,7 +993,7 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { has_more, next_cursor, sort: sort.to_string(), - source_limit: Some(1_000), + source_limit: Some(candidate_limit), source_truncated, }, items, @@ -1240,6 +1617,35 @@ fn parse_offset_cursor(cursor: &str, field: &str) -> Result { .map_err(|_| Error::InvalidRecordId(format!("invalid {field}"))) } +fn make_bound_query_cursor(fingerprint: &str, key: &str, id: &str) -> String { + make_query_cursor(&format!("{fingerprint}\n{key}"), id) +} + +fn parse_bound_query_cursor(value: &str, fingerprint: &str) -> Result<(String, String)> { + let (key, id) = parse_query_cursor(value)?; + let prefix = format!("{fingerprint}\n"); + let key = key.strip_prefix(&prefix).ok_or_else(|| { + Error::InvalidRecordId("cursor does not match the current filters or sort".to_string()) + })?; + Ok((key.to_string(), id)) +} + +fn ticket_query_fingerprint(query: &TicketQueryRequest, sort: TicketQuerySort) -> String { + let mut query = query.clone(); + query.cursor = None; + query.limit = None; + query.sort = Some(sort.to_string()); + serde_json::to_string(&query).expect("ticket query fingerprint must serialize") +} + +fn objective_query_fingerprint(query: &ObjectiveQueryRequest, sort: ObjectiveQuerySort) -> String { + let mut query = query.clone(); + query.cursor = None; + query.limit = None; + query.sort = Some(sort.to_string()); + serde_json::to_string(&query).expect("objective query fingerprint must serialize") +} + fn make_query_cursor(key: &str, id: &str) -> String { format!("v1:{}:{key}{id}", key.len()) } @@ -1545,8 +1951,8 @@ fn sort_ticket_query_items(items: &mut [TicketQueryItem], sort: TicketQuerySort) }); } -fn make_ticket_cursor(item: &TicketQueryItem, sort: TicketQuerySort) -> String { - make_query_cursor(&ticket_sort_key(item, sort), &item.id) +fn make_ticket_cursor(item: &TicketQueryItem, sort: TicketQuerySort, fingerprint: &str) -> String { + make_bound_query_cursor(fingerprint, &ticket_sort_key(item, sort), &item.id) } fn ticket_item_after_cursor( @@ -1571,31 +1977,6 @@ fn ticket_item_after_cursor( } } -fn objective_matches_query( - objective: &ObjectiveSummary, - body_md: &str, - query: &ObjectiveQueryRequest, -) -> bool { - if !query.states.is_empty() && !query.states.iter().any(|state| state == &objective.state) { - return false; - } - if query - .updated_after - .as_ref() - .is_some_and(|after| objective.updated_at.as_deref().unwrap_or("") <= after.as_str()) - || query - .updated_before - .as_ref() - .is_some_and(|before| objective.updated_at.as_deref().unwrap_or("") >= before.as_str()) - { - return false; - } - query.query.as_ref().is_none_or(|text| { - let needle = text.to_lowercase(); - objective.title.to_lowercase().contains(&needle) || body_md.to_lowercase().contains(&needle) - }) -} - fn objective_query_item( objective: ObjectiveSummary, linked_tickets: Vec, @@ -1673,8 +2054,12 @@ fn sort_objective_query_items(items: &mut [ObjectiveQueryItem], sort: ObjectiveQ }); } -fn make_objective_cursor(item: &ObjectiveQueryItem, sort: ObjectiveQuerySort) -> String { - make_query_cursor(&objective_sort_key(item, sort), &item.id) +fn make_objective_cursor( + item: &ObjectiveQueryItem, + sort: ObjectiveQuerySort, + fingerprint: &str, +) -> String { + make_bound_query_cursor(fingerprint, &objective_sort_key(item, sort), &item.id) } fn objective_item_after_cursor( @@ -1802,6 +2187,80 @@ fn memory_resolution_from_record(record: MemoryStagingResolutionRecord) -> Memor } } +fn ticket_summary_from_ticket(ticket: &ticket::Ticket) -> TicketSummary { + let summary = ticket::TicketSummary { + id: ticket.meta.id.clone(), + slug: ticket.meta.slug.clone(), + title: ticket.meta.title.clone(), + status: ticket.meta.status.clone(), + kind: ticket.meta.kind.clone(), + priority: ticket.meta.priority.clone(), + labels: ticket.meta.labels.clone(), + readiness: ticket.meta.readiness.clone(), + workflow_state: ticket.meta.workflow_state, + workflow_state_explicit: ticket.meta.workflow_state_explicit, + queued_by: ticket.meta.queued_by.clone(), + queued_at: ticket.meta.queued_at.clone(), + updated_at: ticket.meta.updated_at.clone(), + }; + ticket_summary_from_sqlite_item(SqliteTicketListItem { + summary, + relation_blockers: ticket.relations.blockers.clone(), + }) +} + +fn ticket_summary_from_sqlite_item(item: SqliteTicketListItem) -> TicketSummary { + let projection = project_ticket_workspace_item(&item.summary, &item.relation_blockers, None); + TicketSummary { + id: item.summary.id, + title: item.summary.title, + state: item.summary.workflow_state.as_str().to_string(), + priority: item.summary.priority, + updated_at: item.summary.updated_at, + queued_by: item.summary.queued_by, + queued_at: item.summary.queued_at, + workspace_action_priority: workspace_action_priority_name(projection.priority).to_string(), + record_source: "sqlite_yoi_ticket".to_string(), + } +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +struct TicketSummaryCursorEnvelope { + version: u8, + fingerprint: String, + position: SqliteTicketListCursor, +} + +fn make_ticket_summary_cursor(fingerprint: &str, position: SqliteTicketListCursor) -> String { + make_query_cursor( + &serde_json::to_string(&TicketSummaryCursorEnvelope { + version: 1, + fingerprint: fingerprint.to_string(), + position, + }) + .expect("ticket summary cursor must serialize"), + "", + ) +} + +fn parse_ticket_summary_cursor( + value: &str, + expected_fingerprint: &str, +) -> Result { + let (encoded, trailing) = parse_query_cursor(value)?; + if !trailing.is_empty() { + return Err(Error::InvalidRecordId("cursor is malformed".to_string())); + } + let cursor: TicketSummaryCursorEnvelope = serde_json::from_str(&encoded) + .map_err(|_| Error::InvalidRecordId("cursor is malformed".to_string()))?; + if cursor.version != 1 || cursor.fingerprint != expected_fingerprint { + return Err(Error::InvalidRecordId( + "cursor does not match the current ticket filters or sort".to_string(), + )); + } + Ok(cursor.position) +} + fn workspace_action_priority_name(priority: TicketWorkspaceActionPriority) -> &'static str { match priority { TicketWorkspaceActionPriority::ReadyForQueue => "ready_for_queue", @@ -2135,6 +2594,25 @@ mod tests { .unwrap(); assert_eq!(exact_relation.items.len(), 1); assert_eq!(exact_relation.items[0].id, "00000000001J2"); + let summary_page = authority + .list_ticket_page(TicketListPageRequest { + states: vec!["planning".to_string(), "ready".to_string()], + limit: Some(1), + cursor: None, + }) + .unwrap(); + assert_eq!(summary_page.items.len(), 1); + assert!(summary_page.page.has_more); + let mismatched_summary_cursor = authority.list_ticket_page(TicketListPageRequest { + states: vec!["done".to_string()], + limit: Some(1), + cursor: summary_page.page.next_cursor, + }); + assert!(matches!( + mismatched_summary_cursor, + Err(Error::InvalidRecordId(_)) + )); + let first_page = authority .query_tickets(TicketQueryRequest { sort: Some("title".to_string()), @@ -2154,6 +2632,13 @@ mod tests { assert_eq!(second_page.items.len(), 1); assert_ne!(first_page.items[0].id, second_page.items[0].id); assert!(second_page.page.has_more); + let mismatched_cursor = authority.query_tickets(TicketQueryRequest { + sort: Some("updated_desc".to_string()), + limit: Some(1), + cursor: first_page.page.next_cursor.clone(), + ..TicketQueryRequest::default() + }); + assert!(matches!(mismatched_cursor, Err(Error::InvalidRecordId(_)))); let third_page = authority .query_tickets(TicketQueryRequest { sort: Some("title".to_string()), diff --git a/crates/workspace-server/src/records.rs b/crates/workspace-server/src/records.rs index 35348f60..62f978b7 100644 --- a/crates/workspace-server/src/records.rs +++ b/crates/workspace-server/src/records.rs @@ -12,6 +12,14 @@ pub struct ProjectRecordList { pub record_authority: String, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct TicketSummaryPage { + pub items: Vec, + pub page: QueryPage, + pub invalid_records: Vec, + pub record_authority: String, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] pub struct InvalidProjectRecord { @@ -33,12 +41,23 @@ pub struct TicketSummary { pub record_source: String, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +pub struct TicketListPageRequest { + #[serde(default)] + pub states: Vec, + #[serde(default)] + pub limit: Option, + #[serde(default)] + pub cursor: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] pub struct TicketListResponse { pub workspace_id: String, pub limit: usize, pub items: Vec, + pub page: QueryPage, pub invalid_records: Vec, pub record_authority: String, } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index c2d7654a..4f8bdedd 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -2229,6 +2229,9 @@ struct ObjectiveEditRequest { #[derive(Debug, Deserialize)] struct TicketListQuery { limit: Option, + cursor: Option, + /// Comma-separated workflow states. Repeated lane requests normally pass one state group. + states: Option, } #[derive(Debug, Deserialize)] @@ -8316,17 +8319,35 @@ async fn list_tickets( State(api): State, Query(query): Query, ) -> ApiResult> { - let requested_limit = query.limit.unwrap_or(api.config.max_records); - let limit = requested_limit.min(1000); - let ProjectRecordList { + let limit = query.limit.unwrap_or(30).clamp(1, 100); + let states = query + .states + .as_deref() + .map(|states| { + states + .split(',') + .filter(|state| !state.is_empty()) + .map(str::to_string) + .collect::>() + }) + .unwrap_or_default(); + let crate::records::TicketSummaryPage { items, + page, invalid_records, record_authority, - } = api.authority.list_tickets(limit)?; + } = api + .authority + .list_ticket_page(crate::records::TicketListPageRequest { + states, + limit: Some(limit), + cursor: query.cursor, + })?; Ok(Json(crate::records::TicketListResponse { workspace_id: api.config.workspace_id, limit, items, + page, invalid_records, record_authority, })) diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 06e18e5e..f1d59410 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -196,6 +196,11 @@ const MIGRATIONS: &[Migration] = &[ name: "remove Worker control delegation authority", apply: remove_worker_control_delegation_authority, }, + Migration { + version: 36, + name: "add Objective query indexes", + apply: add_objective_query_indexes, + }, ]; struct Migration { @@ -5046,6 +5051,24 @@ fn create_worker_control_delegation_operation_authority(conn: &Connection) -> Re Ok(()) } +fn add_objective_query_indexes(conn: &Connection) -> Result<()> { + conn.execute_batch( + r#" + CREATE INDEX IF NOT EXISTS objectives_workspace_state_updated + ON objectives(workspace_id, state, updated_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_updated + ON objectives(workspace_id, updated_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_created + ON objectives(workspace_id, created_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_title + ON objectives(workspace_id, title COLLATE NOCASE, objective_id); + CREATE INDEX IF NOT EXISTS objective_ticket_links_workspace_ticket_objective + ON objective_ticket_links(workspace_id, ticket_id, objective_id); + "#, + )?; + Ok(()) +} + fn remove_worker_control_delegation_authority(conn: &Connection) -> Result<()> { let mut statement = conn.prepare("SELECT workspace_id, grant_id, permissions_json FROM worker_control_grants")?; @@ -5799,7 +5822,7 @@ INSERT INTO worker_control_grants ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(!table_exists(&conn, "worker_control_delegation_operations").unwrap()); let (permissions_json, revoked_at): (String, Option) = conn .query_row( @@ -5847,7 +5870,7 @@ INSERT INTO worker_control_grants ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(table_exists(&conn, "worker_workdir_attachment_reservations").unwrap()); } @@ -5880,7 +5903,7 @@ CREATE TABLE flow_events (event_id TEXT PRIMARY KEY); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(table_exists(&conn, "flow_sources").unwrap()); assert!(table_exists(&conn, "flow_source_revisions").unwrap()); assert!(!table_exists(&conn, "flow_instances").unwrap()); @@ -5947,7 +5970,7 @@ INSERT INTO worker_workdir_attachment_reservations ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); let repositories_sql: String = conn .query_row( "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'repositories'", @@ -6127,7 +6150,7 @@ INSERT INTO workdir_registry ( let db = dir.path().join("control-plane.sqlite"); let store = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); assert!( !store .with_conn(|conn| table_exists(conn, "worker_workspace_credentials")) @@ -6144,7 +6167,7 @@ INSERT INTO workdir_registry ( store.upsert_workspace(&record).await.unwrap(); let reopened = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(reopened.schema_version().await.unwrap(), 35); + assert_eq!(reopened.schema_version().await.unwrap(), 36); assert_eq!( reopened.get_workspace("local-dev").await.unwrap(), Some(record) @@ -6691,7 +6714,7 @@ INSERT INTO workdir_registry ( .unwrap(); let store = SqliteWorkspaceStore::from_connection(conn).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); store .with_conn(|conn| { @@ -6880,7 +6903,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn repository_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: None, @@ -6946,7 +6969,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn memory_authority_records_round_trip_and_close_staging() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: None, @@ -7337,7 +7360,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn account_and_login_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let now = "2026-07-22T00:00:00Z".to_string(); let account = AccountRecord { account_id: "acct-user-alice".to_string(), diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index a24976be..a6815a42 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -19,6 +19,7 @@ export type TicketListResponse = { workspace_id: string; limit: number; items: Array; + page: QueryPage; invalid_records: Array; record_authority: string; }; From 6ca5dfbe11e933db10f8460b0f83432b8f5e6da6 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 03:08:39 +0900 Subject: [PATCH 06/13] feat: paginate Ticket board lanes independently --- .../src/lib/workspace/styles/tickets.css | 24 ++++ .../workspace/tickets/ticket-panel.test.ts | 31 ++++- .../w/[workspaceId]/tickets/+page.svelte | 116 +++++++++++++++--- .../routes/w/[workspaceId]/tickets/+page.ts | 53 ++++++-- 4 files changed, 195 insertions(+), 29 deletions(-) diff --git a/web/workspace/src/lib/workspace/styles/tickets.css b/web/workspace/src/lib/workspace/styles/tickets.css index c7098b5f..0fc65d2e 100644 --- a/web/workspace/src/lib/workspace/styles/tickets.css +++ b/web/workspace/src/lib/workspace/styles/tickets.css @@ -169,8 +169,32 @@ display: grid; align-content: start; gap: 0.55rem; + max-height: min(68vh, 48rem); + overflow-y: auto; + overscroll-behavior: contain; padding: 0.6rem; } + .ticket-lane-page-state { + display: flex; + justify-content: center; + gap: 0.5rem; + margin: 0; + padding: 0.45rem; + color: var(--text-muted); + font-size: 0.72rem; + text-align: center; + } + .ticket-lane-page-error { + align-items: center; + color: var(--danger); + } + .ticket-lane-page-error button { + border: 1px solid var(--line); + border-radius: 0.35rem; + background: var(--bg-raised); + color: inherit; + padding: 0.2rem 0.45rem; + } .ticket-card { display: grid; gap: 0.55rem; diff --git a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts index 842103cf..649eb81d 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts @@ -105,6 +105,32 @@ Deno.test("ticket worker launch uses the common Worker route and bounded Ticket ); }); +Deno.test("ticket board paginates every lane independently with bounded summary requests", async () => { + const loadSource = await Deno.readTextFile( + "src/routes/w/[workspaceId]/tickets/+page.ts", + ); + const pageSource = await Deno.readTextFile( + "src/routes/w/[workspaceId]/tickets/+page.svelte", + ); + + assertIncludes(loadSource, 'limit: "30"'); + assertIncludes(loadSource, "states: states.join"); + assertIncludes(pageSource, "lane.page.next_cursor"); + assertIncludes(pageSource, "lane.loading"); + assertIncludes(pageSource, "mergeTickets"); + assertIncludes(pageSource, "onscroll"); + assertIncludes(pageSource, "Retry"); + if (loadSource.includes("limit=1000") || pageSource.includes("limit=1000")) { + throw new Error("Ticket board must not fetch the legacy 1000-item list"); + } + if ( + loadSource.includes("/tickets/query") || + pageSource.includes("/tickets/query") + ) { + throw new Error("Ticket board must use the bounded summary endpoint"); + } +}); + Deno.test("ticket panel starts the Orchestrator explicitly and gates orchestration actions", async () => { const panelSource = await Deno.readTextFile( "src/routes/w/[workspaceId]/tickets/+page.svelte", @@ -113,7 +139,10 @@ Deno.test("ticket panel starts the Orchestrator explicitly and gates orchestrati "src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte", ); - assertIncludes(panelSource, 'workspaceApiPath(data.workspaceId, "/orchestrator")'); + assertIncludes( + panelSource, + 'workspaceApiPath(data.workspaceId, "/orchestrator")', + ); assertIncludes(panelSource, '{ method: "POST" }'); assertIncludes(panelSource, "Start Orchestrator"); assertIncludes(panelSource, "orchestrator.data?.online"); diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte index f4c55b04..ff68779f 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte @@ -2,30 +2,94 @@ import { untrack } from "svelte"; import type { ApiResult } from "$lib/workspace/api/http"; import { loadJson, workspaceApiPath } from "$lib/workspace/api/http"; + import type { + QueryPage, + TicketListResponse, + TicketSummary, + } from "$lib/generated/ticket-api"; import { ticketLanes, type WorkspaceOrchestratorStatus, } from "$lib/workspace/tickets/ticket-panel"; - import type { - TicketListResponse, - TicketSummary, - } from "$lib/workspace/sidebar/types"; + import type { PageData } from "./$types"; - const { data } = $props<{ - data: { - workspaceId: string; - tickets: ApiResult; - orchestrator: ApiResult; - }; - }>(); + type LaneState = { + states: string[]; + tickets: TicketSummary[]; + page: QueryPage; + loading: boolean; + error: string | null; + }; - const initialTickets = untrack(() => data.tickets.data?.items ?? []); - let tickets = $state(initialTickets); + let { data }: { data: PageData } = $props(); + // svelte-ignore state_referenced_locally + let laneState = $state>( + Object.fromEntries( + Object.entries(data.ticketLanes).map(([laneId, lane]) => [ + laneId, + { + states: [...lane.states], + tickets: lane.response.items, + page: lane.response.page, + loading: false, + error: null, + }, + ]), + ), + ); let orchestrator = $state>( untrack(() => data.orchestrator), ); let orchestratorStarting = $state(false); - let lanes = $derived(ticketLanes(tickets)); + const tickets = $derived( + Object.values(laneState).flatMap((lane) => lane.tickets), + ); + const lanes = $derived(ticketLanes(tickets)); + + function mergeTickets( + current: TicketSummary[], + incoming: TicketSummary[], + ): TicketSummary[] { + const byId = new Map(current.map((ticket) => [ticket.id, ticket])); + for (const ticket of incoming) byId.set(ticket.id, ticket); + return [...byId.values()]; + } + + async function loadMore(laneId: string): Promise { + const lane = laneState[laneId]; + if (!lane || lane.loading || !lane.page.has_more || !lane.page.next_cursor) { + return; + } + lane.loading = true; + lane.error = null; + try { + const search = new URLSearchParams({ + limit: "30", + states: lane.states.join(","), + cursor: lane.page.next_cursor, + }); + const response = await fetch( + `/api/w/${encodeURIComponent(data.workspaceId)}/tickets?${search}`, + ); + if (!response.ok) { + throw new Error(`追加読み込みに失敗しました (${response.status})`); + } + const page = (await response.json()) as TicketListResponse; + lane.tickets = mergeTickets(lane.tickets, page.items); + lane.page = page.page; + } catch (error) { + lane.error = error instanceof Error ? error.message : String(error); + } finally { + lane.loading = false; + } + } + + function handleLaneScroll(event: Event, laneId: string): void { + const container = event.currentTarget as HTMLElement; + const remaining = + container.scrollHeight - container.scrollTop - container.clientHeight; + if (remaining <= 96) void loadMore(laneId); + } async function startOrchestrator() { if (orchestratorStarting || orchestrator.data?.online) return; @@ -45,7 +109,9 @@ } -Tickets · Yoi + + Tickets · {data.workspaceId} +
@@ -76,7 +142,7 @@
{tickets.length} - tickets + loaded tickets
@@ -93,6 +159,7 @@
{#each lanes as lane (lane.id)} + {@const pagination = laneState[lane.id]}
@@ -101,8 +168,11 @@
{lane.tickets.length}
- -
+
handleLaneScroll(event, lane.id)} + > {#each lane.tickets as ticket (ticket.id)} No tickets
{/each} + {#if pagination?.loading} +

Loading…

+ {:else if pagination?.error} + + {:else if pagination && !pagination.page.has_more && lane.tickets.length > 0} +

End of lane

+ {/if}
{/each} diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts index e115cab7..04ab28e8 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts @@ -1,23 +1,56 @@ import { loadJson, workspaceApiPath } from "$lib/workspace/api/http"; +import type { TicketListResponse } from "$lib/generated/ticket-api"; import type { WorkspaceOrchestratorStatus } from "$lib/workspace/tickets/ticket-panel"; -import type { TicketListResponse } from "$lib/workspace/sidebar/types"; import type { PageLoad } from "./$types"; -export const load = (async ({ fetch, params }) => { - const [tickets, orchestrator] = await Promise.all([ - loadJson( - fetch, - `${workspaceApiPath(params.workspaceId, "/tickets")}?limit=1000`, +const LANE_STATES = { + "ready-planning": ["ready", "planning"], + "inprogress-queued": ["inprogress", "queued"], + "done-closed": ["done", "closed"], +} as const; + +export type TicketLaneId = keyof typeof LANE_STATES; + +export type TicketLanePage = { + states: readonly string[]; + response: TicketListResponse; +}; + +export const load: PageLoad = async ({ fetch, params }) => { + const workspaceId = params.workspaceId; + const [entries, orchestrator] = await Promise.all([ + Promise.all( + Object.entries(LANE_STATES).map(async ([laneId, states]) => { + const search = new URLSearchParams({ + limit: "30", + states: states.join(","), + }); + const response = await fetch( + `/api/w/${encodeURIComponent(workspaceId)}/tickets?${search}`, + ); + if (!response.ok) { + throw new Error( + `failed to load ${laneId} Ticket lane (${response.status})`, + ); + } + return [ + laneId, + { states: [...states], response: await response.json() }, + ] as const; + }), ), loadJson( fetch, - workspaceApiPath(params.workspaceId, "/orchestrator"), + workspaceApiPath(workspaceId, "/orchestrator"), ), ]); return { - workspaceId: params.workspaceId, - tickets, + workspaceId, + ticketLanes: Object.fromEntries(entries) as unknown as Record< + TicketLaneId, + TicketLanePage + >, orchestrator, }; -}) satisfies PageLoad; +}; From a6f3e30652bade8dc9ba9a195b946e97ca4d8b73 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 07:50:42 +0900 Subject: [PATCH 07/13] fix: preserve Ticket lane order across pages --- crates/ticket/src/lib.rs | 54 +++++++++++++++++++++--- crates/workspace-server/src/authority.rs | 7 ++- 2 files changed, 54 insertions(+), 7 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 909ae183..ccd016e5 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -1426,6 +1426,7 @@ pub struct SqliteTicketListProjection { #[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] pub struct SqliteTicketListCursor { + pub state_rank: i64, pub updated_at: Option, pub ticket_id: String, } @@ -2412,6 +2413,7 @@ impl SqliteTicketBackend { .collect::>(), ) .map_err(|error| TicketError::Sqlite(error.to_string()))?; + let cursor_rank = query.after.as_ref().map(|cursor| cursor.state_rank); let cursor_updated_at = query .after .as_ref() @@ -2431,11 +2433,24 @@ impl SqliteTicketBackend { SELECT 1 FROM json_each(?2) AS state WHERE state.value = ticket.workflow_state )) - AND (?3 IS NULL OR COALESCE(ticket.updated_at, '') < COALESCE(?4, '') - OR (COALESCE(ticket.updated_at, '') = COALESCE(?4, '') - AND ticket.ticket_id > ?3)) - ORDER BY ticket.updated_at DESC, ticket.ticket_id ASC - LIMIT ?5", + AND (?3 IS NULL OR + CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END > ?4 + OR (CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END = ?4 + AND (COALESCE(ticket.updated_at, '') < COALESCE(?5, '') + OR (COALESCE(ticket.updated_at, '') = COALESCE(?5, '') + AND ticket.ticket_id > ?3)))) + ORDER BY CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END ASC, + ticket.updated_at DESC, ticket.ticket_id ASC + LIMIT ?6", ) .map_err(sqlite_err)?; let rows = statement @@ -2444,6 +2459,7 @@ impl SqliteTicketBackend { self.workspace_id, states, cursor_id, + cursor_rank, cursor_updated_at, i64::try_from(fetch_limit).unwrap_or(i64::MAX) ], @@ -2458,6 +2474,14 @@ impl SqliteTicketBackend { let next = has_more.then(|| { let summary = summaries.last().expect("non-empty page with continuation"); SqliteTicketListCursor { + state_rank: match summary.workflow_state { + TicketWorkflowState::Ready => 0, + TicketWorkflowState::Planning => 1, + TicketWorkflowState::InProgress => 2, + TicketWorkflowState::Queued => 3, + TicketWorkflowState::Done => 4, + TicketWorkflowState::Closed => 5, + }, updated_at: summary.updated_at.clone(), ticket_id: summary.id.clone(), } @@ -6896,6 +6920,26 @@ state: planning ids.push(ticket.id); } + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at='2026-08-12T05:00:00Z' WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", ids[2]], + ) + .unwrap(); + let combined_lane = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready, TicketWorkflowState::Planning], + limit: 1, + after: None, + }) + .unwrap(); + assert_eq!( + combined_lane.items[0].summary.workflow_state, + TicketWorkflowState::Ready, + "lane pagination order must match the UI's state-primary order", + ); + let first = backend .list_workspace_projection_page(SqliteTicketListPageQuery { states: vec![TicketWorkflowState::Ready], diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 0f36665d..16c92999 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -782,7 +782,10 @@ impl TicketAuthority for SqliteWorkspaceAuthority { }) }) .collect::>>()?; - let fingerprint = format!("ticket-summary:v1:states={}", states.join(",")); + let fingerprint = format!( + "ticket-summary:v2:sort=priority:states={}", + states.join(",") + ); let after = request .cursor .as_deref() @@ -809,7 +812,7 @@ impl TicketAuthority for SqliteWorkspaceAuthority { returned: items.len(), has_more: page.has_more, next_cursor, - sort: "updated_desc".to_string(), + sort: "priority".to_string(), source_limit: None, source_truncated: false, }, From 4e738ac5ebfaf8129e5e83f53ea90f9c9f4e462b Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 07:57:20 +0900 Subject: [PATCH 08/13] fix: query authoritative merge request review events --- crates/workspace-server/src/authority.rs | 41 +++++++++++++++++++++--- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 16c92999..405ca19f 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -402,11 +402,27 @@ impl SqliteWorkspaceAuthority { JOIN merge_requests request ON request.workspace_id=relation.workspace_id AND request.merge_request_id=relation.merge_request_id WHERE relation.workspace_id=t.workspace_id AND relation.ticket_id=t.ticket_id ORDER BY CASE WHEN request.state='open' THEN 0 ELSE 1 END, request.created_at DESC LIMIT 1)"; - let review_decision = format!("(SELECT json_extract(event.payload_json,'$.decision') FROM merge_request_events event - WHERE event.workspace_id=t.workspace_id AND event.merge_request_id={merge_request_id} AND event.kind='review' - AND json_extract(event.payload_json,'$.subject_ref')=(SELECT selector_from FROM merge_requests request WHERE request.workspace_id=t.workspace_id AND request.merge_request_id={merge_request_id}) - AND NOT EXISTS (SELECT 1 FROM merge_request_events revoked WHERE revoked.workspace_id=event.workspace_id AND revoked.merge_request_id=event.merge_request_id AND revoked.kind='review_revoked' AND json_extract(revoked.payload_json,'$.review_event_id')=event.event_id) - ORDER BY event.sequence DESC LIMIT 1)"); + let review_subject = format!( + "(SELECT json_extract(requested.payload_json,'$.subject_ref') + FROM merge_request_thread_events requested + WHERE requested.workspace_id=t.workspace_id + AND requested.merge_request_id={merge_request_id} + AND requested.kind='review_requested' + ORDER BY requested.sequence DESC LIMIT 1)" + ); + let review_decision = format!( + "(SELECT json_extract(event.payload_json,'$.decision') + FROM merge_request_thread_events event + WHERE event.workspace_id=t.workspace_id + AND event.merge_request_id={merge_request_id} AND event.kind='review' + AND json_extract(event.payload_json,'$.subject_ref')={review_subject} + AND NOT EXISTS (SELECT 1 FROM merge_request_thread_events revoked + WHERE revoked.workspace_id=event.workspace_id + AND revoked.merge_request_id=event.merge_request_id + AND revoked.kind='review_revoked' + AND json_extract(revoked.payload_json,'$.review_event_id')=event.event_id) + ORDER BY event.sequence DESC LIMIT 1)" + ); let review_status = format!( "CASE WHEN {merge_request_id} IS NULL THEN 'none' WHEN {review_decision}='approve' THEN 'approved' WHEN {review_decision}='request_changes' THEN 'request_changes' ELSE 'pending' END" ); @@ -2545,6 +2561,21 @@ mod tests { .contains(&"body".to_string()) ); assert_eq!(ticket_query.page.limit, 1); + let no_review = authority + .query_tickets(TicketQueryRequest { + review_status: Some("none".to_string()), + sort: Some("updated_desc".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap(); + assert!( + no_review + .items + .iter() + .any(|item| item.id == "00000000001J2"), + "review-status storage predicate must query the authoritative MR thread schema" + ); let historical_event_query = authority .query_tickets(TicketQueryRequest { query: Some("Historical event marker".to_string()), From 9a548d2b5ef2a175f2ed6d6a0e859a32d11127c4 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 08:05:57 +0900 Subject: [PATCH 09/13] fix: require observed merge target completion --- crates/merge-request/src/lib.rs | 30 ++++- crates/merge-request/tests/store.rs | 29 ++++- crates/workspace-server/src/repositories.rs | 87 ++------------- crates/workspace-server/src/server.rs | 118 ++++++++++++++------ 4 files changed, 148 insertions(+), 116 deletions(-) diff --git a/crates/merge-request/src/lib.rs b/crates/merge-request/src/lib.rs index ca8821be..ef2d3a5e 100644 --- a/crates/merge-request/src/lib.rs +++ b/crates/merge-request/src/lib.rs @@ -68,6 +68,15 @@ pub struct MergeRequestAuth { pub assignment_id: String, } +impl MergeRequestAuth { + fn actor(&self) -> WorkerIdentity { + WorkerIdentity { + runtime_id: self.runtime_id.clone(), + worker_id: self.worker_id.clone(), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ReviewRequestedEvent { pub event_id: String, @@ -641,7 +650,7 @@ impl MergeRequestStore { } pub fn validate_completion(&self, i: &CompleteMergeRequest) -> Result<(), MergeRequestError> { let mr = self.get(&i.auth.workspace_id, &i.ticket_id)?; - self.completion_auth(&i.auth, &i.ticket_id, &mr.repository_id)?; + self.repo(&i.auth, &mr.repository_id)?; if let Some(existing) = mr.thread.iter().find_map(|event| match event { MergeRequestThreadEvent::Merge(value) if value.operation_id == i.operation_id => { Some(value) @@ -649,8 +658,12 @@ impl MergeRequestStore { _ => None, }) { if existing.approval_event_id == i.approval_event_id + && existing.approved_source_ref == i.current_subject_ref && existing.target_ref_before == i.target_ref_before && existing.target_ref_after == i.target_ref_after + && existing.strategy == i.strategy + && existing.resolution == i.resolution + && existing.merged_by == i.auth.actor() { return Ok(()); } @@ -658,6 +671,7 @@ impl MergeRequestStore { "operation fingerprint mismatch".into(), )); } + self.completion_auth(&i.auth, &i.ticket_id, &mr.repository_id)?; if mr.state != MergeRequestState::Open { return Err(MergeRequestError::Conflict( "Merge Request is not open".into(), @@ -711,8 +725,12 @@ impl MergeRequestStore { _ => None, }) { if existing.approval_event_id == i.approval_event_id + && existing.approved_source_ref == i.current_subject_ref && existing.target_ref_before == i.target_ref_before && existing.target_ref_after == i.target_ref_after + && existing.strategy == i.strategy + && existing.resolution == i.resolution + && existing.merged_by == i.auth.actor() { return Ok(existing.clone()); } @@ -767,6 +785,16 @@ impl MergeRequestStore { WHERE workspace_id=?1 AND ticket_id=?2 AND workflow_state='inprogress'", params![mr.workspace_id, i.ticket_id, i.now.to_rfc3339()], )?; + let released_assignment = transaction.execute( + "DELETE FROM ticket_current_worker_assignments + WHERE workspace_id=?1 AND ticket_id=?2 AND assignment_id=?3", + params![mr.workspace_id, i.ticket_id, i.auth.assignment_id], + )?; + if released_assignment != 1 { + return Err(MergeRequestError::Unauthorized( + "completion assignment changed while closing Ticket".into(), + )); + } let issued_grants = { let mut statement = transaction.prepare( "SELECT request_event_id,subject_ref,capability_token diff --git a/crates/merge-request/tests/store.rs b/crates/merge-request/tests/store.rs index 33968cad..9be7ba0a 100644 --- a/crates/merge-request/tests/store.rs +++ b/crates/merge-request/tests/store.rs @@ -93,7 +93,7 @@ fn approve(s: &MergeRequestStore, subject: &str, token: &str) -> ReviewEvent { } #[test] fn selectors_thread_and_completion_have_no_revision_or_commit_api() { - let (_d, s) = fixture(); + let (d, s) = fixture(); open(&s); let review = approve(&s, "opaque-source-ref", "token"); let ready = s @@ -122,6 +122,33 @@ fn selectors_thread_and_completion_have_no_revision_or_commit_api() { let mr = s.get("W", "T").unwrap(); assert_eq!(mr.selector_from.as_deref(), Some("work/t")); assert_eq!(mr.state, MergeRequestState::Merged); + let current_assignment: bool = Connection::open(d.path().join("db")) + .unwrap() + .query_row( + "SELECT EXISTS( + SELECT 1 FROM ticket_current_worker_assignments + WHERE workspace_id='W' AND ticket_id='T' + )", + [], + |row| row.get(0), + ) + .unwrap(); + assert!(!current_assignment); + let replayed = s + .complete(CompleteMergeRequest { + ticket_id: "T".into(), + operation_id: "op".into(), + approval_event_id: merged.approval_event_id.clone(), + current_subject_ref: merged.approved_source_ref.clone(), + target_ref_before: merged.target_ref_before.clone(), + target_ref_after: merged.target_ref_after.clone(), + strategy: merged.strategy, + resolution: merged.resolution, + auth: auth(), + now: at(6), + }) + .unwrap(); + assert_eq!(replayed, merged); let json = serde_json::to_string(&mr).unwrap(); for banned in [ "revision_id", diff --git a/crates/workspace-server/src/repositories.rs b/crates/workspace-server/src/repositories.rs index 908bc7a3..21b15634 100644 --- a/crates/workspace-server/src/repositories.rs +++ b/crates/workspace-server/src/repositories.rs @@ -97,38 +97,13 @@ pub struct CommitObservation { #[derive(Debug, Clone, PartialEq, Eq)] pub enum RepositoryLookupError { - UnknownRepository { - id: RepositoryId, - }, - UnsupportedProvider { - id: RepositoryId, - provider: String, - }, - MissingDefaultSelector { - id: RepositoryId, - }, - InvalidSelector { - id: RepositoryId, - selector: String, - }, - CommitNotFound { - id: RepositoryId, - commit: String, - }, - InvalidCommitRelation { - id: RepositoryId, - detail: String, - }, - TargetMoved { - id: RepositoryId, - selector: String, - expected: String, - observed: Option, - }, - ProviderFailure { - id: RepositoryId, - operation: String, - }, + UnknownRepository { id: RepositoryId }, + UnsupportedProvider { id: RepositoryId, provider: String }, + MissingDefaultSelector { id: RepositoryId }, + InvalidSelector { id: RepositoryId, selector: String }, + CommitNotFound { id: RepositoryId, commit: String }, + InvalidCommitRelation { id: RepositoryId, detail: String }, + ProviderFailure { id: RepositoryId, operation: String }, } #[derive(Debug, Clone)] @@ -306,45 +281,6 @@ impl RepositoryRegistryReader { } } - pub fn update_merge_target( - &self, - id: &str, - selector: &str, - expected_target: &str, - result_commit: &str, - ) -> Result<(), RepositoryLookupError> { - let repository = self.merge_repository(id)?; - let target_ref = normalize_target_branch_selector(id, selector)?; - self.observe_commit(id, result_commit)?; - let status = Command::new("git") - .arg("-C") - .arg(&repository.path) - .args([ - "update-ref", - target_ref.as_str(), - result_commit, - expected_target, - ]) - .status() - .map_err(|_| RepositoryLookupError::ProviderFailure { - id: id.into(), - operation: "guarded target update".into(), - })?; - if status.success() { - return Ok(()); - } - let observed = self - .observe_merge_target(id, Some(selector)) - .ok() - .map(|target| target.commit); - Err(RepositoryLookupError::TargetMoved { - id: id.into(), - selector: selector.into(), - expected: expected_target.into(), - observed, - }) - } - fn merge_repository(&self, id: &str) -> Result<&ConfiguredRepository, RepositoryLookupError> { let repository = self .find(id) @@ -755,20 +691,13 @@ mod tests { vec![base.clone()] ); reader.ensure_ancestor("main", &base, &source).unwrap(); - reader - .update_merge_target("main", "main", &base, &source) - .unwrap(); assert_eq!( reader .observe_merge_target("main", Some("refs/heads/main")) .unwrap() .commit, - source + base ); - assert!(matches!( - reader.update_merge_target("main", "refs/heads/main", &base, &base), - Err(RepositoryLookupError::TargetMoved { .. }) - )); assert!(matches!( reader.ensure_ancestor("main", &source, &base), Err(RepositoryLookupError::InvalidCommitRelation { .. }) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index a843004d..b12b3c14 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3893,6 +3893,23 @@ fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError { .into() } +fn require_completed_target_observation( + observed: &str, + target_ref_before: &str, + target_ref_after: &str, +) -> ApiResult<()> { + if observed == target_ref_after { + return Ok(()); + } + if observed == target_ref_before { + return Err(Error::InvalidInput( + "target selector is still at target_ref_before; push the verified result from the Orchestrator Workdir before MergeRequestComplete".into(), + ) + .into()); + } + Err(Error::InvalidInput("target selector moved outside completion evidence".into()).into()) +} + async fn scoped_show_merge_request( State(api): State, AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>, @@ -4226,19 +4243,55 @@ async fn scoped_complete_merge_request( require_workspace_access(&workspace_id, &api)?; let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?; require_online_workspace_orchestrator_source(&api, &source)?; + let store = merge_request_store(&api, &workspace_id)?; + let mr = store.get(&workspace_id, &ticket_id)?; + let repositories = api.repository_reader(); + if let Some(existing) = mr.thread.iter().find_map(|event| match event { + merge_request::MergeRequestThreadEvent::Merge(event) + if event.operation_id == input.operation_id => + { + Some(event) + } + _ => None, + }) { + let observed = repositories + .observe_merge_target(&mr.repository_id, Some(&mr.selector_to)) + .map_err(repository_merge_evidence_error)?; + require_completed_target_observation( + &observed.commit, + &input.target_ref_before, + &input.target_ref_after, + )?; + let replay = merge_request::CompleteMergeRequest { + ticket_id, + operation_id: input.operation_id, + approval_event_id: input.approval_event_id, + current_subject_ref: existing.approved_source_ref.clone(), + target_ref_before: input.target_ref_before, + target_ref_after: input.target_ref_after, + strategy: input.strategy, + resolution: input.resolution, + auth: merge_request::MergeRequestAuth { + workspace_id, + repository_id: mr.repository_id.clone(), + runtime_id: source.runtime_id, + worker_id: source.worker_id, + assignment_id: String::new(), + }, + now: Utc::now(), + }; + return store.complete(replay).map(Json).map_err(Into::into); + } let assignment = api .store .get_current_ticket_worker_assignment(&workspace_id, &ticket_id)? .ok_or_else(|| { Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into()) })?; - let store = merge_request_store(&api, &workspace_id)?; - let mr = store.get(&workspace_id, &ticket_id)?; let selector = mr .selector_from .as_deref() .ok_or_else(|| Error::InvalidInput("selector_from requires repair".into()))?; - let repositories = api.repository_reader(); let current_source_ref = repositories .observe_merge_target(&mr.repository_id, Some(selector)) .map_err(repository_merge_evidence_error)? @@ -4246,12 +4299,11 @@ async fn scoped_complete_merge_request( let observed = repositories .observe_merge_target(&mr.repository_id, Some(&mr.selector_to)) .map_err(repository_merge_evidence_error)?; - if observed.commit != input.target_ref_before && observed.commit != input.target_ref_after { - return Err(Error::InvalidInput( - "target selector moved outside completion evidence".into(), - ) - .into()); - } + require_completed_target_observation( + &observed.commit, + &input.target_ref_before, + &input.target_ref_after, + )?; let completion = merge_request::CompleteMergeRequest { ticket_id, operation_id: input.operation_id, @@ -4271,31 +4323,7 @@ async fn scoped_complete_merge_request( now: Utc::now(), }; store.validate_completion(&completion)?; - let already = observed.commit == input.target_ref_after; - if !already { - repositories - .update_merge_target( - &mr.repository_id, - &mr.selector_to, - &input.target_ref_before, - &input.target_ref_after, - ) - .map_err(repository_merge_evidence_error)? - } - match store.complete(completion) { - Ok(v) => Ok(Json(v)), - Err(e) => { - if !already { - let _ = repositories.update_merge_target( - &mr.repository_id, - &mr.selector_to, - &input.target_ref_after, - &input.target_ref_before, - ); - } - Err(e.into()) - } - } + store.complete(completion).map(Json).map_err(Into::into) } fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> { @@ -12443,7 +12471,7 @@ mod tests { assert_eq!(builtin.definition.name, "coder-review"); assert_eq!(builtin.selector.to_string(), "builtin:coder-review"); assert_eq!(builtin.flow_id, "builtin:coder-review"); - assert_eq!(builtin.revision, 2); + assert_eq!(builtin.revision, 3); assert_eq!( api.store .list_flow_sources(&api.config.workspace_id) @@ -12604,6 +12632,26 @@ mod tests { ); } + #[test] + fn merge_request_completion_records_only_an_observed_remote_target_update() { + require_completed_target_observation("after", "before", "after").unwrap(); + + let not_pushed = + require_completed_target_observation("before", "before", "after").unwrap_err(); + assert!(matches!( + not_pushed.error, + Error::InvalidInput(ref message) + if message.contains("push the verified result from the Orchestrator Workdir") + )); + + let moved = require_completed_target_observation("other", "before", "after").unwrap_err(); + assert!(matches!( + moved.error, + Error::InvalidInput(ref message) + if message.contains("moved outside completion evidence") + )); + } + #[test] fn worker_ticket_assignment_projects_coder_intent_and_run_acceptance() { let initial_submit = vec![ From f86ae6d52f6d848b95a0455ce79f30453ccd6c01 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 08:06:02 +0900 Subject: [PATCH 10/13] feat: connect selector-based merge request flow --- crates/flow/src/builtin.rs | 19 ++++++++++--------- crates/worker/src/prompt/catalog.rs | 22 ++++++++++++++++++++++ resources/flows/coder-review.dcdl | 8 ++++---- resources/prompts/role/coder.md | 4 +++- resources/prompts/role/orchestrator.md | 6 +++++- resources/prompts/role/reviewer.md | 2 +- 6 files changed, 45 insertions(+), 16 deletions(-) diff --git a/crates/flow/src/builtin.rs b/crates/flow/src/builtin.rs index 0f40b52e..c707de05 100644 --- a/crates/flow/src/builtin.rs +++ b/crates/flow/src/builtin.rs @@ -24,7 +24,7 @@ pub fn builtin_flow_source(slug: &str) -> Option { match slug { CODER_REVIEW_FLOW_SLUG => Some(BuiltinFlowSource { slug: CODER_REVIEW_FLOW_SLUG, - revision: 2, + revision: 3, path: "builtin/flows/coder-review.dcdl", content: CODER_REVIEW_FLOW_SOURCE, }), @@ -35,7 +35,7 @@ pub fn builtin_flow_source(slug: &str) -> Option { pub fn builtin_flow_sources() -> &'static [BuiltinFlowSource] { const SOURCES: &[BuiltinFlowSource] = &[BuiltinFlowSource { slug: CODER_REVIEW_FLOW_SLUG, - revision: 2, + revision: 3, path: "builtin/flows/coder-review.dcdl", content: CODER_REVIEW_FLOW_SOURCE, }]; @@ -69,20 +69,21 @@ mod tests { } #[test] - fn coder_review_starts_on_a_ticket_branch_and_requires_committed_review_evidence() { + fn coder_review_publishes_immutable_source_before_review_and_preserves_fresh_review() { let source = builtin_flow_source(CODER_REVIEW_FLOW_SLUG).expect("coder-review Flow must exist"); for required in [ - "detached HEAD", + "Workdir Git state", "work/-", - "explicitly authorized", "git add", "git commit", - "Workdir is clean", - "current head commit", - "same Ticket work branch", - "new revision", + "Publish the committed source ref", + "verify that the remote selector resolves to the exact local HEAD", + "immutable Merge Request", + "current Merge Request subject", + "fresh read-only Reviewer child", + "do not update the target selector", ] { assert!( source.content.contains(required), diff --git a/crates/worker/src/prompt/catalog.rs b/crates/worker/src/prompt/catalog.rs index 53981905..7fb93c81 100644 --- a/crates/worker/src/prompt/catalog.rs +++ b/crates/worker/src/prompt/catalog.rs @@ -665,9 +665,31 @@ mod tests { "Do not spawn, restore, assign, or route work to Backend/Runtime Reviewer Workers" )); assert!(prompt.contains("never compensate by creating an independent Reviewer Worker")); + assert!(prompt.contains("durable `ready -> queued` transition delegates")); + assert!(prompt.contains("use the Ticket repository `origin` transport")); + assert!(prompt.contains("guarded non-force push")); + assert!(prompt.contains("Never mutate a Server-side repository path")); + assert!(prompt.contains("If the repository push succeeds but completion recording fails")); + assert!(prompt.contains("closes the current assignment atomically")); assert!(!prompt.contains("sibling Coder/Reviewer Workers")); } + #[test] + fn coder_and_reviewer_roles_preserve_immutable_subject_authority() { + let catalog = PromptCatalog::builtins_only().unwrap(); + let coder = &catalog.projection.templates["role.coder"]; + assert!(coder.contains("publish the committed source selector without force")); + assert!(coder.contains("exact local `HEAD`")); + assert!(coder.contains("keep that source ref immutable")); + assert!(coder.contains("Do not update the target selector")); + + let reviewer = &catalog.projection.templates["role.reviewer"]; + assert!(reviewer.contains("never as a supplied verdict")); + assert!(reviewer.contains("ReviewRequested.subject_ref")); + assert!(reviewer.contains("do not merge")); + assert!(reviewer.contains("update a repository ref")); + } + #[test] fn existing_internal_prompt_render_contracts_are_preserved() { let catalog = PromptCatalog::builtins_only().unwrap(); diff --git a/resources/flows/coder-review.dcdl b/resources/flows/coder-review.dcdl index c01b1d1d..bf5a4e64 100644 --- a/resources/flows/coder-review.dcdl +++ b/resources/flows/coder-review.dcdl @@ -5,7 +5,7 @@ states = { implement = { - instructions = "Before editing, inspect the assigned Workdir's Git state. A newly delegated Git Workdir normally starts at a detached HEAD. If HEAD is detached, create and switch to a local branch named `work/-`, using the canonical Ticket id and a short lowercase kebab-case slug derived from the Ticket title or implementation scope. If the Workdir is already on a suitable work branch after restore, keep it. Never delete, reset, or overwrite an existing branch to resolve a name collision; choose a concise collision-free suffix and report the actual branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Commit coherent, validated implementation slices while working; do not push, merge, force-rewrite a submitted revision, delete branches, or discard pre-existing changes. Implement the requested Ticket scope, run the narrow and dependent validation required by the changed contracts, and record concrete evidence. Open or update the Ticket Merge Request with immutable repository revision evidence. Before requesting independent review, commit all intended changes, confirm that the Workdir is clean, and make the current MR revision authoritative. A Flow transition is never Ticket completion authority."; + instructions = "Inspect the assigned Workdir Git state before editing. Reuse a suitable restored `work/-` branch, or create a collision-safe work branch from detached HEAD; never overwrite an existing branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Implement and validate the Ticket in coherent commits. Publish the committed source ref to the Ticket repository remote without force, verify that the remote selector resolves to the exact local HEAD, and open the Merge Request with immutable `selector_from` / `selector_to` evidence. Do not request review from a dirty Workdir or an unpublished source ref. Do not merge, force-rewrite a submitted revision, delete branches, or discard pre-existing changes. A Flow transition is never Ticket completion authority."; transitions = { review = { target = "review"; @@ -15,7 +15,7 @@ }; review = { - instructions = "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 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, 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."; 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. Do not rewrite the previously reviewed commit or claim approval from the prior request_changes review. When the corrected committed revision is ready for a new independent review, request a Flow transition."; + 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, publish the updated source ref without force, and verify its remote tip. Return to the existing open Merge Request and request review from a fresh read-only Reviewer child so the trusted spawn layer captures a new immutable subject. Do not rewrite the previously reviewed commit or claim approval from the prior request_changes review. When the corrected committed revision is ready for a new independent review, request a Flow transition."; transitions = { review = { target = "review"; @@ -39,7 +39,7 @@ }; complete = { - instructions = "Leave concise implementation and validation evidence on the Ticket, then hand off the exact approved Merge Request revision to the Orchestrator for readiness and integration. Do not call MergeRequestComplete; Coder approval handoff is not Ticket completion authority. After durable handoff evidence exists, request a Flow transition."; + instructions = "Verify the authoritative approval still matches the exact current Merge Request subject, keep the reviewed source ref immutable, leave concise implementation and validation evidence on the Ticket, then hand off to the Orchestrator. Do not call MergeRequestComplete, do not update the target selector, or treat the Flow terminal state as Ticket completion authority. After durable handoff evidence exists, request a Flow transition."; transitions = { completed = { target = "done"; diff --git a/resources/prompts/role/coder.md b/resources/prompts/role/coder.md index 29932418..2706e1d6 100644 --- a/resources/prompts/role/coder.md +++ b/resources/prompts/role/coder.md @@ -2,8 +2,10 @@ You are the assigned Coder. Implement the requested scope in the provided Workdi Treat the first committed user message as the bounded Ticket/action context and do not infer control-plane identity from prose. +Before opening a Merge Request, publish the committed source selector without force and verify that the Ticket repository remote resolves it to the exact local `HEAD`; a local branch name or dirty Workdir is not immutable review evidence. + {% 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. -A request-changes result requires a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request revision has authoritative approval, leave concise implementation evidence on the Ticket and hand off integration to the Orchestrator. Do not call `MergeRequestComplete`. +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, and hand off integration to the Orchestrator. Do not update the target selector. Do not call `MergeRequestComplete`. diff --git a/resources/prompts/role/orchestrator.md b/resources/prompts/role/orchestrator.md index 32092653..68b4d9f5 100644 --- a/resources/prompts/role/orchestrator.md +++ b/resources/prompts/role/orchestrator.md @@ -2,10 +2,14 @@ You are the Ticket Orchestrator role. {% include "common.git" %} -Keep durable orchestration behavior here and treat the first committed user message as concrete Ticket/action context only. Use typed Ticket tools and current repository state as authority. Record `inprogress` before implementation side effects, then use `SpawnTicketCoder` so Worker creation, the fixed Coder profile/Flow, and the current Ticket assignment are one guarded operation. After spawn, reread the Ticket and verify its current assignment names that Coder before asking it to implement; never route implementation to an unassigned Coder. Route implementation work to sibling Coder Workers, and stop for human authority when merge/closure is not explicitly delegated. +Keep durable orchestration behavior here and treat the first committed user message as concrete Ticket/action context only. Use typed Ticket tools and current repository state as authority. Record `inprogress` before implementation side effects, then use `SpawnTicketCoder` so Worker creation, the fixed Coder profile/Flow, and the current Ticket assignment are one guarded operation. After spawn, reread the Ticket and verify its current assignment names that Coder before asking it to implement; never route implementation to an unassigned Coder. Route implementation work to sibling Coder Workers. A durable `ready -> queued` transition delegates ordinary implementation orchestration and integration of the exact approved Merge Request to this Orchestrator unless the Ticket or a later user instruction explicitly limits that delegation; never infer broader repository authority from prose alone. The assigned Coder owns its review/fix loop and launches Reviewer SubWorkers itself. Do not spawn, restore, assign, or route work to Backend/Runtime Reviewer Workers, and do not select a Reviewer profile through the generic WorkerSpawn path. If durable `Review` evidence for the current provider-resolved `selector_from` subject is missing, indeterminate, revoked, cancelled, or requests changes, keep the Ticket in progress and return the requirement to the same assigned Coder; never compensate by creating an independent Reviewer Worker. +Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. In the Orchestrator Workdir, use the Ticket repository `origin` transport to fetch the current target selector and immutable source selector, verify both against readiness evidence, apply the selected fast-forward or merge strategy, and validate the resulting tree. Push only a result that descends from the observed target, using a guarded non-force push whose expected old target is `target_ref_before`; reject target movement and conflicts rather than rewriting the remote. Verify the remote target now resolves exactly to `target_ref_after`, then call `MergeRequestComplete` with that before/after evidence and the authoritative approval event. Never mutate a Server-side repository path or use local `git update-ref` as integration authority. + +If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: the Server accepts the already-observed exact `target_ref_after`, records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Any other observed target is a stale/conflicting completion and must fail closed. + Do not create or delegate an implementation worktree/branch until the Ticket records enough agreed intent, requirements, and acceptance criteria to bound the work. Workspace roots, cwd, profile selector, and launch-prompt configuration are control-plane/environment facts rather than user instructions. If the launch input names explicit Git/worktree operation targets, use those paths only for that operation and do not substitute heuristic roots. diff --git a/resources/prompts/role/reviewer.md b/resources/prompts/role/reviewer.md index 8f2c05cc..24677be6 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. 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, 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 read-only inspection and focused validation; do not merge, close, mutate the Workdir, 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 b723c64fa103cad0f957660412d6d41855f2ead0 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 08:06:21 +0900 Subject: [PATCH 11/13] fix: preserve accepted Ticket query filters --- crates/workspace-server/src/authority.rs | 82 +++++++++++++++++++----- 1 file changed, 66 insertions(+), 16 deletions(-) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 405ca19f..53349022 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -354,11 +354,11 @@ impl SqliteWorkspaceAuthority { } if let Some(value) = &query.updated_after { let value = bind(SqlValue::Text(value.clone())); - predicates.push(format!("COALESCE(t.updated_at,'')>={value}")); + predicates.push(format!("COALESCE(t.updated_at,'')>{value}")); } if let Some(value) = &query.updated_before { let value = bind(SqlValue::Text(value.clone())); - predicates.push(format!("COALESCE(t.updated_at,'')<={value}")); + predicates.push(format!("COALESCE(t.updated_at,'')<{value}")); } if let Some(value) = &query.linked_objective_id { let value = bind(SqlValue::Text(value.clone())); @@ -373,7 +373,9 @@ impl SqliteWorkspaceAuthority { .relation_kind .as_ref() .map(|value| bind(SqlValue::Text(value.clone()))); - let related = related.map(|value| format!("AND ((r.ticket_id=t.ticket_id AND r.target={value}) OR (r.target=t.ticket_id AND r.ticket_id={value}))")).unwrap_or_default(); + let related = related + .map(|value| format!("AND r.ticket_id=t.ticket_id AND r.target={value}")) + .unwrap_or_else(|| "AND r.ticket_id=t.ticket_id".to_string()); let kind = kind .map(|value| format!("AND r.kind={value}")) .unwrap_or_default(); @@ -427,14 +429,20 @@ impl SqliteWorkspaceAuthority { "CASE WHEN {merge_request_id} IS NULL THEN 'none' WHEN {review_decision}='approve' THEN 'approved' WHEN {review_decision}='request_changes' THEN 'request_changes' ELSE 'pending' END" ); let has_commit = format!( - "({merge_request_id} IS NOT NULL OR EXISTS (SELECT 1 FROM typed_ticket_event_references reference WHERE reference.workspace_id=t.workspace_id AND reference.ticket_id=t.ticket_id AND reference.kind='commit'))" + "({review_subject} IS NOT NULL OR EXISTS (SELECT 1 FROM typed_ticket_event_references reference WHERE reference.workspace_id=t.workspace_id AND reference.ticket_id=t.ticket_id AND reference.kind='commit'))" ); - for event_kind in &query.event_kinds { - let event_kind = bind(SqlValue::Text(event_kind.clone())); - predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind={event_kind})")); + if !query.event_kinds.is_empty() { + let event_kinds = query + .event_kinds + .iter() + .map(|event_kind| bind(SqlValue::Text(event_kind.clone()))) + .collect::>(); + predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind IN ({}))", event_kinds.join(","))); } for evidence in &query.evidence { predicates.push(match evidence.as_str() { + "implementation_report" => format!("{report_index} IS NOT NULL"), + "implementation_report_after_rescope" => current_report.clone(), "merge_request" => format!("{merge_request_id} IS NOT NULL"), "commit" => has_commit.clone(), "approved_review" => format!("{review_status}='approved'"), @@ -446,7 +454,7 @@ impl SqliteWorkspaceAuthority { }); } if let Some(status) = &query.review_status { - let status = if status == "unresolved_changes" { + let status = if matches!(status.as_str(), "unresolved_changes" | "changes_requested") { "request_changes" } else { status.as_str() @@ -457,15 +465,29 @@ impl SqliteWorkspaceAuthority { for attention in &query.attention { predicates.push(match attention.as_str() { "done_not_closed" => "t.workflow_state='done'".to_string(), - "unresolved_review" | "unresolved_changes" => format!("{review_status}='request_changes'"), + "implementation_report_not_closed" => { + format!("{report_index} IS NOT NULL AND t.workflow_state!='closed'") + } + "report_after_rescope" => current_report.clone(), + "unresolved_review" | "unresolved_changes" => { + format!("{review_status}='request_changes'") + } "missing_commit" => format!("NOT {has_commit}"), "blocked" => blocker.to_string(), "unblocked" => format!("NOT {blocker}"), - "ready" => format!("t.workflow_state='ready' AND NOT {active_blocker}"), - "awaiting_review" => format!("t.workflow_state='inprogress' AND {current_report} AND {review_status}='pending'"), - "stale_after_rescope" => format!("{report_index} IS NOT NULL AND {edit_index}>{report_index}"), - "missing_evidence" => format!("NOT ({current_report} AND {has_commit} AND {review_status}='approved')"), - other => return Err(Error::InvalidRecordId(format!("unsupported attention filter `{other}`"))), + "ready" => format!("t.workflow_state='ready' AND NOT {blocker}"), + "awaiting_review" => format!("{review_status}='pending'"), + "stale_after_rescope" => { + format!("{report_index} IS NOT NULL AND NOT {current_report}") + } + "missing_evidence" => format!( + "NOT ({current_report} AND {has_commit} AND {review_status}='approved')" + ), + other => { + return Err(Error::InvalidRecordId(format!( + "unsupported attention filter `{other}`" + ))); + } }); } let rank_expression = match sort { @@ -561,11 +583,11 @@ impl SqliteWorkspaceAuthority { } if let Some(value) = &query.updated_after { let value = bind(SqlValue::Text(value.clone())); - predicates.push(format!("o.updated_at>={value}")); + predicates.push(format!("o.updated_at>{value}")); } if let Some(value) = &query.updated_before { let value = bind(SqlValue::Text(value.clone())); - predicates.push(format!("o.updated_at<={value}")); + predicates.push(format!("o.updated_at<{value}")); } if let Some(value) = &query.linked_ticket_id { let value = bind(SqlValue::Text(value.clone())); @@ -2576,6 +2598,34 @@ mod tests { .any(|item| item.id == "00000000001J2"), "review-status storage predicate must query the authoritative MR thread schema" ); + for evidence in [ + "implementation_report", + "implementation_report_after_rescope", + ] { + authority + .query_tickets(TicketQueryRequest { + evidence: vec![evidence.to_string()], + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap_or_else(|error| panic!("accepted evidence filter {evidence}: {error}")); + } + for attention in ["implementation_report_not_closed", "report_after_rescope"] { + authority + .query_tickets(TicketQueryRequest { + attention: vec![attention.to_string()], + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap_or_else(|error| panic!("accepted attention filter {attention}: {error}")); + } + authority + .query_tickets(TicketQueryRequest { + review_status: Some("changes_requested".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .expect("accepted review-status alias must execute"); let historical_event_query = authority .query_tickets(TicketQueryRequest { query: Some("Historical event marker".to_string()), From ab0f57c00ad4eaf1c734006c941ef283b104330e Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 08:14:05 +0900 Subject: [PATCH 12/13] fix: include incoming Ticket relation filters --- crates/workspace-server/src/authority.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index 53349022..0c8f0abb 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -374,8 +374,10 @@ impl SqliteWorkspaceAuthority { .as_ref() .map(|value| bind(SqlValue::Text(value.clone()))); let related = related - .map(|value| format!("AND r.ticket_id=t.ticket_id AND r.target={value}")) - .unwrap_or_else(|| "AND r.ticket_id=t.ticket_id".to_string()); + .map(|value| format!("AND ((r.ticket_id=t.ticket_id AND r.target={value}) OR (r.target=t.ticket_id AND r.ticket_id={value}))")) + .unwrap_or_else(|| { + "AND (r.ticket_id=t.ticket_id OR r.target=t.ticket_id)".to_string() + }); let kind = kind .map(|value| format!("AND r.kind={value}")) .unwrap_or_default(); @@ -2678,6 +2680,16 @@ mod tests { .unwrap(); assert_eq!(exact_relation.items.len(), 1); assert_eq!(exact_relation.items[0].id, "00000000001J2"); + let incoming_relation = authority + .query_tickets(TicketQueryRequest { + related_ticket_id: Some("00000000001J2".to_string()), + relation_kind: Some("related".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap(); + assert_eq!(incoming_relation.items.len(), 1); + assert_eq!(incoming_relation.items[0].id, "00000000001J5"); let summary_page = authority .list_ticket_page(TicketListPageRequest { states: vec!["planning".to_string(), "ready".to_string()], From 981c422122b0d5888cafc44ed94057be7b963861 Mon Sep 17 00:00:00 2001 From: Hare Date: Tue, 18 Aug 2026 08:14:43 +0900 Subject: [PATCH 13/13] fix: preserve completed merge request replays --- crates/worker/src/prompt/catalog.rs | 1 + crates/workspace-server/src/server.rs | 59 +++++++++++++++++++------- resources/prompts/role/orchestrator.md | 2 +- 3 files changed, 45 insertions(+), 17 deletions(-) diff --git a/crates/worker/src/prompt/catalog.rs b/crates/worker/src/prompt/catalog.rs index 7fb93c81..44f617d2 100644 --- a/crates/worker/src/prompt/catalog.rs +++ b/crates/worker/src/prompt/catalog.rs @@ -670,6 +670,7 @@ mod tests { assert!(prompt.contains("guarded non-force push")); assert!(prompt.contains("Never mutate a Server-side repository path")); assert!(prompt.contains("If the repository push succeeds but completion recording fails")); + assert!(prompt.contains("later target movement does not invalidate an idempotent replay")); assert!(prompt.contains("closes the current assignment atomically")); assert!(!prompt.contains("sibling Coder/Reviewer Workers")); } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index b12b3c14..5f69c5e8 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -3893,6 +3893,20 @@ fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError { .into() } +fn recorded_merge_completion<'a>( + thread: &'a [merge_request::MergeRequestThreadEvent], + operation_id: &str, +) -> Option<&'a merge_request::MergeEvent> { + thread.iter().find_map(|event| match event { + merge_request::MergeRequestThreadEvent::Merge(event) + if event.operation_id == operation_id => + { + Some(event) + } + _ => None, + }) +} + fn require_completed_target_observation( observed: &str, target_ref_before: &str, @@ -4246,22 +4260,7 @@ async fn scoped_complete_merge_request( let store = merge_request_store(&api, &workspace_id)?; let mr = store.get(&workspace_id, &ticket_id)?; let repositories = api.repository_reader(); - if let Some(existing) = mr.thread.iter().find_map(|event| match event { - merge_request::MergeRequestThreadEvent::Merge(event) - if event.operation_id == input.operation_id => - { - Some(event) - } - _ => None, - }) { - let observed = repositories - .observe_merge_target(&mr.repository_id, Some(&mr.selector_to)) - .map_err(repository_merge_evidence_error)?; - require_completed_target_observation( - &observed.commit, - &input.target_ref_before, - &input.target_ref_after, - )?; + if let Some(existing) = recorded_merge_completion(&mr.thread, &input.operation_id) { let replay = merge_request::CompleteMergeRequest { ticket_id, operation_id: input.operation_id, @@ -12632,6 +12631,34 @@ mod tests { ); } + #[test] + fn recorded_completion_replay_is_identified_before_later_target_observation() { + let event = merge_request::MergeEvent { + event_id: "merge-event".into(), + sequence: 1, + operation_id: "operation".into(), + approval_event_id: "approval".into(), + approved_source_ref: "source".into(), + target_ref_before: "before".into(), + target_ref_after: "after".into(), + strategy: merge_request::MergeStrategy::FastForward, + resolution: merge_request::ConflictResolution::None, + merged_by: merge_request::WorkerIdentity { + runtime_id: "runtime".into(), + worker_id: "orchestrator".into(), + }, + created_at: Utc::now(), + }; + let thread = vec![merge_request::MergeRequestThreadEvent::Merge(event.clone())]; + + assert_eq!( + recorded_merge_completion(&thread, "operation"), + Some(&event) + ); + assert!(recorded_merge_completion(&thread, "different").is_none()); + assert!(require_completed_target_observation("later", "before", "after").is_err()); + } + #[test] fn merge_request_completion_records_only_an_observed_remote_target_update() { require_completed_target_observation("after", "before", "after").unwrap(); diff --git a/resources/prompts/role/orchestrator.md b/resources/prompts/role/orchestrator.md index 68b4d9f5..31999379 100644 --- a/resources/prompts/role/orchestrator.md +++ b/resources/prompts/role/orchestrator.md @@ -8,7 +8,7 @@ The assigned Coder owns its review/fix loop and launches Reviewer SubWorkers its Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. In the Orchestrator Workdir, use the Ticket repository `origin` transport to fetch the current target selector and immutable source selector, verify both against readiness evidence, apply the selected fast-forward or merge strategy, and validate the resulting tree. Push only a result that descends from the observed target, using a guarded non-force push whose expected old target is `target_ref_before`; reject target movement and conflicts rather than rewriting the remote. Verify the remote target now resolves exactly to `target_ref_after`, then call `MergeRequestComplete` with that before/after evidence and the authoritative approval event. Never mutate a Server-side repository path or use local `git update-ref` as integration authority. -If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: the Server accepts the already-observed exact `target_ref_after`, records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Any other observed target is a stale/conflicting completion and must fail closed. +If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: while no completion event exists, the Server requires the target to remain at the exact `target_ref_after` before it records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Once that exact operation is recorded, later target movement does not invalidate an idempotent replay of the recorded result. Before recording, any other observed target is a stale/conflicting completion and must fail closed. Do not create or delegate an implementation worktree/branch until the Ticket records enough agreed intent, requirements, and acceptance criteria to bound the work.