feat: add upgrade script for v5 migration

[mryel00]

This script adds make upgrade to automatically migrate to crowsnest v5.

1. Backup existing config and migrate to be v5 compatible
2. Uninstall v4, deleting all old files
3. Fetching for new branches and switching to v5 (master->v4, future branches named after major version)
4. Install crowsnest v5

[mryel00]

@coderabbitai full review

[coderabbitai]

✅ Actions performed

Full review triggered.

[coderabbitai]

📝 Walkthrough

Walkthrough

Adds a new upgrade Makefile target to facilitate v4 to v5 migration. Introduces tools/migrate_configs.sh script to migrate crowsnest configurations to Moonraker-compatible format. The process backs up config, uninstalls v4, updates repository, installs v5, and restores migrated config.

Changes

Cohort / File(s)	Summary
Build Configuration Makefile	New upgrade target orchestrates v4 to v5 migration: calls tools/migrate_configs.sh for config backup/migration, uninstalls v4 binaries, updates Git repository to v5 branch, installs v5, and restores migrated configuration with cleanup.
Migration Tooling tools/migrate_configs.sh	New Bash script migrates crowsnest configs to Moonraker format. Locates config via systemd or user home, backs up both crowsnest.conf and moonraker.conf with timestamps, removes unsupported sections/keys, normalizes mode values, cleans legacy RTSP comments, validates crudini availability, and outputs migrated path for restoration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30 minutes

Poem

🐰 From v4 to v5 we hop with cheer,
With migration scripts to hold configs dear,
Backups preserved, old versions retire,
Moonraker awaits—hop higher, hop higher! 🚀

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 12.50% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'feat: add upgrade script for v5 migration' directly describes the main addition to the changeset—a new upgrade mechanism for v5 migration via a Makefile target and migration script.
Description check	✅ Passed	The PR description is directly related to the changeset, detailing the v5 migration steps performed by the upgrade script including backup, uninstall, branch switching, and installation.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

Tip

Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord.

---

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 and usage tips.}

[coderabbitai]

Actionable comments posted: 7

🧹 Nitpick comments (3)

tools/migrate_configs.sh (2)

130-133: Duplicate removal of [update_manager crowsnest] from moonraker.conf.

crudini --del "${moonraker_cfg}" "update_manager crowsnest" is called in migrate_crudini (line 132) and again in cleanup_moonraker_config (line 144). Since both are invoked on the same file in sequence (lines 160, 162), the second call is redundant. Remove it from one location.

