diff --git a/src/index.ts b/src/index.ts index 4a2ffec9..0009e77b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -548,6 +548,12 @@ if (parsedArgs) { to: entry.to, advisories: collectAdvisoryIdsForPackage(findingsBeforeFixList, entry.package), })), + notAutoApplied: { + withinRangeRefreshCount: fixResult?.withinRangeRefreshCount ?? 0, + parentUpgradeCount: fixResult?.parentUpgradeCount ?? 0, + breakingUpgradeCount: fixResult?.breakingUpgradeCount ?? 0, + noFixCount: fixResult?.noFixCount ?? 0, + }, }; writeFixResultJson(fixResultJson, projectPath); diff --git a/src/utils/fix-runner.ts b/src/utils/fix-runner.ts index 635a2b66..bc9f29cb 100644 --- a/src/utils/fix-runner.ts +++ b/src/utils/fix-runner.ts @@ -6,6 +6,7 @@ import { chalk } from "./chalk.js"; import type { ParsedOptions } from "../types.js"; import type { SeverityLabel } from "../types.js"; import { pluralize } from "./string.js"; +import { isMajorVersionBump } from "./version.js"; import type { SuggestedFixCommandPlan, SuggestedFixTarget } from "../remediation/fix-commands.js"; import type { DebugLogger } from "../output/debug.js"; @@ -61,13 +62,56 @@ export function commandLabelForPackageManager(packageManager: SuggestedFixComman return "yarn add"; } +export type UnappliedBreakdown = { + // Safe within-range lockfile refreshes (parent-update): can be run as-is. + withinRangeRefreshCount: number; + // Parent upgrades (parent-upgrade): need review before applying. + parentUpgradeCount: number; + // Subset of parentUpgradeCount that cross a major version (breaking). + breakingUpgradeCount: number; + // Findings with no available fix (skip records). + noFixCount: number; +}; + export type FixExecutionResult = { appliedFixCount: number; - skippedCount: number; - skippedTransitiveCount: number; - skippedNoValidatedTargetCount: number; applied: Array<{ package: string; from: string; to: string }>; note: string | null; +} & UnappliedBreakdown; + +// Categorize the fixes that --fix does not auto-apply so the summary can be +// honest about which are safe to run (within-range refreshes), which need +// review (parent upgrades, breaking ones especially), and which have no fix. +// Applied direct targets are excluded (only parent-* targets are counted). +export function categorizeUnappliedTargets(plan: SuggestedFixCommandPlan): UnappliedBreakdown { + let withinRangeRefreshCount = 0; + let parentUpgradeCount = 0; + let breakingUpgradeCount = 0; + + for (const target of plan.targets) { + if (target.kind === "parent-update") { + withinRangeRefreshCount += 1; + } else if (target.kind === "parent-upgrade") { + parentUpgradeCount += 1; + if (target.currentVersion && isMajorVersionBump(target.currentVersion, target.targetVersion)) { + breakingUpgradeCount += 1; + } + } + } + + return { + withinRangeRefreshCount, + parentUpgradeCount, + breakingUpgradeCount, + noFixCount: plan.skipped.length, + }; +} + +const EMPTY_UNAPPLIED: UnappliedBreakdown = { + withinRangeRefreshCount: 0, + parentUpgradeCount: 0, + breakingUpgradeCount: 0, + noFixCount: 0, }; export type FixResultJson = { @@ -80,6 +124,10 @@ export type FixResultJson = { to: string; advisories: string[]; }>; + // Structured breakdown of fixes that were not auto-applied. Additive - existing + // consumers reading appliedFixCount / findingsBeforeFix / findingsAfterFix / applied + // are unaffected. + notAutoApplied: UnappliedBreakdown; }; export function writeFixResultJson(json: FixResultJson, projectPath: string): void { @@ -100,27 +148,21 @@ export async function applyFixesIfRequested(params: { if (!params.plan) { return { appliedFixCount: 0, - skippedCount: params.totalFindings, - skippedTransitiveCount: 0, - skippedNoValidatedTargetCount: params.totalFindings, + ...EMPTY_UNAPPLIED, applied: [], note: "No package-manager-native fix command is available for this project.", }; } const directTargets = params.plan.targets.filter(target => target.kind === "direct"); - const transitiveTargets = params.plan.targets.filter(target => target.kind !== "direct"); - const skippedDirect = params.plan.skipped.filter(skip => skip.relationship === "direct" || skip.relationship === "unknown"); + const unapplied = categorizeUnappliedTargets(params.plan); if (directTargets.length === 0) { - const skippedCount = transitiveTargets.length + skippedDirect.length; return { appliedFixCount: 0, - skippedCount, - skippedTransitiveCount: transitiveTargets.length, - skippedNoValidatedTargetCount: skippedDirect.length, + ...unapplied, applied: [], - note: "No validated direct dependency fixes were eligible for auto-apply.", + note: "No fixes were auto-applied. `--fix` only auto-applies safe, in-range direct-dependency upgrades. Review the Suggested Fix Plan above and run the remaining fixes manually.", }; } @@ -152,12 +194,9 @@ export async function applyFixesIfRequested(params: { } spinner.succeed(`Applied ${directTargets.length} direct package ${pluralize(directTargets.length, "fix", "fixes")} with ${commandLabel}`); - const skippedCount = transitiveTargets.length + skippedDirect.length; return { appliedFixCount: directTargets.length, - skippedCount, - skippedTransitiveCount: transitiveTargets.length, - skippedNoValidatedTargetCount: skippedDirect.length, + ...unapplied, applied: directTargets.map(target => ({ package: target.package, from: target.currentVersion ?? "unknown", @@ -192,10 +231,21 @@ export function printFixModeSummary(params: { console.log(""); console.log(chalk.bold.cyan("Fix summary")); console.log(`- Applied fixes: ${chalk.green(String(result.appliedFixCount))}`); - console.log(`- Skipped findings: ${chalk.yellow(String(result.skippedCount))}`); - if (result.skippedCount > 0) { - console.log(` - Transitive (v1 skip): ${chalk.yellow(String(result.skippedTransitiveCount))}`); - console.log(` - No validated direct target: ${chalk.yellow(String(result.skippedNoValidatedTargetCount))}`); + + const notAutoApplied = + result.withinRangeRefreshCount + result.parentUpgradeCount + result.noFixCount; + console.log(`- Not auto-applied: ${chalk.yellow(String(notAutoApplied))}`); + if (result.parentUpgradeCount > 0) { + const breakingNote = result.breakingUpgradeCount > 0 + ? `, ${result.breakingUpgradeCount} breaking` + : ""; + console.log(` - Parent upgrades (review + test): ${chalk.yellow(String(result.parentUpgradeCount))}${chalk.gray(breakingNote)}`); + } + if (result.withinRangeRefreshCount > 0) { + console.log(` - Within-range refreshes (safe to run): ${chalk.yellow(String(result.withinRangeRefreshCount))}`); + } + if (result.noFixCount > 0) { + console.log(` - No fix available: ${chalk.yellow(String(result.noFixCount))}`); } console.log(`- Findings before fix: ${chalk.white(String(params.findingsBeforeFix))}`); console.log(`- Remaining findings after fix: ${chalk.white(String(params.findingsAfterFix))}`); diff --git a/tests/create-pr.test.ts b/tests/create-pr.test.ts index f3ea14f9..2743cfbb 100644 --- a/tests/create-pr.test.ts +++ b/tests/create-pr.test.ts @@ -116,9 +116,10 @@ describe("create-pr helpers", () => { const body = buildPullRequestBody({ fixResult: { appliedFixCount: 1, - skippedCount: 0, - skippedTransitiveCount: 0, - skippedNoValidatedTargetCount: 0, + withinRangeRefreshCount: 0, + parentUpgradeCount: 0, + breakingUpgradeCount: 0, + noFixCount: 0, applied: [{ package: "lodash", from: "4.17.20", to: "4.17.21" }], note: null, }, @@ -140,9 +141,10 @@ describe("create-pr helpers", () => { const body = buildPullRequestBody({ fixResult: { appliedFixCount: 1, - skippedCount: 0, - skippedTransitiveCount: 0, - skippedNoValidatedTargetCount: 0, + withinRangeRefreshCount: 0, + parentUpgradeCount: 0, + breakingUpgradeCount: 0, + noFixCount: 0, applied: [{ package: "lodash", from: "4.17.20", to: "4.17.21" }], note: null, }, diff --git a/tests/fix-runner-summary.test.ts b/tests/fix-runner-summary.test.ts new file mode 100644 index 00000000..929150dc --- /dev/null +++ b/tests/fix-runner-summary.test.ts @@ -0,0 +1,124 @@ +import { jest } from "@jest/globals"; +import { categorizeUnappliedTargets, printFixModeSummary } from "../src/utils/fix-runner.js"; +import type { FixExecutionResult } from "../src/utils/fix-runner.js"; +import type { SuggestedFixCommandPlan, SuggestedFixTarget } from "../src/remediation/fix-commands.js"; +import type { SeverityLabel } from "../src/types.js"; + +function target(overrides: Partial): SuggestedFixTarget { + return { + package: "pkg", + currentVersion: "1.0.0", + targetVersion: "1.0.1", + kind: "direct", + urgent: true, + severity: "high", + adjusted: false, + adjustmentNote: null, + reason: "", + usage: null, + ...overrides, + }; +} + +function plan(targets: SuggestedFixTarget[], skippedCount: number): SuggestedFixCommandPlan { + return { + packageManager: "npm", + sourceLabel: "package-lock.json", + command: null, + sections: [], + targets, + skipped: Array.from({ length: skippedCount }, (_, i) => ({ + package: `skip-${i}`, + version: "1.0.0", + relationship: "transitive" as const, + reason: "no fix", + })), + coveredFindingCount: 0, + totalFindingCount: 0, + }; +} + +describe("categorizeUnappliedTargets", () => { + it("counts within-range refreshes (parent-update) separately from parent upgrades", () => { + const result = categorizeUnappliedTargets(plan([ + target({ kind: "parent-update", package: "refresh-a" }), + target({ kind: "parent-update", package: "refresh-b" }), + target({ kind: "parent-upgrade", package: "up", currentVersion: "1.0.0", targetVersion: "1.2.0" }), + ], 0)); + + expect(result.withinRangeRefreshCount).toBe(2); + expect(result.parentUpgradeCount).toBe(1); + }); + + it("counts breaking major-bump parent upgrades as a subset of parent upgrades", () => { + const result = categorizeUnappliedTargets(plan([ + target({ kind: "parent-upgrade", package: "major", currentVersion: "21.2.6", targetVersion: "22.1.0" }), + target({ kind: "parent-upgrade", package: "minor", currentVersion: "9.2.1", targetVersion: "9.2.4" }), + ], 0)); + + expect(result.parentUpgradeCount).toBe(2); + expect(result.breakingUpgradeCount).toBe(1); + }); + + it("counts skip records as no-fix and excludes applied direct targets", () => { + const result = categorizeUnappliedTargets(plan([ + target({ kind: "direct", package: "applied-direct" }), + target({ kind: "parent-update", package: "refresh" }), + ], 3)); + + expect(result.noFixCount).toBe(3); + expect(result.withinRangeRefreshCount).toBe(1); + expect(result.parentUpgradeCount).toBe(0); + expect(result.breakingUpgradeCount).toBe(0); + }); +}); + +describe("printFixModeSummary", () => { + const noSeverity: Record = { critical: 0, high: 0, medium: 0, low: 0, unknown: 0 }; + + function capture(result: FixExecutionResult, before: number, after: number): string { + const spy = jest.spyOn(console, "log").mockImplementation(() => {}); + try { + printFixModeSummary({ fixResult: result, findingsBeforeFix: before, findingsAfterFix: after, remainingBySeverity: noSeverity }); + return spy.mock.calls.map(c => String(c[0])).join("\n"); + } finally { + spy.mockRestore(); + } + } + + it("renders an honest categorized breakdown with no 'v1 skip' jargon", () => { + const out = capture({ + appliedFixCount: 0, + applied: [], + note: null, + withinRangeRefreshCount: 3, + parentUpgradeCount: 27, + breakingUpgradeCount: 27, + noFixCount: 1, + }, 45, 45); + + expect(out).toContain("Not auto-applied: 31"); + expect(out).toContain("Parent upgrades (review + test): 27"); + expect(out).toContain("27 breaking"); + expect(out).toContain("Within-range refreshes (safe to run): 3"); + expect(out).toContain("No fix available: 1"); + expect(out).not.toContain("v1 skip"); + expect(out).not.toContain("Skipped findings"); + }); + + it("omits the breaking note when no upgrade is a major bump", () => { + const out = capture({ + appliedFixCount: 1, + applied: [{ package: "axios", from: "0.21.1", to: "0.33.0" }], + note: null, + withinRangeRefreshCount: 0, + parentUpgradeCount: 2, + breakingUpgradeCount: 0, + noFixCount: 0, + }, 3, 2); + + expect(out).toContain("Parent upgrades (review + test): 2"); + expect(out).not.toContain("breaking"); + expect(out).not.toContain("Within-range refreshes"); + }); +});