fix: skip same-run self-comments (Path A heartbeat-reopen + Path B implicit-todo move) (#4973)
## Thinking Path - Paperclip treats issue comments as both communication and wake signals, so comment attribution affects whether completed work reopens. - The bug lived in two independent paths: deferred comment wake promotion in `heartbeat.ts`, and implicit reopen-on-comment logic in `routes/issues.ts`. - Both paths need the same core rule: a comment from the same run that just closed the issue must not look like a fresh human follow-up. - Deferred wake batches also need one extra safeguard: if a batch mixes a same-run self-comment with a real human comment, the human follow-up must still reopen the issue. ## What Changed - `server/src/services/heartbeat.ts` now suppresses deferred reopen only when every referenced comment in the batch was created by the closing run. - `server/src/routes/issues.ts` now passes `actorRunId`, `checkoutRunId`, and `executionRunId` into `shouldImplicitlyMoveCommentedIssueToTodo`, and skips the implicit move when the comment came from the run that already owns the issue. - `server/src/__tests__/heartbeat-comment-wake-batching.test.ts` adds coverage for both Path A cases: same-run self-comment stays closed, while a mixed self-comment plus human-comment batch still reopens. - `server/src/__tests__/issue-comment-reopen-routes.test.ts` covers the same-run guard on both POST and PATCH comment paths, plus the negative case where a different run still reopens. ## Verification ```bash pnpm --filter @paperclipai/server exec vitest run src/__tests__/issue-comment-reopen-routes.test.ts pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-comment-wake-batching.test.ts -t "self-authored by the closing run|mixes self-authored and human comments" pnpm --filter @paperclipai/server typecheck ``` ## Risks - Low risk. Both changes are additive guards and preserve existing behavior for comments that do not originate from the owning run. - The deferred-wake change now uses all-self semantics, which is the key correctness detail for mixed batches. - Full CI is still the authoritative validation for the broader heartbeat integration surface. ## Model Used - OpenAI Codex, GPT-5-based coding agent (`codex_local` adapter in Paperclip). ## 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 — N/A, server-only - [x] I have updated relevant documentation to reflect my 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) Refs #6601 Refs #3980 --------- Co-authored-by: Paperclip <paperclip@users.noreply.github.com>
This commit is contained in:
@@ -747,7 +747,23 @@ function shouldImplicitlyMoveCommentedIssueToTodo(input: {
|
||||
assigneeAgentId: string | null | undefined;
|
||||
actorType: "agent" | "user";
|
||||
actorId: string;
|
||||
actorRunId: string | null | undefined;
|
||||
checkoutRunId: string | null | undefined;
|
||||
executionRunId: string | null | undefined;
|
||||
}) {
|
||||
// Local-CLI agents post comments under user auth, so the actor.type is "user"
|
||||
// even though the comment originates from the same heartbeat run that owns
|
||||
// the issue lock. Without this guard, an agent that closes its own issue and
|
||||
// then posts a follow-up comment in the same run silently reopens it.
|
||||
// Suppress the implicit move whenever the comment's source run matches the
|
||||
// issue's checkout/execution run.
|
||||
if (
|
||||
typeof input.actorRunId === "string"
|
||||
&& input.actorRunId.length > 0
|
||||
&& (input.actorRunId === input.checkoutRunId || input.actorRunId === input.executionRunId)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
// Only human comments should implicitly reopen finished work.
|
||||
// Agent-authored comments remain communicative unless reopen was explicit.
|
||||
if (input.actorType !== "user") return false;
|
||||
@@ -4861,6 +4877,9 @@ export function issueRoutes(
|
||||
assigneeAgentId: requestedAssigneeAgentId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
actorRunId: actor.runId,
|
||||
checkoutRunId: existing.checkoutRunId,
|
||||
executionRunId: existing.executionRunId,
|
||||
})) ||
|
||||
shouldResumeInProgressScheduledRetry);
|
||||
const updateReferenceSummaryBefore = titleOrDescriptionChanged
|
||||
@@ -6635,6 +6654,9 @@ export function issueRoutes(
|
||||
assigneeAgentId: issue.assigneeAgentId,
|
||||
actorType: actor.actorType,
|
||||
actorId: actor.actorId,
|
||||
actorRunId: actor.runId,
|
||||
checkoutRunId: issue.checkoutRunId,
|
||||
executionRunId: issue.executionRunId,
|
||||
}) ||
|
||||
shouldResumeInProgressScheduledRetry);
|
||||
const hasUnresolvedFirstClassBlockers =
|
||||
|
||||
Reference in New Issue
Block a user