diff --git a/docs/adding-hosts-on-the-phone.md b/docs/adding-hosts-on-the-phone.md deleted file mode 100644 index 9397adf..0000000 --- a/docs/adding-hosts-on-the-phone.md +++ /dev/null @@ -1,349 +0,0 @@ -# Adding hosts on the phone - -The phone can read a keychain and connect through it. It cannot put anything in one. This document is the -plan for the change that fixes that, and it is written to be picked up cold — the decisions, the reasons, the -ordered work, and the traps that are already known. - -> **Status: built.** All six steps. Each one compiled with the whole suite green before the next began, -> which is the rule the ordering below sets. -> -> | Step | State | Notes | -> | --- | --- | --- | -> | 1. `HostGroupSecret` grows | **Done** | Five fields, a version rule the codec did not have, a byte pin, and the "groups are flat" prose rewritten in all four places it appeared. | -> | 2. The `Tag` item kind | **Done** | Secret, codec, merge, cipher, repository, both registries, EF entity and the generated `AddTagItem` migration. | -> | 3. `HostSecret` grows, `Port` goes nullable | **Done** | `TagSet`, `TagIds`, `Port` as `int?`, `AsksForPassword`, both schema versions, and `HostInheritance` — the resolver. | -> | 4. The shared view model | **Done** | Every choke point below, plus the group editor the plan never assigned a step to and without which nothing could set a default at all. | -> | 5. The phone | **Done** | The `+`, the sheet, both editor cards, the back guard and EDIT on the connect bar. | -> | 6. Tests and false prose | **Done** | Plus a fourth "groups are flat" site in `README.md` this plan did not list, and a cross-vault resolution bug the scouting for it turned up. | -> -> **Tags shipped after the six steps, in a seventh pass.** They were storable and unreachable when step 6 -> closed — a full item kind that no screen drew. Both heads now show a chip per tag on a host row and toggle -> them in the host editor, which also creates one inline, because wanting a tag and tagging a host are the -> same moment. Renaming and deleting live in a TAGS category on the keychain screen: a tag is an item so -> that renaming it is one write instead of twenty, and that rename needed somewhere to happen. -> -> That pass also made the desktop host editor scroll. A picker's height is a chip per tag in the keychain, -> so no fixed height holds it — which the layout suite caught the moment its seeder grew tags. -> -> **`HostFields.From` was answered by a refusal rather than by threading the resolver into the sync engine.** -> A relay host may not inherit its port; `HostSecret.TryValidate` refuses one that tries. A plaintext column -> derived from a *different* item goes stale when that item is edited and nothing re-pushes the hosts beneath -> it, so the relay would keep dialling the old port — which is worse than the restriction. -> -> **One decision was taken that this plan did not specify.** "Three states where there were two" is four, not -> three: a host can bind a key, bind a credential, be pinned to a typed password, or take its group's answer, -> and two nullable ids express three of those. Naming neither id now means *inherit*, so -> `HostSecret.AsksForPassword` was added to say "a typed password, even under a group that lends a key" out -> loud. Nothing stored changed meaning — no group could lend a binding before this build, so every existing -> host resolves exactly as it did. - -## What was asked for - -A floating `+` at the bottom right of the phone's HOSTS screen, opening a menu at the bottom offering **new -host** or **new group**. - -A **host** carries: which vault it is filed into (personal or a team's), an alias, a hostname or IP address, a -group, tags, an SSH port, and a username with either a password or an SSH key. - -A **group** carries: an alias, an optional parent group, and a default SSH port, username and -password-or-key — **defaults for the hosts inside it**. - -## The shape of the problem - -Three of those fields do not exist at any layer, and one of them is refused on the record. - -| Asked for | What exists today | -| --- | --- | -| The `+` and the editors behind it | Nothing. `Theme/Phone.axaml` has no `.fab` class, and its comment says an unused style would be "a claim that the control exists somewhere". `HostsScreen.axaml`'s own v2 note says hosts are created on the desktop and sync down. *(This row named a `ConnectionsScreen.axaml` as a second site. No such file exists or ever has; and the `HostsScreen` statement is an XAML comment, so no phone screen ever rendered that sentence to a user.)* | -| Vault picker | **Built.** `VaultViewModel.TargetVaults` / `SelectedTargetVault` / `HasVaultChoice`, hidden at one vault. The desktop's keychain screen already draws it. | -| Alias, hostname, port, username, key-or-password, group | **Built**, in the shared `VaultViewModel` host editor — `EditorLabel`, `EditorHostname`, `EditorPort`, `EditorUsername`, `EditorAuthenticationChoices`, `EditorGroupChoices`, `SaveHostCommand`. The phone has never bound any of it. | -| Tags | **Nothing.** `SyncEntityType.Tag = 5` and `HostTag = 6` are reserved slots with nothing behind them. `HostSecret` has no tag field. | -| A group's parent | **Refused on the record.** `HostGroupSecret`'s own remark says groups are flat because two clients can each re-parent A under B and B under A offline, a scalar merge accepts both, and the result is a cycle no reader can draw and the server cannot see, because it is inside the payload. | -| A group's defaults | **Nothing.** `HostGroupSecret` has exactly one field, `Label`. | - -So the phone half is mostly wiring. The domain half is three new capabilities, and one of them overturns a -written decision. - -## The three decisions - -**Tags are a real vault item, and a host names them.** A tag is a shared, reusable thing — the point of it is -to put the same tag on twenty machines and filter by it later — so it needs an identity of its own, not a -string repeated inside twenty payloads. `SyncEntityType.Tag = 5` becomes a live item kind. Membership is a -`TagIds` set on `HostSecret`, **not** the reserved `HostTag = 6` join. - -The reason to skip the join is that the merge does not need it. The argument for `HostSecret.GroupId` living on -the host is that filing two hosts into one group must be two writes to two items; the same holds here. What a -join would have bought beyond that is two machines tagging *the same* host without one of them losing — and -`ThreeWayMerge.Map` already gives that. It resolves a keyed collection key by key, honouring presence and -absence, which is set semantics with removals. `HostOptions` is merged through it today via `ToNameMap()`, and -a tag set keyed by tag id is the same shape. So `HostTag = 6` stays reserved and unused, and the second run -through the item-kind checklist is not spent. - -**Group defaults are inherited, not copied.** A host that leaves a field blank uses its group's value; a host -that fills one in overrides it. The editor shows the group's value as the field's placeholder, so the form -says what leaving it blank will get you. Copying the values into the host at create time was the alternative -and it was rejected: it makes a group a one-shot template rather than a live default, and changing a group -would then leave every host that had ever been created under it pinned to the old value. - -**Groups get a parent, and the resolver breaks cycles.** `HostGroupSecret` gains `ParentId`. The editor refuses -a parent that is already a descendant, which stops a cycle being made here; the resolver walks with a visited -set and stops at a repeat, which contains a cycle that arrives from somewhere else. A cycle therefore degrades -to a group that reads as a root — flat headings, defaults unresolved past that point — instead of hanging the -connect path, and clearing the parent in the editor is the repair. **This is the load-bearing part of the -decision:** with inheritance the chain is walked at connect time, so an unguarded cycle is not an undrawable -sidebar, it is a shell that never opens. - -`HostGroupSecret`'s "No parent. Groups are flat." remark and `HostGroupSecretTests`' class remark both become -false prose and must be rewritten to say what replaced the argument, not deleted. Same for the four rows in -`docs/design-import-gaps.md` recording the phone's missing `+`. - -## What the payload becomes - -``` -HostSecret HostGroupSecret - Label Label - Hostname ParentId ← new, optional - Port int? — null inherits DefaultPort ← new - Username null inherits, "" none DefaultUsername ← new - Notes DefaultSshKeyId ← new - JumpHostIds DefaultCredentialId ← new - Options - SshKeyId null inherits TagSecret - CredentialId null inherits Label - AsksForPassword ← new, true only - GroupId null = ungrouped - TagIds ← new - RelayEnabled -``` - -`Port` becoming `int?` is the change with the widest blast radius, and it is unavoidable: `int` with a default -of 22 has no way to say "I have no port of my own". `HostSecret.DefaultPort = 22` stays, as the last fallback -after the chain runs out. - -### Three states where there were two - -`SshKeyId` and `CredentialId` both null currently *means* "ask for a password each time" — a decision, not an -absence. `AuthenticationChoice`'s own remark argues at length that the two must never be conflated. -Inheritance adds a state, so the picker needs an explicit **"Inherit from group"** entry beside **"Password -(ask each time)"**, and `Bound(...)` needs to distinguish them. `Username` has the same problem: null means -"no username" today and is refused at connect; it has to come to mean "inherit", with "no username" still -reachable and still refused. - -**Built as four states, not three, because two nullable ids only carry three.** Key, credential, typed -password, inherit — and naming neither id was the third and is now the fourth. `HostSecret.AsksForPassword` -carries the difference: null is "not stated", which walks the chain and lands on a typed password if the -chain lends nothing, and `true` is "a typed password, even under a group that lends a key". Only `true` is -ever written, and a decoded `false` folds back to null, so a host that never touched the field encodes -exactly as it did before it existed. `Username` needed no field — an empty string is "no username" and null -is "inherit". `Port` needed none either: there is no "explicitly no port", only 22 at the end of the chain. - -Mutual exclusion moves with it. `HostSecret.TryValidate` enforces "a key or a credential, never both" per -record; a host naming a credential under a group naming a key is two individually valid records that resolve to -two bindings. **Exclusion has to be enforced at resolution**, host-over-group, and the resolver must never -return both. - -## The order to build it in - -Each step compiles and its tests pass before the next one starts. Steps 1–3 are the domain and are -prerequisites for everything; step 4 is the only user-visible one. - -### 1. `HostGroupSecret` grows - -`src/DodoSSH.Client.Domain/HostGroupSecret.cs`, `HostGroupSecretCodec.cs`, `HostGroupSecretMerge.cs`. - -- Add `ParentId`, `DefaultPort`, `DefaultUsername`, `DefaultSshKeyId`, `DefaultCredentialId`. -- `TryValidate`: reject `Guid.Empty` for each id, reject a `ParentId` equal to the group's own id where that is - knowable, apply the same 1–65535 range rule to `DefaultPort`, and refuse a group naming both a default key - and a default credential. -- **The codec needs the host codec's rule, which it does not have today.** `HostGroupSecretCodec` stamps - `CurrentSchemaVersion` unconditionally. Copy `HostSecretCodec.SchemaVersionFor` — a *maximum over the fields - actually present*, not a ladder — or upgrading one machine makes every group read-only on every other. A - group carrying none of the new fields must still encode at version 1, byte for byte. -- `HostGroupSecretMerge` gains the five fields, each a scalar. -- Add a group byte pin mirroring `AddingTheKeyField_DidNotChangeTheBytesOfAHostWithoutOne`. The group codec has - none, and the same "old vaults must re-encode identically" argument applies to it. - -### 2. The `Tag` item kind - -`SyncEntityType.Tag = 5`, `CryptoSpec.AadResourceType.Tag = 8`, `ChangeEntityType.Tag = 5` and the -`PublicAPI.Unshipped.txt` line **already exist** — no contract change, no crypto-spec change, no `docs/crypto.md` -change. `LocalCacheProtector`'s AAD switch already maps `Tag`. - -New: `TagSecret.cs`, `TagSecretCodec.cs`, `TagSecretMerge.cs` in `DodoSSH.Client.Domain`; `TagCipher.cs`, -`TagRepository.cs` in `DodoSSH.Client.Sync`. Edited: `Client.Sync/ItemKinds.cs` (registry entry + `TagKind`), -`VaultSession.cs` (a `Tags` repository beside `HostGroups`), `Domain/Hosts.cs` (`VaultTag`), -`Infrastructure/DodoDbContext.cs`, `Infrastructure/Configurations/HostAndSyncConfigurations.cs`, -`Api/Features/Sync/ItemKinds.cs` (registry entry + server `TagKind`). - -**A server migration is required** — a tag is its own table. Generate it, do not hand-write it: -`dotnet ef migrations add AddTagItem --project src/DodoSSH.Infrastructure`, committing all three resulting file -changes. **No client migration**: `ClientCacheContext` has one `item` table keyed by `(VaultId, EntityType, -EntityId)`, so a new kind is new rows. - -Four traps, all of which pass a naive test suite: - -- **`TagCipher` must say `AadResourceType.Tag` as a constant, never cast from `SyncEntityType.Tag`.** Tag is 5 - on the wire and 8 in the crypto enum. A cast seals tags under `Credential`, which encrypts and decrypts - perfectly on the machine that wrote it and is a spec violation nothing notices until another implementation - refuses the item — and the AAD is frozen into stored ciphertext. `AadResourceTypeTests` is the test that - catches this, and it only catches it once `PinnedPairs` and `SealSample` have a Tag arm. -- **The `ItemKinds.Registry` entry is what `SyncEngine` pulls with.** `SyncedTypes` is projected from it. Miss - it and tags encrypt, merge and list perfectly on the machine that made them and are never once requested from - the server. -- **`TagKind.Fields` returns `null`, not an empty `SyncPlaintextFields`.** An empty record still serialises - `relayEnabled: false`, which invites a reader to believe the type has a relay setting that is off. -- **`ValidateFields` must refuse everything, with reasons.** There is no default; an unimplemented one accepts - whatever arrives. `RelatedId` matters more here than it did for a group — it is the field a future `HostTag` - would reach for, and `SyncPlaintextFields` is frozen so it cannot be removed. Follow `SnippetKind` and - `ObjectStoreKind`, which are more complete than `HostGroupKind`. - -### 3. `HostSecret` grows, and `Port` goes nullable - -`HostSecret.cs`, `HostSecretCodec.cs`, `HostSecretMerge.cs`, plus a new `TagSet` value type and a new resolver. - -- `TagSet` is a structural-equality wrapper in the shape of `JumpChain` — a plain `IReadOnlyList` on a - record gets reference equality from the compiler-generated `Equals`, which would make every host read as - changed on every sync pass and two identical edits register as a conflict. Unlike `JumpChain` it is a *set*: - order carries no meaning, so it sorts and dedupes, and it exposes a `ToIdMap()` for the merge. -- `HostSecretMerge` merges `TagIds` through `ThreeWayMerge.Map`, per tag, so two people each adding a different - tag to one host both keep theirs. It is not a scalar; a whole-value merge here would silently drop one side. -- `Port` becomes `int?`. `TryValidate`'s `is < 1 or > 65535` is already false for null, which is the wanted - behaviour but is silent — say so in a comment, and fix the message, which interpolates `{Port}` and renders - empty for null. -- **`SchemaVersionFor` needs branches for both new fields, and this is the one that loses data if forgotten.** - A host with `Port = null` written at version 1 omits the property (`WhenWritingNull`); an older client - deserialises `int Port` as 0, `TryValidate` refuses it, and the item reads as **unreadable rather than - read-only** — gone, not locked. Add `PortInheritSchemaVersion = 5` and `TagIdsSchemaVersion = 5` (or 5 and 6) - and make both bump the maximum. -- **`AddingTheKeyField_DidNotChangeTheBytesOfAHostWithoutOne` pins the exact bytes** of a minimal host, - including `"port":22`. A host that now inherits its port drops that key. Keep the pinned literal for a host - with an explicit port, and add a companion for the inheriting one — do not edit the old literal to match - whatever the code produces. -- `TagIds` must serialise **last** in `HostPayloadDocument`, and empty must serialise as absent, or every host - in every vault looks changed on the first sync after the upgrade. - -New file: the resolver. Given a host and the group list, walk `GroupId` then `ParentId` with a visited set, -returning resolved port, username and binding, plus which group each came from — the editor needs the second -half for its placeholders. It must never return both a key and a credential. - -### 4. The shared view model, and the two heads - -`VaultViewModel` is where nearly all of it lands, and both heads get it at once because both bind the same -properties. - -The choke points, from the connect-path trace: - -- **`TryBuildAuthentication(HostSecret, …)` and `TryBuildConnectionRequest(HostSecret, …)`** both take a bare - `HostSecret` — this signature is where group context is currently lost. Both must take the resolved value. - This is the single highest-leverage change: it is the only auth resolution in the product, and both heads - and both transports go through it. -- The credential-username fallback becomes three levels: credential → host → group chain. -- `Complete(...)` must refuse on an empty username only *after* the chain has been consulted. -- `HostRowViewModel.Address`, `Dialled`, `TransfersViewModel`'s connected-to label and - `TerminalWorkspace.Describe` must show the **dialled** address. `MainWindowViewModel.Rank` searches - `Address`, so an unresolved one makes hosts unfindable by their real port. -- **`HostFields.From` in `HostCipher` must emit the resolved port.** This is the one place where an unresolved - read is a wrong wire rather than a wrong label: it fills the relay's plaintext columns, which is what the - *server* dials. A relay host with a null port also fails server validation outright. -- Host-key pinning keys on host and port. `ForgetHostKeyAsync` and the "pin in use" endpoint set read - `row.Host.Port` directly — a host inheriting 2222 pins under 2222 while both look under 22. -- `HostRowViewModel.Authentication`, `SelectedHostAsksForAPassword` (in **both** `VaultViewModel` and - `TransfersViewModel`) and `SelectedHostAuthenticationNote` all switch on the host's own two ids and must - switch on the resolved binding, with new wording for a binding that came from a group. -- **`HostsBoundTo`** counts hosts naming a key explicitly. A key deleted while only a *group* names it warns - nobody and then refuses every host under that group at connect time. It must walk groups too. -- The editor: load must distinguish "unset" from "explicitly this", `BuildHost` must preserve that in both - directions, and each field needs the resolved value as its placeholder. -- `ImportViewModel.IsAlreadyPresent` compares port and username directly, so a stored host inheriting 22 will - never match an imported host at 22 and the import screen re-offers duplicates. Decide whether an imported - block with no `Port` should inherit rather than pin 22 — `SshConfigResolver` returns `DefaultPort` today. - -### 5. The phone - -`Views/HostsScreen.axaml` and `Theme/Phone.axaml`. No csproj edit — Avalonia globs `**/*.axaml`. Compiled -bindings are on, so a binding to a property that does not exist is a build error; `TreatWarningsAsErrors` is on -repo-wide, so any XAML warning fails CI. - -- A `Button.fab` style, and the comment that currently refuses one rewritten to say what it now opens. -- The `+` needs a `Panel` wrapping the screen's root `Grid` so it can sit bottom-right without moving. Placing - it inside the list row would put it above the connect bar, which appears and disappears with the selection. -- The bottom sheet follows `HostKeySheet.axaml`, the head's one existing sheet: a scrim - (`Border Background="#9E0E1220"`, a literal because the palette holds no alpha surface), a bottom-aligned - `Border` with `CornerRadius="22,22,0,0"`, a decorative grab handle, and **no tap-to-dismiss** unless it is - wired by hand, because nothing on this head implements it. -- **Scrim reach is a real choice.** Declared inside `HostsScreen`, a scrim darkens the screen area only and the - 64px nav bar stays lit and tappable. To cover the display the sheet has to sit in `PhoneShell.axaml` beside - `HostKeySheet`, which is exactly why that control lives there. -- **The back gesture needs a new guard**, above the `switch` in `PhoneShell.axaml.cs`, in the shape of the - host-key guard: back should close the sheet or the editor rather than leave the screen. The comment on that - method states an invariant about the switch cases staying in step with the surface tests — so add a guard, - do not extend the switch. *(This named `IsVaultsSurface` and `IsConnectionsSurface`. Neither exists - anywhere in the repo; the real properties are `IsMoreSurface` and `IsTerminalSurface`, and only the first - is what the switch mirrors.)* -- The editors follow `SnippetsScreen.axaml`: a `Border Classes="card"` in the same grid row as the list, - toggled on `IsEditing`, with the list toggled on `!IsEditing`. There is no dialog, no modal and no - editor-screen anywhere on this head, and the reason is written down — a form stacked over the list hides what - it is about. -- Editing an existing host comes nearly free: the shared `IsEditing` panel serves both, so the connect bar gets - an EDIT button beside CONNECT. Not asked for, but a `+` that adds hosts with no way to correct one is a - strange thing to ship. - -### 6. Tests - -House style: `Subject_Predicate`, articles included, the predicate smuggling in the reason -(`_RefusesRatherThanFallingBackToThePassword`). No `Async` suffix on test methods; always on private helpers. -Every class carries a remark naming the class of failure it exists to catch. xunit.v3 + Shouldly, with the -because-string used as prose. - -- Three shared builders need the new fields: `Domain.Tests/HostFactory.cs`, `Sync.Tests/SyncHarness.cs`, - `Sync.Tests/HostCipherTests.cs`. There is no shared group factory and with five new fields there should be. -- Three guard tests fail by design and that is what they are for: - `HostSecretMergeTests.EveryScalarField_IsRoutedThroughAMerge` (note `GroupId` is missing from its `with` - block today — an existing gap worth closing while there) *(closed: `GroupId` is in that block now)*, - `ValueSemanticsTests.TryValidate_RejectsWhatCannotBeStored`, and the two "groups are flat" remarks. -- `ItemKindsTests.ThePullFilterNamesEveryTypeThisBuildSynchronises` is an ordered list — add Tag in registry - order. `AadResourceTypeTests` needs `PinnedPairs`, a `SealSample` arm and a `NewTag()` builder. -- `SyncEndpointTests.AHostGroupCarryingAParent_IsRejected` asserts the server refuses a plaintext `ParentId` - with a reason containing "flat". Putting the parent **inside the payload** does not break it mechanically, - but its stated reason becomes wrong. It should survive with a rewritten comment — the parent lives in the - payload, the plaintext column stays refused, and ADR 0004 is why. Decide that deliberately rather than - letting it drift. *(Decided that way, and the name says it now: - `SyncEndpointTests.AHostGroupCarryingAPlaintextParent_IsRejected`. The column is still refused and the - reason is the trust model rather than flatness.)* -- `Push_AnUnsupportedEntityType_IsInvalidNotAFailedBatch` picks the first type with no kind registered, very - likely Tag today. It self-heals to `HostTag` via `Assert.SkipWhen`. -- `App.Layout.Tests` measures pixels and bends rather than breaks. Extend the seeders — nesting and a tag row - change what the widest realistic content is — rather than the assertions. `ScreenLayoutTests.cs:1310` pins - `ChangedFields = "Hostname, Port, Username, Options, Group"` as a literal; add `Tags` to keep it measuring - the worst case. -- `ShellFlowTests.ReadyToConnectAsync` gives its host both a username and a port, so the inheritance tests need - a host with neither, or the group is never the source. - -## Found on the way, and out of scope - -`LocalCacheProtector`'s AAD switch is missing `ConnectionLogEntry`, `ActivityLogEntry` and `ObjectStore`, so -`ConflictStore.Record` throws `ArgumentOutOfRangeException` on a conflict for any of those three. Pre-existing, -unrelated to any of this, and `Tag` is already in that switch. Worth a separate fix. - -> **Fixed 2026-08-04**, having outlived the phases that shipped the logs and the buckets — which is exactly -> the drift a note like this is meant to prevent, so it is worth saying what let it last. Of the three callers -> of `AadResourceTypes.For`, two reach it only when an item carries plaintext fields and none of these three -> does; the third, `ConflictStore.RecordAsync`, calls it unconditionally but is reached only by a real merge -> conflict, which every existing test raised against a `Host`. The arms are in, and two tests now hold them -> there: `CacheStoreTests.AConflict_CanBeRecordedForEveryKindOfItem` records one per kind, and -> `AadResourceTypeTests.EverySyncableType_HasAnArmInTheStorageMapping` fails on the *next* item type added -> without one, by name rather than by a list kept by hand. - -## Prose that becomes false - -Not a tidy-up — these are the places the codebase currently tells a user or a maintainer that this feature is -deliberately absent, and each states a reason that will no longer hold. - -- `Theme/Phone.axaml` — the "No floating action button" block. -- `Views/HostsScreen.axaml` — the v2 and v3 notes on the missing `+`. -- ~~`Views/ConnectionsScreen.axaml`~~ — **this file does not exist and never did.** The real inventory is - the two above plus `README.md`, `docs/android-port.md` and `docs/design-import-gaps.md`. Neither phone - site was "rendered on screen": both are XAML comments. -- `HostGroupSecret.cs` and `HostGroupSecretTests.cs` — "No parent. Groups are flat." -- `docs/design-import-gaps.md` — the **Add host** row, the two floating-button rows, the "Tag / HostTag still - reserved and unused" paragraph, and the tag-chips and groups-on-a-team's-hosts rows in the Hosts table. diff --git a/docs/design-import-gaps.md b/docs/design-import-gaps.md index 6e3ab79..6ed49e6 100644 --- a/docs/design-import-gaps.md +++ b/docs/design-import-gaps.md @@ -160,7 +160,7 @@ the chrome, hosts and terminals, file transfer, the vault, teams, and preference > | **Share this host** | Omitted, as the vault screen's `SHARED WITH · 6` is. A grant is per *vault* and per-item sharing is M5; a button here would imply this one host could be handed over, which is the thing the architecture does not do. | > | **Add Telnet**, and **Serial** in the toolbar | Omitted. `ISshConnection` is the only transport there is. This is also why the card subtitle's `ssh` is a constant today rather than a reading — it is stated in `HostRowViewModel.Summary`, which is the one place in this interface where a constant is printed on purpose. | > | **+ SSH ID, Certificate, FIDO2** | Omitted. `IDENTITIES` and `CERTIFICATES` have been on this document's list since the first import — neither is even a reserved `SyncEntityType` — and there is no security-key path anywhere in the SSH layer. One control offering three item types that do not exist. | -> | The **Backspace / Default** row | Omitted. It is a terminal setting, and the client has no preferences store and no frame to carry one to the renderer — see the Preferences section. It would be a control whose value could not survive the window closing. | +> | The **Backspace / Default** row | Omitted, and it is the one row on this list that could now be built cheaply — a setting, an opcode and a control, the way the text size was. What it lacks is a reason: which byte backspace sends is a fact about the remote's `stty`, so a client-side switch fixes a mismatch by hiding it. See the Preferences section. | > | The **chevron beside the vault name** | The name alone, and the move behind the pane's ⋯ menu instead. A host *can* now be moved between vaults, so the gap is no longer that there is nothing to offer — it is that a chevron on a subtitle implies an edit, and this is not one: the two vaults are encrypted under different keys, so it is a re-seal into one and a tombstone in the other, the host takes a new id, and its group and tags stay behind. A control that implied "just change this field" would be describing something else. Where a *new* host goes is still asked in the host editor, as a picker beside the name. A group moves too, from its card's right-click menu, and takes its nested groups and every host filed under them; keys, passwords and buckets take theirs from the keychain screen's standing picker and cannot be moved yet. | > | **Show more ⌄** | Not drawn as a disclosure. What it would hide — notes, the relay switch, forgetting the host key — is in the editor, one press away, and a second fold inside a pane that already scrolls is a second place for a field to be missing from. | > | **Port Forwarding** in the sidebar | Nothing, for the third time in this document. | @@ -173,8 +173,9 @@ it for revocation, for relay session recording and for what locking does not do rendered with invented data to make a screenshot look finished. Where a feature does not exist, the interface either omits it or says so. -Read alongside [the milestone plan](../README.md#milestones). Most of what follows is not a defect; it is -M2 and M3 arriving in a design before it arrives in the code. +Read alongside [the milestone plan](../README.md#milestones). Most of what follows is not a defect. M2 and +M3 have since arrived and their rows say so; what is left is either M5 — per-item content keys, and the +sharing that would rest on them — or a decision recorded here not to build the thing at all. ## The shape of the problem @@ -204,9 +205,12 @@ reason and is gone: membership is granted only to an account that exists, so the reservation fails to cover. The status stays reserved, because the column holds it in nobody's database and a client must not fail on a value a later server might send. See [ADR 0009](adr/0009-team-access-model.md). -**The client has no preferences store.** It writes exactly two files — `cache.db` and `device.key` — and the -cache has six tables, none of them settings. Nothing on the design's TERMINAL preferences panel can be -saved, and there is no frame on the terminal data plane that would carry a change to the renderer anyway. +**The client's settings file holds two things.** It writes three files — `cache.db`, `device.key` and +`settings.json` — and the cache's six tables still hold none of them. `ClientSettings` carries the +terminal's text size and whether this machine looks for a newer build on its own, and both survive a +restart. Everything else on the design's TERMINAL panel is unbuilt rather than unbuildable: the data plane +grew `TerminalServerOpcode.FontSize` to carry the one that shipped, so a second option is an opcode, a +setting and a control rather than a new mechanism. **Three things the design did not ask for and this build now has.** A key can be generated in the client rather than pasted in (`SshKeyGenerator`, and the `openssh-key-v1` container is written by hand — see @@ -326,7 +330,7 @@ caption buttons and window title drawn on top of the application's own — two s | Pane header showing the running command and `following` | client-ssh | The host moves opaque bytes and never parses terminal output. Would need shell integration (OSC 133) on the remote. | Omitted. | | A `local · zsh` tab | client-ssh | Every session here is an SSH channel. Needs ConPTY and a second session kind. | Omitted. | | Tab strip `+` button | ui | Not missing so much as redundant: the real operation is *select a host, press Connect*, which the hosts grid already is. | **Shipped**, as the palette rather than a menu: it opens what Ctrl+K opens, so the strip and the shortcut are one way of doing one thing. A `MenuFlyout` offering "SSH" and "local shell" is the nicer answer and is not verifiably safe above the terminal's native child window — and there is no local shell to offer. | -| Terminal font size (`--termfs`, 11–16px) | client-storage | See preferences. | Fixed at the renderer's 13px. | +| Terminal font size (`--termfs`, 11–16px) | client-storage | — | **Shipped**, and wider than the design's range: 8–32px from the preferences screen or Ctrl+plus/minus/0 over a terminal, carried by `TerminalServerOpcode.FontSize` and kept in `settings.json`. It resizes the grid rather than magnifying it, so every live pane refits and the remotes are told how many columns they now have. | --- @@ -350,11 +354,11 @@ connect, and it goes through the same host key gate, the same pin and the same t | Remote listing with `NAME/SIZE/MODIFIED/PERMS` | client-ssh | All four. `PosixMode` renders `drwxr-xr-x` from the bits SFTP hands over; setuid, setgid and sticky are not shown, because SSH.NET does not surface them and `rwx` where `rws` is true would be worse than nothing. | | Local listing | client-transfer | `LocalDirectory`, which is where this client's `System.IO` now lives. `PERMS` is blank on the local side rather than filled with a plausible-looking POSIX mode that is not a fact about a file on Windows. | | Transfer queue, progress, throughput | client-transfer | `FileTransferQueue`. One transfer at a time, so the rate on a row is the rate of the link rather than a share of it. Throughput is measured over a half-second window, not averaged since the start. | -| `resume supported` | client-transfer | **Within a run of the application.** Every transfer writes to a `.dodossh-part` file beside its destination and is renamed into place at the end, so an interrupted one can never be mistaken for a finished one, and `RESUME` carries on from the part file's own length. A part file found at startup is *not* resumed: nothing records what wrote it, and resuming on the strength of a name matching is how a corrupt artefact gets delivered with nothing reporting a failure. Making it survive a restart needs the preferences store this client has not got — see below. | +| `resume supported` | client-transfer | **Within a run of the application.** Every transfer writes to a `.dodossh-part` file beside its destination and is renamed into place at the end, so an interrupted one can never be mistaken for a finished one, and `RESUME` carries on from the part file's own length. A part file found at startup is *not* resumed: nothing records what wrote it, and resuming on the strength of a name matching is how a corrupt artefact gets delivered with nothing reporting a failure. Making it survive a restart needs somewhere to record *what* wrote each part file — the source, the offset and the run — which is a table rather than a setting; `settings.json` holds two scalars and is not that. | | Design element | Layer | What it would take | What ships instead | | --- | --- | --- | --- | -| Per-host last directory | client-storage | Somewhere to persist two panes' navigation state. There is still no settings table. | The remote pane opens on the account's home directory, which the server canonicalises during the handshake; the local pane opens on the user profile. | +| Per-host last directory | client-storage | Somewhere to persist two panes' navigation state per host, which is a table rather than a scalar — `settings.json` holds two of those and is the wrong shape for a row per machine. | The remote pane opens on the account's home directory, which the server canonicalises during the handshake; the local pane opens on the user profile. | | `sftp over bastion-eu` | client-ssh | Jump hosts, as above. `HostSecret.JumpHostIds` is still stored, synced, merged and read by nothing. | Omitted. | | Overwriting a file that is already there | ui | A prompt, which means a modal this window has no idiom for. | Refused, with the name that is in the way. The remote pane has DELETE and MKDIR so the refusal is not a dead end. | | Dragging between the panes | ui | Drag-and-drop between two `ListBox`es, plus a drop target that is a directory rather than a row. | Two arrow buttons between the panes, pointing at the pane the file is going to. | @@ -476,8 +480,8 @@ lists the rest as absent rather than omitting it silently. | Design element | Layer | What it would take | | --- | --- | --- | -| Any preference, saved | client-storage | A seventh table in the client cache, or a preference item type in the vault. Which one matters: the vault syncs, so a preference stored there follows you between machines, and a terminal font size probably should not. | -| Terminal font, size, cursor style, cursor blink, scrollback | client-terminal | The store, **and** a frame to carry it. `TerminalServerOpcode` has four values (`Output`, `SessionOpened`, `SessionClosed`, `SessionActivated`) and none carries an option. | +| Any preference, saved | — | **Two are.** `settings.json` beside the cache holds the terminal's text size and whether this machine checks for updates on its own. It is deliberately not in the vault: the vault syncs, so a preference kept there would follow you between machines, and a font size chosen for a laptop screen should not arrive on a desktop. Anything per-host or per-item wants a table instead — see the transfers rows above. | +| Terminal font, cursor style, cursor blink, scrollback | client-terminal | A setting, an opcode and a control, in that order — the mechanism is no longer the obstacle. `TerminalServerOpcode.FontSize` proved the path and `ClientSettings` is where the value would live; what none of these four has is somebody deciding it is worth a row on the preferences screen. | | Font family picker offering IBM Plex Mono | ui | Shipping the font, as above. | | Audible bell | client-terminal + ui | xterm.js 6 removed `bellStyle` and `bellSound`; what remains is an `onBell` event, so the page would have to make the sound itself. | | `GENERAL` section | ui | There is no general setting to put in it. The theme is fixed by decision, and window size is not persisted. | diff --git a/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml b/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml index 406dd92..3c8a36a 100644 --- a/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml +++ b/src/DodoSSH.Client.App/Views/PreferencesScreen.axaml @@ -256,9 +256,9 @@ + Text="Terminal font, cursor and scrollback — the renderer hard-codes those three. The text size above is the one that is not, and it is the path the others would follow if they were worth a row here." /> + Text="Changing channel from in here — there is a nightly as well as a release, but which one a copy follows is fixed when it is built, so moving between them means installing the other one." />