From 45c4ababc555e529d2d59ca489d8be07067f58a2 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 6 Nov 2026 07:15:20 -0600 Subject: [PATCH] fix: exclude historical input and returned reviews from task waits Co-Authored-By: Paperclip --- doc/execution-semantics.md | 4 +-- .../__tests__/issue-display-status.test.ts | 26 +++++++++++++++++-- server/src/services/issue-display-status.ts | 15 ++++++----- 3 files changed, 35 insertions(+), 10 deletions(-) diff --git a/doc/execution-semantics.md b/doc/execution-semantics.md index 6dee279bc70..9d9321e017f 100644 --- a/doc/execution-semantics.md +++ b/doc/execution-semantics.md @@ -140,8 +140,8 @@ 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 +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 diff --git a/server/src/__tests__/issue-display-status.test.ts b/server/src/__tests__/issue-display-status.test.ts index 77411d94527..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, issueThreadInteractions, 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"; @@ -68,6 +68,28 @@ describeEmbeddedPostgres("Execution-backed task presentation", () => { 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)); @@ -163,7 +185,7 @@ describeEmbeddedPostgres("Execution-backed task presentation", () => { 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)); + 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 4fba3d009b2..3397f9fc0fd 100644 --- a/server/src/services/issue-display-status.ts +++ b/server/src/services/issue-display-status.ts @@ -1,6 +1,7 @@ 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. * Lifecycle status and locks remain admission/recovery inputs. Neither alone @@ -28,12 +29,14 @@ export function issueDisplayStatusSql(): SQL { AND display_recovery.source_issue_id = ${issueId} AND display_recovery.status IN ('active', 'escalated') ) THEN 'blocked' - WHEN ${issues.executionState}->>'currentParticipant' IS NOT NULL + WHEN (${issues.executionState}->>'status' = 'pending' + AND ${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' + 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