chadtmiller.com/knotless

Switch repository

Loading…

What changes

knotless-agent keep is a second job for knotless-agent beside review: an account, the owner or a collaborator, that keeps the pulls the merge queue turned away moving. For each open pull turned away at the head its branch is still at, which conflicts with its base now, it merges the base in as knotless update-branch does (A pull's branch updates from its base, keeping lines both sides added), queues it again, and comments, beginning Merged. Where both sides changed the same lines it pushes nothing, and comments once at that head and base naming the files, for the author.

It leaves alone a pull turned away for any other reason, one pushed to since (its author is at work on it), one whose body says Keeper: off on a line of its own, and one it has brought up to date --attempts times (3). It counts and dedupes by its own comments, so it keeps no state, and it runs no model. Queue records now ring the agent's door on Jetstream too.

Deploy: a keep compose service under its own profile, reading /etc/knotless/keep.env, off until it has an account.

Why

This is phase 1 of Queued pulls stall until their own author notices and moves them. Conflicts were 62 of 268 ejections from 10-22 to 11-02, 58% of those pulls were ejected again after being queued once more, and the files were mostly docs pages pulls add sections to side by side. Each author handles these by hand today. A failure the pull caused stays its author's to fix: the keeper does not touch it. Model-resolved conflicts and quarantine proposals come in later phases.

How it was checked

crates/knotless-agent/tests/keep.rs runs on the reference PDS with five pulls turned away as a runner does. The one with lines added at one place is updated with main's first, queued and commented on once. The one that changed a line gets a comment and its branch is left as it was. One with no conflict, one saying Keeper: off, and one pushed to since get nothing. A second read says nothing twice. Turned away again after main moves on, it is updated again only while under --attempts. Option parsing has unit tests. Builds and tests are left to CI.

Shortcomings

Only conflicts are handled. Turned-away pulls whose failure was not their own, such as a flaky test or a lost runner, are not requeued yet. It acts once the queue has turned a pull away, and does not wait for an author who may be merging main in by hand at the same time. The push is not forced, so one of the two simply fails. No account runs it on prod yet. AGENTS.md still tells agents to merge main in themselves, and will change once it does.

This adds knotless-agent keep. It also adds knotless update-branch and an MCP update_branch tool, built on branch_update.rs. For a pull the queue turned away at its current head and that now conflicts with its base, the keeper merges the base in and queues the pull again. Where git's conflicts are only lines both sides added, base first, it merges and comments. Where the same lines changed, it comments and pushes nothing. I read every changed file and found nothing that must change. The merge is not forced, trust is limited to the owner and collaborators, and the gating, Keeper: off, attempts and dedupe logic matches the docs. The docs, change files, compose service and tests are all there. test, check, e2e and docs were still queued when I looked, so I haven't seen them pass.

Notes, none blocking:

• Silent stall after a push. In keep, update_branch pushes first, then enqueue runs, then the comment is written. If enqueue or the comment fails after the push, the head has moved off the ejected commit, so no later sweep acts on the pull. It stays unqueued with no Merged comment, and the failure shows only as a keeping failed warning. The same sequence shows up in crates/knotless-agent/src/lib.rs at the Ok(updated) arm. Consider commenting before the enqueue, or logging this case distinctly.
• Conflict comments can pile up. The "conflicts" comment is deduped per head and base tip, and does not count toward --attempts. A pull its author never touches gets a new comment each time main moves. Cap it, for example by skipping once the keeper has already told the author at this head.
• Fork pulls. For a pull whose branch is in another repository, update_branch bails with "in another repository". The keeper then logs a warning on every sweep and writes no comment. Skipping those quietly in keeps would be cleaner.
• Textual merge only. Taking base-first lines is a textual merge, so for code it can yield something that doesn't build, such as two additions to the same use list or enum. That is fine because the queue's pre-flights catch it, and the body already says the keeper only handles conflicts.

The only change since my last review is a merge of client/update-branch. It brings in one commit to the branch-update tests (a final newline, and checkout over a mode change). I found nothing in it that must change. My earlier notes were non-blocking, and none were addressed. They are still worth a follow-up:

• A failure after the push leaves the pull unqueued and uncommented.
• Conflict comments are not capped.
• Fork pulls warn on every sweep.

I haven't looked at the check results for this head.

The only change since my last review is a merge of client/update-branch. It brings in one commit that changes how the branch update test counts a merge's parents apart from its subject. It touches tests only, and I found nothing in it that must change.

The check and docs checks pass at this head. test and e2e were still running when I looked, so I haven't seen them pass.

My earlier notes were non-blocking, and none were addressed. They are still worth a follow-up:

• A failure after the push leaves the pull unqueued and uncommented.
• Conflict comments are not capped.
• Fork pulls warn on every sweep.

The only change since my last review is a merge of client/update-branch. It brings in one commit, "update-branch writes each added path's line whole, which coverage counts". That commit only changes how the added-path lines are written, to satisfy coverage. I did not re-read its diff line by line. I found nothing that must change.

At this head test, e2e, docs and check were still queued, so I haven't seen them pass. The other workflows were skipped.

My earlier notes were non-blocking, and none were addressed. They are still worth a follow-up:

• A failure after the push leaves the pull unqueued and uncommented.
• Conflict comments are not capped.
• Fork pulls warn on every sweep.

Two commits are new since my last review. One makes the keeper pass over pulls whose branch is in another repository, and logs as review does. The other changes docs to say it reads pulls from the repository's own branches. In keeps, the here check (head repo key and owner DID) now skips fork pulls quietly. That answers my fork note, and I found nothing that must change.

At this head test, e2e, docs and check were still queued, so I haven't seen them pass. The other workflows were skipped.

Two earlier notes are still open and non-blocking:

• A failure after the push, in enqueue or the comment, leaves the pull unqueued and uncommented.
• Conflict comments are not capped, so a pull its author never touches gets a new one each time main moves.