Feat/backend versioning#6
Conversation
Add Version, URI, and Digest fields to BackendMetadata for tracking installed backend versions and enabling upgrade detection. Add Version field to GalleryBackend. Add UpgradeAvailable/AvailableVersion fields to SystemBackend. Implement GetImageDigest() for lightweight OCI digest lookups via remote.Head. Record version, URI, and digest at install time in InstallBackend() and propagate version through meta backends.
Add CheckBackendUpgrades() to compare installed backend versions/digests against gallery entries, and UpgradeBackend() to perform atomic upgrades with backup-based rollback on failure. Includes Agent A's data model changes (Version/URI/Digest fields, GetImageDigest).
Add configuration and runtime settings for backend auto-upgrade: - RuntimeSettings field for dynamic config via API/JSON - ApplicationConfig field, option func, and roundtrip conversion - CLI flag with LOCALAI_AUTO_UPGRADE_BACKENDS env var - Config file watcher support for runtime_settings.json - Tests for ToRuntimeSettings, ApplyRuntimeSettings, and roundtrip
- Add upgrade check/trigger API endpoints to config and api module - Backends page: version badge, upgrade indicator, upgrade button - Manage page: version in metadata, context-aware upgrade/reinstall button - Settings page: auto-upgrade backends toggle
- UpgradeChecker background service: checks every 6h, auto-upgrades when enabled - API endpoints: GET /backends/upgrades, POST /backends/upgrades/check, POST /backends/upgrade/:name - CLI: `localai backends upgrade` command, version display in `backends list` - BackendManager interface: add UpgradeBackend and CheckUpgrades methods - Wire upgrade op through GalleryService backend handler - Distributed mode: fan-out upgrade to worker nodes via NATS
In distributed mode with multiple frontend instances, use PostgreSQL advisory lock (KeyBackendUpgradeCheck) so only one instance runs periodic upgrade checks and auto-upgrades. Prevents duplicate upgrade operations across replicas. Standalone mode is unchanged (simple ticker loop).
- Test GET /api/backends/upgrades returns 200 (even with no upgrade checker) - Test POST /api/backends/upgrade/:name accepts request and returns job ID - Test full upgrade flow: trigger upgrade via API, wait for job completion, verify run.sh updated to v2 and metadata.json has version 2.0.0 - Test POST /api/backends/upgrades/check returns 200 - Fix nil check for applicationInstance in upgrade API routes
…kends - Add upgrade banner on Backends page showing count and Upgrade All button - Fix upgrade detection for backends installed before version tracking: flag as upgradeable when gallery has a version but installed has none - Fix OCI digest check to flag backends with no stored digest as upgradeable
PR SummaryWhat Changed
Key Changes by AreaBackend Management: Added Configuration: New HTTP API: New endpoints OCI: New React UI: Upgrade banners, version-aware filtering, per-backend upgrade buttons, and auto-upgrade toggle in Settings. Files Changed
Review Focus Areas
ArchitectureDesign Decisions:
Scalability & Extensibility:
Risks:
Merge StatusNOT MERGEABLE — PR Score 14/100, below threshold (50)
|
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Backend Upgrade System with Distributed CoordinationComplex complexity • Components: UpgradeChecker, Application, BackendsUpgrade CLI command sequenceDiagram
title Backend Upgrade System Workflow
participant User as User/Admin
participant CLI as CLI Commands
participant App as Application
participant UC as UpgradeChecker
participant AL as AdvisoryLock
participant Gallery as Gallery Service
participant DB as PostgreSQL
participant API as HTTP API
Note over App,UC: Startup Phase
App->>App: New() startup.go:234
alt BackendGalleries configured
App->>UC: NewUpgradeChecker(db)
Note right of UC: db is nil in standalone modebr/db is authDB in distributed mode
App->>UC: Run(ctx) as goroutine
UC->>UC: 30s initial delay
UC->>Gallery: CheckBackendUpgrades()
Gallery-->>UC: map of available upgrades
UC->>UC: Cache results in lastUpgrades
end
Note over UC,AL: Distributed Mode Only
alt db is not nil distributed mode
UC->>AL: RunLeaderLoop(key, interval)
loop Every 6 hours
AL->>DB: Acquire advisory lock
alt Lock acquired
AL->>UC: runCheck callback
UC->>Gallery: CheckBackendUpgrades()
Gallery-->>UC: upgrade info
UC->>UC: Update cache
alt AutoUpgradeBackends enabled
UC->>Gallery: UpgradeBackend()
Gallery-->>UC: success/failure
UC->>Gallery: Re-check for fresh cache
end
else Lock not acquired
Note right of AL: Other instance holds lockbr/Skip periodic check
end
end
else Standalone mode
UC->>UC: Simple ticker loop
end
Note over User,CLI: Manual Operations
User->>CLI: local-ai backends list
CLI->>Gallery: CheckBackendUpgrades()
Gallery-->>CLI: Display upgradeable backends
User->>CLI: local-ai backends upgrade name
CLI->>Gallery: UpgradeBackend()
Gallery-->>CLI: Success/failure result
Note over User,API: Runtime Configuration
User->>API: POST runtime settings
API->>App: ApplyRuntimeSettings
App->>App: Update AutoUpgradeBackends
Note right of App: Config watcher appliesbr/settings without restart
Note over UC,API: On-Demand Operations
API->>UC: TriggerCheck()
UC->>UC: Non-blocking trigger to triggerCh
UC->>Gallery: CheckBackendUpgrades()
Gallery-->>UC: Fresh results
UC->>UC: Update cache
API->>UC: GetAvailableUpgrades()
UC-->>API: Return cached lastUpgrades copy
Note over UC: Shutdown
App->>UC: Shutdown()
UC->>UC: Close stop channel
UC->>UC: Wait for done channel
UC-->>App: Graceful exit
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
Greptile SummaryThis PR adds backend versioning and upgrade management to LocalAI: version/URI/OCI-digest fields are recorded in install metadata, a background
Confidence Score: 3/5Not safe to merge as-is: the auto-upgrade path is broken in distributed mode and the upgrade executor can silently leave a backend directory missing on a double-rename failure. The initial runCheck — intentionally run on every instance to warm the cache — also executes UpgradeBackend for every detected upgrade when AutoUpgradeBackends is enabled. This bypasses the advisory lock that was added to prevent concurrent upgrades in distributed deployments, meaning all replicas race to upgrade the same backend directory simultaneously on startup and on every TriggerCheck call. Separately, when the atomic swap in UpgradeBackend fails and the backup restoration rename also fails, the backend directory at its expected path is gone while the backup sits orphaned at backupPath; the returned error does not mention the backup location, leaving operators with no recovery path. Both issues are in the core upgrade execution path that this PR introduces. core/application/upgrade_checker.go and core/gallery/upgrade.go need the most attention; the distributed advisory-lock gap and the error-handling gap on double-rename failure both live there. Important Files Changed
Reviews (1): Last reviewed commit: "feat: upgrade banner with Upgrade All bu..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| uc.lastCheckTime = time.Now() | ||
| if err != nil { | ||
| xlog.Debug("Backend upgrade check failed", "error", err) | ||
| uc.mu.Unlock() | ||
| return | ||
| } | ||
| uc.lastUpgrades = upgrades | ||
| uc.mu.Unlock() | ||
|
|
||
| if len(upgrades) == 0 { | ||
| xlog.Debug("All backends up to date") | ||
| return | ||
| } | ||
|
|
||
| // Log available upgrades | ||
| for name, info := range upgrades { | ||
| if info.AvailableVersion != "" { | ||
| xlog.Info("Backend upgrade available", | ||
| "backend", name, | ||
| "installed", info.InstalledVersion, |
There was a problem hiding this comment.
runCheck is called unconditionally before the advisory-lock loop starts (line 151). Because runCheck also executes UpgradeBackend for each detected upgrade when AutoUpgradeBackends=true, every frontend replica will simultaneously attempt to upgrade the same backends on startup — the advisory lock that is meant to prevent exactly this race does not protect the initial check. Concurrent UpgradeBackend calls on the same backend can corrupt the directory (two instances racing through the backendPath → backupPath → tmpPath → backendPath rename sequence on the same paths).
The same issue applies to TriggerCheck-initiated calls (line 168): on-demand checks run on every instance and also execute auto-upgrades without holding the advisory lock.
| if err := os.Rename(backendPath, backupPath); err != nil { | ||
| os.RemoveAll(tmpPath) | ||
| return fmt.Errorf("failed to move current backend to backup: %w", err) | ||
| } | ||
|
|
||
| if err := os.Rename(tmpPath, backendPath); err != nil { | ||
| // Restore backup on failure | ||
| xlog.Error("Failed to move new backend into place, restoring backup", "error", err) | ||
| if restoreErr := os.Rename(backupPath, backendPath); restoreErr != nil { | ||
| xlog.Error("Failed to restore backup", "error", restoreErr) | ||
| } | ||
| os.RemoveAll(tmpPath) | ||
| return fmt.Errorf("failed to move new backend into place: %w", err) | ||
| } | ||
|
|
There was a problem hiding this comment.
When os.Rename(tmpPath, backendPath) fails (step 2 of the atomic swap), the code attempts to restore the backup. If that restoration rename also fails, the backend at backendPath is permanently gone — the old content sits stranded at backupPath with no reference in the returned error. The caller only sees "failed to move new backend into place" and has no indication that the backend directory is now missing from its expected path. The subsequent os.RemoveAll(tmpPath) also deletes the new download, leaving both paths orphaned. The error should include the backup path so operators can manually recover.
| uc.mu.Lock() | ||
| uc.lastUpgrades = freshUpgrades | ||
| uc.mu.Unlock() | ||
| } |
There was a problem hiding this comment.
Shutdown panics on double-close
Shutdown calls close(uc.stop) with no guard. Calling it a second time, or calling it after the context is already cancelled (which causes Run to return and close uc.done), will panic with "close of closed channel". A sync.Once around the close would make this safe.
|
|
||
| // distributedDB returns the PostgreSQL database for distributed coordination, | ||
| // or nil in standalone mode. | ||
| func (a *Application) distributedDB() *gorm.DB { | ||
| if a.distributed != nil { | ||
| return a.authDB | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
distributedDB may return nil in distributed mode when auth is not configured
distributedDB returns a.authDB when a.distributed != nil. But authDB is only populated when auth is separately enabled (auth.InitDB); a deployment running distributed mode without auth will have distributed != nil but authDB == nil. In that case NewUpgradeChecker receives a nil DB and silently falls back to standalone mode, meaning every frontend replica runs its own independent periodic-check loop without the advisory-lock coordination that was intended for distributed deployments.
![codity-ai[bot]](https://avatars.githubusercontent.com/in/1102452?s=60&v=4){kind=small}
|
|
||
| // UpgradeBackend fans out a backend upgrade to all healthy worker nodes. | ||
| // TODO: Add dedicated NATS subject for upgrade (currently reuses install with force flag) | ||
| func (d *DistributedBackendManager) UpgradeBackend(ctx context.Context, name string, progressCb galleryop.ProgressCallback) error { |
There was a problem hiding this comment.
In distributed mode, POST /api/backends/upgrade/ upgrades only worker nodes because DistributedBackendManager.UpgradeBackend never invokes the local backend manager, so upgrading a backend installed on the frontend itself leaves the frontend on the old version while workers move to the new one.
| func (d *DistributedBackendManager) UpgradeBackend(ctx context.Context, name string, progressCb galleryop.ProgressCallback) error { | |
| Call d.local.UpgradeBackend(ctx, name, progressCb) before or after the worker fan-out and include its error in the joined result so the frontend host is upgraded too. |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/services/nodes/managers_distributed.go
Lines: 180-180
Issue Type: functional-high
Severity: high
Issue Description:
In distributed mode, POST /api/backends/upgrade/<name> upgrades only worker nodes because DistributedBackendManager.UpgradeBackend never invokes the local backend manager, so upgrading a backend installed on the frontend itself leaves the frontend on the old version while workers move to the new one.
Current Code:
func (d *DistributedBackendManager) UpgradeBackend(ctx context.Context, name string, progressCb galleryop.ProgressCallback) error {
allNodes, err := d.registry.List(context.Background())
if err != nil {
return err
}
galleriesJSON, _ := json.Marshal(d.backendGalleries)
var errs []error
for _, node := range allNodes {
if node.Status != StatusHealthy {
continue
}
// Reuse install endpoint which will re-download the backend (force mode)
reply, err := d.adapter.InstallBackend(node.ID, name, "", string(galleriesJSON))
if err != nil {
if errors.Is(err, nats.ErrNoResponders) {
xlog.Warn("No NATS responders for node during upgrade, marking unhealthy", "node", node.Name, "nodeID", node.ID)
d.registry.MarkUnhealthy(context.Background(), node.ID)
continue
}
errs = append(errs, fmt.Errorf("node %s: %w", node.Name, err))
continue
}
if !reply.Success {
errs = append(errs, fmt.Errorf("node %s: %s", node.Name, reply.Error))
}
}
return errors.Join(errs...)
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| func (uc *UpgradeChecker) Shutdown() { | ||
| close(uc.stop) | ||
| <-uc.done |
There was a problem hiding this comment.
Shutdown closes uc.stop unconditionally, so a second call will panic; guard it with sync.Once or a non-blocking close path.
Suggested fix
func (uc *UpgradeChecker) Shutdown() {
uc.shutdownOnce.Do(func() {
close(uc.stop)
})
<-uc.done
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 119-121
Issue Type: functional-high
Severity: high
Issue Description:
Shutdown closes uc.stop unconditionally, so a second call will panic; guard it with sync.Once or a non-blocking close path.
Current Code:
func (uc *UpgradeChecker) Shutdown() {
close(uc.stop)
<-uc.done
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| // Check for upgrades | ||
| upgrades, _ := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState) |
There was a problem hiding this comment.
The error from CheckBackendUpgrades is discarded here, so return or surface it instead of silently hiding upgrade-check failures.
Suggested fix
// Check for upgrades
upgrades, err := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState)
if err != nil {
return err
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/cli/backends.go
Lines: 75-76
Issue Type: robustness-high
Severity: high
Issue Description:
The error from CheckBackendUpgrades is discarded here, so return or surface it instead of silently hiding upgrade-check failures.
Current Code:
// Check for upgrades
upgrades, _ := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState)
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| backendPath := filepath.Join(systemState.Backend.BackendsPath, backendName) | ||
| tmpPath := backendPath + ".upgrade-tmp" | ||
| backupPath := backendPath + ".backup" |
There was a problem hiding this comment.
backendName is joined directly into a filesystem path without validation, so reject path separators and clean-path escapes before using it in backendPath.
Suggested fix
if backendName == "" || filepath.Base(backendName) != backendName || backendName == "." || backendName == ".." {
return fmt.Errorf("invalid backend name %q", backendName)
}
backendPath := filepath.Join(systemState.Backend.BackendsPath, backendName)
tmpPath := backendPath + ".upgrade-tmp"
backupPath := backendPath + ".backup"Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/gallery/upgrade.go
Lines: 145-147
Issue Type: security-high
Severity: high
Issue Description:
backendName is joined directly into a filesystem path without validation, so reject path separators and clean-path escapes before using it in backendPath.
Current Code:
backendPath := filepath.Join(systemState.Backend.BackendsPath, backendName)
tmpPath := backendPath + ".upgrade-tmp"
backupPath := backendPath + ".backup"
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| uri := downloader.URI(galleryEntry.URI) | ||
| if uri.LooksLikeDir() { | ||
| if err := cp.Copy(string(uri), tmpPath); err != nil { |
There was a problem hiding this comment.
Copying a directory URI without enforcing it stays under a trusted root allows gallery-controlled local path reads, so validate and constrain local directory sources before cp.Copy.
Suggested fix
uri := downloader.URI(galleryEntry.URI)
if uri.LooksLikeDir() {
srcPath := filepath.Clean(string(uri))
trustedRoot := filepath.Clean(systemState.Backend.BackendsPath)
rel, err := filepath.Rel(trustedRoot, srcPath)
if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) {
os.RemoveAll(tmpPath)
return fmt.Errorf("backend source path %q is outside trusted root", srcPath)
}
if err := cp.Copy(srcPath, tmpPath); err != nil {
os.RemoveAll(tmpPath)
return fmt.Errorf("failed to copy backend from directory: %w", err)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/gallery/upgrade.go
Lines: 158-160
Issue Type: security-high
Severity: high
Issue Description:
Copying a directory URI without enforcing it stays under a trusted root allows gallery-controlled local path reads, so validate and constrain local directory sources before cp.Copy.
Current Code:
uri := downloader.URI(galleryEntry.URI)
if uri.LooksLikeDir() {
if err := cp.Copy(string(uri), tmpPath); err != nil {
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| galleryService.BackendGalleryChannel <- galleryop.ManagementOp[gallery.GalleryBackend, any]{ | ||
| ID: uid.String(), | ||
| GalleryElementName: backendName, | ||
| Galleries: appConfig.BackendGalleries, | ||
| Upgrade: true, | ||
| } |
There was a problem hiding this comment.
Sending directly to BackendGalleryChannel can block the handler indefinitely if the channel is unbuffered or saturated, so enqueue in a goroutine or use a bounded non-blocking/select-based send.
Suggested fix
op := galleryop.ManagementOp[gallery.GalleryBackend, any]{
ID: uid.String(),
GalleryElementName: backendName,
Galleries: appConfig.BackendGalleries,
Upgrade: true,
}
go func() {
galleryService.BackendGalleryChannel <- op
}()Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/routes/ui_api.go
Lines: 1228-1233
Issue Type: functional-high
Severity: high
Issue Description:
Sending directly to BackendGalleryChannel can block the handler indefinitely if the channel is unbuffered or saturated, so enqueue in a goroutine or use a bounded non-blocking/select-based send.
Current Code:
galleryService.BackendGalleryChannel <- galleryop.ManagementOp[gallery.GalleryBackend, any]{
ID: uid.String(),
GalleryElementName: backendName,
Galleries: appConfig.BackendGalleries,
Upgrade: true,
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| for name, info := range toUpgrade { | ||
| versionStr := "" | ||
| if info.AvailableVersion != "" { | ||
| versionStr = " to v" + info.AvailableVersion | ||
| } | ||
| fmt.Printf("Upgrading %s%s...\n", name, versionStr) | ||
|
|
||
| progressBar := progressbar.NewOptions( | ||
| 1000, | ||
| progressbar.OptionSetDescription(fmt.Sprintf("downloading %s", name)), | ||
| progressbar.OptionShowBytes(false), | ||
| progressbar.OptionClearOnFinish(), | ||
| ) | ||
| progressCallback := func(fileName string, current string, total string, percentage float64) { | ||
| v := int(percentage * 10) | ||
| if err := progressBar.Set(v); err != nil { | ||
| xlog.Error("error updating progress bar", "error", err) | ||
| } | ||
| } | ||
|
|
||
| if err := gallery.UpgradeBackend(context.Background(), systemState, modelLoader, galleries, name, progressCallback); err != nil { | ||
| fmt.Printf("Failed to upgrade %s: %v\n", name, err) | ||
| } else { | ||
| fmt.Printf("Backend %s upgraded successfully\n", name) | ||
| } | ||
| } |
There was a problem hiding this comment.
The upgrade command returns success even when one or more backend upgrades fail, so accumulate failures and return a non-nil error at the end.
Suggested fix
var upgradeErrs []error
for name, info := range toUpgrade {
versionStr := ""
if info.AvailableVersion != "" {
versionStr = " to v" + info.AvailableVersion
}
fmt.Printf("Upgrading %s%s...\n", name, versionStr)
progressBar := progressbar.NewOptions(
1000,
progressbar.OptionSetDescription(fmt.Sprintf("downloading %s", name)),
progressbar.OptionShowBytes(false),
progressbar.OptionClearOnFinish(),
)
progressCallback := func(fileName string, current string, total string, percentage float64) {
v := int(percentage * 10)
if err := progressBar.Set(v); err != nil {
xlog.Error("error updating progress bar", "error", err)
}
}
if err := gallery.UpgradeBackend(context.Background(), systemState, modelLoader, galleries, name, progressCallback); err != nil {
fmt.Printf("Failed to upgrade %s: %v\n", name, err)
upgradeErrs = append(upgradeErrs, fmt.Errorf("%s: %w", name, err))
} else {
fmt.Printf("Backend %s upgraded successfully\n", name)
}
}
if len(upgradeErrs) > 0 {
return errors.Join(upgradeErrs...)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/cli/backends.go
Lines: 180-205
Issue Type: functional-medium
Severity: medium
Issue Description:
The upgrade command returns success even when one or more backend upgrades fail, so accumulate failures and return a non-nil error at the end.
Current Code:
for name, info := range toUpgrade {
versionStr := ""
if info.AvailableVersion != "" {
versionStr = " to v" + info.AvailableVersion
}
fmt.Printf("Upgrading %s%s...\n", name, versionStr)
progressBar := progressbar.NewOptions(
1000,
progressbar.OptionSetDescription(fmt.Sprintf("downloading %s", name)),
progressbar.OptionShowBytes(false),
progressbar.OptionClearOnFinish(),
)
progressCallback := func(fileName string, current string, total string, percentage float64) {
v := int(percentage * 10)
if err := progressBar.Set(v); err != nil {
xlog.Error("error updating progress bar", "error", err)
}
}
if err := gallery.UpgradeBackend(context.Background(), systemState, modelLoader, galleries, name, progressCallback); err != nil {
fmt.Printf("Failed to upgrade %s: %v\n", name, err)
} else {
fmt.Printf("Backend %s upgraded successfully\n", name)
}
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if (!preferDevLoaded && data?.preferDevelopmentBackends) { | ||
| setShowDevelopment(true) | ||
| setPreferDevLoaded(true) | ||
| } |
There was a problem hiding this comment.
The server-provided development preference is only marked as loaded when it is true, so a false value causes repeated first-load logic on every refresh; set the loaded flag regardless of the preference value.
Suggested fix
if (!preferDevLoaded) {
setShowDevelopment(!!data?.preferDevelopmentBackends)
setPreferDevLoaded(true)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert javascript developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/react-ui/src/pages/Backends.jsx
Lines: 46-49
Issue Type: functional-medium
Severity: medium
Issue Description:
The server-provided development preference is only marked as loaded when it is true, so a false value causes repeated first-load logic on every refresh; set the loaded flag regardless of the preference value.
Current Code:
if (!preferDevLoaded && data?.preferDevelopmentBackends) {
setShowDevelopment(true)
setPreferDevLoaded(true)
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow javascript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| deleteInstalledBackend: (name) => `/api/backends/system/delete/${name}`, | ||
| backendsUpgrades: '/api/backends/upgrades', | ||
| backendsUpgradesCheck: '/api/backends/upgrades/check', | ||
| upgradeBackend: (name) => `/api/backends/upgrade/${name}`, |
There was a problem hiding this comment.
The backend name is interpolated into the URL path without encoding, so names containing slashes or reserved characters can target the wrong endpoint; wrap name with encodeURIComponent().
| upgradeBackend: (name) => `/api/backends/upgrade/${name}`, | |
| upgradeBackend: (name) => `/api/backends/upgrade/${encodeURIComponent(name)}`, |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert javascript developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/react-ui/src/utils/config.js
Lines: 28-28
Issue Type: security-medium
Severity: medium
Issue Description:
The backend name is interpolated into the URL path without encoding, so names containing slashes or reserved characters can target the wrong endpoint; wrap `name` with `encodeURIComponent()`.
Current Code:
upgradeBackend: (name) => `/api/backends/upgrade/${name}`,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow javascript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
Consider addressing security findings before merging Scan completed in 26.2s View vulnerability details (2 items)1. MATH_RANDOM_USED (CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. 2. REACT_DANGEROUSLYSET (CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. Security scan powered by Codity.ai |
Dependency vulnerability scanning
Top 10 Vulnerabilities (12 total found)1. @hono/node-server various CVE: GHSA-92pp-h63x-v22m
2. brace-expansion various CVE: GHSA-f886-m6hf-6m8v
3. dompurify various CVE: GHSA-39q2-94rc-95cp, GHSA-h7mw-gpvr-xq4m, GHSA-crv5-9vww-q3g8, GHSA-v9jr-rg53-9pgp
4. express-rate-limit various CVE: N/A
5. fast-uri various CVE: GHSA-q3j6-qgpj-74h6, GHSA-v39h-62p7-jpjc
6. flatted various CVE: GHSA-25h7-pfq9-p65f, GHSA-rf6f-7fwh-wjgh
7. hono various CVE: GHSA-26pp-8wgv-hjvm, GHSA-r5rp-j6wh-rvv4, GHSA-xf4j-xp2r-rqqx, GHSA-wmmm-f939-6g9c, GHSA-458j-xx4x-4375, GHSA-xpcf-pg52-r92g, GHSA-qp7p-654g-cw7p, GHSA-hm8q-7f3q-5f36, GHSA-p77w-8qqv-26rm, GHSA-9vqf-7f2p-gf9v, GHSA-69xw-7hcm-h432
8. ip-address various CVE: GHSA-v2v4-37r5-5v8g
9. path-to-regexp various CVE: GHSA-j3q9-mxjg-w52f, GHSA-27v5-c462-wpq7
10. picomatch various CVE: GHSA-3v7f-55p6-f55p, GHSA-c2c7-rcm5-vvqj
2 more vulnerabilities not shown. Update dependencies to fix these issues. Powered by Codity.ai · Docs |
Dependency vulnerability scanning
View vulnerability details (4 items)1. pip 24.0 CVE: GHSA-4xh5-x5gv-qwph
2. pip 24.0 CVE: GHSA-6vgw-5pg2-w6jp
3. pip 24.0 CVE: GHSA-58qw-9mgm-455v
4. pip 24.0 CVE: GHSA-jp4c-xjxw-mgf9
Powered by Codity.ai · Docs |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 6 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (5 packages):
Unknown Licenses - 153 packages
...and 133 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/LocalAI · PR #6Scanned: 2026-05-18 19:13 UTC | Score: 24/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
core/application/upgrade_checker.go |
0 | 1 | 1 | 1 | 3 |
core/cli/backends.go |
0 | 0 | 2 | 4 | 6 |
core/cli/run.go |
0 | 0 | 0 | 1 | 1 |
core/config/runtime_settings.go |
0 | 0 | 0 | 1 | 1 |
core/gallery/backends_version_test.go |
0 | 0 | 0 | 3 | 3 |
core/gallery/upgrade.go |
0 | 0 | 0 | 4 | 4 |
core/gallery/upgrade_test.go |
0 | 0 | 0 | 6 | 6 |
core/http/endpoints/localai/backend.go |
0 | 0 | 0 | 7 | 7 |
core/http/react-ui/src/pages/Backends.jsx |
0 | 0 | 0 | 38 | 38 |
core/http/react-ui/src/pages/Manage.jsx |
0 | 0 | 0 | 20 | 20 |
core/http/react-ui/src/pages/Settings.jsx |
0 | 0 | 0 | 1 | 1 |
core/http/routes/ui_api.go |
0 | 0 | 0 | 5 | 5 |
core/http/routes/ui_api_backends_test.go |
0 | 0 | 0 | 8 | 8 |
core/services/advisorylock/keys.go |
0 | 0 | 0 | 2 | 2 |
core/services/nodes/managers_distributed.go |
0 | 0 | 0 | 1 | 1 |
pkg/oci/image.go |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
Policy Check Failed✗ 3/3 policy checks failed: • Need 2 more approval(s) (0/2) — comment LGTM or approve via review To merge this PR:
|
PR SummaryWhat Changed
Key Changes by AreaBackend Management: Added API: New endpoints Configuration: New UI: Version display, upgrade banners, filter toggles for meta/development backends, and per-backend upgrade buttons OCI: New Files Changed
Review Focus Areas
ArchitectureDesign Decisions: Used PostgreSQL advisory locks for distributed coordination rather than external coordination services. This keeps infrastructure simple but requires PostgreSQL. Atomic directory swaps via symlinks provide safe rollback but assume atomic rename operations on the target filesystem. Scalability & Extensibility: Upgrade checks run on a single leader in distributed mode, but on-demand checks and cache warming run everywhere for fast API response. This trades some consistency for availability. The Risks:
Merge StatusNOT MERGEABLE — PR Score 14/100, below threshold (50)
|
![codity-dm[bot]](https://avatars.githubusercontent.com/u/234452856?s=60&v=4){kind=small}
| func (uc *UpgradeChecker) Shutdown() { | ||
| close(uc.stop) | ||
| <-uc.done |
There was a problem hiding this comment.
Shutdown closes uc.stop unconditionally, so a second call panics; guard the close with sync.Once or a non-blocking state check.
Suggested fix
type UpgradeChecker struct {
appConfig *config.ApplicationConfig
modelLoader *model.ModelLoader
galleries []config.Gallery
systemState *system.SystemState
db *gorm.DB
checkInterval time.Duration
stop chan struct{}
done chan struct{}
triggerCh chan struct{}
shutdownOnce sync.Once
mu sync.RWMutex
lastUpgrades map[string]gallery.UpgradeInfo
lastCheckTime time.Time
}
func (uc *UpgradeChecker) Shutdown() {
uc.shutdownOnce.Do(func() {
close(uc.stop)
})
<-uc.done
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 119-121
Issue Type: functional-critical
Severity: critical
Issue Description:
Shutdown closes uc.stop unconditionally, so a second call panics; guard the close with sync.Once or a non-blocking state check.
Current Code:
func (uc *UpgradeChecker) Shutdown() {
close(uc.stop)
<-uc.done
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| go advisorylock.RunLeaderLoop(ctx, uc.db, advisorylock.KeyBackendUpgradeCheck, uc.checkInterval, func() { | ||
| uc.runCheck(ctx) | ||
| }) | ||
|
|
||
| // Still listen for on-demand triggers (from API / settings change) | ||
| // and stop signal — these run on every instance. | ||
| for { | ||
| select { | ||
| case <-ctx.Done(): | ||
| return | ||
| case <-uc.stop: | ||
| return | ||
| case <-uc.triggerCh: | ||
| uc.runCheck(ctx) |
There was a problem hiding this comment.
runCheck can execute concurrently from the local trigger path and the leader loop, causing overlapping upgrades of the same backend; serialize the whole check-and-upgrade routine with a dedicated mutex.
Suggested fix
type UpgradeChecker struct {
appConfig *config.ApplicationConfig
modelLoader *model.ModelLoader
galleries []config.Gallery
systemState *system.SystemState
db *gorm.DB
checkInterval time.Duration
stop chan struct{}
done chan struct{}
triggerCh chan struct{}
runMu sync.Mutex
mu sync.RWMutex
lastUpgrades map[string]gallery.UpgradeInfo
lastCheckTime time.Time
}
func (uc *UpgradeChecker) runCheck(ctx context.Context) {
uc.runMu.Lock()
defer uc.runMu.Unlock()
upgrades, err := gallery.CheckBackendUpgrades(ctx, uc.galleries, uc.systemState)
// existing logic...
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 82-95
Issue Type: functional-critical
Severity: critical
Issue Description:
runCheck can execute concurrently from the local trigger path and the leader loop, causing overlapping upgrades of the same backend; serialize the whole check-and-upgrade routine with a dedicated mutex.
Current Code:
go advisorylock.RunLeaderLoop(ctx, uc.db, advisorylock.KeyBackendUpgradeCheck, uc.checkInterval, func() {
uc.runCheck(ctx)
})
...
case <-uc.triggerCh:
uc.runCheck(ctx)
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
|
|
||
| // distributedDB returns the PostgreSQL database for distributed coordination, | ||
| // or nil in standalone mode. | ||
| func (a *Application) distributedDB() *gorm.DB { |
There was a problem hiding this comment.
In distributed mode the new upgrade checker is passed authDB instead of the distributed coordination DB, so starting two frontend replicas against separate auth databases will both acquire their own advisory locks and auto-upgrade the same backend concurrently.
Return the actual distributed coordination/postgres DB from a.distributed here (or rename/remove this helper if advisory locks are not supported in the current mode) so all replicas contend on the same database lock.
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/application.go
Lines: 91-91
Issue Type: functional-high
Severity: high
Issue Description:
In distributed mode the new upgrade checker is passed `authDB` instead of the distributed coordination DB, so starting two frontend replicas against separate auth databases will both acquire their own advisory locks and auto-upgrade the same backend concurrently.
Current Code:
func (a *Application) distributedDB() *gorm.DB {
if a.distributed != nil {
return a.authDB
}
return nil
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| // Check for upgrades | ||
| upgrades, _ := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState) |
There was a problem hiding this comment.
The error from CheckBackendUpgrades is discarded, so listing can silently hide upgrade-check failures; return or surface the error instead of ignoring it.
Suggested fix
// Check for upgrades
upgrades, err := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState)
if err != nil {
return fmt.Errorf("failed to check backend upgrades: %w", err)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/cli/backends.go
Lines: 75-76
Issue Type: robustness-high
Severity: high
Issue Description:
The error from CheckBackendUpgrades is discarded, so listing can silently hide upgrade-check failures; return or surface the error instead of ignoring it.
Current Code:
// Check for upgrades
upgrades, _ := gallery.CheckBackendUpgrades(context.Background(), galleries, systemState)
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| // Record the OCI digest for upgrade detection (non-fatal on failure) | ||
| if uri.LooksLikeOCI() { | ||
| digest, digestErr := oci.GetImageDigest(string(uri), "", nil, nil) | ||
| if digestErr != nil { | ||
| xlog.Warn("Failed to get OCI image digest for backend", "uri", string(uri), "error", digestErr) | ||
| } else { | ||
| metadata.Digest = digest |
There was a problem hiding this comment.
The digest is fetched by tag after installation, so a mutable OCI tag can change between download and metadata recording; capture and persist the exact resolved digest from the install step instead of re-resolving the tag here.
Suggested fix
// Record the resolved OCI digest from the install step so mutable tags cannot drift.
if uri.LooksLikeOCI() {
resolvedDigest := downloadStatus.ResolvedDigest
if resolvedDigest == "" {
xlog.Warn("Resolved OCI digest unavailable after backend install", "uri", string(uri))
} else {
metadata.Digest = resolvedDigest
}
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/gallery/backends.go
Lines: 288-294
Issue Type: security-high
Severity: high
Issue Description:
The digest is fetched by tag after installation, so a mutable OCI tag can change between download and metadata recording; capture and persist the exact resolved digest from the install step instead of re-resolving the tag here.
Current Code:
// Record the OCI digest for upgrade detection (non-fatal on failure)
if uri.LooksLikeOCI() {
digest, digestErr := oci.GetImageDigest(string(uri), "", nil, nil)
if digestErr != nil {
xlog.Warn("Failed to get OCI image digest for backend", "uri", string(uri), "error", digestErr)
} else {
metadata.Digest = digest
}
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| // Reuse install endpoint which will re-download the backend (force mode) | ||
| reply, err := d.adapter.InstallBackend(node.ID, name, "", string(galleriesJSON)) | ||
| if err != nil { |
There was a problem hiding this comment.
The upgrade path still passes an empty version/force argument to InstallBackend, so it does not guarantee a re-download; pass an explicit upgrade signal or call a dedicated upgrade endpoint.
Suggested fix
// Use a dedicated upgrade call or pass an explicit force/version argument.
reply, err := d.adapter.InstallBackend(node.ID, name, "force", string(galleriesJSON))Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/services/nodes/managers_distributed.go
Lines: 193-195
Issue Type: functional-high
Severity: high
Issue Description:
The upgrade path still passes an empty version/force argument to InstallBackend, so it does not guarantee a re-download; pass an explicit upgrade signal or call a dedicated upgrade endpoint.
Current Code:
// Reuse install endpoint which will re-download the backend (force mode)
reply, err := d.adapter.InstallBackend(node.ID, name, "", string(galleriesJSON))
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if settings.AutoUpgradeBackends != nil { | ||
| appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends |
There was a problem hiding this comment.
This setting bypasses the existing env var precedence checks, so a writable runtime_settings.json can override startup policy; gate it behind the corresponding env flag like the surrounding settings.
Suggested fix
if settings.AutoUpgradeBackends != nil && !envAutoUpgradeBackends {
appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert go developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/config_file_watcher.go
Lines: 338-339
Issue Type: security-medium
Severity: medium
Issue Description:
This setting bypasses the existing env var precedence checks, so a writable runtime_settings.json can override startup policy; gate it behind the corresponding env flag like the surrounding settings.
Current Code:
if settings.AutoUpgradeBackends != nil {
appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow go best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if (!preferDevLoaded && data?.preferDevelopmentBackends) { | ||
| setShowDevelopment(true) | ||
| setPreferDevLoaded(true) | ||
| } |
There was a problem hiding this comment.
The server preference flag is only marked as loaded when the preference is true, so a false value causes this branch to re-run on every fetch; set the loaded flag regardless after the first response.
Suggested fix
if (!preferDevLoaded) {
setShowDevelopment(!!data?.preferDevelopmentBackends)
setPreferDevLoaded(true)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert javascript developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/react-ui/src/pages/Backends.jsx
Lines: 46-49
Issue Type: functional-medium
Severity: medium
Issue Description:
The server preference flag is only marked as loaded when the preference is true, so a false value causes this branch to re-run on every fetch; set the loaded flag regardless after the first response.
Current Code:
if (!preferDevLoaded && data?.preferDevelopmentBackends) {
setShowDevelopment(true)
setPreferDevLoaded(true)
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow javascript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| backendsApi.checkUpgrades() | ||
| .then(data => setUpgrades(data || {})) | ||
| .catch(() => {}) |
There was a problem hiding this comment.
The upgrades refresh silently swallows errors, so stale update data can persist indefinitely; clear the upgrades state on failure.
Suggested fix
backendsApi.checkUpgrades()
.then(data => setUpgrades(data || {}))
.catch(() => setUpgrades({}))Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert javascript developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/react-ui/src/pages/Backends.jsx
Lines: 68-70
Issue Type: robustness-medium
Severity: medium
Issue Description:
The upgrades refresh silently swallows errors, so stale update data can persist indefinitely; clear the upgrades state on failure.
Current Code:
backendsApi.checkUpgrades()
.then(data => setUpgrades(data || {}))
.catch(() => {})
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow javascript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| deleteInstalledBackend: (name) => `/api/backends/system/delete/${name}`, | ||
| backendsUpgrades: '/api/backends/upgrades', | ||
| backendsUpgradesCheck: '/api/backends/upgrades/check', | ||
| upgradeBackend: (name) => `/api/backends/upgrade/${name}`, |
There was a problem hiding this comment.
The backend name is interpolated into the path without URL encoding, so names containing slashes or reserved characters can hit the wrong endpoint; wrap the parameter with encodeURIComponent.
| upgradeBackend: (name) => `/api/backends/upgrade/${name}`, | |
| upgradeBackend: (name) => `/api/backends/upgrade/${encodeURIComponent(name)}`, |
Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert javascript developer with deep knowledge of security, performance, and best practices.
### Context
File: core/http/react-ui/src/utils/config.js
Lines: 28-28
Issue Type: security-medium
Severity: medium
Issue Description:
The backend name is interpolated into the path without URL encoding, so names containing slashes or reserved characters can hit the wrong endpoint; wrap the parameter with encodeURIComponent.
Current Code:
upgradeBackend: (name) => `/api/backends/upgrade/${name}`,
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow javascript best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
Consider addressing security findings before merging Scan completed in 55.8s View vulnerability details (2 items)1. MATH_RANDOM_USED (CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. 2. REACT_DANGEROUSLYSET (CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. Security scan powered by Codity.ai |
Dependency vulnerability scanning
Top 10 Vulnerabilities (12 total found)1. @hono/node-server various CVE: GHSA-92pp-h63x-v22m
2. brace-expansion various CVE: GHSA-f886-m6hf-6m8v
3. dompurify various CVE: GHSA-39q2-94rc-95cp, GHSA-h7mw-gpvr-xq4m, GHSA-crv5-9vww-q3g8, GHSA-v9jr-rg53-9pgp
4. express-rate-limit various CVE: N/A
5. fast-uri various CVE: GHSA-q3j6-qgpj-74h6, GHSA-v39h-62p7-jpjc
6. flatted various CVE: GHSA-25h7-pfq9-p65f, GHSA-rf6f-7fwh-wjgh
7. hono various CVE: GHSA-26pp-8wgv-hjvm, GHSA-r5rp-j6wh-rvv4, GHSA-xf4j-xp2r-rqqx, GHSA-wmmm-f939-6g9c, GHSA-458j-xx4x-4375, GHSA-xpcf-pg52-r92g, GHSA-qp7p-654g-cw7p, GHSA-hm8q-7f3q-5f36, GHSA-p77w-8qqv-26rm, GHSA-9vqf-7f2p-gf9v, GHSA-69xw-7hcm-h432
8. ip-address various CVE: GHSA-v2v4-37r5-5v8g
9. path-to-regexp various CVE: GHSA-j3q9-mxjg-w52f, GHSA-27v5-c462-wpq7
10. picomatch various CVE: GHSA-3v7f-55p6-f55p, GHSA-c2c7-rcm5-vvqj
2 more vulnerabilities not shown. Update dependencies to fix these issues. Powered by Codity.ai · Docs |
Dependency vulnerability scanning
View vulnerability details (4 items)1. pip 24.0 CVE: GHSA-4xh5-x5gv-qwph
2. pip 24.0 CVE: GHSA-6vgw-5pg2-w6jp
3. pip 24.0 CVE: GHSA-58qw-9mgm-455v
4. pip 24.0 CVE: GHSA-jp4c-xjxw-mgf9
Powered by Codity.ai · Docs |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 6 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (5 packages):
Unknown Licenses - 93 packages
...and 73 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/LocalAI · PR #6Scanned: 2026-05-18 20:13 UTC | Score: 23/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
core/application/upgrade_checker.go |
0 | 1 | 1 | 3 | 5 |
core/cli/backends.go |
0 | 1 | 2 | 3 | 6 |
core/cli/run.go |
0 | 0 | 0 | 1 | 1 |
core/config/runtime_settings.go |
0 | 0 | 0 | 1 | 1 |
core/gallery/backends_version_test.go |
0 | 0 | 0 | 3 | 3 |
core/gallery/upgrade.go |
0 | 0 | 0 | 4 | 4 |
core/gallery/upgrade_test.go |
0 | 0 | 0 | 6 | 6 |
core/http/endpoints/localai/backend.go |
0 | 0 | 0 | 7 | 7 |
core/http/react-ui/src/pages/Backends.jsx |
0 | 0 | 0 | 38 | 38 |
core/http/react-ui/src/pages/Manage.jsx |
0 | 0 | 0 | 20 | 20 |
core/http/react-ui/src/pages/Settings.jsx |
0 | 0 | 0 | 1 | 1 |
core/http/routes/ui_api.go |
0 | 0 | 0 | 5 | 5 |
core/http/routes/ui_api_backends_test.go |
0 | 0 | 0 | 8 | 8 |
core/services/advisorylock/keys.go |
0 | 0 | 0 | 2 | 2 |
core/services/nodes/managers_distributed.go |
0 | 0 | 0 | 1 | 1 |
pkg/oci/image.go |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
PR SummaryWhat Changed
Key Changes by AreaBackend Management: New CLI: New API: Three new endpoints for upgrade operations ( UI: Upgrade banners with "Upgrade All" button, version-aware filtering, and individual backend upgrade/reinstall controls in React components. Configuration: New Files Changed
Review Focus Areas
ArchitectureDesign Decisions:
Scalability & Extensibility:
Risks:
Merge StatusNOT MERGEABLE — PR Score 36/100, below threshold (50)
|
Workflow DiagramsAutomatically generated sequence diagrams showing the workflows in this PR 1. Backend Upgrade Service WorkflowComplex complexity • Components: UpgradeChecker, Application, BackendsUpgrade CLI command sequenceDiagram
title Backend Upgrade Checker Workflow
participant Startup as ApplicationStartup
participant UC as UpgradeChecker
participant AL as AdvisoryLock
participant Gallery as GalleryService
participant DB as PostgreSQL
participant API as HTTPAPI
participant Config as ConfigWatcher
Startup->>UC: NewUpgradeChecker(appConfig, modelLoader, db)
Note over Startup,UC: db is nil in standalone mode, or PostgreSQL connection in distributed mode
Startup->>UC: Run(ctx) as goroutine
UC->>UC: Wait 30s initial delay
UC->>Gallery: CheckBackendUpgrades(galleries, systemState)
Gallery-->>UC: map[string]UpgradeInfo
UC->>UC: Cache results with RWMutex
alt Distributed Mode (db != nil)
UC->>AL: RunLeaderLoop(ctx, db, key, interval, callback)
loop Every 6 hours
AL->>DB: Acquire advisory lock
alt Lock acquired (leader elected)
AL->>UC: Execute callback: runCheck(ctx)
UC->>Gallery: CheckBackendUpgrades
Gallery-->>UC: upgrades
UC->>UC: Update cache
opt AutoUpgradeBackends enabled
UC->>Gallery: UpgradeBackend(ctx, systemState, modelLoader, galleries, name, nil)
Gallery-->>UC: success/error
UC->>Gallery: CheckBackendUpgrades (recheck after upgrade)
end
else Lock not acquired
Note over AL: Skip check, another instance is leader
end
end
UC->>UC: Listen for triggerCh (on-demand checks)
else Standalone Mode (db == nil)
UC->>UC: Start ticker (6h interval)
loop Ticker fires or triggerCh received
UC->>UC: runCheck(ctx)
UC->>Gallery: CheckBackendUpgrades
Gallery-->>UC: upgrades
UC->>UC: Update cache
opt AutoUpgradeBackends enabled
UC->>Gallery: UpgradeBackend
end
end
end
API->>UC: GetAvailableUpgrades()
UC->>UC: RLock, copy cache
UC-->>API: map[string]UpgradeInfo
API->>UC: TriggerCheck()
UC->>UC: Send to triggerCh (non-blocking)
Config->>UC: AutoUpgradeBackends setting changed
Note over Config: Applied via runtime_settings.json or env vars
Config->>UC: TriggerCheck() (immediate re-check)
Startup->>UC: Shutdown()
UC->>UC: Close stop channel
UC-->>Startup: Wait for done channel
Note: Diagrams show detected patterns only. Complex workflows may require manual review. |
![codity-chait[bot]](https://avatars.githubusercontent.com/u/228176587?s=60&v=4){kind=small}
| if err := json.Unmarshal([]byte(bu.BackendGalleries), &galleries); err != nil { | ||
| xlog.Error("unable to load galleries", "error", err) | ||
| } |
There was a problem hiding this comment.
JSON unmarshal error is logged and silently swallowed in BackendsUpgrade.Run. Execution continues with an empty galleries slice, causing CheckBackendUpgrades to return zero results. The command then prints "All backends are up to date." even when the gallery configuration is malformed: the user gets a misleading success message instead of an actionable error. The identical pattern in the pre-existing BackendsList.Run and BackendsInstall.Run should also be fixed, but this is a new, user-facing subcommand that makes the silent failure especially harmful.
Suggested fix
if err := json.Unmarshal([]byte(bu.BackendGalleries), &galleries); err != nil {
return fmt.Errorf("unable to load galleries: %w", err)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/cli/backends.go
Lines: 139-141
Issue Type: functional-high
Severity: high
Issue Description:
JSON unmarshal error is logged and silently swallowed in `BackendsUpgrade.Run`. Execution continues with an empty `galleries` slice, causing `CheckBackendUpgrades` to return zero results. The command then prints "All backends are up to date." even when the gallery configuration is malformed: the user gets a misleading success message instead of an actionable error. The identical pattern in the pre-existing `BackendsList.Run` and `BackendsInstall.Run` should also be fixed, but this is a new, user-facing subcommand that makes the silent failure especially harmful.
Current Code:
if err := json.Unmarshal([]byte(bu.BackendGalleries), &galleries); err != nil {
xlog.Error("unable to load galleries", "error", err)
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if installed.Metadata != nil && installed.Metadata.MetaBackendFor != "" { | ||
| xlog.Info("Meta backend detected, upgrading concrete backend", "meta", backendName, "concrete", installed.Metadata.MetaBackendFor) | ||
| return UpgradeBackend(ctx, systemState, modelLoader, galleries, installed.Metadata.MetaBackendFor, downloadStatus) | ||
| } |
There was a problem hiding this comment.
Unbounded recursion in UpgradeBackend when resolving meta-backends. If a metadata.json file sets MetaBackendFor to the backend's own name (or a chain A→B→A forms a cycle), UpgradeBackend recurses without any depth limit or visited-set guard until the goroutine stack is exhausted, crashing the process. A malformed or manually edited metadata file in the backends directory is sufficient to trigger this. The self-reference case is the easiest to guard against.
Suggested fix
// If this is a meta backend, recursively upgrade the concrete backend it points to
if installed.Metadata != nil && installed.Metadata.MetaBackendFor != "" {
if installed.Metadata.MetaBackendFor == backendName {
return fmt.Errorf("meta backend %q has a self-referential MetaBackendFor; refusing to recurse", backendName)
}
xlog.Info("Meta backend detected, upgrading concrete backend", "meta", backendName, "concrete", installed.Metadata.MetaBackendFor)
return UpgradeBackend(ctx, systemState, modelLoader, galleries, installed.Metadata.MetaBackendFor, downloadStatus)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/gallery/upgrade.go
Lines: 129-132
Issue Type: robustness-high
Severity: high
Issue Description:
Unbounded recursion in `UpgradeBackend` when resolving meta-backends. If a `metadata.json` file sets `MetaBackendFor` to the backend's own name (or a chain A→B→A forms a cycle), `UpgradeBackend` recurses without any depth limit or visited-set guard until the goroutine stack is exhausted, crashing the process. A malformed or manually edited metadata file in the backends directory is sufficient to trigger this. The self-reference case is the easiest to guard against.
Current Code:
// If this is a meta backend, recursively upgrade the concrete backend it points to
if installed.Metadata != nil && installed.Metadata.MetaBackendFor != "" {
xlog.Info("Meta backend detected, upgrading concrete backend", "meta", backendName, "concrete", installed.Metadata.MetaBackendFor)
return UpgradeBackend(ctx, systemState, modelLoader, galleries, installed.Metadata.MetaBackendFor, downloadStatus)
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if _, err := os.Stat(newRunFile); os.IsNotExist(err) { | ||
| os.RemoveAll(tmpPath) | ||
| return fmt.Errorf("upgrade validation failed: run.sh not found in new backend") |
There was a problem hiding this comment.
The validation check only handles os.IsNotExist. Any other error from os.Stat (e.g., permission denied, I/O error, path-too-long) is silently ignored and treated as "run.sh is present". The code then continues with the atomic swap, potentially installing an inaccessible or corrupt backend while the old installation has already been moved to .backup. The fix is to treat any non-nil stat error as a validation failure.
Suggested fix
if _, statErr := os.Stat(newRunFile); statErr != nil {
os.RemoveAll(tmpPath)
return fmt.Errorf("upgrade validation failed: run.sh not accessible in new backend: %w", statErr)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/gallery/upgrade.go
Lines: 173-175
Issue Type: robustness-medium
Severity: medium
Issue Description:
The validation check only handles `os.IsNotExist`. Any other error from `os.Stat` (e.g., permission denied, I/O error, path-too-long) is silently ignored and treated as "run.sh is present". The code then continues with the atomic swap, potentially installing an inaccessible or corrupt backend while the old installation has already been moved to `.backup`. The fix is to treat any non-nil stat error as a validation failure.
Current Code:
if _, err := os.Stat(newRunFile); os.IsNotExist(err) {
os.RemoveAll(tmpPath)
return fmt.Errorf("upgrade validation failed: run.sh not found in new backend")
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| func (uc *UpgradeChecker) Shutdown() { | ||
| close(uc.stop) | ||
| <-uc.done | ||
| } |
There was a problem hiding this comment.
Shutdown() calls close(uc.stop) without any guard against a second call. Closing an already-closed channel panics in Go. If Shutdown() is ever called twice: e.g., via a deferred call and an explicit call in an error path, or in tests: the process crashes. sync.Once is the idiomatic fix; requires adding a stopOnce sync.Once field to UpgradeChecker.
Suggested fix
// Shutdown stops the upgrade checker loop. Safe to call more than once.
func (uc *UpgradeChecker) Shutdown() {
uc.stopOnce.Do(func() { close(uc.stop) })
<-uc.done
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 119-122
Issue Type: robustness-medium
Severity: medium
Issue Description:
`Shutdown()` calls `close(uc.stop)` without any guard against a second call. Closing an already-closed channel panics in Go. If `Shutdown()` is ever called twice: e.g., via a deferred call and an explicit call in an error path, or in tests: the process crashes. `sync.Once` is the idiomatic fix; requires adding a `stopOnce sync.Once` field to `UpgradeChecker`.
Current Code:
// Shutdown stops the upgrade checker loop.
func (uc *UpgradeChecker) Shutdown() {
close(uc.stop)
<-uc.done
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| go advisorylock.RunLeaderLoop(ctx, uc.db, advisorylock.KeyBackendUpgradeCheck, uc.checkInterval, func() { | ||
| uc.runCheck(ctx) | ||
| }) |
There was a problem hiding this comment.
In distributed mode, Run() spawns a RunLeaderLoop goroutine whose only cancellation signal is ctx. When Shutdown() is called (closes uc.stop), the outer for loop returns and close(uc.done) is executed: but the spawned goroutine keeps running until the parent context is cancelled. Any caller (including tests) that invokes Shutdown() before cancelling ctx incorrectly believes all activity has stopped, while the goroutine continues to hold the advisory lock and run runCheck. Wrapping ctx in a derived cancellable context and cancelling it via defer in Run() would propagate the stop signal to the goroutine.
Suggested fix
runCtx, cancelRun := context.WithCancel(ctx)
defer cancelRun()
go advisorylock.RunLeaderLoop(runCtx, uc.db, advisorylock.KeyBackendUpgradeCheck, uc.checkInterval, func() {
uc.runCheck(runCtx)
})Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 82-84
Issue Type: robustness-medium
Severity: medium
Issue Description:
In distributed mode, `Run()` spawns a `RunLeaderLoop` goroutine whose only cancellation signal is `ctx`. When `Shutdown()` is called (closes `uc.stop`), the outer `for` loop returns and `close(uc.done)` is executed: but the spawned goroutine keeps running until the parent context is cancelled. Any caller (including tests) that invokes `Shutdown()` before cancelling `ctx` incorrectly believes all activity has stopped, while the goroutine continues to hold the advisory lock and run `runCheck`. Wrapping `ctx` in a derived cancellable context and cancelling it via `defer` in `Run()` would propagate the stop signal to the goroutine.
Current Code:
go advisorylock.RunLeaderLoop(ctx, uc.db, advisorylock.KeyBackendUpgradeCheck, uc.checkInterval, func() {
uc.runCheck(ctx)
})
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
Security Scan Summary
Consider addressing security findings before merging Scan completed in 68.8s View vulnerability details (2 items)1. MATH_RANDOM_USED (CWE-338: Use of Cryptographically Weak Pseudo-Random Number Generator (PRNG)) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. 2. REACT_DANGEROUSLYSET (CWE-79: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting')) HIGH File:
Fix: Review and fix the security issue following OWASP guidelines. Security scan powered by Codity.ai |
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 6 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (5 packages):
Unknown Licenses - 93 packages
...and 73 more Powered by Codity.ai · Docs |
Code Quality Report — test-org-codity/LocalAI · PR #6Scanned: 2026-06-03 23:11 UTC | Score: 23/100 | Provider: github Executive Summary
Top Findings[CQ-LLM-001]
|
| File | Critical | High | Medium | Low | Total |
|---|---|---|---|---|---|
core/application/upgrade_checker.go |
0 | 1 | 2 | 2 | 5 |
core/cli/backends.go |
0 | 1 | 1 | 3 | 5 |
core/cli/run.go |
0 | 0 | 0 | 1 | 1 |
core/config/runtime_settings.go |
0 | 0 | 0 | 1 | 1 |
core/gallery/backends_version_test.go |
0 | 0 | 0 | 3 | 3 |
core/gallery/upgrade.go |
0 | 0 | 0 | 4 | 4 |
core/gallery/upgrade_test.go |
0 | 0 | 0 | 6 | 6 |
core/http/endpoints/localai/backend.go |
0 | 0 | 0 | 7 | 7 |
core/http/react-ui/src/pages/Backends.jsx |
0 | 0 | 0 | 38 | 38 |
core/http/react-ui/src/pages/Manage.jsx |
0 | 0 | 0 | 20 | 20 |
core/http/react-ui/src/pages/Settings.jsx |
0 | 0 | 0 | 1 | 1 |
core/http/routes/ui_api.go |
0 | 0 | 0 | 5 | 5 |
core/http/routes/ui_api_backends_test.go |
0 | 0 | 0 | 8 | 8 |
core/services/advisorylock/keys.go |
0 | 0 | 0 | 2 | 2 |
core/services/nodes/managers_distributed.go |
0 | 0 | 0 | 1 | 1 |
pkg/oci/image.go |
0 | 0 | 0 | 1 | 1 |
Recommendations
- Resolve High severity issues, especially error handling gaps and performance bottlenecks.
- Run automated tests after applying fixes to verify no regressions.
|
@codity review |
PR SummaryWhat Changed
Key Changes by AreaBackend Management: Added Configuration: New HTTP API: New endpoints at UI: Upgrade banners, version badges, toggle filters for "Show all" and "Development" backends, and settings toggle for auto-upgrades. Distributed Coordination: Files Changed
Review Focus Areas
Architecture
|
| // First check always runs locally (to warm the cache on this instance) | ||
| uc.runCheck(ctx) |
There was a problem hiding this comment.
The initial uc.runCheck(ctx) call at line 77 executes unconditionally on every frontend instance before the distributed advisory-lock guard is installed. When AutoUpgradeBackends = true and multiple frontend replicas start simultaneously, each replica calls gallery.UpgradeBackend concurrently for the same backends, racing on filesystem paths like .upgrade-tmp and .backup directories, causing corrupted binaries and undefined ordering of atomic rename operations.
Suggested fix
// First check always runs locally (to warm the cache on this instance)
if uc.db == nil {
// Only run initial check in standalone mode
// In distributed mode, wait for advisory lock to avoid concurrent upgrade races
uc.runCheck(ctx)
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/upgrade_checker.go
Lines: 76-77
Issue Type: robustness-critical
Severity: critical
Issue Description:
The initial `uc.runCheck(ctx)` call at line 77 executes unconditionally on every frontend instance before the distributed advisory-lock guard is installed. When `AutoUpgradeBackends = true` and multiple frontend replicas start simultaneously, each replica calls `gallery.UpgradeBackend` concurrently for the same backends, racing on filesystem paths like `.upgrade-tmp` and `.backup` directories, causing corrupted binaries and undefined ordering of atomic rename operations.
Current Code:
// First check always runs locally (to warm the cache on this instance)
uc.runCheck(ctx)
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
| if settings.AutoUpgradeBackends != nil { | ||
| appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends | ||
| } |
There was a problem hiding this comment.
The AutoUpgradeBackends setting from runtime_settings.json is applied without checking the envAutoUpgradeBackends guard that all other similarly-gated settings use in the same function. This causes environment variable precedence to be violated: if LOCALAI_AUTO_UPGRADE_BACKENDS=false is explicitly set as an environment variable, a runtime_settings.json file containing {"auto_upgrade_backends": true} will silently override this policy.
Suggested fix
if settings.AutoUpgradeBackends != nil && !envAutoUpgradeBackends {
appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends
}Prompt for AI assistance
Copy the prompt below and paste it into ChatGPT, Claude, or any LLM:
You are an expert bash developer with deep knowledge of security, performance, and best practices.
### Context
File: core/application/config_file_watcher.go
Lines: 338-340
Issue Type: robustness-high
Severity: high
Issue Description:
The `AutoUpgradeBackends` setting from `runtime_settings.json` is applied without checking the `envAutoUpgradeBackends` guard that all other similarly-gated settings use in the same function. This causes environment variable precedence to be violated: if `LOCALAI_AUTO_UPGRADE_BACKENDS=false` is explicitly set as an environment variable, a `runtime_settings.json` file containing `{"auto_upgrade_backends": true}` will silently override this policy.
Current Code:
if settings.AutoUpgradeBackends != nil {
appConfig.AutoUpgradeBackends = *settings.AutoUpgradeBackends
}
---
### Instructions
1. Fix the issue described above
2. Maintain the exact indentation and code style from the original
3. Follow bash best practices and language-specific idioms
4. Ensure the fix addresses the root cause, not just the symptoms
5. Add brief inline comments explaining the fix if needed
### Constraints
- Do not change functionality beyond fixing the identified issue
- Preserve existing variable names and function signatures unless they are part of the problem
- Ensure the fix is production-ready
---
License Compliance Scan
Weak copyleft licenses found - verify compatibility Some packages have unknown licenses - manual review required Medium Risk Licenses - 6 packages(MPL-2.0 OR Apache-2.0) (1 packages):
MPL-2.0 (5 packages):
Unknown Licenses - 93 packages
...and 73 more Powered by Codity.ai · Docs |
3 participants
Description
This PR fixes #
Notes for Reviewers
Signed commits