From 587ff9ba04126bb413671671bdb508f2eca3d07f Mon Sep 17 00:00:00 2001 From: Pedro Pereira Date: Tue, 28 Jul 2026 17:00:16 +0100 Subject: [PATCH 1/7] feat: Add vulnerable functions to CLI issue/finding OD-296 Surfaces advisoryInformation (advisory ID, vulnerable functions, published date) across issue, issues, pull-request --issue, finding, and findings: compact one-liners on list/card views, full blocks on detail views. finding skips its own block when a linked Codacy issue already renders the same data via printIssueCodeContext, so SCA/dependency findings (which have no linked issue) are the case this closes out, now that SrmItem carries advisoryInformation directly (server-side, API 57.3.9). --- SPECS/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/SPECS/README.md b/SPECS/README.md index 41d52e4..4f86560 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -17,11 +17,11 @@ _No pending tasks._ All commands implemented. | `repository` | `repo` | ✅ Done (actions added) | [repository.md](commands/repository.md) | | `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) | -| `issues` | `is` | ✅ Done | [issues.md](commands/issues.md) | -| `issue` | `iss` | ✅ Done | [issue.md](commands/issue.md) | -| `findings` | `fins` | ✅ Done | [findings.md](commands/findings.md) | -| `finding` | `fin` | ✅ Done (CVE enrichment included) | [finding.md](commands/finding.md) | +| `pull-request` | `pr` | ✅ Done (--diff + Diff Coverage Summary + vulnerable functions added) | [pull-request.md](commands/pull-request.md) | +| `issues` | `is` | ✅ Done (vulnerable functions added) | [issues.md](commands/issues.md) | +| `issue` | `iss` | ✅ Done (vulnerable functions added) | [issue.md](commands/issue.md) | +| `findings` | `fins` | ✅ Done (advisoryInformation added) | [findings.md](commands/findings.md) | +| `finding` | `fin` | ✅ Done (CVE enrichment + advisoryInformation included) | [finding.md](commands/finding.md) | | `tools` | `tls` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | | `tool` | `tl` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | | `patterns` | `pats` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | From 6762a85f4586b379577b4ebf0482e53b7e3576b5 Mon Sep 17 00:00:00 2001 From: Pedro Pereira Date: Tue, 28 Jul 2026 17:40:35 +0100 Subject: [PATCH 2/7] doc: Simplify command inventory rows OD-296 Drop the vulnerable-functions/advisoryInformation qualifiers from the pull-request/issues/issue/findings/finding rows in the command inventory. --- SPECS/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/SPECS/README.md b/SPECS/README.md index 4f86560..41d52e4 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -17,11 +17,11 @@ _No pending tasks._ All commands implemented. | `repository` | `repo` | ✅ Done (actions added) | [repository.md](commands/repository.md) | | `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 + vulnerable functions added) | [pull-request.md](commands/pull-request.md) | -| `issues` | `is` | ✅ Done (vulnerable functions added) | [issues.md](commands/issues.md) | -| `issue` | `iss` | ✅ Done (vulnerable functions added) | [issue.md](commands/issue.md) | -| `findings` | `fins` | ✅ Done (advisoryInformation added) | [findings.md](commands/findings.md) | -| `finding` | `fin` | ✅ Done (CVE enrichment + advisoryInformation included) | [finding.md](commands/finding.md) | +| `pull-request` | `pr` | ✅ Done (--diff + Diff Coverage Summary added) | [pull-request.md](commands/pull-request.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) | +| `finding` | `fin` | ✅ Done (CVE enrichment included) | [finding.md](commands/finding.md) | | `tools` | `tls` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | | `tool` | `tl` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | | `patterns` | `pats` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | From 9a9e883471e5674b1f3b3a4b7858af1871b76885 Mon Sep 17 00:00:00 2001 From: Pedro Pereira Date: Tue, 28 Jul 2026 19:29:20 +0100 Subject: [PATCH 3/7] feat: Add pull-requests list command OD-378 New `pull-requests`/`prs` command lists PRs for a repository with the same analysis-gated columns as repository's Open Pull Requests table; -q/--search-text and -b/--branch map to the API's textQuery/targetBranch params from OD-376. Also fixes package-lock.json drift (update-notifier was declared in package.json but missing from node_modules/lockfile, breaking ts-node runs). --- .changeset/pull-requests-list-command.md | 5 + README.md | 1 + SPECS/README.md | 2 + SPECS/commands/pull-requests.md | 49 +++++ package-lock.json | 4 +- src/commands/AGENTS.md | 10 + src/commands/pull-requests.test.ts | 241 +++++++++++++++++++++++ src/commands/pull-requests.ts | 180 +++++++++++++++++ src/index.ts | 2 + 9 files changed, 492 insertions(+), 2 deletions(-) create mode 100644 .changeset/pull-requests-list-command.md create mode 100644 SPECS/commands/pull-requests.md create mode 100644 src/commands/pull-requests.test.ts create mode 100644 src/commands/pull-requests.ts diff --git a/.changeset/pull-requests-list-command.md b/.changeset/pull-requests-list-command.md new file mode 100644 index 0000000..f40f4bd --- /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-text` and `-b, --branch` filter by free text (title/author handle) and target branch, mapping to the API's `textQuery`/`targetBranch` params. 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..807e557 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,4 @@ _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) | diff --git a/SPECS/commands/pull-requests.md b/SPECS/commands/pull-requests.md new file mode 100644 index 0000000..ab65953 --- /dev/null +++ b/SPECS/commands/pull-requests.md @@ -0,0 +1,49 @@ +# `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 and a target-branch 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-text "fix flaky" +codacy prs gh my-org my-repo --branch main +codacy prs gh my-org my-repo --output json +``` + +## Options + +| Option | Short | Description | +|---|---|---| +| `--search-text ` | `-q` | Filter by free-text search matched against the PR title or author handle (maps to the API's `textQuery` param) | +| `--branch ` | `-b` | Filter by target branch name (maps to the API's `targetBranch` param) | +| `--limit ` | `-n` | Maximum number of pull requests to return (default: 100, max: 1000) | + +Omitting both filters mirrors the API's own default: all branches, no text filter, `Open` status, most-recently-updated first (the API's own `search`/classification param, which toggles Merged vs. last-updated, is not exposed here — it's a different axis from these two filters and out of scope for this command). + +## API Endpoint + +[`listRepositoryPullRequests`](https://api.codacy.com/api/api-docs#listrepositorypullrequests) — `AnalysisService.listRepositoryPullRequests(provider, org, repository, limit, cursor, search, textQuery, targetBranch)`. `search` (classification) is always passed as `undefined` by this command. + +Pages through `cursor` up to `--limit` (page size capped at 100 per request), same loop shape as `findings`/`issues`. + +## Output + +Table format, columns: + +| # | Title | Branch | ✓ | Issues | Coverage | Complexity | Duplication | Updated | +|---|---|---|---|---|---|---|---|---| + +- **Branch** shows `pullRequest.targetBranch` (truncated at 20) — the branch this command's `--branch` filter narrows by. +- **✓**, **Issues**, **Coverage**, **Complexity**, **Duplication** reuse the same shared helpers as `repository`'s "Open Pull Requests" table (`buildGateStatus`, `formatStandards`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`) and `pull-request`'s own Analysis section — gate-colored the same way. +- Shows pagination warning (suggesting `--limit`, `--search-text`, `--branch`) if more results exist than were fetched. + +## Tests + +File: `src/commands/pull-requests.test.ts` — 10 tests. 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..cc897ad 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -351,6 +351,16 @@ 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-text ` and `-b, --branch ` map straight 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`). The endpoint's own `search` param (Merged vs. last-updated classification) is a different axis and deliberately not exposed by this command — always passed as `undefined` +- `-n, --limit ` (default 100, max 1000) with the same paginate-to-limit loop as `findings`: request `pageSize = min(limit, 100)`, follow `pagination.cursor`, stop once `items.length >= limit`, then trim to the exact limit +- Table columns (`#`, Title, Branch, ✓, Issues, Coverage, Complexity, Duplication, Updated) and their rendering are the **same shared helpers** `repository`'s "Open Pull Requests" table uses (`buildGateStatus`, `formatStandards`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`) — Branch shows `targetBranch` (truncated 20) rather than `originBranch`, since that's the dimension this command's `--branch` filters on +- Shows pagination warning (suggesting `--limit`, `--search-text`, `--branch`) 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/pull-requests.test.ts b/src/commands/pull-requests.test.ts new file mode 100644 index 0000000..ac69b52 --- /dev/null +++ b/src/commands/pull-requests.test.ts @@ -0,0 +1,241 @@ +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", 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, undefined, 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"); + }); + + it("should map --search-text 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-text", "flaky test", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, undefined, "flaky test", undefined, + ); + }); + + it("should map --branch 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", + "--branch", "release/1.0", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, undefined, undefined, "release/1.0", + ); + }); + + it("should combine --search-text and --branch", 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-text", "flaky test", "--branch", "release/1.0", + ]); + + expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", 100, undefined, undefined, "flaky test", "release/1.0", + ); + }); + + 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 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", undefined, 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 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"'), + ); + }); + + 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, undefined, undefined, undefined, + ); + }); + }); +}); diff --git a/src/commands/pull-requests.ts b/src/commands/pull-requests.ts new file mode 100644 index 0000000..bacee27 --- /dev/null +++ b/src/commands/pull-requests.ts @@ -0,0 +1,180 @@ +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, +} 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 { + const table = createTable({ + head: [ + "#", + "Title", + "Branch", + ansis.dim("✓"), + "Issues", + "Coverage", + "Complexity", + "Duplication", + "Updated", + ], + }); + for (const pr of pullRequests) { + const gates = buildGateStatus(pr); + table.push([ + String(pr.pullRequest.number), + truncate(sanitizeText(pr.pullRequest.title), 40), + truncate(sanitizeText(pr.pullRequest.targetBranch) || "N/A", 20), + formatStandards(pr), + formatPrIssues(pr, gates.issues), + formatPrCoverage(pr, gates.coverage), + formatDelta(pr.deltaComplexity, gates.complexity), + formatDelta(pr.deltaClonesCount, gates.duplication), + formatFriendlyDate(pr.pullRequest.updated), + ]); + } + console.log(table.toString()); +} + +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-text ", + "filter by free-text search matched against the PR title or author handle", + ) + .option("-b, --branch ", "filter by target branch name") + .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-text "fix flaky" + $ codacy pull-requests gh my-org my-repo --branch main + $ 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); + + const spinner = ora("Fetching pull requests...").start(); + + 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, + undefined, // search (merged/last-updated classification) — not exposed by this command + opts.searchText, + opts.branch, + ); + items = items.concat(response.data); + total ??= response.pagination?.total; + cursor = response.pagination?.cursor; + } while (cursor && items.length < limit); + + // Trim to exact limit + if (items.length > limit) items = items.slice(0, limit); + total ??= items.length; + spinner.stop(); + + if (format === "json") { + printJson({ + pullRequests: items.map((pr: any) => pickDeep(pr, [ + "isUpToStandards", + "isAnalysing", + "pullRequest.number", + "pullRequest.title", + "pullRequest.status", + "pullRequest.originBranch", + "pullRequest.targetBranch", + "pullRequest.updated", + "pullRequest.owner.name", + "newIssues", + "fixedIssues", + "deltaComplexity", + "deltaClonesCount", + "coverage.deltaCoverage", + "coverage.diffCoverage", + ])), + 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) { + printPaginationWarning( + { cursor: "more", limit: items.length }, + "Use --limit (max 1000) to fetch more, or --search-text, --branch to filter.", + ); + } + } catch (err) { + handleError(err); + } + }); +} 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); From 605c7ee7860a65aba4e6957f483e2f1ca6900f0a Mon Sep 17 00:00:00 2001 From: Pedro Pereira Date: Tue, 28 Jul 2026 19:36:35 +0100 Subject: [PATCH 4/7] doc: Record manual verification for pull-requests OD-378 Confirms branch/search-text filters and JSON output against a real repo (gh codacy codacy-website), closing the ticket's manual-verify step. --- SPECS/commands/pull-requests.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SPECS/commands/pull-requests.md b/SPECS/commands/pull-requests.md index ab65953..600869b 100644 --- a/SPECS/commands/pull-requests.md +++ b/SPECS/commands/pull-requests.md @@ -47,3 +47,5 @@ Table format, columns: ## Tests File: `src/commands/pull-requests.test.ts` — 10 tests. + +Manually verified against `gh codacy codacy-website`: `--branch main` → 0 results (repo's PRs all target `master`), `--branch master` → matches the unfiltered count, `--search-text "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. From de4ca0f31e33d76b69f6a0448522a55ac68ef493 Mon Sep 17 00:00:00 2001 From: Pedro Pereira Date: Thu, 30 Jul 2026 14:32:08 +0100 Subject: [PATCH 5/7] Address pull-requests review feedback OD-378 Fixes a pagination-warning bug where a live cursor went unreported whenever the API omitted `total` (same latent bug fixed in findings), adds proper --state open/closed filtering (previously hardcoded, mixing closed PRs into results), tightens the JSON whitelist, renames -q/--search-text and -b/--branch to -q/--search and -B/--base to match existing CLI conventions, shows origin -> target in the Branches column, and surfaces isAnalysing via a shared formatStandards() fix. Responds to review comments on PR #35. --- .changeset/fix-findings-pagination-warning.md | 5 + .changeset/format-standards-analysing.md | 5 + .changeset/pull-requests-list-command.md | 2 +- SPECS/commands/pull-requests.md | 39 +++-- src/commands/AGENTS.md | 12 +- src/commands/findings.test.ts | 22 +++ src/commands/findings.ts | 2 +- src/commands/pull-requests.test.ts | 83 ++++++++-- src/commands/pull-requests.ts | 149 +++++++++++------- src/utils/formatting.ts | 1 + 10 files changed, 233 insertions(+), 87 deletions(-) create mode 100644 .changeset/fix-findings-pagination-warning.md create mode 100644 .changeset/format-standards-analysing.md 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 index f40f4bd..e6fc0c9 100644 --- a/.changeset/pull-requests-list-command.md +++ b/.changeset/pull-requests-list-command.md @@ -2,4 +2,4 @@ "@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-text` and `-b, --branch` filter by free text (title/author handle) and target branch, mapping to the API's `textQuery`/`targetBranch` params. +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/SPECS/commands/pull-requests.md b/SPECS/commands/pull-requests.md index 600869b..8b40642 100644 --- a/SPECS/commands/pull-requests.md +++ b/SPECS/commands/pull-requests.md @@ -4,7 +4,7 @@ ## 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 and a target-branch filter. +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 @@ -12,8 +12,9 @@ List pull requests for a repository, with analysis data — the plural counterpa 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-text "fix flaky" -codacy prs gh my-org my-repo --branch main +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 ``` @@ -21,31 +22,41 @@ codacy prs gh my-org my-repo --output json | Option | Short | Description | |---|---|---| -| `--search-text ` | `-q` | Filter by free-text search matched against the PR title or author handle (maps to the API's `textQuery` param) | -| `--branch ` | `-b` | Filter by target branch name (maps to the API's `targetBranch` param) | +| `--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) | -Omitting both filters mirrors the API's own default: all branches, no text filter, `Open` status, most-recently-updated first (the API's own `search`/classification param, which toggles Merged vs. last-updated, is not exposed here — it's a different axis from these two filters and out of scope for this command). +`--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)`. `search` (classification) is always passed as `undefined` by this command. +[`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`. +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 | Branch | ✓ | Issues | Coverage | Complexity | Duplication | Updated | +| # | Title | Branches | ✓ | Issues | Coverage | Complexity | Duplication | Updated | |---|---|---|---|---|---|---|---|---| -- **Branch** shows `pullRequest.targetBranch` (truncated at 20) — the branch this command's `--branch` filter narrows by. -- **✓**, **Issues**, **Coverage**, **Complexity**, **Duplication** reuse the same shared helpers as `repository`'s "Open Pull Requests" table (`buildGateStatus`, `formatStandards`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`) and `pull-request`'s own Analysis section — gate-colored the same way. -- Shows pagination warning (suggesting `--limit`, `--search-text`, `--branch`) if more results exist than were fetched. +- **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. +- **✓** is 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` as before — this is a shared fix in `formatStandards()` (`utils/formatting.ts`), so it also applies to `repository`'s and `pull-request`'s uses of the same helper. +- **Issues**, **Coverage**, **Complexity**, **Duplication** 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. +- 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) and `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column programmatically), and 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` — 10 tests. +File: `src/commands/pull-requests.test.ts` — 13 tests. -Manually verified against `gh codacy codacy-website`: `--branch main` → 0 results (repo's PRs all target `master`), `--branch master` → matches the unfiltered count, `--search-text "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. +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. diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index cc897ad..48f57c3 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -203,7 +203,7 @@ 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) @@ -356,10 +356,12 @@ Keeps the two command handlers thin: they only supply the API-specific callbacks 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-text ` and `-b, --branch ` map straight 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`). The endpoint's own `search` param (Merged vs. last-updated classification) is a different axis and deliberately not exposed by this command — always passed as `undefined` -- `-n, --limit ` (default 100, max 1000) with the same paginate-to-limit loop as `findings`: request `pageSize = min(limit, 100)`, follow `pagination.cursor`, stop once `items.length >= limit`, then trim to the exact limit -- Table columns (`#`, Title, Branch, ✓, Issues, Coverage, Complexity, Duplication, Updated) and their rendering are the **same shared helpers** `repository`'s "Open Pull Requests" table uses (`buildGateStatus`, `formatStandards`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`) — Branch shows `targetBranch` (truncated 20) rather than `originBranch`, since that's the dimension this command's `--branch` filters on -- Shows pagination warning (suggesting `--limit`, `--search-text`, `--branch`) when more results exist than were fetched, same as `findings` +- `-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, Coverage, Complexity, Duplication, Updated): Issues/Coverage/Complexity/Duplication reuse the **same shared helpers** `repository`'s "Open Pull Requests" table uses (`buildGateStatus`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`). 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) +- JSON whitelist: `pullRequest.originBranch`/`targetBranch` (both rendered via Branches), `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column — `formatStandards` reads exactly these two), 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`) 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-requests.test.ts b/src/commands/pull-requests.test.ts index ac69b52..e5e3e30 100644 --- a/src/commands/pull-requests.test.ts +++ b/src/commands/pull-requests.test.ts @@ -68,7 +68,7 @@ describe("pull-requests command", () => { process.env.CODACY_API_TOKEN = "test-token"; }); - it("should fetch and display pull requests for a repository", async () => { + 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); @@ -79,16 +79,17 @@ describe("pull-requests command", () => { ]); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( - "gh", "test-org", "test-repo", 100, undefined, undefined, undefined, undefined, + "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-text to the textQuery API param", async () => { + it("should map --search to the textQuery API param", async () => { vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ data: [mockPr()], } as any); @@ -96,15 +97,15 @@ describe("pull-requests command", () => { const program = createProgram(); await program.parseAsync([ "node", "test", "pull-requests", "gh", "test-org", "test-repo", - "--search-text", "flaky test", + "--search", "flaky test", ]); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( - "gh", "test-org", "test-repo", 100, undefined, undefined, "flaky test", undefined, + "gh", "test-org", "test-repo", 100, undefined, "last-updated", "flaky test", undefined, ); }); - it("should map --branch to the targetBranch API param", async () => { + it("should map --base to the targetBranch API param", async () => { vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ data: [mockPr()], } as any); @@ -112,15 +113,15 @@ describe("pull-requests command", () => { const program = createProgram(); await program.parseAsync([ "node", "test", "pull-requests", "gh", "test-org", "test-repo", - "--branch", "release/1.0", + "--base", "release/1.0", ]); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( - "gh", "test-org", "test-repo", 100, undefined, undefined, undefined, "release/1.0", + "gh", "test-org", "test-repo", 100, undefined, "last-updated", undefined, "release/1.0", ); }); - it("should combine --search-text and --branch", async () => { + it("should combine --search and --base", async () => { vi.mocked(AnalysisService.listRepositoryPullRequests).mockResolvedValue({ data: [mockPr()], } as any); @@ -128,11 +129,27 @@ describe("pull-requests command", () => { const program = createProgram(); await program.parseAsync([ "node", "test", "pull-requests", "gh", "test-org", "test-repo", - "--search-text", "flaky test", "--branch", "release/1.0", + "--search", "flaky test", "--base", "release/1.0", ]); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( - "gh", "test-org", "test-repo", 100, undefined, undefined, "flaky test", "release/1.0", + "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, ); }); @@ -150,6 +167,21 @@ describe("pull-requests command", () => { 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 paginate up to --limit, following the cursor", async () => { vi.mocked(AnalysisService.listRepositoryPullRequests) .mockResolvedValueOnce({ @@ -169,7 +201,7 @@ describe("pull-requests command", () => { expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledTimes(2); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenNthCalledWith( - 2, "gh", "test-org", "test-repo", 2, "page2", undefined, undefined, undefined, + 2, "gh", "test-org", "test-repo", 2, "page2", "last-updated", undefined, undefined, ); }); @@ -188,6 +220,22 @@ describe("pull-requests command", () => { 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()], @@ -205,6 +253,15 @@ describe("pull-requests command", () => { 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 fail when CODACY_API_TOKEN is not set", async () => { @@ -234,7 +291,7 @@ describe("pull-requests command", () => { await program.parseAsync(["node", "test", "pull-requests"]); expect(AnalysisService.listRepositoryPullRequests).toHaveBeenCalledWith( - "gh", "auto-org", "auto-repo", 100, undefined, undefined, undefined, undefined, + "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 index bacee27..32fc397 100644 --- a/src/commands/pull-requests.ts +++ b/src/commands/pull-requests.ts @@ -30,7 +30,7 @@ function printPullRequestsList(pullRequests: PullRequestWithAnalysis[]): void { head: [ "#", "Title", - "Branch", + "Branches", ansis.dim("✓"), "Issues", "Coverage", @@ -41,10 +41,12 @@ function printPullRequestsList(pullRequests: PullRequestWithAnalysis[]): void { }); for (const pr of pullRequests) { const gates = buildGateStatus(pr); + const origin = sanitizeText(pr.pullRequest.originBranch) || "N/A"; + const target = sanitizeText(pr.pullRequest.targetBranch) || "N/A"; table.push([ String(pr.pullRequest.number), truncate(sanitizeText(pr.pullRequest.title), 40), - truncate(sanitizeText(pr.pullRequest.targetBranch) || "N/A", 20), + truncate(`${origin} → ${target}`, 30), formatStandards(pr), formatPrIssues(pr, gates.issues), formatPrCoverage(pr, gates.coverage), @@ -56,6 +58,72 @@ function printPullRequestsList(pullRequests: PullRequestWithAnalysis[]): void { 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.isUpToStandards", + ])), + total, + }); +} + export function registerPullRequestsCommand(program: Command) { program .command("pull-requests") @@ -68,10 +136,15 @@ export function registerPullRequestsCommand(program: Command) { .argument("[organization]", "organization name") .argument("[repository]", "repository name") .option( - "-q, --search-text ", + "-q, --search ", "filter by free-text search matched against the PR title or author handle", ) - .option("-b, --branch ", "filter by target branch name") + .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)", @@ -83,8 +156,9 @@ export function registerPullRequestsCommand(program: Command) { 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-text "fix flaky" - $ codacy pull-requests gh my-org my-repo --branch main + $ 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 ( @@ -106,56 +180,25 @@ Examples: 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 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, - undefined, // search (merged/last-updated classification) — not exposed by this command - opts.searchText, - opts.branch, - ); - items = items.concat(response.data); - total ??= response.pagination?.total; - cursor = response.pagination?.cursor; - } while (cursor && items.length < limit); - - // Trim to exact limit - if (items.length > limit) items = items.slice(0, limit); - total ??= items.length; + const { items, total, hasMore } = await fetchPullRequests( + provider, + organization, + repository, + limit, + search, + opts.search, + opts.base, + ); spinner.stop(); if (format === "json") { - printJson({ - pullRequests: items.map((pr: any) => pickDeep(pr, [ - "isUpToStandards", - "isAnalysing", - "pullRequest.number", - "pullRequest.title", - "pullRequest.status", - "pullRequest.originBranch", - "pullRequest.targetBranch", - "pullRequest.updated", - "pullRequest.owner.name", - "newIssues", - "fixedIssues", - "deltaComplexity", - "deltaClonesCount", - "coverage.deltaCoverage", - "coverage.diffCoverage", - ])), - total, - }); + printPullRequestsJson(items, total); return; } @@ -167,10 +210,10 @@ Examples: printPullRequestsList(items); - if (total > items.length) { + if (total > items.length || hasMore) { printPaginationWarning( { cursor: "more", limit: items.length }, - "Use --limit (max 1000) to fetch more, or --search-text, --branch to filter.", + "Use --limit (max 1000) to fetch more, or --search, --base, --state to filter.", ); } } catch (err) { diff --git a/src/utils/formatting.ts b/src/utils/formatting.ts index bbea005..46d4b48 100644 --- a/src/utils/formatting.ts +++ b/src/utils/formatting.ts @@ -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("-"); From 6808b45a2c5298670546add1dfcdec01abebaaf8 Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Thu, 30 Jul 2026 15:30:53 +0100 Subject: [PATCH 6/7] Fix PR complexity and polish pull-requests table OD-378 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complexity rendered as no data on every PR: the pull-request endpoints return `quality.deltaComplexity` and omit the flat top-level `deltaComplexity` (while still sending a top-level `deltaClonesCount`). New shared `prQualityMetric()` reads the nested `quality` value first, falling back to the flat field — also applied to `repository`'s Open PR table and `pull-request`'s Analysis section, which had the same bug. Table changes for `pull-requests`: - `✓` moved to the first column - metric order matches `repositories`: issues, complexity, duplication, coverage - Coverage column dropped entirely when no listed PR carries a coverage value (repos without coverage return `diffCoverage.cause` and no numbers on any PR) - missing metrics render as a dim `-` instead of `N/A`, matching `formatStandards`/`formatCountCell`/`formatCoverageCell` JSON output gains `quality.resultReasons`/`coverage.resultReasons` (addresses the Codacy review comment — they drive the per-metric gate coloring) plus the `quality.*` metric mirrors the table renders. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/pull-requests-table-polish.md | 5 + SPECS/README.md | 1 + SPECS/commands/pull-requests.md | 15 ++- src/commands/AGENTS.md | 9 +- src/commands/pull-request.ts | 11 +- src/commands/pull-requests.test.ts | 155 +++++++++++++++++++++++ src/commands/pull-requests.ts | 33 +++-- src/commands/repository.ts | 5 +- src/utils/formatting.test.ts | 122 ++++++++++++++++++ src/utils/formatting.ts | 46 ++++++- 10 files changed, 375 insertions(+), 27 deletions(-) create mode 100644 .changeset/pull-requests-table-polish.md diff --git a/.changeset/pull-requests-table-polish.md b/.changeset/pull-requests-table-polish.md new file mode 100644 index 0000000..513846f --- /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, and shows `-` instead of `N/A` for metrics with no value. `--output json` now includes the quality and coverage `resultReasons`, so consumers can see which gates passed or failed. diff --git a/SPECS/README.md b/SPECS/README.md index 807e557..fd9fd12 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -82,3 +82,4 @@ _No pending tasks._ All commands implemented. | 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`. **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 (22 new tests, 543 total) | diff --git a/SPECS/commands/pull-requests.md b/SPECS/commands/pull-requests.md index 8b40642..0a61fe1 100644 --- a/SPECS/commands/pull-requests.md +++ b/SPECS/commands/pull-requests.md @@ -46,17 +46,22 @@ Pages through `cursor` up to `--limit` (page size capped at 100 per request), sa Table format, columns: -| # | Title | Branches | ✓ | Issues | Coverage | Complexity | Duplication | Updated | +| ✓ | # | 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. -- **✓** is 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` as before — this is a shared fix in `formatStandards()` (`utils/formatting.ts`), so it also applies to `repository`'s and `pull-request`'s uses of the same helper. -- **Issues**, **Coverage**, **Complexity**, **Duplication** 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. +- **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". +- **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) and `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column programmatically), and 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"). +- 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` — 13 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/src/commands/AGENTS.md b/src/commands/AGENTS.md index 48f57c3..e925dfc 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -206,6 +206,10 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - `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) +- `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 @@ -359,8 +363,9 @@ The plural counterpart to `pull-request` — lists PRs for a repository instead - `-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, Coverage, Complexity, Duplication, Updated): Issues/Coverage/Complexity/Duplication reuse the **same shared helpers** `repository`'s "Open Pull Requests" table uses (`buildGateStatus`, `formatPrIssues`, `formatPrCoverage`, `formatDelta`). 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) -- JSON whitelist: `pullRequest.originBranch`/`targetBranch` (both rendered via Branches), `coverage.isUpToStandards`/`quality.isUpToStandards` (needed to reproduce the ✓ column — `formatStandards` reads exactly these two), no `pullRequest.status`/`owner.name` (neither rendered anywhere in the table) +- 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`) 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 index e5e3e30..5b9241e 100644 --- a/src/commands/pull-requests.test.ts +++ b/src/commands/pull-requests.test.ts @@ -182,6 +182,115 @@ describe("pull-requests command", () => { 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({ @@ -264,6 +373,52 @@ describe("pull-requests command", () => { ); }); + 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; diff --git a/src/commands/pull-requests.ts b/src/commands/pull-requests.ts index 32fc397..0e653d2 100644 --- a/src/commands/pull-requests.ts +++ b/src/commands/pull-requests.ts @@ -20,38 +20,44 @@ import { 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", - ansis.dim("✓"), + // Metric order matches the `repositories` command. "Issues", - "Coverage", "Complexity", "Duplication", + ...(showCoverage ? ["Coverage"] : []), "Updated", ], }); for (const pr of pullRequests) { const gates = buildGateStatus(pr); - const origin = sanitizeText(pr.pullRequest.originBranch) || "N/A"; - const target = sanitizeText(pr.pullRequest.targetBranch) || "N/A"; + 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), - 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), + ...(showCoverage ? [formatPrCoverage(pr, gates.coverage)] : []), formatFriendlyDate(pr.pullRequest.updated), ]); } @@ -118,7 +124,18 @@ function printPullRequestsJson( "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, }); 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/utils/formatting.test.ts b/src/utils/formatting.test.ts index 85022c4..beb7c73 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,123 @@ 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 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 46d4b48..067c0c3 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); @@ -525,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)}%)` : ""; @@ -534,6 +534,40 @@ 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 PR issues: +newIssues / -fixedIssues. * New issues colored by gate status (red if failing), fixed issues always gray. @@ -542,8 +576,10 @@ 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 newIssues = prQualityMetric(pr, "newIssues"); + const fixedIssues = prQualityMetric(pr, "fixedIssues"); + const newI = newIssues !== undefined ? `+${newIssues}` : "-"; + const fixI = fixedIssues !== undefined ? `-${fixedIssues}` : "-"; const newColored = colorByGate(newI, passing); return `${newColored} / ${ansis.dim(fixI)}`; } From f55c6eb379b19a1b48ed22d6e7cf989c20df590f Mon Sep 17 00:00:00 2001 From: Alejandro Rizzo Date: Thu, 30 Jul 2026 15:42:23 +0100 Subject: [PATCH 7/7] Drop the sign from zero issue counts OD-378 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `formatPrIssues` rendered a zero fixed-issue count as `-0`, which reads as a negative number. A zero count now renders as a bare `0` — the sign says nothing when nothing was added or fixed. Same rule `pull-request.ts`'s Files table and `formatFileDelta` already follow, and `formatDelta` already did for complexity/duplication. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/pull-requests-table-polish.md | 2 +- SPECS/README.md | 2 +- SPECS/commands/pull-requests.md | 1 + src/commands/AGENTS.md | 2 +- src/utils/formatting.test.ts | 9 +++++++++ src/utils/formatting.ts | 16 ++++++++++++---- 6 files changed, 25 insertions(+), 7 deletions(-) diff --git a/.changeset/pull-requests-table-polish.md b/.changeset/pull-requests-table-polish.md index 513846f..374712e 100644 --- a/.changeset/pull-requests-table-polish.md +++ b/.changeset/pull-requests-table-polish.md @@ -2,4 +2,4 @@ "@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, and shows `-` instead of `N/A` for metrics with no value. `--output json` now includes the quality and coverage `resultReasons`, so consumers can see which gates passed or failed. +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/SPECS/README.md b/SPECS/README.md index fd9fd12..a8dba29 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -82,4 +82,4 @@ _No pending tasks._ All commands implemented. | 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`. **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 (22 new tests, 543 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 index 0a61fe1..4b4da09 100644 --- a/SPECS/commands/pull-requests.md +++ b/SPECS/commands/pull-requests.md @@ -54,6 +54,7 @@ Table format, columns: - **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"). diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index e925dfc..ed66911 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -205,7 +205,7 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - `buildGateStatus(pr)` — maps `resultReasons` gate names to metric columns - `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" diff --git a/src/utils/formatting.test.ts b/src/utils/formatting.test.ts index beb7c73..d975f8c 100644 --- a/src/utils/formatting.test.ts +++ b/src/utils/formatting.test.ts @@ -283,6 +283,15 @@ describe("formatPrIssues", () => { ).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("- / -"); }); diff --git a/src/utils/formatting.ts b/src/utils/formatting.ts index 067c0c3..fe87e50 100644 --- a/src/utils/formatting.ts +++ b/src/utils/formatting.ts @@ -568,6 +568,16 @@ export function prQualityMetric( 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. @@ -576,10 +586,8 @@ export function formatPrIssues( pr: PullRequestWithAnalysis, passing?: boolean, ): string { - const newIssues = prQualityMetric(pr, "newIssues"); - const fixedIssues = prQualityMetric(pr, "fixedIssues"); - const newI = newIssues !== undefined ? `+${newIssues}` : "-"; - const fixI = fixedIssues !== undefined ? `-${fixedIssues}` : "-"; + const newI = formatIssueCount(prQualityMetric(pr, "newIssues"), "+"); + const fixI = formatIssueCount(prQualityMetric(pr, "fixedIssues"), "-"); const newColored = colorByGate(newI, passing); return `${newColored} / ${ansis.dim(fixI)}`; }