Fix safety, security, and portability issues

[davidkreps]

• Back up existing files before overwriting with symlinks (safe_link helper)
• Trim git/ignore to OS/editor artifacts only; remove language-specific patterns
• Remove surprising merge.ff=false and merge.commit=no from git/config
• Guard cursor-shape escape sequences behind [[ -o interactive ]]
• Bind phpserver to 127.0.0.1 instead of LAN IP
• Remove pip='pip3' alias (breaks virtualenvs) and d='docker' alias (conflict risk)

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Claude finished @davidkreps's task in 2m 29s —— View job

---

PR Review

• Gather context: read changed files and diff
• Review against CLAUDE.md conventions
• Check test.sh coverage
• Post detailed feedback

---

Overall this is a well-reasoned cleanup. All six changes are correctly motivated and the implementation is sound. A few issues worth addressing before merge:

---

Issues

1. install.sh — safe_link silently overwrites .backup on repeated installs

install.sh:12–14

if [ -e "$link" ] && [ ! -L "$link" ]; then
    mv "$link" "${link}.backup"
fi

If a regular file exists at $link, we back it up. But if .backup already exists from a prior run, mv silently clobbers it. The scenario is rare (requires the user to have somehow replaced the symlink with a regular file between two installs), but it could silently destroy a backup. Consider appending a timestamp or checking for an existing backup: Fix this →

2. test.sh — no coverage for safe_link backup behavior

The idempotency test runs install.sh into a clean TEMP_HOME, so there are never pre-existing regular files at the link paths. The backup code path (-e "$link" && ! -L "$link") is never exercised. A test that places a regular file at a link target before calling install.sh would cover this. Fix this →

3. zsh/.zshrc:60–61 — edit-command-line setup is outside the interactive guard

autoload edit-command-line; zle -N edit-command-line
bindkey '^e' edit-command-line

These three statements are outside any [[ -o interactive ]] block, unlike every other zle/bindkey call in the file. zle -N and bindkey don't produce output in non-interactive shells, so this is harmless in practice and the test passes — but it's inconsistent with the pattern established in this very PR. Fix this →

---

Pre-existing issues (not introduced here, but worth noting)

4. zsh/aliases.zsh — tool aliases not guarded by command -v

Per CLAUDE.md: "Aliases and functions for optional tools must not error when the tool is missing." Aliases don't error on definition, but dc, dps, dim, k, kg, kd, kl are unconditionally defined for docker and kubectl. If neither is installed, these are dead aliases (harmless), but it's inconsistent with the pattern used for phpserver in functions.zsh. Not a blocker.

5. zsh/aliases.zsh:86 — trailing whitespace

alias nowdate='date +"%d-%m-%Y"' has a trailing space. Minor.

---

What looks correct

• safe_link logic: [ -e "$link" ] && [ ! -L "$link" ] correctly handles dangling symlinks (a broken symlink passes ! -L but fails -e, so it's left alone and overwritten with ln -sf — correct behavior).
• merge.ff=false / merge.commit=no removal: These are non-obvious global defaults that override per-repo merge strategies. Right call to remove them.
• git/ignore trim: Global gitignore should only contain universal artifacts (OS files, editors, env files). Language-specific patterns belong in per-repo .gitignore. The duplicate .DS_Store/Thumbs.db/*.swp entries are also cleaned up.
• Cursor-shape guard: The if [[ -o interactive ]] wrapper correctly prevents escape sequences on stdout during non-interactive sourcing. The test at line 240–241 of test.sh confirms this passes.
• phpserver → 127.0.0.1: Correct security fix; also simplifies the OS-detection code significantly.
• pip='pip3' removal: Correct — overriding pip globally breaks virtualenvs where pip correctly resolves to the venv pip.
• d='docker' removal: Correct — d is also a git alias in git/config:64; having both is confusing.
• Trailing newlines: All modified files end with a newline. File hygiene test covers this.
• Idempotency: safe_link is idempotent when the symlink already exists ([ ! -L "$link" ] is false, so we skip backup and ln -sf just re-creates the symlink). The test at line 209 confirms this.