~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/knotless

git that lives in your Atmosphere account

knotless.dev
1 collaborator

knotless 0.13.0 Latest 2 packages, 1 image

1
10

A branch waits for a review agent's verdict before its pulls land

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/knotless:queue/review-gate

What changes

Gates. A branch's rule can name gates: accounts, such as the review agent, whose review of a pull's head the branch waits for, though they may not write to the repository (knotless protect <url> main --gate review.knotless.dev, MCP protect_branch with gates, reviews.gates in the repository record). A gate's newest verdict stands. With none yet, or one of an earlier head, the pull waits. changes blocks it at any head until the gate reviews again. approve of the head lets it go, and counts toward no approvals. An approval of the head from the owner or a collaborator, given after the gate's newest review, stands in for that gate's.

The merge queue. Queueing refuses a pull a gate asked changes of, and takes one still waiting for a gate. The runner runs its pre-flights meanwhile, but leaves it out of batches until the gate has reviewed its head. It is ejected, saying so, after --review-wait (30 minutes), so a gate that is down holds a pull no longer than that. A pull waiting so reads Queued, 2nd of 5, waiting for its review (stage review).

Elsewhere. knotless rules lists gate <did>. merge --check lists a review of its head from @<gate> while one is awaited. The gateway and plain merge hold to it as to any review rule. The settings page keeps a rule's gates when another of its boxes is toggled.

Why

The review agent's verdict is advice, since its account may not write, so pulls landed over its requests for changes: 8 merged since it started reviewing on 2026-10-30. Agents queue a pull as soon as it is pushed and do not come back, so telling them to read reviews first would not hold. Other forges let only writers block. This keeps that for approvals, but lets a named account veto without write access, as Gerrit's -2 does, and makes the queue wait for its verdict as it waits for pre-flights. A collaborator's approval is the way past a wrong or stalled gate.

How it was checked

Unit tests of how gates stand (approvals::tests::a_gate_is_waited_for_and_its_changes_block), of the wait (a_pull_waits_for_its_gates_a_while), and a runner test over a PDS (a_merge_queue_waits_for_its_gate): changes turn a pull away, an approval lands it, and one never reviewed waits, then is ejected. The rest is left to CI.

Shortcomings

A client that does not know gates refuses to merge or queue into a branch that names one, as with any rule field it does not know. Turn it on for main only once a release that knows it is installed where agents run. Without pre-flights, a pull waiting for its gate reads only Queued, 2nd of 5. The settings page has no control for gates yet; that follows in its own pull.

Review

Start at gated in crates/knotless/src/approvals.rs, then reviewed in crates/knotless-run/src/queue.rs.

🤖 Generated with Claude Code

Approved

This adds gates to a branch's review rule. A gate is an account without write access whose review of the head the pull waits for. Its changes verdict blocks the pull until it reviews again. A writer's later approval of the head stands in for it. The merge queue holds a pull that lacks only a gate's review, runs its pre-flights meanwhile, and ejects it after --review-wait.

I read gated, stand, Standing::awaits and missing in approvals.rs, and reviewed and preflown in queue.rs. I did not read the full diff: it was too large for the tool, so I did not read the CLI, MCP, gateway or runner-test changes. I found nothing wrong in what I read:

  • Reviews by the pull's author are already dropped in reviews::standing. So the stand-in rule can't be used to self-approve past a gate.
  • A gate's approval is not counted toward approvals, because uncounted rejects non-writers.
  • A rule field this version doesn't know in a gate (gates.*) is reported as unknown, so a client refuses rather than loosening the rule.
  • The change files, docs (collaboration.md Gates) and lexicon descriptions are present.

At this head every check is still queued, none has run. The 100% coverage gate in test is the one most likely to catch something, so wait for it before queueing.

Approved

This round has one new commit, and it changes stand in approvals.rs. The old changes.extend(gate_changes.filter(|g| !changes.contains(g))) borrowed changes both mutably and immutably, so it likely would not compile. The new loop pushes each gate's changes verdict once, with the same result as before. I found nothing wrong with it, and it needs no extra tests: the existing gate tests exercise that path.

My earlier review's findings still hold. At this head the checks are still queued or running, so the 100% coverage gate in test has not run. Wait for test to pass before queueing.

Approved

This round has one new commit. It adds the review_wait field, set to 30 minutes like the runner's default, to the Options literals in crates/knotless-run/tests/run.rs. Without it those literals would not compile. I found nothing wrong. The change is mechanical and the value matches the runner's default.

My earlier findings still hold, and nothing I asked for is outstanding. At this head docs has passed. test, check, e2e, web, shots and lexicons are still queued or running. Wait for test, which holds the 100% coverage gate, and for check, before queueing.

Approved

This round has one new commit. In the runner test a_merge_queue_waits_for_its_gate, it sets KNOTLESS_IDENTIFIER to the gate sandbox's handle, so the gate's reviews are made as its own account. I found nothing wrong. It is a test-only fix that matches the test's intent.

My earlier findings still hold, and nothing I asked for is outstanding. At this head docs has passed. check and e2e are running, and test, web, shots and lexicons are still queued. Wait for test, which holds the 100% coverage gate, and for check, before queueing.

Approved

This round has one new commit. In a_merge_queue_waits_for_its_gate it reads the refusal from stderr through knotless(), which returns (ok, stdout, stderr), instead of from stdout through cli(). That fixes the failure in the last test run, where the refusal text ("changes asked by @…") was printed correctly but the assertion looked in the wrong stream. I found nothing wrong. It is a test-only change, and the knotless helper exists with that signature in the test harness.

My earlier findings still hold and nothing I asked for is outstanding. At this head test is running and the other checks are queued. That test run is the first to cover the gate test after this fix. Wait for test, which holds the 100% coverage gate, and for check, before queueing.

Merged as 44e7207