Skip to content

feat(server): wire MS-RDPEI server-side into ironrdp-server - #1773

Merged
Marc-André Moreau (mamoreau-devolutions) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/rdpei-server-wiring-clean
Aug 23, 2026
Merged

Marc-André Moreau (mamoreau-devolutions) merged 2 commits into
Devolutions:masterfrom
lamco-admin:feat/rdpei-server-wiring-clean

Conversation

@glamberson

@glamberson Greg Lamberson (glamberson) commented Aug 23, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • MS-RDPEI (multitouch and pen) got server-side plumbing in ironrdp-rdpei (the recent Input DVC PR), but nothing in ironrdp-server ever registers the RdpeiServer/RdpeiHandler it added. Grepped the whole crate for rdpei before starting: zero references. This wires it in.
  • Follows the exact shape of the existing SoundServerFactory/with_sound_factory (the simplest of the three existing opt-in factories, and RDPEI's needs are equally simple): a new RdpeiServerFactory trait, a with_rdpei_factory builder method, and registration in attach_channels alongside the other optional DVC factories. The factory builds and returns the whole configured RdpeiServer, not just a handler, so it can reach with_protocol_version/with_supported_features to advertise non-default capabilities.
  • Re-exports everything a downstream RdpeiHandler implementation needs from the crate root: RdpeiServerFactory, RdpeiHandler, RdpeiServer, and the touch/pen/ready PDU types plus the nested payload types their callbacks embed (CsReadyFlags, PenContact and its fields/flags, PenFrame, TouchContactFields/TouchContactDataFlags).

Validation

cargo xtask check fmt/lints/tests/typos/locks all pass. Adds a regression test in ironrdp-testsuite-core (ironrdp-server has [lib] test = false, so an inline test would never run in CI) covering that a registered RdpeiServerFactory is invoked during per-connection channel setup.

Notes

Two crates: ironrdp-server (Cargo.toml, Cargo.lock, builder.rs, lib.rs, server.rs, the new rdpei.rs) and ironrdp-testsuite-core (Cargo.toml, tests/server/mod.rs, the new tests/server/rdpei.rs). RdpServer::new is pub(crate), only ever called from the builder, so adding the new parameter there has no external API impact beyond the new builder method itself.

ironrdp-rdpei gained a server-side DVC processor (RdpeiServer,
RdpeiHandler) in the recent client-side RDPEI landing, but nothing in
ironrdp-server ever registered it. Add RdpeiServerFactory following
the exact shape of the existing SoundServerFactory, an opt-in
with_rdpei_factory builder method, and wire it into attach_channels
alongside the other optional DVC factories. Re-export everything a
downstream handler implementation needs from the crate root.
@github-actions github-actions Bot added needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure labels Aug 23, 2026

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

Wires MS-RDPEI multitouch and pen input into ironrdp-server.

Changes:

  • Adds an RDPEI factory and builder configuration.
  • Registers an RDPEI server on the DRDYNVC channel.
  • Re-exports RDPEI APIs and adds the dependency.

Reviewed changes

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

Show a summary per file
File Description
Cargo.lock Records the RDPEI dependency.
crates/ironrdp-server/Cargo.toml Adds ironrdp-rdpei.
crates/ironrdp-server/src/builder.rs Adds RDPEI factory configuration.
crates/ironrdp-server/src/lib.rs Exposes RDPEI APIs.
crates/ironrdp-server/src/rdpei.rs Defines the RDPEI factory facade.
crates/ironrdp-server/src/server.rs Registers RDPEI per connection.

Comment thread crates/ironrdp-server/src/rdpei.rs Outdated
Comment thread crates/ironrdp-server/src/rdpei.rs
Comment thread crates/ironrdp-server/src/server.rs Outdated
RdpeiServerFactory::build_handler returned only a handler, and
attach_channels always wrapped it with RdpeiServer::new, so a factory
had no way to reach RdpeiServer::with_protocol_version or
with_supported_features to advertise non-default capabilities. The
factory now builds and returns the whole RdpeiServer.

Also re-exports the nested touch/pen/ready payload types the callback
PDUs embed (CsReadyFlags, PenContact and its fields/flags, PenFrame,
TouchContactFields/DataFlags), which a downstream RdpeiHandler
implementation needs to interpret cs_ready/touch/pen callbacks without
a direct ironrdp-rdpei dependency.

Adds a regression test in ironrdp-testsuite-core (this crate has
test = false, so an inline test would never run in CI) covering the
bug this PR itself fixed: a registered RdpeiServerFactory must be
invoked during per-connection channel setup, not silently skipped.
@mamoreau-devolutions
Marc-André Moreau (mamoreau-devolutions) merged commit aa260fe into Devolutions:master Aug 23, 2026
43 checks passed
Anton Mostovoy (antonmos) added a commit to antonmos/IronRDP that referenced this pull request Aug 24, 2026
…e public API)

