[se-wo]

MS-RDPBCGR 2.2.12.1 requires the pduSource field of the Monitor Layout PDU to be zero, but the acceptor wrapped it with the I/O channel ID like the finalization PDUs. The acceptor now passes zero for this PDU only; the share data helper takes the pduSource it writes instead of naming it after the I/O channel.

[se-wo]

Lines changed:

	Added	Removed
Production (ironrdp-acceptor)	4	3
Tests (ironrdp-testsuite-core)	31	0

Note

Human-tuned, LLM-assisted content.

[github-actions]

🟢 Minimal, spec-correct fix: the acceptor now sends the Monitor Layout PDU with pduSource = 0 per MS-RDPBCGR 2.2.12.1 instead of the I/O channel ID, and wrap_share_data's parameter is renamed to pdu_source since the value is no longer always the I/O channel. The zero override applies only to the Share Control Header; the outer MCS Send Data Indication still targets the I/O channel. Other call sites are unchanged: the existing Set Error Info call already passed 0, and the finalization PDUs (synchronize/cooperate/control/font map) correctly retain the I/O channel ID. The new wire-level test drives the real acceptor through multitransport setup to MonitorLayoutSend, decodes the emitted MCS Send Data Indication with existing helpers, and asserts pdu_source == 0, so it would fail on pre-fix code. All five specialists reported valid with no findings; independent inspection found no correctness, protocol, or maintainability issues.

[Marc-André Moreau (mamoreau-devolutions)]

Diff line counts:

	Added	Removed
Production (ironrdp-acceptor)	4	3
Tests (ironrdp-testsuite-core, ironrdp-testsuite-extra)	37	3
Other	0	0

Note

LLM-assisted content (no human feedback).

[github-actions]

The PR correctly sends pduSource=0 for the server Monitor Layout PDU per MS-RDPBCGR 2.2.12.1, with a minimal wrap_share_data parameter rename that keeps the four finalization call sites passing the I/O channel; the new wire-level regression test matches the acceptor's real state transitions and gating on SUPPORT_MONITOR_LAYOUT_PDU. The production fix and test are sound. Two low-severity notes are published: the e2e.rs GfxContext hunk is a compile-required adaptation to a GfxServerFactory trait change absent from this PR's file list (stacked-work scope concern), and the test can use Option::get_or_insert_default(). The speculative spec-section doubt about the oracle pin is rejected because the section citation is corroborated by pre-existing in-repo references.

1. [skeptical] e2e.rs GfxContext adaptation compiles against a trait change outside this PR's file list — low 🟡 ❓ — crates/ironrdp-testsuite-extra/tests/e2e.rs
   WindowGfxFactory::build_gfx_handler now takes a GfxContext that is threaded through build_server_with_handle. This is compile-required, not optional: ironrdp-server/src/gfx.rs (absent from changed-files.txt) already defines GfxContext and requires build_gfx_handler(&self, ctx: GfxContext), and the untouched TestGfxFactory (e2e.rs:1957) already conforms. The hunk must stay, but its provenance cannot be resolved from the evidence: either the PR stacks on an unmerged GfxServerFactory signature change belonging to a separate PR, or the changed-file list is incomplete relative to the branch. Since the PR body mentions only the zero-pduSource fix, the author should justify the hunk in the description or move it to the PR that owns the trait change. Impact limited to review accuracy and test maintenance.

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]

[code-compressor] Use Option::get_or_insert_default() instead of get_or_insert_with(ClientEarlyCapabilityFlags::empty) — low 🟡 — The new test inserts SUPPORT_MONITOR_LAYOUT_PDU via .get_or_insert_with(ClientEarlyCapabilityFlags::empty). ClientEarlyCapabilityFlags is a bitflags 2.x generated type (ironrdp-pdu/src/gcc/core_data/client.rs:641-660) and the workspace pins bitflags 2.13.2 (Cargo.lock), which implements Default for generated flag types as the empty set, so .get_or_insert_default() is behavior-identical and shorter. Cosmetic test-only compression; no correctness impact.

[github-actions]

Verified PR #2230: the acceptor now sends a zero pduSource in the Monitor Layout PDU per MS-RDPBCGR 2.2.12.1, with wrap_share_data renamed to take an explicit pdu_source. Confirmed both call sites: Monitor Layout passes 0 and the access-denied path already passed 0, so no other behavior changes. The added wire-decode regression test asserts pduSource == 0 and follows the repository's test conventions. The change is correct, minimal, and well-tested. The only validated finding is a low-severity rust-style nit: the touched wrap_share_data signature still lacks a doc comment even though STYLE.md's Documentation section marks undocumented items as BAD, and the renamed pdu_source parameter now carries a per-PDU contract (0 for Monitor Layout per 2.2.12.1, I/O channel ID for other share data) that deserves a reference-style spec link. Accepted that nit unchanged as a non-blocking follow-up candidate.

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]

[rust-style] Touched wrap_share_data signature has no doc comment — low 🟡 — STYLE.md 'Documentation > Doc comments should link to reference documents' flags an item without a doc comment as BAD. The PR rewrote the signature, renaming io_channel_id to pdu_source precisely because the value is now caller-dependent (0 for the Monitor Layout PDU per MS-RDPBCGR 2.2.12.1, I/O channel ID for other share data PDUs). A doc comment on the touched signature stating that contract with a reference-style spec link would satisfy the style rule; non-blocking nit suitable for a follow-up.