diff --git a/docs/cli.md b/docs/cli.md index 1d8ac2bc..8c9ca2ca 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -246,7 +246,7 @@ The repository is resolved from `--repo`, then `GITHUB_REPOSITORY`, then the `or ### `releasekit labels sync` -Idempotently create every config-implied label that is missing from the repo. Existing labels are left untouched (a 422 "already exists" is ignored). +Idempotently create every config-implied label that is missing from the repo. Existing labels, matched case-insensitively, are left untouched and never re-requested. | Flag | Type | Default | Description | |---|---|---|---| diff --git a/packages/forge/test/unit/github.spec.ts b/packages/forge/test/unit/github.spec.ts index d7054626..e0a3fa81 100644 --- a/packages/forge/test/unit/github.spec.ts +++ b/packages/forge/test/unit/github.spec.ts @@ -388,6 +388,17 @@ describe('GitHubForge', () => { expect(await forge.listLabelNames()).toEqual(['a', 'b']); }); + it('should list label names from every page', async () => { + const { octokit, fns } = makeOctokit(); + fns.iterator.mockReturnValue(pages([{ name: 'a' }, { name: 'b' }], [{ name: 'c' }])); + + expect(await new GitHubForge(octokit, 'o', 'r').listLabelNames()).toEqual(['a', 'b', 'c']); + expect(fns.iterator).toHaveBeenCalledWith( + { __ref: 'listLabelsForRepo' }, + { owner: 'o', repo: 'r', per_page: 100 }, + ); + }); + it('should set a commit status', async () => { const { octokit, fns } = makeOctokit(); await new GitHubForge(octokit, 'o', 'r').setCommitStatus({ diff --git a/packages/release/src/label-definitions.ts b/packages/release/src/label-definitions.ts index bf71a10a..48f49edf 100644 --- a/packages/release/src/label-definitions.ts +++ b/packages/release/src/label-definitions.ts @@ -131,18 +131,21 @@ export interface LabelSyncResult { } /** - * Idempotently create every config-implied label that is missing from the repo. Already-existing - * labels are left untouched (createLabel throws 422; ignored). Returns which labels were created - * vs already present. + * Create every config-implied label that is missing from the repo. Labels already present are never + * requested: each rejected create is logged by Octokit as a 422 and reads like a failure in CI. */ export async function syncLabels(forge: Forge, definitions: LabelDefinition[]): Promise { + const repoLabels = await listRepoLabelNames(forge); const created: string[] = []; const existing: string[] = []; for (const def of definitions) { - // The forge treats GitHub's "already exists" 422 as idempotent (returns 'exists') and surfaces - // any other failure (auth, rate limit, validation) by throwing. + if (repoLabels.has(def.name.toLowerCase())) { + existing.push(def.name); + continue; + } const result = await forge.createLabel({ name: def.name, color: def.color, description: def.description }); + repoLabels.add(def.name.toLowerCase()); (result === 'created' ? created : existing).push(def.name); } diff --git a/packages/release/test/unit/label-definitions.spec.ts b/packages/release/test/unit/label-definitions.spec.ts index cac5110f..f02a91f2 100644 --- a/packages/release/test/unit/label-definitions.spec.ts +++ b/packages/release/test/unit/label-definitions.spec.ts @@ -128,9 +128,7 @@ describe('syncLabels', () => { expect(forge.createdLabels).toHaveLength(2); }); - it('should treat a 422 with already_exists error code as idempotent', async () => { - // The forge resolves an already-existing label to 'exists' (the raw-422 'already_exists' - // detection now lives in the GitHubForge adapter). Seed the existing label to reproduce it. + it('should not request a create for labels the repo already has', async () => { const forge = createFakeForge({ labelNames: ['bump:minor'] }); const defs: LabelDefinition[] = [ { name: 'bump:minor', color: '0e8a16', description: 'x' }, @@ -141,6 +139,56 @@ describe('syncLabels', () => { expect(result.existing).toEqual(['bump:minor']); expect(result.created).toEqual(['release:skip']); + expect(forge.createdLabels.map((l) => l.name)).toEqual(['release:skip']); + }); + + it('should match existing labels case-insensitively', async () => { + const forge = createFakeForge({ labelNames: ['Bump:Minor', 'RELEASE:SKIP'] }); + const defs: LabelDefinition[] = [ + { name: 'bump:minor', color: '0e8a16', description: 'x' }, + { name: 'release:skip', color: 'd93f0b', description: 'y' }, + ]; + + const result = await syncLabels(forge, defs); + + expect(result.existing).toEqual(['bump:minor', 'release:skip']); + expect(forge.createdLabels).toHaveLength(0); + }); + + it('should create a missing label once when definitions differ only in case', async () => { + const forge = createFakeForge(); + const defs: LabelDefinition[] = [ + { name: 'bump:minor', color: '0e8a16', description: 'x' }, + { name: 'Bump:Minor', color: '0e8a16', description: 'x' }, + ]; + + const result = await syncLabels(forge, defs); + + expect(forge.createdLabels.map((l) => l.name)).toEqual(['bump:minor']); + expect(result).toEqual({ created: ['bump:minor'], existing: ['Bump:Minor'] }); + }); + + it('should list the repo labels once however many definitions there are', async () => { + const forge = createFakeForge(); + const listSpy = vi.spyOn(forge, 'listLabelNames'); + const defs: LabelDefinition[] = [ + { name: 'bump:minor', color: '0e8a16', description: 'x' }, + { name: 'release:skip', color: 'd93f0b', description: 'y' }, + ]; + + await syncLabels(forge, defs); + + expect(listSpy).toHaveBeenCalledTimes(1); + }); + + it('should count a label created by another run after the listing as existing', async () => { + const forge = createFakeForge({ labelNames: ['bump:minor'] }); + vi.spyOn(forge, 'listLabelNames').mockResolvedValueOnce([]); + const defs: LabelDefinition[] = [{ name: 'bump:minor', color: '0e8a16', description: 'x' }]; + + const result = await syncLabels(forge, defs); + + expect(result).toEqual({ created: [], existing: ['bump:minor'] }); }); it('should rethrow 422 validation errors (e.g. label name too long) so the caller sees the real failure', async () => { diff --git a/packages/release/test/unit/labels-command.spec.ts b/packages/release/test/unit/labels-command.spec.ts index 3e7607cf..6500e18a 100644 --- a/packages/release/test/unit/labels-command.spec.ts +++ b/packages/release/test/unit/labels-command.spec.ts @@ -21,7 +21,6 @@ import { createFakeForge } from '@releasekit/forge'; import { runLabelsSync } from '../../src/commands/labels-command.js'; import { forgeFor } from '../../src/github.js'; -/** A forge whose repo already has `existingLabels` — `createLabel` resolves 'exists' for those. */ function mockForge(existingLabels: string[] = []) { const forge = createFakeForge({ labelNames: existingLabels }); vi.mocked(forgeFor).mockReturnValue(forge); @@ -57,11 +56,15 @@ describe('runLabelsSync', () => { expect(exitSpy).not.toHaveBeenCalled(); }); - it('should be idempotent — 422 already-exists does not fail the run', async () => { - // The forge resolves an existing label to 'exists' rather than throwing — the run succeeds. - mockForge(['bump:minor']); + it('should create only the labels missing from the repo', async () => { + const forge = mockForge(['bump:minor', 'Release']); await expect(runLabelsSync({ repo: 'owner/repo' })).resolves.toBeUndefined(); + + const created = forge.createdLabels.map((l) => l.name); + expect(created).toContain('bump:patch'); + expect(created).not.toContain('bump:minor'); + expect(created).not.toContain('release'); expect(exitSpy).not.toHaveBeenCalled(); }); diff --git a/packages/release/test/unit/standing-pr.spec.ts b/packages/release/test/unit/standing-pr.spec.ts index 8a3f4091..27338d90 100644 --- a/packages/release/test/unit/standing-pr.spec.ts +++ b/packages/release/test/unit/standing-pr.spec.ts @@ -2076,6 +2076,23 @@ describe('runStandingPRUpdate', () => { expect(result.prNumber).toBe(42); }); + it('should create only the ReleaseKit labels missing from the repo', async () => { + const { runVersionStep, runNotesStep } = await import('../../src/steps.js'); + const versionOutput = createMockVersionOutput([{ packageName: '@scope/core', newVersion: '1.2.3' }]); + vi.mocked(runVersionStep).mockResolvedValue(versionOutput as unknown as Awaited>); + vi.mocked(runNotesStep).mockResolvedValue({ packageNotes: {}, releaseNotes: {}, files: [] }); + + const forge = await mockForge({ standingPR: openStandingPR(99), labelNames: ['release', 'Bump:Minor'] }); + + const result = await runStandingPRUpdate({ projectDir: '/test', verbose: false, quiet: false, json: false }); + + expect(result.action).toBe('updated'); + const created = forge.createdLabels.map((l) => l.name); + expect(created).toContain('bump:patch'); + expect(created).not.toContain('release'); + expect(created).not.toContain('bump:minor'); + }); + describe('standing-PR labels as overrides', () => { async function setupWithStandingPRLabels(labelNames: string[]) { const { runVersionStep, runNotesStep } = await import('../../src/steps.js');