fix(alertmanager): don't silently assign new alert issues to an uninvokable agent

[allyblockcast]

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Issue creation/routing is a capability plugins and server routes both do — an alertmanager-origin issue's assignee agentId is resolved (label override, owner-map, or issue-route) and handed straight to ctx.issues.create
• That resolution never checks whether the resolved agent can actually act on the issue — a paused/terminated agent is still assignable (server-side assertAssignableAgent), so the create call succeeds and the row silently lands on a dead run path
• Confirmed live in BLO-26613: three plugin:paperclip-plugin-alertmanager alerts (BLO-26609/26610/26611) fired against a paused Release Engineer ~37h after its pause, and unlike an inert todo row a firing alert keeps re-triggering while looking handled — the sibling Blockcast/paperclip#1344 already fixed the analogous Dependabot alert path
• This PR closes the same gap in the alertmanager plugin specifically, since it's the other confirmed-live intake path named in the issue
• The benefit is new alertmanager-origin issues fall back to unassigned (pickable via allow_company_agent) instead of queuing invisibly on an agent that will never look at them

Linked Issues or Issue Description

Refs: BLO-26613 — "Issue creation performs no assignee-invokability check". That issue's AC calls out paperclip-plugin-alertmanager's assignee resolution (webhook-handler.ts) as a confirmed-live instance of the same gap already fixed for Dependabot in #1344. Searched open/recent PRs for alertmanager + invokab + BLO-26613 — no duplicate or overlapping PR found.

What Changed

• Added resolveInvokableAssigneeAgentId in owner-resolver.ts — looks up the resolved agent via ctx.agents.get and gates on @paperclipai/shared's isAgentStatusInvokable (mirrors server/src/services/dependabot-alert-issues.ts's resolveDependabotIssueAssigneeId). Falls back to undefined (unassigned) and logs a distinct warning when the agent is missing or not invokable.
• Wired it into webhook-handler.ts's handleFiring issue-creation path (first-time-seen-fingerprint branch only — re-fires against an already-created issue don't re-resolve or reassign).
• agents.read was already a declared plugin capability; no manifest change needed.
• Added regression coverage in owner-resolver.test.ts (6 new cases: invokable pass-through, paused fallback, agent-not-found fallback, terminated/pending_approval fallback, no-agentId short-circuit).
• Updated the pre-existing worker.test.ts and escalation.test.ts ctx mocks to stub ctx.agents.get (default: invokable), since those fixtures predate the guard and would otherwise throw on the new call.

Verification

• pnpm exec vitest run in packages/plugins/paperclip-plugin-alertmanager — 179/179 pass.
• Confirmed the new test is a real regression test: stashed just the two source-file changes (kept the test), reran — 6 failures (TypeError: ... is not a function / assignment not gated), matching "must fail against current HEAD". Restored the fix, reran green.
• pnpm exec tsc --noEmit in the plugin package — clean.

Risks

Low. Scoped to one plugin's issue-creation path; re-fire/resolve paths and every other assignee-resolution branch (users, routes without an agent target) are untouched. Worst case on a bug here is an alertmanager issue landing unassigned instead of on its configured owner — recoverable via allow_company_agent, not a silent failure. Does not touch the generic assertAssignableAgent/issueService.create() check (deliberately out of scope, same as #1344 — tightening "assignable" company-wide needs a separate blast-radius review).

Model Used

Claude Sonnet 5 (claude-sonnet-5), extended reasoning, via Claude Code — tool use (repo clone, vitest, tsc) to implement and verify.

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, internal fix mirrors an existing documented pattern
• I have considered and documented any risks above
• All Paperclip CI gates are green — pending CI run 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-26613

[allyblockcast]

🔗 Paperclip issue: BLO-26611
🔗 Paperclip issue: BLO-26609
🔗 Paperclip issue: BLO-26610
🔗 Paperclip issue: BLO-26613

[allyblockcast]

🔗 Paperclip issue: BLO-26611
🔗 Paperclip issue: BLO-26609
🔗 Paperclip issue: BLO-26610
🔗 Paperclip issue: BLO-26613

[allyblockcast]

Ally — Consolidated PR Review

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

The core change is sound and the helper semantics are correct — isAgentStatusInvokable treats paused as non-invokable while idle/active/running/error pass (packages/shared/src/agent-eligibility.ts:59-69), which is exactly the BLO-26613 gap. Two issues below concern what happens around the new call rather than the check itself.

Critical Issues (0)

Important Issues (2)

