Skip to content

fix(poetry plugin): activate the in-project virtualenv so scripts land on PATH - #2958

Open
mikeland73 wants to merge 2 commits into
mainfrom
claude/focused-goldberg-s11hj2
Open

mikeland73 wants to merge 2 commits into
mainfrom
claude/focused-goldberg-s11hj2

Conversation

@mikeland73

Copy link
Copy Markdown
Collaborator

Summary

Fixes #2676.

Since Poetry 2.0.0 removed the bundled poetry shell command, the poetry plugin relies on poetry env use (in initHook.sh) to point Poetry at the Devbox-provided Python. That selects the virtualenv but never activates it, so console scripts and entry points installed by poetry install live in <project>/.venv/bin and are not on the Devbox shell's PATH. Users are forced to prefix everything with poetry run, which is the regression reported in the issue:

Scripts from the python package are not available on the path of the devbox shell since updating from 1.7.1.

Fix

The plugin already sets POETRY_VIRTUALENVS_IN_PROJECT=true, so the virtualenv is always created at <pyproject-dir>/.venv. I activate it from the plugin's init_hook — which runs in the sourced shellrc context, unlike the executed initHook.sh, so it can actually modify the shell's PATH:

"init_hook": [
    "\"{{ .Virtenv }}/bin/initHook.sh\"",
    "if [ -d \"${DEVBOX_PYPROJECT_DIR:-$DEVBOX_DEFAULT_PYPROJECT_DIR}/.venv\" ]; then export VIRTUAL_ENV=\"${DEVBOX_PYPROJECT_DIR:-$DEVBOX_DEFAULT_PYPROJECT_DIR}/.venv\"; export PATH=\"$VIRTUAL_ENV/bin:$PATH\"; fi"
]

Ordering makes this robust:

  • Plugin init hooks run before the user's own hooks (Config.InitHook() appends included-plugin hooks ahead of the root config's hooks).
  • poetry env use inside initHook.sh creates .venv, so the [ -d … ] guard already passes when the activation line runs.
  • By the time the user's poetry install executes, .venv/bin is already on PATH, so the freshly installed scripts resolve by name.

The path is derived from env vars and every expansion is double-quoted, so project directories containing spaces are handled correctly. The plugin version is bumped 0.0.50.0.6 so existing environments regenerate with the new hook, matching the convention used by prior plugin behavior changes.

How was it tested?

  • go test ./plugins/ ./internal/plugin/ passes, including plugins/init_hook_quoting_test.go (the new line uses no {{ }} templates, and all path expansions are quoted).
  • Validated the JSON parses and bash -n syntax-checked the rendered activation line.
  • Simulated a spaced project dir end-to-end: exported DEVBOX_DEFAULT_PYPROJECT_DIR="/tmp/pdir with space", placed an executable at .venv/bin/myscript, ran the activation line, and confirmed VIRTUAL_ENV was set correctly and myscript resolved and ran by name from PATH.

cc @tlelson — thanks for the clear root-cause analysis and reproduction in the issue.

Community Contribution License

All community contributions in this pull request are licensed to the project
maintainers under the terms of the
Apache 2 License.

By creating this pull request, I represent that I have the right to license the
contributions to the project maintainers under the Apache 2 License as stated in
the
Community Contribution License.


Generated by Claude Code

…d on PATH

Since Poetry 2.0.0 removed the bundled `poetry shell` command, the poetry
plugin relies on `poetry env use` (in initHook.sh) to point Poetry at the
Devbox-provided Python. That selects the virtualenv but never activates
it, so console scripts and entry points installed by `poetry install`
live in `<project>/.venv/bin` and are not on the Devbox shell's PATH.
Users are forced to prefix everything with `poetry run`.

The plugin already sets POETRY_VIRTUALENVS_IN_PROJECT=true, so the
virtualenv is always created at `<pyproject-dir>/.venv`. Activate it from
the init_hook (which runs in the sourced shellrc context, unlike the
executed initHook.sh) by exporting VIRTUAL_ENV and prepending its `bin`
to PATH once the directory exists. Plugin init hooks run before the
user's hooks, and `poetry env use` creates `.venv`, so the guard passes
by the time the user's `poetry install` populates `.venv/bin`.

The path templates are env-var based and fully double-quoted, so project
directories containing spaces are handled correctly. Bump the plugin
version so existing environments pick up the new hook.

Fixes #2676
Copilot AI lite review requested due to automatic review settings August 17, 2026 14:19

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@savil
savil marked this pull request as draft September 2, 2026 22:13
@mikeland73
mikeland73 marked this pull request as ready for review September 15, 2026 20:21
@mikeland73

Copy link
Copy Markdown
Collaborator Author

@claude please review this PR for correctness, and flag anything that would block merging.

@claude

claude Bot commented Sep 15, 2026

Copy link
Copy Markdown

Claude finished @mikeland73's task in 4m 30s —— View job


Review: correctness check on the poetry plugin fix

  • Gather context (diff, related files, CLAUDE.md)
  • Analyze correctness of the fix
  • Check version bump / plugin regen behavior
  • Check quoting/edge cases
  • Post review feedback

The core idea is sound — sourcing the init hook (rather than executing initHook.sh) is the right way to mutate PATH for the interactive shell, and the ordering argument (plugin hooks run before root-config hooks, and poetry env use in initHook.sh creates .venv before the new line runs) checks out against Config.InitHook() in internal/devconfig/config.go:363-370.

🔴 Blocking: the new hook line will break devbox shell under fish

plugins/poetry.json:17 embeds a POSIX-only compound statement directly in init_hook:

if [ -d "..." ]; then export VIRTUAL_ENV="..."; export PATH="$VIRTUAL_ENV/bin:$PATH"; fi

All init_hook lines (from every plugin plus the user's own hooks) are concatenated with newlines into a single .hooks file (internal/devbox/shellcmd/command.go:118-123, internal/shellgen/scripts.go:57). That file is sourced two different ways depending on shell:

  • bash/zsh run it through POSIX sh (internal/shellgen/tmpl/script-wrapper.tmpl:11, and internal/devbox/shellrc.tmpl:68), so if/then/fi is fine there.
  • fish sources the exact same raw file natively (internal/devbox/shellrc_fish.tmpl:62: source "{{ .HooksFilePath }}"), and fish is a first-class supported shell here (internal/devbox/shell.go:46,163-165,478-480).

Fish has no then/fi keywords — its block syntax is if COND; ...; end. Since this line has no matching end, fish's parser will hit a syntax error (unterminated if block) when sourcing .hooks, and that failure aborts the entire hooks file — not just the poetry plugin's activation. Any fish-shell user with the poetry plugin included would see devbox shell init hooks fail wholesale (their own init_hook and any other plugin hooks included in the same file too).

This matches the existing codebase convention: other plugins that need multi-statement/conditional logic (e.g. plugins/python/venvShellHook.sh) put it in a separate script that's executed as a subprocess, not inlined as a compound statement in the JSON init_hook array that gets sourced verbatim. plugins/poetry.json is currently the only plugin with if/then/fi in its init_hook (verified via grep -rn 'if \[\|then\|; fi' plugins/*.json).

Suggested fix: replace the single if/then/fi line with POSIX+fish-safe short-circuit lines (both fish and POSIX sh support [ COND ] && cmd), e.g. as two init_hook array entries instead of one compound statement — first guard-export VIRTUAL_ENV if .venv exists, then guard-prepend PATH if VIRTUAL_ENV is set. This avoids then/fi/end entirely so the same line parses correctly under both sh and native fish.

Fix this →

Other notes (non-blocking)

  • Quoting is correct and double-quoted throughout, so spaced project directories are handled (confirmed the new line has no {{ }} template expansions, so plugins/init_hook_quoting_test.go's TestInitHookPathsAreQuoted — which only checks templated paths — won't flag it and should still pass; I wasn't able to execute go test in this review sandbox to confirm directly).
  • Version bump 0.0.50.0.6 correctly follows the plugin README convention ("bump [version] whenever you merge an update").
  • If poetry env use fails to create .venv (e.g. no pyproject.toml yet at shell init), the guard silently no-ops and PATH isn't updated even if the user runs poetry install manually later in the same session — this is a pre-existing/acceptable limitation, not a regression, so not blocking.

Recommendation: the fish-shell breakage should be fixed before merging — it's a shell startup regression for an entire supported shell, not just a poetry-specific edge case.

Copy link
Copy Markdown
Collaborator Author

Review: correctness & merge-blockers

I reviewed the change adversarially. No correctness blockers — a few non-blocking notes below for your call.

Verified correct

  • Hook ordering holds. Config.InitHook() (internal/devconfig/config.go:363-369) appends included-plugin hooks before the root config's hooks, so this plugin's initHook.sh (which runs poetry env use, creating <project>/.venv in in-project mode) and the new activation line both run before a user's poetry install. By the time poetry install populates .venv/bin, that dir is already on PATH.
  • Env vars are available. DEVBOX_DEFAULT_PYPROJECT_DIR is exported in the plugin's env, and shellrc.tmpl emits ExportEnv before sourcing the hooks file, so the expansion resolves.
  • Spaces are handled. Every expansion is double-quoted; simulated end-to-end with a project dir containing a space and a script in .venv/bin resolved and ran by name.
  • Graceful degradation. The [ -d … ] guard means that if poetry env use fails (no pyproject.toml, etc.) the line is a no-op rather than prepending a bogus path.
  • Checks: go build ./..., go test ./plugins/ ./internal/plugin/ (incl. init_hook_quoting_test.go — the new line uses no {{ }} templates and is fully quoted), go vet, and gofmt are clean. No snapshot/golden/testscript records the poetry init_hook, so nothing to regenerate.

Non-blocking considerations

  1. .venv is hardcoded. It's correct because the plugin forces POETRY_VIRTUALENVS_IN_PROJECT=true, but if a user overrides that to false, Poetry creates the venv under POETRY_VIRTUALENVS_PATH and the guard silently skips activation. That's not a regression (no worse than today), but the fix won't help that case. A more robust alternative is deriving the path from poetry env info --path at the cost of one extra Poetry invocation per shell start — your call on the trade-off.
  2. PATH duplication on refresh. The hook re-runs on shell refresh / nested devbox run, unconditionally prepending .venv/bin, so PATH can accumulate duplicates within a session. Functionally harmless, and it mirrors how the existing python plugin's venvShellHook auto-activates, but a "skip if already present" guard would keep PATH tidy.
  3. Behavior change (intentional). Every poetry-plugin user now gets the in-project venv auto-activated (VIRTUAL_ENV + PATH). This is the fix and it matches the python plugin's convention; the 0.0.5 → 0.0.6 bump reflects it.

CI caveat

No CI checks have reported on this PR (status is pending with 0 checks). The shell logic was validated by simulation, but I could not exercise it against a live Poetry 2.x install (no Nix in my environment). If the repo's full integration suite runs devbox shell with the poetry example, that's the remaining real-world signal worth confirming before merge.


Generated by Claude Code

The previous approach inlined a POSIX `if [ -d … ]; then … fi` compound
statement in the plugin's init_hook. That breaks the fish shell: fish
sources the combined .hooks file natively (internal/devbox/shellrc_fish.tmpl),
and fish has no `then`/`fi` keywords (its blocks close with `end`), so the
unterminated `if` is a parse error that aborts the entire hooks file — not
just poetry's activation — for any fish user with the poetry plugin.

Prepend the in-project virtualenv's bin directory to PATH via the plugin's
`env` field instead, matching the existing, shell-portable convention used
by the nodejs and ruby plugins (e.g. nodejs `"PATH": "{{ .Virtenv }}/corepack-bin:$PATH"`).
Devbox expands the value with os.Expand and emits it as a literal export,
so it is applied uniformly across bash, zsh, and fish with no inline shell
syntax. The path uses the {{ .DevboxProjectDir }} template (POETRY_VIRTUALENVS_IN_PROJECT
is true, so the venv lives at <project>/.venv); a not-yet-created dir on
PATH is harmless and resolves once `poetry install` populates it.

Fixes #2676

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012aiRPVpidRdyKDJiUQJhxN

Copy link
Copy Markdown
Collaborator Author

Good catch on the fish-shell breakage — that's a real blocker, and I've verified and fixed it in b6d4906.

Confirmed the mechanism: fish sources the combined .hooks file natively (internal/devbox/shellrc_fish.tmpl:62), and fish has no then/fi (blocks close with end), so the inlined if … then … fi was an unterminated-if parse error that would abort the entire hooks file for any fish user with the poetry plugin — not just poetry's own line.

Rather than rewrite it as [ … ] && … short-circuits (fish also lacks export and ${VAR:-default}, so that would still not be portable), I moved the PATH change to the plugin's env field, which is the existing shell-portable convention:

"env": {
    ...
    "PATH": "{{ .DevboxProjectDir }}/.venv/bin:$PATH"
}

This matches what the nodejs ("{{ .Virtenv }}/corepack-bin:$PATH") and ruby plugins already do. Devbox expands the value with os.Expand (internal/conf/env.go:27) and exportify emits it as a single literal export, so it applies uniformly across bash, zsh, and fish with no inline shell syntax. The init_hook is back to just the executed initHook.sh line. go test ./plugins/ ./internal/plugin/, go vet, and gofmt are clean, and grep confirms no plugin init_hook contains if/then/fi.

One deliberate trade-off worth flagging: because os.Expand doesn't support ${VAR:-default}, the env value uses the static {{ .DevboxProjectDir }} rather than the runtime DEVBOX_PYPROJECT_DIR override. So a user who relocates pyproject.toml via DEVBOX_PYPROJECT_DIR won't get that subdir's .venv/bin auto-added to PATH (they can still use poetry run). This covers the default case — which is what #2676 reports — and is a no-op, not a regression, for the override case. Happy to fold in VIRTUAL_ENV too, or handle the override, if you'd prefer.


Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Poetry shell missing scripts

3 participants