diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9b44a05..43718d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -28,7 +28,7 @@ jobs: e2e: name: Run the action on the fixture - # The fixture rule reports a finding only on changed lines, and only a pull request has changed lines. + # The summary comment needs a pull request. if: github.event_name == 'pull_request' runs-on: ubuntu-latest permissions: @@ -43,5 +43,26 @@ jobs: version: 0.7.3 working-directory: test/fixture check-name: alint (fixture) - - name: Report the outputs - run: echo "errors=${{ steps.alint.outputs.errors }} warnings=${{ steps.alint.outputs.warnings }}" + # Most pull requests do not change the fixture, so the changed-line scope finds nothing there. + files: src + # The fixture rule does not call this provider. The configuration proves that alint accepts the file. + provider-config: | + version = 1 + + [[providers]] + id = "fixture" + type = "openai-compatible" + endpoint = "https://provider.invalid/v1" + + [[providers.models]] + id = "fixture-model" + size = "small" + aliases = [ "default" ] + capabilities = [ "tool-call" ] + + [providers.models.default_params] + thinking = { type = "disabled" } + - name: Check that alint reads the provider configuration + run: alint config models show fixture/fixture-model + - name: Check the outputs + run: test "${{ steps.alint.outputs.errors }}/${{ steps.alint.outputs.warnings }}" = "0/1" diff --git a/README.md b/README.md index 565fb2f..3fdda5a 100644 --- a/README.md +++ b/README.md @@ -41,14 +41,45 @@ If the `alint` configuration imports packages, install the project dependencies | Input | Default | Description | | --- | --- | --- | | `version` | | Version of `@alint-js/cli` to install. When empty, the action uses the `alint` in `node_modules`, or installs the latest version. | -| `provider-endpoint` | | Endpoint of an OpenAI-compatible model provider. When empty, the action does not write provider configuration. | +| `provider-endpoint` | | Endpoint of an OpenAI-compatible model provider. | | `provider-model` | | Model id for the provider. Required with `provider-endpoint`. | | `api-key` | | API key for the provider. The action sends it as a Bearer token. | +| `provider-config` | | Complete provider configuration in TOML. Use it instead of `provider-endpoint`. | | `files` | | Files or directories to lint, separated by spaces or lines. | | `working-directory` | `.` | Directory that contains the `alint` configuration. | | `check-name` | `alint` | Name of the check run. | | `github-token` | `github.token` | Token that creates the check run and the summary comment. | +When `provider-endpoint` and `provider-config` are empty, the action does not write provider configuration. + +### Provider configuration + +`provider-endpoint` writes one model with no other properties. Use `provider-config` when a rule requests a model by alias or by capability, or when the model needs default parameters: + +```yaml +- uses: alint-dev/setup@main + with: + provider-config: | + version = 1 + + [[providers]] + id = "deepseek" + type = "openai-compatible" + endpoint = "https://api.deepseek.com/v1" + + [providers.headers] + Authorization = "Bearer ${{ secrets.DEEPSEEK_API_KEY }}" + + [[providers.models]] + id = "deepseek-flash" + size = "small" + aliases = [ "default" ] + capabilities = [ "tool-call" ] + + [providers.models.default_params] + thinking = { type = "disabled" } +``` + ## Outputs | Output | Description | @@ -62,9 +93,11 @@ If the `alint` configuration imports packages, install the project dependencies - The step itself fails only when `alint` cannot complete the run or GitHub rejects a request. - Each run rewrites the same summary comment. A pull request that never had findings gets no comment. - On a pull request, the action reads the changed files from the GitHub API. The checkout does not need the full history. +- On a push, the action lints the files that the push changed and writes no comment. A new branch is compared with the default branch. +- On other events, for example a manual run, the action lints the working directory. - When `files` is set, the action lints those files and keeps each finding. -- On other events, the action lints the working directory and writes no comment. - A pull request from a fork gets no secrets and a read-only token. The action skips it with a notice. +- With `provider-endpoint`, an empty `api-key` also skips the action. A Dependabot pull request has no access to secrets, for example. With `provider-config`, add your own `if` condition for such runs. ## When not to use it diff --git a/action.yml b/action.yml index d54f84d..4ce06c9 100644 --- a/action.yml +++ b/action.yml @@ -13,14 +13,21 @@ inputs: default: '' provider-endpoint: description: >- - Endpoint of an OpenAI-compatible model provider. When empty, the action does not write - provider configuration. + Endpoint of an OpenAI-compatible model provider. When this input and provider-config are + empty, the action does not write provider configuration. default: '' provider-model: description: Model id for the provider. Required with provider-endpoint. default: '' api-key: - description: API key for the provider. The action sends it as a Bearer token. + description: >- + API key for the provider. The action sends it as a Bearer token. With provider-endpoint, + an empty key skips the action, because the run has no access to secrets. + default: '' + provider-config: + description: >- + Complete alint provider configuration in TOML. Use it instead of provider-endpoint when a + model needs aliases, capabilities, or default parameters. default: '' files: description: >- @@ -48,13 +55,26 @@ outputs: runs: using: composite steps: - # A pull request from a fork gets no secrets and a read-only token, so the action cannot publish there. - id: guard shell: bash + env: + ALINT_API_KEY: ${{ inputs.api-key }} + ALINT_CONFIG: ${{ inputs.provider-config }} + ALINT_ENDPOINT: ${{ inputs.provider-endpoint }} run: | + if [ -n "$ALINT_CONFIG" ] && [ -n "$ALINT_ENDPOINT" ]; then + echo "::error title=alint::Set provider-config or provider-endpoint, not the two." + exit 1 + fi + + # A pull request from a fork gets no secrets and a read-only token, so the action cannot publish there. if [ "$GITHUB_EVENT_NAME" = "pull_request" ] && [ "$(jq -r '.pull_request.head.repo.full_name' "$GITHUB_EVENT_PATH")" != "$GITHUB_REPOSITORY" ]; then echo "::notice title=alint::Skipped. A pull request from a fork cannot publish the result." echo "skip=true" >> "$GITHUB_OUTPUT" + # Other runs without secrets, for example a Dependabot pull request, show as an empty key. + elif [ -n "$ALINT_ENDPOINT" ] && [ -z "$ALINT_API_KEY" ]; then + echo "::notice title=alint::Skipped. The api-key input is empty." + echo "skip=true" >> "$GITHUB_OUTPUT" fi - id: install @@ -85,6 +105,16 @@ runs: --provider-model "$ALINT_MODEL" \ ${ALINT_API_KEY:+--provider-header "Authorization=Bearer $ALINT_API_KEY"} + - if: steps.guard.outputs.skip != 'true' && inputs.provider-config != '' + shell: bash + env: + ALINT_CONFIG: ${{ inputs.provider-config }} + run: | + # alint reads the global configuration from this path on each platform. + directory="${XDG_CONFIG_HOME:-$HOME/.config}/alint" + mkdir -p "$directory" + (umask 077 && printf '%s\n' "$ALINT_CONFIG" > "$directory/config.toml") + - if: steps.guard.outputs.skip != 'true' shell: bash working-directory: ${{ inputs.working-directory }} diff --git a/dist/index.mjs b/dist/index.mjs index 839e027..766c926 100644 --- a/dist/index.mjs +++ b/dist/index.mjs @@ -513,6 +513,7 @@ function toChangedLines(files) { //#region src/findings.ts /** Identifies the summary comment so that later runs rewrite it instead of adding another one. */ const summaryMarker = ""; +const suggestionParagraph = /\n\s*Suggestion:/u; const maxSummaryRows = 100; function formatSummary(findings, context) { const icon = toConclusion(findings) === "failure" ? "❌" : findings.length > 0 ? "⚠️" : "✅"; @@ -535,7 +536,7 @@ function formatSummary(findings, context) { const footer = [ `Commit \`${shortSha}\``, context.runUrl === void 0 ? void 0 : `[run](${context.runUrl})`, - `${context.totalTokens.toLocaleString("en-US")} tokens` + context.usage ].filter((part) => part !== void 0); lines.push("", `${footer.join(" · ")}`); return lines.join("\n"); @@ -546,6 +547,11 @@ function formatTitle(findings) { const warnings = findings.length - errors; return [errors > 0 ? pluralize(errors, "error") : void 0, warnings > 0 ? pluralize(warnings, "warning") : void 0].filter((part) => part !== void 0).join(", "); } +/** States the cost of a run, so that a reader can see from the check run whether the cache works. */ +function formatUsage(result) { + const tokens = `${result.usage.inputTokens.toLocaleString("en-US")} input / ${result.usage.outputTokens.toLocaleString("en-US")} output tokens`; + return result.execution === void 0 ? tokens : `${pluralize(result.execution.completed, "rule run")}, ${result.execution.cached} cached · ${tokens}`; +} function toAnnotations(findings) { return findings.map((finding) => ({ annotation_level: finding.severity === "error" ? "failure" : "warning", @@ -562,14 +568,17 @@ function toConclusion(findings) { } /** Converts diagnostics to findings with repository-relative paths. Errors come first. */ function toFindings(diagnostics, toRepositoryPath) { - return diagnostics.map((diagnostic) => ({ - line: diagnostic.loc?.start.line ?? 1, - message: diagnostic.message.trim(), - path: toRepositoryPath(diagnostic.filePath), - ruleId: diagnostic.ruleId, - severity: diagnostic.severity, - suggestion: suggestionFrom(diagnostic.evidence) - })).sort((a, b) => Number(b.severity === "error") - Number(a.severity === "error") || a.path.localeCompare(b.path) || a.line - b.line); + return diagnostics.map((diagnostic) => { + const [message = "", ...paragraphs] = diagnostic.message.split(suggestionParagraph); + return { + line: diagnostic.loc?.start.line ?? 1, + message: message.trim(), + path: toRepositoryPath(diagnostic.filePath), + ruleId: diagnostic.ruleId, + severity: diagnostic.severity, + suggestion: suggestionFrom(diagnostic.evidence) ?? (paragraphs.join("\n").trim() || void 0) + }; + }).sort((a, b) => Number(b.severity === "error") - Number(a.severity === "error") || a.path.localeCompare(b.path) || a.line - b.line); } function escapeCell(value) { return value.replaceAll("|", "\\|").replaceAll(/\s*\n\s*/gu, " "); @@ -595,6 +604,11 @@ const filesSchema = /* @__PURE__ */ array(/* @__PURE__ */ object({ patch: /* @__PURE__ */ optional(/* @__PURE__ */ string()), status: /* @__PURE__ */ string() })); +const comparisonSchema = /* @__PURE__ */ object({ files: filesSchema }); +/** Lists the files that differ between two commits. GitHub returns at most 300 files here. */ +async function listComparedFiles(repository, base, head) { + return parse(comparisonSchema, await request(repository, "GET", `/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`)).files; +} async function listPullRequestFiles(repository, pullRequest) { const files = []; for (let page = 1;; page += 1) { @@ -1018,7 +1032,14 @@ const runResultSchema = /* @__PURE__ */ object({ ruleId: /* @__PURE__ */ string(), severity: /* @__PURE__ */ picklist(["error", "warn"]) })), - usage: /* @__PURE__ */ object({ totalTokens: /* @__PURE__ */ number() }) + execution: /* @__PURE__ */ optional(/* @__PURE__ */ object({ + cached: /* @__PURE__ */ number(), + completed: /* @__PURE__ */ number() + })), + usage: /* @__PURE__ */ object({ + inputTokens: /* @__PURE__ */ number(), + outputTokens: /* @__PURE__ */ number() + }) }); function parseRunResult(json) { return parse(runResultSchema, JSON.parse(json)); @@ -1037,10 +1058,16 @@ async function runAlint(command, targets, cwd) { } //#endregion //#region src/main.ts -const eventSchema = /* @__PURE__ */ object({ pull_request: /* @__PURE__ */ optional(/* @__PURE__ */ object({ - head: /* @__PURE__ */ object({ sha: /* @__PURE__ */ string() }), - number: /* @__PURE__ */ number() -})) }); +const eventSchema = /* @__PURE__ */ object({ + after: /* @__PURE__ */ optional(/* @__PURE__ */ string()), + before: /* @__PURE__ */ optional(/* @__PURE__ */ string()), + pull_request: /* @__PURE__ */ optional(/* @__PURE__ */ object({ + head: /* @__PURE__ */ object({ sha: /* @__PURE__ */ string() }), + number: /* @__PURE__ */ number() + })), + repository: /* @__PURE__ */ optional(/* @__PURE__ */ object({ default_branch: /* @__PURE__ */ string() })) +}); +const zeroCommit = /^0+$/u; async function run(env, cwd) { const repository = { apiUrl: env.GITHUB_API_URL ?? "https://api.github.com", @@ -1049,13 +1076,17 @@ async function run(env, cwd) { }; const workspace = env.GITHUB_WORKSPACE ?? cwd; const serverUrl = env.GITHUB_SERVER_URL ?? "https://github.com"; - const pullRequest = await readPullRequest(env.GITHUB_EVENT_PATH); - const sha = pullRequest?.sha ?? required(env, "GITHUB_SHA"); + const event = await readEvent(env.GITHUB_EVENT_PATH); + const pullRequest = event.pull_request; + const sha = pullRequest?.head.sha ?? required(env, "GITHUB_SHA"); const toRepositoryPath = (filePath) => relative(workspace, resolve$1(cwd, filePath)); - const scope = await resolveScope(repository, pullRequest, env.INPUT_FILES ?? "", cwd, workspace); + const scope = await resolveScope(repository, event, env.INPUT_FILES ?? "", cwd, workspace); const result = scope.targets.length === 0 ? { diagnostics: [], - usage: { totalTokens: 0 } + usage: { + inputTokens: 0, + outputTokens: 0 + } } : await runAlint(required(env, "ALINT_COMMAND"), scope.targets, cwd); const findings = toFindings(scope.changedLines === void 0 ? result.diagnostics : filterToChangedLines(result.diagnostics, scope.changedLines, toRepositoryPath), toRepositoryPath); const title = formatTitle(findings); @@ -1064,7 +1095,7 @@ async function run(env, cwd) { runUrl: env.GITHUB_RUN_ID === void 0 ? void 0 : `${serverUrl}/${repository.repository}/actions/runs/${env.GITHUB_RUN_ID}`, serverUrl, sha, - totalTokens: result.usage.totalTokens + usage: formatUsage(result) }); if (env.GITHUB_STEP_SUMMARY !== void 0) await appendFile(env.GITHUB_STEP_SUMMARY, `${summary}\n`); if (env.GITHUB_OUTPUT !== void 0) { @@ -1082,13 +1113,16 @@ async function run(env, cwd) { if (pullRequest !== void 0) await publishSummaryComment(repository, pullRequest.number, summary, findings.length > 0); console.info(`Published ${title.toLowerCase()} to ${repository.repository}@${sha.slice(0, 7)}.`); } -async function readPullRequest(eventPath) { - if (eventPath === void 0) return; - const event = parse(eventSchema, JSON.parse(await readFile(eventPath, "utf8"))); - return event.pull_request === void 0 ? void 0 : { - number: event.pull_request.number, - sha: event.pull_request.head.sha - }; +/** Lists the files that the event changed. `undefined` means that the event has no changes, for example a manual run. */ +async function listChangedFiles(repository, event) { + if (event.pull_request !== void 0) return listPullRequestFiles(repository, event.pull_request.number); + if (event.before === void 0 || event.after === void 0) return; + if (zeroCommit.test(event.after)) return []; + const base = zeroCommit.test(event.before) ? event.repository?.default_branch : event.before; + return base === void 0 ? void 0 : listComparedFiles(repository, base, event.after); +} +async function readEvent(eventPath) { + return eventPath === void 0 ? {} : parse(eventSchema, JSON.parse(await readFile(eventPath, "utf8"))); } function required(env, name) { const value = env[name]; @@ -1098,14 +1132,15 @@ function required(env, name) { /** * Selects what alint reads. * -* Explicit files win. A pull request without explicit files gets its changed files, +* Explicit files win. A pull request or a push without explicit files gets its changed files, * and the result keeps only the changed lines. Other events get the whole working directory. */ -async function resolveScope(repository, pullRequest, files, cwd, workspace) { +async function resolveScope(repository, event, files, cwd, workspace) { const explicit = files.split(/\s+/u).filter(Boolean); if (explicit.length > 0) return { targets: explicit }; - if (pullRequest === void 0) return { targets: ["."] }; - const changedLines = toChangedLines(await listPullRequestFiles(repository, pullRequest.number)); + const changedFiles = await listChangedFiles(repository, event); + if (changedFiles === void 0) return { targets: ["."] }; + const changedLines = toChangedLines(changedFiles); return { changedLines, targets: [...changedLines.keys()].map((path) => relative(cwd, resolve$1(workspace, path))).filter((path) => !path.startsWith("..") && existsSync(resolve$1(cwd, path))) diff --git a/src/changes.ts b/src/changes.ts index b057c9d..9aa681e 100644 --- a/src/changes.ts +++ b/src/changes.ts @@ -1,5 +1,12 @@ import type { Diagnostic } from './output' +export interface ChangedFile { + filename: string + /** GitHub omits the patch of a binary file and of a very large diff. */ + patch?: string + status: string +} + /** Changed lines of each file. `all` means that GitHub did not send a patch, so each line counts. */ export type ChangedLines = ReadonlyMap @@ -8,13 +15,6 @@ export interface LineRange { startLine: number } -export interface PullRequestFile { - filename: string - /** GitHub omits the patch of a binary file and of a very large diff. */ - patch?: string - status: string -} - const hunkHeader = /^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@/u /** Keeps file-wide diagnostics and diagnostics whose location intersects a changed line. */ @@ -64,7 +64,7 @@ export function parsePatch(patch: string): LineRange[] { return ranges } -export function toChangedLines(files: readonly PullRequestFile[]): ChangedLines { +export function toChangedLines(files: readonly ChangedFile[]): ChangedLines { return new Map( files .filter(file => file.status !== 'removed') diff --git a/src/findings.test.ts b/src/findings.test.ts index 6445d45..e7b11cf 100644 --- a/src/findings.test.ts +++ b/src/findings.test.ts @@ -2,9 +2,9 @@ import type { Diagnostic } from './output' import { describe, expect, it } from 'vitest' -import { formatSummary, formatTitle, summaryMarker, toAnnotations, toConclusion, toFindings } from './findings' +import { formatSummary, formatTitle, formatUsage, summaryMarker, toAnnotations, toConclusion, toFindings } from './findings' -const context = { repository: 'acme/app', serverUrl: 'https://github.com', sha: '0123456789abcdef', totalTokens: 1234 } +const context = { repository: 'acme/app', serverUrl: 'https://github.com', sha: '0123456789abcdef', usage: '1,000 input / 234 output tokens' } const identity = (filePath: string) => filePath function diagnostic(overrides: Partial): Diagnostic { @@ -51,6 +51,25 @@ describe('findings', () => { }) }) + it('reads the suggestion from a Suggestion paragraph of the message', () => { + const findings = toFindings([ + diagnostic({ message: 'Two layers are mixed.\nSuggestion: Extract the storage calls.' }), + diagnostic({ evidence: { suggestion: 'From evidence.' }, message: 'Two layers are mixed.\nSuggestion: From message.' }), + ], identity) + + expect(findings[0]?.message).toBe('Two layers are mixed.') + expect(findings[0]?.suggestion).toBe('Extract the storage calls.') + expect(findings[1]?.message).toBe('Two layers are mixed.') + expect(findings[1]?.suggestion).toBe('From evidence.') + }) + + it('states the rule runs and the tokens of a run', () => { + const usage = { inputTokens: 1000, outputTokens: 234 } + + expect(formatUsage({ execution: { cached: 12, completed: 1 }, usage })).toBe('1 rule run, 12 cached · 1,000 input / 234 output tokens') + expect(formatUsage({ usage })).toBe('1,000 input / 234 output tokens') + }) + it('fails the check only when a finding is an error', () => { const warnings = toFindings([diagnostic({})], identity) const errors = toFindings([diagnostic({}), diagnostic({ severity: 'error' })], identity) @@ -71,7 +90,7 @@ describe('findings', () => { expect(summary.startsWith(summaryMarker)).toBe(true) expect(summary).toContain('### ❌ alint · 1 error') expect(summary).toContain('| ❌ | [`src/a.ts:3`](https://github.com/acme/app/blob/0123456789abcdef/src/a.ts#L3) | `demo/rule` | First line. Second line.
💡 Use a \\| b. |') - expect(summary).toContain('Commit `0123456` · [run](https://github.com/acme/app/actions/runs/7) · 1,234 tokens') + expect(summary).toContain('Commit `0123456` · [run](https://github.com/acme/app/actions/runs/7) · 1,000 input / 234 output tokens') }) it('formats a clean result without a table', () => { diff --git a/src/findings.ts b/src/findings.ts index bfee7f5..69a2c0b 100644 --- a/src/findings.ts +++ b/src/findings.ts @@ -1,4 +1,4 @@ -import type { Diagnostic } from './output' +import type { Diagnostic, RunResult } from './output' export interface Annotation { annotation_level: 'failure' | 'warning' @@ -25,12 +25,15 @@ export interface SummaryContext { runUrl?: string serverUrl: string sha: string - totalTokens: number + /** The cost of the run, from `formatUsage`. */ + usage: string } /** Identifies the summary comment so that later runs rewrite it instead of adding another one. */ export const summaryMarker = '' +const suggestionParagraph = /\n\s*Suggestion:/u + // GitHub rejects a comment or a check summary above 65536 characters. // One row is a few hundred characters, so this cap keeps the table far below the limit. const maxSummaryRows = 100 @@ -63,7 +66,7 @@ export function formatSummary(findings: readonly Finding[], context: SummaryCont const footer = [ `Commit \`${shortSha}\``, context.runUrl === undefined ? undefined : `[run](${context.runUrl})`, - `${context.totalTokens.toLocaleString('en-US')} tokens`, + context.usage, ].filter(part => part !== undefined) lines.push('', `${footer.join(' · ')}`) @@ -85,6 +88,15 @@ export function formatTitle(findings: readonly Finding[]): string { ].filter(part => part !== undefined).join(', ') } +/** States the cost of a run, so that a reader can see from the check run whether the cache works. */ +export function formatUsage(result: Pick): string { + const tokens = `${result.usage.inputTokens.toLocaleString('en-US')} input / ${result.usage.outputTokens.toLocaleString('en-US')} output tokens` + + return result.execution === undefined + ? tokens + : `${pluralize(result.execution.completed, 'rule run')}, ${result.execution.cached} cached · ${tokens}` +} + export function toAnnotations(findings: readonly Finding[]): Annotation[] { return findings.map(finding => ({ annotation_level: finding.severity === 'error' ? 'failure' : 'warning', @@ -110,15 +122,21 @@ export function toFindings( toRepositoryPath: (filePath: string) => string, ): Finding[] { return diagnostics - .map((diagnostic): Finding => ({ - // A diagnostic without a location is about the whole file. An annotation needs a line. - line: diagnostic.loc?.start.line ?? 1, - message: diagnostic.message.trim(), - path: toRepositoryPath(diagnostic.filePath), - ruleId: diagnostic.ruleId, - severity: diagnostic.severity, - suggestion: suggestionFrom(diagnostic.evidence), - })) + .map((diagnostic): Finding => { + // Some rules append the remediation to the message as a `Suggestion:` paragraph. + // https://github.com/alint-dev/alint/blob/ab0be9a/packages/plugin-js/src/rules/no-mixed-layers-without-abstraction/rule.ts#L391-L396 + const [message = '', ...paragraphs] = diagnostic.message.split(suggestionParagraph) + + return { + // A diagnostic without a location is about the whole file. An annotation needs a line. + line: diagnostic.loc?.start.line ?? 1, + message: message.trim(), + path: toRepositoryPath(diagnostic.filePath), + ruleId: diagnostic.ruleId, + severity: diagnostic.severity, + suggestion: suggestionFrom(diagnostic.evidence) ?? (paragraphs.join('\n').trim() || undefined), + } + }) .sort((a, b) => Number(b.severity === 'error') - Number(a.severity === 'error') || a.path.localeCompare(b.path) diff --git a/src/github.ts b/src/github.ts index dc638ee..8ba6a7f 100644 --- a/src/github.ts +++ b/src/github.ts @@ -1,4 +1,4 @@ -import type { PullRequestFile } from './changes' +import type { ChangedFile } from './changes' import type { Annotation, Conclusion } from './findings' import { array, number, object, optional, parse, string } from 'valibot' @@ -28,9 +28,17 @@ const itemsPerPage = 100 const checkRunSchema = object({ id: number() }) const commentsSchema = array(object({ body: optional(string()), id: number() })) const filesSchema = array(object({ filename: string(), patch: optional(string()), status: string() })) +const comparisonSchema = object({ files: filesSchema }) -export async function listPullRequestFiles(repository: Repository, pullRequest: number): Promise { - const files: PullRequestFile[] = [] +/** Lists the files that differ between two commits. GitHub returns at most 300 files here. */ +export async function listComparedFiles(repository: Repository, base: string, head: string): Promise { + const comparison = parse(comparisonSchema, await request(repository, 'GET', `/compare/${encodeURIComponent(base)}...${encodeURIComponent(head)}`)) + + return comparison.files +} + +export async function listPullRequestFiles(repository: Repository, pullRequest: number): Promise { + const files: ChangedFile[] = [] for (let page = 1; ; page += 1) { const items = parse(filesSchema, await request(repository, 'GET', `/pulls/${pullRequest}/files?per_page=${itemsPerPage}&page=${page}`)) diff --git a/src/main.test.ts b/src/main.test.ts index 6cfc33a..10f245f 100644 --- a/src/main.test.ts +++ b/src/main.test.ts @@ -1,6 +1,6 @@ import type { AddressInfo } from 'node:net' -import type { PullRequestFile } from './changes' +import type { ChangedFile } from './changes' import type { Diagnostic } from './output' import { chmod, mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises' @@ -15,7 +15,7 @@ import { run } from './main' interface FakeApi { comments?: Array<{ body: string, id: number }> - files?: PullRequestFile[] + files?: ChangedFile[] status?: number } @@ -112,15 +112,38 @@ describe('action', () => { expect(api.requests[0]?.body?.output.annotations).toHaveLength(1) }) - it('lints the working directory and writes no comment outside a pull request', async () => { + it('lints the changes of a push and writes no comment', async () => { + const api = await startGitHubApi({ files: [{ filename: 'src/a.ts', patch: '@@ -1,0 +2 @@', status: 'modified' }] }) + const workspace = await createWorkspace({ diagnostics: [warning('src/a.ts', 2), warning('src/a.ts', 40)] }) + + await run({ ...environment(workspace, api.url), GITHUB_EVENT_PATH: join(workspace, 'push.json') }, workspace) + + expect(await readArguments(workspace)).toEqual(['--format', 'json', 'src/a.ts']) + expect(api.requests.map(request => `${request.method} ${request.url}`)).toEqual([ + 'GET /repos/acme/app/compare/1111111...2222222', + 'POST /repos/acme/app/check-runs', + ]) + expect(api.requests[1]?.body?.head_sha).toBe('push-commit') + expect(api.requests[1]?.body?.output.annotations).toHaveLength(1) + }) + + it('compares a new branch with the default branch', async () => { + const api = await startGitHubApi({ files: [] }) + const workspace = await createWorkspace({ diagnostics: [] }) + + await run({ ...environment(workspace, api.url), GITHUB_EVENT_PATH: join(workspace, 'new-branch.json') }, workspace) + + expect(api.requests[0]?.url).toBe('/repos/acme/app/compare/main...2222222') + }) + + it('lints the working directory for an event without changes', async () => { const api = await startGitHubApi({}) const workspace = await createWorkspace({ diagnostics: [warning('src/a.ts', 40)] }) - await run({ ...environment(workspace, api.url), GITHUB_EVENT_PATH: join(workspace, 'push.json') }, workspace) + await run({ ...environment(workspace, api.url), GITHUB_EVENT_PATH: join(workspace, 'manual.json') }, workspace) expect(await readArguments(workspace)).toEqual(['--format', 'json', '.']) expect(api.requests.map(request => `${request.method} ${request.url}`)).toEqual(['POST /repos/acme/app/check-runs']) - expect(api.requests[0]?.body?.head_sha).toBe('push-commit') }) it('sends annotations above the request limit in update requests', async () => { @@ -173,7 +196,9 @@ async function createWorkspace(result: { diagnostics: Diagnostic[], exitCode?: n await writeFile(join(workspace, 'src/a.ts'), '') await writeFile(join(workspace, 'packages/app/src/a.ts'), '') await writeFile(join(workspace, 'event.json'), JSON.stringify({ pull_request: { head: { sha: 'abcdef0123' }, number: 12 } })) - await writeFile(join(workspace, 'push.json'), JSON.stringify({ ref: 'refs/heads/main' })) + await writeFile(join(workspace, 'push.json'), JSON.stringify({ after: '2222222', before: '1111111', repository: { default_branch: 'main' } })) + await writeFile(join(workspace, 'new-branch.json'), JSON.stringify({ after: '2222222', before: '0000000', repository: { default_branch: 'main' } })) + await writeFile(join(workspace, 'manual.json'), JSON.stringify({ inputs: {} })) await writeFile(join(workspace, 'result.json'), JSON.stringify({ exitCode: result.exitCode ?? 0, stderr: result.stderr ?? '', @@ -232,6 +257,11 @@ async function startGitHubApi(options: FakeApi) { return } + if (url.includes('/compare/')) { + response.end(JSON.stringify({ files: options.files ?? [] })) + return + } + response.end(JSON.stringify(url.includes('/pulls/') ? options.files ?? [] : options.comments ?? [])) }) }) diff --git a/src/main.ts b/src/main.ts index 3375b99..6cc33b4 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1,5 +1,8 @@ -import type { ChangedLines } from './changes' +import type { InferOutput } from 'valibot' + +import type { ChangedFile, ChangedLines } from './changes' import type { Repository } from './github' +import type { RunResult } from './output' import { existsSync } from 'node:fs' import { appendFile, readFile } from 'node:fs/promises' @@ -8,25 +11,29 @@ import { relative, resolve } from 'pathe' import { number, object, optional, parse, string } from 'valibot' import { filterToChangedLines, toChangedLines } from './changes' -import { formatSummary, formatTitle, toAnnotations, toConclusion, toFindings } from './findings' -import { listPullRequestFiles, publishCheckRun, publishSummaryComment } from './github' +import { formatSummary, formatTitle, formatUsage, toAnnotations, toConclusion, toFindings } from './findings' +import { listComparedFiles, listPullRequestFiles, publishCheckRun, publishSummaryComment } from './github' import { runAlint } from './lint' -interface PullRequest { - number: number - sha: string -} +type Event = InferOutput interface Scope { - /** Present when the scope is the changes of a pull request. */ + /** Present when the scope is the changes of a pull request or a push. */ changedLines?: ChangedLines targets: string[] } +// The part of the pull request and push event payloads that this action reads. const eventSchema = object({ + after: optional(string()), + before: optional(string()), pull_request: optional(object({ head: object({ sha: string() }), number: number() })), + repository: optional(object({ default_branch: string() })), }) +// Git uses this id for the missing side when a push creates or deletes a branch. +const zeroCommit = /^0+$/u + export async function run(env: NodeJS.ProcessEnv, cwd: string): Promise { const repository: Repository = { apiUrl: env.GITHUB_API_URL ?? 'https://api.github.com', @@ -35,15 +42,16 @@ export async function run(env: NodeJS.ProcessEnv, cwd: string): Promise { } const workspace = env.GITHUB_WORKSPACE ?? cwd const serverUrl = env.GITHUB_SERVER_URL ?? 'https://github.com' - const pullRequest = await readPullRequest(env.GITHUB_EVENT_PATH) + const event = await readEvent(env.GITHUB_EVENT_PATH) + const pullRequest = event.pull_request // On a pull request event, GITHUB_SHA is the temporary merge commit. // A check run on that commit does not show on the pull request, so the head commit comes first. - const sha = pullRequest?.sha ?? required(env, 'GITHUB_SHA') + const sha = pullRequest?.head.sha ?? required(env, 'GITHUB_SHA') const toRepositoryPath = (filePath: string) => relative(workspace, resolve(cwd, filePath)) - const scope = await resolveScope(repository, pullRequest, env.INPUT_FILES ?? '', cwd, workspace) - const result = scope.targets.length === 0 - ? { diagnostics: [], usage: { totalTokens: 0 } } + const scope = await resolveScope(repository, event, env.INPUT_FILES ?? '', cwd, workspace) + const result: RunResult = scope.targets.length === 0 + ? { diagnostics: [], usage: { inputTokens: 0, outputTokens: 0 } } : await runAlint(required(env, 'ALINT_COMMAND'), scope.targets, cwd) const diagnostics = scope.changedLines === undefined ? result.diagnostics @@ -56,7 +64,7 @@ export async function run(env: NodeJS.ProcessEnv, cwd: string): Promise { runUrl: env.GITHUB_RUN_ID === undefined ? undefined : `${serverUrl}/${repository.repository}/actions/runs/${env.GITHUB_RUN_ID}`, serverUrl, sha, - totalTokens: result.usage.totalTokens, + usage: formatUsage(result), }) if (env.GITHUB_STEP_SUMMARY !== undefined) { @@ -85,16 +93,31 @@ export async function run(env: NodeJS.ProcessEnv, cwd: string): Promise { console.info(`Published ${title.toLowerCase()} to ${repository.repository}@${sha.slice(0, 7)}.`) } -async function readPullRequest(eventPath: string | undefined): Promise { - if (eventPath === undefined) { +/** Lists the files that the event changed. `undefined` means that the event has no changes, for example a manual run. */ +async function listChangedFiles(repository: Repository, event: Event): Promise { + if (event.pull_request !== undefined) { + return listPullRequestFiles(repository, event.pull_request.number) + } + + if (event.before === undefined || event.after === undefined) { return undefined } - const event = parse(eventSchema, JSON.parse(await readFile(eventPath, 'utf8'))) + // A push that deletes a branch has no commit to read. + if (zeroCommit.test(event.after)) { + return [] + } + + // A push that creates a branch has no previous commit, so the default branch is the base. + const base = zeroCommit.test(event.before) ? event.repository?.default_branch : event.before - return event.pull_request === undefined - ? undefined - : { number: event.pull_request.number, sha: event.pull_request.head.sha } + return base === undefined ? undefined : listComparedFiles(repository, base, event.after) +} + +async function readEvent(eventPath: string | undefined): Promise { + return eventPath === undefined + ? {} + : parse(eventSchema, JSON.parse(await readFile(eventPath, 'utf8'))) } function required(env: NodeJS.ProcessEnv, name: string): string { @@ -110,12 +133,12 @@ function required(env: NodeJS.ProcessEnv, name: string): string { /** * Selects what alint reads. * - * Explicit files win. A pull request without explicit files gets its changed files, + * Explicit files win. A pull request or a push without explicit files gets its changed files, * and the result keeps only the changed lines. Other events get the whole working directory. */ async function resolveScope( repository: Repository, - pullRequest: PullRequest | undefined, + event: Event, files: string, cwd: string, workspace: string, @@ -126,11 +149,13 @@ async function resolveScope( return { targets: explicit } } - if (pullRequest === undefined) { + const changedFiles = await listChangedFiles(repository, event) + + if (changedFiles === undefined) { return { targets: ['.'] } } - const changedLines = toChangedLines(await listPullRequestFiles(repository, pullRequest.number)) + const changedLines = toChangedLines(changedFiles) const targets = [...changedLines.keys()] .map(path => relative(cwd, resolve(workspace, path))) // A file outside the working directory belongs to another project. A file that the checkout does not have cannot be read. diff --git a/src/output.ts b/src/output.ts index 3542bdd..412112d 100644 --- a/src/output.ts +++ b/src/output.ts @@ -14,7 +14,9 @@ const runResultSchema = object({ ruleId: string(), severity: picklist(['error', 'warn']), })), - usage: object({ totalTokens: number() }), + // Counts of rule executions. Output from an old alint version does not have them. + execution: optional(object({ cached: number(), completed: number() })), + usage: object({ inputTokens: number(), outputTokens: number() }), }) export type Diagnostic = RunResult['diagnostics'][number]