[FastDeploy2] Clean orphan staging files

[jonathanpeppers]

Summary

• remove /data/local/tmp/fastdeploy2/<package>/<user> staging directories when the package is definitely no longer installed for that Android user
• run cleanup after every successful APK install/reinstall, while skipping managed-only incremental deployments
• query installed packages once per user and fail closed on missing tools, failed, empty, or malformed pm output, and ambiguous grep results
• reject symlinked staging roots, package directories, and user directories before deletion
• keep the cleanup command below and explicitly enforce $(_AndroidFastDeployMaxShellCommandLength)
• retain private $(_AndroidFastDeploySkipCleanup) as an escape hatch
• add an MSBuildDeviceIntegration test covering orphan deletion, installed-package retention, symlink confinement, and cleanup skipping on a managed-only incremental deployment
• document FastDeploy2 commands and approximate Android API support

Validation

• built Xamarin.Android.Build.Debugging.Tasks.csproj
• validated the generated 837-character cleanup command with shell syntax checking
• validated Xamarin.Android.Common.Debugging.targets as XML
• reproduced the previous root/package symlink traversal on an attached API 37 device and verified the guarded command retains the outside sentinel while deleting a real orphan
• verified malformed status-0 package output retains staging and fails closed on the attached device

The full device integration test requires the repository's .NET 11 local SDK, which is not available in this worktree.

[Copilot]

Pull request overview

This PR adds a FastDeploy2 “cold deploy” maintenance step to clean up orphaned on-device staging directories under /data/local/tmp/fastdeploy2/<package>/<user> when the corresponding package is no longer installed, with rate limiting and a safety window. It also wires a private MSBuild escape hatch to skip the cleanup, adds a unit test for the generated shell command, and documents command/tool compatibility expectations.

Changes:

• Implement orphan staging cleanup shell command generation and invoke it on the cold deployment path (rate-limited, safety-window guarded).
• Add $(_AndroidFastDeploySkipCleanup) → FastDeploySkipCleanup task wiring as an internal escape hatch.
• Add a unit test for the cleanup command structure and extend FastDeploy2 documentation (including command compatibility notes).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/DebuggingTasksTests.cs	Adds a unit test validating key fragments/order of the generated cleanup command.
src/Xamarin.Android.Build.Debugging.Tasks/Xamarin.Android.Common.Debugging.targets	Passes the new MSBuild property into the FastDeploy2 task invocation.
src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.Manifest.cs	Adds cleanup command generation + execution helper (adb shell) for orphan staging directories.
src/Xamarin.Android.Build.Debugging.Tasks/Tasks/FastDeploy2.cs	Adds the FastDeploySkipCleanup task property and runs cleanup only on the cold path.
Documentation/guides/FastDeploy2.md	Documents the new escape hatch, the cleanup behavior, and command compatibility expectations.

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

[github-actions]

Reviewed the full diff and surrounding implementation. I found no correctness or safety defects in the fail-closed cleanup logic. I left one non-blocking test-coverage suggestion for the production safety guards.

CI is still in progress: 29 of 44 checks have completed successfully, 14 are running, and the aggregate check remains queued; no failures are currently reported.

Generated by Android PR Reviewer for #12310 · gpt56 · 132.7 AIC · ⌖ 10.1 AIC · ⊞ 25.3K
_{Comment /review to run again}

[simonrozsival]

I think this is a practical solution to the cleanup problem right now and we can later consider improving it based on customer feedback.

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12310

[github-actions]

Found one correctness warning and one test-coverage suggestion. The package lookup and grep handling are otherwise appropriately fail-closed, and cleanup is correctly restricted to successful APK-install paths rather than managed-only incremental deployments.

CI is still in progress: the visible completed checks are passing, six jobs are running, and the aggregate dotnet-android check is queued; no failure is currently reported.

Generated by Android PR Reviewer for #12310 · gpt56 · 152.2 AIC · ⌖ 20.1 AIC · ⊞ 25.6K
_{Comment /review to run again}

[jonathanpeppers]

/review

[github-actions]

✅ Android PR Reviewer completed successfully!

Generated by Android PR Reviewer for #12310

[github-actions]

Verdict: ✅ LGTM pending CI completion — no blocking correctness or safety issues found. I left 1 💡 testing suggestion for the destructive command's fail-closed branches.

The package matching is exact, ambiguous command results avoid deletion, installed-package staging is preserved, and cleanup is correctly limited to successful APK-install paths. At the latest CI snapshot, 39 checks had completed successfully, two were still running, and the aggregate check was queued; no failures were reported.

Generated by Android PR Reviewer for #12310 · gpt56 · 161.5 AIC · ⌖ 8.93 AIC · ⊞ 25.7K
_{Comment /review to run again}

[jonathanpeppers]

@dalexsoto review

[dalexsoto]

Three destructive-cleanup blockers remain:

• The two-level staging glob follows a symlinked root or package directory, allowing rm -rf "$d" to delete a numeric directory outside /data/local/tmp/fastdeploy2 (reproduced with the exact command).
• Any nonempty pm stdout with status 0 is trusted as a complete package list; supported Android versions can print server-error diagnostics while returning 0, causing every package lookup to look absent and permitting delete-all.
• The once-per-user package snapshot races concurrent uninstall/reinstall/deploy flows, so cleanup can delete staging after the package has become installed and is being reused.

Please enforce no-follow/root confinement immediately before deletion, validate package output as package records, and coordinate/revalidate candidates against concurrent deployments.

[jonathanpeppers]

Addressed the two reproduced cleanup safety issues in 08921ce:

• Reject symlinked staging roots, package directories, and user directories immediately before deletion. On the attached API 37 device, the previous command deleted sentinels through both root and package-directory symlinks; the updated command retains the outside sentinel while still deleting a real orphan.
• Accept package enumeration only when every output line is a package record. A status-0 malformed-output probe now retains the orphan and returns cleanup status 1.

I intentionally did not add deployment locking or concurrency coordination here. That is a separate deployment-lifecycle concern and would be scope creep for this cleanup PR.

stale