From bc99726f5c8994119822fabf92f8fae6da339324 Mon Sep 17 00:00:00 2001 From: Luke Tornquist Date: Thu, 5 Nov 2026 11:57:01 -0500 Subject: [PATCH] docs(agents): add Review-Proven Rule 9, design client paths for the whole fleet (#9106) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## What Adds Review-Proven Rule 9 to `AGENTS.md`: **design every client-driven path for the whole fleet running it at once.** Before an author adds or changes a connect, reconnect, renewal, poll, or catch-up path, they: - count the relay reads it costs per client, and keep reconnect to a delta from the client's cursor - jitter its start and back off with a cap - prefer an event-driven refresh with a long backstop over a short poll - on the relay, check every cache on a connect or auth path at a 0% hit rate, with single-flight misses that survive a saturated pool ## Why Relay load tests keep finding the same class of defect. A path is cheap for one client, but when a rollout drain, node eviction, relay restart, or network blip makes every client run it in the same few seconds, it collapses the relay. Each bullet cites a merged PR that fixed an instance: #4542, #8846, #9012, block/buzz-app#2215 and #2308. Unlike rules 1–8, this one comes from load tests rather than review mining, and the rule text says so. The section intro and the Reviewing bullet now say the rules list includes non-review sources. ## What got simpler Nothing in code. This is one rule added to the existing list. It turns a recurring load-test finding into a check made at planning and review time, so it doesn't keep resurfacing as a late load-test result. ## Validation Docs-only. I checked the diff, the Markdown rendering, and every cited PR number and title. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Luke Tornquist Co-authored-by: Elrond <28d6302a099e5225b02c4155ac4236e4912603df2ab08dbfc2f4fef08ce598c8@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 --- AGENTS.md | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index ec7e7f87cf..c7b6d363ac 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,7 +59,7 @@ confirmation, not app runs. - Before reviewing, read [VISION.md](VISION.md), the `VISION_*.md` docs for the affected surface, and the PR's stated goal and linked issue. Review the change against what it is trying to do. -- Check the change against the [Review-Proven Rules](#review-proven-rules): the defects reviewers here find most often. +- Check the change against the [Review-Proven Rules](#review-proven-rules): the defects reviewers and load tests here find most often. - Judge minimalism, elegance, and correctness, aiming for 9/10 on each. A score below 9 names the concrete defect and the fix. - Recommend blocking only for concrete correctness, security, or agreed-contract @@ -214,7 +214,8 @@ once flagged: test-seam binding and unbounded-resource findings were fixed **100%** of the time, swallowed-error findings **90%**, stale-state races **70%** — these are not style opinions, they are defects authors agree with on sight. Authors apply them **before writing code**, and reviewers check -against them; each cites the PRs where reviewers litigated it. +against them; each cites the PRs where reviewers litigated it. Rules +mined from other sources, such as load tests, say so in their text. 1. **Every caught failure must leave a durable retry record or propagate.** Never catch-log-and-return-success (opt-out revocation permanently @@ -286,6 +287,32 @@ against them; each cites the PRs where reviewers litigated it. non-primary ones — that's where the defects were. (PRs #5958, #5972, #6793, #6860, #6862, #6908, #7006) +9. **Design every client-driven path for the whole fleet running it at + once.** This rule comes from relay load tests rather than review + mining. A rollout drain, node eviction, relay restart, or network blip + makes every connected client run its reconnect, renewal, and catch-up + paths in the same few seconds, so cost per client times clients is the + load that matters. Before adding or changing a connect, reconnect, + renewal, poll, or catch-up path: + - **Count its relay reads per client** and keep reconnect to a delta + from the client's cursor, not a rebuild. One replay filter per + channel meant ~306 filters per reconnect for a 300-channel client + (block/buzz-app#2308). + - **Jitter its start and back off on failure with a cap** (backoff: + see rule 4). Spread the server side of a mass close, too (drain close + jitter, PR #4542). + - **Prefer an event-driven refresh with a long backstop over a short + poll.** For example, a full channel-list re-read every 60 s became + every 15 min (block/buzz-app#2215). + - **On the relay, assume a 0% hit rate for every cache on a connect + or auth path.** Concurrent misses for one key share one read, with a + bounded wait and a fallback read if the leader fails or hangs. Date + each entry from when its read checked out a connection: stamped before + the pool wait, 1 s entries expired on insert and storms ran uncached; + stamped at insert, they would outlive their staleness bound (PRs + #8846, #9012). + (PRs #4542, #8846, #9012; block/buzz-app#2215, #2308) + --- ## Key Patterns