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 847d00bdc3.
**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 <noreply@paperclip.ing>
This commit is contained in:
parent
2fc5e3e43b
commit
2904a3a6cc
|
|
@ -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.
|
||||
|
|
|
|||
|
|
@ -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(
|
||||
<>
|
||||
<IssueMonitorBanner issue={next} onCheckNow={vi.fn()} />
|
||||
<IssueMonitorComposerStrip issue={next} onCheckNow={vi.fn()} />
|
||||
</>,
|
||||
));
|
||||
|
||||
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());
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue