Skip to content

feat: add do-not-merge-gate reusable workflow - #108

Merged
kyle-sexton merged 10 commits into
mainfrom
feat/do-not-merge-gate
Jul 16, 2026
Merged

feat: add do-not-merge-gate reusable workflow#108
kyle-sexton merged 10 commits into
mainfrom
feat/do-not-merge-gate

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

Step 1 of 3 for decision #13 (metadata-do-not-merge-wire-or-walk-back, Decisions Log: https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63).

  • do-not-merge-gate.yml: a new reusable workflow mirroring semantic-pr.yml's shape exactly (same prerequisite-result fail-closed pattern, same runner-selection input). Fails the job while the calling PR carries a configured label (default do-not-merge).
  • README: documents the canonical caller block, matching the existing semantic-pr doc entry's format.

Why a new reusable workflow, not a ruleset rule

github-iac's Labels.cs has declared do-not-merge as a "Hard merge gate" since it was introduced, but nothing has ever enforced it. GitHub Rulesets have no native label-condition rule type — confirmed empirically by enumerating every OrganizationRulesetRules* type in the pulumi-github v6.14.0 provider assembly. A required-status-check is the only real mechanism, so this follows the exact precedent pr-title/semantic-pr already established in this repo.

Re-evaluation on label change

A status check binds to a SHA. If it runs once (no label) and passes, then someone adds do-not-merge, nothing re-runs and the merge isn't actually blocked. The canonical caller block triggers on labeled/unlabeled (in addition to opened/reopened/synchronize/merge_group) specifically so the gate re-fires when the label is toggled.

Rollout sequencing (this PR is step 1 only)

  1. This PR — land the reusable workflow.
  2. Once merged, add the canonical caller to every requires-ci repo (9 repos: .github, ci-runner, ci-workflows, claude-code-plugins, github-iac, medley, standards, dotfiles, provisioning), pinned to this PR's merge commit SHA.
  3. Only after all 9 callers are merged and emitting the check: add do-not-merge / do-not-merge to github-iac's ci-gate RequiredStatusChecks (OrgRulesets.cs) — never before, per this repo's own documented rule ("open PRs block on a check that never runs").

