fix: hotspot sync — || null → ?? null (Issue #146) + ACKNOWLEDGED → SAFE mapping#154
Merged
Conversation
…sion testing ## Bug 1: `|| null` → `?? null` in syncHotspotStatus early-return (Issue #146) ### Problem The early-return comparison in `syncHotspotStatus.js` used `|| null` to normalize resolution values before comparing SQ and SC hotspot states: if (scHotspot.status === sqHotspot.status && (scHotspot.resolution || null) === (sqHotspot.resolution || null)) return false; `|| null` treats BOTH `null` and `undefined` as `null`, but also coerces ALL falsy values (`''`, `0`, `false`) to `null`. While resolution values are only ever `SAFE`, `ACKNOWLEDGED`, `FIXED`, or `null`/`undefined`, using `??` (nullish coalescing) is semantically correct: it only treats `null` and `undefined` as nullish, preserving actual string values. ### Fix Changed `|| null` → `?? null` in all 4 pipeline versions: - src/pipelines/sq-2025/sonarcloud/migrators/hotspot-sync/helpers/sync-hotspot-status.js - src/pipelines/sq-10.0/sonarcloud/migrators/hotspot-sync/helpers/sync-hotspot-status.js - src/pipelines/sq-9.9/sonarcloud/migrators/hotspot-sync/helpers/sync-hotspot-status.js - src/pipelines/sq-10.4/sonarcloud/migrators/hotspot-sync/helpers/sync-hotspot-status.js ## Bug 2: ACKNOWLEDGED resolution rejected by SonarCloud API (discovered during regression) ### Problem During regression testing (see "Regression Test Procedure" below), a second bug was discovered: when SQ hotspots have `status=REVIEWED` with `resolution=ACKNOWLEDGED`, the migration failed with: Failed to apply hotspot transition on AZ4BL...: SonarCloud API error (400): Value of parameter 'resolution' (ACKNOWLEDGED) must be one of: [FIXED, SAFE] SonarCloud only accepts `FIXED` or `SAFE` as valid resolution values. `ACKNOWLEDGED` is a valid SQ resolution but is rejected by the SC API. ### Fix Two complementary changes were made across all 4 pipeline versions: 1. Resolution mapper (`mapHotspotResolution.js` / `hotspot-fallback-action.js`): SQ `ACKNOWLEDGED` is mapped to SC-compatible `SAFE`: function mapHotspotResolution(sqHotspot) { if (sqHotspot.resolution === 'ACKNOWLEDGED') return 'SAFE'; // was: return 'ACKNOWLEDGED' if (sqHotspot.resolution === 'FIXED') return 'FIXED'; if (sqHotspot.resolution === 'SAFE' || sqHotspot.status === 'REVIEWED') return 'SAFE'; return null; } 2. Changelog diff mapper: Added a `toScResolution()` helper that maps SQ resolutions to SC-compatible values during changelog replay. This ensures that when hotspot history (changelog) is replayed on SC, ACKNOWLEDGED transitions are converted to SAFE rather than being sent as-is: function toScResolution(resolution) { if (resolution === 'ACKNOWLEDGED') return 'SAFE'; if (resolution === 'FIXED') return 'FIXED'; if (resolution === 'SAFE') return 'SAFE'; return null; } ### Files Changed (12 files) sq-2025 pipeline: - hotspot-sync/helpers/sync-hotspot-status.js — `|| null` → `?? null` - hotspot-sync/helpers/hotspot-fallback-action.js — ACKNOWLEDGED → SAFE - hotspot-sync/helpers/map-hotspot-changelog.js — added toScResolution() sq-10.0 pipeline: - hotspot-sync/helpers/sync-hotspot-status.js — `|| null` → `?? null` - hotspot-sync/helpers/map-hotspot-resolution.js — ACKNOWLEDGED → SAFE - hotspot-sync/helpers/hotspot-transition-mapping.js — added toScResolution() sq-9.9 pipeline: - hotspot-sync/helpers/sync-hotspot-status.js — `|| null` → `?? null` - hotspot-sync/helpers/map-hotspot-resolution.js — ACKNOWLEDGED → SAFE - hotspot-sync/helpers/map-hotspot-changelog-diff.js — added toScResolution() sq-10.4 pipeline: - hotspot-sync/helpers/sync-hotspot-status.js — `|| null` → `?? null` - hotspot-sync/helpers/map-hotspot-resolution.js — ACKNOWLEDGED → SAFE - hotspot-sync/helpers/map-changelog-diff.js — added toScResolution() ## Regression Test Procedure ### Test Design 1. Created 4 reviewed hotspots on SQS (angular project) via API: - 1× REVIEWED/SAFE - 3× REVIEWED/ACKNOWLEDGED 2. Deleted the existing SC test project (`angular-test-issue-146-v2`) 3. Created fresh SC project (`angular-test-issue-146-v3`) to start clean 4. Ran full transfer (15,051 issues, 94 hotspots) with both fixes applied ### Before ACKNOWLEDGED Fix (v2 transfer) Hotspot sync: 94 matched, 1 status changed, 0 comments, 94 metadata-sync-commented, 94 source-linked, 0 failed Only the SAFE hotspot was migrated. The 3 ACKNOWLEDGED hotspots failed with: Failed to apply hotspot transition on AZ4BL...: SonarCloud API error (400): Value of parameter 'resolution' (ACKNOWLEDGED) must be one of: [FIXED, SAFE] ### After ACKNOWLEDGED Fix (v3 transfer) Hotspot sync: 94 matched, 4 status changed, 0 comments, 94 metadata-sync-commented, 94 source-linked, 0 failed All 4 reviewed hotspots migrated successfully. The 3 ACKNOWLEDGED SQ resolutions were mapped to SAFE on SC. ### Phase 4 Verification SQS source (4 reviewed hotspots): 5c2399d4... → REVIEWED/SAFE 33ead69f... → REVIEWED/ACKNOWLEDGED 2e6c851f... → REVIEWED/ACKNOWLEDGED e99ed0c7... → REVIEWED/ACKNOWLEDGED SC result (all migrated as REVIEWED/SAFE): AZ4BgDTpDmYDTQ6gRLd- → REVIEWED/SAFE AZ4Bf_5SDmYDTQ6gRIkN → REVIEWED/SAFE AZ4BgDmYDmYDTQ6gRLtH → REVIEWED/SAFE AZ4BgDzuDmYDTQ6gRL3a → REVIEWED/SAFE Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
SummaryTwo regression bugs in hotspot sync, both verified by end-to-end migration testing: Bug 1: Falsy value coercion in status/resolution comparison — The early-return check in Bug 2: SonarCloud API rejects ACKNOWLEDGED resolution — SonarCloud only accepts
The test results confirm both fixes: all 4 hotspots (including 3× ACKNOWLEDGED) now sync successfully with zero API errors. What reviewers should knowWhere the code changes are:
What to focus on:
Non-obvious decision:
Test results to trust:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two hotspot sync bugs found and fixed during exhaustive regression testing:
|| null→?? nullinsyncHotspotStatus(Issue Hotspots are properly migrated to the proper state but for some reason the hotspot review % metric is wrong #146 original fix) — the early-return comparison used||which coerces all falsy values tonull, instead of??which only treatsnull/undefinedas nullish. Fixed across all 4 pipeline versions.ACKNOWLEDGED resolution rejected by SonarCloud API (bonus bug found during regression) — SC only accepts
FIXEDorSAFEas resolutions, but SQ'sACKNOWLEDGEDwas being sent as-is. AddedtoScResolution()helper to map ACKNOWLEDGED→SAFE in changelog replay, and fixed the resolution mappers in all 4 pipeline versions.Test Plan
angular-test-issue-146-v3Hotspot sync: 94 matched, 1 status changed— only SAFE hotspot migrated; 3 ACKNOWLEDGED failed with SC API 400 errorHotspot sync: 94 matched, 4 status changed, 0 failed— all 4 hotspots migrated successfullyREVIEWED/SAFEin SC🤖 Generated with Claude Code