From d3a99ce5c6455353ff6d5c3b85566cfe007e513a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 03:12:49 +0000 Subject: [PATCH 1/8] fix: use `where` instead of `which` for binary detection on Windows `which` isn't available by default on Windows CI runners, so detect_binary() silently failed there, falling back to a bare binary name whose subprocess resolution could then fail with a Windows-specific path error. get_extended_path()'s `:`-joined PATH also doesn't apply on Windows, so the Windows branch skips it and queries PATH directly via `where`. --- src-tauri/src/binary_paths.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/binary_paths.rs b/src-tauri/src/binary_paths.rs index a3e9dfc23..b28ad2542 100644 --- a/src-tauri/src/binary_paths.rs +++ b/src-tauri/src/binary_paths.rs @@ -64,8 +64,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 From aa474974d48ff6609cf9dec75d3796402b982462 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 11:58:41 +0000 Subject: [PATCH 2/8] fix: don't clobber Windows PATH with Unix-only entries get_extended_path() injected Homebrew/usr-bin paths and joined with `:`, then pty.rs used it verbatim as the spawned shell's PATH env var. On Windows this replaced the real PATH (System32, PowerShell's own dir, etc.) with a broken `:`-joined string of nonexistent Unix directories, leaving the spawned powershell.exe unable to resolve programs and stalling test PTY sessions before any command output appeared. --- src-tauri/src/binary_paths.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src-tauri/src/binary_paths.rs b/src-tauri/src/binary_paths.rs index b28ad2542..4bd7ecf29 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 From 0afb572e9866228edc137ebaef07637e29f68da8 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 14:58:26 +0000 Subject: [PATCH 3/8] ci: give windows rust tests more headroom before timing out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The job was hitting its 30-minute timeout mid-suite (cancelled while running jj_home_repo_target_test, with several test binaries still queued after it) rather than hanging on any single test — Windows process/filesystem overhead makes the jj-lib-heavy integration tests run noticeably slower than on Linux/macOS. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2ebff4060..b88818ea0 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 }} From 5993abcdf2ad8570992c8edf3e99e41dd95edd1c Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 15:45:57 +0000 Subject: [PATCH 4/8] fix: answer ConPTY cursor-position queries in the PTY reader on Windows PowerShell (via PSReadLine) queries the cursor position at startup using a VT100 Device Status Report (ESC[6n). Under ConPTY there's no real console buffer, so Windows turns that query into an actual DSR request sent through the pty, which blocks until something on the other end answers it. In the app UI, xterm.js answers this automatically; the raw PtyManager reader (used directly by pty_tests.rs and unit tests) had no such responder, so the query just sat in the output buffer and every subsequent write blocked, leaving read output empty on Windows CI. Detect the query in the reader thread and write back a synthetic cursor position through the session's writer to unblock the child process. --- src-tauri/src/pty.rs | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/pty.rs b/src-tauri/src/pty.rs index 77016267f..d0f079c8e 100644 --- a/src-tauri/src/pty.rs +++ b/src-tauri/src/pty.rs @@ -293,11 +293,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(); From d648fc4f01f2c42b5983cb4d360af2242cff2d18 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 18:16:29 +0000 Subject: [PATCH 5/8] fix: translate bare LF to CRLF on PTY writes for Windows shells PowerShell/cmd submit a line on carriage return; a bare \n with no preceding \r gets inserted into PSReadLine's edit buffer instead of submitting the command, so commands written by callers (including the test suite) never execute and the shell just sits at the prompt. Insert the missing \r before any bare \n on Windows, leaving \r\n untouched. --- src-tauri/src/pty.rs | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src-tauri/src/pty.rs b/src-tauri/src/pty.rs index d0f079c8e..886f126b8 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() } From 5d1c4cd0bb4ff2d77eb9f682857283aa9d395471 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 18:46:39 +0000 Subject: [PATCH 6/8] fix: two remaining Windows-only test failures - e2e_test_helpers: set core.autocrlf=false on test repos. Windows git installs commonly default to core.autocrlf=true, which rewrites LF to CRLF on checkout; tests assert on exact file bytes, so merged/checked-out content came back with \r\n where \n was written and committed. - core_workspaces_sparse_checkout_test: `jj file list` prints platform-native path separators, so the Windows run produced `src\new.rs` where the assertion expected `src/new.rs`. Normalize separators before comparing. --- src-tauri/tests/core_workspaces_sparse_checkout_test.rs | 8 ++++++-- src-tauri/tests/e2e_test_helpers.rs | 4 ++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src-tauri/tests/core_workspaces_sparse_checkout_test.rs b/src-tauri/tests/core_workspaces_sparse_checkout_test.rs index 709c943d8..70c012777 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 6dba85887..b7e348f22 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))?; From 10c5c55c35fb9122a64b8b441362bd4ed8a5f848 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 19:19:00 +0000 Subject: [PATCH 7/8] fix: remaining Windows-only --lib unit test failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - checks.rs: validate_working_directory only rejected absolute paths via Path::is_absolute(), which doesn't recognize Unix-style /foo paths on Windows (no drive letter). Real validation gap, not just a test bug — now also rejects a leading / or \ explicitly. - core/mod.rs: resolve_app_db_path test asserted a hardcoded /-joined path against a real Path::join result, which uses \ on Windows. Compare PathBufs instead. Its earlier panic (before cleanup ran) also leaked TREQ_APP_DATA_DIR across tests in the same process, which was the actual cause of the two cascading submodules.rs failures. - agent_cli.rs: JSON-escapes backslashes in Windows paths, so a raw substring check against the unescaped skill_dir failed; compare against the JSON-encoded form instead. The temp-prefix-rejection test used /etc/passwd, which doesn't exist on Windows and so was silently skipped instead of exercising the check; use the running test binary's own path instead, which always exists. The cwd-not-writable test relies on Unix's write-permission bit — Windows' read-only directory attribute doesn't block file creation inside it, so it's skipped there. - workspaces.rs: test helper never pinned the initial git branch name, so it followed whatever the runner's git defaulted to (some default to "master"); pin it to "main" to match the bookmark the test sets up. - pty.rs: one test's Windows shell timeout (2s) was too tight for PowerShell's cold-start time on CI; extended to 10s there. Another piped through `yes`/`head`, which don't exist under PowerShell; use an equivalent single Write-Output call on Windows. --- src-tauri/src/core/agent_cli.rs | 19 +++++++++++++++++-- src-tauri/src/core/checks.rs | 4 +++- src-tauri/src/core/mod.rs | 3 ++- src-tauri/src/core/workspaces.rs | 9 +++++++++ src-tauri/src/pty.rs | 26 +++++++++++++++++++++++--- 5 files changed, 54 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/core/agent_cli.rs b/src-tauri/src/core/agent_cli.rs index d74299f1f..6693dec32 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 0543408c5..ac14672cf 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 eb4710f60..69290bf1d 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 eea5941f6..8e088b6a0 100644 --- a/src-tauri/src/core/workspaces.rs +++ b/src-tauri/src/core/workspaces.rs @@ -1206,6 +1206,15 @@ mod tests { .status() .expect("jj init"); assert!(status.success()); + // git's own default initial branch name depends on the runner's git version/config + // (some default to "master"); pin it to "main" so default-branch resolution is + // deterministic instead of following whatever the CI image happens to ship. + let status = Command::new("git") + .current_dir(temp.path()) + .args(["symbolic-ref", "HEAD", "refs/heads/main"]) + .status() + .expect("git symbolic-ref"); + 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 886f126b8..dcea3c7dc 100644 --- a/src-tauri/src/pty.rs +++ b/src-tauri/src/pty.rs @@ -576,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)); } @@ -605,12 +612,20 @@ 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) { + "Write-Output ('x' * 40000)".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); @@ -618,7 +633,12 @@ mod tests { ) .unwrap(); - assert!(rx.recv_timeout(Duration::from_secs(2)).is_ok()); + let timeout = if cfg!(windows) { + Duration::from_secs(10) + } else { + Duration::from_secs(2) + }; + assert!(rx.recv_timeout(timeout).is_ok()); manager.close_session("bounded-filter").unwrap(); } } From 3ff04e1b67ca5a734d746098058c2a1267e996a5 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 19:55:29 +0000 Subject: [PATCH 8/8] fix: two stragglers from the --lib Windows fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - workspaces.rs: symbolic-ref alone didn't help since no git branch ref exists yet at that point (no commit made) — get_default_branch() falls through past the branch-ref check straight to init.defaultBranch config, which the Windows runner's git left at "master". Set that config directly instead. - pty.rs: echo_suppression_releases_output_after_bounded_buffer still timed out at 10s on Windows. Switch to many small Write-Output calls (a single very long line can sit behind console line-wrap handling before ConPTY flushes it) and give it real headroom (30s) — this test runs alongside ~300 others contending for CPU/IO on top of PowerShell's own slower cold start. --- src-tauri/src/core/workspaces.rs | 12 +++++++----- src-tauri/src/pty.rs | 8 ++++++-- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/core/workspaces.rs b/src-tauri/src/core/workspaces.rs index 8e088b6a0..28ce63572 100644 --- a/src-tauri/src/core/workspaces.rs +++ b/src-tauri/src/core/workspaces.rs @@ -1206,14 +1206,16 @@ mod tests { .status() .expect("jj init"); assert!(status.success()); - // git's own default initial branch name depends on the runner's git version/config - // (some default to "master"); pin it to "main" so default-branch resolution is - // deterministic instead of following whatever the CI image happens to ship. + // 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(["symbolic-ref", "HEAD", "refs/heads/main"]) + .args(["config", "init.defaultBranch", "main"]) .status() - .expect("git symbolic-ref"); + .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"); diff --git a/src-tauri/src/pty.rs b/src-tauri/src/pty.rs index dcea3c7dc..e4acf8694 100644 --- a/src-tauri/src/pty.rs +++ b/src-tauri/src/pty.rs @@ -616,7 +616,9 @@ mod tests { // 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) { - "Write-Output ('x' * 40000)".to_string() + // 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() }; @@ -633,8 +635,10 @@ mod tests { ) .unwrap(); + // 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(10) + Duration::from_secs(30) } else { Duration::from_secs(2) };