diff --git a/.changeset/sarif-lint-regression-compare.md b/.changeset/sarif-lint-regression-compare.md new file mode 100644 index 00000000..73d385f3 --- /dev/null +++ b/.changeset/sarif-lint-regression-compare.md @@ -0,0 +1,16 @@ +--- +'@gtbuchanan/cli': minor +--- + +Rework lint enforcement as a SARIF ratchet. Lint tasks are now +reporters: `lint:eslint` runs ESLint through its programmatic API +(`eslint` becomes an optional peer dependency), writing +`dist/sarif/eslint.sarif` and a stylish console report from one lint +run; it no longer fails on warnings (errors still fail) and accepts +only its supported argument surface (patterns, `--fix`, +`--ignore-pattern`). The new `gtb sarif compare` command fails only on +findings that are new relative to the merge base — reported in the +same stylish layout as lint output — and `gtb sarif baseline` +snapshots HEAD's logs so CI can seed a cross-PR baseline cache — see +the new `lint-regression.yml` and `lint-baseline.yml` reusable +workflows. diff --git a/.github/dependency-review-config.yml b/.github/dependency-review-config.yml index 13f997d3..f9eabaaa 100644 --- a/.github/dependency-review-config.yml +++ b/.github/dependency-review-config.yml @@ -1,5 +1,12 @@ --- allow-dependencies-licenses: + # jschardet LGPL-2.1-or-later: transitive dep of + # @microsoft/eslint-formatter-sarif, used only by its + # SARIF_ESLINT_EMBED feature (which the gtb lint pipeline never + # enables). Unmodified and installed separately from the registry — + # the clean LGPL case. Name-scoped so LGPL stays gated elsewhere; + # retire if sarif-sdk makes its embed deps optional. + - pkg:npm/jschardet # lightningcss MPL-2.0 (dev-only build tool): excluded by name rather # than allow-listing MPL globally, so future MPL deps still get gated. # All platform binaries enumerated (no wildcard support). diff --git a/.github/workflows/lint-baseline.yml b/.github/workflows/lint-baseline.yml new file mode 100644 index 00000000..e5771a55 --- /dev/null +++ b/.github/workflows/lint-baseline.yml @@ -0,0 +1,63 @@ +--- +env: + # GitHub Actions has no TTY, so force color; Actions renders the ANSI. + FORCE_COLOR: '1' + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +jobs: + seed: + name: Lint Baseline Seed + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - uses: gtbuchanan/tooling/.github/actions/mise-setup@main + + # Produce this commit's SARIF logs. `turbo-run` wires up the turbo + # remote cache, so the lint dedupes against the CI workflow's lint + # run of the same commit. + - uses: gtbuchanan/tooling/.github/actions/turbo-run@main + with: + task: lint + + # `pnpm-tasks` restores node_modules for `gtb sarif baseline`, since + # turbo-run skips install on a full lint cache hit. + - uses: gtbuchanan/tooling/.github/actions/pnpm-tasks@main + + - name: Snapshot baselines + run: >- + ${{ inputs.gtb-from-source && 'pnpm run gtb' || 'pnpm exec gtb' }} + sarif baseline + + - uses: actions/cache/save@v6 + with: + key: lint-baseline-${{ github.sha }} + path: | + **/dist/sarif/base/** + !**/node_modules/** + dist/sarif/base.ref + +name: Lint Baseline + +# Reusable: seed the cross-PR lint baseline cache from a default-branch +# commit. Any future PR's merge base is that commit itself, so its +# baseline is just the commit's own lint output — snapshot it via +# `gtb sarif baseline` and save it keyed on the SHA. Caches created on +# the default branch are readable from every PR, so the +# `lint-regression.yml` compare restores it and skips merge-base +# production entirely. The gate stays self-sufficient without it — a +# cache miss just means the compare lints the merge base itself. +on: + workflow_call: + inputs: + gtb-from-source: + default: false + description: >- + Run gtb from the workspace source (`pnpm run gtb`) instead of + the installed bin (`pnpm exec gtb`). Set true only by the repo + that vendors `@gtbuchanan/cli` as a workspace package (tooling + itself). Consumers leave it false. + type: boolean + +permissions: + contents: read diff --git a/.github/workflows/lint-regression.yml b/.github/workflows/lint-regression.yml new file mode 100644 index 00000000..aa8d4d02 --- /dev/null +++ b/.github/workflows/lint-regression.yml @@ -0,0 +1,228 @@ +--- +env: + # GitHub Actions has no TTY, so force color; Actions renders the ANSI. + FORCE_COLOR: '1' + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + +jobs: + run: + name: Lint Regression + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + with: + # Depth 2 resolves both parents of the PR merge ref the + # checkout lands on; the first parent is the target branch + # head — the merge base of the merged checkout — so no branch + # fetch or merge-base resolution is needed. + fetch-depth: 2 + + - uses: gtbuchanan/tooling/.github/actions/mise-setup@main + + # A label only vouches for the commits it was applied to, so it's + # stale on the first attempt of a synchronize run (a new push) or + # a reopened run (labels survive closure, and pushes to a closed + # PR emit no synchronize events, so a reopened head may have + # moved under the label). Re-runs replay the original payload but + # bump the attempt counter, so a label applied after the push + # stays fresh — re-running the job is exactly how it takes effect + # (see Enforce). Computed once so the dismissal below and the + # Enforce branch can't drift apart. + - env: + STALE: >- + ${{ contains(fromJSON('["synchronize", "reopened"]'), github.event.action) && + github.run_attempt == '1' }} + id: label + name: Classify override label freshness + run: echo "stale=$STALE" >> "$GITHUB_OUTPUT" + + # Fork PRs run with a read-only token that can't edit labels; + # Enforce discounts a stale label anyway, so skipping the + # dismissal there costs nothing. + - env: + GH_TOKEN: ${{ github.token }} + LABEL: ${{ inputs.override-label }} + PR: ${{ github.event.pull_request.number }} + if: >- + steps.label.outputs.stale == 'true' && + contains(github.event.pull_request.labels.*.name, inputs.override-label) && + !github.event.pull_request.head.repo.fork + name: Dismiss stale regression acceptance + run: gh pr edit "$PR" --remove-label "$LABEL" + + # Produce this commit's SARIF logs. `turbo-run` wires up the turbo + # remote cache, so the lint dedupes against the CI workflow's lint + # run of the same commit. + - uses: gtbuchanan/tooling/.github/actions/turbo-run@main + with: + task: lint + + # `pnpm-tasks` restores node_modules for `gtb sarif compare`, since + # turbo-run skips install on a full lint cache hit. + - uses: gtbuchanan/tooling/.github/actions/pnpm-tasks@main + + # The checkout must be the merge ref (a two-parent commit): + # re-checking-out the PR head would make HEAD^1 the head's own + # parent, silently gating against the wrong commit. The guard + # fails the step with an explicit error rather than proceeding. + - id: merge-base + name: Resolve the merge base (merge ref first parent) + run: | + git rev-parse --verify HEAD^2 > /dev/null 2>&1 || { + echo '::error::Checkout is not the PR merge ref; gating' \ + 'against a PR-head parent would use the wrong baseline' + exit 1 + } + echo "sha=$(git rev-parse HEAD^1)" >> "$GITHUB_OUTPUT" + + # Restore baselines produced by an earlier run against the same + # merge base; the compare's stamp check skips the worktree lint on + # a hit. Exact key only — a near-miss baseline is a wrong baseline + # (the stamp check would reject it anyway). + - uses: actions/cache@v6 + with: + key: lint-baseline-${{ steps.merge-base.outputs.sha }} + path: | + **/dist/sarif/base/** + !**/node_modules/** + dist/sarif/base.ref + + - continue-on-error: true + env: + BASE_SHA: ${{ steps.merge-base.outputs.sha }} + id: compare + name: Compare lint results against the merge base + run: | + ${{ inputs.gtb-from-source && 'pnpm run gtb' || 'pnpm exec gtb' }} \ + sarif compare --base-sha "$BASE_SHA" 2>&1 \ + | tee lint-regression.txt + exit "${PIPESTATUS[0]}" + + # The findings land in the run's job summary rather than a PR + # comment: a summary is pinned to its run, so it can't go stale + # after a green re-run or collide with other bot comments, and it + # needs no write token, so fork PRs get the full report too. + - env: + LABEL: ${{ inputs.override-label }} + if: steps.compare.outcome == 'failure' + name: Report new violations in the job summary + run: | + { + echo '### New lint violations relative to the merge base' + echo + echo '```text' + # FORCE_COLOR makes the report carry ANSI codes for the log + # view; the summary's code fence renders them literally, so + # strip them here. + sed 's/\x1b\[[0-9;]*m//g' lint-regression.txt + echo '```' + echo + echo 'Fix the new findings or suppress them in-source with' \ + 'a reason (suppressed findings are exempt). For a bulk' \ + 'introduction (e.g. a dependency bump adding a rule), a' \ + "maintainer can apply the \`$LABEL\`" \ + 'label and re-run this job to accept them.' + } >> "$GITHUB_STEP_SUMMARY" + + # Run-but-neutral override: the label lets the check pass, but the + # job summary above still records what was accepted. Labels are + # read live (not from the event payload) so the accept flow is: + # apply the label, then re-run this job — a re-run replays the + # original payload, which predates the label. The label doesn't + # count when the freshness step classified it stale — the + # dismissal just dropped it (or would have, for forks). + # GitHub has no per-label permission rules — anyone with triage + # can apply any label — so the label is honored only when its most + # recent applier holds `override-role` or higher; anything + # unresolvable fails closed. Known residual: cancelling a run + # before its dismissal step and then re-running preserves a stale + # label, but that needs write access and deliberate timing from + # someone already looking at the PR. + - env: + GH_TOKEN: ${{ github.token }} + LABEL: ${{ inputs.override-label }} + LABEL_STALE: ${{ steps.label.outputs.stale }} + OVERRIDE_ROLE: ${{ inputs.override-role }} + PR: ${{ github.event.pull_request.number }} + REPO: ${{ github.repository }} + if: steps.compare.outcome == 'failure' + name: Enforce + run: | + if [ "$LABEL_STALE" != 'true' ] && + gh pr view "$PR" --json labels --jq '.labels[].name' | + grep -Fxq "$LABEL"; then + applier=$(gh api "repos/$REPO/issues/$PR/events" --paginate \ + --jq '.[] | select(.event == "labeled") + | "\(.label.name)\t\(.actor.login)"' | + awk -F '\t' -v l="$LABEL" '$1 == l { a = $2 } END { print a }' \ + || true) + role=$(gh api "repos/$REPO/collaborators/$applier/permission" \ + --jq '.role_name' || true) + case "$OVERRIDE_ROLE" in + admin) allowed='admin' ;; + write) allowed='admin maintain write' ;; + *) allowed='admin maintain' ;; + esac + case " $allowed " in + *" $role "*) + echo "'$LABEL' applied by $applier ($role); accepting" \ + 'the new findings' + exit 0 + ;; + *) + echo "'$LABEL' applied by ${applier:-unknown}" \ + "(${role:-unknown}) — needs $OVERRIDE_ROLE or higher;" \ + 'ignoring the label' + ;; + esac + fi + exit 1 + +name: Lint Regression + +# Reusable: fail a PR only on lint violations that are new relative to +# its merge base. `gtb sarif compare --base` lints the merge base +# in a temporary git worktree to build the SARIF baseline, then +# `sarif-multitool` classifies each HEAD result as new or matched — +# pre-existing (accepted) violations never block. The routine response +# to new findings is fixing or suppressing them in-source (suppressed +# findings are gate-exempt); for bulk introductions (e.g. a dependency +# bump adding a rule) the override label turns a failing compare into a +# pass for one merge — apply it, then re-run the failed job — while the +# failing run's job summary keeps the accepted violations on record. +# The label counts only when applied by `override-role` or higher, +# since GitHub has no per-label permission rules of its own. The +# cross-PR baseline cache is seeded by the `lint-baseline.yml` reusable +# on default-branch pushes. +on: + workflow_call: + inputs: + gtb-from-source: + default: false + description: >- + Run gtb from the workspace source (`pnpm run gtb`) instead of + the installed bin (`pnpm exec gtb`). Set true only by the repo + that vendors `@gtbuchanan/cli` as a workspace package (tooling + itself). Consumers leave it false. + type: boolean + override-label: + default: accepted-lint-regression + description: >- + PR label that accepts the new violations for one merge (takes + effect on a job re-run; dismissed automatically on every new + push; honored only when applied by `override-role` or + higher). The escape hatch for bulk introductions — the + routine path is fixing or suppressing findings in-source. + type: string + override-role: + default: maintain + description: >- + Minimum repository role (`write`, `maintain`, or `admin`) + the override label's applier must hold for it to count. + type: string + +# `pull-requests: write` covers the label dismissal; everything else +# reads. +permissions: + contents: read + pull-requests: write diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index b2748e7c..8ae690f6 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -17,6 +17,12 @@ jobs: with: config-file: .github/dependency-review-config.yml + lint-regression: + name: Lint + uses: ./.github/workflows/lint-regression.yml + with: + gtb-from-source: true + pre-commit: name: Pre-Commit # This repo's eslint step shells out to `pnpm exec eslint`, so hk diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a85d7c2b..b1b547ea 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,6 +18,16 @@ jobs: secrets: inherit uses: ./.github/workflows/ci.yml + # Seed the cross-PR lint baseline cache from this commit's own lint + # output. `needs: ci` orders it after the CI gate; with a turbo + # remote cache configured its lint also becomes a cache hit. + lint-baseline: + name: Lint + needs: ci + uses: ./.github/workflows/lint-baseline.yml + with: + gtb-from-source: true + name: Release # The main-branch pipeline: re-run CI as a gate, then CD (version + diff --git a/AGENTS.md b/AGENTS.md index 75412960..5936ba8f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -25,9 +25,11 @@ mise.toml — Pin dev-tool versions for local + CI; postinstall hoo changeset-check.yml — Reusable: verify a changeset exists ci.yml — Reusable: build + slow + e2e + coverage dependency-review.yml — Reusable: scan dep changes (vulns + licenses) - pr.yml — Pipeline (PR): ci + changeset + deps + pre-commit + lint-baseline.yml — Reusable: seed the cross-PR lint baseline cache (default branch) + lint-regression.yml — Reusable: fail PRs only on lint violations new vs. the merge base (SARIF diff) + pr.yml — Pipeline (PR): ci + changeset + deps + lint regression + pre-commit pre-commit.yml — Reusable: run hk hooks - release.yml — Pipeline (push): CI gate + CD + release.yml — Pipeline (push): CI gate + CD + lint baseline seed packages/ cli/ — @gtbuchanan/cli (gtb build CLI for consumers) skills/ — Authored Agent Skills deployed by `gtb task deploy:skills` @@ -88,6 +90,44 @@ coverage, setupFiles, and mock reset. - **Vitest** — Per-package `vitest.config.ts` using `configurePackage()` from `@gtbuchanan/vitest-config/configure`. +### SARIF lint baselining + +Lint enforcement is a ratchet: PRs may not introduce _new_ findings, +while pre-existing (accepted) ones never block. Consumer-facing usage +(override label flow, workflow wiring) lives in `README.md`; the +invariants agents need: + +- **Reporters, not gates.** `lint:eslint` runs ESLint through its + programmatic API (`@gtbuchanan/cli` declares `eslint` as an optional + peer), so one lint feeds both `dist/sarif/eslint.sarif` (a turbo + task output) and the stylish console report — the CLI's + single-`--format` limit is why the API replaced it. Warnings never + fail it — errors (parse/config breakage under the warnings-only + convention) still do, and only after the SARIF log is written. Any + reporter that drops a `.sarif` into `dist/sarif/` is gated + with no extra wiring. +- **`gtb sarif compare` is the gate.** Pairs every + `dist/sarif/*.sarif` in each lint cwd with + `dist/sarif/base/.sarif` via + `sarif-multitool match-results-forward`, failing only on results + classified `new` (fingerprint/content matching, so moved findings + stay matched). + `--base ` lints the merge base in a throwaway git worktree to + produce baselines (`gtb sarif compare --base origin/main` locally); + CI passes `--base-sha` with the PR merge ref's first parent, which + _is_ the merge base on the merged checkout. The + `dist/sarif/base.ref` stamp skips production when the on-disk + baselines are current; `gtb sarif baseline` snapshots HEAD's own + logs for the default-branch cache seed (`lint-baseline.yml`). A + missing baseline is an empty baseline, not a pass — a new reporter + can't slip findings in silently. In-source-suppressed findings are + gate-exempt but stay in the logs. +- **Changed-file enforcement stays local.** The hk pre-commit `eslint` + step keeps `--max-warnings=0` on staged files; the ratchet only + governs what lands. Findings accepted via the override label or a + suppression enter the baseline once merged and surface in every + SARIF log until paid down. + ### Pre-commit hooks - **hk** — Rust pre-commit hook runner configured in Pkl (`hk.pkl`). No @@ -203,12 +243,13 @@ Two **pipeline** workflows own this repo's triggers and define the work as peer jobs, each calling a single-concern **reusable** workflow: - **`pr.yml`** (on `pull_request`) — jobs `CI`, `Changeset`, - `Dependencies`, `Pre-Commit`. -- **`release.yml`** (on `push` to main) — jobs `CI` (gate), `CD` - (`needs: ci`). `CI` and `CD` are peers. + `Dependencies`, `Lint`, `Pre-Commit`. +- **`release.yml`** (on `push` to main) — jobs `CI` (gate), `CD` and + `Lint` (baseline seed), both `needs: ci`. The reusables (`workflow_call`-only): `ci.yml`, `cd.yml`, -`changeset-check.yml`, `dependency-review.yml`, `pre-commit.yml`. +`changeset-check.yml`, `dependency-review.yml`, `lint-baseline.yml`, +`lint-regression.yml`, `pre-commit.yml`. Consumers copy `pr.yml` / `release.yml`, swapping `./` for `gtbuchanan/tooling/.github/workflows/@main`: @@ -220,7 +261,8 @@ on: branches: [main] permissions: contents: read - pull-requests: write # Dependencies posts a PR comment + # Dependencies posts a PR comment; Lint dismisses override labels + pull-requests: write jobs: ci: name: CI @@ -232,6 +274,9 @@ jobs: dependencies: name: Dependencies uses: gtbuchanan/tooling/.github/workflows/dependency-review.yml@main + lint-regression: + name: Lint + uses: gtbuchanan/tooling/.github/workflows/lint-regression.yml@main pre-commit: name: Pre-Commit uses: gtbuchanan/tooling/.github/workflows/pre-commit.yml@main @@ -258,6 +303,10 @@ jobs: id-token: write # npm trusted publishing (OIDC) uses: gtbuchanan/tooling/.github/workflows/cd.yml@main secrets: inherit + lint-baseline: + name: Lint + needs: ci + uses: gtbuchanan/tooling/.github/workflows/lint-baseline.yml@main ``` **Naming / required checks.** Branch protection keys on the **leaf job @@ -357,6 +406,24 @@ through `package.json` scripts backed by `gtb` leaf commands. to ecosystems it indexes (npm + Actions here), so mise tools and `hk.pkl` steps aren't covered — Renovate's managers handle those independently. +- **`lint-baseline.yml`** — Seeds the cross-PR lint baseline cache + from a default-branch commit (its own lint output, snapshotted via + `gtb sarif baseline` and cached on the SHA). An optimization only: + on a cache miss the gate lints the merge base itself. +- **`lint-regression.yml`** — Fails a PR only on lint violations that + are new relative to its merge base (the ratchet gate; see the SARIF + lint baselining section). Lints HEAD, then runs + `gtb sarif compare --base-sha` with the merge ref's first parent, + which restores or produces the baseline and diffs the SARIF logs via + `sarif-multitool`. New findings land in the run's job summary + (fork-PR safe — no write token needed) and are routinely fixed or + suppressed in-source; for bulk introductions the override label + (default `accepted-lint-regression`, dismissed on every new push, + honored only when applied by `override-role`+ — default `maintain`) + turns a failing compare into a pass for one merge — apply it, then + re-run the failed job (labels are read live, so the replayed event + payload doesn't matter). Caller must grant `pull-requests: write` + for the label dismissal. - **`pre-commit.yml`** — Runs the `hk:base` mise task on PR changed files (hk resolved from mise). The `use-pnpm` input (default `false`) opts into `pnpm install` for steps that shell out to the diff --git a/README.md b/README.md index 0af17959..b5f52bf6 100644 --- a/README.md +++ b/README.md @@ -32,7 +32,8 @@ on: branches: [main] permissions: contents: read - pull-requests: write # Dependencies posts a PR comment + # Dependencies posts a PR comment; Lint dismisses override labels + pull-requests: write jobs: ci: name: CI @@ -44,6 +45,9 @@ jobs: dependencies: name: Dependencies uses: gtbuchanan/tooling/.github/workflows/dependency-review.yml@main + lint-regression: + name: Lint + uses: gtbuchanan/tooling/.github/workflows/lint-regression.yml@main pre-commit: name: Pre-Commit uses: gtbuchanan/tooling/.github/workflows/pre-commit.yml@main @@ -70,6 +74,10 @@ jobs: id-token: write # npm trusted publishing (OIDC) uses: gtbuchanan/tooling/.github/workflows/cd.yml@main secrets: inherit + lint-baseline: + name: Lint + needs: ci + uses: gtbuchanan/tooling/.github/workflows/lint-baseline.yml@main ``` The reusable workflows each job calls: @@ -80,6 +88,8 @@ The reusable workflows each job calls: | `cd.yml` | changesets version + publish (OIDC) | | `changeset-check.yml` | Verify changeset exists | | `dependency-review.yml` | Scan PR dep changes for vulns + licenses | +| `lint-baseline.yml` | Seed the cross-PR lint baseline cache | +| `lint-regression.yml` | Fail PRs only on new lint violations | | `pre-commit.yml` | Run hk hooks on changed files | Permissions narrow down the call chain but never elevate, so each @@ -172,6 +182,47 @@ jobs: fail-on-severity: low ``` +### Lint regression gate + +`lint-regression.yml` fails a PR only on lint violations that are +_new_ relative to its merge base — pre-existing findings never block. +Lint tasks are reporters that write SARIF logs (`dist/sarif/*.sarif`); +`gtb sarif compare` diffs them against the merge base's logs (linting +the merge base in a throwaway git worktree on a cache miss) and fails +only on findings classified as new. The findings are reported in the +run's job summary. + +The routine response is to fix the new findings or suppress them +in-source with a reason (suppressed findings are exempt from the +gate). For bulk introductions — e.g. a dependency bump shipping a new +rule, where per-instance suppression would be noise — a maintainer +applies the override label (`accepted-lint-regression` by default) and +re-runs the failed job to accept the findings for that merge. The +label is dismissed automatically on every new push, and it only counts +when its applier holds the `override-role` repository role or higher +(default `maintain`) — GitHub has no per-label permission rules of its +own. + +`lint-baseline.yml` seeds a cross-PR baseline cache from each +default-branch commit (the `Lint` job in the release pipeline above). +It's an optimization, not a requirement — on a cache miss the gate +lints the merge base itself. + +Customize via `with:` on the `Lint` job: + +```yaml +jobs: + lint-regression: + name: Lint + uses: gtbuchanan/tooling/.github/workflows/lint-regression.yml@main + with: + override-label: accepted-lint-regression + override-role: maintain +``` + +The PR pipeline must grant `pull-requests: write` (shown in the setup +above) so the gate can dismiss stale override labels. + ### Build pipeline conventions Turborepo manages task orchestration. The task graph is defined in diff --git a/packages/cli/e2e/sarif-compare.test.ts b/packages/cli/e2e/sarif-compare.test.ts new file mode 100644 index 00000000..7a21be95 --- /dev/null +++ b/packages/cli/e2e/sarif-compare.test.ts @@ -0,0 +1,110 @@ +import path from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { type ProjectFixture, createProjectFixture } from '@gtbuchanan/test-utils'; +import { describe, it } from 'vitest'; + +const createFixture = (): ProjectFixture => + createProjectFixture({ + packageName: '@gtbuchanan/cli', + }); + +const jsonIndent = 2; + +interface SarifViolation { + readonly line: number; + readonly message: string; + readonly ruleId: string; + readonly snippet: string; +} + +/** Builds a SARIF log in the shape the `lint:eslint` reporter emits. */ +const sarifLog = (fileUri: string, violations: readonly SarifViolation[]): string => + `${JSON.stringify({ + $schema: 'https://json.schemastore.org/sarif-2.1.0-rtm.5', + runs: [{ + artifacts: [{ location: { uri: fileUri } }], + results: violations.map(violation => ({ + level: 'warning', + locations: [{ + physicalLocation: { + artifactLocation: { index: 0, uri: fileUri }, + region: { + snippet: { text: violation.snippet }, + startColumn: 1, + startLine: violation.line, + }, + }, + }], + message: { text: violation.message }, + ruleId: violation.ruleId, + })), + tool: { driver: { name: 'ESLint', rules: [] } }, + }], + version: '2.1.0', + }, undefined, jsonIndent)}\n`; + +const existing: SarifViolation = { + line: 2, + message: "'unused' is assigned a value but never used.", + ruleId: 'no-unused-vars', + snippet: 'const unused = 1;', +}; + +const source = 'export const app = () => {\n const unused = 1;\n console.log("hi");\n};\n'; + +describe.concurrent('gtb sarif compare', () => { + it('passes when every violation is in the baseline', async ({ expect }) => { + using fixture = createFixture(); + const file = fixture.writeFile(path.join('src', 'app.js'), source); + const uri = pathToFileURL(file).href; + fixture.writeFile( + path.join('dist', 'sarif', 'base', 'eslint.sarif'), + sarifLog(uri, [existing]), + ); + fixture.writeFile(path.join('dist', 'sarif', 'eslint.sarif'), sarifLog(uri, [existing])); + + const result = await fixture.run('gtb', ['sarif', 'compare']); + + expect(result).toMatchObject({ exitCode: 0 }); + expect(result.stdout).toContain('No new findings'); + }); + + it('fails on a violation missing from the baseline', async ({ expect }) => { + using fixture = createFixture(); + const file = fixture.writeFile(path.join('src', 'app.js'), source); + const uri = pathToFileURL(file).href; + const added: SarifViolation = { + line: 3, + message: 'Unexpected console statement.', + ruleId: 'no-console', + snippet: 'console.log("hi");', + }; + fixture.writeFile( + path.join('dist', 'sarif', 'base', 'eslint.sarif'), + sarifLog(uri, [existing]), + ); + fixture.writeFile( + path.join('dist', 'sarif', 'eslint.sarif'), + sarifLog(uri, [existing, added]), + ); + + const result = await fixture.run('gtb', ['sarif', 'compare']); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain('no-console'); + expect(result.stderr).not.toContain('no-unused-vars'); + }); + + it('treats every finding as new when no baseline exists', async ({ expect }) => { + using fixture = createFixture(); + const file = fixture.writeFile(path.join('src', 'app.js'), source); + const uri = pathToFileURL(file).href; + fixture.writeFile(path.join('dist', 'sarif', 'eslint.sarif'), sarifLog(uri, [existing])); + + const result = await fixture.run('gtb', ['sarif', 'compare']); + + expect(result.exitCode).not.toBe(0); + expect(result.stderr).toContain('No baseline'); + expect(result.stderr).toContain('no-unused-vars'); + }); +}); diff --git a/packages/cli/package.json b/packages/cli/package.json index ba069434..251885f3 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -27,6 +27,8 @@ "typecheck:ts": "pnpm run gtb task typecheck:ts" }, "dependencies": { + "@microsoft/eslint-formatter-sarif": "catalog:", + "@microsoft/sarif-multitool": "catalog:", "citty": "catalog:", "cross-spawn": "catalog:", "find-up-simple": "catalog:", @@ -42,14 +44,19 @@ "@gtbuchanan/test-utils": "workspace:*", "@gtbuchanan/vitest-config": "workspace:*", "@types/cross-spawn": "catalog:", - "@types/hosted-git-info": "catalog:" + "@types/hosted-git-info": "catalog:", + "eslint": "catalog:" }, "peerDependencies": { + "eslint": "^10.0.0", "skills": "^1.5.1", "skills-npm": "^1.1.1", "typescript": ">=5.0.0" }, "peerDependenciesMeta": { + "eslint": { + "optional": true + }, "skills-npm": { "optional": true } diff --git a/packages/cli/src/commands/index.ts b/packages/cli/src/commands/index.ts index fd394176..ca25002f 100644 --- a/packages/cli/src/commands/index.ts +++ b/packages/cli/src/commands/index.ts @@ -3,6 +3,7 @@ import { hk } from './root/hk.ts'; import { rootNames } from './root/names.ts'; import { prepare } from './root/prepare.ts'; import { publish } from './root/publish.ts'; +import { sarif } from './root/sarif.ts'; import { sync } from './root/sync.ts'; import { turbo } from './root/turbo.ts'; import { verify } from './root/verify.ts'; @@ -20,6 +21,7 @@ export const main = defineCommand({ [rootNames.hk]: hk, [rootNames.prepare]: prepare, [rootNames.publish]: publish, + [rootNames.sarif]: sarif, [rootNames.sync]: sync, [rootNames.turbo]: turbo, [rootNames.verify]: verify, diff --git a/packages/cli/src/commands/root/names.ts b/packages/cli/src/commands/root/names.ts index f8545277..38d55e6f 100644 --- a/packages/cli/src/commands/root/names.ts +++ b/packages/cli/src/commands/root/names.ts @@ -3,6 +3,7 @@ export const rootNames = { hk: 'hk', prepare: 'prepare', publish: 'publish', + sarif: 'sarif', sync: 'sync', turbo: 'turbo', verify: 'verify', diff --git a/packages/cli/src/commands/root/sarif.ts b/packages/cli/src/commands/root/sarif.ts new file mode 100644 index 00000000..036fcd11 --- /dev/null +++ b/packages/cli/src/commands/root/sarif.ts @@ -0,0 +1,56 @@ +import { defineCommand } from 'citty'; +import { executeSarifBaseline, executeSarifCompare } from '../../lib/sarif-compare.ts'; +import { rootNames } from './names.ts'; + +/** + * `gtb sarif compare` — gates on regressions by diffing the current + * SARIF logs against baseline logs from the merge base. A user command + * rather than a turbo task: it consumes two generations of the same + * task outputs, which turbo's task model can't express, and its result + * depends on git state turbo can't hash. + */ +const compare = defineCommand({ + args: { + 'base': { + description: + 'Git ref to diff against; lints its merge base with HEAD in a ' + + 'temporary worktree to produce the baseline (e.g. origin/main)', + type: 'string', + }, + 'base-sha': { + description: + 'Exact baseline commit, skipping merge-base resolution; CI ' + + "passes the PR merge ref's first parent (git rev-parse HEAD^1)", + type: 'string', + }, + }, + meta: { + description: 'Fail when SARIF findings are new relative to the baseline', + name: 'compare', + }, + run: ({ args }) => + executeSarifCompare({ baseRef: args.base, baseSha: args['base-sha'] }), +}); + +/** + * `gtb sarif baseline` — snapshots HEAD's SARIF logs as the compare + * baseline. Run on the default branch (where any future PR's merge base + * is HEAD itself) so CI can cache the result for PR `sarif compare` + * runs to restore. + */ +const baseline = defineCommand({ + meta: { + description: "Snapshot HEAD's SARIF logs as the compare baseline", + name: 'baseline', + }, + run: () => executeSarifBaseline(), +}); + +/** `gtb sarif` — SARIF baselining: regression compare and baseline snapshot. */ +export const sarif = defineCommand({ + meta: { + description: 'Compare or snapshot SARIF static-analysis baselines', + name: rootNames.sarif, + }, + subCommands: { baseline, compare }, +}); diff --git a/packages/cli/src/commands/root/turbo.ts b/packages/cli/src/commands/root/turbo.ts index 8912298f..1dbce0a4 100644 --- a/packages/cli/src/commands/root/turbo.ts +++ b/packages/cli/src/commands/root/turbo.ts @@ -1,86 +1,8 @@ -import { existsSync } from 'node:fs'; -import path from 'node:path'; import { defineCommand } from 'citty'; import { run } from '../../lib/process.ts'; +import { planTurboInvocation } from '../../lib/turbo-invocation.ts'; import { rootNames } from './names.ts'; -/** - * Setup help shown when the global turbo binary is missing on Android. - * Termux ships a native turbo via its package registry; the npm - * `@turbo/linux-` workaround is no longer needed and the - * launcher in `node_modules/.bin/turbo` rejects - * `process.platform === 'android'` upfront. - * - * Native android binaries are not coming from upstream — vercel/turborepo#5616 - * was closed as "not planned" — so the Termux-pkg turbo (Bionic-built - * against `aarch64-linux-android`) is the supported path. - */ -const androidSetupHelp = ` -gtb turbo: the global turbo binary is not installed. - -On Android (Termux), gtb turbo execs the native turbo from the Termux -package registry instead of the npm-distributed Linux binary. The -node_modules launcher refuses to start when process.platform === 'android'. - -Install it from the Termux registry: - - pkg install turbo - -If your Termux prefix is non-standard, set $PREFIX before running. -`.trimStart(); - -/** - * Resolves the global Termux-pkg-installed turbo binary. Honors - * Termux's $PREFIX env var; falls back to the standard install path - * when $PREFIX is unset (matching the convention used by - * `@gtbuchanan/pnpm-termux-shim`). - */ -const resolveAndroidTurboBinary = (): string | undefined => { - const prefix = process.env['PREFIX'] ?? '/data/data/com.termux/files/usr'; - const candidate = path.join(prefix, 'bin', 'turbo'); - return existsSync(candidate) ? candidate : undefined; -}; - -/** Discriminated plan for how to invoke turbo from the current host. */ -export type TurboInvocation = - | { readonly kind: 'error'; readonly message: string } - | { readonly kind: 'spawn'; readonly args: readonly string[]; readonly bin: string }; - -/** Inputs to {@link planTurboInvocation}. */ -export interface PlanTurboInvocationOptions { - readonly platform: string; - readonly rawArgs: readonly string[]; - readonly resolveAndroidBinary?: () => string | undefined; -} - -/** - * Computes the turbo invocation plan for a given host. On Android - * resolves the Termux-pkg turbo binary directly (bypassing the - * node_modules launcher, which rejects `android` upfront). On every - * other platform delegates to the launcher on PATH so its native - * install behavior is preserved. - * - * The Termux-pkg turbo is Bionic-built, so its child-process spawns - * honor Termux's `LD_PRELOAD` shebang rewriter and resolve - * `#!/usr/bin/env ` correctly. The companion - * `@gtbuchanan/pnpm-termux-shim` package is retained defensively in - * case turbo reintroduces a glibc npm distribution, or another glibc - * binary in the graph needs to spawn `pnpm`. - */ -export const planTurboInvocation = ( - options: PlanTurboInvocationOptions, -): TurboInvocation => { - if (options.platform !== 'android') { - return { args: [...options.rawArgs], bin: 'turbo', kind: 'spawn' }; - } - const resolveBin = options.resolveAndroidBinary ?? resolveAndroidTurboBinary; - const resolved = resolveBin(); - if (resolved === undefined) { - return { kind: 'error', message: androidSetupHelp }; - } - return { args: [...options.rawArgs], bin: resolved, kind: 'spawn' }; -}; - /** `gtb turbo` — runs turbo, with an Android (Termux) escape hatch. */ export const turbo = defineCommand({ meta: { diff --git a/packages/cli/src/commands/task/lint-eslint.ts b/packages/cli/src/commands/task/lint-eslint.ts index b5877176..07bf23d4 100644 --- a/packages/cli/src/commands/task/lint-eslint.ts +++ b/packages/cli/src/commands/task/lint-eslint.ts @@ -1,19 +1,17 @@ import { defineCommand } from 'citty'; -import { run } from '../../lib/process.ts'; +import { executeLintEslint } from '../../lib/lint-eslint.ts'; -/** Runs ESLint with caching and zero-warning threshold. */ +/** + * Runs ESLint via its programmatic API (see `lib/lint-eslint.ts` for + * the reporter-not-gate semantics): one lint feeds both the SARIF log + * consumed by `gtb sarif compare` and the stylish console report. + * Enforcement lives in the changed-files pre-commit step locally and in + * the compare (new-findings-only) in CI. + */ export const lintEslint = defineCommand({ meta: { - description: 'Run ESLint with caching and --max-warnings=0', + description: 'Run ESLint with caching, reporting to dist/sarif/eslint.sarif', name: 'lint:eslint', }, - run: async ({ rawArgs }) => { - await run('eslint', { - args: [ - '--cache', '--cache-location', 'dist/.eslintcache', - '--max-warnings=0', - ...rawArgs, - ], - }); - }, + run: ({ rawArgs }) => executeLintEslint(rawArgs), }); diff --git a/packages/cli/src/lib/finding-report.ts b/packages/cli/src/lib/finding-report.ts new file mode 100644 index 00000000..55a6792b --- /dev/null +++ b/packages/cli/src/lib/finding-report.ts @@ -0,0 +1,91 @@ +/** Parts of a lint finding the console report renders. */ +export interface Finding { + readonly column: number | undefined; + readonly level: string; + readonly line: number | undefined; + readonly message: string; + readonly ruleId: string; + readonly uri: string; +} + +/** Style names the report applies (a subset of `util.styleText` formats). */ +export type FindingStyleName = 'bold' | 'dim' | 'red' | 'underline' | 'yellow'; + +/** + * Applies terminal styling to a report part. `util.styleText` satisfies + * this directly, keeping color detection (TTY, `NO_COLOR`, + * `FORCE_COLOR`) with the caller and the renderer pure. + */ +export type StyleText = (format: FindingStyleName, text: string) => string; + +/** Identity style: plain (uncolored) report text. */ +export const plainText: StyleText = (_format, text) => text; + +const width = (parts: readonly string[]): number => + Math.max(0, ...parts.map(part => part.length)); + +/** + * `line:col` with the line right-aligned so the colons line up within a + * file section (the ESLint `stylish` convention). Empty when the + * finding has no line (e.g. a SARIF result without a region). + */ +const formatPosition = (finding: Finding, lineWidth: number): string => { + if (finding.line === undefined) return ''; + const line = String(finding.line).padStart(lineWidth); + return finding.column === undefined ? line : `${line}:${String(finding.column)}`; +}; + +const count = (total: number, noun: string): string => + `${String(total)} ${noun}${total === 1 ? '' : 's'}`; + +const formatSection = ( + uri: string, + group: readonly Finding[], + style: StyleText, +): string => { + const lineWidth = width(group.map(finding => + finding.line === undefined ? '' : String(finding.line))); + const positions = group.map(finding => formatPosition(finding, lineWidth)); + const positionWidth = width(positions); + const levelWidth = width(group.map(finding => finding.level)); + const messageWidth = width(group.map(finding => finding.message)); + const rows = group.map((finding, index) => { + const position = positions[index] ?? ''; + const cells = [ + ...positionWidth === 0 + ? [] + : [style('dim', position.padEnd(positionWidth))], + style( + finding.level === 'error' ? 'red' : 'yellow', + finding.level.padEnd(levelWidth), + ), + finding.message.padEnd(messageWidth), + style('dim', finding.ruleId), + ]; + return ` ${cells.join(' ')}`; + }); + return [style('underline', uri), ...rows].join('\n'); +}; + +/** + * Renders findings in the ESLint `stylish` layout: per-file sections + * with aligned position/level/message columns, then a problem-count + * summary. Shared by the `lint:eslint` console output and the SARIF + * compare's new-findings report so lint output and the CI gate read + * identically. + */ +export const formatFindingReport = ( + findings: readonly Finding[], + style: StyleText = plainText, +): string => { + if (findings.length === 0) return ''; + const sections = [...Map.groupBy(findings, finding => finding.uri)] + .map(([uri, group]) => formatSection(uri, group, style)); + const errors = findings.filter(finding => finding.level === 'error').length; + const summary = style('bold', style( + errors > 0 ? 'red' : 'yellow', + `✖ ${count(findings.length, 'problem')} ` + + `(${count(errors, 'error')}, ${count(findings.length - errors, 'warning')})`, + )); + return `${sections.join('\n\n')}\n\n${summary}`; +}; diff --git a/packages/cli/src/lib/internal-rule-id.ts b/packages/cli/src/lib/internal-rule-id.ts new file mode 100644 index 00000000..8036a589 --- /dev/null +++ b/packages/cli/src/lib/internal-rule-id.ts @@ -0,0 +1,12 @@ +/** + * Rule label substituted when a lint message carries no rule at all. + * ESLint reports `ruleId: null` for messages that don't originate from + * a rule — fatal parse or config errors — and SARIF results inherit + * that absence. Labeling them `internal` marks the finding as + * tool-internal breakage rather than leaving a hole in the output + * line. + * + * Kept dependency-free: the ESLint formatter imports this from inside + * the ESLint process. + */ +export const internalRuleId = 'internal'; diff --git a/packages/cli/src/lib/lint-eslint.ts b/packages/cli/src/lib/lint-eslint.ts new file mode 100644 index 00000000..baf802ab --- /dev/null +++ b/packages/cli/src/lib/lint-eslint.ts @@ -0,0 +1,214 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { styleText } from 'node:util'; +import sarifFormat from '@microsoft/eslint-formatter-sarif'; +import { + type Finding, type StyleText, formatFindingReport, +} from './finding-report.ts'; +import { internalRuleId } from './internal-rule-id.ts'; +import { type Logger, createLogger } from './logger.ts'; +import { sarifLogPath } from './sarif-paths.ts'; + +/** SARIF log path written by `lint:eslint`, relative to the lint cwd. */ +export const sarifOutputPath = sarifLogPath('eslint'); + +/** Structural subset of an ESLint lint message the task consumes. */ +export interface EslintMessage { + readonly column?: number | undefined; + readonly line?: number | undefined; + readonly message: string; + readonly ruleId: string | null; + readonly severity: number; +} + +/** Structural subset of an ESLint lint result the task consumes. */ +export interface EslintResult { + readonly errorCount: number; + readonly filePath: string; + readonly messages: readonly EslintMessage[]; +} + +/** Constructor options the task passes to ESLint. */ +export interface EslintOptions { + readonly cache: boolean; + readonly cacheLocation: string; + readonly fix: boolean; + readonly ignorePatterns?: string[]; +} + +/* + * The interfaces below use method syntax deliberately: it keeps their + * parameters bivariant, which is what lets the real ESLint class — + * whose signatures take the full `LintResult` — satisfy these narrower + * structural subsets without type assertions or plumbing eslint's own + * types through the public deps surface. + */ +/* eslint-disable @typescript-eslint/method-signature-style -- + Parameter bivariance is the point; see the comment above. */ + +/** Structural subset of an ESLint instance the task consumes. */ +export interface EslintInstance { + getRulesMetaForResults(results: EslintResult[]): unknown; + lintFiles(patterns: string[]): Promise; +} + +/** Structural subset of the ESLint class itself (constructor + statics). */ +export interface EslintConstructor { + new (options: EslintOptions): EslintInstance; + outputFixes(results: EslintResult[]): Promise; +} + +/* eslint-enable @typescript-eslint/method-signature-style */ + +/** Parsed `lint:eslint` command arguments. */ +export interface LintEslintArgs { + readonly fix: boolean; + readonly ignorePatterns: readonly string[]; + readonly patterns: readonly string[]; +} + +const ignorePatternFlag = '--ignore-pattern'; + +/** + * Parses the narrow flag surface `lint:eslint` supports: positional + * lint patterns, `--fix`, and repeatable `--ignore-pattern`. Anything + * else is rejected rather than silently dropped — the ESLint CLI no + * longer sits behind this command, so unknown flags would otherwise + * vanish without effect. + */ +export const parseLintEslintArgs = ( + rawArgs: readonly string[], +): LintEslintArgs => { + let shouldFix = false; + const ignorePatterns: string[] = []; + const patterns: string[] = []; + for (let index = 0; index < rawArgs.length; index += 1) { + const arg = rawArgs[index] ?? ''; + if (arg === '--fix') { + shouldFix = true; + } else if (arg === ignorePatternFlag) { + index += 1; + const value = rawArgs[index]; + if (value === undefined) { + throw new Error(`${ignorePatternFlag} requires a value`); + } + ignorePatterns.push(value); + } else if (arg.startsWith(`${ignorePatternFlag}=`)) { + ignorePatterns.push(arg.slice(ignorePatternFlag.length + 1)); + } else if (arg.startsWith('-')) { + throw new Error( + `Unsupported lint:eslint argument ${arg} ` + + `(supported: patterns, --fix, ${ignorePatternFlag})`, + ); + } else { + patterns.push(arg); + } + } + return { fix: shouldFix, ignorePatterns, patterns }; +}; + +/** Side-effecting dependencies of `lint:eslint`, injected for tests. */ +export interface LintEslintDeps { + readonly cwd: () => string; + readonly loadEslint: () => Promise; + readonly logger: Logger; + /** Styles the console report (identity in tests). */ + readonly style: StyleText; + readonly writeFile: (filePath: string, content: string) => void; +} + +/** + * Loads the consumer's own ESLint through the optional peer dependency, + * so the consumer keeps ownership of the ESLint version. The import is + * dynamic because hk-only adopters install `@gtbuchanan/cli` without + * eslint; the failure surfaces here, at first use, with the remedy. + */ +const loadEslintConstructor = async (): Promise => { + try { + const { ESLint } = await import('eslint'); + return ESLint; + } catch (error) { + throw new Error( + 'lint:eslint could not load eslint — install it alongside ' + + '@gtbuchanan/cli (declared as an optional peer dependency)', + { cause: error }, + ); + } +}; + +/** + * Real I/O implementations backing {@link LintEslintDeps}. + * @internal + */ +export const defaultLintEslintDeps: LintEslintDeps = { + cwd: () => process.cwd(), + loadEslint: loadEslintConstructor, + logger: createLogger(), + // The report goes to stdout, styleText's default detection stream. + style: styleText, + writeFile: (filePath, content) => { + mkdirSync(path.dirname(filePath), { recursive: true }); + writeFileSync(filePath, content); + }, +}; + +/** ESLint's numeric severity for an error (1 is a warning). */ +const errorSeverity = 2; + +const toFindings = (results: readonly EslintResult[]): readonly Finding[] => + results.flatMap(result => result.messages.map(message => ({ + column: message.column, + level: message.severity === errorSeverity ? 'error' : 'warning', + line: message.line, + message: message.message, + ruleId: message.ruleId ?? internalRuleId, + uri: result.filePath, + }))); + +/** + * Runs ESLint through its programmatic API as a reporter, not a gate: + * warnings never fail the task (the repo convention downgrades every + * rule to a warning), while errors — parse or config breakage under + * that convention — still do, and only after the SARIF log is written + * (a baseline must exist for every commit, including failing ones). + * The API replaces the CLI because ESLint accepts a single `--format`; + * programmatic results feed the SARIF log and the stylish console + * report from one lint run. + */ +export const executeLintEslint = async ( + rawArgs: readonly string[], + deps: LintEslintDeps = defaultLintEslintDeps, +): Promise => { + const args = parseLintEslintArgs(rawArgs); + const Eslint = await deps.loadEslint(); + const eslint = new Eslint({ + cache: true, + cacheLocation: 'dist/.eslintcache', + fix: args.fix, + ...args.ignorePatterns.length > 0 && + { ignorePatterns: [...args.ignorePatterns] }, + }); + const results = await eslint.lintFiles( + args.patterns.length > 0 ? [...args.patterns] : ['.'], + ); + if (args.fix) { + await Eslint.outputFixes(results); + } + deps.writeFile( + path.resolve(deps.cwd(), sarifOutputPath), + sarifFormat(results, { + rulesMeta: eslint.getRulesMetaForResults(results), + }), + ); + const findings = toFindings(results); + if (findings.length > 0) { + deps.logger.info(formatFindingReport(findings, deps.style)); + } + const errorCount = results + .reduce((total, result) => total + result.errorCount, 0); + if (errorCount > 0) { + throw new Error( + `ESLint found ${String(errorCount)} error${errorCount === 1 ? '' : 's'}`, + ); + } +}; diff --git a/packages/cli/src/lib/sarif-compare.ts b/packages/cli/src/lib/sarif-compare.ts new file mode 100644 index 00000000..70b06e99 --- /dev/null +++ b/packages/cli/src/lib/sarif-compare.ts @@ -0,0 +1,345 @@ +import { + copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, readdirSync, + rmSync, writeFileSync, +} from 'node:fs'; +import { createRequire } from 'node:module'; +import { tmpdir } from 'node:os'; +import path from 'node:path'; +import { styleText } from 'node:util'; +import * as v from 'valibot'; +import { readJsonFile } from './file-writer.ts'; +import { type StyleText, formatFindingReport } from './finding-report.ts'; +import { type Logger, createLogger } from './logger.ts'; +import { type RunOptions, capture, run } from './process.ts'; +import { + type NewFinding, extractAllFindings, extractNewFindings, parseSarifLog, +} from './sarif-log.ts'; +import { sarifPaths } from './sarif-paths.ts'; +import { localeComparer } from './sort.ts'; +import { planTurboInvocation } from './turbo-invocation.ts'; +import { type WorkspaceContext, resolveWorkspace } from './workspace.ts'; + +/** + * Side-effecting I/O the compare depends on. Injected so the + * orchestration (baseline production, per-package matching, gating) is + * unit-testable without spawning git, turbo, or the SARIF multitool. + */ +export interface SarifCompareDeps { + readonly capture: (command: string, args: readonly string[]) => Promise; + readonly copyFile: (source: string, destination: string) => void; + readonly ensureDir: (dir: string) => void; + readonly exists: (filePath: string) => boolean; + /** Names of `*.sarif` files directly in `dir` (empty when missing). */ + readonly list: (dir: string) => readonly string[]; + readonly logger: Logger; + readonly makeTempDir: () => string; + readonly readJson: (filePath: string) => unknown; + readonly readText: (filePath: string) => string; + readonly remove: (filePath: string) => void; + readonly resolveMultitool: () => string; + readonly run: (command: string, options?: RunOptions) => Promise; + /** Styles the new-findings report (identity in tests). */ + readonly style: StyleText; + readonly workspace: (cwd?: string) => WorkspaceContext; + readonly writeText: (filePath: string, content: string) => void; +} + +/** + * The multitool npm package exports the path to its platform-specific + * self-contained binary. Resolved lazily (and spawned directly, skipping + * the package's `shell: true` bin shim) so merely loading the CLI never + * requires the optional platform package. + */ +const resolveMultitoolBinary = (): string => { + const require = createRequire(import.meta.url); + return v.parse(v.string(), require('@microsoft/sarif-multitool')); +}; + +const listSarifFiles = (dir: string): readonly string[] => { + if (!existsSync(dir)) return []; + return readdirSync(dir, { withFileTypes: true }) + .filter(entry => entry.isFile() && entry.name.endsWith('.sarif')) + .map(entry => entry.name) + .toSorted(localeComparer); +}; + +/** + * Real I/O implementations backing {@link SarifCompareDeps}. Exported + * for direct unit coverage; the commands use them via default params. + * @internal + */ +export const defaultSarifDeps: SarifCompareDeps = { + capture, + copyFile: (source, destination) => { + mkdirSync(path.dirname(destination), { recursive: true }); + copyFileSync(source, destination); + }, + ensureDir: (dir) => { + mkdirSync(dir, { recursive: true }); + }, + exists: existsSync, + list: listSarifFiles, + logger: createLogger(), + makeTempDir: () => mkdtempSync(path.join(tmpdir(), 'gtb-sarif-base-')), + readJson: readJsonFile, + readText: filePath => readFileSync(filePath, 'utf8'), + remove: (filePath) => { + rmSync(filePath, { force: true, recursive: true }); + }, + resolveMultitool: resolveMultitoolBinary, + run, + // The report goes to stderr; let styleText gate color on that stream. + style: (format, text) => styleText(format, text, { stream: process.stderr }), + workspace: cwd => resolveWorkspace(cwd === undefined ? undefined : { cwd }), + writeText: (filePath, content) => { + mkdirSync(path.dirname(filePath), { recursive: true }); + writeFileSync(filePath, content); + }, +}; + +/** + * Unique lint cwds of a workspace. The root is itself a lint cwd and, + * in a single-package repo, coincides with the sole package dir. + */ +const lintDirs = (workspace: WorkspaceContext): readonly string[] => + [...new Set([workspace.rootDir, ...workspace.packageDirs])]; + +/** Absolute path of the baseline stamp file for a workspace root. */ +const baselineStampPath = (rootDir: string): string => + path.join(rootDir, sarifPaths.stamp); + +/** + * Whether the on-disk baselines were produced from the given merge-base + * SHA (per the stamp file), making production skippable. + */ +const hasCurrentBaseline = ( + sha: string, + rootDir: string, + deps: SarifCompareDeps, +): boolean => { + const stamp = baselineStampPath(rootDir); + return deps.exists(stamp) && deps.readText(stamp).trim() === sha; +}; + +/** + * Ensures the commit's objects exist locally, fetching just that commit + * on shallow clones (GitHub serves reachable SHAs directly). + */ +const ensureCommit = async (sha: string, deps: SarifCompareDeps): Promise => { + try { + await deps.capture('git', ['cat-file', '-e', sha]); + } catch { + await deps.run('git', { args: ['fetch', '--depth=1', 'origin', sha] }); + } +}; + +/** + * Produces per-package baseline SARIF logs by linting the given + * merge-base SHA in a throwaway git worktree and copying each + * `dist/sarif/*.sarif` into the corresponding head package under + * `dist/sarif/base/`, then stamps the SHA. A failing base lint is + * tolerated: reporters write their SARIF logs before exiting, and a + * baseline carrying findings is exactly what the ratchet diffs against. + * Base commits that predate SARIF output simply produce no baseline, + * and the compare skips those packages. + */ +export const produceBaseline = async ( + sha: string, + head: WorkspaceContext, + deps: SarifCompareDeps, +): Promise => { + await ensureCommit(sha, deps); + const baseDir = deps.makeTempDir(); + try { + await deps.run('git', { args: ['worktree', 'add', '--detach', baseDir, sha] }); + /* + * The bootstrap assumes the lint graph needs only the node + * toolchain: pnpm install plus whatever the ambient PATH provides. + * The temp worktree lives outside the repo, so per-directory tool + * managers (mise) never activate there — a base commit pinning + * different tool versions resolves the head environment's binaries + * instead. Fine while every SARIF reporter is node-based; revisit + * with a bootstrap seam when a non-node reporter joins the graph. + */ + await deps.run('pnpm', { + args: ['install', '--frozen-lockfile', '--prefer-offline'], + cwd: baseDir, + }); + /* + * The planner keeps the Android (Termux) escape hatch working here + * too: its PATH plan (`bin: 'turbo'`) is delegated to the + * worktree's own pnpm so the base's pinned turbo runs, while on + * Android the resolved Termux binary is spawned directly — the + * node_modules launcher rejects `android` upfront. + */ + const lintArgs = ['run', 'lint', '--output-logs=errors-only']; + const plan = planTurboInvocation({ + platform: process.platform, + rawArgs: lintArgs, + }); + if (plan.kind === 'error') throw new Error(plan.message); + try { + await (plan.bin === 'turbo' + ? deps.run('pnpm', { args: ['exec', 'turbo', ...lintArgs], cwd: baseDir }) + : deps.run(plan.bin, { args: plan.args, cwd: baseDir })); + } catch { + // Pre-ratchet gtb fails lint on warnings after writing the SARIF log. + deps.logger.error(`Base lint at ${sha} failed; using whatever SARIF it wrote`); + } + copyBaselineSarifs(deps.workspace(baseDir), head, deps); + deps.writeText(baselineStampPath(head.rootDir), `${sha}\n`); + } finally { + await deps.run('git', { args: ['worktree', 'remove', '--force', baseDir] }); + } +}; + +const copyBaselineSarifs = ( + base: WorkspaceContext, + head: WorkspaceContext, + deps: SarifCompareDeps, +): void => { + /* + * Clear baselines from any earlier production first: a package whose + * new merge base wrote no SARIF must not keep a stale baseline from a + * previous merge base. + */ + for (const dir of lintDirs(head)) { + deps.remove(path.join(dir, sarifPaths.base)); + } + for (const dir of lintDirs(base)) { + const relative = path.relative(base.rootDir, dir); + const names = deps.list(path.join(dir, sarifPaths.dir)); + for (const name of names) { + deps.copyFile( + path.join(dir, sarifPaths.dir, name), + path.join(head.rootDir, relative, sarifPaths.base, name), + ); + } + } +}; + +const matchFileForward = async ( + dir: string, + name: string, + deps: SarifCompareDeps, +): Promise => { + const base = path.join(dir, sarifPaths.base, name); + if (!deps.exists(base)) { + /* + * A missing baseline is an empty baseline, not a pass: every + * finding is new and needs explicit acceptance. This keeps a newly + * added reporter (or the bootstrap PR) from slipping findings in + * silently. + */ + deps.logger.error(`No baseline for ${name} in ${dir}; all findings are new`); + const current = deps.readJson(path.join(dir, sarifPaths.dir, name)); + return extractAllFindings(parseSarifLog(current)); + } + const matched = path.join(dir, sarifPaths.matched, name); + // The multitool won't create the output file's parent directory. + deps.ensureDir(path.join(dir, sarifPaths.matched)); + await deps.run(deps.resolveMultitool(), { + args: [ + 'match-results-forward', path.join(dir, sarifPaths.dir, name), + '--previous', base, + '--output-file-path', matched, + // Reruns are routine (retries, local iteration); replace stale output. + '--log', 'ForceOverwrite', + ], + }); + return extractNewFindings(parseSarifLog(deps.readJson(matched))); +}; + +/** + * Snapshots the current SARIF logs as the baseline. On the default + * branch the merge base of any future PR is HEAD itself, so those PRs' + * baseline is just this commit's own reporter output: copy each + * `dist/sarif/*.sarif` under `dist/sarif/base/` and stamp HEAD's SHA. + * CI saves the result in a cache keyed on that SHA for PR compare runs + * to restore. + */ +export const executeSarifBaseline = async ( + deps: SarifCompareDeps = defaultSarifDeps, +): Promise => { + const sha = await deps.capture('git', ['rev-parse', 'HEAD']); + const head = deps.workspace(); + copyBaselineSarifs(head, head, deps); + deps.writeText(baselineStampPath(head.rootDir), `${sha}\n`); + deps.logger.info(`Seeded SARIF baselines for ${sha}`); +}; + +/** Options for {@link executeSarifCompare}. */ +export interface SarifCompareOptions { + /** + * Git ref to diff against. The baseline commit is the merge base of + * this ref and HEAD (a `git merge-base` call, so it needs local + * history — the local mode). + */ + readonly baseRef?: string | undefined; + /** + * Exact baseline commit, no merge-base resolution. CI passes the PR + * merge ref's first parent (`git rev-parse HEAD^1`): on the merged + * checkout, the target branch head *is* the merge base, so this + * needs no branch fetch or history. Mutually exclusive with + * `baseRef`. When neither is set, `dist/sarif/base/` must already be + * populated (e.g. restored from a cache or a prior run). + */ + readonly baseSha?: string | undefined; +} + +const resolveBaselineSha = async ( + options: SarifCompareOptions, + deps: SarifCompareDeps, +): Promise => { + if (options.baseRef !== undefined && options.baseSha !== undefined) { + throw new Error('--base and --base-sha are mutually exclusive'); + } + if (options.baseSha !== undefined) { + return options.baseSha; + } + if (options.baseRef !== undefined) { + return deps.capture('git', ['merge-base', options.baseRef, 'HEAD']); + } + return undefined; +}; + +/** + * Compares every SARIF log under each lint cwd's `dist/sarif/` against + * its baseline via `sarif-multitool match-results-forward` and rejects + * when any result is classified `new`. Matching is fingerprint and + * content based, so baseline findings that merely moved (edits above + * them) stay matched — only genuine regressions gate. + */ +export const executeSarifCompare = async ( + options: SarifCompareOptions = {}, + deps: SarifCompareDeps = defaultSarifDeps, +): Promise => { + const head = deps.workspace(); + const sha = await resolveBaselineSha(options, deps); + if (sha !== undefined) { + if (hasCurrentBaseline(sha, head.rootDir, deps)) { + deps.logger.info(`Baselines for merge base ${sha} already present; reusing`); + } else { + await produceBaseline(sha, head, deps); + } + } + /* + * Each pairing writes to its own `matched/`, so the multitool + * spawns are independent — run them concurrently. Gathering in pair + * order keeps the report deterministic. + */ + const pairs = lintDirs(head).flatMap(dir => + deps.list(path.join(dir, sarifPaths.dir)).map(name => ({ dir, name }))); + const matchedFindings = await Promise.all( + pairs.map(pair => matchFileForward(pair.dir, pair.name, deps)), + ); + const findings = matchedFindings.flat(); + + if (findings.length > 0) { + deps.logger.error(formatFindingReport(findings, deps.style)); + throw new Error( + `${String(findings.length)} new finding(s) not present in the baseline`, + ); + } + deps.logger.info('No new findings'); +}; diff --git a/packages/cli/src/lib/sarif-log.ts b/packages/cli/src/lib/sarif-log.ts new file mode 100644 index 00000000..2bbfa8f3 --- /dev/null +++ b/packages/cli/src/lib/sarif-log.ts @@ -0,0 +1,111 @@ +import * as v from 'valibot'; +import type { Finding } from './finding-report.ts'; +import { internalRuleId } from './internal-rule-id.ts'; + +const SarifRegionSchema = v.object({ + startColumn: v.optional(v.number()), + startLine: v.optional(v.number()), +}); + +const SarifArtifactLocationSchema = v.object({ + uri: v.optional(v.string()), +}); + +const SarifPhysicalLocationSchema = v.object({ + artifactLocation: v.optional(SarifArtifactLocationSchema), + region: v.optional(SarifRegionSchema), +}); + +const SarifLocationSchema = v.object({ + physicalLocation: v.optional(SarifPhysicalLocationSchema), +}); + +const SarifSuppressionsSchema = v.array(v.unknown()); + +const SarifResultSchema = v.object({ + baselineState: v.optional(v.string()), + level: v.optional(v.string()), + locations: v.optional(v.array(SarifLocationSchema)), + message: v.object({ text: v.string() }), + ruleId: v.optional(v.string()), + suppressions: v.optional(SarifSuppressionsSchema), +}); + +const SarifRunSchema = v.object({ + results: v.optional(v.array(SarifResultSchema)), +}); + +const SarifLogSchema = v.object({ + runs: v.array(SarifRunSchema), +}); + +/** Parsed subset of a SARIF log the compare consumes. */ +export type SarifLog = v.InferOutput; + +type SarifResult = v.InferOutput; + +/** Validates untrusted JSON as a {@link SarifLog}. */ +export const parseSarifLog = (data: unknown): SarifLog => + v.parse(SarifLogSchema, data); + +/** A finding present in HEAD but not matched to the baseline. */ +export type NewFinding = Finding; + +interface LocationParts { + readonly column: number | undefined; + readonly line: number | undefined; + readonly uri: string | undefined; +} + +// Split out of toNewFinding to keep its complexity within the lint limit. +const toLocationParts = (result: SarifResult): LocationParts => { + const location = result.locations?.[0]?.physicalLocation; + return { + column: location?.region?.startColumn, + line: location?.region?.startLine, + uri: location?.artifactLocation?.uri, + }; +}; + +const toNewFinding = (result: SarifResult): NewFinding => { + const { column, line, uri } = toLocationParts(result); + return { + column, + level: result.level ?? 'error', + line, + message: result.message.text, + ruleId: result.ruleId ?? internalRuleId, + uri: uri ?? '', + }; +}; + +/** + * Suppressed findings (e.g. reasoned `eslint-disable` comments) are + * exempt from the gate: an in-source suppression is already the + * accepted mechanism for carrying a finding, reviewed with the code. + * They stay in the SARIF logs for visibility; they just never block. + */ +const isUnsuppressed = (result: SarifResult): boolean => + (result.suppressions ?? []).length === 0; + +const extractFindings = ( + log: SarifLog, + isIncluded: (result: SarifResult) => boolean, +): readonly NewFinding[] => + log.runs.flatMap(run => + (run.results ?? []) + .filter(isUnsuppressed) + .filter(isIncluded) + .map(toNewFinding), + ); + +/** Extracts unsuppressed results the baseliner classified as `new`. */ +export const extractNewFindings = (log: SarifLog): readonly NewFinding[] => + extractFindings(log, result => result.baselineState === 'new'); + +/** + * Extracts every unsuppressed result — the classification of a log + * whose baseline is empty (all findings are new by definition). + */ +export const extractAllFindings = (log: SarifLog): readonly NewFinding[] => + extractFindings(log, () => true); diff --git a/packages/cli/src/lib/sarif-paths.ts b/packages/cli/src/lib/sarif-paths.ts new file mode 100644 index 00000000..f6807318 --- /dev/null +++ b/packages/cli/src/lib/sarif-paths.ts @@ -0,0 +1,24 @@ +/** + * SARIF artifact layout under each lint cwd. Reporters (any tool) drop + * `.sarif` files into `dist/sarif/`; the compare pairs each with + * `dist/sarif/base/.sarif` by filename and writes match results + * to `dist/sarif/matched/.sarif`. The stamp lives only at the + * workspace root and records the merge-base SHA the on-disk baselines + * were produced from, letting `--base` skip production when current — + * locally across repeat runs, and in CI via a cache keyed on that SHA. + * + * POSIX-form segments: usable verbatim as turbo.json globs, and + * `node:fs` accepts forward slashes on every platform. + */ +const dir = 'dist/sarif'; + +/** SARIF artifact paths relative to the lint cwd. */ +export const sarifPaths = { + base: `${dir}/base`, + dir, + matched: `${dir}/matched`, + stamp: `${dir}/base.ref`, +} as const; + +/** SARIF log path a tool's reporter writes, relative to the lint cwd. */ +export const sarifLogPath = (tool: string): string => `${dir}/${tool}.sarif`; diff --git a/packages/cli/src/lib/turbo-config.ts b/packages/cli/src/lib/turbo-config.ts index a13b860f..4e0e4451 100644 --- a/packages/cli/src/lib/turbo-config.ts +++ b/packages/cli/src/lib/turbo-config.ts @@ -1,5 +1,6 @@ import { taskNames } from '../commands/task/names.ts'; import type { WorkspaceDiscovery } from './discovery.ts'; +import { sarifLogPath } from './sarif-paths.ts'; import { skillsConfigFilename } from './skills-config.ts'; import { localeComparer } from './sort.ts'; import { typeCheckInclude } from './tsconfig-gen.ts'; @@ -244,7 +245,7 @@ const lintTasks = (flags: ToolFlags): readonly ConditionalEntry[] => value: { dependsOn: deps, inputs: ['$TURBO_ROOT$/eslint.config.*', ...inputs, 'eslint.config.*'], - outputs: ['dist/.eslintcache'], + outputs: ['dist/.eslintcache', sarifLogPath('eslint')], }, }, ]; @@ -279,7 +280,7 @@ const rootLintTasks = ( '$TURBO_DEFAULT$', ...packageGlobs.map(glob => `!${toPackageIgnore(glob)}`), ], - outputs: ['dist/.eslintcache'], + outputs: ['dist/.eslintcache', sarifLogPath('eslint')], }, }, ]; diff --git a/packages/cli/src/lib/turbo-invocation.ts b/packages/cli/src/lib/turbo-invocation.ts new file mode 100644 index 00000000..a3908b74 --- /dev/null +++ b/packages/cli/src/lib/turbo-invocation.ts @@ -0,0 +1,79 @@ +import { existsSync } from 'node:fs'; +import path from 'node:path'; + +/** + * Setup help shown when the global turbo binary is missing on Android. + * Termux ships a native turbo via its package registry; the npm + * `@turbo/linux-` workaround is no longer needed and the + * launcher in `node_modules/.bin/turbo` rejects + * `process.platform === 'android'` upfront. + * + * Native android binaries are not coming from upstream — vercel/turborepo#5616 + * was closed as "not planned" — so the Termux-pkg turbo (Bionic-built + * against `aarch64-linux-android`) is the supported path. + */ +const androidSetupHelp = ` +gtb turbo: the global turbo binary is not installed. + +On Android (Termux), gtb turbo execs the native turbo from the Termux +package registry instead of the npm-distributed Linux binary. The +node_modules launcher refuses to start when process.platform === 'android'. + +Install it from the Termux registry: + + pkg install turbo + +If your Termux prefix is non-standard, set $PREFIX before running. +`.trimStart(); + +/** + * Resolves the global Termux-pkg-installed turbo binary. Honors + * Termux's $PREFIX env var; falls back to the standard install path + * when $PREFIX is unset (matching the convention used by + * `@gtbuchanan/pnpm-termux-shim`). + */ +const resolveAndroidTurboBinary = (): string | undefined => { + const prefix = process.env['PREFIX'] ?? '/data/data/com.termux/files/usr'; + const candidate = path.join(prefix, 'bin', 'turbo'); + return existsSync(candidate) ? candidate : undefined; +}; + +/** Discriminated plan for how to invoke turbo from the current host. */ +export type TurboInvocation = + | { readonly kind: 'error'; readonly message: string } + | { readonly kind: 'spawn'; readonly args: readonly string[]; readonly bin: string }; + +/** Inputs to {@link planTurboInvocation}. */ +export interface PlanTurboInvocationOptions { + readonly platform: string; + readonly rawArgs: readonly string[]; + readonly resolveAndroidBinary?: () => string | undefined; +} + +/** + * Computes the turbo invocation plan for a given host. On Android + * resolves the Termux-pkg turbo binary directly (bypassing the + * node_modules launcher, which rejects `android` upfront). On every + * other platform delegates to the launcher on PATH (`bin: 'turbo'`) so + * its native install behavior is preserved. + * + * The Termux-pkg turbo is Bionic-built, so its child-process spawns + * honor Termux's `LD_PRELOAD` shebang rewriter and resolve + * `#!/usr/bin/env ` correctly. The companion + * `@gtbuchanan/pnpm-termux-shim` package is retained defensively in + * case turbo reintroduces a glibc npm distribution, or another glibc + * binary in the graph needs to spawn `pnpm`. + */ +export const planTurboInvocation = ( + options: PlanTurboInvocationOptions, +): TurboInvocation => { + if (options.platform !== 'android') { + return { args: [...options.rawArgs], bin: 'turbo', kind: 'spawn' }; + } + const resolveBin = options.resolveAndroidBinary ?? resolveAndroidTurboBinary; + const resolved = resolveBin(); + if (resolved === undefined) { + return { kind: 'error', message: androidSetupHelp }; + } + return { args: [...options.rawArgs], bin: resolved, kind: 'spawn' }; +}; diff --git a/packages/cli/src/types/microsoft-eslint-formatter-sarif.d.ts b/packages/cli/src/types/microsoft-eslint-formatter-sarif.d.ts new file mode 100644 index 00000000..62120555 --- /dev/null +++ b/packages/cli/src/types/microsoft-eslint-formatter-sarif.d.ts @@ -0,0 +1,5 @@ +/** Untyped CJS formatter; minimal structural signature for the wrapper. */ +declare module '@microsoft/eslint-formatter-sarif' { + const format: (results: readonly unknown[], data?: unknown) => string; + export = format; +} diff --git a/packages/cli/test/finding-report.test.ts b/packages/cli/test/finding-report.test.ts new file mode 100644 index 00000000..56591afc --- /dev/null +++ b/packages/cli/test/finding-report.test.ts @@ -0,0 +1,111 @@ +import { describe, it } from 'vitest'; +import { + type Finding, + type StyleText, + formatFindingReport, +} from '#src/lib/finding-report.js'; + +const finding = (overrides: Partial): Finding => ({ + column: 1, + level: 'error', + line: 1, + message: 'boom', + ruleId: 'internal', + uri: 'src/app.ts', + ...overrides, +}); + +/** Tags each styled part so assertions can see where styles landed. */ +const markerStyle: StyleText = (format, text) => `«${format}»${text}`; + +describe.concurrent(formatFindingReport, () => { + it('returns empty output for no findings', ({ expect }) => { + expect(formatFindingReport([])).toBe(''); + }); + + it('aligns columns per file and appends a summary', ({ expect }) => { + const findings = [ + finding({ + column: 10, level: 'error', line: 1, + message: 'Unexpected console statement.', ruleId: 'no-console', + }), + finding({ + column: 2, level: 'warning', line: 22, + message: 'Missing JSDoc.', ruleId: 'jsdoc/require-jsdoc', + }), + ]; + + expect(formatFindingReport(findings)).toBe([ + 'src/app.ts', + ' 1:10 error Unexpected console statement. no-console', + ' 22:2 warning Missing JSDoc. jsdoc/require-jsdoc', + '', + '✖ 2 problems (1 error, 1 warning)', + ].join('\n')); + }); + + it('groups findings by file in first-appearance order', ({ expect }) => { + const findings = [ + finding({ message: 'first', ruleId: 'rule-a', uri: 'b.ts' }), + finding({ level: 'warning', message: 'second', ruleId: 'rule-b', uri: 'a.ts' }), + finding({ message: 'third', ruleId: 'rule-c', uri: 'b.ts' }), + ]; + + expect(formatFindingReport(findings)).toBe([ + 'b.ts', + ' 1:1 error first rule-a', + ' 1:1 error third rule-c', + '', + 'a.ts', + ' 1:1 warning second rule-b', + '', + '✖ 3 problems (2 errors, 1 warning)', + ].join('\n')); + }); + + it('omits the position column when no finding has one', ({ expect }) => { + const findings = [ + finding({ column: undefined, level: 'warning', line: undefined }), + ]; + + expect(formatFindingReport(findings)).toBe([ + 'src/app.ts', + ' warning boom internal', + '', + '✖ 1 problem (0 errors, 1 warning)', + ].join('\n')); + }); + + it('renders a line-only position when the column is absent', ({ expect }) => { + const findings = [finding({ column: undefined, line: 7 })]; + + expect(formatFindingReport(findings)).toBe([ + 'src/app.ts', + ' 7 error boom internal', + '', + '✖ 1 problem (1 error, 0 warnings)', + ].join('\n')); + }); + + it('styles each report part through the given style', ({ expect }) => { + const findings = [ + finding({ message: 'boom', ruleId: 'no-console' }), + finding({ level: 'warning', message: 'hmmm', ruleId: 'no-alert' }), + ]; + + expect(formatFindingReport(findings, markerStyle)).toBe([ + '«underline»src/app.ts', + ' «dim»1:1 «red»error boom «dim»no-console', + ' «dim»1:1 «yellow»warning hmmm «dim»no-alert', + '', + '«bold»«red»✖ 2 problems (1 error, 1 warning)', + ].join('\n')); + }); + + it('styles an error-free summary yellow', ({ expect }) => { + const findings = [finding({ level: 'warning' })]; + + expect(formatFindingReport(findings, markerStyle)) + .toContain('«bold»«yellow»✖ 1 problem (0 errors, 1 warning)'); + }); +}); diff --git a/packages/cli/test/lint-eslint.test.ts b/packages/cli/test/lint-eslint.test.ts new file mode 100644 index 00000000..626ae579 --- /dev/null +++ b/packages/cli/test/lint-eslint.test.ts @@ -0,0 +1,246 @@ +import path from 'node:path'; +import { faker } from '@faker-js/faker'; +import { describe, it } from 'vitest'; +import { plainText } from '#src/lib/finding-report.js'; +import { + type EslintConstructor, + type EslintOptions, + type EslintResult, + type LintEslintDeps, + executeLintEslint, + parseLintEslintArgs, + sarifOutputPath, +} from '#src/lib/lint-eslint.js'; +import { parseSarifLog } from '#src/lib/sarif-log.js'; +import { captureLogger, createTempDir } from './helpers.ts'; + +describe.concurrent(parseLintEslintArgs, () => { + it('defaults to no patterns, no ignores, and no fix', ({ expect }) => { + expect(parseLintEslintArgs([])).toStrictEqual({ + fix: false, + ignorePatterns: [], + patterns: [], + }); + }); + + it('collects positional arguments as lint patterns', ({ expect }) => { + expect(parseLintEslintArgs(['src', 'test'])).toMatchObject({ + patterns: ['src', 'test'], + }); + }); + + it('enables fix mode via --fix', ({ expect }) => { + expect(parseLintEslintArgs(['--fix'])).toMatchObject({ fix: true }); + }); + + it('accepts --ignore-pattern in both spaced and equals forms', ({ expect }) => { + const args = ['--ignore-pattern', 'packages/*/**', '--ignore-pattern=dist/**']; + + expect(parseLintEslintArgs(args)).toMatchObject({ + ignorePatterns: ['packages/*/**', 'dist/**'], + }); + }); + + it('rejects flags outside the supported surface', ({ expect }) => { + expect(() => parseLintEslintArgs(['--max-warnings=0'])) + .toThrow('--max-warnings=0'); + }); + + it('rejects a trailing --ignore-pattern with no value', ({ expect }) => { + expect(() => parseLintEslintArgs(['--ignore-pattern'])) + .toThrow('--ignore-pattern'); + }); +}); + +/** Builds an ESLint result in the subset shape the task consumes. */ +const lintResult = (overrides?: Partial): EslintResult => ({ + errorCount: 0, + filePath: faker.system.filePath(), + messages: [], + ...overrides, +}); + +interface StubEslint { + readonly Eslint: EslintConstructor; + readonly ctorOptions: readonly EslintOptions[]; + readonly lintedPatterns: readonly (readonly string[])[]; + readonly outputFixesCalls: readonly (readonly EslintResult[])[]; +} + +const stubEslint = (results: EslintResult[]): StubEslint => { + const ctorOptions: EslintOptions[] = []; + const lintedPatterns: string[][] = []; + const outputFixesCalls: EslintResult[][] = []; + class Eslint { + public static outputFixes = (fixed: EslintResult[]): Promise => { + outputFixesCalls.push(fixed); + return Promise.resolve(); + }; + + private readonly results = results; + + private readonly rulesMeta = {}; + + public constructor(options: EslintOptions) { + ctorOptions.push(options); + } + + public getRulesMetaForResults(): unknown { + return this.rulesMeta; + } + + public lintFiles(patterns: string[]): Promise { + lintedPatterns.push(patterns); + return Promise.resolve(this.results); + } + } + return { Eslint, ctorOptions, lintedPatterns, outputFixesCalls }; +}; + +interface StubbedDeps { + readonly cwd: string; + readonly deps: LintEslintDeps; + readonly out: () => string; + readonly writes: readonly { content: string; filePath: string }[]; +} + +const stubDeps = (Eslint: EslintConstructor): StubbedDeps => { + const cwd = createTempDir(); + const writes: { content: string; filePath: string }[] = []; + const { logger, out } = captureLogger(); + return { + cwd, + deps: { + cwd: () => cwd, + loadEslint: () => Promise.resolve(Eslint), + logger, + style: plainText, + writeFile: (filePath, content) => void writes.push({ content, filePath }), + }, + out, + writes, + }; +}; + +describe.concurrent(executeLintEslint, () => { + it('lints the current directory with caching by default', async ({ expect }) => { + const { Eslint, ctorOptions, lintedPatterns } = stubEslint([lintResult()]); + const { deps } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(ctorOptions[0]).toStrictEqual({ + cache: true, + cacheLocation: 'dist/.eslintcache', + fix: false, + }); + expect(lintedPatterns).toStrictEqual([['.']]); + }); + + it('forwards patterns and ignore patterns', async ({ expect }) => { + const { Eslint, ctorOptions, lintedPatterns } = stubEslint([lintResult()]); + const { deps } = stubDeps(Eslint); + + await executeLintEslint(['src', '--ignore-pattern', 'packages/*/**'], deps); + + expect(ctorOptions[0]).toMatchObject({ ignorePatterns: ['packages/*/**'] }); + expect(lintedPatterns).toStrictEqual([['src']]); + }); + + it('writes the SARIF log under the lint cwd', async ({ expect }) => { + const { Eslint } = stubEslint([lintResult({ + messages: [{ + column: 3, line: 2, message: 'Unexpected console statement.', + ruleId: 'no-console', severity: 1, + }], + })]); + const { cwd, deps, writes } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(writes).toHaveLength(1); + expect(writes[0]?.filePath).toBe(path.resolve(cwd, sarifOutputPath)); + + const log = parseSarifLog(JSON.parse(writes[0]?.content ?? '')); + + expect(log.runs[0]?.results).toHaveLength(1); + }); + + it('prints a stylish report of the findings', async ({ expect }) => { + const { Eslint } = stubEslint([lintResult({ + messages: [{ + column: 3, line: 2, message: 'Unexpected console statement.', + ruleId: 'no-console', severity: 1, + }], + })]); + const { deps, out } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(out()).toContain('warning Unexpected console statement. no-console'); + expect(out()).toContain('✖ 1 problem (0 errors, 1 warning)'); + }); + + it('prints nothing for a clean run', async ({ expect }) => { + const { Eslint } = stubEslint([lintResult()]); + const { deps, out } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(out()).toBe(''); + }); + + it('labels a message with no rule as internal', async ({ expect }) => { + const { Eslint } = stubEslint([lintResult({ + messages: [{ + column: 0, + line: 0, + message: 'Parsing error', + /* eslint-disable-next-line unicorn/no-null -- + ESLint reports a null ruleId for parse/internal errors; the + test mirrors that external shape. */ + ruleId: null, + severity: 1, + }], + })]); + const { deps, out } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(out()).toContain('internal'); + }); + + it('rejects on lint errors after writing the SARIF log', async ({ expect }) => { + const { Eslint } = stubEslint([lintResult({ + errorCount: 2, + messages: [ + { column: 1, line: 1, message: 'boom', ruleId: 'no-console', severity: 2 }, + { column: 1, line: 2, message: 'bang', ruleId: 'no-alert', severity: 2 }, + ], + })]); + const { deps, writes } = stubDeps(Eslint); + + await expect(executeLintEslint([], deps)).rejects.toThrow('2 errors'); + expect(writes).toHaveLength(1); + }); + + it('writes fixes through outputFixes in fix mode', async ({ expect }) => { + const results = [lintResult()]; + const { Eslint, ctorOptions, outputFixesCalls } = stubEslint(results); + const { deps } = stubDeps(Eslint); + + await executeLintEslint(['--fix'], deps); + + expect(ctorOptions[0]).toMatchObject({ fix: true }); + expect(outputFixesCalls).toStrictEqual([results]); + }); + + it('leaves fixes unwritten outside fix mode', async ({ expect }) => { + const { Eslint, outputFixesCalls } = stubEslint([lintResult()]); + const { deps } = stubDeps(Eslint); + + await executeLintEslint([], deps); + + expect(outputFixesCalls).toHaveLength(0); + }); +}); diff --git a/packages/cli/test/sarif-baseline.test.ts b/packages/cli/test/sarif-baseline.test.ts new file mode 100644 index 00000000..6066ae4b --- /dev/null +++ b/packages/cli/test/sarif-baseline.test.ts @@ -0,0 +1,237 @@ +import { describe, it } from 'vitest'; +import { + executeSarifBaseline, + executeSarifCompare, + produceBaseline, +} from '#src/lib/sarif-compare.js'; +import type { WorkspaceContext } from '#src/lib/workspace.js'; +import { sarifLog, stubDeps } from './sarif-compare.stub.ts'; + +describe.concurrent(produceBaseline, () => { + const headWorkspace: WorkspaceContext = { + packageDirs: ['/repo/packages/a'], + packageGlobs: ['packages/*'], + rootDir: '/repo', + }; + const baseWorkspace: WorkspaceContext = { + packageDirs: ['/tmp/base/packages/a'], + packageGlobs: ['packages/*'], + rootDir: '/tmp/base', + }; + const baseFiles = [ + '/tmp/base/dist/sarif/eslint.sarif', + '/tmp/base/packages/a/dist/sarif/eslint.sarif', + ]; + + it('lints the merge base in a temp worktree and copies baselines', async ({ + expect, + }) => { + const { copyCalls, deps, removedPaths, runCalls, writeTextCalls } = stubDeps( + headWorkspace, baseFiles, sarifLog([]), { baseWorkspace }, + ); + + await produceBaseline('abc1234', headWorkspace, deps); + + expect(runCalls.map(call => `${call.command} ${call.args[0] ?? ''}`)).toStrictEqual([ + 'git worktree', + 'pnpm install', + 'pnpm exec', + 'git worktree', + ]); + expect(runCalls[0]?.args).toContain('abc1234'); + expect(removedPaths).toStrictEqual([ + '/repo/dist/sarif/base', + '/repo/packages/a/dist/sarif/base', + ]); + expect(copyCalls).toStrictEqual([ + { + destination: '/repo/dist/sarif/base/eslint.sarif', + source: '/tmp/base/dist/sarif/eslint.sarif', + }, + { + destination: '/repo/packages/a/dist/sarif/base/eslint.sarif', + source: '/tmp/base/packages/a/dist/sarif/eslint.sarif', + }, + ]); + expect(writeTextCalls).toStrictEqual([ + { content: 'abc1234\n', filePath: '/repo/dist/sarif/base.ref' }, + ]); + }); + + it('tolerates a failing base lint and copies what it wrote', async ({ expect }) => { + const { copyCalls, deps, err } = stubDeps( + headWorkspace, baseFiles, sarifLog([]), + { baseWorkspace, failing: ['pnpm exec'] }, + ); + + await produceBaseline('abc1234', headWorkspace, deps); + + expect(err()).toContain('Base lint'); + expect(copyCalls).toHaveLength(2); + }); + + it('copies nothing when the base wrote no SARIF logs', async ({ expect }) => { + const { copyCalls, deps } = stubDeps( + headWorkspace, [], sarifLog([]), { baseWorkspace }, + ); + + await produceBaseline('abc1234', headWorkspace, deps); + + expect(copyCalls).toHaveLength(0); + }); + + it('removes the worktree when the base install fails', async ({ expect }) => { + const { deps, runCalls, writeTextCalls } = stubDeps( + headWorkspace, baseFiles, sarifLog([]), + { baseWorkspace, failing: ['pnpm install'] }, + ); + + await expect(produceBaseline('abc1234', headWorkspace, deps)).rejects.toThrow( + /pnpm install failed/v, + ); + expect(runCalls.at(-1)?.args).toStrictEqual([ + 'worktree', 'remove', '--force', '/tmp/base', + ]); + expect(writeTextCalls).toHaveLength(0); + }); +}); + +describe.concurrent(executeSarifBaseline, () => { + const workspace: WorkspaceContext = { + packageDirs: ['/repo/packages/a'], + packageGlobs: ['packages/*'], + rootDir: '/repo', + }; + + it('copies current SARIF logs to baselines and stamps HEAD', async ({ + expect, + }) => { + const files = [ + '/repo/dist/sarif/eslint.sarif', + '/repo/packages/a/dist/sarif/eslint.sarif', + ]; + const { copyCalls, deps, writeTextCalls } = stubDeps(workspace, files, sarifLog([])); + + await executeSarifBaseline(deps); + + expect(copyCalls).toStrictEqual([ + { + destination: '/repo/dist/sarif/base/eslint.sarif', + source: '/repo/dist/sarif/eslint.sarif', + }, + { + destination: '/repo/packages/a/dist/sarif/base/eslint.sarif', + source: '/repo/packages/a/dist/sarif/eslint.sarif', + }, + ]); + expect(writeTextCalls).toStrictEqual([ + { content: 'abc1234\n', filePath: '/repo/dist/sarif/base.ref' }, + ]); + }); + + it('spawns nothing: seeding is pure file copying', async ({ expect }) => { + const { deps, runCalls } = stubDeps( + workspace, + ['/repo/dist/sarif/eslint.sarif'], + sarifLog([]), + ); + + await executeSarifBaseline(deps); + + expect(runCalls).toHaveLength(0); + }); +}); + +describe.concurrent('executeSarifCompare --base', () => { + const workspace: WorkspaceContext = { + packageDirs: [], + packageGlobs: [], + rootDir: '/repo', + }; + const stampedFiles = [ + '/repo/dist/sarif/base.ref', + '/repo/dist/sarif/base/eslint.sarif', + '/repo/dist/sarif/eslint.sarif', + ]; + + it('reuses on-disk baselines when the stamp matches the merge base', async ({ + expect, + }) => { + const { deps, out, runCalls } = stubDeps( + workspace, stampedFiles, sarifLog([]), + { readTextContent: 'abc1234\n' }, + ); + + await executeSarifCompare({ baseRef: 'origin/main' }, deps); + + expect(out()).toContain('already present'); + expect(runCalls.map(call => call.command)).toStrictEqual(['sarif-multitool']); + }); + + it('produces the baseline when the stamp records another merge base', async ({ + expect, + }) => { + const { deps, runCalls } = stubDeps( + workspace, stampedFiles, sarifLog([]), + { readTextContent: 'other999\n' }, + ); + + await executeSarifCompare({ baseRef: 'origin/main' }, deps); + + expect(runCalls[0]).toMatchObject({ command: 'git' }); + expect(runCalls[0]?.args.slice(0, 2)).toStrictEqual(['worktree', 'add']); + }); + + it('produces the baseline when no stamp exists', async ({ expect }) => { + const { deps, runCalls } = stubDeps( + workspace, + ['/repo/dist/sarif/eslint.sarif', '/repo/dist/sarif/base/eslint.sarif'], + sarifLog([]), + ); + + await executeSarifCompare({ baseRef: 'origin/main' }, deps); + + expect(runCalls[0]?.args.slice(0, 2)).toStrictEqual(['worktree', 'add']); + }); + + it('uses an exact commit from --base-sha without merge-base resolution', async ({ + expect, + }) => { + const { deps, runCalls } = stubDeps( + workspace, stampedFiles, sarifLog([]), + { readTextContent: 'fedc987\n' }, + ); + + await executeSarifCompare({ baseSha: 'fedc987' }, deps); + + // Stamp matches the given SHA, so no git commands run at all. + expect(runCalls.map(call => call.command)).toStrictEqual(['sarif-multitool']); + }); + + it('rejects when --base and --base-sha are both given', async ({ expect }) => { + const { deps } = stubDeps(workspace, stampedFiles, sarifLog([])); + + await expect( + executeSarifCompare({ baseRef: 'origin/main', baseSha: 'fedc987' }, deps), + ).rejects.toThrow(/mutually exclusive/v); + }); + + it('fetches the baseline commit when absent from a shallow clone', async ({ + expect, + }) => { + const { deps, runCalls } = stubDeps( + workspace, + ['/repo/dist/sarif/eslint.sarif', '/repo/dist/sarif/base/eslint.sarif'], + sarifLog([]), + { failingCapture: ['cat-file -e'] }, + ); + + await executeSarifCompare({ baseSha: 'fedc987' }, deps); + + expect(runCalls[0]).toMatchObject({ + args: ['fetch', '--depth=1', 'origin', 'fedc987'], + command: 'git', + }); + expect(runCalls[1]?.args.slice(0, 2)).toStrictEqual(['worktree', 'add']); + }); +}); diff --git a/packages/cli/test/sarif-compare.stub.ts b/packages/cli/test/sarif-compare.stub.ts new file mode 100644 index 00000000..de93f5a7 --- /dev/null +++ b/packages/cli/test/sarif-compare.stub.ts @@ -0,0 +1,152 @@ +import { faker } from '@faker-js/faker'; +import { vi } from 'vitest'; +import { plainText } from '#src/lib/finding-report.js'; +import type { SarifCompareDeps } from '#src/lib/sarif-compare.js'; +import { localeComparer } from '#src/lib/sort.js'; +import type { WorkspaceContext } from '#src/lib/workspace.js'; +import { captureLogger } from './helpers.ts'; + +/** Overrides for {@link sarifResult}. */ +export interface SarifResultOverrides { + readonly ruleId?: string; + readonly startLine?: number; + readonly suppressions?: readonly object[]; +} + +/** Builds a SARIF result in the subset shape the compare consumes. */ +export const sarifResult = ( + baselineState: string, + overrides?: SarifResultOverrides, +): object => ({ + baselineState, + level: 'error', + locations: [{ + physicalLocation: { + artifactLocation: { uri: faker.system.filePath() }, + region: { startColumn: 1, startLine: overrides?.startLine ?? 1 }, + }, + }], + message: { text: faker.lorem.sentence() }, + ruleId: overrides?.ruleId ?? 'no-unused-vars', + ...(overrides?.suppressions !== undefined && + { suppressions: overrides.suppressions }), +}); + +/** Wraps results in a single-run SARIF log. */ +export const sarifLog = (results: readonly object[]): object => ({ + runs: [{ results }], +}); + +/** A recorded deps.run invocation. */ +export interface RunCall { + readonly args: readonly string[]; + readonly command: string; +} + +/** A recorded deps.copyFile invocation. */ +export interface CopyCall { + readonly destination: string; + readonly source: string; +} + +/** A recorded deps.writeText invocation. */ +export interface WriteTextCall { + readonly content: string; + readonly filePath: string; +} + +/** Stubbed deps plus the calls they record. */ +export interface StubDeps { + readonly copyCalls: readonly CopyCall[]; + readonly deps: SarifCompareDeps; + /** Buffered stderr text (from the captured logger). */ + readonly err: () => string; + /** Buffered stdout text (from the captured logger). */ + readonly out: () => string; + readonly removedPaths: readonly string[]; + readonly runCalls: readonly RunCall[]; + readonly writeTextCalls: readonly WriteTextCall[]; +} + +/** Options for {@link stubDeps}. */ +export interface StubOptions { + /** Workspace returned for the base worktree cwd. Defaults to `workspace`. */ + readonly baseWorkspace?: WorkspaceContext; + /** `" "` prefixes whose run() rejects. */ + readonly failing?: readonly string[]; + /** `" "` prefixes whose capture() rejects. */ + readonly failingCapture?: readonly string[]; + /** Content returned by readText for any path (e.g. the stamp file). */ + readonly readTextContent?: string; +} + +const normalize = (filePath: string): string => filePath.replaceAll('\\', '/'); + +/** + * Fakes a workspace whose files are the members of `files`; `list` + * derives directory listings from it, and readJson yields `log` for + * any path (matched output or current log alike). + */ +export const stubDeps = ( + workspace: WorkspaceContext, + files: readonly string[], + log: object, + options?: StubOptions, +): StubDeps => { + const copyCalls: CopyCall[] = []; + const removedPaths: string[] = []; + const runCalls: RunCall[] = []; + const writeTextCalls: WriteTextCall[] = []; + const { err, logger, out } = captureLogger(); + return { + copyCalls, + deps: { + capture: (_command, args) => { + const key = `${args[0] ?? ''} ${args[1] ?? ''}`; + return options?.failingCapture?.includes(key) === true + ? Promise.reject(new Error(`${key} failed`)) + : Promise.resolve('abc1234'); + }, + copyFile: (source, destination) => { + copyCalls.push({ + destination: normalize(destination), + source: normalize(source), + }); + }, + ensureDir: vi.fn<(dir: string) => void>(), + exists: filePath => files.includes(normalize(filePath)), + list: (dir) => { + const prefix = `${normalize(dir)}/`; + return files + .filter(filePath => filePath.startsWith(prefix)) + .map(filePath => filePath.slice(prefix.length)) + .filter(name => !name.includes('/') && name.endsWith('.sarif')) + .toSorted(localeComparer); + }, + logger, + makeTempDir: () => '/tmp/base', + readJson: () => log, + readText: () => options?.readTextContent ?? '', + remove: filePath => void removedPaths.push(normalize(filePath)), + resolveMultitool: () => 'sarif-multitool', + run: (command, runOptions) => { + runCalls.push({ args: runOptions?.args ?? [], command }); + const key = `${command} ${runOptions?.args?.[0] ?? ''}`; + return options?.failing?.includes(key) === true + ? Promise.reject(new Error(`${key} failed`)) + : Promise.resolve(); + }, + style: plainText, + workspace: cwd => + (cwd === undefined ? workspace : options?.baseWorkspace ?? workspace), + writeText: (filePath, content) => { + writeTextCalls.push({ content, filePath: normalize(filePath) }); + }, + }, + err, + out, + removedPaths, + runCalls, + writeTextCalls, + }; +}; diff --git a/packages/cli/test/sarif-compare.test.ts b/packages/cli/test/sarif-compare.test.ts new file mode 100644 index 00000000..852bd975 --- /dev/null +++ b/packages/cli/test/sarif-compare.test.ts @@ -0,0 +1,144 @@ +import { describe, it } from 'vitest'; +import { executeSarifCompare } from '#src/lib/sarif-compare.js'; +import { + extractAllFindings, + extractNewFindings, + parseSarifLog, +} from '#src/lib/sarif-log.js'; +import type { WorkspaceContext } from '#src/lib/workspace.js'; +import { sarifLog, sarifResult, stubDeps } from './sarif-compare.stub.ts'; + +describe.concurrent(extractNewFindings, () => { + it('keeps only results the baseliner classified as new', ({ expect }) => { + const newResult = sarifResult('new', { ruleId: 'no-console' }); + const log = sarifLog([ + sarifResult('unchanged'), + sarifResult('updated'), + newResult, + sarifResult('absent'), + ]); + + const findings = extractNewFindings(parseSarifLog(log)); + + expect(findings).toHaveLength(1); + expect(findings[0]).toMatchObject({ level: 'error', ruleId: 'no-console' }); + }); + + it('exempts suppressed findings from the gate', ({ expect }) => { + const log = parseSarifLog(sarifLog([ + sarifResult('new', { suppressions: [{ kind: 'inSource' }] }), + ])); + + expect(extractNewFindings(log)).toHaveLength(0); + }); + + it('falls back to placeholders when location or rule is absent', ({ expect }) => { + const log = parseSarifLog(sarifLog([ + { baselineState: 'new', message: { text: 'boom' } }, + ])); + + expect(extractNewFindings(log)).toStrictEqual([{ + column: undefined, + level: 'error', + line: undefined, + message: 'boom', + ruleId: 'internal', + uri: '', + }]); + }); +}); + +describe.concurrent(extractAllFindings, () => { + it('treats every unsuppressed result as new regardless of state', ({ expect }) => { + const log = parseSarifLog(sarifLog([ + sarifResult('unchanged'), + sarifResult('unchanged', { suppressions: [{ kind: 'inSource' }] }), + ])); + + expect(extractAllFindings(log)).toHaveLength(1); + }); +}); + +describe.concurrent(executeSarifCompare, () => { + const workspace: WorkspaceContext = { + packageDirs: ['/repo/packages/a'], + packageGlobs: ['packages/*'], + rootDir: '/repo', + }; + const currentFiles = [ + '/repo/dist/sarif/eslint.sarif', + '/repo/dist/sarif/base/eslint.sarif', + '/repo/packages/a/dist/sarif/eslint.sarif', + '/repo/packages/a/dist/sarif/base/eslint.sarif', + ]; + + it('matches every SARIF log forward against its baseline', async ({ expect }) => { + const { deps, runCalls } = stubDeps(workspace, currentFiles, sarifLog([])); + + await executeSarifCompare({}, deps); + + expect(runCalls).toHaveLength(2); + expect(runCalls[0]).toMatchObject({ command: 'sarif-multitool' }); + expect(runCalls[0]?.args[0]).toBe('match-results-forward'); + }); + + it('resolves and reports when no results are new', async ({ expect }) => { + const { deps, out } = stubDeps( + workspace, + currentFiles, + sarifLog([sarifResult('unchanged'), sarifResult('updated')]), + ); + + await executeSarifCompare({}, deps); + + expect(out()).toContain('No new findings'); + }); + + it('rejects when any result is new and reports it in the stylish layout', async ({ + expect, + }) => { + const { deps, err } = stubDeps( + workspace, + currentFiles, + sarifLog([sarifResult('new', { ruleId: 'no-console' })]), + ); + + await expect(executeSarifCompare({}, deps)).rejects.toThrow(/new finding/v); + expect(err()).toContain(' no-console'); + expect(err()).toContain('✖ 2 problems (2 errors, 0 warnings)'); + }); + + it('skips dirs with no current SARIF logs', async ({ expect }) => { + const { deps, runCalls } = stubDeps(workspace, [], sarifLog([])); + + await executeSarifCompare({}, deps); + + expect(runCalls).toHaveLength(0); + }); + + it('treats a missing baseline as empty: all findings are new', async ({ + expect, + }) => { + const { deps, err, runCalls } = stubDeps( + workspace, + ['/repo/dist/sarif/eslint.sarif'], + sarifLog([sarifResult('unchanged')]), + ); + + await expect(executeSarifCompare({}, deps)).rejects.toThrow(/new finding/v); + expect(runCalls).toHaveLength(0); + expect(err()).toContain('all findings are new'); + }); + + it('passes with a missing baseline when the log is clean', async ({ expect }) => { + const { deps, out } = stubDeps( + workspace, + ['/repo/dist/sarif/eslint.sarif'], + sarifLog([sarifResult('unchanged', { suppressions: [{ kind: 'inSource' }] })]), + ); + + await executeSarifCompare({}, deps); + + expect(out()).toContain('No new findings'); + }); +}); diff --git a/packages/cli/test/sarif-deps.test.ts b/packages/cli/test/sarif-deps.test.ts new file mode 100644 index 00000000..01522f1e --- /dev/null +++ b/packages/cli/test/sarif-deps.test.ts @@ -0,0 +1,116 @@ +import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import path from 'node:path'; +import { describe, it } from 'vitest'; +import { defaultSarifDeps } from '#src/lib/sarif-compare.js'; +import { createTempDir } from './helpers.ts'; + +interface SandboxFixture { + readonly dir: string; + readonly [Symbol.dispose]: () => void; +} + +const createSandbox = (): SandboxFixture => { + const dir = createTempDir(); + return { + dir, + [Symbol.dispose]() { + rmSync(dir, { force: true, recursive: true }); + }, + }; +}; + +describe.concurrent('defaultSarifDeps', () => { + it('writeText creates parent directories and readText round-trips', ({ + expect, + }) => { + using sandbox = createSandbox(); + const filePath = path.join(sandbox.dir, 'dist', 'sarif', 'base.ref'); + + defaultSarifDeps.writeText(filePath, 'abc1234\n'); + + expect(defaultSarifDeps.readText(filePath)).toBe('abc1234\n'); + }); + + it('copyFile creates the destination directory', ({ expect }) => { + using sandbox = createSandbox(); + const source = path.join(sandbox.dir, 'eslint.sarif'); + writeFileSync(source, '{}'); + const destination = path.join(sandbox.dir, 'dist', 'sarif', 'base', 'eslint.sarif'); + + defaultSarifDeps.copyFile(source, destination); + + expect(readFileSync(destination, 'utf8')).toBe('{}'); + }); + + it('list returns only top-level sarif files, sorted', ({ expect }) => { + using sandbox = createSandbox(); + const dir = path.join(sandbox.dir, 'dist', 'sarif'); + defaultSarifDeps.ensureDir(path.join(dir, 'base')); + writeFileSync(path.join(dir, 'oxlint.sarif'), '{}'); + writeFileSync(path.join(dir, 'eslint.sarif'), '{}'); + writeFileSync(path.join(dir, 'base.ref'), 'abc\n'); + writeFileSync(path.join(dir, 'base', 'eslint.sarif'), '{}'); + + expect(defaultSarifDeps.list(dir)).toStrictEqual([ + 'eslint.sarif', + 'oxlint.sarif', + ]); + }); + + it('list returns empty for a missing directory', ({ expect }) => { + using sandbox = createSandbox(); + + expect(defaultSarifDeps.list(path.join(sandbox.dir, 'missing'))).toStrictEqual([]); + }); + + it('remove deletes directories recursively and ignores missing paths', ({ + expect, + }) => { + using sandbox = createSandbox(); + const dir = path.join(sandbox.dir, 'dist', 'sarif', 'base'); + defaultSarifDeps.ensureDir(dir); + writeFileSync(path.join(dir, 'eslint.sarif'), '{}'); + + defaultSarifDeps.remove(dir); + defaultSarifDeps.remove(dir); + + expect(existsSync(dir)).toBe(false); + }); + + it('ensureDir is recursive and idempotent', ({ expect }) => { + using sandbox = createSandbox(); + const dir = path.join(sandbox.dir, 'a', 'b', 'c'); + + defaultSarifDeps.ensureDir(dir); + defaultSarifDeps.ensureDir(dir); + + expect(defaultSarifDeps.exists(dir)).toBe(true); + }); + + it('makeTempDir creates a fresh directory', ({ expect }) => { + const dir = defaultSarifDeps.makeTempDir(); + try { + expect(existsSync(dir)).toBe(true); + expect(path.basename(dir)).toMatch(/^gtb-sarif-base-/v); + } finally { + rmSync(dir, { force: true, recursive: true }); + } + }); + + it('resolveMultitool resolves an existing binary path', ({ expect }) => { + const binary = defaultSarifDeps.resolveMultitool(); + + expect(existsSync(binary)).toBe(true); + }); + + it('workspace resolves single-package mode from an isolated cwd', ({ + expect, + }) => { + using sandbox = createSandbox(); + + expect(defaultSarifDeps.workspace(sandbox.dir)).toMatchObject({ + packageDirs: [sandbox.dir], + rootDir: sandbox.dir, + }); + }); +}); diff --git a/packages/cli/test/turbo.test.ts b/packages/cli/test/turbo.test.ts index 5301c138..ceda5ab7 100644 --- a/packages/cli/test/turbo.test.ts +++ b/packages/cli/test/turbo.test.ts @@ -2,7 +2,7 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { describe, it, vi } from 'vitest'; -import { type TurboInvocation, planTurboInvocation } from '#src/commands/root/turbo.js'; +import { type TurboInvocation, planTurboInvocation } from '#src/lib/turbo-invocation.js'; interface PrefixFixture { readonly prefix: string; diff --git a/packages/eslint-config/skills/gtb-eslint-config/SKILL.md b/packages/eslint-config/skills/gtb-eslint-config/SKILL.md index eac7807a..71a7bb5c 100644 --- a/packages/eslint-config/skills/gtb-eslint-config/SKILL.md +++ b/packages/eslint-config/skills/gtb-eslint-config/SKILL.md @@ -320,9 +320,12 @@ under pnpm strict hoisting. - **Warnings-only in IDE, errors in CI.** `onlyWarn: true` (the default) surfaces every lint violation as a warning so TypeScript diagnostics - stand out in editors. CI runs `eslint --max-warnings=0` to enforce - zero violations. The `gtb task lint:eslint` command sets this flag - along with `--cache --cache-location dist/.eslintcache`. + stand out in editors. Enforcement lives elsewhere: the hk pre-commit + step runs `eslint --max-warnings=0` on staged files, and CI's SARIF + ratchet fails only findings new relative to the merge base. The + `gtb task lint:eslint` command itself is a reporter — it runs ESLint + through its programmatic API with caching (`dist/.eslintcache`), + writes `dist/sarif/eslint.sarif`, and never fails on warnings. - **Inline suppressions require a `--` reason suffix.** Enforced by `@eslint-community/eslint-plugin-eslint-comments`. Use the multiline format for readability: @@ -372,9 +375,9 @@ markdownlint/lint -->` would suppress every markdownlint rule at once. Monorepos using `@gtbuchanan/cli` follow a two-tier ESLint setup: - **Per-package `eslint.config.ts`** — calls `configure()` and lints - source under that package. The generated `lint:eslint` task runs with - `--cache --cache-location dist/.eslintcache --max-warnings=0`. Cache - files live under each package's `dist/`. + source under that package. The generated `lint:eslint` task runs + ESLint through its programmatic API with caching; cache files live + under each package's `dist/`. - **Root `eslint.config.ts`** — when present, `gtb sync` also generates a `//#lint:eslint` turbo task and a root `lint:eslint` script. The root script lints workspace-root files (`package.json`, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 82924f09..8ca0d05c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -21,6 +21,12 @@ catalogs: '@faker-js/faker': specifier: ^10.4.0 version: 10.5.0 + '@microsoft/eslint-formatter-sarif': + specifier: ^3.1.0 + version: 3.1.0 + '@microsoft/sarif-multitool': + specifier: ^5.5.0 + version: 5.5.0 '@pnpm/lockfile.types': specifier: ^1100.0.5 version: 1100.0.16 @@ -163,6 +169,9 @@ catalogs: specifier: ^2.8.3 version: 2.9.0 +overrides: + '@microsoft/eslint-formatter-sarif>eslint': '-' + pnpmfileChecksum: sha256-hgl/TpYs239ORonvRcYC8xhrwWZfg+PQ6jMTDNGBxxQ= importers: @@ -238,6 +247,12 @@ importers: packages/cli: dependencies: + '@microsoft/eslint-formatter-sarif': + specifier: 'catalog:' + version: 3.1.0 + '@microsoft/sarif-multitool': + specifier: 'catalog:' + version: 5.5.0 citty: specifier: 'catalog:' version: 0.2.2 @@ -290,6 +305,9 @@ importers: '@types/hosted-git-info': specifier: 'catalog:' version: 3.0.5 + eslint: + specifier: 'catalog:' + version: 10.8.0(jiti@2.7.0)(supports-color@7.2.0) publishDirectory: dist/source packages/eslint-config: @@ -815,6 +833,30 @@ packages: '@manypkg/get-packages@1.1.3': resolution: {integrity: sha512-fo+QhuU3qE/2TQMQmbVMqaQ6EWbMhi4ABWP+O4AM1NqPBuy0OrApV5LO6BrrgnhtAHS2NH6RrVk9OL181tTi8A==} + '@microsoft/eslint-formatter-sarif@3.1.0': + resolution: {integrity: sha512-/mn4UXziHzGXnKCg+r8HGgPy+w4RzpgdoqFuqaKOqUVBT5x2CygGefIrO4SusaY7t0C4gyIWMNu6YQT6Jw64Cw==} + engines: {node: '>= 14'} + + '@microsoft/sarif-multitool-darwin@5.5.0': + resolution: {integrity: sha512-HgCqNs9XKfvHjFtC6lJocj9o2zXXv7NjgeLIW2iCnCfsm7IOUQ6JKMXXb6C8GVmyCuEg7Xl8vwNudTTV0D1paA==} + os: [darwin] + hasBin: true + + '@microsoft/sarif-multitool-linux@5.5.0': + resolution: {integrity: sha512-k/86uqCJFIo33WSiDMfHkRMF1/5S/bbMFhh+doAlBGxvaV8+zDK7hY/tjO8+AQxhU18UdTLCu/VacuK4+uOmNQ==} + os: [linux] + hasBin: true + + '@microsoft/sarif-multitool-win32@5.5.0': + resolution: {integrity: sha512-h9axK25H+FmHTWViBqoywQI0tfKRn9txwI+W/IrRFCuFmKUXHpNmpQl9JMFz77PVsNM+pCOde3QT7qo573/SKQ==} + os: [win32] + hasBin: true + + '@microsoft/sarif-multitool@5.5.0': + resolution: {integrity: sha512-KCO8ELLnoV0Z/4QeoKxe+YL3061Kolg1he18iTrWCEoq7+Zme9oUQ2NCuLD0he+lML5TnWtER7fG1zE1tObjxQ==} + os: [darwin, linux, win32] + hasBin: true + '@napi-rs/wasm-runtime@0.2.12': resolution: {integrity: sha512-ZVWUcfwY4E/yPitQJl481FjFo3K22D6qF0DuFH6Y/nbnE11GY5uguDxZMGXPQ8WQ0128MXQD7TnfHyK4oWoIJQ==} @@ -2114,6 +2156,10 @@ packages: resolution: {integrity: sha512-1td788aAnnZ5qs7V2QIRl1owjtYpbKt749Y3xauqQgwIIGF/xXWz1wMTEBx5O3LK3lXLVuqXPdPxj2BoFHaW9Q==} hasBin: true + jschardet@3.1.4: + resolution: {integrity: sha512-/kmVISmrwVwtyYU40iQUOp3SUPk2dhNCMsZBQX0R1/jZ8maaXJ/oZIzUOiyOqcgtLnETFKYChbJ5iDC/eWmFHg==} + engines: {node: '>=0.1.90'} + jsdoc-type-pratt-parser@7.3.0: resolution: {integrity: sha512-DoyJXo7x/n48M3NsGOs9QnEws0ft0tV3YsSgvWMNxz2hZtz+Q6fpqUD96lVYX4a+jcEkzHeFNDJOn68TzXfbdA==} engines: {node: '>=20.0.0'} @@ -2247,6 +2293,9 @@ packages: lodash.startcase@4.4.0: resolution: {integrity: sha512-+WKqsK294HMSc2jEbNgpHpd0JfIBhp7rEV4aqXWqFr6AlXov+SlcgB1Fv01y2kGe3Gc8nMW7VA0SrGuSkRfIEg==} + lodash@4.18.1: + resolution: {integrity: sha512-dMInicTPVE8d1e5otfwmmjlxkZoUpiVLwyeTdUsi/Caj/gfzzblBcCE5sRHV/AsjuCmxWrte2TNGSYuCeCq+0Q==} + longest-streak@3.1.0: resolution: {integrity: sha512-9Ri+o0JYgehTaVBBDoMqIl8GXtbWg711O3srftcHhZ0dqnETqLaoIK0x17fUw9rFSlK/0NlsKe0Ahhyl5pXE2g==} @@ -2942,6 +2991,9 @@ packages: uri-js@4.4.1: resolution: {integrity: sha512-7rKUyy33Q1yc98pQ1DAmLtwX109F7TIfWlW1Ydo8Wl1ii1SeHieeh0HHfPeL2fMXK6z0s8ecKs9frCuLJvndBg==} + utf8@3.0.0: + resolution: {integrity: sha512-E8VjFIQ/TyQgp+TZfS6l8yp/xWppSAHzidGiRrqe4bK4XP9pTRyKFgGJpO3SN7zdX4DeomTrwaseCHovfpFcqQ==} + valibot@1.4.2: resolution: {integrity: sha512-gjdCvJ6d3RyHAneqxMYMW9QMCwYMb3jpOO0IyHZV1bnRHFBHrX3VkIILt5XYR0WhwHiH7Mty8ovuPZ/O3gamrg==} peerDependencies: @@ -3462,6 +3514,27 @@ snapshots: globby: 11.1.0 read-yaml-file: 1.1.0 + '@microsoft/eslint-formatter-sarif@3.1.0': + dependencies: + jschardet: 3.1.4 + lodash: 4.18.1 + utf8: 3.0.0 + + '@microsoft/sarif-multitool-darwin@5.5.0': + optional: true + + '@microsoft/sarif-multitool-linux@5.5.0': + optional: true + + '@microsoft/sarif-multitool-win32@5.5.0': + optional: true + + '@microsoft/sarif-multitool@5.5.0': + optionalDependencies: + '@microsoft/sarif-multitool-darwin': 5.5.0 + '@microsoft/sarif-multitool-linux': 5.5.0 + '@microsoft/sarif-multitool-win32': 5.5.0 + '@napi-rs/wasm-runtime@0.2.12': dependencies: '@emnapi/core': 1.9.2 @@ -4619,6 +4692,8 @@ snapshots: dependencies: argparse: 2.0.1 + jschardet@3.1.4: {} + jsdoc-type-pratt-parser@7.3.0: {} jsesc@3.1.0: {} @@ -4717,6 +4792,8 @@ snapshots: lodash.startcase@4.4.0: {} + lodash@4.18.1: {} + longest-streak@3.1.0: {} lru-cache@11.5.1: {} @@ -5641,6 +5718,8 @@ snapshots: dependencies: punycode: 2.3.1 + utf8@3.0.0: {} + valibot@1.4.2(typescript@6.0.3): optionalDependencies: typescript: 6.0.3 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 7be05b3c..8e805d39 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -1,4 +1,8 @@ allowBuilds: + # Platform packages chmod their bundled self-contained binary. + '@microsoft/sarif-multitool-darwin': true + '@microsoft/sarif-multitool-linux': true + '@microsoft/sarif-multitool-win32': true esbuild: true unrs-resolver: false @@ -8,6 +12,8 @@ catalog: '@eslint/json': ^2.0.1 '@eslint/markdown': ^8.0.3 '@faker-js/faker': ^10.4.0 + '@microsoft/eslint-formatter-sarif': ^3.1.0 + '@microsoft/sarif-multitool': ^5.5.0 '@pnpm/lockfile.types': ^1100.0.5 '@prettier/plugin-xml': ^3.4.2 '@stylistic/eslint-plugin': ^5.10.0 @@ -67,6 +73,13 @@ minimumReleaseAge: 4320 minimumReleaseAgeExclude: - '@gtbuchanan/*' +overrides: + # The SARIF formatter wrongly declares eslint v8 as a regular dependency + # (it only uses the host ESLint via require.main.require). Left in place, + # its v8 bin shadows the workspace's ESLint in packages that depend on + # the formatter. Remove it from the graph entirely. + '@microsoft/eslint-formatter-sarif>eslint': '-' + packages: - 'packages/*' diff --git a/turbo.json b/turbo.json index b6ac48aa..e6a19faa 100644 --- a/turbo.json +++ b/turbo.json @@ -12,7 +12,8 @@ "!packages/*/**" ], "outputs": [ - "dist/.eslintcache" + "dist/.eslintcache", + "dist/sarif/eslint.sarif" ] }, "build": { @@ -128,7 +129,8 @@ "eslint.config.*" ], "outputs": [ - "dist/.eslintcache" + "dist/.eslintcache", + "dist/sarif/eslint.sarif" ] }, "pack": {