Scope confirmed with the repo owner: requires-ci repos only (not the 6 RequiresCi: false archived/content repos), caller-workflow mechanism (not required_workflows, since that ruleset primitive's re-evaluation-on-label-toggle behavior is unverified).

Verification

  • actionlint .github/workflows/do-not-merge-gate.yml — clean.
  • zizmor .github/workflows/do-not-merge-gate.yml — no findings.
  • Repo's own lefthook pre-commit hooks passed at commit time.

New standalone required check mirroring semantic-pr's shape: fails while the
calling PR carries a configured blocking label (default do-not-merge).
Re-evaluates on labeled/unlabeled (not just push), so the gate can't go stale
after the label is added post-check. pull_request_target-based per the
existing semantic-pr doctrine — reads label metadata only, no head-code
checkout, so a head-branch edit can't bypass it.

Part of melodic-software decision #13 (do-not-merge-wire-or-walk-back):
github-iac's Labels.cs has declared do-not-merge as a "Hard merge gate" since
its introduction, but nothing ever enforced it — GitHub rulesets have no
native label-condition rule (confirmed against the pulumi-github provider
schema), so a required-status-check is the only real mechanism. This is step
1 of 3: reusable workflow (here) -> per-repo callers on requires-ci repos,
pinned to this commit once merged -> github-iac ruleset RequiredCheck, added
last so open PRs never block on a check that isn't emitted yet.

Decisions Log: https://claude.ai/code/artifact/232ecdce-8316-4880-8c0a-dc3c7dcf3a63

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8d30d6d7ec

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml Outdated
The called job was `do-not-merge-gate`, so a caller using the README's
canonical block (caller job `do-not-merge`) emitted the check context
`do-not-merge / do-not-merge-gate` — not `do-not-merge / do-not-merge`,
which the README and PR rollout plan require github-iac's ruleset to
enforce. Rename the called job to `do-not-merge` so the emitted context
matches; otherwise the required context would never report once enabled
and callers would block instead of enforcing the label gate.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6be070ede5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml Outdated
The reusable workflow previously skipped its label check entirely on
merge_group, always reporting success — a PR could enter the merge
queue clean and then have the blocking label added while queued,
without the required check ever catching it. merge_group carries no
pull_request context, so recover a PR number from the merge queue's
temporary ref (gh-readonly-queue/<base>/pr-<number>-<sha>) and
re-fetch that PR's current labels via the API.

Known residual gap, documented inline and in the README: GitHub
batches multiple PRs into one merge group by default (max group size
5), and the temp ref names only the last PR in the batch, so a
non-last PR's labels are not re-evaluated by this check. Closing that
fully needs either a verified way to enumerate every PR in a batch
from a merge_group run, or capping merge queue max group size at 1 in
the ruleset layer.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cfcafeeca1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml Outdated
pull_request/pull_request_target runs trusted the triggering event's
label payload, which GitHub can leave stale if a same-repo automation
labels the PR with the default token right before this gate runs (that
suppresses the follow-up labeled run). Re-fetch current labels via the
API for those events too, matching the merge_group path.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2f75e7f120

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml
…ondition

Batched merge groups are not detectable or enumerable from inside the
workflow today, so merge_group coverage is only exhaustive when the
adopting repo's merge queue produces single-PR groups. State that as an
explicit adoption precondition (README) with a pointer from the
workflow's KNOWN GAP comment, rather than silently shipping incomplete
coverage on repos that batch.
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d2fb61e20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50a9e326ae

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread README.md Outdated
GitHub never starts a workflow run for labeled/unlabeled events produced
by the default GITHUB_TOKEN (recursive-run guard), so a same-repo
automation job that applies this gate's blocking label with that token
after the check already passed leaves the earlier green check-run
un-invalidated -- no run, live-refetch or otherwise, is ever triggered.
No trigger-list change can close this since the gap is that GitHub never
starts a run to begin with; document it as a known limitation and require
label-setting automation to use a GitHub App/PAT token instead, matching
the existing batching-gap documentation pattern. Also corrects the
README's earlier overclaim that labeled/unlabeled triggers alone
guarantee re-evaluation, and its incorrect edited-event example (edited
is not one of the caller's configured trigger types).
@claude

claude Bot commented Jul 15, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e5d76f12a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread .github/workflows/do-not-merge-gate.yml Outdated
The prerequisite-result input's contract told callers to use
if: always(), but GitHub documents always() as true during
cancellation, so a manually cancelled or cancel-in-progress-superseded
run could still dispatch this gate on ubuntu-slim and publish a
fail-closed required check instead of yielding to the superseding run.

Match the semantic-pr precedent and require if: !cancelled() instead.
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 0s —— View job


I'll analyze this and get back to you.

@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 4m 21s —— View job


Review: feat/do-not-merge-gate

  • Gather context and understand request
  • Read changed files (do-not-merge-gate.yml, README.md)
  • Review workflow for correctness, security, and CLAUDE.md alignment
  • Review README documentation
  • Post findings

Summary

Solid implementation. The architecture mirrors semantic-pr.yml precisely, all prior Codex P1/P2 findings are resolved or honestly documented, and the comments + README are unusually thorough about known gaps. One medium correctness issue found; one minor hardening suggestion.


Findings

[Medium — Correctness] API failure silently passes the gate

do-not-merge-gate.yml lines 138–153

The run: block has no set -euo pipefail. If gh api exits non-zero (network error, rate-limit 429, transient 500, deleted PR returning 404), pr_labels_json is empty. jq receives empty input, exits non-zero itself, the if branch is not taken, the script falls through to echo "No blocking label present." and exits 0 — the gate passes vacuously despite the API call never completing.

This inverts the intended fail-closed semantics for a transient error scenario.

# Current — API failure is silent
pr_labels_json="$(gh api "repos/${REPOSITORY}/pulls/${pr_number}" --jq '[.labels[].name]')"
if jq -e --arg label "$BLOCKING_LABEL" 'any(. == $label)' <<<"$pr_labels_json" >/dev/null; then

Fix: add set -euo pipefail as the first line of the run: block. With pipefail, a non-zero exit from gh api propagates and fails the step immediately, keeping the gate fail-closed on API errors.

Fix this →


[Low — Defensive hardening] Empty PR_NUMBER not validated in the non-merge_group path

do-not-merge-gate.yml line 146

else
  pr_number="$PR_NUMBER"
fi

$PR_NUMBER derives from github.event.pull_request.number, which GitHub guarantees is populated on every pull_request / pull_request_target event — so this is safe in practice. But if pr_number were empty, gh api "repos/.../pulls/" would produce a non-404 API response (it lists all PRs, not 404), jq would receive an array rather than a single PR object, and the gate would pass without checking any label. With set -euo pipefail in place (finding above) the malformed URL case would still fail-close, but an explicit guard mirrors the merge_group path's own check and makes the failure message actionable:

else
  pr_number="$PR_NUMBER"
  if [[ -z "$pr_number" ]]; then
    echo "::error::PR number is empty for a pull_request/pull_request_target event."
    exit 1
  fi
fi

Prior Codex findings — status

Finding Status
P1 — Job ID emits do-not-merge-gate not do-not-merge ✅ Resolved — job is do-not-merge: (line 37); caller block matches
P1 — merge_group skipped label check entirely ✅ Resolved — PR number parsed from queue ref; live API re-fetch
P2 — Stale event payload on concurrent GITHUB_TOKEN labeling ✅ Resolved — all paths re-fetch live labels via gh api
P2 — Batched merge groups: only last PR's labels checked ✅ Documented honestly as KNOWN GAP (lines 82–95 + README); adoption precondition documented
P2 — GITHUB_TOKEN-authored labeling after gate already passed ✅ Documented as KNOWN LIMITATION (lines 97–113 + README) with adoption requirement
P2 — always() vs !cancelled() for selector-dependent caller ✅ Resolved (commit 6ef1910) — prerequisite-result docs now specify if: !cancelled()

What's well-done

  • No injection surface. Every GitHub context value (label, event_name, PR_NUMBER, MERGE_GROUP_HEAD_REF, REPOSITORY) flows through env: before the shell script; the run: block never interpolates ${{ }} directly. jq --arg is used for the label comparison. Clean.
  • No external uses: pins needed. The workflow is pure shell (gh + jq, both pre-installed on hosted runners); no supply-chain pin surface.
  • permissions: pull-requests: read declared at workflow level, narrowing the token for all jobs.
  • Live label re-fetch on every code path is the right call — the stale-payload problem for pull_request/pull_request_target is subtle and correctly addressed here.
  • Known gaps are explicitly named, not papered over. The inline comments (lines 82–125) and the README sections for batched merge queues and GITHUB_TOKEN labeling are honest about what this mechanism cannot cover.
  • Rollout sequencing in the PR body is correct — requiring the check before callers are merged would deadlock open PRs.

CLAUDE.md alignment

  • SHA-pin rule: no uses: in this workflow, nothing to pin. ✅
  • pull_request_target in the README canonical caller: the CLAUDE.md restriction on pull_request_target is scoped to the claude-review workflow (org credentials, public repo). This workflow carries only a scoped pull-requests: read token and executes no fork code; pull_request_target is safe and intentional here, and the README explains the rationale. ✅
  • The new workflow-level permissions: pull-requests: read correctly narrows the token. ✅

Without set -euo pipefail, a failed gh api call (network error, rate
limit, transient 5xx) left pr_labels_json empty; jq then failed
non-fatally and the step fell through to 'No blocking label present.',
passing the gate despite never checking current labels. Also guard the
non-merge_group path against an empty PR_NUMBER for a clearer failure
message, mirroring the merge_group path's existing check.
@claude

claude Bot commented Jul 16, 2026

Copy link
Copy Markdown

Claude finished @kyle-sexton's task in 1m 32s —— View job


Review: feat/do-not-merge-gate (follow-up)

  • Gather context and understand request
  • Read changed files and diff since last review
  • Assess resolution of prior findings
  • Post updated review findings

Prior findings — resolved

Both findings from the 2026-07-16 review are resolved in commit b383126.

Finding Status
[Medium] set -euo pipefail missing — API failure silently passed the gate ✅ Fixed — set -euo pipefail is now line 139; a failed gh api call now fails the step immediately
[Low] Empty PR_NUMBER unguarded in the non-merge_group path ✅ Fixed — lines 147–151 now mirror the merge_group path's check with an actionable error message

Shell correctness verification

With set -euo pipefail in place, the label-check logic is fail-closed across all paths:

  • gh api exits non-zero (network error, 429, 5xx) → assignment fails → set -e exits 1 immediately. Gate blocks. ✅
  • gh api succeeds, PR has no labels → --jq '[.labels[].name]' returns []jq -e 'any(. == $label)' over empty array exits 1 → if condition false → falls through to "No blocking label present.". Gate passes. ✅
  • gh api succeeds, blocking label present → jq -e exits 0 → then branch → exit 1. Gate blocks. ✅
  • merge_group ref doesn't match the expected pattern → sed produces empty output → [[ -z "$pr_number" ]] guard fires → exit 1. Gate blocks. ✅
  • PR_NUMBER empty on a pull_request event → same guard fires → exit 1. Gate blocks. ✅

No new findings. The implementation is correct and the fail-closed invariant holds on all failure modes.


No new findings

The implementation is complete. CLAUDE.md alignment, security posture, and the prior review's table of Codex findings are all unchanged from the previous review — all confirmed resolved or documented.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Deferred steps 2 and 3 now tracked as real issues instead of only living in this PR body + the Decisions Log artifact:

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Tracking the batched-merge-queue closure decision from the thread above as melodic-software/github-iac#139 (option chosen: cap this repo's merge queue `max_entries_to_merge` to 1, closing the gap structurally rather than enumerating batch membership in-workflow).

This PR's open review thread stays unresolved pending that ruleset change landing — future babysitting passes: no need to re-investigate the batching gap, just check #139's status.

@kyle-sexton

Copy link
Copy Markdown
Contributor Author

Kyle's decision: merge this PR now with the batching gap documented as a known limitation (tracked separately in melodic-software/github-iac#139), rather than blocking on that ruleset change landing first. The gap is disclosed inline in the workflow, in the README's "Known gap with batched merge queues" / "Adoption precondition" sections, and in #139 — not silently accepted.

Resolving this thread and merging.

@kyle-sexton
kyle-sexton merged commit 8853021 into main Jul 16, 2026
30 checks passed
@kyle-sexton
kyle-sexton deleted the feat/do-not-merge-gate branch July 16, 2026 17:34
kyle-sexton added a commit to melodic-software/standards that referenced this pull request Jul 17, 2026
Registers two reusable-workflow contracts required by the Wave 3
per-repo floor conversion (melodic-software/github-iac#78):

- `do-not-merge-gate.yml@8853021…` (melodic-software/ci-workflows#108)
and `pr-issue-linkage.yml@f7e94a8…` (melodic-software/ci-workflows#118)
— both ship the `runner`/`prerequisite-result` shape; registration
unblocks the fleet-routed caller rollouts
(melodic-software/ci-workflows#120 / #121).

The contract set is fail-closed: an unregistered reference fails every
caller's runner-policy lane, so registration must precede the Wave 3
caller PRs.

A `standards-sync.yml@3dfb1845…` registration was included originally
and removed after Codex review: a secret-bearing runner-input contract
without `allowedCallerPermissions` still fails
`privilegedHostedRequirement()`, and the write-permission rule forbids a
read-only `allowedCallerPermissions` map — sync-lane fleet routing is
deferred behind a reviewed secret-capable waiver (tracked on
melodic-software/github-iac#78).

No linked issue.

## Verification

- Contract entries mirror the registered `semantic-pr.yml@51012e2c…`
runner-input shape (`selectorResultInput: prerequisite-result`);
`allowedCallerPermissions` omitted per the validator's write-permission
rule (both are read-only).
- `allowedInputs`/`allowedSecrets` transcribed from each reusable at its
pinned SHA.
- Full component suite passes (181/181).

## Related

- melodic-software/github-iac#78 (epic — Wave 3)
- melodic-software/ci-workflows#120
- melodic-software/ci-workflows#121

🤖 Generated with [Claude Code](https://claude.com/claude-code)

https://claude.ai/code/session_01K3QehVwmWzkBLpKokNCkkt

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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