Repository navigation
Conversation
Decisions from the PR #16 follow-up list: a 10 s drain window with derived Remove and Ending timeouts, JVM tests through small seams for /proc scanning and the service, an app-scoped runner for install, remove and test, and no "0 sessions running" notification.
SessionService now executes the actions a plain ServiceState returns for each start and running-list change: start foreground first, notify, wake lock, and stop with the last start id once foreground and empty. JVM tests cover the immediate stop, a start arriving while stopping, a conflated empty list and the wake lock. A start that finds no session titles its notification "Stopping..." instead of "0 sessions running".
ProotProcesses holds ProotPty's scan (TracerPid tracees, the children files of every task, the stat-scan fallback, alive) and reads through a small ProcFs interface with the real /proc as the default. JVM tests with a fake process table cover tracee selection, another proot's tracees, a daemon reparented to init, the descendant-walk fallback, a pid vanishing mid-scan, zombies, and the force-kill order.
End now waits up to 10 s for the signalled programs to exit, still stopping as soon as they have. ManagedSession defines End's worst case (17.5 s) and the timeout derived from it (20 s), which Remove's wait and the "Ending the previous session" give-up now use instead of fixed 10 s.
ProcFs gains exists(pid), so a pid whose status is unreadable while /proc/<pid> still exists forces the descendant walk again, as before the seam. The fake process table now vanishes a pid after a chosen listing and records reads made after it vanished, so one test removes a pid between the status listing and its read and another during the fallback walk. The runningTitle tests move to SessionNotificationsTest.
The slowest-End test now uses 400 ms steps and expects the sum of the worst case's steps, without the scan margin, so dropping any term from endWorstCaseMs fails it. The 4 s drain-window test is dropped: early stopping and the 10 s default are covered elsewhere.
Every step of the slowest End is now 1 s, the kill-exit wait included, which becomes a constructor parameter like the other waits, and the upper slack is 800 ms. The slack stays below every step, so a step missing from endWorstCaseMs still fails the test.
Install, remove and test from the list ran in the screen's coroutine scope with their output in remember state, so rotating or switching dark mode dropped the output, the busy state and Remove's session wait while pr-cli kept running, and a second operation could start. DistroOperations, owned by App, runs one pr-cli operation at a time through a DistroCommand seam (PrCliCommand, the former runDistroCommand) and exposes it as a StateFlow. Remove ends a running session first and waits up to END_TIMEOUT_MS. MainActivity only renders the state; the custom image fields use rememberSaveable.
The design and architecture docs describe DistroOperations, how the list renders it, Remove's stopping line, and its JVM tests.
Material3 settles a swiped sheet to Hidden before it calls onDismissRequest, so refusing dismiss() there left a hidden sheet whose popup swallowed every touch until the operation finished. The sheet state now refuses Hidden while the operation runs, which also covers scrim taps and Back; once it has finished they dismiss and refresh the list as before.
PrCliCommand closes the process output when the read ends. The worst-case End test now allows the whole derived worst case, scan margin included, so a loaded CI host does not fail it; dropping a step from the formula still fails it. The timeout test no longer restates the constants. The docs say a list operation does not survive the process being killed, and integration-tests.md names the runner instead of the removed runDistroCommand.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from #16, planned in
docs/plans/background-sessions-followups.md.What changes
ManagedSession, instead of separate fixed 10 s values.DistroOperations, owned byApplikeSessionManager). Before, they ran in the list screen's own coroutine scope with their state inremember. A rotation or a dark-mode switch therefore lost the output and the busy state whilepr-clikept running, and a second operation could start on top. Now the list only renders the runner's state, one operation runs at a time, and the custom-image fields userememberSaveable.runDistroCommandmoved toPrCliCommand.ServiceState), whichSessionServiceexecutes. The running notification is never titled "0 sessions running"; a login that fails at once now shows "Stopping…" for the moment it is visible./procscanning goes through a seam (ProcFs/ProotProcesses), with the real/procas the default. The existence check is kept exactly as before (exists, a sixth read beside the five the plan listed).Verification
Not covered
/proc. A refactor that changes it makes them fail loudly, not pass silently.pr-cli remove(for example from a terminal in split screen waiting on "Ending…") would run under the removal. Worth its own follow-up.SessionServiceandPrCliCommandthemselves (the Android calls) remain device-verified only. Their decisions and line handling are JVM-tested.