Say when a vault has moved, so nobody waits out the minute

The delta pull was cheap enough to run on a timer and the client did, once a
minute. That is fine for a machine and wrong for two people: an edit a colleague
makes is up to a minute stale, which is long enough for both of them to make it
and produce a conflict neither needed to have. Shortening the interval is the
obvious answer and the wrong one — it costs a request per client per interval
whether or not anything happened, and it converges on a busier server that is
still late.

So the server now says so. A client holds a WebSocket open at GET /api/v1/events,
subprotocol dodossh.events.v1, and gets a line down it when something it can read
has changed. ADR 0012 has the reasoning; three parts of it are worth repeating
here, because they are what everything else rests on.

**What crosses the socket is a notice, never data.** A frame names a vault and
how far its change log has got. No item, no ciphertext, not even which item it
was. The client's answer is the delta pull it would have run anyway, so there is
still exactly one code path that applies a change to a keychain, and it is not
this one. Pushing the items themselves would save a round trip and fork that path
in two, with the cursor, the merge and the tombstone rules duplicated across both
— ADR 0003 put every mutation through one write path for that reason, and this
keeps every read on one for the same one. It also makes a dropped notice
harmless, which is what lets the fan-out below be as simple as it is.

**Polling stays, and is what guarantees a pass.** The minute timer is unchanged.
A network that eats WebSockets, a server with Events:Enabled off, an older
server, a proxy that will not upgrade, a notice dropped under backpressure —
every one of those leaves a client behaving exactly as it did before this commit.
Nothing is reachable only over the socket and nothing is meant to become so;
VaultViewModel's AutoSyncInterval remark now says that where somebody changing it
will read it.

**The bearer token authorises the upgrade, unlike the relay's ticket.** Not an
inconsistency with ADR 0004: the relay's socket is a byte pipe whose whole
authorization decision — which host, which IPs, which port — is made before it
opens and never revisited, and it is the extraction seam for a process that must
hold no ACL code. This one is a view of the caller's own vault list and has to
keep answering "what may this account read" for as long as it is held. A ticket
would carry that answer in a token and be wrong the moment the account's access
changed. The two bounds that arrangement needs are met rather than waved at: the
socket is closed at the token's exp with close code 4401 and the client comes
straight back with a fresh one, and the vault set is re-resolved every few
minutes as well as on the changes known to affect it. Both bound *metadata*,
because a notice contains nothing else and reading a vault still needs a key this
server has never held.

**The fan-out.** VaultEventHub is a singleton holding the sockets this node
accepted; publishing walks them and asks each whether it cares, rather than
keeping a vault-to-subscriber index that every re-subscription would have to move
entries between under a lock publishing also takes. At a few hundred sockets per
node and an event rate bounded by how often people edit keychains, the walk is
not measurable and its races are obvious. Per-connection queues are bounded and
drop the *oldest*: a notice means "pull vault X, which is at least at sequence
N", so the newest subsumes what it displaces and the client's answer is identical
either way — which is what lets the publish path be void, never block, and never
fail.

Announced from the endpoint rather than from SyncService, and that placement is
the point: by then the push has committed and released the per-vault advisory
lock. From inside it would name a sequence no reader can see yet and would hold
the lock that serialises writers across a socket write. Only the highest
*applied* sequence, so a batch of pure conflicts announces nothing, and a
duplicate — already announced when it first landed — announces nothing either.

Grants and membership publish too, and those take the *recipient* rather than the
actor. This is what AdmitNewVaultsAsync has been apologising for since sharing
shipped — "the recipient is handed nothing, there is no push channel" — and the
README with it. A vault shared with somebody now turns up as it is shared. The
comment and the README paragraph both say what is true now, and both keep saying
that the pass is what *discovers* the vault, because a client with no socket has
to arrive at the same place.

**On the client**, VaultEventStream is really a reconnection policy wrapped round
a ClientWebSocket: a dropped socket is the ordinary case here — laptops sleep,
proxies time out, tokens expire, servers are redeployed — so nothing in it treats
a failure as exceptional, and every path ends in "wait, then dial again". A
connection that lived long enough to say hello resets the backoff, so a laptop
that woke, worked, and lost its network an hour later does not inherit a
minute-long wait it has already proved it need not take. A 4401 close skips the
backoff entirely and asks the token provider again, which is the whole reason
that close code is distinct. A server that does not advertise the events feature
gets IdleVaultEventStream, which never delivers — so IVaultServer.Events is never
null and every caller stays on one shape, because the correct behaviour without a
socket is the behaviour with a silent one.

