10 Commits
Author SHA1 Message Date
jaap-jan cc8bf37321 Merge branch 'claude/terminal-reattach'
ci / build and test (push) Successful in 2m30s
ci / android head (push) Successful in 3m25s
ci / desktop nightly (push) Successful in 37s
ci / api image (push) Successful in 37s
Brings the Android keep-alive corrections and the terminal renderer
reattach: the foreground service now actually comes up for shells and
an idle Files session, survives refreshes from the background, and the
terminal's data plane lets a reloaded WebView page take its socket back
over instead of freezing every session behind a dead one.
2026-08-09 10:59:01 +02:00
jaap-jan 3f5979d639 Record the renderer-reattach correction and its phone checks
The port notes carry the third correction of this round: the data
plane assumed a renderer that attaches once and lives forever, which no
foreground service can make true of Android's separate WebView renderer
process. Phase 11 gains the two checks a phone can run — close and
reopen a connection, and a backgrounded shell surviving its renderer
being killed, banner and all.
2026-08-09 10:54:45 +02:00
jaap-jan aaff81272a Teach the page and the shell to put a reattached view back together
The page's socket now retries itself forever with backoff — a dropped
socket is an ordinary event on a phone, not the end of the terminal's
life — and createSession is idempotent, so a replay landing on a pane
that survived changes nothing. A replay creating a pane that did not
survive writes one dim line saying the earlier output stayed on the
host, because that is the truth about a reloaded page's scrollback.

The shell answers RendererReattached with the two things only it owns:
the font size, and which tab is active.
2026-08-09 10:54:38 +02:00
jaap-jan 4d1f07f253 Replay the live sessions to a renderer that just attached
Each live session gets its credit window reset — the unacknowledged
bytes died with the old page, and their acknowledgement is never
coming — and its SessionOpened frame again, flagged as a replay so the
page can tell a reattach from a genuinely new session. A session whose
shell already ended gets nothing: its scrollback lived only in the page
that is gone, and a frame implying otherwise would lie.

RendererReattached is the seam for what the workspace has no business
owning: the font size and the selected tab live in the shell, which
re-pushes them from its own subscription.
2026-08-09 10:54:29 +02:00
jaap-jan 095774c498 Take a returning renderer's socket over instead of refusing it
One attach per process was WebView2's truth, not Android's: the phone
kills the WebView's renderer independently of the app process, the page
reloads, and its fresh socket was answered 409 by a guard that never
reset — with no way back short of restarting the app. Only our own page
knows the token, so a second valid upgrade is that page returning; it
now displaces the old socket, which may never notice it is dead on its
own, since a killed renderer sends no FIN.

A send into the dead socket also no longer escapes as a fault. It used
to unwind the pump's flush loop, after which nothing drained the credit
window and the still-live shell froze behind it for good — including
the BCL quirk where such a send surfaces as an OperationCanceledException
nobody's token asked for.
2026-08-09 10:54:20 +02:00
jaap-jan 3977f68870 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.
2026-08-09 10:14:24 +02:00
jaap-jan 48ea5e22d5 Actually keep the phone's sessions alive when the app is backgrounded
The foreground service existed, and four defects in its wiring meant it
mostly did not run. A shell opening was never announced to it — only the
ending was — so the service never came up for a shell at all. An idle
connected Files session counted as nothing. Every refresh restarted the
service, which Android 12+ answers with a crash the moment the app is
backgrounded — a transfer finishing in the pocket took the remaining
connections with it. And POST_NOTIFICATIONS was declared but never
requested, so on Android 13+ the receipt was silently invisible.

Updates while backgrounded now go through the notification manager; a
foregrounded refresh still prefers a real start, so a stop still in
flight cannot leave an orphan receipt over an unprotected process.
2026-08-09 10:14:17 +02:00
jaap-jan 810bc48d3f Tell the keep-alive wire when the Files session opens and closes
HasLiveFileSession answers the phone's foreground-service question — is
there a connection here that dying with the process would sever — and a
bucket answers no, because HTTP holds nothing open. ActivityChanged now
also fires at the end of MarkHostConnected and CloseSessionAsync, where
both facts it reads are finally true together.

Also makes the bucket pins test actually open a bucket: it never set
Remote, so CONNECT dialled the auto-selected host, and its assertions
passed only because that host had no pins either.
2026-08-09 10:14:10 +02:00
jaap-jan 671611a9a0 Merge branch 'claude/phone-pins' 2026-08-09 09:46:50 +02:00
jaap-jan dbf6ce1bcf Give the phone its pins: an editor section and chips on Files
The data was never the gap — HostSecret.PinnedPaths syncs and merges on both
heads, and the desktop's drawer has staged it since v5 — the phone just had
nowhere to add, remove or use a pin. Now it has both halves.

The host editor page gains a QUICK ACCESS section over the same shared
staging the drawer binds (EditorPinnedPaths, AddEditorPin, RemoveEditorPin),
with the remove target at this head's 44dp touch floor rather than the
desktop's 22-pixel close box, and no folder glyph because this head embeds no
icon font for one. The page also gains a Status line of its own: the add
command's five refusals speak through Status, and this page covers the screen
that normally draws it — a refusal nothing shows is no refusal at all.

