Skip to content

fix(server)!: pass a DisplayContext to RdpServerDisplay::updates - #2073

Open
uchouT (uchouT) wants to merge 3 commits into
Devolutions:masterfrom
uchouT:server-display
Open

uchouT (uchouT) wants to merge 3 commits into
Devolutions:masterfrom
uchouT:server-display

Conversation

@uchouT

Copy link
Copy Markdown
Contributor

Second step of #1978, stacked on #2059.

RdpServerDisplay::updates() now takes a DisplayContext with the connection's display_suppressed flag and auto-detect handles. Each connection creates its own, replacing the manual resets at connection start. After a Deactivation-Reactivation Sequence, updates() is called again with the same handles.

Breaking: the RdpServerBuilder::with_*_handle methods and RdpServer::*_handle() accessors for these handles are removed.

cc Greg Lamberson (@glamberson) does this signature fit your lamco-rdp-server?

@glamberson

Copy link
Copy Markdown
Contributor

Yes, it fits. I ported lamco-rdp-server and lamco-qemu-rdp to this branch (f8223b8) and both type-check against it, apart from calls into two of our own unmerged PRs. The with_autodetect_*_handle builder calls are gone from both products, and each of our two RdpServerDisplay implementations now takes the handles from updates(ctx).

The one part that wasn't a straight swap is the RTT. Our EGFX flow controller reads it, and that lives on the GfxServerFactory side rather than in the display backend, so it takes the connection's handle from the backend's updates call. That works because the controller reads the handle lazily, and a second updates call after a reactivation passes the same handles, which is harmless for us. The generation restarting at 0 for each connection doesn't matter either, since we only compare generations for inequality.

One gap in the PR: The tests removed with the accessors included autodetect_handles_start_at_the_sentinel_for_each_connection, and nothing replaces it. A new connection starting from the sentinel values and generation 0 is no longer pinned, and neither is AutoDetectHandles::default(). A test that serves two connections and checks that the second updates call gets handles distinct from the first connection's, back at those values, would cover both.

@uchouT

Copy link
Copy Markdown
Contributor Author

Yes, it fits. I ported lamco-rdp-server and lamco-qemu-rdp to this branch (f8223b8) and both type-check against it, apart from calls into two of our own unmerged PRs. The with_autodetect_*_handle builder calls are gone from both products, and each of our two RdpServerDisplay implementations now takes the handles from updates(ctx).

The one part that wasn't a straight swap is the RTT. Our EGFX flow controller reads it, and that lives on the GfxServerFactory side rather than in the display backend, so it takes the connection's handle from the backend's updates call. That works because the controller reads the handle lazily, and a second updates call after a reactivation passes the same handles, which is harmless for us. The generation restarting at 0 for each connection doesn't matter either, since we only compare generations for inequality.

For the GFX side the natural delivery point is GfxServerFactory's build methods, the same way device_added hands a USB device its handle when it is created. What blocks that today is ordering: run_connection_inner attaches the channel backends before negotiation, before the connection state exists. The acceptor first reads the static channels at the MCS Connect Initial in accept_finalize, and serve_negotiated already attaches after negotiation, so attach_channels can move into finalize_negotiated after the connection state is created. The factories can then receive a per-connection context. I'd do that as a separate step, together with moving gfx_handle off RdpServer. Would that work on the lamco side?

One gap in the PR: The tests removed with the accessors included autodetect_handles_start_at_the_sentinel_for_each_connection, and nothing replaces it. A new connection starting from the sentinel values and generation 0 is no longer pinned, and neither is AutoDetectHandles::default(). A test that serves two connections and checks that the second updates call gets handles distinct from the first connection's, back at those values, would cover both.

I've added new e2e test. The shared helper functions in #2046 seem to help, I'll reuse the helpers once #2046 is merged.

@uchouT
uchouT (uchouT) force-pushed the server-display branch 3 times, most recently from 6f3a41e to c884b97 Compare October 8, 2026 14:44
@uchouT
uchouT (uchouT) marked this pull request as ready for review October 8, 2026 14:44
Copilot AI balanced review requested due to automatic review settings October 8, 2026 14:44

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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@uchouT uchouT (uchouT) changed the title refactor(server)!: pass a DisplayContext to RdpServerDisplay::updates fix(server)!: pass a DisplayContext to RdpServerDisplay::updates Oct 8, 2026
@github-actions github-actions Bot added breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny scope/core Touches the core architectural tier size/XL Size: up to 1299 counted lines and 49 files; exceeds L in either measure labels Oct 8, 2026

