Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -288,10 +288,11 @@ Every key holds its real value, so a Run reading it does exactly what it does wi
2. Every Run a Merge run? (`merge.always`)
3. With that on, every Run may start a Base fix when the Base branch's CI is red? (`base.fix`). With it off, this is not asked, and `base.fix` is written at its default, `false`.
4. Every Run first fast-forwards your checkout of the Base branch? (`launch.pull`)
5. Run notifications? (`email.always`). If yes, the address (`email.to`), asked again until it has an `@`, then the sender (`email.from`).
6. With notifications on, the Resend API key, with input hidden. With none saved it asks `Resend API key (input hidden, Enter to skip):`; with one in the [Credentials](#email), `Resend API key (input hidden, Enter keeps the saved one):`, and a new one replaces it. Surrounding spaces are trimmed, and anything that doesn't start with `re_` is asked again. A key you give is saved in the Credentials, `~/.thirdshift/credentials.toml`, created with mode 600 (and `~/.thirdshift` with it) or edited in place, keeping its comments and anything else in it and changing only `resend.key`; `setup` then prints `wrote the Credentials <path>`. The key is never printed, nor written to the User config. Skipping writes no Credentials and says how to add a key later: rerun `thirdshift setup`, or set `RESEND_API_KEY`. With `RESEND_API_KEY` set and not empty, which wins over the Credentials, nothing is asked, and it says the key comes from `RESEND_API_KEY`. With a key found or given, it offers to send a test email (default No), as `thirdshift email-test` does, once the files are written.
5. Security runs may fix reproduced findings? (`security.fix`), off by default.
6. Run notifications? (`email.always`). If yes, the address (`email.to`), asked again until it has an `@`, then the sender (`email.from`).
7. With notifications on, the Resend API key, with input hidden. With none saved it asks `Resend API key (input hidden, Enter to skip):`; with one in the [Credentials](#email), `Resend API key (input hidden, Enter keeps the saved one):`, and a new one replaces it. Surrounding spaces are trimmed, and anything that doesn't start with `re_` is asked again. A key you give is saved in the Credentials, `~/.thirdshift/credentials.toml`, created with mode 600 (and `~/.thirdshift` with it) or edited in place, keeping its comments and anything else in it and changing only `resend.key`; `setup` then prints `wrote the Credentials <path>`. The key is never printed, nor written to the User config. Skipping writes no Credentials and says how to add a key later: rerun `thirdshift setup`, or set `RESEND_API_KEY`. With `RESEND_API_KEY` set and not empty, which wins over the Credentials, nothing is asked, and it says the key comes from `RESEND_API_KEY`. With a key found or given, it offers to send a test email (default No), as `thirdshift email-test` does, once the files are written.

Pressing Enter takes the default shown, which is the file's current value, or else the setting's default, and for the address the suggested email above. `logs.dir`, `activity.quiet_skips`, `spec.parallel` and `pickup.limit` are not asked about. The answers are written like everything else below: in place, keeping your comments. Ctrl-C, SIGTERM or SIGHUP during the questions or a Model/Effort check exits `1` with `interrupted` and writes nothing: neither the User config nor the Credentials. Ordinary and hidden questions stop without another line of input, and hidden input restores the saved terminal settings, including echo. With notifications off, nothing about a key is asked, and saved Credentials stay as they were, so `--email` on a single Run still works. Credentials a Run would refuse (see [Email](#email)) are refused before any question, exit `1`, and not touched. With no terminal, as from cron or `thirdshift setup </dev/null`, `setup` asks nothing and never writes the Credentials. Either way it prints the file's path on stderr and exits `0` with stdout empty. Over a User config that is already there, `setup` edits it in place: its comments and key order stay, as do the values it didn't ask about, and each key it lacks is added at its default with its comment, so afterwards the file lists every setting this version knows. One that already does, down to the commented-out `email.to` line, is left byte for byte as it was. A key added to an inline table, such as `launch = { pull = true }`, gets no comment, since TOML has no place for one there. One a Run would refuse is refused the same way, exit `1`, and not touched. Any argument after `setup` is an argument error (exit `2`).
Pressing Enter takes the default shown, which is the file's current value, or else the setting's default, and for the address the suggested email above. `logs.dir`, `activity.quiet_skips`, `spec.parallel`, `pickup.limit` and `security.harness` are not asked about. Setup adds a missing `security.harness` at its default, blank, with a comment explaining it; a configured Harness is kept. The answers are written like everything else below: in place, keeping your comments. Ctrl-C, SIGTERM or SIGHUP during the questions or a Model/Effort check exits `1` with `interrupted` and writes nothing: neither the User config nor the Credentials. Ordinary and hidden questions stop without another line of input, and hidden input restores the saved terminal settings, including echo. With notifications off, nothing about a key is asked, and saved Credentials stay as they were, so `--email` on a single Run still works. Credentials a Run would refuse (see [Email](#email)) are refused before any question, exit `1`, and not touched. With no terminal, as from cron or `thirdshift setup </dev/null`, `setup` asks nothing and never writes the Credentials. Either way it prints the file's path on stderr and exits `0` with stdout empty. Over a User config that is already there, `setup` edits it in place: its comments and key order stay, as do the values it didn't ask about, and each key it lacks is added at its default with its comment, so afterwards the file lists every setting this version knows. One that already does, down to the commented-out `email.to` line, is left byte for byte as it was. A key added to an inline table, such as `launch = { pull = true }`, gets no comment, since TOML has no place for one there. One a Run would refuse is refused the same way, exit `1`, and not touched. Any argument after `setup` is an argument error (exit `2`).

The first Run on a machine with no User config, started from a terminal, offers Setup before any work, on stderr: `No User config at <path>. Set your defaults now? [Y/n]`. Yes (or Enter) asks the questions above, writes the file, and the Run carries on using your answers; a flag in the command, such as `--no-merge`, `--email` or `--no-email`, still wins over them. No writes every setting at its default, as `setup` with no terminal does, asks nothing about a key, writes no Credentials, says that `thirdshift setup` changes it, and the Run carries on; later Runs find the file and don't offer again. If the file can't be written, stderr gets a `warning:` line and the Run carries on with the defaults. Ctrl-C, SIGTERM or SIGHUP during the offer, the questions or a Model/Effort check exits `1` with `interrupted`, writes nothing and ends the command before any work, with no Run notification. A command thirdshift can't parse exits `2` before any offer. A Run with no terminal, as from cron, CI, `nohup` or an agent's shell, offers nothing, writes nothing, and runs on the defaults, so a later Run from a terminal still gets the offer.

Expand Down
7 changes: 7 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ pub struct UserConfigChanges {
pub merge_always: bool,
pub base_fix: bool,
pub launch_pull: bool,
pub security_fix: bool,
/// None disables Run notifications while retaining their addresses.
pub notifications: Option<NotificationAddresses>,
/// None retains every Harness setting. Unset Model/Effort writes blank.
Expand Down Expand Up @@ -260,6 +261,7 @@ impl UserConfigDocument {
set(&mut document, "merge", "always", changes.merge_always);
set(&mut document, "base", "fix", changes.base_fix);
set(&mut document, "launch", "pull", changes.launch_pull);
set(&mut document, "security", "fix", changes.security_fix);
set(
&mut document,
"email",
Expand Down Expand Up @@ -1185,6 +1187,7 @@ mod tests {
merge_always: true,
base_fix: true,
launch_pull: true,
security_fix: false,
notifications: Some(NotificationAddresses {
to: "me@example.com".to_string(),
from: "ts@example.com".to_string(),
Expand Down Expand Up @@ -1338,6 +1341,7 @@ mod tests {
merge_always: false,
base_fix: false,
launch_pull: false,
security_fix: false,
notifications: Some(NotificationAddresses {
to: to.to_string(),
from: crate::email::DEFAULT_FROM.to_string(),
Expand Down Expand Up @@ -1430,6 +1434,7 @@ mod tests {
merge_always: true,
base_fix: false,
launch_pull: false,
security_fix: false,
notifications: None,
harness: Some((
Harness::Codex,
Expand Down Expand Up @@ -1473,6 +1478,7 @@ mod tests {
merge_always: true,
base_fix: true,
launch_pull: true,
security_fix: false,
notifications: Some(NotificationAddresses {
to: "o\"brien@example.com".to_string(),
from: "ts@example.com".to_string(),
Expand Down Expand Up @@ -1557,6 +1563,7 @@ mod tests {
merge_always: true,
base_fix: false,
launch_pull: true,
security_fix: false,
notifications: None,
harness: None,
};
Expand Down
46 changes: 38 additions & 8 deletions src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,7 @@ mod tests {
const MERGE: &str = "Every Run a Merge run?";
const BASE_FIX: &str = "Every Run may start a Base fix when the Base branch's CI is red?";
const PULL: &str = "fast-forward";
const SECURITY_FIX: &str = "Security runs may fix reproduced findings?";
const NOTIFY: &str = "Run notifications, an email";
const TO: &str = "Send Run notifications to";
const FROM: &str = "Send them from";
Expand All @@ -733,7 +734,8 @@ mod tests {
const KEY: &str = "re_secret_123";

/// The answers that take every default and turn nothing on.
const ENTER_THROUGHOUT: [(&str, &str); 3] = [(MERGE, ""), (PULL, ""), (NOTIFY, "")];
const ENTER_THROUGHOUT: [(&str, &str); 4] =
[(MERGE, ""), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "")];

/// The answers that turn Run notifications on, to `me@example.com` from
/// the default sender, then `rest`.
Expand All @@ -743,6 +745,7 @@ mod tests {
let mut answers = vec![
(MERGE, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, "me@example.com"),
(FROM, ""),
Expand Down Expand Up @@ -1044,6 +1047,7 @@ to = \"me@example.com\" # my inbox
[
"Every Run a Merge run? [y/N] ",
"Every Run first fast-forwards your checkout of the Base branch? [y/N] ",
"Security runs may fix reproduced findings? [y/N] ",
"Run notifications, an email as each Run ends? [y/N] ",
]
);
Expand All @@ -1057,6 +1061,7 @@ to = \"me@example.com\" # my inbox
(MERGE, "y"),
(BASE_FIX, "y"),
(PULL, "yes"),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, "me@example.com"),
(FROM, "ts@acme.dev"),
Expand Down Expand Up @@ -1164,6 +1169,7 @@ effort = \"\"
(MERGE, ""),
(BASE_FIX, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
(TO, ""),
(FROM, ""),
Expand All @@ -1184,6 +1190,7 @@ effort = \"\"
"Every Run a Merge run? [Y/n] ",
"Every Run may start a Base fix when the Base branch's CI is red? [Y/n] ",
"Every Run first fast-forwards your checkout of the Base branch? [Y/n] ",
"Security runs may fix reproduced findings? [y/N] ",
"Run notifications, an email as each Run ends? [Y/n] ",
"Send Run notifications to [mine@example.net]: ",
"Send them from [ts@acme.dev]: ",
Expand All @@ -1196,7 +1203,7 @@ effort = \"\"
fn on_a_terminal_changed_answers_are_written_over_the_user_config() {
let mut outside = Scripted {
user_config: Some(DEFAULTS.to_string()),
..Scripted::answering(&[(MERGE, ""), (PULL, "y"), (NOTIFY, "")])
..Scripted::answering(&[(MERGE, ""), (PULL, "y"), (SECURITY_FIX, ""), (NOTIFY, "")])
};

let done = setup(&mut outside).unwrap();
Expand Down Expand Up @@ -1227,6 +1234,7 @@ always = false # quiet, please
(MERGE, "y"),
(BASE_FIX, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, "me@example.com"),
(FROM, ""),
Expand Down Expand Up @@ -1328,8 +1336,13 @@ always = false # quiet, please
#[test]
fn with_merging_on_setup_asks_about_base_fixes_and_writes_the_answer() {
for (answer, allowed) in [("y", true), ("", false), ("n", false)] {
let mut outside =
Scripted::answering(&[(MERGE, "y"), (BASE_FIX, answer), (PULL, ""), (NOTIFY, "")]);
let mut outside = Scripted::answering(&[
(MERGE, "y"),
(BASE_FIX, answer),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
]);

setup(&mut outside).unwrap();

Expand All @@ -1349,7 +1362,12 @@ always = false # quiet, please
#[test]
fn with_merging_off_setup_asks_nothing_about_base_fixes_and_writes_the_default() {
for answer in ["", "n"] {
let mut outside = Scripted::answering(&[(MERGE, answer), (PULL, ""), (NOTIFY, "")]);
let mut outside = Scripted::answering(&[
(MERGE, answer),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
]);

setup(&mut outside).unwrap();

Expand All @@ -1363,7 +1381,13 @@ always = false # quiet, please
fn the_base_fix_question_defaults_to_the_user_configs_base_fix() {
let mut outside = Scripted {
user_config: Some("[merge]\nalways = true\n\n[base]\nfix = true # mine\n".to_string()),
..Scripted::answering(&[(MERGE, ""), (BASE_FIX, ""), (PULL, ""), (NOTIFY, "")])
..Scripted::answering(&[
(MERGE, ""),
(BASE_FIX, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
])
};

setup(&mut outside).unwrap();
Expand All @@ -1383,7 +1407,7 @@ always = false # quiet, please
fn turning_merging_off_writes_base_fix_at_its_default() {
let mut outside = Scripted {
user_config: Some("[merge]\nalways = true\n\n[base]\nfix = true # mine\n".to_string()),
..Scripted::answering(&[(MERGE, "n"), (PULL, ""), (NOTIFY, "")])
..Scripted::answering(&[(MERGE, "n"), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "")])
};

setup(&mut outside).unwrap();
Expand All @@ -1400,6 +1424,7 @@ always = false # quiet, please
(MERGE, "YES"),
(BASE_FIX, "No"),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
]);

Expand All @@ -1413,6 +1438,7 @@ always = false # quiet, please
let mut outside = Scripted::answering(&[
(MERGE, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, ""),
(TO, "me.example.com"),
Expand Down Expand Up @@ -1441,7 +1467,7 @@ always = false # quiet, please
let mut outside = Scripted {
key: Ok(Some(Source::Credentials(CREDENTIALS.into()))),
github_email: Ok(Some("octo@example.com")),
..Scripted::answering(&[(MERGE, ""), (PULL, ""), (NOTIFY, "n")])
..Scripted::answering(&[(MERGE, ""), (PULL, ""), (SECURITY_FIX, ""), (NOTIFY, "n")])
};

setup(&mut outside).unwrap();
Expand Down Expand Up @@ -1683,6 +1709,7 @@ always = false # quiet, please
..Scripted::answering(&[
(MERGE, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, ""),
(FROM, ""),
Expand Down Expand Up @@ -1783,6 +1810,7 @@ always = false # quiet, please
(MERGE, "y"),
(BASE_FIX, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, ""),
]);

Expand All @@ -1801,6 +1829,7 @@ always = false # quiet, please
(OFFER, "y"),
(MERGE, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, "me@example.com"),
(FROM, ""),
Expand Down Expand Up @@ -1898,6 +1927,7 @@ always = false # quiet, please
(OFFER, "y"),
(MERGE, ""),
(PULL, ""),
(SECURITY_FIX, ""),
(NOTIFY, "y"),
(TO, "me@example.com"),
(FROM, ""),
Expand Down
10 changes: 10 additions & 0 deletions src/setup/questions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ pub struct Answers {
pub base_fix: bool,
/// `launch.pull`.
pub launch_pull: bool,
/// `security.fix`: Security runs may fix reproduced findings.
pub security_fix: bool,
/// With Run notifications on, `email.always`, their settings; with them
/// off, `None`, and the email settings stay as they were.
pub notifications: Option<Notifications>,
Expand All @@ -38,6 +40,7 @@ impl Answers {
merge_always: self.merge_always,
base_fix: self.base_fix,
launch_pull: self.launch_pull,
security_fix: self.security_fix,
notifications: self
.notifications
.as_ref()
Expand Down Expand Up @@ -118,6 +121,11 @@ pub fn ask(
"Every Run first fast-forwards your checkout of the Base branch?",
current.launch_pull,
)?;
let security_fix = yes_or_no(
outside,
"Security runs may fix reproduced findings?",
current.security_fix.unwrap_or(false),
)?;
if !yes_or_no(
outside,
"Run notifications, an email as each Run ends?",
Expand All @@ -128,6 +136,7 @@ pub fn ask(
merge_always,
base_fix,
launch_pull,
security_fix,
notifications: None,
});
}
Expand Down Expand Up @@ -169,6 +178,7 @@ pub fn ask(
merge_always,
base_fix,
launch_pull,
security_fix,
notifications: Some(Notifications {
to,
from,
Expand Down
1 change: 1 addition & 0 deletions tests/muse_sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ fn setup_checks_muses_proposed_model_with_the_same_flags_and_environment() {
("Effort for muse", "low"),
("Every Run a Merge run", ""),
("Every Run first fast-forwards", ""),
("Security runs may fix reproduced findings?", ""),
("Run notifications", ""),
],
);
Expand Down
2 changes: 2 additions & 0 deletions tests/opencode_sessions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ fn setup_proposes_no_model_and_checks_the_answer_standalone() {
("Effort for opencode", "high"),
("Every Run a Merge run", ""),
("Every Run first fast-forwards", ""),
("Security runs may fix reproduced findings?", ""),
("Run notifications", ""),
],
);
Expand Down Expand Up @@ -482,6 +483,7 @@ fn setup_retries_a_refused_model_and_effort_with_the_same_preflight_check() {
("Effort for opencode", "low"),
("Every Run a Merge run", ""),
("Every Run first fast-forwards", ""),
("Security runs may fix reproduced findings?", ""),
("Run notifications", ""),
],
);
Expand Down
Loading
Loading