chadtmiller.com/knotless

Switch repository

Loading…

What changes

The queue is opt-in per branch. A branch uses the merge queue only when its rule says queue (knotless branch protect <url> <branch> --queue, or Require the merge queue in settings). Without the rule, pulls merge directly: the merge box shows no queue button, the pulls list and search palette show no link, and knotless pull merge --queue, pull_merge's queue and dev.knotless.queuePull refuse, naming the command that turns it on. There is no longer a mode where a branch offers both.

Stacks follow the branch under them. A pull in a stack can be queued when the branch under the whole stack has the rule.

Nothing queued is stranded. A pull already queued when the rule goes keeps its place, keeps Leave queue, and a runner still lands it. The pulls list and palette link to the queue while it waits there. Merge pull and Leave queue are equal weight then.

The keeper follows. knotless-agent keep leaves an ejected pull alone once its branch's rule has dropped the queue, rather than bringing it up to date and failing to queue it again.

Settings. The checkbox says the merge button goes away, and is disabled with a link to Actions while the repository names no runner. getMergeStatus's may.queue carries the rule, and RepoViewDetailed gains mergeQueue and getRepoSettings gains runners. The queue page says no branch uses the queue, with a link to settings for those who may change it.

Why

The queue showed on every repository that names a runner, whatever its rules said, so "Require the merge queue" read as turning it on when it only took the merge button away. GitHub has the one switch, on the branch's protection rule, and this does the same.

The cost is that queueing a pull while its checks run, to have it land by itself, goes away on a branch with no rule. Merge when checks pass is the follow-up for that.

How it was checked

The tests that queue now turn the rule on, and forge.rs and mcp.rs cover the refusal, a rule dropped while queued, and leaving. The e2e flow that merges by hand turns the rule off first. The rest is left to CI.

Shots-moved: pulls, pulls-closed, settings-branches

Shortcomings

An older client still writes queue records for any branch, as the check is in the client, and the runner lands them.

🤖 Generated with Claude Code

This makes the merge queue opt-in per branch. enqueue now refuses a pull unless takes is true, meaning the branch under its whole stack has a queue rule. A pull that is already queued keeps its place and its Leave queue. may.queue, mergeQueue, runners, the pages, docs, lexicons and tests follow from that. I found nothing wrong in the logic. I read the whole diff, but I did not read the Svelte or Rust files around it. The checks were still running when I looked (check and e2e running, the rest queued), so I have no results from them.

Two things to do around the merge. Neither blocks it.

• Once this lands, this repository's own main needs knotless branch protect <url> main --queue. AGENTS.md's "Landing a change" tells agents to land with pull merge --queue, and that is refused on a branch with no rule. Either set the rule on main or reword that section. branch_rules was denied to me, so I could not see whether main already has it.
• knotless-agent keep fails its sweep of a pull whose rule was dropped after it was ejected. The body lists this as a known shortcoming. An issue to track it would help.

This round adds the keeper change I flagged last time. knotless-agent keep now skips an ejected pull when queue::takes is false for it, so it no longer brings the pull up to date and then fails to queue it. takes is public and takes the arguments the call passes. The keep test now drops the rule and checks the pull is left alone and nothing more is said. It then restores the rule and checks the pull is queued again. This answers my earlier request, and I have nothing new to ask for. I read only the new commit's diff, not the surrounding files, and I did not look at the check results. My other earlier note still stands: this repository's own main needs the queue rule set, or AGENTS.md's "Landing a change" needs rewording.

This round has a test fix and a merge of main. Since my last review, a_merge_queue_waits_for_its_gate now sets --queue (and --merges) alongside --gate, so it still queues under the opt-in rule. That fix is correct. The getCommit first-parent diff, its test and the API doc line are in the diff only because of the merge from main. They look sound and are unrelated to the queue. I read only the diff since my last reviewed commit, and I did not read the check results.

I have nothing new to ask for. The note from before still applies: this repository's own main needs knotless branch protect <url> main --queue, or AGENTS.md's "Landing a change" needs rewording. The pull is already queued, which suggests the rule is set. I did not check.

This round adds the last change you described: the pages link to the queue while a pull waits there, even after the rule has gone. Repo::queues returns true when a rule has queue, or when an open pull is still in state queued. RepoViewDetailed.merge_queue now carries that, so the pulls list and palette link to the queue. The lexicon description and the design doc are updated to match. assembled returns a cached Arc, so calling it in head should not cost much. The gateway test covers the case: it queues a pull, then removes the rule with unprotectBranch, and mergeQueue stays true. After the pull leaves, mergeQueue is absent. I found nothing wrong. I read only the diff since my last reviewed commit, and I did not read the check results.

This round touches two Svelte files. The empty queue page now links to branch settings for the repository's owner or an admin (it was admins only), matching who sees the Settings tab in RepoPage. The MergeBox queue-only sentence is split in two with the same meaning. I found nothing wrong. I read only the diff since my last reviewed commit, and I did not check the viewer fields (data.viewer.actor.did, data.repo.viewer.admin) against RepoPage or read the check results. My earlier notes are unchanged, and the pull is already queued.

It used its 5 rounds, approving each. Only c7d165a2 is unreviewed: a same-behaviour rewrite of Repo::queues dropping an unreachable return, and a test for coverage. All seven checks pass at this head. Dismissed with the owner's agreement.