Skip to content

fix(heartbeat): stop self-authored status comments from defeating the rewake throttle (BLO-23081) - #1165

Open
allyblockcast[bot] wants to merge 1 commit into
masterfrom
platformsre/blo23081-rewake-throttle-self-comment
Open

fix(heartbeat): stop self-authored status comments from defeating the rewake throttle (BLO-23081)#1165
allyblockcast[bot] wants to merge 1 commit into
masterfrom
platformsre/blo23081-rewake-throttle-self-comment

Conversation

@allyblockcast

@allyblockcast allyblockcast Bot commented Aug 8, 2026

Copy link
Copy Markdown

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • Agent-wake economics: server/src/services/issue-rewake-throttle.ts exists to damp full-price adapter sessions that pay for zero new information while an agent waits on a slow external gate (merge queue, CI, review, deploy)
  • It never actually engages for an agent following the documented run-comment protocol — measured 12.1min median re-wake on BLO-21953 instead of the intended 30min cap
  • issue.comment_added sits in both the progress-action set and the new-input-action set, so the run that posts a routine status comment marks itself as progress, and that same comment reads as fresh input for the very next evaluation — the no-progress streak can never accumulate past 0
  • This pull request excludes bare comments from progress entirely and makes the new-input check actor-aware, so only a comment from someone other than the evaluating agent bypasses the throttle
  • The benefit is the throttle finally does what it was built for: an agent polling a slow external gate backs off to the 30-minute cap instead of paying a full session every ~12 minutes

Linked Issues or Issue Description

Fixes: BLO-23081
Refs: BLO-21953 (where the defect was found and measured)

What Changed

  • issue-rewake-throttle.ts: removed issue.comment_added from ISSUE_PROGRESS_ACTIVITY_ACTIONS — a bare comment no longer counts as issue-visible progress by itself; a run still counts as progress if it leaves a real mutation (status change, document, work product, etc.) behind, with or without an accompanying comment.
  • Added isIssueRewakeNewInputActivity(row, evaluatingAgentId): a comment_added row only counts as new input when its agentId differs from the evaluating agent (or is null, i.e. a board user) — a self-authored status ping is excluded, a comment from anyone else still bypasses the throttle immediately.
  • evaluateIssueRewakeThrottle now takes raw activityRows ({ runId, action, agentId, createdAt }) plus the evaluating agentId, instead of two pre-computed booleans (runIdsWithIssueProgress, hasNewIssueInputSinceLastRun) — the actor-aware decision now lives in the pure, directly-unit-testable function rather than being baked into the caller's SQL.
  • heartbeat.ts: collapsed the two separate DB queries (progress rows scoped by runId, new-input rows scoped by createdAt) into one query fetching {runId, action, agentId, createdAt} for the union of both conditions, and passes the raw rows straight into evaluateIssueRewakeThrottle.
  • Updated/added tests in both issue-rewake-throttle.test.ts (unit, no DB) and heartbeat-issue-rewake-throttle.test.ts (embedded-Postgres integration) covering: self-authored comment doesn't reset the streak, foreign-agent/board-user comment still bypasses immediately, commenting alongside a real mutation still counts, failed-run recovery is unaffected, and escalation reaches the 30-minute cap within 6 no-progress runs.

Verification

  • npx vitest run src/__tests__/issue-rewake-throttle.test.ts — 20/20 passed (pure unit tests, no DB).
  • npx vitest run src/__tests__/heartbeat-issue-rewake-throttle.test.ts — 8/8 passed (embedded-Postgres integration, includes the pre-existing suite updated to use real mutations instead of comments for its "progress" fixtures, since a bare comment is no longer progress by design).
  • npx tsc --noEmit -p server/tsconfig.json — clean.

