Skip to content

fix: send notification on single container update #2357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
kmendell merged 2 commits into from
Apr 12, 2026
+12 −0
Merged

fix: send notification on single container update #2357

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
baa004f
fix: single container update now pings your notifiers too
GiulioSavini Apr 12, 2026
ffbd881
fix: pass raw imageRef to notification, not the normalized copy twice
GiulioSavini Apr 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions backend/internal/services/updater_service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,12 @@ func (s *UpdaterService) UpdateSingleContainer(ctx context.Context, containerID
out.Checked = 1
out.Duration = time.Since(start).String()

if s.notificationService != nil {
if notifErr := s.notificationService.SendContainerUpdateNotification(ctx, containerName, normalizedRef, inspectBefore.Image, s.normalizeRef(normalizedRef)); notifErr != nil {
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant double-normalization for newDigest

normalizedRef is already s.normalizeRef(imageRef), so passing s.normalizeRef(normalizedRef) as newDigest applies the normalizer twice to the same string, making imageRef and newDigest identical in the notification. The bulk-update path intentionally passes the raw p.newRef as imageRef and s.normalizeRef(p.newRef) as newDigest so the two fields can differ (e.g. "nginx:latest" vs "docker.io/library/nginx:latest"). To match that pattern here, pass the pre-normalized imageRef as the second argument and normalizedRef as the last:

Suggested change
if notifErr := s.notificationService.SendContainerUpdateNotification(ctx, containerName, normalizedRef, inspectBefore.Image, s.normalizeRef(normalizedRef)); notifErr != nil {
if notifErr := s.notificationService.SendContainerUpdateNotification(ctx, containerName, imageRef, inspectBefore.Image, normalizedRef); notifErr != nil {

The same applies to the standalone path at line 662.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: backend/internal/services/updater_service.go
Line: 628

Comment:
**Redundant double-normalization for `newDigest`**

`normalizedRef` is already `s.normalizeRef(imageRef)`, so passing `s.normalizeRef(normalizedRef)` as `newDigest` applies the normalizer twice to the same string, making `imageRef` and `newDigest` identical in the notification. The bulk-update path intentionally passes the raw `p.newRef` as `imageRef` and `s.normalizeRef(p.newRef)` as `newDigest` so the two fields can differ (e.g. `"nginx:latest"` vs `"docker.io/library/nginx:latest"`). To match that pattern here, pass the pre-normalized `imageRef` as the second argument and `normalizedRef` as the last:

```suggestion
				if notifErr := s.notificationService.SendContainerUpdateNotification(ctx, containerName, imageRef, inspectBefore.Image, normalizedRef); notifErr != nil {
```

The same applies to the standalone path at line 662.

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

Fix in Codex

slog.WarnContext(ctx, "Failed to send container update notification", "containerID", containerID, "containerName", containerName, "imageRef", normalizedRef, "error", notifErr.Error())
}
}

// Prune old image if no longer in use
if inspectBefore.Image != "" {
_ = s.pruneImageIDsWithInUseSetInternal(ctx, []string{inspectBefore.Image}, nil)
Expand Down Expand Up @@ -652,6 +658,12 @@ func (s *UpdaterService) UpdateSingleContainer(ctx context.Context, containerID
})
out.Updated++

if s.notificationService != nil {
if notifErr := s.notificationService.SendContainerUpdateNotification(ctx, containerName, normalizedRef, inspectBefore.Image, s.normalizeRef(normalizedRef)); notifErr != nil {
slog.WarnContext(ctx, "Failed to send container update notification", "containerID", containerID, "containerName", containerName, "imageRef", normalizedRef, "error", notifErr.Error())
}
}

// Clear the update record for this exact image ID when no running container still uses it.
// This avoids repo-name canonicalization mismatches (e.g. "nginx" vs "docker.io/library/nginx").
if inspectBefore.Image != "" {
Expand Down
Loading