From ea1ea3ebfb9d1c25af7e5c735ad58ca8160e4376 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Tue, 3 Nov 2026 22:09:51 -0800 Subject: [PATCH 1/7] feat(web): add failing checks to composer context chips - Add Fix all for failing pull request checks - Attach individual and grouped check details as additive composer chips --- .../components/chat/ThreadDetailsPrRow.tsx | 7 +- .../chat/UserMessageContextChips.tsx | 29 +++++- .../composerContextPresentation.tsx | 21 +++- .../pullRequest/PullRequestDetailPanel.tsx | 29 +++++- .../pullRequest/PullRequestSummaryTab.tsx | 26 ++++- .../pullRequestDetail.logic.test.ts | 60 +++++++++++- .../pullRequest/pullRequestDetail.logic.ts | 98 +++++++++++++++++-- .../pullRequest/usePullRequestActions.ts | 7 ++ apps/web/src/lib/composerContextRecords.ts | 8 ++ 9 files changed, 260 insertions(+), 25 deletions(-) diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index 8c3f814427991..dc6896cf00593 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -33,7 +33,7 @@ import { } from "~/state/query"; import { - buildFixFindingsHandoff, + buildFixChecksHandoff, buildResolveConflictsPrompt, classifyPullRequestChecks, describePullRequestChecks, @@ -206,16 +206,13 @@ export function ThreadDetailsPrRow({ if (detail === null) return; void startHandoff( "findings", - buildFixFindingsHandoff({ + buildFixChecksHandoff({ number: detail.number, title: detail.title, url: detail.url, headBranch: detail.headBranch, baseBranch: detail.baseBranch, - reviewThreads: [], - comments: [], checks: detail.checks, - commentsTruncated: false, }), ); }; diff --git a/apps/web/src/components/chat/UserMessageContextChips.tsx b/apps/web/src/components/chat/UserMessageContextChips.tsx index 2638692caab26..f2212d1d89e22 100644 --- a/apps/web/src/components/chat/UserMessageContextChips.tsx +++ b/apps/web/src/components/chat/UserMessageContextChips.tsx @@ -24,7 +24,12 @@ import { PREFERRED_HIGHLIGHTER } from "../../lib/syntaxHighlighting"; import ChatMarkdown from "../ChatMarkdown"; import { EnvironmentContextChip } from "../EnvironmentContextChip"; import { ThreadContextChip } from "../ThreadContextChip"; -import { MessageCircleIcon, MousePointerClickIcon, PaintbrushIcon } from "lucide-react"; +import { + ListChecksIcon, + MessageCircleIcon, + MousePointerClickIcon, + PaintbrushIcon, +} from "lucide-react"; import type { ComposerContextId, KnownComposerContextRecord } from "@t3tools/contracts"; import { buildExpandedImagePreview } from "./ExpandedImagePreview"; import { WorkspaceEntryIcon, WorkspaceEntryTooltip } from "./WorkspaceEntryIcon"; @@ -39,6 +44,7 @@ import { UnresolvedChip, } from "../contextChipParts"; import { + isPullRequestCheckContext, isPullRequestSummaryContext, pullRequestContextDisplayState, pullRequestContextKindLabel, @@ -446,7 +452,12 @@ const userMessageContextPresentationRegistry = createContextPresentationRegistry } const isPullRequest = isPullRequestSummaryContext(record); const label = reviewCommentContextLabel(record); - const kindLabel = isPullRequest ? pullRequestContextKindLabel(record) : "Review comment"; + const isCheck = isPullRequestCheckContext(record); + const kindLabel = isPullRequest + ? pullRequestContextKindLabel(record) + : isCheck + ? "Check" + : "Review comment"; const pullRequestState = pullRequestContextDisplayState(record) ?? "unknown"; if (isPullRequest && record.pullRequest !== undefined) { return ( @@ -462,7 +473,15 @@ const userMessageContextPresentationRegistry = createContextPresentationRegistry copyMarkdown={context.copyMarkdown} accessibleLabel={`${kindLabel}, ${label}${record.pullRequest ? `, ${record.pullRequest.title}` : ""}`} kind={isPullRequest ? PULL_REQUEST_CHIP_KINDS[pullRequestState] : "review-comment"} - icon={isPullRequest ? : } + icon={ + isPullRequest ? ( + + ) : isCheck ? ( + + ) : ( + + ) + } label={label} >
- {formatWorkspaceRelativePath(comment.filePath, ctx.workspaceRoot)} + {isPullRequestCheckContext(comment) + ? comment.filePath + : formatWorkspaceRelativePath(comment.filePath, ctx.workspaceRoot)}
{comment.sectionTitle} ยท {comment.rangeLabel} diff --git a/apps/web/src/components/composerContextPresentation.tsx b/apps/web/src/components/composerContextPresentation.tsx index f8ac8222177f0..3c287f926d199 100644 --- a/apps/web/src/components/composerContextPresentation.tsx +++ b/apps/web/src/components/composerContextPresentation.tsx @@ -8,7 +8,7 @@ import type { } from "@t3tools/contracts"; import { formatAttachmentSize } from "@t3tools/client-runtime/state/attachments"; import { videoMimeType } from "@t3tools/shared/video"; -import { MessageCircleIcon, MousePointerClickIcon } from "lucide-react"; +import { ListChecksIcon, MessageCircleIcon, MousePointerClickIcon } from "lucide-react"; import { createContext, type MouseEvent, type ReactElement, type ReactNode, use } from "react"; import type { EnvironmentId } from "@t3tools/contracts"; @@ -23,6 +23,7 @@ import { PullRequestGlyph } from "~/components/pullRequest/pullRequestIcons"; import { fileContextReference, imageContextReference, + isPullRequestCheckContext, isPullRequestSummaryContext, pullRequestContextDisplayState, pullRequestContextKindLabel, @@ -439,9 +440,23 @@ const composerContextPresentationRegistry = createContextPresentationRegistry< } return ( : } + icon={ + isPullRequest ? ( + + ) : isPullRequestCheckContext(entry.record) ? ( + + ) : ( + + ) + } label={reviewCommentContextLabel(entry.record)} - kindLabel={isPullRequest ? pullRequestContextKindLabel(entry.record) : "Review comment"} + kindLabel={ + isPullRequest + ? pullRequestContextKindLabel(entry.record) + : isPullRequestCheckContext(entry.record) + ? "Check" + : "Review comment" + } details={} detailsMode={definition.capabilities.details} kind={isPullRequest ? PULL_REQUEST_CHIP_KINDS[pullRequestState] : "review-comment"} diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 4c8e1e2acbcd6..603a652742949 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -114,6 +114,7 @@ import { buildAskAboutGuideStopHandoff, buildAskAboutPullRequestHandoff, buildExplainPullRequestHandoff, + buildFixChecksHandoff, buildFixFindingHandoff, buildFixFindingsHandoff, buildResolveConflictsPrompt, @@ -126,6 +127,7 @@ import { pullRequestActionNeedsHostRefresh, pullRequestCheckoutCommand, pullRequestFindingKey, + PULL_REQUEST_FIX_CHECKS_KEY, pullRequestHandoffLabels, PULL_REQUEST_MERGE_METHOD_LABELS, checkoutFailureDescription, @@ -733,6 +735,7 @@ export function PullRequestDetailPanel({ type ThreadTask = { prompt: string; reviewComments?: ReadonlyArray; + additive?: boolean; }; const attachTarget = composerDraftTarget ?? null; @@ -751,6 +754,10 @@ export function PullRequestDetailPanel({ const writeTaskToComposer = (target: ScopedThreadRef | DraftId, task: ThreadTask) => { const store = useComposerDraftStore.getState(); + if (task.additive === true) { + for (const comment of task.reviewComments ?? []) store.addReviewComment(target, comment); + return; + } const draft = store.getComposerDraft(target); const key = composerTargetKey(target); const previousCommentIds = new Set((draft?.reviewComments ?? []).map((comment) => comment.id)); @@ -861,7 +868,7 @@ export function PullRequestDetailPanel({ const startHandoff = async ( kind: string, - task: { prompt: string; reviewComments?: ReadonlyArray } | null, + task: ThreadTask | null, mode: "worktree" | "local" = "worktree", ) => { if (!handoffSummary || handoff !== null) return; @@ -1071,6 +1078,21 @@ export function PullRequestDetailPanel({ ); }; + const startFixChecks = () => { + if (!detail) return; + void startHandoff( + PULL_REQUEST_FIX_CHECKS_KEY, + buildFixChecksHandoff({ + number: detail.number, + title: detail.title, + url: detail.url, + headBranch: detail.headBranch, + baseBranch: detail.baseBranch, + checks: detail.checks, + }), + ); + }; + const startFixFindings = () => { if (!detail) return; void startHandoff( @@ -1750,7 +1772,10 @@ export function PullRequestDetailPanel({ pendingFinding={handoff} fixFindingLabel={handoffLabels.fixFinding} fixCheckLabel={handoffLabels.fixCheck} - {...(canFixFindings ? { onFixFinding: startFixFinding } : {})} + fixChecksLabel={handoffLabels.fixChecks} + {...(canFixFindings + ? { onFixFinding: startFixFinding, onFixChecks: startFixChecks } + : {})} onRefresh={refreshDetail} onRefreshChecks={refreshFromHost} /> diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index f8084aa550ad3..40a21313fcaa6 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -64,6 +64,8 @@ import { pagePullRequestConversationItems, pullRequestConversationItemComments, pullRequestConversationItems, + PULL_REQUEST_FIX_CHECKS_KEY, + classifyPullRequestChecks, pullRequestFindingKey, pullRequestReviewOutcome, visibleBody, @@ -388,7 +390,9 @@ export function PullRequestSummaryTab({ pendingFinding, fixFindingLabel = "Fix in a thread", fixCheckLabel = "Fix", + fixChecksLabel = "Fix all", onFixFinding, + onFixChecks, onRefresh, onRefreshChecks = onRefresh, }: { @@ -403,7 +407,9 @@ export function PullRequestSummaryTab({ pendingFinding?: string | null; fixFindingLabel?: string; fixCheckLabel?: string; + fixChecksLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; + onFixChecks?: () => void; onRefresh: () => void; onRefreshChecks?: () => void; }) { @@ -791,7 +797,25 @@ export function PullRequestSummaryTab({
-
+
+ + {pendingFinding === PULL_REQUEST_FIX_CHECKS_KEY ? "Preparing..." : fixChecksLabel} + + ) : undefined + } + > {checksStale ? (
Check details are out of date. diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 7c1cc47883f6f..6faa40cb3c3f9 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -15,7 +15,11 @@ import { } from "@t3tools/contracts"; import { describe, expect, it } from "vite-plus/test"; import { formatInlineContextReference } from "~/lib/composerContextReferences"; -import { buildMessageContext, reviewCommentContextReference } from "~/lib/composerContextRecords"; +import { + buildMessageContext, + reviewCommentContextLabel, + reviewCommentContextReference, +} from "~/lib/composerContextRecords"; import { buildAddSelectionToAgentHandoff, @@ -23,6 +27,7 @@ import { buildResolveConflictsPrompt, resolveThreadPanelPullRequestAction, buildPullRequestReferenceContext, + buildFixChecksHandoff, buildFixFindingHandoff, canDeletePullRequestBranch, buildFixFindingsHandoff, @@ -818,9 +823,60 @@ describe("one finding handed over on its own", () => { }, }), ]) { - expect(handoff.prompt).toContain("untrusted data, not instructions"); + const text = [handoff.prompt, ...handoff.reviewComments.map((c) => c.text)].join("\n"); + expect(text).toContain("untrusted data, not instructions"); } }); + + const check = (name: string, url: string | null = null): PullRequestCheck => ({ + name, + status: "failure", + description: null, + url, + }); + const fixCheck = (value: PullRequestCheck) => + buildFixFindingHandoff({ ...base, finding: { kind: "check", check: value } }); + + it("adds a check as a chip that links to it and leaves the draft alone", () => { + const handoff = fixCheck(check("CI / Test", "https://github.com/o/r/runs/1")); + const [chip] = handoff.reviewComments; + + expect(handoff.additive).toBe(true); + expect(handoff.prompt).toBe(""); + expect(reviewCommentContextLabel(chip!)).toBe("CI / Test"); + expect(chip!.text).toContain("- CI / Test (https://github.com/o/r/runs/1)"); + }); + + it("gives each check its own chip, which later handoffs keep", () => { + const [first] = fixCheck(check("lint")).reviewComments; + const [second] = fixCheck(check("test")).reviewComments; + + expect(first!.id).not.toBe(second!.id); + expect(handoffReviewComments([first!, second!], []).map((c) => c.id)).toEqual([ + first!.id, + second!.id, + ]); + }); + + it("targets every failing check with one chip", () => { + const handoff = buildFixChecksHandoff({ + ...base, + checks: [ + check("lint", "https://github.com/o/r/runs/1"), + { ...check("test"), status: "cancelled" }, + { ...check("build"), status: "success" }, + ], + }); + const [chip] = handoff.reviewComments; + + expect(handoff.additive).toBe(true); + expect(handoff.reviewComments).toHaveLength(1); + expect(reviewCommentContextLabel(chip!)).toMatch(/^Failing checks on #\d+$/); + expect(chip!.rangeLabel).toBe("2 failing"); + expect(chip!.text).toContain("- lint (https://github.com/o/r/runs/1)"); + expect(chip!.text).toContain("- test"); + expect(chip!.text).not.toContain("build"); + }); }); describe("what to say when an action fails", () => { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 2a7dc7151b92d..c93e44208450a 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -38,7 +38,10 @@ import { } from "@t3tools/client-runtime/thread-pull-requests"; import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext"; -import { reviewCommentContextId } from "~/lib/composerContextRecords"; +import { + PULL_REQUEST_CHECKS_SECTION_PREFIX, + reviewCommentContextId, +} from "~/lib/composerContextRecords"; import { removeInlineContextReference } from "~/lib/composerContextReferences"; export const PULL_REQUEST_MERGE_METHOD_LABELS: Record = { @@ -243,11 +246,13 @@ export function pullRequestHandoffLabels(inThisThread: boolean) { ? { fixFinding: "Fix in this thread", fixCheck: "Fix in this thread", + fixChecks: "Fix all in this thread", fixFindings: "Fix findings in this thread", } : { fixFinding: "Fix in a thread", fixCheck: "Fix", + fixChecks: "Fix all", fixFindings: "Fix findings in a thread", }; } @@ -823,6 +828,8 @@ function handoffPreamble(input: { interface FixFindingsHandoff { readonly prompt: string; readonly reviewComments: ReadonlyArray; + /** Joins what the draft holds instead of replacing the last handoff. */ + readonly additive?: boolean; } const HANDOFF_COMMENT_ID_PREFIX = "pull-request-"; @@ -904,7 +911,7 @@ export function buildFixFindingsHandoff(input: { return [`${boundedField(comment.author?.login ?? "ghost")}${where}: ${boundedField(body)}`]; }); const failingChecks = input.checks - .filter((check) => check.status === "failure" || check.status === "cancelled") + .filter(isFailingCheck) .map((check) => boundedField(check.description ? `${check.name}: ${check.description}` : check.name), ); @@ -961,11 +968,45 @@ export function buildFixFindingsHandoff(input: { }; } +function isFailingCheck(check: PullRequestCheck): boolean { + return check.status === "failure" || check.status === "cancelled"; +} + +function checkListItem(check: PullRequestCheck): string { + const name = boundedField(check.description ? `${check.name}: ${check.description}` : check.name); + return check.url ? `- ${name} (${boundedField(check.url)})` : `- ${name}`; +} + +function checksContextComment( + pullRequest: { readonly number: number; readonly url: string }, + input: { + readonly id: string; + readonly label: string; + readonly status: string; + readonly text: ReadonlyArray; + }, +): ReviewCommentContext { + const { shortLabel } = terminologyForUrl(pullRequest.url); + return { + id: input.id, + sectionId: `${PULL_REQUEST_CHECKS_SECTION_PREFIX}${pullRequest.number}`, + sectionTitle: `${shortLabel} #${pullRequest.number} checks`, + filePath: input.label, + startIndex: 0, + endIndex: 0, + rangeLabel: input.status, + text: input.text.join("\n"), + diff: "", + }; +} + export type PullRequestFinding = | { readonly kind: "thread"; readonly thread: PullRequestReviewThread } | { readonly kind: "check"; readonly check: PullRequestCheck } | { readonly kind: "comment"; readonly comment: PullRequestComment }; +export const PULL_REQUEST_FIX_CHECKS_KEY = "finding:checks"; + export function pullRequestFindingKey(finding: PullRequestFinding): string { switch (finding.kind) { case "thread": @@ -1010,12 +1051,53 @@ export function buildFixFindingHandoff(input: { } const check = input.finding.check; return { - prompt: [ - "Fix the failing check quoted below. Reproduce it locally first. The name is all the host reported, and the run may fail for a reason the code cannot show.", - ...preamble, - `> ${boundedField(check.description ? `${check.name}: ${check.description}` : check.name)}`, - ].join("\n"), - reviewComments: [], + prompt: "", + reviewComments: [ + checksContextComment(input, { + // Outside the handoff prefix, so a later handoff keeps the checks already added. + id: `pr-check:${input.url}:${check.name}:${check.url ?? ""}`, + label: boundedField(check.name), + status: check.status === "cancelled" ? "Cancelled" : "Failed", + text: [ + checkListItem(check), + "", + "Fix this failing check. Reproduce it locally first. The host may report little beyond its name, and the run may fail for a reason the code cannot show.", + ...preamble, + ], + }), + ], + additive: true, + }; +} + +export function buildFixChecksHandoff(input: { + readonly number: number; + readonly title: string; + readonly url: string; + readonly headBranch: string; + readonly baseBranch: string; + readonly checks: ReadonlyArray; +}): FixFindingsHandoff { + const term = terminologyForUrl(input.url).singular; + const failing = input.checks.filter(isFailingCheck); + const omitted = Math.max(0, failing.length - FINDING_LIMIT); + return { + prompt: "", + reviewComments: [ + checksContextComment(input, { + id: `pr-checks:${input.url}`, + label: `Failing checks on #${input.number}`, + status: `${failing.length} failing`, + text: [ + ...failing.slice(0, FINDING_LIMIT).map(checkListItem), + "", + ...(omitted > 0 ? [`${omitted} further failing checks were omitted.`] : []), + `Fix every failing check on this ${term}. Fetch the current check results first, since more may have failed after this was attached. Reproduce each failure locally, and note that a run may fail for a reason the code cannot show.`, + ...handoffPreamble(input), + ], + }), + ], + additive: true, }; } diff --git a/apps/web/src/components/pullRequest/usePullRequestActions.ts b/apps/web/src/components/pullRequest/usePullRequestActions.ts index 82ce86ab8f429..49ee6e11e9516 100644 --- a/apps/web/src/components/pullRequest/usePullRequestActions.ts +++ b/apps/web/src/components/pullRequest/usePullRequestActions.ts @@ -239,6 +239,7 @@ export function usePullRequestCloseBatch(onClosed: (entry: EnvironmentPullReques export interface PullRequestThreadTask { prompt: string; reviewComments?: ReadonlyArray; + additive?: boolean; } export type PullRequestHandoffDetail = Pick< @@ -283,6 +284,12 @@ export function usePullRequestHandoffs({ if (session === null) return null; const store = useComposerDraftStore.getState(); if (task === null) return session; + if (task.additive === true) { + for (const comment of task.reviewComments ?? []) { + store.addReviewComment(session.draftId, comment); + } + return session; + } const draft = store.getComposerDraft(session.draftId); const existingComments = draft?.reviewComments ?? []; const prompt = handoffPrompt( diff --git a/apps/web/src/lib/composerContextRecords.ts b/apps/web/src/lib/composerContextRecords.ts index 2779950ed22a0..6d5cee96f9c5e 100644 --- a/apps/web/src/lib/composerContextRecords.ts +++ b/apps/web/src/lib/composerContextRecords.ts @@ -66,7 +66,15 @@ function basename(filePath: string): string { return filePath.split(/[\\/]/).at(-1) ?? filePath; } +export const PULL_REQUEST_CHECKS_SECTION_PREFIX = "pull-request-checks:"; + +export function isPullRequestCheckContext(comment: ReviewCommentPresentation): boolean { + return comment.sectionId.startsWith(PULL_REQUEST_CHECKS_SECTION_PREFIX); +} + export function reviewCommentContextLabel(comment: ReviewCommentPresentation): string { + // Check names often contain slashes, so they are not paths to shorten. + if (isPullRequestCheckContext(comment)) return comment.filePath; const pullRequestNumber = pullRequestContextNumber(comment); if (isPullRequestSummaryContext(comment) && pullRequestNumber !== null) { return `#${pullRequestNumber}`; From 12c4c3bc8402aed4144842a04b0fae53360c9371 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Thu, 5 Nov 2026 06:19:50 -0800 Subject: [PATCH 2/7] fix(web): keep bulk check context within the sent-message limit Twenty failing checks with long descriptions could exceed the 16,000 character limit, so sending clamped away the later checks and the instructions. Measure the instructions first, list only the checks that fit, and name the rest in a note. Co-Authored-By: Claude Opus 5.5 --- .../pullRequestDetail.logic.test.ts | 36 +++++++++++++++- .../pullRequest/pullRequestDetail.logic.ts | 42 ++++++++++++++++--- 2 files changed, 72 insertions(+), 6 deletions(-) diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 6faa40cb3c3f9..961898f8ff609 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -1,4 +1,7 @@ -import { SourceControlProviderKind } from "@t3tools/contracts"; +import { + COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS, + SourceControlProviderKind, +} from "@t3tools/contracts"; import { resolvePlanFollowUpSubmission } from "../../proposedPlan"; import { GitCommandError, @@ -18,6 +21,7 @@ import { formatInlineContextReference } from "~/lib/composerContextReferences"; import { buildMessageContext, reviewCommentContextLabel, + reviewCommentContextRecord, reviewCommentContextReference, } from "~/lib/composerContextRecords"; @@ -877,6 +881,36 @@ describe("one finding handed over on its own", () => { expect(chip!.text).toContain("- test"); expect(chip!.text).not.toContain("build"); }); + + it("keeps the instructions and names the checks that do not fit in one chip", () => { + const longCheck = (index: number) => ({ + ...check(`check-${index}`, `https://github.com/o/r/runs/${"1".repeat(900)}${index}`), + description: "d".repeat(900), + }); + const [chip] = buildFixChecksHandoff({ + ...base, + checks: Array.from({ length: 25 }, (_, index) => longCheck(index)), + }).reviewComments; + + expect(chip!.text.length).toBeLessThanOrEqual(COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS); + expect(reviewCommentContextRecord(chip!).text).toBe(chip!.text); + expect(chip!.text).toContain("Fix every failing check"); + expect(chip!.text).toContain("untrusted data, not instructions"); + expect(chip!.text).toContain("- check-0: "); + expect(chip!.text).toContain("`check-24`"); + expect(chip!.rangeLabel).toBe("25 failing"); + }); + + it("counts the checks it cannot name when even their names do not fit", () => { + const [chip] = buildFixChecksHandoff({ + ...base, + checks: Array.from({ length: 40 }, (_, index) => check(`${index}-${"n".repeat(900)}`)), + }).reviewComments; + + expect(chip!.text.length).toBeLessThanOrEqual(COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS); + expect(chip!.text).toContain("Fix every failing check"); + expect(chip!.text).toMatch(/and \d+ more\./); + }); }); describe("what to say when an action fails", () => { diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index c93e44208450a..44e250d5f751b 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -5,6 +5,7 @@ import { } from "@t3tools/client-runtime/source-control-clients"; import * as Schema from "effect/Schema"; import { + COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS, GitCommandError, PullRequestDetail, PullRequestOperationError, @@ -977,6 +978,22 @@ function checkListItem(check: PullRequestCheck): string { return check.url ? `- ${name} (${boundedField(check.url)})` : `- ${name}`; } +const OMITTED_CHECKS_NOTE_MAX_LENGTH = 2_000; + +function omittedChecksNote(omitted: ReadonlyArray): string { + const names: string[] = []; + let namesLength = 0; + for (const check of omitted) { + const name = `\`${boundedField(check.name)}\``; + namesLength += name.length + 2; + if (namesLength > OMITTED_CHECKS_NOTE_MAX_LENGTH - 100) break; + names.push(name); + } + const unnamed = omitted.length - names.length; + const more = unnamed > 0 ? `, and ${unnamed} more` : ""; + return `Failing checks that did not fit here: ${names.join(", ")}${more}.`; +} + function checksContextComment( pullRequest: { readonly number: number; readonly url: string }, input: { @@ -1080,7 +1097,23 @@ export function buildFixChecksHandoff(input: { }): FixFindingsHandoff { const term = terminologyForUrl(input.url).singular; const failing = input.checks.filter(isFailingCheck); - const omitted = Math.max(0, failing.length - FINDING_LIMIT); + const instructions = [ + `Fix every failing check on this ${term}. Fetch the current check results first, since more may have failed after this was attached. Reproduce each failure locally, and note that a run may fail for a reason the code cannot show.`, + ...handoffPreamble(input), + ]; + // Sending clamps the end of the text, so budget here to keep the instructions whole. + const listBudget = + COMPOSER_CONTEXT_REVIEW_TEXT_MAX_CHARS - + instructions.join("\n").length - + OMITTED_CHECKS_NOTE_MAX_LENGTH; + const listed: string[] = []; + let listLength = 0; + for (const item of failing.slice(0, FINDING_LIMIT).map(checkListItem)) { + listLength += item.length + 1; + if (listLength > listBudget) break; + listed.push(item); + } + const omitted = failing.slice(listed.length); return { prompt: "", reviewComments: [ @@ -1089,11 +1122,10 @@ export function buildFixChecksHandoff(input: { label: `Failing checks on #${input.number}`, status: `${failing.length} failing`, text: [ - ...failing.slice(0, FINDING_LIMIT).map(checkListItem), + ...listed, "", - ...(omitted > 0 ? [`${omitted} further failing checks were omitted.`] : []), - `Fix every failing check on this ${term}. Fetch the current check results first, since more may have failed after this was attached. Reproduce each failure locally, and note that a run may fail for a reason the code cannot show.`, - ...handoffPreamble(input), + ...(omitted.length > 0 ? [omittedChecksNote(omitted)] : []), + ...instructions, ], }), ], From 61b294f94673fcf45567b9c69481454a168225a9 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Thu, 5 Nov 2026 06:49:47 -0800 Subject: [PATCH 3/7] feat(web): fix all failing checks from the pull request summary row The failing count in the tab row had no way to act on it, so fixing meant opening the Checks section first. Add a Fix all button beside the count, sharing the Checks section's button and handoff. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestDetailPanel.tsx | 65 +++++++++++-------- .../pullRequest/PullRequestSummaryTab.tsx | 39 ++++++++--- 2 files changed, 69 insertions(+), 35 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 603a652742949..e5cbc2cbbd7d2 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -107,7 +107,7 @@ import { openOnHostLabel } from "./pullRequestLinkContextMenu"; import { PullRequestMarkdownContext } from "./PullRequestMarkdown"; import { pullRequestAutolinkPlugins } from "./pullRequestMarkdown.logic"; import { PullRequestComposer } from "./PullRequestComposer"; -import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; +import { PullRequestFixChecksButton, PullRequestSummaryTab } from "./PullRequestSummaryTab"; import { PullRequestTimelineTab } from "./PullRequestTimelineTab"; import { buildAddSelectionToAgentHandoff, @@ -118,6 +118,7 @@ import { buildFixFindingHandoff, buildFixFindingsHandoff, buildResolveConflictsPrompt, + classifyPullRequestChecks, handoffPrompt, handoffReviewComments, latestPullRequestReviewOutcomes, @@ -1606,31 +1607,43 @@ export function PullRequestDetailPanel({ }} > {tab === "summary" ? ( - showsApproveWorkflows ? ( - - ) : ( - - ) : ( - - ) - } - label={checksSummary} - /> - ) + <> + {showsApproveWorkflows ? ( + + ) : ( + + ) : ( + + ) + } + label={checksSummary} + /> + )} + {canFixFindings && + !checksStale && + classifyPullRequestChecks(detail.checks) === "failing" ? ( + + ) : null} + ) : tab === "timeline" ? (
void; +}) { + return ( + + ); +} + function Section({ title, defaultOpen = true, @@ -803,16 +829,11 @@ export function PullRequestSummaryTab({ defaultOpen={false} actions={ onFixChecks && !checksStale && classifyPullRequestChecks(detail.checks) === "failing" ? ( - + /> ) : undefined } > From 030a4802726fbdfa13679aea43571b558c8a406c Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Thu, 5 Nov 2026 06:52:25 -0800 Subject: [PATCH 4/7] refactor(web): one Fix action for all checks, in the summary row The summary row's button replaces the Checks section's Fix all, so the section keeps only its per-check actions. Both now just say Fix. Co-Authored-By: Claude Opus 5.5 --- .../pullRequest/PullRequestDetailPanel.tsx | 24 +++++---- .../pullRequest/PullRequestSummaryTab.tsx | 53 +------------------ .../pullRequest/pullRequestDetail.logic.ts | 4 -- 3 files changed, 15 insertions(+), 66 deletions(-) diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index e5cbc2cbbd7d2..61dcd67b4df1b 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -31,6 +31,7 @@ import { CircleDotIcon, FileDiffIcon, GitCommitHorizontalIcon, + HammerIcon, MessageSquareIcon, PanelRightIcon, } from "lucide-react"; @@ -107,7 +108,7 @@ import { openOnHostLabel } from "./pullRequestLinkContextMenu"; import { PullRequestMarkdownContext } from "./PullRequestMarkdown"; import { pullRequestAutolinkPlugins } from "./pullRequestMarkdown.logic"; import { PullRequestComposer } from "./PullRequestComposer"; -import { PullRequestFixChecksButton, PullRequestSummaryTab } from "./PullRequestSummaryTab"; +import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; import { PullRequestTimelineTab } from "./PullRequestTimelineTab"; import { buildAddSelectionToAgentHandoff, @@ -1636,12 +1637,17 @@ export function PullRequestDetailPanel({ {canFixFindings && !checksStale && classifyPullRequestChecks(detail.checks) === "failing" ? ( - + > + + {handoff === PULL_REQUEST_FIX_CHECKS_KEY ? "Preparing..." : "Fix"} + ) : null} ) : tab === "timeline" ? ( @@ -1784,11 +1790,7 @@ export function PullRequestDetailPanel({ activityError={activityError} pendingFinding={handoff} fixFindingLabel={handoffLabels.fixFinding} - fixCheckLabel={handoffLabels.fixCheck} - fixChecksLabel={handoffLabels.fixChecks} - {...(canFixFindings - ? { onFixFinding: startFixFinding, onFixChecks: startFixChecks } - : {})} + {...(canFixFindings ? { onFixFinding: startFixFinding } : {})} onRefresh={refreshDetail} onRefreshChecks={refreshFromHost} /> diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index 05759c072b2e0..9ac0befe18e31 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -64,8 +64,6 @@ import { pagePullRequestConversationItems, pullRequestConversationItemComments, pullRequestConversationItems, - PULL_REQUEST_FIX_CHECKS_KEY, - classifyPullRequestChecks, pullRequestFindingKey, pullRequestReviewOutcome, visibleBody, @@ -309,32 +307,6 @@ function MetaRow({ ); } -export function PullRequestFixChecksButton({ - label, - accessibleLabel, - pendingFinding, - onClick, -}: { - label: string; - accessibleLabel?: string; - pendingFinding: string | null | undefined; - onClick: () => void; -}) { - return ( - - ); -} - function Section({ title, defaultOpen = true, @@ -415,10 +387,7 @@ export function PullRequestSummaryTab({ activityError, pendingFinding, fixFindingLabel = "Fix in a thread", - fixCheckLabel = "Fix", - fixChecksLabel = "Fix all", onFixFinding, - onFixChecks, onRefresh, onRefreshChecks = onRefresh, }: { @@ -432,10 +401,7 @@ export function PullRequestSummaryTab({ activityError: string | null; pendingFinding?: string | null; fixFindingLabel?: string; - fixCheckLabel?: string; - fixChecksLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; - onFixChecks?: () => void; onRefresh: () => void; onRefreshChecks?: () => void; }) { @@ -823,20 +789,7 @@ export function PullRequestSummaryTab({
-
- ) : undefined - } - > +
{checksStale ? (
Check details are out of date. @@ -883,9 +836,7 @@ export function PullRequestSummaryTab({ onClick={() => onFixFinding(finding)} > - {pendingFinding === pullRequestFindingKey(finding) - ? "Preparing..." - : fixCheckLabel} + {pendingFinding === pullRequestFindingKey(finding) ? "Preparing..." : "Fix"} ) : null}
diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 44e250d5f751b..baaf5de140cd4 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -246,14 +246,10 @@ export function pullRequestHandoffLabels(inThisThread: boolean) { return inThisThread ? { fixFinding: "Fix in this thread", - fixCheck: "Fix in this thread", - fixChecks: "Fix all in this thread", fixFindings: "Fix findings in this thread", } : { fixFinding: "Fix in a thread", - fixCheck: "Fix", - fixChecks: "Fix all", fixFindings: "Fix findings in a thread", }; } From 60cce602e9aec7266cfb6ebccdc2e8372e06b8ec Mon Sep 17 00:00:00 2001 From: maria-rcks Date: Fri, 6 Nov 2026 03:10:50 +0000 Subject: [PATCH 5/7] fix(web): check handoffs attach the pull request chip like other fixes --- apps/web/src/components/chat/ThreadDetailsPrRow.tsx | 2 ++ .../web/src/components/pullRequest/PullRequestDetailPanel.tsx | 2 ++ .../components/pullRequest/pullRequestDetail.logic.test.ts | 2 +- .../web/src/components/pullRequest/pullRequestDetail.logic.ts | 4 ++++ 4 files changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index a5c6402d203a0..dec635a5f5d3c 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -211,6 +211,8 @@ export function ThreadDetailsPrRow({ url: detail.url, headBranch: detail.headBranch, baseBranch: detail.baseBranch, + state: detail.state, + isDraft: detail.isDraft, checks: detail.checks, }), ); diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 32eb25c23c8d7..b9b453298b50c 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1090,6 +1090,8 @@ export function PullRequestDetailPanel({ url: detail.url, headBranch: detail.headBranch, baseBranch: detail.baseBranch, + state: detail.state, + isDraft: detail.isDraft, checks: detail.checks, }), ); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts index 5375034a3ae4b..90d30db49223f 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts @@ -933,7 +933,7 @@ describe("one finding handed over on its own", () => { const [chip] = handoff.reviewComments; expect(handoff.additive).toBe(true); - expect(handoff.reviewComments).toHaveLength(1); + expect(handoff.reviewComments).toHaveLength(2); expect(reviewCommentContextLabel(chip!)).toMatch(/^Failing checks on #\d+$/); expect(chip!.rangeLabel).toBe("2 failing"); expect(chip!.text).toContain("- lint (https://github.com/o/r/runs/1)"); diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index b77612b3584e1..309a7762ea930 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -1113,6 +1113,7 @@ export function buildFixFindingHandoff(input: { ...preamble, ], }), + pullRequestContextComment(input, []), ], additive: true, }; @@ -1124,6 +1125,8 @@ export function buildFixChecksHandoff(input: { readonly url: string; readonly headBranch: string; readonly baseBranch: string; + readonly state: PullRequestState; + readonly isDraft: boolean; readonly checks: ReadonlyArray; }): FixFindingsHandoff { const term = terminologyForUrl(input.url).singular; @@ -1159,6 +1162,7 @@ export function buildFixChecksHandoff(input: { ...instructions, ], }), + pullRequestContextComment(input, []), ], additive: true, }; From ec5beadbc4dc7affcee6e142f7c1c36cf19060a8 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Thu, 5 Nov 2026 19:14:55 -0800 Subject: [PATCH 6/7] feat(web): fix checks from the thread details panel in this thread Share one check row and Fix button across the Checks section and popovers. The row's Fix now adds to this thread's composer instead of opening a new thread, so it uses the hammer rather than the new-thread arrow. Co-Authored-By: Claude Opus 5.5 --- .../BranchToolbarBranchSelector.tsx | 1 + .../chat/ThreadDetailsPrRow.browser.test.tsx | 51 +++++- .../components/chat/ThreadDetailsPrRow.tsx | 68 +++++--- .../components/chat/ThreadDetailsPrRows.tsx | 12 +- .../pullRequest/PullRequestChecksPopover.tsx | 162 +++++++++++++----- .../pullRequest/PullRequestDetailPanel.tsx | 23 +-- .../pullRequest/PullRequestSummaryTab.tsx | 62 ++----- .../pullRequest/pullRequestDetail.logic.ts | 2 +- 8 files changed, 258 insertions(+), 123 deletions(-) diff --git a/apps/web/src/components/BranchToolbarBranchSelector.tsx b/apps/web/src/components/BranchToolbarBranchSelector.tsx index 9782f258afe19..681314396447c 100644 --- a/apps/web/src/components/BranchToolbarBranchSelector.tsx +++ b/apps/web/src/components/BranchToolbarBranchSelector.tsx @@ -738,6 +738,7 @@ export function BranchToolbarBranchSelector({ {displayMode === "panel" && prNumber !== undefined && prUrl !== undefined ? ( { label="Test PR" openAriaLabel="Open PR" onOpen={vi.fn()} + composerTarget={DraftId.make("draft")} />, ); }); @@ -136,6 +137,53 @@ it("merges with the last selected merge method", async () => { expect(state.perform).toHaveBeenCalledWith("merge", "squash"); }); +it("adds fixes from the row and checks popover to this thread's composer without a new thread", async () => { + await using screen = await setup(); + state.status = "failure"; + const composerTarget = { + environmentId: EnvironmentId.make("environment"), + threadId: ThreadId.make("thread"), + }; + await act( + async () => + await screen.rerender( + , + ), + ); + await act(async () => + [...screen.container.querySelectorAll("button")] + .find((button) => button.textContent?.trim() === "Fix")! + .click(), + ); + await act(async () => + screen.container.querySelector('[aria-label^="Open checks"]')!.click(), + ); + await act(async () => + document.querySelector('[aria-label="Fix CI-failure"]')!.click(), + ); + + const comments = useComposerDraftStore + .getState() + .getComposerDraft(composerTarget)?.reviewComments; + expect(comments?.map((comment) => comment.filePath)).toEqual([ + "Failing checks on #1", + "CI-failure", + "PR #1", + ]); + expect(state.newThread).not.toHaveBeenCalled(); +}); + it("requires a new merge click after passing checks become pending and pass again", async () => { await using screen = await setup(); const { container } = screen; @@ -149,6 +197,7 @@ it("requires a new merge click after passing checks become pending and pass agai label="Test PR" openAriaLabel="Open PR" onOpen={vi.fn()} + composerTarget={DraftId.make("draft")} /> ); const clickMerge = async () => diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index dec635a5f5d3c..46588cd1d9670 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -8,23 +8,32 @@ import { ThreadDetailsControl } from "./ThreadDetailsControl"; import { PullRequestGlyph } from "../pullRequest/pullRequestIcons"; import { selectEnvironmentQueryData } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentProject } from "@t3tools/client-runtime/state/shell"; -import type { EnvironmentId, ProjectId, PullRequestRef } from "@t3tools/contracts"; +import type { + EnvironmentId, + ProjectId, + PullRequestCheck, + PullRequestRef, + ScopedThreadRef, +} from "@t3tools/contracts"; import { ArrowUpRightIcon, EyeIcon, EyeOffIcon, FileDiffIcon, GitBranchIcon, + HammerIcon, TriangleAlertIcon, } from "lucide-react"; import { AsyncResult } from "effect/reactivity"; import { useState, type MouseEvent as ReactMouseEvent } from "react"; +import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; import { usePullRequestChecksRefresh } from "~/hooks/usePullRequestChecksRefresh"; import { cn } from "~/lib/utils"; import { useServerConfigs } from "~/state/entities"; import { pullRequestEnvironment } from "~/state/pullRequests"; +import type { ReviewCommentContext } from "~/reviewCommentContext"; import { useUiStateStore } from "~/uiStateStore"; import { useEnvironmentQueryIsNewer, @@ -34,6 +43,7 @@ import { import { buildFixChecksHandoff, + buildFixFindingHandoff, buildResolveConflictsPrompt, classifyPullRequestChecks, describePullRequestChecks, @@ -41,6 +51,7 @@ import { resolvePullRequestMergeMethod, allowedPullRequestMergeMethods, resolveThreadPanelPullRequestAction, + type PullRequestFinding, } from "../pullRequest/pullRequestDetail.logic"; import { PullRequestChecksPopover } from "../pullRequest/PullRequestChecksPopover"; import { @@ -69,6 +80,7 @@ import { AlertDialogTitle, } from "../ui/alert-dialog"; import { Button } from "../ui/button"; +import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { THREAD_DETAILS_PANEL_ICON_CLASS, @@ -93,6 +105,8 @@ export function ThreadDetailsPrRow({ onOpen, onActed, onStopWatching, + composerTarget, + threadRef = null, }: { environmentId: EnvironmentId; pr: ThreadPr; @@ -105,6 +119,8 @@ export function ThreadDetailsPrRow({ onOpen: (event: ReactMouseEvent) => void; onActed?: () => void; onStopWatching?: (() => void) | undefined; + composerTarget: ScopedThreadRef | DraftId; + threadRef?: ScopedThreadRef | null; }) { const serverConfigs = useServerConfigs(); const supportsPullRequests = serverConfigs.get(environmentId) !== undefined; @@ -201,22 +217,30 @@ export function ThreadDetailsPrRow({ }); }; - const startFixChecks = () => { + const addToComposer = (comments: ReadonlyArray) => { + const store = useComposerDraftStore.getState(); + for (const comment of comments) store.addReviewComment(composerTarget, comment); + toastManager.add({ + type: "success", + title: "Added to the composer", + description: "The task is in the composer. Read it over, then send.", + }); + }; + const fixAllChecks = () => { if (detail === null) return; - void startHandoff( - "findings", - buildFixChecksHandoff({ - number: detail.number, - title: detail.title, - url: detail.url, - headBranch: detail.headBranch, - baseBranch: detail.baseBranch, - state: detail.state, - isDraft: detail.isDraft, - checks: detail.checks, - }), - ); + addToComposer(buildFixChecksHandoff(detail).reviewComments); }; + const checksFix = + detail !== null && open + ? { + onFixCheck: (check: PullRequestCheck) => { + const finding: PullRequestFinding = { kind: "check", check }; + addToComposer(buildFixFindingHandoff({ ...detail, finding }).reviewComments); + }, + onFixAll: fixAllChecks, + pending: null, + } + : undefined; const statePresentation = detail === null @@ -339,12 +363,13 @@ export function ThreadDetailsPrRow({ : rowAction === "fix" ? { label: "Fix", - pendingLabel: "Preparing...", - pending: handoff === "findings", + pendingLabel: "Fix", + pending: false, destructive: true, - suffix: , - tooltip: "Fix the failing checks in a new thread", - onClick: startFixChecks, + icon: , + suffix: null, + tooltip: "Add the failing checks to the composer", + onClick: fixAllChecks, } : rowAction === "merge" ? { @@ -442,6 +467,8 @@ export function ThreadDetailsPrRow({ } /> @@ -463,6 +490,7 @@ export function ThreadDetailsPrRow({ /> } > + {trailingAction.icon} {trailingAction.pending ? trailingAction.pendingLabel : trailingAction.label} {trailingAction.suffix} diff --git a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx index 2da6019787f5a..0823ca35aadea 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRows.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRows.tsx @@ -28,12 +28,16 @@ function ThreadDetailsPrLinkRow({ onOpen, onActed, onStopWatching, + composerTarget, + threadRef, }: { environmentId: EnvironmentId; link: ThreadPullRequestLink; onOpen: (event: ReactMouseEvent) => void; onActed?: (() => void) | undefined; onStopWatching?: (() => void) | undefined; + composerTarget: ComponentProps["composerTarget"]; + threadRef: ScopedThreadRef; }) { const projects = useProjects(); const parsed = sourceControlClients.parseChangeRequestUrl(link.url)?.link; @@ -58,6 +62,8 @@ function ThreadDetailsPrLinkRow({ openAriaLabel={link.url} onOpen={onOpen} onStopWatching={onStopWatching} + composerTarget={composerTarget} + threadRef={threadRef} {...(onActed ? { onActed } : {})} /> ); @@ -91,7 +97,9 @@ export function ThreadDetailsPrRows({ watching: false, }, }); - const currentRow = ; + const currentRow = ( + + ); const rest = currentLink === null ? [] @@ -113,6 +121,8 @@ export function ThreadDetailsPrRows({ onOpen={(event) => onOpenLink(event, link.url)} onActed={row.onActed} onStopWatching={stopWatching(link)} + composerTarget={row.composerTarget} + threadRef={threadRef} /> ))}
diff --git a/apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx b/apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx index cfcea204a56ee..cf13030deb852 100644 --- a/apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx +++ b/apps/web/src/components/pullRequest/PullRequestChecksPopover.tsx @@ -6,6 +6,7 @@ import type { PullRequestRef, ScopedThreadRef, } from "@t3tools/contracts"; +import { HammerIcon } from "lucide-react"; import { useState } from "react"; import { useOpenLink } from "~/browser/useOpenLink"; @@ -18,7 +19,12 @@ import { Button } from "../ui/button"; import { ScrollArea } from "../ui/scroll-area"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { toastManager } from "../ui/toast"; -import { groupPullRequestChecks } from "./pullRequestDetail.logic"; +import { + groupPullRequestChecks, + isFailingCheck, + PULL_REQUEST_FIX_CHECKS_KEY, + pullRequestFindingKey, +} from "./pullRequestDetail.logic"; import { PullRequestCheckStatusIcon, pullRequestCheckStatusLabel, @@ -52,62 +58,126 @@ function LazyChecksBody({ return ; } +export function useOpenPullRequestCheck(threadRef: ScopedThreadRef | null) { + const openLink = useOpenLink(threadRef); + return (url: string) => { + void openLink(url).catch((error: unknown) => { + console.error(error); + toastManager.add({ type: "error", title: "Unable to open check details" }); + }); + }; +} + +export function PullRequestFixButton({ + pending, + disabled, + onClick, + "aria-label": ariaLabel, +}: { + pending: boolean; + disabled: boolean; + onClick: () => void; + "aria-label": string; +}) { + return ( + + ); +} + +export function PullRequestCheckRow({ + check, + onOpen, + onFix, + pendingFix, +}: { + check: PullRequestCheck; + onOpen: (url: string) => void; + onFix?: ((check: PullRequestCheck) => void) | undefined; + pendingFix?: string | null | undefined; +}) { + const name = {check.name}; + return ( +
+ + {onFix && isFailingCheck(check) ? ( + onFix(check)} + /> + ) : null} +
+ ); +} + +export interface PullRequestChecksFix { + readonly onFixCheck: (check: PullRequestCheck) => void; + readonly onFixAll: () => void; + readonly pending: string | null; +} + function ChecksBody({ checks, threadRef, + fix, }: { checks: ReadonlyArray; threadRef: ScopedThreadRef | null; + fix?: PullRequestChecksFix | undefined; }) { - const openLink = useOpenLink(threadRef); + const openCheck = useOpenPullRequestCheck(threadRef); const [showAll, setShowAll] = useState(false); const { attention, running, completed } = groupPullRequestChecks(checks); const canCollapse = attention.length + running.length > 0 && completed.length > 0; const visibleChecks = [...attention, ...running, ...(showAll || !canCollapse ? completed : [])]; - const hasDetails = visibleChecks.some((check) => check.url !== null); if (checks.length === 0) { return

No checks reported

; } return ( <> - -
    + +
      {visibleChecks.map( (check, index) => ( /* oxlint-disable react/no-array-index-key -- Check names can repeat; these status rows have no local state. */
    • - - - {check.name}} /> - {check.description ?? check.name} - - - {pullRequestCheckStatusLabel(check)} - - {check.url === null ? null : ( - - )} +
    • ) /* oxlint-enable react/no-array-index-key */, )} @@ -134,6 +204,7 @@ export function PullRequestChecksPopover({ environmentId, reference, threadRef = null, + fix, variant = "icon", className, render, @@ -144,6 +215,7 @@ export function PullRequestChecksPopover({ environmentId?: EnvironmentId; reference?: PullRequestRef; threadRef?: ScopedThreadRef | null; + fix?: PullRequestChecksFix | undefined; variant?: "icon" | "count"; className?: string; render?: React.ReactElement; @@ -192,14 +264,26 @@ export function PullRequestChecksPopover({ ) : null} -

      {presentation.label}

      - {summary === null ? null :

      {summary}

      } +
      +
      +

      {presentation.label}

      + {summary === null ? null :

      {summary}

      } +
      + {fix && !stale && failedCount > 0 ? ( + + ) : null} +
      {stale ? (

      Check details are out of date. Refresh the pull request to update them.

      ) : checks !== undefined ? ( - + ) : environmentId !== undefined && reference !== undefined ? ( startFixFinding({ kind: "check", check }), + onFixAll: startFixChecks, + pending: handoff, + } + : undefined + } /> ) : ( @@ -1641,17 +1649,12 @@ export function PullRequestDetailPanel({ {canFixFindings && !checksStale && classifyPullRequestChecks(detail.checks) === "failing" ? ( - + /> ) : null} ) : tab === "timeline" ? ( diff --git a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx index eae16b81131b3..8c7b709d89b44 100644 --- a/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestSummaryTab.tsx @@ -28,7 +28,6 @@ import { useRef, useState, type ReactNode } from "react"; import { pullRequestEnvironment } from "~/state/pullRequests"; import { useLocalStorage } from "~/hooks/useLocalStorage"; import { cn } from "~/lib/utils"; -import { useOpenLink } from "~/browser/useOpenLink"; import { RelativeTime } from "~/components/RelativeTime"; import { Button } from "../ui/button"; @@ -43,11 +42,10 @@ import { MenuTrigger, } from "../ui/menu"; import { toastManager } from "../ui/toast"; +import { PullRequestCheckRow, useOpenPullRequestCheck } from "./PullRequestChecksPopover"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { PullRequestActorLabel, - PullRequestCheckStatusIcon, - pullRequestCheckStatusLabel, PullRequestLabelChip, PullRequestReviewOutcomeBadge, pullRequestReviewOutcomeLabel, @@ -523,13 +521,7 @@ export function PullRequestSummaryTab({ })), ]; - const openLink = useOpenLink(threadRef); - const openCheck = (url: string) => { - void openLink(url).catch((error: unknown) => { - console.error(error); - toastManager.add({ type: "error", title: "Unable to open check details" }); - }); - }; + const openCheck = useOpenPullRequestCheck(threadRef); const update = useAtomCommand(pullRequestEnvironment.update, { reportFailure: false }); const updateComment = useAtomCommand(pullRequestEnvironment.updateComment, { @@ -885,47 +877,15 @@ export function PullRequestSummaryTab({

      No checks reported.

      ) : ( detail.checks.map( - ( - check, - index, - ) => /* oxlint-disable react/no-array-index-key -- Check names and URLs can repeat; these status rows have no local state. */ { - const finding = { kind: "check", check } as const; - const failing = check.status === "failure" || check.status === "cancelled"; - return ( -
      - - {onFixFinding && failing ? ( - - ) : null} -
      - ); - } /* oxlint-enable react/no-array-index-key */, + (check, index) => ( + /* oxlint-disable react/no-array-index-key -- Check names and URLs can repeat; these status rows have no local state. */ onFixFinding({ kind: "check", check: value }))} + pendingFix={pendingFinding} + /> + ) /* oxlint-enable react/no-array-index-key */, ) )} diff --git a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts index 309a7762ea930..d4c721ee109e4 100644 --- a/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts +++ b/apps/web/src/components/pullRequest/pullRequestDetail.logic.ts @@ -995,7 +995,7 @@ export function buildFixFindingsHandoff(input: { }; } -function isFailingCheck(check: PullRequestCheck): boolean { +export function isFailingCheck(check: PullRequestCheck): boolean { return check.status === "failure" || check.status === "cancelled"; } From dbd12d17a2dc4f6a54dff2b7d629ffacc96372b4 Mon Sep 17 00:00:00 2001 From: wSedlacek Date: Thu, 5 Nov 2026 19:25:08 -0800 Subject: [PATCH 7/7] fix(web): check fixes append to the draft, not a question's answer While a provider question is active, the composer caret edits the answer, so check references landed there and never reached the draft. Additive handoffs now skip caret insertion, and the thread details row uses the same path. Co-Authored-By: Claude Opus 5.5 --- .../chat/ThreadDetailsPrRow.browser.test.tsx | 12 +++++++++--- .../web/src/components/chat/ThreadDetailsPrRow.tsx | 14 +++++++------- .../pullRequest/usePullRequestActions.ts | 5 ++++- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx index 2b9ce8b7e147d..1b42da8688b91 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.browser.test.tsx @@ -161,6 +161,11 @@ it("adds fixes from the row and checks popover to this thread's composer without />, ), ); + const insertAtCaret = vi.fn(() => true); + const removeHandler = useComposerDraftStore + .getState() + .setContextInsertionHandler(composerTarget, insertAtCaret); + using _handler = { [Symbol.dispose]: () => removeHandler?.() }; await act(async () => [...screen.container.querySelectorAll("button")] .find((button) => button.textContent?.trim() === "Fix")! @@ -173,14 +178,15 @@ it("adds fixes from the row and checks popover to this thread's composer without document.querySelector('[aria-label="Fix CI-failure"]')!.click(), ); - const comments = useComposerDraftStore - .getState() - .getComposerDraft(composerTarget)?.reviewComments; + const draft = useComposerDraftStore.getState().getComposerDraft(composerTarget); + const comments = draft?.reviewComments; expect(comments?.map((comment) => comment.filePath)).toEqual([ "Failing checks on #1", "CI-failure", "PR #1", ]); + expect(insertAtCaret).not.toHaveBeenCalled(); + expect(draft?.prompt).toContain("Failing checks on #1"); expect(state.newThread).not.toHaveBeenCalled(); }); diff --git a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx index 46588cd1d9670..a99d5f897125c 100644 --- a/apps/web/src/components/chat/ThreadDetailsPrRow.tsx +++ b/apps/web/src/components/chat/ThreadDetailsPrRow.tsx @@ -27,13 +27,12 @@ import { import { AsyncResult } from "effect/reactivity"; import { useState, type MouseEvent as ReactMouseEvent } from "react"; -import { type DraftId, useComposerDraftStore } from "~/composerDraftStore"; +import type { DraftId } from "~/composerDraftStore"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; import { usePullRequestChecksRefresh } from "~/hooks/usePullRequestChecksRefresh"; import { cn } from "~/lib/utils"; import { useServerConfigs } from "~/state/entities"; import { pullRequestEnvironment } from "~/state/pullRequests"; -import type { ReviewCommentContext } from "~/reviewCommentContext"; import { useUiStateStore } from "~/uiStateStore"; import { useEnvironmentQueryIsNewer, @@ -64,6 +63,8 @@ import { usePullRequestActionRunner, usePullRequestDefaultMergeMethodResolver, usePullRequestHandoffs, + writeHandoffToComposer, + type PullRequestThreadTask, } from "../pullRequest/usePullRequestActions"; import { ChangeRequestStatusIcon, @@ -217,9 +218,8 @@ export function ThreadDetailsPrRow({ }); }; - const addToComposer = (comments: ReadonlyArray) => { - const store = useComposerDraftStore.getState(); - for (const comment of comments) store.addReviewComment(composerTarget, comment); + const addToComposer = (task: PullRequestThreadTask) => { + writeHandoffToComposer(composerTarget, task); toastManager.add({ type: "success", title: "Added to the composer", @@ -228,14 +228,14 @@ export function ThreadDetailsPrRow({ }; const fixAllChecks = () => { if (detail === null) return; - addToComposer(buildFixChecksHandoff(detail).reviewComments); + addToComposer(buildFixChecksHandoff(detail)); }; const checksFix = detail !== null && open ? { onFixCheck: (check: PullRequestCheck) => { const finding: PullRequestFinding = { kind: "check", check }; - addToComposer(buildFixFindingHandoff({ ...detail, finding }).reviewComments); + addToComposer(buildFixFindingHandoff({ ...detail, finding })); }, onFixAll: fixAllChecks, pending: null, diff --git a/apps/web/src/components/pullRequest/usePullRequestActions.ts b/apps/web/src/components/pullRequest/usePullRequestActions.ts index 90f0d760e94ea..db75fb3ba8737 100644 --- a/apps/web/src/components/pullRequest/usePullRequestActions.ts +++ b/apps/web/src/components/pullRequest/usePullRequestActions.ts @@ -251,7 +251,10 @@ export function writeHandoffToComposer( ) { const store = useComposerDraftStore.getState(); if (task.additive === true) { - for (const comment of task.reviewComments ?? []) store.addReviewComment(target, comment); + // The caret may be in a provider question's answer, which is not this draft. + for (const comment of task.reviewComments ?? []) { + store.addReviewComment(target, comment, { insertAtCaret: false }); + } return; } const draft = store.getComposerDraft(target);