A Fix Pass records a failure for every ready pull request whose head has no agent review #2020

Activity

1. JacobStephens2

   added

   bugSomething isn't workingSomething isn't working

   needs-triageMaintainer needs to evaluate this issueMaintainer needs to evaluate this issue

   in-progressA Claim: thirdshift has taken this issueA Claim: thirdshift has taken this issue

   and removed

   needs-triageMaintainer needs to evaluate this issueMaintainer needs to evaluate this issue

   on Nov 11, 2026
2. JacobStephens2 commented on Nov 11, 2026

   

   JacobStephens2

   on Nov 11, 2026

   OwnerAuthor

   More actions

   This was generated by AI during triage.

   Investigation summary

   Confirmed on selected Base commit de91156. A scratch reproducer ran two successive fix merge no-base-fix no-email Passes against real fixture Git repositories and fake services. Each selected a different unreviewed PR, successfully repaired CI, left the PR open and ready, then exited 1 and recorded outcome=failed. Reservations and in-progress labels were released.

   Diagnosis and evidence

   • src/fix.rs:62–90 selects candidates using needs_fix, which checks conflict/current-head CI eligibility without checking review evidence (src/github/review.rs:75–108).
   • After worktree acquisition, src/fix.rs:175–184 checks the actual adopted head and sets a Self-merge hold when evidence is absent.
   • src/pull_request.rs:550 raises MergeHold. Delivery preserves the ready PR, but src/run_ending.rs:551–568 classifies this hold as failed. src/logs/reading/inbox.rs:234 consequently identifies a failed Command for Needs you.
   • tests/standards_1795.rs:91 explicitly expects exit 1 for this hold, so the failure behavior is intentional under the current contract.
   • Evidence includes exact-head authored_head as well as reviewed_head or pushed_head from the authenticated account's trailing marker (src/github/review.rs:196–205,248–272). A fix must retain all currently accepted evidence and the acquired-head check.

   Recommended next step

   Prefer skipping heads without this evidence during merge-goal Fix Pass selection, logging the review requirement and continuing to the next candidate. Resolve the effective goal from flags and User config. Keep no-merge repair eligibility and targeted Fix behavior unchanged. Preserve the acquired-head guard against branch movement; missing or stale evidence must never permit Self-merge. This keeps ADR 0004's merge rule intact.

   The Day shift must select whether repair should instead proceed and finish successfully Ready for review despite the requested merge. That changes the outcome contract and requires different implementation and tests.

   After the decision, cover candidate ordering past an unreviewed PR, all-skipped success without failure notification, authored/reviewed evidence, merge defaults and explicit no-merge, reservation cleanup, and acquired-head races. Update the Fix-run glossary/manual and hand-written site descriptions with the chosen behavior.

   Validation

   cargo test --test standards_1795 --test fix_pass --test fix_run: 24 passed, 3 fixture entry points ignored. Standalone scratch reproducer: 1 passed, 1 fixture entry point ignored; evidence at target/triage-evidence/repro.rs and target/triage-evidence/repro.log. scripts/affected-tests main: no changed files, no affected tests. Source files and Git history remain unchanged.

   Questionnaire

   For the Day shift deciding issue #2020:

   The repository reproduces the reported failures: two successive merge-goal Fix Passes repaired two unreviewed PRs but recorded failed Commands. The exact-head review gate correctly prevented Self-merge.

   1. Should a merge-goal Fix Pass skip PRs lacking recorded authorship or review evidence and continue searching (recommended), or repair them and finish successfully Ready for review? Skipping defers repair until evidence exists; successful handoff requires changing the requested-merge outcome contract.
   2. If choosing successful handoff, should it apply only to Pass-dispatched Fix runs or also targeted Fix runs? Recommend limiting this issue to Passes and retaining existing outcomes for other policy, security and protected-path holds.
3. JacobStephens2

   added

   ready-for-humanRequires human implementationRequires human implementation

   and removed

   in-progressA Claim: thirdshift has taken this issueA Claim: thirdshift has taken this issue

   on Nov 11, 2026