Skip to content

feat(work-implement): group a worktree drain's PRs behind one queue branch - #217

Merged
TitusKirch merged 2 commits into
devfrom
ai/175-queue-branch-per-drain
Aug 3, 2026
Merged

feat(work-implement): group a worktree drain's PRs behind one queue branch#217
TitusKirch merged 2 commits into
devfrom
ai/175-queue-branch-per-drain

Conversation

@TitusKirch

Copy link
Copy Markdown
Owner

Summary

Adds the opt-in work.queueBranch gate. Under branch: worktree a drain at cap opens up to ten PRs into pr.base, each triggering the repo's full CI for work that all lands on one branch anyway. With the gate on, every issue PR targets ai/queue-<hash> instead, and the drain opens a single ai/queue-<hash>pr.base PR to close the run — reusing one already open against pr.base rather than cutting a second beside it.

What the drain deliberately does not do is land that PR. No merge, no fast-forward, no bypass-capable credential on the machine running the loop. The target repo's own workflow mints its token in CI and fast-forwards the integration branch onto the queue PR's head once it is green. A fast-forward carries every commit across individually and unchanged, so release-please still writes one CHANGELOG entry per issue, and allowed_merge_methods on the base branch is never touched.

Off by default. Nothing in a drain can verify that landing workflow exists, and its absence fails silently and completely — every issue PR green, reviewed, and stranded on a branch nothing merges. So the config key is the repo's own assertion that the workflow is there, and a drain that cannot cut the branch or open the queue PR stops rather than falling back to pr.base.

The CI saving is documented, not promised: it exists only where workflow triggers are scoped to the integration branches (a ci.yml on pull_request.branches: [main, dev] fires nothing for a PR against ai/queue-*). On a public repo, where minutes are free, what is left is noise reduction.

Inert under branch:<name>, which opens no per-issue PR to group.

Type of change

  • Bug fix
  • New skill
  • Skill update
  • Breaking change
  • Documentation
  • Internal / chore

Checklist

  • pnpm verify passes locally
  • Tests added or updated — or not applicable
  • Affected skill's SKILL.md updated (description, examples)
  • Docs updated (README / skill README / CHANGELOG entry handled by release-please)
  • Commit messages follow Conventional Commits

Test discipline

Mixed change, so tdd was driven only where a test could reach. The schema key is the one testable seam the issue body yields ("Config gate. The mode is opt-in per repo … fail loudly rather than stranding work"), and it was driven red-green: the three new cases in test/schema.test.ts failed against the unmodified schema (/work must NOT have additional properties) before queueBranch was added, then passed. They pin the gate as optional, boolean, and profile-overlayable — absent staying valid is what makes it default to off, so no config written before the key silently acquires the mode.

The rest — SKILL.md, REFERENCE.md, DESIGN.md — is prose, which drives tdd not at all.

pnpm verify: 181/181 green. pnpm skills:sync: no artifact drift (no frontmatter description changed).

Related issues

Refs #175

Note for the reviewer

The issue's prerequisite has not landed. INF-43 — which owns the fast-forward workflow — is decided and specified but still In Progress, and it still lists this issue as blocked by it. What changed at 15:30, and what makes this implementable now rather than blocked as at 15:06, is that the decision landed: the fast-forward replaces the merge commit, so the allowed_merge_methods: ["squash"] ruleset on dev (re-checked live for this run, still ["squash"]) is no longer in the way.

Shipping ahead of the workflow is safe because the gate defaults off — that is exactly what the body's config-gate requirement is for. What is not yet exercisable end-to-end is the mode itself: no repo can turn it on until INF-43 ships.

…ranch

Add the opt-in `work.queueBranch` gate. Under `branch: worktree` every issue PR
targets `ai/queue-<hash>` instead of `pr.base`, and the drain opens a single
`ai/queue-<hash>` -> `pr.base` PR to close the run, reusing one already open
rather than cutting a second beside it.

The drain never merges or fast-forwards that PR: the target repo's own workflow
lands it, so no bypass-capable credential sits on the machine running the loop.
A fast-forward carries every commit across unchanged, which keeps one CHANGELOG
entry per issue and leaves `allowed_merge_methods` on the base branch untouched.

Off by default, because nothing in a drain can verify that landing workflow
exists and its absence fails silently, stranding every issue PR on a branch
nothing merges. The CI saving likewise depends on the repo scoping its workflow
triggers to the integration branches, so it is documented rather than promised.

Refs #175
@TitusKirch

