Skip to content
Closed
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
75 changes: 39 additions & 36 deletions src/github/assignees.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createInstallationToken } from "./app";
import { withInstallationTokenRetry } from "./app";
import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client";
import type { AgentActionMode } from "../settings/agent-execution";

Expand Down Expand Up @@ -35,44 +35,47 @@ export async function ensurePullRequestAssignee(
): Promise<{ applied: boolean }> {
const { owner, repo } = parseRepoFullName(repoFullName);

const token = await createInstallationToken(env, installationId);
// Non-live mode suppresses the assign write; the GET dedup probe below still runs.
const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId));
const existing = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}", {
owner,
repo,
issue_number: pullNumber,
});
const existingAssignees = (existing.data.assignees ?? []) as GitHubUser[];
if (existingAssignees.some((assignee) => assignee.login?.toLowerCase() === login.toLowerCase())) {
return { applied: true };
}

let result;
try {
result = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/assignees", {
// Wrap the whole write path in withInstallationTokenRetry so a stale cached installation token self-heals
// exactly once — matching comments.ts / pr-actions.ts (#6191).
return withInstallationTokenRetry(env, installationId, async (token) => {
// Non-live mode suppresses the assign write; the GET dedup probe below still runs.
const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId));
const existing = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}", {
owner,
repo,
issue_number: pullNumber,
assignees: [login],
});
} catch (error: unknown) {
// GitHub blocks assigning bot/agent logins via App installation tokens (HTTP 403). This is a GitHub
// platform restriction with no workaround using installation-token auth. Return applied:false so the
// caller can fall back to a by:{login} label instead of propagating an unactionable error.
if (
typeof error === "object" &&
error !== null &&
"status" in error &&
(error as { status: number }).status === 403 &&
"message" in error &&
typeof (error as { message: unknown }).message === "string" &&
(error as { message: string }).message.includes("Assigning agents is not supported")
) {
return { applied: false };
const existingAssignees = (existing.data.assignees ?? []) as GitHubUser[];
if (existingAssignees.some((assignee) => assignee.login?.toLowerCase() === login.toLowerCase())) {
return { applied: true };
}
throw error;
}
const resultAssignees = (result.data.assignees ?? []) as GitHubUser[];
return { applied: resultAssignees.some((assignee) => assignee.login?.toLowerCase() === login.toLowerCase()) };

let result;
try {
result = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/assignees", {
owner,
repo,
issue_number: pullNumber,
assignees: [login],
});
} catch (error: unknown) {
// GitHub blocks assigning bot/agent logins via App installation tokens (HTTP 403). This is a GitHub
// platform restriction with no workaround using installation-token auth. Return applied:false so the
// caller can fall back to a by:{login} label instead of propagating an unactionable error.
if (
typeof error === "object" &&
error !== null &&
"status" in error &&
(error as { status: number }).status === 403 &&
"message" in error &&
typeof (error as { message: unknown }).message === "string" &&
(error as { message: string }).message.includes("Assigning agents is not supported")
) {
return { applied: false };
}
throw error;
}
const resultAssignees = (result.data.assignees ?? []) as GitHubUser[];
return { applied: resultAssignees.some((assignee) => assignee.login?.toLowerCase() === login.toLowerCase()) };
});
}
106 changes: 56 additions & 50 deletions src/github/labels.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createInstallationToken } from "./app";
import { withInstallationTokenRetry } from "./app";
import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client";
import type { AgentActionMode } from "../settings/agent-execution";

Expand Down Expand Up @@ -28,45 +28,48 @@ export async function ensurePullRequestLabel(
): Promise<{ applied: boolean; created: boolean }> {
const { owner, repo } = parseRepoFullName(repoFullName);

const token = await createInstallationToken(env, installationId);
// Non-live mode suppresses the label create + apply writes; the GET dedup probe below still runs.
const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId));
const existing = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/labels", {
owner,
repo,
issue_number: pullNumber,
per_page: 100,
});
const labels = existing.data as GitHubLabel[];
if (labels.some((label) => label.name?.toLowerCase() === labelName.toLowerCase())) {
return { applied: false, created: false };
}
// Wrap the whole write path in withInstallationTokenRetry so a stale cached installation token self-heals
// exactly once — matching comments.ts / pr-actions.ts (#6191).
return withInstallationTokenRetry(env, installationId, async (token) => {
// Non-live mode suppresses the label create + apply writes; the GET dedup probe below still runs.
const octokit = makeInstallationOctokit(env, token, options.mode ?? "live", githubRateLimitAdmissionKeyForInstallation(installationId));
const existing = await octokit.request("GET /repos/{owner}/{repo}/issues/{issue_number}/labels", {
owner,
repo,
issue_number: pullNumber,
per_page: 100,
});
const labels = existing.data as GitHubLabel[];
if (labels.some((label) => label.name?.toLowerCase() === labelName.toLowerCase())) {
return { applied: false, created: false };
}

let created = false;
if (options.createMissingLabel) {
try {
await octokit.request("POST /repos/{owner}/{repo}/labels", {
owner,
repo,
name: labelName,
color: "7ee787",
description: "Gittensor contributor context",
});
created = true;
} catch (error) {
const e = error as { status?: number; message?: string };
// Only swallow the specific "already_exists" duplicate; other 422s (e.g. invalid name) must propagate.
if (e.status !== 422 || !e.message?.includes("already_exists")) throw error;
let created = false;
if (options.createMissingLabel) {
try {
await octokit.request("POST /repos/{owner}/{repo}/labels", {
owner,
repo,
name: labelName,
color: "7ee787",
description: "Gittensor contributor context",
});
created = true;
} catch (error) {
const e = error as { status?: number; message?: string };
// Only swallow the specific "already_exists" duplicate; other 422s (e.g. invalid name) must propagate.
if (e.status !== 422 || !e.message?.includes("already_exists")) throw error;
}
}
}

await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/labels", {
owner,
repo,
issue_number: pullNumber,
labels: [labelName],
await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/labels", {
owner,
repo,
issue_number: pullNumber,
labels: [labelName],
});
return { applied: true, created };
});
return { applied: true, created };
}

