feat(windows) implement windows support #320

sassman · 2026-01-12T22:53:18Z

finally solve [FEAT] windows support #2
refine how we shell out to ffmpeg and imagemagick (for all platforms)
use the win-screenshot crate to do the actual window capturing
use the windows crate for all the PTY related stuff
extend the build pipeline to run on windows too
extend the release binary artefacts pipeline to produce windows executables as well

Window capture via win-screenshot crate with GetForegroundWindow. ConPTY pseudo-terminal with CreatePseudoConsole and named pipes. Platform-conditional integration in main.rs and session.rs.

Set STARTF_USESTDHANDLES with INVALID_HANDLE_VALUE to force child to use only the pseudo-console when parent stdout is redirected.

cmd.exe sends ESC[6n at startup and blocks waiting for response. Detect and respond to CPR in forward_output loop.

Windows convert.exe conflicts with filesystem utility. Use magick command and verify output contains "ImageMagick".

Allow dead_code for crop on Windows. Add ffmpeg install instruction.

Centralize ImageMagick command selection in decors/mod.rs. Fixes wallpaper not appearing in generated GIFs on Windows.

Convert backslashes for ffmpeg glob matching. Check exit status and report stderr on failure.

Windows ffmpeg builds lack glob support. Use concat demuxer with file list instead.

- Add Windows to list of supported platforms - Add Windows installation section with winget commands for prerequisites - Document ImageMagick and ffmpeg installation via winget - Remove outdated note about Windows support needing to be done

- Add windows-latest to build matrix for check, lint, and test jobs - Install ImageMagick and ffmpeg prerequisites on Windows CI runners - Add x86_64-pc-windows-msvc target to release binary builds - Create .zip archives for Windows releases (instead of .tar.gz) - Windows binaries will now be built and tested in CI

…corder

Copilot

Pull request overview

This pull request implements comprehensive Windows support for t-rec, enabling terminal recording on Windows platforms using ConPTY for shell handling and the win-screenshot crate for window capture. The implementation refactors the WindowId type into a platform-independent newtype wrapper and updates the ffmpeg/ImageMagick integration to work cross-platform.

Changes:

Implements Windows platform API using ConPTY for PTY handling and win-screenshot for window capture
Introduces type-safe WindowId wrapper to replace raw u64 usage across all platforms
Refactors ffmpeg to use concat demuxer instead of glob patterns for cross-platform compatibility
Updates ImageMagick command to use 'magick' on Windows to avoid conflicts with system utilities
Extends CI/CD pipelines to build, test, and release Windows binaries

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
crates/t-rec/src/windows/window_id.rs	Implements window enumeration and foreground window detection using Win32 APIs
crates/t-rec/src/windows/screenshot.rs	Screenshot capture using win-screenshot crate with RGBA conversion
crates/t-rec/src/windows/pty.rs	ConPTY-based pseudo-terminal implementation for spawning shells on Windows
crates/t-rec/src/windows/mod.rs	Windows platform API implementation with transparency detection and margin cropping
crates/t-rec/src/win/mod.rs	Removed old stub implementation
crates/t-rec/src/common/window_id.rs	New type-safe WindowId wrapper with platform-specific conversion methods
crates/t-rec/src/common/mod.rs	Exports WindowId type for public API
crates/t-rec/src/common/image.rs	Updates dead_code attribute to include Windows
crates/t-rec/src/macos/window_id.rs	Updates to use WindowId type
crates/t-rec/src/macos/mod.rs	Updates PlatformApi implementation for WindowId type
crates/t-rec/src/linux/x11_api.rs	Updates X11 API to use WindowId type with proper conversions
crates/t-rec/src/generators/mp4.rs	Switches from glob patterns to concat demuxer for cross-platform ffmpeg support
crates/t-rec/src/generators/gif.rs	Updates ImageMagick detection to use 'magick' command on Windows
crates/t-rec/src/decors/shadow.rs	Uses IMAGEMAGICK_CMD constant for cross-platform compatibility
crates/t-rec/src/decors/big_sur_corner.rs	Uses IMAGEMAGICK_CMD constant for cross-platform compatibility
crates/t-rec/src/decors/mod.rs	Defines platform-specific ImageMagick command constant
crates/t-rec/src/main.rs	Updates to use WindowId type and adds TODO comments for future refactoring
crates/t-rec/src/recorder/session.rs	Adds conditional import for Windows PTY module
crates/t-rec/src/lib.rs	Exports WindowId type and adds Windows module
crates/t-rec/src/capture.rs	Updates tests to use WindowId::default()
crates/t-rec/Cargo.toml	Adds Windows dependencies: win-screenshot and windows crates
Cargo.lock	Updates with new Windows dependency versions
.github/workflows/build.yml	Adds Windows to CI matrix with ImageMagick and ffmpeg installation
.github/workflows/release-binary-assets.yml	Adds Windows binary releases with zip packaging
README.md	Adds Windows installation instructions and updates platform support

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-14T07:23:24Z

crates/t-rec/src/windows/screenshot.rs

+    use super::*;
+
+    #[test]
+    #[should_panic(expected = "Cannot grab screenshot of window id 999999")]


The test expects 7 digits in the window ID but only passes 6 digits (999999 vs 9999999). The panic message says "Cannot grab screenshot of window id 999999" but the code calls the function with 9999999 (7 nines).

Suggested change

#[should_panic(expected = "Cannot grab screenshot of window id 999999")]

#[should_panic(expected = "Cannot grab screenshot of window id 9999999")]

Copilot · 2026-01-14T07:23:25Z

crates/t-rec/src/windows/pty.rs

+        CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0)
+            .context("Failed to create output pipe")?;


