Repository navigation
ci: make required checks impossible to satisfy with an echo - #561
Merged
Merged
Conversation
ci-noop.yml existed because a workflow skipped by a trigger path filter never creates its check runs, which leaves every required status check Pending and blocks the merge. It fired on the inverse path set and published job names identical to ci.yml's real jobs as trivial echo steps. GitHub runs a paths-ignore workflow whenever any changed file falls outside the ignore set, so a pull request touching both code and docs fired both workflows and the echo reported success on every required context in two to four seconds. This repository's visual proof rule puts screenshots under docs/ in the same commit as the code they prove, so that mixed shape is the common case, not an edge case. Path filtering now lives inside ci.yml. A leading changes job lists the changed files and every other job is gated on its single boolean output. A job skipped by a conditional reports Success to branch protection, so a docs-only change still merges without a second workflow standing in for the real one, and each required check name now has exactly one producer. The changes job is fail-safe by construction: it holds an allow-list of documentation paths and treats anything else, including an unrecognized path, an API error, or an event that carries no diff, as a reason to run the full suite. Also removes the web-e2e-shim job, which was the last remaining pair of jobs sharing a check name, and adds .github/ci/lint-workflow-check-names.mjs to the repo policy lints so a duplicate check name or a required context with no producer fails the build instead of silently weakening the gate. Required status check names and branch protection are unchanged. Closes #553
|
Warning Review limit reached
Next review available in: 30 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe CI workflow now computes change scope internally, preserves required check conclusions for docs-only changes, removes the no-op workflow workaround, and adds a linter plus policy documentation for validating required-check producers and branch-protection configuration. ChangesCI Required Check Integrity
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant PullRequest
participant changes
participant CIJobs
PullRequest->>changes: changed file list
changes->>CIJobs: run output
CIJobs->>CIJobs: conditionally execute steps or jobs
CIJobs-->>PullRequest: required check conclusions
Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
The first pass gated the three required jobs with a job-level `if:`, which relies on GitHub treating a check run whose conclusion is `skipped` as satisfying a required context. That is what the documentation says, but it is the one property that cannot be verified on a pull request into a protected branch before the change is already merged, and guessing wrong deadlocks every documentation-only pull request. The required jobs now carry `if: always()` and gate their individual steps instead. On a documentation-only change they run, execute nothing, and conclude success in a few seconds, which is unambiguous regardless of how branch protection treats a skipped check. Jobs that are not required keep the simpler job-level condition, because a skipped non-required check cannot block anything. The step condition is `!= 'false'` rather than `== 'true'` so a detector job that failed or was cancelled leaves an empty output and the real suite runs anyway. The merge-gate guard now also enforces this shape: a job that publishes a required context must use `if: always()`.
This was referenced Jul 27, 2026
sakibsadmanshajib
marked this pull request as ready for review
July 27, 2026 20:46
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
sakibsadmanshajib
added a commit
that referenced
this pull request
Jul 28, 2026
…cleanly (#570) ## Problem The OpenWolf hooks in `.wolf/hooks/` rewrite `.wolf/` state on every tool call, and five of those files were tracked by git. Observed today, not hypothetical: - `.wolf/buglog.json` conflicted across five separate pull requests in one day, because every agent appended to the same tracked JSON array. - A `git pull --ff-only` on the shared checkout was blocked three consecutive times by hook churn that regenerated between commands, and had to be stashed to proceed. - Parallel agents in separate worktrees each produced a competing version of the same telemetry. ### The mechanism, worth writing down This is not obvious from reading either the hook sources or the settings file, and it explains why the pull blocker landed on the shared checkout rather than on the individual worktrees. The hooks are invoked as `node "$CLAUDE_PROJECT_DIR/.wolf/hooks/<hook>.js"`, and `getWolfDir()` in `shared.js` resolves its target from the same variable. For an agent running in a git worktree, `CLAUDE_PROJECT_DIR` points at the **shared checkout**, not at the worktree. So every parallel agent, whichever worktree it is working in, writes its telemetry into `/home/sakib/hive/.wolf/`. Measured while doing this work. The worktree was created at 16:33:34, and with the agent working only inside it, the shared checkout's telemetry kept advancing: ``` $ stat -c '%y %n' /home/sakib/hive/.wolf/anatomy.md /home/sakib/hive/.wolf/memory.md 2026-07-27 16:37:51 /home/sakib/hive/.wolf/anatomy.md 2026-07-27 16:37:51 /home/sakib/hive/.wolf/memory.md ``` That is a Claude Code harness characteristic rather than a repository bug, so there is nothing to fix in code. Untracking the telemetry makes it harmless, because the shared checkout stops going dirty. Recorded here and in #571 because the symptom, a shared checkout that will not fast-forward while nobody is editing it, is genuinely confusing without knowing the cause. ## The invariant No file that a hook writes on every tool call may be tracked by git. Files that a person or an agent updates deliberately stay tracked and reviewable. | File | Before | After | Why | |---|---|---|---| | `.wolf/cerebrum.md` | tracked | tracked | deliberately edited | | `.wolf/decisions.md` | tracked | tracked | deliberately appended ledger | | `.wolf/buglog.jsonl` | did not exist | **tracked** | deliberately appended bug memory | | `.wolf/GOAL.md`, `fleet.json`, `cost-ledger.md`, `hooks/*.js` | tracked | tracked | no hook writes them | | `.wolf/anatomy.md` | tracked | **untracked** | rewritten per write | | `.wolf/memory.md` | tracked | **untracked** | per-session table of tool actions | | `.wolf/hooks/_session.json` | tracked | **untracked** | pure session state | | `.wolf/buglog.json` | tracked | **untracked** | now a generated aggregate | | `.wolf/token-ledger.json` | already ignored | untracked | unchanged | The local files are all left in place. `git rm --cached` only removed them from the index; they are live state. ## The anatomy.md judgement call **Untracked and regenerated on demand, not throttled.** Reasoning: 1. It is derived entirely from the working tree, so it carries no information the repository does not already have. It is a cache, and caches do not belong in git. 2. "Regenerate only when content materially changes" cannot stop the churn here. `serializeAnatomy` stamps a fresh `Last scanned` ISO timestamp on every call, and `post-write.js` recomputes the token estimate for the file just edited. A real source edit therefore *does* change the content materially, so a change-detection gate would only suppress no-op rewrites while leaving a diff in every pull request that touches any file. 3. It is 150 KB of machine-generated inventory that no reviewer reads. 4. Every parallel worktree produces a different, equally correct version, so there is no single value that could be the tracked one. Nothing is lost: each checkout keeps its own local copy, the `pre-read.js` consult path reads that local copy exactly as before (see the `OpenWolf anatomy:` hit lines in the verification below), and it regenerates with `openwolf scan`. ## The buglog contention fix `.wolf/buglog.jsonl`, one JSON object per line, append only, declared `merge=union` in a new `.gitattributes`. Five branches appending to one JSON array is a structural conflict generator: every append touches the array's closing bracket and the comma before it. JSON Lines has no shared terminator, and `union` is a git built-in so there is no per-clone `git config` step to forget. Proven against the exact two-branch append case with `git merge-file --union`: ``` JSONL union merge: exit 0 (no conflict) JSONL merged lines: 3 JSONL every line parses: true JSONL ids after merge: bug-base,bug-one,bug-two ARRAY union merge: exit 0 ARRAY result valid JSON: false — Expected ',' or ']' after array element in JSON at position 136 ``` Note the second half: a union merge of the old array shape exits 0 and produces **invalid JSON**, silently. That is why a merge driver on the existing format was not an option. `.wolf/buglog.json` survives as a generated projection of the JSONL in the `{ version, bugs }` shape the openwolf CLI and dashboard expect, so `openwolf bug search` keeps working unchanged. The session-start hook rebuilds it; `node .wolf/hooks/bugstore.js sync` rebuilds it by hand outside a session. Anything `openwolf bug add` writes straight into the aggregate is absorbed into the JSONL on the next rebuild instead of being silently dropped. **Hook guesses no longer touch the tracked store.** `autoDetectBugFix` in `post-write.js` fabricates a bug entry from a regex over the diff, and it was the real churn source: 107 of the 135 entries were `auto-detected`, with content like `inline fix` and `added error handling`. Those now go to the generated aggregate only. They stay searchable for the rest of the session and are dropped on the next rebuild. Deliberate entries, the ones `.claude/rules/openwolf.md` asks for with `error_message`, `root_cause`, `fix`, `tags`, are the only thing that reaches git. Bug ids are now collision free. The old `bug-${count + 1}` scheme derived the id from a count of the store, so parallel agents minted the same number; the migrated log already carries three such collisions (`bug-003`, `bug-004`, `bug-005`) plus eight entries with no id at all. ### Migration is lossless All 135 entries carried over. Evidence, generated from the pristine pre-change blob out of git rather than from the working copy: ``` BEFORE entries: 135 | distinct ids: 125 AFTER entries: 135 | distinct ids: 125 id multiset identical: true deep-equal roundtrip: true auto-detected carried over: 107 deliberate carried over: 28 LOSSLESS OK ``` `distinct ids: 125` against `135` entries is the pre-existing id collision, preserved rather than papered over: renumbering would have failed an honest before-and-after id comparison. ### `openwolf bug search` still works `openwolf` 2.0.1 is installed on this box. After `node .wolf/hooks/bugstore.js sync` reported `buglog: 135 entries`: ``` $ openwolf bug search caddy Found 6 matching bug(s): [console-caddy-host-mismatch-empty-200] ... [console-redirect-origin-leaks-bind-address] ... [caddy-artifacts-auto-https-redirect-loop] ... ... ``` ## Proof that the churn stops `churn-probe.sh` drives the real hooks the way Claude Code drives them, feeding each one the same stdin payload shape: `session-start`, then `pre-read` + `post-read` + `post-write` for four repository files, then `stop`. Run against a clean tree at this commit: ``` --- session-start --- pre-read + post-read README.md 📋 OpenWolf anatomy: README.md — Project documentation (~2686 tok) --- post-write (Edit) README.md --- pre-read + post-read Makefile 📋 OpenWolf anatomy: Makefile — (~259 tok) --- post-write (Edit) Makefile --- pre-read + post-read go.work 📋 OpenWolf anatomy: go.work — (~50 tok) --- post-write (Edit) go.work --- pre-read + post-read .gitignore 📋 OpenWolf anatomy: .gitignore — Git ignore rules (~790 tok) --- post-write (Edit) .gitignore --- stop === git status --porcelain .wolf/ === === end (empty above means no telemetry churn) === ``` Empty. The `OpenWolf anatomy:` hit lines confirm the hooks actually ran rather than no-opping, and the writes are independently confirmed by mtimes plus counters: ``` 2026-07-27 16:48:15 150784 .wolf/anatomy.md 2026-07-27 16:48:15 624268 .wolf/memory.md 2026-07-27 16:48:15 1761 .wolf/hooks/_session.json 2026-07-27 16:48:15 99574 .wolf/buglog.json 2026-07-27 16:46:47 80821 .wolf/buglog.jsonl <- not touched aggregate entries: 139 tracked jsonl lines: 135 (unchanged by the probe) session-only auto guesses added by probe: 4 ``` All five telemetry files were freshly written, four hook guesses were recorded into the aggregate, the tracked JSONL stayed at 135 lines, and git saw nothing. ## Store self-check `node .wolf/hooks/bugstore.selfcheck.js` prints `bugstore selfcheck OK`. No framework, no fixtures, `node:assert` against a throwaway directory. It asserts append-only behaviour (the first line is byte-identical after a second append), aggregate fidelity, pickup of a line a union merge left behind, absorption of a CLI-added entry with idempotent re-sync, exclusion of hook guesses from the JSONL on both write and rebuild, and id uniqueness across 500 mints. ## Documentation corrected `.claude/rules/openwolf.md` previously told every agent, on line 3, to *"update .wolf/anatomy.md + append .wolf/memory.md"* after writing files. That is both the hooks' job and the instruction that kept recreating this problem. It now records which files are tracked, which are hook-owned telemetry that must never be committed or force-added, and how to read and append bug memory. `CLAUDE.md` carries the same convention in short form under the OpenWolf section. ## Decision ledger Recorded as **D-025**, not D-024: `D-024` was already taken on `origin/main` at `425ac853` by the origin.ts ruling, so the next free id was used. > D-025 | .wolf/ splits by writer: anything a hook rewrites on every tool call is gitignored (anatomy.md, memory.md, token-ledger.json, hooks/_session.json, buglog.json), only deliberately curated files stay tracked (cerebrum.md, decisions.md, buglog.jsonl, GOAL.md, fleet.json, cost-ledger.md, hooks/\*.js). Bug memory becomes append-only .wolf/buglog.jsonl with merge=union in .gitattributes, so parallel branches merge appends automatically; buglog.json survives only as a generated aggregate so `openwolf bug search` keeps working. anatomy.md is untracked rather than throttled because it is fully derivable from the working tree and its per-call rewrite stamps a fresh timestamp, so no "only when materially changed" gate can stop the churn | owner 2026-07-27; branch chore/wolf-telemetry-untrack | 2026-07-27 ## Owner action needed outside this pull request None. The hooks that caused this are configured in the repository's own `.claude/settings.json`, pointing at `$CLAUDE_PROJECT_DIR/.wolf/hooks/*.js`, so the fix ships in-repo and needs no change to the owner's machine config. Two findings worth a look, both out of scope here: 1. `CLAUDE.md` opens with `@.wolf/OPENWOLF.md`, but that file does not exist and is not tracked. The import is dangling. Writing its contents would be inventing project doctrine, so it is left alone. 2. `/home/sakib/hive/.wolf/token-ledger.json` has grown to 44 MB. It is already gitignored so there is no git impact, but it grows every session. ## Scope Touches only `.wolf/**`, `.gitignore`, `.gitattributes`, `CLAUDE.md`, and `.claude/rules/openwolf.md`. No `apps/**`, `.github/workflows/**`, `deploy/**`, `scripts/**`, or `.env.example`. No UI or UX surface is involved, so the visual-proof directive does not apply; the equivalent evidence for a git-hygiene change is the `git status` and merge output above. ## On CI The required checks on this pull request report green in 1 to 4 seconds, including "Web E2E (full stack)" in 2 seconds. Those are the `ci-noop.yml` placeholders from issue #553, not real runs, and they are not being cited as evidence of anything. The skip path is the legitimate one for this branch regardless: it changes no Go, no TypeScript, no workflow, and no deployment file, so there is genuinely nothing for the Go or web suites to exercise. The verification that matters here was run locally and is quoted in full above: the hook probe against a clean tree, `git merge-file --union` on the two-branch append case, the lossless migration comparison, `openwolf bug search`, and `node .wolf/hooks/bugstore.selfcheck.js`. PR #561 deletes `ci-noop.yml` in parallel. ## Review follow-up Three CodeRabbit findings, all genuine, fixed in `2a9f555e` and replied to in thread: 1. **Deduplicate by full relative path, not basename.** `apps/web-console/lib/index.ts` and `apps/agent-console/lib/index.ts` were collapsed into one file, suppressing a valid detection. Probe against the real hook: aggregate goes 135 to 136 to 137 for two same-basename files in different directories, and a genuine repeat of one path stays suppressed at 137, while the tracked JSONL holds at 135. 2. **Preserve volatile guesses across a durable append.** `appendBugs` rebuilt the aggregate from the JSONL alone, so the first deliberate entry in a session discarded every hook guess recorded before it. Guesses are now carried over, and `syncBugAggregate` rewrites explicitly from the JSONL so session-start remains the only path that clears them. Covered by a new self-check case. 3. **Base the durable-log warning on the tracked entry count.** I implemented the intent but not the proposed patch, and argued it in the thread: `post-write.js` exits before the session tracker for every `.wolf/` path, so matching `buglog.jsonl` in `files_written` would have been permanently false and would have fired the warning on every three-edit session. The stop hook now compares `readBugs(wolfDir).length` against a `buglog_entries_at_start` baseline, which reads the tracked JSONL only. Verified both ways: it warns with four hook guesses and zero durable entries, and falls silent once one real entry is appended. The dangling `@.wolf/OPENWOLF.md` import and the two other findings from this work are filed as #571 for the owner to decide. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Introduced an append-only JSONL bug memory with a generated aggregate, designed for safe union-style merging; the aggregate is now synchronized at session start. * Added a bugstore CLI to add and sync bug entries, plus a self-check tool to validate invariants. * Auto-detected recurring issues are now recorded as volatile entries without rewriting durable history. * **Documentation** * Updated OpenWolf protocol/setup guidance to clearly define what’s tracked vs generated/ignored and the updated bug-log workflows and invariants. * **Chores** * Removed old durable aggregate and stored per-session telemetry content. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # .wolf/buglog.json
Review follow-up on the merge-gate integrity work.
The guard collapsed every `${{ }}` expression in a job name to a wildcard and
only failed when a required context had zero producers. Both halves of that
were reproducible holes. A second workflow whose jobs were named literally
"Go tests (edge-api)" and friends never collided with the template string
"Go tests (${{ matrix.module }})", so issue #553 could be reintroduced for
four of the six required contexts with the guard still exiting 0. Dropping a
leg from the matrix left the corresponding required context with a producer
that only appeared to exist, so every pull request would have blocked forever
on a check that could not arrive, which is precisely what the guard's own
header claims to prevent.
Job names are now expanded against the real `strategy.matrix` values, so a
templated name is compared as the concrete set of names it publishes.
A required context with more than one producer is an error, not just zero
producers. A job name built from anything the guard cannot resolve, such as
`github.*` or `env.*`, is itself an error rather than a silent wildcard.
The guard also never looked at trigger path filters, so promoting a
path-filtered workflow to a required context would have reproduced the
original deadlock with the guard green. It now fails when a workflow
publishing a required context carries `paths:` or `paths-ignore:` on any
trigger. The two workflows that legitimately path-filter, license-gate.yml
and agent-engine-sif.yml, publish no required context and stay green.
In ci.yml, the docs-only allow-list granted whole directories, which cannot
stay inert as those directories grow: `.wolf/` and `.claude/` already hold
hook scripts. A leading deny arm now runs the full suite for any changed
path with an executable extension regardless of directory, which needs no
maintenance and still lets genuinely inert paths skip. The changed-file
lookup asks for 100 entries per page and runs everything when the list
reaches the API cap, since a truncated list could otherwise hide a code path
behind a docs-only prefix.
MERGE-POLICY.md now describes what the guard actually enforces, and no
longer asserts that a job skipped by a conditional reports Success, which is
the claim this implementation deliberately refuses to depend on. It also
records that a docs-only pull request legitimately turns all six required
checks green in seconds, so duration is not evidence either way and readers
should verify one producer per context instead.
A pull request opened as a draft and then marked ready for review with no further pushes fires none of the activity types ci.yml listened for, so the workflow never ran and none of the required contexts were published. With ci-noop.yml deleted, nothing else publishes those names, so the pull request would sit unmergeable with nothing visibly failing. Add ready_for_review to the pull_request trigger, and extend the merge-gate integrity guard so the same gap cannot come back: any workflow publishing a required context must list opened, synchronize, reopened and ready_for_review under on.pull_request.types.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
441-442: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
persist-credentials: falseto checkout in required-check jobs.Static analysis flags both
repo-policy-lints(441-442) andweb-unit(516-517) checkouts for credential persistence (artipacked). Both jobs runnpm ci/npm scripts afterward; a compromised dependency's install/postinstall script could read the persistedGITHUB_TOKENfrom the git config. Since neither job needs to push or use the token post-checkout, disabling persistence closes that surface.🔒️ Proposed fix
- uses: actions/checkout@v4 if: needs.changes.outputs.run != 'false' + with: + persist-credentials: falseApply the same
with:block to both checkout steps.Also applies to: 516-517
🤖 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/ci.yml around lines 441 - 442, Update the checkout steps in the repo-policy-lints and web-unit jobs to set persist-credentials to false via the checkout action’s with configuration, while preserving their existing conditions and subsequent job steps.Source: Linters/SAST tools
🤖 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/ci.yml:
- Around line 163-183: Update the extension deny-list in the changed-files loop
to include both *.jsx and *.tsx alongside the existing executable-code patterns,
ensuring these files call run_everything before any allow-listed directory rule
can continue.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 441-442: Update the checkout steps in the repo-policy-lints and
web-unit jobs to set persist-credentials to false via the checkout action’s with
configuration, while preserving their existing conditions and subsequent job
steps.
🪄 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: e0970315-9ffa-4f2d-ada6-c1e177bf01f1
📒 Files selected for processing (5)
.github/MERGE-POLICY.md.github/ci/lint-workflow-check-names.mjs.github/workflows/ci-noop.yml.github/workflows/ci.yml.wolf/buglog.jsonl
💤 Files with no reviewable changes (1)
- .github/workflows/ci-noop.yml
GOAL.md still claimed staging was live on cp-hive.scubed.co, a hostname that was decommissioned and now returns NXDOMAIN. Point the line at control-hive instead and say what it checks. Add D-026 so the remaining cp-hive strings on main are not mistaken for stale leftovers: the RETIRED_HOST guard, the deploy/cloudflare README section and the buglog entry exist to stop the hostname coming back.
The docs-only path classifier denies by extension before it grants by directory, but the deny arm listed only .js, .mjs, .cjs, .ts, .sh and .py. A .tsx or .jsx file added under any allow-listed prefix (docs/, .wolf/, .claude/, .vscode/, .cursor/) fell through to the directory grant and was classified as docs-only, which skipped every real suite. The repo ships a Next.js web console and an agent console, so .tsx is a live risk rather than a hypothetical one. Add .tsx and .jsx, plus .mts and .cts for the same reason .mjs and .cjs were already there. The comment now states the ordering invariant, that the deny arm must stay ahead of the directory grants because every arm below it ends in continue, and it stops duplicating the extension list so there is only one copy to maintain. Verified by extracting the case block verbatim from the workflow and running it against representative file lists. Before this change a list of docs/agent/README.md, docs/agent/ExampleWidget.tsx and .wolf/decisions.md returned SKIP. After it returns RUN because docs/agent/ExampleWidget.tsx is executable code. The same flip holds for .jsx, .mts and .cts. A purely inert list still returns SKIP, .claude/hooks/foo.mjs still returns RUN, and apps/edge-api/main.go still returns RUN as outside the allow list.
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #553.
The defect
ci.ymlfiltered on the trigger (paths: [apps/**, packages/**, deploy/**, supabase/**, go.work*, .github/workflows/ci.yml]). GitHub skips a whole workflow when a trigger path filter does not match, and a skipped workflow never creates its check runs, so every required status check stayed Pending and blocked the merge.The workaround was
ci-noop.yml: a second workflow, also namedCI, firing on the inversepaths-ignoreset, publishing job names that matchedci.yml's exactly (including the matrix-expandedGo tests (<module>)names) asrun: echo "Skipped (docs-only change)".GitHub runs a
paths-ignoreworkflow whenever any changed file falls outside the ignore set. It does not care whether the same pull request also touches real code. So a pull request touching both code and docs fired both workflows, and each required context received two check runs: one real, one a two-second echo. Either satisfies branch protection. This repository's visual-proof rule commits screenshots underdocs/in the same commit as the code they prove, so that mixed shape is the common case, not an edge case.This is not theoretical. PR #559, merged onto
maintwenty minutes before this branch was cut, touchedapps/web-console/e2e/**plus.wolf/**. Both workflows fired on commitd05b7ba1:ci.ymlrun 30301329356ci-noop.ymlrun 30301329536Go tests (agent-engine)Go tests (control-plane)Go tests (edge-api)Go tests (storage)Repo policy lints (tenant + audit)Web console (type + unit + build)Both runs are literally named
CI; only thepathfield of the run object distinguishes them. The echo run reported success on every required context in under five seconds. That is whygh pr checkshad stopped being a usable merge signal.Approach
Path filtering moves off the trigger and into the workflow, so
ci.ymlbecomes the only producer of every required check name.paths:orpaths-ignore:on any trigger. The workflow always starts, so it can never leave a required check Pending.changesjob asks the GitHub API for the same changed-file list GitHub itself would have filtered on (pulls/{n}/filesfor pull requests,compare/{before}...{after}for pushes) and emits one boolean.changesholds an allow-list of paths that cannot affect any build, test or lint (*.md,LICENSE,NOTICE,docs/,.wolf/,.claude/,.vscode/,.cursor/). Anything else means run, including an unrecognized path, an API error, or an event that carries no diff such asscheduleandworkflow_dispatch. A false "run" costs CI minutes; a false "skip" would defeat the gate, so the default is never "skip".go-tests,repo-policy-lints,web-unit) carryif: always()and gate their individual steps. They always run to completion and always report their own conclusion. On a docs-only change they execute nothing and concludesuccessin a few seconds.if:and are simply skipped, because a skipped non-required check cannot block anything.!= 'false', not== 'true', so achangesjob that failed or was cancelled leaves an empty output and the real suite runs anyway.web-e2e-shimis deleted. It duplicated theWeb E2E (full stack)name held byweb-e2e, which was the last remaining pair of jobs sharing a check name, and it was dead weight anyway: that context is not inrequired_status_checks.contexts..github/ci/lint-workflow-check-names.mjsruns inside theRepo policy lints (tenant + audit)required check and fails the build on (1) two pull-request jobs publishing the same check name, (2) a required context in.github/branch-protection-main.jsonthat no job publishes, (3) a job publishing a required context withoutif: always(). This is the regression guard: reintroducing anything shaped likeci-noop.ymlnow fails CI.There is no longer any ordering dependence. Two workflows cannot both report a required name because only one workflow declares those names, and a re-run cannot rescue a failure because the same job either does the work or does not, decided by a single boolean.
Rejected alternatives
Keep two workflows, make the path sets provably disjoint (the "smallest fix" sketched in #553, for example an explicit docs allow-list on
ci-noop.yml). Rejected: it leaves two producers for every required check name, so correctness rests on the two globs staying complementary forever. Any future path added to one set and not the other silently reopens the hole, and nothing fails when it does.dorny/paths-filterfor the detector. Rejected:gh apiplus acasestatement is about twenty lines, needs no checkout, has no merge-base or clone-depth pitfalls, and adds no third-party action to the one code path that guards every merge.Job-level
if:on the required jobs, relying on a skipped job satisfying the gate. This is what GitHub documents ("A job is skipped by a conditional" gives "The job reports Success", troubleshooting required status checks) and it was the first version of this branch. Rejected after building it: whether branch protection accepts askippedconclusion for a required context cannot be verified on a pull request into a protected branch until the change is already onmain, community reports contradict the docs, and this repository's ownweb-e2e-shimcomment asserted the opposite. Guessing wrong deadlocks every docs-only pull request.if: always()plus step gating gives a job that concludessuccesson its own merits, which is unambiguous on every GitHub version. The guard now enforces that shape.Merge queue (
merge_group). Not adopted. This repository does not use a merge queue, andstrictisfalsedeliberately. Adding one would be a separate change; note for later that any workflow feeding a merge queue must also list themerge_grouptrigger or the queue deadlocks.Drop path filtering entirely and always run the full suite. Simplest of all and fully correct, but it costs roughly four minutes of runner time on every documentation-only change. Kept the skip because the requirement was explicitly that a docs-only pull request go green without running the full suite.
Verification
Every case was exercised on real pull requests against this repository, not reasoned about.
A pull request into
mainmust carry.github/workflows/ci.ymlitself in order to be evaluated by the new logic, and a workflow file is not a docs-only path. Therun=falsebranch is therefore unreachable on a pull request intomainuntil this is merged. To close that gap without weakening the allow-list, a throwaway harness (probe/ci553-base) carried a workflow whosechangesjob was copied verbatim out of this branch'sci.yml(sha2560e81807ca11ca5408e24c1ff1f91689f60d5d0ebf4d2e70ba8fabda3b2163b3a, 76 lines) plus one job with the exact shape of a required job (if: always()and per-step gating). Pull requests into that base have clean single-purpose diffs, so both verdicts are reachable against the real decision logic.Case 1 — documentation only
PR #567, run 30303490144, diff =
docs/verification/ci-553-harness.md.Detect changed pathsverdict: SKIP — every changed path is docs-only,run=falsePROBE required-shaped jobskipped; the job itself concludedsuccessThe required-shaped job is not a skipped job. It ran, executed nothing, and reported success, which is what makes the gate independent of skipped-check semantics.
Case 2 — code only
PR #563, run 30303342319, head
32077382.Detect changed pathsrun=true)Go tests (agent-engine)Go tests (control-plane)Go tests (edge-api)Go tests (storage)Repo policy lints (tenant + audit)Web console (type + unit + build)Case 3 — code plus documentation, the case that was broken
PR #564, run 30303353621, head
98f44d76, diff includesdocs/verification/ci-553-probe.mdalongside code.Detect changed pathsrun=true)Go tests (agent-engine)Go tests (control-plane)Go tests (edge-api)Go tests (storage)Repo policy lints (tenant + audit)Web console (type + unit + build)Exactly one
CIrun per commit, where the same shape previously produced two. The docs half of the diff no longer buys the code half a two-second pass.The harness control for this case is PR #568, run 30303499232: diff =
apps/edge-api/docs/swagger.goplusdocs/verification/ci-553-harness.md, detector 2s,verdict: RUN the real suite — 'apps/edge-api/docs/swagger.go' is outside the docs-only allow-list, and the required-shaped job executed its real step (all stepssuccess, 7s). Same workflow, same commit shape, opposite verdict from Case 1, decided only by the file list.Case 4 — mixed pull request whose code deliberately fails
PR #565, run 30303363079, head
6f628256. At.Fataltest was added underapps/edge-api/docs/alongside a documentation change.Go tests (edge-api)Go tests (agent-engine)Go tests (control-plane)Go tests (storage)Repo policy lints (tenant + audit)Web console (type + unit + build)GET /commits/6f628256/check-runsreturns exactly one check run per required context, all from run30303363079(.github/workflows/ci.yml). There is no second producer to rescue the failure, and re-running the workflow re-runs the same failing job.mergeStateStatusfor #565 isBLOCKED. For contrast, #561 and #563 with no failing required check reportUNSTABLE, soBLOCKEDhere is the merge gate refusing the pull request, not an artifact of draft status.Case 5 — this pull request
This is itself a mixed pull request:
.github/workflows/**and.github/ci/**alongside.github/MERGE-POLICY.mdand.wolf/buglog.json. Two of those four paths are on the docs allow-list, and the verdict is stillrun=truebecause of the other two, which is the whole point of the polarity.07f08280run=trueac6c37fa(head)run=trueThe
Repo policy lintsleg includes the new merge-gate guard, so the guard is green against the tree it guards.Guard, both directions, locally
Merge-gate integrity OK: 15 check names across 5 pull-request workflows, 6 required contexts each with exactly one producer.ci-noop.ymlrestored as a second workflow: fails, naming all four duplicated check names and both producing jobs.web-unitreverted to a plain job-levelif:: fails withA required job must use if: always() and gate its steps.Branch protection
No change is required. All six required check names are unchanged, and
.github/branch-protection-main.jsonis untouched:Confirmed against the live gate (
gh api repos/sakibsadmanshajib/hive/branches/main/protection) before and after: same six contexts, sameapp_id15368,strict: false,enforce_admins: true,required_conversation_resolution: true. Nothing to apply.One thing worth knowing for later:
Web E2E (full stack)is not in the live required list, which is why deletingweb-e2e-shimcannot deadlock anything. If it is ever promoted to required,web-e2eneeds the sameif: always()plus step-gating treatment the other required jobs now have, and the guard will say so.Cleanup
All throwaway probe branches and pull requests (#562, #563, #564, #565, #567, #568,
probe/ci553-*) are closed and deleted.docs/verification/ci-553-*.md,apps/edge-api/docs/ci553_probe_test.goand the harness workflow existed only on those branches and never touched this one.Files
.github/workflows/ci.yml— triggers unfiltered,changesjob added, required jobsif: always()with gated steps, non-required jobs job-level gated,web-e2e-shimremoved..github/workflows/ci-noop.yml— deleted..github/ci/lint-workflow-check-names.mjs— new merge-gate integrity guard..github/MERGE-POLICY.md— documents the in-workflow filter, why a trigger filter cannot be used here, and the one-producer-per-check-name rule..wolf/buglog.json— appended entryci-noop-echo-satisfies-required-checks.Summary by CodeRabbit
CI & Reliability
Documentation
Validation