Conversation
Salvage checkpoint before restoring sim link-control wrappers.
Adds impair/impair_dir/link_counters/buffered/heal_link_faults so a scenario can name a fault, drive known tones across a broken link, and assert the receiver saw what the link says it sent. Two ordering defects surfaced while writing them: - nestler_send injects on a detached task, so consecutive sends have no wire order. A fault script names a tone by position, so it needs nestler_send_ordered, which injects in the caller's task. - a broadcast receiver only sees tones published after it exists, so a test that subscribes inside a spawned task races its own first send. humd_peer_sub takes the handle before the scenario sends. Link setup is traffic the link really carried, so scenarios measure LinkCounters::since a post-handshake baseline rather than a total. heal flushes the partition buffer through the link's own fault verdict, so a lossy heal loses the head of the buffer. Test asserts that. 9 scenarios: intact, loss, dup, reorder, partition, lossy heal, recovery, directional impairment, and inbox-level drop accounting.
A peer is live while traffic has arrived within ttl. The drainer stamps a per-peer lease on every inbound tone, so a busy peer needs no probing; a wedged link stops stamping and goes stale. The drainer previously exited on a closed transport leaving the registry entry orphaned, and nothing anywhere removed a peer except the peer-remove CLI. The exit now records TransportClosed, which a sweep can reap. chi:peer-ping / chi:peer-pong as a literal pair, matching how gossip-publish and kad-find-node avoid pulling the chi enum into this crate. Probes are answered in the drainer and never reach subscribers. Dead outranks stale: the transport's word about its own link is a fact, whereas staleness is inferred from a quiet clock.
Boot dialed every peer once and never looked back, so a peer that died stayed in the registry forever: routing kept addressing a dead link, and a peer that restarted was unreachable because nobody dialed it again. Recovery required restarting the daemon. The supervisor probes, then sweeps. A peer that stops answering goes stale; after ttl it is evicted and redialled. Backoff is per-peer with full jitter, so one dead peer doesn't slow the others and a mesh restarting together doesn't synchronise onto the same retry instant. tick() is public and returns a SweepReport so a scenario can drive the supervisor deterministically instead of waiting on a timer. Each transport gains dial_one, so a redial failure is attributable to one peer rather than a whole sweep. A transport that isn't configured is skipped rather than failed.
InMemoryEndpoint::kill drops the outbound *sender*, which is what closes the peer's receiver and ends its drainer. close() dropped only our own receiver, so it stopped us reading while leaving the peer with a link that looked fine — a kill built on it detected nothing. kill is distinct from partition on purpose: a partitioned link stays nominally up and its peer keeps renewing its lease, so a partition heals itself and a dead peer needs a redial. A liveness sweep that reaped partitions would churn the peer set on every network blip. The sender moved behind a lock so kill can drop it. Sends go through emit/try_emit, which take the sender out for the duration rather than holding a parking_lot guard across the await — holding it would block kill, and kill would deadlock against an in-flight send.
tmp() keyed the directory on pid + now_ms(). Tests run in parallel, so two starting in the same millisecond shared a directory; whichever finished first called remove_dir_all and the other found it empty. Seen as flag_persists_through_wilt failing in a full workspace run and passing alone. Add a per-call counter to the name.
Three defects, all found by driving the real supervisor and sim rather than the units underneath them. An unproven peer never expired. Lease::state returned Live whenever last_seen was None, so a link that swallowed every probe stayed Live forever — "unproven" had quietly become "immortal", and the black-hole case was the one case that could not be detected at all. A lease is a deadline: count silence from install, not from the first reply. Sim::probe(a, b) looked up b and probed b's peers, so it ignored a entirely and reported on the reverse direction. It happened to agree with the fault scripts written alongside it, which is how the bug survived. Add Ensemble::probe_one so a probe says something about the direction it travels; clone the conn out from under the registry lock while we are here, since holding that read lock across a send blocks every add_peer/remove_peer for the length of the send. The redial set was "just evicted", which misses every peer that failed its *first* dial: those were never installed, so no sweep can name them, and a peer down at boot stayed down permanently. It is now "configured but absent". Related, dial() returned !tried, so a peer with no usable hint counted as a successful redial, cleared its own backoff, and would be re-dialled forever. Correction to 4e630e6: I claimed a sweep that reaped partitions would churn the peer set. It does reap them, and that is right. A partition is not distinguishable from a death by observation — both are silence — so sweeping on silence is honest, and backoff plus redial is what prevents churn. What a partition is not is Dead, which stays reserved for a transport that reported closure: a fact rather than an inference, and the only case that earns an immediate redial.
link_recovers_after_lossy_heal subscribed to b's inbox after healing, but a subscription only sees tones delivered after it exists and the healed buffer reaches the inbox asynchronously. When a partition drops fewer than three, the window is short enough for a late arrival to displace an "ok", so the assertion failed under load and passed alone. Subscribe first. That makes the arrival order fixed, and lets the assertion cover the lossy heal as well as the recovery: two buffered tones survive, heal's own wane-sync sits between them and the new marks, and the three "ok" marks follow in order.
`rid` is not a message identity, so the answer to the open "should we
dedup on rid" question is no, and the reason is in WIRE.md: `chi:"echo"`
is the ack *for* an rid, so a request and its response share one, and a
receiver deduping on rid would drop every response as a duplicate of
its request. rids are also explicitly per-originator and format-
agnostic, and reference clients mint per-session values (`p-<sid>`)
that recur across restarts.
The gossip id was therefore neither. mint_msg_id hashed
(topic, rid, from, payload) — content addressing, which cannot tell
"delivered twice" from "sent twice", and repeats are ordinary for the
tones gossip exists to carry: a heartbeat, a standing overload alert, a
retry. Worse, publish marks its own id seen before sending, so a repeat
that hashed alike was dropped at the origin and never left. Since the
rid was `gossip-{topic}-{now_ms()}`, the reachable case was two
identical publishes inside one millisecond.
The id is now assigned by the originator, once per publish, as
`{origin6}-{ms:x}-{seq:x}`. A per-process counter makes every publish
distinct whatever the clock or the content does, and the origin prefix
keeps two humds from colliding. The wire field name is unchanged, and
the existing canary test — which builds one tone and sends it twice —
still dedups, because the dedup happens on the id in the tone, not on
how it was minted.
Also enforce `dusk`, the other half. A tone arriving past its own dusk
is dropped rather than delivered or re-fanned, and counted in
`expired_dusk()` so a scenario can assert on it rather than infer from
absence. Checked after the liveness stamp, because an expired tone
still proves the link is alive. No dusk means no expiry — the field is
optional. This also bounds what the seen-set must remember: an id only
has to outlive the window in which a duplicate could still be in
flight.
Unicast `route()` is deliberately left without dedup. There is no sound
key for it — the only candidate, rid, repeats by design — and inventing
one would drop correlated responses.
sim/tests/delivery.rs pins the line the old id could not draw: an
identical payload published twice arrives twice, and one publish
duplicated by the link arrives once. The same bytes; only the sender
knows which it meant.
peers and identity both set XDG_CONFIG_HOME / XDG_STATE_HOME and removed them on the way out. Both variables are process-global and the humd test binary runs its tests in parallel threads, so a test setting one pointed every other test in the process at its own fixture, and the remove_var left the rest reading whatever came next. peers.rs failed this way on a full workspace run and passed alone. Split load_from(path) and load_or_mint_key_at(path) out of the env-derived defaults and have the tests name their own files. The production entry points are unchanged. hum-identity already serializes its own env-touching tests behind a lock and shares a binary with no other path-reading test, so it stays as it is.
Closes the last open item from the delivery work. `route()` had no dedup because no sound key existed for it: rid, the only candidate, repeats by design. The key already existed — the originator-assigned id used for gossip — it just had no unicast spelling. `mid` is that spelling, an optional envelope field, and the resolution follows from the same distinction the gossip fix turned on: mid answers *which message is this*, rid answers *which conversation is it in*. A response therefore carries the request's rid and its own mid. Same rid, different mid, both delivered. A retry carries same rid, same mid, delivered once. The prior state is that sending the same body twice means two messages, and that stays true: a tone with no mid makes no at-most-once claim and is delivered every time it is sent. Absence is meaningful, and a receiver must not substitute rid for a missing mid. That substitution is the failure this is built to prevent, so it is now a test. a_response_echoing_the_request_rid_still_arrives sends a request and a response under one rid and requires both; mutated to dedup on rid it fails, and so does the no-mid test, because rid-dedup swallows exactly the sends the sender meant. Mutated the other way — mid check disabled — a_retransmitted_mid_is_delivered_once fails. Both directions of the mistake are caught rather than assumed away. Dusk moves into the new ensemble::delivery module alongside the mid seen-set, since identity and lifetime are the same question asked twice: may this tone be delivered, and has it already been. Dusk is checked first so a dead tone cannot occupy seen-set capacity and evict a live id. The two seen-sets stay separate on purpose — a mid records that this node already delivered a tone, a gossip msg_id also governs whether it is re-fanned, and sharing one set would let each evict the other's entries early. mid is a first-class Envelope field rather than left in the tone body, so the existing "envelope fields win" rule applies and a body mid cannot shadow it. The generated clients pick the field up from codegen. 8 delivery scenarios green; workspace 51/51 across three runs; clippy clean.
Follows up the delivery work by reading the code instead of assuming it.
Six of the open questions had answers, and two of them contradicted what
I had written down.
The seen-set was bounded in entries but not in bytes. `mid` is
attacker-controlled and the TCP transport reads NDJSON with
`BufReader::lines()`, which has no length cap, so 4096 entries of
unbounded string is not a bound — a peer could put a megabyte in one
field. `mid_key` now digests to 32 bytes, making the footprint
`cap * 32` whatever arrives, with dedup still exact. `mid_prefix` keeps
logs to a char-safe 12 chars so a 4 MB mid cannot land in a log line.
On signing: there is none, per tone. `handshake_message` covers only
`chi:"hello"`, and after that a connection is trusted wholesale, so
`mid` is exactly as (un)protected as `chi`, `rid`, `to` and the payload.
Nothing about mid is uniquely weak, but a mid being unbounded is real
because a connected peer controls every field.
`DeliveryState::with_cap` exists so capacity is reachable, and
the_seen_set_evicts_at_its_cap now proves eviction at a cap of 2. While
adding it I found `seen_set_dedups_within_capacity` in gossip never came
close to GOSSIP_SEEN_CAP — it asserted three lookups and the name
promised a bound. Renamed to `seen_set_dedups_repeats`, and gave the
real thing its own `seen_set_evicts_at_its_cap`.
`mint_msg_id` sliced `to_hex()[..12]` as bytes. Hid renders hex today
so it cannot panic, but the slice is now char-wise with a fallback,
since a panic in a publisher is not a good failure mode for a cosmetic
prefix.
The originator question resolves against humd, and the answer is that
humd is a relay. Worker replies are forwarded with `to`/`from` rewritten
and everything else — including any `mid` — carried through untouched,
which is the only correct behaviour: a relay that minted its own mid
would break dedup, because two relays of one message would disagree.
The originator is the nest side, and the one in this repo is
`hives/bp7`, which mints `p-{sid}` — the per-session rid that recurs
across restarts, the exact shape that deduping on rid would break. It
now also mints a `mid`, which is load-bearing there specifically because
bp7 is a DTN store-and-forward bridge and re-running a prompt is not
idempotent. bp7 is excluded from the workspace, so it builds
standalone.
Gossip could not set its own `dusk`, and still does not. A library TTL
would silently drop announcements on a congested mesh, and how stale an
alert may be is the publisher's call — a heartbeat wants seconds, a
"worker moved" notice wants minutes. So `publish_with_dusk` makes the
mechanism reachable and the default stays `None`. The new test also
pins where expiry lands: at the FIRST hop, not the last, so a dead tone
dies on entry to the mesh instead of being carried to every subscriber
and dropped N times. My first draft asserted on the far hop and was
wrong.
WIRE.md's chi tables were contradicting `thrum-core/src/views.rs`, which
calls itself the single source of truth for body shapes and is locked by
a golden-bytes test. Ten rows named fields that do not exist:
`permission-ask` and `release-permit` were described with a `permitId`
neither body has (both key on `callId`), `chunk` had a `part` and
`index`, `session-ready` had a `claudeSessionId` against a real
`nestId`, `pulse` had a `cellId` against a real `pid`, `tendril-reach`
had four fields against a real `task`/`tools`. All corrected against
views.rs, and the tables now say that `body fields` excludes `sid`,
which is an envelope field and was listed as a body field throughout.
The claude-cli `HOME` tests turned out to be fine. Every test in both
files holds a per-file lock for its whole body, and the two files are
separate binaries, so they cannot race each other — the same discipline
as hum-identity. Left alone on the evidence.
Workspace 51/51 across three runs; clippy clean.
`cargo clippy -- -D warnings` aborts in codegen/hum-paths before it ever lints ensemble, so it cannot see its own mistakes here. Linting the packages directly with --no-deps found two, both mine. `a_dead_mid_does_not_evict_a_live_one` had no `#[test]`. An earlier edit left its attribute attached to the function that followed it, so the test compiled as dead code and the suite stayed green without ever running it. The orphaned `#[test]` and a duplicated doc comment are gone and the test runs — and passes, which is the less interesting part. The interesting part is that "51/51 green" was reporting on a test that was not in the suite. Also collapsed a nested if in `Supervisor::dial`, plus a needless borrow in the delivery tests. Everything else under `-D warnings` in these crates is pre-existing: the collapsible ifs in the gossip/KAD dispatch blocks, the penny/drone/ drift dead code, `humd/src/lib.rs` broadly. The remaining hard failures are all in codegen/src/protocol.rs and hum-paths, untouched here. Workspace 51/51 across three runs. Note the count was 51 before this commit too — a non-running test does not change the number of green test binaries, which is exactly why it went unnoticed.
Owner
Author
|
Pushed one more commit (
Everything else under |
…mesh A peer that stops reading doesn't close its socket. The buffer fills, the write stops completing, and the connection keeps looking healthy to a liveness lease — which renews on unrelated traffic, so silence never trips it. Meanwhile the sender is parked inside write_all, and every fan-out that walks peers sequentially is now parked with it. The only symptom was a Lagged(n) on a broadcast receiver: a count, and no cause. Every send is now bounded by SEND_TIMEOUT (5s). On timeout the connection is closed rather than retried, so the lease can mark the peer dead and the supervisor redial it, and the event lands in send_timeouts() where it can be alerted on. Applied to all seven send sites: hello, both probes, publish, re-fan, KAD replies, and route. The probes matter most, and not just for the wedge: a probe that blocks forever is a probe that never reports the stall it exists to find. sim: InMemoryEndpoint::stall() models a peer whose buffer has filled — the write hangs rather than erroring, because an error would let a fix that only checks the return value pass without touching the deadline. The five tests in sim/tests/stalled_peer.rs were each checked against a reverted fix. The two fan-out tests fail without the deadline (the publish itself hangs, so the test bounds it and fails with a message instead of eating the CI timeout). Note the honest limits: route() targets one peer, so two separate routes don't serialise — the fan-out tests target publish(), which is the real single loop. A test asserting "a healthy peer still gets through" on the route path would have passed either way.
Each of these was green while asserting nothing, or claiming a coverage it did not have. All three are now checked against a deliberate break. iroh_integration: try_bind() turned every failure into `None` and the test returned, so a broken transport and a working one looked identical. The skip was hiding an actual bug — see below. Skips are now classified against a short list of sandbox signatures, anything unrecognised is a failure, HUM_REQUIRE_TRANSPORT_TESTS=1 turns even a forgiven skip into a panic so the job with real UDP can't quietly lose the coverage, and the classifier has its own tests so it can't rot into "ignore everything". Underneath was a genuine defect. The test built dial hints from bound_sockets(), which reports the addresses we *bound* — 0.0.0.0 and [::]. Neither is a destination, so the dial hung for the full 30s timeout and the swallowed error called it a skip. The test had never once completed a handshake. IrohTransport::dial_hints maps the unspecified address to loopback and leaves a concrete address alone (rewriting a real LAN address to 127.0.0.1 would break T1). Runtime went from a 31s hang to 0.9s, and the test asserts its hints are dialable so the shape can't come back. partition_and_heal: the header claimed it caught "the wire silently keeps delivering during partition". It did not, and could not — nothing is sent until the heal, so the tips stay divergent whether the link is sealed or wide open. The test now pushes a real wane-sync into the outage and asserts it is refused. Verified: un-partitioning the link now fails two tests, where it failed none before. Also added a negative control (an unhealed outage must never reconcile) and a rewind check — a min-merge satisfies "both sides are equal" and would pass a naive convergence assertion while silently rewinding a Lamport clock. sim::wane_sync is split out of heal() so a test can emit one during an outage.
This branch has not been deployed
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
1 participant
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 this is
Twelve commits taking the ensemble from "looks distributed" to something
whose failure behaviour is specified, tested, and mostly correct. Three
phases: a fault model for the simulator, real peer liveness, and a fix to
the identity that delivery dedup was built on.
The through-line is that most of the bugs here were invisible — code
that could not fail visibly because nothing modelled failure. So the
simulator came first.
1. The simulator can now fail (4e630e6, 4843051, 8a9ea8f)
Loss, duplication, reordering, partition, bounded-buffering overflow,
and lossy heal — each with a
LinkCountersoracle, so a test assertswhat the wire actually did rather than what arrived. Two test-only races
fixed along the way: a lossy-heal test that subscribed after healing, and
drifttests that collided on a shared temp directory.2. Peers are live, dead, or absent — and each is handled (8784e07, 655d9c0, 9fa8fa8, 7ec3b37)
A liveness lease with directional probe, plus a redial supervisor with
exponential backoff. Four real bugs fell out:
pings never expired for a peer that never pinged. Silence now means
Stale;Deadmeans the transport reported closure.seq, not theirs of us.
never connected was never retried. Configured-but-absent is now a
first-class case.
longer counts as connected.
killdrops the outbound sender, so a dead peer produces silence ratherthan a silently live connection.
3.
ridis not a message identity (7c45d2f, 359d8c3, fbaf776, 523aea6)The parked question — "should we dedup on
rid?" — has an answer, and itis no:
chi:"echo"is the ack for an rid, so a request and itsresponse share one, and a receiver deduping on rid drops every response as
a duplicate of its request. Reference clients mint per-session rids
(
p-<sid>) that recur across restarts.The gossip id was neither.
mint_msg_idhashed(topic, rid, from, payload)— content addressing, which cannot tell"delivered twice" from "sent twice", and repeats are what gossip is for
(a heartbeat, a standing overload alert, a retry). Worse,
publishmarkedits own id seen before sending, so a repeat that hashed alike was
dropped at the origin and never left; with a ms-precision rid, two
identical publishes in one millisecond hit it.
Ids are now assigned by the originator, once per publish, as
{origin6}-{ms:x}-{seq:x}, and unicast got the same treatment via anoptional
mid— wheremidanswers which message is this andridanswers which conversation is it in. A response carries the request's
rid and its own mid.
Also enforced
dusk: a tone arriving past its own deadline is droppedand counted, checked after the liveness stamp (an expired tone still
proves the link is alive) and before gossip re-fan.
These tests were mutation-tested, not just made green
middisabled →a_retransmitted_mid_is_delivered_oncefailsridinstead →a_response_echoing_the_request_rid_still_arrivesand
a_tone_with_no_mid_is_delivered_every_time_it_is_sentfailThe second is the one that matters: rid-dedup silently eats exactly the
sends the sender meant, and nothing else would look wrong.
4. Read the code instead of assuming it (523aea6)
Two claims in my own prior summary turned out to be wrong on inspection:
handshake_messagecovers onlychi:"hello"; after that a connection is trusted wholesale, somidisexactly as unprotected as
chiandrid. Not uniquely weak — butBufReader::lines()has no frame cap, so amidis genuinely unbounded.The seen-set is now bounded in bytes (
cap * 32, digested), not justentries.
to/fromrewritten and everything else untouched, which is the only correct
behaviour — a relay that minted its own mid would break dedup. The
in-repo originator is
hives/bp7, which mintsp-{sid}and now alsomints a
mid. That matters there specifically because bp7 is a DTNstore-and-forward bridge and re-running a prompt is not idempotent.
Two doc defects found by looking rather than reading:
seen_set_dedups_within_capacityin gossip never approached capacity —three lookups, with a name promising a bound. Renamed honestly, and
given a real eviction test.
WIRE.md's chi tables contradictedthrum-core/src/views.rs, which calls itself the source of truth and islocked by a golden-bytes test.
permission-askandrelease-permitwereboth documented with a
permitIdneither body has;session-readyhadclaudeSessionIdagainst a realnestId;pulsehadcellIdagainst areal
pid. Corrected, and the tables now notesidis an envelope field,which it listed as a body field throughout.
Two latent test races fixed:
humd::peersandhumd::identityweresetting and removing process-global
XDG_CONFIG_HOME/XDG_STATE_HOME,which under parallel tests repointed every other test in the process at
their own fixture. Split out
load_from/load_or_mint_key_at; theproduction entry points are unchanged.
Deliberately not done
midis enforced but nothing mints one except bp7. Thenest-side originators are the TS/Go/Python clients, outside this repo.
a congested mesh, and how stale an alert may be is the publisher's call.
publish_with_duskmakes it reachable; the default staysNone.stream with no duplication source, so one set per receiving edge is the
correct topology.
Verification
cargo test --workspace— 51/51 test binaries, three consecutive runs.cargo clippy --workspace --all-targets— 0 errors (45 pre-existingcollapsible_ifwarnings at baseline, unchanged).hives/bp7is excludedfrom the workspace and was built standalone.