From da4953b4a580ec7a5305657016a82aee4e134c8d Mon Sep 17 00:00:00 2001 From: Brent Rager Date: Thu, 23 Jul 2026 18:13:38 -0500 Subject: [PATCH] =?UTF-8?q?SMOODEV-2720:=20th=20jira=20sync=20safe-by-defa?= =?UTF-8?q?ult=20=E2=80=94=20reconcile=20only;=20--pull/--push=20opt-in;?= =?UTF-8?q?=20--dry-run?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The old sync unconditionally created a pearl for every untracked open Jira ticket AND a Jira ticket for every open pearl without a SMOODEV- prefix, and it reconciled against PearlQuery::new()'s default (100 rows, no status filter) — a random slice including closed pearls. Running it on a real project would have exploded both trackers. Now the default run only reconciles: closes active pearls whose every referenced issue key is Done in Jira, and transitions Jira tickets to Done once every referencing pearl is closed. Mass-creation is opt-in via --pull / --push, --dry-run previews the plan, pearls load with with_limit(0), and key extraction requires digits + a word boundary so SMOODEV-XXX placeholders never match. Planner is a pure function in smooth-diver::jira with unit tests. Dogfooded against the live smooai pearls + SMOODEV project. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0166oMz2kqo7J8e7TFxdYytY --- .changeset/jira-sync-safe-by-default.md | 5 + CLAUDE.md | 6 +- crates/smooth-cli/src/main.rs | 238 ++++++++++++++---------- crates/smooth-diver/src/jira.rs | 214 +++++++++++++++++++++ 4 files changed, 360 insertions(+), 103 deletions(-) create mode 100644 .changeset/jira-sync-safe-by-default.md diff --git a/.changeset/jira-sync-safe-by-default.md b/.changeset/jira-sync-safe-by-default.md new file mode 100644 index 00000000..d70f573d --- /dev/null +++ b/.changeset/jira-sync-safe-by-default.md @@ -0,0 +1,5 @@ +--- +'@smooai/smooth': patch +--- + +SMOODEV-2720: `th jira sync` is now safe by default — it only reconciles (closes pearls whose Jira tickets are all Done, transitions Jira tickets to Done once every referencing pearl is closed). The old unconditional mass-creation moved behind explicit `--pull` (Jira→pearls) and `--push` (pearls→Jira) flags, and `--dry-run` previews the full plan. Also fixes the sync reconciling against `PearlQuery::new()`'s default 100-row unfiltered slice (now loads all pearls) and key matching that treated placeholders like `SMOODEV-XXX` as issue keys. diff --git a/CLAUDE.md b/CLAUDE.md index 8b45a6f6..356f4e96 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,11 @@ th api orgs|agents|smooth-operator|knowledge|jobs|members|config|keys|observabil th admin onboard-customer / mint-key / set-secret / org list|show # Jira — replaces curl -u "$JIRA_EMAIL:$JIRA_API_TOKEN" .../rest/api/3/... -th jira sync / status +# sync is reconcile-only by default (close pearls done in Jira, transition +# Jira tickets whose pearls are all closed); creating anything is opt-in: +# --pull (Jira→pearls), --push (pearls→Jira), --dry-run previews the plan. +# Config = env vars: JIRA_URL, JIRA_PROJECT, JIRA_EMAIL, JIRA_API_TOKEN. +th jira sync [--dry-run] [--pull] [--push] / status # Pearls (the only spelling — no `th issues` / `th beads` aliases) th pearls create / ready / list / show / update / close / push / pull diff --git a/crates/smooth-cli/src/main.rs b/crates/smooth-cli/src/main.rs index e68d4d65..663628f3 100644 --- a/crates/smooth-cli/src/main.rs +++ b/crates/smooth-cli/src/main.rs @@ -1022,8 +1022,20 @@ enum DbCommands { #[derive(Subcommand)] enum JiraCommands { - /// Sync with Jira - Sync, + /// Reconcile pearls ↔ Jira: close pearls whose Jira tickets are Done, and + /// transition Jira tickets to Done once every referencing pearl is closed. + /// Creating anything is opt-in via --pull / --push. + Sync { + /// Print the plan without changing anything + #[arg(long)] + dry_run: bool, + /// Also create a local pearl for every open Jira ticket no pearl references + #[arg(long)] + pull: bool, + /// Also create a Jira ticket for every active pearl without an issue key in its title + #[arg(long)] + push: bool, + }, /// Show Jira status Status, } @@ -3557,7 +3569,7 @@ llm-errors/ async fn cmd_jira(cmd: JiraCommands) -> Result<()> { match cmd { JiraCommands::Status => cmd_jira_status().await, - JiraCommands::Sync => cmd_jira_sync().await, + JiraCommands::Sync { dry_run, pull, push } => cmd_jira_sync(dry_run, pull, push).await, } } @@ -3623,7 +3635,9 @@ async fn cmd_jira_status() -> Result<()> { Ok(()) } -async fn cmd_jira_sync() -> Result<()> { +async fn cmd_jira_sync(dry_run: bool, pull: bool, push: bool) -> Result<()> { + use smooth_diver::jira::{plan_sync, SyncPearl}; + let Some(config) = smooth_diver::jira::JiraConfig::from_env() else { anyhow::bail!("Jira not configured. Set JIRA_URL, JIRA_PROJECT, JIRA_EMAIL, JIRA_API_TOKEN env vars."); }; @@ -3634,61 +3648,114 @@ async fn cmd_jira_sync() -> Result<()> { } let store = open_pearl_store()?; - println!("{}", "Syncing pearls ↔ Jira...".bold().cyan()); + println!("{}", "Reconciling pearls ↔ Jira...".bold().cyan()); - // --- Pull: Jira → Pearls (create local pearls for Jira tickets) --- - let http = reqwest::Client::new(); - let mut jira_issues: Vec = Vec::new(); - let mut next_page: Option = None; - loop { - let mut url = format!( - "{}/rest/api/3/search/jql?jql=project%3D{}+AND+status+!%3D+Done+ORDER+BY+key+DESC&maxResults=100&fields=key,summary,status,description", - config.url, config.project + let jira_issues = client.list_project_issues().await?; + let all_pearls = store.list(&smooth_pearls::PearlQuery::new().with_limit(0))?; + let sync_pearls: Vec = all_pearls + .iter() + .map(|p| SyncPearl { + id: p.id.clone(), + status: p.status.as_str().to_string(), + title: p.title.clone(), + }) + .collect(); + let plan = plan_sync(&sync_pearls, &jira_issues, &config.project); + + println!( + " {} pearl(s), {} Jira issue(s) → close {} pearl(s), transition {} ticket(s) to Done", + sync_pearls.len(), + jira_issues.len(), + plan.close_pearls.len().to_string().green(), + plan.transition_keys.len().to_string().green(), + ); + if !pull && !plan.untracked_jira.is_empty() { + println!( + " {} open Jira ticket(s) have no pearl — pass --pull to create pearls for them", + plan.untracked_jira.len().to_string().yellow() ); - if let Some(ref token) = next_page { - url.push_str(&format!("&nextPageToken={token}")); + } + if !push && !plan.unkeyed_pearls.is_empty() { + println!( + " {} active pearl(s) have no Jira key — pass --push to create tickets for them", + plan.unkeyed_pearls.len().to_string().yellow() + ); + } + + if dry_run { + for pearl in &plan.close_pearls { + println!(" {} would close {} ({})", "◌".cyan(), pearl.id, pearl.title); } - let resp = http.get(&url).basic_auth(&config.email, Some(&config.api_token)).send().await?; - let body: serde_json::Value = resp.json().await?; - if let Some(issues) = body["issues"].as_array() { - jira_issues.extend(issues.iter().cloned()); + for key in &plan.transition_keys { + println!(" {} would transition {} → Done", "◌".cyan(), key); } - if body["isLast"].as_bool().unwrap_or(true) { - break; + if pull { + for key in &plan.untracked_jira { + println!(" {} would create pearl for {}", "◌".cyan(), key); + } + } + if push { + for pearl in &plan.unkeyed_pearls { + println!(" {} would create Jira ticket for {} ({})", "◌".cyan(), pearl.id, pearl.title); + } } - next_page = body["nextPageToken"].as_str().map(String::from); + println!("{}", "dry run — nothing changed".yellow()); + return Ok(()); } - // Get all open pearls - let open_pearls = store.list(&smooth_pearls::PearlQuery::new())?; - - // Find Jira tickets not yet tracked as pearls (by title prefix match) - let mut pulled = 0u32; - for issue in &jira_issues { - let key = issue["key"].as_str().unwrap_or(""); - let summary = issue["fields"]["summary"].as_str().unwrap_or(""); + // Close pearls whose Jira work is Done. + let close_ids: Vec<&str> = plan.close_pearls.iter().map(|p| p.id.as_str()).collect(); + let closed = if close_ids.is_empty() { 0 } else { store.close(&close_ids)? }; + for pearl in &plan.close_pearls { + println!(" {} closed {} ({})", "✓".green(), pearl.id, pearl.title); + } - // Check if any pearl already has this Jira key in its title - let already_tracked = open_pearls.iter().any(|p| p.title.contains(key)); - if already_tracked { - continue; + // Transition Jira tickets whose pearls are all closed. + let mut transitioned = 0u32; + for key in &plan.transition_keys { + match client.transition_ticket(key, "done").await { + Ok(()) => { + println!(" {} {} → Done", "✓".green(), key); + transitioned += 1; + } + Err(e) => eprintln!(" {} {} transition failed: {e}", "✗".red(), key), } + } - // Create a pearl for this Jira ticket - let title = format!("{key}: {summary}"); - let desc = issue["fields"]["description"] - .as_object() - .and_then(|d| d["content"].as_array()) - .and_then(|a| a.first()) - .and_then(|p| p["content"].as_array()) - .and_then(|a| a.first()) - .and_then(|t| t["text"].as_str()) - .unwrap_or("") - .to_string(); + // Opt-in: Jira → pearls. + let pulled = if pull { + jira_sync_pull(&store, &jira_issues, &plan.untracked_jira) + } else { + 0 + }; + // Opt-in: pearls → Jira. + let pushed = if push { + jira_sync_push(&store, &client, &all_pearls, &plan.unkeyed_pearls).await + } else { + 0 + }; + + println!(); + println!( + "{} pearl(s) closed, {} ticket(s) transitioned, {} pulled, {} pushed", + closed.to_string().green(), + transitioned.to_string().green(), + pulled.to_string().cyan(), + pushed.to_string().cyan(), + ); + + Ok(()) +} + +/// Create local pearls for open Jira tickets nothing references (`th jira sync --pull`). +fn jira_sync_pull(store: &smooth_pearls::PearlStore, jira_issues: &[smooth_diver::jira::JiraIssue], untracked: &[String]) -> u32 { + let mut pulled = 0u32; + for key in untracked { + let Some(issue) = jira_issues.iter().find(|i| &i.key == key) else { continue }; let new = smooth_pearls::NewPearl { - title, - description: desc, + title: format!("{}: {}", issue.key, issue.summary), + description: issue.description.clone().unwrap_or_default(), pearl_type: smooth_pearls::PearlType::Task, priority: smooth_pearls::Priority::Medium, assigned_to: None, @@ -3700,73 +3767,40 @@ async fn cmd_jira_sync() -> Result<()> { println!(" {} {} → {}", "↓".cyan(), key, pearl.id); pulled += 1; } - Err(e) => { - eprintln!(" {} {} failed: {e}", "✗".red(), key); - } + Err(e) => eprintln!(" {} {} failed: {e}", "✗".red(), key), } } + pulled +} - // --- Push: Pearls → Jira (create Jira tickets for pearls without SMOODEV prefix) --- +/// Create Jira tickets for active pearls with no issue key (`th jira sync --push`). +async fn jira_sync_push( + store: &smooth_pearls::PearlStore, + client: &smooth_diver::jira::JiraClient, + all_pearls: &[smooth_pearls::Pearl], + unkeyed: &[smooth_diver::jira::SyncPearl], +) -> u32 { let mut pushed = 0u32; - for pearl in &open_pearls { - // Skip if already has a Jira key in title - if pearl.title.starts_with("SMOODEV-") { - continue; - } - - match client.create_ticket(&pearl.title, &pearl.description).await { + for sync_pearl in unkeyed { + let description = all_pearls + .iter() + .find(|p| p.id == sync_pearl.id) + .map(|p| p.description.clone()) + .unwrap_or_default(); + match client.create_ticket(&sync_pearl.title, &description).await { Ok(ticket) => { - // Update pearl title with Jira key - let new_title = format!("{}: {}", ticket.key, pearl.title); let update = smooth_pearls::PearlUpdate { - title: Some(new_title), + title: Some(format!("{}: {}", ticket.key, sync_pearl.title)), ..Default::default() }; - let _ = store.update(&pearl.id, &update); - println!(" {} {} → {}", "↑".green(), pearl.id, ticket.key); + let _ = store.update(&sync_pearl.id, &update); + println!(" {} {} → {}", "↑".green(), sync_pearl.id, ticket.key); pushed += 1; } - Err(e) => { - eprintln!(" {} {} failed: {e}", "✗".red(), pearl.id); - } - } - } - - // --- Close: Transition Jira tickets to Done for closed pearls --- - let closed_pearls = store.list(&smooth_pearls::PearlQuery::new().with_status(smooth_pearls::PearlStatus::Closed))?; - let mut transitioned = 0u32; - for pearl in &closed_pearls { - // Extract SMOODEV-XXX from title - let jira_key = pearl.title.split(':').next().filter(|k| k.starts_with("SMOODEV-")).map(str::trim); - - let Some(key) = jira_key else { continue }; - - // Check if Jira ticket is still open - let is_open = jira_issues.iter().any(|i| i["key"].as_str() == Some(key)); - if !is_open { - continue; - } - - match client.transition_ticket(key, "done").await { - Ok(()) => { - println!(" {} {} → Done", "✓".green(), key); - transitioned += 1; - } - Err(e) => { - eprintln!(" {} {} transition failed: {e}", "✗".red(), key); - } + Err(e) => eprintln!(" {} {} failed: {e}", "✗".red(), sync_pearl.id), } } - - println!(); - println!( - "{} pulled, {} pushed, {} transitioned", - pulled.to_string().cyan(), - pushed.to_string().green(), - transitioned.to_string().green() - ); - - Ok(()) + pushed } // ── Pearls ───────────────────────────────────────────────────────── diff --git a/crates/smooth-diver/src/jira.rs b/crates/smooth-diver/src/jira.rs index 07abc0da..74d0651d 100644 --- a/crates/smooth-diver/src/jira.rs +++ b/crates/smooth-diver/src/jira.rs @@ -197,6 +197,147 @@ impl JiraClient { pub fn project(&self) -> &str { &self.config.project } + + /// Fetch every issue in the configured project (all statuses), paginated. + /// + /// The description is flattened to the first paragraph's plain text. + /// + /// # Errors + /// Fails if a search request cannot be sent, returns a non-success status, or the response is not valid JSON. + pub async fn list_project_issues(&self) -> Result> { + let mut issues = Vec::new(); + let mut next_page: Option = None; + loop { + let base = format!( + "{}/rest/api/3/search/jql?jql=project%3D{}+ORDER+BY+key+DESC&maxResults=100&fields=key,summary,status,description", + self.config.url, self.config.project + ); + let url = match next_page { + Some(ref token) => format!("{base}&nextPageToken={token}"), + None => base, + }; + let resp = self + .http + .get(&url) + .basic_auth(&self.config.email, Some(&self.config.api_token)) + .send() + .await + .context("jira: search issues")?; + if !resp.status().is_success() { + let status = resp.status(); + let body = resp.text().await.unwrap_or_default(); + anyhow::bail!("jira: search failed ({status}): {body}"); + } + let body: serde_json::Value = resp.json().await.context("jira: parse search response")?; + for issue in body["issues"].as_array().unwrap_or(&Vec::new()) { + issues.push(JiraIssue { + key: issue["key"].as_str().unwrap_or_default().to_string(), + summary: issue["fields"]["summary"].as_str().unwrap_or_default().to_string(), + status: issue["fields"]["status"]["name"].as_str().unwrap_or_default().to_string(), + description: adf_first_paragraph_text(&issue["fields"]["description"]), + }); + } + if body["isLast"].as_bool().unwrap_or(true) { + break; + } + next_page = body["nextPageToken"].as_str().map(String::from); + } + Ok(issues) + } +} + +/// Pull the first paragraph's plain text out of an ADF description document. +fn adf_first_paragraph_text(doc: &serde_json::Value) -> Option { + let text = doc["content"].as_array()?.first()?["content"].as_array()?.first()?["text"].as_str()?; + Some(text.to_string()) +} + +/// A pearl as the sync planner needs it: id, status (`open`/`in_progress`/`closed`), title. +#[derive(Debug, Clone)] +pub struct SyncPearl { + pub id: String, + pub status: String, + pub title: String, +} + +/// What a `jira sync` run would do. Computed purely from local pearls + +/// fetched Jira issues so it can be previewed (`--dry-run`) and unit-tested. +#[derive(Debug, Default)] +pub struct SyncPlan { + /// Active pearls whose every referenced issue key is Done in Jira → close. + pub close_pearls: Vec, + /// Issue keys where every referencing pearl is closed but Jira is still open → transition to Done. + pub transition_keys: Vec, + /// Open Jira keys no pearl references → pearl created only with `--pull`. + pub untracked_jira: Vec, + /// Active pearls with no issue key in the title → Jira ticket created only with `--push`. + pub unkeyed_pearls: Vec, +} + +/// Extract `PROJECT-123` issue keys from a title. +/// +/// Requires at least one digit after the dash and a non-alphanumeric boundary +/// before the project name, so placeholders like `SMOODEV-XXX` or +/// `XSMOODEV-1` never match. +pub fn extract_keys(title: &str, project: &str) -> Vec { + let needle = format!("{project}-"); + let mut keys = Vec::new(); + let mut from = 0; + while let Some(pos) = title[from..].find(&needle) { + let start = from + pos; + from = start + needle.len(); + let boundary_ok = start == 0 || !title[..start].chars().next_back().is_some_and(char::is_alphanumeric); + let digits: String = title[from..].chars().take_while(char::is_ascii_digit).collect(); + if boundary_ok && !digits.is_empty() { + let key = format!("{project}-{digits}"); + if !keys.contains(&key) { + keys.push(key); + } + } + } + keys +} + +/// Compute the reconciliation plan between local pearls and Jira issues. +pub fn plan_sync(pearls: &[SyncPearl], jira: &[JiraIssue], project: &str) -> SyncPlan { + use std::collections::HashMap; + let jira_status: HashMap<&str, &str> = jira.iter().map(|i| (i.key.as_str(), i.status.as_str())).collect(); + + let mut plan = SyncPlan::default(); + // active/closed pearl counts per referenced key + let mut refs: HashMap = HashMap::new(); + for pearl in pearls { + let keys = extract_keys(&pearl.title, project); + let active = pearl.status != "closed"; + for key in &keys { + let entry = refs.entry(key.clone()).or_default(); + if active { + entry.0 += 1; + } else { + entry.1 += 1; + } + } + if active { + if keys.is_empty() { + plan.unkeyed_pearls.push(pearl.clone()); + } else if keys.iter().all(|k| jira_status.get(k.as_str()) == Some(&"Done")) { + plan.close_pearls.push(pearl.clone()); + } + } + } + for issue in jira { + if issue.status == "Done" { + continue; + } + match refs.get(&issue.key) { + None => plan.untracked_jira.push(issue.key.clone()), + Some((0, closed)) if *closed > 0 => plan.transition_keys.push(issue.key.clone()), + Some(_) => {} + } + } + plan.transition_keys.sort(); + plan.untracked_jira.sort(); + plan } #[cfg(test)] @@ -227,4 +368,77 @@ mod tests { std::env::remove_var("JIRA_URL"); assert!(JiraClient::from_env().is_none()); } + + #[test] + fn extract_keys_matches_real_keys_only() { + assert_eq!(extract_keys("SMOODEV-42: fix thing", "SMOODEV"), vec!["SMOODEV-42"]); + assert_eq!(extract_keys("re-fix (SMOODEV-7) and SMOODEV-8", "SMOODEV"), vec!["SMOODEV-7", "SMOODEV-8"]); + // no digits, embedded prefix, wrong project → no match + assert!(extract_keys("SMOODEV-XXX: placeholder", "SMOODEV").is_empty()); + assert!(extract_keys("XSMOODEV-1: not ours", "SMOODEV").is_empty()); + assert!(extract_keys("OTHER-12: different project", "SMOODEV").is_empty()); + // duplicates collapse + assert_eq!(extract_keys("SMOODEV-5 dup SMOODEV-5", "SMOODEV"), vec!["SMOODEV-5"]); + } + + fn pearl(id: &str, status: &str, title: &str) -> SyncPearl { + SyncPearl { + id: id.into(), + status: status.into(), + title: title.into(), + } + } + + fn issue(key: &str, status: &str) -> JiraIssue { + JiraIssue { + key: key.into(), + summary: String::new(), + status: status.into(), + description: None, + } + } + + #[test] + fn plan_closes_pearls_only_when_every_key_is_done() { + let pearls = [ + pearl("th-1", "open", "SMOODEV-1: done in jira"), + pearl("th-2", "in_progress", "SMOODEV-1 + SMOODEV-2 spans two"), + pearl("th-3", "open", "SMOODEV-9: key missing from jira"), + ]; + let jira = [issue("SMOODEV-1", "Done"), issue("SMOODEV-2", "To Do")]; + let plan = plan_sync(&pearls, &jira, "SMOODEV"); + let ids: Vec<_> = plan.close_pearls.iter().map(|p| p.id.as_str()).collect(); + assert_eq!(ids, vec!["th-1"]); + } + + #[test] + fn plan_transitions_jira_only_when_all_referencing_pearls_closed() { + let pearls = [ + pearl("th-1", "closed", "SMOODEV-1: shipped"), + pearl("th-2", "closed", "SMOODEV-2: shipped"), + pearl("th-3", "open", "SMOODEV-2: follow-up still active"), + ]; + let jira = [issue("SMOODEV-1", "In Progress"), issue("SMOODEV-2", "In Progress"), issue("SMOODEV-3", "Done")]; + let plan = plan_sync(&pearls, &jira, "SMOODEV"); + assert_eq!(plan.transition_keys, vec!["SMOODEV-1"]); + } + + #[test] + fn plan_reports_untracked_and_unkeyed_without_acting_on_them() { + let pearls = [pearl("th-1", "open", "no key here"), pearl("th-2", "closed", "also unkeyed, but closed")]; + let jira = [issue("SMOODEV-1", "To Do"), issue("SMOODEV-2", "Done")]; + let plan = plan_sync(&pearls, &jira, "SMOODEV"); + assert_eq!(plan.untracked_jira, vec!["SMOODEV-1"]); // Done tickets never pulled + let ids: Vec<_> = plan.unkeyed_pearls.iter().map(|p| p.id.as_str()).collect(); + assert_eq!(ids, vec!["th-1"]); // closed pearls never pushed + assert!(plan.close_pearls.is_empty()); + assert!(plan.transition_keys.is_empty()); + } + + #[test] + fn adf_description_flattens_first_paragraph() { + let doc = serde_json::json!({"content":[{"content":[{"text":"hello world"}]}]}); + assert_eq!(adf_first_paragraph_text(&doc), Some("hello world".to_string())); + assert_eq!(adf_first_paragraph_text(&serde_json::Value::Null), None); + } }