From d27d9a081977be2ad992aa467036a583d246e75c Mon Sep 17 00:00:00 2001 From: Daveswo <969dwi@gmail.com> Date: Tue, 18 Aug 2026 01:48:53 -0400 Subject: [PATCH] Fix code-review findings: 10-known.sh value-match bug (proven live), bootstrap.sh symlink race + missing sources + accidental exit-on-eval, fx.sh misleading picker text + FX_AUTO short-circuit --- contrib/fx/bootstrap.sh | 38 +++++++++++++++++++++++++------- contrib/fx/fx.sh | 27 +++++++++++++++++++---- contrib/fx/sources.d/10-known.sh | 15 ++++++++++++- 3 files changed, 67 insertions(+), 13 deletions(-) diff --git a/contrib/fx/bootstrap.sh b/contrib/fx/bootstrap.sh index d3c3f05..8f3189e 100755 --- a/contrib/fx/bootstrap.sh +++ b/contrib/fx/bootstrap.sh @@ -11,15 +11,37 @@ # them in every new shell, add the same eval line to your rc file. _base="https://huggingface.co/spaces/Daveswo/self-healing-autopoietic-shell/raw/main" -_dir="${TMPDIR:-/tmp}/self-healing-shell-$$" -mkdir -p "$_dir/sources.d" 2>/dev/null +# mktemp -d, not `mkdir -p .../self-healing-shell-$$`: a PID-based name +# under shared, world-writable /tmp is guessable, and `mkdir -p` follows +# a pre-existing symlink at that path without complaint -- an attacker +# who plants one before this runs gets the curl-fetched files written +# through it. mktemp -d always creates a fresh, unpredictable directory +# or fails outright; it never silently reuses an existing path. +_dir="$(mktemp -d "${TMPDIR:-/tmp}/self-healing-shell-XXXXXX" 2>/dev/null)" -_fetch_ok=1 -curl -fsSL "$_base/src/f.sh" -o "$_dir/f.sh" || _fetch_ok=0 -curl -fsSL "$_base/contrib/fx/fx.sh" -o "$_dir/fx.sh" || _fetch_ok=0 -for s in 10-known.sh 20-history.sh 30-selfdiag.sh 40-pathfuzzy.sh 50-thefuck.sh 90-team-shared.sh; do - curl -fsSL "$_base/contrib/fx/sources.d/$s" -o "$_dir/sources.d/$s" || _fetch_ok=0 -done +# This runs via `eval` inline in your live interactive shell -- never +# `exit`/`return` on failure here, that would close your terminal, not +# just abort the script. Fall through to the same "skip the rest" +# if/else the original script already used. +if [ -z "$_dir" ] || [ ! -d "$_dir" ]; then + echo "bootstrap: mktemp -d failed" >&2 + _fetch_ok=0 +else + mkdir -p "$_dir/sources.d" 2>/dev/null + # The directory has to outlive this script -- FX_SOURCES_DIR keeps + # pointing into it for the rest of the shell session -- so it can't + # be removed right after sourcing. Clean it up when the shell + # itself exits instead, so repeated bootstrapping doesn't + # accumulate copies in /tmp. + trap 'rm -rf "$_dir"' EXIT + + _fetch_ok=1 + curl -fsSL "$_base/src/f.sh" -o "$_dir/f.sh" || _fetch_ok=0 + curl -fsSL "$_base/contrib/fx/fx.sh" -o "$_dir/fx.sh" || _fetch_ok=0 + for s in 10-known.sh 20-history.sh 30-selfdiag.sh 40-pathfuzzy.sh 50-thefuck.sh 70-local-llm.sh 80-remote-api.sh 90-team-shared.sh; do + curl -fsSL "$_base/contrib/fx/sources.d/$s" -o "$_dir/sources.d/$s" || _fetch_ok=0 + done +fi if [ "$_fetch_ok" != 1 ]; then echo "bootstrap: fetch failed, check your connection" >&2 diff --git a/contrib/fx/fx.sh b/contrib/fx/fx.sh index 1887c87..fdd028c 100755 --- a/contrib/fx/fx.sh +++ b/contrib/fx/fx.sh @@ -62,13 +62,27 @@ fx() { fi export FX_OUTPUT + # Under FX_AUTO only the first candidate is ever used (head -1 + # below), so gathering every source unconditionally would mean + # always paying for the 25-30s local-LLM/remote-API tiers even + # when a free source already answered and that answer is + # guaranteed to win -- exactly the cost the NN- numbering claims to + # avoid. Stop at the first source that produces anything once + # FX_AUTO is set. Without it (the interactive picker), keep + # gathering everything: a later, better-verified candidate + # shouldn't be hidden behind an earlier weak match when a human is + # the one choosing. candidates=$( for src in "$FX_SOURCES_DIR"/*; do [ -x "$src" ] || continue label="${src##*/}" - "$src" "$cmd" 2>/dev/null | while IFS= read -r line; do - [ -n "$line" ] && printf '[%s] %s\n' "$label" "$line" - done + out=$("$src" "$cmd" 2>/dev/null) + if [ -n "$out" ]; then + printf '%s\n' "$out" | while IFS= read -r line; do + [ -n "$line" ] && printf '[%s] %s\n' "$label" "$line" + done + [ -n "$FX_AUTO" ] && break + fi done | awk -F'] ' '!seen[$2]++' ) @@ -83,7 +97,12 @@ fx() { printf '%s\n' "$candidates" | while IFS= read -r line; do i=$((i+1)); printf '%d) %s\n' "$i" "$line" done - printf 'pick a number (blank to type your own): ' + # "blank to give up", not "type your own": fx already + # committed f's stdin to this pipe, so a blank answer here + # can't fall through to a real terminal read the way f()'s + # own bare prompt does -- it just pipes an empty line into + # f(), same as when zero candidates were found at all. + printf 'pick a number (blank to give up): ' read -r n [ -n "$n" ] && chosen=$(printf '%s\n' "$candidates" | sed -n "${n}p" | sed 's/^\[[^]]*\] //') fi diff --git a/contrib/fx/sources.d/10-known.sh b/contrib/fx/sources.d/10-known.sh index 112fd64..6d4cf54 100755 --- a/contrib/fx/sources.d/10-known.sh +++ b/contrib/fx/sources.d/10-known.sh @@ -2,8 +2,21 @@ # Fuzzy-known source: surfaces fixes already taught for a command that # *starts the same way* as this one, since f()'s own p lookup only ever # matches the exact, literal string. +# +# Matches only against the KEY (left of "="), not the whole line -- an +# earlier version used `grep -F "$first_word " p`, which also matched +# inside a fix's *value*. Proven wrong live: with p containing +# `gti --version=gti(){ command git "$@"; }`, querying "git push" (never +# taught) matched that line anyway, because "git " appears inside the +# value text, not because the key is related. [ -f p ] || exit 0 cmd="$1" first_word=$(printf '%s' "$cmd" | awk '{print $1}') [ -n "$first_word" ] || exit 0 -grep -F "${first_word} " p 2>/dev/null | cut -d= -f2- | awk '!seen[$0]++' +awk -v fw="$first_word " ' + substr($0, 1, length(fw)) == fw { + line = $0 + sub(/^[^=]*=/, "", line) + if (!seen[line]++) print line + } +' p 2>/dev/null