Skip to content

feat: implement per-block zsh history isolation #3016

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
duerzy wants to merge 2 commits into
base: main
Choose a base branch
from
Open

feat: implement per-block zsh history isolation #3016

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ce7441f
feat: implement per-block zsh history isolation
duerzy Mar 9, 2026
6e8cbe0
Merge branch 'main' into main
duerzy Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions pkg/util/shellutil/shellintegration/zsh_zshrc.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ if [[ -n ${_comps+x} ]]; then
source <(wsh completion zsh)
fi

# fix history (macos)
if [[ "$HISTFILE" == "$WAVETERM_ZDOTDIR/.zsh_history" ]]; then
# per-block independent history
if [[ -n "$WAVETERM_BLOCKID" ]]; then
_waveterm_hist_dir="$HOME/.waveterm/shell/zsh/history/$WAVETERM_BLOCKID"
[[ -d "$_waveterm_hist_dir" ]] || mkdir -p "$_waveterm_hist_dir"
HISTFILE="$_waveterm_hist_dir/.zsh_history"
unset _waveterm_hist_dir
elif [[ "$HISTFILE" == "$WAVETERM_ZDOTDIR/.zsh_history" ]]; then
Comment on lines +21 to +27
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Gate per-block history here unless non-darwin merge support exists.

This branch now isolates zsh history for every block, but pkg/util/shellutil/shellutil.go still makes FixupWaveZshHistory() a no-op when runtime.GOOS != "darwin". On Linux, those commands never get merged back into ~/.zsh_history, so the new history path becomes a dead end. Either scope this behavior to the platforms that already have merge support, or add the same merge path for the other supported zsh platforms.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/util/shellutil/shellintegration/zsh_zshrc.sh` around lines 21 - 27, The
per-block history branch that sets HISTFILE when WAVETERM_BLOCKID is present
should be limited to platforms that have merge support; either gate the zsh
snippet with a platform check (e.g. only run the WAVETERM_BLOCKID branch when
OSTYPE/uname indicates darwin) or implement the corresponding merge logic in
FixupWaveZshHistory() so histories are merged back on non-darwin systems; update
the zsh block that references WAVETERM_BLOCKID and HISTFILE or modify the Go
function FixupWaveZshHistory() accordingly so non-darwin installs do not write
to a dead-end history path.

HISTFILE="$HOME/.zsh_history"
fi

Expand Down
109 changes: 85 additions & 24 deletions pkg/util/shellutil/shellutil.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,42 @@ func HasWaveZshHistory() (bool, int64) {
return true, fileInfo.Size()
}

func GetBlockZshHistoryDir(blockId string) string {
zshDir := GetLocalZshZDotDir()
return filepath.Join(zshDir, "history", blockId)
}

func CleanupBlockZshHistory(blockId string) error {
histDir := GetBlockZshHistoryDir(blockId)
err := os.RemoveAll(histDir)
if err != nil && !os.IsNotExist(err) {
return fmt.Errorf("error removing block history dir %s: %w", histDir, err)
}
log.Printf("cleaned up block zsh history dir: %s\n", histDir)
return nil
}

// getPerBlockHistoryFiles returns a list of per-block .zsh_history file paths
func getPerBlockHistoryFiles() []string {
zshDir := GetLocalZshZDotDir()
historyBaseDir := filepath.Join(zshDir, "history")
entries, err := os.ReadDir(historyBaseDir)
if err != nil {
return nil
}
var files []string
for _, entry := range entries {
if !entry.IsDir() {
continue
}
histFile := filepath.Join(historyBaseDir, entry.Name(), ZshHistoryFileName)
if _, err := os.Stat(histFile); err == nil {
files = append(files, histFile)
}
}
return files
}

func IsExtendedZshHistoryFile(fileName string) (bool, error) {
file, err := os.Open(fileName)
if err != nil {
Expand Down Expand Up @@ -550,29 +586,34 @@ func FixupWaveZshHistory() error {
return nil
}

hasHistory, size := HasWaveZshHistory()
if !hasHistory {
return nil
homeDir, err := os.UserHomeDir()
if err != nil {
return fmt.Errorf("error getting home directory: %w", err)
}
realHistFile := filepath.Join(homeDir, ".zsh_history")

// Collect all history files to merge (legacy wave history + per-block histories)
var histFilesToMerge []string

// Check for legacy wave zsh history file
zshDir := GetLocalZshZDotDir()
waveHistFile := filepath.Join(zshDir, ZshHistoryFileName)

if size == 0 {
err := os.Remove(waveHistFile)
if err != nil {
log.Printf("error removing wave zsh history file %s: %v\n", waveHistFile, err)
}
return nil
if fileInfo, err := os.Stat(waveHistFile); err == nil && fileInfo.Size() > 0 {
histFilesToMerge = append(histFilesToMerge, waveHistFile)
} else if err == nil && fileInfo.Size() == 0 {
// remove empty file
os.Remove(waveHistFile)
}

log.Printf("merging wave zsh history %s into ~/.zsh_history\n", waveHistFile)
// Collect per-block history files
perBlockFiles := getPerBlockHistoryFiles()
histFilesToMerge = append(histFilesToMerge, perBlockFiles...)

homeDir, err := os.UserHomeDir()
if err != nil {
return fmt.Errorf("error getting home directory: %w", err)
if len(histFilesToMerge) == 0 {
return nil
}
realHistFile := filepath.Join(homeDir, ".zsh_history")

log.Printf("merging %d wave zsh history file(s) into ~/.zsh_history\n", len(histFilesToMerge))

isExtended, err := IsExtendedZshHistoryFile(realHistFile)
if err != nil {
Expand All @@ -584,7 +625,12 @@ func FixupWaveZshHistory() error {
hasExtendedStr = "true"
}

quotedWaveHistFile := utilfn.ShellQuote(waveHistFile, true, -1)
// Build fc -RI commands for each history file
var fcCommands string
for _, hf := range histFilesToMerge {
quotedFile := utilfn.ShellQuote(hf, true, -1)
fcCommands += fmt.Sprintf("\t\tfc -RI %s\n", quotedFile)
}

script := fmt.Sprintf(`
HISTFILE=~/.zsh_history
Expand All @@ -593,11 +639,10 @@ func FixupWaveZshHistory() error {
has_extended_history=%s
[[ $has_extended_history == true ]] && setopt EXTENDED_HISTORY
fc -RI
fc -RI %s
fc -W
`, hasExtendedStr, quotedWaveHistFile)
%s fc -W
`, hasExtendedStr, fcCommands)

