refactor(airc-bash): extract cmd_daemon family — Phase 0 monolith split#214
refactor(airc-bash): extract cmd_daemon family — Phase 0 monolith split#214
Conversation
Pulls the cmd_daemon command group (cmd_daemon + cmd_daemon_install/
uninstall/status/log + 8 private _daemon_* helpers) out of the airc
top-level into lib/airc_bash/cmd_daemon.sh, sourced via the same
lib-dir resolver as cmd_doctor.sh / cmd_connect.sh /
platform_adapters.sh.
airc: 5265 → 4834 lines (-431)
lib/airc_bash/cmd_daemon.sh: +461 (432 body + 29 header)
Behavior unchanged. Cross-references resolve at call-time:
- cmd_daemon.sh calls airc top-level helpers (die, detect_platform)
- airc top-level (monitor self-heal, line ~1292) calls
_daemon_installed defined in cmd_daemon.sh
Verified:
- bash -n on both files
- airc daemon status — full plist/launchctl readout, log path correct
Stacks alongside #213 (cmd_connect extraction). Each PR independently
removes a major block from the bash monolith.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Extracts the cmd_daemon command family from the airc monolith into a dedicated bash module (lib/airc_bash/cmd_daemon.sh) and sources it via the existing lib-dir resolver, continuing the ongoing monolith split.
Changes:
- Added
lib/airc_bash/cmd_daemon.shcontainingcmd_daemon*functions and_daemon_*helpers. - Removed the inlined
cmd_daemonblock fromaircand replaced it with a guardedsourceof the new module.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
lib/airc_bash/cmd_daemon.sh |
New extracted module implementing airc daemon {install,status,uninstall,log} and platform-specific installers. |
airc |
Replaces the inlined daemon implementation with sourcing of cmd_daemon.sh via _airc_lib_dir. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| _daemon_install_schtasks() { | ||
| # Windows daemon via HKCU Run-key (no admin; HKCU\...\Run is user- | ||
| # scope, so per-user autostart at logon without UAC). PRs #200/#202 | ||
| # for the why; this function for the how. | ||
| local airc_bin="$1" scope="$2" | ||
| local entry_name="airc-monitor" |
There was a problem hiding this comment.
The name _daemon_install_schtasks is misleading: the implementation uses an HKCU Run-key launcher .bat, not Windows Scheduled Tasks. Renaming this helper (and its callsite/comment header) would make the platform behavior clearer for future maintainers.
| [ -f "$unit_path" ] && rm "$unit_path" && systemctl --user daemon-reload && echo " ✓ Removed $unit_path" \ | ||
| || echo " (no unit on disk)" |
There was a problem hiding this comment.
This && ... || echo "(no unit on disk)" chain reports "no unit" for any failure in the removal path (e.g. systemctl --user daemon-reload failing), not just when the unit file is absent. Consider separating the checks so errors are surfaced accurately and the message reflects the real failure mode.
| [ -f "$unit_path" ] && rm "$unit_path" && systemctl --user daemon-reload && echo " ✓ Removed $unit_path" \ | |
| || echo " (no unit on disk)" | |
| if [ -f "$unit_path" ]; then | |
| if rm "$unit_path" && systemctl --user daemon-reload; then | |
| echo " ✓ Removed $unit_path" | |
| else | |
| echo " Failed to remove $unit_path or reload systemd user units" | |
| return 1 | |
| fi | |
| else | |
| echo " (no unit on disk)" | |
| fi |
| echo " Plist: $plist_path" | ||
| # launchctl print returns rich state; grep the key fields. | ||
| local state; state=$(launchctl print "gui/$(id -u)/com.cambriantech.airc" 2>/dev/null \ | ||
| | grep -E 'state =|pid =|last exit code' | head -3) |
There was a problem hiding this comment.
In strict mode (set -euo pipefail), this command substitution will cause airc daemon status to exit when the job isn't loaded (e.g. launchctl print fails or grep finds no matches). Consider making the pipeline non-fatal (e.g. tolerate non-zero from launchctl print/grep) so the subsequent if [ -n "$state" ] branch can run as intended.
| | grep -E 'state =|pid =|last exit code' | head -3) | |
| | grep -E 'state =|pid =|last exit code' | head -3 || true) |
| local active; active=$(systemctl --user is-active airc.service 2>/dev/null) | ||
| local enabled; enabled=$(systemctl --user is-enabled airc.service 2>/dev/null) |
There was a problem hiding this comment.
systemctl is-active/is-enabled return non-zero for common states like inactive/disabled. Under set -e, these assignments will terminate airc daemon status instead of printing the state. Capture the output while tolerating non-zero exit codes and defaulting to a sensible value when the command fails.
| local active; active=$(systemctl --user is-active airc.service 2>/dev/null) | |
| local enabled; enabled=$(systemctl --user is-enabled airc.service 2>/dev/null) | |
| local active; active="$(systemctl --user is-active airc.service 2>/dev/null || true)" | |
| [ -n "$active" ] || active="unknown" | |
| local enabled; enabled="$(systemctl --user is-enabled airc.service 2>/dev/null || true)" | |
| [ -n "$enabled" ] || enabled="unknown" |
| # Scope: defaults to the GLOBAL scope ($HOME/.airc), since the daemon is the | ||
| # user's "always-on" mesh presence — not tied to a specific project dir. If | ||
| # the user wants a per-project always-on daemon, they pass AIRC_HOME=<dir> | ||
| # in the environment when running install (and the generated unit/plist | ||
| # will carry that scope). |
There was a problem hiding this comment.
The header docs say the daemon scope defaults to $HOME/.airc, but _daemon_scope (and detect_scope in airc) default to $(pwd -P)/.airc. Please align the comment with the actual behavior (or change _daemon_scope if the intended default really is global).
| # they call this helper. Pass the platform-specific lead line as $1 and | ||
| # any optional trailing note as $2 (heredoc-style multi-line OK). |
There was a problem hiding this comment.
This comment describes _daemon_install_done's parameters as $1 lead line and $2 optional note, but the function signature is (lead, scope, note) where $2 is the scope path and $3 is the optional note. Update the comment to match the actual parameters to avoid misuses in future edits.
| # they call this helper. Pass the platform-specific lead line as $1 and | |
| # any optional trailing note as $2 (heredoc-style multi-line OK). | |
| # they call this helper. Pass the platform-specific lead line as $1, | |
| # the scope path as $2, and any optional trailing note as $3 | |
| # (heredoc-style multi-line OK). |
| # Convert paths to Windows form; cmd.exe can't read /c/Users/... . | ||
| local airc_bin_win; airc_bin_win=$(_to_win_path "$airc_bin") | ||
| local scope_win; scope_win=$(_to_win_path "$scope") | ||
|
|
There was a problem hiding this comment.
airc_bin_win is computed but never used. Dropping it (or using it in the generated launcher) would reduce dead code and avoid confusion about which path form is expected.
| # Convert paths to Windows form; cmd.exe can't read /c/Users/... . | |
| local airc_bin_win; airc_bin_win=$(_to_win_path "$airc_bin") | |
| local scope_win; scope_win=$(_to_win_path "$scope") |
Summary
Splits the cmd_daemon command-group (5 cmd_X functions + 8 private helpers, 432 lines) out of the airc bash monolith into `lib/airc_bash/cmd_daemon.sh`, sourced via the same lib-dir resolver as cmd_connect.sh / cmd_doctor.sh / platform_adapters.sh.
```
airc: 5265 → 4834 lines (-431)
lib/airc_bash/cmd_daemon.sh: +461 (432 body + 29 header)
```
Stacks alongside #213 (cmd_connect, -1354 from airc). Independent — they touch different line ranges in airc and can land in either order.
Scope
Functions extracted:
Plus 8 `daemon*` private helpers (airc_path, scope, installed, install_done, install_launchd, install_schtasks, install_systemd).
Verification
Test plan
🤖 Generated with Claude Code