feat(agent): add window recording support via now proto dvc #1583

pacmancoder · 2025-11-25T15:51:47Z

Adds window recording support to the Devolutions Agent. The implementation uses Windows event hooks to receive foreground window change notifications and optional polling to detect title changes within the same window.

Issue: ARC-353

github-actions · 2025-11-25T15:52:06Z

Let maintainers know that an action is required on their side

Add the label release-required Please cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module) when you request a maintainer to cut a new release (Devolutions Gateway, Devolutions Agent, Jetsocat, PowerShell module)
Add the label release-blocker Follow-up is required before cutting a new release if a follow-up is required before cutting a new release
Add the label publish-required Please publish libraries (`Devolutions.Gateway.Utils`, OpenAPI clients, etc) when you request a maintainer to publish libraries (Devolutions.Gateway.Utils, OpenAPI clients, etc.)
Add the label publish-blocker Follow-up is required before publishing libraries if a follow-up is required before publishing libraries

devolutions-session/Cargo.toml

Copilot

Pull request overview

This PR adds window recording support to the devolutions-session agent by implementing a window monitoring system that tracks active window changes on Windows systems. The implementation uses Windows Event Hooks to receive foreground window change notifications and optional polling to detect title changes within the same window.

Key Changes

Added a new window_monitor module that uses Windows Event Hooks (SetWinEventHook) to monitor foreground window changes
Integrated window recording capabilities into the DVC task processing system with start/stop message handlers
Updated the dependency on now-proto-pdu to use a Git branch for new window recording protocol messages

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
devolutions-session/src/dvc/window_monitor.rs	New module implementing window monitoring using Windows Event Hooks and optional polling for title changes, with async event handling through channels
devolutions-session/src/dvc/task.rs	Added window recording start/stop handlers, updated capabilities to include WINDOW_RECORDING flag, and integrated window recording events into the message processing loop
devolutions-session/src/dvc/process.rs	Extended ServerChannelEvent enum with WindowRecordingEvent variant for propagating window change events
devolutions-session/src/dvc/mod.rs	Added window_monitor module declaration
devolutions-session/Cargo.toml	Updated now-proto-pdu dependency to use Git branch and added Win32_UI_Accessibility feature for Windows API
Cargo.lock	Updated now-proto-pdu source to Git repository

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

devolutions-session/src/dvc/task.rs

devolutions-session/Cargo.toml

devolutions-session/src/dvc/window_monitor.rs

pacmancoder · 2025-12-02T14:05:09Z

@copilot open a new pull request to apply changes based on the comments in this thread

Copilot · 2025-12-02T14:05:20Z

@pacmancoder I've opened a new pull request, #1600, to work on those changes. Once the pull request is ready, I'll request review from you.

Co-authored-by: pacmancoder <[email protected]>

…#1600) Addresses review feedback from PR #1583 on the window recording feature implementation. ### Dependency Management - Updated `now-proto-pdu` to use v0.4.2 from crates.io for stable, reproducible builds ### Task Lifecycle - Track window monitor task with `JoinHandle` and await completion on shutdown to prevent race conditions - Made `stop_window_recording` async to properly await task termination ### Code Quality - Removed redundant type annotation on `run_dvc_io` result - Extracted duplicate timestamp logic into `get_current_timestamp()` helper - Fixed module docs to clarify polling is used for title change detection ### Bug Fix - When title tracking is disabled, preserve previous title in snapshot to avoid missing actual window changes: ```rust // Before: updating full snapshot could mask process/exe_path changes last_snapshot = Some(snapshot); // After: preserve previous title when tracking disabled let prev_title = last_snapshot.as_ref().map_or_else(String::new, |s| s.title.clone()); last_snapshot = Some(WindowSnapshot { process_id: snapshot.process_id, exe_path: snapshot.exe_path.clone(), title: prev_title, }); ```  --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: pacmancoder <[email protected]>

pacmancoder · 2025-12-05T16:22:07Z

Testing

Install devolutions-agent to target machine
Build and copy devolutions-session.exe to target machine (%PROGRAMFILES%/Devolutions/Agent/DevolutionsSession.exe)
Use IronRDP to connect to the target machine with DVC proxying via named pipe:
- cargo run -p ironrdp-client -- -u <LOGIN> -p <PASSWORD> --dvc-proxy Devolutions::Now::Agent=now-proto-GLOBAL <IP>
Within a few seconds from launching IronRDP launch now proto dvc test CLI:
- cd <now-proto repo>\dotnet\Devolutions.NowClient.Cli
- dotnet run --pipe=now-proto-GLOBAL
When now proto cli opens, execute winrec-start command and observe the received messages

[18:10:02] ACTIVE WINDOW: Program Manager
             Process ID: 7564
             Executable: C:\Windows\explorer.exe
[18:10:02] ACTIVE WINDOW:
             Process ID: 8060
             Executable: C:\Program Files\Mozilla Firefox\firefox.exe
[18:10:03] ACTIVE WINDOW: Mozilla Firefox
             Process ID: 8060
             Executable: C:\Program Files\Mozilla Firefox\firefox.exe
[18:10:04] TITLE CHANGED: What’s new with Firefox 144 — Mozilla Firefox
[18:10:19] TITLE CHANGED: New Tab — Mozilla Firefox

