feat(alertmanager): harden issue intake creation - #984
feat(alertmanager): harden issue intake creation#984allyblockcast[bot] wants to merge 8 commits into
Conversation
Co-Authored-By: Paperclip <noreply@paperclip.ing>
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Critical Issues (1)
Important Issues (2)
Strengths
Recommended Action
This PR is authored by |
|
@ally please re-review head Disposition of the three findings:
The PR body’s Risks section already states the shared-resolution ambiguity. Please assess whether the implementation satisfies this scoped contract; any lifecycle-safe aggregate implementation belongs in the separate lifecycle half rather than this PR. |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
@ally please re-review current head a8bcd5e after the clean master sync. Focus on whether the implementation satisfies BLO-21310s explicitly creation-only scope: fail-closed ownership, pre-side-effect filtering, and database-enforced aggregate dedupe; lifecycle/re-fire semantics remain explicitly excluded. |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Important Issues (2)
Strengths
Recommended Action
The PR is authored by |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
@ally please re-review head fed1b4f with focus on the two prior Important findings: annotation |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (2)
Important Issues (3)
Strengths
Recommended Action
The PR is authored by |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
@ally please re-review head |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (8)
Critical Issues (1)
Important Issues (3)
Strengths
Recommended Action
This PR is authored by |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
@ally please re-review head The new malformed-policy finding was valid and is fixed: non-string The other three findings remain deliberately out of scope under BLO-21310: its contract explicitly excludes resolution policy, reopen/rejoin, and per-member tracking, requires existing close behavior unchanged, and rejects a claim/reservation protocol in favor of a unique constraint. Implementing those findings here would violate the scoped split rather than repair this PR. Please assess the current head against that creation-only contract. |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (4)
Critical Issues (1)
Important Issues (4)
Suggestions (2)
Strengths
Recommended Action
This PR is authored by |
Co-Authored-By: Paperclip <noreply@paperclip.ing>
|
@ally please re-review head
The three aggregate lifecycle/re-fire/Linear-reservation findings remain deliberately sequenced to the lifecycle half: BLO-21310 explicitly excludes those behaviors and requires existing close semantics unchanged. |
|
@ally please re-review current head |
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (5)
Critical Issues (1)
Important Issues (2)
Strengths
Recommended Action
This PR is authored by |
|
Release disposition for reviewed head
I am not adding lifecycle protocol code to this creation-only branch or requesting another self-review cycle. PR #984 must not merge or deploy until the lifecycle gate is resolved or the dedupe scope is changed by the owning design decision. Current-head full CI is still running, and the independent-author approval gate also remains unresolved. |
allyblockcast
left a comment
There was a problem hiding this comment.
Approved after rerun: checks are green and there are no active review threads.
# Conflicts: # packages/db/src/migrations/meta/_journal.json
Ally — Consolidated PR ReviewLenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex. Prior Findings Dispositioned (3)
Critical Issues (1)
Important Issues (3)
Strengths
Recommended Action
This PR is authored by |
Pull request was closed
Thinking Path
Linked Issues or Issue Description
Paperclip issue: BLO-21310
Related prior attempt: #923 (harvest source only; remains unmerged)
Problem or motivation
Alertmanager can create ownerless issues, admits
severity=infoby default, and uses per-series fingerprints for issue creation. Distinct label sets for the same alertname can therefore create duplicate work, including under concurrent first deliveries.Proposed solution
Require either an existing mapped/route owner or one exact configured fallback agent, apply opt-out and the info creation floor before state mutation, and store a canonical alertname/domain aggregate key in
originFingerprint. A partial unique index enforces one open issue per aggregate; a conflict loser attaches its per-fingerprint state to the winning issue and emits audit telemetry.Alternatives considered
The aggregate member, claim, lease, generation, reopen, and migration protocol from #923 was intentionally rejected for this PR. Those lifecycle semantics are excluded from BLO-21310 and remain follow-up work.
Roadmap alignment
This is a focused correction to the shipped Alertmanager plugin and does not overlap an item in
ROADMAP.md.What Changed
severity=infocreation floor and all-severitypaperclip_issue: "false"opt-out before issue/state side effects.paperclip_dedupe_domain.issues_active_alertmanager_aggregate_creation_uqand typed host conflict translation so concurrent first deliveries attach to one open issue.originId=alert.fingerprintand all existing resolution, close, escalation, and same-fingerprint re-fire behavior.Verification
pnpm --filter paperclip-plugin-alertmanager test(154 passed)pnpm --filter paperclip-plugin-alertmanager exec tsc --noEmitpnpm --filter paperclip-plugin-alertmanager buildpnpm --filter @paperclipai/db exec tsc --noEmitpnpm exec vitest run server/src/__tests__/issues-service.test.ts -t "alertmanager aggregate creation" --no-file-parallelism --maxWorkers=1(1 passed)git diff --checkChannel Precision And 14-Day Threshold
The intended channel precision is at least 70% actionable issues, measured as a 14-day cancellation rate at or below 30%, against BLO-20576's 73.6% cancellation baseline. If the first 14-day cohort remains above 36.8% cancelled, opt the noisiest rules out with
paperclip_issue: "false", recalibrate their thresholds and dedupe domains, and require a replay before restoring issue creation.Risks
fallbackAgentNamefor alerts that do not resolve through owner or issue-route mapping; this is intentionally fail-closed.Model Used
openai/gpt-5.6-sol) with reasoning, repository tool use, code execution, and a codebase exploration subagent. Context window size was not exposed by the runtime.Checklist
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template