Skip to content

fix(do-not-merge-gate): drop gh CLI dependency from label refetch#144

Merged
kyle-sexton merged 1 commit into
mainfrom
fix/do-not-merge-gate-gh-free
Jul 18, 2026
Merged

fix(do-not-merge-gate): drop gh CLI dependency from label refetch#144
kyle-sexton merged 1 commit into
mainfrom
fix/do-not-merge-gate-gh-free

Conversation

@kyle-sexton

Copy link
Copy Markdown
Contributor

Summary

The do-not-merge-gate reusable's live label refetch ran gh api in a run step. The gate executes on whatever runner the caller selects, and a self-hosted image is not guaranteed to ship the gh CLI — observed on melodic-software/claude-code-plugins#330: exit 127 gh: command not found failed the required check with no blocking label present.

This ports the step to actions/github-script (same SHA-pinned v9.0.0 the pr-issue-linkage reusable already uses on the same runners). The merge_group PR-number parse, live-refetch semantics, and error messages are unchanged; the runtime is the action's bundled Node, removing the runner-image tooling dependency.

Other reusables still call gh (claude-review, link-check, queue-monitor-liveness, tool-version-drift-check); they are out of scope here — this fixes the one gate observed failing as a required consumer check.

No linked issue

Related

Verification

  • actionlint clean; zizmor no findings on the edited workflow
  • Logic port is 1:1 (parse → live refetch → label test → identical error strings)

🤖 Generated with Claude Code

The gate's live label refetch ran `gh api` in a run step, but the gate
executes on whatever runner the caller selects and a self-hosted image
is not guaranteed to ship the gh CLI. Observed on a consumer PR: exit
127 `gh: command not found` failed this required check with no blocking
label present.

Port the step to actions/github-script (same SHA-pinned version the
pr-issue-linkage reusable already uses on the same runners): the
merge_group PR-number parse, live refetch, and error messages are
unchanged; the runtime is the action's bundled Node, so the check no
longer depends on runner-image tooling.

Co-Authored-By: Claude Fable 5 (1M context) <noreply@anthropic.com>
@claude

claude Bot commented Jul 18, 2026

Copy link
Copy Markdown

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


Review

  • Gather context and read changed file
  • Analyze diff for correctness, security, and CLAUDE.md compliance
  • Post review findings

Summary: The port is correct and clean. No blockers. A few observations below.

Correctness — logic is 1:1

Concern Old (bash) New (JS) Match?
merge_group PR parse sed -nE 's#.*/pr-([0-9]+)-[0-9a-f]+$#\1#p' /\/pr-(\d+)-[0-9a-f]+$/.exec(headRef)
pull_request PR number ${{ github.event.pull_request.number }} via env context.payload.pull_request?.number
Live label refetch gh api "repos/${REPO}/pulls/${n}" github.rest.pulls.get({…})
Label test jq -e 'any(. == $label)' Array.includes()
Error strings unchanged unchanged

context.payload.pull_request?.number is the correct accessor for both pull_request and pull_request_target — both events carry the full PR object in the payload.

Security

Script injection prevention is correct. Both inputs.label and github.event.merge_group.head_ref flow through env: at the step level and are read as process.env.* inside the script — never interpolated directly into the script: body. This is the recommended pattern for untrusted/user-supplied values. ✅

SHA pin on line 139: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 — matches CLAUDE.md's requirement and the version already used in pr-issue-linkage. ✅

Token: github-script defaults to github.token, which is the same credential the old GH_TOKEN: ${{ github.token }} used. Scoped to the workflow-level pull-requests: read. ✅

pull_request_target on forks: The API call only needs pull-requests: read, and on fork PRs the token is already read-only. No privilege escalation risk. ✅

Minor upside worth noting

github-script uses Octokit with its built-in retry/throttle plugins, so the live refetch is slightly more resilient to transient 5xx/429 responses than the raw gh api call was. Not a required change — just a free improvement.

One nit (non-blocking)

Line 148: the ?? "" fallback on MERGE_GROUP_HEAD_REF is defensive but harmless — the env var is always a string when set via env: (GitHub renders the missing expression as an empty string before the runner sees it). Not a problem, just belt-and-suspenders.