• [gstack/review, native-codex] packages/plugins/paperclip-plugin-alertmanager/src/owner-resolver.ts:192 — the new ctx.agents.get call is unguarded, adding a hard RPC dependency to the alert-issue creation path with no degraded fallback.

  • The sibling resolver 60 lines above does the opposite: resolveOwnerUserId wraps ctx.users.findByEmail in try/catch and degrades to undefined (owner-resolver.ts:129-137). The two owner-resolution helpers in the same file now have opposite failure semantics, and the new docstring doesn't say which is intended.
  • Concretely, if the agents RPC is unavailable, the throw is caught per-alert at webhook-handler.ts:638, the fingerprint is pushed to failedFingerprints, and AlertDeliveryIncompleteError fails the delivery so Alertmanager retries. That retry path is well-built, but if the RPC stays down through Alertmanager's retry budget the alert is lost with no issue row at all — strictly worse than the paused-assignee bug this PR fixes, where at least a visible issue existed.
  • Recommendation: wrap the lookup and return undefined on error. That degrades to unassigned — precisely the fallback this PR already argues for — instead of trading a silent-assignment failure for a silent-loss failure. If failing the delivery is the deliberate choice, say so in the docstring so the asymmetry with resolveOwnerUserId is intentional rather than incidental.

• [pr-review-toolkit/code] packages/plugins/paperclip-plugin-alertmanager/src/webhook-handler.ts:308 — createAssigneeUserId is derived from the pre-guard resolvedAssigneeAgentId, so when the guard drops a non-invokable agent it silently discards an already-resolved human owner too, and the issue is filed with no assignee of either kind.

  • Repro path: ownerOverride === false, the issue-route supplies an assigneeAgentId for a paused agent, and the owner-map separately resolves an email to a real assigneeUserId. Line 305 makes resolvedAssigneeAgentId truthy → line 308 forces createAssigneeUserId = undefined → line 317 nulls the agent → lines 356-357 spread neither key, so the issue lands fully unassigned.
  • The same applies when the route itself carries both assigneeAgentId and assigneeUserId: the route's own human fallback is dropped.
  • This isn't a regression (the old code assigned the paused agent and dropped the user as well), but it is an incomplete fix — a resolvable human owner is a better landing spot than allow_company_agent pickup, and the PR's stated goal is getting the alert in front of someone who will act.
  • Recommendation: move the guard above the user derivation and key it off the post-guard id (const createAssigneeUserId = createAssigneeAgentId ? undefined : …). The ownerOverride branch is unaffected — resolveAssigneeUserId already returns email and agent mutually exclusively (owner-resolver.ts:155-168) — so the reorder only adds the fallback in the route case.

Suggestions (3)

• [pr-review-toolkit/comments] packages/plugins/paperclip-plugin-alertmanager/src/owner-resolver.ts:176-177 — the docstring cites server/src/services/dependabot-alert-issues.ts resolveDependabotIssueAssigneeId as the mirrored source, but that symbol does not exist at this head; #1344 is still open, and the file's only exports are findOpenDependabotAlertIssue, recordDependabotWebhookDiagnostic, and resolveStaleDependabotAlertWakeIssue. The PR body's "the sibling #1344 already fixed the analogous Dependabot alert path" reads the same way. Suggest "mirrors the approach proposed in #1344" until it merges, so someone grepping for the symbol isn't chasing a ghost.
• [native-codex] The guard runs only on the first-seen-fingerprint branch, so an agent paused after its issue was created keeps collecting re-fires against the stored existing.assigneeAgentId. The scope call is reasonable and stated in the PR body, but BLO-26613's symptom explicitly includes "keeps re-triggering while looking handled", so the re-fire path is worth a follow-up issue rather than leaving it implicit.
• [pr-review-toolkit/tests] packages/plugins/paperclip-plugin-alertmanager/src/__tests__/owner-resolver.test.ts:447 — the six new cases cover status outcomes well, but there's no case where agents.get rejects. Adding one would pin down whichever failure semantics you pick for the first Important finding.

Strengths

• The regression test was verified as a real one — stashing only the source changes and rerunning produced 6 failures. That's the check most PRs skip.
• Mutual exclusivity of email vs. agent resolution is preserved; the guard is inserted without disturbing the existing precedence chain.
• agents.read was genuinely already declared (src/manifest.ts:29), so the "no manifest change needed" claim checks out.
• Pre-existing ctx fixtures were updated with an explanatory comment about why the default is invokable, rather than silently stubbed.
• Scope discipline: the generic assertAssignableAgent path is deliberately left alone, with the blast-radius reasoning stated.

CI note

Both red checks are infrastructure, not this change: General tests (workspaces-b) failed at the Checkout repository step, and Typecheck + Release Registry has no failed step at all — both logs end in The runner has received a shutdown signal / The operation was canceled. The pnpm "Failed to create bin … dev-cli.js" warnings are pre-existing plugin build-order noise across every plugin package, unrelated to this diff. These need a re-run, not a code fix.

Recommended Action

1. No Critical issues — nothing blocks on correctness of the invokability check itself.
2. Address both Important issues this cycle: guard the agents.get call so an RPC outage degrades to unassigned rather than risking alert loss, and reorder the user-fallback derivation so a resolved human owner survives the agent being dropped.
3. Consider the Suggestions opportunistically; the docstring reference to the unmerged #1344 symbol is the cheapest of them to fix.
4. Re-run the two cancelled CI jobs.