fix(cwd): close the agent-cwd bypass and validate inherited dirs

Follow-ups to the `local_cwd` gate.

The agent-reported cwd sat ahead of `local_cwd` in the git-status chain
and bypassed the gate entirely. A native-SSH pane keeps sentinel-sourced
agent state on purpose, so an agent running *on the remote host* reported
a remote path that won unconditionally and reached the local `git` — the
exact collision the gate exists to prevent, on its most likely trigger
(running `claude` in an SSH pane). Route the agent's report through the
same remote check.

Completion's fallback to `std::env::current_dir()` meant a remote pane
now offers *this* machine's filenames for insertion into a remote command
line, where before the remote path simply failed `read_dir` and produced
nothing. `complete` takes `Option<&Path>` so "no local filesystem" is an
explicit contract: command completion still runs, path and signature
sources are skipped.

`apply_remote_context` left `st.cwd` pointing into the namespace it just
left, so after `exit` from `ssh` a local shell without shell integration
kept serving the remote's last path to the local `git` probe. Clear it on
both sides of the boundary; `DaemonMsg::Cwd` has no cleared form, so the
client mirrors it off `RemoteContext`.

Finally, validate in `initial_working_directory`: whatever wins must be a
directory *here*. This bounds the whole class rather than one shell's
spelling — an unresolvable path now falls through to the next candidate
instead of failing the spawn with "The directory name is invalid".

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
thomas
2026-07-19 19:04:59 +08:00
co-authored by Claude Opus 4.8
parent 4e97253b53
commit 08ca3a3cf7
5 changed files with 164 additions and 39 deletions
+63 -1
View File
@@ -212,7 +212,17 @@ fn initial_working_directory(cwd: Option<PathBuf>) -> Option<PathBuf> {
// client didn't pass an explicit cwd (tab-inherit / session restore still // client didn't pass an explicit cwd (tab-inherit / session restore still
// win). Inherit -> `forced` is `None`, so we keep the fallback as before. // win). Inherit -> `forced` is `None`, so we keep the fallback as before.
let forced = crate::core::config::working_directory_base(); let forced = crate::core::config::working_directory_base();
cwd.or(forced).or(fallback) // Whatever wins must actually be a directory *here*. A client cwd is only
// as good as the OSC 7 that produced it, and a shell that reports a path
// this machine cannot resolve — a remote namespace, or an msys path like
// `/c/Users/x` that Windows reads as drive-relative — would otherwise turn
// a new tab or split into a hard spawn failure ("The directory name is
// invalid") instead of quietly falling back. Cheap to check, and it bounds
// the whole class rather than one shell's spelling at a time.
[cwd, forced, fallback]
.into_iter()
.flatten()
.find(|d| d.is_dir())
} }
fn apply_common_command_setup(cmd: &mut CommandBuilder, initial_cwd: &Option<PathBuf>) { fn apply_common_command_setup(cmd: &mut CommandBuilder, initial_cwd: &Option<PathBuf>) {
@@ -1736,6 +1746,16 @@ fn apply_remote_context(st: &mut PaneState, remote: Option<RemoteContext>) {
if st.remote == remote { if st.remote == remote {
return; return;
} }
// The cwd belonged to whichever side we are leaving, and it does not
// survive the crossing: a remote path is meaningless locally, and a local
// one is meaningless on the remote. Drop it so the pane reports no cwd
// until the new shell's OSC 7 lands, rather than attributing the old
// namespace's directory to the new one — otherwise a local shell without
// shell integration keeps serving the remote's last path to the local
// `git` probe for the rest of its life.
// `DaemonMsg::Cwd` carries a bare path with no "cleared" form, so the
// client mirrors this on its own when it sees the `RemoteContext` below.
st.cwd = None;
if let Some(sub) = &st.subscriber { if let Some(sub) = &st.subscriber {
let _ = sub.send(DaemonMsg::RemoteContext(remote.clone())); let _ = sub.send(DaemonMsg::RemoteContext(remote.clone()));
} }
@@ -2102,6 +2122,48 @@ fn proc_name(pid: i32) -> Option<String> {
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use super::*; use super::*;
use std::path::Path;
/// A cwd the client reports is only as trustworthy as the OSC 7 behind it.
/// Passing one this machine cannot resolve straight to `cmd.cwd()` turns a
/// new tab or split into a hard spawn failure, so anything that isn't a
/// real directory here must fall through to the next candidate instead.
#[test]
fn initial_working_directory_skips_paths_that_are_not_directories() {
let real = std::env::temp_dir();
assert!(real.is_dir(), "temp dir should exist");
// A usable client cwd still wins outright.
assert_eq!(
initial_working_directory(Some(real.clone())),
Some(real.clone())
);
// A remote-namespace path, and the msys shape Windows reads as
// drive-relative: neither resolves here, so neither may be used.
for bogus in [
"/home/someone/definitely-not-here",
"/c/Users/definitely-not-here",
] {
let got = initial_working_directory(Some(PathBuf::from(bogus)));
assert_ne!(
got.as_deref(),
Some(Path::new(bogus)),
"{bogus} is not a directory here and must not be handed to spawn"
);
// Whatever we fall back to must itself be usable.
if let Some(d) = got {
assert!(d.is_dir(), "fallback {d:?} must be a real directory");
}
}
// A file is not a directory either.
let file = real.join("tty7-iwd-probe");
std::fs::write(&file, b"x").expect("write probe file");
let got = initial_working_directory(Some(file.clone()));
assert_ne!(got.as_deref(), Some(file.as_path()));
let _ = std::fs::remove_file(&file);
}
/// End-to-end check of the *live* agent-detection chain this feature rides /// End-to-end check of the *live* agent-detection chain this feature rides
/// on macOS/Linux: spawn a real PTY child whose `argv[0]` names a coding /// on macOS/Linux: spawn a real PTY child whose `argv[0]` names a coding
+63 -20
View File
@@ -121,7 +121,11 @@ const MAX_CANDIDATES: usize = 400;
/// Compute completions for `line` at char position `cursor`, resolving relative /// Compute completions for `line` at char position `cursor`, resolving relative
/// paths against `cwd`: command names in command position, filesystem paths /// paths against `cwd`: command names in command position, filesystem paths
/// elsewhere. Returns `None` when there's nothing to offer. /// elsewhere. Returns `None` when there's nothing to offer.
pub fn complete(line: &str, cursor: usize, cwd: &Path) -> Option<Completion> { ///
/// `cwd` is `None` when the pane has no directory on *this* machine — a remote
/// pane. Command completion still runs; everything that would touch the local
/// filesystem is skipped rather than answered from the wrong machine.
pub fn complete(line: &str, cursor: usize, cwd: Option<&Path>) -> Option<Completion> {
let chars: Vec<char> = line.chars().collect(); let chars: Vec<char> = line.chars().collect();
let cursor = cursor.min(chars.len()); let cursor = cursor.min(chars.len());
@@ -141,9 +145,18 @@ pub fn complete(line: &str, cursor: usize, cwd: &Path) -> Option<Completion> {
// back to filesystem paths. A signature slot that declares suggestions // back to filesystem paths. A signature slot that declares suggestions
// or generators owns the position: it returns `Some` (possibly with no // or generators owns the position: it returns `Some` (possibly with no
// sync candidates but pending scripts) rather than ceding to paths. // sync candidates but pending scripts) rather than ceding to paths.
match complete_signature(&chars, word_start, &word, cwd) { match cwd {
Some(sig) => (sig.cands, sig.pending), // No local cwd — a remote pane. The filesystem here is not the one
None => (complete_path(&word, cwd), Vec::new()), // the command will run against, so offer nothing rather than this
// machine's names, which would be inserted into a remote command
// line where they do not exist. Command completion above still
// works; real remote-aware path completion would have to source
// the listing from the remote and is out of scope.
None => (Vec::new(), Vec::new()),
Some(cwd) => match complete_signature(&chars, word_start, &word, cwd) {
Some(sig) => (sig.cands, sig.pending),
None => (complete_path(&word, cwd), Vec::new()),
},
} }
}; };
let candidates: Vec<Candidate> = word_cands let candidates: Vec<Candidate> = word_cands
@@ -688,7 +701,7 @@ mod tests {
/// The candidate texts `complete` returns for `line` with the cursor at the /// The candidate texts `complete` returns for `line` with the cursor at the
/// end, or an empty vec when it offers nothing. /// end, or an empty vec when it offers nothing.
fn texts(line: &str) -> Vec<String> { fn texts(line: &str) -> Vec<String> {
complete(line, line.chars().count(), Path::new("/")) complete(line, line.chars().count(), Some(Path::new("/")))
.map(|c| c.candidates.into_iter().map(|c| c.text).collect()) .map(|c| c.candidates.into_iter().map(|c| c.text).collect())
.unwrap_or_default() .unwrap_or_default()
} }
@@ -699,7 +712,7 @@ mod tests {
assert!(t.iter().any(|s| s == "commit"), "git subcommands: {t:?}"); assert!(t.iter().any(|s| s == "commit"), "git subcommands: {t:?}");
assert!(t.iter().any(|s| s == "status")); assert!(t.iter().any(|s| s == "status"));
// Descriptions ride along for the menu's second column. // Descriptions ride along for the menu's second column.
let c = complete("git ", 4, Path::new("/")).unwrap(); let c = complete("git ", 4, Some(Path::new("/"))).unwrap();
let commit = c.candidates.iter().find(|c| c.text == "commit").unwrap(); let commit = c.candidates.iter().find(|c| c.text == "commit").unwrap();
assert_eq!(commit.kind, CandidateKind::Value); assert_eq!(commit.kind, CandidateKind::Value);
assert!(commit.description.is_some()); assert!(commit.description.is_some());
@@ -714,7 +727,7 @@ mod tests {
#[test] #[test]
fn signature_offers_flags_for_the_active_subcommand() { fn signature_offers_flags_for_the_active_subcommand() {
let c = complete("git commit --", 13, Path::new("/")).unwrap(); let c = complete("git commit --", 13, Some(Path::new("/"))).unwrap();
let msg = c.candidates.iter().find(|c| c.text == "--message").unwrap(); let msg = c.candidates.iter().find(|c| c.text == "--message").unwrap();
assert_eq!(msg.kind, CandidateKind::Flag); assert_eq!(msg.kind, CandidateKind::Flag);
assert_eq!( assert_eq!(
@@ -741,7 +754,8 @@ mod tests {
// position — it returns the generator scripts and no path candidates. // position — it returns the generator scripts and no path candidates.
let dir = temp_tree("gen-checkout", &[("sentinel.txt", false), ("subdir", true)]); let dir = temp_tree("gen-checkout", &[("sentinel.txt", false), ("subdir", true)]);
let line = "git checkout "; let line = "git checkout ";
let c = complete(line, line.chars().count(), &dir).expect("generator slot is a completion"); let c = complete(line, line.chars().count(), Some(dir.as_path()))
.expect("generator slot is a completion");
assert!( assert!(
!c.pending.is_empty(), !c.pending.is_empty(),
"the branch/tag generators ride along as pending scripts" "the branch/tag generators ride along as pending scripts"
@@ -767,7 +781,7 @@ mod tests {
fn generator_script_tokens_join_with_single_spaces() { fn generator_script_tokens_join_with_single_spaces() {
// The converter word-split original string scripts; joining restores a // The converter word-split original string scripts; joining restores a
// single `/bin/sh -c` command. // single `/bin/sh -c` command.
let c = complete("git checkout ", 13, Path::new("/")).unwrap(); let c = complete("git checkout ", 13, Some(Path::new("/"))).unwrap();
let branch = c let branch = c
.pending .pending
.iter() .iter()
@@ -825,7 +839,12 @@ mod tests {
fn unknown_command_falls_back_to_paths() { fn unknown_command_falls_back_to_paths() {
// A command with no signature still path-completes (no panic, no menu here). // A command with no signature still path-completes (no panic, no menu here).
let dir = temp_tree("fallback", &[("readme.md", false)]); let dir = temp_tree("fallback", &[("readme.md", false)]);
let c = complete("frobnicate read", "frobnicate read".chars().count(), &dir).unwrap(); let c = complete(
"frobnicate read",
"frobnicate read".chars().count(),
Some(dir.as_path()),
)
.unwrap();
assert_eq!(c.candidates[0].text, "readme.md"); assert_eq!(c.candidates[0].text, "readme.md");
} }
@@ -925,7 +944,7 @@ mod tests {
#[test] #[test]
fn command_position_offers_builtins_with_word_range() { fn command_position_offers_builtins_with_word_range() {
let c = complete("ech", 3, Path::new("/")).unwrap(); let c = complete("ech", 3, Some(Path::new("/"))).unwrap();
let echo = c.candidates.iter().find(|c| c.text == "echo").unwrap(); let echo = c.candidates.iter().find(|c| c.text == "echo").unwrap();
assert_eq!(echo.kind, CandidateKind::Command); assert_eq!(echo.kind, CandidateKind::Command);
assert_eq!((echo.start, echo.end), (0, 3)); // replaces the word "ech" assert_eq!((echo.start, echo.end), (0, 3)); // replaces the word "ech"
@@ -938,7 +957,7 @@ mod tests {
&[("apple.txt", false), ("apply.sh", false), ("assets", true)], &[("apple.txt", false), ("apply.sh", false), ("assets", true)],
); );
let line = "cat a"; let line = "cat a";
let c = complete(line, line.chars().count(), &dir).unwrap(); let c = complete(line, line.chars().count(), Some(dir.as_path())).unwrap();
let names: Vec<&str> = c.candidates.iter().map(|c| c.text.as_str()).collect(); let names: Vec<&str> = c.candidates.iter().map(|c| c.text.as_str()).collect();
// Closeness order: assets(6) < apply.sh(8) < apple.txt(9). // Closeness order: assets(6) < apply.sh(8) < apple.txt(9).
assert_eq!(names, vec!["assets", "apply.sh", "apple.txt"]); assert_eq!(names, vec!["assets", "apply.sh", "apple.txt"]);
@@ -952,7 +971,7 @@ mod tests {
let dir = temp_tree("nested", &[("sub", true)]); let dir = temp_tree("nested", &[("sub", true)]);
std::fs::write(dir.join("sub/file.rs"), b"").unwrap(); std::fs::write(dir.join("sub/file.rs"), b"").unwrap();
let line = "cat sub/f"; let line = "cat sub/f";
let c = complete(line, line.chars().count(), &dir).unwrap(); let c = complete(line, line.chars().count(), Some(dir.as_path())).unwrap();
assert_eq!(c.candidates[0].text, "sub/file.rs"); assert_eq!(c.candidates[0].text, "sub/file.rs");
assert_eq!(c.candidates[0].start, 4); assert_eq!(c.candidates[0].start, 4);
} }
@@ -960,9 +979,9 @@ mod tests {
#[test] #[test]
fn hidden_files_only_with_dot_prefix() { fn hidden_files_only_with_dot_prefix() {
let dir = temp_tree("hidden", &[(".secret", false), ("visible", false)]); let dir = temp_tree("hidden", &[(".secret", false), ("visible", false)]);
let c = complete("ls v", 4, &dir).unwrap(); let c = complete("ls v", 4, Some(dir.as_path())).unwrap();
assert!(c.candidates.iter().all(|c| !c.text.starts_with('.'))); assert!(c.candidates.iter().all(|c| !c.text.starts_with('.')));
let c = complete("ls .", 4, &dir).unwrap(); let c = complete("ls .", 4, Some(dir.as_path())).unwrap();
assert!(c.candidates.iter().any(|c| c.text == ".secret")); assert!(c.candidates.iter().any(|c| c.text == ".secret"));
} }
@@ -978,18 +997,42 @@ mod tests {
], ],
); );
let line = "cat x"; let line = "cat x";
let c = complete(line, line.chars().count(), &dir).unwrap(); let c = complete(line, line.chars().count(), Some(dir.as_path())).unwrap();
let names: Vec<&str> = c.candidates.iter().map(|c| c.text.as_str()).collect(); let names: Vec<&str> = c.candidates.iter().map(|c| c.text.as_str()).collect();
assert_eq!(names, vec!["xa", "xy", "xyz", "xyzzy"]); assert_eq!(names, vec!["xa", "xy", "xyz", "xyzzy"]);
} }
/// A remote pane has no local cwd. Path candidates must come back empty
/// rather than from tty7's own directory — inserting a local filename into
/// a remote command line names a file that isn't there. Command completion
/// is unaffected: it reads `$PATH`, not the cwd.
#[test]
fn a_remote_pane_completes_commands_but_never_local_paths() {
let dir = temp_tree("remote", &[("only-here.txt", false), ("subdir", true)]);
// With a local cwd the file is offered...
let c = complete("cat only", 8, Some(dir.as_path())).expect("local pane completes paths");
assert!(c.candidates.iter().any(|c| c.text.starts_with("only-here")));
// ...and with none it is not, from the same line.
assert!(complete("cat only", 8, None).is_none());
// Nor does a bare argument position dump anything.
assert!(complete("cat ", 4, None).is_none());
// A signature-owned slot must not shell out to a local generator either.
assert!(complete("git checkout ", 13, None).is_none());
// Command position still works — that source never touches the cwd.
let c = complete("ech", 3, None).expect("command completion needs no cwd");
assert!(c.candidates.iter().any(|c| c.text == "echo"));
}
#[test] #[test]
fn no_candidates_returns_none() { fn no_candidates_returns_none() {
let dir = temp_tree("empty", &[("zzz", false)]); let dir = temp_tree("empty", &[("zzz", false)]);
assert!(complete("cat q", 5, &dir).is_none()); assert!(complete("cat q", 5, Some(dir.as_path())).is_none());
// A blank line offers nothing (no dump of every command on bare Tab). // A blank line offers nothing (no dump of every command on bare Tab).
assert!(complete("", 0, &dir).is_none()); assert!(complete("", 0, Some(dir.as_path())).is_none());
assert!(complete(" ", 3, &dir).is_none()); assert!(complete(" ", 3, Some(dir.as_path())).is_none());
} }
#[test] #[test]
@@ -997,7 +1040,7 @@ mod tests {
// Caret sits right after "ap" with more text following; the candidate // Caret sits right after "ap" with more text following; the candidate
// replaces only `word_start..cursor`, leaving the tail untouched. // replaces only `word_start..cursor`, leaving the tail untouched.
let dir = temp_tree("midline", &[("apple.txt", false)]); let dir = temp_tree("midline", &[("apple.txt", false)]);
let c = complete("cat ap x.log", 6, &dir).unwrap(); let c = complete("cat ap x.log", 6, Some(dir.as_path())).unwrap();
let apple = c.candidates.iter().find(|c| c.text == "apple.txt").unwrap(); let apple = c.candidates.iter().find(|c| c.text == "apple.txt").unwrap();
assert_eq!((apple.start, apple.end), (4, 6)); assert_eq!((apple.start, apple.end), (4, 6));
// Applying it splices over just that range. // Applying it splices over just that range.
+11
View File
@@ -668,6 +668,17 @@ impl RemoteTerminal {
} }
DaemonMsg::RemoteContext(ctx) => { DaemonMsg::RemoteContext(ctx) => {
flush_batch!(); flush_batch!();
// Crossing the local/remote boundary invalidates
// the cwd: it names a directory in the namespace
// we just left. Drop it so the pane reports none
// until the new shell's OSC 7 lands — otherwise
// an `exit` from `ssh` leaves the remote's last
// path in place, and a local shell without shell
// integration never overwrites it, so the local
// `git` probe keeps running against it.
if let Ok(mut guard) = cwd.lock() {
*guard = None;
}
if let Ok(mut guard) = remote.lock() { if let Ok(mut guard) = remote.lock() {
*guard = ctx; *guard = ctx;
} }
+26 -12
View File
@@ -2600,11 +2600,22 @@ impl TerminalView {
// doesn't (Windows). The claim dies with the session (`session-end` // doesn't (Windows). The claim dies with the session (`session-end`
// clears it, and the agent leaving the foreground drops the whole // clears it, and the agent leaving the foreground drops the whole
// state), so an exited agent falls back to the pane's real directory. // state), so an exited agent falls back to the pane's real directory.
// The agent's report goes through the same remote gate as the pane's own
// cwd. A native-SSH pane keeps sentinel-sourced agent state on purpose
// (`spawn_native_ssh`), so an agent running *on the remote host* reports
// a remote path — and being first in the chain it would win over
// `local_cwd` unconditionally and hand that path straight to the local
// `git`, which is the collision `local_cwd` exists to prevent.
let cwd_now = self let cwd_now = self
.terminal .remote_context()
.agent_session() .is_none()
.and_then(|s| s.cwd) .then(|| {
.or_else(|| self.local_cwd()); self.terminal
.agent_session()
.and_then(|s| s.cwd)
.or_else(|| self.cwd())
})
.flatten();
if cwd_now.as_ref() != self.git_status_cwd.as_ref() || cmd_finished || turn_finished { if cwd_now.as_ref() != self.git_status_cwd.as_ref() || cmd_finished || turn_finished {
self.refresh_git_status(cwd_now, cx); self.refresh_git_status(cwd_now, cx);
} }
@@ -3344,17 +3355,17 @@ impl TerminalView {
} }
// Fresh completion. Path candidates come off the local filesystem, so // Fresh completion. Path candidates come off the local filesystem, so
// the cwd must be a local one; on a remote pane this falls back to // they need a local cwd — a remote pane passes `None` and gets command
// tty7's own dir, which is what already happened there before OSC 7 // completion only. Falling back to tty7's own directory there would
// landed. Command/history completion is unaffected either way. Real // offer *this* machine's filenames for insertion into a remote command
// remote-aware path completion would need the listing to come from the // line, where they don't exist.
// remote and is out of scope here. let cwd = match self.remote_context() {
let Some(cwd) = self.local_cwd().or_else(|| std::env::current_dir().ok()) else { Some(_) => None,
return; None => self.local_cwd().or_else(|| std::env::current_dir().ok()),
}; };
let line = self.cmd.text(); let line = self.cmd.text();
let cursor = self.cmd.cursor(); let cursor = self.cmd.cursor();
let Some(comp) = super::completion::complete(&line, cursor, &cwd) else { let Some(comp) = super::completion::complete(&line, cursor, cwd.as_deref()) else {
return; return;
}; };
@@ -3404,6 +3415,9 @@ impl TerminalView {
// Kick off each generator on the background executor and merge results // Kick off each generator on the background executor and merge results
// back on the main thread, tagged with this session's generation. // back on the main thread, tagged with this session's generation.
// Generators are local shell-outs and only ever come from `complete`'s
// `Some(cwd)` branch, so a remote pane has none to run.
let Some(cwd) = cwd else { return };
for pending in comp.pending { for pending in comp.pending {
let script = pending.script; let script = pending.script;
let cwd = cwd.clone(); let cwd = cwd.clone();
+1 -6
View File
@@ -2667,12 +2667,7 @@ impl Tty7App {
/// for the worktree operations, which shell out to a local `git`. "Copy /// for the worktree operations, which shell out to a local `git`. "Copy
/// Working Directory" deliberately keeps using `tab_cwd`: copying a remote /// Working Directory" deliberately keeps using `tab_cwd`: copying a remote
/// pane's remote path is exactly what the user wants there. /// pane's remote path is exactly what the user wants there.
fn tab_local_cwd( fn tab_local_cwd(&self, index: usize, window: &Window, cx: &App) -> Option<std::path::PathBuf> {
&self,
index: usize,
window: &Window,
cx: &App,
) -> Option<std::path::PathBuf> {
self.tabs self.tabs
.get(index)? .get(index)?
.pane .pane