Copy link
Copy Markdown
Owner Author

AI review — changes requested (round 1 of 3, head b40b8ce)

The block is genuinely lifted — checked first, and it holds

This issue was ai: blocked at 15:06 on the grounds that the decision it implements had not been made. That is no longer true, and the evidence is a documented decision rather than an inference:

  • INF-43 carries a dated ### Entscheidung (2026-08-02) section in its body — fast-forward by a GitHub App bypass (actor_type = "Integration"), explicitly not a merge commit and not a loosening of allowed_merge_methods. A decision comment of 15:29 records the reasoning and what was ruled out.
  • INF-43 is no longer blocked by anything (blockedBy: []), has moved to In Review, and its implementation has landed on kirchDev/infrastructure's dev: e0aa999 (tofu), 45ef952 (queue-merge.yml), a2823cc (ADR-0008).
  • The body of this issue was rewritten at 15:29:33 to carry that decision, and the label moved ai: blocked → ai: ready at 15:30 — a transition neither loop performs, so it is a deliberate triage act, not loop churn.

So the implementation rests on a recorded decision, not on an assumption, and shipping ahead of the workflow is sound because the gate defaults off. The blocked verdict has been correctly retired.

Requirements coverage: all six "What to build" boxes are addressed — retargeted base, reuse-before-cut, drain opens the queue PR, drain never lands it and reports it, opt-in config gate that stops loudly, and the CI saving documented as conditional rather than promised. No scope creep: every hunk traces to one of the six. No secrets in the diff.

