From 195a846fcb230b55431581cf29772bfd769717d3 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sat, 17 Oct 2026 04:30:32 -0400 Subject: [PATCH 1/2] Clarify CI progress counts as checks listed so far (#289) --- README.md | 2 ++ src/ci.rs | 91 ++++++++++++++++++++++++++++++++++++++++++++++------- tests/ci.rs | 20 +++++++++--- 3 files changed, 96 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 07c7b4fc6..adf2b4e95 100644 --- a/README.md +++ b/README.md @@ -184,6 +184,8 @@ It sends one [Run notification](#run-notifications), whatever the outcome. To ha - **stderr** carries everything else: errors, cleanup problems, and progress lines while sessions run. Each line starts with the local time it was printed, as in `thirdshift: 12:14:49 pushing issue-7`, so a quiet terminal shows how long the Run has been on its last step. The first line of a Run, a Spec run or a Pass also names the date and the UTC offset, as in `thirdshift: 12:00:01 starting on https://github.com/acme/widgets/issues/7, 2026-10-03 -0400`, so a log that collects many commands, such as a crontab line's, reads as a dated list. Everything on stderr and stdout is kept in the command's [Command log](#logs) too. A top-level Run or Spec run prints its elapsed wall time near the end, as `Took: 1h 2m 3s`, before its final outcome or failure cause. This also covers graceful interruption, works without email, and adds no summary for child Runs or skipped Passes. A successful Run's last line names the pull request too: `PR is ready for review`, or `PR is merged` after a Merge run, followed only by a `warning:` line if a [Run notification](#run-notifications) can't be sent. - **Exit code** `0` means the Run ended with a pull request the factory stands behind, merged in a Merge run. Once the Self-merge has merged, the Run succeeds even if deleting the Issue branch on `origin`, closing the issue or removing `in-progress` from it then fails: the merge can't be undone, so each failed step is a `warning:` line on stderr naming the command to run by hand, and the Run still exits `0` with the URL on stdout. Ctrl-C likewise: before the merge it makes a Failed run, after it thirdshift finishes these steps and exits as merged. `2` means the command is one thirdshift can't use: the issue argument is missing or isn't a GitHub Issue URL or positive decimal number, there is an argument other than the issue and the Run flags, a Run flag is repeated or contradicts another, or `parallel` isn't followed by a whole number from 1 up; the error and the help text go to stderr, before any work. Failure to resolve a number against the clone's origin exits `1`. A [User config](#user-config) thirdshift can't use exits `1`, also before any work. Any other failure exits `1`. +During a CI wait, `CI on : 3 of 4 checks listed so far are still running` counts the checks GitHub currently lists, including commit statuses. Dependent jobs and matrix jobs may become visible later, so both the pending count and the listed total can rise as CI progresses. A change to either count prints a new progress line; identical repeated counts stay quiet. The listed total is not a prediction of the final workflow total. + The other commands: ```sh diff --git a/src/ci.rs b/src/ci.rs index dfc914b71..666269173 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -271,7 +271,7 @@ fn watch_to_end( rerun_checks: Option<&RerunChecks<'_>>, ) -> Result { let short = short(sha); - let mut reported_pending = None; + let mut reported_counts = None; let checks = until(outside, |outside| { let checks = outside.checks_on(sha)?; ensure!( @@ -282,12 +282,13 @@ fn watch_to_end( if pending == 0 { return Ok(Some(checks)); } - if reported_pending != Some(pending) { + let counts = (pending, checks.len()); + if reported_counts != Some(counts) { outside.step(format!( - "CI on {short}: {pending} of {} checks still running", + "CI on {short}: {pending} of {} checks listed so far are still running", checks.len() )); - reported_pending = Some(pending); + reported_counts = Some(counts); } Ok(None) })?; @@ -612,14 +613,80 @@ mod tests { github.steps, [ "waiting up to 60s for CI on abcdef0", - "CI on abcdef0: 2 of 2 checks still running", - "CI on abcdef0: 1 of 2 checks still running", + "CI on abcdef0: 2 of 2 checks listed so far are still running", + "CI on abcdef0: 1 of 2 checks listed so far are still running", "CI passed on abcdef0", ] ); assert_eq!(github.reads_of(BASE), 0); } + #[test] + fn dependent_jobs_appearing_later_can_raise_pending_and_listed_counts() { + let mut github = InMemory::new().listing( + HEAD, + vec![ + vec![pending("build"), pending("lint")], + vec![pending("build"), pending("lint")], + vec![green("build"), pending("lint")], + vec![ + green("build"), + pending("lint"), + pending("test-linux"), + pending("test-macos"), + ], + vec![ + green("build"), + green("lint"), + green("test-linux"), + green("test-macos"), + ], + ], + ); + + let ci = watch_through(&mut github, HEAD, Some(BASE)).unwrap(); + + assert!(matches!(ci, Ci::Passed)); + assert_eq!( + github.steps, + [ + "waiting up to 60s for CI on abcdef0", + "CI on abcdef0: 2 of 2 checks listed so far are still running", + "CI on abcdef0: 1 of 2 checks listed so far are still running", + "CI on abcdef0: 3 of 4 checks listed so far are still running", + "CI passed on abcdef0", + ] + ); + } + + #[test] + fn changed_listed_totals_are_reported_even_when_pending_is_unchanged() { + let mut github = InMemory::new().listing( + HEAD, + vec![ + vec![green("build"), pending("test-linux")], + vec![green("build"), pending("test-linux")], + vec![green("build"), pending("test-linux")], + vec![green("build"), green("test-linux"), pending("test-macos")], + vec![green("build"), green("test-linux"), pending("test-macos")], + vec![green("build"), green("test-linux"), green("test-macos")], + ], + ); + + let ci = watch_through(&mut github, HEAD, Some(BASE)).unwrap(); + + assert!(matches!(ci, Ci::Passed)); + assert_eq!( + github.steps, + [ + "waiting up to 60s for CI on abcdef0", + "CI on abcdef0: 1 of 2 checks listed so far are still running", + "CI on abcdef0: 1 of 3 checks listed so far are still running", + "CI passed on abcdef0", + ] + ); + } + #[test] fn the_base_branch_commit_is_read_once_after_every_check_has_finished() { let mut github = InMemory::new() @@ -1008,9 +1075,9 @@ mod tests { assert_eq!(listed(&failed.on_base), ["lint: https://head/lint"]); assert_eq!(github.reruns, [1]); assert!( - github - .steps - .contains(&"CI on abcdef0: 1 of 2 checks still running".to_string()) + github.steps.contains( + &"CI on abcdef0: 1 of 2 checks listed so far are still running".to_string() + ) ); } @@ -1058,9 +1125,9 @@ mod tests { assert!(failed.inherited.is_empty()); assert_eq!(github.reruns, [1, 2]); assert!( - github - .steps - .contains(&"CI on abcdef0: 1 of 3 checks still running".to_string()) + github.steps.contains( + &"CI on abcdef0: 1 of 3 checks listed so far are still running".to_string() + ) ); } diff --git a/tests/ci.rs b/tests/ci.rs index 53f2e1165..2d5086f53 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -66,7 +66,9 @@ fn checks_pending_then_green_is_success() { assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); assert_eq!(result.stdout, "https://github.com/acme/widgets/pull/1\n"); assert!( - result.stderr.contains("1 of 2 checks still running"), + result + .stderr + .contains("1 of 2 checks listed so far are still running"), "stderr: {}", result.stderr ); @@ -303,7 +305,9 @@ fn a_repair_that_already_re_ran_ci_to_green_delivers_or_self_merges_the_unchange assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], false); if pending { assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 of 1 checks listed so far are still running"), "{}", result.stderr ); @@ -385,7 +389,9 @@ fn a_refused_request_still_waits_for_an_earlier_accepted_workflows_new_attempt() result.stderr ); assert!( - result.stderr.contains("1 of 2 checks still running"), + result + .stderr + .contains("1 of 2 checks listed so far are still running"), "{}", result.stderr ); @@ -474,7 +480,9 @@ fn a_refused_request_pending_then_red_is_a_declined_ci_fix_without_another_repai result.stderr ); assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 of 1 checks listed so far are still running"), "{}", result.stderr ); @@ -651,7 +659,9 @@ fn the_attempt_before_a_check_re_run_is_never_taken_for_its_result() { result.stderr ); assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 of 1 checks listed so far are still running"), "stderr: {}", result.stderr ); From fbc871130586687324d06ce0000f8c0b5022d3f2 Mon Sep 17 00:00:00 2001 From: Jacob Stephens Date: Sat, 17 Oct 2026 08:30:42 +0000 Subject: [PATCH 2/2] Clarify CI progress counts as checks listed so far (#289) --- README.md | 4 +++ src/ci.rs | 91 ++++++++++++++++++++++++++++++++++++++++++++++------- tests/ci.rs | 20 +++++++++--- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 07c7b4fc6..dea8d79ff 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,10 @@ From a clone of the issue's repository, `thirdshift `: 7. In a **Merge run** (`thirdshift merge `), does the **Self-merge**: once the pull request is open, ready for review, mergeable and green, thirdshift merges it into the Base branch with a merge commit, on exactly the head commit whose CI it watched (`gh pr merge --merge --match-head-commit `). Just before each attempt, it reads the PR's `reviewDecision`: `REVIEW_REQUIRED` or `CHANGES_REQUESTED` ends the Run as **review required**, a policy refusal, without attempting the merge or taking another Repair round. An empty or `APPROVED` decision, or a failed read, keeps the existing merge behavior. It never uses GitHub's auto-merge ([ADR-0004](docs/adr/0004-self-merge-by-thirdshift-not-github-auto-merge.md)). A merge that fails goes back round step 6, within the same budgets, and is tried again on the new green head. If that round finds nothing to fix, the refusal is a **policy refusal**, such as merge commits being disallowed or a review being required. After the merge, thirdshift deletes the Issue branch on `origin`, and closes the issue if it is still open, with the comment `Closed by #, merged into by a thirdshift Merge run.` GitHub closes it on its own only for a merge into the repository's default branch, and then thirdshift leaves it alone. Once the issue is closed, either way, thirdshift takes `in-progress` off it, [removing the Claim](#when-the-claim-ends). 8. Cleans up: removes the worktree, the local Issue branch and the temporary directory the Factory skills were written out to, whether the Run succeeded or not. The one exception is a **Failed run** whose work could not be pushed: see below. +### CI progress + +While waiting for CI, stderr reports, for example, `CI on abcdef0: 2 still running out of 6 checks listed so far`. The total is the checks and commit statuses GitHub currently lists, not a fixed workflow total. Dependent jobs and matrix jobs may become visible later, so both the pending count and the total can rise as CI progresses. A change to either count produces a progress line; an identical repeated pair stays quiet. This reporting does not change which checks count as passing or when Delivery may merge. + ### The Claim The **Claim** is the mark that the factory has taken an issue, so the issue list shows what it is working on. When a Run or a [Spec run](#spec-runs) starts on an issue, thirdshift labels the issue `in-progress`, in place of `ready-for-agent` if it has that. An issue with no `ready-for-agent` label still gets `in-progress`, so the label means the same thing however the Run was started. diff --git a/src/ci.rs b/src/ci.rs index dfc914b71..9074a26ff 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -271,7 +271,7 @@ fn watch_to_end( rerun_checks: Option<&RerunChecks<'_>>, ) -> Result { let short = short(sha); - let mut reported_pending = None; + let mut reported_counts = None; let checks = until(outside, |outside| { let checks = outside.checks_on(sha)?; ensure!( @@ -282,12 +282,13 @@ fn watch_to_end( if pending == 0 { return Ok(Some(checks)); } - if reported_pending != Some(pending) { + let counts = (pending, checks.len()); + if reported_counts != Some(counts) { outside.step(format!( - "CI on {short}: {pending} of {} checks still running", + "CI on {short}: {pending} still running out of {} checks listed so far", checks.len() )); - reported_pending = Some(pending); + reported_counts = Some(counts); } Ok(None) })?; @@ -612,14 +613,80 @@ mod tests { github.steps, [ "waiting up to 60s for CI on abcdef0", - "CI on abcdef0: 2 of 2 checks still running", - "CI on abcdef0: 1 of 2 checks still running", + "CI on abcdef0: 2 still running out of 2 checks listed so far", + "CI on abcdef0: 1 still running out of 2 checks listed so far", "CI passed on abcdef0", ] ); assert_eq!(github.reads_of(BASE), 0); } + #[test] + fn dependent_jobs_can_raise_both_pending_and_listed_counts() { + let mut github = InMemory::new().listing( + HEAD, + vec![ + vec![pending("build"), pending("lint")], + vec![pending("build"), pending("lint")], + vec![green("build"), pending("lint")], + vec![ + green("build"), + pending("lint"), + pending("test/linux"), + pending("test/macos"), + ], + vec![ + green("build"), + green("lint"), + green("test/linux"), + green("test/macos"), + ], + ], + ); + + let ci = watch_through(&mut github, HEAD, Some(BASE)).unwrap(); + + assert!(matches!(ci, Ci::Passed)); + assert_eq!( + github.steps, + [ + "waiting up to 60s for CI on abcdef0", + "CI on abcdef0: 2 still running out of 2 checks listed so far", + "CI on abcdef0: 1 still running out of 2 checks listed so far", + "CI on abcdef0: 3 still running out of 4 checks listed so far", + "CI passed on abcdef0", + ] + ); + } + + #[test] + fn a_changed_listed_count_is_reported_even_when_pending_is_unchanged() { + let mut github = InMemory::new().listing( + HEAD, + vec![ + vec![pending("test")], + vec![pending("test")], + vec![pending("test")], + vec![pending("test"), green("lint")], + vec![pending("test"), green("lint")], + vec![green("test"), green("lint")], + ], + ); + + let ci = watch_through(&mut github, HEAD, Some(BASE)).unwrap(); + + assert!(matches!(ci, Ci::Passed)); + assert_eq!( + github.steps, + [ + "waiting up to 60s for CI on abcdef0", + "CI on abcdef0: 1 still running out of 1 checks listed so far", + "CI on abcdef0: 1 still running out of 2 checks listed so far", + "CI passed on abcdef0", + ] + ); + } + #[test] fn the_base_branch_commit_is_read_once_after_every_check_has_finished() { let mut github = InMemory::new() @@ -1008,9 +1075,9 @@ mod tests { assert_eq!(listed(&failed.on_base), ["lint: https://head/lint"]); assert_eq!(github.reruns, [1]); assert!( - github - .steps - .contains(&"CI on abcdef0: 1 of 2 checks still running".to_string()) + github.steps.contains( + &"CI on abcdef0: 1 still running out of 2 checks listed so far".to_string() + ) ); } @@ -1058,9 +1125,9 @@ mod tests { assert!(failed.inherited.is_empty()); assert_eq!(github.reruns, [1, 2]); assert!( - github - .steps - .contains(&"CI on abcdef0: 1 of 3 checks still running".to_string()) + github.steps.contains( + &"CI on abcdef0: 1 still running out of 3 checks listed so far".to_string() + ) ); } diff --git a/tests/ci.rs b/tests/ci.rs index 53f2e1165..7ab174b69 100644 --- a/tests/ci.rs +++ b/tests/ci.rs @@ -66,7 +66,9 @@ fn checks_pending_then_green_is_success() { assert_eq!(result.code, Some(0), "stderr: {}", result.stderr); assert_eq!(result.stdout, "https://github.com/acme/widgets/pull/1\n"); assert!( - result.stderr.contains("1 of 2 checks still running"), + result + .stderr + .contains("1 still running out of 2 checks listed so far"), "stderr: {}", result.stderr ); @@ -303,7 +305,9 @@ fn a_repair_that_already_re_ran_ci_to_green_delivers_or_self_merges_the_unchange assert_eq!(scenario.gh_state()["prs"][0]["isDraft"], false); if pending { assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 still running out of 1 checks listed so far"), "{}", result.stderr ); @@ -385,7 +389,9 @@ fn a_refused_request_still_waits_for_an_earlier_accepted_workflows_new_attempt() result.stderr ); assert!( - result.stderr.contains("1 of 2 checks still running"), + result + .stderr + .contains("1 still running out of 2 checks listed so far"), "{}", result.stderr ); @@ -474,7 +480,9 @@ fn a_refused_request_pending_then_red_is_a_declined_ci_fix_without_another_repai result.stderr ); assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 still running out of 1 checks listed so far"), "{}", result.stderr ); @@ -651,7 +659,9 @@ fn the_attempt_before_a_check_re_run_is_never_taken_for_its_result() { result.stderr ); assert!( - result.stderr.contains("1 of 1 checks still running"), + result + .stderr + .contains("1 still running out of 1 checks listed so far"), "stderr: {}", result.stderr );