feat: detect deployed-image vs catalog-image drift and warn (CashPilot-Desktop-kca)#97
Merged
Merged
Conversation
… (CashPilot-Desktop-kca) When a provider retires its client image (as ProxyBase did), an existing deployment keeps running — and so looks healthy (sampleHealth records health_up for any running container) — while earning nothing, and notifications never flagged it. Add catalog.ImageOutdated (with splitImage) comparing each deployment's image against the catalog's current docker.image; surface the drifted slugs on AppState.outdatedServices, badge them 'update available' on the deployed- services table and catalog card, and raise a warning notification prompting a re-deploy. Deliberately conservative: empty images and pure tag-vs-digest differences of the same repo are not flagged. Mirrors the web repo's 5wi.
📝 WalkthroughWalkthroughAdds conservative Docker image-drift detection, exposes outdated service slugs through app state, emits redeployment warnings, and displays update-available badges in the service catalog and deployed-services table. ChangesOutdated service detection and presentation
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #97 +/- ##
==========================================
+ Coverage 74.92% 75.03% +0.10%
==========================================
Files 14 14
Lines 3418 3433 +15
==========================================
+ Hits 2561 2576 +15
Misses 669 669
Partials 188 188
🚀 New features to boost your workflow:
|
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
app.go (1)
365-376: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueAvoid redundant iteration by passing the pre-computed
outdatedServicestonotifications.
a.outdatedServices(deployments)is called once for theAppStatestruct and then again insidea.notifications(..., deployments). You can compute it once and pass the result down to keep things DRY.♻️ Proposed refactor
Compute it once in
GetAppState:deployments := a.store.ListDeployments() earnings := a.store.ListLatestEarnings() + outdated := a.outdatedServices(deployments) return AppState{ Config: a.cfg.Config(), Runtime: runtimeStatus, Services: a.catalog.ListVisible(), Deployments: deployments, - OutdatedServices: a.outdatedServices(deployments), + OutdatedServices: outdated, Earnings: earnings, Guides: runtime.InstallGuides(), - Notifications: a.notifications(runtimeStatus, earnings, deployments), + Notifications: a.notifications(runtimeStatus, earnings, deployments, outdated), Currencies: supportedCurrencies(),Then update the
notificationssignature to accept it:-func (a *App) notifications(status runtime.Status, earnings []store.EarningsRecord, deployments []store.Deployment) []Notification { +func (a *App) notifications(status runtime.Status, earnings []store.EarningsRecord, deployments []store.Deployment, outdated []string) []Notification { var items []Notification // ... - for _, slug := range a.outdatedServices(deployments) { + for _, slug := range outdated { name := slug🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app.go` around lines 365 - 376, In GetAppState, compute a.outdatedServices(deployments) once and reuse that value for both the AppState.OutdatedServices field and the notifications call. Update notifications to accept the precomputed outdated services parameter and remove its internal recomputation, preserving existing notification behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@app.go`:
- Around line 365-376: In GetAppState, compute a.outdatedServices(deployments)
once and reuse that value for both the AppState.OutdatedServices field and the
notifications call. Update notifications to accept the precomputed outdated
services parameter and remove its internal recomputation, preserving existing
notification behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b0950f20-f9a9-4850-ab4b-920b9f4d2a9e
📒 Files selected for processing (6)
app.goapp_test.gofrontend/src/main.tsfrontend/src/wails.d.tsinternal/catalog/catalog.gointernal/catalog/catalog_test.go
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.
Problem (from the ProxyBase migration review — CashPilot#103 / desktop PR #96)
sampleHealthrecordshealth_upfor any container that is running, regardless of its image, andnotifications()never alerted on a deployed-but-worthless container. So when a provider retires its client image (as ProxyBase did,proxybase/proxybase→ghcr.io/proxybaseorg/peer-cli), an existing deployment stays green while earning $0.Fix
catalog.ImageOutdated(deployed, catalogImage)(+splitImage) — true when the provider changed the image repo/path or the catalog re-pinned to a different digest. Conservative: empty images and a pure tag-vs-digest difference of the same repo are not flagged.App.outdatedServices(deployments)compares each deployment'sImageagainstcatalog.Get(slug).Docker.Image; the drifted slugs ride onAppState.outdatedServices.update availablebadge on both the deployed-services table row (next to status) and the catalog card (next to "Deployed").notifications()raises a warning per drifted service prompting a re-deploy.Health semantics are unchanged (a running container still counts as up); drift is an additive, separate signal — matching the web repo's 5wi.
Tests
catalog:TestSplitImage+TestImageOutdated(repo migration, digest re-pin, identical, empty, tag-vs-digest, tag-only).app:TestOutdatedServices(drift flagged, match ignored, unknown slug ignored) +TestNotificationsFlagsOutdated.go build,go vet,go test -race ./...green; frontendtsctypechecks.Closes bead CashPilot-Desktop-kca. Mirror of the web bead CashPilot-5wi.
Summary by CodeRabbit