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
21 changes: 16 additions & 5 deletions src/github/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,10 @@ export async function ensurePullRequestLabel(
return { applied: true, created };
}

/** Remove a single label from a PR if present. Best-effort — a 404 (label not on the PR) is ignored. Used to
* keep the mutually-exclusive managed TYPE labels (gittensor:bug/feature/priority) down to exactly one. */
/** Remove a single label from a PR if present. Best-effort — a 404 (label not on the PR) is ignored; any other
* GitHub API failure (403/5xx/rate-limit) is re-thrown, matching {@link ensurePullRequestLabel}'s narrow-swallow
* convention. Used to keep the mutually-exclusive managed TYPE labels (gittensor:bug/feature/priority) down to
* exactly one. */
export async function removePullRequestLabel(env: Env, installationId: number, repoFullName: string, pullNumber: number, labelName: string, mode: AgentActionMode = "live"): Promise<void> {
let owner: string;
let repo: string;
Expand All @@ -81,7 +83,16 @@ export async function removePullRequestLabel(env: Env, installationId: number, r
}
const token = await createInstallationToken(env, installationId);
const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId));
await octokit
.request("DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}", { owner, repo, issue_number: pullNumber, name: labelName })
.catch(() => undefined);
try {
await octokit.request("DELETE /repos/{owner}/{repo}/issues/{issue_number}/labels/{name}", {
owner,
repo,
issue_number: pullNumber,
name: labelName,
});
} catch (error) {
const e = error as { status?: number };
// Only swallow the documented "label not on the PR" 404; other failures must propagate (#6192).
if (e.status !== 404) throw error;
}
}
53 changes: 53 additions & 0 deletions test/unit/github-labels.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,59 @@ describe("GitHub PR labels", () => {
expect(called).toBe(false);
});

it("removePullRequestLabel: swallows a 404 when the label is already absent (#6192)", async () => {
const calls: string[] = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
calls.push(`${method} ${url}`);
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/4/labels/") && method === "DELETE") {
return Response.json({ message: "Label does not exist" }, { status: 404 });
}
return new Response("unexpected", { status: 500 });
});

await expect(
removePullRequestLabel(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "gittensor"),
).resolves.toBeUndefined();
expect(calls.some((call) => call.startsWith("DELETE ") && call.includes("/labels/"))).toBe(true);
});

it("removePullRequestLabel: re-throws non-404 failures (403/5xx) instead of swallowing them (#6192)", async () => {
for (const status of [403, 500]) {
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/4/labels/") && method === "DELETE") {
return Response.json({ message: status === 403 ? "Resource not accessible by integration" : "Internal Server Error" }, { status });
}
return new Response("unexpected", { status: 599 });
});

await expect(
removePullRequestLabel(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "gittensor"),
).rejects.toMatchObject({ status });
}
});

it("removePullRequestLabel: succeeds when DELETE returns 200/204", async () => {
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" });
if (url.includes("/issues/4/labels/") && method === "DELETE") {
return new Response(null, { status: 204 });
}
return new Response("unexpected", { status: 500 });
});

await expect(
removePullRequestLabel(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "gittensor"),
).resolves.toBeUndefined();
});

it("does nothing when the label is already applied", async () => {
const calls: string[] = [];
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
Expand Down