fix(contrib): fix scm-prompt triggering chpwd hooks during directory traversal#1325
fix(contrib): fix scm-prompt triggering chpwd hooks during directory traversal#1325vegerot wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Updates SCM prompt path-canonicalization to avoid chpwd_functions side effects while walking up parent directories.
Changes:
- Clears
chpwd_functionsbeforebuiltin cd -Pduring the prompt’s “realpath” loop to suppress Zsh chpwd hook execution in that subshell.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
|
@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this in D106557429. (Because this pull request was imported automatically, there will not be any future comments.) |
…traversal
Summary: Currently, trying to open a new tab in Ghostty within a repository
causes the tab to open in the repository root instead of the current working
directory. This commit fixes the issue.
When scm-prompt.sh walks up the directory tree to find the root of the
repository (.sl, .git, or .hg), it uses `builtin cd -P`. In Zsh,
executing `cd` (even inside a subshell) triggers all functions registered
in `chpwd_functions`.
This causes unexpected side effects if a `chpwd` hook performs operations
that bypass standard output capture. For example, Ghostty's shell integration
registers a `chpwd` hook that writes an OSC 7 escape sequence directly to a
persistent TTY file descriptor (/dev/tty) to report the current working
directory. Because this writes to the TTY directly, the traversal performed by
`scm-prompt.sh` leaks temporary working directories to the terminal emulator,
causing Ghostty to open new tabs in the repository root rather than the actual
current working directory.
By localizing and clearing `chpwd_functions` within the subshell before executing
`cd`, we prevent these hooks from firing during the directory traversal, resolving
the side effects while keeping the traversal logic intact.
Test plan:
Run this script. Before this commit, it will print the path to the repository
root. After this commit, it will print the current working directory.
```sh
#!/usr/bin/env zsh
# Clear previous output
rm -f /tmp/ghostty_bug_repro.txt
rm -f /tmp/ghostty_log.txt
# Create a self-contained temporary home/zdotdir for this test
REPRO_ZDOTDIR=$(mktemp -d)
# Write out the minimal reproducible .zshrc
cat << 'ZSHRC_EOF' > "$REPRO_ZDOTDIR/.zshrc"
# We need prompt_subst to allow function execution in prompt
setopt prompt_subst
# Load the buggy sapling prompt (assuming it's installed at this path)
source ~/workspace/github.com/facebook/sapling/eden/scm/contrib/scm-prompt.sh
# Set the prompt to trigger the bug
export PS1='$(_scm_prompt) %~ %# '
# To test Ghostty OSC 7 tracking, we need Ghostty shell integration
source "$GHOSTTY_RESOURCES_DIR/shell-integration/zsh/ghostty-integration"
ZSHRC_EOF
# Launch Ghostty with logging and our minimal zshrc
# We use ZDOTDIR to point zsh to our custom zshrc
GHOSTTY_LOG=stderr ZDOTDIR="$REPRO_ZDOTDIR" /Users/bytedance/workspace/github.com/ghostty-org/ghostty/zig-out/Ghostty.app/Contents/MacOS/ghostty > /tmp/ghostty_log.txt 2>&1 &
GHOSTTY_PID=$!
# Give it a moment to start
sleep 1
# Run the AppleScript to drive the UI
osascript << 'APPLESCRIPT'
tell application "System Events"
tell application "/Users/bytedance/workspace/github.com/ghostty-org/ghostty/zig-out/Ghostty.app" to activate
delay 1
-- cd to a deeply nested directory in the sapling repo
keystroke "cd ~/workspace/github.com/facebook/sapling/eden/scm/"
keystroke return
delay 1
-- open a new tab
keystroke "t" using command down
delay 1
-- get the pwd in the new tab and write it to a file
keystroke "pwd > /tmp/ghostty_bug_repro.txt"
keystroke return
delay 1
-- quit Ghostty
keystroke "q" using command down
end tell
APPLESCRIPT
# Read the output
echo "The new tab's working directory was:"
cat /tmp/ghostty_bug_repro.txt
echo "Ghostty PID was $GHOSTTY_PID"
echo "Ghostty logs are in: /tmp/ghostty_log.txt"
# Clean up
rm -rf "$REPRO_ZDOTDIR"
```
Co-authored-by: Trae <trae@bytedance.com>
|
@vegerot has updated the pull request. You must reimport the pull request before landing. |
quark-zju
left a comment
There was a problem hiding this comment.
Review automatically exported from Phabricator review in Meta.
|
This pull request has been merged in e4453e0. |
Summary: Currently, trying to open a new tab in Ghostty within a repository
causes the tab to open in the repository root instead of the current working
directory. This commit fixes the issue.
When scm-prompt.sh walks up the directory tree to find the root of the
repository (.sl, .git, or .hg), it uses
builtin cd -P. In Zsh,executing
cd(even inside a subshell) triggers all functions registeredin
chpwd_functions.This causes unexpected side effects if a
chpwdhook performs operationsthat bypass standard output capture. For example, Ghostty's shell integration
registers a
chpwdhook that writes an OSC 7 escape sequence directly to apersistent TTY file descriptor (/dev/tty) to report the current working
directory. Because this writes to the TTY directly, the traversal performed by
scm-prompt.shleaks temporary working directories to the terminal emulator,causing Ghostty to open new tabs in the repository root rather than the actual
current working directory.
By localizing and clearing
chpwd_functionswithin the subshell before executingcd, we prevent these hooks from firing during the directory traversal, resolvingthe side effects while keeping the traversal logic intact.
Test plan:
Run this script. Before this commit, it will print the path to the repository
root. After this commit, it will print the current working directory.
Co-authored-by: Trae trae@bytedance.com