diff --git a/src/github/issues.ts b/src/github/issues.ts index 8248addd9..f51725206 100644 --- a/src/github/issues.ts +++ b/src/github/issues.ts @@ -18,6 +18,10 @@ export type CreateInstallationIssueInput = { title: string; body: string; labels?: string[] | undefined; + /** GitHub milestone NUMBER (not title) to assign on create (#7427). Resolving a title to a number is the + * caller's job (see src/github/milestones.ts + the service-layer resolve-or-create heuristic) -- this + * primitive just passes through whatever number it's given. */ + milestone?: number | undefined; }; export type CreatedInstallationIssue = { number: number; url: string }; @@ -52,6 +56,7 @@ export async function createInstallationIssue( title: issue.title, body: issue.body, ...(issue.labels && issue.labels.length > 0 ? { labels: issue.labels } : {}), + ...(issue.milestone !== undefined ? { milestone: issue.milestone } : {}), }); const data = response.data as { number?: number; html_url?: string; dryRunSuppressed?: boolean }; if (data.dryRunSuppressed) return null; diff --git a/src/github/milestones.ts b/src/github/milestones.ts new file mode 100644 index 000000000..9632b9fde --- /dev/null +++ b/src/github/milestones.ts @@ -0,0 +1,61 @@ +import { withInstallationTokenRetry } from "./app"; +import { githubRateLimitAdmissionKeyForInstallation, makeInstallationOctokit } from "./client"; +import type { AgentActionMode } from "../settings/agent-execution"; + +// Mirrors parseRepoFullName in labels.ts / assignees.ts / issues.ts (#7427): each GitHub-write module keeps its +// own copy rather than importing a shared one, matching the existing house convention for this tiny pure check. +function parseRepoFullName(repoFullName: string): { owner: string; repo: string } { + const parts = repoFullName.split("/"); + const owner = parts[0]; + const repo = parts[1]; + if (parts.length !== 2 || !owner || !repo || /\s/.test(repoFullName)) { + throw new Error(`Invalid repository full name: ${repoFullName}`); + } + return { owner, repo }; +} + +export type InstallationMilestone = { number: number; title: string; description: string | null; dueOn: string | null }; + +export type CreateInstallationMilestoneInput = { title: string; description?: string | undefined; dueOn?: string | undefined }; + +/** + * List OPEN milestones via the installation-token/Orb-broker path (#7427) -- the same auth path createInstallationIssue + * (issues.ts, #7425) uses. Capped to GitHub's max per_page (100) with no further pagination: unlike e.g. + * cancelInFlightWorkflowRunsForHeadSha's bounded multi-page workflow-run loop, there is no realistic scenario + * where a repo has more than 100 OPEN milestones, so a second page is not worth the added complexity here. + */ +export async function listOpenInstallationMilestones(env: Env, installationId: number, repoFullName: string, mode: AgentActionMode = "live"): Promise { + const { owner, repo } = parseRepoFullName(repoFullName); + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("GET /repos/{owner}/{repo}/milestones", { owner, repo, state: "open", per_page: 100 }); + const data = response.data as Array<{ number?: number; title?: string; description?: string | null; due_on?: string | null }>; + return data + .filter((entry): entry is { number: number; title: string; description?: string | null; due_on?: string | null } => typeof entry.number === "number" && typeof entry.title === "string") + .map((entry) => ({ number: entry.number, title: entry.title, description: entry.description ?? null, dueOn: entry.due_on ?? null })); + }); +} + +/** + * Create a milestone via the same installation-token/Orb-broker path. Returns null (never throws for a + * suppressed/malformed write) when the write is suppressed by a non-live mode or GitHub's response omits the + * fields a caller needs -- mirrors createInstallationIssue's identical contract (issues.ts). A genuine GitHub + * API failure (permission gap, 5xx, rate limit) is NOT swallowed here; it propagates via Octokit's + * throw-on-non-2xx, matching that same contract. + */ +export async function createInstallationMilestone(env: Env, installationId: number, repoFullName: string, input: CreateInstallationMilestoneInput, mode: AgentActionMode = "live"): Promise { + const { owner, repo } = parseRepoFullName(repoFullName); + return withInstallationTokenRetry(env, installationId, async (token) => { + const octokit = makeInstallationOctokit(env, token, mode, githubRateLimitAdmissionKeyForInstallation(installationId)); + const response = await octokit.request("POST /repos/{owner}/{repo}/milestones", { + owner, + repo, + title: input.title, + ...(input.description ? { description: input.description } : {}), + ...(input.dueOn ? { due_on: input.dueOn } : {}), + }); + const data = response.data as { number?: number; title?: string; description?: string | null; due_on?: string | null; dryRunSuppressed?: boolean }; + if (data.dryRunSuppressed) return null; + return data.number && data.title ? { number: data.number, title: data.title, description: data.description ?? null, dueOn: data.due_on ?? null } : null; + }); +} diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 64fa1098f..85d59a46e 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -666,6 +666,15 @@ const generateContributorIssueDraftsOutputSchema = { // #7426: dryRun/create/limit mirror generateContributorIssueDraftsShape's own bounds/defaults (create alone is // rejected -- the handler re-applies the explicit_create_requires_dry_run_false guard). `limit` is capped lower // (10, not 20): every draft here costs real LLM spend, unlike that tool's zero-cost static signals. +// #7427: title/description/dueOn are the CALLER's own input, never model-generated -- milestone metadata is +// maintainer-authored/approved by design. Only ever consulted when actually creating; a dry-run preview makes +// no milestone-related GitHub calls at all. +const planRepoIssuesMilestoneShape = z.object({ + title: z.string().min(1).max(200), + description: z.string().max(2000).optional(), + dueOn: z.string().datetime({ offset: true }).optional(), +}); + const planRepoIssuesShape = { owner: z.string().min(1), repo: z.string().min(1), @@ -673,6 +682,7 @@ const planRepoIssuesShape = { dryRun: z.boolean().optional().default(true), create: z.boolean().optional().default(false), limit: z.number().int().min(1).max(10).optional().default(5), + milestone: planRepoIssuesMilestoneShape.optional(), }; const planRepoIssuesOutputSchema = { @@ -703,6 +713,9 @@ const planRepoIssuesOutputSchema = { }), ) .optional(), + // Set only when a milestone target was given AND creation actually ran AND resolution succeeded (#7427) -- + // absent on a dry run, no milestone requested, or a degraded (failed) resolution. + milestoneNumber: z.number().optional(), }; // #784 (MCP slice) — the agent audit feed: executed actions + approval decisions for a repo. @@ -2624,7 +2637,7 @@ export class LoopoverMcp { "loopover_plan_repo_issues", { description: - "AI-plan a small set of concrete GitHub issues from a maintainer-supplied free-form goal, for ANY repo the caller's App/Orb is installed on -- repo-agnostic and gittensor-optional (#7426). Dry-run BY DEFAULT: only PREVIEWS drafts (full title/body/labels) unless the caller passes BOTH create:true and dryRun:false, so it can never silently open issues. Creates exclusively via the installation-token/Orb-broker path (#7425), never a flat PAT. Makes a real LLM call subject to the shared daily AI budget and the fleet AI_SUMMARIES_ENABLED/AI_PUBLIC_COMMENTS_ENABLED switches. Maintainer access required.", + "AI-plan a small set of concrete GitHub issues from a maintainer-supplied free-form goal, for ANY repo the caller's App/Orb is installed on -- repo-agnostic and gittensor-optional (#7426). Dry-run BY DEFAULT: only PREVIEWS drafts (full title/body/labels) unless the caller passes BOTH create:true and dryRun:false, so it can never silently open issues. Creates exclusively via the installation-token/Orb-broker path (#7425), never a flat PAT. An optional `milestone` (title/description/dueOn, all maintainer-supplied -- never model-generated) is resolved against existing OPEN milestones by exact normalized title before creating a new one, and assigned to every created issue (#7427). Makes a real LLM call subject to the shared daily AI budget and the fleet AI_SUMMARIES_ENABLED/AI_PUBLIC_COMMENTS_ENABLED switches. Maintainer access required.", inputSchema: planRepoIssuesShape, outputSchema: planRepoIssuesOutputSchema, }, @@ -4321,6 +4334,7 @@ export class LoopoverMcp { create: input.create, limit: input.limit, requestedBy: this.identity.kind === "session" ? this.identity.actor : "mcp", + milestone: input.milestone, }); return { summary: `Issue plan for ${fullName} (status=${result.status}, dryRun=${result.dryRun}): ${result.proposed} proposed, ${result.created} created, ${result.skippedDuplicate} duplicate, ${result.skippedDeclined} declined, ${result.skippedUnsafe} unsafe.`, @@ -4335,6 +4349,7 @@ export class LoopoverMcp { skippedDeclined: result.skippedDeclined, skippedUnsafe: result.skippedUnsafe, created: result.created, + ...(result.milestoneNumber !== undefined ? { milestoneNumber: result.milestoneNumber } : {}), skippedCreateFailed: result.skippedCreateFailed, drafts: result.drafts.map((draft) => ({ title: draft.title, diff --git a/src/services/issue-plan-draft.ts b/src/services/issue-plan-draft.ts index 666674eb7..7c727df1c 100644 --- a/src/services/issue-plan-draft.ts +++ b/src/services/issue-plan-draft.ts @@ -22,6 +22,7 @@ import { utcDayStartIso, } from "./ai-review"; import { createInstallationIssue } from "../github/issues"; +import { createInstallationMilestone, listOpenInstallationMilestones } from "../github/milestones"; import { isMaintainerAssociation } from "../github/commands"; import { getRepository, @@ -242,10 +243,11 @@ async function createIssuePlanDraftIssue( repoFullName: string, draft: Pick, installationId: number | null | undefined, + milestoneNumber: number | undefined, ): Promise<{ number: number; url: string } | null> { if (!installationId) return null; try { - return await createInstallationIssue(env, installationId, repoFullName, { title: draft.title, body: draft.body, labels: draft.labels }); + return await createInstallationIssue(env, installationId, repoFullName, { title: draft.title, body: draft.body, labels: draft.labels, milestone: milestoneNumber }); } catch (error) { console.warn( JSON.stringify({ level: "warn", event: "issue_plan_draft_create_failed", repoFullName, message: errorMessage(error).slice(0, 200) }), @@ -254,11 +256,52 @@ async function createIssuePlanDraftIssue( } } +export type IssuePlanMilestoneTarget = { title: string; description?: string | undefined; dueOn?: string | undefined }; + +/** + * Resolve-or-create a milestone for the batch about to be created (#7427). Reuse is an EXACT normalized-title + * match against existing OPEN milestones only (normalizeIssueTitleKey, already generic despite living on the + * issue-draft side) -- deliberately not fuzzy: a fuzzy match risks silently grouping issues under the wrong + * milestone, which is worse than creating a new, differently-named one. Mirrors the manual convention + * `.claude/skills/contributor-pipeline-gardening` already follows (reusing one milestone across recurring + * rounds) instead of spawning a fresh one every planning run under the same title. + * + * Degrades to null on ANY failure (including a malformed/suppressed create) rather than aborting the batch -- + * the maintainer still gets their issues, just ungrouped, which is preferable to filing nothing at all over a + * milestone-specific glitch. Titles/descriptions/due dates are the CALLER's own input, never model-generated + * (see the epic's #7427 boundary: milestone metadata is maintainer-authored/approved, not invented by the AI). + */ +async function resolveOrCreateIssuePlanMilestone( + env: Env, + installationId: number, + repoFullName: string, + target: IssuePlanMilestoneTarget, +): Promise { + try { + const titleKey = normalizeIssueTitleKey(target.title); + if (titleKey) { + const existing = await listOpenInstallationMilestones(env, installationId, repoFullName); + const match = existing.find((milestone) => normalizeIssueTitleKey(milestone.title) === titleKey); + if (match) return match.number; + } + const created = await createInstallationMilestone(env, installationId, repoFullName, { title: target.title, description: target.description, dueOn: target.dueOn }); + return created?.number; + } catch (error) { + console.warn( + JSON.stringify({ level: "warn", event: "issue_plan_milestone_resolve_failed", repoFullName, message: errorMessage(error).slice(0, 200) }), + ); + return undefined; + } +} + export type IssuePlanDraftOptions = { dryRun?: boolean | undefined; create?: boolean | undefined; limit?: number | undefined; requestedBy?: string | undefined; + /** Optional milestone to resolve-or-create and assign created issues to (#7427). Only ever consulted when + * actually creating ({create:true, dryRun:false}) -- a dry-run preview makes no GitHub calls for it at all. */ + milestone?: IssuePlanMilestoneTarget | undefined; }; export type IssuePlanGenerationStatus = "ok" | "disabled" | "unavailable" | "quota_exceeded" | "no_output"; @@ -276,6 +319,10 @@ export type IssuePlanGenerationResult = { created: number; skippedCreateFailed: number; drafts: IssuePlanDraft[]; + /** Set only when a milestone was actually resolved-or-created this call (i.e. a milestone target was given + * AND creation actually ran AND resolution succeeded). Absent on a dry run, a disabled/unavailable/quota + * short-circuit, no milestone requested, or a degraded (failed) resolution. */ + milestoneNumber?: number | undefined; }; function emptyIssuePlanResult( @@ -342,6 +389,11 @@ export async function generateIssuePlanDrafts(env: Env, repoFullName: string, go }); if (rawCandidates.length === 0) return empty("no_output"); + const milestoneNumber = + !dryRun && createRequested && options.milestone && repo?.installationId + ? await resolveOrCreateIssuePlanMilestone(env, repo.installationId, repoFullName, options.milestone) + : undefined; + const drafts: IssuePlanDraft[] = []; let proposed = 0; let skippedDuplicate = 0; @@ -389,7 +441,7 @@ export async function generateIssuePlanDrafts(env: Env, repoFullName: string, go } if (!dryRun && createRequested) { - const issue = await createIssuePlanDraftIssue(env, repoFullName, draft, repo?.installationId); + const issue = await createIssuePlanDraftIssue(env, repoFullName, draft, repo?.installationId, milestoneNumber); if (issue) { draft.status = "created"; draft.issue = issue; @@ -413,5 +465,5 @@ export async function generateIssuePlanDrafts(env: Env, repoFullName: string, go }); } - return { repoFullName, generatedAt, status: "ok", dryRun, createRequested, proposed, skippedDuplicate, skippedDeclined, skippedUnsafe, created, skippedCreateFailed, drafts }; + return { repoFullName, generatedAt, status: "ok", dryRun, createRequested, proposed, skippedDuplicate, skippedDeclined, skippedUnsafe, created, skippedCreateFailed, drafts, milestoneNumber }; } diff --git a/test/unit/github-milestones.test.ts b/test/unit/github-milestones.test.ts new file mode 100644 index 000000000..64b23675d --- /dev/null +++ b/test/unit/github-milestones.test.ts @@ -0,0 +1,125 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { generateKeyPairSync } from "node:crypto"; +import { clearInstallationTokenCacheForTest } from "../../src/github/app"; +import { createInstallationMilestone, listOpenInstallationMilestones } from "../../src/github/milestones"; +import { createTestEnv } from "../helpers/d1"; + +function generateRsaPrivateKeyPem(): string { + const { privateKey } = generateKeyPairSync("rsa", { modulusLength: 2048 }); + return privateKey.export({ type: "pkcs1", format: "pem" }).toString(); +} + +describe("github/milestones (#7427)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + clearInstallationTokenCacheForTest(); + }); + + it("rejects invalid repository names before making any GitHub call", async () => { + let called = false; + vi.stubGlobal("fetch", async () => { + called = true; + return Response.json({ token: "t" }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + for (const malformed of ["invalid", "owner/repo/extra", " owner/repo ", "owner/ repo", "owner /repo"]) { + await expect(listOpenInstallationMilestones(env, 123, malformed)).rejects.toThrow(/Invalid repository full name/); + await expect(createInstallationMilestone(env, 123, malformed, { title: "t" })).rejects.toThrow(/Invalid repository full name/); + } + expect(called).toBe(false); + }); + + it("lists open milestones via the installation-token path, filtering out malformed entries", 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("/milestones") && method === "GET") { + return Response.json([ + { number: 1, title: "Wave 5", description: "the wave", due_on: "2026-08-01T00:00:00Z" }, + { title: "missing number" }, + { number: 2 }, + { number: 3, title: "Wave 6", description: null, due_on: null }, + ]); + } + return new Response("unexpected", { status: 599 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + const result = await listOpenInstallationMilestones(env, 123, "JSONbored/loopover"); + expect(result).toEqual([ + { number: 1, title: "Wave 5", description: "the wave", dueOn: "2026-08-01T00:00:00Z" }, + { number: 3, title: "Wave 6", description: null, dueOn: null }, + ]); + expect(calls.some((call) => call.startsWith("GET") && call.includes("state=open"))).toBe(true); + }); + + it("creates a milestone via the installation-token path, including description and dueOn when provided", async () => { + const calls: { method: string; url: string; body: unknown }[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + const method = init?.method ?? "GET"; + calls.push({ method, url, body: init?.body ? JSON.parse(init.body as string) : undefined }); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + if (url.includes("/milestones") && method === "POST") { + return Response.json({ number: 42, title: "Wave 5", description: "the wave", due_on: "2026-08-01T00:00:00Z" }); + } + return new Response("unexpected", { status: 599 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + const result = await createInstallationMilestone(env, 123, "JSONbored/loopover", { title: "Wave 5", description: "the wave", dueOn: "2026-08-01T00:00:00Z" }); + expect(result).toEqual({ number: 42, title: "Wave 5", description: "the wave", dueOn: "2026-08-01T00:00:00Z" }); + const createCall = calls.find((call) => call.method === "POST" && call.url.includes("/milestones")); + expect(createCall?.body).toMatchObject({ title: "Wave 5", description: "the wave", due_on: "2026-08-01T00:00:00Z" }); + }); + + it("omits description and dueOn from the request when not provided", async () => { + const calls: { body: unknown }[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL, init?: RequestInit) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + calls.push({ body: init?.body ? JSON.parse(init.body as string) : undefined }); + return Response.json({ number: 1, title: "Wave 5" }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await createInstallationMilestone(env, 123, "JSONbored/loopover", { title: "Wave 5" }); + expect(calls[0]?.body).not.toHaveProperty("description"); + expect(calls[0]?.body).not.toHaveProperty("due_on"); + }); + + it("returns null when GitHub's response is missing the number or title", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return Response.json({ description: "no title or number" }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(createInstallationMilestone(env, 123, "JSONbored/loopover", { title: "Wave 5" })).resolves.toBeNull(); + }); + + it("propagates a non-2xx GitHub response instead of swallowing it", async () => { + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + const url = input.toString(); + if (url.includes("/access_tokens")) return Response.json({ token: "installation-token" }); + return Response.json({ message: "Resource not accessible by integration" }, { status: 403 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await expect(createInstallationMilestone(env, 123, "JSONbored/loopover", { title: "Wave 5" })).rejects.toMatchObject({ status: 403 }); + }); + + it("suppresses the write and returns null in a non-live mode", 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" }); + return new Response("unexpected", { status: 599 }); + }); + const env = createTestEnv({ GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + const result = await createInstallationMilestone(env, 123, "JSONbored/loopover", { title: "Wave 5" }, "dry_run"); + expect(result).toBeNull(); + expect(calls.some((call) => call.startsWith("POST") && call.includes("/milestones"))).toBe(false); + }); +}); diff --git a/test/unit/issue-plan-draft.test.ts b/test/unit/issue-plan-draft.test.ts index ceb64777a..727d7a848 100644 --- a/test/unit/issue-plan-draft.test.ts +++ b/test/unit/issue-plan-draft.test.ts @@ -308,6 +308,136 @@ describe("generateIssuePlanDrafts (#7426)", () => { expect(result.skippedCreateFailed).toBe(1); }); + it("does not resolve or create a milestone on a dry run, even when one is requested", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + calls.push(input.toString()); + return new Response("unexpected", { status: 599 }); + }); + const env = baseEnv({ AI: { run } as unknown as Ai }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { milestone: { title: "Wave 5" } }); + expect(result.milestoneNumber).toBeUndefined(); + expect(calls).toHaveLength(0); + }); + + it("creates a new milestone and assigns it to every created issue when no matching open milestone exists (#7427)", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry to the sync job", body: "Retries transient failures." }])); + const bodies: Array<{ url: string; body: Record }> = []; + 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("/milestones") && method === "GET") return Response.json([{ number: 9, title: "Unrelated milestone" }]); + if (url.includes("/milestones") && method === "POST") { + bodies.push({ url, body: JSON.parse(init?.body as string) }); + return Response.json({ number: 55, title: "Wave 5" }); + } + if (url.includes("/issues") && method === "POST") { + bodies.push({ url, body: JSON.parse(init?.body as string) }); + return Response.json({ number: 900, html_url: "https://github.com/acme/widgets/issues/900" }); + } + return new Response("unexpected", { status: 599 }); + }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const env = baseEnv({ AI: { run } as unknown as Ai, GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { create: true, dryRun: false, milestone: { title: "Wave 5", description: "the wave", dueOn: "2026-08-01T00:00:00Z" } }); + expect(result.milestoneNumber).toBe(55); + expect(result.created).toBe(1); + const milestoneCreate = bodies.find((entry) => entry.url.includes("/milestones")); + expect(milestoneCreate?.body).toMatchObject({ title: "Wave 5", description: "the wave", due_on: "2026-08-01T00:00:00Z" }); + const issueCreate = bodies.find((entry) => entry.url.includes("/issues")); + expect(issueCreate?.body).toMatchObject({ milestone: 55 }); + }); + + it("reuses an existing open milestone by exact normalized title instead of creating a new one (#7427)", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); + let milestonePostCalled = false; + 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("/milestones") && method === "GET") return Response.json([{ number: 7, title: " Wave 5!!" }]); + if (url.includes("/milestones") && method === "POST") { + milestonePostCalled = true; + return Response.json({ number: 999, title: "Wave 5" }); + } + if (url.includes("/issues") && method === "POST") return Response.json({ number: 901, html_url: "https://github.com/acme/widgets/issues/901" }); + return new Response("unexpected", { status: 599 }); + }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const env = baseEnv({ AI: { run } as unknown as Ai, GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { create: true, dryRun: false, milestone: { title: "Wave 5" } }); + expect(result.milestoneNumber).toBe(7); + expect(milestonePostCalled).toBe(false); + }); + + it("creates directly (skipping the existing-milestone lookup) when the milestone title normalizes to an empty key", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); + let listCalled = false; + 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("/milestones") && method === "GET") { + listCalled = true; + return Response.json([]); + } + if (url.includes("/milestones") && method === "POST") return Response.json({ number: 77, title: "###" }); + if (url.includes("/issues") && method === "POST") return Response.json({ number: 903, html_url: "https://github.com/acme/widgets/issues/903" }); + return new Response("unexpected", { status: 599 }); + }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const env = baseEnv({ AI: { run } as unknown as Ai, GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { create: true, dryRun: false, milestone: { title: "###" } }); + expect(result.milestoneNumber).toBe(77); + expect(listCalled).toBe(false); + }); + + it("degrades gracefully (still creates the issues, without a milestone) when milestone resolution fails", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); + 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("/milestones")) return new Response("nope", { status: 500 }); + if (url.includes("/issues") && method === "POST") return Response.json({ number: 902, html_url: "https://github.com/acme/widgets/issues/902" }); + return new Response("unexpected", { status: 599 }); + }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const env = baseEnv({ AI: { run } as unknown as Ai, GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { create: true, dryRun: false, milestone: { title: "Wave 5" } }); + expect(result.milestoneNumber).toBeUndefined(); + expect(result.created).toBe(1); + expect(result.drafts[0]?.status).toBe("created"); + }); + + it("never attempts milestone resolution when the repo has no installation", async () => { + const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); + const calls: string[] = []; + vi.stubGlobal("fetch", async (input: RequestInfo | URL) => { + calls.push(input.toString()); + return new Response("unexpected", { status: 599 }); + }); + vi.spyOn(repositories, "getRepository").mockResolvedValue(null); + vi.spyOn(repositories, "listOpenIssues").mockResolvedValue([]); + const env = baseEnv({ AI: { run } as unknown as Ai }); + const result = await generateIssuePlanDrafts(env, "acme/widgets", "goal", { create: true, dryRun: false, milestone: { title: "Wave 5" } }); + expect(result.milestoneNumber).toBeUndefined(); + expect(calls).toHaveLength(0); + expect(result.skippedCreateFailed).toBe(1); + }); + it("REGRESSION: the global agent brake / freeze overrides {dryRun:false}, so nothing is created", async () => { const run = vi.fn(async () => issuesResponse([{ title: "Add retry", body: "Body" }])); vi.spyOn(repositories, "getRepository").mockResolvedValue(installedRepo("acme/widgets")); diff --git a/test/unit/mcp-plan-repo-issues.test.ts b/test/unit/mcp-plan-repo-issues.test.ts index 156f2582a..9b55a09ad 100644 --- a/test/unit/mcp-plan-repo-issues.test.ts +++ b/test/unit/mcp-plan-repo-issues.test.ts @@ -76,6 +76,27 @@ describe("MCP loopover_plan_repo_issues (#7426)", () => { expect(drafts[0]).toMatchObject({ status: "created", issueNumber: 42, issueUrl: "https://github.com/owner/widgets/issues/42" }); }); + it("resolves the requested milestone and surfaces its number when create succeeds (#7427)", async () => { + const run = async () => issuesResponse([{ title: "Add retry to the sync job", body: "Retries transient failures." }]); + 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("/milestones") && method === "GET") return Response.json([]); + if (url.includes("/milestones") && method === "POST") return Response.json({ number: 55, title: "Wave 5" }); + return Response.json({ number: 42, html_url: "https://github.com/owner/widgets/issues/42" }); + }); + const env = createTestEnv({ ...AI_ENABLED, AI: { run } as unknown as Ai, GITHUB_APP_PRIVATE_KEY: generateRsaPrivateKeyPem() }); + await seedRepo(env); + const client = await connect(env, API_IDENTITY); + const result = await client.callTool({ + name: "loopover_plan_repo_issues", + arguments: { owner: "owner", repo: "widgets", goal: "improve sync reliability", create: true, dryRun: false, milestone: { title: "Wave 5" } }, + }); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toMatchObject({ milestoneNumber: 55 }); + }); + it("REJECTS create without an explicit dryRun:false — the tool can never silently create", async () => { const env = createTestEnv({ ...AI_ENABLED }); await seedRepo(env);