fix(update|watch|apply): re-exec with separate args, poll debugger without busy-loop, guard trailing spicetify_map comment

[Xuepoo]

Summary

Three small independent bugs found during a review of the v2.44.0 codebase. Three commits, one per fix.

1. fix(update): re-run remaining commands with updated binary

spicetify.go:293 previously did:

cmd = exec.Command(ex, strings.Join(commands[:], " "))

exec.Command does not split on spaces, so after a successful update the child receives e.g. "update backup apply" as one argv. The child's own arg parser (spicetify.go:55-67) then treats it as a single command token, which matches no case in the chainable-commands loop → fatal: Command "update backup apply" not found. printed to the user, and the intended re-run of the chain on the freshly-updated binary never happens.

Reproduction: any spicetify update ... invocation that finds an update (e.g. spicetify update backup apply) ends with the fatal "Command not found" message. Introduced in d5c773f (PR #2227, 2023-03-30).

Fix: pass the commands as separate arguments, filtering out the update/upgrade token (so it works regardless of position in the chain), and skip re-exec when nothing remains:

remaining := slices.DeleteFunc(slices.Clone(commands), func(c string) bool {
    return c == "update" || c == "upgrade"
})
if len(remaining) > 0 {
    cmd = exec.Command(ex, remaining...)
    utils.CmdScanner(cmd)
}

2. fix(watch): sleep while waiting for Spotify debugger

src/cmd/watch.go:240-242 previously busy-looped:

for len(utils.GetDebuggerPath()) == 0 {
    // Wait until debugger is up
}

No sleep in the loop; each iteration performs a full HTTP GET + JSON parse against localhost:9222/json/list (src/utils/watcher.go). While Spotify takes several seconds to launch, one core spins at 100% CPU and the local debugger endpoint is hammered. Present since d30dcba (2020).

Fix: poll with time.Sleep(500 * time.Millisecond).

3. fix(apply): guard remap when spicetify_map comment is on last line

src/cmd/apply.go:271-273 (pushExtensions) previously did:

if len(mapping) > 0 {
    lines[i+1] = strings.Replace(lines[i+1], mapping[0], mapping[1], 1)
}

If the //spicetify_map{...}{...} comment sits on the final line of an .mjs extension, lines[i+1] is out of range → index-out-of-range panic during spicetify apply / refresh -e / watch of that extension. A trailing comment has no following line to remap, so it should simply be skipped.

Fix: add the i+1 < len(lines) guard.

Verification

• go build ./... passes, go vet ./... clean, gofmt clean.
• Bug 1 reproduced with a subprocess test: child receives ["update backup apply"] as one argv before the fix; after the fix the args are passed separately.
• Bug 3 reproduced with "line1\n// spicetify_map{A}{B}" → panic before, no-op after.

No test infrastructure exists in this repo; changes validated by build + the reproductions above. Happy to adjust if you want different polling intervals or a different approach for the re-exec.

Summary by CodeRabbit

• Bug Fixes
  • Improved post-update command reruns so remaining commands execute correctly and are skipped when none remain.
  • Fixed an issue that could cause errors when processing an extension mapping on the final line.
  • Improved debugger startup reliability by adding a brief delay between availability checks.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: ee4af051-b8ab-460a-835e-02a74c4819dc

📥 Commits

Reviewing files that changed from the base of the PR and between c316acf and 39f5553.

📒 Files selected for processing (1)

• spicetify.go

🚧 Files skipped from review as they are similar to previous changes (1)

• spicetify.go

---

📝 Walkthrough

Walkthrough

The changes revise post-update command reruns, prevent out-of-range extension mapping access, and add a delay between debugger readiness checks.

Changes

Runtime safeguards

Layer / File(s)	Summary
Post-update command rerun arguments spicetify.go	The update flow forwards global flags, removes update and upgrade, passes remaining commands as separate arguments, and skips execution when no commands remain.
Extension mapping bounds check src/cmd/apply.go	The mapping logic checks that a following line exists before accessing it.
Debugger readiness polling src/cmd/watch.go	The readiness loop waits 500 milliseconds between availability checks.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• spicetify/cli#3493: Both changes modify the debugger startup and reload flow in src/cmd/watch.go.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes all three bug fixes: update re-execution, debugger polling, and trailing comment handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@spicetify.go`:
- Around line 295-300: Preserve parsed global options when spawning child
commands in the automatic backup/apply flow and the remaining-command rerun
around commands and flags. Build each child argument list by forwarding the
relevant original flags, including --bypass-admin when required, before
appending the command arguments; keep the existing len(remaining) > 0 guard and
ensure options such as -e and --no-restart reach both child processes.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e4bd4483-d6a0-46e3-9160-224e29d110e2

📥 Commits

Reviewing files that changed from the base of the PR and between c9571cd and c316acf.

📒 Files selected for processing (3)

• spicetify.go
• src/cmd/apply.go
• src/cmd/watch.go

[Xuepoo]

Addressed the review comment in commit 39f5553 (fix(update): forward global options to child processes): parsed global options (including --bypass-admin restored from bypassAdminCheck, -n, -q, ...) are now forwarded to both child processes before the command arguments, keeping the len(remaining) > 0 guard.

[rxri]

The first "fix" does not have the merit because update command is not supposed to be chainable. It's literally under the "Unchainable commands" group. Also, please stop using AI to reply or make pull request descriptions in our org's repositories. Thank you.