From f1a5db886e6555e243c8192858eeb0742e7bf100 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:00:45 +0530 Subject: [PATCH 01/11] fix(ios): recognise every shape a blown launch bound arrives in The runner transport reports a blown budget two ways, its own comment says so: the context's error once cancellation has landed, and the connection's i/o timeout when the deadline armed from that context fires first. The legacy transport reports it as a gRPC status. errors.Is against context.DeadlineExceeded only matches the first, so the session restart never fired for the other two and a wedged session stayed wedged. --- internal/driver/ioscompanion/driver.go | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 125a421..5321346 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -484,6 +484,26 @@ func isConnectionError(err error) bool { return false } +// isBudgetExpiry reports whether err is a call that outlived its bound rather +// than a failure the transport can name. Each transport says so differently: +// the runner wraps the context's error when cancellation has landed and the +// connection's i/o timeout when the deadline it armed from that context fires +// first, and the legacy companion returns a gRPC status. Only an expiry earns +// a session restart; an error the runner reports has already said what a fresh +// session would say. +func isBudgetExpiry(err error) bool { + if err == nil { + return false + } + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, os.ErrDeadlineExceeded) { + return true + } + if statusValue, ok := status.FromError(err); ok { + return statusValue.Code() == codes.DeadlineExceeded + } + return false +} + func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, env map[string]string) error { if bundleID != "" { d.bundleID = bundleID @@ -545,7 +565,7 @@ func (d *Driver) launchWithSessionRecovery(ctx context.Context) error { err := d.lifecycleCall(ctx, launch) // A caller whose own budget ran out gets no restart: the bound that expired // was the caller's to spend, and the second attempt would inherit it dead. - if err == nil || !errors.Is(err, context.DeadlineExceeded) || ctx.Err() != nil || d.restart == nil { + if !isBudgetExpiry(err) || ctx.Err() != nil || d.restart == nil { return err } fmt.Fprintf(d.output, "launch %s blew its %v bound (%v); restarting the session and launching once more\n", From f911079e32642293698c6df868339e0bc74d8a33 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:00:49 +0530 Subject: [PATCH 02/11] test(ios): drive the launch recovery with what the transports return The wedged-session fake answered with ctx.Err() raw, which is the one shape the guard already matched. The recovery now runs against the error each transport really produces for the same expiry, taken from a runner and a legacy companion that never answer. --- internal/driver/ioscompanion/driver_test.go | 194 ++++++++++++++++---- 1 file changed, 161 insertions(+), 33 deletions(-) diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index f8ae5b9..bb68213 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -18,10 +18,12 @@ import ( "testing" "time" + "google.golang.org/grpc" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "github.com/priyanshujain/sanderling/internal/driver" + "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/companionpb" "github.com/priyanshujain/sanderling/internal/driver/ioscompanion/transport" ) @@ -1116,14 +1118,134 @@ func TestLaunchLeavesATighterCallerDeadlineAlone(t *testing.T) { } } +// blownBudgetShape is one way a transport the driver launches through reports +// a lifecycle call outliving its budget. +type blownBudgetShape struct { + name string + err error +} + +// blownBudgetShapes drives every such transport against a server that never +// answers and returns the error each one really produces. The runner transport +// has two: wrapTransport wraps the context's own error once cancellation has +// landed, and the connection's i/o timeout when the deadline it armed from +// that context fires first. The legacy transport reports the same expiry as a +// gRPC status. Only the first satisfies errors.Is(err, context.DeadlineExceeded), +// so a fake that returns ctx.Err() raw shows the driver a recovery that two +// thirds of production can never reach. +func blownBudgetShapes(t *testing.T) []blownBudgetShape { + t.Helper() + return []blownBudgetShape{ + {"runner context deadline", runnerContextDeadlineError(t)}, + {"runner connection deadline", silentRunnerLaunchError(t, connectionDeadlineOnly{time.Now().Add(100 * time.Millisecond)})}, + {"legacy grpc deadline", silentGRPCLaunchError(t)}, + } +} + +// runnerContextDeadlineError is the shape the runner transport produces once +// the context's own cancellation has landed. +func runnerContextDeadlineError(t *testing.T) error { + t.Helper() + ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer cancel() + return silentRunnerLaunchError(t, ctx) +} + +// connectionDeadlineOnly carries a deadline the runner transport arms the +// connection with, while its own cancellation never lands. That is the race +// wrapTransport's second branch exists for: the connection's deadline fires +// while ctx.Err() is still nil. +type connectionDeadlineOnly struct{ deadline time.Time } + +func (c connectionDeadlineOnly) Deadline() (time.Time, bool) { return c.deadline, true } +func (c connectionDeadlineOnly) Done() <-chan struct{} { return nil } +func (c connectionDeadlineOnly) Err() error { return nil } +func (c connectionDeadlineOnly) Value(any) any { return nil } + +// silentRunnerLaunchError returns what the real runner transport produces for a +// launch nobody ever answers. +func silentRunnerLaunchError(t *testing.T, ctx context.Context) error { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + closed := make(chan struct{}) + go func() { + conn, acceptErr := listener.Accept() + if acceptErr != nil { + return + } + defer conn.Close() + <-closed + }() + companion, err := transport.DialRunner(listener.Addr().String(), "SIM-UDID", "com.example.app") + if err != nil { + listener.Close() + t.Fatal(err) + } + t.Cleanup(func() { + close(closed) + _ = companion.Close() + listener.Close() + }) + + launchErr := companion.Launch(ctx, "com.example.app", true) + if launchErr == nil { + t.Fatal("the runner transport reported a launch no server ever answered") + } + return launchErr +} + +// silentCompanionServer is the legacy companion with a launch that never +// answers, so the caller's own deadline is what ends the call. +type silentCompanionServer struct { + companionpb.UnimplementedCompanionServiceServer +} + +func (silentCompanionServer) Launch(stream grpc.BidiStreamingServer[companionpb.LaunchRequest, companionpb.LaunchResponse]) error { + <-stream.Context().Done() + return stream.Context().Err() +} + +// silentGRPCLaunchError returns what the legacy transport produces for the same +// launch, which SANDERLING_SIMULATOR_COMPANION=legacy still runs on. +func silentGRPCLaunchError(t *testing.T) error { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + server := grpc.NewServer() + companionpb.RegisterCompanionServiceServer(server, silentCompanionServer{}) + go server.Serve(listener) + t.Cleanup(server.Stop) + + companion, err := transport.Dial(listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { companion.Close() }) + + ctx, cancel := context.WithTimeout(context.Background(), 250*time.Millisecond) + defer cancel() + launchErr := companion.Launch(ctx, "com.example.app", true) + if launchErr == nil { + t.Fatal("the legacy transport reported a launch the companion never answered") + } + return launchErr +} + // wedgedUntilRestartCompanion models the session a refused launch leaves -// behind: the refusal is never reported, and no later launch is answered until -// the session itself is replaced. +// behind: the refusal is never reported, no later launch is answered until the +// session itself is replaced, and the expired bound reaches the driver in +// whatever shape its transport gives it. type wedgedUntilRestartCompanion struct { fakeCompanion - mutex sync.Mutex - replaced bool - attempted int + blownBudget error + mutex sync.Mutex + replaced bool + attempted int } func (w *wedgedUntilRestartCompanion) replaceSession() { @@ -1147,7 +1269,7 @@ func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bo return nil } <-ctx.Done() - return ctx.Err() + return w.blownBudget } // TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound covers the FrontBoard @@ -1155,34 +1277,40 @@ func (w *wedgedUntilRestartCompanion) Launch(ctx context.Context, _ string, _ bo // makes the session refuse the launch, and XCTest answers that refusal with // minutes of diagnostics instead of an error, so the bound expires and every // later call queues behind the same wedge. Calling launch again on that session -// cannot work; the run only recovers if the session is replaced first. +// cannot work; the run only recovers if the session is replaced first. The +// recovery has to fire on every shape the driver's transports report that +// expiry in, because which one arrives is a race the driver does not control. func TestLaunchReplacesTheSessionAfterALaunchBlowsItsBound(t *testing.T) { - previous := launchTimeout - launchTimeout = 100 * time.Millisecond - defer func() { launchTimeout = previous }() + for _, shape := range blownBudgetShapes(t) { + t.Run(shape.name, func(t *testing.T) { + previous := launchTimeout + launchTimeout = 100 * time.Millisecond + defer func() { launchTimeout = previous }() - companion := &wedgedUntilRestartCompanion{} - output := &bytes.Buffer{} - d := newTestDriver(companion) - d.output = output - restarts := 0 - d.restart = func(context.Context) error { - restarts++ - companion.replaceSession() - return nil - } + companion := &wedgedUntilRestartCompanion{blownBudget: shape.err} + output := &bytes.Buffer{} + d := newTestDriver(companion) + d.output = output + restarts := 0 + d.restart = func(context.Context) error { + restarts++ + companion.replaceSession() + return nil + } - if err := d.Launch(context.Background(), "", false, nil); err != nil { - t.Fatalf("Launch: %v", err) - } - if restarts != 1 { - t.Fatalf("session restarts = %d, want exactly 1", restarts) - } - if attempts := companion.launchAttempts(); attempts != 2 { - t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) - } - if !strings.Contains(output.String(), "restarting the session") { - t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + if err := d.Launch(context.Background(), "", false, nil); err != nil { + t.Fatalf("Launch: %v", err) + } + if restarts != 1 { + t.Fatalf("session restarts = %d, want exactly 1 (the session reported %v)", restarts, shape.err) + } + if attempts := companion.launchAttempts(); attempts != 2 { + t.Fatalf("launch attempts = %d, want 2: one that wedged and one on the replaced session", attempts) + } + if !strings.Contains(output.String(), "restarting the session") { + t.Fatalf("the recovery was silent, so a run that needed it never says so; output was %q", output.String()) + } + }) } } @@ -1196,7 +1324,7 @@ func TestLaunchBoundsTheSessionRestartItTriggers(t *testing.T) { launchRecoveryTimeout = 200 * time.Millisecond defer func() { launchTimeout, launchRecoveryTimeout = previousLaunch, previousRecovery }() - d := newTestDriver(&wedgedUntilRestartCompanion{}) + d := newTestDriver(&wedgedUntilRestartCompanion{blownBudget: runnerContextDeadlineError(t)}) d.restart = func(restartCtx context.Context) error { <-restartCtx.Done() return restartCtx.Err() @@ -1222,7 +1350,7 @@ func TestLaunchKeepsTheSessionWhenTheCallersOwnDeadlineExpires(t *testing.T) { launchTimeout = 30 * time.Second defer func() { launchTimeout = previous }() - companion := &wedgedUntilRestartCompanion{} + companion := &wedgedUntilRestartCompanion{blownBudget: runnerContextDeadlineError(t)} d := newTestDriver(companion) restarts := 0 d.restart = func(context.Context) error { From 119877c78a143bb2cc6b6709025cdc59cba4999a Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:02:57 +0530 Subject: [PATCH 03/11] fix(ios): stop the app before clearing its state Launch terminated and then cleared; the clear moved to construction and left nothing stopping the app first. The container wipe deletes files a live app still holds open, and the CI ios leg passes no app path so the wipe is the path it takes. simctl stops it, since the clear now runs before any automation session exists. On a device the uninstall that is its only clear takes the running app with it. --- internal/driver/ioscompanion/device.go | 6 +++++- internal/driver/ioscompanion/driver.go | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 1193636..de10374 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -111,13 +111,17 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { // Device seams: clear-state reinstalls via devicectl; the container reset and // paste grant are simulator-only and become no-ops. The runner types - // natively, so no paste prompt is ever hit. + // natively, so no paste prompt is ever hit. Stopping the app before the + // clear is a no-op too: devicectl addresses processes by pid rather than by + // bundle, and the uninstall that is the device's only clear takes the + // running app with it, which is what a terminate here would be for. d.reinstallApp = options.reinstallApp if d.reinstallApp == nil { d.reinstallApp = d.devicectlReinstall } d.resetContainer = d.deviceResetContainerUnsupported d.grantPaste = func(context.Context) error { return nil } + d.terminateApp = func(context.Context) error { return nil } d.restart = d.respawnDevice d.processContext, d.processCancel = context.WithCancel(ctx) diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index 5321346..c1abc1b 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -91,6 +91,7 @@ type Options struct { dialRunner func(address string) (transport.Companion, error) reinstallApp func(ctx context.Context) error resetContainer func(ctx context.Context) error + terminateApp func(ctx context.Context) error } // Driver implements driver.DeviceDriver against an iOS simulator companion. @@ -136,6 +137,10 @@ type Driver struct { // A seam so tests skip the simctl shell-outs. reinstallApp func(ctx context.Context) error + // terminateApp stops the app before its state is cleared. A seam so tests + // skip the simctl shell-out. + terminateApp func(ctx context.Context) error + // grantPaste pre-authorizes the app's pasteboard access. A seam so tests // skip the sqlite shell-out. grantPaste func(ctx context.Context) error @@ -237,6 +242,7 @@ func New(ctx context.Context, options Options) (*Driver, error) { dialRunner: options.dialRunner, reinstallApp: options.reinstallApp, resetContainer: options.resetContainer, + terminateApp: options.terminateApp, hybrid: hybridCompanionEnabled(), } if driverInstance.spawnChild == nil { @@ -271,6 +277,9 @@ func New(ctx context.Context, options Options) (*Driver, error) { if driverInstance.reinstallApp == nil { driverInstance.reinstallApp = driverInstance.simctlReinstall } + if driverInstance.terminateApp == nil { + driverInstance.terminateApp = driverInstance.simctlTerminate + } driverInstance.grantPaste = driverInstance.grantPasteboardAccess driverInstance.processContext, driverInstance.processCancel = context.WithCancel(ctx) @@ -619,6 +628,11 @@ func (d *Driver) lifecycleCompanion() transport.Companion { // container and warns once that a full reinstall needs the app path. Called // only from construction, before any automation session is attached to the app. func (d *Driver) clearAppState(ctx context.Context) error { + // Nothing may be writing to the state while it goes, which is the ordering + // Launch used to hold: uninstall copes with a running app, deleting the + // data container out from under one does not. Best effort, because an app + // that is not running reports a failure that means nothing here. + _ = d.terminateApp(ctx) if d.appPath != "" { if err := d.reinstallApp(ctx); err != nil { return fmt.Errorf("reinstall %s: %w", d.appPath, err) @@ -650,6 +664,18 @@ func (d *Driver) simctlReinstall(ctx context.Context) error { return nil } +// simctlTerminate stops the app under test. Launch used to terminate through +// the automation session before clearing; the clear now runs before any session +// exists, so simctl is what is left to stop the app with. An app that is not +// running reports a failure that means nothing to the caller, which is why +// clearAppState treats this as best effort. +func (d *Driver) simctlTerminate(ctx context.Context) error { + if output, err := exec.CommandContext(ctx, "xcrun", "simctl", "terminate", d.udid, d.bundleID).CombinedOutput(); err != nil { + return fmt.Errorf("simctl terminate %s: %w: %s", d.bundleID, err, strings.TrimSpace(string(output))) + } + return nil +} + // grantPasteboardAccess authorizes the app to read the pasteboard without the // iOS permission prompt, by writing an allow row into the simulator's privacy // (TCC) database. This is the simulator counterpart to `simctl privacy grant`, From 9ede9dc3e2a3971c920897f06beaa23ba539de6d Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:03:02 +0530 Subject: [PATCH 04/11] test(ios): pin the stop that has to precede a clear The ordering probe now records the stop, and a scripted xcrun holds what reaches the tool: terminate before get_app_container, with the previous run's files gone after. A simctl terminate that finds nothing to stop still leaves the clear a success. --- internal/driver/ioscompanion/driver_test.go | 58 +++++++++++++++++++-- 1 file changed, 54 insertions(+), 4 deletions(-) diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index bb68213..b4f66ad 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -232,6 +232,7 @@ func clearStateOptions(t *testing.T, probe *clearStateProbe, udid string, clearS }, reinstallApp: func(context.Context) error { probe.record("reinstall"); return nil }, resetContainer: func(context.Context) error { probe.record("reset container"); return nil }, + terminateApp: func(context.Context) error { probe.record("stop app"); return nil }, } } @@ -249,9 +250,9 @@ func TestClearStateReinstallsOnceBeforeTheRunnerSession(t *testing.T) { t.Fatalf("Launch: %v", err) } - want := []string{"reinstall", "runner session"} + want := []string{"stop app", "reinstall", "runner session"} if got := probe.recorded(); !slices.Equal(got, want) { - t.Fatalf("calls = %v, want %v: the reinstall must run once, before the automation session attaches", got, want) + t.Fatalf("calls = %v, want %v: the reinstall must run once, on a stopped app, before the automation session attaches", got, want) } } @@ -270,9 +271,9 @@ func TestClearStateWithoutAppPathWipesContainerBeforeTheRunnerSession(t *testing t.Fatalf("Launch: %v", err) } - want := []string{"reset container", "runner session"} + want := []string{"stop app", "reset container", "runner session"} if got := probe.recorded(); !slices.Equal(got, want) { - t.Fatalf("calls = %v, want %v: the fallback must wipe the container once, before the session, and never reinstall", got, want) + t.Fatalf("calls = %v, want %v: the fallback must wipe a stopped app's container once, before the session, and never reinstall", got, want) } if warnings := strings.Count(output.String(), "resetting the data container only"); warnings != 1 { t.Fatalf("warning emitted %d times, want once", warnings) @@ -393,6 +394,55 @@ func TestSimctlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { } } +// TestClearStateStopsTheAppBeforeWipingItsContainer covers the ordering Launch +// used to hold. simctl uninstall copes with a running app; deleting the data +// container out from under one does not, and the CI iOS leg passes no app path +// so it is the wipe that runs. A run whose previous run was interrupted finds +// the app still up. +func TestClearStateStopsTheAppBeforeWipingItsContainer(t *testing.T) { + container := t.TempDir() + stale := filepath.Join(container, "Documents") + if err := os.Mkdir(stale, 0o755); err != nil { + t.Fatal(err) + } + log := scriptedXcrun(t, `"simctl terminate "*) :;; +"simctl get_app_container "*) echo `+container+`;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", output: &bytes.Buffer{}} + d.terminateApp = d.simctlTerminate + d.resetContainer = d.resetDataContainer + + if err := d.clearAppState(context.Background()); err != nil { + t.Fatalf("clearAppState: %v", err) + } + + want := []string{ + "simctl terminate SIM-UDID app.example", + "simctl get_app_container SIM-UDID app.example data", + } + if got := xcrunCalls(t, log); !slices.Equal(got, want) { + t.Fatalf("xcrun calls = %v, want %v: the app was still writing to the container being deleted", got, want) + } + if _, err := os.Stat(stale); !os.IsNotExist(err) { + t.Fatalf("stat %s = %v, want the previous run's state gone", stale, err) + } +} + +// TestClearStateSurvivesAnAppThatIsNotRunning holds the terminate to best +// effort. simctl exits non-zero when there is nothing to stop, and a first run +// on a fresh simulator must not fail on it. +func TestClearStateSurvivesAnAppThatIsNotRunning(t *testing.T) { + container := t.TempDir() + scriptedXcrun(t, `"simctl terminate "*) echo "No matching processes belonging to bundle identifier app.example"; exit 3;; +"simctl get_app_container "*) echo `+container+`;;`) + d := &Driver{udid: "SIM-UDID", bundleID: "app.example", output: &bytes.Buffer{}} + d.terminateApp = d.simctlTerminate + d.resetContainer = d.resetDataContainer + + if err := d.clearAppState(context.Background()); err != nil { + t.Fatalf("clearAppState: %v: an app that is not running is not a failure to clear", err) + } +} + func TestLaunchRejectsEnvironment(t *testing.T) { d := newTestDriver(&fakeCompanion{accessibilityJSON: "[]"}) err := d.Launch(context.Background(), "", false, map[string]string{"K": "V"}) From 220c7fe1d70c812e49fe437ee104edfad39b6c32 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:03:48 +0530 Subject: [PATCH 05/11] fix(ios): a device clear-state that cannot happen must fail --clear-data on a physical device with no --ios-app-path warned and then ran anyway, so the run started on the previous run's data while the flag said it started clean. There is no data-container wipe on a device, so there is nothing to fall back to. --- internal/driver/ioscompanion/device.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index de10374..1fa428f 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -243,13 +243,11 @@ func (d *Driver) devicectlReinstall(ctx context.Context) error { return nil } -// deviceResetContainerUnsupported warns once that device clear-state needs an -// app path for a devicectl reinstall: there is no simulator-style data-container -// wipe on a physical device. +// deviceResetContainerUnsupported ends the run: there is no simulator-style +// data-container wipe on a physical device, so a clear-state with no app path +// to reinstall from cannot happen. Warning and carrying on hands the run every +// previous run's data while the flag says it started clean. func (d *Driver) deviceResetContainerUnsupported(context.Context) error { - if !d.clearStateWarned { - fmt.Fprintln(d.output, "clear-state on a physical device requires --ios-app-path for a reinstall; skipping (state not cleared)") - d.clearStateWarned = true - } - return nil + return errors.New("clear-state on a physical device requires --ios-app-path for a reinstall; " + + "there is no data-container wipe on a device, so the run would start on the previous run's state") } From 99b9fe23f5eabafd0ea34c31817cb44f3201ea68 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:03:48 +0530 Subject: [PATCH 06/11] test(ios): a device clear-state without an app path ends the run --- internal/driver/ioscompanion/device_test.go | 35 +++++++++++++++------ 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index abc5ced..dfbf706 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -222,17 +222,32 @@ func TestDevicectlReinstallProceedsWhenNothingIsInstalled(t *testing.T) { } } -func TestDeviceClearStateWithoutAppPathWarnsOnce(t *testing.T) { - output := &bytes.Buffer{} - d := &Driver{output: output, deviceMode: true} - d.resetContainer = d.deviceResetContainerUnsupported - for i := 0; i < 2; i++ { - if err := d.deviceResetContainerUnsupported(context.Background()); err != nil { - t.Fatal(err) - } +// TestNewDeviceRefusesClearStateWithoutAnAppPath keeps the device from starting +// a run whose clear-state cannot happen. There is no data-container wipe on a +// physical device, so without an app path to reinstall from, carrying on hands +// the run every previous run's data under a flag that says otherwise. +func TestNewDeviceRefusesClearStateWithoutAnAppPath(t *testing.T) { + address := startLoopbackListener(t) + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-NO-APP-PATH" + options.ClearState = true + spawned := false + spawn := options.spawnRunner + options.spawnRunner = func(ctx context.Context, runnerAddress string) (*exec.Cmd, error) { + spawned = true + return spawn(ctx, runnerAddress) } - if got := bytes.Count(output.Bytes(), []byte("requires --ios-app-path")); got != 1 { - t.Fatalf("warning emitted %d times, want once", got) + + d, err := NewDevice(context.Background(), options) + if err == nil { + d.Close() + t.Fatal("NewDevice returned a driver whose clear-state never happened") + } + if !strings.Contains(err.Error(), "--ios-app-path") { + t.Fatalf("err = %v, want it to name the flag that makes the clear possible", err) + } + if spawned { + t.Fatal("the run started anyway; a clear-state that cannot happen must end the run, not open it") } } From 6beea910461aa9a0a7d9a8c44ab63dbac41727c5 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:04:58 +0530 Subject: [PATCH 07/11] fix(ios): the clear-state guard checks the bundle that was cleared A bool only said that something was cleared, so Launch(ctx, otherBundle, clearState=true) passed the guard and reported a reset that had reached a different app. Record what was cleared and compare against the bundle being launched. --- internal/driver/ioscompanion/device.go | 2 +- internal/driver/ioscompanion/driver.go | 20 +++++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 1fa428f..5d05982 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -83,7 +83,6 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { coreDeviceID: options.CoreDeviceID, bundleID: options.BundleID, appPath: options.AppPath, - clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, deviceMode: true, @@ -137,6 +136,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { d.Close() return nil, err } + d.clearedBundleID = options.BundleID } if err := d.bringUpDevice(ctx); err != nil { diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index c1abc1b..f629675 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -102,11 +102,12 @@ type Driver struct { appPath string output io.Writer - // clearStateAtStartup records that New (or NewDevice) reset the app to - // first-launch state before attaching, which is the only point in a run - // where clearing is safe. Launch refuses a clear-state request the driver - // was not built for rather than reinstalling under a live session. - clearStateAtStartup bool + // clearedBundleID names the app New (or NewDevice) reset to first-launch + // state before attaching, which is the only point in a run where clearing + // is safe. Launch refuses a clear-state request for anything else rather + // than reinstalling under a live session or reporting a reset that only + // ever reached another bundle. + clearedBundleID string screenWidth int screenHeight int @@ -233,7 +234,6 @@ func New(ctx context.Context, options Options) (*Driver, error) { udid: options.UniqueDeviceIdentifier, bundleID: options.BundleID, appPath: options.AppPath, - clearStateAtStartup: options.ClearState, output: output, doubleTapGapMilliseconds: gap, spawnChild: options.spawnChild, @@ -295,6 +295,7 @@ func New(ctx context.Context, options Options) (*Driver, error) { driverInstance.Close() return nil, err } + driverInstance.clearedBundleID = options.BundleID } if err := driverInstance.bringUp(ctx); err != nil { @@ -524,12 +525,13 @@ func (d *Driver) Launch(ctx context.Context, bundleID string, clearState bool, e // loudly rather than silently dropping the request. return errors.New("ios companion: launch with environment variables is unsupported on this backend") } - if clearState && !d.clearStateAtStartup { + if clearState && (d.clearedBundleID == "" || d.clearedBundleID != d.bundleID) { // Clearing here would uninstall and reinstall the app underneath a live // automation session, which is what races FrontBoard's registration and // leaves the session launching a bundle FrontBoard has not registered. - return errors.New("ios companion: clear-state must be requested when the driver is created (Options.ClearState); " + - "this backend clears the app before its automation session exists") + return fmt.Errorf("ios companion: clear-state must be requested when the driver is created (Options.ClearState) "+ + "for the bundle being launched; this backend cleared %q before its automation session existed, not %q", + d.clearedBundleID, d.bundleID) } // Terminate first so the launch is a clean cold start regardless of the From d1243543798eaaab7db7bf4e44592df465ab8c4b Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:04:58 +0530 Subject: [PATCH 08/11] test(ios): a clear-state launch for an uncleared bundle is refused --- internal/driver/ioscompanion/driver_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/driver/ioscompanion/driver_test.go b/internal/driver/ioscompanion/driver_test.go index b4f66ad..21e85a8 100644 --- a/internal/driver/ioscompanion/driver_test.go +++ b/internal/driver/ioscompanion/driver_test.go @@ -319,6 +319,25 @@ func TestLaunchRefusesClearStateTheDriverWasNotBuiltFor(t *testing.T) { } } +// TestLaunchRefusesClearStateForABundleItDidNotClear holds the guard to the +// fact it is guarding. A driver built to clear one bundle has cleared nothing +// for another, so reporting that launch as a clear-state launch is a reset the +// caller was told happened and did not. +func TestLaunchRefusesClearStateForABundleItDidNotClear(t *testing.T) { + companion := &fakeCompanion{accessibilityJSON: "[]"} + d := newTestDriver(companion) + d.clearedBundleID = "com.example.app" + + err := d.Launch(context.Background(), "com.other.app", true, nil) + + if err == nil || !strings.Contains(err.Error(), "clear-state") { + t.Fatalf("Launch err = %v, want a refusal naming clear-state: com.other.app was never cleared", err) + } + if indexOf(companion.recorded(), "launch") >= 0 { + t.Fatalf("a launch reporting a clear that never happened must not reach the companion; got %v", companion.recorded()) + } +} + func TestNewRejectsClearStateWithoutBundleID(t *testing.T) { probe := &clearStateProbe{} options := clearStateOptions(t, probe, "NO-BUNDLE-UDID", true) From 01f9ea942cd6b301b56406ac281e1847d9354280 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:05:59 +0530 Subject: [PATCH 09/11] fix(ios): one address picker for every bring-up bringUpRunner reads the picker from a field, and NewDevice only ever set the device one, so a device driver that reached bringUpRunner would call nil. The two fields held the same function; keeping one leaves no path that can be wired without it. --- internal/driver/ioscompanion/device.go | 6 +++--- internal/driver/ioscompanion/driver.go | 25 ++++++++++++++----------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/internal/driver/ioscompanion/device.go b/internal/driver/ioscompanion/device.go index 5d05982..b8dbdff 100644 --- a/internal/driver/ioscompanion/device.go +++ b/internal/driver/ioscompanion/device.go @@ -103,9 +103,9 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { } } if options.pickAddress != nil { - d.pickDeviceAddress = options.pickAddress + d.pickRunnerAddress = options.pickAddress } else { - d.pickDeviceAddress = pickLoopbackAddress + d.pickRunnerAddress = pickLoopbackAddress } // Device seams: clear-state reinstalls via devicectl; the container reset and @@ -159,7 +159,7 @@ func NewDevice(ctx context.Context, options DeviceOptions) (*Driver, error) { // health. The build runs inside spawnRunner under the process context, so the // startup timeout only bounds the post-spawn wait, not the build. func (d *Driver) bringUpDevice(ctx context.Context) error { - address, err := d.pickDeviceAddress() + address, err := d.pickRunnerAddress() if err != nil { return err } diff --git a/internal/driver/ioscompanion/driver.go b/internal/driver/ioscompanion/driver.go index f629675..aa12589 100644 --- a/internal/driver/ioscompanion/driver.go +++ b/internal/driver/ioscompanion/driver.go @@ -157,23 +157,26 @@ type Driver struct { // lifecycle, screenshot) with an in-simulator runner that serves // collapse-free accessibility snapshots and native unicode typing. // runnerClient is nil on the legacy-only path. - runnerClient transport.Companion - runnerChild *exec.Cmd - runnerAddress string - spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) - dialRunner func(address string) (transport.Companion, error) + runnerClient transport.Companion + runnerChild *exec.Cmd + runnerAddress string + spawnRunner func(ctx context.Context, address string) (*exec.Cmd, error) + dialRunner func(address string) (transport.Companion, error) + hybrid bool + + // pickRunnerAddress hands every bring-up a free loopback port, on the + // simulator and the device alike. One field, so no path can be wired + // without it. pickRunnerAddress func() (string, error) - hybrid bool // Device-mode fields. On the physical-device path d.companion is the runner // dialed over a usbmux tunnel, hybrid is false, and runnerClient is nil. // coreDeviceID feeds devicectl; tunnel is the in-process usbmux forwarder // bridging the host loopback port to the runner's device-side port. - deviceMode bool - coreDeviceID string - tunnel io.Closer - startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) - pickDeviceAddress func() (string, error) + deviceMode bool + coreDeviceID string + tunnel io.Closer + startTunnel func(ctx context.Context, hardwareUDID, localAddress, devicePort string) (io.Closer, error) // processContext owns the companion child's lifetime: it is derived from // New's context (so a canceled run still reaps the child) and canceled by From cb45508e7214c305771aad23a755a4d27726d9dd Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:05:59 +0530 Subject: [PATCH 10/11] test(ios): a device driver can bring a runner up --- internal/driver/ioscompanion/device_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index dfbf706..6a58be9 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -83,6 +83,25 @@ func TestNewDeviceWiresRunnerOnlyMode(t *testing.T) { } } +// TestNewDeviceWiresEveryBringUpsAddressPicker keeps the device driver whole. +// bringUpRunner reads its picker from a field rather than calling the package +// function, and NewDevice left that field nil, so the only thing standing +// between a device run and a nil call was which restart path happened to run. +func TestNewDeviceWiresEveryBringUpsAddressPicker(t *testing.T) { + address := startLoopbackListener(t) + options := testDeviceOptions(address, newDeviceCompanion()) + options.HardwareUDID = "00008140-PICKER" + d, err := NewDevice(context.Background(), options) + if err != nil { + t.Fatalf("NewDevice: %v", err) + } + defer d.Close() + + if err := d.bringUpRunner(context.Background()); err != nil { + t.Fatalf("bringUpRunner: %v", err) + } +} + func TestNewDeviceRequiresIdentifiers(t *testing.T) { if _, err := NewDevice(context.Background(), DeviceOptions{CoreDeviceID: "x"}); err == nil { t.Fatal("missing HardwareUDID must error") From 4406d3b5abef2a2640ce38dff62ae2550725429e Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 01:08:51 +0530 Subject: [PATCH 11/11] test(ios): name the picker test for what it covers --- internal/driver/ioscompanion/device_test.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/internal/driver/ioscompanion/device_test.go b/internal/driver/ioscompanion/device_test.go index 6a58be9..15bf23a 100644 --- a/internal/driver/ioscompanion/device_test.go +++ b/internal/driver/ioscompanion/device_test.go @@ -83,11 +83,11 @@ func TestNewDeviceWiresRunnerOnlyMode(t *testing.T) { } } -// TestNewDeviceWiresEveryBringUpsAddressPicker keeps the device driver whole. -// bringUpRunner reads its picker from a field rather than calling the package -// function, and NewDevice left that field nil, so the only thing standing -// between a device run and a nil call was which restart path happened to run. -func TestNewDeviceWiresEveryBringUpsAddressPicker(t *testing.T) { +// TestNewDeviceWiresTheAddressPickerEveryBringUpUses keeps the device driver +// whole. bringUpRunner reads its picker from a field rather than calling the +// package function, and NewDevice left that field nil, so the only thing +// standing between a device run and a nil call was which restart path ran. +func TestNewDeviceWiresTheAddressPickerEveryBringUpUses(t *testing.T) { address := startLoopbackListener(t) options := testDeviceOptions(address, newDeviceCompanion()) options.HardwareUDID = "00008140-PICKER"