From a2cd8601991827b29db82001cb10220f131382e3 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 17 Aug 2026 14:23:47 +0900 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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);