diff --git a/__tests__/integration/dependabot.test.js b/__tests__/integration/dependabot.test.js index 5802cb3..bbec3ca 100644 --- a/__tests__/integration/dependabot.test.js +++ b/__tests__/integration/dependabot.test.js @@ -229,6 +229,83 @@ describe('dependabot', () => { expect(result.matchesTarget).toBe(false); }); + // Bug #27 (wave-1 QA finding): the previous JSON.stringify-based equality + // check was order-sensitive, so identical configs in different key order + // were reported as drift and triggered unnecessary writes (or PRs when + // `change_strategy.use_pull_requests: true`). Object-key order must not + // matter; array element order must still matter (dependabot.yml + // `updates` / `labels` / `allow` / `ignore` lists are intentionally + // ordered). + it('treats identical configs with different key order as matching (Bug #27)', async () => { + _setConfigForTesting({ + dependabot: { + version: 2, + updates: [ + { 'package-ecosystem': 'npm', directory: '/', schedule: { interval: 'weekly' }, labels: ['dependencies'] } + ] + } + }); + const reorderedCurrent = { + updates: [ + { labels: ['dependencies'], schedule: { interval: 'weekly' }, directory: '/', 'package-ecosystem': 'npm' } + ], + version: 2 + }; + octokit.request.mockResolvedValue({ data: { content: encodeConfig(reorderedCurrent) } }); + octokit.paginate.mockResolvedValue([]); + + const result = await checkExistingDependabotConfig(octokit, 'o', 'r'); + + expect(result.exists).toBe(true); + expect(result.matchesTarget).toBe(true); + }); + + it('treats genuinely different configs as not matching', async () => { + _setConfigForTesting({ + dependabot: { + version: 2, + updates: [ + { 'package-ecosystem': 'npm', directory: '/', schedule: { interval: 'weekly' }, labels: ['dependencies'] } + ] + } + }); + const differentValues = { + version: 2, + updates: [ + { 'package-ecosystem': 'npm', directory: '/', schedule: { interval: 'daily' }, labels: ['dependencies'] } + ] + }; + octokit.request.mockResolvedValue({ data: { content: encodeConfig(differentValues) } }); + octokit.paginate.mockResolvedValue([]); + + const result = await checkExistingDependabotConfig(octokit, 'o', 'r'); + + expect(result.matchesTarget).toBe(false); + }); + + it('treats arrays with different element order as not matching (intentional)', async () => { + _setConfigForTesting({ + dependabot: { + version: 2, + updates: [ + { 'package-ecosystem': 'npm', directory: '/', schedule: { interval: 'weekly' }, labels: ['a', 'b'] } + ] + } + }); + const reorderedArray = { + version: 2, + updates: [ + { 'package-ecosystem': 'npm', directory: '/', schedule: { interval: 'weekly' }, labels: ['b', 'a'] } + ] + }; + octokit.request.mockResolvedValue({ data: { content: encodeConfig(reorderedArray) } }); + octokit.paginate.mockResolvedValue([]); + + const result = await checkExistingDependabotConfig(octokit, 'o', 'r'); + + expect(result.matchesTarget).toBe(false); + }); + it('detects PRs with missing labels', async () => { const configYaml = yaml.dump({ version: 2, updates: [{ labels: ['dependencies'] }] }); const b64 = Buffer.from(configYaml).toString('base64'); diff --git a/src/dependabot.js b/src/dependabot.js index 88174b4..7bc89e6 100644 --- a/src/dependabot.js +++ b/src/dependabot.js @@ -5,6 +5,30 @@ import { upsertRepoFile, createConfigurationPR } from './github-api.js'; import { detectEcosystems } from './ecosystem-detector.js'; import { callLocalAI } from './ai-review.js'; +/** + * Recursively sort object keys so two structurally identical configs serialize + * to identical JSON regardless of key insertion order. Arrays preserve element + * order (intentional — dependabot.yml `updates`, `labels`, `allow`/`ignore` + * lists are order-sensitive). Primitives pass through. + * + * @param {*} value + * @returns {*} + */ +function canonicalize(value) { + if (Array.isArray(value)) return value.map(canonicalize); + if (value && typeof value === 'object') { + const out = {}; + for (const k of Object.keys(value).sort()) out[k] = canonicalize(value[k]); + return out; + } + return value; +} + +/** Key-order-independent deep equal for plain JSON-like configs. */ +function configsEqual(a, b) { + return JSON.stringify(canonicalize(a)) === JSON.stringify(canonicalize(b)); +} + function extractLabelsFromConfig(dependabotConfig) { if (!dependabotConfig?.updates) return []; const labels = new Set(); @@ -105,7 +129,7 @@ async function checkExistingDependabotConfig(octokit, owner, repo) { return { exists: true, currentConfig, - matchesTarget: JSON.stringify(currentConfig) === JSON.stringify(targetConfig), + matchesTarget: configsEqual(currentConfig, targetConfig), labelIssues, dependabotPRCount: dependabotPRs.length }; @@ -184,7 +208,7 @@ async function checkDependabotConfiguration(octokit, owner, repo) { report += yaml.dump(targetConfig) + '\n'; report += '```\n\n'; - if (JSON.stringify(currentConfig) === JSON.stringify(targetConfig)) { + if (configsEqual(currentConfig, targetConfig)) { report += '✅ Dependabot configuration matches target!\n'; } else { report +=