diff --git a/src/config.rs b/src/config.rs index b2843dcc..2915f00a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -17,7 +17,7 @@ use toml_edit::{DocumentMut, Item}; use crate::harness::{self, Harness, ModelAndEffort}; mod console; -pub use console::ConsoleSettings; +pub use console::{ConsoleAccess, ConsoleSettings}; /// The settings a User config can hold. Each is what a Run does when its /// command says nothing about it. @@ -289,6 +289,8 @@ pub struct UserConfigDocument { /// The Setup settings to change. Other settings keep their current values. /// Credentials and the decision to send a test email stay with Setup. pub struct UserConfigChanges { + /// None retains all Console settings. + pub console: Option, pub merge_always: bool, pub base_fix: bool, pub launch_pull: bool, @@ -340,6 +342,18 @@ impl UserConfigDocument { pub fn render(self, changes: Option) -> String { let mut document = self.document; if let Some(changes) = changes { + if let Some(console) = changes.console { + set( + &mut document, + "console", + "address", + console.address.as_str(), + ); + if console.allowed_emails != self.settings.console.allowed_emails { + let emails: toml_edit::Array = console.allowed_emails.iter().collect(); + set(&mut document, "console", "allowed_emails", emails); + } + } set(&mut document, "merge", "always", changes.merge_always); set(&mut document, "base", "fix", changes.base_fix); set(&mut document, "launch", "pull", changes.launch_pull); @@ -464,6 +478,10 @@ fn set(document: &mut DocumentMut, section: &str, key: &str, value: impl Into value.as_bool() == Some(old), (_, Some(old)) => value.as_str() == Some(old), @@ -488,6 +506,45 @@ fn set(document: &mut DocumentMut, section: &str, key: &str, value: impl Into replacement.len() { + let removed = existing.remove(replacement.len()); + removed_decor.push_str(&decor_prefix(removed.decor())); + removed_decor.push_str(&decor_suffix(removed.decor())); + } + if removed_decor.contains('#') { + removed_decor.push_str(existing.trailing().as_str().unwrap_or("")); + existing.set_trailing(removed_decor); + existing.set_trailing_comma(!existing.is_empty()); + } +} + /// Set `email.to` in `document` to `to`. With no `email.to` there yet, it /// takes the place of the commented-out one, if there is one, written as /// the defaults would write it, with its comment. @@ -1365,6 +1422,7 @@ mod tests { assert_eq!(current.logs_dir, home.join(".thirdshift/logs")); let text = document.render(Some(UserConfigChanges { + console: None, merge_always: true, base_fix: true, launch_pull: true, @@ -1519,6 +1577,7 @@ mod tests { fn notifications_to(to: &str) -> UserConfigChanges { UserConfigChanges { + console: None, harness: None, merge_always: false, base_fix: false, @@ -1614,6 +1673,7 @@ mod tests { let text = "# top\n[merge]\n# before\nalways = false # merge\n# after\n\n\ [harness.codex]\nmodel = 'old' # model\neffort = 'high' # effort\n"; let changes = UserConfigChanges { + console: None, merge_always: true, base_fix: false, launch_pull: false, @@ -1659,6 +1719,7 @@ mod tests { ] { let before = parse(text).unwrap(); let changes = UserConfigChanges { + console: None, merge_always: true, base_fix: true, launch_pull: true, @@ -1745,6 +1806,7 @@ mod tests { } let before = parse(&text).unwrap(); let changes = UserConfigChanges { + console: None, merge_always: true, base_fix: false, launch_pull: true, diff --git a/src/config/console.rs b/src/config/console.rs index 85aea889..7056f08a 100644 --- a/src/config/console.rs +++ b/src/config/console.rs @@ -2,6 +2,13 @@ use anyhow::{Result, ensure}; use serde::Deserialize; +/// The Console settings Setup asks about; other Console settings stay put. +#[derive(Clone)] +pub struct ConsoleAccess { + pub address: String, + pub allowed_emails: Vec, +} + #[derive(Debug, PartialEq, Eq, Deserialize)] #[serde(default, deny_unknown_fields)] pub struct ConsoleSettings { @@ -49,7 +56,7 @@ impl ConsoleSettings { Ok(host) } - pub(super) fn validate(&self) -> Result<()> { + pub(crate) fn validate(&self) -> Result<()> { if !self.address.is_empty() { self.hostname()?; } diff --git a/src/setup.rs b/src/setup.rs index 991ac553..90d351c6 100644 --- a/src/setup.rs +++ b/src/setup.rs @@ -12,6 +12,9 @@ pub(crate) mod questions; +#[cfg(test)] +mod console_tests; + use crate::terminal::EchoOff; use std::io::{IsTerminal, Write}; use std::path::Path; @@ -105,6 +108,7 @@ fn run_setup(outside: &mut impl Outside, home: &Path, path: &Path) -> Result Result<()> return Ok(()); }; outside.step(format!("wrote the User config {}", path.display())); + offer_console_install(outside, answers); if let Some(key) = answers.key() { match outside.save_key(key) { Ok(wrote) => outside.step(wrote), @@ -164,6 +169,14 @@ fn ask_if( outside: &mut impl Outside, document: UserConfigDocument, ) -> Result<(String, Option)> { + let settings = document.settings(); + let waiting = settings.effective_waiting_line_interval(); + if waiting != settings.waiting_line_interval { + outside.say(format!( + "warning: logs.waiting_line_minutes must be less than inbox.silent_minutes; using half the silent time ({} sec) for waiting lines", + waiting.as_secs() + )); + } let answers = if asking { let found = outside.find_key()?; Some(questions::ask(outside, document.settings(), found)?) @@ -174,6 +187,13 @@ fn ask_if( Ok((document.render(changes), answers)) } +/// Installation is an owner-run command that prints the script for sudo. +fn offer_console_install(outside: &mut impl Outside, answers: &Answers) { + if answers.console.is_some() { + outside.say("To install the Console, run `thirdshift console install`; it prints a script to run with sudo.".into()); + } +} + /// Write `text` as the User config at `path`, where there is none yet. fn write_new(outside: &mut impl Outside, path: &Path, text: &str) -> Result<()> { outside @@ -639,6 +659,7 @@ mod tests { const SECURITY_REVIEW: &str = "Runs review their change for Security findings?"; const SECURITY_FIX: &str = "Security runs may fix reproduced findings?"; const NOTIFY: &str = "Run notifications, an email"; + const CONSOLE: &str = "Run the Console?"; const TO: &str = "Send Run notifications to"; const FROM: &str = "Send them from"; const TEST_EMAIL: &str = "Send a test email now? [y/N] "; @@ -652,12 +673,13 @@ mod tests { const KEY: &str = "re_secret_123"; /// The answers that take every default and turn nothing on. - const ENTER_THROUGHOUT: [(&str, &str); 5] = [ + const ENTER_THROUGHOUT: [(&str, &str); 6] = [ (MERGE, ""), (PULL, ""), (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, ""), ]; /// The answers that turn Run notifications on, to `me@example.com` from @@ -719,6 +741,84 @@ mod tests { // What Setup writes. + #[test] + fn setup_warns_when_waiting_lines_could_read_as_silent() { + for terminal in [false, true] { + for (waiting, warns) in [(1, false), (2, true), (3, true)] { + let mut outside = Scripted { + terminal, + user_config: Some(format!( + "[logs]\nwaiting_line_minutes = {waiting}\n[inbox]\nsilent_minutes = 2\n" + )), + ..Scripted::answering(if terminal { &ENTER_THROUGHOUT } else { &[] }) + }; + setup(&mut outside).unwrap(); + let warning = outside.said().join("\n"); + assert_eq!( + warning.contains( + "logs.waiting_line_minutes must be less than inbox.silent_minutes" + ), + warns, + "{warning}" + ); + if warns { + assert!( + warning.contains("using half the silent time (60 sec)"), + "{warning}" + ); + } + assert_eq!( + table(&outside)["logs"]["waiting_line_minutes"].as_integer(), + Some(waiting) + ); + } + } + } + + #[test] + fn console_answers_are_written_with_comments_and_install_is_offered() { + let mut answers = ENTER_THROUGHOUT[..5].to_vec(); + answers.extend([ + ("Run the Console?", "y"), + ("Console's whole address", "https://thirdshift.example.com"), + ("Allowed emails", "me@example.com, other@example.net"), + ]); + let mut outside = Scripted { + user_config: Some("[console] # my Console\naddress = '' # site\nallowed_emails = [] # people\nport = 9000 # local\n".into()), + ..Scripted::answering(&answers) + }; + + setup(&mut outside).unwrap(); + + let config = table(&outside); + assert_eq!( + config["console"]["address"].as_str(), + Some("https://thirdshift.example.com") + ); + assert_eq!( + config["console"]["allowed_emails"].as_array().unwrap(), + &[ + toml::Value::from("me@example.com"), + toml::Value::from("other@example.net") + ] + ); + let text = outside.user_config.as_deref().unwrap(); + for comment in [ + "[console] # my Console", + "# site", + "# people", + "port = 9000 # local", + ] { + assert!(text.contains(comment), "{text}"); + } + assert!( + outside + .said() + .iter() + .any(|line| line.contains("thirdshift console install")) + ); + } + #[test] fn the_harness_interaction_receives_every_current_setting_once_before_other_questions() { let mine = "[harness]\ndefault = 'codex'\n[harness.claude]\nmodel = 'opus'\neffort = 'high'\n[harness.codex]\nmodel = 'GPT-6-Luna'\neffort = 'High'\n[harness.agy]\nmodel = 'gemini-3.8-flash'\neffort = 'medium'\n[harness.grok]\nmodel = 'grok-4.5'\neffort = 'low'\n[harness.muse]\nmodel = 'muse-spark-1.3'\neffort = 'high'\n[harness.opencode]\nmodel = 'provider/model'\neffort = 'high'\n"; @@ -971,6 +1071,7 @@ to = \"me@example.com\" # my inbox "Security runs may fix reproduced findings? [y/N] ", "Runs review their change for Security findings? [y/N] ", "Run notifications, an email as each Run ends? [y/N] ", + "Run the Console? [y/N] ", ] ); } @@ -989,6 +1090,7 @@ to = \"me@example.com\" # my inbox (TO, "me@example.com"), (FROM, "ts@acme.dev"), (TEST_EMAIL, ""), + (CONSOLE, "n"), ]) }; @@ -1112,6 +1214,7 @@ effort = \"\" (TO, ""), (FROM, ""), (TEST_EMAIL, ""), + (CONSOLE, "n"), ]) }; @@ -1134,6 +1237,7 @@ effort = \"\" "Send Run notifications to [mine@example.net]: ", "Send them from [ts@acme.dev]: ", "Send a test email now? [y/N] ", + "Run the Console? [y/N] ", ] ); } @@ -1148,6 +1252,7 @@ effort = \"\" (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]) }; @@ -1185,6 +1290,7 @@ always = false # quiet, please (TO, "me@example.com"), (FROM, ""), (KEY_PROMPT, ""), + (CONSOLE, "n"), ]) }; @@ -1205,8 +1311,11 @@ always = false # quiet, please #[test] fn the_credentials_are_saved_after_the_user_config_and_the_test_email_goes_last() { - let mut outside = - Scripted::answering(¬ifications_on(&[(KEY_PROMPT, KEY), (TEST_EMAIL, "y")])); + let mut outside = Scripted::answering(¬ifications_on(&[ + (KEY_PROMPT, KEY), + (TEST_EMAIL, "y"), + (CONSOLE, "n"), + ])); let done = setup(&mut outside).unwrap(); @@ -1228,8 +1337,11 @@ always = false # quiet, please #[test] fn an_entered_key_alone_ends_with_the_line_saying_where_it_was_saved() { - let mut outside = - Scripted::answering(¬ifications_on(&[(KEY_PROMPT, KEY), (TEST_EMAIL, "n")])); + let mut outside = Scripted::answering(¬ifications_on(&[ + (KEY_PROMPT, KEY), + (TEST_EMAIL, "n"), + (CONSOLE, "n"), + ])); let done = setup(&mut outside).unwrap(); @@ -1289,6 +1401,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]); setup(&mut outside).unwrap(); @@ -1315,6 +1428,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]); setup(&mut outside).unwrap(); @@ -1336,6 +1450,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]) }; @@ -1362,6 +1477,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]) }; @@ -1382,6 +1498,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]); setup(&mut outside).unwrap(); @@ -1402,6 +1519,7 @@ always = false # quiet, please (TO, "me@example.com"), (FROM, ""), (KEY_PROMPT, ""), + (CONSOLE, "n"), ]); setup(&mut outside).unwrap(); @@ -1430,6 +1548,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, "n"), + (CONSOLE, "n"), ]) }; @@ -1448,7 +1567,8 @@ always = false # quiet, please #[test] fn with_no_key_anywhere_skipping_it_saves_nothing_offers_no_test_email_and_says_how_to_add_one() { - let mut outside = Scripted::answering(¬ifications_on(&[(KEY_PROMPT, "")])); + let mut outside = + Scripted::answering(¬ifications_on(&[(KEY_PROMPT, ""), (CONSOLE, "n")])); let done = setup(&mut outside).unwrap(); @@ -1465,6 +1585,7 @@ always = false # quiet, please (KEY_PROMPT, "sk_not_resend"), (KEY_PROMPT, KEY), (TEST_EMAIL, ""), + (CONSOLE, "n"), ])); setup(&mut outside).unwrap(); @@ -1491,7 +1612,11 @@ always = false # quiet, please fn with_a_saved_key_pressing_enter_keeps_it() { let mut outside = Scripted { key: Ok(Some(Source::Credentials(CREDENTIALS.into()))), - ..Scripted::answering(¬ifications_on(&[(KEPT, ""), (TEST_EMAIL, "")])) + ..Scripted::answering(¬ifications_on(&[ + (KEPT, ""), + (TEST_EMAIL, ""), + (CONSOLE, "n"), + ])) }; setup(&mut outside).unwrap(); @@ -1509,7 +1634,11 @@ always = false # quiet, please fn with_a_saved_key_a_new_one_replaces_it() { let mut outside = Scripted { key: Ok(Some(Source::Credentials(CREDENTIALS.into()))), - ..Scripted::answering(¬ifications_on(&[(KEPT, KEY), (TEST_EMAIL, "")])) + ..Scripted::answering(¬ifications_on(&[ + (KEPT, KEY), + (TEST_EMAIL, ""), + (CONSOLE, "n"), + ])) }; setup(&mut outside).unwrap(); @@ -1521,7 +1650,7 @@ always = false # quiet, please fn with_resend_api_key_set_no_key_is_asked_and_its_source_is_said() { let mut outside = Scripted { key: Ok(Some(Source::Environment)), - ..Scripted::answering(¬ifications_on(&[(TEST_EMAIL, "")])) + ..Scripted::answering(¬ifications_on(&[(TEST_EMAIL, ""), (CONSOLE, "n")])) }; setup(&mut outside).unwrap(); @@ -1540,7 +1669,7 @@ always = false # quiet, please for (answer, sent) in [("y", true), ("n", false), ("", false)] { let mut outside = Scripted { key: Ok(Some(Source::Environment)), - ..Scripted::answering(¬ifications_on(&[(TEST_EMAIL, answer)])) + ..Scripted::answering(¬ifications_on(&[(TEST_EMAIL, answer), (CONSOLE, "n")])) }; let done = setup(&mut outside).unwrap(); @@ -1678,6 +1807,7 @@ always = false # quiet, please (TO, ""), (FROM, ""), (KEY_PROMPT, ""), + (CONSOLE, "n"), ]) }; @@ -1777,6 +1907,7 @@ always = false # quiet, please (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ]); offer(&mut outside).unwrap(); @@ -1801,6 +1932,7 @@ always = false # quiet, please (FROM, ""), (KEY_PROMPT, KEY), (TEST_EMAIL, "y"), + (CONSOLE, "n"), ]); offer(&mut outside).unwrap(); @@ -1824,7 +1956,11 @@ always = false # quiet, please fn an_unwritable_user_config_is_a_warning_and_the_run_carries_on_with_the_defaults() { let mut outside = Scripted { failing: vec![Fails::WriteNew], - ..Scripted::answering(¬ifications_on(&[(KEY_PROMPT, KEY), (TEST_EMAIL, "y")])) + ..Scripted::answering(¬ifications_on(&[ + (KEY_PROMPT, KEY), + (TEST_EMAIL, "y"), + (CONSOLE, "n"), + ])) }; outside.answers.push_front((OFFER, "y")); @@ -1844,7 +1980,11 @@ always = false # quiet, please fn credentials_that_cant_be_saved_or_a_test_email_that_cant_go_are_warnings() { let mut outside = Scripted { failing: vec![Fails::SaveKey, Fails::SendTest], - ..Scripted::answering(¬ifications_on(&[(KEY_PROMPT, KEY), (TEST_EMAIL, "y")])) + ..Scripted::answering(¬ifications_on(&[ + (KEY_PROMPT, KEY), + (TEST_EMAIL, "y"), + (CONSOLE, "n"), + ])) }; outside.answers.push_front((OFFER, "y")); diff --git a/src/setup/console_tests.rs b/src/setup/console_tests.rs new file mode 100644 index 00000000..6552d1fd --- /dev/null +++ b/src/setup/console_tests.rs @@ -0,0 +1,238 @@ +//! Console choices through Setup's existing outside-world script. +use super::scripted::{Call, Scripted}; +use super::*; + +const HOME: &str = "/home/me"; +const PATH: &str = "/home/me/.thirdshift/config.toml"; +const NO_NOTIFICATIONS: &[(&str, &str)] = &[ + ("Merge run?", ""), + ("fast-forwards", ""), + ("Security runs may fix", ""), + ("Runs review", ""), + ("Run notifications", "n"), +]; + +fn setup(outside: &mut Scripted) { + run_setup(outside, Path::new(HOME), Path::new(PATH)).unwrap(); + assert!(outside.answers.is_empty(), "{:?}", outside.answers); +} + +fn config(outside: &Scripted) -> UserConfig { + UserConfig::parse( + outside.user_config.as_deref().unwrap(), + Path::new(PATH), + Path::new(HOME), + ) + .unwrap() +} + +#[test] +fn console_allowlist_defaults_to_email_to_even_with_notifications_off() { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.extend([ + ("Run the Console?", "y"), + ("Console's whole address", "thirdshift.example.com"), + ("Allowed emails", ""), + ]); + let mut outside = Scripted { + user_config: Some("[email]\nto = 'owner@example.com'\n".into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + assert_eq!( + config(&outside).console.allowed_emails, + ["owner@example.com"] + ); + assert!( + outside + .prompts() + .iter() + .any(|prompt| prompt.contains("[owner@example.com]")) + ); +} + +#[test] +fn console_allowlist_defaults_to_the_notification_address_just_chosen() { + let mut answers = NO_NOTIFICATIONS[..4].to_vec(); + answers.extend([ + ("Run notifications", "y"), + ("Send Run notifications to", "new@example.com"), + ("Send them from", ""), + ("Resend API key", ""), + ("Run the Console?", "y"), + ("Console's whole address", "thirdshift.example.com"), + ("Allowed emails", ""), + ]); + let mut outside = Scripted { + user_config: Some("[email]\nto = 'old@example.com'\n".into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + assert_eq!(config(&outside).console.allowed_emails, ["new@example.com"]); +} + +#[test] +fn declining_console_retains_existing_access_and_offers_no_install() { + for saved in [ + "", + "[console]\naddress = 'saved.example.com' # site\nallowed_emails = ['saved@example.com'] # people\n", + ] { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.push(("Run the Console?", "n")); + let mut outside = Scripted { + user_config: Some(saved.into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + let before = UserConfig::parse(saved, Path::new(PATH), Path::new(HOME)).unwrap(); + assert_eq!(config(&outside).console, before.console); + assert!(outside.user_config.as_deref().unwrap().starts_with(saved)); + assert!( + !outside + .said() + .iter() + .any(|line| line.contains("console install")) + ); + assert!(!outside.prompts().iter().any(|prompt| prompt.contains("Allowed emails") || prompt.contains("whole address"))); + } +} + +#[test] +fn entering_existing_console_defaults_preserves_allowlist_layout_and_comments() { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.extend([ + ("Run the Console? [Y/n]", ""), + ("[https://saved.example.com]", ""), + ("[saved@example.com, other@example.com]", ""), + ]); + let saved = "[console] # Console\naddress = 'https://saved.example.com' # site\nallowed_emails = [\n 'saved@example.com', # owner\n 'other@example.com', # colleague\n] # people\n"; + let mut outside = Scripted { + user_config: Some(saved.into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + assert!(outside.user_config.as_deref().unwrap().starts_with(saved)); +} + +#[test] +fn console_reasks_invalid_addresses_and_allowlists_without_using_the_example_as_a_default() { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.extend([ + ("Run the Console?", "y"), + ("whole address (e.g. thirdshift.example.com):", ""), + ("whole address", "http://bad.example.com"), + ("whole address", "https://good.example.com/path"), + ("whole address", "https://good.example.com"), + ("Allowed emails", ""), + ("Allowed emails", "missing-at"), + ("Allowed emails", "bad @example.com"), + ("Allowed emails", "owner@example.com,"), + ("Allowed emails", "owner@example.com"), + ]); + let mut outside = Scripted::answering(&answers); + setup(&mut outside); + assert_eq!(config(&outside).console.address, "https://good.example.com"); + assert_eq!( + config(&outside).console.allowed_emails, + ["owner@example.com"] + ); +} + +#[test] +fn eof_during_console_questions_saves_no_config_credentials_or_test_email() { + let mut answers = NO_NOTIFICATIONS[..4].to_vec(); + answers.extend([ + ("Run notifications", "y"), + ("Send Run notifications to", "owner@example.com"), + ("Send them from", ""), + ("Resend API key", "re_secret"), + ("Send a test email", "y"), + ("Run the Console?", "y"), + ("whole address", "https://good.example.com"), + ]); + for end in answers.len() - 2..=answers.len() { + let mut outside = Scripted::answering(&answers[..end]); + let error = run_setup(&mut outside, Path::new(HOME), Path::new(PATH)).unwrap_err(); + assert!(error.to_string().contains("nothing written")); + assert!(outside.user_config.is_none()); + assert!( + !outside + .calls + .iter() + .any(|call| matches!(call, Call::SaveKey(_) | Call::SendTest { .. })) + ); + } +} + +#[test] +fn first_run_setup_also_saves_console_access_and_offers_install() { + let mut answers = vec![("Set your defaults now?", "y")]; + answers.extend_from_slice(NO_NOTIFICATIONS); + answers.extend([ + ("Run the Console?", "y"), + ("whole address", "thirdshift.example.com"), + ("Allowed emails", "owner@example.com"), + ]); + let mut outside = Scripted::answering(&answers); + run_offer(&mut outside, Path::new(HOME), Path::new(PATH)).unwrap(); + assert!(outside.answers.is_empty()); + assert_eq!(config(&outside).console.address, "thirdshift.example.com"); + assert!( + outside + .said() + .iter() + .any(|line| line.contains("thirdshift console install")) + ); +} + +#[test] +fn review_changed_console_allowlist_keeps_entry_comments() { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.extend([ + ("Run the Console?", "y"), + ("Console's whole address", ""), + ("Allowed emails", "saved@example.com, added@example.net"), + ]); + let saved = "[console] # Console\naddress = 'https://saved.example.com' # site\nallowed_emails = [\n 'saved@example.com', # owner retained\n] # people\n"; + let mut outside = Scripted { + user_config: Some(saved.into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + assert_eq!( + config(&outside).console.allowed_emails, + ["saved@example.com", "added@example.net"] + ); + let text = outside.user_config.as_deref().unwrap(); + for comment in ["# Console", "# site", "# owner retained", "# people"] { + assert!(text.contains(comment), "lost {comment}:\n{text}"); + } +} + +#[test] +fn replacing_or_removing_allowlist_entries_keeps_the_comments() { + for (answer, expected) in [ + ( + "new@example.com, other@example.com", + vec!["new@example.com", "other@example.com"], + ), + ("saved@example.com", vec!["saved@example.com"]), + ] { + let mut answers = NO_NOTIFICATIONS.to_vec(); + answers.extend([ + ("Run the Console?", "y"), + ("whole address", ""), + ("Allowed emails", answer), + ]); + let mut outside = Scripted { + user_config: Some("[console]\naddress = 'saved.example.com'\nallowed_emails = [\n 'saved@example.com', # owner\n 'other@example.com', # colleague\n] # people\n".into()), + ..Scripted::answering(&answers) + }; + setup(&mut outside); + assert_eq!(config(&outside).console.allowed_emails, expected); + let text = outside.user_config.as_deref().unwrap(); + for comment in ["# owner", "# colleague", "# people"] { + assert!(text.contains(comment), "lost {comment}:\n{text}"); + } + } +} diff --git a/src/setup/questions.rs b/src/setup/questions.rs index 0fb5c21b..178c48f6 100644 --- a/src/setup/questions.rs +++ b/src/setup/questions.rs @@ -8,13 +8,17 @@ use std::path::Path; use anyhow::{Result, bail}; use super::{Outside, suggested_address}; -use crate::config::{NotificationAddresses, UserConfig, UserConfigChanges}; +use crate::config::{ + ConsoleAccess, ConsoleSettings, NotificationAddresses, UserConfig, UserConfigChanges, +}; use crate::email::DEFAULT_FROM; use crate::harness::{Harness, ModelAndEffort}; use crate::resend_key::{self, Source}; /// What the user chose. pub struct Answers { + /// None leaves the Console settings as they were. + pub console: Option, /// `harness.default`, and the Model and Effort of the Harness it names, /// each none for the Harness's own default; `None` when sessions can run /// on no Harness here, and the harness settings stay as they were. @@ -38,6 +42,7 @@ impl Answers { /// The User config changes, without Credentials or email effects. pub fn changes(&self) -> UserConfigChanges { UserConfigChanges { + console: self.console.clone(), merge_always: self.merge_always, base_fix: self.base_fix, launch_pull: self.launch_pull, @@ -133,20 +138,38 @@ pub fn ask( "Runs review their change for Security findings?", current.security_review, )?; + let notifications = ask_notifications(outside, current, found)?; + let console = ask_console( + outside, + ¤t.console, + notifications + .as_ref() + .map(|email| email.to.as_str()) + .or(current.email.to.as_deref()), + )?; + Ok(Answers { + harness, + merge_always, + base_fix, + launch_pull, + security_fix, + security_review, + notifications, + console, + }) +} + +fn ask_notifications( + outside: &mut impl Outside, + current: &UserConfig, + found: Option, +) -> Result> { if !yes_or_no( outside, "Run notifications, an email as each Run ends?", current.email.always, )? { - return Ok(Answers { - harness, - merge_always, - base_fix, - launch_pull, - security_fix, - security_review, - notifications: None, - }); + return Ok(None); } let suggested = match ¤t.email.to { Some(to) => Some(to.clone()), @@ -181,20 +204,63 @@ pub fn ask( ); false }; - Ok(Answers { - harness, - merge_always, - base_fix, - launch_pull, - security_fix, - security_review, - notifications: Some(Notifications { - to, - from, - key, - send_test, - }), - }) + Ok(Some(Notifications { + to, + from, + key, + send_test, + })) +} + +/// Keep existing access as the default, then the chosen notification address. +/// The example is a hint, never an address written by pressing Enter. +fn ask_console( + outside: &mut impl Outside, + current: &ConsoleSettings, + email_to: Option<&str>, +) -> Result> { + if !yes_or_no(outside, "Run the Console?", !current.address.is_empty())? { + return Ok(None); + } + let address = loop { + let address = answer( + outside, + "Console's whole address (e.g. thirdshift.example.com)", + (!current.address.is_empty()).then_some(current.address.as_str()), + )?; + let settings = ConsoleSettings { + address: address.clone(), + ..ConsoleSettings::default() + }; + match settings.hostname() { + Ok(_) => break address, + Err(error) => outside.say(error.to_string()), + } + }; + let saved_emails = current.allowed_emails.join(", "); + let default_emails = if saved_emails.is_empty() { + email_to + } else { + Some(saved_emails.as_str()) + }; + let allowed_emails = loop { + let emails = answer(outside, "Allowed emails (comma-separated)", default_emails)?; + let settings = ConsoleSettings { + allowed_emails: emails + .split(',') + .map(|email| email.trim().to_string()) + .collect(), + ..ConsoleSettings::default() + }; + match settings.validate() { + Ok(()) => break settings.allowed_emails, + Err(error) => outside.say(error.to_string()), + } + }; + Ok(Some(ConsoleAccess { + address, + allowed_emails, + })) } /// Ask `question` until the answer is yes, no or nothing, which is `default`. diff --git a/tests/muse_sessions.rs b/tests/muse_sessions.rs index 2ae627ef..f4b77247 100644 --- a/tests/muse_sessions.rs +++ b/tests/muse_sessions.rs @@ -300,6 +300,7 @@ fn setup_checks_muses_proposed_model_with_the_same_flags_and_environment() { ("Security runs may fix reproduced findings?", ""), ("Runs review their change for Security findings?", ""), ("Run notifications", ""), + ("Run the Console?", ""), ], ); assert_eq!(result.code, Some(0), "{}", result.stderr); diff --git a/tests/opencode_sessions.rs b/tests/opencode_sessions.rs index e5db8990..2e616cd1 100644 --- a/tests/opencode_sessions.rs +++ b/tests/opencode_sessions.rs @@ -409,6 +409,7 @@ fn setup_proposes_no_model_and_checks_the_answer_standalone() { ("Security runs may fix reproduced findings?", ""), ("Runs review their change for Security findings?", ""), ("Run notifications", ""), + ("Run the Console?", ""), ], ); assert_eq!(result.code, Some(0), "{}", result.stderr); @@ -487,6 +488,7 @@ fn setup_retries_a_refused_model_and_effort_with_the_same_preflight_check() { ("Security runs may fix reproduced findings?", ""), ("Runs review their change for Security findings?", ""), ("Run notifications", ""), + ("Run the Console?", ""), ], ); assert_eq!(result.code, Some(0), "{}", result.stderr); diff --git a/tests/setup.rs b/tests/setup.rs index 6f88f4bc..83020847 100644 --- a/tests/setup.rs +++ b/tests/setup.rs @@ -328,6 +328,7 @@ const PULL: &str = "fast-forward"; const SECURITY_REVIEW: &str = "Runs review their change for Security findings?"; const SECURITY_FIX: &str = "Security runs may fix reproduced findings?"; const NOTIFY: &str = "Run notifications, an email"; +const CONSOLE: &str = "Run the Console?"; const TO: &str = "Send Run notifications to"; const FROM: &str = "Send them from"; const TEST_EMAIL: &str = "test email now?"; @@ -336,6 +337,85 @@ const KEPT: &str = "Resend API key (input hidden, Enter keeps the saved one):"; const WROTE_CREDENTIALS: &str = "wrote the Credentials"; const KEY: &str = "re_secret_123"; +#[test] +fn setup_console_questions_use_email_to_keep_comments_and_offer_install() { + let scenario = Scenario::new(); + scenario.user_config_is("[email]\nto = 'owner@example.com'\n[logs]\nwaiting_line_minutes = 2\n[inbox]\nsilent_minutes = 2\n[console] # my Console\naddress = '' # site\nallowed_emails = [] # people\n"); + let result = setup_on_terminal( + &scenario, + &[], + &[ + (HARNESS, ""), + (MODEL, ""), + (EFFORT, ""), + (MERGE, ""), + (PULL, ""), + (SECURITY_FIX, ""), + (SECURITY_REVIEW, ""), + (NOTIFY, "n"), + (CONSOLE, "y"), + ( + "Console's whole address (e.g. thirdshift.example.com):", + "https://thirdshift.example.com", + ), + ("Allowed emails (comma-separated) [owner@example.com]", ""), + ], + ); + let text = result.user_config.unwrap(); + let config: toml::Table = text.parse().unwrap(); + assert_eq!( + config["console"]["address"].as_str(), + Some("https://thirdshift.example.com") + ); + assert_eq!( + config["console"]["allowed_emails"][0].as_str(), + Some("owner@example.com") + ); + for comment in ["[console] # my Console", "# site", "# people"] { + assert!(text.contains(comment), "{text}"); + } + assert!( + result.stderr.contains("thirdshift console install"), + "{}", + result.stderr + ); + assert!( + result + .stderr + .contains("logs.waiting_line_minutes must be less than inbox.silent_minutes"), + "{}", + result.stderr + ); +} + +#[test] +fn cancelling_the_console_questions_preserves_the_user_config() { + let scenario = Scenario::new(); + let saved = "[email]\nto = 'owner@example.com'\n"; + scenario.user_config_is(saved); + let result = scenario.run_on_terminal( + &["setup"], + &[], + &[ + (HARNESS, ""), + (MODEL, ""), + (EFFORT, ""), + (MERGE, ""), + (PULL, ""), + (SECURITY_FIX, ""), + (SECURITY_REVIEW, ""), + (NOTIFY, "n"), + (CONSOLE, "y"), + ("Console's whole address", "thirdshift.example.com"), + ("Allowed emails", CTRL_C), + ], + ); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!(result.stderr.contains("interrupted"), "{}", result.stderr); + assert_eq!(result.user_config.as_deref(), Some(saved)); + assert!(result.terminal_restored); +} + #[test] fn setup_can_enable_security_review_and_keeps_it_as_the_next_default() { let scenario = Scenario::new(); @@ -352,6 +432,7 @@ fn setup_can_enable_security_review_and_keeps_it_as_the_next_default() { (SECURITY_FIX, ""), (SECURITY_REVIEW, answer), (NOTIFY, ""), + (CONSOLE, "n"), ], ); assert!( @@ -379,6 +460,7 @@ fn setup_asks_once_about_security_fixing_with_off_as_the_default_and_writes_yes( (SECURITY_FIX, "y"), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ], ); assert_eq!(result.stderr.matches(SECURITY_FIX).count(), 1); @@ -418,6 +500,7 @@ fn rerunning_setup_keeps_security_answers_as_defaults_and_can_turn_fixing_off() (SECURITY_FIX, answer), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, "n"), ], ); assert!(result.stderr.contains(&format!("{SECURITY_FIX} {choices}"))); @@ -624,7 +707,7 @@ fn pasted_answers_are_left_available_to_later_terminal_questions() { &[], &[( HARNESS, - " claude \n\n\n\n\n\n\ny\n café@example.com \n\n", + " claude \n\n\n\n\n\n\ny\n café@example.com \n\n\n", )], ); let config: toml::Table = result.user_config.unwrap().parse().unwrap(); @@ -737,6 +820,7 @@ fn on_a_terminal_pressing_enter_throughout_writes_what_setup_with_no_terminal_wr (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + (CONSOLE, ""), ], ); @@ -761,7 +845,11 @@ fn on_a_terminal_an_entered_key_is_written_to_the_credentials_and_never_shown() let result = setup_on_terminal( &scenario, &[], - ¬ifications_on(&[(KEY_PROMPT, &format!(" {KEY} ")), (TEST_EMAIL, "")]), + ¬ifications_on(&[ + (KEY_PROMPT, &format!(" {KEY} ")), + (TEST_EMAIL, ""), + (CONSOLE, "n"), + ]), ); let path = scenario.path("home/.thirdshift/credentials.toml"); diff --git a/tests/setup_offer.rs b/tests/setup_offer.rs index e9c7bacb..c5fbc686 100644 --- a/tests/setup_offer.rs +++ b/tests/setup_offer.rs @@ -87,6 +87,7 @@ fn accepting_and_choosing_merge_always_makes_that_run_a_merge_run() { (SECURITY_FIX, ""), (SECURITY_REVIEW, ""), (NOTIFY, ""), + ("Run the Console?", ""), ], );