ctx, cancelFn := context.WithTimeout(context.Background(), 5*time.Second)
ctx, cancelFn := context.WithTimeout(context.Background(), 10*time.Second)
defer cancelFn()

cmd := exec.CommandContext(ctx, "zsh", "-f", "-i", "-c", script)
Expand All @@ -610,11 +655,27 @@ func FixupWaveZshHistory() error {
return fmt.Errorf("error executing zsh history fixup script: %w, output: %s", err, string(output))
}

err = os.Remove(waveHistFile)
if err != nil {
log.Printf("error removing wave zsh history file %s: %v\n", waveHistFile, err)
// Clean up merged history files
for _, hf := range histFilesToMerge {
err = os.Remove(hf)
if err != nil {
log.Printf("error removing wave zsh history file %s: %v\n", hf, err)
}
}
log.Printf("successfully merged wave zsh history %s into ~/.zsh_history\n", waveHistFile)

// Remove empty per-block history directories
historyBaseDir := filepath.Join(zshDir, "history")
entries, err := os.ReadDir(historyBaseDir)
if err == nil {
for _, entry := range entries {
if entry.IsDir() {
blockDir := filepath.Join(historyBaseDir, entry.Name())
os.Remove(blockDir) // only removes if empty
}
}
}

log.Printf("successfully merged %d wave zsh history file(s) into ~/.zsh_history\n", len(histFilesToMerge))

return nil
}
Expand Down
5 changes: 5 additions & 0 deletions pkg/wcore/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/wavetermdev/waveterm/pkg/panichandler"
"github.com/wavetermdev/waveterm/pkg/telemetry"
"github.com/wavetermdev/waveterm/pkg/telemetry/telemetrydata"
"github.com/wavetermdev/waveterm/pkg/util/shellutil"
"github.com/wavetermdev/waveterm/pkg/util/utilfn"
"github.com/wavetermdev/waveterm/pkg/waveobj"
"github.com/wavetermdev/waveterm/pkg/wps"
Expand Down Expand Up @@ -170,6 +171,10 @@ func DeleteBlock(ctx context.Context, blockId string, recursive bool) error {
if err != nil {
return fmt.Errorf("error deleting block: %w", err)
}
// Clean up per-block zsh history directory
if cleanupErr := shellutil.CleanupBlockZshHistory(blockId); cleanupErr != nil {
log.Printf("error cleaning up block zsh history for %s: %v\n", blockId, cleanupErr)
}
Comment on lines +174 to +177
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Defer block-history cleanup until after shell teardown or merge.

Line 175 removes the directory that the live zsh session is using as HISTFILE, and it does that before sendBlockCloseEvent(blockId) runs. If the shell is still alive, its final history write loses the target path; if the block is deleted before the later FixupWaveZshHistory() pass, that block’s history never gets merged at all. Cleanup should happen after the shell has been stopped and its history has been persisted.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@pkg/wcore/block.go` around lines 174 - 177, The cleanup call currently
deletes the zsh history dir too early (shell may still be writing); move the
call to shellutil.CleanupBlockZshHistory(blockId) so it runs only after the
shell teardown/merge sequence completes — i.e., after
sendBlockCloseEvent(blockId) and after FixupWaveZshHistory() (or whatever path
ensures the shell has stopped and history has been persisted/merged); ensure the
cleanup is deferred until those functions return/confirm completion to avoid
losing the final history write or preventing history merge.

log.Printf("DeleteBlock: parentBlockCount: %d", parentBlockCount)
parentORef := waveobj.ParseORefNoErr(block.ParentORef)

Expand Down