feat(ai-sre-relay): auto-close the relay's Jira ticket after an alert stays resolved [JDWLABS-476] - #213
Merged
Conversation
The relay opened one ticket per fingerprint and never closed one. Once a condition was fixed the ticket stayed open until a human noticed, and every later firing of that alertname was absorbed into it as a "still firing" repeat comment. One fixed job's ticket collected two dozen of those while a different failing job of the same alertname never got its own investigation, and six tickets for a single fixed condition sat open for four days. The missing input was the resolve notification: the Alertmanager receiver has send_resolved off, and the webhook handler dropped resolved alerts anyway, so nothing in the relay ever learned that a condition had cleared. Resolved notifications now reach the pipeline. A resolve is commented on the ticket immediately and starts a grace window (RESOLVED_CLOSE_GRACE, 6h); once the alert has stayed resolved for the whole window the relay comments the resolve time and transitions the ticket to Done. A firing alert cancels a pending close before the investigation starts, because an investigation runs for minutes and a sweep landing inside one would otherwise close the ticket it is about to write to. There are no per-ticket timers. Pending closes are re-evaluated on each resolve notification and on a periodic sweep, so a restart drops them — the ticket simply stays open as it did before — rather than leaving a timer to fire against state that no longer holds. Repeat suppression already re-checked ticket status per refire; a Done ticket therefore never absorbs a firing alert, however it was closed, and a re-fire after an automatic close reopens the same ticket through the existing reopen path. The closing transition is resolved by target status category like the reopen path, not by a hardcoded id. The dispatcher's in-flight coalescing now keys on fingerprint and status: keyed on the fingerprint alone, a resolve arriving during the investigation it belongs to would be coalesced away as a repeat of it and the condition would never be reported as cleared. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvYBM6o7wARm2v9jmDp1yY
Eight risks and five smaller problems came out of review of the auto-close work. The ones that changed behaviour: A resolve consulted only the in-process fingerprint map, while the firing path recovers its ticket from Jira by label. A resolve arriving after a restart with no firing in between was therefore a permanent no-op, and the ticket it belonged to could never be closed by the relay. The resolve path now does the same fingerprint-label lookup and adopts what it finds, so neither direction is stranded by a restart. The sweep decided to close from a snapshot and then made two Jira calls before acting on it. A re-fire landing in that window could not stop the close, and the ticket for a currently-firing alert went to Done. The pending close is now re-read immediately before the transition, and a re-fire that still beats it — landing while the transition is in flight — is repaired by reopening the ticket rather than left for a human to notice. The sweep lock was held across every Jira round-trip in the whole tracking table, and a resolve notification took that lock on a dispatcher worker. A slow Jira would have parked all four workers and turned an outage into refused alerts. A resolve now evaluates only its own fingerprint and gives up rather than waiting; the periodic sweep keeps the full scan. That work also runs on a context detached from the per-alert deadline, because a resolve dropped on an expired context is invisible — the ticket simply never closes — and it was logging healthy tickets as unreadable when the deadline expired mid-sweep. The closing comment was posted before the transition, so a workflow that could not reach Done collected a fresh "Closed automatically" comment on every sweep, forever. The transition goes first now, and a ticket that reached Done without its comment reports that distinctly so the caller counts it closed instead of retrying a Done ticket. Repeat counting wrote a zero-value entry back when the sweep had already forgotten the fingerprint, inserting an unusable entry that held a slot in the bounded tracking table. It is a no-op now, and the repeat it was counting is treated as new work. Also: the resolve now posts the Discord notice the green resolved embed was written for and never reached; a resolve is no longer logged as a completed investigation; and the Nx test target enables the race detector, which is what CI runs, so the goroutine added for the sweep is actually covered there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvYBM6o7wARm2v9jmDp1yY
…ion PR The README named the first attempt at the Alertmanager change, which was auto-closed when its branch was reset and re-committed through the API — the signature check rejects a locally signed commit under the bot author. Name the live one instead, and say plainly that it has not landed: the auto-close path does nothing until the flag flips, and a reader who takes the reference as merged will look for a working feature that cannot run yet. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvYBM6o7wARm2v9jmDp1yY
Round-two review found one real bug. A resolve is allowed to arrive while an investigation for the same fingerprint is still running — the dispatcher tracks firing and resolved as separate in-flight keys precisely so it can — but the investigation finishing rebuilt the tracking entry from scratch and wiped the resolve time with it. The ticket had already been commented and announced in Discord as resolving; the close was then dropped with no log, no metric and nothing to distinguish it from an alert that never resolved. Recording the ticket now merges into the existing entry instead of replacing it. Repeat numbering still restarts when the firing episode changes, since that count belongs to an episode rather than to the fingerprint. The rest are risks that had not bitten yet: Reopening a ticket did not check whether it was Done first, so a close that never landed, or one a human had already reversed, moved the ticket to whatever open status the workflow listed first and commented about a reopen that did not happen. It now re-reads the status and does nothing when the ticket is open. The closing transition took the first transition in the Done category. Real workflows put "Won't Do" and "Duplicate" there too, and closing an incident as a duplicate says something the relay does not mean. It now prefers the target status by name, configurable, with the category as the fallback. The fingerprint search had drifted apart between the two paths: the upsert saw Done tickets, since it has to reopen them, and the resolve lookup filtered them out in the query. With both ordering by creation date and taking one row, a fingerprint carrying a newer Done ticket and an older open one would have sent the two paths to different issues. They share one query now and the resolve path decides on the result instead. A label matching more than one ticket is also worth knowing about, so the search fetches two rows to notice and warns. The sweep never checked its context between tickets, so a shutdown or an expired deadline still walked the whole table one Jira round-trip at a time, and the periodic sweep had no deadline at all. A close that raced a re-fire was counted as an auto-close although the ticket ends up open again, and a failed reopen — which is terminal, leaving a Done ticket for a firing alert — was invisible. Both now have counters, as does an alert dropped at the tracking ceiling, which used to lose repeat suppression and auto-close in silence. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FvYBM6o7wARm2v9jmDp1yY
jdwillmsen
force-pushed
the
feat/JDWLABS-476-relay-ticket-auto-close
branch
from
September 5, 2026 03:51
5896520 to
5be3ea1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The relay opens one Jira ticket per alert fingerprint and, while that ticket is
open, treats every later firing as a repeat and skips the investigation. It
never closed a ticket itself, so once a condition was fixed the ticket stayed
open until a human noticed — and every later firing of that alertname was
absorbed into it as a "still firing" comment. A fixed
porkbun-ddnsKubeJobFailedticket collected 24 repeat comments while a different failingjob of the same alertname never got its own investigation; six tickets for one
fixed
runAsNonRootfailure sat open for four days.The missing input was the resolve notification. The relay had no idea a
condition had ever cleared:
webhook.goenqueued onlystatus: firinganddropped resolved alerts on the floor.
Design
Resolve → grace → close, with no timers.
resolvedfor a fingerprint the relay ownsresolvedrepeatedfiringinside the grace windowfiringafter an automatic closeRESOLVED_CLOSE_GRACE(default6h) andRESOLVED_SWEEP_INTERVAL(default
10m), read through the same env mechanism as the rest of theservice. An unparseable or non-positive value logs a warning and falls back —
a typo must not silently turn auto-close into never-close.
in-process
fingerprint → firingmap as aresolvedAtfield. There are noper-ticket timers:
SweepResolvedevaluates "resolved since" on each incomingresolve notification and on a periodic sweep. A restart therefore drops pending
closes and the ticket simply stays open as it did before this change, rather
than a persisted timer firing against state that no longer holds.
landing mid-investigation would otherwise close the ticket that investigation
is about to write to.
TestPipelineRefireCancelsPendingCloseBeforeInvestigatingpins this by sweeping from inside the investigation.
re-checked
IsOpenper refire, so a Done ticket never suppresses a newinvestigation — that now covers relay-closed tickets too, and is pinned by a
test rather than left implied.
matching the convention the reopen path already established.
reopenand thenew
Closenow share onetransition(ctx, key, toDone bool).fingerprint alone, a resolve arriving while the investigation it belongs to is
still running would be coalesced away as a repeat of it, and nothing else would
ever report the condition cleared.
ai_sre_relay_tickets_auto_closed_total.recovers its ticket through the same
amfp-<fingerprint>label search thefiring path uses, so neither direction is stranded by a restart. What a
restart still loses is an already-pending close, because Alertmanager sends a
resolved notification once rather than repeating it — that ticket waits for a
human, which is the pre-existing behaviour.
the lock immediately before the transition, and a re-fire that lands while
the transition is in flight is repaired by reopening the ticket rather than
leaving a Done ticket for a firing alert.
fingerprint and uses
TryLock, so a slow Jira cannot park the dispatcherworkers behind a full-table sweep; the ticker keeps the full scan. That work
runs on a context detached from the per-alert deadline (
WithoutCancel+timeout), like the Holmes failure notice.
old order meant a workflow that could not reach Done collected one "Closed
automatically" comment per sweep interval forever. A ticket that reached Done
without its comment reports
ErrClosedWithoutNoteso the caller counts itclosed instead of retrying a Done ticket.
testtargetnow sets
race: true. CI runsnx affected -t lint test, so without it thedetector never ran against the goroutine this PR adds.
apps/backend/ai-sre-relay/README.md— lifecycle table, config,metrics, and a "a ticket is not closing" runbook.
Platform dependency:
send_resolved(companion PR open)The
ai-srereceiver intenants/platform/services/kube-prometheus-stack/postInstall/alertmanager-config-externalsecret.yamlstill has
send_resolved: falseon platformmain. The companion change isjdwlabs/platform#420 — open, all 19 checks green, awaiting a human codeowner
approval:
(An earlier attempt, platform#419, was auto-closed when its branch had to be
reset and re-committed through the GitHub API — the signature check rejects a
locally signed commit under the bot author. #420 is the live one.)
This PR is inert until #420 merges: without resolved notifications the
relay never learns a condition cleared and the auto-close path never runs.
Neither PR is merged, so the two can land in either order — this one is safe to
merge first, it simply does nothing until the flag flips. No platform change
was made from this repo. No deployment change is needed for the new config:
both new variables have working defaults.
Worth noting the relay was already half-prepared for resolves —
discord.gorenders a resolved alert green, a branch that had never executed. It is now
reached: a resolve posts a short "resolved, ticket X closes after the grace
period" notice.
Review round 2
One real bug and nine risks, all fixed on this branch (
58965201).The bug:
rememberrebuilt the tracking entry from scratch when aninvestigation finished, wiping
resolvedAt. A resolve arriving mid-investigationfor the same fingerprint — which
inflightKeydeliberately permits — wascommented on the ticket and announced in Discord, then silently discarded: no
log, no metric, ticket never closes. Reproduced two ways (prior-episode mapping
and the adoption path) as failing tests first, then fixed by merging into the
existing entry. Repeat numbering still resets when the firing episode changes,
because that count belongs to an episode, not to a fingerprint.
Reopenre-reads the status and returns nil when the ticket is not Done, instead of moving an open ticket to an arbitrary status with a spurious commentJIRA_DONE_STATUS, defaultDone) with the Done category as fallback, so an incident is not closed as "Won't Do" or "Duplicate"SweepResolvedchecksctx.Err()per iteration, so shutdown and deadlines actually stop itcontext.WithTimeout(sweepCtx, sweepEvery)ai_sre_relay_ticket_reopens_total{result=...}, not as an auto-close; a failed reopen (terminal) has its own result labelmaxTrackedFiringswarns and incrementsai_sre_relay_fingerprints_untracked_totalsend_resolvedTryLocktest joins its goroutine before failingAlso documented in the README restart section, per the reviewer's note: a
resolve's Jira work has a 60s budget while shutdown drains for 25s, so a
rolling update can cut a close between its transition and its comment.
One disagreement, on finding 4
The finding asked for
statusCategory != Doneon both fingerprintsearches. Applying it to
Upsertwould break the reopen path this ticketrequires:
Upserthas to see a Done ticket in order to reopen it rather thanfile a duplicate (
TestJiraUpsertReopensDoneDuplicate). Filtering there wouldsilently start creating a second ticket per fixed-then-refired alert — the
exact duplication the dedup design exists to prevent.
The underlying concern — the two paths diverging when a fingerprint has a newer
Done ticket and an older open one — is real, so it is fixed in the other
direction: both paths now call one
fingerprintHithelper with the identicalunfiltered query, and
FindOpenByFingerprintdecides on the result(
hit.done()→ nothing to close). Same issue selected by both, reopen intact.The ambiguity detection was implemented as asked:
maxResults=2with a warningnaming both keys when a label matches more than one ticket.
Tests
go build ./...,go vet ./...,go test ./... -raceall pass, as doesnx run-many -t lint test build --projects=ai-sre-relay(the targets CI runsvia
nx affected). Each new behaviour was written test-first and the tests weremutation-checked — dropping the up-front cancel, ignoring the grace window, and
keying in-flight on the fingerprint alone each make the corresponding test fail.
Added in the first round:
TestPipelineResolvedAlertIsNotedWithoutClosingTestPipelineRepeatedResolveNotesOnceTestPipelineClosesTicketAfterGraceTestPipelineRefireCancelsPendingCloseTestPipelineRefireCancelsPendingCloseBeforeInvestigatingTestPipelineAutoClosedTicketDoesNotSuppressNextFiringTestPipelineDoneTicketNeverSuppressesRepeatTestPipelineSkipsCloseWhenTicketAlreadyDoneTestPipelineRetriesCloseAfterFailureTestPipelineResolvedAlertWithoutTicketIsIgnoredTestPipelineResolvedAlertWithoutEndsAtUsesReceiptTimeTestCountersExposeTicketsAutoClosedTestJiraNoteResolvedRecordsTheResolveTimeTestJiraCloseCommentsThenTransitionsToDoneTestJiraCloseFailsWithoutDoneTransitionTestDispatcherDoesNotCoalesceResolvedWithFiringAdded for the review findings:
TestPipelineResolveAdoptsTicketAfterRestartTestPipelineResolveSkipsSearchWhenAlreadyTrackedTestPipelineResolveWithNoRecoverableTicketIsIgnoredTestPipelineSweepAbandonsCloseCancelledAfterSnapshotTestPipelineReopensTicketClosedByARacedRefireTestPipelineResolveDoesNotBlockOnARunningSweepTestPipelineResolveSurvivesDeadContextTestPipelineFailedCloseDoesNotRepeatTheClosingCommentTestPipelineCloseWithoutNoteIsNotRetriedTestPipelineRepeatDoesNotResurrectAForgottenFingerprintTestPipelineResolveNotifiesDiscordTestJiraFindOpenByFingerprintSearchesTheFingerprintLabelTestJiraFindOpenByFingerprintReturnsEmptyOnNoMatchTestJiraCloseTransitionsBeforeCommentingTestJiraCloseReportsATransitionedTicketWithNoNoteTestJiraReopenMovesTicketOutOfDoneTestDispatcherLogsResolveSeparatelyFromInvestigationStrengthened:
TestPipelineRepeatedResolveNotesOncenow clearsendsAtandresolves twice five hours apart, so it fails if a later resolve restarts the
grace window (it could not before).
TestPipelineDoneTicketNeverSuppressesRepeatnow keeps the mapping intact and asserts the status check was consulted, so it
passes for the right reason.
TestJiraCloseCommentsThenTransitionsToDonebecameTestJiraCloseUsesTheDoneCategoryTransitionwith the ordering assertioninverted to match the new, correct order.
Every fix above was mutation-checked: reverting each one individually (drop the
restart fallback, drop the pre-close re-read, drop the reopen, swap
TryLockfor
Lock, re-attach the context, letcountRepeatwrite back, drop theDiscord notice, let a repeated resolve restart the clock, restore the old close
order) makes the corresponding test fail.
Added for round 2:
TestPipelineResolveDuringInvestigationSurvivesItsCompletion(the bug)TestPipelineAdoptedResolveSurvivesAConcurrentInvestigation(the bug, adoption path)TestPipelineNewEpisodeRestartsRepeatNumberingTestPipelineRacedCloseIsCountedAsAReopenNotACloseTestPipelineFailedReopenIsCountedTestPipelineSweepStopsOnCancelledContextTestPipelineFullTrackingTableIsVisibleTestCountersExposeReopenAndTrackingOutcomesTestJiraReopenLeavesAnOpenTicketAloneTestJiraClosePrefersTheNamedDoneStatusTestJiraCloseFallsBackToTheDoneCategoryTestJiraCloseHonoursAConfiguredDoneStatusTestJiraFindOpenByFingerprintIgnoresADoneMatchUpdated for the corrected semantics:
TestJiraFindOpenByFingerprintSearchesTheFingerprintLabelnow asserts thequery matches the upsert's (unfiltered) rather than excluding Done;
TestJiraReopenMovesTicketOutOfDoneserves the status re-check.All eight round-2 behaviour changes were mutation-checked individually — most
importantly, restoring the old overwriting
rememberfails both bug tests.Changed:
TestWebhookEnqueuesOnlyFiringAlerts→TestWebhookEnqueuesFiringAndResolvedAlerts(it encoded the behaviour this PRremoves; it now also asserts that other statuses such as
suppressedare stillignored).
Out of scope
Open-ticket dedup is unchanged and still on; no alert-rule thresholds were
touched.
🤖 Generated with Claude Code
https://claude.ai/code/session_01FvYBM6o7wARm2v9jmDp1yY