diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 6377a142b8a..30b9f5a02bf 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -375,6 +375,20 @@ Before a heartbeat finalizes, its issue disposition must therefore be evaluated If useful deliverable work can continue without the external result, the agent should continue that work or delegate it rather than parking the issue. Use `blocked` only for a real dependency that prevents productive progress. Use a monitor when the assignee owns a bounded future check, and use delegated child work when another owner can make progress independently. +Saved pull-request work products remain linked in task properties even when +external-object detection or provider access is unavailable. Live updates read +saved rows independently of GitHub refreshes. A delayed provider response only +enriches matching PR versions; it cannot replace the saved work-product list. +During a scheduled +GitHub monitor wait, an outstanding `needs_board_review` PR also appears beside +the composer with its link and the next check time. **Check status** invokes the +existing bounded monitor check; it does not merge the PR or attest that it merged. +Merged, closed, and archived PRs do not request review even when their saved review +flag is stale. This display is not an approval gate or a new execution path. +When work actually needs a human answer, the agent must still create the existing +durable interaction and leave the task `in_review` rather than relying on a PR +review flag or a monitor comment to request that answer. + Recovery from an invalid external wait is bounded and idempotent: 1. Record bounded evidence that the completed heartbeat left no durable action path, including the terminal run and any reported local watcher metadata without treating that metadata as liveness. diff --git a/server/src/__tests__/work-products.test.ts b/server/src/__tests__/work-products.test.ts index c7f80f53942..476e1156184 100644 --- a/server/src/__tests__/work-products.test.ts +++ b/server/src/__tests__/work-products.test.ts @@ -110,6 +110,27 @@ describe("workProductService", () => { expect(product.metadata.state).toBe("open"); }); + it.each([ + { url: "https://github.com/example/private-repo/pull/42" }, + { repository: "example/private-repo", number: 42 }, + ])("refreshes a PR whose identity is stored only in metadata: %j", async (metadata) => { + const product = createWorkProductRow({ url: null, metadata }) as any; + const resolve = vi.fn(async () => ({ state: "merged" as const, workProductState: "merged" as const, headRef: null, headSha: null })); + const [refreshed] = await refreshPullRequestWorkProductMetadata([product], resolve); + expect(resolve).toHaveBeenCalledWith("company-1", { + host: "github.com", owner: "example", repo: "private-repo", number: 42, + }); + expect(refreshed.metadata).toMatchObject({ state: "merged" }); + expect(product.metadata).toEqual(metadata); + }); + + it("retains the saved PR when the provider cannot access it", async () => { + const product = createWorkProductRow({ url: null, metadata: { url: "https://github.com/example/private-repo/pull/42" } }) as any; + const resolve = vi.fn(async () => { throw new Error("Not found"); }); + const [refreshed] = await refreshPullRequestWorkProductMetadata([product], resolve); + expect(refreshed).toBe(product); + }); + it("resolves GitHub commit stats when runner diff events are unavailable", async () => { const resolveCommitDetails = vi.fn(async () => ({ additions: 13, deletions: 2, changedFiles: 3 })); const svc = workProductService({} as any, { resolveCommitDetails }); diff --git a/server/src/services/work-products.ts b/server/src/services/work-products.ts index 6e6c40a1a9e..0f3dc35ae2e 100644 --- a/server/src/services/work-products.ts +++ b/server/src/services/work-products.ts @@ -92,10 +92,12 @@ export async function refreshPullRequestWorkProductMetadata( return await Promise.all(products.map(async (product) => { if (product.type !== "pull_request") return product; const metadata = product.metadata ?? {}; - const repo = typeof metadata.repo === "string" ? metadata.repo : null; + const repo = typeof metadata.repo === "string" ? metadata.repo + : typeof metadata.repository === "string" ? metadata.repository : null; const number = nonNegativeInteger(metadata.number); const references = extractGitHubPullRequestReferences([ product.url, + typeof metadata.url === "string" ? metadata.url : null, repo && number ? `${repo}#${number}` : null, ]); const reference = references[0]; diff --git a/skills/paperclip/SKILL.md b/skills/paperclip/SKILL.md index 9b887c5d8bc..c7a3cc11e48 100644 --- a/skills/paperclip/SKILL.md +++ b/skills/paperclip/SKILL.md @@ -759,3 +759,21 @@ improvement. This is not a mandatory report. Their shared helper is `scripts/submit-agent-commentary.mjs`, relative to this skill directory. Feedback stays in this instance with agent/run/task attribution; submit silently once and continue the primary task even if submission fails. + +**External review handoffs.** + +For a PR, put its link in the work product's top-level `url` field. If a human +must review or merge it before you can continue, name that action in a durable +human-only interaction with an appropriate continuation policy and leave the +task `in_review`. A `needs_board_review` work-product flag and a periodic monitor +do not create an interaction card. Keep any merge check bounded, record its +purpose in the monitor's `notes`, and verify the actual provider state when you +resume; a confirmation response is not proof of a merge. Update the existing PR +work product when the PR merges or closes instead of registering a duplicate. + +The same rule applies to external release approval gates: link the exact run, +create a human-only confirmation asking whether the user approved it in the +provider, and keep the agent assigned with `continuationPolicy: "wake_assignee"` +so the answer resumes verification. A handoff comment asking the user to comment +back or reassign the task is not a confirmation card. The card records the user's +answer; verify the provider's gate and publish result before continuing. diff --git a/ui/src/api/issues.ts b/ui/src/api/issues.ts index 129961dbb5b..4f1b6d907d5 100644 --- a/ui/src/api/issues.ts +++ b/ui/src/api/issues.ts @@ -36,7 +36,7 @@ import type { UpsertIssueWatchdog, UpsertIssueDocument, } from "@paperclipai/shared"; -import { api, ApiError, type RequestOptions } from "./client"; +import { api, ApiError, detachInflightGet, type RequestOptions } from "./client"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; function hasCommentReceipt(value: unknown): boolean { @@ -596,10 +596,12 @@ export const issuesApi = { api.post(`/issues/${id}/approvals`, { approvalId }), unlinkApproval: (id: string, approvalId: string) => api.delete<{ ok: true }>(`/issues/${id}/approvals/${approvalId}`), - listWorkProducts: (id: string, options?: { refreshPullRequests?: boolean }) => - api.get( - `/issues/${id}/work-products${options?.refreshPullRequests ? "?refreshPullRequests=true" : ""}`, - ), + listWorkProducts: (id: string, options?: { refreshPullRequests?: boolean; signal?: AbortSignal; fresh?: boolean }) => { + const path = `/issues/${id}/work-products${options?.refreshPullRequests ? "?refreshPullRequests=true" : ""}`; + // Query invalidations must not rejoin a request that read rows before the write. + if (options?.fresh) detachInflightGet(path); + return api.get(path, { signal: options?.signal }); + }, ensureWorkProductReviewDocument: (id: string, workProductId: string) => api.post( `/issues/${id}/work-products/${workProductId}/review-document`, diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 100be024fd8..07598d50a2f 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -2,7 +2,7 @@ import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; -import type { Issue } from "@paperclipai/shared"; +import type { Issue, IssueWorkProduct } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -145,6 +145,37 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { } as unknown as Issue; } + it("shows the saved PR and a status check above the composer during a GitHub review wait", () => { + const issue = issueWithMonitor(new Date(NOW.getTime() + 6 * 60 * 60_000).toISOString()); + issue.executionState!.monitor!.serviceName = "github"; + const product = { + id: "pr-1", type: "pull_request", title: "Update runtime probe", url: null, + metadata: { url: "https://github.com/example/private-repo/pull/42" }, + status: "ready_for_review", reviewState: "needs_board_review", updatedAt: NOW, + } as unknown as IssueWorkProduct; + const onCheckNow = vi.fn(); + const root = createRoot(container); + const render = (products: IssueWorkProduct[], error?: string) => flushSync(() => root.render( + , + )); + render([product]); + expect(container.textContent).toContain("Pull request review requested"); + expect(container.textContent).toContain("Next check in 6h"); + expect(container.textContent).not.toContain("Resumes in"); + expect(container.querySelector("a")?.href).toBe("https://github.com/example/private-repo/pull/42"); + expect(container.querySelector("a")?.textContent).toBe("example/private-repo#42"); + const button = container.querySelector("button")!; + expect(button.textContent).toBe("Check status"); + flushSync(() => button.click()); + expect(onCheckNow).toHaveBeenCalledTimes(1); + render([product], "GitHub status check could not start"); + expect(container.querySelector('[role="alert"]')?.textContent).toContain("could not start"); + render([{ ...product, metadata: { ...product.metadata, state: "merged" } }]); + expect(container.textContent).not.toContain("review requested"); + expect(container.querySelector("a")).toBeNull(); + flushSync(() => root.unmount()); + }); + it("explains automatic workspace waiting without promising that a reply bypasses the lock", () => { const issue = { status: "todo", scheduledRetry: { status: "scheduled_retry", scheduledRetryReason: "workspace_busy", scheduledRetryAt: NOW.toISOString() }, diff --git a/ui/src/components/IssueMonitorBanner.tsx b/ui/src/components/IssueMonitorBanner.tsx index b46223ae87c..035eca29b84 100644 --- a/ui/src/components/IssueMonitorBanner.tsx +++ b/ui/src/components/IssueMonitorBanner.tsx @@ -1,9 +1,11 @@ import { useMemo } from "react"; import { Clock } from "lucide-react"; -import type { Issue } from "@paperclipai/shared"; +import type { Issue, IssueWorkProduct } from "@paperclipai/shared"; import { Button } from "@/components/ui/button"; import { InlineBanner } from "@/components/InlineBanner"; +import { IssuePullRequestLinks } from "@/components/IssuePullRequestLinks"; +import { getIssuePullRequests, pullRequestNeedsReview } from "@/lib/issue-pull-requests"; import { cn } from "@/lib/utils"; import { deriveMonitorState, @@ -50,6 +52,7 @@ export interface MonitorSurfaceCopy { /** `warning` (amber) once overdue, `info` (blue) while still on schedule. */ tone: "info" | "warning"; workspaceWait?: boolean; + pullRequestReview?: boolean; } function capitalize(value: string): string { @@ -65,6 +68,7 @@ export function buildMonitorSurfaceCopy( derived: DerivedMonitorState, now: MonitorDate, scheduledRetryReason?: string | null, + pendingPullRequestReviews = 0, ): MonitorSurfaceCopy | null { if (!isWaitingMonitorState(derived.state) || !derived.nextCheckAt) return null; @@ -81,6 +85,16 @@ export function buildMonitorSurfaceCopy( const eta = formatMonitorEta(derived.nextCheckAt, now); // "in 2h 12m" | "due now" | "overdue by 18m" const absolute = formatMonitorAbsolute(derived.nextCheckAt, {}, now); // local time, e.g. "Today, 4:08 PM" + if (derived.source === "monitor" && derived.serviceName?.toLowerCase() === "github" && pendingPullRequestReviews > 0) { + return { + bannerTitle: "Pull request review requested", + stripTitle: "Pull request review requested", + bannerMeta: [`Next check ${eta} · ${absolute} (your time)`], + stripMeta: [`Next check ${eta} · ${absolute}`], + tone: derived.state === "overdue" ? "warning" : "info", + pullRequestReview: true, + }; + } if (derived.source === "scheduled-retry" && scheduledRetryReason === "ai_connection_pool_wait") { return { bannerTitle: "Pool exhausted", @@ -134,21 +148,23 @@ export function buildMonitorSurfaceCopy( }; } -function useMonitorSurfaceCopy(issue: Issue): MonitorSurfaceCopy | null { +function useMonitorSurfaceCopy(issue: Issue, pendingPullRequestReviews: number): MonitorSurfaceCopy | null { // `nextCheckAt` is stable for a given issue; derive once to seed the ticking // countdown cadence, then re-derive against the live clock so the surfaces // roll scheduled → due → overdue on their own. const nextCheckAt = useMemo(() => deriveMonitorState(issue).nextCheckAt, [issue]); const now = useMonitorCountdown(nextCheckAt); - return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason), [issue, now]); + return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason, pendingPullRequestReviews), [issue, now, pendingPullRequestReviews]); } function CheckNowButton({ onCheckNow, checkingNow, + reviewRequested = false, }: { onCheckNow: () => void; checkingNow: boolean; + reviewRequested?: boolean; }) { return ( ); } @@ -168,6 +184,8 @@ export interface IssueMonitorSurfaceProps { issue: Issue; onCheckNow?: (() => void) | null; checkingNow?: boolean; + workProducts?: IssueWorkProduct[] | null; + checkError?: string | null; } /** @@ -179,8 +197,11 @@ export function IssueMonitorBanner({ issue, onCheckNow = null, checkingNow = false, + workProducts, + checkError, }: IssueMonitorSurfaceProps) { - const copy = useMonitorSurfaceCopy(issue); + const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview); + const copy = useMonitorSurfaceCopy(issue, reviews.length); if (!copy) return null; return ( @@ -189,9 +210,13 @@ export function IssueMonitorBanner({ icon={Clock} title={copy.bannerTitle} className="my-3" - actions={onCheckNow && !copy.workspaceWait ? : null} + actions={onCheckNow && !copy.workspaceWait ? : null} > - {copy.bannerMeta.join(" · ")} +
+ {copy.pullRequestReview ? : null} + {copy.bannerMeta.join(" · ")} + {checkError ?

{checkError}

: null} +
); } @@ -205,9 +230,12 @@ export function IssueMonitorComposerStrip({ issue, onCheckNow = null, checkingNow = false, + workProducts, + checkError, className, }: IssueMonitorSurfaceProps & { className?: string }) { - const copy = useMonitorSurfaceCopy(issue); + const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview); + const copy = useMonitorSurfaceCopy(issue, reviews.length); if (!copy) return null; return ( @@ -219,18 +247,22 @@ export function IssueMonitorComposerStrip({

- {copy.workspaceWait + {copy.pullRequestReview + ? "Review on GitHub, then check status to have the agent verify the result." + : copy.workspaceWait ? "You can keep sending instructions while the agent waits." : "Sending a reply wakes the agent now — before the scheduled check."}

+ {checkError ?

{checkError}

: null}
); } diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index 4a6b329a045..38a550a2217 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -465,6 +465,123 @@ function renderProperties(container: HTMLDivElement, props: ComponentProps { let container: HTMLDivElement; + it("surfaces saved PR review links even when external objects are unavailable", async () => { + mockIssuesApi.listWorkProducts.mockResolvedValue([{ + id: "pr-1", type: "pull_request", provider: "github", title: "Update runtime probe", + url: null, metadata: { url: "https://github.com/example/private-repo/pull/42" }, + status: "ready_for_review", reviewState: "needs_board_review", updatedAt: new Date(), + }]); + const root = renderProperties(container, { + issue: createIssue(), childIssues: [], onUpdate: vi.fn(), inline: true, + externalObjectsError: true, + }); + await waitForAssertion(() => { + const link = container.querySelector('a[href="https://github.com/example/private-repo/pull/42"]'); + expect(link?.textContent).toBe("example/private-repo#42"); + expect(container.textContent).toContain("Review requested"); + }); + act(() => root.unmount()); + }); + + it("refreshes PR state when opening Properties over an already loaded task", async () => { + const saved = { + id: "pr-1", type: "pull_request", provider: "github", title: "Update runtime probe", + url: "https://github.com/example/private-repo/pull/42", metadata: {}, + status: "ready_for_review", reviewState: "needs_board_review", updatedAt: new Date(), + }; + mockIssuesApi.listWorkProducts.mockImplementation(async (_id, options) => [ + options?.refreshPullRequests ? { ...saved, metadata: { state: "merged" } } : saved, + ]); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.issues.workProducts("issue-1"), [{ ...saved, metadata: { state: "merged" } }]); + const root = createRoot(container); + act(() => root.render( + + )); + await waitForAssertion(() => { + expect(mockIssuesApi.listWorkProducts).toHaveBeenCalledWith("issue-1", expect.objectContaining({ refreshPullRequests: true })); + expect(container.textContent).toContain("merged"); + expect(container.textContent).not.toContain("Review requested"); + }); + act(() => root.unmount()); + }); + + it("refreshes PRs on first panel open when the thread uses an identifier cache key", async () => { + const saved = { + id: "pr-1", type: "pull_request", title: "Update runtime probe", + url: "https://github.com/example/private-repo/pull/42", metadata: {}, + status: "ready_for_review", reviewState: "needs_board_review", updatedAt: new Date(), + }; + mockIssuesApi.listWorkProducts.mockImplementation(async (_id, options) => [ + options?.refreshPullRequests ? { ...saved, metadata: { state: "merged" } } : saved, + ]); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.issues.workProducts("PAP-1"), [{ ...saved, metadata: { state: "merged" } }]); + const root = createRoot(container); + act(() => root.render( + + )); + await waitForAssertion(() => { + expect(container.textContent).toContain("merged"); + expect(container.textContent).not.toContain("Review requested"); + }); + act(() => root.unmount()); + }); + + it("shows saved PRs while the first provider refresh is stalled", async () => { + const saved = { + id: "pr-1", type: "pull_request", title: "Update runtime probe", + url: "https://github.com/example/private-repo/pull/42", metadata: {}, + status: "ready_for_review", reviewState: "needs_board_review", updatedAt: new Date(), + }; + let finishRefresh!: (products: unknown[]) => void; + const refresh = new Promise((resolve) => { finishRefresh = resolve; }); + mockIssuesApi.listWorkProducts.mockImplementation(async (_id, options) => options?.refreshPullRequests ? refresh : [saved]); + const root = renderProperties(container, { issue: createIssue(), childIssues: [], onUpdate: vi.fn(), inline: true }); + await waitForAssertion(() => { + expect(mockIssuesApi.listWorkProducts).toHaveBeenCalledWith("issue-1", expect.objectContaining({ refreshPullRequests: true })); + expect(container.querySelector(`a[href="${saved.url}"]`)).not.toBeNull(); + expect(container.textContent).toContain("Review requested"); + }); + finishRefresh([{ ...saved, metadata: { state: "merged" } }]); + await waitForAssertion(() => { + expect(container.textContent).toContain("merged"); + expect(container.textContent).not.toContain("Review requested"); + }); + act(() => root.unmount()); + }); + + it.each([ + { statusLabel: "Open", statusCategory: "open", liveness: "stale", statusIconKey: "git-pull-request", review: true }, + { statusLabel: "Merged", statusCategory: "succeeded", liveness: "fresh", statusIconKey: "git-merge", review: false }, + { statusLabel: "Not found", statusCategory: "archived", liveness: "stale", statusIconKey: null, review: true }, + ] as const)("combines a saved PR with its $statusLabel provider status and freshness", async (provider) => { + const canonical = "https://github.com/example/private-repo/pull/42"; + const savedUrl = `${canonical}?diff=split#discussion_r123`; + mockIssuesApi.listWorkProducts.mockResolvedValue([{ + id: "pr-1", type: "pull_request", title: "Update runtime probe", url: savedUrl, + metadata: {}, status: "ready_for_review", reviewState: "needs_board_review", updatedAt: new Date(), + }]); + const root = renderProperties(container, { + issue: createIssue(), childIssues: [], onUpdate: vi.fn(), inline: true, + externalObjects: [{ + pill: { providerKey: "github", objectType: "pull_request", url: canonical, ...provider }, + mentionCount: 1, sourceLabels: ["Comment"], + group: { object: null, mentions: [], mentionCount: 1, sourceLabels: ["Comment"] }, + }], + }); + await waitForAssertion(() => { + const links = container.querySelectorAll('a[href*="/pull/42"]'); + expect(links).toHaveLength(1); + expect(links[0].getAttribute("href")).toBe(savedUrl); + const row = links[0].closest("li")!; + expect(row.textContent).toContain(provider.statusLabel); + expect(row.querySelector("[data-external-liveness]")?.getAttribute("data-external-liveness")).toBe(provider.liveness); + expect(row.textContent?.includes("Review requested")).toBe(provider.review); + }); + act(() => root.unmount()); + }); + beforeEach(() => { mockSidebarState.isMobile = false; container = document.createElement("div"); diff --git a/ui/src/components/IssuePullRequestLinks.tsx b/ui/src/components/IssuePullRequestLinks.tsx new file mode 100644 index 00000000000..99cdccf9a19 --- /dev/null +++ b/ui/src/components/IssuePullRequestLinks.tsx @@ -0,0 +1,51 @@ +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { Badge } from "@/components/ui/badge"; +import { pullRequestHref, pullRequestIdentity, pullRequestLabel, pullRequestNeedsReview, pullRequestState } from "@/lib/issue-pull-requests"; +import type { ExternalObjectPillData } from "./ExternalObjectPill"; +import { ExternalObjectStatusIcon } from "./ExternalObjectStatusIcon"; +import { externalObjectDisplayStatusLabel, externalObjectLivenessLabel } from "@/lib/external-objects"; + +/** Saved PRs remain inspectable even when the provider lookup is unavailable. */ +export function IssuePullRequestLinks({ products, externalObjects = [] }: { + products: IssueWorkProduct[]; + externalObjects?: ExternalObjectPillData[]; +}) { + return ( +
    + {products.map((product) => { + const href = pullRequestHref(product); + const label = pullRequestLabel(product); + const state = pullRequestState(product); + const identity = pullRequestIdentity(href); + const provider = identity ? externalObjects.find((entry) => pullRequestIdentity(entry.url) === identity) : undefined; + const providerStatus = provider ? externalObjectDisplayStatusLabel(provider) : null; + const providerTerminal = provider?.liveness === "fresh" + && (provider.statusIconKey === "git-merge" || ["merged", "closed"].includes(providerStatus?.toLowerCase() ?? "")); + return ( +
  • + {href ? ( + + {label} + + ) : {label} · No PR link provided} + {provider ? ( + + + {providerStatus}{!["fresh", "unknown"].includes(provider.liveness) ? ` (${externalObjectLivenessLabel(provider.liveness)})` : ""} + + ) : null} + {pullRequestNeedsReview(product) && !providerTerminal ? ( + Review requested + ) : ["merged", "closed", "archived", "draft", "changes_requested"].includes(state) + && providerStatus?.toLowerCase() !== state ? ( + {provider ? "Saved: " : ""}{state.replaceAll("_", " ")} + ) : null} +
  • + ); + })} +
+ ); +} diff --git a/ui/src/components/issue-properties/IssueProperties.tsx b/ui/src/components/issue-properties/IssueProperties.tsx index 640ae943bbc..8e2f6763199 100644 --- a/ui/src/components/issue-properties/IssueProperties.tsx +++ b/ui/src/components/issue-properties/IssueProperties.tsx @@ -1,3 +1,6 @@ +import { IssuePullRequestLinks } from "../IssuePullRequestLinks"; +import { useIssueWorkProducts } from "../../hooks/useIssueWorkProducts"; +import { getIssuePullRequests, pullRequestHref, pullRequestIdentity } from "../../lib/issue-pull-requests"; import { useWorkspaceIsolationControls } from "@/hooks/useWorkspaceIsolationControls"; import { AgentIdentity } from "@/components/AgentIdentity"; import { AgentAvatar } from "@/components/AgentAvatar"; @@ -295,11 +298,12 @@ export function IssueProperties({ queryFn: () => issuesApi.listAttachments(issue.id), enabled: taskChatShellEnabled, }); - const { data: paneTabWorkProducts } = useQuery({ - queryKey: queryKeys.issues.workProducts(issue.id), - queryFn: () => issuesApi.listWorkProducts(issue.id), - enabled: taskChatShellEnabled, - }); + const { data: paneTabWorkProducts, isError: workProductsError, refetch: refetchWorkProducts } = useIssueWorkProducts(issue.id); + const pullRequests = useMemo(() => getIssuePullRequests(paneTabWorkProducts), [paneTabWorkProducts]); + const remainingExternalObjects = useMemo(() => { + const identities = new Set(pullRequests.map((product) => pullRequestIdentity(pullRequestHref(product))).filter(Boolean)); + return externalObjects?.filter((entry) => !identities.has(pullRequestIdentity(entry.pill.url))); + }, [externalObjects, pullRequests]); const { data: paneTabDocuments } = useIssueDocuments(taskChatShellEnabled ? issue.id : null); // Proxy `artifact-review-*` documents surface only through their Work // product row, so they must not summon the Plan or Documents surfaces. @@ -2582,8 +2586,20 @@ export function IssueProperties({ ) : null} + {pullRequests.length > 0 || workProductsError ? ( + +
+ entry.pill)} /> + {workProductsError ? ( + + Couldn’t load pull requests. + + ) : null} +
+
+ ) : null} issuesApi.listWorkProducts(issue.id), - }); + } = useIssueWorkProducts(issue.id); const { data: documents, isPending: documentsPending, diff --git a/ui/src/components/issue-properties/IssuePropertiesDocumentAnnotations.test.tsx b/ui/src/components/issue-properties/IssuePropertiesDocumentAnnotations.test.tsx index ea3b53e283c..77a38ca2293 100644 --- a/ui/src/components/issue-properties/IssuePropertiesDocumentAnnotations.test.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesDocumentAnnotations.test.tsx @@ -35,6 +35,7 @@ vi.mock("@/hooks/useIssuePlanDocument", () => ({ useIssuePlanDocument: () => ({ data: { ...issueDocument, key: "plan" }, isLoading: false }), })); vi.mock("@/hooks/useIssueDocuments", () => ({ useIssueDocuments: () => ({ data: [issueDocument] }) })); +vi.mock("@/hooks/useIssueWorkProducts", () => ({ useIssueWorkProducts: () => ({ data: [] }) })); vi.mock("@/lib/router", () => ({ Link: ({ children, to }: { children: React.ReactNode; to: string }) => {children}, useLocation: () => ({ hash: "" }), diff --git a/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx index a633cc80de9..0b52378d5e4 100644 --- a/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx +++ b/ui/src/components/issue-properties/IssuePropertiesMarkdownWorkProduct.test.tsx @@ -13,7 +13,7 @@ import { RichWorkProductCard } from "@/components/task-chat/RichWorkProductCard" const mockIssuesApi = vi.hoisted(() => ({ listAttachments: vi.fn(async (): Promise => []), - listWorkProducts: vi.fn(async (): Promise => []), + listWorkProducts: vi.fn(async (_issueId: string, _options?: { refreshPullRequests?: boolean }): Promise => []), ensureWorkProductReviewDocument: vi.fn(async (): Promise => ({})), })); vi.mock("@/api/issues", () => ({ issuesApi: mockIssuesApi })); @@ -223,6 +223,23 @@ describe("markdown work product review row", () => { expect(download?.getAttribute("href")).toBe(`/api/attachments/${ATTACHMENT_ID}/content?download=1`); }); + it("shows saved artifacts while the first PR refresh is stalled", async () => { + const saved = makeMarkdownWorkProduct({ + type: "pull_request", provider: "github", title: "Saved PR during refresh", + url: "https://github.com/example/repo/pull/42", metadata: {}, + }); + let finishRefresh!: (products: unknown[]) => void; + const refresh = new Promise((resolve) => { finishRefresh = resolve; }); + mockIssuesApi.listWorkProducts.mockImplementation(async (_id, options) => options?.refreshPullRequests ? refresh : [saved]); + await renderTab({}, saved.title); + await waitForAssertion(() => { + expect(mockIssuesApi.listWorkProducts).toHaveBeenCalledWith(issue.id, expect.objectContaining({ refreshPullRequests: true })); + expect(container.querySelector(`a[href="${saved.url}"]`)).not.toBeNull(); + }); + finishRefresh([{ ...saved, metadata: { state: "merged" } }]); + await waitForAssertion(() => expect(container.textContent?.toLowerCase()).toContain("merged")); + }); + it("expands into the existing document surface without a server call when the document exists", async () => { mockUseIssueDocuments.mockReturnValue({ data: [makeReviewDocument()] }); await renderTab(); diff --git a/ui/src/context/LiveUpdatesProvider.test.ts b/ui/src/context/LiveUpdatesProvider.test.ts index fde36aa5d4e..a51d76423cf 100644 --- a/ui/src/context/LiveUpdatesProvider.test.ts +++ b/ui/src/context/LiveUpdatesProvider.test.ts @@ -937,6 +937,9 @@ describe("LiveUpdatesProvider issue invalidation", () => { expect(invalidations).toContainEqual({ queryKey: queryKeys.issues.comments("issue-1"), }); + for (const ref of ["PAP-759", "issue-1"]) { + expect(invalidations).toContainEqual({ queryKey: queryKeys.issues.workProductPullRequestRefresh(ref) }); + } expect(cache.get(JSON.stringify(queryKeys.issues.activeRun("PAP-759")))).toBeNull(); expect(cache.get(JSON.stringify(queryKeys.issues.liveRuns("PAP-759")))).toEqual([]); expect(cache.get(JSON.stringify(queryKeys.issues.detail("PAP-759")))).toMatchObject({ @@ -977,6 +980,7 @@ describe("LiveUpdatesProvider issue invalidation", () => { )).toBe(true); for (const ref of ["PAP-759", "issue-1"]) { expect(invalidations).not.toContainEqual({ queryKey: queryKeys.issues.comments(ref) }); + expect(invalidations).not.toContainEqual({ queryKey: queryKeys.issues.workProductPullRequestRefresh(ref) }); } }, ); diff --git a/ui/src/context/LiveUpdatesProvider.tsx b/ui/src/context/LiveUpdatesProvider.tsx index 3a38ad3b479..6727ae13837 100644 --- a/ui/src/context/LiveUpdatesProvider.tsx +++ b/ui/src/context/LiveUpdatesProvider.tsx @@ -533,6 +533,7 @@ function invalidateVisibleIssueRunQueries( queryClient.invalidateQueries({ queryKey: queryKeys.issues.comments(issueRef) }); queryClient.invalidateQueries({ queryKey: queryKeys.issues.attachments(issueRef) }); queryClient.invalidateQueries({ queryKey: queryKeys.issues.workProducts(issueRef) }); + queryClient.invalidateQueries({ queryKey: queryKeys.issues.workProductPullRequestRefresh(issueRef) }); queryClient.invalidateQueries({ queryKey: ["issues", "tree-control-state", issueRef] }); } } diff --git a/ui/src/hooks/useIssueWorkProducts.test.tsx b/ui/src/hooks/useIssueWorkProducts.test.tsx new file mode 100644 index 00000000000..b08476b9e5e --- /dev/null +++ b/ui/src/hooks/useIssueWorkProducts.test.tsx @@ -0,0 +1,150 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { afterEach, beforeEach, expect, it, vi } from "vitest"; +import { queryKeys } from "@/lib/queryKeys"; +import { pullRequestNeedsReview } from "@/lib/issue-pull-requests"; +import { __liveUpdatesTestUtils } from "@/context/LiveUpdatesProvider"; +import { useIssueWorkProducts } from "./useIssueWorkProducts"; + +(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + +function response(products: IssueWorkProduct[]) { + return { ok: true, status: 200, json: async () => products } as Response; +} + +function product(id: string, overrides: Partial = {}): IssueWorkProduct { + return { + id, type: "pull_request", provider: "github", title: id, + url: "https://github.com/example/repo/pull/42", metadata: {}, + status: "ready_for_review", reviewState: "needs_board_review", + updatedAt: new Date("2026-10-06T12:00:00Z"), ...overrides, + } as IssueWorkProduct; +} + +let client: QueryClient; +let root: Root; +let host: HTMLDivElement; +let issueId: string; +let observed: ReturnType; +let saved: IssueWorkProduct[]; +let requests: Array<{ path: string; signal: AbortSignal; resolve: (products: IssueWorkProduct[]) => void }>; +let stallSaved: boolean; + +function Harness() { observed = useIssueWorkProducts(issueId); return null; } +async function render() { + await act(async () => root.render()); +} +async function waitFor(assertion: () => void) { + await vi.waitFor(async () => { + await act(async () => { await new Promise((resolve) => setTimeout(resolve, 0)); }); + assertion(); + }); +} +async function liveUpdate() { + await act(async () => { void client.invalidateQueries({ queryKey: queryKeys.issues.workProducts(issueId) }); }); +} + +beforeEach(() => { + client = new QueryClient({ defaultOptions: { queries: { retry: false, gcTime: Infinity } } }); + host = document.createElement("div"); + root = createRoot(host); + issueId = "work-product-race"; + saved = [product("pr")]; + requests = []; + stallSaved = false; + // Keep the real API client, including GET coalescing and abort handling. + vi.stubGlobal("fetch", vi.fn((path: string, init: RequestInit) => { + if (!stallSaved && !path.includes("refreshPullRequests=true")) return Promise.resolve(response(saved)); + return new Promise((resolve) => { + requests.push({ path, signal: init.signal as AbortSignal, resolve: (products) => resolve(response(products)) }); + }); + })); +}); +afterEach(async () => { + await act(async () => { root.unmount(); client.clear(); requests.forEach((request) => request.resolve([])); }); + host.remove(); + vi.unstubAllGlobals(); +}); + +it("shows a newly saved artifact during GitHub refresh and keeps it after the old response", async () => { + const pr = saved[0]; + await render(); + await waitFor(() => expect(requests).toHaveLength(1)); + saved = [pr, product("new-artifact", { type: "artifact" })]; + await liveUpdate(); + await waitFor(() => expect(observed.data?.map((row) => row.id)).toEqual(["pr", "new-artifact"])); + // An artifact arrival does not cancel or restart the unrelated provider request. + expect(requests).toHaveLength(1); + await act(async () => requests[0].resolve([{ ...pr, metadata: { state: "merged" } }])); + await waitFor(() => expect(observed.data?.[0].metadata?.state).toBe("merged")); + expect(observed.data?.map((row) => row.id)).toEqual(["pr", "new-artifact"]); +}); + +it("keeps a changed review request when an older provider response arrives last", async () => { + const original = saved[0]; + await render(); + await waitFor(() => expect(requests).toHaveLength(1)); + saved = [{ ...original, status: "changes_requested", reviewState: "changes_requested", updatedAt: new Date("2026-10-06T12:01:00Z") }]; + await liveUpdate(); + await waitFor(() => { + expect(observed.data?.[0].reviewState).toBe("changes_requested"); + expect(requests).toHaveLength(2); + }); + await act(async () => requests[1].resolve([{ ...saved[0], metadata: { state: "open" } }])); + await waitFor(() => expect(observed.data?.[0].metadata?.state).toBe("open")); + await act(async () => requests[0].resolve([{ ...original, metadata: { state: "merged" } }])); + expect(observed.data?.[0].reviewState).toBe("changes_requested"); + expect(observed.data?.[0].metadata?.state).toBe("open"); +}); + +it("starts a fresh stored read when a live update invalidates an in-flight read", async () => { + const old = product("old", { type: "artifact" }); + client.setQueryData(queryKeys.issues.workProducts(issueId), [old]); + stallSaved = true; + await render(); + await waitFor(() => expect(requests).toHaveLength(1)); + await liveUpdate(); + await waitFor(() => expect(requests).toHaveLength(2)); + expect(requests[0].signal.aborted).toBe(true); + const added = product("new", { type: "artifact" }); + await act(async () => requests[1].resolve([old, added])); + await waitFor(() => expect(observed.data?.map((row) => row.id)).toEqual(["old", "new"])); + await act(async () => requests[0].resolve([old])); + expect(observed.data?.map((row) => row.id)).toEqual(["old", "new"]); +}); + +it("does not show an old task's provider response after navigating", async () => { + const original = saved[0]; + await render(); + await waitFor(() => expect(requests).toHaveLength(1)); + issueId = "another-task"; + saved = [product("another-artifact", { type: "artifact" })]; + await render(); + await waitFor(() => expect(observed.data?.[0].id).toBe("another-artifact")); + await act(async () => requests[0].resolve([{ ...original, metadata: { state: "merged" } }])); + expect(observed.data?.map((row) => row.id)).toEqual(["another-artifact"]); +}); + +it.each(["merged", "closed"])("refreshes GitHub after a run finishes with unchanged saved rows (%s)", async (state) => { + client.setQueryData(queryKeys.issues.detail("PAP-42"), { + id: issueId, identifier: "PAP-42", assigneeAgentId: "agent", executionRunId: "run", + }); + await render(); + await waitFor(() => expect(requests).toHaveLength(1)); + await act(async () => requests[0].resolve([{ ...saved[0], metadata: { state: "open" } }])); + await waitFor(() => expect(observed.data?.[0].metadata?.state).toBe("open")); + expect(pullRequestNeedsReview(observed.data![0])).toBe(true); + + await act(async () => { + __liveUpdatesTestUtils.invalidateVisibleIssueRunQueries(client, "/PAP/issues/PAP-42", { + runId: "run", agentId: "agent", status: "succeeded", + }, { isForegrounded: true }); + }); + await waitFor(() => expect(requests).toHaveLength(2)); + await act(async () => requests[1].resolve([{ ...saved[0], metadata: { state } }])); + await waitFor(() => expect(pullRequestNeedsReview(observed.data![0])).toBe(false)); + expect(observed.data?.[0].metadata?.state).toBe(state); +}); diff --git a/ui/src/hooks/useIssueWorkProducts.ts b/ui/src/hooks/useIssueWorkProducts.ts new file mode 100644 index 00000000000..329bb78a1dc --- /dev/null +++ b/ui/src/hooks/useIssueWorkProducts.ts @@ -0,0 +1,46 @@ +import { useMemo } from "react"; +import { useQuery } from "@tanstack/react-query"; +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { issuesApi } from "@/api/issues"; +import { queryKeys } from "@/lib/queryKeys"; +import { keepPreviousDataForSameQueryTail } from "@/lib/query-placeholder-data"; + +/** Saved rows stay authoritative while GitHub enriches matching PR versions. */ +export function useIssueWorkProducts(issueId: string | null | undefined) { + const stored = useQuery({ + queryKey: queryKeys.issues.workProducts(issueId ?? "__none__"), + queryFn: ({ signal }) => issuesApi.listWorkProducts(issueId!, { signal, fresh: true }), + enabled: Boolean(issueId), + refetchOnMount: "always", + placeholderData: keepPreviousDataForSameQueryTail(issueId ?? "__none__"), + }); + const pullRequests = stored.data?.filter((product) => product.type === "pull_request") ?? []; + const hasPullRequests = Boolean(issueId) && pullRequests.length > 0; + const provider = useQuery({ + // A saved PR change gets its own refresh; adding an artifact does not restart GitHub. + queryKey: [...queryKeys.issues.workProductPullRequestRefresh(issueId ?? "__none__"), pullRequests], + queryFn: ({ signal }) => issuesApi.listWorkProducts(issueId!, { refreshPullRequests: true, signal, fresh: true }), + enabled: hasPullRequests, + refetchOnMount: "always", + }); + const data = useMemo(() => { + const refreshed = new Map(provider.data?.map((product) => [product.id, product])); + return stored.data?.map((product) => { + const enriched = refreshed.get(product.id); + if (product.type !== "pull_request" || !enriched + || new Date(enriched.updatedAt).getTime() !== new Date(product.updatedAt).getTime()) return product; + // Never replace the saved list, its review flags, or its links with a provider snapshot. + return { ...product, metadata: enriched.metadata }; + }); + }, [stored.data, provider.data]); + return { + ...stored, + data, + isFetching: stored.isFetching || provider.isFetching, + isError: stored.isError || provider.isError, + error: stored.error ?? provider.error, + refetch: async () => { + await Promise.all([stored.refetch(), ...(hasPullRequests ? [provider.refetch()] : [])]); + }, + }; +} diff --git a/ui/src/lib/issue-pull-requests.test.ts b/ui/src/lib/issue-pull-requests.test.ts new file mode 100644 index 00000000000..55784aaf1f3 --- /dev/null +++ b/ui/src/lib/issue-pull-requests.test.ts @@ -0,0 +1,51 @@ +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { describe, expect, it } from "vitest"; +import { getIssuePullRequests, pullRequestHref, pullRequestLabel, pullRequestNeedsReview } from "./issue-pull-requests"; + +function product(overrides: Partial = {}): IssueWorkProduct { + return { + id: "pr-1", type: "pull_request", title: "Update runtime probe", url: null, + status: "ready_for_review", reviewState: "needs_board_review", isPrimary: true, + metadata: { url: "https://github.com/example/private-repo/pull/42" }, + updatedAt: new Date("2026-10-06T12:00:00Z"), ...overrides, + } as IssueWorkProduct; +} + +describe("saved issue pull requests", () => { + it("keeps a private PR inspectable from its saved metadata without a provider lookup", () => { + const saved = product(); + expect(pullRequestHref(saved)).toBe("https://github.com/example/private-repo/pull/42"); + expect(pullRequestLabel(saved)).toBe("example/private-repo#42"); + expect(pullRequestNeedsReview(saved)).toBe(true); + }); + + it.each(["merged", "closed", "archived"])("does not ask for review of %s work even with a stale review flag", (state) => { + expect(pullRequestNeedsReview(product({ status: state }))).toBe(false); + expect(pullRequestNeedsReview(product({ metadata: { state } }))).toBe(false); + }); + + it("keeps the newer merged record instead of an old duplicate review request", () => { + const old = product({ id: "old", updatedAt: new Date("2026-10-05T12:00:00Z") }); + const merged = product({ status: "merged" }); + expect(getIssuePullRequests([old, merged])).toEqual([merged]); + }); + + it.each(["?diff=split", "#discussion_r123", "/files", "/"])("deduplicates GitHub navigation variants: %s", (suffix) => { + const old = product({ id: "old", updatedAt: new Date("2026-10-05T12:00:00Z") }); + const merged = product({ status: "merged", url: `https://github.com/Example/Private-Repo/pull/42${suffix}` }); + expect(getIssuePullRequests([old, merged])).toEqual([merged]); + expect(pullRequestHref(merged)).toBe(merged.url); + }); + + it("puts actionable reviews before historical PRs and ignores other work products", () => { + const review = product(); + const merged = product({ id: "merged", url: "https://github.com/example/repo/pull/12", status: "merged" }); + expect(getIssuePullRequests([merged, product({ type: "artifact" }), review])).toEqual([review, merged]); + expect(pullRequestNeedsReview(product({ reviewState: "none" }))).toBe(false); + }); + + it.each(["javascript:alert(1)", "https://token@github.com/example/repo/pull/42", "not a URL"])("does not turn an unsafe saved URL into a link: %s", (url) => { + expect(pullRequestHref(product({ metadata: { url } }))).toBeNull(); + expect(getIssuePullRequests([product({ metadata: { url } })])).toHaveLength(1); + }); +}); diff --git a/ui/src/lib/issue-pull-requests.ts b/ui/src/lib/issue-pull-requests.ts new file mode 100644 index 00000000000..c4becf7b667 --- /dev/null +++ b/ui/src/lib/issue-pull-requests.ts @@ -0,0 +1,69 @@ +import type { IssueWorkProduct } from "@paperclipai/shared"; +import { workProductHref } from "./issue-artifacts"; + +const TERMINAL_STATES = new Set(["merged", "closed", "archived"]); + +export function pullRequestState(product: IssueWorkProduct): string { + if (TERMINAL_STATES.has(product.status)) return product.status; + return typeof product.metadata?.state === "string" ? product.metadata.state : product.status; +} + +export function pullRequestNeedsReview(product: IssueWorkProduct): boolean { + const state = pullRequestState(product); + return !TERMINAL_STATES.has(state) + && state !== "changes_requested" + && product.reviewState === "needs_board_review"; +} + +export function pullRequestHref(product: IssueWorkProduct): string | null { + const href = workProductHref(product); + if (!href) return null; + try { + const url = new URL(href); + if (!["http:", "https:"].includes(url.protocol) || url.username || url.password) return null; + return url.href; + } catch { + return null; + } +} + +export function pullRequestLabel(product: IssueWorkProduct): string { + const href = pullRequestHref(product); + if (href) { + const url = new URL(href); + const match = url.pathname.match(/^\/([^/]+)\/([^/]+)\/pull\/(\d+)\/?$/); + if (url.hostname === "github.com" && match) return `${match[1]}/${match[2]}#${match[3]}`; + } + return product.title; +} + +/** GitHub navigation options and comment anchors do not identify a different PR. */ +export function pullRequestIdentity(href: string | null | undefined): string | null { + if (!href) return null; + try { + const url = new URL(href); + if (!["http:", "https:"].includes(url.protocol) || url.username || url.password) return null; + const match = url.pathname.match(/^\/([^/]+)\/([^/]+)\/pull\/(\d+)(?:\/|$)/); + if (url.hostname === "github.com" && !url.port && match) { + return `github.com/${match[1].toLowerCase()}/${match[2].toLowerCase()}/pull/${Number(match[3])}`; + } + return url.href; + } catch { + return null; + } +} + +/** Prefer the newest saved record when an agent registered the same PR twice. */ +export function getIssuePullRequests(products: IssueWorkProduct[] | null | undefined): IssueWorkProduct[] { + const sorted = (products ?? []).filter((product) => product.type === "pull_request") + .sort((a, b) => new Date(b.updatedAt).getTime() - new Date(a.updatedAt).getTime() + || Number(TERMINAL_STATES.has(pullRequestState(b))) - Number(TERMINAL_STATES.has(pullRequestState(a)))); + const seen = new Set(); + return sorted.filter((product) => { + const key = pullRequestIdentity(pullRequestHref(product)) ?? product.id; + if (seen.has(key)) return false; + seen.add(key); + return true; + }).sort((a, b) => Number(pullRequestNeedsReview(b)) - Number(pullRequestNeedsReview(a)) + || Number(b.isPrimary) - Number(a.isPrimary)); +} diff --git a/ui/src/lib/queryKeys.ts b/ui/src/lib/queryKeys.ts index 8e0c8ff2479..27a5a65a438 100644 --- a/ui/src/lib/queryKeys.ts +++ b/ui/src/lib/queryKeys.ts @@ -397,6 +397,8 @@ export const queryKeys = { ["issues", "runner-goal", issueId, agentId ?? "__effective__"] as const, workProducts: (issueId: string) => ["issues", "work-products", issueId] as const, + workProductPullRequestRefresh: (issueId: string) => + ["issues", "work-product-pr-refresh", issueId] as const, fileResources: ( issueId: string, options: { diff --git a/ui/src/pages/IssueDetail.test.tsx b/ui/src/pages/IssueDetail.test.tsx index 1055e4be7c0..af4a0d6cde1 100644 --- a/ui/src/pages/IssueDetail.test.tsx +++ b/ui/src/pages/IssueDetail.test.tsx @@ -54,6 +54,7 @@ const mockIssuesApi = vi.hoisted(() => ({ listComments: vi.fn(), listAttachments: vi.fn(), listWorkProducts: vi.fn(), + checkMonitorNow: vi.fn(), listFeedbackVotes: vi.fn(), listInteractions: vi.fn(), getQueuedComments: vi.fn(), @@ -1330,6 +1331,7 @@ describe("IssueDetail", () => { mockIssuesApi.listComments.mockResolvedValue([]); mockIssuesApi.listAttachments.mockResolvedValue([]); mockIssuesApi.listWorkProducts.mockResolvedValue([]); + mockIssuesApi.checkMonitorNow.mockReset(); mockIssuesApi.listFeedbackVotes.mockResolvedValue([]); mockIssuesApi.listInteractions.mockResolvedValue([]); mockIssuesApi.getQueuedComments.mockResolvedValue( @@ -1420,6 +1422,37 @@ describe("IssueDetail", () => { mockRouteParams.companyPrefix = "PAP"; }); + it.each([false, true])("keeps monitor errors on the checked task (late response: %s)", async (lateResponse) => { + const monitor = { monitor: { status: "scheduled", nextCheckAt: new Date(Date.now() + 60_000).toISOString(), attemptCount: 1, serviceName: "github" } } as Issue["executionState"]; + mockIssuesApi.get.mockResolvedValue(createIssue({ status: "in_progress", executionState: monitor })); + const check = createDeferred(); + mockIssuesApi.checkMonitorNow.mockReturnValue(check.promise); + await act(async () => root.render()); + let button: HTMLButtonElement | undefined; + await waitForAssertion(() => { + button = Array.from(container.querySelectorAll("button")).find((entry) => entry.textContent === "Check now"); + expect(button).toBeTruthy(); + }); + await act(async () => button!.click()); + if (!lateResponse) { + check.reject(new Error("First task monitor failed")); + await waitForAssertion(() => expect(container.textContent).toContain("First task monitor failed")); + } + const second = createIssue({ id: "issue-2", identifier: "PAP-2", title: "Second monitored task", status: "in_progress", executionState: monitor }); + mockIssuesApi.get.mockResolvedValue(second); + queryClient.setQueryData(queryKeys.issues.detail("PAP-2"), second); + mockRouteParams.issueId = "PAP-2"; + mockLocation.pathname = "/issues/PAP-2"; + await act(async () => root.render()); + if (lateResponse) check.reject(new Error("First task monitor failed")); + await flushReact(); + await waitForAssertion(() => { + expect(container.textContent).toContain("Second monitored task"); + expect(container.textContent).not.toContain("First task monitor failed"); + expect(Array.from(container.querySelectorAll("button")).find((entry) => entry.textContent === "Check now")?.disabled).toBe(false); + }); + }); + afterEach(async () => { await act(async () => { root.unmount(); diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index 07b3c07216e..c740003a6b3 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -94,6 +94,7 @@ import { } from "../lib/issue-timeline-events"; import { queryKeys } from "../lib/queryKeys"; import { keepPreviousDataForSameQueryTail } from "../lib/query-placeholder-data"; +import { useIssueWorkProducts } from "../hooks/useIssueWorkProducts"; import { mergePendingIssueQueuedComments, normalizeIssueQueuedCommentQueue, @@ -3276,34 +3277,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS isLoading: workProductsLoading, isError: workProductsError, refetch: refetchWorkProducts, - } = useQuery({ - queryKey: queryKeys.issues.workProducts(issueId!), - queryFn: () => - issuesApi.listWorkProducts(issueId!, { - // Initial geometry needs stored artifacts, not a network round-trip to - // GitHub. Enrich PR status after the stored list has painted. - refreshPullRequests: - queryClient.getQueryData(queryKeys.issues.workProducts(issueId!)) !== - undefined, - }), - enabled: !!issueId, - refetchOnMount: "always", - placeholderData: keepPreviousDataForSameQueryTail( - issueId ?? "pending", - ), - }); - - const enrichedWorkProductsIssue = useRef(null); - useEffect(() => { - if ( - !issueId || - enrichedWorkProductsIssue.current === issueId || - !workProducts?.some((product) => product.type === "pull_request") - ) - return; - enrichedWorkProductsIssue.current = issueId; - void refetchWorkProducts(); - }, [issueId, workProducts, refetchWorkProducts]); + } = useIssueWorkProducts(issueId); const { data: liveRunCount = 0 } = useQuery( { @@ -4554,6 +4528,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS ); const checkIssueMonitorNow = useMutation({ + mutationKey: ["check-issue-monitor-now", issueId], mutationFn: () => issuesApi.checkMonitorNow(issueId!), onSuccess: () => { invalidateIssueDetail(); @@ -7461,6 +7436,8 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS checkIssueMonitorNow.mutate()} checkingNow={checkIssueMonitorNow.isPending} /> @@ -8014,6 +7991,8 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS hasVisibleMonitorSurface(issue) ? ( checkIssueMonitorNow.mutate()} checkingNow={checkIssueMonitorNow.isPending} /> diff --git a/ui/storybook/stories/task-artifact-gallery.stories.tsx b/ui/storybook/stories/task-artifact-gallery.stories.tsx index 6f44a2dd3e6..79a021486e2 100644 --- a/ui/storybook/stories/task-artifact-gallery.stories.tsx +++ b/ui/storybook/stories/task-artifact-gallery.stories.tsx @@ -45,8 +45,11 @@ const images = [manilaImage, nightImage].map((src, index) => product(index ? "Ni })); type Scenario = "videos" | "mixed" | "rows" | "fallback" | "empty"; +function workProductsForScenario(scenario: Scenario = "videos") { + return scenario === "empty" ? [] : scenario === "rows" ? [...videos.slice(0, 1), ...images.slice(0, 1), ...files] : scenario === "mixed" ? [...videos.slice(0, 2), ...images, ...files] : scenario === "fallback" ? [product("Video preview unavailable", 30, { metadata: { contentType: "video/mp4", contentPath: "data:video/mp4;base64,AA==", originalFilename: "unavailable.mp4" } }), product("Image preview unavailable", 31, { metadata: { contentType: "image/png", contentPath: "data:image/png;base64,AA==" } })] : videos; +} function GalleryStory({ scenario = "videos", width = 480 }: { scenario?: Scenario; width?: number }) { - const workProducts = scenario === "empty" ? [] : scenario === "rows" ? [...videos.slice(0, 1), ...images.slice(0, 1), ...files] : scenario === "mixed" ? [...videos.slice(0, 2), ...images, ...files] : scenario === "fallback" ? [product("Video preview unavailable", 30, { metadata: { contentType: "video/mp4", contentPath: "data:video/mp4;base64,AA==", originalFilename: "unavailable.mp4" } }), product("Image preview unavailable", 31, { metadata: { contentType: "image/png", contentPath: "data:image/png;base64,AA==" } })] : videos; + const workProducts = workProductsForScenario(scenario); const attachments = scenario === "mixed" ? [{ id: "loose-image", companyId: issue.companyId, issueId: issue.id, createdByAgentId: storybookAgents[0].id, contentPath: nightImage, contentType: "image/png", originalFilename: "Unpromoted agent attachment.png", objectKey: "cover.png", byteSize: 42000, createdAt: date } as IssueAttachment] : []; const [client] = useState(() => { const cache = new QueryClient({ defaultOptions: { queries: { staleTime: Infinity, retry: false } } }); @@ -89,7 +92,24 @@ function GalleryStory({ scenario = "videos", width = 480 }: { scenario?: Scenari ); } -const meta = { title: "Tasks/Artifact Gallery", component: GalleryStory, parameters: { layout: "fullscreen" }, args: { scenario: "videos", width: 480 }, render: (args, context) => } satisfies Meta; +const meta = { + title: "Tasks/Artifact Gallery", component: GalleryStory, parameters: { layout: "fullscreen" }, + args: { scenario: "videos", width: 480 }, + beforeEach: ({ args }) => { + const originalFetch = window.fetch; + const fixtureFetch: typeof window.fetch = async (input, init) => { + const url = new URL(typeof input === "string" ? input : input instanceof URL ? input.href : input.url, window.location.origin); + const method = (init?.method ?? (input instanceof Request ? input.method : "GET")).toUpperCase(); + if (method === "GET" && url.pathname === `/api/issues/${issue.id}/work-products`) { + return Response.json(workProductsForScenario(args.scenario)); + } + return originalFetch(input, init); + }; + window.fetch = fixtureFetch; + return () => { if (window.fetch === fixtureFetch) window.fetch = originalFetch; }; + }, + render: (args, context) => , +} satisfies Meta; export default meta; type Story = StoryObj; export const EightVideoOutputs: Story = {};