From 5447a26fd1c86d9589e17cc4a4882182cfe2e9ff Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sun, 25 Oct 2026 00:23:38 -0700 Subject: [PATCH] perf(lint): keep JS plugins out of default oxlint and adopt ast-grep (#33430) * perf(lint): keep JS plugins out of default oxlint and adopt ast-grep Any jsPlugin makes Oxlint hand every file's AST to Node, so the seven custom plugins cost the default `oxlint` run several times its native time. - Move app-store-performance, quadratic-buffer-concat, renderer-scrollbar-style and ime-enter-guard (scope- or dataflow-aware) to config/oxlint-js-plugins.json, run by `pnpm lint:js-plugins`, CI, lint-staged and the changed-lines gate. - Port sort-comparator-performance, mobile-pairing and native-chat-design-tokens to ast-grep rules with rule tests, and delete the plugins. - Add @ast-grep/cli, config/sgconfig.yml, `pnpm lint:ast-grep` / `test:ast-grep`, a CI step, and an ast-grep pass in check-changed-code-quality. - Add flaky-test rules (wall-clock reads, real-time waits, polling loops, unquarantined skips, unexplained timeout bumps), baselined by a count ratchet. * Enforce flaky-test ratchet on changed files; refresh baseline * docs(agents): keep custom-rule details out of AGENTS.md * Resolve ast-grep from the repo so gate scans of other roots work --- .github/workflows/performance-contracts.yml | 1 - .github/workflows/pr.yml | 20 + .oxlintrc.json | 54 +- .../rule-tests/flaky-test-patterns-test.yml | 83 + .../native-chat-design-tokens-test.yml | 32 + .../no-eager-qrcode-import-test.yml | 11 + .../rule-tests/no-repeated-collator-test.yml | 18 + .../flaky-tests/test-no-polling-loop.yml | 139 ++ .../flaky-tests/test-no-real-time-wait.yml | 130 ++ .../rules/flaky-tests/test-no-wall-clock.yml | 55 + .../test-skip-needs-quarantine.yml | 62 + .../flaky-tests/test-timeout-needs-reason.yml | 86 + .../native-chat-no-arbitrary-font-size.yml | 29 + .../native-chat-no-softened-foreground.yml | 29 + .../native-chat-no-weakened-focus-ring.yml | 29 + .../ast-grep/rules/no-eager-qrcode-import.yml | 33 + .../ast-grep/rules/no-repeated-collator.yml | 61 + config/flaky-test-pattern-baseline.txt | 1629 +++++++++++++++++ config/oxlint-js-plugins.json | 55 + config/oxlint-performance-audit.json | 7 +- .../mobile-pairing-qrcode-import.mjs | 29 - .../native-chat-design-tokens.mjs | 49 - .../sort-comparator-performance.mjs | 60 - config/performance-audit.md | 8 +- config/scripts/ast-grep-scan.mjs | 50 + config/scripts/check-changed-code-quality.mjs | 49 +- .../check-changed-code-quality.test.mjs | 3 + .../check-flaky-test-pattern-ratchet.mjs | 103 ++ .../check-flaky-test-pattern-ratchet.test.mjs | 34 + config/scripts/ci-shard-timings.json | 3 - .../locale-collator-sort-benchmark.mjs | 4 +- ...bile-pairing-qrcode-import-plugin.test.mjs | 48 - config/scripts/moved-code-matcher.test.mjs | 7 +- .../native-chat-design-tokens-plugin.test.mjs | 99 - config/scripts/pr-code-change-scope.mjs | 1 + config/scripts/pr-preflight-gates.test.mjs | 2 + config/scripts/ratchet-baseline.mjs | 4 +- ...ort-comparator-performance-plugin.test.mjs | 70 - config/sgconfig.yml | 9 + config/vitest.performance.config.ts | 1 - docs/styleguide/native-chat.md | 2 +- package.json | 10 +- pnpm-lock.yaml | 125 +- pnpm-workspace.yaml | 2 + src/main/skills/skill-discovery-order.test.ts | 2 +- src/main/skills/skill-discovery-wsl.test.ts | 2 +- src/main/warp-themes/discovery.test.ts | 4 +- .../manual-warp-theme-files.test.ts | 4 +- .../warp-themes/theme-file-scanner.test.ts | 2 +- .../skills/skill-delete-copy.test.ts | 2 +- 50 files changed, 2878 insertions(+), 473 deletions(-) create mode 100644 config/ast-grep/rule-tests/flaky-test-patterns-test.yml create mode 100644 config/ast-grep/rule-tests/native-chat-design-tokens-test.yml create mode 100644 config/ast-grep/rule-tests/no-eager-qrcode-import-test.yml create mode 100644 config/ast-grep/rule-tests/no-repeated-collator-test.yml create mode 100644 config/ast-grep/rules/flaky-tests/test-no-polling-loop.yml create mode 100644 config/ast-grep/rules/flaky-tests/test-no-real-time-wait.yml create mode 100644 config/ast-grep/rules/flaky-tests/test-no-wall-clock.yml create mode 100644 config/ast-grep/rules/flaky-tests/test-skip-needs-quarantine.yml create mode 100644 config/ast-grep/rules/flaky-tests/test-timeout-needs-reason.yml create mode 100644 config/ast-grep/rules/native-chat-no-arbitrary-font-size.yml create mode 100644 config/ast-grep/rules/native-chat-no-softened-foreground.yml create mode 100644 config/ast-grep/rules/native-chat-no-weakened-focus-ring.yml create mode 100644 config/ast-grep/rules/no-eager-qrcode-import.yml create mode 100644 config/ast-grep/rules/no-repeated-collator.yml create mode 100644 config/flaky-test-pattern-baseline.txt create mode 100644 config/oxlint-js-plugins.json delete mode 100644 config/oxlint-plugins/mobile-pairing-qrcode-import.mjs delete mode 100644 config/oxlint-plugins/native-chat-design-tokens.mjs delete mode 100644 config/oxlint-plugins/sort-comparator-performance.mjs create mode 100644 config/scripts/ast-grep-scan.mjs create mode 100644 config/scripts/check-flaky-test-pattern-ratchet.mjs create mode 100644 config/scripts/check-flaky-test-pattern-ratchet.test.mjs delete mode 100644 config/scripts/mobile-pairing-qrcode-import-plugin.test.mjs delete mode 100644 config/scripts/native-chat-design-tokens-plugin.test.mjs delete mode 100644 config/scripts/sort-comparator-performance-plugin.test.mjs create mode 100644 config/sgconfig.yml diff --git a/.github/workflows/performance-contracts.yml b/.github/workflows/performance-contracts.yml index aaac40c6488..16b84f2910e 100644 --- a/.github/workflows/performance-contracts.yml +++ b/.github/workflows/performance-contracts.yml @@ -14,7 +14,6 @@ on: - 'config/oxlint-plugins/quadratic-buffer-concat.mjs' - 'config/scripts/*-plugin.test.mjs' - '!config/scripts/renderer-scrollbar-style-plugin.test.mjs' - - '!config/scripts/mobile-pairing-qrcode-import-plugin.test.mjs' # Keep in sync with the contract list in config/vitest.performance.config.ts; # without these a rename lands green and only breaks the next nightly. - 'src/main/sqlite/sync-database.test.ts' diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 33e4c5edc59..4b41b45bd88 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -276,6 +276,24 @@ jobs: if [ "$PREFLIGHT_PHASE_SELECTED" != true ] || [ "$PREFLIGHT_PRIOR_SUCCESS" != true ]; then exit 0; fi node node_modules/oxlint/bin/oxlint --format github + # JS plugins make Oxlint hand every AST to Node, so they run apart from the native root lint. + - name: Enforce JS-plugin lint rules + if: '!cancelled()' + id: js-plugin-lint + background: true + env: + PREFLIGHT_PHASE_SELECTED: ${{ needs.code_paths.outputs.static_analysis == 'true' }} + PREFLIGHT_PRIOR_SUCCESS: ${{ job.status == 'success' }} + run: | + if [ "$PREFLIGHT_PHASE_SELECTED" != true ] || [ "$PREFLIGHT_PRIOR_SUCCESS" != true ]; then exit 0; fi + node node_modules/oxlint/bin/oxlint --config config/oxlint-js-plugins.json --format github src config tests mobile native .github + + - name: Enforce ast-grep rules + if: needs.code_paths.outputs.static_analysis == 'true' + run: | + node_modules/.bin/ast-grep test --config config/sgconfig.yml --skip-snapshot-tests + node_modules/.bin/ast-grep scan --config config/sgconfig.yml --min-severity warning --format github src config tests mobile + # Why the full tree: it takes ~2s, and the pre-commit hook is skippable. - name: Check formatting if: needs.code_paths.outputs.static_analysis == 'true' @@ -533,6 +551,8 @@ jobs: - wait: main-sync-spawn-ratchet + - wait: js-plugin-lint + - wait: unit-plan - uses: actions/upload-artifact@v7 diff --git a/.oxlintrc.json b/.oxlintrc.json index 5957be6e444..ef1c2a14101 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -1,48 +1,13 @@ { "$schema": "./node_modules/oxlint/configuration_schema.json", "plugins": ["typescript", "react", "react-hooks", "react-perf", "unicorn"], - "jsPlugins": [ - { - "name": "sort-comparator-performance", - "specifier": "./config/oxlint-plugins/sort-comparator-performance.mjs" - }, - { - "name": "mobile-pairing", - "specifier": "./config/oxlint-plugins/mobile-pairing-qrcode-import.mjs" - }, - { - "name": "app-store-performance", - "specifier": "./config/oxlint-plugins/app-store-performance.mjs" - }, - { - "name": "quadratic-buffer-concat", - "specifier": "./config/oxlint-plugins/quadratic-buffer-concat.mjs" - }, - { - "name": "renderer-scrollbar-style", - "specifier": "./config/oxlint-plugins/renderer-scrollbar-style.mjs" - }, - { - "name": "native-chat-design-tokens", - "specifier": "./config/oxlint-plugins/native-chat-design-tokens.mjs" - }, - { - "name": "ime-enter-guard", - "specifier": "./config/oxlint-plugins/ime-enter-guard.mjs" - } - ], "categories": { "correctness": "error" }, "rules": { - "sort-comparator-performance/no-repeated-collator": "warn", - "app-store-performance/require-selector": "error", - "app-store-performance/no-identity-selector": "error", - "app-store-performance/no-fresh-selector-result": "error", "no-fallthrough": "error", "eslint/no-useless-call": "error", "oxc/no-accumulating-spread": "error", - "quadratic-buffer-concat/no-loop-carried-concat": "error", "react/jsx-no-duplicate-props": "error", "react/jsx-no-undef": "error", "react/no-children-prop": "error", @@ -117,7 +82,6 @@ ] } ], - "mobile-pairing/no-eager-qrcode-import": "error", "no-useless-return": "error", "prefer-template": "error", "unicorn/consistent-empty-array-spread": "error", @@ -183,8 +147,6 @@ { "files": ["src/renderer/src/**/*.{ts,tsx}"], "rules": { - "renderer-scrollbar-style/require-styled-vertical-scrollbar": "error", - "ime-enter-guard/require-ime-guard": "error", "no-restricted-properties": [ "error", { @@ -194,14 +156,6 @@ ] } }, - { - "files": ["src/renderer/src/components/native-chat/**/*.{ts,tsx}"], - "rules": { - "native-chat-design-tokens/no-arbitrary-font-size": "error", - "native-chat-design-tokens/no-softened-foreground": "error", - "native-chat-design-tokens/no-weakened-focus-ring": "error" - } - }, { "files": ["src/shared/**/*.ts"], "rules": { @@ -217,13 +171,7 @@ { "files": ["**/*.test.*", "**/*.spec.*", "**/*-benchmark.*"], "rules": { - "react/globals": "off", - "quadratic-buffer-concat/no-loop-carried-concat": "off", - "renderer-scrollbar-style/require-styled-vertical-scrollbar": "off", - "ime-enter-guard/require-ime-guard": "off", - "native-chat-design-tokens/no-arbitrary-font-size": "off", - "native-chat-design-tokens/no-softened-foreground": "off", - "native-chat-design-tokens/no-weakened-focus-ring": "off" + "react/globals": "off" } }, { diff --git a/config/ast-grep/rule-tests/flaky-test-patterns-test.yml b/config/ast-grep/rule-tests/flaky-test-patterns-test.yml new file mode 100644 index 00000000000..f1f92854d9b --- /dev/null +++ b/config/ast-grep/rule-tests/flaky-test-patterns-test.yml @@ -0,0 +1,83 @@ +id: test-no-wall-clock +valid: + - "const createdAt = Date.parse('2026-01-01T00:00:00Z')" + - "const fixed = new Date('2026-01-01T00:00:00Z')" + - 'const dir = join(tmpdir(), `run-${Date.now()}`)' + - | + vi.useFakeTimers() + const now = Date.now() +invalid: + - const started = Date.now() + - 'const record = { closedAt: new Date() }' +--- +id: test-no-real-time-wait +valid: + - await new Promise((resolve) => setTimeout(resolve, 0)) + - await new Promise((resolve) => setImmediate(resolve)) + - "await new Promise((resolve, reject) => { setTimeout(() => reject(new Error('timeout')), 5_000); emitter.once('ready', resolve) })" + - await vi.advanceTimersByTimeAsync(1_000) + - await expect.poll(() => read()).toBe(true) +invalid: + - await new Promise((resolve) => setTimeout(resolve, 50)) + - await new Promise((r) => setTimeout(r, 50)) + - await new Promise((resolve) => setTimeout(() => resolve(), DELAY_MS)) + - 'await new Promise((resolve) => { setTimeout(resolve, 20) })' + - await page.waitForTimeout(500) + - await sleep(100) + - await setTimeout(100) + - 'while (!ready()) { await sleep(0); await sleep(25) }' +--- +id: test-no-polling-loop +valid: + - 'for (const item of items) { await sleep(10) }' + - 'while (queue.length > 0) { await drain(queue.shift()) }' + - 'await expect.poll(() => read(), { timeout: 5_000 }).toBe(true)' +invalid: + - 'while (!ready()) { await new Promise((resolve) => setTimeout(resolve, 20)) }' + - 'do { value = read(); await sleep(50) } while (value === undefined)' + - 'while (!ready()) { await sleep(0); await sleep(25) }' + - 'for (let attempt = 0; attempt < 20; attempt++) { if (done()) break; await page.waitForTimeout(100) }' +--- +id: test-skip-needs-quarantine +valid: + - | + // quarantine: #1234 times out on Windows runners + it.skip('restores the session', async () => {}) + - | + // quarantine: #1234 flakes under load + // Re-enable once the PTY fixture is deterministic. + describe.skip('pty fixture', () => {}) + - test.skip(process.platform === 'win32', 'POSIX shell only') + - it.skipIf(isWindows)('uses POSIX paths', () => {}) +invalid: + - it.skip('restores the session', async () => {}) + - test.fixme('drag reorders tabs', async () => {}) + - test.describe.skip('layout', () => {}) + - it.only('focuses', () => {}) + - | + // TODO: re-enable + it.skip('restores the session', async () => {}) + - | + // quarantine: flaky + it.skip('restores the session', async () => {}) +--- +id: test-timeout-needs-reason +valid: + - "it('reads', async () => {})" + - | + // Spawns a real shell, which is slow on Windows runners. + it('spawns', async () => {}, 20_000) + - | + it('spawns', async () => {}, + // Spawns a real shell. + 20_000) + - | + // Builds the packaged app first. + test.setTimeout(300_000) + - 'await expect(locator).toBeVisible({ timeout: 10_000 })' +invalid: + - "it('spawns', async () => {}, 20_000)" + - "it('spawns', { timeout: 20_000 }, async () => {})" + - "describe('suite', { timeout: 60_000 }, () => {})" + - test.setTimeout(300_000) + - test.slow() diff --git a/config/ast-grep/rule-tests/native-chat-design-tokens-test.yml b/config/ast-grep/rule-tests/native-chat-design-tokens-test.yml new file mode 100644 index 00000000000..af4978c510f --- /dev/null +++ b/config/ast-grep/rule-tests/native-chat-design-tokens-test.yml @@ -0,0 +1,32 @@ +id: native-chat-no-arbitrary-font-size-tsx +valid: + - 'export const X = () =>

' + - 'export const X = () => 

'
+  - 'export const X = () => 
'
+  - 'export const X = () => 
'
+invalid:
+  - 'export const X = () => 
'
+  - 'export const X = () => 
'
+  - 'export const X = () => 
'
+  - "export const X = () => 
"
+  - 'export const X = () => 
'
+---
+id: native-chat-no-arbitrary-font-size
+valid:
+  - "const ROW = 'flex text-[var(--size)]'"
+invalid:
+  - "const ROW = 'flex text-[10px]'"
+---
+id: native-chat-no-softened-foreground-tsx
+valid:
+  - 'export const X = () => 
'
+  - 'export const X = () => 
'
+invalid:
+  - 'export const X = () => 
'
+  - 'export const X = () => 
'
+---
+id: native-chat-no-weakened-focus-ring-tsx
+valid:
+  - 'export const X = () =>