diff --git a/crates/ticket/src/assignment_tracking_tests.rs b/crates/ticket/src/assignment_tracking_tests.rs index f432636ee..a58951612 100644 --- a/crates/ticket/src/assignment_tracking_tests.rs +++ b/crates/ticket/src/assignment_tracking_tests.rs @@ -408,7 +408,7 @@ fn migration_preserves_original_records_and_only_reconciles_current_fenced_deleg |r| r.get::<_, i64>(0) ) .unwrap(), - 12 + 13 ); let value = sqlite_ticket_assignment_tracking(&conn, "w", "t", "a") .unwrap() diff --git a/crates/ticket/src/content_changes.rs b/crates/ticket/src/content_changes.rs new file mode 100644 index 000000000..499c8c35d --- /dev/null +++ b/crates/ticket/src/content_changes.rs @@ -0,0 +1,62 @@ +//! Immutable facts of committed edits, not notification decisions. The caller's write +//! transaction owns both the edit and this INSERT; hosts may capture assignment identity +//! with an INSERT trigger. No text snapshots or processing state are retained here. + +use rusqlite::{Connection, params}; + +use crate::{Result, sqlite_err}; + +pub(crate) enum ContentChange<'a> { + ContentEdit { + title_changed: bool, + body_event_ref: Option<&'a str>, + targets_changed: bool, + }, + CommentEdit { + event_ref: &'a str, + }, +} + +impl ContentChange<'_> { + pub(crate) fn insert( + self, + conn: &Connection, + workspace_id: &str, + ticket_id: &str, + actor: Option<&str>, + at: &str, + ) -> Result<()> { + let (kind, title_changed, source_event_ref, targets_changed) = match self { + Self::ContentEdit { + title_changed, + body_event_ref, + targets_changed, + } => ( + "content_edit", + title_changed, + body_event_ref, + targets_changed, + ), + Self::CommentEdit { event_ref } => ("comment_edit", false, Some(event_ref), false), + }; + conn.execute( + "INSERT INTO typed_ticket_content_changes + (change_id, workspace_id, ticket_id, kind, actor, at, + title_changed, body_changed, targets_changed, source_event_ref) + VALUES (lower(hex(randomblob(16))), ?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9)", + params![ + workspace_id, + ticket_id, + kind, + actor, + at, + title_changed, + source_event_ref.is_some(), + targets_changed, + source_event_ref + ], + ) + .map_err(sqlite_err)?; + Ok(()) + } +} diff --git a/crates/ticket/src/content_changes.sql b/crates/ticket/src/content_changes.sql new file mode 100644 index 000000000..784a3645f --- /dev/null +++ b/crates/ticket/src/content_changes.sql @@ -0,0 +1,20 @@ +-- Ordered component 12 -> 13. Facts have no delivery policy or mutable processing state. +CREATE TABLE typed_ticket_content_changes ( + change_id TEXT NOT NULL PRIMARY KEY CHECK (length(trim(change_id)) > 0), + workspace_id TEXT NOT NULL, + ticket_id TEXT NOT NULL, + kind TEXT NOT NULL CHECK (kind IN ('content_edit', 'comment_edit')), + actor TEXT CHECK (actor IS NULL OR length(trim(actor)) > 0), + at TEXT NOT NULL, + title_changed INTEGER NOT NULL CHECK (title_changed IN (0, 1)), + body_changed INTEGER NOT NULL CHECK (body_changed IN (0, 1)), + targets_changed INTEGER NOT NULL CHECK (targets_changed IN (0, 1)), + source_event_ref TEXT, + CHECK (title_changed + body_changed + targets_changed > 0), + CHECK ((body_changed = 0 AND source_event_ref IS NULL) OR + (body_changed = 1 AND source_event_ref IS NOT NULL AND length(trim(source_event_ref)) > 0)), + CHECK (kind != 'comment_edit' OR (title_changed = 0 AND body_changed = 1 AND targets_changed = 0)), + FOREIGN KEY (workspace_id, ticket_id) REFERENCES typed_tickets(workspace_id, ticket_id) ON DELETE CASCADE +); +CREATE TRIGGER typed_ticket_content_changes_no_update BEFORE UPDATE ON typed_ticket_content_changes +BEGIN SELECT RAISE(ABORT, 'Ticket content change facts are immutable'); END; diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index 62423f1f6..1253da2d2 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -29,6 +29,7 @@ mod assignment_answer; pub use assignment_answer::*; mod assignment_tracking; pub mod config; +mod content_changes; mod sqlite_schema; mod tags; pub use assignment_tracking::*; @@ -3116,6 +3117,9 @@ impl TicketBackend for SqliteTicketBackend { if post.body != edit.expected_body { return Err(TicketError::Conflict("stale Ticket post body".into())); } + if post.body == edit.body { + return Ok((previous.clone(), previous)); + } let now = now_utc(); let changed = conn.execute( "UPDATE typed_ticket_events SET body = ?4, edited_at = ?5, edited_by = ?6 WHERE workspace_id = ?1 AND ticket_id = ?2 AND event_ref = ?3 AND body = ?7 AND kind = 'comment' AND archived = 0", @@ -3123,6 +3127,16 @@ impl TicketBackend for SqliteTicketBackend { ).map_err(sqlite_err)?; if changed != 1 { return Err(TicketError::Conflict("stale Ticket post body".into())); } self.touch_ticket(conn, &ticket_id, &now)?; + let fact = if post.is_initial { + content_changes::ContentChange::ContentEdit { + title_changed: false, + body_event_ref: Some(&post.event_ref), + targets_changed: false, + } + } else { + content_changes::ContentChange::CommentEdit { event_ref: &post.event_ref } + }; + fact.insert(conn, &self.workspace_id, &ticket_id, Some(&edit.author), &now)?; let current = self.load_ticket(conn, &ticket_id)?; Ok((previous, current)) }) @@ -3315,37 +3329,41 @@ impl TicketBackend for SqliteTicketBackend { } let ticket_id = self.resolve_ticket_id(conn, id)?; let previous = self.load_ticket(conn, &ticket_id)?; - let now = now_utc(); - if let Some(title) = edit.title.as_ref() { - conn.execute("UPDATE typed_tickets SET title = ?3, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, title, now]).map_err(sqlite_err)?; - } - let mut updated_body = None; - if let Some(body) = edit.body.as_ref() { - updated_body = Some(body.clone()); - } + let mut updated_body = edit.body.clone(); if let Some(replacement) = edit.body_replacement.as_ref() { - let current_body: String = conn - .query_row( - "SELECT body FROM typed_ticket_events WHERE workspace_id = ?1 AND ticket_id = ?2 AND event_index = -1", - params![self.workspace_id, ticket_id], - |row| row.get(0), - ) - .map_err(sqlite_err)?; - let outcome = replacement.apply(current_body.as_str())?; - updated_body = Some(outcome.body); + updated_body = Some(replacement.apply(previous.document.body.as_str())?.body); } - if let Some(body) = updated_body.as_ref() { - conn.execute("UPDATE typed_ticket_events SET body = ?3, edited_at = ?4, edited_by = ?5 WHERE workspace_id = ?1 AND ticket_id = ?2 AND event_index = -1", params![self.workspace_id, ticket_id, body.as_str(), now, edit.author]).map_err(sqlite_err)?; - self.touch_ticket(conn, &ticket_id, &now)?; + let targets = edit.targets.as_ref().map(|targets| match targets { + TicketTargetsEdit::Set { targets } => targets.as_slice(), + TicketTargetsEdit::Clear => &[], + }); + let title_changed = edit.title.as_ref().is_some_and(|title| *title != previous.meta.title); + let body_changed = updated_body.as_ref().is_some_and(|body| *body != previous.document.body); + let targets_changed = targets.is_some_and(|targets| targets != previous.meta.targets); + if !title_changed && !body_changed && !targets_changed { + return Ok((previous.clone(), previous)); } - if let Some(targets) = edit.targets.as_ref() { - let targets = match targets { - TicketTargetsEdit::Set { targets } => targets.as_slice(), - TicketTargetsEdit::Clear => &[], - }; - self.replace_ticket_targets(conn, &ticket_id, targets)?; - self.touch_ticket(conn, &ticket_id, &now)?; + let now = now_utc(); + if title_changed { + conn.execute("UPDATE typed_tickets SET title = ?3 WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, edit.title]).map_err(sqlite_err)?; } + let body_event_ref = if body_changed { + let initial = previous.events.iter().find(|event| event.is_initial) + .ok_or_else(|| TicketError::Conflict("Ticket initial post is missing".into()))?; + conn.execute("UPDATE typed_ticket_events SET body = ?3, edited_at = ?4, edited_by = ?5 WHERE workspace_id = ?1 AND ticket_id = ?2 AND event_index = -1", params![self.workspace_id, ticket_id, updated_body.as_ref().expect("changed body").as_str(), now, edit.author]).map_err(sqlite_err)?; + Some(initial.event_ref.as_str()) + } else { + None + }; + if targets_changed { + self.replace_ticket_targets(conn, &ticket_id, targets.expect("changed targets"))?; + } + self.touch_ticket(conn, &ticket_id, &now)?; + content_changes::ContentChange::ContentEdit { + title_changed, + body_event_ref, + targets_changed, + }.insert(conn, &self.workspace_id, &ticket_id, edit.author.as_deref(), &now)?; let current = self.load_ticket(conn, &ticket_id)?; Ok((previous, current)) diff --git a/crates/ticket/src/sqlite_schema.rs b/crates/ticket/src/sqlite_schema.rs index 431f824d0..1af6049ee 100644 --- a/crates/ticket/src/sqlite_schema.rs +++ b/crates/ticket/src/sqlite_schema.rs @@ -20,7 +20,7 @@ use crate::{Result, TicketError, sqlite_err}; const MIGRATION_TABLE: &str = "ticket_schema_migrations"; const MAX_SCHEMA_DIAGNOSTICS: usize = 32; -const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 12; +const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 13; const BASELINE_MIGRATION_NAME: &str = "ticket schema baseline"; #[derive(Clone, Copy)] @@ -320,11 +320,10 @@ pub fn migrate_sqlite_ticket_schema(connection: &Connection) -> Result<()> { } } -/// Composes retained 6/7, tags-only 8 or cases-only 9 through component 10, then -/// appends the 10→11 assignment and 11→12 post migrations without replacing applied markers. -/// The host owns the surrounding transaction (for example Workspace 91→92). +/// Appends assignment, post, and content-change fact migrations without replacing applied markers. +/// The host owns the surrounding transaction. /// A savepoint rolls back this component on failure, without committing host work. -/// Current component 12 is verified only; immutable judgments are never reapplied. +/// Current component 13 is verified only; historical edits are never backfilled. pub fn migrate_sqlite_ticket_schema_in_transaction(connection: &Connection) -> Result<()> { if connection.is_autocommit() { return Err(TicketError::Sqlite( @@ -348,7 +347,7 @@ pub fn migrate_sqlite_ticket_schema_in_transaction(connection: &Connection) -> R create_latest_ticket_schema(connection)?; connection .execute( - "INSERT INTO ticket_schema_migrations VALUES (12, ?1, ?2)", + "INSERT INTO ticket_schema_migrations VALUES (13, ?1, ?2)", params![BASELINE_MIGRATION_NAME, chrono::Utc::now().to_rfc3339()], ) .map_err(sqlite_err)?; @@ -374,7 +373,8 @@ pub fn migrate_sqlite_ticket_schema_in_transaction(connection: &Connection) -> R TicketSchemaShape::Cases9 => add_tag_schema(connection)?, TicketSchemaShape::Composed10 | TicketSchemaShape::Tracking11 - | TicketSchemaShape::Posts12 => {} + | TicketSchemaShape::Posts12 + | TicketSchemaShape::ContentChanges13 => {} } if version < 10 { connection @@ -392,6 +392,10 @@ pub fn migrate_sqlite_ticket_schema_in_transaction(connection: &Connection) -> R migrate_posts(connection)?; connection.execute("INSERT INTO ticket_schema_migrations VALUES (12, 'Canonical Ticket posts', ?1)", params![chrono::Utc::now().to_rfc3339()]).map_err(sqlite_err)?; } + if version < 13 { + migrate_content_changes(connection)?; + connection.execute("INSERT INTO ticket_schema_migrations VALUES (13, 'Ticket content change facts', ?1)", params![chrono::Utc::now().to_rfc3339()]).map_err(sqlite_err)?; + } } verify_sqlite_ticket_schema(connection) })(); @@ -416,6 +420,7 @@ enum TicketSchemaShape { Composed10, Tracking11, Posts12, + ContentChanges13, } impl TicketSchemaShape { fn from_version(version: i64) -> Self { @@ -426,6 +431,7 @@ impl TicketSchemaShape { 10 => Self::Composed10, 11 => Self::Tracking11, 12 => Self::Posts12, + 13 => Self::ContentChanges13, _ => unreachable!(), } } @@ -437,25 +443,34 @@ impl TicketSchemaShape { Self::Composed10 => 10, Self::Tracking11 => 11, Self::Posts12 => 12, + Self::ContentChanges13 => 13, } } fn tags(self) -> bool { matches!( self, - Self::Tags8 | Self::Composed10 | Self::Tracking11 | Self::Posts12 + Self::Tags8 + | Self::Composed10 + | Self::Tracking11 + | Self::Posts12 + | Self::ContentChanges13 ) } fn cases(self) -> bool { matches!( self, - Self::Cases9 | Self::Composed10 | Self::Tracking11 | Self::Posts12 + Self::Cases9 + | Self::Composed10 + | Self::Tracking11 + | Self::Posts12 + | Self::ContentChanges13 ) } } /// Verifies the current Ticket-owned SQLite schema without executing DDL. pub fn verify_sqlite_ticket_schema(connection: &Connection) -> Result<()> { - verify_ticket_schema_shape(connection, TicketSchemaShape::Posts12) + verify_ticket_schema_shape(connection, TicketSchemaShape::ContentChanges13) } /// Retained Workspace schema-66 migration produces the pre-classification v7 @@ -569,7 +584,10 @@ fn verify_ticket_schema_shape(connection: &Connection, shape: TicketSchemaShape) continue; } let mut columns = columns.to_vec(); - if matches!(shape, TicketSchemaShape::Posts12) { + if matches!( + shape, + TicketSchemaShape::Posts12 | TicketSchemaShape::ContentChanges13 + ) { if table == "typed_tickets" { columns.retain(|c| c.name != "body"); columns.push(column("created_by", "TEXT", false, 0)); @@ -810,7 +828,9 @@ fn verify_ticket_schema_shape(connection: &Connection, shape: TicketSchemaShape) if matches!( shape, - TicketSchemaShape::Tracking11 | TicketSchemaShape::Posts12 + TicketSchemaShape::Tracking11 + | TicketSchemaShape::Posts12 + | TicketSchemaShape::ContentChanges13 ) { if let Err(error) = crate::assignment_tracking::verify_assignment_tracking_schema(connection) @@ -818,7 +838,10 @@ fn verify_ticket_schema_shape(connection: &Connection, shape: TicketSchemaShape) push_diagnostic(&mut diagnostics, error.to_string()); } } - if matches!(shape, TicketSchemaShape::Posts12) { + if matches!( + shape, + TicketSchemaShape::Posts12 | TicketSchemaShape::ContentChanges13 + ) { collect_index_diagnostics( connection, "typed_ticket_events", @@ -828,6 +851,9 @@ fn verify_ticket_schema_shape(connection: &Connection, shape: TicketSchemaShape) &mut diagnostics, ); } + if matches!(shape, TicketSchemaShape::ContentChanges13) { + collect_content_change_diagnostics(connection, &mut diagnostics)?; + } if diagnostics.is_empty() { Ok(()) } else { @@ -849,7 +875,90 @@ fn create_latest_ticket_schema(connection: &Connection) -> Result<()> { .execute_batch(include_str!("latest_schema.sql")) .map_err(sqlite_err)?; crate::assignment_tracking::migrate_assignment_tracking(connection)?; - migrate_posts(connection) + migrate_posts(connection)?; + migrate_content_changes(connection) +} + +fn migrate_content_changes(connection: &Connection) -> Result<()> { + connection + .execute_batch(include_str!("content_changes.sql")) + .map_err(sqlite_err) +} + +fn collect_content_change_diagnostics( + connection: &Connection, + diagnostics: &mut Vec, +) -> Result<()> { + const TABLE: &str = "typed_ticket_content_changes"; + collect_table_diagnostics( + connection, + TABLE, + &[ + column("change_id", "TEXT", true, 1), + column("workspace_id", "TEXT", true, 0), + column("ticket_id", "TEXT", true, 0), + column("kind", "TEXT", true, 0), + column("actor", "TEXT", false, 0), + column("at", "TEXT", true, 0), + column("title_changed", "INTEGER", true, 0), + column("body_changed", "INTEGER", true, 0), + column("targets_changed", "INTEGER", true, 0), + column("source_event_ref", "TEXT", false, 0), + ], + CHILD_FOREIGN_KEYS, + diagnostics, + ); + // These new objects are wholly Ticket-owned. Compare their complete definitions, + // including CHECK predicates and the composite FK, not just column affinities. + // Additional host triggers are independent and remain permitted. + let ddl = include_str!("content_changes.sql"); + let table_sql = format!( + "CREATE TABLE {}", + ddl.split_once("CREATE TABLE ") + .expect("table DDL") + .1 + .split_once(";\n") + .expect("table terminator") + .0 + ); + let trigger_sql = format!( + "CREATE TRIGGER {}", + ddl.split_once("CREATE TRIGGER ") + .expect("trigger DDL") + .1 + .trim() + ); + for (kind, name, expected) in [ + ("table", TABLE, table_sql), + ( + "trigger", + "typed_ticket_content_changes_no_update", + trigger_sql, + ), + ] { + let actual: Option = connection + .query_row( + "SELECT sql FROM sqlite_master WHERE type = ?1 AND name = ?2", + params![kind, name], + |row| row.get(0), + ) + .optional() + .map_err(sqlite_err)?; + let normalize = |sql: &str| { + sql.trim() + .trim_end_matches(';') + .split_whitespace() + .collect::() + }; + if actual.as_deref().map(normalize) != Some(normalize(&expected)) { + push_diagnostic( + diagnostics, + format!("invalid Ticket content change {kind} {name}"), + ); + } + } + collect_foreign_key_check_diagnostics(connection, TABLE, diagnostics); + Ok(()) } fn migrate_posts(connection: &Connection) -> Result<()> { @@ -1105,22 +1214,25 @@ fn load_applied_migrations(connection: &Connection) -> Result) -> Result { if applied.len() == 1 { if let Some((&version, name)) = applied.first_key_value() { - if (6..=12).contains(&version) && name == BASELINE_MIGRATION_NAME { + if (6..=13).contains(&version) && name == BASELINE_MIGRATION_NAME { return Ok(version); } } } - for baseline in [10, 11] { + for baseline in [10, 11, 12] { let mut expected = BTreeMap::from([(baseline, BASELINE_MIGRATION_NAME.to_owned())]); - if baseline == 10 { - expected.insert(11, "Ticket-owned assignment tracking".to_owned()); + for (version, name) in [ + (11, "Ticket-owned assignment tracking"), + (12, "Canonical Ticket posts"), + (13, "Ticket content change facts"), + ] { + if version <= baseline { + continue; + } + expected.insert(version, name.to_owned()); if applied == &expected { - return Ok(11); + return Ok(version); } - } - expected.insert(12, "Canonical Ticket posts".to_owned()); - if applied == &expected { - return Ok(12); } } Err(TicketError::Sqlite(format!( @@ -1623,7 +1735,8 @@ mod tests { BTreeMap::from([ (10, BASELINE_MIGRATION_NAME.to_owned()), (11, "Ticket-owned assignment tracking".to_owned()), - (12, "Canonical Ticket posts".to_owned()) + (12, "Canonical Ticket posts".to_owned()), + (13, "Ticket content change facts".to_owned()) ]) ); } @@ -1766,7 +1879,7 @@ mod tests { let error = migrate_sqlite_ticket_schema(&connection).unwrap_err(); assert!(error.to_string().contains( - "migration history must contain a canonical baseline and ordered migrations through 12" + "migration history must contain a canonical baseline and ordered migrations through 13" )); assert_eq!(load_applied_migrations(&connection).unwrap().len(), 2); } @@ -1788,7 +1901,7 @@ mod tests { let error = migrate_sqlite_ticket_schema(&connection).unwrap_err(); assert!(error.to_string().contains( - "migration history must contain a canonical baseline and ordered migrations through 12" + "migration history must contain a canonical baseline and ordered migrations through 13" )); assert!(!table_exists(&connection, "typed_tickets").unwrap()); } diff --git a/crates/ticket/tests/posts.rs b/crates/ticket/tests/posts.rs index 8d71d3049..516717ce6 100644 --- a/crates/ticket/tests/posts.rs +++ b/crates/ticket/tests/posts.rs @@ -1,3 +1,9 @@ +//! Real SQLite component tests through TicketBackend, including durable edit facts. +//! Host capture is represented only by test triggers; Workspace routing is not tested here. + +#[path = "posts/content_changes.rs"] +mod content_changes; + use rusqlite::{Connection, params}; use tempfile::TempDir; use ticket::{ @@ -264,7 +270,7 @@ fn schema12_migrates_body_authors_archived_history_and_stable_references_then_re |r| r.get::<_, i64>(0) ) .unwrap(), - 12 + 13 ); backend .add_event("known".into(), comment("Next", "author")) diff --git a/crates/ticket/tests/posts/content_changes.rs b/crates/ticket/tests/posts/content_changes.rs new file mode 100644 index 000000000..40103bca9 --- /dev/null +++ b/crates/ticket/tests/posts/content_changes.rs @@ -0,0 +1,609 @@ +use super::*; +use ticket::{TicketBodyReplacement, TicketTarget, TicketTargetAccess, TicketTargetsEdit}; + +struct Fixture { + _temp: TempDir, + path: std::path::PathBuf, + backend: SqliteTicketBackend, + conn: Connection, + id: String, +} + +impl Fixture { + fn new() -> Self { + let temp = TempDir::new().unwrap(); + let path = temp.path().join("facts.db"); + let backend = SqliteTicketBackend::open(&path, "workspace").unwrap(); + let mut input = NewTicket::new("Original title"); + input.body = MarkdownText::new("Private original body"); + input.author = Some("creator".into()); + let id = backend.create(input).unwrap().id; + let conn = Connection::open(&path).unwrap(); + conn.execute_batch("PRAGMA foreign_keys=ON").unwrap(); + Self { + _temp: temp, + path, + backend, + conn, + id, + } + } + + fn ticket(&self) -> ticket::Ticket { + self.backend.show(self.id.clone().into()).unwrap() + } + + fn edit(&self, edit: TicketItemEdit) -> ticket::Ticket { + self.backend + .edit_item(self.id.clone().into(), edit) + .unwrap() + } + + fn facts(&self) -> Vec { + self.conn.prepare("SELECT change_id,workspace_id,ticket_id,kind,actor,at,title_changed,body_changed,targets_changed,source_event_ref FROM typed_ticket_content_changes ORDER BY rowid").unwrap() + .query_map([], |r| Ok(Fact { + id: r.get(0)?, workspace: r.get(1)?, ticket: r.get(2)?, kind: r.get(3)?, actor: r.get(4)?, at: r.get(5)?, + title: r.get(6)?, body: r.get(7)?, targets: r.get(8)?, source: r.get(9)?, + })).unwrap().collect::>().unwrap() + } +} + +#[derive(Debug, PartialEq, Eq)] +struct Fact { + id: String, + workspace: String, + ticket: String, + kind: String, + actor: Option, + at: String, + title: bool, + body: bool, + targets: bool, + source: Option, +} + +fn targets(names: &[&str]) -> TicketTargetsEdit { + TicketTargetsEdit::Set { + targets: names + .iter() + .map(|name| TicketTarget { + repository_key: (*name).into(), + ref_selector: None, + access: TicketTargetAccess::ReadWrite, + }) + .collect(), + } +} + +#[test] +fn item_edits_record_actual_fields_and_contemporaneous_actor_once_without_text() { + let f = Fixture::new(); + assert!(f.facts().is_empty()); + let original = f.ticket(); + let edits = [ + ( + TicketItemEdit { + title: Some("Changed title".into()), + ..Default::default() + }, + (true, false, false), + ), + ( + TicketItemEdit { + targets: Some(targets(&["main", "other"])), + ..Default::default() + }, + (false, false, true), + ), + ( + TicketItemEdit { + targets: Some(targets(&["other", "main"])), + ..Default::default() + }, + (false, false, true), + ), + ( + TicketItemEdit { + title: Some("Combined title".into()), + body: Some(MarkdownText::new("Private replacement body")), + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }, + (true, true, true), + ), + ( + TicketItemEdit { + title: Some("Combined title".into()), + body_replacement: Some(TicketBodyReplacement { + old_string: "replacement".into(), + new_string: "updated".into(), + replace_all: false, + }), + ..Default::default() + }, + (false, true, false), + ), + ]; + for (index, (mut edit, flags)) in edits.into_iter().enumerate() { + let actor = format!("editor-{index}"); + edit.author = Some(actor.clone()); + let current = f.edit(edit); + let facts = f.facts(); + assert_eq!(facts.len(), index + 1); + let fact = facts.last().unwrap(); + assert_eq!(fact.workspace, "workspace"); + assert_eq!(fact.ticket, f.id); + assert_eq!(fact.kind, "content_edit"); + assert_eq!(fact.actor.as_deref(), Some(actor.as_str())); + assert_eq!((fact.title, fact.body, fact.targets), flags); + assert_eq!( + fact.source.as_deref(), + flags.1.then_some(original.events[0].event_ref.as_str()) + ); + assert_eq!(Some(fact.at.as_str()), current.meta.updated_at.as_deref()); + assert_eq!(current.events[0].author, original.events[0].author); + } + let facts = f.facts(); + assert_eq!( + facts + .iter() + .map(|f| &f.id) + .collect::>() + .len(), + facts.len() + ); + // The schema itself excludes content snapshots, not just this particular INSERT. + let columns: Vec = f + .conn + .prepare("SELECT name FROM pragma_table_info('typed_ticket_content_changes')") + .unwrap() + .query_map([], |r| r.get(0)) + .unwrap() + .collect::>() + .unwrap(); + assert_eq!( + columns, + [ + "change_id", + "workspace_id", + "ticket_id", + "kind", + "actor", + "at", + "title_changed", + "body_changed", + "targets_changed", + "source_event_ref" + ] + ); + let reopened = SqliteTicketBackend::open(&f.path, "workspace").unwrap(); + assert_eq!(reopened.show(f.id.clone().into()).unwrap(), f.ticket()); + assert_eq!(f.facts(), facts); + f.edit(TicketItemEdit { + title: Some("Unknown local editor".into()), + ..Default::default() + }); + assert_eq!(f.facts().last().unwrap().actor, None); +} + +#[test] +fn post_edits_distinguish_initial_and_discussion_and_preserve_prior_edit_actors() { + let f = Fixture::new(); + f.backend + .add_event( + f.id.clone().into(), + comment("Private discussion", "commenter"), + ) + .unwrap(); + assert!(f.facts().is_empty()); + let before = f.ticket(); + for (index, original) in before.events.iter().enumerate() { + let current = f + .backend + .edit_post( + f.id.clone().into(), + TicketPostEdit { + event_ref: original.event_ref.clone(), + expected_body: original.body.clone(), + body: MarkdownText::new(format!("Private edited post {index}")), + author: format!("post-editor-{index}"), + }, + ) + .unwrap(); + let facts = f.facts(); + assert_eq!(facts.len(), index + 1); + let fact = facts.last().unwrap(); + assert_eq!( + fact.kind, + if original.is_initial { + "content_edit" + } else { + "comment_edit" + } + ); + assert_eq!((fact.title, fact.body, fact.targets), (false, true, false)); + assert_eq!(fact.source.as_ref(), Some(&original.event_ref)); + assert_eq!( + fact.actor.as_deref(), + Some(format!("post-editor-{index}").as_str()) + ); + assert_eq!( + current.events[index].edited_at.as_deref(), + Some(fact.at.as_str()) + ); + } + let facts = f.facts(); + let post = &f.ticket().events[0]; + f.backend + .edit_post( + f.id.clone().into(), + TicketPostEdit { + event_ref: post.event_ref.clone(), + expected_body: post.body.clone(), + body: MarkdownText::new("Another private edit"), + author: "later-editor".into(), + }, + ) + .unwrap(); + assert_eq!(&f.facts()[..2], facts.as_slice()); +} + +#[test] +fn noops_preserve_metadata_and_do_not_bypass_validation_or_emit_facts() { + let f = Fixture::new(); + f.edit(TicketItemEdit { + targets: Some(targets(&["main"])), + author: Some("first".into()), + ..Default::default() + }); + let before = f.ticket(); + let facts = f.facts(); + for edit in [ + TicketItemEdit { + title: Some(before.meta.title.clone()), + ..Default::default() + }, + TicketItemEdit { + body: Some(before.document.body.clone()), + ..Default::default() + }, + TicketItemEdit { + targets: Some(targets(&["main"])), + ..Default::default() + }, + TicketItemEdit { + title: Some(before.meta.title.clone()), + body: Some(before.document.body.clone()), + targets: Some(targets(&["main"])), + ..Default::default() + }, + ] { + assert_eq!( + f.edit(TicketItemEdit { + author: Some("noop-editor".into()), + ..edit + }), + before + ); + } + let post = &before.events[0]; + let edit = TicketPostEdit { + event_ref: post.event_ref.clone(), + expected_body: post.body.clone(), + body: post.body.clone(), + author: "noop-editor".into(), + }; + assert_eq!( + f.backend + .edit_post(f.id.clone().into(), edit.clone()) + .unwrap(), + before + ); + assert!( + f.backend + .edit_post( + f.id.clone().into(), + TicketPostEdit { + expected_body: MarkdownText::new("stale"), + ..edit + } + ) + .is_err() + ); + assert!( + f.backend + .edit_item( + f.id.clone().into(), + TicketItemEdit { + title: Some(before.meta.title.clone()), + author: Some(" ".into()), + ..Default::default() + } + ) + .is_err() + ); + assert_eq!(f.facts(), facts); + assert_eq!(f.ticket(), before); + f.edit(TicketItemEdit { + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }); + let empty = f.ticket(); + let facts = f.facts(); + assert_eq!( + f.edit(TicketItemEdit { + targets: Some(TicketTargetsEdit::Clear), + ..Default::default() + }), + empty + ); + assert_eq!(f.facts(), facts); +} + +#[test] +fn failed_host_capture_rolls_back_edits_fact_and_trigger_writes_together() { + let f = Fixture::new(); + f.conn.execute_batch("CREATE TABLE host_capture(change_id TEXT, actor TEXT); CREATE TRIGGER capture_fact AFTER INSERT ON typed_ticket_content_changes BEGIN INSERT INTO host_capture VALUES(NEW.change_id, NEW.actor); SELECT RAISE(ABORT, 'capture failed'); END;").unwrap(); + let before = f.ticket(); + assert!( + f.backend + .edit_item( + f.id.clone().into(), + TicketItemEdit { + title: Some("Changed title".into()), + body: Some(MarkdownText::new("Changed body")), + targets: Some(targets(&["main"])), + author: Some("item-editor".into()), + ..Default::default() + } + ) + .is_err() + ); + let post = &before.events[0]; + assert!( + f.backend + .edit_post( + f.id.clone().into(), + TicketPostEdit { + event_ref: post.event_ref.clone(), + expected_body: post.body.clone(), + body: MarkdownText::new("Changed post"), + author: "post-editor".into(), + } + ) + .is_err() + ); + assert_eq!(f.ticket(), before); + assert!(f.facts().is_empty()); + assert_eq!( + f.conn + .query_row("SELECT count(*) FROM host_capture", [], |r| r + .get::<_, i64>(0)) + .unwrap(), + 0 + ); + f.conn.execute_batch("DROP TRIGGER capture_fact; CREATE TRIGGER capture_fact AFTER INSERT ON typed_ticket_content_changes BEGIN INSERT INTO host_capture VALUES(NEW.change_id, NEW.actor); END;").unwrap(); + f.edit(TicketItemEdit { + title: Some("Successful title".into()), + author: Some("actual-editor".into()), + ..Default::default() + }); + let fact = f.facts().remove(0); + assert_eq!( + f.conn + .query_row("SELECT change_id, actor FROM host_capture", [], |r| Ok(( + r.get::<_, String>(0)?, + r.get::<_, String>(1)? + ))) + .unwrap(), + (fact.id, "actual-editor".into()) + ); +} + +fn schema12(conn: &Connection) { + super::schema11(conn); + super::seed_legacy(conn, "retained"); + conn.execute_batch(include_str!("../../src/post_migration.sql")) + .unwrap(); + conn.execute_batch("INSERT INTO ticket_schema_migrations VALUES(12,'Canonical Ticket posts','original-post-marker');").unwrap(); +} + +fn schema(conn: &Connection) -> Vec<(String, String, String)> { + conn.prepare("SELECT type,name,sql FROM sqlite_master WHERE sql IS NOT NULL ORDER BY type,name") + .unwrap() + .query_map([], |r| { + Ok(( + r.get(0)?, + r.get(1)?, + r.get::<_, String>(2)? + .split_whitespace() + .collect::(), + )) + }) + .unwrap() + .collect::>() + .unwrap() +} + +#[test] +fn content_change_upgrade_matches_fresh_schema_without_backfill_and_preserves_markers() { + let upgraded = Connection::open_in_memory().unwrap(); + upgraded + .execute_batch("CREATE TABLE workspaces(workspace_id TEXT PRIMARY KEY);") + .unwrap(); + schema12(&upgraded); + migrate_sqlite_ticket_schema(&upgraded).unwrap(); + let fresh = Connection::open_in_memory().unwrap(); + fresh + .execute_batch("CREATE TABLE workspaces(workspace_id TEXT PRIMARY KEY);") + .unwrap(); + migrate_sqlite_ticket_schema(&fresh).unwrap(); + let upgraded_schema = schema(&upgraded); + let fresh_schema = schema(&fresh); + assert_eq!(upgraded_schema.len(), fresh_schema.len()); + for (upgraded_object, fresh_object) in upgraded_schema.iter().zip(&fresh_schema) { + assert_eq!(upgraded_object, fresh_object); + } + assert_eq!( + upgraded + .query_row( + "SELECT count(*) FROM typed_ticket_content_changes", + [], + |r| r.get::<_, i64>(0) + ) + .unwrap(), + 0 + ); + assert_eq!( + upgraded + .query_row( + "SELECT body FROM typed_ticket_events WHERE event_index=-1", + [], + |r| r.get::<_, String>(0) + ) + .unwrap(), + "Latest body" + ); + assert_eq!( + upgraded + .query_row( + "SELECT applied_at FROM ticket_schema_migrations WHERE version=12", + [], + |r| r.get::<_, String>(0) + ) + .unwrap(), + "original-post-marker" + ); + let markers: Vec<(i64, String, String)> = upgraded + .prepare("SELECT version,name,applied_at FROM ticket_schema_migrations ORDER BY version") + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?))) + .unwrap() + .collect::>() + .unwrap(); + migrate_sqlite_ticket_schema(&upgraded).unwrap(); + let after: Vec<(i64, String, String)> = upgraded + .prepare("SELECT version,name,applied_at FROM ticket_schema_migrations ORDER BY version") + .unwrap() + .query_map([], |r| Ok((r.get(0)?, r.get(1)?, r.get(2)?))) + .unwrap() + .collect::>() + .unwrap(); + assert_eq!(markers, after); +} + +#[test] +fn failed_content_change_migration_rolls_back_ddl_and_marker_without_committing_host() { + let conn = Connection::open_in_memory().unwrap(); + schema12(&conn); + conn.execute_batch("CREATE TABLE host_work(value TEXT); CREATE TRIGGER typed_ticket_content_changes_no_update BEFORE UPDATE ON host_work BEGIN SELECT RAISE(ABORT, 'collision'); END;").unwrap(); + let before = schema(&conn); + conn.execute_batch("BEGIN IMMEDIATE; INSERT INTO host_work VALUES('pending');") + .unwrap(); + assert!(migrate_sqlite_ticket_schema_in_transaction(&conn).is_err()); + assert!(!conn.is_autocommit()); + assert_eq!(schema(&conn), before); + assert_eq!( + conn.query_row( + "SELECT max(version) FROM ticket_schema_migrations", + [], + |r| r.get::<_, i64>(0) + ) + .unwrap(), + 12 + ); + assert_eq!( + conn.query_row("SELECT count(*) FROM host_work", [], |r| r.get::<_, i64>(0)) + .unwrap(), + 1 + ); + conn.execute_batch("ROLLBACK; DROP TRIGGER typed_ticket_content_changes_no_update;") + .unwrap(); + migrate_sqlite_ticket_schema(&conn).unwrap(); + verify_sqlite_ticket_schema(&conn).unwrap(); +} + +#[test] +fn fact_constraints_reject_invalid_shapes_updates_and_cross_workspace_parent() { + let f = Fixture::new(); + let insert = |id: &str, + workspace: &str, + kind: &str, + title: i64, + body: i64, + targets: i64, + source: Option<&str>| { + f.conn.execute("INSERT INTO typed_ticket_content_changes VALUES(?1,?2,?3,?4,'actor','time',?5,?6,?7,?8)", params![id, workspace, f.id, kind, title, body, targets, source]) + }; + for (kind, title, body, targets, source) in [ + ("unknown", 1, 0, 0, None), + ("content_edit", 0, 0, 0, None), + ("content_edit", 2, 0, 0, None), + ("content_edit", 0, 2, 0, Some("post")), + ("content_edit", 0, 0, 2, None), + ("content_edit", 1, 0, 0, Some("post")), + ("content_edit", 0, 1, 0, None), + ("content_edit", 0, 1, 0, Some(" ")), + ("comment_edit", 1, 1, 0, Some("post")), + ("comment_edit", 0, 1, 1, Some("post")), + ] { + assert!(insert("invalid", "workspace", kind, title, body, targets, source).is_err()); + } + assert!(insert("foreign", "other-workspace", "content_edit", 1, 0, 0, None).is_err()); + insert("valid", "workspace", "content_edit", 1, 0, 0, None).unwrap(); + assert!(insert("valid", "workspace", "content_edit", 1, 0, 0, None).is_err()); + assert!( + f.conn + .execute( + "UPDATE typed_ticket_content_changes SET actor='changed'", + [] + ) + .is_err() + ); + f.conn + .execute("DELETE FROM typed_ticket_content_changes", []) + .unwrap(); + assert!(f.facts().is_empty()); +} + +#[test] +fn verifier_rejects_weakened_fact_constraints_trigger_and_migration_history() { + for (from, to) in [ + ("CHECK (kind IN ('content_edit', 'comment_edit'))", ""), + ("CHECK (title_changed IN (0, 1))", ""), + ("CHECK (body_changed IN (0, 1))", ""), + ("CHECK (targets_changed IN (0, 1))", ""), + ( + "CHECK (title_changed + body_changed + targets_changed > 0),", + "", + ), + ( + "REFERENCES typed_tickets(workspace_id, ticket_id)", + "REFERENCES typed_tickets(ticket_id, workspace_id)", + ), + ("ON DELETE CASCADE", "ON DELETE RESTRICT"), + ( + "SELECT RAISE(ABORT, 'Ticket content change facts are immutable')", + "SELECT 1", + ), + ] { + let f = Fixture::new(); + f.conn + .execute_batch("DROP TABLE typed_ticket_content_changes") + .unwrap(); + let sql = include_str!("../../src/content_changes.sql").replace(from, to); + f.conn.execute_batch(&sql).unwrap(); + assert!(verify_sqlite_ticket_schema(&f.conn).is_err(), "{from}"); + assert!(migrate_sqlite_ticket_schema(&f.conn).is_err(), "{from}"); + } + for mutation in [ + "UPDATE ticket_schema_migrations SET name='unrecognized' WHERE version=13", + "INSERT INTO ticket_schema_migrations VALUES(12,'Canonical Ticket posts','invalid extra history')", + "UPDATE ticket_schema_migrations SET version=14 WHERE version=13", + ] { + let f = Fixture::new(); + f.conn.execute_batch(mutation).unwrap(); + assert!(verify_sqlite_ticket_schema(&f.conn).is_err()); + assert!(migrate_sqlite_ticket_schema(&f.conn).is_err()); + } +} diff --git a/crates/workspace-server/src/assignment_delivery.rs b/crates/workspace-server/src/assignment_delivery.rs index efee9e1cd..6ce5bc790 100644 --- a/crates/workspace-server/src/assignment_delivery.rs +++ b/crates/workspace-server/src/assignment_delivery.rs @@ -1,13 +1,14 @@ //! Durable operational delivery and startup fencing, independent of Ticket handling. //! -//! All delivery triggers use the same pending-input scan: current actionable delegation, -//! committed input origin and open case, never Queue/open state alone. Waiting is not a -//! resend reason; comments, self-reports and case/routing history are not new requests. -//! Migration preserves evidence but creates origins only for reconciled assignment-specific -//! legacy Queue fences. Unknown/history/done/released assignments are not fresh work. +//! Notification eligibility comes from the shared routing policy and its persisted decisions. +//! This adapter retains the original assignment delivery identity, immutable request and ACK. +//! Retained migration SQL establishes origins only for reconciled assignment-specific legacy +//! Queue fences; migration does not reinterpret historical comments as new requests. //! -//! Reserve, prepare and acknowledge recheck delegation, singleton and live binding under -//! IMMEDIATE ownership. Persist the entire rendered request before Runtime I/O, then reuse +//! Reserve and prepare recheck delegation, singleton and live binding under +//! IMMEDIATE ownership. ACK instead verifies the target authorized before I/O; +//! a later responsibility change must not erase Runtime acceptance evidence. +//! Persist the entire rendered request before Runtime I/O, then reuse //! its request ID and content on response loss. Send outside the transaction; acknowledgement //! proves Runtime acceptance, not model consideration, execution, handling or fulfillment. //! Async callers offload whole helpers, never split authorization/receipt/CAS/audit across awaits. @@ -132,46 +133,6 @@ pub(super) fn install_post_inputs(conn: &Connection) -> Result<()> { Ok(()) } -const INPUTS_SQL: &str = r#" -WITH eligible AS ( - SELECT a.*,o.input_event_id,o.after_ticket_event_index - FROM ticket_current_worker_assignments c - JOIN ticket_worker_assignments a ON a.workspace_id=c.workspace_id AND a.assignment_id=c.assignment_id - JOIN ticket_assignment_delivery_origins o ON o.workspace_id=a.workspace_id AND o.assignment_id=a.assignment_id - JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id - WHERE a.workspace_id=?1 AND a.role='orchestrator' AND a.principal_kind='workspace_agent' - AND a.principal_id='workspace-orchestrator' AND t.workflow_state='open' - AND json_extract(a.tracking_json,'$.status') IN ('waiting','inprogress') - AND NOT EXISTS(SELECT 1 FROM ticket_assignment_work_releases r - WHERE r.workspace_id=a.workspace_id AND r.assignment_id=a.assignment_id) -), inputs AS ( - SELECT ticket_id,assignment_id,input_event_id,tracking_json,-1 AS input_order FROM eligible - UNION ALL - SELECT a.ticket_id,a.assignment_id,'ticket-event:' || e.event_index,a.tracking_json,e.event_index - FROM eligible a JOIN typed_ticket_events e ON e.workspace_id=a.workspace_id AND e.ticket_id=a.ticket_id - WHERE e.event_index>a.after_ticket_event_index - -- Continuing request intent is explicit: fenced assignment answers or item content edits. - -- Ordinary comments and all case/routing/handling history are not new requests. - AND e.kind IN ('assignment_answer','item_edit') - AND (e.kind<>'assignment_answer' OR EXISTS(SELECT 1 FROM typed_ticket_event_attributes attr WHERE attr.workspace_id=e.workspace_id AND attr.ticket_id=e.ticket_id AND attr.event_index=e.event_index AND attr.key='orchestrator_assignment_id' AND attr.value=a.assignment_id)) - AND e.kind NOT LIKE 'assignment_tracking%' - AND e.kind NOT LIKE 'assignment_delivery%' - AND COALESCE(e.state_field,'') NOT IN ('case','routing','assignment_tracking') - AND NOT EXISTS(SELECT 1 FROM typed_ticket_event_attributes attr WHERE attr.workspace_id=e.workspace_id AND attr.ticket_id=e.ticket_id AND attr.event_index=e.event_index AND attr.key='orchestrator_assignment_id' AND attr.value<>a.assignment_id) - UNION ALL - SELECT a.ticket_id,a.assignment_id,'ticket-edit:' || printf('%020d',e.input_sequence),a.tracking_json,e.after_ticket_event_index+1 - FROM eligible a JOIN ticket_assignment_content_inputs e - ON e.workspace_id=a.workspace_id AND e.ticket_id=a.ticket_id AND e.assignment_id=a.assignment_id -) -SELECT i.ticket_id,i.assignment_id,i.input_event_id, - json_extract(i.tracking_json,'$.waiting_reason'),json_extract(i.tracking_json,'$.next_action'),d.payload_json -FROM inputs i -LEFT JOIN ticket_assignment_deliveries d ON d.workspace_id=?1 AND d.assignment_id=i.assignment_id AND d.input_event_id=i.input_event_id -WHERE NOT EXISTS(SELECT 1 FROM ticket_assignment_delivery_acknowledgements ack - WHERE ack.workspace_id=?1 AND ack.delivery_id=d.delivery_id) -AND (?3 IS NULL OR i.assignment_id=?3) AND (?4 IS NULL OR i.input_event_id=?4) -"#; - fn decode_payload(payload: &str) -> Result { serde_json::from_str(payload).map_err(|e| Error::Store(e.to_string())) } @@ -181,57 +142,51 @@ pub(super) fn list_in_tx( workspace_id: &str, limit: usize, ) -> Result> { - inputs_in_tx(conn, workspace_id, limit, None, None) + if !(1..=100).contains(&limit) { + return Err(Error::InvalidInput("Delivery limit must be 1..100".into())); + } + notification_routing::pending_sources(conn, workspace_id, "orchestrator", limit)? + .into_iter() + .map(|(ticket, assignment, input, id)| { + delivery(conn, workspace_id, ticket, assignment, input, id) + }) + .collect() +} +fn delivery( + conn: &Connection, + workspace: &str, + ticket_id: String, + assignment_id: String, + input_event_id: String, + delivery_id: String, +) -> Result { + let saved: Option = conn.query_row("SELECT payload_json FROM ticket_assignment_deliveries WHERE workspace_id=?1 AND assignment_id=?2 AND input_event_id=?3",params![workspace,assignment_id,input_event_id],|r|r.get(0)).optional()?; + if let Some(saved) = saved { + return decode_payload(&saved); + } + let (waiting_reason,next_action)=conn.query_row("SELECT json_extract(tracking_json,'$.waiting_reason'),json_extract(tracking_json,'$.next_action') FROM ticket_worker_assignments WHERE workspace_id=?1 AND assignment_id=?2",params![workspace,assignment_id],|r|Ok((r.get(0)?,r.get(1)?)))?; + Ok(TicketAssignmentDelivery { + ticket_id, + assignment_id, + input_event_id, + request_id: delivery_id.clone(), + delivery_id, + waiting_reason, + next_action, + }) } fn inputs_in_tx( conn: &Connection, - workspace_id: &str, - limit: usize, - assignment_id: Option<&str>, - input_event_id: Option<&str>, + workspace: &str, + _limit: usize, + assignment: Option<&str>, + input: Option<&str>, ) -> Result> { - let sql = format!( - "{INPUTS_SQL} ORDER BY i.ticket_id,i.assignment_id,i.input_order,i.input_event_id LIMIT ?2" - ); - let mut statement = conn.prepare(&sql)?; - let rows = statement.query_map( - params![workspace_id, limit as i64, assignment_id, input_event_id], - |row| { - Ok(( - row.get::<_, String>(0)?, - row.get::<_, String>(1)?, - row.get::<_, String>(2)?, - row.get::<_, Option>(3)?, - row.get::<_, Option>(4)?, - row.get::<_, Option>(5)?, - )) - }, - )?; - rows.map(|row| { - let (ticket_id, assignment_id, input_event_id, waiting_reason, next_action, payload) = row?; - if let Some(payload) = payload { - return decode_payload(&payload); - } - // Hash the identities, not mutable handling. Scope is Workspace-qualified. - let identities = serde_json::to_string(&(workspace_id, &assignment_id, &input_event_id)) - .map_err(|e| Error::Store(e.to_string()))?; - let digest = Sha256::digest(identities.as_bytes()); - let hex = digest - .iter() - .map(|byte| format!("{byte:02x}")) - .collect::(); - let delivery_id = format!("assignment-delivery:{hex}"); - Ok(TicketAssignmentDelivery { - ticket_id, - assignment_id, - input_event_id, - request_id: delivery_id.clone(), - delivery_id, - waiting_reason, - next_action, - }) - }) - .collect() + notification_routing::route_pending(conn, workspace)?; + let row: Option<(String,String,String,String)>=conn.query_row("SELECT s.ticket_id,s.assignment_id,s.input_event_id,d.delivery_id FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.workspace_id=?1 AND s.assignment_id=?2 AND s.input_event_id=?3 AND s.role='orchestrator' AND d.disposition='deliver' AND NOT EXISTS(SELECT 1 FROM ticket_assignment_delivery_acknowledgements a WHERE a.workspace_id=?1 AND a.delivery_id=d.delivery_id)",params![workspace,assignment,input],|r|Ok((r.get(0)?,r.get(1)?,r.get(2)?,r.get(3)?))).optional()?; + row.into_iter() + .map(|(ticket, a, i, id)| delivery(conn, workspace, ticket, a, i, id)) + .collect() } fn source_guard( @@ -255,22 +210,45 @@ pub(super) fn require_current( ticket_id: &str, assignment_id: &str, ) -> Result<()> { - let current:bool=conn.query_row(r#"SELECT EXISTS(SELECT 1 FROM ticket_current_worker_assignments c - JOIN ticket_worker_assignments a ON a.workspace_id=c.workspace_id AND a.assignment_id=c.assignment_id - JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id - WHERE c.workspace_id=?1 AND c.ticket_id=?2 AND c.assignment_id=?3 AND c.role='orchestrator' - AND a.principal_kind='workspace_agent' AND a.principal_id='workspace-orchestrator' - AND t.workflow_state='open' AND json_extract(a.tracking_json,'$.status') IN ('waiting','inprogress') - AND EXISTS(SELECT 1 FROM ticket_assignment_delivery_origins o WHERE o.workspace_id=a.workspace_id AND o.assignment_id=a.assignment_id) - AND NOT EXISTS(SELECT 1 FROM ticket_assignment_work_releases r WHERE r.workspace_id=a.workspace_id AND r.assignment_id=a.assignment_id))"#,params![workspace_id,ticket_id,assignment_id],|r|r.get(0))?; - if !current { + notification_routing::require_current( + conn, + workspace_id, + ticket_id, + assignment_id, + "orchestrator", + )?; + let known: bool = conn.query_row("SELECT EXISTS(SELECT 1 FROM ticket_assignment_delivery_origins WHERE workspace_id=?1 AND assignment_id=?2)", params![workspace_id, assignment_id], |r|r.get(0))?; + if !known { return Err(Error::TicketAssignmentConflict( - "Assignment is no longer a current actionable delegation".into(), + "Assignment has no committed input origin".into(), )); } Ok(()) } +// Call only after source_guard and require_current in the reservation/preparation +// transaction. A replacement target gets its own evidence for the same delivery, +// not a new request or authority to revive a completed/released assignment. +fn save_authorized_target( + conn: &Connection, + workspace: &str, + delivery: &str, + source: &TicketAssignmentTrackingSourceContext, +) -> Result<()> { + conn.execute( + "INSERT OR IGNORE INTO notification_assignment_targets VALUES(?1,?2,?3,?4,?5,?6)", + params![ + workspace, + delivery, + source.runtime_id, + source.worker_id, + source.agent_key, + source.authentication_binding_id + ], + )?; + Ok(()) +} + pub(super) fn reserve( store: &SqliteWorkspaceStore, workspace_id: &str, @@ -284,13 +262,14 @@ pub(super) fn reserve( let ticket_id:String=tx.query_row("SELECT ticket_id FROM ticket_worker_assignments WHERE workspace_id=?1 AND assignment_id=?2",params![workspace_id,assignment_id],|r|r.get(0))?; require_current(&tx,workspace_id,&ticket_id,assignment_id)?; let saved:Option=tx.query_row("SELECT payload_json FROM ticket_assignment_deliveries WHERE workspace_id=?1 AND assignment_id=?2 AND input_event_id=?3",params![workspace_id,assignment_id,input_event_id],|r|r.get(0)).optional()?; - if let Some(saved)=saved {let result=decode_payload(&saved)?;tx.commit()?;return Ok(result);} + if let Some(saved)=saved {let result=decode_payload(&saved)?;save_authorized_target(&tx,workspace_id,&result.delivery_id,source)?;tx.commit()?;return Ok(result);} // A caller cannot fabricate an input. Restrict the same common query by // exact assignment/input identity rather than trusting a scan snapshot. let candidates=inputs_in_tx(&tx,workspace_id,1,Some(assignment_id),Some(input_event_id))?; let delivery=candidates.into_iter().find(|d|d.assignment_id==assignment_id && d.input_event_id==input_event_id) .ok_or_else(||Error::TicketAssignmentConflict("Assignment input is not pending".into()))?; tx.execute("INSERT INTO ticket_assignment_deliveries VALUES(?1,?2,?3,?4,?5,?6)",params![workspace_id,assignment_id,input_event_id,delivery.delivery_id,serde_json::to_string(&delivery).map_err(|e|Error::Store(e.to_string()))?,serde_json::to_string(source).map_err(|e|Error::Store(e.to_string()))?])?; + save_authorized_target(&tx,workspace_id,&delivery.delivery_id,source)?; tx.commit()?; Ok(delivery) }) @@ -317,6 +296,7 @@ pub(super) fn prepare_content( let payload:String=tx.query_row("SELECT payload_json FROM ticket_assignment_deliveries WHERE workspace_id=?1 AND delivery_id=?2",params![workspace_id,delivery_id],|r|r.get(0))?; let delivery=decode_payload(&payload)?; require_current(&tx,workspace_id,&delivery.ticket_id,&delivery.assignment_id)?; + save_authorized_target(&tx,workspace_id,delivery_id,source)?; let saved:Option=tx.query_row("SELECT content FROM ticket_assignment_delivery_requests WHERE workspace_id=?1 AND delivery_id=?2",params![workspace_id,delivery_id],|r|r.get(0)).optional()?; let result=if let Some(saved)=saved {saved} else { tx.execute("INSERT INTO ticket_assignment_delivery_requests VALUES(?1,?2,?3)",params![workspace_id,delivery_id,content])?; @@ -335,10 +315,12 @@ pub(super) fn ack( ) -> Result<()> { store.with_conn_mut(|conn| { let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; - source_guard(&tx,workspace_id,source)?; - let payload:String=tx.query_row("SELECT payload_json FROM ticket_assignment_deliveries WHERE workspace_id=?1 AND delivery_id=?2",params![workspace_id,delivery_id],|r|r.get(0))?; - let delivery=decode_payload(&payload)?; - require_current(&tx,workspace_id,&delivery.ticket_id,&delivery.assignment_id)?; + // This is an acceptance receipt, not another send authorization. Match all + // authenticated target identity fields against evidence saved while current. + // Actor is audit attribution; it is not target authority. Existing ACK bytes + // still have to match on replay and are never replaced. + let authorized:bool=tx.query_row("SELECT EXISTS(SELECT 1 FROM notification_assignment_targets WHERE workspace_id=?1 AND delivery_id=?2 AND runtime_id=?3 AND worker_id=?4 AND agent_key=?5 AND authentication_binding_id=?6)",params![workspace_id,delivery_id,source.runtime_id,source.worker_id,source.agent_key,source.authentication_binding_id],|r|r.get(0))?; + if !authorized {return Err(Error::WorkspacePermissionDenied("Acknowledgement target was not authorized for this delivery".into()));} let source_json=serde_json::to_string(source).map_err(|e|Error::Store(e.to_string()))?; let saved:Option=tx.query_row("SELECT source_json FROM ticket_assignment_delivery_acknowledgements WHERE workspace_id=?1 AND delivery_id=?2",params![workspace_id,delivery_id],|r|r.get(0)).optional()?; if let Some(saved)=saved { @@ -413,41 +395,6 @@ pub(super) fn save_operation_fence( Ok(()) } -// Same input selection as delivery, plus immutable delivered history. Never -// SELECT payload/source/content into the public diagnostic projection. -const DIAGNOSTIC_INPUTS_SQL: &str = r#" -WITH raw_inputs AS ( - SELECT o.input_event_id,-1 AS input_order FROM ticket_assignment_delivery_origins o - WHERE o.workspace_id=?1 AND o.assignment_id=?3 - UNION - SELECT d.input_event_id,COALESCE(e.after_ticket_event_index+1,CASE WHEN d.input_event_id LIKE 'ticket-event:%' THEN CAST(substr(d.input_event_id,14) AS INTEGER) ELSE -1 END) - FROM ticket_assignment_deliveries d - LEFT JOIN ticket_assignment_content_inputs e ON e.workspace_id=d.workspace_id AND e.assignment_id=d.assignment_id - AND d.input_event_id='ticket-edit:' || printf('%020d',e.input_sequence) - WHERE d.workspace_id=?1 AND d.assignment_id=?3 - UNION - SELECT 'ticket-event:' || e.event_index,e.event_index - FROM ticket_assignment_delivery_origins o JOIN typed_ticket_events e ON e.workspace_id=o.workspace_id AND e.ticket_id=o.ticket_id - WHERE o.workspace_id=?1 AND o.ticket_id=?2 AND o.assignment_id=?3 AND e.event_index>o.after_ticket_event_index - AND e.kind IN ('assignment_answer','item_edit') - AND (e.kind<>'assignment_answer' OR EXISTS(SELECT 1 FROM typed_ticket_event_attributes attr WHERE attr.workspace_id=e.workspace_id AND attr.ticket_id=e.ticket_id AND attr.event_index=e.event_index AND attr.key='orchestrator_assignment_id' AND attr.value=?3)) - AND e.kind NOT LIKE 'assignment_tracking%' AND e.kind NOT LIKE 'assignment_delivery%' - AND COALESCE(e.state_field,'') NOT IN ('case','routing','assignment_tracking') - -- Retain historical input identity after done/release. A later delegation's - -- origin bounds ordinary events; an explicit answer's attribute is authoritative. - AND e.event_index<=COALESCE((SELECT MIN(later.after_ticket_event_index) FROM ticket_assignment_delivery_origins later - JOIN ticket_worker_assignments la ON la.workspace_id=later.workspace_id AND la.assignment_id=later.assignment_id - JOIN ticket_worker_assignments original ON original.workspace_id=o.workspace_id AND original.assignment_id=o.assignment_id - WHERE later.workspace_id=o.workspace_id AND later.ticket_id=o.ticket_id AND la.rowid>original.rowid),9223372036854775807) - AND NOT EXISTS(SELECT 1 FROM typed_ticket_event_attributes attr WHERE attr.workspace_id=e.workspace_id AND attr.ticket_id=e.ticket_id AND attr.event_index=e.event_index AND attr.key='orchestrator_assignment_id' AND attr.value<>?3) - UNION - SELECT 'ticket-edit:' || printf('%020d',input_sequence),after_ticket_event_index+1 - FROM ticket_assignment_content_inputs WHERE workspace_id=?1 AND ticket_id=?2 AND assignment_id=?3 -), inputs AS ( - SELECT input_event_id,ROW_NUMBER() OVER (ORDER BY input_order,input_event_id) AS input_order FROM raw_inputs -) -"#; - pub(super) fn diagnostics_page( store: &SqliteWorkspaceStore, workspace_id: &str, @@ -473,20 +420,9 @@ pub(super) fn diagnostics_page( Err(Error::TicketAssignmentConflict(_)) => false, Err(error) => return Err(error), }; - let after = match after_input_event_id { - None => -2, - Some(id) => tx.query_row(&format!("{DIAGNOSTIC_INPUTS_SQL} SELECT input_order FROM inputs WHERE input_event_id=?5"),params![workspace_id,ticket_id,assignment_id,actionable,id],|r| r.get::<_,i64>(0)).optional()?.ok_or_else(|| Error::InvalidInput("Input cursor does not belong to this assignment".into()))?, - }; - let sql = format!("{DIAGNOSTIC_INPUTS_SQL} SELECT i.input_event_id,d.delivery_id,EXISTS(SELECT 1 FROM ticket_assignment_delivery_requests req WHERE req.workspace_id=?1 AND req.delivery_id=d.delivery_id),EXISTS(SELECT 1 FROM ticket_assignment_delivery_acknowledgements ack WHERE ack.workspace_id=?1 AND ack.delivery_id=d.delivery_id) FROM inputs i LEFT JOIN ticket_assignment_deliveries d ON d.workspace_id=?1 AND d.assignment_id=?3 AND d.input_event_id=i.input_event_id WHERE i.input_order>?5 ORDER BY i.input_order LIMIT ?6"); - let mut stmt = tx.prepare(&sql)?; - let mut inputs = stmt.query_map(params![workspace_id,ticket_id,assignment_id,actionable,after,(limit+1) as i64],|r| { - let delivery_id: Option = r.get(1)?; - let prepared: bool = r.get(2)?; - let ack: bool = r.get(3)?; - use server_api::TicketAssignmentInputDeliveryState::*; - let notification = if ack { RuntimeAccepted } else if prepared { RequestPersisted } else if delivery_id.is_some() { Reserved } else { NotReserved }; - Ok(server_api::TicketAssignmentInputDiagnostic { input_event_id:r.get(0)?, delivery_id,notification,acceptance_outcome_unknown:prepared && !ack }) - })?.collect::,_>>()?; + let mut inputs = notification_routing::diagnostic_inputs( + &tx, workspace_id, ticket_id, assignment_id, after_input_event_id, limit, + )?; let more = inputs.len()>limit; inputs.truncate(limit); let next_input_event_id = if more { inputs.last().map(|i| i.input_event_id.clone()) } else { None }; diff --git a/crates/workspace-server/src/assignment_delivery_tests.rs b/crates/workspace-server/src/assignment_delivery_tests.rs index ae8ff43cd..bff949207 100644 --- a/crates/workspace-server/src/assignment_delivery_tests.rs +++ b/crates/workspace-server/src/assignment_delivery_tests.rs @@ -302,11 +302,9 @@ async fn assignment_delivery_clear_reassign_fences_scanned_and_reserved_inputs() .reserve_ticket_assignment_delivery("w", "a", &old.input_event_id, &source) .is_err() ); - assert!( - store - .ack_ticket_assignment_delivery("w", &old.delivery_id, &source) - .is_err() - ); + // Acceptance of the previously authorized request is evidence, not permission + // to resend or transfer work to the new assignment. + store.ack_ticket_assignment_delivery("w", &old.delivery_id, &source).unwrap(); let pending = store .list_pending_ticket_assignment_deliveries("w", 100) .unwrap(); @@ -327,7 +325,7 @@ async fn assignment_delivery_clear_reassign_fences_scanned_and_reserved_inputs() } #[tokio::test] -async fn assignment_delivery_old_worker_or_authenticated_binding_cannot_reserve_or_ack() { +async fn assignment_delivery_old_target_cannot_resend_but_can_record_authorized_acceptance() { for change in ["replacement", "revoke", "binding-change"] { let (_directory, store) = tracking_store_fixture().await; tracking_store_assign(&store, "a", "assign-a"); @@ -347,13 +345,9 @@ async fn assignment_delivery_old_worker_or_authenticated_binding_cannot_reserve_ ), "{change}" ); - assert!( - matches!( - store.ack_ticket_assignment_delivery("w", &old.delivery_id, &source), - Err(Error::WorkspacePermissionDenied(_)) - ), - "{change}" - ); + let mut unreserved = source.clone(); + unreserved.worker_id = "never-authorized-target".into(); + assert!(store.ack_ticket_assignment_delivery("w", &old.delivery_id, &unreserved).is_err()); if change == "replacement" { let mut replacement = source.clone(); replacement.worker_id = "replacement".into(); @@ -373,6 +367,8 @@ async fn assignment_delivery_old_worker_or_authenticated_binding_cannot_reserve_ .len(), 1 ); + } else { + store.ack_ticket_assignment_delivery("w", &old.delivery_id, &source).unwrap(); } } } @@ -1111,9 +1107,11 @@ async fn assignment_content_edits_never_revive_done_closed_or_reissued_delegatio let source = tracking_store_source(); let initial = delivery_store_reserve_initial(&store, &source); store.ack_ticket_assignment_delivery("w", &initial.delivery_id, &source).unwrap(); - store.with_conn(|conn| { - conn.execute("UPDATE typed_tickets SET title='Input before authority change' WHERE ticket_id='tracking'", [])?; - Ok(()) + use ticket::TicketBackend; + let backend = ticket::SqliteTicketBackend::open_verified(directory.path().join("workspace.db"), "w").unwrap(); + backend.edit_item("tracking".into(), ticket::TicketItemEdit { + title: Some("Input before authority change".into()), + author: Some("fixture-editor".into()), ..Default::default() }).unwrap(); let old = store.list_pending_ticket_assignment_deliveries("w", 100).unwrap().remove(0); match change { @@ -1121,8 +1119,6 @@ async fn assignment_content_edits_never_revive_done_closed_or_reissued_delegatio "close-reopen" => { store.with_conn(|conn| { conn.execute_batch("UPDATE typed_tickets SET workflow_state='closed' WHERE ticket_id='tracking'; UPDATE typed_tickets SET workflow_state='open' WHERE ticket_id='tracking';")?; Ok(()) }).unwrap(); } _ => { tracking_store_assign(&store, "b", "assign-b"); } } - use ticket::TicketBackend; - let backend = ticket::SqliteTicketBackend::open_verified(directory.path().join("workspace.db"), "w").unwrap(); backend.edit_item("tracking".into(), ticket::TicketItemEdit { body: Some(ticket::MarkdownText::new("New requirements")), author: Some("fixture-editor".into()), ..Default::default() }).unwrap(); diff --git a/crates/workspace-server/src/assignment_tracking_tests.rs b/crates/workspace-server/src/assignment_tracking_tests.rs index 6df1bb407..b9392b4bd 100644 --- a/crates/workspace-server/src/assignment_tracking_tests.rs +++ b/crates/workspace-server/src/assignment_tracking_tests.rs @@ -442,7 +442,7 @@ fn tracking_store_real_schema91_to92_retains_queue_cases_principals_audit_receip .iter() .map(|m| m.version) .collect::>(), - vec![92, 93, 94] + (92..=LATEST_SCHEMA_VERSION).collect::>() ); let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); store @@ -465,7 +465,11 @@ fn tracking_store_real_schema91_to92_retains_queue_cases_principals_audit_receip 93, "Durable assignment deliveries and source-bound startup fences".into() ), - (94, "Canonical Ticket posts".into()) + (94, "Canonical Ticket posts".into()), + ( + 95, + "Common notification routing and durable Worker delivery".into() + ) ] ); for (table, rows) in retained.iter().zip(original.iter()) { @@ -604,7 +608,7 @@ fn tracking_store_schema91_closed_unknown_assignment_uses_assign_audit_for_first ); store.with_conn(|conn| { assert_eq!(current_schema_version(conn)?, LATEST_SCHEMA_VERSION); - assert_eq!(conn.query_row("SELECT MAX(version) FROM ticket_schema_migrations", [], |row| row.get::<_,i64>(0))?, 12); + assert_eq!(conn.query_row("SELECT MAX(version) FROM ticket_schema_migrations", [], |row| row.get::<_,i64>(0))?, 13); assert_eq!(conn.query_row("SELECT workflow_state FROM typed_tickets WHERE ticket_id='historical-done'", [], |row| row.get::<_,String>(0))?, "closed"); let stored: (Option, Option) = conn.query_row("SELECT tracking_json,last_tracking_event_id FROM ticket_worker_assignments WHERE assignment_id='a-historical-done'", [], |row| Ok((row.get(0)?,row.get(1)?)))?; assert_eq!(stored, (None, None), "read baseline must not manufacture tracking storage"); @@ -1566,12 +1570,21 @@ async fn assignment_answer_replay_returns_saved_event_without_new_assignment_or_ let pending = store .list_pending_ticket_assignment_deliveries("w", 100) .unwrap(); - assert_eq!(pending.len(), 2); + assert_eq!( + pending.len(), + 1, + "self-answer preserves the receipt without waking the same Orchestrator" + ); assert!( pending .iter() - .any(|p| p.input_event_id == saved.input_event_id) + .all(|p| p.input_event_id != saved.input_event_id) ); + store.with_conn(|conn| { + let reason: String = conn.query_row("SELECT d.reason FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.workspace_id='w' AND s.assignment_id='answer-a' AND s.input_event_id=?1 AND s.role='orchestrator'", [&saved.input_event_id], |row| row.get(0))?; + assert_eq!(reason, "self_notification"); + Ok(()) + }).unwrap(); store .with_conn(|conn| { assert_eq!( diff --git a/crates/workspace-server/src/notification_delivery.rs b/crates/workspace-server/src/notification_delivery.rs new file mode 100644 index 000000000..0e72d430a --- /dev/null +++ b/crates/workspace-server/src/notification_delivery.rs @@ -0,0 +1,341 @@ +//! Backend delivery consumes committed routing decisions, never event-kind policy. +//! +//! A mutation callback is only a low-latency wakeup. The Workspace hook also runs +//! bounded recovery rounds independently of mutations and Runtime subscriptions. +//! Requests are pinned before network I/O; acceptance is not model processing. +//! +//! Routing boundary map: +//! | Source / trigger | Owner | Delivery / recovery | +//! | --- | --- | --- | +//! | Assign, Answer, content and post edits, case, tags, tracking, release | Ticket transaction captures facts; `notification_routing` decides both recipients and non-notification reasons | Saved assignment/Worker requests and ACKs; mutation wakeup and periodic rounds | +//! | Workspace startup, Orchestrator create/restore/replacement, idle | Wakeup only; never a new request or a second policy | Same saved pending decisions | +//! | MR/handoff and Backend Job | Separate adapters to be connected by follow-up work | Not inferred from comments or display state | +//! | Explicit WorkerNotify/WorkerSendInput | Authenticated caller communication | Not automatic business routing | +//! | Internal SubWorker result and Web subscription | Parent/child execution and UI synchronization respectively | Remain separate; no Ticket policy in Runtime | +//! +//! Runtime receipt retention is finite. This layer preserves request identity and +//! bytes across retries; it does not claim unbounded exactly-once model execution. +use super::*; +use crate::store::{ + NotificationDeliveryKey, NotificationFailureCode, TicketWorkerNotification, + TicketWorkerNotificationTarget, +}; + +#[derive(Default)] +struct NotificationDeliveryActivity { + stopped: bool, + in_flight: usize, +} + +#[derive(Default)] +pub(super) struct NotificationDeliveryLifecycle { + state: Mutex, + drained: tokio::sync::Notify, +} +pub(super) struct NotificationDeliveryPermit(Arc); +impl NotificationDeliveryLifecycle { + pub(super) fn begin(self: &Arc) -> Option { + let mut state = self.state.lock().unwrap_or_else(|p| p.into_inner()); + if state.stopped { + return None; + } + state.in_flight += 1; + Some(NotificationDeliveryPermit(self.clone())) + } + pub(super) fn stopped(&self) -> bool { + self.state.lock().unwrap_or_else(|p| p.into_inner()).stopped + } + pub(super) async fn stop_and_drain(&self) { + self.state.lock().unwrap_or_else(|p| p.into_inner()).stopped = true; + loop { + let drained = self.drained.notified(); + tokio::pin!(drained); + drained.as_mut().enable(); + if self + .state + .lock() + .unwrap_or_else(|p| p.into_inner()) + .in_flight + == 0 + { + return; + } + drained.await; + } + } +} +impl Drop for NotificationDeliveryPermit { + fn drop(&mut self) { + let mut state = self.0.state.lock().unwrap_or_else(|p| p.into_inner()); + state.in_flight -= 1; + if state.in_flight == 0 { + self.0.drained.notify_waiters(); + } + } +} + +pub(super) fn notification_failure( + result: &std::result::Result, +) -> Option { + match result { + Err(_) => Some(NotificationFailureCode::AcceptanceUnknown), + Ok(result) if result.disposition == WorkerInputDisposition::Unknown => { + Some(NotificationFailureCode::AcceptanceUnknown) + } + Ok(result) + if result.state == InternalWorkerOperationState::Accepted + && result.disposition == WorkerInputDisposition::Accepted => + { + None + } + Ok(result) if result.state == InternalWorkerOperationState::Unsupported => { + Some(NotificationFailureCode::RuntimeUnsupported) + } + Ok(_) => Some(NotificationFailureCode::RuntimeRejected), + } +} + +pub(super) fn assignment_delivery_key( + input: &crate::store::TicketAssignmentDelivery, +) -> NotificationDeliveryKey { + NotificationDeliveryKey::AssignmentInput { + assignment_id: input.assignment_id.clone(), + input_event_id: input.input_event_id.clone(), + } +} + +pub(super) fn record_notification_failure( + api: &WorkspaceApi, + key: &NotificationDeliveryKey, + code: NotificationFailureCode, +) { + if let Err(error) = + api.store + .record_notification_delivery_failure(&api.config.workspace_id, key, code) + { + // Do not turn failure to save diagnostics into a successful suppression. + tracing::warn!(%error, "notification failure could not be persisted"); + } +} + +pub(super) fn notification_target( + api: &WorkspaceApi, + input: &TicketWorkerNotification, +) -> std::result::Result { + let worker = api + .runtime + .worker(&input.worker) + .map_err(|_| NotificationFailureCode::TargetUnavailable)?; + if worker.workspace.workspace_id.as_deref() != Some(&api.config.workspace_id) { + return Err(NotificationFailureCode::WorkspaceMismatch); + } + if !matches!(worker.state.as_str(), "idle" | "running" | "paused") { + return Err(NotificationFailureCode::TargetStopped); + } + let binding = api + .config_store + .get_workspace_runtime_binding(&api.config.workspace_id, &input.worker.runtime_id) + .map_err(|_| NotificationFailureCode::SourceLookupFailed)? + .ok_or(NotificationFailureCode::TargetUnavailable)?; + if binding.revoked_at.is_some() { + return Err(NotificationFailureCode::BindingRevoked); + } + Ok(TicketWorkerNotificationTarget { + worker: input.worker.clone(), + authentication_binding_id: binding.binding_id, + }) +} + +pub(super) fn dispatch_ticket_worker_notifications(api: &WorkspaceApi) { + let Some(_permit) = api.notification_delivery_lifecycle.begin() else { + return; + }; + let inputs = match api + .store + .list_pending_ticket_worker_notifications(&api.config.workspace_id, 20) + { + Ok(inputs) => inputs, + Err(error) => { + tracing::warn!(%error, "notification scan failed; committed sources remain recoverable"); + return; + } + }; + for input in inputs { + if api.notification_delivery_lifecycle.stopped() { + break; + } + let key = NotificationDeliveryKey::WorkerNotification { + delivery_id: input.delivery_id.clone(), + }; + let target = match notification_target(api, &input) { + Ok(target) => target, + Err(code) => { + record_notification_failure(api, &key, code); + continue; + } + }; + let resource_key = match api.store.resource_key( + &api.config.workspace_id, + WorkspaceResourceKind::Ticket, + &input.ticket_id, + ) { + Ok(Some(key)) if canonical_ticket_resource_key(&key).is_some() => key, + _ => { + record_notification_failure(api, &key, NotificationFailureCode::SourceLookupFailed); + continue; + } + }; + // An attention notification grants no new assignment or execution authority. + // The immutable source reference lets the recipient reread authoritative facts. + let content = format!( + "Ticket {resource_key} has committed activity (input {}). Reread the current Ticket and assignment before acting. This notification does not grant or extend execution authority.", + input.input_event_id, + ); + let content = match api.store.prepare_ticket_worker_notification_content( + &api.config.workspace_id, + &input.delivery_id, + &content, + &target, + ) { + Ok(content) => content, + Err(error) => { + let code = match error { + Error::TicketAssignmentConflict(_) => { + NotificationFailureCode::AssignmentChanged + } + _ => NotificationFailureCode::PreparationFailed, + }; + record_notification_failure(api, &key, code); + continue; + } + }; + let result = api.runtime.send_input( + &input.worker, + WorkerInputRequest { + kind: WorkerInputKind::Notify, + content, + submission_request_id: Some(input.request_id.clone()), + segments: None, + }, + ); + if let Some(failure) = notification_failure(&result) { + record_notification_failure(api, &key, failure); + } else if api + .store + .ack_ticket_worker_notification(&api.config.workspace_id, &input.delivery_id, &target) + .is_err() + { + record_notification_failure(api, &key, NotificationFailureCode::AcknowledgementFailed); + } + // One recipient's failure never prevents the other recipients in this round. + } +} + +pub(super) fn orchestrator_target_failure( + api: &WorkspaceApi, + worker: &RuntimeWorkerRef, +) -> NotificationFailureCode { + match api.runtime.worker(worker) { + Err(_) => return NotificationFailureCode::TargetUnavailable, + Ok(summary) + if summary.workspace.workspace_id.as_deref() != Some(&api.config.workspace_id) => + { + return NotificationFailureCode::WorkspaceMismatch; + } + Ok(summary) if !matches!(summary.state.as_str(), "idle" | "running" | "paused") => { + return NotificationFailureCode::TargetStopped; + } + _ => {} + } + match api + .config_store + .get_workspace_runtime_binding(&api.config.workspace_id, &worker.runtime_id) + { + Err(_) => NotificationFailureCode::SourceLookupFailed, + Ok(None) => NotificationFailureCode::TargetUnavailable, + Ok(Some(binding)) if binding.revoked_at.is_some() => { + NotificationFailureCode::BindingRevoked + } + _ => NotificationFailureCode::AssignmentChanged, + } +} + +pub(super) fn recover_ticket_notifications(api: &WorkspaceApi) { + let Some(_permit) = api.notification_delivery_lifecycle.begin() else { + return; + }; + dispatch_ticket_worker_notifications(api); + if let Some(orchestrator) = find_workspace_orchestrator(api) { + dispatch_orchestrator_assignment_attention(api, &orchestrator.worker); + } else { + // Still reconcile committed sources and explain target absence even when + // there is no Runtime Worker. Do not start or restore one as a side effect. + match api + .store + .list_pending_ticket_assignment_deliveries(&api.config.workspace_id, 20) + { + Ok(inputs) => { + for input in inputs { + record_notification_failure( + api, + &assignment_delivery_key(&input), + NotificationFailureCode::TargetUnavailable, + ); + } + } + Err(error) => tracing::warn!(%error, "assignment notification scan failed"), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn transport_unknown_disposition_is_not_a_confirmed_rejection() { + for (state, disposition, expected) in [ + ( + InternalWorkerOperationState::Rejected, + WorkerInputDisposition::Unknown, + Some(NotificationFailureCode::AcceptanceUnknown), + ), + ( + InternalWorkerOperationState::Rejected, + WorkerInputDisposition::Rejected, + Some(NotificationFailureCode::RuntimeRejected), + ), + ( + InternalWorkerOperationState::Accepted, + WorkerInputDisposition::Accepted, + None, + ), + ] { + let result = Ok(WorkerInputResult { + state, + disposition, + worker: RuntimeWorkerRef::new("runtime", "worker"), + runtime_run_id: None, + diagnostics: Vec::new(), + }); + assert_eq!(notification_failure(&result), expected); + } + } + + #[tokio::test] + async fn shutdown_fences_new_rounds_and_drains_the_synchronous_io_owner() { + let lifecycle = Arc::new(NotificationDeliveryLifecycle::default()); + let admitted = lifecycle.begin().unwrap(); + let stopped = lifecycle.stop_and_drain(); + tokio::pin!(stopped); + // Poll shutdown while a synchronous owner still holds its permit. This + // models an aborted async waiter without pretending its I/O was cancelled. + assert!(futures::poll!(stopped.as_mut()).is_pending()); + assert!(lifecycle.begin().is_none()); + drop(admitted); + tokio::time::timeout(std::time::Duration::from_secs(1), stopped) + .await + .unwrap(); + assert!(lifecycle.begin().is_none()); + } +} diff --git a/crates/workspace-server/src/notification_routing.rs b/crates/workspace-server/src/notification_routing.rs new file mode 100644 index 000000000..46aeb6b22 --- /dev/null +++ b/crates/workspace-server/src/notification_routing.rs @@ -0,0 +1,813 @@ +//! Domain writes capture facts atomically; this module alone interprets notification intent. +//! Runtime acceptance, handling and execution authority remain independent. Explicit user +//! communication, internal child notifications and Web subscriptions are not business routes. +use super::*; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NotificationPurpose { + NewDelegation, + ContinuingRequest, + Attention, + Result, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NotificationEventKind { + Assignment, + Answer, + ContentEdit, + Comment, + Case, + Tracking, + Tags, + Release, + AssignmentChanged, + Result, + Unknown, +} +impl NotificationEventKind { + fn from_source(kind: &str) -> Self { + match kind { + "assignment" => Self::Assignment, + "assignment_answer" => Self::Answer, + "content_edit" | "item_edit" => Self::ContentEdit, + "comment" | "comment_edit" => Self::Comment, + "case" | "case_update" => Self::Case, + "tracking" | "assignment_tracking" => Self::Tracking, + "tags" | "tags_set" | "tags_changed" => Self::Tags, + "release" => Self::Release, + "assignment_changed" => Self::AssignmentChanged, + _ => Self::Unknown, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NotificationReason { + Requested, + Attention, + Result, + NoRecipient, + UnsupportedRecipient, + HistoricalOnly, + CaseClosed, + HandlingDone, + HandlingUnknown, + SelfNotification, + NotRequest, + AssignmentChanged, + UnknownEvent, +} +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum NotificationFailureCode { + ConfigurationUnavailable, + RenderingFailed, + SourceLookupFailed, + TargetUnavailable, + TargetStopped, + WorkspaceMismatch, + BindingRevoked, + AssignmentChanged, + RuntimeRejected, + AcceptanceUnknown, + PreparationFailed, + AcknowledgementFailed, + RuntimeUnsupported, + RuntimeUnavailable, +} +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum NotificationDeliveryKey { + AssignmentInput { + assignment_id: String, + input_event_id: String, + }, + WorkerNotification { + delivery_id: String, + }, +} +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TicketWorkerNotification { + pub ticket_id: String, + pub assignment_id: String, + pub input_event_id: String, + pub delivery_id: String, + pub request_id: String, + pub worker: RuntimeWorkerRef, + pub purpose: NotificationPurpose, +} +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TicketWorkerNotificationTarget { + pub worker: RuntimeWorkerRef, + pub authentication_binding_id: String, +} +#[derive(Debug, Clone, Serialize, Deserialize)] +struct Facts { + principal_kind: Option, + principal_id: Option, + runtime_id: Option, + worker_id: Option, + status: Option, + case_state: String, + released: bool, +} +#[derive(Debug)] +struct Source { + sequence: i64, + workspace: String, + ticket: String, + input: String, + role: String, + assignment: String, + kind: String, + actor: Option, + event_index: Option, + facts: Facts, +} +fn json(value: &T) -> Result { + serde_json::to_string(value).map_err(|e| Error::Store(e.to_string())) +} +fn code(value: T) -> Result { + Ok(json(&value)?.trim_matches('"').to_owned()) +} +fn decode(value: &str) -> Result { + serde_json::from_str(value).map_err(|e| Error::Store(e.to_string())) +} +fn purpose(kind: NotificationEventKind, role: &str) -> NotificationPurpose { + match (kind, role) { + (NotificationEventKind::Assignment, "orchestrator") => NotificationPurpose::NewDelegation, + (NotificationEventKind::Answer | NotificationEventKind::ContentEdit, "orchestrator") => { + NotificationPurpose::ContinuingRequest + } + (NotificationEventKind::Result, _) => NotificationPurpose::Result, + _ => NotificationPurpose::Attention, + } +} +fn policy( + kind: NotificationEventKind, + role: &str, + assignment: &str, + facts: &Facts, + self_notification: bool, +) -> (&'static str, NotificationReason) { + use NotificationReason as R; + if kind == NotificationEventKind::Unknown { + return ("defer", R::UnknownEvent); + } + if assignment.is_empty() { + return ("suppress", R::NoRecipient); + } + let supported = if role == "orchestrator" { + facts.principal_kind.as_deref() == Some("workspace_agent") + && facts.principal_id.as_deref() == Some("workspace-orchestrator") + } else { + role == "worker" && facts.principal_kind.as_deref() == Some("worker") + }; + if !supported { + return ("suppress", R::UnsupportedRecipient); + } + if self_notification { + return ("suppress", R::SelfNotification); + } + if facts.released { + return ("suppress", R::HistoricalOnly); + } + if facts.case_state != "open" { + return ("suppress", R::CaseClosed); + } + if role == "orchestrator" { + match facts.status.as_deref() { + Some("waiting" | "inprogress") => {} + Some("done") => return ("suppress", R::HandlingDone), + _ => return ("suppress", R::HandlingUnknown), + } + if !matches!( + kind, + NotificationEventKind::Assignment + | NotificationEventKind::Answer + | NotificationEventKind::ContentEdit + | NotificationEventKind::Result + ) { + return ("suppress", R::NotRequest); + } + } + if matches!( + kind, + NotificationEventKind::Tracking | NotificationEventKind::Release + ) { + return ("suppress", R::NotRequest); + } + ( + "deliver", + match purpose(kind, role) { + NotificationPurpose::Result => R::Result, + NotificationPurpose::Attention => R::Attention, + _ => R::Requested, + }, + ) +} + +const FACTS_SQL: &str = "json_object('principal_kind',a.principal_kind,'principal_id',a.principal_id,'runtime_id',COALESCE(a.runtime_id,(SELECT runtime_id FROM worker_singleton_owners WHERE workspace_id=a.workspace_id AND singleton_key=a.principal_id AND a.principal_kind='workspace_agent')),'worker_id',COALESCE(a.worker_id,(SELECT worker_id FROM worker_singleton_owners WHERE workspace_id=a.workspace_id AND singleton_key=a.principal_id AND a.principal_kind='workspace_agent')),'status',json_extract(a.tracking_json,'$.status'),'case_state',t.workflow_state,'released',json(CASE WHEN EXISTS(SELECT 1 FROM ticket_assignment_work_releases r WHERE r.workspace_id=a.workspace_id AND r.assignment_id=a.assignment_id) THEN 'true' ELSE 'false' END))"; + +fn capture(input: &str, kind: &str, actor: &str, event_index: &str) -> String { + format!("INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,actor,event_index,facts_json) + SELECT NEW.workspace_id,NEW.ticket_id,{input},roles.role,COALESCE(a.assignment_id,''),{kind},{actor},{event_index},{FACTS_SQL} + FROM (SELECT 'orchestrator' role UNION ALL SELECT 'worker') roles + JOIN typed_tickets t ON t.workspace_id=NEW.workspace_id AND t.ticket_id=NEW.ticket_id + LEFT JOIN ticket_current_worker_assignments c ON c.workspace_id=t.workspace_id AND c.ticket_id=t.ticket_id AND c.role=roles.role + LEFT JOIN ticket_worker_assignments a ON a.workspace_id=c.workspace_id AND a.assignment_id=c.assignment_id;") +} + +pub(super) fn install(conn: &Connection) -> Result<()> { + conn.execute_batch(include_str!("notification_routing.sql"))?; + // Import only already recognized request identities and receipts. Never scan old comments. + conn.execute_batch(&format!("INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,event_index,facts_json) + SELECT a.workspace_id,a.ticket_id,o.input_event_id,a.role,a.assignment_id,'assignment',NULL,{FACTS_SQL} + FROM ticket_assignment_delivery_origins o JOIN ticket_worker_assignments a USING(workspace_id,assignment_id) JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id; + INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,event_index,facts_json) + SELECT a.workspace_id,a.ticket_id,'ticket-edit:' || printf('%020d',e.input_sequence),a.role,a.assignment_id,'content_edit',NULL,{FACTS_SQL} + FROM ticket_assignment_content_inputs e JOIN ticket_worker_assignments a USING(workspace_id,assignment_id) JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id; + INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,event_index,facts_json) + SELECT a.workspace_id,a.ticket_id,'ticket-event:' || e.event_index,a.role,a.assignment_id,e.kind,e.event_index,{FACTS_SQL} + FROM ticket_assignment_delivery_origins o JOIN ticket_worker_assignments a USING(workspace_id,assignment_id) JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id + JOIN typed_ticket_events e ON e.workspace_id=a.workspace_id AND e.ticket_id=a.ticket_id AND e.event_index>o.after_ticket_event_index + WHERE e.kind IN ('assignment_answer','item_edit') AND + (EXISTS(SELECT 1 FROM typed_ticket_event_attributes x WHERE x.workspace_id=e.workspace_id AND x.ticket_id=e.ticket_id AND x.event_index=e.event_index AND x.key='orchestrator_assignment_id' AND x.value=a.assignment_id) + OR (e.kind='item_edit' AND NOT EXISTS(SELECT 1 FROM ticket_assignment_delivery_origins later JOIN ticket_worker_assignments la ON la.workspace_id=later.workspace_id AND la.assignment_id=later.assignment_id WHERE later.workspace_id=a.workspace_id AND later.ticket_id=a.ticket_id AND la.rowid>a.rowid AND e.event_index>later.after_ticket_event_index))); + INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,event_index,facts_json) + SELECT a.workspace_id,a.ticket_id,d.input_event_id,a.role,a.assignment_id,'content_edit',NULL,{FACTS_SQL} + FROM ticket_assignment_deliveries d JOIN ticket_worker_assignments a USING(workspace_id,assignment_id) JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id;"))?; + let event = capture( + "'ticket-event:' || NEW.event_index", + "CASE WHEN NEW.state_field='case' THEN 'case' WHEN NEW.state_field='tags' THEN 'tags' ELSE NEW.kind END", + "NEW.author", + "NEW.event_index", + ); + let history = capture( + "'assignment-event:' || NEW.event_id", + "CASE WHEN json_extract(NEW.source_context_json,'$.kind')='assignment' THEN 'assignment' ELSE 'tracking' END", + "json_extract(NEW.source_context_json,'$.actor')", + "NULL", + ); + let change = capture( + "'assignment-change:' || NEW.event_id", + "'assignment_changed'", + "NEW.actor", + "NULL", + ); + let release = capture( + "'assignment-release:' || NEW.assignment_id", + "'release'", + "NULL", + "NULL", + ); + conn.execute_batch(&format!("CREATE TRIGGER notification_ticket_event AFTER INSERT ON typed_ticket_events WHEN NEW.event_index>=0 BEGIN {event} END; + CREATE TRIGGER notification_assignment_history AFTER INSERT ON typed_ticket_assignment_tracking_history BEGIN {history} END; + CREATE TRIGGER notification_assignment_change AFTER INSERT ON ticket_worker_assignment_events BEGIN {change} + INSERT OR IGNORE INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,actor,event_index,facts_json) + SELECT a.workspace_id,a.ticket_id,'assignment-change:' || NEW.event_id,a.role,a.assignment_id,'release',NEW.actor,NULL,json_set({FACTS_SQL},'$.released',json('true')) + FROM ticket_worker_assignments a JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id + WHERE a.workspace_id=NEW.workspace_id AND a.assignment_id=NEW.previous_assignment_id AND NEW.previous_assignment_id IS NOT NEW.assignment_id; + END;"))?; + // Release rows have no Ticket column; use the assignment's immutable parent. + let release = release.replace("NEW.ticket_id", "(SELECT ticket_id FROM ticket_worker_assignments WHERE workspace_id=NEW.workspace_id AND assignment_id=NEW.assignment_id)"); + conn.execute_batch(&format!("CREATE TRIGGER notification_assignment_release AFTER INSERT ON ticket_assignment_work_releases BEGIN {release} END; + CREATE TRIGGER notification_content_input AFTER INSERT ON ticket_assignment_content_inputs BEGIN + INSERT INTO notification_sources(workspace_id,ticket_id,input_event_id,role,assignment_id,kind,actor,event_index,facts_json) + SELECT NEW.workspace_id,NEW.ticket_id,'ticket-edit:' || printf('%020d',NEW.input_sequence),a.role,a.assignment_id,NEW.kind,NEW.actor,NULL,json_set({FACTS_SQL},'$.content_change_id',NEW.source_change_id) + FROM ticket_worker_assignments a JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id + WHERE a.workspace_id=NEW.workspace_id AND a.assignment_id=NEW.assignment_id; + END;"))?; + let missing = capture( + "'ticket-content:' || NEW.change_id", + "NEW.kind", + "NEW.actor", + "NULL", + ) + .trim_end_matches(';') + .to_owned() + + " WHERE a.assignment_id IS NULL;"; + conn.execute_batch(&format!("CREATE TRIGGER notification_unassigned_content AFTER INSERT ON typed_ticket_content_changes BEGIN {missing} END;"))?; + for table in [ + "notification_sources", + "notification_assignment_targets", + "notification_requests", + "notification_acknowledgements", + "notification_failures", + ] { + for action in ["update", "delete"] { + conn.execute_batch(&format!("CREATE TRIGGER {table}_no_{action} BEFORE {action} ON {table} BEGIN SELECT RAISE(ABORT,'Notification evidence is immutable'); END;"))?; + } + } + conn.execute_batch("CREATE TRIGGER notification_decisions_no_delete BEFORE DELETE ON notification_decisions BEGIN SELECT RAISE(ABORT,'Notification decision is retained'); END; + CREATE TRIGGER notification_decisions_identity_no_update BEFORE UPDATE OF source_sequence,workspace_id,delivery_id,disposition,reason,purpose ON notification_decisions BEGIN SELECT RAISE(ABORT,'Notification decision is immutable'); END;")?; + Ok(()) +} + +pub(super) fn verify(conn: &Connection) -> Result<()> { + let mut names = vec![ + "notification_sources".to_owned(), + "notification_assignment_targets".into(), + "notification_decisions".into(), + "notification_requests".into(), + "notification_acknowledgements".into(), + "notification_failures".into(), + ]; + for table in [ + "notification_sources", + "notification_assignment_targets", + "notification_requests", + "notification_acknowledgements", + "notification_failures", + ] { + for action in ["update", "delete"] { + names.push(format!("{table}_no_{action}")); + } + } + for name in [ + "notification_decisions_no_delete", + "notification_decisions_identity_no_update", + "notification_ticket_event", + "notification_assignment_history", + "notification_assignment_change", + "notification_assignment_release", + "notification_content_input", + "notification_unassigned_content", + ] { + names.push(name.into()); + } + for name in names { + let present: bool = conn.query_row( + "SELECT EXISTS(SELECT 1 FROM sqlite_schema WHERE name=?1)", + [&name], + |r| r.get(0), + )?; + if !present { + return Err(Error::Store(format!( + "Notification routing constraint {name} is missing" + ))); + } + } + Ok(()) +} + +fn read_source( + row: &rusqlite::Row<'_>, +) -> rusqlite::Result<( + i64, + String, + String, + String, + String, + String, + String, + Option, + Option, + String, +)> { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get(3)?, + row.get(4)?, + row.get(5)?, + row.get(6)?, + row.get(7)?, + row.get(8)?, + row.get(9)?, + )) +} +fn source_from_row(row: &rusqlite::Row<'_>) -> Result { + let (sequence, workspace, ticket, input, role, assignment, kind, actor, event_index, facts) = + read_source(row)?; + Ok(Source { + sequence, + workspace, + ticket, + input, + role, + assignment, + kind, + actor, + event_index, + facts: decode(&facts)?, + }) +} +fn stable_id(source: &Source) -> Result { + let identities = if source.role == "orchestrator" && !source.assignment.is_empty() { + json(&(&source.workspace, &source.assignment, &source.input))? + } else { + json(&( + &source.workspace, + &source.ticket, + &source.role, + &source.assignment, + &source.input, + ))? + }; + let digest = Sha256::digest(identities.as_bytes()); + let hex = digest + .iter() + .map(|b| format!("{b:02x}")) + .collect::(); + Ok(format!( + "{}:{hex}", + if source.role == "orchestrator" { + "assignment-delivery" + } else { + "worker-notification" + } + )) +} +fn self_notification(conn: &Connection, source: &Source) -> Result { + // Actor strings are authenticated audit attribution, not authorization grants. + if let (Some(runtime), Some(worker), Some(actor)) = ( + &source.facts.runtime_id, + &source.facts.worker_id, + &source.actor, + ) { + if actor == &format!("worker:{runtime}/{worker}") + || actor == &format!("authenticated-worker:{runtime}/{worker}") + { + return Ok(true); + } + } + if let Some(index) = source.event_index { + let pair: (Option,Option)=conn.query_row("SELECT MAX(CASE WHEN key='source_runtime_id' THEN value END),MAX(CASE WHEN key='source_worker_id' THEN value END) FROM typed_ticket_event_attributes WHERE workspace_id=?1 AND ticket_id=?2 AND event_index=?3",params![source.workspace,source.ticket,index],|r|Ok((r.get(0)?,r.get(1)?)))?; + if pair.0.is_some() && pair.0 == source.facts.runtime_id && pair.1 == source.facts.worker_id + { + return Ok(true); + } + } + Ok(false) +} +fn decision_for_source( + conn: &Connection, + source: &Source, +) -> Result<(&'static str, NotificationReason, NotificationPurpose)> { + let workspace = &source.workspace; + let kind = NotificationEventKind::from_source(&source.kind); + let (mut disposition, mut reason) = policy( + kind, + &source.role, + &source.assignment, + &source.facts, + self_notification(conn, source)?, + ); + if kind == NotificationEventKind::Answer && source.role == "orchestrator" { + let fence:Option=conn.query_row("SELECT value FROM typed_ticket_event_attributes WHERE workspace_id=?1 AND ticket_id=?2 AND event_index=?3 AND key='orchestrator_assignment_id'",params![workspace,source.ticket,source.event_index],|r|r.get(0)).optional()?; + if fence.as_deref() != Some(&source.assignment) { + disposition = "suppress"; + reason = NotificationReason::AssignmentChanged; + } + } + Ok((disposition, reason, purpose(kind, &source.role))) +} + +pub(super) fn route_pending(conn: &Connection, workspace: &str) -> Result<()> { + let mut stmt = conn.prepare("SELECT s.* FROM notification_sources s LEFT JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.workspace_id=?1 AND d.source_sequence IS NULL ORDER BY s.sequence LIMIT 500")?; + let mut rows = stmt.query([workspace])?; + while let Some(row) = rows.next()? { + let source = source_from_row(row)?; + let (disposition, reason, purpose) = decision_for_source(conn, &source)?; + let saved_id: Option = if source.role == "orchestrator" { + conn.query_row("SELECT delivery_id FROM ticket_assignment_deliveries WHERE workspace_id=?1 AND assignment_id=?2 AND input_event_id=?3", params![workspace,source.assignment,source.input], |r|r.get(0)).optional()? + } else { + None + }; + let id = match saved_id { + Some(id) => id, + None => stable_id(&source)?, + }; + conn.execute("INSERT OR IGNORE INTO notification_decisions(source_sequence,workspace_id,delivery_id,disposition,reason,purpose) VALUES(?1,?2,?3,?4,?5,?6)",params![source.sequence,workspace,id,disposition,code(reason)?,code(purpose)?])?; + } + Ok(()) +} + +// Preview only this assignment's sources in sequence order. Persisted decisions and +// receipts win; unrouted sources use exactly the delivery policy, without saving it. +// Streaming avoids a global routing batch cap and bounds retained rows to limit + 1. +pub(super) fn diagnostic_inputs( + conn: &Connection, + workspace: &str, + ticket: &str, + assignment: &str, + after: Option<&str>, + limit: usize, +) -> Result> { + let mut stmt = conn.prepare("SELECT s.*,d.disposition,r.delivery_id,EXISTS(SELECT 1 FROM ticket_assignment_delivery_requests p WHERE p.workspace_id=s.workspace_id AND p.delivery_id=r.delivery_id),EXISTS(SELECT 1 FROM ticket_assignment_delivery_acknowledgements a WHERE a.workspace_id=s.workspace_id AND a.delivery_id=r.delivery_id) FROM notification_sources s LEFT JOIN notification_decisions d ON d.source_sequence=s.sequence LEFT JOIN ticket_assignment_deliveries r ON r.workspace_id=s.workspace_id AND r.assignment_id=s.assignment_id AND r.input_event_id=s.input_event_id WHERE s.workspace_id=?1 AND s.ticket_id=?2 AND s.assignment_id=?3 AND s.role='orchestrator' ORDER BY s.sequence")?; + let mut rows = stmt.query(params![workspace, ticket, assignment])?; + let mut cursor_seen = after.is_none(); + let mut inputs = Vec::new(); + while let Some(row) = rows.next()? { + let source = source_from_row(row)?; + let disposition: Option = row.get(10)?; + let delivery_id: Option = row.get(11)?; + let eligible = match disposition { + Some(disposition) => disposition == "deliver", + None => decision_for_source(conn, &source)?.0 == "deliver", + }; + if !eligible && delivery_id.is_none() { + continue; + } + if !cursor_seen { + cursor_seen = after == Some(source.input.as_str()); + continue; + } + let prepared: bool = row.get(12)?; + let ack: bool = row.get(13)?; + use server_api::TicketAssignmentInputDeliveryState::*; + let notification = if ack { + RuntimeAccepted + } else if prepared { + RequestPersisted + } else if delivery_id.is_some() { + Reserved + } else { + NotReserved + }; + inputs.push(server_api::TicketAssignmentInputDiagnostic { + input_event_id: source.input, + delivery_id, + notification, + acceptance_outcome_unknown: prepared && !ack, + }); + if inputs.len() > limit { + break; + } + } + if !cursor_seen { + return Err(Error::InvalidInput( + "Input cursor does not belong to this assignment".into(), + )); + } + Ok(inputs) +} + +pub(super) fn require_current( + conn: &Connection, + workspace: &str, + ticket: &str, + assignment: &str, + role: &str, +) -> Result<()> { + let facts:Option=conn.query_row(&format!("SELECT {FACTS_SQL} FROM ticket_current_worker_assignments c JOIN ticket_worker_assignments a ON a.workspace_id=c.workspace_id AND a.assignment_id=c.assignment_id JOIN typed_tickets t ON t.workspace_id=a.workspace_id AND t.ticket_id=a.ticket_id WHERE c.workspace_id=?1 AND c.ticket_id=?2 AND c.assignment_id=?3 AND c.role=?4"),params![workspace,ticket,assignment,role],|r|r.get(0)).optional()?; + if let Some(facts) = facts { + let facts: Facts = decode(&facts)?; + if policy( + NotificationEventKind::ContentEdit, + role, + assignment, + &facts, + false, + ) + .0 == "deliver" + { + return Ok(()); + } + } + Err(Error::TicketAssignmentConflict( + "Notification assignment is no longer current actionable responsibility".into(), + )) +} +pub(super) fn pending_sources( + conn: &Connection, + workspace: &str, + role: &str, + limit: usize, +) -> Result> { + route_pending(conn, workspace)?; + let mut stmt=conn.prepare("SELECT s.ticket_id,s.assignment_id,s.input_event_id,d.delivery_id FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.workspace_id=?1 AND s.role=?2 AND d.disposition='deliver' AND NOT EXISTS(SELECT 1 FROM ticket_assignment_delivery_acknowledgements a WHERE a.workspace_id=?1 AND a.delivery_id=d.delivery_id) AND NOT EXISTS(SELECT 1 FROM notification_acknowledgements a WHERE a.workspace_id=?1 AND a.delivery_id=d.delivery_id) ORDER BY d.last_offered,s.sequence LIMIT ?3")?; + let candidates = stmt + .query_map(params![workspace, role, limit.clamp(1, 100) as i64], |r| { + Ok(( + r.get::<_, String>(0)?, + r.get::<_, String>(1)?, + r.get::<_, String>(2)?, + r.get::<_, String>(3)?, + )) + })? + .collect::>>()?; + let tick: i64 = conn.query_row( + "SELECT COALESCE(MAX(last_offered),0)+1 FROM notification_decisions WHERE workspace_id=?1", + [workspace], + |r| r.get(0), + )?; + let mut result = Vec::new(); + for row in candidates { + conn.execute("UPDATE notification_decisions SET last_offered=?3 WHERE workspace_id=?1 AND delivery_id=?2",params![workspace,row.3,tick])?; + match require_current(conn, workspace, &row.0, &row.1, role) { + Ok(()) => result.push(row), + Err(Error::TicketAssignmentConflict(_)) => { + save_failure( + conn, + workspace, + &row.3, + NotificationFailureCode::AssignmentChanged, + )?; + } + Err(error) => return Err(error), + } + } + Ok(result) +} +fn save_failure( + conn: &Connection, + workspace: &str, + id: &str, + failure: NotificationFailureCode, +) -> Result<()> { + let value = code(failure)?; + conn.execute("INSERT INTO notification_failures(workspace_id,delivery_id,code) SELECT ?1,?2,?3 WHERE NOT EXISTS(SELECT 1 FROM notification_failures WHERE sequence=(SELECT MAX(sequence) FROM notification_failures WHERE workspace_id=?1 AND delivery_id=?2) AND code=?3)",params![workspace,id,value])?; + Ok(()) +} +pub(super) fn failure( + store: &SqliteWorkspaceStore, + workspace: &str, + key: &NotificationDeliveryKey, + failure: NotificationFailureCode, +) -> Result<()> { + store.with_conn_mut(|conn| {let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + route_pending(&tx,workspace)?; + let id:String=match key { + NotificationDeliveryKey::WorkerNotification{delivery_id}=>tx.query_row("SELECT delivery_id FROM notification_decisions WHERE workspace_id=?1 AND delivery_id=?2",params![workspace,delivery_id],|r|r.get(0))?, + NotificationDeliveryKey::AssignmentInput{assignment_id,input_event_id}=>tx.query_row("SELECT d.delivery_id FROM notification_decisions d JOIN notification_sources s ON s.sequence=d.source_sequence WHERE s.workspace_id=?1 AND s.assignment_id=?2 AND s.input_event_id=?3 AND s.role='orchestrator'",params![workspace,assignment_id,input_event_id],|r|r.get(0))?, + }; + save_failure(&tx,workspace,&id,failure)?;tx.commit()?;Ok(()) + }) +} +pub(super) fn list_workers( + store: &SqliteWorkspaceStore, + workspace: &str, + limit: usize, +) -> Result> { + if limit == 0 || limit > 100 { + return Err(Error::InvalidInput( + "Notification limit must be 1..100".into(), + )); + } + store.with_conn_mut(|conn| {let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let mut result=Vec::new(); + for (ticket_id,assignment_id,input_event_id,delivery_id) in pending_sources(&tx,workspace,"worker",limit)? { + let (runtime,worker):(String,String)=tx.query_row("SELECT runtime_id,worker_id FROM ticket_worker_assignments WHERE workspace_id=?1 AND assignment_id=?2",params![workspace,assignment_id],|r|Ok((r.get(0)?,r.get(1)?)))?; + result.push(TicketWorkerNotification{ticket_id,assignment_id,input_event_id,request_id:delivery_id.clone(),delivery_id,worker:RuntimeWorkerRef::new(runtime,worker),purpose:NotificationPurpose::Attention}); + } + tx.commit()?;Ok(result) + }) +} +fn worker_guard( + conn: &Connection, + workspace: &str, + id: &str, + target: &TicketWorkerNotificationTarget, +) -> Result<()> { + let (ticket,assignment,runtime,worker):(String,String,String,String)=conn.query_row("SELECT s.ticket_id,s.assignment_id,json_extract(s.facts_json,'$.runtime_id'),json_extract(s.facts_json,'$.worker_id') FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE d.workspace_id=?1 AND d.delivery_id=?2 AND s.role='worker' AND d.disposition='deliver'",params![workspace,id],|r|Ok((r.get(0)?,r.get(1)?,r.get(2)?,r.get(3)?)))?; + require_current(conn, workspace, &ticket, &assignment, "worker")?; + if target.worker != RuntimeWorkerRef::new(runtime, worker) { + return Err(Error::WorkspacePermissionDenied( + "Notification target differs from captured assignment".into(), + )); + } + let valid:bool=conn.query_row("SELECT EXISTS(SELECT 1 FROM worker_registry w JOIN workspace_runtime_bindings b ON b.workspace_id=w.workspace_id AND b.runtime_id=w.runtime_id WHERE w.workspace_id=?1 AND w.runtime_id=?2 AND w.worker_id=?3 AND b.binding_id=?4 AND b.revoked_at IS NULL AND NOT EXISTS(SELECT 1 FROM worker_removal_operations r WHERE r.workspace_id=w.workspace_id AND r.runtime_id=w.runtime_id AND r.worker_id=w.worker_id AND r.state IN ('executing','failed','succeeded')))",params![workspace,target.worker.runtime_id,target.worker.worker_id,target.authentication_binding_id],|r|r.get(0))?; + if !valid { + return Err(Error::WorkspacePermissionDenied( + "Notification target binding is unavailable or revoked".into(), + )); + } + Ok(()) +} +pub(super) fn prepare_worker( + store: &SqliteWorkspaceStore, + workspace: &str, + id: &str, + content: &str, + target: &TicketWorkerNotificationTarget, +) -> Result { + if content.is_empty() || content.len() > 262144 || content.contains('\0') { + return Err(Error::InvalidInput( + "Notification content must be nonempty, NUL-free and at most 262144 bytes".into(), + )); + } + store.with_conn_mut(|conn| {let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + worker_guard(&tx,workspace,id,target)?; + let saved:Option<(String,String)>=tx.query_row("SELECT content,target_json FROM notification_requests WHERE workspace_id=?1 AND delivery_id=?2",params![workspace,id],|r|Ok((r.get(0)?,r.get(1)?))).optional()?; + let result=if let Some((saved,binding))=saved { + if binding!=json(target)? { return Err(Error::TicketAssignmentConflict("Prepared notification target binding changed".into())); } + saved + } else {tx.execute("INSERT INTO notification_requests VALUES(?1,?2,?3,?4)",params![workspace,id,content,json(target)?])?;content.to_owned()}; + tx.commit()?;Ok(result) + }) +} +pub(super) fn ack_worker( + store: &SqliteWorkspaceStore, + workspace: &str, + id: &str, + target: &TicketWorkerNotificationTarget, +) -> Result<()> { + store.with_conn_mut(|conn| {let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + // Acceptance is evidence about the prepared target, even if responsibility changed during I/O. + let saved:String=tx.query_row("SELECT target_json FROM notification_requests WHERE workspace_id=?1 AND delivery_id=?2",params![workspace,id],|r|r.get(0))?; + if saved!=json(target)? {return Err(Error::TicketAssignmentConflict("Acknowledgement target differs from prepared notification".into()));} + tx.execute("INSERT OR IGNORE INTO notification_acknowledgements VALUES(?1,?2,?3)",params![workspace,id,saved])?; + tx.commit()?;Ok(()) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn notification_routing_policy_distinguishes_requests_attention_results_and_suppression() { + let mut facts = Facts { + principal_kind: Some("workspace_agent".into()), + principal_id: Some("workspace-orchestrator".into()), + runtime_id: None, + worker_id: None, + status: Some("waiting".into()), + case_state: "open".into(), + released: false, + }; + use NotificationEventKind as K; + use NotificationPurpose as P; + use NotificationReason as R; + for (kind, disposition, reason, purpose_value) in [ + (K::Assignment, "deliver", R::Requested, P::NewDelegation), + (K::Answer, "deliver", R::Requested, P::ContinuingRequest), + ( + K::ContentEdit, + "deliver", + R::Requested, + P::ContinuingRequest, + ), + (K::Comment, "suppress", R::NotRequest, P::Attention), + (K::Case, "suppress", R::NotRequest, P::Attention), + (K::Tracking, "suppress", R::NotRequest, P::Attention), + (K::Tags, "suppress", R::NotRequest, P::Attention), + (K::Release, "suppress", R::NotRequest, P::Attention), + ( + K::AssignmentChanged, + "suppress", + R::NotRequest, + P::Attention, + ), + (K::Result, "deliver", R::Result, P::Result), + (K::Unknown, "defer", R::UnknownEvent, P::Attention), + ] { + assert_eq!( + policy(kind, "orchestrator", "a", &facts, false), + (disposition, reason) + ); + assert_eq!(purpose(kind, "orchestrator"), purpose_value); + } + facts.status = Some("done".into()); + assert_eq!( + policy(K::Result, "orchestrator", "a", &facts, false), + ("suppress", R::HandlingDone) + ); + assert_eq!( + purpose(K::Result, "orchestrator"), + P::Result, + "terminal result never changes into a new delegation" + ); + facts.principal_kind = Some("worker".into()); + facts.principal_id = None; + for kind in [ + K::Assignment, + K::Answer, + K::ContentEdit, + K::Comment, + K::Case, + K::Tags, + K::AssignmentChanged, + ] { + assert_eq!( + policy(kind, "worker", "a", &facts, false), + ("deliver", R::Attention) + ); + assert_eq!(purpose(kind, "worker"), P::Attention); + } + assert_eq!( + policy(K::Comment, "worker", "a", &facts, true), + ("suppress", R::SelfNotification) + ); + assert_eq!( + policy(K::Comment, "worker", "", &facts, false), + ("suppress", R::NoRecipient) + ); + facts.case_state = "closed".into(); + assert_eq!( + policy(K::Comment, "worker", "a", &facts, false), + ("suppress", R::CaseClosed) + ); + facts.released = true; + facts.case_state = "open".into(); + assert_eq!( + policy(K::Comment, "worker", "a", &facts, false), + ("suppress", R::HistoricalOnly) + ); + } +} diff --git a/crates/workspace-server/src/notification_routing.sql b/crates/workspace-server/src/notification_routing.sql new file mode 100644 index 000000000..3e65b4485 --- /dev/null +++ b/crates/workspace-server/src/notification_routing.sql @@ -0,0 +1,80 @@ +-- Source facts commit with the domain write. Policy belongs to Rust, not these triggers. +CREATE TABLE notification_sources ( + sequence INTEGER PRIMARY KEY AUTOINCREMENT, + workspace_id TEXT NOT NULL, ticket_id TEXT NOT NULL, input_event_id TEXT NOT NULL, + role TEXT NOT NULL, assignment_id TEXT NOT NULL, kind TEXT NOT NULL, + actor TEXT, event_index INTEGER, facts_json TEXT NOT NULL CHECK(json_valid(facts_json)), + UNIQUE(workspace_id,ticket_id,input_event_id,role,assignment_id), + FOREIGN KEY(workspace_id,ticket_id) REFERENCES typed_tickets(workspace_id,ticket_id) +); +CREATE TABLE notification_decisions ( + source_sequence INTEGER PRIMARY KEY REFERENCES notification_sources(sequence), + workspace_id TEXT NOT NULL, delivery_id TEXT NOT NULL, + disposition TEXT NOT NULL CHECK(disposition IN ('deliver','suppress','defer')), + reason TEXT NOT NULL, purpose TEXT NOT NULL, + last_offered INTEGER NOT NULL DEFAULT 0, + UNIQUE(workspace_id,delivery_id) +); +CREATE TABLE notification_requests ( + workspace_id TEXT NOT NULL, delivery_id TEXT NOT NULL, content TEXT NOT NULL, + target_json TEXT NOT NULL CHECK(json_valid(target_json)), + PRIMARY KEY(workspace_id,delivery_id), + FOREIGN KEY(workspace_id,delivery_id) REFERENCES notification_decisions(workspace_id,delivery_id) +); +CREATE TABLE notification_acknowledgements ( + workspace_id TEXT NOT NULL, delivery_id TEXT NOT NULL, + target_json TEXT NOT NULL CHECK(json_valid(target_json)), + PRIMARY KEY(workspace_id,delivery_id), + FOREIGN KEY(workspace_id,delivery_id) REFERENCES notification_requests(workspace_id,delivery_id) +); +CREATE TABLE notification_failures ( + sequence INTEGER PRIMARY KEY AUTOINCREMENT, workspace_id TEXT NOT NULL, + delivery_id TEXT NOT NULL, code TEXT NOT NULL, + recorded_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%fZ','now')), + FOREIGN KEY(workspace_id,delivery_id) REFERENCES notification_decisions(workspace_id,delivery_id) +); +-- Authorized send targets survive responsibility/binding changes during Runtime I/O. +-- Reservation evidence is sufficient even before rendering; it was authorized under +-- the same write lock. Existing payload/request/ACK rows are never rewritten. +CREATE TABLE notification_assignment_targets ( + workspace_id TEXT NOT NULL, delivery_id TEXT NOT NULL, + runtime_id TEXT NOT NULL, worker_id TEXT NOT NULL, agent_key TEXT NOT NULL, + authentication_binding_id TEXT NOT NULL, + PRIMARY KEY(workspace_id,delivery_id,runtime_id,worker_id,agent_key,authentication_binding_id), + FOREIGN KEY(workspace_id,delivery_id) REFERENCES ticket_assignment_deliveries(workspace_id,delivery_id) +); +INSERT INTO notification_assignment_targets + SELECT workspace_id,delivery_id,json_extract(reserved_source_json,'$.runtime_id'), + json_extract(reserved_source_json,'$.worker_id'),json_extract(reserved_source_json,'$.agent_key'), + json_extract(reserved_source_json,'$.authentication_binding_id') FROM ticket_assignment_deliveries; +-- A pre-upgrade ACK also proves its target passed the old current/source guards; +-- it may be a replacement different from the immutable original reservation. +INSERT OR IGNORE INTO notification_assignment_targets + SELECT workspace_id,delivery_id,json_extract(source_json,'$.runtime_id'), + json_extract(source_json,'$.worker_id'),json_extract(source_json,'$.agent_key'), + json_extract(source_json,'$.authentication_binding_id') FROM ticket_assignment_delivery_acknowledgements; +CREATE INDEX notification_sources_assignment ON notification_sources(workspace_id,ticket_id,assignment_id,role,sequence); +CREATE INDEX notification_sources_unrouted ON notification_sources(workspace_id,sequence); +CREATE INDEX notification_decisions_pending ON notification_decisions(workspace_id,disposition,last_offered,source_sequence); +CREATE INDEX notification_failures_delivery ON notification_failures(workspace_id,delivery_id,sequence); +-- Retain old content identities and rows; remove notification eligibility from capture. +DROP TRIGGER ticket_assignment_title_input; +DROP TRIGGER ticket_assignment_initial_post_input; +ALTER TABLE ticket_assignment_content_inputs ADD COLUMN actor TEXT; +ALTER TABLE ticket_assignment_content_inputs ADD COLUMN kind TEXT NOT NULL DEFAULT 'content_edit'; +ALTER TABLE ticket_assignment_content_inputs ADD COLUMN source_change_id TEXT REFERENCES typed_ticket_content_changes(change_id); +-- Keep the operational input identity while consuming complete domain facts, including editor. +CREATE TRIGGER ticket_assignment_title_input AFTER INSERT ON typed_ticket_content_changes +WHEN NEW.title_changed=1 BEGIN + INSERT INTO ticket_assignment_content_inputs(workspace_id,ticket_id,assignment_id,after_ticket_event_index,actor,kind,source_change_id) + SELECT NEW.workspace_id,NEW.ticket_id,c.assignment_id, + COALESCE((SELECT MAX(event_index) FROM typed_ticket_events WHERE workspace_id=NEW.workspace_id AND ticket_id=NEW.ticket_id),-1),NEW.actor,NEW.kind,NEW.change_id + FROM ticket_current_worker_assignments c WHERE c.workspace_id=NEW.workspace_id AND c.ticket_id=NEW.ticket_id; +END; +CREATE TRIGGER ticket_assignment_initial_post_input AFTER INSERT ON typed_ticket_content_changes +WHEN NEW.title_changed=0 BEGIN + INSERT INTO ticket_assignment_content_inputs(workspace_id,ticket_id,assignment_id,after_ticket_event_index,actor,kind,source_change_id) + SELECT NEW.workspace_id,NEW.ticket_id,c.assignment_id, + COALESCE((SELECT MAX(event_index) FROM typed_ticket_events WHERE workspace_id=NEW.workspace_id AND ticket_id=NEW.ticket_id),-1),NEW.actor,NEW.kind,NEW.change_id + FROM ticket_current_worker_assignments c WHERE c.workspace_id=NEW.workspace_id AND c.ticket_id=NEW.ticket_id; +END; diff --git a/crates/workspace-server/src/notification_routing_tests.rs b/crates/workspace-server/src/notification_routing_tests.rs new file mode 100644 index 000000000..ac62b6a32 --- /dev/null +++ b/crates/workspace-server/src/notification_routing_tests.rs @@ -0,0 +1,929 @@ +// Real SQLite through the production Store. Runtime acceptance is supplied by the +// caller here; server_assignment_delivery_tests owns the real NotifyTracked boundary. +fn routing_worker_assign(store: &SqliteWorkspaceStore, id: &str) -> TicketWorkerNotificationTarget { + let assignment = TicketRoleAssignmentRecord { + principal: TicketAssignmentPrincipal::Worker { + runtime_id: "r".into(), + worker_id: "replacement".into(), + }, + ..manual_worker_assignment("tracking", id) + }; + let current = store + .get_current_ticket_role_assignment("w", "tracking", TicketAssignmentRole::Worker) + .unwrap(); + store + .set_current_ticket_role_assignment( + &assignment, + current.as_ref().map(|a| a.assignment_id.as_str()), + &format!("event-{id}"), + &format!("operation-{id}"), + true, + ) + .unwrap(); + TicketWorkerNotificationTarget { + worker: RuntimeWorkerRef::new("r", "replacement"), + authentication_binding_id: "tracking-binding".into(), + } +} +fn routing_ack_worker_pending( + store: &SqliteWorkspaceStore, + target: &TicketWorkerNotificationTarget, +) { + for input in store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + { + store + .prepare_ticket_worker_notification_content( + "w", + &input.delivery_id, + "initial notice", + target, + ) + .unwrap(); + store + .ack_ticket_worker_notification("w", &input.delivery_id, target) + .unwrap(); + } +} + +#[tokio::test] +async fn notification_routing_diagnostics_preview_unrouted_requests_readonly_beyond_global_batch() { + let (_directory, store) = tracking_store_fixture().await; + seed_manual_start_ticket(&store, "other-ticket", "open"); + store.with_conn_mut(|conn| { + let tx = conn.transaction()?; + for index in 0..260 { + tx.execute("INSERT INTO typed_ticket_events(workspace_id,ticket_id,event_index,event_ref,kind,author,body) VALUES('w','other-ticket',?1,'other:' || ?1,'comment','human','unrelated')",[index])?; + } + tx.commit()?; + Ok(()) + }).unwrap(); + tracking_store_assign(&store, "a", "assign-a"); + delivery_store_answer(&store, "preview-answer"); + // An answer without the assignment fence must be suppressed by the same Rust policy. + delivery_store_external_event(&store, 1, "assignment_answer", None); + let snapshot = || { + store + .with_conn(|conn| { + let changes: i64 = conn.query_row("SELECT total_changes()", [], |r| r.get(0))?; + let rows = [ + "notification_sources", + "notification_decisions", + "notification_requests", + "notification_failures", + "notification_acknowledgements", + "notification_assignment_targets", + "ticket_assignment_deliveries", + "ticket_assignment_delivery_requests", + "ticket_assignment_delivery_acknowledgements", + ] + .map(|table| stored_fixture_rows(conn, table)); + Ok((changes, rows)) + }) + .unwrap() + }; + let before = snapshot(); + store + .with_conn(|conn| { + conn.execute_batch("PRAGMA query_only=ON")?; + Ok(()) + }) + .unwrap(); + let first = store + .page_ticket_assignment_diagnostics("w", "tracking", "a", None, None, 1) + .unwrap(); + assert_eq!(first.inputs.len(), 1); + assert_eq!( + first.inputs[0].input_event_id, + "assignment-event:event-assign-a" + ); + assert_eq!( + first.inputs[0].notification, + server_api::TicketAssignmentInputDeliveryState::NotReserved + ); + assert_eq!( + first.next_input_event_id.as_deref(), + Some("assignment-event:event-assign-a") + ); + let next = store + .page_ticket_assignment_diagnostics( + "w", + "tracking", + "a", + first.next_input_event_id.as_deref(), + None, + 1, + ) + .unwrap(); + assert_eq!(next.inputs.len(), 1); + assert_eq!(next.inputs[0].input_event_id, "ticket-event:0"); + assert!(next.next_input_event_id.is_none()); + for (ticket, assignment, input, start) in [ + ("tracking", "missing", None, None), + ("other-ticket", "a", None, None), + ("tracking", "a", Some("missing"), None), + ("tracking", "a", Some("ticket-event:1"), None), + ("tracking", "a", None, Some("missing")), + ] { + assert!( + store + .page_ticket_assignment_diagnostics("w", ticket, assignment, input, start, 1) + .is_err() + ); + } + assert_eq!( + snapshot(), + before, + "diagnostics must not mutate even on invalid cursors/assignment" + ); + store + .with_conn(|conn| { + conn.execute_batch("PRAGMA query_only=OFF")?; + Ok(()) + }) + .unwrap(); + // Only the delivery path persists routing, in bounded batches. + assert!( + store + .list_pending_ticket_assignment_deliveries("w", 100) + .unwrap() + .is_empty() + ); + let pending = store + .list_pending_ticket_assignment_deliveries("w", 100) + .unwrap(); + assert_eq!( + pending + .iter() + .map(|d| d.input_event_id.as_str()) + .collect::>(), + vec![ + first.inputs[0].input_event_id.as_str(), + next.inputs[0].input_event_id.as_str() + ] + ); + let persisted = snapshot(); + let page = store + .page_ticket_assignment_diagnostics("w", "tracking", "a", None, None, 100) + .unwrap(); + assert_eq!(page.inputs.len(), 2); + assert_eq!(snapshot(), persisted); +} + +#[tokio::test] +async fn notification_routing_orchestrator_ack_retains_authorized_target_after_responsibility_changes() + { + for change in [ + "done", + "release", + "reassign", + "revoked", + "binding-replaced", + "worker-replaced", + ] { + for prepare in [false, true] { + let (_directory, store) = tracking_store_fixture().await; + tracking_store_assign(&store, "a", "assign-a"); + let source = tracking_store_source(); + let delivery = delivery_store_reserve_initial(&store, &source); + if prepare { + store + .prepare_ticket_assignment_delivery_content( + "w", + &delivery.delivery_id, + "immutable request", + &source, + ) + .unwrap(); + } + match change { + "done" => { store.update_ticket_assignment_tracking("w", "tracking", &tracking_store_input("a", "event-assign-a", "done-a"), &source).unwrap(); } + "release" => { store.clear_current_ticket_role_assignment("w", "tracking", TicketAssignmentRole::Orchestrator, "a", "clear-a", "clear-a", "human", "now", None).unwrap(); } + "reassign" => { tracking_store_assign(&store, "b", "assign-b"); } + _ => store.with_conn(|conn| { + match change { + "revoked" => conn.execute("UPDATE workspace_runtime_bindings SET revoked_at='revoked' WHERE workspace_id='w'", [])?, + "binding-replaced" => conn.execute("UPDATE workspace_runtime_bindings SET binding_id='replacement-binding' WHERE workspace_id='w'", [])?, + _ => conn.execute("UPDATE worker_singleton_owners SET worker_id='replacement' WHERE workspace_id='w'", [])?, + }; + Ok(()) + }).unwrap(), + } + for field in ["runtime", "worker", "agent", "binding"] { + let mut forged = source.clone(); + match field { + "runtime" => forged.runtime_id = "forged".into(), + "worker" => forged.worker_id = "replacement".into(), + "agent" => forged.agent_key = "forged".into(), + _ => forged.authentication_binding_id = "replacement-binding".into(), + } + assert!( + matches!( + store.ack_ticket_assignment_delivery("w", &delivery.delivery_id, &forged), + Err(Error::WorkspacePermissionDenied(_)) + ), + "{change}/{field}" + ); + } + assert!( + store + .ack_ticket_assignment_delivery("other", &delivery.delivery_id, &source) + .is_err() + ); + store + .ack_ticket_assignment_delivery("w", &delivery.delivery_id, &source) + .unwrap(); + store + .ack_ticket_assignment_delivery("w", &delivery.delivery_id, &source) + .unwrap(); + assert!( + store + .reserve_ticket_assignment_delivery("w", "a", &delivery.input_event_id, &source) + .is_err(), + "{change}" + ); + assert!( + store + .prepare_ticket_assignment_delivery_content( + "w", + &delivery.delivery_id, + "retry", + &source + ) + .is_err(), + "{change}" + ); + let page = store + .page_ticket_assignment_diagnostics("w", "tracking", "a", None, None, 100) + .unwrap(); + let input = page + .inputs + .iter() + .find(|i| i.input_event_id == delivery.input_event_id) + .unwrap(); + assert_eq!( + input.notification, + server_api::TicketAssignmentInputDeliveryState::RuntimeAccepted + ); + assert!(!input.acceptance_outcome_unknown); + store.with_conn(|conn| { + assert_eq!(stored_fixture_rows(conn, "ticket_assignment_delivery_acknowledgements").len(), 1); + let payload: String = conn.query_row("SELECT payload_json FROM ticket_assignment_deliveries WHERE delivery_id=?1", [&delivery.delivery_id], |r| r.get(0))?; + assert_eq!(serde_json::from_str::(&payload).unwrap(), delivery); + Ok(()) + }).unwrap(); + } + } +} + +#[tokio::test] +async fn notification_routing_replacement_target_requires_explicit_authorized_reserve_or_prepare() { + for prepare in [false, true] { + let (_directory, store) = tracking_store_fixture().await; + tracking_store_assign(&store, "a", "assign-a"); + let source = tracking_store_source(); + let delivery = delivery_store_reserve_initial(&store, &source); + store + .prepare_ticket_assignment_delivery_content( + "w", + &delivery.delivery_id, + "original request", + &source, + ) + .unwrap(); + store.with_conn(|conn| { + conn.execute("UPDATE worker_singleton_owners SET worker_id='replacement' WHERE workspace_id='w'", [])?; + Ok(()) + }).unwrap(); + let replacement = TicketAssignmentTrackingSourceContext { + worker_id: "replacement".into(), + ..source.clone() + }; + assert!( + store + .ack_ticket_assignment_delivery("w", &delivery.delivery_id, &replacement) + .is_err() + ); + if prepare { + assert_eq!( + store + .prepare_ticket_assignment_delivery_content( + "w", + &delivery.delivery_id, + "different rendering", + &replacement + ) + .unwrap(), + "original request" + ); + } else { + assert_eq!( + store + .reserve_ticket_assignment_delivery( + "w", + "a", + &delivery.input_event_id, + &replacement + ) + .unwrap(), + delivery + ); + } + store + .ack_ticket_assignment_delivery("w", &delivery.delivery_id, &replacement) + .unwrap(); + // A different target's receipt cannot overwrite the first ACK, even if both were authorized. + assert!( + store + .ack_ticket_assignment_delivery("w", &delivery.delivery_id, &source) + .is_err() + ); + store + .with_conn(|conn| { + assert_eq!( + stored_fixture_rows(conn, "notification_assignment_targets").len(), + 2 + ); + assert_eq!( + stored_fixture_rows(conn, "ticket_assignment_deliveries").len(), + 1 + ); + assert_eq!( + stored_fixture_rows(conn, "ticket_assignment_delivery_requests").len(), + 1 + ); + assert_eq!( + stored_fixture_rows(conn, "ticket_worker_assignments").len(), + 1 + ); + assert!( + conn.execute( + "UPDATE notification_assignment_targets SET worker_id='forged'", + [] + ) + .is_err() + ); + assert!( + conn.execute("DELETE FROM notification_assignment_targets", []) + .is_err() + ); + Ok(()) + }) + .unwrap(); + } +} + +#[tokio::test] +async fn notification_routing_commit_without_callback_recovers_and_rollback_has_no_source() { + let (directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + store.with_conn_mut(|conn| { + let tx=conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + tx.execute("INSERT INTO typed_ticket_events(workspace_id,ticket_id,event_index,event_ref,kind,author,body) VALUES('w','tracking',0,'tracking:0','comment','human','rollback')",[])?; + tx.rollback()?; + let count:i64=conn.query_row("SELECT COUNT(*) FROM notification_sources WHERE input_event_id='ticket-event:0'",[],|r|r.get(0))?; + assert_eq!(count,0);Ok(()) + }).unwrap(); + delivery_store_external_event(&store, 0, "comment", None); + store.with_conn(|conn| { + let count:i64=conn.query_row("SELECT COUNT(*) FROM notification_sources s LEFT JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.input_event_id='ticket-event:0' AND d.source_sequence IS NULL",[],|r|r.get(0))?; + assert_eq!(count,2,"both logical destinations are captured before routing");Ok(()) + }).unwrap(); + drop(store); + let store = SqliteWorkspaceStore::open_for_test(directory.path().join("workspace.db")).unwrap(); + let inputs = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap(); + assert_eq!(inputs.len(), 1); + assert_eq!(inputs[0].input_event_id, "ticket-event:0"); + assert_eq!(inputs[0].assignment_id, "worker-a"); + assert_eq!(inputs[0].purpose, NotificationPurpose::Attention); + store.with_conn(|conn| { + let reason:String=conn.query_row("SELECT d.reason FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.input_event_id='ticket-event:0' AND s.role='orchestrator'",[],|r|r.get(0))?; + assert_eq!(reason,"no_recipient");Ok(()) + }).unwrap(); +} + +#[tokio::test] +async fn notification_routing_parallel_scan_and_render_reuse_request_after_response_loss() { + let (directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + delivery_store_external_event(&store, 0, "comment", None); + let barrier = std::sync::Arc::new(std::sync::Barrier::new(2)); + let threads = (0..2) + .map(|i| { + let barrier = barrier.clone(); + let path = directory.path().join("workspace.db"); + let target = target.clone(); + std::thread::spawn(move || { + let store = SqliteWorkspaceStore::open_for_test(path).unwrap(); + barrier.wait(); + let input = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .remove(0); + let text = store + .prepare_ticket_worker_notification_content( + "w", + &input.delivery_id, + &format!("rendered {i}"), + &target, + ) + .unwrap(); + (input, text) + }) + }) + .collect::>(); + let receipts = threads + .into_iter() + .map(|t| t.join().unwrap()) + .collect::>(); + assert_eq!(receipts[0], receipts[1]); + let (input, content) = &receipts[0]; + drop(store); + let store = SqliteWorkspaceStore::open_for_test(directory.path().join("workspace.db")).unwrap(); + assert_eq!( + &store + .prepare_ticket_worker_notification_content( + "w", + &input.delivery_id, + "new rendering", + &target + ) + .unwrap(), + content + ); + assert_eq!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap(), + vec![input.clone()] + ); + store + .ack_ticket_worker_notification("w", &input.delivery_id, &target) + .unwrap(); + store + .ack_ticket_worker_notification("w", &input.delivery_id, &target) + .unwrap(); + assert!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .is_empty() + ); + store + .with_conn(|conn| { + assert!( + conn.execute("UPDATE notification_requests SET content='changed'", []) + .is_err() + ); + assert!( + conn.execute("DELETE FROM notification_acknowledgements", []) + .is_err() + ); + let count: i64 = conn.query_row( + "SELECT COUNT(*) FROM notification_requests WHERE delivery_id=?1", + [&input.delivery_id], + |r| r.get(0), + )?; + assert_eq!(count, 1); + Ok(()) + }) + .unwrap(); +} + +#[tokio::test] +async fn notification_routing_assignment_change_between_commit_and_scan_never_transfers_input() { + let (_directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + delivery_store_external_event(&store, 0, "comment", None); + routing_worker_assign(&store, "worker-b"); + let pending = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap(); + assert!(pending.iter().all(|i| i.assignment_id == "worker-b")); + assert!(pending.iter().all(|i| i.input_event_id != "ticket-event:0")); + store.with_conn(|conn| { + let captured:String=conn.query_row("SELECT assignment_id FROM notification_sources WHERE input_event_id='ticket-event:0' AND role='worker'",[],|r|r.get(0))?; + assert_eq!(captured,"worker-a"); + let failure:String=conn.query_row("SELECT f.code FROM notification_failures f JOIN notification_decisions d USING(workspace_id,delivery_id) JOIN notification_sources s ON s.sequence=d.source_sequence WHERE s.input_event_id='ticket-event:0' AND s.role='worker'",[],|r|r.get(0))?; + assert_eq!(failure,"assignment_changed");Ok(()) + }).unwrap(); +} + +#[tokio::test] +async fn notification_routing_ack_retains_acceptance_after_responsibility_changes() { + let (_directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + delivery_store_external_event(&store, 0, "comment", None); + let input = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .remove(0); + store + .prepare_ticket_worker_notification_content( + "w", + &input.delivery_id, + "accepted during reassignment", + &target, + ) + .unwrap(); + routing_worker_assign(&store, "worker-b"); + assert!( + store + .prepare_ticket_worker_notification_content("w", &input.delivery_id, "retry", &target) + .is_err() + ); + store + .ack_ticket_worker_notification("w", &input.delivery_id, &target) + .unwrap(); + store + .with_conn(|conn| { + let count: i64 = conn.query_row( + "SELECT COUNT(*) FROM notification_acknowledgements WHERE delivery_id=?1", + [input.delivery_id], + |r| r.get(0), + )?; + assert_eq!(count, 1); + Ok(()) + }) + .unwrap(); +} + +#[tokio::test] +async fn notification_routing_editor_is_not_original_author_and_unknown_events_are_deferred() { + let (directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + use ticket::TicketBackend; + let backend = + ticket::SqliteTicketBackend::open_verified(directory.path().join("workspace.db"), "w") + .unwrap(); + backend + .edit_item( + "tracking".into(), + ticket::TicketItemEdit { + body: Some(ticket::MarkdownText::new("Worker edited")), + author: Some("worker:r/replacement".into()), + ..Default::default() + }, + ) + .unwrap(); + assert!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .is_empty() + ); + backend + .edit_item( + "tracking".into(), + ticket::TicketItemEdit { + title: Some("Worker title edit".into()), + author: Some("worker:r/replacement".into()), + ..Default::default() + }, + ) + .unwrap(); + assert!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .is_empty(), + "title-only edits retain the editor and suppress self-notification" + ); + backend + .edit_item( + "tracking".into(), + ticket::TicketItemEdit { + body: Some(ticket::MarkdownText::new("Human edited")), + author: Some("human".into()), + ..Default::default() + }, + ) + .unwrap(); + let pending = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap(); + assert_eq!(pending.len(), 1); + delivery_store_external_event(&store, 0, "future_event", None); + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap(); + store.with_conn(|conn| { + let self_reason:String=conn.query_row("SELECT d.reason FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.actor='worker:r/replacement' AND s.role='worker'",[],|r|r.get(0))?; + assert_eq!(self_reason,"self_notification"); + let decision:(String,String)=conn.query_row("SELECT d.disposition,d.reason FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.kind='future_event' AND s.role='worker'",[],|r|Ok((r.get(0)?,r.get(1)?)))?; + assert_eq!(decision,("defer".into(),"unknown_event".into()));Ok(()) + }).unwrap(); +} + +#[tokio::test] +async fn notification_routing_failures_do_not_starve_other_inputs_or_replace_requests() { + let (_directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + delivery_store_external_event(&store, 0, "comment", None); + delivery_store_external_event(&store, 1, "comment", None); + let first = store + .list_pending_ticket_worker_notifications("w", 1) + .unwrap() + .remove(0); + let key = NotificationDeliveryKey::WorkerNotification { + delivery_id: first.delivery_id.clone(), + }; + store + .prepare_ticket_worker_notification_content("w", &first.delivery_id, "pinned", &target) + .unwrap(); + for _ in 0..2 { + store + .record_notification_delivery_failure( + "w", + &key, + NotificationFailureCode::AcceptanceUnknown, + ) + .unwrap(); + } + let second = store + .list_pending_ticket_worker_notifications("w", 1) + .unwrap() + .remove(0); + assert_ne!(first.delivery_id, second.delivery_id); + assert_eq!( + store + .prepare_ticket_worker_notification_content("w", &first.delivery_id, "changed", &target) + .unwrap(), + "pinned" + ); + store + .with_conn(|conn| { + let failures: Vec = conn + .prepare("SELECT code FROM notification_failures WHERE delivery_id=?1")? + .query_map([first.delivery_id], |r| r.get(0))? + .collect::>()?; + assert_eq!(failures, vec!["acceptance_unknown"]); + Ok(()) + }) + .unwrap(); +} + +#[tokio::test] +async fn notification_routing_target_binding_and_workspace_are_rechecked_before_prepare() { + let (_directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + let input = store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .remove(0); + let wrong = TicketWorkerNotificationTarget { + worker: RuntimeWorkerRef::new("r", "worker"), + ..target.clone() + }; + assert!( + store + .prepare_ticket_worker_notification_content("w", &input.delivery_id, "text", &wrong) + .is_err() + ); + assert!( + store + .prepare_ticket_worker_notification_content( + "other", + &input.delivery_id, + "text", + &target + ) + .is_err() + ); + store + .with_conn(|conn| { + conn.execute( + "UPDATE workspace_runtime_bindings SET revoked_at='revoked' WHERE workspace_id='w'", + [], + )?; + Ok(()) + }) + .unwrap(); + assert!( + store + .prepare_ticket_worker_notification_content("w", &input.delivery_id, "text", &target) + .is_err() + ); +} + +#[test] +fn notification_routing_migration_preserves_receipts_does_not_replay_comments_and_rolls_back() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("workspace.db"); + let conn = tracking_schema91_fixture(&path); + migrate_ticket_tracking_v91_to_v92(&conn).unwrap(); + migrate_assignment_delivery_v92_to_v93(&conn).unwrap(); + migrate_ticket_posts_v93_to_v94(&conn).unwrap(); + conn.execute("INSERT INTO typed_ticket_events(workspace_id,ticket_id,event_index,event_ref,kind,author,body) VALUES('w','pending',999,'pending:999','comment','human','historical')",[]).unwrap(); + let payload = TicketAssignmentDelivery { + ticket_id: "pending".into(), + assignment_id: "a-pending".into(), + input_event_id: "assignment-event:migration-tracking:a-pending".into(), + delivery_id: "preserved-id".into(), + request_id: "preserved-request".into(), + waiting_reason: None, + next_action: None, + }; + conn.execute( + "INSERT INTO ticket_assignment_deliveries VALUES('w','a-pending',?1,'preserved-id',?2,?3)", + params![ + payload.input_event_id, + serde_json::to_string(&payload).unwrap(), + serde_json::to_string(&tracking_store_source()).unwrap() + ], + ) + .unwrap(); + conn.execute("INSERT INTO ticket_assignment_delivery_requests VALUES('w','preserved-id','immutable request')",[]).unwrap(); + let acknowledged_target = TicketAssignmentTrackingSourceContext { + worker_id: "old-replacement".into(), + ..tracking_store_source() + }; + conn.execute( + "INSERT INTO ticket_assignment_delivery_acknowledgements VALUES('w','preserved-id',?1)", + [serde_json::to_string(&acknowledged_target).unwrap()], + ) + .unwrap(); + let requests_before = stored_fixture_rows(&conn, "ticket_assignment_delivery_requests"); + let acks_before = stored_fixture_rows(&conn, "ticket_assignment_delivery_acknowledgements"); + let before = stored_fixture_rows(&conn, "ticket_assignment_deliveries"); + conn.execute_batch("CREATE TRIGGER notification_ticket_event AFTER INSERT ON typed_ticket_events BEGIN SELECT 1; END;").unwrap(); + assert!(migrate_notification_routing_v94_to_v95(&conn).is_err()); + assert_eq!(current_schema_version(&conn).unwrap(), 94); + let missing: bool = conn + .query_row( + "SELECT NOT EXISTS(SELECT 1 FROM sqlite_schema WHERE name='notification_sources')", + [], + |r| r.get(0), + ) + .unwrap(); + assert!(missing); + assert!( + conn.prepare("SELECT actor FROM ticket_assignment_content_inputs") + .is_err() + ); + conn.execute_batch("DROP TRIGGER notification_ticket_event;") + .unwrap(); + migrate_notification_routing_v94_to_v95(&conn).unwrap(); + assert_eq!( + stored_fixture_rows(&conn, "ticket_assignment_deliveries"), + before + ); + assert_eq!( + stored_fixture_rows(&conn, "ticket_assignment_delivery_requests"), + requests_before + ); + assert_eq!( + stored_fixture_rows(&conn, "ticket_assignment_delivery_acknowledgements"), + acks_before + ); + assert_eq!( + stored_fixture_rows(&conn, "notification_assignment_targets").len(), + 2 + ); + let old_comments: i64 = conn + .query_row( + "SELECT COUNT(*) FROM notification_sources WHERE input_event_id='ticket-event:999'", + [], + |r| r.get(0), + ) + .unwrap(); + assert_eq!(old_comments, 0); + drop(conn); + let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); + let pending = store + .list_pending_ticket_assignment_deliveries("w", 100) + .unwrap(); + assert!( + !pending.iter().any(|p| p.assignment_id == "a-pending"), + "saved ACK remains authoritative even with non-derived legacy delivery ID" + ); + store + .ack_ticket_assignment_delivery("w", "preserved-id", &acknowledged_target) + .unwrap(); +} + +#[tokio::test] +async fn notification_routing_missing_authorized_target_guard_refuses_startup() { + for guard in [ + "notification_assignment_targets_no_update", + "notification_assignment_targets_no_delete", + ] { + let (directory, store) = tracking_store_fixture().await; + store + .with_conn(|conn| { + conn.execute_batch(&format!("DROP TRIGGER {guard}"))?; + Ok(()) + }) + .unwrap(); + drop(store); + let error = match SqliteWorkspaceStore::open_for_test(directory.path().join("workspace.db")) + { + Ok(_) => panic!("missing target evidence guard accepted"), + Err(error) => error, + }; + assert!(error.to_string().contains(guard)); + } +} + +#[tokio::test] +async fn notification_routing_fresh_and_upgraded_schema_and_guards_match() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("upgraded.db"); + let conn = tracking_schema91_fixture(&path); + migrate_ticket_tracking_v91_to_v92(&conn).unwrap(); + migrate_assignment_delivery_v92_to_v93(&conn).unwrap(); + migrate_ticket_posts_v93_to_v94(&conn).unwrap(); + migrate_notification_routing_v94_to_v95(&conn).unwrap(); + let snapshot = |conn: &Connection| -> Vec<(String, String)> { + conn.prepare("SELECT name,sql FROM sqlite_schema WHERE name LIKE 'notification_%' OR name LIKE 'ticket_assignment_content%' OR name IN ('ticket_assignment_title_input','ticket_assignment_initial_post_input') ORDER BY name").unwrap().query_map([],|r|Ok((r.get(0)?,r.get(1)?))).unwrap().collect::>().unwrap() + }; + let upgraded = snapshot(&conn); + let (fresh_directory, fresh) = tracking_store_fixture().await; + fresh + .with_conn(|conn| { + assert_eq!(snapshot(conn), upgraded); + Ok(()) + }) + .unwrap(); + fresh + .with_conn(|conn| { + conn.execute_batch("DROP TRIGGER notification_sources_no_update;")?; + Ok(()) + }) + .unwrap(); + drop(fresh); + let error = + match SqliteWorkspaceStore::open_for_test(fresh_directory.path().join("workspace.db")) { + Ok(_) => panic!("missing guard accepted"), + Err(error) => error, + }; + assert!(error.to_string().contains("notification_sources_no_update")); +} + +#[tokio::test] +async fn notification_routing_content_without_recipient_records_reason_and_domain_reference() { + let (directory, store) = tracking_store_fixture().await; + use ticket::TicketBackend; + let backend = + ticket::SqliteTicketBackend::open_verified(directory.path().join("workspace.db"), "w") + .unwrap(); + backend + .edit_item( + "tracking".into(), + ticket::TicketItemEdit { + title: Some("No recipient yet".into()), + author: Some("human".into()), + ..Default::default() + }, + ) + .unwrap(); + assert!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .is_empty() + ); + store.with_conn(|conn| { + let count:i64=conn.query_row("SELECT COUNT(*) FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence JOIN typed_ticket_content_changes c ON s.input_event_id='ticket-content:' || c.change_id WHERE d.reason='no_recipient' AND s.actor='human'",[],|r|r.get(0))?; + assert_eq!(count,2);Ok(()) + }).unwrap(); +} + +#[tokio::test] +async fn notification_routing_clear_retains_original_assignment_and_suppression_reason() { + let (_directory, store) = tracking_store_fixture().await; + let target = routing_worker_assign(&store, "worker-a"); + routing_ack_worker_pending(&store, &target); + store + .clear_current_ticket_role_assignment( + "w", + "tracking", + TicketAssignmentRole::Worker, + "worker-a", + "clear-a", + "clear-a", + "human", + "now", + None, + ) + .unwrap(); + assert!( + store + .list_pending_ticket_worker_notifications("w", 100) + .unwrap() + .is_empty() + ); + store.with_conn(|conn| { + let row:(String,String)=conn.query_row("SELECT s.assignment_id,d.reason FROM notification_sources s JOIN notification_decisions d ON d.source_sequence=s.sequence WHERE s.input_event_id='assignment-change:clear-a' AND s.assignment_id='worker-a'",[],|r|Ok((r.get(0)?,r.get(1)?)))?; + assert_eq!(row,("worker-a".into(),"historical_only".into()));Ok(()) + }).unwrap(); +} diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 4f939d7f8..3a3459592 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -1568,6 +1568,13 @@ impl workdir::WorkdirSession for ExternalProviderWorkdirSession { } } +#[path = "notification_delivery.rs"] +mod notification_delivery; +use crate::store::NotificationFailureCode; +use notification_delivery::{ + assignment_delivery_key, record_notification_failure, recover_ticket_notifications, +}; + // Per-recipient reservation. No lock is held across Runtime calls: subscription // callbacks may reenter the attention path while send_input runs. #[derive(Default)] @@ -1641,6 +1648,7 @@ pub struct WorkspaceApi { orchestrator_spawn_lock: Arc>, orchestrator_attention: Arc>>>>, + notification_delivery_lifecycle: Arc, backend_job_drain_requests: Arc, observation_proxy: BackendObservationProxy, runtime_subscription_broker: RuntimeSubscriptionBroker, @@ -2682,6 +2690,9 @@ impl WorkspaceServerApi { } async fn shutdown_feature_storage(&self) -> Result<()> { + for (_, hook) in self.hook_handles.lock().await.drain() { + hook.abort(); + } let apis = self.apis.lock().await.values().cloned().collect::>(); for api in apis { shutdown_workspace_feature_storage(&api).await?; @@ -3878,6 +3889,7 @@ impl WorkspaceApi { runtime_binding_expectations: Arc::new(RwLock::new(HashMap::new())), companion, orchestrator_spawn_lock: Arc::new(std::sync::Mutex::new(())), + notification_delivery_lifecycle: Arc::new(Default::default()), orchestrator_attention: Arc::new(Mutex::new(HashMap::new())), backend_job_drain_requests: Arc::new(AtomicU64::new(0)), observation_proxy, @@ -11190,9 +11202,7 @@ impl server_api::ServerApi for ServerApiContractService { .map_err(ApiError::into_repository_api_error)?; // The entire event/audit/receipt transaction has committed. Response loss // retries the saved receipt; delivery scan deduplicates accepted inputs. - if let Some(orchestrator) = find_workspace_orchestrator(&api) { - dispatch_orchestrator_assignment_attention(&api, &orchestrator.worker); - } + recover_ticket_notifications(&api); Ok(server_api::TicketAssignmentAnswerResponse(receipt)) }) .await @@ -12640,6 +12650,9 @@ pub async fn serve( // Feature shutdown fences new operations and waits for synchronous connection // holders. Accepted close retains ownership after its async waiter disappears. async fn shutdown_workspace_feature_storage(api: &WorkspaceApi) -> Result<()> { + // Aborting the async hook cannot cancel an already admitted Host closure. + // Fence all delivery entry points and wait for their actual I/O owners first. + api.notification_delivery_lifecycle.stop_and_drain().await; api.worker_projection.shutdown_async().await; let storage = api.feature_storage.clone(); api.store @@ -16186,8 +16199,9 @@ fn execute_ticket_rest_operation_on_host( None, ), }; - // Classification has no routing or execution side effects. - if is_mutation && !matches!(operation_kind, "mutate_workspace_tag" | "set_ticket_tags") { + // All committed facts enter the same routing policy, including reasons for + // non-notification. Waking the dispatcher is not itself a request to a Worker. + if is_mutation { if let TicketBackendOperationResult::QueueOutcome(outcome) = &result { for ticket_id in &outcome.queued_tickets { notify_ticket_recipients( @@ -18349,6 +18363,7 @@ async fn apply_ticket_case_update( let ticket = backend .update_case(&ticket_id, request) .map_err(Error::from)?; + recover_ticket_notifications(&api); Ok(Json(ticket)) }).await @@ -18721,6 +18736,7 @@ fn canonical_ticket_resource_key(resource_key: &str) -> Option<&str> { .then_some(resource_key) } +#[cfg(test)] fn ticket_notification_content( resource_key: &str, previous_state: &str, @@ -18777,6 +18793,9 @@ fn ticket_notification_delivery_error_category( result: &std::result::Result, ) -> Option<&'static str> { match result { + Ok(result) if result.disposition == WorkerInputDisposition::Unknown => { + Some("acceptance_unknown") + } Ok(result) => match result.state { InternalWorkerOperationState::Accepted => None, InternalWorkerOperationState::Rejected => Some("runtime_rejected"), @@ -18809,61 +18828,15 @@ fn emit_ticket_notification_delivery_warning(warning: TicketNotificationDelivery fn notify_ticket_recipients( api: &WorkspaceApi, - workspace_id: &str, - ticket_id: &str, - previous_state: &str, - current_state: &str, - source: Option, + _workspace_id: &str, + _ticket_id: &str, + _previous_state: &str, + _current_state: &str, + _source: Option, ) { - let Ok(Some(resource_key)) = - api.store - .resource_key(workspace_id, WorkspaceResourceKind::Ticket, ticket_id) - else { - return; - }; - let Some(resource_key) = canonical_ticket_resource_key(&resource_key) else { - return; - }; - - let mut recipients = Vec::new(); - if let Some(assignment) = api - .store - .get_active_ticket_worker_assignment(workspace_id, ticket_id) - .ok() - .flatten() - { - recipients.push(assignment.worker.clone()); - } - if let Some(orchestrator) = find_workspace_orchestrator(api) { - dispatch_orchestrator_assignment_attention(api, &orchestrator.worker); - } - recipients.sort(); - recipients.dedup(); - - let content = ticket_notification_content(resource_key, previous_state, current_state); - for recipient in recipients { - if source.as_ref().is_some_and(|source| source == &recipient) { - continue; - } - let result = api.runtime.send_input( - &recipient, - WorkerInputRequest { - kind: WorkerInputKind::Notify, - content: content.clone(), - submission_request_id: None, - segments: None, - }, - ); - if let Some(error_category) = ticket_notification_delivery_error_category(&result) { - emit_ticket_notification_delivery_warning(TicketNotificationDeliveryWarning::new( - workspace_id, - ticket_id, - current_state, - &recipient, - error_category, - )); - } - } + // Source identity and recipients were captured by the domain transaction. + // Call-site state strings and current Worker observations are not routing authority. + recover_ticket_notifications(api); } fn authenticate_worker_mutation_source( @@ -20362,36 +20335,61 @@ async fn execute_workdir_session_operation( } async fn run_orchestrator_turn_end_hook(api: WorkspaceApi) { - let Ok(mut subscription) = api.runtime_subscription_broker.subscribe( - EMBEDDED_WORKER_RUNTIME_ID, - protocol::subscription::EventSubscriptionSelector::RuntimeWorkers, - ) else { - return; - }; + // Fixed, bounded recovery independent of unrelated Ticket updates. A missed + // tick is skipped rather than accumulated; rounds do not overlap on this owner. + let mut recovery = tokio::time::interval(std::time::Duration::from_secs(30)); + recovery.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Skip); + let mut subscription: Option = None; let mut worker_states = HashMap::new(); - while let Some(update) = subscription.recv().await { - let stage_api = api.clone(); - let result = api.host_executors.host().map(|executor| async move { - executor - .run(move || -> Result<_> { - let keep_running = - observe_orchestrator_turn_end(&stage_api, &mut worker_states, update); - Ok((keep_running, worker_states)) - }) - .await - }); - match result { - Ok(operation) => match operation.await { - Ok((true, states)) => worker_states = states, - Ok((false, _)) => return, - Err(error) => { - tracing::warn!(%error, "Orchestrator hook stopped; durable inputs remain recoverable"); - return; + loop { + let update = tokio::select! { + _ = recovery.tick() => { + if subscription.is_none() { + subscription = api.runtime_subscription_broker.subscribe( + EMBEDDED_WORKER_RUNTIME_ID, + protocol::subscription::EventSubscriptionSelector::RuntimeWorkers, + ).ok(); } - }, + None + } + update = async { + match subscription.as_mut() { + Some(subscription) => subscription.recv().await, + None => std::future::pending().await, + } + } => { + if update.is_none() { subscription = None; } + update + } + }; + let stage_api = api.clone(); + let states = std::mem::take(&mut worker_states); + let result = match api.host_executors.host() { + Ok(executor) => { + executor + .run(move || -> Result<_> { + let mut states = states; + let keep_subscription = if let Some(update) = update { + observe_orchestrator_turn_end(&stage_api, &mut states, update) + } else { + recover_ticket_notifications(&stage_api); + true + }; + Ok((keep_subscription, states)) + }) + .await + } + Err(error) => Err(error), + }; + match result { + Ok((keep_subscription, states)) => { + worker_states = states; + if !keep_subscription { + subscription = None; + } + } Err(error) => { - tracing::warn!(%error, "Orchestrator hook executor unavailable"); - return; + tracing::warn!(%error, "notification recovery round failed; retry on the next bounded tick"); } } } @@ -20479,9 +20477,7 @@ fn enqueue_orchestrator_assignment_attention( let (done, completion) = tokio::sync::oneshot::channel(); match api.host_executors.host().and_then(|executor| { executor.dispatch(move || { - if let Some(orchestrator) = find_workspace_orchestrator(&api) { - dispatch_orchestrator_assignment_attention(&api, &orchestrator.worker); - } + recover_ticket_notifications(&api); let _ = done.send(()); }) }) { @@ -20512,6 +20508,9 @@ fn with_orchestrator_attention_reservation( Option, ), ) { + let Some(_permit) = api.notification_delivery_lifecycle.begin() else { + return; + }; let state = orchestrator_attention_state(api, worker); loop { let pending = { @@ -20566,76 +20565,113 @@ fn check_orchestrator_assignment_attention( state: &Arc>, pending: Option, ) { - let Ok(target) = orchestrator_delivery_target(api, worker) else { - return; - }; - if let Some(delivery) = pending { - // Validate current identity before retrying an immutable request. Clear, - // reissue, close and Worker replacement cannot revive an old delivery. - if api - .store - .reserve_ticket_assignment_delivery( - &api.config.workspace_id, - &delivery.input.assignment_id, - &delivery.input.input_event_id, - &target, - ) - .is_ok() - { - send_orchestrator_attention(api, worker, state, delivery); - if state - .lock() - .unwrap_or_else(|p| p.into_inner()) - .pending - .is_some() - { - return; // Unknown/failed acceptance needs an explicit later trigger. - } - } else { - state.lock().unwrap_or_else(|p| p.into_inner()).pending = None; - } - // Discarding or acknowledging an old retry must not consume this new- - // input trigger. Continue the common scan in the same reservation. - } - let Ok(inputs) = api + let inputs = match api .store .list_pending_ticket_assignment_deliveries(&api.config.workspace_id, 20) - else { - return; + { + Ok(inputs) => inputs, + Err(error) => { + tracing::warn!(%error, "assignment notification scan failed"); + return; + } }; - let Ok(Some(config_state)) = api - .config_store - .load_workspace_config(&api.config.workspace_id) - else { - return; - }; - let Ok(projection) = api - .prompt_projection_cache - .resolve(&api.config.workspace_id, &config_state) - else { - return; - }; - let Ok(catalog) = worker::PromptCatalog::from_projection(projection.catalog().clone()) else { - return; + let target = match orchestrator_delivery_target(api, worker) { + Ok(target) => target, + Err(_) => { + for input in inputs { + record_notification_failure( + api, + &assignment_delivery_key(&input), + notification_delivery::orchestrator_target_failure(api, worker), + ); + } + return; + } }; + let retried_id = pending + .as_ref() + .map(|delivery| delivery.input.delivery_id.clone()); + if let Some(delivery) = pending { + // The saved bytes are useful on retry, but process memory is never backlog authority. + send_orchestrator_attention(api, worker, state, delivery); + } for input in inputs { - // Reserve from the same pending-input contract, rechecking current - // assignment and target binding under the write lock before any I/O. - let Ok(input) = api.store.reserve_ticket_assignment_delivery( + if api.notification_delivery_lifecycle.stopped() { + break; + } + if retried_id.as_deref() == Some(&input.delivery_id) { + continue; + } + let key = assignment_delivery_key(&input); + let input = match api.store.reserve_ticket_assignment_delivery( &api.config.workspace_id, &input.assignment_id, &input.input_event_id, &target, - ) else { - continue; + ) { + Ok(input) => input, + Err(error) => { + record_notification_failure( + api, + &key, + match error { + Error::TicketAssignmentConflict(_) => { + NotificationFailureCode::AssignmentChanged + } + _ => NotificationFailureCode::PreparationFailed, + }, + ); + continue; + } }; - let Ok(ticket) = api.authority.ticket(&input.ticket_id) else { - continue; + let config_state = match api + .config_store + .load_workspace_config(&api.config.workspace_id) + { + Ok(Some(config)) => config, + _ => { + record_notification_failure( + api, + &key, + NotificationFailureCode::ConfigurationUnavailable, + ); + continue; + } }; - let Ok(projected) = - worker::OrchestratorQueueAttentionTicket::new(ticket.resource_key, ticket.title) - else { - continue; + let catalog = api + .prompt_projection_cache + .resolve(&api.config.workspace_id, &config_state) + .and_then(|projection| { + worker::PromptCatalog::from_projection(projection.catalog().clone()) + .map_err(|error| Error::Config(error.to_string())) + }); + let catalog = match catalog { + Ok(catalog) => catalog, + Err(_) => { + record_notification_failure( + api, + &key, + NotificationFailureCode::ConfigurationUnavailable, + ); + continue; + } + }; + let ticket = match api.authority.ticket(&input.ticket_id) { + Ok(ticket) => ticket, + Err(_) => { + record_notification_failure(api, &key, NotificationFailureCode::SourceLookupFailed); + continue; + } + }; + let projected = match worker::OrchestratorQueueAttentionTicket::new( + ticket.resource_key, + ticket.title, + ) { + Ok(ticket) => ticket, + Err(_) => { + record_notification_failure(api, &key, NotificationFailureCode::SourceLookupFailed); + continue; + } }; let context = worker::OrchestratorQueueAttentionContext::new(vec![projected.with_assignment_input( @@ -20644,22 +20680,27 @@ fn check_orchestrator_assignment_attention( input.waiting_reason.as_deref(), input.next_action.as_deref(), )]); - let Ok(content) = catalog.orchestrator_queue_attention( + let content = match catalog.orchestrator_queue_attention( worker::OrchestratorQueueAttentionPrompt::Server, &context, - ) else { - continue; + ) { + Ok(content) => content, + Err(_) => { + record_notification_failure(api, &key, NotificationFailureCode::RenderingFailed); + continue; + } }; - // The complete rendered request, including title/catalog projection, is - // pinned before dispatch; restart retries never render a new payload - // under an already accepted Runtime request id. - let Ok(content) = api.store.prepare_ticket_assignment_delivery_content( + let content = match api.store.prepare_ticket_assignment_delivery_content( &api.config.workspace_id, &input.delivery_id, &content, &target, - ) else { - continue; + ) { + Ok(content) => content, + Err(_) => { + record_notification_failure(api, &key, NotificationFailureCode::PreparationFailed); + continue; + } }; let delivery = OrchestratorAttentionDelivery { fingerprint: input.delivery_id.clone(), @@ -20669,14 +20710,8 @@ fn check_orchestrator_assignment_attention( }; state.lock().unwrap_or_else(|p| p.into_inner()).pending = Some(delivery.clone()); send_orchestrator_attention(api, worker, state, delivery); - if state - .lock() - .unwrap_or_else(|p| p.into_inner()) - .pending - .is_some() - { - break; - } + // Continue other recipients/inputs even after a failure. Unacknowledged + // requests remain in SQLite, including those not kept in this small cache. } } @@ -20687,20 +20722,38 @@ fn send_orchestrator_attention( delivery: OrchestratorAttentionDelivery, ) { tracing::debug!(delivery_id = %delivery.fingerprint, assignment_id = %delivery.input.assignment_id, "dispatch committed assignment input"); + let key = assignment_delivery_key(&delivery.input); for _ in 0..2 { - let Ok(target) = orchestrator_delivery_target(api, worker) else { + if api.notification_delivery_lifecycle.stopped() { return; + } + let target = match orchestrator_delivery_target(api, worker) { + Ok(target) => target, + Err(_) => { + record_notification_failure( + api, + &key, + notification_delivery::orchestrator_target_failure(api, worker), + ); + return; + } }; - if api - .store - .reserve_ticket_assignment_delivery( - &api.config.workspace_id, - &delivery.input.assignment_id, - &delivery.input.input_event_id, - &target, - ) - .is_err() - { + if let Err(error) = api.store.reserve_ticket_assignment_delivery( + &api.config.workspace_id, + &delivery.input.assignment_id, + &delivery.input.input_event_id, + &target, + ) { + record_notification_failure( + api, + &key, + match error { + Error::TicketAssignmentConflict(_) => { + NotificationFailureCode::AssignmentChanged + } + _ => NotificationFailureCode::PreparationFailed, + }, + ); state.lock().unwrap_or_else(|p| p.into_inner()).pending = None; return; } @@ -20713,10 +20766,7 @@ fn send_orchestrator_attention( segments: None, }, ); - if result - .as_ref() - .is_ok_and(|result| result.state == InternalWorkerOperationState::Accepted) - { + if notification_delivery::notification_failure(&result).is_none() { if api .store .ack_ticket_assignment_delivery( @@ -20729,9 +20779,18 @@ fn send_orchestrator_attention( let mut state = state.lock().unwrap_or_else(|p| p.into_inner()); state.accepted = Some(delivery); state.pending = None; + } else { + record_notification_failure( + api, + &key, + NotificationFailureCode::AcknowledgementFailed, + ); } return; } + let failure = notification_delivery::notification_failure(&result) + .expect("non-accepted delivery has a classified failure"); + record_notification_failure(api, &key, failure); if let Some(error_category) = ticket_notification_delivery_error_category(&result) { tracing::warn!( event = "ticket_assignment_delivery_pending", @@ -51427,14 +51486,35 @@ mod tests { .unwrap(); } + let initial_notifications = execution.take_inputs(); assert_eq!( - execution.take_inputs().len(), + initial_notifications + .iter() + .filter(|(recipient, _)| recipient.worker_id.to_string() == orchestrator.worker_id) + .count(), 1, "ordinary history must not create unfenced continuing input" ); + assert_eq!( + initial_notifications + .iter() + .filter(|(recipient, _)| recipient.worker_id.to_string() == source.worker_id) + .count(), + 1, + "the Worker receives assignment attention, not its own later comments" + ); seed_test_api_token(api.store.as_ref(), TEST_WORKSPACE_ID); author_notification_test_answer(&api, &ticket.id, "notification-explicit-answer").await; - let inputs = execution.take_inputs(); + let notifications = execution.take_inputs(); + assert_eq!( + notifications.len(), + 2, + "fenced answer and separate Worker attention" + ); + let inputs = notifications + .into_iter() + .filter(|(recipient, _)| recipient.worker_id.to_string() == orchestrator.worker_id) + .collect::>(); assert_eq!(inputs.len(), 1, "one fenced continuing answer"); let unique = inputs .iter() @@ -53429,25 +53509,24 @@ mod tests { api.authority.ticket(&ticket_ref.id).unwrap().state.as_str(), TicketState::Open.as_str() ); - let warnings = TICKET_NOTIFICATION_DELIVERY_WARNING_CAPTURE - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()) - .iter() - .filter(|warning| warning.ticket_id == ticket_ref.id) - .cloned() - .collect::>(); - assert_eq!(warnings.len(), 1); - let warning = &warnings[0]; - assert_eq!(warning.level, "warning"); - assert_eq!(warning.event, "ticket_notification_delivery_failed"); - assert_eq!(warning.workspace_id, TEST_WORKSPACE_ID); - assert_eq!(warning.current_state, TicketState::Open.as_str()); - assert_eq!(warning.recipient_runtime_id, missing_recipient.runtime_id); - assert_eq!(warning.recipient_worker_id, missing_recipient.worker_id); - assert_eq!(warning.error_category, "unknown_worker"); - let serialized = serde_json::to_string(warning).unwrap(); - assert!(!serialized.contains("queued update")); - assert!(!serialized.contains("Ticket notification:")); + let failures = api.config_store.with_conn(|conn| { + let mut stmt = conn.prepare("SELECT f.code FROM notification_failures f JOIN notification_decisions d ON d.workspace_id=f.workspace_id AND d.delivery_id=f.delivery_id JOIN notification_sources s ON s.sequence=d.source_sequence WHERE s.workspace_id=?1 AND s.ticket_id=?2 AND s.role='worker'")?; + Ok(stmt.query_map(rusqlite::params![TEST_WORKSPACE_ID, ticket_ref.id], |row| row.get::<_, String>(0))?.collect::>>()?) + }).unwrap(); + assert!( + !failures.is_empty(), + "missing recipient must remain diagnosable after restart" + ); + assert!(failures.iter().all(|code| code == "target_unavailable")); + let pending = api + .store + .list_pending_ticket_worker_notifications(TEST_WORKSPACE_ID, 100) + .unwrap(); + assert!( + pending + .iter() + .any(|input| input.ticket_id == ticket_ref.id && input.worker == missing_recipient) + ); seed_test_api_token(api.store.as_ref(), TEST_WORKSPACE_ID); author_notification_test_answer(&api, &ticket_ref.id, "notification-saved-answer").await; @@ -53510,16 +53589,12 @@ mod tests { .collect::>(); assert_eq!( warnings.len(), - 3, - "one missing Worker and two bounded Orchestrator delivery attempts" + 2, + "two bounded Orchestrator transport attempts; missing Worker is recorded without I/O" ); - assert!(warnings.iter().any(|warning| { - warning.recipient_worker_id == missing_recipient.worker_id - && warning.error_category == "unknown_worker" - })); assert!(warnings.iter().any(|warning| { warning.recipient_worker_id == orchestrator.worker_id - && warning.error_category == "runtime_rejected" + && warning.error_category == "acceptance_unknown" })); let serialized = serde_json::to_string(&warnings).unwrap(); assert!(!serialized.contains("all delivery failure update")); diff --git a/crates/workspace-server/src/server_assignment_delivery_tests.rs b/crates/workspace-server/src/server_assignment_delivery_tests.rs index e43bac8fe..bf33cb92b 100644 --- a/crates/workspace-server/src/server_assignment_delivery_tests.rs +++ b/crates/workspace-server/src/server_assignment_delivery_tests.rs @@ -1,5 +1,7 @@ // Included as a child module of server::tests. These tests exercise committed -// Assign + HTTP + embedded Runtime boundaries, not the old Queue predicate. +// Ticket activity + SQLite routing + HTTP + embedded Runtime boundaries. Only +// the model and explicit response-loss boundary are replaced in the real fixture. +// Run with cargo test -p yoi-workspace-server --lib assignment_delivery. use super::*; use session_store::{LogEntry, Store, WorkerSessionStore}; use worker_runtime::execution as delivery_execution; @@ -198,7 +200,7 @@ fn delivery_commit_answer_without_scan(api: &WorkspaceApi, ticket: &str, text: & .unwrap(); } -fn delivery_external_comment(api: &WorkspaceApi, ticket: &str, text: &str) { +fn delivery_comment_without_scan(api: &WorkspaceApi, ticket: &str, text: &str) { browser_ticket_backend(api) .unwrap() .add_event( @@ -206,6 +208,10 @@ fn delivery_external_comment(api: &WorkspaceApi, ticket: &str, text: &str) { ticket::NewTicketEvent::new(ticket::TicketEventKind::Comment, text), ) .unwrap(); +} + +fn delivery_external_comment(api: &WorkspaceApi, ticket: &str, text: &str) { + delivery_comment_without_scan(api, ticket, text); notify_ticket_recipients(api, TEST_WORKSPACE_ID, ticket, "open", "open", None); } @@ -1135,3 +1141,627 @@ async fn assignment_worker_replacement_receives_pending_input_without_recreating assert_eq!(execution.attempts.lock().unwrap().len(), 2); delivery_stop(&api, &replacement); } + +fn delivery_assign_worker( + api: &WorkspaceApi, + ticket: &str, + worker: &RuntimeWorkerRef, + assignment: &str, + previous: Option<&str>, +) -> TicketWorkerAssignmentRecord { + let record = TicketWorkerAssignmentRecord { + workspace_id: TEST_WORKSPACE_ID.into(), + ticket_id: ticket.into(), + assignment_id: assignment.into(), + worker: worker.clone(), + assigned_by: "fixture-account".into(), + assigned_at: TEST_CREATED_AT.into(), + }; + api.store + .set_current_ticket_worker_assignment( + &record, + previous, + &format!("{assignment}-event"), + &format!("{assignment}-operation"), + previous.is_some(), + ) + .unwrap(); + record +} + +fn delivery_worker_pending(api: &WorkspaceApi) -> Vec { + api.store + .list_pending_ticket_worker_notifications(TEST_WORKSPACE_ID, 20) + .unwrap() +} + +async fn delivery_settle_real_worker_assignment( + api: &WorkspaceApi, + worker: &RuntimeWorkerRef, +) -> usize { + let initial = delivery_worker_pending(api).len(); + recover_ticket_notifications(api); + if initial > 0 { + delivery_wait_idle(api, worker, initial).await; + } + assert!(delivery_worker_pending(api).is_empty()); + initial +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn worker_notification_missing_callback_is_recovered_to_real_worker_once() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, _) = delivery_real_fixture(dir.path(), false).await; + let worker = spawn_ticket_check_source(&api, "notification-recipient"); + let ticket = delivery_ticket(&api, "Recover committed Worker activity"); + let assignment = delivery_assign_worker(&api, &ticket.id, &worker, "worker-owner", None); + let initial = delivery_settle_real_worker_assignment(&api, &worker).await; + let before = execution.attempts.lock().unwrap().len(); + + // Commit the actual domain event but deliberately omit the mutation callback. + delivery_comment_without_scan(&api, &ticket.id, "A human supplied useful context"); + assert_eq!(execution.attempts.lock().unwrap().len(), before); + let pending = delivery_worker_pending(&api); + assert_eq!(pending.len(), 1); + assert_eq!(pending[0].worker, worker); + recover_ticket_notifications(&api); + delivery_wait_idle(&api, &worker, initial + 1).await; + let attempts = execution.attempts.lock().unwrap().clone(); + assert_eq!(attempts.len(), before + 1); + assert_eq!(attempts[before].0, pending[0].request_id); + assert!( + attempts[before] + .1 + .contains(ticket.resource_key.as_deref().unwrap()) + ); + assert!(attempts[before].1.contains(&pending[0].input_event_id)); + assert_eq!(delivery_receipts(&api, &worker).len(), initial + 1); + assert!(delivery_worker_pending(&api).is_empty()); + for _ in 0..3 { + recover_ticket_notifications(&api); + } + assert_eq!(*execution.attempts.lock().unwrap(), attempts); + assert_eq!( + api.store + .get_current_ticket_worker_assignment(TEST_WORKSPACE_ID, &ticket.id) + .unwrap(), + Some(assignment), + "attention acceptance must not replace assignment authority" + ); + assert_eq!( + api.authority.ticket(&ticket.id).unwrap().state.as_str(), + "open" + ); + delivery_stop(&api, &worker); +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn worker_notification_response_loss_retries_same_request_without_duplicate_acceptance() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, _) = delivery_real_fixture(dir.path(), false).await; + let worker = spawn_ticket_check_source(&api, "lost-response-recipient"); + let ticket = delivery_ticket(&api, "Worker response loss"); + delivery_assign_worker(&api, &ticket.id, &worker, "worker-owner", None); + let initial = delivery_settle_real_worker_assignment(&api, &worker).await; + let before = execution.attempts.lock().unwrap().len(); + execution + .lose_responses + .store(true, std::sync::atomic::Ordering::SeqCst); + delivery_comment_without_scan(&api, &ticket.id, "Preserve this notification identity"); + let pending = delivery_worker_pending(&api); + assert_eq!(pending.len(), 1); + recover_ticket_notifications(&api); + delivery_wait_idle(&api, &worker, initial + 1).await; + let accepted = execution.attempts.lock().unwrap()[before].clone(); + assert_eq!(accepted.0, pending[0].request_id); + let failure: String = api.config_store.with_conn(|conn| { + Ok(conn.query_row("SELECT code FROM notification_failures WHERE workspace_id=?1 AND delivery_id=?2 ORDER BY sequence DESC LIMIT 1", rusqlite::params![TEST_WORKSPACE_ID, pending[0].delivery_id], |row| row.get(0))?) + }).unwrap(); + assert_eq!( + failure, "acceptance_unknown", + "response loss is not a confirmed rejection" + ); + assert_eq!(delivery_receipts(&api, &worker).len(), initial + 1); + assert_eq!( + delivery_worker_pending(&api), + pending, + "unknown ACK was treated as success" + ); + + // Reopen the durable Store while keeping the same real Runtime/session. A + // later domain edit must not change the frozen text of the uncertain request. + browser_ticket_backend(&api) + .unwrap() + .edit_item( + ticket.id.clone().into(), + ticket::TicketItemEdit { + title: Some("Title changed after Worker acceptance".into()), + ..Default::default() + }, + ) + .unwrap(); + let mut recovered = api.clone(); + recovered.store = + Arc::new(SqliteWorkspaceStore::open_for_test(&api.config.database_path).unwrap()); + execution + .lose_responses + .store(false, std::sync::atomic::Ordering::SeqCst); + recover_ticket_notifications(&recovered); + let attempts = execution.attempts.lock().unwrap().clone(); + let retries = attempts + .iter() + .filter(|attempt| attempt.0 == accepted.0) + .collect::>(); + assert!( + retries.len() >= 2, + "uncertain Runtime acceptance was not retried" + ); + assert!( + retries.iter().all(|attempt| **attempt == accepted), + "retry changed frozen ID/body" + ); + let distinct = attempts + .iter() + .map(|attempt| &attempt.0) + .collect::>() + .len(); + delivery_wait_idle(&recovered, &worker, distinct).await; + assert_eq!(delivery_receipts(&recovered, &worker).len(), distinct); + assert_eq!( + delivery_commits(&recovered, &worker) + .iter() + .filter(|body| **body == accepted.1) + .count(), + 1 + ); + assert!(delivery_worker_pending(&recovered).is_empty()); + recover_ticket_notifications(&recovered); + assert_eq!(*execution.attempts.lock().unwrap(), attempts); + delivery_stop(&recovered, &worker); +} + +#[tokio::test] +async fn worker_notification_stopped_recipient_does_not_block_other_recipient() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, _) = delivery_recording_fixture(dir.path()).await; + let worker = spawn_ticket_check_source(&api, "stopped-notification-recipient"); + let ticket = delivery_ticket(&api, "One input with independent recipients"); + delivery_assign(&api, &ticket.id, "assign-orchestrator").await; + delivery_assign_worker(&api, &ticket.id, &worker, "worker-owner", None); + recover_ticket_notifications(&api); + assert!(delivery_worker_pending(&api).is_empty()); + execution.take_inputs(); + execution.take_input_request_ids(); + delivery_stop(&api, &worker); + delivery_commit_answer_without_scan( + &api, + &ticket.id, + "Continue with this answer", + "answer-both", + ); + let pending = delivery_worker_pending(&api); + assert_eq!(pending.len(), 1); + recover_ticket_notifications(&api); + let inputs = execution.take_inputs(); + assert_eq!( + inputs.len(), + 1, + "stopped Worker must not block Orchestrator delivery" + ); + assert_eq!(inputs[0].0.worker_id.to_string(), orchestrator.worker_id); + assert!( + inputs[0] + .1 + .contains(ticket.resource_key.as_deref().unwrap()) + ); + assert!(attention_pending(&api).is_empty()); + assert_eq!( + delivery_worker_pending(&api), + pending, + "other recipient ACK consumed the failed delivery" + ); + recover_ticket_notifications(&api); + assert!( + execution.take_inputs().is_empty(), + "successful recipient was sent again" + ); + delivery_stop(&api, &orchestrator); +} + +#[tokio::test] +async fn worker_notification_reassignment_never_forwards_old_pending_input() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, _) = delivery_recording_fixture(dir.path()).await; + let old_worker = spawn_ticket_check_source(&api, "old-notification-recipient"); + let replacement = spawn_ticket_check_source(&api, "new-notification-recipient"); + let ticket = delivery_ticket(&api, "Do not forward old authority"); + delivery_assign_worker(&api, &ticket.id, &old_worker, "old-owner", None); + recover_ticket_notifications(&api); + execution.take_inputs(); + execution.take_input_request_ids(); + delivery_comment_without_scan(&api, &ticket.id, "Activity belonging to the old owner"); + let pending = delivery_worker_pending(&api); + assert_eq!(pending.len(), 1); + execution.reject_inputs("response outcome unknown"); + recover_ticket_notifications(&api); + let failed = execution.take_inputs(); + assert!(!failed.is_empty()); + assert!( + failed + .iter() + .all(|input| input.0.worker_id.to_string() == old_worker.worker_id) + ); + assert!( + execution + .take_input_request_ids() + .iter() + .all(|id| id.as_deref() == Some(pending[0].request_id.as_str())) + ); + + let current = delivery_assign_worker( + &api, + &ticket.id, + &replacement, + "new-owner", + Some("old-owner"), + ); + *execution.input_failure.lock().unwrap() = None; + delivery_comment_without_scan(&api, &ticket.id, "Activity belonging to the new owner"); + recover_ticket_notifications(&api); + let inputs = execution.take_inputs(); + assert!(!inputs.is_empty(), "new owner's current activity was lost"); + assert!( + inputs + .iter() + .all(|input| input.0.worker_id.to_string() == replacement.worker_id) + ); + assert!( + inputs + .iter() + .all(|input| !input.1.contains(&pending[0].input_event_id)), + "old activity was forwarded to the replacement" + ); + assert!( + execution + .take_input_request_ids() + .iter() + .all(|id| id.as_deref() != Some(pending[0].request_id.as_str())) + ); + assert!(delivery_worker_pending(&api).is_empty()); + assert_eq!( + api.store + .get_current_ticket_worker_assignment(TEST_WORKSPACE_ID, &ticket.id) + .unwrap(), + Some(current) + ); + recover_ticket_notifications(&api); + assert!(execution.take_inputs().is_empty()); + delivery_stop(&api, &old_worker); + delivery_stop(&api, &replacement); + delivery_stop(&api, &orchestrator); +} + +async fn delivery_drain_host_with_paused_time(api: &WorkspaceApi) { + let executor = api.host_executors.host().unwrap(); + let drain = executor.drain(); + tokio::pin!(drain); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + loop { + if let std::task::Poll::Ready(result) = futures::poll!(drain.as_mut()) { + result.unwrap(); + return; + } + assert!( + std::time::Instant::now() < deadline, + "notification host round did not finish" + ); + // Keep a runnable future while the real host executor finishes: Tokio + // must not auto-advance the paused retry clock to infer completion. + tokio::task::yield_now().await; + } +} + +#[tokio::test(start_paused = true)] +async fn worker_notification_periodic_recovery_runs_without_runtime_subscription() { + let dir = tempfile::tempdir().unwrap(); + let (mut api, execution, orchestrator, _) = delivery_recording_fixture(dir.path()).await; + let worker = spawn_ticket_check_source(&api, "periodic-notification-recipient"); + let ticket = delivery_ticket(&api, "Recover without subscription or callback"); + delivery_assign_worker(&api, &ticket.id, &worker, "worker-owner", None); + recover_ticket_notifications(&api); + execution.take_inputs(); + execution.take_input_request_ids(); + api.runtime_subscription_broker = RuntimeSubscriptionBroker::new(TEST_WORKSPACE_ID); + assert!( + api.runtime_subscription_broker + .subscribe( + EMBEDDED_WORKER_RUNTIME_ID, + protocol::subscription::EventSubscriptionSelector::RuntimeWorkers, + ) + .is_err() + ); + + // Poll the production hook itself, not a timer or recovery stand-in. Keeping + // its future local also guarantees cancellation on assertion failure. + delivery_comment_without_scan(&api, &ticket.id, "Missed startup callback"); + let hook = run_orchestrator_turn_end_hook(api.clone()); + tokio::pin!(hook); + assert!(futures::poll!(hook.as_mut()).is_pending()); + delivery_drain_host_with_paused_time(&api).await; + assert_eq!( + execution.take_inputs().len(), + 1, + "startup round did not recover" + ); + assert!(delivery_worker_pending(&api).is_empty()); + assert!(futures::poll!(hook.as_mut()).is_pending()); + + delivery_comment_without_scan(&api, &ticket.id, "Missed callback after startup"); + tokio::time::advance(std::time::Duration::from_secs(29)).await; + assert!(futures::poll!(hook.as_mut()).is_pending()); + delivery_drain_host_with_paused_time(&api).await; + assert!( + execution.take_inputs().is_empty(), + "recovery retried before the bounded interval" + ); + tokio::time::advance(std::time::Duration::from_secs(1)).await; + assert!(futures::poll!(hook.as_mut()).is_pending()); + delivery_drain_host_with_paused_time(&api).await; + let inputs = execution.take_inputs(); + assert_eq!( + inputs.len(), + 1, + "periodic recovery required a subscription or mutation callback" + ); + assert_eq!(inputs[0].0.worker_id.to_string(), worker.worker_id); + assert!(delivery_worker_pending(&api).is_empty()); + assert!(api.runtime_subscription_broker.runtime_ids().is_empty()); + delivery_stop(&api, &worker); + delivery_stop(&api, &orchestrator); +} + +async fn delivery_signed_item_edit( + api: &WorkspaceApi, + identity: &RuntimeIdentityMaterial, + worker: &RuntimeWorkerRef, + ticket: &str, + edit: Value, +) -> ticket::Ticket { + let path = format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{ticket}/item"); + let response = build_router(api.clone()) + .oneshot(runtime_source_request( + identity, + Some(&worker.worker_id), + "PATCH", + &path, + serde_json::to_vec(&edit).unwrap(), + )) + .await + .unwrap(); + let status = response.status(); + let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!( + status, + StatusCode::OK, + "{}", + String::from_utf8_lossy(&bytes) + ); + let edited: ticket::Ticket = serde_json::from_slice(&bytes).unwrap(); + assert_eq!( + edited, + browser_ticket_backend(api) + .unwrap() + .show(ticket.to_owned().into()) + .unwrap() + ); + edited +} + +#[tokio::test] +async fn notification_item_edit_worker_title_only_self_edit_does_not_notify_itself() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, identity) = delivery_recording_fixture(dir.path()).await; + let worker = spawn_ticket_check_source(&api, "self-title-editor"); + seed_worker_source_member(&api, &worker.runtime_id, &worker.worker_id); + let ticket = delivery_ticket(&api, "Worker will clarify its own title"); + delivery_assign_worker(&api, &ticket.id, &worker, "self-title-owner", None); + recover_ticket_notifications(&api); + execution.take_inputs(); + execution.take_input_request_ids(); + let before = browser_ticket_backend(&api) + .unwrap() + .show(ticket.id.clone().into()) + .unwrap(); + + let edited = delivery_signed_item_edit( + &api, + &identity, + &worker, + &ticket.id, + json!({"title":"Clarified by the assigned Worker"}), + ) + .await; + assert_eq!(edited.meta.title, "Clarified by the assigned Worker"); + assert_eq!(edited.document.body, before.document.body); + assert_eq!(edited.meta.targets, before.meta.targets); + for _ in 0..3 { + recover_ticket_notifications(&api); + } + let inputs = execution.take_inputs(); + delivery_stop(&api, &worker); + delivery_stop(&api, &orchestrator); + assert!( + inputs + .iter() + .all(|(recipient, _)| recipient.worker_id.to_string() != worker.worker_id), + "authenticated title-only self-edit became fresh Worker input" + ); + assert!(delivery_worker_pending(&api).is_empty()); +} + +#[tokio::test] +async fn notification_item_edit_orchestrator_initial_body_self_edit_does_not_reenter_delegation() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, identity) = delivery_recording_fixture(dir.path()).await; + let ticket = delivery_ticket(&api, "Orchestrator clarifies the initial post"); + let assigned = delivery_assign(&api, &ticket.id, "assign-self-body").await; + let assignment = assigned["assignment"]["assignment_id"].as_str().unwrap(); + let initial = delivery_tracking(&api, &ticket.id, assignment); + execution.take_inputs(); + execution.take_input_request_ids(); + let before = browser_ticket_backend(&api) + .unwrap() + .show(ticket.id.clone().into()) + .unwrap(); + + let edited = delivery_signed_item_edit( + &api, + &identity, + &orchestrator, + &ticket.id, + json!({"body":"Clarified requirements written by the current Orchestrator"}), + ) + .await; + assert_eq!( + edited.document.body.as_str(), + "Clarified requirements written by the current Orchestrator" + ); + assert_eq!(edited.meta.title, before.meta.title); + assert_eq!( + edited.events[0].edited_by.as_deref(), + Some( + format!( + "worker:{}/{}", + orchestrator.runtime_id, orchestrator.worker_id + ) + .as_str() + ) + ); + for _ in 0..3 { + recover_ticket_notifications(&api); + } + let inputs = execution.take_inputs(); + assert_eq!(delivery_tracking(&api, &ticket.id, assignment), initial); + delivery_stop(&api, &orchestrator); + assert!( + inputs + .iter() + .all(|(recipient, _)| recipient.worker_id.to_string() != orchestrator.worker_id), + "workspace_agent principal did not recognize its authenticated Orchestrator self-edit" + ); + assert!(attention_pending(&api).is_empty()); +} + +#[tokio::test] +async fn notification_item_edit_external_targets_only_change_notifies_existing_recipients_once() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, _) = delivery_recording_fixture(dir.path()).await; + let worker = spawn_ticket_check_source(&api, "target-change-recipient"); + let ticket = delivery_ticket(&api, "External target change must not disappear"); + delivery_assign(&api, &ticket.id, "assign-targets").await; + delivery_assign_worker(&api, &ticket.id, &worker, "targets-owner", None); + recover_ticket_notifications(&api); + execution.take_inputs(); + execution.take_input_request_ids(); + let before = browser_ticket_backend(&api) + .unwrap() + .show(ticket.id.clone().into()) + .unwrap(); + let targets = vec![test_ticket_target( + "test-repository", + "develop", + TicketTargetAccess::ReadWrite, + )]; + let result = delivery_http( + &api, + "PATCH", + &format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{}/item", ticket.id), + Some(json!({"targets":{"action":"set","targets":targets}})), + StatusCode::OK, + ) + .await; + let edited: ticket::Ticket = serde_json::from_value(result).unwrap(); + assert_eq!( + edited, + browser_ticket_backend(&api) + .unwrap() + .show(ticket.id.clone().into()) + .unwrap() + ); + assert_eq!(edited.meta.targets, targets); + assert_eq!(edited.meta.title, before.meta.title); + assert_eq!(edited.document.body, before.document.body); + recover_ticket_notifications(&api); + let inputs = execution.take_inputs(); + let ids = execution.take_input_request_ids(); + recover_ticket_notifications(&api); + let repeats = execution.take_inputs(); + delivery_stop(&api, &worker); + delivery_stop(&api, &orchestrator); + for recipient in [&orchestrator, &worker] { + assert_eq!( + inputs + .iter() + .filter(|(target, _)| target.worker_id.to_string() == recipient.worker_id) + .count(), + 1, + "targets-only external edit did not reach {} exactly once: {inputs:?}", + recipient.worker_id + ); + } + assert_eq!(ids.len(), 2); + assert!(ids.iter().all(Option::is_some)); + assert_ne!(ids[0], ids[1]); + assert!(repeats.is_empty(), "targets edit was delivered repeatedly"); + assert!(delivery_worker_pending(&api).is_empty()); + assert!(attention_pending(&api).is_empty()); +} + +#[tokio::test] +async fn catalog_shutdown_stops_notification_hook_and_fences_later_dispatch() { + let dir = tempfile::tempdir().unwrap(); + let (api, execution, orchestrator, _) = delivery_recording_fixture(dir.path()).await; + let ticket = delivery_ticket(&api, "Pending at server shutdown"); + delivery_assign(&api, &ticket.id, "shutdown-assignment").await; + execution.take_inputs(); + delivery_commit_answer_without_scan( + &api, + &ticket.id, + "Retain for the next server", + "shutdown-answer", + ); + let catalog = WorkspaceServerApi::new(api.config.clone(), api.store.clone()); + catalog + .apis + .lock() + .await + .insert(TEST_WORKSPACE_ID.into(), api.clone()); + let hook = tokio::spawn(run_orchestrator_turn_end_hook(api.clone())); + catalog + .hook_handles + .lock() + .await + .insert(TEST_WORKSPACE_ID.into(), hook.abort_handle()); + catalog.shutdown_feature_storage().await.unwrap(); + assert!( + tokio::time::timeout(std::time::Duration::from_secs(2), hook) + .await + .unwrap() + .unwrap_err() + .is_cancelled() + ); + assert!(catalog.hook_handles.lock().await.is_empty()); + execution.take_inputs(); + recover_ticket_notifications(&api); + dispatch_orchestrator_assignment_attention(&api, &orchestrator); + assert!( + execution.take_inputs().is_empty(), + "stopped server dispatched a saved notification" + ); + assert_eq!( + attention_pending(&api).len(), + 1, + "shutdown discarded durable work" + ); + delivery_stop(&api, &orchestrator); +} diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 71176e3df..442a3b09e 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -62,6 +62,12 @@ use crate::{Error, Result}; #[path = "assignment_delivery.rs"] mod assignment_delivery; +#[path = "notification_routing.rs"] +mod notification_routing; +pub use notification_routing::{ + NotificationDeliveryKey, NotificationEventKind, NotificationFailureCode, NotificationPurpose, + NotificationReason, TicketWorkerNotification, TicketWorkerNotificationTarget, +}; #[path = "assignment_tracking.rs"] mod assignment_tracking; pub use assignment_delivery::TicketAssignmentDelivery; @@ -73,7 +79,7 @@ pub use ticket::{ }; const OLDEST_SCHEMA_VERSION: i64 = 50; -const LATEST_SCHEMA_VERSION: i64 = 94; +const LATEST_SCHEMA_VERSION: i64 = 95; const WORKSPACE_DRIVE_GRANTS_MIGRATION_NAME: &str = "durable Workspace Drive grants"; const WORKSPACE_CONFIG_GRANTS_MIGRATION_NAME: &str = "Workspace config grants and logical Workdirs"; const SCHEMA_BASELINE_NAME: &str = "workspace schema baseline"; @@ -351,6 +357,11 @@ const MIGRATIONS: &[Migration] = &[ name: "Canonical Ticket posts", apply: migrate_ticket_posts_v93_to_v94, }, + Migration { + version: 95, + name: "Common notification routing and durable Worker delivery", + apply: migrate_notification_routing_v94_to_v95, + }, ]; #[derive(Clone, Copy)] @@ -1886,6 +1897,30 @@ pub trait ControlPlaneStore: Send + Sync + WorkspaceDeletionStore { delivery_id: &str, source: &TicketAssignmentTrackingSourceContext, ) -> Result<()>; + fn list_pending_ticket_worker_notifications( + &self, + workspace_id: &str, + limit: usize, + ) -> Result>; + fn prepare_ticket_worker_notification_content( + &self, + workspace_id: &str, + delivery_id: &str, + content: &str, + target: &TicketWorkerNotificationTarget, + ) -> Result; + fn ack_ticket_worker_notification( + &self, + workspace_id: &str, + delivery_id: &str, + target: &TicketWorkerNotificationTarget, + ) -> Result<()>; + fn record_notification_delivery_failure( + &self, + workspace_id: &str, + key: &NotificationDeliveryKey, + code: NotificationFailureCode, + ) -> Result<()>; fn reserve_ticket_assignment_operation_for_orchestrator( &self, workspace_id: &str, @@ -8006,6 +8041,38 @@ impl ControlPlaneStore for SqliteWorkspaceStore { None, ) } + fn list_pending_ticket_worker_notifications( + &self, + workspace_id: &str, + limit: usize, + ) -> Result> { + notification_routing::list_workers(self, workspace_id, limit) + } + fn prepare_ticket_worker_notification_content( + &self, + workspace_id: &str, + delivery_id: &str, + content: &str, + target: &TicketWorkerNotificationTarget, + ) -> Result { + notification_routing::prepare_worker(self, workspace_id, delivery_id, content, target) + } + fn ack_ticket_worker_notification( + &self, + workspace_id: &str, + delivery_id: &str, + target: &TicketWorkerNotificationTarget, + ) -> Result<()> { + notification_routing::ack_worker(self, workspace_id, delivery_id, target) + } + fn record_notification_delivery_failure( + &self, + workspace_id: &str, + key: &NotificationDeliveryKey, + code: NotificationFailureCode, + ) -> Result<()> { + notification_routing::failure(self, workspace_id, key, code) + } fn reserve_ticket_assignment_operation_for_orchestrator( &self, workspace_id: &str, @@ -8051,7 +8118,12 @@ impl ControlPlaneStore for SqliteWorkspaceStore { "Assignment delivery scan limit must be 1..=1000".into(), )); } - self.with_conn(|conn| assignment_delivery::list_in_tx(conn, workspace_id, limit)) + self.with_conn_mut(|conn| { + let tx = conn.transaction_with_behavior(TransactionBehavior::Immediate)?; + let result = assignment_delivery::list_in_tx(&tx, workspace_id, limit)?; + tx.commit()?; + Ok(result) + }) } fn reserve_ticket_assignment_delivery( &self, @@ -14023,10 +14095,39 @@ fn migrate_ticket_posts_v93_to_v94(conn: &Connection) -> Result<()> { Ok(()) } +fn migrate_notification_routing_v94_to_v95(conn: &Connection) -> Result<()> { + if current_schema_version(conn)? != 94 { + return Err(Error::Store( + "Notification routing migration requires schema 94".into(), + )); + } + let tx = conn.unchecked_transaction()?; + ticket::migrate_sqlite_ticket_schema_in_transaction(&tx) + .map_err(|e| Error::Store(format!("Ticket content facts migration failed: {e}")))?; + notification_routing::install(&tx)?; + tx.execute( + "INSERT INTO __yoi_schema_migrations(version,name) VALUES(95,?1)", + ["Common notification routing and durable Worker delivery"], + )?; + verify_workspace_resource_constraints(&tx)?; + let violations: i64 = + tx.query_row("SELECT COUNT(*) FROM pragma_foreign_key_check", [], |r| { + r.get(0) + })?; + if violations != 0 { + return Err(Error::Store( + "Notification routing migration left foreign-key violations".into(), + )); + } + tx.commit()?; + Ok(()) +} + fn create_latest_workspace_schema(conn: &Connection) -> Result<()> { create_workspace_schema_structure(conn)?; assignment_delivery::install(conn)?; assignment_delivery::install_post_inputs(conn)?; + notification_routing::install(conn)?; ticket::verify_sqlite_ticket_schema(conn).map_err(|error| { Error::Store(format!( "fresh Workspace Ticket schema verification failed: {error}" @@ -14282,6 +14383,9 @@ fn verify_workspace_resource_constraints(conn: &Connection) -> Result<()> { } } } + if current_schema_version(conn)? >= 95 { + notification_routing::verify(conn)?; + } Ok(()) } @@ -15036,6 +15140,7 @@ fn table_columns(conn: &Connection, table_name: &str) -> Result> { mod tests { include!("assignment_tracking_tests.rs"); include!("assignment_delivery_tests.rs"); + include!("notification_routing_tests.rs"); include!("store_workspace_config_tests.rs"); fn drive_grant_fixture(store: &SqliteWorkspaceStore) -> server_api::DriveGrantResponse { @@ -17248,6 +17353,7 @@ CREATE UNIQUE INDEX ticket_current_worker_role_idx WorkspaceSchemaMigrationStep {version:92, name:"Ticket-owned assignment tracking".into()}, WorkspaceSchemaMigrationStep {version:93, name:"Durable assignment deliveries and source-bound startup fences".into()}, WorkspaceSchemaMigrationStep {version:94, name:"Canonical Ticket posts".into()}, + WorkspaceSchemaMigrationStep {version:95, name:"Common notification routing and durable Worker delivery".into()}, ] ); @@ -17352,6 +17458,7 @@ CREATE UNIQUE INDEX ticket_current_worker_role_idx (92, "Ticket-owned assignment tracking".to_string()), (93, "Durable assignment deliveries and source-bound startup fences".to_string()), (94, "Canonical Ticket posts".to_string()), +(95, "Common notification routing and durable Worker delivery".to_string()), ] ); assert!(!table_exists(conn, "trusted_runtime_records")?); @@ -19954,11 +20061,7 @@ CREATE UNIQUE INDEX ticket_current_worker_role_idx .iter() .map(|step| step.version) .collect::>(), - if version == 89 { - vec![90, 91, 92, 93, 94] - } else { - vec![91, 92, 93, 94] - } + ((version + 1)..=LATEST_SCHEMA_VERSION).collect::>() ); drop(conn); let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); @@ -19974,7 +20077,8 @@ CREATE UNIQUE INDEX ticket_current_worker_role_idx (91, "Ticket open/closed cases and retained routing evidence".into()), (92, "Ticket-owned assignment tracking".into()), (93, "Durable assignment deliveries and source-bound startup fences".into()), - (94, "Canonical Ticket posts".into())]); + (94, "Canonical Ticket posts".into()), + (95, "Common notification routing and durable Worker delivery".into())]); for (table, rows) in unchanged.iter().zip(before.iter()) { assert_eq!(&stored_fixture_rows(conn, table), rows, "preserve {table}"); } diff --git a/crates/workspace-server/src/ticket_post_upgrade_tests.rs b/crates/workspace-server/src/ticket_post_upgrade_tests.rs index 913b5c524..4a433cc6b 100644 --- a/crates/workspace-server/src/ticket_post_upgrade_tests.rs +++ b/crates/workspace-server/src/ticket_post_upgrade_tests.rs @@ -61,7 +61,7 @@ fn ticket_posts_schema93_preserves_initial_body_creator_comments_and_archived_au .iter() .map(|step| step.version) .collect::>(), - [94] + (94..=LATEST_SCHEMA_VERSION).collect::>() ); let conn = Connection::open(&path).unwrap(); assert!( @@ -76,14 +76,15 @@ fn ticket_posts_schema93_preserves_initial_body_creator_comments_and_archived_au assert_eq!(SqliteWorkspaceStore::migrate_database(&path).unwrap(), plan); let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); store.with_conn(|conn| { - assert_eq!(current_schema_version(conn)?, 94); + assert_eq!(current_schema_version(conn)?, LATEST_SCHEMA_VERSION); assert!(!column_exists(conn,"typed_tickets","body")?); assert_eq!(stored_fixture_rows(conn,"typed_ticket_events"),old_events); assert_eq!(stored_fixture_rows(conn,"typed_ticket_event_attributes"),old_attributes); assert_eq!(stored_fixture_rows(conn,"typed_ticket_event_references"),old_references); let markers = stored_fixture_rows(conn,"ticket_schema_migrations"); assert_eq!(&markers[..old_markers.len()],old_markers.as_slice()); - assert_eq!(markers.len(),old_markers.len()+1); + assert_eq!(markers.len(),old_markers.len()+2); + assert_eq!(conn.query_row("SELECT COUNT(*) FROM typed_ticket_content_changes",[],|r|r.get::<_,i64>(0))?,0,"migration does not invent content change facts"); assert_eq!(conn.query_row("SELECT COUNT(*) FROM ticket_assignment_content_inputs",[],|r|r.get::<_,i64>(0))?,0,"migration does not turn historical edits into new inputs"); let initial: (String,Option,Option,String,String) = conn.query_row("SELECT e.body,t.created_by,e.author,e.kind,e.event_ref FROM typed_tickets t JOIN typed_ticket_events e USING(workspace_id,ticket_id) WHERE t.ticket_id='authored' AND e.event_index=-1",[],|row|Ok((row.get(0)?,row.get(1)?,row.get(2)?,row.get(3)?,row.get(4)?)))?; assert_eq!(initial,("Current requirements after edits".into(),Some("creator".into()),Some("creator".into()),"comment".into(),"authored:-1".into())); @@ -341,7 +342,7 @@ fn ticket_posts_oldest_supported_host50_preserves_content_through_full_chain() { let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); store .with_conn(|conn| { - assert_eq!(current_schema_version(conn)?, 94); + assert_eq!(current_schema_version(conn)?, LATEST_SCHEMA_VERSION); ticket::verify_sqlite_ticket_schema(conn).unwrap(); assert_eq!( conn.query_row("SELECT COUNT(*) FROM pragma_foreign_key_check", [], |r| r diff --git a/crates/workspace-server/src/ticket_tag_upgrade_tests.rs b/crates/workspace-server/src/ticket_tag_upgrade_tests.rs index 4fb3836a0..d1c2ea06f 100644 --- a/crates/workspace-server/src/ticket_tag_upgrade_tests.rs +++ b/crates/workspace-server/src/ticket_tag_upgrade_tests.rs @@ -266,7 +266,7 @@ fn assert_retained_classification_upgrade(workspace_version: i64, component: i64 let store = SqliteWorkspaceStore::open_for_test(&path).unwrap(); store.with_conn(|conn| { assert_eq!(current_schema_version(conn)?,LATEST_SCHEMA_VERSION); - assert_eq!(conn.query_row("SELECT MAX(version) FROM ticket_schema_migrations",[],|row|row.get::<_,i64>(0))?,12); + assert_eq!(conn.query_row("SELECT MAX(version) FROM ticket_schema_migrations",[],|row|row.get::<_,i64>(0))?,13); assert_eq!(fixture_auth_rows(conn),auth_before); for (table, rows) in preserved_tables.iter().zip(&preserved) { let mut migrated = stored_fixture_rows(conn, table); @@ -500,7 +500,7 @@ fn workspace90_cases9_add_classification_without_reinterpreting_reasoned_history [], |row| row.get::<_, i64>(0) )?, - 12 + 13 ); assert_eq!( stored_fixture_rows(conn, "typed_ticket_case_history"), diff --git a/crates/workspace-server/src/workspace_deletion.rs b/crates/workspace-server/src/workspace_deletion.rs index 13a7fe9d1..ad2462434 100644 --- a/crates/workspace-server/src/workspace_deletion.rs +++ b/crates/workspace-server/src/workspace_deletion.rs @@ -45,6 +45,12 @@ const WORKSPACE_DELETION_PURGE_TABLES: &[&str] = &[ "repository_ssh_host_trust_keys", "repository_ssh_host_trusts", "server_secret_objects", + "notification_assignment_targets", + "notification_acknowledgements", + "notification_requests", + "notification_failures", + "notification_decisions", + "notification_sources", "ticket_assignment_delivery_acknowledgements", "ticket_assignment_delivery_requests", "ticket_assignment_deliveries", @@ -66,6 +72,7 @@ const WORKSPACE_DELETION_PURGE_TABLES: &[&str] = &[ "typed_ticket_artifacts", "typed_ticket_event_attributes", "typed_ticket_event_references", + "typed_ticket_content_changes", "typed_ticket_events", "typed_ticket_labels", "typed_ticket_orchestration_plans", @@ -482,6 +489,12 @@ impl WorkspaceDeletionStore for SqliteWorkspaceStore { // commit; update guards remain installed throughout. let mut ticket_delete_guards = Vec::new(); for (table, trigger) in [ + ("notification_assignment_targets", "notification_assignment_targets_no_delete"), + ("notification_sources", "notification_sources_no_delete"), + ("notification_decisions", "notification_decisions_no_delete"), + ("notification_requests", "notification_requests_no_delete"), + ("notification_acknowledgements", "notification_acknowledgements_no_delete"), + ("notification_failures", "notification_failures_no_delete"), ("ticket_worker_assignments", "typed_ticket_assignment_identity_no_delete"), ("ticket_worker_assignment_events", "typed_ticket_assignment_events_no_delete"), ("ticket_assignment_operations", "typed_ticket_assignment_receipt_no_delete"),