From e76be254dda0afd294f56665d72d51d3d4a54260 Mon Sep 17 00:00:00 2001 From: Alexey <1556417+alex-solovyev@users.noreply.github.com> Date: Tue, 17 Mar 2026 16:28:35 +0100 Subject: [PATCH 1/2] fix: clean up orphaned supervisor-helper.sh and supervisor/ during update (GH#5147) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After the supervisor-to-pulse-wrapper migration, aidevops update (rsync) only adds/overwrites files — it doesn't delete files that no longer exist in the source. Users who installed before the migration retain orphaned supervisor-helper.sh and supervisor/ modules that shadow pulse-wrapper.sh. Add migrate_orphaned_supervisor() to setup-modules/migrations.sh that: - Removes orphaned scripts/supervisor-helper.sh from deployed agents - Removes orphaned scripts/supervisor/ module directory (with safety check) - Removes cron entries referencing supervisor-helper.sh - Removes old launchd plist with com.aidevops.supervisor-pulse label (macOS) The migration is wired into both interactive and non-interactive setup paths, running after cleanup_deprecated_paths and before the pulse scheduler section so old entries are cleaned before new ones are installed. Closes #5147 --- setup-modules/migrations.sh | 83 ++++++++ setup.sh | 2 + tests/test-migrate-orphaned-supervisor.sh | 235 ++++++++++++++++++++++ 3 files changed, 320 insertions(+) create mode 100755 tests/test-migrate-orphaned-supervisor.sh diff --git a/setup-modules/migrations.sh b/setup-modules/migrations.sh index eda72f98f5..792ca5b3e1 100644 --- a/setup-modules/migrations.sh +++ b/setup-modules/migrations.sh @@ -963,3 +963,86 @@ migrate_pulse_repos_to_repos_json() { return 0 } + +# Migrate orphaned supervisor-helper.sh and supervisor/ modules (GH#5147) +# After the supervisor-to-pulse-wrapper migration (PR #2291, PR #2475), the +# upstream repo moved supervisor files to supervisor-archived/. But aidevops +# update (rsync) only adds/overwrites — it doesn't delete files that no longer +# exist in the source. Users who installed before the migration retain: +# - ~/.aidevops/agents/scripts/supervisor-helper.sh (old entry point) +# - ~/.aidevops/agents/scripts/supervisor/ (old module directory) +# - cron/launchd entries invoking supervisor-helper.sh pulse +# These orphaned files shadow the new pulse-wrapper.sh architecture. +# This migration removes the orphaned files and rewrites scheduler entries. +migrate_orphaned_supervisor() { + local agents_dir="$HOME/.aidevops/agents" + local scripts_dir="$agents_dir/scripts" + local cleaned=0 + + # 1. Remove orphaned supervisor-helper.sh from deployed scripts + # The canonical location is now supervisor-archived/supervisor-helper.sh + # (shipped for reference/tests only, not as an active entry point) + if [[ -f "$scripts_dir/supervisor-helper.sh" ]]; then + rm -f "$scripts_dir/supervisor-helper.sh" + print_info "Removed orphaned supervisor-helper.sh from deployed scripts" + ((++cleaned)) + fi + + # 2. Remove orphaned supervisor/ module directory + # The canonical location is now supervisor-archived/ (shipped by rsync) + # Only remove if it's the old modules dir (contains pulse.sh, dispatch.sh, etc.) + # Do NOT remove supervisor-archived/ — that's the intentional archive + if [[ -d "$scripts_dir/supervisor" && ! -L "$scripts_dir/supervisor" ]]; then + # Verify it's the old module directory (not something user-created) + if [[ -f "$scripts_dir/supervisor/pulse.sh" ]] || + [[ -f "$scripts_dir/supervisor/dispatch.sh" ]] || + [[ -f "$scripts_dir/supervisor/_common.sh" ]]; then + rm -rf "$scripts_dir/supervisor" + print_info "Removed orphaned supervisor/ module directory from deployed scripts" + ((++cleaned)) + fi + fi + + # 3. Migrate cron entries from supervisor-helper.sh to pulse-wrapper.sh + # Old pattern: */2 * * * * ... supervisor-helper.sh pulse ... + # New pattern: already installed by setup.sh's pulse section + # Strategy: remove old entries; setup.sh will install the new one if pulse is enabled + local current_crontab + current_crontab=$(crontab -l 2>/dev/null) || current_crontab="" + if echo "$current_crontab" | grep -qF "supervisor-helper.sh"; then + # Remove all cron lines referencing supervisor-helper.sh + local new_crontab + new_crontab=$(echo "$current_crontab" | grep -v "supervisor-helper.sh") + if [[ -n "$new_crontab" ]]; then + printf '%s\n' "$new_crontab" | crontab - 2>/dev/null || true + else + # All entries were supervisor-helper.sh — clear crontab + echo "" | crontab - 2>/dev/null || true + fi + print_info "Removed orphaned supervisor-helper.sh cron entries" + print_info " pulse-wrapper.sh will be installed by setup.sh if supervisor pulse is enabled" + ((++cleaned)) + fi + + # 4. Migrate launchd entries from old supervisor label (macOS only) + # Old label: com.aidevops.supervisor-pulse (from cron.sh/launchd.sh) + # New label: com.aidevops.aidevops-supervisor-pulse (from setup.sh) + # setup.sh already handles the new label cleanup at line ~1000, but + # the old label from cron.sh may also be present + if [[ "$(uname -s)" == "Darwin" ]]; then + local old_label="com.aidevops.supervisor-pulse" + local old_plist="$HOME/Library/LaunchAgents/${old_label}.plist" + if [[ -f "$old_plist" ]] || _launchd_has_agent "$old_label" 2>/dev/null; then + launchctl unload "$old_plist" 2>/dev/null || true + rm -f "$old_plist" + print_info "Removed orphaned supervisor-pulse LaunchAgent ($old_label)" + ((++cleaned)) + fi + fi + + if [[ $cleaned -gt 0 ]]; then + print_success "Cleaned up $cleaned orphaned supervisor artifact(s) — pulse-wrapper.sh is the active system" + fi + + return 0 +} diff --git a/setup.sh b/setup.sh index 9792f1e240..facdb08672 100755 --- a/setup.sh +++ b/setup.sh @@ -716,6 +716,7 @@ main() { migrate_mcp_env_to_credentials migrate_pulse_repos_to_repos_json cleanup_deprecated_paths + migrate_orphaned_supervisor cleanup_deprecated_mcps cleanup_stale_bun_opencode validate_opencode_config @@ -795,6 +796,7 @@ main() { confirm_step "Migrate mcp-env.sh -> credentials.sh" && migrate_mcp_env_to_credentials confirm_step "Migrate pulse-repos.json into repos.json" && migrate_pulse_repos_to_repos_json confirm_step "Cleanup deprecated agent paths" && cleanup_deprecated_paths + confirm_step "Migrate orphaned supervisor to pulse-wrapper" && migrate_orphaned_supervisor confirm_step "Cleanup deprecated MCP entries (hetzner, serper, etc.)" && cleanup_deprecated_mcps confirm_step "Cleanup stale bun opencode install" && cleanup_stale_bun_opencode confirm_step "Validate and repair OpenCode config schema" && validate_opencode_config diff --git a/tests/test-migrate-orphaned-supervisor.sh b/tests/test-migrate-orphaned-supervisor.sh new file mode 100755 index 0000000000..ccb51f165e --- /dev/null +++ b/tests/test-migrate-orphaned-supervisor.sh @@ -0,0 +1,235 @@ +#!/usr/bin/env bash +# Tests for migrate_orphaned_supervisor() (GH#5147) +# Verifies cleanup of orphaned supervisor-helper.sh and supervisor/ modules + +set -euo pipefail + +REPO_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +MIGRATIONS_SCRIPT="$REPO_DIR/setup-modules/migrations.sh" + +PASS_COUNT=0 +FAIL_COUNT=0 +TOTAL_COUNT=0 + +pass() { + local name="$1" + PASS_COUNT=$((PASS_COUNT + 1)) + TOTAL_COUNT=$((TOTAL_COUNT + 1)) + printf "\033[0;32mPASS\033[0m %s\n" "$name" + return 0 +} + +fail() { + local name="$1" + local detail="${2:-}" + FAIL_COUNT=$((FAIL_COUNT + 1)) + TOTAL_COUNT=$((TOTAL_COUNT + 1)) + printf "\033[0;31mFAIL\033[0m %s\n" "$name" + if [[ -n "$detail" ]]; then + printf " %s\n" "$detail" + fi + return 0 +} + +section() { + echo "" + echo "=== $1 ===" + return 0 +} + +# Create a temporary HOME to isolate tests from the real system +TEST_HOME=$(mktemp -d) +# shellcheck disable=SC2064 +trap "rm -rf '$TEST_HOME'" EXIT + +# Override HOME for the duration of the test +export HOME="$TEST_HOME" + +# Create the deployed agents directory structure (simulating pre-migration state) +AGENTS_DIR="$TEST_HOME/.aidevops/agents" +SCRIPTS_DIR="$AGENTS_DIR/scripts" +mkdir -p "$SCRIPTS_DIR" + +# Stub functions that migrations.sh expects from setup.sh +print_info() { echo "[INFO] $1"; } +print_success() { echo "[SUCCESS] $1"; } +print_warning() { echo "[WARNING] $1"; } +print_error() { echo "[ERROR] $1"; } +# _launchd_has_agent is defined in setup.sh; stub it for Linux tests +_launchd_has_agent() { return 1; } +# Stubs for other functions referenced by migrations.sh +find_opencode_config() { return 1; } +create_backup_with_rotation() { return 0; } +should_overwrite_user_file() { return 0; } +resolve_mcp_binary_path() { + echo "" + return 1 +} +update_mcp_paths_in_opencode() { return 0; } +sanitize_plugin_namespace() { + echo "$1" + return 0 +} +# Export stubs so sourced script can find them +export -f print_info print_success print_warning print_error _launchd_has_agent +export -f find_opencode_config create_backup_with_rotation should_overwrite_user_file +export -f resolve_mcp_binary_path update_mcp_paths_in_opencode sanitize_plugin_namespace + +# Source the migrations script to get the function +# shellcheck disable=SC1090 +source "$MIGRATIONS_SCRIPT" + +# ============================================================================ +section "Test: removes orphaned supervisor-helper.sh" +# ============================================================================ + +# Create orphaned file +echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor-helper.sh" + +if [[ -f "$SCRIPTS_DIR/supervisor-helper.sh" ]]; then + pass "Setup: orphaned supervisor-helper.sh exists before migration" +else + fail "Setup: orphaned supervisor-helper.sh should exist" +fi + +migrate_orphaned_supervisor >/dev/null 2>&1 + +if [[ ! -f "$SCRIPTS_DIR/supervisor-helper.sh" ]]; then + pass "supervisor-helper.sh removed after migration" +else + fail "supervisor-helper.sh should have been removed" +fi + +# ============================================================================ +section "Test: removes orphaned supervisor/ module directory" +# ============================================================================ + +# Create orphaned module directory with recognizable files +mkdir -p "$SCRIPTS_DIR/supervisor" +echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/pulse.sh" +echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/dispatch.sh" +echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/_common.sh" + +if [[ -d "$SCRIPTS_DIR/supervisor" ]]; then + pass "Setup: orphaned supervisor/ directory exists before migration" +else + fail "Setup: orphaned supervisor/ directory should exist" +fi + +migrate_orphaned_supervisor >/dev/null 2>&1 + +if [[ ! -d "$SCRIPTS_DIR/supervisor" ]]; then + pass "supervisor/ directory removed after migration" +else + fail "supervisor/ directory should have been removed" +fi + +# ============================================================================ +section "Test: does NOT remove supervisor-archived/" +# ============================================================================ + +mkdir -p "$SCRIPTS_DIR/supervisor-archived" +echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor-archived/supervisor-helper.sh" + +migrate_orphaned_supervisor >/dev/null 2>&1 + +if [[ -d "$SCRIPTS_DIR/supervisor-archived" ]]; then + pass "supervisor-archived/ preserved after migration" +else + fail "supervisor-archived/ should NOT have been removed" +fi + +# ============================================================================ +section "Test: does NOT remove unrelated supervisor/ directory" +# ============================================================================ + +# Create a supervisor/ directory without the expected module files +mkdir -p "$SCRIPTS_DIR/supervisor" +echo 'custom content' >"$SCRIPTS_DIR/supervisor/my-custom-file.txt" + +migrate_orphaned_supervisor >/dev/null 2>&1 + +if [[ -d "$SCRIPTS_DIR/supervisor" ]]; then + pass "Unrelated supervisor/ directory preserved (no module files)" +else + fail "Unrelated supervisor/ directory should NOT have been removed" +fi + +# Clean up for next test +rm -rf "$SCRIPTS_DIR/supervisor" + +# ============================================================================ +section "Test: idempotent — no errors when nothing to clean" +# ============================================================================ + +# Run migration when there's nothing to clean +output=$(migrate_orphaned_supervisor 2>&1) +exit_code=$? + +if [[ $exit_code -eq 0 ]]; then + pass "Migration succeeds when nothing to clean (exit 0)" +else + fail "Migration should succeed when nothing to clean" "exit code: $exit_code" +fi + +# Should NOT print the success message when nothing was cleaned +if [[ "$output" != *"Cleaned up"* ]]; then + pass "No 'Cleaned up' message when nothing to clean" +else + fail "Should not print 'Cleaned up' when nothing was cleaned" "$output" +fi + +# ============================================================================ +section "Test: removes cron entries referencing supervisor-helper.sh" +# ============================================================================ + +# Install a fake cron entry (only if crontab is available) +if command -v crontab &>/dev/null; then + # Save current crontab + original_crontab=$(crontab -l 2>/dev/null) || original_crontab="" + + # Install test crontab with supervisor-helper.sh entry + { + echo "# aidevops: test-keep-this" + echo "*/5 * * * * echo keep-this # aidevops: test-keep" + echo "*/2 * * * * /bin/bash /home/test/.aidevops/agents/scripts/supervisor-helper.sh pulse >> /tmp/test.log 2>&1 # aidevops: supervisor-pulse" + } | crontab - 2>/dev/null + + migrate_orphaned_supervisor >/dev/null 2>&1 + + new_crontab=$(crontab -l 2>/dev/null) || new_crontab="" + + if echo "$new_crontab" | grep -qF "supervisor-helper.sh"; then + fail "Cron entry referencing supervisor-helper.sh should have been removed" + else + pass "Cron entry referencing supervisor-helper.sh removed" + fi + + if echo "$new_crontab" | grep -qF "keep-this"; then + pass "Non-supervisor cron entries preserved" + else + fail "Non-supervisor cron entries should have been preserved" + fi + + # Restore original crontab + if [[ -n "$original_crontab" ]]; then + printf '%s\n' "$original_crontab" | crontab - 2>/dev/null || true + else + crontab -r 2>/dev/null || true + fi +else + pass "Cron test skipped (crontab not available)" +fi + +# ============================================================================ +# Summary +# ============================================================================ +echo "" +echo "================================" +echo "Results: $PASS_COUNT passed, $FAIL_COUNT failed, $TOTAL_COUNT total" +echo "================================" + +if [[ $FAIL_COUNT -gt 0 ]]; then + exit 1 +fi +exit 0 From 91da7ea9f22cab555b74ffa7b9412e23ff55055a Mon Sep 17 00:00:00 2001 From: Alexey <1556417+alex-solovyev@users.noreply.github.com> Date: Tue, 17 Mar 2026 16:37:51 +0100 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20address=20PR=20#5151=20review=20?= =?UTF-8?q?=E2=80=94=20crontab=20-r=20and=20launchctl=20remove=20by=20labe?= =?UTF-8?q?l?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Line 1020: replace 'echo "" | crontab -' with 'crontab -r' to truly clear the crontab when all entries were supervisor-helper.sh references (Gemini Code Assist MEDIUM suggestion) - Line 1036: replace 'launchctl unload "$old_plist"' with 'launchctl remove "$old_label"' so orphaned agents loaded without a backing plist file on disk are still unloaded correctly (Gemini Code Assist HIGH suggestion) shellcheck: 0 violations | tests: 10/10 pass --- setup-modules/migrations.sh | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/setup-modules/migrations.sh b/setup-modules/migrations.sh index 792ca5b3e1..b99888f991 100644 --- a/setup-modules/migrations.sh +++ b/setup-modules/migrations.sh @@ -1016,8 +1016,8 @@ migrate_orphaned_supervisor() { if [[ -n "$new_crontab" ]]; then printf '%s\n' "$new_crontab" | crontab - 2>/dev/null || true else - # All entries were supervisor-helper.sh — clear crontab - echo "" | crontab - 2>/dev/null || true + # All entries were supervisor-helper.sh — remove crontab entirely + crontab -r 2>/dev/null || true fi print_info "Removed orphaned supervisor-helper.sh cron entries" print_info " pulse-wrapper.sh will be installed by setup.sh if supervisor pulse is enabled" @@ -1033,7 +1033,9 @@ migrate_orphaned_supervisor() { local old_label="com.aidevops.supervisor-pulse" local old_plist="$HOME/Library/LaunchAgents/${old_label}.plist" if [[ -f "$old_plist" ]] || _launchd_has_agent "$old_label" 2>/dev/null; then - launchctl unload "$old_plist" 2>/dev/null || true + # Use launchctl remove by label — works even when the plist file is + # missing (orphaned agent loaded without a backing file on disk) + launchctl remove "$old_label" 2>/dev/null || true rm -f "$old_plist" print_info "Removed orphaned supervisor-pulse LaunchAgent ($old_label)" ((++cleaned))