fix: canonicalize releases after concurrent create (#746)

Signed-off-by: Rui Chen <rui@chenrui.dev>
This commit is contained in:
Rui Chen
2026-03-14 20:48:22 -04:00
committed by GitHub
parent 71d29a04ae
commit ab416a1836
3 changed files with 280 additions and 20 deletions

View File

@@ -77,6 +77,7 @@ describe('github', () => {
}, },
listReleaseAssets: () => Promise.reject('Not implemented'), listReleaseAssets: () => Promise.reject('Not implemented'),
deleteReleaseAsset: () => Promise.reject('Not implemented'), deleteReleaseAsset: () => Promise.reject('Not implemented'),
deleteRelease: () => Promise.reject('Not implemented'),
uploadReleaseAsset: () => Promise.reject('Not implemented'), uploadReleaseAsset: () => Promise.reject('Not implemented'),
} as const; } as const;
@@ -185,6 +186,7 @@ describe('github', () => {
}, },
listReleaseAssets: () => Promise.reject('Not implemented'), listReleaseAssets: () => Promise.reject('Not implemented'),
deleteReleaseAsset: () => Promise.reject('Not implemented'), deleteReleaseAsset: () => Promise.reject('Not implemented'),
deleteRelease: () => Promise.reject('Not implemented'),
uploadReleaseAsset: () => Promise.reject('Not implemented'), uploadReleaseAsset: () => Promise.reject('Not implemented'),
}; };
@@ -262,6 +264,7 @@ describe('github', () => {
}, },
listReleaseAssets: () => Promise.reject('Not implemented'), listReleaseAssets: () => Promise.reject('Not implemented'),
deleteReleaseAsset: () => Promise.reject('Not implemented'), deleteReleaseAsset: () => Promise.reject('Not implemented'),
deleteRelease: () => Promise.reject('Not implemented'),
uploadReleaseAsset: () => Promise.reject('Not implemented'), uploadReleaseAsset: () => Promise.reject('Not implemented'),
} as const; } as const;
@@ -269,5 +272,114 @@ describe('github', () => {
assert.ok(result); assert.ok(result);
assert.equal(result.id, 1); assert.equal(result.id, 1);
}); });
it('reuses a canonical release after concurrent create success and removes empty duplicates', async () => {
const canonicalRelease: Release = {
id: 1,
upload_url: 'canonical-upload',
html_url: 'canonical-html',
tag_name: 'v1.0.0',
name: 'canonical',
body: 'test',
target_commitish: 'main',
draft: true,
prerelease: false,
assets: [],
};
const duplicateRelease: Release = {
id: 2,
upload_url: 'duplicate-upload',
html_url: 'duplicate-html',
tag_name: 'v1.0.0',
name: 'duplicate',
body: 'test',
target_commitish: 'main',
draft: true,
prerelease: false,
assets: [],
};
let lookupCount = 0;
const deleteReleaseSpy = vi.fn(async () => undefined);
const mockReleaser: Releaser = {
getReleaseByTag: () => {
lookupCount += 1;
if (lookupCount === 1) {
return Promise.reject({ status: 404 });
}
return Promise.resolve({ data: canonicalRelease });
},
createRelease: () => Promise.resolve({ data: duplicateRelease }),
updateRelease: () => Promise.reject('Not implemented'),
finalizeRelease: () => Promise.reject('Not implemented'),
allReleases: async function* () {
yield { data: [duplicateRelease, canonicalRelease] };
},
listReleaseAssets: () => Promise.reject('Not implemented'),
deleteReleaseAsset: () => Promise.reject('Not implemented'),
deleteRelease: deleteReleaseSpy,
uploadReleaseAsset: () => Promise.reject('Not implemented'),
};
const result = await release(config, mockReleaser, 2);
assert.equal(result.id, canonicalRelease.id);
expect(deleteReleaseSpy).toHaveBeenCalledWith({
owner: 'owner',
repo: 'repo',
release_id: duplicateRelease.id,
});
});
it('falls back to recent releases when tag lookup still lags after create', async () => {
const canonicalRelease: Release = {
id: 1,
upload_url: 'canonical-upload',
html_url: 'canonical-html',
tag_name: 'v1.0.0',
name: 'canonical',
body: 'test',
target_commitish: 'main',
draft: true,
prerelease: false,
assets: [],
};
const duplicateRelease: Release = {
id: 2,
upload_url: 'duplicate-upload',
html_url: 'duplicate-html',
tag_name: 'v1.0.0',
name: 'duplicate',
body: 'test',
target_commitish: 'main',
draft: true,
prerelease: false,
assets: [],
};
const deleteReleaseSpy = vi.fn(async () => undefined);
const mockReleaser: Releaser = {
getReleaseByTag: () => Promise.reject({ status: 404 }),
createRelease: () => Promise.resolve({ data: duplicateRelease }),
updateRelease: () => Promise.reject('Not implemented'),
finalizeRelease: () => Promise.reject('Not implemented'),
allReleases: async function* () {
yield { data: [duplicateRelease, canonicalRelease] };
},
listReleaseAssets: () => Promise.reject('Not implemented'),
deleteReleaseAsset: () => Promise.reject('Not implemented'),
deleteRelease: deleteReleaseSpy,
uploadReleaseAsset: () => Promise.reject('Not implemented'),
};
const result = await release(config, mockReleaser, 1);
assert.equal(result.id, canonicalRelease.id);
expect(deleteReleaseSpy).toHaveBeenCalledWith({
owner: 'owner',
repo: 'repo',
release_id: duplicateRelease.id,
});
});
}); });
}); });

36
dist/index.js vendored

File diff suppressed because one or more lines are too long

View File

@@ -75,6 +75,8 @@ export interface Releaser {
deleteReleaseAsset(params: { owner: string; repo: string; asset_id: number }): Promise<void>; deleteReleaseAsset(params: { owner: string; repo: string; asset_id: number }): Promise<void>;
deleteRelease(params: { owner: string; repo: string; release_id: number }): Promise<void>;
uploadReleaseAsset(params: { uploadReleaseAsset(params: {
url: string; url: string;
size: number; size: number;
@@ -224,6 +226,10 @@ export class GitHubReleaser implements Releaser {
await this.github.rest.repos.deleteReleaseAsset(params); await this.github.rest.repos.deleteReleaseAsset(params);
} }
async deleteRelease(params: { owner: string; repo: string; release_id: number }): Promise<void> {
await this.github.rest.repos.deleteRelease(params);
}
async uploadReleaseAsset(params: { async uploadReleaseAsset(params: {
url: string; url: string;
size: number; size: number;
@@ -525,6 +531,141 @@ export async function findTagFromReleases(
} }
} }
const CREATED_RELEASE_DISCOVERY_RETRY_DELAY_MS = 1000;
const RECENT_RELEASE_SCAN_PAGES = 2;
async function sleep(ms: number): Promise<void> {
await new Promise((resolve) => setTimeout(resolve, ms));
}
async function recentReleasesByTag(
releaser: Releaser,
owner: string,
repo: string,
tag: string,
): Promise<Release[]> {
const matches: Release[] = [];
let pages = 0;
for await (const page of releaser.allReleases({ owner, repo })) {
matches.push(...page.data.filter((release) => release.tag_name === tag));
pages += 1;
if (pages >= RECENT_RELEASE_SCAN_PAGES) {
break;
}
}
return matches;
}
function pickCanonicalRelease(
releases: Release[],
releaseByTag: Release | undefined,
): Release | undefined {
if (releaseByTag && releases.some((release) => release.id === releaseByTag.id)) {
return releaseByTag;
}
if (releases.length === 0) {
return releaseByTag;
}
return [...releases].sort((left, right) => {
if (left.draft !== right.draft) {
return Number(left.draft) - Number(right.draft);
}
return left.id - right.id;
})[0];
}
async function cleanupDuplicateDraftReleases(
releaser: Releaser,
owner: string,
repo: string,
tag: string,
canonicalReleaseId: number,
recentReleases: Release[],
): Promise<void> {
for (const duplicate of recentReleases) {
if (duplicate.id === canonicalReleaseId || !duplicate.draft || duplicate.assets.length > 0) {
continue;
}
try {
console.log(`🧹 Removing duplicate draft release ${duplicate.id} for tag ${tag}...`);
await releaser.deleteRelease({
owner,
repo,
release_id: duplicate.id,
});
} catch (error) {
console.warn(`error deleting duplicate release ${duplicate.id}: ${error}`);
}
}
}
async function canonicalizeCreatedRelease(
releaser: Releaser,
owner: string,
repo: string,
tag: string,
createdRelease: Release,
maxRetries: number,
): Promise<Release> {
const attempts = Math.max(maxRetries, 1);
for (let attempt = 1; attempt <= attempts; attempt += 1) {
let releaseByTag: Release | undefined;
try {
releaseByTag = await findTagFromReleases(releaser, owner, repo, tag);
} catch (error) {
console.warn(`error reloading release for tag ${tag}: ${error}`);
}
let recentReleases: Release[] = [];
try {
recentReleases = await recentReleasesByTag(releaser, owner, repo, tag);
} catch (error) {
console.warn(`error listing recent releases for tag ${tag}: ${error}`);
}
const canonicalRelease = pickCanonicalRelease(recentReleases, releaseByTag);
if (canonicalRelease) {
if (canonicalRelease.id !== createdRelease.id) {
console.log(
`↪️ Using release ${canonicalRelease.id} for tag ${tag} instead of duplicate draft ${createdRelease.id}`,
);
}
await cleanupDuplicateDraftReleases(
releaser,
owner,
repo,
tag,
canonicalRelease.id,
recentReleases,
);
return canonicalRelease;
}
if (attempt < attempts) {
console.log(
`Release ${createdRelease.id} is not yet discoverable by tag ${tag}, retrying... (${
attempts - attempt
} retries remaining)`,
);
await sleep(CREATED_RELEASE_DISCOVERY_RETRY_DELAY_MS);
}
}
console.log(
`⚠️ Continuing with newly created release ${createdRelease.id} because tag ${tag} is still not discoverable`,
);
return createdRelease;
}
async function createRelease( async function createRelease(
tag: string, tag: string,
config: Config, config: Config,
@@ -547,7 +688,7 @@ async function createRelease(
} }
console.log(`👩‍🏭 Creating new GitHub release for tag ${tag_name}${commitMessage}...`); console.log(`👩‍🏭 Creating new GitHub release for tag ${tag_name}${commitMessage}...`);
try { try {
let release = await releaser.createRelease({ const release = await releaser.createRelease({
owner, owner,
repo, repo,
tag_name, tag_name,
@@ -560,7 +701,14 @@ async function createRelease(
generate_release_notes, generate_release_notes,
make_latest, make_latest,
}); });
return release.data; return await canonicalizeCreatedRelease(
releaser,
owner,
repo,
tag_name,
release.data,
maxRetries,
);
} catch (error) { } catch (error) {
// presume a race with competing matrix runs // presume a race with competing matrix runs
console.log(`⚠️ GitHub release failed with status: ${error.status}`); console.log(`⚠️ GitHub release failed with status: ${error.status}`);