fix(github-sdlc-planning): make query-pipeline's --admin retry reachable when only approval is missing - #327
Merged
Conversation
…ble when only approval is missing (#326) mergeStage's JS guard short-circuited on `dev.settle.settled !== true` before the merge agent (and #306's --admin-retry authorization) was ever invoked. Per #305, check_pr_readiness correctly reports settled:false when the ONLY blocker is a missing approval (checks green, threads resolved, reviewDecision REVIEW_REQUIRED — Copilot can only COMMENT, never APPROVE). That is exactly the condition #306's retry exists to handle, but the guard treated it the same as any other unsettled reason and returned a generic "not settled — automerge skipped" without ever reaching the agent. settleStage now derives a structured `blockedOnApprovalOnly` field deterministically from its own check_pr_readiness read, distinct from string-matching notes after the fact. mergeStage's guard now invokes the merge agent when either settled is true or blockedOnApprovalOnly is true, and the merge agent's own re-verification step mirrors the same distinction so it doesn't stop on stale state. All other unsettled reasons (failing or pending checks, unresolved threads, CHANGES_REQUESTED, open code-scanning alerts) still skip the merge agent entirely, unchanged. Adds regression coverage executing the real workflow file body (same technique as the existing gdlc#307 guard test) proving the merge agent is reached for the approval-only case and still skipped for a genuine blocker.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a reachability bug in the github-sdlc-planning query-pipeline workflow where the merge-stage guard prevented the merge agent (and its --admin retry authorization) from running when the only remaining blocker was a missing approving review.
Changes:
- Adds a structured
blockedOnApprovalOnlyboolean to the settle-stage output schema so the workflow can distinguish “approval-only blocked” from other unsettled states without string-matching notes. - Updates
mergeStageto invoke the merge agent when eithersettled === trueorblockedOnApprovalOnly === true, while preserving the existing skip behavior for all other unsettled reasons. - Adds a new unit test that executes the real shipped workflow body to prevent regressions in merge-stage reachability logic; updates
SKILL.mddocumentation accordingly.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugins/github-sdlc-planning/skills/query-pipeline/SKILL.md | Documents the new blockedOnApprovalOnly settle signal and the restored merge-stage behavior/authorization semantics. |
| plugins/github-sdlc-planning/skills/query-pipeline/scripts/query-pipeline.workflow.js | Extends SETTLE_SCHEMA, updates settle-stage instructions, and fixes mergeStage guard + merge-agent re-verification semantics for approval-only blocks. |
| plugins/github-sdlc-planning/mcp-server/test/unit/query-pipeline-workflow-merge.test.ts | Adds regression coverage proving the merge agent is reachable for the approval-only unsettled shape and still skipped for other unsettled cases. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #326
Bug
mergeStage's JS guard short-circuited ondev.settle.settled !== truebefore the merge agent (and #306's
--admin-retry authorization) was everinvoked. Per #305,
check_pr_readinesscorrectly reportssettled: falsewhen the ONLY blocker is a missing approval (checks green, threads
resolved,
reviewDecision: REVIEW_REQUIRED— Copilot can only leaveCOMMENTED, never APPROVED). That is exactly the condition #306's retry
exists to handle, but the guard treated it identically to any other
unsettled reason and returned the generic
'not settled — automerge skipped'string without ever reaching the agent — leaving--automergesilently non-functional for the org's most common branch-protection shape.
Fix
SETTLE_SCHEMAgains a structuredblockedOnApprovalOnly: boolean,derived deterministically by the settle-stage agent from its own
check_pr_readinessread — never inferred from notes text after thefact.
mergeStage's JS guard now invokes the merge agent when eithersettledistrueorblockedOnApprovalOnlyistrue; every otherunsettled reason (failing/pending checks, unresolved threads,
CHANGES_REQUESTED, open code-scanning alerts) still skips the agententirely, unchanged.
distinction (state can change between stages), so it doesn't stop on a
stale
settled: truerequirement that would otherwise reintroduce thesame bug one level down.
SKILL.mdupdated to describe the new field and the merge-stagereachability it restores.
Tests
New
query-pipeline-workflow-merge.test.tsexecutes the real,currently-shipped workflow file body (same technique as the existing
gdlc#307 guard test) and proves:
settled: false+blockedOnApprovalOnly: truesettled: false+blockedOnApprovalOnly: falsesettled: trueautomerge: falsestill skips the merge agent regardless ofblockedOnApprovalOnlyVerified the new test fails against the pre-fix guard logic (reverted
locally, re-ran, restored) before finalizing, per the "bug fix isn't done
without a test that fails before and passes after" convention.
Gates run locally
npm run lint— cleannpm run typecheck— cleannpx vitest run— 603/603 passed (mcp-server package)npm run build— cleanmarkdownlint-cli2on the touchedSKILL.md— clean