From a7b8d406b97079f87018b751c45e603320ca4c53 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Mon, 26 Oct 2026 20:51:27 -0400 Subject: [PATCH 1/2] Show a Process by name and send its Run notification (#1387) status, show, the Console and the Activity log name a Custom process as Process . A succeeded, failed or interrupted Process sends a Run notification under the same email rules as a Ticket run. A skipped one sends none. The stored kind stays Process. --- README.md | 2 +- docs/guide.md | 13 +- site/docs/index.html | 12 +- site/how-it-works/index.html | 2 +- site/index.html | 2 +- src/args.rs | 87 ++++-- src/logs/command_record.rs | 7 + src/logs/status.rs | 29 +- src/logs/status/inbox.rs | 28 +- src/logs/status/inspect.rs | 60 +++- src/main.rs | 2 +- src/notification.rs | 65 +++- src/processes.rs | 66 ++++- tests/process.rs | 555 +++++++++++++++++++++++++++++++++++ 14 files changed, 871 insertions(+), 59 deletions(-) diff --git a/README.md b/README.md index fb96b528..6890a2aa 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Use `thirdshift help` for the command reference, or read [Ticket run behavior](d | A security audit | `thirdshift secure` | A [Security run](docs/guide.md#security-runs) records findings privately; fixing a reproduced finding is optional. | | Merged factory work | `thirdshift score` | A [Scoring run](docs/guide.md#scoring-runs) samples and grades it, and proposes Bug origins; `thirdshift scores --json` lists samples, five criterion Scores, calibration and proposals by week; `thirdshift scores --mine` records yours before revealing the model's. | | One skill | `thirdshift skill [issue ]` | A [standalone skill](docs/guide.md#standalone-skills) runs headlessly and prints its final message. | -| A Custom process | `thirdshift process [issue ]` | An experimental [Custom process](docs/guide.md#custom-processes) runs skill or command Steps from `~/.thirdshift/processes/.toml`, in named throwaway worktrees when the file declares them. An optional Step can fail without stopping the process. `issue ` gives every session Step that issue. | +| A Custom process | `thirdshift process [issue ]` | An experimental [Custom process](docs/guide.md#custom-processes) runs skill or command Steps from `~/.thirdshift/processes/.toml`, in named throwaway worktrees when the file declares them. An optional Step can fail without stopping the process. `issue ` gives every session Step that issue. Status, the Console and the Activity log list it as Process ``. A failed one is in Needs you, and it sends a Run notification under the same email rules as a Ticket run. A skipped Process sends none. | Each Architect, Pickup or Security run makes one pass. Use cron or another operating system scheduler to repeat it. See [scheduling](docs/guide.md#on-a-schedule), [Architect runs every few minutes](docs/guide.md#architect-runs-every-few-minutes) and [Security runs on a schedule](docs/guide.md#security-runs-on-a-schedule-with-fixing-allowed). diff --git a/docs/guide.md b/docs/guide.md index 6c588132..cc9656b7 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -505,7 +505,7 @@ thirdshift secure # audit the Base branch and record findings thirdshift secure base # do that with as the Base branch, from a clone on any branch thirdshift skill [issue ] # run one skill headlessly and print its final message thirdshift process # list Custom processes (experimental) -thirdshift process [issue ] # run a Custom process, optionally on an issue (experimental) +thirdshift process [issue ] # run a Custom process, optionally on an issue; email flags match a Ticket run (experimental) thirdshift process check # validate one and print its plan (experimental) thirdshift score # sample merged factory pull requests (see Scoring runs) thirdshift scores [] [--json] # show Scores and calibration @@ -1003,6 +1003,17 @@ command that exits non-zero is failed, and the line names its exit status. Stop and Ctrl-C interrupt a command Step the way they interrupt a session, and `[resources]` is checked before each Step. +`thirdshift status` and `status --json` list a running or finished Process as +Process ``, with the ending succeeded, failed, skipped or interrupted. +The stored kind stays `Process`, and the selector used by `show` and `stop` +stays `process--`. `thirdshift show` prints each Step's Command +log line and, when that Step wrote a result, the path of the result file. The +Console lists the Process, and a failed one is in Needs you. The Activity log +says Process ``. A succeeded, failed or interrupted Process sends a Run +notification under the same `email`, `email-unless-merged` and `no-email` +flags and the same User config rules as a Ticket run. A missing address or +Resend key stops the Process before any Step. A skipped Process sends none. + ### User config A **User config** at `~/.thirdshift/config.toml` sets this machine's defaults for every Ticket run. It is optional: with no file, or one that says nothing about a setting, a Ticket run does only what its command asks for. diff --git a/site/docs/index.html b/site/docs/index.html index d3e4a8fe..e55c0e09 100644 --- a/site/docs/index.html +++ b/site/docs/index.html @@ -464,7 +464,7 @@ thirdshift secure base <branch> # do that with <branch> as the Base branch, from a clone on any branch thirdshift skill <skill> [issue <issue>] # run one skill headlessly and print its final message thirdshift process # list Custom processes (experimental) -thirdshift process <name> [issue <issue>] # run a Custom process, optionally on an issue (experimental) +thirdshift process <name> [issue <issue>] # run a Custom process, optionally on an issue; email flags match a Ticket run (experimental) thirdshift process check <name> # validate one and print its plan (experimental) thirdshift score # sample merged factory pull requests (see Scoring runs) thirdshift scores [<pull request>] [--json] # show Scores and calibration @@ -911,6 +911,16 @@ command that exits non-zero is failed, and the line names its exit status. Stop and Ctrl-C interrupt a command Step the way they interrupt a session, and [resources] is checked before each Step.

+

thirdshift status and status --json list a running or finished Process as +Process <name>, with the ending succeeded, failed, skipped or interrupted. +The stored kind stays Process, and the selector used by show and stop +stays process-<name>-<stamp>. thirdshift show prints each Step's Command +log line and, when that Step wrote a result, the path of the result file. The +Console lists the Process, and a failed one is in Needs you. The Activity log +says Process <name>. A succeeded, failed or interrupted Process sends a Run +notification under the same email, email-unless-merged and no-email +flags and the same User config rules as a Ticket run. A missing address or +Resend key stops the Process before any Step. A skipped Process sends none.

User config
diff --git a/site/how-it-works/index.html b/site/how-it-works/index.html index b0a026d9..8e571a11 100644 --- a/site/how-it-works/index.html +++ b/site/how-it-works/index.html @@ -211,7 +211,7 @@

How thirdshift works

A direct Issue URL starts work you have already chosen. Other commands find work, then dispatch the same Ticket run or Spec run you just followed. Each invocation makes one pass; your operating system's scheduler starts the next one.

To run one skill directly, use thirdshift skill <skill> [issue <issue>]. It starts one headless session on your default Harness, Model and Effort in a temporary worktree at your checkout's committed HEAD, and prints the final message. Worktrees with local work are kept. Standalone skills.

-

thirdshift process is experimental, beside thirdshift skill. It runs the Steps in ~/.thirdshift/processes/<name>.toml, a skill session or a command. Consecutive Steps can run together, each in a throwaway worktree the file names, and the last finished Step's result is printed. An optional Step can fail without stopping the process. thirdshift process <name> issue <issue> gives the process an issue, so each session Step works from that issue. Custom processes.

+

thirdshift process is experimental, beside thirdshift skill. It runs the Steps in ~/.thirdshift/processes/<name>.toml, a skill session or a command. Consecutive Steps can run together, each in a throwaway worktree the file names, and the last finished Step's result is printed. An optional Step can fail without stopping the process. thirdshift process <name> issue <issue> gives the process an issue, so each session Step works from that issue. Status, the Console and the Activity log list it as Process <name>, and a failed one is in Needs you. It sends a Run notification under the same email rules as a Ticket run, except a skipped Process sends none. Custom processes.

Different entry points, the same implementation and Delivery
diff --git a/site/index.html b/site/index.html index 2fc7386a..dd0c7d7e 100644 --- a/site/index.html +++ b/site/index.html @@ -91,7 +91,7 @@

Your issues become pull requests overnight.

  1. /thirdshift-implement
  2. /thirdshift-code-review
  3. /thirdshift-pr

thirdshift takes an Issue URL: one issue starts a Ticket run; an issue with sub-issues starts a Spec run. Its Factory skills, adapted to run with no human in the loop, implement the issue with /thirdshift-implement, review the work with /thirdshift-code-review and write the pull request with /thirdshift-pr.

Issues labelled bug first use /thirdshift-diagnosing-bugs to reproduce the reported failure and write a failing regression test before fixing it. If it cannot reproduce the bug, the Ticket run leaves triage notes and needs-info on the issue, ending successfully without a new pull request. Security fixes and Base fixes keep their own flow.

-

thirdshift skill runs one skill. thirdshift process is experimental: it runs skill or command Steps from ~/.thirdshift/processes, including Steps that run together in throwaway worktrees the file names. An optional Step can fail without stopping the process. Pass issue to give the process an issue, so each session Step works from that issue. Custom processes.

+

thirdshift skill runs one skill. thirdshift process is experimental: it runs skill or command Steps from ~/.thirdshift/processes, including Steps that run together in throwaway worktrees the file names. An optional Step can fail without stopping the process. Pass issue to give the process an issue, so each session Step works from that issue. Status, the Console and the Activity log list it as Process <name>, and a failed one is in Needs you. It sends a Run notification under the same email rules as a Ticket run, except a skipped Process sends none. Custom processes.

Read every prompt and skill the agents get →

diff --git a/src/args.rs b/src/args.rs index 41550efb..51b0f74b 100644 --- a/src/args.rs +++ b/src/args.rs @@ -24,6 +24,8 @@ pub enum ProcessAction { Run { name: String, issue: Option, + /// `email`, `email-unless-merged` or `no-email`, when the command gave one. + email: Option, }, } @@ -835,32 +837,81 @@ fn metrics(args: &[String]) -> Result { Ok(asked) } +const PROCESS_USAGE: &str = "usage: thirdshift process [check | [issue ] [email [address] | email-unless-merged [address] | no-email]]"; + fn parse_process(args: &[String]) -> Result { - match args { - [] => Ok(ProcessAction::List), - [keyword] if keyword == "check" => bail!("usage: thirdshift process check "), - [name] => Ok(ProcessAction::Run { - name: process_name(name)?, - issue: None, - }), - [keyword, name] if keyword == "check" => Ok(ProcessAction::Check(process_name(name)?)), - [name, keyword] if keyword == "issue" => { - process_name(name)?; - bail!("usage: thirdshift process [issue ]"); - } - [name, keyword, issue] if keyword == "issue" => Ok(ProcessAction::Run { - name: process_name(name)?, - issue: Some(IssueInput::parse(issue)?), - }), - _ => bail!("usage: thirdshift process [check | [issue ]]"), + if args.is_empty() { + return Ok(ProcessAction::List); + } + let mut name = None; + let mut issue = None; + let mut email = None; + let mut check = false; + let mut expect_issue = false; + let mut words = args.iter().peekable(); + while let Some(arg) = words.next() { + if take_email_flag(&mut email, arg, &mut words)? { + continue; + } + // `check` is the subcommand only before the name. A later `check` is + // the name, so a reserved file called check is still refused as one. + if arg == "check" && !check && name.is_none() && issue.is_none() && email.is_none() { + check = true; + continue; + } + if arg == "issue" { + if check || name.is_none() || expect_issue || issue.is_some() { + bail!(PROCESS_USAGE); + } + expect_issue = true; + continue; + } + if expect_issue { + issue = Some(IssueInput::parse(arg)?); + expect_issue = false; + continue; + } + if name.is_some() || arg.starts_with('-') { + bail!(PROCESS_USAGE); + } + name = Some(process_name(arg)?); } + if expect_issue || (email.is_some() && (check || name.is_none())) { + bail!(PROCESS_USAGE); + } + match (check, name) { + (true, Some(name)) if issue.is_none() => Ok(ProcessAction::Check(name)), + (true, _) => bail!(PROCESS_USAGE), + (false, Some(name)) => Ok(ProcessAction::Run { name, issue, email }), + (false, None) => bail!(PROCESS_USAGE), + } +} + +/// Record an email flag in `email`. False, taking nothing, when `arg` is not one. +fn take_email_flag<'a>( + email: &mut Option, + arg: &str, + rest: &mut Peekable>, +) -> Result { + let asked = match arg { + "email" | "--email" => { + NotificationAsk::Send(rest.next_if(|next| is_address(next)).cloned()) + } + "email-unless-merged" | "--email-unless-merged" => { + NotificationAsk::SendUnlessMerged(rest.next_if(|next| is_address(next)).cloned()) + } + "no-email" | "--no-email" => NotificationAsk::Skip, + _ => return Ok(false), + }; + ask_once(email, asked, arg, EMAIL_FLAGS)?; + Ok(true) } /// A process name is one path component. A slash is an argument error, not a /// file rule: the file is `~/.thirdshift/processes/.toml`. fn process_name(name: &str) -> Result { if name.is_empty() || name == "." || name == ".." || name.contains(['/', '\\', '\0']) { - bail!("usage: thirdshift process [check | [issue ]]"); + bail!(PROCESS_USAGE); } Ok(name.to_string()) } diff --git a/src/logs/command_record.rs b/src/logs/command_record.rs index d8f4a3e1..f9d90539 100644 --- a/src/logs/command_record.rs +++ b/src/logs/command_record.rs @@ -101,6 +101,9 @@ pub(super) struct CommandRecord { pub security_findings: Vec, #[serde(default, skip_serializing_if = "Option::is_none")] pub experiment: Option, + /// The Custom process name. The selector in `name` stays `process--`. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub process_name: Option, } #[derive(Clone, Serialize, Deserialize)] @@ -384,6 +387,10 @@ impl CommandRecord { security_findings_waiting: false, security_findings: Vec::new(), experiment: None, + process_name: match work { + Work::Process { name, .. } => Some(name.to_string()), + _ => None, + }, } } diff --git a/src/logs/status.rs b/src/logs/status.rs index e9652872..2ad1d1e6 100644 --- a/src/logs/status.rs +++ b/src/logs/status.rs @@ -37,14 +37,27 @@ struct Status<'a> { fn print_command(command: &Command, state_word: &str, indent: &str) { let facts = &command.record; let stage = stage_text(command); - let mut subject = match &facts.issue { - Some(issue) => format!( - "{}#{} {}", - facts.repository, - issue.number, - issue.title.as_deref().unwrap_or("(title unavailable)") - ), - None => format!("{} on {}", facts.name, facts.repository), + let mut subject = if facts.kind == "Process" { + let name = facts.process_name.as_deref().unwrap_or(facts.name.as_str()); + match &facts.issue { + Some(issue) => format!( + "{name} {}#{} {}", + facts.repository, + issue.number, + issue.title.as_deref().unwrap_or("(title unavailable)") + ), + None => format!("{name} on {}", facts.repository), + } + } else { + match &facts.issue { + Some(issue) => format!( + "{}#{} {}", + facts.repository, + issue.number, + issue.title.as_deref().unwrap_or("(title unavailable)") + ), + None => format!("{} on {}", facts.name, facts.repository), + } }; if let Some(pull_request) = &facts.pull_request { subject.push_str(&format!(" · {pull_request}")); diff --git a/src/logs/status/inbox.rs b/src/logs/status/inbox.rs index c864e7a6..784f82b2 100644 --- a/src/logs/status/inbox.rs +++ b/src/logs/status/inbox.rs @@ -292,11 +292,7 @@ pub(crate) fn inbox(config: &UserConfig) -> Result { let Observation::Record(command) = item.command else { return None; }; - let (subject, _) = subject( - &command.record.repository, - &command.record.name, - command.record.issue.as_ref(), - ); + let (subject, _) = row_subject(&command.record); Some(format!( "{subject} · {} · see Needs you", item.needs_you_kind.expect("Needs you has a kind").title() @@ -331,6 +327,26 @@ pub(crate) fn inbox(config: &UserConfig) -> Result { }) } +fn row_subject(facts: &crate::logs::command_record::CommandRecord) -> (String, String) { + if facts.kind == "Process" { + let name = facts.process_name.as_deref().unwrap_or(facts.name.as_str()); + return match facts.issue.as_ref() { + Some(issue) => ( + format!("Process {name} · {}#{}", facts.repository, issue.number), + issue + .title + .clone() + .unwrap_or_else(|| "(title unavailable)".into()), + ), + None => ( + format!("Process {name} on {}", facts.repository), + String::new(), + ), + }; + } + subject(&facts.repository, &facts.name, facts.issue.as_ref()) +} + fn subject(repository: &str, name: &str, issue: Option<&Issue>) -> (String, String) { match issue { Some(issue) => ( @@ -357,7 +373,7 @@ fn row(command: &Command, now: DateTime, root: &Path) -> Result { let facts = &command.record; let (log_length, session_length) = command.activity_lengths(); let live = command.state == "live"; - let (subject, title) = subject(&facts.repository, &facts.name, facts.issue.as_ref()); + let (subject, title) = row_subject(facts); Ok(Row { // A Pass's dispatched Runs share its storage path. Their immutable // Command facts distinguish them without tying identity to ordering. diff --git a/src/logs/status/inspect.rs b/src/logs/status/inspect.rs index 391afd81..3d665c18 100644 --- a/src/logs/status/inspect.rs +++ b/src/logs/status/inspect.rs @@ -40,6 +40,60 @@ pub(crate) fn url(route: &str, command: &str, session: Option<&std::path::Path>) format!("{}?{}", url.path(), url.query().unwrap_or_default()) } +fn shown_heading(command: &Command) -> String { + if command.record.kind == "Process" { + return format!( + "Process {}", + command + .record + .process_name + .as_deref() + .unwrap_or(command.record.name.as_str()) + ); + } + format!( + "{} {}", + crate::logs::display_name(&command.record.kind), + command.record.name + ) +} + +/// Each Process Step's Command log line, and the result file when the Step wrote one. +fn print_steps(command: &Command, indent: &str) { + if command.record.kind != "Process" { + return; + } + let Ok(log) = std::fs::read_to_string(&command.command_log) else { + return; + }; + let results = command.command_log.with_extension("steps"); + for line in log.lines() { + let Some(message) = step_message(line) else { + continue; + }; + println!("{indent}{message}"); + let Some(name) = step_name(message) else { + continue; + }; + let path = results.join(format!("{name}.txt")); + if path.is_file() { + println!("{indent}result: {}", path.display()); + } + } +} + +fn step_message(line: &str) -> Option<&str> { + let rest = line.strip_prefix("thirdshift: ")?; + let (_, message) = rest.split_once(' ')?; + message.starts_with("step ").then_some(message) +} + +fn step_name(message: &str) -> Option<&str> { + let rest = message.strip_prefix("step ")?; + let (name, _) = rest.split_once(':')?; + (!name.is_empty() && !name.contains(' ')).then_some(name) +} + fn print(command: Observation<'_>) -> Result<()> { match command { Observation::Record(command) => print_details(command, "")?, @@ -58,9 +112,8 @@ fn print(command: Observation<'_>) -> Result<()> { fn print_details(command: &Command, indent: &str) -> Result<()> { println!( - "{indent}{} {} · {}", - crate::logs::display_name(&command.record.kind), - command.record.name, + "{indent}{} · {}", + shown_heading(command), super::stage_text(command) ); if let Some(issue) = &command.record.issue { @@ -87,6 +140,7 @@ fn print_details(command: &Command, indent: &str) -> Result<()> { if let Some(event) = command.latest_event() { println!("{indent}{event}"); } + print_steps(command, indent); super::print_tickets(&command.record, &format!("{indent} ")); for child in command.dispatched.iter().chain(&command.children) { print_details(child, &format!("{indent} "))?; diff --git a/src/main.rs b/src/main.rs index f386e6c8..f4fbf9b3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -92,7 +92,7 @@ usage: thirdshift Run the factory on the iss thirdshift secure base Use as the Base branch thirdshift skill [issue ] Run one skill and print its final message thirdshift process List Custom processes (experimental) - thirdshift process [issue ] Run a Custom process (experimental) + thirdshift process [issue ] Run a Custom process; email flags match a Ticket run (experimental) thirdshift process check Validate a Custom process and print its plan (experimental) thirdshift score Sample merged factory work and propose Bug origins thirdshift scores [] [--mine] [--json] Show Scores; --mine enters yours diff --git a/src/notification.rs b/src/notification.rs index 6ef26a08..7d490120 100644 --- a/src/notification.rs +++ b/src/notification.rs @@ -1,6 +1,6 @@ //! The Run notification: saved beside the Command log when a Run, a Spec -//! run or a Pass ends, unless the Pass was skipped, and optionally emailed -//! through the same checks and the same send as `email-test`. +//! run, a Pass or a Process ends, unless the Pass or Process was skipped, +//! and optionally emailed through the same checks and the same send as `email-test`. use std::path::Path; use std::time::{Duration, Instant}; @@ -43,6 +43,10 @@ pub enum About<'a> { /// A Security run, from the Launch directory. SecurityRun, ScoringRun, + /// A Custom process, named by its file, from the Launch directory. + Process { + name: &'a str, + }, } /// What a Run notification's subject names. @@ -64,16 +68,23 @@ enum Subject { ScoringRun, /// A Pass whose notification subject has not been established yet. Pending, + /// A Custom process, with the repository the Launch directory's `origin` + /// names, if it names one. + Process { + name: String, + repo: Option, + }, } impl Subject { /// The Command a notification about an Architect or Security run is /// about, by name, with its repository if known. - fn command(&self) -> Option<(&'static str, Option<&Repo>)> { + fn command(&self) -> Option<(String, Option<&Repo>)> { match self { - Subject::ArchitectRun(repo) => Some(("Architect run", repo.as_ref())), - Subject::SecurityRun(repo) => Some(("Security run", repo.as_ref())), - Subject::ScoringRun => Some(("Scoring run", None)), + Subject::ArchitectRun(repo) => Some(("Architect run".into(), repo.as_ref())), + Subject::SecurityRun(repo) => Some(("Security run".into(), repo.as_ref())), + Subject::ScoringRun => Some(("Scoring run".into(), None)), + Subject::Process { name, repo } => Some((format!("Process {name}"), repo.as_ref())), Subject::Issue { .. } | Subject::Pending => None, } } @@ -129,6 +140,11 @@ impl RunNotification { About::PickupRun => Subject::Pending, About::SecurityRun => Subject::SecurityRun(launch::repo().ok()), About::ScoringRun => Subject::ScoringRun, + // The command subject names the process. No issue title is read. + About::Process { name } => Subject::Process { + name: name.to_string(), + repo: launch::repo().ok(), + }, }; Ok(RunNotification { resend, @@ -223,14 +239,18 @@ impl RunNotification { } /// The subject of a notification about `subject` for a command whose -/// outcome is `outcome`: none about a Pickup run that took no issue. +/// outcome is `outcome`: none about a Pickup run that took no issue, and +/// none about a skipped Process. fn subject_line(subject: &Subject, outcome: &str) -> Option { + if matches!(subject, Subject::Process { .. }) && outcome == "skipped" { + return None; + } if let Subject::Issue { issue, title, .. } = subject { return Some(issue_subject(issue, title.as_deref(), outcome)); } subject .command() - .map(|(command, repo)| command_subject(command, repo, outcome)) + .map(|(command, repo)| command_subject(&command, repo, outcome)) } /// `[thirdshift] /# : <outcome>`, without the title @@ -292,7 +312,7 @@ fn body( text += &command_lines(command, None); } if let Some((command, repo)) = subject.command() { - text += &command_lines(command, repo); + text += &command_lines(&command, repo); } if let Some(findings) = account.security_findings { // A fix's ending is told by the Result and Pull request lines. @@ -1084,4 +1104,31 @@ mod tests { "[thirdshift] Architect run: review failed" ); } + + fn about_process(outcome: &'static str) -> (Subject, Account<'static>) { + let subject = Subject::Process { + name: "notes".into(), + repo: Repo::of_origin("https://github.com/acme/widgets.git"), + }; + let account = Account { + outcome, + interrupted: outcome == "interrupted", + ..failed("exit status 1") + }; + (subject, account) + } + + #[test] + fn a_skipped_process_sends_nothing() { + let (subject, skipped) = about_process("skipped"); + assert_eq!(told(&subject, &skipped), None); + let (subject, failed_process) = about_process("failed"); + let (line, text) = told(&subject, &failed_process).unwrap(); + assert_eq!(line, "[thirdshift] acme/widgets Process notes: failed"); + assert!(text.contains("Command: Process notes\n"), "{text}"); + assert!(text.contains("Cause: exit status 1\n"), "{text}"); + let (subject, interrupted) = about_process("interrupted"); + let (_, text) = told(&subject, &interrupted).unwrap(); + assert!(!text.contains("Cause:"), "{text}"); + } } diff --git a/src/processes.rs b/src/processes.rs index 23740082..b3d5581f 100644 --- a/src/processes.rs +++ b/src/processes.rs @@ -17,12 +17,15 @@ use chrono::{DateTime, Utc}; use toml_edit::{Document, Item, Key, Table}; use crate::args::ProcessAction; +use crate::asks::Flags; use crate::config::UserConfig; use crate::git::Git; use crate::harness::{Asked, Choice}; use crate::issue::{IssueInput, IssueUrl}; use crate::logs::{self, Begin, Work}; +use crate::notification::{About, NotificationAsk, RunNotification}; use crate::process::{self, Control, Interruption}; +use crate::run_ending::{Account, Cause}; use crate::skill; use crate::worktree::SkillWorktree; use crate::{delivery, interrupt, launch, progress, prompt, session, skills, trust}; @@ -118,7 +121,7 @@ pub fn execute(action: ProcessAction) -> Result<String> { match action { ProcessAction::List => list(), ProcessAction::Check(name) => check(&name), - ProcessAction::Run { name, issue } => run(&name, issue), + ProcessAction::Run { name, issue, email } => run(&name, issue, email), } } @@ -167,9 +170,22 @@ fn check(name: &str) -> Result<String> { Ok(plan(name, &prepared)) } -fn run(name: &str, issue: Option<IssueInput>) -> Result<String> { +fn run(name: &str, issue: Option<IssueInput>, email: Option<NotificationAsk>) -> Result<String> { interrupt::install()?; let config = UserConfig::load()?; + logs::begin(Begin::Process(name)); + logs::configured(&config); + // The same check as a Ticket run, before any worktree or Step: a missing + // address or Resend key fails the Process before it starts. + let mut notification = RunNotification::new( + Flags { + email, + ..Flags::default() + } + .notification(&config), + &config.email, + About::Process { name }, + )?; // Before any worktree: a foreign Issue URL is an origin mismatch, and a // number that cannot be resolved stops the same way. let issue = resolve_process_issue(issue)?; @@ -180,17 +196,49 @@ fn run(name: &str, issue: Option<IssueInput>) -> Result<String> { prepared.issue = issue; prepared.started = started; let repo = launch::repo()?; - logs::begin(Begin::Process(name)); - logs::configured(&config); logs::started(Work::Process { repo: &repo, name }, &prepared.choice); + notification.built_with(&prepared.choice); let result = execute_steps(name, &prepared); let (outcome, cause) = classify(&result); drop(prepared); - logs::ended(&outcome); - logs::complete(&outcome, cause.as_deref()); + logs::ended(outcome); + logs::complete(outcome, cause.as_deref()); + notify(notification, outcome, cause.as_deref()); result } +/// Save and, when asked, send the Run notification. A skipped Process sends none. +fn notify(notification: RunNotification, outcome: &'static str, cause: Option<&str>) { + if outcome == "skipped" { + return; + } + let ended = if outcome == "succeeded" { + Ok(outcome.to_string()) + } else { + Err(Cause::of(&anyhow!("{}", cause.unwrap_or(outcome)))) + }; + let account = Account { + recovery: None, + outcome, + ended, + interrupted: outcome == "interrupted", + pr_url: None, + advice: &[], + base_fix: None, + log: None, + ticket_lines: &[], + review: None, + failed_reproduction: None, + pre_audit_failure: None, + security_findings: None, + security_findings_waiting: false, + security_fix: None, + security_close: None, + urls: Vec::new(), + }; + notification.finish(&account); +} + fn execute_steps(process: &str, prepared: &Prepared) -> Result<String> { let adapter = prepared.choice.harness.adapter(); for worktree in &prepared.worktrees { @@ -539,10 +587,10 @@ fn worktree_path<'a>(worktrees: &'a [OwnedWorktree], name: Option<&str>) -> &'a .path() } -fn classify(result: &Result<String>) -> (String, Option<String>) { +fn classify(result: &Result<String>) -> (&'static str, Option<String>) { match result { - Ok(_) => ("succeeded".to_string(), None), - Err(error) => (failure_outcome().to_string(), Some(one_line(error))), + Ok(_) => ("succeeded", None), + Err(error) => (failure_outcome(), Some(one_line(error))), } } diff --git a/tests/process.rs b/tests/process.rs index c5f59ba6..c91e6ac7 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -3,9 +3,13 @@ mod support; use std::fs; +use std::io::{BufRead, BufReader}; use std::os::unix::fs::symlink; use std::path::Path; +use std::process::{Command, Stdio}; +use std::time::Duration; +use support::resend::{ResendStandIn, saved_notification}; use support::{RunResult, Scenario, leaves_running}; const REVIEW: &str = "\ @@ -2237,3 +2241,554 @@ fn step_line<'a>(log: &'a str, name: &str) -> &'a str { .find(|line| line.contains(&format!("step {name}:"))) .unwrap_or_else(|| panic!("no step {name} in {log}")) } + +const NOTES: &str = "\ +version = 1 +description = \"Notes\" + +[[steps]] +name = \"gather\" +command = \"printf gather\" + +[[steps]] +name = \"review\" +command = \"printf review\" +"; + +const KEY: &str = "re_test_123"; +const ACCEPTED: &str = r#"{"id":"49a3999c-0ce1-4ea6-ab68-afcd6dc2e794"}"#; + +fn process_command(status: &serde_json::Value) -> &serde_json::Value { + status["commands"] + .as_array() + .unwrap() + .iter() + .find(|command| command["kind"] == "Process") + .unwrap_or_else(|| panic!("no Process in {status}")) +} + +fn status_json(scenario: &Scenario) -> serde_json::Value { + let json = scenario.run(&["status", "--json"]); + assert_eq!(json.code, Some(0), "{}", json.stderr); + serde_json::from_str(&json.stdout).unwrap() +} + +/// The step message after its `thirdshift: HH:MM:SS` stamp. +fn step_message(line: &str) -> &str { + let rest = line.strip_prefix("thirdshift: ").unwrap_or(line); + rest.split_once(' ') + .map(|(_, message)| message) + .unwrap_or(rest) +} + +fn activity_log(scenario: &Scenario) -> String { + fs::read_to_string(scenario.path("home/.thirdshift/logs/acme/widgets/activity.log")).unwrap() +} + +struct Console { + child: std::process::Child, + address: String, +} + +impl Console { + fn start(scenario: &Scenario) -> Self { + let mut child = Command::new(env!("CARGO_BIN_EXE_thirdshift")) + .args(["console", "--no-sign-in"]) + .env("HOME", scenario.path("home")) + .env_remove("RESEND_API_KEY") + .stdout(Stdio::piped()) + .stderr(Stdio::inherit()) + .spawn() + .unwrap(); + let stdout = child.stdout.take().unwrap(); + let mut lines = BufReader::new(stdout).lines(); + let address = lines + .next() + .expect("Console startup") + .expect("Console address"); + assert!(address.starts_with("http://127.0.0.1:"), "{address}"); + // Keep the pipe reader alive so the Console can keep writing. + std::thread::spawn(move || while lines.next().is_some() {}); + Self { child, address } + } + + fn get(&self, path: &str) -> String { + reqwest::blocking::Client::builder() + .timeout(Duration::from_secs(5)) + .build() + .unwrap() + .get(format!("{}{path}", self.address)) + .send() + .unwrap() + .text() + .unwrap() + } +} + +impl Drop for Console { + fn drop(&mut self) { + let _ = self.child.kill(); + let _ = self.child.wait(); + } +} + +#[test] +fn status_and_show_name_a_finished_process_and_where_each_step_result_is() { + let scenario = Scenario::new(); + write_process(&scenario, "notes", NOTES); + + let result = scenario.run(&["process", "notes"]); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert_eq!(result.stdout, "review\n"); + let text = scenario.run(&["status"]); + assert_eq!(text.code, Some(0), "{}", text.stderr); + assert!( + text.stdout.contains("Done:\nProcess notes on acme/widgets"), + "{}", + text.stdout + ); + assert!(text.stdout.contains("· succeeded"), "{}", text.stdout); + let status = status_json(&scenario); + let command = process_command(&status); + assert_eq!(command["kind"], "Process"); + assert_eq!(command["process_name"], "notes"); + assert!( + command["name"] + .as_str() + .unwrap() + .starts_with("process-notes-"), + "{}", + command["name"] + ); + assert_eq!(command["outcome"], "succeeded"); + assert_eq!(command["state"], "ended"); + assert_eq!(command["group"], "done"); + let name = command["name"].as_str().unwrap(); + let shown = scenario.run(&["show", name]); + assert_eq!(shown.code, Some(0), "{}", shown.stderr); + assert!( + shown.stdout.starts_with("Process notes · succeeded\n"), + "{}", + shown.stdout + ); + let (log_path, log) = command_log(&scenario); + for step in ["gather", "review"] { + assert!( + shown.stdout.contains(step_message(step_line(&log, step))), + "{step}: {}", + shown.stdout + ); + let result_path = log_path.with_extension("steps").join(format!("{step}.txt")); + assert!( + shown + .stdout + .contains(&format!("result: {}", result_path.display())), + "{step}: {}", + shown.stdout + ); + } + let activity = activity_log(&scenario); + assert!(activity.contains("Process notes started:"), "{activity}"); + assert!( + activity.contains("Process notes ended: succeeded"), + "{activity}" + ); + assert_eq!(process_record(&scenario)["kind"], "Process"); +} + +#[test] +fn status_lists_a_running_process_by_name_until_it_is_interrupted() { + let scenario = Scenario::new(); + let started = scenario.path("started"); + write_process( + &scenario, + "notes", + &format!( + "\ +version = 1 +description = \"Loop until stopped\" + +[[steps]] +name = \"wait\" +command = \"touch '{}'; while true; do sleep 0.05; done\" +", + started.display() + ), + ); + let mut held = scenario.run_until(&["process", "notes"], &[], "started"); + let text = scenario.run(&["status"]); + let status = status_json(&scenario); + let command = process_command(&status); + let name = command["name"].as_str().unwrap().to_string(); + let stopped = scenario.run(&["stop", &name]); + if stopped.code != Some(0) { + held.signal("TERM"); + } + let ended = held.finish(); + + assert_eq!(text.code, Some(0), "{}", text.stderr); + assert!( + text.stdout + .contains("Working:\nProcess notes on acme/widgets"), + "{}", + text.stdout + ); + assert!(text.stdout.contains("· live ·"), "{}", text.stdout); + assert_eq!(command["kind"], "Process"); + assert_eq!(command["process_name"], "notes"); + assert!(name.starts_with("process-notes-"), "{name}"); + assert!(command["outcome"].is_null(), "{command}"); + assert_eq!(command["state"], "live"); + assert_eq!(command["group"], "working"); + assert_eq!(stopped.code, Some(0), "{}", stopped.stderr); + assert_eq!(ended.code, Some(1), "{}", ended.stderr); + let after = scenario.run(&["status"]); + assert!( + after.stdout.contains("Process notes on acme/widgets") + && after.stdout.contains("· interrupted"), + "{}", + after.stdout + ); + let finished_status = status_json(&scenario); + let finished = process_command(&finished_status); + assert_eq!(finished["outcome"], "interrupted"); + assert_eq!(finished["process_name"], "notes"); + let (_, saved) = saved_notification(&scenario, "process"); + assert!( + saved.starts_with("Subject: [thirdshift] acme/widgets Process notes: interrupted\n"), + "{saved}" + ); + assert!(saved.contains("\nCommand: Process notes\n"), "{saved}"); + assert!(!saved.contains("\nCause:"), "{saved}"); + let activity = activity_log(&scenario); + assert!( + activity.contains("Process notes ended: interrupted"), + "{activity}" + ); +} + +#[test] +fn show_prints_a_failed_step_line_without_a_result_path() { + let scenario = Scenario::new(); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Fail the second step\" + +[[steps]] +name = \"gather\" +command = \"printf gather\" + +[[steps]] +name = \"review\" +command = \"exit 1\" +", + ); + + let result = scenario.run(&["process", "notes"]); + + assert_eq!(result.code, Some(1), "{}", result.stderr); + let text = scenario.run(&["status"]); + assert!( + text.stdout + .contains("Needs you:\nProcess notes on acme/widgets") + && text.stdout.contains("· Failed ·") + && text.stdout.contains("· failed"), + "{}", + text.stdout + ); + let status = status_json(&scenario); + let command = process_command(&status); + assert_eq!(command["group"], "needs you"); + assert_eq!(command["outcome"], "failed"); + assert_eq!(command["process_name"], "notes"); + let shown = scenario.run(&["show", command["name"].as_str().unwrap()]); + let (log_path, log) = command_log(&scenario); + assert!( + shown + .stdout + .contains(step_message(step_line(&log, "review"))), + "{}", + shown.stdout + ); + let missing = log_path.with_extension("steps").join("review.txt"); + assert!(!missing.exists(), "{}", missing.display()); + assert!( + !shown + .stdout + .contains(&format!("result: {}", missing.display())), + "{}", + shown.stdout + ); + let kept = log_path.with_extension("steps").join("gather.txt"); + assert!( + shown + .stdout + .contains(&format!("result: {}", kept.display())), + "{}", + shown.stdout + ); +} + +#[test] +fn the_console_lists_a_failed_process_in_needs_you() { + let scenario = Scenario::new(); + scenario.user_config_is("[console]\nport = 0\n"); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Fail\" + +[[steps]] +name = \"review\" +command = \"exit 1\" +", + ); + + let result = scenario.run(&["process", "notes"]); + + assert_eq!(result.code, Some(1), "{}", result.stderr); + let console = Console::start(&scenario); + let html = console.get("/"); + let section = html + .split("data-group=\"needs you\"") + .nth(1) + .unwrap_or_else(|| panic!("{html}")) + .split("</details>") + .next() + .unwrap(); + assert!( + section.contains("Process notes on acme/widgets"), + "{section}" + ); + assert!(section.contains("data-state=\"Failed\""), "{section}"); +} + +#[test] +fn a_finished_process_saves_a_run_notification_without_emailing_it() { + let scenario = Scenario::new(); + write_process(&scenario, "notes", NOTES); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "notes"], + &[("THIRDSHIFT_RESEND_URL", resend.url())], + ); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + assert!(resend.requests().is_empty()); + let (log_path, saved) = saved_notification(&scenario, "process"); + assert!( + saved.starts_with("Subject: [thirdshift] acme/widgets Process notes: succeeded\n"), + "{saved}" + ); + assert!(saved.contains("\nResult: succeeded\n"), "{saved}"); + assert!(saved.contains("\nCommand: Process notes\n"), "{saved}"); + assert!(saved.contains("\nRepository: acme/widgets\n"), "{saved}"); + assert!( + saved.contains(&format!("\nCommand log: {}\n", log_path.display())), + "{saved}" + ); +} + +#[test] +fn a_failed_process_emails_its_run_notification_when_asked() { + let scenario = Scenario::new(); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Fail\" + +[[steps]] +name = \"review\" +command = \"exit 1\" +", + ); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "--email", "me@example.com", "notes"], + &[ + ("THIRDSHIFT_RESEND_URL", resend.url()), + ("RESEND_API_KEY", KEY), + ], + ); + + assert_eq!(result.code, Some(1), "{}", result.stderr); + let requests = resend.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + let request = &requests[0]; + assert_eq!(request.body["to"], "me@example.com"); + assert_eq!( + request.body["subject"], + "[thirdshift] acme/widgets Process notes: failed" + ); + let text = request.body["text"].as_str().unwrap(); + assert!(text.contains("Result: failed\n"), "{text}"); + assert!(text.contains("Command: Process notes\n"), "{text}"); + assert!(text.contains("Repository: acme/widgets\n"), "{text}"); + assert!(text.contains("Cause: exit status 1\n"), "{text}"); + let (log_path, _) = saved_notification(&scenario, "process"); + assert!( + text.contains(&format!("Command log: {}\n", log_path.display())), + "{text}" + ); + support::resend::assert_saved_notification(&scenario, "process", request); +} + +#[test] +fn email_unless_merged_still_emails_a_process_because_it_does_not_merge() { + let scenario = Scenario::new(); + scenario.user_config_is("[email]\nto = \"config@example.com\"\n"); + write_process(&scenario, "notes", NOTES); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "notes", "email-unless-merged"], + &[ + ("THIRDSHIFT_RESEND_URL", resend.url()), + ("RESEND_API_KEY", KEY), + ], + ); + + assert_eq!(result.code, Some(0), "{}", result.stderr); + let requests = resend.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert_eq!(requests[0].body["to"], "config@example.com"); + assert_eq!( + requests[0].body["subject"], + "[thirdshift] acme/widgets Process notes: succeeded" + ); +} + +#[test] +fn the_user_config_emails_a_failed_process_under_the_ticket_run_rules() { + let scenario = Scenario::new(); + scenario.user_config_is("[email]\nunless_merged = true\nto = \"config@example.com\"\n"); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Fail\" + +[[steps]] +name = \"review\" +command = \"exit 1\" +", + ); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "notes"], + &[ + ("THIRDSHIFT_RESEND_URL", resend.url()), + ("RESEND_API_KEY", KEY), + ], + ); + + assert_eq!(result.code, Some(1), "{}", result.stderr); + let requests = resend.requests(); + assert_eq!(requests.len(), 1, "{requests:?}"); + assert_eq!(requests[0].body["to"], "config@example.com"); + assert!( + requests[0].body["subject"] + .as_str() + .unwrap() + .ends_with("Process notes: failed"), + "{}", + requests[0].body["subject"] + ); +} + +#[test] +fn no_email_saves_a_failed_process_notification_without_sending_it() { + let scenario = Scenario::new(); + scenario.user_config_is("[email]\nalways = true\nto = \"config@example.com\"\n"); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Fail\" + +[[steps]] +name = \"review\" +command = \"exit 1\" +", + ); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "notes", "--no-email"], + &[ + ("THIRDSHIFT_RESEND_URL", resend.url()), + ("RESEND_API_KEY", KEY), + ], + ); + + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!(resend.requests().is_empty()); + let (_, saved) = saved_notification(&scenario, "process"); + assert!(saved.contains("Process notes: failed"), "{saved}"); +} + +#[test] +fn a_missing_resend_key_stops_a_process_before_any_step() { + let scenario = Scenario::new(); + let marker = scenario.path("step-ran"); + write_process( + &scenario, + "notes", + &format!( + "\ +version = 1 +description = \"Must not run\" + +[[steps]] +name = \"review\" +command = \"touch '{}'\" +", + marker.display() + ), + ); + let resend = ResendStandIn::replying(200, ACCEPTED); + + let result = scenario.run_with_env( + &["process", "notes", "email", "me@example.com"], + &[("THIRDSHIFT_RESEND_URL", resend.url())], + ); + + scenario.assert_rejected_before_any_work(&result, "RESEND_API_KEY"); + assert!(!marker.exists(), "a step ran"); + assert!(resend.requests().is_empty()); + assert!(!process_dir(&scenario).exists()); +} + +#[test] +fn process_email_flags_are_usage_errors_on_check_and_ticket_flags_are_rejected() { + for args in [ + vec!["process", "notes", "merge"], + vec!["process", "notes", "harness", "codex"], + vec!["process", "check", "notes", "--no-email"], + vec!["process", "--email"], + ] { + let scenario = Scenario::new(); + write_process(&scenario, "notes", NOTES); + let result = scenario.run(&args); + assert_eq!(result.code, Some(2), "{args:?}: {}", result.stderr); + assert!( + result.stderr.contains("usage: thirdshift process"), + "{args:?}: {}", + result.stderr + ); + assert!(!process_dir(&scenario).exists(), "{args:?}"); + } +} From 79ee65b1496ddd106484f36242ad7a5d6dfdcdd1 Mon Sep 17 00:00:00 2001 From: Jacob Stephens <jacob@stephens.page> Date: Mon, 26 Oct 2026 21:24:33 -0400 Subject: [PATCH 2/2] Name a Process in Console run views and notify before any Step (#1387) Lanes, Stages and spans say Process <name>. show and the Console details print a Step result when the name has a space or a colon. A failure after the email check and before any Step still sends the Run notification. --- src/args.rs | 24 +------ src/console/peek.rs | 32 ++++++++- src/logs/command_record.rs | 16 +++++ src/logs/spans.rs | 27 ++++---- src/logs/status.rs | 3 +- src/logs/status/inbox.rs | 22 +++--- src/logs/status/inspect.rs | 106 ++++++++++++++++++++-------- src/logs/status/time.rs | 5 +- src/processes.rs | 26 +++++-- tests/process.rs | 137 +++++++++++++++++++++++++++++++++++++ 10 files changed, 315 insertions(+), 83 deletions(-) diff --git a/src/args.rs b/src/args.rs index 51b0f74b..d92f09ad 100644 --- a/src/args.rs +++ b/src/args.rs @@ -1026,32 +1026,14 @@ fn take_flag<'a>( arg: &str, rest: &mut Peekable<impl Iterator<Item = &'a String>>, ) -> Result<bool> { + if take_email_flag(&mut flags.email, arg, rest)? { + return Ok(true); + } match arg { "merge" | "--merge" => ask_once(&mut flags.goal, Goal::Merged, arg, MERGE_FLAGS)?, "no-merge" | "--no-merge" => { ask_once(&mut flags.goal, Goal::ReadyForReview, arg, MERGE_FLAGS)? } - "email" | "--email" => { - let to = rest.next_if(|next| is_address(next)).cloned(); - ask_once( - &mut flags.email, - NotificationAsk::Send(to), - arg, - EMAIL_FLAGS, - )? - } - "email-unless-merged" | "--email-unless-merged" => { - let to = rest.next_if(|next| is_address(next)).cloned(); - ask_once( - &mut flags.email, - NotificationAsk::SendUnlessMerged(to), - arg, - EMAIL_FLAGS, - )? - } - "no-email" | "--no-email" => { - ask_once(&mut flags.email, NotificationAsk::Skip, arg, EMAIL_FLAGS)? - } "parallel" | "--parallel" => ask_parallel(&mut flags.parallel, arg, rest.next())?, BASE_FIX | "--base-fix" => { ask_once(&mut flags.base_fix, BaseFixAsk::Allow, arg, BASE_FIX_FLAGS)? diff --git a/src/console/peek.rs b/src/console/peek.rs index b2bc2561..e426b990 100644 --- a/src/console/peek.rs +++ b/src/console/peek.rs @@ -97,7 +97,12 @@ impl Peek { issue_label, command, cli, - facts: Fact::children(value, false), + facts: { + let steps = step_facts(&value); + let mut facts = Fact::children(value, false); + facts.extend(steps); + facts + }, logs, } } @@ -106,6 +111,31 @@ impl Peek { } } +fn step_facts(value: &Value) -> Vec<Fact> { + if value.get("kind").and_then(Value::as_str) != Some("Process") { + return Vec::new(); + } + let Some(log) = value.get("command_log").and_then(Value::as_str) else { + return Vec::new(); + }; + crate::logs::status::inspect::process_steps(std::path::Path::new(log)) + .into_iter() + .map(|step| Fact { + label: "step".into(), + value: step.message, + children: step + .result + .into_iter() + .map(|path| Fact { + label: "result".into(), + value: path.display().to_string(), + children: Vec::new(), + }) + .collect(), + }) + .collect() +} + impl Fact { fn new(label: String, value: Value, in_metrics: bool) -> Self { let in_metrics = in_metrics || label == "run_metrics"; diff --git a/src/logs/command_record.rs b/src/logs/command_record.rs index f9d90539..17805ebf 100644 --- a/src/logs/command_record.rs +++ b/src/logs/command_record.rs @@ -268,6 +268,22 @@ impl StageName { } impl CommandRecord { + /// The Custom process's own name. An older record with no `process_name` + /// falls back to the selector in `name`. + pub(super) fn process_label(&self) -> Option<&str> { + (self.kind == "Process").then(|| self.process_name.as_deref().unwrap_or(self.name.as_str())) + } + + /// `Process <name> on <repo>`, or `Process <name> · <repo>#<n>` when the + /// record names an issue. Status text adds the kind word itself. + pub(super) fn process_heading(&self) -> Option<String> { + let name = self.process_label()?; + Some(match self.issue.as_ref() { + Some(issue) => format!("Process {name} · {}#{}", self.repository, issue.number), + None => format!("Process {name} on {}", self.repository), + }) + } + /// The next transition closes a stage; only a recorded outcome closes the /// final stage. A live caller can supply its single sampled observation time. pub fn stage_times( diff --git a/src/logs/spans.rs b/src/logs/spans.rs index ece6bfa7..a4ca1d52 100644 --- a/src/logs/spans.rs +++ b/src/logs/spans.rs @@ -324,18 +324,7 @@ fn record_span( id, parent_id, kind, - name: format!( - "{} · {}{}", - command.record.kind, - subject(command), - command - .record - .issue - .as_ref() - .and_then(|issue| issue.title.as_ref()) - .map(|title| format!(" · {title}")) - .unwrap_or_default() - ), + name: span_name(command), start, end, duration_ms: duration(start, end), @@ -353,6 +342,20 @@ fn record_span( } } +fn span_name(command: &Command) -> String { + let title = command + .record + .issue + .as_ref() + .and_then(|issue| issue.title.as_ref()) + .map(|title| format!(" · {title}")) + .unwrap_or_default(); + if let Some(heading) = command.record.process_heading() { + return format!("{heading}{title}"); + } + format!("{} · {}{title}", command.record.kind, subject(command)) +} + fn subject(command: &Command) -> String { command .record diff --git a/src/logs/status.rs b/src/logs/status.rs index 2ad1d1e6..e5df143e 100644 --- a/src/logs/status.rs +++ b/src/logs/status.rs @@ -37,8 +37,7 @@ struct Status<'a> { fn print_command(command: &Command, state_word: &str, indent: &str) { let facts = &command.record; let stage = stage_text(command); - let mut subject = if facts.kind == "Process" { - let name = facts.process_name.as_deref().unwrap_or(facts.name.as_str()); + let mut subject = if let Some(name) = facts.process_label() { match &facts.issue { Some(issue) => format!( "{name} {}#{} {}", diff --git a/src/logs/status/inbox.rs b/src/logs/status/inbox.rs index 784f82b2..832e2722 100644 --- a/src/logs/status/inbox.rs +++ b/src/logs/status/inbox.rs @@ -328,21 +328,15 @@ pub(crate) fn inbox(config: &UserConfig) -> Result<Inbox> { } fn row_subject(facts: &crate::logs::command_record::CommandRecord) -> (String, String) { - if facts.kind == "Process" { - let name = facts.process_name.as_deref().unwrap_or(facts.name.as_str()); - return match facts.issue.as_ref() { - Some(issue) => ( - format!("Process {name} · {}#{}", facts.repository, issue.number), - issue - .title - .clone() - .unwrap_or_else(|| "(title unavailable)".into()), - ), - None => ( - format!("Process {name} on {}", facts.repository), - String::new(), - ), + if let Some(heading) = facts.process_heading() { + let title = match facts.issue.as_ref() { + Some(issue) => issue + .title + .clone() + .unwrap_or_else(|| "(title unavailable)".into()), + None => String::new(), }; + return (heading, title); } subject(&facts.repository, &facts.name, facts.issue.as_ref()) } diff --git a/src/logs/status/inspect.rs b/src/logs/status/inspect.rs index 3d665c18..a09d41ce 100644 --- a/src/logs/status/inspect.rs +++ b/src/logs/status/inspect.rs @@ -41,15 +41,8 @@ pub(crate) fn url(route: &str, command: &str, session: Option<&std::path::Path>) } fn shown_heading(command: &Command) -> String { - if command.record.kind == "Process" { - return format!( - "Process {}", - command - .record - .process_name - .as_deref() - .unwrap_or(command.record.name.as_str()) - ); + if let Some(name) = command.record.process_label() { + return format!("Process {name}"); } format!( "{} {}", @@ -58,40 +51,97 @@ fn shown_heading(command: &Command) -> String { ) } +/// One Process Step as `show` and the Console details print it. +pub(crate) struct ShownStep { + pub message: String, + pub result: Option<std::path::PathBuf>, +} + /// Each Process Step's Command log line, and the result file when the Step wrote one. +pub(crate) fn process_steps(command_log: &std::path::Path) -> Vec<ShownStep> { + let Ok(log) = std::fs::read_to_string(command_log) else { + return Vec::new(); + }; + let results = command_log.with_extension("steps"); + log.lines() + .filter_map(step_message) + .map(|message| { + let result = step_name(message) + .map(|name| results.join(format!("{name}.txt"))) + .filter(|path| path.is_file()); + ShownStep { + message: message.to_string(), + result, + } + }) + .collect() +} + fn print_steps(command: &Command, indent: &str) { if command.record.kind != "Process" { return; } - let Ok(log) = std::fs::read_to_string(&command.command_log) else { - return; - }; - let results = command.command_log.with_extension("steps"); - for line in log.lines() { - let Some(message) = step_message(line) else { - continue; - }; - println!("{indent}{message}"); - let Some(name) = step_name(message) else { - continue; - }; - let path = results.join(format!("{name}.txt")); - if path.is_file() { + for step in process_steps(&command.command_log) { + println!("{indent}{}", step.message); + if let Some(path) = &step.result { println!("{indent}result: {}", path.display()); } } } fn step_message(line: &str) -> Option<&str> { - let rest = line.strip_prefix("thirdshift: ")?; - let (_, message) = rest.split_once(' ')?; - message.starts_with("step ").then_some(message) + match crate::progress::ChildLine::of(line) { + crate::progress::ChildLine::Own(message) if message.starts_with("step ") => Some(message), + _ => None, + } } +/// The Step name in `step <name>: <duration> ...`. A name may contain spaces +/// and colons; the duration `finish_step` writes is what separates it. fn step_name(message: &str) -> Option<&str> { let rest = message.strip_prefix("step ")?; - let (name, _) = rest.split_once(':')?; - (!name.is_empty() && !name.contains(' ')).then_some(name) + let colon = duration_separator(rest)?; + let name = &rest[..colon]; + (!name.is_empty()).then_some(name) +} + +fn duration_separator(rest: &str) -> Option<usize> { + let mut from = 0; + while let Some(at) = rest[from..].find(": ") { + let absolute = from + at; + if starts_with_duration(&rest[absolute + 2..]) { + return Some(absolute); + } + from = absolute + 2; + } + None +} + +fn starts_with_duration(text: &str) -> bool { + let mut words = text.split(' '); + let Some(first) = words.next() else { + return false; + }; + if let Some(hours) = first.strip_suffix('h') { + return is_uint(hours) + && words + .next() + .is_some_and(|word| word.strip_suffix('m').is_some_and(is_uint)) + && words + .next() + .is_some_and(|word| word.strip_suffix('s').is_some_and(is_uint)); + } + if let Some(minutes) = first.strip_suffix('m') { + return is_uint(minutes) + && words + .next() + .is_some_and(|word| word.strip_suffix('s').is_some_and(is_uint)); + } + first.strip_suffix('s').is_some_and(is_uint) +} + +fn is_uint(text: &str) -> bool { + !text.is_empty() && text.bytes().all(|byte| byte.is_ascii_digit()) } fn print(command: Observation<'_>) -> Result<()> { diff --git a/src/logs/status/time.rs b/src/logs/status/time.rs index 5b56e2fd..459b4be8 100644 --- a/src/logs/status/time.rs +++ b/src/logs/status/time.rs @@ -174,11 +174,14 @@ fn run(command: &Command, now: DateTime<Local>, cutoff: i64) -> Run { .collect(); let (issue_url, issue_label) = issue_link(record.issue.as_ref()); Run { - subject: subject(&record.kind, &record.repository, record.issue.as_ref()), + subject: record + .process_heading() + .unwrap_or_else(|| subject(&record.kind, &record.repository, record.issue.as_ref())), title: record .issue .as_ref() .and_then(|issue| issue.title.clone()) + .or_else(|| record.process_label().map(str::to_string)) .unwrap_or_else(|| record.name.clone()), state: if command.state == "ended" { record.outcome.clone().unwrap_or_else(|| "ended".into()) diff --git a/src/processes.rs b/src/processes.rs index b3d5581f..e4aeda10 100644 --- a/src/processes.rs +++ b/src/processes.rs @@ -187,15 +187,26 @@ fn run(name: &str, issue: Option<IssueInput>, email: Option<NotificationAsk>) -> About::Process { name }, )?; // Before any worktree: a foreign Issue URL is an origin mismatch, and a - // number that cannot be resolved stops the same way. - let issue = resolve_process_issue(issue)?; + // number that cannot be resolved stops the same way. A failure here is + // still a failed Process, so it sends the Run notification a Ticket run + // would send for the same kind of stop. + let issue = match resolve_process_issue(issue) { + Ok(issue) => issue, + Err(error) => return Err(failed_before_steps(notification, error)), + }; // The same margin as a Ticket run. Edits in the gap since this moment, // or dated by a slow GitHub clock, still count. let started = Utc::now() - crate::run::STARTED_MARGIN; - let mut prepared = prepare(name, &config)?; + let mut prepared = match prepare(name, &config) { + Ok(prepared) => prepared, + Err(error) => return Err(failed_before_steps(notification, error)), + }; prepared.issue = issue; prepared.started = started; - let repo = launch::repo()?; + let repo = match launch::repo() { + Ok(repo) => repo, + Err(error) => return Err(failed_before_steps(notification, error)), + }; logs::started(Work::Process { repo: &repo, name }, &prepared.choice); notification.built_with(&prepared.choice); let result = execute_steps(name, &prepared); @@ -207,6 +218,13 @@ fn run(name: &str, issue: Option<IssueInput>, email: Option<NotificationAsk>) -> result } +/// A stop after the email check and before any Step is still a failed Process. +fn failed_before_steps(notification: RunNotification, error: anyhow::Error) -> anyhow::Error { + let cause = format!("{error:#}"); + notify(notification, "failed", Some(&cause)); + error +} + /// Save and, when asked, send the Run notification. A skipped Process sends none. fn notify(notification: RunNotification, outcome: &'static str, cause: Option<&str>) { if outcome == "skipped" { diff --git a/tests/process.rs b/tests/process.rs index c91e6ac7..380b4aef 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -2792,3 +2792,140 @@ fn process_email_flags_are_usage_errors_on_check_and_ticket_flags_are_rejected() assert!(!process_dir(&scenario).exists(), "{args:?}"); } } + +#[test] +fn show_prints_the_result_path_when_the_step_name_has_a_space() { + let scenario = Scenario::new(); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Spaced step name\" + +[[steps]] +name = \"gather notes\" +command = \"printf gather\" +", + ); + let result = scenario.run(&["process", "notes"]); + assert_eq!(result.code, Some(0), "{}", result.stderr); + let (log_path, _) = command_log(&scenario); + let kept = log_path.with_extension("steps").join("gather notes.txt"); + assert!(kept.is_file(), "{}", kept.display()); + let status = status_json(&scenario); + let shown = scenario.run(&["show", process_command(&status)["name"].as_str().unwrap()]); + assert!( + shown + .stdout + .contains(&format!("result: {}", kept.display())), + "{}", + shown.stdout + ); +} + +#[test] +fn lanes_and_spans_name_the_process() { + let scenario = Scenario::new(); + scenario.user_config_is("[console]\nport = 0\n"); + write_process(&scenario, "notes", NOTES); + assert_eq!(scenario.run(&["process", "notes"]).code, Some(0)); + let status = status_json(&scenario); + let name = process_command(&status)["name"].as_str().unwrap(); + let console = Console::start(&scenario); + let lanes = console.get("/?page=lanes"); + assert!(lanes.contains("Process notes"), "{lanes}"); + let stages = console.get("/?page=stages"); + assert!(stages.contains("Process notes"), "{stages}"); + let spans = scenario.run(&["show", name, "--spans"]); + assert!(spans.stdout.contains("Process notes"), "{}", spans.stdout); +} + +#[test] +fn peek_shows_each_step_line_and_result_path() { + let scenario = Scenario::new(); + scenario.user_config_is("[console]\nport = 0\n"); + write_process(&scenario, "notes", NOTES); + assert_eq!(scenario.run(&["process", "notes"]).code, Some(0)); + let status = status_json(&scenario); + let name = process_command(&status)["name"] + .as_str() + .unwrap() + .to_string(); + let (log_path, _) = command_log(&scenario); + let result = log_path.with_extension("steps").join("gather.txt"); + // `urlencoding` is not a crate here. The query is the one the finding wrote. + let mut url = reqwest::Url::parse("http://localhost/peek").unwrap(); + url.query_pairs_mut().append_pair("command", &name); + let html = Console::start(&scenario).get(&format!("{}?{}", url.path(), url.query().unwrap())); + assert!(html.contains("step gather:"), "{html}"); + assert!(html.contains(&result.display().to_string()), "{html}"); +} + +#[test] +fn show_prints_the_result_path_when_the_step_name_has_a_space_or_a_colon() { + let scenario = Scenario::new(); + write_process( + &scenario, + "notes", + "\ +version = 1 +description = \"Names\" + +[[steps]] +name = \"gather notes\" +command = \"printf gather\" + +[[steps]] +name = \"a:b\" +command = \"printf ab\" +", + ); + assert_eq!(scenario.run(&["process", "notes"]).code, Some(0)); + let status = status_json(&scenario); + let name = process_command(&status)["name"] + .as_str() + .unwrap() + .to_string(); + let (log_path, _) = command_log(&scenario); + let shown = scenario.run(&["show", &name]); + for step in ["gather notes", "a:b"] { + let path = log_path.with_extension("steps").join(format!("{step}.txt")); + assert!(path.is_file(), "{}", path.display()); + assert!( + shown + .stdout + .contains(&format!("result: {}", path.display())), + "{step}: {}", + shown.stdout + ); + } +} + +#[test] +fn an_origin_mismatch_still_sends_the_run_notification() { + let scenario = Scenario::new(); + write_process(&scenario, "notes", NOTES); + let resend = ResendStandIn::replying(200, ACCEPTED); + let result = scenario.run_with_env( + &[ + "process", + "--email", + "me@example.com", + "notes", + "issue", + "https://github.com/Other/Widgets/issues/7", + ], + &[ + ("THIRDSHIFT_RESEND_URL", resend.url()), + ("RESEND_API_KEY", KEY), + ], + ); + assert_eq!(result.code, Some(1), "{}", result.stderr); + assert!( + result.stderr.contains("origin mismatch"), + "{}", + result.stderr + ); + assert_eq!(resend.requests().len(), 1, "{:?}", resend.requests()); +}