Skip to content

feat(kix): merge clean pull requests from the decision inbox - #163

Merged
kixelated merged 3 commits into
mainfrom
quest/m0/kix/merge
Oct 21, 2026
Merged

kixelated merged 3 commits into
mainfrom
quest/m0/kix/merge

Conversation

@kixelated

Copy link
Copy Markdown
Collaborator

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:

    • the item is idle, meaning no queued, running, or waiting job holds the PR,
    • the project's observations are complete,
    • the PR is OPEN, and
    • GitHub reports mergeStateStatus as CLEAN or HAS_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/merge route checks again through enrolledProject, then once more inside mergePullRequest.

  • The form binds the headRefOid on screen. mergePullRequest sends it as sha with the first method the repository allows (squash, then merge commit, then rebase).

  • Refusals are never retried:

    • GitHub's 409 (moved head) reads "The pull request's head moved since you looked; review it again".
    • A 403, 405, or 422 refusal shows GitHub's own message, for example "GitHub refused the merge: At least 1 approving review is required…".
    • The route also refuses while a job still holds the PR.
  • 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)

  1. "Clean" means CLEAN or HAS_HOOKS only. UNSTABLE (non-required checks failing), BEHIND, BLOCKED, DIRTY, DRAFT, and UNKNOWN hide the button. These are the cases that merge jobs will widen to. Recommendation: keep.
  2. GitHub's refusal text is shown. #api gained an opt-in explain flag that only mergePullRequest uses. It keeps the response's message field 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.
  3. No custom review requirement. GitHub's branch rules decide whether reviews are needed. An unreviewed PR in a repository without required reviews is CLEAN and can be merged by the maintainer's click. Recommendation: keep. The maintainer's click is the review decision.
  4. The merge is offered on failed-job (retry) PR items as well as idle ones. Recommendation: keep.
  5. A one-time unread bump. Adding mergeStateStatus to stored PR data changes each open PR's observation on the next refresh, so those items show as new once. Recommendation: accept.

Quest tree

  • Deleted quest/m0/kix/merge.md and removed it from the kix epic's Required list and from merge-job.md's Required list.
  • merge-job.md now names where the clean-only gate lives, so that quest can widen it.
  • Reworded the reference in landed-quests.md.
  • cloudflare/GITHUB.md documents 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.
  • New inbox tests cover:
    • the button and its head binding,
    • a moved head (409),
    • a refused merge (405 shown in GitHub's words, not retried),
    • a squash-only repository,
    • a non-clean (BLOCKED) PR without the button,
    • a PR held by a job (no button, and the server refuses),
    • a non-maintainer (refused before any merge request).

Suggested follow-ups

  • The merged item stays visible to co-maintainers until the next refresh, because the skip is per-maintainer. Webhook reconcile already covers this. Recommendation: no new quest.
  • Show the merge state in the item's meta line (for example "Merge: blocked by branch rules") so the maintainer knows why there is no button. Recommendation: fold this into merge jobs when it widens the button, rather than a separate quest.

🤖 Generated with Claude Code

(Written by Claude Opus 5.5)

kixelated and others added 2 commits October 21, 2026 10:21
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>
@kixelated

Copy link
Copy Markdown
Collaborator Author

Outcome: /quest-start finished the quest. It ran as a background /quest-spawn agent, so it asked the user nothing. just check and just test passed locally. The five decisions it made are listed in the PR description under "Decisions made without asking", each with a recommendation; the user has not confirmed them yet. /quest-ready has not been run.

(Written by Claude Opus 5.5)

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-21T18:49:25.152712Z b341bf6 New commits
🔒 Security Review ✅ Completed 2026-10-21T18:52:07.684387Z b341bf6 New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread cloudflare/src/index.tsx
Comment on lines +248 to +249
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);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.ts reads 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
@kixelated

Copy link
Copy Markdown
Collaborator Author

/quest-merge summary for b341bf6

Changes since the last review

  • Merged main, which brought in kixelated/quest#132. Two conflicts were resolved:
    • state.ts: mergeStateStatus moved into the shared pullFields. The history pass and the new occupancy pass (open PRs) both read it, so every refresh after an invalidation rereads the merge state of open PRs.
    • project-fixtures.ts: the merge mock is kept alongside fix(github): keep project availability live on busy repositories #132's live-target mock. The installation grants contents/pull_requests: write plus issues: read.
  • The rest stays coherent with fix(github): keep project availability live on busy repositories #132:
    • The merge button sits behind blocked(), which now gates on project.available.
    • The post-merge invalidation restarts only the occupancy pass, the same as a pull_request closed webhook.
    • jobSource's live reread narrows the race Codex raised.

Review

  • Codex reviewed b341bf6 and found nothing new.
  • I declined the earlier P1 (reserving the pull target before a merge) with reasons: a job that slips in cannot publish to a merged PR, and merge jobs will serialize merges through the jobs_active index.

Decisions: the user accepted the five decisions in the PR body.

Validation: just check and just test pass locally (Worker suite 259/259), and CI is green on b341bf6.

Follow-up suggested: GitHub briefly reports mergeStateStatus as UNKNOWN while it recomputes mergeability. That happens after a default-branch push, which invalidates observations. Each open PR's activity can then change twice (to UNKNOWN and back) and resurface as unread. Consider leaving mergeStateStatus out of change detection.

(Written by Claude Opus 5.5)

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.

1 participant