From dad9f4e484245e1b8398cddbfeb70338eb2725a2 Mon Sep 17 00:00:00 2001 From: Charis <26616127+charislam@users.noreply.github.com> Date: Mon, 10 Nov 2025 13:49:22 -0500 Subject: [PATCH] ci: add eslint ratcheting (#40156) * chore: add eslint ratchet script * chore: add eslint ratchet action * refactor(ratchet script): convert to typescript * ci(ratchet script): add --decrease-baselines flag Allows us to decrease the baselines on schedule as we fix warnings * ci(ratchet): add action to decrease baseline if possible every week * chore(eslint): fix exhaustive-deps error * docs(internal): improve docs for eslint ratchet script * chore(ratchet): add new ratchet rules Add: - import/no-anonymous-default-export - @tanstack/query/exhaustive-deps - @tanstack/query/no-deprecated-options Not adding `no-restricted-exports` even though we have many violations because we first need to reconfigure it (if possible) to ignore those files where Next.js requires a default export. --- .../studio-lint-ratchet-decrease.yml | 80 +++++ .github/workflows/studio-lint-ratchet.yml | 45 +++ .../studio/.github/eslint-rule-baselines.json | 8 + apps/studio/package.json | 1 + apps/studio/pages/project/[ref]/merge.tsx | 16 +- apps/studio/scripts/ratchet-eslint-rules.ts | 314 ++++++++++++++++++ 6 files changed, 449 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/studio-lint-ratchet-decrease.yml create mode 100644 .github/workflows/studio-lint-ratchet.yml create mode 100644 apps/studio/.github/eslint-rule-baselines.json create mode 100644 apps/studio/scripts/ratchet-eslint-rules.ts diff --git a/.github/workflows/studio-lint-ratchet-decrease.yml b/.github/workflows/studio-lint-ratchet-decrease.yml new file mode 100644 index 00000000000..25fcf61b092 --- /dev/null +++ b/.github/workflows/studio-lint-ratchet-decrease.yml @@ -0,0 +1,80 @@ +name: Decrease studio lint ratchet baselines + +on: + schedule: + - cron: '0 0 * * SUN' + workflow_dispatch: + +permissions: + contents: write + pull-requests: write + +jobs: + decrease-baselines: + runs-on: blacksmith-4vcpu-ubuntu-2404 + + steps: + - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + sparse-checkout: | + .github + apps/studio + packages + + - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + name: Install pnpm + with: + run_install: false + + - name: Use Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version-file: '.nvmrc' + cache: 'pnpm' + + - name: Install deps + run: pnpm install --frozen-lockfile + + - name: Decrease ESLint ratchet baselines and open PR + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: | + set -eo pipefail + DEFAULT_BRANCH=${DEFAULT_BRANCH:-master} + + BRANCH="bot/decrease-eslint-ratchet-baselines" + + git fetch origin "$DEFAULT_BRANCH" --depth=1 + if git ls-remote --exit-code --heads origin "$BRANCH" > /dev/null 2>&1; then + git fetch origin "$BRANCH":"$BRANCH" --depth=1 + git switch "$BRANCH" + git reset --hard "origin/$DEFAULT_BRANCH" + else + git switch --create "$BRANCH" "origin/$DEFAULT_BRANCH" + fi + + pnpm --filter studio run lint:ratchet --decrease-baselines + + if git diff --quiet; then + echo "No baseline updates detected." + exit 0 + fi + + git config user.name 'github-actions[bot]' + git config user.email 'github-actions[bot]@users.noreply.github.com' + + git add apps/studio/.github/eslint-rule-baselines.json + git commit --message "chore: decrease ESLint ratchet baselines" + git push --force origin "$BRANCH" + + pr_url=$(gh pr list --state open --head "$BRANCH" --json url --jq '.[0].url // ""' 2>/dev/null || echo "") + if [ -z "$pr_url" ]; then + gh pr create \ + --title "[bot] Decrease ESLint ratchet baselines" \ + --body "Automated weekly decrease of ESLint ratchet baselines." \ + --base "$DEFAULT_BRANCH" \ + --head "$BRANCH" + else + gh pr comment "$pr_url" --body "Updated ESLint ratchet baselines with the latest weekly decreases." + fi diff --git a/.github/workflows/studio-lint-ratchet.yml b/.github/workflows/studio-lint-ratchet.yml new file mode 100644 index 00000000000..b998adef317 --- /dev/null +++ b/.github/workflows/studio-lint-ratchet.yml @@ -0,0 +1,45 @@ +name: Ratchet studio lint checks + +on: + pull_request: + branches: + - master + paths: + - 'apps/studio/**' + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + ratchet: + # Uses larger hosted runner as it significantly decreases build times + runs-on: blacksmith-4vcpu-ubuntu-2404 + + steps: + - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + sparse-checkout: | + .github + apps/studio + packages + + - uses: pnpm/action-setup@a7487c7e89a18df4991f7f222e4898a00d66ddda # v4.1.0 + name: Install pnpm + with: + run_install: false + + - name: Use Node.js + uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4.4.0 + with: + node-version-file: '.nvmrc' + cache: 'pnpm' + + - name: Install deps + run: pnpm install --frozen-lockfile + + - name: Run ratchet script + run: pnpm --filter studio run lint:ratchet diff --git a/apps/studio/.github/eslint-rule-baselines.json b/apps/studio/.github/eslint-rule-baselines.json new file mode 100644 index 00000000000..b5b413c54cf --- /dev/null +++ b/apps/studio/.github/eslint-rule-baselines.json @@ -0,0 +1,8 @@ +{ + "rules": { + "react-hooks/exhaustive-deps": 238, + "import/no-anonymous-default-export": 62, + "@tanstack/query/exhaustive-deps": 19, + "@tanstack/query/no-deprecated-options": 2 + } +} diff --git a/apps/studio/package.json b/apps/studio/package.json index 294f7837d8f..73d5238afb1 100644 --- a/apps/studio/package.json +++ b/apps/studio/package.json @@ -8,6 +8,7 @@ "build": "next build && ./../../scripts/upload-static-assets.sh", "start": "next start", "lint": "eslint .", + "lint:ratchet": "tsx scripts/ratchet-eslint-rules.ts --rule react-hooks/exhaustive-deps --rule import/no-anonymous-default-export --rule @tanstack/query/exhaustive-deps --rule @tanstack/query/no-deprecated-options", "clean": "rimraf node_modules tsconfig.tsbuildinfo .next .turbo", "test": "vitest --run --coverage", "test:watch": "vitest watch", diff --git a/apps/studio/pages/project/[ref]/merge.tsx b/apps/studio/pages/project/[ref]/merge.tsx index 19f17496757..27f237d8bf8 100644 --- a/apps/studio/pages/project/[ref]/merge.tsx +++ b/apps/studio/pages/project/[ref]/merge.tsx @@ -313,20 +313,6 @@ const MergePage: NextPageWithLayout = () => { }) } - const handleReadyForReview = () => { - if (!ref || !parentProjectRef) return - updateBranch( - { - branchRef: ref, - projectRef: parentProjectRef, - requestReview: true, - }, - { - onSuccess: () => toast.success('Successfully marked as ready for review'), - } - ) - } - const breadcrumbs = useMemo( () => [ { @@ -334,7 +320,7 @@ const MergePage: NextPageWithLayout = () => { href: `/project/${project?.ref}/branches/merge-requests`, }, ], - [parentProjectRef] + [project?.ref] ) const currentTab = (router.query.tab as string) || 'database' diff --git a/apps/studio/scripts/ratchet-eslint-rules.ts b/apps/studio/scripts/ratchet-eslint-rules.ts new file mode 100644 index 00000000000..09613141d43 --- /dev/null +++ b/apps/studio/scripts/ratchet-eslint-rules.ts @@ -0,0 +1,314 @@ +/* eslint-disable turbo/no-undeclared-env-vars */ +/** + * Ratchet ESLint violations for selected rules. + * + * Examples: + * # Initialize baselines for two rules + * tsx scripts/ratchet-eslint-rules.ts --init \ + * --rule react-hooks/exhaustive-deps --rule no-console + * + * # Compare current counts vs baselines + * tsx scripts/ratchet-eslint-rules.ts \ + * --rule react-hooks/exhaustive-deps --rule no-console + * + # Decrease baselines when improvements occur + * tsx scripts/ratchet-eslint-rules.ts \ + * --rule react-hooks/exhaustive-deps --rule no-console \ + * --decrease-baselines + * + * Flags: + * --metadata Path to baseline file (default .github/eslint-rule-baselines.json) + * --init Write current counts for the provided --rule(s) into metadata and exit 0 + * --eslint "" ESLint command to run (default "npx eslint"). Do not pass untrusted input. + * --eslint-args "<...>" Extra args/paths for ESLint (e.g., "."). Do not pass untrusted input. + * --rule [,...] Rule id(s). Repeat flag or comma-separate. REQUIRED. + * --decrease-baselines When improvements occur, lower stored baselines to match the new counts. + * + * Notes: + * - Counts occurrences regardless of severity (warn/error). + * - Fails if any selected rule has currentCount > baselineCount. + */ + +import { spawnSync } from 'node:child_process' +import { appendFileSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs' +import path from 'node:path' + +interface Args { + metadata: string + init: boolean + eslint: string + eslintArgs: string + decreaseBaselines: boolean + rules: string[] +} + +interface ESLintMessage { + ruleId?: string | null +} + +interface ESLintResult { + messages?: ESLintMessage[] +} + +interface ESLintExecutionResult { + results: ESLintResult[] + stderr: string +} + +interface BaselineData { + rules: Record +} + +function parseArgs(argv: string[]): Args { + const args: Args = { + metadata: '.github/eslint-rule-baselines.json', + init: false, + eslint: 'npx eslint', + eslintArgs: '', + decreaseBaselines: false, + rules: [], + } + + for (let i = 2; i < argv.length; i += 1) { + const a = argv[i] + if (a === '--init') { + args.init = true + } else if (a === '--metadata') { + args.metadata = argv[++i] + } else if (a === '--eslint') { + args.eslint = argv[++i] + } else if (a === '--eslint-args') { + args.eslintArgs = argv[++i] + } else if (a === '--rule') { + const val = (argv[++i] ?? '').trim() + if (val) { + args.rules.push( + ...val + .split(',') + .map((s) => s.trim()) + .filter(Boolean) + ) + } + } else if (a === '--decrease-baselines') { + args.decreaseBaselines = true + } else { + console.warn(`Unknown argument: ${a}`) + } + } + + if (args.rules.length === 0) { + console.error('Error: You must provide at least one --rule .') + console.error('Example: --rule exhaustive-deps --rule no-console') + process.exit(2) + } + + const dedupedRules = new Set(args.rules) + args.rules = Array.from(dedupedRules) + + return args +} + +/** + * SECURITY: + * Directly spawns a command from its arguments. Should not be called with + * untrusted input. + */ +function dangerouslyRunEsLint(eslintCmd: string, eslintArgs: string): ESLintExecutionResult { + const fullCmd = `${eslintCmd} ${eslintArgs || ''} --format json`.trim() + const proc = spawnSync(fullCmd, { + shell: true, + encoding: 'utf8', + stdio: ['ignore', 'pipe', 'pipe'], + env: process.env, + maxBuffer: 32 * 1024 * 1024, // allow large ESLint JSON payloads + }) + + const stdout = typeof proc.stdout === 'string' ? proc.stdout : '' + const stderr = typeof proc.stderr === 'string' ? proc.stderr : '' + + if (!stdout.trim()) { + console.error('ESLint did not produce JSON output. stderr:\n', stderr) + process.exit(2) + } + + let results: ESLintResult[] + try { + results = JSON.parse(stdout) as ESLintResult[] + } catch (e) { + console.error('Failed to parse ESLint JSON output:', e) + console.error('Raw output (truncated to 4k):\n', stdout.slice(0, 4096)) + process.exit(2) + } + + return { results, stderr } +} + +function countRules(results: ESLintResult[], ruleIds: string[]): Record { + const checkedIds = new Set(ruleIds) + const counts: Record = {} + + for (const id of ruleIds) { + counts[id] = 0 + } + + for (const file of results) { + if (!file || !Array.isArray(file.messages)) continue + for (const msg of file.messages) { + const id = msg?.ruleId ?? '' + if (id && checkedIds.has(id)) { + counts[id] += 1 + } + } + } + return counts +} + +function readBaselines(fp: string): BaselineData { + if (!existsSync(fp)) return { rules: {} } + try { + const data = JSON.parse(readFileSync(fp, 'utf8')) as Partial + if (data && typeof data === 'object' && data.rules && typeof data.rules === 'object') { + return { rules: data.rules } + } + } catch { + // ignore invalid metadata files and fall back to blank baselines + } + return { rules: {} } +} + +function writeBaselines(fp: string, updates: Record, merge = true): void { + const dir = path.dirname(fp) + mkdirSync(dir, { recursive: true }) + + let current: BaselineData = { rules: {} } + if (merge && existsSync(fp)) { + current = readBaselines(fp) + } + + const next: BaselineData = { rules: { ...current.rules, ...updates } } + writeFileSync(fp, `${JSON.stringify(next, null, 2)}\n`, 'utf8') +} + +function writeSummary(markdown: string): void { + const summaryFile = process.env.GITHUB_STEP_SUMMARY + if (summaryFile) { + try { + appendFileSync(summaryFile, `${markdown}\n`, 'utf8') + } catch { + // ignore summary write errors because they shouldn't block the script + } + } +} + +function main(): void { + const args = parseArgs(process.argv) + + // SECURITY: + // Offloaded to user. Must document that they should not pass untrusted input + // via --eslint or --eslint-args. + const { results, stderr } = dangerouslyRunEsLint(args.eslint, args.eslintArgs) + const currentCounts = countRules(results, args.rules) + + if (args.init) { + writeBaselines(args.metadata, currentCounts, true) + + const rows = Object.entries(currentCounts) + .map(([rule, count]) => `| \`${rule}\` | **${count}** |`) + .join('\n') + + writeSummary( + [ + `### ESLint rule baselines initialized`, + `Metadata: \`${args.metadata}\``, + ``, + `| Rule | Baseline |`, + `| --- | ---: |`, + rows, + ``, + ].join('\n') + ) + + console.log( + `Initialized/updated baselines for: ${args.rules.join(', ')} (saved to ${args.metadata}).` + ) + process.exit(0) + } + + const baselines = readBaselines(args.metadata).rules || {} + + const missing = args.rules.filter((r) => typeof baselines[r] !== 'number') + if (missing.length) { + const msg = `Missing baselines for: ${missing.join(', ')} in ${args.metadata}. Run with --init to set them.` + console.error(msg) + writeSummary(`### ESLint rule ratchet\n${msg}`) + console.log(`::error title=Missing baselines::${msg}`) + process.exit(2) + } + + let failed = false + const tableRows: string[] = [] + const improvedRules: string[] = [] + const decreasedBaselines: Record = {} + for (const rule of args.rules) { + const baseline = baselines[rule] ?? 0 + const current = currentCounts[rule] ?? 0 + const delta = current - baseline + + tableRows.push( + `| \`${rule}\` | **${baseline}** | **${current}** | ${delta >= 0 ? '+' : '-'}${delta} |` + ) + + if (current > baseline) { + failed = true + const delta = current - baseline + const msg = `You added ${delta === 1 ? 'a new violation' : `${delta} new violations`} of ${rule}. Please fix it: baseline=${baseline}, current=${current}` + console.error(msg) + console.log(`::error title=New violations::${msg}`) + } else if (current < baseline) { + improvedRules.push(rule) + if (args.decreaseBaselines) { + decreasedBaselines[rule] = { from: baseline, to: current } + } + } + } + + const summaryLines = [ + `### ESLint rule ratchet`, + `Metadata: \`${args.metadata}\``, + ``, + `| Rule | Baseline | Current | Δ |`, + `| --- | ---: | ---: | ---: |`, + ...tableRows, + ``, + ] + + if (args.decreaseBaselines && Object.keys(decreasedBaselines).length > 0) { + const updates: Record = {} + const details: string[] = [] + const logParts: string[] = [] + for (const [rule, { from, to }] of Object.entries(decreasedBaselines)) { + updates[rule] = to + details.push(`- \`${rule}\`: ${from} -> ${to}`) + logParts.push(`${rule}: ${from} -> ${to}`) + } + writeBaselines(args.metadata, updates, true) + summaryLines.push('', 'Baselines decreased for improved rules:', ...details, '') + console.log(`Baselines decreased for improved rules: ${logParts.join(', ')}`) + } + + writeSummary(summaryLines.join('\n')) + + if (failed) { + if (stderr && stderr.trim()) console.error('\nESLint stderr:\n', stderr) + process.exit(1) + } else { + console.log( + improvedRules.length > 0 + ? 'Nice! Some rules improved.' + : 'Stable: No regressions for selected rules.' + ) + process.exit(0) + } +} + +main()