Six upstream commits since the last sync, in crates/ironrdp-server:
Devolutions#1242 (typed ServerError, breaking), Devolutions#1417 (rdpeusb server integration),
Devolutions#1773 (RDPEI wiring), Devolutions#1737 (RTT baseline), Devolutions#1769 (mouse position, no
server.rs footprint), Devolutions#1754 (test relocation, no server.rs footprint).

Only one real conflict, in run()'s loop -- the same class as prior
syncs: HEAD's preemption race vs upstream's plain accept-loop tail.
Resolved by taking HEAD's side (a strict superset).

The change that needed real attention, not just conflict resolution:
Devolutions#1242 makes run/run_connection/run_connection_with return ServerResult
(a new typed error), keeping the anyhow-returning bodies as private
_inner methods -- exactly the same wrap-and-rename shape Devolutions#1721 already
used for run_connection_with (now a third layer: run_connection_with ->
run_connection_with_inner [Devolutions#1242, does the Devolutions#1721 static-channels reset]
-> run_connection_inner [the real body, PendingConnection-based here]).
Upstream's own accept loop switched to calling run_connection_with_inner
directly instead of the now-ServerResult-returning public wrapper, since
its `conn`-equivalent needs anyhow::Error for on_disconnected and a
consistent type across match arms.

This branch's preemption race has the identical two call sites
(Entry::Fresh's stream, in both the preempt-enabled race and the
plain-queue-behind fallback) that were still calling the public
run_connection -- which would have silently started returning the wrong
error type (ServerResult vs serve_negotiated's anyhow Result, breaking
the match arms' common type) had the merge not been caught by an actual
build. Switched both to run_connection_with_inner(stream,
TransportTls::Managed), matching upstream's own fix exactly, with a
comment explaining why (this is not a stylistic change -- it's required
for the code to typecheck at all once the two arms feeding into `conn`
must share one Result type).

Devolutions#1417 (USB) and Devolutions#1773 (RDPEI)'s server.rs footprint is additive and,
for USB, feature-gated (`#[cfg(feature = "usb")]`) so it's a no-op in
every build this branch's own verification exercises; Devolutions#1754 relocated
the autodetect unit tests out of this crate into ironrdp-testsuite-core,
so ironrdp-server's own suite drops from 24 to 14 tests -- confirmed by
grep that they still exist there, not lost.

14/14 tests pass in both default and --features egfx (down from 24,
accounted for above, not a regression), clippy --all-targets clean in
both, fmt clean, full cargo build --workspace green.
@glamberson
Greg Lamberson (glamberson) deleted the feat/rdpei-server-wiring-clean branch August 30, 2026 15:12

This branch was previously deployed

1 inactive deployment
llm-providers — 69a8c1fb Deployed Aug 23, 2026 by glamberson via Classify pull request #3587
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review A human reviewer is the current next actor risk/unknown Risk could not be determined automatically; needs maintainer-level scrutiny size/M Size: up to 449 counted lines and 10 files; exceeds S in either measure

Development

Successfully merging this pull request may close these issues.

3 participants