From 21137db805f8e265178d0293709ec0e6e3f3fc15 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Wed, 28 Oct 2026 13:01:11 -0500 Subject: [PATCH] fix: make Check now work for retained monitors on blocked tasks (#18061) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip lets people manage agents and inspect their work. > - Task monitors schedule checks of work that is still pending. > - A task can retain a monitor after it becomes blocked. > - The UI still offered Check now, but the server rejected the task status. > - The UI also claimed that an automatic check would run on the next tick. > - This change admits an explicit human check through the normal execution gates and shows that automatic checks are paused. > - People can request the check without changing task status, and skipped checks no longer report success. ## Linked Issues or Issue Description **What happened?** A blocked task with a retained monitor showed Check now and an overdue countdown. Clicking the button failed because manual checks required an in-progress or in-review task. The endpoint also returned success when admission skipped a check. **Expected behavior** A human can request one check through normal admission. Automatic checks stay paused while the task is blocked. A check that cannot start reports a conflict. **Steps to reproduce** 1. Schedule a monitor on an agent-owned task. 2. Let the task become blocked while it retains the schedule. 3. Open the task and click Check now. Related: #14909 addresses scheduled-retry promotion. This PR addresses retained task monitors. **Paperclip version or commit** Base: 7f10cc058568e3c1ac133cf24f3b4000cbdfca0e. ## What Changed - Allow a human to check a retained monitor on a blocked task. Keep agent and automatic checks limited to active task statuses. - Keep the existing admission path and status guard. Do not change task status to start a check. - Return 409 when admission skips the check. Preserve the schedule when a dependency or recovery hold prevents dispatch. Keep a successful result when a new schedule replaces the consumed schedule after admission. - Show Automatic checks paused in the banner, composer strip and properties. Hide Check now on backlog and todo tasks. Provider-quota recovery stays paused on blocked tasks because it uses a direct retry path; its monitor is retained until the task resumes. - Add database, process, route and UI regression coverage, a Storybook state, API metadata, and behavior documentation. ## Verification - `pnpm -r typecheck` passed. - `pnpm build` passed. - Server scheduling, process admission and route suites: 94 tests passed. - UI monitor, properties and time-formatting suites: 142 tests passed. - Additional admission checks passed: an explicit check starts a process on a blocked task; an unresolved dependency prevents dispatch and preserves the monitor. - Token gates, map checks, Storybook catalog checks and 30 map/catalog tests passed. - The full local `pnpm test:run` attempt reported 1 timeout among 1,621 chat integration tests: “keeps an ambiguous github text part blocked across restart and retries only that part and its tail” exceeded 15 seconds. The exact case passed in isolation on the final head (1 passed, 1,620 skipped). The remaining full-run groups were stopped to isolate that failure. This does not establish a full local pass or explain the initial timeout. Final-head CI is green, including the full chat lanes and the successful single server-shard retry. - Browser inspection passed for the blocked-task banner, composer strip and properties row. Clicking Check now set both visible buttons to disabled Checking state. The preview uses isolated fixtures. - CI server shard 7/12 failed one decision-expiry deadline assertion. The entire unchanged decision-expiry suite passed locally on the final head (20/20). The single retry of that shard passed after the first workflow attempt completed. The initial failure remains recorded here. Neither that test nor its production path changed before retry; the cause is not established by a passing retry. - Greptile reviewed final head ed936f74331816884a1170474984b9bd103a84bd at 5/5. Both prior review threads are resolved. - The first CI chat job stopped after its runner received a shutdown signal. The final head completed a fresh CI run. - The process fixture uses a local no-op adapter. It proves admission and execution, not a live provider's response. The Storybook state simulates dispatch. ## Risks - An explicit human check can start work on a blocked task with no unresolved execution gate. This is intentional. Dependencies, recovery, native finalization, ownership, pause and budgets retain their existing checks. - Skipped manual checks now return 409 instead of 200. The UI already displays API errors in context. - No schema change or migration. ## Model Used OpenAI Codex, based on GPT-6. The session does not expose a more specific served model ID or context-window size. Used repository tools, code execution, and browser inspection. ## 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/PRODUCT.md | 6 +++ doc/SPEC-implementation.md | 1 + maps/agent-access-map/tasks.md | 2 +- maps/feature-map/tasks.md | 7 +++ .../issue-execution-policy-routes.test.ts | 20 ++++++-- .../__tests__/issue-monitor-scheduler.test.ts | 46 ++++++++++++++++-- server/src/routes/issues.ts | 12 ++++- server/src/routes/openapi.ts | 3 +- .../src/services/heartbeat/scheduling.test.ts | 24 +++++++++- server/src/services/heartbeat/scheduling.ts | 19 ++++++-- ui/src/components/IssueMonitorBanner.test.tsx | 45 ++++++++++++++++- ui/src/components/IssueMonitorBanner.tsx | 48 ++++++++++++++----- ui/src/components/IssueProperties.test.tsx | 16 +++++++ .../issue-properties/IssueProperties.tsx | 16 +++++-- ui/src/lib/issue-monitor.ts | 10 ++++ .../monitor-surfaces.stories.tsx | 26 ++++++++++ 16 files changed, 271 insertions(+), 30 deletions(-) diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index c4c28fedf37..8bd41009632 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -93,6 +93,12 @@ Tasks have parentage. Every task exists in service of a parent task, all the way The current issue model includes stable issue identifiers, parent/sub-issues, blockers, a single assignee, comments, issue documents, attachments and work products, and review/approval handoffs. That structure keeps work inspectable by both the board and agents while still allowing agents to decompose work into smaller tasks. +Automatic monitor checks pause when a task is blocked. A person can use Check now +to request one check of a retained monitor through normal execution admission. +Dependencies, recovery, pause, budgets and ownership still apply. Provider-usage +recovery stays paused until the task resumes. A check that +cannot start reports that result instead of claiming it was queued. + The experimental Cases feature also supports persistent work records and prompt-driven agent handoffs. Native case creation requires kickoff and creates one primary task; later handoffs move that same task diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index bac9acad7b9..3b9d918e848 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -767,6 +767,7 @@ V1 non-terminal liveness rule: - A published advisory GitHub review can also remain `in_review` through an authorized external response wait or a saved monitor. Rating, CI state, and merge state alone do not create human attention. The current committed wait is an action path while its assigned endpoint and conversation remain active; it creates no Mine participation. - a blocked chain is covered only when each unresolved leaf issue is live or explicitly waiting - external waits are durable only when persisted as a bounded monitor/scheduled wake, a first-class blocker with a named owner and action, or healthy delegated child work connected by a blocker edge when the source must wait; parent/child structure alone is not a wait path +- automatic monitor checks require an agent-owned task in `in_progress` or `in_review`; an authorized human may request one check of a retained monitor on a `blocked` task through normal admission without changing its status or bypassing dependency, recovery, ownership, pause or budget gates; provider-quota recovery monitors remain paused until the task resumes; a skipped manual dispatch returns a conflict instead of claiming success - unmanaged shell jobs, detached sessions, adapter child processes, local polling loops, PIDs, logs, and comments are evidence rather than liveness; a managed runtime service counts only when paired with a persisted monitor, wake, blocker, or delegated issue that owns the next check - heartbeat finalization evaluates liveness from persisted Paperclip state; an issue cannot remain healthy `in_progress` solely because the exiting heartbeat started a local/background watcher - a continuation cancelled as `issue_continuation_waiting_on_review` first converts a current typed wait target into a first-class wait; without a current target it is classified as `deliberate_wait_without_target` and gives the invokable original owner five normal-model disposition-repair attempts (immediate, then after 60, 120, 240, and 480 seconds, with up to 10 percent jitter) diff --git a/maps/agent-access-map/tasks.md b/maps/agent-access-map/tasks.md index 859c5304653..ce801ef5f2e 100644 --- a/maps/agent-access-map/tasks.md +++ b/maps/agent-access-map/tasks.md @@ -222,7 +222,7 @@ API: `present` — PATCH /api/issues/{id} with executionPolicy.monitor; POST /ap Agent access: `partial` — native set_task_monitor standard-mode binding; permitted agent REST/call_api policy PATCH/check-now; no CLI monitor flags. No REST operation named set_task_monitor. Queue and HTTP/2 sandbox callbacks admit the wake-diagnostics read and monitor check-now request. Controller task-read, monitor-management and assignment checks still apply. Recovery resolution and scheduled-retry controls are outside this callback allowance. See the [callback bridge](../../packages/adapter-utils/src/sandbox-callback-bridge.ts) and [callback boundary tests](../../packages/adapter-utils/src/sandbox-callback-context-routes.test.ts). -Policy: Native nested monitor|null, optional OWNED target taskId, key<=240, notes1..500, future check/timeouts/attempts<=100; reserved quota service denied, externalRef sanitized. Target assigned same agent/no human, in_progress|in_review; issue:read,runtime:manage,responsible-user authority. REST monitor changes require runtime:manage+current assignee for agent; board role gate differs. Receipt replay returns current schedule without rearming consumed monitor. Check-now cannot bypass recovery/finalization/eligibility. +Policy: Native nested monitor|null, optional OWNED target taskId, key<=240, notes1..500, future check/timeouts/attempts<=100; reserved quota service denied, externalRef sanitized. Target assigned same agent/no human, in_progress|in_review; issue:read,runtime:manage,responsible-user authority. REST monitor changes require runtime:manage+current assignee for agent; board role gate differs. Receipt replay returns current schedule without rearming consumed monitor. Human check-now may request one check of a blocked task with a retained monitor; agents and automatic ticks still require in_progress|in_review. This does not change status or bypass dependency, recovery, finalization, pause, budget or ownership gates. Provider-quota recovery uses a direct retry path and stays paused on blocked tasks; check-now rejects it before consuming the schedule. A skipped check-now returns 409 instead of success. Evidence: `source-only` — reviewed at `3bbc2104b65caf9f7763938a01260a5673e12f7a`; [issues.ts](../../server/src/routes/issues.ts), [paperclip-runner-tool-authority.ts](../../server/src/services/native-runtime/paperclip-runner-tool-authority.ts), [issue-monitors.ts](../../server/src/services/issue-monitors.ts), [issue-monitor-scheduler.test.ts](../../server/src/__tests__/issue-monitor-scheduler.test.ts), [task-monitor.integration.test.ts](../../server/src/services/native-runtime/task-monitor.integration.test.ts), [scheduling.test.ts](../../server/src/services/heartbeat/scheduling.test.ts). No live execution claimed. diff --git a/maps/feature-map/tasks.md b/maps/feature-map/tasks.md index 8716319460c..37e39a58ce3 100644 --- a/maps/feature-map/tasks.md +++ b/maps/feature-map/tasks.md @@ -138,6 +138,13 @@ schema feedback supports a corrected call before a wait is claimed. For a due monitor held by execution recovery, confirm that the schedule remains and task activity explains the deferral. Repeated checks must not replay the held work. Resolve the hold through its existing recovery action, then verify one wake. +On a blocked task with a retained monitor, verify that automatic checks show as +paused. Check now requests one check through normal admission without changing +the task status. Confirm that an unresolved dependency or recovery hold prevents +dispatch and does not produce a success message. Backlog and todo tasks must not +offer Check now. Blocked provider-quota recovery must offer no manual check and +retain its schedule until the task resumes. Agents and automatic ticks cannot +check a blocked task. Hold native finalization after the provider has stopped, and verify that both scheduled and manual checks retain the due monitor. After settlement, verify one wake. If its timeout passes during workspace recovery, verify settlement followed diff --git a/server/src/__tests__/issue-execution-policy-routes.test.ts b/server/src/__tests__/issue-execution-policy-routes.test.ts index f759f98659b..26dacf2f1c1 100644 --- a/server/src/__tests__/issue-execution-policy-routes.test.ts +++ b/server/src/__tests__/issue-execution-policy-routes.test.ts @@ -19,7 +19,7 @@ const mockIssueService = vi.hoisted(() => ({ const mockHeartbeatService = vi.hoisted(() => ({ wakeup: vi.fn(async () => undefined), - triggerIssueMonitor: vi.fn(async () => ({ outcome: "triggered" as const })), + triggerIssueMonitor: vi.fn(async (): Promise<{ outcome: "triggered" } | { outcome: "skipped"; reason: string }> => ({ outcome: "triggered" })), reportRunActivity: vi.fn(async () => undefined), getRun: vi.fn(async () => null), getActiveRunForAgent: vi.fn(async () => null), @@ -1157,11 +1157,11 @@ describe("issue execution policy routes", () => { expect(mockHeartbeatService.wakeup).not.toHaveBeenCalled(); }); - it("triggers a scheduled monitor immediately from the dedicated route", async () => { + it.each(["in_progress", "blocked"])("triggers a scheduled monitor on an %s task from the dedicated route", async (status) => { const issue = { id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", companyId: "company-1", - status: "in_progress", + status, assigneeAgentId: "33333333-3333-4333-8333-333333333333", assigneeUserId: null, createdByUserId: "local-board", @@ -1194,6 +1194,20 @@ describe("issue execution policy routes", () => { ); }); + it.each(["execution_reconciliation_required", "monitor_dispatch_deferred", "monitor_replaced"])("does not claim success for a skipped monitor check: %s", async (reason) => { + mockIssueService.getById.mockResolvedValue({ + id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", companyId: "company-1", status: "blocked", + assigneeAgentId: "33333333-3333-4333-8333-333333333333", assigneeUserId: null, + }); + mockHeartbeatService.triggerIssueMonitor.mockResolvedValueOnce({ outcome: "skipped", reason }); + const res = await request(await createApp()) + .post("/api/issues/aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa/monitor/check-now").send({}); + expect(res.status).toBe(409); + expect(res.body).toMatchObject({ details: { code: "issue_monitor_check_deferred", reason } }); + if (reason === "monitor_replaced") expect(res.body.error).toContain("monitor changed"); + expect(res.body.ok).toBeUndefined(); + }); + it("lets a board user create a child issue with a scheduled monitor", async () => { mockIssueService.getById.mockResolvedValue({ id: "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", diff --git a/server/src/__tests__/issue-monitor-scheduler.test.ts b/server/src/__tests__/issue-monitor-scheduler.test.ts index 4cfe7d95801..8c051e38f38 100644 --- a/server/src/__tests__/issue-monitor-scheduler.test.ts +++ b/server/src/__tests__/issue-monitor-scheduler.test.ts @@ -17,6 +17,7 @@ import { heartbeatRunEvents, heartbeatRuns, issueComments, + issueRelations, issueRecoveryActions, issueDocuments, issues, @@ -424,7 +425,7 @@ describeEmbeddedPostgres("issue monitor scheduler", () => { if (change === "cleared") expect(issue.monitorNextCheckAt).toBeNull(); }); - it("does not erase a replacement scheduled after wake admission", async () => { + it.each(["manual", "scheduled"] as const)("reports an accepted %s check and preserves a replacement schedule", async (source) => { const { companyId, issueId } = await seedFixture(); const nextCheckAt = new Date("2026-04-11T13:30:00.000Z"); let replaced = false; @@ -443,7 +444,14 @@ describeEmbeddedPostgres("issue monitor scheduler", () => { }; }, }); - await heartbeatService(racingDb).tickTimers(new Date("2026-04-11T12:31:00.000Z")); + const heartbeat = heartbeatService(racingDb); + const now = new Date("2026-04-11T12:31:00.000Z"); + if (source === "manual") { + expect(await heartbeat.triggerIssueMonitor(issueId, { now, actorType: "user", actorId: "local-board" })) + .toMatchObject({ outcome: "triggered" }); + } else { + expect(await heartbeat.tickTimers(now)).toMatchObject({ enqueued: 1, skipped: 0 }); + } expect(replaced).toBe(true); const [issue] = await db.select().from(issues).where(eq(issues.id, issueId)); expect(issue.monitorNextCheckAt).toEqual(nextCheckAt); @@ -547,8 +555,9 @@ describeEmbeddedPostgres("issue monitor scheduler", () => { expect(participantRuns.find((run) => run.id === scheduled.id)?.errorCode).not.toBe("issue_assignee_changed"); }); - it("lets the board trigger a scheduled issue monitor immediately", async () => { + it.each(["in_progress", "blocked"] as const)("lets the board check a scheduled monitor on an %s task", async (status) => { const { issueId, agentId, nextCheckAt } = await seedFixture(); + await db.update(issues).set({ status }).where(eq(issues.id, issueId)); const heartbeat = heartbeatService(db); const triggeredAt = new Date("2026-04-11T12:00:00.000Z"); @@ -577,6 +586,11 @@ describeEmbeddedPostgres("issue monitor scheduler", () => { nextCheckAt: nextCheckAt.toISOString(), source: "manual", }); + expect(wakeup?.runId).toBeTruthy(); + await waitForHeartbeatIdle(); + const [run] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, wakeup!.runId!)); + expect(run?.status).toBe("succeeded"); + expect(run?.startedAt).not.toBeNull(); const activity = await db .select() @@ -593,6 +607,32 @@ describeEmbeddedPostgres("issue monitor scheduler", () => { }); }); + it("retains a blocked task's monitor when a human check is held by a dependency", async () => { + const { issueId, agentId, companyId, nextCheckAt } = await seedFixture(); + await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, issueId)); + const [blocker] = await db.insert(issues).values({ companyId, title: "Unfinished dependency", status: "todo" }).returning(); + await db.insert(issueRelations).values({ companyId, issueId: blocker.id, relatedIssueId: issueId, type: "blocks" }); + const result = await heartbeatService(db).triggerIssueMonitor(issueId, { actorType: "user", actorId: "local-board" }); + expect(result).toMatchObject({ outcome: "skipped" }); + const [issue] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(issue).toMatchObject({ status: "blocked", monitorNextCheckAt: nextCheckAt, monitorWakeRequestedAt: null, monitorAttemptCount: 0 }); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId))).toHaveLength(0); + }); + + it("does not consume blocked quota recovery when a dependency is unresolved", async () => { + const { issueId, agentId, companyId, nextCheckAt } = await seedFixture({ + monitor: { serviceName: PROVIDER_QUOTA_MONITOR_SERVICE_NAME }, + }); + await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, issueId)); + const [blocker] = await db.insert(issues).values({ companyId, title: "Unfinished dependency", status: "todo" }).returning(); + await db.insert(issueRelations).values({ companyId, issueId: blocker.id, relatedIssueId: issueId, type: "blocks" }); + await expect(heartbeatService(db).triggerIssueMonitor(issueId, { actorType: "user", actorId: "local-board" })) + .rejects.toThrow("Resume this task before checking provider usage."); + const [issue] = await db.select().from(issues).where(eq(issues.id, issueId)); + expect(issue).toMatchObject({ status: "blocked", monitorNextCheckAt: nextCheckAt, monitorWakeRequestedAt: null, monitorAttemptCount: 0 }); + expect(await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.agentId, agentId))).toHaveLength(0); + }); + it("clears due monitors that cannot be dispatched and records a skip", async () => { const { issueId } = await seedFixture({ agentStatus: "paused" }); const heartbeat = heartbeatService(db); diff --git a/server/src/routes/issues.ts b/server/src/routes/issues.ts index 55649ef9f81..9ab7951bd46 100644 --- a/server/src/routes/issues.ts +++ b/server/src/routes/issues.ts @@ -13379,12 +13379,22 @@ export function issueRoutes( ); const actor = getActorInfo(req); - await heartbeat.triggerIssueMonitor(issue.id, { + const result = await heartbeat.triggerIssueMonitor(issue.id, { actorType: actor.actorType, actorId: actor.actorId, agentId: actor.agentId ?? null, runId: actor.runId ?? null, }); + if (result.outcome !== "triggered") { + throw conflict( + result.reason === "execution_reconciliation_required" + ? "The previous run is still being recovered. Try again when recovery finishes." + : result.reason === "monitor_replaced" + ? "The monitor changed during this check. Review the task's current state before trying again." + : "The monitor check did not start. Refresh the task to see its current state.", + { code: "issue_monitor_check_deferred", reason: result.reason }, + ); + } res.json({ ok: true }); }); diff --git a/server/src/routes/openapi.ts b/server/src/routes/openapi.ts index 66f617e2592..eea69ddc69e 100644 --- a/server/src/routes/openapi.ts +++ b/server/src/routes/openapi.ts @@ -10791,7 +10791,8 @@ registerCurrentRoute({ method: "post", path: "/api/issues/{id}/monitor/check-now", tags: ["issues"], - summary: "Run an issue monitor check now", + summary: "Run an issue monitor check now; humans may check blocked tasks", + responses: { 200: r.ok(), 401: r.unauthorized, 403: r.forbidden, 404: r.notFound, 409: r.conflict }, }); registerCurrentRoute({ diff --git a/server/src/services/heartbeat/scheduling.test.ts b/server/src/services/heartbeat/scheduling.test.ts index b07e30ab3c6..26b84d48b82 100644 --- a/server/src/services/heartbeat/scheduling.test.ts +++ b/server/src/services/heartbeat/scheduling.test.ts @@ -168,7 +168,7 @@ describe.skipIf(!postgres.supported)("heartbeat scheduling database boundary", ( await db.update(issues).set({ monitorNextCheckAt: replacement, monitorWakeRequestedAt: null }).where(eq(issues.id, f.issue.id)); return admitted; }); - expect(await f.scheduler.tickTimers(now)).toEqual({ checked: 1, enqueued: 0, skipped: 1 }); + expect(await f.scheduler.tickTimers(now)).toEqual({ checked: 1, enqueued: 1, skipped: 0 }); expect(await readIssue(f.issue.id)).toMatchObject({ monitorNextCheckAt: replacement, monitorAttemptCount: 0 }); }); @@ -197,6 +197,28 @@ describe.skipIf(!postgres.supported)("heartbeat scheduling database boundary", ( expect(await readIssue(f.issue.id)).toMatchObject({ monitorNextCheckAt: null }); }); + it("lets a human check a blocked task without resuming automatic checks", async () => { + const f = await fixture(); + await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, f.issue.id)); + expect(await f.scheduler.tickTimers(now)).toEqual({ checked: 0, enqueued: 0, skipped: 0 }); + expect(f.deps.enqueueWakeup).not.toHaveBeenCalled(); + await expect(f.scheduler.triggerIssueMonitor(f.issue.id, { now, actorType: "user", actorId: "board-user" })) + .resolves.toMatchObject({ outcome: "triggered" }); + expect(f.deps.enqueueWakeup).toHaveBeenCalledExactlyOnceWith(f.agent.id, expect.objectContaining({ + source: "on_demand", requestedByActorType: "user", requestedByActorId: "board-user", + issueStateGuard: expect.objectContaining({ statuses: ["blocked"], assigneeAgentId: f.agent.id }), + })); + expect(await readIssue(f.issue.id)).toMatchObject({ status: "blocked", monitorNextCheckAt: null, monitorAttemptCount: 1 }); + }); + + it.each(["agent", "system"] as const)("does not let a %s check a blocked task", async (actorType) => { + const f = await fixture(); + await db.update(issues).set({ status: "blocked" }).where(eq(issues.id, f.issue.id)); + await expect(f.scheduler.triggerIssueMonitor(f.issue.id, { now, actorType, actorId: "caller" })).rejects.toThrow(); + expect(f.deps.enqueueWakeup).not.toHaveBeenCalled(); + expect(await readIssue(f.issue.id)).toMatchObject({ status: "blocked", monitorNextCheckAt: dueAt, monitorWakeRequestedAt: null }); + }); + it("retains monitors after transient admission failures", async () => { const f = await fixture(); f.deps.enqueueWakeup.mockRejectedValueOnce(new Error("Temporary admission failure")); diff --git a/server/src/services/heartbeat/scheduling.ts b/server/src/services/heartbeat/scheduling.ts index c1714de7b55..34a3821b0bf 100644 --- a/server/src/services/heartbeat/scheduling.ts +++ b/server/src/services/heartbeat/scheduling.ts @@ -792,7 +792,9 @@ export function createHeartbeatScheduling( }) .where(issueMonitorClaimCondition(claimed)) .returning({ id: issues.id }); - if (consumed.length === 0) return { outcome: "skipped" as const, reason: "monitor_replaced" }; + // Admission already accepted the check. A replacement schedule must stay + // intact, but losing its consumption fence does not undo that accepted wake. + if (consumed.length === 0) return { outcome: "triggered" as const }; await logActivity(db, { companyId: claimed.companyId, @@ -907,7 +909,18 @@ export function createHeartbeatScheduling( if (!issue.assigneeAgentId || issue.assigneeUserId) { throw conflict("Issue monitor requires an agent assignee"); } - if (!["in_progress", "in_review"].includes(issue.status)) { + if (issue.status === "blocked" && actorType === "user" && + normalizeIssueExecutionPolicy(issue.executionPolicy ?? null)?.monitor?.serviceName === PROVIDER_QUOTA_MONITOR_SERVICE_NAME) { + // Quota recovery schedules a bounded retry directly, rather than using + // ordinary wake admission. It must not consume a blocked task's monitor. + throw conflict("Resume this task before checking provider usage.", { code: "blocked_quota_monitor" }); + } + // A human can request one check of a blocked task. Admission still owns + // dependencies, recovery, pause and budget gates; automatic ticks stay idle. + const allowedStatuses = actorType === "user" + ? ["in_progress", "in_review", "blocked"] + : ["in_progress", "in_review"]; + if (!allowedStatuses.includes(issue.status)) { throw conflict( "Issue monitor can only run while the issue is in progress or in review", ); @@ -928,7 +941,7 @@ export function createHeartbeatScheduling( sql`${issues.monitorNextCheckAt} is not null`, isNull(issues.assigneeUserId), sql`${issues.assigneeAgentId} is not null`, - inArray(issues.status, ["in_progress", "in_review"]), + inArray(issues.status, allowedStatuses), or( isNull(issues.monitorWakeRequestedAt), lt(issues.monitorWakeRequestedAt, staleClaimThreshold), diff --git a/ui/src/components/IssueMonitorBanner.test.tsx b/ui/src/components/IssueMonitorBanner.test.tsx index 5680dc963af..9156408fc95 100644 --- a/ui/src/components/IssueMonitorBanner.test.tsx +++ b/ui/src/components/IssueMonitorBanner.test.tsx @@ -3,7 +3,7 @@ import { act } from "react"; import { flushSync } from "react-dom"; import { createRoot } from "react-dom/client"; -import type { Issue, IssueWorkProduct } from "@paperclipai/shared"; +import { PROVIDER_QUOTA_MONITOR_SERVICE_NAME, type Issue, type IssueWorkProduct } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { @@ -260,6 +260,49 @@ describe("IssueMonitorBanner / IssueMonitorComposerStrip rendering", () => { flushSync(() => root.unmount()); }); + it("offers a manual check on a blocked task without promising an automatic wake", () => { + const onCheckNow = vi.fn(); + const issue = { ...issueWithMonitor(new Date(NOW.getTime() - 12 * 60_000).toISOString()), status: "blocked" as const }; + const root = createRoot(container); + flushSync(() => root.render(<> + + + )); + expect(container.textContent).toContain("Automatic checks paused"); + expect(container.textContent).not.toMatch(/overdue|next tick|scheduled check/i); + const buttons = Array.from(container.querySelectorAll("button")); + expect(buttons.map(button => button.textContent)).toEqual(["Check now", "Check now"]); + flushSync(() => buttons[1]!.click()); + expect(onCheckNow).toHaveBeenCalledTimes(1); + flushSync(() => root.unmount()); + }); + + it.each(["backlog", "todo"] as const)("does not offer a monitor check on a %s task", (status) => { + const issue = { ...issueWithMonitor(NOW.toISOString()), status }; + const root = createRoot(container); + flushSync(() => root.render(<> + + + )); + expect(container.textContent).toContain("Resume this task to continue checking."); + expect(container.querySelector("button")).toBeNull(); + expect(container.textContent).not.toMatch(/next tick|momentarily|reply wakes/i); + flushSync(() => root.unmount()); + }); + + it("keeps blocked quota recovery paused until the task resumes", () => { + const issue = { ...issueWithMonitor(NOW.toISOString()), status: "blocked" as const }; + issue.executionState!.monitor!.serviceName = PROVIDER_QUOTA_MONITOR_SERVICE_NAME; + const root = createRoot(container); + flushSync(() => root.render(<> + + + )); + expect(container.textContent).toContain("Resume this task to continue checking."); + expect(container.querySelector("button")).toBeNull(); + flushSync(() => root.unmount()); + }); + it("hides the banner and strip when there is no monitor", () => { expect(hasVisibleMonitorSurface(issueWithMonitor(null))).toBe(false); const root = createRoot(container); diff --git a/ui/src/components/IssueMonitorBanner.tsx b/ui/src/components/IssueMonitorBanner.tsx index 78f36b8ca11..01021dd1c3a 100644 --- a/ui/src/components/IssueMonitorBanner.tsx +++ b/ui/src/components/IssueMonitorBanner.tsx @@ -18,6 +18,8 @@ import { IssuePullRequestLinks } from "@/components/IssuePullRequestLinks"; import { getIssuePullRequests, pullRequestNeedsReview } from "@/lib/issue-pull-requests"; import { cn } from "@/lib/utils"; import { + areAutomaticMonitorChecksPaused, + canCheckIssueMonitorNow, deriveMonitorState, formatMonitorAbsolute, formatMonitorEta, @@ -63,6 +65,8 @@ export interface MonitorSurfaceCopy { tone: "info" | "warning"; workspaceWait?: boolean; pullRequestReview?: boolean; + automaticChecksPaused?: boolean; + checkUnavailable?: boolean; } function capitalize(value: string): string { @@ -79,9 +83,26 @@ export function buildMonitorSurfaceCopy( now: MonitorDate, scheduledRetryReason?: string | null, pendingPullRequestReviews = 0, + issueStatus?: string, ): 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", @@ -162,9 +183,12 @@ function useMonitorSurfaceCopy(issue: Issue, pendingPullRequestReviews: number): // `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 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, now, pendingPullRequestReviews]); + return useMemo(() => buildMonitorSurfaceCopy(deriveMonitorState(issue, now), now, issue.scheduledRetry?.scheduledRetryReason, pendingPullRequestReviews, issue.status), [issue, now, pendingPullRequestReviews]); } function CheckNowButton({ @@ -262,7 +286,7 @@ export function IssueMonitorBanner({ icon={Clock} title={copy.bannerTitle} className={cn("relative my-3", canCancel && "pr-12")} - actions={onCheckNow && !copy.workspaceWait ? : null} + actions={onCheckNow && !copy.workspaceWait && !copy.checkUnavailable ? : null} > {canCancel ? : null}
@@ -306,15 +330,17 @@ export function IssueMonitorComposerStrip({
{copy.stripMeta.join(" · ")}
- {onCheckNow && !copy.workspaceWait ? : null} + {onCheckNow && !copy.workspaceWait && !copy.checkUnavailable ? : null} -

- {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."} -

+ {!copy.automaticChecksPaused ? ( +

+ {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."} +

+ ) : null} {checkError ?

{checkError}

: null} ); diff --git a/ui/src/components/IssueProperties.test.tsx b/ui/src/components/IssueProperties.test.tsx index cb981180726..5ed4d721649 100644 --- a/ui/src/components/IssueProperties.test.tsx +++ b/ui/src/components/IssueProperties.test.tsx @@ -3032,6 +3032,7 @@ describe("IssueProperties", () => { }; renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, serviceName: "vercel-deploy" } }), executionState: createExecutionState({ monitor: baseMonitorState }), monitorAttemptCount: 1, @@ -3045,6 +3046,7 @@ describe("IssueProperties", () => { expect(monitorRowText()).toContain("Today, 4:08 PM · Attempt 1"); renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, nextCheckAt: at(252) } }), executionState: createExecutionState({ monitor: { ...baseMonitorState, nextCheckAt: at(132) } }), monitorNextCheckAt: new Date(at(192)), @@ -3054,6 +3056,7 @@ describe("IssueProperties", () => { expect(monitorRowText()).toContain("Today, 4:08 PM"); renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, serviceName: "vercel-deploy" } }), executionState: createExecutionState({ monitor: { ...baseMonitorState, attemptCount: 3 } }), monitorAttemptCount: 3, @@ -3062,6 +3065,7 @@ describe("IssueProperties", () => { expect(monitorRowText()).toContain("Attempt 3"); renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, nextCheckAt: at(0) } }), executionState: createExecutionState({ monitor: { ...baseMonitorState, nextCheckAt: at(0) } }), })); @@ -3070,6 +3074,7 @@ describe("IssueProperties", () => { expect(monitorRowText()).toContain("checking momentarily…"); renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, nextCheckAt: at(-18) } }), executionState: createExecutionState({ monitor: { ...baseMonitorState, nextCheckAt: at(-18) } }), })); @@ -3078,6 +3083,17 @@ describe("IssueProperties", () => { expect(monitorRowText()).toContain("Today, 1:38 PM · fires on next tick"); renderMonitor(createIssue({ + status: "blocked", + executionPolicy: createExecutionPolicy({ monitor: { ...baseMonitorState, nextCheckAt: at(-18) } }), + executionState: createExecutionState({ monitor: { ...baseMonitorState, nextCheckAt: at(-18) } }), + })); + await flush(); + expect(monitorRowText()).toContain("Automatic checks paused"); + expect(monitorRowText()).toContain("Check now to request a manual check"); + expect(monitorRowText()).not.toMatch(/overdue|next tick/i); + + renderMonitor(createIssue({ + status: "in_progress", executionPolicy: createExecutionPolicy(), executionState: createExecutionState({ monitor: { ...baseMonitorState, diff --git a/ui/src/components/issue-properties/IssueProperties.tsx b/ui/src/components/issue-properties/IssueProperties.tsx index 924c7c8eb05..91ccdb43932 100644 --- a/ui/src/components/issue-properties/IssueProperties.tsx +++ b/ui/src/components/issue-properties/IssueProperties.tsx @@ -50,6 +50,8 @@ import { formatAssigneeUserLabel, formatUserLabel } from "../../lib/assignees"; import { buildExecutionPolicy, stageParticipantValues } from "../../lib/issue-execution-policy"; import { useExecutionPolicy } from "../../hooks/useExecutionPolicy"; import { + areAutomaticMonitorChecksPaused, + canCheckIssueMonitorNow, formatMonitorAbsolute, formatMonitorAbsoluteFull, formatMonitorEta, @@ -1323,17 +1325,21 @@ export function IssueProperties({ const monitorLastTriggeredAt = issue.monitorLastTriggeredAt ?? monitorState?.lastTriggeredAt ?? null; const monitorServiceName = executionPolicy?.monitor?.serviceName ?? monitorState?.serviceName ?? null; const monitorNotes = executionPolicy?.monitor?.notes ?? monitorState?.notes ?? null; - const monitorNow = useMonitorCountdown(monitorNextCheckAt); + const monitorPaused = areAutomaticMonitorChecksPaused(issue); + const canCheckMonitorNow = canCheckIssueMonitorNow(issue, monitorServiceName); + const monitorNow = useMonitorCountdown(monitorPaused ? null : monitorNextCheckAt); const monitorRelative = monitorNextCheckAt ? formatMonitorEta(monitorNextCheckAt, monitorNow) : null; const monitorIsDueNow = monitorRelative === "due now"; const monitorIsOverdue = Boolean(monitorRelative?.startsWith("overdue by ")); const monitorPrimary = monitorNextCheckAt - ? formatMonitorEtaLabel(monitorNextCheckAt, monitorNow) + ? monitorPaused ? "Automatic checks paused" : formatMonitorEtaLabel(monitorNextCheckAt, monitorNow) : monitorState?.status === "cleared" ? "Cleared" : "None"; const monitorSecondary = monitorNextCheckAt - ? monitorIsDueNow + ? monitorPaused + ? canCheckMonitorNow ? "Check now to request a manual check" : "Resume this task to continue checking" + : monitorIsDueNow ? "checking momentarily…" : `${formatMonitorAbsolute(monitorNextCheckAt, {}, monitorNow)}${monitorIsOverdue ? " · fires on next tick" : monitorAttemptCount > 0 ? ` · Attempt ${monitorAttemptCount}` : ""}` : monitorState?.status === "cleared" @@ -1376,7 +1382,7 @@ export function IssueProperties({
Next check
{formatMonitorAbsoluteFull(monitorNextCheckAt)}
-
{monitorRelative}
+
{monitorPaused ? "Automatic checks paused" : monitorRelative}
Watching
@@ -1392,7 +1398,7 @@ export function IssueProperties({
- {onCheckMonitorNow ? ( + {onCheckMonitorNow && canCheckMonitorNow ? ( diff --git a/ui/src/lib/issue-monitor.ts b/ui/src/lib/issue-monitor.ts index 3a313b1ba17..90eecaeb9c4 100644 --- a/ui/src/lib/issue-monitor.ts +++ b/ui/src/lib/issue-monitor.ts @@ -1,4 +1,5 @@ import { useEffect, useState } from "react"; +import { PROVIDER_QUOTA_MONITOR_SERVICE_NAME } from "@paperclipai/shared"; const SECOND_MS = 1_000; const MINUTE_MS = 60 * SECOND_MS; @@ -56,6 +57,15 @@ export interface MonitorDateTimeFormatOptions { timeZone?: string; } +export function areAutomaticMonitorChecksPaused(issue: Pick): boolean { + return issue.status !== undefined && issue.status !== "in_progress" && issue.status !== "in_review"; +} + +export function canCheckIssueMonitorNow(issue: Pick, serviceName?: string | null): boolean { + return !areAutomaticMonitorChecksPaused(issue) || + (issue.status === "blocked" && serviceName !== PROVIDER_QUOTA_MONITOR_SERVICE_NAME); +} + function toTimestamp(value: MonitorDate): number { const timestamp = new Date(value).getTime(); if (Number.isNaN(timestamp)) throw new RangeError("Invalid monitor date"); diff --git a/ui/storybook/stories/task-coordination/monitor-surfaces.stories.tsx b/ui/storybook/stories/task-coordination/monitor-surfaces.stories.tsx index 35585c557b3..c74133c5ee0 100644 --- a/ui/storybook/stories/task-coordination/monitor-surfaces.stories.tsx +++ b/ui/storybook/stories/task-coordination/monitor-surfaces.stories.tsx @@ -1,7 +1,9 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; +import { useState } from "react"; import type { Issue } from "@paperclipai/shared"; import { IssueMonitorBanner, IssueMonitorComposerStrip } from "@/components/IssueMonitorBanner"; import { IssueProperties } from "@/components/IssueProperties"; +import { prepareFixtureApi } from "../../support/capability-gaps/api"; import { storybookExecutionWorkspaces, storybookIssueDocuments, @@ -335,3 +337,27 @@ export default meta; type Story = StoryObj; export const MonitorSurfaces: Story = {}; + +export const BlockedTask: Story = { + beforeEach: () => prepareFixtureApi([ + { path: `/api/issues/${overdueMonitorIssue.id}/work-products`, read: () => [] }, + ]).cleanup, + render: function BlockedMonitor() { + const [checkingNow, setCheckingNow] = useState(false); + const issue = { ...overdueMonitorIssue, status: "blocked" as const }; + const onCheckNow = () => setCheckingNow(true); + return
+ + + undefined} onCheckMonitorNow={onCheckNow} checkingMonitorNow={checkingNow} inline /> +
; + }, + parameters: { + paperclip: { + capabilities: ["task-coordination/tree-control", "tasks/properties"], + implementation: "production", + boundary: "fixtures", + notes: "A retained monitor on a blocked task. Automatic checks are paused. Clicking Check now shows the shared pending state; dispatch is simulated.", + }, + }, +};