diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ebff406..b88818ea 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -58,7 +58,7 @@ jobs: - platform: windows-latest kind: rust task: test - timeout-minutes: 30 + timeout-minutes: 45 name: ${{ matrix.kind }} / ${{ matrix.task }} / ${{ matrix.platform }} runs-on: ${{ matrix.platform }} diff --git a/src-tauri/src/binary_paths.rs b/src-tauri/src/binary_paths.rs index a3e9dfc2..4bd7ecf2 100644 --- a/src-tauri/src/binary_paths.rs +++ b/src-tauri/src/binary_paths.rs @@ -9,6 +9,27 @@ static BINARY_PATHS_CACHE: OnceLock> = OnceLock::new(); pub fn get_extended_path() -> String { let current_path = env::var("PATH").unwrap_or_default(); + if cfg!(windows) { + // The extra locations and `:` joiner below are Unix-specific (Homebrew, + // /usr/bin, etc.) and would clobber the inherited PATH — which includes + // System32 and PowerShell's own directory — with a broken value. Just + // append the running exe's directory, using the Windows `;` separator. + let mut all_paths: Vec = current_path + .split(';') + .filter(|p| !p.is_empty()) + .map(String::from) + .collect(); + if let Ok(exe_path) = std::env::current_exe() { + if let Some(exe_dir) = exe_path.parent() { + let exe_dir_str = exe_dir.to_string_lossy().to_string(); + if !exe_dir_str.is_empty() && !all_paths.contains(&exe_dir_str) { + all_paths.push(exe_dir_str); + } + } + } + return all_paths.join(";"); + } + // Common binary locations to add let additional_paths = vec![ "/opt/homebrew/bin", // macOS ARM Homebrew @@ -64,8 +85,25 @@ pub fn get_exe_dir() -> Option { } } -/// Detect binary path using `which` command with extended PATH +/// Detect binary path using `which` (Unix) or `where` (Windows) with extended PATH pub fn detect_binary(name: &str) -> Option { + if cfg!(windows) { + // `which` isn't available by default on Windows, and `get_extended_path` + // joins entries with `:` (a Unix path separator), so neither applies here. + // `where` uses the process's own PATH and is present on all supported Windows versions. + let output = Command::new("where").arg(name).output().ok()?; + + if output.status.success() { + let stdout = String::from_utf8(output.stdout).ok()?; + let path = stdout.lines().next().unwrap_or("").trim().to_string(); + if !path.is_empty() { + return Some(path); + } + } + + return None; + } + let extended_path = get_extended_path(); // Try using `which` with extended PATH diff --git a/src-tauri/src/core/agent_cli.rs b/src-tauri/src/core/agent_cli.rs index d74299f1..6693dec3 100644 --- a/src-tauri/src/core/agent_cli.rs +++ b/src-tauri/src/core/agent_cli.rs @@ -230,7 +230,10 @@ mod tests { let settings_path = files.settings_path.expect("settings path"); let settings = fs::read_to_string(&settings_path).expect("read settings"); - assert!(settings.contains(&files.skill_dir)); + // skill_dir may contain backslashes (Windows), which JSON escapes as `\\`; + // compare against the JSON-encoded form rather than the raw path string. + let skill_dir_json = serde_json::to_string(&files.skill_dir).expect("encode skill_dir"); + assert!(settings.contains(skill_dir_json.trim_matches('"'))); assert!(settings.contains("/ws")); assert!(!settings.contains("sandbox")); let value: Value = serde_json::from_str(&settings).expect("parse settings"); @@ -292,7 +295,15 @@ mod tests { #[test] fn cleanup_refuses_paths_outside_temp_prefix() { - let result = cleanup_agent_cli_files(&["/etc/passwd".to_string()]); + // Must exist on every platform: canonicalize() only rejects paths that don't + // resolve; /etc/passwd doesn't exist on Windows, so it would be silently + // skipped instead of exercising the temp-prefix check. The running test + // binary always exists and is never under the treq-agent temp prefix. + let outside_path = std::env::current_exe() + .expect("current_exe") + .to_string_lossy() + .into_owned(); + let result = cleanup_agent_cli_files(&[outside_path]); assert!(result.is_err()); } @@ -377,6 +388,10 @@ mod tests { cleanup_agent_cli_files(&[files.prompt_path, files.skill_dir]).expect("cleanup"); } + // Windows' read-only directory attribute doesn't block creating files inside + // the directory (unlike Unix's write-permission bit), so this test's simulated + // permission denial is a no-op there and the skip path never triggers. + #[cfg(not(windows))] #[test] fn skips_project_skills_when_cwd_is_not_writable() { let cwd = tempfile::TempDir::new().expect("cwd"); diff --git a/src-tauri/src/core/checks.rs b/src-tauri/src/core/checks.rs index 0543408c..ac14672c 100644 --- a/src-tauri/src/core/checks.rs +++ b/src-tauri/src/core/checks.rs @@ -88,7 +88,9 @@ fn validate_filename(filename: &str) -> Result<(), String> { } fn validate_working_directory(wd: &str) -> Result<(), String> { - if Path::new(wd).is_absolute() { + // `Path::is_absolute()` only recognizes drive-rooted paths on Windows (e.g. `C:\foo`), + // so a Unix-style `/foo` slips past it there. Reject a leading separator explicitly. + if Path::new(wd).is_absolute() || wd.starts_with('/') || wd.starts_with('\\') { return Err(format!( "working-directory must be a relative path, got: '{}'", wd diff --git a/src-tauri/src/core/mod.rs b/src-tauri/src/core/mod.rs index eb4710f6..69290bf1 100644 --- a/src-tauri/src/core/mod.rs +++ b/src-tauri/src/core/mod.rs @@ -95,7 +95,8 @@ mod tests { std::env::set_var("TREQ_APP_DATA_DIR", "/tmp/app-data"); let resolved = resolve_app_db_path("/repo/path"); - assert_eq!(resolved.to_string_lossy(), "/tmp/app-data/treq.db"); + let expected = std::path::Path::new("/tmp/app-data").join("treq.db"); + assert_eq!(resolved, expected); std::env::remove_var("TREQ_APP_DATA_DIR"); } diff --git a/src-tauri/src/core/workspaces.rs b/src-tauri/src/core/workspaces.rs index eea5941f..28ce6357 100644 --- a/src-tauri/src/core/workspaces.rs +++ b/src-tauri/src/core/workspaces.rs @@ -1206,6 +1206,17 @@ mod tests { .status() .expect("jj init"); assert!(status.success()); + // get_default_branch() falls back to git's init.defaultBranch config once no + // "main"/"master" branch ref exists yet (true here: no commit has been made, + // so the colocated repo has no real git branch ref at all). That config + // depends on the runner's git version/settings (observed "master" on the + // Windows CI image); pin it to "main" so resolution is deterministic. + let status = Command::new("git") + .current_dir(temp.path()) + .args(["config", "init.defaultBranch", "main"]) + .status() + .expect("git config init.defaultBranch"); + assert!(status.success()); let repo_path = temp.path().to_str().expect("utf8 path").to_string(); crate::jj::jj_set_bookmark(&repo_path, "main", "@").expect("set main bookmark"); repo_path diff --git a/src-tauri/src/pty.rs b/src-tauri/src/pty.rs index 77016267..e4acf869 100644 --- a/src-tauri/src/pty.rs +++ b/src-tauri/src/pty.rs @@ -78,9 +78,31 @@ pub struct PtySession { auto_command: Arc>>, } +/// Windows shells (PowerShell/cmd) submit a line on carriage return; a bare `\n` +/// with no preceding `\r` is inserted into PSReadLine's buffer instead of +/// submitting it, so callers on Windows never see command output. Insert the +/// missing `\r` before any bare `\n`, leaving existing `\r\n` untouched. +fn translate_line_endings_for_windows(data: &[u8]) -> Vec { + let mut out = Vec::with_capacity(data.len()); + let mut prev = 0u8; + for &b in data { + if b == b'\n' && prev != b'\r' { + out.push(b'\r'); + } + out.push(b); + prev = b; + } + out +} + impl PtySession { pub fn write(&mut self, data: &[u8]) -> std::io::Result<()> { - self.writer.write_all(data)?; + if cfg!(windows) { + let translated = translate_line_endings_for_windows(data); + self.writer.write_all(&translated)?; + } else { + self.writer.write_all(data)?; + } self.writer.flush() } @@ -293,11 +315,29 @@ impl PtyManager { break; } Ok(n) => { - let data = process_utf8_chunk(&mut pending_bytes, &buffer[..n]); + let mut data = process_utf8_chunk(&mut pending_bytes, &buffer[..n]); if data.is_empty() { continue; } + // Windows console apps running under ConPTY (e.g. PowerShell/PSReadLine + // at startup) query the cursor position via a VT100 Device Status Report + // and block on the pty until something answers. The app's real terminal + // (xterm.js) answers this itself; this raw reader has no such emulator + // attached, so answer it here with a fixed position to unblock the child. + const CURSOR_POS_QUERY: &str = "\x1b[6n"; + if cfg!(windows) && data.contains(CURSOR_POS_QUERY) { + let mut sessions = reader_sessions.lock().unwrap(); + if let Some(session) = sessions.get_mut(&reader_session_id) { + let _ = session.write(b"\x1b[1;1R"); + } + drop(sessions); + data = data.replace(CURSOR_POS_QUERY, ""); + if data.is_empty() { + continue; + } + } + // Check if filtering is active let filter_cmd = { let guard = auto_command_reader.lock().unwrap(); @@ -536,7 +576,14 @@ mod tests { ) .unwrap(); - let deadline = Instant::now() + Duration::from_secs(2); + // PowerShell's cold-start time on CI Windows runners can exceed 2s on its own, + // before it even processes the queued "exit" — give it more headroom there. + let timeout = if cfg!(windows) { + Duration::from_secs(10) + } else { + Duration::from_secs(2) + }; + let deadline = Instant::now() + timeout; while manager.session_exists("eof") && Instant::now() < deadline { thread::sleep(Duration::from_millis(10)); } @@ -565,12 +612,22 @@ mod tests { fn echo_suppression_releases_output_after_bounded_buffer() { let manager = PtyManager::new(); let (tx, rx) = mpsc::channel(); + // `yes`/`head` aren't available under PowerShell; emit the same >32KB of + // filler in one shot so the filter's MAX_FILTER_BUFFER release path is + // exercised the same way on both platforms. + let command = if cfg!(windows) { + // Many small writes flush through ConPTY more reliably than one very long + // line, which can sit behind console line-wrap handling before it's sent. + "for($i=0;$i -lt 1000;$i++){Write-Output ('x' * 50)}".to_string() + } else { + "yes x | head -c 40000".to_string() + }; manager .create_session( "bounded-filter".into(), None, shell(), - Some("yes x | head -c 40000".into()), + Some(command), Some("echo-that-will-never-appear-0123456789".into()), Box::new(move |chunk| { let _ = tx.send(chunk); @@ -578,7 +635,14 @@ mod tests { ) .unwrap(); - assert!(rx.recv_timeout(Duration::from_secs(2)).is_ok()); + // Windows CI runs this alongside ~300 other tests contending for CPU/IO, on + // top of PowerShell's own slower cold-start; give it much more headroom. + let timeout = if cfg!(windows) { + Duration::from_secs(30) + } else { + Duration::from_secs(2) + }; + assert!(rx.recv_timeout(timeout).is_ok()); manager.close_session("bounded-filter").unwrap(); } } diff --git a/src-tauri/tests/core_workspaces_sparse_checkout_test.rs b/src-tauri/tests/core_workspaces_sparse_checkout_test.rs index 709c943d..70c01277 100644 --- a/src-tauri/tests/core_workspaces_sparse_checkout_test.rs +++ b/src-tauri/tests/core_workspaces_sparse_checkout_test.rs @@ -352,12 +352,16 @@ fn files_outside_sparse_patterns_are_not_snapshotted() { let tree_files = TestRepo::run_jj(ws_str, &["file", "list", "-r", "feat/sparse-snapshot"]) .expect("Failed to list files at branch"); assert!( - tree_files.lines().any(|l| l.trim() == "src/new.rs"), + tree_files + .lines() + .any(|l| l.trim().replace('\\', "/") == "src/new.rs"), "committed tree should contain src/new.rs, got: {}", tree_files ); assert!( - !tree_files.lines().any(|l| l.trim() == "docs/new.md"), + !tree_files + .lines() + .any(|l| l.trim().replace('\\', "/") == "docs/new.md"), "committed tree should not contain docs/new.md, got: {}", tree_files ); diff --git a/src-tauri/tests/e2e_test_helpers.rs b/src-tauri/tests/e2e_test_helpers.rs index 6dba8588..b7e348f2 100644 --- a/src-tauri/tests/e2e_test_helpers.rs +++ b/src-tauri/tests/e2e_test_helpers.rs @@ -77,6 +77,10 @@ impl TestRepo { Self::run_git(&repo_path, &["config", "user.email", "test@example.com"])?; Self::run_git(&repo_path, &["config", "user.name", "Test User"])?; + // Tests assert on exact file bytes; without this, Windows git installs + // that default to core.autocrlf=true rewrite LF to CRLF on checkout. + Self::run_git(&repo_path, &["config", "core.autocrlf", "false"])?; + let default_branch = random_default_branch_name(); Self::run_git(&repo_path, &["branch", "-M", &default_branch]) .map_err(|e| format!("Failed to create default branch: {}", e))?;