Skip to content

Commit 86fe229

Browse files
committed
Fix what review found in the installers
- An MCP result row whose error text had braces was dropped, so a failure could read as a list of successes. Rows are now split where each one starts, and a failure no row accounts for gets its own warning. - A sign-in URL a client prints during `mapbox mcp install` was cut to the screen and erased with the spinner. Lines with a URL are now printed in full above it and stay. - On macOS, bash got a new ~/.bash_profile even next to an existing ~/.profile, which a login bash then stops reading. An existing .bash_profile, .bash_login or .profile is used instead. - "PATH is already set up" matched the install dir as any substring, so a comment or a longer dir (/x/bin2) skipped the profile line. It now needs the dir as a whole path, outside a comment. - The spinner ran under TERM=dumb, printing its cursor movement as text. - An interrupt now stops the spinner and erases its lines too. - install.ps1 no longer uses White, which all but vanishes on a light console theme. - README and CHANGELOG said the log has every command's output; it covers the commands run after the binary is installed.
1 parent 89ab960 commit 86fe229

5 files changed

Lines changed: 130 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,9 @@ that may never merge. They are not releases and are not listed here.
6666
`MAPBOX_CLI_NO_AGENT_SETUP=1` skips it along with the skill question.
6767

6868
- `install.sh` now writes a log of each run to
69-
`~/.local/state/mapbox-cli/install.log` (`$XDG_STATE_HOME` when set), with
70-
the full output of every command it ran, and keeps the previous run's as
69+
`~/.local/state/mapbox-cli/install.log` (`$XDG_STATE_HOME` when set):
70+
every step, and the full output of each `mapbox` and Tilesets command it
71+
ran after installing the binary. The previous run's is kept as
7172
`install.log.1`.
7273

7374
- New command: `mapbox styles download <style-id> > style.zip` saves a

