Improve Android DevFlow port forwarding by Redth · Pull Request #273 · dotnet/maui-labs

Redth · 2026-05-12T14:06:46Z

Android DevFlow connectivity depended on manual ADB setup, and the existing docs had conflicting guidance about when to use adb reverse versus adb forward. This makes the CLI prepare and diagnose the Android forwarding path as part of normal broker discovery and direct fallback behavior.

Changes

Add a testable Android DevFlow forwarding helper that selects a deterministic device, checks existing ADB mappings, and repairs broker reverse plus agent forwards when requested.
Extend broker resolution to return full agent metadata so Android agents can be identified before connecting to their HTTP port.
Wire forwarding into maui devflow list, maui devflow wait, diagnose output, auto-resolved agent commands, and MCP agent sessions.
Add --android-device <serial> and ANDROID_SERIAL support for multi-device scenarios instead of guessing.
Update broker docs and the bundled DevFlow debug skill to document adb reverse tcp:19223 tcp:19223 for app-to-broker and adb forward tcp:<port> tcp:<port> for CLI-to-agent.

Validation

dotnet test src/Cli/Microsoft.Maui.Cli.UnitTests/Microsoft.Maui.Cli.UnitTests.csproj --no-restore --logger "console;verbosity=minimal"

github-actions

Expert Code Review: 4 findings posted inline (3 moderate, 1 minor). See the lean summary comment for full methodology, discarded findings, and CI status.

Generated by Expert Code Review (auto) for issue #273 · ● 23.8M

Carry over the more defensive adb forwarding patterns from PR #273: - Scope `adb forward --list` to the active device with `-s <serial>` so parsing tolerates both the 2-column `<local> <remote>` and 3-column `<serial> <local> <remote>` output shapes regardless of how many devices are attached. - Pre-emptively run `adb forward --remove tcp:<agent>` before re-adding the mapping so a stale entry left by a prior adb-server crash cannot block recovery. - After re-establishing the forward, re-list and verify the mapping is visible. `adb forward` can return 0 while the daemon is in an inconsistent state, and silently passing through amplified into a ~4 minute opaque retry storm in the agent client. - Capture both `adb -s <serial> forward --list` and `reverse --list` in the crash diagnostics bundle so port-forwarding state is visible when triaging app-loss artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* Harden Android DevFlow integration tests Add opt-in transient transport retries for AgentClient and enable them for Android integration tests to absorb short-lived ADB forward interruptions. Debounce Android app process monitoring, restore missing adb forwards, and capture richer diagnostics when the app appears to disappear. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #287 review feedback - AgentClient: narrow IsTransientTransportException so HttpClient timeout TaskCanceledException no longer triggers retries; only retry HttpRequestException with SocketException inner, IOException, or non-timeout TaskCanceledException. - AndroidEmulatorFixture: parse ps -A PID via positional column 1 instead of the first int-parseable column (custom ROMs may use numeric UIDs in USER). - AndroidEmulatorFixture: short-circuit AVD provisioning when DEVFLOW_TEST_ANDROID_SERIAL points at an online physical device, so the physical-device path no longer requires cmdline-tools to be installed. - WebViewDriverTests: add a 5s CancellationTokenSource and finally-block accept guard to GetStatus_WithTransientRetry_RetriesConnectionRefused so a missed accept can no longer hang the test, and set ExclusiveAddressUse=false on both listeners to reduce port-rebind flakiness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Harden adb forward usage in Android integration fixture Carry over the more defensive adb forwarding patterns from PR #273: - Scope `adb forward --list` to the active device with `-s <serial>` so parsing tolerates both the 2-column `<local> <remote>` and 3-column `<serial> <local> <remote>` output shapes regardless of how many devices are attached. - Pre-emptively run `adb forward --remove tcp:<agent>` before re-adding the mapping so a stale entry left by a prior adb-server crash cannot block recovery. - After re-establishing the forward, re-list and verify the mapping is visible. `adb forward` can return 0 while the daemon is in an inconsistent state, and silently passing through amplified into a ~4 minute opaque retry storm in the agent client. - Capture both `adb -s <serial> forward --list` and `reverse --list` in the crash diagnostics bundle so port-forwarding state is visible when triaging app-loss artifacts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #287 review feedback (round 2) - AgentClient: split SendWithTransientRetriesAsync into GET vs explicit-method overloads and add a new RetryMutatingRequests property (default true) gating whether transient retries apply to POST/PUT/DELETE. XML docs spell out the duplicate-side-effect risk on TransientFailureRetryCount and the new RetryMutatingRequests so production callers can opt out cleanly without changing existing integration-test behavior. - IntegrationTestBase: narrow IsTransientTransportException to mirror the AgentClient predicate (HttpRequestException with SocketException inner, IOException, or non-timeout TaskCanceledException) so HttpClient timeouts are no longer amplified into ~8x60s of retries. - WebViewDriverTests: bump the server startup delay to 500ms and wrap the client call in a Stopwatch with an elapsed >= retry-delay assertion so the retry test fails loudly if the retry path is ever skipped. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address PR #287 review feedback (round 3) - AndroidEmulatorFixture: in the 3-column `adb forward --list` fallback, skip rows whose serial does not match the active device. Defensive guard against unexpected multi-device output from older adb releases. - AgentClient / IntegrationTestBase: tighten transient-exception predicates so a bare `TaskCanceledException` with no inner exception is no longer classified as transient. Caller-initiated CancellationToken cancellations must not be retried; only TCEs wrapping a real (non-timeout) transport failure should now flow through the retry loop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Add deterministic Android ADB forwarding checks for broker discovery, direct agent fallback paths, MCP sessions, and DevFlow diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

- AndroidDevFlowForwardingRequest now accepts BrokerPort so reverse setup targets the resolved broker port instead of the hardcoded 19223 constant. CLI and MCP callers pass BrokerClient.ReadBrokerPortPublic() ?? DefaultPort. - BuildMappingSuggestions takes the broker port so the human-facing 'adb reverse tcp:N tcp:N' hint reflects the actual port. - Add AndroidDevFlowPortForwarder.IsAdbLikelyAvailable() as a cheap, no-spawn probe and use it to short-circuit EnsureAndroidForwardingForPortsAsync / McpAgentSession.TryEnsureAndroidForwardingAsync, so 'maui devflow list' on desktop-only machines no longer instantiates AndroidProvider on every run. - Add two unit tests covering custom-broker-port repair and suggestion paths. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR improves DevFlow’s Android connectivity by having the CLI proactively prepare/repair the required ADB port mappings (broker reverse + agent forwards) during normal broker/agent discovery flows, and by clarifying the correct adb reverse vs adb forward guidance in documentation.

Changes:

Added an Android ADB forwarding helper (AndroidDevFlowPortForwarder) with deterministic device selection and a structured report for diagnostics/JSON output.
Extended broker resolution to return full agent registrations (not just ports) and wired Android forwarding into devflow list, devflow wait, devflow diagnose, and MCP agent sessions.
Updated DevFlow docs/skills to document adb reverse tcp:19223 tcp:19223 (app→broker) and adb forward tcp:<port> tcp:<port> (CLI→agent), including multi-device selection via --android-device / ANDROID_SERIAL.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Cli/Microsoft.Maui.Cli/DevFlow/Properties/InternalsVisibleTo.cs	Exposes DevFlow internals to CLI unit tests so the new forwarding helper can be tested.
src/Cli/Microsoft.Maui.Cli/DevFlow/Mcp/McpAgentSession.cs	Adds Android forwarding attempts during MCP agent client creation.
src/Cli/Microsoft.Maui.Cli/DevFlow/DevFlowCommands.cs	Adds `--android-device`, integrates forwarding into list/wait/diagnose flows, and prints forwarding diagnostics.
src/Cli/Microsoft.Maui.Cli/DevFlow/DevFlowCliJsonContext.cs	Adds source-gen JSON serialization support for forwarding report types.
src/Cli/Microsoft.Maui.Cli/DevFlow/Broker/BrokerClient.cs	Adds `ResolveAgentAsync` / `ResolveAgentForProjectAsync` to return full agent metadata for better decisions upstream.
src/Cli/Microsoft.Maui.Cli/DevFlow/Android/AndroidDevFlowPortForwarder.cs	New forwarding helper that inspects/repairs adb reverse/forward mappings and emits a structured report.
src/Cli/Microsoft.Maui.Cli.UnitTests/DevFlowCliIntegrationTests.cs	Adds a diagnose JSON test asserting forwarding state is reported for Android agents.
src/Cli/Microsoft.Maui.Cli.UnitTests/AndroidDevFlowPortForwarderTests.cs	Adds focused unit tests for device selection and mapping repair/reporting behavior.
plugins/dotnet-maui/skills/maui-devflow-debug/references/connectivity.md	Updates debugging guidance for Android forwarding and multi-device selection.
docs/DevFlow/broker.md	Fixes/clarifies Android forwarding directions and documents CLI auto-repair behavior + serial selection.

github-actions · 2026-05-20T19:10:27Z

Expert Code Review — PR #273

Methodology: 3 independent reviewers with adversarial consensus (+ 6 follow-up dispute evaluations for 3 contested findings).

Result: 7 findings posted as inline comments (4 moderate, 3 minor).

Severity	Count
🟡 Moderate	4
🟢 Minor	3

Key Themes

Hardcoded broker port in diagnostics (3/3 consensus) — report.BrokerPort should be used instead of DefaultBrokerPort
Redundant ADB calls in ListAgentsCommandAsync (2/3 consensus) — double device enumeration can be consolidated
MCP session forwarding guard gaps (2/3 consensus) — DefaultAgentPort path skips forwarding; fallback path is unconditional
ResolveAgentPort() sync ADB without platform evidence (disputed, 2/3 after follow-up) — config/fallback paths invoke ADB for any machine with SDK installed

Discarded Findings (single reviewer, no consensus)

MCP session lacks DeviceSerial parameter for multi-device — Reviewer 2 only
Test hooks are mutable static state without thread-safety guards — Reviewer 1 only
ResolveAgentForProjectAsync drops config-port fallback contract — Reviewer 2 only
Test creates temp directory not cleaned up — Reviewer 2 only (factually incorrect — test has try/finally)

CI Status

✅ macOS build: passed
🔄 Windows build: in progress
✅ CLA: passed

Test Coverage

The PR includes good unit test coverage (AndroidDevFlowPortForwarderTests.cs — 8 test cases covering no-ADB, multi-device, explicit serial, existing mappings, repair, missing mappings, and custom broker port scenarios) plus an integration test for the diagnose command. Coverage appears solid for the new AndroidDevFlowPortForwarder class.

Generated by Expert Code Review · 3 independent reviewers with adversarial consensus

Generated by Expert Code Review (auto) for issue #273 · ● 35.2M · ◷

github-actions

Expert Code Review: 7 findings posted inline (4 moderate, 3 minor). See the lean summary comment for full details.

Generated by Expert Code Review (auto) for issue #273 · ● 35.2M

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

Use a non-Android mock agent in the generic diagnose broker test so it does not probe the local Android SDK or start adb on Windows CI. Android forwarding behavior remains covered by dedicated tests with a fake forwarder. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Resolve the default DevFlow agent port only against an already-running broker before falling back to .mauidevflow/default. This keeps commands such as diagnose from spawning a foreground broker while parsing options in Windows tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Make the DevFlow ADB availability probe injectable and have Android diagnose tests use the fake path fully. This avoids touching the runner Android SDK/adb while still exercising forwarding diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

The CLI tests mutate process-wide state such as Console.Out and the current directory. Disable xUnit test parallelization for the assembly to avoid Windows-only races that can leave adb running from a temporary working directory and corrupt captured JSON output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Avoid constructing Xamarin.Android.Tools.AdbRunner when the CLI only needs to probe whether adb exists. This keeps unit tests from spawning a real adb process on Windows runners while preserving adb operations when devices are actually queried. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Do not run the Android forwarding probe during devflow diagnose unless an Android agent or explicit Android device is involved. This avoids starting adb for generic desktop diagnostics and keeps Windows unit tests isolated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

This comment has been minimized.

Sign in to view

github-actions Bot reviewed May 12, 2026

View reviewed changes

Redth force-pushed the redth/android-devflow-ports branch from a2ee575 to 02e6768 Compare May 19, 2026 16:18

Redth and others added 2 commits May 20, 2026 14:44

Improve Android DevFlow port forwarding

4f8e274

Add deterministic Android ADB forwarding checks for broker discovery, direct agent fallback paths, MCP sessions, and DevFlow diagnostics. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Redth force-pushed the redth/android-devflow-ports branch from 02e6768 to b361f6f Compare May 20, 2026 18:44

Redth marked this pull request as ready for review May 20, 2026 18:46

Copilot AI review requested due to automatic review settings May 20, 2026 18:46

Copilot started reviewing on behalf of Redth May 20, 2026 18:47 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Comment thread src/Cli/Microsoft.Maui.Cli/DevFlow/Mcp/McpAgentSession.cs Outdated

Comment thread src/Cli/Microsoft.Maui.Cli/DevFlow/DevFlowCommands.cs Outdated

github-actions Bot reviewed May 20, 2026

View reviewed changes

Address Android DevFlow review feedback

2dafcaf

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

Redth added integration-tests and removed integration-tests labels May 20, 2026

Redth and others added 6 commits May 21, 2026 09:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve Android DevFlow port forwarding#273

Improve Android DevFlow port forwarding#273
Redth wants to merge 9 commits into
mainfrom
redth/android-devflow-ports

Redth commented May 12, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 20, 2026

Uh oh!

github-actions Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Redth commented May 12, 2026

Changes

Validation

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented May 20, 2026

Expert Code Review — PR #273

Key Themes

Discarded Findings (single reviewer, no consensus)

CI Status

Test Coverage

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants