Skip to content

Commit 94d5052

Browse files
GH#5147: Clean up orphaned supervisor-helper.sh and supervisor/ during update (#5151)
* fix: clean up orphaned supervisor-helper.sh and supervisor/ during update (GH#5147) 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 * fix: address PR #5151 review — crontab -r and launchctl remove by label - 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
1 parent 0d2ddce commit 94d5052

3 files changed

Lines changed: 322 additions & 0 deletions

File tree

setup-modules/migrations.sh

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -963,3 +963,88 @@ migrate_pulse_repos_to_repos_json() {
963963

964964
return 0
965965
}
966+
967+
# Migrate orphaned supervisor-helper.sh and supervisor/ modules (GH#5147)
968+
# After the supervisor-to-pulse-wrapper migration (PR #2291, PR #2475), the
969+
# upstream repo moved supervisor files to supervisor-archived/. But aidevops
970+
# update (rsync) only adds/overwrites — it doesn't delete files that no longer
971+
# exist in the source. Users who installed before the migration retain:
972+
# - ~/.aidevops/agents/scripts/supervisor-helper.sh (old entry point)
973+
# - ~/.aidevops/agents/scripts/supervisor/ (old module directory)
974+
# - cron/launchd entries invoking supervisor-helper.sh pulse
975+
# These orphaned files shadow the new pulse-wrapper.sh architecture.
976+
# This migration removes the orphaned files and rewrites scheduler entries.
977+
migrate_orphaned_supervisor() {
978+
local agents_dir="$HOME/.aidevops/agents"
979+
local scripts_dir="$agents_dir/scripts"
980+
local cleaned=0
981+
982+
# 1. Remove orphaned supervisor-helper.sh from deployed scripts
983+
# The canonical location is now supervisor-archived/supervisor-helper.sh
984+
# (shipped for reference/tests only, not as an active entry point)
985+
if [[ -f "$scripts_dir/supervisor-helper.sh" ]]; then
986+
rm -f "$scripts_dir/supervisor-helper.sh"
987+
print_info "Removed orphaned supervisor-helper.sh from deployed scripts"
988+
((++cleaned))
989+
fi
990+
991+
# 2. Remove orphaned supervisor/ module directory
992+
# The canonical location is now supervisor-archived/ (shipped by rsync)
993+
# Only remove if it's the old modules dir (contains pulse.sh, dispatch.sh, etc.)
994+
# Do NOT remove supervisor-archived/ — that's the intentional archive
995+
if [[ -d "$scripts_dir/supervisor" && ! -L "$scripts_dir/supervisor" ]]; then
996+
# Verify it's the old module directory (not something user-created)
997+
if [[ -f "$scripts_dir/supervisor/pulse.sh" ]] ||
998+
[[ -f "$scripts_dir/supervisor/dispatch.sh" ]] ||
999+
[[ -f "$scripts_dir/supervisor/_common.sh" ]]; then
1000+
rm -rf "$scripts_dir/supervisor"
1001+
print_info "Removed orphaned supervisor/ module directory from deployed scripts"
1002+
((++cleaned))
1003+
fi
1004+
fi
1005+
1006+
# 3. Migrate cron entries from supervisor-helper.sh to pulse-wrapper.sh
1007+
# Old pattern: */2 * * * * ... supervisor-helper.sh pulse ...
1008+
# New pattern: already installed by setup.sh's pulse section
1009+
# Strategy: remove old entries; setup.sh will install the new one if pulse is enabled
1010+
local current_crontab
1011+
current_crontab=$(crontab -l 2>/dev/null) || current_crontab=""
1012+
if echo "$current_crontab" | grep -qF "supervisor-helper.sh"; then
1013+
# Remove all cron lines referencing supervisor-helper.sh
1014+
local new_crontab
1015+
new_crontab=$(echo "$current_crontab" | grep -v "supervisor-helper.sh")
1016+
if [[ -n "$new_crontab" ]]; then
1017+
printf '%s\n' "$new_crontab" | crontab - 2>/dev/null || true
1018+
else
1019+
# All entries were supervisor-helper.sh — remove crontab entirely
1020+
crontab -r 2>/dev/null || true
1021+
fi
1022+
print_info "Removed orphaned supervisor-helper.sh cron entries"
1023+
print_info " pulse-wrapper.sh will be installed by setup.sh if supervisor pulse is enabled"
1024+
((++cleaned))
1025+
fi
1026+
1027+
# 4. Migrate launchd entries from old supervisor label (macOS only)
1028+
# Old label: com.aidevops.supervisor-pulse (from cron.sh/launchd.sh)
1029+
# New label: com.aidevops.aidevops-supervisor-pulse (from setup.sh)
1030+
# setup.sh already handles the new label cleanup at line ~1000, but
1031+
# the old label from cron.sh may also be present
1032+
if [[ "$(uname -s)" == "Darwin" ]]; then
1033+
local old_label="com.aidevops.supervisor-pulse"
1034+
local old_plist="$HOME/Library/LaunchAgents/${old_label}.plist"
1035+
if [[ -f "$old_plist" ]] || _launchd_has_agent "$old_label" 2>/dev/null; then
1036+
# Use launchctl remove by label — works even when the plist file is
1037+
# missing (orphaned agent loaded without a backing file on disk)
1038+
launchctl remove "$old_label" 2>/dev/null || true
1039+
rm -f "$old_plist"
1040+
print_info "Removed orphaned supervisor-pulse LaunchAgent ($old_label)"
1041+
((++cleaned))
1042+
fi
1043+
fi
1044+
1045+
if [[ $cleaned -gt 0 ]]; then
1046+
print_success "Cleaned up $cleaned orphaned supervisor artifact(s) — pulse-wrapper.sh is the active system"
1047+
fi
1048+
1049+
return 0
1050+
}

setup.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -716,6 +716,7 @@ main() {
716716
migrate_mcp_env_to_credentials
717717
migrate_pulse_repos_to_repos_json
718718
cleanup_deprecated_paths
719+
migrate_orphaned_supervisor
719720
cleanup_deprecated_mcps
720721
cleanup_stale_bun_opencode
721722
validate_opencode_config
@@ -795,6 +796,7 @@ main() {
795796
confirm_step "Migrate mcp-env.sh -> credentials.sh" && migrate_mcp_env_to_credentials
796797
confirm_step "Migrate pulse-repos.json into repos.json" && migrate_pulse_repos_to_repos_json
797798
confirm_step "Cleanup deprecated agent paths" && cleanup_deprecated_paths
799+
confirm_step "Migrate orphaned supervisor to pulse-wrapper" && migrate_orphaned_supervisor
798800
confirm_step "Cleanup deprecated MCP entries (hetzner, serper, etc.)" && cleanup_deprecated_mcps
799801
confirm_step "Cleanup stale bun opencode install" && cleanup_stale_bun_opencode
800802
confirm_step "Validate and repair OpenCode config schema" && validate_opencode_config
Lines changed: 235 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,235 @@
1+
#!/usr/bin/env bash
2+
# Tests for migrate_orphaned_supervisor() (GH#5147)
3+
# Verifies cleanup of orphaned supervisor-helper.sh and supervisor/ modules
4+
5+
set -euo pipefail
6+
7+
REPO_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
8+
MIGRATIONS_SCRIPT="$REPO_DIR/setup-modules/migrations.sh"
9+
10+
PASS_COUNT=0
11+
FAIL_COUNT=0
12+
TOTAL_COUNT=0
13+
14+
pass() {
15+
local name="$1"
16+
PASS_COUNT=$((PASS_COUNT + 1))
17+
TOTAL_COUNT=$((TOTAL_COUNT + 1))
18+
printf "\033[0;32mPASS\033[0m %s\n" "$name"
19+
return 0
20+
}
21+
22+
fail() {
23+
local name="$1"
24+
local detail="${2:-}"
25+
FAIL_COUNT=$((FAIL_COUNT + 1))
26+
TOTAL_COUNT=$((TOTAL_COUNT + 1))
27+
printf "\033[0;31mFAIL\033[0m %s\n" "$name"
28+
if [[ -n "$detail" ]]; then
29+
printf " %s\n" "$detail"
30+
fi
31+
return 0
32+
}
33+
34+
section() {
35+
echo ""
36+
echo "=== $1 ==="
37+
return 0
38+
}
39+
40+
# Create a temporary HOME to isolate tests from the real system
41+
TEST_HOME=$(mktemp -d)
42+
# shellcheck disable=SC2064
43+
trap "rm -rf '$TEST_HOME'" EXIT
44+
45+
# Override HOME for the duration of the test
46+
export HOME="$TEST_HOME"
47+
48+
# Create the deployed agents directory structure (simulating pre-migration state)
49+
AGENTS_DIR="$TEST_HOME/.aidevops/agents"
50+
SCRIPTS_DIR="$AGENTS_DIR/scripts"
51+
mkdir -p "$SCRIPTS_DIR"
52+
53+
# Stub functions that migrations.sh expects from setup.sh
54+
print_info() { echo "[INFO] $1"; }
55+
print_success() { echo "[SUCCESS] $1"; }
56+
print_warning() { echo "[WARNING] $1"; }
57+
print_error() { echo "[ERROR] $1"; }
58+
# _launchd_has_agent is defined in setup.sh; stub it for Linux tests
59+
_launchd_has_agent() { return 1; }
60+
# Stubs for other functions referenced by migrations.sh
61+
find_opencode_config() { return 1; }
62+
create_backup_with_rotation() { return 0; }
63+
should_overwrite_user_file() { return 0; }
64+
resolve_mcp_binary_path() {
65+
echo ""
66+
return 1
67+
}
68+
update_mcp_paths_in_opencode() { return 0; }
69+
sanitize_plugin_namespace() {
70+
echo "$1"
71+
return 0
72+
}
73+
# Export stubs so sourced script can find them
74+
export -f print_info print_success print_warning print_error _launchd_has_agent
75+
export -f find_opencode_config create_backup_with_rotation should_overwrite_user_file
76+
export -f resolve_mcp_binary_path update_mcp_paths_in_opencode sanitize_plugin_namespace
77+
78+
# Source the migrations script to get the function
79+
# shellcheck disable=SC1090
80+
source "$MIGRATIONS_SCRIPT"
81+
82+
# ============================================================================
83+
section "Test: removes orphaned supervisor-helper.sh"
84+
# ============================================================================
85+
86+
# Create orphaned file
87+
echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor-helper.sh"
88+
89+
if [[ -f "$SCRIPTS_DIR/supervisor-helper.sh" ]]; then
90+
pass "Setup: orphaned supervisor-helper.sh exists before migration"
91+
else
92+
fail "Setup: orphaned supervisor-helper.sh should exist"
93+
fi
94+
95+
migrate_orphaned_supervisor >/dev/null 2>&1
96+
97+
if [[ ! -f "$SCRIPTS_DIR/supervisor-helper.sh" ]]; then
98+
pass "supervisor-helper.sh removed after migration"
99+
else
100+
fail "supervisor-helper.sh should have been removed"
101+
fi
102+
103+
# ============================================================================
104+
section "Test: removes orphaned supervisor/ module directory"
105+
# ============================================================================
106+
107+
# Create orphaned module directory with recognizable files
108+
mkdir -p "$SCRIPTS_DIR/supervisor"
109+
echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/pulse.sh"
110+
echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/dispatch.sh"
111+
echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor/_common.sh"
112+
113+
if [[ -d "$SCRIPTS_DIR/supervisor" ]]; then
114+
pass "Setup: orphaned supervisor/ directory exists before migration"
115+
else
116+
fail "Setup: orphaned supervisor/ directory should exist"
117+
fi
118+
119+
migrate_orphaned_supervisor >/dev/null 2>&1
120+
121+
if [[ ! -d "$SCRIPTS_DIR/supervisor" ]]; then
122+
pass "supervisor/ directory removed after migration"
123+
else
124+
fail "supervisor/ directory should have been removed"
125+
fi
126+
127+
# ============================================================================
128+
section "Test: does NOT remove supervisor-archived/"
129+
# ============================================================================
130+
131+
mkdir -p "$SCRIPTS_DIR/supervisor-archived"
132+
echo '#!/usr/bin/env bash' >"$SCRIPTS_DIR/supervisor-archived/supervisor-helper.sh"
133+
134+
migrate_orphaned_supervisor >/dev/null 2>&1
135+
136+
if [[ -d "$SCRIPTS_DIR/supervisor-archived" ]]; then
137+
pass "supervisor-archived/ preserved after migration"
138+
else
139+
fail "supervisor-archived/ should NOT have been removed"
140+
fi
141+
142+
# ============================================================================
143+
section "Test: does NOT remove unrelated supervisor/ directory"
144+
# ============================================================================
145+
146+
# Create a supervisor/ directory without the expected module files
147+
mkdir -p "$SCRIPTS_DIR/supervisor"
148+
echo 'custom content' >"$SCRIPTS_DIR/supervisor/my-custom-file.txt"
149+
150+
migrate_orphaned_supervisor >/dev/null 2>&1
151+
152+
if [[ -d "$SCRIPTS_DIR/supervisor" ]]; then
153+
pass "Unrelated supervisor/ directory preserved (no module files)"
154+
else
155+
fail "Unrelated supervisor/ directory should NOT have been removed"
156+
fi
157+
158+
# Clean up for next test
159+
rm -rf "$SCRIPTS_DIR/supervisor"
160+
161+
# ============================================================================
162+
section "Test: idempotent — no errors when nothing to clean"
163+
# ============================================================================
164+
165+
# Run migration when there's nothing to clean
166+
output=$(migrate_orphaned_supervisor 2>&1)
167+
exit_code=$?
168+
169+
if [[ $exit_code -eq 0 ]]; then
170+
pass "Migration succeeds when nothing to clean (exit 0)"
171+
else
172+
fail "Migration should succeed when nothing to clean" "exit code: $exit_code"
173+
fi
174+
175+
# Should NOT print the success message when nothing was cleaned
176+
if [[ "$output" != *"Cleaned up"* ]]; then
177+
pass "No 'Cleaned up' message when nothing to clean"
178+
else
179+
fail "Should not print 'Cleaned up' when nothing was cleaned" "$output"
180+
fi
181+
182+
# ============================================================================
183+
section "Test: removes cron entries referencing supervisor-helper.sh"
184+
# ============================================================================
185+
186+
# Install a fake cron entry (only if crontab is available)
187+
if command -v crontab &>/dev/null; then
188+
# Save current crontab
189+
original_crontab=$(crontab -l 2>/dev/null) || original_crontab=""
190+
191+
# Install test crontab with supervisor-helper.sh entry
192+
{
193+
echo "# aidevops: test-keep-this"
194+
echo "*/5 * * * * echo keep-this # aidevops: test-keep"
195+
echo "*/2 * * * * /bin/bash /home/test/.aidevops/agents/scripts/supervisor-helper.sh pulse >> /tmp/test.log 2>&1 # aidevops: supervisor-pulse"
196+
} | crontab - 2>/dev/null
197+
198+
migrate_orphaned_supervisor >/dev/null 2>&1
199+
200+
new_crontab=$(crontab -l 2>/dev/null) || new_crontab=""
201+
202+
if echo "$new_crontab" | grep -qF "supervisor-helper.sh"; then
203+
fail "Cron entry referencing supervisor-helper.sh should have been removed"
204+
else
205+
pass "Cron entry referencing supervisor-helper.sh removed"
206+
fi
207+
208+
if echo "$new_crontab" | grep -qF "keep-this"; then
209+
pass "Non-supervisor cron entries preserved"
210+
else
211+
fail "Non-supervisor cron entries should have been preserved"
212+
fi
213+
214+
# Restore original crontab
215+
if [[ -n "$original_crontab" ]]; then
216+
printf '%s\n' "$original_crontab" | crontab - 2>/dev/null || true
217+
else
218+
crontab -r 2>/dev/null || true
219+
fi
220+
else
221+
pass "Cron test skipped (crontab not available)"
222+
fi
223+
224+
# ============================================================================
225+
# Summary
226+
# ============================================================================
227+
echo ""
228+
echo "================================"
229+
echo "Results: $PASS_COUNT passed, $FAIL_COUNT failed, $TOTAL_COUNT total"
230+
echo "================================"
231+
232+
if [[ $FAIL_COUNT -gt 0 ]]; then
233+
exit 1
234+
fi
235+
exit 0

0 commit comments

Comments
 (0)