Skip to content

Keep Review Passes out of live issue Delivery - #2073

Merged
JacobStephens2 merged 3 commits into
mainfrom
issue-2037
Nov 12, 2026
Merged

JacobStephens2 merged 3 commits into
mainfrom
issue-2037

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Nov 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

A Review Pass could take a PR while a live Ticket run or Spec run still held its closing issue, stranding Review fixes when Delivery pushed again. Author Passes now check closing issues before and after taking the PR reservation.

Review Pass candidate
  page linked closing issues
  Claim or issue reservation held -> log PR, issue and signal; try next PR
  reserve PR -> recheck closing issues -> inspect worktree -> Review run

Observations preserve repository identity and fail closed. Explicit PR reviews keep their current behavior. This admission guard does not exclude a Delivery started later; existing captured-head checks still apply.

Closes #2037

Evidence

Execution evidence

Seams: the CLI against fake gh and real Git, with two thirdshift processes in separate clones sharing a bare origin. The existing regression file was copied into a detached checkout at the supplied merge base; baseline production code was unchanged.

  • Before: at 482507a4edaa4bb1737d4e40b751995f05a48b4c, cargo test --test review_pass delivery -- --nocapture failed all four direct/Pickup Ticket/Spec cases with Review took a pull request during live Delivery; test result: FAILED. 0 passed; 4 failed.
    After: the same command on issue-2037 passed: test result: ok. 4 passed; 0 failed.
  • cargo test --test review_pass -- --nocapture: 25 passed, 0 failed, 1 ignored fixture entry point. Covers reservation-only holds, release, pagination, multiple issues, cross-repository identity, post-reservation recheck, observation failures, next-candidate selection, no linked issue and explicit-URL behavior.
  • npm run docs:check --prefix site: passed. Browser captures assert the updated documentation is present and has no horizontal page scrolling at 1440×900 and 390×844.
  • NEXTEST_TEST_THREADS=3 CARGO_BUILD_JOBS=2 /usr/bin/time -p scripts/affected-tests main: site tests passed (68/68, 77.97s). The Rust run stopped after 543 tests: 542 passed and one pre-existing Console timing assertion failed; 3701 were not run after fail-fast. Rust wall time was 748.80s including 248s compilation; total selector wall time was 827.02s.
  • The failure in tests/console/tokens.rs:507 compares full API and CLI snapshots; only stage.age_seconds differed (86400 versus 86401). Neither this assertion nor Console production code is modified by this PR.
  • cargo test --test console tokens::read_scope_cannot_read_security_findings -- --exact --nocapture: focused rerun passed (1 passed, 0 failed).
  • Full-suite CI passed on the exact head cb83d141bc7f60c4ef8c042b625bcdb500e9d3d6: 4320 tests passed, 100 skipped; Rust execution took 1545.246s after 89s compilation. Formatting, clippy, site and Console browser checks also passed. Successful CI covers the remaining suite after the local fail-fast result; no duplicate full-suite run was started.

Confirmed cause: PR reservations and Claims do not cover differently numbered closing issues. The old Pass never observed those issue holds. The added read-only guard observes both the Claim used by direct commands and the origin reservation used by Pickup.

Visual evidence

Matching states show the Review Pass documentation, with the homepage's supporting details open. Captures are hosted in a secret Gist.

Homepage

1440×900

Before
After

390×844

Before
After

User manual

1440×900

Before
After

390×844

Before
After

Architecture

1440×900

Before
After

390×844

Before
After

How it works

1440×900

Before
After

390×844

Before
After

Review coverage

  • Standards: 0 findings. All 11 changed files read.
  • Spec: 0 findings. All 11 changed files read.
  • Unread changed files: none on either axis. Generated site/docs/index.html was read directly; no generated-output exclusions.

Unaddressed findings

  • Standards: none.
  • Spec: none.
  • Security: none.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

Reverting restores the previous admission behavior. No persistent format or reservation ownership changes.

Blast Radius: Review

Author Passes make additional read-only GitHub and origin requests. An unavailable hold observation stops admission; retained issue reservations continue to require operator recovery.

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

JacobStephens2 and others added 3 commits November 11, 2026 11:59
2026-11-11T17:33:14Z, host dev.etadventures.com. Uncommitted work at the time of failure is included in this commit.
@JacobStephens2
JacobStephens2 merged commit 35169e4 into main Nov 12, 2026
24 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-2037 branch November 12, 2026 02:05
@JacobStephens2 JacobStephens2 mentioned this pull request Nov 12, 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.

A Review Pass takes a pull request that a live Ticket run or Spec run is still delivering

1 participant