fix(android): a refused uninstall must not pass for clear-state

adb uninstall answers Failure [DELETE_FAILED_INTERNAL_ERROR] both when the package was never installed and when it refuses to remove one, so the failure text cannot say which happened and the old code installed over the top either way, keeping the data clear-state was asked to drop. Ask pm path instead, and fall back to pm clear when the app is still there.

internal/android/android.go

@@ -123,14 +123,17 @@ func antiFreezeCommands() [][]string {
 // reinstalling it. This replaces `pm clear` for clear-state: ColorOS and other
 // hardened OEM builds deny CLEAR_APP_USER_DATA even to the adb shell user, so a
 // clear aborts the launch, whereas uninstall+install is always permitted.
-// The uninstall is best effort so a not-installed app is not an error.
+// A failed uninstall is not passed over: `install -r` keeps the app's data, so
+// the reinstall would report a clear-state that never happened.
 func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout io.Writer) error {
 	adb, err := AdbBinary()
 	if err != nil {
 		return err
 	}
 	if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "uninstall", bundleID)...).CombinedOutput(); err != nil {
-		fmt.Fprintf(stdout, "clear-state: uninstall %s skipped (%v: %s)\n", bundleID, err, strings.TrimSpace(string(output)))
+		if err := clearDataUninstallLeftBehind(ctx, adb, serial, bundleID, strings.TrimSpace(string(output)), stdout); err != nil {
+			return err
+		}
 	}
 	if output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "install", "-r", apkPath)...).CombinedOutput(); err != nil {
 		return fmt.Errorf("install %s: %w: %s", apkPath, err, strings.TrimSpace(string(output)))
@@ -138,6 +141,37 @@ func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout
 	return nil
 }
 
+// clearDataUninstallLeftBehind reaches first-launch state after `adb uninstall`
+// failed. The failure text cannot say whether it failed: an API 34 emulator
+// answers "Failure [DELETE_FAILED_INTERNAL_ERROR]" both for a package that was
+// never installed and for one it refuses to remove. So ask the package manager
+// which happened. Nothing installed means nothing to clear. Still installed
+// means the data survives the reinstall, and `pm clear` is the one remaining
+// way to reach first-launch state; when that fails too, so does clear-state.
+func clearDataUninstallLeftBehind(ctx context.Context, adb, serial, bundleID, uninstallOutput string, stdout io.Writer) error {
+	if !packageInstalled(ctx, adb, serial, bundleID) {
+		return nil
+	}
+	output, err := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "pm", "clear", bundleID)...).CombinedOutput()
+	cleared := strings.TrimSpace(string(output))
+	if err != nil || !strings.Contains(cleared, "Success") {
+		return fmt.Errorf(
+			"clear-state: %s is still installed after `adb uninstall` said %q, and `pm clear` said %q: its data was not cleared",
+			bundleID, uninstallOutput, cleared,
+		)
+	}
+	fmt.Fprintf(stdout, "clear-state: uninstall %s said %q and left it installed; cleared its data with `pm clear` instead\n", bundleID, uninstallOutput)
+	return nil
+}
+
+// packageInstalled reports whether the package manager resolves an APK path for
+// bundleID. The printed path is the signal rather than the exit status, which
+// `adb shell` does not forward from devices below API 24.
+func packageInstalled(ctx context.Context, adb, serial, bundleID string) bool {
+	output, _ := exec.CommandContext(ctx, adb, adbArgs(serial, "shell", "pm", "path", bundleID)...).Output()
+	return strings.HasPrefix(strings.TrimSpace(string(output)), "package:")
+}
+
 const threeButtonNavOverlay = "com.android.internal.systemui.navbar.threebutton"
 
 // navModeOverlays are the system navigation-mode overlays. Only one is active at

internal/android/android_test.go

@@ -441,3 +441,134 @@ func TestNavModeToRestore(t *testing.T) {
 		}
 	}
 }
