Commit Graph
6 Commits
Author SHA1 Message Date
jaap-jan 9a76eced14 Give hosts and terminals their own screen, and the rest of the vault another
ci / build and test (ubuntu) (pull_request) Canceled after 0s
ci / build (windows) (pull_request) Canceled after 0s
Rebuilds the client's shell from an imported design: a titlebar and nav rail
it draws itself, real multi-session tabs over the one WebView, a Ctrl+K host
search, and a vault screen that merges keys, passwords and pinned host keys
into one table. Hosts left the vault column for their own screen beside the
terminal, which is what the design asks for and turned out to be the better
split anyway.

Two screens the design shows have nothing behind them yet — file transfer
and teams — and say so plainly rather than rendering invented data; every
other gap between the design and this build is recorded in
docs/design-import-gaps.md.
2026-07-31 08:39:37 +02:00
jaap-jan 586cb303d5 Merge branch 'claude/gallant-brahmagupta-1f8244'
ci / build and test (ubuntu) (push) Has been cancelled
ci / build (windows) (push) Has been cancelled
Writes down that locking the vault leaves shells running, and shows the count on
the unlock screen rather than leaving it to be inferred.

Conflict resolution:

- ShellFlowTests' fixture keeps main's FakeSshConnectionFactory. The branch added
  an IdleSshConnectionFactory for exactly what main's fake already does — a shell
  that is open, silent and never closes on its own — so FakeSshConnections.cs is
  dropped rather than merged, leaving one fake SSH stack in the suite instead of
  two that would drift apart.
- MainWindowViewModel and TerminalWorkspace: both sides added their own members,
  so both are kept.
- TerminalWorkspaceTests was added by both branches, with the renderer gate on
  one side and session lifetime on the other. Merged into one class over one set
  of helpers; the gate tests now use FakeConnectionFactory rather than an
  NSubstitute stub, since the suite already has the fake. gallant's polling
  Timeout constant is PollTimeout, which no longer reads as the renderer's.
- platform-flags.md keeps main's measured focus section and drops the short
  "nothing hands the terminal keyboard focus" entry the branch still carried,
  which that section supersedes.

One genuine disagreement between the branches, left visible rather than
flattened: this branch measured that a collapsed WebView cannot be typed into and
attributed it to a hidden WS_CHILD window being ineligible for keyboard focus,
while main's focus work measured Win32 focus still held by that hidden window and
added a lock path that moves the keyboard off it. Both results stand; the
mechanism sentence now defers to the focus entry, which makes the input barrier
something the lock path maintains rather than something the platform guarantees.

Full suite green, including the container-backed SSH tests.
2026-07-29 15:46:41 +02:00
jaap-jan 5a899afd78 Decide what Lock does to a running shell, and say it
Pressing Lock nulled and disposed the vault view model and touched nothing else.
TerminalWorkspace is injected from App.axaml.cs and outlives every lock, so the SSH
connection, the pty and the pump all kept running while the window said "Unlock your
vault" — and since 0500e43 collapsed the WebView while locked, that live session was
invisible as well as unstopped. CloseSessionAsync was reachable in production only from
DisposeAsync, i.e. shutdown. None of this was written down anywhere, so it was neither a
policy nor a bug, which is the actual problem.

Shells now deliberately outlive the lock, and every layer says so.

The reason to prefer this over making Lock a disconnect: locking is what a person does
when they walk away from the machine, which is exactly when a long upgrade, build or
transfer is most likely to be in flight. Ending every shell would make Lock a button that
destroys work, and the predictable response is to stop pressing it and leave the vault
open instead. The idle auto-lock this will grow decides it outright — an unattended
timeout that killed a running job would be worse than the exposure it removes. Closing
the channel also buys less than it looks: the session was authorised at connect time by a
credential the remote verified itself, and no vault key participates in keeping it alive,
so locking cannot retroactively un-authorise it any more than removing a member can.

Stated honestly rather than implied, because the lock screen is what hides it:

- The unlock screen shows how many shells are still connected, and that locking closes
  the vault and not the connections — so a machine still holding authenticated SSH
  channels does not present itself as merely "locked". Shown only when there is something
  to disclose. Quitting is what ends them, and the text admits that.
- The Lock button carries the same thing in a tooltip, since its name implies the
  opposite of what it does to a shell.
- README lists it as a third architecture consequence beside non-retroactive revocation,
  which is the same shape of honest limit; docs/crypto.md §10 records it as a threat-model
  boundary; TerminalWorkspace and LockAsync carry the argument next to the code.

LiveSessionCount deliberately does not count dictionary entries. Nothing removes a
session when the remote closes the channel by itself — RunSessionAsync only drops the
renderer registration — so sessions.Count would report a shell that exited half an hour
ago as still running, on the one screen where a user is deciding whether it is safe to
walk away. A completed Run task is what "the shell is gone" actually looks like. While
locked the number can only fall, since opening a session needs the vault, so a stale
value over-reports rather than under-reports.

Both new tests fail when the policy is reverted: the count test times out against
sessions.Count, and the shell test reports "workspace.LiveSessionCount should be 1 but was
0" when Lock closes sessions. ShellFlowTests also stops building its workspace with a real
SshNetConnectionFactory that nothing ever called, which had made the suite's independence
from the network a coincidence rather than a property.

Verified by hand with a live shell, which nothing had done: a harness mirroring
MainWindow.axaml's 340,* grid with a real NativeWebView, the shipped WebAssets, a real
sshd in a container, and an ISshShellSession decorator recording every window-change the
remote is actually told about. Across lock and unlock, no window-change reached the remote
at all, stty size answered 50 118 before and after, the renderer's own buffer came back
byte for byte with the wrapped line intact, and the session stayed live throughout. A
control run that never hides the WebView behaves identically, so nothing above is startup
or idle behaviour. Keystrokes injected while locked reach nothing: twelve of twelve
SendInput events accepted with the harness confirmed as the foreground window, no probe
character in the remote's output, and a following Ctrl-U answered BEL, so nothing was
queued in the line editor either. A hidden WS_CHILD window is not eligible for keyboard
focus, which is what makes surviving the lock defensible rather than merely convenient.

Correction to a claim made in f80b3d4: terminal.js's guard comment listed "a host that
hides the WebView while the vault is locked" among the paths that reach a degenerate fit.
It does not. Collapsing the control hides a native child window without resizing it, so
the page still reports paneWidth 840 and paneHeight 760 with unchanged cols and rows, no
ResizeObserver callback fires and the fit never runs. Establishing that rather than
assuming it: the same cycle with MINIMUM_FITTABLE_PIXELS patched to 0 — the guard fully
disabled — is equally clean. The guard is still right for minimising and for a splitter
dragged to the edge; it is simply not what makes locking safe, and must not be cited as
though it were.

Recorded, not fixed:

- Nothing closes one terminal from the interface, so a user reading "1 shell is still
  connected" can only act on it by quitting. CloseSessionAsync is tested and correct;
  VaultViewModel discards the session id it would need.
- A session whose remote exits keeps its ISshConnection, and the thread ShellStream parks,
  until the process ends.
- Suspected and seen once: before the harness waited for the window's scale to settle, a
  DPI settle pushed a 2202x1328 pane for a window 1180 logical units wide and a later
  re-push reflowed the wrapped line. Three later runs at RenderScaling 1.00 never showed
  it, so it is filed as a lead, not a finding.
- WebView2 fails to initialise with CO_E_SERVER_EXEC_FAILURE when the host executable
  sits under a very long path. Cost an hour on the harness; relevant to packaging.
2026-07-29 14:44:02 +02:00
jaap-jan dbddbcd711 Hand the terminal the keyboard on connect, and take it back on lock
After a successful connect the first keystrokes went to the shell's UI rather
than the remote shell. The page's own term.focus() focuses the textarea inside
the document, which does nothing while the window's keyboard focus is still on
the Connect button, so the terminal had to be clicked before it would accept
anything.

The obvious guess about the fix — that reaching a native child window needs
SetFocus through P/Invoke — is backwards, and measuring it first is what kept
this small. NativeWebView overrides Focusable to true and its OnGotFocus calls
the adapter's Focus(), which on Windows is
ICoreWebView2Controller::MoveFocus(PROGRAMMATIC). So a plain Avalonia
Terminal.Focus() really does move Win32 focus into WebView2. Measured in a
standalone harness with no DodoSSH code, on the same 340,* grid as the shell,
reporting GetFocus() and the page's own document.hasFocus() at each step: focus
lands on the Chrome_WidgetWin_1 child and the page reports hasFocus: true.

