fix: herdr protocol drift is directional, and fields are the real contract - #6
Merged
Conversation
herdr 0.8.2 reports protocol 20 while paddock pinned 19, and checkProtocol uses strict inequality — so paddock refused to start at all. Regenerating changes two lines: the number and its comment. The status enum is byte-identical, so nothing paddock reads moved between 19 and 20. The bump then failed two tests in socket.test.ts, which hardcoded 19 and 20 as literals: "matching" stopped matching and "different" became the new pin and stopped throwing. Both now derive from HERDR_PROTOCOL. The mismatch case uses an OLDER protocol deliberately — an older herdr genuinely lacks what this paddock reads, so it must refuse under any policy this project holds, which keeps the test valid across the compatibility work that follows. Verified against the live daemon: stream connected, 6 panes subscribed, dashboard served. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
checkProtocol compared herdr's protocol with !==, so any drift either way was a fatal startup error. Measured: herdr 0.8.0 to 0.8.2 moved the protocol 19 to 20, the regenerated types differed by two lines, and the status enum was byte-identical — nothing paddock reads had changed, and paddock refused to start at all. scripts/protocol-guard.ts already encoded the asymmetry that matters: it refuses to regenerate against an older herdr because that silently narrows the contract, while allowing an upgrade. The runtime gate now matches. Older throws; newer is accepted, reported once at INFO, and exposed as health.herdrProtocol so drift is visible from a phone. What replaces the version comparison is herdr/shape.ts, checking the fields paddock actually reads against the live agent.list response on every reconcile — not only at startup, because the protocol is read once when the daemon is first reached, so a running paddock survives a herdr upgrade and only a restart reveals the break. It covers REQUIRED fields only. `name` and `terminal_title_stripped` are optional and adapter.ts already falls back for both, so an unnamed pane legitimately lacks them; checking those would report a break on an ordinary install. Zero panes yields unknown, never broken. Broken refuses at startup, and once running is logged on change and exposed as health.schemaWarning. Rendering every agent in one wrong state is worse than not starting — the operator would act on it. Verified against the live daemon with the pin held at 19 against herdr's 20: started, reported the drift, confirmed every field present, subscribed 6 panes, served, and health carried herdrProtocol 20 with schemaWarning null. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ing a break Seven review findings on the compatibility work, four of them holes the relaxation itself opened. The serious one: checkProtocol accepts any newer herdr, but the shape check only asserted key PRESENCE. adapter.ts:toState returns null for any status outside the four it renders, so toAgent drops the row — a herdr that ADDS a status would make every agent in that state vanish from the dashboard with schemaWarning null and nothing logged, where the old `!==` gate would have refused loudly. agent_status is now validated against the generated enum, which the generator emits as a runtime array so `make types` keeps the two in step rather than a copy drifting beside the check. Two ordered comparisons had a hole `!==` did not: `undefined < N` and `undefined > N` are both false, so an absent or non-numeric protocol read as an exact match — paddock started as if verified, and doctor scored 0 while printing "herdr reports undefined", lying to install.sh. Both now guard the type before comparing. An `unknown` verdict no longer overwrites what was already observed. A break was recorded, the operator closed their panes, and zero rows silently cleared schemaWarning while the contract was still broken. Zero rows means we learned nothing, and learning nothing must not erase what we learned; only positive evidence of health clears a break. The change log likewise stopped announcing "every field present" for data it never inspected. A field missing from ONE row is per-row variation, not a contract change — agent.list demonstrably carries rows toAgent discards, and `broken` exits 1, so one anomalous entry was a self-inflicted outage. Missing now means absent from every row, and a malformed entry is skipped rather than counted as every field gone. Also dropped the claim that schemaWarning reaches the phone: nothing under src/web reads /api/health, so it is the same operator-diagnostic surface lastNotifyError already uses. And the recovery path re-pings, so the reported protocol is the daemon actually answering rather than the one that answered at boot — stale in exactly the scenario the field exists for. Verified against the live daemon by narrowing paddock's known enum so herdr's real "working" status fell outside it: refused to start, exit 1, naming "working", never reaching listen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four conflicts, all resolved keeping both sides. docs/decisions.md — both branches added a decision 13. The pairing gate keeps 13, because the plan document already on main and decision 3's own scope note both say "see decision 13" for it; protocol drift becomes 14. Renumbering the other way would have left two dangling references. src/server/doctor.ts — the cloudflared line (unconditional, from the tunnel work) and the newer-protocol note (conditional) both belong in the compatible branch. cloudflared first, so the version facts stay together above the advisory paragraph. src/server/index.ts — two import hunks, pure additions on both sides. `type HubClient` was dropped with main's version: ws/serve owns the websocket handlers now, so index.ts no longer names it. tests/doctor.test.ts — both test suites kept. The shared closing brace sat outside the conflict region, so concatenating the two sides orphaned it. Also added the two new required HealthBody fields to the tunnel branch's test fixtures, which the typechecker found — the reason those fields are required rather than optional. 819 pass, 0 fail. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What and why
paddock would not start against the installed herdr at all. herdr 0.8.2
reports protocol 20, paddock pinned 19, and
checkProtocolcompared them with!==— so any drift in either direction was a fatal startup error before a portwas bound:
Regenerating changes two lines: the number and its comment. The status enum
is byte-identical. Nothing paddock reads moved between 19 and 20, so an integer
with no consequence took the whole dashboard down — and it will do so again on
every herdr release unless the policy changes.
The policy
scripts/protocol-guard.tsalready encoded the asymmetry that matters:make typesrefuses to regenerate against an OLDER herdr, because that shrinks theenums and silently narrows the contract, while an upgrade is allowed. The
runtime gate now matches it.
health.herdrProtocolso the drift is visible from a phone, not only a log.doctorscores older as 1 and newer as 0.install.shreads that code,and herdr moving ahead is not a broken install.
What replaces the version comparison
A protocol number was never the contract.
src/server/herdr/shape.tschecks thefields paddock actually reads against the live
agent.listresponse.daemon is first reached, so a long-lived paddock keeps working across a herdr
upgrade and only a restart reveals the break. That is the ordinary case, not
an edge one.
pane_id,workspace_id,agent_status.nameand
terminal_title_strippedare?:inHerdrAgentRawandadapter.tsalready falls back for both, so a pane that was never named legitimately lacks
them. Checking those would report a protocol break on an ordinary install, and
a check that cries wolf on normal data trains you to ignore it.
unknown, neverbroken. You cannot conclude a field isgone from no rows, and refusing because herdr happens to have nothing open
would be a self-inflicted outage.
herdr api schema --json— for the reasondoctor.tsalready records: the CLI answers from the binary on disk while the socket
answers from the running daemon, and the two disagree after an upgrade, which
is the exact confusion this work exists to end.
A broken shape refuses at startup; once running it is logged on change and
exposed as
health.schemaWarning. Rendering every agent in one wrong state, orevery row under the same label, is worse than not starting — the operator would
act on it.
Also fixed, found by the bump
Two tests in
socket.test.tshardcoded19and20as literals, so everylegitimate pin bump broke them: "matching" stopped matching, and "different"
became the new pin and stopped throwing. Both now derive from
HERDR_PROTOCOL, and the mismatch case uses an OLDER protocol deliberately, soit stays valid under this policy. A test that fails on a routine regeneration is
noise that trains you to edit tests instead of reading them.
Verified against the live daemon
With the pin held at 19 against herdr's 20:
/api/healthreportedherdrProtocol: 20,schemaWarning: null, 6 agents.Reviewer note: numbering conflict with #5
Both this branch and #5 add a decision 13 — #5's is the quick-tunnel pairing
gate, this one is protocol drift. Each is numbered correctly against
main, sowhichever merges second needs renumbering to 14. One line, but it will conflict.
Checklist
make check && make check-clean && make testpass — 692 pass, 0 fail,tscclean, scanner clean. Note this branch also GREENS a suite that wasred on
main: the drift test was failing there before this change.empty-list-returns-ok, inspecting only the first agent, treating optional
nameas required, and a pin ahead of the installed herdr. Each failed theexpected test, then was reverted.
two lines with a byte-identical status enum is in the commit message.
🤖 Generated with Claude Code