From 397bab40659fcb80576fe8ca3cfb053fd59d653a Mon Sep 17 00:00:00 2001 From: Olivier Korach Date: Wed, 29 Apr 2026 11:58:23 +0200 Subject: [PATCH] fix: skip closed/fixed issues during migration (#133) Closed and fixed issues from SonarQube were being recreated on the SonarCloud destination with imaginary line numbers (sometimes beyond the files actual length). They should not be migrated at all. Filter at three layers across all pipeline versions (sq-9.9, sq-10.0, sq-10.4, sq-2025): - API search: drop FIXED from issueStatuses so closed issues are not fetched - Protobuf build: skip closed/fixed issues when building the migration payload - Issue sync: skip closed/fixed issues so transitions are not propagated Centralized the predicate in src/shared/utils/issue-filters/is-closed-or-fixed.js covering both modern (CLOSED/FIXED) and classic (RESOLVED + resolution=FIXED) lifecycles. Also fix syncIssues so the closed/fixed filter is preserved when sqClient is provided (the manual-changes pre-filter branch was reusing the unfiltered sqIssues). Update the syncIssues CLOSED-status test to assert no transition is propagated. Closes #133 Co-Authored-By: Claude Opus 4.7 --- src/pipelines/sq-10.0/protobuf/build-issues/index.js | 2 ++ .../sq-10.0/sonarcloud/migrators/issue-sync/index.js | 9 +++++++-- .../sq-10.0/sonarqube/api/issues-hotspots.js | 5 ++++- .../protobuf/build-issues/helpers/build-issues.js | 2 ++ .../migrators/issue-sync/helpers/sync-issues.js | 9 +++++++-- .../api-client/helpers/create-sonarqube-client.js | 2 ++ .../sonarqube/api-client/helpers/handle-api-error.js | 2 +- .../sq-10.4/sonarqube/api/issues-hotspots.js | 4 ++-- src/pipelines/sq-2025/protobuf/build-issues/index.js | 2 ++ .../sq-2025/sonarcloud/migrators/issue-sync/index.js | 9 +++++++-- .../sonarqube/api-client/helpers/issue-methods.js | 3 ++- .../sq-2025/sonarqube/api/issues-hotspots.js | 3 ++- src/pipelines/sq-9.9/protobuf/build-issues/index.js | 2 ++ .../migrators/issue-sync/helpers/sync-issues.js | 9 +++++++-- src/pipelines/sq-9.9/sonarqube/api/issues-hotspots.js | 5 ++++- src/shared/utils/issue-filters/is-closed-or-fixed.js | 11 +++++++++++ test/sonarcloud/migrators/migrators.test.js | 6 +++--- 17 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 src/shared/utils/issue-filters/is-closed-or-fixed.js diff --git a/src/pipelines/sq-10.0/protobuf/build-issues/index.js b/src/pipelines/sq-10.0/protobuf/build-issues/index.js index 82ad00b6..419e515b 100644 --- a/src/pipelines/sq-10.0/protobuf/build-issues/index.js +++ b/src/pipelines/sq-10.0/protobuf/build-issues/index.js @@ -2,6 +2,7 @@ import logger from '../../../../shared/utils/logger.js'; import { isExternalIssue } from '../build-external-issues.js'; +import { isClosedOrFixed } from '../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { buildIssueMessage } from './helpers/build-issue-message.js'; export function buildIssues(builder) { @@ -12,6 +13,7 @@ export function buildIssues(builder) { let skippedIssues = 0; builder.data.issues.forEach(issue => { + if (isClosedOrFixed(issue)) return; if (isExternalIssue(issue, sonarCloudRepos)) return; if (!builder.validComponentKeys?.has(issue.component)) { diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/index.js b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/index.js index 8d5b5622..b5dd81e4 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/index.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/index.js @@ -1,5 +1,6 @@ import logger from '../../../../../shared/utils/logger.js'; import { mapConcurrent, createProgressLogger } from '../../../../../shared/utils/concurrency.js'; +import { isClosedOrFixed } from '../../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { applyManualChangesPreFilter } from '../../../../../shared/utils/issue-sync/apply-pre-filter.js'; import { waitForScIndexing } from '../../../../../shared/utils/issue-sync/wait-for-sc-indexing.js'; import { matchIssues } from './helpers/match-issues.js'; @@ -19,11 +20,15 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { const userMappings = options.userMappings || null; const stats = createEmptyStats(); - let issuesToSync = sqIssues; + const beforeFilter = sqIssues.length; + let issuesToSync = sqIssues.filter(i => !isClosedOrFixed(i)); + if (issuesToSync.length < beforeFilter) { + logger.info(`Filtered out ${beforeFilter - issuesToSync.length} closed/fixed issues`); + } let changelogMap = new Map(); if (sqClient) { - ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(sqIssues, sqClient, stats, concurrency)); + ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(issuesToSync, sqClient, stats, concurrency)); if (issuesToSync.length === 0) { logSyncSummary(stats); return stats; } } diff --git a/src/pipelines/sq-10.0/sonarqube/api/issues-hotspots.js b/src/pipelines/sq-10.0/sonarqube/api/issues-hotspots.js index 195fc4ab..a0073a23 100644 --- a/src/pipelines/sq-10.0/sonarqube/api/issues-hotspots.js +++ b/src/pipelines/sq-10.0/sonarqube/api/issues-hotspots.js @@ -3,7 +3,10 @@ import { fetchWithSlicing } from '../../../../shared/utils/search-slicer/index.j // All issue statuses — pre-10.4 lifecycle + 10.4+ lifecycle. // The SonarQube API ignores unknown values, so including both sets is safe. -const ALL_STATUSES = 'OPEN,CONFIRMED,REOPENED,RESOLVED,CLOSED,FALSE_POSITIVE,ACCEPTED,FIXED'; +// CLOSED and FIXED are excluded — resolved issues should not be recreated on the destination. +// RESOLVED is kept because it includes FALSE-POSITIVE and WONTFIX resolutions +// that should be migrated (downstream filter handles resolution=FIXED). +const ALL_STATUSES = 'OPEN,CONFIRMED,REOPENED,RESOLVED,FALSE_POSITIVE,ACCEPTED'; export async function getIssues(probeTotal, getPaginated, projectKey, filters = {}) { logger.info(`Fetching issues for project: ${projectKey}`); diff --git a/src/pipelines/sq-10.4/protobuf/build-issues/helpers/build-issues.js b/src/pipelines/sq-10.4/protobuf/build-issues/helpers/build-issues.js index 32684067..2c7ccd75 100644 --- a/src/pipelines/sq-10.4/protobuf/build-issues/helpers/build-issues.js +++ b/src/pipelines/sq-10.4/protobuf/build-issues/helpers/build-issues.js @@ -1,5 +1,6 @@ import logger from '../../../../../shared/utils/logger.js'; import { isExternalIssue } from '../../build-external-issues.js'; +import { isClosedOrFixed } from '../../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { buildIssueMessage } from './build-issue-message.js'; // -------- Main Logic -------- @@ -13,6 +14,7 @@ export function buildIssues(builder) { let skippedIssues = 0; builder.data.issues.forEach(issue => { + if (isClosedOrFixed(issue)) return; if (isExternalIssue(issue, sonarCloudRepos)) return; if (!builder.validComponentKeys?.has(issue.component)) { diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issues.js b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issues.js index 752b55d4..39ac9835 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issues.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issues.js @@ -3,6 +3,7 @@ import { syncSingleIssue } from './sync-single-issue.js'; import { createSyncStats } from './create-sync-stats.js'; import { logSyncStats } from './log-sync-stats.js'; import { mapConcurrent, createProgressLogger } from '../../../../../../shared/utils/concurrency.js'; +import { isClosedOrFixed } from '../../../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { applyManualChangesPreFilter } from '../../../../../../shared/utils/issue-sync/apply-pre-filter.js'; import { waitForScIndexing } from '../../../../../../shared/utils/issue-sync/wait-for-sc-indexing.js'; import logger from '../../../../../../shared/utils/logger.js'; @@ -19,11 +20,15 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { const userMappings = options.userMappings || null; const stats = createSyncStats(); - let issuesToSync = sqIssues; + const beforeFilter = sqIssues.length; + let issuesToSync = sqIssues.filter(i => !isClosedOrFixed(i)); + if (issuesToSync.length < beforeFilter) { + logger.info(`Filtered out ${beforeFilter - issuesToSync.length} closed/fixed issues`); + } let changelogMap = new Map(); if (sqClient) { - ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(sqIssues, sqClient, stats, concurrency)); + ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(issuesToSync, sqClient, stats, concurrency)); if (issuesToSync.length === 0) { logSyncStats(stats); return stats; } } diff --git a/src/pipelines/sq-10.4/sonarqube/api-client/helpers/create-sonarqube-client.js b/src/pipelines/sq-10.4/sonarqube/api-client/helpers/create-sonarqube-client.js index 536ecfe9..f8399513 100644 --- a/src/pipelines/sq-10.4/sonarqube/api-client/helpers/create-sonarqube-client.js +++ b/src/pipelines/sq-10.4/sonarqube/api-client/helpers/create-sonarqube-client.js @@ -7,6 +7,7 @@ import { buildMeasureMethods } from './measure-methods.js'; import { buildSourceMethods } from './source-methods.js'; import { buildConnectionMethods } from './connection-methods.js'; import { buildDelegateMethods } from './delegate-methods.js'; +import logger from '../../../../../shared/utils/logger.js'; // -------- Main Logic -------- @@ -14,6 +15,7 @@ import { buildDelegateMethods } from './delegate-methods.js'; export function createSonarQubeClient(config) { const baseURL = config.url.replace(/\/$/, ''); const client = createHttpClient(baseURL, config.token); + logger.info(`OKO Created SonarQube client for ${baseURL} with token: ${client.token}`); const projectKey = config.projectKey; const gp = (endpoint, params, dataKey) => getPaginated(client, endpoint, params, dataKey); diff --git a/src/pipelines/sq-10.4/sonarqube/api-client/helpers/handle-api-error.js b/src/pipelines/sq-10.4/sonarqube/api-client/helpers/handle-api-error.js index ae7dbcb5..b2606a38 100644 --- a/src/pipelines/sq-10.4/sonarqube/api-client/helpers/handle-api-error.js +++ b/src/pipelines/sq-10.4/sonarqube/api-client/helpers/handle-api-error.js @@ -7,7 +7,7 @@ export function handleApiError(error, baseURL) { if (error.response) { const { status, data, config } = error.response; if (status === 401 || status === 403) { - throw new AuthenticationError(`Authentication failed for SonarQube: ${data.errors?.[0]?.msg || 'Invalid credentials'}`, 'SonarQube'); + throw new AuthenticationError(`Authentication failed for SonarQube: DATA = ${data} - ${data.errors?.[0]?.msg || 'Invalid credentials I believe'}`, 'SonarQube'); } const message = data.errors?.[0]?.msg || data.message || error.message; throw new SonarQubeAPIError(`SonarQube API error (${status}): ${message}`, status, config.url); diff --git a/src/pipelines/sq-10.4/sonarqube/api/issues-hotspots.js b/src/pipelines/sq-10.4/sonarqube/api/issues-hotspots.js index e8b36524..a5cc6a11 100644 --- a/src/pipelines/sq-10.4/sonarqube/api/issues-hotspots.js +++ b/src/pipelines/sq-10.4/sonarqube/api/issues-hotspots.js @@ -2,8 +2,8 @@ import logger from '../../../../shared/utils/logger.js'; import { fetchWithSlicing } from '../../../../shared/utils/search-slicer/index.js'; // 10.4+ uses the `issueStatuses` parameter with the modern lifecycle values. -// Include CLOSED so that closed issues are also extracted for migration. -const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,FIXED,CLOSED'; +// FIXED and CLOSED are excluded — resolved issues should not be recreated on the destination. +const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED'; export async function getIssues(probeTotal, getPaginated, projectKey, filters = {}) { logger.info(`Fetching issues for project: ${projectKey}`); diff --git a/src/pipelines/sq-2025/protobuf/build-issues/index.js b/src/pipelines/sq-2025/protobuf/build-issues/index.js index 96974907..d8caa6ff 100644 --- a/src/pipelines/sq-2025/protobuf/build-issues/index.js +++ b/src/pipelines/sq-2025/protobuf/build-issues/index.js @@ -1,5 +1,6 @@ import logger from '../../../../shared/utils/logger.js'; import { isExternalIssue } from '../build-external-issues.js'; +import { isClosedOrFixed } from '../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { buildSingleIssue } from './helpers/build-single-issue.js'; // -------- Build Issues -------- @@ -12,6 +13,7 @@ export function buildIssues(builder) { let skippedIssues = 0; builder.data.issues.forEach(issue => { + if (isClosedOrFixed(issue)) return; if (isExternalIssue(issue, sonarCloudRepos)) return; if (!builder.validComponentKeys?.has(issue.component)) { skippedIssues++; return; } diff --git a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js index 7a0b8ab0..e9b18802 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js @@ -1,6 +1,7 @@ import logger from '../../../../../shared/utils/logger.js'; import { mapConcurrent, createProgressLogger } from '../../../../../shared/utils/concurrency.js'; import { parallelSyncIssues } from '../../../../../shared/utils/concurrency/helpers/parallel-issue-sync.js'; +import { isClosedOrFixed } from '../../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { applyManualChangesPreFilter } from '../../../../../shared/utils/issue-sync/apply-pre-filter.js'; import { waitForScIndexing } from '../../../../../shared/utils/issue-sync/wait-for-sc-indexing.js'; import { matchIssues } from './helpers/match-issues.js'; @@ -24,11 +25,15 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { const userMappings = options.userMappings || null; const stats = createSyncStats(); - let issuesToSync = sqIssues; + const beforeFilter = sqIssues.length; + let issuesToSync = sqIssues.filter(i => !isClosedOrFixed(i)); + if (issuesToSync.length < beforeFilter) { + logger.info(`Filtered out ${beforeFilter - issuesToSync.length} closed/fixed issues`); + } let changelogMap = new Map(); if (sqClient) { - ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(sqIssues, sqClient, stats, concurrency)); + ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(issuesToSync, sqClient, stats, concurrency)); if (issuesToSync.length === 0) { logSyncSummary(stats); return stats; } } diff --git a/src/pipelines/sq-2025/sonarqube/api-client/helpers/issue-methods.js b/src/pipelines/sq-2025/sonarqube/api-client/helpers/issue-methods.js index c7fdabfd..9d86ce8a 100644 --- a/src/pipelines/sq-2025/sonarqube/api-client/helpers/issue-methods.js +++ b/src/pipelines/sq-2025/sonarqube/api-client/helpers/issue-methods.js @@ -4,7 +4,8 @@ import { probeTotal } from './probe-total.js'; // -------- Issue Methods -------- -const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,FIXED,IN_SANDBOX'; +// FIXED is excluded — resolved issues should not be recreated on the destination. +const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,IN_SANDBOX'; /** Attach issue and duplication methods to the client instance. */ export function attachIssueMethods(inst) { diff --git a/src/pipelines/sq-2025/sonarqube/api/issues-hotspots.js b/src/pipelines/sq-2025/sonarqube/api/issues-hotspots.js index cd15f39e..30c0a6d1 100644 --- a/src/pipelines/sq-2025/sonarqube/api/issues-hotspots.js +++ b/src/pipelines/sq-2025/sonarqube/api/issues-hotspots.js @@ -2,7 +2,8 @@ import logger from '../../../../shared/utils/logger.js'; import { fetchWithSlicing } from '../../../../shared/utils/search-slicer/index.js'; // 2025.x uses the `issueStatuses` parameter with the modern lifecycle values. -const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,FIXED,IN_SANDBOX'; +// FIXED is excluded — resolved issues should not be recreated on the destination. +const ISSUE_STATUSES = 'OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,IN_SANDBOX'; export async function getIssues(probeTotal, getPaginated, projectKey, filters = {}) { logger.info(`Fetching issues for project: ${projectKey}`); diff --git a/src/pipelines/sq-9.9/protobuf/build-issues/index.js b/src/pipelines/sq-9.9/protobuf/build-issues/index.js index ed593c65..3720f8ef 100644 --- a/src/pipelines/sq-9.9/protobuf/build-issues/index.js +++ b/src/pipelines/sq-9.9/protobuf/build-issues/index.js @@ -1,5 +1,6 @@ import logger from '../../../../shared/utils/logger.js'; import { isExternalIssue } from '../build-external-issues.js'; +import { isClosedOrFixed } from '../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; // -------- Build Issue Messages -------- @@ -10,6 +11,7 @@ export function buildIssues(builder) { let skippedIssues = 0; builder.data.issues.forEach(issue => { + if (isClosedOrFixed(issue)) return; if (isExternalIssue(issue, sonarCloudRepos)) return; if (!builder.validComponentKeys?.has(issue.component)) { skippedIssues++; return; } const componentRef = builder.componentRefMap.get(issue.component); diff --git a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issues.js b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issues.js index a1d7783e..ce79eca8 100644 --- a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issues.js +++ b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issues.js @@ -1,5 +1,6 @@ import logger from '../../../../../../shared/utils/logger.js'; import { mapConcurrent, createProgressLogger } from '../../../../../../shared/utils/concurrency.js'; +import { isClosedOrFixed } from '../../../../../../shared/utils/issue-filters/is-closed-or-fixed.js'; import { applyManualChangesPreFilter } from '../../../../../../shared/utils/issue-sync/apply-pre-filter.js'; import { waitForScIndexing } from '../../../../../../shared/utils/issue-sync/wait-for-sc-indexing.js'; import { matchIssues } from './match-issues.js'; @@ -17,11 +18,15 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { const userMappings = options.userMappings || null; const stats = createEmptyStats(); - let issuesToSync = sqIssues; + const beforeFilter = sqIssues.length; + let issuesToSync = sqIssues.filter(i => !isClosedOrFixed(i)); + if (issuesToSync.length < beforeFilter) { + logger.info(`Filtered out ${beforeFilter - issuesToSync.length} closed/fixed issues`); + } let changelogMap = new Map(); if (sqClient) { - ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(sqIssues, sqClient, stats, concurrency)); + ({ issuesToSync, changelogMap } = await applyManualChangesPreFilter(issuesToSync, sqClient, stats, concurrency)); if (issuesToSync.length === 0) { logSyncStats(stats); return stats; } } diff --git a/src/pipelines/sq-9.9/sonarqube/api/issues-hotspots.js b/src/pipelines/sq-9.9/sonarqube/api/issues-hotspots.js index 50a95a53..d11861d5 100644 --- a/src/pipelines/sq-9.9/sonarqube/api/issues-hotspots.js +++ b/src/pipelines/sq-9.9/sonarqube/api/issues-hotspots.js @@ -3,7 +3,10 @@ import { fetchWithSlicing } from '../../../../shared/utils/search-slicer/index.j // SonarQube 9.9 only supports the classic issue lifecycle statuses. // FALSE_POSITIVE, ACCEPTED, FIXED are NOT valid in 9.9 (causes 400 error). -const ALL_STATUSES = 'OPEN,CONFIRMED,REOPENED,RESOLVED,CLOSED'; +// CLOSED is excluded — resolved issues should not be recreated on the destination. +// RESOLVED is kept because it includes FALSE-POSITIVE and WONTFIX resolutions +// that should be migrated (downstream filter handles resolution=FIXED). +const ALL_STATUSES = 'OPEN,CONFIRMED,REOPENED,RESOLVED'; export async function getIssues(probeTotal, getPaginated, projectKey, filters = {}) { logger.info(`Fetching issues for project: ${projectKey}`); diff --git a/src/shared/utils/issue-filters/is-closed-or-fixed.js b/src/shared/utils/issue-filters/is-closed-or-fixed.js new file mode 100644 index 00000000..1786da63 --- /dev/null +++ b/src/shared/utils/issue-filters/is-closed-or-fixed.js @@ -0,0 +1,11 @@ +/** + * Returns true if the issue is closed or fixed and should be excluded from migration. + * Covers all lifecycle variants: + * - Modern lifecycle: explicit CLOSED or FIXED status + * - Classic lifecycle: RESOLVED status with resolution=FIXED + */ +export function isClosedOrFixed(issue) { + if (issue.status === 'CLOSED' || issue.status === 'FIXED') return true; + if (issue.status === 'RESOLVED' && issue.resolution === 'FIXED') return true; + return false; +} diff --git a/test/sonarcloud/migrators/migrators.test.js b/test/sonarcloud/migrators/migrators.test.js index 0c5f8823..db97db27 100644 --- a/test/sonarcloud/migrators/migrators.test.js +++ b/test/sonarcloud/migrators/migrators.test.js @@ -1696,7 +1696,7 @@ test('syncIssues handles RESOLVED status', async t => { t.is(client.transitionIssue.firstCall.args[1], 'resolve'); }); -test('syncIssues handles CLOSED status', async t => { +test('syncIssues filters out CLOSED issues (no transition propagation)', async t => { const client = mockClient({ searchIssues: sinon.stub().resolves([ { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'OPEN' } @@ -1715,8 +1715,8 @@ test('syncIssues handles CLOSED status', async t => { const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1 }); - t.is(stats.transitioned, 1); - t.is(client.transitionIssue.firstCall.args[1], 'resolve'); + t.is(stats.transitioned, 0); + t.false(client.transitionIssue.called); }); test('syncIssues handles ACCEPTED status', async t => {