+
+// scriptedAdb puts an adb under a fake SDK root that logs each invocation's
+// arguments and answers from replies, a `case "$*" in` body. SDK lookup is
+// isolated onto that root, so ReinstallApp runs its real command sequence
+// against the script and the log holds what reached adb.
+func scriptedAdb(t *testing.T, replies string) string {
+	t.Helper()
+	root := t.TempDir()
+	log := filepath.Join(root, "adb.log")
+	adb := filepath.Join(root, "platform-tools", "adb")
+	if err := os.MkdirAll(filepath.Dir(adb), 0o755); err != nil {
+		t.Fatalf("mkdir %s: %v", filepath.Dir(adb), err)
+	}
+	script := "#!/bin/sh\necho \"$*\" >> " + log + "\ncase \"$*\" in\n" + replies + "\nesac\n"
+	if err := os.WriteFile(adb, []byte(script), 0o755); err != nil {
+		t.Fatalf("write %s: %v", adb, err)
+	}
+	isolateSDKLookup(t, root)
+	return log
+}
+
+func adbCalls(t *testing.T, log string) []string {
+	t.Helper()
+	contents, err := os.ReadFile(log)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return nil
+		}
+		t.Fatalf("read %s: %v", log, err)
+	}
+	return strings.Split(strings.TrimSpace(string(contents)), "\n")
+}
+
+const (
+	uninstallRefused      = `"uninstall "*) echo "Failure [DELETE_FAILED_INTERNAL_ERROR]"; exit 1;;`
+	stillInstalled        = `"shell pm path "*) echo "package:/data/app/app.example-1/base.apk";;`
+	notInstalled          = `"shell pm path "*) exit 1;;`
+	installedSuccessfully = `"install "*) echo "Success";;`
+)
+
+func TestReinstallApp_RefusedUninstallClearsTheDataItLeftBehind(t *testing.T) {
+	log := scriptedAdb(t, strings.Join([]string{
+		uninstallRefused,
+		stillInstalled,
+		`"shell pm clear "*) echo "Success";;`,
+		installedSuccessfully,
+	}, "\n"))
+	output := &strings.Builder{}
+
+	if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", output); err != nil {
+		t.Fatalf("ReinstallApp: %v", err)
+	}
+
+	want := []string{
+		"uninstall app.example",
+		"shell pm path app.example",
+		"shell pm clear app.example",
+		"install -r /tmp/app.apk",
+	}
+	if got := adbCalls(t, log); !slices.Equal(got, want) {
+		t.Fatalf("adb calls = %v, want %v", got, want)
+	}
+	if !strings.Contains(output.String(), "pm clear") {
+		t.Errorf("output %q does not say the data was cleared some other way", output.String())
+	}
+}
+
+func TestReinstallApp_RefusedUninstallThatCannotBeClearedIsFatal(t *testing.T) {
+	log := scriptedAdb(t, strings.Join([]string{
+		uninstallRefused,
+		stillInstalled,
+		`"shell pm clear "*) echo "Failed"; exit 1;;`,
+		installedSuccessfully,
+	}, "\n"))
+
+	err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{})
+
+	if err == nil {
+		t.Fatal("ReinstallApp reported success while app.example kept the data clear-state was asked to remove")
+	}
+	for _, want := range []string{"app.example", "DELETE_FAILED_INTERNAL_ERROR", "Failed"} {
+		if !strings.Contains(err.Error(), want) {
+			t.Errorf("error %q does not quote %q", err, want)
+		}
+	}
+	if slices.Contains(adbCalls(t, log), "install -r /tmp/app.apk") {
+		t.Error("installed over an app whose data survived, which is the reinstall reporting a clear it did not perform")
+	}
+}
+
+func TestReinstallApp_UninstallFailureWithNothingInstalledIsQuiet(t *testing.T) {
+	log := scriptedAdb(t, strings.Join([]string{
+		uninstallRefused,
+		notInstalled,
+		installedSuccessfully,
+	}, "\n"))
+	output := &strings.Builder{}
+
+	if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", output); err != nil {
+		t.Fatalf("ReinstallApp: %v", err)
+	}
+
+	calls := adbCalls(t, log)
+	if !slices.Contains(calls, "install -r /tmp/app.apk") {
+		t.Fatalf("adb calls = %v, want the install to go ahead: a first run has no app to uninstall", calls)
+	}
+	if slices.Contains(calls, "shell pm clear app.example") {
+		t.Errorf("adb calls = %v, want no data clear: there was no app holding data", calls)
+	}
+	if output.String() != "" {
+		t.Errorf("output = %q, want nothing: a first run has no app to uninstall", output.String())
+	}
+}
+
+func TestReinstallApp_SuccessfulUninstallNeedsNoFallback(t *testing.T) {
+	log := scriptedAdb(t, strings.Join([]string{
+		`"uninstall "*) echo "Success";;`,
+		stillInstalled,
+		`"shell pm clear "*) echo "Success";;`,
+		installedSuccessfully,
+	}, "\n"))
+
+	if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{}); err != nil {
+		t.Fatalf("ReinstallApp: %v", err)
+	}
+
+	want := []string{"uninstall app.example", "install -r /tmp/app.apk"}
+	if got := adbCalls(t, log); !slices.Equal(got, want) {
+		t.Fatalf("adb calls = %v, want %v", got, want)
+	}
+}