Skip to content

refactor(airc-bash): extract cmd_status + cmd_logs#217

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

refactor(airc-bash): extract cmd_status + cmd_logs#217
joelteply merged 1 commit intocanaryfrom
refactor/extract-cmd-status

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

Pulls introspection verbs (cmd_status + cmd_logs, 154 lines) into lib/airc_bash/cmd_status.sh. airc: 2909 → 2764 (-145). Stacks alongside merged #213-#216.

Pulls the introspection verbs (cmd_status + cmd_logs, 154 lines combined)
out of the airc top-level into lib/airc_bash/cmd_status.sh.

  airc:                        2909 → 2764 lines (-145)
  lib/airc_bash/cmd_status.sh:           +170 (155 body + 15 header)

cmd_status and cmd_logs were not contiguous in airc (cmd_logs lived
~30 lines below the cmd_doctor source-block); a single source-block
in airc top-level now provides both functions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 28, 2026 15:59
@joelteply joelteply merged commit dfe91bd into canary Apr 28, 2026
@joelteply joelteply deleted the refactor/extract-cmd-status branch April 28, 2026 15:59
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

This PR continues the ongoing airc bash monolith split by moving the introspection commands (cmd_status, cmd_logs) into a dedicated sourced module, reducing top-level script size while keeping dispatch behavior intact.

Changes:

  • Extract cmd_status and cmd_logs into lib/airc_bash/cmd_status.sh.
  • Update airc to source the new module via _airc_lib_dir and remove the inlined function bodies.

Reviewed changes

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

File Description
lib/airc_bash/cmd_status.sh New sourced module containing cmd_status and cmd_logs.
airc Sources cmd_status.sh and removes the previously inlined implementations.

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

Comment on lines +124 to +133
# Pending queue — how many sends are waiting for a drain. Populated by
# cmd_send's wire-failure branch; drained by flush_pending_loop.
local pending="$AIRC_WRITE_DIR/pending.jsonl"
local pending_count=0
[ -f "$pending" ] && pending_count=$(grep -c '^.' "$pending" 2>/dev/null || echo 0)
if [ "$pending_count" -gt 0 ]; then
echo " queue: ${pending_count} pending (auto-retries every ~5s)"
else
echo " queue: empty"
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.

pending_count is computed with grep -c ... || echo 0. When the file exists but has 0 matching lines, grep -c prints 0 and exits 1, so the || echo 0 appends a second 0, yielding a non-integer value like "0 0" and causing [ "$pending_count" -gt 0 ] to emit an error. Use a counting approach that doesn't double-print on exit-1 (e.g. wc -l < file, or grep -c with || true and a separate default).

Copilot uses AI. Check for mistakes.
Comment on lines +152 to +160
local host_target
host_target=$(get_config_val host_target "")

local raw
if [ -n "$host_target" ]; then
local rhome; rhome=$(remote_home)
raw=$(relay_ssh "$host_target" "tail -${count} $rhome/messages.jsonl 2>/dev/null" 2>/dev/null) || true
else
raw=$(tail -"$count" "$MESSAGES" 2>/dev/null) || true
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.

count is passed directly into tail (locally and inside the remote command). Since it comes from argv, a non-numeric value can break tail parsing, and in the remote case it is interpolated unquoted into a shell command, enabling command injection via something like airc logs '20; ...'. Validate count is an integer (or support explicit --count N parsing) and build the tail invocation in a way that prevents shell injection (e.g. tail -n with a validated number, and quote the remote path).

Suggested change
local host_target
host_target=$(get_config_val host_target "")
local raw
if [ -n "$host_target" ]; then
local rhome; rhome=$(remote_home)
raw=$(relay_ssh "$host_target" "tail -${count} $rhome/messages.jsonl 2>/dev/null" 2>/dev/null) || true
else
raw=$(tail -"$count" "$MESSAGES" 2>/dev/null) || true
case "$count" in
''|*[!0-9]*)
echo "airc logs: count must be a non-negative integer" >&2
return 1
;;
esac
local host_target
host_target=$(get_config_val host_target "")
local raw
if [ -n "$host_target" ]; then
local rhome remote_messages remote_messages_quoted
rhome=$(remote_home)
remote_messages="$rhome/messages.jsonl"
remote_messages_quoted=$(printf "%s" "$remote_messages" | sed "s/'/'\\\\''/g")
raw=$(relay_ssh "$host_target" "tail -n $count '$remote_messages_quoted' 2>/dev/null" 2>/dev/null) || true
else
raw=$(tail -n "$count" "$MESSAGES" 2>/dev/null) || true

Copilot uses AI. Check for mistakes.
Comment on lines +11 to +12
# references (call-time): die, ensure_init, get_config_val, relay_ssh,
# remote_home, MESSAGES, AIRC_PYTHON.
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’s “External cross-references” list is now incomplete: this file also calls get_name and references IDENTITY_DIR. Keeping this list accurate helps when auditing module dependencies during the ongoing monolith split.

Suggested change
# references (call-time): die, ensure_init, get_config_val, relay_ssh,
# remote_home, MESSAGES, AIRC_PYTHON.
# references (call-time): die, ensure_init, get_config_val, get_name,
# relay_ssh, remote_home, MESSAGES, AIRC_PYTHON, IDENTITY_DIR.

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