Verdict: LGTM. The fix is minimal, targeted, and leaves all existing semantics intact. The only change is the runtime surface (bundled Node via github-script instead of the runner's gh binary), which is exactly the right move for a workflow that may land on arbitrary self-hosted images.

@kyle-sexton
kyle-sexton merged commit 90f1c54 into main Jul 18, 2026
35 checks passed
@kyle-sexton
kyle-sexton deleted the fix/do-not-merge-gate-gh-free branch July 18, 2026 06:58
kyle-sexton added a commit to melodic-software/standards that referenced this pull request Jul 18, 2026
…I-free SHA (#190)

## Summary

Advances the reviewed runner-input contract key for the
`do-not-merge-gate` reusable from `8853021` to `90f1c54`
(melodic-software/ci-workflows#144 — label refetch via
`actions/github-script` instead of `gh api`, fixing exit-127 failures on
self-hosted runners without the gh CLI).

Input surface is unchanged: `runner`, `prerequisite-result`, `label`; no
secrets; same routing (`runner-input`). SHA re-review only.

Consumer follow-up: melodic-software/claude-code-plugins#332 bumps the
workflow pin and is blocked on this entry syncing.

No linked issue

## Related

- melodic-software/ci-workflows#144 (the gate fix)
- melodic-software/claude-code-plugins#332 (blocked consumer pin bump)

## Verification

- `policy.json` parses; biome/typos/gitleaks/editorconfig pre-commit
lanes pass; entry identical apart from the SHA

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

Co-authored-by: Claude Fable 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit to melodic-software/ci-runner that referenced this pull request Jul 18, 2026
## What

Move the ci-workflows compatibility pin `c36e881` (v0.5.0) → `90f1c54`
(v0.6.1) across all 20 workflow references, the pin-metadata test
constants, and `release/dependencies.json`.

## Why

v0.5.0's `do-not-merge-gate` shells `gh` in its label refetch; v0.6.1
carries the github-script rewrite (melodic-software/ci-workflows#144)
plus the ubuntu-slim fallback removal
(melodic-software/ci-workflows#141). This repo's Product policy gate
requires every ci-workflows reference to move as one reviewed pin with a
canonical `@<sha> # vX.Y.Z` comment, so the org's narrow gate-only bump
pattern converges here as a full pin move instead.

## Verification

All 28 `.github/scripts` tests pass locally, including the pin-metadata
inventory (20 references, one SHA, one release). This PR's own
do-not-merge run resolves the bumped ref. `git grep c36e881` = 0.

v0.5.0..v0.6.1 caller impact reviewed: new `link-check` inputs all carry
defaults; fallback runner labels and gate internals changed inside the
reusables only. No caller contract changes.

## Related

No linked issue. Sibling bumps: melodic-software/provisioning#167,
melodic-software/claude-code-plugins#332, melodic-software/dotfiles#220;
gate fix melodic-software/ci-workflows#144.

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

https://claude.ai/code/session_01YZxBP1B8Hf7ZRaqDrP6ma9

---------

Co-authored-by: Claude Fable 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit to melodic-software/claude-code-plugins that referenced this pull request Jul 18, 2026
## Summary

Bumps the `do-not-merge-gate` reusable pin to ci-workflows `90f1c54`
(melodic-software/ci-workflows#144): the label refetch now runs via
`actions/github-script` instead of `gh api`, so the required-context no
longer fails `exit 127: gh: command not found` on self-hosted runners
without the gh CLI (observed on #330).

This PR's own `do-not-merge / do-not-merge` check still runs the OLD pin
from main (`pull_request_target` evaluates the base-branch definition)
and may fail the same way — expected until this merges; the check is not
in the required set.

No linked issue

## Related

- melodic-software/ci-workflows#144 (the gate fix)
- #330 (where the failure surfaced)

## Verification

- actionlint clean

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

Co-authored-by: Claude Fable 5 (1M context) <noreply@anthropic.com>
kyle-sexton added a commit that referenced this pull request Jul 22, 2026
)

Closes #209

## Summary

The 2026-07-18 `do-not-merge` fleet spike (~191 failures across 6 repos,
`exit 127: gh: command not found`) traced to a `gh api` shell step in a
reusable workflow running on a caller-selected runner, exposed when
routing shifted callers onto a self-hosted image that does not ship the
gh CLI. PR #144 fixed that one reusable (`do-not-merge-gate.yml`) and
named four sibling reusables still out of scope; this issue expanded the
sweep to all nine gh-calling reusables in the fleet and ports every
confirmed at-risk instance to `actions/github-script` (the #144 pattern
— bundled Node runtime, no runner-image tooling dependency).

## Per-reusable audit verdict

| Reusable | `runs-on` | Verdict | Notes |
| --- | --- | --- | --- |
| `claude-review.yml` | `${{ inputs.runner }}` | **Ported** | Freshness
check + both infra-failure comment-marker steps |
| `claude-security-review.yml` | `${{ inputs.runner }}` | **Ported** |
`changes` job's PR-file listing (split into a github-script fetch +
unchanged `git check-ignore` matching), freshness check, both
comment-marker steps |
| `claude-e2e-verify.yml` | `${{ inputs.runner }}` | **Ported** |
Freshness check only. The prompt-embedded `gh pr diff` / `gh pr comment`
text is agent-environment usage (the Claude agent's own tool-use, not a
workflow `run:` step) — out of scope by the same principle decision 1
applied to `claude-review.yml`'s prompt text, just not previously
spelled out for this file |
| `link-check.yml` | `${{ inputs.runner }}` | **Ported** | All three
gh-calling steps: "Find existing tracking issue" (folds in
`find-tracking-issue.sh`'s resolver logic — dropped from that generated
block's consumer list since only a fixed-runner consumer still needs the
shared bash source), "Assert native issue type" (raw `PATCH` request —
the `type` field isn't guaranteed on every octokit release's typed
`issues.update`), "Close recovered tracking issue" |
| `pulumi-version-drift-check.yml` | `${{ inputs.runner }}` | **Ported**
| Full drift-detection + tracking-issue lifecycle, reproducing the bash
`gh_read`/`gh_mutate` bounded-read-with-single-retry semantics in JS.
Retires `pulumi-version-drift.sh`, `render-pulumi-version-drift.cjs`,
and `pulumi-version-drift.test.sh` — the workflow no longer embeds a
generated shell copy to keep in sync |
| `queue-monitor-liveness.yml` | `ubuntu-24.04` (hardcoded) | Cleared |
Not `workflow_call` — a standalone scheduled workflow, always on the
fixed GitHub-hosted image, which ships gh. Not caller-selectable |
| `tool-version-drift-check.yml` | `ubuntu-24.04` (hardcoded) | Cleared
| Same reasoning as above |
| `release.yml` | `ubuntu-24.04` (hardcoded) | Cleared |
`workflow_dispatch`-only, not `workflow_call`; same reasoning |
| `do-not-merge-gate.yml` | `${{ inputs.runner }}` | Cleared (residual)
| Re-audited for a residual shell `gh` call per the issue's "(residual)"
flag — none found; #144 already fully ported this file |

## Other changes

- README: new "Triage: fleet-wide single-workflow failure spikes"
section capturing the 2026-07-18 lesson (check commit history + runner
routing before attributing to infra flake), plus an updated
`pulumi-version-drift-check.yml` bullet reflecting the github-script
implementation.
- REVIEW.md: new "Always check" line — a reusable-workflow step shelling
out to a CLI on a caller-selected runner must use
`actions/github-script` or carry a justified guard, citing
`conventions/review/cross-platform.md#tools-processes-and-committed-artifacts`.

## Verification

- `node --test .github/scripts/*.test.cjs` — 209/209 passing, including
rewritten assertions for the ported steps (`link-check.test.cjs`,
`pulumi-deploy-guard.test.cjs`, `network-timeout-policy.test.cjs`,
`find-tracking-issue-render.test.cjs`,
`claude-review-superseded-guard.test.cjs`)
- `bash .github/scripts/find-tracking-issue.test.sh` — passing
(unchanged shared source, now consumed only by the two fixed-runner
workflows)
- `actionlint` — clean on all five ported workflows
- `shellcheck` — clean on the remaining bash sources
- `zizmor` — identical finding count before and after (18 findings: 1
low, 3 medium, 1 high, all pre-existing and unrelated to this change —
verified by diffing against the `origin/main` version of each ported
file)
- `markdownlint-cli2` — clean on README.md and REVIEW.md

## Related

- Closes #209, which extends PR #144's fix to the rest of the fleet.

---------

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