diff --git a/.changeset/fix-findings-pagination-warning.md b/.changeset/fix-findings-pagination-warning.md new file mode 100644 index 0000000..1fab588 --- /dev/null +++ b/.changeset/fix-findings-pagination-warning.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": patch +--- + +Fix `findings`'s pagination warning silently not firing when the API response omits `pagination.total`: the guard now also checks for a remaining `cursor`, so a trailing page of results is no longer hidden from the `--limit` hint. diff --git a/.changeset/format-standards-analysing.md b/.changeset/format-standards-analysing.md new file mode 100644 index 0000000..0b958f6 --- /dev/null +++ b/.changeset/format-standards-analysing.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": patch +--- + +`formatStandards()` (used by `repository`'s Open Pull Requests table, `pull-request`'s Up to Standards row, and `pull-requests`' ✓ column) now shows a dim `⋯` while a pull request is still being analysed, instead of falling through to a hard ✗ on gate data that isn't final yet. diff --git a/.changeset/pull-requests-list-command.md b/.changeset/pull-requests-list-command.md new file mode 100644 index 0000000..e6fc0c9 --- /dev/null +++ b/.changeset/pull-requests-list-command.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +New `pull-requests` (`prs`) command: lists pull requests for a repository, with the same analysis-gated columns as `repository`'s "Open Pull Requests" table. `-q, --search` and `-B, --base` filter by free text (title/author handle) and target branch, mapping to the API's `textQuery`/`targetBranch` params; `-S, --state` filters by open (default) or closed. diff --git a/.changeset/pull-requests-table-polish.md b/.changeset/pull-requests-table-polish.md new file mode 100644 index 0000000..374712e --- /dev/null +++ b/.changeset/pull-requests-table-polish.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": patch +--- + +Fix PR complexity showing as no data, and polish the `pull-requests` table. Complexity is now read from the API's nested `quality` object, which is where the pull-request endpoints actually return it — `pull-requests`, `pull-request` and `repository` all previously rendered it as empty. The `pull-requests` table now leads with the up-to-standards column, orders metrics the same way `repositories` does (issues, complexity, duplication, coverage), hides the Coverage column when no listed PR has coverage data, shows `-` instead of `N/A` for metrics with no value, and no longer signs a zero issue count (`0` instead of `-0`). `--output json` now includes the quality and coverage `resultReasons`, so consumers can see which gates passed or failed. diff --git a/README.md b/README.md index c155403..6459f6b 100644 --- a/README.md +++ b/README.md @@ -85,6 +85,7 @@ Supported providers: GitHub (`gh`), GitLab (`gl`), Bitbucket (`bb`). | `findings [provider] [org] [repo]` | Show security findings for a repository or organization | | `finding ` | Show details for a single security finding, or ignore/unignore it | | `pull-request [provider] [org] [repo] ` | Show PR analysis, issues, diff coverage, and changed files; or reanalyze it (optionally waiting for results) | +| `pull-requests [provider] [org] [repo]` | List pull requests for a repository, with analysis data and text/branch filters | | `tools [provider] [org] [repo]` | List analysis tools configured for a repository | | `tool [provider] [org] [repo] ` | Enable, disable, or configure an analysis tool | | `patterns [provider] [org] [repo] ` | List patterns for a tool, or bulk enable/disable them | diff --git a/SPECS/README.md b/SPECS/README.md index 41d52e4..a8dba29 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -18,6 +18,7 @@ _No pending tasks._ All commands implemented. | `ls` | N/A | ✅ Done | [ls.md](commands/ls.md) | | `directories` | `dirs` | ✅ Done | [directories.md](commands/directories.md) | | `pull-request` | `pr` | ✅ Done (--diff + Diff Coverage Summary added) | [pull-request.md](commands/pull-request.md) | +| `pull-requests` | `prs` | ✅ Done | [pull-requests.md](commands/pull-requests.md) | | `issues` | `is` | ✅ Done | [issues.md](commands/issues.md) | | `issue` | `iss` | ✅ Done | [issue.md](commands/issue.md) | | `findings` | `fins` | ✅ Done | [findings.md](commands/findings.md) | @@ -80,3 +81,5 @@ _No pending tasks._ All commands implemented. | 2026-07-24 | Security (CWE-150, HackerOne): neutralize terminal control characters in human-readable output. New `src/utils/sanitize.ts` (`sanitizeText`) strips C0 (0x00–0x1F except TAB/LF), DEL (0x7F) and C1 (0x80–0x9F) — CR included — replacing each with visible caret/`\xNN` notation so a crafted PR can't inject ANSI/OSC sequences to repaint or hide findings, spoof gate status, or drive terminal side effects (OSC 52 clipboard, OSC 8 hyperlinks). Applied *before* the CLI's own `ansis` styling (can't sanitize at the console boundary — that would strip the CLI's legitimate colours, and allow-listing SGR would still pass attacker SGR through). Covers every render path: shared helpers in `utils/formatting.ts` (issue cards/detail, code context, CVE block, dependency chains, version segments) plus `pull-request` (About table, Files, diff-coverage, annotated diff line/hunk/path), `findings`/`finding`, `issues` (overview tables, noise suggestions), `issue`, `repository` (About, PRs, overview), `ls`/`directories`. JSON output left intact (JSON encoding already escapes control bytes). New `src/utils/sanitize.test.ts` + pull-request table/diff regression tests (12 new tests, 490 total) | | 2026-07-24 | (OD-296) Vulnerable/affected functions for SCA issues: `CommitIssue.advisoryInformation` (added server-side in API `57.3.0`, bumped from pinned `56.2.9`) is now read and rendered. `issues`/`issue`/`pull-request` default cards show a compact "Vulnerable functions: fn1, fn2 (+N more)" line (`printIssueCard`); `issue`/`pull-request --issue` detail views show the full block with advisory ID + published date (`printAdvisoryBlock`, wired into `printIssueCodeContext`). Not shown for ignored issues (`IgnoredIssue` has no `advisoryInformation`). Gated purely on `advisoryInformation` presence. Added to all three commands' JSON `pickDeep` whitelists (10 new tests, 488 total). Originally scoped to the `findings` command, but `SrmItem` has no equivalent field anywhere server-side — redirected to `issues`, where the data already existed in the public API. | | 2026-07-28 | (OD-296, findings side) `SrmItem` gained its own `advisoryInformation` field server-side (bumped pinned API `57.3.0` → `57.3.9`), closing the gap noted on 2026-07-24. `findings` (list) now shows the same compact "Vulnerable functions: fn1, fn2 (+N more)" line as `issues`, via the newly-exported `summarizeFunctions`. `finding` (detail) shows the full `printAdvisoryBlock` — but only when there's no linked Codacy issue, since `printIssueCodeContext` already renders the equivalent block from `issue.advisoryInformation` in that case; this is what makes vulnerable functions visible for SCA/dependency findings (and any other non-Codacy-source finding) that have no linked issue to borrow it from at all. Added to both commands' JSON `pickDeep` whitelists (6 new tests, 494 total) | +| 2026-07-28 | (OD-378) New `pull-requests` (`prs`) command — the plural counterpart to `pull-request`, listing PRs for a repository with the same analysis-gated table columns as `repository`'s "Open Pull Requests" section (reuses `buildGateStatus`/`formatStandards`/`formatPrIssues`/`formatPrCoverage`/`formatDelta`). `--search-text`/`-q` and `--branch`/`-b` map to the API's `textQuery`/`targetBranch` params added in OD-376; the classification param (`search`, Merged vs. last-updated) is deliberately not exposed — different axis, out of scope. `[provider] [org] [repo]` auto-detect via `resolveRepoArgs`, paginate-to-`--limit` loop matching `findings`. Registered in `src/index.ts` (10 new tests, 516 total) | +| 2026-07-30 | (OD-378, review follow-up) `pull-requests` table polish + a real data bug. **Bug:** Complexity rendered as "no data" on every PR because the API omits the flat top-level `deltaComplexity` and only returns `quality.deltaComplexity` (while still sending a top-level `deltaClonesCount`) — new shared `prQualityMetric(pr, key)` in `utils/formatting.ts` reads the nested `quality` value first and falls back to the flat field; also applied to `repository`'s Open PR table and `pull-request`'s Analysis section, which had the same bug. **Layout:** `✓` moved to the first column; metric order now matches `repositories` (issues → complexity → duplication → coverage); the Coverage column is dropped entirely when no listed PR has a coverage value (new `hasAnyPrCoverage()` — repos without coverage return `diffCoverage.cause` and no numbers on any PR); missing metric values now render as a dim `-` instead of `N/A` in `formatDelta`/`formatPrCoverage`/`formatPrIssues`, matching `formatStandards`/`formatCountCell`/`formatCoverageCell`; and a zero issue count renders as a bare `0` rather than `+0`/`-0` (`-0` read as a negative), matching what `pull-request`'s Files table and `formatDelta` already did. **JSON:** added `quality.resultReasons`/`coverage.resultReasons` (Codacy review suggestion — they drive the per-metric gate coloring, so consumers need them to see which gates passed/failed) plus the `quality.*` metric mirrors the table actually renders (23 new tests, 544 total) | diff --git a/SPECS/commands/pull-requests.md b/SPECS/commands/pull-requests.md new file mode 100644 index 0000000..4b4da09 --- /dev/null +++ b/SPECS/commands/pull-requests.md @@ -0,0 +1,68 @@ +# `pull-requests` Command Spec + +**Status:** ✅ Done (2026-07-28) + +## Purpose + +List pull requests for a repository, with analysis data — the plural counterpart to `pull-request` (which shows a single PR by number). Supports a free-text search filter, a target-branch filter, and an open/closed state filter. + +## Usage + +``` +codacy pull-requests [provider] [organization] [repository] +codacy pull-requests # auto-detect from git remote +codacy pull-requests gh my-org my-repo +codacy prs gh my-org my-repo --search "fix flaky" +codacy prs gh my-org my-repo --base main +codacy prs gh my-org my-repo --state closed +codacy prs gh my-org my-repo --output json +``` + +## Options + +| Option | Short | Description | +|---|---|---| +| `--search ` | `-q` | Filter by free-text search matched against the PR title or author handle (maps to the API's `textQuery` param) | +| `--base ` | `-B` | Filter by target (base) branch name (maps to the API's `targetBranch` param) | +| `--state ` | `-S` | `open` (default) or `closed` — maps to the API's `search` classification param (see below) | +| `--limit ` | `-n` | Maximum number of pull requests to return (default: 100, max: 1000) | + +`--search`/`-q` and `--base`/`-B` match the flag names already established elsewhere in the CLI (`findings`/`patterns` use `-q, --search`; `--base` avoids colliding with `-b, --branch`'s different meaning — "the analysed branch" — in `ls`/`directories`/`issues`). + +## State filtering + +The API's `search` param (not exposed directly) toggles between a `last-updated` classification (default) and a `merged` one; leaving it `undefined` returns closed/merged PRs mixed in with open ones with no way to tell them apart (no Status column in this command's table) — the same problem `repository.ts`'s "Open Pull Requests" table works around by filtering `status === "open" || "Open"` client-side. This command instead always passes an explicit `search` value: + +- `--state open` (default) → `search = "last-updated"` +- `--state closed` → `search = "merged"` — named `closed` on the CLI, not `merged`, because that API classification also returns closed-but-not-merged PRs, so `"merged"` would be a factually wrong label to expose. + +## API Endpoint + +[`listRepositoryPullRequests`](https://api.codacy.com/api/api-docs#listrepositorypullrequests) — `AnalysisService.listRepositoryPullRequests(provider, org, repository, limit, cursor, search, textQuery, targetBranch)`. + +Pages through `cursor` up to `--limit` (page size capped at 100 per request), same loop shape as `findings`/`issues`. The pagination-warning guard checks `total > items.length || hasMore` (not just `total`) — the API can omit `pagination.total`, and falling back to `total = items.length` would otherwise mask a page still pending behind a live `cursor`. The same latent bug (guard was `total > items.length` alone) was fixed in `findings.ts` at the same time, since that's where this loop shape was copied from. + +## Output + +Table format, columns: + +| ✓ | # | Title | Branches | Issues | Complexity | Duplication | Coverage | Updated | +|---|---|---|---|---|---|---|---|---| + +- **✓** is the leading column — the gate verdict is the first thing you scan a PR list for, so it reads before the number and title. It's dim `⋯` while `isAnalysing` is true (rather than falling through to a hard ✗ for gate data that isn't final yet), otherwise ✓/✗/`-` from quality + coverage `isUpToStandards` — a shared fix in `formatStandards()` (`utils/formatting.ts`), so it also applies to `repository`'s and `pull-request`'s uses of the same helper. +- **Branches** shows `originBranch → targetBranch` (truncated at 30) — same pairing `pull-request.ts`'s About section uses, so the "Branch" label doesn't collide with `repository.ts`'s Open PR table (which shows `originBranch` alone under the same header) or ambiguously imply which branch `--base` filters on. +- **Issues**, **Complexity**, **Duplication**, **Coverage** reuse the same shared helpers as `repository`'s "Open Pull Requests" table (`buildGateStatus`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`) and `pull-request`'s own Analysis section — gate-colored the same way. The metric order matches the `repositories` command (issues → complexity → duplication → coverage) so the same four metrics read in the same order across the CLI. +- **Complexity/Duplication read `quality.deltaComplexity`/`quality.deltaClonesCount` first**, via the shared `prQualityMetric()` helper. The API populates the flat top-level fields and the nested `quality` object inconsistently — the pull-request endpoints return `quality.deltaComplexity` but omit the top-level `deltaComplexity` (while still sending a top-level `deltaClonesCount`), so reading only the flat field rendered every PR's complexity as "no data". `quality` is the newer, structured shape (same direction as `coverage` vs. the deprecated top-level coverage fields), so it wins with the flat field as fallback. Applied to `repository` and `pull-request` too, which had the same bug. +- **Coverage is hidden entirely when no listed PR has a coverage value** (`hasAnyPrCoverage()`). A repo without coverage set up returns `diffCoverage.cause` (e.g. `MissingRequirements`) and no numbers on any PR, so the column would otherwise be a full column of `-`. The helper lives next to `formatPrCoverage` so both agree on what counts as "has data". +- **A zero issue count renders as a bare `0`**, not `+0`/`-0` — `-0` reads as a negative number, and the sign adds nothing when nothing was added or fixed. Matches the rule `pull-request.ts`'s Files table and `formatFileDelta` already use; `formatDelta` already did the same for complexity/duplication. +- **Missing values render as a dim `-`, not `N/A`** — in `formatDelta`, `formatPrCoverage`, and `formatPrIssues`, matching the convention already used by `formatStandards`, `formatCountCell`, and `formatCoverageCell`. Being shared helpers, this also applies to `repository`'s Open PR table and `pull-request`'s Analysis section. (`N/A` survives elsewhere in the CLI for non-metric fields — grades, dates, author names, default branch — which is a separate sweep.) +- Shows pagination warning (suggesting `--limit`, `--search`, `--base`, `--state`) if more results exist than were fetched. +- JSON whitelist includes `pullRequest.originBranch`/`targetBranch` (both rendered in the Branches column), `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column programmatically), the `quality.*` mirrors of the flat metric fields (the API omits some of the top-level ones, and these are what the table actually renders), and `quality.resultReasons`/`coverage.resultReasons` (which drive the per-metric gate coloring — without them a JSON consumer can't tell which gate passed or failed). It drops `pullRequest.status`/`pullRequest.owner.name` (neither rendered anywhere in the table) — matching the `pickDeep` convention in `AGENTS.md` ("only includes fields that correspond to what's shown in the console table"). + +## Tests + +File: `src/commands/pull-requests.test.ts` — 19 tests. + +Manually verified against `gh codacy codacy-website`: `--base main` → 0 results (repo's PRs all target `master`), `--base master` → matches the unfiltered count, `--search "AI"` → narrows to exactly the one matching title, `--output json` shape correct, and `[provider] [org] [repo]` auto-detect from the git remote works from inside a real checkout. + +Also verified against `gh codacy codacy-worker` (a repo with no coverage set up): PR #1218 renders Complexity `+21` (previously `N/A`, since the API omits the flat `deltaComplexity`), the Coverage column is dropped from the table, and `codacy pr gh codacy codacy-worker 1218` shows the same `+21` in its Analysis section. diff --git a/package-lock.json b/package-lock.json index 4b3df83..428bee8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@codacy/codacy-cloud-cli", - "version": "1.4.0", + "version": "1.6.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@codacy/codacy-cloud-cli", - "version": "1.4.0", + "version": "1.6.0", "license": "ISC", "dependencies": { "@codacy/tooling": "0.1.0", diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index 7d05238..ed66911 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -203,9 +203,13 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - `colorByGate(display, passing)` — green/red based on gate status - `formatDelta(value, passing)` — +/- signed value with optional gate coloring - `buildGateStatus(pr)` — maps `resultReasons` gate names to metric columns -- `formatStandards(pr)` — ✓/✗/- from quality + coverage `isUpToStandards` +- `formatStandards(pr)` — dim `⋯` while `pr.isAnalysing`, else ✓/✗/- from quality + coverage `isUpToStandards` - `formatPrCoverage(pr, passing)` — diffCoverage% (+/-deltaCoverage%) -- `formatPrIssues(pr, passing)` — +newIssues (colored by gate) / -fixedIssues (always gray) +- `formatPrIssues(pr, passing)` — +newIssues (colored by gate) / -fixedIssues (always gray). A zero count renders as a bare `0`, no sign — `-0` reads as a negative number, and neither `+0` nor `-0` says anything a plain `0` doesn't (same rule `pull-request.ts`'s Files table and `formatFileDelta` already follow) +- `prQualityMetric(pr, key)` — reads `newIssues`/`fixedIssues`/`deltaComplexity`/`deltaClonesCount`, **preferring `pr.quality[key]` over the flat top-level `pr[key]`**. The API populates the two inconsistently: the pull-request endpoints return `quality.deltaComplexity` but omit the top-level `deltaComplexity` (while still sending a top-level `deltaClonesCount`), so reading the flat field alone made every PR's complexity render as "no data". `quality` is the newer structured shape — same direction as `coverage` vs. the deprecated top-level coverage fields — so it wins, flat field as fallback. Use this instead of `pr.deltaComplexity` / `pr.deltaClonesCount` in any new PR rendering +- `hasAnyPrCoverage(prs)` — true when at least one PR in the list carries a coverage number. Callers listing many PRs use it to drop the Coverage column when the repo has no coverage set up (the API returns `diffCoverage.cause` and no values). Lives next to `formatPrCoverage` so both agree on what counts as "has data" + +**Empty-metric convention:** `formatDelta`, `formatPrCoverage`, and `formatPrIssues` render missing values as a dim `-`, matching `formatStandards`, `formatCountCell`, and `formatCoverageCell`. `N/A` is still used elsewhere in the CLI for non-metric fields (grades, dates, author names, default branch) — new metric rendering should use `-`. Dependency-chain helpers shared between `findings.ts` (list) and `finding.ts` (detail): - `formatVersionSegment(affectedVersion, fixedVersion, { includeUpdatePrefix })` — the `affected → fixed` status-line segment shown when a finding has no dependency chains; `includeUpdatePrefix` prepends `Update ` (list uses it, detail doesn't); returns `null` when there's no affected version @@ -351,6 +355,19 @@ Keeps the two command handlers thin: they only supply the API-specific callbacks - **`--reanalyze-and-wait` mode** (`-w`): blocking variant — see "Reanalyze and wait" below. Baseline comes from paging `listPullRequestIssues(status="new")`; polling reads the PR's first commit via `getPullRequestCommits(limit=1)` analysis timestamps - **Analysis status in About**: replaced "Head Commit" with "Analysis" row using `formatAnalysisStatus()` from `utils/formatting.ts`; fetches `getPullRequestCommits(limit=1)` and `listCoverageReports(limit=1)` in parallel with existing calls +## pull-requests command (`pull-requests.ts`) + +The plural counterpart to `pull-request` — lists PRs for a repository instead of showing one by number. + +- Takes `[provider] [organization] [repository]` — all optional, auto-detected from the git remote via `resolveRepoArgs(..., 0, ...)` when omitted (same pattern as `repository`/`ls`/`directories`), unlike `findings`' older hand-rolled arg-count branching +- `-q, --search ` and `-B, --base ` map to `AnalysisService.listRepositoryPullRequests`'s `textQuery`/`targetBranch` params (added server-side in OD-376, on the same `57.3.9` client bump that also brought `SrmItem.advisoryInformation`). `-q, --search` matches `findings`/`patterns`'s flag for the same free-text concept; `--base` (not `--branch`) avoids colliding with `-b, --branch`'s different meaning ("the analysed branch") in `ls`/`directories`/`issues` — this command's branch filter is the PR's *target* branch, a different axis +- `-S, --state ` (`open` default, or `closed`) always passes an explicit value for the endpoint's own `search` classification param — `open` → `"last-updated"`, `closed` → `"merged"` (the API's actual enum value, but named `closed` on the CLI since that classification also returns closed-but-not-merged PRs, so `"merged"` would mislabel it). Passing `undefined` here would return closed/merged PRs mixed into an "open" list with no Status column to tell them apart — the same problem `repository.ts`'s Open Pull Requests table works around with a client-side `status === "open" || "Open"` filter +- `-n, --limit ` (default 100, max 1000) with the same paginate-to-limit loop as `findings`, extracted into a local `fetchPullRequests()` helper: request `pageSize = min(limit, 100)`, follow `pagination.cursor`, stop once `items.length >= limit`, then trim to the exact limit. Returns `hasMore` (whether a `cursor` was still set) alongside `items`/`total` — the pagination-warning check is `total > items.length || hasMore`, not `total` alone, since the API can omit `pagination.total` (falls back to `items.length`, which would otherwise always read as "no more pages" even with a live cursor). `findings.ts` had the same latent bug (same loop shape, copied from here) fixed at the same time +- Table columns (`✓`, `#`, Title, Branches, Issues, Complexity, Duplication, Coverage, Updated): Issues/Complexity/Duplication/Coverage reuse the **same shared helpers** `repository`'s "Open Pull Requests" table uses (`buildGateStatus`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`). `✓` leads the row — the gate verdict is what you scan a PR list for — and the four metrics follow the `repositories` command's order (issues → complexity → duplication → coverage) so they read the same way across the CLI. Branches shows `originBranch → targetBranch` (truncated 30, same pairing as `pull-request.ts`'s About section) rather than either alone — disambiguates against `repository.ts`'s Open PR table, which shows `originBranch` alone under the same "Branch" header. `✓` uses `formatStandards()`, now shared-fixed to show a dim `⋯` while `isAnalysing` is true instead of falling through to a hard ✗ on gate data that isn't final yet (also benefits `repository`/`pull-request`'s uses of the same helper) +- **Coverage column is dropped entirely** when `hasAnyPrCoverage()` is false — a repo with no coverage set up returns `diffCoverage.cause` (`MissingRequirements`) and no numbers on *any* PR, so the column would render as nothing but `-`. Both the header and the cells are spread conditionally +- JSON whitelist: `pullRequest.originBranch`/`targetBranch` (both rendered via Branches), `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column — `formatStandards` reads exactly these two), the `quality.*` metric mirrors (`newIssues`/`fixedIssues`/`deltaComplexity`/`deltaClonesCount` — what the table actually renders, see `prQualityMetric` below), and `quality.resultReasons`/`coverage.resultReasons` (they drive the per-metric gate coloring, so JSON consumers need them to tell which gate passed or failed). No `pullRequest.status`/`owner.name` (neither rendered anywhere in the table) +- Shows pagination warning (suggesting `--limit`, `--search`, `--base`, `--state`) when more results exist than were fetched, same as `findings` + ## JSON Output Filtering (`pickDeep`) All commands that output JSON now filter their response using `pickDeep(data, paths)` from `utils/output.ts`. This ensures the JSON output only includes fields that correspond to what's shown in the console table/card output. diff --git a/src/commands/findings.test.ts b/src/commands/findings.test.ts index b1e5ac1..95b8b55 100644 --- a/src/commands/findings.test.ts +++ b/src/commands/findings.test.ts @@ -569,6 +569,28 @@ describe("findings command", () => { expect(output).toContain("Showing the first 100 results"); }); + it("should warn about pagination when a cursor remains even though the API omitted total", async () => { + vi.mocked(SecurityService.searchSecurityItems).mockResolvedValue({ + data: mockFindings, + pagination: { cursor: "next-cursor", limit: 1 }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "findings", + "gh", + "test-org", + "test-repo", + "--limit", + "1", + ]); + + const output = getAllOutput(); + expect(output).toContain("Showing the first 1 results"); + }); + it("should output JSON when --output json is specified", async () => { vi.mocked(SecurityService.searchSecurityItems).mockResolvedValue({ data: mockFindings, diff --git a/src/commands/findings.ts b/src/commands/findings.ts index 5fce393..613cd95 100644 --- a/src/commands/findings.ts +++ b/src/commands/findings.ts @@ -353,7 +353,7 @@ Examples: // Show repository column only when browsing org-wide (no repo filter) printFindingsList(items, total, !repository); - if (total > items.length) { + if (total > items.length || cursor) { printPaginationWarning( { cursor: "more", limit: items.length }, "Use --limit (max 1000) to fetch more, or --severities, --statuses to filter.", diff --git a/src/commands/pull-request.ts b/src/commands/pull-request.ts index 7be9f69..5a2bf82 100644 --- a/src/commands/pull-request.ts +++ b/src/commands/pull-request.ts @@ -26,6 +26,7 @@ import { printIssueCard, printIssueDetail, formatAnalysisStatus, + prQualityMetric, GateStatusMap, } from "../utils/formatting"; import { @@ -105,16 +106,16 @@ function metricHasData( switch (gateKey) { case "issues": case "security": - return pr.newIssues !== undefined; + return prQualityMetric(pr, "newIssues") !== undefined; case "coverage": return ( pr.coverage?.diffCoverage?.value !== undefined || pr.coverage?.deltaCoverage !== undefined ); case "complexity": - return pr.deltaComplexity !== undefined; + return prQualityMetric(pr, "deltaComplexity") !== undefined; case "duplication": - return pr.deltaClonesCount !== undefined; + return prQualityMetric(pr, "deltaClonesCount") !== undefined; default: return true; } @@ -258,7 +259,7 @@ function printAnalysis(pr: PullRequestWithAnalysis): void { // Complexity table.push({ Complexity: withHint( - formatDelta(pr.deltaComplexity, gates.complexity), + formatDelta(prQualityMetric(pr, "deltaComplexity"), gates.complexity), hints["complexity"], ), }); @@ -266,7 +267,7 @@ function printAnalysis(pr: PullRequestWithAnalysis): void { // Duplication table.push({ Duplication: withHint( - formatDelta(pr.deltaClonesCount, gates.duplication), + formatDelta(prQualityMetric(pr, "deltaClonesCount"), gates.duplication), hints["duplication"], ), }); diff --git a/src/commands/pull-requests.test.ts b/src/commands/pull-requests.test.ts new file mode 100644 index 0000000..5b9241e --- /dev/null +++ b/src/commands/pull-requests.test.ts @@ -0,0 +1,453 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Command } from "commander"; +import { registerPullRequestsCommand } from "./pull-requests"; +import { AnalysisService } from "../api/client/services/AnalysisService"; + +vi.mock("../api/client/services/AnalysisService"); +vi.mock("../utils/credentials", () => ({ loadCredentials: vi.fn(() => null) })); +vi.mock("../utils/git-remote", () => ({ + detectRepoContext: vi.fn(() => ({ + provider: "gh", + organization: "auto-org", + repository: "auto-repo", + })), +})); +vi.spyOn(console, "log").mockImplementation(() => {}); +vi.spyOn(console, "error").mockImplementation(() => {}); + +function createProgram(): Command { + const program = new Command(); + program.option("-o, --output ", "output format", "table"); + registerPullRequestsCommand(program); + return program; +} + +function mockPr(overrides: Record = {}) { + return { + isUpToStandards: true, + isAnalysing: false, + pullRequest: { + id: 1, + number: 42, + updated: "2025-06-14T10:00:00Z", + status: "Open", + repository: "test-repo", + title: "Add new feature", + owner: { id: 1, name: "dev-user" }, + headCommitSha: "abc1234567890", + commonAncestorCommitSha: "def456", + originBranch: "feature/new", + targetBranch: "main", + gitHref: "https://github.com/test-org/test-repo/pull/42", + }, + newIssues: 3, + fixedIssues: 1, + deltaComplexity: 2, + deltaClonesCount: -1, + coverage: { + deltaCoverage: -1.5, + diffCoverage: { value: 85.0, cause: "ValueIsPresent" }, + isUpToStandards: true, + resultReasons: [], + }, + quality: { isUpToStandards: true, resultReasons: [] }, + meta: {}, + ...overrides, + }; +} + +function getAllOutput(): string { + return (console.log as ReturnType).mock.calls + .map((c) => c[0]) + .join("\n"); +} + +describe("pull-requests command", () => { + beforeEach(() => { + vi.clearAllMocks(); + process.env.CODACY_API_TOKEN = "test-token"; + }); + + it("should fetch and display pull requests for a repository, defaulting to open state", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr(), mockPr({ pullRequest: { ...mockPr().pullRequest, number: 43, title: "Fix bug" } })], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, "last-updated", undefined, undefined, + ); + + const output = getAllOutput(); + expect(output).toContain("Add new feature"); + expect(output).toContain("Fix bug"); + expect(output).toContain("Pull Requests — Found 2 pull requests"); + expect(output).toContain("feature/new → main"); + }); + + it("should map --search to the textQuery API param", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--search", "flaky test", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, "last-updated", "flaky test", undefined, + ); + }); + + it("should map --base to the targetBranch API param", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--base", "release/1.0", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, "last-updated", undefined, "release/1.0", + ); + }); + + it("should combine --search and --base", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--search", "flaky test", "--base", "release/1.0", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, "last-updated", "flaky test", "release/1.0", + ); + }); + + it("should map --state closed to the API's merged search classification", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--state", "closed", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, "merged", undefined, undefined, + ); + }); + + it("should show a message when there are no pull requests", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("No pull requests found."); + }); + + it("should show a dim in-progress marker instead of ✓/✗ while a PR is still analysing", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr({ isAnalysing: true, isUpToStandards: undefined })], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("⋯"); + expect(output).not.toContain("✗"); + }); + + it("should render the up-to-standards column first, then metrics in repositories order", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const header = getAllOutput() + .split("\n") + .find((line) => line.includes("Issues"))!; + const order = ["✓", "#", "Title", "Branches", "Issues", "Complexity", "Duplication", "Coverage", "Updated"]; + const positions = order.map((h) => header.indexOf(h)); + expect(positions.every((p) => p >= 0)).toBe(true); + expect(positions).toEqual([...positions].sort((a, b) => a - b)); + }); + + it("should read complexity from the nested quality object when the flat field is absent", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [ + mockPr({ + deltaComplexity: undefined, + deltaClonesCount: undefined, + quality: { + deltaComplexity: 21, + deltaClonesCount: 2, + isUpToStandards: true, + resultReasons: [], + }, + }), + ], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("+21"); + expect(output).toContain("+2"); + }); + + it("should render a dash instead of N/A for metrics with no value", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [ + mockPr({ + newIssues: undefined, + fixedIssues: undefined, + deltaComplexity: undefined, + deltaClonesCount: undefined, + quality: { isUpToStandards: true, resultReasons: [] }, + }), + ], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + // Strip the dim/reset codes ansis wraps each empty cell in. + const output = getAllOutput().replace(/\[[0-9;]*m/g, ""); + expect(output).not.toContain("N/A"); + expect(output).toContain("- / -"); + }); + + it("should hide the Coverage column when no pull request has coverage data", async () => { + const noCoverage = { + diffCoverage: { cause: "MissingRequirements" }, + isUpToStandards: true, + resultReasons: [], + }; + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr({ coverage: noCoverage }), mockPr({ coverage: noCoverage })], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).not.toContain("Coverage"); + expect(output).toContain("Duplication"); + }); + + it("should keep the Coverage column when at least one pull request has coverage data", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [ + mockPr({ + coverage: { diffCoverage: { cause: "MissingRequirements" } }, + }), + mockPr(), + ], + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("Coverage"); + expect(output).toContain("85.0%"); + }); + + it("should paginate up to --limit, following the cursor", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests) + .mockResolvedValueOnce({ + data: [mockPr({ pullRequest: { ...mockPr().pullRequest, number: 1 } })], + pagination: { cursor: "page2", total: 3 }, + } as any) + .mockResolvedValueOnce({ + data: [mockPr({ pullRequest: { ...mockPr().pullRequest, number: 2 } })], + pagination: { total: 3 }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--limit", "2", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledTimes(2); + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenNthCalledWith( + 2, "gh", "test-org", "test-repo", 2, "page2", "last-updated", undefined, undefined, + ); + }); + + it("should warn about pagination when more results exist than were fetched", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + pagination: { total: 5 }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const output = getAllOutput(); + expect(output).toContain("--limit"); + }); + + it("should warn about pagination when a cursor remains even though the API omitted total", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + pagination: { cursor: "next" }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + "--limit", "1", + ]); + + const output = getAllOutput(); + expect(output).toContain("--limit"); + }); + + it("should emit structured JSON with --output json", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [mockPr()], + pagination: { total: 1 }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "--output", "json", "pull-requests", "gh", "test-org", "test-repo", + ]); + + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('"Add new feature"'), + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('"targetBranch": "main"'), + ); + expect(console.log).toHaveBeenCalledWith( + expect.stringContaining('"originBranch": "feature/new"'), + ); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('"owner"'), + ); + expect(console.log).not.toHaveBeenCalledWith( + expect.stringContaining('"status"'), + ); + }); + + it("should include the gate result reasons and nested quality metrics in JSON output", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [ + mockPr({ + deltaComplexity: undefined, + quality: { + deltaComplexity: 21, + isUpToStandards: false, + resultReasons: [ + { + gate: "issueThreshold", + expectedThreshold: { threshold: 1, minimumSeverity: "Warning" }, + isUpToStandards: false, + expected: 1, + }, + ], + }, + coverage: { + deltaCoverage: -1.5, + diffCoverage: { value: 85.0, cause: "ValueIsPresent" }, + isUpToStandards: true, + resultReasons: [ + { gate: "coverageThreshold", expectedThreshold: { threshold: 50 }, isUpToStandards: true }, + ], + }, + }), + ], + pagination: { total: 1 }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "--output", "json", "pull-requests", "gh", "test-org", "test-repo", + ]); + + const json = JSON.parse( + (console.log as ReturnType).mock.calls.at(-1)![0] as string, + ); + const pr = json.pullRequests[0]; + expect(pr.quality.resultReasons[0].gate).toBe("issueThreshold"); + expect(pr.quality.resultReasons[0].isUpToStandards).toBe(false); + expect(pr.coverage.resultReasons[0].gate).toBe("coverageThreshold"); + // The rendered complexity comes from `quality`, so JSON must carry it too. + expect(pr.quality.deltaComplexity).toBe(21); + }); + + it("should fail when CODACY_API_TOKEN is not set", async () => { + delete process.env.CODACY_API_TOKEN; + + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", "test", "pull-requests", "gh", "test-org", "test-repo", + ]), + ).rejects.toThrow("process.exit called"); + + mockExit.mockRestore(); + }); + + describe("auto-detect from git remote", () => { + it("should auto-detect provider/org/repo when no positional args are provided", async () => { + vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ + data: [], + } as any); + + const program = createProgram(); + await program.parseAsync(["node", "test", "pull-requests"]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "auto-org", "auto-repo", 100, undefined, "last-updated", undefined, undefined, + ); + }); + }); +}); diff --git a/src/commands/pull-requests.ts b/src/commands/pull-requests.ts new file mode 100644 index 0000000..0e653d2 --- /dev/null +++ b/src/commands/pull-requests.ts @@ -0,0 +1,240 @@ +import { Command } from "commander"; +import ora from "ora"; +import ansis from "ansis"; +import { checkApiToken } from "../utils/auth"; +import { handleError } from "../utils/error"; +import { resolveRepoArgs } from "../utils/resolve-repo-args"; +import { + createTable, + formatFriendlyDate, + getOutputFormat, + pickDeep, + printJson, + printPaginationWarning, +} from "../utils/output"; +import { + printSection, + truncate, + buildGateStatus, + formatStandards, + formatDelta, + formatPrCoverage, + formatPrIssues, + hasAnyPrCoverage, + prQualityMetric, +} from "../utils/formatting"; +import { sanitizeText } from "../utils/sanitize"; +import { AnalysisService } from "../api/client/services/AnalysisService"; +import { PullRequestWithAnalysis } from "../api/client/models/PullRequestWithAnalysis"; + +function printPullRequestsList(pullRequests: PullRequestWithAnalysis[]): void { + // A repo with no coverage set up returns no coverage numbers on any PR — + // drop the column instead of rendering it entirely empty. + const showCoverage = hasAnyPrCoverage(pullRequests); + const table = createTable({ + head: [ + ansis.dim("✓"), + "#", + "Title", + "Branches", + // Metric order matches the `repositories` command. + "Issues", + "Complexity", + "Duplication", + ...(showCoverage ? ["Coverage"] : []), + "Updated", + ], + }); + for (const pr of pullRequests) { + const gates = buildGateStatus(pr); + const origin = sanitizeText(pr.pullRequest.originBranch) || "-"; + const target = sanitizeText(pr.pullRequest.targetBranch) || "-"; + table.push([ + formatStandards(pr), + String(pr.pullRequest.number), + truncate(sanitizeText(pr.pullRequest.title), 40), + truncate(`${origin} → ${target}`, 30), + formatPrIssues(pr, gates.issues), + formatDelta(prQualityMetric(pr, "deltaComplexity"), gates.complexity), + formatDelta(prQualityMetric(pr, "deltaClonesCount"), gates.duplication), + ...(showCoverage ? [formatPrCoverage(pr, gates.coverage)] : []), + formatFriendlyDate(pr.pullRequest.updated), + ]); + } + console.log(table.toString()); +} + +async function fetchPullRequests( + provider: string, + organization: string, + repository: string, + limit: number, + search: string, + textQuery?: string, + targetBranch?: string, +): Promise<{ items: PullRequestWithAnalysis[]; total: number; hasMore: boolean }> { + const pageSize = Math.min(limit, 100); + let items: PullRequestWithAnalysis[] = []; + let cursor: string | undefined; + let total: number | undefined; + + do { + const response = await AnalysisService.listRepositoryPullRequests( + provider, + organization, + repository, + pageSize, + cursor, + search, + textQuery, + targetBranch, + ); + items = items.concat(response.data); + total ??= response.pagination?.total; + cursor = response.pagination?.cursor; + } while (cursor && items.length < limit); + + // A cursor can still be set even once `total` (when the API omits it and we + // fall back to `items.length`) makes the naive `total > items.length` check + // false — that's how a trailing page silently went unreported before. + const hasMore = Boolean(cursor); + if (items.length > limit) items = items.slice(0, limit); + total ??= items.length; + + return { items, total, hasMore }; +} + +function printPullRequestsJson( + items: PullRequestWithAnalysis[], + total: number, +): void { + printJson({ + pullRequests: items.map((pr: any) => pickDeep(pr, [ + "isUpToStandards", + "isAnalysing", + "pullRequest.number", + "pullRequest.title", + "pullRequest.originBranch", + "pullRequest.targetBranch", + "pullRequest.updated", + "newIssues", + "fixedIssues", + "deltaComplexity", + "deltaClonesCount", + "coverage.deltaCoverage", + "coverage.diffCoverage", + "coverage.isUpToStandards", + // `quality.*` mirrors of the flat metric fields: the API omits some of + // the top-level ones (notably `deltaComplexity`), and these are what the + // table actually renders. + "quality.newIssues", + "quality.fixedIssues", + "quality.deltaComplexity", + "quality.deltaClonesCount", + "quality.isUpToStandards", + // resultReasons drive the per-metric gate coloring in the table output — + // include them so JSON consumers can tell which gates passed or failed. + "quality.resultReasons", + "coverage.resultReasons", + ])), + total, + }); +} + +export function registerPullRequestsCommand(program: Command) { + program + .command("pull-requests") + .alias("prs") + .description("List pull requests for a repository, with analysis data") + .argument( + "[provider]", + "git provider (gh, gl, or bb) — auto-detected from git remote if omitted", + ) + .argument("[organization]", "organization name") + .argument("[repository]", "repository name") + .option( + "-q, --search ", + "filter by free-text search matched against the PR title or author handle", + ) + .option("-B, --base ", "filter by target (base) branch name") + .option( + "-S, --state ", + "filter by PR state (open|closed)", + "open", + ) + .option( + "-n, --limit ", + "maximum number of pull requests to return (default: 100, max: 1000)", + "100", + ) + .addHelpText( + "after", + ` +Examples: + $ codacy pull-requests # auto-detect from git remote + $ codacy pull-requests gh my-org my-repo + $ codacy pull-requests gh my-org my-repo --search "fix flaky" + $ codacy pull-requests gh my-org my-repo --base main + $ codacy pull-requests gh my-org my-repo --state closed + $ codacy pull-requests gh my-org my-repo --output json`, + ) + .action(async function ( + this: Command, + providerArg?: string, + organizationArg?: string, + repositoryArg?: string, + ) { + try { + checkApiToken(); + + const { provider, organization, repository } = resolveRepoArgs( + [providerArg, organizationArg, repositoryArg], + 0, + "pull-requests", + [], + ); + + const format = getOutputFormat(this); + const opts = this.opts(); + const limit = Math.min(Math.max(parseInt(opts.limit, 10) || 100, 1), 1000); + // API classification param: "closed" here maps to the API's "merged" search + // value, which also includes closed-but-not-merged PRs — "merged" would be + // a misleading name to expose on the CLI for that reason. + const search = opts.state === "closed" ? "merged" : "last-updated"; + + const spinner = ora("Fetching pull requests...").start(); + const { items, total, hasMore } = await fetchPullRequests( + provider, + organization, + repository, + limit, + search, + opts.search, + opts.base, + ); + spinner.stop(); + + if (format === "json") { + printPullRequestsJson(items, total); + return; + } + + printSection("Pull Requests", total, "pull request"); + if (items.length === 0) { + console.log(ansis.dim(" No pull requests found.")); + return; + } + + printPullRequestsList(items); + + if (total > items.length || hasMore) { + printPaginationWarning( + { cursor: "more", limit: items.length }, + "Use --limit (max 1000) to fetch more, or --search, --base, --state to filter.", + ); + } + } catch (err) { + handleError(err); + } + }); +} diff --git a/src/commands/repository.ts b/src/commands/repository.ts index e3d2172..9a6ab30 100644 --- a/src/commands/repository.ts +++ b/src/commands/repository.ts @@ -24,6 +24,7 @@ import { formatPrCoverage, formatPrIssues, formatAnalysisStatus, + prQualityMetric, } from "../utils/formatting"; import { sanitizeText } from "../utils/sanitize"; import { @@ -180,8 +181,8 @@ function printPullRequests(pullRequests: PullRequestWithAnalysis[]): void { formatStandards(pr), formatPrIssues(pr, gates.issues), formatPrCoverage(pr, gates.coverage), - formatDelta(pr.deltaComplexity, gates.complexity), - formatDelta(pr.deltaClonesCount, gates.duplication), + formatDelta(prQualityMetric(pr, "deltaComplexity"), gates.complexity), + formatDelta(prQualityMetric(pr, "deltaClonesCount"), gates.duplication), formatFriendlyDate(pr.pullRequest.updated), ]); } diff --git a/src/index.ts b/src/index.ts index 8a0576f..7b7fa36 100644 --- a/src/index.ts +++ b/src/index.ts @@ -10,6 +10,7 @@ import { registerRepositoryCommand } from "./commands/repository"; import { registerLsCommand } from "./commands/ls"; import { registerDirectoriesCommand } from "./commands/directories"; import { registerPullRequestCommand } from "./commands/pull-request"; +import { registerPullRequestsCommand } from "./commands/pull-requests"; import { registerIssuesCommand } from "./commands/issues"; import { registerIssueCommand } from "./commands/issue"; import { registerFindingsCommand } from "./commands/findings"; @@ -54,6 +55,7 @@ registerRepositoryCommand(program); registerLsCommand(program); registerDirectoriesCommand(program); registerPullRequestCommand(program); +registerPullRequestsCommand(program); registerIssuesCommand(program); registerIssueCommand(program); registerFindingsCommand(program); diff --git a/src/utils/formatting.test.ts b/src/utils/formatting.test.ts index 85022c4..d975f8c 100644 --- a/src/utils/formatting.test.ts +++ b/src/utils/formatting.test.ts @@ -11,6 +11,11 @@ import { formatGrade, formatCountCell, formatCoverageCell, + formatDelta, + formatPrCoverage, + formatPrIssues, + prQualityMetric, + hasAnyPrCoverage, } from "./formatting"; // Mock ansis to return raw text for easier testing @@ -234,6 +239,132 @@ describe("formatCoverageCell", () => { }); }); +describe("formatDelta", () => { + it("signs the delta", () => { + expect(formatDelta(21)).toBe("+21"); + expect(formatDelta(-3)).toBe("-3"); + expect(formatDelta(0)).toBe("0"); + }); + + it("renders a dash when the delta is absent", () => { + expect(formatDelta(undefined)).toBe("-"); + }); +}); + +describe("formatPrCoverage", () => { + it("renders diff coverage with the delta in parentheses", () => { + expect( + formatPrCoverage({ + coverage: { diffCoverage: { value: 85 }, deltaCoverage: -1.5 }, + } as any), + ).toBe("85.0% (-1.5%)"); + }); + + it("renders a dash when the repo reports no coverage numbers", () => { + expect( + formatPrCoverage({ + coverage: { diffCoverage: { cause: "MissingRequirements" } }, + } as any), + ).toBe("-"); + expect(formatPrCoverage({} as any)).toBe("-"); + }); +}); + +describe("formatPrIssues", () => { + it("renders new / fixed issue counts", () => { + expect(formatPrIssues({ newIssues: 3, fixedIssues: 1 } as any)).toBe( + "+3 / -1", + ); + }); + + it("reads the counts from the nested quality object", () => { + expect( + formatPrIssues({ quality: { newIssues: 3, fixedIssues: 1 } } as any), + ).toBe("+3 / -1"); + }); + + it("renders zero without a sign", () => { + expect(formatPrIssues({ newIssues: 0, fixedIssues: 0 } as any)).toBe( + "0 / 0", + ); + expect(formatPrIssues({ newIssues: 2, fixedIssues: 0 } as any)).toBe( + "+2 / 0", + ); + }); + + it("renders dashes when the counts are absent", () => { + expect(formatPrIssues({} as any)).toBe("- / -"); + }); +}); + +describe("prQualityMetric", () => { + it("prefers the nested quality value over the flat one", () => { + expect( + prQualityMetric( + { deltaComplexity: 1, quality: { deltaComplexity: 21 } } as any, + "deltaComplexity", + ), + ).toBe(21); + }); + + // The pull-request endpoints omit the top-level `deltaComplexity` entirely + // while still populating `quality.deltaComplexity`. + it("reads the nested value when the flat field is missing", () => { + expect( + prQualityMetric({ quality: { deltaComplexity: 21 } } as any, "deltaComplexity"), + ).toBe(21); + }); + + it("falls back to the flat field when quality is absent", () => { + expect( + prQualityMetric({ deltaClonesCount: 2 } as any, "deltaClonesCount"), + ).toBe(2); + }); + + it("is undefined when neither is present", () => { + expect(prQualityMetric({} as any, "deltaComplexity")).toBeUndefined(); + }); + + it("keeps a nested zero rather than falling through to the flat field", () => { + expect( + prQualityMetric( + { deltaClonesCount: 5, quality: { deltaClonesCount: 0 } } as any, + "deltaClonesCount", + ), + ).toBe(0); + }); +}); + +describe("hasAnyPrCoverage", () => { + it("is true when at least one PR has a diff coverage value", () => { + expect( + hasAnyPrCoverage([ + { coverage: { diffCoverage: { cause: "MissingRequirements" } } }, + { coverage: { diffCoverage: { value: 85 } } }, + ] as any), + ).toBe(true); + }); + + it("is true when a PR only has a coverage delta", () => { + expect(hasAnyPrCoverage([{ coverage: { deltaCoverage: -1.5 } }] as any)).toBe( + true, + ); + }); + + it("is false when no PR carries coverage numbers", () => { + expect( + hasAnyPrCoverage([ + { coverage: { diffCoverage: { cause: "MissingRequirements" } } }, + {}, + ] as any), + ).toBe(false); + }); + + it("is false for an empty list", () => { + expect(hasAnyPrCoverage([])).toBe(false); + }); +}); + describe("isBeingAnalyzed", () => { it("is true when started but never finished", () => { expect(isBeingAnalyzed("2025-06-15T10:00:00Z", undefined)).toBe(true); diff --git a/src/utils/formatting.ts b/src/utils/formatting.ts index bbea005..fe87e50 100644 --- a/src/utils/formatting.ts +++ b/src/utils/formatting.ts @@ -467,7 +467,7 @@ export function formatDelta( value: number | undefined, passing?: boolean, ): string { - if (value === undefined || value === null) return ansis.dim("N/A"); + if (value === undefined || value === null) return ansis.dim("-"); const sign = value > 0 ? "+" : ""; const display = `${sign}${value}`; if (passing !== undefined) return colorByGate(display, passing); @@ -508,6 +508,7 @@ export function buildGateStatus(pr: PullRequestWithAnalysis): GateStatusMap { * Red ✗ if either is false, green ✓ if all available are true, dim - if no data. */ export function formatStandards(pr: PullRequestWithAnalysis): string { + if (pr.isAnalysing) return ansis.dim("⋯"); const covUp = pr.coverage?.isUpToStandards; const qualUp = pr.quality?.isUpToStandards; if (covUp === undefined && qualUp === undefined) return ansis.dim("-"); @@ -524,8 +525,8 @@ export function formatPrCoverage( ): string { const diff = pr.coverage?.diffCoverage?.value; const delta = pr.coverage?.deltaCoverage; - if (diff === undefined && delta === undefined) return ansis.dim("N/A"); - const diffStr = diff !== undefined ? `${diff.toFixed(1)}%` : "N/A"; + if (diff === undefined && delta === undefined) return ansis.dim("-"); + const diffStr = diff !== undefined ? `${diff.toFixed(1)}%` : "-"; const deltaSign = delta !== undefined && delta > 0 ? "+" : ""; const deltaStr = delta !== undefined ? `(${deltaSign}${delta.toFixed(1)}%)` : ""; @@ -533,6 +534,50 @@ export function formatPrCoverage( return colorByGate(display, passing); } +/** + * True when at least one PR carries a coverage value. Repositories with no + * coverage set up return `diffCoverage.cause` (e.g. "MissingRequirements") and + * no numbers at all, so callers listing many PRs use this to drop the Coverage + * column entirely rather than print a full column of "-". + * + * Kept next to `formatPrCoverage` so both agree on what counts as "has data". + */ +export function hasAnyPrCoverage(prs: PullRequestWithAnalysis[]): boolean { + return prs.some( + (pr) => + pr.coverage?.diffCoverage?.value !== undefined || + pr.coverage?.deltaCoverage !== undefined, + ); +} + +/** + * Read a PR quality metric, preferring the nested `quality` object over the + * flat top-level field. + * + * The API populates the two inconsistently: the pull-request endpoints return + * `quality.deltaComplexity` but omit the top-level `deltaComplexity` (while + * still sending a top-level `deltaClonesCount`), so reading only the flat field + * made every PR's complexity render as "no data". `quality` is the newer, + * structured shape — same direction as `coverage` vs. the deprecated top-level + * coverage fields — so it wins, with the flat field as fallback. + */ +export function prQualityMetric( + pr: PullRequestWithAnalysis, + key: "newIssues" | "fixedIssues" | "deltaComplexity" | "deltaClonesCount", +): number | undefined { + return pr.quality?.[key] ?? pr[key]; +} + +/** + * Format an issue count for the `+new / -fixed` pair: dim `-` when absent, + * a bare `0` (no sign — nothing was added or fixed), else the signed count. + */ +function formatIssueCount(value: number | undefined, sign: "+" | "-"): string { + if (value === undefined) return "-"; + if (value === 0) return "0"; + return `${sign}${value}`; +} + /** * Format PR issues: +newIssues / -fixedIssues. * New issues colored by gate status (red if failing), fixed issues always gray. @@ -541,8 +586,8 @@ export function formatPrIssues( pr: PullRequestWithAnalysis, passing?: boolean, ): string { - const newI = pr.newIssues !== undefined ? `+${pr.newIssues}` : "N/A"; - const fixI = pr.fixedIssues !== undefined ? `-${pr.fixedIssues}` : "N/A"; + const newI = formatIssueCount(prQualityMetric(pr, "newIssues"), "+"); + const fixI = formatIssueCount(prQualityMetric(pr, "fixedIssues"), "-"); const newColored = colorByGate(newI, passing); return `${newColored} / ${ansis.dim(fixI)}`; }