From 19454ce38599f1ebebaaf2e98d18512d18d4f575 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 7 Jul 2026 05:54:23 -0500 Subject: [PATCH] Deduplicate open watchdog review wakes (#9148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Watchdogs keep issue execution moving by waking agents or creating recovery paths when work stalls. > - Open review states should generate useful follow-up, not repeated duplicate wake requests for the same unresolved review condition. > - Duplicate wakes create noise and can make the control plane look busier without increasing progress. > - This pull request deduplicates open watchdog review wake scheduling and covers the behavior with scheduler tests. > - The benefit is cleaner review wake behavior and fewer redundant agent runs. ## Linked Issues or Issue Description No public GitHub issue exists. Inline bug report: **Pre-submission checklist** - [x] I have searched existing open and closed issues and this is not a duplicate. - [x] I am on the latest released version of Paperclip (or can reproduce on `master`). - [x] I have confirmed the error originates in Paperclip itself — not in my agent adapter, API provider, or local configuration. **What happened?** Watchdog scheduling could enqueue duplicate open review wake requests while the same unresolved review condition was already pending. **Expected behavior** A watchdog should avoid scheduling redundant review wakes for the same unresolved condition while preserving legitimate wake paths. **Steps to reproduce** 1. Create an issue state that requires an open watchdog review wake. 2. Run the watchdog scheduler once and observe a wake request. 3. Run the scheduler again before resolving the original review condition. 4. Observe whether a duplicate wake is created. **Paperclip version or commit** `master` at the PR base. **Deployment mode** Local dev (`pnpm dev`) and server deployments running watchdog scheduling. **Installation method** Built from source (`pnpm dev` / `pnpm build`). **Agent adapter(s) involved** - [x] Not adapter-specific (core bug) **Database mode** Not database-related beyond scheduler persistence. **Access context** Agent wake scheduling and board-visible review state. **Relevant logs or output** Covered by the added scheduler regression test. **Relevant config (if applicable)** Not applicable. **Additional context** This suppresses duplicate wake scheduling only while the open review state is still unresolved. **Privacy checklist** - [x] I have reviewed all pasted output for PII (usernames, file paths, API keys, tokens, company names) and redacted where necessary. ## What Changed - Added deduplication logic for open watchdog review wake scheduling. - Added scheduler regression coverage for duplicate open review wake suppression. ## Verification - `/srv/paperclip/home/paperclipai/paperclip/node_modules/.bin/vitest run server/src/__tests__/task-watchdogs-scheduler.test.ts` ## Risks Low-to-medium risk. The change intentionally suppresses duplicate wake scheduling, so reviewers should confirm no legitimate repeated wake path depends on creating multiple open requests for the same unresolved review state. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5.5 coding agent with repository tool use and local shell execution. Context window was not surfaced by the runtime. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip --- .../task-watchdogs-scheduler.test.ts | 74 +++++++++++++++++++ server/src/services/task-watchdogs.ts | 67 ++++++++++++++++- 2 files changed, 139 insertions(+), 2 deletions(-) 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; }