@github-actions github-actions Bot 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.

Breaking but declared refactor moving display_suppressed and auto-detect measurement handles from server-global Arcs (with manual per-connection resets) into per-connection ConnectionState fields, delivered to RdpServerDisplay::updates via a new DisplayContext. Verified against the head: behavior is preserved for the removed API, the reactivation re-invocation with the same handles holds, no dangling references to removed methods remain, and the removed accessor test coverage is replaced by a Default test plus a new e2e test pinning per-connection isolation, sentinel starts, and RTT propagation. Two low-severity findings published: auto-detect measurements now reach embedders only through the display backend's updates() call (EGFX/GfxServerFactory delivery deferred to a follow-up), and the new e2e test's connect helper duplicates the pre-existing connect_client helper. A third candidate (inlining the single-caller display_context() helper) is rejected: the helper documents the per-act…

Comment on lines +298 to +321
pub struct DisplayContext {
/// `true` while the client has sent `SuppressOutput { desktop_rect: None }`
/// (e.g., mstsc minimized), `false` at the start of the connection. Cleared
/// on `SuppressOutput { Some(rect) }` or `RefreshRectangle`.
///
/// A backend can skip frame emission while it's set, so the client doesn't
/// accumulate a backlog of frames it can't present until refocus.
///
/// **Caveat:** some clients (notably mstsc) send
/// `SuppressOutput { desktop_rect: None }` during their connect
/// handshake *before* their display surface is fully initialized; a
/// backend that honors the flag blindly will block that first frame
/// and leave the client with a half-initialized surface that doesn't
/// recover on un-suppress (visible as a frozen desktop on first
/// connect). Backends are advised to defer acting on the flag until
/// after the first frame has been delivered to the client, and to
/// debounce transient flaps (some clients pulse this PDU under wire
/// pressure on heavy CPU/IO loads) — e.g., only engage the gate once
/// the flag has been steady-`true` for ~1 s.
pub display_suppressed: Arc<AtomicBool>,

/// The connection's auto-detect measurements, for flow control.
pub autodetect: AutoDetectHandles,
}

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.

[skeptical] Auto-detect handles lose every observation point except RdpServerDisplay::updates — low 🟡 — The PR removes both the with_autodetect_*_handle builder methods and the RdpServer::autodetect_*_handle() accessors, leaving DisplayContext (delivered only at the single updates(conn.display_context()) call in client_loop) as the sole way to observe RTT/baseline/bandwidth figures whose documented purpose is flow control. Consumers whose flow control lives outside the display backend — notably the EGFX path via GfxServerFactory — must capture the handles indirectly from a backend's updates() call, and an embedder running with no display backend has no access to measurements. The author defers GfxServerFactory delivery (via finalize_negotiated attachment ordering) to a follow-up PR and the affected consumer confirmed the workaround type-checks and works in their products, so this is a temporary capability narrowing, not a blocking defect; recorded so the follow-up does not silently lapse.

Comment thread crates/ironrdp-testsuite-extra/tests/e2e.rs Outdated
@github-actions github-actions Bot added ai-reviewed/1 One automated review completed needs-author-action The pull request author is the current next actor labels Oct 8, 2026
@glamberson

Copy link
Copy Markdown
Contributor

This now conflicts with #1413, which merged after your push. The textual conflicts are in builder.rs, where the removed handle builders sit exactly where #1413 added with_connection_binder, and in server.rs, in an import, a clear_bound_connection() line and a test stub. There is also a break that git won't show. I resolved those hunks in a scratch copy and compiled ironrdp-server, and the library fails with three errors, all in BoundDisplaySlot::updates. #1413 added it with the old signature, and it needs the context passed through to the bound or default display. The two stub displays in #1413's tests in server.rs declare updates the old way as well. I didn't get as far as the fixtures in ironrdp-testsuite-extra, because the library stops the build first.

The lamco-rdp-server port I described still matches the fields in this head by name. I haven't recompiled it against it.

