[JacobStephens2]

Summary

A Fix Pass (thirdshift fix base <branch>) used to take the lowest-numbered broken pull request even when its head branch was checked out locally (the main checkout or a retained Review worktree). It then failed at git worktree add on every cron tick, so higher-numbered pull requests never got repaired. Now it passes over such candidates and moves on to the next one:

 fix Pass
   base_ci::run                                   # unchanged, runs first
   for pr in fix_candidates (ascending)
-    if !needs_fix(pr) continue
+    branch = fix_candidate_branch(pr) or continue  # eligibility + head branch, one observation
+    if Worktree::occupying_checkout(branch) = path # git worktree list, under the worktree lock
+      stderr "passed over <PR URL>: head branch <b> is checked out at <path>"
+      remember reason; continue
     Reservation::take(pr) or continue
-    recheck needs_fix(pr, reserved)
+    recheck fix_candidate_branch(pr, reserved)
     run Fix run
-  skipped "no eligible pull request targeting <base> on <repo>"
+  skipped "no eligible pull request …; passed over <PR URL>: … at <path>"   # Activity log, exit 0

• The check leaves occupied checkouts and their registrations alone; it never prunes or deletes them (ADR 0014).
• An explicit thirdshift fix <PR URL> still fails as before, naming the occupying checkout.
• acquisition::registration_on is now the single lookup for "which registration has this branch checked out". The Review path's inspect_review and the Fix Pass both use it.
• Docs updated: docs/guide.md (Fix runs, plus the regenerated site/docs/index.html), GLOSSARY.md (Fix run), docs/code-navigation.md, site/index.html, site/how-it-works/, and site/architecture/.
• Also merges current origin/main (95 commits; no conflicts).

Evidence

Execution evidence

• Before: I ran the branch's tests/fix_pass.rs at merge base 3beacb7f: cargo test --test fix_pass -- occupied checked_out_locally

  thirdshift: git worktree add /tmp/.tmppkuuzQ/work/widgets-occupied occupied failed: fatal: 'occupied' is already used by worktree at '/tmp/.tmppkuuzQ/retained review'
    left: Some(1)
   right: Some(0)
  failures:
      locally_occupied_head_skips_without_starting_a_fix_run
      pass_repairs_the_next_candidate_when_the_first_head_is_checked_out_locally
  test result: FAILED. 0 passed; 2 failed; 0 ignored; 0 measured; 12 filtered out
  

  After: the same file on this branch: cargo test --test fix_pass

  test explicit_target_still_fails_with_the_occupying_checkout_path ... ok
  test locally_occupied_head_skips_without_starting_a_fix_run ... ok
  test pass_repairs_the_next_candidate_when_the_first_head_is_checked_out_locally ... ok
  test result: ok. 13 passed; 0 failed; 1 ignored
  

• Cause: fix.rs committed to the first candidate GitHub returned. It never looked at local git worktree list occupancy, which the Review Pass already checks.
• New tests:
  • When the only broken PR's head is checked out locally, the Pass skips twice and writes only one Activity log line. It writes no Command log, starts no session, and leaves no reservation.
  • When the first PR's head is checked out locally, PR Spec: thirdshift, turn a GitHub issue into a ready-for-review PR unattended #2 is repaired. This is tested with the occupying checkout in a retained worktree and in the Launch directory, and edits in that checkout are preserved.
  • An explicit target still exits 1 and names the occupying path.
• Other checks:
  • cargo clippy --all-targets -- -D warnings: clean.
  • cargo test --bin thirdshift: 1808 passed.
  • fix_run, review_pass and review_run suites: all passed.
  • npm test --prefix site: 68 passed (it runs docs:check).
• scripts/affected-tests main: fails, but only in tests/process findings:: tests that this branch doesn't touch. fixing_dispatches_each_reproduced_finding_and_keeps_unreproduced_findings_private fails the same way on a clean origin/main checkout (cargo test --test process findings:: there: 17 passed, 1 failed), and the others in that group fail only intermittently. Every other affected suite passed.

Visual evidence

None. The site edits change sentences inside existing paragraphs of hand-written and generated pages, with no markup, CSS or layout changes, so I took no screenshots. The site browser suite passes.

Merge Danger

Door: two-way

A Pass now skips a PR it used to crash on. Reverting brings back the crash loop and nothing else.

Blast Radius: narrow

The change only affects candidate selection in unattended Fix Passes. Explicit Fix runs and the Review path behave the same; the Review path only switched to the shared lookup helper.

Unaddressed findings

Standards

• S4: pass-over reasons are pre-formatted Strings rather than a typed Reason like Pickup's (src/ready.rs). Declined. The Fix Pass has exactly one pass-over kind (src/fix.rs:72-81), so an enum now would be the Speculative Generality that the same smell baseline warns against. Revisit if a second kind appears.

Spec

• P1: a registration whose checkout directory is missing is still treated as occupied, so its PR is passed over until someone prunes it. I ran the reviewer's test stale_prunable_registration_does_not_pass_over_the_candidate as written, and it fails as P1 predicts: passed over https://github.com/acme/widgets/pull/1: head branch occupied is checked out at /tmp/.tmpXmganJ/tmp-review. I declined to change the code anyway:
  • The issue only asks to pass over branches that "cannot be safely pruned".
  • ADR 0014 (docs/adr/0014-successful-review-acquisition-authorizes-stale-disposal.md) withholds disposal authority without ownership evidence. It even disables Git's automatic maintenance because that "can otherwise prune a damaged Worktree registration without the ownership inspector's authority".
  • Worktree::occupying_checkout (src/worktree.rs:170) follows that ADR, as does the Review path (a_missing_registered_review_reports_its_available_head_and_retains_registration).
  • Whether thirdshift should prune registrations that Git itself reports as prunable is for the Day shift to decide. Open issue Stale worktree registrations fail a Run's acquisition with 'cannot inspect every registration' #2041 already asks exactly that ("Choose in triage"), so I did not file a new one. Meanwhile the skip reason names the stale path, so the operator can run git worktree prune.

Review coverage

The reviewers read every changed file except site/docs/index.html. Both excluded it as generated output from docs/guide.md: npm run docs:check --prefix site exits 0, and the site browser suite passes.

Closes #2045

Built with claude 2.1.314 · opus · default effort

Security review outcome

No unaddressed introduced findings.