Skip to content

ci: queue PRs in Mergify once verify and Unfret pass - #101

Open
ericlitman wants to merge 2 commits into
mainfrom
ci/100-mergify-auto-queue
Open

ericlitman wants to merge 2 commits into
mainfrom
ci/100-mergify-auto-queue

Conversation

@ericlitman

@ericlitman ericlitman commented Oct 1, 2026 •

Copy link
Copy Markdown
Owner

Closes #100

What changed

Adds .mergify.yml. Mergify now queues a non-draft PR against main once verify and Unfret pass. It merges with one PR per batch and no checks timeout, and only after verify and Unfret pass on the queued merge commit (merge_conditions). open-pstack has no branch protection for Mergify to inject required checks from. This is the same shape as mastra-pilot's config, written standalone because mastra-pilot extends an org-only shared config. @Mergifyio requeue is restricted to writers. Until now there was no config, and #98 merged only after a manual @Mergifyio queue.

Unfret rebuttal for head 635d077, finding v1:b39df1497abe8077412bd4881a908445c16acc0aab28368bcfda0d77401dae11
Claim: an outside contributor or bot can open a PR that auto-merges once checks named verify and Unfret pass, with no writer acting.
Disproof: no one outside the collaborators can open a PR here. GET repos/ericlitman/open-pstack returns pull_request_creation_policy: collaborators_only. The only collaborator is ericlitman (GET repos/ericlitman/open-pstack/collaborators). The repository has no Dependabot config (.github/ holds only pull_request_template.md and workflows). Vulnerability alerts are disabled (GET …/vulnerability-alerts returns 404), and automated security fixes are enabled: false. Every PR author is therefore a writer, the same trust #98 relied on when its writer posted @Mergifyio queue.

Verification

  • Bun tests, strict typecheck, static invariants, and plugin validation pass. This PR touches no plugin file; verify CI runs on it.
  • The exact candidate is installed in every affected harness. Not applicable: no plugin content changes.
  • The changed behavior passes from each real user surface.
  • The installed version, action, and observed result appear below.

Live evidence:

Mergify reads its queue rules from main, so this PR cannot exercise them. The evidence is Mergify's configuration check on this PR, then the next open-pstack PR merging through the queue with no manual queue comment. I'll record that result on #100.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mergify

mergify Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🟢 Merge protection satisfied — ready to merge.

Show 1 satisfied protection

🟢 📃 Configuration Change Requirements

Mergify configuration change

  • any of:
    • check-success = @mergify/Configuration changed
    • check-success = @mergify/Configuration has been deleted

@greptile-apps

greptile-apps Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 3/5

[High risk] Adds automated merge queue configuration for pull requests.

This PR should not merge until the queue requires a current checked result and the independent live verdict.

Findings

  1. P1 Queued code skips final checks ▶
  2. P1 PR queues before live verdict ▶

Summary

Adds Mergify rules that queue non-draft pull requests for main after verify and Unfret pass.

  • The queue handles one PR per batch with no checks timeout.
  • Requeue commands require write permission.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[Non-draft PR to main] --> B[verify and Unfret pass]
  B --> C[Mergify queues PR]
  C --> D[Queue merges PR]
  V[Independent live verdict] -. not a queue condition .-> C
  F[Checks on queued result] -. not a merge condition .-> D
Loading

Reviews (1) · Last reviewed commit: "ci: queue PRs in Mergify once verify and..."

Comment thread .mergify.yml Outdated
batch_size: 1
checks_timeout: null
queue_conditions: []
merge_conditions: []

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 Queued code skips final checks

If main changes after a PR passes verify and Unfret, the empty merge_conditions do not require either check to pass on the queued result. Code that the checks did not cover can then merge. Require both checks at merge time.

Comment thread .mergify.yml
Comment on lines +17 to +20
success_conditions:
- -draft
- check-success=verify
- check-success=Unfret

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 PR queues before live verdict

If Unfret does not require the independent live verdict, this rule queues any non-draft PR to main once the two checks pass. The PR can then merge while the live test required by the shipping flow is still in progress. Require that verdict before automatic queuing, or leave queuing to the guarded shipping flow.

Knowledge Base Used: Quality assurance and review

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Drafts can't queue: the merge protection requires -draft. AGENTS.md keeps a PR in draft until the installed-candidate evidence is recorded, so marking it ready is the live-verdict gate. That holds only if PRs open as drafts. The live-gate skill in #90 will enforce it by being the step that marks a PR ready. The empty merge_conditions finding is fixed in 635d077.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That resolves the concern. With PRs opened as drafts and the live-gate skill being the step that marks them ready after installed-candidate evidence, -draft makes the merge protection a live-verdict gate. The added merge_conditions checks also ensure the queued merge commit reruns both verify and Unfret before merging.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Correction to my reply above: the draft convention doesn't close this gap. Unfret doesn't review drafts (drafting cancels an in-flight review), so a PR has to be non-draft to get its Unfret check, and once it is, this rule queues it as soon as verify and Unfret pass, possibly before the live test. The live verdict needs to be its own required check, for example a commit status on the exact head that the #90 skill posts after the installed-harness test, listed in both merge_conditions and the protection's success_conditions.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You're right—the draft convention does not close this gap. Unfret cancels its review for drafts, so the PR must become non-draft before check-success=Unfret can pass; at that point, this protection can queue it without any installed-harness/live verdict.