CBenoit · 2025-12-11T16:44:19Z

devolutions-session/src/dvc/task.rs

    capabilities: NowChannelCapsetMsg,
    sessions: HashMap<u32, WinApiProcess>,
+    /// Shutdown signal sender for window monitoring task.
+    window_monitor_shutdown_tx: Option<tokio::sync::oneshot::Sender<()>>,


suggestion: Add inline comment explaining why you use a oneshot channel over other obvious synchronisation primitives such as Notify. From what I’ve seen in the code, you are after the "exactly once" semantic that Notify does not provide.

CBenoit · 2025-12-11T16:45:51Z

devolutions-session/src/dvc/window_monitor.rs

+use std::path::PathBuf;
+use std::time::SystemTime;
+
+use anyhow::{Context, Result, bail};


suggestion: Do not use anyhow::Result, instead use anyhow::Result everywhere.

rationale: Makes it clear which Result it is at the callsite. Don’t shadow the default std::result::Result part of the prelude.

This is part of the IronRDP code style.

CBenoit · 2025-12-11T16:50:35Z

devolutions-session/src/dvc/window_monitor.rs

+/// storage to communicate with the async runtime. This is safe because WINEVENT_OUTOFCONTEXT
+/// ensures the callback runs on the same thread that installed the hook.
+struct HookContext {
+    sender: mpsc::UnboundedSender<WindowSnapshot>,


nitpick: Unbounded channels are typically a yellow/red-ish flag. If you really need one, document why it’s really better than using a bounded channel with proper backpressure.

CBenoit · 2025-12-11T16:52:33Z

devolutions-session/src/dvc/window_monitor.rs

+    /// This is only accessed from the hook thread, making it immutable after initialization.
+    static HOOK_CONTEXT: RefCell<Option<HookContext>> = const { RefCell::new(None) };


suggestion: You might be interested into OnceCell for the "initialize once, never change" semantics in a thread local: https://doc.rust-lang.org/std/cell/struct.OnceCell.html

EDIT: Reading further, I does not seem to be as immutable as the comment make it sound. I understand this is repeatedly updated each time an ActiveWindowHook is created, and cleaned when the hook is unregistered. The value stored inside the cell is indeed never changed during that span.

CBenoit · 2025-12-11T17:04:26Z

devolutions-session/src/dvc/window_monitor.rs

+                break;
+            }
+
+            // SAFETY: DispatchMessageW is safe to call with a valid MSG structure obtained from GetMessageW.


nitpick: More straightforward statements reads better: "msg is a valid MSG structure obtained from GetMessageW."

CBenoit

Excellent work! Feel free to adjust the code and merge yourself!

pacmancoder changed the title ~~feat(agent): add window recording support to now proto dvc~~ feat(agent): add window recording support via now proto dvc Nov 25, 2025

pacmancoder commented Dec 2, 2025

View reviewed changes

devolutions-session/Cargo.toml Outdated Show resolved Hide resolved

pacmancoder requested a review from Copilot December 2, 2025 13:54

Copilot started reviewing on behalf of pacmancoder December 2, 2025 13:55 View session

Copilot finished reviewing on behalf of pacmancoder December 2, 2025 13:57

Copilot AI reviewed Dec 2, 2025

View reviewed changes

Copilot AI mentioned this pull request Dec 2, 2025

refactor(agent): address review comments for window recording support #1600

Merged

Copilot AI added a commit that referenced this pull request Dec 2, 2025

Address review comments from PR #1583

6370d6a

Co-authored-by: pacmancoder <[email protected]>

pacmancoder force-pushed the feat/window-title-recording branch from 8a56ad0 to 8ef6b83 Compare December 2, 2025 16:09

feat(agent): add window recording support to now proto dvc

92b4729

pacmancoder requested a review from CBenoit December 5, 2025 16:22

pacmancoder marked this pull request as ready for review December 5, 2025 16:22

pacmancoder force-pushed the feat/window-title-recording branch from 305f497 to 92b4729 Compare December 5, 2025 16:22

vnikonov-devolutions self-assigned this Dec 5, 2025

CBenoit reviewed Dec 11, 2025

View reviewed changes

CBenoit approved these changes Dec 11, 2025

View reviewed changes

		/// This is only accessed from the hook thread, making it immutable after initialization.
		static HOOK_CONTEXT: RefCell<Option<HookContext>> = const { RefCell::new(None) };

feat(agent): add window recording support via now proto dvc #1583

Are you sure you want to change the base?

feat(agent): add window recording support via now proto dvc #1583

Conversation

pacmancoder commented Nov 25, 2025 • edited by atlassian bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Nov 25, 2025

Let maintainers know that an action is required on their side

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pacmancoder commented Dec 2, 2025

Uh oh!

Copilot AI commented Dec 2, 2025

Uh oh!

pacmancoder commented Dec 5, 2025

Testing

Uh oh!

CBenoit Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

CBenoit Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

CBenoit left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

pacmancoder commented Nov 25, 2025 •

edited by atlassian bot

Loading

CBenoit Dec 11, 2025 •

edited

Loading

CBenoit Dec 11, 2025 •

edited

Loading