[Rophy Tsai (rophy)]

The client's maxUnacknowledgedFrameCount from the core Frame Acknowledge Capability Set no longer caps the EGFX frame window by default. MS-RDPRFX 2.2.1.3 defines that value for surface-command frames acknowledged with TS_FRAME_ACKNOWLEDGE_PDU; EGFX has its own acknowledgement and queueDepth (MS-RDPEGFX 2.2.2.13, 3.2.5.13). Windows 10 mstsc sends 1, which allowed one EGFX frame per round trip.

RdpServerBuilder::with_egfx_core_frame_ack_limit(true) (or RdpServer::set_egfx_core_frame_ack_limit) keeps the cap that #2102 added. #2102's typed backpressure result (FrameSubmissionError::Backpressured) does not depend on the cap.

Fixes #2237

[Rophy Tsai (rophy)]

Validation

• Tests in ironrdp-testsuite-extra: a client that advertises EGFX and a bitmap codec (so maxUnacknowledgedFrameCount 20) connects to a server with an EGFX window of 30. Each test reads the window at activation, sets a stale value of 1, triggers a Deactivation-Reactivation Sequence with a display resize, and reads the window again. core_frame_ack_capability_does_not_cap_the_egfx_window_by_default leaves the option unset (the library default) and expects 30 both times; core_frame_ack_capability_caps_the_egfx_window_when_enabled expects 20 both times. With the cap always applied, the first fails; with it never applied, the second fails; with it applied only at the first activation, both fail at the reactivation (they read 1). With the library default switched to on, the default test fails.
• Frame rate: from ironrdp-server: surface-command frame ack limit unexpectedly caps the EGFX window #2237, mstsc on Windows 10 with an EGFX H.264 server on ironrdp-server went from 19 fps (window 1) to 34 fps (window 3, the limit of our test VM). Not measured again on this branch: the server example has no EGFX output.
• cargo xtask check fmt, check lints and check tests pass on macOS (Apple silicon), 4316 tests. Not run: WASM and FFI/.NET.

Lines changed: production code +47 / -1 (server.rs +31 / -1, builder.rs +16), tests +185 (ironrdp-testsuite-extra/tests/e2e.rs).

Note

Human-tuned, LLM-assisted content.

[github-actions]

The fix is correct and protocol-conformant: the core Frame Acknowledge capability (MS-RDPRFX 2.2.1.3, defined for TS_FRAME_ACKNOWLEDGE_PDU surface frames) no longer caps the EGFX window by default, with a documented opt-in that restores the #2102 behavior. The cap is applied at the single shared activation site, so it correctly covers reactivation, and passing None clears the ceiling as the egfx API documents. The skeptical call to delete the cap instead of adding opt-in APIs was rejected: removal would be a silent breaking behavior change for #2102 consumers, and operators retain control via set_max_frames_in_flight and client queueDepth. Published findings are limited maintainability/test-quality notes: the egfx-level cap API is left without an in-repo caller, the no-cap test's oracle omits MS-RDPEGFX, the enabled-path test's expected value 20 depends on the temporary FIXME(#447) connector constant, reactivation coverage is asserted in docs but untested, and the new helper repeats h…

1. [test] Documented reapplication of the core ack cap on reactivation is not pinned by any test — medium 🟠 — crates/ironrdp-testsuite-extra/tests/e2e.rs
   server.rs:5167-5178 applies the core Frame Acknowledge ceiling on every capability exchange, and both set_egfx_core_frame_ack_limit's doc and the egfx API's doc promise reactivation coverage, but the new e2e tests cover only initial activation. If a future change reapplies or clears the limit only on the non-reactivation pass, renegotiating clients would silently regress to the #2237 capped-window stall with no failing test. Extend the helper (e.g. a resize-triggered second capability exchange) to re-read max_frames_in_flight afterwards.

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

[Rophy Tsai (rophy)]

On the review's medium finding (reactivation not tested): done in 121afb0. Both tests now set a stale window of 1, trigger a Deactivation-Reactivation Sequence with a display resize, and check that the server applies the window again (30 by default, 20 with the option on). With the cap applied only at the first activation, both tests fail at that step.

Note

Human-tuned, LLM-assisted content.

[github-actions]

PR #2238 adds an opt-in, off-by-default egfx_core_frame_ack_limit option to ironrdp-server so the client's core Frame Acknowledge capability (MS-RDPRFX 2.2.1.3) no longer caps the EGFX frame window by default, fixing the mstsc window-of-1 regression (#2237). The disabled path passes None to set_client_frame_ack_limit on every capability exchange, correctly clearing a stale ceiling on reactivation; the enabled path preserves #2102's behavior. The change is a server-side throttle policy, not a wire-format change, and both behaviors conform to the specs since maxUnacknowledgedFrameCount is a non-mandatory hint and EGFX uses its own RDPGFX_FRAME_ACKNOWLEDGE_PDU. Two low-severity e2e tests with spec and regression oracle markers exercise activation and reactivation and are mutation-adequate. I independently verified the three candidates: the opt-in truly has no in-repo consumer besides the new test, the enabled-path test lacks the reference marker the repo's oracle rules require for its co…

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.

[github-actions]

PR #2238 makes the client's core Frame Acknowledge capability (MS-RDPRFX 2.2.1.3) an opt-in cap for the EGFX frame window, defaulting off, applied on both activation and reactivation in server.rs (set_client_frame_ack_limit call site), with new builder/runtime setters in builder.rs and mutation-covered e2e tests. I independently verified the wiring: the flag is read in the shared activation/reactivation pass, None correctly clears a stale ceiling, the default is false in both builder constructors and RdpServer::new, and the tests exercise the reactivation path. The core behavior is well-justified by the spec distinction (MS-RDPRFX 2.2.1.3 vs MS-RDPEGFX 2.2.2.13) and regression #2237. Remaining issues are non-blocking: the opt-in flag preserves a spec-incorrect path behind two new public entry points with only an unconfirmed embedder need (refined to low, kept as a question); the new e2e helper duplicates TLS/activation scaffolding already repeated in the file (low, tracked debt, delib…

1. [skeptical] Opt-in flag keeps a spec-incorrect EGFX cap path behind an unverified embedder need — low 🟡 ❓ — crates/ironrdp-server/src/builder.rs
   The PR's own rationale (MS-RDPRFX 2.2.1.3 vs MS-RDPEGFX 2.2.2.13) says applying the core maxUnacknowledgedFrameCount to EGFX frames is wrong, yet the change preserves that behavior behind two new public entry points (RdpServerBuilder::with_egfx_core_frame_ack_limit, RdpServer::set_egfx_core_frame_ack_limit) plus a bool threaded through both builder constructors and RdpServer::new. The only candidate consumer (WRDP) is unconfirmed in #2237. Verified this is a deliberate compatibility tradeoff documented by the author with removal deferred, so it is a design question rather than a defect; if no embedder materializes, a later PR should revert to hard-coded no-cap behavior.
2. [skeptical] New e2e helper repeats TLS and activation scaffolding duplicated across the file — low 🟡 — crates/ironrdp-testsuite-extra/tests/e2e.rs
   egfx_windows re-implements TLS identity setup, client config, connect_client, ActiveStageBuilder wiring, and the PDU pump loop that existing tests (e.g. egfx_moves_onto_the_udp_tunnel_with_soft_sync) already repeat, growing the duplication the file's header FIXME flags. The author deliberately deferred helper extraction to a separate cleanup to avoid touching existing tests, which is the right scoping call here; publish as a non-blocking debt-tracking note.

Push a commit after addressing these findings. If no code change is needed, you may resolve inline threads and comment @github-actions review-ready to request human review.