The live-gate result therefore needs to be an independent status/check on the exact tested head, included in both success_conditions and the queue rule's merge_conditions so it is also rerun and required for the queued merge commit. My previous resolution was incorrect; this finding remains valid.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Refinement, per the review on #104: list live-gate in queue_conditions and the protection's success_conditions only, not in merge_conditions. Mergify validates a temporary queue head that the installed-harness test never sees, so requiring live-gate there would block forever under checks_timeout: null.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That refinement is correct. live-gate should be added to the queue rule’s queue_conditions and to the protection’s success_conditions, but not to queue_rules[].merge_conditions. Mergify evaluates merge_conditions against the temporary queued merge commit; because the installed-harness test posts its result only for the original PR head, requiring check-success=live-gate there would leave the queued commit unsatisfied and prevent merging indefinitely when checks_timeout: null. The live-gate therefore controls entry into the queue and protection success, while the queued commit itself only reruns the checks that can report on that commit (verify and Unfret).

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Second refinement, from #104's review: putting live-gate only at queue entry still lets a rebuilt queue head merge untested, and non-empty merge_conditions make Mergify create draft batch PRs, which Unfret never reviews. The likely shape: in-place checks (empty merge_conditions, batch_size: 1) plus queue_conditions requiring -draft, verify, Unfret, live-gate, and an up-to-date branch, so the merged tree is the tested tree. Please verify against Mergify's docs before adopting it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The docs support the in-place direction, with two qualifications:

  • merge_conditions is the merge-time/speculative-check path. A distinct non-empty set can make Mergify validate a temporary draft/batch PR, and batch_size: 1 alone does not guarantee in-place checks. An empty merge_conditions is appropriate for the single-step path, but in-place behavior also requires serial operation (max_parallel_checks: 1) and batch_size: 1 (along with the other documented eligibility constraints).
  • queue_conditions control admission and continued queue membership. Requiring check-success=live-gate there means that if the tested SHA changes, the PR no longer satisfies the condition and should leave the queue until a fresh verdict is posted. The status must be attached to the exact commit SHA tested.

Thus, the proposed shape is reasonable:

queue_rules:
  - name: default
    batch_size: 1
    checks_timeout: null
    queue_conditions:
      - -draft
      - check-success=verify
      - check-success=Unfret
      - check-success=live-gate
    merge_conditions: []

The queue must also be configured for serial checks (max_parallel_checks: 1), and live-gate should be included in merge_protections[].success_conditions alongside verify and Unfret.

I would not add an invented updated or up-to-date queue condition. Mergify documents GitHub branch protection/rulesets’ strict required-status-checks policy (“Require branches to be up to date before merging”) as the mechanism for that. Since this repository currently has no such protection, enabling it is a separate repository configuration change, not a .mergify.yml condition. Strict protection is also problematic with temporary batch PRs, reinforcing the choice of in-place checks here.

References: queue rules, queue lifecycle, batches and in-place eligibility, and GitHub rulesets.

@unfret-eal

unfret-eal Bot commented Oct 1, 2026 •

Copy link
Copy Markdown

✅ No blocking findings

Walkthrough

Adds a `.mergify.yml` so Mergify automatically queues non-draft PRs targeting `main` once the `verify` and `Unfret` checks pass, rather than needing a manual `@Mergifyio queue` comment as #98 did. The queue merges one PR at a time with no checks timeout, and it re-requires `verify` and `Unfret` to succeed before merging. `@Mergifyio requeue` is limited to users with write permission. Mergify reads its config from `main`, so this PR can't exercise the rules itself. The author plans to confirm the behavior on the next PR and record the result on #100.

Since the last review
  • Withdrawn: Queued merge commits can bypass the required checks. Why: The current head repairs the carried defect: .mergify.yml lines 6–8 explicitly require both check-success=verify and check-success=Unfret in the queue's merge_conditions. These checks now gate the queued merge candidate rather than appearing only in admission protections. The finding's premise that merge_conditions is empty no longer holds, and the current configuration satisfies the prior repair demand.
Details
Lane / stage Model Effort Outcome Duration
broad-gpt-5.6-sol-xhigh / review gpt-5.6-sol (reported model unavailable) xhigh succeeded 22 s
broad-opus-5.5-max / review anthropic/claude-opus-5-5 (reported model unavailable) max succeeded 4 min 3 s
focal-gpt-6.1-sol-xhigh / review openai/gpt-6.1-sol (reported model unavailable) xhigh succeeded 4 min 54 s
broad-gpt-5.6-sol-xhigh / reread gpt-5.6-sol (reported model unavailable) xhigh succeeded 37 s
broad-opus-5.5-max / reread anthropic/claude-opus-5-5 (reported model unavailable) max succeeded 4 min 37 s
focal-gpt-6.1-sol-xhigh / reread openai/gpt-6.1-sol (reported model unavailable) xhigh succeeded 6 min 8 s
judge / judge openai/gpt-6-astra (reported model unavailable) high succeeded 19 s

Reviewed the whole pull request.
3 reviewers finished.
Commit 635d077.
Check run.
Run: GET /unfret/run/panel:635d077bbaabc455e3954557504d7543cd4cee39:ogzPXD0gyFs:mVYQkY7dAUw.

@unfret review · @unfret status · @unfret help

@unfret-eal unfret-eal 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.

🛑 1 blocking finding
🟡 Medium · Queued merge commits can bypass the required checks
Check

Comment thread .mergify.yml Outdated
batch_size: 1
checks_timeout: null
queue_conditions: []
merge_conditions: []

@unfret-eal unfret-eal Bot Oct 1, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Medium · Queued merge commits can bypass the required checks

merge_conditions is empty, so the queue does not require verify or Unfret on its temporary merge commit. If a predecessor lands or main changes after those checks pass on the original PR, the combined commit can fail the newly triggered checks while Mergify still considers queue validation successful and merges it.

Withdrawn in the latest review.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@unfret-eal unfret-eal 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.

✅ No blocking findings
Check

@mergify

mergify Bot commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

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.

Configure Mergify to auto-queue PRs once verify and Unfret pass

1 participant