diff --git a/packages/shared/src/validators/issue.test.ts b/packages/shared/src/validators/issue.test.ts index 4fa908e40cb..662224e4dac 100644 --- a/packages/shared/src/validators/issue.test.ts +++ b/packages/shared/src/validators/issue.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it } from "vitest"; +import { MAX_ISSUE_REQUEST_DEPTH } from "../index.js"; import { addIssueCommentSchema, createIssueSchema, @@ -75,4 +76,21 @@ describe("issue validators", () => { expect(response.summaryMarkdown).toBe("Summary\n\nNext action"); expect(document.body).toBe("# Plan\n\nShip it"); }); + + it("clamps oversized requestDepth values on create", () => { + const parsed = createIssueSchema.parse({ + title: "Clamp request depth", + requestDepth: MAX_ISSUE_REQUEST_DEPTH + 500, + }); + + expect(parsed.requestDepth).toBe(MAX_ISSUE_REQUEST_DEPTH); + }); + + it("clamps oversized requestDepth values on update", () => { + const parsed = updateIssueSchema.parse({ + requestDepth: MAX_ISSUE_REQUEST_DEPTH + 1, + }); + + expect(parsed.requestDepth).toBe(MAX_ISSUE_REQUEST_DEPTH); + }); }); diff --git a/packages/shared/src/validators/issue.ts b/packages/shared/src/validators/issue.ts index d8f58a831b6..4658cd8d37a 100644 --- a/packages/shared/src/validators/issue.ts +++ b/packages/shared/src/validators/issue.ts @@ -5,6 +5,7 @@ import { ISSUE_EXECUTION_STAGE_TYPES, ISSUE_EXECUTION_STATE_STATUSES, ISSUE_PRIORITIES, + clampIssueRequestDepth, ISSUE_STATUSES, ISSUE_THREAD_INTERACTION_CONTINUATION_POLICIES, ISSUE_THREAD_INTERACTION_KINDS, @@ -123,6 +124,12 @@ export const issueExecutionStateSchema = z.object({ lastDecisionOutcome: z.enum(ISSUE_EXECUTION_DECISION_OUTCOMES).nullable(), }); +const issueRequestDepthInputSchema = z + .number() + .int() + .nonnegative() + .transform((value) => clampIssueRequestDepth(value)); + export const createIssueSchema = z.object({ projectId: z.string().uuid().optional().nullable(), projectWorkspaceId: z.string().uuid().optional().nullable(), @@ -136,7 +143,7 @@ export const createIssueSchema = z.object({ priority: z.enum(ISSUE_PRIORITIES).optional().default("medium"), assigneeAgentId: z.string().uuid().optional().nullable(), assigneeUserId: z.string().optional().nullable(), - requestDepth: z.number().int().nonnegative().optional().default(0), + requestDepth: issueRequestDepthInputSchema.optional().default(0), billingCode: z.string().optional().nullable(), assigneeAdapterOverrides: issueAssigneeAdapterOverridesSchema.optional().nullable(), executionPolicy: issueExecutionPolicySchema.optional().nullable(), @@ -168,6 +175,7 @@ export const createIssueLabelSchema = z.object({ export type CreateIssueLabel = z.infer; export const updateIssueSchema = createIssueSchema.partial().extend({ + requestDepth: issueRequestDepthInputSchema.optional(), assigneeAgentId: z.string().trim().min(1).optional().nullable(), comment: multilineTextSchema.pipe(z.string().min(1)).optional(), reviewRequest: issueReviewRequestSchema.optional().nullable(), diff --git a/server/src/__tests__/issues-service.test.ts b/server/src/__tests__/issues-service.test.ts index b6e8e6d01cf..09223276540 100644 --- a/server/src/__tests__/issues-service.test.ts +++ b/server/src/__tests__/issues-service.test.ts @@ -24,7 +24,7 @@ import { } from "./helpers/embedded-postgres.js"; import { instanceSettingsService } from "../services/instance-settings.ts"; import { clampIssueListLimit, ISSUE_LIST_MAX_LIMIT, issueService } from "../services/issues.ts"; -import { buildProjectMentionHref } from "@paperclipai/shared"; +import { buildProjectMentionHref, MAX_ISSUE_REQUEST_DEPTH } from "@paperclipai/shared"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -1451,6 +1451,56 @@ describeEmbeddedPostgres("issueService.create workspace inheritance", () => { }), ]); }); + + it("clamps helper-created child requestDepth to the safe maximum", async () => { + const companyId = randomUUID(); + const projectId = randomUUID(); + const goalId = randomUUID(); + const parentIssueId = randomUUID(); + + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await instanceSettingsService(db).updateExperimental({ enableIsolatedWorkspaces: false }); + + await db.insert(goals).values({ + id: goalId, + companyId, + title: "Ship child helpers", + level: "task", + status: "active", + }); + + await db.insert(projects).values({ + id: projectId, + companyId, + goalId, + name: "Workspace project", + status: "in_progress", + }); + + await db.insert(issues).values({ + id: parentIssueId, + companyId, + projectId, + goalId, + title: "Parent issue", + status: "in_progress", + priority: "medium", + requestDepth: MAX_ISSUE_REQUEST_DEPTH, + }); + + const { issue: child } = await svc.createChild(parentIssueId, { + title: "Child helper", + status: "todo", + requestDepth: MAX_ISSUE_REQUEST_DEPTH + 100, + }); + + expect(child.requestDepth).toBe(MAX_ISSUE_REQUEST_DEPTH); + }); }); describeEmbeddedPostgres("issueService blockers and dependency wake readiness", () => { diff --git a/server/src/__tests__/productivity-review-service.test.ts b/server/src/__tests__/productivity-review-service.test.ts index 84ffcf9b317..f23768c2c58 100644 --- a/server/src/__tests__/productivity-review-service.test.ts +++ b/server/src/__tests__/productivity-review-service.test.ts @@ -14,6 +14,7 @@ import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; +import { MAX_ISSUE_REQUEST_DEPTH } from "@paperclipai/shared"; import { DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS, PRODUCTIVITY_REVIEW_ORIGIN_KIND, @@ -243,6 +244,44 @@ describeEmbeddedPostgres("productivity review service", () => { expect(review?.description).toContain("Runs in rolling windows: 10/1h"); }); + it("ignores non-assignee comments when evaluating high-churn productivity reviews", async () => { + const now = new Date("2026-04-28T12:00:00.000Z"); + const seeded = await seedAssignedIssue(); + await insertRuns({ + companyId: seeded.companyId, + agentId: seeded.coderId, + issueId: seeded.issueId, + count: 9, + now, + }); + const managerRuns = await insertRuns({ + companyId: seeded.companyId, + agentId: seeded.managerId, + issueId: seeded.issueId, + count: 10, + now, + }); + await db.insert(issueComments).values( + managerRuns.map((run, index) => ({ + companyId: seeded.companyId, + issueId: seeded.issueId, + authorAgentId: seeded.managerId, + createdByRunId: run.id, + body: `Manager note ${index}`, + createdAt: run.createdAt as Date, + updatedAt: run.createdAt as Date, + })), + ); + + const result = await productivityReviewService(db).reconcileProductivityReviews({ + now, + companyId: seeded.companyId, + }); + + expect(result.created).toBe(0); + expect(await listProductivityReviews(seeded.companyId)).toHaveLength(0); + }); + it("skips productivity-review descendants so reviews cannot recursively spawn reviews", async () => { const now = new Date("2026-04-28T12:00:00.000Z"); const seeded = await seedAssignedIssue(); @@ -358,4 +397,31 @@ describeEmbeddedPostgres("productivity review service", () => { expect(activities).toHaveLength(1); expect(activities[0]?.entityId).toBe(seeded.issueId); }); + + it("clamps poisoned requestDepth metadata instead of aborting productivity reconciliation", async () => { + const now = new Date("2026-04-28T12:00:00.000Z"); + const seeded = await seedAssignedIssue(); + + await db + .update(issues) + .set({ requestDepth: 2_147_483_647 }) + .where(eq(issues.id, seeded.issueId)); + + await insertRuns({ + companyId: seeded.companyId, + agentId: seeded.coderId, + issueId: seeded.issueId, + count: DEFAULT_PRODUCTIVITY_REVIEW_NO_COMMENT_STREAK_RUNS, + now, + }); + + const result = await productivityReviewService(db).reconcileProductivityReviews({ + now, + companyId: seeded.companyId, + }); + + expect(result.failed).toBe(0); + const [review] = await listProductivityReviews(seeded.companyId); + expect(review?.requestDepth).toBe(MAX_ISSUE_REQUEST_DEPTH); + }); }); diff --git a/server/src/index.ts b/server/src/index.ts index d5564bfcd5a..44eb4567f91 100644 --- a/server/src/index.ts +++ b/server/src/index.ts @@ -707,7 +707,7 @@ export async function startServer(): Promise { }) .then(async () => { const reviewed = await heartbeat.reconcileProductivityReviews(); - if (reviewed.created > 0 || reviewed.updated > 0) { + if (reviewed.created > 0 || reviewed.updated > 0 || reviewed.failed > 0) { logger.warn({ ...reviewed }, "startup productivity reconciliation created or updated review work"); } }) @@ -772,7 +772,7 @@ export async function startServer(): Promise { }) .then(async () => { const reviewed = await heartbeat.reconcileProductivityReviews(); - if (reviewed.created > 0 || reviewed.updated > 0) { + if (reviewed.created > 0 || reviewed.updated > 0 || reviewed.failed > 0) { logger.warn({ ...reviewed }, "periodic productivity reconciliation created or updated review work"); } }) diff --git a/server/src/services/productivity-review.ts b/server/src/services/productivity-review.ts index d9d04c9a3fb..739483d90ab 100644 --- a/server/src/services/productivity-review.ts +++ b/server/src/services/productivity-review.ts @@ -1,5 +1,6 @@ import { and, asc, desc, eq, gt, inArray, isNull, notInArray, sql } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; +import { clampIssueRequestDepth } from "@paperclipai/shared"; import { agents, companies, @@ -9,6 +10,7 @@ import { issues, projects, } from "@paperclipai/db"; +import { logger } from "../middleware/logger.js"; import { logActivity } from "./activity-log.js"; import { budgetService } from "./budgets.js"; import { issueService } from "./issues.js"; @@ -262,14 +264,19 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque .then((rows) => rows[0]?.count ?? 0); } - async function countIssueCommentsSince(companyId: string, issueId: string, since?: Date) { + async function countIssueCommentsSince(companyId: string, issueId: string, agentId: string, since?: Date) { return db .select({ count: sql`count(*)::int` }) .from(issueComments) + .innerJoin(heartbeatRuns, eq(heartbeatRuns.id, issueComments.createdByRunId)) .where( and( eq(issueComments.companyId, companyId), eq(issueComments.issueId, issueId), + eq(issueComments.authorAgentId, agentId), + eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.agentId, agentId), + issueRunScopeSql(issueId), since ? sql`${issueComments.createdAt} >= ${since.toISOString()}::timestamptz` : undefined, ), ) @@ -328,23 +335,34 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque const [ runCountLastHour, runCountLastSixHours, - commentCount, - commentCountLastHour, - commentCountLastSixHours, + assigneeRunCommentCount, + assigneeRunCommentCountLastHour, + assigneeRunCommentCountLastSixHours, latestComments, costRow, ] = await Promise.all([ countIssueRunsSince(sourceIssue.companyId, sourceAgent.id, sourceIssue.id, oneHourAgo), countIssueRunsSince(sourceIssue.companyId, sourceAgent.id, sourceIssue.id, sixHoursAgo), - countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id), - countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id, oneHourAgo), - countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id, sixHoursAgo), + countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id, sourceAgent.id), + countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id, sourceAgent.id, oneHourAgo), + countIssueCommentsSince(sourceIssue.companyId, sourceIssue.id, sourceAgent.id, sixHoursAgo), db - .select() + .select({ comment: issueComments }) .from(issueComments) - .where(and(eq(issueComments.companyId, sourceIssue.companyId), eq(issueComments.issueId, sourceIssue.id))) + .innerJoin(heartbeatRuns, eq(heartbeatRuns.id, issueComments.createdByRunId)) + .where( + and( + eq(issueComments.companyId, sourceIssue.companyId), + eq(issueComments.issueId, sourceIssue.id), + eq(issueComments.authorAgentId, sourceAgent.id), + eq(heartbeatRuns.companyId, sourceIssue.companyId), + eq(heartbeatRuns.agentId, sourceAgent.id), + issueRunScopeSql(sourceIssue.id), + ), + ) .orderBy(desc(issueComments.createdAt), desc(issueComments.id)) - .limit(5), + .limit(5) + .then((rows) => rows.map((row) => row.comment)), db .select({ costCents: sql`coalesce(sum(${costEvents.costCents}), 0)::int` }) .from(costEvents) @@ -364,9 +382,9 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque const longActive = elapsedMs !== null && elapsedMs >= thresholds.longActiveMs; const highChurn = runCountLastHour >= thresholds.highChurnHourly || - commentCountLastHour >= thresholds.highChurnHourly || + assigneeRunCommentCountLastHour >= thresholds.highChurnHourly || runCountLastSixHours >= thresholds.highChurnSixHours || - commentCountLastSixHours >= thresholds.highChurnSixHours; + assigneeRunCommentCountLastSixHours >= thresholds.highChurnSixHours; const trigger = choosePrimaryTrigger({ noComment, longActive, highChurn }); if (!trigger) return null; @@ -375,7 +393,7 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque if (longActive) triggerReasons.push(`current active episode has lasted ${msToHuman(elapsedMs)}`); if (highChurn) { triggerReasons.push( - `${runCountLastHour} runs/${commentCountLastHour} comments in 1h; ${runCountLastSixHours} runs/${commentCountLastSixHours} comments in 6h`, + `${runCountLastHour} runs/${assigneeRunCommentCountLastHour} assignee-run comments in 1h; ${runCountLastSixHours} runs/${assigneeRunCommentCountLastSixHours} assignee-run comments in 6h`, ); } @@ -390,9 +408,9 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque activeRunCount, runCountLastHour, runCountLastSixHours, - commentCount, - commentCountLastHour, - commentCountLastSixHours, + commentCount: assigneeRunCommentCount, + commentCountLastHour: assigneeRunCommentCountLastHour, + commentCountLastSixHours: assigneeRunCommentCountLastSixHours, elapsedMs, latestRuns: latestRuns.slice(0, 5), latestComments, @@ -474,7 +492,7 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque `- No-comment completed-run streak: ${evidence.noCommentStreak}`, `- Current active elapsed time: ${msToHuman(evidence.elapsedMs)}`, `- Runs in rolling windows: ${evidence.runCountLastHour}/1h, ${evidence.runCountLastSixHours}/6h`, - `- Comments total/window: ${evidence.commentCount} total, ${evidence.commentCountLastHour}/1h, ${evidence.commentCountLastSixHours}/6h`, + `- Assignee run-linked comments total/window: ${evidence.commentCount} total, ${evidence.commentCountLastHour}/1h, ${evidence.commentCountLastSixHours}/6h`, `- Cost events total: ${evidence.costCents} cents`, `- Current next action: ${evidence.nextAction ? truncateInline(evidence.nextAction, 500) : "none recorded"}`, "", @@ -482,14 +500,14 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque "", `- No-comment streak: ${evidence.thresholds.noCommentStreakRuns} completed runs`, `- Long active duration: ${msToHuman(evidence.thresholds.longActiveMs)}`, - `- High churn: ${evidence.thresholds.highChurnHourly}/1h or ${evidence.thresholds.highChurnSixHours}/6h runs/comments`, + `- High churn: ${evidence.thresholds.highChurnHourly}/1h or ${evidence.thresholds.highChurnSixHours}/6h runs/assignee-run comments`, `- Resolved-review snooze: ${msToHuman(evidence.thresholds.resolvedSnoozeMs)}`, "", "## Latest Runs", "", latestRuns, "", - "## Latest Comments", + "## Latest Assignee Run Comments", "", latestComments, "", @@ -513,7 +531,7 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque `- Trigger: \`${evidence.trigger}\` (${formatTrigger(evidence.trigger)})`, `- Reasons: ${evidence.triggerReasons.join("; ")}`, `- No-comment streak: ${evidence.noCommentStreak}`, - `- Runs/comments: ${evidence.runCountLastHour}/${evidence.commentCountLastHour} in 1h, ${evidence.runCountLastSixHours}/${evidence.commentCountLastSixHours} in 6h`, + `- Runs/assignee comments: ${evidence.runCountLastHour}/${evidence.commentCountLastHour} in 1h, ${evidence.runCountLastSixHours}/${evidence.commentCountLastSixHours} in 6h`, `- Next action: ${evidence.nextAction ? truncateInline(evidence.nextAction, 300) : "none recorded"}`, ].join("\n"); } @@ -561,7 +579,7 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque originKind: PRODUCTIVITY_REVIEW_ORIGIN_KIND, originId: evidence.sourceIssue.id, originFingerprint: productivityReviewFingerprint(evidence.sourceIssue.id), - requestDepth: evidence.sourceIssue.requestDepth + 1, + requestDepth: clampIssueRequestDepth(clampIssueRequestDepth(evidence.sourceIssue.requestDepth) + 1), }); } catch (error) { const maybe = error as { code?: string; constraint?: string; message?: string }; @@ -650,7 +668,9 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque existing: 0, snoozed: 0, skipped: 0, + failed: 0, reviewIssueIds: [] as string[], + failedIssueIds: [] as string[], }; const prefixCache = new Map(); @@ -682,11 +702,25 @@ export function productivityReviewService(db: Db, deps?: { enqueueWakeup?: Enque prefix = await getCompanyIssuePrefix(candidate.companyId); prefixCache.set(candidate.companyId, prefix); } - const outcome = await createOrUpdateReview(evidence, { prefix }); - if (outcome.kind === "created") result.created += 1; - else if (outcome.kind === "updated") result.updated += 1; - else result.existing += 1; - result.reviewIssueIds.push(outcome.reviewIssueId); + try { + const outcome = await createOrUpdateReview(evidence, { prefix }); + if (outcome.kind === "created") result.created += 1; + else if (outcome.kind === "updated") result.updated += 1; + else result.existing += 1; + result.reviewIssueIds.push(outcome.reviewIssueId); + } catch (err) { + result.failed += 1; + result.failedIssueIds.push(candidate.id); + logger.warn( + { + err, + companyId: candidate.companyId, + issueId: candidate.id, + requestDepth: candidate.requestDepth, + }, + "productivity review reconciliation skipped malformed candidate", + ); + } } return result;