Update tab CWD and git branch from OSC 7 escape sequences#9279
Update tab CWD and git branch from OSC 7 escape sequences#9279waynehoover wants to merge 19 commits into
Conversation
Warp's per-block working directory was only refreshed during the shell's prompt rendering cycle (precmd), so external tools that change directory mid-command — e.g. worktrunk's `wt switch` — left the tab title and git branch chip stuck on the old value until the user submitted another command. The OSC 7 escape `\e]7;file://host/path\a` is the standard way for those tools to inform the terminal of a new CWD. Wire OSC 7 through the terminal handler: * Parse the sequence in the OSC dispatcher, validating that the host is empty, `localhost`, or the local hostname (case-insensitive) and percent-decoding the path. * Add `Handler::set_current_working_directory`, which Block applies by updating its pwd and emitting `BlockMetadataReceived` so the tab title and git-status watcher pick up the change immediately. * Delegate the call from TerminalModel through Blocks to the active block. Tests: 10 ansi unit tests for parsing, ST/BEL terminators, percent decoding, host validation, and malformed input; a block unit test for the pwd update and event emission; a bash+zsh integration test that runs `printf '\033]7;file:///tmp/osc7-test\a'` and asserts the block's pwd. Fixes warpdotdev#9125.
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @waynehoover on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
I'm starting a first review of this pull request. I requested changes on this pull request and posted feedback. Comment I approved this pull request. No matching stakeholder was found for the changed files, so no human reviewers were requested. Comment I'm re-reviewing this pull request in response to a review request. You can view the conversation on Warp. I reviewed this pull request and requested human review from: @zachbai. I left feedback as a comment so a maintainer can approve. Comment Powered by Oz |
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
![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 OSC 7 parsing and handler propagation so block CWD metadata can update immediately from terminal escape sequences, with unit and integration coverage.
Concerns
- OSC 7 metadata from in-band command blocks is emitted as a normal metadata update, bypassing the existing in-band guard around repo detection and prompt chip refreshes.
- The percent decoder still accepts truncated
%escapes as literal bytes, contrary to the malformed-input rejection intent.
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
| .send_terminal_event(Event::BlockMetadataReceived(BlockMetadataReceivedEvent { | ||
| block_metadata: self.metadata(), | ||
| block_index: self.block_index, | ||
| is_after_in_band_command: false, |
There was a problem hiding this comment.
| is_after_in_band_command: false, | |
| is_after_in_band_command: self.is_for_in_band_command, |
| let mut i = 0; | ||
| while i < bytes.len() { | ||
| match bytes[i] { | ||
| b'%' if i + 2 < bytes.len() => { |
There was a problem hiding this comment.
% or %x falls through to the literal-byte arm and is accepted as a CWD, so malformed OSC 7 URI escapes still update state; reject any % that is not followed by two hex digits.
* Preserve the block's in-band status when emitting the `BlockMetadataReceived` event from `set_current_working_directory`. Reporting `is_after_in_band_command: false` for an in-band block bypassed the listener-side guard that suppresses repo detection / prompt chip refreshes for in-band command blocks, which could reintroduce the refresh loop that flag exists to prevent. * Reject truncated `%` escapes in the OSC 7 percent decoder. A trailing `%` or `%x` previously fell through to the literal-byte arm and was accepted as part of the CWD; now it returns `None` so the OSC 7 sequence is ignored. Adds two tests covering both cases.
|
/oz-review |
|
I'm starting a first review of this pull request. You can follow along in the session on Warp. I requested changes on this pull request and posted feedback. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds OSC 7 CWD parsing and routes parsed paths through TerminalModel/BlockList/Block so tab metadata and git status update when tools emit CWD changes, with unit and integration coverage.
Concerns
- The OSC 7 host/path payload is terminal-controlled. Accepting hostless or
localhostpayloads without checking the active session locality lets remote terminal output spoof a local CWD and trigger local repo detection/git-status work on arbitrary local paths. gethostnameis only declared for non-wasm app builds, but the new parser references it unconditionally.
Security
- OSC 7 application needs a trusted session-locality check before using terminal output to drive local filesystem/repo metadata.
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
| } | ||
|
|
||
| fn osc_7_host_is_local(host: &str) -> bool { | ||
| if host.is_empty() || host.eq_ignore_ascii_case("localhost") { |
There was a problem hiding this comment.
host comes from untrusted terminal output, so an SSH/remote process can emit file:///... or file://localhost/... and make Warp treat an arbitrary local path as the active CWD, which then drives local repo detection/git status; gate OSC 7 application on trusted session locality before updating metadata.
| if host.is_empty() || host.eq_ignore_ascii_case("localhost") { | ||
| return true; | ||
| } | ||
| gethostname::gethostname() |
There was a problem hiding this comment.
gethostname is only a non-wasm dependency in app/Cargo.toml, so referencing it unconditionally here will break wasm builds; route through the existing cfg-gated hostname helper or cfg-gate this branch.
- Tighten OSC 7 host check to require an explicit hostname match: empty and `localhost` are now rejected because OSC 7 is terminal-controlled and a remote shell streamed through a legacy SSH session can emit either form. Shells that want OSC 7 honored must include the hostname (the de-facto convention; see wezterm/iTerm2). - Gate the TerminalModel handler so OSC 7 updates are dropped while we're inside an SSH-launching block (legacy SSH or warpified SSH), as a defense in depth against a coincident-hostname remote. - Replace the unconditional `gethostname::gethostname()` call with `session::get_local_hostname()`, which is cfg-gated for non-wasm targets — fixes the wasm build. - Update parser tests to use the dynamic local hostname for the happy path; add explicit rejection cases for empty and `localhost` hosts. - Update the OSC 7 integration test to substitute `\$(hostname)` so the emitted payload matches the host the test runs on.
The alt-screen handler (`AltScreen`) has no override for `set_current_working_directory`, so it falls through to the trait's no-op default. Routing OSC 7 through `delegate!` therefore silently swallows updates whenever a full-screen TUI program (vim, htop, worktree manager, etc.) is running — even though those are exactly the tools that emit OSC 7 the most. Also log SSH-block drops at debug level so legitimate parse failures and SSH-suppressed updates can be told apart in traces.
Folds in the three doubled-word fixes from warpdotdev#9338 that the author deferred while warpdotdev#9279 was open: "that that", "the the", "the the".
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds OSC 7 parsing and routes accepted CWD updates through the terminal model to update block metadata immediately, with unit and integration coverage for hostname validation and metadata events.
Concerns
- OSC 7 payloads containing unescaped semicolons are truncated because the dispatcher parses only the first post-command parameter after splitting on
;. This is non-blocking but can produce an incorrect CWD for valid URI paths.
Verdict
Found: 0 critical, 0 important, 1 suggestions
Approve with nits
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Reference: https://wezterm.org/shell-integration.html#osc-7-escape-sequence-to-set-the-working-directory | ||
| b"7" => { | ||
| if params.len() >= 2 { | ||
| if let Some(path) = parse_osc_7_cwd(params[1]) { |
There was a problem hiding this comment.
💡 [SUGGESTION] OSC payloads can contain semicolons in URI paths; since the dispatcher splits params on ;, parsing only params[1] truncates those paths. Rejoin params[1..] before decoding.
## Description While reading through a few crates I kept noticing duplicated words in comments — "the the", "in in", "to to", "and and", "a a", "that that". A quick `grep` turned up 17 of them across 15 files. All edits are in `//` and `///` comments — no code, test, or behavior change. Files touched: - `crates/settings/src/manager.rs` - `crates/websocket/src/sink_map_err.rs` - `crates/warp_completer/src/completer/suggest/test.rs` - `crates/warp_completer/src/completer/engine/argument/legacy.rs` - `crates/warp_completer/src/completer/engine/argument/v2.rs` - `crates/warp_core/src/ui/color/contrast.rs` - `crates/warp_logging/src/wasm.rs` - `crates/warpui_core/src/core/app.rs` - `crates/warpui_core/src/elements/selectable_area.rs` - `crates/warpui_core/src/elements/flex/mod_test.rs` - `crates/warpui/src/platform/mac/text_layout_test.rs` - `crates/warpui/src/platform/mac/rendering/wgpu/mod.rs` - `crates/warpui/src/platform/app.rs` - `crates/warpui/src/windowing/winit/fonts/font_handle.rs` - `crates/warpui/src/fonts/text_layout_test.rs` Distinct from #9318, which sweeps single-word misspellings (Surpress, occassionally, etc.) under `app/`. I also skipped the two doubled-word lines in `crates/integration/src/test.rs` since #9279 is currently editing that file — happy to fold them in if that PR lands first. ## Testing Comments-only. --------- Co-authored-by: anshul-garg27 <anshul-garg27@users.noreply.github.com> Co-authored-by: Aloke Desai <aloke.desai@gmail.com> Co-authored-by: oz-for-oss[bot] <277970191+oz-for-oss[bot]@users.noreply.github.com>
OSC parameters are split on `;`, so a URI path containing a literal semicolon (which is allowed and not always percent-encoded in the wild) arrived as multiple params and we silently truncated the payload at the first split. Rejoin params[1..] before decoding, matching what the OSC 2 (title) handler already does.
|
/oz-review |
There was a problem hiding this comment.
Overview
This PR adds OSC 7 parsing, validates local-host payloads before updating CWD metadata, routes the new handler through TerminalModel/BlockList/Block, and adds unit plus integration coverage for the parser and block metadata update path.
Concerns
- No blocking correctness or security concerns found in the annotated diff. The security pass did not identify commentable issues on changed lines.
Verdict
Found: 0 critical, 0 important, 0 suggestions
Approve
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
captainsafia
added
the
external-contributor
Warp Dev Github Integration
## Description While reading through a few crates I kept noticing duplicated words in comments — "the the", "in in", "to to", "and and", "a a", "that that". A quick `grep` turned up 17 of them across 15 files. All edits are in `//` and `///` comments — no code, test, or behavior change. Files touched: - `crates/settings/src/manager.rs` - `crates/websocket/src/sink_map_err.rs` - `crates/warp_completer/src/completer/suggest/test.rs` - `crates/warp_completer/src/completer/engine/argument/legacy.rs` - `crates/warp_completer/src/completer/engine/argument/v2.rs` - `crates/warp_core/src/ui/color/contrast.rs` - `crates/warp_logging/src/wasm.rs` - `crates/warpui_core/src/core/app.rs` - `crates/warpui_core/src/elements/selectable_area.rs` - `crates/warpui_core/src/elements/flex/mod_test.rs` - `crates/warpui/src/platform/mac/text_layout_test.rs` - `crates/warpui/src/platform/mac/rendering/wgpu/mod.rs` - `crates/warpui/src/platform/app.rs` - `crates/warpui/src/windowing/winit/fonts/font_handle.rs` - `crates/warpui/src/fonts/text_layout_test.rs` Distinct from warpdotdev#9318, which sweeps single-word misspellings (Surpress, occassionally, etc.) under `app/`. I also skipped the two doubled-word lines in `crates/integration/src/test.rs` since warpdotdev#9279 is currently editing that file — happy to fold them in if that PR lands first. ## Testing Comments-only. --------- Co-authored-by: anshul-garg27 <anshul-garg27@users.noreply.github.com> Co-authored-by: Aloke Desai <aloke.desai@gmail.com> Co-authored-by: oz-for-oss[bot] <277970191+oz-for-oss[bot]@users.noreply.github.com>
zachbai
left a comment
zachbai
left a comment
There was a problem hiding this comment.
Thanks for the PR! Mostly looks good, except for the one comment i left.
| } | ||
| self.pwd = Some(path); | ||
| self.event_proxy | ||
| .send_terminal_event(Event::BlockMetadataReceived(BlockMetadataReceivedEvent { |
There was a problem hiding this comment.
I'm fairly certain that we implicitly rely on this event being emitted once per block - as that's been the case since the event was introduced years ago.
It's probably safer to introduce a new, more tightly scoped event variant and just ensure that it's probably broadcasted to dependent logic that is currently solely relying on this event to subscribe to updated working directory.
There was a problem hiding this comment.
Good call — pushed 2eaf7eb that does exactly that.
OSC 7 now emits a new BlockWorkingDirectoryUpdated event variant instead of reusing BlockMetadataReceived. The new event mirrors the BlockMetadataReceivedEvent payload (block_metadata, block_index, is_after_in_band_command, is_done_bootstrapping) so CWD-dependent listeners can reuse the same handling, but it is opt-in:
TerminalView(tab title / repo detection / git status / prompt refresh)ActiveSession(UpdatedPwd emission)BlocklistAIContextModel(directory context)
…each subscribe to both events via a small shared helper, so OSC 7 still drives every listener that genuinely cares about the CWD.
Subscribers tied to precmd semantics — most importantly the requested-command finish detector in ai/blocklist/action_model/execute/shell_command.rs — are unchanged and continue listening only to BlockMetadataReceived, preserving the once-per-block contract you flagged.
The block test was updated to assert that set_current_working_directory emits BlockWorkingDirectoryUpdated and that it does not emit BlockMetadataReceived, so any future regression on this contract will fail loudly.
`set_current_working_directory` (the OSC 7 entry point on `Block`) previously emitted `BlockMetadataReceived` to drive tab-title / git-status refreshes. That event has been around for years and a number of subscribers implicitly rely on it firing exactly once per block at precmd — most notably the requested-command finish detector in `ai/blocklist/action_model/execute/shell_command.rs`. Reusing it for mid-command CWD changes risked firing those subscribers twice per block. Introduce `BlockWorkingDirectoryUpdated`, a tightly-scoped event variant mirroring `BlockMetadataReceivedEvent`, and emit that from the OSC 7 path instead. The three subscribers that genuinely need CWD updates — `TerminalView` (tab title, repo detection, git status, prompt), the ActiveSession PWD tracker, and the AI context model — now also handle the new variant via small shared helpers. Subscribers tied to precmd semantics (`shell_command.rs`) keep listening only to `BlockMetadataReceived` and preserve their once-per-block contract. Block test now asserts that `set_current_working_directory` emits the new variant *and* does not emit `BlockMetadataReceived`.
- Rename `is_after_in_band_command` → `is_for_in_band_command` on `BlockWorkingDirectoryUpdatedEvent` so the field name matches its actual semantics (it describes the *current* block, while the same-named field on `BlockMetadataReceivedEvent` describes the *previous* block — they diverge because precmd fires after a block runs while OSC 7 fires during it). Added a doc comment explaining the divergence. - Add a contract test asserting that `ShellCommandExecutor`'s requested-command finish detector reacts only to `BlockMetadataReceived` and never to `BlockWorkingDirectoryUpdated`. Locks in the once-per-block guarantee that motivated the dedicated event variant in the first place.
| @@ -1,3 +1,7 @@ | |||
| #[cfg(test)] | |||
There was a problem hiding this comment.
this is a mega nit but please move test modules to the bottom of the file per convention in the rest of the codebase
|
@waynehoover Will give this a spin tomorrow and just do some testing locally, but change itself looks good. |
|
@zachbai any progress on this? It would be nice to land on a solution here. Others are also expressing desire for a resolution: #9125 (comment) |
|
@waynehoover sorry for the delay, lost this in my inbox. Will look at this today |
…update # Conflicts: # app/src/terminal/model/block_tests.rs # app/src/terminal/view.rs
zachbai
left a comment
zachbai
left a comment
There was a problem hiding this comment.
@waynehoover I took a look at this and was seeing the correct behavior for updating git branch, but wasn't seeing the same for current working directoyr (at insofar as actually updating the user-facing UI)
https://www.loom.com/share/f00b9513ad9d4072874759f18c41f32e
Were you able to actually test this change locally and see a difference?
I'm guessing that the working directly update is not actually being propagated to the tab UI surfaces + trriggering a re-render.
Zach saw that after the OSC 7 wiring landed, the block's `pwd` and the git-branch chip updated correctly mid-command, but the user-visible CWD (the `WorkingDirectory` chip text, which feeds the vertical-tab subtitle via `display_working_directory`) stayed stuck on the previous directory. Root cause: the `WorkingDirectory` chip is a contextual generator that reads from `CurrentPrompt::latest_context.active_block_metadata`, and `latest_context` is only refreshed via `refresh_warp_prompt` → `current_prompt.update_context(active_block, …)`. In the normal precmd flow that refresh is triggered by `BlockCompleted`, but an OSC 7 fires mid-command — the block never completes — so the chip text was never recomputed even though the block metadata had the new CWD. Git branch updated independently because the repo-detection path in `apply_block_metadata_update` refreshes `git_repo_status` directly. Fix: after dispatching `BlockWorkingDirectoryUpdated` through `apply_block_metadata_update`, also call `refresh_warp_prompt` so the chip generators re-run against the latest block metadata. Skip the call for in-band-command blocks (same reason `apply_block_metadata_update` bails on them — to avoid re-firing chip generators that schedule more in-band commands). Extend the OSC 7 integration test to assert that `view.display_working_directory(ctx)` reflects the new path, so this user-visible regression is locked in alongside the existing block-pwd assertion.
|
@zachbai good catch — you were right that the update wasn't propagating to the UI. Just pushed a fix in 2a8186c. Root cause: the block's Fix: call Also extended the OSC 7 integration test to assert |
|
Awesome, thanks, will get to this today |
`apply_block_metadata_update` is called from both `BlockMetadataReceived`
(precmd, once per block) and `BlockWorkingDirectoryUpdated` (OSC 7, may
fire many times mid-block from chatty prompts). Two behaviors were wrong
under repeated OSC 7:
* `detect_possible_git_repo` was respawned on every OSC 7, redoing the
filesystem walk + LSP restart + `RepoChanged` plumbing even when the
CWD was unchanged.
* `block_completed_callbacks` (scheduled via `on_next_block_completed`,
e.g. `maybe_set_pending_repo_init_path`'s post-clone project init)
were drained on every OSC 7, firing before the block had completed.
Distinguish the two call sites with a `BlockMetadataUpdateSource` enum;
skip repo detection on OSC 7 unless the CWD actually changed, and never
drain block-completion callbacks on OSC 7.
4 participants
Description
Fixes #9125.
Warp's per-block working directory was only refreshed during the shell's prompt rendering cycle (precmd), so external tools that change directory mid-command — for example worktrunk's
wt switch— left the tab title and git branch chip stuck on the old value until the user submitted another command. The OSC 7 escape\e]7;file://host/path\ais the standard way for those tools to inform the terminal of a new CWD.This PR wires OSC 7 through the terminal handler:
localhost, or the local hostname (case-insensitive) and percent-decoding the path. Unescaped semicolons in the path are preserved.Handler::set_current_working_directory.Blockupdates itspwdand emits a new dedicatedBlockWorkingDirectoryUpdatedevent — notBlockMetadataReceived, because that event is contracted to fire exactly once per block at precmd and several subscribers (notably the requested-command finish detector inai/blocklist/action_model/execute/shell_command.rs) rely on that contract.TerminalModel::set_current_working_directoryroutes the call directly toBlockList(not through the alt-screendelegate!) so updates aren't swallowed when a TUI program is on the alt screen. It also drops the update entirely while inside an SSH-launching block, since a remote shell with a coincident hostname could otherwise slip through the host check.ActiveSession,BlocklistAIContextModel, andTerminalView(via the new sharedapply_block_metadata_update) — opt in by listening toBlockWorkingDirectoryUpdatedin addition toBlockMetadataReceived. Subscribers tied to precmd semantics keep listening only toBlockMetadataReceived.Testing
app/src/terminal/model/ansi/mod_tests.rscovering parsing, ST/BEL terminators, percent decoding (including malformed/truncated escapes), unescaped semicolons in paths, host validation (empty /localhost/ local hostname / case-insensitive / non-local), non-fileschemes, and missing/empty payloads.test_set_current_working_directory_updates_pwd_and_emits_cwd_eventinapp/src/terminal/model/block_tests.rsverifies thepwdupdate, the no-op on duplicate path, that exactly oneBlockWorkingDirectoryUpdatedis emitted per distinct path, and asserts that noBlockMetadataReceivedevent is emitted (preserving the once-per-block precmd contract).block_working_directory_updated_does_not_drain_finish_sendersinapp/src/ai/blocklist/action_model/execute/shell_command_tests.rslocks in the contract thatShellCommandExecutor's requested-command finish detector reacts only toBlockMetadataReceived(precmd) and ignoresBlockWorkingDirectoryUpdated(OSC 7).test_osc7_updates_current_working_directory) undercrates/integration/runsprintf '\033]7;file:///tmp/osc7-test\a'and asserts the previous block'spwdwas updated.cargo fmt --checkis clean. The new code passescargo clippy -p warpwithout introducing any new warnings.Server API dependencies
No server changes.
Agent Mode
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Update the tab's working directory and git branch indicator when a process emits an OSC 7 escape sequence to report a new CWD (e.g. worktrunk's
wt switch).