Keep Setup Security settings outside the Harness area

README.md

@@ -253,11 +253,6 @@ parallel = 3   # how many Tickets a Spec run runs at once; default 3
 limit = 3   # how many open issues labelled in-progress stop a Pickup run taking another; default 3
 wait_minutes = 30   # minutes since the latest shaping event before an issue is Ready; 0 disables waiting; default 30
 
-[security]
-fix = false   # Security runs may fix reproduced findings; default false
-review = false   # Runs review their change for Security findings; default false
-harness = ""   # the Harness a Security run uses unless its command names one; default blank, for harness.default or Claude Code
-
 [harness]
 default = "claude"   # the Harness every Run's sessions run on, claude, codex, agy, grok, muse or opencode; default claude
 
@@ -284,6 +279,11 @@ effort = ""   # how hard that Model reasons; default blank, for Muse Code's own
 [harness.opencode]
 model = ""    # the Model OpenCode's sessions run on, provider/model; default blank, for OpenCode's own
 effort = ""   # the Model's variant, passed as #effort; default blank, for OpenCode's own
+
+[security]
+fix = false   # Security runs may fix reproduced findings; default false
+review = false   # Runs review their change for Security findings; default false
+harness = ""   # the Harness a Security run uses unless its command names one; default blank, for harness.default or Claude Code
 ```
 
 Every key holds its real value, so a Run reading it does exactly what it does with no file. `email.to` has no default, so `setup` suggests one: the public email of your GitHub profile (from `gh api user`), else your global git `user.email`, unless that is a `@users.noreply.github.com` address, which can't receive mail. With neither, `email.to` is the only line written commented out, as above. `setup` never asks `gh` for more scopes, so a private GitHub email is not read, and a Run never looks the suggestion up: `--email` with no address and no `email.to` still stops the Run. From a terminal (stdin and stderr both terminals), `setup` first asks, on stderr:
@@ -297,7 +297,7 @@ Every key holds its real value, so a Run reading it does exactly what it does wi
 7. Run notifications? (`email.always`). If yes, the address (`email.to`), asked again until it has an `@`, then the sender (`email.from`).
 8. 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`, `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`).
+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. If `[security]` falls between Harness sections, Setup moves it after them, keeping its values and comments. One that already lists every setting, down to the commented-out `email.to` line, with Security outside the Harness area 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.
 

src/config.rs

