From de1b56464f97882f9efceeb283bc5bfd98836165 Mon Sep 17 00:00:00 2001 From: Olivier Korach Date: Wed, 29 Apr 2026 14:43:51 +0200 Subject: [PATCH 1/2] fix: target issue ends in current source status, not first changelog state (#140) When a SonarQube issue had been transitioned multiple times (e.g. OPEN -> FIXED -> OPEN), the migrated issue on SonarCloud ended up in the *first* changed status instead of the current one. Cause: syncIssueStatus replayed every changelog transition in API response order. Each transition was applied via client.transitionIssue, but SonarCloud rejected most of them with state-machine errors (those rejections were swallowed silently). The first transition that SonarCloud accepted from its OPEN starting state stuck, regardless of what came after. Fix: skip the changelog replay entirely. syncIssueStatus now applies a single transition derived from the source issue's current status/resolution (applyFallbackTransition's logic). This is what the user-facing requirement asks for - target issue should reflect the *current* source state, not its history - and avoids the silent rejection problem completely. Also add a 'FIXED' case to getFallbackTransition for SonarQube 2025.x modern lifecycle, where FIXED is a direct status value rather than a resolution. Defense in depth: closed/fixed issues are already filtered upstream by the #133 filter, but the case ensures the fallback is correct if anything ever bypasses it. Applied across all four pipelines (sq-9.9, sq-10.0, sq-10.4, sq-2025) and the shared parallel-issue-sync worker. Removed the now-dead changelog plumbing (changelogMap parameter, embedded extractTransitionsFromChangelog/ mapChangelogDiffToTransition copies) from the worker and parallelSyncIssues signature. The shared apply-pre-filter still fetches changelogs - they're used by hasManualChanges to filter out issues with no manual changes. mapChangelogDiffToTransition and extractTransitionsFromChangelog remain exported from each pipeline's issue-sync module - verify-issue-pair.js still uses them for status-history verification reporting. Tests: replaced five replay-related tests in test/sonarcloud/migrators/migrators.test.js that asserted multi-transition replay (and were already failing on main due to the manual-changes prefilter dropping issues with no user field on the changelog). The replacements add user fields to the changelog fixtures so the prefilter keeps the issues, and assert the new single-transition behavior. Added a regression test covering the exact #140 scenario. Closes #140 Co-Authored-By: Claude Opus 4.7 --- .../helpers/get-fallback-transition.js | 3 +- .../issue-sync/helpers/sync-issue-status.js | 26 ++--- .../issue-sync/helpers/sync-single-issue.js | 3 +- .../issue-sync/helpers/transition-mapping.js | 2 +- .../helpers/get-fallback-transition.js | 3 +- .../helpers/get-fallback-transition.js | 3 +- .../issue-sync/helpers/sync-issue-status.js | 31 +---- .../issue-sync/helpers/sync-single-issue.js | 3 +- .../helpers/get-fallback-transition.js | 3 +- .../issue-sync/helpers/sync-issue-status.js | 36 +----- .../issue-sync/helpers/sync-one-issue.js | 3 +- .../sonarcloud/migrators/issue-sync/index.js | 2 +- .../helpers/get-fallback-transition.js | 3 +- .../helpers/get-fallback-transition.js | 3 +- .../issue-sync/helpers/sync-issue-status.js | 33 +----- .../issue-sync/helpers/sync-issues.js | 3 +- .../helpers/parallel-issue-sync.js | 71 ++---------- test/sonarcloud/migrators/migrators.test.js | 106 ++++-------------- 18 files changed, 67 insertions(+), 270 deletions(-) diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js index c8df2ab1..5f0c74c2 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js @@ -13,7 +13,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js index 0a0a44d8..46a2bb4d 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js @@ -1,27 +1,15 @@ import logger from '../../../../../../shared/utils/logger.js'; -import { extractTransitionsFromChangelog, getFallbackTransition } from './transition-mapping.js'; +import { getFallbackTransition } from './transition-mapping.js'; // -------- Sync Issue Status -------- -export async function syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog) { +/** + * Sync the SC issue's status to match the current SQ issue status. + * Applies a single transition derived from the SQ issue's current state. + */ +export async function syncIssueStatus(scIssue, sqIssue, client) { if (scIssue.status === sqIssue.status) return false; - if (sqClient) { - try { - const changelog = preloadedChangelog ?? await sqClient.getIssueChangelog(sqIssue.key); - const transitions = extractTransitionsFromChangelog(changelog); - if (transitions.length === 0) return await applyFallbackTransition(scIssue, sqIssue, client); - let applied = false; - for (const transition of transitions) { - try { await client.transitionIssue(scIssue.key, transition); applied = true; } - catch (error) { logger.debug(`Failed to apply transition '${transition}' on issue ${scIssue.key}: ${error.message}`); } - } - return applied; - } catch (error) { - logger.debug(`Failed to fetch changelog for issue ${sqIssue.key}, falling back: ${error.message}`); - return await applyFallbackTransition(scIssue, sqIssue, client); - } - } - return await applyFallbackTransition(scIssue, sqIssue, client); + return applyFallbackTransition(scIssue, sqIssue, client); } async function applyFallbackTransition(scIssue, sqIssue, client) { diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js index 5ebbe447..38596024 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js @@ -8,8 +8,7 @@ import { addSourceLink } from './add-source-link.js'; // -------- Sync Single Issue -------- export async function syncSingleIssue(sqIssue, scIssue, client, sqClient, userMappings, stats, changelogMap = new Map()) { - const preloadedChangelog = changelogMap.get(sqIssue.key); - const transitioned = await syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog); + const transitioned = await syncIssueStatus(scIssue, sqIssue, client); if (transitioned) stats.transitioned++; await syncIssueAssignment(sqIssue, scIssue, client, userMappings, stats); await syncIssueComments(sqIssue, scIssue, client, stats); diff --git a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/transition-mapping.js b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/transition-mapping.js index 3deb19e3..1fc53feb 100644 --- a/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/transition-mapping.js +++ b/src/pipelines/sq-10.0/sonarcloud/migrators/issue-sync/helpers/transition-mapping.js @@ -35,7 +35,7 @@ export function getFallbackTransition(sqIssue) { if (sqIssue.resolution === 'WONTFIX' || sqIssue.status === 'WONTFIX') return 'wontfix'; switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; - case 'RESOLVED': case 'CLOSED': return 'resolve'; + case 'RESOLVED': case 'CLOSED': case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js index e48a64d7..71aba59a 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js @@ -9,7 +9,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; // ACCEPTED in SQ 10.4+ maps to 'wontfix' in SonarCloud case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js index 67c630ea..9d18c135 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js @@ -10,7 +10,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js index cbc98bfb..b3afce2c 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js @@ -1,38 +1,17 @@ -import { extractTransitionsFromChangelog } from './extract-transitions.js'; import { getFallbackTransition } from './get-fallback-transition.js'; import logger from '../../../../../../shared/utils/logger.js'; -// -------- Main Logic -------- +// -------- Sync Issue Status -------- /** - * Sync issue status by replaying the full changelog transition sequence. + * Sync the SC issue's status to match the current SQ issue status. + * Applies a single transition derived from the SQ issue's current state. */ -export async function syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog) { +export async function syncIssueStatus(scIssue, sqIssue, client) { if (scIssue.status === sqIssue.status) return false; - - if (sqClient) { - try { - const changelog = preloadedChangelog ?? await sqClient.getIssueChangelog(sqIssue.key); - const transitions = extractTransitionsFromChangelog(changelog); - if (transitions.length === 0) return await applyFallbackTransition(scIssue, sqIssue, client); - - let applied = false; - for (const transition of transitions) { - try { await client.transitionIssue(scIssue.key, transition); applied = true; } - catch (e) { logger.debug(`Failed to apply transition '${transition}' on issue ${scIssue.key}: ${e.message}`); } - } - return applied; - } catch (error) { - logger.debug(`Failed to fetch changelog for issue ${sqIssue.key}, falling back: ${error.message}`); - return await applyFallbackTransition(scIssue, sqIssue, client); - } - } - - return await applyFallbackTransition(scIssue, sqIssue, client); + return applyFallbackTransition(scIssue, sqIssue, client); } -// -------- Helper Functions -------- - async function applyFallbackTransition(scIssue, sqIssue, client) { const transition = getFallbackTransition(sqIssue); if (!transition) return false; diff --git a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js index 80988d85..3665e191 100644 --- a/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js +++ b/src/pipelines/sq-10.4/sonarcloud/migrators/issue-sync/helpers/sync-single-issue.js @@ -10,8 +10,7 @@ import logger from '../../../../../../shared/utils/logger.js'; */ export async function syncSingleIssue(sqIssue, scIssue, client, sqClient, userMappings, stats, changelogMap = new Map()) { try { - const preloadedChangelog = changelogMap.get(sqIssue.key); - const transitioned = await syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog); + const transitioned = await syncIssueStatus(scIssue, sqIssue, client); if (transitioned) stats.transitioned++; await syncIssueAssignment(sqIssue, scIssue, client, userMappings, stats); diff --git a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js index ca439fb3..ad981f97 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js @@ -11,7 +11,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js index cba697df..034418aa 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js @@ -1,40 +1,12 @@ -import logger from '../../../../../../shared/utils/logger.js'; -import { extractTransitionsFromChangelog } from './extract-transitions.js'; import { applyFallbackTransition } from './apply-fallback-transition.js'; // -------- Sync Issue Status -------- /** - * Sync issue status by replaying the full changelog transition sequence. - * Falls back to single-transition when no SQ client is provided. + * Sync the SC issue's status to match the current SQ issue status. + * Applies a single transition derived from the SQ issue's current state. */ -export async function syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog) { +export async function syncIssueStatus(scIssue, sqIssue, client) { if (scIssue.status === sqIssue.status) return false; - - if (sqClient) { - try { - const changelog = preloadedChangelog ?? await sqClient.getIssueChangelog(sqIssue.key); - const transitions = extractTransitionsFromChangelog(changelog); - - if (transitions.length === 0) { - return await applyFallbackTransition(scIssue, sqIssue, client); - } - - let applied = false; - for (const transition of transitions) { - try { - await client.transitionIssue(scIssue.key, transition); - applied = true; - } catch (error) { - logger.debug(`Failed to apply transition '${transition}' on issue ${scIssue.key}: ${error.message}`); - } - } - return applied; - } catch (error) { - logger.debug(`Failed to fetch changelog for issue ${sqIssue.key}, falling back: ${error.message}`); - return await applyFallbackTransition(scIssue, sqIssue, client); - } - } - - return await applyFallbackTransition(scIssue, sqIssue, client); + return applyFallbackTransition(scIssue, sqIssue, client); } diff --git a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-one-issue.js b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-one-issue.js index bb63a8ad..52849732 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-one-issue.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/helpers/sync-one-issue.js @@ -10,8 +10,7 @@ import { addSourceLink } from './add-source-link.js'; /** Sync a single matched issue pair (status, assignment, comments, tags, source link). */ export async function syncOneIssue({ sqIssue, scIssue }, client, sqClient, userMappings, stats, changelogMap = new Map()) { try { - const preloadedChangelog = changelogMap.get(sqIssue.key); - const transitioned = await syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog); + const transitioned = await syncIssueStatus(scIssue, sqIssue, client); if (transitioned) stats.transitioned++; await syncIssueAssignment(sqIssue, scIssue, client, userMappings, stats); 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 e9b18802..5463f03f 100644 --- a/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js +++ b/src/pipelines/sq-2025/sonarcloud/migrators/issue-sync/index.js @@ -59,7 +59,7 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { if (matchedPairs.length >= PARALLEL_THRESHOLD) { const scConfig = { baseURL: client.baseURL, token: client.token, organization: client.organization, projectKey }; const sqClientConfig = sqClient ? { baseURL: sqClient.baseURL, token: sqClient.token, projectKey: sqClient.projectKey } : null; - const mergedStats = await parallelSyncIssues(matchedPairs, changelogMap, scConfig, sqClientConfig, userMappings); + const mergedStats = await parallelSyncIssues(matchedPairs, scConfig, sqClientConfig, userMappings); Object.assign(stats, mergedStats); } else { logger.info(`Syncing ${matchedPairs.length} issues with concurrency=${concurrency}`); diff --git a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js index 98b33275..6a4fba44 100644 --- a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-status-mapper/helpers/get-fallback-transition.js @@ -7,7 +7,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js index d004301d..8a7448fd 100644 --- a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js +++ b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/get-fallback-transition.js @@ -7,7 +7,8 @@ export function getFallbackTransition(sqIssue) { switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; diff --git a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js index 4826e47c..034418aa 100644 --- a/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js +++ b/src/pipelines/sq-9.9/sonarcloud/migrators/issue-sync/helpers/sync-issue-status.js @@ -1,33 +1,12 @@ -import logger from '../../../../../../shared/utils/logger.js'; -import { extractTransitionsFromChangelog } from './extract-transitions-from-changelog.js'; import { applyFallbackTransition } from './apply-fallback-transition.js'; -// -------- Sync Issue Status via Changelog Replay -------- +// -------- Sync Issue Status -------- -export async function syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog) { +/** + * Sync the SC issue's status to match the current SQ issue status. + * Applies a single transition derived from the SQ issue's current state. + */ +export async function syncIssueStatus(scIssue, sqIssue, client) { if (scIssue.status === sqIssue.status) return false; - - if (sqClient) { - try { - const changelog = preloadedChangelog ?? await sqClient.getIssueChangelog(sqIssue.key); - const transitions = extractTransitionsFromChangelog(changelog); - if (transitions.length === 0) return applyFallbackTransition(scIssue, sqIssue, client); - - let applied = false; - for (const transition of transitions) { - try { - await client.transitionIssue(scIssue.key, transition); - applied = true; - } catch (error) { - logger.debug(`Failed to apply transition '${transition}' on issue ${scIssue.key}: ${error.message}`); - } - } - return applied; - } catch (error) { - logger.debug(`Failed to fetch changelog for issue ${sqIssue.key}, falling back: ${error.message}`); - return applyFallbackTransition(scIssue, sqIssue, client); - } - } - return applyFallbackTransition(scIssue, sqIssue, client); } 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 ce79eca8..d5ae527d 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 @@ -50,8 +50,7 @@ export async function syncIssues(projectKey, sqIssues, client, options = {}) { matchedPairs, async ({ sqIssue, scIssue }) => { try { - const preloadedChangelog = changelogMap.get(sqIssue.key); - const transitioned = await syncIssueStatus(scIssue, sqIssue, client, sqClient, preloadedChangelog); + const transitioned = await syncIssueStatus(scIssue, sqIssue, client); if (transitioned) stats.transitioned++; await syncIssueAssignment(scIssue, sqIssue, client, stats, userMappings); await syncIssueCommentsAndTags(scIssue, sqIssue, client, stats, sqClient); diff --git a/src/shared/utils/concurrency/helpers/parallel-issue-sync.js b/src/shared/utils/concurrency/helpers/parallel-issue-sync.js index c77f1561..44b37c65 100644 --- a/src/shared/utils/concurrency/helpers/parallel-issue-sync.js +++ b/src/shared/utils/concurrency/helpers/parallel-issue-sync.js @@ -10,7 +10,7 @@ const { parentPort, workerData } = require('worker_threads'); const https = require('https'); const http = require('http'); -const { chunk, scConfig, sqConfig, userMappings, changelogEntries, concurrencyPerWorker } = workerData; +const { chunk, scConfig, sqConfig, userMappings, concurrencyPerWorker } = workerData; function makePost(baseURL, token, path, params) { return new Promise((resolve, reject) => { @@ -60,68 +60,22 @@ function scPost(path, params) { return makePostWithRetry(scConfig.baseURL, scConfig.token, path, params, 6); } -function mapChangelogDiffToTransition(diffs) { - const statusDiff = diffs.find(d => d.key === 'status'); - const resolutionDiff = diffs.find(d => d.key === 'resolution'); - const newStatus = statusDiff && statusDiff.newValue; - const newResolution = resolutionDiff && resolutionDiff.newValue; - if (!newStatus) return null; - if (newResolution === 'FALSE-POSITIVE' || newStatus === 'FALSE-POSITIVE') return 'falsepositive'; - if (newResolution === 'WONTFIX' || newStatus === 'WONTFIX') return 'wontfix'; - switch (newStatus) { - case 'CONFIRMED': return 'confirm'; - case 'REOPENED': return 'reopen'; - case 'OPEN': return 'unconfirm'; - case 'RESOLVED': return 'resolve'; - case 'CLOSED': return 'resolve'; - case 'ACCEPTED': return 'wontfix'; - default: return null; - } -} - -function extractTransitionsFromChangelog(changelog) { - const transitions = []; - for (const entry of changelog) { - const diffs = entry.diffs || []; - if (!diffs.some(d => d.key === 'status')) continue; - const t = mapChangelogDiffToTransition(diffs); - if (t) transitions.push(t); - } - return transitions; -} - function getFallbackTransition(sqIssue) { if (sqIssue.resolution === 'FALSE-POSITIVE' || sqIssue.status === 'FALSE-POSITIVE') return 'falsepositive'; if (sqIssue.resolution === 'WONTFIX' || sqIssue.status === 'WONTFIX') return 'wontfix'; switch (sqIssue.status) { case 'CONFIRMED': return 'confirm'; case 'RESOLVED': - case 'CLOSED': return 'resolve'; + case 'CLOSED': + case 'FIXED': return 'resolve'; case 'ACCEPTED': return 'wontfix'; case 'REOPENED': return 'reopen'; default: return null; } } -async function syncIssueStatus(scIssue, sqIssue, changelog) { +async function syncIssueStatus(scIssue, sqIssue) { if (scIssue.status === sqIssue.status) return false; - - if (changelog) { - const transitions = extractTransitionsFromChangelog(changelog); - if (transitions.length === 0) { - const t = getFallbackTransition(sqIssue); - if (!t) return false; - try { await scPost('/api/issues/do_transition', { issue: scIssue.key, transition: t }); return true; } - catch { return false; } - } - let applied = false; - for (const transition of transitions) { - try { await scPost('/api/issues/do_transition', { issue: scIssue.key, transition }); applied = true; } - catch { /* expected: some transitions are invalid for current state */ } - } - return applied; - } - const t = getFallbackTransition(sqIssue); if (!t) return false; try { await scPost('/api/issues/do_transition', { issue: scIssue.key, transition: t }); return true; } @@ -179,11 +133,10 @@ async function addSourceLink(sqIssue, scIssue, stats) { } catch { stats.apiErrors++; } } -async function syncOneIssue(pair, userMappingsMap, stats, changelogMap) { +async function syncOneIssue(pair, userMappingsMap, stats) { try { const { sqIssue, scIssue } = pair; - const changelog = changelogMap.get(sqIssue.key) || null; - const transitioned = await syncIssueStatus(scIssue, sqIssue, changelog); + const transitioned = await syncIssueStatus(scIssue, sqIssue); if (transitioned) stats.transitioned++; await syncIssueAssignment(sqIssue, scIssue, userMappingsMap, stats); await syncIssueComments(sqIssue, scIssue, stats); @@ -216,11 +169,10 @@ async function run() { }; const userMappingsMap = new Map(userMappings || []); - const changelogMap = new Map(changelogEntries || []); let completed = 0; await mapConcurrentWorker(chunk, async (pair) => { - await syncOneIssue(pair, userMappingsMap, stats, changelogMap); + await syncOneIssue(pair, userMappingsMap, stats); completed++; if (completed % 50 === 0) parentPort.postMessage({ type: 'progress', completed }); }, concurrencyPerWorker); @@ -233,7 +185,7 @@ run().catch((err) => { }); `; -export async function parallelSyncIssues(matchedPairs, changelogMap, scConfig, sqConfig, userMappings, options = {}) { +export async function parallelSyncIssues(matchedPairs, scConfig, sqConfig, userMappings, options = {}) { const workerCount = options.workerCount || 20; const concurrencyPerWorker = options.concurrencyPerWorker || 5; const totalPairs = matchedPairs.length; @@ -246,12 +198,6 @@ export async function parallelSyncIssues(matchedPairs, changelogMap, scConfig, s let totalCompleted = 0; const workerPromises = chunks.map((chunk, i) => { - const changelogEntries = []; - for (const pair of chunk) { - const entry = changelogMap.get(pair.sqIssue.key); - if (entry) changelogEntries.push([pair.sqIssue.key, entry]); - } - return new Promise((resolve, reject) => { const worker = new Worker(WORKER_CODE, { eval: true, @@ -260,7 +206,6 @@ export async function parallelSyncIssues(matchedPairs, changelogMap, scConfig, s scConfig, sqConfig, userMappings: serializedUserMappings, - changelogEntries, concurrencyPerWorker, }, }); diff --git a/test/sonarcloud/migrators/migrators.test.js b/test/sonarcloud/migrators/migrators.test.js index db97db27..5568f5f8 100644 --- a/test/sonarcloud/migrators/migrators.test.js +++ b/test/sonarcloud/migrators/migrators.test.js @@ -2222,7 +2222,11 @@ test('extractTransitionsFromChangelog skips unknown status transitions', t => { // issue-sync.js - syncIssues with sqClient (changelog replay) // ============================================================================ -test('syncIssues with sqClient replays changelog transitions in order', async t => { +test('syncIssues applies a single transition derived from current SQ status, ignoring older changelog states (#140)', async t => { + // Source went OPEN -> CONFIRMED -> RESOLVED+FALSE-POSITIVE -> REOPENED. Current source: REOPENED. + // Bug #140: pre-fix, replaying every transition meant the FIRST applicable one won + // (later ones rejected by SC's state machine). After the fix we apply exactly one + // transition based on the current source status -> 'reopen'. const client = mockClient({ searchIssues: sinon.stub().resolves([ { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'OPEN' } @@ -2230,12 +2234,12 @@ test('syncIssues with sqClient replays changelog transitions in order', async t }); const sqClient = { getIssueChangelog: sinon.stub().resolves([ - { diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] }, - { diffs: [ + { user: 'alice', diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] }, + { user: 'alice', diffs: [ { key: 'status', oldValue: 'CONFIRMED', newValue: 'RESOLVED' }, { key: 'resolution', oldValue: '', newValue: 'FALSE-POSITIVE' } ] }, - { diffs: [{ key: 'status', oldValue: 'RESOLVED', newValue: 'REOPENED' }] } + { user: 'alice', diffs: [{ key: 'status', oldValue: 'RESOLVED', newValue: 'REOPENED' }] } ]) }; const sqIssues = [ @@ -2245,98 +2249,24 @@ test('syncIssues with sqClient replays changelog transitions in order', async t const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); t.is(stats.transitioned, 1); - t.is(client.transitionIssue.callCount, 3); - t.is(client.transitionIssue.getCall(0).args[1], 'confirm'); - t.is(client.transitionIssue.getCall(1).args[1], 'falsepositive'); - t.is(client.transitionIssue.getCall(2).args[1], 'reopen'); -}); - -test('syncIssues with sqClient falls back when changelog has no status changes', async t => { - const client = mockClient({ - searchIssues: sinon.stub().resolves([ - { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'OPEN' } - ]) - }); - const sqClient = { - getIssueChangelog: sinon.stub().resolves([ - { diffs: [{ key: 'assignee', oldValue: '', newValue: 'alice' }] } - ]) - }; - const sqIssues = [ - { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'CONFIRMED' } - ]; - - const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); - - t.is(stats.transitioned, 1); - t.is(client.transitionIssue.firstCall.args[1], 'confirm'); -}); - -test('syncIssues with sqClient falls back when changelog fetch fails', async t => { - const client = mockClient({ - searchIssues: sinon.stub().resolves([ - { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'OPEN' } - ]) - }); - const sqClient = { - getIssueChangelog: sinon.stub().rejects(new Error('changelog API error')) - }; - const sqIssues = [ - { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'CONFIRMED' } - ]; - - const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); - - // Falls back to single transition - t.is(stats.transitioned, 1); - t.is(client.transitionIssue.firstCall.args[1], 'confirm'); -}); - -test('syncIssues with sqClient handles partial transition failures in replay', async t => { - const transitionStub = sinon.stub(); - transitionStub.onFirstCall().resolves(); - transitionStub.onSecondCall().rejects(new Error('transition not available')); - transitionStub.onThirdCall().resolves(); - - const client = mockClient({ - searchIssues: sinon.stub().resolves([ - { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'OPEN' } - ]), - transitionIssue: transitionStub - }); - const sqClient = { - getIssueChangelog: sinon.stub().resolves([ - { diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] }, - { diffs: [{ key: 'status', oldValue: 'CONFIRMED', newValue: 'RESOLVED' }] }, - { diffs: [{ key: 'status', oldValue: 'RESOLVED', newValue: 'REOPENED' }] } - ]) - }; - const sqIssues = [ - { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'REOPENED' } - ]; - - const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); - - // Still counted as transitioned since at least one succeeded - t.is(stats.transitioned, 1); - t.is(transitionStub.callCount, 3); + t.is(client.transitionIssue.callCount, 1); + t.is(client.transitionIssue.firstCall.args[1], 'reopen'); }); -test('syncIssues with sqClient returns not transitioned when all replay transitions fail', async t => { +test('syncIssues with sqClient returns not transitioned when transition fails', async t => { const client = mockClient({ searchIssues: sinon.stub().resolves([ { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'OPEN' } ]), - transitionIssue: sinon.stub().rejects(new Error('all fail')) + transitionIssue: sinon.stub().rejects(new Error('transition rejected')) }); const sqClient = { getIssueChangelog: sinon.stub().resolves([ - { diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] }, - { diffs: [{ key: 'status', oldValue: 'CONFIRMED', newValue: 'RESOLVED' }] } + { user: 'alice', diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] } ]) }; const sqIssues = [ - { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'RESOLVED' } + { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 10, status: 'CONFIRMED' } ]; const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); @@ -2344,14 +2274,16 @@ test('syncIssues with sqClient returns not transitioned when all replay transiti t.is(stats.transitioned, 0); }); -test('syncIssues with sqClient skips changelog fetch when statuses match', async t => { +test('syncIssues skips transition when SC and SQ statuses already match', async t => { const client = mockClient({ searchIssues: sinon.stub().resolves([ { key: 'sc-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'CONFIRMED' } ]) }); const sqClient = { - getIssueChangelog: sinon.stub().resolves([]) + getIssueChangelog: sinon.stub().resolves([ + { user: 'alice', diffs: [{ key: 'status', oldValue: 'OPEN', newValue: 'CONFIRMED' }] } + ]) }; const sqIssues = [ { key: 'sq-i1', rule: 'js:S1001', component: 'proj:src/a.js', line: 5, status: 'CONFIRMED' } @@ -2360,7 +2292,7 @@ test('syncIssues with sqClient skips changelog fetch when statuses match', async const stats = await syncIssues('proj', sqIssues, client, { concurrency: 1, sqClient }); t.is(stats.transitioned, 0); - t.is(sqClient.getIssueChangelog.callCount, 0); + t.is(client.transitionIssue.callCount, 0); }); // ============================================================================ From 6ac408491295cba800488738fa6a5ed432efb053 Mon Sep 17 00:00:00 2001 From: Olivier Korach Date: Fri, 1 May 2026 16:00:57 +0200 Subject: [PATCH 2/2] docs: update issue-sync notes after #140 fix --- docs/CONTRIBUTING.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/CONTRIBUTING.md b/docs/CONTRIBUTING.md index 21d3f0c4..2615cc45 100644 --- a/docs/CONTRIBUTING.md +++ b/docs/CONTRIBUTING.md @@ -247,13 +247,12 @@ An issue is considered to have **manual changes** if any of: - `fetchSqChangelogs(sqIssues, sqClient, concurrency)` — batch-fetches changelogs via `mapConcurrent`, returns `Map` - `hasManualChanges(issue, changelog)` — pure function, returns `boolean` -The pre-fetched changelogs are passed through to `syncIssueStatus` via a `changelogMap` to avoid redundant per-issue changelog API calls. +The pre-fetched changelogs are used only to evaluate `hasManualChanges` during pre-filtering. Status sync derives the target SC transition from the issue's *current* SQ status (not the changelog history), so changelogs are not propagated past the filter step. **Performance test:** `test/utils/issue-sync.test.js` includes regression tests that verify the filter ratio stays within tolerance (<=10% of issues pass through) and that filtering 50K issues completes in <1 second. **Rules:** -- Always pass `changelogMap` through from orchestrator to per-issue sync functions -- The `preloadedChangelog` parameter in `syncIssueStatus` uses `??` (nullish coalescing) so it falls back to fetching if the caller doesn't provide preloaded data +- `syncIssueStatus(scIssue, sqIssue, client)` applies a single transition derived from the SQ issue's current state — it does not replay changelog history - The `stats.filtered` counter tracks how many issues were skipped by the pre-filter ---