diff --git a/.claude/rules/cli.rule.md b/.claude/rules/cli.rule.md index 051642fa..ac9f1b39 100644 --- a/.claude/rules/cli.rule.md +++ b/.claude/rules/cli.rule.md @@ -704,7 +704,9 @@ by re-reading the file and separates *applied* (0) from *could not write* (1) fr from this state* (2). Two rules live there rather than in any caller, so no writer can skip them: `_add` **refuses** a disabled path (appending under a winning `!` is the duplicate-pair bug), and `_remove` takes **both** line kinds, so de-registering a parked project leaves no `!` behind to -park whatever is claimed at that path next. `_enable` additionally collapses an existing duplicate +park whatever is claimed at that path next. `_add` also opens a line of its own for the entry it +writes: the readers keep a hand-edited last line that runs to EOF, so an entry appended straight on +would join two paths into a third naming no project, taking the one above it off the gate. `_enable` additionally collapses an existing duplicate pair to one live entry, in the earliest position it held. Before this, the same edit existed three times — an append here, a hand-escaped `sed -i` there, a diff --git a/.claude/rules/launch.rule.md b/.claude/rules/launch.rule.md index 6f269693..f0f843bc 100644 --- a/.claude/rules/launch.rule.md +++ b/.claude/rules/launch.rule.md @@ -296,7 +296,12 @@ launch path. The fragment is sourced per-account: `ai-tools-admin operator add` offers to add the guard line to the operator's `~/.bashrc` and `~/.bash_profile` **after** their nvm init, the one position where the ordering holds (the dedup must follow anything that prepends to PATH, and -non-login interactive shells read `~/.bashrc` only). Per-account wiring scopes the reorder +non-login interactive shells read `~/.bashrc` only). Those two files govern **bash**, so an +account whose login shell reads its own init instead is named in the enrolment output, with the +ordering left to the operator to place there. A `~/.bash_profile` the wiring creates opens with +the `. ~/.bashrc` block EL's skel carries, since bash reads that file alone at login: the +account's own init — its nvm init among it — stays read at login, and the guard line follows it. +Per-account wiring scopes the reorder to the operators who launch the agent: root and accounts unrelated to ai-tools keep their stock PATH, and ai-tools ships nothing into `/etc/profile.d`, keeping the host's every-login-shell code surface untouched. The sandbox account needs no wiring: diff --git a/.claude/rules/tests.rule.md b/.claude/rules/tests.rule.md index 6f92b4a0..62eb79b1 100644 --- a/.claude/rules/tests.rule.md +++ b/.claude/rules/tests.rule.md @@ -293,7 +293,13 @@ the account, since an administrator acts on that line at the moment of the decis function and the helper is **sourced** rather than run (its root check and its dispatch are guarded for exactly that), so one function is driven with no host to administer and nothing written anywhere; each case runs in its own `bash`, because the helper and the harness both -declare `SANDBOX_USER` readonly. +declare `SANDBOX_USER` readonly. Its second section covers the enrolment's other edit — the guard +line that sources the PATH dedup, which is what ranks the wrapper above the nvm shims — by driving +`wire_init_file` against fixture files in the testdir. Two of the three assertions are about a file +the command **creates**: `~/.bash_profile` is what bash reads at login, so the fixture home is run +through a real `bash -l` to assert the account's `.bashrc` is still read through it, and a file the +operator already has keeps its content and takes one guard line however often the accumulating +`operator add` runs. `services.sh` pins the service-health registry (`services.lib.sh`) that `ai-tools --status` and the launch wrapper's pre-launch warning share. Two properties carry weight beyond the accessors. diff --git a/docs/install-from-source.md b/docs/install-from-source.md index 74ef6e62..4f59849b 100644 --- a/docs/install-from-source.md +++ b/docs/install-from-source.md @@ -43,6 +43,11 @@ after your nvm init: # ai-tools PATH dedup (must follow nvm init) [[ -f /usr/local/lib/ai-tools/path-dedup.sh ]] && source /usr/local/lib/ai-tools/path-dedup.sh +Those two files are bash's, and `operator add` names your login shell when it +reads something else. The fragment sources cleanly under zsh, so the same line +goes in `~/.zshrc` and `~/.zprofile`; a shell that reads no bash (fish) takes +the same tier ordering in its own syntax. + nvm must be sourced **before** path-dedup: nvm prepends its versioned bin dir to `$PATH`, and path-dedup then restructures it into Tier 4, behind the T1 system bins (which include the wrapper) and T2 `~/.local/bin`. path-dedup.sh diff --git a/src/usr/local/lib/ai-tools/conf.lib.sh b/src/usr/local/lib/ai-tools/conf.lib.sh index e797dcff..8ee07ff5 100644 --- a/src/usr/local/lib/ai-tools/conf.lib.sh +++ b/src/usr/local/lib/ai-tools/conf.lib.sh @@ -585,17 +585,23 @@ ai_tools_conf_allowlist_state() { fi } -# ai_tools_conf_allowlist_add : append as an allow entry. A path -# already listed is left alone (a re-claim must not duplicate a line); a DISABLED path is -# refused with 2 rather than appended, because the appended line would not take effect. +# ai_tools_conf_allowlist_add : append as an allow entry, on a line +# of its own. A path already listed is left alone (a re-claim must not duplicate a line); a +# DISABLED path is refused with 2 rather than appended, because the appended line would not take +# effect. ai_tools_conf_allowlist_add() { - local file="$1" path="$2" + local file="$1" path="$2" line_break='' [[ -f "${file}" ]] || return 1 case "$(ai_tools_conf_allowlist_state "${file}" "${path}")" in listed) return 0 ;; disabled) return 2 ;; esac - printf '%s\n' "${path}" >> "${file}" 2>/dev/null || return 1 + # A hand-edited registry can run to EOF part-way through its last line, and every reader here + # keeps that entry (the read loops take a final unbroken line). So the append opens a new line + # first: written straight, it would join the two paths into a third that names no project, + # dropping the claimed one from the launch gate while the entry above it changed meaning. + [[ -n "$(tail -c 1 -- "${file}" 2>/dev/null)" ]] && line_break=$'\n' + printf '%s%s\n' "${line_break}" "${path}" >> "${file}" 2>/dev/null || return 1 ai_tools_conf_allowlist_has_entry "${file}" "${path}" || return 1 } diff --git a/src/usr/local/lib/ai-tools/path-dedup.sh b/src/usr/local/lib/ai-tools/path-dedup.sh index 8cc54e5d..b6f510c0 100644 --- a/src/usr/local/lib/ai-tools/path-dedup.sh +++ b/src/usr/local/lib/ai-tools/path-dedup.sh @@ -4,11 +4,13 @@ # entries of an operator shell and orders them so the root-owned system tiers # win first-match. That places /usr/local/bin/claude — the wrapper that # launches claude restricted — ahead of any nvm-managed claude, so typing -# `claude` always enters the sandbox. Sourced per-account: `ai-tools-admin -# operator add` wires it into the operator's ~/.bashrc and ~/.bash_profile -# after their nvm init (it must follow anything that prepends to PATH), which -# scopes the reorder to the operators who need it — root and unrelated -# accounts keep their stock PATH. The sandbox session needs no sourcing: +# `claude` in a shell that has sourced this enters the sandbox. Sourced +# per-account: `ai-tools-admin operator add` wires it into the operator's +# ~/.bashrc and ~/.bash_profile after their nvm init (it must follow anything +# that prepends to PATH), which scopes the reorder to the operators who need +# it — root and unrelated accounts keep their stock PATH. Those two are bash's +# init files; `operator add` names a login shell that reads its own instead, +# and the operator ranks the tiers there. The sandbox session needs no sourcing: # ai-tools-run pins the session PATH as a unit property. # # PATH is first-match-wins: an early directory shadows every later one. The diff --git a/src/usr/local/libexec/ai-tools/ai-tools-admin.sh b/src/usr/local/libexec/ai-tools/ai-tools-admin.sh index a1d40669..1b48861f 100755 --- a/src/usr/local/libexec/ai-tools/ai-tools-admin.sh +++ b/src/usr/local/libexec/ai-tools/ai-tools-admin.sh @@ -150,35 +150,61 @@ seed_allowlist() { rm -f "${tmp}" } -# wire_dedup : offer (interactively) to source the ai-tools PATH dedup from the operator's -# ~/.bashrc and ~/.bash_profile after their nvm init, so /usr/local/bin (the claude wrapper) -# wins over the nvm shim in every shell. This wiring is the dedup's only delivery: the file -# lives in the ai-tools lib dir, not /etc/profile.d, so unwired accounts keep their stock PATH. -# Edits the operator's home, so it asks first and never rewrites non-interactively; a piped run -# prints the line to add. +# The line an operator's bash init carries: sources the PATH dedup when it is installed, and +# leaves the shell's own PATH standing when it is not. readonly DEDUP_GUARD='[[ -f /usr/local/lib/ai-tools/path-dedup.sh ]] && source /usr/local/lib/ai-tools/path-dedup.sh || true' + +# wire_init_file [login-chain] : add the guard line to one bash init file, +# creating it owned by the account when it is absent. Idempotent -- a file already naming the +# fragment is left as it is. `login-chain` seeds a created file with the `. ~/.bashrc` block EL's +# skel carries, which the caller passes for ~/.bash_profile alone: bash reads that file by itself +# at login, so one holding only the guard line would leave a login shell without the account's own +# .bashrc, its nvm init among it. Top-level so tests/unit/admin-operator-add.sh drives it against +# its own fixture files, apart from the prompt in wire_dedup. +wire_init_file() { + local f="$1" user="$2" group="$3" seed="${4-}" + if [[ ! -e "${f}" ]]; then + install -o "${user}" -g "${group}" -m 644 /dev/null "${f}" || return 1 + [[ "${seed}" == login-chain ]] && printf '%s\n' \ + "# Created by ai-tools-admin: read this account's .bashrc at login." \ + 'if [ -f ~/.bashrc ]; then' ' . ~/.bashrc' 'fi' >> "${f}" + fi + if grep -qF '/usr/local/lib/ai-tools/path-dedup.sh' "${f}"; then + log "PATH dedup already present in ${f}"; return 0 + fi + grep -qF 'NVM_DIR' "${f}" \ + || log "note: NVM_DIR not found in ${f} -- path-dedup still works, but it is meant to follow your nvm init" + printf '\n# Added by ai-tools-admin: source the ai-tools PATH dedup (must follow nvm init).\n%s\n' \ + "${DEDUP_GUARD}" >> "${f}" + log "wired PATH dedup into ${f}" +} + +# wire_dedup : offer (interactively) to source the ai-tools PATH dedup from the operator's +# ~/.bashrc and ~/.bash_profile after their nvm init, so /usr/local/bin (the claude wrapper) wins +# over the nvm shim in the operator's bash shells. This wiring is the dedup's only delivery: the +# file lives in the ai-tools lib dir, not /etc/profile.d, so unwired accounts keep their stock +# PATH. Those two files are what bash reads, so an account that logs in through another shell is +# told where its own ordering stands. Edits the operator's home, so it asks first and never +# rewrites non-interactively; a piped run prints the line to add. wire_dedup() { - local user="$1" home group bashrc bashprof f + local user="$1" home group login_shell bashrc bashprof home="$(getent passwd "${user}" | cut -d: -f6)" + login_shell="$(getent passwd "${user}" | cut -d: -f7)" group="$(id -gn "${user}")" [[ -n "${home}" && -d "${home}" ]] || return 0 bashrc="${home}/.bashrc"; bashprof="${home}/.bash_profile" - _wire_one() { - f="$1" - [[ -e "${f}" ]] || install -o "${user}" -g "${group}" -m 644 /dev/null "${f}" - if grep -qF '/usr/local/lib/ai-tools/path-dedup.sh' "${f}"; then - log "PATH dedup already present in ${f}"; return - fi - grep -qF 'NVM_DIR' "${f}" \ - || log "note: NVM_DIR not found in ${f} -- path-dedup still works, but it is meant to follow your nvm init" - printf '\n# Added by ai-tools-admin: source the ai-tools PATH dedup (must follow nvm init).\n%s\n' \ - "${DEDUP_GUARD}" >> "${f}" - log "wired PATH dedup into ${f}" - } + # The two files below govern bash. Another login shell reads its own, so the operator hears + # which ordering their sessions actually get, at the moment the wiring is offered. + case "${login_shell}" in + */bash|'') ;; + *) log "note: ${user}'s login shell is ${login_shell}, which reads its own init files rather than ${bashrc} or ${bashprof}." + log " rank /usr/local/bin ahead of the nvm shims there too, so that typing claude reaches the ai-tools wrapper in that shell" ;; + esac if [[ -t 0 && -e /dev/tty ]]; then if ai_tools_msg_confirm \ "Wire the ai-tools PATH dedup into ${bashrc} and ${bashprof}?" y; then - _wire_one "${bashrc}"; _wire_one "${bashprof}" + wire_init_file "${bashrc}" "${user}" "${group}" + wire_init_file "${bashprof}" "${user}" "${group}" login-chain else log "skipped PATH dedup; add this line after your nvm init in ${bashrc} and ${bashprof}:" log " ${DEDUP_GUARD}" diff --git a/tests/unit/admin-operator-add.sh b/tests/unit/admin-operator-add.sh index edab9199..1b36dcd3 100644 --- a/tests/unit/admin-operator-add.sh +++ b/tests/unit/admin-operator-add.sh @@ -105,4 +105,60 @@ else fail "a host without sudo should report the host limit, got: ${out}" fi +# --- wire_init_file: the PATH dedup reaches the operator's bash init --- +# The guard line is what ranks /usr/local/bin (the wrapper) above the nvm shims, so a shell that +# never sources it resolves `claude` to the nvm-managed binary instead. Driven against fixture +# files in TESTDIR: the function takes the file as an argument, so no real home is touched. +section "ai-tools-admin operator add: bash init wiring (unit)" + +mktestdir +DEDUP_LINE="/usr/local/lib/ai-tools/path-dedup.sh" + +# wire_file [login-chain] : source the helper in a fresh shell and wire one fixture file. +wire_file() { + bash -c ' + set -euo pipefail + # shellcheck source=/dev/null + source "$1" + declare -F wire_init_file >/dev/null 2>&1 || { printf "NO SUCH FUNCTION\n"; exit 0; } + wire_init_file "$2" "$3" "$4" "${5-}" + ' _ "${HELPER}" "$1" "${PROJECTS_USER}" "${PROJECTS_GROUP}" "${2-}" 2>&1 || true +} + +out="$(wire_file "${TESTDIR}/.bashrc")" +if [[ "${out}" == *"NO SUCH FUNCTION"* ]]; then + fail "sourcing ${HELPER} did not define wire_init_file" + finish; exit +fi +if grep -qF "${DEDUP_LINE}" "${TESTDIR}/.bashrc"; then + pass "a created .bashrc carries the dedup guard line" +else + fail "a created .bashrc has no guard line: $(cat "${TESTDIR}/.bashrc")" +fi + +# A created .bash_profile opens with the .bashrc source EL's skel carries. bash reads +# .bash_profile ALONE at login, so one holding only the guard line leaves a login shell without +# the account's own init -- its nvm init among it, which the dedup is placed after. +wire_file "${TESTDIR}/.bash_profile" login-chain >/dev/null +printf 'export AI_TOOLS_TEST_MARKER=from_bashrc\n' >> "${TESTDIR}/.bashrc" +marker="$(HOME="${TESTDIR}" bash -lc 'printf "%s" "${AI_TOOLS_TEST_MARKER:-unset}"' 2>/dev/null || true)" +if [[ "${marker}" == "from_bashrc" ]]; then + pass "a login shell reads .bashrc through the created .bash_profile" +else + fail "the created .bash_profile left a login shell without .bashrc (marker '${marker}')" +fi + +# An init file the operator already has is appended to, never replaced, and a second run adds +# nothing: `operator add` is accumulating and idempotent, and this runs on every re-enrolment. +# shellcheck disable=SC2016 # the fixture's ${HOME} is init-file text, expanded by the shell reading it +printf '# my own bashrc\nexport NVM_DIR="${HOME}/.nvm"\n' > "${TESTDIR}/.bashrc" +wire_file "${TESTDIR}/.bashrc" >/dev/null +out="$(wire_file "${TESTDIR}/.bashrc")" +if [[ "$(grep -cF "${DEDUP_LINE}" "${TESTDIR}/.bashrc")" == 1 \ + && "${out}" == *"already present"* ]] && grep -qF '# my own bashrc' "${TESTDIR}/.bashrc"; then + pass "an existing init file keeps its content and takes one guard line" +else + fail "re-wiring changed an existing file:"$'\n'"$(cat "${TESTDIR}/.bashrc")" +fi + finish diff --git a/tests/unit/conf.sh b/tests/unit/conf.sh index e3dbdfde..5d2810b8 100644 --- a/tests/unit/conf.sh +++ b/tests/unit/conf.sh @@ -443,6 +443,7 @@ check_entry "an unmatched quote is taken as-is" '/home/me/project' '"/h # saying something other than what the caller was told: # * the three-state read, where a DISABLED project used to read as absent; # * add refusing to append under a winning '!' (the duplicate-pair bug); +# * add opening a line of its own, so a file that runs to EOF mid-line keeps that entry; # * remove taking BOTH line kinds, so no '!' is left to park the next claim at that path; # * enable/disable preserving position, indentation and comment -- their reason to exist # rather than being an add+remove pair, for an operator whose allowlist is an ordered, @@ -500,6 +501,14 @@ if [[ "$(grep -cxF "${P1}" "${AL}")" == 1 ]]; then else fail "add duplicated the line ($(grep -cxF "${P1}" "${AL}") copies)" fi +# A hand-edited registry can run to EOF part-way through its last line, and the readers keep that +# entry, so the append opens a line of its own for the new one. Written straight it would join the +# two paths into a third naming no project, taking the entry above it off the launch gate. +printf '%s\n%s' "# header" "${P2}" > "${AL}" +rc_is 0 "add opens a line for an entry that runs to EOF" ai_tools_conf_allowlist_add "${AL}" "${P1}" +state_is listed "${P1}" "the added path reads as listed" +state_is listed "${P2}" "the entry that ran to EOF is still listed" + seed_al "# header" "!${P1}" rc_is 2 "add REFUSES a disabled path" ai_tools_conf_allowlist_add "${AL}" "${P1}" state_is disabled "${P1}" "the refused add left the path disabled"