The shell's background loop now selects between the timer and a notice, and both
waits are held across iterations. That is load-bearing rather than tidy:
PeriodicTimer permits one outstanding WaitForNextTickAsync and throws on a
second, and an abandoned channel read stays registered and consumes the next
notice written. Either defect leaves the first notice working and every one after
it silently lost, which is why NoticesKeepWakingTheLoop_NotJustTheFirst pushes
three and not one. Notices are coalesced over a quarter of a second, so one
person's save — a host and its log entry are two items — and a colleague clearing
a folder each cost one pass rather than a dozen.

**The kind is a string, not an enum**, and that is a compatibility decision.
UseStringEnumConverter throws on a value it does not know, so a newer server
sending a kind an older client had never heard of would not add an unreadable
frame — it would break that client's socket outright. A string is ignored
instead. ProblemCodes is the same shape for the same reason.

**Tested on both sides, through the real pipeline.** The endpoint suite opens a
genuine socket against TestServer and proves a push produces a notice, that
another account's push does not reach it, that a ping is answered, and that a
frame this server cannot parse does not end the connection. Two of those assert
on *ordering* rather than on absence within a timeout — the stranger's write goes
first, so a socket that leaked would have announced it before the one the test
waits for — because "nothing arrived in two seconds" is a test that passes on a
slow machine for the wrong reason. And ANoticeCarriesNoCiphertext asserts on the
bytes that crossed the wire rather than on the record's fields, since the latter
would only prove that this type has no payload member, which is a tautology; the
former is what catches a field added later without anybody thinking about
disclosure.

The client suite drives VaultEventStream through an injected connector, because
the one thing a test cannot do to a real network is make it fail on cue — and
failure is the entire subject. The shell suite proves a notice produces a pull
inside ten seconds against a sixty-second timer, so the timer cannot be what
caused it.

**Two limits, stated rather than left to be discovered.** Fan-out is in-process,
so a deployment running more than one API replica only pushes for writes its own
replica handled and the rest arrive on the timer. IVaultEventPublisher is the
seam a PostgreSQL LISTEN/NOTIFY backplane implements and it is deliberately not
implemented: an untested backplane is worse than a documented gap, and multiple
replicas degrade to the behaviour before this commit rather than breaking. And a
client is notified of its own writes; it pushed, so it already pulled, and the
extra pass finds nothing. Suppressing that echo correctly needs a per-device
identity on the socket, and the same user's other machines must still be told.

Manual checks phase 15 covers what no test here can reach, which is the network
in between: a proxy that will not upgrade, one that drops an idle socket without
telling either end, a laptop lid, a token expiring. Every one of those is
invisible inside a test host, and every check there passes only if the change
arrives quickly *and* still arrives with the socket taken away.

ADR 0012 also fixes one thing about the shared terminal session this is the
transport for, so it need not be renegotiated later: session data will be binary
frames on this same socket, because base64 in a JSON envelope is the wrong shape
for the one payload here that is continuous rather than occasional. Two questions
it explicitly does not answer by implication — whether those bytes go through the
API at all, and what end-to-end encryption means when the second party watches a
stream rather than holding a key — are ADR 0001 questions and get their own
decision.

