From f51dedd4183992438710221576ad533213fe2194 Mon Sep 17 00:00:00 2001 From: Darren Ehlers Date: Sun, 17 May 2026 19:58:15 -0500 Subject: [PATCH 1/2] =?UTF-8?q?Fix:=20XACA-0512=20=E2=80=94=20Rework=20mig?= =?UTF-8?q?rate.sh=20update=5Flaunchagents=20to=20render=20from=20template?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aiteamforge-migrate.sh::update_launchagents was doing in-place `sed -i.bak` / `sed -i.bak2` path-rewrites on whatever plist already existed in ~/Library/LaunchAgents — never rendering from canonical templates. Drifted or hand-edited plists weren't recoverable, and the hack left .bak/.bak2 droppings behind. This rework brings migrate.sh in line with install-kanban.sh and the XACA-0510 fix in upgrade.sh: render *.template files to *.new tempfiles, diff against the live target, atomic mv + launchctl reload only on change. Tempfiles cleaned up on all no-op / DRY_RUN / interrupt paths. Three-agent dispatch with per-template-family substitution maps — kanban templates (USER_HOME, AITEAMFORGE_DIR, BACKUP_INTERVAL, PYTHON3_PATH) and fleet-monitor template (NODE_PATH, FLEET_SERVER_PATH, LOG_DIR, HOMEBREW_PREFIX, HOME_DIR, FLEET_PORT, AITEAMFORGE_DIR) have incompatible placeholder sets and can't share a single sed expression. This is the migrate-specific wrinkle that doesn't apply to upgrade.sh. Migrate-specific semantic: {{AITEAMFORGE_DIR}} resolves to NEW_DATA_DIR (the migration's destination), not WORKING_DIR. That's what makes the rewritten plist correct after a migration completes. LAUNCHAGENTS_DIR seam preserved so tests inject sandbox paths without touching ~/Library/LaunchAgents (M3Pro install-ban rule). Defaults mirror install-kanban.sh:16 and install-fleet-monitor.sh:15 with sibling-drift consolidation hooks (XACA-0516). 16 test cases in tests/test-xaca-0512-migrate-launchagent-render.sh, all pass. Explicit regression assertion for the old .bak/.bak2 behavior. shellcheck clean. Sibling-heuristic drift, third datapoint in this surface: XACA-0476 (missing share/ prefix) XACA-0510 (no template render in upgrade.sh) XACA-0512 (no template render in migrate.sh) All three launchagent-render sites are now consistent. aiteamforge-migrate-check.sh has the same EXPECTED_AGENTS list but is read-only (presence + launchctl list check, no render logic) — audited, no change required. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CHANGELOG.md | 16 + libexec/commands/aiteamforge-migrate.sh | 171 +++++++-- ...st-xaca-0512-migrate-launchagent-render.sh | 345 ++++++++++++++++++ 3 files changed, 505 insertions(+), 27 deletions(-) create mode 100755 tests/test-xaca-0512-migrate-launchagent-render.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index f32d03e..8065026 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,22 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) ## [Unreleased] +### Fix: XACA-0512 — Rework `aiteamforge-migrate.sh::update_launchagents` to render from templates + +- **`libexec/commands/aiteamforge-migrate.sh`** — Replaced the in-place `sed -i.bak` / `sed -i.bak2` path-rewrite loop in `update_launchagents()` with a render-from-template flow that mirrors `install-kanban.sh` (and the XACA-0510 fix in `upgrade.sh`). The old code never rendered from canonical templates — it patched whatever plist already existed at `~/Library/LaunchAgents/`, leaving `.bak`/`.bak2` files behind and silently failing to recover from any drift or hand-editing of the on-disk plist. +- **Three-agent, per-template-family dispatch.** The agent set spans two template directories with incompatible substitution maps, so a single `sed` expression can't serve all three (the XACA-0510 wrinkle that doesn't apply to `upgrade.sh`): + - `com.aiteamforge.kanban-backup.plist` → `share/templates/kanban/backup-plist.template` via `_render_kanban_template()` — substitutes `{{USER_HOME}}`, `{{AITEAMFORGE_DIR}}`, `{{BACKUP_INTERVAL}}`, `{{PYTHON3_PATH}}`. + - `com.aiteamforge.lcars-health.plist` → `share/templates/kanban/lcars-health-plist.template` via the same kanban renderer (harmless extras prevent sibling-drift bugs). + - `com.aiteamforge.fleet-monitor.plist` → `share/templates/fleet-monitor/fleet-launchagent.template.plist` via `_render_fleet_template()` — substitutes `{{NODE_PATH}}`, `{{FLEET_SERVER_PATH}}`, `{{LOG_DIR}}`, `{{HOMEBREW_PREFIX}}`, `{{HOME_DIR}}`, `{{FLEET_PORT}}`, `{{AITEAMFORGE_DIR}}`. +- **Migrate-specific semantic:** `{{AITEAMFORGE_DIR}}` resolves to `${NEW_DATA_DIR}` (the migration's destination), not `${WORKING_DIR}` as in `upgrade.sh`. This is what makes the rendered plist correct after a migration — the rewritten plist now points to the migrated data location. +- **Defaults consolidated for sibling-drift consolidation (XACA-0516):** module-level `readonly XACA_0512_KANBAN_BACKUP_INTERVAL_DEFAULT=900` (mirrors `install-kanban.sh:16`) and `XACA_0512_FLEET_MONITOR_PORT_DEFAULT=3000` (mirrors `install-fleet-monitor.sh:15`). Honors `KANBAN_BACKUP_INTERVAL` / `FLEET_MONITOR_PORT` env overrides at render time. +- **Semantics preserved:** `FORCE=true` re-renders even when target is up to date; `DRY_RUN=true` writes nothing and prints `[DRY RUN] Would update`; `launchctl unload` runs before `mv`, `launchctl load` runs after (both tolerate failure via `2>/dev/null || true`); agents absent from `~/Library/LaunchAgents/` are skipped — migrate must not silently materialise agents the user opted out of at install time. +- **`LAUNCHAGENTS_DIR` seam** — function respects `LAUNCHAGENTS_DIR` env var (defaults to `$HOME/Library/LaunchAgents`) so tests can inject a sandbox path without touching the real user LaunchAgents dir (M3Pro tap-install ban). Same seam pattern as XACA-0510. +- **`_cleanup_migrate_tmpfiles()` + `RETURN` trap** — any `*.new` tempfile created during the render loop is cleaned up on early return or interrupt. No `*.new` leakage on no-op, DRY_RUN, or missing-template paths. +- **`tests/test-xaca-0512-migrate-launchagent-render.sh`** — 16 test cases covering: all three targets absent → all skipped; explicit `.bak`/`.bak2` regression assertion (the old in-place `sed -i.bak` behavior must be gone); kanban + fleet-monitor render with no `{{…}}` placeholders; rendered kanban plist contains resolved `NEW_DATA_DIR` (not `WORKING_DIR`); fleet-monitor renders all seven fleet-specific placeholders and resolves the `FLEET_PORT` default; selective opt-in (kanban present, fleet absent → fleet not materialised); second run no-op (mtime unchanged, no tempfile leak); `FORCE=true` re-renders; `DRY_RUN=true` preserves sentinel content with no tempfile leak; missing template warns without crashing; all-three-present rendered cleanly; DRY_RUN + no-change reports "All LaunchAgents up to date". +- **Audit-only on `aiteamforge-migrate-check.sh`** — that script's `analyze_launchagents()` (line 381) has the same `EXPECTED_AGENTS` list but is read-only (presence + `launchctl list` check, no render logic). No change required; documented for completeness. +- **Sibling-heuristic drift, third datapoint:** XACA-0476 (missing `share/` prefix) → XACA-0510 (no template render in `upgrade.sh`) → XACA-0512 (no template render in `migrate.sh`). All three sites in the launchagent-render surface are now consistent. + ### Fix: XACA-0510 — Rework `update_launchagents` to render from templates - **`libexec/commands/aiteamforge-upgrade.sh`** — Added `_render_launchagent_template()` private helper that applies the full `sed` substitution map (`USER_HOME`, `AITEAMFORGE_DIR`, `BACKUP_INTERVAL`, `PYTHON3_PATH`) to any plist template. All four substitutions are applied to every template — harmless extras prevent sibling-drift bugs (per the pattern catalogued in XACA-0501). diff --git a/libexec/commands/aiteamforge-migrate.sh b/libexec/commands/aiteamforge-migrate.sh index 15c5596..b20c9e4 100755 --- a/libexec/commands/aiteamforge-migrate.sh +++ b/libexec/commands/aiteamforge-migrate.sh @@ -48,6 +48,12 @@ ROLLBACK_FROM="" MIGRATION_STATE_FILE="${HOME}/.aiteamforge/migration-state.json" MIGRATION_LOG="${HOME}/.aiteamforge/migration.log" +# NOTE: Defaults mirror install-kanban.sh:16 (KANBAN_BACKUP_INTERVAL) and +# install-fleet-monitor.sh:15 (FLEET_MONITOR_PORT). If you change one, +# change the other. Sibling-drift consolidation tracked in XACA-0516. +readonly XACA_0512_KANBAN_BACKUP_INTERVAL_DEFAULT=900 +readonly XACA_0512_FLEET_MONITOR_PORT_DEFAULT=3000 + # Usage usage() { cat < +# Returns: 0 on success, 1 if template not found. +_render_kanban_template() { + local template="$1" + local dest="$2" + + if [ ! -f "$template" ]; then + warning "LaunchAgent template not found: $template" + return 1 + fi + + local kanban_backup_interval="${KANBAN_BACKUP_INTERVAL:-$XACA_0512_KANBAN_BACKUP_INTERVAL_DEFAULT}" + local python3_path + # NOTE: PYTHON3_PATH is re-resolved on every migrate — PATH changes between + # original install and migrate will re-pin the plist interpreter. See XACA-0510. + python3_path="$(command -v python3 2>/dev/null || echo "/usr/bin/python3")" + + sed -e "s|{{USER_HOME}}|$HOME|g" \ + -e "s|{{AITEAMFORGE_DIR}}|${NEW_DATA_DIR}|g" \ + -e "s|{{BACKUP_INTERVAL}}|${kanban_backup_interval}|g" \ + -e "s|{{PYTHON3_PATH}}|${python3_path}|g" \ + "$template" > "$dest" +} + +# Render the fleet-monitor LaunchAgent template to a tempfile. +# Substitution map mirrors install-fleet-monitor.sh:569-577. +# Usage: _render_fleet_template +# Returns: 0 on success, 1 if template not found or node missing. +_render_fleet_template() { + local template="$1" + local dest="$2" + + if [ ! -f "$template" ]; then + warning "LaunchAgent template not found: $template" + return 1 + fi + + local node_path + if command -v node >/dev/null 2>&1; then + node_path="$(command -v node)" + elif [ -x "/opt/homebrew/bin/node" ]; then + node_path="/opt/homebrew/bin/node" + else + warning "Node.js not found — skipping fleet-monitor LaunchAgent render" + return 1 + fi + + local fleet_port="${FLEET_MONITOR_PORT:-$XACA_0512_FLEET_MONITOR_PORT_DEFAULT}" + local homebrew_prefix + homebrew_prefix="$(brew --prefix 2>/dev/null || echo '/opt/homebrew')" + + sed -e "s|{{NODE_PATH}}|${node_path}|g" \ + -e "s|{{FLEET_SERVER_PATH}}|${NEW_DATA_DIR}/fleet-monitor/server|g" \ + -e "s|{{LOG_DIR}}|${NEW_DATA_DIR}/logs|g" \ + -e "s|{{HOMEBREW_PREFIX}}|${homebrew_prefix}|g" \ + -e "s|{{HOME_DIR}}|$HOME|g" \ + -e "s|{{FLEET_PORT}}|${fleet_port}|g" \ + -e "s|{{AITEAMFORGE_DIR}}|${NEW_DATA_DIR}|g" \ + "$template" > "$dest" +} + update_launchagents() { section "Updating LaunchAgents" - LAUNCHAGENT_DIR="${HOME}/Library/LaunchAgents" - AGENTS_UPDATED=0 + # Resolve framework dir (where templates ship) lazily — get_framework_dir + # is provided by lib/config.sh, sourced at script load. + local framework_dir="${FRAMEWORK_DIR:-$(get_framework_dir)}" + + # Allow tests to inject a sandbox path instead of the real LaunchAgents dir. + local launchagents_dir="${LAUNCHAGENTS_DIR:-$HOME/Library/LaunchAgents}" - EXPECTED_AGENTS=( - "com.aiteamforge.kanban-backup.plist" - "com.aiteamforge.lcars-health.plist" - "com.aiteamforge.fleet-monitor.plist" + # Pairs of "plist-filename|render-fn|template-relpath". The render function + # selects which substitution map to apply — kanban vs fleet-monitor have + # different placeholder sets and cannot share a single sed expression. + local agents=( + "com.aiteamforge.kanban-backup.plist|_render_kanban_template|share/templates/kanban/backup-plist.template" + "com.aiteamforge.lcars-health.plist|_render_kanban_template|share/templates/kanban/lcars-health-plist.template" + "com.aiteamforge.fleet-monitor.plist|_render_fleet_template|share/templates/fleet-monitor/fleet-launchagent.template.plist" ) - for agent in "${EXPECTED_AGENTS[@]}"; do - AGENT_PATH="${LAUNCHAGENT_DIR}/${agent}" + local agents_updated=0 + # Reset module-level tempfile tracker; RETURN trap cleans up any survivors. + _migrate_tmpfiles=() + trap '_cleanup_migrate_tmpfiles' RETURN + + for entry in "${agents[@]}"; do + local agent="${entry%%|*}" + local rest="${entry#*|}" + local render_fn="${rest%%|*}" + local tmpl_relpath="${rest#*|}" + local template="${framework_dir}/${tmpl_relpath}" + local target="${launchagents_dir}/${agent}" + local tmpfile="${target}.new" + _migrate_tmpfiles+=("$tmpfile") + + # Opt-in guard: only re-render agents the user already has installed. + # Migrate must not silently materialise agents that weren't there. + if [[ ! -f "${target}" ]]; then + continue + fi + + log "Rendering ${agent} from template..." - if [[ -f "${AGENT_PATH}" ]]; then - log "Updating ${agent}..." + # Dispatch to the per-agent render function. Failures (missing template, + # missing node, etc.) are reported by the render fn and we move on. + if ! "$render_fn" "$template" "$tmpfile"; then + continue + fi + # Diff rendered output against live target. If unchanged, no-op. + if ! diff -q "$tmpfile" "$target" >/dev/null 2>&1 || [[ "${FORCE}" == "true" ]]; then if [[ "${DRY_RUN}" == "true" ]]; then - echo "[DRY RUN] Would update paths in: ${agent}" + echo "[DRY RUN] Would update: ${agent}" + rm -f "$tmpfile" + agents_updated=$((agents_updated + 1)) continue fi - # Unload first - launchctl unload "${AGENT_PATH}" 2>/dev/null || true - - # Update paths in plist - sed -i.bak "s|${HOME}/aiteamforge|${NEW_DATA_DIR}|g" "${AGENT_PATH}" + # Unload current agent (ignore failure — may not be loaded) + launchctl unload "${target}" 2>/dev/null || true - # Also update to use new framework location - sed -i.bak2 "s|${HOME}/aiteamforge/|/opt/homebrew/opt/aiteamforge/libexec/|g" "${AGENT_PATH}" + # Atomically replace with rendered version + mv "$tmpfile" "${target}" - # Reload - launchctl load "${AGENT_PATH}" 2>/dev/null || true + # Reload agent (ignore failure — may need user session context) + launchctl load "${target}" 2>/dev/null || true - AGENTS_UPDATED=$((AGENTS_UPDATED + 1)) + success "Updated ${agent}" + agents_updated=$((agents_updated + 1)) + else + # No change — clean up tempfile, count as no-op + rm -f "$tmpfile" fi done - if [[ "${DRY_RUN}" != "true" ]]; then - if [[ "${AGENTS_UPDATED}" -gt 0 ]]; then - success "LaunchAgents updated: ${AGENTS_UPDATED}" - else - info "No LaunchAgents to update" - fi + if [[ "${agents_updated}" -eq 0 ]]; then + success "All LaunchAgents up to date" + elif [[ "${DRY_RUN}" == "true" ]]; then + success "Would update ${agents_updated} LaunchAgent(s)" + else + success "LaunchAgents updated: ${agents_updated}" fi echo "" diff --git a/tests/test-xaca-0512-migrate-launchagent-render.sh b/tests/test-xaca-0512-migrate-launchagent-render.sh new file mode 100755 index 0000000..d5a65ea --- /dev/null +++ b/tests/test-xaca-0512-migrate-launchagent-render.sh @@ -0,0 +1,345 @@ +#!/bin/bash + +# test-xaca-0512-migrate-launchagent-render.sh +# Tests for aiteamforge-migrate.sh::update_launchagents() render-from-template +# logic (XACA-0512). Sibling of test-xaca-0510-launchagent-render.sh — covers +# the three-agent case with per-template-family dispatch (kanban vs fleet-monitor). +# +# All filesystem activity is sandboxed to $TEST_TMP_DIR. +# NEVER touches ~/Library/LaunchAgents — see M3Pro install-ban rule. + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +TAP_ROOT="$(cd "$SCRIPT_DIR/.." && pwd)" +MIGRATE_SH="$TAP_ROOT/libexec/commands/aiteamforge-migrate.sh" +KANBAN_TEMPLATES_DIR="$TAP_ROOT/share/templates/kanban" +FLEET_TEMPLATES_DIR="$TAP_ROOT/share/templates/fleet-monitor" + +# ═══════════════════════════════════════════════════════════════════════════ +# Sandbox setup +# ═══════════════════════════════════════════════════════════════════════════ + +SANDBOX_DIR="$TEST_TMP_DIR/xaca0512" +FAKE_LAUNCHAGENTS="$SANDBOX_DIR/LaunchAgents" +FAKE_FRAMEWORK="$SANDBOX_DIR/framework" +FAKE_NEW_DATA="$SANDBOX_DIR/.aiteamforge" + +mkdir -p "$FAKE_LAUNCHAGENTS" +mkdir -p "$FAKE_FRAMEWORK/share/templates/kanban" +mkdir -p "$FAKE_FRAMEWORK/share/templates/fleet-monitor" +mkdir -p "$FAKE_NEW_DATA" + +# Copy real templates into sandbox framework +cp "$KANBAN_TEMPLATES_DIR/backup-plist.template" "$FAKE_FRAMEWORK/share/templates/kanban/" +cp "$KANBAN_TEMPLATES_DIR/lcars-health-plist.template" "$FAKE_FRAMEWORK/share/templates/kanban/" +cp "$FLEET_TEMPLATES_DIR/fleet-launchagent.template.plist" "$FAKE_FRAMEWORK/share/templates/fleet-monitor/" + +# ═══════════════════════════════════════════════════════════════════════════ +# Load only the helper functions from migrate.sh. The script main body has +# side effects (sets up MIGRATION_LOG redirects, parses argv) so we extract +# the four functions we care about instead of sourcing. +# ═══════════════════════════════════════════════════════════════════════════ + +# Stub print/log helpers if test-runner.sh hasn't defined them. +if ! declare -f section >/dev/null 2>&1; then section() { :; }; fi +if ! declare -f info >/dev/null 2>&1; then info() { echo "$@"; }; fi +if ! declare -f success >/dev/null 2>&1; then success() { echo "$@"; }; fi +if ! declare -f warning >/dev/null 2>&1; then warning() { echo "$@" >&2; }; fi +if ! declare -f log >/dev/null 2>&1; then log() { echo "$@"; }; fi + +# Stub get_framework_dir — overridden per-call via FRAMEWORK_DIR env injection, +# but the function needs to *exist* for the unsourced extraction to evaluate. +get_framework_dir() { echo "$FAKE_FRAMEWORK"; } + +# No-op launchctl stub — CRITICAL: must never call the real launchctl here. +launchctl() { return 0; } +export -f launchctl + +# No-op brew stub so _render_fleet_template doesn't depend on host brew prefix. +brew() { + if [ "$1" = "--prefix" ]; then echo "/opt/homebrew"; return 0; fi + return 0 +} +export -f brew + +# Extract from the cleanup helper comment through update_launchagents (4 funcs, +# 4 closing top-level braces). +_extracted_funcs="$(awk ' + /^# Cleanup helper invoked by update_launchagents/ { capture=1 } + capture { print } + /^}$/ && capture { + brace_count++ + if (brace_count == 4) { capture=0 } + } +' "$MIGRATE_SH")" + +if [ -z "$_extracted_funcs" ]; then + test_start "Sanity: can extract functions from migrate.sh" + test_fail "Failed to extract helpers + update_launchagents from migrate.sh — awk returned empty" + return 0 +fi + +# Defaults that the extracted functions reference (normally set at script top). +# shellcheck disable=SC2034 # referenced by eval'd functions; shellcheck can't trace eval +readonly XACA_0512_KANBAN_BACKUP_INTERVAL_DEFAULT=900 +# shellcheck disable=SC2034 # referenced by eval'd functions; shellcheck can't trace eval +readonly XACA_0512_FLEET_MONITOR_PORT_DEFAULT=3000 + +eval "$_extracted_funcs" + +# Hard precondition: abort loudly if extraction was incomplete. +declare -f _cleanup_migrate_tmpfiles >/dev/null || \ + { echo "FATAL: _cleanup_migrate_tmpfiles not extracted from migrate.sh"; exit 1; } +declare -f _render_kanban_template >/dev/null || \ + { echo "FATAL: _render_kanban_template not extracted from migrate.sh"; exit 1; } +declare -f _render_fleet_template >/dev/null || \ + { echo "FATAL: _render_fleet_template not extracted from migrate.sh"; exit 1; } +declare -f update_launchagents >/dev/null || \ + { echo "FATAL: update_launchagents not extracted from migrate.sh"; exit 1; } + +# ═══════════════════════════════════════════════════════════════════════════ +# Helper: run update_launchagents with our sandbox variables injected +# ═══════════════════════════════════════════════════════════════════════════ + +# shellcheck disable=SC2120,SC2119 +run_update_launchagents() { + FRAMEWORK_DIR="$FAKE_FRAMEWORK" \ + NEW_DATA_DIR="$FAKE_NEW_DATA" \ + LAUNCHAGENTS_DIR="$FAKE_LAUNCHAGENTS" \ + HOME="$SANDBOX_DIR/home" \ + KANBAN_BACKUP_INTERVAL="900" \ + FLEET_MONITOR_PORT="3000" \ + update_launchagents 2>&1 +} + +reset_launchagents() { + rm -rf "$FAKE_LAUNCHAGENTS" + mkdir -p "$FAKE_LAUNCHAGENTS" +} + +BACKUP_PLIST="$FAKE_LAUNCHAGENTS/com.aiteamforge.kanban-backup.plist" +LCARS_PLIST="$FAKE_LAUNCHAGENTS/com.aiteamforge.lcars-health.plist" +FLEET_PLIST="$FAKE_LAUNCHAGENTS/com.aiteamforge.fleet-monitor.plist" + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 1: Missing targets — all three absent → all three skipped (opt-in guard) +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Missing targets: all three plists absent → function skips all" +reset_launchagents +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +assert_file_not_exists "$BACKUP_PLIST" "kanban-backup.plist must not be created when absent" +assert_file_not_exists "$LCARS_PLIST" "lcars-health.plist must not be created when absent" +assert_file_not_exists "$FLEET_PLIST" "fleet-monitor.plist must not be created when absent" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 2: In-place sed regression — old code did `sed -i.bak`, creating .bak +# files. Render-from-template flow must NOT leave .bak or .bak2 droppings. +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Regression: no .bak/.bak2 files left behind (old sed -i.bak behavior gone)" +reset_launchagents +touch "$BACKUP_PLIST" +touch "$LCARS_PLIST" +touch "$FLEET_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +assert_file_not_exists "${BACKUP_PLIST}.bak" "kanban-backup.plist.bak must not exist (sed -i.bak removed)" +assert_file_not_exists "${BACKUP_PLIST}.bak2" "kanban-backup.plist.bak2 must not exist (sed -i.bak2 removed)" +assert_file_not_exists "${FLEET_PLIST}.bak" "fleet-monitor.plist.bak must not exist (sed -i.bak removed)" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 3: Fresh render — kanban targets present → rendered, no placeholders +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Fresh render: kanban plists rendered with no {{...}} placeholders" +reset_launchagents +touch "$BACKUP_PLIST" +touch "$LCARS_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +backup_content="$(cat "$BACKUP_PLIST")" +lcars_content="$(cat "$LCARS_PLIST")" +assert_not_contains "$backup_content" "{{" "kanban-backup.plist must contain no {{ placeholders" +assert_not_contains "$lcars_content" "{{" "lcars-health.plist must contain no {{ placeholders" +test_pass + +test_start "Fresh render: kanban plists contain resolved NEW_DATA_DIR path" +reset_launchagents +touch "$BACKUP_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +backup_content="$(cat "$BACKUP_PLIST")" +assert_contains "$backup_content" "$FAKE_NEW_DATA" "kanban-backup.plist must contain resolved NEW_DATA_DIR (migrate uses NEW_DATA_DIR, not WORKING_DIR)" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 4: Fleet-monitor template (the migrate-specific wrinkle) — separate +# template dir, different substitution map. +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Fleet-monitor: target present → rendered from fleet-monitor template dir" +reset_launchagents +touch "$FLEET_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +assert_file_exists "$FLEET_PLIST" "fleet-monitor.plist must exist after render" +fleet_content="$(cat "$FLEET_PLIST")" +assert_not_contains "$fleet_content" "{{" "fleet-monitor.plist must contain no {{ placeholders" +test_pass + +test_start "Fleet-monitor: rendered plist contains all fleet-specific substitutions" +reset_launchagents +touch "$FLEET_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +fleet_content="$(cat "$FLEET_PLIST")" +assert_not_contains "$fleet_content" "{{NODE_PATH}}" "{{NODE_PATH}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{FLEET_SERVER_PATH}}" "{{FLEET_SERVER_PATH}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{LOG_DIR}}" "{{LOG_DIR}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{HOMEBREW_PREFIX}}" "{{HOMEBREW_PREFIX}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{HOME_DIR}}" "{{HOME_DIR}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{FLEET_PORT}}" "{{FLEET_PORT}} placeholder must be substituted" +assert_not_contains "$fleet_content" "{{AITEAMFORGE_DIR}}" "{{AITEAMFORGE_DIR}} placeholder must be substituted" +test_pass + +test_start "Fleet-monitor: rendered plist contains resolved FLEET_PORT default (3000)" +reset_launchagents +touch "$FLEET_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +fleet_content="$(cat "$FLEET_PLIST")" +assert_contains "$fleet_content" "3000" "fleet-monitor.plist must contain resolved FLEET_PORT value" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 5: Selective opt-in — kanban present, fleet absent → only kanban rendered +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Selective opt-in: kanban-backup present, fleet absent → fleet not materialised" +reset_launchagents +touch "$BACKUP_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +assert_file_exists "$BACKUP_PLIST" "kanban-backup.plist must be rendered (was present)" +assert_file_not_exists "$LCARS_PLIST" "lcars-health.plist must not be created (was absent)" +assert_file_not_exists "$FLEET_PLIST" "fleet-monitor.plist must not be created (was absent)" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 6: No-op on second run — same template → unchanged file +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "No-op: second run with same template leaves file mtime unchanged" +reset_launchagents +touch "$BACKUP_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +mtime_first="$(stat -f '%m' "$BACKUP_PLIST" 2>/dev/null || stat -c '%Y' "$BACKUP_PLIST" 2>/dev/null)" +sleep 1 +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +mtime_second="$(stat -f '%m' "$BACKUP_PLIST" 2>/dev/null || stat -c '%Y' "$BACKUP_PLIST" 2>/dev/null)" +assert_equal "$mtime_first" "$mtime_second" "File mtime must be unchanged on no-op second run" +assert_file_not_exists "${BACKUP_PLIST}.new" "No .new tempfile must remain after no-op run" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 7: FORCE=true → re-renders even when target is up to date +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "FORCE=true: re-renders plist even when target is up to date" +reset_launchagents +touch "$BACKUP_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +mtime_first="$(stat -f '%m' "$BACKUP_PLIST" 2>/dev/null || stat -c '%Y' "$BACKUP_PLIST" 2>/dev/null)" +sleep 1 +FORCE=true DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +mtime_force="$(stat -f '%m' "$BACKUP_PLIST" 2>/dev/null || stat -c '%Y' "$BACKUP_PLIST" 2>/dev/null)" +assert_not_equal "$mtime_first" "$mtime_force" "File mtime must change after FORCE=true run" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 8: DRY_RUN=true → writes nothing to target +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "DRY_RUN=true: writes nothing to target plist" +reset_launchagents +echo "SENTINEL_DO_NOT_OVERWRITE" > "$BACKUP_PLIST" +FORCE=false DRY_RUN=true run_update_launchagents >/dev/null 2>&1 +content="$(cat "$BACKUP_PLIST")" +assert_equal "SENTINEL_DO_NOT_OVERWRITE" "$content" "DRY_RUN must not overwrite target plist" +test_pass + +test_start "DRY_RUN=true: prints '[DRY RUN] Would update' for changed agent" +reset_launchagents +echo "SENTINEL_DO_NOT_OVERWRITE" > "$BACKUP_PLIST" +output="$(FORCE=false DRY_RUN=true run_update_launchagents 2>&1)" +assert_contains "$output" "Would update" "DRY_RUN must print 'Would update' for changed agent" +test_pass + +test_start "DRY_RUN=true: no .new tempfile leaked after run" +reset_launchagents +echo "SENTINEL_DO_NOT_OVERWRITE" > "$BACKUP_PLIST" +FORCE=false DRY_RUN=true run_update_launchagents >/dev/null 2>&1 +assert_file_not_exists "${BACKUP_PLIST}.new" "No .new tempfile must remain after DRY_RUN run" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 9: Missing template → reports cleanly without aborting +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "Missing template: function reports warning and continues without crash" +reset_launchagents +touch "$BACKUP_PLIST" +touch "$LCARS_PLIST" + +MISSING_FRAMEWORK="$SANDBOX_DIR/framework-missing" +mkdir -p "$MISSING_FRAMEWORK/share/templates/kanban" +mkdir -p "$MISSING_FRAMEWORK/share/templates/fleet-monitor" +# Only copy lcars-health template; leave backup-plist.template absent +cp "$KANBAN_TEMPLATES_DIR/lcars-health-plist.template" "$MISSING_FRAMEWORK/share/templates/kanban/" + +set +e +output="$(FRAMEWORK_DIR="$MISSING_FRAMEWORK" \ + NEW_DATA_DIR="$FAKE_NEW_DATA" \ + LAUNCHAGENTS_DIR="$FAKE_LAUNCHAGENTS" \ + HOME="$SANDBOX_DIR/home" \ + KANBAN_BACKUP_INTERVAL="900" \ + FLEET_MONITOR_PORT="3000" \ + FORCE=false DRY_RUN=false \ + update_launchagents 2>&1)" +exit_code=$? +set -e + +assert_exit_success "$exit_code" "update_launchagents must not crash when a template is missing" +assert_contains "$output" "not found" "Missing template must produce a 'not found' warning" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 10: All three sentinels present → all three rendered cleanly +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "All three sentinels: all plists rendered with no placeholders" +reset_launchagents +touch "$BACKUP_PLIST" +touch "$LCARS_PLIST" +touch "$FLEET_PLIST" +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +backup_content="$(cat "$BACKUP_PLIST")" +lcars_content="$(cat "$LCARS_PLIST")" +fleet_content="$(cat "$FLEET_PLIST")" +assert_not_contains "$backup_content" "{{" "kanban-backup.plist must contain no {{ placeholders" +assert_not_contains "$lcars_content" "{{" "lcars-health.plist must contain no {{ placeholders" +assert_not_contains "$fleet_content" "{{" "fleet-monitor.plist must contain no {{ placeholders" +test_pass + +# ═══════════════════════════════════════════════════════════════════════════ +# TEST 11: DRY_RUN + no-change path — tempfile cleanup, target untouched, +# summary reports "All LaunchAgents up to date". +# ═══════════════════════════════════════════════════════════════════════════ + +test_start "DRY_RUN=true + no-change: target unchanged, no tempfile, 'up to date' summary" +reset_launchagents +touch "$BACKUP_PLIST" +# Seed target with real rendered content so diff is clean +FORCE=false DRY_RUN=false run_update_launchagents >/dev/null 2>&1 +content_before="$(cat "$BACKUP_PLIST")" +output="$(FORCE=false DRY_RUN=true run_update_launchagents 2>&1)" +content_after="$(cat "$BACKUP_PLIST")" +assert_equal "$content_before" "$content_after" "DRY_RUN=true + no-change must leave target file unmodified" +assert_file_not_exists "${BACKUP_PLIST}.new" "No .new tempfile must remain after DRY_RUN no-change run" +assert_contains "$output" "All LaunchAgents up to date" "No-change run must report 'All LaunchAgents up to date'" +test_pass From 18b2fc3cb63d1cc7adef154f9630bf3956d9621f Mon Sep 17 00:00:00 2001 From: Darren Ehlers Date: Mon, 18 May 2026 11:39:20 -0500 Subject: [PATCH 2/2] =?UTF-8?q?Fix:=20XACA-0512=20=E2=80=94=20Address=20[R?= =?UTF-8?q?eview]=20subitem=20from=20PR=20#32=20reno?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds 17th test case exercising DRY_RUN on the fleet-monitor render path (subitem XACA-0512-002). The three pre-existing DRY_RUN tests all targeted kanban-backup, which routes through _render_kanban_template; fleet-monitor routes through a separate _render_fleet_template with a different substitution map, and a regression in its DRY_RUN handling could slip through unobserved. New test seeds the fleet-monitor target with sentinel content, runs under DRY_RUN=true, and asserts: - sentinel preserved (no write to target) - no .new tempfile leak - "Would update" printed - output references the fleet-monitor agent specifically 17/17 tests pass. shellcheck clean. CHANGELOG updated. Subitem XACA-0512-003 (shared {{AITEAMFORGE_DIR}} helper) cancelled with user approval — reviewer's wording was explicitly conditional ("if a third template family is ever added"), only one substitution is shared between renderers, and extracting a one-line helper would add indirection without removing meaningful duplication. CLAUDE.md governance: don't design for hypothetical future requirements. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- CHANGELOG.md | 2 +- ...est-xaca-0512-migrate-launchagent-render.sh | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8065026..5b059af 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,7 +19,7 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) - **Semantics preserved:** `FORCE=true` re-renders even when target is up to date; `DRY_RUN=true` writes nothing and prints `[DRY RUN] Would update`; `launchctl unload` runs before `mv`, `launchctl load` runs after (both tolerate failure via `2>/dev/null || true`); agents absent from `~/Library/LaunchAgents/` are skipped — migrate must not silently materialise agents the user opted out of at install time. - **`LAUNCHAGENTS_DIR` seam** — function respects `LAUNCHAGENTS_DIR` env var (defaults to `$HOME/Library/LaunchAgents`) so tests can inject a sandbox path without touching the real user LaunchAgents dir (M3Pro tap-install ban). Same seam pattern as XACA-0510. - **`_cleanup_migrate_tmpfiles()` + `RETURN` trap** — any `*.new` tempfile created during the render loop is cleaned up on early return or interrupt. No `*.new` leakage on no-op, DRY_RUN, or missing-template paths. -- **`tests/test-xaca-0512-migrate-launchagent-render.sh`** — 16 test cases covering: all three targets absent → all skipped; explicit `.bak`/`.bak2` regression assertion (the old in-place `sed -i.bak` behavior must be gone); kanban + fleet-monitor render with no `{{…}}` placeholders; rendered kanban plist contains resolved `NEW_DATA_DIR` (not `WORKING_DIR`); fleet-monitor renders all seven fleet-specific placeholders and resolves the `FLEET_PORT` default; selective opt-in (kanban present, fleet absent → fleet not materialised); second run no-op (mtime unchanged, no tempfile leak); `FORCE=true` re-renders; `DRY_RUN=true` preserves sentinel content with no tempfile leak; missing template warns without crashing; all-three-present rendered cleanly; DRY_RUN + no-change reports "All LaunchAgents up to date". +- **`tests/test-xaca-0512-migrate-launchagent-render.sh`** — 17 test cases covering: all three targets absent → all skipped; explicit `.bak`/`.bak2` regression assertion (the old in-place `sed -i.bak` behavior must be gone); kanban + fleet-monitor render with no `{{…}}` placeholders; rendered kanban plist contains resolved `NEW_DATA_DIR` (not `WORKING_DIR`); fleet-monitor renders all seven fleet-specific placeholders and resolves the `FLEET_PORT` default; selective opt-in (kanban present, fleet absent → fleet not materialised); second run no-op (mtime unchanged, no tempfile leak); `FORCE=true` re-renders; `DRY_RUN=true` preserves sentinel content with no tempfile leak on *both* the kanban renderer (3 cases) and the fleet-monitor renderer (1 case, added per PR #32 [Review] subitem XACA-0512-002); missing template warns without crashing; all-three-present rendered cleanly; DRY_RUN + no-change reports "All LaunchAgents up to date". - **Audit-only on `aiteamforge-migrate-check.sh`** — that script's `analyze_launchagents()` (line 381) has the same `EXPECTED_AGENTS` list but is read-only (presence + `launchctl list` check, no render logic). No change required; documented for completeness. - **Sibling-heuristic drift, third datapoint:** XACA-0476 (missing `share/` prefix) → XACA-0510 (no template render in `upgrade.sh`) → XACA-0512 (no template render in `migrate.sh`). All three sites in the launchagent-render surface are now consistent. diff --git a/tests/test-xaca-0512-migrate-launchagent-render.sh b/tests/test-xaca-0512-migrate-launchagent-render.sh index d5a65ea..0e1b5ec 100755 --- a/tests/test-xaca-0512-migrate-launchagent-render.sh +++ b/tests/test-xaca-0512-migrate-launchagent-render.sh @@ -277,6 +277,24 @@ FORCE=false DRY_RUN=true run_update_launchagents >/dev/null 2>&1 assert_file_not_exists "${BACKUP_PLIST}.new" "No .new tempfile must remain after DRY_RUN run" test_pass +# Per-renderer DRY_RUN coverage — the three tests above all exercise +# _render_kanban_template via the kanban-backup target. The fleet-monitor +# target routes through _render_fleet_template, which has a different +# substitution map. XACA-0512-002 [Review] from PR #32 — exercise DRY_RUN +# on the fleet-monitor path so a regression in the fleet renderer's +# DRY_RUN handling can't slip through. + +test_start "DRY_RUN=true (fleet-monitor): sentinel preserved, no .new leak, 'Would update' printed" +reset_launchagents +echo "FLEET_SENTINEL_DO_NOT_OVERWRITE" > "$FLEET_PLIST" +output="$(FORCE=false DRY_RUN=true run_update_launchagents 2>&1)" +fleet_content="$(cat "$FLEET_PLIST")" +assert_equal "FLEET_SENTINEL_DO_NOT_OVERWRITE" "$fleet_content" "DRY_RUN must not overwrite fleet-monitor target plist" +assert_file_not_exists "${FLEET_PLIST}.new" "No .new tempfile must remain after fleet-monitor DRY_RUN run" +assert_contains "$output" "fleet-monitor" "DRY_RUN output must reference fleet-monitor agent" +assert_contains "$output" "Would update" "DRY_RUN must print 'Would update' for fleet-monitor change" +test_pass + # ═══════════════════════════════════════════════════════════════════════════ # TEST 9: Missing template → reports cleanly without aborting # ═══════════════════════════════════════════════════════════════════════════