@@ -227,6 +227,7 @@ impl UserConfig {
 
 /// A validated User config with every setting present, ready for Setup.
 /// Preparation and edits preserve the user's TOML spelling and comments.
+/// A Security section between Harness sections moves after them.
 pub struct UserConfigDocument {
     document: DocumentMut,
     settings: UserConfig,
@@ -271,7 +272,8 @@ impl UserConfigDocument {
         let settings = UserConfig::parse(&completed, path, home)?;
         // Completion appends missing implicit sections after the original
         // text. Reparse that text so subsequent edits keep their placement.
-        let document = completed.parse().context("can't parse the User config")?;
+        let mut document = completed.parse().context("can't parse the User config")?;
+        separate_security_from_harness(&mut document);
         Ok(Self { document, settings })
     }
 
@@ -312,6 +314,41 @@ impl UserConfigDocument {
     }
 }
 
+/// Keep Security in its own area when completion or an earlier Setup put
+/// it between Harness sections. Moving only its table preserves its keys,
+/// comments and values, and every other section's relative order.
+fn separate_security_from_harness(document: &mut DocumentMut) {
+    let Some(security_position) = document
+        .get("security")
+        .and_then(Item::as_table)
+        .filter(|table| !table.is_implicit() && !table.is_dotted())
+        .and_then(toml_edit::Table::position)
+    else {
+        return;
+    };
+    let Some(harness) = document.get("harness").and_then(Item::as_table) else {
+        return;
+    };
+    let positions: Vec<isize> = std::iter::once(harness)
+        .chain(harness.iter().filter_map(|(_, item)| item.as_table()))
+        .filter(|table| !table.is_implicit() && !table.is_dotted())
+        .filter_map(toml_edit::Table::position)
+        .collect();
+    let (Some(first), Some(last)) = (positions.iter().min(), positions.iter().max()) else {
+        return;
+    };
+    if security_position > *first && security_position < *last {
+        let mut end = *last;
+        while let Some(next) = following_section(document.as_table(), end) {
+            end = next.position().expect("a written section has a position");
+        }
+        document["security"]
+            .as_table_mut()
+            .expect("Security has a section")
+            .set_position(Some(end + 1));
+    }
+}
+
 /// Replace the file at `path` with `text` all at once, keeping its
 /// permissions, so a failed write leaves it as it was.
 pub fn replace(path: &Path, text: &str) -> std::io::Result<()> {
@@ -704,11 +741,6 @@ parallel = 3   # how many Tickets a Spec run runs at once; default 3
 limit = 3   # how many open issues labelled in-progress stop a Pickup run taking another; default 3
 wait_minutes = 30   # minutes since the latest shaping event before an issue is Ready; 0 disables waiting; default 30
 
-[security]
-fix = false   # Security runs may fix reproduced findings; default false
-review = false   # Runs review their change for Security findings; default false
-harness = ""   # the Harness a Security run uses unless its command names one; default blank, for harness.default or Claude Code
-
 [harness]
 default = "claude"   # the Harness every Run's sessions run on, claude, codex, agy, grok, muse or opencode; default claude
 
@@ -735,6 +767,11 @@ effort = ""   # how hard that Model reasons; default blank, for Muse Code's own
 [harness.opencode]
 model = ""    # the Model OpenCode's sessions run on, provider/model; default blank, for OpenCode's own
 effort = ""   # the Model's variant, passed as #effort; default blank, for OpenCode's own
+
+[security]
+fix = false   # Security runs may fix reproduced findings; default false
+review = false   # Runs review their change for Security findings; default false
+harness = ""   # the Harness a Security run uses unless its command names one; default blank, for harness.default or Claude Code
 "#;
 
 /// The line `DEFAULTS` holds for `email.to`, which has no default.
@@ -865,9 +902,6 @@ mod tests {
                 "spec.parallel",
                 "pickup.limit",
                 "pickup.wait_minutes",
-                "security.fix",
-                "security.review",
-                "security.harness",
                 "harness.default",
                 "harness.claude.model",
                 "harness.claude.effort",
@@ -880,7 +914,10 @@ mod tests {
                 "harness.muse.model",
                 "harness.muse.effort",
                 "harness.opencode.model",
-                "harness.opencode.effort"
+                "harness.opencode.effort",
+                "security.fix",
+                "security.review",
+                "security.harness",
             ]
         );
         let commented_out: Vec<&str> = DEFAULTS

src/setup.rs

@@ -38,7 +38,8 @@ use questions::Answers;
 /// An existing User config is edited in place, once it parses as a Run would
 /// parse it: its comments and key order stay, as do the values Setup didn't
 /// ask about, and each key it lacks is added at its default, so Setup with no
-/// terminal never resets a configured machine. A Resend API key the user
+/// terminal never resets a configured machine. A Security section between
+/// Harness sections moves after them. A Resend API key the user
 /// gave is saved in the Credentials once the User config is written; with no
 /// terminal, the Credentials are never read or written. Nothing is written
 /// until the last answer is in. A test email, if the user asked for one, goes
@@ -96,10 +97,7 @@ fn run_setup(outside: &mut impl Outside, home: &Path, path: &Path) -> Result<Str
             if asked {
                 format!("wrote your answers to the User config {}", path.display())
             } else {
-                format!(
-                    "added the missing settings to the User config {}",
-                    path.display()
-                )
+                format!("updated the User config {}", path.display())
             }
         }
     };
@@ -956,10 +954,7 @@ to = \"me@example.com\"  # my inbox
 
         let done = setup(&mut outside).unwrap();
 
-        assert_eq!(
-            done,
-            format!("added the missing settings to the User config {PATH}")
-        );
+        assert_eq!(done, format!("updated the User config {PATH}"));
         let completed = UserConfigDocument::existing(partial, Path::new(PATH), Path::new(HOME))
             .unwrap()
             .render(None);

tests/setup.rs

@@ -51,14 +51,14 @@ fn with_no_terminal_and_no_user_config_setup_writes_the_defaults() {
             "[activity]",
             "[spec]",
             "[pickup]",
-            "[security]",
             "[harness]",
             "[harness.claude]",
             "[harness.codex]",
             "[harness.agy]",
             "[harness.grok]",
             "[harness.muse]",
-            "[harness.opencode]"
+            "[harness.opencode]",
+            "[security]"
         ],
         "{text}"
     );
@@ -91,6 +91,124 @@ fn with_no_terminal_and_no_user_config_setup_writes_the_defaults() {
     );
 }
 
