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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -507,10 +507,10 @@ A review session that fails or is interrupted, a final message without one of th

### 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, skipped included, [either](#one-at-a-time) [way](#one-architect-plan-at-a-time), 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.
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, with or without `--plan-only`, unless it is skipped. A skipped Architect run, [however](#one-at-a-time) [it is](#one-architect-plan-at-a-time) [skipped](#an-architect-idea-waits-for-triage), sends none, even when asked, so a schedule can start one every few minutes without flooding your inbox: its line is in the log its scheduler keeps. It makes a Run's checks before any other work all the same, an address and a Resend API key, and stops with exit `1` if either is missing, so a run that would have been skipped fails on them too. 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`. For a skipped run, it is `skipped`. 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), with the [lines after it](#an-inherited-failure-links-the-base-branchs-checks) of a dispatched run that failed on an Inherited failure, a `Base fix:` line for a dispatched run that started or waited on a [Base fix](#base-fix), the session log path (if any), the `Command 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. A skipped run's body has a `Skipped:` line giving the reason in place of the `Review:` line, then the hostname and how long it took. The reason is `an Architect run or a Pickup run is already running on <owner>/<repo>`, each open Architect plan with the command that picks it up: `Architect plan #<n> "<title>" is still open: pick it up with thirdshift <plan URL>`, or each Architect idea waiting for triage: `Architect idea #<n> "<title>" is waiting for triage: <Issue URL>`.
- **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), with the [lines after it](#an-inherited-failure-links-the-base-branchs-checks) of a dispatched run that failed on an Inherited failure, a `Base fix:` line for a dispatched run that started or waited on a [Base fix](#base-fix), the session log path (if any), the `Command 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

Expand Down Expand Up @@ -539,7 +539,7 @@ The repository is the one `origin` names, so two clones of one repository count

thirdshift holds the rule with an operating-system lock on a file under `~/.thirdshift/architect-locks/`, one for both kinds of run, which the run's process takes without waiting, after its checks in step 1, and holds until it exits. The operating system releases the lock when that process ends for any reason, a crash, a kill or a reboot included, so there is never a stale lock to clear: the next Architect run or Pickup run on the repository runs normally. The file itself stays, and means nothing on its own. A review worktree that a killed Architect run left behind is removed by the next one.

A skipped Architect run asked for a [Run notification](#one-run-notification) still sends its one: the notification's checks are made before the lock is tried, so a skip is always notified. A skipped Pickup run [sends none](#one-run-notification-for-the-issue-taken), though it makes the same checks first.
A skipped Architect run sends no [Run notification](#one-run-notification), even when asked, and neither does a skipped Pickup run ([sends none](#one-run-notification-for-the-issue-taken)). Both make the notification's checks before the lock is tried all the same, so a broken setup fails the pass with exit `1` rather than being skipped.

### One Architect plan at a time

Expand All @@ -555,7 +555,7 @@ It applies to every Architect run, with or without `--plan-only`, and no flag ov

An Architect run never retries or dispatches an existing Architect plan. One whose dispatched run failed stays open, and picking it up is yours to do, with `thirdshift <plan URL>`, which continues whatever branch and pull request the failed run left. What isn't labelled `architect-plan` never counts here: a plan that a failed or interrupted review left `needs-triage`, or an [Architect idea](#no-strong-candidate), which [pauses Architect runs](#an-architect-idea-waits-for-triage) by its own rule, only while it waits for triage.

The check comes after the lock so that the plan of an Architect run that is still running, labelled already while its Spec run or Run goes on, is reported as [already running](#one-at-a-time), not as an unfinished plan. A skipped run asked for a [Run notification](#one-run-notification) sends its one, with the open Architect plans and their commands in its `Skipped:` line.
The check comes after the lock so that the plan of an Architect run that is still running, labelled already while its Spec run or Run goes on, is reported as [already running](#one-at-a-time), not as an unfinished plan. A skipped run sends no [Run notification](#one-run-notification), even when asked: its stdout carries the open Architect plans' URLs, and its stderr line their commands.

### On a schedule

Expand All @@ -581,7 +581,7 @@ The command's flags and the User config decide what a scheduled Architect run do

The first leaves the pull request for you to review, and the second stops at the Architect plan, for you to read and run with `thirdshift <Issue URL>`.

A scheduled Architect run is skipped when another on the repository, or a Pickup run, is [still running](#one-at-a-time), the Spec run or Run it dispatched included, when an [Architect plan is still open](#one-architect-plan-at-a-time), or when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage): no new refactor is planned until last night's Architect plan is closed or loses its `architect-plan` label. Merging its pull request closes it; if its run failed, pick it up with `thirdshift <plan URL>` first. Closing the pull request alone leaves the Architect plan open. A skip exits `0`, so the scheduler sees no failure, and the log has the line that says why. With Run notifications on, by `email.always = true` in the User config or `email` on the line, every night sends one email, a skipped night included, so a night without one means something is wrong: look in the log file.
A scheduled Architect run is skipped when another on the repository, or a Pickup run, is [still running](#one-at-a-time), the Spec run or Run it dispatched included, when an [Architect plan is still open](#one-architect-plan-at-a-time), or when an [Architect idea waits for triage](#an-architect-idea-waits-for-triage): no new refactor is planned until last night's Architect plan is closed or loses its `architect-plan` label. Merging its pull request closes it; if its run failed, pick it up with `thirdshift <plan URL>` first. Closing the pull request alone leaves the Architect plan open. A skip exits `0`, so the scheduler sees no failure, and the log file has the line that says why. With Run notifications on, by `email.always = true` in the User config or `email` on the line, an Architect run that did work sends one email, and a skipped one sends none, so a crontab line can fire every few minutes without flooding your inbox. A night without an email means it was skipped, or failed before the notification's checks passed: look in the log file.

Any scheduler that runs the command works; a systemd user timer needs lingering on (`loginctl enable-linger`) to fire while you are logged out.

Expand Down Expand Up @@ -682,7 +682,7 @@ A Pickup run asked for a [Run notification](#run-notifications), by `email` or b

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. The dispatched run sends no notification of its own, whatever the User config says, as with an [Architect run](#one-run-notification), nor do a Spec run's Ticket Runs.

A skipped pass sends no notification, even when one was asked for: a pass every half hour would otherwise send dozens a day. This differs from a skipped Architect run, which does send one.
A skipped pass sends no notification, even when one was asked for: a pass every half hour would otherwise send dozens a day. A skipped Architect run sends none either.

The notification's checks, an address and a Resend API key, are made before any other work on every pass, before the checks in step 1, the lock and any label read or changed. A broken setup therefore stops the pass with exit `1`, naming what is missing, even a pass that would have been skipped, so it shows in the scheduler's log on the first pass, not only once an issue is ready. A pass that asks for no notification makes none of these checks.

Expand Down
2 changes: 1 addition & 1 deletion docs/adr/0009-the-operating-system-schedules-thirdshift.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,4 @@ An **Architect run** and a **Pickup run** find their own work, so they are meant
- A **Ready issue** waits up to the scheduler's interval before it is taken, and throughput is set by how often the crontab line fires.
- An issue is taken at most once across passes by state on GitHub, the **Claim**, since no process outlives a pass to remember it. The lock only covers one machine.
- Several repositories means several crontab lines, at hours the user staggers. thirdshift keeps no list of repositories.
- A frequent pass must be quiet when it does nothing: a skipped Pickup run sends no **Run notification**, though a skipped Architect run, started nightly, does.
- A frequent pass must be quiet when it does nothing: a skipped Pickup run or Architect run sends no **Run notification**, so either can be started every few minutes. A skip leaves only its line in the scheduler's log.
11 changes: 1 addition & 10 deletions src/architect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,6 @@ pub enum Outcome {
}

impl Outcome {
/// How the Architect run ended, as its Run notification's subject says
/// it when nothing was dispatched.
pub fn name(&self) -> &'static str {
match self {
Self::Skipped(_) => "skipped",
Self::Reviewed(reviewed) => reviewed.review(),
}
}

/// The URLs of the issues the Architect run ended on, as its stdout
/// carries them when nothing was dispatched: the issue its Architecture
/// review ended on, or each open Architect plan, or each Architect idea
Expand All @@ -98,7 +89,7 @@ impl fmt::Display for Outcome {
}

/// Why an Architect run was skipped. Its `Display` is the reason, as the
/// skipped run's progress line and its Run notification give it.
/// skipped run's progress line gives it.
#[derive(Debug)]
pub enum Skipped {
/// Another Architect run on this repository, or a Pickup run, is still
Expand Down
21 changes: 16 additions & 5 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -245,8 +245,10 @@ from the clone: the README's \"A Pickup run on a schedule\" has a crontab entry.
they do a Run, with or without --plan-only, and email.always sets the default. It sends one
for the whole Architect run, however it ends: how the review ended, with the plan or idea
issue it named, and how the run the plan was dispatched as ended, with a line on each Ticket
of a Spec run. The run the plan is dispatched as sends none of its own. A skipped run still
sends its Run notification, with the outcome skipped and the reason.
of a Spec run. The run the plan is dispatched as sends none of its own. A skipped run sends
none, even when asked, so a scheduler can start one every few minutes. The notification's
checks, an address and a Resend API key, are made before any other work on every run, so
one that would be skipped fails on them too, with exit 1.

The User config, ~/.thirdshift/config.toml, sets defaults for every Run on this machine;
thirdshift setup asks for your defaults and writes one listing every setting, to edit.
Expand Down Expand Up @@ -366,7 +368,9 @@ fn main() -> ExitCode {
/// repository, or a Pickup run, is still running, it puts nothing on stdout,
/// and as Architect plans are still open there, the URL of each. If asked, by
/// the command or the User config, it sends one Run notification, however it
/// ended, skipped included; the run it dispatched sends none of its own. Its
/// ended, short of being skipped; the run it dispatched sends none of its
/// own. The notification's checks are made before any other work all the
/// same, so a run that would be skipped fails on them too. Its
/// Command log, kept once it is past its skip checks, covers that run too.
fn architect(args: ArchitectArgs) -> ExitCode {
command_log::begin("Architect run starting");
Expand Down Expand Up @@ -407,8 +411,15 @@ fn architect(args: ArchitectArgs) -> ExitCode {
(Ok(outcome), None) => run_ending::show_architect(outcome),
(Err(failed), None) => run_ending::show_failure(failed),
};
if let Some(notification) = notification {
notification.send(&ended, dispatched.as_ref());
// A skipped Architect run sends none, so one started every few minutes
// for Weeding doesn't flood the inbox.
let reviewed = match &ended {
Ok(Outcome::Skipped(_)) => None,
Ok(Outcome::Reviewed(reviewed)) => Some(Ok(reviewed)),
Err(failed) => Some(Err(failed)),
};
if let (Some(notification), Some(reviewed)) = (notification, reviewed) {
notification.send(reviewed, dispatched.as_ref());
}
code
}
Expand Down
Loading
Loading