diff --git a/server/src/__tests__/task-watchdogs-scheduler.test.ts b/server/src/__tests__/task-watchdogs-scheduler.test.ts index 5800e15154..e8138f85b1 100644 --- a/server/src/__tests__/task-watchdogs-scheduler.test.ts +++ b/server/src/__tests__/task-watchdogs-scheduler.test.ts @@ -216,6 +216,80 @@ describeEmbeddedPostgres("task watchdog scheduler", () => { expect(watchdog?.triggerCount).toBe(1); }); + it("does not append duplicate review comments for an already-open same-fingerprint review", async () => { + const companyId = await seedCompany(); + const sourceId = await seedIssue(companyId, { identifier: "WDOG-DUPE", status: "done" }); + const agentId = await seedAgent(companyId); + await seedWatchdog(companyId, sourceId, agentId); + const { service, wakes } = createService(); + + const first = await service.reconcileTaskWatchdogs({ companyId }); + expect(first).toMatchObject({ checked: 1, triggered: 1 }); + + const [firstWatchdog] = await db.select().from(issueWatchdogs).where(eq(issueWatchdogs.issueId, sourceId)); + const watchdogIssueId = firstWatchdog!.watchdogIssueId!; + const initialComments = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, watchdogIssueId)); + expect(initialComments).toHaveLength(1); + + const second = await service.reconcileTaskWatchdogs({ companyId }); + + expect(second).toMatchObject({ checked: 1, triggered: 0, live: 1 }); + expect(wakes).toHaveLength(1); + const comments = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, watchdogIssueId)); + expect(comments).toHaveLength(1); + const [watchdog] = await db.select().from(issueWatchdogs).where(eq(issueWatchdogs.issueId, sourceId)); + expect(watchdog?.lastObservedFingerprint).toBe(firstWatchdog?.lastObservedFingerprint); + expect(watchdog?.triggerCount).toBe(1); + }); + + it("re-wakes a same-fingerprint watchdog review stuck in stale in_review", async () => { + const companyId = await seedCompany(); + const sourceId = await seedIssue(companyId, { identifier: "WDOG-STALE", status: "done" }); + const agentId = await seedAgent(companyId); + await seedWatchdog(companyId, sourceId, agentId); + const { service, wakes } = createService(); + + const first = await service.reconcileTaskWatchdogs({ companyId }); + expect(first).toMatchObject({ checked: 1, triggered: 1 }); + + const [firstWatchdog] = await db.select().from(issueWatchdogs).where(eq(issueWatchdogs.issueId, sourceId)); + const watchdogIssueId = firstWatchdog!.watchdogIssueId!; + await db + .update(issues) + .set({ + status: "in_review", + assigneeAgentId: null, + assigneeUserId: null, + executionState: null, + monitorNextCheckAt: null, + }) + .where(eq(issues.id, watchdogIssueId)); + + const second = await service.reconcileTaskWatchdogs({ companyId }); + + expect(second).toMatchObject({ checked: 1, triggered: 1 }); + expect(wakes).toHaveLength(2); + const [watchdogIssue] = await db.select().from(issues).where(eq(issues.id, watchdogIssueId)); + expect(watchdogIssue).toMatchObject({ + status: "todo", + assigneeAgentId: agentId, + originFingerprint: firstWatchdog?.lastObservedFingerprint, + }); + const comments = await db + .select() + .from(issueComments) + .where(eq(issueComments.issueId, watchdogIssueId)); + expect(comments).toHaveLength(2); + const [watchdog] = await db.select().from(issueWatchdogs).where(eq(issueWatchdogs.issueId, sourceId)); + expect(watchdog?.triggerCount).toBe(2); + }); + it("does not trigger while a non-watchdog descendant has live work", async () => { const companyId = await seedCompany(); const sourceId = await seedIssue(companyId, { identifier: "WDOG-2", status: "in_progress" }); diff --git a/server/src/services/task-watchdogs.ts b/server/src/services/task-watchdogs.ts index f5dc6eb3f4..df84b8fc0a 100644 --- a/server/src/services/task-watchdogs.ts +++ b/server/src/services/task-watchdogs.ts @@ -1073,6 +1073,26 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {}) return Boolean(run || issueRun || wake); } + async function sameFingerprintWatchdogReviewIsStillOpen( + watchdogIssue: IssueRow | null, + stopFingerprint: string, + ) { + if (!watchdogIssue) return false; + if (watchdogIssue.originFingerprint !== stopFingerprint) return false; + if (isTerminalIssueStatus(watchdogIssue.status) || watchdogIssue.status === "backlog") return false; + if (watchdogIssue.status === "in_review") { + const hasPendingReviewPath = await watchdogIssueHasPendingReviewPath(watchdogIssue.companyId, watchdogIssue.id); + return isWatchdogReviewDisposition(watchdogIssue, hasPendingReviewPath); + } + return true; + } + + async function watchdogIssueNeedsFreshWake(watchdogIssue: IssueRow) { + if (watchdogIssue.status !== "in_review") return false; + const hasPendingReviewPath = await watchdogIssueHasPendingReviewPath(watchdogIssue.companyId, watchdogIssue.id); + return !isWatchdogReviewDisposition(watchdogIssue, hasPendingReviewPath); + } + async function watchdogIssueHasPendingReviewPath(companyId: string, issueId: string) { const [interaction, approval] = await Promise.all([ db @@ -1164,7 +1184,9 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {}) const fallback = existing ?? await findTaskWatchdogIssue(input.watchdog.companyId, input.sourceIssue.id); if (fallback) { - const shouldReopen = isTerminalIssueStatus(fallback.status) || fallback.status === "backlog"; + const shouldReopen = isTerminalIssueStatus(fallback.status) || + fallback.status === "backlog" || + await watchdogIssueNeedsFreshWake(fallback); const watchdogIssue = shouldReopen ? await issuesSvc.update(fallback.id, { status: "todo", @@ -1285,6 +1307,37 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {}) .where(eq(issueWatchdogs.id, watchdog.id)); return { state: "watchdog_live" as const, classification, watchdogIssueId: existingWatchdogIssueId }; } + const existingWatchdogIssue = existingWatchdogIssueId + ? await db + .select() + .from(issues) + .where(and( + eq(issues.companyId, watchdog.companyId), + eq(issues.id, existingWatchdogIssueId), + isNull(issues.hiddenAt), + )) + .then((rows) => rows[0] ?? null) + : null; + if (await sameFingerprintWatchdogReviewIsStillOpen(existingWatchdogIssue, classification.stopFingerprint)) { + if ( + watchdog.watchdogIssueId !== existingWatchdogIssue!.id || + watchdog.lastObservedFingerprint !== classification.stopFingerprint + ) { + await db + .update(issueWatchdogs) + .set({ + watchdogIssueId: existingWatchdogIssue!.id, + lastObservedFingerprint: classification.stopFingerprint, + updatedAt: new Date(), + }) + .where(eq(issueWatchdogs.id, watchdog.id)); + } + return { + state: "watchdog_review_open" as const, + classification, + watchdogIssueId: existingWatchdogIssue!.id, + }; + } const watchdogIssue = await ensureReusableWatchdogIssue({ watchdog, @@ -1519,7 +1572,11 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {}) if (evaluated.state === "triggered") { result.triggered += 1; result.watchdogIssueIds.push(evaluated.watchdogIssueId); - } else if (evaluated.state === "live" || evaluated.state === "watchdog_live") { + } else if ( + evaluated.state === "live" || + evaluated.state === "watchdog_live" || + evaluated.state === "watchdog_review_open" + ) { result.live += 1; } else if (evaluated.state === "pending_first_run") { result.pendingFirstRun += 1; @@ -1553,6 +1610,12 @@ export function taskWatchdogService(db: Db, deps: TaskWatchdogServiceDeps = {}) result.watchdogIssueIds.push(evaluated.watchdogIssueId); } else if (evaluated.state === "pending_first_run") { result.pendingFirstRun += 1; + } else if ( + evaluated.state === "watchdog_review_open" || + evaluated.state === "watchdog_live" || + evaluated.state === "live" + ) { + // Existing review work is already open for this stopped state. } else { result.skipped += 1; }