Skip to content

Commit 1f9e5bc

Browse files
committed
Clean up CI bypassers exposed by PR #1318 merge
- Backend CI `changes` job: scope to pull_request events. dorny/paths-filter@v3 shells out to `git branch --show-current` on push events and fails without a checkout, so every push to main produced a red-X `changes` job silently masked by `continue-on-error: true` plus the fail-open `|| push` gate on downstream jobs. Downstream `if:` conditions rewritten with `always()` so the intentional skip on push no longer cascades. - Frontend CI: drop the leftover tippy-debug step in the lint job. - codecov-notify: sort `matching` by created_at desc before taking [0] instead of relying on undocumented API ordering. Closes #1319 follow-ups 1 and 2.
1 parent 297d9cb commit 1f9e5bc

4 files changed

Lines changed: 32 additions & 21 deletions

File tree

.github/workflows/backend.yml

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,13 @@ concurrency:
2525

2626
jobs:
2727
changes:
28+
# Path filtering only gates pull_request events; pushes to protected
29+
# branches always run downstream regardless of which files changed.
30+
# dorny/paths-filter@v3 additionally needs a checked-out repo on push
31+
# events (it shells out to `git branch --show-current`), so scoping
32+
# this job to pull_request avoids the "fatal: not a git repository"
33+
# red X that would otherwise appear on every push to main.
34+
if: github.event_name == 'pull_request'
2835
runs-on: ubuntu-latest
2936
# Fail open: if this job errors (e.g., transient GitHub API failure),
3037
# downstream jobs should still run rather than being silently skipped.
@@ -35,7 +42,8 @@ jobs:
3542
outputs:
3643
backend: ${{ steps.filter.outputs.backend }}
3744
steps:
38-
# No checkout needed: dorny/paths-filter@v3 fetches diffs via the GitHub API.
45+
# No checkout needed: dorny/paths-filter@v3 reads diffs from the
46+
# pull_request event payload.
3947
- uses: dorny/paths-filter@v3
4048
id: filter
4149
with:
@@ -55,12 +63,13 @@ jobs:
5563
5664
linter:
5765
needs: changes
58-
# Skipping via path filter only applies to PRs with no backend changes.
59-
# Push events to protected branches always run unconditionally.
60-
# Using `!= 'false'` rather than `== 'true'` so a transient `changes` failure
61-
# (outputs.backend is '' when the job errors under continue-on-error) still
62-
# runs downstream jobs — the actual "fail open" behavior.
63-
if: needs.changes.outputs.backend != 'false' || github.event_name == 'push'
66+
# Run on every push, and on PRs unless the filter explicitly reports
67+
# no backend changes ('false'). `always()` is required because the
68+
# `changes` job is skipped on push events (see its `if:` above), and
69+
# a skipped `needs` target would otherwise cascade-skip this job.
70+
# The `!= 'false'` form also preserves the fail-open behaviour on
71+
# PRs where `changes` errors transiently (outputs.backend is '').
72+
if: always() && (github.event_name == 'push' || needs.changes.outputs.backend != 'false')
6473
runs-on: ubuntu-latest
6574
permissions:
6675
contents: read
@@ -86,7 +95,7 @@ jobs:
8695
pytest:
8796
needs: [changes, linter]
8897
# Same fail-open semantics as the linter gate above.
89-
if: needs.changes.outputs.backend != 'false' || github.event_name == 'push'
98+
if: always() && needs.linter.result == 'success' && (github.event_name == 'push' || needs.changes.outputs.backend != 'false')
9099
runs-on: yuge
91100
timeout-minutes: 180
92101
permissions:

