[irvingouj@Devolutions (irvingoujAtDevolution)]

The Diplomat version we are moving to makes IDisposable opt-in. Without the attribute, a generated wrapper only has a finalizer.

Mark the types that ffi/INTENT.md says are disposable:

• operating-system resources: ActiveStage, whose DVC pipe proxies own named-pipe worker threads, and WinCliprdr, which owns the clipboard window
• large buffers the garbage collector cannot see: ActiveStageOutputIterator, VecU8, WriteBuf, and the clipboard payloads ClipboardMessage, ClipboardSvcMessage, FormatDataResponse and FfiFileContentsResponse

DvcPipeProxyMessage stays unmarked: sending one moves its payload out, so nothing large is left behind.

Also mark CredsspProcessGenerator. None of the three conditions names it, but it holds &mut of its CredsspSequence, and the sequence rejects every other call until the generator is released. Releasing it costs nothing.

DecodedImage, DecodedPointer and ActiveStageOutput also qualify, but they hand out borrows that outlive the call, which the new generator refuses for disposable types; a later change returns owned data so they can be marked too.

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

[github-actions]

This pull request may overlap with #2167.

Both pull requests work on the same area: the disposal semantics of opaque types in the .NET FFI layer. PR 2167 made DecodedImage, DecodedPointer and ActiveStageOutput IDisposable as part of returning owned data; this pull request adds manually_disposable dotnet attributes to ten other opaque types (clipboard, CredSSP, PDU, session and VecU8 wrappers) under the same ffi/INTENT.md disposal policy. Shared scope is the ffi crate's disposable-type surface for .NET, not identical changes.

This notice is advisory only. Automated review continues as usual, and how these pull requests relate is for maintainers and authors to decide.

Note

LLM-assisted content (no human feedback).

[Copilot]

🟢 Approval recommended

The narrowly scoped annotations match the intended ownership boundaries, with no identified blocking issues.

No open issues 🎉

What changed in this PR

Prepares the FFI bridge for opt-in .NET IDisposable support in the upcoming Diplomat upgrade.

Changes:

• Marks resource-owning and large-buffer wrappers as manually disposable.
• Marks CredsspProcessGenerator for explicit release of its exclusive borrow.
• Leaves runtime logic unchanged.

File	Description
ffi/​src/​utils/​mod.rs	Marks VecU8 disposable.
ffi/​src/​session/​mod.rs	Marks active-stage and output-iterator wrappers disposable.
ffi/​src/​pdu.rs	Marks WriteBuf disposable.
ffi/​src/​credssp/​network.rs	Marks the CredSSP generator disposable.
ffi/​src/​clipboard/​windows.rs	Marks the Windows clipboard owner disposable.
ffi/​src/​clipboard/​mod.rs	Marks clipboard service messages disposable.
ffi/​src/​clipboard/​message.rs	Marks clipboard payload wrappers disposable.

_{Review effort: Balanced}

[github-actions]

The PR adds #[diplomat::attr(dotnet, manually_disposable)] to nine opaque FFI types in seven files, matching ffi/INTENT.md's disposal policy for OS-owned resources (ActiveStage, WinCliprdr), large GC-invisible buffers (VecU8, WriteBuf, clipboard payloads, ActiveStageOutputIterator), and leaves borrow-returning types unmarked. No generated bindings or toolchain files change. Two low-severity concerns are published: CredsspProcessGenerator is marked despite meeting none of the policy conditions (CredsspSequence itself is not disposable), and the change's correctness rests on pinned-fork toolchain behavior that cannot be verified in-repo. The proposed GeneratorState inconsistency is rejected: its Suspended payload is a small, transient SSPI auth token, not a large GC-invisible resource.

Reduced coverage: optional reviewer code-compressor was unavailable.

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 2164 adds ten declarative #[diplomat::attr(dotnet, manually_disposable)] attributes across seven ffi bridge files (ActiveStage, ActiveStageOutputIterator, WinCliprdr, ClipboardSvcMessage, ClipboardMessage, FormatDataResponse, FfiFileContentsResponse, CredsspProcessGenerator, WriteBuf, VecU8). Verified against pr-head: the diff applies cleanly and matches the head sources exactly, with no code changes beyond attribute placement. The pinned diplomat 0.7 bridge macro strips #[diplomat::attr], so no generated bindings change today; the attributes only take effect when the toolchain is upgraded. Each marking is consistent with ffi/INTENT.md's disposal policy: OS-resource owners (WinCliprdr's clipboard window, ActiveStage's pipe workers) and GC-invisible large buffers (VecU8, WriteBuf, clipboard payloads) qualify, and the documented exceptions (DvcPipeProxyMessage consumes its payload; DecodedImage/DecodedPointer/ActiveStageOutput hand out borrows the new generator rejects) are reasonabl…

[github-actions]

Attribute-only PR adding #[diplomat::attr(dotnet, manually_disposable)] to ten opaque FFI types ahead of a Diplomat upgrade. Verified inert today (pinned 0.7 bridge macro strips diplomat::attr), so no runtime behavior change. Ten of the marks align with ffi/INTENT.md's disposable conditions. Two issues survive independent review: DvcPipeProxyMessage is left unmarked on a rationale that only holds on the successful-send path, since send_dvc_pipe_proxy_message drains the payload via mem::take only when sent, mirroring the marked FormatDataResponse; and CredsspProcessGenerator is marked despite meeting none of INTENT.md's conditions, resting on an unverifiable claim that the new backend unlocks the borrowed CredsspSequence on Dispose.

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] Disposable set inconsistent: DvcPipeProxyMessage holds large payloads until sent but is left unmarked — medium 🟠 — FormatDataResponse is newly marked because it holds a large buffer until submit_clipboard_format_data takes it. DvcPipeProxyMessage follows the same pattern: send_dvc_pipe_proxy_message drains its Vec<SvcMessage> payload via core::mem::take only when sent, so a message fetched via next_message/next_message_blocking and dropped unsent keeps GC-invisible DVC payloads until the finalizer runs. The PR's 'sending moves the payload out' omission rationale holds only on the success path, contradicting INTENT.md condition 1. Impact is deferred because the pinned 0.7 macro strips the attribute, but this PR freezes the disposable set ahead of the upgrade, so the omission would persist and exclude large native buffers from deterministic disposal.

[github-actions]

[skeptical] CredsspProcessGenerator marked despite meeting none of INTENT.md's conditions, with unverified benefit — low 🟡 ❓ — INTENT.md defaults to no IDisposable and forbids adding it defensively. The PR concedes none of the three conditions names CredsspProcessGenerator, justifying the mark only by the claim that disposing it releases the CredsspSequence it borrows (created via process_ts_request taking &'a mut self). That mechanism cannot be confirmed here: the wrapper holds a Rust borrow, and whether the new Diplomat .NET backend actually unlocks the sequence on Dispose, versus merely suppressing an absent finalizer, is unverified. Since the pinned 0.7 macro strips the attribute, the deviation has no observable effect in this PR, but it documents a policy exception whose justification rests on behavior outside the reviewed code — confirm the bridge behavior or drop the mark until the upgrade lands.