Skip to content

feat(maestro): support killApp via close mode - #2749

Open
Rohit3523 wants to merge 3 commits into
callstack:mainfrom
Rohit3523:feat/maestro-killapp-impl
Open

Rohit3523 wants to merge 3 commits into
callstack:mainfrom
Rohit3523:feat/maestro-killapp-impl

Conversation

@Rohit3523

Copy link
Copy Markdown
Contributor

Summary

Supports Maestro killApp (system-initiated process death) end to end, mirroring stopApp plumbing. On Android it dispatches adb shell am kill; every other platform aliases stopApp through the shared close dispatcher (Interactor.kill fallback). No new public CLI surface — killApp stays inside the Maestro compatibility layer.

  • contracts: optional Interactor.kill, CloseApplicationInput.mode, kill-with-fallback in invokeApplicationClose
  • android: killAndroidApp (am kill) wired onto the interactor
  • maestro: killApp IR kind, parser, runtime port, daemon projection to an app-only close carrying killApp dispatch
  • daemon: replay dispatch folds killApp; session close passes mode: kill into closeApplication
  • upstream/116_kill_app now classifies identical; declared divergence removed. Touched 30 files.

Validation

Tested commit 369a51b35. Focused suites pass: contracts interaction, maestro parser/runtime-port/daemon-port/conformance-verify (node --test, 14 pass), android app-lifecycle, session-close, replay-maestro-request, fuzz arbitraries, maestro help/docs sync. pnpm check:quick (lint + typecheck) passes. pnpm check:affected --run: fallow audit clean on changed files; complexity gate shows only the 2 inherited findings after table-driving the lifecycle dispatch. Unresolved: check:production-exports reports 68 unused exports — reproduced identical on origin/main, pre-existing and unrelated. No live-device run; risk is Android am kill semantics on a real emulator (unit asserts the exact argv).

Maestro killApp triggers system-initiated process death (adb shell am
kill on Android) instead of stopApp's force-stop. On other platforms it
aliases stopApp through the shared close dispatcher.

- contracts: optional Interactor.kill, CloseApplicationInput.mode, with
  fallback to close in invokeApplicationClose
- android: killAndroidApp (am kill) wired onto the interactor
- maestro: killApp IR kind, parser, runtime port, daemon projection to
  an app-only close carrying killApp dispatch, conformance canonical
- daemon: replay dispatch folds killApp; session close passes
  mode kill into closeApplication
- upstream/116_kill_app now classifies identical; divergence removed
executeLifecycleCommand grew a sixth case for killApp and tripped the
complexity gate. The stopApp/killApp/clearState legs share one shape,
so dispatch them through a lookup instead of three switch cases.
am kill only reaps background processes, so a foreground killApp used
to succeed without killing anything. Refuse a foreground target naming
the background-first precondition, and verify the process is gone after
the kill instead of reporting an unproven success.
@thymikee

Copy link
Copy Markdown
Member

Reviewed 78727e3. I found no code defect, but the kill route has only run against adb fakes, and I have one smaller-design question.

The route (Maestro replay → close with closeAppOnly + killApp → Android closeApplication with mode kill → killAndroidApp: focus check, am kill, pidof check) is exercised only through adb exec fakes, and the PR body names real-emulator am kill behaviour as the open risk (app-lifecycle.ts#L510). Two things are unproven: that am kill reaps an app sent to the background with Home, and that the post-kill pidof check does not misread a cached process that is still alive. Can you run a live Android emulator replay through agent-device replay: launchApp → pressKey Home → killApp → launchApp with stopApp false? It passes when killApp succeeds, adb shell pidof <pkg> prints nothing after it, and the relaunch is a cold start. In the same session, launchApp → killApp with the app in the foreground should fail with android-kill-requires-background-app. Please paste both outputs.

Could this be smaller? Only the Android lifecycle owner reads mode. Could packages/platform-android/src/lifecycle.ts call killAndroidApp directly when mode === 'kill', instead of adding Interactor.kill and a fallback branch to the shared contracts type? That would also make it explicit that owners that ignore mode keep force-stop, which matches what Maestro does on iOS. What would have to change first?

Not blocking: the support matrix and replay-e2e.md do not say that a foreground killApp is refused rather than a silent no-op; a process that survives am kill while in the background (for example a foreground service) gets the same android-kill-requires-background-app reason, which is misleading; and mode?: 'stop' | 'kill' is declared twice although nothing writes 'stop'.

All checks are green and there are no conflicts. I could not check whether upstream Maestro's killApp also refuses a foreground app.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants