mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-09-06 06:28:10 +00:00
🪥 chore: Run CI's Static Checks on Each Commit's Diff (#15303)
* 🪝 chore: Run Static Checks on Every Commit Adds `scripts/static-checks.mts`, a local port of the Static Checks CI job (.github/workflows/static-checks.yml) scoped to the files in a diff. It resolves the changed-file list, applies the same `dorny/paths-filter` groups the job uses, and runs whichever checks those paths activate — ESLint, Prettier, import order, ESLint config validation, package.json validation, and (behind `--full`) config migration tests, unused i18n keys and depcheck. Like the job, every selected check runs even after one fails and the failures are summarized at the end. The pre-commit hook keeps lint-staged for the per-file layer, which verifies the exact staged content of partially staged files, then runs the script for everything lint-staged cannot cover. lint-staged now uses the job's ESLint invocation, so warnings fail locally the way they fail CI. The slow gates stay opt-in (`npm run static-checks:full`, or `STATIC_CHECKS_FULL=1`) to keep commit latency unchanged. Hooks were never installed: `config/prepare.js` existed but no `prepare` script called it, so the hook only ran where `core.hooksPath` had been set by hand. Replaces it with an inline `prepare` — both Dockerfiles run `npm ci` after copying only the manifests, so a `node config/prepare.js` step would fail the image build, and husky is absent from `--omit=dev` installs. The i18n scan is a single pass over the source identifiers rather than one grep per key, verified to flag exactly the same keys as the CI loop (including the substring and dynamic-key cases) in 0.5s instead of 14s. * 🩹 fix: Address Codex Round 1 on the Static Checks Runner Activate gates from the unfiltered changed-path list. `dorny/paths-filter` matches deleted paths too, so gating on the `--diff-filter=ACMRTUXB` list the per-file steps use let a delete-only commit — the last reference to a translation key, say — slip past the i18n and depcheck gates. The two lists are now derived separately, the way CI derives them. Pass `-m` to `git diff-tree` in `--commit` mode. Without it a merge commit emits no paths at all, so `--commit <merge-sha>` reported "Nothing to check"; a real merge in this repo's history goes from 0 to 97 files. Build the workspaces the config suite imports instead of skipping when `dist` is absent. The three `dist` directories are gitignored and `npm ci` does not produce them, so a fresh checkout reported a pass for a gate that never ran — and an existing `dist` could be stale. Each is a sub-second tsdown build. Resolve a global depcheck through a shell on Windows, where npm exposes it as `depcheck.cmd` and `spawnSync` cannot see the shim. Skip dot directories when walking for imports. `.claude/worktrees/` can hold a full checkout per branch — 117 on this machine — and the root-wide scan behind the depcheck gate walked every one of them. Records the remaining boundary in the header: the per-file checks see exact staged content via lint-staged, while the tree-wide gates read the working tree, as running them by hand would. * 🩹 fix: Address Codex Round 2 on the Static Checks Runner Treat an unresolvable checker as a failure. ESLint and Prettier missing meant the runner printed "All affected static checks passed" without having linted anything; only depcheck, which CI installs globally and this documents as optional, may still skip. Catch per-check exceptions. The runner promises that every selected check runs even after a failure, but a throw — a malformed translation JSON, say — escaped and cancelled the checks after it. Each is now recorded as that check's failure; verified that depcheck still runs after i18n throws. Restrict `--commit` to the checked-out commit. Paths came from the named commit while contents came from the working tree, so an older revision was scored against the wrong file contents: a file added then deleted vanished, and one modified since was read at its newer contents. It now fails with a pointer to `--against`. Reject unknown options. `--ful` silently ran the fast tier and `--commmit HEAD` treated `HEAD` as a file path, both exiting 0 and implying gates had run. Cover the runner in CI. `scripts/**` was absent from the workflow's trigger paths, so a PR touching only the script the pre-commit hook now depends on got no Static Checks run — and ESLint has no flat-config match for `scripts/**/*.mts`, so nothing else loads it either. Adds the trigger path, a `runner` filter group and a step that runs the script against the PR's own diff. * 🔗 feat: Add Circular Dependency and TypeScript Gates Both already run in CI as jobs of the Backend Unit Tests workflow; this brings them to the local runner so they land before a push rather than after. Circular dependencies (`node config/circular-deps.mjs`) is fast enough at 0.9s to sit in the per-commit tier, gated on the same paths that trigger the CI job. TypeScript stays opt-in behind `--full`: the five projects cost between 2.3s and 20.9s each, which is too much per commit. Each project declares the paths that can affect it — its own sources plus its upstream packages — so an edit to data-provider still typechecks data-schemas, api, packages/client and client, while an `api/**`-only change runs none of them, since no typechecked project includes that directory. The builds a project's imports resolve through are made first, mirroring the CI jobs' dependency on the build artifacts, through a helper the config suite now shares. Also addresses codex round 3: Reject conflicting target selectors. `--against origin/dev package.json` silently checked only the file, and `--against <bad-ref> --commit HEAD` never resolved the bad base, so a caller could believe a range had been checked. Require a clean worktree in `--commit` mode. The HEAD-only restriction was not enough: contents still come from the working tree, so an uncommitted edit was scored against the named commit — an invalid uncommitted package.json failing a valid HEAD, or an uncommitted fix masking a defect in it. The summary now names how many checks were skipped rather than reporting a bare pass, and a typecheck failure carries the stale-workspace-build hint — inside a git worktree `librechat-data-provider` resolves to the main checkout, whose dist can predate the branch and shows up as missing properties. * 🩹 fix: Address Codex Round 4 on the Static Checks Runner Diff `--against` from the merge base. A two-dot diff reports the base branch's own commits in reverse once it advances, so `--against origin/dev` scored 64 files for a branch that changed 7, activating gates for files the branch never touched. Three dots makes the documented PR-style command mean what it says. Typecheck on root manifest changes. Both review workflows trigger their TypeScript jobs on package.json and package-lock.json, because a dependency or @types bump breaks compilation on its own; the local filter ignored them, so `static-checks:full` passed where CI would fail. Validate every workspace manifest. The list mirrored the four the CI step happens to name, so a malformed packages/api, data-provider or data-schemas manifest passed validation in the revision modes, which have no lint-staged pass behind them. Both lists now cover all seven. Make the runner smoke execute a check. `--list` never runs one, and a script-only PR activates no group, so the CI coverage added for exactly that case could pass with the execution path untouched. It now runs against an explicit target. * 🩹 fix: Address Codex Round 5 on the Static Checks Runner Activate the JSON gate for every manifest it validates. Round 4 added the four workspace manifests to the validation list but not to the filter that turns the gate on, so a malformed packages/data-provider or data-schemas manifest still passed when it was the only changed file — the list grew and the trigger did not. The same two entries also feed the unused-package calculation, reached through api/package.json's @librechat/data-schemas dependency. Include the owning workflows in the imported gates' filters. Circular dependencies and TypeScript come from the review workflows, both of which list their own YAML in `on.paths` and therefore rerun those jobs when the workflow changes; locally the gates stayed inactive, so a change to how they are built or invoked could bypass the local equivalent. Added to the group filters and to the per-project predicates, since a workflow-only change would otherwise activate the group and then select no project. Bound command batches by characters rather than file count. Windows caps a command line at 32767 characters, far below POSIX ARG_MAX, and a count does not bound that: 400 of this repository's longer paths already come to 30176 characters before the executable and fixed arguments. Verified that a list spanning several batches still reports a defect in its final file.
This commit is contained in:
parent
14d6ce7caf
commit
d369c649ed
7 changed files with 1266 additions and 15 deletions
3
.github/CONTRIBUTING.md
vendored
3
.github/CONTRIBUTING.md
vendored
|
|
@ -45,6 +45,9 @@ Project maintainers have the right and responsibility to remove, edit, or reject
|
|||
|
||||
1. Before starting work, make sure your main branch has the latest commits with `npm run update`.
|
||||
2. Run linting command to find errors: `npm run lint`. Alternatively, ensure husky pre-commit checks are functioning.
|
||||
- `npm install` sets the hooks up for you; set `HUSKY=0` to opt out.
|
||||
- The pre-commit hook runs the Static Checks CI job locally, scoped to the files in the commit. Run it by hand with `npm run static-checks`, against a base ref with `npm run static-checks -- --against origin/dev`, or with the slow gates (TypeScript, config migration tests, unused i18n keys, unused npm packages) via `npm run static-checks:full`.
|
||||
- Every commit gets ESLint, Prettier, import order and circular-dependency detection; the slower gates stay opt-in so commits stay fast.
|
||||
3. After your changes, reinstall packages in your current branch using `npm run reinstall` and ensure everything still works.
|
||||
- Restart the ESLint server ("ESLint: Restart ESLint Server" in VS Code command bar) and your IDE after reinstalling or updating.
|
||||
4. Clear web app localStorage and cookies before and after changes.
|
||||
|
|
|
|||
32
.github/workflows/static-checks.yml
vendored
32
.github/workflows/static-checks.yml
vendored
|
|
@ -7,6 +7,7 @@ on:
|
|||
- 'client/**'
|
||||
- 'config/**'
|
||||
- 'packages/**'
|
||||
- 'scripts/**'
|
||||
- 'package.json'
|
||||
- 'package-lock.json'
|
||||
- 'eslint.config.mjs'
|
||||
|
|
@ -40,6 +41,8 @@ jobs:
|
|||
# config via git show — a shallow checkout breaks both.
|
||||
fetch-depth: 0
|
||||
|
||||
# scripts/static-checks.mts mirrors these filters to run the same checks
|
||||
# locally on a commit's diff — keep the two in sync.
|
||||
- name: Detect affected checks
|
||||
id: paths
|
||||
uses: dorny/paths-filter@v4
|
||||
|
|
@ -70,11 +73,19 @@ jobs:
|
|||
- 'packages/data-schemas/src/**'
|
||||
- '.github/workflows/static-checks.yml'
|
||||
- '!**.md'
|
||||
runner:
|
||||
- 'scripts/static-checks.mts'
|
||||
- '.github/workflows/static-checks.yml'
|
||||
unused_packages:
|
||||
- 'api/**'
|
||||
- 'client/**'
|
||||
- 'packages/api/**'
|
||||
- 'packages/client/**'
|
||||
# Every workspace manifest the JSON validation step covers, plus
|
||||
# the ones whose dependencies feed the unused-package calculation
|
||||
# through api/package.json's @librechat/data-schemas entry.
|
||||
- 'packages/data-provider/package.json'
|
||||
- 'packages/data-schemas/package.json'
|
||||
- 'package.json'
|
||||
- 'package-lock.json'
|
||||
- '.github/workflows/static-checks.yml'
|
||||
|
|
@ -202,6 +213,23 @@ jobs:
|
|||
# Directory args, not `npm run lint`: the root brace-expansion@^5
|
||||
# override breaks minimatch@3's brace expansion, so that script's
|
||||
# braced glob crashes on a clean install; dir args never brace-expand.
|
||||
# scripts/static-checks.mts runs these same checks locally from the
|
||||
# pre-commit hook, and nothing else in this job loads it: ESLint has no
|
||||
# flat-config match for scripts/**/*.mts. Run it against the PR's own
|
||||
# diff so a syntax error or a broken filter fails here rather than in
|
||||
# every contributor's next commit.
|
||||
- name: Smoke the local static-checks runner
|
||||
id: runner
|
||||
if: always() && steps.paths.outputs.runner == 'true'
|
||||
continue-on-error: true
|
||||
run: |
|
||||
BASE_SHA=$(jq --raw-output .pull_request.base.sha "$GITHUB_EVENT_PATH")
|
||||
node scripts/static-checks.mts --against "$BASE_SHA" --list
|
||||
# An explicit target, because --list never executes a check and a
|
||||
# script-only PR activates no group — so neither would exercise the
|
||||
# execution path this step exists to protect.
|
||||
node scripts/static-checks.mts package.json --only json
|
||||
|
||||
- name: Validate ESLint config on config changes
|
||||
id: eslint_config
|
||||
if: always() && steps.paths.outputs.eslint_config == 'true'
|
||||
|
|
@ -393,7 +421,7 @@ jobs:
|
|||
if: always() && steps.paths.outputs.unused_packages == 'true'
|
||||
continue-on-error: true
|
||||
run: |
|
||||
for FILE in package.json client/package.json api/package.json packages/client/package.json; do
|
||||
for FILE in package.json client/package.json api/package.json packages/api/package.json packages/client/package.json packages/data-provider/package.json packages/data-schemas/package.json; do
|
||||
if [[ -f "$FILE" ]]; then
|
||||
jq empty "$FILE" || (echo "::error title=Invalid JSON::$FILE is invalid" && exit 1)
|
||||
fi
|
||||
|
|
@ -642,6 +670,7 @@ jobs:
|
|||
ESLINT_CONFIG_OUTCOME: ${{ steps.eslint_config.outcome }}
|
||||
PRETTIER_OUTCOME: ${{ steps.prettier.outcome }}
|
||||
IMPORT_SORT_OUTCOME: ${{ steps.import_sort.outcome }}
|
||||
RUNNER_OUTCOME: ${{ steps.runner.outcome }}
|
||||
CACHE_DATA_PROVIDER_OUTCOME: ${{ steps.cache-data-provider.outcome }}
|
||||
CONFIG_DATA_PROVIDER_OUTCOME: ${{ steps.config_data_provider.outcome }}
|
||||
CACHE_DATA_SCHEMAS_OUTCOME: ${{ steps.cache-data-schemas.outcome }}
|
||||
|
|
@ -678,6 +707,7 @@ jobs:
|
|||
record_failure "ESLint config validation" "$ESLINT_CONFIG_OUTCOME"
|
||||
record_failure "Prettier" "$PRETTIER_OUTCOME"
|
||||
record_failure "Import sorting" "$IMPORT_SORT_OUTCOME"
|
||||
record_failure "Local static-checks runner" "$RUNNER_OUTCOME"
|
||||
record_failure "Config data-provider cache" "$CACHE_DATA_PROVIDER_OUTCOME"
|
||||
record_failure "Config data-provider build" "$CONFIG_DATA_PROVIDER_OUTCOME"
|
||||
record_failure "Config data-schemas cache" "$CACHE_DATA_SCHEMAS_OUTCOME"
|
||||
|
|
|
|||
|
|
@ -3,7 +3,9 @@ module.exports = {
|
|||
'node scripts/sort-imports.mts',
|
||||
'prettier --write',
|
||||
'eslint --fix',
|
||||
'eslint',
|
||||
// Same invocation as the Static Checks CI job: warnings are failures there,
|
||||
// and changed files under config-ignored paths must not trip it.
|
||||
'eslint --config eslint.config.mjs --no-warn-ignored --max-warnings=0',
|
||||
],
|
||||
'*.json': ['prettier --write'],
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1,3 +1,10 @@
|
|||
#!/bin/sh
|
||||
[ -n "$CI" ] && exit 0
|
||||
npx lint-staged --config ./.husky/lint-staged.config.js
|
||||
|
||||
# Formats, sorts and lints the exact staged content of each changed file.
|
||||
npx lint-staged --config ./.husky/lint-staged.config.js || exit 1
|
||||
|
||||
# The rest of the Static Checks CI job, gated on the paths this commit touches.
|
||||
# Add STATIC_CHECKS_FULL=1 for the slow gates (TypeScript, config migration
|
||||
# tests, unused i18n keys, unused npm packages).
|
||||
node scripts/static-checks.mts --skip eslint,prettier,imports
|
||||
|
|
|
|||
|
|
@ -1,12 +0,0 @@
|
|||
const { exec } = require('child_process');
|
||||
|
||||
if (process.env.NODE_ENV !== 'CI') {
|
||||
exec('npx husky install', (error, stdout, stderr) => {
|
||||
if (error) {
|
||||
console.error(`exec error: ${error}`);
|
||||
return;
|
||||
}
|
||||
console.log(`stdout: ${stdout}`);
|
||||
console.error(`stderr: ${stderr}`);
|
||||
});
|
||||
}
|
||||
|
|
@ -9,6 +9,7 @@
|
|||
"packages/*"
|
||||
],
|
||||
"scripts": {
|
||||
"prepare": "node -e \"import('husky').then((h) => h.default()).catch(() => {})\"",
|
||||
"update": "node config/update.js",
|
||||
"add-balance": "node config/add-balance.js",
|
||||
"set-balance": "node config/set-balance.js",
|
||||
|
|
@ -104,6 +105,8 @@
|
|||
"lint": "eslint .",
|
||||
"sort-imports": "node scripts/sort-imports.mts",
|
||||
"sort-imports:check": "node scripts/sort-imports.mts --check",
|
||||
"static-checks": "node scripts/static-checks.mts",
|
||||
"static-checks:full": "node scripts/static-checks.mts --full",
|
||||
"format": "npx prettier --write \"{,!(node_modules|venv)/**/}*.{js,jsx,ts,tsx}\"",
|
||||
"b:api": "NODE_ENV=production bun run api/server/index.js",
|
||||
"b:api-inspect": "NODE_ENV=production bun --inspect run api/server/index.js",
|
||||
|
|
|
|||
1218
scripts/static-checks.mts
Normal file
1218
scripts/static-checks.mts
Normal file
File diff suppressed because it is too large
Load diff
Loading…
Add table
Add a link
Reference in a new issue