fix(fleet): always bake runtime-bin into pane PATH (ignore launcher PATH)
The previous `_build_runtime_bin_prefix()` skipped candidate dirs that
were already present in the LAUNCHER process's \$PATH. This is wrong:
the tmux pane inherits the tmux SERVER environment, not the launcher's
env. A dir on the launcher's \$PATH may be absent from the server env,
so the prefix could come back empty and the pane would fail with
'command not found'.
Remove the `case ":${PATH}:"` check that tested against the live launcher
PATH. Keep the existence check (`[ -d "$dir" ]`) and the dedup-within-
the-constructed-prefix guard. The pane command's `export PATH="<prefix>:${PATH}"`
harmlessly absorbs any overlap with the server PATH.
Add test 5 to test-start-agent-session.sh: sets FAKE_RUNTIME_BIN5 on the
launcher's \$PATH and asserts it still appears in the generated pane PATH
export — directly guarding this regression.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RMoEx7hfdFGjUiCHuN1RRi
This commit is contained in:
@@ -32,8 +32,12 @@ fi
|
||||
# 2. $(npm config get prefix)/bin (if npm is on PATH)
|
||||
# 3. Fallbacks: $HOME/.npm-global/bin and $HOME/.local/bin
|
||||
#
|
||||
# Only directories that already exist are included. Directories already
|
||||
# present in PATH are skipped so we never duplicate entries.
|
||||
# Only directories that already exist are included. The prefix is baked into
|
||||
# the pane command regardless of what the LAUNCHER process's $PATH contains,
|
||||
# because the tmux pane inherits the tmux SERVER environment (not this script's
|
||||
# environment). A dir on the launcher's PATH may be absent from the server PATH,
|
||||
# so every existing candidate must always be included. Dedup within the
|
||||
# constructed prefix avoids listing the same dir twice.
|
||||
_build_runtime_bin_prefix() {
|
||||
local candidates=()
|
||||
|
||||
@@ -55,14 +59,11 @@ _build_runtime_bin_prefix() {
|
||||
local prefix=""
|
||||
for dir in "${candidates[@]}"; do
|
||||
[ -d "$dir" ] || continue
|
||||
case ":${PATH}:" in
|
||||
*":${dir}:"*) continue ;; # already present
|
||||
esac
|
||||
if [ -z "$prefix" ]; then
|
||||
prefix="$dir"
|
||||
else
|
||||
case ":${prefix}:" in
|
||||
*":${dir}:"*) ;; # already in our prefix
|
||||
*":${dir}:"*) ;; # already in our prefix — skip
|
||||
*) prefix="${prefix}:${dir}" ;;
|
||||
esac
|
||||
fi
|
||||
|
||||
@@ -148,4 +148,61 @@ rm -rf "$WORKDIR4"
|
||||
echo "$all_args4" | grep -qF "exec " || fail "pane command (no prefix dirs) does not use exec"
|
||||
echo "$all_args4" | grep -qF "mosaic yolo pi" || fail "pane command does not include agent command when no prefix"
|
||||
|
||||
# ── Test 5: candidate dir already in LAUNCHER $PATH is still baked into pane ──
|
||||
#
|
||||
# Regression guard for the bug where _build_runtime_bin_prefix() used to skip
|
||||
# a candidate because it was already present in the launcher process's $PATH.
|
||||
# That check was wrong: the pane inherits the tmux SERVER environment, not the
|
||||
# launcher's env. Even if a dir is on the launcher's PATH it must always be
|
||||
# baked into the pane's PATH export.
|
||||
#
|
||||
# We prove this by setting PATH to include FAKE_RUNTIME_BIN5 (the candidate),
|
||||
# then asserting the generated new-session command still exports it.
|
||||
TMUX_ARGS_FILE5=$(mktemp)
|
||||
FAKE_BIN5=$(mktemp -d)
|
||||
FAKE_RUNTIME_BIN5=$(mktemp -d) # this dir IS on the launcher's PATH below
|
||||
CLEANUP_DIRS+=("$FAKE_BIN5" "$FAKE_RUNTIME_BIN5")
|
||||
|
||||
cat > "$FAKE_BIN5/tmux" <<SHIM5
|
||||
#!/usr/bin/env bash
|
||||
subcmd="\$3"
|
||||
if [ "\$subcmd" = "has-session" ]; then exit 1; fi
|
||||
if [ "\$subcmd" = "new-session" ]; then
|
||||
printf '%s\n' "\$@" > "$TMUX_ARGS_FILE5"
|
||||
exit 0
|
||||
fi
|
||||
exit 0
|
||||
SHIM5
|
||||
chmod +x "$FAKE_BIN5/tmux"
|
||||
|
||||
SOCKET5="mosaic-agent-test5-$RANDOM-$$"
|
||||
AGENT5="agent5-$RANDOM"
|
||||
WORKDIR5=$(mktemp -d)
|
||||
CLEANUP_DIRS+=("$WORKDIR5")
|
||||
CLEANUP_SOCKETS+=("$SOCKET5")
|
||||
|
||||
# FAKE_RUNTIME_BIN5 is deliberately placed on the LAUNCHER PATH so that the
|
||||
# old (buggy) code would have skipped it. The correct code must still include
|
||||
# it in the pane PATH export.
|
||||
PATH="$FAKE_BIN5:$FAKE_RUNTIME_BIN5:$PATH" \
|
||||
MOSAIC_TMUX_SOCKET="$SOCKET5" \
|
||||
MOSAIC_AGENT_WORKDIR="$WORKDIR5" \
|
||||
MOSAIC_AGENT_RUNTIME="pi" \
|
||||
MOSAIC_RUNTIME_BIN="$FAKE_RUNTIME_BIN5" \
|
||||
MOSAIC_AGENT_COMMAND="mosaic yolo pi" \
|
||||
"$START" "$AGENT5"
|
||||
|
||||
all_args5=$(cat "$TMUX_ARGS_FILE5" 2>/dev/null || true)
|
||||
rm -f "$TMUX_ARGS_FILE5"
|
||||
rm -rf "$WORKDIR5"
|
||||
|
||||
echo "--- test 5: launcher-PATH candidate must still appear in pane export ---"
|
||||
echo "$all_args5"
|
||||
echo "--- end test 5 args ---"
|
||||
|
||||
echo "$all_args5" | grep -qF "export PATH=" || \
|
||||
fail "test5: pane command does not export PATH when candidate is on launcher PATH"
|
||||
echo "$all_args5" | grep -qF "$FAKE_RUNTIME_BIN5" || \
|
||||
fail "test5: candidate dir (already on launcher PATH) was NOT baked into pane PATH — regression"
|
||||
|
||||
echo "ok - start-agent-session"
|
||||
|
||||
Reference in New Issue
Block a user