ci: unblock docs-only PRs without weakening the gate - #1185
Conversation
`ci.yml` carries `paths-ignore: ['**/*.md', '.myrobots/**', 'LICENSE']` so a prose-only change doesn't burn a ~25-minute pipeline. But ruleset 16042163 REQUIRES `typecheck + unit + ART + E2E` and `vrt-strict (visual regression — strict subset)`, and a path-skipped workflow reports NOTHING — GitHub treats a never-reported required check as pending forever. A docs-only PR therefore sits at mergeStateStatus=BLOCKED / mergeable=MERGEABLE with zero failures and can never auto-merge (#1184, armed since 06:06Z, permanently stuck). Adds `.github/workflows/docs-only-gate.yml`: fires on the EXACT INVERSE path filter (same list P — ci.yml `paths-ignore: P`, this `paths: P`) and posts the two required contexts as commit statuses, but only when TWO independent guards agree — every changed file is a doc (G1) AND GitHub started no ci.yml run for this head SHA (G2). A PR touching docs AND code fires both workflows; the bypass posts nothing and the real suite gates it. Posts statuses rather than naming jobs after the required contexts: a job-level `if:` skip reports as SUCCESS to branch protection, which would satisfy the gate on exactly the mixed docs+code case where it must not. `scripts/docs-only-gate.test.ts` (unit lane, 52 tests) pins the complement invariant, the context strings against ci.yml's job names, and the negative controls — no changeset containing a source file can ever be bypassed. ci.yml is untouched, so code PRs' wall time does not move. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a docs-only GitHub Actions gate, shared decision logic, and Vitest coverage. The gate detects documentation-only pull requests without a ChangesDocs-only CI bypass
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant DocsOnlyGate
participant GitHubAPI
participant DecideBypass
participant CommitStatuses
PullRequest->>DocsOnlyGate: open or update pull request
DocsOnlyGate->>GitHubAPI: fetch changed files
DocsOnlyGate->>GitHubAPI: query ci.yml runs for head SHA
DocsOnlyGate->>DecideBypass: pass files, CI state, and repository state
DecideBypass-->>DocsOnlyGate: return post decision and reason
DocsOnlyGate->>CommitStatuses: post required success contexts when approved
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
`actions/github-script` evaluates `script:` inside a `new AsyncFunction(...)` body, where dynamic `import()` has no reliable module referrer — the one code path the live both-touched run on this PR could not exercise, because the posting step is (correctly) guarded off. The decide step now emits `contexts=<json>` from REQUIRED_CONTEXTS and the posting step does `JSON.parse(process.env.CONTEXTS)`. docs-only-gate.mjs stays the single source of truth, still pinned to ci.yml's job names by the unit test, with no ESM-in-vm dependency. Adds 4 tests: the workflow wiring (`CONTEXTS` env in, `JSON.parse` out, no `await import(`), plus three real `node scripts/docs-only-gate.mjs decide` invocations asserting the exact $GITHUB_OUTPUT the workflow reads — including the JSON round-trip back to REQUIRED_CONTEXTS. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/docs-only-gate.yml:
- Around line 64-67: Update the Checkout step using actions/checkout@v4 to
disable credential persistence by setting persist-credentials to false, while
leaving the rest of the workflow unchanged.
- Around line 139-142: Update the “Real CI is authoritative — no bypass” step to
avoid interpolating steps.decide.outputs.reason directly into the shell script.
Pass the reason through the step’s environment and reference the environment
variable with shell-safe quoting in the notice command, preserving the existing
message and condition without allowing PR-controlled filenames to execute as
shell syntax.
- Around line 74-89: Update the Resolve the PR's changed files step to generate
a unique multiline-output delimiter for each invocation instead of using the
static CHANGED_FILES_EOF marker. Ensure the delimiter cannot collide with
PR-supplied filenames, then emit all resolved filenames and the matching closing
delimiter to the CHANGED_FILES output so decideBypass receives the complete file
list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 11f61c20-edf5-4ac7-ab9a-fad16ed1a17a
📒 Files selected for processing (4)
.github/workflows/docs-only-gate.ymlCLAUDE.mdscripts/docs-only-gate.mjsscripts/docs-only-gate.test.ts
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v4 | ||
|
|
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable credential persistence on checkout.
actions/checkout@v4 defaults to persist-credentials: true, leaving the job's token in .git/config for the rest of the job. Given this workflow subsequently runs run: steps that interpolate PR-controlled data (see the template-injection finding below), a persisted token compounds the blast radius of any code-execution primitive into token exfiltration.
🔒 Proposed fix
- name: Checkout
uses: actions/checkout@v4
+ with:
+ persist-credentials: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| steps: | |
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| steps: | |
| - name: Checkout | |
| uses: actions/checkout@v4 | |
| with: | |
| persist-credentials: false |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/docs-only-gate.yml around lines 64 - 67, Update the
Checkout step using actions/checkout@v4 to disable credential persistence by
setting persist-credentials to false, while leaving the rest of the workflow
unchanged.
Source: Linters/SAST tools
Adversarial pass over the bypass workflow. Filenames from a PR diff are
attacker-controllable (a branch can carry a path containing quotes, backticks or
even a newline — git allows it), and they flow into three places:
1. `run:` bodies — the "no bypass" step interpolated the decision reason, which
embeds those filenames, directly into the shell via `${{ }}`. Now passed via
`env:`, like every other step output. Guarded by a test that walks all four
`run:` bodies plus the github-script `script:` body and rejects any `${{ }}`
interpolation of `steps.*` / `github.event.*`.
2. `$GITHUB_OUTPUT` — an unflattened reason carrying CR/LF would inject a second
`key=value` line, and the last value wins, so a crafted path could forge
`post=true` past the guarded posting step. Reason is now flattened on `\r`
AND `\n`; negative-control test included.
3. The changed-file heredoc — a fixed `CHANGED_FILES_EOF` delimiter appearing in
the payload would TRUNCATE the file list, the one truncation that could make a
mixed PR look docs-only to G1. Delimiter is now randomised per run (GitHub's
documented practice). G2 would still have refused; not relying on the second
guard for something this cheap.
Also converts the G2 poll's trailing `[ … ] && sleep` to an explicit `if` —
verified it does NOT trip `set -e` today (the failing test is a non-final command
of an AND-OR list, so it is exempt), but the exhaustion path is the docs-only
path, and it should not depend on that subtlety.
60 tests, 3× clean; actionlint clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
CI note — The service container failed to start; zero tests executed (the upload steps confirm it: "No files were found with the provided path: e2e/test-results" / "e2e/blob-report"). This is not a test failure and it is unrelated to this PR, which touches only a workflow file, two scripts and CLAUDE.md. Root cause: Worth noting for a follow-up: the comment above the Re-running the failed job only (the diagnosis is complete and the cause is provably external to the repo). |
1 participant
The deadlock
.github/workflows/ci.ymlcarries, on bothpushandpull_request:The intent is right — a prose-only change should not burn a ~25-minute pipeline.
But ruleset 16042163 ("main: green PRs only") requires two status contexts:
typecheck + unit + ART + E2Eci.ymljobci(the umbrella)vrt-strict (visual regression — strict subset)ci.ymljobvrt-strictWhen CI is path-skipped those contexts never report, and GitHub treats a
never-reported required check as pending forever. The PR lands at
mergeStateStatus: BLOCKEDwithmergeable: MERGEABLE, zero failures, zerorunning jobs, and can never auto-merge. There is nothing to re-run: the run
does not exist.
Verified on the live cases
#1184 — docs-only (
.myrobots/26-07-22-roundup.md), auto-merge armed since2026-07-27T06:06:49Z,BLOCKED.gh run list --branch docs/roundup-2026-07-27returns exactly one run —
Deploy— and noCIrun at all. The onlycontexts that ever reported are
Deploy → PR-1184 previewandCodeRabbit.#1175 — the
.myrobotscorpus PR — merged fine, and that confirms thediagnosis rather than contradicting it: it touched 62 files, 61 under
.myrobots/**plus.gitignore, which is not inpaths-ignore. Onenon-ignored file ⇒ CI ran ⇒ contexts reported ⇒ mergeable.
Verify CI passed for this branch(reported skipped on #1184) is notrelated: it is
deploy.yml'sverify-cijob,if: github.event_name == 'workflow_dispatch'— a guard for manual Actions-UI deploys, not a PR gate. Itwas never meant to solve this and is left alone.
This is about to bite repeatedly:
.myrobots/is now a tracked planningdirectory and a large DX7/UI program plan is being written into
.myrobots/plans/right now.The option chosen — and why
Option 1 (companion workflow on the inverse path filter), made sound.
Rejected alternatives:
paths-ignore(option 2's premise). ~25 min of CI wall-time on everytypo fix, on a repo where a cycle is ~25 min under load and
.myrobots/is nowa tracked planning corpus. That deliberate optimisation is worth keeping.
detector has to be a dependency of every job, which serialises ~20 s in front
of a ~4-5 min critical path on every code PR — paying a permanent tax on
the common case to fix the rare one. It also rewrites the most load-bearing
workflow in the repo (125 KB, 20 jobs, 3 attest gates) to fix a trigger bug.
form). Unsound — see below.
Why the naïve form is unsound, and what this does instead
GitHub path filters are per-file ANY-match. With the same list
P:so for a non-empty changeset:
Path filters cannot express "every changed file is a doc" — they are
ANY-match, and
!negation insidepaths:only re-expressespaths-ignore. Sothe companion does fire on a mixed PR, and the naïve form breaks there:
starts and cannot withdraw it; and
if:skip reports as SUCCESS to branch protection — so gatingthe job on "docs only" would satisfy the context on precisely the mixed PRs
where the real CI is still running or red.
This PR therefore posts commit statuses from an explicitly guarded step, not
job names. On a mixed PR the workflow emits nothing at all. The job is
named
docs-only gate (bypass check, not a required context)and a unit testasserts no job in the file is named after a required context.
Both-touched-paths handling — two independent guards
A status is posted only when both hold:
gh api pulls/N/files --paginate) matchesP, computed locally..github/workflows/ci.ymlexists for this headSHA, polled for 60 s. This asks GitHub what it did rather than re-deriving
glob semantics, so it is exact by construction.
Either guard alone closes the mixed case: G2 because a mixed PR always produces a
CI run, G1 because it sees the source file. Requiring both means any disagreement
between this matcher and GitHub's — e.g. whether
**/*.mdmatches a rootREADME.md, which GitHub's filter engine and minimatch read differently —degrades to "no bypass, real CI gates", never to "bypass while code went
ungated". G2 also fails safe: an API error is treated as run present.
Two smaller cases: a fork PR is refused (its
GITHUB_TOKENcannot writestatuses); a PR with >300 files has its path filters skipped entirely by
GitHub, so both workflows run and G2 refuses.
Statuses are per-commit, so a docs-only SHA that later gains a code commit gets a
new SHA with no bypass status and a real CI run.
It also never creates a
ci.ymlrun, sodaily-prod-deploy.yml'sfind-greenscan (which queries
actions/workflows/ci.yml/runs?head_sha=and asserts theumbrella job is present) cannot be fooled into treating a docs-only commit as a
fully-green deploy candidate.
Negative-control evidence
This PR IS the both-touched case, live
It touches
CLAUDE.mdand three code files, so both workflows fired on35344f34— the exact scenario the design has to survive. From the bypass job'slog:
Both guards fired independently — G2 found the real CI run on its first poll,
G1 named the three source files — and the commit-status list for that SHA is:
Zero statuses from the bypass. The required contexts on this PR come only
from the real
ciumbrella andvrt-strictjobs.The unit gate
scripts/docs-only-gate.test.ts— 60 tests, pure-unit,unitlane, zeroadded CI wall-time. The decision is a pure function (
decideBypass) so thenegative control is a test, not a CI experiment.
post: false,G1 FAILED):packages/web/src/lib/audio/modules/adsr.ts,packages/dsp/src/cube.ts,packages/server/src/relay.ts,e2e/tests/ai-smoke.spec.ts,e2e/vrt/vrt-exemptions.ts,.github/workflows/ci.yml,.github/workflows/docs-only-gate.yml,scripts/docs-only-gate.mjs,package.json,package-lock.json,Taskfile.yml,.gitignore,contract-lock.txt,db/schema/001_init.sql, a VRT baseline PNG, an ART.f32baseline, plus the adversarialCLAUDE.md.tsanddocs.md/thing.ts.docs-only +
ciRunExists: true→ refused by G2. Each asserted separately.post === (sameRepo && !ciRunExists && isDocsOnly(files))over the full cross-product.
.gitignore) → refused, reproducing why it merged normally; this PR's ownfile list (
.github/**,scripts/**) → refused, full suite runs.pathslist is byte-identical to both of ci.yml'spaths-ignorelists; the posted contexts are byte-identical to ci.yml'sciand
vrt-strictjob names (U+2014 em-dash asserted explicitly); the bypassdeclares no job named after a required context and no
name: CI; thestatus-posting step is guarded on
steps.decide.outputs.post == 'true'; andci.yml still declares
paths-ignore(if that optimisation is ever removed,this workflow is dead weight and the test says so).
node scripts/docs-only-gate.mjs decideinvocations assert the exact
$GITHUB_OUTPUTthe workflow reads, includingthe
JSON.parse(process.env.CONTEXTS)round-trip back toREQUIRED_CONTEXTS.(The second commit moved the contexts from a dynamic
import()insidegithub-scriptto a step output:AsyncFunctionbodies have no reliablemodule referrer, and that was the one path the live run could not exercise
because the posting step is correctly guarded off. It is now covered by test
rather than by hope.)
attacker-controllable, so: no
${{ }}interpolation ofsteps.*/github.event.*in anyrun:orscript:body (asserted over all fourrun:bodies); the reason is flattened on
\rand\nso a crafted path cannotinject a second
post=trueline into$GITHUB_OUTPUT; and the changed-fileheredoc delimiter is randomised so a path containing a newline cannot truncate
the file list. Each has its own test.
CI wall-time delta
+0 s on the critical path.
ci.ymlis untouched, so code PRs are byte-for-byteunchanged. The new workflow is a separate job that fires only on PRs touching a
doc path — measured at 12 s on this PR — and nothing depends on it. The unit
test adds ~0.1 s to the
unitlane. Docs-only PRs go from never mergeable to~1 min (the G2 poll's absence window) instead of ~25 min of full suite.
Verification
flox activate -- task typecheck— 0 errors (3108 files).actionlint 1.7.12 -shellcheck=— clean, whole.github/workflows/tree.REPEAT=3 task test:one PKG=scripts -- docs-only-gate— 3× green, 60 tests.scripts/suite — green (ledger + reconciliation unaffected; they walkpackages/,e2e/,art/, notscripts/).webgl 0effdd16…,collab c2324ef7…,grand f0417877…each still resolve to their committedci-*-attest/<hash>.json; all three attest gates passed on CI. Neither basiscovers
.github/**orscripts/*.mjs.contract-lock.txtuntouched (empty diff).Known, pre-existing, unchanged
**/*.md/.myrobots/**/LICENSEfeed nothing in the build or the testsuites — every reference in source is a comment (verified by grep). The one
exception is
docs/testing/test-ledger.generated.md, a GENERATED golden assertedby
scripts/test-ledger.test.ts: hand-editing it alone is ungated. That is apre-existing consequence of ci.yml's
paths-ignore, identical before and afterthis PR, and the next code PR's
unitlane catches the staleness. Closing itwould require
paths-ignoreto express a negation, which GitHub does notsupport.
Follow-up once this is on main
#1184 should become mergeable: close+reopening it re-fires
pull_request, thebypass runs, G1 sees one
.md, G2 finds noci.ymlrun, and both contexts gogreen — satisfying the armed auto-merge.
🤖 Generated with Claude Code