Repository navigation
Wait for Review pushes to appear in GitHub - #2168
Merged
Merged
Conversation
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
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_headrejection to rename.Closes #2164.
Evidence
Execution evidence
cargo test --test review_head_lag -- --nocapturefailed withstep delivery: 0s failed: pull request changed during Review runimmediately afterpushing 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.9b47d13433fea405a3436fe8d841a7e77a00e806, asdocs/testing.mddirects: all four Rust partitions and the Rust coverage gate passed, along with Console and site checks.CARGO_BUILD_JOBS=3 NEXTEST_TEST_THREADS=3 scripts/affected-tests mainwas 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
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 commit4d24f79d170c8e72be238f405efc4d3215962375): 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