1512 tests pass. DodoSSH.SystemTests was not run — it needs the whole compose
stack — so the end-to-end path is unverified for this change beyond what the
manual checks describe.
This commit is contained in:
2026-08-04 16:37:41 +02:00
parent 176df67861
commit 4b706bc3c3
31 changed files with 3285 additions and 13 deletions
+167
View File
@@ -0,0 +1,167 @@
# ADR 0012 — A WebSocket that carries notices, not data
- Status: accepted
- Date: 2026-08-04
## Context
[ADR 0003](0003-sync-protocol.md) built a delta pull that is cheap enough to run on a timer, and
the client does: one pass a minute. That is the difference between a colleague's change appearing
"soon" and appearing *now*, and it shows up in three places that are not equally forgivable.
- **A vault shared with you** arrives on the next pass. `AdmitNewVaultsAsync` says so in its own
remarks — "the recipient is handed nothing — there is no push channel" — and the README repeats
it. Sharing works and looks broken.
- **Two people editing one keychain** see each other up to a minute late, which is long enough to
make the same edit twice and produce a conflict that nobody needed to have.
- **A revoked grant** keeps serving a client that has not noticed yet, for up to a pass.
Shortening the interval is the obvious answer and the wrong one: it costs a request per client per
interval whether or not anything happened, and it does not converge on *immediate* — it converges
on a busier server that is still late.
There is also a second thing coming that this decision has to not preclude. The intended feature is
a **shared terminal session** — one person's shell, watched or driven by another, TeamViewer-shaped.
That is bidirectional, continuous, and latency-sensitive in a way a keychain notice is not.
## Decision
### One WebSocket per signed-in client, at `GET /api/v1/events`
Subprotocol `dodossh.events.v1`. The client opens it after unlock and keeps it open; the server
sends a notice whenever something the client can read has changed.
**Not SSE.** Server-sent events would carry today's notices perfectly well and would be less code.
It is one-directional, so the shared-session feature would need a second mechanism next to it, and
then two transports would need reconnection, authorization and lifetime rules that agree. The cost
of a WebSocket over SSE is small; the cost of two transports is not.
**Not SignalR.** It brings hub protocol negotiation, its own serialisation and transport fallbacks,
none of which are wanted here: `DodoSSH.Contracts` and its source-generated serialiser are "the
actual contract between the two sides", and a second wire format alongside it is exactly the silent
drift `Setup/Json.cs` records having already cost this project once.
### The notice carries no ciphertext
A `vault.changed` frame is `{ kind, vaultId, sequence }` and nothing else. The client's answer to it
is the pull it would have done on the timer anyway.
This is the load-bearing decision, and it is worth being explicit about why the tempting alternative
is refused. Pushing the changed items themselves would save a round trip and would fork the code
path that applies a change into two — one that arrives by pull and one that arrives by socket — with
the cursor, the merge and the tombstone rules duplicated across both. ADR 0003 put every mutation
through one write path for exactly that reason; this keeps every *read* on one path for the same
one. The socket decides *when* to sync. It never decides *what* a vault contains.
It also means a dropped notice is harmless, which is what lets everything below be simple.
### Polling stays, and is the fallback rather than a legacy path
The one-minute pass is unchanged. The socket makes it *early*; it does not make it *necessary*. A
client on a network that eats WebSockets, an older client, a server that has the feature off, a
notice dropped under backpressure, a second API replica that did not see the write — every one of
those degrades to what the product does today, which is correct and up to a minute late.
Nothing may be reachable only by socket. That is a rule about future features, not an observation
about this one.
### Authorization: the bearer token on the upgrade, not a ticket
[ADR 0004](0004-relay-authorization.md) gives the relay a two-step ticket so its WebSocket carries
no API authority. This one goes the other way and takes the ordinary bearer JWT on the upgrade
request, which is an ordinary authenticated HTTP request. The difference is not inconsistency:
- The relay's socket is a **byte pipe to a third party**, and its whole authorization decision —
which host, which IPs, which port — is made *before* the socket opens and never revisited. It is
also the extraction seam for a standalone relay process that must not hold ACL code.
- This socket is a **view of the caller's own vault list**, and it has to keep answering "what may
this account read" for as long as it is open. It needs the full ACL context, in-process, for the
life of the connection. A ticket would carry that context in a token instead, and it would be
wrong the moment the account's access changed.
A long-lived connection authorised by a short-lived token is the problem this creates, and it is met
head-on rather than ignored:
1. **The socket does not outlive the token.** The `exp` claim is read at accept, and the connection
is closed with `4401` when it passes. The client reconnects with a fresh token; that is a
sub-second gap in a channel whose failure mode is already "poll instead".
2. **The vault set is re-resolved periodically** (`Events:AccessRefreshInterval`, default five
minutes) as well as on the changes that are known to affect it. A withdrawn grant therefore stops
producing notices within that window at the latest, and immediately in the ordinary case.
Both are bounds on **metadata** — the fact that a vault changed and roughly when — because that is
all a notice contains. Nobody's ciphertext is behind this socket, and a client that stayed subscribed
one interval too long could still not read a byte of it: reading requires a vault key grant, which
this server has never held.
### The frames
Text frames, JSON, `DodoSshJsonContext`. Server to client:
| kind | meaning |
| --- | --- |
| `hello` | accepted; carries the heartbeat interval and the vault count subscribed |
| `vault.changed` | `vaultId` moved to `sequence`; pull it |
| `vaults.changed` | the set of vaults this account can reach is different; re-read it |
| `ping` | heartbeat; the client answers `pong` |
Client to server: `ping`, answered with `pong`. Nothing else — subscription is decided by the server
from the caller's access, not asked for by the client, because a client that could ask to subscribe
to a vault id is a client that can probe for vault ids.
`kind` is a **string**, not an enum, and that is deliberate. `UseStringEnumConverter` throws on a
value it does not know, so a newer server sending a kind an older client has never heard of would
not add an unknown frame — it would break that client's socket entirely. A string is ignored
instead, which is what makes the table above extensible. `ProblemCodes` is the same shape for the
same reason.
### Where the shared session will attach
The socket is the seam, and one thing about it is chosen now so that it need not be renegotiated
later: **session data will be binary frames on this same connection, not JSON on the table above.**
Terminal output base64'd into a JSON envelope would cost a third of the bandwidth for nothing, on
the one payload here that is continuous rather than occasional. Control — offer, accept, resize,
end — is JSON like everything else.
That is as far as this ADR goes. Two questions are open and are not being answered by implication:
whether a shared session's bytes go through the API at all or peer-to-peer past it, and what
end-to-end encryption means when the second party is watching a stream rather than holding a key.
Both are ADR 0001 questions and deserve their own decision. What this one buys is that they will not
also be transport questions.
## Consequences
- **Fan-out is in-process, and the deployment is therefore single-node for this feature.** Every
connection is held by the node that accepted it; a write handled by another node produces no
notice on this one. `IVaultEventPublisher` is the seam a backplane implements — PostgreSQL
`LISTEN`/`NOTIFY` needs no infrastructure this stack does not already run — and it is deliberately
**not implemented**, because an untested backplane is worse than a documented gap. Multiple API
replicas do not break: they degrade to polling, which is the state before this ADR. `/api/v1/meta`
advertises `events` so a client knows which it is getting.
- **Per-connection queues are bounded and drop the oldest.** A notice is "pull vault X, which is at
least at sequence N", so the newest is strictly more useful than the one it displaces and the
client's answer is identical either way. A slow reader costs itself latency, never the publisher's
progress — the publish path never blocks and never awaits a socket.
- **The publish happens after the transaction commits**, outside the advisory lock ADR 0003 takes.
A notice sent from inside it would name a sequence a reader cannot yet see, and would hold the
per-vault write lock across a socket write.
- **A client is notified of its own writes.** It pushed, so it already pulled; the extra pass finds
nothing. The client coalesces notices over a short window rather than the server suppressing an
echo, because suppressing it correctly needs a per-*device* identity on the socket and the same
user's other machines must still be told.
- **Connections are capped** per user and per node (`Events:MaxConnectionsPerUser`,
`Events:MaxConnectionsTotal`). A socket is cheap but not free, and an unbounded count of them is a
denial of service that authenticates first.
- The feature can be turned off entirely (`Events:Enabled`). A deployment behind a proxy that will
not upgrade should say so rather than have every client discover it by failing.
### Rejected
- **Shorter polling.** Cheaper to build, converges on a busier server that is still late.
- **Long polling.** No new transport and genuinely immediate, but it holds a request thread and a
connection per client for the same money as a WebSocket while offering none of the bidirectionality
the shared session needs.
- **Pushing the changed items down the socket.** Saves a round trip; forks the apply path in two. See
above.
- **Client-chosen subscriptions.** A `subscribe(vaultId)` frame is an existence oracle for vault ids,
which is the disclosure `SyncPullEndpoint` answers 404 rather than 403 to avoid.