[JacobStephens2]

Summary

A Check re-run was abandoned when one failed job needs another failed job. GitHub creates the dependent job's new attempt only once what it needs has finished, often minutes later. thirdshift required every failed job's new attempt within the 60s grace period, so it reported no re-run appeared while GitHub's re-run was still going (flintmark #7: verified needs editors).

 watch_new_attempts            # grace period after the re-run request
-  wait for a new attempt of EVERY failed job in each requested workflow run
+  wait for a new attempt of AT LEAST ONE failed job in each requested workflow run
   none in time → "no re-run appeared on <sha> within 60s" (unchanged)
 watch_to_end
   pending = checks still running
+          + requested jobs whose old failed attempt is still listed
+          + requested jobs absent from the listing (jobs API filter=latest)
+  once nothing else is running, those get one more grace period:
+    an old failed attempt still listed → counts as failed (CI red)
+    a job still absent → "re-run checks never reappeared on <sha>; cannot verify CI"

The same watch serves the Abandoned job re-run, so that path gets the fix too. A new waiting line is CI on <sha>: 1 still running, plus 1 awaiting new attempts, out of 1 checks listed so far. The Consequences sections of ADR-0010 and ADR-0020, and the user manual (docs/guide.md, regenerated site/docs/index.html), now describe the new rule. The hand-written site pages don't describe how a re-run's start is detected, so they needed no change.

Closes #2098

Evidence

Execution evidence

The bug was reproduced at merge base acd9be19 by applying this branch's regression tests to a detached worktree there. The branch's src/ci.rs was not applied.

• Before (merge base):
  cargo test --bin thirdshift dependent

  test ci::tests::review_dependent_job_missing_from_latest_attempt_stays_pending ... FAILED
  test ci::tests::a_check_rerun_waits_for_a_dependent_job_beyond_the_start_grace ... FAILED
  …must not abandon the re-run: Some(CI red on abcdef0 and the Repair found nothing to fix on the branch); steps: ["re-running the failed checks on abcdef0: editors, verified", "no re-run appeared on abcdef0 within 60s"]
  test result: FAILED. 9 passed; 2 failed
  

  cargo test --test ci a_check_re_run_watches_dependent_jobs (fake GitHub: editors pending_polls: 6, verified stale_polls: 8)

  test a_check_re_run_watches_dependent_jobs_to_their_new_result ... FAILED
  thirdshift: no re-run appeared on 131eda2 within 0s
  

• After (this branch; the tests were later renamed and split):
  cargo test --bin thirdshift ci::tests → test result: ok. 64 passed; 0 failed
cargo test --test ci dependent → a_check_re_run_watches_a_dependent_job_to_its_new_success ... ok, a_check_re_run_watches_a_dependent_job_to_its_new_failure ... ok

Confirmed cause: RerunChecks::new_attempts_listed needed a new attempt of every requested failed check before the grace period ended. A dependent job can't get one until its dependencies finish.

Review-raised bound (Spec finding 3): the reviewer's test a_dependent_jobs_attempt_that_never_appears_does_not_wait_forever failed on the branch before this round with watch never ended: Some(interrupted). It is now kept as two tests:

• a_dependent_jobs_old_attempt_still_listed_a_grace_period_after_ci_settles_is_red
• a_dependent_job_still_absent_a_grace_period_after_ci_settles_cannot_verify_ci

Both pass.

Final validation: scripts/affected-tests main → PASS npm test --prefix site (33.40s), PASS cargo test (1001.23s)

Review

thirdshift-code-review against refs/remotes/origin/main found 8 Standards and 3 Spec findings. Fixed in this round:

• Standards 1: the waiting line is now documented in the guide.
• Standards 2: the new waiting line is pinned in a test.
• Standards 3, 4, 6: the awaiting counts are consolidated into RerunChecks::awaiting_attempts, with is_previous_attempt and same_workflow_check reused and one format string.
• Standards 7: the review_ test is renamed.
• Standards 8: the looped scenario is split into two tests.
• Spec 3: the wait is bounded.

Both reviewers read every changed file except site/docs/index.html. That file is generated, and both accepted excluding it: npm run docs:check --prefix site exits 0 and its diff only mirrors the docs/guide.md sentences.

Unaddressed findings

Standards

• 5. RerunChecks.observed: HashSet<usize> couples to indices of checks (Primitive Obsession, judgement call). Declined. checks is built once in request_reruns (src/ci.rs:431, checks: failed.own.iter().chain(&failed.inherited).collect()), it is a private field, and nothing pushes to, sorts or retains it afterwards, so the indices are stable for the struct's life.

Review of the commits merged from main (#2135 for #2052, #2136 for #2112), with fixed point e6ded0af. These come from their commits. S2 (redundant blank check in resolve_recommendation) and S3 (Fleet::text renamed to Fleet::with_machine_lines) were fixed in f432785. Both reviewers read every changed file. prompts/triage-correction.md, prompts/triage-issue.md, site/prompts/index.html and site/docs/index.html were left unread on purpose as generated output: cargo test --bin thirdshift prompts_page gave 14 passed, npm run docs:check --prefix site exited 0, and both reviewers checked that the output diffs mirror the source edits.

• S1, from their commits: the "questionnaire only for needs-info" rule is restated across skills/thirdshift-triage/SKILL.md and skills/thirdshift-to-questionnaire/SKILL.md, and the questionnaire skill keeps decision wording left over from ready-for-human (judgement call). Declined for now. Whether ready-for-human should carry a questionnaire is the open question in Confirm Triage should reject questionnaires on ready-for-human (reversed by #2135) #2144 (Spec P1), and pruning the skills now would pre-empt that decision. The repetition is also Treat empty-string Triage questionnaire as absent and show a ready-for-agent example in thirdshift-triage #2052's fix itself: its brief asked for a second example "reinforcing that questionnaire is present only when "state" is "needs-info"".
• S4, from their commits: the machine row is built in both Machine::text and src/console/metrics_fleet.html, and the label "unreadable accounts" differs from "Unreadable OS accounts" (judgement call). Declined. Fleet metrics drops unavailable_accounts from machine summaries #2112 specifies that wording: "render them in src/console/metrics_fleet.html's Machines list (for example · unreadable accounts: conductor, vsto)". One renderer is Rust text and the other is browser HTML, so there is no shared code to extract.
• S5, from their commits: Machine is built by struct literal in four places (judgement call, mostly pre-existing). Declined. Each literal fills its fields from a different source: local, live, saved, and the test. A constructor would take all of them as parameters, and the compiler already flags any literal that misses a new field.
• S6, from their commits: duplicated test setup for other_accounts and the blank-questionnaire Scenarios (judgement call). Declined. The setups sit in different test binaries (tests/console/metrics.rs, tests/console/metrics_report.rs), and docs/testing.md has no rule against inline Scenario setup.
• S7, from their commits: the new <p> in site/architecture/index.html (~293) sits outside .reading-content (judgement call). Declined. Three other <details> on that page use the same structure, so it follows the page's existing pattern.

Spec

• 1. New waiting-line wording is unrequested (scope creep, minor). Kept. The issue requires dependent jobs to be "treated as pending", and the line reports that pending state. It is now documented in docs/guide.md and pinned by a_dependent_job_absent_from_the_latest_attempt_stays_pending.
• 2. Handling of absent dependent jobs goes beyond the "old attempt" framing (scope creep, minor). Kept. GitHub's jobs listing (filter=latest) can omit the dependent job instead of listing its old failure, and the issue's "treated as pending, not as still failed" covers that case. The reviewer called it justified.

From their commits (#2135, #2136):

• P1, from their commits: Treat blank Triage questionnaires as absent #2135 dropped ready-for-human from needs_answer, which reverses Triage run: turn a Report into triaged issues #1323's rule that ready-for-human requires a questionnaire. Declined: the Day shift must decide, so I filed Confirm Triage should reject questionnaires on ready-for-human (reversed by #2135) #2144 (needs-triage). The behaviour is deliberate in Treat blank Triage questionnaires as absent #2135 and encoded by a test: cargo test --test triage_run questionnaires_are_required_only_for_needs_info gives 1 passed. Whether ready-for-human should carry a questionnaire is a product decision about the operator workflow, so a CI-fix branch should not reverse it either way.
• P2, from their commits: Machine::text hand-builds the Fleet machine line instead of the generic document walk, so a future Machine field could be missing from text (scope/design, minor). Declined. Fleet metrics drops unavailable_accounts from machine summaries #2112 asks to "mention unreadable accounts in text Machines lines", and 7e4bce5 ("keep Fleet machine diagnostics on one text line") chose this layout on purpose. Only machines is replaced. The rest of the report still goes through the shared walk (Fleet::with_machine_lines, src/logs/metrics/fleet.rs:134), and the reviewer found every current field rendered.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

A revert restores the previous strict grace check. No data, labels or GitHub state change.

Blast Radius: CI-watching

This only affects the watch after a Check re-run or an Abandoned job re-run. The worst case is a dependent job being judged one grace period after the rest of its workflow run settles.

CI notes

• console-test (run 43293144147): tests/working-targets.test.cjs failed once on a remote Fleet row's PR link (1440px remote review-pass: visible PR target). This branch changes only src/ci.rs, tests/ci.rs and docs, not the console or that test. The test passed in 6 of 6 local runs at this commit, and the full npm test --prefix console passed locally, so it looks intermittent.

Built with codex codex-cli 0.167.2 · gpt-6.1-sol · xhigh