Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions spicetify.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,10 @@ func main() {

// Unchainable commands
switch commands[0] {
case "doctor":
cmd.Doctor()
return

case "config":
commands = commands[1:]
if len(commands) == 0 {
Expand Down Expand Up @@ -397,6 +401,9 @@ watch Enter watch mode.
restart Restart Spotify client.

` + utils.Bold("NON-CHAINABLE COMMANDS") + `
doctor Run diagnostics on Spotify installation, backup state, and config,
and report anything that looks broken.

spotify-updates Block Spotify updates by patching spotify executable.
Accepts "block" or "unblock" as the parameter.

Expand Down
17 changes: 11 additions & 6 deletions src/cmd/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,20 +224,25 @@ func getExtractFolder() (string, string) {
}

func getThemeFolder(themeName string) string {
folder := findThemeFolder(themeName)
if len(folder) == 0 {
utils.PrintError(`Theme "` + themeName + `" not found`)
os.Exit(1)
}
return folder
}

func findThemeFolder(themeName string) string {
folder := filepath.Join(userThemesFolder, themeName)
_, err := os.Stat(folder)
if err == nil {
if _, err := os.Stat(folder); err == nil {
return folder
}

folder = filepath.Join(utils.GetExecutableDir(), "Themes", themeName)
_, err = os.Stat(folder)
if err == nil {
if _, err := os.Stat(folder); err == nil {
return folder
}

utils.PrintError(`Theme "` + themeName + `" not found`)
os.Exit(1)
return ""
}

Expand Down
132 changes: 132 additions & 0 deletions src/cmd/doctor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package cmd

import (
"fmt"
"os"
"path/filepath"
"runtime"
"strings"

backupstatus "github.com/spicetify/cli/src/status/backup"
spotifystatus "github.com/spicetify/cli/src/status/spotify"
"github.com/spicetify/cli/src/utils"
)

// Doctor runs a series of diagnostic checks on the Spotify installation,
// backup state, and spicetify config, reporting anything that looks broken.
func Doctor() {
utils.PrintBold("Running spicetify doctor")

problems := 0
pass := func(msg string) {
utils.PrintSuccess(msg)
}
fail := func(msg string) {
utils.PrintError(msg)
problems++
}

spotifyPathCfg := utils.ReplaceEnvVarsInString(settingSection.Key("spotify_path").String())
prefsPathCfg := utils.ReplaceEnvVarsInString(settingSection.Key("prefs_path").String())

spotifyAppsPath := filepath.Join(spotifyPathCfg, "Apps")
_, appsErr := os.Stat(spotifyAppsPath)
if appsErr == nil {
pass("Spotify installation found at \"" + spotifyPathCfg + "\"")
} else {
fail("Spotify installation not found at \"" + spotifyPathCfg + "\". Run \"spicetify config spotify_path <path>\" to fix it, or reinstall Spotify")
}

_, prefsErr := os.Stat(prefsPathCfg)
if prefsErr == nil {
pass("Spotify \"prefs\" file found at \"" + prefsPathCfg + "\"")
} else {
fail("Spotify \"prefs\" file not found at \"" + prefsPathCfg + "\". Launch Spotify at least once, then run \"spicetify config prefs_path <path>\" to fix it")
}
Comment on lines +29 to +45

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Empty spotify_path/prefs_path config can produce a misleading "found" result.

If spotify_path or prefs_path is unset (empty string), filepath.Join("", "Apps") yields the relative path "Apps", and os.Stat will resolve it against the current working directory rather than reporting the config as unset. This could make Doctor falsely report the install/prefs as "found" when the config simply hasn't been set, undermining its diagnostic accuracy.

🩺 Guard against empty config values
+	if len(spotifyPathCfg) == 0 {
+		fail("\"spotify_path\" is not configured. Run \"spicetify config spotify_path <path>\"")
+	} else {
+		spotifyAppsPath := filepath.Join(spotifyPathCfg, "Apps")
+		_, appsErr := os.Stat(spotifyAppsPath)
+		...
+	}
🤖 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 `@src/cmd/doctor.go` around lines 29 - 45, The Doctor checks in `main`
currently treat empty `spotify_path` and `prefs_path` as valid paths because
`filepath.Join` and `os.Stat` can resolve them relative to the current
directory. Update the `spotifyPathCfg` and `prefsPathCfg` handling to explicitly
detect unset/empty config values before calling `os.Stat`, and report them as
missing/unconfigured instead of “found.” Keep the fix localized to the existing
Spotify and prefs validation flow in `doctor.go`, using the current
`pass`/`fail` messages and `settingSection.Key(...).String()` values.


if runtime.GOOS == "windows" && (strings.Contains(spotifyPathCfg, "SpotifyAB.SpotifyMusic") || strings.Contains(prefsPathCfg, "SpotifyAB.SpotifyMusic")) {
utils.PrintWarning("Spotify was installed via Microsoft Store, which is only partly supported")
}

if appsErr == nil {
spotStat := spotifystatus.Get(spotifyAppsPath)
switch {
case spotStat.IsInvalid():
fail("Spotify \"Apps\" folder is empty or invalid. Reinstall Spotify")
case spotStat.IsMixed():
fail("Spotify has a mix of modified and stock files, likely from a recent Spotify update. Run \"spicetify backup apply\" to fix it")
case spotStat.IsStock():
pass("Spotify is in stock (unmodified) state")
case spotStat.IsApplied():
pass("Spotify has spicetify customizations applied")
}

if prefsErr == nil {
backupVersion := backupSection.Key("version").MustString("")
backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
switch {
case backStat.IsEmpty():
fail("No backup found. Run \"spicetify backup\"")
case backStat.IsOutdated():
fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
default:
pass("Backup is up to date with installed Spotify version")
}
}
Comment on lines +64 to +75

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Backup check can crash the whole doctor command via log.Fatal.

backupstatus.Get (src/status/backup/backup.go) does os.ReadDir(backupPath) and calls log.Fatal(err) on failure. Since log.Fatal is log.Print + os.Exit(1) with no recovery, if backupFolder doesn't exist yet (very plausible on a first doctor run before the user has ever run spicetify backup), this call aborts the entire program with a raw OS error instead of Doctor's intended graceful "no backup found" diagnostic. This defeats the purpose of a diagnostic tool that's supposed to report problems, not crash on them.

🩺 Guard against a missing backup folder before calling backupstatus.Get
 		if prefsErr == nil {
 			backupVersion := backupSection.Key("version").MustString("")
-			backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
-			switch {
-			case backStat.IsEmpty():
-				fail("No backup found. Run \"spicetify backup\"")
-			case backStat.IsOutdated():
-				fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
-			default:
-				pass("Backup is up to date with installed Spotify version")
-			}
+			if _, err := os.Stat(backupFolder); err != nil {
+				fail("No backup found. Run \"spicetify backup\"")
+			} else {
+				backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
+				switch {
+				case backStat.IsEmpty():
+					fail("No backup found. Run \"spicetify backup\"")
+				case backStat.IsOutdated():
+					fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
+				default:
+					pass("Backup is up to date with installed Spotify version")
+				}
+			}
 		}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if prefsErr == nil {
backupVersion := backupSection.Key("version").MustString("")
backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
switch {
case backStat.IsEmpty():
fail("No backup found. Run \"spicetify backup\"")
case backStat.IsOutdated():
fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
default:
pass("Backup is up to date with installed Spotify version")
}
}
if prefsErr == nil {
backupVersion := backupSection.Key("version").MustString("")
if _, err := os.Stat(backupFolder); err != nil {
fail("No backup found. Run \"spicetify backup\"")
} else {
backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
switch {
case backStat.IsEmpty():
fail("No backup found. Run \"spicetify backup\"")
case backStat.IsOutdated():
fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
default:
pass("Backup is up to date with installed Spotify version")
}
}
}
🤖 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 `@src/cmd/doctor.go` around lines 64 - 75, The backup check in doctor should
not call backupstatus.Get unconditionally because it can terminate the process
with log.Fatal when the backup folder is missing. Update the doctor command
logic around the backupSection.Key("version") / backupstatus.Get path to first
verify the backup directory exists (or otherwise handle the missing-folder case)
and then emit the existing “No backup found” diagnostic instead of letting
backupstatus.Get abort the program. Keep the fix localized to the doctor backup
check and the backupstatus.Get call site.

}
Comment on lines +51 to +76

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Backup diagnostics unnecessarily coupled to Apps-folder detection.

The backup check (lines 64-75) only runs when both appsErr == nil and prefsErr == nil, but backupstatus.Get only depends on prefsPathCfg and backupFolder, not on the Apps folder. If spotify_path is misconfigured while prefs_path/backup are actually fine, Doctor will report the install-path problem but silently skip telling the user whether their backup is missing or outdated — reducing the diagnostic value precisely when the user needs more information to fix things.

🩺 Decouple the backup check from the Apps-folder check
-	if appsErr == nil {
-		spotStat := spotifystatus.Get(spotifyAppsPath)
-		switch {
-		case spotStat.IsInvalid():
-			fail("Spotify \"Apps\" folder is empty or invalid. Reinstall Spotify")
-		case spotStat.IsMixed():
-			fail("Spotify has a mix of modified and stock files, likely from a recent Spotify update. Run \"spicetify backup apply\" to fix it")
-		case spotStat.IsStock():
-			pass("Spotify is in stock (unmodified) state")
-		case spotStat.IsApplied():
-			pass("Spotify has spicetify customizations applied")
-		}
-
-		if prefsErr == nil {
-			backupVersion := backupSection.Key("version").MustString("")
-			backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
-			...
-		}
-	}
+	if appsErr == nil {
+		spotStat := spotifystatus.Get(spotifyAppsPath)
+		switch {
+		case spotStat.IsInvalid():
+			fail("Spotify \"Apps\" folder is empty or invalid. Reinstall Spotify")
+		case spotStat.IsMixed():
+			fail("Spotify has a mix of modified and stock files, likely from a recent Spotify update. Run \"spicetify backup apply\" to fix it")
+		case spotStat.IsStock():
+			pass("Spotify is in stock (unmodified) state")
+		case spotStat.IsApplied():
+			pass("Spotify has spicetify customizations applied")
+		}
+	}
+
+	if prefsErr == nil {
+		backupVersion := backupSection.Key("version").MustString("")
+		// backup check here, independent of appsErr
+	}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if appsErr == nil {
spotStat := spotifystatus.Get(spotifyAppsPath)
switch {
case spotStat.IsInvalid():
fail("Spotify \"Apps\" folder is empty or invalid. Reinstall Spotify")
case spotStat.IsMixed():
fail("Spotify has a mix of modified and stock files, likely from a recent Spotify update. Run \"spicetify backup apply\" to fix it")
case spotStat.IsStock():
pass("Spotify is in stock (unmodified) state")
case spotStat.IsApplied():
pass("Spotify has spicetify customizations applied")
}
if prefsErr == nil {
backupVersion := backupSection.Key("version").MustString("")
backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
switch {
case backStat.IsEmpty():
fail("No backup found. Run \"spicetify backup\"")
case backStat.IsOutdated():
fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
default:
pass("Backup is up to date with installed Spotify version")
}
}
}
if appsErr == nil {
spotStat := spotifystatus.Get(spotifyAppsPath)
switch {
case spotStat.IsInvalid():
fail("Spotify \"Apps\" folder is empty or invalid. Reinstall Spotify")
case spotStat.IsMixed():
fail("Spotify has a mix of modified and stock files, likely from a recent Spotify update. Run \"spicetify backup apply\" to fix it")
case spotStat.IsStock():
pass("Spotify is in stock (unmodified) state")
case spotStat.IsApplied():
pass("Spotify has spicetify customizations applied")
}
}
if prefsErr == nil {
backupVersion := backupSection.Key("version").MustString("")
backStat := backupstatus.Get(prefsPathCfg, backupFolder, backupVersion)
switch {
case backStat.IsEmpty():
fail("No backup found. Run \"spicetify backup\"")
case backStat.IsOutdated():
fail("Backup version doesn't match installed Spotify version. Run \"spicetify backup apply\"")
default:
pass("Backup is up to date with installed Spotify version")
}
}
🤖 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 `@src/cmd/doctor.go` around lines 51 - 76, The backup diagnostics in doctor are
unnecessarily gated by the Apps-folder check, so `backupstatus.Get` never runs
when `appsErr` is non-nil. Update the logic in `doctor.go` so the backup
validation tied to `backupSection`, `prefsPathCfg`, and `backupFolder` runs
independently of the `spotifystatus.Get`/Apps-folder branch, while still keeping
the `prefsErr == nil` guard if needed. Preserve the existing `fail`/`pass`
messages and structure, but decouple the backup check from the `appsErr == nil`
condition.


themeName := settingSection.Key("current_theme").String()
if len(themeName) == 0 {
pass("No theme selected")
} else {
themeFolder := findThemeFolder(themeName)
if len(themeFolder) == 0 {
fail("Theme \"" + themeName + "\" is set as current_theme but its folder cannot be found")
} else {
pass("Theme \"" + themeName + "\" found")
checkThemeAsset(themeFolder, "replace_colors", "color.ini", pass, fail)
checkThemeAsset(themeFolder, "inject_css", "user.css", pass, fail)
checkThemeAsset(themeFolder, "inject_theme_js", "theme.js", pass, fail)
checkThemeAsset(themeFolder, "overwrite_assets", "assets", pass, fail)
}
}

for _, ext := range featureSection.Key("extensions").Strings("|") {
if len(ext) == 0 {
continue
}
if _, err := utils.GetExtensionPath(ext); err != nil {
fail("Extension \"" + ext + "\" is enabled in config but its file cannot be found")
}
}

for _, app := range featureSection.Key("custom_apps").Strings("|") {
if len(app) == 0 {
continue
}
if _, err := utils.GetCustomAppPath(app); err != nil {
fail("Custom app \"" + app + "\" is enabled in config but cannot be found")
}
}

if problems == 0 {
utils.PrintSuccess("No problems found")
return
}

utils.PrintWarning(fmt.Sprintf("%d problem(s) found", problems))
os.Exit(1)
}

func checkThemeAsset(themeFolder, settingKey, assetName string, pass, fail func(string)) {
if !settingSection.Key(settingKey).MustBool(false) {
return
}

if _, err := os.Stat(filepath.Join(themeFolder, assetName)); err != nil {
fail("\"" + settingKey + "\" is enabled but \"" + assetName + "\" is missing from theme \"" + filepath.Base(themeFolder) + "\"")
return
}

pass("\"" + settingKey + "\" asset \"" + assetName + "\" found in theme \"" + filepath.Base(themeFolder) + "\"")
}