Skip to content

refactor(agent): reuse NOW execution for PSU gRPC#1844

Open
Marc-André Moreau (mamoreau-devolutions) wants to merge 1 commit into
masterfrom
agent-remote-execution-reuse
Open

refactor(agent): reuse NOW execution for PSU gRPC#1844
Marc-André Moreau (mamoreau-devolutions) wants to merge 1 commit into
masterfrom
agent-remote-execution-reuse

Conversation

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor

Reuses the existing Devolutions Session NOW_EXEC process backend for PSU gRPC remote execution on Windows while keeping the gRPC protocol unchanged. This aligns process launch, stdin/stdout handling, cancellation, and cleanup with the Agent's existing NOW behavior instead of maintaining a separate Windows execution path.

Keeps the non-Windows fallback on tokio process execution and hardens duplicate process/stream validation and failure cleanup around the gRPC adapter.

Issue: N/A

@mamoreau-devolutions

Copy link
Copy Markdown
Contributor Author

Implementation notes:

  • Windows PSU gRPC StartProcess now adapts into the existing NOW/DVC WinApiProcessBuilder path from devolutions-session, using raw data encoding because the current gRPC protocol has no NOW encoding flags.
  • StopProcess maps kill_process=true to NOW abort and kill_process=false to NOW cancel; stdin StreamData is forwarded through the DVC process input channel.
  • The gRPC protocol still has no stdout/stderr stream kind, so stdout remains StreamData and stderr is surfaced as diagnostics instead of being silently dropped.
  • Added duplicate process and stream ID handling before spawning process work, plus explicit working-directory validation and registry cleanup on spawn failure.
  • Added opt-in kill-on-drop support to the shared WinApiProcess wrapper and enabled it only for the gRPC adapter so dropped gRPC process tasks do not orphan child processes; existing NOW/DVC sessions keep the previous default.
  • Validation: cargo +nightly fmt --all; cargo check -p devolutions-agent -p devolutions-session --features devolutions-session/dvc; cargo test -p devolutions-agent psu_grpc_agent -- --nocapture; cargo clippy --workspace --tests -- -D warnings; cargo test --workspace --exclude devolutions-agent-updater -- --test-threads=1. Full cargo test --workspace requires elevation for the updater test binary in this environment.

Base automatically changed from psu-devoagent-poc to master July 2, 2026 12:20
Route the PSU gRPC remote execution path through the existing NOW/DVC Windows process backend while preserving the current gRPC wire protocol.

Keep non-Windows execution on the existing tokio fallback and tighten process/stream cleanup and validation around the gRPC adapter.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors the Devolutions Agent's PSU gRPC remote-execution feature so that, on Windows, process launch, stdin/stdout handling, cancellation, and cleanup are delegated to the Agent's existing NOW_EXEC (WinApiProcessBuilder) backend instead of a separate tokio::process path. The gRPC protocol is unchanged. The non-Windows fallback keeps using tokio::process::Command, and duplicate process/stream validation plus failure cleanup around the gRPC adapter are hardened. This aligns the two agent execution surfaces (DVC NOW sessions and PSU gRPC) on a single Windows implementation.

Changes:

  • Added a Windows-only run_process_inner_windows that drives the NOW backend (IO redirection, raw encoding, kill-on-drop) and maps NOW ServerChannelEvents to PSU protocol messages.
  • Extended WinApiProcessBuilder/WinApiProcess with opt-in with_kill_on_drop/disable_kill_on_drop, and added process_id to ServerChannelEvent::SessionStarted.
  • Hardened PSU gRPC handling: reject empty/duplicate correlation & stream IDs, stricter working-directory validation, raw stdout chunking, and pass-through stdin (no injected newlines).

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
devolutions-agent/src/psu_grpc_agent/process.rs Adds the Windows NOW-backed execution path, duplicate-registry rejection, and raw stdout/stdin handling; adds tests.
devolutions-agent/src/psu_grpc_agent/mod.rs Validates StartProcess IDs and handles duplicate process/stream registration with failure responses.
devolutions-session/src/dvc/process.rs Adds process_id to SessionStarted and opt-in kill-on-drop for WinApiProcess.
devolutions-session/src/dvc/task.rs Updates the SessionStarted consumer to ignore the new field.
devolutions-agent/Cargo.toml, Cargo.lock, src/main.rs Wire in devolutions-session (dvc) and now-proto-pdu as Windows-only dependencies.

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

Comment on lines +569 to +573
AgentPayload::StreamClosed(stream_closed(
request.stream_id.clone(),
"child process completed".to_owned(),
false,
)),
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants