Skip to content

Commit affae4e

Browse files
committed
fix(github-client): distinguish API error from no-tags in fetchLatestSemverTag
Changed fetchLatestSemverTag to throw on API errors (401, 403, 404, 5xx, etc.) and return null only when the repo genuinely has no semver tags. Previously, both cases returned undefined, making them indistinguishable to callers. This enables callers to: - Catch API errors and handle them appropriately via classifyError() - Distinguish "repo has no tags" (null return) from network/auth failures Updated all call sites (release-readiness-tool and changelog-draft-tool) to check for null specifically, and updated tests to expect exceptions for API errors.
1 parent 05b8c5a commit affae4e

4 files changed

Lines changed: 31 additions & 24 deletions

File tree

src/server/changelog-draft-tool.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,8 @@ export function registerChangelogDraftTool(server: FastMCP): void {
8282
}
8383

8484
if (!base) {
85-
base = await fetchLatestSemverTag(owner, repo);
86-
if (!base) {
85+
const fetchedTag = await fetchLatestSemverTag(owner, repo);
86+
if (fetchedTag === null) {
8787
return errorRespond(
8888
mkError(
8989
"NOT_FOUND",
@@ -92,6 +92,7 @@ export function registerChangelogDraftTool(server: FastMCP): void {
9292
),
9393
);
9494
}
95+
base = fetchedTag;
9596
}
9697

9798
const cmp = await octokit.repos.compareCommitsWithBasehead({

src/server/github-client.test.ts

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -219,15 +219,27 @@ describe("getOctokit", () => {
219219

220220
describe("fetchLatestSemverTag", () => {
221221
test("returns a semver tag string for a repo that has releases", async () => {
222-
const tag = await fetchLatestSemverTag("Rethunk-AI", "rethunk-github-mcp");
223-
// If auth is absent the function returns undefined — acceptable
224-
if (tag === undefined) return;
225-
expect(tag).toMatch(/^v?\d+\.\d+\.\d+$/);
222+
try {
223+
const tag = await fetchLatestSemverTag("Rethunk-AI", "rethunk-github-mcp");
224+
// If we get a result, it should match semver
225+
if (tag !== null) {
226+
expect(tag).toMatch(/^v?\d+\.\d+\.\d+$/);
227+
}
228+
} catch {
229+
// If auth is absent the function will throw — skip gracefully
230+
return;
231+
}
226232
});
227233

228-
test("returns undefined for a nonexistent repo", async () => {
229-
const tag = await fetchLatestSemverTag("Rethunk-AI", "repo-that-does-not-exist-xyzzy");
230-
expect(tag).toBeUndefined();
234+
test("throws on API errors like 404", async () => {
235+
try {
236+
await fetchLatestSemverTag("Rethunk-AI", "repo-that-does-not-exist-xyzzy");
237+
// Should not reach here if API throws
238+
expect(false).toBe(true);
239+
} catch {
240+
// Expected to throw on API errors or auth failure
241+
return;
242+
}
231243
});
232244
});
233245

src/server/github-client.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -226,18 +226,11 @@ export async function fetchPRMetadata(
226226

227227
/**
228228
* Return the most recent semver tag (vX.Y.Z or X.Y.Z) in a repo.
229-
* Returns `undefined` when no matching tag exists or the request fails.
229+
* Returns `null` when no matching tag exists. Throws on API errors.
230230
*/
231-
export async function fetchLatestSemverTag(
232-
owner: string,
233-
repo: string,
234-
): Promise<string | undefined> {
235-
try {
236-
const octokit = getOctokit();
237-
const res = await octokit.repos.listTags({ owner, repo, per_page: 20 });
238-
const semverRe = /^v?\d+\.\d+\.\d+$/;
239-
return res.data.filter((t) => semverRe.test(t.name))[0]?.name;
240-
} catch {
241-
return undefined;
242-
}
231+
export async function fetchLatestSemverTag(owner: string, repo: string): Promise<string | null> {
232+
const octokit = getOctokit();
233+
const res = await octokit.repos.listTags({ owner, repo, per_page: 20 });
234+
const semverRe = /^v?\d+\.\d+\.\d+$/;
235+
return res.data.filter((t) => semverRe.test(t.name))[0]?.name ?? null;
243236
}

src/server/release-readiness-tool.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ export function registerReleaseReadinessTool(server: FastMCP): void {
9191
}
9292

9393
if (!base) {
94-
base = await fetchLatestSemverTag(owner, repo);
95-
if (!base) {
94+
const fetchedTag = await fetchLatestSemverTag(owner, repo);
95+
if (fetchedTag === null) {
9696
return errorRespond(
9797
mkError(
9898
"NOT_FOUND",
@@ -103,6 +103,7 @@ export function registerReleaseReadinessTool(server: FastMCP): void {
103103
),
104104
);
105105
}
106+
base = fetchedTag;
106107
}
107108

108109
const cmp = await octokit.repos.compareCommitsWithBasehead({

0 commit comments

Comments
 (0)