From 180f2de8539a9d9d91b76a641cee3104ccfe92b2 Mon Sep 17 00:00:00 2001 From: August Cayzer Date: Sat, 5 Sep 2026 21:06:41 +0100 Subject: [PATCH] fix: make a renamed runner's bin and externals links relative GitHub's runner updater points bin and externals at its versioned directories with absolute symlinks, so moving a runner directory takes two dangling links with it and ./config.sh dies on a missing Runner.Listener before registering anything. Every runner install has them, so rename failed on the first runner every time. migrate-storage has always rewritten them relative. rename did not reuse that helper, which is now named for what it does rather than for the one command that used to call it. The test did not catch this because its fake config.sh ignored the links. It now refuses unless ./bin/Runner.Listener resolves, as the real one does, and the fixture carries absolute links like a real install. Closes #73 --- bin/runpool | 2 +- lib/lifecycle.sh | 21 +++++++++++++++------ tests/pool-rename.sh | 30 ++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/bin/runpool b/bin/runpool index 40b8189..a54ab63 100755 --- a/bin/runpool +++ b/bin/runpool @@ -43,7 +43,7 @@ export RUNPOOL_INVOKED # The released version, and the only place it is written. The Homebrew formula # builds from a git tag, so a tag without a matching bump here ships a binary # that misreports itself. -RUNPOOL_VERSION="0.12.0" +RUNPOOL_VERSION="0.12.1" # shellcheck source=lib/common.sh . "${RUNPOOL_ROOT}/lib/common.sh" diff --git a/lib/lifecycle.sh b/lib/lifecycle.sh index dab2f1a..bf8793a 100644 --- a/lib/lifecycle.sh +++ b/lib/lifecycle.sh @@ -571,7 +571,15 @@ _rp_move_dir() { mv "${from}" "${to}" } -_rp_migrate_rewrite_runner_links() { +# GitHub's runner updater points `bin` and `externals` at its versioned +# directories with ABSOLUTE symlinks, so a runner directory that moves takes +# two dangling links with it and `./config.sh` dies on a missing +# Runner.Listener. Rewriting them relative makes the installation +# self-contained, and therefore safe to move again. +# +# Every caller that relocates a runner directory has to do this, and do it +# before running config.sh rather than after. +_rp_rewrite_runner_links() { local runner_dir="$1" name link target target_name for name in bin externals; do link="${runner_dir}/${name}" @@ -579,12 +587,9 @@ _rp_migrate_rewrite_runner_links() { target=$(readlink "${link}") || return 1 target_name="${target##*/}" [ -n "${target_name}" ] && [ -d "${runner_dir}/${target_name}" ] || { - _rp_err "migration copied an unresolved runner link: ${link} -> ${target}" + _rp_err "unresolved runner link: ${link} -> ${target}" return 1 } - # GitHub's runner updater creates absolute links to its versioned bin and - # externals directories. Make the copied installation self-contained so - # removing the legacy tree cannot break Runner.Listener afterwards. ln -sfn "${target_name}" "${link}" || return 1 done } @@ -697,7 +702,7 @@ _rp_migrate_storage() { while [ "${i}" -le "${POOL_COUNT}" ]; do runner_dir="${new_dir}/runner-${i}" cache_dir="${cache_pool}/runner-${i}" - _rp_migrate_rewrite_runner_links "${runner_dir}" || return 1 + _rp_rewrite_runner_links "${runner_dir}" || return 1 mkdir -p "${cache_dir}" || return 1 # A work tree is regenerable but not necessarily portable. Compilers can # bake its absolute path into build artifacts, and a remote cache can @@ -1074,6 +1079,10 @@ CONF _rp_log "${new} runner-${i}: past the pool's count of ${POOL_COUNT}, deregistered and left on disk" continue } + # Before config.sh, not after: the runner's bin and externals links are + # absolute and now point into the directory this pool used to live in, so + # ./config.sh cannot even start until they are made relative. + _rp_rewrite_runner_links "${runner_dir}" || return 1 _rp_migrate_update_work_folder "${runner_dir}" "$(_rp_runner_work_dir "${new}" "${i}")" || return 1 runner_name="$(hostname -s)-${new}-${i}" _rp_log "${new} runner-${i}: registering as '${runner_name}'" diff --git a/tests/pool-rename.sh b/tests/pool-rename.sh index 1d16082..2a7d384 100755 --- a/tests/pool-rename.sh +++ b/tests/pool-rename.sh @@ -81,12 +81,27 @@ build_pool() { >"${dir}/runner-${i}/.runner" # A config.sh that behaves like the real one: refuses nothing, writes a # .runner, and records exactly what it was asked for. + # Faithful to the real one in the way that matters here: it runs + # ./bin/Runner.Listener, so a dangling bin link makes it fail before it + # registers anything. A stub that ignored the links would pass whether or + # not they were rewritten, and prove nothing. cat >"${dir}/runner-${i}/config.sh" <&2; exit 1; } echo "\$*" >> "${cfg_log}" printf '{"agentId": 99, "workFolder": "x"}\n' > .runner STUB chmod +x "${dir}/runner-${i}/config.sh" + # GitHub's runner updater points bin and externals at versioned directories + # with ABSOLUTE links. A directory that moves therefore takes two dangling + # links with it, and config.sh dies on a missing Runner.Listener before it + # can register anything. The fixture has to carry them or the rename looks + # like it works. + mkdir -p "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/externals.2.337.0" + printf '#!/bin/bash\n' >"${dir}/runner-${i}/bin.2.337.0/Runner.Listener" + chmod +x "${dir}/runner-${i}/bin.2.337.0/Runner.Listener" + ln -sfn "${dir}/runner-${i}/bin.2.337.0" "${dir}/runner-${i}/bin" + ln -sfn "${dir}/runner-${i}/externals.2.337.0" "${dir}/runner-${i}/externals" printf 'plist for %s runner-%s\n' "${name}" "${i}" \ >"${RUNPOOL_AGENT_DIR}/runpool.${name}.${i}.plist" i=$(( i + 1 )) @@ -136,6 +151,21 @@ absent "${RUNPOOL_BASE}/runners/alpha" "the old runner tree" present "${RUNPOOL_CACHE_DIR}/pools/bravo" "the cache moved" absent "${RUNPOOL_CACHE_DIR}/pools/alpha" "the old cache" +# The links have to be relative afterwards, and they have to resolve. An +# absolute link left pointing into the old path is a runner that cannot start, +# and config.sh fails before registering, which leaves the pool half renamed. +for r in 1 2; do + link="${RUNPOOL_BASE}/runners/bravo/runner-${r}/bin" + [ -L "${link}" ] || fail "runner-${r}: bin is not a symlink" + case "$(readlink "${link}")" in + /*) fail "runner-${r}: bin is still an absolute link into the old path" ;; + esac + [ -d "${link}/" ] || fail "runner-${r}: bin does not resolve after the move" + ok + [ -d "${RUNPOOL_BASE}/runners/bravo/runner-${r}/externals/" ] || fail "runner-${r}: externals does not resolve" + ok +done + # Written by _rp_write_plist against the new label, and the old ones removed by # hand: _rp_rewrite_plists only ever writes. present "${RUNPOOL_AGENT_DIR}/runpool.bravo.1.plist" "the new launch agent"