From c30a7c79bf034bc5286688ece0148b2ee31d7eff Mon Sep 17 00:00:00 2001 From: cryppadotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 6 Nov 2026 17:18:20 +0000 Subject: [PATCH] fix(ui): explain monitor waits during active runs Co-Authored-By: Paperclip --- doc/idle-issue-monitors.md | 7 +++++ .../src/services/heartbeat/scheduling.test.ts | 31 +++++++++++++++++++ ui/src/components/IssueMonitorBanner.test.tsx | 31 ++++++++++++++++++- ui/src/components/IssueMonitorBanner.tsx | 27 +++++++++++++--- ui/src/pages/IssueDetail.tsx | 2 ++ 5 files changed, 92 insertions(+), 6 deletions(-) diff --git a/doc/idle-issue-monitors.md b/doc/idle-issue-monitors.md index db0903a914b..7923ac5268e 100644 --- a/doc/idle-issue-monitors.md +++ b/doc/idle-issue-monitors.md @@ -12,6 +12,13 @@ already-known monitor deadline can fire earlier than the batching window. After all work settles, no periodic monitor scan remains. Pending claims and failed or deferred admissions retain their bounded deadlines. +A monitor deadline is the earliest time for a check. An active native run or an +unfinished finalization holds the check until the prior work settles. The task +banner explains the active run wait instead of promising the next scheduler tick +or showing it as overdue. Once the run clears, the banner resumes its countdown +and offers Check now. Finalization notifications restore monitor scheduling from +the saved rows; they do not bypass execution admission. + Both idle drains and ordinary maintenance drains pause monitor dispatch. The worker retains its pending request without repeatedly reading monitor tables. After an explicit drain stop or TTL expiry, a bounded in-memory retry resumes diff --git a/server/src/services/heartbeat/scheduling.test.ts b/server/src/services/heartbeat/scheduling.test.ts index ba6d6ec9803..a46e6abe880 100644 --- a/server/src/services/heartbeat/scheduling.test.ts +++ b/server/src/services/heartbeat/scheduling.test.ts @@ -16,6 +16,7 @@ import { issueService } from "../issues.js"; import { createDeliveryWorkCoordinator } from "../delivery-work-coordinator.js"; import { DELIVERY_QUEUES } from "../delivery-work-notifications.js"; import { registerScheduledWork } from "../scheduled-work.js"; +import { notifyNativeFinalizationWork } from "../native-finalization-notifications.js"; import { idleWorkSnapshot, readTaskDrain, startTaskDrain, stopTaskDrain } from "../task-admission.js"; import { conflict } from "../../errors.js"; import { createHeartbeatRunPreparation } from "./run-preparation.js"; @@ -585,6 +586,36 @@ describe.skipIf(!postgres.supported)("heartbeat scheduling database boundary", ( } }); } + it.each(["committed", "terminal_failure"] as const)("reactivates a due monitor after its last finalization hold becomes %s", async phase => { + const f = await fixture(); + const [run] = await db.insert(heartbeatRuns).values({ + companyId: f.company.id, agentId: f.agent.id, nativeIssueId: f.issue.id, + runtimeMode: "native", status: "succeeded", finishedAt: now, + }).returning(); + await db.insert(nativeRunFinalizations).values({ + companyId: f.company.id, issueId: f.issue.id, runId: run!.id, phase: "arbitrating", + }); + await start(f); + await quiet(); + expect(f.deps.enqueueWakeup).not.toHaveBeenCalled(); + const settle = (owner: Db) => owner.transaction(async tx => { + await notifyNativeFinalizationWork(tx, { companyId: f.company.id, runId: run!.id, issueId: f.issue.id }); + await tx.update(nativeRunFinalizations).set({ phase }).where(eq(nativeRunFinalizations.runId, run!.id)); + }); + await expect(db.transaction(async tx => { + await settle(tx as unknown as Db); + throw new Error("rollback finalization"); + })).rejects.toThrow("rollback finalization"); + await quiet(); + expect(f.deps.enqueueWakeup).not.toHaveBeenCalled(); + await settle(db); + await advance(31_000); + await vi.waitFor(async () => expect((await readIssue(f.issue.id)).monitorNextCheckAt).toBeNull()); + expect(f.deps.enqueueWakeup).toHaveBeenCalledOnce(); + expect(await readIssue(f.issue.id)).toMatchObject({ monitorAttemptCount: 1 }); + await quiet(); + }); + it.each(["commit", "rollback"] as const)("reconciles worktree activation only after its outer transaction %s", async outcome => { const f = await fixture(); const deadline = now.getTime() + 3_600_000; diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 9156408fc95..9b259f9a554 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -33,7 +33,7 @@ describe("buildMonitorSurfaceCopy", () => { it.each(["retrying", "due-now", "overdue"] as const)("keeps workspace contention neutral when %s", (state) => { const copy = buildMonitorSurfaceCopy(derived({ state, source: "scheduled-retry", nextCheckAt: NOW.toISOString(), attemptCount: 4, - }), NOW, "workspace_busy"); + }), NOW, "workspace_busy", 0, "in_progress", true); expect(copy!.bannerTitle).toBe("Waiting for workspace"); expect(copy!.stripTitle).toBe("Waiting for workspace"); expect(copy!.tone).toBe("info"); @@ -90,6 +90,14 @@ describe("buildMonitorSurfaceCopy", () => { expect(copy!.stripTitle).toBe("Resumes in 1h 30m"); }); + it("keeps blocked checks paused while a run is active", () => { + const copy = buildMonitorSurfaceCopy(derived({ state: "overdue", nextCheckAt: NOW.toISOString() }), + NOW, null, 0, "blocked", true); + expect(copy!.bannerTitle).toBe("Automatic checks paused"); + expect(copy!.executionWait).toBeUndefined(); + expect(copy!.checkUnavailable).toBe(false); + }); + it("switches copy for due-now and overdue states", () => { const dueNow = buildMonitorSurfaceCopy( derived({ state: "due-now", nextCheckAt: NOW.toISOString(), attemptCount: 1 }), @@ -146,6 +154,27 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { } as unknown as Issue; } + it.each([-120_000, 0, 120_000])("explains the active run hold on both monitor surfaces (offset: %s)", offset => { + const issue = { ...issueWithMonitor(new Date(NOW.getTime() + offset).toISOString()), status: "in_progress" as const }; + const onCheckNow = vi.fn(); + const root = createRoot(container); + const render = (hasActiveRun: boolean) => flushSync(() => root.render(<> + + + )); + try { + render(true); + expect(container.textContent).toContain("Monitor waits for current run"); + expect(container.textContent).toContain("The check can start after the current run finishes saving its work."); + expect(container.textContent).not.toMatch(/overdue|next tick|Checking momentarily|wakes the agent now/i); + expect(Array.from(container.querySelectorAll("button")).some(button => button.textContent === "Check now")).toBe(false); + render(false); + expect(container.textContent).not.toContain("Monitor waits for current run"); + expect(Array.from(container.querySelectorAll("button")).filter(button => button.textContent === "Check now")).toHaveLength(2); + if (offset < 0) expect(container.textContent).toContain("overdue by 2m"); + } finally { flushSync(() => root.unmount()); } + }); + it("requires confirmation, keeps the monitor on dismissal, and reports cancellation errors", async () => { const issue = issueWithMonitor(new Date(NOW.getTime() + 60_000).toISOString()); const onCancel = vi.fn().mockRejectedValueOnce(new Error("Unable to save monitor")).mockResolvedValueOnce(undefined); diff --git a/ui/src/components/IssueMonitorBanner.tsx b/ui/src/components/IssueMonitorBanner.tsx index c0382a51f1d..ad1af571352 100644 --- a/ui/src/components/IssueMonitorBanner.tsx +++ b/ui/src/components/IssueMonitorBanner.tsx @@ -67,6 +67,7 @@ export interface MonitorSurfaceCopy { pullRequestReview?: boolean; automaticChecksPaused?: boolean; checkUnavailable?: boolean; + executionWait?: boolean; } function capitalize(value: string): string { @@ -84,6 +85,7 @@ export function buildMonitorSurfaceCopy( scheduledRetryReason?: string | null, pendingPullRequestReviews = 0, issueStatus?: string, + hasActiveRun = false, ): MonitorSurfaceCopy | null { if (!isWaitingMonitorState(derived.state) || !derived.nextCheckAt) return null; @@ -116,6 +118,18 @@ 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" && 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, + }; + } if (derived.source === "monitor" && derived.serviceName?.toLowerCase() === "github" && pendingPullRequestReviews > 0) { return { bannerTitle: "Pull request review requested", @@ -179,7 +193,7 @@ export function buildMonitorSurfaceCopy( }; } -function useMonitorSurfaceCopy(issue: Issue, pendingPullRequestReviews: number): MonitorSurfaceCopy | null { +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. @@ -188,7 +202,7 @@ function useMonitorSurfaceCopy(issue: Issue, pendingPullRequestReviews: number): 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), [issue, now, pendingPullRequestReviews]); + return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason, pendingPullRequestReviews, issue.status, hasActiveRun), [issue, now, pendingPullRequestReviews, hasActiveRun]); } function CheckNowButton({ @@ -254,6 +268,7 @@ function CancelMonitorButton({ onCancel }: { onCancel: () => Promise }) export interface IssueMonitorSurfaceProps { issue: Issue; + hasActiveRun?: boolean; onCheckNow?: (() => void) | null; checkingNow?: boolean; workProducts?: IssueWorkProduct[] | null; @@ -268,6 +283,7 @@ export interface IssueMonitorSurfaceProps { */ export function IssueMonitorBanner({ issue, + hasActiveRun = false, onCheckNow = null, checkingNow = false, workProducts, @@ -275,7 +291,7 @@ export function IssueMonitorBanner({ onCancelMonitor, }: IssueMonitorSurfaceProps) { const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview); - const copy = useMonitorSurfaceCopy(issue, reviews.length); + const copy = useMonitorSurfaceCopy(issue, reviews.length, hasActiveRun); if (!copy) return null; const canCancel = onCancelMonitor && deriveMonitorState(issue).source === "monitor"; @@ -305,6 +321,7 @@ export function IssueMonitorBanner({ */ export function IssueMonitorComposerStrip({ issue, + hasActiveRun = false, onCheckNow = null, checkingNow = false, workProducts, @@ -312,7 +329,7 @@ export function IssueMonitorComposerStrip({ className, }: IssueMonitorSurfaceProps & { className?: string }) { const reviews = getIssuePullRequests(workProducts).filter(pullRequestNeedsReview); - const copy = useMonitorSurfaceCopy(issue, reviews.length); + const copy = useMonitorSurfaceCopy(issue, reviews.length, hasActiveRun); if (!copy) return null; return ( @@ -336,7 +353,7 @@ export function IssueMonitorComposerStrip({

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

diff --git a/ui/src/pages/IssueDetail.tsx b/ui/src/pages/IssueDetail.tsx index d125987d019..4306f6fb770 100644 --- a/ui/src/pages/IssueDetail.tsx +++ b/ui/src/pages/IssueDetail.tsx @@ -8048,6 +8048,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS cancelIssueMonitor.mutateAsync()} issue={issue} workProducts={workProducts} @@ -8629,6 +8630,7 @@ export function TaskDetailSurface({ conversation, tasksTab }: { tasksTab?: TaskS hasVisibleMonitorSurface(issue) ? ( checkIssueMonitorNow.mutate()}