Risks

  • Behavioral shift, not just a bugfix: an agent that previously relied on posting a comment to keep itself immediately re-wakeable on a slow-moving issue will now back off per the documented cooldown schedule (2m → 4m → 8m → 16m → 30m cap) unless something else (another agent, a board user, or a real mutation) is happening on the issue. This is the intended behavior change per BLO-23081's acceptance criteria.
  • No schema change. The activityLog.agentId column already existed and is nullable for non-agent actors (board users), so the actor check degrades safely (null !== agentId → always counts as new input) for those rows.
  • Low risk to unrelated call sites: issue-rewake-throttle.ts has exactly one caller (heartbeat.ts), confirmed via repo-wide grep before making the interface change.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), 1M context window, via Claude Code / Paperclip agent adapter. Read the throttle source and its caller, wrote the fix, rewrote/extended both test files, ran the embedded-Postgres integration suite and tsc --noEmit locally before opening this PR.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have specified the model used (with version and capability details)
  • I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
  • I have searched GitHub for duplicate or related PRs and linked them above
  • I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots — N/A, no UI
  • I have updated relevant documentation to reflect my changes — N/A, module docstring updated in-place
  • I have considered and documented any risks above
  • All Paperclip CI gates are green — pending CI on this PR
  • Greptile is 5/5 with no open P2s, recommendations, or follow-ups — pending review
  • I will address all Greptile and reviewer comments before requesting merge

https://paperclip.blockcast.net/BLO/issues/BLO-23081

… rewake throttle (BLO-23081)

issue.comment_added was in both ISSUE_PROGRESS_ACTIVITY_ACTIONS and
ISSUE_NEW_INPUT_ACTIVITY_ACTIONS, so the run that posts a routine status
comment marks itself as progress, and the comment then reads as new input
for the next evaluation. noProgressStreak never accumulates past 0, so the
2-consecutive-run threshold never engages for an agent following the
documented comment-every-run protocol — measured at a 12.1min median
re-wake interval on BLO-21953 instead of the intended 30min cap.

Exclude comment_added from progress entirely (a bare comment carries no
state change) and make the new-input check actor-aware: a comment from the
evaluating agent itself no longer counts as either signal, while a comment
from any other agent or a board user still bypasses the throttle
immediately. evaluateIssueRewakeThrottle now takes raw activity rows
(action/agentId/createdAt) instead of pre-computed booleans so the
actor-aware logic is covered by direct unit tests.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@allyblockcast

allyblockcast Bot commented Aug 8, 2026

Copy link
Copy Markdown
Author

🔗 Paperclip issue: BLO-21953
🔗 Paperclip issue: BLO-23081

@allyblockcast allyblockcast Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: d3e6ea9

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

  • The core fix is correct: ISSUE_PROGRESS_ACTIVITY_ACTIONS no longer includes issue.comment_added, and the new isIssueRewakeNewInputActivity helper makes the new-input check actor-aware (row.agentId !== evaluatingAgentId), so a self-authored status comment can neither reset the no-progress streak nor count as fresh input for itself — while a comment from another agent or a board user (agentId === null) still bypasses the throttle immediately. This matches BLO-23081's acceptance criteria.
  • Verified the single-query refactor in heartbeat.ts (server/src/services/heartbeat.ts:25595-25620) is behavior-preserving: ISSUE_PROGRESS_ACTIVITY_ACTIONS is a structural subset of ISSUE_NEW_INPUT_ACTIVITY_ACTIONS (built via spread), so filtering the combined query on action in ISSUE_NEW_INPUT_ACTIVITY_ACTIONS still returns every row needed for both the progress and new-input computations that now live inside evaluateIssueRewakeThrottle.
  • Confirmed evaluateIssueRewakeThrottle has exactly one caller (heartbeat.ts) via repo-wide grep, so widening its signature (agentId + raw activityRows instead of precomputed runIdsWithIssueProgress/hasNewIssueInputSinceLastRun) is a safe interface change.
  • Failed-run recovery, streak escalation to the 30-minute cap, and the "comment alongside a real mutation still counts" case are all preserved and covered by new tests; test counts in both files (20 unit, 8 integration) match the PR description's claims.
  • agentId is a non-null string parameter throughout the call chain (enqueueWakeup(agentId: string, ...)), so the new agentId: string field on IssueRewakeThrottleInput is type-safe with no optionality mismatch.

Recommended Action

  1. No blocking issues — safe to merge once CI is green.

@allyblockcast
allyblockcast Bot added this pull request to the merge queue Aug 8, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants