Skip to content

refactor(airc): _reexec_into helper consolidates 5 exec sites (#205 target 1, net -21)#206

Merged
joelteply merged 1 commit intocanaryfrom
refactor/reexec-into-helper
Apr 28, 2026
Merged

refactor(airc): _reexec_into helper consolidates 5 exec sites (#205 target 1, net -21)#206
joelteply merged 1 commit intocanaryfrom
refactor/reexec-into-helper

Conversation

@joelteply
Copy link
Copy Markdown
Contributor

Net-negative diff: -21 lines

Additions 25
Deletions 46
Net removed 21
airc.sh size 5469 → 5448

What changed

Five cmd_connect sites previously duplicated the same 3-line pattern:

local _preserved_name; _preserved_name=$(get_config_val name "")
_write_reexec_marker
exec env [AIRC_NO_DISCOVERY=1] ${_preserved_name:+AIRC_NAME=$_preserved_name} "$0" connect <args>

Plus 3 inline comment paragraphs explaining what each exec did (also duplicated).

After: one _reexec_into <mode> <args> helper, mode ∈ {rejoin, host}. Five call sites collapse to one line each. The standalone _write_reexec_marker function (11-line block comment + 4-line body, only one logical caller after this) is folded into the helper.

Behavior

Unchanged. Same env vars passed the same way, same exec arguments, same marker file written. Verified:

Bonus side-effect

Pre-fix, every new exec site was a fresh chance to forget the _write_reexec_marker call — which is exactly what triggered #203 (the daemon crashloop). Post-fix, the only way to re-exec is _reexec_into, which always writes the marker. Future fix sites can't regress.

#205 progress

Target 1 of 6 ✅. Authenticator-fd63 owns target 3 in parallel. Combined target 1+3 should land ~50 lines net negative.

🤖 Generated with Claude Code

…tes (#205 target 1)

Net: -15 lines (38 deletions, 23 additions). First compression PR per
#205's net-negative-diff mandate from Joel.

Five sites in cmd_connect previously duplicated the same 3-line pattern:
  local _preserved_name; _preserved_name=\$(get_config_val name "")
  _write_reexec_marker
  exec env [\${AIRC_NO_DISCOVERY=1}] \${_preserved_name:+AIRC_NAME=\$_preserved_name} "\$0" connect <args>

That's 5 × 3 = 15 lines of copy-paste, three of which were stale-host-
takeover paths that diverged from the rejoin-race-loser paths only by
the AIRC_NO_DISCOVERY=1 prefix. Plus 3 inline comment paragraphs
explaining what the exec was for, also duplicated.

Now: one helper _reexec_into <mode> <args>, mode in {rejoin, host}.
Folds in the sentinel marker write (used to be its own _write_reexec_
marker function with 11-line block comment — collapsed into helper).
Five call sites become one line each.

Behavior unchanged: same env vars passed in same way, same exec
arguments, same marker file written. Only the structure changed.

Bonus: caller can no longer forget to call _write_reexec_marker
before exec — the only path is via _reexec_into which always writes
it. (Pre-fix, every new exec site was a fresh chance to forget it,
which is exactly what triggered #203.)

#205 Target 1 of 6. Joel's bar: every PR net-negative or extension-
point-bearing. This is net-negative.
Copilot AI review requested due to automatic review settings April 28, 2026 14:17
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 airc connect self re-exec flows by consolidating repeated “write marker + preserve name + exec env … connect …” patterns into a single _reexec_into helper, keeping Windows daemon re-exec sentinel behavior intact (#203/#204) while reducing duplication as part of #205.

Changes:

  • Introduces _reexec_into <mode> ... to centralize marker writing, name preservation, and host takeover env (AIRC_NO_DISCOVERY=1).
  • Replaces five duplicated exec call sites in cmd_connect with one-line _reexec_into calls.
  • Trims duplicated daemon-launcher marker commentary to reference the new helper.

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

Comment thread airc
Comment on lines +306 to +310
if [ "$mode" = "host" ]; then
exec env AIRC_NO_DISCOVERY=1 ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
else
exec env ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
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.

_reexec_into passes AIRC_NAME to env via an unquoted parameter expansion. If the configured name ever contains whitespace/shell metacharacters, this will split into multiple args (the embedded quotes in AIRC_NAME="$_name" are literal characters and do not prevent word-splitting). Build an env argv array (as done in spawn_general_sidecar_if_wanted) and exec env "${env_args[@]}" ... to make this quoting-safe.

Suggested change
if [ "$mode" = "host" ]; then
exec env AIRC_NO_DISCOVERY=1 ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
else
exec env ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
fi
local -a env_args=()
if [ "$mode" = "host" ]; then
env_args+=(AIRC_NO_DISCOVERY=1)
fi
if [ -n "$_name" ]; then
env_args+=("AIRC_NAME=$_name")
fi
exec env "${env_args[@]}" "$0" connect "$@"

Copilot uses AI. Check for mistakes.
Comment thread airc
Comment on lines +303 to +310
local mode="$1"; shift # "rejoin" or "host"
printf '%d:%d\n' "$$" "$(date +%s)" > "$AIRC_WRITE_DIR/airc.reexec-marker" 2>/dev/null || true
local _name; _name=$(get_config_val name "")
if [ "$mode" = "host" ]; then
exec env AIRC_NO_DISCOVERY=1 ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
else
exec env ${_name:+AIRC_NAME="$_name"} "$0" connect "$@"
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.

_reexec_into silently treats any mode other than host as the rejoin path. Since the function contract/comment says mode is rejoin|host, consider validating mode (including missing $1) and failing fast on unexpected values to avoid accidental behavior changes when adding new call sites.

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