DodoSSH/docs/adding-hosts-on-the-phone.md

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 are stored and still not editable, and that is the one thing asked for that did not ship. Tag is a full item kind and HostSecret.TagIds merges, encodes and resolves — but no screen on either head draws a chip or offers to add one, so the tags a client can store are ones nothing here can see. Both editors carry the set through a save untouched, so a client that can set them does not lose them to somebody editing a port. What is missing is a picker and a chip row, on both heads: the plan's step 5 list does not mention them, and building the sheet, both editors and the back guard was already the larger half of it. Recorded as half-shipped in docs/design-import-gaps.md rather than quietly left out.

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 VaultScreen.axaml 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<Guid> 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), 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.
• 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.

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.