diff --git a/ts/packages/agents/github-cli/README.md b/ts/packages/agents/github-cli/README.md index b147ecbfff..cc122c0256 100644 --- a/ts/packages/agents/github-cli/README.md +++ b/ts/packages/agents/github-cli/README.md @@ -11,18 +11,19 @@ The agent runs a `gh auth status` readiness probe at startup and pre-flights it ## Supported Actions -| Category | Actions | -| ----------------- | ----------------------------------------------------------------------------------------------------- | -| **Auth** | Login, logout, check status | -| **Issues** | Create, close, reopen, list, view, browse | -| **Pull Requests** | Create (including draft), close, merge, list, view, checkout, browse | -| **Repos** | Create, clone, delete, view (with field-specific queries like stars/forks), fork, star/unstar, browse | -| **Search** | Search repositories by keyword | -| **Status** | Dashboard summary of notifications, PRs, and issues | -| **Contributors** | Top N contributors for a repo | -| **Dependabot** | List alerts with severity/state filters | -| **Workflows** | View workflow runs and workflow details | -| **Other** | Codespaces, gists, releases, projects, labels, secrets, SSH keys, config, orgs | +| Category | Actions | +| ------------------ | ----------------------------------------------------------------------------------------------------------------- | +| **Auth** | Login, logout, check status | +| **Issues** | Create, close, reopen, list, view, browse | +| **Pull Requests** | Create (including draft), close, merge, list, view, checkout, browse | +| **PR diagnostics** | List a PR's changed files (optionally with diff excerpts); explain failing checks with GitHub's error annotations | +| **Repos** | Create, clone, delete, view (with field-specific queries like stars/forks), fork, star/unstar, browse | +| **Search** | Search repositories by keyword | +| **Status** | Dashboard summary of notifications, PRs, and issues | +| **Contributors** | Top N contributors for a repo | +| **Dependabot** | List alerts with severity/state filters | +| **Workflows** | View workflow runs and workflow details | +| **Other** | Codespaces, gists, releases, projects, labels, secrets, SSH keys, config, orgs | ## Example Phrases @@ -33,6 +34,10 @@ how many stars does microsoft/TypeAgent have show top 10 contributors for microsoft/TypeAgent create issue "Fix login bug" in microsoft/TypeAgent close issue 42 in microsoft/TypeAgent +show files changed in PR 2196 +show the diff for PR 2196 +why is CI failing on PR 2196 +why is https://github.com/cli/cli/pull/9000 failing open a draft PR for my-feature branch show newest 5 dependabot alerts in microsoft/TypeAgent fork microsoft/TypeAgent @@ -46,6 +51,8 @@ star microsoft/TypeAgent - Status output uses **bold section headers** for readability - Dependabot alerts are color-coded by severity (🔴 critical, 🟠 high, 🟡 medium, 🟢 low) - Mutation actions (create, close, star, fork) return friendly emoji confirmation messages +- `prFiles` and `prFailedChecks` return typed structured data (`rawData`) alongside their display, and state explicitly when output was truncated, so an external MCP client can act on the result directly +- `prFiles` and `prFailedChecks` accept a pull request's web link in place of an `OWNER/REPO` slug, so a PR in another repository (or on a GitHub Enterprise host) can be diagnosed without leaving the current checkout ## Demo diff --git a/ts/packages/agents/github-cli/package.json b/ts/packages/agents/github-cli/package.json index 9e69117b10..7b0ed2c84e 100644 --- a/ts/packages/agents/github-cli/package.json +++ b/ts/packages/agents/github-cli/package.json @@ -37,6 +37,7 @@ "@typeagent/agent-sdk": "workspace:*" }, "devDependencies": { + "@typeagent/action-grammar": "workspace:*", "@typeagent/action-grammar-compiler": "workspace:*", "@typeagent/action-schema-compiler": "workspace:*", "@types/jest": "^29.5.7", @@ -73,6 +74,14 @@ "dist/github-cliSchema.pas.json" ] } + }, + "tsc": { + "dependsOn": [ + "@typeagent/action-grammar#tsc" + ], + "after": [ + "^*" + ] } } }, diff --git a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts index d8efe97609..6d3fed2141 100644 --- a/ts/packages/agents/github-cli/src/github-cliActionHandler.ts +++ b/ts/packages/agents/github-cli/src/github-cliActionHandler.ts @@ -37,6 +37,8 @@ import { runSetupCommand, whichExists, } from "./setup.js"; +import { buildTableBlock } from "./structuredResults.js"; +import { GhResult, runPrFailedChecks, runPrFiles } from "./prDiagnostics.js"; const execFileAsync = promisify(execFile); @@ -362,6 +364,42 @@ async function runGh(args: string[], timeoutMs = 30_000): Promise { return stdout.trim(); } +// Run a gh CLI command and return its full result, including a non-zero exit +// code, instead of throwing. Some gh commands report status through the exit +// code while still writing the JSON we asked for (`gh pr checks` exits +// non-zero when checks fail), and some failures should degrade a section of a +// result rather than fail the whole action. `exitCode` is -1 when gh never ran +// at all — a missing binary or a timeout. +// +// The buffer is larger than `runGh`'s because GitHub returns patches before we +// can truncate them. Eight MiB accommodates large PR responses while still +// bounding memory if a generated file contains an unusually large patch. +async function runGhCapture( + args: string[], + timeoutMs = 60_000, +): Promise { + try { + const { stdout, stderr } = await execFileAsync("gh", args, { + timeout: timeoutMs, + maxBuffer: 8 * 1024 * 1024, + windowsHide: true, + }); + return { stdout, stderr, exitCode: 0 }; + } catch (e) { + const err = e as { + stdout?: string; + stderr?: string; + code?: unknown; + message?: string; + }; + return { + stdout: err.stdout ?? "", + stderr: err.stderr || err.message || "gh failed to run", + exitCode: typeof err.code === "number" ? err.code : -1, + }; + } +} + // Sentinel values that mean "no assignee". `gh issue list --assignee ` // treats as a literal GitHub login, so "--assignee none" fails with // "Could not find an assignee with the login 'none'". The supported way to @@ -1258,13 +1296,9 @@ function makeStructuredTable( pageSize?: number; }, ): ActionResultSuccess { - const columns = colSpecs.map(({ value: _v, ...col }) => col); - const rows: TableCell[][] = objects.map((obj) => - colSpecs.map((col) => col.value(obj)), - ); // Cap long lists to a first page (client reveals the rest via "Show // more") unless the caller overrode it. All rows still ship. - const table: TableBlock = createTable(columns, rows, { + const table: TableBlock = buildTableBlock(colSpecs, objects, { pageSize: 15, ...tableOptions, }); @@ -1923,6 +1957,16 @@ async function executeAction( } action = validated.action; + // Multi-call read-only diagnostics. These compose several gh invocations + // into one structured result, so they run ahead of the single-command + // buildArgs/runGh path below. + if (action.actionName === "prFiles") { + return runPrFiles(action.parameters, runGhCapture); + } + if (action.actionName === "prFailedChecks") { + return runPrFailedChecks(action.parameters, runGhCapture); + } + const args = buildArgs(action); if (!args) { return createActionResultFromTextDisplay( diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.agr b/ts/packages/agents/github-cli/src/github-cliSchema.agr index c3fbd72312..04865236f8 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.agr +++ b/ts/packages/agents/github-cli/src/github-cliSchema.agr @@ -270,6 +270,65 @@ } }; + = show files changed in PR $(number:number) -> { + actionName: "prFiles", + parameters: { + number + } +} + | what files does PR $(number:number) (change | touch | edit | modify) -> { + actionName: "prFiles", + parameters: { + number + } +} + | show what's changed in PR $(number:number) -> { + actionName: "prFiles", + parameters: { + number + } +} + | show files changed in PR $(number:number) in $(repo:wildcard) -> { + actionName: "prFiles", + parameters: { + number, + repo + } +} + | show the diff for PR $(number:number) -> { + actionName: "prFiles", + parameters: { + number, + includePatch: true + } +}; + + = show failing checks for PR $(number:number) -> { + actionName: "prFailedChecks", + parameters: { + number + } +} + | why is (the)? CI failing on PR $(number:number) -> { + actionName: "prFailedChecks", + parameters: { + number + } +} + | why is (the)? pipeline failing on PR $(number:number) -> { + actionName: "prFailedChecks", + parameters: { + number + } +} + | show failing checks for PR $(number:number) in $(repo:wildcard) -> { + actionName: "prFailedChecks", + parameters: { + number, + repo + } +}; + = create a new repository named $(name:wildcard) -> { actionName: "repoCreate", parameters: { @@ -498,6 +557,8 @@ import { GithubCliActions } from "./github-cliSchema.ts"; | | | + | + | | | | diff --git a/ts/packages/agents/github-cli/src/github-cliSchema.ts b/ts/packages/agents/github-cli/src/github-cliSchema.ts index 080dc3a585..e34e59c097 100644 --- a/ts/packages/agents/github-cli/src/github-cliSchema.ts +++ b/ts/packages/agents/github-cli/src/github-cliSchema.ts @@ -30,6 +30,8 @@ export type GithubCliActions = | PrViewAction | PrCheckoutAction | PrChecksAction + | PrFilesAction + | PrFailedChecksAction | ProjectCreateAction | ProjectDeleteAction | ProjectListAction @@ -391,6 +393,78 @@ export type PrChecksAction = { }; }; +// List the files a pull request changes, with per-file status and line counts, +// and optionally an excerpt of each file's diff. Use this to find out what a +// pull request actually touches. Read-only. +// +// Example: +// User: what files does PR 2196 change? +// Agent: { actionName: "prFiles", parameters: { number: 2196 } } +// +// Example: +// User: show me the diff for pull request 42 in microsoft/TypeAgent +// Agent: { actionName: "prFiles", parameters: { number: 42, repo: "microsoft/TypeAgent", includePatch: true } } +// +// Example: +// User: what does https://github.com/microsoft/TypeAgent/pull/42 change? +// Agent: { actionName: "prFiles", parameters: { number: 42, repo: "microsoft/TypeAgent" } } +export type PrFilesAction = { + actionName: "prFiles"; + parameters: { + // The pull request number. + number: number; + + // OWNER/REPO slug (e.g. "microsoft/TypeAgent"), or the pull request's + // web link. Omit to use the repository in the current directory. + repo?: string; + + // Include an excerpt of each file's diff. Off by default because + // patches are large; turn it on to see the actual code changes. + includePatch?: boolean; + + // How many files to return, newest API order. Defaults to 50 to keep + // the structured display manageable; callers can request up to 300. + maxFiles?: number; + + // How many lines of each file's patch to keep. 1-200, default 40. + // Only meaningful with includePatch. + maxPatchLines?: number; + }; +}; + +// Explain why a pull request's checks are red: which checks failed, when, and +// the specific error annotations GitHub recorded for each one. Use this to +// diagnose CI failures. Read-only. +// +// Example: +// User: why is CI failing on PR 2196? +// Agent: { actionName: "prFailedChecks", parameters: { number: 2196 } } +// +// Example: +// User: show the failing checks for pull request 42 in microsoft/TypeAgent +// Agent: { actionName: "prFailedChecks", parameters: { number: 42, repo: "microsoft/TypeAgent" } } +// +// Example: +// User: why is https://github.com/microsoft/TypeAgent/pull/42 red? +// Agent: { actionName: "prFailedChecks", parameters: { number: 42, repo: "microsoft/TypeAgent" } } +export type PrFailedChecksAction = { + actionName: "prFailedChecks"; + parameters: { + // The pull request number. + number: number; + + // OWNER/REPO slug (e.g. "microsoft/TypeAgent"), or the pull request's + // web link. Omit to use the repository in the current directory. + repo?: string; + + // How many failing checks to describe in detail. 1-20, default 5. + maxChecks?: number; + + // How many annotations to return per failing check. 1-50, default 10. + maxAnnotations?: number; + }; +}; + export type ProjectCreateAction = { actionName: "projectCreate"; parameters: { diff --git a/ts/packages/agents/github-cli/src/prDiagnostics.ts b/ts/packages/agents/github-cli/src/prDiagnostics.ts new file mode 100644 index 0000000000..8647c2160d --- /dev/null +++ b/ts/packages/agents/github-cli/src/prDiagnostics.ts @@ -0,0 +1,1458 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Read-only pull request diagnostics: what a PR changed, and why its checks + * are red. Both actions are deterministic - they shell out to `gh`, parse the + * JSON it returns, and build a typed result. No LLM is involved, so they are + * safe to call directly from an external client via `@action github-cli ...`. + * + * Everything here is bounded on purpose. A pull request can touch thousands of + * files and a check can carry hundreds of annotations, so each result carries + * explicit truncation flags rather than silently dropping data. + */ + +import { + ActionResult, + ActionResultSuccess, + BadgeTone, + KeyValuePair, + StructuredBlock, + TableCell, +} from "@typeagent/agent-sdk"; +import { createActionResultFromError } from "@typeagent/agent-sdk/helpers/action"; +import { + ColumnSpec, + createStructuredContent, +} from "@typeagent/agent-sdk/helpers/display"; +import { PrFailedChecksAction, PrFilesAction } from "./github-cliSchema.js"; +import { buildTableBlock } from "./structuredResults.js"; + +// ============================================================================ +// gh invocation +// ============================================================================ + +// The result of one `gh` invocation. A non-zero exit is data here, not an +// exception: `gh pr checks` exits non-zero when checks fail or are still +// pending yet still writes the JSON we asked for, and a failed annotations +// fetch must degrade to "annotations unavailable" instead of failing the whole +// action. `exitCode` is -1 when gh never ran (missing binary, timeout). +export type GhResult = { + stdout: string; + stderr: string; + exitCode: number; +}; + +export type GhRunner = (args: string[]) => Promise; + +// ============================================================================ +// Output bounds +// ============================================================================ + +const FILES_DEFAULT = 50; +const FILES_MAX = 300; +const PATCH_LINES_DEFAULT = 40; +const PATCH_LINES_MAX = 200; +// Ceiling on the combined patch excerpt across all files, so a high file cap +// combined with includePatch can't produce an unbounded payload. +const PATCH_TOTAL_LINES_MAX = 600; +// Line counts alone do not bound a patch: a generated or minified file can put +// megabytes on a single line. Every patch excerpt is therefore capped by +// characters as well, per file and in aggregate. +const PATCH_CHARS_MAX = 8_000; +const PATCH_TOTAL_CHARS_MAX = 40_000; +const CHECKS_DEFAULT = 5; +const CHECKS_MAX = 20; +const ANNOTATIONS_DEFAULT = 10; +const ANNOTATIONS_MAX = 50; +// GitHub's per-page ceiling for the pull request files endpoint. +const FILES_PAGE_SIZE = 100; +// 100 is the per-page ceiling for the check run annotations endpoint. The +// endpoint returns annotations in the order the check reported them, so a +// noisy check can emit a page of warnings before the failure that actually +// explains the red state - read a few pages before giving up on finding one. +const ANNOTATIONS_FETCH_PAGE_SIZE = 100; +const ANNOTATIONS_MAX_PAGES = 3; +// How many annotation fetches to have in flight at once. Failing checks are +// independent, but firing all of them at once would spike the API rate limit +// for no useful gain. +const ANNOTATIONS_FETCH_CONCURRENCY = 4; +// Annotation messages are free-form and occasionally carry a whole stack +// trace, so cap each one. +const ANNOTATION_MESSAGE_MAX_CHARS = 400; + +// Clamp a caller-supplied count into [1, max], falling back when it is absent +// or not a number. Callers come from a validated action schema, but the schema +// can't express a range. +export function clampCount( + value: number | undefined, + fallback: number, + max: number, +): number { + if (typeof value !== "number" || !Number.isFinite(value)) { + return fallback; + } + return Math.min(Math.max(Math.floor(value), 1), max); +} + +// ============================================================================ +// URL parsing +// ============================================================================ + +// A repository on a specific GitHub host. `gh api` talks to the host you tell +// it to, so carrying the host through keeps GitHub Enterprise callers working. +export type RepoRef = { + host: string; + owner: string; + repo: string; +}; + +export function repoSlug(ref: RepoRef): string { + return `${ref.owner}/${ref.repo}`; +} + +// A PR's web URL is the one place `gh pr view` reports the *base* repository, +// which is what the files and check-runs REST endpoints key on - a PR opened +// from a fork still lives in the base repo. +export function parsePrUrl(url: string): RepoRef | undefined { + const m = /^https?:\/\/([^/]+)\/([^/]+)\/([^/]+)\/pull\/\d+/.exec(url); + return m ? { host: m[1], owner: m[2], repo: m[3] } : undefined; +} + +// Reduce whatever the caller used to name a repository down to something +// `gh --repo` accepts. +// +// gh takes `OWNER/REPO`, `HOST/OWNER/REPO`, and a plain repository URL, but it +// rejects a URL that points at anything *inside* the repository - a pull +// request link fails with "invalid path: /owner/repo/pull/123", which tells the +// user nothing useful. Yet a PR link is the most natural way to refer to a pull +// request in some other repository, and it already carries the host, so trim it +// back to `HOST/OWNER/REPO` instead of letting gh reject it. +// +// The PR number is NOT taken from the link: `number` is a required parameter +// and stays authoritative, so there is only ever one place the number comes +// from. Anything unrecognized is passed through untouched for gh (or the shared +// repo validation) to rule on. +export function normalizeRepoParam( + repo: string | undefined, +): string | undefined { + if (repo === undefined || repo.length === 0) { + return repo; + } + const url = /^https?:\/\/([^/]+)\/([^/]+)\/([^/]+)(?:\/.*)?$/.exec(repo); + if (!url) { + return repo; + } + // Strip a trailing ".git" so a clone URL names the same repo as its web URL. + const name = url[3].replace(/\.git$/, ""); + return `${url[1]}/${url[2]}/${name}`; +} + +// `gh pr checks` reports a `link` per check. Two GitHub URL shapes carry a +// check run id, which is the key the annotations endpoint takes: +// +// https://HOST/OWNER/REPO/runs/ +// https://HOST/OWNER/REPO/actions/runs//job/ +// +// For GitHub Actions the job id *is* the check run id (a job's `check_run_url` +// ends in the same number), so both shapes resolve the same way. Links to +// third-party CI (Azure Pipelines and friends) and to GitHub Apps carry no +// check run id and return undefined - those checks have no annotations to +// fetch. +export function parseCheckRunRef( + link: string | undefined, +): (RepoRef & { checkRunId: string }) | undefined { + if (!link) { + return undefined; + } + const job = + /^https?:\/\/([^/]+)\/([^/]+)\/([^/]+)\/actions\/runs\/\d+\/job\/(\d+)/.exec( + link, + ); + if (job) { + return { + host: job[1], + owner: job[2], + repo: job[3], + checkRunId: job[4], + }; + } + const run = /^https?:\/\/([^/]+)\/([^/]+)\/([^/]+)\/runs\/(\d+)/.exec(link); + if (run) { + return { + host: run[1], + owner: run[2], + repo: run[3], + checkRunId: run[4], + }; + } + return undefined; +} + +// ============================================================================ +// gh argument construction +// ============================================================================ + +const PR_VIEW_FIELDS = + "number,title,url,state,isDraft,additions,deletions,changedFiles,headRefName,baseRefName,headRepository,headRepositoryOwner"; + +const PR_CHECKS_FIELDS = + "bucket,name,state,link,workflow,event,startedAt,completedAt,description"; + +export function buildPrViewArgs(prNumber: number, repo?: string): string[] { + const args = ["pr", "view", String(prNumber)]; + if (repo) { + args.push("--repo", repo); + } + args.push("--json", PR_VIEW_FIELDS); + return args; +} + +export function buildPrChecksArgs(prNumber: number, repo?: string): string[] { + const args = ["pr", "checks", String(prNumber)]; + if (repo) { + args.push("--repo", repo); + } + args.push("--json", PR_CHECKS_FIELDS); + return args; +} + +// `gh api` defaults to the account's default host, which is not necessarily +// the host the PR lives on, so the hostname is always explicit. +function buildApiArgs(host: string, path: string): string[] { + return ["api", "--hostname", host, path]; +} + +// The pull request files endpoint always includes each file's full patch, and +// there is no query parameter to suppress it. Without a projection a +// metadata-only request still transfers - and buffers - every patch in the PR. +// `gh --jq` applies the filter inside gh, so the patches never reach us. +const FILES_METADATA_JQ = + "[.[] | {filename, status, additions, deletions, changes, previous_filename}]"; + +export function buildPrFilesArgs( + ref: RepoRef, + prNumber: number, + page: number, + perPage: number, + includePatch: boolean, +): string[] { + const args = buildApiArgs( + ref.host, + `repos/${ref.owner}/${ref.repo}/pulls/${prNumber}/files?per_page=${perPage}&page=${page}`, + ); + if (!includePatch) { + args.push("--jq", FILES_METADATA_JQ); + } + return args; +} + +export function buildAnnotationsArgs( + ref: RepoRef & { checkRunId: string }, + page: number, +): string[] { + return buildApiArgs( + ref.host, + `repos/${ref.owner}/${ref.repo}/check-runs/${ref.checkRunId}/annotations?per_page=${ANNOTATIONS_FETCH_PAGE_SIZE}&page=${page}`, + ); +} + +// ============================================================================ +// Error reporting +// ============================================================================ + +// Turn a gh failure into one line the caller can act on. gh puts the useful +// text on stderr, but writes nothing there when it fails to start at all. +export function describeGhFailure(args: string[], res: GhResult): string { + const detail = (res.stderr || res.stdout).trim(); + const firstLine = + detail + .split("\n") + .find((line) => line.trim().length > 0) + ?.trim() ?? `gh exited with code ${res.exitCode}`; + const hint = ghFailureHint(detail); + return `\`gh ${args.join(" ")}\` failed: ${firstLine}${hint ? ` ${hint}` : ""}`; +} + +// gh surfaces auth, permission, and not-found problems as bare HTTP status +// lines. Say what to do about them instead of passing the status through. +export function ghFailureHint(detail: string): string | undefined { + if (/HTTP 401|not logged in|gh auth login/i.test(detail)) { + return "Run `gh auth login` and retry."; + } + if (/HTTP 403/i.test(detail)) { + return "The authenticated account lacks permission for this repository, or the API rate limit is exhausted."; + } + if (/HTTP 404/i.test(detail)) { + return "Check the OWNER/REPO slug and pull request number, and that the account can see this repository."; + } + return undefined; +} + +function parseJsonArray(stdout: string): Record[] | undefined { + try { + const parsed: unknown = JSON.parse(stdout); + return Array.isArray(parsed) + ? (parsed as Record[]) + : undefined; + } catch { + return undefined; + } +} + +function parseJsonObject(stdout: string): Record | undefined { + try { + const parsed: unknown = JSON.parse(stdout); + return parsed !== null && + typeof parsed === "object" && + !Array.isArray(parsed) + ? (parsed as Record) + : undefined; + } catch { + return undefined; + } +} + +// ============================================================================ +// Pull request metadata (shared by both actions) +// ============================================================================ + +export type PrMeta = { + ref: RepoRef; + number: number; + title: string; + url: string; + state: string; + isDraft: boolean; + headRefName: string; + baseRefName: string; + additions: number; + deletions: number; + changedFiles: number; + // Set only for a PR opened from another repository. CI for a fork PR runs + // with a read-only token and no secrets, which is a common root cause of a + // check failing there but not on a branch PR. + headRepo?: string; + fromFork: boolean; +}; + +type Fetched = { ok: true; value: T } | { ok: false; error: string }; + +function nestedString( + data: Record, + key: string, + field: string, +): string | undefined { + const nested = data[key]; + if (nested === null || typeof nested !== "object") { + return undefined; + } + const value = (nested as Record)[field]; + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +export function toPrMeta( + data: Record, +): PrMeta | { error: string } { + const url = typeof data.url === "string" ? data.url : ""; + const ref = parsePrUrl(url); + if (ref === undefined) { + return { + error: `Could not determine the repository from the pull request URL (${url || "missing"}).`, + }; + } + const headRepo = nestedString(data, "headRepository", "nameWithOwner"); + const headOwner = nestedString(data, "headRepositoryOwner", "login"); + // `headRepository.nameWithOwner` is the reliable comparison; fall back to + // the owner login when an older gh omits it. + const fromFork = + headRepo !== undefined + ? headRepo.toLowerCase() !== repoSlug(ref).toLowerCase() + : headOwner !== undefined + ? headOwner.toLowerCase() !== ref.owner.toLowerCase() + : false; + return { + ref, + number: Number(data.number ?? 0), + title: String(data.title ?? ""), + url, + state: String(data.state ?? ""), + isDraft: Boolean(data.isDraft), + headRefName: String(data.headRefName ?? ""), + baseRefName: String(data.baseRefName ?? ""), + additions: Number(data.additions ?? 0), + deletions: Number(data.deletions ?? 0), + changedFiles: Number(data.changedFiles ?? 0), + ...(fromFork && headRepo !== undefined ? { headRepo } : {}), + fromFork, + }; +} + +async function fetchPrMeta( + gh: GhRunner, + prNumber: number, + repo: string | undefined, +): Promise> { + const args = buildPrViewArgs(prNumber, repo); + const res = await gh(args); + const data = parseJsonObject(res.stdout); + if (res.exitCode !== 0 || data === undefined) { + return { ok: false, error: describeGhFailure(args, res) }; + } + const meta = toPrMeta(data); + if ("error" in meta) { + return { ok: false, error: meta.error }; + } + return { ok: true, value: meta }; +} + +// The identity fields both result payloads carry, so one helper can render +// the shared metadata block for either of them. +type PrIdentity = { + state: string; + isDraft: boolean; + headRefName: string; + baseRefName: string; + headRepo?: string; + fromFork: boolean; + url: string; + retrievedAt: string; +}; + +// Metadata both actions show, so a caller always knows which pull request and +// which head/base pair the result describes, and how fresh it is. +function prMetaPairs(pr: PrIdentity): KeyValuePair[] { + const pairs: KeyValuePair[] = [ + { + label: "State", + value: { + text: pr.isDraft ? "Draft" : pr.state, + badge: pr.isDraft + ? "warning" + : pr.state.toUpperCase() === "OPEN" + ? "info" + : "neutral", + }, + }, + { + label: "Branch", + value: `${pr.headRefName} → ${pr.baseRefName}`, + }, + ]; + if (pr.fromFork) { + pairs.push({ + label: "Fork", + value: { + text: pr.headRepo ?? "head branch is in another repository", + badge: "warning", + }, + }); + } + pairs.push({ + label: "Link", + value: { text: pr.url, href: pr.url }, + }); + pairs.push({ label: "Retrieved", value: pr.retrievedAt }); + return pairs; +} + +// ============================================================================ +// prFiles +// ============================================================================ + +export type PrFileEntry = { + path: string; + // Where a renamed or copied file came from. Without it a rename reads as + // an unrelated delete plus add. + previousPath?: string; + // added | removed | modified | renamed | copied | changed | unchanged + status: string; + additions: number; + deletions: number; + changes: number; + // Present only when patches were requested and GitHub returned one. + patch?: string; + // Why the patch is missing, when one was requested. "unavailable" means + // GitHub itself omitted it (binary blob, or a diff it considers too large + // to inline); "budget" means the combined patch cap was already spent. + patchOmitted?: "unavailable" | "budget"; + // Lines dropped from the end of this file's patch. + patchTruncatedLines?: number; + // Characters dropped by the per-patch character budget. Set only when a + // single line was longer than the budget allows, which the line count + // alone cannot express. + patchTruncatedChars?: number; +}; + +export type PrFilesData = { + kind: "prFiles"; + repo: string; + number: number; + title: string; + url: string; + state: string; + isDraft: boolean; + headRefName: string; + baseRefName: string; + headRepo?: string; + fromFork: boolean; + totals: { + additions: number; + deletions: number; + changedFiles: number; + }; + files: PrFileEntry[]; + truncated: { + // More files exist than were returned. + files: boolean; + // At least one patch was shortened or dropped. + patches: boolean; + }; + retrievedAt: string; +}; + +// Keep the head of a patch rather than the tail: the hunk header and the first +// changed lines say what the edit is, while the tail is usually trailing +// context. +// +// Both a line cap and a character cap apply. The line cap is what a reader +// thinks in, but it bounds nothing on its own - a generated or minified file +// can hold megabytes on one line - so the character cap is the real bound. +export function truncatePatch( + patch: string, + maxLines: number, + maxChars: number, +): { + text: string; + keptLines: number; + omittedLines: number; + omittedChars: number; +} { + const lines = patch.split("\n"); + const kept = lines.length > maxLines ? lines.slice(0, maxLines) : lines; + const head = kept.join("\n"); + if (head.length <= maxChars) { + return { + text: head, + keptLines: kept.length, + omittedLines: lines.length - kept.length, + omittedChars: 0, + }; + } + const cut = head.slice(0, maxChars); + const lastNewline = cut.lastIndexOf("\n"); + // Prefer to end on a line boundary, but not when that would throw away + // most of the budget: a single line longer than the budget is cut mid-line + // so its start - where the diff marker and the useful content are - still + // shows. A partial trailing line does not count as kept. + const onBoundary = lastNewline >= Math.floor(maxChars / 2); + const text = onBoundary ? cut.slice(0, lastNewline) : cut; + const keptLines = onBoundary + ? text.split("\n").length + : text.split("\n").length - 1; + return { + text, + keptLines, + omittedLines: lines.length - keptLines, + omittedChars: head.length - text.length, + }; +} + +export function buildFileEntries( + rawFiles: Record[], + includePatch: boolean, + maxPatchLines: number, +): PrFileEntry[] { + let remainingPatchLines = PATCH_TOTAL_LINES_MAX; + let remainingPatchChars = PATCH_TOTAL_CHARS_MAX; + return rawFiles.map((raw) => { + const entry: PrFileEntry = { + path: String(raw.filename ?? ""), + status: String(raw.status ?? "unknown"), + additions: Number(raw.additions ?? 0), + deletions: Number(raw.deletions ?? 0), + changes: Number(raw.changes ?? 0), + }; + if (typeof raw.previous_filename === "string") { + entry.previousPath = raw.previous_filename; + } + if (!includePatch) { + return entry; + } + if (typeof raw.patch !== "string") { + entry.patchOmitted = "unavailable"; + return entry; + } + if (remainingPatchLines <= 0 || remainingPatchChars <= 0) { + entry.patchOmitted = "budget"; + return entry; + } + const { text, keptLines, omittedLines, omittedChars } = truncatePatch( + raw.patch, + Math.min(maxPatchLines, remainingPatchLines), + Math.min(PATCH_CHARS_MAX, remainingPatchChars), + ); + remainingPatchLines -= keptLines; + remainingPatchChars -= text.length; + entry.patch = text; + if (omittedLines > 0) { + entry.patchTruncatedLines = omittedLines; + } + if (omittedChars > 0) { + entry.patchTruncatedChars = omittedChars; + } + return entry; + }); +} + +// Page through the pull request files endpoint until we have as many files as +// the caller asked for, or GitHub runs out. +// +// One file more than the cap is asked for, so that "are there more?" is +// answered by the API rather than inferred. A full page proves nothing on its +// own - a PR of exactly `maxFiles` files fills the page too - and the PR's own +// `changedFiles` total is read in a separate, earlier call, so a push landing +// between the two can leave it stale. +async function fetchPrFiles( + gh: GhRunner, + ref: RepoRef, + prNumber: number, + maxFiles: number, + includePatch: boolean, +): Promise[]; hasMore: boolean }>> { + const files: Record[] = []; + // `page` is an offset in units of `per_page`, so the page size has to stay + // fixed for the whole walk. Below GitHub's ceiling the extra file rides + // along in the first request; at or above it, the probe below is needed. + const perPage = Math.min(FILES_PAGE_SIZE, maxFiles + 1); + let lastPageFull = false; + for (let page = 1; files.length < maxFiles; page++) { + const args = buildPrFilesArgs( + ref, + prNumber, + page, + perPage, + includePatch, + ); + const res = await gh(args); + const batch = + res.exitCode === 0 ? parseJsonArray(res.stdout) : undefined; + if (batch === undefined) { + return { ok: false, error: describeGhFailure(args, res) }; + } + files.push(...batch); + lastPageFull = batch.length >= perPage; + if (!lastPageFull) { + break; + } + } + + if (files.length > maxFiles) { + return { + ok: true, + value: { files: files.slice(0, maxFiles), hasMore: true }, + }; + } + if (!lastPageFull) { + return { ok: true, value: { files, hasMore: false } }; + } + + // Exactly `maxFiles` files arrived on whole pages, so whether a further one + // exists is still open. Ask for that single file rather than another full + // page, and never with patches - it is evidence, not output. + const probeArgs = buildPrFilesArgs(ref, prNumber, maxFiles + 1, 1, false); + const probeRes = await gh(probeArgs); + const probe = + probeRes.exitCode === 0 ? parseJsonArray(probeRes.stdout) : undefined; + if (probe === undefined) { + return { ok: false, error: describeGhFailure(probeArgs, probeRes) }; + } + return { ok: true, value: { files, hasMore: probe.length > 0 } }; +} + +function fileStatusBadge(status: string): BadgeTone { + switch (status) { + case "added": + return "success"; + case "removed": + return "error"; + case "renamed": + case "copied": + return "info"; + default: + return "neutral"; + } +} + +// The patch excerpts, one section per file that has one, plus the notes +// explaining anything that was cut or never available. +function patchBlocks(files: PrFileEntry[]): StructuredBlock[] { + const blocks: StructuredBlock[] = []; + for (const file of files) { + if (file.patch === undefined) { + continue; + } + blocks.push({ kind: "divider" }); + blocks.push({ kind: "heading", level: 3, text: file.path }); + blocks.push({ kind: "code", code: file.patch, language: "diff" }); + const cuts: string[] = []; + if (file.patchTruncatedLines !== undefined) { + cuts.push( + `${file.patchTruncatedLines} more patch line${file.patchTruncatedLines === 1 ? "" : "s"}`, + ); + } + if (file.patchTruncatedChars !== undefined) { + cuts.push( + `${file.patchTruncatedChars} more character${file.patchTruncatedChars === 1 ? "" : "s"} on a line too long to show`, + ); + } + if (cuts.length > 0) { + blocks.push({ + kind: "text", + text: `*…${cuts.join(" and ")} omitted.*`, + format: "markdown", + }); + } + } + + const countOmitted = (reason: PrFileEntry["patchOmitted"]) => + files.filter((f) => f.patchOmitted === reason).length; + const unavailable = countOmitted("unavailable"); + const overBudget = countOmitted("budget"); + const notes: string[] = []; + if (unavailable > 0) { + notes.push( + `${unavailable} file${unavailable === 1 ? "" : "s"} had no patch (binary, or a diff GitHub considers too large)`, + ); + } + if (overBudget > 0) { + notes.push( + `${overBudget} file${overBudget === 1 ? "" : "s"} exceeded the combined patch limit of ${PATCH_TOTAL_LINES_MAX} lines / ${PATCH_TOTAL_CHARS_MAX} characters`, + ); + } + if (notes.length > 0) { + blocks.push({ + kind: "text", + text: `*${notes.join("; ")}.*`, + format: "markdown", + }); + } + return blocks; +} + +export function buildStructuredPrFiles(data: PrFilesData): ActionResultSuccess { + const shown = data.files.length; + const headingText = `#${data.number} ${data.title} — ${shown} of ${data.totals.changedFiles} file${data.totals.changedFiles === 1 ? "" : "s"}`; + + const pairs: KeyValuePair[] = [ + { + label: "Changes", + value: `+${data.totals.additions} −${data.totals.deletions} across ${data.totals.changedFiles} file${data.totals.changedFiles === 1 ? "" : "s"}`, + }, + ]; + + const blocks: StructuredBlock[] = [ + { kind: "heading", level: 3, text: headingText }, + { kind: "keyValue", pairs }, + { kind: "keyValue", pairs: prMetaPairs(data) }, + ]; + + if (shown === 0) { + blocks.push({ + kind: "text", + text: "This pull request changes no files.", + }); + } else { + const cols: ColumnSpec[] = [ + { + id: "path", + header: "File", + type: "code", + value: (f) => + f.previousPath ? `${f.previousPath} → ${f.path}` : f.path, + }, + { + id: "status", + header: "Status", + type: "badge", + value: (f): TableCell => ({ + text: f.status, + badge: fileStatusBadge(f.status), + }), + }, + { + id: "additions", + header: "+", + type: "number", + align: "right", + value: (f) => f.additions, + }, + { + id: "deletions", + header: "−", + type: "number", + align: "right", + value: (f) => f.deletions, + }, + ]; + blocks.push( + buildTableBlock(cols, data.files, { + sortable: true, + pageSize: 15, + }), + ); + } + + // `changedFiles` is read in an earlier call than the file list, so a push + // landing in between can leave it equal to what is shown even though more + // files exist. Only claim a total when it is actually larger. + if (data.truncated.files) { + blocks.push({ + kind: "text", + text: + data.totals.changedFiles > shown + ? `*Showing ${shown} of ${data.totals.changedFiles} changed files. Raise \`maxFiles\` to see more.*` + : `*Showing the first ${shown} files; the pull request has more. Raise \`maxFiles\` to see them.*`, + format: "markdown", + }); + } + + blocks.push(...patchBlocks(data.files)); + + return { + historyText: headingText, + entities: [], + displayContent: createStructuredContent(blocks, { rawData: data }), + }; +} + +export async function runPrFiles( + params: PrFilesAction["parameters"], + gh: GhRunner, +): Promise { + const maxFiles = clampCount(params.maxFiles, FILES_DEFAULT, FILES_MAX); + const maxPatchLines = clampCount( + params.maxPatchLines, + PATCH_LINES_DEFAULT, + PATCH_LINES_MAX, + ); + const includePatch = params.includePatch === true; + const repo = normalizeRepoParam(params.repo); + + const meta = await fetchPrMeta(gh, params.number, repo); + if (!meta.ok) { + return createActionResultFromError(meta.error); + } + const pr = meta.value; + + const raw = await fetchPrFiles( + gh, + pr.ref, + params.number, + maxFiles, + includePatch, + ); + if (!raw.ok) { + return createActionResultFromError(raw.error); + } + + const files = buildFileEntries( + raw.value.files, + includePatch, + maxPatchLines, + ); + // Two independent signals, either of which means files were left out: the + // extra file GitHub handed back for this request, and the PR's own + // changedFiles total (which also covers GitHub capping the files endpoint + // at 3000 entries). + const changedFiles = Math.max(pr.changedFiles, files.length); + const moreFiles = raw.value.hasMore || files.length < changedFiles; + const data: PrFilesData = { + kind: "prFiles", + repo: repoSlug(pr.ref), + number: pr.number, + title: pr.title, + url: pr.url, + state: pr.state, + isDraft: pr.isDraft, + headRefName: pr.headRefName, + baseRefName: pr.baseRefName, + ...(pr.headRepo !== undefined ? { headRepo: pr.headRepo } : {}), + fromFork: pr.fromFork, + totals: { + additions: pr.additions, + deletions: pr.deletions, + changedFiles, + }, + files, + truncated: { + files: moreFiles, + patches: files.some( + (f) => + f.patchOmitted !== undefined || + f.patchTruncatedLines !== undefined || + f.patchTruncatedChars !== undefined, + ), + }, + retrievedAt: new Date().toISOString(), + }; + + return buildStructuredPrFiles(data); +} + +// ============================================================================ +// prFailedChecks +// ============================================================================ + +export type CheckCounts = { + total: number; + passing: number; + failing: number; + pending: number; + skipping: number; + cancelled: number; +}; + +export type CheckAnnotation = { + // failure | warning | notice + level: string; + message: string; + path?: string; + startLine?: number; + endLine?: number; + title?: string; +}; + +export type FailedCheckDetail = { + name: string; + state: string; + workflow?: string; + event?: string; + description?: string; + link?: string; + startedAt?: string; + completedAt?: string; + annotations: CheckAnnotation[]; + // More annotations exist on this check than were returned. + annotationsTruncated: boolean; + // Why annotations are missing or incomplete, when that is not simply + // "there are none". + annotationsUnavailable?: string; +}; + +export type PrFailedChecksData = { + kind: "prFailedChecks"; + repo: string; + number: number; + title: string; + url: string; + state: string; + isDraft: boolean; + headRefName: string; + baseRefName: string; + headRepo?: string; + fromFork: boolean; + counts: CheckCounts; + // True while any check is still queued or running. The failing set can + // still grow, so "nothing else is broken" is not a safe conclusion yet. + inProgress: boolean; + failedChecks: FailedCheckDetail[]; + truncated: { + // More checks failed than were detailed. + checks: boolean; + }; + retrievedAt: string; +}; + +export function countBuckets(checks: Record[]): CheckCounts { + const counts: CheckCounts = { + total: checks.length, + passing: 0, + failing: 0, + pending: 0, + skipping: 0, + cancelled: 0, + }; + for (const check of checks) { + switch (String(check.bucket ?? "")) { + case "pass": + counts.passing++; + break; + case "fail": + counts.failing++; + break; + case "pending": + counts.pending++; + break; + case "skipping": + counts.skipping++; + break; + case "cancel": + counts.cancelled++; + break; + } + } + return counts; +} + +// Only the "fail" bucket is a failure. A cancelled check is reported in the +// counts but not detailed - it has no failure to explain. +export function selectFailedChecks( + checks: Record[], +): Record[] { + return checks.filter((check) => String(check.bucket ?? "") === "fail"); +} + +function optionalString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function optionalNumber(value: unknown): number | undefined { + return typeof value === "number" && Number.isFinite(value) + ? value + : undefined; +} + +// Failure-level annotations are the ones that explain a red check; warnings +// and notices only get through when there are no failures to show. The level +// is rendered on every row, so a warnings-only result is visibly that. +export function selectAnnotations( + raw: Record[], + maxAnnotations: number, + morePages = false, +): { annotations: CheckAnnotation[]; truncated: boolean } { + const mapped: CheckAnnotation[] = raw.map((a) => { + const message = String(a.message ?? ""); + const annotation: CheckAnnotation = { + level: String(a.annotation_level ?? "unknown"), + message: + message.length > ANNOTATION_MESSAGE_MAX_CHARS + ? `${message.slice(0, ANNOTATION_MESSAGE_MAX_CHARS)}…` + : message, + }; + const path = optionalString(a.path); + if (path !== undefined) { + annotation.path = path; + } + const startLine = optionalNumber(a.start_line); + if (startLine !== undefined) { + annotation.startLine = startLine; + } + const endLine = optionalNumber(a.end_line); + if (endLine !== undefined) { + annotation.endLine = endLine; + } + const title = optionalString(a.title); + if (title !== undefined) { + annotation.title = title; + } + return annotation; + }); + const failures = mapped.filter((a) => a.level === "failure"); + const chosen = failures.length > 0 ? failures : mapped; + return { + annotations: chosen.slice(0, maxAnnotations), + // Annotations left unread on a later page count as truncation just as + // much as ones dropped by the cap - otherwise a check whose failures + // sit past the page budget would claim to be showing all of them. + truncated: chosen.length > maxAnnotations || morePages, + }; +} + +// Read annotations a page at a time, stopping as soon as GitHub runs out or +// the page budget is spent. `morePages` is true only when we stopped with a +// full page in hand, meaning GitHub has more we chose not to read. +// +// A failure part-way through keeps whatever earlier pages returned: a check's +// first-page failures are the useful part, and discarding them because page +// three hit a rate limit would throw away the answer we came for. +async function fetchAnnotationPages( + gh: GhRunner, + ref: RepoRef & { checkRunId: string }, +): Promise<{ + annotations: Record[]; + morePages: boolean; + error?: string; +}> { + const annotations: Record[] = []; + for (let page = 1; page <= ANNOTATIONS_MAX_PAGES; page++) { + const args = buildAnnotationsArgs(ref, page); + const res = await gh(args); + const batch = + res.exitCode === 0 ? parseJsonArray(res.stdout) : undefined; + if (batch === undefined) { + return { + annotations, + morePages: true, + error: describeGhFailure(args, res), + }; + } + annotations.push(...batch); + if (batch.length < ANNOTATIONS_FETCH_PAGE_SIZE) { + return { annotations, morePages: false }; + } + } + return { annotations, morePages: true }; +} + +async function fetchAnnotations( + gh: GhRunner, + link: string | undefined, + maxAnnotations: number, +): Promise< + Pick< + FailedCheckDetail, + "annotations" | "annotationsTruncated" | "annotationsUnavailable" + > +> { + const ref = parseCheckRunRef(link); + if (ref === undefined) { + return { + annotations: [], + annotationsTruncated: false, + annotationsUnavailable: + "This check is not a GitHub check run, so GitHub has no annotations for it. Follow the link for details.", + }; + } + const raw = await fetchAnnotationPages(gh, ref); + const { annotations, truncated } = selectAnnotations( + raw.annotations, + maxAnnotations, + raw.morePages, + ); + return { + annotations, + annotationsTruncated: truncated, + ...(raw.error !== undefined + ? { annotationsUnavailable: raw.error } + : {}), + }; +} + +// Run an async mapping with a fixed number of calls in flight, preserving the +// order of the input. +async function mapWithConcurrency( + items: T[], + limit: number, + fn: (item: T) => Promise, +): Promise { + const results = new Array(items.length); + let next = 0; + const workers = Array.from( + { length: Math.min(limit, items.length) }, + async () => { + while (next < items.length) { + const index = next++; + results[index] = await fn(items[index]); + } + }, + ); + await Promise.all(workers); + return results; +} + +function annotationBadge(level: string): BadgeTone { + switch (level) { + case "failure": + return "error"; + case "warning": + return "warning"; + default: + return "neutral"; + } +} + +function annotationLocation(annotation: CheckAnnotation): string { + if (annotation.path === undefined) { + return ""; + } + if (annotation.startLine === undefined) { + return annotation.path; + } + const end = + annotation.endLine !== undefined && + annotation.endLine !== annotation.startLine + ? `-${annotation.endLine}` + : ""; + return `${annotation.path}:${annotation.startLine}${end}`; +} + +function checkDetailBlocks(check: FailedCheckDetail): StructuredBlock[] { + const pairs: KeyValuePair[] = [ + { label: "Result", value: { text: check.state, badge: "error" } }, + ]; + if (check.workflow !== undefined) { + pairs.push({ label: "Workflow", value: check.workflow }); + } + if (check.event !== undefined) { + pairs.push({ label: "Event", value: check.event }); + } + if (check.description !== undefined) { + pairs.push({ label: "Summary", value: check.description }); + } + if (check.startedAt !== undefined) { + pairs.push({ label: "Started", value: check.startedAt }); + } + if (check.completedAt !== undefined) { + pairs.push({ label: "Completed", value: check.completedAt }); + } + if (check.link !== undefined) { + pairs.push({ + label: "Link", + value: { text: check.link, href: check.link }, + }); + } + + const blocks: StructuredBlock[] = [ + { kind: "divider" }, + { kind: "heading", level: 3, text: check.name }, + { kind: "keyValue", pairs }, + ]; + + if (check.annotations.length > 0) { + const cols: ColumnSpec[] = [ + { + id: "level", + header: "Level", + type: "badge", + value: (a): TableCell => ({ + text: a.level, + badge: annotationBadge(a.level), + }), + }, + { + id: "location", + header: "Location", + type: "code", + value: (a) => annotationLocation(a), + }, + { + id: "message", + header: "Message", + value: (a) => + a.title ? `${a.title}: ${a.message}` : a.message, + }, + ]; + blocks.push( + buildTableBlock(cols, check.annotations, { sortable: false }), + ); + if (check.annotationsTruncated) { + blocks.push({ + kind: "text", + text: "*More annotations exist on this check than are shown. Raise `maxAnnotations`, or follow the link for the full list.*", + format: "markdown", + }); + } + // A partial fetch still shows what it got, but must say it is partial. + if (check.annotationsUnavailable !== undefined) { + blocks.push({ + kind: "text", + text: `*Annotations are incomplete: ${check.annotationsUnavailable}*`, + format: "markdown", + }); + } + } else { + blocks.push({ + kind: "text", + text: + check.annotationsUnavailable ?? + "GitHub recorded no annotations for this check.", + }); + } + return blocks; +} + +export function buildStructuredPrFailedChecks( + data: PrFailedChecksData, +): ActionResultSuccess { + const { failing } = data.counts; + const headingText = `#${data.number} ${data.title} — ${failing} failing check${failing === 1 ? "" : "s"}`; + + const pairs: KeyValuePair[] = [ + { + label: "Checks", + value: `${data.counts.passing} passing, ${data.counts.failing} failing, ${data.counts.pending} pending, ${data.counts.skipping} skipped, ${data.counts.cancelled} cancelled`, + }, + ]; + + const blocks: StructuredBlock[] = [ + { kind: "heading", level: 3, text: headingText }, + { kind: "keyValue", pairs }, + { kind: "keyValue", pairs: prMetaPairs(data) }, + ]; + + if (data.inProgress) { + blocks.push({ + kind: "text", + text: `*${data.counts.pending} check${data.counts.pending === 1 ? " is" : "s are"} still running, so more may yet fail.*`, + format: "markdown", + }); + } + + if (failing === 0) { + blocks.push({ + kind: "text", + text: "No checks are failing on this pull request.", + }); + } + + for (const check of data.failedChecks) { + blocks.push(...checkDetailBlocks(check)); + } + + if (data.truncated.checks) { + blocks.push({ + kind: "text", + text: `*Showing ${data.failedChecks.length} of ${failing} failing checks. Raise \`maxChecks\` to see more.*`, + format: "markdown", + }); + } + + return { + historyText: headingText, + entities: [], + displayContent: createStructuredContent(blocks, { rawData: data }), + }; +} + +// Copy a value onto the target only when it is a non-empty string, so a field +// gh reported as "" (common for third-party status descriptions) is absent +// rather than blank. +function assignOptionalString( + target: Record, + key: string, + value: unknown, +): void { + const str = optionalString(value); + if (str !== undefined) { + target[key] = str; + } +} + +function toFailedCheckDetail( + check: Record, + annotations: Pick< + FailedCheckDetail, + "annotations" | "annotationsTruncated" | "annotationsUnavailable" + >, +): FailedCheckDetail { + const detail: Record = { + name: optionalString(check.name) ?? "(unnamed check)", + state: optionalString(check.state) ?? "FAILURE", + ...annotations, + }; + for (const key of [ + "workflow", + "event", + "description", + "link", + "startedAt", + "completedAt", + ]) { + assignOptionalString(detail, key, check[key]); + } + return detail as unknown as FailedCheckDetail; +} + +export async function runPrFailedChecks( + params: PrFailedChecksAction["parameters"], + gh: GhRunner, +): Promise { + const maxChecks = clampCount(params.maxChecks, CHECKS_DEFAULT, CHECKS_MAX); + const maxAnnotations = clampCount( + params.maxAnnotations, + ANNOTATIONS_DEFAULT, + ANNOTATIONS_MAX, + ); + + // `gh pr checks` is addressed by PR number and repo, exactly like + // `gh pr view`, so neither call depends on the other's result. + const repo = normalizeRepoParam(params.repo); + const checksArgs = buildPrChecksArgs(params.number, repo); + const [meta, checksRes] = await Promise.all([ + fetchPrMeta(gh, params.number, repo), + gh(checksArgs), + ]); + if (!meta.ok) { + return createActionResultFromError(meta.error); + } + const pr = meta.value; + + // `gh pr checks` exits non-zero when checks fail or are pending, so the + // JSON on stdout - not the exit code - decides whether the call worked. + const checks = parseJsonArray(checksRes.stdout); + if (checks === undefined) { + if (/no checks reported/i.test(checksRes.stderr)) { + return buildStructuredPrFailedChecks({ + ...prFailedChecksBase(pr), + counts: { + total: 0, + passing: 0, + failing: 0, + pending: 0, + skipping: 0, + cancelled: 0, + }, + inProgress: false, + failedChecks: [], + truncated: { checks: false }, + retrievedAt: new Date().toISOString(), + }); + } + return createActionResultFromError( + describeGhFailure(checksArgs, checksRes), + ); + } + + const counts = countBuckets(checks); + const failed = selectFailedChecks(checks); + const detailed = failed.slice(0, maxChecks); + + // Each failing check's annotations are independent, so fetch them with a + // few calls in flight rather than one round trip at a time. + const failedChecks = await mapWithConcurrency( + detailed, + ANNOTATIONS_FETCH_CONCURRENCY, + async (check) => + toFailedCheckDetail( + check, + await fetchAnnotations( + gh, + optionalString(check.link), + maxAnnotations, + ), + ), + ); + + const data: PrFailedChecksData = { + ...prFailedChecksBase(pr), + counts, + inProgress: counts.pending > 0, + failedChecks, + truncated: { checks: failed.length > detailed.length }, + retrievedAt: new Date().toISOString(), + }; + + return buildStructuredPrFailedChecks(data); +} + +function prFailedChecksBase( + pr: PrMeta, +): Omit< + PrFailedChecksData, + "counts" | "inProgress" | "failedChecks" | "truncated" | "retrievedAt" +> { + return { + kind: "prFailedChecks", + repo: repoSlug(pr.ref), + number: pr.number, + title: pr.title, + url: pr.url, + state: pr.state, + isDraft: pr.isDraft, + headRefName: pr.headRefName, + baseRefName: pr.baseRefName, + ...(pr.headRepo !== undefined ? { headRepo: pr.headRepo } : {}), + fromFork: pr.fromFork, + }; +} diff --git a/ts/packages/agents/github-cli/src/structuredResults.ts b/ts/packages/agents/github-cli/src/structuredResults.ts new file mode 100644 index 0000000000..829d680538 --- /dev/null +++ b/ts/packages/agents/github-cli/src/structuredResults.ts @@ -0,0 +1,24 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +import { TableBlock, TableCell } from "@typeagent/agent-sdk"; +import { + ColumnSpec, + TableBuildOptions, + createTable, +} from "@typeagent/agent-sdk/helpers/display"; + +// Build a TableBlock from column specs and records. A ColumnSpec pairs a +// column definition with an accessor; the accessor is stripped here so only +// the column definition goes on the wire. +export function buildTableBlock( + colSpecs: ColumnSpec[], + records: T[], + options?: TableBuildOptions, +): TableBlock { + const columns = colSpecs.map(({ value: _value, ...col }) => col); + const rows: TableCell[][] = records.map((record) => + colSpecs.map((col) => col.value(record)), + ); + return createTable(columns, rows, options); +} diff --git a/ts/packages/agents/github-cli/test/githubCliPrDiagnostics.spec.ts b/ts/packages/agents/github-cli/test/githubCliPrDiagnostics.spec.ts new file mode 100644 index 0000000000..6e8f878d2c --- /dev/null +++ b/ts/packages/agents/github-cli/test/githubCliPrDiagnostics.spec.ts @@ -0,0 +1,1290 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Tests for the read-only pull request diagnostics actions (`prFiles` and + * `prFailedChecks`). + * + * Everything here drives the real orchestrators through a fake `GhRunner`, so + * the gh argument construction, JSON parsing, output bounds, and error paths + * are all covered without touching the network. + */ + +import type { + ActionResult, + ActionResultSuccess, + StructuredBlock, +} from "@typeagent/agent-sdk"; +import { + GhResult, + GhRunner, + PrFailedChecksData, + PrFilesData, + buildAnnotationsArgs, + buildFileEntries, + buildPrChecksArgs, + buildPrFilesArgs, + buildPrViewArgs, + clampCount, + countBuckets, + describeGhFailure, + ghFailureHint, + normalizeRepoParam, + parseCheckRunRef, + parsePrUrl, + runPrFailedChecks, + runPrFiles, + selectAnnotations, + selectFailedChecks, + truncatePatch, +} from "../src/prDiagnostics.js"; + +// ── fake gh ────────────────────────────────────────────────────────────────── + +function ok(stdout: string): GhResult { + return { stdout, stderr: "", exitCode: 0 }; +} + +function fail(stderr: string, exitCode = 1): GhResult { + return { stdout: "", stderr, exitCode }; +} + +// A gh runner backed by a list of matcher/response pairs. Records every +// invocation so tests can assert on the exact gh commands that ran. +function fakeGh( + responses: Array<{ match: (args: string[]) => boolean; res: GhResult }>, +): GhRunner & { calls: string[][] } { + const calls: string[][] = []; + const runner = async (args: string[]): Promise => { + calls.push(args); + const hit = responses.find((r) => r.match(args)); + if (hit === undefined) { + throw new Error(`unexpected gh invocation: gh ${args.join(" ")}`); + } + return hit.res; + }; + return Object.assign(runner, { calls }); +} + +const isPrView = (a: string[]) => a[0] === "pr" && a[1] === "view"; +const isPrChecks = (a: string[]) => a[0] === "pr" && a[1] === "checks"; +const isFilesApi = (a: string[]) => + a[0] === "api" && a[3].includes("/pulls/") && a[3].includes("/files"); +const isAnnotationsApi = (a: string[]) => + a[0] === "api" && a[3].includes("/annotations"); +// `per_page=100` contains "page=1", so page matching has to be anchored. +const onPage = (n: number) => (a: string[]) => a[3].endsWith(`&page=${n}`); + +const PR_VIEW_JSON = JSON.stringify({ + number: 42, + title: "Add widget", + url: "https://github.com/microsoft/TypeAgent/pull/42", + state: "OPEN", + isDraft: false, + additions: 10, + deletions: 3, + changedFiles: 2, + headRefName: "feature", + baseRefName: "main", + headRepository: { name: "TypeAgent", nameWithOwner: "microsoft/TypeAgent" }, + headRepositoryOwner: { login: "microsoft" }, +}); + +function successOf(result: ActionResult): ActionResultSuccess { + const err = errorOf(result); + if (err !== undefined) { + throw new Error(`expected a successful result, got: ${err}`); + } + return result as ActionResultSuccess; +} + +function errorOf(result: ActionResult): string | undefined { + return (result as { error?: string }).error; +} + +function rawData(result: ActionResult): T { + return (successOf(result).displayContent as { rawData: T }).rawData; +} + +function blocks(result: ActionResult): StructuredBlock[] { + return (successOf(result).displayContent as { blocks: StructuredBlock[] }) + .blocks; +} + +function markdown(result: ActionResult): string { + const alternates = ( + successOf(result).displayContent as { + alternates: Array<{ type: string; content: string }>; + } + ).alternates; + return alternates.find((a) => a.type === "markdown")!.content; +} + +// ── bounds ─────────────────────────────────────────────────────────────────── + +describe("clampCount", () => { + test("falls back when the value is missing or not a number", () => { + expect(clampCount(undefined, 50, 300)).toBe(50); + expect(clampCount(Number.NaN, 50, 300)).toBe(50); + expect(clampCount(Number.POSITIVE_INFINITY, 50, 300)).toBe(50); + }); + + test("clamps to [1, max] and floors fractions", () => { + expect(clampCount(0, 50, 300)).toBe(1); + expect(clampCount(-7, 50, 300)).toBe(1); + expect(clampCount(9999, 50, 300)).toBe(300); + expect(clampCount(12.9, 50, 300)).toBe(12); + }); +}); + +// ── URL parsing ────────────────────────────────────────────────────────────── + +describe("parsePrUrl", () => { + test("extracts the base repository, including on GitHub Enterprise", () => { + expect( + parsePrUrl("https://github.com/microsoft/TypeAgent/pull/42"), + ).toEqual({ + host: "github.com", + owner: "microsoft", + repo: "TypeAgent", + }); + expect(parsePrUrl("https://ghe.example.com/org/repo/pull/7")).toEqual({ + host: "ghe.example.com", + owner: "org", + repo: "repo", + }); + }); + + test("rejects anything that is not a pull request URL", () => { + expect(parsePrUrl("")).toBeUndefined(); + expect( + parsePrUrl("https://github.com/microsoft/TypeAgent/issues/42"), + ).toBeUndefined(); + }); +}); + +describe("normalizeRepoParam", () => { + test("reduces a pull request link to something gh --repo accepts", () => { + // gh rejects a URL pointing inside a repo ("invalid path: ..."), but a + // PR link is the most natural way to name someone else's PR. + expect( + normalizeRepoParam( + "https://github.com/microsoft/TypeAgent/pull/2974", + ), + ).toBe("github.com/microsoft/TypeAgent"); + expect( + normalizeRepoParam( + "https://github.com/microsoft/TypeAgent/pull/2974/files#diff-abc", + ), + ).toBe("github.com/microsoft/TypeAgent"); + }); + + test("keeps the host so GitHub Enterprise links stay on their host", () => { + expect( + normalizeRepoParam("https://ghe.contoso.com/team/app/pull/7"), + ).toBe("ghe.contoso.com/team/app"); + }); + + test("normalizes plain repository and clone URLs too", () => { + expect( + normalizeRepoParam("https://github.com/microsoft/TypeAgent"), + ).toBe("github.com/microsoft/TypeAgent"); + expect( + normalizeRepoParam("https://github.com/microsoft/TypeAgent.git"), + ).toBe("github.com/microsoft/TypeAgent"); + }); + + test("passes through the forms gh already understands", () => { + expect(normalizeRepoParam("microsoft/TypeAgent")).toBe( + "microsoft/TypeAgent", + ); + expect(normalizeRepoParam("github.com/microsoft/TypeAgent")).toBe( + "github.com/microsoft/TypeAgent", + ); + // A bare word is resolved by the shared repo validation, not here. + expect(normalizeRepoParam("TypeAgent")).toBe("TypeAgent"); + expect(normalizeRepoParam(undefined)).toBeUndefined(); + expect(normalizeRepoParam("")).toBe(""); + }); +}); + +describe("parseCheckRunRef", () => { + test("reads the check run id from a GitHub Actions job link", () => { + // For GitHub Actions the job id is also the check run id. + expect( + parseCheckRunRef( + "https://github.com/microsoft/TypeAgent/actions/runs/123/job/100784451953", + ), + ).toEqual({ + host: "github.com", + owner: "microsoft", + repo: "TypeAgent", + checkRunId: "100784451953", + }); + }); + + test("reads the check run id from a bare check run link", () => { + expect( + parseCheckRunRef("https://github.com/microsoft/TypeAgent/runs/999"), + ).toEqual({ + host: "github.com", + owner: "microsoft", + repo: "TypeAgent", + checkRunId: "999", + }); + }); + + test("returns undefined for links that carry no check run", () => { + // Third-party CI. + expect( + parseCheckRunRef("https://dev.azure.com/org/proj/_build/results"), + ).toBeUndefined(); + // A GitHub App page — "apps" must not be mistaken for an owner. + expect( + parseCheckRunRef( + "https://github.com/apps/microsoft-github-policy-service", + ), + ).toBeUndefined(); + expect(parseCheckRunRef(undefined)).toBeUndefined(); + }); +}); + +// ── argument construction ──────────────────────────────────────────────────── + +describe("gh argument construction", () => { + test("pr view requests every field the results depend on", () => { + const args = buildPrViewArgs(42, "microsoft/TypeAgent"); + expect(args.slice(0, 5)).toEqual([ + "pr", + "view", + "42", + "--repo", + "microsoft/TypeAgent", + ]); + const fields = args[args.indexOf("--json") + 1].split(","); + expect(fields).toEqual( + expect.arrayContaining([ + "url", + "changedFiles", + "additions", + "deletions", + "isDraft", + "headRepository", + ]), + ); + }); + + test("omits --repo when the caller did not name a repository", () => { + expect(buildPrViewArgs(42)).not.toContain("--repo"); + expect(buildPrChecksArgs(42)).not.toContain("--repo"); + }); + + test("pr checks requests link and timing fields", () => { + const fields = + buildPrChecksArgs(42)[buildPrChecksArgs(42).indexOf("--json") + 1]; + expect(fields.split(",")).toEqual( + expect.arrayContaining([ + "bucket", + "name", + "link", + "startedAt", + "completedAt", + ]), + ); + }); + + test("api calls pin the host so enterprise pull requests resolve", () => { + const ref = { host: "ghe.example.com", owner: "org", repo: "repo" }; + expect(buildPrFilesArgs(ref, 7, 2, 100, true)).toEqual([ + "api", + "--hostname", + "ghe.example.com", + "repos/org/repo/pulls/7/files?per_page=100&page=2", + ]); + expect(buildAnnotationsArgs({ ...ref, checkRunId: "55" }, 3)).toEqual([ + "api", + "--hostname", + "ghe.example.com", + "repos/org/repo/check-runs/55/annotations?per_page=100&page=3", + ]); + }); + + test("projects patches away with --jq when they were not requested", () => { + const ref = { host: "github.com", owner: "org", repo: "repo" }; + const args = buildPrFilesArgs(ref, 7, 1, 100, false); + const jq = args[args.indexOf("--jq") + 1]; + expect(args).toContain("--jq"); + expect(jq).toContain("filename"); + expect(jq).not.toContain("patch"); + // Requesting patches must not filter them out again. + expect(buildPrFilesArgs(ref, 7, 1, 100, true)).not.toContain("--jq"); + }); +}); + +// ── patch bounding ─────────────────────────────────────────────────────────── + +describe("truncatePatch", () => { + test("keeps the head of the patch, where the hunk header lives", () => { + const patch = ["@@ -1,3 +1,4 @@", "a", "b", "c", "d"].join("\n"); + const { text, keptLines, omittedLines, omittedChars } = truncatePatch( + patch, + 2, + 10_000, + ); + expect(text).toBe("@@ -1,3 +1,4 @@\na"); + expect(keptLines).toBe(2); + expect(omittedLines).toBe(3); + expect(omittedChars).toBe(0); + }); + + test("passes short patches through untouched", () => { + const patch = "@@ -1 +1 @@\n-a\n+b"; + expect(truncatePatch(patch, 40, 10_000)).toEqual({ + text: patch, + keptLines: 3, + omittedLines: 0, + omittedChars: 0, + }); + }); + + test("cuts on a line boundary when the character budget runs out first", () => { + const patch = ["@@", "aaaa", "bbbb", "cccc"].join("\n"); + const { text, keptLines, omittedLines, omittedChars } = truncatePatch( + patch, + 40, + 12, + ); + expect(text).toBe("@@\naaaa"); + expect(keptLines).toBe(2); + expect(omittedLines).toBe(2); + expect(omittedChars).toBe(patch.length - text.length); + }); + + test("bounds a single line longer than the whole budget", () => { + // A minified or generated file passes any line-based cap while still + // carrying megabytes, so the character cap has to be the real bound. + const patch = `@@\n${"x".repeat(500_000)}`; + const { text, keptLines, omittedLines, omittedChars } = truncatePatch( + patch, + 40, + 100, + ); + // Cut mid-line rather than falling back to the "@@" header alone. + expect(text).toHaveLength(100); + expect(text.startsWith("@@\nxxx")).toBe(true); + // Only the header line is shown in full. + expect(keptLines).toBe(1); + expect(omittedLines).toBe(1); + expect(omittedChars).toBeGreaterThan(400_000); + }); +}); + +describe("buildFileEntries", () => { + const raw = [ + { + filename: "src/a.ts", + status: "modified", + additions: 5, + deletions: 2, + changes: 7, + patch: "@@\n1\n2\n3\n4", + }, + { + filename: "src/new.ts", + previous_filename: "src/old.ts", + status: "renamed", + additions: 0, + deletions: 0, + changes: 0, + patch: "@@\nx", + }, + { + filename: "assets/logo.png", + status: "modified", + additions: 0, + deletions: 0, + changes: 0, + }, + ]; + + test("omits patches entirely unless the caller asked for them", () => { + const entries = buildFileEntries(raw, false, 40); + expect(entries.every((e) => e.patch === undefined)).toBe(true); + // No patch was requested, so nothing counts as omitted. + expect(entries.every((e) => e.patchOmitted === undefined)).toBe(true); + }); + + test("carries the previous path for a rename", () => { + expect(buildFileEntries(raw, false, 40)[1].previousPath).toBe( + "src/old.ts", + ); + }); + + test("marks a binary or oversized file as having no patch available", () => { + const entries = buildFileEntries(raw, true, 40); + expect(entries[2].patch).toBeUndefined(); + expect(entries[2].patchOmitted).toBe("unavailable"); + }); + + test("truncates a patch past the per-file line cap", () => { + const entries = buildFileEntries(raw, true, 2); + expect(entries[0].patch).toBe("@@\n1"); + expect(entries[0].patchTruncatedLines).toBe(3); + }); + + test("stops emitting patches once the combined budget is spent", () => { + // 20 files of 100 lines each far exceeds the 600-line total budget. + const many = Array.from({ length: 20 }, (_, i) => ({ + filename: `f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + patch: Array.from({ length: 100 }, (_, n) => `line ${n}`).join( + "\n", + ), + })); + const entries = buildFileEntries(many, true, 200); + const kept = entries + .filter((e) => e.patch !== undefined) + .reduce((sum, e) => sum + e.patch!.split("\n").length, 0); + expect(kept).toBeLessThanOrEqual(600); + expect(entries.some((e) => e.patchOmitted === "budget")).toBe(true); + }); + + test("bounds the combined payload by characters, not just lines", () => { + // Ten files, each one enormous line: every line-based cap passes, so + // only the character budget keeps the payload finite. + const many = Array.from({ length: 10 }, (_, i) => ({ + filename: `min${i}.js`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + patch: `@@\n${"x".repeat(1_000_000)}`, + })); + const entries = buildFileEntries(many, true, 200); + const totalChars = entries.reduce( + (sum, e) => sum + (e.patch?.length ?? 0), + 0, + ); + expect(totalChars).toBeLessThanOrEqual(40_000); + expect(entries[0].patchTruncatedChars).toBeGreaterThan(0); + expect(entries.some((e) => e.patchOmitted === "budget")).toBe(true); + }); +}); + +// ── check classification ───────────────────────────────────────────────────── + +describe("check bucket handling", () => { + const checks = [ + { bucket: "pass" }, + { bucket: "fail" }, + { bucket: "fail" }, + { bucket: "pending" }, + { bucket: "skipping" }, + { bucket: "cancel" }, + ]; + + test("counts every bucket gh reports", () => { + expect(countBuckets(checks)).toEqual({ + total: 6, + passing: 1, + failing: 2, + pending: 1, + skipping: 1, + cancelled: 1, + }); + }); + + test("details only genuine failures, not cancellations or pending runs", () => { + expect(selectFailedChecks(checks)).toHaveLength(2); + }); +}); + +describe("selectAnnotations", () => { + test("prefers failure-level annotations over warnings and notices", () => { + const { annotations } = selectAnnotations( + [ + { annotation_level: "warning", message: "style" }, + { annotation_level: "failure", message: "boom" }, + { annotation_level: "notice", message: "fyi" }, + ], + 10, + ); + expect(annotations).toHaveLength(1); + expect(annotations[0].message).toBe("boom"); + }); + + test("falls back to warnings when nothing failed at annotation level", () => { + const { annotations } = selectAnnotations( + [{ annotation_level: "warning", message: "style" }], + 10, + ); + expect(annotations).toHaveLength(1); + expect(annotations[0].level).toBe("warning"); + }); + + test("caps the count and reports the truncation", () => { + const raw = Array.from({ length: 5 }, (_, i) => ({ + annotation_level: "failure", + message: `e${i}`, + })); + const { annotations, truncated } = selectAnnotations(raw, 2); + expect(annotations).toHaveLength(2); + expect(truncated).toBe(true); + }); + + test("reports truncation when pages were left unread", () => { + // Everything fetched fits under the cap, but GitHub still had more, + // so claiming a complete picture would be wrong. + const { annotations, truncated } = selectAnnotations( + [{ annotation_level: "failure", message: "boom" }], + 10, + true, + ); + expect(annotations).toHaveLength(1); + expect(truncated).toBe(true); + }); + + test("caps a runaway annotation message", () => { + const { annotations } = selectAnnotations( + [{ annotation_level: "failure", message: "x".repeat(5000) }], + 10, + ); + expect(annotations[0].message.length).toBeLessThanOrEqual(401); + expect(annotations[0].message.endsWith("…")).toBe(true); + }); + + test("carries file and line location through", () => { + const { annotations } = selectAnnotations( + [ + { + annotation_level: "failure", + message: "Type error", + path: "src/a.ts", + start_line: 12, + end_line: 14, + title: "TS2339", + }, + ], + 10, + ); + expect(annotations[0]).toEqual({ + level: "failure", + message: "Type error", + path: "src/a.ts", + startLine: 12, + endLine: 14, + title: "TS2339", + }); + }); +}); + +// ── error reporting ────────────────────────────────────────────────────────── + +describe("gh failure reporting", () => { + test("reports the first meaningful stderr line and the command", () => { + const msg = describeGhFailure( + ["pr", "view", "42"], + fail("\n\ngh: Not Found (HTTP 404)\nmore detail"), + ); + expect(msg).toContain("`gh pr view 42` failed"); + expect(msg).toContain("gh: Not Found (HTTP 404)"); + expect(msg).not.toContain("more detail"); + }); + + test("falls back to the exit code when gh said nothing", () => { + expect(describeGhFailure(["api", "x"], fail("", 7))).toContain( + "gh exited with code 7", + ); + }); + + test("turns bare HTTP statuses into actionable advice", () => { + expect(ghFailureHint("HTTP 401")).toContain("gh auth login"); + expect(ghFailureHint("gh: Forbidden (HTTP 403)")).toContain( + "permission", + ); + expect(ghFailureHint("HTTP 404")).toContain("OWNER/REPO"); + expect(ghFailureHint("something else entirely")).toBeUndefined(); + }); +}); + +// ── prFiles end to end ─────────────────────────────────────────────────────── + +describe("runPrFiles", () => { + test("returns files with totals, and reports no truncation", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isFilesApi, + res: ok( + JSON.stringify([ + { + filename: "src/a.ts", + status: "modified", + additions: 8, + deletions: 3, + changes: 11, + patch: "@@\n+x", + }, + { + filename: "src/b.ts", + status: "added", + additions: 2, + deletions: 0, + changes: 2, + patch: "@@\n+y", + }, + ]), + ), + }, + ]); + + const result = await runPrFiles({ number: 42 }, gh); + const data = rawData(result); + + expect(errorOf(result)).toBeUndefined(); + expect(data.kind).toBe("prFiles"); + expect(data.repo).toBe("microsoft/TypeAgent"); + expect(data.totals).toEqual({ + additions: 10, + deletions: 3, + changedFiles: 2, + }); + expect(data.files.map((f) => f.path)).toEqual(["src/a.ts", "src/b.ts"]); + expect(data.truncated).toEqual({ files: false, patches: false }); + // Patches were not requested, so none are returned. + expect(data.files.every((f) => f.patch === undefined)).toBe(true); + expect(data.fromFork).toBe(false); + expect(Date.parse(data.retrievedAt)).not.toBeNaN(); + }); + + test("accepts a pull request link in place of an OWNER/REPO slug", async () => { + // Naming an out-of-repo PR by pasting its link is the common case; gh + // itself rejects the link, so it has to be reduced first. + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isFilesApi, res: ok("[]") }, + ]); + + const result = await runPrFiles( + { + number: 42, + repo: "https://github.com/microsoft/TypeAgent/pull/42", + }, + gh, + ); + + expect(errorOf(result)).toBeUndefined(); + const view = gh.calls.find(isPrView)!; + expect(view).toContain("--repo"); + expect(view[view.indexOf("--repo") + 1]).toBe( + "github.com/microsoft/TypeAgent", + ); + }); + + test("includes patches, rendered as diff code blocks, on request", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isFilesApi, + res: ok( + JSON.stringify([ + { + filename: "src/a.ts", + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + patch: "@@ -1 +1,2 @@\n a\n+b", + }, + ]), + ), + }, + ]); + + const result = await runPrFiles({ number: 42, includePatch: true }, gh); + const data = rawData(result); + expect(data.files[0].patch).toBe("@@ -1 +1,2 @@\n a\n+b"); + expect( + blocks(result).some( + (b) => b.kind === "code" && b.language === "diff", + ), + ).toBe(true); + }); + + test("pages until the file cap is reached", async () => { + const page = (n: number) => + JSON.stringify( + Array.from({ length: 100 }, (_, i) => ({ + filename: `p${n}-f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + })), + ); + const big = JSON.parse(PR_VIEW_JSON); + big.changedFiles = 400; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(big)) }, + { + match: (a) => isFilesApi(a) && onPage(1)(a), + res: ok(page(1)), + }, + { + match: (a) => isFilesApi(a) && onPage(2)(a), + res: ok(page(2)), + }, + ]); + + const data = rawData( + await runPrFiles({ number: 42, maxFiles: 150 }, gh), + ); + expect(data.files).toHaveLength(150); + expect(gh.calls.filter(isFilesApi)).toHaveLength(2); + expect(data.truncated.files).toBe(true); + }); + + test("does not claim truncation for a pull request of exactly maxFiles", async () => { + // One file more than the cap is requested, so a PR of exactly maxFiles + // files comes back short of that and is known to be complete. + const exact = JSON.parse(PR_VIEW_JSON); + exact.changedFiles = 10; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(exact)) }, + { + match: isFilesApi, + res: ok( + JSON.stringify( + Array.from({ length: 10 }, (_, i) => ({ + filename: `f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + })), + ), + ), + }, + ]); + const result = await runPrFiles({ number: 42, maxFiles: 10 }, gh); + expect(rawData(result).files).toHaveLength(10); + expect(rawData(result).truncated.files).toBe(false); + expect(markdown(result)).not.toContain("Raise `maxFiles`"); + // The extra file is asked for in the same request, not another one. + expect(gh.calls.filter(isFilesApi)).toHaveLength(1); + expect(gh.calls.find(isFilesApi)![3]).toContain("per_page=11"); + }); + + test("detects more files from the API even when the PR total is stale", async () => { + // A push between `gh pr view` and the files request leaves changedFiles + // equal to what was fetched, so only the extra file reveals the truth. + const stale = JSON.parse(PR_VIEW_JSON); + stale.changedFiles = 10; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(stale)) }, + { + match: isFilesApi, + res: ok( + JSON.stringify( + Array.from({ length: 11 }, (_, i) => ({ + filename: `f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + })), + ), + ), + }, + ]); + const result = await runPrFiles({ number: 42, maxFiles: 10 }, gh); + const data = rawData(result); + // The extra file is used as evidence only, never reported. + expect(data.files).toHaveLength(10); + expect(data.truncated.files).toBe(true); + expect(markdown(result)).toContain("the pull request has more"); + }); + + test("probes for one more file instead of fetching another whole page", async () => { + // Above GitHub's page ceiling the extra file cannot ride along in the + // first request, so the follow-up must ask for exactly one file - and + // never with patches, since it is only evidence. + const big = JSON.parse(PR_VIEW_JSON); + big.changedFiles = 100; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(big)) }, + { + match: (a) => isFilesApi(a) && a[3].includes("per_page=100"), + res: ok( + JSON.stringify( + Array.from({ length: 100 }, (_, i) => ({ + filename: `f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + patch: "@@\n+x", + })), + ), + ), + }, + { + match: (a) => isFilesApi(a) && a[3].includes("per_page=1&"), + res: ok( + JSON.stringify([ + { + filename: "extra.ts", + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + }, + ]), + ), + }, + ]); + const data = rawData( + await runPrFiles( + { number: 42, maxFiles: 100, includePatch: true }, + gh, + ), + ); + expect(data.files).toHaveLength(100); + // The probed file is evidence only and is never reported. + expect(data.files.some((f) => f.path === "extra.ts")).toBe(false); + expect(data.truncated.files).toBe(true); + + const probe = gh.calls.find((a) => a[3]?.includes("per_page=1&"))!; + expect(probe[3]).toContain("page=101"); + // Patches are pointless for a file we discard. + expect(probe).toContain("--jq"); + expect(gh.calls.filter(isFilesApi)).toHaveLength(2); + }); + + test("reports truncation when the PR has more files than were fetched", async () => { + const big = JSON.parse(PR_VIEW_JSON); + big.changedFiles = 40; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(big)) }, + { + match: isFilesApi, + res: ok( + JSON.stringify( + Array.from({ length: 10 }, (_, i) => ({ + filename: `f${i}.ts`, + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + })), + ), + ), + }, + ]); + const result = await runPrFiles({ number: 42, maxFiles: 10 }, gh); + expect(rawData(result).truncated.files).toBe(true); + expect(markdown(result)).toContain("Showing 10 of 40 changed files"); + }); + + test("stops paging as soon as a short page comes back", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isFilesApi, + res: ok( + JSON.stringify([ + { + filename: "only.ts", + status: "modified", + additions: 1, + deletions: 0, + changes: 1, + }, + ]), + ), + }, + ]); + await runPrFiles({ number: 42, maxFiles: 300 }, gh); + expect(gh.calls.filter(isFilesApi)).toHaveLength(1); + }); + + test("caps the page size so a small maxFiles fetches one small page", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isFilesApi, res: ok("[]") }, + ]); + await runPrFiles({ number: 42, maxFiles: 5 }, gh); + // One over the cap, so "are there more?" needs no extra request. + expect(gh.calls.find(isFilesApi)![3]).toContain("per_page=6"); + }); + + test("flags a fork pull request", async () => { + const forked = JSON.parse(PR_VIEW_JSON); + forked.headRepository = { + name: "TypeAgent", + nameWithOwner: "contributor/TypeAgent", + }; + forked.headRepositoryOwner = { login: "contributor" }; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(forked)) }, + { match: isFilesApi, res: ok("[]") }, + ]); + const data = rawData(await runPrFiles({ number: 42 }, gh)); + expect(data.fromFork).toBe(true); + expect(data.headRepo).toBe("contributor/TypeAgent"); + // The files still come from the base repository, not the fork. + expect(gh.calls.find(isFilesApi)![3]).toContain( + "repos/microsoft/TypeAgent/", + ); + }); + + test("surfaces a permissions failure from gh with advice", async () => { + const gh = fakeGh([ + { + match: isPrView, + res: fail("gh: Must have push access (HTTP 403)"), + }, + ]); + const result = await runPrFiles({ number: 42 }, gh); + expect(errorOf(result)).toContain("HTTP 403"); + expect(errorOf(result)).toContain("permission"); + // We never attempted the files call after metadata failed. + expect(gh.calls.filter(isFilesApi)).toHaveLength(0); + }); + + test("fails clearly when a files page cannot be read", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isFilesApi, res: fail("gh: Not Found (HTTP 404)") }, + ]); + const result = await runPrFiles({ number: 42 }, gh); + expect(errorOf(result)).toContain("HTTP 404"); + }); + + test("handles a pull request that changes nothing", async () => { + const empty = JSON.parse(PR_VIEW_JSON); + empty.changedFiles = 0; + empty.additions = 0; + empty.deletions = 0; + const gh = fakeGh([ + { match: isPrView, res: ok(JSON.stringify(empty)) }, + { match: isFilesApi, res: ok("[]") }, + ]); + const result = await runPrFiles({ number: 42 }, gh); + expect(rawData(result).files).toEqual([]); + expect(markdown(result)).toContain("changes no files"); + }); +}); + +// ── prFailedChecks end to end ──────────────────────────────────────────────── + +const FAILING_CHECKS_JSON = JSON.stringify([ + { bucket: "pass", name: "lint", state: "SUCCESS", link: "" }, + { + bucket: "fail", + name: "build (ubuntu)", + state: "FAILURE", + workflow: "CI", + event: "pull_request", + description: "", + link: "https://github.com/microsoft/TypeAgent/actions/runs/1/job/1234", + startedAt: "2024-05-01T10:00:00Z", + completedAt: "2024-05-01T10:12:00Z", + }, + { bucket: "pending", name: "e2e", state: "IN_PROGRESS", link: "" }, +]); + +describe("runPrFailedChecks", () => { + test("accepts a pull request link in place of an OWNER/REPO slug", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok("[]") }, + ]); + + const result = await runPrFailedChecks( + { + number: 42, + repo: "https://github.com/microsoft/TypeAgent/pull/42", + }, + gh, + ); + + expect(errorOf(result)).toBeUndefined(); + // Both gh calls are addressed by repo, so both must be normalized. + for (const call of [ + gh.calls.find(isPrView)!, + gh.calls.find(isPrChecks)!, + ]) { + expect(call[call.indexOf("--repo") + 1]).toBe( + "github.com/microsoft/TypeAgent", + ); + } + }); + + test("details failing checks with their annotations", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + // gh exits non-zero when checks are failing, but still prints JSON. + { + match: isPrChecks, + res: { stdout: FAILING_CHECKS_JSON, stderr: "", exitCode: 8 }, + }, + { + match: isAnnotationsApi, + res: ok( + JSON.stringify([ + { + annotation_level: "failure", + message: "Property 'x' does not exist.", + path: "src/a.ts", + start_line: 85, + end_line: 85, + }, + ]), + ), + }, + ]); + + const result = await runPrFailedChecks({ number: 42 }, gh); + const data = rawData(result); + + expect(errorOf(result)).toBeUndefined(); + expect(data.counts).toEqual({ + total: 3, + passing: 1, + failing: 1, + pending: 1, + skipping: 0, + cancelled: 0, + }); + // A check is still running, so the failing set may yet grow. + expect(data.inProgress).toBe(true); + expect(data.failedChecks).toHaveLength(1); + const check = data.failedChecks[0]; + expect(check.name).toBe("build (ubuntu)"); + expect(check.workflow).toBe("CI"); + expect(check.startedAt).toBe("2024-05-01T10:00:00Z"); + expect(check.annotations[0].path).toBe("src/a.ts"); + expect(check.annotationsTruncated).toBe(false); + // An empty description from gh is dropped rather than shown blank. + expect(check.description).toBeUndefined(); + // Annotations are keyed on the job id from the check link. + expect(gh.calls.find(isAnnotationsApi)![3]).toContain( + "check-runs/1234/annotations", + ); + }); + + test("reads past the first page to find the failures that explain a check", async () => { + // A noisy check reports a page of warnings before the real failure, so + // stopping at page 1 would show only style noise. + const fullPage = JSON.stringify( + Array.from({ length: 100 }, (_, i) => ({ + annotation_level: "warning", + message: `style ${i}`, + })), + ); + const secondPage = JSON.stringify([ + { annotation_level: "failure", message: "the real failure" }, + ]); + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok(FAILING_CHECKS_JSON) }, + { + match: (a) => isAnnotationsApi(a) && onPage(1)(a), + res: ok(fullPage), + }, + { + match: (a) => isAnnotationsApi(a) && onPage(2)(a), + res: ok(secondPage), + }, + ]); + const data = rawData( + await runPrFailedChecks({ number: 42 }, gh), + ); + const check = data.failedChecks[0]; + expect(check.annotations).toHaveLength(1); + expect(check.annotations[0].message).toBe("the real failure"); + // Page 2 was short, so GitHub had nothing further to offer. + expect(check.annotationsTruncated).toBe(false); + expect(gh.calls.filter(isAnnotationsApi)).toHaveLength(2); + }); + + test("admits truncation when the annotation page budget runs out", async () => { + const fullPage = JSON.stringify( + Array.from({ length: 100 }, (_, i) => ({ + annotation_level: "warning", + message: `style ${i}`, + })), + ); + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok(FAILING_CHECKS_JSON) }, + { match: isAnnotationsApi, res: ok(fullPage) }, + ]); + const result = await runPrFailedChecks({ number: 42 }, gh); + const check = rawData(result).failedChecks[0]; + expect(check.annotationsTruncated).toBe(true); + // Paging is bounded even when GitHub keeps returning full pages. + expect(gh.calls.filter(isAnnotationsApi)).toHaveLength(3); + // Raising maxAnnotations is not the only remedy, so don't imply it is. + expect(markdown(result)).toContain("follow the link for the full list"); + }); + + test("keeps the annotations it did fetch when a later page fails", async () => { + // Page 1 carries the failures that explain the check; a rate limit on + // page 2 must not throw them away. + const fullPage = JSON.stringify( + Array.from({ length: 100 }, (_, i) => ({ + annotation_level: "failure", + message: `error ${i}`, + })), + ); + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok(FAILING_CHECKS_JSON) }, + { + match: (a) => isAnnotationsApi(a) && onPage(1)(a), + res: ok(fullPage), + }, + { + match: isAnnotationsApi, + res: fail("gh: API rate limit exceeded (HTTP 403)"), + }, + ]); + const result = await runPrFailedChecks({ number: 42 }, gh); + const check = rawData(result).failedChecks[0]; + expect(check.annotations).toHaveLength(10); + expect(check.annotations[0].message).toBe("error 0"); + expect(check.annotationsTruncated).toBe(true); + expect(check.annotationsUnavailable).toContain("HTTP 403"); + expect(markdown(result)).toContain("Annotations are incomplete"); + }); + + test("degrades gracefully when annotations cannot be fetched", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok(FAILING_CHECKS_JSON) }, + { + match: isAnnotationsApi, + res: fail("gh: Not Found (HTTP 404)"), + }, + ]); + const data = rawData( + await runPrFailedChecks({ number: 42 }, gh), + ); + // The action still succeeds — only this check's detail is reduced. + expect(data.failedChecks[0].annotations).toEqual([]); + expect(data.failedChecks[0].annotationsUnavailable).toContain( + "HTTP 404", + ); + }); + + test("explains why a third-party check has no annotations", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isPrChecks, + res: ok( + JSON.stringify([ + { + bucket: "fail", + name: "Azure Pipelines", + state: "FAILURE", + link: "https://dev.azure.com/org/proj/_build/results?buildId=1", + }, + ]), + ), + }, + ]); + const data = rawData( + await runPrFailedChecks({ number: 42 }, gh), + ); + expect(data.failedChecks[0].annotationsUnavailable).toContain( + "not a GitHub check run", + ); + // No pointless annotations request was made. + expect(gh.calls.filter(isAnnotationsApi)).toHaveLength(0); + }); + + test("caps the number of checks detailed and reports the truncation", async () => { + const checks = Array.from({ length: 6 }, (_, i) => ({ + bucket: "fail", + name: `job ${i}`, + state: "FAILURE", + link: "", + })); + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: ok(JSON.stringify(checks)) }, + ]); + const data = rawData( + await runPrFailedChecks({ number: 42, maxChecks: 2 }, gh), + ); + expect(data.counts.failing).toBe(6); + expect(data.failedChecks).toHaveLength(2); + expect(data.truncated.checks).toBe(true); + }); + + test("reports an all-green pull request without listing failures", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isPrChecks, + res: ok( + JSON.stringify([ + { bucket: "pass", name: "lint", state: "SUCCESS" }, + ]), + ), + }, + ]); + const result = await runPrFailedChecks({ number: 42 }, gh); + const data = rawData(result); + expect(data.failedChecks).toEqual([]); + expect(data.inProgress).toBe(false); + expect(markdown(result)).toContain("No checks are failing"); + }); + + test("treats a pull request with no checks at all as a valid result", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isPrChecks, + res: fail("no checks reported on the 'feature' branch"), + }, + ]); + const result = await runPrFailedChecks({ number: 42 }, gh); + expect(errorOf(result)).toBeUndefined(); + expect(rawData(result).counts.total).toBe(0); + }); + + test("fails when gh cannot report checks for another reason", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { match: isPrChecks, res: fail("gh: Unauthorized (HTTP 401)") }, + ]); + const result = await runPrFailedChecks({ number: 42 }, gh); + expect(errorOf(result)).toContain("gh auth login"); + }); + + test("counts cancelled checks without trying to explain them", async () => { + const gh = fakeGh([ + { match: isPrView, res: ok(PR_VIEW_JSON) }, + { + match: isPrChecks, + res: ok( + JSON.stringify([ + { bucket: "cancel", name: "build", state: "CANCELLED" }, + ]), + ), + }, + ]); + const data = rawData( + await runPrFailedChecks({ number: 42 }, gh), + ); + expect(data.counts.cancelled).toBe(1); + expect(data.failedChecks).toEqual([]); + }); +}); diff --git a/ts/packages/agents/github-cli/test/githubCliSchema.grammar.spec.ts b/ts/packages/agents/github-cli/test/githubCliSchema.grammar.spec.ts new file mode 100644 index 0000000000..93f0db358c --- /dev/null +++ b/ts/packages/agents/github-cli/test/githubCliSchema.grammar.spec.ts @@ -0,0 +1,129 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +/** + * Grammar contract for the pull request diagnostics rules in + * github-cliSchema.agr. + * + * These phrases sit close to the existing prView ("show PR N") and prChecks + * ("show checks for PR N") rules, so the point of these tests is that each + * phrasing lands on the action the user actually meant. + */ + +import { existsSync, readFileSync } from "node:fs"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { loadGrammarRules, matchGrammar } from "@typeagent/action-grammar"; + +function resolveAgrPath(): string { + const here = dirname(fileURLToPath(import.meta.url)); + const candidates = [ + join(here, "../src/github-cliSchema.agr"), + join(here, "../../src/github-cliSchema.agr"), + ]; + for (const c of candidates) { + if (existsSync(c)) { + return c; + } + } + throw new Error(`github-cliSchema.agr not found from ${here}`); +} + +function loadGithubCliGrammar() { + const source = readFileSync(resolveAgrPath(), "utf8") + .replace(/^import .*$/m, "") + .replace(/\s*:\s*GithubCliActions\s*=/, " ="); + return loadGrammarRules("github-cliSchema.agr", source); +} + +const grammar = loadGithubCliGrammar(); + +function actions(request: string) { + return matchGrammar(grammar, request).map( + (m) => (m as { match: unknown }).match ?? m, + ) as Array<{ + actionName: string; + parameters: { + number?: number; + repo?: string; + includePatch?: boolean; + }; + }>; +} + +describe("github-cliSchema.agr — prFiles", () => { + it.each([ + "show files changed in PR 2196", + "what files does PR 2196 change", + "what files does PR 2196 touch", + "what files does PR 2196 edit", + "what files does PR 2196 modify", + "show what's changed in PR 2196", + ])("%s", (request) => { + expect(actions(request)[0]).toMatchObject({ + actionName: "prFiles", + parameters: { number: 2196 }, + }); + }); + + it("carries the repository through", () => { + expect( + actions("show files changed in PR 42 in microsoft/TypeAgent")[0], + ).toMatchObject({ + actionName: "prFiles", + parameters: { number: 42, repo: "microsoft/TypeAgent" }, + }); + }); + + it("asking for the diff turns patches on", () => { + expect(actions("show the diff for PR 42")[0]).toMatchObject({ + actionName: "prFiles", + parameters: { number: 42, includePatch: true }, + }); + }); +}); + +describe("github-cliSchema.agr — prFailedChecks", () => { + it.each([ + "show failing checks for PR 2196", + "why is CI failing on PR 2196", + "why is the CI failing on PR 2196", + "why is pipeline failing on PR 2196", + "why is the pipeline failing on PR 2196", + ])("%s", (request) => { + expect(actions(request)[0]).toMatchObject({ + actionName: "prFailedChecks", + parameters: { number: 2196 }, + }); + }); + + it("carries the repository through", () => { + expect( + actions("show failing checks for PR 42 in microsoft/TypeAgent")[0], + ).toMatchObject({ + actionName: "prFailedChecks", + parameters: { number: 42, repo: "microsoft/TypeAgent" }, + }); + }); +}); + +describe("github-cliSchema.agr — neighbouring rules still win their phrasings", () => { + it.each([ + ["show PR 42", "prView"], + ["show checks for PR 42", "prChecks"], + ["show check runs for PR 42", "prChecks"], + ["show CI status for PR 42", "prChecks"], + ])("%s stays %s", (request, expected) => { + const matched = actions(request); + expect(matched.length).toBeGreaterThan(0); + expect(matched[0].actionName).toBe(expected); + // The diagnostics rules must not have stolen these phrasings. + expect( + matched.every( + (a) => + a.actionName !== "prFiles" && + a.actionName !== "prFailedChecks", + ), + ).toBe(true); + }); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 35e2c59267..7aef8ff3e6 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -2648,6 +2648,9 @@ importers: specifier: workspace:* version: link:../../agentSdk devDependencies: + '@typeagent/action-grammar': + specifier: workspace:* + version: link:../../actionGrammar '@typeagent/action-grammar-compiler': specifier: workspace:* version: link:../../actionGrammarCompiler