mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-09 16:02:36 +01:00
## Thinking Path > - Paperclip manages work performed by AI agents. > - Task monitors schedule later checks through execution admission. > - A native run must save its work before a monitor starts another run. > - The monitor banner can show an overdue warning during this valid wait. > - It also promises the next tick and offers a check that cannot start. > - This change explains the active run hold on both monitor surfaces. > - Users can see why the check waits without changing execution guards. ## Linked Issues or Issue Description Refs #16975 and #17863. The branch also preserves the quiet workspace-wait behavior from #19507. These changes added the native settlement guard and monitor deadline worker. A search found no open PR for this display case. **What happened?** A monitor became due while its native run was still saving work. The banner showed an overdue warning and “Fires on next tick.” The check later ran. The warning did not explain the execution hold. **Expected behavior** Show the active run wait. Keep the saved check time. Restore the normal display when the run clears. **Steps to reproduce** 1. Keep a task run active and set a monitor on that task. 2. Let the saved check time pass. 3. Inspect the monitor banner and composer strip. **Paperclip version or commit** Reproduced in mounted production components ata434edf991. The branch also includes current master. **Deployment mode** Source checkout on Linux. PostgreSQL tests use an isolated embedded database. No staging deployment is part of this PR. ## What Changed - Pass the existing task live-run state to both monitor surfaces. - Explain the current run wait. Keep its tone neutral and its saved check time visible. - Hide Check now during the hold. Restore it when the run clears. - Keep pending GitHub PR links visible during the hold. Restore Check status after the run clears. - Preserve blocked-task, retry, and idle-overdue display behavior. - Add rendered regressions and PostgreSQL finalization-release tests. - Document the distinction between a deadline and admission. The monitor display owns this change. The page already reads task run state, so this adds no API request. The scheduler and its guards are unchanged. The PostgreSQL tests confirm existing behavior. They are not evidence of a repaired scheduler fault. ## Verification - Before the repair, all three new rendered cases failed: future, due-now, and two-minute-overdue monitors during an active run. - After the repair, 50 UI tests pass across IssueMonitorBanner and issue-monitor. The same tests check return to normal display when the run clears. - Deadline, scheduled-work, and delivery-coordinator tests pass: 50 tests. The combined five-file run before the final safety assertion passed 99 tests. - PostgreSQL regressions cover release of committed and terminal_failure holds, rollback, one admitted wake, and quiet workers after settlement. - UI typecheck, token gates, and git diff --check pass. - A frozen install and plugin SDK dependency build were required for server test collection. The first collection attempt could not resolve the unbuilt SDK. That attempt had no test verdict. - At initial headee030bdf22, the focused three-file run passes 52 tests: 50 UI tests plus both PostgreSQL finalization-release cases. UI typecheck and token gates pass again after the master merge. - The complete PostgreSQL scheduling suite passes all 56 tests before the master merge. The earlier 54-test baseline also passes. - A real local task page passes in Chromium with fresh embedded PostgreSQL and a process run. Both monitor surfaces explain the hold. Check now stays hidden. Advancing the client clock past the deadline keeps the neutral wait. Reload preserves it. No page errors occur. - The first browser probe used an already-due monitor on a legacy process fixture. That path consumed the schedule before the page loaded, so the expected banner was absent. The corrected probe saves a future monitor and advances only the client clock. It verifies the display; it does not claim native scheduling acceptance. - [CI run 42260573923](https://github.com/paperclipai/paperclip/actions/runs/42260573923) passed foree030bdf22. A later review found hidden PR links during an active run;c3604bf4b4repairs that case. That revision later needed a master conflict repair before build/test CI could start. Final head is 7fe2417e4005378a8c2aee9451ae14f6f49c54ff; its required checks have completed successfully. The incident data establishes an active-run hold at the first due time. It does not fully explain the later interval between terminal status and monitor admission. This PR fixes the confirmed display defect. It does not claim to repair an unproven lost wake. - The review regression failed first in two cases with no PR links: future and overdue GitHub monitors during an active run. After the repair, 52 UI tests pass. They cover links on both surfaces, held check actions, return to Check status, and removal of merged PR links. UI typecheck and token gates also pass. - Current master was merged at7fe2417e40. Both the PR-link tests and master’s quiet workspace-wait tests are retained. All 65 tests across IssueMonitorBanner, issue-monitor, and IssueScheduledRetryCard pass. UI typecheck, token gates, and diff checks pass. [Final-head CI](https://github.com/paperclipai/paperclip/actions/runs/42267525907) completed successfully: 55 successful checks, three skipped, and one neutral. Build, Typecheck + Release Registry, and ci / verify passed. Greptile reviewed this exact head at 5/5 with no actionable findings. The prior reviewer thread is resolved; no review threads remain open. The real Chromium acceptance above was performed at ee030bdf22; the final link handling and merged workspace-wait cases are covered by the mounted component tests and fresh CI. ## Risks - The wait display depends on the page’s existing live-run query. A stale query can briefly keep the wait visible. - A monitor with no active run still shows an overdue warning. A blocked task keeps its existing pause controls. - No schema, scheduling, ownership, or authorization change. ## Model Used OpenAI Codex, GPT-6 family, with repository inspection, code editing, shell tools, and test execution. The exact serving snapshot and context-window size are 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 - [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>
373 lines
14 KiB
TypeScript
373 lines
14 KiB
TypeScript
import { useMemo, useState } from "react";
|
|
import { Clock, X } from "lucide-react";
|
|
import type { Issue, IssueWorkProduct } from "@paperclipai/shared";
|
|
|
|
import { Button } from "@/components/ui/button";
|
|
import {
|
|
AlertDialog,
|
|
AlertDialogTrigger,
|
|
AlertDialogContent,
|
|
AlertDialogHeader,
|
|
AlertDialogTitle,
|
|
AlertDialogDescription,
|
|
AlertDialogFooter,
|
|
AlertDialogCancel,
|
|
} from "@/components/ui/alert-dialog";
|
|
import { InlineBanner } from "@/components/InlineBanner";
|
|
import { IssuePullRequestLinks } from "@/components/IssuePullRequestLinks";
|
|
import { getIssuePullRequests, pullRequestNeedsReview } from "@/lib/issue-pull-requests";
|
|
import { cn } from "@/lib/utils";
|
|
import {
|
|
areAutomaticMonitorChecksPaused,
|
|
canCheckIssueMonitorNow,
|
|
deriveMonitorState,
|
|
formatMonitorAbsolute,
|
|
formatMonitorEta,
|
|
useMonitorCountdown,
|
|
type DerivedMonitorState,
|
|
type MonitorDisplayState,
|
|
} from "@/lib/issue-monitor";
|
|
|
|
/** Matches the `Date | string` inputs accepted by the issue-monitor helpers. */
|
|
type MonitorDate = Date | string;
|
|
|
|
/**
|
|
* States in which the waiting-monitor surfaces (top banner + composer strip)
|
|
* are shown. `cleared` and `none` hide both surfaces entirely — see
|
|
* wireframe 04 (PAP-14557).
|
|
*/
|
|
const WAITING_STATES: readonly MonitorDisplayState[] = [
|
|
"scheduled",
|
|
"retrying",
|
|
"due-now",
|
|
"overdue",
|
|
];
|
|
|
|
export function isWaitingMonitorState(state: MonitorDisplayState): boolean {
|
|
return WAITING_STATES.includes(state);
|
|
}
|
|
|
|
export function hasVisibleMonitorSurface(issue: Issue): boolean {
|
|
const derived = deriveMonitorState(issue);
|
|
return isWaitingMonitorState(derived.state) && derived.nextCheckAt !== null;
|
|
}
|
|
|
|
export interface MonitorSurfaceCopy {
|
|
/** Prominent lead for the top banner, e.g. "Waiting on monitor — resumes in 2h 12m". */
|
|
bannerTitle: string;
|
|
/** Prominent lead for the composer strip, e.g. "Resumes in 2h 12m". */
|
|
stripTitle: string;
|
|
/** Muted detail line for the banner (absolute time carries a "(your time)" hint). */
|
|
bannerMeta: string[];
|
|
/** Muted detail line for the composer strip. */
|
|
stripMeta: string[];
|
|
/** `warning` (amber) once overdue, `info` (blue) while still on schedule. */
|
|
tone: "info" | "warning";
|
|
workspaceWait?: boolean;
|
|
pullRequestReview?: boolean;
|
|
automaticChecksPaused?: boolean;
|
|
checkUnavailable?: boolean;
|
|
executionWait?: boolean;
|
|
}
|
|
|
|
function capitalize(value: string): string {
|
|
return value.length === 0 ? value : `${value.charAt(0).toUpperCase()}${value.slice(1)}`;
|
|
}
|
|
|
|
/**
|
|
* Pure copy builder shared by the banner and the composer strip so both
|
|
* surfaces render one consistent copy system (see wireframe 04). Kept free of
|
|
* hooks/`Date.now()` so it is deterministic under test.
|
|
*/
|
|
export function buildMonitorSurfaceCopy(
|
|
derived: DerivedMonitorState,
|
|
now: MonitorDate,
|
|
scheduledRetryReason?: string | null,
|
|
pendingPullRequestReviews = 0,
|
|
issueStatus?: string,
|
|
hasActiveRun = false,
|
|
): MonitorSurfaceCopy | null {
|
|
if (!isWaitingMonitorState(derived.state) || !derived.nextCheckAt) return null;
|
|
|
|
if (derived.source === "monitor" && areAutomaticMonitorChecksPaused({ status: issueStatus })) {
|
|
const canCheckNow = canCheckIssueMonitorNow({ status: issueStatus }, derived.serviceName);
|
|
const hint = canCheckNow
|
|
? "This task is blocked. You can ask the agent to check now."
|
|
: "Resume this task to continue checking.";
|
|
return {
|
|
bannerTitle: "Automatic checks paused",
|
|
stripTitle: "Automatic checks paused",
|
|
bannerMeta: [hint],
|
|
stripMeta: [hint],
|
|
tone: "info",
|
|
automaticChecksPaused: true,
|
|
checkUnavailable: !canCheckNow,
|
|
};
|
|
}
|
|
|
|
if (derived.source === "scheduled-retry" && scheduledRetryReason === "workspace_busy") {
|
|
return {
|
|
bannerTitle: "Waiting for workspace",
|
|
stripTitle: "Waiting for workspace",
|
|
bannerMeta: ["Another task is using this workspace. Work starts automatically when it is available."],
|
|
stripMeta: ["Work starts automatically when the workspace is available."],
|
|
tone: "info",
|
|
workspaceWait: true,
|
|
};
|
|
}
|
|
|
|
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"
|
|
const awaitingPullRequestReview = derived.source === "monitor" && derived.serviceName?.toLowerCase() === "github" && pendingPullRequestReviews > 0;
|
|
if (derived.source === "monitor" && hasActiveRun) {
|
|
const hint = "The check can start after the current run finishes saving its work.";
|
|
return {
|
|
bannerTitle: "Monitor waits for current run",
|
|
stripTitle: "Monitor waits for current run",
|
|
bannerMeta: [hint, `Scheduled for ${absolute} (your time)`],
|
|
stripMeta: [hint, `Scheduled for ${absolute}`],
|
|
tone: "info",
|
|
executionWait: true,
|
|
checkUnavailable: true,
|
|
pullRequestReview: awaitingPullRequestReview,
|
|
};
|
|
}
|
|
if (awaitingPullRequestReview) {
|
|
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",
|
|
stripTitle: "Pool exhausted",
|
|
bannerMeta: [`Usage recheck ${eta} · ${absolute} (your time)`, "Tasks with a selected account keep it while waiting. Work resumes when usage permits."],
|
|
stripMeta: [`Usage recheck ${eta} · ${absolute}`, "Tasks with a selected account keep it while waiting."],
|
|
tone: "info",
|
|
};
|
|
}
|
|
|
|
const isScheduledRetryOnly = derived.source === "scheduled-retry";
|
|
|
|
let bannerTitle: string;
|
|
let stripTitle: string;
|
|
let statusHint: string | null = null;
|
|
switch (derived.state) {
|
|
case "scheduled":
|
|
case "retrying":
|
|
bannerTitle = isScheduledRetryOnly ? `Agent resumes ${eta}` : `Waiting on monitor — resumes ${eta}`;
|
|
stripTitle = `Resumes ${eta}`;
|
|
break;
|
|
case "due-now":
|
|
bannerTitle = isScheduledRetryOnly ? "Agent retry due now" : "Waiting on monitor — due now";
|
|
stripTitle = "Due now";
|
|
statusHint = "Checking momentarily…";
|
|
break;
|
|
case "overdue":
|
|
default:
|
|
bannerTitle = isScheduledRetryOnly ? `Agent retry ${eta}` : `Waiting on monitor — ${eta}`;
|
|
stripTitle = capitalize(eta);
|
|
statusHint = "Fires on next tick";
|
|
break;
|
|
}
|
|
|
|
const attemptLabel = derived.attemptCount >= 1 ? `Attempt ${derived.attemptCount}` : null;
|
|
const serviceLabel = derived.serviceName ? `Watching: ${derived.serviceName}` : null;
|
|
|
|
const bannerMeta = [statusHint, `${absolute} (your time)`, attemptLabel, serviceLabel].filter(
|
|
(piece): piece is string => Boolean(piece),
|
|
);
|
|
const stripMeta = [statusHint, absolute, attemptLabel, serviceLabel].filter(
|
|
(piece): piece is string => Boolean(piece),
|
|
);
|
|
|
|
return {
|
|
bannerTitle,
|
|
stripTitle,
|
|
bannerMeta,
|
|
stripMeta,
|
|
tone: derived.state === "overdue" ? "warning" : "info",
|
|
};
|
|
}
|
|
|
|
function useMonitorSurfaceCopy(issue: Issue, pendingPullRequestReviews: number, hasActiveRun: boolean): 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(() => {
|
|
const derived = deriveMonitorState(issue);
|
|
return derived.source === "monitor" && areAutomaticMonitorChecksPaused(issue) ? null : derived.nextCheckAt;
|
|
}, [issue]);
|
|
const now = useMonitorCountdown(nextCheckAt);
|
|
return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason, pendingPullRequestReviews, issue.status, hasActiveRun), [issue, now, pendingPullRequestReviews, hasActiveRun]);
|
|
}
|
|
|
|
function CheckNowButton({
|
|
onCheckNow,
|
|
checkingNow,
|
|
reviewRequested = false,
|
|
}: {
|
|
onCheckNow: () => void;
|
|
checkingNow: boolean;
|
|
reviewRequested?: boolean;
|
|
}) {
|
|
return (
|
|
<Button
|
|
type="button"
|
|
variant="outline"
|
|
size="sm"
|
|
className="shrink-0 shadow-none"
|
|
onClick={onCheckNow}
|
|
disabled={checkingNow}
|
|
>
|
|
{checkingNow ? "Checking…" : reviewRequested ? "Check status" : "Check now"}
|
|
</Button>
|
|
);
|
|
}
|
|
|
|
function CancelMonitorButton({ onCancel }: { onCancel: () => Promise<unknown> }) {
|
|
const [open, setOpen] = useState(false);
|
|
const [pending, setPending] = useState(false);
|
|
const [error, setError] = useState<string | null>(null);
|
|
const cancel = async () => {
|
|
setPending(true);
|
|
setError(null);
|
|
try {
|
|
await onCancel();
|
|
setOpen(false);
|
|
} catch (err) {
|
|
setError(err instanceof Error ? err.message : "Unable to cancel the monitor. Try again.");
|
|
} finally {
|
|
setPending(false);
|
|
}
|
|
};
|
|
return (
|
|
<AlertDialog open={open} onOpenChange={(next) => { if (!pending) { setOpen(next); setError(null); } }}>
|
|
<AlertDialogTrigger asChild>
|
|
<Button type="button" variant="ghost" size="icon" className="absolute right-2 top-2" aria-label="Cancel monitor" title="Cancel monitor">
|
|
<X className="h-4 w-4" aria-hidden="true" />
|
|
</Button>
|
|
</AlertDialogTrigger>
|
|
<AlertDialogContent>
|
|
<AlertDialogHeader>
|
|
<AlertDialogTitle>Cancel monitor?</AlertDialogTitle>
|
|
<AlertDialogDescription>This removes the scheduled monitor check. The agent will no longer resume from this monitor. You can send a message to continue the task.</AlertDialogDescription>
|
|
</AlertDialogHeader>
|
|
{error ? <p role="alert" className="text-sm text-destructive">{error}</p> : null}
|
|
<AlertDialogFooter>
|
|
<AlertDialogCancel disabled={pending}>Keep monitor</AlertDialogCancel>
|
|
<Button type="button" variant="destructive" disabled={pending} onClick={() => void cancel()}>{pending ? "Cancelling…" : "Cancel monitor"}</Button>
|
|
</AlertDialogFooter>
|
|
</AlertDialogContent>
|
|
</AlertDialog>
|
|
);
|
|
}
|
|
|
|
export interface IssueMonitorSurfaceProps {
|
|
issue: Issue;
|
|
hasActiveRun?: boolean;
|
|
onCheckNow?: (() => void) | null;
|
|
checkingNow?: boolean;
|
|
workProducts?: IssueWorkProduct[] | null;
|
|
checkError?: string | null;
|
|
onCancelMonitor?: (() => Promise<unknown>) | null;
|
|
}
|
|
|
|
/**
|
|
* Pinned banner rendered between the issue title and description while a
|
|
* monitor is waiting. Replaces the description-area "Monitor scheduled" card
|
|
* for the waiting state (PAP-14557 decision 1) — the two never render at once.
|
|
*/
|
|
export function IssueMonitorBanner({
|
|
issue,
|
|
hasActiveRun = false,
|
|
onCheckNow = null,
|
|
checkingNow = false,
|
|
workProducts,
|
|
checkError,
|
|
onCancelMonitor,
|
|
}: IssueMonitorSurfaceProps) {
|
|
const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview);
|
|
const copy = useMonitorSurfaceCopy(issue, reviews.length, hasActiveRun);
|
|
if (!copy) return null;
|
|
|
|
// A busy workspace is a transient system wait the user cannot act on. It stays
|
|
// a quiet status (composer strip, properties row), never a pinned notice.
|
|
if (copy.workspaceWait) return null;
|
|
|
|
const canCancel = onCancelMonitor && deriveMonitorState(issue).source === "monitor";
|
|
|
|
return (
|
|
<InlineBanner
|
|
tone={copy.tone}
|
|
icon={Clock}
|
|
title={copy.bannerTitle}
|
|
className={cn("relative my-3", canCancel && "pr-12")}
|
|
actions={onCheckNow && !copy.workspaceWait && !copy.checkUnavailable ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
|
|
>
|
|
{canCancel ? <CancelMonitorButton key={issue.id} onCancel={onCancelMonitor} /> : null}
|
|
<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>
|
|
);
|
|
}
|
|
|
|
/**
|
|
* Slim, inline (not sticky) strip anchored directly above the reply composer.
|
|
* Mirrors the banner's monitor state and reminds the reader that replying wakes
|
|
* the agent early (PAP-14557 decisions 2 + wireframe 02).
|
|
*/
|
|
export function IssueMonitorComposerStrip({
|
|
issue,
|
|
hasActiveRun = false,
|
|
onCheckNow = null,
|
|
checkingNow = false,
|
|
workProducts,
|
|
checkError,
|
|
className,
|
|
}: IssueMonitorSurfaceProps & { className?: string }) {
|
|
const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview);
|
|
const copy = useMonitorSurfaceCopy(issue, reviews.length, hasActiveRun);
|
|
if (!copy) return null;
|
|
|
|
return (
|
|
<div
|
|
role="note"
|
|
data-testid="issue-monitor-composer-strip"
|
|
className={cn("rounded-lg border border-border bg-muted/30 px-3 py-2", className)}
|
|
>
|
|
<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={cn("mt-0.5 h-4 w-4 shrink-0", issue.displayStatus === "monitoring" ? "text-(--status-task-icon-monitoring)" : "text-muted-foreground")} aria-hidden="true" />
|
|
<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 && !copy.checkUnavailable ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
|
|
</div>
|
|
{!copy.automaticChecksPaused ? (
|
|
<p className="mt-1.5 text-xs text-muted-foreground">
|
|
{copy.pullRequestReview && copy.executionWait
|
|
? "Review on GitHub while the current run finishes saving its work."
|
|
: copy.pullRequestReview
|
|
? "Review on GitHub, then check status to have the agent verify the result."
|
|
: copy.workspaceWait || copy.executionWait
|
|
? "You can keep sending instructions while the agent waits."
|
|
: "Sending a reply wakes the agent now — before the scheduled check."}
|
|
</p>
|
|
) : null}
|
|
{checkError ? <p role="alert" className="text-sm text-destructive">{checkError}</p> : null}
|
|
</div>
|
|
);
|
|
}
|