Deduplicate open watchdog review wakes (#9148)
## 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 <noreply@paperclip.ing>
This commit is contained in:
parent
be821a4f7e
commit
19454ce385
|
|
@ -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" });
|
||||
|
|
|
|||
|
|
@ -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;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue