Ship the presence tick as the loops it moved, not the whole graph (#288)

[scgopi]

Stacked on #296 (encode once), both now based on main post-#293.

The defect

The presence poll runs every fifteen seconds and, when anything moved, broadcast the whole graph — 50 KB after #293 — to every client. On a busy graph something moves almost every tick, so the tick was the daemon's steady-state cost: a full snapshot per project per client, four times a minute, to say which pill changed colour.

The change

The tick ships the loops it moved and nothing else.

• DaemonEvent.nodesChanged(projectPath:revision:nodes:) carries the top-level LoopNodes whose reading, activity, summary or board changed on this tick, as whole values — the diff is exact because everything the poll edits is a field on a top-level node. A tick that moved nothing sends nothing, as before; a tick that moved one loop is ~1.5 KB instead of the snapshot.
• A revision on every frame. GraphStore stamps snapshots and deltas from one counter (LoopGraph.revision, wire-only like mailroomDigest). That is what makes deltas safe beside Never block the daemon actor on a client write (#288) #291's superseding: an undelivered snapshot can be replaced by a newer one while a delta queued behind it still arrives — the client sees the delta is older than the graph it holds and drops it. Deltas themselves are never superseded (a newer one does not carry what an older one said).
• The app folds a delta into the snapshot it holds (LoopGraph.applying(nodesChanged:revision:)) and then handles the result as a .graphChanged, so every reader of a snapshot — the activity log, the open workspace, the project reducer — keeps one shape. A delta never invents a loop. The CLI and the remote shim ignore the event, as they ignore any frame that is not the acknowledgement they wait for; that is more correct than before, when a presence tick could be mistaken for one.
• Commands still broadcast whole snapshots, stamped later in the same sequence.

Version skew (from ReviewBroadcastDelta)

nodesChanged would have been the first event a daemon broadcasts unasked, and the app's reader took an undecodable frame for a dead socket — a 0.1.63 app against this daemon would have torn down and rejoined every fifteen seconds for ever. Two fixes, both here:

• DaemonCommand.announce(capabilities:). The first frame the app puts on every socket, dial and redial alike, says what it can read (ClientCapability.nodesChanged). A store sends the delta only to a connection that announced it and the whole snapshot — same tick, same revision — to every other, so an older app keeps getting exactly what it always got. The registry forwards a late announcement to stores the connection already joined, so which of the launch's frames lands first does not decide anything. Unknown capability names are ignored. This is the general answer for every future event, not only this one (Helper install order puts a new daemon in front of an old CLI, so a protocol change can lose data silently #298).
• The app's reader skips a frame it cannot decode instead of redialling — the mirror of how graphcoded treats a command it does not know (main.swift's "unrecognized command" reply).

Tests: a tick reaches an announcing connection as a delta and a silent one as a snapshot with the same revision and no posts; the client announces first on a fresh socket and again on a redial; an unknown frame is skipped and the next readable event arrives over the same connection (one accept, no redial).

Verification

Gate: full Xcode suite (1619 tests in 169 suites, 0 failures), swiftlint 0 errors, swift-format clean, graphcoded and graphcode-cli schemes build; SwiftPM swift build and .build/debug/graphcode pass locally.

PresenceDeltaTests: over a socket, the first tick ships both loops (first readings), the second ships only the one that moved with a higher revision, the third ships nothing, and a rename after that is a whole snapshot stamped later still; applying merges by id and never invents a loop; the app folds a delta into its snapshot and an open workspace's node, and drops one older than what it holds.

Refs #288

🤖 Generated with Claude Code

https://claude.ai/code/session_01DeGL2CxuGmq16RSZpJYm2N

[scgopi]

Independent review of #297 at 02935b3e — do not merge yet. One blocker, reproduced with a failing test.

Reviewed in my own worktree (review/296-297-probes). The delta design itself is sound and I tried hard to break it; the revision stamp does the job it claims. The blocker is not in the delta logic — it's in what nodesChanged is.

---

🔴 BLOCKER: an app one release behind reconnect-loops every fifteen seconds, forever

nodesChanged is the first event graphcoded ever broadcasts unasked. Every other DaemonEvent a client can meet is either one it already knew about or the answer to a command it chose to send. .mailbox from #293 looks like a counter-example and isn't — an old client never sends the command, so it never receives the reply. This is different in kind: the daemon pushes it, to every connection, four times a minute, whether or not the client has ever heard of it.

And the app cannot skip what it cannot read. OrchestratorClient.events() (graphcode/Sources/Clients/OrchestratorClient.swift:88-95) decodes inside the read loop with try, so a DecodingError lands in the same catch as a dead socket:

let data = try await readFrameAsync(from: fileDescriptor)
let event = try JSONDecoder().decode(DaemonEvent.self, from: data)   // throws on an unknown case
continuation.yield(event)
} catch {
  if let connectedDescriptor { await invalidate(connectedDescriptor) }
  try? await Task.sleep(for: .seconds(1))
}

So a shipped 0.1.63 app talking to a daemon carrying #297 does this on every presence tick: decode throws → invalidate the socket → sleep 1s → redial → rejoinProjects() → re-broadcast every snapshot → 14 seconds later, again. Forever, with no user action, until the app happens to be upgraded.

It is reachable exactly the way the #293 blocker was: DaemonBootstrap.helpers installs graphcoded before graphcode, so a partial install leaves a new daemon under an old app.

The daemon already refuses to do this in the mirror direction, and says why (graphcoded/Sources/main.swift:188):

// A frame that read fine but didn't decode is version skew, not a dead socket: a newer CLI sent a command this daemon predates. Dropping the connection here failed *silently* — the client just saw a hang-up — so answer instead and keep serving the commands this daemon does understand.

That reasoning is correct and it applies unchanged to the client half. The client half never got it.

Reproduced — two failing tests on review/296-297-probes (0cd9e40, one commit atop 02935b3e)

graphcode/Tests/BroadcastDeltaReviewTests.swift:

1. anOlderClientCannotDecodeAPresenceDelta — passes, establishing the premise: a real nodesChanged frame decoded against DaemonEvent as 0.1.63 knows it (four cases, no nodesChanged) throws. It is undecodable, not merely ignorable.
2. aFrameTheClientCannotDecodeDoesNotTearDownTheConnection — fails on 02935b3e:

✘ Expectation failed: (daemon.acceptedConnectionCount → 2) == 1
✘ Test aFrameTheClientCannotDecodeDoesNotTearDownTheConnection() failed after 3.118 seconds

One undecodable frame on a healthy socket, and the app opened a second connection. (It reuses the StubDaemon shape from OrchestratorClientTests with a writeRawFrame hook, since that stub can only write frames the app can decode.)

On DaemonCommand.announce(capabilities) — it narrows this, it does not close it

I'm told this is the planned fix, and @PerfTriage asked for my judgement specifically. Ship it if you like it for other reasons, but it is not a substitute for the client-side fix, for three reasons:

1. The client with the bug is the one that doesn't know to announce. 0.1.63 is already in the field and will never send the command. So the daemon's real protection has to be its default — "a connection that never announced gets no new event types" — and once you have that default, the handshake is doing no work here that the default wasn't already doing.
2. It moves the invariant from the type system into human memory. After negotiation, safety depends on every future author of a broadcast event remembering to gate it. This exact failure returns, identically, the first time someone forgets. That is a rule enforced by discipline; the client-side fix is enforced by construction.
3. The defect is not "the daemon sent something new." It is that the client conflates "a frame I can't parse" with "the socket is dead" — and that is already reachable without Ship the presence tick as the loops it moved, not the whole graph (#288) #297 at all: a truncated payload, a field whose type changed, any decode failure at all takes the connection down. Ship the presence tick as the loops it moved, not the whole graph (#288) #297 is what makes it routine, not what makes it possible.

What I'd actually ship: skip the frame, keep the socket — the same shape DaemonSocketClient.waitForEvent already uses (guard let event = try? ... else { continue }). One line, and it covers every future event.

But skip loudly. My brief for this review named the #293 finding — an old CLI silently printing an empty board — and a silently-skipped delta is the same failure wearing different clothes: a UI that is quietly stale is worse than one that is obviously broken. So pair the skip with a log line naming the unknown event, and consider surfacing "this app is older than the daemon" the way connectionError already surfaces. That is the part I'd hold the PR for; the skip alone just trades a loud wrong behaviour for a quiet one.

---

What I tried to break and could not

I went after the delta protocol hard, because a delta protocol is where correctness goes to die. These all hold:

Attack	Result
Client misses a delta and never recovers	Can't get there. Deltas are never superseded (supersedingKey == nil), and the only other way a frame is dropped is the backlog valve — which calls beginClosingLocked(), so the connection dies, send returns false, GraphStore forgets the client, and it reconnects to a fresh snapshot. A delta is delivered or the socket dies; it is never silently skipped.
Snapshot superseded while a delta queues behind it	Correct in both queue orders. Replace-in-place → client gets the newer snapshot then drops the older delta on revision >. Append-at-tail → client applies the delta, then the snapshot overwrites it. Converges either way. This is precisely what the revision stamp buys and it earns its place.
A node deleted rather than changed	Safe. moved never carries a deletion, and applying only replaces ids the client already holds — so a delta arriving after a deletion snapshot can't resurrect the node. The where merged.nodes[id:] != nil guard is load-bearing; please keep the comment on it.
Client joins mid-tick	Safe. addConnection stamps the joining snapshot with the current revision and notifyClients(nodesChanged:) increments before encoding, with no await between the filter and the send — so the tick is atomic against a join.
App and daemon disagree about the revision	Couldn't produce it. Snapshots are applied unconditionally and carry the daemon's number, so the app's held revision is always a daemon number and can never run ahead. A daemon restart resets the counter to 0, but it also drops the socket, and the rejoining snapshot re-stamps the app at 0 before any delta arrives. Stores are cached by path and never replaced within a daemon lifetime, so the counter can't reset under a live connection.
Actor reentrancy on the stale before	Harmless. before is captured before four awaits, so an interleaved command can make moved carry nodes that already went out in a snapshot — redundant, never wrong, and applying merges by id.
CLI / remote shim	Both skip unknown frames by construction (waitForEvent's try? + continue; the shim's wait_for key match). mail watch registers and exits rather than long-polling, so nothing burns the 64-frame budget on 15-second ticks. The CLI is genuinely more correct than before — a presence tick can no longer be mistaken for a command's acknowledgement.

Payload claim — reproduced, and it's better than the PR body says

Measured against this repo's own persisted graph (34 loops, 215 posts), post-#293 wire shape:

Frame	Bytes	vs a snapshot
graphChanged (wire snapshot)	63,482	—
nodesChanged, 1 median loop	1,155	1.8 %
nodesChanged, 3 loops	1,692	2.7 %
nodesChanged, 5 loops	2,755	4.3 %
nodesChanged, 10 loops	9,137	14.4 %

The PR body's "~1.5 KB instead of the snapshot" is accurate; a fifteen-second tick that moves one loop is a 98.2 % reduction. Worth noting the shape for the record: the delta only wins while few loops move, and crosses back over the snapshot somewhere past ~30 of 34 — which is fine and expected, just not stated anywhere.

Two small things

guard !moved.isEmpty else { return } swallows a broadcast the old code sent. If changed is true but no top-level node value differs, nothing goes out at all where a full snapshot used to. I could not construct a reachable case — all four refreshes only write graph.nodes[id:] fields — so I read this as a correct safety net rather than a bug. Flagging it because the comment above it asserts "the diff is exact" and that assertion is doing real work: it stops holding the moment anything in that tick touches graph.edges, graph.mailroom or a graph-level field. Worth a line in the comment saying so, for whoever adds the fifth refresh.

This commit carries an unrelated change from another PR. graphcode/Tests/MailboxTests.swift — splitting the suite into an extension for swiftlint's type_body_length — is #295's test file and #295's problem; #297 adds nothing to it. The rebase needed it to stay green, which is understandable, but a PR carrying another PR's change is how a revert later takes something unexpected with it. Either split it out or say so in the body.

Verification I ran myself

• Full Xcode gate: 1619 tests / 169 suites / 0 failures, run by me to completion — on 4392e12, the pre-rebase tip. I want to be exact: I have no full gate on 02935b3e. The only difference between the two is the MailboxTests swiftlint split above, so I'm treating the number as representative, but I'm not going to imply I gated the current tip when I didn't. (I killed a second full-gate run part-way on a load request; its wrapper exited 0 while xcodebuild exited 143, which is not a result.)
• Focused run on 02935b3e: BroadcastDeltaReviewTests, 2 tests — 1 passed, 1 failed as quoted above.
• Linux CI green on 4392e12; in progress on 02935b3e when I looked.
• All four Never block the daemon actor on a client write (#288) #291 invariants re-verified intact after both PRs — details in my Encode a broadcast once, not once per connection (#288) #296 comment.

---

Verdict: request changes. The delta design is right, the revision stamp genuinely earns its place, and the payload win is real and larger than claimed. But nodesChanged changes what a graphcoded connection is — a push channel — and the app's read loop was never built for that. Make an undecodable frame skippable and loud, then I'd merge it.

Probes: branch review/296-297-probes, one commit 0cd9e40 on top of 02935b3e — take them wholesale if they're useful.

🤖 Generated with Claude Code

https://claude.ai/code/session_01QhMkgJFi1Nhnh9UWwkSxDm