Skip to content

feat(install): prefer ~/bin when already on PATH (B2, RFC 0001) - #392

Merged
shujaatTracebloc merged 5 commits into
developfrom
fix/cli-install-prefer-onpath-dir
Jul 23, 2026
Merged

feat(install): prefer ~/bin when already on PATH (B2, RFC 0001)#392
shujaatTracebloc merged 5 commits into
developfrom
fix/cli-install-prefer-onpath-dir

Conversation

@LukasWodka

@LukasWodkaLukasWodka commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

What (B2, cli side)

The RFC 0001 B2 fix, step 1: prefer a writable dir already on $PATH so the CLI is usable in the same terminal.

When /usr/local/bin isn't writable, the installer fell straight to ~/.local/bin — which on many setups isn't on the current shell's $PATH, so tracebloc isn't resolvable until a new terminal (or an rc reload). This adds one step before that fallback:

  • If ~/bin already exists, is on $PATH, and is writable → install there. The binary is then usable in this shell and every new one, with no rc edit and no new terminal.
  • Otherwise → the existing ~/.local/bin fallback + shell-rc persistence, unchanged.

Deliberately restricted to the conventional general-purpose ~/bin — never a language-specific dir (~/.cargo/bin, ~/go/bin, …) that merely happens to be on $PATH, which would be a surprising place to drop tracebloc.

