Skip to content

Commit 8c6798b

Browse files
authored
ci: a branch nobody named got no checks at all (#512)
* ci: fast structural checks on every branch * ci: close the branch-trigger gap * ci: scope the JSON check — JSONC configs are not a defect * ci: scope the marker check to code — prose that documents a conflict is not one * ci: the JSON check reached a template's editor settings
1 parent b7916e9 commit 8c6798b

2 files changed

Lines changed: 141 additions & 1 deletion

File tree

.github/workflows/code-quality.yml

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,36 @@ on:
2727
# `enable-coverage-guard` was switched on here in the previous commit; without
2828
# this trigger its push-side half would have been dead on arrival.
2929
push:
30-
branches: [main, beta, development, feature/**, bugfix/**, hotfix/**]
30+
# An ALLOW-LIST of branch prefixes is a gate with a hole in it, and the
31+
# hole is SILENT: a branch matching nothing gets no CI at all, and its last
32+
# visible status is whatever it inherited — indistinguishable, on every
33+
# dashboard, from a branch that passed.
34+
#
35+
# Two live examples, both found 2026-08-14: `perf/**` was uncovered in
36+
# openconnector, where a merge carrying unresolved conflict markers and 84
37+
# failing tests was pushed and nothing ran; and `feat/**` was uncovered in
38+
# openregister — note the list said `feature/**`, so every branch anyone
39+
# named `feat/...` had been running unchecked.
40+
#
41+
# Prefixes are added rather than replaced with `**` because this workflow is
42+
# expensive (PHPUnit matrix, Newman, Playwright). The fast structural checks
43+
# DO run on `**` — see merge-hygiene.yml, added in the same change.
44+
#
45+
# ⚠️ Adding prefixes is not the durable fix; the next invented one is
46+
# uncovered again. The durable fix is branch protection requiring a PR into
47+
# development, which the pull_request trigger below already gates correctly.
48+
branches:
49+
- main
50+
- beta
51+
- development
52+
- feature/**
53+
- feat/**
54+
- bugfix/**
55+
- hotfix/**
56+
- perf/**
57+
- refactor/**
58+
- chore/**
59+
- fix/**
3160
pull_request:
3261
branches: [main, beta, development]
3362
# Same family of defect as the missing `push:` above, one step further along:
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
name: Merge Hygiene
2+
3+
# WHY THIS EXISTS, and why it is separate from Code Quality.
4+
#
5+
# On 2026-08-14 a merge of origin/development was committed and PUSHED to
6+
# `perf/predicted-page-fanout` with UNRESOLVED CONFLICT MARKERS in two files.
7+
# `lib/Service/SynchronizationService.php` did not parse. Eighty-four tests were
8+
# red. Nothing stopped it, and nothing reported it — because Code Quality's push
9+
# trigger allows only `[main, development, feature/**, bugfix/**, hotfix/**]`,
10+
# and `perf/**` matches none of them. The branch had no CI at all, so its last
11+
# visible state was green from before the branch existed.
12+
#
13+
# The lesson is not "add perf/** to the list" — that fixes this branch and leaves
14+
# the next prefix uncovered. Any branch anyone pushes should get at least the
15+
# checks that take seconds, so this runs on `**` and stays deliberately cheap:
16+
# no matrix, no containers, no dependencies, no Playwright. It is a smoke alarm,
17+
# not the fire brigade. Code Quality remains the real gate on PRs.
18+
on:
19+
push:
20+
branches: ['**']
21+
pull_request:
22+
workflow_dispatch:
23+
24+
concurrency:
25+
group: merge-hygiene-${{ github.ref }}
26+
cancel-in-progress: true
27+
28+
permissions:
29+
contents: read
30+
31+
jobs:
32+
hygiene:
33+
name: Conflict markers and PHP syntax
34+
runs-on: ubuntu-latest
35+
steps:
36+
- uses: actions/checkout@v4
37+
38+
# Conflict markers, anywhere in the tree we author. A marker means a merge
39+
# was committed half-finished; every downstream signal from that commit is
40+
# meaningless, so this fails first and says so plainly.
41+
#
42+
# Anchored to line start: `<<<<<<<` inside a string, a diff fixture or a
43+
# docs example is legitimate and must not fail the build. Matching only at
44+
# column 0 is what git itself writes.
45+
- name: No unresolved conflict markers
46+
run: |
47+
set -euo pipefail
48+
# SCOPED TO CODE, and to paths we author. A marker is only a defect
49+
# where it would break something: prose that DOCUMENTS a conflict is
50+
# legitimate, and so are agent-eval artifacts that capture one as
51+
# sample output. openbuild failed this gate on
52+
# `.claude/skills/create-pr/evals/.../summary.md` — a correct file.
53+
#
54+
# That matters more than the miss it allows. A gate that fails on
55+
# correct files gets switched off, and takes the checks that were
56+
# working with it; a marker in a markdown file breaks nothing.
57+
if git grep -nE '^(<{7}|={7}|>{7})( |$)' -- \
58+
'*.php' '*.js' '*.mjs' '*.ts' '*.vue' '*.json' '*.yml' '*.yaml' '*.css' '*.scss' \
59+
':!vendor' ':!node_modules' ':!*.lock' ':!tests/fixtures' ':!.claude' \
60+
':!**/evals/**' ':!**/fixtures/**' > /tmp/markers.txt; then
61+
echo "::error::Unresolved merge conflict markers are committed. This branch does not build."
62+
cat /tmp/markers.txt
63+
exit 1
64+
fi
65+
echo "No conflict markers."
66+
67+
- uses: shivammathur/setup-php@v2
68+
with:
69+
php-version: '8.3'
70+
coverage: none
71+
72+
# Every PHP file parses. A conflict marker is caught above, but so is any
73+
# other way a file can be committed unparseable — and this is the check
74+
# that would have failed within seconds of the merge landing.
75+
- name: PHP syntax
76+
run: |
77+
set -euo pipefail
78+
fail=0
79+
while IFS= read -r f; do
80+
php -l "$f" > /dev/null 2>&1 || { echo "::error file=$f::PHP syntax error"; php -l "$f" || true; fail=1; }
81+
done < <(git ls-files '*.php' | grep -v '^vendor/' | grep -v '^tests/fixtures/')
82+
exit "$fail"
83+
84+
# JSON that will not parse breaks register fragments and app metadata,
85+
# and is the other thing a bad merge leaves behind.
86+
#
87+
# SCOPED TWICE, because each widening found another honest file. The
88+
# first version parsed every tracked .json and died on tsconfig/eslint
89+
# JSONC. The second still reached `lib/**/*.json`, which in openbuild
90+
# includes an entire app TEMPLATE — `.vscode/settings.json` and all.
91+
# A template is not this app's configuration, and an editor file is not
92+
# loaded by anything. What is left is what OpenRegister actually reads.
93+
#
94+
# SCOPED, because the first version was not and failed immediately on
95+
# honest files: editor and tooling configs (tsconfig, eslint, devcontainer)
96+
# are JSONC — comments and trailing commas — which is valid for their
97+
# consumers and invalid for a strict parser. A gate that fails on correct
98+
# files is worse than no gate: it gets switched off, and takes the checks
99+
# that were working with it. Only the JSON the app itself loads is checked.
100+
- name: JSON parses
101+
run: |
102+
set -euo pipefail
103+
fail=0
104+
while IFS= read -r f; do
105+
[ -f "$f" ] || continue
106+
python3 -c "import json,sys; json.load(open(sys.argv[1]))" "$f" \
107+
|| { echo "::error file=$f::invalid JSON"; fail=1; }
108+
done < <(git ls-files 'composer.json' 'package.json' 'appinfo/*.json' 'lib/Settings/**/*.json' \
109+
| grep -v '^vendor/' | grep -v '^node_modules/' \
110+
| grep -v '/\.vscode/' | grep -v '^lib/Resources/template/')
111+
exit "$fail"

0 commit comments

Comments
 (0)