shellenv: emit zsh fpath in nested/child shells by sumanthratna · Pull Request #21884 · Homebrew/brew

sumanthratna · 2026-04-01T18:23:42Z

The early-return idempotency check skipped all output when PATH already contained Homebrew's bin/sbin. Unlike PATH, zsh's fpath is a shell-local array not inherited by child processes, so completions broke in tmux/zellij panes and nested zsh shells.

Move shell detection before the early return and always emit the fpath line for zsh.

Fixes #21879

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same change?
Have you added an explanation of what your changes do and why you'd like us to include them?
Have you written new tests (excluding integration tests) for your changes? Here's an example.
Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?
- yes; brew lgtm complains about unrelated typechecking config

==> brew style --changed --fix
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got false.
# typed: false
^^^^^^^^^^^^^^
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/TrueSigil: Sorbet sigil should be at least true got false.
# typed: false
^^^^^^^^^^^^^^

AI was used to generate or assist with generating this PR. Please specify below how you used AI to help you, and what steps you have taken to manually verify the changes.
- Claude Code (Opus 4.6 (1M context) with high effort) was used to read the issue, explore the relevant source code (shellenv.sh, shellenv_spec.rb, bin/brew), design the fix, and implement the changes.
- The changes were manually verified by:
  1. Smoke-testing brew shellenv zsh with PATH set to trigger the early return — confirmed it now emits the fpath[1,0]= line (previously produced zero output)
  2. Smoke-testing brew shellenv bash with the same PATH — confirmed it still produces no output on early return (no regression)
  3. Running brew style, brew typecheck, and brew tests --only=cmd/shellenv — all pass

The early-return idempotency check skipped all output when PATH already contained Homebrew's bin/sbin. Unlike PATH, zsh's fpath is a shell-local array not inherited by child processes, so completions broke in tmux/zellij panes and nested zsh shells. Move shell detection before the early return and always emit the fpath line for zsh. Fixes Homebrew#21879

Copilot

Pull request overview

Fixes brew shellenv for zsh in nested/child shells by ensuring the zsh fpath initialization is still emitted even when the PATH idempotency/early-return condition is met (so zsh completions work in tmux/zellij panes and nested zsh sessions).

Changes:

Move shell detection ahead of the PATH-based early return in shellenv.sh.
When early return triggers, still emit the fpath[1,0]=... line for zsh.
Add an integration spec asserting fpath output for brew shellenv zsh even when PATH already contains the Homebrew prefix.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
Library/Homebrew/cmd/shellenv.sh	Reorders early-return logic and ensures zsh `fpath` is emitted in the idempotent PATH case.
Library/Homebrew/test/cmd/shellenv_spec.rb	Adds an integration test covering the zsh `fpath` output when PATH already includes Homebrew.

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

The unconditional fpath[1,0]= caused duplicate entries when brew shellenv zsh was evaluated from both .zprofile and .zshrc in the same login shell. Use zsh's ${fpath[(Ie)...]} exact-match lookup so the directory is only prepended when not already present.

Deduplicate Homebrew's zsh site-functions entry while always moving it to the front of fpath. This keeps repeated `brew shellenv zsh` evals returning success and preserves Homebrew's completion precedence. Add a regression test for the pre-existing non-first fpath case.

MikeMcQuaid · 2026-04-02T11:18:20Z

==> brew style --changed --fix
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got false.

typed: false

^^^^^^^^^^^^^^
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/TrueSigil: Sorbet sigil should be at least true got false.

typed: false

^^^^^^^^^^^^^^

CC @issyl0 can you take a look at this?

MikeMcQuaid

Thanks @sumanthratna, some notes!

issyl0 · 2026-04-02T11:56:23Z

==> brew style --changed --fix
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/StrictSigil: Sorbet sigil should be at least strict got false.
Library/Homebrew/test/cmd/shellenv_spec.rb:1:1: C: Sorbet/TrueSigil: Sorbet sigil should be at least true got false.

CC @issyl0 can you take a look at this?

@MikeMcQuaid Hmm, I cannot reproduce these errors on this branch or master? Test files are excluded from the Sorbet/TrueSigil and Sorbet/FalseSigil cops.

MikeMcQuaid · 2026-04-02T12:03:56Z

@issyl0 no worries, thanks for trying!

MikeMcQuaid · 2026-04-02T18:22:06Z

+    # zsh's fpath is not inherited by child processes, so skip the early
+    # return for zsh to always emit the full output including fpath setup.


I think instead this should keep the early return if the zsh fpath is set correctly/as expected. Always bypassing early return just for zsh kinda sucks, particularly given it's the system default on macOS now. It'll slow it down for everyone always even if they don't care about their fpath being passed through to other shells. Sorry!

correct me if I'm wrong -- I think checking fpath has the same challenge as MANPATH / INFOPATH above; not sure if we can check whether or not fpath is set correctly from within shellenv.sh

