From 1378581940550d714fbe0eb9facdeac400601d52 Mon Sep 17 00:00:00 2001 From: Hare Date: Sun, 26 Jul 2026 20:46:06 +0900 Subject: [PATCH] ticket: add exact body replacement edits --- crates/ticket/src/lib.rs | 314 ++++++++++++++++++++++++++++++++++++-- crates/ticket/src/tool.rs | 109 ++++++++++++- 2 files changed, 408 insertions(+), 15 deletions(-) diff --git a/crates/ticket/src/lib.rs b/crates/ticket/src/lib.rs index c8cfb34f..05f38972 100644 --- a/crates/ticket/src/lib.rs +++ b/crates/ticket/src/lib.rs @@ -520,10 +520,98 @@ impl NewTicket { #[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct TicketItemEdit { pub title: Option, + /// Whole-body replacement for legacy authoring surfaces. Prefer `body_replacement` + /// for small edits that should be validated against the current body. pub body: Option, + #[serde(default)] + pub body_replacement: Option, pub author: Option, } +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct TicketBodyReplacement { + pub old_string: String, + pub new_string: String, + #[serde(default)] + pub replace_all: bool, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct TicketBodyReplacementOutcome { + body: MarkdownText, + replacement_count: usize, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum TicketBodyEditAudit { + None, + WholeBody, + Partial { replacement_count: usize }, +} + +impl TicketItemEdit { + fn validate_body_edit_request(&self) -> Result<()> { + if self.body.is_some() && self.body_replacement.is_some() { + return Err(TicketError::Conflict( + "body and body_replacement cannot both be provided".into(), + )); + } + + if let Some(replacement) = &self.body_replacement { + replacement.validate()?; + } + + Ok(()) + } + + fn has_changes(&self) -> bool { + self.title.is_some() || self.body.is_some() || self.body_replacement.is_some() + } +} + +impl TicketBodyReplacement { + fn validate(&self) -> Result<()> { + if self.old_string.is_empty() { + return Err(TicketError::Conflict( + "body_replacement.old_string must not be empty".into(), + )); + } + if self.old_string == self.new_string { + return Err(TicketError::Conflict( + "body_replacement.old_string and new_string must differ".into(), + )); + } + Ok(()) + } + + fn apply(&self, current_body: &str) -> Result { + self.validate()?; + + let replacement_count = current_body.matches(&self.old_string).count(); + if replacement_count == 0 { + return Err(TicketError::NotFound(format!( + "old_string not found in ticket item body" + ))); + } + if replacement_count > 1 && !self.replace_all { + return Err(TicketError::Conflict(format!( + "old_string matched {replacement_count} times; set replace_all=true to replace all occurrences" + ))); + } + + let body = if self.replace_all { + current_body.replace(&self.old_string, &self.new_string) + } else { + current_body.replacen(&self.old_string, &self.new_string, 1) + }; + + Ok(TicketBodyReplacementOutcome { + body: MarkdownText(body), + replacement_count, + }) + } +} + #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] pub struct TicketDependencyCheck { pub ticket: TicketSummary, @@ -2857,17 +2945,81 @@ impl TicketBackend for SqliteTicketBackend { fn edit_item(&self, id: TicketIdOrSlug, edit: TicketItemEdit) -> Result { self.with_write(|conn| { - if edit.title.is_none() && edit.body.is_none() { return Err(TicketError::Conflict("TicketEditItem requires at least one of title or body".to_string())); } - let ticket_id = self.resolve_ticket_id(conn, id)?; - let now = now_utc(); + edit.validate_body_edit_request()?; + if !edit.has_changes() { + return Err(TicketError::Conflict( + "TicketEditItem requires at least one of title, body, or body_replacement" + .to_string(), + )); + } if let Some(title) = edit.title.as_ref() { validate_required_event_value("title", title)?; + } + if let Some(author) = edit.author.as_deref() { + validate_required_event_value("author", author)?; + } + let ticket_id = self.resolve_ticket_id(conn, id)?; + let now = now_utc(); + let mut body_edit_audit = TicketBodyEditAudit::None; + 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()); + body_edit_audit = TicketBodyEditAudit::WholeBody; + } + if let Some(replacement) = edit.body_replacement.as_ref() { + let current_body: String = conn + .query_row( + "SELECT body FROM typed_tickets WHERE workspace_id = ?1 AND ticket_id = ?2", + 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); + body_edit_audit = TicketBodyEditAudit::Partial { + replacement_count: outcome.replacement_count, + }; + } + if let Some(body) = updated_body.as_ref() { conn.execute("UPDATE typed_tickets SET body = ?3, updated_at = ?4 WHERE workspace_id = ?1 AND ticket_id = ?2", params![self.workspace_id, ticket_id, body.as_str(), now]).map_err(sqlite_err)?; } - let event = TicketEvent { kind: TicketEventKind::Other("item_edit".to_string()), author: Some(edit.author.unwrap_or_else(default_author)), at: Some(now), status: None, from: None, to: None, reason: None, state_field: None, heading: Some("Item edit".to_string()), body: edit.body.unwrap_or_else(|| MarkdownText::new("Ticket item metadata updated.")), references: Vec::new(), attributes: BTreeMap::new() }; + + let mut changes = Vec::new(); + if edit.title.is_some() { + changes.push("title"); + } + if !matches!(body_edit_audit, TicketBodyEditAudit::None) { + changes.push("body"); + } + let mut attributes = BTreeMap::new(); + attributes.insert("changes".to_string(), changes.join(",")); + let body = match body_edit_audit { + TicketBodyEditAudit::None => { + MarkdownText::new(format!("Ticket item updated: {}.", changes.join(", "))) + } + TicketBodyEditAudit::WholeBody => { + attributes.insert("body_edit".to_string(), "whole".to_string()); + MarkdownText::new(format!( + "Ticket item updated: {}. Body was replaced as a whole.", + changes.join(", ") + )) + } + TicketBodyEditAudit::Partial { replacement_count } => { + attributes.insert("body_edit".to_string(), "partial".to_string()); + attributes.insert( + "replacement_count".to_string(), + replacement_count.to_string(), + ); + MarkdownText::new(format!( + "Ticket item updated: {}. Body replacement applied to {replacement_count} occurrence(s).", + changes.join(", ") + )) + } + }; + let event = TicketEvent { kind: TicketEventKind::Other("item_edit".to_string()), author: Some(edit.author.unwrap_or_else(default_author)), at: Some(now), status: None, from: None, to: None, reason: None, state_field: None, heading: Some("Item edit".to_string()), body, references: Vec::new(), attributes }; self.insert_event(conn, &ticket_id, &event)?; self.load_ticket(conn, &ticket_id) }) @@ -3244,9 +3396,11 @@ impl TicketBackend for LocalTicketBackend { } fn edit_item(&self, id: TicketIdOrSlug, edit: TicketItemEdit) -> Result { - if edit.title.is_none() && edit.body.is_none() { + edit.validate_body_edit_request()?; + if !edit.has_changes() { return Err(TicketError::Conflict( - "TicketEditItem requires at least one of title or body".to_string(), + "TicketEditItem requires at least one of title, body, or body_replacement" + .to_string(), )); } if let Some(title) = edit.title.as_deref() { @@ -3259,6 +3413,7 @@ impl TicketBackend for LocalTicketBackend { let dir = self.find_ticket_dir(&id)?; let item = dir.join("item.md"); let mut content = fs::read_to_string(&item).map_err(|e| io_err(&item, e))?; + let mut body_edit_audit = TicketBodyEditAudit::None; let mut updates = Vec::new(); if let Some(title) = edit.title.as_deref() { updates.push(("title", title)); @@ -3278,6 +3433,23 @@ impl TicketBackend for LocalTicketBackend { message, } })?; + body_edit_audit = TicketBodyEditAudit::WholeBody; + } + if let Some(replacement) = edit.body_replacement.as_ref() { + let parsed = parse_item(&content).map_err(|message| TicketError::Parse { + path: item.clone(), + message, + })?; + let outcome = replacement.apply(parsed.body.as_str())?; + content = replace_item_body(&content, outcome.body.as_str()).map_err(|message| { + TicketError::Parse { + path: item.clone(), + message, + } + })?; + body_edit_audit = TicketBodyEditAudit::Partial { + replacement_count: outcome.replacement_count, + }; } atomic_write(&item, content.as_bytes())?; @@ -3286,17 +3458,41 @@ impl TicketBackend for LocalTicketBackend { if edit.title.is_some() { changes.push("title"); } - if edit.body.is_some() { + if !matches!(body_edit_audit, TicketBodyEditAudit::None) { changes.push("body"); } - let body = MarkdownText::new(format!("Ticket item updated: {}.", changes.join(", "))); + let mut attrs = Vec::new(); + attrs.push(("changes", changes.join(","))); + let body = match body_edit_audit { + TicketBodyEditAudit::None => { + MarkdownText::new(format!("Ticket item updated: {}.", changes.join(", "))) + } + TicketBodyEditAudit::WholeBody => { + attrs.push(("body_edit", "whole".to_string())); + MarkdownText::new(format!( + "Ticket item updated: {}. Body was replaced as a whole.", + changes.join(", ") + )) + } + TicketBodyEditAudit::Partial { replacement_count } => { + attrs.push(("body_edit", "partial".to_string())); + attrs.push(("replacement_count", replacement_count.to_string())); + MarkdownText::new(format!( + "Ticket item updated: {}. Body replacement applied to {replacement_count} occurrence(s).", + changes.join(", ") + )) + } + }; self.append_thread_event( &dir, "item_edit", self.generated_heading("Item updated", "項目更新"), &author, None, - &[], + &attrs + .iter() + .map(|(key, value)| (*key, value.as_str())) + .collect::>(), &body, )?; self.ticket_from_dir(&dir) @@ -5496,6 +5692,92 @@ mod tests { LocalTicketBackend::new(dir.path().join("tickets")) } + fn assert_partial_body_replacement_semantics(backend: &B) { + let mut input = NewTicket::new("Body Edit Ticket"); + input.body = MarkdownText::new("alpha\nbeta\nalpha\n"); + let created = backend.create(input).unwrap(); + + let edited = backend + .edit_item( + TicketIdOrSlug::Id(created.id.clone()), + TicketItemEdit { + body_replacement: Some(TicketBodyReplacement { + old_string: "alpha".to_string(), + new_string: "ALPHA".to_string(), + replace_all: true, + }), + author: Some("tester".to_string()), + ..Default::default() + }, + ) + .unwrap(); + assert_eq!( + edited.document.body.as_str().trim_start_matches('\n'), + "ALPHA\nbeta\nALPHA\n" + ); + let edit_event = edited + .events + .iter() + .rev() + .find(|event| event.kind == TicketEventKind::Other("item_edit".to_string())) + .expect("item_edit event"); + assert_eq!( + edit_event.attributes.get("body_edit"), + Some(&"partial".to_string()) + ); + assert_eq!( + edit_event.attributes.get("replacement_count"), + Some(&"2".to_string()) + ); + assert!(edit_event.body.as_str().contains("2 occurrence")); + + let duplicate_err = backend + .edit_item( + TicketIdOrSlug::Id(created.id.clone()), + TicketItemEdit { + body_replacement: Some(TicketBodyReplacement { + old_string: "ALPHA".to_string(), + new_string: "alpha".to_string(), + replace_all: false, + }), + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!(duplicate_err, TicketError::Conflict(_))); + + let missing_err = backend + .edit_item( + TicketIdOrSlug::Id(created.id.clone()), + TicketItemEdit { + body_replacement: Some(TicketBodyReplacement { + old_string: "missing".to_string(), + new_string: "present".to_string(), + replace_all: false, + }), + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!(missing_err, TicketError::NotFound(_))); + + let ambiguous_err = backend + .edit_item( + TicketIdOrSlug::Id(created.id), + TicketItemEdit { + body: Some(MarkdownText::new("whole body")), + body_replacement: Some(TicketBodyReplacement { + old_string: "beta".to_string(), + new_string: "BETA".to_string(), + replace_all: false, + }), + ..Default::default() + }, + ) + .unwrap_err(); + assert!(matches!(ambiguous_err, TicketError::Conflict(_))); + } + fn summary_with_state(state: TicketWorkflowState) -> TicketSummary { TicketSummary { id: "000TEST".to_string(), @@ -5810,6 +6092,20 @@ state: planning assert!(report.is_ok(), "{:?}", report.diagnostics); } + #[test] + fn local_backend_edit_item_supports_partial_body_replacement() { + let tmp = TempDir::new().unwrap(); + let backend = backend(&tmp); + assert_partial_body_replacement_semantics(&backend); + } + + #[test] + fn sqlite_backend_edit_item_supports_partial_body_replacement() { + let tmp = TempDir::new().unwrap(); + let backend = SqliteTicketBackend::new(tmp.path().join("workspace.db"), "workspace-test"); + assert_partial_body_replacement_semantics(&backend); + } + #[test] fn sqlite_backend_persists_core_ticket_operations() { let tmp = TempDir::new().unwrap(); diff --git a/crates/ticket/src/tool.rs b/crates/ticket/src/tool.rs index c4d07499..6477a05d 100644 --- a/crates/ticket/src/tool.rs +++ b/crates/ticket/src/tool.rs @@ -14,11 +14,11 @@ use serde_json::{Value, json}; use crate::{ AcceptedOrchestrationPlan, LocalTicketBackend, MarkdownText, NewOrchestrationPlanRecord, NewTicket, NewTicketEvent, NewTicketRelation, OrchestrationPlanKind, OrchestrationPlanRecord, - Result as TicketResult, Ticket, TicketBackend, TicketDoctorDiagnostic, TicketDoctorReport, - TicketDoctorSeverity, TicketError, TicketEventKind, TicketIdOrSlug, TicketIntakeSummary, - TicketListState, TicketRef, TicketRelation, TicketRelationKind, TicketRelationView, - TicketReview, TicketReviewResult, TicketStateChange, TicketSummary, TicketWorkflowState, - default_author, + Result as TicketResult, Ticket, TicketBackend, TicketBodyReplacement, TicketDoctorDiagnostic, + TicketDoctorReport, TicketDoctorSeverity, TicketError, TicketEventKind, TicketIdOrSlug, + TicketIntakeSummary, TicketListState, TicketRef, TicketRelation, TicketRelationKind, + TicketRelationView, TicketReview, TicketReviewResult, TicketStateChange, TicketSummary, + TicketWorkflowState, default_author, }; const DEFAULT_LIST_LIMIT: usize = 50; @@ -391,9 +391,18 @@ struct TicketEditItemParams { /// Optional replacement title. #[serde(default)] title: Option, - /// Optional replacement Markdown body. + /// Optional replacement Markdown body. This replaces the entire item body. #[serde(default)] body: Option, + /// Exact body substring to replace. Must be provided with `new_string`; omitted for whole-body edits. + #[serde(default)] + old_string: Option, + /// Replacement text for `old_string`. Must be provided with `old_string`. + #[serde(default)] + new_string: Option, + /// Replace every occurrence of `old_string`; by default exactly one occurrence is required. + #[serde(default)] + replace_all: bool, /// Optional thread author for the audited item_edit event. #[serde(default)] author: Option, @@ -930,9 +939,26 @@ impl Tool for TicketEditItemTool { _ctx: llm_engine::tool::ToolExecutionContext, ) -> Result { let params: TicketEditItemParams = parse_input("TicketEditItem", input_json)?; + let body_replacement = match (params.old_string, params.new_string) { + (Some(old_string), Some(new_string)) => Some(TicketBodyReplacement { + old_string, + new_string, + replace_all: params.replace_all, + }), + (None, None) => None, + _ => { + return Err(backend_error( + "TicketEditItem", + TicketError::Conflict( + "old_string and new_string must be provided together".to_string(), + ), + )); + } + }; let edit = crate::TicketItemEdit { title: params.title, body: params.body.map(MarkdownText::new), + body_replacement, author: params.author, }; let ticket = self @@ -2180,6 +2206,67 @@ mod tests { } } + #[tokio::test] + async fn ticket_edit_item_tool_supports_exact_body_replacement() { + let temp = TempDir::new().unwrap(); + let backend = backend(&temp); + let mut input = NewTicket::new("Tool Body Edit"); + input.body = MarkdownText::new("one\ntwo\none\n"); + let created = backend.create(input).unwrap(); + let edit = tool_by_name(backend.clone(), "TicketEditItem"); + + let output = edit + .execute( + &json!({ + "ticket": created.id.clone(), + "old_string": "one", + "new_string": "ONE", + "replace_all": true, + "author": "tool-test" + }) + .to_string(), + Default::default(), + ) + .await + .unwrap(); + let value: Value = serde_json::from_str(&output.content.unwrap()).unwrap(); + assert_eq!( + value["body"].as_str().unwrap().trim_start_matches('\n'), + "ONE\ntwo\nONE\n" + ); + + let record = backend + .show(TicketIdOrSlug::Id(created.id.clone())) + .unwrap(); + let event = record + .events + .iter() + .rev() + .find(|event| event.kind == TicketEventKind::Other("item_edit".to_string())) + .expect("item_edit event"); + assert_eq!( + event.attributes.get("body_edit"), + Some(&"partial".to_string()) + ); + assert_eq!( + event.attributes.get("replacement_count"), + Some(&"2".to_string()) + ); + + let error = edit + .execute( + &json!({ + "ticket": created.id, + "old_string": "ONE" + }) + .to_string(), + Default::default(), + ) + .await + .unwrap_err(); + assert!(error.to_string().contains("old_string and new_string")); + } + #[tokio::test] async fn ticket_relation_tools_record_query_and_show_derived_view() { let temp = TempDir::new().unwrap(); @@ -2698,6 +2785,16 @@ mod tests { .input_schema .to_string(); assert!(plan_query_schema.contains("relation_kind")); + let edit_schema = tools + .iter() + .map(|definition| definition().0) + .find(|meta| meta.name == "TicketEditItem") + .unwrap() + .input_schema + .to_string(); + assert!(edit_schema.contains("old_string")); + assert!(edit_schema.contains("new_string")); + assert!(edit_schema.contains("replace_all")); let names = tools .into_iter() .map(|definition| definition().0)