diff --git a/src/lib/github.ts b/src/lib/github.ts index e30aa8b..7d71b98 100644 --- a/src/lib/github.ts +++ b/src/lib/github.ts @@ -398,6 +398,34 @@ export async function addIssueLabel( } } + +export interface UpdateIssueFields { + title?: string; + body?: string | null; +} + +/** + * Update issue title and/or body via GitHub API. + */ +export async function updateIssueTitleAndBody( + repoFullName: string, + issueNumber: number, + fields: UpdateIssueFields, +): Promise { + const [owner, repo] = repoFullName.split("/"); + const url = `${GITHUB_API}/repos/${owner}/${repo}/issues/${issueNumber}`; + + const response = await fetch(url, { + method: "PATCH", + headers: await getHeadersAsync(), + body: JSON.stringify(fields), + }); + + if (!response.ok) { + const text = await response.text(); + throw new Error(`GitHub API error updating issue #${issueNumber}: ${response.status} ${text}`); + } +} export async function removeIssueLabel( repoFullName: string, issueNumber: number, diff --git a/src/lib/groomer/llm.ts b/src/lib/groomer/llm.ts index 00e75ed..6e96580 100644 --- a/src/lib/groomer/llm.ts +++ b/src/lib/groomer/llm.ts @@ -30,7 +30,9 @@ Return ONLY valid JSON with this exact schema: "githubComment": "optional comment to post on the issue (omit if nothing to say)", "needsInfoReason": "optional reason if info is needed", "blockedReason": "optional reason if blocked", - "nextGroomingAction": "optional: promote_to_ready|escalate|mark_not_ready|mark_needs_info|mark_blocked" + "nextGroomingAction": "optional: promote_to_ready|escalate|mark_not_ready|mark_needs_info|mark_blocked", + "proposedTitle": "optional: rewritten title if current one is bad", + "proposedBody": "optional: enriched body if current one is sparse" } Rules: @@ -40,7 +42,20 @@ Rules: - Valid type labels: ${typeLabels} - Never remove agent/* labels - Lane must be one of the configured lane ids -- Be concise in summary and reason fields`; +- Be concise in summary and reason fields + +Title rewriting rules: +- Only propose a new title when the current title is bad: length < 10 chars, matches generic patterns (single word like "P0", "TODO", "bug", "fix"), or is clearly just a priority/label token +- If the title is already descriptive (>= 10 chars and looks like a real sentence/phrase), omit proposedTitle +- The new title should be 10-200 chars, imperative verb form, specific and actionable +- Base the rewritten title on body content, labels, and comments + +Body enrichment rules: +- Only propose an enriched body when the current body is missing, empty, or < 100 characters (excluding markdown/HTML comments) +- If the body already has substantial content, omit proposedBody +- The enriched body should add structure: brief context, what's known, suggested approach based on labels/body/comments +- Do NOT clobber existing body content — if there's any meaningful body, append rather than replace; if empty/missing, create from scratch +- Keep enriched body under 10000 characters`; } export async function callGroomerLLM(options: CallLlmOptions): Promise { diff --git a/src/lib/groomer/run.test.ts b/src/lib/groomer/run.test.ts index 04c2ca9..33482fc 100644 --- a/src/lib/groomer/run.test.ts +++ b/src/lib/groomer/run.test.ts @@ -23,6 +23,7 @@ const { mocks } = vi.hoisted(() => ({ getHostedGroomerConfig: vi.fn(), updateIssueLabels: vi.fn(), addIssueComment: vi.fn(), + updateIssueTitleAndBody: vi.fn(), findActiveLeasesForIssue: vi.fn(), upsertLease: vi.fn(), releaseLease: vi.fn(), @@ -64,6 +65,7 @@ vi.mock("./config", () => ({ vi.mock("@/lib/github", () => ({ updateIssueLabels: mocks.updateIssueLabels, addIssueComment: mocks.addIssueComment, + updateIssueTitleAndBody: mocks.updateIssueTitleAndBody, addIssueLabel: mocks.addIssueLabel, removeIssueLabel: mocks.removeIssueLabel, })); @@ -131,6 +133,7 @@ describe("runHostedGroomer", () => { mocks.getHostedGroomerConfig.mockReturnValue(mockConfig); mocks.callGroomerLLM.mockResolvedValue(mockOutput); mocks.updateIssueLabels.mockResolvedValue(undefined); + mocks.updateIssueTitleAndBody.mockResolvedValue(undefined); mocks.addIssueComment.mockResolvedValue({ url: null }); mocks.findActiveLeasesForIssue.mockResolvedValue([]); mocks.upsertLease.mockResolvedValue({ created: true, lease: { id: "lease-1" } }); @@ -469,4 +472,187 @@ describe("runHostedGroomer", () => { }), ); }); + + // ─── Title rewriting tests ─── + + it("does not rewrite a good title", async () => { + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Fix the login bug" }, + }); + + const result = await runHostedGroomer(); + + // "Fix login bug" (13 chars) is a good title — should not be rewritten + expect(result!.mutationPlan?.titleRewritten).toBe(false); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("rewrites a bad short title", async () => { + const badCandidate = { ...mockCandidate, title: "P0" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { + ...mockOutput, + proposedTitle: "Fix SSO/OIDC callback state verification mismatch causing 400 errors", + }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(result!.mutationPlan?.originalTitle).toBe("P0"); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ title: "Fix SSO/OIDC callback state verification mismatch causing 400 errors" }), + ); + expect(result!.appliedMutations?.titleUpdated).toBe(true); + }); + + it("rewrites a single-word generic title like TODO", async () => { + const badCandidate = { ...mockCandidate, title: "TODO" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Implement user authentication flow" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + it("rewrites an empty title", async () => { + const badCandidate = { ...mockCandidate, title: "" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Add missing error handling for database connections" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + // ─── Body enrichment tests ─── + + it("does not enrich a substantial body", async () => { + const goodCandidate = { + ...mockCandidate, + body: "This is a detailed issue description that explains the problem clearly with enough context and detail for developers to understand what needs to be done.", + }; + mocks.selectGroomingCandidate.mockResolvedValue(goodCandidate); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: "Enriched body content." }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(false); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("enriches a sparse body", async () => { + const sparseCandidate = { ...mockCandidate, body: "Broken." }; + mocks.selectGroomingCandidate.mockResolvedValue(sparseCandidate); + const enrichedBody = `## Context +This issue relates to the login flow. + +## What's known +- Login fails after password reset + +## Suggested approach +Investigate session handling in auth module.`; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: enrichedBody }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ body: enrichedBody }), + ); + expect(result!.appliedMutations?.bodyUpdated).toBe(true); + }); + + it("enriches a null body", async () => { + const noBodyCandidate = { ...mockCandidate, body: null }; + mocks.selectGroomingCandidate.mockResolvedValue(noBodyCandidate); + const enrichedBody = "## Description\nMore detail needed.\n\n## Labels\npriority/p0"; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedBody: enrichedBody }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalled(); + }); + + it("applies both title rewrite and body enrichment together", async () => { + const badCandidate = { ...mockCandidate, title: "P0", body: "Fix." }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + const enrichedBody = "## Context\nSSO login is broken.\n\n## What's known\nState verification fails on callback."; + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { + ...mockOutput, + proposedTitle: "Fix SSO callback state mismatch", + proposedBody: enrichedBody, + }, + }); + + const result = await runHostedGroomer(); + + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(result!.mutationPlan?.bodyEnriched).toBe(true); + expect(mocks.updateIssueTitleAndBody).toHaveBeenCalledWith( + "org/repo", + 42, + expect.objectContaining({ + title: "Fix SSO callback state mismatch", + body: enrichedBody, + }), + ); + expect(result!.appliedMutations?.titleUpdated).toBe(true); + expect(result!.appliedMutations?.bodyUpdated).toBe(true); + }); + + it("dry-run includes title/body plan but does not call GitHub API", async () => { + const badCandidate = { ...mockCandidate, title: "P0" }; + mocks.selectGroomingCandidate.mockResolvedValue(badCandidate); + mocks.getHostedGroomerConfig.mockReturnValue({ ...mockConfig, dryRun: true }); + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: { ...mockOutput, proposedTitle: "Fix the thing" }, + }); + + const result = await runHostedGroomer(); + + expect(result!.dryRun).toBe(true); + expect(result!.mutationPlan?.titleRewritten).toBe(true); + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); + + it("skips title/body update when LLM does not propose changes", async () => { + mocks.validateGroomerOutput.mockReturnValue({ + valid: true, + parsed: mockOutput, // no proposedTitle or proposedBody + }); + + await runHostedGroomer(); + + expect(mocks.updateIssueTitleAndBody).not.toHaveBeenCalled(); + }); }); diff --git a/src/lib/groomer/run.ts b/src/lib/groomer/run.ts index 7c0d40b..afd13a3 100644 --- a/src/lib/groomer/run.ts +++ b/src/lib/groomer/run.ts @@ -1,10 +1,10 @@ import { prisma } from "@/lib/prisma"; -import { addIssueComment, updateIssueLabels } from "@/lib/github"; +import { addIssueComment, updateIssueLabels, updateIssueTitleAndBody } from "@/lib/github"; import { findActiveLeasesForIssue, releaseLease, upsertLease } from "@/lib/lease"; import { selectGroomingCandidate } from "./selector"; import { buildIssueContext, fetchIssueComments } from "./context"; import { callGroomerLLM } from "./llm"; -import { validateGroomerOutput } from "./schema"; +import { validateGroomerOutput, type GroomerOutput } from "./schema"; import { getHostedGroomerConfig } from "./config"; import { buildRepositoryContext } from "./repository-context"; import type { RepositoryContextInput, RepositoryContextConfig } from "./repository-context"; @@ -41,6 +41,7 @@ export interface GroomerDeps { getConfig: typeof getHostedGroomerConfig; updateLabels: typeof updateIssueLabels; addComment: typeof addIssueComment; + updateTitleAndBody: typeof updateIssueTitleAndBody; findActiveLeases: typeof findActiveLeasesForIssue; upsertLease: typeof upsertLease; releaseLease: typeof releaseLease; @@ -57,6 +58,7 @@ const defaultDeps: GroomerDeps = { getConfig: getHostedGroomerConfig, updateLabels: updateIssueLabels, addComment: addIssueComment, + updateTitleAndBody: updateIssueTitleAndBody, findActiveLeases: findActiveLeasesForIssue, upsertLease, releaseLease, @@ -185,6 +187,9 @@ export async function runHostedGroomer( const newLabels = applyLabelChanges(candidate.labels, output.labelsToAdd, output.labelsToRemove); + // Compute title/body enrichment decisions + const titleBodyMutations = computeTitleBodyMutations(candidate, output); + // Build mutationPlan const mutationPlan: Record = { labelsToAdd: output.labelsToAdd, @@ -192,6 +197,11 @@ export async function runHostedGroomer( lane: output.lane, summary: output.summary ?? null, willComment: Boolean(output.githubComment?.trim()), + titleRewritten: titleBodyMutations.shouldRewrite, + originalTitle: titleBodyMutations.shouldRewrite ? candidate.title : undefined, + proposedTitle: titleBodyMutations.proposedTitle, + bodyEnriched: titleBodyMutations.shouldEnrich, + proposedBody: titleBodyMutations.proposedBody, }; // Persist stage planned @@ -231,6 +241,20 @@ export async function runHostedGroomer( await deps.updateLabels(candidate.repoFullName, candidate.number, newLabels); appliedMutations.labelsUpdated = true; + // Apply title and/or body updates if guardrails pass + const titleBodyFields: Record = {}; + if (titleBodyMutations.shouldRewrite && titleBodyMutations.proposedTitle) { + titleBodyFields.title = titleBodyMutations.proposedTitle; + } + if (titleBodyMutations.shouldEnrich && titleBodyMutations.proposedBody) { + titleBodyFields.body = titleBodyMutations.proposedBody; + } + if (Object.keys(titleBodyFields).length > 0) { + await deps.updateTitleAndBody(candidate.repoFullName, candidate.number, titleBodyFields as Parameters[2]); + appliedMutations.titleUpdated = titleBodyMutations.shouldRewrite; + appliedMutations.bodyUpdated = titleBodyMutations.shouldEnrich; + } + // Comment with cooldown enforcement if (output.githubComment?.trim()) { let commentPosted = false; @@ -400,6 +424,72 @@ function getCurrentStage(run: { stage?: string }): string { return run.stage ?? "selected"; } +/** + * Check if a title is "bad" and should be rewritten. + * Bad titles: length < 10 chars, or matches generic patterns (single word like "P0", "TODO", etc.), + * or is clearly just a priority/label token. + */ +function shouldRewriteTitle(title: string): boolean { + const trimmed = title.trim(); + if (trimmed.length === 0) return true; + if (trimmed.length < 10) return true; + + // Single word that looks like a generic token + const words = trimmed.split(/\s+/); + if (words.length === 1) { + const lower = trimmed.toLowerCase(); + const GENERIC_TOKENS = ["p0", "p1", "p2", "p3", "p4", "todo", "bug", "fix", "fixme", "wip", "help", "urgent", "critical"]; + if (GENERIC_TOKENS.includes(lower)) return true; + + // Priority/label-like tokens: starts with a letter/digit, no spaces, looks like a label prefix + if (/^[a-z0-9]+$/i.test(trimmed) && trimmed.length <= 6) return true; + } + + return false; +} + +/** + * Check if a body is "sparse" and should be enriched. + * Sparse bodies: missing, empty, or < 100 chars (excluding markdown/HTML comments). + */ +function shouldEnrichBody(body: string | null): boolean { + if (body === null || body.trim().length === 0) return true; + + // Strip HTML comments + let stripped = body.replace(//g, ""); + // Strip markdown-style block comments (if any) + stripped = stripped.replace(/^/gm, ""); + + if (stripped.trim().length < 100) return true; + return false; +} + +/** + * Compute title/body enrichment decisions and build the mutation plan entries. + */ +function computeTitleBodyMutations( + candidate: { title: string; body: string | null }, + output: GroomerOutput, +): { + shouldRewrite: boolean; + shouldEnrich: boolean; + proposedTitle?: string; + proposedBody?: string; +} { + const proposedTitle = typeof output.proposedTitle === "string" ? output.proposedTitle : undefined; + const proposedBody = typeof output.proposedBody === "string" ? output.proposedBody : undefined; + + const shouldRewrite = proposedTitle !== undefined && shouldRewriteTitle(candidate.title); + const shouldEnrich = proposedBody !== undefined && shouldEnrichBody(candidate.body); + + return { + shouldRewrite, + shouldEnrich, + ...(shouldRewrite ? { proposedTitle } : {}), + ...(shouldEnrich ? { proposedBody } : {}), + }; +} + function applyLabelChanges( current: string[], toAdd: string[], diff --git a/src/lib/groomer/schema.test.ts b/src/lib/groomer/schema.test.ts index 706b50b..489b74e 100644 --- a/src/lib/groomer/schema.test.ts +++ b/src/lib/groomer/schema.test.ts @@ -401,4 +401,98 @@ describe("groomer schema validation", () => { const result = validateGroomerOutput(null as any); expect(result.valid).toBe(false); }); + + // ─── proposedTitle validation ─── + + it("accepts valid proposedTitle (10-200 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix SSO/OIDC callback state verification mismatch", + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedTitle).toBe("Fix SSO/OIDC callback state verification mismatch"); + }); + + it("rejects proposedTitle that is too short (< 10 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix bug", + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be between 10 and 200 characters"); + }); + + it("rejects proposedTitle that is too long (> 200 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "A".repeat(201), + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be between 10 and 200 characters"); + }); + + it("accepts null proposedTitle (treated as omitted)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: null as any, + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedTitle).toBeUndefined(); + }); + + // ─── proposedBody validation ─── + + it("accepts valid proposedBody (under 10000 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: "## Context\nThis issue relates to the login flow.", + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedBody).toBe("## Context\nThis issue relates to the login flow."); + }); + + it("rejects proposedBody that is too long (> 10000 chars)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: "A".repeat(10_001), + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedBody must be under 10000 characters"); + }); + + it("accepts null proposedBody (treated as omitted)", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: null as any, + }); + expect(result.valid).toBe(true); + expect(result.parsed?.proposedBody).toBeUndefined(); + }); + + it("accepts both proposedTitle and proposedBody together", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: "Fix SSO callback state mismatch in auth module", + proposedBody: "## Context\nThe SSO login flow has a bug.", + }); + expect(result.valid).toBe(true); + }); + + it("rejects non-string proposedTitle when provided", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedTitle: 123 as any, + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedTitle must be a string"); + }); + + it("rejects non-string proposedBody when provided", () => { + const result = validateGroomerOutput({ + ...validOutput, + proposedBody: 123 as any, + }); + expect(result.valid).toBe(false); + expect(result.errors?.[0]).toContain("proposedBody must be a string"); + }); }); diff --git a/src/lib/groomer/schema.ts b/src/lib/groomer/schema.ts index 5cc9c47..6d41c24 100644 --- a/src/lib/groomer/schema.ts +++ b/src/lib/groomer/schema.ts @@ -17,6 +17,8 @@ export interface GroomerOutput { needsInfoReason?: string; blockedReason?: string; nextGroomingAction?: GroomAction; + proposedTitle?: string; + proposedBody?: string; } export interface ValidationResult { @@ -113,11 +115,13 @@ const baseSchema = z.object({ // ─── Optional String Fields ─────────────────────────────────────────────────── -const OPTIONAL_STRING_FIELDS: (keyof Pick)[] = [ +const OPTIONAL_STRING_FIELDS: (keyof Pick)[] = [ "summary", "githubComment", "needsInfoReason", "blockedReason", + "proposedTitle", + "proposedBody", ]; // ─── Main Validator ─────────────────────────────────────────────────────────── @@ -277,6 +281,28 @@ export function validateGroomerOutput(data: unknown): ValidationResult { parsed.confidence = resolvedConfidence as GroomerOutput["confidence"]; } + // proposedTitle — length guardrails (10-200 chars) + if ("proposedTitle" in obj) { + const rawTitle = obj.proposedTitle; + if (rawTitle !== null && typeof rawTitle === "string") { + if (rawTitle.length < 10 || rawTitle.length > 200) { + errors.push(`proposedTitle must be between 10 and 200 characters, got ${rawTitle.length}`); + return { valid: false, errors, resolutions }; + } + } + } + + // proposedBody — length guardrails (< 10000 chars) + if ("proposedBody" in obj) { + const rawBody = obj.proposedBody; + if (rawBody !== null && typeof rawBody === "string") { + if (rawBody.length > 10_000) { + errors.push(`proposedBody must be under 10000 characters, got ${rawBody.length}`); + return { valid: false, errors, resolutions }; + } + } + } + // Optional string fields — null tolerance (null treated as absent) for (const field of OPTIONAL_STRING_FIELDS) { if (!(field in obj)) continue;