fix(workflows): extract workflow_run fields via jq from $GITHUB_EVENT_PATH#27
Conversation
…_PATH PR #26's 'none' fallback also didn't fix the queue-time parse error on workflow_dispatch. Confirmed via diagnostic: pr-labels-ci.yml's API metadata still reports name as the file path, not 'PR Label Automation (CI)' — meaning GitHub's workflow catalogue still considers it unregistered. Taking QA's suggested approach: eliminate the ${{ }} expression surface on workflow_run fields entirely. Read them from the event payload on disk via jq instead. Changes per job: - Remove RUN_ID and HEAD_BRANCH from step-level env: (the only remaining ${{ github.event.workflow_run.* }} sites outside the job-level if: guards). - Add at the top of run:: RUN_ID=$(jq -r '.workflow_run.id // empty' "$GITHUB_EVENT_PATH") HEAD_BRANCH=$(jq -r '.workflow_run.head_branch // empty' "$GITHUB_EVENT_PATH") - Add defensive early-exit if RUN_ID is empty — belt-and-suspenders for the existing if: guard in case someone loosens it later. Shell-injection safety: jq -r writes raw bytes into a shell variable via command substitution, and every subsequent use is double-quoted. Same guarantee as the prior env: pattern. jq and $GITHUB_EVENT_PATH are both available on all GitHub-hosted runners (ubuntu-latest / macos-latest) — documented, not a hack. Unchanged: - Job-level if: guards still reference github.event.workflow_run.*. These are identical to mcp-clipboard's working version and should short-circuit cleanly on non-workflow_run events via &&. If this PR still hits the same queue-time parse error, the root cause is in the if: surface rather than the env: surface, which points toward reverting the whole seeding-attempt chain (#24, #25, #26, this PR). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
cmeans
left a comment
There was a problem hiding this comment.
[QA] Review — zero findings, Ready for QA Signoff.
Verified in this session
| Check | Result |
|---|---|
| YAML parse | Clean. on: carries workflow_dispatch + workflow_run. |
No remaining step-level ${{ github.event.workflow_run.* }} |
Confirmed via grep -n "github.event.workflow_run" .github/workflows/pr-labels-ci.yml — all 6 remaining hits are inside job-level if: guards (lines 44–46 and 117–119), which is what we want. |
jq paths use // empty |
Both RUN_ID and HEAD_BRANCH extractions use // empty → missing key produces zero bytes on stdout → $RUN_ID="". |
| Shell-injection safety preserved | $HEAD_BRANCH is always double-quoted (--head "$HEAD_BRANCH"); $RUN_ID is interpolated into a URL path only (GitHub-generated numeric ID, not contributor-controlled). |
| Happy-path behavior | jq -r '.workflow_run.id // empty' reads the same value that ${{ github.event.workflow_run.id }} would have produced. Byte-identical RUN_ID/HEAD_BRANCH values feed the exact same downstream script. Zero change on the path that promotes PRs. |
pytest tests/ |
259/259 pass. |
ruff check src/ tests/ |
Clean. |
jq semantics sanity-check
Ran three cases against synthetic event files:
- Happy path (
{"workflow_run":{"id":12345,"head_branch":"foo/bar"}}) →RUN_ID='12345',HEAD_BRANCH='foo/bar'✓ - Missing context (
{"other":"thing"}) → both empty strings ✓ (early-exit fires) - Injection attempt (
head_branch = "foo;rm -rf /;#") →HEAD_BRANCH='foo;rm -rf /;#'captured as a single literal value; quoted usage downstream means no shell interpretation ✓
Diagnostic finding worth flagging
While validating the fix I pulled the run history for mcp-clipboard's pr-labels-ci.yml too, to cross-check:
| Repo | API name field |
workflow_run runs | push/failure runs |
|---|---|---|---|
cmeans/mcp-clipboard |
.github/workflows/pr-labels-ci.yml (path) |
8 (success + skipped) | 2 |
cmeans/yt-dont-recommend |
.github/workflows/pr-labels-ci.yml (path) |
0 | 16 |
mcp-clipboard shows the exact same name == path symptom, but still dispatches workflow_run events normally. So name == path is not a reliable diagnostic for non-registration — it's just how the API reports workflows whose declared name: hasn't propagated yet. The real signal is workflow_run runs > 0, and that's what we're actually trying to unstick.
Practical consequence: after merging #27, the better post-merge check is "does a new workflow_run run (success or skipped) appear in the history after the next CI completes on a PR?" rather than "did the name field flip?" The name may or may not flip independently.
On the fix itself
This is the right structural change regardless of whether it fixes the registration issue. Moving contributor-influenced context from step-level env: (queue-time evaluation) to jq reads of $GITHUB_EVENT_PATH (step-time) is strictly safer and more forgiving, and the defensive early-exit is a reasonable belt-and-suspenders over the if: gate. If the registration still doesn't recover, the code we're left with is still better than where we started — the revert path is clean either way.
Verdict
Applying Ready for QA Signoff. Zero findings. Post-merge: try the manual dispatch as planned, but also watch the next PR's CI completion for a workflow_run entry in pr-labels-ci.yml's run history — that's the ground truth.
…actual bug) Root cause for the workflow_dispatch parse failures that defeated PRs #24, #25, #26, and #27: GitHub Actions substitutes '\${{ ... }}' expressions inside run: blocks *before* the shell sees them, including sequences inside shell comments. Three comments in this file contained the literal string '\${{ }}' (empty expression). GHA's queue-time parser tried to evaluate an empty expression and bailed with 'An expression was expected', always pointing at 'run: |' (col 14) because that's the parent scope. Diagnostic that confirmed this: the reported line numbers shifted between my earlier attempts (55/111 -> 53/126 after the jq refactor). The error position is file-structure-dependent; col 14 is always the '|' of 'run: |'. Meanwhile the literal '\${{ }}' was identical across every version of the file since the security-hardening cascade (mcp-clipboard#87), which is why every '|| X' fallback experiment missed — the fallbacks were on the wrong expressions. Fix: rewrite the three comments to describe the concept without the literal '\${{ }}' characters. Zero logic change. Why this only surfaced on workflow_dispatch (not on normal workflow_run firings): the normal workflow_run code path apparently tolerates empty expressions where the queue-time parser for workflow_dispatch does not. mcp-clipboard has the same latent bug — it just never tripped it because nobody dispatched manually. Cascade: the same fix needs to land on cmeans/mcp-clipboard's file to preserve verbatim parity and to unblock manual dispatch there as well. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
QA's proposed approach after PR #26's
'none'fallback also failed to fix the queue-time parse error. Diagnostic confirmed the workflow is still unregistered in GitHub's catalogue (namefield on the API still reports the file path, not"PR Label Automation (CI)").Strategy: eliminate the
${{ }}expression surface on workflow_run fields entirely. Read them from the event payload on disk via jq.Change (+34 / −9, one file)
For both
on-ci-passandon-ci-fail:Remove
RUN_IDandHEAD_BRANCHfrom step-levelenv:(the only remaining${{ github.event.workflow_run.* }}sites outside job-levelif:).At the top of each
run:block, add:Add a defensive early-exit if
RUN_IDis empty — belt-and-suspenders for the existingif:guard.Why this might succeed where
|| ''/|| 'none'didn'tAll three of my prior attempts modified step-level env expressions. GitHub's queue-time parser kept returning the same error at
run: |in both jobs. The suspected cause shifted from "missing context" to "||fallback" to "empty-string literal" — none of those fixes worked.The jq path routes around the expression surface entirely:
${{ }}in the workflow_run fields. Whatever GitHub's queue-time parser disliked about the env expressions can't fire here — there's no such expression.$GITHUB_EVENT_PATHis always set on GitHub-hosted runners, on every event type, per official docs. Canonical way to read the raw event.jqis pre-installed on allubuntu-latest/macos-latestimages. No install step.jq -rwrites raw bytes to stdout; command substitution captures them into a shell variable; every subsequent use is"$HEAD_BRANCH"quoted. Byte-equivalent safety to the prior env pattern.Unchanged
if:guards still referencegithub.event.workflow_run.*(identical to mcp-clipboard's working version). These use&&and should short-circuit cleanly on non-workflow_run events.on-ci-passandon-ci-failhappy-path behavior is identical to today's.Decision rule
if:surface (or elsewhere entirely). Revert PRs fix(workflows): add workflow_dispatch to pr-labels-ci.yml to prime workflow_run listener #24, fix(workflows): guard pr-labels-ci.yml env against missing workflow_run context #25, fix(workflows): use 'none' instead of '' as null-coalesce fallback in pr-labels-ci.yml #26, and this one in a single cleanup PR, restore byte-parity withcmeans/mcp-clipboard, document theDev Activetoggle as the permanent workaround in CLAUDE.md. Stop spending time on the seeding path.Test plan (QA)
yaml.safe_loadon the file — clean parse (verified locally)${{ github.event.workflow_run.* }}expressions remain outside job-levelif:guards// emptyso missing keys resolve to empty string, not"null"$RUN_IDand$HEAD_BRANCHpreserved everywhere (no shell-injection regression)on-ci-passandon-ci-failhappy-path behavior unchanged on real workflow_run eventsif:on a workflow_dispatch event; if they don't skip, the defensive early-exit catches it)gh api repos/cmeans/yt-dont-recommend/actions/workflows/pr-labels-ci.yml --jq '.name'— watch for"PR Label Automation (CI)"Caveat
Same as PRs #24 / #25 / #26 — this PR itself will stick at
Awaiting CIbecause the fix only activates post-merge-and-seed. Expect the Dev Active toggle once more.🤖 Generated with Claude Code