diff --git a/apps/ade-cli/src/adeRpcServer.test.ts b/apps/ade-cli/src/adeRpcServer.test.ts index e8d9ebea8..b08690a14 100644 --- a/apps/ade-cli/src/adeRpcServer.test.ts +++ b/apps/ade-cli/src/adeRpcServer.test.ts @@ -4876,6 +4876,8 @@ describe("adeRpcServer", () => { failingCheckCount: 0, pendingCheckCount: 0, actionableReviewThreadCount: 1, + actionableIssueCommentCount: 1, + actionableCommentCount: 2, hasActionableChecks: false, hasActionableComments: true, }), diff --git a/apps/ade-cli/src/adeRpcServer.ts b/apps/ade-cli/src/adeRpcServer.ts index 8450906dd..85d2f5842 100644 --- a/apps/ade-cli/src/adeRpcServer.ts +++ b/apps/ade-cli/src/adeRpcServer.ts @@ -32,7 +32,10 @@ import { runGit } from "../../desktop/src/main/services/git/git"; import { resolvePathWithinRoot } from "../../desktop/src/main/services/shared/utils"; import { getDefaultModelDescriptor } from "../../desktop/src/shared/modelRegistry"; import { ADE_CLI_INLINE_GUIDANCE } from "../../desktop/src/shared/adeCliGuidance"; -import { getPrIssueResolutionAvailability } from "../../desktop/src/shared/prIssueResolution"; +import { + getPrIssueResolutionAvailability, + isActionablePrIssueComment, +} from "../../desktop/src/shared/prIssueResolution"; import { type LinearWorkflowConfig, type ComputerUseArtifactOwner, @@ -2694,11 +2697,6 @@ function requirePrService(runtime: AdeRuntime): NonNullable check.conclusion === "success").length; const failing = checks.filter((check) => check.conclusion === "failure").length; @@ -2722,12 +2720,7 @@ function summarizePrReviewComments( checks: PrCheck[], reviewThreads: PrReviewThread[], ) { - // Issue comments: still skip bot chatter (e.g. CI status echoes). Inline review-thread - // comments are NOT filtered here — they come from review bots like Greptile/CodeRabbit, - // which are exactly the actionable signal the agent needs to address. - const actionableIssueComments = comments.filter( - (comment) => Boolean(comment.body?.trim()) && comment.source === "issue" && !isBotAuthor(comment.author), - ); + const actionableIssueComments = comments.filter(isActionablePrIssueComment); const unresolvedThreads = reviewThreads.filter((thread) => !thread.isResolved && !thread.isOutdated); const actionableThreadCommentCount = unresolvedThreads.reduce((acc, thread) => acc + thread.comments.length, 0); const pendingReviews = reviews.filter((review) => review.state === "changes_requested" || review.state === "commented"); @@ -2782,7 +2775,7 @@ function summarizePrIssueInventory(args: { reviewThreads: PrReviewThread[]; comments: PrComment[]; }) { - const availability = getPrIssueResolutionAvailability(args.checks, args.reviewThreads); + const availability = getPrIssueResolutionAvailability(args.checks, args.reviewThreads, args.comments); const failingRuns = args.actionRuns .filter((run) => run.conclusion === "failure" || run.conclusion === "timed_out" || run.conclusion === "action_required") .map((run) => ({ @@ -2824,7 +2817,7 @@ function summarizePrIssueInventory(args: { })), })), issueComments: args.comments - .filter((comment) => comment.source === "issue") + .filter(isActionablePrIssueComment) .map((comment) => ({ id: comment.id, author: comment.author, diff --git a/apps/ade-cli/src/tuiClient/__tests__/commands.test.ts b/apps/ade-cli/src/tuiClient/__tests__/commands.test.ts index 55f3b2f7e..8efedb9fb 100644 --- a/apps/ade-cli/src/tuiClient/__tests__/commands.test.ts +++ b/apps/ade-cli/src/tuiClient/__tests__/commands.test.ts @@ -17,6 +17,17 @@ describe("commands", () => { expect(parsed ? commandPlacement(parsed) : null).toBe("right"); }); + it("routes PR comments to the right pane", () => { + const parsed = parseCommand("/pr comments"); + expect(parsed?.name).toBe("/pr comments"); + expect(parsed ? commandPlacement(parsed) : null).toBe("right"); + expect(paletteCommands("/pr comm")).toContainEqual(expect.objectContaining({ + name: "/pr comments", + source: "ade", + description: "Show actionable PR comments", + })); + }); + it("routes /feedback to the ADE Code right pane", () => { const parsed = parseCommand("/feedback"); expect(parsed?.spec?.name).toBe("/feedback"); diff --git a/apps/ade-cli/src/tuiClient/app.tsx b/apps/ade-cli/src/tuiClient/app.tsx index 18d791f02..ec2e3e0aa 100644 --- a/apps/ade-cli/src/tuiClient/app.tsx +++ b/apps/ade-cli/src/tuiClient/app.tsx @@ -4222,10 +4222,13 @@ export function AdeCodeApp({ project, forceEmbedded, requireSocket, socketPath } } const pr = name === "/pr checks" ? await conn.actionList("pr", "getChecks", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })) - : await Promise.all([ - conn.actionList("pr", "getReviews", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })), - conn.actionList("pr", "getReviewThreads", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })), - ]).then(([reviews, threads]) => ({ reviews, threads })); + : name === "/pr comments" + ? await conn.tool("pr_get_review_comments", { prId }).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })) + : await Promise.all([ + conn.actionList("pr", "getReviews", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })), + conn.actionList("pr", "getReviewThreads", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })), + conn.actionList("pr", "getComments", [prId]).catch((err) => ({ error: err instanceof Error ? err.message : String(err) })), + ]).then(([reviews, threads, comments]) => ({ reviews, threads, comments })); setRightPane({ kind: "details", title: name.slice(1), body: renderObject(pr, 24) }); return; } diff --git a/apps/ade-cli/src/tuiClient/commands.ts b/apps/ade-cli/src/tuiClient/commands.ts index 5144d4310..7bfc8380f 100644 --- a/apps/ade-cli/src/tuiClient/commands.ts +++ b/apps/ade-cli/src/tuiClient/commands.ts @@ -48,6 +48,7 @@ export const BUILTIN_COMMANDS: BuiltinCommand[] = [ { name: "/pr", description: "Show pull request state", placement: "right" }, { name: "/pr open", description: "Create or open a PR for the active lane", placement: "right" }, { name: "/pr review", description: "Show PR reviews", placement: "right" }, + { name: "/pr comments", description: "Show actionable PR comments", placement: "right" }, { name: "/pr checks", description: "Show PR checks", placement: "right" }, { name: "/linear", description: "Run Linear workflow, route, sync, or ingress commands", placement: "right", argumentHint: "" }, { name: "/linear list", description: "List Linear work", placement: "right" }, diff --git a/apps/desktop/src/main/services/ai/tools/workflowTools.test.ts b/apps/desktop/src/main/services/ai/tools/workflowTools.test.ts index e1c46e2b5..4a07c8c4e 100644 --- a/apps/desktop/src/main/services/ai/tools/workflowTools.test.ts +++ b/apps/desktop/src/main/services/ai/tools/workflowTools.test.ts @@ -86,6 +86,8 @@ describe("createWorkflowTools", () => { hasActionableComments: true, failingCheckCount: 1, actionableReviewThreadCount: 1, + actionableIssueCommentCount: 1, + actionableCommentCount: 2, }); expect(result.reviewThreads).toHaveLength(1); expect(result.failingWorkflowRuns[0]).toMatchObject({ name: "CI" }); diff --git a/apps/desktop/src/main/services/ai/tools/workflowTools.ts b/apps/desktop/src/main/services/ai/tools/workflowTools.ts index f4fb2cd20..31447a854 100644 --- a/apps/desktop/src/main/services/ai/tools/workflowTools.ts +++ b/apps/desktop/src/main/services/ai/tools/workflowTools.ts @@ -381,7 +381,7 @@ export function createWorkflowTools( prService.getReviewThreads(prId), prService.getComments(prId), ]); - const availability = getPrIssueResolutionAvailability(checks, reviewThreads); + const availability = getPrIssueResolutionAvailability(checks, reviewThreads, comments); const failingRuns = actionRuns .filter((run) => run.conclusion === "failure" || run.conclusion === "timed_out" || run.conclusion === "action_required") .map((run) => ({ diff --git a/apps/desktop/src/main/services/chat/agentChatService.test.ts b/apps/desktop/src/main/services/chat/agentChatService.test.ts index 0b91d060c..ee4c553ca 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.test.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.test.ts @@ -3579,7 +3579,9 @@ describe("createAgentChatService", () => { }); it("emits a rate-limit notice when the Claude SDK reports usage pressure", async () => { + vi.useFakeTimers(); const send = vi.fn().mockResolvedValue(undefined); + const close = vi.fn(); let streamCall = 0; const stream = vi.fn(() => (async function* () { streamCall += 1; @@ -3608,41 +3610,64 @@ describe("createAgentChatService", () => { vi.mocked(claudeSdkCreateSessionCompat).mockReturnValue({ send, stream, - close: vi.fn(), + close, + sessionId: "sdk-session-rate-limit", + } as any); + vi.mocked(claudeSdkResumeSessionCompat).mockReturnValue({ + send, + stream, + close, sessionId: "sdk-session-rate-limit", } as any); const onEvent = vi.fn(); const { service } = createService({ onEvent }); - const session = await service.createSession({ - laneId: "lane-1", - provider: "claude", - model: "sonnet", - }); + try { + const session = await service.createSession({ + laneId: "lane-1", + provider: "claude", + model: "sonnet", + }); - await service.runSessionTurn({ - sessionId: session.id, - text: "show usage pressure", - timeoutMs: 15_000, - }); - await new Promise((resolve) => setTimeout(resolve, 25)); + await service.runSessionTurn({ + sessionId: session.id, + text: "show usage pressure", + timeoutMs: 15_000, + }); - const rateLimitNotices = onEvent.mock.calls - .map((call) => call[0]) - .filter((env: any) => env?.event?.type === "system_notice" && env.event.noticeKind === "rate_limit"); - expect(rateLimitNotices).toHaveLength(1); - expect(rateLimitNotices[0].event).toMatchObject({ - type: "system_notice", - noticeKind: "rate_limit", - severity: "warning", - status: "allowed_warning", - message: "Claude rate limit allowed warning", - }); - expect(rateLimitNotices[0].event.detail).toContain("82% utilized"); - expect(rateLimitNotices[0].event.detail).toContain("resets"); - expect(claudeSdkCreateSessionCompat.mock.calls.some(([options]) => - options?.pathToClaudeCodeExecutable === "/usr/local/bin/claude", - )).toBe(true); + let rateLimitNotices = onEvent.mock.calls + .map((call) => call[0]) + .filter((env: any) => env?.event?.type === "system_notice" && env.event.noticeKind === "rate_limit"); + expect(rateLimitNotices).toHaveLength(1); + expect(rateLimitNotices[0].event).toMatchObject({ + type: "system_notice", + noticeKind: "rate_limit", + severity: "info", + status: "allowed_warning", + message: "Approaching Claude plan limit", + }); + expect(rateLimitNotices[0].event.detail).toContain("82% utilized"); + expect(rateLimitNotices[0].event.detail).toContain("resets"); + + await vi.advanceTimersByTimeAsync(6 * 60_000); + expect(close).toHaveBeenCalledTimes(1); + + await service.runSessionTurn({ + sessionId: session.id, + text: "show usage pressure again", + timeoutMs: 15_000, + }); + + rateLimitNotices = onEvent.mock.calls + .map((call) => call[0]) + .filter((env: any) => env?.event?.type === "system_notice" && env.event.noticeKind === "rate_limit"); + expect(rateLimitNotices).toHaveLength(1); + expect(claudeSdkCreateSessionCompat.mock.calls.some(([options]) => + options?.pathToClaudeCodeExecutable === "/usr/local/bin/claude", + )).toBe(true); + } finally { + vi.useRealTimers(); + } }); it("registers a PreCompact hook on non-lightweight Claude sessions", async () => { diff --git a/apps/desktop/src/main/services/chat/agentChatService.ts b/apps/desktop/src/main/services/chat/agentChatService.ts index 942ddbe42..f73bf79c2 100644 --- a/apps/desktop/src/main/services/chat/agentChatService.ts +++ b/apps/desktop/src/main/services/chat/agentChatService.ts @@ -544,6 +544,8 @@ type ClaudeRuntime = { pauseIdleWatchdog?: (() => void) | null; /** Resume the active-turn idle watchdog after the blocking wait finishes. */ resumeIdleWatchdog?: (() => void) | null; + /** Set after we've emitted the once-per-session "Approaching plan limit" notice. */ + rateLimitWarningEmitted: boolean; }; const CODEX_BUILT_IN_SLASH_COMMANDS: AgentChatSlashCommand[] = [ @@ -1451,6 +1453,8 @@ type ManagedChatSession = { selectedExecutionLaneId: string | null; lastLaneDirectiveKey: string | null; runtimeInvalidated: boolean; + /** Set after we've emitted the once-per-session Claude plan-limit notice. */ + claudeRateLimitWarningEmitted: boolean; codexTerminalTurnIds: Set; todoItems: Extract["items"]; localPendingInputs: Map(persisted?.codexTerminalTurnIds ?? []), todoItems: [], activeAssistantMessageId: null, @@ -9414,13 +9419,16 @@ export function createAgentChatService(args: { const rateMsg = msg as any; const info = rateMsg.rate_limit_info ?? {}; const rawStatus = typeof info.status === "string" ? info.status : "updated"; - const status = rawStatus.replace(/_/g, " "); - const severity: "info" | "warning" | "error" = - rawStatus === "allowed" - ? "info" - : rawStatus === "allowed_warning" - ? "warning" - : "error"; + const isError = rawStatus !== "allowed" && rawStatus !== "allowed_warning"; + // "allowed" = under threshold (no signal needed). "allowed_warning" = approaching limit; + // surface as an informational once-per-session notice. Anything else = real failure. + if (rawStatus === "allowed") continue; + if (rawStatus === "allowed_warning" && managed.claudeRateLimitWarningEmitted) continue; + if (rawStatus === "allowed_warning") { + managed.claudeRateLimitWarningEmitted = true; + runtime.rateLimitWarningEmitted = true; + } + const severity: "info" | "warning" | "error" = isError ? "error" : "info"; const details: string[] = []; if (typeof info.utilization === "number") { const percent = info.utilization <= 1 @@ -9433,12 +9441,15 @@ export function createAgentChatService(args: { const resetDate = new Date(resetMs); if (!Number.isNaN(resetDate.getTime())) details.push(`resets ${resetDate.toISOString()}`); } + const message = isError + ? `Claude rate limit ${rawStatus.replace(/_/g, " ")}` + : "Approaching Claude plan limit"; emitChatEvent(managed, { type: "system_notice", noticeKind: "rate_limit", severity, status: rawStatus, - message: `Claude rate limit ${status}`, + message, detail: details.length ? details.join(" | ") : undefined, turnId, }); @@ -14298,6 +14309,7 @@ export function createAgentChatService(args: { turnMemoryPolicyState: null, approvalOverrides: new Set(persisted?.approvalOverrides ?? []), resolvedToolUseIds: new Set(), + rateLimitWarningEmitted: managed.claudeRateLimitWarningEmitted, }; managed.runtime = runtime; managed.runtimeInvalidated = false; @@ -14345,6 +14357,7 @@ export function createAgentChatService(args: { selectedExecutionLaneId: null, lastLaneDirectiveKey: null, runtimeInvalidated: false, + claudeRateLimitWarningEmitted: false, codexTerminalTurnIds: new Set(), todoItems: [], activeAssistantMessageId: null, @@ -14790,6 +14803,7 @@ export function createAgentChatService(args: { selectedExecutionLaneId: null, lastLaneDirectiveKey: null, runtimeInvalidated: false, + claudeRateLimitWarningEmitted: false, codexTerminalTurnIds: new Set(), todoItems: [], activeAssistantMessageId: null, diff --git a/apps/desktop/src/main/services/ipc/registerIpc.ts b/apps/desktop/src/main/services/ipc/registerIpc.ts index 247c88caa..a6c27ab34 100644 --- a/apps/desktop/src/main/services/ipc/registerIpc.ts +++ b/apps/desktop/src/main/services/ipc/registerIpc.ts @@ -8112,12 +8112,16 @@ export function registerIpc({ ipcMain.handle(IPC.prsCreateFromLane, async (_event, arg: CreatePrFromLaneArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.createFromLane(arg); + const result = await ctx.prService.createFromLane(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsLinkToLane, async (_event, arg: LinkPrToLaneArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.linkToLane(arg); + const result = await ctx.prService.linkToLane(arg); + ctx.prPollingService.poke(); + return result; }); const ensurePrPolling = () => { @@ -8200,11 +8204,14 @@ export function registerIpc({ ipcMain.handle(IPC.prsUpdateDescription, async (_event, arg: UpdatePrDescriptionArgs): Promise => { const ctx = getCtx(); await ctx.prService.updateDescription(arg); + ctx.prPollingService.poke(); }); ipcMain.handle(IPC.prsDelete, async (_event, arg: DeletePrArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.delete(arg); + const result = await ctx.prService.delete(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsDraftDescription, async (_event, arg: DraftPrDescriptionArgs): Promise<{ title: string; body: string }> => { @@ -8214,17 +8221,22 @@ export function registerIpc({ ipcMain.handle(IPC.prsLand, async (_event, arg: LandPrArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.land(arg); + const result = await ctx.prService.land(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsLandStack, async (_event, arg: LandStackArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.landStack(arg); + const result = await ctx.prService.landStack(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsRetargetBase, async (_event, arg: { prId: string; baseBranch: string }): Promise => { const ctx = getCtx(); - return await ctx.prService.retargetBase(arg.prId, arg.baseBranch); + await ctx.prService.retargetBase(arg.prId, arg.baseBranch); + ctx.prPollingService.poke(); }); ipcMain.handle(IPC.prsOpenInGitHub, async (_event, arg: { prId: string }): Promise => { @@ -8234,12 +8246,16 @@ export function registerIpc({ ipcMain.handle(IPC.prsCreateIntegration, async (_event, arg: CreateIntegrationPrArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.createIntegrationPr(arg); + const result = await ctx.prService.createIntegrationPr(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsLandStackEnhanced, async (_event, arg: LandStackEnhancedArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.landStackEnhanced(arg); + const result = await ctx.prService.landStackEnhanced(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsGetConflictAnalysis, async (_event, arg: { prId: string }) => getCtx().prService.getConflictAnalysis(arg.prId)); @@ -8269,14 +8285,18 @@ export function registerIpc({ ipcMain.handle(IPC.prsCreateQueue, async (_event, arg: CreateQueuePrsArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.createQueuePrs(arg); + const result = await ctx.prService.createQueuePrs(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsSimulateIntegration, async (_event, arg: SimulateIntegrationArgs): Promise => getCtx().prService.simulateIntegration(arg)); ipcMain.handle(IPC.prsCommitIntegration, async (_event, arg: CommitIntegrationArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.commitIntegration(arg); + const result = await ctx.prService.commitIntegration(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsListProposals, async (): Promise => @@ -8305,7 +8325,9 @@ export function registerIpc({ ipcMain.handle(IPC.prsLandQueueNext, async (_event, arg: LandQueueNextArgs): Promise => { const ctx = getCtx(); - return await ctx.prService.landQueueNext(arg); + const result = await ctx.prService.landQueueNext(arg); + ctx.prPollingService.poke(); + return result; }); ipcMain.handle(IPC.prsStartQueueAutomation, async (_event, arg) => { @@ -8323,7 +8345,9 @@ export function registerIpc({ ipcMain.handle(IPC.prsCancelQueueAutomation, async (_event, arg) => getCtx().queueLandingService.cancelQueue(arg.queueId)); ipcMain.handle(IPC.prsReorderQueue, async (_event, arg: ReorderQueuePrsArgs): Promise => { - await getCtx().prService.reorderQueuePrs(arg); + const ctx = getCtx(); + await ctx.prService.reorderQueuePrs(arg); + ctx.prPollingService.poke(); }); ipcMain.handle(IPC.prsGetHealth, async (_event, arg: { prId: string }): Promise => getCtx().prService.getPrHealth(arg.prId)); diff --git a/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts b/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts index d997800b1..8f4eae3ff 100644 --- a/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts +++ b/apps/desktop/src/main/services/prs/pathToMergeOrchestrator.ts @@ -584,11 +584,12 @@ export function createPathToMergeOrchestrator(deps: PathToMergeDeps): PathToMerg requestedScope: PrIssueResolutionScope, options: { allowFallback: boolean } = { allowFallback: true }, ): Promise { - const [checks, reviewThreads] = await Promise.all([ + const [checks, reviewThreads, comments] = await Promise.all([ prService.getChecks(prId), prService.getReviewThreads(prId), + prService.getComments(prId).catch(() => []), ]); - const availability = getPrIssueResolutionAvailability(checks, reviewThreads); + const availability = getPrIssueResolutionAvailability(checks, reviewThreads, comments); if (requestedScope === "both") { return defaultPrIssueResolutionScope(availability); } diff --git a/apps/desktop/src/main/services/prs/prIssueResolution.test.ts b/apps/desktop/src/main/services/prs/prIssueResolution.test.ts index f2c465ad2..c64c700f2 100644 --- a/apps/desktop/src/main/services/prs/prIssueResolution.test.ts +++ b/apps/desktop/src/main/services/prs/prIssueResolution.test.ts @@ -2665,7 +2665,7 @@ describe("buildPrIssueResolutionPrompt", () => { recentCommits: [{ sha: "abcdef123456", subject: "Refine PR detail header" }], }); - expect(prompt).toContain("Selected scope: checks and review comments"); + expect(prompt).toContain("Selected scope: checks and PR comments"); expect(prompt).toContain("ADE PR id (for ADE tools): pr-80"); expect(prompt).toContain("Runtime: Workflow chat with ADE PR tools"); expect(prompt).toContain("Please keep the PR description accurate if behavior changes."); @@ -3096,12 +3096,24 @@ describe("buildPrIssueResolutionPrompt — inventory items", () => { author: null, url: "https://example.com/check/1", }), + makeInventoryItem({ + id: "inv-3", + type: "issue_comment", + source: "human", + severity: "minor", + filePath: null, + line: null, + headline: "Clarify rollout note", + externalId: "issue-comment:c-1", + author: "reviewer", + url: "https://example.com/comment/c-1", + }), ], })); // Should use inventory section instead of raw threads/checks expect(prompt).toContain("Current issues to address (from inventory"); - expect(prompt).toContain("[Major] Thread thread:t-1 at src/prs.ts:55"); + expect(prompt).toContain("[Major] Review thread thread:t-1 at src/prs.ts:55"); expect(prompt).toContain("source: coderabbit"); expect(prompt).toContain("author: coderabbitai"); expect(prompt).toContain("Summary: Handle the loading state"); @@ -3110,6 +3122,8 @@ describe("buildPrIssueResolutionPrompt — inventory items", () => { // Check failure formatting expect(prompt).toContain('Check check:ci / lint at unknown location'); expect(prompt).toContain('Summary: CI check "ci / lint" failing'); + expect(prompt).toContain("[Minor] PR comment issue-comment:c-1 at unknown location"); + expect(prompt).toContain("Summary: Clarify rollout note"); // Should NOT contain the raw threads/checks sections expect(prompt).not.toContain("Current failing checks"); diff --git a/apps/desktop/src/main/services/prs/prIssueResolver.ts b/apps/desktop/src/main/services/prs/prIssueResolver.ts index a4fb81ae2..fa42ad316 100644 --- a/apps/desktop/src/main/services/prs/prIssueResolver.ts +++ b/apps/desktop/src/main/services/prs/prIssueResolver.ts @@ -325,13 +325,18 @@ function formatInventoryItemsSummary(items: IssueInventoryItem[]): string { const severityPrefix = item.severity ? `[${item.severity[0].toUpperCase()}${item.severity.slice(1)}] ` : ""; const sourceTag = item.source !== "unknown" ? ` | source: ${item.source}` : ""; const authorTag = item.author ? ` | author: ${item.author}` : ""; - return `${index + 1}. ${severityPrefix}${item.type === "check_failure" ? "Check" : "Thread"} ${item.externalId} at ${location}${sourceTag}${authorTag}\n Summary: ${item.headline}\n Reference: ${item.url ?? "(no URL available)"}`; + const itemLabel = item.type === "check_failure" + ? "Check" + : item.type === "issue_comment" + ? "PR comment" + : "Review thread"; + return `${index + 1}. ${severityPrefix}${itemLabel} ${item.externalId} at ${location}${sourceTag}${authorTag}\n Summary: ${item.headline}\n Reference: ${item.url ?? "(no URL available)"}`; }).join("\n"); } function buildSelectedScopeDescription(scope: PrIssueResolutionScope): string { - if (scope === "both") return "checks and review comments"; - if (scope === "comments") return "review comments"; + if (scope === "both") return "checks and PR comments"; + if (scope === "comments") return "PR comments"; return "checks"; } @@ -394,7 +399,7 @@ function listRequiredRuntimeTools(runtimeCapabilities: PrIssueResolutionRuntimeC export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): string { const actionableThreads = args.reviewThreads.filter((thread) => !thread.isResolved && !thread.isOutdated); - const availability = getPrIssueResolutionAvailability(args.checks, args.reviewThreads); + const availability = getPrIssueResolutionAvailability(args.checks, args.reviewThreads, args.issueComments); const prBodySummary = summarizePrBody(args.detail?.body); const purposeBits = [ args.pr.title.trim(), @@ -432,6 +437,7 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s promptSections.push( `- Actionable failing checks: ${availability.hasActionableChecks ? availability.failingCheckCount : 0}`, `- Actionable unresolved review threads: ${actionableThreads.length}`, + `- Actionable top-level PR comments: ${availability.actionableIssueCommentCount}`, "", "Purpose / intent", purposeBits.length > 0 ? purposeBits.join("\n\n") : "- No additional PR purpose text was available.", @@ -538,9 +544,9 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s "- Run `ade doctor` if readiness is unclear. Use `--json` for structured output and `--text` for readable summaries.", "- Discover additional ADE actions with `ade actions list --text`; prefer typed PR commands first and `ade actions run ...` only as an escape hatch.", `- Start by refreshing the PR issue inventory with \`${runtimeCapabilities.refreshInventoryTool} ${args.pr.id}\`.`, - `- Immediately after that, run \`${runtimeCapabilities.getReviewCommentsTool} ${args.pr.id}\` to load the full review-thread bodies and line context before deciding which comments are stale, valid, or already addressed.`, + `- Immediately after that, run \`${runtimeCapabilities.getReviewCommentsTool} ${args.pr.id}\` to load the full PR comment bodies and line context before deciding which comments are stale, valid, or already addressed.`, `- Reply to review threads with \`${runtimeCapabilities.replyThreadTool} ${args.pr.id} --thread --body \` and resolve completed threads with \`${runtimeCapabilities.resolveThreadTool} ${args.pr.id} --thread \`.`, - "- Treat the refreshed inventory as a triage index, not as the full source of truth for long comment bodies. If a summary looks compact or truncated, fetch the detailed review comments instead of guessing.", + "- Treat the refreshed inventory as a triage index, not as the full source of truth for long comment bodies. If a summary looks compact or truncated, fetch the detailed PR comments instead of guessing.", "- Do not spend your first steps reading local skill docs, repo docs, or unrelated files before those PR context calls succeed.", "- If the ADE CLI is unavailable in-session, continue with the prompt's issue context and linked GitHub thread/check URLs instead of reverse-engineering ADE internals.", ); @@ -548,8 +554,8 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s const toolList = listRequiredRuntimeTools(runtimeCapabilities).map((toolName) => `\`${toolName}\``).join(", "); promptSections.push( `- This workflow chat is expected to expose ADE PR tools. Start by refreshing the PR issue inventory with \`${runtimeCapabilities.refreshInventoryTool}\`.`, - `- Immediately after that, call \`${runtimeCapabilities.getReviewCommentsTool}\` to load the full review-thread bodies and line context before deciding which comments are stale, valid, or already addressed.`, - "- Treat the refreshed inventory as a triage index, not as the full source of truth for long comment bodies. If a summary looks compact or truncated, fetch the detailed review comments instead of guessing.", + `- Immediately after that, call \`${runtimeCapabilities.getReviewCommentsTool}\` to load the full PR comment bodies and line context before deciding which comments are stale, valid, or already addressed.`, + "- Treat the refreshed inventory as a triage index, not as the full source of truth for long comment bodies. If a summary looks compact or truncated, fetch the detailed PR comments instead of guessing.", "- Do not spend your first steps reading local skill docs, repo docs, or unrelated files before those PR context calls succeed.", `- Required PR tools for this run: ${toolList}. If any of them are unavailable, stop and report that the chat was launched without the required ADE PR tools.`, "- Do not waste time reverse-engineering ADE runtime wiring from inside the task session.", @@ -557,8 +563,8 @@ export function buildPrIssueResolutionPrompt(args: IssueResolutionPromptArgs): s } promptSections.push( - "- Verify review comments before changing code. Some comments may be stale, incorrect, or already addressed.", - "- If you work on review comments, reply on the review thread when useful and resolve the thread only after the fix is truly in place or the thread is clearly outdated/invalid.", + "- Verify PR comments before changing code. Some comments may be stale, incorrect, or already addressed.", + "- If you work on review threads, reply on the thread when useful and resolve it only after the fix is truly in place or the thread is clearly outdated/invalid.", "- If you are running outside ADE, use the linked GitHub thread/check URLs together with your local git and CI tooling.", runtimeCapabilities.executionMode ? "- Use parallel agents or subagents when they will materially speed up independent fixes." @@ -612,7 +618,7 @@ async function preparePrIssueResolutionPrompt( deps.prService.getComments(pr.id).catch(() => [] as PrComment[]), ]); - const availability = getPrIssueResolutionAvailability(checks, reviewThreads); + const availability = getPrIssueResolutionAvailability(checks, reviewThreads, comments); if (args.scope === "checks" && !availability.hasActionableChecks) { throw new Error("Failing checks are not currently actionable. Checks must be finished running and still failing."); } diff --git a/apps/desktop/src/main/services/prs/resolverUtils.ts b/apps/desktop/src/main/services/prs/resolverUtils.ts index 842b57b6a..3cd08f06a 100644 --- a/apps/desktop/src/main/services/prs/resolverUtils.ts +++ b/apps/desktop/src/main/services/prs/resolverUtils.ts @@ -1,35 +1,13 @@ import type { AgentChatPermissionMode, PrAgentPermissionMode, PrComment } from "../../../shared/types"; +import { isActionablePrIssueComment } from "../../../shared/prIssueResolution"; import { runGit } from "../git/git"; // --------------------------------------------------------------------------- // Noisy comment detection — shared by issue inventory and issue resolver // --------------------------------------------------------------------------- -const NOISY_BOT_AUTHORS = new Set(["vercel", "vercel[bot]", "mintlify", "mintlify[bot]"]); - -const NOISY_BODY_PATTERNS = [ - /\[vc\]:/i, - /mintlify-preview/i, - /this is an auto-generated comment/i, - //i, - /\bcapy auto-review is paused\b/i, - /pre-merge checks/i, - /thanks for using \[coderabbit\]/i, - //i, + /\bcapy auto-review is paused\b/i, + /pre-merge checks/i, + /thanks for using \[coderabbit\]/i, + /