Safety / tests

  • POSIX sh; shellcheck --shell=sh --severity=error (CI's invocation) and--severity=warning clean; dash -n clean.
  • The ~/.local/bin fallback path — what the cross-repo fresh-shell PATH guard (install-path-persist.yml → client's path-persist.sh) exercises — is untouched. Labeled e2e so that guard runs against this PR and confirms no PATH-persistence regression.

Scope

The cli half of the least-privilege install's B2. The client half (honest final CTA — never tell the user to run a command this shell can't find) is tracebloc/client#371.

Part of the least-privilege install epic — tracebloc/backend#1168, ticket tracebloc/backend#1174.

🤖 Generated with Claude Code


Note

Low Risk
Installer-only UX and fallback logic; the ~/.local/bin path and security-sensitive download/verify flow are unchanged.

Overview
When the chosen install prefix isn’t writable, the installer now tries writable ~/bin that’s already on $PATH before falling back to ~/.local/bin, so tracebloc can work in the current shell without an rc change or new terminal (RFC 0001 B2). That path is limited to conventional ~/bin (not tool-specific dirs like ~/.cargo/bin), with guards for empty//HOME and trailing-slash PATH entries.

PATH detection for “already on PATH” and “needs rc persistence” now treats $dir/ the same as $dir. Post-install messaging uses a new on_path flag so users see “ready to use now” when the install dir is already on the current shell’s PATH, instead of being told to open a new terminal.

Reviewed by Cursor Bugbot for commit d4035fc. Bugbot is set up for automated code reviews on this repo. Configure here.

…se (RFC 0001 B2)
When /usr/local/bin is not writable, the installer fell straight to
~/.local/bin, which is often not on the current shell PATH — so tracebloc was
not resolvable until a new terminal or an rc reload. Now, before that fallback,
prefer ~/bin when it already exists on $PATH and is writable: the binary is
usable in this shell and every new one with no rc edit at all. Restricted to the
conventional ~/bin (never a language-specific dir like ~/.cargo/bin that merely
happens to be on PATH); the ~/.local/bin fallback + rc persistence are unchanged.
POSIX sh; shellcheck --severity=warning + dash -n clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodkaLukasWodka added the e2e Run the kind e2e integration suite on this PR label Jul 22, 2026
@LukasWodkaLukasWodka self-assigned this Jul 22, 2026
Comment threadscripts/install.sh
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 878a7fd. Configure here.

…nudge (Bugbot #392)
When the installer prefers ~/bin because it is ALREADY on $PATH, the binary is
usable immediately and in the user's shells (they configured ~/bin), so the
persist step must not rewrite their rc or tell them to open a new terminal — that
undercut the B2 goal. Flag that selection (PREFIX_PRESELECTED_ON_PATH) and skip
persist for it, same clean no-message outcome as an on-PATH /usr/local/bin. The
~/.local/bin fallback (created mid-session, needs the rc line for non-login
shells, #304) is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/install.sh Outdated
…sage says "ready now" (Bugbot #392 r2)
My earlier skip-persist for ~/bin broke a SESSION-ONLY $PATH entry (direnv, a
one-off export): those are not in the rc, so new terminals lost tracebloc with no
guidance. Revert to always persisting a $HOME prefix (idempotent — a no-op when
the rc already has it), so new terminals are covered. To still honour B2 (do not
nag "open a new terminal" for a dir usable NOW), the message branches on whether
$PREFIX is on the current $PATH: "ready to use now" (+ note the rc was updated for
new terminals) instead of "open a new terminal".
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

Comment threadscripts/install.sh
Comment threadscripts/install.sh
…ailure too (Bugbot #392 r3)
1) "${HOME%/}/bin" collapses to /bin when HOME is "/" or empty, so a root
process could drop the CLI into /bin — require a real, non-root $HOME before
preferring ~/bin.
2) The on_path "ready now" acknowledgement was applied to the added/present
messages but not the rc-write-failure branch; a ~/bin (usable now) whose rc
could not be written still nagged "open a new terminal". Add it there too.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

divyasinghds
divyasinghds previously approved these changes Jul 23, 2026

@divyasinghdsdivyasinghds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve. Prefers ~/bin only when on PATH + existing + writable (conservatively excludes language-specific dirs); rc-persist is idempotent (comment-stripped, case-insensitive PATH-op match with a [^A-Za-z_] guard) and messages are honest in all three states. Well-reasoned.

Comment threadscripts/install.sh Outdated

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d5881bf. Configure here.

The PATH membership checks matched only ":$dir:" and missed a
trailing-slash entry like "$HOME/bin/". A user with "~/bin/" on PATH would
be wrongly classified as not-on-PATH: the installer would skip the usable
~/bin prefix and fall back to ~/.local/bin, and would nag "open a new
terminal" for a dir that is in fact already on PATH (Bugbot #392).
Add a "|*":$dir/:"*" alternative to all three checks (home_bin detection,
persist decision, on_path message).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@LukasWodka

Copy link
Copy Markdown
ContributorAuthor

bugbot run

@cursorcursorBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Fix All in Cursor

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit d5881bf. Configure here.

@divyasinghdsdivyasinghds left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve (re-review after force-push). The refinements since my first approval are all sound, no regressions:

  • Trailing-slash tolerance in the ~/bin-on-PATH detection (*":$home_bin/:"*), applied consistently to the persist and on_path checks too — a $HOME/bin/ PATH entry no longer wrongly falls back to ~/.local/bin.
  • HOME=""/HOME="/" guard before preferring ~/bin — prevents a root process (HOME=/ → ${HOME%/}/bin = /bin) from dropping the CLI into /bin.
  • Messaging now keys off on_path across added/present/failed states, so it says "ready to use now" instead of nagging "open a new terminal" when the dir is already on PATH.

Core logic (mkdir-then-fallback, idempotent rc write) unchanged and still correct. CI green. LGTM.

LukasWodka added a commit that referenced this pull request Jul 30, 2026
… second (#433) (#434)
* fix(install): own ONE PATH block in the shell profile, never append a second (#433)
`scripts/install.sh` guarded its profile append with "does the rc mention
$PREFIX", so an install with a DIFFERENT prefix appended another block every
time and the profile grew without bound. The v0.10.1 validation box collected
TEN blocks, each naming a temp dir that no longer existed.
The polluter was our own test harness: scripts/tests/install-verify.sh mints a
fresh mktemp --prefix per case and never sandboxed $HOME, so it wrote 3 blocks
into the developer's real ~/.bash_profile per invocation.
- Tag our block with a stable $PATH_MARKER and REPLACE it on a re-run instead of
stacking a second one. A profile already polluted by an older installer
collapses to one block on the next install.
- Decide "is $PREFIX already handled?" from the rc with our block stripped out,
so the answer comes from the user's own lines only — that is what makes the
write idempotent.
- Compare whole path COMPONENTS, not substrings. `grep -F "$PREFIX"` matched
--prefix /opt/tb against an existing /opt/tb2 line and then claimed "already in
your PATH config" for a directory that was on nobody's PATH.
- Append when there is nothing of ours to clean up; only rewrite the file when a
stale block must go. The rewrite truncates in place, so the inode, mode and
owner survive and an rc symlinked into a dotfiles repo is written THROUGH
rather than replaced by a regular file.
- Removal only takes the line under the marker when it is shaped like a PATH op
we wrote, so a dangling marker can never eat unrelated user content.
- Quote the fish line (`fish_add_path "$PREFIX"`), matching the client
installer's hint, so a prefix containing a space survives.
- Sandbox $HOME in install-verify.sh and add 7 assertions: same prefix x3 → one
block; three different prefixes → one block naming the newest; prefix already
on PATH → no rc written at all; unrelated lines preserved byte-for-byte;
dangling marker harmless; zsh and fish route to the right rc.
A prefix already on $PATH still writes nothing (unchanged), and a $HOME prefix
is still persisted even when it looks on-PATH — that hit can be session-only
(Bugbot #392 r2), and it now costs at most one line rather than one per run.
Fixes#433
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(install): a same-prefix re-run must not rewrite the rc at all
The first pass consolidated correctly but reached the replace path even when our
one block already said exactly what this run would write — so a re-install, and
every `tracebloc upgrade` (which re-execs this installer), rewrote the user's
profile byte-for-byte and reported "Updated the tracebloc PATH entry".
Recognise that case and leave the file completely alone: count our blocks, and
when there is exactly one whose PATH line already matches, report `present`
("already in your PATH config — nothing to add") without touching the rc.
Asserted: re-install with the same prefix leaves the rc byte-identical.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(install): restore the rc if the rewrite dies partway
The redirection in replace_rc truncates before cat writes, so a write that died
halfway (no space left, a vanishing mount) would leave the user's profile in
pieces. Put the original contents back on failure — best effort, but far better
than a half-written file we don't own. The caller already reports `failed` and
prints the line to add by hand.
Verified with a read-only rc: the file keeps its user content and its previous
block, and the manual-add advice is printed.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(install): require positive proof a PATH line is ours before removing it (Bugbot)
Bugbot on #434: the marker alone was treated as proof of ownership, so a marker
left dangling by a hand-edit directly above the USER's own PATH export would take
that export with it on the next install — silently deleting a PATH entry we never
added, while still reporting success.
Removal now demands positive evidence about the directory the following line
names (_tb_owns_dir):
* a '$' anywhere -> never ours. We always write a literal, expanded path;
"$HOME/mytools" is the user's own idiom. This kills the
most realistic form of the bug.
* == the prefix we are installing to now -> ours
* a directory that no longer exists -> ours (the #433 cruft)
* a directory still holding a tracebloc binary -> ours (a prior install)
Anything else is the user's line: we drop only the orphaned marker comment and
leave their PATH op alone. The bias is deliberate — for an installer editing a
file it does not own, failing to clean one line is much cheaper than deleting a
PATH entry someone depends on. The residue is bounded: at most one unmarked line
per pathological cycle, never renewed growth.
The pair-then-verdict pass goes through a temp file rather than a pipe (so the
verdicts land in this shell) and rather than a heredoc (so the awk program needs
no nested-expansion escaping).
3 new assertions, 23/23 in install-verify.sh: a dangling marker above the user's
own PATH export keeps it, likewise with an unexpanded $HOME, and a block naming a
vanished directory is still cleaned up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* style(install): state the empty-prefix exit status outright
Bugbot read the bare `exit` in the BEGIN rule as exiting 0, i.e. 'prefix
already listed', which would skip persistence. It does not: a bare exit in
BEGIN still runs END, so the status is END's exit(found ? 0 : 1) = 1, the
fail-safe direction. Verified 1 on BSD awk and by the POSIX rule.
Making it `exit 1` costs nothing and means the intent cannot be misread by
a later reader or a stricter awk, rather than resting on the fallthrough.
* fix(install): prove ownership from the marker, and stop mislabelling a failed tidy-up (Bugbot)
Two Medium findings, both in the ownership/reporting logic.
1. _tb_owns_dir claimed ANY non-directory as ours, so a dangling marker above the
user's own literal-path line for a directory they hadn't created yet was
stripped as an owned block — deleting a PATH entry we never wrote, which is not
recoverable.
The marker we write now records the directory:
# Added by the tracebloc CLI installer (prefix: /opt/tracebloc)
export PATH="/opt/tracebloc:$PATH"
When the recorded prefix and the directory on the line below agree, we provably
wrote both halves, so the block is reclaimable even if that directory has since
been deleted — which is what keeps the #433 cleanup working, ten blocks and all.
$PATH_MARKER stays the stable BEGINNING of the line (matched literally via
index(), never as a whole line) so older blocks are still recognised.
A legacy marker that recorded nothing can now only be claimed when the
directory still holds a tracebloc binary, or is the prefix being installed to.
A legacy marker over a vanished directory is indistinguishable from the user's
own entry for a directory they plan to create, so we leave it alone. That gives
up auto-healing pre-#433 cruft in exactly one case; Lukas's profile was already
cleaned by hand, so that is a nice-to-have, whereas deleting someone's PATH line
is not.
Blocks we cannot claim are now left ENTIRELY intact, comment included, rather
than losing their marker: a dead line still labelled "Added by the tracebloc CLI
installer" tells the user what it is, a bare one doesn't.
2. A failed replace_rc reported `failed` even when the user's own line already
persisted the prefix and all we'd failed to do was drop a redundant block — so
the installer told them to hand-edit their profile while their PATH was in fact
correct. Split into `tidied`/`tidy_failed` (PATH is right; at most a cosmetic
note) versus `failed` (prefix genuinely not persisted; manual instruction
warranted), and the write decision now hinges on content equality (rc_same), so
a run that changes nothing writes nothing.
rc_same deliberately avoids cmp(1) — it lives in diffutils, which a minimal
container image can lack, and a missing tool must not silently turn "leave the
file alone" into "rewrite it every run".
Also restores the `mkdir -p "$(dirname "$rc")"` that the state-machine rewrite
dropped; without it fish's ~/.config/fish/config.fish could never be created. The
matrix caught it.
install-verify.sh: 29/29, with six new cases — legacy marker can't claim a missing
dir, nor one without our binary; a recorded-prefix block naming a vanished dir is
still cleaned; ten of them collapse to one; an unclaimable legacy block is kept
without churning the rc; a failed tidy-up never asks for a manual PATH line, while
a real failure still does.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2eRun the kind e2e integration suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@LukasWodka@divyasinghds@shujaatTracebloc