From 9ec7491bd196e020eaa8e9e3187c454f5b2a9ba6 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Thu, 9 Jul 2026 03:51:20 -0700 Subject: [PATCH] fix(review): assign the PR's linked issues, not just the PR itself gittensor:priority propagation (resolveIssueLabelsForPropagation, linked-issue-label-propagation-fetch.ts) requires the PR author to be the linked issue's own author OR an assignee of it -- deliberately stricter than bug/feature, which trust a maintainer-authored issue without formal assignment. The existing auto-assign action (#3182) only ever assigned the PR's opening contributor to the PR itself, never to the issue(s) it closes. Since our issues are almost always opened for open pickup and rarely formally assigned, a contributor picking one up could never actually satisfy the assignee check -- so gittensor:priority could never propagate to a contributor's PR in practice, even when the linked issue clearly carried it. Extends the assign action to also best-effort assign the same login to each linked issue (capped at 10), reusing the same GitHub issues-assignee endpoint the PR-assign call already hits. This makes the existing, already-reviewed "author or assignee" ownership check actually satisfiable for the real-world open-pickup workflow it was designed for, without loosening it. --- src/queue/processors.ts | 1 + src/services/agent-action-executor.ts | 12 ++++++++ src/settings/agent-actions.ts | 27 ++++++++++++++++ src/types.ts | 3 ++ test/unit/agent-action-executor.test.ts | 41 +++++++++++++++++++++++++ test/unit/agent-actions.test.ts | 26 ++++++++++++++++ 6 files changed, 110 insertions(+) diff --git a/src/queue/processors.ts b/src/queue/processors.ts index f9b8a59fd3..c2a2342375 100644 --- a/src/queue/processors.ts +++ b/src/queue/processors.ts @@ -3087,6 +3087,7 @@ async function runAgentMaintenancePlanAndExecute( mergeBlockedSha: pr.mergeBlockedSha, approvedHeadSha: pr.approvedHeadSha, authorLogin: pr.authorLogin, + linkedIssues: pr.linkedIssues, }, }); // 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 71fae8e225..9a26b02511 100644 --- a/src/services/agent-action-executor.ts +++ b/src/services/agent-action-executor.ts @@ -932,6 +932,17 @@ async function performAction(env: Env, ctx: AgentActionExecutionContext, action: const login = action.assignee ?? ""; if (!login) return undefined; const result = await ensurePullRequestAssignee(env, ctx.installationId, ctx.repoFullName, ctx.pullNumber, login); + // Best-effort mirror the same assignment onto every linked issue (#priority-linked-issue-gate-ownership): + // gittensor:priority propagation requires the PR author to be the linked issue's own author OR assignee, + // so without this a contributor who picks up a maintainer-authored issue can never actually satisfy that + // check (see maybePlanAssign's doc comment, agent-actions.ts). Independent per-issue call: one issue's + // write failing (permissions, deleted issue, rate limit) must not affect another issue's assignment or the + // PR-assign outcome already computed above. + await Promise.all( + (action.assignLinkedIssues ?? []).map((issueNumber) => + ensurePullRequestAssignee(env, ctx.installationId, ctx.repoFullName, issueNumber, login).catch(() => undefined), + ), + ); 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 @@ -959,6 +970,7 @@ export function actionParams(action: PlannedAgentAction): AgentPendingActionPara ...(action.reviewBody !== undefined ? { reviewBody: action.reviewBody } : {}), ...(action.mergeMethod !== undefined ? { mergeMethod: action.mergeMethod } : {}), ...(action.assignee !== undefined ? { assignee: action.assignee } : {}), + ...(action.assignLinkedIssues !== undefined ? { assignLinkedIssues: action.assignLinkedIssues } : {}), ...(action.closeComment !== undefined ? { closeComment: action.closeComment } : {}), ...(action.closeReasons !== undefined ? { closeReasons: [...boundStructuredCloseReasonsForPersistence(action.closeReasons)] } : {}), ...(action.expectedHeadSha !== undefined ? { expectedHeadSha: action.expectedHeadSha } : {}), diff --git a/src/settings/agent-actions.ts b/src/settings/agent-actions.ts index d9562b4b13..bb3bff3e6d 100644 --- a/src/settings/agent-actions.ts +++ b/src/settings/agent-actions.ts @@ -150,6 +150,12 @@ export type PlannedAgentAction = { // 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; + // For an `assign` action (#priority-linked-issue-gate-ownership): the SAME login also best-effort-assigned to + // each of these linked issue numbers (already capped, see maybePlanAssign). Without this, a contributor is + // assigned to their own PR but never to the issue(s) it closes -- and gittensor:priority propagation + // (resolveIssueLabelsForPropagation, linked-issue-label-propagation-fetch.ts) requires the PR author to be the + // linked issue's own author OR assignee, which an issue-opened-for-open-pickup contributor otherwise never is. + assignLinkedIssues?: number[]; }; // Gate-blocker codes backed by CONCRETE, non-judgment evidence: a committed secret, a deterministic @@ -381,6 +387,9 @@ export type AgentActionPlanInput = { // 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; + // The PR's linked/closing issue numbers (#priority-linked-issue-gate-ownership), threaded through ONLY for + // the `assign` disposition below -- same "harmless when absent" reasoning as authorLogin just above. + linkedIssues?: number[] | undefined; }; }; @@ -549,21 +558,39 @@ function screenshotTableCloseMessage(reason: string): string { return `${reason} This is an automated maintenance action.`; } +// Best-effort assign-to-linked-issue fan-out cap (#priority-linked-issue-gate-ownership): a PR overwhelmingly +// closes 1 (rarely 2-3) issues. `pr.linkedIssues` is already capped much higher (50, MAX_LINKED_ISSUE_NUMBERS) +// for extraction/storage purposes only -- this narrower cap bounds the number of extra GitHub assignee-WRITE +// calls the executor makes for one `assign` action. +const ASSIGN_LINKED_ISSUES_MAX = 10; + /** * Plan best-effort assignment of the PR's opening contributor (#3182), independent of merge/close/CI outcome. * MUST run before the CI-pending settle-before-decide return below (#assign-before-ci-pending) — a PR that has * already been reviewed/evaluated (conclusion isn't "skipped") should get an assignee for triage even while an * unrelated check is still pending; assign has no bearing on mergeability so it never needs CI to settle first. * Gated purely on its own `assign` autonomy class, same as every other independent action here. + * + * Also best-effort assigns the SAME contributor to the PR's own linked issues (#priority-linked-issue-gate- + * ownership), capped at ASSIGN_LINKED_ISSUES_MAX. Without this, gittensor:priority propagation could never fire + * in practice for a contributor's PR: resolveIssueLabelsForPropagation only unlocks that label when the PR + * author is the linked issue's own author OR a GitHub assignee of it, but our issues are almost always opened + * for open pickup and rarely formally assigned (see the propagation config's own comment in + * gittensory-repo-focus-manifest.ts) -- so before this, only the PR itself ever got an assignee, never the + * issue it closes. */ function maybePlanAssign(actions: PlannedAgentAction[], input: AgentActionPlanInput): void { const level = resolveAutonomy(input.autonomy, "assign"); if (!isActingAutonomyLevel(level) || !input.pr.authorLogin) return; + const linkedIssues = input.pr.linkedIssues; actions.push({ actionClass: "assign", requiresApproval: autonomyRequiresApproval(level), reason: "auto-assign PR opener", assignee: input.pr.authorLogin, + ...(linkedIssues && linkedIssues.length > 0 + ? { assignLinkedIssues: linkedIssues.slice(0, ASSIGN_LINKED_ISSUES_MAX) } + : {}), }); } diff --git a/src/types.ts b/src/types.ts index f07e6cf820..d4c0ff1ca1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1253,6 +1253,9 @@ export type AgentPendingActionParams = { mergeMethod?: AutoMergeMethod; // For an `assign` action (#3182): the GitHub login to assign when a staged action is accepted. assignee?: string; + // For an `assign` action (#priority-linked-issue-gate-ownership): the linked issue numbers to ALSO assign + // `assignee` to when a staged action is accepted (see PlannedAgentAction.assignLinkedIssues). + assignLinkedIssues?: number[]; closeComment?: string; // Individual close reasons, persisted for approval-queue replay so the eventual audit row keeps the structured // reason list rather than only the flattened `reason` field. diff --git a/test/unit/agent-action-executor.test.ts b/test/unit/agent-action-executor.test.ts index 2eb28023fe..45fbd81ff0 100644 --- a/test/unit/agent-action-executor.test.ts +++ b/test/unit/agent-action-executor.test.ts @@ -144,6 +144,16 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(replayed).toMatchObject({ actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice" }); }); + it("REGRESSION (#priority-linked-issue-gate-ownership): actionParams round-trips assignLinkedIssues through approval replay", () => { + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: true, reason: "auto-assign PR opener", assignee: "alice", assignLinkedIssues: [42, 43] }; + + const persisted = actionParams(assign); + const replayed = pendingActionToPlanned({ actionClass: "assign", params: persisted, reason: assign.reason }); + + expect(persisted).toEqual({ assignee: "alice", assignLinkedIssues: [42, 43] }); + expect(replayed).toMatchObject({ actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice", assignLinkedIssues: [42, 43] }); + }); + it("actionParams round-trips structured closeReasons so approval replay preserves every close cause", () => { const closeWithReasons: PlannedAgentAction = { actionClass: "close", @@ -854,6 +864,37 @@ describe("executeAgentMaintenanceActions (#778 gate stack)", () => { expect(audit?.detail).toBe("assignee refused by GitHub — fell back to a by:external-contributor label"); }); + it("LIVE assign (#priority-linked-issue-gate-ownership): also assigns the PR's linked issues, not just the PR itself", async () => { + const env = createTestEnv({}); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice", assignLinkedIssues: [42, 43] }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "alice"); // the PR itself + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 42, "alice"); // linked issue #42 + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 43, "alice"); // linked issue #43 + expect(ensurePullRequestAssignee).toHaveBeenCalledTimes(3); + expect(outcomes[0]?.outcome).toBe("completed"); + }); + + it("LIVE assign: a linked issue's assignment failing does not affect the PR-assign outcome or the other linked issue", async () => { + const env = createTestEnv({}); + vi.mocked(ensurePullRequestAssignee).mockImplementation(async (_env, _installationId, _repoFullName, issueOrPrNumber) => { + if (issueOrPrNumber === 42) throw new Error("boom"); + return { applied: true }; + }); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice", assignLinkedIssues: [42, 43] }; + const outcomes = await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(outcomes[0]?.outcome).toBe("completed"); + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 7, "alice"); + expect(ensurePullRequestAssignee).toHaveBeenCalledWith(env, 123, "owner/repo", 43, "alice"); + }); + + it("LIVE assign: no linkedIssues means no extra assignee calls beyond the PR itself", async () => { + const env = createTestEnv({}); + const assign: PlannedAgentAction = { actionClass: "assign", requiresApproval: false, reason: "auto-assign PR opener", assignee: "alice" }; + await executeAgentMaintenanceActions(env, ctx({ autonomy: { assign: "auto" } }), [assign]); + expect(ensurePullRequestAssignee).toHaveBeenCalledTimes(1); + }); + 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" }; diff --git a/test/unit/agent-actions.test.ts b/test/unit/agent-actions.test.ts index 9d19aedac3..9eb4da7d89 100644 --- a/test/unit/agent-actions.test.ts +++ b/test/unit/agent-actions.test.ts @@ -1370,6 +1370,32 @@ describe("assign — auto-assign PR opener (#3182)", () => { expect(classes(plan)).not.toContain("assign"); expect(classes(plan)).toContain("close"); }); + + describe("#priority-linked-issue-gate-ownership: also assigns the PR's linked issues", () => { + it("threads linkedIssues into assignLinkedIssues when present", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [], authorLogin: "alice", linkedIssues: [42, 43] } })); + expect(plan).toContainEqual(expect.objectContaining({ actionClass: "assign", assignee: "alice", assignLinkedIssues: [42, 43] })); + }); + + it("omits assignLinkedIssues when linkedIssues is absent", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [], authorLogin: "alice" } })); + const assign = plan.find((a) => a.actionClass === "assign"); + expect(assign).not.toHaveProperty("assignLinkedIssues"); + }); + + it("omits assignLinkedIssues when linkedIssues is an empty array", () => { + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [], authorLogin: "alice", linkedIssues: [] } })); + const assign = plan.find((a) => a.actionClass === "assign"); + expect(assign).not.toHaveProperty("assignLinkedIssues"); + }); + + it("caps assignLinkedIssues at ASSIGN_LINKED_ISSUES_MAX (10) — a PR cannot fan out unbounded assignee writes", () => { + const linkedIssues = Array.from({ length: 15 }, (_, i) => i + 1); + const plan = planAgentMaintenanceActions(input({ conclusion: "success", autonomy: { assign: "auto" }, pr: { labels: [], authorLogin: "alice", linkedIssues } })); + const assign = plan.find((a) => a.actionClass === "assign"); + expect(assign?.assignLinkedIssues).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]); + }); + }); }); describe("isProtectedAutomationAuthor", () => {