Public Access
Stop the docs claiming absences that have since been built
An audit of README.md, the seven docs and the fourteen ADRs against the code, looking for what is described as absent or planned. Most of it held. What did not is here, and it clusters: every stale claim but one is downstream of the settings file arriving without this document noticing. design-import-gaps said the client has no preferences store and writes exactly two files. It writes three — ClientSettings is in settings.json beside the cache — and two preferences are saved through it. From that one error followed four more: the terminal font size row said "fixed at the renderer's 13px" when it has been 8 to 32 from a screen and three chords for some time; the transfer-resume row and the per-host last directory row both blamed a store that now exists, when what they actually want is a table and a scalar file is the wrong shape for one; and the Preferences table asserted no preference could be saved at all. It also said TerminalServerOpcode has four values and none carries an option. It has eight, and one of them is FontSize — which is the interesting part rather than a counting error, because that opcode is the proof that the frame these rows say is missing can be built. The rows now say what each one would actually take, which for three of the four is a setting, an opcode and a control, and for the Backspace row 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. THE SAME TWO ERRORS WERE SHIPPED IN THE INTERFACE. The preferences screen carries a NOT BUILT YET list, so that what the screen does not do is as legible as what it does — and it said terminal size was hard-coded a hundred lines below a working size control, and said there is one release channel a month after the nightly shipped. A list of absences is only worth having if it is true, and a screen contradicting itself in the same scroll is worse than no list. Both lines are corrected rather than removed: the first now says which three of the four are genuinely hard-coded, and the second says what is actually missing, which is a way to change channel from inside the application rather than by installing the other build. docs/adding-hosts-on-the-phone.md is deleted. It was a work plan whose own header says "Status: built. All six steps." — nothing links to it, and the decisions it records are in the code it produced, including the one it is proudest of: HostSecret.AsksForPassword carries its own remark on why naming neither binding had to stop meaning "ask me". What was left was step ordering and per-test instructions for work that shipped. Git keeps it. crypto.md is deliberately untouched. It is normative and frozen, and its claims are about the DSH1 format rather than about this build — including the one that reads oddly next to the code, that a passphrase is one of four ways to open a vault. Under the spec it is. What is missing is a statement about what this build can open, and that belongs beside the spec rather than inside it; see docs/unlocking-without-the-passphrase.md. VERIFIED. Build clean, 112 layout tests, 354 app tests. The preferences screen is measured by the layout suite, so the longer copy is held inside the window at the minimum size rather than assumed to fit.
This commit is contained in:
@@ -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<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) *(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.
|
||||
+15
-11
@@ -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. |
|
||||
|
||||
Reference in New Issue
Block a user