diff --git a/crates/diffr-core/Cargo.toml b/crates/diffr-core/Cargo.toml index 57186f80c..f4c2e5714 100644 --- a/crates/diffr-core/Cargo.toml +++ b/crates/diffr-core/Cargo.toml @@ -122,6 +122,7 @@ serde_path_to_error = "0.1.20" schemars = { version = "1.2.2", features = ["preserve_order"] } dirs = "5" toml_edit = "0.25.15" +tempfile = "3.27.0" jsonschema = { version = "0.56.0", default-features = false } # The plugin contract's records, generated from the SDK's WIT without a # component runtime. diff --git a/crates/diffr-core/src/config.rs b/crates/diffr-core/src/config.rs index e5bbc4257..6b3d1cd1d 100644 --- a/crates/diffr-core/src/config.rs +++ b/crates/diffr-core/src/config.rs @@ -17,6 +17,7 @@ //! query files of every enabled plugin (see [`crate::plugin::queries`]): its //! `@fold` captures decide which folds exist, and its tags what they are. //! Every tag a query sets must be written `:`. +pub mod migrate; pub(crate) mod prune; pub(crate) mod query; pub mod store; @@ -226,7 +227,7 @@ impl Config { let mut config: Self = serde_path_to_error::deserialize(toml::Deserializer::new(source)) .map_err(|error| { let path = error.path().to_string(); - let message = error.inner().to_string(); + let message = error.inner().message().to_owned(); ConfigError(match path.as_str() { "." => message, _ => format!("{path}: {message}"), diff --git a/crates/diffr-core/src/config/migrate.rs b/crates/diffr-core/src/config/migrate.rs new file mode 100644 index 000000000..64d33c02a --- /dev/null +++ b/crates/diffr-core/src/config/migrate.rs @@ -0,0 +1,300 @@ +//! Convert the v1 settings supported by Whiteboard. Custom plugins and +//! pipeline configuration are not carried forward. No plugin files are read +//! until the completed v2 candidate is validated. +use super::{store, Config, ConfigError}; +use serde::Serialize; +use std::path::Path; +use toml_edit::{DocumentMut, Item}; + +const SUMMARY_FIELDS: &[&str] = &[ + "enabled", + "provider", + "model", + "api_key", + "endpoint", + "system_prompt", + "tests", +]; +const STOCK_PROMPTS: &[&str] = &[ + r#"For each listed fold, rewrite that function body as short python-flavored pseudocode. Keep the names. No prose, no comments, no code fences. Use as few lines as possible: about one pseudocode line per five source lines, and never more than a third of the body's lines. When a fold lists a doc, also set "summary" to one sentence copied verbatim from that doc; otherwise leave it empty. Answer with a JSON array of {"id", "summary", "pseudocode"} objects, one per fold."#, + r#"For each listed fold, rewrite that function body as short pseudocode. Keep the names. No prose, no comments, no code fences. Use as few lines as possible: about one pseudocode line per five source lines, and never more than a third of the body's lines. When a fold lists a doc, also set "summary" to one sentence copied verbatim from that doc; otherwise leave it empty. Answer with a JSON array of {"id", "summary", "pseudocode"} objects, one per fold."#, + r#"For each listed fold, rewrite that function body as short pseudocode. Keep the names. No prose, no comments, no code fences. Use as few lines as possible: about one pseudocode line per five source lines, and never more than a third of the body's lines. When a fold lists a doc, also set "summary" to one sentence copied verbatim from that doc; otherwise leave it empty. Answer with a JSON object whose "summaries" array holds one {"id", "summary", "pseudocode"} object per fold."#, +]; + +#[derive(Debug, Serialize)] +pub struct Migration { + pub changed: bool, + pub from: Option, + pub to: u32, + pub dropped: Vec, +} + +/// Migrate the global file's supported v1 settings. Current and missing files +/// are unchanged. A parse or validation failure leaves the original intact. +pub fn migrate(path: &Path) -> Result { + let mut result = Migration { + changed: false, + from: None, + to: super::CONFIG_VERSION, + dropped: Vec::new(), + }; + if !path + .try_exists() + .map_err(|error| ConfigError(format!("{}: {error}", path.display())))? + { + return Ok(result); + } + result.changed = store::edit(path, |source, directory| { + let mut document = store::parse(source)?; + let version = match document.get("version") { + None => 1, + Some(item) => item + .as_integer() + .ok_or_else(|| ConfigError("version must be an integer".into()))?, + }; + result.from = Some(version); + if version == i64::from(super::CONFIG_VERSION) { + return Ok(document); + } + if version != 1 { + return Err(ConfigError(format!( + "unsupported config version {version}; migration supports version 1" + ))); + } + let old = document.clone(); + document.remove("plugins"); + document.remove("classifier"); + document.insert( + "version", + toml_edit::value(i64::from(super::CONFIG_VERSION)), + ); + let mut retained = Vec::new(); + for name in [ + "context", + "deleted-bodies", + "test-bodies", + "removed-runs", + "summarize", + ] { + let fields: &[&str] = match name { + "summarize" => SUMMARY_FIELDS, + "context" => &["enabled", "lines"], + _ => &["enabled"], + }; + for field in fields { + let path = ["plugins", "bundled", name, field]; + if let Some(item) = get(&old, &path) { + if name == "summarize" + && *field == "system_prompt" + && item + .as_str() + .is_some_and(|text| STOCK_PROMPTS.contains(&text)) + { + continue; + } + store::insert( + &mut document, + &["plugins", "shape", "bundled", name, field], + item.clone(), + )?; + retained.push(path.join(".")); + } + } + } + // Explicit classifier values in intermediate v1 files take precedence. + for field in ["hide", "hide_deleted"] { + if let Some(item) = get(&old, &["classifier", field]) { + store::insert( + &mut document, + &["plugins", "classify", "bundled", field], + item.clone(), + )?; + retained.push(format!("classifier.{field}")); + } + } + if let Some(hide) = get(&old, &["plugins", "bundled", "hide-files"]) { + let flag = |field: &str| -> Result { + hide.get(field) + .map(|item| { + item.as_bool().ok_or_else(|| { + ConfigError(format!( + "plugins.bundled.hide-files.{field}: expected boolean" + )) + }) + }) + .unwrap_or(Ok(true)) + }; + let enabled = flag("enabled")?; + let deleted = flag("deleted")?; + if get(&old, &["classifier", "hide"]).is_none() { + let tags = if enabled { + hide.get("tags").cloned().unwrap_or_else(|| { + let mut tags = toml_edit::Array::new(); + for tag in ["generated", "vendored", "test"] { + tags.push(tag); + } + Item::Value(tags.into()) + }) + } else { + Item::Value(toml_edit::Array::new().into()) + }; + store::insert( + &mut document, + &["plugins", "classify", "bundled", "hide"], + tags, + )?; + } + if get(&old, &["classifier", "hide_deleted"]).is_none() { + store::insert( + &mut document, + &["plugins", "classify", "bundled", "hide_deleted"], + toml_edit::value(enabled && deleted), + )?; + } + for field in ["enabled", "tags", "deleted"] { + retained.push(format!("plugins.bundled.hide-files.{field}")); + } + } + for root in ["plugins", "classifier"] { + if let Some(item) = old.get(root) { + dropped(item, &mut vec![root.into()], &retained, &mut result.dropped); + } + } + Config::from_toml_in(&document.to_string(), directory)?; + Ok(document) + })?; + Ok(result) +} + +fn get<'a>(document: &'a DocumentMut, parts: &[&str]) -> Option<&'a Item> { + parts + .iter() + .try_fold(document.as_item(), |item, part| item.get(part)) +} + +fn dropped(item: &Item, path: &mut Vec, retained: &[String], result: &mut Vec) { + if let Some(table) = item.as_table_like() { + for (key, item) in table.iter() { + path.push(key.into()); + dropped(item, path, retained, result); + path.pop(); + } + } else { + let key = path.join("."); + if !retained.contains(&key) { + result.push(key); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn file(source: &str) -> (tempfile::TempDir, std::path::PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write(&path, source).unwrap(); + (dir, path) + } + + #[test] + fn migrates_whiteboard_values_and_drops_unmanaged_plugins_without_reading_them() { + let (_dir, path) = file(concat!( + "version = 1\n[plugins]\norder = ['external.bundled', 'bundled.summarize']\n", + "[plugins.external.bundled]\npath = 'does-not-exist'\n", + "[plugins.bundled.context]\nenabled = false\nlines = 17 # keep this\n", + "[plugins.bundled.summarize]\nenabled = true\ninstances = 3\nmax_concurrency = 9\n", + "provider = 'openai'\napi_key = 'saved-key'\nmodel = 'my-model'\nendpoint = 'https://example.test'\n", + "system_prompt = ''' Custom prompt\nwith two lines.\n'''\ntests = false\nmin_lines = 999\n", + "[plugins.bundled.group]\nenabled = true\n", + "[plugins.bundled.hide-files]\nenabled = true\ntags = ['test']\ndeleted = false\n", + "[diff]\nbyte_limit = 123456\n[theme]\npath = 'themes/mine.toml'\n" + )); + let result = migrate(&path).unwrap(); + assert!(result.changed); + assert!(result + .dropped + .contains(&"plugins.bundled.summarize.instances".into())); + assert!(result + .dropped + .contains(&"plugins.external.bundled.path".into())); + let text = std::fs::read_to_string(&path).unwrap(); + let config = Config::from_toml_in(&text, path.parent().unwrap()).unwrap(); + let values = store::show(&config, true); + assert_eq!( + values["plugins"]["shape"]["bundled"]["context"]["lines"], + 17 + ); + assert_eq!( + values["plugins"]["shape"]["bundled"]["context"]["enabled"], + false + ); + assert_eq!( + values["plugins"]["shape"]["bundled"]["summarize"]["api_key"], + "saved-key" + ); + assert_eq!( + values["plugins"]["shape"]["bundled"]["summarize"]["system_prompt"], + " Custom prompt\nwith two lines.\n" + ); + assert_eq!( + values["plugins"]["classify"]["bundled"]["hide"], + json!(["test"]) + ); + assert_eq!( + values["plugins"]["classify"]["bundled"]["hide_deleted"], + false + ); + assert_eq!(values["diff"]["byte_limit"], 123456); + assert!(text.contains("# keep this")); + assert!(!migrate(&path).unwrap().changed); + assert_eq!(std::fs::read_to_string(&path).unwrap(), text); + } + + #[test] + fn disabled_hide_and_explicit_classifier_values_are_retained() { + for (source, tags, deleted) in [ + ("[plugins.bundled.hide-files]\nenabled = false\n", json!([]), false), + ("[classifier]\npath = 'missing'\nhide = ['team']\nhide_deleted = true\n[plugins.bundled.hide-files]\nenabled = false\n", json!(["team"]), true), + ] { + let (_dir, path) = file(source); + migrate(&path).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + let values = store::show(&Config::from_toml(&text).unwrap(), false); + assert_eq!(values["plugins"]["classify"]["bundled"]["hide"], tags); + assert_eq!(values["plugins"]["classify"]["bundled"]["hide_deleted"], deleted); + } + } + + #[test] + fn stock_prompts_follow_the_new_default() { + for prompt in STOCK_PROMPTS { + let (_dir, path) = file(&format!( + "[plugins.bundled.summarize]\nsystem_prompt = {prompt:?}\n" + )); + migrate(&path).unwrap(); + assert!(!std::fs::read_to_string(path) + .unwrap() + .contains("system_prompt")); + } + } + + #[test] + fn errors_preserve_the_file_and_do_not_echo_credentials() { + for source in [ + "version = 9\n", + "version = 1\n[plugins.bundled.summarize]\napi_key = 'private-key'\nprovider = 'unknown'\n", + "[plugins.bundled.context]\nlines = -1\n", + "api_key = 'private-key\n", + ] { + let (_dir, path) = file(source); + let error = migrate(&path).unwrap_err().to_string(); + assert!(!error.contains("private-key"), "{error}"); + assert_eq!(std::fs::read_to_string(path).unwrap(), source); + } + let dir = tempfile::tempdir().unwrap(); + assert!(!migrate(&dir.path().join("absent.toml")).unwrap().changed); + } +} diff --git a/crates/diffr-core/src/config/prune/tests.rs b/crates/diffr-core/src/config/prune/tests.rs index 2ba72141e..4490f34cc 100644 --- a/crates/diffr-core/src/config/prune/tests.rs +++ b/crates/diffr-core/src/config/prune/tests.rs @@ -137,7 +137,7 @@ fn every_setting_is_dropped_at_its_default_and_kept_otherwise() { if let Some(value) = other(node) { let path = dir.path().join("other.toml"); match store::set(&path, key, &text(&value)) { - Ok(()) => { + Ok(_) => { assert_eq!( file_keys(&path), BTreeSet::from(["version".to_owned(), key.clone()]), diff --git a/crates/diffr-core/src/config/store.rs b/crates/diffr-core/src/config/store.rs index 94957243e..42704fd32 100644 --- a/crates/diffr-core/src/config/store.rs +++ b/crates/diffr-core/src/config/store.rs @@ -34,20 +34,146 @@ pub fn redacted(config: &Config, reveal: bool) -> Config { /// [`typed_value`]), then the whole file is validated, before anything /// touches the disk: unknown keys, text that is not the key's type, and /// values the configuration rejects are errors. -pub fn set(path: &Path, key: &str, value: &str) -> Result<(), ConfigError> { +pub fn set(path: &Path, key: &str, value: &str) -> Result { if key.is_empty() || key.split('.').any(str::is_empty) { return Err(ConfigError(format!("invalid key {key:?}"))); } - let existing = read(path)?; - let directory = directory_of(path); - let typed = typed_value(key, &setting_schema(key, &existing, directory)?, value)?; - let mut document: toml_edit::DocumentMut = existing - .parse() - .map_err(|error| ConfigError(format!("{}: {error}", path.display())))?; - reset(&mut document, key, &typed, &existing, directory)?; - assign(&mut document, key, &typed)?; + edit(path, |existing, directory| { + let typed = typed_value(key, &setting_schema(key, existing, directory)?, value)?; + let parts: Vec<_> = key.split('.').collect(); + apply(existing, directory, &[(parts, typed)]) + }) +} + +/// Apply a typed partial configuration. Resets run against the old values +/// before any patch values are assigned, so a new provider and its key can +/// be saved together. Omitted fields are retained; null is not a TOML value. +pub fn patch(path: &Path, patch: &Value) -> Result { + let table = patch + .as_object() + .ok_or_else(|| ConfigError("config patch must be an object".into()))?; + let mut entries = Vec::new(); + flatten(table, &mut Vec::new(), &mut entries)?; + edit(path, |existing, directory| { + let entries: Vec<_> = entries + .iter() + .map(|(parts, value)| (parts.iter().map(String::as_str).collect(), value.clone())) + .collect(); + let candidate = apply(existing, directory, &entries)?; + let before = Config::from_toml_in(existing, directory)?; + let after = Config::from_toml_in(&candidate.to_string(), directory)?; + if show(&before, true) == show(&after, true) { + parse(existing) + } else { + Ok(candidate) + } + }) +} + +fn flatten( + table: &serde_json::Map, + path: &mut Vec, + entries: &mut Vec<(Vec, toml::Value)>, +) -> Result<(), ConfigError> { + for (key, value) in table { + if key.is_empty() { + return Err(ConfigError("config patch contains an empty key".into())); + } + path.push(key.clone()); + let parts: Vec<_> = path.iter().map(String::as_str).collect(); + if let Some(table) = value.as_object().filter(|_| { + plugin_setting(&parts).is_none() + || matches!(parts.as_slice(), ["plugins", "shape", "bundled", _]) + }) { + if table.is_empty() { + return Err(ConfigError(format!( + "{}: empty patch table", + path.join(".") + ))); + } + flatten(table, path, entries)?; + } else { + let value = toml::Value::try_from(value) + .map_err(|_| ConfigError(format!("{}: not a TOML value", path.join("."))))?; + entries.push((path.clone(), value)); + } + path.pop(); + } + Ok(()) +} + +fn apply( + existing: &str, + directory: &Path, + entries: &[(Vec<&str>, toml::Value)], +) -> Result { + let mut document = parse(existing)?; + for (parts, value) in entries { + reset(&mut document, parts, value, existing, directory)?; + } + for (parts, value) in entries { + assign_parts(&mut document, parts, value)?; + } prune(&mut document, directory)?; - write(path, document) + Ok(document) +} + +pub(super) fn parse(source: &str) -> Result { + source.parse::().map_err(|error| { + // TOML's Display includes the source line, which may contain a key. + let line = error.span().map(|span| { + source.as_bytes()[..span.start] + .iter() + .filter(|byte| **byte == b'\n') + .count() + + 1 + }); + ConfigError(match line { + Some(line) => format!("invalid TOML at line {line}: {}", error.message()), + None => format!("invalid TOML: {}", error.message()), + }) + }) +} + +/// Serialize native writers with a file lock, and reject outside edits made +/// while the candidate was being built. The candidate is validated in memory. +pub(super) fn edit( + path: &Path, + convert: impl FnOnce(&str, &Path) -> Result, +) -> Result { + let parent = directory_of(path); + std::fs::create_dir_all(parent) + .map_err(|error| ConfigError(format!("{}: {error}", parent.display())))?; + let path = if path.exists() { + std::fs::canonicalize(path) + } else { + std::fs::canonicalize(parent) + .map(|parent| parent.join(path.file_name().expect("config filename"))) + } + .map_err(|error| ConfigError(format!("{}: {error}", path.display())))?; + let lock_path = path.with_extension("toml.lock"); + let lock = std::fs::OpenOptions::new() + .read(true) + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + .map_err(|error| ConfigError(format!("{}: {error}", lock_path.display())))?; + lock.lock() + .map_err(|error| ConfigError(format!("{}: {error}", lock_path.display())))?; + let existing = read(&path)?; + let document = convert(&existing, directory_of(&path))?; + let candidate = document.to_string(); + if candidate == existing { + return Ok(false); + } + if read(&path)? != existing { + return Err(ConfigError( + "config changed while preparing the update; retry".into(), + )); + } + write(&path, document)?; + Ok(true) } fn read(path: &Path) -> Result { @@ -62,13 +188,12 @@ fn read(path: &Path) -> Result { /// belong to it: a provider's key, endpoint and model go with the provider. fn reset( document: &mut toml_edit::DocumentMut, - key: &str, + segments: &[&str], value: &toml::Value, existing: &str, directory: &Path, ) -> Result<(), ConfigError> { - let segments: Vec<_> = key.split('.').collect(); - let Some(("shape", name, field)) = plugin_setting(&segments) else { + let Some(("shape", name, field)) = plugin_setting(segments) else { return Ok(()); }; let config = Config::from_toml_in(existing, directory)?; @@ -104,12 +229,17 @@ fn prune(document: &mut toml_edit::DocumentMut, directory: &Path) -> Result<(), } fn write(path: &Path, document: toml_edit::DocumentMut) -> Result<(), ConfigError> { - if let Some(parent) = path.parent() { - std::fs::create_dir_all(parent) - .map_err(|error| ConfigError(format!("{}: {error}", parent.display())))?; - } - std::fs::write(path, document.to_string()) - .map_err(|error| ConfigError(format!("{}: {error}", path.display()))) + use std::io::Write; + let mut temporary = tempfile::NamedTempFile::new_in(directory_of(path)) + .map_err(|error| ConfigError(format!("{}: {error}", path.display())))?; + temporary + .write_all(document.to_string().as_bytes()) + .and_then(|_| temporary.as_file().sync_all()) + .map_err(|error| ConfigError(format!("{}: {error}", path.display())))?; + temporary + .persist(path) + .map_err(|error| ConfigError(format!("{}: {}", path.display(), error.error)))?; + Ok(()) } /// The JSON Schema of the setting at the dotted `key`, as `diffr config @@ -246,24 +376,27 @@ fn plugin_setting<'a>(parts: &'a [&str]) -> Option<(&'a str, String, &'a str)> { } } -fn assign( +fn assign_parts( document: &mut toml_edit::DocumentMut, - key: &str, + parts: &[&str], value: &toml::Value, ) -> Result<(), ConfigError> { - let mut segments = key.split('.').peekable(); + insert(document, parts, toml_edit::Item::Value(edit_value(value)?)) +} + +pub(super) fn insert( + document: &mut toml_edit::DocumentMut, + parts: &[&str], + value: toml_edit::Item, +) -> Result<(), ConfigError> { + let (last, parents) = parts + .split_last() + .ok_or_else(|| ConfigError("empty config key".into()))?; let mut item = document.as_item_mut(); - while let Some(segment) = segments.next() { - if segments.peek().is_none() { - match item.as_table_like_mut() { - Some(table) => table.insert(segment, toml_edit::Item::Value(edit_value(value)?)), - None => return Err(ConfigError(format!("{key}: parent is not a table"))), - }; - return Ok(()); - } + for segment in parents { let table = item .as_table_like_mut() - .ok_or_else(|| ConfigError(format!("{key}: parent is not a table")))?; + .ok_or_else(|| ConfigError(format!("{}: parent is not a table", parts.join("."))))?; if !table.contains_key(segment) { let mut nested = toml_edit::Table::new(); nested.set_implicit(true); @@ -271,7 +404,10 @@ fn assign( } item = table.get_mut(segment).expect("just inserted"); } - Err(ConfigError(format!("invalid key {key:?}"))) + item.as_table_like_mut() + .ok_or_else(|| ConfigError(format!("{}: parent is not a table", parts.join("."))))? + .insert(last, value); + Ok(()) } fn edit_value(value: &toml::Value) -> Result { @@ -287,7 +423,14 @@ fn edit_value(value: &toml::Value) -> Result { } toml_edit::Value::Array(array) } - toml::Value::Table(_) | toml::Value::Datetime(_) => { + toml::Value::Table(table) => { + let mut result = toml_edit::InlineTable::new(); + for (key, value) in table { + result.insert(key, edit_value(value)?); + } + toml_edit::Value::InlineTable(result) + } + toml::Value::Datetime(_) => { return Err(ConfigError( "config set takes a scalar or array value".into(), )); @@ -777,3 +920,73 @@ mod sparse_tests { assert!(!text.contains("deleted-bodies")); } } + +#[cfg(test)] +mod patch_tests { + use super::*; + + #[test] + fn provider_reset_precedes_explicit_batch_values() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write(&path, "version = 2\n[plugins.shape.bundled.summarize]\nprovider = 'gemini'\napi_key = 'old-key'\nmodel = 'old-model'\nendpoint = 'old-endpoint'\n").unwrap(); + patch(&path, &json!({"plugins": {"shape": {"bundled": {"summarize": {"enabled": true, "provider": "openai", "api_key": "new-key", "model": "new-model"}}}}})).unwrap(); + let text = std::fs::read_to_string(&path).unwrap(); + let values = show(&Config::from_toml(&text).unwrap(), true); + let summary = &values["plugins"]["shape"]["bundled"]["summarize"]; + assert_eq!(summary["api_key"], "new-key"); + assert_eq!(summary["model"], "new-model"); + assert_eq!(summary["enabled"], true); + assert!(!text.contains("old-endpoint")); + } + + #[test] + fn a_rejected_batch_saves_nothing() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + let original = "version = 2\n[plugins.shape.bundled.context]\nlines = 8 # saved value\n"; + std::fs::write(&path, original).unwrap(); + for patch_value in [ + json!({"plugins": {"shape": {"bundled": {"summarize": {"api_key": "private-key"}, "context": {"lines": -1}}}}}), + json!({"diff": {"graph_limit": "wrong-type"}}), + json!({"diff": {"unknown": true}}), + json!({"diff": {"graph_limit": null}}), + ] { + assert!(patch(&path, &patch_value).is_err()); + assert_eq!(std::fs::read_to_string(&path).unwrap(), original); + } + } +} + +#[cfg(test)] +mod concurrent_writes { + use super::*; + + #[test] + fn separate_writers_keep_both_updates() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("config.toml"); + std::fs::write(&path, "version = 2\n").unwrap(); + let barrier = std::sync::Barrier::new(2); + std::thread::scope(|scope| { + for patch_value in [ + json!({"diff": {"graph_limit": 1234}}), + json!({"plugins": {"shape": {"bundled": {"context": {"lines": 17}}}}}), + ] { + let path = &path; + let barrier = &barrier; + scope.spawn(move || { + barrier.wait(); + patch(path, &patch_value).unwrap(); + }); + } + }); + let text = std::fs::read_to_string(&path).unwrap(); + let values = show(&Config::from_toml(&text).unwrap(), false); + assert_eq!(values["diff"]["graph_limit"], 1234); + assert_eq!( + values["plugins"]["shape"]["bundled"]["context"]["lines"], + 17 + ); + } +} diff --git a/src/cli.rs b/src/cli.rs index 52d1b6514..1f4e9ef4f 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -159,7 +159,27 @@ enum ConfigCommand { reveal: bool, }, /// Write one key to the global configuration file - Set { key: String, value: String }, + Set { + #[arg(required_unless_present = "stdin", conflicts_with = "stdin")] + key: Option, + #[arg( + requires = "key", + required_unless_present = "stdin", + conflicts_with = "stdin" + )] + value: Option, + /// Read a typed partial configuration as JSON from stdin + #[arg(long)] + stdin: bool, + /// Print a JSON result, including errors + #[arg(long)] + json: bool, + }, + /// Convert the v1 settings supported by Whiteboard to config version 2 + Migrate { + #[arg(long)] + json: bool, + }, } pub(crate) fn run(runtime: &tokio::runtime::Runtime) -> Result { @@ -644,8 +664,33 @@ fn run_config(config: &ConfigArgs) -> Result { )?; } } - Some(ConfigCommand::Set { key, value }) => { - config::store::set(&config::global_path()?, key, value)?; + Some(ConfigCommand::Set { + key, + value, + stdin, + json, + }) => { + let result = (|| -> Result { + let path = config::global_path()?; + let changed = if *stdin { + let patch: serde_json::Value = serde_json::from_reader(io::stdin().lock())?; + config::store::patch(&path, &patch)? + } else { + config::store::set( + &path, + key.as_deref().expect("clap requires key"), + value.as_deref().expect("clap requires value"), + )? + }; + Ok(serde_json::json!({ "changed": changed })) + })(); + return config_result(&mut stdout, result, *json); + } + Some(ConfigCommand::Migrate { json }) => { + let result = config::global_path() + .and_then(|path| config::migrate::migrate(&path)) + .map(|migration| serde_json::to_value(migration).expect("migration serializes")); + return config_result(&mut stdout, result.map_err(Into::into), *json); } None => { let mut frontend = vec![OsString::from("--settings")]; @@ -658,6 +703,35 @@ fn run_config(config: &ConfigArgs) -> Result { Ok(0) } +fn config_result( + stdout: &mut impl Write, + result: Result, + json: bool, +) -> Result { + match result { + Ok(value) => { + if json { + serde_json::to_writer(stdout.by_ref(), &value)?; + stdout.write_all(b"\n")?; + } + Ok(0) + } + Err(error) if json => { + let message = error.to_string(); + let file = config::global_path().ok(); + let failure = serde_json::json!({ "error": { + "message": message, + "path": file, + "repair_prompt": format!("diffr could not update my config. The original file was kept.\nConfig file: {}\nProblem: {message}\nRead the config, fix the reported problem, and retry the command. Preserve my settings, custom prompts, and credentials. Do not print credentials or reset the config.", file.as_ref().map(|path| path.display().to_string()).unwrap_or_default()) + }}); + serde_json::to_writer(stdout.by_ref(), &failure)?; + stdout.write_all(b"\n")?; + Ok(2) + } + Err(error) => Err(error), + } +} + /// `comparison` passes the arguments after `--` as the comparison to open; /// otherwise they are frontend flags such as `--settings`. fn launch_tui(args: &[OsString], comparison: bool) -> Result { diff --git a/tests/cli.rs b/tests/cli.rs index 088abed62..21ea15ac9 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -188,3 +188,73 @@ fn debug_needs_exactly_one_action() { .code(2) .stderr(predicate::str::contains("cannot be used with")); } + +#[test] +fn config_migration_and_typed_batch_set() { + use std::io::Write; + use std::process::Stdio; + let dir = tempfile::tempdir().unwrap(); + let file = dir.path().join("diffr/config.toml"); + std::fs::create_dir_all(file.parent().unwrap()).unwrap(); + std::fs::write( + &file, + "version = 1\n[plugins.bundled.summarize]\ninstances = 3\nsystem_prompt = 'My prompt'\n", + ) + .unwrap(); + let mut command = get_base_command(); + command + .env("XDG_CONFIG_HOME", dir.path()) + .args(["config", "migrate", "--json"]); + let output = command.output().unwrap(); + assert!( + output.status.success(), + "{}", + String::from_utf8_lossy(&output.stderr) + ); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert_eq!(result["changed"], true); + assert!(result["dropped"] + .as_array() + .unwrap() + .contains(&serde_json::json!("plugins.bundled.summarize.instances"))); + + let original = std::fs::read_to_string(&file).unwrap(); + for (patch, success) in [ + ( + r#"{"plugins":{"shape":{"bundled":{"summarize":{"provider":"openai","api_key":"new-key","model":"new-model"}}}}}"#, + true, + ), + ( + r#"{"plugins":{"shape":{"bundled":{"context":{"lines":-1},"summarize":{"api_key":"private-key"}}}}}"#, + false, + ), + ] { + let before = std::fs::read_to_string(&file).unwrap(); + let mut command = get_base_command(); + let mut child = command + .env("XDG_CONFIG_HOME", dir.path()) + .args(["config", "set", "--stdin", "--json"]) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(patch.as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert_eq!(output.status.success(), success); + let result: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap(); + assert!(!String::from_utf8_lossy(&output.stdout).contains("private-key")); + if success { + assert_eq!(result["changed"], true); + } else { + assert_eq!(std::fs::read_to_string(&file).unwrap(), before); + assert!(result["error"]["repair_prompt"].is_string()); + } + } + assert!(original.contains("My prompt")); + assert!(std::fs::read_to_string(file).unwrap().contains("My prompt")); +}