Skip to content

docs(merge-gate): require gate query and merge as separate invocations#67

Merged
CybotTM merged 2 commits into
mainfrom
feat/merge-gate-separation
Jun 12, 2026
Merged

docs(merge-gate): require gate query and merge as separate invocations#67
CybotTM merged 2 commits into
mainfrom
feat/merge-gate-separation

Conversation

@CybotTM

@CybotTM CybotTM commented Jun 12, 2026

Copy link
Copy Markdown
Member

Why

Incident (2026-06-12): a PR was merged while 3 review threads were still unresolved. The merge-gate query and gh pr merge had been chained in one compound shell command (gate-query && gh pr merge style). The gate's output ("unresolved: 3") only became visible after the merge had already executed. The unresolved threads contained a high-severity security finding (worker running as root) that then required a follow-up PR.

The root cause is structural, not a one-off mistake: shell chaining decides on exit codes, not on the gate's content. gh pr view exits 0 whether it reports zero unresolved threads or three, so any query && merge (or query-then-merge inside one heredoc/compound command) merges unconditionally. A CLEAN mergeStateStatus does not close this gap either — GitHub only couples merge state to thread resolution when the "require conversation resolution" branch-protection rule is enabled, which most repos don't turn on.

What

  • skills/git-workflow/references/pull-request-workflow.md (Merge-Gate Command section): new rule — the gate and the merge are two separate invocations; run the gate query, read its output, only then issue gh pr merge as a new command. Includes a wrong/right example pair in the file's existing style.
  • skills/git-workflow/evals/evals.json: new eval merge_gate_separate_invocation — given a PR with green CI, CLEAN merge state, and possibly-open threads, the assistant must not merge in the same command as the gate query.

Validation

  • pre-commit run --all-files: all hooks pass on the changed files; the only failures are two pre-existing shellcheck findings in untouched files (.envrc SC2148, verify-git-workflow.sh SC2001), present on main before this change.
  • evals.json parsed and all assertion regexes compile.

A PR was merged while 3 review threads were still unresolved because the
merge-gate query and gh pr merge were chained in one compound shell
command (gate-query && gh pr merge). Shell chaining decides on exit
codes, not on the gate's content: gh pr view exits 0 regardless of how
many unresolved threads it reports, so the merge executed before the
gate output ("unresolved: 3") was ever read. The threads contained a
high-severity security finding that required a follow-up PR.

Add an explicit rule to the Merge Gate section: run the gate query, read
its output, and only then issue gh pr merge as a new command — never
chain them. Also note that mergeStateStatus CLEAN does not imply zero
unresolved threads (GitHub only couples those when the "require
conversation resolution" branch-protection rule is enabled), plus a
wrong/right example pair and an eval asserting the two-invocation
behavior.

Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Copilot AI review requested due to automatic review settings June 12, 2026 22:18
@github-actions

github-actions Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a new evaluation test case (merge_gate_separate_invocation) and updates the pull request workflow documentation to emphasize that running a merge gate check and executing the merge must be done in separate invocations rather than chained together (e.g., via &&). The review feedback suggests expanding the assertion regex patterns in the new evaluation test case to be more robust and prevent false negatives by matching more variations of refusal words and terms like "conversation" instead of just "thread".

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread skills/git-workflow/evals/evals.json
Match additional refusal verbs (cannot, should not, must not) and
common phrasing variants (exits with 0, unresolved conversations) to
reduce false negatives in the merge_gate_separate_invocation eval.

Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
@sonarqubecloud

Copy link
Copy Markdown

@CybotTM CybotTM merged commit 30d50c0 into main Jun 12, 2026
17 checks passed
@CybotTM CybotTM deleted the feat/merge-gate-separation branch June 12, 2026 22:23
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.

2 participants