Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 77 additions & 0 deletions __tests__/integration/dependabot.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
28 changes: 26 additions & 2 deletions src/dependabot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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
};
Expand Down Expand Up @@ -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 +=
Expand Down
Loading