From 2904a3a6ccc6b2d76fe2886057d15dcf6290af9d Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Fri, 11 Sep 2026 15:05:07 -0500 Subject: [PATCH] fix(ui): hide retry countdown after execution starts (#13258) ## Thinking Path > - Paperclip helps operators manage AI agents and their tasks. > - Task pages show countdowns for deferred checks and automatic retries. > - A retry keeps its scheduled start time after it enters the queue or starts running. > - The countdown treated that historical time as a pending deadline and showed an overdue warning beside active work. > - This pull request limits retry countdowns to retries that are still scheduled and hides waiting surfaces on terminal tasks. > - Operators now see a warning only when the displayed retry is still waiting to start. ## Linked Issues or Issue Description Refs #9783, which added the monitor surfaces. Searched related PRs and issues; no duplicate fix was found. **What happened?** After a service restart resumed a task through an automatic retry, the task showed an overdue retry banner while the agent was running. The banner also remained when the task became done. **Expected behavior** A queued or running retry must not show a countdown against its past scheduled start time. Done and cancelled tasks must not show waiting banners. **Steps to reproduce** 1. Open a task with an automatic retry scheduled for a known time. 2. Let the retry enter the queue and start running. 3. Wait until its scheduled time is more than one minute in the past. 4. Observe the overdue banner and Check now button while the agent is working. **Paperclip version or commit** Reproduced on source commit 847d00bdc37bdcb13bb3e7d93b0bd5dc239deda1. **Deployment mode** Self-hosted server built from source. This is a core UI bug. ## What Changed - Derive a retry countdown only when the retry status is `scheduled_retry`. - Ignore retained retry times for queued, running, and cancelled retries. - Hide waiting banners for done and cancelled tasks. - Keep a separate scheduled monitor visible on an open task. - Add state and rendered-transition regression tests. Document the display rule. - Stub the process start time in one restart-recovery test. CI exposed that the fixture read real host metadata for its fake PID. ## Verification - Focused monitor tests: 34 passed on the rebased commit. - Token gates passed on the rebased commit. - `pnpm -r typecheck` and `pnpm build` passed on the rebased commit. The full local test run was stopped after embedded PostgreSQL failed to load missing macOS library aliases. After repairing the local dependency aliases, the 11-test native status corpus passed. CI exposed an unrelated restart-recovery fixture that read real host process metadata. A test-only fix removes that host dependency. All 31 CI checks passed on the final commit; the two optional Storybook jobs were skipped. - The runner transport test that timed out in the first CI run passed locally: 2 tests passed. The final CI Build job passed. - Restart-recovery fixture suite: 20 tests passed after the test-only change. - Greptile reviewed final commit `b815ac97d36cfcb6d09f31c852465b6474f78fea`: 5/5, with no open review threads. - The rendering regression checks waiting, queued, running, rescheduled, and done states. It also checks that hidden surfaces remove their buttons and countdown timers. ## Risks Low risk. This changes display state only. It does not change retry dispatch, monitor scheduling, database records, or API contracts. A retry that is still scheduled retains its overdue warning. ## Model Used OpenAI GPT-6 through Codex. Used reasoning, repository inspection, code editing, browser inspection, and test execution. The exact runtime model identifier 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 --- doc/execution-semantics.md | 6 +++ ui/src/components/IssueMonitorBanner.test.tsx | 38 +++++++++++++++++++ ui/src/lib/issue-monitor.test.tsx | 29 ++++++++++++++ ui/src/lib/issue-monitor.ts | 24 +++++++----- 4 files changed, 87 insertions(+), 10 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index a9cf075ffc..accf02b8e6 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -499,6 +499,12 @@ Monitor policy lives under `executionPolicy.monitor` and includes: Monitors are not recurring intervals. When a monitor fires, Paperclip clears the scheduled monitor and queues an `issue_monitor_due` wake for the assignee. If the external service is still pending, the assignee must explicitly re-arm the monitor with a new `nextCheckAt`. If the issue moves to `done`, `cancelled`, an invalid status, or a human/unassigned owner, the monitor is cleared. +The task's waiting banner and composer countdown also display automatic retries +while their run is `scheduled_retry`. Once a retry is `queued` or `running`, its +retained `scheduledRetryAt` is historical and must not produce a waiting or overdue +warning. A separately scheduled monitor remains visible. Completed and cancelled +tasks hide both waiting surfaces even if a stale schedule remains in the response. + Because `serviceName` and `notes` remain visible in issue activity and wake context, operators should keep them short and non-secret. Put enough context for the assignee to know what to inspect, but do not include signed URLs, bearer tokens, customer secrets, tenant-private identifiers, or provider links with embedded credentials. Monitor bounds are enforced. Paperclip rejects attempts to re-arm a monitor whose `timeoutAt` or `maxAttempts` is already exhausted. When a scheduled monitor reaches an exhausted bound at trigger time, Paperclip clears it and follows `recoveryPolicy`: `wake_owner` queues a bounded recovery wake for the assignee, `create_recovery_issue` opens visible issue-backed recovery work, and `escalate_to_board` records a board-visible escalation comment/activity. diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 53c9f0fc7b..0a8bd470c1 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -193,4 +193,42 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { flushSync(() => root.unmount()); }); + + it("removes both countdowns and Check now when the retry starts, then shows a newly scheduled retry", () => { + const root = createRoot(container); + const issue = { + status: "in_progress", + scheduledRetry: { + status: "scheduled_retry", + scheduledRetryAt: new Date(NOW.getTime() - 2 * 60_000).toISOString(), + scheduledRetryAttempt: 1, + }, + } as Issue; + const render = (next: Issue) => flushSync(() => root.render( + <> + + + , + )); + + render(issue); + expect(container.textContent).toContain("Overdue by 2m"); + + for (const status of ["queued", "running"] as const) { + const promoted = { ...issue, scheduledRetry: { ...issue.scheduledRetry!, status } }; + render(promoted); + expect(hasVisibleMonitorSurface(promoted)).toBe(false); + expect(container.textContent).toBe(""); + expect(container.querySelector("button")).toBeNull(); + expect(vi.getTimerCount()).toBe(0); + } + + render({ ...issue, scheduledRetry: { ...issue.scheduledRetry!, scheduledRetryAt: new Date(NOW.getTime() + 5 * 60_000).toISOString() } }); + expect(container.textContent).toContain("Resumes in 5m"); + + render({ ...issue, status: "done" }); + expect(container.textContent).toBe(""); + expect(vi.getTimerCount()).toBe(0); + flushSync(() => root.unmount()); + }); }); diff --git a/ui/src/lib/issue-monitor.test.tsx b/ui/src/lib/issue-monitor.test.tsx index 84b7ced040..82526fb4af 100644 --- a/ui/src/lib/issue-monitor.test.tsx +++ b/ui/src/lib/issue-monitor.test.tsx @@ -140,6 +140,35 @@ describe("deriveMonitorState", () => { expect(deriveMonitorState(issue("2026-07-17T19:59:00.000Z"), now).state).toBe("overdue"); }); + it.each(["queued", "running", "cancelled"] as const)("ignores a %s retry's historical start time", (status) => { + const issue = { + status: "in_progress", + scheduledRetry: { + status, + scheduledRetryAt: "2026-07-17T19:58:00.000Z", + scheduledRetryAttempt: 1, + }, + }; + + expect(deriveMonitorState(issue, now)).toMatchObject({ state: "none", nextCheckAt: null }); + // A separate, explicitly scheduled monitor must still be visible. + expect(deriveMonitorState({ ...issue, monitorNextCheckAt: "2026-07-17T20:05:00.000Z" }, now)) + .toMatchObject({ state: "scheduled", source: "monitor" }); + }); + + it("keeps overdue warnings for retries that have not been promoted", () => { + expect(deriveMonitorState({ + scheduledRetry: { status: "scheduled_retry", scheduledRetryAt: "2026-07-17T19:58:00.000Z" }, + }, now)).toMatchObject({ state: "overdue", source: "scheduled-retry" }); + }); + + it.each(["done", "cancelled"])("ignores stale monitor and retry schedules on %s tasks", (status) => { + const scheduledRetry = { status: "scheduled_retry" as const, scheduledRetryAt: "2026-07-17T19:58:00.000Z" }; + expect(deriveMonitorState({ status, scheduledRetry }, now)).toMatchObject({ state: "none", nextCheckAt: null }); + expect(deriveMonitorState({ status, monitorNextCheckAt: scheduledRetry.scheduledRetryAt }, now)) + .toMatchObject({ state: "none", nextCheckAt: null }); + }); + it("derives cleared, none, and scheduled retry states", () => { expect( deriveMonitorState({ executionState: { monitor: { status: "cleared", attemptCount: 2 } } }, now), diff --git a/ui/src/lib/issue-monitor.ts b/ui/src/lib/issue-monitor.ts index adc6f2a476..3a313b1ba1 100644 --- a/ui/src/lib/issue-monitor.ts +++ b/ui/src/lib/issue-monitor.ts @@ -27,6 +27,7 @@ type ScheduledRetry = { }; export interface MonitorIssueLike { + status?: string; executionState?: { monitor?: MonitorDetails | null } | null; executionPolicy?: { monitor?: MonitorPolicy | null } | null; monitorNextCheckAt?: MonitorDate | null; @@ -173,25 +174,28 @@ export function formatMonitorAbsoluteFull( } export function deriveMonitorState(issue: MonitorIssueLike, now: MonitorDate = new Date()): DerivedMonitorState { + if (issue.status === "done" || issue.status === "cancelled") { + return { state: "none", source: "none", nextCheckAt: null, attemptCount: 0, serviceName: null }; + } + const runtimeMonitor = issue.executionState?.monitor ?? null; const policyMonitor = issue.executionPolicy?.monitor ?? null; const scheduledRetry = issue.scheduledRetry ?? null; - const retryIsActive = - scheduledRetry?.status === "scheduled_retry" || - scheduledRetry?.status === "queued" || - scheduledRetry?.status === "running"; + // Promotion preserves scheduledRetryAt as history. Once queued or running, + // the retry is no longer waiting for that timestamp and cannot be overdue. + const retryIsScheduled = scheduledRetry?.status === "scheduled_retry"; const nextCheckAt = runtimeMonitor?.nextCheckAt ?? issue.monitorNextCheckAt ?? policyMonitor?.nextCheckAt ?? - (retryIsActive ? scheduledRetry?.scheduledRetryAt : null) ?? + (retryIsScheduled ? scheduledRetry?.scheduledRetryAt : null) ?? null; const hasMonitor = runtimeMonitor !== null || policyMonitor !== null || issue.monitorNextCheckAt != null; - const source = hasMonitor ? "monitor" : retryIsActive ? "scheduled-retry" : "none"; + const source = hasMonitor ? "monitor" : retryIsScheduled ? "scheduled-retry" : "none"; const attemptCount = runtimeMonitor?.attemptCount ?? (hasMonitor ? issue.monitorAttemptCount : null) ?? - (retryIsActive ? scheduledRetry?.scheduledRetryAttempt : null) ?? + (retryIsScheduled ? scheduledRetry?.scheduledRetryAttempt : null) ?? 0; const serviceName = runtimeMonitor?.serviceName ?? policyMonitor?.serviceName ?? null; @@ -199,11 +203,11 @@ export function deriveMonitorState(issue: MonitorIssueLike, now: MonitorDate = n return { state: "cleared", source, nextCheckAt, attemptCount, serviceName }; } - if (!hasMonitor && !retryIsActive) { + if (!hasMonitor && !retryIsScheduled) { return { state: "none", source, nextCheckAt: null, attemptCount: 0, serviceName: null }; } if (!nextCheckAt) { - return { state: retryIsActive || attemptCount > 1 ? "retrying" : "scheduled", source, nextCheckAt, attemptCount, serviceName }; + return { state: retryIsScheduled || attemptCount > 1 ? "retrying" : "scheduled", source, nextCheckAt, attemptCount, serviceName }; } const deltaMs = toTimestamp(nextCheckAt) - toTimestamp(now); @@ -214,7 +218,7 @@ export function deriveMonitorState(issue: MonitorIssueLike, now: MonitorDate = n return { state: "due-now", source, nextCheckAt, attemptCount, serviceName }; } return { - state: retryIsActive || attemptCount > 1 ? "retrying" : "scheduled", + state: retryIsScheduled || attemptCount > 1 ? "retrying" : "scheduled", source, nextCheckAt, attemptCount,