[Benoît Cortier (CBenoit)]

Document ironrdp-usb's allocation-free caller-owned storage and USB-format-only parsing boundary, and the RDPEUSB adapter's complete request translation.
Keep request IDs, device state, and pending requests in the channel state machines and their backends, outside the translation module.

Disable the USB crate's inline unit-test and doctest targets so its tests use the centralized testsuite.

Co-authored-by: Copilot App 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

Aligns the internal USB model crate with shared testing policy and documents its core-tier role.

Changes:

• Disables publishing and generated library/doctest targets.
• Adds ironrdp-usb to the architecture map.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
crates/ironrdp-usb/Cargo.toml	Applies internal publishing and testing settings.
ARCHITECTURE.md	Documents the USB crate under Core Tier.

[Benoît Cortier (CBenoit)]

Correct. However, I’ll not add fuzzing as part of this PR.

uchouT (@uchouT) is this a follow up you intended to do in the future?

[uchouT (uchouT)]

Yes, in my future plan, and #1690 seems to help.

[Benoît Cortier (CBenoit)]

Oh, right! I’ll go check it

[github-actions]

Docs-only "chore" PR: sets publish = false and disables doctest/test for ironrdp-usb (mirroring the sibling ironrdp-rdpeusb crate's existing settings), and adds ARCHITECTURE.md entries for ironrdp-usb and ironrdp-rdpeusb. The ironrdp-usb entry (no-alloc, USB-only parsing) is verified accurate against the crate's #![no_std] source. The ironrdp-rdpeusb entry, however, describes a "`usb` module" that translates ironrdp-usb operations into RDPEUSB packets; no such module exists (the crate exposes client/io/pdu/server, with pdu::usb_dev handling TS_URB), and ironrdp-rdpeusb does not depend on or import ironrdp-usb at all. Packaging changes are otherwise low-risk and consistent with existing crate conventions.

Protocol analysis: partially_accepted — The handoff correctly scoped itself to a docs/packaging-only diff and correctly flagged the DVC-naming invariant (line 119) as incomplete_evidence since the implementing crate wasn't changed here. However, its 'conforms' assessment for the TS_URB-translation invariant (line 123-125) cites pdu/usb_dev/ts_urb/mod.rs as matching the documentation, without noting that the documentation names a nonexistent '`usb` module' and asserts an ironrdp-usb integration that does not exist in the current source (no dependency, no imports, no module named `usb`). That is a concrete discrepancy between the new architecture text and the actual crate structure that the handoff should have surfaced but did not.

[github-actions]

blocking / medium: This new architectural-invariant text asserts that ironrdp-rdpeusb has a `usb` module that translates protocol-independent `ironrdp-usb` operations into RDPEUSB packets and does not allocate request IDs or track device state. Neither claim matches the current crate: lib.rs only exposes `client`, `io`, `pdu`, and `server` modules (no module named `usb`), and ironrdp-rdpeusb's Cargo.toml has no dependency on ironrdp-usb and no source file imports it anywhere in the crate. The closest existing code is `pdu::usb_dev::ts_urb`, which builds TS_URB PDUs directly rather than adapting an `ironrdp-usb` abstraction. Since ARCHITECTURE.md is designated in this repository's own CLAUDE.md as an authoritative source of truth for both human and AI contributors, checking in a factually incorrect description of the crate's module structure and its (nonexistent) dependency on ironrdp-usb will mislead future readers into believing an integration exists that has not been built yet.

[Benoît Cortier (CBenoit)]

The usb module is added as part of another PR.
And yes, I need to update CLAUDE.md/AGENTS.md file.

[uchouT (uchouT)]

Architectural Invariant: the usb module does not allocate request IDs, track device state, or manage pending-request lifetimes.
Those responsibilities belong to the layer above the channel implementation.

client and server modules in this crate provide channel implementation, including request IDs allocation, device state track, etc. Not sure if this is suitable. Because I think we are talking about crate layer here, not just module layer.

Other parts LGTM

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

Rubber Duck Review — PR #1695

• Reviewed HEAD SHA: 26bcd947913fee91bae5f2d842e51bc69075cab3 (merge-base cec0c1487fad817435a9a70de387d2c57eb9caad; compared against current master 3ca12ee63687980ace30cd6ed59c2ab4f57cf157)

Findings

1. [High] crates/ironrdp-usb/Cargo.toml:14 — publish = false breaks workspace publishing on master

   • Impact: Since this PR branched from cec0c148, ironrdp-usb was made publishable in build(release): make all new crates publishable #2180 (bf5d4a47) and published to crates.io as 0.1.0 (ironrdp-usb-v0.1.0). On current master, publishable crates depend on ironrdp-usb = { path = "../ironrdp-usb", version = "0.1" } (crates/ironrdp-rdpeusb/Cargo.toml:26 as # public, crates/ironrdp-server/Cargo.toml:63 as optional, and crates/ironrdp-rdpeusb-native/Cargo.toml:23), and release-plz.toml:55 explicitly awaits the next ironrdp-usb release before ironrdp-rdpeusb-native can be published. Setting publish = false here will cause Cargo and release-plz to reject packaging/publishing its publishable dependents and stop releasing ironrdp-usb.
   • Correction: Remove publish = false at crates/ironrdp-usb/Cargo.toml:14 and keep only the [lib] test-target settings (doctest = false and test = false at crates/ironrdp-usb/Cargo.toml:16-18).

2. [Medium] ARCHITECTURE.md:119-128 — ironrdp-rdpeusb invariant misattributes channel state-machine responsibilities and conflicts with master

   • Impact:
     1. Lines 127–128 state that allocating request IDs, tracking device state, and managing pending-request lifetimes "belong to the layer above the channel implementation." However, crates/ironrdp-rdpeusb is the channel implementation, and its server and client modules own those responsibilities directly: UrbdrcDeviceServer (crates/ironrdp-rdpeusb/src/server.rs:71-85, 212-225) allocates RequestIds via RequestIdAllocator, tracks per-device channel state (DeviceState), and tracks in-flight requests in pending_io: BTreeMap<RequestId, Pending>, while UrbdrcDeviceClient (crates/ironrdp-rdpeusb/src/client.rs:289-298) tracks ready_for_io and pending_io. Those responsibilities live in the channel state machines (client/server) and caller backends above the usb translation module—not above the channel implementation.
     2. On master, feat: redirect USB devices over URBDRC with nusb and RemoteFX USB backends #2181 (35247071) already added a #### [crates/ironrdp-rdpeusb](./crates/ironrdp-rdpeusb) section (ARCHITECTURE.md:113-116), creating a merge conflict with this block (and crates/ironrdp-rdpeusb/src/usb.rs landed on master in feat(rdpeusb): add the usb translation module #1683 / b5c1d4da after this branch's merge-base).
   • Correction: Rebase onto master, merge these invariants into the existing crates/ironrdp-rdpeusb entry, and attribute request-ID allocation, per-device state tracking, and pending-request lifetimes to the channel state machines (client/server) and callers/backends above the usb module rather than "the layer above the channel implementation."

3. [Low] ARCHITECTURE.md:123-124 — Sentence split leaves The generated packet as the subject of maps USBD completions back

   • Impact: Commit fa1d2260 split the original sentence from 9736d615 into two, leaving , and maps USBD completions back. attached to The generated packet ("The generated packet keeps its TS_URB payload, URB function code, transfer envelope, flags, and buffer shape consistent, and maps USBD completions back."). The usb module—not the generated request packet—maps USBD completions back.
   • Correction: Move and maps USBD completions back back to the first sentence:

     **Architectural Invariant**: the `usb` module translates protocol-independent `ironrdp-usb` operations and descriptor semantics into complete backend-facing RDPEUSB packets and maps USBD completions back.
     The generated packet keeps its `TS_URB` payload, URB function code, transfer envelope, flags, and buffer shape consistent.

4. [Low] ARCHITECTURE.md:26, 77-86 — Core Tier fuzzing invariant vs. ironrdp-usb fuzz coverage (follow-up note)

   • Impact: Listing crates/ironrdp-usb under ### Core Tier subjects it to ARCHITECTURE.md:26 ("all these crates must be fuzzed"). Note that test(core): add a rdpeusb (URBDRC) decode fuzz target #1690 (04320a0c on master, referenced in the review thread on line 77) added rdpeusb_decode, which only exercises ironrdp_rdpeusb::pdu client-to-server decoders (UrbdrcClientControlPdu and UrbdrcClientDevicePdu<Raw>) and does not call any ironrdp-usb parsers (DeviceDescriptor::parse, ConfigurationDescriptorSet::parse/validate, SetupPacket::from_bytes).
   • Correction: Track a follow-up fuzz oracle/target for ironrdp-usb descriptor parsing in crates/ironrdp-fuzzing and fuzz/.

Review Limitations & Skipped Passes

• Passes executed: code-compressor, docs-compressor, prose-verifier, skeptical-reviewer.
• Passes skipped: protocol-reviewer (the diff touches only ARCHITECTURE.md and crates/ironrdp-usb/Cargo.toml and introduces no protocol wire-format or state-machine code changes).
• Limitations: Static verification of the diff against its merge-base (cec0c148), current master (3ca12ee6), and the ironrdp-usb / ironrdp-rdpeusb sources; no test/build execution was needed for this documentation and manifest-only diff.

Note

LLM-assisted content (no human feedback).

[github-actions]

🟢 Documentation-only plus a harmless manifest tweak: ARCHITECTURE.md documents the ironrdp-usb crate boundaries (no allocation, USB-format-only parsing) and two ironrdp-rdpeusb usb-module invariants; crates/ironrdp-usb/Cargo.toml adds [lib] doctest=false and test=false. Verified against the head: ironrdp-usb has zero dependencies and no inline unit tests (its tests live in ironrdp-testsuite-core tests/usb.rs and tests/rdpeusb/usb.rs), so disabling the lib test/doctest targets has no build or dependency-graph effect and matches the workspace convention (e.g., ironrdp-pdu). The rdpeusb invariants match the code: usb.rs contains no RequestId handling, while server.rs owns RequestIdAllocator and client.rs owns pending_io, exactly as documented and consistent with the review-thread wording resolution. No correctness, protocol, dependency, or maintainability issues found; the validated specialist aggregate reported no candidates.

[Benoît Cortier (CBenoit)]

Diff and validation for head 6059e36d66e88fcbaacb4b192e1ce8be5806a086.

• git diff --numstat against 30bf9df1738d90b14360d03305a0f5ad065225ff reports 2 files, 21 insertions, and 0 deletions: 0 production/runtime lines, 0 test lines, and 21 other lines in ARCHITECTURE.md and crates/ironrdp-usb/Cargo.toml.
• The lack of direct USB parser fuzz coverage is pre-existing and remains unaddressed; this change adds no new UNTESTED: property.
• The exact-head CI run passed.
• The exact-head AI automated review check completed successfully with a validated no-findings outcome.
• The current needs-review label indicates human review is next.

Note

LLM-assisted content (no human feedback).