Also applies to: 141-146

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/migrate_configs.sh` around lines 130 - 133, The script currently
deletes the "update_manager crowsnest" section from moonraker.conf twice: once
in the migrate_crudini function (the call to crudini --del "${moonraker_cfg}"
"update_manager crowsnest") and again in cleanup_moonraker_config; remove the
redundant crudini --del invocation from one of those functions (keep it in
whichever function logically owns the migration step—e.g., keep it in
migrate_crudini and delete it from cleanup_moonraker_config) so the section is
only removed once.

---

159-162: No error handling between migration steps.

With set -e, any failure in migrate_crudini or cleanup_legacy_comments will abort the script after backup_config has already been performed but before the migrated file is moved to .v5. This leaves the config in a partially-migrated state with no clear indication of what happened. Consider adding a trap to log the failure context or restore the backup on error.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/migrate_configs.sh` around lines 159 - 162, The sequence of steps
calling backup_config, migrate_crudini, cleanup_legacy_comments, and
cleanup_moonraker_config lacks error handling and with set -e a failure leaves a
partial migration; add a trap (e.g., using trap '...' ERR EXIT) before calling
these functions that logs the failing step and either restores the backup
(invoking backup_config's inverse or a restore function) or moves the original
CROWSNEST_CFG_PATH back into place, and ensure the trap is cleared on success;
reference the existing functions backup_config, migrate_crudini,
cleanup_legacy_comments, and cleanup_moonraker_config so the trap can report
which function failed and perform the restore/cleanup.

Makefile (1)

47-74: Consider extracting the upgrade logic into a dedicated shell script.

The upgrade target is 27 lines of inline shell, well beyond typical Makefile recipe length (also flagged by checkmake). A dedicated tools/upgrade.sh would allow proper set -e, trap for cleanup/rollback, and clearer control flow — similar to how other targets delegate to tools/install.sh and tools/uninstall.sh.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@Makefile` around lines 47 - 74, The upgrade Makefile recipe (target
"upgrade") should be extracted into a new shell script (e.g., tools/upgrade.sh)
that performs the same steps (calling tools/migrate_configs.sh,
tools/uninstall.sh, git switch/pull v5, sudo tools/install.sh, and restoring
tools/.migrated_conf_path) but runs with strict error handling (set -e) and a
trap for cleanup/rollback; then replace the multiline recipe with a single
invocation of that script from the "upgrade" target. Ensure the new script
preserves use of tools/.migrated_conf_path, replicates the current prints/yes
piping to tools/uninstall.sh, and exits with non-zero on failure so the Makefile
sees errors.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@Makefile`:
- Line 64: The Makefile recipe currently hardcodes a nested sudo call for
running tools/install.sh which can cause nested sudo prompts or mid-recipe
password prompts; change the upgrade target to avoid a hardcoded sudo by
detecting whether the current process is already root and only prefixing sudo
when needed (e.g. check EUID or UID and run either bash -c 'tools/install.sh' or
sudo bash -c 'tools/install.sh'), and also add a brief note in the target's help
or README that upgrade should be run with/without sudo consistently so users are
prompted upfront rather than mid-recipe; reference the recipe line invoking
tools/install.sh when making the change.
- Around line 47-74: The upgrade target is destructive because it uninstalls v4
(tools/uninstall.sh) before verifying v5 is available or ensuring subsequent
steps will succeed; add prechecks and a rollback plan: in the upgrade recipe
(target "upgrade") first verify the remote v5 branch exists (e.g., git fetch &&
git ls-remote --heads origin v5) and network reachability, capture and save the
current branch/commit (e.g., save git rev-parse HEAD to a temp file), create a
full backup of current install and configs (tarball of installed files and
tools/.migrated_conf_path), and only run tools/uninstall.sh after those checks
succeed; after uninstall, wrap the remaining steps so failures trigger an
automated rollback that uses the saved commit/backup to restore v4 (git switch
back to saved ref and re-run tools/install.sh from the backup) and emit clear
manual-recovery instructions if automatic rollback fails; ensure references to
tools/migrate_configs.sh, tools/.migrated_conf_path, git switch v5, and
tools/install.sh are used when saving/restoring state.
- Around line 60-62: Ensure the Makefile pre-fetches and validates the remote v5
branch before running uninstall.sh: run git fetch --all and verify the remote
branch exists (e.g., check origin/v5 via git ls-remote or git rev-parse) and
only then perform git switch v5 (or create and track with a switch --track from
origin/v5) and git pull origin v5; if fetch or branch validation fails, abort
and emit a clear error message so uninstall.sh is not run on a missing branch.

In `@tools/migrate_configs.sh`:
- Around line 148-150: The script currently assumes MOONRAKER_CFG_PATH (derived
from CROWSNEST_CFG_PATH via find_config) exists; add a check after
MOONRAKER_CFG_PATH is computed to verify the file exists (e.g., test [[ -f
"${MOONRAKER_CFG_PATH}" ]]) and when missing log a warning (e.g., log_warn
"moonraker.conf not found at ${MOONRAKER_CFG_PATH}, skipping moonraker
migration.") and skip any moonraker-specific actions (backups, moves, use of
MIGRATED_TEMP) by gating those steps behind this existence check so operations
don't target a non-existent file.
- Around line 32-78: In find_config(), the parsing of the systemd
EnvironmentFile uses cut -d= -f2 which will truncate values containing '=';
change that extraction to use cut -d= -f2- so everything after the first '=' is
preserved (and then strip any surrounding quotes), and ensure subsequent logic
that reads EnvironmentFile into env_file_path still checks -n and -f on the full
path; update the code around the EnvironmentFile extraction and the variables
service_content/env_file_path to use the new extraction and trimming before
using args_line and extracted_path.
- Around line 135-139: The sed range delete in cleanup_legacy_comments() can
wipe the whole file if the closing '^#\+$' marker is missing; update
cleanup_legacy_comments (and use the cfg variable) to first check whether a
closing-only-# line exists (e.g., grep -q '^#\+$' "$cfg") and only run sed -i
'/RTSP Stream URL:/,/^#\+$/d' when that marker is present; if the marker is
absent, fallback to a safer action such as deleting a limited number of lines
after the match (e.g., sed -i '/RTSP Stream URL:/,+10d' "$cfg"), or skip
deletion and emit a warning log, so you never perform an unbounded in-place
deletion.
- Around line 80-88: The backup_config function currently ignores its second
argument and blindly derives the moonraker path from $1; update backup_config to
accept and use both parameters (cfg="$1", moonraker_cfg="$2") or remove the
second argument at the call site; additionally, before copying the
derived/received moonraker path in backup_config, verify the file exists (e.g.,
test -f "$moonraker_cfg") and only perform cp when present, emitting a clear
error or warning if missing so the script doesn't fail silently under set -e and
the crowsnest backup isn't lost; reference function name backup_config and the
call that passes two args to locate the change.

---

Nitpick comments:
In `@Makefile`:
- Around line 47-74: The upgrade Makefile recipe (target "upgrade") should be
extracted into a new shell script (e.g., tools/upgrade.sh) that performs the
same steps (calling tools/migrate_configs.sh, tools/uninstall.sh, git
switch/pull v5, sudo tools/install.sh, and restoring tools/.migrated_conf_path)
but runs with strict error handling (set -e) and a trap for cleanup/rollback;
then replace the multiline recipe with a single invocation of that script from
the "upgrade" target. Ensure the new script preserves use of
tools/.migrated_conf_path, replicates the current prints/yes piping to
tools/uninstall.sh, and exits with non-zero on failure so the Makefile sees
errors.

In `@tools/migrate_configs.sh`:
- Around line 130-133: The script currently deletes the "update_manager
crowsnest" section from moonraker.conf twice: once in the migrate_crudini
function (the call to crudini --del "${moonraker_cfg}" "update_manager
crowsnest") and again in cleanup_moonraker_config; remove the redundant crudini
--del invocation from one of those functions (keep it in whichever function
logically owns the migration step—e.g., keep it in migrate_crudini and delete it
from cleanup_moonraker_config) so the section is only removed once.
- Around line 159-162: The sequence of steps calling backup_config,
migrate_crudini, cleanup_legacy_comments, and cleanup_moonraker_config lacks
error handling and with set -e a failure leaves a partial migration; add a trap
(e.g., using trap '...' ERR EXIT) before calling these functions that logs the
failing step and either restores the backup (invoking backup_config's inverse or
a restore function) or moves the original CROWSNEST_CFG_PATH back into place,
and ensure the trap is cleared on success; reference the existing functions
backup_config, migrate_crudini, cleanup_legacy_comments, and
cleanup_moonraker_config so the trap can report which function failed and
perform the restore/cleanup.