Skip to content

fix(heartbeat): eagerly cancel a former assignee's stale queued runs on reassignment - #2

Closed
rendedennis6-byte wants to merge 1 commit into
masterfrom
rena-55610-eager-queue-cancel
Closed

fix(heartbeat): eagerly cancel a former assignee's stale queued runs on reassignment#2
rendedennis6-byte wants to merge 1 commit into
masterfrom
rena-55610-eager-queue-cancel

Conversation

@rendedennis6-byte

Copy link
Copy Markdown
Owner

Thinking Path

  • Paperclip is the open source app people use to manage AI agents for work
  • The heartbeat/run-scheduler subsystem tracks queued runs per agent and lazily reconciles them (via a staleness check) only when that agent's own claim loop reaches them and has a free concurrency slot
  • When an issue gets reassigned away from an agent, that agent can still be holding a queued continuation/recovery run tied to the issue - and the lazy check that would normally clean it up never fires if the agent is at its concurrency cap, or if the stale run sits behind other live queued work in priority order
  • Left unresolved, the queued run (and any execution lock it holds) can be stranded for hours, blocking legitimate work on the reassigned issue
  • This pull request adds an eager cancellation path: on issue reassignment, immediately sweep the previous assignee's queued runs for that issue; additionally, the claim-loop itself now sweeps its whole queued list for staleness up front, before the concurrency-slot gate, instead of stopping once slots run out
  • The benefit is queued runs orphaned by reassignment are cleaned up promptly instead of depending on a lazy check that can be starved indefinitely

Linked Issues or Issue Description

No public GitHub issue exists for this (tracked internally). Describing per the bug report template:

Bug: Reassigning an issue away from its current assignee agent does not cancel that agent's still-queued continuation/recovery run for the issue. The run is only reconciled by a lazy staleness check inside the claim path (claimQueuedRun), which only runs once (a) the agent's own queue-processing loop reaches that specific run, and (b) the agent has a free concurrency slot. If the agent is at its concurrency cap, or the stale run is queued behind other live/ready work for that agent, the stale run - and any execution lock it may still hold on the issue - can remain stranded indefinitely (observed: hours).

Expected: Reassigning an issue should promptly cancel any queued run the previous assignee was still holding for that issue, freeing any lock it holds so the new assignee (or future claims) aren't blocked.

Actual: The stale queued run lingers until the lazy check happens to reach it, which is not guaranteed to happen in a timely manner.

What Changed

  • Added heartbeatService.cancelStaleQueuedRunsForIssue(companyId, issueId, previousAssigneeAgentId), which loads the previous assignee's queued runs for the given issue, evaluates each with the existing evaluateQueuedRunStaleness logic (so interaction-wake / current-review-participant exceptions still apply), and cancels the ones found stale.
  • Wired this into the issue-update route in server/src/routes/issues.ts: whenever assigneeAgentId changes during an update, the previous assignee's queued runs for that issue are eagerly swept (best-effort, logged on failure, does not block the response).
  • In heartbeat.ts's queued-run claim path, moved the staleness sweep to run over the entire queued list up front, before the concurrency-slot gate, instead of only reconciling runs the claim loop happens to walk past while slots remain. This closes the gap where a stale run sitting behind enough live work (or arriving after the agent is already at its concurrency cap) would never get re-examined.
  • Updated the mock setups in the 4 affected route test files (issue-agent-mutation-ownership-routes.test.ts, issue-comment-reopen-routes.test.ts, issue-execution-policy-routes.test.ts, issue-update-comment-wakeup-routes.test.ts) to include the new cancelStaleQueuedRunsForIssue mock on mockHeartbeatService, since the route code now calls it unconditionally on assignee changes.

Verification

  • pnpm -r typecheck (repo-wide): passes clean.
  • pnpm run build (repo-wide): passes clean.
  • Full server package vitest run (409 files / 4340 tests): 381 files / 4183 tests pass. The 28 failing files are all pre-existing, unrelated to this change - adapter/CLI execution tests (Claude/Codex/Gemini/Cursor/Pi local adapters) and workspace-runtime/environment-runtime tests that require external binaries not present in this shell (sshd, taskkill, adapter CLIs), which is exactly the class of suite the repo's scripts/run-vitest-stable.mjs wrapper normally serializes/excludes from a plain vitest run. None of the 4 edited test files appear in that failure list.
  • Scoped re-run of the 4 edited route test files directly: 159/160 pass. The 1 remaining failure (denies company-wide issue list routes for task bridge keys in issue-agent-mutation-ownership-routes.test.ts, a 5000ms timeout) was confirmed pre-existing and unrelated by re-running it against the unmodified base branch (git stash), where it fails identically.
  • Manually traced the reassignment code path against evaluateQueuedRunStaleness / cancelQueuedRunForStaleIssue to confirm the eager sweep reuses the same staleness rules (including the interaction-wake / review-participant exceptions) as the existing lazy check, rather than introducing a second, divergent cancellation policy.

Risks

  • The eager cancellation on reassignment is best-effort and wrapped in a .catch that logs a warning rather than failing the issue-update request, so a transient DB error here cannot block a legitimate reassignment.
  • The claim-path sweep now evaluates staleness for every queued run up front (previously only for runs actually walked while slots remained), which is slightly more work per claim cycle, but bounded by the same queued-run set the claim loop was already loading, and avoids doing the (usually cheap) staleness check per queue drain.
  • This is a scheduler-internal behavior change (server-side); it does not touch API response shapes or public contracts. Deploying to a live Paperclip instance is a separate deliberate action, out of scope for this PR/branch.

Model Used

Claude Sonnet 4.6, via Claude Code (agentic coding tool with file read/write, bash execution, and test running tool use).

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 not referenced internal/instance-local Paperclip issues or links (only public GitHub #NNN / github.com/paperclipai/paperclip URLs)
  • My branch name describes the change (e.g. docs/..., fix/...) and contains no internal Paperclip ticket id or instance-derived details
  • I have run tests locally and they pass
  • I have added or updated tests where applicable
  • I have updated relevant documentation to reflect my changes
  • 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

…on issue reassignment

Reassigning an issue away from an agent left that agent's queued
continuation/recovery run for the issue in place. The run only got
swept by the lazy staleness check inside claimQueuedRun, which
requires the queue to reach that specific run AND a concurrency slot
to be free - otherwise the run, and any execution lock it holds, could
be stranded for hours.

Adds heartbeatService.cancelStaleQueuedRunsForIssue(), called from the
issue-update route right when assigneeAgentId changes, and also sweeps
the whole queued-run list for staleness up front in the claim path
(before the concurrency-slot gate) so a stale entry buried behind live
work no longer gets skipped indefinitely.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@rendedennis6-byte

Copy link
Copy Markdown
Owner Author

Closing as duplicate: this fix (eager cancellation of a former assignee's stale queued runs on issue reassignment) was already implemented and submitted upstream as paperclipai#11085 (paperclipai#11085), opened from this same fork slightly earlier. This PR was accidentally opened against my own fork's master instead of upstream, redoing the same work in a separate branch. Please refer to paperclipai#11085 for review/merge.

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.

2 participants