[irvingouj@Devolutions (irvingoujAtDevolution)]

The Diplomat version we are moving to no longer turns get_*/set_* methods into C# properties from a config prefix. Each accessor needs its own attribute, or the property silently disappears from the bindings.

Annotate every accessor that the current bindings expose as a property: 86 getters and 19 setters. The nine &mut self getters are left as methods, because the new generator refuses getters that take &mut self.

The pinned 0.7 macro strips #[diplomat::attr], so this changes no generated code.

Co-Authored-By: Claude Opus 5.5 (1M context) noreply@anthropic.com

[Copilot]

🟢 Approval recommended

The changes are limited to matching accessor attributes, with no implementation changes or unresolved findings.

No open issues 🎉

What changed in this PR

Prepares IronRDP’s FFI accessors for explicit C# property generation in the upcoming Diplomat upgrade.

Changes:

• Adds explicit attributes to 86 getters and 19 setters.
• Leaves nine mutable getters as methods.
• Preserves implementations and current generated bindings.

File	Description
ffi/​src/​utils/​mod.rs	Annotates collection sizes.
ffi/​src/​session/​mod.rs	Annotates session output and pointer accessors.
ffi/​src/​session/​image.rs	Annotates image data and dimensions.
ffi/​src/​rdcleanpath.rs	Annotates response and error properties.
ffi/​src/​pdu.rs	Annotates buffer, rectangle, and PDU properties.
ffi/​src/​graphics.rs	Annotates pointer properties.
ffi/​src/​error.rs	Annotates error kind.
ffi/​src/​dvc/​mod.rs	Annotates message sink.
ffi/​src/​dvc/​dvc_pipe_proxy_message_queue.rs	Annotates channel ID and queue sink.
ffi/​src/​credssp/​network.rs	Annotates authentication and network properties.
ffi/​src/​connector/​state.rs	Annotates connector state type.
ffi/​src/​connector/​result.rs	Annotates connection result properties.
ffi/​src/​connector/​mod.rs	Annotates dynamic state and name.
ffi/​src/​connector/​config.rs	Annotates configuration accessors.
ffi/​src/​connector/​activation.rs	Annotates activation state properties.
ffi/​src/​clipboard/​message.rs	Annotates clipboard message properties.

_{Review effort: Balanced}

[github-actions]

Annotation-only PR adding #[diplomat::attr(auto, getter/setter)] to 105 existing FFI accessors across 16 changed files; no Rust logic is modified. Independently verified both skeptical candidates: (1) the no-op claim (pinned CBenoit/diplomat@6dc806e fork's macro strips #[diplomat::attr]) has no regenerated bindings, snapshots, or CI evidence in the repo, so generated-code neutrality is unconfirmed — if wrong, the C# property surface could silently change; (2) four writeable-parameter methods (DynState::get_name, NetworkRequest::get_url, RDCleanPathPdu::get_server_addr/get_error_message) are annotated as getters although the target generator's rules for writeable-parameter getters are unestablished — inert today, deferred upgrade friction. Both published as open questions; no additional independent findings.

1. [skeptical] No-op claim (pinned diplomat macro strips #[diplomat::attr]) is asserted without evidence — medium 🟠 ❓ — ffi/src/connector/config.rs
   The PR's safety rests on the claim that the pinned 0.7 fork's macro strips #[diplomat::attr], so no generated code changes. The diff contains only attribute additions — no regenerated bindings, snapshots, or CI checks — and root Cargo.toml pins CBenoit/diplomat@6dc806e patching ffi's diplomat = "0.7", so the claim cannot be verified from the repo. If the pinned rev instead honors the attribute, all 105 annotations could silently reshape the public C# property surface (e.g., getMessageType -> message_type), breaking downstream consumers such as ironrdp-activex. Missing context: a regenerated-binding diff or CI confirmation that the attribute is inert at the pinned rev.

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]

Mechanical FFI change adding 105 #[diplomat::attr(auto, getter/setter = ...)] annotations across 16 files so get_*/set_* accessors remain C# properties under the upcoming Diplomat generator. Annotation names were independently checked in the head tree and match the stripped get_/set_ prefixes, so generated property names are unchanged versus the old prefix scheme; no logic, signature, or visibility changes are bundled. The one residual issue is the PR's central no-op claim: Cargo.toml pins Diplomat to a fork revision (rev 6dc806e) whose macro treatment of #[diplomat::attr] cannot be verified from repository evidence, so whether the change is inert under the pinned macro rests on out-of-band testimony from a follow-up PR. Impact if the claim is wrong is bounded to a compile failure or bindings drift check rather than silent FFI behavior change, so severity is low and the item is flagged as an open question.

1. [skeptical] No-op claim under the pinned Diplomat revision is unverifiable from repository evidence — low 🟡 ❓ — ffi/src/connector/config.rs
   The PR's central safety argument is that the pinned Diplomat macro strips #[diplomat::attr], so no generated code changes today. Cargo.toml pins a fork revision (CBenoit/diplomat rev 6dc806e), whose macro behavior cannot be inspected from the repository, and the supporting evidence (regenerated bindings passing a drift check) is author testimony from a separate PR (#2165). If the claim is wrong, impact is bounded: either the crate fails to compile or a bindings drift check flags it, rather than silently changing FFI behavior. The annotation coverage and name consistency were verified in the head tree, so this does not block the mechanical change, but the no-op conclusion rests on out-of-band evidence.

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 2163 annotates 105 existing FFI get_*/set_* accessors across 16 changed files with #[diplomat::attr(auto, getter/setter = "...")] to preserve C# property generation under an upcoming Diplomat upgrade. Independent inspection confirms the diff is purely additive attribute lines with no logic changes, and ffi/Cargo.toml pins diplomat 0.7, which strips these attributes, so no generated code or CI output changes in this PR. Coverage claims (86 getters, 19 setters, 9 &mut self getters left as methods) are consistent with the diff and the PR body, and the PR comment reports regenerated bindings in #2165 pass the drift check. The one residual issue is that nothing in-repo validates the annotations: the pinned 0.7 macro strips them, so the bindings CI check exercises none of them, and a future property-eligible accessor added without an attribute would silently lose its property at the next generator bump. This is a low-severity maintainability gap; the annotations themselves are correct an…

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]

[skeptical] Annotations are inert under pinned Diplomat and no in-repo mechanism validates coverage or naming — low 🟡 — The PR adds 105 per-accessor #[diplomat::attr(auto, ...)] annotations, but ffi/Cargo.toml pins diplomat 0.7, whose macro strips these attributes, so the CI regeneration check cannot detect a wrong or missing attribute name and generates identical output. Correctness is established only by the follow-up regeneration in #2165 cited in a PR comment. Nothing enforces the convention going forward: a future get_/set_ accessor added without an attribute will silently lose its property binding at the next generator bump, recreating the regression this PR fixes. Consider landing with the generator bump or adding an xtask check that fails when a property-eligible accessor lacks the attribute. Low severity: the annotations are correct today and the risk only materializes at a future upgrade.