If CreatePipe for output fails, the input pipe handles (pipe_in_read and pipe_in_write) are not cleaned up, causing a resource leak. Consider using RAII wrappers or explicit cleanup on error paths.

Suggested change

CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0)

.context("Failed to create output pipe")?;

// Create output pipe; on failure, clean up already-created input pipe handles

match CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0) {

Ok(()) => {}

Err(e) => {

// Best-effort cleanup of input pipe handles before propagating the error

let _ = CloseHandle(pipe_in_read);

let _ = CloseHandle(pipe_in_write);

return Err(e).context("Failed to create output pipe");

}

}

Copilot · 2026-01-14T07:23:25Z

crates/t-rec/src/generators/mp4.rs

+    drop(concat_file); // Ensure file is closed before ffmpeg reads it
+
+    // Convert concat file path to forward slashes for ffmpeg
+    let concat_file_str = concat_file_path.to_string_lossy().replace('\\', "/");


The concat file path is converted using to_string_lossy() which can lose information if the path contains invalid UTF-8. While unlikely on most systems, consider using to_str() with proper error handling or escaping special characters that might cause ffmpeg to fail.

Copilot · 2026-01-14T07:23:27Z

crates/t-rec/src/common/window_id.rs

+    pub const fn as_u32(self) -> u32 {
+        self.0 as u32


Converting WindowId (u64) to u32 with as u32 can truncate values on platforms where window IDs exceed 32-bit range. While X11 uses 32-bit IDs, the function is public and could be misused. Consider adding a debug assertion or returning a Result to handle potential data loss.

Copilot · 2026-01-14T07:23:27Z

crates/t-rec/src/common/window_id.rs

+impl From<WindowId> for u32 {
+    #[inline]
+    fn from(id: WindowId) -> Self {
+        id.0 as u32


The From for u32 implementation silently truncates the u64 value to u32. This could cause data loss if a WindowId from Windows or macOS (which may use 64-bit handles) is converted to u32. Consider using TryFrom instead to make the potential failure explicit.

Suggested change

impl From<WindowId> for u32 {

#[inline]

fn from(id: WindowId) -> Self {

id.0 as u32

impl std::convert::TryFrom<WindowId> for u32 {

type Error = std::num::TryFromIntError;

#[inline]

fn try_from(id: WindowId) -> Result<Self, Self::Error> {

u32::try_from(id.0)

Copilot · 2026-01-14T07:23:28Z

crates/t-rec/src/windows/window_id.rs

+        ));
+    }
+
+    Ok(WindowId::from(hwnd.0 as u64))


Similar to line 15, the hwnd.0 value is cast to u64 with as u64. HWND.0 is an isize on Windows. Consider documenting the cast or ensuring it's safe across all Windows architectures.

Copilot · 2026-01-14T07:23:28Z

crates/t-rec/src/windows/pty.rs

+unsafe impl Sync for PtyReader {}
+
+impl PtyReader {
+    /// Read with a timeout in milliseconds.


The variable _timeout_ms parameter is unused (indicated by the underscore prefix), but there's no explanation of why timeout functionality isn't implemented. Consider adding a comment explaining why timeout is not needed or a TODO if it should be implemented in the future.

Suggested change

/// Read with a timeout in milliseconds.

/// Read with a timeout in milliseconds.

///

/// Note: `_timeout_ms` is currently unused. This function approximates

/// non-blocking behavior by using `PeekNamedPipe` and returning `Ok(0)`

/// when no data is available; callers are expected to implement any

/// higher-level timeout / retry logic.

///

/// TODO: If stricter timeout semantics are required on Windows, implement

/// them here (for example using `WaitForSingleObject` or overlapped I/O)

/// and make use of `_timeout_ms`.

Copilot · 2026-01-14T07:23:29Z

crates/t-rec/src/windows/pty.rs

+        let mut process_info: PROCESS_INFORMATION = zeroed();
+
+        // Create the command line - spawn the program directly
+        let mut cmd_wide: Vec<u16> = program.encode_utf16().chain(std::iter::once(0)).collect();


The program string is passed directly to CreateProcessW without validation or sanitization. Consider validating that the program path doesn't contain potentially dangerous sequences or ensuring it's properly quoted if it could contain spaces.

Copilot · 2026-01-14T07:23:29Z

crates/t-rec/src/windows/pty.rs

+        SetHandleInformation(pipe_in_write, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))
+            .context("Failed to clear inherit on pipe_in_write")?;
+        SetHandleInformation(pipe_out_read, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))
+            .context("Failed to clear inherit on pipe_out_read")?;
+
+        // Get console size (default to 120x30 if we can't determine)
+        let size = COORD { X: 120, Y: 30 };
+
+        // Create the pseudo-console
+        let hpc = CreatePseudoConsole(
+            size,
+            pipe_in_read,   // PTY reads input from this pipe
+            pipe_out_write, // PTY writes output to this pipe
+            PSEUDOCONSOLE_INHERIT_CURSOR,
+        )
+        .context("Failed to create pseudo-console")?;


If SetHandleInformation or CreatePseudoConsole fails after CreatePipe succeeds, the pipe handles created earlier are not cleaned up, causing a resource leak. Consider implementing proper cleanup on error paths or using RAII wrappers.

Suggested change

SetHandleInformation(pipe_in_write, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))

.context("Failed to clear inherit on pipe_in_write")?;

SetHandleInformation(pipe_out_read, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))

.context("Failed to clear inherit on pipe_out_read")?;

// Get console size (default to 120x30 if we can't determine)

let size = COORD { X: 120, Y: 30 };

// Create the pseudo-console

let hpc = CreatePseudoConsole(

size,

pipe_in_read, // PTY reads input from this pipe

pipe_out_write, // PTY writes output to this pipe

PSEUDOCONSOLE_INHERIT_CURSOR,

)

.context("Failed to create pseudo-console")?;

if let Err(e) = SetHandleInformation(pipe_in_write, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))

