Repository navigation
Conversation
|
@rerun0510 is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughPreserves the user's original ZDOTDIR in Changes
Sequence DiagramsequenceDiagram
participant Shell as Shell Startup
participant zshenv as .zshenv
participant user_zshenv as User .zshenv
participant zprofile as .zprofile
participant user_zprofile as User .zprofile
participant zlogin as .zlogin
participant user_zlogin as User .zlogin
participant zshrc as .zshrc
participant user_zshrc as User .zshrc
participant Integration as Integrations (Ghostty/cmux)
Shell->>zshenv: Start (all shells)
zshenv->>zshenv: Set _CMUX_ORIG_ZDOTDIR (env fallbacks), unset injected vars
zshenv->>user_zshenv: Source user's .zshenv from _CMUX_ORIG_ZDOTDIR
user_zshenv-->>zshenv: Return
Shell->>zprofile: Login shells
zprofile->>user_zprofile: Source user's .zprofile from _CMUX_ORIG_ZDOTDIR
user_zprofile-->>zprofile: Return
Shell->>zlogin: Login shells
zlogin->>user_zlogin: Source user's .zlogin from _CMUX_ORIG_ZDOTDIR
user_zlogin-->>zlogin: Return
Shell->>zshrc: Interactive shells
zshrc->>user_zshrc: Source user's .zshrc from _CMUX_ORIG_ZDOTDIR
user_zshrc-->>zshrc: Return
zshrc->>zshrc: Possibly adjust HISTFILE
zshrc->>Integration: Conditionally load Ghostty/cmux integrations
Integration-->>zshrc: Return
zshrc->>zshrc: Restore/unset ZDOTDIR, cleanup
zshrc->>Shell: Complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Greptile SummaryThis PR fixes shell integration not loading when cmux is launched from Finder by deferring the loading of Ghostty and cmux hooks from Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant GK as GhosttyKit
participant ZSH as zsh
participant ENV as integration/.zshenv
participant PRO as integration/.zprofile
participant RC as integration/.zshrc
participant LOG as integration/.zlogin
participant UENV as user/.zshenv
participant UPRO as user/.zprofile
participant URC as user/.zshrc
participant ULOG as user/.zlogin
GK->>ZSH: set ZDOTDIR=integration_dir<br/>set GHOSTTY_ZSH_ZDOTDIR=orig_zdotdir
ZSH->>ENV: load (ZDOTDIR=integration_dir)
ENV->>ENV: save _CMUX_ORIG_ZDOTDIR=orig_zdotdir<br/>ZDOTDIR stays → integration_dir
ENV->>UENV: source user/.zshenv
ZSH->>PRO: load (ZDOTDIR still=integration_dir)
PRO->>UPRO: source user/.zprofile<br/>(Kiro CLI may reset precmd_functions here)
ZSH->>RC: load (ZDOTDIR still=integration_dir)
RC->>URC: source user/.zshrc<br/>(Kiro CLI may reset precmd_functions here)
RC->>RC: fix HISTFILE if pointing to integration_dir
RC->>RC: load Ghostty integration (after user files ✓)
RC->>RC: load cmux integration (after user files ✓)
RC->>RC: restore ZDOTDIR=orig_zdotdir<br/>unset _CMUX_ORIG_ZDOTDIR
Note over ZSH,ULOG: ZDOTDIR now = orig_zdotdir<br/>zsh re-evaluates ZDOTDIR → loads user/.zlogin directly
ZSH->>ULOG: load user/.zlogin directly (wrapper bypassed)
Reviews (1): Last reviewed commit: "Fix shell integration not loading on Fin..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # Restore the user's original ZDOTDIR now that all startup files are done. | ||
| if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then | ||
| builtin unset ZDOTDIR | ||
| else | ||
| builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}" | ||
| fi | ||
| builtin unset _CMUX_ORIG_ZDOTDIR _cmux_ghostty _cmux_integ |
There was a problem hiding this comment.
_CMUX_ORIG_ZDOTDIR unset before .zlogin runs
The comment says "all startup files are done," but .zlogin executes after .zshrc in the zsh startup sequence (.zshenv → .zprofile → .zshrc → .zlogin). _CMUX_ORIG_ZDOTDIR is being cleaned up here while the .zlogin wrapper still depends on it.
In practice, the .zlogin wrapper is bypassed because zsh re-evaluates ZDOTDIR before loading each startup file — so restoring ZDOTDIR here causes zsh to load the user's .zlogin directly. This is the same mechanism that made .zprofile/.zshrc/.zlogin "never reached" in the old design. The user's .zlogin is therefore loaded correctly in the common case.
However, if _CMUX_ORIG_ZDOTDIR were still needed by the .zlogin wrapper (e.g. on a zsh build that pre-computes startup file paths), the ${_CMUX_ORIG_ZDOTDIR-$HOME} fallback in .zlogin would silently resolve to $HOME, sourcing the wrong .zlogin for any user with a custom ZDOTDIR. The PR description also lists .zlogin as "Sources user's .zlogin via _CMUX_ORIG_ZDOTDIR", which will not work as written once this cleanup has run.
Consider deferring the _CMUX_ORIG_ZDOTDIR unset (and ideally the ZDOTDIR restoration) to .zlogin for login shells, or — since the wrapper is bypassed anyway — document explicitly that .zlogin is always loaded directly after ZDOTDIR is restored here.
| # Restore the user's original ZDOTDIR now that all startup files are done. | |
| if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then | |
| builtin unset ZDOTDIR | |
| else | |
| builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}" | |
| fi | |
| builtin unset _CMUX_ORIG_ZDOTDIR _cmux_ghostty _cmux_integ | |
| # Restore the user's original ZDOTDIR now that .zshrc integration is done. | |
| # NOTE: .zlogin runs after .zshrc; because ZDOTDIR is restored here, zsh will | |
| # load the user's .zlogin directly (bypassing the wrapper), so cleanup is safe. | |
| if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then | |
| builtin unset ZDOTDIR | |
| else | |
| builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}" | |
| fi | |
| builtin unset _CMUX_ORIG_ZDOTDIR _cmux_ghostty _cmux_integ |
There was a problem hiding this comment.
@greptile-apps Updated the PR description — .zlogin wrapper is indeed not reached since ZDOTDIR is restored in .zshrc. It exists only as a safety fallback.
There was a problem hiding this comment.
That makes sense — since ZDOTDIR is restored to the user's original value (or unset) at the end of .zshrc, zsh will naturally load the user's .zlogin directly from their real ZDOTDIR. The wrapper is effectively unreachable in the normal flow.
Good call documenting it explicitly in the table. The fallback remains useful in case an edge case or future zsh behavior change causes the wrapper to be invoked, and the ${_CMUX_ORIG_ZDOTDIR-$HOME} fallback ensures it won't break catastrophically even though the variable is already cleaned up.
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv" | ||
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | ||
| builtin unset _cmux_file |
There was a problem hiding this comment.
_CMUX_ORIG_ZDOTDIR
Unlike the wrappers in .zprofile, .zshrc, and .zlogin which all use ${_CMUX_ORIG_ZDOTDIR-$HOME}, this line uses ${_CMUX_ORIG_ZDOTDIR} without a fallback. It is safe here because _CMUX_ORIG_ZDOTDIR is always set in the block immediately above, but for consistency and defensive robustness (e.g. if this file were somehow sourced out-of-order), adding the same default would be cleaner:
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv" | |
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | |
| builtin unset _cmux_file | |
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zshenv" |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if [[ "$HISTFILE" == "${ZDOTDIR}"/* ]]; then | ||
| HISTFILE="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zsh_history" | ||
| fi |
There was a problem hiding this comment.
HISTFILE check uses glob pattern matching
[[ "$HISTFILE" == "${ZDOTDIR}"/* ]] — in zsh [[...]], the right-hand side of == is a pattern. The trailing * is treated as a glob wildcard, so this correctly matches any path whose prefix is ${ZDOTDIR}/. This is fine in practice, but worth being explicit about the intent. If ZDOTDIR ever contained glob meta-characters (e.g. [, (), the match could behave unexpectedly. Using a prefix-string comparison would be more robust:
| if [[ "$HISTFILE" == "${ZDOTDIR}"/* ]]; then | |
| HISTFILE="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zsh_history" | |
| fi | |
| if [[ "${HISTFILE}" == "${ZDOTDIR}/"* ]]; then |
(No functional change — just clarifying the intended prefix match with the slash inside the literal part.)
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/shell-integration/.zshenv`:
- Around line 16-30: The wrapper currently sources the user's file while ZDOTDIR
still points to the wrapper; before sourcing set ZDOTDIR to the saved original
and after sourcing capture any user-modified ZDOTDIR back into
_CMUX_ORIG_ZDOTDIR so later wrappers honor user overrides. Concretely, in the
.zshenv wrapper save the wrapper's ZDOTDIR to a temp variable, export
ZDOTDIR="$_CMUX_ORIG_ZDOTDIR" (or restore from
GHOSTTY_ZSH_ZDOTDIR/CMUX_ZSH_ZDOTDIR logic) before sourcing _cmux_file, then
after sourcing set _CMUX_ORIG_ZDOTDIR="$ZDOTDIR" and restore the wrapper ZDOTDIR
from the temp var and cleanup temp vars; apply the same pattern to the
.zprofile, .zshrc, and .zlogin wrappers referencing _CMUX_ORIG_ZDOTDIR and
_cmux_file to locate the source points.
In `@Resources/shell-integration/.zshrc`:
- Around line 10-19: The wrapper must capture the post-user-init ZDOTDIR and use
it when sourcing user files and setting HISTFILE: in .zshenv (after sourcing the
user's .zshenv) export a new variable _CMUX_EFFECTIVE_ZDOTDIR set to
"${ZDOTDIR:-${_CMUX_ORIG_ZDOTDIR:-$HOME}}", then update .zshrc (and mirror the
same changes in .zprofile and .zlogin) to replace uses of _CMUX_ORIG_ZDOTDIR
with _CMUX_EFFECTIVE_ZDOTDIR for sourcing the user's .zshrc and for computing
HISTFILE so the wrapper respects any ZDOTDIR the user's .zshenv set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc4bceab-e4e1-4ea7-a7ce-cfc5d4c556d5
📒 Files selected for processing (4)
Resources/shell-integration/.zloginResources/shell-integration/.zprofileResources/shell-integration/.zshenvResources/shell-integration/.zshrc
| # Save the user's original ZDOTDIR so wrappers can find user files. | ||
| if [[ -n "${GHOSTTY_ZSH_ZDOTDIR+X}" ]]; then | ||
| builtin export ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" | ||
| builtin export _CMUX_ORIG_ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" | ||
| builtin unset GHOSTTY_ZSH_ZDOTDIR | ||
| elif [[ -n "${CMUX_ZSH_ZDOTDIR+X}" ]]; then | ||
| builtin export ZDOTDIR="$CMUX_ZSH_ZDOTDIR" | ||
| builtin export _CMUX_ORIG_ZDOTDIR="$CMUX_ZSH_ZDOTDIR" | ||
| builtin unset CMUX_ZSH_ZDOTDIR | ||
| else | ||
| builtin unset ZDOTDIR | ||
| builtin export _CMUX_ORIG_ZDOTDIR="${HOME}" | ||
| fi | ||
|
|
||
| { | ||
| # zsh treats unset ZDOTDIR as if it were HOME. We do the same. | ||
| builtin typeset _cmux_file="${ZDOTDIR-$HOME}/.zshenv" | ||
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | ||
| } always { | ||
| if [[ -o interactive ]]; then | ||
| # We overwrote GhosttyKit's injected ZDOTDIR, so manually load Ghostty's | ||
| # zsh integration if available. | ||
| # | ||
| # We can't rely on GHOSTTY_ZSH_ZDOTDIR here because Ghostty's own zsh | ||
| # bootstrap unsets it before chaining into this cmux wrapper. | ||
| if [[ "${CMUX_LOAD_GHOSTTY_ZSH_INTEGRATION:-0}" == "1" ]]; then | ||
| if [[ -n "${CMUX_SHELL_INTEGRATION_DIR:-}" ]]; then | ||
| builtin typeset _cmux_ghostty="$CMUX_SHELL_INTEGRATION_DIR/ghostty-integration.zsh" | ||
| fi | ||
| if [[ ! -r "${_cmux_ghostty:-}" && -n "${GHOSTTY_RESOURCES_DIR:-}" ]]; then | ||
| builtin typeset _cmux_ghostty="$GHOSTTY_RESOURCES_DIR/shell-integration/zsh/ghostty-integration" | ||
| fi | ||
| [[ -r "$_cmux_ghostty" ]] && builtin source -- "$_cmux_ghostty" | ||
| fi | ||
|
|
||
| # Load cmux integration (unless disabled) | ||
| if [[ "${CMUX_SHELL_INTEGRATION:-1}" != "0" && -n "${CMUX_SHELL_INTEGRATION_DIR:-}" ]]; then | ||
| builtin typeset _cmux_integ="$CMUX_SHELL_INTEGRATION_DIR/cmux-zsh-integration.zsh" | ||
| [[ -r "$_cmux_integ" ]] && builtin source -- "$_cmux_integ" | ||
| fi | ||
| fi | ||
|
|
||
| builtin unset _cmux_file _cmux_ghostty _cmux_integ | ||
| } | ||
| # Source the user's .zshenv (runs for all shells, interactive or not). | ||
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv" | ||
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | ||
| builtin unset _cmux_file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== shell wrappers ==="
for f in \
Resources/shell-integration/.zshenv \
Resources/shell-integration/.zprofile \
Resources/shell-integration/.zshrc \
Resources/shell-integration/.zlogin
do
echo "--- $f ---"
sed -n '1,140p' "$f"
echo
done
echo "=== ZDOTDIR-related tests ==="
if [ -d tests ]; then
rg -n -C3 'export ZDOTDIR=|user startup files see original ZDOTDIR|expected \.zshrc from alt ZDOTDIR|CMUX_ZSH_ZDOTDIR' tests || true
else
echo "tests/ not present"
fiRepository: manaflow-ai/cmux
Length of output: 7983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== test_shell_zdotdir_wrapper.py (full) ==="
if [ -f tests/test_shell_zdotdir_wrapper.py ]; then
cat tests/test_shell_zdotdir_wrapper.py
fi
echo ""
echo "=== test_shell_zdotdir_user_override.py (full) ==="
if [ -f tests/test_shell_zdotdir_user_override.py ]; then
cat tests/test_shell_zdotdir_user_override.py
fiRepository: manaflow-ai/cmux
Length of output: 6042
Restore the user's ZDOTDIR before sourcing startup files so wrappers honor user overrides.
The current .zshenv sources the user's real startup file while ZDOTDIR still points at the wrapper directory. This breaks shell integration semantics in two ways:
-
User code sees wrong
ZDOTDIR— The testtest_shell_zdotdir_wrapper.pyexpects user startup files to see the original userZDOTDIR, not the wrapper directory. -
User
ZDOTDIRoverrides are ignored — If a user's.zshenvexportsZDOTDIRto a different path (common in dotfiles-managed setups), the testtest_shell_zdotdir_user_override.pyexpects later startup files (.zshrc,.zlogin) to be sourced from the updated path. Currently,.zprofileand.zshrcwrappers source from the frozen_CMUX_ORIG_ZDOTDIR, ignoring the user's override, which breaks plugin loading and diverges from Ghostty behavior.
Apply this pattern to all wrapper files:
- Before sourcing the user's file: save wrapper
ZDOTDIR, then unset or restore the user'sZDOTDIR. - After sourcing: capture the potentially-modified
$ZDOTDIRfor use by later wrappers, then re-wrap.
The same fix applies to .zprofile, .zshrc, and .zlogin wrappers.
Minimal example for .zshenv
+# Keep the wrapper path so later stages still re-enter these wrappers.
+builtin export _CMUX_WRAPPER_ZDOTDIR="${ZDOTDIR}"
+
# Source the user's .zshenv (runs for all shells, interactive or not).
builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv"
-[[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file"
+if [[ -r "$_cmux_file" ]]; then
+ if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then
+ builtin unset ZDOTDIR
+ else
+ builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}"
+ fi
+ builtin source -- "$_cmux_file"
+ builtin export _CMUX_EFFECTIVE_ZDOTDIR="${ZDOTDIR:-$HOME}"
+ builtin export ZDOTDIR="${_CMUX_WRAPPER_ZDOTDIR}"
+else
+ builtin export _CMUX_EFFECTIVE_ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}"
+fi
builtin unset _cmux_file📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Save the user's original ZDOTDIR so wrappers can find user files. | |
| if [[ -n "${GHOSTTY_ZSH_ZDOTDIR+X}" ]]; then | |
| builtin export ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" | |
| builtin export _CMUX_ORIG_ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" | |
| builtin unset GHOSTTY_ZSH_ZDOTDIR | |
| elif [[ -n "${CMUX_ZSH_ZDOTDIR+X}" ]]; then | |
| builtin export ZDOTDIR="$CMUX_ZSH_ZDOTDIR" | |
| builtin export _CMUX_ORIG_ZDOTDIR="$CMUX_ZSH_ZDOTDIR" | |
| builtin unset CMUX_ZSH_ZDOTDIR | |
| else | |
| builtin unset ZDOTDIR | |
| builtin export _CMUX_ORIG_ZDOTDIR="${HOME}" | |
| fi | |
| { | |
| # zsh treats unset ZDOTDIR as if it were HOME. We do the same. | |
| builtin typeset _cmux_file="${ZDOTDIR-$HOME}/.zshenv" | |
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | |
| } always { | |
| if [[ -o interactive ]]; then | |
| # We overwrote GhosttyKit's injected ZDOTDIR, so manually load Ghostty's | |
| # zsh integration if available. | |
| # | |
| # We can't rely on GHOSTTY_ZSH_ZDOTDIR here because Ghostty's own zsh | |
| # bootstrap unsets it before chaining into this cmux wrapper. | |
| if [[ "${CMUX_LOAD_GHOSTTY_ZSH_INTEGRATION:-0}" == "1" ]]; then | |
| if [[ -n "${CMUX_SHELL_INTEGRATION_DIR:-}" ]]; then | |
| builtin typeset _cmux_ghostty="$CMUX_SHELL_INTEGRATION_DIR/ghostty-integration.zsh" | |
| fi | |
| if [[ ! -r "${_cmux_ghostty:-}" && -n "${GHOSTTY_RESOURCES_DIR:-}" ]]; then | |
| builtin typeset _cmux_ghostty="$GHOSTTY_RESOURCES_DIR/shell-integration/zsh/ghostty-integration" | |
| fi | |
| [[ -r "$_cmux_ghostty" ]] && builtin source -- "$_cmux_ghostty" | |
| fi | |
| # Load cmux integration (unless disabled) | |
| if [[ "${CMUX_SHELL_INTEGRATION:-1}" != "0" && -n "${CMUX_SHELL_INTEGRATION_DIR:-}" ]]; then | |
| builtin typeset _cmux_integ="$CMUX_SHELL_INTEGRATION_DIR/cmux-zsh-integration.zsh" | |
| [[ -r "$_cmux_integ" ]] && builtin source -- "$_cmux_integ" | |
| fi | |
| fi | |
| builtin unset _cmux_file _cmux_ghostty _cmux_integ | |
| } | |
| # Source the user's .zshenv (runs for all shells, interactive or not). | |
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv" | |
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | |
| builtin unset _cmux_file | |
| # Save the user's original ZDOTDIR so wrappers can find user files. | |
| if [[ -n "${GHOSTTY_ZSH_ZDOTDIR+X}" ]]; then | |
| builtin export _CMUX_ORIG_ZDOTDIR="$GHOSTTY_ZSH_ZDOTDIR" | |
| builtin unset GHOSTTY_ZSH_ZDOTDIR | |
| elif [[ -n "${CMUX_ZSH_ZDOTDIR+X}" ]]; then | |
| builtin export _CMUX_ORIG_ZDOTDIR="$CMUX_ZSH_ZDOTDIR" | |
| builtin unset CMUX_ZSH_ZDOTDIR | |
| else | |
| builtin export _CMUX_ORIG_ZDOTDIR="${HOME}" | |
| fi | |
| # Keep the wrapper path so later stages still re-enter these wrappers. | |
| builtin export _CMUX_WRAPPER_ZDOTDIR="${ZDOTDIR}" | |
| # Source the user's .zshenv (runs for all shells, interactive or not). | |
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR}/.zshenv" | |
| if [[ -r "$_cmux_file" ]]; then | |
| if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then | |
| builtin unset ZDOTDIR | |
| else | |
| builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}" | |
| fi | |
| builtin source -- "$_cmux_file" | |
| builtin export _CMUX_EFFECTIVE_ZDOTDIR="${ZDOTDIR:-$HOME}" | |
| builtin export ZDOTDIR="${_CMUX_WRAPPER_ZDOTDIR}" | |
| else | |
| builtin export _CMUX_EFFECTIVE_ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}" | |
| fi | |
| builtin unset _cmux_file |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/shell-integration/.zshenv` around lines 16 - 30, The wrapper
currently sources the user's file while ZDOTDIR still points to the wrapper;
before sourcing set ZDOTDIR to the saved original and after sourcing capture any
user-modified ZDOTDIR back into _CMUX_ORIG_ZDOTDIR so later wrappers honor user
overrides. Concretely, in the .zshenv wrapper save the wrapper's ZDOTDIR to a
temp variable, export ZDOTDIR="$_CMUX_ORIG_ZDOTDIR" (or restore from
GHOSTTY_ZSH_ZDOTDIR/CMUX_ZSH_ZDOTDIR logic) before sourcing _cmux_file, then
after sourcing set _CMUX_ORIG_ZDOTDIR="$ZDOTDIR" and restore the wrapper ZDOTDIR
from the temp var and cleanup temp vars; apply the same pattern to the
.zprofile, .zshrc, and .zlogin wrappers referencing _CMUX_ORIG_ZDOTDIR and
_cmux_file to locate the source points.
| builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zshrc" | ||
| [[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file" | ||
| builtin unset _cmux_file | ||
|
|
||
| # Fix HISTFILE: /etc/zshrc sets HISTFILE based on ZDOTDIR which currently | ||
| # points to our integration dir. Only fix if it still points there; | ||
| # if the user's .zshrc set a custom HISTFILE, respect it. | ||
| if [[ "$HISTFILE" == "${ZDOTDIR}"/* ]]; then | ||
| HISTFILE="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zsh_history" | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the shell integration files
find . -path "*/Resources/shell-integration/*.zsh*" -type f | head -20Repository: manaflow-ai/cmux
Length of output: 189
🏁 Script executed:
# Check the current state of .zshrc
cat -n Resources/shell-integration/.zshrc | head -50Repository: manaflow-ai/cmux
Length of output: 2234
🏁 Script executed:
# Check .zshenv and .zprofile for variable initialization
cat -n Resources/shell-integration/.zshenv | head -50Repository: manaflow-ai/cmux
Length of output: 1557
🏁 Script executed:
# Look for the test file mentioned
find . -name "test_shell_zdotdir_user_override.py" -type fRepository: manaflow-ai/cmux
Length of output: 103
🏁 Script executed:
# Now let's examine the test file to understand the expected behavior
cat -n tests/test_shell_zdotdir_user_override.pyRepository: manaflow-ai/cmux
Length of output: 4116
🏁 Script executed:
# Also check .zprofile for completeness
cat -n Resources/shell-integration/.zprofile | head -50Repository: manaflow-ai/cmux
Length of output: 382
🏁 Script executed:
# Let's verify if _CMUX_EFFECTIVE_ZDOTDIR is currently being set anywhere
rg "_CMUX_EFFECTIVE_ZDOTDIR" Resources/shell-integration/Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Let's also check what happens in the zsh startup flow by examining cmux-zsh-integration.zsh
cat -n Resources/shell-integration/cmux-zsh-integration.zsh | head -30Repository: manaflow-ai/cmux
Length of output: 1571
🏁 Script executed:
# Let's trace through the zsh startup sequence more carefully
# Check if there are more .zprofile details or if it's modified anywhere
rg "ZDOTDIR" Resources/shell-integration/ -A 2 -B 2Repository: manaflow-ai/cmux
Length of output: 42
🏁 Script executed:
# Let's run the test to see if it currently passes or fails
python3 tests/test_shell_zdotdir_user_override.pyRepository: manaflow-ai/cmux
Length of output: 869
🏁 Script executed:
# Let's check if there's any attempt to capture post-user-init ZDOTDIR in .zshenv
cat -n Resources/shell-integration/.zshenvRepository: manaflow-ai/cmux
Length of output: 1557
🏁 Script executed:
# Also check .zlogin to see if it has similar issues
cat -n Resources/shell-integration/.zlogin 2>/dev/null || echo "No .zlogin file"Repository: manaflow-ai/cmux
Length of output: 376
Capture post-user-init ZDOTDIR and use it in .zshrc.
After the user's .zshenv runs (line 30), it may redirect ZDOTDIR to a custom location (e.g., ~/dotfiles). The wrapper .zshrc currently uses _CMUX_ORIG_ZDOTDIR to source files, causing it to ignore the user's updated directory. This breaks the test expectation that .zshrc and HISTFILE are sourced from the post-user-init ZDOTDIR.
To fix this:
- At the end of
.zshenv(after sourcing the user's.zshenv), capture the currentZDOTDIRas_CMUX_EFFECTIVE_ZDOTDIR - In
.zshrc, use_CMUX_EFFECTIVE_ZDOTDIRinstead of_CMUX_ORIG_ZDOTDIRfor lines 10, 18, and 39-42 - Do the same for
.zprofileand.zlogin
Expected changes to .zshenv
After line 29 (after sourcing the user's .zshenv), add:
builtin export _CMUX_EFFECTIVE_ZDOTDIR="${ZDOTDIR:-${_CMUX_ORIG_ZDOTDIR:-$HOME}}"
Expected changes to .zshrc
-builtin typeset _cmux_file="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zshrc"
+builtin typeset _cmux_file="${_CMUX_EFFECTIVE_ZDOTDIR-${_CMUX_ORIG_ZDOTDIR:-$HOME}}/.zshrc"
[[ ! -r "$_cmux_file" ]] || builtin source -- "$_cmux_file"
builtin unset _cmux_file
...
- HISTFILE="${_CMUX_ORIG_ZDOTDIR-$HOME}/.zsh_history"
+ HISTFILE="${_CMUX_EFFECTIVE_ZDOTDIR-${_CMUX_ORIG_ZDOTDIR:-$HOME}}/.zsh_history"
...
-if [[ "${_CMUX_ORIG_ZDOTDIR}" == "${HOME}" ]]; then
+if [[ "${_CMUX_EFFECTIVE_ZDOTDIR}" == "${HOME}" ]]; then
builtin unset ZDOTDIR
else
- builtin export ZDOTDIR="${_CMUX_ORIG_ZDOTDIR}"
+ builtin export ZDOTDIR="${_CMUX_EFFECTIVE_ZDOTDIR}"
fi
-builtin unset _CMUX_ORIG_ZDOTDIR _cmux_ghostty _cmux_integ
+builtin unset _CMUX_ORIG_ZDOTDIR _CMUX_EFFECTIVE_ZDOTDIR _cmux_ghostty _cmux_integAlso applies to: .zprofile and .zlogin
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/shell-integration/.zshrc` around lines 10 - 19, The wrapper must
capture the post-user-init ZDOTDIR and use it when sourcing user files and
setting HISTFILE: in .zshenv (after sourcing the user's .zshenv) export a new
variable _CMUX_EFFECTIVE_ZDOTDIR set to
"${ZDOTDIR:-${_CMUX_ORIG_ZDOTDIR:-$HOME}}", then update .zshrc (and mirror the
same changes in .zprofile and .zlogin) to replace uses of _CMUX_ORIG_ZDOTDIR
with _CMUX_EFFECTIVE_ZDOTDIR for sourcing the user's .zshrc and for computing
HISTFILE so the wrapper respects any ZDOTDIR the user's .zshenv set.
1321a02 to
372d640
Compare
Defer integration loading to .zshrc wrapper instead of .zshenv so that tools like Kiro CLI that reset precmd_functions in .zprofile cannot clobber cmux's hooks. Keep ZDOTDIR pointing to the integration directory through the full zsh startup sequence and restore it after loading.
372d640 to
ccf948f
Compare
|
@lawrencecchen |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Resources/shell-integration/.zshenv (1)
17-29:⚠️ Potential issue | 🔴 CriticalRestore the effective
ZDOTDIRaround the user.zshenv.On Lines 17-25 and Lines 28-29,
_CMUX_ORIG_ZDOTDIRis fixed before the user's.zshenvruns, whileZDOTDIRstill points at the integration directory. That makes dotfiles observe the wrong$ZDOTDIR, leaves non-interactive shells with the wrapper value because they never reach.zshrc, and prevents later wrappers from following a userexport ZDOTDIR=...inside.zshenv. Restore/unsetZDOTDIRbeforesource, then capture the post-source effective value back into_CMUX_ORIG_ZDOTDIRbefore re-wrapping.Expect
.zshenvto restore/unsetZDOTDIRaround the usersource, then later wrappers to use the updated_CMUX_ORIG_ZDOTDIR.#!/bin/bash set -euo pipefail echo "=== shell wrappers ===" for f in \ Resources/shell-integration/.zshenv \ Resources/shell-integration/.zprofile \ Resources/shell-integration/.zshrc \ Resources/shell-integration/.zlogin do echo "--- $f ---" sed -n '1,180p' "$f" echo done echo "=== ZDOTDIR tests ===" for f in \ tests/test_shell_zdotdir_wrapper.py \ tests/test_shell_zdotdir_user_override.py do if [ -f "$f" ]; then echo "--- $f ---" sed -n '1,220p' "$f" echo fi done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Resources/shell-integration/.zshenv` around lines 17 - 29, The wrapper sets _CMUX_ORIG_ZDOTDIR while leaving ZDOTDIR pointing at the integration dir, so user .zshenv sees the wrong $ZDOTDIR and later exports are ignored; fix by saving the current wrapper vars (GHOSTTY_ZSH_ZDOTDIR/CMUX_ZSH_ZDOTDIR) into _CMUX_ORIG_ZDOTDIR as you do, then unset or restore ZDOTDIR (unset ZDOTDIR or set it to the user's original value) immediately before sourcing _cmux_file, and after sourcing capture the effective ZDOTDIR back into _CMUX_ORIG_ZDOTDIR (e.g., inspect $ZDOTDIR or default to $HOME) and re-export/reset any wrapper variables so later wrappers see the updated value; update the logic around variables _CMUX_ORIG_ZDOTDIR, GHOSTTY_ZSH_ZDOTDIR, CMUX_ZSH_ZDOTDIR and _cmux_file accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Resources/shell-integration/.zshenv`:
- Around line 17-29: The wrapper sets _CMUX_ORIG_ZDOTDIR while leaving ZDOTDIR
pointing at the integration dir, so user .zshenv sees the wrong $ZDOTDIR and
later exports are ignored; fix by saving the current wrapper vars
(GHOSTTY_ZSH_ZDOTDIR/CMUX_ZSH_ZDOTDIR) into _CMUX_ORIG_ZDOTDIR as you do, then
unset or restore ZDOTDIR (unset ZDOTDIR or set it to the user's original value)
immediately before sourcing _cmux_file, and after sourcing capture the effective
ZDOTDIR back into _CMUX_ORIG_ZDOTDIR (e.g., inspect $ZDOTDIR or default to
$HOME) and re-export/reset any wrapper variables so later wrappers see the
updated value; update the logic around variables _CMUX_ORIG_ZDOTDIR,
GHOSTTY_ZSH_ZDOTDIR, CMUX_ZSH_ZDOTDIR and _cmux_file accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65d678de-fc75-44ac-9daa-fb6250060139
📒 Files selected for processing (4)
Resources/shell-integration/.zloginResources/shell-integration/.zprofileResources/shell-integration/.zshenvResources/shell-integration/.zshrc
✅ Files skipped from review due to trivial changes (1)
- Resources/shell-integration/.zlogin
🚧 Files skipped from review as they are similar to previous changes (2)
- Resources/shell-integration/.zprofile
- Resources/shell-integration/.zshrc
teamleaderleo
added
area: terminal
This branch has not been deployed
2 participants
Summary
.zshenv, but tools that resetprecmd_functionsin.zprofileor.zshrcclobbered the hooks before the first prompt..zshrcwrapper — after all user startup files have been processed.Affected tools
Confirmed with Kiro CLI (formerly Amazon Q / Fig), which resets
precmd_functionsin.zprofileduring full initialization. Any other tool that resets (rather than appends to)precmd_functionsduring.zprofileor.zshrcprocessing could cause the same issue. The new design is immune because integration loads after all user startup files.Why Finder-only?
When launched via
reload.shoropenfrom a terminal, the app inherits environment variables (e.g.Q_TERM) from the parent shell. Some tools skip their full initialization when these variables are present, preservingprecmd_functions. From Finder, these variables are absent, triggering a full init that resets the array.What changed
.zshenvalwaysblock_CMUX_ORIG_ZDOTDIR, sources user's.zshenv, keeps ZDOTDIR set to integration dir.zprofile.zprofilevia_CMUX_ORIG_ZDOTDIR.zshrc.zshrc, fixes HISTFILE if still pointing to integration dir (respects custom HISTFILE), loads Ghostty + cmux integration, restores ZDOTDIR.zloginTesting
type _cmux_precmd→ shell function found. Workspace sidebar updates oncd.reload.sh --taglaunch: same result, no regression.openfrom terminal launch: same result, no regression.HISTFILEpoints to~/.zsh_history(not the integration dir).HISTFILE(set in user's.zshrc) is preserved after shell startup.ZDOTDIRis unset (or restored to user's original) after shell startup.Known limitation
During the user's
.zshrcexecution,$ZDOTDIRstill points to the integration directory. If a user's.zshrcreferences$ZDOTDIR(e.g.source "$ZDOTDIR/.zsh_aliases"), it will resolve to the wrong path. Affected users are likely rare, but the issue would be hard to diagnose. Please consider addressing this separately.ZDOTDIRbefore sourcing the user's.zshrc, then re-set it afterward.Demo Video
Kiro is not installed
Directory changes are properly reflected in the workspace.
2026-03-23.13.57.22.mov
Kiro is installed
This is cmux built from the latest main branch
Directory changes are not reflected in the workspace.
2026-03-23.14.03.19.mov
Directory changes are properly reflected in the workspace.
2026-03-23.14.07.37.mov
The version with this fix
Directory changes are properly reflected in the workspace.
2026-03-23.14.09.59.mov
Summary by cubic
Fixes
cmuxzsh integration not loading when launched from Finder by keepingZDOTDIRpinned during startup and moving integration load to the.zshrcwrapper. Prevents tools that resetprecmd_functionsfrom clobberingcmuxhooks, then restores the user'sZDOTDIR..zshenvsaves the originalZDOTDIRto_CMUX_ORIG_ZDOTDIR, sources the user’s.zshenv, and defers integration to.zshrc..zshrcsources the user’s file, loads Ghostty andcmuxintegration, fixesHISTFILEif it points to the integration dir, then restoresZDOTDIR..zprofileand.zloginwrappers source the user’s files from_CMUX_ORIG_ZDOTDIR.Written for commit ccf948f. Summary will update on new commits.
Summary by CodeRabbit