mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 11:01:16 +01:00
fix: make Check now work for retained monitors on blocked tasks (#18061)
## 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:7f10cc0585. ## 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 headed936f7433at 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 <noreply@paperclip.ing>
This commit is contained in:
1 parent
d30c8f42b8
commit
21137db805
16 files changed
+271
-30
No files matched your search
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 });
|
||||
});
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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"));
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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(<>
|
||||
<IssueMonitorBanner issue={issue} onCheckNow={onCheckNow} />
|
||||
<IssueMonitorComposerStrip issue={issue} onCheckNow={onCheckNow} />
|
||||
</>));
|
||||
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(<>
|
||||
<IssueMonitorBanner issue={issue} onCheckNow={vi.fn()} />
|
||||
<IssueMonitorComposerStrip issue={issue} onCheckNow={vi.fn()} />
|
||||
</>));
|
||||
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(<>
|
||||
<IssueMonitorBanner issue={issue} onCheckNow={vi.fn()} />
|
||||
<IssueMonitorComposerStrip issue={issue} onCheckNow={vi.fn()} />
|
||||
</>));
|
||||
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);
|
||||
|
||||
@@ -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 ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
|
||||
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">
|
||||
@@ -306,15 +330,17 @@ export function IssueMonitorComposerStrip({
|
||||
<div className="text-xs text-muted-foreground">{copy.stripMeta.join(" · ")}</div>
|
||||
</div>
|
||||
</div>
|
||||
{onCheckNow && !copy.workspaceWait ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
|
||||
{onCheckNow && !copy.workspaceWait && !copy.checkUnavailable ? <CheckNowButton onCheckNow={onCheckNow} checkingNow={checkingNow} reviewRequested={copy.pullRequestReview} /> : null}
|
||||
</div>
|
||||
<p className="mt-1.5 text-xs text-muted-foreground">
|
||||
{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."}
|
||||
</p>
|
||||
{!copy.automaticChecksPaused ? (
|
||||
<p className="mt-1.5 text-xs text-muted-foreground">
|
||||
{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."}
|
||||
</p>
|
||||
) : null}
|
||||
{checkError ? <p role="alert" className="text-sm text-destructive">{checkError}</p> : null}
|
||||
</div>
|
||||
);
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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({
|
||||
<div>
|
||||
<div className="text-xs text-muted-foreground">Next check</div>
|
||||
<div className="text-sm">{formatMonitorAbsoluteFull(monitorNextCheckAt)}</div>
|
||||
<div className="text-xs text-muted-foreground">{monitorRelative}</div>
|
||||
<div className="text-xs text-muted-foreground">{monitorPaused ? "Automatic checks paused" : monitorRelative}</div>
|
||||
</div>
|
||||
<div>
|
||||
<div className="text-xs text-muted-foreground">Watching</div>
|
||||
@@ -1392,7 +1398,7 @@ export function IssueProperties({
|
||||
</div>
|
||||
</div>
|
||||
<div className="flex gap-2 border-t border-border px-4 py-3">
|
||||
{onCheckMonitorNow ? (
|
||||
{onCheckMonitorNow && canCheckMonitorNow ? (
|
||||
<Button type="button" size="sm" variant="outline" disabled={checkingMonitorNow} onClick={() => { setMonitorDetailsOpen(false); onCheckMonitorNow(); }}>
|
||||
{checkingMonitorNow ? "Checking…" : "Check now"}
|
||||
</Button>
|
||||
|
||||
@@ -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<MonitorIssueLike, "status">): boolean {
|
||||
return issue.status !== undefined && issue.status !== "in_progress" && issue.status !== "in_review";
|
||||
}
|
||||
|
||||
export function canCheckIssueMonitorNow(issue: Pick<MonitorIssueLike, "status">, 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");
|
||||
|
||||
@@ -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<typeof meta>;
|
||||
|
||||
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 <div className="space-y-4">
|
||||
<IssueMonitorBanner issue={issue} onCheckNow={onCheckNow} checkingNow={checkingNow} />
|
||||
<IssueMonitorComposerStrip issue={issue} onCheckNow={onCheckNow} checkingNow={checkingNow} />
|
||||
<IssueProperties issue={issue} onUpdate={() => undefined} onCheckMonitorNow={onCheckNow} checkingMonitorNow={checkingNow} inline />
|
||||
</div>;
|
||||
},
|
||||
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.",
|
||||
},
|
||||
},
|
||||
};
|
||||
Reference in new issue
Block a user