@github-actions github-actions Bot added scope/cross-cutting Spans multiple architectural boundaries and removed needs-author-action The pull request author is the current next actor kind/protocol Affects RDP or related protocol behavior labels Oct 9, 2026

@github-actions github-actions Bot 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.

The PR replaces the five server-level Arc handles (display_suppressed plus four auto-detect figures), their builder injectors, accessors, and the scattered manual sentinel resets with a per-connection DisplayContext delivered to RdpServerDisplay::updates. I verified independently that per-connection ConnectionState-owned handles genuinely fix the stale-state bugs the removed resets patched (minimized-disconnect suppression leak, prior connection's RTT/bandwidth leaking into a new session), that Default matches the old reset semantics, and that the serve loop re-enters client_accepted after Deactivation-Reactivation with the same ConnectionState, so the same handles are redelivered. The head already reconciles with #1413 (with_connection_binder coexists with the removals, BoundDisplaySlot::updates forwards ctx). Three publishable issues remain: GfxServerFactory consumers have no delivery path for the flow-control handles at this head, the documented same-handles-after-reactivation guar…

Comment on lines +287 to +321
/// What a connection publishes to its display backend, handed to
/// [`RdpServerDisplay::updates`].
///
/// The server writes these values while the connection runs; the backend keeps
/// the handles it needs and reads them. Each connection has its own handles,
/// starting from the initial values documented on each field, so nothing the
/// previous connection set or measured carries over. A
/// Deactivation-Reactivation Sequence keeps the connection, so `updates` is
/// then called again with a context holding the same handles.
#[derive(Debug)]
#[non_exhaustive]
pub struct DisplayContext {
/// `true` while the client has sent `SuppressOutput { desktop_rect: None }`
/// (e.g., mstsc minimized), `false` at the start of the connection. Cleared
/// on `SuppressOutput { Some(rect) }` or `RefreshRectangle`.
///
/// A backend can skip frame emission while it's set, so the client doesn't
/// accumulate a backlog of frames it can't present until refocus.
///
/// **Caveat:** some clients (notably mstsc) send
/// `SuppressOutput { desktop_rect: None }` during their connect
/// handshake *before* their display surface is fully initialized; a
/// backend that honors the flag blindly will block that first frame
/// and leave the client with a half-initialized surface that doesn't
/// recover on un-suppress (visible as a frozen desktop on first
/// connect). Backends are advised to defer acting on the flag until
/// after the first frame has been delivered to the client, and to
/// debounce transient flaps (some clients pulse this PDU under wire
/// pressure on heavy CPU/IO loads) — e.g., only engage the gate once
/// the flag has been steady-`true` for ~1 s.
pub display_suppressed: Arc<AtomicBool>,

/// The connection's auto-detect measurements, for flow control.
pub autodetect: AutoDetectHandles,
}

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.

[skeptical] DisplayContext is the only delivery point for the flow-control handles, leaving GfxServerFactory consumers with none — medium 🟠 — The struct's docs bill the auto-detect handles 'for flow control' (line 319), whose primary consumer is the EGFX frame pacer behind GfxServerFactory. At this head GfxServerFactory::build_gfx_handler/build_server_with_handle (gfx.rs:32, called at server.rs:2264-2268) take no per-connection context, and this PR deletes the only construction-time sharing mechanism (with_autodetect_*_handle). EGFX flow controllers must therefore obtain the RTT/bandwidth handles sideways through the display backend's updates(ctx), coupling two independent components in every embedder. The author defers factory-context delivery to a later step, but neither DisplayContext nor AutoDetectHandles documents that they are display-only for now; land the factory delivery or state the gap on these types.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment thread crates/ironrdp-server/src/display.rs
Comment thread crates/ironrdp-testsuite-extra/tests/e2e.rs Outdated
@github-actions github-actions Bot removed the ai-reviewed/1 One automated review completed label Oct 9, 2026
@github-actions github-actions Bot added ai-reviewed/2 Two automated reviews completed needs-author-action The pull request author is the current next actor labels Oct 9, 2026
@github-actions github-actions Bot added kind/protocol Affects RDP or related protocol behavior and removed needs-author-action The pull request author is the current next actor scope/cross-cutting Spans multiple architectural boundaries labels Oct 9, 2026

@github-actions github-actions Bot 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.

The PR replaces server-lifetime shared display-suppression and auto-detect handles (five builder injectors, five accessors, manual resets at connection start) with per-connection ConnectionState state delivered to RdpServerDisplay::updates via a new DisplayContext. I independently verified the change: the single updates() call site runs after ConnectionState exists, SuppressOutput/RefreshRectangle handling now writes the per-connection flag, fresh AutoDetectHandles::default() per connection removes the manual resets, and all in-tree trait implementers plus tests are updated. The change is protocol-clean and a net simplification. Of the six candidates, five are confirmed verbatim in the head and published (four accepted, one refined with an exact line range). The sixth (stale-against-merged-1413) is rejected: it was derived from PR-thread comments about an earlier push, but the inspected head already contains #1413's with_connection_binder and the re-signatured BoundDisplaySlot::update…

  1. [code-compressor] Bandwidth(Some) and Bandwidth(None) arms duplicate the store-then-increment pair — low 🟡 — crates/ironrdp-server/src/server.rs
    Both arms (server.rs:4819-4833 and 4834-4846) perform autodetect_handles.bandwidth.store(...) followed by autodetect_handles.bandwidth_generation.fetch_add(1, Ordering::Release); they differ only in the stored value (bandwidth_kbps vs u32::MAX) and the log call. Matching on AutoDetectOutcome::Bandwidth(bandwidth_kbps) once, storing bandwidth_kbps.unwrap_or(u32::MAX), doing the single fetch_add, then branching only for the debug!/trace! calls (keeping each arm's log and its 'mirror the manager's reset' comment) removes duplicated state mutation and one match arm while preserving the exact stores, ordering, and log output. This matters because the store/fetch_add pairing is a Release/Acquire protocol pair — writing it once makes it harder to break in one arm only.

Comment on lines +287 to +321
/// What a connection publishes to its display backend, handed to
/// [`RdpServerDisplay::updates`].
///
/// The server writes these values while the connection runs; the backend keeps
/// the handles it needs and reads them. Each connection has its own handles,
/// starting from the initial values documented on each field, so nothing the
/// previous connection set or measured carries over. A
/// Deactivation-Reactivation Sequence keeps the connection, so `updates` is
/// then called again with a context holding the same handles.
#[derive(Debug)]
#[non_exhaustive]
pub struct DisplayContext {
/// `true` while the client has sent `SuppressOutput { desktop_rect: None }`
/// (e.g., mstsc minimized), `false` at the start of the connection. Cleared
/// on `SuppressOutput { Some(rect) }` or `RefreshRectangle`.
///
/// A backend can skip frame emission while it's set, so the client doesn't
/// accumulate a backlog of frames it can't present until refocus.
///
/// **Caveat:** some clients (notably mstsc) send
/// `SuppressOutput { desktop_rect: None }` during their connect
/// handshake *before* their display surface is fully initialized; a
/// backend that honors the flag blindly will block that first frame
/// and leave the client with a half-initialized surface that doesn't
/// recover on un-suppress (visible as a frozen desktop on first
/// connect). Backends are advised to defer acting on the flag until
/// after the first frame has been delivered to the client, and to
/// debounce transient flaps (some clients pulse this PDU under wire
/// pressure on heavy CPU/IO loads) — e.g., only engage the gate once
/// the flag has been steady-`true` for ~1 s.
pub display_suppressed: Arc<AtomicBool>,

/// The connection's auto-detect measurements, for flow control.
pub autodetect: AutoDetectHandles,
}

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.

[skeptical] DisplayContext is the only delivery point for connection auto-detect handles, forcing non-display consumers through the display backend — low 🟡 — DisplayContext hands the RTT/bandwidth handles to RdpServerDisplay::updates and nowhere else. A consumer needing them outside the display backend must capture them from updates() and pass them sideways, as the known downstream's GfxServerFactory-side EGFX flow controller does (per the PR thread). The author's stated follow-up — moving attach_channels into finalize_negotiated so factories receive a per-connection context — confirms this delivery point is transitional and the API shape will be extended again. Tolerable as an explicitly staged step with a confirmed consumer; #[non_exhaustive] keeps the later addition non-breaking, but the interim indirection should not be treated as the settled design.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See #2073 (comment):

For the GFX side the natural delivery point is GfxServerFactory's build methods, the same way device_added hands a USB device its handle when it is created. What blocks that today is ordering: run_connection_inner attaches the channel backends before negotiation, before the connection state exists. The acceptor first reads the static channels at the MCS Connect Initial in accept_finalize, and serve_negotiated already attaches after negotiation, so attach_channels can move into finalize_negotiated after the connection state is created. The factories can then receive a per-connection context. I'd do that as a separate step, together with moving gfx_handle off RdpServer.

Comment on lines +562 to +626
'probe: loop {
let (action, payload) = framed.read_pdu().await.expect("valid PDU");
for out in stage.process(&mut image, action, &payload).expect("stage process") {
if let ActiveStageOutput::ResponseFrame(frame) = out {
framed.write_all(&frame).await.expect("write RTT response");
break 'probe;
}
}
}
tokio::time::timeout(Duration::from_secs(5), async {
while first.autodetect.rtt.load(Ordering::Relaxed) == u32::MAX {
tokio::time::sleep(Duration::from_millis(10)).await;
}
})
.await
.expect("the RTT reaches the display's handle");

// A resize deactivates and reactivates the same connection,
// and the display is asked for its updates again.
display_tx
.send(DisplayUpdate::Resize(DesktopSize {
width: 2048,
height: 2048,
}))
.unwrap();
'deactivate: loop {
let (action, payload) = framed.read_pdu().await.expect("valid PDU");
for out in stage.process(&mut image, action, &payload).expect("stage process") {
if matches!(out, ActiveStageOutput::DeactivateAll) {
break 'deactivate;
}
}
}
let mut connection_activation = activation_factory.create();
let mut buf = pdu::WriteBuf::new();
loop {
let written =
ironrdp_async::single_sequence_step_read(&mut framed, &mut connection_activation, &mut buf)
.await
.expect("read deactivation-reactivation sequence step");
if written.size().is_some() {
framed
.write_all(buf.filled())
.await
.expect("write deactivation-reactivation sequence step");
}
if let connector::connection_activation::ConnectionActivationState::Finalized {
share_id,
enable_server_pointer,
pointer_software_rendering,
static_channel_chunk_size,
..
} = connection_activation.connection_activation_state()
{
assert!(stage.reactivate(
connection_activation.io_channel_id(),
connection_activation.user_channel_id(),
share_id,
enable_server_pointer,
pointer_software_rendering,
static_channel_chunk_size,
));
break;
}
}

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.

[skeptical] New e2e test can hang CI instead of failing: unbounded read loops with no timeout — low 🟡 — In each_connection_gets_its_own_display_context only the RTT-poll wait (lines 571-577) is bounded by a 5s tokio::time::timeout. The 'probe loop (562-570), the 'deactivate loop (587-594), the activation-sequence loop (597-626), and the ctx_rx.recv() awaits have no timeout, and #[tokio::test] imposes none. If a regression stops the server from sending the expected PDU (e.g., updates() no longer re-invoked after reactivation, or the auto-detect probe never issued), framed.read_pdu() blocks forever and the test hangs the CI run rather than failing with a diagnostic. Wrapping each phase in tokio::time::timeout would make regressions fail loudly.

Comment thread crates/ironrdp-server/src/autodetect.rs
Comment on lines +1446 to +1452
/// Whether the client asked the server to stop sending display updates
/// (`SuppressOutput { desktop_rect: None }`), published to the display
/// backend through [`DisplayContext::display_suppressed`]. Without it, a
/// server keeps streaming high-bitrate EGFX/H.264 frames into a minimized
/// client, which accumulates them and locks up its input dispatch for
/// seconds on refocus while it chews through the backlog.
display_suppressed: Arc<AtomicBool>,

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.

[code-compressor] ConnectionState::display_suppressed doc is the third near-copy of the same rationale — low 🟡 — The new doc on ConnectionState::display_suppressed (server.rs:1446-1452) repeats the 'keeps streaming high-bitrate EGFX/H.264 frames into a minimized client... locks up its input dispatch' rationale that already lives, in substance, on the public DisplayContext::display_suppressed field (display.rs:299-316) and in the pre-existing SuppressOutput handler arm comment. Three copies of the same client-behavior lore will drift independently. Keep the full rationale on the public DisplayContext field where backends read it, and reduce the private field doc to the mechanism plus a pointer (e.g. 'published to the backend via DisplayContext::display_suppressed; see there for why'). Documentation only; no behavior impact.

@github-actions github-actions Bot added ai-reviewed/3 Final automated review completed needs-author-action The pull request author is the current next actor and removed ai-reviewed/2 Two automated reviews completed labels Oct 9, 2026
@glamberson

Copy link
Copy Markdown
Contributor

Yes, that works for lamco, and it fits better than what lamco does now. Lamco's factory builds a new GraphicsPipelineServer for each connection in build_server_with_handle, and the flow controller it feeds lives on the factory side, so today the RTT handle reaches it sideways through updates(ctx). A per-connection context on the factory's build methods would put it where it's used. Bandwidth and the generation are read in the display pipeline, so those stay in DisplayContext. Nothing in lamco calls RdpServer::gfx_handle(), so moving it off RdpServer costs nothing, and lamco's other factories only construct their handlers when built, so building them after authentication is fine too.

Four things from lamco's side. Make the context non_exhaustive and carry the same handles as DisplayContext, the same Arcs. Document that a connection's factory build comes before its updates() call, since lamco's display handler relies on that order. If it's easy, deliver the connection's GfxServerHandle there too, because lamco keeps a shared cell and a pointer comparison just to track the current one. And I agree with the automated review's finding on this PR. Until the factory step lands, say on DisplayContext and AutoDetectHandles that they reach the display only.

I'd keep that as the separate step you proposed, so it doesn't need to hold this PR. I haven't built lamco against this head yet, and my port was checked against f8223b8.

@CBenoit

Copy link
Copy Markdown
Member

Hi Greg Lamberson (@glamberson)
Just confirming: is it okay for you if I merge this PR once the last findings are addressed?

@CBenoit

Copy link
Copy Markdown
Member

I also merged #2046

The display suppression flag and the auto-detect handles describe one
connection, but were created with the server and reset by hand at the
start of each connection. Each connection now creates its own and hands
them to the display backend through `updates()`, which is called again
with the same handles after a Deactivation-Reactivation Sequence.

BREAKING CHANGE: `RdpServerDisplay::updates` takes a `DisplayContext`.
`RdpServer::display_suppressed_handle`, `autodetect_rtt_handle`,
`autodetect_baseline_rtt_handle`, `autodetect_bandwidth_handle`,
`autodetect_bandwidth_generation_handle` and the matching
`RdpServerBuilder::with_*_handle` methods are removed; read the handles
from the `DisplayContext` instead.

Signed-off-by: uchouT <i@uchout.moe>
Signed-off-by: uchouT <i@uchout.moe>
Signed-off-by: uchouT <i@uchout.moe>
@github-actions github-actions Bot added scope/cross-cutting Spans multiple architectural boundaries needs-review A human reviewer is the current next actor and removed needs-author-action The pull request author is the current next actor labels Oct 9, 2026
@glamberson

Copy link
Copy Markdown
Contributor

Yes, that's fine with me. I built lamco-rdp-server against the current head with the port I described, and it compiles with no changes beyond it, so nothing here needs to hold the merge. The open threads about GfxServerFactory not receiving the context are the separate step uchouT and I talked about, and I'm happy for that to land afterwards. One heads-up for the merge order. My #1985 touches the same lines in BoundDisplaySlot, so if this merges first I'll rebase it.

This branch was successfully deployed

1 active deployment
llm-providers — ce2f4c8d Deployed Oct 9, 2026 by uchouT via Classify pull request #2383
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed/3 Final automated review completed breaking-change Includes a breaking change, and requires special scrutiny at the boundaries kind/protocol Affects RDP or related protocol behavior needs-review A human reviewer is the current next actor risk/high Substantial core public API impact, or fail-closed triage; needs maintainer-level scrutiny scope/core Touches the core architectural tier scope/cross-cutting Spans multiple architectural boundaries size/XL Size: up to 1299 counted lines and 49 files; exceeds L in either measure

Development

Successfully merging this pull request may close these issues.

4 participants