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
27 changes: 23 additions & 4 deletions __tests__/integration/labels.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,37 @@ describe('synchronizeIssueLabels', () => {
);
});

it('removes labels not in target', async () => {
it('removes labels not in target when mode is replace', async () => {
octokit.paginate.mockResolvedValue([{ name: 'old-label', color: 'fff', description: '' }]);
const target = [];

await synchronizeIssueLabels(octokit, 'owner', 'repo', target);
await synchronizeIssueLabels(octokit, 'owner', 'repo', target, { mode: 'replace' });

expect(octokit.request).toHaveBeenCalledWith(
'DELETE /repos/{owner}/{repo}/labels/{name}',
expect.objectContaining({ name: 'old-label' })
);
});

it('does NOT remove labels in default merge mode (Bug #1 fix)', async () => {
octokit.paginate.mockResolvedValue([{ name: 'priority/p0', color: 'fff', description: 'user-created' }]);
const target = [];

await synchronizeIssueLabels(octokit, 'owner', 'repo', target);

expect(octokit.request).not.toHaveBeenCalledWith(
'DELETE /repos/{owner}/{repo}/labels/{name}',
expect.anything()
);
});

it('rejects an invalid mode value', async () => {
octokit.paginate.mockResolvedValue([]);
await expect(
synchronizeIssueLabels(octokit, 'owner', 'repo', [], { mode: 'destroy' })
).rejects.toThrow(/invalid mode/i);
});

it('skips update when label matches', async () => {
octokit.paginate.mockResolvedValue([{ name: 'bug', color: 'red', description: 'Bug' }]);
const target = [{ name: 'bug', color: 'red', description: 'Bug' }];
Expand All @@ -71,7 +90,7 @@ describe('synchronizeIssueLabels', () => {
);
});

it('handles create, update, and delete in one pass', async () => {
it('handles create, update, and delete in one pass (replace mode)', async () => {
octokit.paginate.mockResolvedValue([
{ name: 'keep', color: 'aaa', description: 'Keep' },
{ name: 'update-me', color: 'old', description: 'Old desc' },
Expand All @@ -83,7 +102,7 @@ describe('synchronizeIssueLabels', () => {
{ name: 'create-me', color: 'ccc', description: 'Created' }
];

await synchronizeIssueLabels(octokit, 'owner', 'repo', target);
await synchronizeIssueLabels(octokit, 'owner', 'repo', target, { mode: 'replace' });

// Should not update 'keep' (unchanged)
expect(octokit.request).not.toHaveBeenCalledWith(
Expand Down
11 changes: 11 additions & 0 deletions config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,17 @@ scheduler:
max_tasks_per_tick: 5
rate_limit_threshold: 100

# How `synchronizeIssueLabels` reconciles a repo's labels with `issue_labels`:
# - `merge` (default, non-destructive): create or update labels listed below,
# but never delete labels that are not in this list. Safe for repos where
# users have created their own custom labels (per-issue tags, milestones,
# etc.).
# - `replace` (destructive, opt-in): also delete any label that is not in
# this list. This was the behaviour prior to fixing Bug #1
# (`docs/agent-fleet/bugs.md`); set this only if you explicitly want every
# repo's labels reduced to exactly `issue_labels`.
issue_labels_sync_mode: merge

issue_labels:
- name: "bug"
color: "d73a4a"
Expand Down
46 changes: 36 additions & 10 deletions src/labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,33 @@ async function ensureLabelsExist(octokit, owner, repo, labelNames) {
}
}

async function synchronizeIssueLabels(octokit, owner, repo, targetLabels) {
const VALID_SYNC_MODES = new Set(['merge', 'replace']);

/**
* Synchronize a repo's issue labels with `targetLabels`.
*
* @param {object} octokit
* @param {string} owner
* @param {string} repo
* @param {Array<{name: string, color: string, description?: string}>} targetLabels
* @param {object} [options]
* @param {'merge'|'replace'} [options.mode='merge'] - synchronization mode:
* - `'merge'` (default): only create / update labels listed in `targetLabels`.
* Labels not in the target list are left untouched. Non-destructive.
* - `'replace'`: in addition to create/update, deletes any label that is not
* in the target list. Destructive — the historical (pre-Bug #1) behaviour,
* preserved as opt-in.
*/
async function synchronizeIssueLabels(octokit, owner, repo, targetLabels, options = {}) {
const mode = options.mode || 'merge';
if (!VALID_SYNC_MODES.has(mode)) {
throw new Error(
`synchronizeIssueLabels: invalid mode "${mode}" (expected "merge" or "replace")`
);
}

try {
getLogger().info(`Synchronizing labels for ${owner}/${repo}`);
getLogger().info(`Synchronizing labels for ${owner}/${repo} (mode=${mode})`);

const currentLabels = await octokit.paginate('GET /repos/{owner}/{repo}/labels', {
owner,
Expand Down Expand Up @@ -66,14 +90,16 @@ async function synchronizeIssueLabels(octokit, owner, repo, targetLabels) {
}
}

for (const currentLabel of currentLabels) {
if (!targetLabels.some((tl) => tl.name === currentLabel.name)) {
await octokit.request('DELETE /repos/{owner}/{repo}/labels/{name}', {
owner,
repo,
name: currentLabel.name
});
getLogger().info(`Removed label: ${currentLabel.name}`);
if (mode === 'replace') {
for (const currentLabel of currentLabels) {
if (!targetLabels.some((tl) => tl.name === currentLabel.name)) {
await octokit.request('DELETE /repos/{owner}/{repo}/labels/{name}', {
owner,
repo,
name: currentLabel.name
});
getLogger().info(`Removed label: ${currentLabel.name}`);
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion src/repository.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,13 @@ async function configureRepository(
const targetLabels = getTargetIssueLabels();
if (targetLabels.length > 0) {
// Labels don't need a default branch — safe even on empty repos.
await synchronizeIssueLabels(octokit, owner, repo, targetLabels);
// Default sync mode is 'merge' (non-destructive). Set
// `issue_labels_sync_mode: replace` in config.yml to opt back into the
// pre-Bug-#1 behaviour that deletes labels not in the target list.
const labelSyncMode = config?.issue_labels_sync_mode || 'merge';
await synchronizeIssueLabels(octokit, owner, repo, targetLabels, {
mode: labelSyncMode
});
}

if (skipBranchScopedWork) {
Expand Down
7 changes: 7 additions & 0 deletions src/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ export function validateConfig(config) {
}
}

if (config.issue_labels_sync_mode !== undefined) {
const validModes = ['merge', 'replace'];
if (!validModes.includes(config.issue_labels_sync_mode)) {
errors.push('issue_labels_sync_mode must be one of: merge, replace');
}
}

if (config.dependabot !== undefined) {
if (typeof config.dependabot.version !== 'number') {
errors.push('dependabot.version must be a number');
Expand Down
Loading