fix(install): only manage strix in the installer's own directory by itzzdev09 · Pull Request #1296 · usestrix/strix

itzzdev09 · 2026-09-12T03:30:38Z

Problem

The installer deletes strix executables it does not own.

check_existing_installation enumerates which -a strix and rm -fs every match outside $INSTALL_DIR. If the path merely contains .local/bin it also runs pipx uninstall strix-agent.
verify_installation deletes whatever executable wins PATH resolution when that is not the managed one.

Path discovery shows another strix exists; it does not establish that the installer owns it. A pipx install, a uv tool shim, or a development checkout on PATH gets removed with no prompt.

I confirmed this on the current script with the test harness in tests/test_install_script.py: with an unrelated strix ahead of the installer on PATH, the file is gone after the run, and in the .local/bin case pipx uninstall strix-agent is invoked too.

Change

The installer now writes only inside $INSTALL_DIR.

check_existing_installation reports other executables instead of removing them, and says plainly that only $INSTALL_DIR is managed.
verify_installation explains that the newly installed binary will not run until the conflict is resolved, and offers the two ways to fix it — reorder PATH, or remove the other file. A new describe_removal helper prints the right command (pipx uninstall strix-agent for a pipx-shaped path, otherwise rm <path>). Nothing is deleted and pipx is never invoked.

Tests

Two regression tests, built on the existing sandboxed installer harness:

test_installer_leaves_unrelated_strix_executables_alone — a decoy strix ahead on PATH survives byte-for-byte and the managed binary still installs.
test_installer_does_not_uninstall_a_pipx_managed_strix — a decoy under a .local/bin-shaped directory survives and a mock pipx records no invocation.

Both fail on main and pass with this change. bash -n clean; ruff, ruff format, and mypy clean on the touched test module.

🤖 Generated with Claude Code

`check_existing_installation` walked `which -a strix` and deleted every match outside `$INSTALL_DIR`, and `verify_installation` deleted whatever executable won PATH resolution. Path discovery shows that another `strix` exists; it does not show that the installer owns it. A pipx install or a development checkout on `PATH` was removed without being asked about, including a `pipx uninstall strix-agent` triggered purely by the path containing `.local/bin`. The installer now touches only `$INSTALL_DIR`. Other executables are reported, and when one wins PATH resolution the user is told how to reorder `PATH` or remove it themselves. Fixes usestrix#1262 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

greptile-apps · 2026-09-12T03:32:33Z

Confidence Score: 3/5

The PR should not merge until its manual removal guidance reliably identifies pipx ownership and safely escapes filesystem paths.

Findings

Pipx ownership is misidentified ▶
Removal path is unescaped ▶

Fix with agent prompt

### Issue 1
scripts/install.sh:105-106
If a manually installed or uv-managed `strix` is in a path containing `.local/bin` while `pipx` is available, this code labels it as a pipx installation without confirming ownership. Following the suggested command can leave the actual PATH conflict unresolved or uninstall a separate pipx-managed `strix-agent`. Please determine ownership through pipx itself or provide neutral removal guidance.

### Issue 2
scripts/install.sh:108
If the resolved executable path contains spaces or glob characters, copying this displayed `rm` command causes the shell to split or expand the path. The command can then fail to remove the conflicting executable or target unintended matching files. Please print a shell-escaped path.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Records and reports unmanaged installations rather than modifying them.
Adds manual PATH-conflict remediation guidance during verification.
Adds regression coverage ensuring unrelated and .local/bin-located executables survive installation.
The new remediation guidance still misidentifies pipx ownership and emits an unsafe, unescaped removal command.

_{Reviews (1) · Last reviewed commit: "fix(install): only manage strix in the i..."}

greptile-apps · 2026-09-12T03:32:36Z

+    if [[ "$path" == *".local/bin"* ]] && command -v pipx >/dev/null 2>&1; then
+        echo -e "${MUTED}  It looks like a pipx installation. To remove it: ${NC}pipx uninstall strix-agent"


Pipx ownership is misidentified

If a manually installed or uv-managed strix is in a path containing .local/bin while pipx is available, this code labels it as a pipx installation without confirming ownership. Following the suggested command can leave the actual PATH conflict unresolved or uninstall a separate pipx-managed strix-agent. Please determine ownership through pipx itself or provide neutral removal guidance.

Prompt To Fix With AI

This is a comment left during a code review. Path: scripts/install.sh Line: 105-106 Comment: **Pipx ownership is misidentified** If a manually installed or uv-managed `strix` is in a path containing `.local/bin` while `pipx` is available, this code labels it as a pipx installation without confirming ownership. Following the suggested command can leave the actual PATH conflict unresolved or uninstall a separate pipx-managed `strix-agent`. Please determine ownership through pipx itself or provide neutral removal guidance. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

greptile-apps · 2026-09-12T03:32:36Z

