Skip to content

refactor(airc-bash): extract channel/peer cluster (rooms/part/send-file/invite/peers)#220

Merged
joelteply merged 1 commit intocanaryfrom
refactor/extract-cmd-rooms
Apr 28, 2026
Merged

refactor(airc-bash): extract channel/peer cluster (rooms/part/send-file/invite/peers)#220
joelteply merged 1 commit intocanaryfrom
refactor/extract-cmd-rooms

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

Pulls cmd_rooms + cmd_part + cmd_send_file + cmd_invite + cmd_peers (413 lines) into lib/airc_bash/cmd_rooms.sh. airc: 2300 → 1898 (-402). Stacks alongside merged #213-#219.

Pulls cmd_rooms + cmd_part + cmd_send_file + cmd_invite + cmd_peers
(413 lines combined) out of the airc top-level into
lib/airc_bash/cmd_rooms.sh.

  airc:                       2300 → 1898 lines (-402)
  lib/airc_bash/cmd_rooms.sh:           +441 (413 body + 28 header)

Bundled because in IRC mental model these are all the same conceptual
surface ("what rooms exist? who's in this one? how do I leave/invite/
transfer?"). One domain = one file.

Verified: airc rooms / peers / invite all dispatch correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 28, 2026 16:03
@joelteply joelteply merged commit 1762a17 into canary Apr 28, 2026
@joelteply joelteply deleted the refactor/extract-cmd-rooms branch April 28, 2026 16:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors the Bash CLI by extracting the “channel/peer” command cluster (rooms/list, part, send-file, invite, peers) out of the airc monolith into a dedicated library script, and updates airc to source it via the existing lib-dir resolver.

Changes:

  • Added lib/airc_bash/cmd_rooms.sh containing cmd_rooms, cmd_part, cmd_send_file, cmd_invite, and cmd_peers plus small helpers.
  • Removed the inlined implementations from airc and replaced them with a source block pointing at the new lib file.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
lib/airc_bash/cmd_rooms.sh New extracted implementation for rooms/part/send-file/invite/peers commands.
airc Sources the new cmd_rooms.sh instead of embedding these commands inline.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +70 to +74
local raw; raw=$(gh gist list --limit 50 2>/dev/null \
| awk -F'\t' '
/airc room:/ { print "room\t" $1 "\t" $2 "\t" $5 }
/airc invite for/ { print "invite\t" $1 "\t" $2 "\t" $5 }
')
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because airc runs with set -e, this unguarded command substitution will cause the whole script to exit if gh gist list returns non-zero (e.g., not authenticated / missing gist scope / network error). That can make airc list fail silently since stderr is redirected. Please wrap this in an if raw=$(...) block (or add || true + explicit error handling) so the command can emit a clear message and return a controlled exit code.

Suggested change
local raw; raw=$(gh gist list --limit 50 2>/dev/null \
| awk -F'\t' '
/airc room:/ { print "room\t" $1 "\t" $2 "\t" $5 }
/airc invite for/ { print "invite\t" $1 "\t" $2 "\t" $5 }
')
local raw
if raw=$(gh gist list --limit 50 2>/dev/null \
| awk -F'\t' '
/airc room:/ { print "room\t" $1 "\t" $2 "\t" $5 }
/airc invite for/ { print "invite\t" $1 "\t" $2 "\t" $5 }
'); then
:
else
echo " Failed to list gists from the gh CLI." >&2
echo " Check that you are authenticated and that gh has gist access." >&2
return 1
fi

Copilot uses AI. Check for mistakes.
# joiner, just local teardown. Records parted_rooms
# so re-join doesn't auto-resume.
# cmd_send_file — host-mediated file transfer to a peer. Pre-pairing-
# aware: writes to the host's files/<peer>/ dir.
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The header comment says send-file writes to files/<peer>/, but the implementation writes to files/${my_name}/ (sender). Either adjust the comment to match the actual behavior (as exercised by the integration test expecting files/beta/... on the host) or change the implementation if the destination directory is meant to be keyed by the recipient.

Suggested change
# aware: writes to the host's files/<peer>/ dir.
# aware: writes to the host's files/<sender>/ dir.

Copilot uses AI. Check for mistakes.
Comment on lines +274 to +280
scp_out=$(scp -i "$ssh_key" -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1)
else
scp_out=$(scp -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1)
fi
if [ $? -ne 0 ]; then
die "Failed to transfer $filename: $scp_out"
fi
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With set -e enabled, if scp fails these command substitutions will terminate the script immediately, so the subsequent if [ $? -ne 0 ] check (and the helpful die message) will never run. Capture the exit status using an if scp_out=$(scp ...); then ... else ... pattern (or temporarily disable -e) so failures surface as the intended error message.

Suggested change
scp_out=$(scp -i "$ssh_key" -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1)
else
scp_out=$(scp -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1)
fi
if [ $? -ne 0 ]; then
die "Failed to transfer $filename: $scp_out"
fi
if scp_out=$(scp -i "$ssh_key" -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1); then
:
else
die "Failed to transfer $filename: $scp_out"
fi
else
if scp_out=$(scp -o StrictHostKeyChecking=accept-new -q "$filepath" "${target_host}:${rhome}/files/${my_name}/${filename}" 2>&1); then
:
else
die "Failed to transfer $filename: $scp_out"
fi
fi

Copilot uses AI. Check for mistakes.
Comment on lines +168 to +175
local ts="${1:-}"
local threshold_hours="${AIRC_STALE_HOURS:-24}"
[ -z "$ts" ] && return 1
local epoch; epoch=$(iso_to_epoch "$ts")
[ -z "$epoch" ] && return 1
local now; now=$(date -u +%s)
local diff=$((now - epoch))
[ "$diff" -gt $((threshold_hours * 3600)) ]
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

threshold_hours comes from $AIRC_STALE_HOURS and is used in arithmetic expansion. If it’s unset or non-numeric, $((threshold_hours * 3600)) will raise an arithmetic error and (because airc uses set -e) abort airc list. Consider validating/sanitizing threshold_hours to an integer (fallback to 24) before doing arithmetic.

Copilot uses AI. Check for mistakes.
Comment thread airc
Comment on lines +1636 to +1639
else
echo "ERROR: airc_bash/cmd_rooms.sh not found via lib-dir resolver." >&2
exit 1
fi
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This source failure path is less actionable than the other lib extraction blocks (which print the resolved lib_dir and suggest reinstalling/checking AIRC_DIR). Consider including ${_airc_lib_dir:-<empty>} in the error output and a next-step hint to make failures easier to debug.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants