Conversation
src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt
Outdated
Show resolved
Hide resolved
src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbReversePortRule.cs
Outdated
Show resolved
Hide resolved
src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt
Outdated
Show resolved
Hide resolved
b4bba1e to
3075d2c
Compare
…tRule - Add AdbProtocol enum (Tcp, LocalAbstract, LocalReserved, LocalFilesystem) - Add AdbPortSpec record with TryParse/ToSocketSpec for typed port+protocol - Rename AdbReversePortRule → AdbPortRule using AdbPortSpec (not raw strings) - Add AdbPortSpec overloads for ReversePortAsync and RemoveReversePortAsync - Int convenience overloads delegate through AdbPortSpec - ParseReverseListOutput returns typed AdbPortRule via AdbPortSpec.TryParse - Update PublicAPI surface for both TFMs - Add 18 new tests for AdbPortSpec parsing, serialization, and validation Addresses @jonathanpeppers review feedback on PR #305. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tRule - Add AdbProtocol enum (Tcp, LocalAbstract, LocalReserved, LocalFilesystem) - Add AdbPortSpec record with TryParse/ToSocketSpec for typed port+protocol - Rename AdbReversePortRule → AdbPortRule using AdbPortSpec (not raw strings) - Add AdbPortSpec overloads for ReversePortAsync and RemoveReversePortAsync - Int convenience overloads delegate through AdbPortSpec - ParseReverseListOutput returns typed AdbPortRule via AdbPortSpec.TryParse - Update PublicAPI surface for both TFMs - Add 18 new tests for AdbPortSpec parsing, serialization, and validation Addresses @jonathanpeppers review feedback on PR #305. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
40512c4 to
798b585
Compare
…loads, stdout in errors Add three rules learned from PR #305 review feedback: - Prefer strongly-typed APIs (enum+record) over string parameters - Avoid convenience overloads (string, int, typed) — pick one - Include stdout in ProcessUtils.ThrowIfFailed error diagnostics Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds shared adb reverse (reverse port forwarding) support to Xamarin.Android.Tools.AndroidSdk via AdbRunner, enabling downstream tooling (e.g., MAUI DevTools CLI) to manage hot-reload tunnels without ServiceHub.
Changes:
- Introduces strongly-typed port forwarding models (
AdbProtocol,AdbPortSpec,AdbPortRule). - Adds
AdbRunnerAPIs for reverse port forwarding management (reverse,--remove,--remove-all,--list) plus list-output parsing. - Extends unit tests to cover parsing,
AdbPortSpecparsing/formatting, and parameter validation; updates PublicAPI unshipped files.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs | Adds unit tests for reverse list parsing, AdbPortSpec parsing/formatting, and new API parameter validation. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs | Adds reverse port forwarding methods and adb reverse --list output parsing helper. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbProtocol.cs | Adds AdbProtocol enum for socket spec protocol kinds. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortSpec.cs | Adds AdbPortSpec record with ToSocketSpec() and TryParse(). |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortRule.cs | Adds AdbPortRule record to represent a forwarding rule (remote/local). |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Declares newly added public API surface for netstandard2.0. |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Declares newly added public API surface for net10.0. |
You can also share your feedback on Copilot code review. Take the survey.
There was a problem hiding this comment.
Pull request overview
Adds shared adb reverse (reverse port forwarding) support to Xamarin.Android.Tools.AndroidSdk via AdbRunner, intended to be consumed by MAUI DevTools CLI (and later VS Code / VS) to manage Hot Reload tunnels without ServiceHub.
Changes:
- Introduces strongly-typed reverse port forwarding models (
AdbProtocol,AdbPortSpec,AdbPortRule) and adds reverse operations toAdbRunner. - Implements parsing for
adb reverse --listoutput. - Adds unit tests covering parsing, round-tripping, value equality, and parameter validation; updates PublicAPI unshipped files for both TFMs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs | Adds unit tests for reverse-list parsing, AdbPortSpec parsing/formatting, AdbPortRule equality, and new API argument validation. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs | Adds ReversePortAsync, removal APIs, list API, and adb reverse --list output parser. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbProtocol.cs | Adds AdbProtocol enum for socket spec protocol kinds. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortSpec.cs | Adds AdbPortSpec record with ToSocketSpec() and TryParse(). |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortRule.cs | Adds AdbPortRule record to represent a reverse/forward rule pair. |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Declares new public API surface for netstandard2.0. |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Declares new public API surface for net10.0. |
Add three rules learned from PR #305 review feedback: - Prefer strongly-typed APIs (enum+record) over string parameters - Avoid convenience overloads (string, int, typed) — pick one - Include stdout in ProcessUtils.ThrowIfFailed error diagnostics Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
There was a problem hiding this comment.
Pull request overview
Adds reverse port forwarding support to Xamarin.Android.Tools.AndroidSdk’s AdbRunner, enabling downstream tools (e.g., MAUI DevTools CLI) to manage adb reverse tunnels via the adb CLI.
Changes:
- Added new public types (
AdbProtocol,AdbPortSpec,AdbPortRule) for strongly-typed reverse/forward socket specs and rules. - Added
AdbRunnerAPIs foradb reverseoperations: add rule, remove rule, remove all, and list rules (with list output parsing). - Added unit tests covering list parsing,
AdbPortSpecparsing/formatting, and parameter validation.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Xamarin.Android.Tools.AndroidSdk-Tests/AdbRunnerTests.cs | Adds unit tests for reverse list parsing, AdbPortSpec/AdbPortRule, and parameter validation. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbRunner.cs | Implements adb reverse command wrappers and a parser for adb reverse --list output. |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbProtocol.cs | Introduces AdbProtocol enum (currently TCP only). |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortSpec.cs | Introduces AdbPortSpec record with ToSocketSpec() + TryParse(). |
| src/Xamarin.Android.Tools.AndroidSdk/Runners/AdbPortRule.cs | Introduces AdbPortRule record representing a reverse/forward rule. |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/netstandard2.0/PublicAPI.Unshipped.txt | Declares new public API surface for netstandard2.0. |
| src/Xamarin.Android.Tools.AndroidSdk/PublicAPI/net10.0/PublicAPI.Unshipped.txt | Declares new public API surface for net10.0. |
|
|
||
| var rules = AdbRunner.ParseReverseListOutput (output); | ||
|
|
||
| // localabstract:chrome_devtools_remote has a non-numeric port, so it is skipped |
There was a problem hiding this comment.
This comment is a bit misleading: localabstract:... isn’t a “non-numeric port” (it’s a named socket). Consider rewording to clarify that the spec is skipped because AdbPortSpec.TryParse only supports numeric tcp:<port> at the moment.
| // localabstract:chrome_devtools_remote has a non-numeric port, so it is skipped | |
| // localabstract:chrome_devtools_remote is a named localabstract socket, not a tcp:<port>, so TryParse skips it |
Add ReversePortAsync, RemoveReversePortAsync, RemoveAllReversePortsAsync, and ListReversePortsAsync methods to AdbRunner for managing reverse port forwarding rules. These APIs enable the MAUI DevTools CLI to manage hot-reload tunnels without going through ServiceHub. New type AdbReversePortRule represents entries from 'adb reverse --list'. Internal ParseReverseListOutput handles parsing the output format. Includes 14 new tests covering parsing and parameter validation. Closes #303 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Based on multi-model code review (GPT-5.1 + Gemini-3-Pro): - Convert AdbReversePortRule from class to positional record for value equality and concise construction - Add string socket-spec overloads for ReversePortAsync and RemoveReversePortAsync to support non-TCP protocols (e.g., localabstract:, localfilesystem:) matching existing patterns in ClientTools.Platform and vscode-maui - Extract ValidatePort helper for consistent validation - Int overloads now delegate to string overloads as convenience wrappers - Add 8 new tests: NonTcpSpecs, WindowsLineEndings, ValueEquality, Deconstruct, ToString, and string overload validation tests (total: 25 reverse-port tests) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tRule - Add AdbProtocol enum (Tcp, LocalAbstract, LocalReserved, LocalFilesystem) - Add AdbPortSpec record with TryParse/ToSocketSpec for typed port+protocol - Rename AdbReversePortRule → AdbPortRule using AdbPortSpec (not raw strings) - Add AdbPortSpec overloads for ReversePortAsync and RemoveReversePortAsync - Int convenience overloads delegate through AdbPortSpec - ParseReverseListOutput returns typed AdbPortRule via AdbPortSpec.TryParse - Update PublicAPI surface for both TFMs - Add 18 new tests for AdbPortSpec parsing, serialization, and validation Addresses @jonathanpeppers review feedback on PR #305. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Remove string and int convenience overloads per @jonathanpeppers review. The strongly-typed AdbPortSpec is now the only API surface for ReversePortAsync and RemoveReversePortAsync. Removed ValidatePort helper and RS0026/RS0027 suppressions (no longer needed). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Pass stdout to ThrowIfFailed so failures include any diagnostics written to stdout, not just stderr. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ble TryParse - Split AdbPortSpec into its own file (one public type per file convention) - AdbPortRule.cs now contains only AdbPortRule record - ToSocketSpec() throws ArgumentOutOfRangeException for unknown AdbProtocol values - TryParse parameter changed from string to string? to match null-handling behavior - Remove unused 'using System' from AdbPortRule.cs - Update PublicAPI files for TryParse signature change Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…range
- Replace null! with null in tests (no nullable context in test project)
- Replace socketSpec! with property pattern (is not { Length: > 0 }) for null flow
- Add port range validation (1-65535) in ReversePortAsync and RemoveReversePortAsync
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- ParseReverseListOutput now splits on all whitespace (tabs, spaces) instead of single space - Added ParseReverseListOutput_TabSeparated test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove LocalAbstract, LocalReserved, LocalFilesystem from AdbProtocol enum (add later when needed) - Simplify ToSocketSpec and TryParse to only handle TCP - Add ToSocketSpec unit tests: HighPort, LowPort, InvalidProtocol_Throws - Add TryParse test: NonTcpProtocol_ReturnsNull - Remove 3 non-TCP TryParse tests - Update PublicAPI files (remove 3 enum entries per TFM) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
8e65808 to
733b982
Compare
Summary
Add reverse port forwarding APIs to
AdbRunner, enabling the MAUI DevTools CLI to manage hot-reload tunnels without going through ServiceHub.Closes #303
Context
Reverse port forwarding (
adb reverse) is used across three separate codebases today, each with its own implementation:ClientTools.Platform) — raw ADB wire protocol viaTransportRunCommandWithStatusvscode-maui) — ServiceHub JSON-RPC to C#AdbServerEach IDE reimplements the same ADB operations. This PR adds a shared, CLI-based implementation in
android-tools.New API Surface
Types
AdbRunnerMethodsReversePortAsync(serial, AdbPortSpec remote, AdbPortSpec local, ct)adb reverse <remote> <local>RemoveReversePortAsync(serial, AdbPortSpec remote, ct)adb reverse --remove <remote>RemoveAllReversePortsAsync(serial, ct)adb reverse --remove-allListReversePortsAsync(serial, ct)adb reverse --listUsage
Key Design Decisions
AdbPortSpec— enum + int instead of raw strings like"tcp:5000", per @jonathanpeppers reviewAdbPortSpec, no string or int convenience overloadsAdbPortRulerecord — value equality, structural matching on list outputListReversePortsAsyncpasses both stdout and stderr toThrowIfFailedTests
22 tests, all passing:
ParseReverseListOutput: single/multiple rules, empty output, non-reverse lines, malformed lines, non-TCP specs, Windows line endingsAdbPortSpec.TryParse: valid TCP/LocalAbstract/LocalReserved/LocalFilesystem, null, empty, no colon, non-numeric port, zero port, unknown protocolArgumentNullException)RemoveAllReversePortsAsync: empty serial validationListReversePortsAsync: empty serial validationReview Feedback Addressed
"tcp:5000"AdbPortSpec(AdbProtocol, int)— single overloadAdbPortSpecadb forwardtoo — rename toAdbPortRuleAdbReversePortRuletoAdbPortRuleListReversePortsAsyncThrowIfFailedMigration Path
Related