Skip to content

fix(issues): recover agent attribution when bearer token is absent#4441

Open
supertaz wants to merge 4 commits intopaperclipai:masterfrom
supertaz:fix/comment-attribution-20260424
Open

fix(issues): recover agent attribution when bearer token is absent#4441
supertaz wants to merge 4 commits intopaperclipai:masterfrom
supertaz:fix/comment-attribution-20260424

Conversation

@supertaz
Copy link
Copy Markdown

Problem

When an agent sends x-paperclip-run-id but omits Authorization: Bearer, the server in local_trusted deployment mode resolves the request actor as board rather than agent. This causes two compounding bugs:

  1. Comment misattributionaddComment writes author_user_id = 'local-board' and author_agent_id = NULL, so the UI shows the board user's avatar as the comment author instead of the agent that actually wrote it.

  2. Wake cascade — Because actor.actorType !== "agent", the self-wake suppression check (selfComment = actorIsAgent && actor.actorId === assigneeId) evaluates to false. Every comment the agent posts triggers a new wakeup for itself. Each wakeup has a unique wakeCommentId, so the existing dedup gate also misses. The agent is repeatedly woken until the runaway detector auto-pauses it.

Fix

Route handlers (PATCH /issues/:id and POST /issues/:id/comments):

When the resolved actor is not an agent but a runId is present, look up the run record and recover the true agent identity. Use the recovered agentId for comment attribution and self-wake suppression — without expanding auth scope.

let resolvedActorAgentId: string | null = actor.agentId;
let resolvedActorIsAgent = actor.actorType === "agent";
if (actor.actorType !== "agent" && actor.runId) {
  const run = await heartbeat.getRun(actor.runId);
  if (run?.agentId) {
    resolvedActorAgentId = run.agentId;
    resolvedActorIsAgent = true;
  }
}

heartbeatService.wakeup (belt-and-suspenders):

Add self-wake suppression at the execution layer so the cascade is stopped even if the route-level check is somehow bypassed. Two paths covered:

  • Normal agent-actor path: requestedByActorType === "agent" && requestedByActorId === agentId
  • Board-actor path: look up the triggering comment's created_by_run_id and check whether that run belongs to the agent being woken.

Scope

Only attribution metadata and self-wake suppression are affected. The local_trusted fallback actor type, access control, and all other authz checks are unchanged.

When an agent sends `x-paperclip-run-id` but omits `Authorization: Bearer`,
the server in `local_trusted` mode resolves the actor as board, causing:

1. Issue comments are attributed to the board user instead of the authoring
   agent (`author_agent_id = NULL`, `author_user_id = 'local-board'`).
2. Self-wake suppression on the comment route is bypassed because
   `actor.actorType !== "agent"`, triggering a repeated wake cascade until
   the runaway detector auto-pauses the agent.

This fix addresses both issues:

- In `issueRoutes` (PATCH and POST comment handlers): when the resolved actor
  is not an agent but a `runId` is present, look up the run record and recover
  the true `agentId`. Use the recovered identity for comment attribution and
  self-wake suppression.
- In `heartbeatService.wakeup`: add belt-and-suspenders self-wake suppression.
  The first check handles the normal agent-actor path. The second handles the
  board-actor path by looking up whether the triggering comment's
  `created_by_run_id` belongs to the agent being woken.
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 24, 2026

Greptile Summary

This PR fixes agent comment misattribution and self-wake cascade in local_trusted mode when an agent omits its Authorization: Bearer token but supplies x-paperclip-run-id. The fix is applied in both the PATCH /issues/:id and POST /issues/:id/comments handlers, with a belt-and-suspenders suppression layer added to heartbeatService.wakeup.

  • The PATCH handler's @-mention self-skip guard (line 2366) still references actor.actorType / actor.actorId instead of the resolved values — the same guard in the POST handler was updated, leaving an inconsistency that can cause erroneous mention-wakeups from the PATCH path for token-omitting agents.

Confidence Score: 3/5

Partially safe — the primary attribution fix is correct, but a missed self-skip guard in the PATCH handler leaves the @-mention wake-cascade path incompletely fixed.

One P1 finding (missed mentions self-skip in PATCH handler) pulls the score to the P1 ceiling of 4; the partially incomplete fix combined with the PR template violations brings it to 3.

server/src/routes/issues.ts — specifically the @-mention loop in the PATCH handler (line 2366) and all wakeup requestedByActor fields that still pass the unresolved actor.

Important Files Changed

Filename Overview
server/src/routes/issues.ts Adds agent-recovery blocks in PATCH and POST handlers; fixes selfComment and attribution — but the PATCH handler's @-mention self-skip guard still uses the raw actor, leaving an incomplete fix for that path.
server/src/services/heartbeat.ts Adds two belt-and-suspenders self-wake suppression checks: one for the resolved-agent path and one for the board-actor path via createdByRunId lookup; logic is correct and well-scoped.