The Files screen draws the connected host's pins as chips between the
breadcrumb and the listing, each running GoRemoteCommand exactly as a crumb
does. They are captured at connect, like ConnectedTo and the session facts
before them; a bucket gets none, having no HostSecret to pin anything on.
Covered headlessly in ShellFlowTests — connect populates, disconnect clears,
a bucket stays empty — and by manual checks 8.18 and 8.19, whose phase
preamble also stops claiming thirteen checks when it lists twenty-one.
2026-08-08 23:09:57 +02:00
18 changed files with 1429 additions and 98 deletions
+69 -3
View File
@@ -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 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, 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, 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: **What was actually checked**, so the rest can be read with the right amount of trust:
@@ -235,6 +238,37 @@ The parts that are definitely different are the on-screen keyboard, and the fact
needs Ctrl, Esc, Tab and arrows that the software keyboard does not offer — every Android SSH client ships an needs Ctrl, Esc, Tab and arrows that the software keyboard does not offer — every Android SSH client ships an
accessory key row for this. That is UI work, not porting. accessory key row for this. That is UI work, not porting.
> **⚠️ Corrected by the build. The data plane assumed a renderer that attaches once and lives forever, and
> that assumption is WebView2's truth, not Android's.** Desktop's WebView2 process starts with the window and
> dies with it; `TerminalDataPlane` was written to that reality — one socket, attached once,
> `Interlocked.Exchange`-guarded against a second attach ever happening at all. On a phone the WebView's own
> renderer process is a separate thing from the app process the foreground service above is keeping alive,
> and Android kills *that* independently — under memory pressure, or simply for being backgrounded — with no
> foreground service able to save it. The page then reloads with a fresh socket, and three things broke on
> that reload before this was found: the second attach was refused outright (`409 Conflict`), because a
> second valid upgrade could only mean a bug or a hostile second process, never our own page coming back; a
> send into the dead first socket threw, and that exception unwound `TerminalSessionPump`'s flush loop,
> freezing the still-live shell behind it — `LiveSessionCount` kept counting a session nothing would ever
> drain again; and every byte sent while no page was attached had already spent flow-control credit that no
> acknowledgement could ever return, so a session outliving 256 KiB of output into a dead page stalled for
> good regardless of the other two. Waiting for the old socket to notice it was dead and close on its own
> was never going to be enough either — a killed renderer sends no TCP FIN, so the old receive loop could sit
> unaware for the whole 30-second keepalive.
>
> Fixed as a takeover rather than a guard: a second valid upgrade — origin, token and subprotocol all
> checked exactly as before — now displaces whatever socket was attached instead of being refused, since
> only this app's own page ever knows the token, so a second valid attach *is* that page, back again.
> `TerminalDataPlane.SendAsync` no longer lets a dead-socket send escape as a fault; it reads as "nobody
> listening," same as no socket being attached at all. `TerminalWorkspace` resets each live session's credit
> window on every attach and resends its `SessionOpened` frame, flagged as a replay, so the fresh page
> rebuilds the pane and the pump stops waiting on an acknowledgement that was never coming. And
> `terminal.js`'s socket now retries itself, forever, with backoff, instead of reporting the connection
> failed and stopping — the page dies with the app anyway, so there is no case where retrying is the wrong
> call. What is **not** recovered, and says so rather than pretending otherwise: scrollback across a page
> reload. It lived in the page's own DOM, and a reloaded page is a new DOM. The replay banner — *"the view
> reconnected; earlier output stayed on the host"* — is that honesty put where the person looking at the
> terminal will actually read it, not buried in a log.
--- ---
## Decisions taken ## Decisions taken
@@ -280,13 +314,40 @@ What is desktop-only is the *left* pane — `LocalDirectory`, the drive list, th
### Sessions survive backgrounding, via a foreground service ### 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 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 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 to make `TerminalWorkspace`'s guarantee desktop-only, which is a worse thing to have to write down than a
notification is to look at. 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 ### Phone first
About 360dp wide. The tablet route was cheaper — a landscape tablet is close to the existing 880×560 minimum About 360dp wide. The tablet route was cheaper — a landscape tablet is close to the existing 880×560 minimum
@@ -523,7 +584,12 @@ go at 360dp:
stopping it from a count rather than a lifecycle. `TerminalWorkspace.LiveSessionCount` is the source of 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 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 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, 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 plus the two states the design does not draw because it starts at an enrolled phone (naming a server, and
choosing a passphrase). choosing a passphrase).
+1 -1
View File
@@ -258,7 +258,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference
> | The host card as a link straight to a terminal | Click still selects, double-click still connects, and the pencil still opens the pane — the mock's card-as-link is not adopted, because multi-select (Ctrl, Shift, the marquee band) depends on a plain click meaning "choose this one" rather than "go". | > | The host card as a link straight to a terminal | Click still selects, double-click still connects, and the pencil still opens the pane — the mock's card-as-link is not adopted, because multi-select (Ctrl, Shift, the marquee band) depends on a plain click meaning "choose this one" rather than "go". |
> | Quick connect's SSH/SFTP kind column | The auth word — credential, key, or password — see above. | > | Quick connect's SSH/SFTP kind column | The auth word — credential, key, or password — see above. |
> | A collapsed section staying collapsed after a restart | In memory only, for the running session. `settings.json` holds two scalars by decision — the terminal's text size and whether this machine checks for updates on its own — and collapse state is not judged worth a third. | > | A collapsed section staying collapsed after a restart | In memory only, for the running session. `settings.json` holds two scalars by decision — the terminal's text size and whether this machine checks for updates on its own — and collapse state is not judged worth a third. |
> | QUICK ACCESS's editor, on the phone | **Deferred; the data is not.** `HostSecret.PinnedPaths` is shared, synced and merged on both heads, so a pin made on the desktop reaches the phone and back — Android just has nowhere yet to add or remove one itself. | > | QUICK ACCESS's editor, on the phone | ◆ **Shipped, over the same shared data the deferral above described.** The phone's host editor draws its own QUICK ACCESS section on the same staged `VaultViewModel.EditorPinnedPaths` the desktop's drawer binds — a row per pin, an add field and button, and a remove target sized to this head's 44dp touch floor rather than the desktop's 22-pixel close box. `AddEditorPinCommand`'s refusals surface through a `Status` line the editor page draws for itself, since that page covers the whole screen and the list behind it draws its own `Status` off-screen for as long as it is open. The pins themselves reach a second surface this head has that the desktop does not need: `TransfersViewModel.ConnectedPinnedPaths` carries them as chips on the Files screen while connected, and tapping one runs `GoRemoteCommand` — the same command the breadcrumb trail already used to navigate. Two deviations from the desktop, both named where they land: the chip row is a snapshot taken at connect rather than a live follow of the vault, so a pin edited mid-session shows up on the next connect rather than this one; and the editor's own hint sentence says the pins appear on the Files screen, not above a terminal — this head has no terminal strip for them to sit above, the same honesty the row below already states for the desktop's own hint. |
> | Collapse All beside every section's own collapse chevron | Bound on every heading's view model and shown on only the first — `SidebarGroupHeader.IsFirstBoardSection` is what a virtualised list of sections uses in place of a control of the board's own that would otherwise have to sit above all of them. | > | Collapse All beside every section's own collapse chevron | Bound on every heading's view model and shown on only the first — `SidebarGroupHeader.IsFirstBoardSection` is what a virtualised list of sections uses in place of a control of the board's own that would otherwise have to sit above all of them. |
> | The QUICK ACCESS hint's claim that pins live in a sidebar | "Pinned folders appear above the terminal for this host." — no sidebar exists on this screen for the sentence to point at, so the shipped hint says where they actually draw. | > | The QUICK ACCESS hint's claim that pins live in a sidebar | "Pinned folders appear above the terminal for this host." — no sidebar exists on this screen for the sentence to point at, so the shipped hint says where they actually draw. |
> | The mock's "Saving to **DodoTech ▾** vault" subtitle, with a picker's chevron inside a sentence | The pre-existing `DrawerSubtitle` wording — the vault's name alone, unchanged by this pass. A chevron inside running text implies the text itself is the control, which it is not: the vault picker is its own element, shown only while creating and only above one writable vault, as it always has been. | > | The mock's "Saving to **DodoTech ▾** vault" subtitle, with a picker's chevron inside a sentence | The pre-existing `DrawerSubtitle` wording — the vault's name alone, unchanged by this pass. A chevron inside running text implies the text itself is the control, which it is not: the vault picker is its own element, shown only while creating and only above one writable vault, as it always has been. |
+137 -2
View File
@@ -1101,7 +1101,7 @@ time)" — is the worse failure of the two: it rebinds a host as a side effect o
## Phase 8 — Adding and removing on the phone's host list ## Phase 8 — Adding and removing on the phone's host list
Thirteen checks, and the reason there are thirteen rather than none is worth stating: **the layout suite Twenty-one checks, and the reason there are twenty-one rather than none is worth stating: **the layout suite
cannot see any of this and structurally never will.** `DodoSSH.Client.App.Layout.Tests` targets `net10.0` and cannot see any of this and structurally never will.** `DodoSSH.Client.App.Layout.Tests` targets `net10.0` and
`DodoSSH.Client.Android` targets `net10.0-android`, so a project reference is impossible; Avalonia's `DodoSSH.Client.Android` targets `net10.0-android`, so a project reference is impossible; Avalonia's
application, dispatcher and platform are one-shot process globals, so a second head cannot share the application, dispatcher and platform are one-shot process globals, so a second head cannot share the
@@ -1418,6 +1418,60 @@ destinations, so switching under a live one would show a screen titled S3 listin
the row — that screen keeps its own copy of the host list, so it has to be re-found there by entity id the row — that screen keeps its own copy of the host list, so it has to be re-found there by entity id
rather than handed the vault's object. rather than handed the vault's object.
### 8.18 QUICK ACCESS in the phone's host editor
Open a host's editor and scroll to QUICK ACCESS.
**Pass:** an empty list, an add field and an ADD button. Type `/var/www/app` and press ADD.
**Pass:** a row appears carrying that path and a ✕ at least 44dp on a side. Type the same path again and
press ADD.
**Pass:** nothing is added, and a sentence appears on the page saying the path is already pinned — this page
covers the whole screen while it is open, so that sentence is this page's own `Status` line rather than the
one the host list draws above HOSTS, which is off-screen right now. Clear the box and press ADD with nothing
typed.
**Pass:** a sentence saying a pinned path cannot be blank, in the same place.
Press the ✕ on the pinned row, then SAVE.
**Pass:** back on HOSTS with the pin gone. Open the editor on that host again.
**Pass:** QUICK ACCESS is empty — the removal was saved, not merely staged. Re-pin `/var/www/app`, SAVE, and
check the same host on the desktop.
**Pass:** the pin is there. `HostSecret.PinnedPaths` is shared and merged like every other field on a host,
so nothing about this page keeps its own copy.
**Failure means:** a refusal that changes nothing on screen is `AddEditorPinCommand` writing to `Status`
with nothing on this page bound to it — the honesty rule broken silently, since the command still behaves
correctly and only the telling of it is missing. A pin gone after SAVE-then-reopen but present on the
desktop is `BuildHost` not reading `EditorPinnedPaths`, or `EditSelectedHost` not loading it back in.
### 8.19 Pin chips on the Files screen
Pin a folder on a host, then connect to it on the files screen (SFTP), either directly or via **Connect via
SFTP**.
**Pass:** once connected, a row of chips appears between the breadcrumb and the listing, one per pin, each
at least 44dp tall. Tap one.
**Pass:** the listing navigates straight to that directory, the same as tapping a breadcrumb crumb does.
Disconnect, then connect to a host with nothing pinned.
**Pass:** no chip row at all — not an empty one. Connect to a bucket instead.
**Pass:** still no chip row, on any bucket. A bucket has no `HostSecret` underneath it and so nothing to
pin.
**Failure means:** chips that do not move the listing are the row's `GoRemoteCommand` binding pointed at the
wrong `DataContext` — see the `$parent[views:FilesScreen]` escape every other command in this file uses. A
chip row surviving a disconnect, or appearing under a bucket, is `TransfersViewModel.ConnectedPinnedPaths`
not being cleared in `CloseSessionAsync` or `OpenBucketAsync`. A chip row missing a pin added *after* this
connect is not a bug — see `ConnectedPinnedPaths`'s own remark on why this is a snapshot rather than a live
follow, and try disconnecting and reconnecting instead.
--- ---
## Phase 9 — Tag chips and the picker ## Phase 9 — Tag chips and the picker
@@ -1691,6 +1745,39 @@ is a terminal that answers the buttons and ignores the keyboard: it reads as the
Worth doing on the software keyboard too, where the same fault shows as the keyboard closing on the first Worth doing on the software keyboard too, where the same fault shows as the keyboard closing on the first
tap of an arrow key. tap of an arrow key.
### 11.11 Closing a connection and opening a new one both take you somewhere real
Open a shell, close its tab, then open a different one from HOSTS.
**Pass:** the new terminal renders and takes input straight away — no stuck "Connecting…" status, no blank
pane that never receives the prompt.
**Failure means:** `TerminalDataPlane` refused the page's reattach. The renderer's `WebSocket` does not
survive a tab going from one to zero and back to one on every device, and a host that answers a second valid
upgrade with `409 Conflict` instead of taking the socket over leaves every terminal after the first
permanently unreachable — see the correction in `docs/android-port.md`'s terminal section.
### 11.12 A backgrounded shell survives its renderer being killed · **needs several minutes, or developer tooling**
With a shell open and something worth reading in its scrollback, background the app (home button, not back)
for several minutes — long enough for Android to consider reclaiming it — then return. If the device exposes
it, forcing a stop of the WebView renderer process from Developer Options while backgrounded is the more
reliable way to trigger the same thing on demand rather than waiting on the OS's own judgement. Either way,
type something once you are back.
**Pass:** one of two honest outcomes, both good. Either the pane is exactly as it was — the renderer process
survived, so nothing needed to happen — or the pane is empty but for a dim line reading `── the view
reconnected; earlier output stayed on the host ──`, meaning the page reloaded and reattached. In both cases
what is typed now reaches the shell, and the shell is still the same one — not a new tab, not a reconnect
sheet, no "Connecting…" status stuck on screen.
**Failure means:** if the status stays stuck or nothing typed arrives, the renderer's socket did not retry
itself — see `terminal.js`'s `connect()` and its backoff. If the pane came back empty with **no** banner, a
session that survived a reload is being shown as though its scrollback had too, which is not true and is
worse than saying nothing: the banner exists so this is never silently wrong. If typing does nothing but the
banner is there, the session's credit window was not reset on reattach and the shell is frozen behind it —
see `TerminalWorkspace.ReplayAfterAttachAsync`.
--- ---
## Phase 12 — Shared vaults: the operations that span two accounts ## Phase 12 — Shared vaults: the operations that span two accounts
@@ -2021,9 +2108,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 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. 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 **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 `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.114.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 ### 14.7 SAVE FILE writes where you pointed it, and the file opens
+51 -22
View File
@@ -90,32 +90,61 @@ public sealed partial class DodoSshApp : Avalonia.Application
shell.DataContext = viewModel; shell.DataContext = viewModel;
// Difference 2: the foreground service, which is what makes TerminalWorkspace's promise — that a // Difference 2, wired up in its own method purely for length — see ComposeKeepAlive for what it
// shell outlives a vault lock — true on a platform that stops backgrounded processes. // does and why.
// ComposeKeepAlive(workspace, viewModel);
// The transfer count is real now that the document picker gives this head a way to start one, and
// it is the half that matters most here: a shell survives backgrounding because somebody is looking
// at it, and an upload has to survive precisely when nobody is — the screen is off and the phone is
// in a pocket. Queued counts as active, so putting five files in the queue and locking the phone
// moves five files.
//
// A local rather than a field, matching the desktop head: an Avalonia Application has no disposal
// hook, so a field holding a disposable would have nowhere honest to release it. It stays alive
// because it is subscribed to the workspace, which lives as long as the process.
var keepAlive = new SessionKeepAlive(
workspace,
activeTransfers: () => viewModel.Transfers.ActiveTransfers);
// The other end of the same wire: the workspace announces its own sessions ending, and the queue
// announces transfers appearing and finishing. Without this the notification would come up when an
// upload started and stay up after it finished, which is the failure this class exists to prevent.
viewModel.Transfers.ActivityChanged += (_, _) => keepAlive.Refresh();
keepAlive.Refresh();
return shell; return shell;
} }
/// <summary>
/// Wires up the foreground service that makes <c>TerminalWorkspace</c>'s promise — that a shell outlives
/// a vault lock — true on a platform that stops backgrounded processes.
/// </summary>
/// <remarks>
/// <para>
/// Split out of <see cref="Compose"/> for length rather than for reuse; there is exactly one caller.
/// </para>
/// <para>
/// The transfer count is real now that the document picker gives this head a way to start one, and it
/// matters exactly when nobody is looking: a shell survives backgrounding because somebody opened it,
/// and an upload has to survive precisely when nobody is — the screen is off and the phone is in a
/// pocket. Queued counts as active, so putting five files in the queue and locking the phone moves five
/// files. <c>holdsFileSession</c> covers the third case a count alone cannot: a host connected on the
/// Files screen with no transfer moving is still a live SFTP session that backgrounding would sever, and
/// <c>TransfersViewModel.HasLiveFileSession</c> is the existing fact — <c>IsConnected</c> with a real
/// cipher, which a bucket never has — that answers whether one is open.
/// </para>
/// <para>
/// <c>keepAlive</c> is a local rather than a field, matching the desktop head: an Avalonia Application
/// has no disposal hook, so a field holding a disposable would have nowhere honest to release it. It
/// stays alive because it is subscribed to the workspace, which lives as long as the process.
/// </para>
/// </remarks>
private static void ComposeKeepAlive(TerminalWorkspace workspace, MainWindowViewModel viewModel)
{
var keepAlive = new SessionKeepAlive(
workspace,
activeTransfers: () => viewModel.Transfers.ActiveTransfers,
holdsFileSession: () => viewModel.Transfers.HasLiveFileSession);
// The other end of the same wire: the workspace announces its own sessions ending, and the queue
// announces transfers appearing and finishing, and a Files session connecting or disconnecting.
// Without this the notification would come up when an upload started and stay up after it
// finished, which is the failure this class exists to prevent.
viewModel.Transfers.ActivityChanged += (_, _) => keepAlive.Refresh();
// The half that was missing until now: a shell opening. SessionKeepAlive already heard the
// workspace announce a session ending, but nothing announced the opposite — a user who opened a
// shell and backgrounded the app had no foreground service at all, because the only wire in was the
// one for taking it down. TerminalSessionOpened is that other half, forwarded from
// VaultViewModel.SessionOpened, and without this line the service could never come up for a shell
// in the first place, which was precisely the promise this whole arrangement exists to keep.
viewModel.TerminalSessionOpened += (_, _) => keepAlive.Refresh();
keepAlive.Refresh();
}
/// <summary> /// <summary>
/// Writing to this phone's clipboard. /// Writing to this phone's clipboard.
/// </summary> /// </summary>
@@ -6,7 +6,7 @@ using global::Android.OS;
namespace DodoSSH.Client.Android.Platform; namespace DodoSSH.Client.Android.Platform;
/// <summary> /// <summary>
/// Keeps the process alive for as long as a shell or a transfer is live. /// Keeps the process alive for as long as a shell, a transfer, or a connected Files session is live.
/// </summary> /// </summary>
/// <remarks> /// <remarks>
/// <para> /// <para>
@@ -41,6 +41,26 @@ internal sealed class SessionForegroundService : Service
private const string ChannelId = "dodossh.sessions"; private const string ChannelId = "dodossh.sessions";
private const int NotificationId = 1; private const int NotificationId = 1;
// API 33+ requires the request to name a code the RequestPermissionsResult callback would be handed
// back — 1 is fine because this head never implements that callback at all, see
// RequestNotificationPermission's own remark for why a result is not worth listening for.
private const int NotificationPermissionRequestCode = 1;
/// <summary>
/// Whether <see cref="OnStartCommand"/> has run for this process without a matching
/// <see cref="OnDestroy"/> since — i.e. whether Android currently considers this service foregrounded.
/// </summary>
/// <remarks>
/// Volatile because <see cref="Reconcile"/> can run on whatever thread called
/// <see cref="SessionKeepAlive.Refresh"/>, while this is set from the binder thread Android delivers
/// service lifecycle callbacks on — two threads with no other synchronisation between them, and a stale
/// read here is the difference between updating a notification in place and calling
/// <c>StartForegroundService</c> on a service that is already running, which is what defect 3 was.
/// </remarks>
private static volatile bool running;
private static bool notificationPermissionRequested;
/// <remarks> /// <remarks>
/// A bound service would tie the sessions' lifetime to a binding, which is the opposite of what is /// A bound service would tie the sessions' lifetime to a binding, which is the opposite of what is
/// wanted here: the point is that they outlive whatever the user does with the interface. /// wanted here: the point is that they outlive whatever the user does with the interface.
@@ -49,7 +69,9 @@ internal sealed class SessionForegroundService : Service
public override StartCommandResult OnStartCommand(Intent? intent, StartCommandFlags flags, int startId) public override StartCommandResult OnStartCommand(Intent? intent, StartCommandFlags flags, int startId)
{ {
StartForeground(NotificationId, BuildNotification(intent?.GetStringExtra("summary") ?? "Working")); running = true;
StartForeground(NotificationId, BuildNotification(this, intent?.GetStringExtra("summary") ?? "Working"));
// NotSticky: if Android does kill this process, the SSH connections died with it and there is // NotSticky: if Android does kill this process, the SSH connections died with it and there is
// nothing to resume. Restarting the service would produce a notification claiming sessions that no // nothing to resume. Restarting the service would produce a notification claiming sessions that no
@@ -58,14 +80,36 @@ internal sealed class SessionForegroundService : Service
return StartCommandResult.NotSticky; return StartCommandResult.NotSticky;
} }
/// <inheritdoc />
/// <remarks> /// <remarks>
/// The other half of <see cref="running"/>. Android calls this whether the service stopped itself or
/// was stopped from outside — <see cref="Reconcile"/>'s down case calls <c>StopService</c> rather than
/// clearing the flag directly, so this override is the one place that actually knows the service has
/// gone, matching how <see cref="OnStartCommand"/> is the one place that knows it has come up.
/// </remarks>
public override void OnDestroy()
{
running = false;
base.OnDestroy();
}
/// <remarks>
/// <para>
/// Low importance on purpose. This notification is a receipt, not an alert — it exists because Android /// Low importance on purpose. This notification is a receipt, not an alert — it exists because Android
/// requires one, and because the user is entitled to know the app is holding connections open. Making /// requires one, and because the user is entitled to know the app is holding connections open. Making
/// it buzz would be a notification about nothing having happened. /// it buzz would be a notification about nothing having happened.
/// </para>
/// <para>
/// Static, taking the <see cref="Context"/> it needs rather than reading <c>this</c>: the instance path
/// through <see cref="OnStartCommand"/> passes the service itself, and the in-place update path through
/// <see cref="Reconcile"/> has no service instance at all — only <see cref="PhoneEnvironment.Require"/>
/// — because posting to an already-running notification never touches the service's own lifecycle.
/// </para>
/// </remarks> /// </remarks>
private Notification BuildNotification(string summary) private static Notification BuildNotification(Context context, string summary)
{ {
var manager = (NotificationManager)GetSystemService(NotificationService)!; var manager = (NotificationManager)context.GetSystemService(Context.NotificationService)!;
if (OperatingSystem.IsAndroidVersionAtLeast(26)) if (OperatingSystem.IsAndroidVersionAtLeast(26))
{ {
@@ -79,12 +123,12 @@ internal sealed class SessionForegroundService : Service
} }
var reopen = PendingIntent.GetActivity( var reopen = PendingIntent.GetActivity(
this, context,
0, 0,
new Intent(this, typeof(MainActivity)).SetFlags(ActivityFlags.SingleTop), new Intent(context, typeof(MainActivity)).SetFlags(ActivityFlags.SingleTop),
PendingIntentFlags.Immutable | PendingIntentFlags.UpdateCurrent); PendingIntentFlags.Immutable | PendingIntentFlags.UpdateCurrent);
return new Notification.Builder(this, ChannelId) return new Notification.Builder(context, ChannelId)
.SetContentTitle("DodoSSH") .SetContentTitle("DodoSSH")
.SetContentText(summary) .SetContentText(summary)
.SetSmallIcon(global::Android.Resource.Drawable.IcDialogInfo) .SetSmallIcon(global::Android.Resource.Drawable.IcDialogInfo)
@@ -93,37 +137,138 @@ internal sealed class SessionForegroundService : Service
.Build(); .Build();
} }
/// <summary>Starts or stops the service to match what is actually running.</summary> /// <summary>Starts, stops, or refreshes the service's notification to match what is actually running.</summary>
/// <param name="liveSessions">Shells with a live channel behind them.</param> /// <param name="liveSessions">Shells with a live channel behind them.</param>
/// <param name="activeTransfers">Transfers still moving bytes.</param> /// <param name="activeTransfers">Transfers still moving bytes.</param>
public static void Reconcile(int liveSessions, int activeTransfers) /// <param name="holdsFileSession">Whether the Files screen holds a live, idle SFTP connection.</param>
/// <remarks>
/// Three outcomes, not the two a plain start-or-stop would have. Nothing live stops the service, as
/// always. Something live and the service not yet running starts it. Something live and the service
/// already running is the case that used to call <c>StartForegroundService</c> a second time, which on
/// API 31+ throws <c>ForegroundServiceStartNotAllowedException</c> the instant the app is backgrounded
/// — a transfer finishing in the pocket, one of two shells dying — crashing the process and taking the
/// remaining connections with it. That case — running, and the app backgrounded — now only posts a
/// fresh notification through the <see cref="NotificationManager"/> already holding the channel open,
/// which needs no foreground-start permission at all. A foregrounded refresh still prefers a real
/// start even over a service that looks like it is running; the inline remark below is why.
/// </remarks>
public static void Reconcile(int liveSessions, int activeTransfers, bool holdsFileSession)
{ {
var context = PhoneEnvironment.Require(); var context = PhoneEnvironment.Require();
var intent = new Intent(context, typeof(SessionForegroundService));
if (liveSessions == 0 && activeTransfers == 0) if (liveSessions == 0 && activeTransfers == 0 && !holdsFileSession)
{ {
context.StopService(intent); context.StopService(new Intent(context, typeof(SessionForegroundService)));
return; return;
} }
// The summary says what is actually held, counted rather than generic — the same principle the // The summary says what is actually held, counted rather than generic — the same principle the
// delete confirmations follow. "DodoSSH is running" would tell the user nothing they could act on. // delete confirmations follow. "DodoSSH is running" would tell the user nothing they could act on.
intent.PutExtra("summary", Summarise(liveSessions, activeTransfers)); var summary = Summarise(liveSessions, activeTransfers, holdsFileSession);
context.StartForegroundService(intent); // In-place only while backgrounded, where it is the only legal move. Foregrounded, a real start is
// always allowed and is preferred even when `running` says the service is up: a disconnect followed
// by a quick reconnect can land here while the StopService just issued is still in flight, and
// posting to that dying service's notification would leave an orphan receipt over an unprotected
// process — restarting instead makes the flag's small lag harmless. A start on a service that
// really is running only re-delivers OnStartCommand, whose StartForeground updates the same
// notification anyway. CurrentActivity is the foreground signal: set on resume, cleared on pause.
if (running && PhoneEnvironment.CurrentActivity is null)
{
var manager = (NotificationManager)context.GetSystemService(Context.NotificationService)!;
manager.Notify(NotificationId, BuildNotification(context, summary));
return;
}
RequestNotificationPermission(context);
Start(context, summary);
} }
private static string Summarise(int liveSessions, int activeTransfers) /// <remarks>
/// Split out of <see cref="Reconcile"/> for the try/catch alone, which needs its own remark and would
/// otherwise crowd the three-way branch above it.
/// </remarks>
private static void Start(Context context, string summary)
{ {
var parts = new List<string>(2); var intent = new Intent(context, typeof(SessionForegroundService));
intent.PutExtra("summary", summary);
try
{
context.StartForegroundService(intent);
}
catch (Java.Lang.IllegalStateException)
{
// ForegroundServiceStartNotAllowedException (API 31+) derives from this, and reaching it here
// means a start-worthy transition — a shell opening, a Files connection completing, the first
// transfer landing in an empty queue — happened while the app was backgrounded, which is
// precisely when Android refuses a new foreground start. There is no retry that helps: by the
// time this catch runs, the moment such a start would have been allowed has already passed.
// Swallowing it is the honest choice and not just the available one — letting the exception
// propagate would crash the process and drop the very shells and transfers this service exists
// to keep alive. A process that keeps running unprotected outlives one that does not run at all.
}
}
/// <summary>
/// Asks for the receipt notification's own permission, the first time this process actually has
/// something to show rather than at launch.
/// </summary>
/// <remarks>
/// At most once per process, via <see cref="notificationPermissionRequested"/> — not to work around a
/// platform limit, since Android already refuses to show the dialogue twice, but because a second call
/// to <c>RequestPermissions</c> after the first is still pending is its own kind of noise. No result is
/// read back: there is nothing this class would do differently for a grant versus a refusal, so a
/// callback would exist only to be empty. What refusal costs is stated rather than hidden — the
/// notification stays invisible — and what it does not cost is the point: the service still starts,
/// still holds the process in the foreground, and the shells and transfers it protects are exactly as
/// safe as if the user had said yes. See the manifest's own comment on this permission.
/// </remarks>
private static void RequestNotificationPermission(Context context)
{
// Isolated as its own guard clause rather than folded into the compound condition below: the
// platform-compatibility analyzer only recognises a version check as guarding what follows when it
// is the sole condition of its own early return, and PostNotifications is annotated API 33+.
if (!OperatingSystem.IsAndroidVersionAtLeast(33))
{
return;
}
// Read into a local rather than referenced again inside the lambda below: the guard clause above
// covers a direct call in this method's own body, but the platform-compatibility analyzer treats a
// lambda as reachable from anywhere and will not extend the guard across that boundary. Capturing
// the already-validated string sidesteps the false positive without weakening the actual check.
var postNotifications = global::Android.Manifest.Permission.PostNotifications;
if (notificationPermissionRequested
|| PhoneEnvironment.CurrentActivity is not { } activity
|| context.CheckSelfPermission(postNotifications) == Permission.Granted)
{
return;
}
notificationPermissionRequested = true;
activity.RunOnUiThread(() =>
activity.RequestPermissions([postNotifications], NotificationPermissionRequestCode));
}
private static string Summarise(int liveSessions, int activeTransfers, bool holdsFileSession)
{
var parts = new List<string>(3);
if (liveSessions > 0) if (liveSessions > 0)
{ {
parts.Add(liveSessions == 1 ? "1 shell connected" : $"{liveSessions} shells connected"); parts.Add(liveSessions == 1 ? "1 shell connected" : $"{liveSessions} shells connected");
} }
if (holdsFileSession)
{
parts.Add("Files connected");
}
if (activeTransfers > 0) if (activeTransfers > 0)
{ {
parts.Add(activeTransfers == 1 ? "1 transfer running" : $"{activeTransfers} transfers running"); parts.Add(activeTransfers == 1 ? "1 transfer running" : $"{activeTransfers} transfers running");
@@ -17,22 +17,34 @@ namespace DodoSSH.Client.Android.Platform;
/// tally kept here. It already knows that a session whose shell exited half an hour ago is not live, which /// tally kept here. It already knows that a session whose shell exited half an hour ago is not live, which
/// a counter incremented on open and decremented on close would not. /// a counter incremented on open and decremented on close would not.
/// </para> /// </para>
/// <para>
/// Three facts feed <see cref="SessionForegroundService.Reconcile"/>, not one: live shells, moving
/// transfers, and an idle-but-connected Files session. The last of those used to be missing entirely —
/// a shell survives backgrounding because somebody opened it, but a Files connection with nothing moving
/// looked, to this class, exactly like nothing being open at all. <c>holdsFileSession</c> below is that
/// gap closed, read the same way the other two facts are: asked, not cached.
/// </para>
/// </remarks> /// </remarks>
internal sealed class SessionKeepAlive : IDisposable internal sealed class SessionKeepAlive : IDisposable
{ {
private readonly TerminalWorkspace workspace; private readonly TerminalWorkspace workspace;
private readonly Func<int> activeTransfers; private readonly Func<int> activeTransfers;
private readonly Func<bool> holdsFileSession;
/// <param name="workspace">The live shells.</param> /// <param name="workspace">The live shells.</param>
/// <param name="activeTransfers"> /// <param name="activeTransfers">
/// How many transfers are moving bytes. A delegate rather than a queue, because file transfer is out /// How many transfers are moving bytes. A delegate rather than a queue, because ownership of the
/// of this head's first scope — see the decision in docs/android-port.md — and this is the seam it /// transfer queue stays with <c>TransfersViewModel</c> — this class only ever asks it a question.
/// will arrive through rather than a dependency taken before there is anything to depend on.
/// </param> /// </param>
public SessionKeepAlive(TerminalWorkspace workspace, Func<int> activeTransfers) /// <param name="holdsFileSession">
/// Whether the Files screen holds a live SFTP connection with nothing moving on it — the idle-but-
/// connected case a transfer count alone would miss. See <c>TransfersViewModel.HasLiveFileSession</c>.
/// </param>
public SessionKeepAlive(TerminalWorkspace workspace, Func<int> activeTransfers, Func<bool> holdsFileSession)
{ {
this.workspace = workspace; this.workspace = workspace;
this.activeTransfers = activeTransfers; this.activeTransfers = activeTransfers;
this.holdsFileSession = holdsFileSession;
// Raised on whatever thread the pump unwound on, which is fine: starting and stopping a service is // Raised on whatever thread the pump unwound on, which is fine: starting and stopping a service is
// a binder call and needs no particular thread. Nothing here touches the interface. // a binder call and needs no particular thread. Nothing here touches the interface.
@@ -41,13 +53,14 @@ internal sealed class SessionKeepAlive : IDisposable
/// <summary>Re-reads the counts and starts or stops the service to match.</summary> /// <summary>Re-reads the counts and starts or stops the service to match.</summary>
/// <remarks> /// <remarks>
/// Called after anything that could change either count — opening a shell, closing a tab, a transfer /// Called after anything that could change any of the three facts — opening a shell, closing a tab, a
/// finishing. Calling it when nothing changed is free: reconciling to the state it is already in is /// transfer finishing, a Files connection opening or closing. Calling it when nothing changed is free:
/// either a redundant <c>startForegroundService</c> on a running service or a <c>stopService</c> on a /// reconciling to the state it is already in is either a redundant <c>startForegroundService</c> — or,
/// stopped one, and Android treats both as no-ops. /// now, a redundant notification post — on a running service, or a <c>stopService</c> on a stopped one,
/// and Android treats all of those as no-ops.
/// </remarks> /// </remarks>
public void Refresh() => public void Refresh() =>
SessionForegroundService.Reconcile(workspace.LiveSessionCount, activeTransfers()); SessionForegroundService.Reconcile(workspace.LiveSessionCount, activeTransfers(), holdsFileSession());
/// <inheritdoc /> /// <inheritdoc />
public void Dispose() public void Dispose()
@@ -56,7 +69,7 @@ internal sealed class SessionKeepAlive : IDisposable
// The notification goes with the composition root. Leaving it up over a process that is shutting // The notification goes with the composition root. Leaving it up over a process that is shutting
// down is how an SSH client acquires a reputation for a notification you cannot get rid of. // down is how an SSH client acquires a reputation for a notification you cannot get rid of.
SessionForegroundService.Reconcile(0, 0); SessionForegroundService.Reconcile(0, 0, false);
} }
private void OnSessionEnded(object? sender, TerminalSessionEndedEventArgs e) => Refresh(); private void OnSessionEnded(object? sender, TerminalSessionEndedEventArgs e) => Refresh();
@@ -12,7 +12,12 @@
<uses-permission android:name="android.permission.FOREGROUND_SERVICE" /> <uses-permission android:name="android.permission.FOREGROUND_SERVICE" />
<uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" /> <uses-permission android:name="android.permission.FOREGROUND_SERVICE_DATA_SYNC" />
<!-- The service's persistent notification. Runtime-requested on API 33+, and refusal is survivable. --> <!--
The service's persistent notification. Requested on API 33+ from SessionForegroundService.Reconcile,
the first time in this process there is actually something to show — not at launch, where the ask
would justify nothing on screen yet. Refusal is survivable: the service still starts and still holds
the process in the foreground either way, so a "no" costs the notification and nothing else.
-->
<uses-permission android:name="android.permission.POST_NOTIFICATIONS" /> <uses-permission android:name="android.permission.POST_NOTIFICATIONS" />
<!-- Releases the device key. See AndroidDeviceKeyStore. --> <!-- Releases the device key. See AndroidDeviceKeyStore. -->
@@ -48,7 +48,7 @@
the refusal has no continue button here either. the refusal has no continue button here either.
--> -->
<Grid RowDefinitions="Auto,Auto,Auto,Auto,*,Auto"> <Grid RowDefinitions="Auto,Auto,Auto,Auto,Auto,*,Auto">
<!-- ============ header ============ --> <!-- ============ header ============ -->
<Grid Grid.Row="0" ColumnDefinitions="Auto,Auto,*,Auto" Height="56" Margin="8,0"> <Grid Grid.Row="0" ColumnDefinitions="Auto,Auto,*,Auto" Height="56" Margin="8,0">
@@ -199,8 +199,48 @@
</StackPanel> </StackPanel>
</ScrollViewer> </ScrollViewer>
<!-- ============ pins ============ -->
<!--
◆ The phone's answer to the desktop's QUICK ACCESS sidebar — the same PinnedPathList this host was
connected with, drawn where this head actually browses files rather than beside a terminal it has no
strip for. See TransfersViewModel.ConnectedPinnedPaths for why this is a snapshot taken at connect
rather than a live follow of the host row: a pin added or removed mid-session shows up here on the
next connect, not while this one is still open.
Its own row rather than folded into the breadcrumb above: the two scroll independently and mean
different things — the breadcrumb is where this pane is, the chips are places it can jump to — and a
shared row would make ↑ look like one more pin among several.
Gone rather than empty when nothing is pinned or nothing is connected, which HasConnectedPins already
answers: an empty scrolling strip under the breadcrumb would read as a loading row rather than as
"this host has nothing pinned".
-->
<ScrollViewer Grid.Row="4" HorizontalScrollBarVisibility="Auto" VerticalScrollBarVisibility="Disabled"
IsVisible="{Binding HasConnectedPins}" Margin="0,0,0,4">
<ItemsControl ItemsSource="{Binding ConnectedPinnedPaths}">
<ItemsControl.ItemsPanel>
<ItemsPanelTemplate><StackPanel Orientation="Horizontal" Spacing="6" Margin="14,0" /></ItemsPanelTemplate>
</ItemsControl.ItemsPanel>
<ItemsControl.ItemTemplate>
<DataTemplate x:DataType="x:String">
<!--
Classes="row" at its default 44dp MinHeight rather than the breadcrumb's own 36 — these chips
are the destination, not the trail behind it, and a target worth a deliberate tap gets the
full touch floor this head holds everything else to. Not trimmed: a chip scrolls sideways with
the row rather than being squeezed to fit it, so the full path is always what a tap commits to.
-->
<Button Classes="row" MinHeight="44" Padding="11,0" CornerRadius="10"
Command="{Binding $parent[views:FilesScreen].((vm:TransfersViewModel)DataContext).GoRemoteCommand}"
CommandParameter="{Binding}">
<TextBlock Classes="mono" FontSize="12" Text="{Binding}" />
</Button>
</DataTemplate>
</ItemsControl.ItemTemplate>
</ItemsControl>
</ScrollViewer>
<!-- ============ the listing ============ --> <!-- ============ the listing ============ -->
<Panel Grid.Row="4"> <Panel Grid.Row="5">
<TextBlock Classes="body" IsVisible="{Binding !IsConnected}" Margin="24,12" <TextBlock Classes="body" IsVisible="{Binding !IsConnected}" Margin="24,12"
VerticalAlignment="Top" Text="{Binding Status}" /> VerticalAlignment="Top" Text="{Binding Status}" />
@@ -266,7 +306,7 @@
<!-- ============ what to do with the chosen entry ============ --> <!-- ============ what to do with the chosen entry ============ -->
<!-- DeepChrome rather than Chrome, since v5 — see the remark on PhoneShell's vault header. --> <!-- DeepChrome rather than Chrome, since v5 — see the remark on PhoneShell's vault header. -->
<Border Grid.Row="5" IsVisible="{Binding IsConnected}" Background="{StaticResource DeepChrome}" <Border Grid.Row="6" IsVisible="{Binding IsConnected}" Background="{StaticResource DeepChrome}"
BorderBrush="{StaticResource Border}" BorderThickness="0,1,0,0" Padding="12,10"> BorderBrush="{StaticResource Border}" BorderThickness="0,1,0,0" Padding="12,10">
<StackPanel Spacing="9"> <StackPanel Spacing="9">
@@ -1098,6 +1098,60 @@
Command="{Binding AddEditorTagCommand}" /> Command="{Binding AddEditorTagCommand}" />
</Grid> </Grid>
<!--
◆ QUICK ACCESS. Staged on the shared VaultViewModel.EditorPinnedPaths the way EditorTagChoices
stages tags above — populated when the editor opens, read back by BuildHost on SAVE, left alone
by CANCEL. Nothing here is phone-only: a pin added on this page is the same PinnedPathList entry
the desktop's drawer stages under the same field, so it syncs to every other client the moment
this vault does, exactly as a tag would.
No folder glyph in the row, unlike the desktop's — this head carries MonoFont and a heading font
(see Theme/Phone.axaml) and no icon font, so the desktop's &#xE2C7; would draw as a tofu box
rather than a folder here. The path text carries the row on its own, trimmed in a Grid's star
column rather than a StackPanel's — a StackPanel measures its children at infinity, so
TextTrimming never actually engages inside one; this is the same fix TransfersScreen's pane
headers needed for the same reason.
The remove button is Classes="icon" at its default 44x44, not the desktop's 22x22 close box —
see this file's touch-floor rule at the top of the class: a target sized for a mouse pointer is
not one a thumb can reliably land on.
See surface B, the Files screen's pin chips, for where these pins actually get used on this
head — the hint sentence below names it rather than the desktop's terminal strip, which does
not exist here.
-->
<TextBlock Classes="label" Text="QUICK ACCESS" Margin="0,4,0,0" />
<ItemsControl ItemsSource="{Binding EditorPinnedPaths}">
<ItemsControl.ItemsPanel>
<ItemsPanelTemplate><StackPanel Spacing="6" /></ItemsPanelTemplate>
</ItemsControl.ItemsPanel>
<ItemsControl.ItemTemplate>
<DataTemplate x:DataType="x:String">
<Border MinHeight="44" CornerRadius="10" Background="{StaticResource Field}"
BorderBrush="{StaticResource BorderMid}" BorderThickness="1" Padding="12,0">
<Grid ColumnDefinitions="*,Auto">
<TextBlock Grid.Column="0" Classes="mono" FontSize="12.5" Text="{Binding}"
VerticalAlignment="Center" TextTrimming="CharacterEllipsis" />
<Button Grid.Column="1" Classes="icon" Content="✕"
Command="{Binding $parent[ItemsControl].((vm:VaultViewModel)DataContext).RemoveEditorPinCommand}"
CommandParameter="{Binding}" ToolTip.Tip="Unpins this path" />
</Grid>
</Border>
</DataTemplate>
</ItemsControl.ItemTemplate>
</ItemsControl>
<Grid ColumnDefinitions="*,8,Auto">
<TextBox Grid.Column="0" Classes="field" Text="{Binding EditorNewPin}"
PlaceholderText="/path/to/folder" />
<Button Grid.Column="2" Classes="secondary" Height="44" Width="72" Content="ADD"
Command="{Binding AddEditorPinCommand}" />
</Grid>
<TextBlock Classes="body"
Text="Pins show up as chips on the Files screen while you are connected to this host — tap one to jump straight there." />
<!-- <!--
◆ The one control here that publishes something. Turning it on copies this host's address and ◆ The one control here that publishes something. Turning it on copies this host's address and
port into plaintext columns the server can read, which is the single deliberate concession in port into plaintext columns the server can read, which is the single deliberate concession in
@@ -1117,6 +1171,18 @@
<TextBlock Classes="body" <TextBlock Classes="body"
Text="Not built yet: this app always dials the host itself, so ticking this stores the address on the server and changes nothing about how the host is reached. When it works, the relay will dial on your behalf — which is why the address and port have to be stored in the clear. Everything else about the host stays encrypted either way, and a relayed host needs a port of its own rather than its group's." /> Text="Not built yet: this app always dials the host itself, so ticking this stores the address on the server and changes nothing about how the host is reached. When it works, the relay will dial on your behalf — which is why the address and port have to be stored in the clear. Everything else about the host stays encrypted either way, and a relayed host needs a port of its own rather than its group's." />
<!--
This page covers the whole screen while it is open, which is why it needs a Status line of its
own rather than relying on the one the list behind it draws at the top of HOSTS (see this file's
other Classes="detail" Text="{Binding Status}") — that TextBlock is off-screen for as long as
IsEditing is true. AddEditorTagCommand and AddEditorPinCommand both refuse silently into Status
rather than throwing, so without a line to draw it here the honesty rule would be broken: a
refusal that writes a message nothing on screen shows is the same, to whoever pressed ADD, as no
refusal at all.
-->
<TextBlock Classes="detail" Text="{Binding Status}" TextWrapping="Wrap"
IsVisible="{Binding Status, Converter={x:Static StringConverters.IsNotNullOrEmpty}}" />
<Grid ColumnDefinitions="*,8,*" Margin="0,6,0,0"> <Grid ColumnDefinitions="*,8,*" Margin="0,6,0,0">
<Button Grid.Column="0" Classes="primary" Height="44" Content="SAVE" <Button Grid.Column="0" Classes="primary" Height="44" Content="SAVE"
Command="{Binding SaveHostCommand}" /> Command="{Binding SaveHostCommand}" />
@@ -525,6 +525,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
// list. Detached in DisposeAsync, which is the only point either of them ends. // list. Detached in DisposeAsync, which is the only point either of them ends.
this.workspace.SessionEnded += OnWorkspaceSessionEnded; this.workspace.SessionEnded += OnWorkspaceSessionEnded;
this.workspace.FontSizeStepRequested += OnFontSizeStepRequested; this.workspace.FontSizeStepRequested += OnFontSizeStepRequested;
this.workspace.RendererReattached += OnRendererReattached;
settings = new ClientSettingsStore(paths); settings = new ClientSettingsStore(paths);
@@ -635,6 +636,31 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
private void OnFontSizeStepRequested(object? sender, TerminalFontSizeStepEventArgs e) => private void OnFontSizeStepRequested(object? sender, TerminalFontSizeStepEventArgs e) =>
Dispatcher.UIThread.Post(() => StepTerminalFontSize(e.Step)); Dispatcher.UIThread.Post(() => StepTerminalFontSize(e.Step));
/// <remarks>
/// <para>
/// Marshalled for the same reason as <see cref="OnFontSizeStepRequested"/>: this arrives on the data
/// plane's socket-accept thread, and both properties it reads here — <see cref="TerminalFontSize"/> and
/// <see cref="SelectedTab"/> — are bound to by the interface.
/// </para>
/// <para>
/// <see cref="TerminalWorkspace.RendererReattached"/> fires once the workspace has replayed what it
/// owns — the live sessions. Font size and the choice of active tab are not the workspace's to know;
/// they live here, so this is the other half of putting a reattached page back the way it was. The size
/// is sent exactly as <see cref="TellRendererTheFontSizeAsync"/> sends it at startup, because nothing
/// has changed — the page has merely forgotten, and this is only a reminder.
/// </para>
/// </remarks>
private void OnRendererReattached(object? sender, EventArgs e) =>
Dispatcher.UIThread.Post(() =>
{
_ = workspace.SetFontSizeAsync(TerminalFontSize, CancellationToken.None).AsTask();
if (SelectedTab is { } tab)
{
_ = workspace.ActivateSessionAsync(tab.SessionId, CancellationToken.None).AsTask();
}
});
[ObservableProperty] [ObservableProperty]
private ShellState state = ShellState.Starting; private ShellState state = ShellState.Starting;
@@ -3051,6 +3077,7 @@ internal sealed partial class MainWindowViewModel : ObservableObject, IAsyncDisp
workspace.SessionEnded -= OnWorkspaceSessionEnded; workspace.SessionEnded -= OnWorkspaceSessionEnded;
workspace.FontSizeStepRequested -= OnFontSizeStepRequested; workspace.FontSizeStepRequested -= OnFontSizeStepRequested;
workspace.RendererReattached -= OnRendererReattached;
transfers.PropertyChanged -= OnTransfersPropertyChanged; transfers.PropertyChanged -= OnTransfersPropertyChanged;
// Stopped here rather than left to the process exiting with it: the loop holds no vault key and // Stopped here rather than left to the process exiting with it: the loop holds no vault key and
@@ -620,6 +620,19 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
[ObservableProperty] [ObservableProperty]
private string? connectedCipher; private string? connectedCipher;
/// <summary>
/// Whether there is a live SFTP connection this session would lose by dying — the phone's foreground-
/// service question, not the desktop's.
/// </summary>
/// <remarks>
/// <see cref="ConnectedCipher"/> is already the fact that tells a host apart from a bucket, because only
/// a host set it — a bucket is HTTP, per-request, and closes nothing a dying process would have kept
/// open, so it answers false here even while <see cref="IsConnected"/> is true. Android reads this to
/// decide whether an idle Files screen with no transfer moving still needs the process kept alive; the
/// desktop has no such question because nothing stops its process for having gone quiet.
/// </remarks>
internal bool HasLiveFileSession => IsConnected && ConnectedCipher is not null;
/// <summary>The accepted host key's algorithm, e.g. <c>ssh-ed25519</c>. See <see cref="ConnectedCipher"/>.</summary> /// <summary>The accepted host key's algorithm, e.g. <c>ssh-ed25519</c>. See <see cref="ConnectedCipher"/>.</summary>
[ObservableProperty] [ObservableProperty]
private string? connectedHostKeyAlgorithm; private string? connectedHostKeyAlgorithm;
@@ -637,6 +650,31 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
[ObservableProperty] [ObservableProperty]
private string? connectedIdentityLabel; private string? connectedIdentityLabel;
/// <summary>
/// The paths pinned on the connected host, for the phone's Files-screen chip row — the desktop draws
/// the same list in its QUICK ACCESS sidebar, over the terminal surface rather than this one. See
/// <see cref="VaultViewModel.EditorPinnedPaths"/> for where a pin is actually added or removed; this is
/// a read of what was already saved there.
/// </summary>
/// <remarks>
/// Captured at connect, the same moment <see cref="ConnectedTo"/> is, rather than followed live off the
/// host row's own <c>PinnedPaths</c>. A pin edited while this session stays open shows up on the next
/// connect rather than mid-session — the same lag <see cref="ConnectedTo"/> itself already carries for
/// a relabel — because this screen reads the vault once, at the moment it dials, rather than staying
/// wired to a collection it otherwise never has a reason to watch. A bucket has no pins at all:
/// <see cref="OpenBucketAsync"/> leaves this empty rather than reading as "not yet known", which is what
/// empty already means for a host that connected with none pinned.
/// </remarks>
internal ObservableCollection<string> ConnectedPinnedPaths { get; } = [];
/// <summary>Whether the connected host or bucket has any pins to draw as chips.</summary>
/// <remarks>
/// A read of <see cref="ConnectedPinnedPaths"/> rather than an <c>[ObservableProperty]</c> of its own,
/// so it is raised by hand at each of the three places that collection is repopulated or cleared —
/// <see cref="MarkHostConnected"/>, <see cref="OpenBucketAsync"/> and <see cref="CloseSessionAsync"/>.
/// </remarks>
internal bool HasConnectedPins => ConnectedPinnedPaths.Count > 0;
[ObservableProperty] [ObservableProperty]
private HostKeyPresentation? pendingHostKey; private HostKeyPresentation? pendingHostKey;
@@ -720,13 +758,18 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
internal ObservableCollection<TransferRowViewModel> Transfers { get; } = []; internal ObservableCollection<TransferRowViewModel> Transfers { get; } = [];
/// <summary>Raised on the UI thread whenever a transfer appears or changes state.</summary> /// <summary>
/// Raised on the UI thread whenever a transfer appears or changes state, or a host or bucket connects or
/// disconnects.
/// </summary>
/// <remarks> /// <remarks>
/// For a head that has to tell the operating system what this process is doing — Android's foreground /// For a head that has to tell the operating system what this process is doing — Android's foreground
/// service, which must be up for as long as bytes are moving and down afterwards. An event rather than /// service, which must be up for as long as bytes are moving, or a host session sits open, and down
/// letting that head watch <see cref="Transfers"/> itself: the collection announces rows arriving and /// afterwards. An event rather than letting that head watch <see cref="Transfers"/> itself: the
/// leaving, and the transition that matters most is neither of those but a row going from RUNNING to /// collection announces rows arriving and leaving, and the transition that matters most is neither of
/// DONE without moving. /// those but a row going from RUNNING to DONE without moving. Connecting and disconnecting are the other
/// two transitions the service cares about — see <see cref="HasLiveFileSession"/> — and neither touches
/// <see cref="Transfers"/> at all, so they need this same announcement made by hand.
/// </remarks> /// </remarks>
internal event EventHandler? ActivityChanged; internal event EventHandler? ActivityChanged;
@@ -947,6 +990,10 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
ConnectedHostKeyAlgorithm = null; ConnectedHostKeyAlgorithm = null;
ConnectedIdentityLabel = null; ConnectedIdentityLabel = null;
// And no pins either — see ConnectedPinnedPaths's own remark.
ConnectedPinnedPaths.Clear();
OnPropertyChanged(nameof(HasConnectedPins));
connected = (ConnectedTo, row.Label, row.EntityId, TimeProvider.System.GetUtcNow()); connected = (ConnectedTo, row.Label, row.EntityId, TimeProvider.System.GetUtcNow());
await ListRemoteAsync(session.HomeDirectory, cancellationToken).ConfigureAwait(true); await ListRemoteAsync(session.HomeDirectory, cancellationToken).ConfigureAwait(true);
@@ -1033,10 +1080,26 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
ConnectedHostKeyAlgorithm = opened.HostKey.Algorithm; ConnectedHostKeyAlgorithm = opened.HostKey.Algorithm;
ConnectedIdentityLabel = identityLabel; ConnectedIdentityLabel = identityLabel;
// See ConnectedPinnedPaths's own remark for why this is a snapshot rather than a live follow.
ConnectedPinnedPaths.Clear();
foreach (var path in row.Host.PinnedPaths)
{
ConnectedPinnedPaths.Add(path);
}
OnPropertyChanged(nameof(HasConnectedPins));
// Recorded, and not hidden because it is "only" the file browser. Opening this is a second login as // Recorded, and not hidden because it is "only" the file browser. Opening this is a second login as
// far as the remote's own auth.log is concerned, so a log of ours that omitted it would disagree with // far as the remote's own auth.log is concerned, so a log of ours that omitted it would disagree with
// the host's — and anybody comparing the two would be right to believe the host. // the host's — and anybody comparing the two would be right to believe the host.
connected = (ConnectedTo, row.Label, row.EntityId, TimeProvider.System.GetUtcNow()); connected = (ConnectedTo, row.Label, row.EntityId, TimeProvider.System.GetUtcNow());
// Raised here rather than from OnIsConnectedChanged, on purpose: IsConnected is set first, above,
// and ConnectedCipher second — a partial method firing off the first assignment would read
// HasLiveFileSession against a ConnectedCipher still holding whatever the previous session left
// there. Only at the end of this method are both facts actually true together.
OnPropertyChanged(nameof(HasLiveFileSession));
ActivityChanged?.Invoke(this, EventArgs.Empty);
} }
/// <summary> /// <summary>
@@ -1811,11 +1874,18 @@ internal sealed partial class TransfersViewModel : ObservableObject, IAsyncDispo
ConnectedCipher = null; ConnectedCipher = null;
ConnectedHostKeyAlgorithm = null; ConnectedHostKeyAlgorithm = null;
ConnectedIdentityLabel = null; ConnectedIdentityLabel = null;
ConnectedPinnedPaths.Clear();
OnPropertyChanged(nameof(HasConnectedPins));
RemotePath = string.Empty; RemotePath = string.Empty;
RemoteEntries.Clear(); RemoteEntries.Clear();
RemoteTrail.Clear(); RemoteTrail.Clear();
SelectedRemoteEntry = null; SelectedRemoteEntry = null;
// Same ordering reason as the raise at the end of MarkHostConnected: both properties this reads are
// already null above, so the raise belongs after them rather than in OnIsConnectedChanged. This also
// covers OpenBucketAsync, which calls this method first and never itself turns HasLiveFileSession on.
OnPropertyChanged(nameof(HasLiveFileSession));
ActivityChanged?.Invoke(this, EventArgs.Empty);
} }
/// <remarks> /// <remarks>
+94 -11
View File
@@ -3,7 +3,7 @@
/* /*
The renderer half of the terminal data plane. The renderer half of the terminal data plane.
Two things here are load-bearing and easy to get wrong: Three things here are load-bearing and easy to get wrong:
1. Output is acknowledged from term.write's completion callback, never on receipt. The 1. Output is acknowledged from term.write's completion callback, never on receipt. The
acknowledgement returns flow-control credit to the host, so acknowledging early would tell acknowledgement returns flow-control credit to the host, so acknowledging early would tell
@@ -15,6 +15,14 @@
partial sequences across writes. Decoding here would corrupt any multi-byte character that partial sequences across writes. Decoding here would corrupt any multi-byte character that
happened to straddle a frame boundary, which shows up as occasional mojibake in exactly the happened to straddle a frame boundary, which shows up as occasional mojibake in exactly the
conditions that are hardest to reproduce. conditions that are hardest to reproduce.
3. The socket reconnects itself, forever, with backoff. This page's WebView is routinely killed
and reloaded by Android under memory pressure or simply for being backgrounded, so "the
socket closed" is an ordinary event here, not the end of the terminal's life see connect().
A reloaded page starts with an empty session map, so createSession is idempotent (a session
that already has a pane is left alone) and a SESSION_OPENED frame carries a flag telling this
page whether it is a replay: nothing to do for a pane that is still here, and a short banner
for one that is not, because that pane's scrollback genuinely did not survive.
*/ */
const SERVER_OUTPUT = 1; const SERVER_OUTPUT = 1;
@@ -33,6 +41,25 @@ const CLIENT_FONT_SIZE_STEP = 4;
const HEADER_LENGTH = 5; const HEADER_LENGTH = 5;
const SCROLLBACK_LINES = 5000; const SCROLLBACK_LINES = 5000;
/*
How long to wait before trying the socket again, and how that wait grows. Starting quick matters
because the ordinary case is a page that just finished loading after its WebView came back the
host's listener has been sitting there the whole time and capping it matters because there is no
point spacing attempts further apart than a person notices. Forever rather than giving up, because
giving up would need a way to try again and there is none better than the one already here: the page
dies with the app.
*/
const RECONNECT_INITIAL_DELAY_MS = 1000;
const RECONNECT_MAX_DELAY_MS = 5000;
/*
Styled like the SESSION_CLOSED banner (matching \x1b[38;5;244, the same dim grey), but written by
createSession's caller rather than by createSession itself: only a *replay* landing on a pane that
does not exist yet means the page reloaded and lost it, and createSession has no way to know which
of its callers that is.
*/
const REPLAY_BANNER = '\x1b[38;5;244m── the view reconnected; earlier output stayed on the host ──\x1b[0m\r\n';
/* /*
The size panes are created at, until the host says otherwise which it does as soon as it has read The size panes are created at, until the host says otherwise which it does as soon as it has read
the stored preference, usually before the first session exists. Kept here as well so a pane opened the stored preference, usually before the first session exists. Kept here as well so a pane opened
@@ -192,7 +219,19 @@ function handleKey(event) {
return true; return true;
} }
/**
* Builds a pane for a session, or returns the one already there.
*
* Idempotent because a replay can land on a page that never lost its pane the socket dropped and
* came back, but this page's own process survived and asking for a session that already has a pane
* must not build a second one on top of it, orphaning the first one's WebGL context and scrollback.
*/
function createSession(sessionId) { function createSession(sessionId) {
const existing = sessions.get(sessionId);
if (existing) {
return existing;
}
const pane = document.createElement('div'); const pane = document.createElement('div');
pane.className = 'pane'; pane.className = 'pane';
pane.dataset.sessionId = String(sessionId); pane.dataset.sessionId = String(sessionId);
@@ -290,10 +329,23 @@ function handleFrame(buffer) {
const payload = new Uint8Array(buffer, HEADER_LENGTH); const payload = new Uint8Array(buffer, HEADER_LENGTH);
switch (opcode) { switch (opcode) {
case SERVER_SESSION_OPENED: case SERVER_SESSION_OPENED: {
createSession(sessionId); // Checked before createSession, which would otherwise erase the answer by creating the pane
// this check is asking about.
const hadPaneAlready = sessions.has(sessionId);
const session = createSession(sessionId);
// Byte 1 means the host is replaying a session that existed before this socket attached — see
// TerminalWorkspace.ReplayAfterAttachAsync. A replay landing on a pane that is still here has
// nothing left to do beyond the idempotent create above; one landing on a pane that is not means
// this page reloaded and that pane's scrollback went with it, which is worth a line saying so.
if (payload.length > 0 && payload[0] === 1 && !hadPaneAlready) {
session.term.write(REPLAY_BANNER);
}
setStatus(''); setStatus('');
break; break;
}
case SERVER_OUTPUT: { case SERVER_OUTPUT: {
const session = sessions.get(sessionId) ?? createSession(sessionId); const session = sessions.get(sessionId) ?? createSession(sessionId);
@@ -414,6 +466,31 @@ function handleFrame(buffer) {
} }
} }
/** @type {number | null} */
let reconnectTimer = null;
let reconnectDelay = RECONNECT_INITIAL_DELAY_MS;
/**
* Tries the socket again after a wait, unless a try is already pending.
*
* The guard is what keeps 'close' and 'error' from stacking two timers for one failure a socket
* that fails to open typically fires both, and each would otherwise schedule its own reconnect.
*/
function scheduleReconnect() {
if (reconnectTimer !== null) {
return;
}
setStatus('Reconnecting the terminal view…');
reconnectTimer = setTimeout(() => {
reconnectTimer = null;
connect();
}, reconnectDelay);
reconnectDelay = Math.min(reconnectDelay * 2, RECONNECT_MAX_DELAY_MS);
}
function connect() { function connect() {
const token = root.dataset.token; const token = root.dataset.token;
const url = root.dataset.socket; const url = root.dataset.socket;
@@ -423,16 +500,22 @@ function connect() {
socket = new WebSocket(url, ['dodossh.terminal.v1', `token.${token}`]); socket = new WebSocket(url, ['dodossh.terminal.v1', `token.${token}`]);
socket.binaryType = 'arraybuffer'; socket.binaryType = 'arraybuffer';
socket.addEventListener('open', () => setStatus('')); socket.addEventListener('open', () => {
setStatus('');
// Back to the quick attempt for whatever the next failure turns out to be. Kept slow between
// attempts within one outage, reset once the outage is actually over.
reconnectDelay = RECONNECT_INITIAL_DELAY_MS;
});
socket.addEventListener('message', (event) => handleFrame(event.data)); socket.addEventListener('message', (event) => handleFrame(event.data));
socket.addEventListener('close', () => { // Both close and error retry. They are not the same event on every failure — a socket that never
setStatus('Disconnected from DodoSSH.'); // opens can fire only 'error', one that opens and later drops fires only 'close' — and the host
}); // side of this same problem (TerminalDataPlane.UpgradeAsync's takeover) is exactly why retrying is
// safe: whichever attempt eventually reaches the host, a fresh valid upgrade always wins the socket.
socket.addEventListener('error', () => { socket.addEventListener('close', scheduleReconnect);
setStatus('The terminal connection failed.'); socket.addEventListener('error', scheduleReconnect);
});
} }
// One observer for the whole root rather than one per pane: resizes arrive in bursts while a // One observer for the whole root rather than one per pane: resizes arrive in bursts while a
@@ -66,7 +66,6 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
new(TaskCreationOptions.RunContinuationsAsynchronously); new(TaskCreationOptions.RunContinuationsAsynchronously);
private WebSocket? socket; private WebSocket? socket;
private int accepted;
private int disposed; private int disposed;
/// <param name="assets">Where the renderer's files come from.</param> /// <param name="assets">Where the renderer's files come from.</param>
@@ -106,6 +105,26 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
/// </remarks> /// </remarks>
public event EventHandler<TerminalFontSizeStepEventArgs>? FontSizeStepRequested; public event EventHandler<TerminalFontSizeStepEventArgs>? FontSizeStepRequested;
/// <summary>
/// Raised after a socket attaches — the first one, and every later takeover.
/// </summary>
/// <remarks>
/// <para>
/// Raised after <see cref="socket"/> has been swapped in but before <see cref="ReceiveLoopAsync"/> starts
/// consuming it, on the socket-accept thread — the same thread that is in the middle of
/// <see cref="UpgradeAsync"/> for this connection. <see cref="TerminalWorkspace"/> is this event's one
/// subscriber, and it uses the ordering to replay session state before anything the fresh page sends
/// (a resize, an early acknowledgement) can be dispatched; see its remark for why the two racing is
/// harmless regardless.
/// </para>
/// <para>
/// Unlike <see cref="RendererAttached"/>, which resolves once and answers "has a renderer ever attached"
/// for <see cref="TerminalWorkspace.WaitForRendererAsync"/>, this fires every time — because a takeover
/// is exactly the case <see cref="RendererAttached"/> was never meant to describe again.
/// </para>
/// </remarks>
public event EventHandler? SocketAttached;
/// <summary>Registers a session so inbound frames can be routed to it.</summary> /// <summary>Registers a session so inbound frames can be routed to it.</summary>
public void Register(uint sessionId, TerminalSessionPump pump) public void Register(uint sessionId, TerminalSessionPump pump)
{ {
@@ -148,8 +167,9 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
// Each connection on its own task, and deliberately not awaited. An upgraded WebSocket // Each connection on its own task, and deliberately not awaited. An upgraded WebSocket
// lives for the whole session, so handling connections in sequence would leave the accept // lives for the whole session, so handling connections in sequence would leave the accept
// loop parked inside the receive loop and every later request unanswered — the page's // loop parked inside the receive loop and every later request unanswered — the page's
// script and stylesheet among them. Concurrency needs no coordination here because the // script and stylesheet among them. Two upgrades racing each other need no coordination
// single-attach guard is an interlocked exchange. // here either, because the takeover in UpgradeAsync swaps the shared socket field with an
// interlocked exchange rather than assuming it is the only writer.
_ = HandleConnectionAsync(client, linked.Token); _ = HandleConnectionAsync(client, linked.Token);
} }
} }
@@ -192,6 +212,29 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
.SendAsync(frame, WebSocketMessageType.Binary, endOfMessage: true, cancellationToken) .SendAsync(frame, WebSocketMessageType.Binary, endOfMessage: true, cancellationToken)
.ConfigureAwait(false); .ConfigureAwait(false);
} }
catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested)
{
// Not a cancellation despite the type: .NET's ManagedWebSocket wraps a send that fails because
// the underlying connection is already gone — which is exactly what a killed renderer's socket
// looks like — in an OperationCanceledException of its own manufacture, regardless of whether
// anyone actually cancelled anything. The filter is what tells the two apart: if the caller's
// own token were the cause, IsCancellationRequested would be true here and this catch does not
// apply, so a real cancellation still propagates. Everything below about why this must not
// fault the caller applies here exactly as it does to the exception types in the next catch.
}
catch (Exception exception)
when (exception is WebSocketException or ObjectDisposedException
or InvalidOperationException or IOException)
{
// The state check above is not atomic with the send, and a WebView renderer process killed by
// Android leaves its socket reporting Open long after nobody is reading from the other end. This
// has to read as "nobody listening" — the same as the no-socket case above — and never as a
// fault: SendAsync is called from TerminalSessionPump.SendOutputAsync inside the flush loop, and
// letting this exception escape would fault that loop. A faulted flush loop stops draining the
// credit window, the reader blocks once it fills, and the SSH session behind it freezes for good
// while LiveSessionCount still counts it as running. A dropped frame is recoverable — a frozen
// session is not.
}
finally finally
{ {
sendGate.Release(); sendGate.Release();
@@ -267,6 +310,25 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
.ConfigureAwait(false); .ConfigureAwait(false);
} }
/// <remarks>
/// <para>
/// <b>Takeover, not rejection.</b> A valid upgrade always wins the socket, even when one is already
/// attached — the old socket is aborted and the newcomer takes its place. Refusing a second attach used
/// to be the rule, on the theory that one renderer lives for the whole process. That is WebView2's
/// truth and not Android's: the platform kills the WebView's renderer process under memory pressure or
/// simply for being backgrounded, the page reloads, and the reload's socket is a second valid upgrade —
/// refusing it left the terminal permanently unreachable with no way back short of restarting the app.
/// </para>
/// <para>
/// Refusing protects nothing here anyway: only our own page knows the token (see the type-level remark
/// on what the token defends against), so a second valid upgrade <em>is</em> our page, reattaching.
/// Waiting for the old socket to notice it is dead and close on its own is not a safer alternative
/// either — a killed renderer process sends no TCP FIN, so the old receive loop can sit unaware for the
/// whole 30-second keepalive interval, and every reload landing in that window would still find the
/// door held shut by a socket nobody is on the other end of. Taking over immediately is what makes a
/// reload actually reattach.
/// </para>
/// </remarks>
private async Task UpgradeAsync( private async Task UpgradeAsync(
Stream stream, Stream stream,
HttpRequestLine request, HttpRequestLine request,
@@ -280,16 +342,6 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
return; return;
} }
if (Interlocked.Exchange(ref accepted, 1) == 1)
{
// One renderer, one socket. A second attach would be either a bug or something else on the
// machine having found the port.
await WriteResponseAsync(
stream, "409 Conflict", "text/plain", "Already attached"u8.ToArray(), cancellationToken)
.ConfigureAwait(false);
return;
}
var key = request.Headers.GetValueOrDefault("sec-websocket-key")!; var key = request.Headers.GetValueOrDefault("sec-websocket-key")!;
var accept = ComputeHandshakeAccept(key); var accept = ComputeHandshakeAccept(key);
@@ -314,10 +366,28 @@ public sealed class TerminalDataPlane : ITerminalTransport, IAsyncDisposable
KeepAliveInterval = TimeSpan.FromSeconds(30), KeepAliveInterval = TimeSpan.FromSeconds(30),
}); });
socket = webSocket; // Whatever was attached before is displaced, not merely overwritten: Exchange hands back the old
rendererAttached.TrySetResult(); // reference so it can be aborted rather than left to linger as a socket nothing reads from again.
// Abort rather than a graceful close — a close frame would wait on a peer that, per the remark
// above, may never notice it should reply, and the newcomer already proved it is our page.
var previous = Interlocked.Exchange(ref socket, webSocket);
previous?.Abort();
await ReceiveLoopAsync(webSocket, cancellationToken).ConfigureAwait(false); rendererAttached.TrySetResult();
SocketAttached?.Invoke(this, EventArgs.Empty);
try
{
await ReceiveLoopAsync(webSocket, cancellationToken).ConfigureAwait(false);
}
finally
{
// Cleared only if the field still holds this connection's own socket. A takeover has already
// swapped in a newer one by the time an aborted receive loop unwinds to here, and clearing the
// field regardless would race the newcomer: whichever of the two finished last would win, and
// it must always be the newcomer, never this one going away.
Interlocked.CompareExchange(ref socket, null, webSocket);
}
} }
/// <remarks> /// <remarks>
@@ -101,6 +101,13 @@ public sealed class TerminalWorkspace : IAsyncDisposable
private readonly Lock sessionGate = new(); private readonly Lock sessionGate = new();
private readonly CancellationTokenSource lifetime = new(); private readonly CancellationTokenSource lifetime = new();
/// <summary>
/// The payload that marks a <see cref="TerminalServerOpcode.SessionOpened"/> frame as a replay rather
/// than a fresh open. A one-byte non-empty payload, so terminal.js's existing length check (empty
/// payload for a real open) tells the two apart without a second opcode.
/// </summary>
private static readonly byte[] ReplayMarker = [1];
private uint nextSessionId = 1; private uint nextSessionId = 1;
private Task? server; private Task? server;
private int disposed; private int disposed;
@@ -125,6 +132,12 @@ public sealed class TerminalWorkspace : IAsyncDisposable
// came from. Nothing here decides anything about the size: the shell owns it, because the shell is // came from. Nothing here decides anything about the size: the shell owns it, because the shell is
// what remembers it between launches. // what remembers it between launches.
dataPlane.FontSizeStepRequested += (_, e) => FontSizeStepRequested?.Invoke(this, e); dataPlane.FontSizeStepRequested += (_, e) => FontSizeStepRequested?.Invoke(this, e);
// Fire-and-forget: this fires on the socket-accept thread, in the middle of the data plane's own
// handshake handling, and has no business making that wait on however long a replay takes. See
// ReplayAfterAttachAsync for what "replay" means and why racing the fresh page's own first frames
// is harmless.
dataPlane.SocketAttached += (_, _) => _ = ReplayAfterAttachAsync();
} }
/// <summary> /// <summary>
@@ -223,6 +236,26 @@ public sealed class TerminalWorkspace : IAsyncDisposable
} }
} }
/// <summary>
/// A live session's flow-control window, or null when the id names no session this workspace still has
/// open.
/// </summary>
/// <remarks>
/// A test seam rather than something the shell has ever needed: nothing outside this assembly has a
/// reason to see a pump's credit window rather than what the transport does with it, but
/// <see cref="ReplayAfterAttachAsync"/>'s reset of that window on reattach is exactly the kind of thing
/// that is easy to get backwards, and worth asserting directly rather than only through its side
/// effects. Internal rather than public, reachable from the test assembly through the
/// <c>InternalsVisibleTo</c> this project already declares for it.
/// </remarks>
internal CreditWindow? CreditsFor(uint sessionId)
{
lock (sessionGate)
{
return sessions.TryGetValue(sessionId, out var session) ? session.Pump.Credits : null;
}
}
/// <summary> /// <summary>
/// Raised with the session id when a shell ends on its own. /// Raised with the session id when a shell ends on its own.
/// </summary> /// </summary>
@@ -254,6 +287,24 @@ public sealed class TerminalWorkspace : IAsyncDisposable
/// </remarks> /// </remarks>
public event EventHandler<TerminalFontSizeStepEventArgs>? FontSizeStepRequested; public event EventHandler<TerminalFontSizeStepEventArgs>? FontSizeStepRequested;
/// <summary>
/// Raised once a (re)attached renderer has been sent everything this workspace owns for it.
/// </summary>
/// <remarks>
/// <para>
/// The workspace's own share of "put the page back the way it was" is the sessions — each live one gets
/// its <c>SessionOpened</c> frame again, done by the time this fires. What is left is what the workspace
/// has no business owning: the font size and which tab is selected are both remembered by the shell, not
/// by a terminal, so this is the seam the shell uses to re-push them. See
/// <c>MainWindowViewModel</c>'s subscription for the other half.
/// </para>
/// <para>
/// Raised on the socket-accept thread, same as <see cref="TerminalDataPlane.SocketAttached"/> that
/// triggers it — a handler that touches a view model has to marshal.
/// </para>
/// </remarks>
public event EventHandler? RendererReattached;
/// <summary>Starts the loopback listener.</summary> /// <summary>Starts the loopback listener.</summary>
public void Start() => server = dataPlane.RunAsync(lifetime.Token); public void Start() => server = dataPlane.RunAsync(lifetime.Token);
@@ -563,6 +614,71 @@ public sealed class TerminalWorkspace : IAsyncDisposable
lifetime.Dispose(); lifetime.Dispose();
} }
/// <summary>
/// Rebuilds a freshly (re)attached page's idea of what is running, then tells the shell to rebuild its
/// own.
/// </summary>
/// <remarks>
/// <para>
/// Runs on the socket-accept thread that raised <see cref="TerminalDataPlane.SocketAttached"/> — the
/// constructor wires it up fire-and-forget for exactly that reason, so this method owns its own error
/// handling rather than leaving an unobserved exception for nobody to see.
/// </para>
/// <para>
/// Every live session — one whose <c>Run</c> has not completed — gets two things. Its credit window is
/// reset, because whatever was outstanding was reserved against bytes sent to a page that is now gone;
/// the acknowledgement that would return that credit died with it, and without this reset the session
/// would stall the moment 256 KiB of history had accumulated. And it gets its <c>SessionOpened</c> frame
/// again, marked with <see cref="ReplayMarker"/> so the page can tell a reattach from a session that is
/// genuinely new — the same frame a page that survived the socket drop already has a pane for, and one a
/// reloaded page does not.
/// </para>
/// <para>
/// A session whose shell has already ended gets nothing here. Its scrollback lived only in the page that
/// is gone, and sending a frame that implied otherwise would be exactly the kind of dishonesty this
/// fix is supposed to remove, not add. The tab strip still shows that session ended; nothing about this
/// method changes what <see cref="LiveSessionCount"/> or <see cref="IsSessionLive"/> report.
/// </para>
/// <para>
/// This can race the fresh page's own first frames — an early resize, an acknowledgement for output it
/// already had. That is harmless: every frame in both directions names its session, delivery order
/// within a session is preserved by both xterm and the socket, and a frame for a pane the page has not
/// created yet is simply dropped, the same as any frame for a session it does not know — see
/// <c>terminal.js</c>'s <c>handleFrame</c>.
/// </para>
/// </remarks>
private async Task ReplayAfterAttachAsync()
{
KeyValuePair<uint, LiveSession>[] live;
lock (sessionGate)
{
live = [.. sessions.Where(entry => !entry.Value.Run.IsCompleted)];
}
try
{
foreach (var (sessionId, session) in live)
{
session.Pump.Credits.Reset();
await dataPlane
.SendAsync(
TerminalFrame.Create((byte)TerminalServerOpcode.SessionOpened, sessionId, ReplayMarker),
CancellationToken.None)
.ConfigureAwait(false);
}
RendererReattached?.Invoke(this, EventArgs.Empty);
}
catch (Exception exception) when (exception is not OutOfMemoryException)
{
// Best-effort, same as every other fire-and-forget path here: a page that dies again mid-replay
// leaves nothing worse than the problem this method exists to fix, and there is no caller on
// this thread left to hand a failure to.
}
}
private async Task RunSessionAsync(uint sessionId, TerminalSessionPump pump) private async Task RunSessionAsync(uint sessionId, TerminalSessionPump pump)
{ {
try try
@@ -2,6 +2,7 @@ using System.Globalization;
using DodoSSH.Client.Auth; using DodoSSH.Client.Auth;
using DodoSSH.Client.Domain; using DodoSSH.Client.Domain;
using DodoSSH.Client.Import; using DodoSSH.Client.Import;
using DodoSSH.Client.ObjectStore;
using DodoSSH.Client.Session; using DodoSSH.Client.Session;
// FakeDeviceKeyStore is compiled into this assembly from a source link and keeps its original namespace; // FakeDeviceKeyStore is compiled into this assembly from a source link and keeps its original namespace;
// see the csproj for why it is shared rather than reimplemented. // see the csproj for why it is shared rather than reimplemented.
@@ -8059,6 +8060,221 @@ public sealed class ShellFlowTests : IAsyncLifetime
shell.Transfers.RemoteEntries.Select(entry => entry.Name).ShouldBe(["notes.txt"]); shell.Transfers.RemoteEntries.Select(entry => entry.Name).ShouldBe(["notes.txt"]);
} }
/// <remarks>
/// The phone's Files-screen chip row, proven at the view model rather than through Avalonia: a pin
/// saved on the host before this screen ever connects to it is read straight off the row's own
/// <c>HostSecret.PinnedPaths</c> at the moment <c>MarkHostConnected</c> runs, which is what
/// <see cref="TransfersViewModel.ConnectedPinnedPaths"/>'s own remark promises rather than a live follow
/// of the vault.
/// </remarks>
[Fact]
public async Task ConnectingATransfersHostWithPins_PopulatesConnectedPinnedPaths()
{
var vault = await ReadyToConnectAsync();
vault.EditSelectedHostCommand.Execute(null);
vault.EditorNewPin = "/var/www/app";
vault.AddEditorPinCommand.Execute(null);
await vault.SaveHostCommand.ExecuteAsync(null);
shell.Transfers.Attach(vault, knownHosts);
shell.Transfers.SelectedHost = shell.Transfers.Hosts[0];
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status);
shell.Transfers.ConnectedPinnedPaths.ShouldBe(["/var/www/app"]);
shell.Transfers.HasConnectedPins.ShouldBeTrue();
}
/// <remarks>
/// The other half of <see cref="ConnectingATransfersHostWithPins_PopulatesConnectedPinnedPaths"/>: the
/// chip row has to go with the connection it belongs to, or a later connect to a host with no pins would
/// show the previous host's.
/// </remarks>
[Fact]
public async Task DisconnectingTheTransfersScreen_ClearsConnectedPinnedPaths()
{
var vault = await ReadyToConnectAsync();
vault.EditSelectedHostCommand.Execute(null);
vault.EditorNewPin = "/var/www/app";
vault.AddEditorPinCommand.Execute(null);
await vault.SaveHostCommand.ExecuteAsync(null);
shell.Transfers.Attach(vault, knownHosts);
shell.Transfers.SelectedHost = shell.Transfers.Hosts[0];
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.HasConnectedPins.ShouldBeTrue();
await shell.Transfers.DisconnectCommand.ExecuteAsync(null);
shell.Transfers.ConnectedPinnedPaths.ShouldBeEmpty();
shell.Transfers.HasConnectedPins.ShouldBeFalse();
}
/// <remarks>
/// The phone's foreground-service question, proven at the view model rather than through Android: a
/// connect that opens an SFTP session is exactly the transition <c>SessionKeepAlive</c> needs to hear
/// about even when no transfer ever moves — see <see cref="TransfersViewModel.ActivityChanged"/>'s own
/// remark for why the queue's own raise, in <c>OnTransferChanged</c>, cannot cover a connect that never
/// touches <c>Transfers</c> at all.
/// </remarks>
[Fact]
public async Task ConnectingATransfersHost_RaisesActivityChangedAndTurnsOnHasLiveFileSession()
{
var vault = await ReadyToConnectAsync();
shell.Transfers.Attach(vault, knownHosts);
shell.Transfers.SelectedHost = shell.Transfers.Hosts[0];
var raised = 0;
shell.Transfers.ActivityChanged += (_, _) => raised++;
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status);
shell.Transfers.HasLiveFileSession.ShouldBeTrue();
raised.ShouldBeGreaterThan(0);
}
/// <remarks>
/// The other half: a disconnect is as much a transition the service must hear about as a connect is,
/// because it is the moment the connection <see cref="TransfersViewModel.HasLiveFileSession"/> promised
/// was open stops being true — and the foreground service would otherwise keep the process alive over a
/// session that has already closed.
/// </remarks>
[Fact]
public async Task DisconnectingTheTransfersScreen_RaisesActivityChangedAndTurnsOffHasLiveFileSession()
{
var vault = await ReadyToConnectAsync();
shell.Transfers.Attach(vault, knownHosts);
shell.Transfers.SelectedHost = shell.Transfers.Hosts[0];
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.HasLiveFileSession.ShouldBeTrue();
var raised = 0;
shell.Transfers.ActivityChanged += (_, _) => raised++;
await shell.Transfers.DisconnectCommand.ExecuteAsync(null);
shell.Transfers.HasLiveFileSession.ShouldBeFalse();
raised.ShouldBeGreaterThan(0);
}
/// <remarks>
/// A bucket is an <c>IRemoteFileStore</c> with no <c>HostSecret</c> underneath it, so there is no
/// <c>PinnedPaths</c> to read at all — see <see cref="TransfersViewModel.OpenBucketAsync"/>'s own remark.
/// The bucket here is created through the same keychain route
/// <see cref="TheS3ScreenWithNoBuckets_SaysWhereOneIsMadeAndGoesThere"/> exercises, and
/// <see cref="FakeObjectStoreFactory"/> stands in for the network the way <see cref="FakeSshConnectionFactory"/>
/// already does for SFTP.
/// </remarks>
[Fact]
public async Task ConnectingABucket_LeavesConnectedPinnedPathsEmpty()
{
var vault = await ReadyToConnectAsync();
shell.Transfers.Attach(vault, knownHosts, buckets: new FakeObjectStoreFactory());
vault.NewObjectStoreCommand.Execute(null);
vault.BucketEditorLabel = "Backups";
vault.BucketEditorBucket = "backups";
vault.BucketEditorAccessKeyId = "AKIAEXAMPLE";
vault.BucketEditorSecretAccessKey = "a-secret-access-key";
vault.BucketEditorRegion = "eu-west-1";
await vault.SaveObjectStoreCommand.ExecuteAsync(null);
// Remote is what ConnectAsync branches on, and Attach's RefreshHosts has already auto-selected the
// host ReadyToConnectAsync left in the picker — without this line the command below dialled that
// host, and every assertion here passed only because that host happens to have no pins either. The
// ConnectedTo check is the proof the bucket path was actually taken.
shell.Transfers.Remote = RemoteKind.Bucket;
shell.Transfers.SelectedBucket = shell.Transfers.Buckets[0];
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.ConnectedTo.ShouldBe("s3://backups");
shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status);
shell.Transfers.ConnectedPinnedPaths.ShouldBeEmpty();
shell.Transfers.HasConnectedPins.ShouldBeFalse();
}
/// <remarks>
/// A bucket is HTTP, per-request, with nothing open that a dying process would lose — see
/// <see cref="TransfersViewModel.HasLiveFileSession"/>'s own remark. <c>IsConnected</c> alone would have
/// answered this wrongly, which is exactly why the flag reads <c>ConnectedCipher</c> as well: nothing
/// underneath a bucket ever sets it.
/// </remarks>
[Fact]
public async Task ConnectingABucket_LeavesHasLiveFileSessionOff()
{
var vault = await ReadyToConnectAsync();
shell.Transfers.Attach(vault, knownHosts, buckets: new FakeObjectStoreFactory());
vault.NewObjectStoreCommand.Execute(null);
vault.BucketEditorLabel = "Backups";
vault.BucketEditorBucket = "backups";
vault.BucketEditorAccessKeyId = "AKIAEXAMPLE";
vault.BucketEditorSecretAccessKey = "a-secret-access-key";
vault.BucketEditorRegion = "eu-west-1";
await vault.SaveObjectStoreCommand.ExecuteAsync(null);
// ReadyToConnectAsync already left a host in the picker, and Attach's own RefreshHosts auto-selects
// it — so without this the CONNECT command below would dial that host rather than open the bucket,
// and a host with no pins would make ConnectedPinnedPathsEmpty-style assertions pass for the wrong
// reason. Remote is what ConnectAsync actually branches on.
shell.Transfers.Remote = RemoteKind.Bucket;
shell.Transfers.SelectedBucket = shell.Transfers.Buckets[0];
await shell.Transfers.ConnectCommand.ExecuteAsync(null);
shell.Transfers.ConnectedTo.ShouldBe("s3://backups", "proof this opened the bucket rather than the host");
shell.Transfers.IsConnected.ShouldBeTrue(shell.Transfers.Status);
shell.Transfers.HasLiveFileSession.ShouldBeFalse();
}
/// <summary>A bucket that opens and lists as empty, so a bucket connect can be proven with no network.</summary>
private sealed class FakeObjectStoreFactory : IObjectStoreFactory
{
public IRemoteFileStore Open(ObjectStoreSecret store) => new FakeBucketStore();
}
/// <summary>The minimum <see cref="IRemoteFileStore"/> a bucket connect touches: home, then a listing.</summary>
private sealed class FakeBucketStore : IRemoteFileStore
{
public bool IsConnected => true;
public string HomeDirectory => "/";
public Task<IReadOnlyList<SftpEntry>> ListAsync(string path, CancellationToken cancellationToken) =>
Task.FromResult<IReadOnlyList<SftpEntry>>([]);
public Task<SftpEntry?> StatAsync(string path, CancellationToken cancellationToken) =>
Task.FromResult<SftpEntry?>(null);
public Task<Stream> OpenReadAsync(string path, long offset, CancellationToken cancellationToken) =>
throw new NotSupportedException("Not exercised by proving a bucket connect leaves no pins.");
public Task<Stream> OpenWriteAsync(string path, long offset, CancellationToken cancellationToken) =>
throw new NotSupportedException("Not exercised by proving a bucket connect leaves no pins.");
public Task CreateDirectoryAsync(string path, CancellationToken cancellationToken) =>
throw new NotSupportedException("Not exercised by proving a bucket connect leaves no pins.");
public Task DeleteAsync(string path, CancellationToken cancellationToken) =>
throw new NotSupportedException("Not exercised by proving a bucket connect leaves no pins.");
public Task RenameAsync(string fromPath, string toPath, CancellationToken cancellationToken) =>
throw new NotSupportedException("Not exercised by proving a bucket connect leaves no pins.");
public ValueTask DisposeAsync() => ValueTask.CompletedTask;
}
/// <remarks> /// <remarks>
/// <para> /// <para>
/// The picker that replaced the desktop's connect bar, and the four things that put it away again. It /// The picker that replaced the desktop's connect bar, and the four things that put it away again. It
@@ -8,6 +8,8 @@ internal sealed class FakeShellSession : ISshShellSession
private readonly List<byte> written = []; private readonly List<byte> written = [];
private readonly Lock gate = new(); private readonly Lock gate = new();
private readonly bool blockReads;
private long remaining; private long remaining;
private byte pattern; private byte pattern;
@@ -16,7 +18,19 @@ internal sealed class FakeShellSession : ISshShellSession
/// endless producer, which is what a runaway remote process looks like — those sessions are ended /// endless producer, which is what a runaway remote process looks like — those sessions are ended
/// by disposing the pump rather than by running out of data. /// by disposing the pump rather than by running out of data.
/// </param> /// </param>
internal FakeShellSession(long bytesToProduce = 0) => remaining = bytesToProduce; /// <param name="blockReads">
/// True for a shell that is open and live but has nothing to say — an idle prompt, rather than either
/// end of the "produces bytes" and "hit end of stream" spectrum <paramref name="bytesToProduce"/>
/// covers. <see cref="ReadAsync"/> then blocks until cancelled, which is what a real idle SSH channel's
/// read does. Exists for tests that need a session whose <c>Run</c> stays live without a background
/// read loop racing the test for control of the pump's credit window — see the reattach tests in
/// <c>TerminalWorkspaceTests</c>.
/// </param>
internal FakeShellSession(long bytesToProduce = 0, bool blockReads = false)
{
remaining = bytesToProduce;
this.blockReads = blockReads;
}
/// <inheritdoc /> /// <inheritdoc />
public bool IsOpen { get; private set; } = true; public bool IsOpen { get; private set; } = true;
@@ -52,6 +66,13 @@ internal sealed class FakeShellSession : ISshShellSession
{ {
ReadCount++; ReadCount++;
if (blockReads)
{
// Never completes on its own. The only way out is the same way a real blocked read ends: the
// token being cancelled, which is what disposing the pump does.
await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken).ConfigureAwait(false);
}
await Task.Yield(); await Task.Yield();
cancellationToken.ThrowIfCancellationRequested(); cancellationToken.ThrowIfCancellationRequested();
@@ -109,7 +130,8 @@ internal sealed class FakeShellSession : ISshShellSession
/// workspace is the layer that decides when a session is over, and that decision is what needs a /// workspace is the layer that decides when a session is over, and that decision is what needs a
/// connection whose shell can be made to end on cue. /// connection whose shell can be made to end on cue.
/// </remarks> /// </remarks>
internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue) : ISshConnectionFactory internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue, bool blockShellReads = false)
: ISshConnectionFactory
{ {
/// <summary>Connections handed out, in order.</summary> /// <summary>Connections handed out, in order.</summary>
internal List<FakeConnection> Connections { get; } = []; internal List<FakeConnection> Connections { get; } = [];
@@ -119,7 +141,7 @@ internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue)
SshConnectionRequest request, SshConnectionRequest request,
CancellationToken cancellationToken) CancellationToken cancellationToken)
{ {
var connection = new FakeConnection(request, bytesPerShell); var connection = new FakeConnection(request, bytesPerShell, blockShellReads);
Connections.Add(connection); Connections.Add(connection);
return Task.FromResult<ISshConnection>(connection); return Task.FromResult<ISshConnection>(connection);
@@ -127,7 +149,8 @@ internal sealed class FakeConnectionFactory(long bytesPerShell = long.MaxValue)
} }
/// <summary>A connection that opens fake shells and records its own disposal.</summary> /// <summary>A connection that opens fake shells and records its own disposal.</summary>
internal sealed class FakeConnection(SshConnectionRequest request, long bytesPerShell) : ISshConnection internal sealed class FakeConnection(SshConnectionRequest request, long bytesPerShell, bool blockShellReads = false)
: ISshConnection
{ {
/// <inheritdoc /> /// <inheritdoc />
public bool IsConnected { get; private set; } = true; public bool IsConnected { get; private set; } = true;
@@ -148,7 +171,7 @@ internal sealed class FakeConnection(SshConnectionRequest request, long bytesPer
/// <inheritdoc /> /// <inheritdoc />
public Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken) public Task<ISshShellSession> OpenShellAsync(TerminalSize size, CancellationToken cancellationToken)
{ {
Shell = new FakeShellSession(bytesPerShell); Shell = new FakeShellSession(bytesPerShell, blockShellReads);
return Task.FromResult<ISshShellSession>(Shell); return Task.FromResult<ISshShellSession>(Shell);
} }
@@ -142,15 +142,96 @@ public sealed class TerminalDataPlaneTests : IAsyncDisposable
await ConnectAsync(origin: "https://evil.example")); await ConnectAsync(origin: "https://evil.example"));
} }
/// <remarks>
/// The truth this replaced: a second valid attach used to be a 409, on the theory that one renderer
/// lives for the whole process. Android's WebView does not honour that theory — its renderer process is
/// routinely killed and the page reloads with a fresh socket — so a second valid attach is now a
/// takeover. This asserts both halves: the newcomer gets the connection, and the displaced socket
/// actually goes rather than lingering as a phantom nothing is reading from.
/// </remarks>
[Fact] [Fact]
public async Task ASecondRenderer_IsRejected() public async Task ASecondRenderer_TakesOver_AndTheFirstSocketIsDropped()
{ {
Start(); Start();
using var first = await ConnectAsync(); using var first = await ConnectAsync();
first.State.ShouldBe(WebSocketState.Open); first.State.ShouldBe(WebSocketState.Open);
await Should.ThrowAsync<WebSocketException>(async () => await ConnectAsync()); using var second = await ConnectAsync();
second.State.ShouldBe(WebSocketState.Open);
// The first socket was aborted rather than closed gracefully — Abort skips the close handshake
// entirely, so there is no Close frame for this side to see coming. What a receive on it sees
// instead is the connection simply gone, which the client surfaces as an exception rather than as
// a state that quietly flips on its own; nothing here reads from the socket otherwise, so the
// state alone would not move.
var firstBuffer = new byte[16];
await Should.ThrowAsync<Exception>(async () =>
await first.ReceiveAsync(firstBuffer.AsMemory(), TestContext.Current.CancellationToken));
await using var session = new FakeShellSession(bytesToProduce: 64);
await using var pump = CreatePump(session);
plane.Register(SessionId, pump);
var run = pump.RunAsync(TestContext.Current.CancellationToken);
var opened = await ReceiveAsync(second);
opened.Opcode.ShouldBe((byte)TerminalServerOpcode.SessionOpened);
var output = await ReceiveAsync(second);
output.Opcode.ShouldBe((byte)TerminalServerOpcode.Output);
output.Payload.Length.ShouldBe(64);
await SendAsync(
second,
(byte)TerminalClientOpcode.Acknowledge,
TerminalFrame.CreateAcknowledgementPayload((uint)output.Payload.Length));
await run;
}
/// <remarks>
/// <para>
/// The other half of the takeover: a renderer process that dies without a close handshake — which is
/// what a killed Android WebView actually does, no FIN, nothing — must not fault the send path. A
/// faulted send would propagate into <see cref="TerminalSessionPump"/>'s flush loop and freeze a live
/// session; see <see cref="TerminalDataPlane.SendAsync"/>'s remark for why. Disposing the client socket
/// abruptly, with no close handshake sent, is the closest this harness gets to that: the server-side
/// socket is left believing itself open until it actually tries to write to it.
/// </para>
/// <para>
/// Driven straight through <see cref="TerminalDataPlane.SendAsync"/> rather than through a pump, because
/// a pump adds nothing here — the point is entirely about the transport's own contract, and a session
/// layered on top would only leave it unclear whether a passing test proved the transport never threw or
/// merely that the frames never happened to need a live socket.
/// </para>
/// </remarks>
[Fact]
public async Task SendAsync_DoesNotThrow_WhenTheAttachedRendererDiedWithoutClosing_AndAFreshAttachStillReceives()
{
Start();
var first = await ConnectAsync();
first.State.ShouldBe(WebSocketState.Open);
first.Dispose();
// Whether this particular send lands on the OS's send buffer before the peer's absence is noticed,
// or fails immediately, is not the point — either way it must not throw.
await Should.NotThrowAsync(async () =>
await plane.SendAsync(
TerminalFrame.Create((byte)TerminalServerOpcode.Output, SessionId, "before"u8.ToArray()),
TestContext.Current.CancellationToken));
using var second = await ConnectAsync();
await Should.NotThrowAsync(async () =>
await plane.SendAsync(
TerminalFrame.Create((byte)TerminalServerOpcode.Output, SessionId, "after"u8.ToArray()),
TestContext.Current.CancellationToken));
var output = await ReceiveAsync(second);
output.Opcode.ShouldBe((byte)TerminalServerOpcode.Output);
Encoding.UTF8.GetString(output.Payload).ShouldBe("after");
} }
// ---- Frames ---- // ---- Frames ----
@@ -1,3 +1,6 @@
using System.Globalization;
using System.Net.WebSockets;
using System.Text;
using DodoSSH.Client.Ssh; using DodoSSH.Client.Ssh;
namespace DodoSSH.Client.Terminal.Tests; namespace DodoSSH.Client.Terminal.Tests;
@@ -291,6 +294,85 @@ public sealed class TerminalWorkspaceTests
connections.Connections.ShouldAllBe(connection => connection.IsDisposed); connections.Connections.ShouldAllBe(connection => connection.IsDisposed);
} }
// ---- Reattach ----
/// <remarks>
/// <para>
/// The scenario the whole fix exists for: a page that lost its socket — killed WebView renderer, or
/// simply a reload — reattaches, and the session that was already running has to come back rather than
/// sit there forever with its output going nowhere.
/// </para>
/// <para>
/// The session's shell blocks on every read rather than producing output, which is what an idle prompt
/// looks like and — for this test — is what keeps its <c>Run</c> live without a background read loop
/// competing with this test over the credit window's exact value.
/// </para>
/// <para>
/// Neither assertion below polls, deliberately. The "before" one does not need to: reserving credit is
/// a synchronous call, so it is true the instant it returns. The "after" one does not need to either,
/// for a subtler reason — <see cref="TerminalWorkspace.ReplayAfterAttachAsync"/> calls
/// <c>Credits.Reset()</c> and only then awaits sending the replay frame for that same session, with no
/// suspension between the two, so by the time this test has received that frame the reset has
/// necessarily already happened. A poll here would only have hidden a real ordering bug behind a
/// generous timeout instead of catching it.
/// </para>
/// </remarks>
[Fact]
public async Task ANewRenderer_ReplaysTheLiveSessionAndResetsItsCredits()
{
var connections = new FakeConnectionFactory(blockShellReads: true);
await using var workspace = CreateWorkspace(connections);
workspace.Start();
using var first = await ConnectRendererAsync(workspace);
var sessionId = await workspace.OpenSessionAsync(
Request(), TerminalSize.Default, TestContext.Current.CancellationToken);
// The session's own opening frame, sent as soon as the pump starts running. Not a replay, and not
// what this test is about — read and discarded so it cannot be confused for one below.
await ReceiveFrameAsync(first);
var credits = workspace.CreditsFor(sessionId).ShouldNotBeNull();
credits.TryReserve(4096);
credits.Outstanding.ShouldBeGreaterThanOrEqualTo(
4096, "the pump's own read loop may have reserved a buffer's worth on top of this");
using var second = await ConnectRendererAsync(workspace);
var replay = await ReceiveFrameAsync(second);
replay.Opcode.ShouldBe((byte)TerminalServerOpcode.SessionOpened);
replay.SessionId.ShouldBe(sessionId);
replay.Payload.ShouldBe(new byte[] { 1 }, "a replay is flagged so the page can tell it apart from a fresh open");
credits.Outstanding.ShouldBe(0, "the replay frame above cannot have been sent before the reset that precedes it");
}
/// <remarks>
/// The other half of a reattach: the workspace has replayed what it owns, and this is the seam the
/// shell uses to replay what it owns instead — the font size and the selected tab, neither of which a
/// terminal session knows anything about. <c>MainWindowViewModel</c>'s subscription is what actually
/// does that; this only asserts that the workspace hands it the chance to.
/// </remarks>
[Fact]
public async Task ANewRenderer_RaisesRendererReattached()
{
var connections = new FakeConnectionFactory();
await using var workspace = CreateWorkspace(connections);
workspace.Start();
using var first = await ConnectRendererAsync(workspace);
var reattachedCount = 0;
workspace.RendererReattached += (_, _) => Interlocked.Increment(ref reattachedCount);
using var second = await ConnectRendererAsync(workspace);
await WaitUntilAsync(() => Volatile.Read(ref reattachedCount) > 0);
}
// ---- Helpers ---- // ---- Helpers ----
/// <remarks> /// <remarks>
@@ -362,12 +444,76 @@ public sealed class TerminalWorkspaceTests
private static InMemoryTerminalAssetProvider StubAssets() => private static InMemoryTerminalAssetProvider StubAssets() =>
new(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal) new(new Dictionary<string, TerminalAsset>(StringComparer.Ordinal)
{ {
[TerminalDataPlane.PagePath] = new("text/html; charset=utf-8", "<!doctype html>"u8.ToArray()), // The placeholders, not a token and URL already filled in — the reattach tests below have to
// connect a real renderer, and doing that by reading them back out of the served page is what
// proves the workspace serves a page a real renderer could actually attach with, rather than
// one that merely looks servable.
[TerminalDataPlane.PagePath] = new(
"text/html; charset=utf-8",
Encoding.UTF8.GetBytes(
$"<html><body data-token=\"{TerminalDataPlane.TokenPlaceholder}\" "
+ $"data-socket=\"{TerminalDataPlane.SocketUrlPlaceholder}\"></body></html>")),
}); });
private static SshConnectionRequest Request() => private static SshConnectionRequest Request() =>
new("host.invalid", 22, "dodo", new SshPasswordCredential("irrelevant")); new("host.invalid", 22, "dodo", new SshPasswordCredential("irrelevant"));
/// <remarks>
/// Attaches the way the real page does: by fetching the served page, reading the token and socket URL
/// back out of it, and presenting them on the upgrade — rather than reaching into the workspace for a
/// token it does not expose. A shortcut here would prove only that a socket can be opened, not that the
/// workspace serves a page a renderer could actually attach with.
/// </remarks>
private static async Task<ClientWebSocket> ConnectRendererAsync(TerminalWorkspace workspace)
{
using var http = new HttpClient();
var page = await http.GetStringAsync(workspace.PageUrl, TestContext.Current.CancellationToken);
var token = ExtractAttribute(page, "data-token");
var socketUrl = ExtractAttribute(page, "data-socket");
var client = new ClientWebSocket();
client.Options.AddSubProtocol(TerminalDataPlane.SubProtocol);
client.Options.AddSubProtocol($"token.{token}");
client.Options.SetRequestHeader(
"Origin",
string.Create(CultureInfo.InvariantCulture, $"http://127.0.0.1:{workspace.PageUrl.Port}"));
try
{
await client.ConnectAsync(new Uri(socketUrl), TestContext.Current.CancellationToken);
}
catch
{
client.Dispose();
throw;
}
return client;
}
private static string ExtractAttribute(string html, string name)
{
var marker = $"{name}=\"";
var start = html.IndexOf(marker, StringComparison.Ordinal) + marker.Length;
var end = html.IndexOf('"', start);
return html[start..end];
}
private static async Task<(byte Opcode, uint SessionId, byte[] Payload)> ReceiveFrameAsync(
ClientWebSocket socket)
{
var buffer = new byte[64 * 1024];
var result = await socket.ReceiveAsync(buffer.AsMemory(), TestContext.Current.CancellationToken);
TerminalFrame.TryRead(buffer.AsSpan(0, result.Count), out var opcode, out var sessionId, out var payload)
.ShouldBeTrue();
return (opcode, sessionId, payload.ToArray());
}
/// <remarks> /// <remarks>
/// Polled rather than awaited on a task, because the point is what an observer of the property /// Polled rather than awaited on a task, because the point is what an observer of the property
/// sees: the pump ends on a thread of its own, and the count has to catch up without anyone /// sees: the pump ends on a thread of its own, and the count has to catch up without anyone