-
Notifications
You must be signed in to change notification settings - Fork 0
ci(mergify): remove human completion gate #544
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,100 @@ | ||||||||||||||||||||||||||
| # Mergify No-Human-Review Policy Implementation Plan | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Goal:** Allow SessionLedger pull requests to auto-merge after configured CI succeeds without a human approval or reviewer-completion predicate. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Architecture:** Modify only `.mergify.yml` for behavior, plus these execution docs for the corrected invariant. The Mergify merge and ready-label rules stop testing approval or reviewer-completion counts while retaining CI, conflict, open-PR, squash, labeling, reviewer notification, and large-PR behavior. Shell validation parses the YAML and asserts the policy invariants without requiring a paid service. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Tech Stack:** Mergify YAML, Ruby standard-library YAML parser, GitHub pull-request automation. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| --- | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ### Task 1: Remove Mergify human-completion predicates while retaining CI gates | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| **Files:** | ||||||||||||||||||||||||||
| - Modify: `.mergify.yml:4-26,67-79` | ||||||||||||||||||||||||||
| - Test: inline Ruby policy assertions against `.mergify.yml` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - [ ] **Step 1: Write the failing policy assertion** | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Run: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||
| ruby -e ' | ||||||||||||||||||||||||||
| require "yaml" | ||||||||||||||||||||||||||
| rules = YAML.load_file(".mergify.yml").fetch("pull_request_rules") | ||||||||||||||||||||||||||
| merge = rules.find { |rule| rule.fetch("name") == "Auto-merge when CI green" } | ||||||||||||||||||||||||||
| ready = rules.find { |rule| rule.fetch("name") == "Add ready-to-merge label" } | ||||||||||||||||||||||||||
| raise "merge rule missing" unless merge | ||||||||||||||||||||||||||
| raise "ready rule missing" unless ready | ||||||||||||||||||||||||||
| raise "human review completion predicate remains" if [merge, ready].flat_map { |rule| rule.fetch("conditions") }.any? { |condition| condition.include?("approved-reviews") || condition.include?("review-requested") } | ||||||||||||||||||||||||||
|
Comment on lines
+27
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Make the red assertion fail for the intended reason. Step 1 searches for Proposed lookup- merge = rules.find { |rule| rule.fetch("name") == "Auto-merge when CI green" }
+ merge = rules.find do |rule|
+ ["Auto-merge when approved + CI green", "Auto-merge when CI green"].include?(rule.fetch("name"))
+ end📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| raise "missing ci gate" unless merge.fetch("conditions").include?("check-success=ci") | ||||||||||||||||||||||||||
| raise "missing lint gate" unless merge.fetch("conditions").include?("check-success=lint") | ||||||||||||||||||||||||||
| raise "missing test gate" unless merge.fetch("conditions").include?("check-success=test") | ||||||||||||||||||||||||||
| raise "missing conflict gate" unless merge.fetch("conditions").include?("-conflict") | ||||||||||||||||||||||||||
|
Comment on lines
+32
to
+35
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert every retained merge safeguard. The validation omits Proposed assertions+raise "missing typecheck gate" unless merge.fetch("conditions").include?("check-success=typecheck")
+raise "missing open-PR gate" unless merge.fetch("conditions").include?("-closed")
+raise "merge method changed" unless merge.fetch("actions").fetch("merge").fetch("method") == "squash"📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||
| puts "Mergify no-human-review policy: valid" | ||||||||||||||||||||||||||
| ' | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Expected: fail because the current merge rule still contains | ||||||||||||||||||||||||||
| `#review-requested=0`. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - [ ] **Step 2: Edit only the two policy rules** | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Replace the merge rule heading and name with: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```yaml | ||||||||||||||||||||||||||
| # Auto-merge when all configured CI checks pass. | ||||||||||||||||||||||||||
| - name: Auto-merge when CI green | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Delete these human-completion condition entries, and no other safeguards: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```yaml | ||||||||||||||||||||||||||
| - "#approved-reviews-by>=1" | ||||||||||||||||||||||||||
| - "#review-requested=0" | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Keep these merge conditions exactly: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```yaml | ||||||||||||||||||||||||||
| - check-success=ci | ||||||||||||||||||||||||||
| - check-success=lint | ||||||||||||||||||||||||||
| - check-success=typecheck | ||||||||||||||||||||||||||
| - check-success=test | ||||||||||||||||||||||||||
| - -conflict | ||||||||||||||||||||||||||
| - -closed | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - [ ] **Step 3: Run policy validation** | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Run the Step 1 Ruby command again. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Expected: `Mergify no-human-review policy: valid`. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - [ ] **Step 4: Run syntax and scope checks** | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Run: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||
| ruby -e 'require "yaml"; YAML.load_file(".mergify.yml"); puts "mergify YAML: valid"' | ||||||||||||||||||||||||||
| git diff --check | ||||||||||||||||||||||||||
| git diff -- .mergify.yml | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Expected: valid YAML, no whitespace errors, and only the human-completion | ||||||||||||||||||||||||||
| predicates plus the merge rule wording changed. | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| - [ ] **Step 5: Commit, push, and open the policy PR** | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Run: | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| ```bash | ||||||||||||||||||||||||||
| git add .mergify.yml docs/superpowers/specs/2026-08-29-mergify-no-human-review-design.md docs/superpowers/plans/2026-08-29-mergify-no-human-review.md | ||||||||||||||||||||||||||
| git commit -m "ci(mergify): remove human completion gate" | ||||||||||||||||||||||||||
| git push -u origin ci/mergify-no-human-review | ||||||||||||||||||||||||||
| gh pr create --base main --head ci/mergify-no-human-review --title "ci(mergify): remove human completion gate" --body "Removes only Mergify human-completion predicates while retaining CI and conflict gates. Validated YAML and policy invariants locally." | ||||||||||||||||||||||||||
| ``` | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Expected: one policy-only PR with normal hosted CI and auto-merge enabled. | ||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| # Mergify no-human-review policy | ||
|
|
||
| ## Goal | ||
|
|
||
| Allow SessionLedger pull requests to merge automatically after the configured | ||
| machine checks pass, without requiring a human approval. Pull requests remain | ||
| the only integration path; direct pushes to `main` remain protected. | ||
|
|
||
| ## Scope | ||
|
|
||
| Change only `.mergify.yml`: | ||
|
|
||
| 1. Rename the merge rule to describe CI-gated auto-merge rather than approval. | ||
| 2. Remove `#approved-reviews-by>=1` and `#review-requested=0` from that | ||
| merge rule, so an automated reviewer notification cannot become a merge | ||
| completion gate. | ||
| 3. Remove the approval predicate from the `ready-to-merge` label rule. | ||
|
|
||
| ## Retained safeguards | ||
|
|
||
| The policy continues to require the configured CI successes, no merge | ||
| conflicts, an open PR, squash merging, current commit-message formatting, and | ||
| the existing reviewer notification / labeling / large-PR rules. Reviewer | ||
| notifications are informational only and do not gate merging. GitHub `main` | ||
| protection continues to require PRs and its required `ci / lint` and `ci / | ||
| test` checks; its approval count is intentionally zero. | ||
|
|
||
| ## Validation | ||
|
|
||
| 1. Parse `.mergify.yml` as YAML. | ||
| 2. Assert neither merge-related rule contains an approval or reviewer- | ||
| completion predicate. | ||
| 3. Confirm the retained CI and conflict predicates remain present. | ||
| 4. Open a PR and let normal hosted checks and Mergify evaluate it. No paid | ||
| review or runner is used. | ||
|
|
||
| ## Rollback | ||
|
|
||
| Reintroduce the approval and reviewer-completion predicates in a follow-up PR | ||
| and set GitHub's required approval count back to one if human review becomes | ||
| required again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use a sequential heading level.
Change
### Task 1to## Task 1. This removes the MD001 heading-increment warning.🧰 Tools
🪛 markdownlint-cli2 (0.23.2)
[warning] 13-13: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🤖 Prompt for AI Agents
Source: Linters/SAST tools