One Run notification per Architect run

CONTEXT.md

@@ -86,7 +86,7 @@ A **Run** asked to end with its pull request merged rather than left for review,
 _Avoid_: auto-merge (GitHub's own feature, which thirdshift does not use)
 
 **Run notification**:
-A message thirdshift sends when a **Run** or a **Spec run** ends, whatever its outcome (ready, merged, failed or interrupted), to the address given with the email flag or the default in the **User config**. A Run or Spec run sends one only when asked to, by the flag or by the User config; a Spec run's notification lists each **Ticket**'s outcome, and a Ticket's **Run** never sends one of its own. Failing to send one never changes the outcome.
+A message thirdshift sends when a **Run**, a **Spec run** or an **Architect run** ends, whatever its outcome (ready, merged, failed or interrupted; for an Architect run that dispatched nothing, plan published, idea filed, idea already filed or review failed), to the address given with the email flag or the default in the **User config**. Each sends one only when asked to, by the flag or by the User config; a Spec run's notification lists each **Ticket**'s outcome, and a Ticket's **Run** never sends one of its own. An Architect run's notification tells how its **Architecture review** ended, naming the plan or idea issue, and how the Spec run or Run it dispatched ended, which sends none of its own. Failing to send one never changes the outcome.
 _Avoid_: completion email, alert
 
 **User config**:

README.md

@@ -242,7 +242,7 @@ no Resend API key. Either:
     (a crontab line, CI secret, or a shell profile the Run's shell reads)
 ```
 
-The Credentials are read only when `RESEND_API_KEY` is unset or empty, and only by `email-test`, `setup`, and a Run that asks for a notification. A missing file just means no key from it. One that isn't valid TOML, or holds anything but a quoted `resend.key`, such as a typo like `kye`, stops the command with exit `1`, naming the file and the offending key, and nothing is sent. One that others can read is still used, with a `warning:` line on stderr saying to `chmod 600` it. Nothing is sent to check the key itself. When Resend accepts the email, it prints `accepted by Resend; check your inbox` and exits `0`; that is all it can verify, so check that the email arrives. When Resend refuses it, for example for a bad key or a sender it won't send from, it prints Resend's error text word for word, with where the key came from, and exits `1`. It gives up after 30 seconds without an answer.
+The Credentials are read only when `RESEND_API_KEY` is unset or empty, and only by `email-test`, `setup`, and a Run or an Architect run that asks for a notification. A missing file just means no key from it. One that isn't valid TOML, or holds anything but a quoted `resend.key`, such as a typo like `kye`, stops the command with exit `1`, naming the file and the offending key, and nothing is sent. One that others can read is still used, with a `warning:` line on stderr saying to `chmod 600` it. Nothing is sent to check the key itself. When Resend accepts the email, it prints `accepted by Resend; check your inbox` and exits `0`; that is all it can verify, so check that the email arrives. When Resend refuses it, for example for a bad key or a sender it won't send from, it prints Resend's error text word for word, with where the key came from, and exits `1`. It gives up after 30 seconds without an answer.
 
 #### Run notifications
 
@@ -255,6 +255,8 @@ A Run that asks for a notification, by the flag or by `email.always`, makes the
 
 A [Spec run](#spec-runs) sends at most one notification for the whole Spec, under the same rules, with its checks made once before any Ticket starts. Its subject names the Spec, its outcome is the Spec run's, and its body, after what a Run's holds (the Spec PR, if any), lists each Ticket's outcome, one line per Ticket as in the summary on stderr, such as `#21 landed with https://github.com/acme/widgets/pull/1` or `#22 blocked by #21`. The Ticket Runs inside it never send a notification of their own, whatever the User config says.
 
+An [Architect run](#architect-runs) takes the same flags and the same `email.always`, and sends [one notification](#one-run-notification) covering its review and the run it dispatched.
+
 A notification that can't be sent is a `warning:` line on stderr with Resend's error. It never changes the Run's outcome, stdout or exit code.
 
 ### Logs
@@ -355,13 +357,14 @@ thirdshift architect                              # review the whole codebase, t
 thirdshift architect "the Spec run"               # point the review at an area
 thirdshift architect merge parallel 2             # merge the plan's pull request, two Tickets at once
 thirdshift architect "the Spec run" --plan-only   # publish the plan, mark it ready and stop
+thirdshift architect --email you@example.com      # email how the Architect run ended
 ```
 
 The focus is optional free text, given as one argument, anywhere among the flags; it goes into the Session prompt. `architect` is a command only as the first argument.
 
-`merge`, `no-merge` and `parallel <n>` (or `--merge`, `--no-merge` and `--parallel <n>`) are for the run the plan is dispatched as, and mean what they do for `thirdshift <Issue URL>`, so none of them is ever read as the focus. `--plan-only` stops the Architect run once the plan is marked ready, and dispatches nothing.
+`merge`, `no-merge` and `parallel <n>` (or `--merge`, `--no-merge` and `--parallel <n>`) are for the run the plan is dispatched as, and mean what they do for `thirdshift <Issue URL>`, so none of them is ever read as the focus. `--plan-only` stops the Architect run once the plan is marked ready, and dispatches nothing. `email`, optionally followed by an address, and `no-email` (or `--email` and `--no-email`) are for the Architect run's own [Run notification](#one-run-notification), with or without `--plan-only`. The word after `email` is the address only if it contains `@`, so a focus without one is never taken for it.
 
-A second focus, a repeated flag, `merge` with `no-merge`, `parallel` without a whole number from 1 up, any other argument starting with a dash, or an empty focus is an argument error (exit `2`). So is `merge`, `no-merge` or `parallel` with `--plan-only`, since nothing is dispatched for them to apply to.
+A second focus, a repeated flag, `merge` with `no-merge`, `email` with `no-email`, `parallel` without a whole number from 1 up, any other argument starting with a dash, or an empty focus is an argument error (exit `2`). So is `merge`, `no-merge` or `parallel` with `--plan-only`, since nothing is dispatched for them to apply to.
 
 An Architect run:
 
@@ -371,7 +374,7 @@ An Architect run:
 4. Checks the plan: the issue is in this repository, is open, was created after the Architect run started, and carries no other label that says it is not agent work (`ready-for-human`, `needs-info` or `wontfix`).
 5. Marks the plan ready, in one request: `needs-triage` is swapped for `ready-for-agent`, and its other labels are kept. A Spec's Tickets are left as the review labelled them.
 6. Removes the worktree and the temporary plugin directory, whatever the outcome.
-7. Dispatches the plan, unless given `--plan-only`, exactly as `thirdshift <plan URL>` would from the same clone: a [Spec run](#spec-runs) when the plan has sub-issues, a Run otherwise. `merge` and `no-merge` apply to the Spec PR or the Run's pull request, and `parallel <n>` to the Spec run, as if given to that command, and the [User config](#user-config) sets what they leave unsaid: `merge.always`, `spec.parallel`, `launch.pull` and `logs.dir`. The one difference is that no [Run notification](#run-notifications) is sent, whatever `email.always` says.
+7. Dispatches the plan, unless given `--plan-only`, exactly as `thirdshift <plan URL>` would from the same clone: a [Spec run](#spec-runs) when the plan has sub-issues, a Run otherwise. `merge` and `no-merge` apply to the Spec PR or the Run's pull request, and `parallel <n>` to the Spec run, as if given to that command, and the [User config](#user-config) sets what they leave unsaid: `merge.always`, `spec.parallel`, `launch.pull` and `logs.dir`. The one difference is that it sends no [Run notification](#run-notifications) of its own, whatever `email.always` says: the Architect run sends [the one](#one-run-notification).
 
 The dispatched run's ending is the Architect run's: its exit code, its pull request's URL alone on stdout, and its last line on stderr, `PR <url> is ready for review` or `PR <url> is merged`. If it fails, the Architect run fails as that Failed run or Failed spec run does, with the cause and the session log on stderr and the pull request's URL on stdout if it left one. The plan stays `ready-for-agent`, for `thirdshift <plan URL>` to take up again. `parallel <n>` on a plan that is a single Ticket fails the same way as it does for `thirdshift <Issue URL>` on an issue that isn't a Spec: `parallel is only for a Spec, and #<n> has no sub-issues`, exit `1`, before any implementing, with the plan left ready to run without it.
 
@@ -381,6 +384,13 @@ Progress lines on stderr say when the review starts, which plan it reported, whe
 
 A review session that fails or is interrupted, a final message without one of the lines the prompt asks for, or a plan that fails a check ends the Architect run as a failure: exit `1`, nothing on stdout, the cause on stderr and then the path of the session log. Nothing is dispatched and no label is changed, so a plan the review did publish stays `needs-triage` for you to finish or close.
 
+### One Run notification
+
+An Architect run asked for a [Run notification](#run-notifications), by `email` or by `email.always = true` in the [User config](#user-config) without `no-email`, sends exactly one, whatever its outcome and with or without `--plan-only`. It makes a Run's checks before any other work, an address and a Resend API key, and stops with exit `1` if either is missing. The email goes after the outcome is final and printed, and a failed send is only a `warning:` line on stderr: it changes neither the exit code nor stdout.
+
+- **Subject**: `[thirdshift] <owner>/<repo> Architect run: <outcome>`. With a dispatched run, the outcome is that run's: `ready for review`, `merged`, `failed` or `interrupted`. Without one, it is the review's: `plan published` (with `--plan-only`), `idea filed`, `idea already filed`, `review failed` (also for a plan that fails a check) or `interrupted`. The repository is left out if `origin` doesn't name one on GitHub.
+- **Body**, plain text: a `Review:` line saying how the Architecture review ended, with the URL of the plan or idea issue it named (`plan published: <url>`, `idea filed: <url>`, `idea already filed: <url>`, `failed` or `interrupted`); when the plan was dispatched, a `Dispatched:` line with that run's outcome; then what a Run's notification holds, for the dispatched run or else the failed review: the pull request URL (if any), the failure cause (if failed), the session log path (if any), the hostname and how long the Architect run took. After a dispatched Spec run, it ends with a line per Ticket, as a Spec run's notification does.
+
 ### No Strong candidate
 
 Only a Strong top recommendation becomes a plan. When the review's top recommendation is Worth exploring or Speculative, it publishes no plan, and the Architect run ends in one of two ways, both a success, with or without `--plan-only`: exit `0`, with one issue's URL alone on stdout. thirdshift changes no label on that issue and dispatches nothing: there is nothing to run.

src/architect.rs

@@ -49,6 +49,16 @@ pub enum Outcome {
 }
 
 impl Outcome {
+    /// How the Architecture review ended, as the Architect run's Run
+    /// notification says it.
+    pub fn review(&self) -> &'static str {
+        match self {
+            Self::PlanReady(_) => "plan published",
+            Self::IdeaFiled(_) => "idea filed",
+            Self::AlreadyFiled(_) => "idea already filed",
+        }
+    }
+
     /// The URL of the issue the Architect run ended on.
     pub fn url(&self) -> &str {
         match self {
@@ -87,11 +97,7 @@ impl fmt::Display for Outcome {
 pub fn run(focus: Option<&str>, logs_dir: &Path, launch_pull: bool) -> Result<Outcome, FailedRun> {
     let started = Utc::now();
     let timestamp = started.format("%Y%m%dT%H%M%SZ").to_string();
-    let launch = Git::new(std::env::current_dir().context("no current directory")?);
-
-    let origin = launch.run(&["config", "remote.origin.url"])?;
-    let repo = Repo::of_origin(&origin)
-        .with_context(|| format!("origin {origin} is not a GitHub repository"))?;
+    let (launch, origin, repo) = launch()?;
     preflight::check_identity(&launch)?;
     let base = launch
         .run(&["symbolic-ref", "--quiet", "--short", "HEAD"])
@@ -116,6 +122,21 @@ pub fn run(focus: Option<&str>, logs_dir: &Path, launch_pull: bool) -> Result<Ou
         })
 }
 
+/// The Launch directory, the URL of its `origin`, and the GitHub repository
+/// that names: an Architect run's, as it has no Issue URL to name one.
+fn launch() -> Result<(Git, String, Repo)> {
+    let launch = Git::new(std::env::current_dir().context("no current directory")?);
+    let origin = launch.run(&["config", "remote.origin.url"])?;
+    let repo = Repo::of_origin(&origin)
+        .with_context(|| format!("origin {origin} is not a GitHub repository"))?;
+    Ok((launch, origin, repo))
+}
+
+/// The repository an Architect run started from the Launch directory is on.
+pub fn repo() -> Result<Repo> {
+    launch().map(|(_, _, repo)| repo)
+}
+
 /// The Architecture review session in `worktree`, which is removed once the
 /// session ends. Returns the session's final message. `log` is left at the
 /// most recent session's log.

src/args.rs

@@ -32,6 +32,9 @@ pub struct ArchitectArgs {
     /// The free text that points the Architecture review at an area, if
     /// given.
     pub focus: Option<String>,
+    /// What `email` or `no-email` asked for, if either was given; without
+    /// one, the User config decides.
+    pub email: Option<NotificationAsk>,
     /// The flags for the Spec run or Run the plan is dispatched as, or none
     /// with [`PLAN_ONLY`], which dispatches nothing.
     pub dispatch: Option<DispatchArgs>,
@@ -144,13 +147,16 @@ pub fn parse(args: &[String]) -> Result<Command> {
 /// Parse the arguments after `architect`: at most one focus, and its flags,
 /// each at most once, in any order. [`PLAN_ONLY`] dispatches nothing, so the
 /// flags for the dispatched run, `merge`, `no-merge` and `parallel`, which a
-/// Run takes too, can't go with it. They are never the focus, and any other
-/// argument that starts with a dash is unexpected rather than a focus.
+/// Run takes too, can't go with it. `email` and `no-email` are for the
+/// Architect run's own Run notification, so they can, and `email` takes an
+/// address as it does for a Run. None of these is ever the focus, and any
+/// other argument that starts with a dash is unexpected rather than a focus.
 fn parse_architect(args: &[String]) -> Result<ArchitectArgs> {
     let mut focus = None;
     let mut plan_only = false;
+    let mut email = None;
     let mut dispatch = DispatchArgs::default();
-    let mut args = args.iter();
+    let mut args = args.iter().peekable();
     while let Some(arg) = args.next() {
         match arg.as_str() {
             PLAN_ONLY => {
@@ -163,6 +169,13 @@ fn parse_architect(args: &[String]) -> Result<ArchitectArgs> {
             "no-merge" | "--no-merge" => {
                 ask_once(&mut dispatch.goal, Goal::ReadyForReview, arg, MERGE_FLAGS)?
             }
+            "email" | "--email" => {
+                let to = args.next_if(|next| is_address(next)).cloned();
+                ask_once(&mut email, NotificationAsk::Send(to), arg, EMAIL_FLAGS)?
+            }
+            "no-email" | "--no-email" => {
+                ask_once(&mut email, NotificationAsk::Skip, arg, EMAIL_FLAGS)?
+            }
             "parallel" | "--parallel" => ask_parallel(&mut dispatch.parallel, arg, args.next())?,
             _ if arg.starts_with('-') => bail!("unexpected argument after architect: {arg}"),
             _ if focus.is_some() => bail!("unexpected argument after the focus: {arg}"),
@@ -173,6 +186,7 @@ fn parse_architect(args: &[String]) -> Result<ArchitectArgs> {
     if !plan_only {
         return Ok(ArchitectArgs {
             focus,
+            email,
             dispatch: Some(dispatch),
         });
     }
@@ -184,6 +198,7 @@ fn parse_architect(args: &[String]) -> Result<ArchitectArgs> {
     }
     Ok(ArchitectArgs {
         focus,
+        email,
         dispatch: None,
     })
 }
@@ -220,7 +235,8 @@ fn ask_parallel(parallel: &mut Option<NonZeroUsize>, arg: &str, n: Option<&Strin
 }
 
 /// Is `arg`, after `email`, the address to send to? Only if it looks like
-/// one, so the Issue URL is never taken for it.
+/// one, so the Issue URL is never taken for it, nor is an Architect run's
+/// focus unless it has an `@`.
 fn is_address(arg: &str) -> bool {
     arg.contains('@') && !arg.starts_with("https://")
 }
@@ -331,6 +347,60 @@ mod tests {
         assert_eq!(architect_args(&args).focus.as_deref(), Some("the Spec run"));
     }
 
+    #[test]
+    fn architect_takes_the_email_flags_with_or_without_plan_only_and_never_as_the_focus() {
+        let bare = || Some(NotificationAsk::Send(None));
+        let to_me = || Some(NotificationAsk::Send(Some("me@example.com".to_string())));
+        let focus = || Some("the Spec run".to_string());
+        for (args, email, focus) in [
+            (vec!["architect"], None, None),
+            (vec!["architect", "email"], bare(), None),
+            (vec!["architect", "--email", "--plan-only"], bare(), None),
+            (vec!["architect", "email", "the Spec run"], bare(), focus()),
+            (vec!["architect", "email", "merge"], bare(), None),
+            (vec!["architect", "email", "me@example.com"], to_me(), None),
+            (
+                vec!["architect", "the Spec run", "--email", "me@example.com"],
+                to_me(),
+                focus(),
+            ),
+            (
+                vec!["architect", "--plan-only", "no-email"],
+                Some(NotificationAsk::Skip),
+                None,
+            ),
+            (
+                vec!["architect", "--no-email", "the Spec run"],
+                Some(NotificationAsk::Skip),
+                focus(),
+            ),
+        ] {
+            let architect_args = architect_args(&args);
+            assert_eq!(architect_args.email, email, "{args:?}");
+            assert_eq!(architect_args.focus, focus, "{args:?}");
+        }
+    }
+
+    #[test]
+    fn architect_rejects_contradictory_and_repeated_email_flags() {
+        for (args, error) in [
+            (
+                vec!["architect", "email", "--no-email"],
+                "email and no-email can't be used together",
+            ),
+            (
+                vec!["architect", "no-email", "--plan-only", "--email"],
+                "email and no-email can't be used together",
+            ),
+            (
+                vec!["architect", "--email", "me@example.com", "email"],
+                "repeated argument: email",
+            ),
+        ] {
+            assert_eq!(rejection(&args), error, "{args:?}");
+        }
+    }
+
     #[test]
     fn architect_rejects_contradictory_repeated_and_malformed_dispatch_flags() {
         for (args, error) in [
@@ -391,8 +461,8 @@ mod tests {
                 "repeated argument: --plan-only",
             ),
             (
-                vec!["architect", "--email", "--plan-only"],
-                "unexpected argument after architect: --email",
+                vec!["architect", "--verbose", "--plan-only"],
+                "unexpected argument after architect: --verbose",
             ),
             (vec!["architect", " ", "--plan-only"], "the focus is empty"),
         ] {