Repository navigation
ci: gate master PRs on unresolved threads in stacked PRs - #5740
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e03f88d4d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Branch protection resolves conversations on master only, so a PR merged into another PR's head branch is never gated. A workflow now posts the stacked-review-threads commit status on each open master-bound PR: failure while any PR merged into its head, at any depth, has an unresolved review thread, naming each PR and thread URL. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
GitHub Actions is billing-locked here, so the workflow never ran; CircleCI is the live hosted check. The job evaluates the pipeline's PR and fails, naming each stacked PR and thread, while any is unresolved. Threads are now read only for PRs inside the stack, and traversal is keyed by parent PR so each lifetime of a reused child branch uses its own merge window. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
e03f88d to
11518c3
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 11518c3dd6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A child merged into a branch before that branch's PR opened, with the parent later squash-merged below the root, left its merge OID only in the parent's commit list. Membership now checks the root's and the immediate parent's commits. The GraphQL request drops its fixed socket timeout. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8779f41a93
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rged Commit ancestry and PR creation windows both fail once a parent is rebased or squashed, so the gate could pass a PR whose stacked code still carried unresolved threads. It now counts every PR merged into the root's head branch, and every PR merged into a stacked head before that head merged. A reused branch name can fail the gate by naming the old PR; resolving that PR's threads clears it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71238359d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A merged child whose head came from a fork names a branch in the fork, so walking that name in this repository picked up unrelated PRs. The child's own threads are still checked. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e4927d8ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CIRCLE_PULL_REQUEST names one PR. When a head also had a PR off master, the gate could evaluate only that sibling and pass. The job now passes every URL in CIRCLE_PULL_REQUESTS, and the gate skips PRs that do not target master. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg
## Summary Removes the CircleCI `stacked-review-threads` job, `devtools/stacked_review_threads.py`, and its tests. The `stacked-review-threads` commit status is now posted by a Sinnix user timer (`stacked-review-threads-status`, Sinity/sinnix 49d11e9b), next to the existing `codex-review` status pass. ## Problem The job, added in #5740, failed on every PR with "no GitHub token". The CircleCI project has no `GH_TOKEN`, and nobody can administer that project from this login. The job also re-ran only on push, so resolving a thread left the result stale until the next commit. ## Solution The Sinnix pass authenticates with gh's persistent login and runs every two minutes over every open PR targeting master. It keeps the semantics of the deleted module. Every PR merged into the head branch counts. The walk recurses into each stacked PR's head branch, bounded by that PR's merge time. Expansion is keyed per PR, so each lifetime of a reused branch name gets its own bound. The walk is cycle-safe and follows pagination on merged PRs and threads. It fails closed: a read error sets `pending`, never `success`. It writes a status only when the state changes. Reads are aliased per stack level, so a head with nothing merged into it costs no query of its own. With two authorities for one gate, one would be dead weight, so this PR deletes the CircleCI copy. ## Verification - `nix develop --command devtools verify --quick`: `verify: PASSED exit=0` (all 12 static gates ok). - `git grep -n "stacked_review_threads\|stacked-review-threads"` on this branch: no matches. - Before the deletion, both implementations were run read-only against every open master PR and gave the same verdict for all 25. - The live Sinnix pass posted `stacked-review-threads: success` on the open heads, for example #5643 (`1 stacked PR has no unresolved review threads`). ## Residuals - `stacked-review-threads` becomes a required master context after this merges. Until then it is advisory. - The gate now depends on the workstation timer running. If the timer stops, no new head gets the status, so once the context is required those merges stay blocked. That failure is closed, not open. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Summary
Adds a CircleCI job,
stacked-review-threads, to thegateworkflow. On a PR targeting master, it fails while any PR merged into that PR's head branch, at any stack depth, has an unresolved review thread. The log names each offending PR and thread URL.Problem
Master's branch protection requires conversation resolution, but a PR merged into another PR's head branch is never checked. #5682 merged into #5643's branch and #5687 into #5684's with 11 unresolved Codex threads between them; that code reaches master through the parent PR.
Solution
devtools/stacked_review_threads.py(stdlib only) walks merged PRs whose base is the root's head branch, then their head branches, recursively. Every PR merged into the root's head branch counts, and so does every PR merged into a stacked head before that head merged. Commit ancestry and PR creation windows are not used, because rebases and squash merges erase both. Each parent PR is expanded once, and threads are read only for PRs inside the stack..circleci/config.ymlgains thestacked-review-threadsjob. It skips non-PR builds and fork PRs, since nothing in this repository can merge into a fork branch. GitHub Actions is billing-locked on this repo, so CircleCI is the mechanism.Verification
nix develop --command devtools test tests/unit/devtools/test_stacked_review_threads.py:devtools test: PASSED exit=0(9 passed). The tests use synthetic GraphQL responses and cover two-level stacks, PRs merged before the root or the child opened, reused child lifetimes, post-merge grandchildren, thread pagination, and a branch cycle.ruff check,ruff format --check, andmypyon both files: clean.python3 -m devtools.stacked_review_threads --repo Sinity/polylogue --pr 5643exits 1 and names fix(intake): stream every ChatGPT node; refuse only unstorable values #5682 with 7 unresolved threads. The same run with--pr 5684names fix(sources): bind acquisition to location and refuse foreign content #5687 with 4.Residuals
no GitHub token: set GH_TOKEN or GITHUB_TOKEN. Add a read-only token (pull requests: read on Sinity/polylogue) as the project'sGH_TOKENvariable. After that, addci/circleci: stacked-review-threadsto master's required contexts.🤖 Generated with Claude Code
https://claude.ai/code/session_015pdKaZWyGe6Ngvsz468dzg