Skip to content

Wait for Review pushes to appear in GitHub - #2168

Merged
JacobStephens2 merged 1 commit into
mainfrom
issue-2164
Nov 13, 2026
Merged

JacobStephens2 merged 1 commit into
mainfrom
issue-2164

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Nov 13, 2026 •

Copy link
Copy Markdown
Owner

Summary

Review Delivery could fail immediately after pushing its own fix because GitHub still reported the pre-push head. Wait within the existing 60-second observation grace period after both initial and CI Repair pushes, tolerating only the pre-push head until the pushed head appears.

push fixes
  pushed head observed → watch CI on that exact head
  pre-push head observed → wait within grace period
  other head or changed branches/eligibility → fail with specific evidence

Head changes now name expected and actual commits; retargeting and branch renames name the changed branches. Base tip advancement remains permitted, as the existing Review tests and architecture documentation require; this checkout had no base_head rejection to rename.

Closes #2164.

Evidence

Execution evidence

  • Before: cargo test --test review_head_lag -- --nocapture failed with step delivery: 0s failed: pull request changed during Review run immediately after pushing colleague; result: 0 passed; 1 failed; 2 ignored. The fixture kept the old REST head for two reads after a real Git push, with no Base move or foreign push.
    After: The same command passed: 1 passed; 0 failed; 2 ignored. The confirmed cause was the one-shot head comparison after the run's own push.
  • cargo test --test review_head_lag --test review_run -- --test-threads=3: 4 new and 37 existing tests passed. Covers initial push lag, CI Repair push lag, deterministic grace expiration retaining the pushed-head marker, an actual foreign push after our push, and precise head/retarget/rename diagnostics. Existing Base-advancement tests still pass.
  • cargo fmt --check: passed.
  • Final validation reuses successful CI for the exact reviewed head 9b47d13433fea405a3436fe8d841a7e77a00e806, as docs/testing.md directs: all four Rust partitions and the Rust coverage gate passed, along with Console and site checks.
  • Local CARGO_BUILD_JOBS=3 NEXTEST_TEST_THREADS=3 scripts/affected-tests main was stopped after CI passed on that same head. It compiled the full suite and recorded 2,067 passing tests before cancellation; 3 in-flight tests received SIGINT and 2,451 selected tests were not run locally. This was not a passing local full-suite result. Measured wall time: 2,528.76 seconds including cold compilation; the retained log is .affected-tests/cargo-nextest-run-kda1dmwl.log.

Test seams

  • The thirdshift review <PR URL> CLI, real Git push to a local origin, and fake GitHub REST observations; assert successful Delivery, posted findings, and ready-for-review notification, or a precise refusal on changed heads or branch names.

Unaddressed findings

Standards

None.

Spec

None.

Security

None.

Security review outcome

No unaddressed introduced findings.

Review coverage

Independent Standards and Spec reviews against refs/remotes/origin/main (fixed commit 4d24f79d170c8e72be238f405efc4d3215962375): 0 Standards findings; 0 Spec findings. Both axes read all six changed files. Unread changed files: none. Generated-output exclusions: none. Final reports and files-read lists are retained with the Session trace.

Merge Danger

Door: two-way

Blast Radius: Review

Adds a bounded wait after Review pushes. Remote branch, eligibility, lease, mergeability and exact-head CI guards remain in place.

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

@JacobStephens2
JacobStephens2 merged commit d58cd1a into main Nov 13, 2026
29 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-2164 branch November 13, 2026 23:13
@JacobStephens2 JacobStephens2 mentioned this pull request Nov 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Review run fails with 'pull request changed' after pushing its own fixes

1 participant