It is the return trip the package does not implement. OnLostFocus calls the
adapter's ResignFocus(), and on Windows that method body is empty, so Avalonia's
focus and Win32's diverge: after textBox.Focus() the focused element is the text
box while the keyboard is still on WebView2 — a caret that silently receives
nothing. Window.Activate() and Window.Focus() were both measured and neither
recovers it, so the hand-back is a SetFocus on the top-level, in
Views/NativeKeyboardFocus.cs. A real mouse click does recover it, because
Avalonia's window sets focus on pointer input, which is why this is invisible to
anyone who clicks before typing.

That turned up a worse defect than the one being fixed, and it shipped in
0500e43. Collapsing the WebView does not release the keyboard: focus stays on
the hidden holder — measured held by a window reporting visible=False — while
Avalonia's focused element becomes (none). So a user who had clicked the
terminal and then pressed Lock got an unlock screen that swallowed the
passphrase. Locking now hands the keyboard back and focuses that box.

Ctrl+Shift+F6 is the way out for someone using only a keyboard. It has to be
handled in terminal.js and posted to the host as a web message, because once the
child window owns Win32 focus Avalonia receives no key events and no KeyBinding
could fire; the package also subscribes MoveFocusRequested and discards it, so
there is no Tab-out to lean on. Not Escape, which vim alone rules out, and not a
bare F6, which TUIs bind — Ctrl+Shift is the range terminal emulators
conventionally keep for themselves and never forward to the remote. Verified
rather than assumed: the posted string arrives verbatim in Body, and the chord
reaches the page as F6 with both modifiers.

Order matters and is now recorded. Focus() on a collapsed control is a measured
no-op and is not replayed when it is revealed, so focus survives a lock/unlock
cycle only because a session can be opened solely from an unlocked vault, which
is what reveals the control in the first place.

The view models still reference no view. VaultViewModel raises SessionOpened on
the success path only, the shell forwards it as TerminalSessionOpened through the
generated OnVaultChanged hook so unlock, lock and dispose all attach and detach
in one place, and the view holds the whole focus policy. An event rather than a
bound flag because connecting a second host while one is open has to move focus
again, and no state change describes that.

Three tests, and what they do not cover is the point. They cover the plumbing:
focus is asked for once per session, a failed connect does not ask at all — a
host-key prompt needs the keyboard on its own buttons — and locking stops the
forwarding. They cannot cover the focus call, because headless Avalonia has no
native window, so a headless test would focus correctly and confirm the wrong
belief; that is measured in the harness and written down in docs instead.
Dropping the forwarding fails two of them and dropping the detach fails one;
deleting the raise outright does not compile, since the event would be unused.

Reaching the connect path at all needed two new fakes. FakeRenderer attaches the
way the real page does — fetch the served page, read back the token and socket
URL the host substituted into it, then open the socket with both subprotocols —
rather than being handed the token, so the part of the handshake that has been
got wrong before stays under test. FakeSsh replaces a factory that would need a
reachable sshd, which DodoSSH.Client.Ssh.Tests already covers against a
container. The suite also never called workspace.Start(), so nothing served the
page and no renderer could have attached.

DllImport rather than the source-generated LibraryImport, which requires
AllowUnsafeBlocks for the whole project. The signature is blittable so there is
no marshalling stub to improve on, and turning unsafe code on across a client
that handles key material to gain nothing is a poor trade.

Correcting an earlier entry: docs/platform-flags.md described this as a
focus-plumbing gap and offered "click inside the terminal first" as the
workaround. Both true, and both stop short of the half that matters — focus
crosses into the WebView readily and never comes back on its own, which is the
same mechanism as the text boxes that mysteriously stopped accepting keystrokes
in the airspace entry above it, not a separate fault.
2026-07-29 14:31:09 +02:00
jaap-jan f80b3d4351 Harden the WebView collapse, and replace its evidence with a measurement
An adversarial review of 0500e43 did not refute the fix but closed the gap I
had left open and found three hazards around it. A standalone spike — a 60-line
Avalonia app with no DodoSSH code — reproduces the airspace bug on a 340,* grid,
and a second harness mirroring the data plane's handshake measures what I had
only reasoned about: with IsVisible=false set before the window is ever shown,
the adapter is created, the page is fetched and the WebSocket 101 is sent, with
frames arriving over the socket while hidden. A cold WebView2 profile behaves
the same. Revealing recomputes bounds in about 7 ms.

So the docs no longer cite "35 msedgewebview2 processes" as the confirmation
that the renderer attaches. A process count cannot show that a socket was
accepted — the same shape of mistake, one level down, as the one that entry was
already correcting. It now cites the handshake, quotes Avalonia's maintainer on
airspace being by design, and links the still-open upstream issue.

Three changes to the fix itself:

- terminal.js skips the fit below 40px in either axis. The vendored fit addon
  floors its proposal at 2 columns by 1 row rather than refusing, so a
  degenerate viewport reflows the *remote* pty through window-change and
  mangles wrapped scrollback unrecoverably. Reachable today by minimising, and
  by dragging a splitter to the edge once splits land — a guard where the sizes
  arrive, not a special case for one caller.
- FallbackValue=False on the binding. A compiled binding with no DataContext
  yields UnsetValue, IsVisible falls back to true, and the occlusion returns
  silently. Not reachable at runtime; it is what the previewer does.
- The comment now says why it must be IsVisible on this control: detaching
  destroys the native control and the whole WebView2 process tree, so
  conditional content would pay a cold start per unlock, and hoisting the
  binding to an ancestor is unverified because NativeWebView's own
  bounds-and-scaling re-push fires only for its own IsVisible.

Also recorded, not fixed: hiding does not suspend the page (visibilityState
stays "visible" and rAF keeps firing at ~115/s, which is *why* the handshake
completes while hidden); the conflict log can squeeze the terminal row toward
nothing; and nothing hands the terminal Win32 focus after Connect, so the first
keystrokes go to the shell's UI rather than the remote shell.
2026-07-29 13:42:23 +02:00
jaap-jan 5fccd53824 Add the Avalonia app and the xterm renderer, and fix two real bugs
The terminal works end to end. A new integration test drives a real sshd in
a container through a real PTY, the real pump, the real loopback WebSocket
with its token and origin checks, and a ClientWebSocket standing in for the
page: the login banner arrives, typed input round-trips, and `stty size`
reports the 100x30 the session asked for. The only untested link left is
xterm drawing bytes it was handed.

The WebView is de-risked on Windows, which was the plan's largest risk. Not
by assertion: with the app running there is an established TCP connection
from msedgewebview2 to the data plane port, so WebView2 launched, navigated
to the loopback page, executed terminal.js, and completed the WebSocket
handshake against the real token and origin checks. Linux remains unproven
and the package's own release notes now corroborate the concern -- Linux uses
a WPE backend, and it ships a NativeWebDialog described as useful where
embedded WebViews may be unavailable.

Two bugs found by building it, both of which would have shipped:

- ShellStream.Write buffers and needs an explicit Flush. Without one a
  keystroke is accepted, reported as written, and never reaches the remote:
  the terminal displays output perfectly and simply stops responding to
  input. SSH.NET's own WriteLine flushes, which is why the earlier spike
  never hit it. Found by isolating the pump against real SSH and reading
  BytesRead=51 -- banner and prompt through, nothing after.
- The Windows app manifest needs a supportedOS list, or Avalonia's native
  control host fails outright and the terminal never starts.

Also fixed a genuinely flaky test I happened to catch: SyncCursorTests
tampered with the *last* base64url character, whose low bits the decoder
ignores when the input length is not a multiple of three -- so a tampered
cursor sometimes decoded to identical bytes and verified. It failed roughly
one run in thirty, depending on a random key. Now tampers the penultimate
character, which is fully significant at every length; 40 consecutive runs
are clean.

xterm 6.0.0 plus the fit and webgl addons are vendored as UMD bundles rather
than built with npm, so a clean clone needs only the .NET SDK. Provenance
and licences are recorded next to them, along with the UMD global names
terminal.js depends on -- a bundle that switched to ES modules would load
without error and leave Terminal undefined.

The renderer acknowledges output from term.write's completion callback, not
on receipt. Acknowledging early would return flow-control credit for bytes
the screen has not caught up with, which is the one thing the credit window
exists to measure.

TerminalWorkspace moved into DodoSSH.Client.Terminal: it has no Avalonia
dependency, and having it there is what let the end-to-end test exist at all.

404 tests pass, zero warnings on a clean rebuild, format clean.
2026-07-28 22:30:42 +02:00