+    if [[ "$path" == *".local/bin"* ]] && command -v pipx >/dev/null 2>&1; then
+        echo -e "${MUTED}  It looks like a pipx installation. To remove it: ${NC}pipx uninstall strix-agent"
+    else
+        echo -e "${MUTED}  To remove it: ${NC}rm $path"


Removal path is unescaped

If the resolved executable path contains spaces or glob characters, copying this displayed rm command causes the shell to split or expand the path. The command can then fail to remove the conflicting executable or target unintended matching files. Please print a shell-escaped path.

Prompt To Fix With AI

This is a comment left during a code review. Path: scripts/install.sh Line: 108 Comment: **Removal path is unescaped** If the resolved executable path contains spaces or glob characters, copying this displayed `rm` command causes the shell to split or expand the path. The command can then fail to remove the conflicting executable or target unintended matching files. Please print a shell-escaped path. --- For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Review feedback on the conflict guidance, both valid. The pipx hint keyed off `.local/bin` appearing in the path, which is where a uv-managed or hand-placed strix lands too. Telling that user to `pipx uninstall strix-agent` either does nothing or removes a different package, and leaves the real PATH conflict in place. Ask pipx instead: compare the executable's directory against `PIPX_BIN_DIR` and confirm `strix-agent` is in `pipx list`. The `rm` suggestion printed the path bare, so copying it would split on spaces or expand a glob. Print it through `printf '%q'`. The decoy tests asserted that pipx was never invoked at all, which the ownership check now legitimately does. They assert no `uninstall` instead, which is the contract that matters. `describe_removal` only runs when another executable wins PATH resolution, which a successful install prevents, so the advice itself is now tested by lifting the functions out of the script and calling them directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

itzzdev09 · 2026-09-12T07:09:12Z

Both findings were valid — fixed in f21c5cf.

Pipx ownership — you're right that .local/bin in the path proves nothing; a uv-managed or hand-placed strix lands there too, and pipx uninstall strix-agent would then either no-op or remove a different package while leaving the real conflict in place. It now asks pipx: compare the executable's directory against PIPX_BIN_DIR and confirm strix-agent is actually in pipx list --short.

Unescaped path — now printed through printf '%q'.

Verified each branch against the real script under bash:

1: pipx genuinely owns it    ->   pipx installed it. To remove it: pipx uninstall strix-agent
2: .local/bin, pipx does NOT ->   To remove it: rm /tmp/.../.local/bin/strix
3: pipx absent entirely      ->   To remove it: rm /tmp/.../.local/bin/strix
4: spaces and a glob         ->   To remove it: rm /tmp/.../od\ d\ bin/strix\*
                                  re-expands to: [/tmp/.../od d bin/strix*]   ROUNDTRIP OK

Case 4 round-trips the printed argument back through the shell and asserts it equals the original path exactly, so the test fails if the quoting is ever dropped.

One thing your comment prompted that you didn't flag

Adding the ownership check meant the installer now queries pipx, and my existing tests asserted not pipx_log_path.exists() — pipx never invoked at all. That would have started failing, and the lazy fix would have been to delete the assertion. The contract that actually matters is narrower, so they now assert no invocation starts with uninstall. Querying is fine; changing anything is not.

Checking that also exposed a weakness in what I'd written: describe_removal only runs when another executable wins PATH resolution, and a successful install prevents that by prepending $INSTALL_DIR. So tests driving the advice through a full install would have passed vacuously — asserting "pipx uninstall" not in stdout against output that never contained any advice at all. The four new tests lift pipx_owns and describe_removal out of the script and call them directly, so they fail if the logic regresses.

Full-install coverage is unchanged and still passes: an unrelated strix on PATH survives byte-for-byte in every configuration, including when pipx reports it as owned.

greptile-apps Bot reviewed Sep 12, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix(install): only manage strix in the installer's own directory - #1296

fix(install): only manage strix in the installer's own directory#1296
itzzdev09 wants to merge 2 commits into
usestrix:mainfrom
itzzdev09:fix/installer-only-manages-own-binary

itzzdev09 commented Sep 12, 2026

Uh oh!

greptile-apps Bot commented Sep 12, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot Sep 12, 2026

Uh oh!

greptile-apps Bot Sep 12, 2026

Uh oh!

itzzdev09 commented Sep 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		if [[ "$path" == ".local/bin" ]] && command -v pipx >/dev/null 2>&1; then
		echo -e "${MUTED} It looks like a pipx installation. To remove it: ${NC}pipx uninstall strix-agent"

Uh oh!

Conversation

itzzdev09 commented Sep 12, 2026

Problem

Change

Tests

Uh oh!

greptile-apps Bot commented Sep 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 3/5

Findings

Summary

Uh oh!

greptile-apps Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 12, 2026

Choose a reason for hiding this comment

Uh oh!

itzzdev09 commented Sep 12, 2026

One thing your comment prompted that you didn't flag

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

greptile-apps Bot commented Sep 12, 2026 •

edited

Loading