.context("Failed to clear inherit on pipe_in_write")

{

let _ = CloseHandle(pipe_in_read);

let _ = CloseHandle(pipe_in_write);

let _ = CloseHandle(pipe_out_read);

let _ = CloseHandle(pipe_out_write);

return Err(e);

}

if let Err(e) = SetHandleInformation(pipe_out_read, HANDLE_FLAG_INHERIT.0, HANDLE_FLAGS(0))

.context("Failed to clear inherit on pipe_out_read")

{

let _ = CloseHandle(pipe_in_read);

let _ = CloseHandle(pipe_in_write);

let _ = CloseHandle(pipe_out_read);

let _ = CloseHandle(pipe_out_write);

return Err(e);

}

// Get console size (default to 120x30 if we can't determine)

let size = COORD { X: 120, Y: 30 };

// Create the pseudo-console

let hpc = match CreatePseudoConsole(

size,

pipe_in_read, // PTY reads input from this pipe

pipe_out_write, // PTY writes output to this pipe

PSEUDOCONSOLE_INHERIT_CURSOR,

)

.context("Failed to create pseudo-console")

{

Ok(hpc) => hpc,

Err(e) => {

let _ = CloseHandle(pipe_in_read);

let _ = CloseHandle(pipe_in_write);

let _ = CloseHandle(pipe_out_read);

let _ = CloseHandle(pipe_out_write);

return Err(e);

}

};

Copilot · 2026-01-14T07:23:30Z

crates/t-rec/src/windows/screenshot.rs

