Repository navigation
Health-check heuristic fix, operator drain/block controls, and websocket close-code correctness - #526
Merged
Merged
Conversation
A CometBFT `health` check answers {"jsonrpc":"2.0","id":1,"result":{}} when the
node is healthy. The heuristic's empty-object rule flagged exactly that, at
confidence 0.95, on every healthy node every cycle.
The carve-out for it already existed in analyzeJSONRPC: skip the rule when the
rpc type is COMET_BFT or isCometBFTMethod(method) holds. Neither guard could
fire. Cosmos services declare the check as type json_rpc, and the method never
reached the heuristic at all.
Two separate sites dropped it, and fixing either one alone changes nothing
observable:
1. buildServicePayload never set JSONRPCMethod. User traffic gets that field
at QoS parsing time; a health check does not go through QoS, so it stayed
empty and protocol/shannon/context.go fell back to payload.Path — "/" for
a JSON-RPC check, matching no method.
2. The executor runs its own heuristic.Analyze after the relay returns, and
that call passed a hardcoded "". This is the site that decides the check's
outcome, and it runs only when site 1 passes.
So repairing site 1 by itself just hands the response to site 2, which rejects
it on the same rule. The check stays at 100% failure and only the metric label
moves — from status_code="error" to status_code="200", both major_error.
Both sites now resolve the method the way the other four callers do (hedge.go,
http_request_context_handle_request.go x3): payload method, Path as fallback,
so REST path-aware rules are unaffected.
Measured 2026-08-06 against production: 16 cosmos services failing this check
100% of the time, one passing. That single check accounted for ~19% of all
failing health-check signals fleet-wide.
The penalty was uniform across every endpoint on those services, so it distorted
no ranking and tripped no alert — it only depressed the absolute baseline.
Scores on cosmos services will rise once this ships, and any threshold tuned
while it was in effect was tuned against that depressed baseline.
Tests drive ExecuteCheckViaProtocol through a mock protocol rather than calling
heuristic.Analyze directly: a test that calls the analyzer restates its rules and
passes with this fix reverted. The EVM case is pinned too — an empty-object
result for eth_blockNumber must still be rejected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Several keys the loader has read for a while were missing from the schema, and active_health_checks sets additionalProperties: false — so a valid config that used them was flagged as invalid in the editor. Adds backend_dedup, sync_allowance (global and per-service), max_workers, and the per-check enabled / headers / archival / sync_check. Corrects the check `type` description, which still named the pre-rename "jsonrpc" and carried no enum, and adds fatal_error to reputation_signal. Schema only. Nothing here changes how a config is parsed: the loader uses a non-strict yaml.Unmarshal, so these keys already worked at runtime and unknown ones are still ignored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds POST /admin/reputation/drain/{serviceId}, which writes a cooldown
expiry onto every scored endpoint of one operator (eTLD+1) for a service.
Selection already excludes endpoints in cooldown regardless of score, so
this reuses a filter every selection path is guaranteed to consult rather
than adding a second exclusion mechanism that some path could miss.
Motivation: "where would this traffic go if operator X were unavailable"
was only answerable by waiting for X to fail. Tumbling a websocket
connection is not a substitute -- a tumble re-dials but leaves every
operator eligible, so the connection can land straight back where it
started. Measured on gnosis: 8 consecutive tumbles failed to move a
~500 frames/s subscription off the two operators already carrying it,
because each rebind could reselect them.
This is NOT a penalty. Value, CriticalStrikes and RecentCriticalRate are
left untouched, so the quality signal stays readable while a drain is in
effect -- which matters, because reading it is usually the entire point of
draining. A drain that rewrote the score would destroy the measurement it
exists to enable.
rpc_type narrows the drain to one protocol, so benching an operator's
websocket endpoints leaves its json_rpc and rest traffic in place.
Release (duration=0) lifts only cooldowns this drain wrote and that nothing
has overwritten since. A cooldown earned for real while the drain was up
survives -- otherwise "undo my experiment" would silently un-bench a
legitimately failing endpoint, the one outcome nobody would intend.
Known limit: the cooldown lives on the score, so an endpoint the reputation
service has never observed is treated by selection as "initial score, not
in cooldown" and stays selectable through a drain. The response carries
unscored_warning rather than letting a partial drain read as airtight.
Per-pod in-memory state like the other admin endpoints, so it must be
issued to each pod. Expires on its own and does not survive a restart.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds path_websocket_connection_frame_rate: every live connection's endpoint→client frames/sec, observed on each sampler pass, bucketed by service_id and domain. Why: path_websocket_messages_total is a per-domain SUM, and a sum cannot distinguish "one firehose plus a hundred idle sockets" from "a hundred ordinary subscribers". Those have opposite explanations -- the first is where a high-volume client happened to land, the second is a property of the operator -- and the distinction is not academic. Measured fleetwide: one operator holds 16.9% of websocket connections and earns 66.1% of websocket relays, a 3.9x over-index that the existing sums cannot attribute. On gnosis, 2 connections out of ~70 carried ~97% of frames, so a handful of connections can produce that entire fleetwide number without the operator doing anything. Only the distribution separates the two. Both frame directions are reward-eligible under the relay miner (each endpoint→client push is signed and mined paired with the most recent request), so this is closer to a settlement-volume distribution than a traffic one. Emitted from the existing sampleRates pass rather than a new ticker, so the histogram and the ranking a capped tumble is spent on can never disagree -- otherwise a dashboard could justify a tumble the tumble itself would then decline to make. Idle connections are observed at 0 deliberately. Dropping them would leave the quantiles describing only the connections that carry traffic, which is exactly the population the metric exists to be measured against. Observations are collected under the registry lock but recorded after releasing it: a Prometheus histogram takes its own internal lock, and nesting that inside the registry mutex would put an unrelated subsystem on the critical path of every admin tumble and rebind. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Documents POST /admin/reputation/drain/{serviceId} alongside the other
admin endpoints, and path_websocket_connection_frame_rate.
Also records the finding that motivated both: every endpoint→client
WebSocket frame is signed and mined as a reward-eligible relay, so one
eth_subscribe anchors an unbounded number of payable relays and the push
rate is chosen by the supplier being paid. A per-domain frames/s number is
therefore a settlement-volume meter, not a load meter -- the opposite of
how the existing panel description read it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects in the drain endpoint, both found by using it against mainnet. 1. The bench did not survive a storage refresh. The drain wrote CooldownUntil onto the Score. refreshFromStorage overwrites the local cache from Redis unconditionally, so every drain evaporated on the next refresh tick -- while the endpoint still reported drained=N. Observed live: bsc endpoints showed 0 in cooldown minutes after a successful drain, and traffic returned to the drained operator on its own. The bench now lives in drainedKeys and is applied as an overlay when scores are read, so nothing that rewrites scores can erase it. Releasing also becomes exact rather than best-effort: because the drain never touches the Score, a cooldown the endpoint earned on its own cannot be disturbed by a release, by construction rather than by an equality check. Same trap CLAUDE.md already documents for the circuit breaker's refreshFromRedis, walked into from the other direction. 2. It could only be targeted by node id. Key granularity is per-service config, so the same operator is a hostname on one service and a pokt1... supplier address on another. An eTLD+1 filter returned matched=0 on supplier-keyed services while looking successful. The handler now resolves an eTLD+1, hostname, or full URL against live endpoint details into every identifier a key could carry, and reports identifiers_resolved / matched_endpoints so a failed resolution is distinguishable from a resolution that matched nothing. Resolution lives in the router because only the protocol layer holds the supplier->URL mapping needed to bridge granularities. Testing: the previous tests asserted Score.CooldownUntil was set, which is exactly the field that got clobbered -- they passed while the mechanism was broken. Tests now assert through GetScores/IsInCooldown, which is the call selection actually makes (FilterByScore ignores cooldown entirely). Verified the regression tests have teeth by neutering the overlay: 5 of 9 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A drain was pod-local, so benching an operator meant hitting all N pods.
In practice that produced partial drains nobody realised were partial --
observed while trying to bench two operators across 16 replicas.
Drains now live in shared storage under a dedicated __drains__ hash and
every replica adopts them on its next refresh, so one admin call applies
fleet-wide. A hash rather than one key per drain: the refresh reads the
whole set each tick, so it is one HGETALL instead of a SCAN, and the score
keyspace SCAN cannot pick it up.
Deliberately NOT stored on the Score. That is what the previous fix was
about: refreshFromStorage overwrites the cache from storage unconditionally,
so anything carried on Score.CooldownUntil is erased within a refresh cycle.
Drains and scores stay strictly separate.
Refresh REPLACES the local set rather than merging, which is what makes a
release propagate; merging would leave a released drain benched forever on
whichever replica did not issue it. A storage failure leaves the local set
intact instead of clearing it -- losing Redis mid-incident must not silently
un-bench everyone -- and a failed write reports propagation_error with a
THIS POD ONLY warning rather than implying a fleet-wide bench.
Expiry, so a drain can be set and forgotten:
- duration capped at 2h, rejected rather than clamped (silently shortening
a drain an operator believed they set is worse than telling them)
- TTL on the shared key, extended never shortened (ExpireGT), so a short
drain cannot cut short a longer one in the same hash
- expired fields filtered on read and reaped, since Redis cannot TTL
individual hash fields
There is no way to bench an operator indefinitely through this endpoint.
Testing: 14 drain tests, including two replicas over shared storage proving
a drain and a release each reach a pod that never received an admin call,
that an expired drain is never applied, that a storage outage keeps existing
drains, and that a failed write is reported as pod-local.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Verifies the drain overlay cannot leak into anything that scores or persists reputation: - the raw cached Score keeps CooldownUntil zero while benched, and nothing benched reaches storage. A leak here would leave an operator carrying a real cooldown after a 20-minute experiment, making the reputation record a lie about their behaviour. - the rate-cooldown escalation ladder does not see it. That ladder keys off the PREVIOUS CooldownUntil and benches longer on each consecutive trip, so a visible drain would silently escalate an operator's next real cooldown -- punishing them for something we did. - recovering the score neither lifts the drain nor picks up the overlay. Isolation holds because RecordSignal and recoverScore read/build the raw Score, and the overlay is applied only on the GetScore/GetScores read paths. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A drain removes an operator's traffic exactly the way a cooldown does, so
path_endpoints_in_cooldown counted both -- making a deliberate bench
indistinguishable from a quality incident on every dashboard.
That is backwards for this tool in particular: a drain is usually applied
in order to OBSERVE an operator, so publishing it as a cooldown corrupts
the signal the drain exists to produce, and would eventually have someone
paged over a bench we applied ourselves.
path_endpoints_in_cooldown now counts only cooldowns the endpoint EARNED,
reading the un-overlaid score via the new GetScoreRaw. Drains are published
on their own gauge, path_endpoints_drained{domain, rpc_type, service_id},
sourced from IsDrained. Selection still excludes both -- only reporting
distinguishes them.
The two counters share one enumeration helper differing solely in the
include predicate, so they can never drift in which endpoints they walk.
Also adds a test pinning the split: a drained endpoint and an endpoint with
a genuinely earned cooldown must each appear in exactly one of the two
metrics, while both stay excluded from selection.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The drain did not work in production for more than one session, and reported success the whole time. DrainDomain resolved the target to a fixed set of EndpointKeys and benched those. EndpointAddr is `supplierAddr-url`, and a session rotates its supplier set every rollover (~20 min), so the benched keys went stale while the live endpoints at that operator had never been benched. Selection picked them freely. path_endpoints_drained kept publishing the stale count, so it looked like it was holding. Observed on gnosis: path_endpoints_drained reported 60 and 160 endpoints benched on mainnet while /ready on the same pod showed 0 in cooldown for every operator, and every tumbled connection rebound straight back onto a "drained" operator (rpcgate->spacebelt, spacebelt->rpcgate, never the third operator). None of that was evidence about the operators; the drain simply was not in force. Drains are now stored as a PREDICATE -- (service, domain, rpc_type) -> expiry -- and evaluated at selection time against the endpoint's LIVE URL, in the shannon reputation filter. An endpoint rotated into the session at a drained operator is benched the moment it appears, and an unscored endpoint is benched too, since matching no longer depends on a score existing. This also removes the score overlay entirely: GetScore is raw again, so GetScoreRaw and IsDrained are gone, replaced by IsDomainDrained. The drain still never touches a Score, so the quality signal stays readable while an operator is benched, and path_endpoints_in_cooldown still reports only earned cooldowns. Empty rpc_type covers every protocol for the service. A drain can never empty the endpoint pool -- if it would, the drained endpoints are restored and it logs, because a drain is an operator preference, not a correctness constraint, and an outage is not a redistribution. Hostname and URL targets widen to the operator, since benching a single machine would be defeated by the next rotation. Testing: the previous tests all drained against a static cache inside one test body, so nothing rotated the keys and none of them could fail. The regression test now asserts the bench holds across a rotated supplier set, which is the property that actually broke. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The ban was completely inert in production while reporting success. filterByReputation builds its result by walking `cached`, but the drain block deleted from the `endpoints` map passed in -- which nothing reads after `cached` is built. So no endpoint was ever excluded, the Warn log never fired (the counter stayed 0), and both the API response and path_endpoints_drained reported the operator benched. Observed on eth: both brands banned in both environments, 2 connections carrying 16.5 f/s tumbled off rpcgate, and the traffic came straight back to rpcgate. Same on gnosis, where every tumbled connection rebound onto a "banned" operator. None of that was evidence about the operators. The drain now filters `cached` itself, so every downstream path -- including the pool-collapse guard -- operates on the surviving set. Testing: this is the third bug in this feature that shipped because the test asserted on a helper rather than on the value the caller receives. The new test drives filterByReputation end to end and asserts on the RETURNED map; verified it fails without the fix. Also pins that a drain is scoped to its rpc_type, and that banning every operator yields rather than emptying the pool. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three bugs shipped in the admin drain, all with passing tests, all the same
mistake: asserting on something the author wrote rather than on the value the
production caller receives. Score field that a storage refresh erased, key
snapshot that session rotation invalidated, and a map the filter did not
build its result from. Each reported success in production while excluding
nothing.
In all three the real question was identical -- does selection still return
this endpoint -- and there was no cheap way to ask it, so each fix grew unit
tests around the edges instead.
selectionScenario makes it one call:
s := newSelectionScenario(t, "gnosis", spacebeltA, rpcgateA, kaloriusA)
s.Drain("spacebelt.xyz", RPCType_WEBSOCKET, time.Hour)
s.AssertExcluded(RPCType_WEBSOCKET, spacebeltA)
s.RotateSuppliers(1)
s.AssertExcluded(RPCType_WEBSOCKET, spacebeltA)
RotateSuppliers is the important part: it re-fronts each backend URL with new
supplier addresses while leaving the URLs alone, which is what a session
rollover does. Anything keyed on EndpointAddr silently goes stale there, and
the key-snapshot drain passed every test until this was expressible.
harnessEndpoint keeps supplier and URL separate for that reason; mockEndpoint
collapses them into one field, which is part of why the bug was invisible.
Six tests covering all three shipped bugs plus rpc_type scoping, release, the
never-empty-the-pool guard, and the independence of an earned cooldown from an
admin bench. Verified 5 of 6 fail against bug 3 by reverting the fix.
Also adds the three rules to CLAUDE.md: assert on the caller's return value,
revert-and-confirm-red before committing a fix, and treat two observables
disagreeing about one state as a bug rather than picking the convenient one.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The rebind path dropped the endpoint TCP socket without a WebSocket close
handshake. To the endpoint that is a peer vanishing mid-read, and rebinds
fire on EVERY session rollover, so this filled operator logs continuously
across the whole fleet.
Confirmed independently by two operators running different clients:
geth websocket: bad close code 1006 ... api=websocket-server
Nethermind An exception caused the WebSocket to enter the Aborted state
(Failed: 0)
Both were reported as suspected node problems. Neither was; it is PATH.
Normal shutdown already sends a close frame (bridge.go) -- only the rebind
path skipped it, which is why the noise looked constant rather than
occasional. Note #521 fixed 1006 in the CLIENT direction; the endpoint
direction was never covered.
Sends 1000 Normal Closure: a rebind is an orderly move, not a fault on the
endpoint's part, and a 1006 misattributes our routing decision as their
error. Best effort with a 1s deadline -- a rollover often begins because the
endpoint closed on us, and a rebind must not stall being polite to a socket
that is already gone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The admin-drain work added IsDomainDrained/DrainDomain to ReputationService but only updated the mocks in the packages whose tests were run at the time, so qos/evm has been failing to compile since. Stubs only: drains are asserted through protocol/shannon's selection harness, which checks what selection actually returns rather than what a mock was told. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
…ket drop Node operators reported a continuous stream of `websocket: close 1006 (abnormal closure): unexpected EOF` (geth) and `An exception caused the WebSocket to enter the Aborted state (Failed: 0)` (Nethermind), all tagged connection_source=gateway. The previous fix addressed the session-rebind path, which was a real instance of the same bug but the wrong site by two orders of magnitude: rebinds run at 0.6/s fleetwide while the websocket health-check probe dials, measures and closes at ~100/s. Every one of those probes ended in a bare conn.Close(), which sends no close frame, so the endpoint's read fails mid-frame and it reports the peer as having vanished. The probe was added recently, which is why the noise appeared to start from nothing. These are our teardowns being logged as the endpoint's fault, and they are indistinguishable in an operator's logs from a real client disconnect — so the signal an operator would use to find an actual problem was buried under ~8.9M synthetic abnormal closures a day. CloseEndpointConn centralises the handshake and is best-effort by construction: the write is unchecked and deadline-bounded, because a teardown often begins BECAUSE the peer already went away and a rebind must not stall being polite to a socket that is not there. Applied to the probe, to the two subscription-replay failure paths that dropped a freshly dialled connection, and the rebind path now routes through it rather than repeating the sequence inline. 1000 Normal Closure throughout: none of these are faults on the endpoint's part, and telling it otherwise misattributes our own routing decisions as its errors. The test asserts on what the ENDPOINT observes, via a real websocket server — asserting that a close helper was called would prove nothing, since the defect was precisely that a bare Close() produces a read error rather than a close frame on the peer. Reverting the fix reproduces the reported string exactly. Known gap, not addressed here: http.Server.Shutdown does not close hijacked connections, so a rollout still drops every live websocket without a handshake. That is one burst per deploy rather than a continuous rate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
… them http.Server.Shutdown explicitly does not close hijacked connections, and every websocket is hijacked, so nothing in the termination path touched them: the process exited, every socket died with its TCP connection, and both peers saw an abnormal closure. The client could not tell a deploy from a crash, and the endpoint logged a fault it did not cause — one 1006 burst per rollout, every service and application address inside the same second. The sweep snapshots the registry's controllers and releases the lock BEFORE closing anything. bridge.Close runs shutdown(), which deregisters, which takes the same mutex for writing; closing while holding even the read lock self-deadlocks. Tumble gets away with holding it only because it is a non-blocking channel send. The test that covers this deregisters from inside Close exactly as production does — a fake that only records the call cannot observe the deadlock at all, and reverting to the naive version hangs it. Closes run concurrently under a five-second ceiling. Serially, at a one-second write deadline per frame, a busy replica would overrun its 30s termination grace period and get SIGKILLed, producing precisely the abrupt teardown this is meant to prevent. The returned count is what completed rather than what was attempted, so a sweep that timed out half way cannot read as a clean one. Clients get 1012 Service Restart, which exists for this and tells them to come back — onto a replica that is not terminating. Endpoints get an orderly close rather than a truncated read. Behind an interface assertion rather than a method on gateway.Protocol: holding live bridges is a property of the implementation, and a protocol that holds none needs nothing here. Also fixes a pre-existing data race in the idle-reaper tests, where the bridge goroutine outlived the test and kept reading the package vars that cleanup restored — it failed -race for the whole package, which is coverage these changes should not ship without. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
shutdown() computed one close code and wrote the same frame to both peers. That is right when propagating (the endpoint's 4000 at session expiry should reach the client verbatim) and wrong for anything PATH initiates, because PATH is a different role to each side: external client <--(PATH is the server)-- PATH --(PATH is the client)--> relay miner RFC 6455 defines 1011/1012/1013 as things a SERVER tells a client. Sent upstream they invert the roles: "service restarting, please reconnect" addressed to the relay miner asks it to reconnect to us, which is not something it does, and "internal server error" reports our own fault as though the endpoint had one. The gateway-shutdown path added in the previous commit would have sent exactly that. The endpoint now gets 1001 Going Away, which RFC 6455 defines for both directions and which describes what happened: the peer that dialed you is leaving. The client still gets 1012, which is correct and useful facing that way — it tells the client to come back, onto a replica that is not terminating. Codes meaning the same thing in both directions, and application codes (3000-4999), pass through untouched. Not a protocol error either way — gorilla accepts 1012 on read. This is about the operator on the other end being told something true. The test reads the close frame off BOTH ends of one live bridge, since the defect is precisely that the directions were not distinguished; either side examined alone passed before the fix. Also waits for the bridge goroutine to exit, like the idle tests: it reads package vars other tests restore, and leaving it running races them under -race. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
…ound to The drain exempted requestedEndpointAddr, and every websocket path supplies one. ReconnectEndpoint passes preferredAddr — the endpoint the connection is ALREADY bound to — so the exemption fired on exactly the connections a drain exists to move. A websocket connection re-selects at every session rollover, and each of those rollovers re-picked the drained endpoint because it was "preferred". Measured in production: drain applied fleet-wide, path_endpoints_drained reporting 170 endpoints benched, every connection tumbled, and four minutes later the drained operator still served 73% of the service's websocket frames. Sticky placement is the thing a drain overrides; it cannot also be the thing that protects an endpoint from one. The pool-empty branch is the real safety net and is sufficient on its own — an endpoint is kept only when dropping it would leave nothing to serve. Covered by its own test rather than assumed. Why three passing tests missed this: the harness's Survivors() hardcodes "" for requestedEndpointAddr, which is the one call shape the websocket path never uses. SurvivorsPreferring closes that gap, and the new test fails against the old code with the production symptom. This is the fourth defect in this feature with an identical shape — reports success, excludes nothing — and the fourth where the test asserted on something other than the value the production caller receives. Two things that made the diagnosis slower and are worth recording: drain propagation was never the cause (storage refresh is 5s, sixty seconds before the tumble), and grepping pod logs for the drain's Warn diagnostics returned zero on every pod because LOG_LEVEL=error suppresses them — the instrument, not the signal. This feature's only diagnostics are invisible in production. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
…y service blocked_domains (gateway_config) permanently excludes every endpoint at a domain — eTLD+1 or exact hostname — from the given RPC types on ALL services. PATH_BLOCKED_DOMAINS appends entries at pod-restart speed (union: env can widen a ban, never narrow one). A malformed entry refuses to boot instead of silently narrowing the ban. Where a drain is temporary, per-service, and yields rather than empty the pool, this is the nuclear option, and every property follows from that: - Covers every path that hands out endpoints: session selection (which feeds HTTP, WebSocket bind/rebind, retry/hedge/batch), fallback endpoints (which bypass every session-endpoint filter and needed explicit coverage), and health checks (paid relays — the ban stops the probes too). - Runs before the supplier allowlist, so Target-Suppliers cannot override it — the header does bypass drains, which is a documented gap. - No preferred-endpoint exemption, and matching is on the live URL, so it holds across session rollovers by construction (drain bugs 4 and 2). - It can empty the pool: failing the request beats serving it from an operator the config says must never serve it. path_blocked_domains_configured reports the loaded entries at any LOG_LEVEL; path_endpoints_domain_blocked_total counts actual removals per selection pass — configured-but-never-engaging is the lying-instrument signature that shipped four drain bugs, so the two must be read together. Tests assert through the production callers (getSessionsUniqueEndpoints, getUniqueEndpoints, GetEndpointsForHealthCheck), and each of the three call sites was revert-checked: filter removed, tests fail, filter restored. Known limit: a ban covering only a subset of the HTTP-carried types leaves the endpoint's other HTTP health probes running (the executor keys HTTP checks on endpoint address, not per-type URL). All-type and websocket bans are exact. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every blocklist test hand-built Protocol{blockedDomains: ...}, leaving the
constructor wiring — env merge, compile, field assignment — with zero
coverage: deleting the assignment kept all tests green while shipping an
inert ban, the exact shape of all four drain bugs.
Two tests drive the real constructor: one proves a ban present only in
PATH_BLOCKED_DOMAINS excludes through selection on the constructed
instance (the field being set is necessary but not sufficient), one proves
a malformed env entry refuses to boot. Revert-checked: deleting the
assignment fails the first test and only it.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The drain's "not airtight — unscored endpoints stay selectable" limitation no longer exists. The predicate rewrite moved the check onto the endpoint's live URL and ahead of GetScores, so an endpoint carrying no score is excluded like any other. Kept the history so the score-based bench is not reintroduced, and replaced the limitation with the constraint that is actually load-bearing: a drain is rollover-paced. Endpoints leave the selectable set at once, but a bound websocket connection only moves at its own next rebind. Measured in production today: 42 connections cleared in ~19 min and 26 in ~11 min, on rollover alone with no tumble. path_supplier_exhausted_total is no longer 0 fleetwide. It fires on exactly one service, bursting 0 to ~11/s across ~4h with long zero troughs between. The earlier reading was an instant query that landed in a trough — the same mistake recurred while checking it today, so the file now says to read it over a range. The conclusion it supported is unchanged: on every other service the brake is dormant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
Test fixtures, code comments and configuration examples named real operator domains. This repository is public, so those names were about to become a permanent record of which providers were blocked and from what. Replaced throughout with neutral placeholders (op-alpha.example ... and matching Go identifiers). Nothing about the assertions depends on the strings being real domains: the blocklist matches on eTLD+1 or exact hostname, and the placeholders exercise both, including the mixed-case path that proves case-insensitive resolution. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S993uPDkQAbhpEAdMupmAP
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.
Twenty-two commits that accumulated on this branch. Grouped by area below.
Health checks
JSONRPCMethod, so the heuristic saw the method as"/"and the CometBFThealthcarve-out never fired — 16 cosmos services failed thehealthcheck 100% of the time. A uniform penalty distorts no ranking and triggers no alert, which is why it went unnoticed.Operator controls
POST /admin/reputation/drain/{serviceId}— temporarily bench every scored endpoint of one operator for a service, to answer "where would this traffic go if operator X were unavailable" without waiting for X to fail. Fleet-wide via shared storage, hard expiry ceiling, reported on its own gauge so a deliberate bench never reads as a quality incident.blocked_domains(gateway config,PATH_BLOCKED_DOMAINSto widen at restart) — permanently excludes every endpoint at a domain from given RPC types across all services. Where a drain is temporary, per-service, and yields rather than empty a pool, this does not yield. Covers session selection, fallback endpoints, and health checks; runs before the supplier allowlist soTarget-Supplierscannot override it. A malformed entry refuses to boot rather than silently narrowing the ban.Six follow-up commits fix defects found while validating the drain in production, all the same shape — reported success while excluding nothing. Root cause each time: the test asserted on a value other than the one the production caller receives.
selection_harness_test.gocloses that gap by asserting through the real selection path, including across session rotation and with a preferred endpoint supplied.WebSocket
1006flood that traced to our probe, not to their infrastructure.Docs
Corrected two claims in
CLAUDE.mdthat had gone stale, and replaced real operator domains in test fixtures and examples with neutral placeholders.Testing
go build ./...clean; unit tests pass.reputation/storagetestcontainer tests require Docker and were not run locally.