diff --git a/.changeset/issues-overview-noise.md b/.changeset/issues-overview-noise.md new file mode 100644 index 0000000..4c60952 --- /dev/null +++ b/.changeset/issues-overview-noise.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +Improve `issues --overview`. The False Positives table now uses human-friendly labels ("Not a False Positive" / "Potential False Positive") instead of the raw `belowThreshold` / `equalOrAboveThreshold` API bucket names. The overview also adds a "Suggested actions to reduce noise" section that flags noisy patterns — those accounting for at least 10% of all issues, or at least 3× the average issues-per-pattern — and prints a ready-to-run `codacy pattern --disable` command for each (the owning tool is resolved automatically; suggestions whose tool can't be resolved are omitted). `--output json` output is unchanged. diff --git a/.changeset/pattern-config-file-awareness.md b/.changeset/pattern-config-file-awareness.md new file mode 100644 index 0000000..410a066 --- /dev/null +++ b/.changeset/pattern-config-file-awareness.md @@ -0,0 +1,10 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +Make the pattern commands aware of local configuration files and coding standards. + +- `pattern ` with no action flag now **shows the pattern's information** (same card as the `patterns` command, with `--output json` support). Since there's no single-pattern endpoint, it searches by ID and keeps the exact match. +- When a tool is driven by a local configuration file, `patterns` (list) and `pattern` (info) print ` is using a local configuration file.` and skip fetching patterns; `patterns --enable-all/--disable-all` and `pattern --enable/--disable/--parameter` refuse with `Tool uses a local configuration file, can't be updated.` +- `pattern --enable/--disable/--parameter` also refuses patterns enforced by a coding standard with `Pattern enforced by coding standard, can't be modified.` +- `issues --overview` noise suggestions now adapt per pattern: a runnable `codacy pattern … --disable` command when possible, otherwise a manual step — `Update your local configuration file to disable the pattern` or `Update to disable the pattern`. diff --git a/.changeset/reanalyze-and-wait.md b/.changeset/reanalyze-and-wait.md new file mode 100644 index 0000000..9775b9d --- /dev/null +++ b/.changeset/reanalyze-and-wait.md @@ -0,0 +1,5 @@ +--- +"@codacy/codacy-cloud-cli": minor +--- + +Add a `--reanalyze-and-wait` (`-w`) variant to the `repository` and `pull-request` commands. Unlike `--reanalyze` (which triggers analysis and exits), this blocking variant captures a baseline of the current issues, triggers the reanalysis, polls until it finishes (every 10s, up to 20 minutes), and then prints how long the analysis took and what changed — issue deltas by pattern, severity, and category. Supports `--output json`. diff --git a/AGENTS.md b/AGENTS.md index 9b8de61..699e131 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -76,6 +76,10 @@ codacy-cloud-cli/ - `dayjs` for date formatting — for "last updated" style dates, use `formatFriendlyDate()` from `utils/output.ts` (relative for today, "Yesterday", otherwise YYYY-MM-DD) - **Output:** Default output is human readable with tables and colors, but can be overridden with the `--output json` flag. - **Pagination:** All commands calling paginated APIs must call `printPaginationWarning(response.pagination, hint)` from `utils/output.ts` after displaying results. The hint should suggest command-specific filtering options. +- **Polling / waiting:** Commands that wait on a remote operation (e.g. `--reanalyze-and-wait`) use the shared helpers in `utils/reanalyze-wait.ts`. + - Route polling delays through the exported `timers.sleep` so tests can stub it (`vi.spyOn(timers, "sleep").mockResolvedValue()`). + - Prefer that over calling `setTimeout`/`sleep` directly in a command, unless you have a clear reason not to. + - Default cadence is `POLL_INTERVAL_MS` (10s), capped at `MAX_WAIT_MS` (20min). - **Error handling:** Use `try/catch` with the shared `handleError()` from `src/utils/error.ts` - **Authentication:** All commands that call the API must call `checkApiToken()` from `src/utils/auth.ts` before making requests - **API base URL:** `https://app.codacy.com/api/v3` (configured in `src/index.ts` via `OpenAPI.BASE`) diff --git a/README.md b/README.md index cd3d5c5..13f90b8 100644 --- a/README.md +++ b/README.md @@ -77,16 +77,16 @@ Supported providers: GitHub (`gh`), GitLab (`gl`), Bitbucket (`bb`). | `logout` | Remove stored Codacy API token | | `info` | Show authenticated user info and their organizations | | `repositories ` | List repositories for an organization | -| `repository [provider] [org] [repo]` | Show metrics for a repository, or add/remove/follow/unfollow/reanalyze it | +| `repository [provider] [org] [repo]` | Show metrics for a repository, or add/remove/follow/unfollow/reanalyze it (optionally waiting for results) | | `issues [provider] [org] [repo]` | Search issues in a repository with filters | | `issue [provider] [org] [repo] ` | Show details for a single issue, or ignore/unignore it | | `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 | +| `pull-request [provider] [org] [repo] ` | Show PR analysis, issues, diff coverage, and changed files; or reanalyze it (optionally waiting for results) | | `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 | -| `pattern [provider] [org] [repo] ` | Enable, disable, or set parameters for a pattern | +| `pattern [provider] [org] [repo] ` | Show a pattern, or enable, disable, or set parameters for it | Run `codacy --help` for full argument and option details for any command. diff --git a/SPECS/README.md b/SPECS/README.md index f2dec8c..efb0490 100644 --- a/SPECS/README.md +++ b/SPECS/README.md @@ -23,7 +23,7 @@ _No pending tasks._ All commands implemented. | `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) | -| `pattern` | `pat` | ✅ Done | [tools-and-patterns.md](commands/tools-and-patterns.md) | +| `pattern` | `pat` | ✅ Done (info mode + guards added) | [tools-and-patterns.md](commands/tools-and-patterns.md) | | `analysis` | N/A | ✅ Done | [analysis.md](commands/analysis.md) | | `json-output` | N/A | ✅ Done | [json-output.md](commands/json-output.md) | | `login` | N/A | ✅ Done | — | @@ -65,3 +65,6 @@ _No pending tasks._ All commands implemented. | 2026-03-05 | JSON output filtering with `pickDeep` across all commands: `info`, `repositories`, `repository`, `pull-request`, `issues`, `issue`, `findings`, `finding`, `tools`, `patterns`; documented pattern in `src/commands/CLAUDE.md` | | 2026-03-12 | `patterns --enable-all` / `--disable-all` bulk update with filter support (6 new tests, 196 total) | | 2026-03-12 | `login` and `logout` commands: encrypted token storage in `~/.codacy/credentials`, masked interactive prompt, `--token` flag for non-interactive use, token resolution chain (env var → stored credentials); `checkApiToken()` updated to set `OpenAPI.HEADERS` dynamically (9 new tests, 219 total) | +| 2026-06-02 | `--reanalyze-and-wait` (`-w`) blocking variant for `repository` and `pull-request`: triggers reanalysis, polls to completion (10s interval, 20min cap), then prints issue deltas by pattern/severity/category. New `src/utils/reanalyze-wait.ts` + `formatDuration`/`isBeingAnalyzed` helpers (26 new tests, 356 total) | +| 2026-06-02 | `issues --overview` improvements: relabel False Positives buckets (`belowThreshold`/`equalOrAboveThreshold` → "Not a False Positive"/"Potential False Positive"), and a "Suggested actions to reduce noise" section that flags noisy patterns (≥10% of issues or ≥3× the average) with a runnable `codacy pattern … --disable` command, resolving the tool via its `prefix` (3 new tests, 360 total) | +| 2026-06-02 | Pattern config-file & coding-standard awareness: new `pattern ` **info mode** (same card as `patterns`); `pattern`/`patterns` skip listing and refuse updates when a tool uses a local config file; `pattern` refuses to modify coding-standard-enforced patterns; `issues --overview` noise suggestions now render a manual "update your config file / coding standard" step instead of a command when a pattern can't be disabled via CLI. `printPatternCard`/`PATTERN_JSON_FIELDS` moved to `utils/formatting.ts` (11 new tests, 371 total) | diff --git a/SPECS/commands/analysis.md b/SPECS/commands/analysis.md index 066db42..43040c8 100644 --- a/SPECS/commands/analysis.md +++ b/SPECS/commands/analysis.md @@ -16,6 +16,54 @@ codacy pull-request --reanalyz On success, show: "Reanalysis requested successfully, new results will be available in a few minutes." On failure, show: "Failed to request reanalysis: \". +### `--reanalyze-and-wait` (`-w`) — blocking variant + +A second variant triggers the reanalysis and **waits** for it to complete, then +prints what changed. The fire-and-forget `--reanalyze` stays as-is. + +``` +codacy repository --reanalyze-and-wait +codacy pull-request --reanalyze-and-wait +``` + +Flow: +1. Capture a **baseline** of current issues — for the repository from + `issuesOverview` (counts by severity / category / pattern as independent + lists); for the pull request by paging `listPullRequestIssues(status="new")` + (each issue carries its pattern's category + severity). +2. Trigger `reanalyzeCommitById` on the HEAD commit. +3. **Poll every 10 s** (giving up after **20 min**), with the spinner showing + `"Analysis requested. Waiting for it to start..."` → `"Analysis in progress. + This may take a few minutes..."` → `"Analysis done. Fetching results to + compare..."`. On each poll, read the **first commit** from the `/commits` + endpoint (`listRepositoryCommits` for the repo, `getPullRequestCommits` for + the PR, both `limit=1`) and look at its `startedAnalysis`/`endedAnalysis`: + - **In progress** = `startedAnalysis` more recent than `endedAnalysis` **and** + more recent than `t0` (the moment we triggered) — i.e. the analysis that + started is ours, not a previous one. + - **Done** = `startedAnalysis` more recent than `t0` **and** `endedAnalysis` + at or after `startedAnalysis` (our analysis has since finished). +4. Fetch a fresh snapshot, diff against the baseline, and print: + - `Analysis finished in ` headline (from the commit's + `startedAnalysis`→`endedAnalysis`, falling back to wall-clock). + - **By pattern / By severity / By category** signed net-delta lists. PR + pattern rows are annotated `(Category · Severity)`; repo pattern rows are + title-only (the overview doesn't map patterns to category/severity). + - `In total: → issues (net ±N)`. + +Notes: +- The overview only exposes **net** per-bucket change, so deltas are signed net + values per dimension, not a literal "added vs removed" split or a + severity×category cross-tab. +- The pattern list is soft-capped at 20 rows with a `… (N more)` line. +- `--output json` emits `{ durationMs, durationHuman, totals, deltas }`. +- On timeout, the spinner fails with a "didn't finish within 20 minutes" message. + +Shared logic lives in `src/utils/reanalyze-wait.ts` (`pollForAnalysis`, +`snapshotFromOverview`, `snapshotFromPrIssues`, `diffSnapshots`, +`renderReanalyzeReport`, `reanalyzeJson`, and a `timers.sleep` indirection that +tests stub for instant polling). `formatDuration` lives in `utils/formatting.ts`. + ## Updates to existing commands ### `repository` command — About section @@ -69,6 +117,13 @@ Implemented in `formatAnalysisStatus()` in `src/utils/formatting.ts`. - [`listRepositoryCommits`](https://api.codacy.com/api/api-docs#listrepositorycommits) with `limit=1` — head commit timing for repo - [`listCoverageReports`](https://api.codacy.com/api/api-docs#listcoveragereports) with `limit=1` — check `hasCoverageOverview` +Additionally used by `--reanalyze-and-wait`: +- `listRepositoryCommits` (`limit=1`) — repo first-commit analysis timestamps, polled for status +- `getPullRequestCommits` (`limit=1`) — PR first-commit analysis timestamps, polled for status +- `getRepositoryPullRequest` — fetched once to resolve the PR `headCommitSha` +- `issuesOverview` — repo baseline/after issue counts (severity / category / pattern) +- `listPullRequestIssues` (`status="new"`, paginated) — PR baseline/after issue list + ## Tasks - [x] Update analysis status in the About section of the `repository` command @@ -77,9 +132,11 @@ Implemented in `formatAnalysisStatus()` in `src/utils/formatting.ts`. - [x] Add `--reanalyze` option to the `pull-request` command - [x] Update existing tests for the status sections - [x] Add tests for the new `--reanalyze` option +- [x] Add `--reanalyze-and-wait` (`-w`) blocking variant to both commands (2026-06-02) ## Tests -- `src/utils/formatting.test.ts` — 6 unit tests for `formatAnalysisStatus` -- `src/commands/repository.test.ts` — 4 new tests (analysis status, reanalyze) -- `src/commands/pull-request.test.ts` — 3 new tests (analysis status, reanalyze) +- `src/utils/formatting.test.ts` — 6 unit tests for `formatAnalysisStatus`; + `formatDuration` and `isBeingAnalyzed` tests +- `src/commands/repository.test.ts` — 4 tests (analysis status, reanalyze) + 3 for `--reanalyze-and-wait` +- `src/commands/pull-request.test.ts` — 3 tests (analysis status, reanalyze) + 3 for `--reanalyze-and-wait` +- `src/utils/reanalyze-wait.test.ts` — 12 unit tests (snapshots, diff, poll loop incl. timeout, render, json) diff --git a/SPECS/commands/issues.md b/SPECS/commands/issues.md index cd82977..458aafe 100644 --- a/SPECS/commands/issues.md +++ b/SPECS/commands/issues.md @@ -19,8 +19,11 @@ codacy is gh my-org my-repo --output json - [`searchRepositoryIssues`](https://api.codacy.com/api/api-docs#searchrepositoryissues) — `AnalysisService.searchRepositoryIssues(provider, org, repo, cursor, limit, body)` - [`issuesOverview`](https://api.codacy.com/api/api-docs#issuesoverview) — `AnalysisService.issuesOverview(provider, org, repo, body)` (only when `--overview` is given) +- [`listTools`](https://api.codacy.com/api/api-docs#listtools) — `ToolsService.listTools(cursor, limit)` (only when `--overview` surfaces noisy patterns, to map each pattern's `prefix` to its owning tool) +- [`listRepositoryTools`](https://api.codacy.com/api/api-docs#listrepositorytools) — `AnalysisService.listRepositoryTools(provider, org, repo)` (only when `--overview` surfaces noisy patterns, to detect config-file-driven tools) +- [`listRepositoryToolPatterns`](https://api.codacy.com/api/api-docs#listrepositorytoolpatterns) — `search=` (only for noisy patterns on non-config-file tools, to detect coding-standard enforcement) -Both accept the same `SearchRepositoryIssuesBody` for filtering. +`searchRepositoryIssues` and `issuesOverview` accept the same `SearchRepositoryIssuesBody` for filtering. ## Options @@ -66,8 +69,46 @@ Shows pagination warning if more results exist. ### Overview mode (`--overview`) -Six count tables sorted descending by count: Category, Severity, Language, Tag, Pattern, Author. +Seven count tables sorted descending by count: Category, Severity, Language, Tag, +Pattern, Author, and False Positives. + +The **False Positives** table relabels the API's raw bucket names for readability: +`belowThreshold` → "Not a False Positive", `equalOrAboveThreshold` → "Potential +False Positive" (the bucket is keyed on FP probability vs. the configured +threshold, so at/above threshold = a potential false positive). + +After the tables, a **"Suggested actions to reduce noise"** section lists patterns +worth disabling. A pattern is "noisy" when it accounts for **≥10% of all issues** +shown, **or** has **≥3× the average** issues-per-pattern. The owning tool is +resolved by matching the pattern ID against each tool's `prefix` (longest match +wins); patterns whose tool can't be resolved (no/unknown prefix) are dropped +silently. The list is capped at 10 with a "… (N more)" note. + +The suggested step depends on **how the pattern is managed**, since not every +pattern can be disabled through the CLI: + +``` +Suggested actions to reduce noise + + Disable "Use of assert detected" (-2.5k issues) + > codacy pattern Bandit Bandit_B101 --disable +``` + +- **Default** — a runnable `> codacy pattern --disable` command. +- **Tool uses a local configuration file** — no command; instead + `→ Update your local configuration file to disable the pattern`. +- **Pattern enforced by a coding standard** — no command; instead + `→ Update to disable the pattern`. + +To classify each noisy pattern, the command additionally fetches the repository +tools (`listRepositoryTools`, for `usesConfigurationFile` and the repo tool +UUID) and, for non-config-file tools, the pattern's `enabledBy` via +`listRepositoryToolPatterns` (`search=`, one call per noisy pattern). +A config file takes precedence over coding-standard enforcement. These extra +calls only run when at least one noisy pattern exists. + +`--output json` is unaffected (raw counts only — no relabeling or suggestions). ## Tests -File: `src/commands/issues.test.ts` — 39 tests. +File: `src/commands/issues.test.ts` — 46 tests. diff --git a/SPECS/commands/tools-and-patterns.md b/SPECS/commands/tools-and-patterns.md index 701e978..bfb8161 100644 --- a/SPECS/commands/tools-and-patterns.md +++ b/SPECS/commands/tools-and-patterns.md @@ -92,6 +92,18 @@ File: `src/commands/tool.test.ts` — 9 tests. List patterns for a specific tool in a repository, with optional filters. Also supports bulk enabling/disabling matching patterns. +## Configuration file guard + +Before listing or bulk-updating, the tool's `settings.usesConfigurationFile` is +checked. When the tool is driven by a local configuration file its patterns are +overwritten and the API can't list or change them: + +- **List mode**: prints ` is using a local configuration file.` and skips + the pattern fetch (exit 0). With `--output json` it emits + `{ "tool": "", "usesConfigurationFile": true, "patterns": [] }`. +- **Bulk mode** (`--enable-all` / `--disable-all`): fails with `Tool uses a + local configuration file, can't be updated.` and exits 1. + ## Usage ``` @@ -157,7 +169,7 @@ After the update, fetches the tool patterns overview and shows a summary: ## Tests -File: `src/commands/patterns.test.ts` — 23 tests. +File: `src/commands/patterns.test.ts` — 27 tests. --- @@ -165,11 +177,12 @@ File: `src/commands/patterns.test.ts` — 23 tests. ## Purpose -Enable, disable, or set parameters for a specific pattern. +Show a single pattern's information, or enable, disable, or set parameters for it. ## Usage ``` +codacy pattern # show info codacy pattern --enable codacy pattern --disable codacy pattern --parameter maxParams=3 @@ -183,16 +196,41 @@ codacy pattern --p | `--disable` | `-d` | Disable the pattern | | `--parameter ` | `-p` | Set a parameter (repeatable) | +## Modes + +- **Info mode** (default — no action flag): renders the pattern using the same + card format as the `patterns` command (`printPatternCard`). Supports + `--output json` (single object via `PATTERN_JSON_FIELDS`). +- **Modify mode** (`--enable` / `--disable` / `--parameter`): updates the pattern. + +There is **no endpoint to fetch a single pattern at repository level**, so both +modes call `listRepositoryToolPatterns` with `search=` and keep only +the **exact** ID match (the search is fuzzy and can return near-matches). + +## Guards (modify mode) + +Before changing a pattern, two conditions are checked. Either one prints the +reason and exits 1 without calling `configureTool`: + +- **Local configuration file** (`tool.settings.usesConfigurationFile`): `Tool + uses a local configuration file, can't be updated.` (In info mode this prints + ` is using a local configuration file.` and exits 0 — patterns aren't + fetched.) +- **Coding-standard enforcement** (the matched pattern's `enabledBy` is + non-empty): `Pattern enforced by coding standard(s), can't be + modified.` + ## API Endpoints -1. [`listRepositoryTools`](https://api.codacy.com/api/api-docs#listrepositorytools) — to resolve tool name to UUID -2. [`listRepositoryToolPatterns`](https://api.codacy.com/api/api-docs#listrepositorytoolpatterns) — only when neither `--enable` nor `--disable` is set, to fetch current enabled state -3. [`configureTool`](https://api.codacy.com/api/api-docs#configuretool) — with a single-pattern `patterns` array in the body +1. [`listRepositoryTools`](https://api.codacy.com/api/api-docs#listrepositorytools) — to resolve tool name to UUID (and read `usesConfigurationFile`) +2. [`listRepositoryToolPatterns`](https://api.codacy.com/api/api-docs#listrepositorytoolpatterns) — `search=`, to fetch the pattern's current state and `enabledBy` (skipped only when the tool uses a configuration file) +3. [`configureTool`](https://api.codacy.com/api/api-docs#configuretool) — with a single-pattern `patterns` array in the body (modify mode only) ## Output -On success: confirmation message per action taken. On failure: error from API. +Info mode: the pattern card (or JSON). Modify mode: confirmation message per +action taken. On failure or refusal: a reason message. ## Tests -File: `src/commands/pattern.test.ts` — 8 tests. +File: `src/commands/pattern.test.ts` — 15 tests. diff --git a/src/commands/AGENTS.md b/src/commands/AGENTS.md index 5b13875..bd876c9 100644 --- a/src/commands/AGENTS.md +++ b/src/commands/AGENTS.md @@ -106,6 +106,8 @@ Instead of a dedicated "Visibility" column (wastes horizontal space), public rep - **Issues Overview**: three count tables — by category, severity level, and language — sorted descending by count within each group - Shows pagination warning for pull requests if more exist - JSON output bundles all three API responses into a single object +- **`--reanalyze` mode** (`-R`): fetches HEAD commit SHA, calls `RepositoryService.reanalyzeCommitById`; early return +- **`--reanalyze-and-wait` mode** (`-w`): blocking variant — see "Reanalyze and wait" below. Baseline comes from `issuesOverview`; polling reads the repo's first commit via `listRepositoryCommits(limit=1)` analysis timestamps ## Shared Formatting Utilities (`utils/formatting.ts`) @@ -120,6 +122,61 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - `formatPrCoverage(pr, passing)` — diffCoverage% (+/-deltaCoverage%) - `formatPrIssues(pr, passing)` — +newIssues (colored by gate) / -fixedIssues (always gray) +Pattern helpers shared between `patterns.ts` (list) and `pattern.ts` (single info): +- `printPatternCard(cp)` — the configured-pattern card (icons, enforced-by line, metadata, why/how, parameters) +- `PATTERN_JSON_FIELDS` — `pickDeep` paths for the JSON projection of a `ConfiguredPattern` +- `patternEnforcedBy(cp)` — coding-standard names from `cp.enabledBy` (empty = not enforced) + +## pattern / patterns commands — config-file & coding-standard guards + +Patterns can be unmodifiable for two reasons, surfaced consistently across +`pattern`, `patterns`, and the `issues --overview` noise suggestions: + +- **Local configuration file** — `tool.settings.usesConfigurationFile` (from + `listRepositoryTools`). The tool's patterns are overwritten by the file, so + the API can't list or change them. +- **Coding-standard enforcement** — a `ConfiguredPattern.enabledBy` array with + entries. The pattern is managed in the standard, not at repo level. + +There is no single-pattern endpoint, so a pattern is looked up via +`listRepositoryToolPatterns(search=)` filtered to the **exact** ID +match. Modify-mode refusals (`pattern --enable/--disable/--parameter`, and +`patterns --enable-all/--disable-all`) print the reason and `process.exit(1)`; +info/list displays print a notice and exit 0. + +## Reanalyze and wait (`utils/reanalyze-wait.ts`) + +Shared by the `repository` and `pull-request` `--reanalyze-and-wait` (`-w`) modes. +Keeps the two command handlers thin: they only supply the API-specific callbacks +(get HEAD SHA, fetch baseline/after snapshot, `getStatus`). + +- **Snapshots**: `IssueSnapshot { total, bySeverity, byCategory, byPattern }`. + `snapshotFromOverview()` (repo) builds independent dimension counts and pattern + buckets with **no** category/severity. `snapshotFromPrIssues()` (PR) tallies the + same dimensions from the raw issue list and **annotates** each pattern bucket + with its category + severity. +- **`diffSnapshots(before, after)`**: signed **net** deltas per dimension (drops + unchanged buckets). Severity sorted Critical→Minor; category/pattern sorted by + |delta|. The overview can't decompose net change into added-vs-removed, so the + report shows net deltas, not a literal "new/resolved" split or a cross-tab. +- **`pollForAnalysis(getStatus, opts)`**: two-phase loop — wait for a new analysis + to start (or detect it already finished), then wait for it to finish. `getStatus` + returns the first commit's `{ startedAnalysis, endedAnalysis }` (from the `/commits` + endpoint). In-progress = `startedAnalysis` more recent than both `endedAnalysis` + and `triggeredAt` (t0); done = `startedAnalysis` after t0 and `endedAnalysis` ≥ + `startedAnalysis` (helpers `isAnalysisInProgress`/`isAnalysisDone`). Comparing to + t0 ensures we track *our* analysis, not a stale one. Returns `{ status, timedOut }`. + Defaults: `POLL_INTERVAL_MS=10s`, `MAX_WAIT_MS=20min`. +- **`timers.sleep`**: polling delay wrapped in an exported object so tests stub it + (`vi.spyOn(timers, "sleep").mockResolvedValue()`) for instant polling. Drive the + poll loop in command tests with sequential `mockResolvedValueOnce` status values; + use the optional `now` injection in `pollForAnalysis` to unit-test the timeout. +- **Rendering**: `renderReanalyzeReport(delta, durationMs)` prints the + `Analysis finished in ` headline + By pattern / By severity / + By category lists (pattern rows soft-capped at `PATTERN_LIMIT=20`) + an + `In total: before → after (net ±N)` line. `reanalyzeJson()` is the `--output json` + payload. `durationFromStatus()` derives the duration from commit timestamps. + ## issue command (`issue.ts`) - Takes ``, ``, ``, and `` (the numeric `resultDataId` shown on issue cards) as required arguments @@ -135,7 +192,9 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - Takes ``, ``, and `` as required arguments - **List mode** (default): card-style format sorted by severity (Error > High > Warning > Info) -- **Overview mode** (`-O, --overview`): six count tables — Category, Severity, Language, Tag, Pattern, Author +- **Overview mode** (`-O, --overview`): seven count tables — Category, Severity, Language, Tag, Pattern, Author, False Positives + - The False Positives table relabels the API's raw bucket names via `FALSE_POSITIVE_LABELS`: `belowThreshold` → "Not a False Positive", `equalOrAboveThreshold` → "Potential False Positive" (the threshold is on FP probability, so at/above = potential FP — matching `printIssueCard`) + - **Noise suggestions**: after the tables, `detectNoisyPatterns()` flags patterns that account for ≥10% of all issues (`NOISE_SHARE`) **or** have ≥3× the average issues-per-pattern (`NOISE_AVG_MULTIPLE`), sorted by count desc. For each, `resolvePatternTool()` maps the pattern ID to its owning tool by matching `Tool.prefix` (e.g. `Bandit_B101` → prefix `Bandit_`; longest match wins) against the global tool list (`fetchAllTools()` / `ToolsService.listTools`). Resolved ones print a `> codacy pattern --disable` line under "Suggested actions to reduce noise". Patterns whose tool can't be resolved (no/unmatched prefix) are **silently discarded**. The tools fetch only happens when noisy patterns exist; JSON output is unaffected (raw counts only) - **Filters**: `--branch`, `--patterns`, `--tools`, `--severities`, `--categories`, `--languages`, `--tags`, `--authors`, `--limit` - **`--false-positives [value]`** (`-F`): tri-state filter — `true` (default when flag present) sends `onlyPotentialFalsePositives: true`, `false` sends `onlyPotentialFalsePositives: false`, omitted sends nothing - **`--ignore` mode** (`-I`): fetches all issues matching current filters (all pages), then calls `AnalysisService.bulkIgnoreIssues` in batches of 100 @@ -185,6 +244,7 @@ Several helpers are shared between `repository.ts` and `pull-request.ts` via `ut - **`--ignore-all-false-positives` mode** (`-F`): fetches all potential false positive issues (onlyPotential=true, paginated), ignores them all in parallel with hardcoded reason `FalsePositive`; supports `-m/--ignore-comment` - **`--unignore-issue ` mode** (`-U`): same lookup as `--ignore-issue`, calls `updateIssueState` with `{ ignored: false }` - **`--reanalyze` mode** (`-A`): fetches PR data to get `headCommitSha`, calls `RepositoryService.reanalyzeCommitById`; early return +- **`--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 ## JSON Output Filtering (`pickDeep`) diff --git a/src/commands/issues.test.ts b/src/commands/issues.test.ts index cdd096a..181f507 100644 --- a/src/commands/issues.test.ts +++ b/src/commands/issues.test.ts @@ -101,6 +101,13 @@ describe("issues command", () => { beforeEach(() => { vi.clearAllMocks(); process.env.CODACY_API_TOKEN = "test-token"; + // Default: no tools. Overrides per-test when a suggestion needs resolving. + vi.mocked(ToolsService.listTools).mockResolvedValue({ data: [] } as any); + // The overview noise path also reads repo tools; default to none so tests + // that don't exercise suggestions don't need to mock it. + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue({ + data: [], + } as any); }); it("should fetch and display issues in card format", async () => { @@ -194,8 +201,211 @@ describe("issues command", () => { expect(output).toContain("Author"); expect(output).toContain("dev@example.com"); expect(output).toContain("False Positives"); - expect(output).toContain("equalOrAboveThreshold"); - expect(output).toContain("belowThreshold"); + // Raw API bucket names are relabeled to human-friendly terms. + expect(output).toContain("Potential False Positive"); + expect(output).toContain("Not a False Positive"); + expect(output).not.toContain("equalOrAboveThreshold"); + expect(output).not.toContain("belowThreshold"); + }); + + describe("overview noise suggestions", () => { + // One dominant pattern plus nine small ones: total 2950, avg 295. + // Bandit_B101 (2500) is both >=10% of total and >=3x average → noisy. + function noisyOverview() { + const patterns = [ + { id: "Bandit_B101", title: "Use of assert detected", total: 2500 }, + ]; + for (let i = 0; i < 9; i++) { + patterns.push({ id: `Other_${i}`, title: `Pattern ${i}`, total: 50 }); + } + return { + data: { + counts: { + categories: [{ name: "Security", total: 2950 }], + levels: [{ name: "Warning", total: 2950 }], + languages: [], + tags: [], + patterns, + authors: [], + potentialFalsePositives: [], + }, + }, + }; + } + + const banditTool = { + data: [ + { uuid: "uuid-bandit", name: "Bandit", shortName: "bandit", prefix: "Bandit_" }, + ], + pagination: undefined, + }; + + // Repo-scoped Bandit tool, matched to the global one by UUID. Helpers build + // variants for the config-file and coding-standard cases. + function banditRepoTool(overrides: Record = {}) { + return { + data: [ + { + uuid: "uuid-bandit", + name: "Bandit", + isClientSide: false, + settings: { + isEnabled: true, + followsStandard: false, + isCustom: false, + hasConfigurationFile: false, + usesConfigurationFile: false, + enabledBy: [], + ...overrides, + }, + }, + ], + pagination: undefined, + }; + } + + // A configured pattern for Bandit_B101 with the given enforcing standards. + function banditPattern(enabledBy: Array<{ id: number; name: string }> = []) { + return { + data: [ + { + patternDefinition: { id: "Bandit_B101", title: "Use of assert detected" }, + enabled: true, + parameters: [], + enabledBy, + }, + ], + pagination: undefined, + }; + } + + it("suggests disabling a noisy pattern with a runnable command", async () => { + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + noisyOverview() as any, + ); + vi.mocked(ToolsService.listTools).mockResolvedValue(banditTool as any); + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue( + banditRepoTool() as any, + ); + vi.mocked(AnalysisService.listRepositoryToolPatterns).mockResolvedValue( + banditPattern() as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", "--overview", + ]); + + // Strip ANSI codes so bold/color don't break substring matches. + const output = getAllOutput().replace(/\x1b\[[0-9;]*m/g, ""); + expect(output).toContain("Suggested actions to reduce noise"); + expect(output).toContain('Disable "Use of assert detected"'); + expect(output).toContain("(-2.5k issues)"); + expect(output).toContain("codacy pattern Bandit Bandit_B101 --disable"); + // The small patterns are listed in the table but never suggested. + expect(output).not.toContain('Disable "Pattern 0"'); + }); + + it("suggests updating the config file when the tool uses one", async () => { + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + noisyOverview() as any, + ); + vi.mocked(ToolsService.listTools).mockResolvedValue(banditTool as any); + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue( + banditRepoTool({ usesConfigurationFile: true }) as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", "--overview", + ]); + + const output = getAllOutput().replace(/\x1b\[[0-9;]*m/g, ""); + expect(output).toContain("Suggested actions to reduce noise"); + expect(output).toContain( + "Update your local Bandit configuration file to disable the pattern", + ); + // No runnable command, and no need to look up the pattern's enforcement. + expect(output).not.toContain("codacy pattern"); + expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + }); + + it("suggests updating the coding standard when the pattern is enforced", async () => { + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + noisyOverview() as any, + ); + vi.mocked(ToolsService.listTools).mockResolvedValue(banditTool as any); + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue( + banditRepoTool() as any, + ); + vi.mocked(AnalysisService.listRepositoryToolPatterns).mockResolvedValue( + banditPattern([{ id: 1, name: "OWASP Top 10" }]) as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", "--overview", + ]); + + const output = getAllOutput().replace(/\x1b\[[0-9;]*m/g, ""); + expect(output).toContain("Update OWASP Top 10 to disable the pattern"); + expect(output).not.toContain("codacy pattern"); + }); + + it("discards suggestions whose owning tool can't be resolved", async () => { + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + noisyOverview() as any, + ); + // No tool prefix matches "Bandit_" → suggestion silently dropped. + vi.mocked(ToolsService.listTools).mockResolvedValue({ + data: [{ uuid: "u", name: "ESLint", shortName: "eslint", prefix: "ESLint_" }], + pagination: undefined, + } as any); + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue({ + data: [], + pagination: undefined, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", "--overview", + ]); + + const output = getAllOutput(); + expect(output).not.toContain("Suggested actions to reduce noise"); + }); + + it("shows no suggestions and skips the tools fetch when nothing is noisy", async () => { + // Twelve evenly-sized patterns: each is ~8.3% of total and equal to the + // average, so none crosses the >=10% or >=3x-average thresholds. + const patterns = Array.from({ length: 12 }, (_, i) => ({ + id: `Tool_${i}`, + title: `Pattern ${i}`, + total: 100, + })); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue({ + data: { + counts: { + categories: [], + levels: [{ name: "Warning", total: 1200 }], + languages: [], + tags: [], + patterns, + authors: [], + potentialFalsePositives: [], + }, + }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "issues", "gh", "test-org", "test-repo", "--overview", + ]); + + const output = getAllOutput(); + expect(output).not.toContain("Suggested actions to reduce noise"); + expect(ToolsService.listTools).not.toHaveBeenCalled(); + }); }); it("should pass filter options to the API body", async () => { diff --git a/src/commands/issues.ts b/src/commands/issues.ts index f592fe6..6911d00 100644 --- a/src/commands/issues.ts +++ b/src/commands/issues.ts @@ -15,10 +15,12 @@ import { printSection, printIssueCard, resolveToolUuids, + formatCount, } from "../utils/formatting"; import { AnalysisService } from "../api/client/services/AnalysisService"; import { ToolsService } from "../api/client/services/ToolsService"; import { Tool } from "../api/client/models/Tool"; +import { AnalysisTool } from "../api/client/models/AnalysisTool"; import { CommitIssue } from "../api/client/models/CommitIssue"; import { SeverityLevel } from "../api/client/models/SeverityLevel"; import { SearchRepositoryIssuesBody } from "../api/client/models/SearchRepositoryIssuesBody"; @@ -28,6 +30,22 @@ import { PatternsCount } from "../api/client/models/PatternsCount"; // API allows a maximum of 100 issue IDs per bulk-ignore call const BULK_BATCH_SIZE = 100; +// A pattern is flagged as "noisy" (and suggested for disabling) when it dominates +// the issue set: either it alone accounts for at least NOISE_SHARE of all issues, +// or it has at least NOISE_AVG_MULTIPLE times the average issues-per-pattern. +const NOISE_SHARE = 0.1; +const NOISE_AVG_MULTIPLE = 3; +// Cap how many disable suggestions we print, to keep the section actionable. +const MAX_NOISE_SUGGESTIONS = 10; + +// Human-friendly labels for the API's false-positive threshold buckets. +// `equalOrAboveThreshold` = FP probability >= threshold (a potential false +// positive); `belowThreshold` = below threshold (treated as a real issue). +const FALSE_POSITIVE_LABELS: Record = { + belowThreshold: "Not a False Positive", + equalOrAboveThreshold: "Potential False Positive", +}; + const SEVERITY_ORDER: Record = { Error: 0, High: 1, @@ -162,7 +180,212 @@ function printOverview(counts: { printCountTable("Author", counts.authors); if (counts.authors.length > 0 && counts.potentialFalsePositives.length > 0) console.log(); - printCountTable("False Positives", counts.potentialFalsePositives); + printCountTable( + "False Positives", + relabelFalsePositives(counts.potentialFalsePositives), + ); +} + +/** Map the API's threshold-bucket names to human-friendly false-positive labels. */ +function relabelFalsePositives(counts: Count[]): Count[] { + return counts.map((c) => ({ + ...c, + name: FALSE_POSITIVE_LABELS[c.name] ?? c.name, + })); +} + +/** + * Identify "noisy" patterns worth suggesting for disabling: a pattern is noisy + * when it accounts for at least NOISE_SHARE of all issues, or has at least + * NOISE_AVG_MULTIPLE times the average issues-per-pattern. Sorted by count desc. + */ +function detectNoisyPatterns(patterns: PatternsCount[]): PatternsCount[] { + if (patterns.length === 0) return []; + const total = patterns.reduce((sum, p) => sum + p.total, 0); + if (total === 0) return []; + const average = total / patterns.length; + const shareFloor = NOISE_SHARE * total; + const avgFloor = NOISE_AVG_MULTIPLE * average; + return patterns + .filter((p) => p.total >= shareFloor || p.total >= avgFloor) + .sort((a, b) => b.total - a.total); +} + +/** + * Find the tool that owns a pattern by matching the pattern ID prefix against + * each tool's `prefix` (the field Codacy uses to keep pattern names unique). + * Longest matching prefix wins. Tools without a prefix can't be matched. + */ +function resolvePatternTool( + patternId: string, + tools: Tool[], +): Tool | undefined { + let best: Tool | undefined; + let bestLen = 0; + for (const tool of tools) { + if (!tool.prefix) continue; + // The API prefix normally already carries the trailing underscore (e.g. + // "ESLint_"), but normalize to "_" so we match either way. + const marker = tool.prefix.endsWith("_") ? tool.prefix : `${tool.prefix}_`; + if (patternId.startsWith(marker) && marker.length > bestLen) { + best = tool; + bestLen = marker.length; + } + } + return best; +} + +interface NoiseSuggestion { + title: string; + total: number; + // Exactly one of these is set: `command` is a runnable `codacy pattern … + // --disable`; `action` is a manual step shown when the pattern can't be + // disabled through the CLI (config-file-driven tool, or coding-standard + // enforced). + command?: string; + action?: string; +} + +/** Match a global tool to its repository-scoped tool by UUID, then by name. */ +function findRepoTool( + repoTools: AnalysisTool[], + tool: Tool, +): AnalysisTool | undefined { + return ( + repoTools.find((t) => t.uuid === tool.uuid) ?? + repoTools.find((t) => t.name.toLowerCase() === tool.name.toLowerCase()) + ); +} + +/** + * Fetch the coding standards (if any) that enforce a pattern, by searching the + * repo tool's patterns for an exact ID match and reading its `enabledBy`. + */ +async function fetchPatternStandards( + ctx: { provider: string; organization: string; repository: string }, + toolUuid: string, + patternId: string, +): Promise { + const resp = await AnalysisService.listRepositoryToolPatterns( + ctx.provider, + ctx.organization, + ctx.repository, + toolUuid, + undefined, // languages + undefined, // categories + undefined, // severityLevels + undefined, // tags + patternId, // search by ID + ); + const match = resp.data.find((cp) => cp.patternDefinition.id === patternId); + return match?.enabledBy?.map((s) => s.name) ?? []; +} + +/** + * Build suggestions for noisy patterns. The owning tool is resolved by prefix; + * patterns whose tool can't be resolved (no/unknown prefix) are silently + * discarded. The suggested step depends on how the pattern is managed: + * - tool driven by a local config file → manual "update your config file" step + * - pattern enforced by a coding standard → manual "update the standard" step + * - otherwise → a runnable `codacy pattern … --disable` command + */ +interface NoiseSuggestionsResult { + suggestions: NoiseSuggestion[]; + /** Resolvable noisy patterns beyond MAX_NOISE_SUGGESTIONS, not detailed. */ + remaining: number; +} + +async function buildNoiseSuggestions( + noisy: PatternsCount[], + globalTools: Tool[], + repoTools: AnalysisTool[], + ctx: { provider: string; organization: string; repository: string }, +): Promise { + // Resolve owning tools synchronously and drop patterns we can't identify, then + // cap to MAX_NOISE_SUGGESTIONS *before* any network calls — only the patterns + // we actually display need their config-file / coding-standard status checked. + const resolved = noisy + .map((pattern) => ({ + pattern, + tool: resolvePatternTool(pattern.id, globalTools), + })) + .filter( + (r): r is { pattern: PatternsCount; tool: Tool } => r.tool !== undefined, + ); + + const candidates = resolved.slice(0, MAX_NOISE_SUGGESTIONS); + + const suggestions = await Promise.all( + candidates.map(async ({ pattern, tool }): Promise => { + // The `pattern` command matches the tool by name; hyphenate spaces per its convention. + const toolToken = tool.name.replace(/\s+/g, "-"); + const repoTool = findRepoTool(repoTools, tool); + + // A local configuration file overrides Codacy-side pattern config, so the + // pattern must be disabled in that file rather than via the CLI. + if (repoTool?.settings.usesConfigurationFile) { + return { + title: pattern.title, + total: pattern.total, + action: `Update your local ${tool.name} configuration file to disable the pattern`, + }; + } + + // A pattern enforced by a coding standard must be changed in the standard. + if (repoTool) { + const standards = await fetchPatternStandards( + ctx, + repoTool.uuid, + pattern.id, + ); + if (standards.length > 0) { + return { + title: pattern.title, + total: pattern.total, + action: `Update ${standards.join(", ")} to disable the pattern`, + }; + } + } + + return { + title: pattern.title, + total: pattern.total, + command: `codacy pattern ${toolToken} ${pattern.id} --disable`, + }; + }), + ); + + return { suggestions, remaining: resolved.length - candidates.length }; +} + +/** Print the "Suggested actions to reduce noise" section. No-op when empty. */ +function printNoiseSuggestions( + suggestions: NoiseSuggestion[], + remaining: number, +): void { + if (suggestions.length === 0) return; + + console.log(ansis.bold("\nSuggested actions to reduce noise\n")); + + for (const s of suggestions) { + const label = s.total === 1 ? "issue" : "issues"; + const reduction = ansis.green(`(-${formatCount(s.total)} ${label})`); + console.log(` Disable ${ansis.bold(`"${s.title}"`)} ${reduction}`); + if (s.command) { + console.log(` ${ansis.dim(">")} ${s.command}`); + } else if (s.action) { + console.log(` ${ansis.dim("→")} ${s.action}`); + } + console.log(); + } + + if (remaining > 0) { + console.log( + ansis.dim( + ` … (${remaining} more noisy pattern${remaining === 1 ? "" : "s"})`, + ), + ); + } } /** @@ -414,11 +637,11 @@ Examples: repository, body, ); - spinner.stop(); const counts = overviewResponse.data.counts; if (format === "json") { + spinner.stop(); printJson( pickDeep({ overview: counts }, [ "overview.categories", @@ -433,6 +656,36 @@ Examples: return; } + // Resolve disable suggestions for noisy patterns. The extra tools fetch + // only happens when there's something to suggest, and stays under the + // spinner so the user sees progress rather than a stall. + const noisy = detectNoisyPatterns(counts.patterns); + let suggestions: NoiseSuggestion[] = []; + let moreNoisy = 0; + if (noisy.length > 0) { + spinner.text = "Checking for noisy patterns..."; + // Global tools give each pattern's owning tool (via prefix); repo + // tools tell us which ones are config-file-driven and let us check + // coding-standard enforcement per pattern. + const [globalTools, repoToolsResponse] = await Promise.all([ + fetchAllTools(), + AnalysisService.listRepositoryTools( + provider, + organization, + repository, + ), + ]); + const built = await buildNoiseSuggestions( + noisy, + globalTools, + repoToolsResponse.data, + { provider, organization, repository }, + ); + suggestions = built.suggestions; + moreNoisy = built.remaining; + } + spinner.stop(); + printOverview({ categories: counts.categories, levels: counts.levels, @@ -442,6 +695,7 @@ Examples: authors: counts.authors, potentialFalsePositives: counts.potentialFalsePositives, }); + printNoiseSuggestions(suggestions, moreNoisy); } else { const pageSize = Math.min(limit, 100); let issues: CommitIssue[] = []; diff --git a/src/commands/pattern.test.ts b/src/commands/pattern.test.ts index bc507c2..779a3a5 100644 --- a/src/commands/pattern.test.ts +++ b/src/commands/pattern.test.ts @@ -48,8 +48,26 @@ const mockConfiguredPattern = { }, enabled: false, // currently disabled parameters: [], + enabledBy: [], }; +// A tool whose patterns are driven by a local configuration file. +const mockConfigFileTools = [ + { + uuid: "uuid-eslint", + name: "ESLint", + isClientSide: false, + settings: { + isEnabled: true, + followsStandard: false, + isCustom: false, + hasConfigurationFile: true, + usesConfigurationFile: true, + enabledBy: [], + }, + }, +]; + function getAllOutput(): string { return (console.log as ReturnType).mock.calls .map((c) => c[0]) @@ -100,8 +118,18 @@ describe("pattern command", () => { }, ); - // Should NOT fetch patterns when enable is specified - expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + // Patterns are fetched to check coding-standard enforcement before updating + expect(AnalysisService.listRepositoryToolPatterns).toHaveBeenCalledWith( + "gh", + "test-org", + "test-repo", + "uuid-eslint", + undefined, + undefined, + undefined, + undefined, + "no-unused-vars", + ); const output = getAllOutput(); expect(output).toContain("enabled"); @@ -227,11 +255,69 @@ describe("pattern command", () => { }, ); - // Should NOT fetch patterns when enable is specified - expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + // Patterns are fetched to check coding-standard enforcement before updating + expect(AnalysisService.listRepositoryToolPatterns).toHaveBeenCalled(); + }); + + it("should show pattern info when no action flag is specified", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + ]); + + // Searches by ID, renders the card, and does not modify anything + expect(AnalysisService.listRepositoryToolPatterns).toHaveBeenCalledWith( + "gh", + "test-org", + "test-repo", + "uuid-eslint", + undefined, + undefined, + undefined, + undefined, + "no-unused-vars", + ); + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + + const output = getAllOutput(); + expect(output).toContain("No Unused Variables"); + expect(output).toContain("no-unused-vars"); + }); + + it("should output pattern info as JSON when --output json is set", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "--output", + "json", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + ]); + + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + const output = getAllOutput(); + const parsed = JSON.parse(output); + expect(parsed.patternDefinition.id).toBe("no-unused-vars"); }); - it("should exit with error when no option is specified", async () => { + it("should exit when pattern not found in info mode", async () => { + vi.mocked(AnalysisService.listRepositoryToolPatterns).mockResolvedValue({ + data: [], + pagination: undefined, + } as any); + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { throw new Error("process.exit called"); }); @@ -246,7 +332,7 @@ describe("pattern command", () => { "test-org", "test-repo", "eslint", - "no-unused-vars", + "nonexistent-pattern", ]), ).rejects.toThrow("process.exit called"); @@ -333,6 +419,116 @@ describe("pattern command", () => { mockExit.mockRestore(); }); + describe("configuration file guard", () => { + beforeEach(() => { + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue({ + data: mockConfigFileTools, + pagination: undefined, + } as any); + }); + + it("shows a notice and skips fetching patterns in info mode", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + ]); + + const output = getAllOutput(); + expect(output).toContain("ESLint is using a local configuration file."); + expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + }); + + it("refuses to modify and exits 1", async () => { + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", + "test", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + "--disable", + ]), + ).rejects.toThrow("process.exit called"); + + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + mockExit.mockRestore(); + }); + }); + + describe("coding standard enforcement guard", () => { + beforeEach(() => { + vi.mocked(AnalysisService.listRepositoryToolPatterns).mockResolvedValue({ + data: [ + { + ...mockConfiguredPattern, + enabled: true, + enabledBy: [{ id: 1, name: "OWASP Top 10" }], + }, + ], + pagination: undefined, + } as any); + }); + + it("refuses to modify an enforced pattern and exits 1", async () => { + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", + "test", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + "--disable", + ]), + ).rejects.toThrow("process.exit called"); + + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + mockExit.mockRestore(); + }); + + it("still shows an enforced pattern in info mode", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "pattern", + "gh", + "test-org", + "test-repo", + "eslint", + "no-unused-vars", + ]); + + const output = getAllOutput(); + expect(output).toContain("Enforced by: OWASP Top 10"); + expect(AnalysisService.configureTool).not.toHaveBeenCalled(); + }); + }); + describe("auto-detect from git remote", () => { it("should auto-detect provider/org/repo when only toolName and patternId are provided", async () => { const program = createProgram(); diff --git a/src/commands/pattern.ts b/src/commands/pattern.ts index 557801f..870c680 100644 --- a/src/commands/pattern.ts +++ b/src/commands/pattern.ts @@ -4,8 +4,16 @@ import ansis from "ansis"; import { checkApiToken } from "../utils/auth"; import { handleError } from "../utils/error"; import { resolveRepoArgs } from "../utils/resolve-repo-args"; +import { getOutputFormat, pickDeep, printJson } from "../utils/output"; import { AnalysisService } from "../api/client/services/AnalysisService"; -import { findToolByName } from "../utils/formatting"; +import { + findToolByName, + printPatternCard, + patternEnforcedBy, + configFileNotice, + CONFIG_FILE_LOCKED_MESSAGE, + PATTERN_JSON_FIELDS, +} from "../utils/formatting"; import { ConfigureToolBody } from "../api/client/models/ConfigureToolBody"; import { ConfigurePattern } from "../api/client/models/ConfigurePattern"; @@ -13,7 +21,9 @@ export function registerPatternCommand(program: Command) { program .command("pattern") .alias("pat") - .description("Enable, disable, or set parameters for a specific pattern") + .description( + "Show a pattern, or enable, disable, or set parameters for it", + ) .argument("[provider]", "git provider (gh, gl, or bb) — auto-detected from git remote if omitted") .argument("[organization]", "organization name") .argument("[repository]", "repository name") @@ -34,7 +44,8 @@ export function registerPatternCommand(program: Command) { "after", ` Examples: - $ codacy-cloud-cli pattern eslint some-pattern-id --enable # auto-detect from git remote + $ codacy-cloud-cli pattern eslint some-pattern-id # show pattern info (auto-detect from git remote) + $ codacy-cloud-cli pattern gh my-org my-repo eslint some-pattern-id $ codacy-cloud-cli pattern gh my-org my-repo eslint some-pattern-id --enable $ codacy-cloud-cli pattern gh my-org my-repo eslint some-pattern-id --disable $ codacy-cloud-cli pattern gh my-org my-repo eslint some-pattern-id --parameter maxParams=3 @@ -59,15 +70,14 @@ Examples: ); const [toolName, patternId] = trailingArgs; const opts = this.opts(); + const format = getOutputFormat(this); - if (!opts.enable && !opts.disable && opts.parameter.length === 0) { - console.error( - ansis.red( - "Error: specify at least one of --enable, --disable, or --parameter.", - ), - ); - process.exit(1); - } + // Modify mode = at least one action flag. With no flags we just show the + // pattern's information. + const isModify = + Boolean(opts.enable) || + Boolean(opts.disable) || + opts.parameter.length > 0; const spinner = ora(`Looking up tool "${toolName}"...`).start(); @@ -83,36 +93,82 @@ Examples: process.exit(1); } - // Determine enabled state + // When the tool is driven by a local configuration file, its patterns + // are overwritten and can't be shown or changed through the API. + if (tool.settings.usesConfigurationFile) { + if (isModify) { + spinner.fail(CONFIG_FILE_LOCKED_MESSAGE); + process.exit(1); + } + spinner.stop(); + if (format === "json") { + printJson({ tool: tool.name, usesConfigurationFile: true }); + } else { + console.log(ansis.yellow(configFileNotice(tool.name))); + } + return; + } + + // There is no endpoint to fetch a single pattern at repo level, so we + // search by ID and keep only the exact match. This also yields the + // current enabled state and any coding-standard enforcement. + spinner.text = `Fetching pattern "${patternId}"...`; + const patternsResponse = + await AnalysisService.listRepositoryToolPatterns( + provider, + organization, + repository, + tool.uuid, + undefined, // languages + undefined, // categories + undefined, // severityLevels + undefined, // tags + patternId, // search by ID + ); + const match = patternsResponse.data.find( + (cp) => cp.patternDefinition.id === patternId, + ); + if (!match) { + spinner.fail( + `Pattern "${patternId}" not found for tool "${tool.name}".`, + ); + process.exit(1); + } + + // ── Info mode: no action flags → render the pattern card ──────────── + if (!isModify) { + spinner.stop(); + if (format === "json") { + printJson(pickDeep(match, PATTERN_JSON_FIELDS)); + } else { + printPatternCard(match); + console.log(ansis.dim("─".repeat(40))); + } + return; + } + + // ── Modify mode ───────────────────────────────────────────────────── + // Patterns enforced by a coding standard are managed there, not at the + // repository level, so they can't be changed here. + const enforcedBy = patternEnforcedBy(match); + if (enforcedBy.length > 0) { + const names = enforcedBy.join(", "); + const noun = + enforcedBy.length === 1 ? "coding standard" : "coding standards"; + spinner.fail( + `Pattern enforced by ${names} ${noun}, can't be modified.`, + ); + process.exit(1); + } + + // Determine target enabled state. For parameters-only updates we keep + // the pattern's current state. let enabled: boolean; if (opts.enable) { enabled = true; } else if (opts.disable) { enabled = false; } else { - // Only parameters are being set — fetch current enabled state - spinner.text = `Fetching current state of pattern "${patternId}"...`; - const patternsResponse = - await AnalysisService.listRepositoryToolPatterns( - provider, - organization, - repository, - tool.uuid, - undefined, // languages - undefined, // categories - undefined, // severityLevels - undefined, // tags - patternId, // search by ID - ); - const match = patternsResponse.data.find( - (cp) => cp.patternDefinition.id === patternId, - ); - if (!match) { - spinner.fail( - `Pattern "${patternId}" not found for tool "${toolName}".`, - ); - process.exit(1); - } enabled = match.enabled; } diff --git a/src/commands/patterns.test.ts b/src/commands/patterns.test.ts index 573370d..0064099 100644 --- a/src/commands/patterns.test.ts +++ b/src/commands/patterns.test.ts @@ -38,6 +38,23 @@ const mockTools = [ }, ]; +// A tool whose patterns are driven by a local configuration file. +const mockConfigFileTools = [ + { + uuid: "uuid-eslint", + name: "ESLint", + isClientSide: false, + settings: { + isEnabled: true, + followsStandard: false, + isCustom: false, + hasConfigurationFile: true, + usesConfigurationFile: true, + enabledBy: [], + }, + }, +]; + const mockPatterns = [ { patternDefinition: { @@ -727,6 +744,81 @@ describe("patterns command", () => { }); }); + describe("configuration file guard", () => { + beforeEach(() => { + vi.mocked(AnalysisService.listRepositoryTools).mockResolvedValue({ + data: mockConfigFileTools, + pagination: undefined, + } as any); + }); + + it("shows a notice and skips fetching patterns in list mode", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "patterns", + "gh", + "test-org", + "test-repo", + "eslint", + ]); + + const output = getAllOutput(); + expect(output).toContain("ESLint is using a local configuration file."); + expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + }); + + it("emits a JSON marker in list mode with --output json", async () => { + const program = createProgram(); + await program.parseAsync([ + "node", + "test", + "--output", + "json", + "patterns", + "gh", + "test-org", + "test-repo", + "eslint", + ]); + + const parsed = JSON.parse(getAllOutput()); + expect(parsed.usesConfigurationFile).toBe(true); + expect(parsed.tool).toBe("ESLint"); + expect(AnalysisService.listRepositoryToolPatterns).not.toHaveBeenCalled(); + }); + + it("refuses bulk update and exits 1", async () => { + vi.mocked( + AnalysisService.updateRepositoryToolPatterns, + ).mockResolvedValue(undefined as any); + + const mockExit = vi.spyOn(process, "exit").mockImplementation(() => { + throw new Error("process.exit called"); + }); + + const program = createProgram(); + await expect( + program.parseAsync([ + "node", + "test", + "patterns", + "gh", + "test-org", + "test-repo", + "eslint", + "--disable-all", + ]), + ).rejects.toThrow("process.exit called"); + + expect( + AnalysisService.updateRepositoryToolPatterns, + ).not.toHaveBeenCalled(); + mockExit.mockRestore(); + }); + }); + describe("auto-detect from git remote", () => { it("should auto-detect provider/org/repo when only toolName is provided", async () => { const program = createProgram(); diff --git a/src/commands/patterns.ts b/src/commands/patterns.ts index 1eada9a..61cb02b 100644 --- a/src/commands/patterns.ts +++ b/src/commands/patterns.ts @@ -10,7 +10,13 @@ import { printJson, printPaginationWarning, } from "../utils/output"; -import { colorSeverity, findToolByName } from "../utils/formatting"; +import { + findToolByName, + printPatternCard, + configFileNotice, + CONFIG_FILE_LOCKED_MESSAGE, + PATTERN_JSON_FIELDS, +} from "../utils/formatting"; import { AnalysisService } from "../api/client/services/AnalysisService"; import { ConfiguredPattern } from "../api/client/models/ConfiguredPattern"; import { SeverityLevel } from "../api/client/models/SeverityLevel"; @@ -54,59 +60,6 @@ function normalizeCategory(input: string): string { return CATEGORY_NORMALIZE[key] ?? input; } -function printPatternCard(cp: ConfiguredPattern): void { - const p = cp.patternDefinition; - const separator = ansis.dim("─".repeat(40)); - const enforcedByStandard = cp.enabledBy && cp.enabledBy.length > 0; - const enabled = cp.enabled || enforcedByStandard; // enabled should be enough, but there is a bug in the API - const enabledIcon = enabled - ? enforcedByStandard - ? "☑️" - : "✅" - : ansis.dim("⬛"); - const titleText = p.title ?? p.id; - const titleColored = enabled ? ansis.white(titleText) : ansis.dim(titleText); - const idStr = ansis.dim(`(${p.id})`); - const recommendedStr = p.enabled ? ` | ${ansis.magenta("Recommended")}` : ""; - - console.log(separator); - console.log(`${enabledIcon} ${titleColored} ${idStr}${recommendedStr}`); - - if (enforcedByStandard) { - const names = cp.enabledBy.map((s) => s.name).join(", "); - console.log(` ${ansis.dim(`Enforced by: ${names}`)}`); - } - - // Metadata line: severity | category subcategory | languages | tags - const meta: string[] = [colorSeverity(p.severityLevel)]; - meta.push(p.category + (p.subCategory ? ` ${ansis.dim(p.subCategory)}` : "")); - if (p.languages && p.languages.length > 0) meta.push(p.languages.join(", ")); - if (p.tags && p.tags.length > 0) meta.push(p.tags.join(", ")); - console.log(` ${meta.join(" | ")}`); - - if (p.description) { - console.log(` ${ansis.dim(p.description)}`); - } - - if (p.rationale) { - console.log(); - console.log(` ${ansis.white("Why?")} ${ansis.dim(p.rationale)}`); - } - - if (p.solution) { - console.log(` ${ansis.white("How to fix?")} ${ansis.dim(p.solution)}`); - } - - // Parameters — only shown when enabled and parameters are set - if (cp.enabled && cp.parameters && cp.parameters.length > 0) { - console.log(); - console.log(" Parameters:"); - for (const param of cp.parameters) { - console.log(` - ${param.name} = ${param.value}`); - } - } -} - interface BulkUpdateArgs { provider: string; organization: string; @@ -183,23 +136,6 @@ function printPatternCards(patterns: ConfiguredPattern[]): void { } } -const JSON_FIELDS = [ - "enabled", - "parameters", - "patternDefinition.id", - "patternDefinition.title", - "patternDefinition.severityLevel", - "patternDefinition.category", - "patternDefinition.subCategory", - "patternDefinition.languages", - "patternDefinition.tags", - "patternDefinition.enabled", - "patternDefinition.description", - "patternDefinition.rationale", - "patternDefinition.solution", - "enabledBy", -]; - function parseFilters(opts: Record) { const severities = opts.severities ? opts.severities @@ -296,6 +232,23 @@ Examples: process.exit(1); } + // When the tool is driven by a local configuration file, the patterns + // stored on Codacy are overwritten and can't be listed or changed via + // the API. Short-circuit before fetching/updating patterns. + if (tool.settings.usesConfigurationFile) { + if (opts.enableAll || opts.disableAll) { + spinner.fail(CONFIG_FILE_LOCKED_MESSAGE); + process.exit(1); + } + spinner.stop(); + if (format === "json") { + printJson({ tool: tool.name, usesConfigurationFile: true, patterns: [] }); + } else { + console.log(ansis.yellow(configFileNotice(tool.name))); + } + return; + } + const { severities, categories } = parseFilters(opts); if (opts.enableAll || opts.disableAll) { @@ -339,7 +292,9 @@ Examples: spinner.stop(); if (format === "json") { - printJson(response.data.map((cp: any) => pickDeep(cp, JSON_FIELDS))); + printJson( + response.data.map((cp: any) => pickDeep(cp, PATTERN_JSON_FIELDS)), + ); return; } diff --git a/src/commands/pull-request.test.ts b/src/commands/pull-request.test.ts index ad4a345..a3f9564 100644 --- a/src/commands/pull-request.test.ts +++ b/src/commands/pull-request.test.ts @@ -6,6 +6,7 @@ import { CoverageService } from "../api/client/services/CoverageService"; import { ToolsService } from "../api/client/services/ToolsService"; import { FileService } from "../api/client/services/FileService"; import { RepositoryService } from "../api/client/services/RepositoryService"; +import { timers } from "../utils/reanalyze-wait"; vi.mock("../api/client/services/AnalysisService"); vi.mock("../api/client/services/CoverageService"); @@ -1533,6 +1534,131 @@ describe("pull-request command", () => { }); }); + describe("--reanalyze-and-wait", () => { + beforeEach(() => { + vi.spyOn(timers, "sleep").mockResolvedValue(undefined); + }); + + function prIssue( + id: string, + title: string, + category: string, + severityLevel: string, + ) { + return { + deltaType: "Added", + commitIssue: { patternInfo: { id, title, category, severityLevel } }, + }; + } + + // Old timestamps are before t0 (Date.now); "new" ones are far in the future + // so they're reliably more recent than the real trigger time. + const OLD_S = "2000-01-01T00:00:00Z"; + const OLD_E = "2000-01-01T00:05:00Z"; + const NEW_S = "2999-01-01T00:01:00Z"; + const NEW_E = "2999-01-01T00:02:00Z"; // 60s after NEW_S → "1m 0s" + + function commits(startedAnalysis?: string, endedAnalysis?: string) { + return { + data: [ + { commit: { sha: "abc1234567890", startedAnalysis, endedAnalysis } }, + ], + } as any; + } + + it("triggers reanalysis, waits, and prints annotated deltas", async () => { + vi.mocked(AnalysisService.getRepositoryPullRequest).mockResolvedValue({ + ...mockPrData, + pullRequest: { ...mockPrData.pullRequest, headCommitSha: "prhead123" }, + } as any); // head sha (only fetched once) + vi.mocked(AnalysisService.getPullRequestCommits) + .mockResolvedValueOnce(commits(OLD_S, OLD_E)) // poll: waiting (before t0) + .mockResolvedValueOnce(commits(NEW_S, OLD_E)) // poll: in progress + .mockResolvedValueOnce(commits(NEW_S, NEW_E)); // poll: done + vi.mocked(AnalysisService.listPullRequestIssues) + .mockResolvedValueOnce({ + data: [prIssue("p.params", "Too many parameters", "Complexity", "Warning")], + pagination: {}, + } as any) // baseline: 1 issue + .mockResolvedValueOnce({ + data: [ + prIssue("p.params", "Too many parameters", "Complexity", "Warning"), + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + ], + pagination: {}, + } as any); // after: 2 issues + vi.mocked(RepositoryService.reanalyzeCommitById).mockResolvedValue( + undefined as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-request", "gh", "test-org", "test-repo", "42", "--reanalyze-and-wait", + ]); + + expect(RepositoryService.reanalyzeCommitById).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", { commitUuid: "prhead123" }, + ); + const out = (console.log as ReturnType).mock.calls + .map((c) => c[0]) + .join("\n"); + expect(out).toContain("Analysis finished in 1m 0s"); + expect(out).toContain("Hardcoded Secret"); + expect(out).toContain("(Security · Critical)"); + expect(out).toContain("In total: 1 → 2 issues"); + }); + + it("fails when the PR has no HEAD commit", async () => { + vi.mocked(AnalysisService.getRepositoryPullRequest).mockResolvedValue({ + ...mockPrData, + pullRequest: { ...mockPrData.pullRequest, headCommitSha: undefined }, + } as any); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-request", "gh", "test-org", "test-repo", "42", "--reanalyze-and-wait", + ]); + + expect(RepositoryService.reanalyzeCommitById).not.toHaveBeenCalled(); + }); + + it("emits structured JSON with --output json", async () => { + vi.mocked(AnalysisService.getRepositoryPullRequest).mockResolvedValue({ + ...mockPrData, + pullRequest: { ...mockPrData.pullRequest, headCommitSha: "prhead123" }, + } as any); // head sha (only fetched once) + vi.mocked(AnalysisService.getPullRequestCommits).mockResolvedValueOnce( + commits(NEW_S, NEW_E), + ); // poll: already done + vi.mocked(AnalysisService.listPullRequestIssues) + .mockResolvedValueOnce({ + data: [prIssue("p.params", "Too many parameters", "Complexity", "Warning")], + pagination: {}, + } as any) + .mockResolvedValueOnce({ + data: [ + prIssue("p.params", "Too many parameters", "Complexity", "Warning"), + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + ], + pagination: {}, + } as any); + vi.mocked(RepositoryService.reanalyzeCommitById).mockResolvedValue( + undefined as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "pull-request", "gh", "test-org", "test-repo", "42", + "--reanalyze-and-wait", "--output", "json", + ]); + + const calls = (console.log as ReturnType).mock.calls; + const parsed = JSON.parse(calls[calls.length - 1][0]); + expect(parsed.durationHuman).toBe("1m 0s"); + expect(parsed.totals).toEqual({ before: 1, after: 2, net: 1 }); + }); + }); + // ─── Analysis status in About ─────────────────────────────────────────── it("should show analysis status in About section", async () => { diff --git a/src/commands/pull-request.ts b/src/commands/pull-request.ts index f73a577..3f9a10e 100644 --- a/src/commands/pull-request.ts +++ b/src/commands/pull-request.ts @@ -28,6 +28,15 @@ import { formatAnalysisStatus, GateStatusMap, } from "../utils/formatting"; +import { + AnalysisStatus, + pollForAnalysis, + durationFromStatus, + snapshotFromPrIssues, + diffSnapshots, + renderReanalyzeReport, + reanalyzeJson, +} from "../utils/reanalyze-wait"; import { AnalysisService } from "../api/client/services/AnalysisService"; import { CoverageService } from "../api/client/services/CoverageService"; import { ToolsService } from "../api/client/services/ToolsService"; @@ -692,6 +701,10 @@ export function registerPullRequestCommand(program: Command) { "-A, --reanalyze", "request reanalysis of the HEAD commit of this pull request", ) + .option( + "-w, --reanalyze-and-wait", + "request reanalysis of this pull request, wait for it to finish, then show what changed", + ) .addHelpText( "after", ` @@ -705,7 +718,8 @@ Examples: $ codacy-cloud-cli pull-request gh my-org my-repo 42 --ignore-issue 9901 --ignore-reason FalsePositive $ codacy-cloud-cli pull-request gh my-org my-repo 42 --ignore-all-false-positives $ codacy-cloud-cli pull-request gh my-org my-repo 42 --unignore-issue 9901 - $ codacy-cloud-cli pull-request gh my-org my-repo 42 --reanalyze`, + $ codacy-cloud-cli pull-request gh my-org my-repo 42 --reanalyze + $ codacy-cloud-cli pull-request gh my-org my-repo 42 --reanalyze-and-wait`, ) .action(async function ( this: Command, @@ -730,6 +744,100 @@ Examples: process.exit(1); } + // --reanalyze-and-wait: trigger reanalysis, wait, then show deltas + if (this.opts().reanalyzeAndWait) { + const format = getOutputFormat(this); + const spinner = ora("Preparing reanalysis...").start(); + try { + // Resolve the PR HEAD commit to reanalyze. + const prResponse = await AnalysisService.getRepositoryPullRequest( + provider, + organization, + repository, + prNumber, + ); + const headSha = (prResponse as any).pullRequest?.headCommitSha; + if (!headSha) { + spinner.fail("No HEAD commit found for this pull request."); + return; + } + + // Capture a baseline of the PR's new issues. + const before = snapshotFromPrIssues( + await fetchAllPrIssues( + provider, + organization, + repository, + prNumber, + false, + ), + ); + + // Trigger the reanalysis (t0 = now). + const triggeredAt = Date.now(); + await RepositoryService.reanalyzeCommitById( + provider, + organization, + repository, + { commitUuid: headSha }, + ); + + // Poll the PR's first commit analysis timestamps until the new + // analysis (started after t0) starts and then finishes. + const getStatus = async (): Promise => { + const commits = (await AnalysisService.getPullRequestCommits( + provider, + organization, + repository, + prNumber, + 1, + )) as any; + const commit = commits.data?.[0]?.commit; + return { + startedAnalysis: commit?.startedAnalysis, + endedAnalysis: commit?.endedAnalysis, + }; + }; + const { status, timedOut } = await pollForAnalysis(getStatus, { + triggeredAt, + spinner, + }); + if (timedOut) { + spinner.fail( + "Analysis didn't finish within 20 minutes. Re-run with --reanalyze-and-wait later, or check the latest status with `codacy pull-request`.", + ); + return; + } + + // Fetch fresh results and compare against the baseline. + spinner.text = "Analysis done. Fetching results to compare..."; + const after = snapshotFromPrIssues( + await fetchAllPrIssues( + provider, + organization, + repository, + prNumber, + false, + ), + ); + const delta = diffSnapshots(before, after); + const durationMs = + durationFromStatus(status) ?? Date.now() - triggeredAt; + + spinner.stop(); + if (format === "json") { + printJson(reanalyzeJson(before, after, delta, durationMs)); + } else { + renderReanalyzeReport(delta, durationMs); + } + } catch (waitErr) { + spinner.fail( + `Failed to reanalyze: ${waitErr instanceof Error ? waitErr.message : waitErr}`, + ); + } + return; + } + // --reanalyze: request reanalysis of the HEAD commit if (this.opts().reanalyze) { const spinner = ora("Requesting reanalysis...").start(); diff --git a/src/commands/repository.test.ts b/src/commands/repository.test.ts index e67b52f..63d6b37 100644 --- a/src/commands/repository.test.ts +++ b/src/commands/repository.test.ts @@ -4,6 +4,7 @@ import { registerRepositoryCommand } from "./repository"; import { AnalysisService } from "../api/client/services/AnalysisService"; import { RepositoryService } from "../api/client/services/RepositoryService"; import { CodingStandardsService } from "../api/client/services/CodingStandardsService"; +import { timers } from "../utils/reanalyze-wait"; vi.mock("../api/client/services/AnalysisService"); vi.mock("../api/client/services/RepositoryService"); @@ -801,4 +802,116 @@ describe("repository command", () => { ); }); }); + + // ─── Reanalyze and wait ─────────────────────────────────────────────────── + + describe("--reanalyze-and-wait", () => { + beforeEach(() => { + vi.spyOn(timers, "sleep").mockResolvedValue(undefined); + }); + + function overview(errorTotal: number, secretTotal: number) { + return { + data: { + counts: { + categories: [{ name: "Security", total: secretTotal }], + levels: [ + { name: "Error", total: errorTotal }, + { name: "Warning", total: 11 }, + ], + languages: [], + tags: [], + patterns: [ + { id: "p.secret", title: "Hardcoded Secret", total: secretTotal }, + ], + authors: [], + potentialFalsePositives: [], + }, + }, + }; + } + + // Old timestamps are before t0 (Date.now); "new" ones are far in the future + // so they're reliably more recent than the real trigger time. + const OLD_S = "2000-01-01T00:00:00Z"; + const OLD_E = "2000-01-01T00:05:00Z"; + const NEW_S = "2999-01-01T00:01:00Z"; + const NEW_E = "2999-01-01T00:02:00Z"; // 60s after NEW_S → "1m 0s" + + function commits(startedAnalysis?: string, endedAnalysis?: string) { + return { + data: [ + { commit: { sha: "abc1234567890", startedAnalysis, endedAnalysis } }, + ], + } as any; + } + + it("triggers reanalysis, waits, and prints deltas", async () => { + vi.mocked(AnalysisService.listRepositoryCommits) + .mockResolvedValueOnce(commits(OLD_S, OLD_E)) // head sha + .mockResolvedValueOnce(commits(OLD_S, OLD_E)) // poll: waiting (before t0) + .mockResolvedValueOnce(commits(NEW_S, OLD_E)) // poll: in progress + .mockResolvedValueOnce(commits(NEW_S, NEW_E)); // poll: done + vi.mocked(AnalysisService.issuesOverview) + .mockResolvedValueOnce(overview(4, 8) as any) // baseline: 15 issues + .mockResolvedValueOnce(overview(6, 13) as any); // after: 17 issues + vi.mocked(RepositoryService.reanalyzeCommitById).mockResolvedValue( + undefined as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", "--reanalyze-and-wait", + ]); + + expect(RepositoryService.reanalyzeCommitById).toHaveBeenCalledWith( + "gh", "test-org", "test-repo", { commitUuid: "abc1234567890" }, + ); + const out = (console.log as ReturnType).mock.calls + .map((c) => c[0]) + .join("\n"); + expect(out).toContain("Analysis finished in 1m 0s"); + expect(out).toContain("Hardcoded Secret"); + expect(out).toContain("In total: 15 → 17 issues"); + }); + + it("fails when there are no commits to reanalyze", async () => { + vi.mocked(AnalysisService.listRepositoryCommits).mockResolvedValue({ + data: [], + } as any); + vi.mocked(AnalysisService.issuesOverview).mockResolvedValue( + overview(4, 8) as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", "--reanalyze-and-wait", + ]); + + expect(RepositoryService.reanalyzeCommitById).not.toHaveBeenCalled(); + }); + + it("emits structured JSON with --output json", async () => { + vi.mocked(AnalysisService.listRepositoryCommits) + .mockResolvedValueOnce(commits(OLD_S, OLD_E)) // head sha + .mockResolvedValueOnce(commits(NEW_S, NEW_E)); // poll: already done + vi.mocked(AnalysisService.issuesOverview) + .mockResolvedValueOnce(overview(4, 8) as any) + .mockResolvedValueOnce(overview(6, 13) as any); + vi.mocked(RepositoryService.reanalyzeCommitById).mockResolvedValue( + undefined as any, + ); + + const program = createProgram(); + await program.parseAsync([ + "node", "test", "repository", "gh", "test-org", "test-repo", + "--reanalyze-and-wait", "--output", "json", + ]); + + const calls = (console.log as ReturnType).mock.calls; + const parsed = JSON.parse(calls[calls.length - 1][0]); + expect(parsed.durationHuman).toBe("1m 0s"); + expect(parsed.totals).toEqual({ before: 15, after: 17, net: 2 }); + }); + }); }); diff --git a/src/commands/repository.ts b/src/commands/repository.ts index 5ed7992..e388068 100644 --- a/src/commands/repository.ts +++ b/src/commands/repository.ts @@ -25,6 +25,15 @@ import { formatPrIssues, formatAnalysisStatus, } from "../utils/formatting"; +import { + AnalysisStatus, + pollForAnalysis, + durationFromStatus, + snapshotFromOverview, + diffSnapshots, + renderReanalyzeReport, + reanalyzeJson, +} from "../utils/reanalyze-wait"; import { AnalysisService } from "../api/client/services/AnalysisService"; import { RepositoryService } from "../api/client/services/RepositoryService"; import { CodingStandardsService } from "../api/client/services/CodingStandardsService"; @@ -222,6 +231,10 @@ export function registerRepositoryCommand(program: Command) { .option("-f, --follow", "follow this repository on Codacy") .option("-u, --unfollow", "unfollow this repository on Codacy") .option("-R, --reanalyze", "request reanalysis of the HEAD commit") + .option( + "-w, --reanalyze-and-wait", + "request reanalysis of the HEAD commit, wait for it to finish, then show what changed", + ) .option("-L, --link-standard ", "link a coding standard to this repository (by standard ID)") .option("-K, --unlink-standard ", "unlink a coding standard from this repository (by standard ID)") .addHelpText( @@ -236,6 +249,7 @@ Examples: $ codacy-cloud-cli repository gh my-org my-repo --follow $ codacy-cloud-cli repository gh my-org my-repo --unfollow $ codacy-cloud-cli repository gh my-org my-repo --reanalyze + $ codacy-cloud-cli repository gh my-org my-repo --reanalyze-and-wait $ codacy-cloud-cli repository gh my-org my-repo --link-standard 12345 $ codacy-cloud-cli repository gh my-org my-repo --unlink-standard 12345`, ) @@ -317,6 +331,93 @@ Examples: return; } + // ── Action: reanalyze-and-wait ─────────────────────────────────── + if (opts.reanalyzeAndWait) { + const format = getOutputFormat(this); + const spinner = ora("Preparing reanalysis...").start(); + try { + // Resolve the HEAD commit to reanalyze + capture baseline issue counts. + const [commitsResponse, baselineOverview] = await Promise.all([ + AnalysisService.listRepositoryCommits( + provider, + organization, + repository, + undefined, + undefined, + 1, + ), + AnalysisService.issuesOverview(provider, organization, repository), + ]); + const headCommit = commitsResponse.data[0]; + if (!headCommit) { + spinner.fail("No commits found in this repository."); + return; + } + const before = snapshotFromOverview(baselineOverview.data.counts); + + // Trigger the reanalysis (t0 = now). + const triggeredAt = Date.now(); + await RepositoryService.reanalyzeCommitById( + provider, + organization, + repository, + { commitUuid: headCommit.commit.sha }, + ); + + // Poll the first commit's analysis timestamps until the new + // analysis (started after t0) starts and then finishes. + const getStatus = async (): Promise => { + const commits = await AnalysisService.listRepositoryCommits( + provider, + organization, + repository, + undefined, + undefined, + 1, + ); + const commit = commits.data[0]?.commit; + return { + startedAnalysis: commit?.startedAnalysis, + endedAnalysis: commit?.endedAnalysis, + }; + }; + const { status, timedOut } = await pollForAnalysis(getStatus, { + triggeredAt, + spinner, + }); + if (timedOut) { + spinner.fail( + "Analysis didn't finish within 20 minutes. Re-run with --reanalyze-and-wait later, or check the latest status with `codacy repository`.", + ); + return; + } + + // Fetch fresh results and compare against the baseline. + spinner.text = "Analysis done. Fetching results to compare..."; + const afterOverview = await AnalysisService.issuesOverview( + provider, + organization, + repository, + ); + const after = snapshotFromOverview(afterOverview.data.counts); + const delta = diffSnapshots(before, after); + const durationMs = + durationFromStatus(status) ?? Date.now() - triggeredAt; + + spinner.stop(); + if (format === "json") { + printJson(reanalyzeJson(before, after, delta, durationMs)); + } else { + renderReanalyzeReport(delta, durationMs); + } + } catch (waitErr) { + spinner.fail( + `Failed to reanalyze: ${waitErr instanceof Error ? waitErr.message : waitErr}`, + ); + } + return; + } + // ── Action: reanalyze ──────────────────────────────────────────── if (opts.reanalyze) { const spinner = ora("Requesting reanalysis...").start(); diff --git a/src/utils/formatting.test.ts b/src/utils/formatting.test.ts index 99c77b3..c31a367 100644 --- a/src/utils/formatting.test.ts +++ b/src/utils/formatting.test.ts @@ -1,5 +1,10 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; -import { formatAnalysisStatus, resolveToolUuids } from "./formatting"; +import { + formatAnalysisStatus, + resolveToolUuids, + formatDuration, + isBeingAnalyzed, +} from "./formatting"; // Mock ansis to return raw text for easier testing vi.mock("ansis", () => ({ @@ -162,3 +167,46 @@ describe("resolveToolUuids", () => { expect(fetchTools).toHaveBeenCalledTimes(1); }); }); + +describe("formatDuration", () => { + it("shows seconds only for sub-minute durations", () => { + expect(formatDuration(45_000)).toBe("45s"); + expect(formatDuration(0)).toBe("0s"); + expect(formatDuration(999)).toBe("1s"); + }); + + it("shows minutes and seconds", () => { + expect(formatDuration(94_000)).toBe("1m 34s"); + expect(formatDuration(60_000)).toBe("1m 0s"); + }); + + it("shows hours and minutes (dropping seconds)", () => { + expect(formatDuration(7_380_000)).toBe("2h 3m"); + }); + + it("clamps negatives to 0s", () => { + expect(formatDuration(-5_000)).toBe("0s"); + }); +}); + +describe("isBeingAnalyzed", () => { + it("is true when started but never finished", () => { + expect(isBeingAnalyzed("2025-06-15T10:00:00Z", undefined)).toBe(true); + }); + + it("is true when started after the last finish (a fresh reanalysis)", () => { + expect( + isBeingAnalyzed("2025-06-15T10:10:00Z", "2025-06-15T10:05:00Z"), + ).toBe(true); + }); + + it("is false when finished after it started", () => { + expect( + isBeingAnalyzed("2025-06-15T10:00:00Z", "2025-06-15T10:05:00Z"), + ).toBe(false); + }); + + it("is false when never started", () => { + expect(isBeingAnalyzed(undefined, undefined)).toBe(false); + }); +}); diff --git a/src/utils/formatting.ts b/src/utils/formatting.ts index 7048a73..4991e61 100644 --- a/src/utils/formatting.ts +++ b/src/utils/formatting.ts @@ -7,6 +7,7 @@ import { AnalysisResultReason } from "../api/client/models/AnalysisResultReason" import { CommitIssue } from "../api/client/models/CommitIssue"; import { SeverityLevel } from "../api/client/models/SeverityLevel"; import { Pattern } from "../api/client/models/Pattern"; +import { ConfiguredPattern } from "../api/client/models/ConfiguredPattern"; import { CodeBlockLine } from "../api/client/models/CodeBlockLine"; import { CveRecord } from "./cve"; import { AnalysisTool } from "../api/client/models/AnalysisTool"; @@ -172,6 +173,37 @@ export function truncate(text: string, max: number): string { return text.length > max ? text.substring(0, max - 3) + "..." : text; } +/** + * Format a duration in milliseconds as a compact human string. + * e.g. 45000 → "45s", 94000 → "1m 34s", 7380000 → "2h 3m" + * Sub-minute durations show seconds only; hour+ durations drop the seconds. + */ +export function formatDuration(ms: number): string { + const totalSeconds = Math.max(0, Math.round(ms / 1000)); + const hours = Math.floor(totalSeconds / 3600); + const minutes = Math.floor((totalSeconds % 3600) / 60); + const seconds = totalSeconds % 60; + + if (hours > 0) return `${hours}h ${minutes}m`; + if (minutes > 0) return `${minutes}m ${seconds}s`; + return `${seconds}s`; +} + +/** + * Whether a commit is currently being analyzed, based on its analysis timestamps. + * True when an analysis has started and either hasn't finished yet, or started + * more recently than the last finish (i.e. a fresh reanalysis is running). + */ +export function isBeingAnalyzed( + startedAnalysis?: string, + endedAnalysis?: string, +): boolean { + return ( + !!startedAnalysis && + (!endedAnalysis || parseISO(startedAnalysis) > parseISO(endedAnalysis)) + ); +} + /** * Color a metric value based on a threshold. * "max" thresholds: green if under, red if over. @@ -509,6 +541,129 @@ export function printIssueDetail( printIssueCodeContext(issue, pattern, lines); } +/** + * pickDeep paths for the JSON projection of a ConfiguredPattern. Shared by the + * `patterns` (list) and `pattern` (single info) commands so both emit the same + * shape. Mirrors the fields rendered by `printPatternCard`. + */ +export const PATTERN_JSON_FIELDS = [ + "enabled", + "parameters", + "patternDefinition.id", + "patternDefinition.title", + "patternDefinition.severityLevel", + "patternDefinition.category", + "patternDefinition.subCategory", + "patternDefinition.languages", + "patternDefinition.tags", + "patternDefinition.enabled", + "patternDefinition.description", + "patternDefinition.rationale", + "patternDefinition.solution", + "enabledBy", +]; + +/** + * Header: enabled icon (☑️ enforced by a standard, ✅ enabled directly, dim ⬛ + * disabled), title, id, "Recommended" tag, and an "Enforced by" line. + */ +/** Status icon: ☑️ standard-enforced, ✅ enabled directly, dim ⬛ disabled. */ +function patternIcon(enabled: boolean, enforcedByStandard: boolean): string { + if (!enabled) return ansis.dim("⬛"); + return enforcedByStandard ? "☑️" : "✅"; +} + +function printPatternHeader( + cp: ConfiguredPattern, + enabled: boolean, + enforcedByStandard: boolean, +): void { + const p = cp.patternDefinition; + const enabledIcon = patternIcon(enabled, enforcedByStandard); + const titleText = p.title ?? p.id; + const titleColored = enabled ? ansis.white(titleText) : ansis.dim(titleText); + const recommendedStr = p.enabled ? ` | ${ansis.magenta("Recommended")}` : ""; + + console.log(ansis.dim("─".repeat(40))); + console.log( + `${enabledIcon} ${titleColored} ${ansis.dim(`(${p.id})`)}${recommendedStr}`, + ); + + if (enforcedByStandard) { + const names = cp.enabledBy.map((s) => s.name).join(", "); + console.log(` ${ansis.dim(`Enforced by: ${names}`)}`); + } +} + +/** Metadata line (severity | category subcategory | languages | tags) + description. */ +function printPatternMeta(p: Pattern): void { + const meta: string[] = [colorSeverity(p.severityLevel)]; + meta.push(p.category + (p.subCategory ? ` ${ansis.dim(p.subCategory)}` : "")); + if (p.languages && p.languages.length > 0) meta.push(p.languages.join(", ")); + if (p.tags && p.tags.length > 0) meta.push(p.tags.join(", ")); + console.log(` ${meta.join(" | ")}`); + + if (p.description) { + console.log(` ${ansis.dim(p.description)}`); + } +} + +/** "Why?" / "How to fix?" documentation lines. */ +function printPatternDocs(p: Pattern): void { + if (p.rationale) { + console.log(); + console.log(` ${ansis.white("Why?")} ${ansis.dim(p.rationale)}`); + } + if (p.solution) { + console.log(` ${ansis.white("How to fix?")} ${ansis.dim(p.solution)}`); + } +} + +/** Configured parameters — only shown when the pattern is enabled and has some. */ +function printPatternParams(cp: ConfiguredPattern): void { + if (!cp.enabled || !cp.parameters || cp.parameters.length === 0) return; + console.log(); + console.log(" Parameters:"); + for (const param of cp.parameters) { + console.log(` - ${param.name} = ${param.value}`); + } +} + +/** + * Print a single configured-pattern card. Shared by the `patterns` (list) and + * `pattern` (single info) commands so both render identically. + * + * (`enabled` is OR'd with `enabledBy` to work around an API quirk where a + * standard-enforced pattern can report `enabled: false`.) + */ +export function printPatternCard(cp: ConfiguredPattern): void { + const enforcedByStandard = !!(cp.enabledBy && cp.enabledBy.length > 0); + const enabled = cp.enabled || enforcedByStandard; + printPatternHeader(cp, enabled, enforcedByStandard); + printPatternMeta(cp.patternDefinition); + printPatternDocs(cp.patternDefinition); + printPatternParams(cp); +} + +/** + * Whether a configured pattern is enforced by one or more coding standards. + * Such patterns can't be enabled/disabled/customized directly at the repo level. + */ +export function patternEnforcedBy(cp: ConfiguredPattern): string[] { + return cp.enabledBy?.map((s) => s.name) ?? []; +} + +/** + * Shared messaging for tools whose patterns are managed by a local config file. + * Centralized so the `pattern` and `patterns` commands stay consistent. + */ +// Read paths (listing / showing a pattern): patterns can't be fetched. +export const configFileNotice = (toolName: string): string => + `${toolName} is using a local configuration file.`; +// Write paths (enable/disable/customize): the update is refused. +export const CONFIG_FILE_LOCKED_MESSAGE = + "Tool uses a local configuration file, can't be updated."; + /** * Find a tool from a list by name using best-match logic: * 1. Exact match (case-insensitive, hyphens treated as spaces) @@ -633,11 +788,7 @@ export function formatAnalysisStatus(opts: { return ansis.dim("Never"); } - const isBeingAnalyzed = - !!startedAnalysis && - (!endedAnalysis || parseISO(startedAnalysis) > parseISO(endedAnalysis)); - - if (isBeingAnalyzed) { + if (isBeingAnalyzed(startedAnalysis, endedAnalysis)) { if (endedAnalysis) { const finishedDate = formatFriendlyDate(endedAnalysis); return `Finished ${finishedDate} (${shortSha}) — ${ansis.blueBright("Reanalysis in progress...")}`; diff --git a/src/utils/reanalyze-wait.test.ts b/src/utils/reanalyze-wait.test.ts new file mode 100644 index 0000000..a740a2a --- /dev/null +++ b/src/utils/reanalyze-wait.test.ts @@ -0,0 +1,358 @@ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { + snapshotFromOverview, + snapshotFromPrIssues, + diffSnapshots, + durationFromStatus, + pollForAnalysis, + renderReanalyzeReport, + reanalyzeJson, + timers, + AnalysisStatus, + IssueSnapshot, +} from "./reanalyze-wait"; + +// Render colors as raw text so output assertions stay readable. +vi.mock("ansis", () => ({ + default: { + dim: (s: string) => s, + bold: (s: string) => s, + red: (s: string) => s, + green: (s: string) => s, + yellow: (s: string) => s, + blue: (s: string) => s, + blueBright: (s: string) => s, + hex: () => (s: string) => s, + white: (s: string) => s, + magenta: (s: string) => s, + }, +})); + +const overviewCounts = { + categories: [ + { name: "Security", total: 10 }, + { name: "CodeStyle", total: 5 }, + ], + languages: [{ name: "TypeScript", total: 15 }], + levels: [ + { name: "Error", total: 4 }, + { name: "Warning", total: 11 }, + ], + tags: [], + patterns: [ + { id: "p.secret", title: "Hardcoded Secret", total: 8 }, + { id: "p.params", title: "Too many parameters", total: 7 }, + ], + authors: [], + potentialFalsePositives: [], +} as any; + +function prIssue( + id: string, + title: string, + category: string, + severityLevel: string, +): any { + return { + deltaType: "Added", + commitIssue: { + patternInfo: { id, title, category, severityLevel }, + }, + }; +} + +describe("snapshotFromOverview", () => { + it("maps levels/categories/patterns and sums the total", () => { + const snap = snapshotFromOverview(overviewCounts); + expect(snap.total).toBe(15); // 4 + 11 + expect(snap.bySeverity).toEqual({ Error: 4, Warning: 11 }); + expect(snap.byCategory).toEqual({ Security: 10, CodeStyle: 5 }); + expect(snap.byPattern["p.secret"]).toEqual({ + title: "Hardcoded Secret", + count: 8, + }); + // overview patterns carry no category/severity + expect(snap.byPattern["p.secret"].category).toBeUndefined(); + expect(snap.byPattern["p.secret"].severity).toBeUndefined(); + }); +}); + +describe("snapshotFromPrIssues", () => { + it("tallies dimensions and annotates pattern buckets", () => { + const snap = snapshotFromPrIssues([ + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + prIssue("p.params", "Too many parameters", "Complexity", "Warning"), + ]); + expect(snap.total).toBe(3); + expect(snap.bySeverity).toEqual({ Error: 2, Warning: 1 }); + expect(snap.byCategory).toEqual({ Security: 2, Complexity: 1 }); + expect(snap.byPattern["p.secret"]).toEqual({ + title: "Hardcoded Secret", + category: "Security", + severity: "Error", + count: 2, + }); + }); +}); + +describe("diffSnapshots", () => { + it("computes nonzero net deltas per dimension", () => { + const before = snapshotFromOverview(overviewCounts); + const after = snapshotFromOverview({ + ...overviewCounts, + levels: [ + { name: "Error", total: 6 }, // +2 + { name: "Warning", total: 11 }, // 0 (dropped) + { name: "Info", total: 3 }, // +3 (new) + ], + categories: [ + { name: "Security", total: 12 }, // +2 + { name: "CodeStyle", total: 5 }, // 0 (dropped) + ], + patterns: [ + { id: "p.secret", title: "Hardcoded Secret", total: 13 }, // +5 + { id: "p.params", title: "Too many parameters", total: 4 }, // -3 + ], + } as any); + + const delta = diffSnapshots(before, after); + + expect(delta.totalBefore).toBe(15); + expect(delta.totalAfter).toBe(20); + expect(delta.netTotal).toBe(5); + + // severity sorted Critical→Minor (Error, then Info); Warning dropped (0) + expect(delta.bySeverity.map((e) => [e.key, e.delta])).toEqual([ + ["Error", 2], + ["Info", 3], + ]); + // category: only Security changed + expect(delta.byCategory.map((e) => [e.key, e.delta])).toEqual([ + ["Security", 2], + ]); + // pattern sorted by |delta| desc + expect(delta.byPattern.map((e) => [e.label, e.delta])).toEqual([ + ["Hardcoded Secret", 5], + ["Too many parameters", -3], + ]); + }); + + it("annotates pattern deltas when category/severity present (PR path)", () => { + const before = snapshotFromPrIssues([ + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + ]); + const after = snapshotFromPrIssues([ + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + ]); + const delta = diffSnapshots(before, after); + expect(delta.byPattern[0]).toMatchObject({ + label: "Hardcoded Secret", + delta: 1, + category: "Security", + severity: "Error", + }); + }); + + it("sorts unknown severities last instead of first", () => { + // A severity outside the canonical order must not jump ahead of known ones. + const before: any = { + total: 0, + bySeverity: {}, + byCategory: {}, + byPattern: {}, + }; + const after: any = { + total: 3, + bySeverity: { Mystery: 1, Error: 1, Info: 1 }, + byCategory: {}, + byPattern: {}, + }; + const delta = diffSnapshots(before, after); + expect(delta.bySeverity.map((e) => e.key)).toEqual([ + "Error", + "Info", + "Mystery", + ]); + }); +}); + +describe("durationFromStatus", () => { + it("computes elapsed ms between started and ended", () => { + expect( + durationFromStatus({ + startedAnalysis: "2025-06-15T10:00:00Z", + endedAnalysis: "2025-06-15T10:01:34Z", + }), + ).toBe(94_000); + }); + + it("returns null when a timestamp is missing or inverted", () => { + expect(durationFromStatus({ startedAnalysis: "x" })).toBeNull(); + expect( + durationFromStatus({ + startedAnalysis: "2025-06-15T10:05:00Z", + endedAnalysis: "2025-06-15T10:00:00Z", + }), + ).toBeNull(); + }); +}); + +describe("pollForAnalysis", () => { + beforeEach(() => { + vi.spyOn(timers, "sleep").mockResolvedValue(undefined); + }); + + // t0 between the "old" timestamps (before) and the "new" ones (after). + const T0 = Date.parse("2025-06-15T10:00:30Z"); + + it("waits for start then completion, transitioning the spinner text", async () => { + const spinner = { text: "" }; + const getStatus = vi + .fn<() => Promise>() + // old finished analysis (both timestamps before t0) → keep waiting + .mockResolvedValueOnce({ + startedAnalysis: "2025-06-15T09:55:00Z", + endedAnalysis: "2025-06-15T10:00:00Z", + }) + // new analysis running (started after t0, after its old finish) + .mockResolvedValueOnce({ + startedAnalysis: "2025-06-15T10:01:00Z", + endedAnalysis: "2025-06-15T10:00:00Z", + }) + // new analysis finished (ended after started) + .mockResolvedValueOnce({ + startedAnalysis: "2025-06-15T10:01:00Z", + endedAnalysis: "2025-06-15T10:02:00Z", + }); + + const result = await pollForAnalysis(getStatus, { + triggeredAt: T0, + spinner, + }); + + expect(result.timedOut).toBe(false); + expect(result.status.endedAnalysis).toBe("2025-06-15T10:02:00Z"); + expect(getStatus).toHaveBeenCalledTimes(3); + expect(spinner.text).toBe( + "Analysis in progress. This may take a few minutes...", + ); + }); + + it("returns immediately if analysis already finished before we saw it run", async () => { + const spinner = { text: "" }; + const getStatus = vi.fn<() => Promise>().mockResolvedValue({ + startedAnalysis: "2025-06-15T10:01:00Z", // after t0 + endedAnalysis: "2025-06-15T10:02:00Z", // already finished + }); + + const result = await pollForAnalysis(getStatus, { + triggeredAt: T0, + spinner, + }); + + expect(result.timedOut).toBe(false); + expect(getStatus).toHaveBeenCalledTimes(1); + // never transitioned to the "in progress" message + expect(spinner.text).toBe("Analysis requested. Waiting for it to start..."); + }); + + it("ignores an analysis that started before we triggered (t0)", async () => { + const spinner = { text: "" }; + let t = 0; + const now = () => { + const v = t; + t += 5_000; + return v; + }; + // started before t0 → not our analysis, so it should keep waiting (and time out) + const getStatus = vi.fn<() => Promise>().mockResolvedValue({ + startedAnalysis: "2025-06-15T10:00:10Z", // before t0 (10:00:30) + endedAnalysis: "2025-06-15T10:00:05Z", + }); + + const result = await pollForAnalysis(getStatus, { + triggeredAt: T0, + spinner, + maxWaitMs: 1_000, + now, + }); + + expect(result.timedOut).toBe(true); + }); + + it("times out when analysis never finishes", async () => { + const spinner = { text: "" }; + let t = 0; + const now = () => { + const v = t; + t += 5_000; + return v; + }; + const getStatus = vi.fn<() => Promise>().mockResolvedValue({ + startedAnalysis: "2025-06-15T10:01:00Z", // after t0, never ends + endedAnalysis: "2025-06-15T10:00:00Z", + }); + + const result = await pollForAnalysis(getStatus, { + triggeredAt: T0, + spinner, + maxWaitMs: 1_000, + now, + }); + + expect(result.timedOut).toBe(true); + }); +}); + +describe("renderReanalyzeReport", () => { + let logSpy: ReturnType; + beforeEach(() => { + logSpy = vi.spyOn(console, "log").mockImplementation(() => {}); + }); + + function output(): string { + return logSpy.mock.calls.map((c: any) => c[0]).join("\n"); + } + + it("prints duration headline, sections, and totals", () => { + const before = snapshotFromPrIssues([ + prIssue("p.params", "Too many parameters", "Complexity", "Warning"), + ]); + const after = snapshotFromPrIssues([ + prIssue("p.secret", "Hardcoded Secret", "Security", "Error"), + ]); + renderReanalyzeReport(diffSnapshots(before, after), 94_000); + const out = output(); + expect(out).toContain("Analysis finished in 1m 34s"); + expect(out).toContain("By pattern:"); + expect(out).toContain("Hardcoded Secret"); + expect(out).toContain("(Security · Critical)"); // annotation present for PR data + expect(out).toContain("By severity:"); + expect(out).toContain("In total: 1 → 1 issues (net 0)"); + }); + + it("reports no change when snapshots are identical", () => { + const snap = snapshotFromOverview(overviewCounts); + renderReanalyzeReport(diffSnapshots(snap, snap), 1000); + expect(output()).toContain("No change in issues."); + }); +}); + +describe("reanalyzeJson", () => { + it("bundles totals and per-dimension deltas", () => { + const before: IssueSnapshot = snapshotFromOverview(overviewCounts); + const after = snapshotFromOverview({ + ...overviewCounts, + levels: [{ name: "Error", total: 6 }, { name: "Warning", total: 11 }], + } as any); + const delta = diffSnapshots(before, after); + const json = reanalyzeJson(before, after, delta, 94_000) as any; + expect(json.durationHuman).toBe("1m 34s"); + expect(json.totals).toEqual({ before: 15, after: 17, net: 2 }); + expect(json.deltas.bySeverity).toEqual([ + { key: "Error", label: "Critical", delta: 2, severity: "Error" }, + ]); + }); +}); diff --git a/src/utils/reanalyze-wait.ts b/src/utils/reanalyze-wait.ts new file mode 100644 index 0000000..3ad4ae5 --- /dev/null +++ b/src/utils/reanalyze-wait.ts @@ -0,0 +1,462 @@ +import ansis from "ansis"; +import { parseISO } from "date-fns"; +import { IssuesOverviewCounts } from "../api/client/models/IssuesOverviewCounts"; +import { CommitDeltaIssue } from "../api/client/models/CommitDeltaIssue"; +import { SeverityLevel } from "../api/client/models/SeverityLevel"; +import { + SEVERITY_DISPLAY, + colorSeverity, + formatDelta, + formatDuration, +} from "./formatting"; + +/** + * Shared logic for the `--reanalyze-and-wait` variant of the `repository` and + * `pull-request` commands: poll for analysis completion, snapshot the issue + * counts before/after, and render the deltas. + * + * The `repository` baseline comes from the issues-overview endpoint (counts by + * severity / category / pattern as independent lists), so its pattern entries + * carry no category/severity. The `pull-request` baseline comes from the raw + * PR issue list, so its pattern entries are annotated with category + severity. + */ + +/** Poll every 10 seconds. */ +export const POLL_INTERVAL_MS = 10_000; +/** Give up after 20 minutes. */ +export const MAX_WAIT_MS = 20 * 60_000; +/** Maximum number of per-pattern rows printed before collapsing into "… (N more)". */ +export const PATTERN_LIMIT = 20; + +/** + * Timers wrapped in an object so tests can replace `sleep` with an instant + * resolver (`vi.spyOn(timers, "sleep").mockResolvedValue()`). + */ +export const timers = { + sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); + }, +}; + +/** Minimal spinner surface used while polling (satisfied by an `ora` instance). */ +export interface SpinnerLike { + text: string; +} + +/** + * Analysis timestamps for the HEAD commit, read from the repo/PR `/commits` + * endpoint (first commit). The polling loop derives in-progress/done from these + * relative to the trigger time, so callers just pass the raw timestamps. + */ +export interface AnalysisStatus { + startedAnalysis?: string; + endedAnalysis?: string; +} + +export interface PatternBucket { + title: string; + category?: string; + severity?: SeverityLevel; + count: number; +} + +/** A point-in-time count of issues across three independent dimensions. */ +export interface IssueSnapshot { + total: number; + /** Keyed by SeverityLevel ('Error' | 'High' | 'Warning' | 'Info'). */ + bySeverity: Record; + /** Keyed by category name. */ + byCategory: Record; + /** Keyed by pattern id. */ + byPattern: Record; +} + +export interface DeltaEntry { + key: string; + label: string; + delta: number; + category?: string; + severity?: SeverityLevel; +} + +export interface SnapshotDelta { + totalBefore: number; + totalAfter: number; + netTotal: number; + byPattern: DeltaEntry[]; + bySeverity: DeltaEntry[]; + byCategory: DeltaEntry[]; +} + +const SEVERITY_ORDER: SeverityLevel[] = ["Error", "High", "Warning", "Info"]; + +// ── Snapshot builders ──────────────────────────────────────────────────────── + +/** + * Build a snapshot from a repository issues-overview response. The overview + * exposes counts per dimension independently, so pattern buckets have no + * category/severity. + */ +export function snapshotFromOverview( + counts: IssuesOverviewCounts, +): IssueSnapshot { + const bySeverity: Record = {}; + for (const c of counts.levels) bySeverity[c.name] = c.total; + + const byCategory: Record = {}; + for (const c of counts.categories) byCategory[c.name] = c.total; + + const byPattern: Record = {}; + for (const p of counts.patterns) { + byPattern[p.id] = { title: p.title || p.id, count: p.total }; + } + + const total = counts.levels.reduce((sum, c) => sum + c.total, 0); + return { total, bySeverity, byCategory, byPattern }; +} + +/** Increment (or create) a pattern bucket for one PR issue's pattern. */ +function tallyPattern( + byPattern: Record, + id: string, + title: string | undefined, + category: string, + severity: SeverityLevel, +): void { + const existing = byPattern[id]; + if (existing) { + existing.count++; + return; + } + byPattern[id] = { title: title || id, category, severity, count: 1 }; +} + +/** + * Build a snapshot from a list of pull-request issues. Each issue carries its + * pattern's category and severity, so pattern buckets are fully annotated. + */ +export function snapshotFromPrIssues(issues: CommitDeltaIssue[]): IssueSnapshot { + const bySeverity: Record = {}; + const byCategory: Record = {}; + const byPattern: Record = {}; + let total = 0; + + for (const di of issues) { + const p = di.commitIssue.patternInfo; + total++; + bySeverity[p.severityLevel] = (bySeverity[p.severityLevel] ?? 0) + 1; + byCategory[p.category] = (byCategory[p.category] ?? 0) + 1; + tallyPattern(byPattern, p.id, p.title, p.category, p.severityLevel); + } + + return { total, bySeverity, byCategory, byPattern }; +} + +// ── Diffing ────────────────────────────────────────────────────────────────── + +function diffCountMap( + before: Record, + after: Record, +): { key: string; delta: number }[] { + const keys = new Set([...Object.keys(before), ...Object.keys(after)]); + const out: { key: string; delta: number }[] = []; + for (const key of keys) { + const delta = (after[key] ?? 0) - (before[key] ?? 0); + if (delta !== 0) out.push({ key, delta }); + } + return out; +} + +/** Canonical sort rank for a severity; unknown values sort last. */ +function severityRank(severity: SeverityLevel | undefined): number { + const idx = SEVERITY_ORDER.indexOf(severity as SeverityLevel); + return idx === -1 ? SEVERITY_ORDER.length : idx; +} + +/** Sort delta entries by magnitude of change, breaking ties alphabetically. */ +function byMagnitude(a: DeltaEntry, b: DeltaEntry): number { + return Math.abs(b.delta) - Math.abs(a.delta) || a.label.localeCompare(b.label); +} + +/** Pattern deltas — union of ids, annotated from whichever snapshot has the bucket. */ +function diffPatternMap( + before: Record, + after: Record, +): DeltaEntry[] { + const ids = new Set([...Object.keys(before), ...Object.keys(after)]); + const out: DeltaEntry[] = []; + for (const id of ids) { + const beforeBucket = before[id]; + const afterBucket = after[id]; + const delta = (afterBucket?.count ?? 0) - (beforeBucket?.count ?? 0); + if (delta === 0) continue; + const bucket = afterBucket ?? beforeBucket; + out.push({ + key: id, + label: bucket.title, + delta, + category: bucket.category, + severity: bucket.severity, + }); + } + return out.sort(byMagnitude); +} + +/** Compute per-dimension net deltas (after − before), dropping unchanged buckets. */ +export function diffSnapshots( + before: IssueSnapshot, + after: IssueSnapshot, +): SnapshotDelta { + // Severity — sorted by canonical order (Critical → Minor; unknown last). + const bySeverity: DeltaEntry[] = diffCountMap(before.bySeverity, after.bySeverity) + .map(({ key, delta }) => ({ + key, + label: SEVERITY_DISPLAY[key] ?? key, + delta, + severity: key as SeverityLevel, + })) + .sort((a, b) => severityRank(a.severity) - severityRank(b.severity)); + + // Category — sorted by magnitude of change. + const byCategory: DeltaEntry[] = diffCountMap(before.byCategory, after.byCategory) + .map(({ key, delta }) => ({ key, label: key, delta })) + .sort(byMagnitude); + + const byPattern = diffPatternMap(before.byPattern, after.byPattern); + + return { + totalBefore: before.total, + totalAfter: after.total, + netTotal: after.total - before.total, + byPattern, + bySeverity, + byCategory, + }; +} + +// ── Polling ────────────────────────────────────────────────────────────────── + +export interface PollOptions { + /** Epoch milliseconds captured when the reanalysis was triggered (t0). */ + triggeredAt: number; + spinner: SpinnerLike; + pollMs?: number; + maxWaitMs?: number; + /** Injectable clock for tests. Defaults to Date.now. */ + now?: () => number; +} + +export interface PollResult { + status: AnalysisStatus; + timedOut: boolean; +} + +/** + * A new analysis is in progress when it started after we triggered the + * reanalysis (`startedAnalysis` more recent than t0) and hasn't finished yet + * (`startedAnalysis` more recent than `endedAnalysis`). + */ +export function isAnalysisInProgress( + status: AnalysisStatus, + triggeredAt: number, +): boolean { + if (!status.startedAnalysis) return false; + const started = parseISO(status.startedAnalysis).getTime(); + if (started <= triggeredAt) return false; + return ( + !status.endedAnalysis || + started > parseISO(status.endedAnalysis).getTime() + ); +} + +/** + * The new analysis is done when it started after t0 and has since finished + * (`endedAnalysis` is at or after `startedAnalysis`). + */ +export function isAnalysisDone( + status: AnalysisStatus, + triggeredAt: number, +): boolean { + if (!status.startedAnalysis || !status.endedAnalysis) return false; + const started = parseISO(status.startedAnalysis).getTime(); + const ended = parseISO(status.endedAnalysis).getTime(); + return started > triggeredAt && ended >= started; +} + +/** + * Two-phase poll for a freshly-triggered reanalysis: + * A) wait until a new analysis is in progress (or has already finished), then + * B) wait until it finishes. + * + * Both transitions are detected from the HEAD commit's analysis timestamps + * relative to `triggeredAt` (see `isAnalysisInProgress` / `isAnalysisDone`). + * Resolves with the latest status, or `timedOut: true` once `maxWaitMs` elapses. + */ +export async function pollForAnalysis( + getStatus: () => Promise, + opts: PollOptions, +): Promise { + const pollMs = opts.pollMs ?? POLL_INTERVAL_MS; + const maxWaitMs = opts.maxWaitMs ?? MAX_WAIT_MS; + const now = opts.now ?? (() => Date.now()); + const { spinner, triggeredAt } = opts; + const startedAt = now(); + const timedOut = () => now() - startedAt > maxWaitMs; + + const inProgress = (s: AnalysisStatus) => isAnalysisInProgress(s, triggeredAt); + const done = (s: AnalysisStatus) => isAnalysisDone(s, triggeredAt); + + spinner.text = "Analysis requested. Waiting for it to start..."; + let status = await getStatus(); + + // Phase A — wait until the new analysis starts (or has already finished). + while (!inProgress(status) && !done(status)) { + if (timedOut()) return { status, timedOut: true }; + await timers.sleep(pollMs); + status = await getStatus(); + } + + // Phase B — analysis is running; wait for it to finish. + if (!done(status)) { + spinner.text = "Analysis in progress. This may take a few minutes..."; + while (!done(status)) { + if (timedOut()) return { status, timedOut: true }; + await timers.sleep(pollMs); + status = await getStatus(); + } + } + + return { status, timedOut: false }; +} + +/** + * Compute the analysis duration in milliseconds from a final status, or null + * if the server didn't report both timestamps. + */ +export function durationFromStatus(status: AnalysisStatus): number | null { + if (!status.startedAnalysis || !status.endedAnalysis) return null; + const ms = + parseISO(status.endedAnalysis).getTime() - + parseISO(status.startedAnalysis).getTime(); + return ms >= 0 ? ms : null; +} + +// ── Rendering ────────────────────────────────────────────────────────────── + +/** Render a signed delta padded so the following label aligns within a section. */ +function deltaCell(value: number, width: number): string { + const raw = `${value > 0 ? "+" : ""}${value}`; + const pad = " ".repeat(Math.max(0, width - raw.length)); + return formatDelta(value) + pad; +} + +function cellWidth(entries: DeltaEntry[]): number { + return entries.reduce((max, e) => { + const raw = `${e.delta > 0 ? "+" : ""}${e.delta}`; + return Math.max(max, raw.length); + }, 0); +} + +/** Print the "By pattern:" section (soft-capped at PATTERN_LIMIT rows). */ +function renderPatternDeltas(entries: DeltaEntry[]): void { + console.log(ansis.bold("By pattern:")); + const width = cellWidth(entries); + const shown = entries.slice(0, PATTERN_LIMIT); + for (const e of shown) { + const annotation = + e.category && e.severity + ? ansis.dim( + ` (${e.category} · ${SEVERITY_DISPLAY[e.severity] ?? e.severity})`, + ) + : ""; + console.log(` ${deltaCell(e.delta, width)} ${e.label}${annotation}`); + } + const more = entries.length - shown.length; + if (more > 0) { + console.log( + ansis.dim(` … (${more} more pattern${more === 1 ? "" : "s"} changed)`), + ); + } + console.log(); +} + +/** Print the "By severity:" section. */ +function renderSeverityDeltas(entries: DeltaEntry[]): void { + console.log(ansis.bold("By severity:")); + const width = cellWidth(entries); + for (const e of entries) { + console.log(` ${deltaCell(e.delta, width)} ${colorSeverity(e.severity!)}`); + } + console.log(); +} + +/** Print the "By category:" section. */ +function renderCategoryDeltas(entries: DeltaEntry[]): void { + console.log(ansis.bold("By category:")); + const width = cellWidth(entries); + for (const e of entries) { + console.log(` ${deltaCell(e.delta, width)} ${e.label}`); + } + console.log(); +} + +/** Print whichever per-dimension sections have changes. */ +function renderDeltaSections(delta: SnapshotDelta): void { + if (delta.byPattern.length > 0) renderPatternDeltas(delta.byPattern); + if (delta.bySeverity.length > 0) renderSeverityDeltas(delta.bySeverity); + if (delta.byCategory.length > 0) renderCategoryDeltas(delta.byCategory); +} + +/** Print the human-readable delta report. */ +export function renderReanalyzeReport( + delta: SnapshotDelta, + durationMs: number | null, +): void { + console.log( + ansis.bold( + durationMs !== null + ? `\nAnalysis finished in ${formatDuration(durationMs)}\n` + : `\nAnalysis finished\n`, + ), + ); + + const hasChanges = + delta.byPattern.length > 0 || + delta.bySeverity.length > 0 || + delta.byCategory.length > 0; + + if (hasChanges) { + renderDeltaSections(delta); + } else { + console.log(ansis.dim("No change in issues.")); + } + + console.log( + `In total: ${delta.totalBefore} → ${delta.totalAfter} issues (net ${formatDelta( + delta.netTotal, + )})`, + ); +} + +/** Build the `--output json` payload for the reanalyze-and-wait report. */ +export function reanalyzeJson( + before: IssueSnapshot, + after: IssueSnapshot, + delta: SnapshotDelta, + durationMs: number | null, +): object { + return { + durationMs, + durationHuman: durationMs !== null ? formatDuration(durationMs) : null, + totals: { + before: before.total, + after: after.total, + net: delta.netTotal, + }, + deltas: { + byPattern: delta.byPattern, + bySeverity: delta.bySeverity, + byCategory: delta.byCategory, + }, + }; +}