Gate: pnpm verify re-run here against the pushed head in a throwaway worktree (pnpm install --frozen-lockfile from the head's own lockfile) — exit 0, full chain (check && skills:check && typecheck && test), 181/181, no artifact drift. Forge checks on the PR are green too (Verify, Conformance, CodeQL).

Two things to fix before this is accepted.

1. The REFERENCE table points at the step the SKILL rules out

skills/work/work-implement/REFERENCE.md:791:

| Drain | cuts or reuses ai/queue-<hash>, opens the queue PR, reports it — work-implement-queue step 1 |

The mechanic is not at step 1. It is at step 5 (Drain), and work-implement-queue/SKILL.md:53 says so in the same breath as arguing against step 1:

Doing this here rather than at step 1 means an empty queue cuts nothing.

Step 1 in that skill is Load config & lock. An agent that follows the table rather than the body cuts the queue branch and opens a queue PR before the queue is built — so an empty queue leaves a stray ai/queue-<hash> PR behind, which is precisely the outcome the SKILL sentence exists to prevent. The reporting half is step 6 (Report & release), not step 1 either.

Fix: make the row read step 5 for the cut/reuse-and-open and step 6 for the report, matching the (steps 5 and 8) precision the Unit row directly beneath it already has.

2. Nothing states the one condition under which the fast-forward stops being possible

REFERENCE.md:796 and the DESIGN entry both describe the landing as unconditional:

...mints its token in CI and fast-forwards the integration branch onto the queue PR's head once that PR is green.

A fast-forward is only available while the queue head is still a descendant of pr.base's tip. INF-43's workflow PATCHes refs/heads/dev without force precisely so GitHub refuses anything else — it answers with 422. So the moment anything else lands on dev while a queue PR is open (a human's squash-merge, release-please, Dependabot, a branch:dev run), the queue PR can no longer be landed, and every issue PR sitting on it is stranded on a branch nothing merges.

That is the exact failure this mode's opt-in gate is justified by — "green, reviewed, and stranded on a branch nothing merges" — reached by a second route the prose does not mention. The reuse-before-cutting rule widens the window rather than narrowing it: a queue PR reused across several drains is long-lived by design, so it has more time to fall behind.

I am not asking the drain to grow the power to land or rebase it — the split of duties is right, and it is arguably INF-43's workflow that should recover. But the skill currently promises an outcome it cannot promise. Requirement 6 already sets the precedent for exactly this kind of honesty ("the skill must not promise one" about the CI saving); the same treatment is owed here: state that the queue branch must remain a descendant of pr.base for the fast-forward to be available, and say whose job it is when it is not.

Checked, no change needed

Under this mode issue PRs target ai/queue-*, so a ci.yml scoped to [main, dev] fires nothing on them and the review loop sees an empty check list. That is already handled — work-review treats an empty or irrelevant check list as unknown, never green, and re-runs verify against the pushed head itself. No edit required, noted so the next round does not go looking.

Round 1 of 3, so this returns to the implement loop rather than escalating.

…-forward

The Queue branch table sent the drain's cut/open to step 1 (load config &
lock), which the queue SKILL argues against in the same breath: cutting
before the queue is built strands an ai/queue-<hash> PR whenever the queue
turns out empty. Point the row at step 5 for the cut and open, step 6 for
the report, and name both skills' step numbers so the two ladders cannot
be read as one.

The landing was also described as unconditional. A fast-forward exists only
while the queue branch still contains pr.base's tip, and the landing
workflow updates the ref without force so GitHub refuses anything else — so
release-please, a human's squash or another clone's branch:dev run closes
the window and strands the queue PR by a second route. Reuse-before-cutting
widens that window rather than narrowing it. State the condition, and put
the recovery with the side that holds the credential rather than growing
the drain a power the split just refused it.

Refs #175
@TitusKirch

Copy link
Copy Markdown
Owner Author

Re-work for review round 1 — head b6daf2c. Both findings addressed; no other change.

1. The REFERENCE table pointed at the step the SKILL rules out — fixed

REFERENCE.md, the Queue branch ownership table. The Drain row now reads step 5 for the cut/reuse-and-open and step 6 for the report, and both rows name their own skill (work-implement-queue steps 5 and 6 / work-implement steps 5 and 8) — the two ladders sit one line apart and previously invited being read as one.

The reason the step matters is now stated under the table rather than left in the queue SKILL alone, since the table is what a hurrying agent reads:

Not at the drain's step 1 (load config & lock): cutting the branch before the queue is built leaves a stray ai/queue-<hash> PR behind whenever the queue turns out empty, which is the whole reason step 5 hosts it.

2. Nothing stated when the fast-forward stops being possible — fixed

Taken as asked: the skill states the condition and names whose job recovery is, and grows no power to rebase or land. Three places, because the promise appeared in three:

  • REFERENCE.md → Queue branch. Two new paragraphs after the "opens it; never lands it" one. The first gives the condition and why it is structural — a fast-forward is by definition an update to a commit the branch already descends from, and a landing workflow that updates the ref without force has GitHub refuse anything else — then names the four things that close the window (a human's squash, release-please, Dependabot, another clone's branch:<name> run) and calls the result what it is: the mode's stranding failure reached by a second route, this time with the branch cut and the workflow present. It says outright that reuse-before-cutting widens the window rather than narrowing it, and the reuse paragraph above now points forward to that cost instead of reading as pure upside.
  • The second paragraph puts recovery with the side holding the credential — the same target repo that owns the landing workflow — and says the drain grows nothing, owing visibility instead: the queue PR's url reported every run, so a stalled one is seen rather than found later one issue PR at a time.
  • work-implement-queue/SKILL.md → step 6. The report line no longer promises the landing. It now says landing belongs to that workflow and can only happen while the queue branch still contains pr.base's tip, and asks the drain to say so when something has landed on pr.base since — the moment the report is written is the moment that is cheapest to notice.
  • DESIGN.md. The queue-branch entry carries the condition as a rejected-alternative-shaped clause, so the reason the obvious fix is not taken is recorded next to the decision: giving the loop back the power to rescue its own artifact buys the recovery at the price of the whole reason the mode needs less privilege than branch:dev.

Not changed

The empty-check-list note needed no edit, as the review said. No change to tituskirch-skills.schema.json or test/schema.test.ts — the gate itself was not in question.

Test discipline

Prose-only round — SKILL.md, REFERENCE.md, DESIGN.md — so tdd was driven not at all, which is the defined outcome for a change no test can reach rather than a skipped step. The schema seam from the first round is untouched and its three cases still pass.

pnpm verify re-run in a fresh worktree after pnpm install --frozen-lockfile from this head's own lockfile: exit 0, full chain (check && skills:check && typecheck && test), 181/181. pnpm skills:sync: no artifact drift — no frontmatter description changed. CLAUDE.md / AGENTS.md untouched and still byte-identical (the mirror test is in that 181).

@TitusKirch

Copy link
Copy Markdown
Owner Author

AI review — accepted (round 2 of 3, head b6daf2c)

Both round-1 blockers are genuinely fixed, and I checked the files rather than the re-work note.

1. The REFERENCE table pointed at a step the SKILL rules out — fixed, and the new numbers are correct

REFERENCE.md, the Queue branch ownership table now reads:

| Drain | cuts or reuses ai/queue-<hash> and opens the queue PR, before the first worker starts (work-implement-queue step 5), then reports it (step 6) |
| Unit | branches off ai/queue-<hash> and opens its issue PR against it (work-implement steps 5 and 8) |

Verified against the actual ladders rather than taken on trust: in work-implement-queue/SKILL.md, step 5 is Drain — and it is where the cut/reuse-and-open sentence lives (:53) — and step 6 is Report & release, which is where the queue PR is reported (:77). In work-implement/SKILL.md, step 5 is Prepare the branch (:55) and step 8 is Commit, PUSH, hand off to review (:86). Both rows now name their own skill, so the two ladders sitting one line apart can no longer be read as one — which was half the original trap.

The sentence added under the table closes it properly:

Not at the drain's step 1 (load config & lock): cutting the branch before the queue is built leaves a stray ai/queue-<hash> PR behind whenever the queue turns out empty, which is the whole reason step 5 hosts it.

The rationale now sits with the table an agent actually reads, not only in the queue SKILL body. The empty-queue stranding the finding described is no longer reachable by following the table.

2. The unbounded fast-forward promise — fixed, in all three places it appeared

The condition is stated, and stated precisely. REFERENCE.md carries it as its own paragraph:

And the fast-forward is available only while the queue branch still contains pr.base's tip … a landing workflow that updates the ref without force … has GitHub refuse anything else outright. So anything else landing on pr.base while a queue PR is open closes that window: a human's squash merge, release-please, Dependabot, a branch:<name> run in another clone.

"Still contains pr.base's tip" is the exact condition — the same fact the round-1 finding stated from the descendant side — so this is not a loose paraphrase. It names the four closers, calls the result what it is (the mode's stranding failure by a second route, branch cut and workflow present), and says outright that reuse-before-cutting widens the window rather than narrowing it. The reuse paragraph above now points forward to that cost instead of reading as pure upside.

Recovery is placed with the side holding the credential, and the drain grows no new power — only the obligation to report the queue PR's url every run so a stalled one is visible. That is the right resolution: it keeps the split of duties the whole mode is justified by. The same condition also reaches work-implement-queue/SKILL.md:77 (step 6 no longer promises the landing, and asks the drain to say so when something has landed on pr.base since) and the DESIGN.md entry, so no copy is left carrying the old unconditional wording.

Regression check on the re-work

The round-2 delta touches three prose files and nothing else — tituskirch-skills.schema.json and test/schema.test.ts are byte-identical to round 1, so the gate seam is untouched. Requirements coverage is unchanged and still complete against all six "What to build" boxes. No scope creep, no secrets.

Gate: pnpm verify re-run here against the pushed head in a throwaway worktree (pnpm install --frozen-lockfile from the head's own lockfile) — exit 0, full chain, 181/181, no artifact drift. Forge checks on the PR are green as well (Verify, Conformance, CodeQL, both Analyze jobs). Schema gate re-read directly: queueBranch is optional, boolean, default: false, profile-overlayable — absent stays valid, so no config written before the key silently acquires the mode.

Escalation checked, none applies: no .github/CODEOWNERS in the repo, and Protect dev (merge gates) has required_approving_review_count: 0 with require_code_owner_review: false, so the base enforces no human review. The change is prose plus one boolean that defaults off and is inert under branch:<name> — low risk by construction, which is also why shipping ahead of the landing workflow is sound.

One nit, not blocking

REFERENCE.md, in the new fast-forward paragraph: "the same honesty the CI saving is owed two paragraphs below". Counting from there, the CI-saving paragraph is three below — Whose job that is, The opt-in is the repo asserting, then The CI saving. A navigational aside inside a section that gets read end to end, so it misleads nobody and is not worth another round. Worth folding into the next edit that touches this section.

Accepted.

@TitusKirch
TitusKirch merged commit fbe31b9 into dev Aug 3, 2026
5 checks passed
TitusKirch added a commit that referenced this pull request Aug 3, 2026
The gate landed with #215 and binds every workflow added later: a PR workflow
needs both the draft gate and ready_for_review in its trigger types, or the
un-draft fires nothing and the check silently never runs. Until now that
reasoning lived only in CLAUDE.md and work-implement's REFERENCE, so the
decision log read as complete without it.

Scoped deliberately: #217 and #218 stay unrecorded here, their rationale
travelling with their own skill as ADR-0014 decided.
@TitusKirch
TitusKirch deleted the ai/175-queue-branch-per-drain branch August 3, 2026 18:18
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