+#[test]
+fn upgrading_setup_puts_security_after_all_harness_settings() {
+    let scenario = Scenario::new();
+    let original = "\
+[merge]
+always = false
+[base]
+fix = false
+[launch]
+pull = false
+[email]
+always = false
+from = 'onboarding@resend.dev'
+[logs]
+dir = '~/.thirdshift/logs'
+[activity]
+quiet_skips = false
+[spec]
+parallel = 3
+[pickup]
+limit = 3
+wait_minutes = 30
+[harness]
+default = 'codex'
+[harness.claude]
+model = ''
+effort = ''
+[harness.codex]
+model = 'gpt-6.1-sol' # my Model
+effort = 'high' # my Effort
+";
+    scenario.user_config_is(original);
+
+    let result = scenario.run(&["setup"]);
+
+    assert_eq!(result.code, Some(0), "stderr: {}", result.stderr);
+    let text = user_config(&scenario).unwrap();
+    let sections: Vec<&str> = text.lines().filter(|line| line.starts_with('[')).collect();
+    let harness = sections
+        .iter()
+        .position(|section| *section == "[harness]")
+        .unwrap();
+    assert_eq!(
+        &sections[harness..],
+        [
+            "[harness]",
+            "[harness.claude]",
+            "[harness.codex]",
+            "[harness.agy]",
+            "[harness.grok]",
+            "[harness.muse]",
+            "[harness.opencode]",
+            "[security]",
+        ],
+        "{text}"
+    );
+    assert!(
+        text.contains("model = 'gpt-6.1-sol' # my Model\n"),
+        "{text}"
+    );
+    assert!(text.contains("effort = 'high' # my Effort\n"), "{text}");
+    let config: toml::Table = text.parse().unwrap();
+    assert_eq!(config["harness"]["default"].as_str(), Some("codex"));
+    assert_eq!(config["security"]["fix"].as_bool(), Some(false));
+    assert_eq!(config["security"]["review"].as_bool(), Some(false));
+    assert_eq!(config["security"]["harness"].as_str(), Some(""));
+
+    let again = scenario.run(&["setup"]);
+    assert_eq!(again.code, Some(0), "stderr: {}", again.stderr);
+    assert_eq!(user_config(&scenario).unwrap(), text);
+}
+
+#[test]
+fn setup_keeps_existing_security_outside_completed_harness_settings() {
+    for later_harness in ["", "\n[harness.agy]\nmodel = ''\neffort = ''\n"] {
+        let scenario = Scenario::new();
+        scenario.user_config_is(&format!(
+            "[harness]\ndefault = 'codex'\n\
+             [harness.codex]\nmodel = 'gpt-6.1-sol'\neffort = 'high'\n\n\
+             # my Security settings\n[security] # separate area\n\
+             fix = true # allow fixes\nreview = true # review changes\n\
+             harness = 'codex' # Security Harness\n{later_harness}"
+        ));
+
+        let result = scenario.run(&["setup"]);
+
+        assert_eq!(result.code, Some(0), "stderr: {}", result.stderr);
+        let text = user_config(&scenario).unwrap();
+        let sections: Vec<&str> = text.lines().filter(|line| line.starts_with('[')).collect();
+        assert_eq!(
+            sections.last(),
+            Some(&"[security] # separate area"),
+            "{text}"
+        );
+        assert!(
+            text.contains(
+                "# my Security settings\n[security] # separate area\n\
+                 fix = true # allow fixes\nreview = true # review changes\n\
+                 harness = 'codex' # Security Harness\n"
+            ),
+            "{text}"
+        );
+        let config: toml::Table = text.parse().unwrap();
+        assert_eq!(config["security"]["fix"].as_bool(), Some(true));
+        assert_eq!(config["security"]["review"].as_bool(), Some(true));
+        assert_eq!(config["security"]["harness"].as_str(), Some("codex"));
+        assert_eq!(
+            config["harness"]["codex"]["model"].as_str(),
+            Some("gpt-6.1-sol")
+        );
+        assert_eq!(config["harness"]["codex"]["effort"].as_str(), Some("high"));
+
+        let again = scenario.run(&["setup"]);
+        assert_eq!(again.code, Some(0), "stderr: {}", again.stderr);
+        assert_eq!(user_config(&scenario).unwrap(), text);
+    }
+}
+
 #[test]
 fn completing_an_existing_file_replaces_it_atomically_keeps_permissions_and_then_avoids_replacement()
  {