Arnike/paperclip
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(watchdog): suppress repeat alerts when source issue is blocked or evaluation board-closed (#5942)

Browse Source 
## Thinking Path

> - Paperclip orchestrates AI agents for zero-human companies; the
stale-active-run watchdog monitors agent heartbeat runs for extended
output silence and fires evaluation issues to alert the responsible
manager
> - The watchdog uses a unique index on open evaluation issues to
prevent duplicate open issues per run, but this only prevents two
*simultaneous* open issues — not sequential ones created after each
closure
> - When a board reviewer closes an alert as done directly (without
recording a watchdog decision), the dismissed_false_positive guard is
bypassed and `findOpenStaleRunEvaluation` returns null on the next scan
— causing a new alert to fire every 30 minutes until the run terminates
> - The previous fix also removed `blockedByIssueIds` mutation from
`ensureSourceIssueBlockedByStaleEvaluation` to break the
block→silence→new-alert loop, but that left no idempotency guard on the
source-issue escalation comment, so every critical scan re-appended the
comment to the source-issue thread
> - Additionally, runs whose source issue is already `blocked` (agent is
correctly idle, waiting on a human action) should never generate alerts
at all — silence is expected in that state
> - This PR fixes all three gaps in `createOrUpdateStaleRunEvaluation` /
`ensureSourceIssueCommentedForStaleEvaluation`: (1) skip when source
issue is `blocked`, (2) auto-record a dismissed_false_positive decision
when a closed evaluation exists with no prior watchdog decision, (3) add
an activity-log-backed idempotency guard so the source-issue escalation
comment fires exactly once per (sourceIssue, evaluationIssue) pair
across scan cycles and process restarts
> - The benefit is that agents correctly paused waiting on board-gated
blockers no longer generate repeated false-positive noise tickets,
board-closed evaluations are permanently suppressed without requiring a
second interaction, and source-issue threads no longer get spammed with
duplicate escalation comments

## What Changed

- `server/src/services/recovery/service.ts`:
- Added `blocked` source-issue guard: `if (sourceIssue?.status ===
"blocked") return { kind: "skipped" }` — idle output is expected when
the source issue is blocked
- Added `findClosedStaleRunEvaluation()` — queries for `done` evaluation
issues for a given run, ordered by most recent update (scoped to `done`
only so system-cancelled evaluations don't permanently suppress alerts)
- Added `hasDismissedFalsePositiveDecision()` — queries for an existing
dismissed_false_positive watchdog decision record
- Added closed-evaluation auto-dismiss: when a prior evaluation was
closed `done` on the board without any watchdog decision, auto-inserts a
dismissed_false_positive record so future scans skip via the existing
guard. The check-then-insert runs inside a transaction guarded by a
per-(company, run) `pg_advisory_xact_lock` so two overlapping scans
cannot both observe `hasAnyDecision = false` and both insert duplicate
rows
- Removed `blockedByIssueIds` mutation from the escalation path and
renamed `ensureSourceIssueBlockedByStaleEvaluation` →
`ensureSourceIssueCommentedForStaleEvaluation` to reflect that the
function now only adds a comment + activity log (no state mutation) —
evaluation issues are observability-only and adding them as hard
blockers created a self-amplifying loop (blocked→silent→new
alert→blocked again)
- Added activity-log-backed idempotency guard at the top of
`ensureSourceIssueCommentedForStaleEvaluation`: query the activity log
for a `heartbeat.output_stale_escalated` row with the same (sourceIssue,
evaluationIssue) pair and return false when one is present. The single
activity-log row written on the first successful escalation is the
suppression record for all later scans, surviving process restarts
- `server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts`:
- Added: "emits the source-issue escalation comment only once across
repeated critical scans" (covers the comment-spam regression path)
  - Added: "skips ticket creation when the source issue is blocked"
- Added: "suppresses repeat alerts when evaluation is closed on the
board without a watchdog decision"
- Added: "still allows re-arm after continue decision even when
evaluation is board-closed" (exception path: if any watchdog decision
exists, human opted in to lifecycle — honour it)

## Verification

```sh
pnpm exec vitest run server/src/__tests__/heartbeat-active-run-output-watchdog.test.ts
```

- All 18 watchdog tests pass locally
- Regression: source-issue escalation comment emits exactly once across
repeated critical scans
- Blocked source → no evaluation created (result.created === 0,
result.skipped === 1)
- Board-closed evaluation + no decisions → auto-records
dismissed_false_positive; second scan creates nothing
- Board-closed evaluation + continue decision → second scan still
creates (re-arm preserved)

## Risks

- **Low risk.** The blocked-status guard is a pure early-return that
adds no state mutation. The auto-dismiss path only inserts a record when
no decisions exist — it cannot fire for runs where a human has opted in
to the watchdog lifecycle via snooze/continue. Removing
`blockedByIssueIds` from the critical-escalation path is safe because
evaluation issues are already parented under the source issue.
- The `dismissed_false_positive` auto-insert is now race-safe under
concurrent scans via `pg_advisory_xact_lock` keyed on `(companyId,
runId)` so the check-then-insert pair is serialized without requiring a
schema change.
- `findClosedStaleRunEvaluation` is scoped to `done` only (not
`cancelled`) so system code paths that cancel evaluation issues cannot
permanently suppress future watchdog alerts for the same run.

## Model Used

- Provider: Anthropic
- Model: Claude Sonnet 4.6 (`claude-sonnet-4-6`) for original change;
Claude Opus 4.7 (`claude-opus-4-7`) for follow-up review fixes
- Context: full repo read with tool use, running as SADE agent in
Paperclip Claude Code
- Mode: agentic code analysis + targeted edit

## 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 run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots (no UI changes)
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge

## Cross-references and status (maintainer)

- Closes #4937
- Closes #5207
- Closes #5767
- Closes #5949

Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
David Bezar
2026-06-11 22:00:13 -07:00
committed by GitHub
parent deef1f479d
commit 01e59c074a
2 changed files with 323 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/src/services/recovery/service.ts
+132 -13
View File
@@ -898,6 +898,51 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
return row ?? null;
}
// Returns a `done` stale-run evaluation issue for this run if one exists.
// Used to detect when a reviewer closed an alert directly on the board without going through
// the watchdog decision API — which would not leave a dismissed_false_positive decision record.
//
// Scoped to `done` only (not `cancelled`): cancellation is used by other system code paths
// and does not imply a reviewer's "false positive" verdict. `done` is the explicit
// board-close path used by reviewers acknowledging the alert. A cancelled evaluation is
// allowed to re-fire on the next scan; if a reviewer wants permanent suppression they
// should mark the alert done or record a watchdog decision.
async function findClosedStaleRunEvaluation(companyId: string, runId: string) {
const [row] = await db
.select({ id: issues.id, identifier: issues.identifier, status: issues.status })
.from(issues)
.where(
and(
eq(issues.companyId, companyId),
eq(issues.originKind, STALE_ACTIVE_RUN_EVALUATION_ORIGIN_KIND),
eq(issues.originId, runId),
isNull(issues.hiddenAt),
eq(issues.status, "done"),
),
)
.orderBy(desc(issues.updatedAt))
.limit(1);
return row ?? null;
}
// Returns true when a reviewer has already dismissed this run's silence as a false positive.
// Used to prevent re-filing after a deliberate close — while still allowing legitimate
// re-arm after a "continue" decision's snooze window expires.
async function hasDismissedFalsePositiveDecision(companyId: string, runId: string) {
const [row] = await db
.select({ id: heartbeatRunWatchdogDecisions.id })
.from(heartbeatRunWatchdogDecisions)
.where(
and(
eq(heartbeatRunWatchdogDecisions.companyId, companyId),
eq(heartbeatRunWatchdogDecisions.runId, runId),
eq(heartbeatRunWatchdogDecisions.decision, "dismissed_false_positive"),
),
)
.limit(1);
return row != null;
}
async function buildRunOutputSilence(
run: Pick<
typeof heartbeatRuns.$inferSelect,
@@ -1462,26 +1507,42 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
);
}
async function ensureSourceIssueBlockedByStaleEvaluation(input: {
async function ensureSourceIssueCommentedForStaleEvaluation(input: {
sourceIssue: typeof issues.$inferSelect | null;
evaluationIssue: { id: string; identifier: string | null };
run: typeof heartbeatRuns.$inferSelect;
}) {
if (!input.sourceIssue || ["done", "cancelled"].includes(input.sourceIssue.status)) return false;
const blockerIds = await existingBlockerIssueIds(input.sourceIssue.companyId, input.sourceIssue.id);
if (blockerIds.includes(input.evaluationIssue.id)) return false;
const nextBlockerIds = [...blockerIds, input.evaluationIssue.id];
await issuesSvc.update(input.sourceIssue.id, {
...(input.sourceIssue.status === "blocked" ? {} : { status: "blocked" }),
blockedByIssueIds: nextBlockerIds,
});
// Idempotency guard: if we've already emitted the escalation comment for this
// (sourceIssue, evaluationIssue) pair, skip. Without this, every subsequent scan
// cycle while the evaluation issue is still open re-fires the comment and spams
// the source-issue thread. The activity log row written below is the persistence
// record we check against — a single row per pair is enough to suppress repeats
// even after process restarts.
const [priorEscalation] = await db
.select({ id: activityLog.id })
.from(activityLog)
.where(
and(
eq(activityLog.companyId, input.sourceIssue.companyId),
eq(activityLog.action, "heartbeat.output_stale_escalated"),
eq(activityLog.entityType, "issue"),
eq(activityLog.entityId, input.sourceIssue.id),
sql`${activityLog.details} ->> 'evaluationIssueId' = ${input.evaluationIssue.id}`,
),
)
.limit(1);
if (priorEscalation) return false;
// Evaluation issues are observability-only — do NOT add them to blockedByIssueIds.
// They are already parented under the source issue. Adding them as hard blockers
// creates a self-amplifying loop: block → silence → new alert → block again.
await issuesSvc.addComment(input.sourceIssue.id, [
"Paperclip detected critical output silence on this issue's active run.",
"",
`- Evaluation issue: ${input.evaluationIssue.identifier ?? input.evaluationIssue.id}`,
`- Run: \`${input.run.id}\``,
"",
"This blocks the source issue on the explicit review task without cancelling the active process.",
"Review the evaluation issue above. The active run has not been cancelled.",
].join("\n"), { runId: input.run.id });
await logActivity(db, {
companyId: input.sourceIssue.companyId,
@@ -1495,7 +1556,6 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
details: {
source: "recovery.scan_silent_active_runs",
evaluationIssueId: input.evaluationIssue.id,
blockerIssueIds: nextBlockerIds,
},
});
return true;
@@ -1549,6 +1609,65 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
});
}
}
// Idle output is expected when the source issue is blocked — skip ticket creation entirely.
if (sourceIssue?.status === "blocked") return { kind: "skipped" as const };
// Dedup: if a reviewer has dismissed this run's silence as a false positive, don't re-file.
// A "continue" decision with a snooze window is allowed to re-arm normally — only an
// explicit dismissed_false_positive blocks all further alerts for this run.
if (await hasDismissedFalsePositiveDecision(input.run.companyId, input.run.id)) {
return { kind: "skipped" as const };
}
// Dedup: if a prior evaluation issue for this run was closed `done` on the board
// without going through the watchdog decision API, no dismissed_false_positive record exists
// and the watchdog would re-fire every cycle. Auto-record the suppression now so future
// cycles skip immediately via hasDismissedFalsePositiveDecision.
//
// Exception: if any watchdog decision exists (snooze/continue), a human explicitly opted
// in to the watchdog lifecycle — honour that and allow re-arm as designed.
//
// Concurrency: the check-then-insert runs inside a transaction with a per-(company,run)
// advisory lock so two overlapping scans cannot both observe `hasAnyDecision = false`
// and both insert a dismissed_false_positive row. The table has no unique constraint
// on (companyId, runId, decision), so the advisory lock is the serialization point.
const closedEvaluation = await findClosedStaleRunEvaluation(input.run.companyId, input.run.id);
if (closedEvaluation) {
const autoDismissed = await db.transaction(async (tx) => {
await tx.execute(
sql`SELECT pg_advisory_xact_lock(hashtextextended(${`watchdog_dismiss:${input.run.companyId}:${input.run.id}`}, 0))`,
);
const hasAnyDecision = await tx
.select({ id: heartbeatRunWatchdogDecisions.id })
.from(heartbeatRunWatchdogDecisions)
.where(
and(
eq(heartbeatRunWatchdogDecisions.companyId, input.run.companyId),
eq(heartbeatRunWatchdogDecisions.runId, input.run.id),
),
)
.limit(1)
.then((rows) => rows.length > 0);
if (hasAnyDecision) return false;
await tx.insert(heartbeatRunWatchdogDecisions).values({
companyId: input.run.companyId,
runId: input.run.id,
evaluationIssueId: closedEvaluation.id,
decision: "dismissed_false_positive",
snoozedUntil: null,
reason: `Auto-recorded: evaluation issue ${closedEvaluation.identifier} was closed as ${closedEvaluation.status} on the board without a watchdog decision.`,
createdByAgentId: null,
createdByUserId: null,
createdByRunId: null,
});
return true;
});
if (autoDismissed) {
return { kind: "skipped" as const };
}
}
const prefix = await getCompanyIssuePrefix(input.run.companyId);
const evidence = await collectStaleRunEvidence({
run: input.run,
@@ -1570,7 +1689,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
`- Silent for: ${formatDuration(evidence.silenceAgeMs)}`,
`- Last output at: ${input.run.lastOutputAt?.toISOString() ?? "none recorded"}`,
].join("\n"), { runId: input.run.id });
await ensureSourceIssueBlockedByStaleEvaluation({
await ensureSourceIssueCommentedForStaleEvaluation({
sourceIssue,
evaluationIssue: existing,
run: input.run,
@@ -1578,7 +1697,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
return { kind: "escalated" as const, evaluationIssueId: existing.id };
}
if (level === "critical") {
await ensureSourceIssueBlockedByStaleEvaluation({
await ensureSourceIssueCommentedForStaleEvaluation({
sourceIssue,
evaluationIssue: existing,
run: input.run,
@@ -1640,7 +1759,7 @@ export function recoveryService(db: Db, deps: { enqueueWakeup: RecoveryWakeup })
},
});
if (level === "critical") {
await ensureSourceIssueBlockedByStaleEvaluation({
await ensureSourceIssueCommentedForStaleEvaluation({
sourceIssue,
evaluationIssue: evaluation,
run: input.run,