Skip to content

fix(plugins): quote templated hook paths so spaces in project dir work#2876

Open
mikeland73 wants to merge 4 commits into
mainfrom
claude/focused-goldberg-pn7wpe
Open

fix(plugins): quote templated hook paths so spaces in project dir work#2876
mikeland73 wants to merge 4 commits into
mainfrom
claude/focused-goldberg-pn7wpe

Conversation

@mikeland73

Copy link
Copy Markdown
Collaborator

Summary

Fixes #2673.

Several builtin plugins reference a templated absolute path in their init_hook without wrapping it in double quotes:

Plugin init_hook line (before)
python {{ .Virtenv }}/bin/venvShellHook.sh
poetry {{ .Virtenv }}/bin/initHook.sh
mariadb bash {{ .Virtenv }}/setup_db.sh
mysql bash {{ .Virtenv }}/setup_db.sh

Because {{ .Virtenv }} expands to the project's absolute path, an unquoted reference is word-split by the shell whenever the project directory contains a space. The hook then fails to run and the shell startup breaks:

me@host:~/Documents/rm scripts$ devbox shell
Starting a devbox shell...
bash: /home/me/Documents/rm: No such file or directory

Fix

Wrap each templated path in double quotes, matching the pattern the nodejs plugin already uses (node "{{ .Virtenv }}/bin/setup-corepack.js"). After rendering, the generated hook becomes a single quoted token, e.g.:

"/home/me/Documents/rm scripts/.devbox/virtenv/python/bin/venvShellHook.sh"

The affected plugins' version fields are bumped (0.0.40.0.5), consistent with how prior plugin behavior changes are versioned.

The hook scripts themselves (venvShellHook.sh, initHook.sh, setup_db.sh) already quote their internal path variables, so the only gap was the invocation line.

Testing

  • Added plugins/init_hook_quoting_test.go, which scans every builtin plugin and asserts that any {{ ... }} template used in an init_hook line is enclosed in double quotes. It fails on the old (unquoted) content and passes on the fix.
  • Reproduced the exact reported error and confirmed the fix at the shell level:
    # unquoted (old): bash: /tmp/rm: No such file or directory
    # quoted   (new): HOOK RAN OK
    
  • Simulated the plugin template render + JSON parse with a spaced project dir for all four plugins and bash -n syntax-checked each rendered line.
  • go test ./plugins/ ./internal/plugin/, go vet, and gofmt are clean.

cc @xorinzor (issue reporter) — thanks for the clear reproduction and root-cause.


Generated by Claude Code

The python, poetry, mariadb and mysql builtin plugins referenced a
templated absolute path (e.g. "{{ .Virtenv }}/bin/venvShellHook.sh") in
their init_hook without wrapping it in double quotes. Because .Virtenv
expands to the project's absolute path, an unquoted reference is word-split
by the shell whenever the project directory contains a space, so the hook
fails to run:

    bash: /home/me/Documents/rm: No such file or directory

Wrap each templated path in double quotes (matching the existing nodejs
plugin), and bump the affected plugins' versions. Adds a regression test
that asserts no builtin plugin's init_hook contains an unquoted templated
path.

Fixes #2673
Copilot AI review requested due to automatic review settings June 20, 2026 14:14

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.

Pull request overview

This PR fixes devbox shell failures when the project directory contains spaces by ensuring builtin plugin shell.init_hook commands properly quote templated absolute paths (e.g. {{ .Virtenv }}), preventing shell word-splitting.

Changes:

  • Quote templated {{ .Virtenv }} paths in init_hook for the python, poetry, mysql, and mariadb plugins.
  • Bump the affected plugin versions from 0.0.4 to 0.0.5.
  • Add a Go regression test to enforce quoting for templated values in builtin plugin init_hook lines.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
plugins/python.json Quotes the templated virtenv hook script path and bumps plugin version.
plugins/poetry.json Quotes the templated init hook script path and bumps plugin version.
plugins/mysql.json Quotes the templated setup_db.sh path in the bash invocation and bumps plugin version.
plugins/mariadb.json Quotes the templated setup_db.sh path in the bash invocation and bumps plugin version.
plugins/init_hook_quoting_test.go Adds a regression test to enforce quoting of templated init_hook content across builtin plugins.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread plugins/init_hook_quoting_test.go Outdated
claude added 3 commits June 20, 2026 14:19
Address review feedback on the init_hook quoting regression test. The
previous insideDoubleQuotes helper only toggled on " and could produce
false negatives: a literal double quote inside a single-quoted segment
would flip the state, and an unterminated quote was treated as quoted.

Replace it with shellQuotedPositions, which models the parts of POSIX
shell quoting relevant to word-splitting: single and double quotes are
mutually exclusive, a backslash escapes a double quote inside double
quotes, and a quote that is never closed leaves its bytes unquoted (so an
unterminated segment is reported as unsafe).
golangci-lint's varnamelen flagged the single-letter byte variable used
across the quote-scanning loop (distance exceeds max-distance: 10). Use a
descriptive name instead.
The project-tests-only job failed only on the elixir example
(mix deps.get / mix run), which is unrelated to this PR's plugin
init_hook quoting change. Re-running to clear the flaky failure.

Copy link
Copy Markdown
Collaborator Author

Heads-up on CI: the substantive change is green across the board — golangci-lint, build-devbox, Test Flake Build, all 6 test-nix-versions, all 4 auto-nix-install, and (in the runs that got far enough) every plugin testscript: plugin.test, php.test, nodejs_corepack_autodetect.test, disable-plugin.test, and the python_patch_* tests.

The remaining red is a single flaky, unrelated test:

  • Latest run: test (not-main, ubuntu-latest, project-tests-off, 2.30.2) failed only on add_platforms_flakeref.test (a remote-flakeref devbox add test, ~180s — a network/flake-fetch flake). The sibling matrix jobs report cancelled due to fail-fast, not a real failure.
  • An earlier run flaked on a different unrelated test — the elixir example's mix deps.get/mix run. The Elixir example doesn't use any plugin this PR touches (the elixir plugin has no init_hook at all).

Two different network-flaky tests across re-runs, none related to the four plugins changed here (python/poetry/mysql/mariadb init_hook quoting), points to CI flakiness rather than a regression.

I can't re-run the failed job from automation (the integration gets 403 Resource not accessible by integration on rerun-failed-jobs). Could a maintainer re-run the failed matrix job when convenient? Happy to make any change if you'd prefer a different approach.


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.

bug: space in directory name prevents hooks from executing in bash

3 participants