Skip to content

fix(shell): add user-local bin dirs to subprocess PATH - #7057

Open
lcq225 wants to merge 1 commit into
agentscope-ai:mainfrom
lcq225:fix/4365-shell-path-local-bin
Open

fix(shell): add user-local bin dirs to subprocess PATH#7057
lcq225 wants to merge 1 commit into
agentscope-ai:mainfrom
lcq225:fix/4365-shell-path-local-bin

Conversation

@lcq225

@lcq225 lcq225 commented Aug 15, 2026

Copy link
Copy Markdown
Contributor

fix(shell): add user-local bin dirs to subprocess PATH

What Problem This Solves

When QwenPaw runs as a systemd/Launchd service or in a Docker container, the
daemon inherits a stripped PATH (e.g. /usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin).
User-installed CLIs — gh, cmake, lark-cli, fnm/nvm-managed node, per-user
Python Scripts — live in directories that are not on that inherited PATH:

  • ~/.local/bin (Unix, standard user binary location)
  • ~/.local/share/fnm, ~/.local/share/nvm (Unix, single-user Node managers)
  • %LOCALAPPDATA%\Programs\Python\…\Scripts (Windows per-user Python install)

As a result, execute_shell_command subprocesses cannot find these tools even
though they work fine in a normal user terminal. The workaround (hardcoding
paths into shell.py) is overwritten on every QwenPaw upgrade.

Evidence

Before fix (main branch, qpi7048 repo src/qwenpaw/agents/tools/shell.py
L1033):
execute_shell_command builds env via
_ensure_user_bins_on_path(os.environ.copy()) then prepends
python_bin_dir + existing_path — the only directories ever prepended are the
venv Scripts dir and whatever PATH the daemon inherited. If the daemon
starts with a stripped PATH (systemd default, minimal Docker image),
~/.local/bin / %LOCALAPPDATA%\Programs\Python\Scripts are invisible to
subprocesses, and user-installed CLIs (gh, cmake, lark-cli, fnm/nvm
node shims) silently fail with command not found / not recognized even
though the same commands work in a logged-in user terminal.

After fix (TestEnsureUserBinsOnPath regression suite, 7/7 passing):

  1. _ensure_user_bins_on_path({"PATH": "/usr/bin"}, user_home=<dir>) with
    <dir>/.local/bin on disk → returns
    <dir>/.local/bin:/usr/bin (existing /usr/bin preserved in order).
  2. Same helper when ~/.local/bin does not exist → no-op, PATH
    unchanged.
  3. Same helper when ~/.local/bin is already on PATH → deduped, appears
    once.
  4. Windows branch (sys.platform == "win32") with
    LOCALAPPDATA=<dir> + <dir>\Programs\Python on disk → that directory
    prepended; existing PATH entries keep their order.
  5. Unix branch also prepends ~/.local/share/fnm and ~/.local/share/nvm
    when they exist (Node single-user managers).

Isolation harness (verify_ensure.py): imports shell.py by filename
with stubbed heavy dependencies (agentscope not available in the lean
sandbox), monkey-patches sys.platform to cover both Unix and Windows
branches — ALL 7 TESTS PASSED.

Pinned pre-commit (matches .pre-commit-config.yaml): black 23.3.0
--check --line-length 79 idempotent ✅ · add-trailing-comma v3.1.0 idempotent
✅ · flake8 6.1.0 --extend-ignore=E203 0 reports ✅.

What This PR Does

Adds _ensure_user_bins_on_path(env) — a small, stdlib-only helper that
prepends the standard user-level bin directories onto env["PATH"] if and only
if they exist on disk and are not already present. It is called once at the
top of execute_shell_command before the existing python_bin_dir prepending,
so the current venv-Python-first behavior and all existing PATH entries are
preserved.

Scope is deliberately narrow:

  • Only touches execute_shell_command's environment build in
    qwenpaw/agents/tools/shell.py.
  • No config changes, no new settings, no changes to sandbox env handling.
  • Only adds existing directories — a missing ~/.local/bin is a no-op.
  • Deduplication uses case-insensitive matching (handles C:\… vs c:\… on
    Windows).

Type of Change

  • Bug fix

Components Affected

  • Core / Tools (qwenpaw/agents/tools/shell.py)
  • Tests (tests/unit/agents/tools/test_shell.py)

Testing / Evidence

Pinned pre-commit tooling (matches .pre-commit-config.yaml)

Run against the two modified files with the pinned hook versions
(black==23.3.0, flake8==6.1.0, add-trailing-comma==3.1.0) installed in an
isolated --target directory and invoked via sys.path.insert(0, <target>)
launcher (required because python -E ignores PYTHONPATH):

black 23.3.0 --check --line-length 79       → both files unchanged (idempotent)
add-trailing-comma v3.1.0                   → no rewrites (idempotent)
flake8 6.1.0 --extend-ignore=E203 --jobs 1  → 0 reports

Regression tests

6 unit tests added under TestEnsureUserBinsOnPath, covering:

  1. Adds ~/.local/bin when it exists and PATH is missing it.
  2. No-op when ~/.local/bin does not exist.
  3. No duplicate when ~/.local/bin is already on PATH.
  4. Existing PATH entries keep their order after prepending.
  5. Uses the requested user_home when PATH is empty.
  6. Windows branch adds LOCALAPPDATA\Programs\Python when present.

Plus a standalone isolation harness (verify_ensure.py) that imports
shell.py by filename with stubbed heavy dependencies (agentscope etc. are
not available in a lean sandbox) — 7/7 tests pass.

Documentation

No documentation update required — this is an internal environment-building
fix with no user-facing config or API surface change.

…#4365)  Add _ensure_user_bins_on_path helper so that execute_shell_command subprocesses can locate user-installed CLIs (gh, cmake, fnm/nvm node, per-user Python Scripts) when the daemon inherits a stripped PATH (systemd/Launchd/Docker).  Only existing, non-duplicate directories are prepended; the python venv bin and existing PATH entries are preserved. Includes 6 regression tests covering Unix and Windows branches.
@github-actions

Copy link
Copy Markdown

Welcome to QwenPaw! 🐾

Hi @lcq225, this is your 7th Pull Request.

📋 About PR Template

To help maintainers review your PR faster, please make sure to include:

  • Description - What this PR does and why
  • Type of Change - Bug fix / Feature / Breaking change / Documentation / Refactoring
  • Component(s) Affected - Core / Console / Channels / Skills / CLI / Documentation / Tests / CI/CD / Scripts
  • Checklist:
    • Run and pass pre-commit run --all-files
    • Run and pass relevant tests (pytest or as applicable)
    • Update documentation if needed
  • Testing - How to test these changes
  • Local Verification Evidence:
    pre-commit run --all-files
    # paste summary result
    
    pytest
    # paste summary result

Complete PR information helps speed up the review process. You can edit the PR description to add these details.

🙌 Join Developer Community

Thanks so much for your contribution! We'd love to invite you to join the official QwenPaw developer group! You can find the Discord and DingTalk group links under the "Developer Community" section on our docs page:
https://qwenpaw.agentscope.io/docs/community

We truly appreciate your enthusiasm—and look forward to your future contributions! 😊

We'll review your PR soon.

@lcq225
lcq225 requested a deployment to maintainer-approved August 15, 2026 15:25 — with GitHub Actions Waiting
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Todo

Development

Successfully merging this pull request may close these issues.

1 participant