From 1ec6eb3847df2fe43dd8c370573e1323c1779858 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 00:00:36 +0530 Subject: [PATCH 1/5] 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 | 38 ++++++++- internal/android/android_test.go | 131 +++++++++++++++++++++++++++++++ 2 files changed, 167 insertions(+), 2 deletions(-) diff --git a/internal/android/android.go b/internal/android/android.go index 5983056..6fe5170 100644 --- a/internal/android/android.go +++ b/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 diff --git a/internal/android/android_test.go b/internal/android/android_test.go index 2161cd3..2d5902d 100644 --- a/internal/android/android_test.go +++ b/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) + } +} From abc5e7db3ee4a9794735d7755a73437212d90daa Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 00:01:40 +0530 Subject: [PATCH 2/5] fix(ios): a failed simctl uninstall must fail the reinstall simctl install over an installed app carries its data container across, so discarding the uninstall error reported a clear-state that never happened. Uninstalling an app that is not installed exits 0 on a booted simulator, so every failure here is a real one. --- internal/driver/ioscompanion/driver.go | 8 ++- internal/driver/ioscompanion/driver_test.go | 62 +++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 04b7d7c..125a421 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -615,8 +615,14 @@ func (d *Driver) clearAppState(ctx context.Context) error { // simctlReinstall uninstalls and reinstalls the app bundle via simctl. App // lifecycle stays with simctl: the companion's install RPC misreads current // simulator targets' architectures and rejects valid bundles. +// A failed uninstall ends the reinstall: `simctl install` over an installed app +// carries its data container across, so clear-state would be reported without +// happening. Uninstalling an app that is not installed exits 0, so there is no +// benign failure here to sort out from a real one. func (d *Driver) simctlReinstall(ctx context.Context) error { - _ = exec.CommandContext(ctx, "xcrun", "simctl", "uninstall", d.udid, d.bundleID).Run() + if output, err := exec.CommandContext(ctx, "xcrun", "simctl", "uninstall", d.udid, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("simctl uninstall %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } output, err := exec.CommandContext(ctx, "xcrun", "simctl", "install", d.udid, d.appPath).CombinedOutput() if err != nil { return fmt.Errorf("simctl install: %w: %s", err, strings.TrimSpace(string(output))) diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index db3b2f4..f8ae5b9 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -329,6 +329,68 @@ func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { } } +// scriptedXcrun puts an xcrun on PATH that logs each invocation's arguments and +// answers from replies, a `case "$*" in` body, so a reinstall runs its real +// command sequence and the log holds what reached the tool. +func scriptedXcrun(t *testing.T, replies string) string { + t.Helper() + directory := t.TempDir() + log := filepath.Join(directory, "xcrun.log") + script := "#!/bin/sh\necho \"$*\" >> " + log + "\ncase \"$*\" in\n" + replies + "\nesac\n" + if err := os.WriteFile(filepath.Join(directory, "xcrun"), []byte(script), 0o755); err != nil { + t.Fatalf("write xcrun: %v", err) + } + t.Setenv("PATH", directory) + return log +} + +func xcrunCalls(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") +} + +func TestSimctlReinstallStopsWhenTheUninstallFails(t *testing.T) { + log := scriptedXcrun(t, `"simctl uninstall "*) echo "Simulator device failed to uninstall app.example."; echo "Uninstall prohibited."; exit 22;; +"simctl install "*) :;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + err := d.simctlReinstall(context.Background()) + + if err == nil { + t.Fatal("simctlReinstall reported success while app.example kept the data clear-state was asked to remove") + } + for _, want := range []string{"app.example", "Uninstall prohibited."} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not quote %q", err, want) + } + } + if calls := xcrunCalls(t, log); slices.Contains(calls, "simctl install SIM-UDID /tmp/Sample.app") { + t.Errorf("xcrun calls = %v: installing over the app carries its data into the run", calls) + } +} + +func TestSimctlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { + log := scriptedXcrun(t, `"simctl uninstall "*) :;; +"simctl install "*) :;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + if err := d.simctlReinstall(context.Background()); err != nil { + t.Fatalf("simctlReinstall: %v", err) + } + + want := []string{"simctl uninstall SIM-UDID app.example", "simctl install SIM-UDID /tmp/Sample.app"} + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v", got, want) + } +} + func TestLaunchRejectsEnvironment(t *testing.T) { d := newTestDriver(&fakeCompanion{accessibilityJSON: "[]"}) err := d.Launch(context.Background(), "", false, map[string]string{"K": "V"}) From 6234bf5ba0bee40b6a343838fc193503eb6217b8 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 00:02:36 +0530 Subject: [PATCH 3/5] fix(ios): a failed devicectl uninstall must fail the reinstall same hole as the simulator path: devicectl install over an app keeps its data, and the discarded uninstall error hid it. Uninstalling a bundle id that is not installed exits 0 with 'App uninstalled.' on a paired iPhone, so a failure here is always real. --- internal/driver/ioscompanion/device.go | 8 ++++- internal/driver/ioscompanion/device_test.go | 40 +++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 52903ab..1193636 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -224,8 +224,14 @@ func (d *Driver) respawnDevice(ctx context.Context) error { // devicectlReinstall uninstalls then installs the app bundle via devicectl, // keyed on the CoreDevice id. App lifecycle stays with devicectl: the runner's // own install path is simulator-specific. +// A failed uninstall ends the reinstall: installing over an app keeps its data, +// so clear-state would be reported without happening. Uninstalling an app that +// is not installed exits 0 ("App uninstalled." on a paired iPhone running iOS +// 26.5), so there is no benign failure here to sort out from a real one. func (d *Driver) devicectlReinstall(ctx context.Context) error { - _ = exec.CommandContext(ctx, "xcrun", "devicectl", "device", "uninstall", "app", "--device", d.coreDeviceID, d.bundleID).Run() + if output, err := exec.CommandContext(ctx, "xcrun", "devicectl", "device", "uninstall", "app", "--device", d.coreDeviceID, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("devicectl uninstall %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } output, err := exec.CommandContext(ctx, "xcrun", "devicectl", "device", "install", "app", "--device", d.coreDeviceID, d.appPath).CombinedOutput() if err != nil { return fmt.Errorf("devicectl install: %w: %s", err, strings.TrimSpace(string(output))) diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index 1ffd11d..abc5ced 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -7,6 +7,7 @@ import ( "net" "os/exec" "slices" + "strings" "testing" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" @@ -182,6 +183,45 @@ func TestNewDeviceReinstallsOnceBeforeTheRunnerSession(t *testing.T) { } } +func TestDevicectlReinstallStopsWhenTheUninstallFails(t *testing.T) { + log := scriptedXcrun(t, `"devicectl device uninstall "*) echo "ERROR: Internal logic error: Connection was invalidated"; exit 1;; +"devicectl device install "*) :;;`) + d := &Driver{coreDeviceID: "CORE-DEVICE", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + err := d.devicectlReinstall(context.Background()) + + if err == nil { + t.Fatal("devicectlReinstall reported success while app.example kept the data clear-state was asked to remove") + } + for _, want := range []string{"app.example", "Connection was invalidated"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("error %q does not quote %q", err, want) + } + } + calls := xcrunCalls(t, log) + if slices.ContainsFunc(calls, func(call string) bool { return strings.HasPrefix(call, "devicectl device install") }) { + t.Errorf("xcrun calls = %v: installing over the app carries its data into the run", calls) + } +} + +func TestDevicectlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { + log := scriptedXcrun(t, `"devicectl device uninstall "*) echo "App uninstalled.";; +"devicectl device install "*) :;;`) + d := &Driver{coreDeviceID: "CORE-DEVICE", bundleID: "app.example", appPath: "/tmp/Sample.app"} + + if err := d.devicectlReinstall(context.Background()); err != nil { + t.Fatalf("devicectlReinstall: %v", err) + } + + want := []string{ + "devicectl device uninstall app --device CORE-DEVICE app.example", + "devicectl device install app --device CORE-DEVICE /tmp/Sample.app", + } + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v", got, want) + } +} + func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { output := &bytes.Buffer{} d := &Driver{output: output, deviceMode: true} From 87aa7d9284777beff7bce6000584c67d14886e2a Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 00:07:12 +0530 Subject: [PATCH 4/5] docs(android): say why the uninstall text cannot be read --- internal/android/android.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/android/android.go b/internal/android/android.go index 6fe5170..1ccd823 100644 --- a/internal/android/android.go +++ b/internal/android/android.go @@ -142,10 +142,10 @@ func ReinstallApp(ctx context.Context, serial, bundleID, apkPath string, stdout } // 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 +// failed. The failure text cannot say why: 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 { From ea952378fe3fd58c6f2642c3a9c3c6f7ed25d69f Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 00:07:58 +0530 Subject: [PATCH 5/5] test(android): name the uninstall failure for what it says, not why --- internal/android/android_test.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/internal/android/android_test.go b/internal/android/android_test.go index 2d5902d..479c886 100644 --- a/internal/android/android_test.go +++ b/internal/android/android_test.go @@ -475,18 +475,18 @@ func adbCalls(t *testing.T, log string) []string { } 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";;` + uninstallFails = `"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;;` + installSucceeds = `"install "*) echo "Success";;` ) func TestReinstallApp_RefusedUninstallClearsTheDataItLeftBehind(t *testing.T) { log := scriptedAdb(t, strings.Join([]string{ - uninstallRefused, + uninstallFails, stillInstalled, `"shell pm clear "*) echo "Success";;`, - installedSuccessfully, + installSucceeds, }, "\n")) output := &strings.Builder{} @@ -510,10 +510,10 @@ func TestReinstallApp_RefusedUninstallClearsTheDataItLeftBehind(t *testing.T) { func TestReinstallApp_RefusedUninstallThatCannotBeClearedIsFatal(t *testing.T) { log := scriptedAdb(t, strings.Join([]string{ - uninstallRefused, + uninstallFails, stillInstalled, `"shell pm clear "*) echo "Failed"; exit 1;;`, - installedSuccessfully, + installSucceeds, }, "\n")) err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{}) @@ -533,9 +533,9 @@ func TestReinstallApp_RefusedUninstallThatCannotBeClearedIsFatal(t *testing.T) { func TestReinstallApp_UninstallFailureWithNothingInstalledIsQuiet(t *testing.T) { log := scriptedAdb(t, strings.Join([]string{ - uninstallRefused, + uninstallFails, notInstalled, - installedSuccessfully, + installSucceeds, }, "\n")) output := &strings.Builder{} @@ -560,7 +560,7 @@ func TestReinstallApp_SuccessfulUninstallNeedsNoFallback(t *testing.T) { `"uninstall "*) echo "Success";;`, stillInstalled, `"shell pm clear "*) echo "Success";;`, - installedSuccessfully, + installSucceeds, }, "\n")) if err := ReinstallApp(t.Context(), "", "app.example", "/tmp/app.apk", &strings.Builder{}); err != nil {