Repository navigation
ci: fix detect-scope dying on content-only PRs + harden silent-skip - #2584
Conversation
Two real-world failures surfaced when PR #2583 (a content-only blog post polish) became the first PR to exercise the content-only skip: 1. detect-scope died with exit 1 on a content-only diff. The script used 'set -uo pipefail' but GitHub Actions runs bash with -e by default, and 'set -uo' doesn't touch -e. When every changed file matched the content patterns, the final 'grep -vE' produced no output and exited 1 (normal grep behavior), set -e escalated that to a job failure. Fix: explicit 'set +e' + a '|| true' guard on the offending grep chain. 2. Because visual-regression has 'needs: detect-scope', Actions auto-skipped visual-regression when detect-scope failed — a silent bypass of the layout gate, the exact failure mode the safety-default comments in the job warned against. Fix: change visual-regression's if: to 'always() && (detect-scope failed OR content_only != true)' so a broken detector ALWAYS runs the regression rather than silently skipping it. Together: detect-scope can no longer die on its intended use case, and even if it did, the regression would still run as a fail-safe. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![wheels-bot[bot]](https://avatars.githubusercontent.com/in/3657588?s=60&v=4){kind=small}
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR fixes two real bugs in the detect-scope CI job introduced in #2581 — a spurious set -e kill on content-only diffs and a silent skip of visual-regression when the job fails. Both fixes are technically correct, the PR body explains the root cause clearly, and the change is minimal. Approving.
Correctness
Bug 1 — set -e kills detect-scope on content-only diffs
.github/workflows/web-deploy.yml lines 99–107:
set +e
set -u
set -o pipefailGitHub Actions runs bash steps with set -eo pipefail by default. The original set -uo pipefail left -e active. On a content-only diff, every file matched the pattern in grep -vE, so the filter produced no output and exited 1. With -e in effect, that exit 1 terminated the whole script before it could emit content_only=true. The set +e fixes the root cause; the || true on the grep chain is correct belt-and-suspenders for the case where a future edit re-enables -e.
Bug 2 — silent skip via auto-skip on failed dependency
.github/workflows/web-deploy.yml lines 192–197:
if: |
always() &&
(needs.detect-scope.result != 'success' ||
needs.detect-scope.outputs.content_only != 'true')Without always(), GitHub Actions auto-skips a job when its needs: dependency fails, which means the layout gate disappears silently. The three-case analysis holds:
- detect-scope success +
content_only=true→ skip (correct) - detect-scope success +
content_only=false→ run (correct) - detect-scope failed → run (correct, fail-open safety behavior)
This is the canonical GitHub Actions pattern for "fail open" and it is applied correctly here.
One edge case (not blocking): When detect-scope fails before emitting blog_content_changed, the downstream Pre-refresh blog baseline step won't fire (blog_content_changed will be empty, not 'true'). The visual regression still runs — just without a pre-refreshed blog baseline, which is safe.
Conventions
No CFML code changed. CI-only diff; CFML anti-patterns checklist does not apply.
Cross-engine
No CFML code changed. Cross-engine checklist does not apply.
Tests
This is CI infrastructure. No CFML test specs are expected or applicable.
The PR's own test plan is reasonable: the workflow-file change makes this PR itself a non-content-only PR (content_only=false), so visual-regression will run and validate the surrounding logic. The stated follow-up (#2583 rebase) will exercise the content-only skip path.
Commits
Single commit: ci: fix detect-scope dying on content-only PRs and silent skip
- Type
ciis in the allowlist. - Subject is 61 characters, not ALL-CAPS.
- Matches
commitlint.config.jsrules.
Docs
No user-facing behavior changed; no guide pages or .ai/ docs update needed.
Wheels Bot — Reviewer B (round 1)A's review is technically accurate and well-evidenced. I re-read the diff independently and agree on all material points. Converging on approve. SycophancyNone detected. The approval is backed by line citations and a three-case enumeration — not a bare LGTM. False positivesNone detected. I verified each of A's claims against the diff:
Missed issuesOne minor observation A did not raise, not blocking: Everything else checks out: Verdict alignmentA's ConvergenceAligned. A's analysis is accurate, the approval is warranted, and I have no unresolved disagreements. Recommending approve with no changes needed. |
Summary
PR #2583 (a content-only blog post polish — exactly the case #2581 introduced the content-only skip for) became the first PR to exercise the new logic, and surfaced two real bugs:
detect-scope dies on its intended happy path. The script used `set -uo pipefail` but GitHub Actions runs bash with `-e` by default — `set -uo` does NOT touch `-e`. On a content-only diff, every changed file matches the content patterns, so the final `grep -vE` filters out everything and exits 1 (normal grep "no matches" behavior). With `-e` still on, that one exit-1 killed the whole detector. Fix: explicit `set +e` early in the script plus a `|| true` guard on the offending grep chain. Belt and suspenders.
Silent skip when detect-scope fails. `visual-regression` has `needs: detect-scope`. When detect-scope failed (as in bug New master #1), Actions auto-skipped visual-regression — the layout gate just disappeared. That's the exact silent-bypass the safety-default comments in the job were trying to prevent. Fix: change visual-regression's `if:` to:
```yaml
if: |
always() &&
(needs.detect-scope.result != 'success' ||
needs.detect-scope.outputs.content_only != 'true')
```
The `always()` is required so the `if:` is evaluated even when the dependency failed. Now: detect-scope healthy + content_only=true → skip; detect-scope healthy + content_only=false → run; detect-scope failed (any reason) → run. There is no path that silently skips.
Why this should land before #2583 is rebased
Without this hotfix, every content-only PR (the whole point of #2581) hits the same exit-1 trap. #2583 will be the validation that the hotfix works.
Test plan
Local validation
Reran the same logic locally with bash 5.x (`set -e` enabled) — confirmed the original script dies at the grep step on a content-only changeset, and the fixed version completes and emits `content_only=true`.
🤖 Generated with Claude Code