Skip to content

fix(install): stop update_shell_rc from duplicating its PATH block forever - #943

Merged
agentforce314 merged 1 commit into
agentforce314:mainfrom
yibocai-airbnb:fix/install-rc-path-dedup
Sep 19, 2026
Merged

agentforce314 merged 1 commit into
agentforce314:mainfrom
yibocai-airbnb:fix/install-rc-path-dedup

Conversation

@yibocai-airbnb

Copy link
Copy Markdown
Contributor

Summary

update_shell_rc()'s dedup check is a real, deterministic bug that affects every user who runs install.sh more than once (reinstall, --local, update, etc.).

The check compares the shell-expanded real home directory:

grep -qF "$HOME/.local/bin" "$rc"   # double-quoted -> bash expands $HOME first

against the line the function itself appends, which comes from a single-quoted variable:

local path_line='export PATH="$HOME/.local/bin:$PATH"'

That line lands in the rc file as the literal, unexpanded text $HOME/.local/bin — a literal dollar sign, never the real path. The dedup check can therefore never match the installer's own prior write, and every re-run appends another duplicate RC_MARKER + PATH block, unbounded, forever.

Concretely reproduced this against a real .zshrc accumulating 8 duplicate blocks from repeated installs over the course of normal use.

Fix

Check for RC_MARKER (the literal marker line we actually write, guaranteed present on any rc we've already patched) plus the other real spellings ~/.local/bin can take in a pre-existing rc file: literal $HOME/.local/bin, ~/.local/bin, and the fully-expanded real path. This fixes the never-matching re-run case while preserving the original intent — a genuinely pre-existing PATH entry (in any of these forms) is still recognized and skipped.

Tests

New tests/test_install_sh_rc_dedup.py — extracts update_shell_rc() verbatim out of install.sh via sed (so it always exercises the real current source, not a hand-copied approximation) and exercises it against a fake $HOME:

  • single run patches exactly once
  • re-running does not duplicate (the core regression guard — fails against the pre-fix code: verified locally by reverting just the fix and confirming 3/5 tests fail with the exact duplication behavior described above)
  • 5 consecutive runs in one process still leave exactly one copy
  • a pre-existing ~/.local/bin (tilde form) is recognized and not re-patched
  • a pre-existing fully-expanded real-path form is recognized and not re-patched

No install.sh tests existed in the repo before this.

Test plan

  • python3 -m pytest tests/test_install_sh_rc_dedup.py -v — 5/5 pass against the fix
  • Reverted only the fix (kept the test) and confirmed 3/5 tests fail with the exact duplication bug described above
  • bash -n install.sh — syntax OK

🤖 Generated with Claude Code

…rever

The dedup check compared the shell-EXPANDED real home directory
(grep -qF "$HOME/.local/bin" "$rc", double-quoted so bash expands $HOME
before grep sees it) against the line the function itself writes from a
SINGLE-quoted variable, which lands in the rc file as the literal,
unexpanded text "$HOME/.local/bin" -- a literal dollar sign, never the
real path. The check could therefore never match the installer's own
prior write, so every re-run (reinstall, --local, update) appended
another duplicate marker+PATH block, unbounded, forever.

Now checks for RC_MARKER (the literal line we actually write) plus the
other real spellings ~/.local/bin can take in a pre-existing rc file
(literal '$HOME/.local/bin', '~/.local/bin', and the fully-expanded real
path), so a genuinely pre-existing PATH entry is still recognized and
skipped, matching the original intent -- only the installer's own
never-matching re-run case is fixed.
@github-actions

Copy link
Copy Markdown

Test Results

     5 files   1 018 suites   28m 1s ⏱️
15 867 tests 15 835 ✅ 22 💤 10 ❌
31 773 runs  31 697 ✅ 66 💤 10 ❌

For more details on these failures, see this check.

Results for commit d312a25.

@agentforce314

Copy link
Copy Markdown
Owner

LGTM. Thank you!

@agentforce314
agentforce314 merged commit bf08262 into agentforce314:main Sep 19, 2026
6 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants