diff --git a/doc/PRODUCT.md b/doc/PRODUCT.md index 99d897a9aa1..a538481b335 100644 --- a/doc/PRODUCT.md +++ b/doc/PRODUCT.md @@ -97,6 +97,12 @@ When an agent creates a subtask or follow-up task, its source task's feed shows a linked card at creation time with the current title, task number, and status. The Tasks tab keeps the full relationship list. +Agent-owned tasks show **In Progress** only while a task-linked run is queued +or running. When execution ends, their visible status reflects recovery +(**Blocked**), a pending response or review (**In Review**), monitoring, or +remaining work (**Todo**). The stored lifecycle status still controls execution +and recovery. Human-owned tasks retain their selected ownership status. + Tasks waiting between automatic checks show **Monitoring** with a blue clock. Task lists, board columns, filters, search, recent navigation and task properties use the same read-only display state. The task retains its execution lifecycle diff --git a/doc/SPEC-implementation.md b/doc/SPEC-implementation.md index 5d6e83a6700..e607a41a137 100644 --- a/doc/SPEC-implementation.md +++ b/doc/SPEC-implementation.md @@ -472,6 +472,7 @@ Invariants: - single assignee only - task must trace to company goal chain via `goal_id`, `parent_id`, or project-goal linkage - `in_progress` requires assignee +- agent task presentation reserves **In Progress** for a task-linked queued or running run; inactive tasks project recovery as **Blocked**, pending responses/review as **In Review**, scheduled checks as **Monitoring**, or remaining work as **Todo** without changing lifecycle/admission state - an `in_review -> done | cancelled` verdict is authorized against the current review policy while the issue row is locked; a policy change in the same request or a concurrent request cannot relax that verdict gate - accepting or rejecting the review-confirmation interaction locks the issue row before resolving the interaction and reauthorizes against the current review policy in that transaction - accepting a fresh `request_confirmation` for the current issue's `plan` revision changes `work_mode = planning` to `work_mode = standard` in the same transaction as the accepted interaction; the existing agent-return transition also moves an eligible `in_review` issue to `todo` without changing its agent owner diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 87aaf4ebc80..9d9321e017f 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -128,7 +128,7 @@ The issue is actively owned work. For agent-owned issues, `in_progress` should not be allowed to become a silent dead state. -### Monitoring presentation +### Execution-backed presentation An agent-owned `in_progress` task displays **Monitoring** with a blue clock while it has a persisted ordinary monitor deadline and no active run, execution or @@ -137,6 +137,16 @@ status remains `in_progress`. Monitor admission, checkout, finalization, recover and agent lifecycle filters keep their existing contracts. Review, blocked, terminal, and provider-quota recovery states keep their own presentation. +For agent-owned tasks whose lifecycle status is `in_progress`, the visible +**In Progress** state requires a task-linked `queued` or `running` run. Stale +execution/checkout locks and scheduled retries alone do not establish activity. +Without an active run, an open recovery action displays **Blocked**, a pending review +stage or active pending thread interaction displays **In Review**, an ordinary +scheduled monitor displays **Monitoring**, and other work displays **Todo**. +These projections do not change stored lifecycle status, locks, admission, +continuation authority, or recovery. Human-owned tasks retain their explicit +ownership status. + Task reads expose optional, read-only `displayStatus`. Board task collections and search can filter by `displayStatus`; filtering happens before pagination and counts. Existing `status` filters retain their lifecycle meaning. Monitoring is diff --git a/server/src/__tests__/issue-display-status.test.ts b/server/src/__tests__/issue-display-status.test.ts index c46d3dc1db6..9722d68e120 100644 --- a/server/src/__tests__/issue-display-status.test.ts +++ b/server/src/__tests__/issue-display-status.test.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import { eq, sql } from "drizzle-orm"; import { expect, it } from "vitest"; import request from "supertest"; -import { agents, heartbeatRuns, issueRecoveryActions, issueRelations, issues, statusCards } from "@paperclipai/db"; +import { agents, heartbeatRuns, issueComments, issueRecoveryActions, issueRelations, issueThreadInteractions, issues, statusCards } from "@paperclipai/db"; import { companySearchQuerySchema, createIssueSchema, defaultStatusCardRefreshPolicy, PROVIDER_QUOTA_MONITOR_SERVICE_NAME, type Issue } from "@paperclipai/shared"; import { issueDisplayStatusSql } from "../services/issue-display-status.js"; import { issueService } from "../services/issues.js"; @@ -14,7 +14,7 @@ import { seedCompanyWithBoardAccess, useEmbeddedPostgres, } from "./helpers/route-test-harness.js"; -describeEmbeddedPostgres("Monitoring task presentation", () => { +describeEmbeddedPostgres("Execution-backed task presentation", () => { const ctx = useEmbeddedPostgres("paperclip-monitoring-status-", { resetEach: async (db) => { await db.execute(sql`SET client_min_messages = warning`); await db.execute(sql`TRUNCATE TABLE companies CASCADE`); }, }); @@ -39,6 +39,69 @@ describeEmbeddedPostgres("Monitoring task presentation", () => { return row; } + it.each(["failed", "cancelled", "timed_out", "succeeded", "scheduled_retry"])("does not show In Progress after a run becomes %s, even with stale locks", async (status) => { + const f = await fixture(); + await ctx.db.update(issues).set({ monitorNextCheckAt: null, executionPolicy: null }).where(eq(issues.id, f.issueId)); + const [run] = await ctx.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, issueId: f.issueId, status }).returning(); + await ctx.db.update(issues).set({ executionRunId: run!.id, checkoutRunId: run!.id }).where(eq(issues.id, f.issueId)); + expect(await displayStatus(f.issueId)).toEqual({ status: "in_progress", displayStatus: "todo" }); + const svc = issueService(ctx.db); + expect(await svc.count(f.companyId, { displayStatus: "in_progress" })).toBe(0); + expect(await svc.list(f.companyId, { displayStatus: "todo", limit: 1 })).toEqual([expect.objectContaining({ id: f.issueId })]); + // Presentation must not clear ownership or change recovery/admission inputs. + expect((await ctx.db.select().from(issues).where(eq(issues.id, f.issueId)))[0]).toMatchObject({ status: "in_progress", executionRunId: run!.id, checkoutRunId: run!.id }); + }); + + it("shows a stopped task waiting for a human as In Review across API reads and search", async () => { + const f = await fixture(); + await ctx.db.update(issues).set({ monitorNextCheckAt: null, executionPolicy: null }).where(eq(issues.id, f.issueId)); + await ctx.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, issueId: f.issueId, status: "failed", errorCode: "authentication_failed" }); + const [interaction] = await ctx.db.insert(issueThreadInteractions).values({ companyId: f.companyId, issueId: f.issueId, kind: "request_confirmation", addresseeUserId: f.actor.userId, + payload: { version: 1, prompt: "Authorize the connection" } }).returning(); + const app = routeApp(ctx.db, f.actor, issueRoutes); + expect((await request(app).get(`/api/issues/${f.issueId}`).expect(200)).body).toMatchObject({ status: "in_progress", displayStatus: "in_review" }); + const summary = await request(app).get(`/api/companies/${f.companyId}/issues/summaries?ids=${f.issueId}`).expect(200); + expect(summary.body[0]).toMatchObject({ displayStatus: "in_review" }); + const result = await companySearchService(ctx.db).search(f.companyId, companySearchQuerySchema.parse({ q: "deployment", displayStatus: "in_review" })); + expect(result.results[0]?.issue).toMatchObject({ id: f.issueId, displayStatus: "in_review" }); + await ctx.db.update(issueThreadInteractions).set({ status: "answered" }).where(eq(issueThreadInteractions.id, interaction!.id)); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); + }); + + it.each(["dismissed", "historical"])("does not turn a monitor into review for a %s request", async (kind) => { + const f = await fixture(); + const createdAt = new Date(Date.now() - 60_000); + await ctx.db.insert(issueThreadInteractions).values({ companyId: f.companyId, issueId: f.issueId, + kind: kind === "dismissed" ? "request_confirmation" : "ask_user_questions", status: "pending", createdAt, + dismissedAt: kind === "dismissed" ? new Date() : null, effectiveResolverPolicySource: "requested", + payload: { version: 1, prompt: "Earlier request" } }); + if (kind === "historical") await ctx.db.insert(issueComments).values({ companyId: f.companyId, issueId: f.issueId, + authorUserId: f.actor.userId, body: "Continue with the current instructions", createdAt: new Date() }); + expect((await displayStatus(f.issueId)).displayStatus).toBe("monitoring"); + expect(await issueService(ctx.db).count(f.companyId, { displayStatus: "in_review" })).toBe(0); + }); + + it("does not treat a retained reviewer as pending after changes are requested", async () => { + const f = await fixture(); + await ctx.db.update(issues).set({ executionState: { status: "changes_requested", currentStageId: "review", + currentStageType: "review", currentParticipant: { type: "user", userId: "reviewer" }, lastDecisionOutcome: "changes_requested" } }).where(eq(issues.id, f.issueId)); + expect((await displayStatus(f.issueId)).displayStatus).toBe("monitoring"); + await ctx.db.update(issues).set({ monitorNextCheckAt: null }).where(eq(issues.id, f.issueId)); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); + }); + + it("requires task-scoped active execution and preserves human ownership", async () => { + const f = await fixture(); + await ctx.db.update(issues).set({ monitorNextCheckAt: null, executionPolicy: null }).where(eq(issues.id, f.issueId)); + await ctx.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, status: "running" }); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); + const foreign = await fixture(); + await ctx.db.insert(heartbeatRuns).values({ companyId: foreign.companyId, agentId: foreign.agentId, issueId: f.issueId, status: "running" }); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); + await ctx.db.update(issues).set({ assigneeAgentId: null, assigneeUserId: f.actor.userId }).where(eq(issues.id, f.issueId)); + expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + }); + it("projects a scheduled wait consistently across detail, compact rows, summaries, search and counts", async () => { const f = await fixture(); const svc = issueService(ctx.db); @@ -61,7 +124,8 @@ describeEmbeddedPostgres("Monitoring task presentation", () => { it("filters before pagination and keeps In Progress separate from Monitoring", async () => { const f = await fixture(); - await ctx.db.insert(issues).values({ companyId: f.companyId, title: "Ordinary work", status: "in_progress", assigneeAgentId: f.agentId }); + const [active] = await ctx.db.insert(issues).values({ companyId: f.companyId, title: "Ordinary work", status: "in_progress", assigneeAgentId: f.agentId }).returning(); + await ctx.db.insert(heartbeatRuns).values({ companyId: f.companyId, agentId: f.agentId, issueId: active!.id, status: "running" }); const svc = issueService(ctx.db); expect(await svc.list(f.companyId, { displayStatus: "monitoring", limit: 1, offset: 0 })).toEqual([expect.objectContaining({ id: f.issueId })]); expect(await svc.list(f.companyId, { displayStatus: "monitoring", limit: 1, offset: 1 })).toEqual([]); @@ -104,24 +168,24 @@ describeEmbeddedPostgres("Monitoring task presentation", () => { it("does not label cleared, consumed, orphaned or quota-recovery schedules Monitoring", async () => { const f = await fixture(); await ctx.db.update(issues).set({ executionState: { monitor: { status: "cleared" } } }).where(eq(issues.id, f.issueId)); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); await ctx.db.update(issues).set({ executionState: null, monitorNextCheckAt: null }).where(eq(issues.id, f.issueId)); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); await ctx.db.update(issues).set({ monitorNextCheckAt: new Date(), executionPolicy: { monitor: { serviceName: PROVIDER_QUOTA_MONITOR_SERVICE_NAME } } }).where(eq(issues.id, f.issueId)); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); await ctx.db.update(issues).set({ executionPolicy: null }).where(eq(issues.id, f.issueId)); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("todo"); }); it("keeps recovery and review holds visible and never writes a new lifecycle state", async () => { const f = await fixture(); await ctx.db.insert(issueRecoveryActions).values({ companyId: f.companyId, sourceIssueId: f.issueId, kind: "source", ownerType: "board", cause: "process_lost", fingerprint: randomUUID(), nextAction: "Inspect stopped run" }); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("blocked"); await ctx.db.execute(sql`UPDATE issue_recovery_actions SET status = 'escalated' WHERE source_issue_id = ${f.issueId}`); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + expect((await displayStatus(f.issueId)).displayStatus).toBe("blocked"); await ctx.db.execute(sql`UPDATE issue_recovery_actions SET status = 'resolved' WHERE source_issue_id = ${f.issueId}`); expect((await displayStatus(f.issueId)).displayStatus).toBe("monitoring"); - await ctx.db.update(issues).set({ executionState: { currentParticipant: { type: "user", userId: "reviewer" } } }).where(eq(issues.id, f.issueId)); - expect((await displayStatus(f.issueId)).displayStatus).toBe("in_progress"); + await ctx.db.update(issues).set({ executionState: { status: "pending", currentStageId: "review", currentStageType: "review", currentParticipant: { type: "user", userId: "reviewer" } } }).where(eq(issues.id, f.issueId)); + expect((await displayStatus(f.issueId)).displayStatus).toBe("in_review"); }); }); diff --git a/server/src/services/issue-display-status.ts b/server/src/services/issue-display-status.ts index 5c9bdefb54b..3397f9fc0fd 100644 --- a/server/src/services/issue-display-status.ts +++ b/server/src/services/issue-display-status.ts @@ -1,10 +1,11 @@ import { sql, type SQL } from "drizzle-orm"; -import { issues } from "@paperclipai/db"; +import { issues, issueThreadInteractions } from "@paperclipai/db"; import { PROVIDER_QUOTA_MONITOR_SERVICE_NAME, type IssueDisplayStatus } from "@paperclipai/shared"; +import { activeIssueInteractionCondition } from "./issue-question-context.js"; -/** Read-only projection, shared by task reads, search, filters and counts. - * The persisted deadline is the scheduler's source of truth. Locks and recovery - * take precedence; a retained monitor alone must not disguise active work. +/** Read-only projection shared by task reads, search, filters and counts. + * Lifecycle status and locks remain admission/recovery inputs. Neither alone + * proves that an agent still has active execution. */ export function issueDisplayStatusSql(): SQL { // Drizzle removes column qualifiers in a single-table SELECT. Keep these @@ -12,26 +13,36 @@ export function issueDisplayStatusSql(): SQL { // the run/recovery table's own id or company_id instead. const issueId = sql`"issues"."id"`; const companyId = sql`"issues"."company_id"`; - return sql`CASE WHEN - ${issues.status} = 'in_progress' - AND ${issues.assigneeAgentId} IS NOT NULL AND ${issues.assigneeUserId} IS NULL - AND ${issues.monitorNextCheckAt} IS NOT NULL - AND ${issues.executionRunId} IS NULL AND ${issues.checkoutRunId} IS NULL - AND ${issues.executionState}->>'currentParticipant' IS NULL - AND COALESCE(${issues.executionState}#>>'{monitor,status}', 'scheduled') = 'scheduled' - AND ${issues.executionPolicy}->'monitor' IS NOT NULL - AND COALESCE(${issues.executionPolicy}#>>'{monitor,serviceName}', '') <> ${PROVIDER_QUOTA_MONITOR_SERVICE_NAME} - AND NOT EXISTS ( + return sql`CASE + WHEN ${issues.status} <> 'in_progress' + OR ${issues.assigneeAgentId} IS NULL OR ${issues.assigneeUserId} IS NOT NULL + THEN ${issues.status} + WHEN EXISTS ( SELECT 1 FROM heartbeat_runs display_run WHERE display_run.company_id = ${companyId} AND display_run.issue_id = ${issueId} AND display_run.status IN ('queued', 'running') - ) - AND NOT EXISTS ( + ) THEN 'in_progress' + WHEN EXISTS ( SELECT 1 FROM issue_recovery_actions display_recovery WHERE display_recovery.company_id = ${companyId} AND display_recovery.source_issue_id = ${issueId} AND display_recovery.status IN ('active', 'escalated') - ) - THEN 'monitoring' ELSE ${issues.status} END`; + ) THEN 'blocked' + WHEN (${issues.executionState}->>'status' = 'pending' + AND ${issues.executionState}->>'currentParticipant' IS NOT NULL) + OR EXISTS ( + SELECT 1 FROM ${issueThreadInteractions} + WHERE ${issueThreadInteractions.companyId} = ${companyId} + AND ${issueThreadInteractions.issueId} = ${issueId} + AND ${issueThreadInteractions.status} = 'pending' + AND ${activeIssueInteractionCondition()} + ) THEN 'in_review' + WHEN ${issues.monitorNextCheckAt} IS NOT NULL + AND ${issues.executionRunId} IS NULL AND ${issues.checkoutRunId} IS NULL + AND COALESCE(${issues.executionState}#>>'{monitor,status}', 'scheduled') = 'scheduled' + AND ${issues.executionPolicy}->'monitor' IS NOT NULL + AND COALESCE(${issues.executionPolicy}#>>'{monitor,serviceName}', '') <> ${PROVIDER_QUOTA_MONITOR_SERVICE_NAME} + THEN 'monitoring' + ELSE 'todo' END`; } diff --git a/ui/src/lib/task-status-presentation.test.ts b/ui/src/lib/task-status-presentation.test.ts new file mode 100644 index 00000000000..8158afef926 --- /dev/null +++ b/ui/src/lib/task-status-presentation.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from "vitest"; +import { resolveTaskStatusPresentation } from "./task-status-presentation"; + +describe("execution-backed task status", () => { + it.each([ + ["todo", "Todo"], + ["in_review", "In Review"], + ["blocked", "Blocked"], + ["monitoring", "Monitoring"], + ["in_progress", "In Progress"], + ])("uses server presentation %s for a task whose lifecycle is in_progress", (displayStatus, label) => { + expect(resolveTaskStatusPresentation({ status: "in_progress", displayStatus })).toEqual({ + glyphStatus: displayStatus, label, ariaLabel: label, + }); + }); + + it("keeps external conversation waits idle after projection", () => { + expect(resolveTaskStatusPresentation({ status: "in_progress", displayStatus: "in_review", externalConversationState: "waiting" })) + .toEqual({ glyphStatus: "idle", label: "Idle", ariaLabel: "Idle" }); + }); + + it.each(["done", "cancelled", "blocked", "in_review", "todo"])("preserves an optimistic %s transition over a stale display status", (status) => { + expect(resolveTaskStatusPresentation({ status, displayStatus: "in_progress" }).glyphStatus).toBe(status); + }); + + it("preserves status when the server did not send a projection", () => { + expect(resolveTaskStatusPresentation({ status: "in_progress" }).label).toBe("In Progress"); + expect(resolveTaskStatusPresentation({ status: "done" }).label).toBe("Done"); + }); +}); diff --git a/ui/src/lib/task-status-presentation.ts b/ui/src/lib/task-status-presentation.ts index fd2cc429585..e31ca0abf69 100644 --- a/ui/src/lib/task-status-presentation.ts +++ b/ui/src/lib/task-status-presentation.ts @@ -78,16 +78,16 @@ function blockedAttentionLabel(blockerAttention: IssueBlockerAttention | null | */ export function resolveTaskStatusPresentation(input: TaskStatusPresentationInput): TaskStatusPresentation { const { status, displayStatus, externalConversationState, blockerAttention } = input; - const visibleStatus = status === "in_review" && externalConversationState === "waiting" + // A saved/optimistic transition must win over the previous read projection. + const projectedStatus = status === "in_progress" ? (displayStatus ?? status) : status; + const visibleStatus = projectedStatus === "in_review" && externalConversationState === "waiting" ? "idle" - : status === "in_progress" && displayStatus === "monitoring" - ? "monitoring" - : status; - const isCoveredBlocked = status === "blocked" && blockerAttention?.state === "covered"; + : projectedStatus; + const isCoveredBlocked = projectedStatus === "blocked" && blockerAttention?.state === "covered"; return { glyphStatus: isCoveredBlocked ? "in_queue" : visibleStatus, label: taskStatusLabel(visibleStatus), - ariaLabel: status === "blocked" ? blockedAttentionLabel(blockerAttention) : taskStatusLabel(visibleStatus), + ariaLabel: projectedStatus === "blocked" ? blockedAttentionLabel(blockerAttention) : taskStatusLabel(visibleStatus), }; }