From 3977f6887065e2e5d985533b6ead52a6d2e9715e Mon Sep 17 00:00:00 2001 From: Jaap-Jan de Wit | DodoTech Date: Sun, 9 Aug 2026 10:14:24 +0200 Subject: [PATCH] Record the keep-alive corrections in the port notes and the manual checks The port doc's backgrounding decision now carries the four corrections rather than describing a wiring that was not true, and Phase 14 gains the checks a phone can actually run: a backgrounded shell surviving, an idle Files connection surviving, the permission ask arriving at the first thing worth showing, and a refusal costing the notification and nothing else. --- docs/android-port.md | 41 ++++++++++++++++++++++++++++++++--- docs/manual-checks.md | 50 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 4 deletions(-) diff --git a/docs/android-port.md b/docs/android-port.md index 6a137b4..de71e79 100644 --- a/docs/android-port.md +++ b/docs/android-port.md @@ -30,7 +30,10 @@ verified is that it compiles, links, packages, and carries the right natives. transfers protected by a **foreground service**. File transfer is not in the first scope; when it arrives it is **one remote pane** with Android's document picker for moving files in and out. *It has since arrived, both ways:* the pane, the queue, `ACTION_OPEN_DOCUMENT` going in and `ACTION_CREATE_DOCUMENT` coming out, -with the foreground service now counting transfers as well as shells. +with the foreground service now counting transfers as well as shells — and, since, an idle-but-connected +Files session as well, which a transfer count alone was blind to. *Corrected the same round:* the service's +other half — a shell's own opening — had never been wired to anything at all, so a shell survived only for +as long as the app stayed foreground; see [Sessions survive backgrounding](#sessions-survive-backgrounding-via-a-foreground-service). **What was actually checked**, so the rest can be read with the right amount of trust: @@ -280,13 +283,40 @@ What is desktop-only is the *left* pane — `LocalDirectory`, the drive list, th ### Sessions survive backgrounding, via a foreground service -A persistent notification for as long as a shell or a transfer is live. +A persistent notification for as long as a shell, a transfer, or an idle-but-connected Files session is +live. It costs the user a notification and some battery. It buys the behaviour the desktop client already promises and documents — that a shell outlives a vault lock, and that a transfer finishes — and the alternative was to make `TerminalWorkspace`'s guarantee desktop-only, which is a worse thing to have to write down than a notification is to look at. +**Three corrections found after the first cut shipped, all in the wiring rather than the design:** + +- **A shell opening never started the service.** `SessionKeepAlive` heard `TerminalWorkspace.SessionEnded` + and refreshed on that, but nothing announced the opposite event — so a user who opened a shell and + backgrounded the app immediately had no foreground service at all, and Android was free to kill the + process holding it. `MainWindowViewModel.TerminalSessionOpened` is now wired the same way in + `App.axaml.cs`'s `ComposeKeepAlive`. +- **A connected-but-idle Files session counted as nothing.** A host open on the Files screen with no + transfer moving is a live SFTP connection a dying process would sever, and the old two-argument + `Reconcile(liveSessions, activeTransfers)` had no way to hear about it. `TransfersViewModel.HasLiveFileSession` + — `IsConnected` with a real `ConnectedCipher`, which a bucket never has — is the third fact `Reconcile` now + takes. +- **Refreshing the notification restarted the service, which throws when backgrounded.** `Reconcile` called + `StartForegroundService` on every refresh, including the common case of a service that was already + running. On API 31+ that throws `ForegroundServiceStartNotAllowedException` the instant the app is + backgrounded — a transfer finishing in the pocket, one of two shells dying — which crashed the process and + took every session with it. `SessionForegroundService` now tracks whether it is already running and, when + it is, posts the updated notification through `NotificationManager.Notify` instead of asking Android to + start anything. + +**The notification permission is requested, not just declared.** API 33+ requires `POST_NOTIFICATIONS` at +runtime or the receipt is silently invisible — the service still runs, but nothing on screen says so. +`SessionForegroundService.Reconcile` asks for it the first time in this process there is actually something +to show, at most once, with no result read back: a refusal costs the notification and nothing else, which is +what the manifest's own comment on the permission says. + ### Phone first About 360dp wide. The tablet route was cheaper — a landscape tablet is close to the existing 880×560 minimum @@ -523,7 +553,12 @@ go at 360dp: stopping it from a count rather than a lifecycle. `TerminalWorkspace.LiveSessionCount` is the source of truth deliberately: it already knows that a session whose shell exited is not live, which a counter incremented on open would not, and a phone showing "1 shell connected" over nothing would be exactly the - dishonesty the unlock screen's count exists to prevent. + dishonesty the unlock screen's count exists to prevent. *Corrected since:* the opened half of a shell's + lifecycle was never wired in, so the service could never come up for a shell at all; an idle-but-connected + Files session now counts as a third live fact rather than nothing; a refresh while backgrounded updates + the notification in place instead of restarting the service, which the API throws on; and + `POST_NOTIFICATIONS` is now actually requested rather than merely declared. See + [Sessions survive backgrounding](#sessions-survive-backgrounding-via-a-foreground-service) for all four. 7. ~~**The interface**, phone-first.~~ **Done for the decided scope** — all seven screens of the design, plus the two states the design does not draw because it starts at an enrolled phone (naming a server, and choosing a passphrase). diff --git a/docs/manual-checks.md b/docs/manual-checks.md index e824381..49d6bcf 100644 --- a/docs/manual-checks.md +++ b/docs/manual-checks.md @@ -2075,9 +2075,57 @@ Queue several files in each direction, put the phone to sleep with the screen of notification goes away when the last one does — with no shell open. With a shell open it stays, because that is what it was already for. +Now, separately: open a shell to the host, press the home button (backgrounding rather than sleeping — the +distinction matters, because backgrounded is the state in which Android is free to kill a process no +foreground service is protecting), wait thirty seconds with the shell doing nothing, and return. + +**Pass:** the notification stayed up the whole time, and the shell is exactly where it was — same scrollback, +same prompt — with typing reaching the host immediately. Exit the shell. + +**Pass:** the notification goes with it, once nothing else is open. + **Failure means:** an upload that stalls with the screen off is the count not reaching `SessionForegroundService`, and Android has stopped the process mid-transfer. A notification left up -afterwards is `ActivityChanged` not being subscribed — the other end of the same wire. +afterwards is `ActivityChanged` not being subscribed — the other end of the same wire. A shell that has +disconnected on return is `MainWindowViewModel.TerminalSessionOpened` never reaching `SessionKeepAlive` — the +service only ever heard about a shell *ending*, so it never came up for one in the first place. + +### 14.6a A Files connection with nothing moving still survives backgrounding + +Connect to a host on the Files screen with no transfer queued — just browse to somewhere and stop. Note the +directory shown, then background the app, wait thirty seconds, and return. + +**Pass:** the notification stayed up the whole time (check the shade if the return is too quick to see it +directly), and the pane is exactly where it was — the same listing, the same breadcrumb — with no reconnect +needed. + +**Failure means:** `TransfersViewModel.HasLiveFileSession` not reaching `SessionKeepAlive`, so an idle but +still-open SFTP connection read as nothing running at all and the process was free to die under it. + +### 14.6b The notification permission is asked for once, at the first thing worth showing · **needs Android 13+** + +On a device running Android 13 or later, on a fresh install that has never connected to anything, open a +shell or the Files screen for the first time. + +**Pass:** a system dialogue asking to allow notifications appears at that moment — not at launch, and not +before this first connect. Answer it either way; the connection completes regardless, and background the app +afterwards to confirm nothing else changed about it. + +**Failure means:** the dialogue appearing at launch is asking before there is anything on screen to justify +it. Never appearing at all on API 33+ is the harder failure to notice, because nothing else surfaces it — +the service still starts and still holds the process open, only the receipt is invisible. See +`SessionForegroundService.RequestNotificationPermission`. + +### 14.6c Refusing the permission costs the notification and nothing else + +Continuing from 14.6b: choose **Don't allow** on the system dialogue. Queue a transfer, or open a shell, and +background the app. + +**Pass:** no notification appears anywhere, but the transfer still finishes, or the shell is still there on +return, exactly as in 14.1–14.6a. + +**Failure means:** anything disconnecting or failing here is the permission refusal being read as though it +had refused the service itself, rather than only the notification Android draws for it. ### 14.7 SAVE FILE writes where you pointed it, and the file opens