refactor(airc-bash): extract cmd_connect — Phase 0 monolith split#213
refactor(airc-bash): extract cmd_connect — Phase 0 monolith split#213
Conversation
…file split
cmd_connect was the single largest block in the airc bash monolith (1355
lines, ~26% of the file). Splits it out verbatim into
lib/airc_bash/cmd_connect.sh, sourced via the same lib-dir resolver that
already loads cmd_doctor.sh and platform_adapters.sh.
airc: 5265 → 3911 lines (-1354)
lib/airc_bash/cmd_connect.sh: +1379 (1355 body + 24 header)
net: +25 (header + source-block overhead)
Behavior unchanged — cmd_connect calls airc top-level helpers (die,
ensure_init, get_config_val, set_config_val, relay_ssh, _reexec_into,
_self_heal_stale_host, spawn_general_sidecar_if_wanted, monitor, …)
and exposes only the cmd_connect function back to the dispatch.
Verified equivalence:
- bash -n on both files clean
- airc help / version / connect dispatch reach the same code paths
(same flag-parser error on `airc connect --help` as pristine canary)
- test/integration.sh tabs: 19/0 passing — full join + send + rename
+ peers flow under harness
This is Phase 0 of the structural decomposition Joel called out: the
"shell scripts are like classes" framing was applied to cmd_doctor and
platform_adapters but never to the bulk of cmd_X functions still inline.
Follow-ups will pull cmd_send / cmd_teardown / cmd_status / cmd_rooms /
cmd_rename / cmd_peers each into their own file. Eventually the airc
top-level retains only: bootstrap, helpers, dispatch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…it (#214) refactor(airc-bash): extract cmd_daemon family — Phase 3 file split 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 large cmd_connect command implementation out of the top-level airc bash monolith into a dedicated sourced module, continuing the ongoing “Phase 0 monolith split” work in lib/airc_bash/.
Changes:
- Moved the full
cmd_connectimplementation intolib/airc_bash/cmd_connect.sh. - Updated
aircto source the extractedcmd_connectmodule via the existing lib-dir resolver pattern.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
lib/airc_bash/cmd_connect.sh |
New sourced module containing the extracted cmd_connect implementation. |
airc |
Removes inlined cmd_connect and sources cmd_connect.sh via $_airc_lib_dir. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| --room|-room) room_name="${2:-general}"; use_room=1; room_explicit=1; shift 2 ;; | ||
| --no-room|-no-room) use_room=0; shift ;; | ||
| --no-general|-no-general) |
There was a problem hiding this comment.
--room parsing sets a default (${2:-general}) but then unconditionally does shift 2. If the user passes --room at the end (or --room --no-general), shift 2 will fail under set -e and/or the next flag gets consumed as the room name. Consider requiring an explicit value (die with usage) or only shifting 2 when a non-flag value is present; this matches how cmd_send validates --room.
| --room-only|-room-only) | ||
| # Combo: explicit project room + skip general sidecar. For | ||
| # focused work where lobby noise would distract. | ||
| room_name="${2:-general}"; use_room=1; room_explicit=1; general_sidecar=0 | ||
| shift 2 ;; |
There was a problem hiding this comment.
--room-only has the same issue as --room: it defaults ${2:-general} but still does shift 2, which can error under set -e when the value is missing and can also consume the next flag as the room name. Add value validation (or conditional shifting) so flag parsing can’t terminate the script unexpectedly.
| # know AIRC_WRITE_DIR (set by ensure_init below) is meaningful — but | ||
| # we have to wait for ensure_init to run, since --general can be | ||
| # called before any prior init. The cleanup happens via a deferred | ||
| # check in spawn_general_sidecar_if_wanted: since _clear_parted_room | ||
| # is idempotent, we can call it eagerly here when config exists, and |
There was a problem hiding this comment.
This comment says we need to wait for ensure_init so AIRC_WRITE_DIR is meaningful, but AIRC_WRITE_DIR is set at script startup (and ensure_init only checks for $CONFIG). Updating the comment would prevent future readers from assuming the wrong initialization order.
| # know AIRC_WRITE_DIR (set by ensure_init below) is meaningful — but | |
| # we have to wait for ensure_init to run, since --general can be | |
| # called before any prior init. The cleanup happens via a deferred | |
| # check in spawn_general_sidecar_if_wanted: since _clear_parted_room | |
| # is idempotent, we can call it eagerly here when config exists, and | |
| # know whether --general was requested. AIRC_WRITE_DIR is already | |
| # available here; the only question is whether config.json exists yet | |
| # for this session. The cleanup also happens via a deferred check in | |
| # spawn_general_sidecar_if_wanted: since _clear_parted_room is | |
| # idempotent, we can call it eagerly here when config exists, and |
| # 1s parent-watch thread (see airc_core.handshake._start_parent_watch) | ||
| # to catch the in-flight-handshake case; this loop check covers the | ||
| # between-iterations case before the next python is spawned. | ||
| _orphan_parent_pid=$$ |
There was a problem hiding this comment.
_orphan_parent_pid=$$ is assigned without local, which makes it a global variable in the shell. Since cmd_connect is sourced into the main script, prefer local _orphan_parent_pid=$$ (and similarly for other internal PIDs) to avoid accidental collisions with other functions/commands.
| _orphan_parent_pid=$$ | |
| local _orphan_parent_pid=$$ |
| # PAIR_PID (TCP-accept loop), and the heartbeat-loop PID (if hosting a | ||
| # room with a gist) so teardown can reap all three. | ||
| _hb_pid_persisted="" | ||
| [ -f "$AIRC_WRITE_DIR/heartbeat.pid" ] && _hb_pid_persisted=$(cat "$AIRC_WRITE_DIR/heartbeat.pid" 2>/dev/null) | ||
| echo "$$ $PAIR_PID $_hb_pid_persisted" > "$AIRC_WRITE_DIR/airc.pid" |
There was a problem hiding this comment.
PAIR_PID and _hb_pid_persisted are assigned without local, so they become globals in the sourced shell environment. Declaring them local (and guarding uses in the EXIT trap) reduces the chance of name collisions and makes it clearer these are cmd_connect internals.
Summary
Splits
cmd_connect(1355 lines, ~26% ofairc) out of the bash monolith intolib/airc_bash/cmd_connect.sh, sourced via the same lib-dir resolver ascmd_doctor.sh/platform_adapters.sh.Why now
Joel's call-out 2026-04-27: "why did you never fix the monolith? thought that was what you had been working on. we just fucking discussed this shit." The recent compression PRs (#206-#211) trimmed lines but never actually decomposed the file.
cmd_connectwas the single largest block — pulling it out is the most impactful first step toward the "shell scripts are like classes" structure.This is Phase 0. Follow-ups will extract
cmd_send,cmd_teardown,cmd_status,cmd_rooms,cmd_rename,cmd_peerseach into their own file. Eventuallyairctop-level retains only bootstrap + helpers + dispatch.Verification
bash -nclean on both filesairc help/airc version/airc connectdispatch reach the same code paths as pristine canary HEAD (verified by stashing the diff)test/integration.sh tabs: 19/0 passing (full join + send + send-file + queue + rename + peers flow)Test plan
airc joinend-to-end on macOS (host) + Windows (joiner)Notes
cmd_connectcalls airc top-level helpers (die,ensure_init,get_config_val,set_config_val,relay_ssh,_reexec_into,_self_heal_stale_host,spawn_general_sidecar_if_wanted,monitor,port_listeners, …) and exposes onlycmd_connectback to the dispatch. No public-surface change.Filed separately: existing
airc connect --helpflag-parsing bug (config writes empty--valuewhen given an unknown flag) — present on canary HEAD, untouched by this PR.🤖 Generated with Claude Code