‎README.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,10 @@ without being asked anything:
117117
curl -fsSL https://cli.mapbox.com/install.sh | MAPBOX_CLI_NO_AGENT_SETUP=1 MAPBOX_INSTALL_TILESETS=no sh
118118
```
119119

120-
`install.sh` records each run, including the full output of every command it
121-
ran, in `~/.local/state/mapbox-cli/install.log` (under `$XDG_STATE_HOME` when
122-
that is set), and keeps the previous run's as `install.log.1`.
120+
`install.sh` records each run in `~/.local/state/mapbox-cli/install.log`
121+
(under `$XDG_STATE_HOME` when that is set): every step, and the full output of
122+
each `mapbox` and Tilesets command it ran after installing the binary. The
123+
previous run's log is kept as `install.log.1`.
123124

124125
### Download the archive yourself
125126

‎scripts/install.ps1‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,14 @@
183183
# A line of detail under the step line before it, aligned with its text.
184184
function Write-Detail([string]$Text) { Write-Host "$($Layout.Indent) $Text" }
185185

186-
# A question reads in three layers, as in install.sh: the question in
187-
# white, what answering does in gray under it, and the command it runs in
188-
# cyan, so the eye lands on the question and the command.
186+
# A question reads in three layers, as in install.sh: the question in the
187+
# console's own text color, what answering does in gray under it, and the
188+
# command it runs in cyan, so the eye lands on the question and the
189+
# command. Not White: on a light console theme it all but disappears.
189190
function Write-AskTitle([string]$Text) {
190191
Write-Part ' '
191192
Write-Part '?' 'Cyan'
192-
Write-Part " $Text" 'White'
193+
Write-Part " $Text"
193194
Write-Host ''
194195
}
195196
# -Parts alternates plain note text and a command: note, command, note...
@@ -474,7 +475,7 @@ This channel is private. Set the credential and run it again:
474475
$shownVersion = $resolvedVersion
475476
if (-not $shownVersion) { $shownVersion = $Version }
476477
Write-Host ''
477-
Write-Part "Installing Mapbox CLI $shownVersion" 'White'
478+
Write-Part "Installing Mapbox CLI $shownVersion"
478479
Write-Part " for $platformName" 'DarkGray'
479480
Write-Host ''
480481
Write-Host ''
@@ -931,7 +932,7 @@ Windows 11 on Arm has and Windows 10 on Arm does not.
931932
Write-Part "$installedVersion is ready." 'Green'
932933
Write-Host ''
933934
Write-Host ''
934-
Write-Part 'Get started' 'White'
935+
Write-Part 'Get started'
935936
Write-Host ''
936937
foreach ($pair in @(
937938
@('mapbox auth login', 'Sign in to your Mapbox account'),

‎scripts/install.sh‎

Lines changed: 74 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -239,12 +239,30 @@ ask_cmd() { printf '%s%s%s%s' "$RESET" "$ACCENT" "$1" "$RESET$DIM"; }
239239
# either way, for a caller to show in full when it is the diagnosis.
240240
# <stdout-file>, when not empty, takes the command's stdout instead, for a
241241
# caller that parses it.
242+
# The child's own colors and control characters would throw off both the
243+
# width and the redraw, so only its text is shown.
244+
run_live_text() {
245+
sed "s/${ESC}\\[[0-9;?]*[A-Za-z]//g" | tr '\t' ' ' | tr -d '\000-\010\013-\037\177'
246+
}
247+
248+
# A line with a URL in it is usually one to open — a sign-in page — and the
249+
# spinner would cut it to the screen and erase it at the end. So each new one
250+
# is printed in full above the spinner, with line wrap back on for it, and
251+
# stays there.
252+
run_live_urls() {
253+
run_total="$(wc -l <"$run_log" 2>/dev/null | tr -d ' ')"
254+
[ -n "$run_total" ] && [ "$run_total" -gt "$run_seen" ] || return 0
255+
sed -n "$((run_seen + 1)),${run_total}p" "$run_log" | grep -E 'https?://' | run_live_text |
256+
while IFS= read -r run_url; do
257+
printf '\r\033[2K\n\033[2K\033[1A\r\033[?7h%s %s\n\033[?7l' "$STEP_INDENT" "$run_url"
258+
done
259+
run_seen="$run_total"
260+
}
261+
242262
# One frame of run_live: the spinner and label, then the log's latest line.
243263
run_live_draw() {
244-
# The child's own colors and control characters would throw off both
245-
# the width and the redraw, so only its text is shown.
246-
run_line="$(tail -n 1 "$run_log" 2>/dev/null | sed "s/${ESC}\\[[0-9;?]*[A-Za-z]//g" |
247-
tr '\t' ' ' | tr -d '\000-\010\013-\037\177' | cut -c "1-${run_width}")"
264+
run_live_urls
265+
run_line="$(tail -n 1 "$run_log" 2>/dev/null | run_live_text | cut -c "1-${run_width}")"
248266
printf '\r\033[2K%s%s%s%s %s\n\033[2K%s %s%s%s\033[1A\r' \
249267
"$STEP_INDENT" "$ACCENT" "$run_frame" "$RESET" "$run_label" "$STEP_INDENT" "$DIM" "$run_line" "$RESET"
250268
}
@@ -254,7 +272,8 @@ run_live() { # label log stdout-file command...
254272
shift 3
255273
: >"$run_log"
256274
[ -n "$run_stdout" ] || run_stdout="$run_log"
257-
if [ ! -t 1 ]; then
275+
# A dumb terminal would print the cursor movement as text, every tick.
276+
if [ ! -t 1 ] || [ "${TERM:-}" = dumb ]; then
258277
printf '%s%s %s\n' "$STEP_INDENT" "$MARK_WAIT" "$run_label"
259278
if [ "$run_stdout" = "$run_log" ]; then
260279
if "$@" </dev/null >>"$run_log" 2>&1; then run_status=0; else run_status=$?; fi
@@ -293,6 +312,7 @@ run_live() { # label log stdout-file command...
293312
TERM_MODES_CHANGED=yes
294313
(
295314
run_frame="${SPIN_FRAMES%% *}"
315+
run_seen=0
296316
while [ -f "$run_flag" ]; do
297317
# Word splitting is the point: one frame per word.
298318
# shellcheck disable=SC2086
@@ -577,7 +597,9 @@ cleanup() {
577597
# run_live hides the cursor and turns off line wrap while its spinner
578598
# runs; an interrupt must not leave the user's terminal that way.
579599
if [ "${TERM_MODES_CHANGED:-no}" = yes ]; then
580-
printf '\033[?7h\033[?25h'
600+
# Stopped first, or it can draw again after the lines are erased.
601+
kill "${run_spinner:-}" 2>/dev/null || true
602+
printf '\r\033[2K\n\033[2K\033[1A\r\033[?7h\033[?25h'
581603
fi
582604
if [ -n "$STAGED" ] && [ -e "$STAGED" ]; then
583605
rm -f "$STAGED"
@@ -730,16 +752,46 @@ case "${SHELL:-}" in
730752
*/zsh) profile="${ZDOTDIR:-$HOME}/.zshrc" ;;
731753
*/bash)
732754
# macOS Terminal opens login shells, which read .bash_profile and
733-
# never .bashrc.
755+
# never .bashrc — or rather the first of .bash_profile, .bash_login
756+
# and .profile that exists, and only that one. Creating a
757+
# .bash_profile next to someone's .profile would stop bash reading
758+
# their .profile at all, so an existing one wins.
734759
if [ "$(uname -s)" = Darwin ]; then
735760
profile="$HOME/.bash_profile"
761+
for candidate in .bash_profile .bash_login .profile; do
762+
if [ -f "$HOME/$candidate" ]; then
763+
profile="$HOME/$candidate"
764+
break
765+
fi
766+
done
736767
else
737768
profile="$HOME/.bashrc"
738769
fi
739770
;;
740771
*) profile="$HOME/.profile" ;;
741772
esac
742773

774+
# Whether a profile line other than a comment names <dir> as a whole path,
775+
# not as a prefix of another (/x/bin in /x/bin2) or a word inside one. Catches
776+
# a line the user wrote by hand, in either spelling of the directory.
777+
profile_names_dir() { # profile_names_dir <profile> <dir>
778+
awk -v d="$2" -v before=":=\"' ( " -v after=":\"' ) " '
779+
/^[ \t]*#/ { next }
780+
{
781+
off = 0
782+
rest = $0
783+
while ((i = index(rest, d)) > 0) {
784+
at = off + i
785+
b = at > 1 ? substr($0, at - 1, 1) : ""
786+
a = substr($0, at + length(d), 1)
787+
if ((b == "" || index(before, b)) && (a == "" || index(after, a))) { found = 1; exit }
788+
off = at
789+
rest = substr($0, at + 1)
790+
}
791+
}
792+
END { exit !found }' "$1" 2>/dev/null
793+
}
794+
743795
# yes when a profile now puts the install dir on PATH but this shell, which
744796
# a child process cannot change, does not have it yet.
745797
needs_new_shell=no
@@ -749,10 +801,11 @@ if [ "$install_dir_on_path" = no ]; then
749801
echo ""
750802
echo " echo '${path_line}' >> $(tildify "$profile")"
751803
echo ""
752-
# A second run must not append a second copy. Matching the directory as
753-
# well as the exact line also catches one the user wrote by hand.
804+
# A second run must not append a second copy.
754805
elif [ -f "$profile" ] &&
755-
{ grep -qF -- "$path_line" "$profile" || grep -qF -- "$INSTALL_DIR" "$profile"; } 2>/dev/null; then
806+
{ grep -qF -- "$path_line" "$profile" 2>/dev/null ||
807+
profile_names_dir "$profile" "$INSTALL_DIR" ||
808+
profile_names_dir "$profile" "$path_dir"; }; then
756809
step_ok "PATH is already set up in $(tildify "$profile")"
757810
needs_new_shell=yes
758811
elif mkdir -p "$(dirname "$profile")" 2>/dev/null &&
@@ -970,7 +1023,9 @@ mcp_field() { # mcp_field <key> <object>
9701023
printf '%s' "$2" | sed -n "s/.*\"$1\":\"\([^\"]*\)\".*/\1/p"
9711024
}
9721025
mcp_rows() { # mcp_rows <json>
973-
printf '%s' "$1" | grep -o '{[^{}]*}' | while IFS= read -r mcp_row; do
1026+
# "client" sorts first, so every row starts with it; splitting there
1027+
# keeps a row whole even when its "error" text has braces of its own.
1028+
printf '%s' "$1" | awk '{ gsub(/[{]"client":"/, "\n{\"client\":\""); print }' | while IFS= read -r mcp_row; do
9741029
mcp_s="$(mcp_field status "$mcp_row")"
9751030
mcp_v="$(mcp_field server "$mcp_row")"
9761031
mcp_c="$(mcp_field client "$mcp_row")"
@@ -1023,7 +1078,12 @@ if agent_setup_allowed; then
10231078
mcp_grouped "$(cat "${WORK_DIR}/mcp-install.json" 2>/dev/null)" >"${WORK_DIR}/mcp-rows"
10241079
# One line per server and outcome, so a server added just now
10251080
# reads differently from one that was already there.
1081+
mcp_trouble=no
10261082
while IFS='|' read -r status what who; do
1083+
case "$status" in
1084+
installed | already_installed) ;;
1085+
*) mcp_trouble=yes ;;
1086+
esac
10271087
case "$status" in
10281088
installed) step_ok "Added ${what} to ${who}" ;;
10291089
already_installed) step_skip "${what} was already in ${who}" ;;
@@ -1034,7 +1094,9 @@ if agent_setup_allowed; then
10341094
esac
10351095
done <"${WORK_DIR}/mcp-rows"
10361096
if [ "$mcp_ok" = no ]; then
1037-
[ -s "${WORK_DIR}/mcp-rows" ] || step_warn "Could not add the Mapbox MCP servers."
1097+
# A failure no row above accounts for still gets a line of
1098+
# its own, rather than a list of successes.
1099+
[ "$mcp_trouble" = yes ] || step_warn "Not every Mapbox MCP server could be added."
10381100
step_detail "Run 'mapbox mcp install --global' to see why and try again."
10391101
fi
10401102
else

‎scripts/test-install.sh‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -342,13 +342,16 @@ case "${1:-}" in
342342
fi
343343
echo '{"results":[{"client":"claude-code","server":"mapbox","status":"'"$status"'"},{"client":"claude-code","server":"mapbox-devkit","status":"'"$status"'"},{"client":"vscode","server":"mapbox","status":"'"$status"'"}]}'
344344
elif [ "$mcp_mode" = fail ]; then
345-
echo '{"results":[{"client":"claude-code","server":"mapbox","status":"installed"},{"client":"vscode","error":"boom","server":"mapbox","status":"failed"}]}'
345+
echo '{"results":[{"client":"claude-code","server":"mapbox","status":"installed"},{"client":"vscode","error":"--add-mcp rejected {\\"name\\":\\"mapbox\\"}","server":"mapbox","status":"failed"}]}'
346346
exit 1
347347
else
348348
# Like a real client's own output: colored, and longer than any
349349
# terminal is wide.
350350
printf '\033[2mFile modified: /%s/zz-end-of-long-line\033[0m\n' \
351351
"$(printf 'a%.0s' $(seq 1 200))" >&2
352+
printf 'Open https://auth.example.com/authorize?state=%s&end=zz-url-end\n' \
353+
"$(printf 'b%.0s' $(seq 1 120))" >&2
354+
echo 'waiting for sign-in' >&2
352355
# Mixed on purpose: added now, already there, and a row with an
353356
# "error" key, which sorts between "client" and "server" as the
354357
# real CLI's serde_json maps do.
@@ -1101,6 +1104,31 @@ export MAPBOX_NO_MODIFY_PATH=0
11011104
run_piped && status=0 || status=$?
11021105
expect_file "${HOME}/.zshrc" 'MAPBOX_NO_MODIFY_PATH=0 is not an opt-out'
11031106

1107+
start 'a mention of the dir that does not put it on PATH is not mistaken for one'
1108+
new_case_env off-path-near-miss
1109+
export SHELL=/bin/zsh
1110+
export MAPBOX_INSTALL_TILESETS=no
1111+
# The $PATH is meant literally: it is profile text.
1112+
# shellcheck disable=SC2016
1113+
printf '# %s is where mapbox goes\nexport PATH="%s2:$PATH"\nalias m=%s/mapbox\n' \
1114+
"$BIN_DIR" "$BIN_DIR" "$BIN_DIR" >"${HOME}/.zshrc"
1115+
run_piped && status=0 || status=$?
1116+
expect_status 0 "$status" 'exits 0'
1117+
expect_no_out 'already set up' 'a comment, a longer dir and a file under it are not the dir'
1118+
expect_in_file "${HOME}/.zshrc" "export PATH=\"${BIN_DIR}:\$PATH\"" 'so it adds the line'
1119+
1120+
if [ "$(uname -s)" = Darwin ]; then
1121+
start 'bash on macOS writes to an existing .profile rather than shadowing it'
1122+
new_case_env off-path-bash-profile
1123+
export SHELL=/bin/bash
1124+
export MAPBOX_INSTALL_TILESETS=no
1125+
echo 'export EDITOR=vi' >"${HOME}/.profile"
1126+
run_piped && status=0 || status=$?
1127+
expect_status 0 "$status" 'exits 0'
1128+
expect_in_file "${HOME}/.profile" "export PATH=\"${BIN_DIR}:\$PATH\"" 'writes the line into ~/.profile'
1129+
expect_no_file "${HOME}/.bash_profile" 'and creates no .bash_profile, which login bash would read instead'
1130+
fi
1131+
11041132
start 'a terminal gets color and NO_COLOR takes it away'
11051133
new_case_env color
11061134
export MAPBOX_INSTALL_TILESETS=no
@@ -1396,7 +1424,8 @@ expect_out 'This runs mapbox mcp install --global' 'names the command it runs'
13961424
expect_out 'This runs mapbox mcp install --global, now or any time later.' 'says it can be run later instead'
13971425
expect_out 'Add them now? [y/N]' 'asks about now'
13981426
expect_out 'Added Mapbox MCP to Claude Code' 'reports each server added now'
1399-
expect_out 'File modified: /aaaa' 'shows the command output live'
1427+
expect_out 'waiting for sign-in' 'shows the command output live'
1428+
expect_out 'zz-url-end' 'prints a URL in full, wider than the screen, so it can be opened'
14001429
expect_no_out 'zz-end-of-long-line' 'cut to the screen, so the spinner can redraw in place'
14011430
expect_out "$(printf '\033[?7l')" 'turns line wrap off while the spinner runs'
14021431
expect_out "$(printf '\033[?7h')" 'and back on after'
@@ -1455,6 +1484,17 @@ expect_status 0 "$status" 'mapbox itself is still installed'
14551484
expect_out 'Could not add Mapbox MCP to VS Code' 'reports which one failed'
14561485
expect_out 'Added Mapbox MCP to Claude Code' 'and still reports the one that worked'
14571486
expect_out "Run 'mapbox mcp install --global' to see why and try again." 'points at the command'
1487+
expect_out 'Could not add Mapbox MCP to VS Code' 'a failure whose error text has braces is still named'
1488+
1489+
start 'MCP, TERM=dumb: no spinner escape codes'
1490+
new_case_env mcp-dumb
1491+
export MAPBOX_CLI_BASE_URL="file://${AGENT_CHANNEL}"
1492+
export MAPBOX_INSTALL_TILESETS=no
1493+
export MAPBOX_TEST_MCP_MODE=fresh
1494+
TERM=dumb run_interactive "$(printf 'n\ny')" && status=0 || status=$?
1495+
expect_status 0 "$status" 'exits 0'
1496+
expect_out 'Added Mapbox MCP to Claude Code' 'still adds them'
1497+
expect_no_out "$(printf '\033[?25l')" 'without hiding the cursor or redrawing in place'
14581498

14591499
# --- result ----------------------------------------------------------------
14601500

0 commit comments

Comments
 (0)