.github/workflows/codecov-notify.yml

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ jobs:
4545
"Frontend E2E Integration",
4646
];
4747
48-
// Fetch every workflow run attached to this SHA. The API returns
49-
// the most recent attempt per workflow first, which is the one we
50-
// care about after re-runs.
48+
// Fetch every workflow run attached to this SHA. Sort explicitly
49+
// by created_at desc so we always pick the most recent attempt
50+
// per workflow (after re-runs) rather than relying on undocumented
51+
// API ordering.
5152
const runs = await github.paginate(
5253
github.rest.actions.listWorkflowRunsForRepo,
5354
{
@@ -61,7 +62,13 @@ jobs:
6162
let allDone = true;
6263
const runIds = {};
6364
for (const name of expected) {
64-
const matching = runs.filter((r) => r.name === name);
65+
const matching = runs
66+
.filter((r) => r.name === name)
67+
.sort(
68+
(a, b) =>
69+
new Date(b.created_at).getTime() -
70+
new Date(a.created_at).getTime()
71+
);
6572
if (matching.length === 0) {
6673
core.info(`${name}: no run for this SHA (path-filtered) — OK`);
6774
continue;

.github/workflows/frontend.yml

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,6 @@ jobs:
3535
- name: Clear Yarn Cache
3636
run: yarn cache clean
3737

38-
- name: Debug - Check for tippy references
39-
run: |
40-
echo "=== Checking package.json for tippy ==="
41-
grep -i tippy package.json || echo "No tippy in package.json"
42-
echo "=== Checking yarn.lock for tippy ==="
43-
grep -i tippy yarn.lock || echo "No tippy in yarn.lock"
44-
echo "=== yarn.lock hash ==="
45-
shasum yarn.lock
46-
4738
- name: Install Dependencies
4839
run: yarn install --frozen-lockfile # Use frozen lockfile for CI
4940

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111

12+
- **Backend CI `changes` job failed on every push to main** (`.github/workflows/backend.yml`): `dorny/paths-filter@v3` was declared without a prior `actions/checkout`, on the stated premise that the action "fetches diffs via the GitHub API". That is only true for `pull_request` events; on `push` events the action shells out to `git branch --show-current`, which fails with `fatal: not a git repository` when the workspace is empty. Every push to a protected branch therefore produced a red X on the `changes` job, silently masked by `continue-on-error: true` plus a `|| github.event_name == 'push'` fail-open gate on downstream jobs. Scoped the `changes` job to `pull_request` events (`if: github.event_name == 'pull_request'`) — where paths-filter actually works — and rewrote the downstream gates on `linter` / `pytest` as `if: always() && (github.event_name == 'push' || needs.changes.outputs.backend != 'false')` so the skip cascade from a `needs`-target that is now intentionally skipped on push does not block the real test jobs. Behaviour preserved: push always runs `linter` + `pytest`; PRs with no backend changes still skip both; PRs where the filter errors transiently (outputs.backend == '') still fail open.
13+
- **Frontend CI tippy-debug step removed** (`.github/workflows/frontend.yml`): Dropped the `Debug - Check for tippy references` step in the `lint` job — a leftover investigation artifact that ran on every PR/push and produced noise with no diagnostic value today. Flagged in Issue #1319.
14+
- **Codecov-notify `matching[0]` relied on undocumented API ordering** (`.github/workflows/codecov-notify.yml`): The cross-workflow coordinator picked the "most recent" workflow run per expected name by indexing `matching[0]` on the filter result, leaning on the GitHub Actions REST API returning results newest-first. That ordering is not guaranteed. Added an explicit `created_at` descending sort before taking `matching[0]`, so re-run detection is correct regardless of API-side quirks. Flagged in Issue #1319.
15+
1216
- **Frontend coverage badge reported ~31% despite months of added tests** (`README.md:12`, `.github/workflows/codecov-notify.yml`, `.codecov.yml`, `frontend/vite.config.ts:210-223`): The README's "Frontend coverage" badge was pointing at `flag=frontend-unit` — the Vitest slice only. The three frontend suites (Vitest unit, Playwright component via `vite-plugin-istanbul`, Playwright E2E via `vite-plugin-istanbul`) upload to separate Codecov flags (`frontend-unit`, `frontend-component`, `frontend-e2e`) and were never merged into a single lcov. Recent PRs almost exclusively added Playwright component and E2E tests, so their coverage landed in `frontend-component`/`frontend-e2e` while the badge stayed stuck reading the Vitest slice.
1317
- Added an `Upload {unit,CT,E2E} lcov artifact` step (using `actions/upload-artifact@v7`) to each producing job in `.github/workflows/frontend.yml` (`component-test` and `unit-test`) and `.github/workflows/frontend-e2e.yml` (`e2e`). The existing per-flag Codecov uploads are untouched, so per-suite drill-in still works.
1418
- Extended the existing cross-workflow coordinator at `.github/workflows/codecov-notify.yml` to download the three artifacts by run id (via `actions/download-artifact@v7` with `continue-on-error: true` to tolerate path-filtered skips and upload failures), merge them with `npx lcov-result-merger@5`, and upload the combined lcov to Codecov under a new `frontend` flag before calling `send-notifications`. The existing `listWorkflowRunsForRepo` check already discovers the producing runs by SHA — it now also emits `frontend_ci_run_id` and `frontend_e2e_run_id` outputs for the downloads to consume.

0 commit comments

Comments
 (0)