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/manifest/src/config.rs b/crates/manifest/src/config.rs index 8be096c7..33f2b295 100644 --- a/crates/manifest/src/config.rs +++ b/crates/manifest/src/config.rs @@ -18,9 +18,10 @@ use crate::model::{AuthRef, ModelManifest, ReasoningControl}; use crate::plugin::PluginConfig; use crate::{ CompactionConfig, EngineManifest, FeatureConfig, FeatureFlagConfig, FileUploadLimits, - McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig, ScopeConfig, - SessionConfig, SkillsConfig, TicketFeatureConfig, ToolOutputLimits, ToolPermissionConfig, - ToolPermissionRule, WebConfig, WorkerFeatureConfig, WorkerManifest, WorkerMeta, + McpConfig, McpEnvValue, McpStdioCwdPolicy, MemoryConfig, MemoryFeatureConfig, + MergeRequestFeatureConfig, ScopeConfig, SessionConfig, SkillsConfig, TicketFeatureConfig, + ToolOutputLimits, ToolPermissionConfig, ToolPermissionRule, WebConfig, WorkerFeatureConfig, + WorkerManifest, WorkerMeta, }; /// Partial-form Worker manifest. Every field is optional; one or more @@ -97,6 +98,8 @@ pub struct FeatureConfigPartial { #[serde(default)] pub ticket: Option, #[serde(default)] + pub merge_request: Option, + #[serde(default)] pub orchestration: Option, #[serde(default)] pub plugins: Option, @@ -127,6 +130,11 @@ impl FeatureConfigPartial { FeatureFlagConfigPartial::merge, ), ticket: merge_option(self.ticket, other.ticket, TicketFeatureConfigPartial::merge), + merge_request: merge_option( + self.merge_request, + other.merge_request, + MergeRequestFeatureConfigPartial::merge, + ), orchestration: merge_option( self.orchestration, other.orchestration, @@ -216,6 +224,28 @@ impl TicketFeatureConfigPartial { } } +#[derive(Debug, Clone, Default, Deserialize, Serialize, PartialEq, Eq)] +#[serde(default, deny_unknown_fields)] +pub struct MergeRequestFeatureConfigPartial { + pub show: Option, + pub open: Option, + pub review: Option, + pub readiness_check: Option, + pub complete: Option, +} + +impl MergeRequestFeatureConfigPartial { + fn merge(self, other: Self) -> Self { + Self { + show: other.show.or(self.show), + open: other.open.or(self.open), + review: other.review.or(self.review), + readiness_check: other.readiness_check.or(self.readiness_check), + complete: other.complete.or(self.complete), + } + } +} + impl From for FeatureConfig { fn from(value: FeatureConfigPartial) -> Self { Self { @@ -247,6 +277,10 @@ impl From for FeatureConfig { .ticket .map(TicketFeatureConfig::from) .unwrap_or_default(), + merge_request: value + .merge_request + .map(MergeRequestFeatureConfig::from) + .unwrap_or_default(), orchestration: value .orchestration .map(FeatureFlagConfig::from) @@ -326,6 +360,30 @@ impl From for TicketFeatureConfigPartial { } } +impl From for MergeRequestFeatureConfig { + fn from(value: MergeRequestFeatureConfigPartial) -> Self { + Self { + show: value.show.unwrap_or_default(), + open: value.open.unwrap_or_default(), + review: value.review.unwrap_or_default(), + readiness_check: value.readiness_check.unwrap_or_default(), + complete: value.complete.unwrap_or_default(), + } + } +} + +impl From for MergeRequestFeatureConfigPartial { + fn from(value: MergeRequestFeatureConfig) -> Self { + Self { + show: Some(value.show), + open: Some(value.open), + review: Some(value.review), + readiness_check: Some(value.readiness_check), + complete: Some(value.complete), + } + } +} + impl From for FeatureConfigPartial { fn from(value: FeatureConfig) -> Self { Self { @@ -339,6 +397,7 @@ impl From for FeatureConfigPartial { objective: Some(value.objective.into()), manage_workdir: Some(value.manage_workdir.into()), ticket: Some(value.ticket.into()), + merge_request: Some(value.merge_request.into()), orchestration: Some(value.orchestration.into()), plugins: Some(value.plugins.into()), } @@ -1880,6 +1939,7 @@ worker_max_turns = 7 assert!(!manifest.feature.objective.enabled); assert!(!manifest.feature.manage_workdir.enabled); assert!(!manifest.feature.ticket.enabled); + assert!(!manifest.feature.merge_request.any()); } #[test] @@ -1899,6 +1959,13 @@ thread = false intake = false workflow = false +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1934,6 +2001,14 @@ enabled = false assert!(!manifest.feature.ticket.thread); assert!(!manifest.feature.ticket.intake); assert!(!manifest.feature.ticket.workflow); + assert_eq!( + manifest.feature.merge_request, + MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert!(!manifest.feature.orchestration.enabled); assert!(!manifest.feature.memory.enabled); assert!(!manifest.feature.memory.staging); @@ -1957,6 +2032,13 @@ thread = false intake = false workflow = false +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1968,6 +2050,11 @@ enabled = false thread = true workflow = true +[feature.merge_request] +open = true +review = false +readiness_check = true + [feature.orchestration] enabled = true @@ -2017,6 +2104,16 @@ enabled = true assert!(manifest.feature.ticket.thread); assert!(!manifest.feature.ticket.intake); assert!(manifest.feature.ticket.workflow); + assert_eq!( + manifest.feature.merge_request, + MergeRequestFeatureConfig { + show: true, + open: true, + review: false, + readiness_check: true, + complete: false, + } + ); assert!(manifest.feature.orchestration.enabled); assert!(manifest.feature.objective.enabled); assert!(manifest.feature.web.enabled); diff --git a/crates/manifest/src/lib.rs b/crates/manifest/src/lib.rs index 1f6d1f74..7c7c418c 100644 --- a/crates/manifest/src/lib.rs +++ b/crates/manifest/src/lib.rs @@ -125,6 +125,8 @@ pub struct FeatureConfig { #[serde(default)] pub ticket: TicketFeatureConfig, #[serde(default)] + pub merge_request: MergeRequestFeatureConfig, + #[serde(default)] pub orchestration: FeatureFlagConfig, #[serde(default)] pub plugins: FeatureFlagConfig, @@ -143,6 +145,7 @@ impl Default for FeatureConfig { objective: FeatureFlagConfig::disabled(), manage_workdir: FeatureFlagConfig::disabled(), ticket: TicketFeatureConfig::default(), + merge_request: MergeRequestFeatureConfig::default(), orchestration: FeatureFlagConfig::disabled(), plugins: FeatureFlagConfig::disabled(), } @@ -252,6 +255,27 @@ pub struct TicketFeatureConfig { pub workflow: bool, } +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Default)] +#[serde(deny_unknown_fields)] +pub struct MergeRequestFeatureConfig { + #[serde(default)] + pub show: bool, + #[serde(default)] + pub open: bool, + #[serde(default)] + pub review: bool, + #[serde(default)] + pub readiness_check: bool, + #[serde(default)] + pub complete: bool, +} + +impl MergeRequestFeatureConfig { + pub fn any(self) -> bool { + self.show || self.open || self.review || self.readiness_check || self.complete + } +} + /// External Agent Skills (`SKILL.md`) ingest configuration. Skills are /// loaded *only* from the directories listed here — there is no /// implicit `$config_dir/skills/` or builtin probe. Profile and Manifest diff --git a/crates/manifest/src/profile.rs b/crates/manifest/src/profile.rs index a5c655ed..60809859 100644 --- a/crates/manifest/src/profile.rs +++ b/crates/manifest/src/profile.rs @@ -919,6 +919,37 @@ fn apply_role_profile( _ => serde_json::json!({ "enabled": true, "authoring": true, "thread": true }), }; value["feature"]["ticket"] = ticket; + let merge_request = match slug { + "coder" => serde_json::json!({ + "show": true, + "open": true, + "review": false, + "readiness_check": false, + "complete": false + }), + "reviewer" => serde_json::json!({ + "show": true, + "open": false, + "review": true, + "readiness_check": false, + "complete": false + }), + "orchestrator" => serde_json::json!({ + "show": true, + "open": false, + "review": false, + "readiness_check": true, + "complete": true + }), + _ => serde_json::json!({ + "show": false, + "open": false, + "review": false, + "readiness_check": false, + "complete": false + }), + }; + value["feature"]["merge_request"] = merge_request; } fn reject_manifest_shaped_profile(value: &serde_json::Value) -> Result<(), ProfileError> { @@ -1448,6 +1479,14 @@ enabled = true authoring = false thread = false intake = false + +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [feature.orchestration] enabled = false "#, @@ -1470,6 +1509,14 @@ enabled = false assert!(!resolved.manifest.feature.ticket.authoring); assert!(!resolved.manifest.feature.ticket.thread); assert!(!resolved.manifest.feature.ticket.intake); + assert_eq!( + resolved.manifest.feature.merge_request, + crate::MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert!(!resolved.manifest.feature.orchestration.enabled); assert_eq!( resolved.manifest.delegation_scope.allow[0].target, diff --git a/crates/merge-request/src/lib.rs b/crates/merge-request/src/lib.rs index adeebebb..1ba3727b 100644 --- a/crates/merge-request/src/lib.rs +++ b/crates/merge-request/src/lib.rs @@ -75,6 +75,15 @@ pub struct MergeRequestAuth { pub assignment_id: String, } +impl MergeRequestAuth { + fn actor(&self) -> WorkerIdentity { + WorkerIdentity { + runtime_id: self.runtime_id.clone(), + worker_id: self.worker_id.clone(), + } + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct ReviewRequestedEvent { pub event_id: String, @@ -648,7 +657,7 @@ impl MergeRequestStore { } pub fn validate_completion(&self, i: &CompleteMergeRequest) -> Result<(), MergeRequestError> { let mr = self.get(&i.auth.workspace_id, &i.ticket_id)?; - self.completion_auth(&i.auth, &i.ticket_id, &mr.repository_id)?; + self.repo(&i.auth, &mr.repository_id)?; if let Some(existing) = mr.thread.iter().find_map(|event| match event { MergeRequestThreadEvent::Merge(value) if value.operation_id == i.operation_id => { Some(value) @@ -656,8 +665,12 @@ impl MergeRequestStore { _ => None, }) { if existing.approval_event_id == i.approval_event_id + && existing.approved_source_ref == i.current_subject_ref && existing.target_ref_before == i.target_ref_before && existing.target_ref_after == i.target_ref_after + && existing.strategy == i.strategy + && existing.resolution == i.resolution + && existing.merged_by == i.auth.actor() { return Ok(()); } @@ -665,6 +678,7 @@ impl MergeRequestStore { "operation fingerprint mismatch".into(), )); } + self.completion_auth(&i.auth, &i.ticket_id, &mr.repository_id)?; if mr.state != MergeRequestState::Open { return Err(MergeRequestError::Conflict( "Merge Request is not open".into(), @@ -718,8 +732,12 @@ impl MergeRequestStore { _ => None, }) { if existing.approval_event_id == i.approval_event_id + && existing.approved_source_ref == i.current_subject_ref && existing.target_ref_before == i.target_ref_before && existing.target_ref_after == i.target_ref_after + && existing.strategy == i.strategy + && existing.resolution == i.resolution + && existing.merged_by == i.auth.actor() { return Ok(existing.clone()); } @@ -774,6 +792,16 @@ impl MergeRequestStore { WHERE workspace_id=?1 AND ticket_id=?2 AND workflow_state='inprogress'", params![mr.workspace_id, i.ticket_id, i.now.to_rfc3339()], )?; + let released_assignment = transaction.execute( + "DELETE FROM ticket_current_worker_assignments + WHERE workspace_id=?1 AND ticket_id=?2 AND assignment_id=?3", + params![mr.workspace_id, i.ticket_id, i.auth.assignment_id], + )?; + if released_assignment != 1 { + return Err(MergeRequestError::Unauthorized( + "completion assignment changed while closing Ticket".into(), + )); + } let issued_grants = { let mut statement = transaction.prepare( "SELECT request_event_id,subject_ref,capability_token diff --git a/crates/merge-request/tests/store.rs b/crates/merge-request/tests/store.rs index 1f4ec9bf..55113aea 100644 --- a/crates/merge-request/tests/store.rs +++ b/crates/merge-request/tests/store.rs @@ -93,7 +93,7 @@ fn approve(s: &MergeRequestStore, subject: &str, token: &str) -> ReviewEvent { } #[test] fn selectors_thread_and_completion_have_no_revision_or_commit_api() { - let (_d, s) = fixture(); + let (d, s) = fixture(); open(&s); let review = approve(&s, "opaque-source-ref", "token"); let ready = s @@ -122,6 +122,33 @@ fn selectors_thread_and_completion_have_no_revision_or_commit_api() { let mr = s.get("W", "T").unwrap(); assert_eq!(mr.selector_from.as_deref(), Some("work/t")); assert_eq!(mr.state, MergeRequestState::Merged); + let current_assignment: bool = Connection::open(d.path().join("db")) + .unwrap() + .query_row( + "SELECT EXISTS( + SELECT 1 FROM ticket_current_worker_assignments + WHERE workspace_id='W' AND ticket_id='T' + )", + [], + |row| row.get(0), + ) + .unwrap(); + assert!(!current_assignment); + let replayed = s + .complete(CompleteMergeRequest { + ticket_id: "T".into(), + operation_id: "op".into(), + approval_event_id: merged.approval_event_id.clone(), + current_subject_ref: merged.approved_source_ref.clone(), + target_ref_before: merged.target_ref_before.clone(), + target_ref_after: merged.target_ref_after.clone(), + strategy: merged.strategy, + resolution: merged.resolution, + auth: auth(), + now: at(6), + }) + .unwrap(); + assert_eq!(replayed, merged); let json = serde_json::to_string(&mr).unwrap(); for banned in [ "revision_id", diff --git a/crates/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..7dd923a9 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}")] @@ -93,6 +120,26 @@ fn io_err(path: impl Into, source: io::Error) -> TicketError { } } +fn read_ticket_summary_row(row: &rusqlite::Row<'_>) -> rusqlite::Result { + let workflow_state = row.get::<_, String>(7)?; + Ok(TicketSummary { + id: row.get(0)?, + slug: row.get(1)?, + title: row.get(2)?, + status: ExtensibleTicketStatus::from(row.get::<_, String>(3)?.as_str()), + kind: row.get(4)?, + priority: row.get(5)?, + labels: Vec::new(), + readiness: row.get(6)?, + workflow_state: TicketWorkflowState::parse(&workflow_state) + .unwrap_or(TicketWorkflowState::Planning), + workflow_state_explicit: row.get::<_, i64>(8)? != 0, + queued_by: row.get(9)?, + queued_at: row.get(10)?, + updated_at: row.get(11)?, + }) +} + fn sqlite_err(error: impl std::fmt::Display) -> TicketError { TicketError::Sqlite(error.to_string()) } @@ -475,6 +522,37 @@ 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, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub intake_summary: Option, +} + impl TicketTargetEdit { fn validate(&self) -> Result<()> { if let Self::Set { @@ -504,6 +582,116 @@ 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()); + } + 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() + .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, @@ -1404,6 +1592,27 @@ pub struct SqliteTicketListProjection { pub items: Vec, } +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct SqliteTicketListCursor { + pub state_rank: i64, + pub updated_at: Option, + pub ticket_id: String, +} + +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +pub struct SqliteTicketListPageQuery { + pub states: Vec, + pub limit: usize, + pub after: Option, +} + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct SqliteTicketListPage { + pub items: Vec, + pub has_more: bool, + pub next: Option, +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TicketInvalidRecord { pub label: String, @@ -1524,12 +1733,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 +1809,9 @@ pub enum TicketBackendOperation { id: TicketIdOrSlug, change: TicketStateChange, }, - MarkIntakeReady { + MarkReady { id: TicketIdOrSlug, - summary: TicketIntakeSummary, - change: TicketStateChange, + request: TicketMarkReady, }, QueueReady { id: TicketIdOrSlug, @@ -1710,13 +1913,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 +1954,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 +1980,7 @@ impl LocalTicketBackend { Self { root: root.into(), record_language: None, + target_authority: None, } } @@ -1775,6 +1989,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 +2295,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 +2316,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 +2477,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 +2494,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 +2510,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 +2552,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() } @@ -2354,6 +2595,112 @@ impl SqliteTicketBackend { }) } + /// Lists one stable keyset-paginated Workspace summary page. + /// + /// Filtering, ordering, and `limit + 1` are applied by SQLite before the bounded blocker + /// hydration query. The returned cursor is storage data; callers must wrap it in their own + /// opaque, query-bound transport cursor. + pub fn list_workspace_projection_page( + &self, + query: SqliteTicketListPageQuery, + ) -> Result { + self.with_read(|conn| { + let states = serde_json::to_string( + &query + .states + .iter() + .map(|state| state.as_str()) + .collect::>(), + ) + .map_err(|error| TicketError::Sqlite(error.to_string()))?; + let cursor_rank = query.after.as_ref().map(|cursor| cursor.state_rank); + let cursor_updated_at = query + .after + .as_ref() + .and_then(|cursor| cursor.updated_at.clone()); + let cursor_id = query + .after + .as_ref() + .map(|cursor| cursor.ticket_id.as_str()); + let fetch_limit = query.limit.saturating_add(1); + let mut statement = conn + .prepare( + "SELECT ticket_id, slug, title, status, kind, priority, readiness, + workflow_state, workflow_state_explicit, queued_by, queued_at, updated_at + FROM typed_tickets AS ticket + WHERE workspace_id = ?1 + AND (json_array_length(?2) = 0 OR EXISTS ( + SELECT 1 FROM json_each(?2) AS state + WHERE state.value = ticket.workflow_state + )) + AND (?3 IS NULL OR + CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END > ?4 + OR (CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END = ?4 + AND (COALESCE(ticket.updated_at, '') < COALESCE(?5, '') + OR (COALESCE(ticket.updated_at, '') = COALESCE(?5, '') + AND ticket.ticket_id > ?3)))) + ORDER BY CASE ticket.workflow_state + WHEN 'ready' THEN 0 WHEN 'planning' THEN 1 + WHEN 'inprogress' THEN 2 WHEN 'queued' THEN 3 + WHEN 'done' THEN 4 WHEN 'closed' THEN 5 ELSE 6 END ASC, + ticket.updated_at DESC, ticket.ticket_id ASC + LIMIT ?6", + ) + .map_err(sqlite_err)?; + let rows = statement + .query_map( + params![ + self.workspace_id, + states, + cursor_id, + cursor_rank, + cursor_updated_at, + i64::try_from(fetch_limit).unwrap_or(i64::MAX) + ], + read_ticket_summary_row, + ) + .map_err(sqlite_err)?; + let mut summaries = rows + .collect::, _>>() + .map_err(sqlite_err)?; + let has_more = summaries.len() > query.limit; + summaries.truncate(query.limit); + let next = has_more.then(|| { + let summary = summaries.last().expect("non-empty page with continuation"); + SqliteTicketListCursor { + state_rank: match summary.workflow_state { + TicketWorkflowState::Ready => 0, + TicketWorkflowState::Planning => 1, + TicketWorkflowState::InProgress => 2, + TicketWorkflowState::Queued => 3, + TicketWorkflowState::Done => 4, + TicketWorkflowState::Closed => 5, + }, + updated_at: summary.updated_at.clone(), + ticket_id: summary.id.clone(), + } + }); + let blockers = self.list_workspace_blockers(conn, &summaries)?; + Ok(SqliteTicketListPage { + items: summaries + .into_iter() + .map(|summary| SqliteTicketListItem { + relation_blockers: blockers.get(&summary.id).cloned().unwrap_or_default(), + summary, + }) + .collect(), + has_more, + next, + }) + }) + } + fn list_workspace_summaries( &self, conn: &Connection, @@ -3166,6 +3513,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 +3687,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 +3729,120 @@ 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() }) + 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, + &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 +4159,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 +4384,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 +4402,62 @@ 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.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, + 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 +4466,22 @@ 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)?; 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 +4499,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 +5241,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 +6590,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 +7124,92 @@ 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()), + intake_summary: None, + }; + 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(); @@ -6745,6 +7381,86 @@ state: planning assert!(!debug.contains("full-artifact-marker")); } + #[test] + fn sqlite_workspace_projection_page_uses_stable_keyset_and_state_filter() { + let tmp = TempDir::new().unwrap(); + let db_path = tmp.path().join("workspace.db"); + let backend = SqliteTicketBackend::open(&db_path, "workspace-test").unwrap(); + let mut ids = Vec::new(); + for (title, state, updated_at) in [ + ("Newest", TicketWorkflowState::Ready, "2026-08-12T03:00:00Z"), + ("Middle", TicketWorkflowState::Ready, "2026-08-12T02:00:00Z"), + ( + "Oldest", + TicketWorkflowState::Planning, + "2026-08-12T01:00:00Z", + ), + ] { + let mut input = NewTicket::new(title); + input.workflow_state = Some(state); + let ticket = backend.create(input).unwrap(); + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at=?3 WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", ticket.id, updated_at], + ) + .unwrap(); + ids.push(ticket.id); + } + + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at='2026-08-12T05:00:00Z' WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", ids[2]], + ) + .unwrap(); + let combined_lane = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready, TicketWorkflowState::Planning], + limit: 1, + after: None, + }) + .unwrap(); + assert_eq!( + combined_lane.items[0].summary.workflow_state, + TicketWorkflowState::Ready, + "lane pagination order must match the UI's state-primary order", + ); + + let first = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready], + limit: 1, + after: None, + }) + .unwrap(); + assert_eq!(first.items[0].summary.id, ids[0]); + assert!(first.has_more); + + let mut inserted = NewTicket::new("Inserted"); + inserted.workflow_state = Some(TicketWorkflowState::Ready); + let inserted = backend.create(inserted).unwrap(); + Connection::open(&db_path) + .unwrap() + .execute( + "UPDATE typed_tickets SET updated_at='2026-08-12T04:00:00Z' WHERE workspace_id=?1 AND ticket_id=?2", + params!["workspace-test", inserted.id], + ) + .unwrap(); + + let second = backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: vec![TicketWorkflowState::Ready], + limit: 1, + after: first.next, + }) + .unwrap(); + assert_eq!(second.items[0].summary.id, ids[1]); + assert!(!second.has_more); + } + #[test] fn sqlite_workspace_projection_sql_shape_is_constant_for_ticket_count() { let source = include_str!("lib.rs"); @@ -6756,8 +7472,8 @@ state: planning .map(|offset| start + offset) .expect("following method"); let projection_source = &source[start..end]; - assert_eq!(projection_source.matches("self.with_read(").count(), 1); - assert_eq!(projection_source.matches(".prepare(").count(), 2); + assert_eq!(projection_source.matches("self.with_read(").count(), 2); + assert_eq!(projection_source.matches(".prepare(").count(), 3); let item_loop = projection_source .split("items: summaries") .nth(1) @@ -6917,24 +7633,25 @@ 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()), + intake_summary: None, + }, + ) .unwrap(); let current_item = tmp.path().join("tickets").join(&ticket.id).join("item.md"); assert!(current_item.exists()); @@ -7127,6 +7844,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") @@ -7156,7 +7875,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); @@ -7192,41 +7911,60 @@ 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()), + intake_summary: None, + }; - 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()), + intake_summary: None, + }, + ), + 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/sqlite_schema.rs b/crates/ticket/src/sqlite_schema.rs index 4f876294..ebbbf8f2 100644 --- a/crates/ticket/src/sqlite_schema.rs +++ b/crates/ticket/src/sqlite_schema.rs @@ -7,7 +7,7 @@ use crate::{Result, TicketError, sqlite_err}; const MIGRATION_TABLE: &str = "ticket_schema_migrations"; const MAX_SCHEMA_DIAGNOSTICS: usize = 32; -pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 3; +pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 4; #[derive(Clone, Copy)] struct Migration { @@ -32,6 +32,11 @@ const MIGRATIONS: &[Migration] = &[ name: "convert_legacy_reviews_to_comments", apply: retire_legacy_ticket_review_events, }, + Migration { + version: 4, + name: "add_ticket_query_indexes", + apply: add_ticket_query_indexes, + }, ]; #[derive(Clone, Copy)] @@ -507,6 +512,29 @@ fn retire_legacy_ticket_review_events(connection: &Connection) -> Result<()> { .map_err(sqlite_err) } +fn add_ticket_query_indexes(connection: &Connection) -> Result<()> { + connection + .execute_batch( + r#" + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_state_updated + ON typed_tickets(workspace_id, workflow_state, updated_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_updated + ON typed_tickets(workspace_id, updated_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_created + ON typed_tickets(workspace_id, created_at DESC, ticket_id); + CREATE INDEX IF NOT EXISTS typed_tickets_workspace_title + ON typed_tickets(workspace_id, title COLLATE NOCASE, ticket_id); + CREATE INDEX IF NOT EXISTS typed_ticket_events_workspace_kind_ticket + ON typed_ticket_events(workspace_id, kind, ticket_id, event_index); + CREATE INDEX IF NOT EXISTS typed_ticket_relations_workspace_source_kind + ON typed_ticket_relations(workspace_id, ticket_id, kind, target); + CREATE INDEX IF NOT EXISTS typed_ticket_relations_workspace_target_kind + ON typed_ticket_relations(workspace_id, target, kind, ticket_id); + "#, + ) + .map_err(sqlite_err) +} + fn add_column_if_missing( connection: &Connection, table: &str, @@ -841,10 +869,10 @@ mod tests { verify_sqlite_ticket_schema(&connection).unwrap(); let versions = load_applied_migrations(&connection).unwrap(); - assert_eq!(versions.len(), 3); + assert_eq!(versions.len(), 4); assert_eq!( versions.get(&LATEST_SQLITE_TICKET_SCHEMA_VERSION), - Some(&"convert_legacy_reviews_to_comments".to_string()) + Some(&"add_ticket_query_indexes".to_string()) ); } @@ -989,7 +1017,7 @@ mod tests { .to_string() .contains("unsupported Ticket schema migration version 99") ); - assert_eq!(load_applied_migrations(&connection).unwrap().len(), 4); + assert_eq!(load_applied_migrations(&connection).unwrap().len(), 5); } #[test] @@ -1090,7 +1118,7 @@ mod tests { connection.execute("INSERT INTO typed_ticket_events (workspace_id,ticket_id,event_index,kind,author,at,status,heading,body) VALUES ('workspace-1','ticket-1',0,'review','reviewer','2026-08-11T00:00:00Z','approve','Review','legacy evidence')",[]).unwrap(); connection.execute("INSERT INTO typed_ticket_event_attributes (workspace_id,ticket_id,event_index,key,value) VALUES ('workspace-1','ticket-1',0,'result','approve')",[]).unwrap(); connection - .execute("DELETE FROM ticket_schema_migrations WHERE version=3", []) + .execute("DELETE FROM ticket_schema_migrations WHERE version>=3", []) .unwrap(); migrate_sqlite_ticket_schema(&connection).unwrap(); let (kind,status,heading,body):(String,Option,Option,Option)=connection.query_row("SELECT kind,status,heading,body FROM typed_ticket_events WHERE workspace_id='workspace-1' AND ticket_id='ticket-1' AND event_index=0",[],|row|Ok((row.get(0)?,row.get(1)?,row.get(2)?,row.get(3)?))).unwrap(); @@ -1129,6 +1157,6 @@ mod tests { let connection = Connection::open(database).unwrap(); verify_sqlite_ticket_schema(&connection).unwrap(); - assert_eq!(load_applied_migrations(&connection).unwrap().len(), 3); + assert_eq!(load_applied_migrations(&connection).unwrap().len(), 4); } } diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index 410f7556..87ee84b2 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", @@ -68,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", @@ -77,6 +78,7 @@ pub const TICKET_TOOL_NAMES: [&str; 19] = [ "TicketPlan", "TicketDecision", "TicketImplementationReport", + "TicketMarkReady", "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", @@ -99,13 +101,14 @@ 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", "TicketPlan", "TicketDecision", "TicketImplementationReport", + "TicketMarkReady", "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", @@ -132,9 +135,12 @@ 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 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."; @@ -174,6 +180,7 @@ fn base_tool_description(name: &str) -> &'static str { "TicketPlan" => PLAN_DESCRIPTION, "TicketDecision" => DECISION_DESCRIPTION, "TicketImplementationReport" => IMPLEMENTATION_REPORT_DESCRIPTION, + "TicketMarkReady" => MARK_READY_DESCRIPTION, "TicketIntakeReady" => INTAKE_READY_DESCRIPTION, "TicketQueue" => QUEUE_DESCRIPTION, "TicketWorkflowState" => WORKFLOW_STATE_DESCRIPTION, @@ -305,13 +312,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<()> { @@ -558,18 +560,24 @@ struct TicketThreadEventParams { body: String, } +#[derive(Debug, Deserialize, schemars::JsonSchema)] +struct TicketMarkReadyParams { + /// Ticket id. + ticket: String, + /// Optional reason attached to the state_changed event. + #[serde(default)] + reason: Option, +} + #[derive(Debug, Deserialize, schemars::JsonSchema)] struct TicketIntakeReadyParams { /// Ticket id. ticket: String, - /// Concise bounded intake summary to append as a typed intake_summary event. + /// Concise bounded intake summary appended before the ready transition. 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)] @@ -836,6 +844,11 @@ struct TicketImplementationReportTool { backend: TicketToolBackend, } +#[derive(Clone)] +struct TicketMarkReadyTool { + backend: TicketToolBackend, +} + #[derive(Clone)] struct TicketIntakeReadyTool { backend: TicketToolBackend, @@ -899,6 +912,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); @@ -1114,41 +1144,68 @@ impl_ticket_thread_event_tool!( TicketEventKind::ImplementationReport ); +#[async_trait] +impl Tool for TicketMarkReadyTool { + async fn execute( + &self, + input_json: &str, + ctx: llm_engine::tool::ToolExecutionContext, + ) -> Result { + let params: TicketMarkReadyParams = parse_input("TicketMarkReady", input_json)?; + let ticket = self + .backend + .mark_ready( + TicketIdOrSlug::Query(params.ticket.clone()), + TicketMarkReady { + operation_key: format!("ticket-mark-ready:{}", ctx.call_id), + reason: params.reason, + author: None, + intake_summary: None, + }, + ) + .map_err(|error| backend_error("TicketMarkReady", error))?; + Ok(json_output( + format!("Marked ticket {} state ready", 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 TicketIntakeReadyTool { 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 ticket = self + .backend + .mark_ready( TicketIdOrSlug::Query(params.ticket.clone()), - summary, - change, + 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", params.ticket), - json!({ "ticket": params.ticket, "state": "ready", "ok": true }), + 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 + }), )) } } @@ -1726,6 +1783,7 @@ fn input_schema(name: &str) -> Value { "TicketComment" | "TicketPlan" | "TicketDecision" | "TicketImplementationReport" => { 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" => { @@ -1774,6 +1832,7 @@ impl_from_backend!(TicketCommentTool); 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); @@ -1801,6 +1860,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()), @@ -1826,8 +1886,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,6 +1955,7 @@ mod tests { "TicketPlan", "TicketDecision", "TicketImplementationReport", + "TicketMarkReady", "TicketIntakeReady", "TicketQueue", "TicketWorkflowState", @@ -2460,16 +2539,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 +2592,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() @@ -2538,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(); @@ -2661,7 +2773,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 +2796,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 +2819,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 +2847,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 +2991,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..e952a46b 100644 --- a/crates/worker/src/controller.rs +++ b/crates/worker/src/controller.rs @@ -769,6 +769,21 @@ where ), ); } + if feature_config.merge_request.any() { + let workspace_client = worker.workspace_client_handle(); + if !workspace_client.is_available() || workspace_client.workspace_id().is_none() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "Merge Request tools require Backend Workspace API authority", + )); + } + feature_registry.add_module( + crate::feature::builtin::merge_request::MergeRequestFeature::new( + workspace_client, + feature_config.merge_request, + ), + ); + } if feature_config.manage_workdir.enabled { // Workdir lifecycle is Workspace control-plane authority. The Worker // receives only the injected WorkspaceClient and never Runtime URLs, diff --git a/crates/worker/src/feature/builtin/merge_request.rs b/crates/worker/src/feature/builtin/merge_request.rs index b2dd5c57..832789d2 100644 --- a/crates/worker/src/feature/builtin/merge_request.rs +++ b/crates/worker/src/feature/builtin/merge_request.rs @@ -1,19 +1,41 @@ -use crate::feature::ToolDefinition; +use crate::feature::{ + FeatureDescriptor, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution, + FeatureInstructionDeclaration, FeatureInstructionId, FeatureModule, ToolContribution, + ToolDeclaration, ToolDefinition, +}; use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod}; use async_trait::async_trait; use llm_engine::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput}; +use manifest::MergeRequestFeatureConfig; use schemars::JsonSchema; use serde::Deserialize; use serde_json::json; use std::sync::Arc; -pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[ - "MergeRequestShow", - "MergeRequestReadinessCheck", - "MergeRequestOpen", - "MergeRequestComplete", +pub const FEATURE_ID: &str = "merge_request"; +const FEATURE_NAME: &str = "Merge Request tools"; +const FEATURE_DESCRIPTION: &str = + "Operation-specific Merge Request workflow tools over Workspace authority."; +const FEATURE_INSTRUCTION_ID: &str = "merge_request.workflow"; +pub const FEATURE_PROMPT_REF: &str = "common.merge_request"; + +fn workflow_instruction() -> FeatureInstructionDeclaration { + FeatureInstructionDeclaration::new( + FeatureInstructionId::builtin(FEATURE_INSTRUCTION_ID), + FEATURE_PROMPT_REF, + "Operation-specific Merge Request workflow guidance", + ) + .expect("static Merge Request workflow instruction declaration is valid") +} + +const ALL_KINDS: [Kind; 5] = [ + Kind::Show, + Kind::Open, + Kind::Review, + Kind::Readiness, + Kind::Complete, ]; -pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit"; + #[derive(Clone, Copy)] enum Kind { Show, @@ -89,13 +111,23 @@ struct ReviewFindingInput { body: String, } impl Kind { + fn enabled(self, config: MergeRequestFeatureConfig) -> bool { + match self { + Self::Show => config.show, + Self::Open => config.open, + Self::Review => config.review, + Self::Readiness => config.readiness_check, + Self::Complete => config.complete, + } + } + fn name(self) -> &'static str { match self { Self::Show => "MergeRequestShow", Self::Readiness => "MergeRequestReadinessCheck", Self::Open => "MergeRequestOpen", Self::Complete => "MergeRequestComplete", - Self::Review => "MergeRequestReviewSubmit", + Self::Review => "MergeRequestReview", } } fn schema(self) -> serde_json::Value { @@ -218,24 +250,53 @@ fn definition(client: Arc, kind: Kind) -> ToolDefinition { ) }) } -pub fn common_tools(c: Arc) -> Vec { - vec![ - definition(c.clone(), Kind::Show), - definition(c.clone(), Kind::Readiness), - definition(c.clone(), Kind::Open), - definition(c, Kind::Complete), - ] +pub struct MergeRequestFeature { + client: Arc, + config: MergeRequestFeatureConfig, } -pub fn reviewer_tools(c: Arc) -> Vec { - if c.reviewer_context().is_some() { - vec![ - definition(c.clone(), Kind::Show), - definition(c, Kind::Review), - ] - } else { - vec![] + +impl MergeRequestFeature { + pub fn new(client: Arc, config: MergeRequestFeatureConfig) -> Self { + Self { client, config } + } + + fn kinds(&self) -> impl Iterator + '_ { + ALL_KINDS + .into_iter() + .filter(|kind| kind.enabled(self.config)) } } + +impl FeatureModule for MergeRequestFeature { + fn descriptor(&self) -> FeatureDescriptor { + let mut descriptor = FeatureDescriptor::builtin(FEATURE_ID, FEATURE_NAME) + .with_description(FEATURE_DESCRIPTION); + if self.config.any() { + descriptor = descriptor.with_instruction(workflow_instruction()); + } + for kind in self.kinds() { + descriptor = descriptor.with_tool(ToolDeclaration::new( + kind.name(), + description(kind.name()).unwrap_or("Merge Request operation."), + )); + } + descriptor + } + + fn install(&self, ctx: &mut FeatureInstallContext<'_>) -> Result<(), FeatureInstallError> { + if self.config.any() { + ctx.instructions() + .register(FeatureInstructionContribution::new(workflow_instruction()))?; + } + let mut tools = ctx.tools(); + for kind in self.kinds() { + let definition = definition(self.client.clone(), kind); + tools.register(ToolContribution::new(kind.name(), definition))?; + } + Ok(()) + } +} + pub fn description(n: &str) -> Option<&'static str> { match n { "MergeRequestShow" => Some("Read the selector-based Merge Request and append-only thread."), @@ -248,7 +309,7 @@ pub fn description(n: &str) -> Option<&'static str> { "MergeRequestComplete" => { Some("Complete using an approved review event and final target-ref evidence.") } - "MergeRequestReviewSubmit" => { + "MergeRequestReview" => { Some("Submit the injected Reviewer capability result for its captured subject ref.") } _ => None, @@ -257,6 +318,72 @@ pub fn description(n: &str) -> Option<&'static str> { #[cfg(test)] mod tests { use super::*; + use crate::feature::FeatureRegistryBuilder; + use crate::hook::HookRegistryBuilder; + use crate::worker::TestWorkspaceHttpClient; + + fn install(config: MergeRequestFeatureConfig) -> (Vec, Vec) { + let client: Arc = + Arc::new(TestWorkspaceHttpClient::new("workspace", "http://unused")); + let mut pending_tools = Vec::new(); + let mut hook_builder = HookRegistryBuilder::default(); + let report = FeatureRegistryBuilder::new() + .with_module(MergeRequestFeature::new(client, config)) + .install_into_pending(&mut pending_tools, &mut hook_builder); + assert!(!report.has_errors(), "{}", report.error_message()); + ( + report.installed_tool_names(), + report + .installed_instruction_contributions() + .into_iter() + .map(|instruction| instruction.prompt_ref) + .collect(), + ) + } + + fn tool_names(config: MergeRequestFeatureConfig) -> Vec { + install(config).0 + } + + #[test] + fn flags_define_the_exact_registered_tool_surface() { + let coder = MergeRequestFeatureConfig { + show: true, + open: true, + ..Default::default() + }; + assert_eq!(tool_names(coder), ["MergeRequestShow", "MergeRequestOpen"]); + + let reviewer = MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + }; + assert_eq!( + tool_names(reviewer), + ["MergeRequestShow", "MergeRequestReview"] + ); + + let orchestrator = MergeRequestFeatureConfig { + show: true, + readiness_check: true, + complete: true, + ..Default::default() + }; + assert_eq!( + tool_names(orchestrator), + [ + "MergeRequestShow", + "MergeRequestReadinessCheck", + "MergeRequestComplete" + ] + ); + assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]); + let unspecified = install(MergeRequestFeatureConfig::default()); + assert!(unspecified.0.is_empty()); + assert!(unspecified.1.is_empty()); + } + #[test] fn schemas_hide_revision_and_commit_authority() { let schemas = [ @@ -276,6 +403,5 @@ mod tests { assert!(!j.contains(banned), "{banned} in {j}") } } - assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRequestReview")); } } diff --git a/crates/worker/src/feature/builtin/ticket.rs b/crates/worker/src/feature/builtin/ticket.rs index a44e37ca..502e34d7 100644 --- a/crates/worker/src/feature/builtin/ticket.rs +++ b/crates/worker/src/feature/builtin/ticket.rs @@ -24,7 +24,6 @@ use ticket::{ tool::{TICKET_TOOL_NAMES, TicketToolBackend, ticket_tool_description, ticket_tools}, }; -use super::merge_request; use crate::feature::{ FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError, FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId, @@ -377,6 +376,7 @@ const READ_ONLY_TOOL_NAMES: &[&str] = &["QueryTicket", "ShowTicket"]; const AUTHORING_TOOL_NAMES: &[&str] = &[ "TicketCreate", "TicketEditItem", + "TicketMarkReady", "TicketQueue", "TicketClose", "TicketRelationRecord", @@ -394,6 +394,7 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[ "QueryTicket", "ShowTicket", "TicketComment", + "TicketMarkReady", "TicketQueue", "TicketClose", "TicketRelationRecord", @@ -584,22 +585,6 @@ impl FeatureModule for TicketFeature { ticket_tool_description(name, self.record_language.as_deref()), )); } - if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend { - let names: Vec<&str> = if client.reviewer_context().is_some() { - vec![ - "MergeRequestShow", - merge_request::MERGE_REQUEST_REVIEW_TOOL_NAME, - ] - } else { - merge_request::MERGE_REQUEST_COMMON_TOOL_NAMES.to_vec() - }; - for name in names { - descriptor = descriptor.with_tool(ToolDeclaration::new( - name, - merge_request::description(name).unwrap_or("Merge Request operation."), - )); - } - } descriptor } @@ -657,17 +642,6 @@ impl FeatureModule for TicketFeature { }; tools.register(ToolContribution::new(name, definition))?; } - if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend { - let definitions = if client.reviewer_context().is_some() { - merge_request::reviewer_tools(client.clone()) - } else { - merge_request::common_tools(client.clone()) - }; - for definition in definitions { - let (meta, _) = definition(); - tools.register(ToolContribution::new(meta.name.clone(), definition))?; - } - } Ok(()) } } @@ -890,16 +864,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, @@ -1115,22 +1088,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<()> { @@ -1299,7 +1265,7 @@ 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(), 10); assert!( tool_names.len() < 13, "authoring catalog must stay below the prior broad catalog" @@ -1516,6 +1482,7 @@ language = "Japanese" 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 == "TicketIntakeReady")); assert!(!installed.iter().any(|tool| *tool == "TicketWorkflowState")); assert!( diff --git a/crates/worker/src/worker.rs b/crates/worker/src/worker.rs index 509f957f..fa0f6596 100644 --- a/crates/worker/src/worker.rs +++ b/crates/worker/src/worker.rs @@ -5890,6 +5890,13 @@ model_id = "claude-sonnet-4-20250514" [engine] instruction = "saved" +[feature.merge_request] +show = true +open = false +review = true +readiness_check = false +complete = false + [[scope.allow]] target = "/snapshot/workspace" permission = "read" @@ -5912,6 +5919,13 @@ model_id = "claude-sonnet-4-20250514" [engine] instruction = "current" +[feature.merge_request] +show = true +open = true +review = true +readiness_check = true +complete = true + [[scope.allow]] target = "/current/workspace" permission = "write" @@ -5931,6 +5945,14 @@ permission = "write" .unwrap(); assert_eq!(restored.engine.instruction, "saved"); + assert_eq!( + restored.feature.merge_request, + manifest::MergeRequestFeatureConfig { + show: true, + review: true, + ..Default::default() + } + ); assert_eq!(restored.scope.allow.len(), 1); assert_eq!( restored.scope.allow[0].target, diff --git a/crates/workspace-server/src/authority.rs b/crates/workspace-server/src/authority.rs index c0c09566..e5f2b82c 100644 --- a/crates/workspace-server/src/authority.rs +++ b/crates/workspace-server/src/authority.rs @@ -6,9 +6,11 @@ use merge_request::{ ReviewDecision, }; use project_record::{allocate_record_id, unix_epoch_millis_now}; +use rusqlite::{params_from_iter, types::Value as SqlValue}; use ticket::{ - SqliteTicketBackend, TicketBackend, TicketEvent, TicketIdOrSlug, TicketWorkspaceActionPriority, + SqliteTicketBackend, SqliteTicketListCursor, SqliteTicketListItem, SqliteTicketListPageQuery, + TicketBackend, TicketEvent, TicketIdOrSlug, TicketWorkflowState, TicketWorkspaceActionPriority, project_ticket_workspace_item, }; @@ -17,8 +19,9 @@ use crate::records::{ ObjectiveQueryItem, ObjectiveQueryRequest, ObjectiveQueryResponse, ObjectiveResourceSummary, ObjectiveShowRequest, ObjectiveSummary, ProjectRecordList, QueryPage, TicketAssignmentSummary, TicketDetail, TicketEventDetail, TicketEvidenceEvent, TicketEvidenceSummary, - TicketMergeRequestSummary, TicketQueryItem, TicketQueryRequest, TicketQueryResponse, - TicketShowRequest, TicketSummary, summarize_body, truncate_body, validate_project_id, + TicketListPageRequest, TicketMergeRequestSummary, TicketQueryItem, TicketQueryRequest, + TicketQueryResponse, TicketShowRequest, TicketSummary, TicketSummaryPage, summarize_body, + truncate_body, validate_project_id, }; use crate::store::{ ControlPlaneStore, MemoryDocumentRecord, MemoryStagingRecord, MemoryStagingResolutionRecord, @@ -43,6 +46,7 @@ impl WorkspaceAuthority for T where T: ObjectiveAuthority + TicketAuthority + pub trait TicketAuthority { fn list_tickets(&self, limit: usize) -> Result>; + fn list_ticket_page(&self, request: TicketListPageRequest) -> Result; fn query_tickets(&self, query: TicketQueryRequest) -> Result; fn ticket(&self, id: &str) -> Result; fn show_ticket(&self, id: &str, query: TicketShowRequest) -> Result; @@ -339,11 +343,358 @@ impl SqliteWorkspaceAuthority { }) } + fn query_ticket_candidate_ids( + &self, + query: &TicketQueryRequest, + sort: TicketQuerySort, + after: Option<&(String, String)>, + limit: usize, + ) -> Result> { + let mut values = vec![SqlValue::Text(self.workspace_id.clone())]; + let mut predicates = vec!["t.workspace_id=?1".to_string()]; + let mut bind = |value: SqlValue| { + values.push(value); + format!("?{}", values.len()) + }; + if !query.states.is_empty() { + let states = query + .states + .iter() + .map(|state| bind(SqlValue::Text(state.clone()))) + .collect::>(); + predicates.push(format!("t.workflow_state IN ({})", states.join(","))); + } + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + predicates.push(format!( + "(lower(t.title) LIKE {p} OR lower(t.body) LIKE {p} OR EXISTS ( + SELECT 1 FROM typed_ticket_events e + WHERE e.workspace_id=t.workspace_id AND e.ticket_id=t.ticket_id + AND lower(e.body) LIKE {p}))", + p = pattern + )); + } + if let Some(value) = &query.updated_after { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("COALESCE(t.updated_at,'')>{value}")); + } + if let Some(value) = &query.updated_before { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("COALESCE(t.updated_at,'')<{value}")); + } + if let Some(value) = &query.linked_objective_id { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("EXISTS (SELECT 1 FROM objective_ticket_links link WHERE link.workspace_id=t.workspace_id AND link.ticket_id=t.ticket_id AND link.objective_id={value})")); + } + if query.related_ticket_id.is_some() || query.relation_kind.is_some() { + let related = query + .related_ticket_id + .as_ref() + .map(|value| bind(SqlValue::Text(value.clone()))); + let kind = query + .relation_kind + .as_ref() + .map(|value| bind(SqlValue::Text(value.clone()))); + let related = related + .map(|value| format!("AND ((r.ticket_id=t.ticket_id AND r.target={value}) OR (r.target=t.ticket_id AND r.ticket_id={value}))")) + .unwrap_or_else(|| { + "AND (r.ticket_id=t.ticket_id OR r.target=t.ticket_id)".to_string() + }); + let kind = kind + .map(|value| format!("AND r.kind={value}")) + .unwrap_or_default(); + predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_relations r WHERE r.workspace_id=t.workspace_id {related} {kind})")); + } + let blocker = "EXISTS (SELECT 1 FROM typed_ticket_relations relation + JOIN typed_tickets blocker ON blocker.workspace_id=relation.workspace_id + AND blocker.ticket_id=CASE WHEN relation.ticket_id=t.ticket_id THEN relation.target ELSE relation.ticket_id END + WHERE relation.workspace_id=t.workspace_id + AND ((relation.ticket_id=t.ticket_id AND relation.kind='depends_on') + OR (relation.target=t.ticket_id AND relation.kind='blocks')) + AND blocker.workflow_state NOT IN ('done','closed'))"; + let active_blocker = "EXISTS (SELECT 1 FROM typed_ticket_relations relation + JOIN typed_tickets blocker ON blocker.workspace_id=relation.workspace_id + AND blocker.ticket_id=CASE WHEN relation.ticket_id=t.ticket_id THEN relation.target ELSE relation.ticket_id END + WHERE relation.workspace_id=t.workspace_id + AND ((relation.ticket_id=t.ticket_id AND relation.kind='depends_on') + OR (relation.target=t.ticket_id AND relation.kind='blocks')) + AND blocker.workflow_state NOT IN ('queued','inprogress','done','closed'))"; + let report_index = "(SELECT max(event.event_index) FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind='implementation_report')"; + let edit_index = "(SELECT max(event.event_index) FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind='item_edit')"; + let current_report = format!( + "({report_index} IS NOT NULL AND ({edit_index} IS NULL OR {report_index}>={edit_index}))" + ); + let merge_request_id = "(SELECT relation.merge_request_id FROM merge_request_ticket_relations relation + JOIN merge_requests request ON request.workspace_id=relation.workspace_id AND request.merge_request_id=relation.merge_request_id + WHERE relation.workspace_id=t.workspace_id AND relation.ticket_id=t.ticket_id + ORDER BY CASE WHEN request.state='open' THEN 0 ELSE 1 END, request.created_at DESC LIMIT 1)"; + let review_subject = format!( + "(SELECT json_extract(requested.payload_json,'$.subject_ref') + FROM merge_request_thread_events requested + WHERE requested.workspace_id=t.workspace_id + AND requested.merge_request_id={merge_request_id} + AND requested.kind='review_requested' + ORDER BY requested.sequence DESC LIMIT 1)" + ); + let review_decision = format!( + "(SELECT json_extract(event.payload_json,'$.decision') + FROM merge_request_thread_events event + WHERE event.workspace_id=t.workspace_id + AND event.merge_request_id={merge_request_id} AND event.kind='review' + AND json_extract(event.payload_json,'$.subject_ref')={review_subject} + AND NOT EXISTS (SELECT 1 FROM merge_request_thread_events revoked + WHERE revoked.workspace_id=event.workspace_id + AND revoked.merge_request_id=event.merge_request_id + AND revoked.kind='review_revoked' + AND json_extract(revoked.payload_json,'$.review_event_id')=event.event_id) + ORDER BY event.sequence DESC LIMIT 1)" + ); + let review_status = format!( + "CASE WHEN {merge_request_id} IS NULL THEN 'none' WHEN {review_decision}='approve' THEN 'approved' WHEN {review_decision}='request_changes' THEN 'request_changes' ELSE 'pending' END" + ); + let has_commit = format!( + "({review_subject} IS NOT NULL OR EXISTS (SELECT 1 FROM typed_ticket_event_references reference WHERE reference.workspace_id=t.workspace_id AND reference.ticket_id=t.ticket_id AND reference.kind='commit'))" + ); + if !query.event_kinds.is_empty() { + let event_kinds = query + .event_kinds + .iter() + .map(|event_kind| bind(SqlValue::Text(event_kind.clone()))) + .collect::>(); + predicates.push(format!("EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND event.kind IN ({}))", event_kinds.join(","))); + } + for evidence in &query.evidence { + predicates.push(match evidence.as_str() { + "implementation_report" => format!("{report_index} IS NOT NULL"), + "implementation_report_after_rescope" => current_report.clone(), + "merge_request" => format!("{merge_request_id} IS NOT NULL"), + "commit" => has_commit.clone(), + "approved_review" => format!("{review_status}='approved'"), + other => { + return Err(Error::InvalidRecordId(format!( + "unsupported evidence filter `{other}`" + ))); + } + }); + } + if let Some(status) = &query.review_status { + let status = if matches!(status.as_str(), "unresolved_changes" | "changes_requested") { + "request_changes" + } else { + status.as_str() + }; + let status = bind(SqlValue::Text(status.to_string())); + predicates.push(format!("{review_status}={status}")); + } + for attention in &query.attention { + predicates.push(match attention.as_str() { + "done_not_closed" => "t.workflow_state='done'".to_string(), + "implementation_report_not_closed" => { + format!("{report_index} IS NOT NULL AND t.workflow_state!='closed'") + } + "report_after_rescope" => current_report.clone(), + "unresolved_review" | "unresolved_changes" => { + format!("{review_status}='request_changes'") + } + "missing_commit" => format!("NOT {has_commit}"), + "blocked" => blocker.to_string(), + "unblocked" => format!("NOT {blocker}"), + "ready" => format!("t.workflow_state='ready' AND NOT {blocker}"), + "awaiting_review" => format!("{review_status}='pending'"), + "stale_after_rescope" => { + format!("{report_index} IS NOT NULL AND NOT {current_report}") + } + "missing_evidence" => format!( + "NOT ({current_report} AND {has_commit} AND {review_status}='approved')" + ), + other => { + return Err(Error::InvalidRecordId(format!( + "unsupported attention filter `{other}`" + ))); + } + }); + } + let rank_expression = match sort { + TicketQuerySort::Priority => format!( + "CASE WHEN t.workflow_state='ready' AND NOT {active_blocker} THEN 0 WHEN t.workflow_state IN ('queued','inprogress') THEN 1 ELSE 2 END" + ), + TicketQuerySort::Relevance => { + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + format!( + "CASE WHEN lower(t.title) LIKE {pattern} THEN 0 WHEN lower(t.body) LIKE {pattern} THEN 1 WHEN EXISTS (SELECT 1 FROM typed_ticket_events event WHERE event.workspace_id=t.workspace_id AND event.ticket_id=t.ticket_id AND lower(event.body) LIKE {pattern}) THEN 2 ELSE 3 END" + ) + } else { + "3".to_string() + } + } + _ => "0".to_string(), + }; + if let Some((key, id)) = after { + match sort { + TicketQuerySort::UpdatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("(COALESCE(t.updated_at,'')<{key} OR (COALESCE(t.updated_at,'')={key} AND t.ticket_id>{id}))")); + } + TicketQuerySort::CreatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("(COALESCE(t.created_at,'')<{key} OR (COALESCE(t.created_at,'')={key} AND t.ticket_id>{id}))")); + } + TicketQuerySort::Title => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(lower(t.title)>{key} OR (lower(t.title)={key} AND t.ticket_id>{id}))" + )); + } + TicketQuerySort::Priority | TicketQuerySort::Relevance => { + let (rank, updated_at) = key.split_once('|').unwrap_or(("9", "")); + let rank = bind(SqlValue::Integer(rank.parse::().unwrap_or(9))); + let updated_at = bind(SqlValue::Text(updated_at.to_string())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("({rank_expression}>{rank} OR ({rank_expression}={rank} AND (COALESCE(t.updated_at,'')<{updated_at} OR (COALESCE(t.updated_at,'')={updated_at} AND t.ticket_id>{id}))))")); + } + } + } + let order = match sort { + TicketQuerySort::Title => "t.title COLLATE NOCASE ASC, t.ticket_id ASC".to_string(), + TicketQuerySort::CreatedDesc => "t.created_at DESC, t.ticket_id ASC".to_string(), + TicketQuerySort::UpdatedDesc => "t.updated_at DESC, t.ticket_id ASC".to_string(), + TicketQuerySort::Priority | TicketQuerySort::Relevance => { + format!("{rank_expression} ASC, t.updated_at DESC, t.ticket_id ASC") + } + }; + let limit = bind(SqlValue::Integer(i64::try_from(limit).unwrap_or(i64::MAX))); + let sql = format!( + "SELECT t.ticket_id FROM typed_tickets t WHERE {} ORDER BY {order} LIMIT {limit}", + predicates.join(" AND ") + ); + self.store.with_conn(|connection| { + let mut statement = connection.prepare(&sql)?; + let rows = statement.query_map(params_from_iter(values.iter()), |row| row.get(0))?; + Ok(rows.collect::, _>>()?) + }) + } + + fn query_objective_candidate_ids( + &self, + query: &ObjectiveQueryRequest, + sort: ObjectiveQuerySort, + after: Option<&(String, String)>, + limit: usize, + ) -> Result> { + let mut values = vec![SqlValue::Text(self.workspace_id.clone())]; + let mut predicates = vec!["o.workspace_id=?1".to_string()]; + let mut bind = |value: SqlValue| { + values.push(value); + format!("?{}", values.len()) + }; + if !query.states.is_empty() { + let states = query + .states + .iter() + .map(|state| bind(SqlValue::Text(state.clone()))) + .collect::>(); + predicates.push(format!("o.state IN ({})", states.join(","))); + } + if let Some(text) = query.query.as_deref().filter(|text| !text.is_empty()) { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + predicates.push(format!( + "(lower(o.title) LIKE {pattern} OR lower(o.body_md) LIKE {pattern})" + )); + } + if let Some(value) = &query.updated_after { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("o.updated_at>{value}")); + } + if let Some(value) = &query.updated_before { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("o.updated_at<{value}")); + } + if let Some(value) = &query.linked_ticket_id { + let value = bind(SqlValue::Text(value.clone())); + predicates.push(format!("EXISTS (SELECT 1 FROM objective_ticket_links link WHERE link.workspace_id=o.workspace_id AND link.objective_id=o.objective_id AND link.ticket_id={value})")); + } + let relevance_rank = if let Some(text) = + query.query.as_deref().filter(|text| !text.is_empty()) + { + let pattern = bind(SqlValue::Text(format!("%{}%", text.to_lowercase()))); + format!( + "CASE WHEN lower(o.title) LIKE {pattern} THEN 0 WHEN lower(o.body_md) LIKE {pattern} THEN 1 ELSE 2 END" + ) + } else { + "2".to_string() + }; + if let Some((key, id)) = after { + match sort { + ObjectiveQuerySort::UpdatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(o.updated_at<{key} OR (o.updated_at={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::CreatedDesc => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(o.created_at<{key} OR (o.created_at={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::Title => { + let key = bind(SqlValue::Text(key.clone())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!( + "(lower(o.title)>{key} OR (lower(o.title)={key} AND o.objective_id>{id}))" + )); + } + ObjectiveQuerySort::Relevance => { + let (rank, updated_at) = key.split_once('|').unwrap_or(("9", "")); + let rank = bind(SqlValue::Integer(rank.parse::().unwrap_or(9))); + let updated_at = bind(SqlValue::Text(updated_at.to_string())); + let id = bind(SqlValue::Text(id.clone())); + predicates.push(format!("({relevance_rank}>{rank} OR ({relevance_rank}={rank} AND (o.updated_at<{updated_at} OR (o.updated_at={updated_at} AND o.objective_id>{id}))))")); + } + } + } + let order = match sort { + ObjectiveQuerySort::Title => { + "o.title COLLATE NOCASE ASC, o.objective_id ASC".to_string() + } + ObjectiveQuerySort::CreatedDesc => "o.created_at DESC, o.objective_id ASC".to_string(), + ObjectiveQuerySort::UpdatedDesc => "o.updated_at DESC, o.objective_id ASC".to_string(), + ObjectiveQuerySort::Relevance => { + format!("{relevance_rank} ASC, o.updated_at DESC, o.objective_id ASC") + } + }; + let limit = bind(SqlValue::Integer(i64::try_from(limit).unwrap_or(i64::MAX))); + let sql = format!( + "SELECT o.objective_id FROM objectives o WHERE {} ORDER BY {order} LIMIT {limit}", + predicates.join(" AND ") + ); + self.store.with_conn(|connection| { + let mut statement = connection.prepare(&sql)?; + let rows = statement.query_map(params_from_iter(values.iter()), |row| row.get(0))?; + Ok(rows.collect::, _>>()?) + }) + } + fn read_ticket_detail(&self, id: &str, request: TicketShowRequest) -> Result { validate_project_id(id)?; let ticket = self .ticket_backend .show(TicketIdOrSlug::Id(id.to_string()))?; + self.ticket_detail_from_ticket(ticket, request) + } + + fn ticket_detail_from_ticket( + &self, + ticket: ticket::Ticket, + request: TicketShowRequest, + ) -> Result { + let id = ticket.meta.id.as_str(); let (body, body_truncated) = truncate_body(ticket.document.body.as_str(), DETAIL_BODY_LIMIT); let event_limit = request @@ -490,42 +841,100 @@ impl TicketAuthority for SqliteWorkspaceAuthority { }) } + fn list_ticket_page(&self, request: TicketListPageRequest) -> Result { + let limit = request.limit.unwrap_or(30).clamp(1, 100); + let mut states = request.states; + states.sort(); + states.dedup(); + let parsed_states = states + .iter() + .map(|state| { + TicketWorkflowState::parse(state).ok_or_else(|| { + Error::InvalidRecordId(format!("unsupported ticket state `{state}`")) + }) + }) + .collect::>>()?; + let fingerprint = format!( + "ticket-summary:v2:sort=priority:states={}", + states.join(",") + ); + let after = request + .cursor + .as_deref() + .map(|cursor| parse_ticket_summary_cursor(cursor, &fingerprint)) + .transpose()?; + let page = + self.ticket_backend + .list_workspace_projection_page(SqliteTicketListPageQuery { + states: parsed_states, + limit, + after, + })?; + let items = page + .items + .into_iter() + .map(ticket_summary_from_sqlite_item) + .collect::>(); + let next_cursor = page + .next + .map(|position| make_ticket_summary_cursor(&fingerprint, position)); + Ok(TicketSummaryPage { + page: QueryPage { + limit, + returned: items.len(), + has_more: page.has_more, + next_cursor, + sort: "priority".to_string(), + source_limit: None, + source_truncated: false, + }, + items, + invalid_records: Vec::new(), + record_authority: RECORD_SOURCE_WORKSPACE_SQLITE.to_string(), + }) + } + fn query_tickets(&self, query: TicketQueryRequest) -> Result { validate_ticket_query(&query)?; let limit = query.limit.unwrap_or(50).clamp(1, 100); let sort = normalize_ticket_sort(query.sort.as_deref(), query.query.is_some())?; + let fingerprint = ticket_query_fingerprint(&query, sort); let cursor = query .cursor .as_deref() - .map(parse_query_cursor) + .map(|cursor| parse_bound_query_cursor(cursor, &fingerprint)) .transpose()?; - let mut summaries = self.list_tickets(1_001)?.items; - let source_truncated = summaries.len() > 1_000; - summaries.truncate(1_000); + let candidate_limit = limit.saturating_add(1); + let candidate_ids = + self.query_ticket_candidate_ids(&query, sort, cursor.as_ref(), candidate_limit)?; + let source_truncated = candidate_ids.len() == candidate_limit; let mut items = Vec::new(); - for summary in summaries { - let detail = self.read_ticket_detail( - &summary.id, + for ticket_id in candidate_ids { + let authoritative = self + .ticket_backend + .show(TicketIdOrSlug::Id(ticket_id.clone()))?; + let summary = ticket_summary_from_ticket(&authoritative); + let authoritative_body = authoritative.document.body.clone(); + let authoritative_events = authoritative.events.clone(); + let detail = self.ticket_detail_from_ticket( + authoritative, TicketShowRequest { event_limit: Some(TICKET_EVENT_LIMIT), event_cursor: None, }, )?; - let authoritative = self - .ticket_backend - .show(TicketIdOrSlug::Id(summary.id.clone()))?; if ticket_matches_query( &summary, &detail, - authoritative.document.body.as_str(), - &authoritative.events, + authoritative_body.as_str(), + &authoritative_events, &query, ) { items.push(ticket_query_item( summary, &detail, - authoritative.document.body.as_str(), - &authoritative.events, + authoritative_body.as_str(), + &authoritative_events, &query, )); } @@ -537,7 +946,11 @@ impl TicketAuthority for SqliteWorkspaceAuthority { let has_more = items.len() > limit; items.truncate(limit); let next_cursor = has_more - .then(|| items.last().map(|item| make_ticket_cursor(item, sort))) + .then(|| { + items + .last() + .map(|item| make_ticket_cursor(item, sort, &fingerprint)) + }) .flatten(); Ok(TicketQueryResponse { page: QueryPage { @@ -546,7 +959,7 @@ impl TicketAuthority for SqliteWorkspaceAuthority { has_more, next_cursor, sort: sort.to_string(), - source_limit: Some(1_000), + source_limit: Some(candidate_limit), source_truncated, }, items, @@ -598,33 +1011,36 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { )?; let limit = query.limit.unwrap_or(50).clamp(1, 100); let sort = normalize_objective_sort(query.sort.as_deref(), query.query.is_some())?; + let fingerprint = objective_query_fingerprint(&query, sort); let cursor = query .cursor .as_deref() - .map(parse_query_cursor) + .map(|cursor| parse_bound_query_cursor(cursor, &fingerprint)) .transpose()?; - let mut objectives = self.list_objectives(1_001)?.items; - let source_truncated = objectives.len() > 1_000; - objectives.truncate(1_000); + let candidate_limit = limit.saturating_add(1); + let objective_ids = + self.query_objective_candidate_ids(&query, sort, cursor.as_ref(), candidate_limit)?; + let source_truncated = objective_ids.len() == candidate_limit; let mut items = Vec::new(); - for objective in objectives { - let body_md = self.objective_record(&objective.id)?.body_md; - if !objective_matches_query(&objective, &body_md, &query) { - continue; - } + for objective_id in objective_ids { + let record = self.objective_record(&objective_id)?; let linked_tickets = self .store - .list_objective_ticket_links(&self.workspace_id, &objective.id)? + .list_objective_ticket_links(&self.workspace_id, &objective_id)? .into_iter() .map(|link| link.ticket_id) .collect::>(); - if query - .linked_ticket_id - .as_ref() - .is_some_and(|id| !linked_tickets.iter().any(|ticket_id| ticket_id == id)) - { - continue; - } + let body_md = record.body_md.clone(); + let objective = ObjectiveSummary { + id: record.objective_id, + title: record.title, + state: record.state, + created_at: Some(record.created_at), + updated_at: Some(record.updated_at), + summary: summarize_body(&body_md), + linked_tickets: linked_tickets.clone(), + record_source: RECORD_SOURCE_WORKSPACE_SQLITE.to_string(), + }; items.push(objective_query_item( objective, linked_tickets, @@ -639,7 +1055,11 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { let has_more = items.len() > limit; items.truncate(limit); let next_cursor = has_more - .then(|| items.last().map(|item| make_objective_cursor(item, sort))) + .then(|| { + items + .last() + .map(|item| make_objective_cursor(item, sort, &fingerprint)) + }) .flatten(); Ok(ObjectiveQueryResponse { page: QueryPage { @@ -648,7 +1068,7 @@ impl ObjectiveAuthority for SqliteWorkspaceAuthority { has_more, next_cursor, sort: sort.to_string(), - source_limit: Some(1_000), + source_limit: Some(candidate_limit), source_truncated, }, items, @@ -1358,6 +1778,35 @@ fn parse_offset_cursor(cursor: &str, field: &str) -> Result { .map_err(|_| Error::InvalidRecordId(format!("invalid {field}"))) } +fn make_bound_query_cursor(fingerprint: &str, key: &str, id: &str) -> String { + make_query_cursor(&format!("{fingerprint}\n{key}"), id) +} + +fn parse_bound_query_cursor(value: &str, fingerprint: &str) -> Result<(String, String)> { + let (key, id) = parse_query_cursor(value)?; + let prefix = format!("{fingerprint}\n"); + let key = key.strip_prefix(&prefix).ok_or_else(|| { + Error::InvalidRecordId("cursor does not match the current filters or sort".to_string()) + })?; + Ok((key.to_string(), id)) +} + +fn ticket_query_fingerprint(query: &TicketQueryRequest, sort: TicketQuerySort) -> String { + let mut query = query.clone(); + query.cursor = None; + query.limit = None; + query.sort = Some(sort.to_string()); + serde_json::to_string(&query).expect("ticket query fingerprint must serialize") +} + +fn objective_query_fingerprint(query: &ObjectiveQueryRequest, sort: ObjectiveQuerySort) -> String { + let mut query = query.clone(); + query.cursor = None; + query.limit = None; + query.sort = Some(sort.to_string()); + serde_json::to_string(&query).expect("objective query fingerprint must serialize") +} + fn make_query_cursor(key: &str, id: &str) -> String { format!("v1:{}:{key}{id}", key.len()) } @@ -1677,8 +2126,8 @@ fn sort_ticket_query_items(items: &mut [TicketQueryItem], sort: TicketQuerySort) }); } -fn make_ticket_cursor(item: &TicketQueryItem, sort: TicketQuerySort) -> String { - make_query_cursor(&ticket_sort_key(item, sort), &item.id) +fn make_ticket_cursor(item: &TicketQueryItem, sort: TicketQuerySort, fingerprint: &str) -> String { + make_bound_query_cursor(fingerprint, &ticket_sort_key(item, sort), &item.id) } fn ticket_item_after_cursor( @@ -1703,31 +2152,6 @@ fn ticket_item_after_cursor( } } -fn objective_matches_query( - objective: &ObjectiveSummary, - body_md: &str, - query: &ObjectiveQueryRequest, -) -> bool { - if !query.states.is_empty() && !query.states.iter().any(|state| state == &objective.state) { - return false; - } - if query - .updated_after - .as_ref() - .is_some_and(|after| objective.updated_at.as_deref().unwrap_or("") <= after.as_str()) - || query - .updated_before - .as_ref() - .is_some_and(|before| objective.updated_at.as_deref().unwrap_or("") >= before.as_str()) - { - return false; - } - query.query.as_ref().is_none_or(|text| { - let needle = text.to_lowercase(); - objective.title.to_lowercase().contains(&needle) || body_md.to_lowercase().contains(&needle) - }) -} - fn objective_query_item( objective: ObjectiveSummary, linked_tickets: Vec, @@ -1805,8 +2229,12 @@ fn sort_objective_query_items(items: &mut [ObjectiveQueryItem], sort: ObjectiveQ }); } -fn make_objective_cursor(item: &ObjectiveQueryItem, sort: ObjectiveQuerySort) -> String { - make_query_cursor(&objective_sort_key(item, sort), &item.id) +fn make_objective_cursor( + item: &ObjectiveQueryItem, + sort: ObjectiveQuerySort, + fingerprint: &str, +) -> String { + make_bound_query_cursor(fingerprint, &objective_sort_key(item, sort), &item.id) } fn objective_item_after_cursor( @@ -1934,6 +2362,80 @@ fn memory_resolution_from_record(record: MemoryStagingResolutionRecord) -> Memor } } +fn ticket_summary_from_ticket(ticket: &ticket::Ticket) -> TicketSummary { + let summary = ticket::TicketSummary { + id: ticket.meta.id.clone(), + slug: ticket.meta.slug.clone(), + title: ticket.meta.title.clone(), + status: ticket.meta.status.clone(), + kind: ticket.meta.kind.clone(), + priority: ticket.meta.priority.clone(), + labels: ticket.meta.labels.clone(), + readiness: ticket.meta.readiness.clone(), + workflow_state: ticket.meta.workflow_state, + workflow_state_explicit: ticket.meta.workflow_state_explicit, + queued_by: ticket.meta.queued_by.clone(), + queued_at: ticket.meta.queued_at.clone(), + updated_at: ticket.meta.updated_at.clone(), + }; + ticket_summary_from_sqlite_item(SqliteTicketListItem { + summary, + relation_blockers: ticket.relations.blockers.clone(), + }) +} + +fn ticket_summary_from_sqlite_item(item: SqliteTicketListItem) -> TicketSummary { + let projection = project_ticket_workspace_item(&item.summary, &item.relation_blockers, None); + TicketSummary { + id: item.summary.id, + title: item.summary.title, + state: item.summary.workflow_state.as_str().to_string(), + priority: item.summary.priority, + updated_at: item.summary.updated_at, + queued_by: item.summary.queued_by, + queued_at: item.summary.queued_at, + workspace_action_priority: workspace_action_priority_name(projection.priority).to_string(), + record_source: "sqlite_yoi_ticket".to_string(), + } +} + +#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] +struct TicketSummaryCursorEnvelope { + version: u8, + fingerprint: String, + position: SqliteTicketListCursor, +} + +fn make_ticket_summary_cursor(fingerprint: &str, position: SqliteTicketListCursor) -> String { + make_query_cursor( + &serde_json::to_string(&TicketSummaryCursorEnvelope { + version: 1, + fingerprint: fingerprint.to_string(), + position, + }) + .expect("ticket summary cursor must serialize"), + "", + ) +} + +fn parse_ticket_summary_cursor( + value: &str, + expected_fingerprint: &str, +) -> Result { + let (encoded, trailing) = parse_query_cursor(value)?; + if !trailing.is_empty() { + return Err(Error::InvalidRecordId("cursor is malformed".to_string())); + } + let cursor: TicketSummaryCursorEnvelope = serde_json::from_str(&encoded) + .map_err(|_| Error::InvalidRecordId("cursor is malformed".to_string()))?; + if cursor.version != 1 || cursor.fingerprint != expected_fingerprint { + return Err(Error::InvalidRecordId( + "cursor does not match the current ticket filters or sort".to_string(), + )); + } + Ok(cursor.position) +} + fn workspace_action_priority_name(priority: TicketWorkspaceActionPriority) -> &'static str { match priority { TicketWorkspaceActionPriority::ReadyForQueue => "ready_for_queue", @@ -2352,6 +2854,28 @@ mod tests { .contains(&"body".to_string()) ); assert_eq!(ticket_query.page.limit, 1); + let no_review = authority + .query_tickets(TicketQueryRequest { + review_status: Some("none".to_string()), + sort: Some("updated_desc".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap(); + assert!( + no_review + .items + .iter() + .any(|item| item.id == "00000000001J2"), + "review-status storage predicate must query the authoritative MR thread schema" + ); + authority + .query_tickets(TicketQueryRequest { + review_status: Some("changes_requested".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .expect("accepted review-status alias must execute"); let historical_event_query = authority .query_tickets(TicketQueryRequest { query: Some("Historical event marker".to_string()), @@ -2404,6 +2928,35 @@ mod tests { .unwrap(); assert_eq!(exact_relation.items.len(), 1); assert_eq!(exact_relation.items[0].id, "00000000001J2"); + let incoming_relation = authority + .query_tickets(TicketQueryRequest { + related_ticket_id: Some("00000000001J2".to_string()), + relation_kind: Some("related".to_string()), + limit: Some(10), + ..TicketQueryRequest::default() + }) + .unwrap(); + assert_eq!(incoming_relation.items.len(), 1); + assert_eq!(incoming_relation.items[0].id, "00000000001J5"); + let summary_page = authority + .list_ticket_page(TicketListPageRequest { + states: vec!["planning".to_string(), "ready".to_string()], + limit: Some(1), + cursor: None, + }) + .unwrap(); + assert_eq!(summary_page.items.len(), 1); + assert!(summary_page.page.has_more); + let mismatched_summary_cursor = authority.list_ticket_page(TicketListPageRequest { + states: vec!["done".to_string()], + limit: Some(1), + cursor: summary_page.page.next_cursor, + }); + assert!(matches!( + mismatched_summary_cursor, + Err(Error::InvalidRecordId(_)) + )); + let first_page = authority .query_tickets(TicketQueryRequest { sort: Some("title".to_string()), @@ -2423,6 +2976,13 @@ mod tests { assert_eq!(second_page.items.len(), 1); assert_ne!(first_page.items[0].id, second_page.items[0].id); assert!(second_page.page.has_more); + let mismatched_cursor = authority.query_tickets(TicketQueryRequest { + sort: Some("updated_desc".to_string()), + limit: Some(1), + cursor: first_page.page.next_cursor.clone(), + ..TicketQueryRequest::default() + }); + assert!(matches!(mismatched_cursor, Err(Error::InvalidRecordId(_)))); let third_page = authority .query_tickets(TicketQueryRequest { sort: Some("title".to_string()), diff --git a/crates/workspace-server/src/records.rs b/crates/workspace-server/src/records.rs index 739b5f6f..d27cd326 100644 --- a/crates/workspace-server/src/records.rs +++ b/crates/workspace-server/src/records.rs @@ -12,6 +12,14 @@ pub struct ProjectRecordList { pub record_authority: String, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct TicketSummaryPage { + pub items: Vec, + pub page: QueryPage, + pub invalid_records: Vec, + pub record_authority: String, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] pub struct InvalidProjectRecord { @@ -33,12 +41,23 @@ pub struct TicketSummary { pub record_source: String, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, Default)] +pub struct TicketListPageRequest { + #[serde(default)] + pub states: Vec, + #[serde(default)] + pub limit: Option, + #[serde(default)] + pub cursor: Option, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[cfg_attr(feature = "typescript", derive(ts_rs::TS))] pub struct TicketListResponse { pub workspace_id: String, pub limit: usize, pub items: Vec, + pub page: QueryPage, pub invalid_records: Vec, pub record_authority: String, } diff --git a/crates/workspace-server/src/repositories.rs b/crates/workspace-server/src/repositories.rs index 908bc7a3..21b15634 100644 --- a/crates/workspace-server/src/repositories.rs +++ b/crates/workspace-server/src/repositories.rs @@ -97,38 +97,13 @@ pub struct CommitObservation { #[derive(Debug, Clone, PartialEq, Eq)] pub enum RepositoryLookupError { - UnknownRepository { - id: RepositoryId, - }, - UnsupportedProvider { - id: RepositoryId, - provider: String, - }, - MissingDefaultSelector { - id: RepositoryId, - }, - InvalidSelector { - id: RepositoryId, - selector: String, - }, - CommitNotFound { - id: RepositoryId, - commit: String, - }, - InvalidCommitRelation { - id: RepositoryId, - detail: String, - }, - TargetMoved { - id: RepositoryId, - selector: String, - expected: String, - observed: Option, - }, - ProviderFailure { - id: RepositoryId, - operation: String, - }, + UnknownRepository { id: RepositoryId }, + UnsupportedProvider { id: RepositoryId, provider: String }, + MissingDefaultSelector { id: RepositoryId }, + InvalidSelector { id: RepositoryId, selector: String }, + CommitNotFound { id: RepositoryId, commit: String }, + InvalidCommitRelation { id: RepositoryId, detail: String }, + ProviderFailure { id: RepositoryId, operation: String }, } #[derive(Debug, Clone)] @@ -306,45 +281,6 @@ impl RepositoryRegistryReader { } } - pub fn update_merge_target( - &self, - id: &str, - selector: &str, - expected_target: &str, - result_commit: &str, - ) -> Result<(), RepositoryLookupError> { - let repository = self.merge_repository(id)?; - let target_ref = normalize_target_branch_selector(id, selector)?; - self.observe_commit(id, result_commit)?; - let status = Command::new("git") - .arg("-C") - .arg(&repository.path) - .args([ - "update-ref", - target_ref.as_str(), - result_commit, - expected_target, - ]) - .status() - .map_err(|_| RepositoryLookupError::ProviderFailure { - id: id.into(), - operation: "guarded target update".into(), - })?; - if status.success() { - return Ok(()); - } - let observed = self - .observe_merge_target(id, Some(selector)) - .ok() - .map(|target| target.commit); - Err(RepositoryLookupError::TargetMoved { - id: id.into(), - selector: selector.into(), - expected: expected_target.into(), - observed, - }) - } - fn merge_repository(&self, id: &str) -> Result<&ConfiguredRepository, RepositoryLookupError> { let repository = self .find(id) @@ -755,20 +691,13 @@ mod tests { vec![base.clone()] ); reader.ensure_ancestor("main", &base, &source).unwrap(); - reader - .update_merge_target("main", "main", &base, &source) - .unwrap(); assert_eq!( reader .observe_merge_target("main", Some("refs/heads/main")) .unwrap() .commit, - source + base ); - assert!(matches!( - reader.update_merge_target("main", "refs/heads/main", &base, &base), - Err(RepositoryLookupError::TargetMoved { .. }) - )); assert!(matches!( reader.ensure_ancestor("main", &source, &base), Err(RepositoryLookupError::InvalidCommitRelation { .. }) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index d3d793a0..6034bee1 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1064,11 +1064,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 @@ -1080,20 +1087,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(()) @@ -1319,8 +1354,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", @@ -1401,6 +1436,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), @@ -2233,6 +2272,9 @@ struct ObjectiveEditRequest { #[derive(Debug, Deserialize)] struct TicketListQuery { limit: Option, + cursor: Option, + /// Comma-separated workflow states. Repeated lane requests normally pass one state group. + states: Option, } #[derive(Debug, Deserialize)] @@ -3111,6 +3153,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}")))?; @@ -3118,7 +3209,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> { @@ -3212,6 +3306,26 @@ 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()), + intake_summary: None, + }, + ) + .map_err(Error::from)?; + browser_ticket_detail(&api, &path.id) +} + async fn scoped_queue_ticket( State(api): State, AxumPath(path): AxumPath, @@ -3273,7 +3387,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(); @@ -3425,6 +3542,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, @@ -3538,9 +3664,12 @@ 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, + #[serde(default)] + intake_summary: Option, } async fn scoped_set_ticket_state_field( @@ -3582,24 +3711,31 @@ 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, + intake_summary: request.intake_summary, + }, }, ) .await?; - ticket_rest_unit(result) + ticket_rest_result(result, |result| match result { + TicketBackendOperationResult::Ticket(ticket) => Some(ticket), + _ => None, + }) } async fn scoped_queue_ticket_record( @@ -3773,6 +3909,37 @@ fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError { .into() } +fn recorded_merge_completion<'a>( + thread: &'a [merge_request::MergeRequestThreadEvent], + operation_id: &str, +) -> Option<&'a merge_request::MergeEvent> { + thread.iter().find_map(|event| match event { + merge_request::MergeRequestThreadEvent::Merge(event) + if event.operation_id == operation_id => + { + Some(event) + } + _ => None, + }) +} + +fn require_completed_target_observation( + observed: &str, + target_ref_before: &str, + target_ref_after: &str, +) -> ApiResult<()> { + if observed == target_ref_after { + return Ok(()); + } + if observed == target_ref_before { + return Err(Error::InvalidInput( + "target selector is still at target_ref_before; push the verified result from the Orchestrator Workdir before MergeRequestComplete".into(), + ) + .into()); + } + Err(Error::InvalidInput("target selector moved outside completion evidence".into()).into()) +} + async fn scoped_show_merge_request( State(api): State, AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>, @@ -4106,19 +4273,40 @@ async fn scoped_complete_merge_request( require_workspace_access(&workspace_id, &api)?; let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?; require_online_workspace_orchestrator_source(&api, &source)?; + let store = merge_request_store(&api, &workspace_id)?; + let mr = store.get(&workspace_id, &ticket_id)?; + let repositories = api.repository_reader(); + if let Some(existing) = recorded_merge_completion(&mr.thread, &input.operation_id) { + let replay = merge_request::CompleteMergeRequest { + ticket_id, + operation_id: input.operation_id, + approval_event_id: input.approval_event_id, + current_subject_ref: existing.approved_source_ref.clone(), + target_ref_before: input.target_ref_before, + target_ref_after: input.target_ref_after, + strategy: input.strategy, + resolution: input.resolution, + auth: merge_request::MergeRequestAuth { + workspace_id, + repository_id: mr.repository_id.clone(), + runtime_id: source.runtime_id, + worker_id: source.worker_id, + assignment_id: String::new(), + }, + now: Utc::now(), + }; + return store.complete(replay).map(Json).map_err(Into::into); + } let assignment = api .store .get_current_ticket_worker_assignment(&workspace_id, &ticket_id)? .ok_or_else(|| { Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into()) })?; - let store = merge_request_store(&api, &workspace_id)?; - let mr = store.get(&workspace_id, &ticket_id)?; let selector = mr .selector_from .as_deref() .ok_or_else(|| Error::InvalidInput("selector_from requires repair".into()))?; - let repositories = api.repository_reader(); let current_source_ref = repositories .observe_merge_target(&mr.repository_id, Some(selector)) .map_err(repository_merge_evidence_error)? @@ -4126,12 +4314,11 @@ async fn scoped_complete_merge_request( let observed = repositories .observe_merge_target(&mr.repository_id, Some(&mr.selector_to)) .map_err(repository_merge_evidence_error)?; - if observed.commit != input.target_ref_before && observed.commit != input.target_ref_after { - return Err(Error::InvalidInput( - "target selector moved outside completion evidence".into(), - ) - .into()); - } + require_completed_target_observation( + &observed.commit, + &input.target_ref_before, + &input.target_ref_after, + )?; let completion = merge_request::CompleteMergeRequest { ticket_id, operation_id: input.operation_id, @@ -4151,31 +4338,7 @@ async fn scoped_complete_merge_request( now: Utc::now(), }; store.validate_completion(&completion)?; - let already = observed.commit == input.target_ref_after; - if !already { - repositories - .update_merge_target( - &mr.repository_id, - &mr.selector_to, - &input.target_ref_before, - &input.target_ref_after, - ) - .map_err(repository_merge_evidence_error)? - } - match store.complete(completion) { - Ok(v) => Ok(Json(v)), - Err(e) => { - if !already { - let _ = repositories.update_merge_target( - &mr.repository_id, - &mr.selector_to, - &input.target_ref_after, - &input.target_ref_before, - ); - } - Err(e.into()) - } - } + store.complete(completion).map(Json).map_err(Into::into) } fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> { @@ -4381,7 +4544,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, .. } @@ -4422,11 +4585,11 @@ 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.clone()); + if let Some(summary) = request.intake_summary.as_mut() { + summary.author = Some(author); + } } TicketBackendOperation::QueueReady { queued_by, .. } => *queued_by = author, TicketBackendOperation::AddTicketRelation { relation, .. } => { @@ -4448,7 +4611,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", @@ -8329,17 +8492,35 @@ async fn list_tickets( State(api): State, Query(query): Query, ) -> ApiResult> { - let requested_limit = query.limit.unwrap_or(api.config.max_records); - let limit = requested_limit.min(1000); - let ProjectRecordList { + let limit = query.limit.unwrap_or(30).clamp(1, 100); + let states = query + .states + .as_deref() + .map(|states| { + states + .split(',') + .filter(|state| !state.is_empty()) + .map(str::to_string) + .collect::>() + }) + .unwrap_or_default(); + let crate::records::TicketSummaryPage { items, + page, invalid_records, record_authority, - } = api.authority.list_tickets(limit)?; + } = api + .authority + .list_ticket_page(crate::records::TicketListPageRequest { + states, + limit: Some(limit), + cursor: query.cursor, + })?; Ok(Json(crate::records::TicketListResponse { workspace_id: api.config.workspace_id, limit, items, + page, invalid_records, record_authority, })) @@ -11896,7 +12077,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 { .. } @@ -12465,6 +12665,54 @@ mod tests { ); } + #[test] + fn recorded_completion_replay_is_identified_before_later_target_observation() { + let event = merge_request::MergeEvent { + event_id: "merge-event".into(), + sequence: 1, + operation_id: "operation".into(), + approval_event_id: "approval".into(), + approved_source_ref: "source".into(), + target_ref_before: "before".into(), + target_ref_after: "after".into(), + strategy: merge_request::MergeStrategy::FastForward, + resolution: merge_request::ConflictResolution::None, + merged_by: merge_request::WorkerIdentity { + runtime_id: "runtime".into(), + worker_id: "orchestrator".into(), + }, + created_at: Utc::now(), + }; + let thread = vec![merge_request::MergeRequestThreadEvent::Merge(event.clone())]; + + assert_eq!( + recorded_merge_completion(&thread, "operation"), + Some(&event) + ); + assert!(recorded_merge_completion(&thread, "different").is_none()); + assert!(require_completed_target_observation("later", "before", "after").is_err()); + } + + #[test] + fn merge_request_completion_records_only_an_observed_remote_target_update() { + require_completed_target_observation("after", "before", "after").unwrap(); + + let not_pushed = + require_completed_target_observation("before", "before", "after").unwrap_err(); + assert!(matches!( + not_pushed.error, + Error::InvalidInput(ref message) + if message.contains("push the verified result from the Orchestrator Workdir") + )); + + let moved = require_completed_target_observation("other", "before", "after").unwrap_err(); + assert!(matches!( + moved.error, + Error::InvalidInput(ref message) + if message.contains("moved outside completion evidence") + )); + } + #[test] fn worker_ticket_assignment_projects_coder_intent_and_run_acceptance() { let initial_submit = vec![ @@ -13562,7 +13810,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()), @@ -13719,7 +13967,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"], ] { @@ -13751,6 +13999,88 @@ 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()), + intake_summary: None, + }; + 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, + intake_summary: 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"); @@ -13792,6 +14122,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 @@ -13849,29 +14180,24 @@ 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, + intake_summary: 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(), @@ -14276,30 +14602,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( @@ -14725,6 +15032,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() @@ -14764,7 +15072,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()), }), @@ -14774,7 +15082,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); @@ -14795,14 +15103,13 @@ 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()), + intake_summary: None, }), ) .await diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 06e18e5e..f1d59410 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -196,6 +196,11 @@ const MIGRATIONS: &[Migration] = &[ name: "remove Worker control delegation authority", apply: remove_worker_control_delegation_authority, }, + Migration { + version: 36, + name: "add Objective query indexes", + apply: add_objective_query_indexes, + }, ]; struct Migration { @@ -5046,6 +5051,24 @@ fn create_worker_control_delegation_operation_authority(conn: &Connection) -> Re Ok(()) } +fn add_objective_query_indexes(conn: &Connection) -> Result<()> { + conn.execute_batch( + r#" + CREATE INDEX IF NOT EXISTS objectives_workspace_state_updated + ON objectives(workspace_id, state, updated_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_updated + ON objectives(workspace_id, updated_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_created + ON objectives(workspace_id, created_at DESC, objective_id); + CREATE INDEX IF NOT EXISTS objectives_workspace_title + ON objectives(workspace_id, title COLLATE NOCASE, objective_id); + CREATE INDEX IF NOT EXISTS objective_ticket_links_workspace_ticket_objective + ON objective_ticket_links(workspace_id, ticket_id, objective_id); + "#, + )?; + Ok(()) +} + fn remove_worker_control_delegation_authority(conn: &Connection) -> Result<()> { let mut statement = conn.prepare("SELECT workspace_id, grant_id, permissions_json FROM worker_control_grants")?; @@ -5799,7 +5822,7 @@ INSERT INTO worker_control_grants ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(!table_exists(&conn, "worker_control_delegation_operations").unwrap()); let (permissions_json, revoked_at): (String, Option) = conn .query_row( @@ -5847,7 +5870,7 @@ INSERT INTO worker_control_grants ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(table_exists(&conn, "worker_workdir_attachment_reservations").unwrap()); } @@ -5880,7 +5903,7 @@ CREATE TABLE flow_events (event_id TEXT PRIMARY KEY); apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); assert!(table_exists(&conn, "flow_sources").unwrap()); assert!(table_exists(&conn, "flow_source_revisions").unwrap()); assert!(!table_exists(&conn, "flow_instances").unwrap()); @@ -5947,7 +5970,7 @@ INSERT INTO worker_workdir_attachment_reservations ( apply_migrations(&conn).unwrap(); - assert_eq!(current_schema_version(&conn).unwrap(), 35); + assert_eq!(current_schema_version(&conn).unwrap(), 36); let repositories_sql: String = conn .query_row( "SELECT sql FROM sqlite_master WHERE type = 'table' AND name = 'repositories'", @@ -6127,7 +6150,7 @@ INSERT INTO workdir_registry ( let db = dir.path().join("control-plane.sqlite"); let store = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); assert!( !store .with_conn(|conn| table_exists(conn, "worker_workspace_credentials")) @@ -6144,7 +6167,7 @@ INSERT INTO workdir_registry ( store.upsert_workspace(&record).await.unwrap(); let reopened = SqliteWorkspaceStore::open(&db).unwrap(); - assert_eq!(reopened.schema_version().await.unwrap(), 35); + assert_eq!(reopened.schema_version().await.unwrap(), 36); assert_eq!( reopened.get_workspace("local-dev").await.unwrap(), Some(record) @@ -6691,7 +6714,7 @@ INSERT INTO workdir_registry ( .unwrap(); let store = SqliteWorkspaceStore::from_connection(conn).unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); store .with_conn(|conn| { @@ -6880,7 +6903,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn repository_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: None, @@ -6946,7 +6969,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn memory_authority_records_round_trip_and_close_staging() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let workspace = WorkspaceRecord { workspace_id: "local-dev".to_string(), owner_account_id: None, @@ -7337,7 +7360,7 @@ CREATE TABLE ticket_assignment_operations ( #[tokio::test] async fn account_and_login_records_round_trip() { let store = SqliteWorkspaceStore::in_memory().unwrap(); - assert_eq!(store.schema_version().await.unwrap(), 35); + assert_eq!(store.schema_version().await.unwrap(), 36); let now = "2026-07-22T00:00:00Z".to_string(); let account = AccountRecord { account_id: "acct-user-alice".to_string(), diff --git a/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/docs/development/work-items.md b/docs/development/work-items.md index 8e096fea..ebd29c50 100644 --- a/docs/development/work-items.md +++ b/docs/development/work-items.md @@ -31,14 +31,15 @@ Maintainers can inspect the local `.yoi/tickets/` files directly when debugging ## Ticket tools inside Workers -Workers with the Ticket built-in feature can use typed Ticket tools: +Workers with the Ticket and operation-specific Merge Request built-in features can use typed workflow tools: - `TicketCreate` - `QueryTicket` — bounded authoritative Ticket discovery with typed state/text/event/evidence/relation/Objective/time/attention filters, stable snippets, and cursor metadata. - `ShowTicket` — detailed authority for one Ticket, including item revision, bounded thread/event references, relations, linked Objectives, implementation reports, and current Merge Request/review evidence. - `TicketComment` -- `MergeRequestShow`, `MergeRequestOpen`, `MergeRequestAddRevision`, `MergeRequestComplete` -- `MergeRequestReviewSubmit` — available only inside the attested direct-child Reviewer attempt; attempt/revision capability material is not model input. +- Coder: `MergeRequestShow`, `MergeRequestOpen` +- Reviewer: `MergeRequestShow`, `MergeRequestReview` — available only inside the attested direct-child Reviewer request; grant and subject-ref capability material are not model input. +- Orchestrator: `MergeRequestShow`, `MergeRequestReadinessCheck`, `MergeRequestComplete` - `TicketClose` - `TicketRelationRecord` @@ -243,7 +244,7 @@ Implementation normally happens in a child git worktree created by the Orchestra The assigned Coder launches the Reviewer as an actual direct-child `builtin:reviewer` SubWorker with read-only scope and a structured handoff bound to the current immutable Merge Request revision. Server authority revalidates the parent assignment, Runtime-owned child session, effective profile, one-shot review attempt, and revision; prose output is not approval. -The Reviewer records the structured result with `MergeRequestReviewSubmit`. Request changes requires a new immutable revision and a fresh child attempt. `MergeRequestComplete` performs guarded Ticket completion with operation-id dedupe/CAS semantics; Flow transitions are not completion authority. +The Reviewer records the structured result with `MergeRequestReview`. Request changes requires a new immutable revision and a fresh child attempt. The Orchestrator uses `MergeRequestReadinessCheck` and then `MergeRequestComplete` for guarded integration with operation-id dedupe/CAS semantics; Flow transitions are not completion authority. Blockers must be fixed or explicitly escalated before merge-ready submission. diff --git a/resources/flows/coder-review.dcdl b/resources/flows/coder-review.dcdl index 348389e3..e90b63df 100644 --- a/resources/flows/coder-review.dcdl +++ b/resources/flows/coder-review.dcdl @@ -5,7 +5,7 @@ states = { implement = { - instructions = "Before editing, inspect the assigned Workdir's Git state. A newly delegated Git Workdir normally starts at a detached HEAD. If HEAD is detached, create and switch to a local branch named `work/-`, using the canonical Ticket id and a short lowercase kebab-case slug derived from the Ticket title or implementation scope. If the Workdir is already on a suitable work branch after restore, keep it. Never delete, reset, or overwrite an existing branch to resolve a name collision; choose a concise collision-free suffix and report the actual branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Implement the requested Ticket scope, run the narrow and dependent validation required by the changed contracts, and record concrete evidence. Commit coherent, validated implementation slices while working. After the implementation is committed, validated, and clean, publish only the current Ticket work branch to the configured repository remote with a normal non-force push, then verify that the published source ref resolves to the exact current HEAD. Do not push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Open or update the Ticket Merge Request from that published source ref with immutable repository revision evidence. Before requesting independent review, confirm that the Workdir is clean, the published source ref and current HEAD are identical, and the current MR revision records that exact subject. A Flow transition is never Ticket completion authority."; + instructions = "Inspect the assigned Workdir Git state before editing. Reuse a suitable restored `work/-` branch, or create a collision-safe work branch from detached HEAD; never overwrite an existing branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Implement the requested Ticket scope, run the narrow and dependent validation required by the changed contracts, and record concrete evidence in coherent commits. After the implementation is committed, validated, and clean, publish only the current Ticket work branch to the Ticket repository remote with a normal non-force push, then verify that the published source selector resolves to the exact local HEAD. Do not push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Open or update the Ticket Merge Request with immutable `selector_from` / `selector_to` revision evidence. Do not request review from a dirty Workdir or an unpublished source ref. A Flow transition is never Ticket completion authority."; transitions = { review = { target = "review"; @@ -15,7 +15,7 @@ }; review = { - instructions = "Spawn one actual direct-child SubWorker with profile builtin:reviewer, read-only scope, and a structured review handoff bound to the current immutable Merge Request revision. The child must commit MergeRequestReviewSubmit; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition."; + instructions = "Use the current Ticket Merge Request as review authority. Confirm its immutable source selector resolves to the exact committed implementation HEAD, then spawn one actual direct-child SubWorker with profile builtin:reviewer, read-only scope, and a structured review handoff bound to the current immutable Merge Request revision. The trusted spawn layer records `ReviewRequested`; do not place commit/ref identity, capability material, or a prewritten verdict in model input. The child must commit MergeRequestReview; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition."; transitions = { approved = { target = "complete"; @@ -29,7 +29,7 @@ }; fix = { - instructions = "Resolve every open Reviewer finding on the same Ticket work branch, rerun the validation affected by the fixes, commit the corrected implementation as a new revision, and preserve concrete evidence. Publish the updated Ticket work branch with a normal non-force push, verify that the configured repository provider resolves the published source ref to the exact new HEAD, and update the linked Merge Request so its current revision records that same subject. Do not rewrite the previously reviewed commit, claim approval from the prior request_changes review, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected committed revision is published and ready for a new independent review."; + instructions = "Resolve every open Reviewer finding on the same Ticket work branch, rerun the validation affected by the fixes, commit the corrected implementation as a new revision, and preserve concrete evidence. Publish only the updated Ticket work branch with a normal non-force push, verify that the configured repository provider resolves the published source ref to the exact new HEAD, and update the linked Merge Request so its current revision records that same subject. Request review from a fresh read-only Reviewer child so the trusted spawn layer captures the new immutable subject. Do not rewrite the previously reviewed commit, claim approval from the prior request_changes review, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected committed revision is published and ready for a new independent review."; transitions = { review = { target = "review"; @@ -39,17 +39,17 @@ }; complete = { - instructions = "The exact current Merge Request subject has authoritative approval. Leave a concise human-facing summary only when useful, then hand off to the Orchestrator. Do not call MergeRequestComplete; approval and the current MR revision are the durable completion evidence. Request a Flow transition only after the Orchestrator's authoritative completion changes the Ticket to done."; + instructions = "Verify the authoritative approval still matches the exact current Merge Request subject, keep the reviewed source ref immutable, leave concise implementation and validation evidence on the Ticket when useful, then hand off to the Orchestrator. Do not call MergeRequestComplete, update the target selector, or treat the Flow terminal state as Ticket completion authority. After durable handoff evidence exists, request a Flow transition."; transitions = { completed = { target = "done"; - condition = "The Orchestrator completed the current approved Merge Request and the authoritative Ticket state is done. A Flow state or prose report alone is never sufficient."; + condition = "The exact approved Merge Request revision and implementation evidence have been durably handed off to the Orchestrator. A Flow state or prose report alone is never Ticket completion authority."; }; }; }; done = { - instructions = "The guarded Merge Request completion operation committed Ticket state done. Flow terminal state only reflects that durable authority."; + instructions = "The approved implementation has been handed off for Orchestrator-owned readiness and integration. Flow terminal state only reflects that handoff."; terminal = true; }; }; diff --git a/resources/profiles/base.dcdl b/resources/profiles/base.dcdl index 04148c7b..2e960d15 100644 --- a/resources/profiles/base.dcdl +++ b/resources/profiles/base.dcdl @@ -30,6 +30,13 @@ feature = { worker = { enabled = false; }; objective = { enabled = true; }; ticket = { enabled = true; authoring = true; thread = true; }; + merge_request = { + show = false; + open = false; + review = false; + readiness_check = false; + complete = false; + }; }; memory = { diff --git a/resources/profiles/coder.dcdl b/resources/profiles/coder.dcdl index 27a85776..93713b55 100644 --- a/resources/profiles/coder.dcdl +++ b/resources/profiles/coder.dcdl @@ -12,5 +12,12 @@ import "./base.dcdl" // { flow = { enabled = true; }; worker = { enabled = true; }; ticket = { enabled = true; thread = true; }; + merge_request = { + show = true; + open = true; + review = false; + readiness_check = false; + complete = false; + }; }; } diff --git a/resources/profiles/orchestrator.dcdl b/resources/profiles/orchestrator.dcdl index feb7f402..cb654b63 100644 --- a/resources/profiles/orchestrator.dcdl +++ b/resources/profiles/orchestrator.dcdl @@ -12,6 +12,13 @@ import "./base.dcdl" // { worker = { enabled = true; direct_spawn = false; }; manage_workdir = { enabled = true; }; ticket = { enabled = true; thread = true; workflow = true; }; + merge_request = { + show = true; + open = false; + review = false; + readiness_check = true; + complete = true; + }; orchestration = { enabled = true; }; }; } diff --git a/resources/profiles/reviewer.dcdl b/resources/profiles/reviewer.dcdl index 82270494..b41f9627 100644 --- a/resources/profiles/reviewer.dcdl +++ b/resources/profiles/reviewer.dcdl @@ -11,5 +11,12 @@ import "./base.dcdl" // { sub_worker = { enabled = false; }; worker = { enabled = false; }; ticket = { enabled = true; thread = false; }; + merge_request = { + show = true; + open = false; + review = true; + readiness_check = false; + complete = false; + }; }; } diff --git a/resources/prompts/catalog.dcdl b/resources/prompts/catalog.dcdl index 84dd4b2f..1144f3a3 100644 --- a/resources/prompts/catalog.dcdl +++ b/resources/prompts/catalog.dcdl @@ -6,6 +6,7 @@ let defaultDocument = import "./default.md"; commonLanguage = import "./common/language.md"; commonGit = import "./common/git.md"; +commonMergeRequest = import "./common/merge-request.md"; commonTickets = import "./common/tickets.md"; commonToolUsage = import "./common/tool-usage.md"; commonWorkerObservation = import "./common/worker-observation.md"; @@ -36,6 +37,7 @@ in common = { git = commonGit.content; language = commonLanguage.content; + merge_request = commonMergeRequest.content; tickets = commonTickets.content; tool_usage = commonToolUsage.content; worker_observation = commonWorkerObservation.content; diff --git a/resources/prompts/common/merge-request.md b/resources/prompts/common/merge-request.md new file mode 100644 index 00000000..4e7a8996 --- /dev/null +++ b/resources/prompts/common/merge-request.md @@ -0,0 +1,19 @@ +## Merge Request workflow + +Use only the exposed Merge Request operations; their availability expresses this Worker's workflow responsibility, not authorization to bypass Backend validation. +{% if "MergeRequestShow" in tools %} +- Reread the current Merge Request and append-only thread with `MergeRequestShow` before making review or integration decisions. +{% endif %} +{% if "MergeRequestOpen" in tools %} +- Open the Merge Request only after all intended changes are committed and the Workdir is clean. Use immutable source and target selectors; do not infer target authority from a branch name or cwd. +- Before requesting independent review, make the exact current MR revision authoritative. +{% endif %} +{% if "MergeRequestReview" in tools %} +- Review the exact current immutable MR revision independently. Submit the authoritative verdict through `MergeRequestReview`; prose alone is not approval. +{% endif %} +{% if "MergeRequestReadinessCheck" in tools %} +- Use `MergeRequestReadinessCheck` to resolve current refs and authoritative review readiness before integration. +{% endif %} +{% if "MergeRequestComplete" in tools %} +- Complete integration only after readiness confirms approval for the exact current revision and all target/ref guards pass. Merge completion is separate from implementation and review evidence. +{% endif %} diff --git a/resources/prompts/role/coder.md b/resources/prompts/role/coder.md index 60e887e1..b8e2dbd3 100644 --- a/resources/prompts/role/coder.md +++ b/resources/prompts/role/coder.md @@ -2,8 +2,10 @@ You are the assigned Coder. Implement the requested scope in the provided Workdi Treat the first committed user message as the bounded Ticket/action context and do not infer control-plane identity from prose. +Before opening a Merge Request, publish only the committed Ticket work branch with a normal non-force push and verify that the Ticket repository remote resolves it to the exact local `HEAD`; a local branch name or dirty Workdir is not immutable review evidence. Do not push the target branch, tags, or unrelated refs, and never force-push. + {% include "common.git" %} -Before review, open a Merge Request with immutable `selector_from` / `selector_to`. Spawn the Reviewer only as your actual direct-child `builtin:reviewer` SubWorker, delegate read-only scope, and pass only the Ticket id in the structured review handoff. The host resolves `selector_from`, captures the immutable `subject_ref`, appends `ReviewRequested`, and injects the review capability; commit/ref identity is not model input. Reviewer prose is not approval: the child must commit `MergeRequestReviewSubmit` through its injected capability authority. +Before review, open a Merge Request with immutable `selector_from` / `selector_to`. Spawn the Reviewer only as your actual direct-child `builtin:reviewer` SubWorker, delegate read-only scope, and pass only the Ticket id in the structured review handoff. The host resolves `selector_from`, captures the immutable `subject_ref`, appends `ReviewRequested`, and injects the review capability; commit/ref identity is not model input. Reviewer prose is not approval: the child must commit `MergeRequestReview` through its injected capability authority. -A request-changes result requires a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request subject has an authoritative approval, leave a concise human-facing summary when useful and hand off to the Orchestrator. Do not call `MergeRequestComplete`; approval and the current MR revision are the durable completion evidence. +A request-changes result requires a freshly published immutable subject and a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request subject has authoritative approval, keep that source ref immutable, leave concise implementation evidence on the Ticket when useful, and hand off integration to the Orchestrator. Do not update the target selector. Do not call `MergeRequestComplete`. diff --git a/resources/prompts/role/orchestrator.md b/resources/prompts/role/orchestrator.md index eb1b23dc..bdbd7801 100644 --- a/resources/prompts/role/orchestrator.md +++ b/resources/prompts/role/orchestrator.md @@ -2,11 +2,15 @@ You are the Ticket Orchestrator role. {% include "common.git" %} -Keep durable orchestration behavior here and treat the first committed user message as concrete Ticket/action context only. Use typed Ticket tools and current repository state as authority. Record `inprogress` before implementation side effects, then use `SpawnTicketCoder` so Worker creation, the fixed Coder profile/Flow, and the current Ticket assignment are one guarded operation. After spawn, reread the Ticket and verify its current assignment names that Coder before asking it to implement; never route implementation to an unassigned Coder. Route implementation work to sibling Coder Workers. The human `ready -> queued` transition delegates guarded implementation, merging the current approved Merge Request, recording completion, and closing the Ticket to the Workspace Orchestrator by default; do not wait for a second merge confirmation. Stop only when the Ticket explicitly records a separate approval gate or completion requires a new decision outside the queued scope. +Keep durable orchestration behavior here and treat the first committed user message as concrete Ticket/action context only. Use typed Ticket tools and current repository state as authority. Record `inprogress` before implementation side effects, then use `SpawnTicketCoder` so Worker creation, the fixed Coder profile/Flow, and the current Ticket assignment are one guarded operation. After spawn, reread the Ticket and verify its current assignment names that Coder before asking it to implement; never route implementation to an unassigned Coder. Route implementation work to sibling Coder Workers. The human `ready -> queued` transition delegates ordinary implementation, publication of the Ticket source work branch, guarded integration of the current approved Merge Request, recording completion, and closing the Ticket to the Workspace Orchestrator by default; do not wait for a second merge confirmation. This queue delegation does not grant broader repository authority from launch prose. Stop only when the Ticket explicitly records a separate approval gate or completion requires a new decision outside the queued scope. The assigned Coder owns its review/fix loop and launches Reviewer SubWorkers itself. Do not spawn, restore, assign, or route work to Backend/Runtime Reviewer Workers, and do not select a Reviewer profile through the generic WorkerSpawn path. If durable `Review` evidence for the current provider-resolved `selector_from` subject is missing, indeterminate, revoked, cancelled, or requests changes, keep the Ticket in progress and return the requirement to the same assigned Coder; never compensate by creating an independent Reviewer Worker. -Treat the current linked Merge Request as implementation-completion authority. A current provider-resolved source ref, commit/repository evidence, an effective approval for that exact subject, review freshness after the latest substantive Ticket item edit, and no unresolved request-changes are sufficient; do not require an `implementation_report`. Human summaries remain optional audit context. Recheck `ShowTicket` and `MergeRequestReadinessCheck` immediately before guarded completion, and only the Orchestrator may call `MergeRequestComplete`. +Treat the current linked Merge Request as implementation-completion authority. A current provider-resolved source ref, commit/repository evidence, an effective approval for that exact subject, review freshness after the latest substantive Ticket item edit, and no unresolved request-changes are sufficient; do not require an `implementation_report`. Human summaries remain optional audit context. Recheck `ShowTicket` and `MergeRequestReadinessCheck` immediately before guarded integration. Require the Merge Request source selector to remain on the exact reviewed commit; any source movement requires a fresh Reviewer attempt. + +Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. In the Orchestrator Workdir, use the Ticket repository `origin` transport to fetch the current target selector and immutable source selector, verify both against readiness evidence, apply the selected fast-forward or merge strategy, and validate the resulting tree. Push only a result that descends from the observed target, using a guarded non-force push whose expected old target is `target_ref_before`; reject target movement and conflicts rather than rewriting the remote. Verify the remote target now resolves exactly to `target_ref_after`, then call `MergeRequestComplete` with that before/after evidence and the authoritative approval event. Never mutate a Server-side repository path or use local `git update-ref` as integration authority. + +If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: while no completion event exists, the Server requires the target to remain at the exact `target_ref_after` before it records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Once that exact operation is recorded, later target movement does not invalidate an idempotent replay of the recorded result. Before recording, any other observed target is a stale/conflicting completion and must fail closed. Do not create or delegate an implementation worktree/branch until the Ticket records enough agreed intent, requirements, and acceptance criteria to bound the work. diff --git a/resources/prompts/role/reviewer.md b/resources/prompts/role/reviewer.md index 112c412c..24677be6 100644 --- a/resources/prompts/role/reviewer.md +++ b/resources/prompts/role/reviewer.md @@ -1,7 +1,7 @@ You are the Ticket Reviewer role running as an actual Runtime-owned direct child of the assigned Coder. -Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only. Review the host-captured `ReviewRequested.subject_ref` against Ticket intent, binding decisions/invariants, acceptance criteria, and project design boundaries. Use read-only inspection and focused validation; do not merge, close, mutate the Workdir, or take over implementation. +Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only, never as a supplied verdict. Review the host-captured `ReviewRequested.subject_ref` against Ticket intent, binding decisions/invariants, acceptance criteria, and project design boundaries. Use read-only inspection and focused validation; do not merge, close, mutate the Workdir, update a repository ref, or take over implementation. -Your prose response is not review authority. Before finishing, call `MergeRequestReviewSubmit` exactly once with `approve` or `request_changes`, a bounded evidence summary, and concrete structured findings. Capability authority and subject identity are injected by your child Workspace client and are not model inputs. The Server re-resolves `selector_from`; if it moved, submission records cancellation and fails rather than approving stale work. +Your prose response is not review authority. Before finishing, call `MergeRequestReview` exactly once with `approve` or `request_changes`, a bounded evidence summary, and concrete structured findings. Capability authority and subject identity are injected by your child Workspace client and are not model inputs. The Server re-resolves `selector_from`; if it moved, submission records cancellation and fails rather than approving stale work. Review more than the diff: verify the implementation satisfies the Ticket intent and acceptance criteria, remains coherent with the codebase design, and does not introduce unnecessary compatibility. diff --git a/web/workspace/src/lib/generated/ticket-api.ts b/web/workspace/src/lib/generated/ticket-api.ts index f3e14081..c7294af8 100644 --- a/web/workspace/src/lib/generated/ticket-api.ts +++ b/web/workspace/src/lib/generated/ticket-api.ts @@ -19,6 +19,7 @@ export type TicketListResponse = { workspace_id: string; limit: number; items: Array; + page: QueryPage; invalid_records: Array; record_authority: string; }; diff --git a/web/workspace/src/lib/workspace/styles/tickets.css b/web/workspace/src/lib/workspace/styles/tickets.css index f172464d..f10b07a4 100644 --- a/web/workspace/src/lib/workspace/styles/tickets.css +++ b/web/workspace/src/lib/workspace/styles/tickets.css @@ -178,7 +178,9 @@ align-content: start; gap: 0.55rem; min-height: 0; + max-height: min(68vh, 48rem); overflow-y: auto; + overscroll-behavior: contain; padding: 0.6rem; scrollbar-gutter: stable; } @@ -189,6 +191,27 @@ font-size: 0.7rem; text-align: center; } + .ticket-lane-page-state { + display: flex; + justify-content: center; + gap: 0.5rem; + margin: 0; + padding: 0.45rem; + color: var(--text-muted); + font-size: 0.72rem; + text-align: center; + } + .ticket-lane-page-error { + align-items: center; + color: var(--danger); + } + .ticket-lane-page-error button { + border: 1px solid var(--line); + border-radius: 0.35rem; + background: var(--bg-raised); + color: inherit; + padding: 0.2rem 0.45rem; + } .ticket-card { display: grid; gap: 0.55rem; diff --git a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts index d3089a7b..3b536088 100644 --- a/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts +++ b/web/workspace/src/lib/workspace/tickets/ticket-panel.test.ts @@ -11,16 +11,9 @@ import type { } from "../../generated/ticket-api.ts"; declare const Deno: { - test(name: string, fn: () => Promise | void): void; - readTextFile(path: string): Promise; + test(name: string, fn: () => void): void; }; -function assertIncludes(actual: string, expected: string): void { - if (!actual.includes(expected)) { - throw new Error(`expected source to include ${JSON.stringify(expected)}`); - } -} - function assertEquals(actual: T, expected: T): void { if (JSON.stringify(actual) !== JSON.stringify(expected)) { throw new Error( @@ -115,29 +108,3 @@ Deno.test("ticket worker launch uses the common Worker route and bounded Ticket "Work on Ticket 00001KYRRDVH9 as its reviewer.", ); }); - -Deno.test("ticket panel starts the Orchestrator explicitly and gates orchestration actions", async () => { - const panelSource = await Deno.readTextFile( - "src/routes/w/[workspaceId]/tickets/+page.svelte", - ); - const detailSource = await Deno.readTextFile( - "src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte", - ); - - assertIncludes( - panelSource, - 'workspaceApiPath(data.workspaceId, "/orchestrator")', - ); - assertIncludes(panelSource, '{ method: "POST" }'); - assertIncludes(panelSource, "Start Orchestrator"); - assertIncludes(panelSource, "orchestrator.data?.online"); - assertIncludes(panelSource, "lane.tickets.slice(0, lane.visibleCount)"); - assertIncludes( - panelSource, - "onscroll={(event) => handleLaneScroll(event, lane.id)}", - ); - assertIncludes(panelSource, "Scroll for"); - assertIncludes(detailSource, "{#if orchestratorOnline}"); - assertIncludes(detailSource, "!orchestratorOnline"); - assertIncludes(detailSource, "Orchestrator offline"); -}); diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte index 2bcdaee8..ff68779f 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.svelte @@ -1,44 +1,95 @@ -Tickets · Yoi + + Tickets · {data.workspaceId} +
+

Delivery

Tickets

+

+ Plan, route, review, and close work without leaving the workspace. +

@@ -107,16 +141,12 @@ {/if}
- {displayedTicketCount} - tickets displayed + {tickets.length} + loaded tickets
- {#if data.tickets.error} -

Tickets: {data.tickets.error}

- {/if} - {#if orchestrator.error}

Orchestrator status: {orchestrator.error} @@ -129,24 +159,21 @@

{#each lanes as lane (lane.id)} - {@const displayedTickets = lane.tickets.slice(0, lane.visibleCount)} - {@const hasMore = lane.visibleCount < lane.tickets.length} + {@const pagination = laneState[lane.id]}

{lane.label}

- - {displayedTickets.length}{hasMore ? "+" : ""} - + {lane.tickets.length}
-
handleLaneScroll(event, lane.id)} > - {#each displayedTickets as ticket (ticket.id)} + {#each lane.tickets as ticket (ticket.id)} No tickets
{/each} - - {#if hasMore} -

- Scroll for {Math.min(TICKET_LANE_PAGE_SIZE, lane.tickets.length - lane.visibleCount)} more -

- {:else if displayedTickets.length > 0} -

All tickets displayed.

+ {#if pagination?.loading} +

Loading…

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

End of lane

{/if}
diff --git a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts index 2520733d..04ab28e8 100644 --- a/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts +++ b/web/workspace/src/routes/w/[workspaceId]/tickets/+page.ts @@ -1,14 +1,43 @@ -import type { TicketListResponse } from "$lib/generated/ticket-api"; import { loadJson, workspaceApiPath } from "$lib/workspace/api/http"; +import type { TicketListResponse } from "$lib/generated/ticket-api"; import type { WorkspaceOrchestratorStatus } from "$lib/workspace/tickets/ticket-panel"; import type { PageLoad } from "./$types"; -export const load = (async ({ fetch, params }) => { +const LANE_STATES = { + "ready-planning": ["ready", "planning"], + "inprogress-queued": ["inprogress", "queued"], + "done-closed": ["done", "closed"], +} as const; + +export type TicketLaneId = keyof typeof LANE_STATES; + +export type TicketLanePage = { + states: readonly string[]; + response: TicketListResponse; +}; + +export const load: PageLoad = async ({ fetch, params }) => { const workspaceId = params.workspaceId; - const [tickets, orchestrator] = await Promise.all([ - loadJson( - fetch, - `${workspaceApiPath(workspaceId, "/tickets")}?limit=1000`, + const [entries, orchestrator] = await Promise.all([ + Promise.all( + Object.entries(LANE_STATES).map(async ([laneId, states]) => { + const search = new URLSearchParams({ + limit: "30", + states: states.join(","), + }); + const response = await fetch( + `/api/w/${encodeURIComponent(workspaceId)}/tickets?${search}`, + ); + if (!response.ok) { + throw new Error( + `failed to load ${laneId} Ticket lane (${response.status})`, + ); + } + return [ + laneId, + { states: [...states], response: await response.json() }, + ] as const; + }), ), loadJson( fetch, @@ -16,5 +45,12 @@ export const load = (async ({ fetch, params }) => { ), ]); - return { workspaceId, tickets, orchestrator }; -}) satisfies PageLoad; + return { + workspaceId, + ticketLanes: Object.fromEntries(entries) as unknown as Record< + TicketLaneId, + TicketLanePage + >, + orchestrator, + }; +}; 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}