Repository navigation
feat(kix): merge clean pull requests from the decision inbox - #163
Conversation
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A PR item offers "Merge this PR" when GitHub reports its merge state as CLEAN or HAS_HOOKS and no job holds it. The form binds the head on screen; mergePullRequest picks the repository's allowed method, and a moved head or GitHub's refusal is shown plainly and never retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Outcome: /quest-start finished the quest. It ran as a background /quest-spawn agent, so it asked the user nothing. (Written by Claude Opus 5.5) |
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2221b78d6c
ℹ️ 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".
| if (await active(c.env.DB, repo.id, target)) throw new JobError("Stop this pull request's job before merging it"); | ||
| await github.mergePullRequest(repo, session.user.id, Number(number), head); |
There was a problem hiding this comment.
Reserve the pull target before beginning the merge
This check is only a point-in-time read, while mergePullRequest performs several asynchronous GitHub requests before the merge lands. If another maintainer submits the Iterate action during that window, cloudflare/src/jobs/routes.ts can create and dispatch a job after this query has returned false; the merge can then succeed at the old head, leaving the newly started job publishing commits to an already-merged PR. Atomically reserve the target for merging, and make job creation reject that reservation, so the stated no-job-under-merge invariant cannot race.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Not fixing in this PR. The window is real, but its effect is a job that fails without publishing, not commits on a merged PR:
jobSource(from kixelated/quest#132) rereads the live pull request right before it creates a job, and refuses one that is not open at the observed head. A job can only slip in if that read happens before the merge lands.- A job that slips in still cannot publish. The iterate publisher in
cloudflare/src/github/service.tsreads the pull request again and refuses with "The pull request was closed or its head branch changed" when it is no longer open. - The other order is covered too: if a job pushes first, the merge is bound to the head on screen and GitHub refuses it with a 409.
An atomic reservation would need a new job state or table. Merge jobs will add a merge workflow, and the jobs_active unique index (one active job per target) will then serialize merges and iterations without extra code. Adding a reservation now would be a stopgap that merge jobs replace.
(Written by Claude Opus 5.5)
# Conflicts: # cloudflare/src/github/state.ts # cloudflare/test/project-fixtures.ts
|
/quest-merge summary for b341bf6 Changes since the last review
Review
Decisions: the user accepted the five decisions in the PR body. Validation: Follow-up suggested: GitHub briefly reports (Written by Claude Opus 5.5) |
Completes
quest/m0/kix/merge.md("[S] Merge PRs from the decision inbox"): before this, PR items in the decision inbox only offered "Review on GitHub".Behavior
A PR item now offers Merge this PR (primary, with "Iterate on this PR" demoted to quiet) when:
OPEN, andmergeStateStatusasCLEANorHAS_HOOKS.Otherwise the item keeps "Review on GitHub" and the existing actions. The activity scan now reads
mergeStateStatus. It is GA in GraphQL and needs no preview header (checked live).Only current maintainers get the button. The inbox already hides items on projects where the viewer fails the per-request maintainer check. The new
POST /projects/github/:owner/:repository/pulls/:number/mergeroute checks again throughenrolledProject, then once more insidemergePullRequest.The form binds the
headRefOidon screen.mergePullRequestsends it asshawith the first method the repository allows (squash, then merge commit, then rebase).Refusals are never retried:
message, for example "GitHub refused the merge: At least 1 approving review is required…".After a merge succeeds, the project's observations are marked stale, as a webhook would mark them. The item is set aside as finished for this maintainer, and the inbox moves to the next decision.
Decisions made without asking (routine; please confirm)
CLEANorHAS_HOOKSonly.UNSTABLE(non-required checks failing),BEHIND,BLOCKED,DIRTY,DRAFT, andUNKNOWNhide the button. These are the cases that merge jobs will widen to. Recommendation: keep.#apigained an opt-inexplainflag that onlymergePullRequestuses. It keeps the response'smessagefield as one line of display text, with control characters stripped and a 300-character cap. This is a narrow exception to "never include provider response bodies in errors". Every other call still reports only the status. Recommendation: keep, since the quest asks to show GitHub's refusal and say why.CLEANand can be merged by the maintainer's click. Recommendation: keep. The maintainer's click is the review decision.mergeStateStatusto stored PR data changes each open PR's observation on the next refresh, so those items show as new once. Recommendation: accept.Quest tree
quest/m0/kix/merge.mdand removed it from the kix epic'sRequiredlist and frommerge-job.md'sRequiredlist.merge-job.mdnow names where the clean-only gate lives, so that quest can widen it.landed-quests.md.cloudflare/GITHUB.mddocuments the inbox merge.Validation
just check: passed.just test: passed. That is 227 CLI tests plus 251 Worker tests, including the hook, installer, and lifecycle suites.BLOCKED) PR without the button,Suggested follow-ups
🤖 Generated with Claude Code
(Written by Claude Opus 5.5)