Comments Outside Diff (1)

  1. server/src/routes/issues.ts, line 2366 (link)

    P1 PATCH handler self-skip for @-mentions still uses unresolved actor

    This guard still reads actor.actorType === "agent" && actor.actorId === mentionedId, so when a bearer-token-omitting agent @-mentions itself in a PATCH comment, the resolved values (resolvedActorIsAgent, resolvedActorAgentId) are not consulted and the skip is bypassed. The corresponding guard in the POST /comments handler was correctly updated (line 3342); this one was missed.

    The belt-and-suspenders check in heartbeatService.wakeup provides a secondary catch, but the route-level fix is inconsistent between the two handlers.

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: server/src/routes/issues.ts
    Line: 2366
    
    Comment:
    **`PATCH` handler self-skip for `@`-mentions still uses unresolved actor**
    
    This guard still reads `actor.actorType === "agent" && actor.actorId === mentionedId`, so when a bearer-token-omitting agent `@`-mentions itself in a PATCH comment, the resolved values (`resolvedActorIsAgent`, `resolvedActorAgentId`) are not consulted and the skip is bypassed. The corresponding guard in the `POST /comments` handler was correctly updated (line 3342); this one was missed.
    
    The belt-and-suspenders check in `heartbeatService.wakeup` provides a secondary catch, but the route-level fix is inconsistent between the two handlers.
    
    
    
    How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 2366

Comment:
**`PATCH` handler self-skip for `@`-mentions still uses unresolved actor**

This guard still reads `actor.actorType === "agent" && actor.actorId === mentionedId`, so when a bearer-token-omitting agent `@`-mentions itself in a PATCH comment, the resolved values (`resolvedActorIsAgent`, `resolvedActorAgentId`) are not consulted and the skip is bypassed. The corresponding guard in the `POST /comments` handler was correctly updated (line 3342); this one was missed.

The belt-and-suspenders check in `heartbeatService.wakeup` provides a secondary catch, but the route-level fix is inconsistent between the two handlers.

```suggestion
if (resolvedActorIsAgent && resolvedActorAgentId === mentionedId) continue;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 3294-3295

Comment:
**Wakeup calls still pass unresolved actor to `heartbeat.wakeup`**

`requestedByActorType` and `requestedByActorId` are passed as the raw `actor` values (potentially `"board"` / `"local-board"`) rather than the resolved agent values. The belt-and-suspenders check in `heartbeatService.wakeup` deliberately handles this via the secondary `createdByRunId` lookup, so the cascade is blocked in practice. Consider passing the resolved values here for clarity and to avoid dependence on the secondary suppression path for the primary use-case. The same pattern appears at lines 2343–2344, 3318–3319, and 3348–3349.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 3125-3138

Comment:
**Agent-recovery block is duplicated verbatim across two route handlers**

The 14-line block that resolves `resolvedActorAgentId` / `resolvedActorIsAgent` from the run record is identical in the `PATCH /issues/:id` and `POST /issues/:id/comments` handlers. Extracting it into a shared helper (e.g. `resolveActorAgent(actor, runLookupFn, companyId)`) would reduce the surface area for future drift like the missed `mentionedId` guard above.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/routes/issues.ts
Line: 1774

Comment:
**PR description does not follow the required template**

`CONTRIBUTING.md` and `.github/PULL_REQUEST_TEMPLATE.md` require every PR to include a **Thinking Path** (blockquote tracing from project context to this change), **Verification** steps, **Risks**, **Model Used** (with provider, version, context window, and reasoning mode), and the **Checklist**. The current description has a clear Problem/Fix/Scope narrative but is missing all of the required template sections. Please fill in the template so reviewers and CI can validate the required fields.

**Context Used:** Contribution guidelines ([source](https://app.greptile.com/review/custom-context?memory=a595932a-f6ed-448b-899b-5ccac43f9148))

How can I resolve this? If you propose a fix, please make it concise.

Reviews (3): Last reviewed commit: "fix(comments): keep userId undefined whe..." | Re-trigger Greptile

Comment thread server/src/routes/issues.ts Outdated
Comment thread server/src/routes/issues.ts
Ensure the run looked up by runId belongs to the same company as the
issue being commented on before using its agentId for attribution.
Prevents cross-company agentId injection via a crafted run-id header.
…odo check

Greptile review: !resolvedActorIsAgent incorrectly passed board actor ID as
userId for board actors with no runId. Revert to actor.actorType === "user"
which returns undefined for board actors, as the original code did.

Also pass resolved actor type/id to shouldImplicitlyMoveCommentedIssueToTodoForAgent
so bearer-less agent requests correctly trigger the move-to-todo behavior.
@supertaz
Copy link
Copy Markdown
Author

@greptile recheck

Address review feedback while preserving the literal actorType-based guard.
The recovery path now writes only the resolved agentId, leaving userId
undefined; for genuine board users (no resolved agent), userId continues
to be set to actor.actorId via the original predicate.
@supertaz
Copy link
Copy Markdown
Author

@greptile recheck

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.

1 participant