fix(ui): make pull request review waits visible (#15375)

## Thinking Path

> - Paperclip lets people supervise agent work and review its outputs.
> - A task can wait for a human to review a pull request while an agent
schedules checks.
> - The PR work product and detected external links use separate display
paths.
> - A private PR can disappear from the useful sidebar view when its
provider lookup fails.
> - The monitor countdown says when the next check runs but omits the
saved review request.
> - This change shows the saved PR and requested review in the task's
existing surfaces.

## Linked Issues or Issue Description

**What happened?**

A task kept checking whether a private GitHub PR had merged. Its work
product requested board review, but the Properties sidebar used detected
external objects instead. The PR URL was stored only in metadata, which
the PR refresh path did not read. The composer showed a generic monitor
countdown with no PR link or review action.

**Expected behavior**

Show saved PR links even if provider access fails. During a GitHub
monitor wait, make outstanding review requests visible with the next
check time and a status-check action. Stop requesting review when a PR
merges or closes.

**Steps to reproduce**

1. Register a PR work product with `reviewState: needs_board_review` and
its URL in `metadata.url`.
2. Schedule an external-service monitor for GitHub.
3. Open the task with external-object lookup unavailable or unable to
access the private repository.
4. Inspect Properties and the composer wait strip.

Related work: #14469 added rich artifact cards. #8759 addresses
attention on monitored task blockers. This change uses the existing work
products and monitor action; it adds no blocker or approval mechanism.

## What Changed

- Show saved PRs in Properties, including metadata-only links,
independent of external-object availability.
- Deduplicate equivalent GitHub PR URLs while preserving the saved
navigation link, and put explicit review requests first.
- Keep provider status and freshness on the combined PR row; keep
private PR links usable when lookup fails.
- Show the saved PR review request above the composer and in the
existing monitor banner during a GitHub monitor wait.
- Label the existing monitor action `Check status` and show check
failures inline.
- Suppress review prompts for merged, closed, or archived PRs even if
their review flag is stale.
- Refresh PR metadata using `metadata.url` and the existing `repository`
alias.
- Share saved work-product reads across the thread, Properties, and
Artifacts. Refresh GitHub in a separate query and enrich only matching
PR versions.
- Start a fresh saved-row request on live invalidation so late provider
responses cannot hide new artifacts or changed review requests.
- Cover stalled GitHub lookups in both panels so refresh latency cannot
hide saved work.
- Scope monitor-check mutation state to the task so failures and late
responses do not leak across navigation.
- Refresh GitHub status when the displayed run finishes, including when
saved PR rows have not changed.
- Clarify that PR review and external release handoffs need a saved
human-input interaction with an agent assigned for continuation; a flag,
monitor, or handoff comment alone does not create that card.

## Verification

- 386 tests pass across eleven affected UI and server suites on
`184c7d8746`. Regressions cover saved links, provider status, cold-cache
loading, panel reopening, monitor errors during navigation, and live
updates during provider refresh.
- Three live-update regressions and two run-completion regressions fail
before their fixes and pass afterward. These tests use the real API
client's GET coalescing and abort handling.
- Full repository typecheck and build passed on the merged parent
`1a49112dd2`. The latest UI changes pass UI typecheck and token gates;
CI also verifies the latest build. Capability contract/inventory checks
pass.
- Storybook build and browser checks passed before the query race fix.
Browser checks cover desktop and 390px mobile review waits, plus video,
mixed-file, and empty artifact galleries after background refresh.
- The earlier full local `pnpm test:run` was stopped under disk
pressure. It reported failures outside the changed suites; an isolated
skill-cache run reproduced three existing macOS permission failures. The
full local suite was not rerun for this follow-up.
- Latest-head CI passes on `184c7d8746`, including the browser shards
and canary dry run. Apex is 5/5 with no actionable findings and no
unresolved threads. All eight reported findings are addressed.

## Risks

- This uses saved PR review state. When GitHub access fails, the saved
state can remain stale until the agent updates it. The link remains
visible and the status check remains available.
- `Check status` wakes the existing monitor owner. It does not merge a
PR, accept an approval, or mark the task done.
- No schema, permissions, or scheduler behavior changes.

## Model Used

OpenAI GPT-6 via Codex, with reasoning, repository analysis, code
editing, and test execution. The exact deployment model ID and context
window were not exposed in this session.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass — 277 affected tests
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
Devin FoleyandPaperclip authored and GitHub committed 2026-10-06 16:03:13 -07:00
1 parent 8cbd21b3e7
commit 228f0e2807
23 files changed
+734 -59

No files matched your search

+14
View File
@@ -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.
@@ -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 });
+3 -1
View File
@@ -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];
+18
View File
@@ -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.
+7 -5
View File
@@ -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<Approval[]>(`/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<IssueWorkProduct[]>(
`/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<IssueWorkProduct[]>(path, { signal: options?.signal });
},
ensureWorkProductReviewDocument: (id: string, workProductId: string) =>
api.post<IssueDocument>(
`/issues/${id}/work-products/${workProductId}/review-document`,
+32 -1
View File
@@ -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(
<IssueMonitorComposerStrip issue={issue} workProducts={products} onCheckNow={onCheckNow} checkError={error} />,
));
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() },
+43 -11
View File
@@ -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 (
<Button
@@ -159,7 +175,7 @@ function CheckNowButton({
onClick={onCheckNow}
disabled={checkingNow}
>
{checkingNow ? "Checking…" : "Check now"}
{checkingNow ? "Checking…" : reviewRequested ? "Check status" : "Check now"}
</Button>
);
}
@@ -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 ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} /> : null}
actions={onCheckNow && !copy.workspaceWait ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
>
<span>{copy.bannerMeta.join(" · ")}</span>
<div className="flex flex-col gap-2">
{copy.pullRequestReview ? <IssuePullRequestLinks products={reviews} /> : null}
<span>{copy.bannerMeta.join(" · ")}</span>
{checkError ? <p role="alert">{checkError}</p> : null}
</div>
</InlineBanner>
);
}
@@ -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({
<div className="flex flex-col gap-2 sm:flex-row sm:items-center sm:justify-between">
<div className="flex min-w-0 items-start gap-2">
<Clock className="mt-0.5 h-4 w-4 shrink-0 text-muted-foreground" aria-hidden="true" />
<div className="min-w-0">
<div className="flex min-w-0 flex-col gap-2">
<div className="text-sm font-medium text-foreground">{copy.stripTitle}</div>
{copy.pullRequestReview ? <IssuePullRequestLinks products={reviews} /> : null}
<div className="text-xs text-muted-foreground">{copy.stripMeta.join(" · ")}</div>
</div>
</div>
{onCheckNow && !copy.workspaceWait ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} /> : null}
{onCheckNow && !copy.workspaceWait ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
</div>
<p className="mt-1.5 text-xs text-muted-foreground">
{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."}
</p>
{checkError ? <p role="alert" className="text-sm text-destructive">{checkError}</p> : null}
</div>
);
}
+117
View File
@@ -465,6 +465,123 @@ function renderProperties(container: HTMLDivElement, props: ComponentProps<typeo
describe("IssueProperties", () => {
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(<QueryClientProvider client={queryClient}>
<IssueProperties issue={createIssue()} childIssues={[]} onUpdate={vi.fn()} inline />
</QueryClientProvider>));
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(<QueryClientProvider client={queryClient}>
<IssueProperties issue={createIssue()} childIssues={[]} onUpdate={vi.fn()} inline />
</QueryClientProvider>));
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<unknown[]>((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");
@@ -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 (
<ul className="flex min-w-0 flex-col gap-2">
{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 (
<li key={product.id} className="flex min-w-0 flex-wrap items-center gap-2">
{href ? (
<a href={href} target="_blank" rel="noopener noreferrer" title={product.title}
className="break-words text-sm text-primary underline-offset-2 hover:underline">
{label}
</a>
) : <span className="break-words text-sm">{label} · No PR link provided</span>}
{provider ? (
<span className="inline-flex items-center gap-1 text-xs text-muted-foreground"
data-external-status={provider.statusCategory} data-external-liveness={provider.liveness}>
<ExternalObjectStatusIcon category={provider.statusCategory} liveness={provider.liveness}
statusIconKey={provider.statusIconKey} label={providerStatus} />
<span>{providerStatus}{!["fresh", "unknown"].includes(provider.liveness) ? ` (${externalObjectLivenessLabel(provider.liveness)})` : ""}</span>
</span>
) : null}
{pullRequestNeedsReview(product) && !providerTerminal ? (
<Badge variant="outline">Review requested</Badge>
) : ["merged", "closed", "archived", "draft", "changes_requested"].includes(state)
&& providerStatus?.toLowerCase() !== state ? (
<span className="text-xs text-muted-foreground">{provider ? "Saved: " : ""}{state.replaceAll("_", " ")}</span>
) : null}
</li>
);
})}
</ul>
);
}
@@ -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({
</PropertyRow>
) : null}
{pullRequests.length > 0 || workProductsError ? (
<PropertyRow label="Pull requests" wrap>
<div className="flex min-w-0 flex-col gap-2">
<IssuePullRequestLinks products={pullRequests} externalObjects={externalObjects?.map((entry) => entry.pill)} />
{workProductsError ? (
<span className="text-xs text-muted-foreground">
Couldn’t load pull requests. <button type="button" className="text-primary hover:underline" onClick={() => void refetchWorkProducts()}>Retry</button>
</span>
) : null}
</div>
</PropertyRow>
) : null}
<ExternalObjectRows
externalObjects={externalObjects}
externalObjects={remainingExternalObjects}
externalObjectsLoading={externalObjectsLoading}
externalObjectsError={externalObjectsError}
onRetryExternalObjects={onRetryExternalObjects}
@@ -20,6 +20,7 @@ import { activityApi } from "@/api/activity";
import { agentsApi } from "@/api/agents";
import { queryKeys } from "@/lib/queryKeys";
import { useIssueDocuments } from "@/hooks/useIssueDocuments";
import { useIssueWorkProducts } from "@/hooks/useIssueWorkProducts";
import {
documentDisplayTitle,
selectAgentArtifactAttachments,
@@ -380,10 +381,7 @@ export function IssuePropertiesArtifactsTab({
isPending: productsPending,
isError: productsError,
refetch: refetchProducts,
} = useQuery({
queryKey: queryKeys.issues.workProducts(issue.id),
queryFn: () => issuesApi.listWorkProducts(issue.id),
});
} = useIssueWorkProducts(issue.id);
const {
data: documents,
isPending: documentsPending,
@@ -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 }) => <a href={to}>{children}</a>,
useLocation: () => ({ hash: "" }),
@@ -13,7 +13,7 @@ import { RichWorkProductCard } from "@/components/task-chat/RichWorkProductCard"
const mockIssuesApi = vi.hoisted(() => ({
listAttachments: vi.fn(async (): Promise<unknown[]> => []),
listWorkProducts: vi.fn(async (): Promise<unknown[]> => []),
listWorkProducts: vi.fn(async (_issueId: string, _options?: { refreshPullRequests?: boolean }): Promise<unknown[]> => []),
ensureWorkProductReviewDocument: vi.fn(async (): Promise<unknown> => ({})),
}));
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<unknown[]>((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();
@@ -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) });
}
},
);
+1
View File
@@ -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] });
}
}
+150
View File
@@ -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> = {}): 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<typeof useIssueWorkProducts>;
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(<QueryClientProvider client={client}><Harness /></QueryClientProvider>));
}
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<Response>((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);
});
+46
View File
@@ -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<IssueWorkProduct[]>(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()] : [])]);
},
};
}
+51
View File
@@ -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> = {}): 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);
});
});
+69
View File
@@ -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<string>();
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));
}
+2
View File
@@ -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: {
+33
View File
@@ -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<never>();
mockIssuesApi.checkMonitorNow.mockReturnValue(check.promise);
await act(async () => root.render(<QueryClientProvider client={queryClient}><IssueDetail /></QueryClientProvider>));
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(<QueryClientProvider client={queryClient}><IssueDetail /></QueryClientProvider>));
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();
+7 -28
View File
@@ -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<IssueWorkProduct[]>(
issueId ?? "pending",
),
});
const enrichedWorkProductsIssue = useRef<string | null>(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<LiveRunForIssue[], Error, number>(
{
@@ -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
<IssueMonitorBanner
issue={issue}
workProducts={workProducts}
checkError={checkIssueMonitorNow.error?.message}
onCheckNow={() => checkIssueMonitorNow.mutate()}
checkingNow={checkIssueMonitorNow.isPending}
/>
@@ -8014,6 +7991,8 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS
hasVisibleMonitorSurface(issue) ? (
<IssueMonitorComposerStrip
issue={issue}
workProducts={workProducts}
checkError={checkIssueMonitorNow.error?.message}
onCheckNow={() => checkIssueMonitorNow.mutate()}
checkingNow={checkIssueMonitorNow.isPending}
/>
@@ -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
</QueryClientProvider>
);
}
const meta = { title: "Tasks/Artifact Gallery", component: GalleryStory, parameters: { layout: "fullscreen" }, args: { scenario: "videos", width: 480 }, render: (args, context) => <GalleryStory key={`${context.id}-${args.scenario}`} {...args} /> } satisfies Meta<typeof GalleryStory>;
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) => <GalleryStory key={`${context.id}-${args.scenario}`} {...args} />,
} satisfies Meta<typeof GalleryStory>;
export default meta;
type Story = StoryObj<typeof meta>;
export const EightVideoOutputs: Story = {};