Skip to content

fix: leave the terminal on Back - #11

Merged
oonid merged 2 commits into
mainfrom
fix/terminal-back
Oct 17, 2026
Merged

oonid merged 2 commits into
mainfrom
fix/terminal-back

Conversation

@oonid

@oonid oonid commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Back never left the terminal: not the navigation button, not the gesture, not a Back key event. Leaving needed Home or the recents screen.

TerminalActivity's OnBackPressedCallback never ran because the activity never saw a complete Back press:

  • with the keyboard up, the IME takes the key-down to hide itself, so only the key-up reaches the activity;
  • with the keyboard hidden, Compose takes the key-down to move focus from the terminal library's input view to the Compose root (logged in dispatchKeyEvent: code=4 action=0 handled=true focus=AndroidComposeView).

TerminalActivity.dispatchKeyEvent now handles Back before the view tree: a key-down followed by a key-up runs the back callback (cleanup and finish()), and a lone key-up (the press the IME used to hide itself) is ignored. The first Back still hides the keyboard; the next one leaves. With a text selection active (long press), Back clears it through termlib's public SelectionController (onSelectionControllerAvailable) and stays, as main already did when the key reached termlib; the next Back leaves. docs/android-apk-architecture.md describes it. vendor/termlib is unchanged.

Verification (Android 16, Samsung)

On main, three Back presses (adb key event) left the terminal open, with a live session and after exit. With this change:

  • Back with the keyboard up: the first press hides it (mInputShown=false), the second returns to the distro list.
  • The navigation bar's Back button: same.
  • After exit in the shell: two presses return to the list.
  • With a selection: Back clears it (handles and Copy bubble gone) and keeps the session; the next Back leaves.
  • The session's proot child is gone after leaving (one child before, none after).
  • Integrated with main (fix: map the app's cache group like its real group #10): pr-cli test alpine and pr-cli test ubuntu 55/55, pit all from an in-app terminal 55/55.

Session lifetime (documented here, built in a follow-up)

Back now ends the session, through the existing callback (cleanup()). Before this change the key never reached the callback, so Back left the terminal open and the session alive; an earlier lifecycle check that "survived Back" was measuring that. Because pr-cli starts proot with --kill-on-exit, nothing started inside the session outlives it (nohup, &, setsid do not help).

Action Session
Back (after the keyboard is hidden) Ends
exit in the shell Ends
Home, or switching apps Keeps running until Android kills the app's process
Swiping the app away Ends

So a long-running program such as the Hermes gateway needs Home, not Back, and still dies when Android reclaims the process. Options for keeping a session after leaving its screen, recorded in android-apk-architecture.md ("Session lifetime and Back"):

  • Back acts like Home (moveTaskToBack): trivial, but lands on the launcher, not the distro list.
  • Sessions owned outside the activity: an app-wide session manager owns the PTY, the emulator and the reader thread (which must keep draining output with no screen open); reopening a distro reattaches, the list shows it running, ending becomes an explicit action.
  • User choice on top: a setting, a prompt on Back, or a per-session detach. The choice is small; keeping the session is the work.

This PR keeps the documented design (Back ends the session). Background sessions come in a follow-up together with the foreground service, which needs the same session manager and is what stops Android from killing the process.

The app module has no instrumented-test setup, so there is no automated test for this.

@oonid
oonid force-pushed the fix/terminal-back branch from 212a175 to 0352340 Compare October 17, 2026 10:33
Back never reached TerminalActivity's OnBackPressedCallback, from the
navigation button, the gesture, or a key event. With the keyboard up,
the IME takes Back's key-down to hide itself and the activity sees a
lone key-up. With it hidden, Compose takes the key-down to move focus
from the terminal library's input view to the Compose root
(dispatchKeyEvent: action=0 handled=true, focus=AndroidComposeView).
Either way the activity never got a full press, so the terminal
stayed open; leaving needed Home or the recents screen.

TerminalActivity now handles Back before the view tree: a key-down
followed by a key-up runs the back callback (cleanup and finish), and
a lone key-up, the press the IME used to hide itself, is ignored. The
first Back still hides the keyboard; the next one leaves. With a text
selection active, Back clears it through termlib's SelectionController
and stays, as it did before when the key reached termlib.
@oonid
oonid force-pushed the fix/terminal-back branch from 0352340 to fc403d1 Compare October 17, 2026 11:43
With Back working, Back ends the terminal session through the existing
cleanup(); before, the key never reached the callback, so Back left the
session alive. android-apk-architecture.md now lists how a session ends
(Back, exit, Home, swipe-away), why nothing inside the container
survives (--kill-on-exit), and the options for keeping a session after
leaving its screen: Back as Home, sessions owned by an app-wide
manager, and a user choice between ending and keeping. The follow-up
is planned with the foreground service.
@oonid
oonid merged commit da90d46 into main Oct 17, 2026
1 check passed
@oonid
oonid deleted the fix/terminal-back branch October 17, 2026 12:02
oonid added a commit that referenced this pull request Oct 17, 2026
build_proot_args_non_isolated_includes_prefix_and_mounts pointed TMPDIR
at <its temp dir>/cache and then removed that temp dir. The other tests
build their scratch paths with std::env::temp_dir(), which reads TMPDIR,
and 25 of them do not take the env lock. One that ran in that window
created its files inside the doomed tree and lost them:
read_helpers_report_expected_access failed with
`assertion failed: can_read_file(...)` on main's CI after #11, and in
1 of 40 local runs.

Both tests that set TMPDIR now set it to the value temp_dir() already
returns, so they still check that TMPDIR wins over the
$APP_PREFIX/tmp fallback while no other test sees a different temp
dir (build_proot_runtime_env_prefers_tmpdir_when_set used
/tmp/custom-cache, where tests running in its window left
directories), and they restore the caller's TMPDIR instead of
removing it. 0 failures in 240 local runs with TMPDIR unset and 60
with it set.
oonid added a commit that referenced this pull request Oct 17, 2026
build_proot_args_non_isolated_includes_prefix_and_mounts pointed TMPDIR
at <its temp dir>/cache and then removed that temp dir. The other tests
build their scratch paths with std::env::temp_dir(), which reads TMPDIR,
and 25 of them do not take the env lock. One that ran in that window
created its files inside the doomed tree and lost them:
read_helpers_report_expected_access failed with
`assertion failed: can_read_file(...)` on main's CI after #11, and in
1 of 40 local runs.

Both tests that set TMPDIR now set it to the value temp_dir() already
returns, so they still check that TMPDIR wins over the
$APP_PREFIX/tmp fallback while no other test sees a different temp
dir (build_proot_runtime_env_prefers_tmpdir_when_set used
/tmp/custom-cache, where tests running in its window left
directories), and they restore the caller's TMPDIR instead of
removing it. 0 failures in 240 local runs with TMPDIR unset and 60
with it set.
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.

1 participant