But if I've understood correctly, I think I've matched your other suggestions?

No duplication in fpath logic

Don't bypass early return for all zsh users

Idempotent — safe to re-eval, no fpath duplicates (since exported code includes fpath check)

Let me know if any other feedback! Thanks!

MikeMcQuaid · 2026-04-03T07:33:13Z

      .and be_a_success
  end
+
+  it "sets zsh fpath idempotently at the front across repeated evals", :integration_test do


Don't need a new integration test just for this but good to use it to verify yourself.

MikeMcQuaid · 2026-04-03T07:33:44Z

+  # from here (not exported, filtered by bin/brew's env -i), so always emit it.
+  if [[ "${HOMEBREW_SHELL_NAME}" == "zsh" ]] || [[ "${HOMEBREW_SHELL_NAME}" == "-zsh" ]]
+  then
+    echo "if (( \${+fpath} )); then fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\" \"\${(@)fpath:#${HOMEBREW_PREFIX}/share/zsh/site-functions}\"); else fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\"); fi;"


Suggested change

echo "if (( \${+fpath} )); then fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\" \"\${(@)fpath:#${HOMEBREW_PREFIX}/share/zsh/site-functions}\"); else fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\"); fi;"

echo "fpath[1,0]=\"${HOMEBREW_PREFIX}/share/zsh/site-functions\";"

is this insufficient?

carlocab · 2026-04-03T07:59:59Z

      echo "export HOMEBREW_REPOSITORY=\"${HOMEBREW_REPOSITORY}\";"
-      if [[ "${HOMEBREW_SHELL_NAME}" == "zsh" ]] || [[ "${HOMEBREW_SHELL_NAME}" == "-zsh" ]]
-      then
-        echo "fpath[1,0]=\"${HOMEBREW_PREFIX}/share/zsh/site-functions\";"


Can we not just add an

echo "export FPATH;"

here to make sure our fpath changes propagate to child shells?

@carlocab If that works: yes, seems vastly preferable.

Thanks @carlocab I tested this out and you're right; zsh does sync fpath and FPATH. I opened a separate clean PR: #21910

zsh's fpath is not an exported environment variable, so child shells (tmux/Zellij panes, nested zsh) lose Homebrew's completions path. The shellenv idempotency check only inspects PATH, which IS inherited, so it early-returns and never emits the fpath line. Exporting FPATH after setting fpath ensures child zsh shells inherit it. Fixes Homebrew#21879 Supersedes Homebrew#21884

Copilot AI review requested due to automatic review settings April 1, 2026 18:23

Copilot started reviewing on behalf of sumanthratna April 1, 2026 18:24 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

sumanthratna added 9 commits April 1, 2026 14:52

Consolidate zsh fpath integration tests into a single test

1a03689

cmd/shellenv: avoid zsh fallback on early return

7e0a9e0

Remove /bin/ps sandbox test

ed31857

shellenv: Preserve empty zsh fpath entries

561c8b6

Fix CI

f6eb296

shellenv: Don't add empty zsh fpath entry when fpath is unset

bcdb262

shellenv: avoid nounset error when checking zsh fpath

53e8db6

MikeMcQuaid reviewed Apr 2, 2026

View reviewed changes

Comment thread Library/Homebrew/cmd/shellenv.sh

shellenv: skip early return for zsh instead of duplicating fpath logic

daabf49

MikeMcQuaid reviewed Apr 2, 2026

View reviewed changes

shellenv: emit zsh fpath before early return to avoid duplicating logic

9ef9c42

MikeMcQuaid reviewed Apr 3, 2026

View reviewed changes

carlocab reviewed Apr 3, 2026

View reviewed changes

sumanthratna mentioned this pull request Apr 3, 2026

shellenv: export FPATH so child zsh shells inherit fpath #21910

Merged

6 tasks

sumanthratna closed this Apr 3, 2026

sumanthratna deleted the shellenv-fpath branch April 3, 2026 21:40

		# zsh's fpath is not inherited by child processes, so skip the early
		# return for zsh to always emit the full output including fpath setup.

	echo "if (( \${+fpath} )); then fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\" \"\${(@)fpath:#${HOMEBREW_PREFIX}/share/zsh/site-functions}\"); else fpath=(\"${HOMEBREW_PREFIX}/share/zsh/site-functions\"); fi;"
	echo "fpath[1,0]=\"${HOMEBREW_PREFIX}/share/zsh/site-functions\";"

Uh oh!

Uh oh!

Conversation

sumanthratna commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

MikeMcQuaid commented Apr 2, 2026

typed: false

typed: false

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

issyl0 commented Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MikeMcQuaid commented Apr 2, 2026

Uh oh!

MikeMcQuaid Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

sumanthratna Apr 2, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

carlocab Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

sumanthratna Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

issyl0 commented Apr 2, 2026 •

edited

Loading