+/// The win-screenshot crate returns RGBA pixel data, which we convert
+/// to our ImageOnHeap format for further processing.
+pub fn capture_window_screenshot(win_id: u64) -> Result<ImageOnHeap> {
+    let hwnd = win_id as isize;


Converting u64 window ID to isize with as isize can cause issues. On 32-bit Windows systems, isize is 32 bits, which would truncate 64-bit window IDs. While 32-bit Windows is less common now, consider validating the conversion or documenting that only 64-bit Windows is supported.

Suggested change

let hwnd = win_id as isize;

let hwnd = isize::try_from(win_id).with_context(|| {

format!(

"Window id {} does not fit in the native pointer size on this platform.",

win_id

)

})?;

sassman added 13 commits January 12, 2026 23:29

feat(windows): add capture backend and ConPTY support

9adf2c0

Window capture via win-screenshot crate with GetForegroundWindow. ConPTY pseudo-terminal with CreatePseudoConsole and named pipes. Platform-conditional integration in main.rs and session.rs.

fix(windows): handle ConPTY output with redirected stdout

d16126b

Set STARTF_USESTDHANDLES with INVALID_HANDLE_VALUE to force child to use only the pseudo-console when parent stdout is redirected.

fix(windows): respond to cursor position query in pty

09a35c0

cmd.exe sends ESC[6n at startup and blocks waiting for response. Detect and respond to CPR in forward_output loop.

fix(windows): use magick command for ImageMagick

a90d46c

Windows convert.exe conflicts with filesystem utility. Use magick command and verify output contains "ImageMagick".

fix(windows): update cfg attributes

9184015

Allow dead_code for crop on Windows. Add ffmpeg install instruction.

fix(windows): use magick command in decors module

1ba6306

Centralize ImageMagick command selection in decors/mod.rs. Fixes wallpaper not appearing in generated GIFs on Windows.

fix(windows): use forward slashes in ffmpeg glob pattern

53732e1

Convert backslashes for ffmpeg glob matching. Check exit status and report stderr on failure.

fix(windows): use ffmpeg concat demuxer instead of glob

4d89590

Windows ffmpeg builds lack glob support. Use concat demuxer with file list instead.

chore: move windows pty into windows module

b216e54

refactor: use WindowId newtype instead of type alias

c9ad670

fix(windows): use WindowId newtype in windows backend

a9cdcf5

sassman marked this pull request as draft January 12, 2026 23:23

Merge branch 'feat/crossterm-input-interception' into feat/windows-re…

7a2784a

…corder

sassman marked this pull request as ready for review January 13, 2026 23:14

sassman requested a review from Copilot January 14, 2026 07:13

Copilot started reviewing on behalf of sassman January 14, 2026 07:13 View session

Copilot AI reviewed Jan 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

feat(windows) implement windows support #320

feat(windows) implement windows support #320

Uh oh!

sassman commented Jan 12, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Copilot AI Jan 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	#[should_panic(expected = "Cannot grab screenshot of window id 999999")]
	#[should_panic(expected = "Cannot grab screenshot of window id 9999999")]

		CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0)
		.context("Failed to create output pipe")?;

-        CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0)
-            .context("Failed to create output pipe")?;
+        // Create output pipe; on failure, clean up already-created input pipe handles
+        match CreatePipe(&mut pipe_out_read, &mut pipe_out_write, Some(&sa), 0) {
+            Ok(()) => {}
+            Err(e) => {
+                // Best-effort cleanup of input pipe handles before propagating the error
+                let _ = CloseHandle(pipe_in_read);
+                let _ = CloseHandle(pipe_in_write);
+                return Err(e).context("Failed to create output pipe");
+            }
+        }

-    /// Read with a timeout in milliseconds.
+    /// Read with a timeout in milliseconds.
+    ///
+    /// Note: `_timeout_ms` is currently unused. This function approximates
+    /// non-blocking behavior by using `PeekNamedPipe` and returning `Ok(0)`
+    /// when no data is available; callers are expected to implement any
+    /// higher-level timeout / retry logic.
+    ///
+    /// TODO: If stricter timeout semantics are required on Windows, implement
+    /// them here (for example using `WaitForSingleObject` or overlapped I/O)
+    /// and make use of `_timeout_ms`.

-    let hwnd = win_id as isize;
+    let hwnd = isize::try_from(win_id).with_context(|| {
+        format!(
+            "Window id {} does not fit in the native pointer size on this platform.",
+            win_id
+        )
+    })?;

Uh oh!

feat(windows) implement windows support #320

Are you sure you want to change the base?

feat(windows) implement windows support #320

Uh oh!

Conversation

sassman commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sassman commented Jan 12, 2026 •

edited

Loading