Live read-side sync between Warp and OS shell history files (#3422)#9584
Live read-side sync between Warp and OS shell history files (#3422)#9584tarun-khatri wants to merge 3 commits intowarpdotdev:masterfrom
Conversation
…ev#3422) When the user opts into the new `terminal.live_sync_os_shell_history` setting, Warp watches the active shell's history file (`~/.zsh_history`, `~/.bash_history`, fish, PSReadLine) for changes made by other terminals and merges new commands into Warp's autocomplete in real time, without needing a session restart. This implements the read-side half of warpdotdevGH-3422 (vscode/external terminal -> warp). The write-side half (warp -> histfile) is intentionally deferred to a follow-up PR — writing to user-owned files in $HOME has materially more edge cases (HISTCONTROL/HISTIGNORE/HISTSIZE semantics, advisory locking against the live shell) and benefits from being its own focused review. Implementation: * New singleton `ShellHistoryWatcher` (`app/src/terminal/ shell_history_watcher.rs`) wraps a dedicated `BulkFilesystemWatcher` with a refcounted `register_histfile` / `unregister_histfile` API. The existing `HomeDirectoryWatcher` only watches $HOME non-recursively, which covers bash and zsh but not fish or PSReadLine, so we register the actual histfile paths instead. * New settings group `ShellHistorySyncSettings` with one boolean `terminal.live_sync_os_shell_history` (default `false`). * `History::init_session_with` calls a new helper `maybe_register_live_sync` which, when the setting is on, expands the active session's `shell_type.history_files()` candidates and registers the existing paths with the watcher. * `History::set_up_external_history_sync` (called once at app startup from `lib.rs`) subscribes to `ShellHistoryWatcherEvent`. On each event, `handle_shell_history_watcher_event` re-reads the affected files via `async_fs`, parses with the existing per-shell `ShellType::parse_history`, and dispatches to `apply_external_history_lines`. * `apply_external_history_lines` is the merge: dedupe via `dedupe_from_last`, append entries not already in `history_file_commands[host]`, shift `session_start_indices` and `session_skip_indices` upward by N for every session on that host (the per-host history list is the concatenation `history_file_commands ++ session_commands`, so inserting at the boundary shifts every index >= old_boundary), and emit a new `HistoryEvent::ExternalHistoryUpdated { host, num_appended }`. * The four call sites that pattern-matched on `HistoryEvent::Initialized` are updated to handle the new variant. Testing: * Two new unit tests in `app/src/terminal/history_tests.rs` cover the merge logic — `external_history_lines_append_new_tail_only` checks that re-applying a histfile re-read appends only the genuinely-new lines and is idempotent on no-op re-applies, and `external_history_lines_emit_external_history_updated_event` confirms the new event variant fires with the right host and count. * `cargo check -p warp` clean locally. Tracks warpdotdev#3422.
|
I'm starting a first review of this pull request. I completed the review and posted feedback on this pull request. Comment I'm re-reviewing this pull request in response to a review request. I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment You can view the conversation on Warp. I completed the review and posted feedback on this pull request. Comment I completed the review and posted feedback on this pull request. Comment Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
Overview
This PR adds an opt-in read-side shell history watcher, a new settings group, and History model merge events so commands from OS history files can appear in Warp autocomplete without restarting.
Concerns
- The new watcher module is compiled unconditionally even though its dependencies are non-wasm-only, which will break wasm builds unless the module and History watcher integration are cfg-gated or stubbed.
- Live sync registration currently applies to remote sessions and resolves local home-directory histfiles, which can merge local shell history into remote-session suggestions.
- The watched path list ignores the actual session HISTFILE value, so custom histfiles are loaded initially but not live-synced.
- The merge path filters by set membership, so rerunning an existing command in another terminal does not update recency/order despite full-file rereads using dedupe-from-last semantics.
Verdict
Found: 1 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| pub mod settings; | ||
| mod share_block_modal; | ||
| pub mod shared_session; | ||
| pub mod shell_history_watcher; |
There was a problem hiding this comment.
🚨 [CRITICAL] This module is compiled on wasm too, but shell_history_watcher.rs imports watcher and notify_debouncer_full, which are only app dependencies under cfg(not(target_family = "wasm")); gate the module and all History watcher integration on non-wasm or provide a wasm stub.
| // this host's histfile path(s) with `ShellHistoryWatcher` so subsequent | ||
| // changes by other terminals are merged into `history_file_commands`. | ||
| // No-op when the setting is off. | ||
| self.maybe_register_live_sync(&host, ctx); |
There was a problem hiding this comment.
WarpifiedRemote; maybe_register_live_sync resolves dirs::home_dir() and reads local histfiles, so local shell history can be merged into a remote session's autocomplete.
| return; | ||
| }; | ||
|
|
||
| let candidate_paths: Vec<PathBuf> = host |
There was a problem hiding this comment.
host.shell_type.history_files() ignores the actual session histfile; users with custom HISTFILE are initially loaded from that path but live sync watches the default file instead.
|
|
||
| let to_append: Vec<String> = new_deduped | ||
| .into_iter() | ||
| .filter(|cmd| !existing_commands.contains(cmd)) |
There was a problem hiding this comment.
captainsafia
added
the
external-contributor
Warp Dev Github Integration
Four follow-up fixes for the read-side shell-history sync, in response to the Oz review on PR warpdotdev#9584: * **wasm**: `shell_history_watcher` and the History-side integration pulled in non-wasm-only `BulkFilesystemWatcher`, breaking wasm builds. The module declaration in `terminal/mod.rs` and the methods that touch it (`set_up_external_history_sync`, `handle_shell_history_watcher_event`, `maybe_register_live_sync`) are now `#[cfg(not(target_family = "wasm"))]`-gated, with no-op wasm stubs for the two pub methods so callers don't need their own cfg branches. * **remote sessions**: live-sync registered the *local* home-dir histfile for any session, including `WarpifiedRemote` sessions — which would mistakenly merge local command history into the remote session's autocomplete. `maybe_register_live_sync` now early-returns unless `session.session_type()` is `SessionType::Local`. * **custom HISTFILE**: the watched paths only consulted `shell_type.history_files()` defaults, so users with `HISTFILE=/some/custom/path` got the initial read but no live updates. Path resolution now prefers `session.histfile()` when set and falls back to the shell-default candidates. * **recency on re-run**: `apply_external_history_lines` filtered new lines by set membership against the current cache, so re-running an existing command in another terminal didn't move it to the most-recent position despite the deduped re-read. Switch from "append-not-already-present" to "replace-with-the-new-deduped-list": cache becomes whatever `dedupe_from_last` produces for the freshly re-read file, with session-index bookkeeping shifted by the size delta and skip-indices that pointed into the now-replaced history_file portion dropped (those entries are gone). Adds a cheap identical-content short-circuit to skip the rebuild + event emit when the watcher fires for a touch that didn't actually change deduped contents. `cargo check -p warp` clean locally. Tracks warpdotdev#3422.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds an opt-in live read-side shell history sync path, including a new watcher singleton, setting registration, History merge logic, and command-search/view event handling.
Concerns
- Command search history results are backed by snapshot data sources; the new
ExternalHistoryUpdatedhandling only notifies the mixer and is only installed in the not-yet-queryable branch, so already-open command search panels will continue showing stale history after live sync updates. - The live reread path uses
async_fs::readdirectly for every shell, bypassing the existing Windows PowerShell/Kaspersky history-read fallback used by initial history loading. - Security pass: no security-specific findings.
Verdict
Found: 0 critical, 2 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| ctx.notify(); | ||
| } | ||
| } | ||
| HistoryEvent::ExternalHistoryUpdated { .. } => { |
There was a problem hiding this comment.
!is_queryable branch and ctx.notify() does not rebuild the source or rerun the current query, so command search stays stale for already-queryable sessions after live history updates.
| let host_for_apply = host.clone(); | ||
| ctx.spawn( | ||
| async move { | ||
| async_fs::read(&path_for_read) |
There was a problem hiding this comment.
async_fs::read path that initial history loading deliberately avoids.
There was a problem hiding this comment.
Overview
This PR adds opt-in live read-side syncing from OS shell history files into Warp history/autocomplete via a new watcher, setting, and History merge path.
Concerns
- Watch registration happens before the initial histfile load completes, so an early watcher event can be overwritten by the startup read.
- Watcher reads are applied without ordering/generation checks, so stale async reads can roll back newer history updates.
- The live read path bypasses the existing PowerShell/Kaspersky history-reading workaround and uses async_fs directly.
- Replacing histfile commands only shifts skip indices; it does not re-dedupe against session commands, so commands written by the Warp shell itself can appear twice.
Found: 0 critical, 4 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // `ShellHistoryWatcher` so subsequent changes by other terminals are | ||
| // merged into `history_file_commands`. No-op when the setting is off, | ||
| // when the session is remote (we only watch local files), or on wasm. | ||
| self.maybe_register_live_sync(&session, &host, ctx); |
There was a problem hiding this comment.
load_history_file_commands can overwrite them when the startup read completes; register after initialization or ignore events until ReadHistoryFileState::Done.
| let host_for_apply = host.clone(); | ||
| ctx.spawn( | ||
| async move { | ||
| async_fs::read(&path_for_read) |
There was a problem hiding this comment.
read_history_contents fallback and always uses async_fs::read, so PSReadLine live sync can reintroduce the Windows antivirus failure path. Reuse the same reader as Session::read_history_for_local_session.
| .ok() | ||
| .map(|bytes| shell_type.parse_history(&bytes)) | ||
| }, | ||
| move |me, lines_opt, ctx| { |
There was a problem hiding this comment.
| .unwrap_or(0); | ||
| let new_history_file_len = new_entries.len(); | ||
|
|
||
| self.history_file_commands |
There was a problem hiding this comment.
session_commands means a command written by Warp's own shell, or any external command matching a session command, appears twice because the skip indices are only shifted. Recompute skips after the replacement or mark duplicates like append_commands does.
Four follow-ups for the live read-side history sync, in response to the third Oz review pass: 1. Race: register watcher AFTER initial load completes The first version called maybe_register_live_sync() before kicking off the async initial-load future, so a watcher event arriving between registration and load completion could trigger a re-read whose result would then be overwritten by the still-pending initial load. Move the registration into the post-load callback (None branch of init_session_with) so it only fires once load_history_file_commands has populated history_file_commands. The Done branch (initial load already happened on a prior session) registers inline as before; InProgress now no-ops because the in-flight sessions
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds an opt-in live shell-history watcher, a synced setting, and merge logic that re-reads OS histfiles into Warp history/autocomplete.
Concerns
- Default Windows PowerShell history is not watched because absolute
history_files()candidates are filtered out instead of preserved. - The generation guard for concurrent watcher re-reads lets the first completion win, which can drop a newer re-read and leave stale history in memory.
- Session skip indices are shifted by aggregate counts after deduping session commands, so indices for removed commands or commands after only some removals can hide the wrong entries.
- Security pass: no additional security issues found in the changed lines.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| .into_iter() | ||
| .filter_map(|p| { | ||
| // `history_files()` returns `~/...`-prefixed strings; expand them. | ||
| let stripped = p.strip_prefix("~/").or_else(|| p.strip_prefix("~"))?; |
There was a problem hiding this comment.
ShellType::PowerShell::history_files() returns an absolute PSReadLine path, so this strip_prefix("~/") / strip_prefix("~") filter drops it and live sync never registers the default PowerShell history file. Preserve absolute candidates and only expand paths that actually start with ~.
| }; | ||
| // Snapshot the generation so a stale completion can't roll | ||
| // back a newer apply that landed first. | ||
| let snapshot_gen = self.live_sync_generation.get(&host).copied().unwrap_or(0); |
There was a problem hiding this comment.
| *skips = skips | ||
| .iter() | ||
| .filter_map(|&i| { | ||
| if i >= old_history_file_len { |
There was a problem hiding this comment.
There was a problem hiding this comment.
Overview
This PR adds opt-in live read-side shell-history syncing by registering histfile watches, re-reading changed history files, and merging the parsed results into the History model so autocomplete consumers can refresh.
Concerns
- Concurrent watcher reads can discard the fresher result when an older read applies first, leaving live history stale until another filesystem event.
- Replacing duplicate session commands by command string can remove this Warp session's own HistoryEntry when the active shell writes to HISTFILE, losing session metadata and preventing completion updates from being applied to that entry.
- The setting is only checked during registration, so turning the opt-in setting off after a session registered a path does not stop watching or merging.
- Live-sync registration does not route candidate histfile paths through the same native-path conversion as the initial read, so WSL/MSYS2 local sessions can fail to watch the file that was actually loaded.
- Supplemental security pass: no direct security vulnerability found in the changed lines beyond the opt-out/lifecycle behavior noted above.
Verdict
Found: 0 critical, 4 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // re-read started, drop the result. Newer state has already landed. | ||
| if let Some(snap) = snapshot_gen { | ||
| let current_gen = self.live_sync_generation.get(&host).copied().unwrap_or(0); | ||
| if current_gen != snap { |
There was a problem hiding this comment.
| // newly-replaced history_file_commands. This keeps each command | ||
| // visible exactly once in the rendered autocomplete list. | ||
| if let Some(session_cmds) = self.session_commands.get_mut(&host) { | ||
| session_cmds.retain(|entry| !new_history_command_set.contains(&entry.command)); |
There was a problem hiding this comment.
session_commands entry by command string can drop this Warp session's own entry when the live shell writes the command to HISTFILE, losing pwd/timestamps/session_id metadata and leaving mark_command_as_finished with nothing to update. Keep session entries and hide duplicates with skip indices, or only remove entries proven to be externally mirrored.
| return; | ||
| } | ||
|
|
||
| let enabled = *ShellHistorySyncSettings::as_ref(ctx) |
There was a problem hiding this comment.
terminal.live_sync_os_shell_history does not unregister or suppress watcher events, so the feature keeps reading and merging until restart. Re-check the setting in the event path or subscribe to setting changes and unregister registered paths.
| // overrides in the user's rc files). Fall back to the shell's default | ||
| // candidates expanded against the home directory. | ||
| let candidate_paths: Vec<PathBuf> = if let Some(custom) = session.histfile().as_deref() { | ||
| vec![PathBuf::from(custom)] |
There was a problem hiding this comment.
Session::maybe_convert_to_native_path logic used by Session::read_history, so WSL/MSYS2 local sessions can load one native histfile path but check/watch a different POSIX path here and never receive live updates. Convert candidate paths through the session before exists() and watcher registration.
There was a problem hiding this comment.
Overview
This PR adds an opt-in live shell-history watcher, a synced setting, and merge logic that re-reads OS histfiles into Warp history/autocomplete.
Concerns
- Default Windows PowerShell history is not watched because absolute
history_files()candidates are filtered out instead of preserved. - The generation guard for concurrent watcher re-reads lets the first completion win, which can drop a newer re-read and leave stale history in memory.
- Session skip indices are shifted by aggregate counts after deduping session commands, so indices for removed commands or commands after only some removals can hide the wrong entries.
- Security pass: no additional security issues found in the changed lines.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| .into_iter() | ||
| .filter_map(|p| { | ||
| // `history_files()` returns `~/...`-prefixed strings; expand them. | ||
| let stripped = p.strip_prefix("~/").or_else(|| p.strip_prefix("~"))?; |
There was a problem hiding this comment.
ShellType::PowerShell::history_files() returns an absolute PSReadLine path, so this strip_prefix("~/") / strip_prefix("~") filter drops it and live sync never registers the default PowerShell history file. Preserve absolute candidates and only expand paths that actually start with ~.
| }; | ||
| // Snapshot the generation so a stale completion can't roll | ||
| // back a newer apply that landed first. | ||
| let snapshot_gen = self.live_sync_generation.get(&host).copied().unwrap_or(0); |
There was a problem hiding this comment.
| *skips = skips | ||
| .iter() | ||
| .filter_map(|&i| { | ||
| if i >= old_history_file_len { |
There was a problem hiding this comment.
2 participants
Description
Tracks #3422.
This PR implements the read-side of bidirectional shell-history sync: when the user has opted in via the new
terminal.live_sync_os_shell_historysetting, Warp watches the active shell's history file (~/.zsh_history,~/.bash_history, fish, PSReadLine) for changes made by other terminals and merges new commands into Warp's autocomplete in real time — without needing the user to restart the session.The write-side half of #3422 (Warp commands appended back to the OS histfile) is intentionally deferred to a follow-up PR; it's materially more invasive (writes to user-owned files in
$HOME, has to honorHISTCONTROL/HISTIGNORE/HISTSIZE, needs advisory locking against the live shell) and benefits from being its own focused review.What changes
app/src/terminal/shell_history_watcher.rs(~130 LOC) wraps a dedicatedBulkFilesystemWatcherand exposes a refcountedregister_histfile/unregister_histfileAPI. The existingHomeDirectoryWatcheronly watches$HOMEnon-recursively, so it covers bash and zsh but not fish (~/.local/share/fish/fish_history) or PSReadLine; this dedicated watcher registers the actual histfile paths.app/src/settings/shell_history_sync.rswith one boolean:terminal.live_sync_os_shell_history(defaultfalse).Historymodel inapp/src/terminal/history.rs:live_sync_paths: HashMap<PathBuf, HashSet<ShellHost>>tracks which histfile paths feed which hosts.set_up_external_history_sync()is called once at app startup fromlib.rsto subscribeHistoryto the new watcher.init_session_with()calls a new helpermaybe_register_live_sync()which registers the active session's histfile path with the watcher when the setting is on. No-op when off.handle_shell_history_watcher_event()re-reads each affected file (async, viaasync_fs) and parses it with the existing per-shellShellType::parse_history(already handles zsh extended-history, bashHISTTIMEFORMAT, fish, PSReadLine).apply_external_history_lines()does the merge: dedupe viadedupe_from_last, append entries not already inhistory_file_commands[host], shiftsession_start_indicesandsession_skip_indicesupward bynfor every session on that host (the per-host history list ishistory_file_commands ++ session_commands, so inserting at the boundary shifts every index>= old_boundaryup byn), and emit a newHistoryEvent::ExternalHistoryUpdated { host, num_appended }event so consumers (autocomplete, command-search) re-query.HistoryEvent::Initializedare updated to handle the new variant.Why opt-in
Live filesystem watches per-shell are cheap but non-zero (one inotify/FSEvents handle per active shell type). Defaulting off avoids changing behavior for existing users; defaulting on can be a separate, lower-risk follow-up once the read path has soaked.
Testing
Added unit tests in
app/src/terminal/history_tests.rsexternal_history_lines_append_new_tail_only— initializes history with["a","b","c"], simulates a re-read producing["a","b","c","d","e"], asserts the autocomplete list ends with[a, b, c, d, e]. Re-applying the same content is a no-op (idempotency).external_history_lines_emit_external_history_updated_event— subscribes toHistoryevents, callsapply_external_history_lines, assertsExternalHistoryUpdated { host, num_appended: 2 }is emitted with the right host.The merge logic (which is the riskiest piece — index shifting against existing
session_start_indices/session_skip_indices) is unit-tested directly. The watcher → re-read pipeline is glue over standard infrastructure (BulkFilesystemWatcher,async_fs::read,ShellType::parse_history) all of which are already tested in their own crates.Manual verification
Built locally with
cargo check -p warpon Linux. Did not run the fullscript/presubmit(host this PR was developed on lacks the full asset toolchain — nowgslfmt/clang-format installed). CI will run the full presubmit on submission.Open design questions for reviewers
A few choices worth flagging — happy to iterate on any of these:
terminal.live_sync_os_shell_history. Considered nesting under an existing group; ended up with a new group because the feature is self-contained and discoverable. Open to renaming or relocating.modifiedevent and lets the dedupe path filter. For typical histfile sizes (few hundred KB) this is well under 1 ms and runs off the main loop. For users withHISTSIZE=1000000this is wasteful; an incremental byte-offset reader is a possible v2 optimization. OK to defer?~/.zsh_history. Trade-off vs. complexity of watching parent dirs. OK as-is for v1?Server API dependencies
This PR has no server dependencies.
Changelog Entries for Stable
CHANGELOG-IMPROVEMENT: Optional bidirectional read-side shell history sync — when enabled, commands run in other terminals (VS Code's integrated terminal, plain
zsh, etc.) appear in Warp's autocomplete in real time. Off by default; toggleterminal.live_sync_os_shell_history. Tracks GH-3422.