/** Remove a single label from a PR if present. Best-effort — a 404 (label not on the PR) is ignored; any other
Expand All @@ -81,18 +84,21 @@ export async function removePullRequestLabel(env: Env, installationId: number, r
} catch {
return;
}
const token = await createInstallationToken(env, installationId);
const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId));
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;
}
// Self-heal a stale cached installation token once before giving up (#6191).
await withInstallationTokenRetry(env, installationId, async (token) => {
const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId));
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).
// Note: a 401 bad_credentials here is NOT swallowed — withInstallationTokenRetry sees it and retries once.
if (e.status !== 404) throw error;
}
});
}
34 changes: 34 additions & 0 deletions test/unit/github-assignees.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { generateKeyPairSync } from "node:crypto";
import { clearInstallationTokenCacheForTest } from "../../src/github/app";
import { ensurePullRequestAssignee } from "../../src/github/assignees";
import { createTestEnv } from "../helpers/d1";

describe("GitHub PR assignees (#3182)", () => {
afterEach(() => {
vi.unstubAllGlobals();
clearInstallationTokenCacheForTest();
});

it("rejects invalid repository names before making GitHub calls", async () => {
Expand Down Expand Up @@ -132,6 +134,38 @@ describe("GitHub PR assignees (#3182)", () => {

expect(result).toEqual({ applied: false });
});

it("ensurePullRequestAssignee: evicts a stale installation token and retries once on 401 (#6191)", async () => {
clearInstallationTokenCacheForTest();
let tokenMints = 0;
let issueGetAttempts = 0;
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) {
tokenMints += 1;
return Response.json({ token: `token-${tokenMints}` });
}
if (url.includes("/issues/4") && !url.includes("/assignees") && method === "GET") {
issueGetAttempts += 1;
if (issueGetAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 });
return Response.json({ assignees: [{ login: "alice" }] });
}
return new Response("unexpected", { status: 500 });
});

const result = await ensurePullRequestAssignee(
createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }),
998877,
"JSONbored/gittensory",
4,
"alice",
);

expect(result).toEqual({ applied: true });
expect(issueGetAttempts).toBe(2);
expect(tokenMints).toBe(2);
});
});

function generateRsaPrivateKeyPem(): string {
Expand Down
61 changes: 61 additions & 0 deletions test/unit/github-labels.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { generateKeyPairSync } from "node:crypto";
import { clearInstallationTokenCacheForTest } from "../../src/github/app";
import { ensurePullRequestLabel, removePullRequestLabel } from "../../src/github/labels";
import { createTestEnv } from "../helpers/d1";

describe("GitHub PR labels", () => {
afterEach(() => {
vi.unstubAllGlobals();
clearInstallationTokenCacheForTest();
});

it("rejects invalid repository names before making GitHub calls", async () => {
Expand Down Expand Up @@ -111,6 +113,65 @@ describe("GitHub PR labels", () => {
).resolves.toBeUndefined();
});

it("ensurePullRequestLabel: evicts a stale installation token and retries once on 401 (#6191)", async () => {
clearInstallationTokenCacheForTest();
let tokenMints = 0;
let labelGetAttempts = 0;
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) {
tokenMints += 1;
return Response.json({ token: `token-${tokenMints}` });
}
if (url.includes("/issues/4/labels") && method === "GET") {
labelGetAttempts += 1;
if (labelGetAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 });
return Response.json([{ name: "gittensor" }]);
}
return new Response("unexpected", { status: 500 });
});

const result = await ensurePullRequestLabel(
createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }),
998877,
"JSONbored/gittensory",
4,
"gittensor",
{ createMissingLabel: true },
);

expect(result).toEqual({ applied: false, created: false });
expect(labelGetAttempts).toBe(2);
expect(tokenMints).toBe(2);
});

it("removePullRequestLabel: evicts a stale installation token and retries once on 401 (#6191)", async () => {
clearInstallationTokenCacheForTest();
let tokenMints = 0;
let deleteAttempts = 0;
vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => {
const url = input.toString();
const method = init?.method ?? "GET";
if (url.includes("/access_tokens")) {
tokenMints += 1;
return Response.json({ token: `token-${tokenMints}` });
}
if (url.includes("/issues/4/labels/") && method === "DELETE") {
deleteAttempts += 1;
if (deleteAttempts === 1) return Response.json({ message: "Bad credentials" }, { status: 401 });
return new Response(null, { status: 204 });
}
return new Response("unexpected", { status: 500 });
});

await expect(
removePullRequestLabel(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 998877, "JSONbored/gittensory", 4, "gittensor"),
).resolves.toBeUndefined();
expect(deleteAttempts).toBe(2);
expect(tokenMints).toBe(2);
});

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