From de36f79e4670e8030815a6d629f4d13bb94dece7 Mon Sep 17 00:00:00 2001 From: cucadmuh <49571317+cucadmuh@users.noreply.github.com> Date: Wed, 24 Jun 2026 18:42:59 +0300 Subject: [PATCH] chore(ci): ESLint workflow and path-aware ci-ok merge gate (#1170) --- .github/scripts/ci-ok-aggregate.mjs | 187 +++++++++++++++++++++++ .github/scripts/ci-ok-aggregate.test.mjs | 54 +++++++ .github/workflows/ci-main.yml | 23 ++- .github/workflows/eslint.yml | 49 ++++++ .github/workflows/frontend-tests.yml | 2 + CHANGELOG.md | 7 + CONTRIBUTING.md | 9 +- src/config/settingsCredits.ts | 1 + 8 files changed, 325 insertions(+), 7 deletions(-) create mode 100644 .github/scripts/ci-ok-aggregate.mjs create mode 100644 .github/scripts/ci-ok-aggregate.test.mjs create mode 100644 .github/workflows/eslint.yml diff --git a/.github/scripts/ci-ok-aggregate.mjs b/.github/scripts/ci-ok-aggregate.mjs new file mode 100644 index 00000000..49e11f75 --- /dev/null +++ b/.github/scripts/ci-ok-aggregate.mjs @@ -0,0 +1,187 @@ +/** + * Path-aware ci-ok gate: wait for required workflow job checks on a ref, then pass + * or fail. Mirrors path filters in frontend-tests.yml, eslint.yml, rust-tests.yml. + */ + +const FRONTEND_PATH_RE = + /^(src\/|package\.json$|package-lock\.json$|vitest\.config\.ts$|vite\.config\.ts$|tsconfig\.json$|eslint\.config\.mjs$|\.github\/workflows\/frontend-tests\.yml$|\.github\/workflows\/eslint\.yml$|\.github\/frontend-hot-path-files\.txt$|scripts\/check-frontend-hot-path-coverage\.sh$|scripts\/check-css-import-graph\.mjs$)/; + +const RUST_PATH_RE = /^(src-tauri\/|\.github\/workflows\/rust-tests\.yml$)/; + +const FRONTEND_JOBS = [ + 'vitest run', + 'tsc --noEmit', + 'vitest --coverage (baseline + hot-path file gate)', + 'eslint', +]; + +const RUST_JOBS = [ + 'cargo test --workspace', + 'cargo clippy --workspace', + 'cargo llvm-cov (baseline + hot-path file gate)', +]; + +const POLL_MS = 30_000; +const TIMEOUT_MS = 90 * 60 * 1000; +const OK_CONCLUSIONS = new Set(['success', 'neutral', 'skipped']); + +export function pathTriggersFrontend(file) { + return FRONTEND_PATH_RE.test(file); +} + +export function pathTriggersRust(file) { + return RUST_PATH_RE.test(file); +} + +export function requiredJobNames(changedFiles) { + const names = []; + if (changedFiles.some(pathTriggersFrontend)) { + names.push(...FRONTEND_JOBS); + } + if (changedFiles.some(pathTriggersRust)) { + names.push(...RUST_JOBS); + } + return names; +} + +export function newestChecksByName(checks, excludeRunId) { + const newest = new Map(); + for (const check of checks) { + if (excludeRunId && (check.details_url || '').includes(`/actions/runs/${excludeRunId}/`)) { + continue; + } + const key = check.name; + const prev = newest.get(key); + if (!prev) { + newest.set(key, check); + continue; + } + const prevTime = Date.parse(prev.started_at || prev.completed_at || '') || 0; + const curTime = Date.parse(check.started_at || check.completed_at || '') || 0; + if (curTime >= prevTime) { + newest.set(key, check); + } + } + return newest; +} + +export function evaluateRequiredJobs(required, newestByName) { + const pending = []; + const failures = []; + + for (const name of required) { + const latest = newestByName.get(name); + if (!latest) { + pending.push(`${name}: not started`); + continue; + } + if (latest.status !== 'completed') { + pending.push(`${name}: status=${latest.status}`); + continue; + } + if (!OK_CONCLUSIONS.has(latest.conclusion || '')) { + failures.push(`${name}: conclusion=${latest.conclusion}`); + } + } + + return { pending, failures, done: pending.length === 0 && failures.length === 0 }; +} + +export async function listChangedFiles(github, context) { + const { owner, repo } = context.repo; + + if (context.eventName === 'pull_request') { + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, + repo, + pull_number: context.payload.pull_request.number, + per_page: 100, + }); + return files.map((f) => f.filename); + } + + if (context.eventName === 'push') { + const before = context.payload.before; + const after = context.sha; + if (!before || /^0+$/.test(before)) { + const commit = await github.rest.repos.getCommit({ owner, repo, ref: after }); + return commit.data.files?.map((f) => f.filename) ?? []; + } + const compare = await github.rest.repos.compareCommits({ + owner, + repo, + base: before, + head: after, + }); + return compare.data.files?.map((f) => f.filename) ?? []; + } + + if (context.eventName === 'workflow_run') { + const pr = context.payload.workflow_run.pull_requests?.[0]; + if (pr?.number) { + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, + repo, + pull_number: pr.number, + per_page: 100, + }); + return files.map((f) => f.filename); + } + const headSha = context.payload.workflow_run.head_sha; + const commit = await github.rest.repos.getCommit({ owner, repo, ref: headSha }); + return commit.data.files?.map((f) => f.filename) ?? []; + } + + return []; +} + +export function resolveTargetSha(context) { + if (context.eventName === 'pull_request') { + return context.payload.pull_request.head.sha; + } + if (context.eventName === 'workflow_run') { + return context.payload.workflow_run.head_sha; + } + return context.sha; +} + +export async function runCiOkAggregate(github, context, core) { + const { owner, repo } = context.repo; + const sha = resolveTargetSha(context); + const excludeRunId = String(context.runId); + const changedFiles = await listChangedFiles(github, context); + const required = requiredJobNames(changedFiles); + + core.info(`ci-ok @ ${sha}; ${changedFiles.length} changed file(s)`); + if (required.length === 0) { + core.info('No path-filtered test workflows apply — ci-ok passes.'); + return; + } + core.info(`Waiting for required job checks: ${required.join(', ')}`); + + const deadline = Date.now() + TIMEOUT_MS; + while (Date.now() < deadline) { + const checksAll = await github.paginate(github.rest.checks.listForRef, { + owner, + repo, + ref: sha, + per_page: 100, + }); + const newestByName = newestChecksByName(checksAll, excludeRunId); + const { pending, failures, done } = evaluateRequiredJobs(required, newestByName); + + if (failures.length > 0) { + core.setFailed(`Required checks failed:\n${failures.join('\n')}`); + return; + } + if (done) { + core.info('All required checks are green.'); + return; + } + + core.info(`Pending (${pending.length}): ${pending.join('; ')}`); + await new Promise((resolve) => setTimeout(resolve, POLL_MS)); + } + + core.setFailed(`Timed out after ${TIMEOUT_MS / 60_000} minutes waiting for: ${required.join(', ')}`); +} diff --git a/.github/scripts/ci-ok-aggregate.test.mjs b/.github/scripts/ci-ok-aggregate.test.mjs new file mode 100644 index 00000000..ef86194f --- /dev/null +++ b/.github/scripts/ci-ok-aggregate.test.mjs @@ -0,0 +1,54 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + evaluateRequiredJobs, + newestChecksByName, + pathTriggersFrontend, + pathTriggersRust, + requiredJobNames, +} from './ci-ok-aggregate.mjs'; + +test('pathTriggersFrontend matches frontend workflow paths', () => { + assert.equal(pathTriggersFrontend('src/App.tsx'), true); + assert.equal(pathTriggersFrontend('eslint.config.mjs'), true); + assert.equal(pathTriggersFrontend('README.md'), false); +}); + +test('pathTriggersRust matches rust workflow paths', () => { + assert.equal(pathTriggersRust('src-tauri/src/lib.rs'), true); + assert.equal(pathTriggersRust('src/App.tsx'), false); +}); + +test('requiredJobNames unions frontend and rust jobs', () => { + const names = requiredJobNames(['src/foo.ts', 'src-tauri/bar.rs']); + assert.ok(names.includes('eslint')); + assert.ok(names.includes('cargo test --workspace')); +}); + +test('evaluateRequiredJobs fails on red conclusions', () => { + const newest = newestChecksByName([ + { + name: 'eslint', + status: 'completed', + conclusion: 'failure', + started_at: '2026-01-01T00:00:00Z', + details_url: '', + }, + ]); + const result = evaluateRequiredJobs(['eslint'], newest); + assert.equal(result.done, false); + assert.equal(result.failures.length, 1); +}); + +test('evaluateRequiredJobs passes when all required jobs succeeded', () => { + const checks = ['eslint', 'vitest run'].map((name) => ({ + name, + status: 'completed', + conclusion: 'success', + started_at: '2026-01-01T00:00:00Z', + details_url: '', + })); + const result = evaluateRequiredJobs(['eslint', 'vitest run'], newestChecksByName(checks)); + assert.equal(result.done, true); +}); diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index 4d58390d..6bef9770 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -1,15 +1,30 @@ name: ci-main on: - push: - branches: [main] pull_request: branches: [main] + push: + branches: [main] + workflow_run: + workflows: [frontend-tests, rust-tests, eslint] + types: [completed] workflow_dispatch: +permissions: + contents: read + checks: read + pull-requests: read + jobs: ci-ok: runs-on: ubuntu-latest steps: - - name: ci sentinel - run: echo "main CI sentinel is green" + - uses: actions/checkout@v5 + with: + ref: ${{ github.event.pull_request.head.sha || github.event.workflow_run.head_sha || github.sha }} + - name: aggregate required checks + uses: actions/github-script@v9 + with: + script: | + const { runCiOkAggregate } = await import('${{ github.workspace }}/.github/scripts/ci-ok-aggregate.mjs'); + await runCiOkAggregate(github, context, core); diff --git a/.github/workflows/eslint.yml b/.github/workflows/eslint.yml new file mode 100644 index 00000000..f1c98b71 --- /dev/null +++ b/.github/workflows/eslint.yml @@ -0,0 +1,49 @@ +name: eslint + +on: + pull_request: + branches: [main] + paths: + - 'src/**' + - 'package.json' + - 'package-lock.json' + - 'vitest.config.ts' + - 'vite.config.ts' + - 'tsconfig.json' + - 'eslint.config.mjs' + - '.github/workflows/eslint.yml' + - '.github/frontend-hot-path-files.txt' + - 'scripts/check-frontend-hot-path-coverage.sh' + - 'scripts/check-css-import-graph.mjs' + push: + branches: [main] + paths: + - 'src/**' + - 'package.json' + - 'package-lock.json' + - 'vitest.config.ts' + - 'vite.config.ts' + - 'tsconfig.json' + - 'eslint.config.mjs' + - '.github/workflows/eslint.yml' + - '.github/frontend-hot-path-files.txt' + - 'scripts/check-frontend-hot-path-coverage.sh' + - 'scripts/check-css-import-graph.mjs' + workflow_dispatch: + +permissions: + contents: read + +jobs: + eslint: + name: eslint + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v5 + - uses: actions/setup-node@v6 + with: + node-version: 'lts/*' + cache: 'npm' + - run: npm ci + - name: eslint + run: npm run lint diff --git a/.github/workflows/frontend-tests.yml b/.github/workflows/frontend-tests.yml index d62e7a8d..a84f1bec 100644 --- a/.github/workflows/frontend-tests.yml +++ b/.github/workflows/frontend-tests.yml @@ -10,6 +10,7 @@ on: - 'vitest.config.ts' - 'vite.config.ts' - 'tsconfig.json' + - 'eslint.config.mjs' - '.github/workflows/frontend-tests.yml' - '.github/frontend-hot-path-files.txt' - 'scripts/check-frontend-hot-path-coverage.sh' @@ -23,6 +24,7 @@ on: - 'vitest.config.ts' - 'vite.config.ts' - 'tsconfig.json' + - 'eslint.config.mjs' - '.github/workflows/frontend-tests.yml' - '.github/frontend-hot-path-files.txt' - 'scripts/check-frontend-hot-path-coverage.sh' diff --git a/CHANGELOG.md b/CHANGELOG.md index 97889e39..95ff610f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -253,6 +253,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added an ESLint config and `npm run lint`, and brought `src/` to zero errors and warnings under the strict React-hooks ruleset. Developer-only — no user-facing behaviour change. +### CI — ESLint gate and path-aware ci-ok merge check + +**By [@cucadmuh](https://github.com/cucadmuh), PR [#1170](https://github.com/Psychotoxical/psysonic/pull/1170)** + +* Strict `npm run lint` runs in CI on frontend path filters via a dedicated workflow parallel to the existing frontend test jobs. +* The `ci-ok` check waits for every applicable test and lint job on a PR (frontend and/or Rust, depending on changed paths) and blocks merge when any required job failed or did not finish in time. + ## [1.48.1] - 2026-06-15 ## Fixed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 53880206..633a3a6a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -73,7 +73,7 @@ If you use **Nix**, `nix develop` (see [`flake.nix`](flake.nix)) provides the pi | Topic | Location | |--------|----------| | Frontend test stack (Vitest, Tauri/Subsonic mocks, store resets, i18n in tests) | [`src/test/README.md`](src/test/README.md) | -| What CI runs for frontend / backend | [`frontend-tests.yml`](.github/workflows/frontend-tests.yml), [`rust-tests.yml`](.github/workflows/rust-tests.yml) | +| What CI runs for frontend / backend | [`frontend-tests.yml`](.github/workflows/frontend-tests.yml), [`eslint.yml`](.github/workflows/eslint.yml), [`rust-tests.yml`](.github/workflows/rust-tests.yml) | | Frontend "hot path" files held to a coverage threshold | [`frontend-hot-path-files.txt`](.github/frontend-hot-path-files.txt), [`check-frontend-hot-path-coverage.sh`](scripts/check-frontend-hot-path-coverage.sh) | | Rust hot-path gate | [`hot-path-files.txt`](.github/hot-path-files.txt), [`check-hot-path-coverage.sh`](scripts/check-hot-path-coverage.sh) | | Nix packaging / release automation | [`flake.nix`](flake.nix), workflows under [`.github/workflows/`](.github/workflows/) | @@ -84,7 +84,7 @@ If you use **Nix**, `nix develop` (see [`flake.nix`](flake.nix)) provides the pi 1. **One pull request, one coherent goal.** Easier review, easier revert, fewer merge conflicts. 2. **Match existing style** in touched files (naming, module layout, comment density). Avoid drive-by refactors unrelated to the task. -3. **Linting and formatting:** there is no enforced JS/TS formatter or ESLint config in the repo today — `tsc --noEmit` is the only frontend gate beyond tests. For Rust, `cargo clippy --workspace --all-targets -- -D warnings` is the lint gate; `cargo fmt` is not currently required but won't hurt. +3. **Linting and formatting:** ESLint (strict `eslint.config.mjs`) runs in CI on frontend paths; run `npm run lint` locally before opening a frontend PR. `tsc --noEmit` is also required. For Rust, `cargo clippy --workspace --all-targets -- -D warnings` is the lint gate; `cargo fmt` is not currently required but won't hurt. 4. **Commit messages:** a short **human-readable** summary of what changed and why; Conventional Commits-style prefixes (`feat:`, `fix:`, ...) are fine if you prefer them. Do not include meta references (IDEs, assistants, or how the message was produced) — only what matters for project history. 5. **License:** new code must remain compatible with the project's GPLv3. 6. **Tests:** when you change behaviour users rely on, add or update tests next to the code (see [`src/test/README.md`](src/test/README.md)). Purely visual tweaks may not need tests, but behavioural regressions should be covered where the suite can catch them. @@ -114,9 +114,11 @@ PRs must target `main`. `next` and `release` are maintainer-driven promotion bra Workflows are path-filtered (see the YAML for exact `paths` / `paths-ignore`): -- **Frontend** (`src/**`, lockfile, Vitest/Vite/tsconfig, etc.): `npm test` (Vitest), `npx tsc --noEmit`, then a coverage run. +- **Frontend** (`src/**`, lockfile, Vitest/Vite/tsconfig, ESLint config, etc.): `npm run lint` (ESLint strict), `npm test` (Vitest), `npx tsc --noEmit`, then a coverage run. - **Rust** (`src-tauri/**`): `cargo test --workspace --all-targets`, `cargo clippy --workspace --all-targets -- -D warnings`, then coverage. +The **`ci-ok`** job in [`ci-main.yml`](.github/workflows/ci-main.yml) is the merge gate: it waits for every required job above whose path filter matched the PR, and fails if any of them failed or did not finish in time. + Hot-path coverage gates are **required** on pull requests: the `coverage` jobs in [`frontend-tests.yml`](.github/workflows/frontend-tests.yml) and [`rust-tests.yml`](.github/workflows/rust-tests.yml) fail when any listed file drops below the floor. See the headers in [`frontend-hot-path-files.txt`](.github/frontend-hot-path-files.txt) and [`hot-path-files.txt`](.github/hot-path-files.txt) for curation rules and thresholds. --- @@ -129,6 +131,7 @@ Assume the repository root is `psysonic/` (for example after `git clone https:// ```bash npm ci +npm run lint npm test npx tsc --noEmit npm run test:coverage diff --git a/src/config/settingsCredits.ts b/src/config/settingsCredits.ts index c90abb8b..577731fb 100644 --- a/src/config/settingsCredits.ts +++ b/src/config/settingsCredits.ts @@ -173,6 +173,7 @@ const CONTRIBUTOR_ENTRIES = [ 'Local library index: Navidrome ignored-articles artist/composer letter buckets (name_sort + server ignoredArticles), idempotent migration with safe open/swap and poisoned-lock recovery (PR #1145)', 'All Albums browse: compilation and favorites filters in slice mode — skip redundant client comp filter, route favorites through getStarred2, pre-index page scan, offline isCompilation from tracks (PR #1151)', 'Custom HTTP headers for reverse-proxy gates — per-server header editor, TS/Rust registry sync, full playback/sync/cover path coverage, log redaction (PR #1156)', + 'CI: ESLint strict workflow and path-aware ci-ok merge gate (PR #1170)', ], }, {