diff --git a/apps/gittensory-ui/public/openapi.json b/apps/gittensory-ui/public/openapi.json index d6401650f5..bbdeeedefa 100644 --- a/apps/gittensory-ui/public/openapi.json +++ b/apps/gittensory-ui/public/openapi.json @@ -8830,6 +8830,26 @@ "auto_with_approval", "auto" ] + }, + "update_branch": { + "type": "string", + "enum": [ + "observe", + "suggest", + "propose", + "auto_with_approval", + "auto" + ] + }, + "assign": { + "type": "string", + "enum": [ + "observe", + "suggest", + "propose", + "auto_with_approval", + "auto" + ] } } }, diff --git a/src/github/assignees.ts b/src/github/assignees.ts new file mode 100644 index 0000000000..6831d9755b --- /dev/null +++ b/src/github/assignees.ts @@ -0,0 +1,59 @@ +import { createInstallationToken } from "./app"; +import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; +import type { AgentActionMode } from "../settings/agent-execution"; + +type GitHubUser = { + login?: string | null; +}; + +function parseRepoFullName(repoFullName: string): { owner: string; repo: string } { + const parts = repoFullName.split("/"); + const owner = parts[0]; + const repo = parts[1]; + // Reject any whitespace (leading, trailing, or per-segment like `owner/ repo`) so a padded slug can never + // reach a GitHub call — a valid owner/repo name never contains spaces. + if (parts.length !== 2 || !owner || !repo || /\s/.test(repoFullName)) { + throw new Error(`Invalid repository full name: ${repoFullName}`); + } + return { owner, repo }; +} + +/** + * Best-effort assign a single login to a PR (#3182). GitHub requires the ASSIGNEE (not just the caller) to + * have push/triage access to the repo -- an external contributor almost never does, and the assignees endpoint + * silently drops an ineligible login from the response rather than erroring. `applied` reflects the actual + * post-call assignee list, not just a lack of a thrown error, so the caller can detect the silent-drop case and + * fall back to something that isn't gated by repo membership (see `performAction`'s "assign" case). + */ +export async function ensurePullRequestAssignee( + env: Env, + installationId: number, + repoFullName: string, + pullNumber: number, + login: string, + options: { mode?: AgentActionMode } = {}, +): 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 }; + } + + const result = await octokit.request("POST /repos/{owner}/{repo}/issues/{issue_number}/assignees", { + owner, + repo, + issue_number: pullNumber, + assignees: [login], + }); + const resultAssignees = (result.data.assignees ?? []) as GitHubUser[]; + return { applied: resultAssignees.some((assignee) => assignee.login?.toLowerCase() === login.toLowerCase()) }; +} diff --git a/src/openapi/schemas.ts b/src/openapi/schemas.ts index faf32a4776..9292d6e456 100644 --- a/src/openapi/schemas.ts +++ b/src/openapi/schemas.ts @@ -673,7 +673,7 @@ export const RepositorySettingsSchema = z ) .optional(), autonomy: z - .record(z.enum(["review", "request_changes", "approve", "merge", "close", "label", "review_state_label"]), z.enum(["observe", "suggest", "propose", "auto_with_approval", "auto"])) + .record(z.enum(["review", "request_changes", "approve", "merge", "close", "label", "review_state_label", "update_branch", "assign"]), z.enum(["observe", "suggest", "propose", "auto_with_approval", "auto"])) .optional(), autoMaintain: z.object({ requireApprovals: z.number().int(), mergeMethod: z.enum(["merge", "squash", "rebase"]) }).optional(), agentPaused: z.boolean().optional(), diff --git a/src/queue/processors.ts b/src/queue/processors.ts index f2c07b36b5..7cdc9be988 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -2553,6 +2553,7 @@ async function runAgentMaintenancePlanAndExecute( headSha: pr.headSha, mergeBlockedSha: pr.mergeBlockedSha, approvedHeadSha: pr.approvedHeadSha, + authorLogin: pr.authorLogin, }, }); // Accuracy circuit-breakers (#self-improve / GAP-4): two INDEPENDENT, fail-open precision breakers, chained. diff --git a/src/services/agent-action-executor.ts b/src/services/agent-action-executor.ts index 78169a7744..b9c418f8f1 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -18,6 +18,7 @@ import { notifyActionToDiscord, notifyActionToSlack, type NotifyOutcome } from " import { cancelInFlightWorkflowRunsForHeadSha, createInstallationToken, githubErrorStatus, isGitHubRateLimitedError } from "../github/app"; import { fetchLiveCiAggregate, mergeRequiredCiContexts, refreshInstallationHealthForInstallation } from "../github/backfill"; import { githubRateLimitAdmissionKeyForToken } from "../github/client"; +import { ensurePullRequestAssignee } from "../github/assignees"; import { ensurePullRequestLabel, removePullRequestLabel } from "../github/labels"; import { closeIssue, closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../github/pr-actions"; import { fetchPullRequestFreshness, pullRequestFreshnessDetail } from "../github/pr-freshness"; @@ -698,6 +699,21 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: await updatePullRequestBranch(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, updateSha ?? undefined); return; } + case "assign": { + const login = action.assignee ?? ""; + if (!login) return; + const result = await ensurePullRequestAssignee(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, login); + if (!result.applied) { + // GitHub silently drops an assignee lacking push/triage access to the repo -- the common case for an + // external contributor. Fall back to a per-login label instead of a comment: ensurePullRequestLabel's + // own GET dedup makes this idempotent, so a repeated sweep never re-posts/spams once the label exists. + // Prefix kept short ("by:", not "contributor:") -- GitHub logins run up to 39 chars and label names cap + // at 50, so a longer prefix can push a valid max-length login past the limit and fail this fallback for + // exactly the contributors it exists to cover. + await ensurePullRequestLabel(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, `by:${login}`, { createMissingLabel: true }); + } + return; + } } } diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index 5c02c7e856..25ccd0fd8a 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -121,6 +121,10 @@ export type PlannedAgentAction = { // (which still counts toward a "require approving reviews" branch-protection rule) must not be left in place. // (#2254) dismissStaleApproval?: boolean; + // For an `assign` action (#3182): the login to set as the PR's GitHub assignee -- the PR's own opening + // contributor. GitHub silently drops an assignee lacking push/triage access rather than erroring, so the + // executor falls back to a per-login label when the real assignment doesn't stick. + assignee?: string; }; // Gate-blocker codes backed by CONCRETE, non-judgment evidence: a committed secret, a deterministic @@ -311,6 +315,11 @@ export type AgentActionPlanInput = { // does NOT reliably flip reviewDecision to APPROVED, so without this the bot re-approves every sweep). A new // commit makes the live head differ → the bot may approve the new code (correct). approvedHeadSha?: string | null | undefined; + // The PR's opening contributor (#3182), threaded through ONLY for the `assign` disposition below. Absent + // (undefined) on the several other, narrower callers of this planner (issue-cap/review-nag short-circuits) + // is harmless -- those all set `conclusion: "skipped"` or hit an earlier short-circuit `return`, so the + // `assign` block below is unreachable from them regardless. + authorLogin?: string | null | undefined; }; }; @@ -759,6 +768,19 @@ export function planAgentMaintenanceActions(input: AgentActionPlanInput): Planne } } + // 2b) assign (#3182) — best-effort: set the PR's own opening contributor as its GitHub assignee, purely for + // triage (who is this PR's author, at a glance). Independent of merge/close/CI outcome, exactly like + // review_state_label above: gated on its own `assign` class (default OFF), not tied to any other disposition. + // Idempotent at the executor (a live GET before the POST), so replanning this every sweep is harmless. + if (acting("assign") && input.pr.authorLogin) { + actions.push({ + actionClass: "assign", + requiresApproval: approval("assign"), + reason: "auto-assign PR opener", + assignee: input.pr.authorLogin, + }); + } + // 3) review — APPROVE a review-good PR only when it is NOT on a guarded path; a guarded PR falls through to the // owner's manual safety review (never auto-approved). The bot NEVER posts a formal CHANGES_REQUESTED review: a // blocking review counts against required approvals and STRANDS a PR when it later goes green (a stale diff --git a/src/settings/autonomy.ts b/src/settings/autonomy.ts index 98b06ba610..594cf62172 100644 --- a/src/settings/autonomy.ts +++ b/src/settings/autonomy.ts @@ -8,8 +8,9 @@ export const AUTONOMY_LEVELS = ["observe", "suggest", "propose", "auto_with_appr // (#label-scoping) is a separate class from `label`: it gates the planner's own disposition-communication // labels (ready-to-merge / changes-requested / manual-review / migration-collision / pending-closure / // new-account), independent of the anti-abuse enforcement labels (blacklist/contributor-cap/review-nag), which -// ride on `close` instead -- see agent-actions.ts. -export const AGENT_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "review_state_label", "update_branch"] as const; +// ride on `close` instead -- see agent-actions.ts. `assign` (#3182) is its own independent class, same shape: +// best-effort assignment of the PR's opening contributor, unrelated to merge/close/approve. +export const AGENT_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label", "review_state_label", "update_branch", "assign"] as const; // Deny-by-default: any action class with no explicit, valid level resolves to this. export const DEFAULT_AUTONOMY_LEVEL: AutonomyLevel = "observe"; diff --git a/src/types.ts b/src/types.ts index 966c1e2ff9..c81dfb5354 100644 --- a/src/types.ts +++ b/src/types.ts @@ -985,8 +985,10 @@ export type AutonomyLevel = "observe" | "suggest" | "propose" | "auto_with_appro * labels (ready-to-merge / changes-requested / manual-review / migration-collision / the linked-issue * pending-closure flag / the account-age new-account label) -- these are advisory signals about the bot's own * verdict, not enforcement actions, and default OFF (`observe`) like every other class so a one-shot-mode repo - * never sees them without an explicit opt-in. */ -export type AgentActionClass = "review" | "request_changes" | "approve" | "merge" | "close" | "label" | "review_state_label" | "update_branch"; + * never sees them without an explicit opt-in. `assign` (#3182) sets the PR's opening contributor as the GitHub + * assignee -- an independent, always-safe triage action with no bearing on merge/close/approve, gated purely + * on its own dial like `review_state_label`. */ +export type AgentActionClass = "review" | "request_changes" | "approve" | "merge" | "close" | "label" | "review_state_label" | "update_branch" | "assign"; /** Per-action-class autonomy. An unset class resolves to `observe` (deny-by-default). */ export type AutonomyPolicy = Partial>; diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index d0a44941f4..3c5029d339 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -13,6 +13,9 @@ vi.mock("../../src/github/labels", () => ({ ensurePullRequestLabel: vi.fn(async () => ({ applied: true, created: false })), removePullRequestLabel: vi.fn(async () => undefined), })); +vi.mock("../../src/github/assignees", () => ({ + ensurePullRequestAssignee: vi.fn(async () => ({ applied: true })), +})); vi.mock("../../src/github/pr-freshness", async (importOriginal) => { const actual = await importOriginal(); return { @@ -38,6 +41,7 @@ vi.mock("../../src/github/backfill", async (importOriginal) => ({ import { closeIssue, closePullRequest, createIssueComment, createPullRequestReview, dismissLatestBotApproval, mergePullRequest, updatePullRequestBranch } from "../../src/github/pr-actions"; import { ensurePullRequestLabel, removePullRequestLabel } from "../../src/github/labels"; +import { ensurePullRequestAssignee } from "../../src/github/assignees"; import { fetchPullRequestFreshness } from "../../src/github/pr-freshness"; import { createInstallationToken } from "../../src/github/app"; import { fetchLiveCiAggregate, refreshInstallationHealthForInstallation } from "../../src/github/backfill"; @@ -493,6 +497,40 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(actionParams(flag)).toEqual({ label: "pending-closure", labelOp: "add", comment: "⚠️ flagged" }); }); + it("LIVE assign (#3182): calls ensurePullRequestAssignee with the planned login and does not fall back to a label when it sticks", async () => { + const env = createTestEnv({}); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice" }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "alice"); + expect(ensurePullRequestLabel).not.toHaveBeenCalled(); + expect(outcomes[0]?.outcome).toBe("completed"); + }); + + it("LIVE assign (#3182): falls back to a per-login label when GitHub silently drops an ineligible assignee", async () => { + const env = createTestEnv({}); + vi.mocked(ensurePullRequestAssignee).mockResolvedValueOnce({ applied: false }); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "external-contributor" }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(ensurePullRequestLabel).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "by:external-contributor", { createMissingLabel: true }); + expect(outcomes[0]?.outcome).toBe("completed"); + }); + + it("assign with no login is a no-op (defensive — the planner always sets it, but the executor must not call GitHub with an empty login)", async () => { + const env = createTestEnv({}); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener" }; + await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(ensurePullRequestAssignee).not.toHaveBeenCalled(); + expect(ensurePullRequestLabel).not.toHaveBeenCalled(); + }); + + it("assign is denied when the assign autonomy class is not acting", async () => { + const env = createTestEnv({}); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice" }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: {} }), [assign]); + expect(outcomes[0]?.outcome).toBe("denied"); + expect(ensurePullRequestAssignee).not.toHaveBeenCalled(); + }); + it("LIVE approve persists the approved head SHA for re-approval idempotency", async () => { const env = createTestEnv({}); await env.DB.prepare("insert into pull_requests (id, repo_full_name, number, title, state, head_sha, payload_json, created_at, updated_at) values (?,?,?,?,?,?,?,?,?)") diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index c553624d87..dcd20c0e85 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -899,6 +899,39 @@ describe("planAgentMaintenanceActions (#778)", () => { }); }); +describe("assign — auto-assign PR opener (#3182)", () => { + it("plans nothing when the assign class is not acting (default observe)", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: {}, pr: { labels: [], authorLogin: "alice" } })); + expect(classes(plan)).not.toContain("assign"); + }); + + it("plans an assign action for the PR's opener when acting", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [], authorLogin: "alice" } })); + expect(plan).toContainEqual(expect.objectContaining({ actionClass: "assign", assignee: "alice", requiresApproval: false })); + }); + + it("stages for approval under auto_with_approval", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto_with_approval" }, pr: { labels: [], authorLogin: "alice" } })); + expect(plan).toContainEqual(expect.objectContaining({ actionClass: "assign", assignee: "alice", requiresApproval: true })); + }); + + it("plans nothing when authorLogin is absent (the several narrower callers that never populate it)", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [] } })); + expect(classes(plan)).not.toContain("assign"); + }); + + it("is independent of merge/close outcome — still plans assign on a failing verdict", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "failure", autonomy: { assign: "auto" }, blockerTitles: ["x"], pr: { labels: [], authorLogin: "alice" } })); + expect(classes(plan)).toContain("assign"); + }); + + it("is unaffected by the blacklist/cap/review-nag short-circuits not reaching this far — plans nothing for a blacklisted contributor even with assign acting", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto", close: "auto" }, blacklistMatch: { matched: true, reason: "test" }, pr: { labels: [], authorLogin: "alice" } })); + expect(classes(plan)).not.toContain("assign"); + expect(classes(plan)).toContain("close"); + }); +}); + describe("isProtectedAutomationAuthor", () => { it("matches the maintainer-managed automation accounts (case-insensitive)", () => { expect(isProtectedAutomationAuthor("github-actions[bot]")).toBe(true); diff --git a/test/unit/github-assignees.test.ts b/test/unit/github-assignees.test.ts new file mode 100644 index 0000000000..27f2346f35 --- /dev/null +++ b/test/unit/github-assignees.test.ts @@ -0,0 +1,98 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { generateKeyPairSync } from "node:crypto"; +import { ensurePullRequestAssignee } from "../../src/github/assignees"; +import { createTestEnv } from "../helpers/d1"; + +describe("GitHub PR assignees (#3182)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("rejects invalid repository names before making GitHub calls", async () => { + await expect(ensurePullRequestAssignee(createTestEnv(), 123, "invalid", 4, "alice")).rejects.toThrow(/Invalid repository full name/); + await expect(ensurePullRequestAssignee(createTestEnv(), 123, "owner/repo/extra", 4, "alice")).rejects.toThrow(/Invalid repository full name/); + let called = false; + vi.stubGlobal("fetch", async () => { + called = true; + return Response.json({ token: "t" }); + }); + for (const padded of [" owner/repo ", "owner/ repo", "owner /repo"]) { + await expect(ensurePullRequestAssignee(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, padded, 4, "alice")).rejects.toThrow( + /Invalid repository full name/, + ); + } + expect(called).toBe(false); + }); + + it("does nothing when the login is already an assignee", async () => { + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + calls.push(`${init?.method ?? "GET"} ${url}`); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/issues/4") && !url.includes("/assignees")) return Response.json({ assignees: [{ login: "Alice" }] }); + return new Response("unexpected", { status: 500 }); + }); + + const result = await ensurePullRequestAssignee(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "alice"); + + expect(result).toEqual({ applied: true }); + expect(calls.some((call) => call.includes("/assignees") && call.startsWith("POST"))).toBe(false); + }); + + it("applies the assignee when the POST response confirms it stuck", 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") && !url.includes("/assignees")) return Response.json({ assignees: [] }); + if (url.includes("/issues/4/assignees") && method === "POST") { + const body = JSON.parse(String(init?.body ?? "{}")) as { assignees?: string[] }; + expect(body).toMatchObject({ assignees: ["alice"] }); + return Response.json({ assignees: [{ login: "alice" }] }); + } + return new Response("unexpected", { status: 500 }); + }); + + const result = await ensurePullRequestAssignee(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "alice"); + + expect(result).toEqual({ applied: true }); + }); + + it("reports NOT applied when GitHub silently drops an ineligible assignee (no push/triage access)", 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") && !url.includes("/assignees")) return Response.json({ assignees: [] }); + // GitHub returns 201 with the login silently omitted -- no error, just an assignees array without it. + if (url.includes("/issues/4/assignees") && method === "POST") return Response.json({ assignees: [] }); + return new Response("unexpected", { status: 500 }); + }); + + const result = await ensurePullRequestAssignee(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "external-contributor"); + + expect(result).toEqual({ applied: false }); + }); + + it("treats a response with no assignees field at all as an empty list, on both the GET and the POST", 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" }); + // Neither response includes an `assignees` key at all -- exercises the `?? []` fallback on both reads. + if (url.includes("/issues/4") && !url.includes("/assignees")) return Response.json({}); + if (url.includes("/issues/4/assignees") && method === "POST") return Response.json({}); + return new Response("unexpected", { status: 500 }); + }); + + const result = await ensurePullRequestAssignee(createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }), 123, "JSONbored/gittensory", 4, "alice"); + + expect(result).toEqual({ applied: false }); + }); +}); + +function generateRsaPrivateKeyPem(): string { + const { privateKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); + return privateKey.export({ type: "pkcs1", format: "pem" }).toString(); +}