mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 21:59:38 +01:00
fix: show agent tasks in progress only with active runs
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
1cd1e8c598
commit
5ea1b1455a
7 files changed
+131
-34
No files matched your search
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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 review
|
||||
participant or 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
|
||||
|
||||
@@ -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, 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,47 @@ 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("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 +102,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 +146,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");
|
||||
expect((await displayStatus(f.issueId)).displayStatus).toBe("in_review");
|
||||
});
|
||||
});
|
||||
@@ -2,9 +2,9 @@ import { sql, type SQL } from "drizzle-orm";
|
||||
import { issues } from "@paperclipai/db";
|
||||
import { PROVIDER_QUOTA_MONITOR_SERVICE_NAME, type IssueDisplayStatus } from "@paperclipai/shared";
|
||||
|
||||
/** 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<IssueDisplayStatus> {
|
||||
// Drizzle removes column qualifiers in a single-table SELECT. Keep these
|
||||
@@ -12,26 +12,34 @@ export function issueDisplayStatusSql(): SQL<IssueDisplayStatus> {
|
||||
// the run/recovery table's own id or company_id instead.
|
||||
const issueId = sql`"issues"."id"`;
|
||||
const companyId = sql`"issues"."company_id"`;
|
||||
return sql<IssueDisplayStatus>`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<IssueDisplayStatus>`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}->>'currentParticipant' IS NOT NULL
|
||||
OR EXISTS (
|
||||
SELECT 1 FROM issue_thread_interactions display_interaction
|
||||
WHERE display_interaction.company_id = ${companyId}
|
||||
AND display_interaction.issue_id = ${issueId}
|
||||
AND display_interaction.status = 'pending'
|
||||
) 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`;
|
||||
}
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user